Merge pull request haskell#10261 from ffaf1/git-protocol-check

Add git:// protocol check
mpickering · Aug 20, 2024 · 34bef80 · 34bef80
2 parents 75711fc + e581306
commit 34bef80
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 0 deletions.
diff --git a/Cabal/src/Distribution/PackageDescription/Check.hs b/Cabal/src/Distribution/PackageDescription/Check.hs
@@ -684,6 +684,7 @@ checkSourceRepos rs = do
         checkP
           (isNothing repoLocation_)
           (PackageDistInexcusable MissingLocation)
+        checkGitProtocol repoLocation_
         checkP
           ( repoType_ == Just (KnownRepoType CVS)
               && isNothing repoModule_
@@ -722,6 +723,17 @@ checkMissingVcsInfo rs =
     repoTypeDirname Monotone = ["_MTN"]
     repoTypeDirname Pijul = [".pijul"]
 
+-- git:// lacks TLS or other encryption, see
+-- https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols#_the_cons_4
+checkGitProtocol
+  :: Monad m
+  => Maybe String -- Repository location
+  -> CheckM m ()
+checkGitProtocol mloc =
+  checkP
+    (fmap (isPrefixOf "git://") mloc == Just True)
+    (PackageBuildWarning GitProtocol)
+
 -- ------------------------------------------------------------
 -- Package and distribution checks
 -- ------------------------------------------------------------

diff --git a/Cabal/src/Distribution/PackageDescription/Check/Warning.hs b/Cabal/src/Distribution/PackageDescription/Check/Warning.hs
@@ -193,6 +193,7 @@ data CheckExplanation
   | UnrecognisedSourceRepo String
   | MissingType
   | MissingLocation
+  | GitProtocol
   | MissingModule
   | MissingTag
   | SubdirRelPath
@@ -355,6 +356,7 @@ data CheckExplanationID
   | CIUnrecognisedSourceRepo
   | CIMissingType
   | CIMissingLocation
+  | CIGitProtocol
   | CIMissingModule
   | CIMissingTag
   | CISubdirRelPath
@@ -496,6 +498,7 @@ checkExplanationId (NoLicenseFile{}) = CINoLicenseFile
 checkExplanationId (UnrecognisedSourceRepo{}) = CIUnrecognisedSourceRepo
 checkExplanationId (MissingType{}) = CIMissingType
 checkExplanationId (MissingLocation{}) = CIMissingLocation
+checkExplanationId (GitProtocol{}) = CIGitProtocol
 checkExplanationId (MissingModule{}) = CIMissingModule
 checkExplanationId (MissingTag{}) = CIMissingTag
 checkExplanationId (SubdirRelPath{}) = CISubdirRelPath
@@ -642,6 +645,7 @@ ppCheckExplanationId CINoLicenseFile = "no-license-file"
 ppCheckExplanationId CIUnrecognisedSourceRepo = "unrecognised-repo-type"
 ppCheckExplanationId CIMissingType = "repo-no-type"
 ppCheckExplanationId CIMissingLocation = "repo-no-location"
+ppCheckExplanationId CIGitProtocol = "git-protocol"
 ppCheckExplanationId CIMissingModule = "repo-no-module"
 ppCheckExplanationId CIMissingTag = "repo-no-tag"
 ppCheckExplanationId CISubdirRelPath = "repo-relative-dir"
@@ -964,6 +968,10 @@ ppExplanation MissingType =
   "The source-repository 'type' is a required field."
 ppExplanation MissingLocation =
   "The source-repository 'location' is a required field."
+ppExplanation GitProtocol =
+  "Cloning over git:// might lead to an arbitrary code execution "
+    ++ "vulnerability. Furthermore, popular forges like GitHub do "
+    ++ "not support it. Use https:// or ssh:// instead."
 ppExplanation MissingModule =
   "For a CVS source-repository, the 'module' is a required field."
 ppExplanation MissingTag =

diff --git a/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out b/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out
@@ -0,0 +1,6 @@
+# cabal check
+The following errors are likely to affect your build negatively:
+Error: [git-protocol] Cloning over git:// might lead to an arbitrary code
+execution vulnerability. Furthermore, popular forges like GitHub do not
+support it. Use https:// or ssh:// instead.
+Error: Hackage would reject this package.
diff --git a/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.test.hs b/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.test.hs
@@ -0,0 +1,3 @@
+import Test.Cabal.Prelude
+
+main = cabalTest $ fails $ cabal "check" []
diff --git a/cabal-testsuite/PackageTests/Check/GitProtocol/pkg.cabal b/cabal-testsuite/PackageTests/Check/GitProtocol/pkg.cabal
@@ -0,0 +1,16 @@
+cabal-version: 3.0
+name: pkg
+version: 0
+category: example
+maintainer: me@example.com
+synopsis: small synopsis
+description: longer description
+license: GPL-3.0-only
+
+library
+  exposed-modules: Foo
+  default-language: Haskell2010
+
+source-repository head
+  type: git
+  location: git://www.example.org/my-repo/
diff --git a/changelog.d/pr-10261 b/changelog.d/pr-10261
@@ -0,0 +1,12 @@
+synopsis: Warn about git:// protocol
+packages: cabal-install
+prs: #10261
+
+description: {
+
+`cabal check` will warn about insecure git:// protocol in `source-repository`.
+
+See [Git Book](https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols#_the_cons_4)
+for an explanation.
+
+}
diff --git a/doc/cabal-commands.rst b/doc/cabal-commands.rst
@@ -1327,6 +1327,9 @@ A list of all warnings with their constructor:
 - ``unrecognised-repo-type``: unrecognised kind of source-repository.
 - ``repo-no-type``: missing ``type`` in ``source-repository``.
 - ``repo-no-location``: missing ``location`` in ``source-repository``.
+- ``git-protocol``: using insecure ``git://`` protocol
+  (`explanation <https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols#_the_cons_4>`__
+  in Git Book).
 - ``repo-no-module``: missing ``module`` in ``source-repository``.
 - ``repo-no-tag``: missing ``tag`` in ``source-repository``.
 - ``repo-relative-dir``: ``subdir`` in ``source-repository`` must be relative.