From 5b0e28fb62e2508e2e3ca3e6ec533408b8571fa1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 02:34:57 +0000 Subject: [PATCH 01/10] Initial plan From 269e30845826f21ae10c2b8f347d19ca3898220c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 02:46:03 +0000 Subject: [PATCH 02/10] Initial progress checkpoint - no changes yet Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../workflows/daily-model-inventory.lock.yml | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/.github/workflows/daily-model-inventory.lock.yml b/.github/workflows/daily-model-inventory.lock.yml index 582b2fc4ff..5ada45ec8f 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 + From ebbcc6ccb6ab5e08e7a2997b888dc856989c2aec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 02:50:37 +0000 Subject: [PATCH 03/10] Add auto-hoist-run-expressions codemod for generic run-block expression hoisting Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...inor-auto-hoist-run-expressions-codemod.md | 5 + .../docs/reference/gh-aw-as-mcp-server.md | 2 +- docs/src/content/docs/setup/cli.md | 1 + pkg/cli/codemod_auto_hoist_run_expressions.go | 374 ++++++++++++++ ...codemod_auto_hoist_run_expressions_test.go | 460 ++++++++++++++++++ pkg/cli/fix_codemods.go | 1 + pkg/cli/fix_codemods_test.go | 2 + 7 files changed, 844 insertions(+), 1 deletion(-) create mode 100644 .changeset/minor-auto-hoist-run-expressions-codemod.md create mode 100644 pkg/cli/codemod_auto_hoist_run_expressions.go create mode 100644 pkg/cli/codemod_auto_hoist_run_expressions_test.go 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 0000000000..ef77f07201 --- /dev/null +++ b/.changeset/minor-auto-hoist-run-expressions-codemod.md @@ -0,0 +1,5 @@ +--- +"gh-aw": minor +--- + +Add `auto-hoist-run-expressions` codemod that rewrites **all** `${{ ... }}` expressions inside `run:` blocks to `$VARNAME` shell references and inserts `EXPR_*` step-level `env:` bindings. This closes the gap left by `steps-run-secrets-to-env` (which only handles `secrets.*`, `env.*`, and `github.token`) by covering arbitrary expressions such as `github.repository`, `github.event.issue.title`, `inputs.*`, and `steps.*.outputs.*`. PowerShell steps (`shell: pwsh` / `shell: powershell`) receive `$env:VARNAME` syntax instead of `$VARNAME`. 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 a5bcc23b6d..70d2b3eb40 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`, `auto-hoist-run-expressions`. ## 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 62c71f8a16..00aab2d1ab 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -245,6 +245,7 @@ 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. +- `auto-hoist-run-expressions` — rewrites **all** `${{ ... }}` expressions in step `run:` scripts to `$VARNAME` references (or `$env:VARNAME` for PowerShell steps) and adds `EXPR_*` step-level `env` bindings. Catches any expression not already handled by `steps-run-secrets-to-env`, preventing the "compiler regression detected" hard error. - `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_auto_hoist_run_expressions.go b/pkg/cli/codemod_auto_hoist_run_expressions.go new file mode 100644 index 0000000000..170234bc0e --- /dev/null +++ b/pkg/cli/codemod_auto_hoist_run_expressions.go @@ -0,0 +1,374 @@ +package cli + +import ( + "fmt" + "regexp" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var autoHoistRunExpressionsLog = logger.New("cli:codemod_auto_hoist_run_expressions") + +// autoHoistSimpleExprBodyRe matches simple JavaScript property-access chains such as +// "github.token", "env.FOO", "inputs.my-input", "steps.my-step.outputs.result". +// Only word characters and hyphens separated by dots are allowed; any expression +// containing spaces, operators, or other punctuation falls through to the hash-based +// name generator. +var autoHoistSimpleExprBodyRe = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_-]*(\.[a-zA-Z_][a-zA-Z0-9_-]*)*$`) + +// getAutoHoistRunExpressionsCodemod creates a codemod that hoists ALL ${{ ... }} +// expressions from run: blocks into step-level env bindings. +// +// Unlike steps-run-secrets-to-env (which only handles secrets.*, env.*, and +// github.token), this codemod covers every expression that would otherwise trigger +// the "compiler regression detected" hard error. +// +// Naming convention: EXPR_ + sanitised uppercase expression body. +// - Simple chains: github.token → EXPR_GITHUB_TOKEN +// - Complex expressions: ${{ secrets.TOKEN || '' }} → EXPR_<8-char-hash> +// +// PowerShell awareness: steps with shell: pwsh or shell: powershell receive +// $env:VARNAME references in the run script instead of $VARNAME. +func getAutoHoistRunExpressionsCodemod() Codemod { + return Codemod{ + ID: "auto-hoist-run-expressions", + Name: "Auto-hoist run-block expressions to env bindings", + Description: "Rewrites all ${{ ... }} expressions in run: scripts to $VARNAME references (or $env:VARNAME for PowerShell steps) and adds EXPR_* step-level env bindings.", + IntroducedIn: "1.0.45", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + sections := []string{"pre-steps", "steps", "post-steps", "pre-agent-steps"} + hasTargetSection := false + for _, section := range sections { + if _, ok := frontmatter[section]; ok { + hasTargetSection = true + break + } + } + if !hasTargetSection { + return content, false, nil + } + + newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) { + modified := false + current := lines + for _, section := range sections { + var sectionChanged bool + current, sectionChanged = transformSectionAutoHoistExpressions(current, section) + modified = modified || sectionChanged + } + return current, modified + }) + if applied { + autoHoistRunExpressionsLog.Print("Auto-hoisted inline run expressions to step-level env bindings") + } + return newContent, applied, err + }, + } +} + +func transformSectionAutoHoistExpressions(lines []string, sectionName string) ([]string, bool) { + sectionStart := -1 + sectionIndent := "" + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if isTopLevelKey(line) && strings.HasPrefix(trimmed, sectionName+":") { + sectionStart = i + sectionIndent = getIndentation(line) + break + } + } + if sectionStart == -1 { + return lines, false + } + + sectionEnd := len(lines) - 1 + for i := sectionStart + 1; i < len(lines); i++ { + trimmed := strings.TrimSpace(lines[i]) + if len(trimmed) == 0 || strings.HasPrefix(trimmed, "#") { + continue + } + if len(getIndentation(lines[i])) <= len(sectionIndent) { + sectionEnd = i - 1 + break + } + } + + autoHoistRunExpressionsLog.Printf("Transforming section '%s': lines %d-%d", sectionName, sectionStart, sectionEnd) + + sectionLines := lines[sectionStart : sectionEnd+1] + updatedSection, changed := transformStepsWithinSectionAutoHoist(sectionLines, sectionIndent) + if !changed { + return lines, false + } + + result := make([]string, 0, len(lines)-(len(sectionLines)-len(updatedSection))) + result = append(result, lines[:sectionStart]...) + result = append(result, updatedSection...) + result = append(result, lines[sectionEnd+1:]...) + return result, true +} + +func transformStepsWithinSectionAutoHoist(sectionLines []string, sectionIndent string) ([]string, bool) { + result := make([]string, 0, len(sectionLines)) + modified := false + + for i := 0; i < len(sectionLines); { + line := sectionLines[i] + trimmed := strings.TrimSpace(line) + indent := getIndentation(line) + + if strings.HasPrefix(trimmed, "- ") && len(indent) > len(sectionIndent) { + stepStart := i + stepIndent := indent + stepEnd := len(sectionLines) - 1 + for j := i + 1; j < len(sectionLines); j++ { + t := strings.TrimSpace(sectionLines[j]) + if len(t) == 0 { + continue + } + jIndent := getIndentation(sectionLines[j]) + if strings.HasPrefix(t, "- ") && len(jIndent) == len(stepIndent) { + stepEnd = j - 1 + break + } + } + + chunk := append([]string(nil), sectionLines[stepStart:stepEnd+1]...) + updatedChunk, changed := rewriteStepAutoHoistExpressions(chunk, stepIndent) + modified = modified || changed + result = append(result, updatedChunk...) + i = stepEnd + 1 + continue + } + + result = append(result, line) + i++ + } + + return result, modified +} + +func rewriteStepAutoHoistExpressions(stepLines []string, stepIndent string) ([]string, bool) { + modified := false + seen := make(map[string]bool) + orderedBindings := make([]string, 0) + bindingExprs := make(map[string]string) + firstRunLine := -1 + envStart := -1 + envEnd := -1 + envIndent := "" + var envKeyIndentLen int + existingEnvKeys := make(map[string]bool) + shellIsPowerShell := false + + // First pass: detect shell type so PowerShell steps get $env:VARNAME syntax. + for _, line := range stepLines { + trimmed := strings.TrimSpace(line) + indent := getIndentation(line) + shellMatch, shellValue, _ := parseStepKeyLine(trimmed, indent, stepIndent, "shell") + if shellMatch { + v := strings.ToLower(strings.TrimSpace(shellValue)) + if v == "pwsh" || v == "powershell" { + shellIsPowerShell = true + } + break + } + } + + // Second pass: scan env block and rewrite run block. + for i := 0; i < len(stepLines); i++ { + line := stepLines[i] + trimmed := strings.TrimSpace(line) + indent := getIndentation(line) + + // Track the existing env: block so new bindings can be appended to it. + envMatchKey, envValue, currentEnvKeyIndentLen := parseStepKeyLine(trimmed, indent, stepIndent, "env") + if envMatchKey && envValue == "" { + envStart = i + envIndent = indent + envKeyIndentLen = currentEnvKeyIndentLen + envEnd = i + for j := i + 1; j < len(stepLines); j++ { + t := strings.TrimSpace(stepLines[j]) + if len(t) == 0 { + envEnd = j + continue + } + if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= envKeyIndentLen { + break + } + envEnd = j + key := parseYAMLMapKey(t) + if key != "" { + existingEnvKeys[key] = true + } + } + } + + runMatch, runValue, runKeyIndentLen := parseStepKeyLine(trimmed, indent, stepIndent, "run") + if !runMatch { + continue + } + if firstRunLine == -1 { + firstRunLine = i + } + + if runValue == "|" || runValue == "|-" || runValue == ">" || runValue == ">-" { + for j := i + 1; j < len(stepLines); j++ { + t := strings.TrimSpace(stepLines[j]) + if len(t) == 0 { + continue + } + if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= runKeyIndentLen { + break + } + updatedLine, bindings := replaceAutoHoistExpressionRefs(stepLines[j], shellIsPowerShell) + if len(bindings) > 0 { + stepLines[j] = updatedLine + modified = true + } + for _, binding := range bindings { + if !seen[binding.Name] { + seen[binding.Name] = true + orderedBindings = append(orderedBindings, binding.Name) + bindingExprs[binding.Name] = binding.Expression + } + } + } + continue + } + + newLine, bindings := replaceAutoHoistExpressionRefs(line, shellIsPowerShell) + if len(bindings) > 0 { + stepLines[i] = newLine + modified = true + } + for _, binding := range bindings { + if !seen[binding.Name] { + seen[binding.Name] = true + orderedBindings = append(orderedBindings, binding.Name) + bindingExprs[binding.Name] = binding.Expression + } + } + } + + if len(orderedBindings) == 0 { + return stepLines, modified + } + + autoHoistRunExpressionsLog.Printf("Found %d unique run expression references in step run commands", len(orderedBindings)) + + missingBindings := make([]string, 0, len(orderedBindings)) + for _, name := range orderedBindings { + if !existingEnvKeys[name] { + missingBindings = append(missingBindings, name) + } + } + if len(missingBindings) == 0 { + return stepLines, true + } + + autoHoistRunExpressionsLog.Printf("Adding env bindings for %d missing expressions: %v", len(missingBindings), missingBindings) + + if envStart != -1 { + insertAt := envEnd + 1 + envValueIndent := envIndent + " " + insertLines := make([]string, 0, len(missingBindings)) + for _, name := range missingBindings { + insertLines = append(insertLines, fmt.Sprintf("%s%s: %s", envValueIndent, name, bindingExprs[name])) + } + stepLines = append(stepLines[:insertAt], append(insertLines, stepLines[insertAt:]...)...) + return stepLines, true + } + + if firstRunLine == -1 { + return stepLines, modified + } + + insertIndent := stepIndent + " " + insertLines := []string{insertIndent + "env:"} + for _, name := range missingBindings { + insertLines = append(insertLines, fmt.Sprintf("%s %s: %s", insertIndent, name, bindingExprs[name])) + } + stepLines = append(stepLines[:firstRunLine], append(insertLines, stepLines[firstRunLine:]...)...) + return stepLines, true +} + +// replaceAutoHoistExpressionRefs replaces every ${{ ... }} expression in line +// with either $VARNAME (bash) or $env:VARNAME (PowerShell) and returns the +// corresponding env bindings to add to the step's env: block. +func replaceAutoHoistExpressionRefs(line string, shellIsPowerShell bool) (string, []stepExpressionBinding) { + matches := stepsAnyExprRe.FindAllStringSubmatchIndex(line, -1) + if len(matches) == 0 { + return line, nil + } + + var result strings.Builder + last := 0 + seen := make(map[string]bool) + ordered := make([]stepExpressionBinding, 0, len(matches)) + + for _, match := range matches { + if len(match) < 4 { + continue + } + fullStart, fullEnd := match[0], match[1] + bodyStart, bodyEnd := match[2], match[3] + fullExpression := line[fullStart:fullEnd] + body := strings.TrimSpace(line[bodyStart:bodyEnd]) + + result.WriteString(line[last:fullStart]) + + envName, canonicalExpression, ok := mapAutoHoistExpressionToEnvBinding(body) + if !ok { + result.WriteString(fullExpression) + last = fullEnd + continue + } + + if shellIsPowerShell { + result.WriteString("$env:" + envName) + } else { + result.WriteString("$" + envName) + } + if !seen[envName] { + seen[envName] = true + ordered = append(ordered, stepExpressionBinding{ + Name: envName, + Expression: canonicalExpression, + }) + } + last = fullEnd + } + + result.WriteString(line[last:]) + return result.String(), ordered +} + +// mapAutoHoistExpressionToEnvBinding maps any ${{ body }} expression to a +// deterministic EXPR_* environment variable name and the canonical expression +// string to bind as its value. +// +// Simple property-access chains (e.g. "github.token", "env.FOO", +// "inputs.my-input") produce pretty names: +// +// github.token → EXPR_GITHUB_TOKEN +// env.FOO → EXPR_ENV_FOO +// inputs.my-input → EXPR_INPUTS_MY_INPUT +// +// Complex expressions (anything containing spaces, operators, function calls, +// etc.) fall back to a hash-based name to guarantee uniqueness: +// +// secrets.TOKEN || '' → EXPR_<8-char-fnv32-hex> +func mapAutoHoistExpressionToEnvBinding(body string) (string, string, bool) { + if autoHoistSimpleExprBodyRe.MatchString(body) { + replacer := strings.NewReplacer(".", "_", "-", "_") + name := "EXPR_" + strings.ToUpper(replacer.Replace(body)) + return name, fmt.Sprintf("${{ %s }}", body), true + } + // Complex expression: include a hash suffix to prevent name collisions + // between different complex expressions in the same step. + name := hashedBindingName("EXPR", body) + return name, fmt.Sprintf("${{ %s }}", body), true +} diff --git a/pkg/cli/codemod_auto_hoist_run_expressions_test.go b/pkg/cli/codemod_auto_hoist_run_expressions_test.go new file mode 100644 index 0000000000..979ef7a71f --- /dev/null +++ b/pkg/cli/codemod_auto_hoist_run_expressions_test.go @@ -0,0 +1,460 @@ +//go:build !integration + +package cli + +import ( + "regexp" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAutoHoistRunExpressionsCodemod(t *testing.T) { + codemod := getAutoHoistRunExpressionsCodemod() + + t.Run("hoists github.token from block run to env binding", func(t *testing.T) { + content := `--- +on: push +steps: + - name: Capture token + run: | + echo "GH_TOKEN=${{ github.token }}" >> "$GITHUB_OUTPUT" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "name": "Capture token", + "run": "echo \"GH_TOKEN=${{ github.token }}\" >> \"$GITHUB_OUTPUT\"", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "EXPR_GITHUB_TOKEN: ${{ github.token }}", "github.token should be bound in env") + assert.Contains(t, result, `echo "GH_TOKEN=$EXPR_GITHUB_TOKEN"`, "run should use hoisted env var") + assert.NotContains(t, result, `echo "GH_TOKEN=${{ github.token }}"`, "inline expression should be removed from run block") + }) + + t.Run("hoists github.repository from inline run to 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, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "EXPR_GITHUB_REPOSITORY: ${{ github.repository }}", "github.repository should be bound in env") + assert.Contains(t, result, "run: echo $EXPR_GITHUB_REPOSITORY", "run should use hoisted env var") + }) + + t.Run("hoists inputs expression from run to 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, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "EXPR_INPUTS_MY_INPUT: ${{ inputs.my-input }}", "inputs.my-input should be bound in env") + assert.Contains(t, result, "run: echo $EXPR_INPUTS_MY_INPUT", "run should use hoisted env var") + }) + + t.Run("hoists steps output expression from run to 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, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "EXPR_STEPS_MY_STEP_OUTPUTS_RESULT: ${{ steps.my-step.outputs.result }}", "steps output should be bound in env") + assert.Contains(t, result, "run: echo $EXPR_STEPS_MY_STEP_OUTPUTS_RESULT", "run should use hoisted env var") + }) + + t.Run("uses hash-based name for complex expressions", func(t *testing.T) { + content := `--- +on: push +steps: + - run: echo "${{ github.token || secrets.PAT }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "run": `echo "${{ github.token || secrets.PAT }}"`, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "${{ github.token || secrets.PAT }}", "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 binding should be created for the complex expression") + }) + + t.Run("deduplicates identical expressions in one step", func(t *testing.T) { + content := `--- +on: push +steps: + - run: | + echo "${{ github.token }}" + echo "${{ github.token }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "run": "echo \"${{ github.token }}\"\necho \"${{ github.token }}\"", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Equal(t, 1, strings.Count(result, "EXPR_GITHUB_TOKEN: ${{ github.token }}"), "binding should appear exactly once") + assert.Equal(t, 2, strings.Count(result, "$EXPR_GITHUB_TOKEN"), "both occurrences in run should be replaced") + }) + + t.Run("handles multiple distinct expressions in one step", func(t *testing.T) { + content := `--- +on: push +steps: + - run: | + echo "${{ github.token }}" + echo "${{ github.repository }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "run": "echo \"${{ github.token }}\"\necho \"${{ github.repository }}\"", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "EXPR_GITHUB_TOKEN: ${{ github.token }}", "github.token binding should be added") + assert.Contains(t, result, "EXPR_GITHUB_REPOSITORY: ${{ github.repository }}", "github.repository binding should be added") + assert.Contains(t, result, `echo "$EXPR_GITHUB_TOKEN"`, "first run line should use hoisted token var") + assert.Contains(t, result, `echo "$EXPR_GITHUB_REPOSITORY"`, "second run line should use hoisted repo var") + }) + + t.Run("appends missing bindings to existing env block", func(t *testing.T) { + content := `--- +on: push +steps: + - name: Run checks + env: + FOO: bar + run: echo ${{ github.actor }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "name": "Run checks", + "env": map[string]any{"FOO": "bar"}, + "run": "echo ${{ github.actor }}", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "FOO: bar", "existing env entry should be preserved") + assert.Contains(t, result, "EXPR_GITHUB_ACTOR: ${{ github.actor }}", "new binding should be appended") + assert.Contains(t, result, "run: echo $EXPR_GITHUB_ACTOR", "run should reference the new env var") + }) + + t.Run("does not duplicate pre-existing EXPR_ bindings", func(t *testing.T) { + content := `--- +on: push +steps: + - env: + EXPR_GITHUB_TOKEN: ${{ github.token }} + run: echo "${{ github.token }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "env": map[string]any{ + "EXPR_GITHUB_TOKEN": "${{ github.token }}", + }, + "run": `echo "${{ github.token }}"`, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should still rewrite run expression reference") + assert.Equal(t, 1, strings.Count(result, "EXPR_GITHUB_TOKEN: ${{ github.token }}"), "existing binding should not be duplicated") + assert.Contains(t, result, `run: echo "$EXPR_GITHUB_TOKEN"`, "run should use the existing binding") + }) + + t.Run("uses $env:VARNAME for PowerShell steps", func(t *testing.T) { + content := `--- +on: push +steps: + - name: PS step + shell: pwsh + run: | + Write-Output "${{ github.token }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "name": "PS step", + "shell": "pwsh", + "run": `Write-Output "${{ github.token }}"`, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "EXPR_GITHUB_TOKEN: ${{ github.token }}", "env binding should be added") + assert.Contains(t, result, `Write-Output "$env:EXPR_GITHUB_TOKEN"`, "PowerShell step should use $env:VARNAME syntax") + }) + + t.Run("uses $env:VARNAME for powershell shell variant", 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, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "EXPR_GITHUB_ACTOR: ${{ github.actor }}", "env binding should be added") + assert.Contains(t, result, "run: Write-Output $env:EXPR_GITHUB_ACTOR", "powershell step should use $env:VARNAME syntax") + }) + + t.Run("bash step uses $VARNAME not $env:VARNAME", 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, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "run: echo $EXPR_GITHUB_ACTOR", "bash step should use $VARNAME syntax") + assert.NotContains(t, result, "$env:EXPR_GITHUB_ACTOR", "bash step should not use $env: syntax") + }) + + t.Run("supports pre-steps section", func(t *testing.T) { + content := `--- +on: pull_request +pre-steps: + - name: Pre check + run: echo ${{ github.event.pull_request.number }} +--- +` + frontmatter := map[string]any{ + "on": "pull_request", + "pre-steps": []any{ + map[string]any{ + "name": "Pre check", + "run": "echo ${{ github.event.pull_request.number }}", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "run: echo $EXPR_GITHUB_EVENT_PULL_REQUEST_NUMBER", "pre-steps run should be rewritten") + assert.Contains(t, result, "EXPR_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }}", "binding should be added") + }) + + t.Run("supports post-steps section", func(t *testing.T) { + content := `--- +on: push +post-steps: + - name: Post + run: echo ${{ github.run_number }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "post-steps": []any{ + map[string]any{ + "name": "Post", + "run": "echo ${{ github.run_number }}", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "EXPR_GITHUB_RUN_NUMBER: ${{ github.run_number }}", "post-steps binding should be added") + }) + + t.Run("no-op when no inline run expressions are present", func(t *testing.T) { + content := `--- +on: push +steps: + - name: Safe + run: echo "hello" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "name": "Safe", + "run": `echo "hello"`, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should not error in no-op case") + assert.False(t, applied, "codemod should not apply") + assert.Equal(t, content, result, "content should be unchanged") + }) + + t.Run("no-op when workflow has no steps sections", func(t *testing.T) { + content := `--- +on: push +--- +` + frontmatter := map[string]any{ + "on": "push", + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should not error with no steps sections") + assert.False(t, applied, "codemod should not apply") + assert.Equal(t, content, result, "content should be unchanged") + }) + + t.Run("two distinct complex expressions get different hash names", func(t *testing.T) { + content := `--- +on: push +steps: + - run: echo "${{ github.token || '' }} ${{ github.token || 'fallback' }}" +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "run": `echo "${{ github.token || '' }} ${{ github.token || 'fallback' }}"`, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + envBindings := regexp.MustCompile(`EXPR_[0-9a-f]{8}:`).FindAllString(result, -1) + assert.Len(t, envBindings, 2, "two distinct complex expressions should produce two distinct hashed bindings") + assert.NotEqual(t, envBindings[0], envBindings[1], "hashed names should differ for different expressions") + }) + + t.Run("list-item-inline run key is hoisted", func(t *testing.T) { + content := `--- +on: push +steps: + - run: echo ${{ github.sha }} +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "run": "echo ${{ github.sha }}", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply") + assert.Contains(t, result, "run: echo $EXPR_GITHUB_SHA", "inline run should be rewritten") + assert.Contains(t, result, "EXPR_GITHUB_SHA: ${{ github.sha }}", "inline run should get env binding") + }) +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index b176729eae..b595075f6b 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -47,6 +47,7 @@ func GetAllCodemods() []Codemod { 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 + getAutoHoistRunExpressionsCodemod(), // Hoist ALL ${{ ... }} run-block expressions to EXPR_* 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 diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 1b90df1d68..760256e079 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -85,6 +85,7 @@ func TestGetAllCodemods_ContainsExpectedCodemods(t *testing.T) { "rate-limit-to-user-rate-limit", "engine-max-runs-to-top-level", "steps-run-secrets-to-env", + "auto-hoist-run-expressions", "engine-env-secrets-to-engine-config", "serena-tools-to-shared-import", "workflow-run-branches-default", @@ -153,6 +154,7 @@ func expectedCodemodOrder() []string { "engine-steps-to-top-level", "engine-max-runs-to-top-level", "steps-run-secrets-to-env", + "auto-hoist-run-expressions", "engine-env-secrets-to-engine-config", "assign-to-agent-default-agent-to-name", "playwright-allowed-domains-migration", From 8fd788dd88c739a2499f1b87d5ac63bf08603c6b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 02:54:27 +0000 Subject: [PATCH 04/10] Rename regex variable for naming consistency (autoHoistSimpleExprRe) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_auto_hoist_run_expressions.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cli/codemod_auto_hoist_run_expressions.go b/pkg/cli/codemod_auto_hoist_run_expressions.go index 170234bc0e..74b3abeed8 100644 --- a/pkg/cli/codemod_auto_hoist_run_expressions.go +++ b/pkg/cli/codemod_auto_hoist_run_expressions.go @@ -10,12 +10,12 @@ import ( var autoHoistRunExpressionsLog = logger.New("cli:codemod_auto_hoist_run_expressions") -// autoHoistSimpleExprBodyRe matches simple JavaScript property-access chains such as +// autoHoistSimpleExprRe matches simple JavaScript property-access chains such as // "github.token", "env.FOO", "inputs.my-input", "steps.my-step.outputs.result". // Only word characters and hyphens separated by dots are allowed; any expression // containing spaces, operators, or other punctuation falls through to the hash-based // name generator. -var autoHoistSimpleExprBodyRe = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_-]*(\.[a-zA-Z_][a-zA-Z0-9_-]*)*$`) +var autoHoistSimpleExprRe = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_-]*(\.[a-zA-Z_][a-zA-Z0-9_-]*)*$`) // getAutoHoistRunExpressionsCodemod creates a codemod that hoists ALL ${{ ... }} // expressions from run: blocks into step-level env bindings. @@ -362,7 +362,7 @@ func replaceAutoHoistExpressionRefs(line string, shellIsPowerShell bool) (string // // secrets.TOKEN || '' → EXPR_<8-char-fnv32-hex> func mapAutoHoistExpressionToEnvBinding(body string) (string, string, bool) { - if autoHoistSimpleExprBodyRe.MatchString(body) { + if autoHoistSimpleExprRe.MatchString(body) { replacer := strings.NewReplacer(".", "_", "-", "_") name := "EXPR_" + strings.ToUpper(replacer.Replace(body)) return name, fmt.Sprintf("${{ %s }}", body), true From 94ee92de74f1927de86842a86642777a28b2fe21 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 04:11:00 +0000 Subject: [PATCH 05/10] Merge auto-hoist-run-expressions into steps-run-secrets-to-env Extend the existing codemod to handle ALL ${{ ... }} expressions in run: blocks, not just secrets.*, env.*, and github.token. Non-legacy expressions receive EXPR_* names. PowerShell steps (shell: pwsh / powershell) get $env:VARNAME syntax. Remove the separate auto-hoist-run-expressions codemod. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...inor-auto-hoist-run-expressions-codemod.md | 2 +- .../docs/reference/gh-aw-as-mcp-server.md | 2 +- docs/src/content/docs/setup/cli.md | 3 +- pkg/cli/codemod_auto_hoist_run_expressions.go | 374 -------------- ...codemod_auto_hoist_run_expressions_test.go | 460 ------------------ pkg/cli/codemod_steps_run_secrets_env.go | 54 +- pkg/cli/codemod_steps_run_secrets_env_test.go | 171 +++++++ pkg/cli/fix_codemods.go | 3 +- pkg/cli/fix_codemods_test.go | 2 - 9 files changed, 220 insertions(+), 851 deletions(-) delete mode 100644 pkg/cli/codemod_auto_hoist_run_expressions.go delete mode 100644 pkg/cli/codemod_auto_hoist_run_expressions_test.go diff --git a/.changeset/minor-auto-hoist-run-expressions-codemod.md b/.changeset/minor-auto-hoist-run-expressions-codemod.md index ef77f07201..b11f86fc7f 100644 --- a/.changeset/minor-auto-hoist-run-expressions-codemod.md +++ b/.changeset/minor-auto-hoist-run-expressions-codemod.md @@ -2,4 +2,4 @@ "gh-aw": minor --- -Add `auto-hoist-run-expressions` codemod that rewrites **all** `${{ ... }}` expressions inside `run:` blocks to `$VARNAME` shell references and inserts `EXPR_*` step-level `env:` bindings. This closes the gap left by `steps-run-secrets-to-env` (which only handles `secrets.*`, `env.*`, and `github.token`) by covering arbitrary expressions such as `github.repository`, `github.event.issue.title`, `inputs.*`, and `steps.*.outputs.*`. PowerShell steps (`shell: pwsh` / `shell: powershell`) receive `$env:VARNAME` syntax instead of `$VARNAME`. +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/docs/src/content/docs/reference/gh-aw-as-mcp-server.md b/docs/src/content/docs/reference/gh-aw-as-mcp-server.md index 70d2b3eb40..5a85b07b19 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`, `auto-hoist-run-expressions`. +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 00aab2d1ab..7c4d2a759e 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -244,8 +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. -- `auto-hoist-run-expressions` — rewrites **all** `${{ ... }}` expressions in step `run:` scripts to `$VARNAME` references (or `$env:VARNAME` for PowerShell steps) and adds `EXPR_*` step-level `env` bindings. Catches any expression not already handled by `steps-run-secrets-to-env`, preventing the "compiler regression detected" hard error. +- `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_auto_hoist_run_expressions.go b/pkg/cli/codemod_auto_hoist_run_expressions.go deleted file mode 100644 index 74b3abeed8..0000000000 --- a/pkg/cli/codemod_auto_hoist_run_expressions.go +++ /dev/null @@ -1,374 +0,0 @@ -package cli - -import ( - "fmt" - "regexp" - "strings" - - "github.com/github/gh-aw/pkg/logger" -) - -var autoHoistRunExpressionsLog = logger.New("cli:codemod_auto_hoist_run_expressions") - -// autoHoistSimpleExprRe matches simple JavaScript property-access chains such as -// "github.token", "env.FOO", "inputs.my-input", "steps.my-step.outputs.result". -// Only word characters and hyphens separated by dots are allowed; any expression -// containing spaces, operators, or other punctuation falls through to the hash-based -// name generator. -var autoHoistSimpleExprRe = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_-]*(\.[a-zA-Z_][a-zA-Z0-9_-]*)*$`) - -// getAutoHoistRunExpressionsCodemod creates a codemod that hoists ALL ${{ ... }} -// expressions from run: blocks into step-level env bindings. -// -// Unlike steps-run-secrets-to-env (which only handles secrets.*, env.*, and -// github.token), this codemod covers every expression that would otherwise trigger -// the "compiler regression detected" hard error. -// -// Naming convention: EXPR_ + sanitised uppercase expression body. -// - Simple chains: github.token → EXPR_GITHUB_TOKEN -// - Complex expressions: ${{ secrets.TOKEN || '' }} → EXPR_<8-char-hash> -// -// PowerShell awareness: steps with shell: pwsh or shell: powershell receive -// $env:VARNAME references in the run script instead of $VARNAME. -func getAutoHoistRunExpressionsCodemod() Codemod { - return Codemod{ - ID: "auto-hoist-run-expressions", - Name: "Auto-hoist run-block expressions to env bindings", - Description: "Rewrites all ${{ ... }} expressions in run: scripts to $VARNAME references (or $env:VARNAME for PowerShell steps) and adds EXPR_* step-level env bindings.", - IntroducedIn: "1.0.45", - Apply: func(content string, frontmatter map[string]any) (string, bool, error) { - sections := []string{"pre-steps", "steps", "post-steps", "pre-agent-steps"} - hasTargetSection := false - for _, section := range sections { - if _, ok := frontmatter[section]; ok { - hasTargetSection = true - break - } - } - if !hasTargetSection { - return content, false, nil - } - - newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) { - modified := false - current := lines - for _, section := range sections { - var sectionChanged bool - current, sectionChanged = transformSectionAutoHoistExpressions(current, section) - modified = modified || sectionChanged - } - return current, modified - }) - if applied { - autoHoistRunExpressionsLog.Print("Auto-hoisted inline run expressions to step-level env bindings") - } - return newContent, applied, err - }, - } -} - -func transformSectionAutoHoistExpressions(lines []string, sectionName string) ([]string, bool) { - sectionStart := -1 - sectionIndent := "" - for i, line := range lines { - trimmed := strings.TrimSpace(line) - if isTopLevelKey(line) && strings.HasPrefix(trimmed, sectionName+":") { - sectionStart = i - sectionIndent = getIndentation(line) - break - } - } - if sectionStart == -1 { - return lines, false - } - - sectionEnd := len(lines) - 1 - for i := sectionStart + 1; i < len(lines); i++ { - trimmed := strings.TrimSpace(lines[i]) - if len(trimmed) == 0 || strings.HasPrefix(trimmed, "#") { - continue - } - if len(getIndentation(lines[i])) <= len(sectionIndent) { - sectionEnd = i - 1 - break - } - } - - autoHoistRunExpressionsLog.Printf("Transforming section '%s': lines %d-%d", sectionName, sectionStart, sectionEnd) - - sectionLines := lines[sectionStart : sectionEnd+1] - updatedSection, changed := transformStepsWithinSectionAutoHoist(sectionLines, sectionIndent) - if !changed { - return lines, false - } - - result := make([]string, 0, len(lines)-(len(sectionLines)-len(updatedSection))) - result = append(result, lines[:sectionStart]...) - result = append(result, updatedSection...) - result = append(result, lines[sectionEnd+1:]...) - return result, true -} - -func transformStepsWithinSectionAutoHoist(sectionLines []string, sectionIndent string) ([]string, bool) { - result := make([]string, 0, len(sectionLines)) - modified := false - - for i := 0; i < len(sectionLines); { - line := sectionLines[i] - trimmed := strings.TrimSpace(line) - indent := getIndentation(line) - - if strings.HasPrefix(trimmed, "- ") && len(indent) > len(sectionIndent) { - stepStart := i - stepIndent := indent - stepEnd := len(sectionLines) - 1 - for j := i + 1; j < len(sectionLines); j++ { - t := strings.TrimSpace(sectionLines[j]) - if len(t) == 0 { - continue - } - jIndent := getIndentation(sectionLines[j]) - if strings.HasPrefix(t, "- ") && len(jIndent) == len(stepIndent) { - stepEnd = j - 1 - break - } - } - - chunk := append([]string(nil), sectionLines[stepStart:stepEnd+1]...) - updatedChunk, changed := rewriteStepAutoHoistExpressions(chunk, stepIndent) - modified = modified || changed - result = append(result, updatedChunk...) - i = stepEnd + 1 - continue - } - - result = append(result, line) - i++ - } - - return result, modified -} - -func rewriteStepAutoHoistExpressions(stepLines []string, stepIndent string) ([]string, bool) { - modified := false - seen := make(map[string]bool) - orderedBindings := make([]string, 0) - bindingExprs := make(map[string]string) - firstRunLine := -1 - envStart := -1 - envEnd := -1 - envIndent := "" - var envKeyIndentLen int - existingEnvKeys := make(map[string]bool) - shellIsPowerShell := false - - // First pass: detect shell type so PowerShell steps get $env:VARNAME syntax. - for _, line := range stepLines { - trimmed := strings.TrimSpace(line) - indent := getIndentation(line) - shellMatch, shellValue, _ := parseStepKeyLine(trimmed, indent, stepIndent, "shell") - if shellMatch { - v := strings.ToLower(strings.TrimSpace(shellValue)) - if v == "pwsh" || v == "powershell" { - shellIsPowerShell = true - } - break - } - } - - // Second pass: scan env block and rewrite run block. - for i := 0; i < len(stepLines); i++ { - line := stepLines[i] - trimmed := strings.TrimSpace(line) - indent := getIndentation(line) - - // Track the existing env: block so new bindings can be appended to it. - envMatchKey, envValue, currentEnvKeyIndentLen := parseStepKeyLine(trimmed, indent, stepIndent, "env") - if envMatchKey && envValue == "" { - envStart = i - envIndent = indent - envKeyIndentLen = currentEnvKeyIndentLen - envEnd = i - for j := i + 1; j < len(stepLines); j++ { - t := strings.TrimSpace(stepLines[j]) - if len(t) == 0 { - envEnd = j - continue - } - if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= envKeyIndentLen { - break - } - envEnd = j - key := parseYAMLMapKey(t) - if key != "" { - existingEnvKeys[key] = true - } - } - } - - runMatch, runValue, runKeyIndentLen := parseStepKeyLine(trimmed, indent, stepIndent, "run") - if !runMatch { - continue - } - if firstRunLine == -1 { - firstRunLine = i - } - - if runValue == "|" || runValue == "|-" || runValue == ">" || runValue == ">-" { - for j := i + 1; j < len(stepLines); j++ { - t := strings.TrimSpace(stepLines[j]) - if len(t) == 0 { - continue - } - if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= runKeyIndentLen { - break - } - updatedLine, bindings := replaceAutoHoistExpressionRefs(stepLines[j], shellIsPowerShell) - if len(bindings) > 0 { - stepLines[j] = updatedLine - modified = true - } - for _, binding := range bindings { - if !seen[binding.Name] { - seen[binding.Name] = true - orderedBindings = append(orderedBindings, binding.Name) - bindingExprs[binding.Name] = binding.Expression - } - } - } - continue - } - - newLine, bindings := replaceAutoHoistExpressionRefs(line, shellIsPowerShell) - if len(bindings) > 0 { - stepLines[i] = newLine - modified = true - } - for _, binding := range bindings { - if !seen[binding.Name] { - seen[binding.Name] = true - orderedBindings = append(orderedBindings, binding.Name) - bindingExprs[binding.Name] = binding.Expression - } - } - } - - if len(orderedBindings) == 0 { - return stepLines, modified - } - - autoHoistRunExpressionsLog.Printf("Found %d unique run expression references in step run commands", len(orderedBindings)) - - missingBindings := make([]string, 0, len(orderedBindings)) - for _, name := range orderedBindings { - if !existingEnvKeys[name] { - missingBindings = append(missingBindings, name) - } - } - if len(missingBindings) == 0 { - return stepLines, true - } - - autoHoistRunExpressionsLog.Printf("Adding env bindings for %d missing expressions: %v", len(missingBindings), missingBindings) - - if envStart != -1 { - insertAt := envEnd + 1 - envValueIndent := envIndent + " " - insertLines := make([]string, 0, len(missingBindings)) - for _, name := range missingBindings { - insertLines = append(insertLines, fmt.Sprintf("%s%s: %s", envValueIndent, name, bindingExprs[name])) - } - stepLines = append(stepLines[:insertAt], append(insertLines, stepLines[insertAt:]...)...) - return stepLines, true - } - - if firstRunLine == -1 { - return stepLines, modified - } - - insertIndent := stepIndent + " " - insertLines := []string{insertIndent + "env:"} - for _, name := range missingBindings { - insertLines = append(insertLines, fmt.Sprintf("%s %s: %s", insertIndent, name, bindingExprs[name])) - } - stepLines = append(stepLines[:firstRunLine], append(insertLines, stepLines[firstRunLine:]...)...) - return stepLines, true -} - -// replaceAutoHoistExpressionRefs replaces every ${{ ... }} expression in line -// with either $VARNAME (bash) or $env:VARNAME (PowerShell) and returns the -// corresponding env bindings to add to the step's env: block. -func replaceAutoHoistExpressionRefs(line string, shellIsPowerShell bool) (string, []stepExpressionBinding) { - matches := stepsAnyExprRe.FindAllStringSubmatchIndex(line, -1) - if len(matches) == 0 { - return line, nil - } - - var result strings.Builder - last := 0 - seen := make(map[string]bool) - ordered := make([]stepExpressionBinding, 0, len(matches)) - - for _, match := range matches { - if len(match) < 4 { - continue - } - fullStart, fullEnd := match[0], match[1] - bodyStart, bodyEnd := match[2], match[3] - fullExpression := line[fullStart:fullEnd] - body := strings.TrimSpace(line[bodyStart:bodyEnd]) - - result.WriteString(line[last:fullStart]) - - envName, canonicalExpression, ok := mapAutoHoistExpressionToEnvBinding(body) - if !ok { - result.WriteString(fullExpression) - last = fullEnd - continue - } - - if shellIsPowerShell { - result.WriteString("$env:" + envName) - } else { - result.WriteString("$" + envName) - } - if !seen[envName] { - seen[envName] = true - ordered = append(ordered, stepExpressionBinding{ - Name: envName, - Expression: canonicalExpression, - }) - } - last = fullEnd - } - - result.WriteString(line[last:]) - return result.String(), ordered -} - -// mapAutoHoistExpressionToEnvBinding maps any ${{ body }} expression to a -// deterministic EXPR_* environment variable name and the canonical expression -// string to bind as its value. -// -// Simple property-access chains (e.g. "github.token", "env.FOO", -// "inputs.my-input") produce pretty names: -// -// github.token → EXPR_GITHUB_TOKEN -// env.FOO → EXPR_ENV_FOO -// inputs.my-input → EXPR_INPUTS_MY_INPUT -// -// Complex expressions (anything containing spaces, operators, function calls, -// etc.) fall back to a hash-based name to guarantee uniqueness: -// -// secrets.TOKEN || '' → EXPR_<8-char-fnv32-hex> -func mapAutoHoistExpressionToEnvBinding(body string) (string, string, bool) { - if autoHoistSimpleExprRe.MatchString(body) { - replacer := strings.NewReplacer(".", "_", "-", "_") - name := "EXPR_" + strings.ToUpper(replacer.Replace(body)) - return name, fmt.Sprintf("${{ %s }}", body), true - } - // Complex expression: include a hash suffix to prevent name collisions - // between different complex expressions in the same step. - name := hashedBindingName("EXPR", body) - return name, fmt.Sprintf("${{ %s }}", body), true -} diff --git a/pkg/cli/codemod_auto_hoist_run_expressions_test.go b/pkg/cli/codemod_auto_hoist_run_expressions_test.go deleted file mode 100644 index 979ef7a71f..0000000000 --- a/pkg/cli/codemod_auto_hoist_run_expressions_test.go +++ /dev/null @@ -1,460 +0,0 @@ -//go:build !integration - -package cli - -import ( - "regexp" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestAutoHoistRunExpressionsCodemod(t *testing.T) { - codemod := getAutoHoistRunExpressionsCodemod() - - t.Run("hoists github.token from block run to env binding", func(t *testing.T) { - content := `--- -on: push -steps: - - name: Capture token - run: | - echo "GH_TOKEN=${{ github.token }}" >> "$GITHUB_OUTPUT" ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "name": "Capture token", - "run": "echo \"GH_TOKEN=${{ github.token }}\" >> \"$GITHUB_OUTPUT\"", - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "EXPR_GITHUB_TOKEN: ${{ github.token }}", "github.token should be bound in env") - assert.Contains(t, result, `echo "GH_TOKEN=$EXPR_GITHUB_TOKEN"`, "run should use hoisted env var") - assert.NotContains(t, result, `echo "GH_TOKEN=${{ github.token }}"`, "inline expression should be removed from run block") - }) - - t.Run("hoists github.repository from inline run to 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, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "EXPR_GITHUB_REPOSITORY: ${{ github.repository }}", "github.repository should be bound in env") - assert.Contains(t, result, "run: echo $EXPR_GITHUB_REPOSITORY", "run should use hoisted env var") - }) - - t.Run("hoists inputs expression from run to 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, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "EXPR_INPUTS_MY_INPUT: ${{ inputs.my-input }}", "inputs.my-input should be bound in env") - assert.Contains(t, result, "run: echo $EXPR_INPUTS_MY_INPUT", "run should use hoisted env var") - }) - - t.Run("hoists steps output expression from run to 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, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "EXPR_STEPS_MY_STEP_OUTPUTS_RESULT: ${{ steps.my-step.outputs.result }}", "steps output should be bound in env") - assert.Contains(t, result, "run: echo $EXPR_STEPS_MY_STEP_OUTPUTS_RESULT", "run should use hoisted env var") - }) - - t.Run("uses hash-based name for complex expressions", func(t *testing.T) { - content := `--- -on: push -steps: - - run: echo "${{ github.token || secrets.PAT }}" ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "run": `echo "${{ github.token || secrets.PAT }}"`, - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "${{ github.token || secrets.PAT }}", "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 binding should be created for the complex expression") - }) - - t.Run("deduplicates identical expressions in one step", func(t *testing.T) { - content := `--- -on: push -steps: - - run: | - echo "${{ github.token }}" - echo "${{ github.token }}" ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "run": "echo \"${{ github.token }}\"\necho \"${{ github.token }}\"", - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Equal(t, 1, strings.Count(result, "EXPR_GITHUB_TOKEN: ${{ github.token }}"), "binding should appear exactly once") - assert.Equal(t, 2, strings.Count(result, "$EXPR_GITHUB_TOKEN"), "both occurrences in run should be replaced") - }) - - t.Run("handles multiple distinct expressions in one step", func(t *testing.T) { - content := `--- -on: push -steps: - - run: | - echo "${{ github.token }}" - echo "${{ github.repository }}" ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "run": "echo \"${{ github.token }}\"\necho \"${{ github.repository }}\"", - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "EXPR_GITHUB_TOKEN: ${{ github.token }}", "github.token binding should be added") - assert.Contains(t, result, "EXPR_GITHUB_REPOSITORY: ${{ github.repository }}", "github.repository binding should be added") - assert.Contains(t, result, `echo "$EXPR_GITHUB_TOKEN"`, "first run line should use hoisted token var") - assert.Contains(t, result, `echo "$EXPR_GITHUB_REPOSITORY"`, "second run line should use hoisted repo var") - }) - - t.Run("appends missing bindings to existing env block", func(t *testing.T) { - content := `--- -on: push -steps: - - name: Run checks - env: - FOO: bar - run: echo ${{ github.actor }} ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "name": "Run checks", - "env": map[string]any{"FOO": "bar"}, - "run": "echo ${{ github.actor }}", - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "FOO: bar", "existing env entry should be preserved") - assert.Contains(t, result, "EXPR_GITHUB_ACTOR: ${{ github.actor }}", "new binding should be appended") - assert.Contains(t, result, "run: echo $EXPR_GITHUB_ACTOR", "run should reference the new env var") - }) - - t.Run("does not duplicate pre-existing EXPR_ bindings", func(t *testing.T) { - content := `--- -on: push -steps: - - env: - EXPR_GITHUB_TOKEN: ${{ github.token }} - run: echo "${{ github.token }}" ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "env": map[string]any{ - "EXPR_GITHUB_TOKEN": "${{ github.token }}", - }, - "run": `echo "${{ github.token }}"`, - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should still rewrite run expression reference") - assert.Equal(t, 1, strings.Count(result, "EXPR_GITHUB_TOKEN: ${{ github.token }}"), "existing binding should not be duplicated") - assert.Contains(t, result, `run: echo "$EXPR_GITHUB_TOKEN"`, "run should use the existing binding") - }) - - t.Run("uses $env:VARNAME for PowerShell steps", func(t *testing.T) { - content := `--- -on: push -steps: - - name: PS step - shell: pwsh - run: | - Write-Output "${{ github.token }}" ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "name": "PS step", - "shell": "pwsh", - "run": `Write-Output "${{ github.token }}"`, - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "EXPR_GITHUB_TOKEN: ${{ github.token }}", "env binding should be added") - assert.Contains(t, result, `Write-Output "$env:EXPR_GITHUB_TOKEN"`, "PowerShell step should use $env:VARNAME syntax") - }) - - t.Run("uses $env:VARNAME for powershell shell variant", 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, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "EXPR_GITHUB_ACTOR: ${{ github.actor }}", "env binding should be added") - assert.Contains(t, result, "run: Write-Output $env:EXPR_GITHUB_ACTOR", "powershell step should use $env:VARNAME syntax") - }) - - t.Run("bash step uses $VARNAME not $env:VARNAME", 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, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "run: echo $EXPR_GITHUB_ACTOR", "bash step should use $VARNAME syntax") - assert.NotContains(t, result, "$env:EXPR_GITHUB_ACTOR", "bash step should not use $env: syntax") - }) - - t.Run("supports pre-steps section", func(t *testing.T) { - content := `--- -on: pull_request -pre-steps: - - name: Pre check - run: echo ${{ github.event.pull_request.number }} ---- -` - frontmatter := map[string]any{ - "on": "pull_request", - "pre-steps": []any{ - map[string]any{ - "name": "Pre check", - "run": "echo ${{ github.event.pull_request.number }}", - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "run: echo $EXPR_GITHUB_EVENT_PULL_REQUEST_NUMBER", "pre-steps run should be rewritten") - assert.Contains(t, result, "EXPR_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }}", "binding should be added") - }) - - t.Run("supports post-steps section", func(t *testing.T) { - content := `--- -on: push -post-steps: - - name: Post - run: echo ${{ github.run_number }} ---- -` - frontmatter := map[string]any{ - "on": "push", - "post-steps": []any{ - map[string]any{ - "name": "Post", - "run": "echo ${{ github.run_number }}", - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "EXPR_GITHUB_RUN_NUMBER: ${{ github.run_number }}", "post-steps binding should be added") - }) - - t.Run("no-op when no inline run expressions are present", func(t *testing.T) { - content := `--- -on: push -steps: - - name: Safe - run: echo "hello" ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "name": "Safe", - "run": `echo "hello"`, - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should not error in no-op case") - assert.False(t, applied, "codemod should not apply") - assert.Equal(t, content, result, "content should be unchanged") - }) - - t.Run("no-op when workflow has no steps sections", func(t *testing.T) { - content := `--- -on: push ---- -` - frontmatter := map[string]any{ - "on": "push", - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should not error with no steps sections") - assert.False(t, applied, "codemod should not apply") - assert.Equal(t, content, result, "content should be unchanged") - }) - - t.Run("two distinct complex expressions get different hash names", func(t *testing.T) { - content := `--- -on: push -steps: - - run: echo "${{ github.token || '' }} ${{ github.token || 'fallback' }}" ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "run": `echo "${{ github.token || '' }} ${{ github.token || 'fallback' }}"`, - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - envBindings := regexp.MustCompile(`EXPR_[0-9a-f]{8}:`).FindAllString(result, -1) - assert.Len(t, envBindings, 2, "two distinct complex expressions should produce two distinct hashed bindings") - assert.NotEqual(t, envBindings[0], envBindings[1], "hashed names should differ for different expressions") - }) - - t.Run("list-item-inline run key is hoisted", func(t *testing.T) { - content := `--- -on: push -steps: - - run: echo ${{ github.sha }} ---- -` - frontmatter := map[string]any{ - "on": "push", - "steps": []any{ - map[string]any{ - "run": "echo ${{ github.sha }}", - }, - }, - } - - result, applied, err := codemod.Apply(content, frontmatter) - require.NoError(t, err, "codemod should apply cleanly") - assert.True(t, applied, "codemod should apply") - assert.Contains(t, result, "run: echo $EXPR_GITHUB_SHA", "inline run should be rewritten") - assert.Contains(t, result, "EXPR_GITHUB_SHA: ${{ github.sha }}", "inline run should get env binding") - }) -} diff --git a/pkg/cli/codemod_steps_run_secrets_env.go b/pkg/cli/codemod_steps_run_secrets_env.go index 239d2f2ca3..c238a0563e 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`) + // stepsSimpleExprRe matches simple JavaScript 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. + stepsSimpleExprRe = 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.", 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,21 @@ 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. + shellIsPowerShell := false + for _, line := range stepLines { + trimmed := strings.TrimSpace(line) + indent := getIndentation(line) + 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 +222,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) if len(bindings) > 0 { stepLines[j] = updatedLine modified = true @@ -214,7 +238,7 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string continue } - newLine, bindings := replaceStepExpressionRefs(line) + newLine, bindings := replaceStepExpressionRefs(line, shellIsPowerShell) if len(bindings) > 0 { stepLines[i] = newLine modified = true @@ -275,7 +299,7 @@ type stepExpressionBinding struct { Expression string } -func replaceStepExpressionRefs(line string) (string, []stepExpressionBinding) { +func replaceStepExpressionRefs(line string, shellIsPowerShell bool) (string, []stepExpressionBinding) { matches := stepsAnyExprRe.FindAllStringSubmatchIndex(line, -1) if len(matches) == 0 { return line, nil @@ -304,7 +328,11 @@ func replaceStepExpressionRefs(line string) (string, []stepExpressionBinding) { continue } - result.WriteString("$" + envName) + if shellIsPowerShell { + result.WriteString("$env:" + envName) + } else { + result.WriteString("$" + envName) + } if !seen[envName] { seen[envName] = true ordered = append(ordered, stepExpressionBinding{ @@ -346,7 +374,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 stepsSimpleExprRe.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_test.go b/pkg/cli/codemod_steps_run_secrets_env_test.go index 1a429a6d77..ec5c179a50 100644 --- a/pkg/cli/codemod_steps_run_secrets_env_test.go +++ b/pkg/cli/codemod_steps_run_secrets_env_test.go @@ -369,4 +369,175 @@ 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") + }) } diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index b595075f6b..b46cbf7664 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -46,8 +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 - getAutoHoistRunExpressionsCodemod(), // Hoist ALL ${{ ... }} run-block expressions to EXPR_* 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 diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 760256e079..1b90df1d68 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -85,7 +85,6 @@ func TestGetAllCodemods_ContainsExpectedCodemods(t *testing.T) { "rate-limit-to-user-rate-limit", "engine-max-runs-to-top-level", "steps-run-secrets-to-env", - "auto-hoist-run-expressions", "engine-env-secrets-to-engine-config", "serena-tools-to-shared-import", "workflow-run-branches-default", @@ -154,7 +153,6 @@ func expectedCodemodOrder() []string { "engine-steps-to-top-level", "engine-max-runs-to-top-level", "steps-run-secrets-to-env", - "auto-hoist-run-expressions", "engine-env-secrets-to-engine-config", "assign-to-agent-default-agent-to-name", "playwright-allowed-domains-migration", From 0c01a36da01ceeb4f752e94dc19946a7a0c7e804 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 04:12:10 +0000 Subject: [PATCH 06/10] =?UTF-8?q?Rename=20stepsSimpleExprRe=20=E2=86=92=20?= =?UTF-8?q?stepsGenericExprRe,=20fix=20comment=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_steps_run_secrets_env.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cli/codemod_steps_run_secrets_env.go b/pkg/cli/codemod_steps_run_secrets_env.go index c238a0563e..2289f6433e 100644 --- a/pkg/cli/codemod_steps_run_secrets_env.go +++ b/pkg/cli/codemod_steps_run_secrets_env.go @@ -18,12 +18,12 @@ 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`) - // stepsSimpleExprRe matches simple JavaScript property-access chains such as + // 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. - stepsSimpleExprRe = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_-]*(\.[a-zA-Z_][a-zA-Z0-9_-]*)*$`) + stepsGenericExprRe = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_-]*(\.[a-zA-Z_][a-zA-Z0-9_-]*)*$`) ) // getStepsRunSecretsToEnvCodemod creates a codemod that moves all ${{ ... }} @@ -375,7 +375,7 @@ func mapRunExpressionToEnvBinding(body string) (string, string, bool) { } // Catch-all: hoist any remaining expression using EXPR_ naming. - if stepsSimpleExprRe.MatchString(body) { + if stepsGenericExprRe.MatchString(body) { replacer := strings.NewReplacer(".", "_", "-", "_") name := "EXPR_" + strings.ToUpper(replacer.Replace(body)) return name, fmt.Sprintf("${{ %s }}", body), true From 7c961938e9c049affdabff48185835b5a629d9c6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 04:44:37 +0000 Subject: [PATCH 07/10] Extend fuzz tests for EXPR_ catch-all and PowerShell paths - Update FuzzStepsRunSecretsToEnvCodemod: check no ${{ ... }} remains in run line (was checking only specific patterns) - Add FuzzStepsRunSecretsToEnvCodemodExpr: covers EXPR_* naming for generic property-access chains (github.X, inputs.X, etc.) - Add FuzzStepsRunSecretsToEnvCodemodPowerShell: verifies $env:VARNAME output for pwsh/powershell steps - Add sanitizeHoistPropertySegment helper for generating valid property-chain segments Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...codemod_steps_run_secrets_env_fuzz_test.go | 158 +++++++++++++++++- 1 file changed, 157 insertions(+), 1 deletion(-) 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 85afecfc28..0313a83c6d 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,38 @@ 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 +} + From c98b9c18fabcc10fc7a7380cdc8f38cbfc00ee14 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 04:45:21 +0000 Subject: [PATCH 08/10] Fix operator precedence in sanitizeHoistPropertySegment Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0313a83c6d..ba5ff8dad3 100644 --- a/pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go +++ b/pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go @@ -332,7 +332,7 @@ func sanitizeHoistPropertySegment(raw string) string { return "prop" } // Ensure the segment starts with a letter or underscore. - if s[0] >= '0' && s[0] <= '9' || s[0] == '-' { + if (s[0] >= '0' && s[0] <= '9') || s[0] == '-' { return "p" + s } return s From cb41c6fc800326da9f2631eab8415b521c084323 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 13:58:16 +0000 Subject: [PATCH 09/10] Fix go fmt formatting in codemod_steps_run_secrets_env_fuzz_test.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 ba5ff8dad3..2c294e704c 100644 --- a/pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go +++ b/pkg/cli/codemod_steps_run_secrets_env_fuzz_test.go @@ -99,8 +99,8 @@ func FuzzStepsRunSecretsToEnvCodemodExpr(f *testing.F) { content := strings.Join(lines, "\n") + "\n" frontmatter := map[string]any{ - "on": "push", - section: []any{map[string]any{"name": "fuzz", "run": run}}, + "on": "push", + section: []any{map[string]any{"name": "fuzz", "run": run}}, "workflow": "fuzz", } @@ -337,4 +337,3 @@ func sanitizeHoistPropertySegment(raw string) string { } return s } - From 26d68bdee232b3e7c07c567afaad48b8b125f57c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 14:28:03 +0000 Subject: [PATCH 10/10] Address reviewer feedback: fix shell detection, EXPR_ collision, and add tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_steps_run_secrets_env.go | 55 ++++++++++++++-- pkg/cli/codemod_steps_run_secrets_env_test.go | 64 +++++++++++++++++++ 2 files changed, 112 insertions(+), 7 deletions(-) diff --git a/pkg/cli/codemod_steps_run_secrets_env.go b/pkg/cli/codemod_steps_run_secrets_env.go index 2289f6433e..ca95743ac7 100644 --- a/pkg/cli/codemod_steps_run_secrets_env.go +++ b/pkg/cli/codemod_steps_run_secrets_env.go @@ -35,7 +35,7 @@ func getStepsRunSecretsToEnvCodemod() Codemod { return Codemod{ ID: "steps-run-secrets-to-env", 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.", + 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"} @@ -163,10 +163,19 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string 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)) @@ -222,7 +231,7 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= runKeyIndentLen { break } - updatedLine, bindings := replaceStepExpressionRefs(stepLines[j], shellIsPowerShell) + updatedLine, bindings := replaceStepExpressionRefs(stepLines[j], shellIsPowerShell, bindingExprs) if len(bindings) > 0 { stepLines[j] = updatedLine modified = true @@ -238,7 +247,7 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string continue } - newLine, bindings := replaceStepExpressionRefs(line, shellIsPowerShell) + newLine, bindings := replaceStepExpressionRefs(line, shellIsPowerShell, bindingExprs) if len(bindings) > 0 { stepLines[i] = newLine modified = true @@ -299,7 +308,7 @@ type stepExpressionBinding struct { Expression string } -func replaceStepExpressionRefs(line string, shellIsPowerShell bool) (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 @@ -307,7 +316,15 @@ func replaceStepExpressionRefs(line string, shellIsPowerShell bool) (string, []s 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 { @@ -321,6 +338,17 @@ func replaceStepExpressionRefs(line string, shellIsPowerShell bool) (string, []s 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) @@ -328,13 +356,26 @@ func replaceStepExpressionRefs(line string, shellIsPowerShell bool) (string, []s continue } + // 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 !seen[envName] { - seen[envName] = true + if !registeredNames[envName] { + registeredNames[envName] = true ordered = append(ordered, stepExpressionBinding{ Name: envName, Expression: canonicalExpression, diff --git a/pkg/cli/codemod_steps_run_secrets_env_test.go b/pkg/cli/codemod_steps_run_secrets_env_test.go index ec5c179a50..2b005a1b8e 100644 --- a/pkg/cli/codemod_steps_run_secrets_env_test.go +++ b/pkg/cli/codemod_steps_run_secrets_env_test.go @@ -540,4 +540,68 @@ steps: 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") + }) }