Merge remote-tracking branch 'dev/master'

14 files changed, 351 insertions, 73 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 175c35650..c971018cb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Gitaly changelog
 
+## 15.2.1 (2022-07-28)
+
+### Security (1 change)
+
+- [Import via git protocol allows to bypass checks on repository](gitlab-org/security/gitaly@d7af133ce2c05dcfc93ebf15a47154a0cd40e9bd) ([merge request](gitlab-org/security/gitaly!56))
+
 ## 15.2.0 (2022-07-21)
 
 ### Added (4 changes)
@@ -50,6 +56,12 @@
 
 - [linguist: Implement Stats in pure Go](gitlab-org/gitaly@efd9a598f50e03f05620b56f2e010600128f3b1c) ([merge request](gitlab-org/gitaly!4580))
 
+## 15.1.4 (2022-07-28)
+
+### Security (1 change)
+
+- [Import via git protocol allows to bypass checks on repository](gitlab-org/security/gitaly@f8909dcbab9ea2ddabad06577a1c2ae50ac5f8f3) ([merge request](gitlab-org/security/gitaly!54))
+
 ## 15.1.3 (2022-07-19)
 
 No changes.
@@ -100,6 +112,12 @@ No changes.
 
 - [repository: Use long-running filter process for converting LFS pointers](gitlab-org/gitaly@0603c758d6e3580adbd3d0d69e326d05baa340b9) ([merge request](gitlab-org/gitaly!4595))
 
+## 15.0.5 (2022-07-28)
+
+### Security (1 change)
+
+- [Import via git protocol allows to bypass checks on repository](gitlab-org/security/gitaly@a110173b56e3bc8c7cb5db2e1e84f6cfb226c39e) ([merge request](gitlab-org/security/gitaly!55))
+
 ## 15.0.4 (2022-06-30)
 
 No changes.
