feat/batch-changes: harden codingAgent v1 port

Three real follow-ups on top of the initial v1 port, all surfaced while
porting feedback from sourcegraph/sourcegraph#12503:

- Stop emitting SRC_BATCHES_MODEL_PROVIDER_TOKEN in src-cli logs.
  The forwarded token is now redacted in StepStarted UI metadata
  (JSON-lines) and in the 'full command' debug log, but still passed
  verbatim to the docker -e flags so the agent CLI inside the
  container can use it. Server-side RedactedValues stays as a backstop.
- Reject v3 codingAgent steps that omit image: at parse time, instead
  of failing later in the executor with an empty-image error.
- Harden the codex install script: extract into a mktemp dir and mv
  into the install path so a failed/retried install can't leave a
  half-written binary behind, and assert that the installed binary's
  --version contains pinnedVersion (catches a stale binary surviving
  from a prior failed install).

Also adds tests for the new redaction helpers, Step.MarshalJSON
canonicalization, and the new codingAgent-requires-image validation.

Author: taras-yemets

internal/batches/executor/run_steps.go

@@ -338,7 +338,7 @@ func executeSingleStep(
 	// ----------
 	// EXECUTION
 	// ----------
-	opts.UI.StepStarted(stepIdx+1, runScript, env)
+	opts.UI.StepStarted(stepIdx+1, runScript, redactSensitiveEnv(env))
 
 	workspaceOpts, err := workspace.DockerRunOpts(ctx, workDir)
 	if err != nil {
@@ -424,7 +424,7 @@ func executeSingleStep(
 	}
 
 	opts.Logger.Logf("[Step %d] run: %q, container: %q", stepIdx+1, step.Run, step.Container)
-	opts.Logger.Logf("[Step %d] full command: %q", stepIdx+1, strings.Join(cmd.Args, " "))
+	opts.Logger.Logf("[Step %d] full command: %q", stepIdx+1, strings.Join(redactSensitiveArgs(cmd.Args), " "))
 
 	// Start the command.
 	t0 := time.Now()
@@ -619,6 +619,46 @@ func forwardCodingAgentEnv(globalEnv []string, stepEnv map[string]string) {
 	}
 }
 
+// sensitiveEnvKeys names env vars that get passed verbatim into the user
+// container but must be scrubbed from UI sinks and log lines.
+var sensitiveEnvKeys = map[string]struct{}{
+	codingagenttypes.ModelProviderTokenEnvVar: {},
+}
+
+const redactedPlaceholder = "REDACTED"
+
+func redactSensitiveEnv(env map[string]string) map[string]string {
+	out := make(map[string]string, len(env))
+	for k, v := range env {
+		if _, sensitive := sensitiveEnvKeys[k]; sensitive && v != "" {
+			out[k] = redactedPlaceholder
+		} else {
+			out[k] = v
+		}
+	}
+	return out
+}
+
+// redactSensitiveArgs scrubs the value side of `-e KEY=VALUE` pairs whose
+// KEY is sensitive, returning a copy of args suitable for logging.
+func redactSensitiveArgs(args []string) []string {
+	out := make([]string, len(args))
+	copy(out, args)
+	for i := 0; i+1 < len(out); i++ {
+		if out[i] != "-e" {
+			continue
+		}
+		key, _, ok := strings.Cut(out[i+1], "=")
+		if !ok {
+			continue
+		}
+		if _, sensitive := sensitiveEnvKeys[key]; sensitive {
+			out[i+1] = key + "=" + redactedPlaceholder
+		}
+	}
+	return out
+}
+
 // writeRunScriptFile writes a pre-rendered run script (e.g. a codingAgent
 // step desugared by the codingagent registry) verbatim to a temp file, with
 // the same permission semantics as createRunScriptFile. Unlike that helper,

internal/batches/executor/run_steps_test.go

@@ -1,11 +1,57 @@
 package executor
 
 import (
+	"slices"
+	"strings"
 	"testing"
 
 	codingagenttypes "github.com/sourcegraph/sourcegraph/lib/batches/codingagent/types"
 )
 
+func TestRedactSensitiveEnv(t *testing.T) {
+	in := map[string]string{
+		codingagenttypes.ModelProviderTokenEnvVar: "tok-abc",
+		codingagenttypes.JobIDEnvVar:              "job-123",
+		"PATH":                                    "/bin",
+	}
+	out := redactSensitiveEnv(in)
+	if got := out[codingagenttypes.ModelProviderTokenEnvVar]; got != redactedPlaceholder {
+		t.Errorf("token: got %q want %q", got, redactedPlaceholder)
+	}
+	if got := out[codingagenttypes.JobIDEnvVar]; got != "job-123" {
+		t.Errorf("job id should not be redacted: got %q", got)
+	}
+	if got := out["PATH"]; got != "/bin" {
+		t.Errorf("PATH should not be redacted: got %q", got)
+	}
+	if in[codingagenttypes.ModelProviderTokenEnvVar] != "tok-abc" {
+		t.Errorf("input must not be mutated")
+	}
+}
+
+func TestRedactSensitiveArgs(t *testing.T) {
+	in := []string{
+		"docker", "run",
+		"-e", codingagenttypes.ModelProviderTokenEnvVar + "=tok-abc",
+		"-e", codingagenttypes.JobIDEnvVar + "=job-123",
+		"-e", "PATH=/bin",
+		"--", "image:tag", "/script",
+	}
+	out := redactSensitiveArgs(in)
+	if slices.Contains(out, codingagenttypes.ModelProviderTokenEnvVar+"=tok-abc") {
+		t.Errorf("token value still present in args: %v", out)
+	}
+	if !slices.Contains(out, codingagenttypes.ModelProviderTokenEnvVar+"="+redactedPlaceholder) {
+		t.Errorf("token not redacted in args: %v", out)
+	}
+	if !slices.Contains(out, codingagenttypes.JobIDEnvVar+"=job-123") {
+		t.Errorf("job id should pass through: %v", out)
+	}
+	if strings.Contains(strings.Join(out, " "), "tok-abc") {
+		t.Errorf("token leaked anywhere in joined args: %v", out)
+	}
+}
+
 // TestForwardCodingAgentEnv verifies that the model-provider auth env vars
 // placed on the v1 CliStep by the Sourcegraph server are forwarded into the
 // user container env for codingAgent steps.

lib/batches/batch_spec.go

@@ -226,6 +226,10 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) {
 		if step.CodingAgent != nil && step.Run != "" {
 			errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent and run cannot be combined in the same step", i+1)))
 		}
+		if step.CodingAgent != nil && step.Container == "" {
+			// v3 image: is mirrored into Container above.
+			errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent step requires an image", i+1)))
+		}
 	}
 
 	return &spec, errs

