Merge branch 'pks-git-dont-advertise-hidden-refs' into 'master'

git: Don't advertise internal references via git-upload-pack(1)

Closes #4358

See merge request gitlab-org/gitaly!4727

7 files changed, 311 insertions, 100 deletions

diff --git a/internal/git/command_description.go b/internal/git/command_description.go
index a56580e0b..a04746913 100644
--- a/internal/git/command_description.go
+++ b/internal/git/command_description.go
@@ -2,7 +2,6 @@ package git
 
 import (
 	"fmt"
-	"log"
 	"math"
 	"runtime"
 	"strings"
@@ -299,12 +298,12 @@ var commandDescriptions = map[string]commandDescription{
 	},
 	"upload-pack": {
 		flags: scNoRefUpdates,
-		opts: append([]GlobalOption{
+		opts: append(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"},
-		}, packConfiguration()...),
+		}, hiddenUploadPackRefPrefixes()...), packConfiguration()...),
 	},
 	"version": {
 		flags: scNoRefUpdates,
@@ -314,34 +313,6 @@ var commandDescriptions = map[string]commandDescription{
 	},
 }
 
-func init() {
-	// This is the poor-mans static assert that all internal ref prefixes are properly hidden
-	// from git-receive-pack(1) such that they cannot be written to when the user pushes.
-	receivePackDesc, ok := commandDescriptions["receive-pack"]
-	if !ok {
-		log.Fatal("could not find command description of git-receive-pack(1)")
-	}
-
-	hiddenRefs := map[string]bool{}
-	for _, opt := range receivePackDesc.opts {
-		configPair, ok := opt.(ConfigPair)
-		if !ok {
-			continue
-		}
-		if configPair.Key != "receive.hideRefs" {
-			continue
-		}
-
-		hiddenRefs[configPair.Value] = true
-	}
-
-	for _, internalRef := range InternalRefPrefixes {
-		if !hiddenRefs[internalRef] {
-			log.Fatalf("command description of receive-pack is missing hidden ref %q", internalRef)
-		}
-	}
-}
-
 // mayUpdateRef indicates if a command is known to update references.
 // This is useful to determine if a command requires reference hook
 // configuration. A non-exhaustive list of commands is consulted to determine if
@@ -404,13 +375,38 @@ func validatePositionalArg(arg string) error {
 }
 
 func hiddenReceivePackRefPrefixes() []GlobalOption {
-	var cps []GlobalOption
+	config := make([]GlobalOption, 0, len(InternalRefPrefixes))
+
+	for refPrefix, refType := range InternalRefPrefixes {
+		switch refType {
+		case InternalReferenceTypeReadonly, InternalReferenceTypeHidden:
+			// We want to hide both read-only and hidden refs in git-receive-pack(1) so
+			// that we make neither of them writeable.
+			config = append(config, ConfigPair{Key: "receive.hideRefs", Value: refPrefix})
+		default:
+			panic(fmt.Sprintf("unhandled internal reference type: %v", refType))
+		}
+	}
+
+	return config
+}
 
