From 0a76345a55aa857831bb1e2ecc2ac77a5ac7160f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:33:57 +0000 Subject: [PATCH 1/3] Initial plan From a179f046a737f37b5e8dcd890ad064561e63d2a6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:45:06 +0000 Subject: [PATCH 2/3] fix: skip bash comment lines in run block expression scanning Expressions inside bash comment lines (lines whose first non-whitespace character is `#`) inside `run: |` blocks are documentation-only and must not trigger env-binding extraction. Add a skip check in `rewriteStepRunSecretsToEnv` and two regression tests. Closes #38681 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_steps_run_secrets_env.go | 5 ++ pkg/cli/codemod_steps_run_secrets_env_test.go | 65 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/pkg/cli/codemod_steps_run_secrets_env.go b/pkg/cli/codemod_steps_run_secrets_env.go index 1d9a5adc036..90585ea356f 100644 --- a/pkg/cli/codemod_steps_run_secrets_env.go +++ b/pkg/cli/codemod_steps_run_secrets_env.go @@ -231,6 +231,11 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= runKeyIndentLen { break } + // Skip bash comment lines – expressions inside comments are + // documentation-only and must not generate env bindings. + if strings.HasPrefix(t, "#") { + continue + } updatedLine, bindings := replaceStepExpressionRefs(stepLines[j], shellIsPowerShell, bindingExprs) if len(bindings) > 0 { stepLines[j] = updatedLine diff --git a/pkg/cli/codemod_steps_run_secrets_env_test.go b/pkg/cli/codemod_steps_run_secrets_env_test.go index 2f34e0fdc6e..39cd0c781e4 100644 --- a/pkg/cli/codemod_steps_run_secrets_env_test.go +++ b/pkg/cli/codemod_steps_run_secrets_env_test.go @@ -569,6 +569,71 @@ steps: assert.NotContains(t, result, "$env:EXPR_GITHUB_ACTOR", "bash step must not use $env:VARNAME") }) + t.Run("ignores expressions inside bash comment lines in run block", func(t *testing.T) { + // Expressions inside bash comments are documentation-only and must not + // generate env bindings or be rewritten. + content := `--- +on: workflow_dispatch +steps: + - name: Use parsed value + env: + VALUE: ${{ steps.parse.outputs.value }} + run: | + echo "Got: $VALUE" + # Note: the prompt placeholders ${{ steps.parse.outputs.* }} resolve to + # empty strings because they're evaluated in a different context. +--- +` + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "steps": []any{ + map[string]any{ + "name": "Use parsed value", + "env": map[string]any{ + "VALUE": "${{ steps.parse.outputs.value }}", + }, + "run": "echo \"Got: $VALUE\"\n# Note: the prompt placeholders ${{ steps.parse.outputs.* }} resolve to\n# empty strings because they're evaluated in a different context.", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.False(t, applied, "codemod should not modify content when only expression is in a bash comment") + assert.Equal(t, content, result, "content should be unchanged") + assert.NotContains(t, result, "EXPR_", "no EXPR_ bindings should be generated for comment-only expressions") + assert.Contains(t, result, "${{ steps.parse.outputs.* }}", "original comment text should be preserved verbatim") + }) + + t.Run("ignores expressions in bash comments but still hoists real run expressions", func(t *testing.T) { + content := `--- +on: workflow_dispatch +steps: + - name: Use parsed value + run: | + echo "${{ steps.parse.outputs.value }}" + # See also ${{ steps.parse.outputs.* }} for all outputs +--- +` + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "steps": []any{ + map[string]any{ + "name": "Use parsed value", + "run": "echo \"${{ steps.parse.outputs.value }}\"\n# See also ${{ steps.parse.outputs.* }} for all outputs", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "codemod should apply for real run expression") + assert.Contains(t, result, "EXPR_STEPS_PARSE_OUTPUTS_VALUE: ${{ steps.parse.outputs.value }}", "real expression should be hoisted") + assert.Contains(t, result, `echo "$EXPR_STEPS_PARSE_OUTPUTS_VALUE"`, "real expression reference should be rewritten") + assert.Contains(t, result, "${{ steps.parse.outputs.* }}", "comment expression should be preserved verbatim") + assert.NotContains(t, result, "steps.parse.outputs.*:", "wildcard comment expression must not generate an env binding") + }) + 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 From 209c091c0aec4378add91dbff884302b5b33be6a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:25:48 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=93=20shell-agnostic=20comment,=20stronger=20test=20asser?= =?UTF-8?q?tions,=20pwsh=20&=20folded-scalar=20tests?= 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 | 4 +- pkg/cli/codemod_steps_run_secrets_env_test.go | 68 +++++++++++++++++-- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/pkg/cli/codemod_steps_run_secrets_env.go b/pkg/cli/codemod_steps_run_secrets_env.go index 90585ea356f..2bf95cc5c09 100644 --- a/pkg/cli/codemod_steps_run_secrets_env.go +++ b/pkg/cli/codemod_steps_run_secrets_env.go @@ -231,8 +231,10 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= runKeyIndentLen { break } - // Skip bash comment lines – expressions inside comments are + // Skip shell comment lines – expressions inside # comments are // documentation-only and must not generate env bindings. + // NOTE: heredoc boundaries are not tracked; lines starting with + // '#' inside a heredoc body are also skipped (follow-up needed). if strings.HasPrefix(t, "#") { continue } diff --git a/pkg/cli/codemod_steps_run_secrets_env_test.go b/pkg/cli/codemod_steps_run_secrets_env_test.go index 39cd0c781e4..21450278626 100644 --- a/pkg/cli/codemod_steps_run_secrets_env_test.go +++ b/pkg/cli/codemod_steps_run_secrets_env_test.go @@ -569,8 +569,8 @@ steps: assert.NotContains(t, result, "$env:EXPR_GITHUB_ACTOR", "bash step must not use $env:VARNAME") }) - t.Run("ignores expressions inside bash comment lines in run block", func(t *testing.T) { - // Expressions inside bash comments are documentation-only and must not + t.Run("ignores expressions inside shell comment lines in run block", func(t *testing.T) { + // Expressions inside shell comments are documentation-only and must not // generate env bindings or be rewritten. content := `--- on: workflow_dispatch @@ -605,7 +605,7 @@ steps: assert.Contains(t, result, "${{ steps.parse.outputs.* }}", "original comment text should be preserved verbatim") }) - t.Run("ignores expressions in bash comments but still hoists real run expressions", func(t *testing.T) { + t.Run("ignores expressions in shell comments but still hoists real run expressions", func(t *testing.T) { content := `--- on: workflow_dispatch steps: @@ -631,7 +631,67 @@ steps: assert.Contains(t, result, "EXPR_STEPS_PARSE_OUTPUTS_VALUE: ${{ steps.parse.outputs.value }}", "real expression should be hoisted") assert.Contains(t, result, `echo "$EXPR_STEPS_PARSE_OUTPUTS_VALUE"`, "real expression reference should be rewritten") assert.Contains(t, result, "${{ steps.parse.outputs.* }}", "comment expression should be preserved verbatim") - assert.NotContains(t, result, "steps.parse.outputs.*:", "wildcard comment expression must not generate an env binding") + assert.NotContains(t, result, ": ${{ steps.parse.outputs.* }}", "wildcard comment expression must not generate an env binding") + }) + + t.Run("ignores expressions inside pwsh comment lines in run block", func(t *testing.T) { + // Expressions inside # comments are documentation-only for all shells, + // including PowerShell. + content := `--- +on: workflow_dispatch +steps: + - name: PS step + shell: pwsh + run: | + Write-Output "hello" + # ${{ steps.parse.outputs.value }} is documented here +--- +` + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "steps": []any{ + map[string]any{ + "name": "PS step", + "shell": "pwsh", + "run": "Write-Output \"hello\"\n# ${{ steps.parse.outputs.value }} is documented here", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.False(t, applied, "codemod should not modify content when only expression is in a comment") + assert.Equal(t, content, result, "content should be unchanged") + assert.NotContains(t, result, "EXPR_", "no EXPR_ bindings should be generated for comment-only expressions") + }) + + t.Run("ignores expressions in shell comments in folded run block", func(t *testing.T) { + // The comment-skip logic applies to folded-scalar (>) run blocks as well. + content := `--- +on: workflow_dispatch +steps: + - name: Use value + run: > + echo "${{ steps.parse.outputs.value }}" + # doc ${{ steps.parse.outputs.* }} +--- +` + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "steps": []any{ + map[string]any{ + "name": "Use value", + "run": "echo \"${{ steps.parse.outputs.value }}\"\n# doc ${{ steps.parse.outputs.* }}", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "codemod should apply cleanly") + assert.True(t, applied, "real expression should be hoisted") + assert.Contains(t, result, "EXPR_STEPS_PARSE_OUTPUTS_VALUE: ${{ steps.parse.outputs.value }}", "real expression should be hoisted to env") + assert.NotContains(t, result, ": ${{ steps.parse.outputs.* }}", "comment expression must not generate an env binding") + assert.Contains(t, result, "${{ steps.parse.outputs.* }}", "comment expression should be preserved verbatim") }) t.Run("uses distinct bindings when different bodies collide to the same EXPR_ name", func(t *testing.T) {