Fail when a git submodule fails to update instead of ignoring the error

.github/scripts/main/preamble.sh

@@ -32,6 +32,7 @@ git config --global user.email "gha@example.com"
 git config --global user.name "Github Actions CI"
 git config --global gc.autoDetach false
 git config --global init.defaultBranch thisShouldNotHappen
+git config --global protocol.file.allow always
 
 if [ -d ~/opam-repository ]; then
   OPAM_REPO_CACHE=file://$HOME/opam-repository

master_changes.md

@@ -89,6 +89,7 @@ users)
 ## Sandbox
 
 ## VCS
+  * Fail when git submodule fails to update instead of showing a warning and ignoring the error [#6132 @kit-ty-kate - fix #6131]
 
 ## Build
  * Synchronise opam-core.opam with opam-repository changes [#6043 @dra27]
@@ -161,6 +162,7 @@ users)
   * Add a test filtering mechanism [#6105 @Keryan-dev]
   * Add a package fetching test [#6146 @rjbou]
   * Add a test showing the behaviour of `opam switch list-available` [#6098 @kit-ty-kate]
+  * Add a test for git packages with submodules [#6132 @kit-ty-kate]
 
 ### Engine
   * Add a test filtering mechanism [#6105 @Keryan-dev]
@@ -181,6 +183,7 @@ users)
   * Update action cache to v4 [#6081 @rjbou]
   * Update action checkout to v4 [#6081 @rjbou]
   * Update action upload-artifact to v4 [#6081 @rjbou]
+  * preamble: Allow local git submodules (ignore CVE-2022-39253) [#6132 @kit-ty-kate]
 
 ## Doc
   * Remove the ppa from the installation instructions on Ubuntu [#5988 @kit-ty-kate - fix #5987]

src/repository/opamGit.ml

@@ -179,9 +179,7 @@ module VCS : OpamVCS.VCS = struct
       if OpamFilename.exists (repo_root // ".gitmodules") then
         git repo_root [ "submodule"; "update"; "--init"; "--recursive" ]
         @@> fun r ->
-        if OpamProcess.is_failure r then
-          OpamConsole.warning "Git submodule update failed in %s"
-            (OpamFilename.Dir.to_string repo_root);
+        OpamSystem.raise_on_process_error r;
         Done ()
       else Done ()
 

tests/reftests/dune.inc

@@ -818,6 +818,27 @@
    %{targets}
    (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:filter-operators.test} %{read-lines:testing-env}))))
 
+(rule
+ (alias reftest-git)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (action
+  (diff git.test git.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (deps (alias reftest-git)))
+
+(rule
+ (targets git.out)
+ (deps root-N0REP0)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:git.test} %{read-lines:testing-env}))))
+
 (rule
  (alias reftest-init-ocaml-eval-variables.unix)
  (enabled_if (and (= %{os_type} "Unix") (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))

tests/reftests/git.test

@@ -0,0 +1,72 @@
+N0REP0
+### : Check the bevahiour when presented with a broken submodule
+### mkdir submodule
+### git -C ./submodule init -q
+### git -C ./submodule config core.autocrlf false
+### touch ./submodule/some-file
+### git -C ./submodule add ./some-file
+### git -C ./submodule commit -qm "first commit"
+### mkdir use-submodule
+### git -C ./use-submodule init -q
+### git -C ./use-submodule config core.autocrlf false
+### ### This test may fail locally depending on your git version
+### ### File protocol is removed locally since git 2.38,
+### ### to fix a CVE: https://www.cve.org/CVERecord?id=CVE-2022-39253
+### git -C ./use-submodule submodule add ../submodule ./vendor
+Cloning into '${BASEDIR}/use-submodule/vendor'...
+done.
+### git -C ./use-submodule commit -qm "first commit"
+### <create-submodule-pkg.sh>
+mkdir -p REPO/packages/submodule/submodule.1
+ESCAPED_BASEDIR=$(printf '%s' "$BASEDIR" | sed 's/\\/\\\\/g')
+cat > REPO/packages/submodule/submodule.1/opam << EOF
+opam-version: "2.0"
+build: ["ls" "vendor/some-file"]
+url {
+  src: "git+file://${ESCAPED_BASEDIR}/use-submodule"
+}
+EOF
+### sh ./create-submodule-pkg.sh
+### opam update
+
+<><> Updating package repositories ><><><><><><><><><><><><><><><><><><><><><><>
+[default] synchronised from file://${BASEDIR}/REPO
+Now run 'opam upgrade' to apply any package updates.
+### opam switch create submodule --empty
+### opam install submodule
+The following actions will be performed:
+=== install 1 package
+  - install submodule 1
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> retrieved submodule.1  (git+file://${BASEDIR}/use-submodule)
+-> installed submodule.1
+Done.
+### rm -r ./submodule
+### opam remove submodule
+The following actions will be performed:
+=== remove 1 package
+  - remove submodule 1
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> removed   submodule.1
+Done.
+### opam install submodule | '".+[/\\]git(\.exe)? ' -> '"git '
+The following actions will be performed:
+=== install 1 package
+  - install submodule 1
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+[ERROR] Could not synchronize ${BASEDIR}/OPAM/submodule/.opam-switch/sources/submodule.1 from "git+file://${BASEDIR}/use-submodule":
+        "git submodule update --init --recursive" exited with code 1
+[ERROR] Failed to get sources of submodule.1: git+file://${BASEDIR}/use-submodule
+
+OpamSolution.Fetch_fail("git+file://${BASEDIR}/use-submodule")
+
+
+<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
++- The following actions failed
+| - fetch submodule 1
++- 
+- No changes have been performed
+# Return code 40 #