hook: Inject ODB envvars for custom hooks passed via repository proto

When executing custom hooks, we forward the GIT_OBJECT_DIRECTORY and
GIT_ALTERNATE_OBJECT_DIRECTORIES environment variables to the spawned
executables. This is required in the context of object quarantine
directories set up by git-receive-pack(1) such that the child process
knows where to find quarantined objects as well as object of the main
object database.

Until now, we only ever forward these environment variables if the
gitaly-hooks executable had them set up by git-receive-pack(1). This
makes sense given that this was the only place where we actually made
use of quarantine directories. But given that we now also support manual
setup of object quarantine directories for use when updating references
with hooks, this is not sufficient anymore. In this context, we perform
execution of Git hooks via manual invocation of the relevant RPCs, and
thus there is no gitaly-hooks executable involved which would forward
the required environment variables. Instead, information on quarantined
object directories is part of the repository protobuf which gets passed
to us.

Implement the logic to make use of this information and correctly set up
the custom hooks environment in case the repository proto does contain
information about object directories. Like this, custom hooks are now
able to access quarantined objects if executed in the context of a
manually set up quarantine.

4 files changed, 182 insertions, 3 deletions

diff --git a/internal/gitaly/hook/custom.go b/internal/gitaly/hook/custom.go
index 2647593ed..8c894e09d 100644
--- a/internal/gitaly/hook/custom.go
+++ b/internal/gitaly/hook/custom.go
@@ -138,10 +138,24 @@ func (m *GitLabHookManager) customHooksEnv(payload git.HooksPayload, pushOptions
 
 	customEnvs := append(command.AllowedEnvironment(envs), pushOptionsEnv(pushOptions)...)
 
-	for _, env := range envs {
-		if strings.HasPrefix(env, "GIT_OBJECT_DIRECTORY=") || strings.HasPrefix(env, "GIT_ALTERNATE_OBJECT_DIRECTORIES=") {
-			customEnvs = append(customEnvs, env)
+	objectDirectory := getEnvVar("GIT_OBJECT_DIRECTORY", envs)
+	if objectDirectory == "" && payload.Repo.GetGitObjectDirectory() != "" {
+		objectDirectory = filepath.Join(repoPath, payload.Repo.GetGitObjectDirectory())
+	}
+	if objectDirectory != "" {
+		customEnvs = append(customEnvs, "GIT_OBJECT_DIRECTORY="+objectDirectory)
+	}
+
+	alternateObjectDirectories := getEnvVar("GIT_ALTERNATE_OBJECT_DIRECTORIES", envs)
+	if alternateObjectDirectories == "" && len(payload.Repo.GetGitAlternateObjectDirectories()) != 0 {
+		var absolutePaths []string
+		for _, alternateObjectDirectory := range payload.Repo.GetGitAlternateObjectDirectories() {
+			absolutePaths = append(absolutePaths, filepath.Join(repoPath, alternateObjectDirectory))
 		}
+		alternateObjectDirectories = strings.Join(absolutePaths, ":")
+	}
+	if alternateObjectDirectories != "" {
+		customEnvs = append(customEnvs, "GIT_ALTERNATE_OBJECT_DIRECTORIES="+alternateObjectDirectories)
 	}
 
 	return append(customEnvs,
diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go
index 705b8891c..9b0ce77db 100644
--- a/internal/gitaly/hook/postreceive_test.go
+++ b/internal/gitaly/hook/postreceive_test.go
@@ -13,6 +13,8 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/backchannel"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/quarantine"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitlab"
@@ -20,6 +22,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/transaction/txinfo"
+	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 )
 
 func TestPrintAlert(t *testing.T) {
@@ -354,3 +357,56 @@ func TestPostReceive_gitlab(t *testing.T) {
 		})
 	}
 }
+
+func TestPostReceive_quarantine(t *testing.T) {
+	ctx, cleanup := testhelper.Context()
+	defer cleanup()
+
+	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+
+	quarantine, err := quarantine.New(ctx, repoProto, config.NewLocator(cfg))
+	require.NoError(t, err)
+
+	quarantinedRepo := localrepo.NewTestRepo(t, cfg, quarantine.QuarantinedRepo())
+	blobID, err := quarantinedRepo.WriteBlob(ctx, "", strings.NewReader("allyourbasearebelongtous"))
+	require.NoError(t, err)
+
+	hookManager := NewManager(config.NewLocator(cfg), nil, gitlab.NewMockClient(), cfg)
+
+	script := fmt.Sprintf("#!/bin/sh\n%s cat-file -p '%s' || true\n",
+		cfg.Git.BinPath, blobID.String())
+	gittest.WriteCustomHook(t, repoPath, "post-receive", []byte(script))
+
+	for repo, isQuarantined := range map[*gitalypb.Repository]bool{
+		quarantine.QuarantinedRepo(): true,
+		repoProto:                    false,
+	} {
+		t.Run(fmt.Sprintf("quarantined: %v", isQuarantined), func(t *testing.T) {
+			env, err := git.NewHooksPayload(cfg, repo, nil,
+				&git.ReceiveHooksPayload{
+					UserID:   "1234",
+					Username: "user",
+					Protocol: "web",
+				},
+				git.PreReceiveHook,
+				featureflag.RawFromContext(ctx),
+			).Env()
+			require.NoError(t, err)
+
+			stdin := strings.NewReader(fmt.Sprintf("%s %s refs/heads/master",
+				git.ZeroOID, git.ZeroOID))
+
+			var stdout, stderr bytes.Buffer
+			require.NoError(t, hookManager.PostReceiveHook(ctx, repo, nil,
+				[]string{env}, stdin, &stdout, &stderr))
+
+			if isQuarantined {
+				require.Equal(t, "allyourbasearebelongtous", stdout.String())
+				require.Empty(t, stderr.String())
+			} else {
+				require.Empty(t, stdout.String())
+				require.Contains(t, stderr.String(), "Not a valid object name")
+			}
+		})
+	}
+}
diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go
index 8221bfe59..b43fbf1db 100644
--- a/internal/gitaly/hook/prereceive_test.go
+++ b/internal/gitaly/hook/prereceive_test.go
@@ -12,6 +12,8 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/backchannel"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/quarantine"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitlab"
@@ -20,6 +22,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/transaction/txinfo"
+	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 )
 
 func TestPrereceive_customHooks(t *testing.T) {
@@ -178,6 +181,59 @@ func TestPrereceive_customHooks(t *testing.T) {
 	}
 }
 