-	for _, ns := range InternalRefPrefixes {
-		cps = append(cps, ConfigPair{Key: "receive.hideRefs", Value: ns})
+func hiddenUploadPackRefPrefixes() []GlobalOption {
+	config := make([]GlobalOption, 0, len(InternalRefPrefixes))
+
+	for refPrefix, refType := range InternalRefPrefixes {
+		switch refType {
+		case InternalReferenceTypeHidden:
+			config = append(config, ConfigPair{Key: "uploadpack.hideRefs", Value: refPrefix})
+		case InternalReferenceTypeReadonly:
+			// git-upload-pack(1) doesn't allow writing references, and we do want to
+			// announce read-only references that aren't hidden.
+		default:
+			panic(fmt.Sprintf("unhandled internal reference type: %v", refType))
+		}
 	}
 
-	return cps
+	return config
 }
 
 // fsckConfiguration generates our fsck configuration, including ignored checks. The prefix must
diff --git a/internal/git/gittest/pktline.go b/internal/git/gittest/pktline.go
index e198f01e0..622719b34 100644
--- a/internal/git/gittest/pktline.go
+++ b/internal/git/gittest/pktline.go
@@ -3,14 +3,25 @@ package gittest
 import (
 	"fmt"
 	"io"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline"
 )
 
+// Pktlinef formats the given formatting string into a pktline.
+func Pktlinef(t *testing.T, format string, a ...interface{}) string {
+	t.Helper()
+	var builder strings.Builder
+	_, err := pktline.WriteString(&builder, fmt.Sprintf(format, a...))
+	require.NoError(t, err)
+	return builder.String()
+}
+
 // WritePktlineString writes the pktline-formatted data into the writer.
 func WritePktlineString(t *testing.T, writer io.Writer, data string) {
+	t.Helper()
 	_, err := pktline.WriteString(writer, data)
 	require.NoError(t, err)
 }
@@ -18,16 +29,19 @@ func WritePktlineString(t *testing.T, writer io.Writer, data string) {
 // WritePktlinef formats the given format string and writes the pktline-formatted data into the
 // writer.
 func WritePktlinef(t *testing.T, writer io.Writer, format string, args ...interface{}) {
+	t.Helper()
 	_, err := pktline.WriteString(writer, fmt.Sprintf(format, args...))
 	require.NoError(t, err)
 }
 
 // WritePktlineFlush writes the pktline-formatted flush into the writer.
 func WritePktlineFlush(t *testing.T, writer io.Writer) {
+	t.Helper()
 	require.NoError(t, pktline.WriteFlush(writer))
 }
 
 // WritePktlineDelim writes the pktline-formatted delimiter into the writer.
 func WritePktlineDelim(t *testing.T, writer io.Writer) {
+	t.Helper()
 	require.NoError(t, pktline.WriteDelim(writer))
 }
diff --git a/internal/git/reference.go b/internal/git/reference.go
index c63230b06..225231188 100644
--- a/internal/git/reference.go
+++ b/internal/git/reference.go
@@ -8,15 +8,44 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/command"
 )
 
+// InternalReferenceType is the type of an internal reference.
+type InternalReferenceType int
+
+const (
+	// InternalReferenceTypeHidden indicates that a reference should never be advertised or
+	// writeable.
+	InternalReferenceTypeHidden = InternalReferenceType(iota + 1)
+	// InternalReferenceTypeReadonly indicates that a reference should be advertised, but not
+	// writeable.
+	InternalReferenceTypeReadonly
+)
+
 // InternalRefPrefixes is an array of all reference prefixes which are used internally by GitLab.
 // These need special treatment in some cases, e.g. to restrict writing to them.
-var InternalRefPrefixes = [...]string{
-	"refs/environments/",
-	"refs/keep-around/",
-	"refs/merge-requests/",
-	"refs/pipelines/",
-	"refs/remotes/",
-	"refs/tmp/",
+var InternalRefPrefixes = map[string]InternalReferenceType{
+	// Environments may be interesting to the user in case they want to figure out what exact
+	// reference an environment has been constructed from.
+	"refs/environments/": InternalReferenceTypeReadonly,
+
+	// Keep-around references are only used internally to keep alive objects, and thus they
+	// shouldn't be exposed to the user.
+	"refs/keep-around/": InternalReferenceTypeHidden,
+
+	// Merge request references should be readable by the user so that they can still fetch the
+	// changes of specific merge requests.
+	"refs/merge-requests/": InternalReferenceTypeReadonly,
+
+	// Pipelines may be interesting to the user in case they want to figure out what exact
+	// reference a specific pipeline has been running with.
+	"refs/pipelines/": InternalReferenceTypeReadonly,
+
+	// Remote references shouldn't typically exist in repositories nowadays anymore, and there
+	// is no reason to expose them to the user.
+	"refs/remotes/": InternalReferenceTypeHidden,
+
+	// Temporary references are used internally by Rails for various operations and should not
+	// be exposed to the user.
+	"refs/tmp/": InternalReferenceTypeHidden,
 }
 
 // ObjectPoolRefNamespace is the namespace used for the references of the primary pool member part
diff --git a/internal/gitaly/service/cleanup/internalrefs/cleaner.go b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
index 1c4094bcc..0ef2986c8 100644
--- a/internal/gitaly/service/cleanup/internalrefs/cleaner.go
+++ b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
@@ -124,10 +124,15 @@ func (c *Cleaner) processEntry(ctx context.Context, oldSHA, newSHA string) error
 // action). It is consulted once per line in the object map. Git is optimized
 // for ref -> SHA lookups, but we want the opposite!
 func buildLookupTable(ctx context.Context, repo git.RepositoryExecutor) (map[string][]git.ReferenceName, error) {
+	internalRefPrefixes := make([]string, 0, len(git.InternalRefPrefixes))
+	for refPrefix := range git.InternalRefPrefixes {
+		internalRefPrefixes = append(internalRefPrefixes, refPrefix)
+	}
+
 	cmd, err := repo.Exec(ctx, git.SubCmd{
 		Name:  "for-each-ref",
 		Flags: []git.Option{git.ValueFlag{Name: "--format", Value: "%(objectname) %(refname)"}},
-		Args:  git.InternalRefPrefixes[:],
+		Args:  internalRefPrefixes,
 	})
 	if err != nil {
 		return nil, err
diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go
index d11706ee8..1908cbbbb 100644
--- a/internal/gitaly/service/repository/replicate_test.go
+++ b/internal/gitaly/service/repository/replicate_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/client"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/backchannel"
+	"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/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
@@ -107,6 +108,88 @@ func TestReplicateRepository(t *testing.T) {
 	gittest.Exec(t, cfg, "-C", targetRepoPath, "cat-file", "-p", blobID)
 }
 
+func TestReplicateRepository_hiddenRefs(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+	cfgBuilder := testcfg.NewGitalyCfgBuilder(testcfg.WithStorages("default", "replica"))
+	cfg := cfgBuilder.Build(t)
+
+	testcfg.BuildGitalyHooks(t, cfg)
+	testcfg.BuildGitalySSH(t, cfg)
+
+	client, serverSocketPath := runRepositoryService(t, cfg, nil, testserver.WithDisablePraefect())
+	cfg.SocketPath = serverSocketPath
+
+	ctx = testhelper.MergeOutgoingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg))
+
+	t.Run("initial seeding", func(t *testing.T) {
+		sourceRepo, sourceRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
+
+		// Create a bunch of internal references, regardless of whether we classify them as hidden
+		// or read-only. We should be able to replicate all of them.
+		var expectedRefs []string
+		for refPrefix := range git.InternalRefPrefixes {
+			commitID := gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithParents(), gittest.WithMessage(refPrefix))
+			gittest.Exec(t, cfg, "-C", sourceRepoPath, "update-ref", refPrefix+"1", commitID.String())
+			expectedRefs = append(expectedRefs, fmt.Sprintf("%s commit\t%s", commitID, refPrefix+"1"))
+		}
+
+		targetRepo := proto.Clone(sourceRepo).(*gitalypb.Repository)
+		targetRepo.StorageName = cfg.Storages[1].Name
+		targetRepoPath := filepath.Join(cfg.Storages[1].Path, targetRepo.GetRelativePath())
+
+		_, err := client.ReplicateRepository(ctx, &gitalypb.ReplicateRepositoryRequest{
+			Repository: targetRepo,
+			Source:     sourceRepo,
+		})
+		require.NoError(t, err)
+
+		require.ElementsMatch(t, expectedRefs, strings.Split(text.ChompBytes(gittest.Exec(t, cfg, "-C", targetRepoPath, "for-each-ref")), "\n"))
+
+		// Perform another sanity-check to verify that source and target repository have the
+		// same references now.
+		require.Equal(t,
+			text.ChompBytes(gittest.Exec(t, cfg, "-C", sourceRepoPath, "for-each-ref")),
+			text.ChompBytes(gittest.Exec(t, cfg, "-C", targetRepoPath, "for-each-ref")),
+		)
+	})
+
+	t.Run("incremental replication", func(t *testing.T) {
+		sourceRepo, sourceRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
+		targetRepo, targetRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[1], gittest.InitRepoOpts{
+			WithRelativePath: sourceRepo.GetRelativePath(),
+		})
+
+		// Create the same commit in both repositories so that they're in a known-good
+		// state.
+		sourceCommitID := gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithParents(), gittest.WithMessage("base"), gittest.WithBranch("main"))
+		targetCommitID := gittest.WriteCommit(t, cfg, targetRepoPath, gittest.WithParents(), gittest.WithMessage("base"), gittest.WithBranch("main"))
+		require.Equal(t, sourceCommitID, targetCommitID)
+
+		// Create the internal references now.
+		for refPrefix := range git.InternalRefPrefixes {
+			commitID := gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithParents(), gittest.WithMessage(refPrefix))
+			gittest.Exec(t, cfg, "-C", sourceRepoPath, "update-ref", refPrefix+"1", commitID.String())
+		}
+
+		// And now replicate the with the new internal references having been created.
+		// Because the target repository exists already we'll do a fetch instead of
+		// replicating via an archive.
+		_, err := client.ReplicateRepository(ctx, &gitalypb.ReplicateRepositoryRequest{
+			Repository: targetRepo,
+			Source:     sourceRepo,
+		})
+		require.NoError(t, err)
+
+		// Verify that the references for both repositories match.
+		require.Equal(t,
+			text.ChompBytes(gittest.Exec(t, cfg, "-C", sourceRepoPath, "for-each-ref")),
+			text.ChompBytes(gittest.Exec(t, cfg, "-C", targetRepoPath, "for-each-ref")),
+		)
+	})
+}
+
 func TestReplicateRepositoryTransactional(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/gitaly/service/repository/size.go b/internal/gitaly/service/repository/size.go
index 3eb0f83a5..1879aead9 100644
--- a/internal/gitaly/service/repository/size.go
+++ b/internal/gitaly/service/repository/size.go
@@ -21,8 +21,8 @@ func (s *server) RepositorySize(ctx context.Context, in *gitalypb.RepositorySize
 	var err error
 
 	var excludes []string
-	for _, prefix := range git.InternalRefPrefixes {
-		excludes = append(excludes, prefix+"*")
+	for refPrefix := range git.InternalRefPrefixes {
+		excludes = append(excludes, refPrefix+"*")
 	}
 
 	path, err := repo.Path()
diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go
index 2ffe65c39..9b371bf7f 100644
--- a/internal/gitaly/service/smarthttp/inforefs_test.go
+++ b/internal/gitaly/service/smarthttp/inforefs_test.go
@@ -35,27 +35,101 @@ import (
 	"google.golang.org/protobuf/proto"
 )
 
-func TestSuccessfulInfoRefsUploadPack(t *testing.T) {
+func TestInfoRefsUploadPack_successful(t *testing.T) {
 	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,
+
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
+	tagID := gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision(), gittest.WriteTagConfig{
+		Message: "annotated tag",
 	})
 
 	rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo}
 	response, err := makeInfoRefsUploadPackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, rpcRequest)
 	require.NoError(t, err)
-	assertGitRefAdvertisement(t, "InfoRefsUploadPack", string(response), "001e# service=git-upload-pack", "0000", []string{
-		"003ef4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 refs/tags/v1.0.0",
-		"00416f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 refs/tags/v1.0.0^{}",
+	requireAdvertisedRefs(t, string(response), "git-upload-pack", []string{
+		commitID.String() + " HEAD",
+		commitID.String() + " refs/heads/main\n",
+		tagID.String() + " refs/tags/v1.0.0\n",
+		commitID.String() + " refs/tags/v1.0.0^{}\n",
 	})
 }
 
+func TestInfoRefsUploadPack_internalRefs(t *testing.T) {
+	t.Parallel()
+
+	cfg := testcfg.Build(t)
+	cfg.SocketPath = runSmartHTTPServer(t, cfg)
+	ctx := testhelper.Context(t)
+
+	for _, tc := range []struct {
+		ref                    string
+		expectedAdvertisements []string
+	}{
+		{
+			ref: "refs/merge-requests/1/head",
+			expectedAdvertisements: []string{
+				"HEAD",
+				"refs/heads/main\n",
+				"refs/merge-requests/1/head\n",
+			},
+		},
+		{
+			ref: "refs/environments/1",
+			expectedAdvertisements: []string{
+				"HEAD",
+				"refs/environments/1\n",
+				"refs/heads/main\n",
+			},
+		},
+		{
+			ref: "refs/pipelines/1",
+			expectedAdvertisements: []string{
+				"HEAD",
+				"refs/heads/main\n",
+				"refs/pipelines/1\n",
+			},
+		},
+		{
+			ref: "refs/tmp/1",
+			expectedAdvertisements: []string{
+				"HEAD",
+				"refs/heads/main\n",
+			},
+		},
+		{
+			ref: "refs/keep-around/1",
+			expectedAdvertisements: []string{
+				"HEAD",
+				"refs/heads/main\n",
+			},
+		},
+	} {
+		t.Run(tc.ref, func(t *testing.T) {
+			repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+
+			commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
+			gittest.Exec(t, cfg, "-C", repoPath, "update-ref", tc.ref, commitID.String())
+
+			var expectedAdvertisements []string
+			for _, expectedRef := range tc.expectedAdvertisements {
+				expectedAdvertisements = append(expectedAdvertisements, commitID.String()+" "+expectedRef)
+			}
+
+			response, err := makeInfoRefsUploadPackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, &gitalypb.InfoRefsRequest{
+				Repository: repo,
+			})
+			require.NoError(t, err)
+			requireAdvertisedRefs(t, string(response), "git-upload-pack", expectedAdvertisements)
+		})
+	}
+}
+
 func TestInfoRefsUploadPack_repositoryDoesntExist(t *testing.T) {
 	t.Parallel()
 
@@ -79,7 +153,7 @@ func TestInfoRefsUploadPack_repositoryDoesntExist(t *testing.T) {
 	testhelper.RequireGrpcError(t, expectedErr, err)
 }
 
-func TestSuccessfulInfoRefsUploadWithPartialClone(t *testing.T) {
+func TestInfoRefsUploadPack_partialClone(t *testing.T) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -106,16 +180,16 @@ func TestSuccessfulInfoRefsUploadWithPartialClone(t *testing.T) {
 	}
 }
 
-func TestSuccessfulInfoRefsUploadPackWithGitConfigOptions(t *testing.T) {
+func TestInfoRefsUploadPack_gitConfigOptions(t *testing.T) {
 	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,
-	})
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
 
 	// transfer.hideRefs=refs will hide every ref that info-refs would normally
 	// output, allowing us to test that the custom configuration is respected
@@ -125,10 +199,12 @@ func TestSuccessfulInfoRefsUploadPackWithGitConfigOptions(t *testing.T) {
 	}
 	response, err := makeInfoRefsUploadPackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, rpcRequest)
 	require.NoError(t, err)
-	assertGitRefAdvertisement(t, "InfoRefsUploadPack", string(response), "001e# service=git-upload-pack", "0000", []string{})
+	requireAdvertisedRefs(t, string(response), "git-upload-pack", []string{
+		commitID.String() + " HEAD",
+	})
 }
 
-func TestSuccessfulInfoRefsUploadPackWithGitProtocol(t *testing.T) {
+func TestInfoRefsUploadPack_gitProtocol(t *testing.T) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -186,30 +262,32 @@ func makeInfoRefsUploadPackRequest(ctx context.Context, t *testing.T, serverSock
 	return response, err
 }
 
-func TestSuccessfulInfoRefsReceivePack(t *testing.T) {
+func TestInfoRefsReceivePack_successful(t *testing.T) {
 	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,
-	})
 
-	rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo}
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
 
-	response, err := makeInfoRefsReceivePackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, rpcRequest)
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
+	tagID := gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision(), gittest.WriteTagConfig{
+		Message: "annotated tag",
+	})
+
+	response, err := makeInfoRefsReceivePackRequest(ctx, t, cfg.SocketPath, cfg.Auth.Token, &gitalypb.InfoRefsRequest{
+		Repository: repo,
+	})
 	require.NoError(t, err)
 
