From 4a789524c7a786a2c8fb0019c3ac20a66c1f9431 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <psteinhardt@gitlab.com>
Date: Tue, 19 Jul 2022 08:17:06 +0200
Subject: git: Don't advertise internal references via git-upload-pack(1)

Gitaly knows two different types of internal references: once those that
are read-only and thus hidden from git-receive-pack(1), and then those
that should only be used internally and which are thus neither readable
nor writeable. We already handle the former type of internal references
by setting up `receive.hideRefs`, but we don't handle the latter type.

Fix this by setting up hidden reference for git-upload-pack(1). This
causes us to not advertise references with a prefix of `refs/tmp` or
`refs/keep-around` when fetching from repositories.

Note that because we set `transport.hideRefs=!refs/` in our gitaly-ssh
executable, this change does not impact internal fetches. This is
expected and required to keep e.g. `ReplicateRepository()` working
correctly.

Changelog: fixed
---
 internal/git/command_description.go                | 22 ++++++++++++++++++++--
 internal/gitaly/service/smarthttp/inforefs_test.go |  4 ----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/internal/git/command_description.go b/internal/git/command_description.go
index c6cabb959..a04746913 100644
--- a/internal/git/command_description.go
+++ b/internal/git/command_description.go
@@ -298,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,
@@ -391,6 +391,24 @@ func hiddenReceivePackRefPrefixes() []GlobalOption {
 	return config
 }
 
+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 config
+}
+
 // fsckConfiguration generates our fsck configuration, including ignored checks. The prefix must
 // either be "receive" or "fetch" and indicates whether it should apply to git-receive-pack(1) or to
 // git-fetch-pack(1).
diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go
index 65c9a080a..9b371bf7f 100644
--- a/internal/gitaly/service/smarthttp/inforefs_test.go
+++ b/internal/gitaly/service/smarthttp/inforefs_test.go
@@ -100,8 +100,6 @@ func TestInfoRefsUploadPack_internalRefs(t *testing.T) {
 			expectedAdvertisements: []string{
 				"HEAD",
 				"refs/heads/main\n",
-				// This is a bug as temporary references should be hidden.
-				"refs/tmp/1\n",
 			},
 		},
 		{
@@ -109,8 +107,6 @@ func TestInfoRefsUploadPack_internalRefs(t *testing.T) {
 			expectedAdvertisements: []string{
 				"HEAD",
 				"refs/heads/main\n",
-				// This is a bug as keep-around references should be hidden.
-				"refs/keep-around/1\n",
 			},
 		},
 	} {
-- 
cgit v1.2.3