Skip to content

Commit 3e6c55e

Browse files
authored
Fix artipacked credential persistence in copilot-token-audit and copilot-token-optimizer (#25873)
1 parent 9f90a5e commit 3e6c55e

5 files changed

Lines changed: 110 additions & 99 deletions

File tree

.github/workflows/copilot-setup-steps.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ jobs:
1717
run: curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh | bash
1818
- name: Checkout code
1919
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
20+
with:
21+
persist-credentials: false
2022
- name: Set up Node.js
2123
uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f
2224
with:

.github/workflows/copilot-token-audit.lock.yml

Lines changed: 42 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/copilot-token-optimizer.lock.yml

Lines changed: 9 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/parser/yaml_import.go

Lines changed: 20 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,12 @@ func extractStepsFromCopilotSetup(workflow map[string]any) (string, error) {
205205
return "", errors.New("steps field is not a list in copilot-setup-steps job")
206206
}
207207

208-
// Ensure checkout step is always included and placed first
209-
stepsSlice = ensureCheckoutStepFirst(stepsSlice)
208+
// Strip checkout steps from the imported copilot-setup-steps. The compiler
209+
// generates its own secure checkout (with persist-credentials: false) via
210+
// CheckoutManager.GenerateDefaultCheckoutStep, so the imported checkout is
211+
// redundant and can introduce artipacked findings when the same job uploads
212+
// artifacts.
213+
stepsSlice = stripCheckoutSteps(stepsSlice)
210214

211215
// Marshal steps array directly to YAML format (without "steps:" wrapper)
212216
// This matches the format expected by the compiler which unmarshals into []any
@@ -215,52 +219,30 @@ func extractStepsFromCopilotSetup(workflow map[string]any) (string, error) {
215219
return "", fmt.Errorf("failed to marshal steps to YAML: %w", err)
216220
}
217221

218-
yamlImportLog.Printf("Extracted steps from copilot-setup-steps job (YAML array format) with checkout step ensured")
222+
yamlImportLog.Printf("Extracted steps from copilot-setup-steps job (YAML array format) with checkout steps stripped")
219223
return string(stepsYAML), nil
220224
}
221225

222-
// ensureCheckoutStepFirst ensures a checkout step exists and is placed first in the steps list
223-
// If a checkout step exists, it's moved to the beginning. If not, one is added.
224-
func ensureCheckoutStepFirst(steps []any) []any {
225-
// Find existing checkout step index
226-
checkoutIndex := -1
227-
for i, step := range steps {
226+
// stripCheckoutSteps removes any actions/checkout steps from the imported
227+
// copilot-setup-steps. The compiler generates its own secure checkout step
228+
// (with persist-credentials: false), so the imported checkout is redundant.
229+
// Stripping it prevents the artipacked finding where checkout + artifact
230+
// upload coexist with persisted credentials, and avoids a duplicate checkout
231+
// in the compiled lock file.
232+
func stripCheckoutSteps(steps []any) []any {
233+
result := make([]any, 0, len(steps))
234+
for _, step := range steps {
228235
if stepMap, ok := step.(map[string]any); ok {
229236
if uses, hasUses := stepMap["uses"]; hasUses {
230237
if usesStr, ok := uses.(string); ok {
231-
// Check if this is a checkout action (actions/checkout@... or exactly "actions/checkout")
232238
if strings.HasPrefix(usesStr, "actions/checkout@") || usesStr == "actions/checkout" {
233-
checkoutIndex = i
234-
break
239+
yamlImportLog.Printf("Stripping checkout step from copilot-setup-steps: %s", usesStr)
240+
continue
235241
}
236242
}
237243
}
238244
}
245+
result = append(result, step)
239246
}
240-
241-
// If checkout step exists and is already first, no changes needed
242-
if checkoutIndex == 0 {
243-
yamlImportLog.Print("Checkout step already at beginning of copilot-setup-steps")
244-
return steps
245-
}
246-
247-
// If checkout step exists but not first, move it to the beginning
248-
if checkoutIndex > 0 {
249-
yamlImportLog.Printf("Moving existing checkout step from position %d to beginning", checkoutIndex)
250-
checkoutStep := steps[checkoutIndex]
251-
// Remove from current position
252-
steps = append(steps[:checkoutIndex], steps[checkoutIndex+1:]...)
253-
// Prepend to beginning
254-
steps = append([]any{checkoutStep}, steps...)
255-
return steps
256-
}
257-
258-
// No checkout step found, add a default one at the beginning
259-
yamlImportLog.Print("No checkout step found in copilot-setup-steps, adding default checkout step at beginning")
260-
defaultCheckoutStep := map[string]any{
261-
"name": "Checkout code",
262-
"uses": "actions/checkout@v6",
263-
}
264-
steps = append([]any{defaultCheckoutStep}, steps...)
265-
return steps
247+
return result
266248
}

0 commit comments

Comments
 (0)