lib/batches/batch_spec_test.go

@@ -1,9 +1,37 @@
 package batches
 
 import (
+	"encoding/json"
+	"strings"
 	"testing"
 )
 
+// Step JSON must always emit `container` (never `image`) so prep-side cache
+// keys computed by marshaling Step match src-cli's serialization.
+func TestStep_MarshalJSON_canonicalizesImageToContainer(t *testing.T) {
+	v3FromImage, err := json.Marshal(Step{Image: "alpine:3", Run: "echo hi"})
+	if err != nil {
+		t.Fatalf("marshal v3-shaped step: %v", err)
+	}
+	v1FromContainer, err := json.Marshal(Step{Container: "alpine:3", Run: "echo hi"})
+	if err != nil {
+		t.Fatalf("marshal v1-shaped step: %v", err)
+	}
+	if string(v3FromImage) != string(v1FromContainer) {
+		t.Errorf("canonical JSON differs:\n  v3 image:     %s\n  v1 container: %s", v3FromImage, v1FromContainer)
+	}
+	var out map[string]any
+	if err := json.Unmarshal(v3FromImage, &out); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if _, ok := out["image"]; ok {
+		t.Errorf("expected no image key in canonical JSON, got %s", v3FromImage)
+	}
+	if got, _ := out["container"].(string); got != "alpine:3" {
+		t.Errorf("container: got %v want alpine:3 (full=%s)", out["container"], v3FromImage)
+	}
+}
+
 // TestParseBatchSpec_v3_imageMirroredToContainer ensures that in v3 specs the
 // step-level `image:` field is mirrored into Step.Container so executor
 // consumers that read step.Container keep working.
