diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go index 6b6a61788e..4dfeb14e63 100644 --- a/internal/batches/executor/run_steps.go +++ b/internal/batches/executor/run_steps.go @@ -316,8 +316,9 @@ func executeSingleStep( } defer cleanup() - // Resolve step.Env given the current environment. - stepEnv, err := step.Env.Resolve(opts.GlobalEnv) + // Resolve step.Env given the current environment. Executor control values + // must never be selectable by an author-controlled step. + stepEnv, err := step.Env.Resolve(withoutReservedExecutorEnv(opts.GlobalEnv)) if err != nil { err = errors.Wrap(err, "resolving step environment") opts.UI.StepPreparingFailed(stepIdx+1, err) @@ -464,6 +465,17 @@ func executeSingleStep( return stdout, stderr, nil } +func withoutReservedExecutorEnv(env []string) []string { + filtered := make([]string, 0, len(env)) + for _, variable := range env { + name, _, found := strings.Cut(variable, "=") + if !found || !strings.HasPrefix(name, "SRC_EXECUTOR_") { + filtered = append(filtered, variable) + } + } + return filtered +} + func setOutputs(stepOutputs batcheslib.Outputs, global map[string]any, stepCtx *template.StepContext) error { for name, output := range stepOutputs { var value bytes.Buffer diff --git a/internal/batches/executor/run_steps_test.go b/internal/batches/executor/run_steps_test.go index 6b072d55c9..ccdab14c9d 100644 --- a/internal/batches/executor/run_steps_test.go +++ b/internal/batches/executor/run_steps_test.go @@ -2,6 +2,7 @@ package executor import ( "context" + "encoding/json" "os" "path/filepath" "runtime" @@ -10,9 +11,40 @@ import ( "github.com/stretchr/testify/require" batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" + batchenv "github.com/sourcegraph/sourcegraph/lib/batches/env" "github.com/sourcegraph/sourcegraph/lib/batches/template" ) +func TestWithoutReservedExecutorEnv(t *testing.T) { + env := []string{ + "ALLOWED=value", + "SRC_EXECUTOR_JOB_TOKEN=secret", + "SRC_EXECUTOR_FUTURE_SECRET=secret", + "VALUE=contains-SRC_EXECUTOR_JOB_TOKEN", + "MALFORMED", + } + + require.Equal(t, []string{ + "ALLOWED=value", + "VALUE=contains-SRC_EXECUTOR_JOB_TOKEN", + "MALFORMED", + }, withoutReservedExecutorEnv(env)) + + var stepEnv batchenv.Environment + require.NoError(t, json.Unmarshal([]byte(`[ + "ALLOWED", + "SRC_EXECUTOR_JOB_TOKEN", + "SRC_EXECUTOR_FUTURE_SECRET" + ]`), &stepEnv)) + resolved, err := stepEnv.Resolve(withoutReservedExecutorEnv(env[:4])) + require.NoError(t, err) + require.Equal(t, map[string]string{ + "ALLOWED": "value", + "SRC_EXECUTOR_JOB_TOKEN": "", + "SRC_EXECUTOR_FUTURE_SECRET": "", + }, resolved) +} + func TestParseContainerTempPath(t *testing.T) { for _, valid := range []string{"/tmp/tmp.abc-123_456", "/tmp/tmp.abc-123_456\n"} { t.Run("valid_"+valid, func(t *testing.T) {