diff --git a/.changeset/minor-auto-hoist-run-expressions-codemod.md b/.changeset/minor-auto-hoist-run-expressions-codemod.md new file mode 100644 index 00000000000..b11f86fc7fb --- /dev/null +++ b/.changeset/minor-auto-hoist-run-expressions-codemod.md @@ -0,0 +1,5 @@ +--- +"gh-aw": minor +--- + +Extend `steps-run-secrets-to-env` codemod to hoist **all** `${{ ... }}` expressions from `run:` blocks — not just secrets, `env.*`, and `github.token`. Arbitrary expressions such as `github.repository`, `github.event.issue.title`, `inputs.*`, and `steps.*.outputs.*` now receive `EXPR_*` step-level `env:` bindings. PowerShell steps (`shell: pwsh` / `shell: powershell`) receive `$env:VARNAME` syntax. This closes the gap that previously required the separate `auto-hoist-run-expressions` codemod. diff --git a/.github/workflows/daily-model-inventory.lock.yml b/.github/workflows/daily-model-inventory.lock.yml index 582b2fc4ffe..5ada45ec8f9 100644 --- a/.github/workflows/daily-model-inventory.lock.yml +++ b/.github/workflows/daily-model-inventory.lock.yml @@ -203,21 +203,21 @@ jobs: run: | bash "${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh" { - cat << 'GH_AW_PROMPT_4beb7ac5e590862b_EOF' + cat << 'GH_AW_PROMPT_04c8251975e742fc_EOF' - GH_AW_PROMPT_4beb7ac5e590862b_EOF + GH_AW_PROMPT_04c8251975e742fc_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/xpia.md" cat "${RUNNER_TEMP}/gh-aw/prompts/temp_folder_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/markdown.md" cat "${RUNNER_TEMP}/gh-aw/prompts/playwright_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_prompt.md" - cat << 'GH_AW_PROMPT_4beb7ac5e590862b_EOF' + cat << 'GH_AW_PROMPT_04c8251975e742fc_EOF' Tools: create_issue, missing_tool, missing_data, noop - GH_AW_PROMPT_4beb7ac5e590862b_EOF + GH_AW_PROMPT_04c8251975e742fc_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/mcp_cli_tools_prompt.md" - cat << 'GH_AW_PROMPT_4beb7ac5e590862b_EOF' + cat << 'GH_AW_PROMPT_04c8251975e742fc_EOF' The following GitHub context information is available for this workflow: {{#if github.actor}} @@ -246,15 +246,15 @@ jobs: {{/if}} - GH_AW_PROMPT_4beb7ac5e590862b_EOF + GH_AW_PROMPT_04c8251975e742fc_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/github_mcp_tools_with_safeoutputs_prompt.md" - cat << 'GH_AW_PROMPT_4beb7ac5e590862b_EOF' + cat << 'GH_AW_PROMPT_04c8251975e742fc_EOF' {{#runtime-import .github/workflows/shared/otel.md}} {{#runtime-import .github/workflows/shared/observability-otlp.md}} {{#runtime-import .github/workflows/shared/noop-reminder.md}} {{#runtime-import .github/workflows/daily-model-inventory.md}} - GH_AW_PROMPT_4beb7ac5e590862b_EOF + GH_AW_PROMPT_04c8251975e742fc_EOF } > "$GH_AW_PROMPT" - name: Interpolate variables and render templates uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 @@ -498,9 +498,9 @@ jobs: mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs" mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs - cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_23b064ae0499db90_EOF' + cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_a31ecd2b69f3aaa1_EOF' {"create_issue":{"close_older_issues":true,"expires":168,"labels":["automation","models"],"max":1,"title_prefix":"[model-inventory] "},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"report_incomplete":{}} - GH_AW_SAFE_OUTPUTS_CONFIG_23b064ae0499db90_EOF + GH_AW_SAFE_OUTPUTS_CONFIG_a31ecd2b69f3aaa1_EOF - name: Generate Safe Outputs Tools env: GH_AW_TOOLS_META_JSON: | @@ -710,7 +710,7 @@ jobs: mkdir -p /home/runner/.copilot GH_AW_NODE=$(which node 2>/dev/null || command -v node 2>/dev/null || echo node) - cat << GH_AW_MCP_CONFIG_ef719422760c1562_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" + cat << GH_AW_MCP_CONFIG_971187abc08ce778_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" { "mcpServers": { "github": { @@ -756,7 +756,7 @@ jobs: } } } - GH_AW_MCP_CONFIG_ef719422760c1562_EOF + GH_AW_MCP_CONFIG_971187abc08ce778_EOF - name: Mount MCP servers as CLIs id: mount-mcp-clis continue-on-error: true @@ -1752,3 +1752,4 @@ jobs: /tmp/gh-aw/safe-output-items.jsonl /tmp/gh-aw/temporary-id-map.json if-no-files-found: ignore + diff --git a/docs/src/content/docs/reference/gh-aw-as-mcp-server.md b/docs/src/content/docs/reference/gh-aw-as-mcp-server.md index a5bcc23b6d1..5a85b07b19d 100644 --- a/docs/src/content/docs/reference/gh-aw-as-mcp-server.md +++ b/docs/src/content/docs/reference/gh-aw-as-mcp-server.md @@ -204,7 +204,7 @@ Apply automatic codemod-style fixes to workflow files. - `write` (optional): Write changes to files (default is dry-run) - `list_codemods` (optional): List available codemods and exit -Available codemods: `timeout-minutes-migration`, `network-firewall-migration`, `sandbox-agent-false-removal`, `mcp-scripts-mode-removal`. +Available codemods: `timeout-minutes-migration`, `network-firewall-migration`, `sandbox-agent-false-removal`, `mcp-scripts-mode-removal`, `steps-run-secrets-to-env`. ## Using GH-AW as an MCP from an Agentic Workflow diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index 62c71f8a164..7c4d2a759e2 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -244,7 +244,7 @@ gh aw fix --list-codemods # List available codemods Available codemods include: - `expires-integer-to-string` — converts bare integer `expires` values (e.g., `expires: 7`) to the preferred day-string format (e.g., `expires: 7d`) in all `safe-outputs` blocks. -- `steps-run-secrets-to-env` — rewrites inline `${{ secrets.NAME }}` interpolations in step `run:` commands to `$NAME` and adds step-level `env` bindings. Required for strict-mode compliance. +- `steps-run-secrets-to-env` — rewrites **all** `${{ ... }}` expressions in step `run:` commands to `$VARNAME` references (or `$env:VARNAME` for PowerShell steps) and adds step-level `env` bindings. Secrets, `env.*`, and `github.token` use stable legacy names; all other expressions receive `EXPR_*` names. Required for strict-mode compliance. - `engine-env-secrets-to-engine-config` — removes secret-bearing entries from `engine.env` that are unsafe under strict mode, preserving required engine credential keys. Run `gh aw fix --list-codemods` to see all available codemods. diff --git a/pkg/cli/codemod_steps_run_secrets_env.go b/pkg/cli/codemod_steps_run_secrets_env.go index 239d2f2ca3d..ca95743ac75 100644 --- a/pkg/cli/codemod_steps_run_secrets_env.go +++ b/pkg/cli/codemod_steps_run_secrets_env.go @@ -18,15 +18,24 @@ var ( stepsSecretRefExprRe = regexp.MustCompile(`\bsecrets\.([A-Za-z_][A-Za-z0-9_]*)\b`) stepsEnvRefExprRe = regexp.MustCompile(`\benv\.([A-Za-z_][A-Za-z0-9_]*)\b`) stepsGitHubTokenRe = regexp.MustCompile(`\bgithub\.token\b`) + // stepsGenericExprRe matches simple GitHub Actions property-access chains such as + // "github.repository", "inputs.my-input", "steps.my-step.outputs.result". + // Only word characters and hyphens separated by dots are allowed; anything + // containing spaces, operators, or other punctuation falls through to a + // hash-based name. + stepsGenericExprRe = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_-]*(\.[a-zA-Z_][a-zA-Z0-9_-]*)*$`) ) -// getStepsRunSecretsToEnvCodemod creates a codemod that moves secrets interpolated directly -// in run fields to step-level env bindings in steps-like sections. +// getStepsRunSecretsToEnvCodemod creates a codemod that moves all ${{ ... }} +// expressions interpolated directly in run fields to step-level env bindings. +// Secrets, env refs, and github.token are given stable legacy names; all other +// expressions receive an EXPR_* name. PowerShell steps (shell: pwsh / powershell) +// receive $env:VARNAME references instead of $VARNAME. func getStepsRunSecretsToEnvCodemod() Codemod { return Codemod{ ID: "steps-run-secrets-to-env", - Name: "Move step run secrets to env bindings", - Description: "Rewrites secrets interpolated directly in run commands to $VARS and adds step-level env bindings for strict-mode compatibility.", + Name: "Move step run expressions to env bindings", + Description: "Rewrites all ${{ ... }} expressions interpolated directly in run commands to $VARS (or $env:VARS for PowerShell steps) and adds step-level env bindings for strict-mode compatibility. Note: expressions inside single-quoted strings are rewritten too; since single quotes suppress shell variable expansion, those sections should be double-quoted if the substituted value is required.", IntroducedIn: "0.26.0", Apply: func(content string, frontmatter map[string]any) (string, bool, error) { sections := []string{"pre-steps", "steps", "post-steps", "pre-agent-steps"} @@ -153,6 +162,30 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string var envKeyIndentLen int existingEnvKeys := make(map[string]bool) + // First pass: detect shell type so PowerShell steps get $env:VARNAME syntax. + // Restrict the scan to lines at the direct step-key indentation level so + // that a run-block body line that happens to contain a literal substring + // like "shell: pwsh" is not misclassified as PowerShell. + shellIsPowerShell := false + directKeyIndent := stepIndent + " " + for _, line := range stepLines { + trimmed := strings.TrimSpace(line) + indent := getIndentation(line) + // Accept only direct step-key lines: standard form at exactly stepIndent+" ", + // or list-item-inline form "- key:" at exactly stepIndent. + if indent != directKeyIndent && !(indent == stepIndent && strings.HasPrefix(trimmed, "- ")) { + continue + } + shellMatch, shellValue, _ := parseStepKeyLine(trimmed, indent, stepIndent, "shell") + if shellMatch { + v := strings.ToLower(strings.TrimSpace(shellValue)) + if v == "pwsh" || v == "powershell" { + shellIsPowerShell = true + } + break + } + } + for i := 0; i < len(stepLines); i++ { line := stepLines[i] trimmed := strings.TrimSpace(line) @@ -198,7 +231,7 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= runKeyIndentLen { break } - updatedLine, bindings := replaceStepExpressionRefs(stepLines[j]) + updatedLine, bindings := replaceStepExpressionRefs(stepLines[j], shellIsPowerShell, bindingExprs) if len(bindings) > 0 { stepLines[j] = updatedLine modified = true @@ -214,7 +247,7 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string continue } - newLine, bindings := replaceStepExpressionRefs(line) + newLine, bindings := replaceStepExpressionRefs(line, shellIsPowerShell, bindingExprs) if len(bindings) > 0 { stepLines[i] = newLine modified = true @@ -275,7 +308,7 @@ type stepExpressionBinding struct { Expression string } -func replaceStepExpressionRefs(line string) (string, []stepExpressionBinding) { +func replaceStepExpressionRefs(line string, shellIsPowerShell bool, existingBindings map[string]string) (string, []stepExpressionBinding) { matches := stepsAnyExprRe.FindAllStringSubmatchIndex(line, -1) if len(matches) == 0 { return line, nil @@ -283,7 +316,15 @@ func replaceStepExpressionRefs(line string) (string, []stepExpressionBinding) { var result strings.Builder last := 0 - seen := make(map[string]bool) + // bodyToName maps expression body → assigned env-var name for same-body dedup + // within this line (avoids re-computing the name for repeated occurrences). + bodyToName := make(map[string]string) + // localNames maps env-var name → canonical expression for within-line + // collision detection (two different bodies that sanitize to the same name). + localNames := make(map[string]string) + // registeredNames tracks which names already appear in ordered, so we never + // add a duplicate binding entry. + registeredNames := make(map[string]bool) ordered := make([]stepExpressionBinding, 0, len(matches)) for _, match := range matches { @@ -297,6 +338,17 @@ func replaceStepExpressionRefs(line string) (string, []stepExpressionBinding) { result.WriteString(line[last:fullStart]) + // Same expression body already resolved in this line – reuse the name. + if cachedName, done := bodyToName[body]; done { + if shellIsPowerShell { + result.WriteString("$env:" + cachedName) + } else { + result.WriteString("$" + cachedName) + } + last = fullEnd + continue + } + envName, canonicalExpression, ok := mapRunExpressionToEnvBinding(body) if !ok { result.WriteString(fullExpression) @@ -304,9 +356,26 @@ func replaceStepExpressionRefs(line string) (string, []stepExpressionBinding) { continue } - result.WriteString("$" + envName) - if !seen[envName] { - seen[envName] = true + // Collision guard: if this env-var name is already bound to a *different* + // expression (from a previous line in this step via existingBindings, or + // from an earlier occurrence within this line via localNames), fall back + // to a hash-based name so both expressions receive unique bindings. + if crossLine := existingBindings[envName]; (crossLine != "" && crossLine != canonicalExpression) || + (localNames[envName] != "" && localNames[envName] != canonicalExpression) { + envName = hashedBindingName("EXPR", body) + canonicalExpression = fmt.Sprintf("${{ %s }}", body) + } + + bodyToName[body] = envName + localNames[envName] = canonicalExpression + + if shellIsPowerShell { + result.WriteString("$env:" + envName) + } else { + result.WriteString("$" + envName) + } + if !registeredNames[envName] { + registeredNames[envName] = true ordered = append(ordered, stepExpressionBinding{ Name: envName, Expression: canonicalExpression, @@ -346,7 +415,15 @@ func mapRunExpressionToEnvBinding(body string) (string, string, bool) { return hashedBindingName("GH_AW_GITHUB_TOKEN", body), fmt.Sprintf("${{ %s }}", body), true } - return "", "", false + // Catch-all: hoist any remaining expression using EXPR_ naming. + if stepsGenericExprRe.MatchString(body) { + replacer := strings.NewReplacer(".", "_", "-", "_") + name := "EXPR_" + strings.ToUpper(replacer.Replace(body)) + return name, fmt.Sprintf("${{ %s }}", body), true + } + // Complex expression: use a hash suffix for collision safety. + name := hashedBindingName("EXPR", body) + return name, fmt.Sprintf("${{ %s }}", body), true } // hashedBindingName returns a collision-resistant binding key by suffixing diff --git a/pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go b/pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go index 85afecfc282..2c294e704c7 100644 --- a/pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go +++ b/pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go @@ -3,6 +3,7 @@ package cli import ( + "fmt" "strings" "testing" ) @@ -46,7 +47,8 @@ func FuzzStepsRunSecretsToEnvCodemod(f *testing.F) { t.Fatalf("expected mutation for run=%q", run) } runLine := extractFuzzRunLine(result) - if strings.Contains(runLine, "${{ secrets.") || strings.Contains(runLine, "${{ env.") || strings.Contains(runLine, "${{ github.token") { + // No ${{ ... }} expression should remain in the run line after applying. + if strings.Contains(runLine, "${{") { t.Fatalf("run line still contains expression interpolation: %q", runLine) } for _, variable := range expectedVars { @@ -66,6 +68,125 @@ func FuzzStepsRunSecretsToEnvCodemod(f *testing.F) { }) } +// FuzzStepsRunSecretsToEnvCodemodExpr tests the EXPR_* catch-all path that hoists +// arbitrary GitHub Actions property-access chains (e.g. github.repository, +// inputs.my-input, steps.step-id.outputs.result) to EXPR_* env bindings. +func FuzzStepsRunSecretsToEnvCodemodExpr(f *testing.F) { + f.Add(uint8(0), "github", "repository", false) + f.Add(uint8(1), "inputs", "my-input", false) + f.Add(uint8(2), "github", "sha", true) + f.Add(uint8(3), "runner", "os", false) + + f.Fuzz(func(t *testing.T, sectionSelector uint8, namespace, propNameRaw string, preseedBinding bool) { + namespace = sanitizeHoistPropertySegment(namespace) + propName := sanitizeHoistPropertySegment(propNameRaw) + section := []string{"pre-steps", "steps", "post-steps", "pre-agent-steps"}[int(sectionSelector)%4] + + // Build a simple two-segment property-access expression like "github.sha". + // stepsGenericExprRe requires valid property-chain characters; both + // segments are sanitised above. + expr := namespace + "." + propName + run := fmt.Sprintf(`echo "${{ %s }}"`, expr) + + expectedEnvVar := "EXPR_" + strings.ToUpper(strings.NewReplacer(".", "_", "-", "_").Replace(expr)) + + var lines []string + lines = append(lines, "---", "on: push", section+":", " - name: fuzz") + if preseedBinding { + lines = append(lines, " env:", " "+expectedEnvVar+": ${{ "+expr+" }}") + } + lines = append(lines, " run: "+run, "---") + content := strings.Join(lines, "\n") + "\n" + + frontmatter := map[string]any{ + "on": "push", + section: []any{map[string]any{"name": "fuzz", "run": run}}, + "workflow": "fuzz", + } + + result, applied, err := getStepsRunSecretsToEnvCodemod().Apply(content, frontmatter) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !applied { + t.Fatalf("expected codemod to apply for expr=%q run=%q", expr, run) + } + runLine := extractFuzzRunLine(result) + if strings.Contains(runLine, "${{") { + t.Fatalf("run line still contains expression interpolation after apply: %q", runLine) + } + if !strings.Contains(runLine, "$"+expectedEnvVar) { + t.Fatalf("run line missing %q: %q", "$"+expectedEnvVar, runLine) + } + if countEnvBindingKey(result, expectedEnvVar) != 1 { + t.Fatalf("expected exactly one env binding for %s, result:\n%s", expectedEnvVar, result) + } + }) +} + +// FuzzStepsRunSecretsToEnvCodemodPowerShell tests that PowerShell steps +// (shell: pwsh / shell: powershell) receive $env:VARNAME references instead +// of $VARNAME for all hoisted expressions. +func FuzzStepsRunSecretsToEnvCodemodPowerShell(f *testing.F) { + f.Add(uint8(0), "MY_TOKEN", true) + f.Add(uint8(1), "DEPLOY_KEY", false) + f.Add(uint8(2), "A", true) + + f.Fuzz(func(t *testing.T, shellSelector uint8, secretNameRaw string, includeGitHubToken bool) { + secretName := sanitizeHoistName(secretNameRaw) + shell := []string{"pwsh", "powershell"}[int(shellSelector)%2] + section := "steps" + + parts := []string{"${{ secrets." + secretName + " }}"} + if includeGitHubToken { + parts = append(parts, "${{ github.token }}") + } + run := `Write-Output "` + strings.Join(parts, " ") + `"` + + content := strings.Join([]string{ + "---", + "on: push", + section + ":", + " - name: ps fuzz", + " shell: " + shell, + " run: " + run, + "---", + }, "\n") + "\n" + + frontmatter := map[string]any{ + "on": "push", + section: []any{map[string]any{ + "name": "ps fuzz", + "shell": shell, + "run": run, + }}, + } + + result, applied, err := getStepsRunSecretsToEnvCodemod().Apply(content, frontmatter) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !applied { + t.Fatalf("expected codemod to apply for shell=%s run=%q", shell, run) + } + runLine := extractFuzzRunLine(result) + if strings.Contains(runLine, "${{") { + t.Fatalf("run line still contains expression interpolation: %q", runLine) + } + // PowerShell steps must use $env:VARNAME — never bare $VARNAME for the + // secret binding (the plain $NAME form must not appear in the run line). + if strings.Contains(runLine, "run: Write-Output \"$"+secretName) { + t.Fatalf("PowerShell run line uses bare $VARNAME instead of $env:VARNAME: %q", runLine) + } + if !strings.Contains(runLine, "$env:"+secretName) { + t.Fatalf("PowerShell run line missing $env:%s: %q", secretName, runLine) + } + if countEnvBindingKey(result, secretName) != 1 { + t.Fatalf("expected exactly one env binding for %s", secretName) + } + }) +} + func buildHoistFuzzRun(includeSecret, includeComplexSecret, includeEnvExpr, includeGitHubToken bool, secretName, envName string) (string, []string) { parts := make([]string, 0, 3) expected := make([]string, 0, 4) @@ -182,3 +303,37 @@ func sanitizeHoistName(raw string) string { } return s } + +// sanitizeHoistPropertySegment converts arbitrary fuzz input into a valid +// GitHub Actions property-access segment accepted by stepsGenericExprRe: +// [a-zA-Z_][a-zA-Z0-9_-]*, max 20 chars. +func sanitizeHoistPropertySegment(raw string) string { + if raw == "" { + return "prop" + } + var b strings.Builder + for _, r := range raw { + switch { + case r >= 'A' && r <= 'Z': + b.WriteRune(r + ('a' - 'A')) // lowercase + case r >= 'a' && r <= 'z': + b.WriteRune(r) + case r >= '0' && r <= '9': + b.WriteRune(r) + case r == '_' || r == '-': + b.WriteRune(r) + } + if b.Len() >= 20 { + break + } + } + s := b.String() + if s == "" { + return "prop" + } + // Ensure the segment starts with a letter or underscore. + if (s[0] >= '0' && s[0] <= '9') || s[0] == '-' { + return "p" + s + } + return s +} diff --git a/pkg/cli/codemod_steps_run_secrets_env_test.go b/pkg/cli/codemod_steps_run_secrets_env_test.go index 1a429a6d777..2b005a1b8ed 100644 --- a/pkg/cli/codemod_steps_run_secrets_env_test.go +++ b/pkg/cli/codemod_steps_run_secrets_env_test.go @@ -369,4 +369,239 @@ steps: assert.False(t, applied, "codemod should not apply") assert.Equal(t, content, result, "content should be unchanged") }) + + t.Run("hoists non-secrets expression to EXPR_ env binding", func(t *testing.T) { + content := `--- +on: push +steps: + - run: echo ${{ github.repository }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{"run": "echo ${{ github.repository }}"}}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "EXPR_GITHUB_REPOSITORY: ${{ github.repository }}") + assert.Contains(t, result, "run: echo $EXPR_GITHUB_REPOSITORY") + }) + + t.Run("hoists inputs expression to EXPR_ env binding", func(t *testing.T) { + content := `--- +on: push +steps: + - run: echo ${{ inputs.my-input }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{"run": "echo ${{ inputs.my-input }}"}}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "EXPR_INPUTS_MY_INPUT: ${{ inputs.my-input }}") + assert.Contains(t, result, "run: echo $EXPR_INPUTS_MY_INPUT") + }) + + t.Run("hoists steps output expression to EXPR_ env binding", func(t *testing.T) { + content := `--- +on: push +steps: + - run: echo ${{ steps.my-step.outputs.result }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{"run": "echo ${{ steps.my-step.outputs.result }}"}}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "EXPR_STEPS_MY_STEP_OUTPUTS_RESULT: ${{ steps.my-step.outputs.result }}") + assert.Contains(t, result, "run: echo $EXPR_STEPS_MY_STEP_OUTPUTS_RESULT") + }) + + t.Run("hoists complex non-secrets expression with hash-based name", func(t *testing.T) { + content := `--- +on: push +steps: + - run: echo "${{ inputs.foo || 'default' }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{"run": `echo "${{ inputs.foo || 'default' }}"`}}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "${{ inputs.foo || 'default' }}", "complex expression should be preserved in env binding") + envBindings := regexp.MustCompile(`EXPR_[0-9a-f]{8}:`).FindAllString(result, -1) + assert.Len(t, envBindings, 1, "one hash-based EXPR_ binding should be created") + }) + + t.Run("uses $env:VARNAME for PowerShell steps (pwsh)", func(t *testing.T) { + content := `--- +on: push +steps: + - name: PS step + shell: pwsh + run: | + Write-Output "${{ github.actor }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{ + "name": "PS step", + "shell": "pwsh", + "run": `Write-Output "${{ github.actor }}"`, + }}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "EXPR_GITHUB_ACTOR: ${{ github.actor }}") + assert.Contains(t, result, `Write-Output "$env:EXPR_GITHUB_ACTOR"`, "PowerShell step should use $env:VARNAME syntax") + }) + + t.Run("uses $env:VARNAME for PowerShell steps (powershell)", func(t *testing.T) { + content := `--- +on: push +steps: + - name: PS step + shell: powershell + run: Write-Output ${{ github.actor }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{ + "name": "PS step", + "shell": "powershell", + "run": "Write-Output ${{ github.actor }}", + }}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "EXPR_GITHUB_ACTOR: ${{ github.actor }}") + assert.Contains(t, result, "run: Write-Output $env:EXPR_GITHUB_ACTOR") + }) + + t.Run("uses $env:VARNAME for PowerShell steps with secrets", func(t *testing.T) { + content := `--- +on: push +steps: + - shell: pwsh + run: Write-Output ${{ secrets.MY_TOKEN }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{ + "shell": "pwsh", + "run": "Write-Output ${{ secrets.MY_TOKEN }}", + }}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "MY_TOKEN: ${{ secrets.MY_TOKEN }}") + assert.Contains(t, result, "run: Write-Output $env:MY_TOKEN", "PowerShell secrets also use $env:VARNAME") + }) + + t.Run("bash step uses $VARNAME not $env:VARNAME for EXPR_ bindings", func(t *testing.T) { + content := `--- +on: push +steps: + - shell: bash + run: echo ${{ github.actor }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{"shell": "bash", "run": "echo ${{ github.actor }}"}}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "run: echo $EXPR_GITHUB_ACTOR") + assert.NotContains(t, result, "$env:EXPR_GITHUB_ACTOR") + }) + + t.Run("does not misclassify shell from run body containing shell: pwsh", func(t *testing.T) { + // A run block whose body contains a literal "shell: pwsh" line should not + // cause the step to be treated as a PowerShell step. + content := `--- +on: push +steps: + - name: Bash step with literal shell text + run: | + echo "shell: pwsh is not a real key here" + echo ${{ github.actor }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{ + "name": "Bash step with literal shell text", + "run": "echo \"shell: pwsh is not a real key here\"\necho ${{ github.actor }}", + }}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + // Must use bare $VARNAME because no real shell: pwsh key is present. + assert.Contains(t, result, "$EXPR_GITHUB_ACTOR", "bash step should use bare $VARNAME") + assert.NotContains(t, result, "$env:EXPR_GITHUB_ACTOR", "bash step must not use $env:VARNAME") + }) + + t.Run("uses distinct bindings when different bodies collide to the same EXPR_ name", func(t *testing.T) { + // inputs.my-input and inputs.my_input both sanitize to EXPR_INPUTS_MY_INPUT. + // The second one must fall back to a hash-based name to avoid being silently + // bound to the wrong expression. + content := `--- +on: push +steps: + - run: echo "${{ inputs.my-input }} ${{ inputs.my_input }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{map[string]any{"run": `echo "${{ inputs.my-input }} ${{ inputs.my_input }}"`}}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + // Both expressions must be present in env bindings. + assert.Contains(t, result, "${{ inputs.my-input }}", "first expression preserved in env binding") + assert.Contains(t, result, "${{ inputs.my_input }}", "second expression preserved in env binding") + // The first expression gets the canonical sanitized name; the second gets a hash-based name. + assert.Contains(t, result, "EXPR_INPUTS_MY_INPUT: ${{ inputs.my-input }}", "first expression should use sanitized EXPR_ name") + // The run line must not contain any raw ${{ ... }} interpolation. + for _, line := range strings.Split(result, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "run:") { + assert.NotContains(t, line, "${{", "run line must not contain raw expression interpolation") + assert.Contains(t, line, "$EXPR_INPUTS_MY_INPUT", "run line should reference sanitized name for first expression") + } + } + // There should be exactly two distinct env-var bindings (one per expression). + exprBindings := regexp.MustCompile(`EXPR_[A-Za-z0-9_]+:`).FindAllString(result, -1) + require.Len(t, exprBindings, 2, "each colliding expression should get a unique binding") + assert.NotEqual(t, exprBindings[0], exprBindings[1], "the two collision bindings must have distinct names") + }) } diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index b176729eae2..b46cbf76649 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -46,7 +46,7 @@ func GetAllCodemods() []Codemod { getBotsToOnBotsCodemod(), // Move top-level bots to on.bots getEngineStepsToTopLevelCodemod(), // Move engine.steps to top-level steps getEngineMaxRunsToTopLevelCodemod(), // Move engine.max-runs to top-level max-runs - getStepsRunSecretsToEnvCodemod(), // Move inline secrets in step run fields to step env bindings + getStepsRunSecretsToEnvCodemod(), // Move all ${{ ... }} expressions in step run fields to step env bindings getEngineEnvSecretsCodemod(), // Remove unsafe secret-bearing engine.env entries getAssignToAgentDefaultAgentCodemod(), // Rename deprecated default-agent to name in assign-to-agent getPlaywrightDomainsToNetworkAllowedCodemod(), // Migrate tools.playwright.allowed_domains to network.allowed