@@ -39,6 +67,36 @@ changesetTemplate:
 	}
 }
 
+// A v3 codingAgent step without image: must be rejected at parse time so
+// it doesn't fail later with an opaque empty-image error in the executor.
+func TestParseBatchSpec_v3_codingAgentRequiresImage(t *testing.T) {
+	spec := []byte(`
+version: 3
+name: test
+description: test
+on:
+  - repository: github.com/sourcegraph/sourcegraph
+steps:
+  - codingAgent:
+      type: codex
+      prompt: do the thing
+changesetTemplate:
+  title: test
+  body: test
+  branch: test
+  commit:
+    message: test
+`)
+	_, err := ParseBatchSpec(spec)
+	if err == nil {
+		t.Fatal("expected validation error, got nil")
+	}
+	if !strings.Contains(err.Error(), "requires an image") &&
+		!strings.Contains(err.Error(), "Must validate") {
+		t.Errorf("error should mention missing image, got: %v", err)
+	}
+}
+
 // TestParseBatchSpec_v3_codingAgentStep ensures that a v3 spec with a
 // codingAgent step parses with the expected typed step.
 func TestParseBatchSpec_v3_codingAgentStep(t *testing.T) {

lib/batches/codex/codex.go

@@ -26,21 +26,31 @@ func (Agent) Type() batcheslib.CodingAgentType                { return batchesli
 func (Agent) ModelProviderRoutes() []types.ModelProviderRoute { return routes }
 func (Agent) ImageRequirements() []string                     { return []string{"curl", "tar"} }
 
-// InstallScript installs codex from GitHub Releases into types.InstallDir.
+// InstallScript installs codex at pinnedVersion into types.InstallDir.
 func (Agent) InstallScript() string {
 	return fmt.Sprintf(`_install_dir=%s
 _version=%s
-_codex_arch=$(uname -m)
-case "$_codex_arch" in
-  x86_64)  _codex_triple=x86_64-unknown-linux-musl ;;
-  aarch64) _codex_triple=aarch64-unknown-linux-musl ;;
-  *) echo "codingAgent codex: unsupported architecture: $_codex_arch" >&2; exit 1 ;;
+
+case "$(uname -m)" in
+  x86_64)  _triple=x86_64-unknown-linux-musl ;;
+  aarch64) _triple=aarch64-unknown-linux-musl ;;
+  *) echo "codingAgent codex: unsupported architecture $(uname -m)" >&2; exit 1 ;;
 esac
+
+# Stage in a temp dir and mv into place so a failed retry can't leave a half-written binary behind.
+_url="https://github.com/openai/codex/releases/download/rust-v${_version}/codex-${_triple}.tar.gz"
+_tmp=$(mktemp -d "${TMPDIR:-/tmp}/sg-codex.XXXXXX")
 mkdir -p "$_install_dir"
-curl -fsSL "https://github.com/openai/codex/releases/download/rust-v${_version}/codex-${_codex_triple}.tar.gz" | tar -xz -C "$_install_dir"
-mv "$_install_dir/codex-${_codex_triple}" "$_install_dir/codex"
-chmod +x "$_install_dir/codex"
-"$_install_dir/codex" --version >/dev/null || { echo "codingAgent codex: install verification failed (cannot exec $_install_dir/codex)" >&2; exit 1; }
+curl -fsSL "$_url" | tar -xz -C "$_tmp" || { echo "codingAgent codex: download/extract failed: $_url" >&2; exit 1; }
+chmod +x "$_tmp/codex-${_triple}"
+mv -f "$_tmp/codex-${_triple}" "$_install_dir/codex"
+rm -rf "$_tmp"
+
+_actual=$("$_install_dir/codex" --version 2>&1) || { echo "codingAgent codex: cannot exec $_install_dir/codex: $_actual" >&2; exit 1; }
+case "$_actual" in
+  *"$_version"*) ;;
+  *) echo "codingAgent codex: version mismatch: want $_version, got: $_actual" >&2; exit 1 ;;
+esac
 `, types.InstallDir, pinnedVersion)
 }