diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-19 15:49:00 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-23 10:22:51 +0300 |
commit | 2c70420ae66af132e431dbe9402c496edcea54da (patch) | |
tree | c38d4268e97502af8d1687ba4a5782a92b56047d | |
parent | 94055b253d05bc04f533c977be892b0cd6f225ea (diff) |
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.
-rw-r--r-- | internal/gitaly/linguist/linguist.go | 16 | ||||
-rw-r--r-- | internal/gitaly/rubyserver/rubyserver.go | 25 | ||||
-rw-r--r-- | internal/helper/env/ruby.go | 22 | ||||
-rw-r--r-- | internal/helper/env/ruby_test.go | 97 |
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)) + }) + } +} |