-	assertGitRefAdvertisement(t, "InfoRefsReceivePack", string(response), "001f# service=git-receive-pack", "0000", []string{
-		"003ef4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 refs/tags/v1.0.0",
-		"003e8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b refs/tags/v1.1.0",
+	requireAdvertisedRefs(t, string(response), "git-receive-pack", []string{
+		commitID.String() + " refs/heads/main",
+		tagID.String() + " refs/tags/v1.0.0\n",
 	})
 }
 
-func TestObjectPoolRefAdvertisementHiding(t *testing.T) {
+func TestInfoRefsReceivePack_hiddenRefs(t *testing.T) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -252,7 +330,7 @@ func TestObjectPoolRefAdvertisementHiding(t *testing.T) {
 	require.NotContains(t, string(response), commitID+" .have")
 }
 
-func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) {
+func TestInfoRefsReceivePack_repoNotFound(t *testing.T) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -272,7 +350,7 @@ func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) {
 	testhelper.RequireGrpcError(t, expectedErr, err)
 }
 
-func TestFailureRepoNotSetInfoRefsReceivePack(t *testing.T) {
+func TestInfoRefsReceivePack_repoNotSet(t *testing.T) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -304,23 +382,28 @@ func makeInfoRefsReceivePackRequest(ctx context.Context, t *testing.T, serverSoc
 	return response, err
 }
 
-func assertGitRefAdvertisement(t *testing.T, rpc, responseBody string, firstLine, lastLine string, middleLines []string) {
-	responseLines := strings.Split(responseBody, "\n")
+func requireAdvertisedRefs(t *testing.T, responseBody, expectedService string, expectedRefs []string) {
+	t.Helper()
 
-	if responseLines[0] != firstLine {
-		t.Errorf("%q: expected response first line to be %q, found %q", rpc, firstLine, responseLines[0])
-	}
+	responseLines := strings.SplitAfter(responseBody, "\n")
+	require.Greater(t, len(responseLines), 2)
 
-	lastIndex := len(responseLines) - 1
-	if responseLines[lastIndex] != lastLine {
-		t.Errorf("%q: expected response last line to be %q, found %q", rpc, lastLine, responseLines[lastIndex])
+	for i, expectedRef := range expectedRefs {
+		expectedRefs[i] = gittest.Pktlinef(t, "%s", expectedRef)
 	}
 
-	for _, ref := range middleLines {
-		if !strings.Contains(responseBody, ref) {
-			t.Errorf("%q: expected response to contain %q, found none", rpc, ref)
-		}
-	}
+	// The first line contains the service announcement.
+	require.Equal(t, gittest.Pktlinef(t, "# service=%s\n", expectedService), responseLines[0])
+
+	// The second line contains the first reference as well as the capability announcement. We
+	// thus split the string at "\x00" and ignore the capability announcement here.
+	refAndCapabilities := strings.SplitN(responseLines[1], "\x00", 2)
+	require.Len(t, refAndCapabilities, 2)
+	// We just replace the first advertised reference to make it easier to compare refs.
+	responseLines[1] = gittest.Pktlinef(t, "%s", refAndCapabilities[0][8:])
+
+	require.Equal(t, responseLines[1:len(responseLines)-1], expectedRefs)
+	require.Equal(t, "0000", responseLines[len(responseLines)-1])
 }
 
 type mockStreamer struct {
@@ -335,7 +418,7 @@ func (ms *mockStreamer) PutStream(ctx context.Context, repo *gitalypb.Repository
 	return ms.Streamer.PutStream(ctx, repo, req, src)
 }
 
-func TestCacheInfoRefsUploadPack(t *testing.T) {
+func TestInfoRefsUploadPack_cache(t *testing.T) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -352,8 +435,12 @@ func TestCacheInfoRefsUploadPack(t *testing.T) {
 	cfg.SocketPath = gitalyServer.Address()
 
 	ctx := testhelper.Context(t)
-	repo, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
+
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
+	tagID := gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision(), gittest.WriteTagConfig{
+		Message: "annotated tag",
 	})
 
 	rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo}
@@ -371,13 +458,12 @@ func TestCacheInfoRefsUploadPack(t *testing.T) {
 		response, err := makeInfoRefsUploadPackRequest(ctx, t, addr, cfg.Auth.Token, rpcRequest)
 		require.NoError(t, err)
 
-		assertGitRefAdvertisement(t, "InfoRefsUploadPack", string(response),
-			"001e# service=git-upload-pack", "0000",
-			[]string{
-				"003ef4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 refs/tags/v1.0.0",
-				"00416f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 refs/tags/v1.0.0^{}",
-			},
-		)
+		requireAdvertisedRefs(t, string(response), "git-upload-pack", []string{
+			commitID.String() + " HEAD",
+			commitID.String() + " refs/heads/main\n",
+			tagID.String() + " refs/tags/v1.0.0\n",
+			commitID.String() + " refs/tags/v1.0.0^{}\n",
+		})
 	}
 
 	assertNormalResponse(gitalyServer.Address())
@@ -395,9 +481,7 @@ func TestCacheInfoRefsUploadPack(t *testing.T) {
 	replaceCachedResponse(t, ctx, cache, rewrittenRequest, strings.Join(replacedContents, "\n"))
 	response, err := makeInfoRefsUploadPackRequest(ctx, t, gitalyServer.Address(), cfg.Auth.Token, rpcRequest)
 	require.NoError(t, err)
-	assertGitRefAdvertisement(t, "InfoRefsUploadPack", string(response),
-		replacedContents[0], replacedContents[3], replacedContents[1:3],
-	)
+	require.Equal(t, strings.Join(replacedContents, "\n"), string(response))
 
 	invalidateCacheForRepo := func() {
 		ender, err := cache.StartLease(rewrittenRequest.Repository)