diff --git a/internal/git/command_description.go b/internal/git/command_description.go
index bfc91117c..2ecd060b3 100644
--- a/internal/git/command_description.go
+++ b/internal/git/command_description.go
@@ -53,12 +53,12 @@ var commandDescriptions = map[string]commandDescription{
 		flags: scNoEndOfOptions,
 	},
 	"clone": {
-		opts: append([]GlobalOption{
+		opts: append(append([]GlobalOption{
 			// See "init" for why we set the template directory to the empty string.
 			ConfigPair{Key: "init.templateDir", Value: ""},
 			// See "fetch" for why we disable following redirects.
 			ConfigPair{Key: "http.followRedirects", Value: "false"},
-		}, packConfiguration()...),
+		}, packConfiguration()...), fsckConfiguration("fetch")...),
 	},
 	"commit": {
 		flags: 0,
@@ -298,12 +298,12 @@ var commandDescriptions = map[string]commandDescription{
 	},
 	"upload-pack": {
 		flags: scNoRefUpdates,
-		opts: append(append([]GlobalOption{
+		opts: append([]GlobalOption{
 			ConfigPair{Key: "uploadpack.allowFilter", Value: "true"},
 			// Enables the capability to request individual SHA1's from the
 			// remote repo.
 			ConfigPair{Key: "uploadpack.allowAnySHA1InWant", Value: "true"},
-		}, hiddenUploadPackRefPrefixes()...), packConfiguration()...),
+		}, packConfiguration()...),
 	},
 	"version": {
 		flags: scNoRefUpdates,
diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go
index 210288dd4..23506f192 100644
--- a/internal/git/command_factory.go
+++ b/internal/git/command_factory.go
@@ -16,6 +16,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/log"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 )
 
 // CommandFactory is designed to create and run git commands in a protected and fully managed manner.
@@ -451,6 +452,11 @@ func (cf *ExecCommandFactory) combineArgs(ctx context.Context, gitConfig []confi
 	}
 
 	combinedGlobals = append(combinedGlobals, commandDescription.opts...)
+
+	if sc.Subcommand() == "upload-pack" && featureflag.UploadPackHideRefs.IsEnabled(ctx) {
+		combinedGlobals = append(combinedGlobals, hiddenUploadPackRefPrefixes()...)
+	}
+
 	combinedGlobals = append(combinedGlobals, cc.globals...)
 	for _, configPair := range gitConfig {
 		combinedGlobals = append(combinedGlobals, ConfigPair{
diff --git a/internal/git/hooks_options_test.go b/internal/git/hooks_options_test.go
index 24ebed76b..384c3c5ed 100644
--- a/internal/git/hooks_options_test.go
+++ b/internal/git/hooks_options_test.go
@@ -11,6 +11,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	grpcmetadata "google.golang.org/grpc/metadata"
@@ -74,6 +75,9 @@ func TestWithPackObjectsHookEnv(t *testing.T) {
 
 	ctx = grpcmetadata.AppendToOutgoingContext(ctx, "user_id", userID, "username", username)
 	ctx = metadata.OutgoingToIncoming(ctx)
+	// We don't care about this feature flag as it doesn't impact behaviour of the system under
+	// test.
+	ctx = featureflag.IncomingCtxWithFeatureFlag(ctx, featureflag.UploadPackHideRefs, true)
 
 	cmd, err := gittest.NewCommandFactory(t, cfg, git.WithSkipHooks()).New(ctx, repo, subCmd, opt)
 	require.NoError(t, err)
diff --git a/internal/git/objectpool/clone.go b/internal/git/objectpool/clone.go
index 79a88a035..45a48f864 100644
--- a/internal/git/objectpool/clone.go
+++ b/internal/git/objectpool/clone.go
@@ -1,7 +1,9 @@
 package objectpool
 
 import (
+	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 
@@ -17,6 +19,7 @@ func (o *ObjectPool) clone(ctx context.Context, repo *localrepo.Repo) error {
 		return err
 	}
 
+	var stderr bytes.Buffer
 	cmd, err := o.gitCmdFactory.NewWithoutRepo(ctx,
 		git.SubCmd{
 			Name: "clone",
@@ -28,12 +31,17 @@ func (o *ObjectPool) clone(ctx context.Context, repo *localrepo.Repo) error {
 			Args: []string{repoPath, o.FullPath()},
 		},
 		git.WithRefTxHook(repo),
+		git.WithStderr(&stderr),
 	)
 	if err != nil {
-		return err
+		return fmt.Errorf("spawning clone: %w", err)
+	}
+
+	if err := cmd.Wait(); err != nil {
+		return fmt.Errorf("cloning to pool: %w, stderr: %q", err, stderr.String())
 	}
 
-	return cmd.Wait()
+	return nil
 }
 
 func (o *ObjectPool) removeHooksDir() error {
diff --git a/internal/git/objectpool/clone_test.go b/internal/git/objectpool/clone_test.go
index 97e9a26ff..4c6d5b0c0 100644
--- a/internal/git/objectpool/clone_test.go
+++ b/internal/git/objectpool/clone_test.go
@@ -3,40 +3,88 @@
 package objectpool
 
 import (
+	"bytes"
+	"fmt"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 )
 
-func TestClone(t *testing.T) {
+func TestClone_successful(t *testing.T) {
+	t.Parallel()
+
 	ctx := testhelper.Context(t)
 
 	cfg, pool, repoProto := setupObjectPool(t, ctx)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 	require.NoError(t, pool.clone(ctx, repo))
-	defer func() {
-		require.NoError(t, pool.Remove(ctx))
-	}()
 
 	require.DirExists(t, pool.FullPath())
 	require.DirExists(t, filepath.Join(pool.FullPath(), "objects"))
 }
 
-func TestCloneExistingPool(t *testing.T) {
+func TestClone_existingPool(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+
+	cfg, pool, repoProto := setupObjectPool(t, ctx)
+	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+
+	// The first time around cloning should succeed, but ...
+	require.NoError(t, pool.clone(ctx, repo))
+
+	// ... when we try to clone the same pool a second time we should get an error because the
+	// destination exists already.
+	require.EqualError(t, pool.clone(ctx, repo), fmt.Sprintf(
+		"cloning to pool: exit status 128, stderr: \"fatal: destination path '%s' already exists and is not an empty directory.\\n\"",
+		pool.FullPath(),
+	))
+}
+
+func TestClone_fsck(t *testing.T) {
+	t.Parallel()
+
 	ctx := testhelper.Context(t)
 
 	cfg, pool, repoProto := setupObjectPool(t, ctx)
+
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+	repoPath, err := repo.Path()
+	require.NoError(t, err)
+
+	// Write a tree into the repository that's known-broken.
+	treeID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+		{Content: "content", Path: "dup", Mode: "100644"},
+		{Content: "content", Path: "dup", Mode: "100644"},
+	})
 
+	gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithParents(),
+		gittest.WithBranch("main"),
+		gittest.WithTree(treeID),
+	)
+
+	// While git-clone(1) would normally complain about the broken tree we have just cloned, we
+	// don't expect the clone to fail. This is because we know that the tree is already in one
+	// of our repositories that we have locally, so raising an error now doesn't make a whole
+	// lot of sense in the first place.
+	//
+	// Note: this works because we use `git clone --local`, which only creates a copy of the
+	// repository without performing consistency checks.
 	require.NoError(t, pool.clone(ctx, repo))
-	defer func() {
-		require.NoError(t, pool.Remove(ctx))
-	}()
 
-	// re-clone on the directory
-	require.Error(t, pool.clone(ctx, repo))
+	// Verify that the broken tree is indeed in the pool repository and that it is reported as
+	// broken by git-fsck(1).
+	var stderr bytes.Buffer
+	fsckCmd := gittest.NewCommand(t, cfg, "-C", pool.FullPath(), "fsck")
+	fsckCmd.Stderr = &stderr
+
+	require.EqualError(t, fsckCmd.Run(), "exit status 1")
+	require.Equal(t, fmt.Sprintf("error in tree %s: duplicateEntries: contains duplicate file entries\n", treeID), stderr.String())
 }
diff --git a/internal/gitaly/service/repository/archive_test.go b/internal/gitaly/service/repository/archive_test.go
index 5f18f8679..e897af1cf 100644
--- a/internal/gitaly/service/repository/archive_test.go
+++ b/internal/gitaly/service/repository/archive_test.go
@@ -5,6 +5,7 @@ package repository
 import (
 	"archive/zip"
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -19,6 +20,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitlab"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
@@ -165,10 +167,13 @@ func TestGetArchive_success(t *testing.T) {
 	}
 }
 
-func TestGetArchive_includeLfsBlobs(t *testing.T) {
+func TestGetArchiveIncludeLfsBlobs(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testGetArchiveIncludeLfsBlobs)
+}
 
-	ctx := testhelper.Context(t)
+func testGetArchiveIncludeLfsBlobs(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	defaultOptions := gitlab.TestServerOptions{
 		SecretToken: secretToken,
diff --git a/internal/gitaly/service/repository/create_fork.go b/internal/gitaly/service/repository/create_fork.go
index cbdff0231..ea9fd3949 100644
--- a/internal/gitaly/service/repository/create_fork.go
+++ b/internal/gitaly/service/repository/create_fork.go
@@ -49,6 +49,12 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest
 			},
 		}, git.WithInternalFetch(&gitalypb.SSHUploadPackRequest{
 			Repository: sourceRepository,
+		}), git.WithConfig(git.ConfigPair{
+			// Disable consistency checks for fetched objects when creating a
+			// fork. We don't want to end up in a situation where it's
+			// impossible to create forks we already have anyway because we have
+			// e.g. retroactively tightened the consistency checks.
+			Key: "fetch.fsckObjects", Value: "false",
 		}), git.WithDisabledHooks())
 		if err != nil {
 			return fmt.Errorf("spawning fetch: %w", err)
diff --git a/internal/gitaly/service/repository/create_fork_test.go b/internal/gitaly/service/repository/create_fork_test.go
index a58ca9b28..cf140b9ae 100644
--- a/internal/gitaly/service/repository/create_fork_test.go
+++ b/internal/gitaly/service/repository/create_fork_test.go
@@ -3,8 +3,10 @@
 package repository
 
 import (
+	"bytes"
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -160,6 +162,62 @@ func TestCreateFork_refs(t *testing.T) {
 	)
 }
 
+func TestCreateFork_fsck(t *testing.T) {
+	t.Parallel()
+
+	cfg := testcfg.Build(t)
+
+	testcfg.BuildGitalyHooks(t, cfg)
+	testcfg.BuildGitalySSH(t, cfg)
+
+	client, socketPath := runRepositoryService(t, cfg, nil)
+	cfg.SocketPath = socketPath
+
+	ctx := testhelper.Context(t)
+	ctx = testhelper.MergeOutgoingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg))
+
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+
+	// Write a tree into the repository that's known-broken.
+	treeID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+		{Content: "content", Path: "dup", Mode: "100644"},
+		{Content: "content", Path: "dup", Mode: "100644"},
+	})
+
+	gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithParents(),
+		gittest.WithBranch("main"),
+		gittest.WithTree(treeID),
+	)
+
+	forkedRepo := &gitalypb.Repository{
+		RelativePath: gittest.NewRepositoryName(t, true),
+		StorageName:  repo.GetStorageName(),
+	}
+
+	// Create a fork from the repository with the broken tree. This should work alright: repos
+	// with preexisting broken objects that we already have on our disk anyway should not be
+	// subject to additional consistency checks. Otherwise we might end up in a situation where
+	// we retroactively tighten consistency checks for repositories such that preexisting repos
+	// wouldn't be forkable anymore.
+	_, err := client.CreateFork(ctx, &gitalypb.CreateForkRequest{
+		Repository:       forkedRepo,
+		SourceRepository: repo,
+	})
+	require.NoError(t, err)
+
+	forkedRepoPath := filepath.Join(cfg.Storages[0].Path, gittest.GetReplicaPath(ctx, t, cfg, forkedRepo))
+
+	// Verify that the broken tree is indeed in the fork and that it is reported as broken by
+	// git-fsck(1).
+	var stderr bytes.Buffer
+	fsckCmd := gittest.NewCommand(t, cfg, "-C", forkedRepoPath, "fsck")
+	fsckCmd.Stderr = &stderr
+
+	require.EqualError(t, fsckCmd.Run(), "exit status 4")
+	require.Equal(t, fmt.Sprintf("error in tree %s: duplicateEntries: contains duplicate file entries\n", treeID), stderr.String())
+}
+
 func TestCreateFork_targetExists(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
diff --git a/internal/gitaly/service/repository/create_repository_from_url_test.go b/internal/gitaly/service/repository/create_repository_from_url_test.go
index 54dd7e79e..764f0a12d 100644
--- a/internal/gitaly/service/repository/create_repository_from_url_test.go
+++ b/internal/gitaly/service/repository/create_repository_from_url_test.go
@@ -186,6 +186,40 @@ func TestCreateRepositoryFromURL_redirect(t *testing.T) {
 	require.Contains(t, err.Error(), "The requested URL returned error: 301")
 }
 
+func TestCreateRepositoryFromURL_fsck(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+
+	cfg, client := setupRepositoryServiceWithoutRepo(t)
+
+	_, sourceRepoPath := gittest.CreateRepository(ctx, t, cfg)
+
+	// We're creating a new commit which has a root tree with duplicate entries. git-mktree(1)
+	// allows us to create these trees just fine, but git-fsck(1) complains.
+	gittest.WriteCommit(t, cfg, sourceRepoPath,
+		gittest.WithParents(),
+		gittest.WithBranch("main"),
+		gittest.WithTreeEntries(
+			gittest.TreeEntry{Content: "content", Path: "dup", Mode: "100644"},
+			gittest.TreeEntry{Content: "content", Path: "dup", Mode: "100644"},
+		),
+	)
+
+	targetRepoProto := &gitalypb.Repository{
+		RelativePath: gittest.NewRepositoryName(t, true),
+		StorageName:  cfg.Storages[0].Name,
+	}
+
+	_, err := client.CreateRepositoryFromURL(ctx, &gitalypb.CreateRepositoryFromURLRequest{
+		Repository: targetRepoProto,
+		Url:        "file://" + sourceRepoPath,
+	})
+	require.Error(t, err)
+	testhelper.RequireGrpcCode(t, err, codes.Internal)
+	require.Contains(t, err.Error(), "duplicateEntries: contains duplicate file entries")
+}
+
 func TestServer_CloneFromURLCommand(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go
index ef47aabb0..467282911 100644
--- a/internal/gitaly/service/smarthttp/inforefs_test.go
+++ b/internal/gitaly/service/smarthttp/inforefs_test.go
@@ -37,10 +37,14 @@ import (
 
 func TestInfoRefsUploadPack_successful(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsUploadPackSuccessful)
+}
+
+func testInfoRefsUploadPackSuccessful(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	cfg := testcfg.Build(t)
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
-	ctx := testhelper.Context(t)
 
 	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
 
@@ -62,10 +66,14 @@ func TestInfoRefsUploadPack_successful(t *testing.T) {
 
 func TestInfoRefsUploadPack_internalRefs(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsUploadPackInternalRefs)
+}
+
+func testInfoRefsUploadPackInternalRefs(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	cfg := testcfg.Build(t)
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
-	ctx := testhelper.Context(t)
 
 	for _, tc := range []struct {
 		ref                    string
@@ -97,17 +105,37 @@ func TestInfoRefsUploadPack_internalRefs(t *testing.T) {
 		},
 		{
 			ref: "refs/tmp/1",
-			expectedAdvertisements: []string{
-				"HEAD",
-				"refs/heads/main\n",
-			},
+			expectedAdvertisements: func() []string {
+				if featureflag.UploadPackHideRefs.IsDisabled(ctx) {
+					return []string{
+						"HEAD",
+						"refs/heads/main\n",
+						"refs/tmp/1\n",
+					}
+				}
+
+				return []string{
+					"HEAD",
+					"refs/heads/main\n",
+				}
+			}(),
 		},
 		{
 			ref: "refs/keep-around/1",
-			expectedAdvertisements: []string{
-				"HEAD",
-				"refs/heads/main\n",
-			},
+			expectedAdvertisements: func() []string {
+				if featureflag.UploadPackHideRefs.IsDisabled(ctx) {
+					return []string{
+						"HEAD",
+						"refs/heads/main\n",
+						"refs/keep-around/1\n",
+					}
+				}
+
+				return []string{
+					"HEAD",
+					"refs/heads/main\n",
+				}
+			}(),
 		},
 	} {
 		t.Run(tc.ref, func(t *testing.T) {
@@ -130,18 +158,21 @@ func TestInfoRefsUploadPack_internalRefs(t *testing.T) {
 	}
 }
 
-func TestInfoRefsUploadPack_repositoryDoesntExist(t *testing.T) {
+func TestInfoRefsUploadPackRepositoryDoesntExist(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsUploadPackRepositoryDoesntExist)
+}
 
-	cfg := testcfg.Build(t)
+func testInfoRefsUploadPackRepositoryDoesntExist(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
+	cfg := testcfg.Build(t)
 	serverSocketPath := runSmartHTTPServer(t, cfg)
 
 	rpcRequest := &gitalypb.InfoRefsRequest{Repository: &gitalypb.Repository{
 		StorageName:  cfg.Storages[0].Name,
 		RelativePath: "doesnt/exist",
 	}}
-	ctx := testhelper.Context(t)
 
 	_, err := makeInfoRefsUploadPackRequest(ctx, t, serverSocketPath, cfg.Auth.Token, rpcRequest)
 
@@ -153,13 +184,16 @@ func TestInfoRefsUploadPack_repositoryDoesntExist(t *testing.T) {
 	testhelper.RequireGrpcError(t, expectedErr, err)
 }
 
-func TestInfoRefsUploadPack_partialClone(t *testing.T) {
+func TestInfoRefsUploadPackPartialClone(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsUploadPackPartialClone)
+}
 
-	cfg := testcfg.Build(t)
+func testInfoRefsUploadPackPartialClone(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
+	cfg := testcfg.Build(t)
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
-	ctx := testhelper.Context(t)
 
 	repo, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
 		Seed: gittest.SeedGitLabTest,
@@ -180,13 +214,17 @@ func TestInfoRefsUploadPack_partialClone(t *testing.T) {
 	}
 }
 
-func TestInfoRefsUploadPack_gitConfigOptions(t *testing.T) {
+func TestInfoRefsUploadPackGitConfigOptions(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsUploadPackGitConfigOptions)
+}
+
+func testInfoRefsUploadPackGitConfigOptions(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
 
-	ctx := testhelper.Context(t)
 	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
 
 	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
@@ -204,11 +242,15 @@ func TestInfoRefsUploadPack_gitConfigOptions(t *testing.T) {
 	})
 }
 
-func TestInfoRefsUploadPack_gitProtocol(t *testing.T) {
+func TestInfoRefsUploadPackGitProtocol(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsUploadPackGitProtocol)
+}
+
+func testInfoRefsUploadPackGitProtocol(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
-	ctx := testhelper.Context(t)
 
 	protocolDetectingFactory := gittest.NewProtocolDetectingCommandFactory(ctx, t, cfg)
 
@@ -262,12 +304,16 @@ func makeInfoRefsUploadPackRequest(ctx context.Context, t *testing.T, serverSock
 	return response, err
 }
 
-func TestInfoRefsReceivePack_successful(t *testing.T) {
+func TestInfoRefsReceivePackSuccessful(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsReceivePackSuccessful)
+}
+
+func testInfoRefsReceivePackSuccessful(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
-	ctx := testhelper.Context(t)
 
 	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
 
@@ -287,7 +333,12 @@ func TestInfoRefsReceivePack_successful(t *testing.T) {
 	})
 }
 
-func TestInfoRefsReceivePack_hiddenRefs(t *testing.T) {
+func TestInfoRefsReceivePackHiddenRefs(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsReceivePackHiddenRefs)
+}
+
+func testInfoRefsReceivePackHiddenRefs(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -295,7 +346,6 @@ func TestInfoRefsReceivePack_hiddenRefs(t *testing.T) {
 	testcfg.BuildGitalyHooks(t, cfg)
 
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
-	ctx := testhelper.Context(t)
 
 	repoProto, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
 		Seed: gittest.SeedGitLabTest,
@@ -330,7 +380,12 @@ func TestInfoRefsReceivePack_hiddenRefs(t *testing.T) {
 	require.NotContains(t, string(response), commitID+" .have")
 }
 
-func TestInfoRefsReceivePack_repoNotFound(t *testing.T) {
+func TestInfoRefsReceivePackRepoNotFound(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsReceivePackRepoNotFound)
+}
+
+func testInfoRefsReceivePackRepoNotFound(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -339,7 +394,6 @@ func TestInfoRefsReceivePack_repoNotFound(t *testing.T) {
 
 	repo := &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "testdata/scratch/another_repo"}
 	rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo}
-	ctx := testhelper.Context(t)
 	_, err := makeInfoRefsReceivePackRequest(ctx, t, serverSocketPath, cfg.Auth.Token, rpcRequest)
 
 	expectedErr := helper.ErrNotFoundf(`GetRepoPath: not a git repository: "` + cfg.Storages[0].Path + "/" + repo.RelativePath + `"`)
@@ -350,7 +404,12 @@ func TestInfoRefsReceivePack_repoNotFound(t *testing.T) {
 	testhelper.RequireGrpcError(t, expectedErr, err)
 }
 
-func TestInfoRefsReceivePack_repoNotSet(t *testing.T) {
+func TestInfoRefsReceivePackRepoNotSet(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsReceivePackRepoNotSet)
+}
+
+func testInfoRefsReceivePackRepoNotSet(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -358,7 +417,6 @@ func TestInfoRefsReceivePack_repoNotSet(t *testing.T) {
 	serverSocketPath := runSmartHTTPServer(t, cfg)
 
 	rpcRequest := &gitalypb.InfoRefsRequest{}
-	ctx := testhelper.Context(t)
 	_, err := makeInfoRefsReceivePackRequest(ctx, t, serverSocketPath, cfg.Auth.Token, rpcRequest)
 	testhelper.RequireGrpcCode(t, err, codes.InvalidArgument)
 }
@@ -418,7 +476,12 @@ func (ms *mockStreamer) PutStream(ctx context.Context, repo *gitalypb.Repository
 	return ms.Streamer.PutStream(ctx, repo, req, src)
 }
 
-func TestInfoRefsUploadPack_cache(t *testing.T) {
+func TestInfoRefsUploadPackCache(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testInfoRefsUploadPackCache)
+}
+
+func testInfoRefsUploadPackCache(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -434,8 +497,6 @@ func TestInfoRefsUploadPack_cache(t *testing.T) {
 	gitalyServer := startSmartHTTPServer(t, cfg, withInfoRefCache(mockInfoRefCache))
 	cfg.SocketPath = gitalyServer.Address()
 
-	ctx := testhelper.Context(t)
-
 	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
 
 	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go
index 02ca6296e..1bd42a95f 100644
--- a/internal/gitaly/service/smarthttp/upload_pack_test.go
+++ b/internal/gitaly/service/smarthttp/upload_pack_test.go
@@ -20,6 +20,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/sidechannel"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
@@ -45,15 +46,15 @@ func runTestWithAndWithoutConfigOptions(
 	makeRequest requestMaker,
 	opts ...testcfg.Option,
 ) {
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, func(t *testing.T, ctx context.Context) {
+		t.Run("no config options", func(t *testing.T) { tf(t, ctx, makeRequest) })
 
-	t.Run("no config options", func(t *testing.T) { tf(t, ctx, makeRequest) })
-
-	if len(opts) > 0 {
-		t.Run("with config options", func(t *testing.T) {
-			tf(t, ctx, makeRequest, opts...)
-		})
-	}
+		if len(opts) > 0 {
+			t.Run("with config options", func(t *testing.T) {
+				tf(t, ctx, makeRequest, opts...)
+			})
+		}
+	})
 }
 
 func TestServer_PostUpload(t *testing.T) {
@@ -262,16 +263,18 @@ func testServerPostUploadPackSuppressDeepenExitError(t *testing.T, ctx context.C
 
 func TestServer_PostUploadPack_usesPackObjectsHook(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
-	testServerPostUploadPackUsesPackObjectsHook(t, ctx, makePostUploadPackRequest)
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, func(t *testing.T, ctx context.Context) {
+		testServerPostUploadPackUsesPackObjectsHook(t, ctx, makePostUploadPackRequest)
+	})
 }
 
 func TestServer_PostUploadPackWithSidechannel_usesPackObjectsHook(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
-	testServerPostUploadPackUsesPackObjectsHook(t, ctx, makePostUploadPackWithSidechannelRequest)
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, func(t *testing.T, ctx context.Context) {
+		testServerPostUploadPackUsesPackObjectsHook(t, ctx, makePostUploadPackWithSidechannelRequest)
+	})
 }
 
 func testServerPostUploadPackUsesPackObjectsHook(t *testing.T, ctx context.Context, makeRequest requestMaker, opts ...testcfg.Option) {
@@ -492,16 +495,18 @@ func testServerPostUploadPackPartialClone(t *testing.T, ctx context.Context, mak
 
 func TestServer_PostUploadPack_allowAnySHA1InWant(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
-	testServerPostUploadPackAllowAnySHA1InWant(t, ctx, makePostUploadPackRequest)
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, func(t *testing.T, ctx context.Context) {
+		testServerPostUploadPackAllowAnySHA1InWant(t, ctx, makePostUploadPackRequest)
+	})
 }
 
 func TestServer_PostUploadPackWithSidechannel_allowAnySHA1InWant(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
-	testServerPostUploadPackAllowAnySHA1InWant(t, ctx, makePostUploadPackWithSidechannelRequest)
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, func(t *testing.T, ctx context.Context) {
+		testServerPostUploadPackAllowAnySHA1InWant(t, ctx, makePostUploadPackWithSidechannelRequest)
+	})
 }
 
 func testServerPostUploadPackAllowAnySHA1InWant(t *testing.T, ctx context.Context, makeRequest requestMaker, opts ...testcfg.Option) {
diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go
index c8b287b72..a366d0f80 100644
--- a/internal/gitaly/service/ssh/upload_pack_test.go
+++ b/internal/gitaly/service/ssh/upload_pack_test.go
@@ -137,7 +137,12 @@ func testUploadPackTimeout(t *testing.T, opts ...testcfg.Option) {
 	})
 }
 
-func TestUploadPackWithSidechannel_client(t *testing.T) {
+func TestUploadPackWithSidechannelClient(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testUploadPackWithSidechannelClient)
+}
+
+func testUploadPackWithSidechannelClient(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -376,7 +381,7 @@ func TestUploadPackWithSidechannel_client(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx, cancel := context.WithCancel(testhelper.Context(t))
+			ctx, cancel := context.WithCancel(ctx)
 
 			ctx, waiter := sidechannel.RegisterSidechannel(ctx, registry, func(clientConn *sidechannel.ClientConn) (returnedErr error) {
 				errCh := make(chan error, 1)
@@ -642,10 +647,13 @@ func testUploadPackSuccessful(t *testing.T, sidechannel bool, opts ...testcfg.Op
 	}
 }
 
-func TestUploadPack_packObjectsHook(t *testing.T) {
+func TestUploadPackPackObjectsHook(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testUploadPackPackObjectsHook)
+}
 
-	ctx := testhelper.Context(t)
+func testUploadPackPackObjectsHook(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	cfg := testcfg.Build(t, testcfg.WithPackObjectsCacheEnabled())
 
@@ -738,10 +746,14 @@ func testUploadPackWithoutSideband(t *testing.T, opts ...testcfg.Option) {
 	require.Contains(t, string(out), "PACK")
 }
 
-func TestUploadPack_invalidStorage(t *testing.T) {
+func TestUploadPackInvalidStorage(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testUploadPackInvalidStorage)
+}
+
+func testUploadPackInvalidStorage(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 	cfg.SocketPath = runSSHServer(t, cfg)
 
@@ -773,10 +785,14 @@ func TestUploadPack_invalidStorage(t *testing.T) {
 	}
 }
 
-func TestUploadPack_gitFailure(t *testing.T) {
+func TestUploadPackGitFailure(t *testing.T) {
+	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.UploadPackHideRefs).Run(t, testUploadPackGitFailure)
+}
+
+func testUploadPackGitFailure(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 	cfg.SocketPath = runSSHServer(t, cfg)
 
diff --git a/internal/metadata/featureflag/ff_upload_pack_hide_refs.go b/internal/metadata/featureflag/ff_upload_pack_hide_refs.go
new file mode 100644
index 000000000..1e26e33e5
--- /dev/null
+++ b/internal/metadata/featureflag/ff_upload_pack_hide_refs.go
@@ -0,0 +1,9 @@
+package featureflag
+
+// UploadPackHideRefs causes git-upload-pack(1) to hide internal references.
+var UploadPackHideRefs = NewFeatureFlag(
+	"upload_pack_hide_refs",
+	"v15.3.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/4390",
+	false,
+)