fix(runner): keep env values out of the command line

executor_buildkit.go

@@ -135,7 +135,7 @@ func (r *buildkitJobRun) build(ctx context.Context, step proto.StepSpec, log fun
 	name, args := r.buildctlCmd(buildArgs)
 
 	log("building " + ref + " (rootless buildkit, context " + contextLabel(r.workdir, cdir) + ")")
-	if code, err := r.e.cmd.run(ctx, r.workdir, log, name, args...); err != nil {
+	if code, err := r.e.cmd.run(ctx, r.workdir, nil, log, name, args...); err != nil {
 		return StepResult{}, fmt.Errorf("buildctl: %w", err)
 	} else if code != 0 {
 		return StepResult{Exit: code}, nil

executor_common.go

@@ -171,10 +171,10 @@ func packArgs(tag, builder string, cfg *proto.BuildConfig) []string {
 // commander runs external commands; abstracted so the executors are unit-testable
 // without a real docker/buildkit/git.
 type commander interface {
-	// run executes name+args in dir, streaming combined output to log line by
-	// line, and returns the process exit code. A non-zero exit is (code, nil); a
-	// failure to start is (-1, err).
-	run(ctx context.Context, dir string, log func(string), name string, args ...string) (int, error)
+	// run executes name+args in dir with env added to the child's environment,
+	// streaming combined output to log line by line, and returns the exit code. A
+	// non-zero exit is (code, nil); a failure to start is (-1, err).
+	run(ctx context.Context, dir string, env []string, log func(string), name string, args ...string) (int, error)
 	// capture runs a command and returns its trimmed stdout.
 	capture(ctx context.Context, dir, name string, args ...string) (string, error)
 	// loginStdin runs a command with secret piped to its stdin (registry login).
@@ -185,11 +185,11 @@ type commander interface {
 // source URL is never logged (it may embed a credential).
 func gitCheckout(ctx context.Context, cmd commander, gitBin, workdir string, job proto.JobSpec, log func(string)) error {
 	log("cloning source")
-	if code, err := cmd.run(ctx, "", log, gitBin, "clone", job.SourceURL, workdir); err != nil || code != 0 {
+	if code, err := cmd.run(ctx, "", nil, log, gitBin, "clone", job.SourceURL, workdir); err != nil || code != 0 {
 		return fmt.Errorf("git clone failed (exit %d): %w", code, err)
 	}
 	if job.Commit != "" {
-		if code, err := cmd.run(ctx, workdir, log, gitBin, "checkout", "--detach", job.Commit); err != nil || code != 0 {
+		if code, err := cmd.run(ctx, workdir, nil, log, gitBin, "checkout", "--detach", job.Commit); err != nil || code != 0 {
 			return fmt.Errorf("git checkout %s failed (exit %d): %w", job.Commit, code, err)
 		}
 	}
@@ -225,9 +225,12 @@ func envMap(env []string) map[string]string {
 // execCommander is the real commander over os/exec.
 type execCommander struct{}
 
-func (execCommander) run(ctx context.Context, dir string, log func(string), name string, args ...string) (int, error) {
+func (execCommander) run(ctx context.Context, dir string, env []string, log func(string), name string, args ...string) (int, error) {
 	cmd := exec.CommandContext(ctx, name, args...)
 	cmd.Dir = dir
+	if len(env) > 0 {
+		cmd.Env = append(os.Environ(), env...)
+	}
 	w := &lineWriter{emit: log}
 	cmd.Stdout, cmd.Stderr = w, w
 	err := cmd.Run()

executor_docker.go

@@ -208,7 +208,7 @@ func (r *dockerJobRun) build(ctx context.Context, step proto.StepSpec, log func(
 		}
 		log(fmt.Sprintf("building %s with buildpacks (builder %s)", tag, builder))
 		name, args := r.authCmd(r.e.pack, packArgs(tag, builder, step.Build)...)
-		if code, err := r.e.cmd.run(ctx, r.workdir, log, name, args...); err != nil {
+		if code, err := r.e.cmd.run(ctx, r.workdir, nil, log, name, args...); err != nil {
 			return StepResult{}, fmt.Errorf("pack build: %w", err)
 		} else if code != 0 {
 			return StepResult{Exit: code}, nil
@@ -230,7 +230,7 @@ func (r *dockerJobRun) build(ctx context.Context, step proto.StepSpec, log func(
 		buildArgs = append(buildArgs, rel)
 		log("building " + tag + " (context " + rel + ")")
 		name, args := r.authCmd(r.e.docker, buildArgs...)
-		if code, err := r.e.cmd.run(ctx, r.workdir, log, name, args...); err != nil {
+		if code, err := r.e.cmd.run(ctx, r.workdir, nil, log, name, args...); err != nil {
 			return StepResult{}, fmt.Errorf("docker build: %w", err)
 		} else if code != 0 {
 			return StepResult{Exit: code}, nil
@@ -239,7 +239,7 @@ func (r *dockerJobRun) build(ctx context.Context, step proto.StepSpec, log func(
 
 	log("pushing " + tag)
 	name, args := r.authCmd(r.e.docker, "push", tag)
-	if code, err := r.e.cmd.run(ctx, r.workdir, log, name, args...); err != nil {
+	if code, err := r.e.cmd.run(ctx, r.workdir, nil, log, name, args...); err != nil {
 		return StepResult{}, fmt.Errorf("docker push: %w", err)
 	} else if code != 0 {
 		return StepResult{Exit: code}, nil
@@ -267,11 +267,19 @@ func (r *dockerJobRun) container(ctx context.Context, step proto.StepSpec, log f
 	}
 	args := []string{"run", "--rm", "-w", "/workspace", "-v", r.workdir + ":/workspace"}
 
+	// `-e NAME` (no value) tells docker to take it from our own environment, so a
+	// resolved secret never lands in a command line every local user can read.
 	stepEnv := append([]string{}, r.job.Env...)
 	stepEnv = append(stepEnv, r.exportedEnv()...)
 	stepEnv = append(stepEnv, step.Env...)
+	childEnv := make([]string, 0, len(stepEnv))
 	for _, e := range stepEnv {
-		args = append(args, "-e", e)
+		name, _, ok := strings.Cut(e, "=")
+		if !ok || name == "" {
+			continue
+		}
+		args = append(args, "-e", name)
+		childEnv = append(childEnv, e)
 	}
 	// Mount the shared env file and point $MIABI_ENV at it so this step can export
 	// its own vars to later steps (`echo KEY=VALUE >> $MIABI_ENV`).
@@ -288,7 +296,7 @@ func (r *dockerJobRun) container(ctx context.Context, step proto.StepSpec, log f
 	}
 
 	name, cargs := r.authCmd(r.e.docker, args...)
-	code, err := r.e.cmd.run(ctx, "", log, name, cargs...)
+	code, err := r.e.cmd.run(ctx, "", childEnv, log, name, cargs...)
 	if err != nil {
 		return StepResult{}, err
 	}

executor_docker_test.go

@@ -26,9 +26,11 @@ type fakeCommander struct {
 	digestOut string
 	loginErr  error
 	logins    int
+	env       []string // child env of the last run
 }
 
-func (f *fakeCommander) run(_ context.Context, _ string, log func(string), name string, args ...string) (int, error) {
+func (f *fakeCommander) run(_ context.Context, _ string, env []string, log func(string), name string, args ...string) (int, error) {
+	f.env = env
 	f.calls = append(f.calls, name+" "+strings.Join(args, " "))
 	log(name + " output")
 	if name == "docker" && len(args) > 0 {
@@ -44,6 +46,16 @@ func (f *fakeCommander) run(_ context.Context, _ string, log func(string), name
 	return 0, nil // git clone/checkout
 }
 
+// inEnv reports whether the last run received this KEY=VALUE in its child env.
+func (f *fakeCommander) inEnv(want string) bool {
+	for _, e := range f.env {
+		if e == want {
+			return true
+		}
+	}
+	return false
+}
+
 func (f *fakeCommander) capture(_ context.Context, _, name string, args ...string) (string, error) {
 	f.calls = append(f.calls, "capture "+name+" "+strings.Join(args, " "))
 	return f.digestOut, nil
@@ -258,8 +270,11 @@ func TestContainerStepMountsWorkspace(t *testing.T) {
 	if !fc.called("docker run --rm -w /workspace -v") || !fc.called("--entrypoint sh golang:1.25 -c go test ./...") {
 		t.Errorf("container run command wrong: %v", fc.calls)
 	}
-	if !fc.called("-e FOO=bar") || !fc.called("-e CI=true") {
-		t.Errorf("job + step env not injected: %v", fc.calls)
+	if !fc.called("-e FOO") || !fc.called("-e CI") {
+		t.Errorf("job + step env not passed through: %v", fc.calls)
+	}
+	if !fc.inEnv("FOO=bar") || !fc.inEnv("CI=true") {
+		t.Errorf("job + step env not injected: %v", fc.env)
 	}
 }
 
@@ -281,11 +296,11 @@ func TestContainerStepSeesBuiltImage(t *testing.T) {
 	}, func(string) {}); err != nil {
 		t.Fatalf("scan step: %v", err)
 	}
-	if !fc.called("-e MIABI_IMAGE=reg.example.com/ws-42/web:run-57") {
-		t.Errorf("MIABI_IMAGE not exported to the scan step: %v", fc.calls)
+	if !fc.inEnv("MIABI_IMAGE=reg.example.com/ws-42/web:run-57") {
+		t.Errorf("MIABI_IMAGE not exported to the scan step: %v", fc.env)
 	}
-	if !fc.called("-e MIABI_IMAGE_DIGEST=reg.example.com/ws-42/web@sha256:cafebabe") {
-		t.Errorf("MIABI_IMAGE_DIGEST not exported: %v", fc.calls)
+	if !fc.inEnv("MIABI_IMAGE_DIGEST=reg.example.com/ws-42/web@sha256:cafebabe") {
+		t.Errorf("MIABI_IMAGE_DIGEST not exported: %v", fc.env)
 	}
 }
 
@@ -305,8 +320,8 @@ func TestStepEnvExportPropagates(t *testing.T) {
 	}, func(string) {}); err != nil {
 		t.Fatalf("step: %v", err)
 	}
-	if !fc.called("-e VERSION=1.2.3") {
-		t.Errorf("exported var not propagated to the next step: %v", fc.calls)
+	if !fc.inEnv("VERSION=1.2.3") {
+		t.Errorf("exported var not propagated to the next step: %v", fc.env)
 	}
 	if !fc.called(":/miabi/env") || !fc.called("-e MIABI_ENV=/miabi/env") {
 		t.Errorf("$MIABI_ENV file not mounted into the step: %v", fc.calls)
@@ -463,3 +478,68 @@ func TestBuildStepBuildArgs(t *testing.T) {
 		t.Errorf("build command wrong:\n got %v\n want %q", fc.calls, want)
 	}
 }
+
+// A resolved workspace secret must not reach the command line: argv is readable
+// by every local user on the runner host via ps.
+func TestContainerStepKeepsEnvValuesOutOfArgv(t *testing.T) {
+	f := &fakeCommander{}
+	e := &dockerExecutor{cmd: f, docker: "docker", git: "git", workRoot: t.TempDir()}
+	job := proto.JobSpec{RunID: 1, Env: []string{"MIABI_REGISTRY_TOKEN=reg_s3cret"}}
+	run, err := e.Begin(context.Background(), job, func(string) {})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer run.Close()
+
+	step := proto.StepSpec{
+		Ordinal: 0, Name: "test", Image: "node:22",
+		Run: []string{"sh", "-c", "npm test"},
+		Env: []string{"NPM_TOKEN=npm_live_SECRET", "CI=true"},
+	}
+	if _, err := run.Step(context.Background(), step, func(string) {}); err != nil {
+		t.Fatal(err)
+	}
+
+	argv := strings.Join(f.calls, " ")
+	for _, secret := range []string{"npm_live_SECRET", "reg_s3cret"} {
+		if strings.Contains(argv, secret) {
+			t.Errorf("secret %q appeared in the command line: %s", secret, argv)
+		}
+	}
+	// The names are still passed, so docker forwards them from our environment.
+	for _, name := range []string{"-e NPM_TOKEN", "-e CI", "-e MIABI_REGISTRY_TOKEN"} {
+		if !strings.Contains(argv, name) {
+			t.Errorf("argv is missing %q: %s", name, argv)
+		}
+	}
+	child := strings.Join(f.env, " ")
+	for _, want := range []string{"NPM_TOKEN=npm_live_SECRET", "CI=true", "MIABI_REGISTRY_TOKEN=reg_s3cret"} {
+		if !strings.Contains(child, want) {
+			t.Errorf("child env is missing %q: %v", want, f.env)
+		}
+	}
+}
+
+// Step env wins over the job's on a collision, matching the control plane's
+// documented precedence.
+func TestContainerStepEnvPrecedence(t *testing.T) {
+	f := &fakeCommander{}
+	e := &dockerExecutor{cmd: f, docker: "docker", git: "git", workRoot: t.TempDir()}
+	job := proto.JobSpec{RunID: 1, Env: []string{"SHARED=pipeline"}}
+	run, _ := e.Begin(context.Background(), job, func(string) {})
+	defer run.Close()
+
+	step := proto.StepSpec{Ordinal: 0, Name: "s", Image: "x", Env: []string{"SHARED=step"}}
+	if _, err := run.Step(context.Background(), step, func(string) {}); err != nil {
+		t.Fatal(err)
+	}
+	var last string
+	for _, e := range f.env {
+		if strings.HasPrefix(e, "SHARED=") {
+			last = e
+		}
+	}
+	if last != "SHARED=step" {
+		t.Errorf("effective SHARED = %q, want the step's value", last)
+	}
+}

joblog_test.go

@@ -171,8 +171,8 @@ func TestJobLogsCarryNoCredentials(t *testing.T) {
 			t.Errorf("the log leaked %q\n---\n%s", leak, out)
 		}
 	}
-	// …while still saying where the code came from.
-	if !strings.Contains(out, "github.com/acme/app.git") {
-		t.Errorf("the source host was stripped along with the credential\n---\n%s", out)
+	// The source URL is not logged at all, so nothing of it may appear.
+	if strings.Contains(out, "github.com/acme/app.git") {
+		t.Errorf("the clone URL reached the log\n---\n%s", out)
 	}
 }