ruby: Unify logic to filter allowed Ruby environment variables

When running Ruby commands we need to make sure to expose certain
environment variables to the spawned processes so that they have a valid
Ruby execution environment. This logic is required both by Linguist and
by the Ruby server, but they duplicate the logic.

Create a new `env.AllowedRubyEnvironment()` helper function to unify
this knowledge into a single place. While at it, remove the logic to
handle the generic environment variables `HOME` and `PATH` from
Linguist: they are already injected by our `command` package.

4 files changed, 129 insertions, 31 deletions

diff --git a/internal/gitaly/linguist/linguist.go b/internal/gitaly/linguist/linguist.go
index eeadebd5d..7976ddfb5 100644
--- a/internal/gitaly/linguist/linguist.go
+++ b/internal/gitaly/linguist/linguist.go
@@ -13,10 +13,9 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/env"
 )
 
-var exportedEnvVars = []string{"HOME", "PATH", "GEM_HOME", "BUNDLE_PATH", "BUNDLE_APP_CONFIG", "BUNDLE_USER_CONFIG"}
-
 // Language is used to parse Linguist's language.json file.
 type Language struct {
 	Color string `json:"color"`
@@ -123,7 +122,7 @@ func (inst *Instance) startGitLinguist(ctx context.Context, repoPath string, com
 	cmd := exec.Command(args[0], args[1:]...)
 	cmd.Dir = inst.cfg.Ruby.Dir
 
-	internalCmd, err := command.New(ctx, cmd, nil, nil, nil, exportEnvironment()...)
+	internalCmd, err := command.New(ctx, cmd, nil, nil, nil, env.AllowedRubyEnvironment(os.Environ())...)
 	if err != nil {
 		return nil, fmt.Errorf("creating command: %w", err)
 	}
@@ -173,14 +172,3 @@ func openLanguagesJSON(cfg config.Cfg) (io.ReadCloser, error) {
 
 	return os.Open(filepath.Join(linguistPathSymlink.Name(), "lib", "linguist", "languages.json"))
 }
-
-func exportEnvironment() []string {
-	var env []string
-	for _, envVarName := range exportedEnvVars {
-		if val, ok := os.LookupEnv(envVarName); ok {
-			env = append(env, fmt.Sprintf("%s=%s", envVarName, val))
-		}
-	}
-
-	return env
-}
diff --git a/internal/gitaly/rubyserver/rubyserver.go b/internal/gitaly/rubyserver/rubyserver.go
index d1f454e9c..1ae315cb3 100644
--- a/internal/gitaly/rubyserver/rubyserver.go
+++ b/internal/gitaly/rubyserver/rubyserver.go
@@ -48,7 +48,7 @@ func setupEnv(cfg config.Cfg, gitCmdFactory git.CommandFactory) ([]string, error
 	gitExecEnv := gitCmdFactory.GetExecutionEnvironment(ctx)
 	hooksPath := gitCmdFactory.HooksPath(ctx)
 
-	env := append(
+	environment := append(
 		command.AllowedEnvironment(os.Environ()),
 		"GITALY_LOG_DIR="+cfg.Logging.Dir,
 		"GITALY_RUBY_GIT_BIN_PATH="+gitExecEnv.BinaryPath,
@@ -61,34 +61,25 @@ func setupEnv(cfg config.Cfg, gitCmdFactory git.CommandFactory) ([]string, error
 		"GITALY_TOKEN="+cfg.Auth.Token,
 		"GITALY_RUGGED_GIT_CONFIG_SEARCH_PATH="+cfg.Ruby.RuggedGitConfigSearchPath,
 	)
-	env = append(env, command.GitEnv...)
-	env = append(env, gitExecEnv.EnvironmentVariables...)
-	env = appendEnvIfSet(env, "BUNDLE_PATH")
-	env = appendEnvIfSet(env, "BUNDLE_USER_CONFIG")
-	env = appendEnvIfSet(env, "GEM_HOME")
+	environment = append(environment, command.GitEnv...)
+	environment = append(environment, gitExecEnv.EnvironmentVariables...)
+	environment = append(environment, env.AllowedRubyEnvironment(os.Environ())...)
 
 	gitConfig, err := gitCmdFactory.SidecarGitConfiguration(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("getting Git configuration: %w", err)
 	}
-	env = append(env, git.ConfigPairsToGitEnvironment(gitConfig)...)
+	environment = append(environment, git.ConfigPairsToGitEnvironment(gitConfig)...)
 
 	if dsn := cfg.Logging.RubySentryDSN; dsn != "" {
-		env = append(env, "SENTRY_DSN="+dsn)
+		environment = append(environment, "SENTRY_DSN="+dsn)
 	}
 
 	if sentryEnvironment := cfg.Logging.Sentry.Environment; sentryEnvironment != "" {
-		env = append(env, "SENTRY_ENVIRONMENT="+sentryEnvironment)
+		environment = append(environment, "SENTRY_ENVIRONMENT="+sentryEnvironment)
 	}
 
-	return env, nil
-}
-
-func appendEnvIfSet(envvars []string, key string) []string {
-	if value, ok := os.LookupEnv(key); ok {
-		envvars = append(envvars, fmt.Sprintf("%s=%s", key, value))
-	}
-	return envvars
+	return environment, nil
 }
 
 // Server represents a gitaly-ruby helper process.
diff --git a/internal/helper/env/ruby.go b/internal/helper/env/ruby.go
new file mode 100644
index 000000000..5f585238e
--- /dev/null
+++ b/internal/helper/env/ruby.go
@@ -0,0 +1,22 @@
+package env
+
+import "strings"
+
+// AllowedRubyEnvironment filters the given set of environment variables and returns only those
+// which are allowed when executing Ruby commands.
+func AllowedRubyEnvironment(environmentVariables []string) []string {
+	var filteredEnv []string
+	for _, environmentVariable := range environmentVariables {
+		for _, prefix := range []string{
+			"BUNDLE_PATH=",
+			"BUNDLE_APP_CONFIG=",
+			"BUNDLE_USER_CONFIG=",
+			"GEM_HOME=",
+		} {
+			if strings.HasPrefix(environmentVariable, prefix) {
+				filteredEnv = append(filteredEnv, environmentVariable)
+			}
+		}
+	}
+	return filteredEnv
+}
diff --git a/internal/helper/env/ruby_test.go b/internal/helper/env/ruby_test.go
new file mode 100644
index 000000000..bfc8d8f6f
--- /dev/null
+++ b/internal/helper/env/ruby_test.go
@@ -0,0 +1,97 @@
+package env
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestAllowedRubyEnvironment(t *testing.T) {
+	for _, tc := range []struct {
+		desc     string
+		input    []string
+		expected []string
+	}{
+		{
+			desc: "empty",
+		},
+		{
+			desc: "unrelated env",
+			input: []string{
+				"FOO=bar",
+				"BAR=baz",
+			},
+		},
+		{
+			desc: "allowed envvars",
+			input: []string{
+				"BUNDLE_PATH=a",
+				"BUNDLE_APP_CONFIG=b",
+				"BUNDLE_USER_CONFIG=c",
+				"GEM_HOME=y",
+			},
+			expected: []string{
+				"BUNDLE_PATH=a",
+				"BUNDLE_APP_CONFIG=b",
+				"BUNDLE_USER_CONFIG=c",
+				"GEM_HOME=y",
+			},
+		},
+		{
+			desc: "mixed",
+			input: []string{
+				"FOO=bar",
+				"BUNDLE_PATH=a",
+				"FOO=bar",
+				"BUNDLE_APP_CONFIG=b",
+				"FOO=bar",
+				"BUNDLE_USER_CONFIG=c",
+				"FOO=bar",
+				"GEM_HOME=d",
+				"FOO=bar",
+			},
+			expected: []string{
+				"BUNDLE_PATH=a",
+				"BUNDLE_APP_CONFIG=b",
+				"BUNDLE_USER_CONFIG=c",
+				"GEM_HOME=d",
+			},
+		},
+		{
+			desc: "almost-prefixes",
+			input: []string{
+				"BUNDLE_PATHx=a",
+				"BUNDLE_APP_CONFIGx=b",
+				"BUNDLE_USER_CONFIGx=c",
+				"GEM_HOMEx=d",
+			},
+		},
+		{
+			desc: "duplicate entries",
+			input: []string{
+				"GEM_HOME=first",
+				"BUNDLE_APP_CONFIG=first",
+				"BUNDLE_USER_CONFIG=first",
+				"BUNDLE_PATH=first",
+				"BUNDLE_PATH=second",
+				"BUNDLE_USER_CONFIG=second",
+				"BUNDLE_APP_CONFIG=second",
+				"GEM_HOME=second",
+			},
+			expected: []string{
+				"GEM_HOME=first",
+				"BUNDLE_APP_CONFIG=first",
+				"BUNDLE_USER_CONFIG=first",
+				"BUNDLE_PATH=first",
+				"BUNDLE_PATH=second",
+				"BUNDLE_USER_CONFIG=second",
+				"BUNDLE_APP_CONFIG=second",
+				"GEM_HOME=second",
+			},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			require.Equal(t, tc.expected, AllowedRubyEnvironment(tc.input))
+		})
+	}
+}