diff --git a/.github/workflows/issue-monster.lock.yml b/.github/workflows/issue-monster.lock.yml index 850d5ca2461..8751f7da43f 100644 --- a/.github/workflows/issue-monster.lock.yml +++ b/.github/workflows/issue-monster.lock.yml @@ -389,7 +389,7 @@ name: "Issue Monster" # return { # number: issue.number, # title: issue.title, - # labels: issue.labels.map(l => l.name), # Label filtering applied via job conditions + # labels: issue.labels.map(l => l.name), # body: issue.body, # created_at: issue.created_at, # score diff --git a/pkg/cli/codemod_checkout_persist_credentials_false.go b/pkg/cli/codemod_checkout_persist_credentials_false.go new file mode 100644 index 00000000000..6802e9e745c --- /dev/null +++ b/pkg/cli/codemod_checkout_persist_credentials_false.go @@ -0,0 +1,250 @@ +package cli + +import ( + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var checkoutPersistCredentialsFalseCodemodLog = logger.New("cli:codemod_checkout_persist_credentials_false") + +// getCheckoutPersistCredentialsFalseCodemod ensures checkout steps set with.persist-credentials: false. +func getCheckoutPersistCredentialsFalseCodemod() Codemod { + return Codemod{ + ID: "checkout-persist-credentials-false", + Name: "Add persist-credentials: false to checkout steps", + Description: "Ensures actions/checkout steps set with.persist-credentials: false in steps-like sections for strict-mode safety.", + IntroducedIn: "1.0.44", + 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 = transformSectionCheckoutPersistCredentials(current, section) + modified = modified || sectionChanged + } + return current, modified + }) + if applied { + checkoutPersistCredentialsFalseCodemodLog.Print("Added persist-credentials: false to actions/checkout step with blocks") + } + return newContent, applied, err + }, + } +} + +func transformSectionCheckoutPersistCredentials(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 + } + } + + sectionLines := lines[sectionStart : sectionEnd+1] + updatedSection, changed := transformCheckoutWithinSection(sectionLines, sectionIndent) + if !changed { + return lines, false + } + + result := make([]string, 0, len(lines)) + result = append(result, lines[:sectionStart]...) + result = append(result, updatedSection...) + result = append(result, lines[sectionEnd+1:]...) + return result, true +} + +func transformCheckoutWithinSection(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 := ensureStepCheckoutPersistCredentials(chunk, stepIndent) + modified = modified || changed + result = append(result, updatedChunk...) + i = stepEnd + 1 + continue + } + + result = append(result, line) + i++ + } + + return result, modified +} + +func ensureStepCheckoutPersistCredentials(stepLines []string, stepIndent string) ([]string, bool) { + usesIdx := -1 + usesIndent := "" + isUsesInline := false + withStart := -1 + withEnd := -1 + withIndent := "" + withKeyIndentLen := 0 + persistIdx := -1 + + for i := 0; i < len(stepLines); i++ { + line := stepLines[i] + trimmed := strings.TrimSpace(line) + indent := getIndentation(line) + + usesMatch, usesValue, _ := parseStepKeyLine(trimmed, indent, stepIndent, "uses") + if usesMatch && isCheckoutUsesValue(usesValue) { + usesIdx = i + isUsesInline = strings.HasPrefix(trimmed, "- uses:") && len(indent) == len(stepIndent) + if isUsesInline { + usesIndent = stepIndent + " " + } else { + usesIndent = indent + } + } + + withMatch, withValue, currentWithKeyIndentLen := parseStepKeyLine(trimmed, indent, stepIndent, "with") + if withMatch { + if withValue != "" && hasPersistKey(withValue) { + if persistExplicitTrue(withValue) { + checkoutPersistCredentialsFalseCodemodLog.Print("Skipping checkout step update: explicit with.persist-credentials: true found") + } + return stepLines, false + } + withStart = i + withEnd = i + withIndent = indent + withKeyIndentLen = currentWithKeyIndentLen + for j := i + 1; j < len(stepLines); j++ { + t := strings.TrimSpace(stepLines[j]) + if len(t) == 0 { + withEnd = j + continue + } + if effectiveStepLineIndentLen(t, getIndentation(stepLines[j]), stepIndent) <= withKeyIndentLen { + break + } + withEnd = j + if parseYAMLMapKey(t) == "persist-credentials" { + persistIdx = j + } + } + } + } + + if usesIdx == -1 { + return stepLines, false + } + + if persistIdx != -1 { + persistLine := strings.TrimSpace(stepLines[persistIdx]) + if persistExplicitTrue(persistLine) { + checkoutPersistCredentialsFalseCodemodLog.Print("Skipping checkout step update: explicit with.persist-credentials: true found") + } + return stepLines, false + } + + if withStart != -1 { + insertAt := withEnd + 1 + insertLine := fmt.Sprintf("%spersist-credentials: false", withIndent+" ") + updated := append([]string{}, stepLines[:insertAt]...) + updated = append(updated, insertLine) + updated = append(updated, stepLines[insertAt:]...) + return updated, true + } + + if usesIndent == "" { + usesIndent = stepIndent + " " + } + insertLines := []string{ + usesIndent + "with:", + usesIndent + " persist-credentials: false", + } + insertAt := usesIdx + 1 + updated := append([]string{}, stepLines[:insertAt]...) + updated = append(updated, insertLines...) + updated = append(updated, stepLines[insertAt:]...) + return updated, true +} + +func isCheckoutUsesValue(raw string) bool { + value := strings.TrimSpace(raw) + value = strings.Trim(value, "\"'") + value = strings.ToLower(value) + return strings.HasPrefix(value, "actions/checkout@") || value == "actions/checkout" +} + +func hasPersistKey(raw string) bool { + return extractPersistCredentialsValue(raw) != "" +} + +func persistExplicitTrue(raw string) bool { + return strings.EqualFold(extractPersistCredentialsValue(raw), "true") +} + +func extractPersistCredentialsValue(raw string) string { + lower := strings.ToLower(raw) + idx := strings.Index(lower, "persist-credentials:") + if idx == -1 { + return "" + } + rest := strings.TrimSpace(raw[idx+len("persist-credentials:"):]) + if rest == "" { + return "" + } + + rest = strings.SplitN(rest, "#", 2)[0] + rest = strings.SplitN(rest, ",", 2)[0] + rest = strings.SplitN(rest, "}", 2)[0] + return strings.TrimSpace(strings.Trim(rest, `"'`)) +} diff --git a/pkg/cli/codemod_checkout_persist_credentials_false_fuzz_test.go b/pkg/cli/codemod_checkout_persist_credentials_false_fuzz_test.go new file mode 100644 index 00000000000..09fef8d7f74 --- /dev/null +++ b/pkg/cli/codemod_checkout_persist_credentials_false_fuzz_test.go @@ -0,0 +1,143 @@ +//go:build !integration + +package cli + +import ( + "fmt" + "strings" + "testing" +) + +const maxCheckoutRefLength = 16 + +func FuzzCheckoutPersistCredentialsFalseCodemod(f *testing.F) { + f.Add(true, "@v5", false, false, false, false, uint8(0)) + f.Add(true, "", true, false, false, false, uint8(1)) + f.Add(true, "@v4", true, true, true, false, uint8(2)) + f.Add(true, "@v4", true, true, false, false, uint8(3)) + f.Add(false, "@v4", false, false, false, false, uint8(0)) + + f.Fuzz(func(t *testing.T, isCheckout bool, ref string, hasWith bool, hasPersist bool, persistTrue bool, inlineUses bool, sectionSelector uint8) { + codemod := getCheckoutPersistCredentialsFalseCodemod() + section := []string{"pre-steps", "steps", "post-steps", "pre-agent-steps"}[int(sectionSelector)%4] + + ref = sanitizeCheckoutRef(ref) + uses := "actions/cache@v4" + if isCheckout { + if ref == "" { + uses = "actions/checkout" + } else { + uses = "actions/checkout" + ref + } + } + + step := map[string]any{} + if inlineUses { + step["uses"] = uses + } else { + step["name"] = "checkout-step" + step["uses"] = uses + } + + if hasWith { + with := map[string]any{"fetch-depth": 0} + if hasPersist { + with["persist-credentials"] = persistTrue + } + step["with"] = with + } + + frontmatter := map[string]any{ + "on": "push", + section: []any{step}, + "workflow": "fuzz", + } + + content := buildCheckoutFuzzContent(section, uses, hasWith, hasPersist, persistTrue, inlineUses) + + result, applied, err := codemod.Apply(content, frontmatter) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expectMutation := isCheckout && (!hasWith || !hasPersist) + + if expectMutation { + if !applied { + t.Fatalf("expected codemod to apply; section=%s uses=%s", section, uses) + } + if !strings.Contains(result, "persist-credentials: false") { + t.Fatalf("expected result to include persist-credentials: false") + } + return + } + + if applied { + t.Fatalf("expected codemod not to apply; section=%s uses=%s", section, uses) + } + if result != content { + t.Fatalf("unexpected content mutation when codemod not applied") + } + }) +} + +func buildCheckoutFuzzContent(section, uses string, hasWith, hasPersist, persistTrue, inlineUses bool) string { + usesLine := fmt.Sprintf(" uses: %s", uses) + if inlineUses { + usesLine = fmt.Sprintf(" - uses: %s", uses) + } + + lines := []string{ + "---", + "on: push", + section + ":", + } + if !inlineUses { + lines = append(lines, " - name: checkout-step") + } + lines = append(lines, usesLine) + + if hasWith { + lines = append(lines, " with:", " fetch-depth: 0") + if hasPersist { + value := "false" + if persistTrue { + value = "true" + } + lines = append(lines, " persist-credentials: "+value) + } + } + + lines = append(lines, "---") + return strings.Join(lines, "\n") + "\n" +} + +func sanitizeCheckoutRef(ref string) string { + if ref == "" { + return "" + } + if len(ref) > maxCheckoutRefLength { + ref = ref[:maxCheckoutRefLength] + } + var b strings.Builder + for _, r := range ref { + switch { + case r >= 'a' && r <= 'z': + b.WriteRune(r) + case r >= 'A' && r <= 'Z': + b.WriteRune(r) + case r >= '0' && r <= '9': + b.WriteRune(r) + case r == '.', r == '-', r == '_': + b.WriteRune(r) + } + } + clean := b.String() + if clean == "" { + return "" + } + if clean[0] != '@' { + return "@" + clean + } + return clean +} diff --git a/pkg/cli/codemod_checkout_persist_credentials_false_test.go b/pkg/cli/codemod_checkout_persist_credentials_false_test.go new file mode 100644 index 00000000000..6ba10b45ba9 --- /dev/null +++ b/pkg/cli/codemod_checkout_persist_credentials_false_test.go @@ -0,0 +1,127 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckoutPersistCredentialsFalseCodemod(t *testing.T) { + codemod := getCheckoutPersistCredentialsFalseCodemod() + assert.Equal(t, "1.0.44", codemod.IntroducedIn) + + t.Run("adds with block when checkout step has none", func(t *testing.T) { + content := `--- +on: push +steps: + - name: Checkout repository + uses: actions/checkout@v5 +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "name": "Checkout repository", + "uses": "actions/checkout@v5", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "uses: actions/checkout@v5\n with:\n persist-credentials: false") + }) + + t.Run("adds persist-credentials under existing with block", func(t *testing.T) { + content := `--- +on: push +steps: + - name: Checkout repository + uses: actions/checkout@v5 + with: + fetch-depth: 0 +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "name": "Checkout repository", + "uses": "actions/checkout@v5", + "with": map[string]any{ + "fetch-depth": 0, + }, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "fetch-depth: 0\n persist-credentials: false") + }) + + t.Run("does not mutate explicit persist-credentials true", func(t *testing.T) { + content := `--- +on: push +steps: + - name: Checkout repository + uses: actions/checkout@v5 + with: + persist-credentials: true +--- +` + frontmatter := map[string]any{ + "on": "push", + "steps": []any{ + map[string]any{ + "name": "Checkout repository", + "uses": "actions/checkout@v5", + "with": map[string]any{ + "persist-credentials": true, + }, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) + }) + + t.Run("supports pre-steps and post-steps sections", func(t *testing.T) { + content := `--- +on: pull_request +pre-steps: + - uses: actions/checkout@v5 +post-steps: + - name: Checkout repo post + uses: actions/checkout@v5 +--- +` + frontmatter := map[string]any{ + "on": "pull_request", + "pre-steps": []any{ + map[string]any{"uses": "actions/checkout@v5"}, + }, + "post-steps": []any{ + map[string]any{ + "name": "Checkout repo post", + "uses": "actions/checkout@v5", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "pre-steps:\n - uses: actions/checkout@v5\n with:\n persist-credentials: false") + assert.Contains(t, result, "uses: actions/checkout@v5\n with:\n persist-credentials: false") + }) +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index b9076b56b94..d9c6ceaba79 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -56,6 +56,7 @@ func GetAllCodemods() []Codemod { getPluginsToDependenciesCodemod(), // Migrate plugins to dependencies (plugins removed in favour of APM) getSerenaToSharedImportCodemod(), // Migrate removed tools.serena to shared/mcp/serena.md import getWorkflowRunBranchesCodemod(), // Add default branches to bare on.workflow_run trigger + getCheckoutPersistCredentialsFalseCodemod(), // Add with.persist-credentials: false to actions/checkout steps getPullRequestTargetCheckoutFalseCodemod(), // Add checkout: false for pull_request_target workflows when safe getDependabotPermissionsCodemod(), // Add vulnerability-alerts: read when dependabot toolset is used getGitHubReposToAllowedReposCodemod(), // Rename deprecated tools.github.repos to tools.github.allowed-repos diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 592d76e2edc..59ed07cfaac 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -87,6 +87,7 @@ func TestGetAllCodemods_ContainsExpectedCodemods(t *testing.T) { "engine-env-secrets-to-engine-config", "serena-tools-to-shared-import", "workflow-run-branches-default", + "checkout-persist-credentials-false", "pull-request-target-checkout-false", "dependabot-toolset-permissions", "features-byok-copilot-removal", @@ -159,6 +160,7 @@ func expectedCodemodOrder() []string { "plugins-to-dependencies", "serena-tools-to-shared-import", "workflow-run-branches-default", + "checkout-persist-credentials-false", "pull-request-target-checkout-false", "dependabot-toolset-permissions", "github-repos-to-allowed-repos",