Merge branch 'pks-reject-pushes-into-internal-namespaces' into 'master'

git: Reject pushes into internal ref namespaces

See merge request gitlab-org/gitaly!3426

4 files changed, 99 insertions, 9 deletions

diff --git a/internal/git/command_description.go b/internal/git/command_description.go
index 5e26d5d7d..43904eff6 100644
--- a/internal/git/command_description.go
+++ b/internal/git/command_description.go
@@ -2,6 +2,7 @@ package git
 
 import (
 	"fmt"
+	"log"
 	"strings"
 )
 
@@ -149,6 +150,16 @@ var commandDescriptions = map[string]commandDescription{
 			// Make git-receive-pack(1) advertise the push options
 			// capability to clients.
 			ConfigPair{Key: "receive.advertisePushOptions", Value: "true"},
+
+			// Hide several reference spaces from being displayed on pushes. This has
+			// two outcomes: first, we reduce the initial ref advertisement and should
+			// speed up pushes for repos which have loads of merge requests, pipelines
+			// and environments. Second, this also prohibits clients to update or delete
+			// these refs.
+			ConfigPair{Key: "receive.hideRefs", Value: "refs/environments/"},
+			ConfigPair{Key: "receive.hideRefs", Value: "refs/keep-around/"},
+			ConfigPair{Key: "receive.hideRefs", Value: "refs/merge-requests/"},
+			ConfigPair{Key: "receive.hideRefs", Value: "refs/pipelines/"},
 		},
 	},
 	"remote": {
@@ -221,6 +232,34 @@ 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
diff --git a/internal/git/reference.go b/internal/git/reference.go
index 494d20c77..96a32190d 100644
--- a/internal/git/reference.go
+++ b/internal/git/reference.go
@@ -6,6 +6,15 @@ import (
 	"strings"
 )
 
+// 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/",
+}
+
 // Revision represents anything that resolves to either a commit, multiple
 // commits or to an object different than a commit. This could be e.g.
 // "master", "master^{commit}", an object hash or similar. See gitrevisions(1)
diff --git a/internal/gitaly/service/cleanup/internalrefs/cleaner.go b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
index 69369299e..5d5a75d19 100644
--- a/internal/gitaly/service/cleanup/internalrefs/cleaner.go
+++ b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
@@ -15,14 +15,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
-// Only references in these namespaces are cleaned up
-var internalRefs = []string{
-	"refs/environments/",
-	"refs/keep-around/",
-	"refs/merge-requests/",
-	"refs/pipelines/",
-}
-
 // A ForEachFunc can be called for every entry in the filter-repo or BFG object
 // map file that the cleaner is processing. Returning an error will stop the
 // cleaner before it has processed the entry in question
@@ -137,7 +129,7 @@ func buildLookupTable(ctx context.Context, gitCmdFactory git.CommandFactory, rep
 	cmd, err := gitCmdFactory.New(ctx, repo, git.SubCmd{
 		Name:  "for-each-ref",
 		Flags: []git.Option{git.ValueFlag{Name: "--format", Value: "%(objectname) %(refname)"}},
-		Args:  internalRefs,
+		Args:  git.InternalRefPrefixes[:],
 	})
 	if err != nil {
 		return nil, err
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index 555fb28c9..d4ba2c428 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -17,6 +17,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
+	"gitlab.com/gitlab-org/gitaly/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook"
@@ -103,6 +104,55 @@ func TestSuccessfulReceivePackRequest(t *testing.T) {
 	}, payload)
 }
 
+func TestReceivePackHiddenRefs(t *testing.T) {
+	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+	repoProto.GlProjectPath = "project/path"
+
+	testhelper.ConfigureGitalyHooksBin(t, cfg)
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	repo := localrepo.New(git.NewExecCommandFactory(cfg), repoProto, cfg)
+	oldHead, err := repo.ResolveRevision(ctx, "HEAD~")
+	require.NoError(t, err)
+	newHead, err := repo.ResolveRevision(ctx, "HEAD")
+	require.NoError(t, err)
+
+	serverSocketPath := runSmartHTTPServer(t, cfg)
+
+	client, conn := newSmartHTTPClient(t, serverSocketPath, cfg.Auth.Token)
+	defer conn.Close()
+
+	for _, ref := range []string{
+		"refs/environments/1",
+		"refs/merge-requests/1/head",
+		"refs/merge-requests/1/merge",
+		"refs/pipelines/1",
+	} {
+		t.Run(ref, func(t *testing.T) {
+			request := &bytes.Buffer{}
+			gittest.WritePktlineString(t, request, fmt.Sprintf("%s %s %s\x00 %s",
+				oldHead, newHead, ref, uploadPackCapabilities))
+			gittest.WritePktlineFlush(t, request)
+
+			// The options passed are the same ones used when doing an actual push.
+			revisions := strings.NewReader(fmt.Sprintf("^%s\n%s\n", oldHead, newHead))
+			pack := testhelper.MustRunCommand(t, revisions, "git", "-C", repoPath, "pack-objects", "--stdout", "--revs", "--thin", "--delta-base-offset", "-q")
+			request.Write(pack)
+
+			stream, err := client.PostReceivePack(ctx)
+			require.NoError(t, err)
+
+			response := doPush(t, stream, &gitalypb.PostReceivePackRequest{
+				Repository: repoProto, GlUsername: "user", GlId: "123", GlRepository: "project-456",
+			}, request)
+
+			require.Contains(t, string(response), fmt.Sprintf("%s deny updating a hidden ref", ref))
+		})
+	}
+}
+
 func TestSuccessfulReceivePackRequestWithGitProtocol(t *testing.T) {
 	cfg, repo, repoPath := testcfg.BuildWithRepo(t)