+func TestPrereceive_quarantine(t *testing.T) {
+	ctx, cleanup := testhelper.Context()
+	defer cleanup()
+
+	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+
+	quarantine, err := quarantine.New(ctx, repoProto, config.NewLocator(cfg))
+	require.NoError(t, err)
+
+	quarantinedRepo := localrepo.NewTestRepo(t, cfg, quarantine.QuarantinedRepo())
+	blobID, err := quarantinedRepo.WriteBlob(ctx, "", strings.NewReader("allyourbasearebelongtous"))
+	require.NoError(t, err)
+
+	hookManager := NewManager(config.NewLocator(cfg), nil, gitlab.NewMockClient(), cfg)
+
+	script := fmt.Sprintf("#!/bin/sh\n%s cat-file -p '%s' || true\n",
+		cfg.Git.BinPath, blobID.String())
+	gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(script))
+
+	for repo, isQuarantined := range map[*gitalypb.Repository]bool{
+		quarantine.QuarantinedRepo(): true,
+		repoProto:                    false,
+	} {
+		t.Run(fmt.Sprintf("quarantined: %v", isQuarantined), func(t *testing.T) {
+			env, err := git.NewHooksPayload(cfg, repo, nil,
+				&git.ReceiveHooksPayload{
+					UserID:   "1234",
+					Username: "user",
+					Protocol: "web",
+				},
+				git.PreReceiveHook,
+				featureflag.RawFromContext(ctx),
+			).Env()
+			require.NoError(t, err)
+
+			stdin := strings.NewReader(fmt.Sprintf("%s %s refs/heads/master",
+				git.ZeroOID, git.ZeroOID))
+
+			var stdout, stderr bytes.Buffer
+			require.NoError(t, hookManager.PreReceiveHook(ctx, repo, nil,
+				[]string{env}, stdin, &stdout, &stderr))
+
+			if isQuarantined {
+				require.Equal(t, "allyourbasearebelongtous", stdout.String())
+				require.Empty(t, stderr.String())
+			} else {
+				require.Empty(t, stdout.String())
+				require.Contains(t, stderr.String(), "Not a valid object name")
+			}
+		})
+	}
+}
+
 type prereceiveAPIMock struct {
 	allowed    func(context.Context, gitlab.AllowedParams) (bool, string, error)
 	prereceive func(context.Context, string) (bool, error)
diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go
index 3e09d4698..89b374155 100644
--- a/internal/gitaly/hook/update_test.go
+++ b/internal/gitaly/hook/update_test.go
@@ -10,6 +10,8 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/backchannel"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/quarantine"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitlab"
@@ -17,6 +19,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/transaction/txinfo"
+	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 )
 
 func TestUpdate_customHooks(t *testing.T) {
@@ -203,3 +206,53 @@ func TestUpdate_customHooks(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdate_quarantine(t *testing.T) {
+	ctx, cleanup := testhelper.Context()
+	defer cleanup()
+
+	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+
+	quarantine, err := quarantine.New(ctx, repoProto, config.NewLocator(cfg))
+	require.NoError(t, err)
+
+	quarantinedRepo := localrepo.NewTestRepo(t, cfg, quarantine.QuarantinedRepo())
+	blobID, err := quarantinedRepo.WriteBlob(ctx, "", strings.NewReader("allyourbasearebelongtous"))
+	require.NoError(t, err)
+
+	hookManager := NewManager(config.NewLocator(cfg), nil, gitlab.NewMockClient(), cfg)
+
+	script := fmt.Sprintf("#!/bin/sh\n%s cat-file -p '%s' || true\n",
+		cfg.Git.BinPath, blobID.String())
+	gittest.WriteCustomHook(t, repoPath, "update", []byte(script))
+
+	for repo, isQuarantined := range map[*gitalypb.Repository]bool{
+		quarantine.QuarantinedRepo(): true,
+		repoProto:                    false,
+	} {
+		t.Run(fmt.Sprintf("quarantined: %v", isQuarantined), func(t *testing.T) {
+			env, err := git.NewHooksPayload(cfg, repo, nil,
+				&git.ReceiveHooksPayload{
+					UserID:   "1234",
+					Username: "user",
+					Protocol: "web",
+				},
+				git.PreReceiveHook,
+				featureflag.RawFromContext(ctx),
+			).Env()
+			require.NoError(t, err)
+
+			var stdout, stderr bytes.Buffer
+			require.NoError(t, hookManager.UpdateHook(ctx, repo, "refs/heads/master",
+				git.ZeroOID.String(), git.ZeroOID.String(), []string{env}, &stdout, &stderr))
+
+			if isQuarantined {
+				require.Equal(t, "allyourbasearebelongtous", stdout.String())
+				require.Empty(t, stderr.String())
+			} else {
+				require.Empty(t, stdout.String())
+				require.Contains(t, stderr.String(), "Not a valid object name")
+			}
+		})
+	}
+}