From 1886073554e4a88934f5d36683a19ecfd1dbc8e4 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <psteinhardt@gitlab.com>
Date: Thu, 24 Nov 2022 08:27:28 +0100
Subject: [PATCH] hook: Replace broken ad-hoc implementation to get envvars
In the `hook` package we have an ad-hoc implementation to retrieve an
environment variable's value from an array. This implementation has two
problems:
- It retrieves the first value instead of the last value as would be
done by `os.Getenv()` in case the environment variable is defined
multiple times.
- It can segfault in case the environment variable doesn't have an
equals sign in it.
Fix both bugs by instead using `env.ExtractValue()`.
Changelog: fixed
---
internal/gitaly/hook/custom.go | 7 ++++---
internal/gitaly/hook/postreceive.go | 11 -----------
internal/gitaly/hook/prereceive.go | 7 ++++---
3 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/internal/gitaly/hook/custom.go b/internal/gitaly/hook/custom.go
index 9db59adaeb..4fb8d34cda 100644
--- a/internal/gitaly/hook/custom.go
+++ b/internal/gitaly/hook/custom.go
@@ -11,6 +11,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/command"
"gitlab.com/gitlab-org/gitaly/v15/internal/git"
+ "gitlab.com/gitlab-org/gitaly/v15/internal/helper/env"
"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
"golang.org/x/sys/unix"
)
@@ -150,7 +151,7 @@ func (m *GitLabHookManager) customHooksEnv(ctx context.Context, payload git.Hook
customEnvs := append(command.AllowedEnvironment(envs), pushOptionsEnv(pushOptions)...)
- objectDirectory := getEnvVar("GIT_OBJECT_DIRECTORY", envs)
+ objectDirectory := env.ExtractValue(envs, "GIT_OBJECT_DIRECTORY")
if objectDirectory == "" && payload.Repo.GetGitObjectDirectory() != "" {
objectDirectory = filepath.Join(repoPath, payload.Repo.GetGitObjectDirectory())
}
@@ -158,7 +159,7 @@ func (m *GitLabHookManager) customHooksEnv(ctx context.Context, payload git.Hook
customEnvs = append(customEnvs, "GIT_OBJECT_DIRECTORY="+objectDirectory)
}
- alternateObjectDirectories := getEnvVar("GIT_ALTERNATE_OBJECT_DIRECTORIES", envs)
+ alternateObjectDirectories := env.ExtractValue(envs, "GIT_ALTERNATE_OBJECT_DIRECTORIES")
if alternateObjectDirectories == "" && len(payload.Repo.GetGitAlternateObjectDirectories()) != 0 {
var absolutePaths []string
for _, alternateObjectDirectory := range payload.Repo.GetGitAlternateObjectDirectories() {
@@ -176,7 +177,7 @@ func (m *GitLabHookManager) customHooksEnv(ctx context.Context, payload git.Hook
// By default, we should take PATH from the given set of environment variables, if
// it's contained in there. Otherwise, we need to take the current process's PATH
// environment, which would also be the default injected by the command package.
- currentPath := getEnvVar("PATH", envs)
+ currentPath := env.ExtractValue(envs, "PATH")
if currentPath == "" {
currentPath = os.Getenv("PATH")
}
diff --git a/internal/gitaly/hook/postreceive.go b/internal/gitaly/hook/postreceive.go
index cc4394c859..a597204830 100644
--- a/internal/gitaly/hook/postreceive.go
+++ b/internal/gitaly/hook/postreceive.go
@@ -31,17 +31,6 @@ const (
maxMessageTextWidth = maxMessageWidth - 2*terminalMessagePadding
)
-func getEnvVar(key string, vars []string) string {
- for _, varPair := range vars {
- kv := strings.SplitN(varPair, "=", 2)
- if kv[0] == key {
- return kv[1]
- }
- }
-
- return ""
-}
-
func printMessages(messages []gitlab.PostReceiveMessage, w io.Writer) error {
for _, message := range messages {
if _, err := w.Write([]byte("\n")); err != nil {
diff --git a/internal/gitaly/hook/prereceive.go b/internal/gitaly/hook/prereceive.go
index 544800ff0d..ad9e034a52 100644
--- a/internal/gitaly/hook/prereceive.go
+++ b/internal/gitaly/hook/prereceive.go
@@ -13,6 +13,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/git"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitlab"
"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
+ "gitlab.com/gitlab-org/gitaly/v15/internal/helper/env"
"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
)
@@ -90,13 +91,13 @@ func (m *GitLabHookManager) PreReceiveHook(ctx context.Context, repo *gitalypb.R
return nil
}
-func (m *GitLabHookManager) preReceiveHook(ctx context.Context, payload git.HooksPayload, repo *gitalypb.Repository, pushOptions, env []string, changes []byte, stdout, stderr io.Writer) error {
+func (m *GitLabHookManager) preReceiveHook(ctx context.Context, payload git.HooksPayload, repo *gitalypb.Repository, pushOptions, envs []string, changes []byte, stdout, stderr io.Writer) error {
repoPath, err := m.locator.GetRepoPath(repo)
if err != nil {
return helper.ErrInternalf("getting repo path: %v", err)
}
- if gitObjDir, gitAltObjDirs := getEnvVar("GIT_OBJECT_DIRECTORY", env), getEnvVar("GIT_ALTERNATE_OBJECT_DIRECTORIES", env); gitObjDir != "" && gitAltObjDirs != "" {
+ if gitObjDir, gitAltObjDirs := env.ExtractValue(envs, "GIT_OBJECT_DIRECTORY"), env.ExtractValue(envs, "GIT_ALTERNATE_OBJECT_DIRECTORIES"); gitObjDir != "" && gitAltObjDirs != "" {
gitObjectDirRel, gitAltObjectDirRel, err := getRelativeObjectDirs(repoPath, gitObjDir, gitAltObjDirs)
if err != nil {
return helper.ErrInternalf("getting relative git object directories: %v", err)
@@ -165,7 +166,7 @@ func (m *GitLabHookManager) preReceiveHook(ctx context.Context, payload git.Hook
return fmt.Errorf("creating custom hooks executor: %w", err)
}
- customEnv, err := m.customHooksEnv(ctx, payload, pushOptions, env)
+ customEnv, err := m.customHooksEnv(ctx, payload, pushOptions, envs)
if err != nil {
return helper.ErrInternalf("constructing custom hook environment: %v", err)
}
--
GitLab