gittest: Do not run commands in seed Git repositories directly

While we already ensure that no seed test Git repositories are being
written into by running tests under a different user in our CI, we don't
check whether the repositories are used to spawn any read-only commands.
And this has in fact been fine until now: permissions were such that
this always worked.

This has changed with CVE-2022-24765 though: Git has started to operate
in repositories completely in case it is owned by a different user. As a
consequence, this breaks our testing infrastructure whenever a test is
trying to run a command in the test seed repositories directly given
that they're owned by `root`, and we ourselves run with UID 9999 in our
CI.

Luckily, commands like git-upload-pack(1) still work, which means that
we're still able to clone such repositories. And there are only very few
tests which don't first use e.g. `gittest.CloneRepository()` before
actually run commands directly in there, too, because we don't expose a
simple way to obtain the path of those seed repositories outside of the
`gittest` package. The only offenders are `gittest.ChecksumTestRepo()`
and `gittest.BundleTestRepo()`.

Refactor those two helper functions to instead operate on a cloned
repository instead of running commands in the seed repositories
directly. This fixes compatibility with all Git versions which include a
fix for above CVE.

4 files changed, 20 insertions, 19 deletions

diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go
index d47221f6a..eecacaa58 100644
--- a/internal/backup/backup_test.go
+++ b/internal/backup/backup_test.go
@@ -301,8 +301,12 @@ func testManagerRestore(t *testing.T, ctx context.Context) {
 		return repo
 	}
 
+	_, repoPath := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
+		Seed: gittest.SeedGitLabTest,
+	})
+	repoChecksum := gittest.ChecksumRepo(t, cfg, repoPath)
+
 	path := testhelper.TempDir(t)
-	testRepoChecksum := gittest.ChecksumTestRepo(t, cfg, "gitlab-test.git")
 
 	for _, tc := range []struct {
 		desc          string
@@ -320,9 +324,9 @@ func testManagerRestore(t *testing.T, ctx context.Context) {
 				repo := createRepo(t)
 				require.NoError(t, os.MkdirAll(filepath.Join(path, repo.RelativePath), os.ModePerm))
 				bundlePath := filepath.Join(path, repo.RelativePath+".bundle")
-				gittest.BundleTestRepo(t, cfg, "gitlab-test.git", bundlePath)
+				gittest.BundleRepo(t, cfg, repoPath, bundlePath)
 
-				return repo, testRepoChecksum
+				return repo, repoChecksum
 			},
 			expectExists: true,
 		},
@@ -334,10 +338,10 @@ func testManagerRestore(t *testing.T, ctx context.Context) {
 				bundlePath := filepath.Join(path, repo.RelativePath+".bundle")
 				customHooksPath := filepath.Join(path, repo.RelativePath, "custom_hooks.tar")
 				require.NoError(t, os.MkdirAll(filepath.Join(path, repo.RelativePath), os.ModePerm))
-				gittest.BundleTestRepo(t, cfg, "gitlab-test.git", bundlePath)
+				gittest.BundleRepo(t, cfg, repoPath, bundlePath)
 				testhelper.CopyFile(t, "../gitaly/service/repository/testdata/custom_hooks.tar", customHooksPath)
 
-				return repo, testRepoChecksum
+				return repo, repoChecksum
 			},
 			expectedPaths: []string{
 				"custom_hooks/pre-commit.sample",
@@ -376,9 +380,9 @@ func testManagerRestore(t *testing.T, ctx context.Context) {
 
 				require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(path, repo.RelativePath)), os.ModePerm))
 				bundlePath := filepath.Join(path, repo.RelativePath+".bundle")
-				gittest.BundleTestRepo(t, cfg, "gitlab-test.git", bundlePath)
+				gittest.BundleRepo(t, cfg, repoPath, bundlePath)
 
-				return repo, testRepoChecksum
+				return repo, repoChecksum
 			},
 			expectExists: true,
 		},
