Merge branch 'pks-hooks-validation' into 'master'

hooks: Improved validation and testing

See merge request gitlab-org/gitaly!2824

24 files changed, 813 insertions, 43 deletions

diff --git a/changelogs/unreleased/pks-hooks-validation.yml b/changelogs/unreleased/pks-hooks-validation.yml
new file mode 100644
index 000000000..76b67cccd
--- /dev/null
+++ b/changelogs/unreleased/pks-hooks-validation.yml
@@ -0,0 +1,5 @@
+---
+title: 'hooks: Improved validation and testing'
+merge_request: 2824
+author:
+type: fixed
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index bbe14e87f..f7cce0575 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -609,7 +609,7 @@ func runHookServiceServer(t *testing.T, token string) (string, func()) {
 func runHookServiceServerWithAPI(t *testing.T, token string, gitlabAPI gitalyhook.GitlabAPI) (string, func()) {
 	server := testhelper.NewServerWithAuth(t, nil, nil, token)
 
-	gitalypb.RegisterHookServiceServer(server.GrpcServer(), hook.NewServer(gitalyhook.NewManager(gitlabAPI, config.Config)))
+	gitalypb.RegisterHookServiceServer(server.GrpcServer(), hook.NewServer(config.Config, gitalyhook.NewManager(gitlabAPI, config.Config)))
 	reflection.Register(server.GrpcServer())
 	require.NoError(t, server.Start())
 
diff --git a/internal/git/hooks.go b/internal/git/hooks.go
index 8716ffc7b..4a51281a8 100644
--- a/internal/git/hooks.go
+++ b/internal/git/hooks.go
@@ -63,9 +63,9 @@ type ReceivePackRequest interface {
 // WithReceivePackHooks returns an option that populates the safe command with the environment
 // variables necessary to properly execute the pre-receive, update and post-receive hooks for
 // git-receive-pack(1).
-func WithReceivePackHooks(ctx context.Context, req ReceivePackRequest, protocol string) CmdOpt {
+func WithReceivePackHooks(ctx context.Context, cfg config.Cfg, req ReceivePackRequest, protocol string) CmdOpt {
 	return func(cc *cmdCfg) error {
-		env, err := receivePackHookEnv(ctx, req, protocol)
+		env, err := receivePackHookEnv(ctx, cfg, req, protocol)
 		if err != nil {
 			return fmt.Errorf("receive-pack hook envvars: %w", err)
 		}
@@ -75,8 +75,8 @@ func WithReceivePackHooks(ctx context.Context, req ReceivePackRequest, protocol
 	}
 }
 
-func receivePackHookEnv(ctx context.Context, req ReceivePackRequest, protocol string) ([]string, error) {
-	gitlabshellEnv, err := gitlabshell.Env()
+func receivePackHookEnv(ctx context.Context, cfg config.Cfg, req ReceivePackRequest, protocol string) ([]string, error) {
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(cfg)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/gitaly/hook/access_test.go b/internal/gitaly/hook/access_test.go
index fab76522d..de970a015 100644
--- a/internal/gitaly/hook/access_test.go
+++ b/internal/gitaly/hook/access_test.go
@@ -1,4 +1,4 @@
-package hook_test
+package hook
 
 import (
 	"context"
@@ -11,7 +11,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
@@ -26,7 +25,7 @@ type postReceiveRequest struct {
 // TestAllowedVerifyParams uses client cert fixtures to test TLS connections. To
 // regenerate these certs, run `go generate access_test.go`.
 //go:generate openssl req -newkey rsa:4096 -new -nodes -x509 -days 3650 -out testdata/certs/server.crt -keyout testdata/certs/server.key -subj "/C=US/ST=California/L=San Francisco/O=GitLab/OU=GitLab-Shell/CN=localhost" -addext "subjectAltName = IP:127.0.0.1"
-func TestAllowedVerifyParams(t *testing.T) {
+func TestAccess_verifyParams(t *testing.T) {
 	user, password := "user", "password"
 	secretToken := "topsecret"
 	glID, glRepository := "key-123", "repo-1"
@@ -71,7 +70,7 @@ func TestAllowedVerifyParams(t *testing.T) {
 	})
 	defer cleanup()
 
-	c, err := hook.NewGitlabAPI(config.Gitlab{
+	c, err := NewGitlabAPI(config.Gitlab{
 		URL:        serverURL,
 		SecretFile: secretFilePath,
 		HTTPSettings: config.HTTPSettings{
@@ -121,7 +120,7 @@ func TestAllowedVerifyParams(t *testing.T) {
 	}
 }
 
-func TestEscapedAndRelativeURLs(t *testing.T) {
+func TestAccess_escapedAndRelativeURLs(t *testing.T) {
 	user, password := "user", "password"
 	secretToken := "topsecret"
 	glID, glRepository := "key-123", "repo-1"
@@ -200,7 +199,7 @@ func TestEscapedAndRelativeURLs(t *testing.T) {
 				serverURL = url.PathEscape(serverURL)
 			}
 
-			c, err := hook.NewGitlabAPI(config.Gitlab{
+			c, err := NewGitlabAPI(config.Gitlab{
 				URL:             serverURL,
 				RelativeURLRoot: tc.relativeURLRoot,
 				SecretFile:      secretFilePath,
@@ -217,7 +216,7 @@ func TestEscapedAndRelativeURLs(t *testing.T) {
 	}
 }
 
-func TestAllowedResponseHandling(t *testing.T) {
+func TestAccess_allowedResponseHandling(t *testing.T) {
 	testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t)
 
 	// set git quarantine directories
@@ -337,7 +336,7 @@ func TestAllowedResponseHandling(t *testing.T) {
 			server := httptest.NewServer(http.HandlerFunc(tc.allowedHandler))
 			defer server.Close()
 
-			c, err := hook.NewGitlabAPI(config.Gitlab{
+			c, err := NewGitlabAPI(config.Gitlab{
 				URL:        server.URL,
 				SecretFile: secretFilePath,
 			}, config.TLS{})
@@ -354,7 +353,7 @@ func TestAllowedResponseHandling(t *testing.T) {
 	}
 }
 
-func TestPrereceive(t *testing.T) {
+func TestAccess_preReceive(t *testing.T) {
 	tempDir, cleanup := testhelper.TempDir(t)
 	defer cleanup()
 
@@ -425,7 +424,7 @@ func TestPrereceive(t *testing.T) {
 			server := httptest.NewServer(http.HandlerFunc(tc.prereceiveHandler))
 			defer server.Close()
 
-			c, err := hook.NewGitlabAPI(config.Gitlab{
+			c, err := NewGitlabAPI(config.Gitlab{
 				URL:        server.URL,
 				SecretFile: secretFilePath,
 			}, config.TLS{})
@@ -440,7 +439,7 @@ func TestPrereceive(t *testing.T) {
 	}
 }
 
-func TestPostReceive(t *testing.T) {
+func TestAccess_postReceive(t *testing.T) {
 	tempDir, cleanup := testhelper.TempDir(t)
 	defer cleanup()
 
@@ -501,7 +500,7 @@ func TestPostReceive(t *testing.T) {
 			server := httptest.NewServer(http.HandlerFunc(tc.postReceiveHandler))
 			defer server.Close()
 
-			c, err := hook.NewGitlabAPI(config.Gitlab{
+			c, err := NewGitlabAPI(config.Gitlab{
 				URL:        server.URL,
 				SecretFile: secretFilePath,
 			}, config.TLS{})
diff --git a/internal/gitaly/hook/postreceive.go b/internal/gitaly/hook/postreceive.go
index 2c77f612c..1ed7ee91b 100644
--- a/internal/gitaly/hook/postreceive.go
+++ b/internal/gitaly/hook/postreceive.go
@@ -131,6 +131,12 @@ func (m *GitLabHookManager) PostReceiveHook(ctx context.Context, repo *gitalypb.
 	}
 
 	glID, glRepo := getEnvVar("GL_ID", env), getEnvVar("GL_REPOSITORY", env)
+	if glID == "" {
+		return helper.ErrInternalf("GL_ID not set")
+	}
+	if glRepo == "" {
+		return helper.ErrInternalf("GL_REPOSITORY not set")
+	}
 
 	ok, messages, err := m.gitlabAPI.PostReceive(ctx, glRepo, glID, string(changes), pushOptions...)
 	if err != nil {
diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go
index 7ac260cab..2e3aff181 100644
--- a/internal/gitaly/hook/postreceive_test.go
+++ b/internal/gitaly/hook/postreceive_test.go
@@ -2,10 +2,18 @@ package hook
 
 import (
 	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
 func TestPrintAlert(t *testing.T) {
@@ -48,3 +56,293 @@ func TestPrintAlert(t *testing.T) {
 		assert.Equal(t, tc.expected, result.String())
 	}
 }
+
+func envWithout(envVars []string, value string) []string {
+	result := make([]string, 0, len(envVars))
+	for _, envVar := range envVars {
+		if !strings.HasPrefix(envVar, fmt.Sprintf("%s=", value)) {
+			result = append(result, envVar)
+		}
+	}
+	return result
+}
+
+func TestPostReceive_customHook(t *testing.T) {
+	repo, repoPath, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	hookManager := NewManager(GitlabAPIStub, config.Config)
+
+	standardEnv := []string{
+		fmt.Sprintf("GITALY_SOCKET=%s", config.Config.GitalyInternalSocketPath()),
+		"GITALY_TOKEN=secret",
+		"GL_ID=1234",
+		fmt.Sprintf("GL_PROJECT_PATH=%s", repo.GetGlProjectPath()),
+		"GL_PROTOCOL=web",
+		fmt.Sprintf("GL_REPO=%s", repo),
+		fmt.Sprintf("GL_REPOSITORY=%s", repo.GetGlRepository()),
+		"GL_USERNAME=user",
+	}
+
+	primaryEnv, err := metadata.Transaction{
+		ID: 1234, Node: "primary", Primary: true,
+	}.Env()
+	require.NoError(t, err)
+
+	secondaryEnv, err := metadata.Transaction{
+		ID: 1234, Node: "secondary", Primary: false,
+	}.Env()
+	require.NoError(t, err)
+
+	testCases := []struct {
+		desc           string
+		env            []string
+		pushOptions    []string
+		hook           string
+		stdin          string
+		expectedErr    string
+		expectedStdout string
+		expectedStderr string
+	}{
+		{
+			desc:           "hook receives environment variables",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\nenv | grep -e '^GL_' -e '^GITALY_' | sort\n",
+			expectedStdout: strings.Join(standardEnv, "\n") + "\n",
+		},
+		{
+			desc: "push options are passed through",
+			env:  standardEnv,
+			pushOptions: []string{
+				"mr.merge_when_pipeline_succeeds",
+				"mr.create",
+			},
+			hook: "#!/bin/sh\nenv | grep -e '^GIT_PUSH_OPTION' | sort\n",
+			expectedStdout: strings.Join([]string{
+				"GIT_PUSH_OPTION_0=mr.merge_when_pipeline_succeeds",
+				"GIT_PUSH_OPTION_1=mr.create",
+				"GIT_PUSH_OPTION_COUNT=2",
+			}, "\n") + "\n",
+		},
+		{
+			desc:           "hook can write to stderr and stdout",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\necho foo >&1 && echo bar >&2\n",
+			expectedStdout: "foo\n",
+			expectedStderr: "bar\n",
+		},
+		{
+			desc:           "hook receives standard input",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\ncat\n",
+			stdin:          "foo\n",
+			expectedStdout: "foo\n",
+		},
+		{
+			desc:           "hook succeeds without consuming stdin",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\necho foo\n",
+			stdin:          "ignore me\n",
+			expectedStdout: "foo\n",
+		},
+		{
+			desc:        "invalid hook results in error",
+			env:         standardEnv,
+			hook:        "",
+			expectedErr: "exec format error",
+		},
+		{
+			desc:        "failing hook results in error",
+			env:         standardEnv,
+			hook:        "#!/bin/sh\nexit 123",
+			expectedErr: "exit status 123",
+		},
+		{
+			desc:           "hook is executed on primary",
+			env:            append(standardEnv, primaryEnv),
+			hook:           "#!/bin/sh\necho foo\n",
+			expectedStdout: "foo\n",
+		},
+		{
+			desc: "hook is not executed on secondary",
+			env:  append(standardEnv, secondaryEnv),
+			hook: "#!/bin/sh\necho foo\n",
+		},
+		{
+			desc:        "missing GL_ID causes error",
+			env:         envWithout(standardEnv, "GL_ID"),
+			expectedErr: "GL_ID not set",
+		},
+		{
+			desc:        "missing GL_REPOSITORY causes error",
+			env:         envWithout(standardEnv, "GL_REPOSITORY"),
+			expectedErr: "GL_REPOSITORY not set",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cleanup := testhelper.Context()
+			defer cleanup()
+
+			cleanup, err := testhelper.WriteCustomHook(repoPath, "post-receive", []byte(tc.hook))
+			require.NoError(t, err)
+			defer cleanup()
+
+			var stdout, stderr bytes.Buffer
+			err = hookManager.PostReceiveHook(ctx, repo, tc.pushOptions, tc.env, strings.NewReader(tc.stdin), &stdout, &stderr)
+
+			if tc.expectedErr != "" {
+				require.Contains(t, err.Error(), tc.expectedErr)
+			} else {
+				require.NoError(t, err)
+			}
+
+			require.Equal(t, tc.expectedStdout, stdout.String())
+			require.Equal(t, tc.expectedStderr, stderr.String())
+		})
+	}
+}
+
+type postreceiveAPIMock struct {
+	postreceive func(context.Context, string, string, string, ...string) (bool, []PostReceiveMessage, error)
+}
+
+func (m *postreceiveAPIMock) Allowed(ctx context.Context, repo *gitalypb.Repository, glRepository, glID, glProtocol, changes string) (bool, string, error) {
+	return true, "", nil
+}
+
+func (m *postreceiveAPIMock) PreReceive(ctx context.Context, glRepository string) (bool, error) {
+	return true, nil
+}
+
+func (m *postreceiveAPIMock) Check(ctx context.Context) (*CheckInfo, error) {
+	return nil, errors.New("unexpected call")
+}
+
+func (m *postreceiveAPIMock) PostReceive(ctx context.Context, glRepository, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) {
+	return m.postreceive(ctx, glRepository, glID, changes, pushOptions...)
+}
+
+func TestPostReceive_gitlab(t *testing.T) {
+	testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	standardEnv := []string{
+		fmt.Sprintf("GITALY_SOCKET=%s", config.Config.GitalyInternalSocketPath()),
+		"GITALY_TOKEN=secret",
+		"GL_ID=1234",
+		fmt.Sprintf("GL_PROJECT_PATH=%s", testRepo.GetGlProjectPath()),
+		"GL_PROTOCOL=web",
+		fmt.Sprintf("GL_REPO=%s", testRepo),
+		fmt.Sprintf("GL_REPOSITORY=%s", testRepo.GetGlRepository()),
+		"GL_USERNAME=user",
+	}
+
+	testCases := []struct {
+		desc           string
+		env            []string
+		pushOptions    []string
+		changes        string
+		postreceive    func(*testing.T, context.Context, string, string, string, ...string) (bool, []PostReceiveMessage, error)
+		expectHookCall bool
+		expectedErr    error
+		expectedStdout string
+		expectedStderr string
+	}{
+		{
+			desc:    "allowed change",
+			env:     standardEnv,
+			changes: "changes\n",
+			postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) {
+				require.Equal(t, testRepo.GlRepository, glRepo)
+				require.Equal(t, "1234", glID)
+				require.Equal(t, "changes\n", changes)
+				require.Empty(t, pushOptions)
+				return true, nil, nil
+			},
+			expectedStdout: "hook called\n",
+		},
+		{
+			desc: "push options are passed through",
+			env:  standardEnv,
+			pushOptions: []string{
+				"mr.merge_when_pipeline_succeeds",
+				"mr.create",
+			},
+			changes: "changes\n",
+			postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) {
+				require.Equal(t, []string{
+					"mr.merge_when_pipeline_succeeds",
+					"mr.create",
+				}, pushOptions)
+				return true, nil, nil
+			},
+			expectedStdout: "hook called\n",
+		},
+		{
+			desc:    "access denied without message",
+			env:     standardEnv,
+			changes: "changes\n",
+			postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) {
+				return false, nil, nil
+			},
+			expectedErr: errors.New(""),
+		},
+		{
+			desc:    "access denied with message",
+			env:     standardEnv,
+			changes: "changes\n",
+			postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) {
+				return false, []PostReceiveMessage{
+					{
+						Message: "access denied",
+						Type:    "alert",
+					},
+				}, nil
+			},
+			expectedStdout: "\n========================================================================\n\n                             access denied\n\n========================================================================\n\n",
+			expectedErr:    errors.New(""),
+		},
+		{
+			desc:    "access check returns error",
+			env:     standardEnv,
+			changes: "changes\n",
+			postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) {
+				return false, nil, errors.New("failure")
+			},
+			expectedErr: errors.New("GitLab: failure"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cleanup := testhelper.Context()
+			defer cleanup()
+
+			gitlabAPI := postreceiveAPIMock{
+				postreceive: func(ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) {
+					return tc.postreceive(t, ctx, glRepo, glID, changes, pushOptions...)
+				},
+			}
+
+			hookManager := NewManager(&gitlabAPI, config.Config)
+
+			cleanup, err := testhelper.WriteCustomHook(testRepoPath, "post-receive", []byte("#!/bin/sh\necho hook called\n"))
+			require.NoError(t, err)
+			defer cleanup()
+
+			var stdout, stderr bytes.Buffer
+			err = hookManager.PostReceiveHook(ctx, testRepo, tc.pushOptions, tc.env, strings.NewReader(tc.changes), &stdout, &stderr)
+
+			if tc.expectedErr != nil {
+				require.Equal(t, tc.expectedErr, err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			require.Equal(t, tc.expectedStdout, stdout.String())
+			require.Equal(t, tc.expectedStderr, stderr.String())
+		})
+	}
+}
diff --git a/internal/gitaly/hook/prereceive.go b/internal/gitaly/hook/prereceive.go
index 2754b572d..021f00cf1 100644
--- a/internal/gitaly/hook/prereceive.go
+++ b/internal/gitaly/hook/prereceive.go
@@ -80,6 +80,15 @@ func (m *GitLabHookManager) preReceiveHook(ctx context.Context, repo *gitalypb.R
 	}
 
 	glID, glRepo, glProtocol := getEnvVar("GL_ID", env), getEnvVar("GL_REPOSITORY", env), getEnvVar("GL_PROTOCOL", env)
+	if glID == "" {
+		return helper.ErrInternalf("GL_ID not set")
+	}
+	if glRepo == "" {
+		return helper.ErrInternalf("GL_REPOSITORY not set")
+	}
+	if glProtocol == "" {
+		return helper.ErrInternalf("GL_PROTOCOL not set")
+	}
 
 	allowed, message, err := m.gitlabAPI.Allowed(ctx, repo, glRepo, glID, glProtocol, string(changes))
 	if err != nil {
diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go
new file mode 100644
index 000000000..19abfd099
--- /dev/null
+++ b/internal/gitaly/hook/prereceive_test.go
@@ -0,0 +1,290 @@
+package hook
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
+)
+
+func TestPrereceive_customHooks(t *testing.T) {
+	repo, repoPath, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	hookManager := NewManager(GitlabAPIStub, config.Config)
+
+	standardEnv := []string{
+		fmt.Sprintf("GITALY_SOCKET=%s", config.Config.GitalyInternalSocketPath()),
+		"GITALY_TOKEN=secret",
+		"GL_ID=1234",
+		fmt.Sprintf("GL_PROJECT_PATH=%s", repo.GetGlProjectPath()),
+		"GL_PROTOCOL=web",
+		fmt.Sprintf("GL_REPO=%s", repo),
+		fmt.Sprintf("GL_REPOSITORY=%s", repo.GetGlRepository()),
+		"GL_USERNAME=user",
+	}
+
+	primaryEnv, err := metadata.Transaction{
+		ID: 1234, Node: "primary", Primary: true,
+	}.Env()
+	require.NoError(t, err)
+
+	secondaryEnv, err := metadata.Transaction{
+		ID: 1234, Node: "secondary", Primary: false,
+	}.Env()
+	require.NoError(t, err)
+
+	testCases := []struct {
+		desc           string
+		env            []string
+		hook           string
+		stdin          string
+		expectedErr    string
+		expectedStdout string
+		expectedStderr string
+	}{
+		{
+			desc:           "hook receives environment variables",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\nenv | grep -e '^GL_' -e '^GITALY_' | sort\n",
+			expectedStdout: strings.Join(standardEnv, "\n") + "\n",
+		},
+		{
+			desc:           "hook can write to stderr and stdout",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\necho foo >&1 && echo bar >&2\n",
+			expectedStdout: "foo\n",
+			expectedStderr: "bar\n",
+		},
+		{
+			desc:           "hook receives standard input",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\ncat\n",
+			stdin:          "foo\n",
+			expectedStdout: "foo\n",
+		},
+		{
+			desc:           "hook succeeds without consuming stdin",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\necho foo\n",
+			stdin:          "ignore me\n",
+			expectedStdout: "foo\n",
+		},
+		{
+			desc:        "invalid hook results in error",
+			env:         standardEnv,
+			hook:        "",
+			expectedErr: "exec format error",
+		},
+		{
+			desc:        "failing hook results in error",
+			env:         standardEnv,
+			hook:        "#!/bin/sh\nexit 123",
+			expectedErr: "exit status 123",
+		},
+		{
+			desc:           "hook is executed on primary",
+			env:            append(standardEnv, primaryEnv),
+			hook:           "#!/bin/sh\necho foo\n",
+			expectedStdout: "foo\n",
+		},
+		{
+			desc: "hook is not executed on secondary",
+			env:  append(standardEnv, secondaryEnv),
+			hook: "#!/bin/sh\necho foo\n",
+		},
+		{
+			desc:        "missing GL_ID causes error",
+			env:         envWithout(standardEnv, "GL_ID"),
+			expectedErr: "GL_ID not set",
+		},
+		{
+			desc:        "missing GL_REPOSITORY causes error",
+			env:         envWithout(standardEnv, "GL_REPOSITORY"),
+			expectedErr: "GL_REPOSITORY not set",
+		},
+		{
+			desc:        "missing GL_PROTOCOL causes error",
+			env:         envWithout(standardEnv, "GL_PROTOCOL"),
+			expectedErr: "GL_PROTOCOL not set",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cleanup := testhelper.Context()
+			defer cleanup()
+
+			cleanup, err := testhelper.WriteCustomHook(repoPath, "pre-receive", []byte(tc.hook))
+			require.NoError(t, err)
+			defer cleanup()
+
+			var stdout, stderr bytes.Buffer
+			err = hookManager.PreReceiveHook(ctx, repo, tc.env, strings.NewReader(tc.stdin), &stdout, &stderr)
+
+			if tc.expectedErr != "" {
+				require.Contains(t, err.Error(), tc.expectedErr)
+			} else {
+				require.NoError(t, err)
+			}
+
+			require.Equal(t, tc.expectedStdout, stdout.String())
+			require.Equal(t, tc.expectedStderr, stderr.String())
+		})
+	}
+}
+
+type prereceiveAPIMock struct {
+	allowed    func(context.Context, *gitalypb.Repository, string, string, string, string) (bool, string, error)
+	prereceive func(context.Context, string) (bool, error)
+}
+
+func (m *prereceiveAPIMock) Allowed(ctx context.Context, repo *gitalypb.Repository, glRepository, glID, glProtocol, changes string) (bool, string, error) {
+	return m.allowed(ctx, repo, glRepository, glID, glProtocol, changes)
+}
+
+func (m *prereceiveAPIMock) PreReceive(ctx context.Context, glRepository string) (bool, error) {
+	return m.prereceive(ctx, glRepository)
+}
+
+func (m *prereceiveAPIMock) Check(ctx context.Context) (*CheckInfo, error) {
+	return nil, errors.New("unexpected call")
+}
+
+func (m *prereceiveAPIMock) PostReceive(context.Context, string, string, string, ...string) (bool, []PostReceiveMessage, error) {
+	return true, nil, errors.New("unexpected call")
+}
+
+func TestPrereceive_gitlab(t *testing.T) {
+	testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	standardEnv := []string{
+		fmt.Sprintf("GITALY_SOCKET=%s", config.Config.GitalyInternalSocketPath()),
+		"GITALY_TOKEN=secret",
+		"GL_ID=1234",
+		fmt.Sprintf("GL_PROJECT_PATH=%s", testRepo.GetGlProjectPath()),
+		"GL_PROTOCOL=web",
+		fmt.Sprintf("GL_REPO=%s", testRepo),
+		fmt.Sprintf("GL_REPOSITORY=%s", testRepo.GetGlRepository()),
+		"GL_USERNAME=user",
+	}
+
+	testCases := []struct {
+		desc           string
+		env            []string
+		changes        string
+		allowed        func(*testing.T, context.Context, *gitalypb.Repository, string, string, string, string) (bool, string, error)
+		prereceive     func(*testing.T, context.Context, string) (bool, error)
+		expectHookCall bool
+		expectedErr    error
+	}{
+		{
+			desc:    "allowed change",
+			env:     standardEnv,
+			changes: "changes\n",
+			allowed: func(t *testing.T, ctx context.Context, repo *gitalypb.Repository, glRepo, glID, glProtocol, changes string) (bool, string, error) {
+				require.Equal(t, testRepo, repo)
+				require.Equal(t, testRepo.GlRepository, glRepo)
+				require.Equal(t, "1234", glID)
+				require.Equal(t, "web", glProtocol)
+				require.Equal(t, "changes\n", changes)
+				return true, "", nil
+			},
+			prereceive: func(t *testing.T, ctx context.Context, glRepo string) (bool, error) {
+				require.Equal(t, testRepo.GlRepository, glRepo)
+				return true, nil
+			},
+			expectHookCall: true,
+		},
+		{
+			desc: "disallowed change",
+			env:  standardEnv,
+			allowed: func(t *testing.T, ctx context.Context, repo *gitalypb.Repository, glRepo, glID, glProtocol, changes string) (bool, string, error) {
+				return false, "you shall not pass", nil
+			},
+			expectHookCall: false,
+			expectedErr:    errors.New("you shall not pass"),
+		},
+		{
+			desc: "allowed returns error",
+			env:  standardEnv,
+			allowed: func(t *testing.T, ctx context.Context, repo *gitalypb.Repository, glRepo, glID, glProtocol, changes string) (bool, string, error) {
+				return false, "", errors.New("oops")
+			},
+			expectHookCall: false,
+			expectedErr:    errors.New("GitLab: oops"),
+		},
+		{
+			desc: "prereceive rejects",
+			env:  standardEnv,
+			allowed: func(t *testing.T, ctx context.Context, repo *gitalypb.Repository, glRepo, glID, glProtocol, changes string) (bool, string, error) {
+				return true, "", nil
+			},
+			prereceive: func(t *testing.T, ctx context.Context, glRepo string) (bool, error) {
+				return false, nil
+			},
+			expectHookCall: true,
+			expectedErr:    errors.New(""),
+		},
+		{
+			desc: "prereceive errors",
+			env:  standardEnv,
+			allowed: func(t *testing.T, ctx context.Context, repo *gitalypb.Repository, glRepo, glID, glProtocol, changes string) (bool, string, error) {
+				return true, "", nil
+			},
+			prereceive: func(t *testing.T, ctx context.Context, glRepo string) (bool, error) {
+				return false, errors.New("prereceive oops")
+			},
+			expectHookCall: true,
+			expectedErr:    helper.ErrInternalf("calling pre_receive endpoint: %v", errors.New("prereceive oops")),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cleanup := testhelper.Context()
+			defer cleanup()
+
+			gitlabAPI := prereceiveAPIMock{
+				allowed: func(ctx context.Context, repo *gitalypb.Repository, glRepo, glID, glProtocol, changes string) (bool, string, error) {
+					return tc.allowed(t, ctx, repo, glRepo, glID, glProtocol, changes)
+				},
+				prereceive: func(ctx context.Context, glRepo string) (bool, error) {
+					return tc.prereceive(t, ctx, glRepo)
+				},
+			}
+
+			hookManager := NewManager(&gitlabAPI, config.Config)
+
+			cleanup, err := testhelper.WriteCustomHook(testRepoPath, "pre-receive", []byte("#!/bin/sh\necho called\n"))
+			require.NoError(t, err)
+			defer cleanup()
+
+			var stdout, stderr bytes.Buffer
+			err = hookManager.PreReceiveHook(ctx, testRepo, tc.env, strings.NewReader(tc.changes), &stdout, &stderr)
+
+			if tc.expectedErr != nil {
+				require.Equal(t, tc.expectedErr, err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			if tc.expectHookCall {
+				require.Equal(t, "called\n", stdout.String())
+			} else {
+				require.Empty(t, stdout.String())
+			}
+			require.Empty(t, stderr.String())
+		})
+	}
+}
diff --git a/internal/gitaly/hook/update.go b/internal/gitaly/hook/update.go
index 2290ff427..54061c1a0 100644
--- a/internal/gitaly/hook/update.go
+++ b/internal/gitaly/hook/update.go
@@ -9,6 +9,14 @@ import (
 )
 
 func (m *GitLabHookManager) UpdateHook(ctx context.Context, repo *gitalypb.Repository, ref, oldValue, newValue string, env []string, stdout, stderr io.Writer) error {
+	primary, err := isPrimary(env)
+	if err != nil {
+		return helper.ErrInternalf("could not check role: %w", err)
+	}
+	if !primary {
+		return nil
+	}
+
 	executor, err := m.newCustomHooksExecutor(repo, "update")
 	if err != nil {
 		return helper.ErrInternal(err)
diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go
new file mode 100644
index 000000000..347522068
--- /dev/null
+++ b/internal/gitaly/hook/update_test.go
@@ -0,0 +1,159 @@
+package hook
+
+import (
+	"bytes"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestUpdate_customHooks(t *testing.T) {
+	repo, repoPath, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	hookManager := NewManager(GitlabAPIStub, config.Config)
+
+	standardEnv := []string{
+		fmt.Sprintf("GITALY_SOCKET=%s", config.Config.GitalyInternalSocketPath()),
+		"GITALY_TOKEN=secret",
+		"GL_ID=1234",
+		fmt.Sprintf("GL_PROJECT_PATH=%s", repo.GetGlProjectPath()),
+		"GL_PROTOCOL=web",
+		fmt.Sprintf("GL_REPO=%s", repo),
+		fmt.Sprintf("GL_REPOSITORY=%s", repo.GetGlRepository()),
+		"GL_USERNAME=user",
+	}
+
+	primaryEnv, err := metadata.Transaction{
+		ID: 1234, Node: "primary", Primary: true,
+	}.Env()
+	require.NoError(t, err)
+
+	secondaryEnv, err := metadata.Transaction{
+		ID: 1234, Node: "secondary", Primary: false,
+	}.Env()
+	require.NoError(t, err)
+
+	hash1 := strings.Repeat("1", 40)
+	hash2 := strings.Repeat("2", 40)
+
+	testCases := []struct {
+		desc           string
+		env            []string
+		hook           string
+		reference      string
+		oldHash        string
+		newHash        string
+		expectedErr    string
+		expectedStdout string
+		expectedStderr string
+	}{
+		{
+			desc:           "hook receives environment variables",
+			env:            standardEnv,
+			hook:           "#!/bin/sh\nenv | grep -e '^GL_' -e '^GITALY_' | sort\n",
+			expectedStdout: strings.Join(standardEnv, "\n") + "\n",
+		},
+		{
+			desc:           "hook receives arguments",
+			env:            standardEnv,
+			reference:      "refs/heads/master",
+			oldHash:        hash1,
+			newHash:        hash2,
+			hook:           "#!/bin/sh\nprintf '%s\\n' \"$@\"\n",
+			expectedStdout: fmt.Sprintf("refs/heads/master\n%s\n%s\n", hash1, hash2),
+		},
+		{
+			desc:           "stdout and stderr are passed through",
+			env:            standardEnv,
+			reference:      "refs/heads/master",
+			oldHash:        hash1,
+			newHash:        hash2,
+			hook:           "#!/bin/sh\necho foo >&1\necho bar >&2\n",
+			expectedStdout: "foo\n",
+			expectedStderr: "bar\n",
+		},
+		{
+			desc:      "standard input is empty",
+			env:       standardEnv,
+			reference: "refs/heads/master",
+			oldHash:   hash1,
+			newHash:   hash2,
+			hook:      "#!/bin/sh\ncat\n",
+		},
+		{
+			desc:        "invalid script causes failure",
+			env:         standardEnv,
+			reference:   "refs/heads/master",
+			oldHash:     hash1,
+			newHash:     hash2,
+			hook:        "",
+			expectedErr: "exec format error",
+		},
+		{
+			desc:        "errors are passed through",
+			env:         standardEnv,
+			reference:   "refs/heads/master",
+			oldHash:     hash1,
+			newHash:     hash2,
+			hook:        "#!/bin/sh\nexit 123\n",
+			expectedErr: "exit status 123",
+		},
+		{
+			desc:           "errors are passed through with stderr and stdout",
+			env:            standardEnv,
+			reference:      "refs/heads/master",
+			oldHash:        hash1,
+			newHash:        hash2,
+			hook:           "#!/bin/sh\necho foo >&1\necho bar >&2\nexit 123\n",
+			expectedStdout: "foo\n",
+			expectedStderr: "bar\n",
+			expectedErr:    "exit status 123",
+		},
+		{
+			desc:           "hook is executed on primary",
+			env:            append(standardEnv, primaryEnv),
+			reference:      "refs/heads/master",
+			oldHash:        hash1,
+			newHash:        hash2,
+			hook:           "#!/bin/sh\necho foo\n",
+			expectedStdout: "foo\n",
+		},
+		{
+			desc:      "hook is not executed on secondary",
+			env:       append(standardEnv, secondaryEnv),
+			reference: "refs/heads/master",
+			oldHash:   hash1,
+			newHash:   hash2,
+			hook:      "#!/bin/sh\necho foo\n",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cleanup := testhelper.Context()
+			defer cleanup()
+
+			cleanup, err := testhelper.WriteCustomHook(repoPath, "update", []byte(tc.hook))
+			require.NoError(t, err)
+			defer cleanup()
+
+			var stdout, stderr bytes.Buffer
+			err = hookManager.UpdateHook(ctx, repo, tc.reference, tc.oldHash, tc.newHash, tc.env, &stdout, &stderr)
+
+			if tc.expectedErr != "" {
+				require.Contains(t, err.Error(), tc.expectedErr)
+			} else {
+				require.NoError(t, err)
+			}
+
+			require.Equal(t, tc.expectedStdout, stdout.String())
+			require.Equal(t, tc.expectedStderr, stderr.String())
+		})
+	}
+}
diff --git a/internal/gitaly/rubyserver/rubyserver.go b/internal/gitaly/rubyserver/rubyserver.go
index 16cde6095..c796a408a 100644
--- a/internal/gitaly/rubyserver/rubyserver.go
+++ b/internal/gitaly/rubyserver/rubyserver.go
@@ -78,7 +78,7 @@ func (s *Server) start() error {
 
 	cfg := config.Config
 
-	gitlabshellEnv, err := gitlabshell.Env()
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(cfg)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/hook/post_receive.go b/internal/gitaly/service/hook/post_receive.go
index fb2d0ee92..546e987e5 100644
--- a/internal/gitaly/service/hook/post_receive.go
+++ b/internal/gitaly/service/hook/post_receive.go
@@ -45,7 +45,7 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
 		return stream.Send(&gitalypb.PostReceiveHookResponse{Stderr: p})
 	})
 
-	env, err := hookRequestEnv(firstRequest)
+	env, err := s.hookRequestEnv(firstRequest)
 	if err != nil {
 		return helper.ErrInternal(err)
 	}
diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go
index d5708e3d8..6e1ebaaf6 100644
--- a/internal/gitaly/service/hook/pre_receive.go
+++ b/internal/gitaly/service/hook/pre_receive.go
@@ -23,16 +23,16 @@ type prePostRequest interface {
 	GetGitPushOptions() []string
 }
 
-func hookRequestEnv(req hookRequest) ([]string, error) {
-	gitlabshellEnv, err := gitlabshell.Env()
+func (s *server) hookRequestEnv(req hookRequest) ([]string, error) {
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(s.cfg)
 	if err != nil {
 		return nil, err
 	}
 	return append(gitlabshellEnv, req.GetEnvironmentVariables()...), nil
 }
 
-func preReceiveEnv(req prePostRequest) ([]string, error) {
-	env, err := hookRequestEnv(req)
+func (s *server) preReceiveEnv(req prePostRequest) ([]string, error) {
+	env, err := s.hookRequestEnv(req)
 	if err != nil {
 		return nil, err
 	}
@@ -67,7 +67,7 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
 		return stream.Send(&gitalypb.PreReceiveHookResponse{Stderr: p})
 	})
 
-	env, err := preReceiveEnv(firstRequest)
+	env, err := s.preReceiveEnv(firstRequest)
 	if err != nil {
 		return fmt.Errorf("getting env vars from request: %v", err)
 	}
diff --git a/internal/gitaly/service/hook/server.go b/internal/gitaly/service/hook/server.go
index fd7086655..134e5911d 100644
--- a/internal/gitaly/service/hook/server.go
+++ b/internal/gitaly/service/hook/server.go
@@ -1,17 +1,20 @@
 package hook
 
 import (
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
 type server struct {
+	cfg     config.Cfg
 	manager gitalyhook.Manager
 }
 
 // NewServer creates a new instance of a gRPC namespace server
-func NewServer(manager gitalyhook.Manager) gitalypb.HookServiceServer {
+func NewServer(cfg config.Cfg, manager gitalyhook.Manager) gitalypb.HookServiceServer {
 	return &server{
+		cfg:     cfg,
 		manager: manager,
 	}
 }
diff --git a/internal/gitaly/service/hook/testhelper_test.go b/internal/gitaly/service/hook/testhelper_test.go
index 9881278e7..97f5fa8c9 100644
--- a/internal/gitaly/service/hook/testhelper_test.go
+++ b/internal/gitaly/service/hook/testhelper_test.go
@@ -45,7 +45,7 @@ func runHooksServer(t *testing.T, cfg config.Cfg) (string, func()) {
 func runHooksServerWithAPI(t *testing.T, gitlabAPI gitalyhook.GitlabAPI, cfg config.Cfg) (string, func()) {
 	srv := testhelper.NewServer(t, nil, nil)
 
-	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), NewServer(gitalyhook.NewManager(gitlabAPI, cfg)))
+	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), NewServer(cfg, gitalyhook.NewManager(gitlabAPI, cfg)))
 	reflection.Register(srv.GrpcServer())
 
 	require.NoError(t, srv.Start())
diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go
index 4caea6dee..7ba048958 100644
--- a/internal/gitaly/service/operations/branches_test.go
+++ b/internal/gitaly/service/operations/branches_test.go
@@ -129,7 +129,7 @@ func testUserCreateBranchWithTransaction(t *testing.T, withRefTxHook bool) {
 	server := NewServer(config.Config, RubyServer, hookManager, locator, conns)
 
 	gitalypb.RegisterOperationServiceServer(srv.GrpcServer(), server)
-	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hook.NewServer(hookManager))
+	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hook.NewServer(config.Config, hookManager))
 	gitalypb.RegisterRefTransactionServer(srv.GrpcServer(), transactionServer)
 
 	require.NoError(t, srv.Start())
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index e42fce7ec..0439f4ec9 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -117,7 +117,7 @@ func hookErrorFromStdoutAndStderr(sout string, serr string) string {
 }
 
 func (s *server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Repository, user *gitalypb.User, reference, newrev, oldrev string) error {
-	gitlabshellEnv, err := gitlabshell.Env()
+	gitlabshellEnv, err := gitlabshell.EnvFromConfig(s.cfg)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go
index 029f0591d..b1182a43c 100644
--- a/internal/gitaly/service/operations/testhelper_test.go
+++ b/internal/gitaly/service/operations/testhelper_test.go
@@ -101,7 +101,7 @@ func runOperationServiceServerWithRubyServer(t *testing.T, ruby *rubyserver.Serv
 	server := NewServer(config.Config, ruby, hookManager, locator, conns)
 
 	gitalypb.RegisterOperationServiceServer(srv.GrpcServer(), server)
-	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hook.NewServer(hookManager))
+	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hook.NewServer(config.Config, hookManager))
 	gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), repository.NewServer(config.Config, ruby, locator, internalSocket))
 	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), ref.NewServer(locator))
 	gitalypb.RegisterCommitServiceServer(srv.GrpcServer(), commit.NewServer(locator))
diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index ed6aed76a..761752e1e 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -90,7 +90,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
 	gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(rubyServer, locator))
 	gitalypb.RegisterServerServiceServer(grpcServer, server.NewServer(cfg.Storages))
 	gitalypb.RegisterObjectPoolServiceServer(grpcServer, objectpool.NewServer(locator))
-	gitalypb.RegisterHookServiceServer(grpcServer, hook.NewServer(hookManager))
+	gitalypb.RegisterHookServiceServer(grpcServer, hook.NewServer(cfg, hookManager))
 	gitalypb.RegisterInternalGitalyServer(grpcServer, internalgitaly.NewServer(cfg.Storages))
 
 	healthpb.RegisterHealthServer(grpcServer, health.NewServer())
diff --git a/internal/gitaly/service/smarthttp/receive_pack.go b/internal/gitaly/service/smarthttp/receive_pack.go
index 119fe6c2f..a04a69c99 100644
--- a/internal/gitaly/service/smarthttp/receive_pack.go
+++ b/internal/gitaly/service/smarthttp/receive_pack.go
@@ -54,7 +54,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 			Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}},
 			Args:  []string{repoPath},
 		},
-		git.WithReceivePackHooks(ctx, req, "http"),
+		git.WithReceivePackHooks(ctx, config.Config, req, "http"),
 		git.WithGitProtocol(ctx, req),
 		git.WithRefTxHook(ctx, req.Repository, config.Config),
 	)
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index 9a1bc7008..85475491f 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -445,7 +445,7 @@ func runSmartHTTPHookServiceServer(t *testing.T) (*grpc.Server, string) {
 	}
 
 	gitalypb.RegisterSmartHTTPServiceServer(server, NewServer(config.NewLocator(config.Config)))
-	gitalypb.RegisterHookServiceServer(server, hook.NewServer(gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
+	gitalypb.RegisterHookServiceServer(server, hook.NewServer(config.Config, gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
 	reflection.Register(server)
 
 	go server.Serve(listener)
@@ -507,7 +507,7 @@ func TestPostReceiveWithTransactionsViaPraefect(t *testing.T) {
 
 	gitalyServer := testhelper.NewServerWithAuth(t, nil, nil, config.Config.Auth.Token)
 	gitalypb.RegisterSmartHTTPServiceServer(gitalyServer.GrpcServer(), NewServer(config.NewLocator(config.Config)))
-	gitalypb.RegisterHookServiceServer(gitalyServer.GrpcServer(), hook.NewServer(gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
+	gitalypb.RegisterHookServiceServer(gitalyServer.GrpcServer(), hook.NewServer(config.Config, gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
 	reflection.Register(gitalyServer.GrpcServer())
 	require.NoError(t, gitalyServer.Start())
 	defer gitalyServer.Stop()
@@ -554,7 +554,7 @@ func TestPostReceiveWithReferenceTransactionHook(t *testing.T) {
 
 	gitalyServer := testhelper.NewTestGrpcServer(t, nil, nil)
 	gitalypb.RegisterSmartHTTPServiceServer(gitalyServer, NewServer(config.NewLocator(config.Config)))
-	gitalypb.RegisterHookServiceServer(gitalyServer, hook.NewServer(gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
+	gitalypb.RegisterHookServiceServer(gitalyServer, hook.NewServer(config.Config, gitalyhook.NewManager(gitalyhook.GitlabAPIStub, config.Config)))
 	gitalypb.RegisterRefTransactionServer(gitalyServer, refTransactionServer)
 	reflection.Register(gitalyServer)
 
diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go
index 05dff7b56..d033709f9 100644
--- a/internal/gitaly/service/ssh/receive_pack.go
+++ b/internal/gitaly/service/ssh/receive_pack.go
@@ -71,7 +71,7 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer,
 			Name: "receive-pack",
 			Args: []string{repoPath},
 		},
-		git.WithReceivePackHooks(ctx, req, "ssh"),
+		git.WithReceivePackHooks(ctx, config.Config, req, "ssh"),
 		git.WithGitProtocol(ctx, req),
 		git.WithRefTxHook(ctx, req.Repository, config.Config),
 	)
diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go
index 6a859c967..99091af0b 100644
--- a/internal/gitlabshell/env.go
+++ b/internal/gitlabshell/env.go
@@ -8,13 +8,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/log"
 )
 
-// Env is a helper that returns a slice with environment variables used by gitlab shell
-func Env() ([]string, error) {
-	cfg := config.Config
-
-	return EnvFromConfig(cfg)
-}
-
 type Config struct {
 	CustomHooksDir string              `json:"custom_hooks_dir"`
 	GitlabURL      string              `json:"gitlab_url"`
diff --git a/internal/gitlabshell/env_test.go b/internal/gitlabshell/env_test.go
index ff5627a4a..45ec94c3c 100644
--- a/internal/gitlabshell/env_test.go
+++ b/internal/gitlabshell/env_test.go
@@ -42,7 +42,7 @@ func TestGitHooksConfig(t *testing.T) {
 		SecretFile: "secret_file_path",
 	}
 
-	env, err := gitlabshell.Env()
+	env, err := gitlabshell.EnvFromConfig(config.Config)
 	require.NoError(t, err)
 
 	require.Contains(t, env, "GITALY_GITLAB_SHELL_DIR="+config.Config.GitlabShell.Dir)