| Message ID | 86a6rabl7a.fsf@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#46182] lint: Add 'check-git-protocol' checker. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/submitting builds | success | |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
On Thu, Mar 11, 2021 at 01:14:33AM +0100, zimoun wrote: > I propose ’match’ which is more coherent with the Guix style. Well, > from my understanding. :-) I have heard that before, but I don't know how to use it 🤷 If this new patch is working for you, please push!
Hi Leo, On Wed, 10 Mar 2021 at 20:46, Leo Famulari <leo@famulari.name> wrote: > On Thu, Mar 11, 2021 at 01:14:33AM +0100, zimoun wrote: >> I propose ’match’ which is more coherent with the Guix style. Well, >> from my understanding. :-) > > I have heard that before, but I don't know how to use it 🤷 The section [1] in the manual is worth to read because running and playing with the examples gives a good feeling on how to use it. :-) > If this new patch is working for you, please push! I do not have this power. :-) Cheers, simon
Hello, zimoun <zimon.toutoune@gmail.com> writes: > Hi Leo, > > On Wed, 10 Mar 2021 at 20:46, Leo Famulari <leo@famulari.name> wrote: >> On Thu, Mar 11, 2021 at 01:14:33AM +0100, zimoun wrote: >>> I propose ’match’ which is more coherent with the Guix style. Well, >>> from my understanding. :-) >> >> I have heard that before, but I don't know how to use it 🤷 > > The section [1] in the manual is worth to read because running and > playing with the examples gives a good feeling on how to use it. :-) > >> If this new patch is working for you, please push! > > I do not have this power. :-) No longer true ;-). Thinking about this change though; why is it bad to fetch from git places? There may be repos out there where it's the only offered way, and as long as we're talking fixed output derivations, it seems moot whether you use HTTPS, HTTP or X to retrieve the files (unless you are worried about your traffic being monitored, but that's not in scope, I'd say).
Hi Maxim, On Thu, 19 Oct 2023 at 22:22, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > Thinking about this change though; why is it bad to fetch from git > places? There may be repos out there where it's the only offered way, > and as long as we're talking fixed output derivations, it seems moot > whether you use HTTPS, HTTP or X to retrieve the files (unless you are > worried about your traffic being monitored, but that's not in scope, I'd > say). Why would not it be in scope? Being able to strongly verify (sha256) that the content you fetch is the data you expect does not imply that the protocol for communicating cannot be exploited for other means. Well, git:// protocol is not supported by well-known forges. Quoting Pro Git book: The Cons Due to the lack of TLS or other cryptography, cloning over git:// might lead to an arbitrary code execution vulnerability, and should therefore be avoided unless you know what you are doing. https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols And I do not have enough imagination to find a way to exploit the git:// protocol. However, it appears to me a good practise to warn when this protocol is used. Somehow, a lint message is a recommendation – a good practise – and not an absolute truth. :-) In short, from my point of view, the general rule reads: avoid git:// protocol if you can. Obviously, if you cannot because it is the only offered way by some repositories, then let make an exception; but it does mean that’s a good practise. Cheers, simon
Hi, Simon Tournier <zimon.toutoune@gmail.com> writes: > Hi Maxim, > > On Thu, 19 Oct 2023 at 22:22, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > >> Thinking about this change though; why is it bad to fetch from git >> places? There may be repos out there where it's the only offered way, >> and as long as we're talking fixed output derivations, it seems moot >> whether you use HTTPS, HTTP or X to retrieve the files (unless you are >> worried about your traffic being monitored, but that's not in scope, I'd >> say). > > Why would not it be in scope? > > Being able to strongly verify (sha256) that the content you fetch is the > data you expect does not imply that the protocol for communicating > cannot be exploited for other means. > > Well, git:// protocol is not supported by well-known forges. Quoting > Pro Git book: > > The Cons > > Due to the lack of TLS or other cryptography, cloning over > git:// might lead to an arbitrary code execution vulnerability, > and should therefore be avoided unless you know what you are > doing. > > https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols > > And I do not have enough imagination to find a way to exploit the git:// > protocol. However, it appears to me a good practise to warn when this > protocol is used. Somehow, a lint message is a recommendation – a good > practise – and not an absolute truth. :-) > > In short, from my point of view, the general rule reads: avoid git:// > protocol if you can. Obviously, if you cannot because it is the only > offered way by some repositories, then let make an exception; but it > does mean that’s a good practise. OK, fair. I remove my objection, but I dislike warnings when they cannot be acted upon (e.g. 'no coverage in software heritage' -- OK neat, but I can't do anything about it, and it may not even support that tarball ingestion yet).
diff --git a/guix/lint.scm b/guix/lint.scm index 311bc94cc3..980f77c736 100644 --- a/guix/lint.scm +++ b/guix/lint.scm @@ -51,7 +51,7 @@ #:use-module (guix gnu-maintenance) #:use-module (guix cve) #:use-module ((guix swh) #:hide (origin?)) - #:autoload (guix git-download) (git-reference? + #:autoload (guix git-download) (git-reference? git-fetch git-reference-url git-reference-commit) #:use-module (guix import stackage) #:use-module (ice-9 match) @@ -84,6 +84,7 @@ check-source check-source-file-name check-source-unstable-tarball + check-git-protocol check-mirror-url check-github-url check-license @@ -918,6 +919,26 @@ descriptions maintained upstream." (origin-uris origin)) '()))) +(define (check-git-protocol package) + "Emit a warning if PACKAGE's source URI protocol is 'git://'." + (define (check-source-uri-scheme uri) + (match (uri-scheme uri) + ('git + (list + (make-warning package + (G_ "the source URI should not use the git:// protocol") + #:field 'source))) + (_ '()))) + + (match (package-source package) + ((? origin? origin) + (match (origin-method origin) + (git-fetch + (check-source-uri-scheme + (string->uri (git-reference-url (origin-uri origin))))) + (_ '()))) + (_ '()))) + (define (check-mirror-url package) "Check whether PACKAGE uses source URLs that should be 'mirror://'." (define (check-mirror-uri uri) ;XXX: could be optimized @@ -1476,6 +1497,10 @@ or a list thereof") (name 'source-unstable-tarball) (description "Check for autogenerated tarballs") (check check-source-unstable-tarball)) + (lint-checker + (name 'git-protocol) + (description "Check for use of the git:// protocol") + (check check-git-protocol)) (lint-checker (name 'derivation) (description "Report failure to compile a package to a derivation")