fix: route untrusted workflow inputs through env to prevent shell injection

e2e/harness/multi_repo_scenario.go

@@ -495,6 +495,25 @@ func (r *MultiRepoRunner) assertState(ctx context.Context, repoName string, expe
 						envName, deployName, expectedVersion, got)
 				}
 			}
+
+			// Artifacts are a string->string map. Asserting them end-to-end proves
+			// the dispatched --artifacts JSON survived the receiver's run: shell
+			// verbatim, which is the contract that breaks if the value is
+			// interpolated into the script text instead of routed through env:.
+			if expectedArtifacts, ok := deployMap["artifacts"].(map[string]interface{}); ok {
+				actualArtifacts := mapAt(actual, "artifacts")
+				for k, v := range expectedArtifacts {
+					want, ok := v.(string)
+					if !ok {
+						continue
+					}
+					want = r.interpolate(want)
+					if got := stringAt(actualArtifacts, k); got != want {
+						return fmt.Errorf("%s.external.%s.artifacts.%s: expected %q, got %q",
+							envName, deployName, k, want, got)
+					}
+				}
+			}
 		}
 	}
 

e2e/scenarios/multi-repo/external-state-promotion.yaml

@@ -86,6 +86,13 @@ steps:
         environment: dev
         sha: "${infra.head_sha}"
         version: "v1.1.0-rc.0"
+        # Shell metacharacters in the artifacts JSON: a single quote, a $(...)
+        # command substitution, and a backtick. If the receiver interpolated this
+        # input into its run: script text instead of routing it through env:, the
+        # single quote would break out of the argument and $(...)/`...` would
+        # execute. Routing through env: lets the raw JSON reach the verb intact,
+        # so the recorded artifact value must equal the dispatched value exactly.
+        artifacts: "{\"image_tag\":\"cdk-it's-$(whoami)-`id`\"}"
 
   # Step 3: Update K8s in satellite
   - name: update-k8s
@@ -118,5 +125,10 @@ expect:
           external:
             cdk:
               version: "v1.1.0-rc.0"
+              # The artifacts payload dispatched in step 2 must round-trip through
+              # the receiver's env-routed run: body byte-for-byte, including the
+              # single quote, $(...), and backtick.
+              artifacts:
+                image_tag: "cdk-it's-$(whoami)-`id`"
             k8s:
               version: "v1.1.0-rc.1"

internal/generate/external.go

@@ -138,18 +138,32 @@ func (g *ExternalUpdateGenerator) writeJob(sb *strings.Builder) {
 	writeGitConfigSteps(sb, g.config, "          ")
 	sb.WriteString("\n")
 
-	// Run external update
+	// Run external update.
+	//
+	// Every workflow_dispatch input is untrusted: GitHub expands ${{ ... }} into
+	// the run: script text before the shell parses it, so a value carrying a single
+	// quote, backtick, or $(...) would break out of its argument and execute as
+	// shell. Bind each input to a step-level env: variable and reference it as a
+	// quoted shell variable so the value reaches the verb as a single inert
+	// argument regardless of its contents.
 	sb.WriteString("      - name: Update External State\n")
+	sb.WriteString("        env:\n")
+	sb.WriteString("          SOURCE_REPO: ${{ inputs.source_repo }}\n")
+	sb.WriteString("          DEPLOY_NAME: ${{ inputs.deploy_name }}\n")
+	sb.WriteString("          ENVIRONMENT: ${{ inputs.environment }}\n")
+	sb.WriteString("          SHA: ${{ inputs.sha }}\n")
+	sb.WriteString("          VERSION: ${{ inputs.version }}\n")
+	sb.WriteString("          ARTIFACTS: ${{ inputs.artifacts }}\n")
 	sb.WriteString("        run: |\n")
 	sb.WriteString("          cascade external update \\\n")
 	fmt.Fprintf(sb, "            --config %s \\\n", g.getManifestFilePath())
 	fmt.Fprintf(sb, "            --manifest-key %s \\\n", g.getManifestKey())
-	sb.WriteString("            --source-repo \"${{ inputs.source_repo }}\" \\\n")
-	sb.WriteString("            --deploy-name \"${{ inputs.deploy_name }}\" \\\n")
-	sb.WriteString("            --environment \"${{ inputs.environment }}\" \\\n")
-	sb.WriteString("            --sha \"${{ inputs.sha }}\" \\\n")
-	sb.WriteString("            --version \"${{ inputs.version }}\" \\\n")
-	sb.WriteString("            --artifacts '${{ inputs.artifacts }}'\n")
+	sb.WriteString("            --source-repo \"$SOURCE_REPO\" \\\n")
+	sb.WriteString("            --deploy-name \"$DEPLOY_NAME\" \\\n")
+	sb.WriteString("            --environment \"$ENVIRONMENT\" \\\n")
+	sb.WriteString("            --sha \"$SHA\" \\\n")
+	sb.WriteString("            --version \"$VERSION\" \\\n")
+	sb.WriteString("            --artifacts \"$ARTIFACTS\"\n")
 }
 
 // writeConcurrency emits a top-level concurrency: block on the external-update