@@ -394,9 +398,9 @@ func testManagerRestore(t *testing.T, ctx context.Context) {
 				require.NoError(t, os.WriteFile(filepath.Join(repoBackupPath, "LATEST"), []byte(backupID), os.ModePerm))
 				require.NoError(t, os.WriteFile(filepath.Join(backupPath, "LATEST"), []byte("001"), os.ModePerm))
 				bundlePath := filepath.Join(backupPath, "001.bundle")
-				gittest.BundleTestRepo(t, cfg, "gitlab-test.git", bundlePath)
+				gittest.BundleRepo(t, cfg, repoPath, bundlePath)
 
-				return repo, testRepoChecksum
+				return repo, repoChecksum
 			},
 			expectExists: true,
 		},
diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go
index 1d606fa7f..020ae8271 100644
--- a/internal/git/gittest/repo.go
+++ b/internal/git/gittest/repo.go
@@ -315,22 +315,18 @@ func CloneRepo(t testing.TB, cfg config.Cfg, storage config.Storage, opts ...Clo
 	return repo, absolutePath
 }
 
-// BundleTestRepo creates a bundle of a local test repo. E.g.
-// `gitlab-test.git`. `patterns` define the bundle contents as per
+// BundleRepo creates a bundle of a repository. `patterns` define the bundle contents as per
 // `git-rev-list-args`. If there are no patterns then `--all` is assumed.
-func BundleTestRepo(t testing.TB, cfg config.Cfg, sourceRepo, bundlePath string, patterns ...string) {
+func BundleRepo(t testing.TB, cfg config.Cfg, repoPath, bundlePath string, patterns ...string) {
 	if len(patterns) == 0 {
 		patterns = []string{"--all"}
 	}
-	repoPath := testRepositoryPath(t, sourceRepo)
 	Exec(t, cfg, append([]string{"-C", repoPath, "bundle", "create", bundlePath}, patterns...)...)
 }
 
-// ChecksumTestRepo calculates the checksum of a local test repo. E.g.
-// `gitlab-test.git`.
-func ChecksumTestRepo(t testing.TB, cfg config.Cfg, sourceRepo string) *git.Checksum {
+// ChecksumRepo calculates the checksum of a repository.
+func ChecksumRepo(t testing.TB, cfg config.Cfg, repoPath string) *git.Checksum {
 	var checksum git.Checksum
-	repoPath := testRepositoryPath(t, sourceRepo)
 	lines := bytes.Split(Exec(t, cfg, "-C", repoPath, "show-ref", "--head"), []byte("\n"))
 	for _, line := range lines {
 		checksum.AddBytes(line)
diff --git a/internal/gitaly/service/repository/fetch_bundle_test.go b/internal/gitaly/service/repository/fetch_bundle_test.go
index f11327fce..4f30b9f18 100644
--- a/internal/gitaly/service/repository/fetch_bundle_test.go
+++ b/internal/gitaly/service/repository/fetch_bundle_test.go
@@ -101,7 +101,7 @@ func TestServer_FetchBundle_transaction(t *testing.T) {
 
 	tmp := testhelper.TempDir(t)
 	bundlePath := filepath.Join(tmp, "test.bundle")
-	gittest.BundleTestRepo(t, cfg, "gitlab-test.git", bundlePath)
+	gittest.BundleRepo(t, cfg, repoPath, bundlePath)
 
 	hookManager.Reset()
 	_, stopGitServer := gittest.HTTPServer(ctx, t, gitCmdFactory, repoPath, nil)
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index 54f07fe26..8b56ce272 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -839,7 +839,8 @@ func TestPostReceive_allRejected(t *testing.T) {
 			ctx context.Context,
 			repo *gitalypb.Repository,
 			pushOptions, env []string,
-			stdin io.Reader, stdout, stderr io.Writer) error {
+			stdin io.Reader, stdout, stderr io.Writer,
+		) error {
 			return errors.New("not allowed")
 		},
 		gitalyhook.NopPostReceive,