internal/generate/external_test.go

@@ -568,6 +568,67 @@ func TestExternalUpdateGenerator_HasConcurrencyBlock(t *testing.T) {
 	assert.Contains(t, content, "cancel-in-progress: false", "external-update default must queue, not cancel")
 }
 
+// TestExternalUpdateGenerator_InputsAreNotInterpolatedIntoRun asserts that the
+// generated "Update External State" step never substitutes workflow_dispatch
+// inputs directly into the shell script text. GitHub Actions expands ${{ ... }}
+// into the run: body before the shell parses it, so a value carrying a single
+// quote, backtick, or $(...) would break out of its argument and execute as
+// shell. Every untrusted input must instead be bound to a step-level env: var
+// and referenced as a quoted shell variable.
+func TestExternalUpdateGenerator_InputsAreNotInterpolatedIntoRun(t *testing.T) {
+	cfg := &config.TrunkConfig{
+		TrunkBranch:  "master",
+		Environments: []string{"dev", "test", "prod"},
+		External: []config.ExternalRepoConfig{
+			{
+				Repo: "example/cdk-infra",
+				Ref:  "main",
+				Deploys: []config.ExternalDeployConfig{
+					{Name: "cdk", Workflow: "example/cdk-infra/.github/workflows/deploy.yaml"},
+				},
+			},
+		},
+	}
+
+	gen := NewExternalUpdateGenerator(cfg, "/tmp")
+	content, err := gen.Generate()
+	require.NoError(t, err)
+
+	runBody := stepRunBody(t, content, "Update External State")
+
+	untrusted := []string{
+		"${{ inputs.source_repo }}",
+		"${{ inputs.deploy_name }}",
+		"${{ inputs.environment }}",
+		"${{ inputs.sha }}",
+		"${{ inputs.version }}",
+		"${{ inputs.artifacts }}",
+	}
+	for _, expr := range untrusted {
+		assert.NotContainsf(t, runBody, expr,
+			"untrusted input %q must not be interpolated into the run: body; bind it to env: and reference the shell variable", expr)
+	}
+	// No ${{ inputs.* }} expansion of any kind should remain in the run body.
+	assert.NotContains(t, runBody, "${{ inputs.",
+		"no workflow_dispatch input may be interpolated into the run: shell body")
+
+	// The values must instead flow through a step-level env: mapping...
+	assert.Contains(t, content, "SOURCE_REPO: ${{ inputs.source_repo }}")
+	assert.Contains(t, content, "DEPLOY_NAME: ${{ inputs.deploy_name }}")
+	assert.Contains(t, content, "ENVIRONMENT: ${{ inputs.environment }}")
+	assert.Contains(t, content, "SHA: ${{ inputs.sha }}")
+	assert.Contains(t, content, "VERSION: ${{ inputs.version }}")
+	assert.Contains(t, content, "ARTIFACTS: ${{ inputs.artifacts }}")
+
+	// ...and be consumed as quoted shell variables in the verb invocation.
+	assert.Contains(t, runBody, "--source-repo \"$SOURCE_REPO\"")
+	assert.Contains(t, runBody, "--deploy-name \"$DEPLOY_NAME\"")
+	assert.Contains(t, runBody, "--environment \"$ENVIRONMENT\"")
+	assert.Contains(t, runBody, "--sha \"$SHA\"")
+	assert.Contains(t, runBody, "--version \"$VERSION\"")
+	assert.Contains(t, runBody, "--artifacts \"$ARTIFACTS\"")
+}
+
 // TestExternalUpdateGenerator_ConcurrencyOverride asserts that a manifest-level
 // concurrency config is forwarded to the generated external-update workflow.
 func TestExternalUpdateGenerator_ConcurrencyOverride(t *testing.T) {

internal/generate/pr_preview.go

@@ -141,11 +141,17 @@ func (g *PRPreviewGenerator) writeValidateStep(sb *strings.Builder) {
 // read-only and only reports which builds/deploys this merge would trigger.
 func (g *PRPreviewGenerator) writeDetectStep(sb *strings.Builder) {
 	sb.WriteString("      - name: Detect changes\n")
+	// pull_request event fields are attacker-influenceable on a fork PR. GitHub
+	// expands ${{ ... }} into the run: script before the shell parses it, so route
+	// the SHAs through env: and reference them as quoted shell variables.
+	sb.WriteString("        env:\n")
+	sb.WriteString("          BASE_SHA: ${{ github.event.pull_request.base.sha }}\n")
+	sb.WriteString("          HEAD_SHA: ${{ github.event.pull_request.head.sha }}\n")
 	sb.WriteString("        run: |\n")
 	sb.WriteString("          cascade detect-changes \\\n")
 	fmt.Fprintf(sb, "            --config %s \\\n", g.config.GetManifestFile())
-	sb.WriteString("            --base-sha \"${{ github.event.pull_request.base.sha }}\" \\\n")
-	sb.WriteString("            --head-sha \"${{ github.event.pull_request.head.sha }}\" \\\n")
+	sb.WriteString("            --base-sha \"$BASE_SHA\" \\\n")
+	sb.WriteString("            --head-sha \"$HEAD_SHA\" \\\n")
 	sb.WriteString("            > cascade-changes.json\n")
 	sb.WriteString("\n")
 }
@@ -156,12 +162,16 @@ func (g *PRPreviewGenerator) writeDetectStep(sb *strings.Builder) {
 // be cut, and which builds/deploys this merge would run.
 func (g *PRPreviewGenerator) writeDeployDryRunStep(sb *strings.Builder) {
 	sb.WriteString("      - name: Compute plan (dry-run)\n")
+	// pull_request.head.sha is attacker-influenceable on a fork PR; route it
+	// through env: rather than interpolating it into the run: script.
+	sb.WriteString("        env:\n")
+	sb.WriteString("          HEAD_SHA: ${{ github.event.pull_request.head.sha }}\n")
 	sb.WriteString("        run: |\n")
 	sb.WriteString("          # READ-ONLY: --dry-run is enforced and never sourced from an input,\n")
 	sb.WriteString("          # so no release is cut, no state is written, and no deploy is triggered.\n")
 	sb.WriteString("          cascade --dry-run orchestrate setup \\\n")
 	fmt.Fprintf(sb, "            --config %s \\\n", g.config.GetManifestFile())
-	sb.WriteString("            --sha \"${{ github.event.pull_request.head.sha }}\" \\\n")
+	sb.WriteString("            --sha \"$HEAD_SHA\" \\\n")
 	sb.WriteString("            > cascade-plan.json\n")
 	sb.WriteString("\n")
 }

internal/generate/pr_preview_test.go

@@ -23,6 +23,32 @@ func prPreviewConfig(comment bool) *config.TrunkConfig {
 	}
 }
 
+// TestPRPreviewGenerator_PRFieldsAreNotInterpolatedIntoRun asserts that the
+// pull_request event fields the preview workflow consumes (head/base SHAs) are
+// not substituted directly into a run: shell body. Although these are SHAs, the
+// values originate from the pull_request event, which is attacker-influenceable
+// on a fork PR, so they are routed through env: like every other untrusted input.
+func TestPRPreviewGenerator_PRFieldsAreNotInterpolatedIntoRun(t *testing.T) {
+	gen := NewPRPreviewGenerator(prPreviewConfig(false), "")
+	content, err := gen.Generate()
+	require.NoError(t, err)
+
+	detectBody := stepRunBody(t, content, "Detect changes")
+	assert.NotContains(t, detectBody, "${{ github.event.",
+		"detect-changes run: body must not interpolate pull_request event fields")
+	assert.Contains(t, detectBody, "--base-sha \"$BASE_SHA\"")
+	assert.Contains(t, detectBody, "--head-sha \"$HEAD_SHA\"")
+
+	planBody := stepRunBody(t, content, "Compute plan (dry-run)")
+	assert.NotContains(t, planBody, "${{ github.event.",
+		"plan dry-run run: body must not interpolate pull_request event fields")
+	assert.Contains(t, planBody, "--sha \"$HEAD_SHA\"")
+
+	// Values bound via step env:.
+	assert.Contains(t, content, "BASE_SHA: ${{ github.event.pull_request.base.sha }}")
+	assert.Contains(t, content, "HEAD_SHA: ${{ github.event.pull_request.head.sha }}")
+}
+
 func TestPRPreviewGenerator_Disabled(t *testing.T) {
 	// nil pr_preview
 	gen := NewPRPreviewGenerator(&config.TrunkConfig{TrunkBranch: "main"}, "")
@@ -59,9 +85,10 @@ func TestPRPreviewGenerator_Enabled(t *testing.T) {
 	assert.Contains(t, content, "cascade detect-changes")
 	assert.Contains(t, content, "cascade --dry-run orchestrate setup")
 
-	// Change detection runs against the PR diff (base..head).
-	assert.Contains(t, content, "--base-sha \"${{ github.event.pull_request.base.sha }}\"")
-	assert.Contains(t, content, "--head-sha \"${{ github.event.pull_request.head.sha }}\"")
+	// Change detection runs against the PR diff (base..head). The SHAs are bound
+	// to env: and passed as quoted shell variables, never interpolated into run:.
+	assert.Contains(t, content, "--base-sha \"$BASE_SHA\"")
+	assert.Contains(t, content, "--head-sha \"$HEAD_SHA\"")
 
 	// Plan written to the step summary.
 	assert.Contains(t, content, "$GITHUB_STEP_SUMMARY")

internal/generate/promote.go

@@ -713,9 +713,15 @@ func (g *PromoteGenerator) writePromoteJob(sb *strings.Builder) {
 	fmt.Fprintf(sb, "          token: %s\n", g.getReleaseTokenRef())
 	fmt.Fprintf(sb, "          version: %s\n", g.config.GetCLIVersion())
 	sb.WriteString("      - name: Validate Promotion\n")
+	// The mode input is untrusted workflow_dispatch data. GitHub expands ${{ ... }}
+	// into the run: script before the shell runs it, so a mode value carrying shell
+	// metacharacters would break out of the echo. Bind it to env: and print the
+	// quoted shell variable instead.
+	sb.WriteString("        env:\n")
+	sb.WriteString("          MODE: ${{ github.event.inputs.mode }}\n")
 	sb.WriteString("        run: |\n")
 	sb.WriteString("          echo \"Promotion validated by preflight job\"\n")
-	sb.WriteString("          echo \"Mode: ${{ github.event.inputs.mode }}\"\n")
+	sb.WriteString("          echo \"Mode: $MODE\"\n")
 	sb.WriteString("          echo \"Source: ${{ needs.preflight.outputs.source_env }}\"\n")
 	sb.WriteString("          echo \"Final Env: ${{ needs.preflight.outputs.target_env }}\"\n")
 	sb.WriteString("          echo \"::notice::Promotion validation completed successfully\"\n\n")

internal/generate/promote_test.go

@@ -29,6 +29,78 @@ func concurrencyGroupLine(t *testing.T, content string) string {
 	return ""
 }
 
+// stepRunBody extracts the run: script body of the step whose "- name: <name>"
+// header matches stepName, from a generated workflow. It returns only the shell
+// lines under that step's "run: |" block, stopping at the next step or key at the
+// same or shallower indentation. This lets injection tests assert on what the
+// shell actually sees, without false matches from a sibling env: mapping (which
+// is the safe place for ${{ ... }} expansions) elsewhere in the same step.
+func stepRunBody(t *testing.T, content, stepName string) string {
+	t.Helper()
+	lines := strings.Split(content, "\n")
+
+	stepIdx := -1
+	for i, line := range lines {
+		if strings.Contains(line, "- name: "+stepName) {
+			stepIdx = i
+			break
+		}
+	}
+	require.GreaterOrEqual(t, stepIdx, 0, "step %q not found in workflow", stepName)
+
+	runIdx := -1
+	for i := stepIdx + 1; i < len(lines); i++ {
+		trimmed := strings.TrimSpace(lines[i])
+		// Stop if we hit the next step before finding this step's run: block.
+		if strings.HasPrefix(trimmed, "- name: ") {
+			break
+		}
+		if trimmed == "run: |" || trimmed == "run: |-" {
+			runIdx = i
+			break
+		}
+	}
+	require.GreaterOrEqual(t, runIdx, 0, "step %q has no block run: body", stepName)
+
+	runIndent := len(lines[runIdx]) - len(strings.TrimLeft(lines[runIdx], " "))
+	var body []string
+	for i := runIdx + 1; i < len(lines); i++ {
+		line := lines[i]
+		if strings.TrimSpace(line) == "" {
+			body = append(body, line)
+			continue
+		}
+		indent := len(line) - len(strings.TrimLeft(line, " "))
+		if indent <= runIndent {
+			break
+		}
+		body = append(body, line)
+	}
+	return strings.Join(body, "\n")
+}
+
+// TestPromoteGenerator_ModeInputNotInterpolatedIntoRun asserts that the
+// workflow_dispatch "mode" input is not echoed into a run: shell body via
+// ${{ github.event.inputs.mode }}. A mode value containing shell metacharacters
+// would otherwise break out of the echo. It must be bound to env: and printed as
+// a quoted shell variable.
+func TestPromoteGenerator_ModeInputNotInterpolatedIntoRun(t *testing.T) {
+	cfg := &config.TrunkConfig{
+		TrunkBranch:  "main",
+		Environments: []string{"dev", "prod"},
+	}
+
+	gen := NewPromoteGenerator(cfg, "")
+	content, err := gen.Generate()
+	require.NoError(t, err)
+
+	body := stepRunBody(t, content, "Validate Promotion")
+	assert.NotContains(t, body, "${{ github.event.inputs.mode }}",
+		"the mode input must not be interpolated into the Validate Promotion run: body")
+	assert.Contains(t, content, "MODE: ${{ github.event.inputs.mode }}")
+	assert.Contains(t, body, "echo \"Mode: $MODE\"")
+}
+
 func TestPromoteGenerator_Generate(t *testing.T) {
 	tests := []struct {
 		name         string