fix: safe-outputs checkout auto-detection must not downgrade contents: write to read

[Copilot]

When actions/checkout appears in safe-outputs.steps, the compiler auto-detects it and injects contents: read — but was doing so unconditionally, overwriting a handler-derived contents: write (e.g., from create-pull-request). The generated GitHub App token then lacked write access, causing 403s on git push and branch creation.

Changes

• safe_outputs_permissions.go: Guard the checkout auto-detection with a GetExplicit(PermissionContents) check. Only inject contents: read when no contents permission is already set; a pre-existing write is preserved unchanged.

// Before — unconditional downgrade
if stepsRequireContentsRead(safeOutputs.Steps) {
    permissions.Set(PermissionContents, PermissionRead)
}

// After — skip if a stronger permission already exists
if stepsRequireContentsRead(safeOutputs.Steps) {
    if _, exists := permissions.GetExplicit(PermissionContents); !exists {
        permissions.Set(PermissionContents, PermissionRead)
    }
}

• safe_outputs_permissions_test.go: Regression test TestComputePermissionsForSafeOutputs_CheckoutDoesNotDowngradeContentsWrite — asserts that create-pull-request + a checkout step yields contents: write, not contents: read.

• Fixes safe-outputs checkout auto-detection downgrades contents: write to read #50905

[Copilot]

Pull request overview

Note

Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.

Prevent the safe-outputs permission auto-detection for actions/checkout from unintentionally reducing an already-required contents: write permission (e.g., from create-pull-request handling).

Changes:

• Add a regression test ensuring checkout detection does not downgrade contents: write to contents: read.
• Update permission computation to only add contents: read when contents is not already set.

Show a summary per file

File	Description
pkg/workflow/safe_outputs_permissions_test.go	Adds coverage for “checkout must not downgrade contents permission”.
pkg/workflow/safe_outputs_permissions.go	Avoids overwriting an existing contents permission when checkout is detected.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

[gh-aw-bot]

@copilot Please refresh this PR branch if needed and run the pr-finisher skill before handing back to maintainers.

Generated by 👨🍳 PR Sous Chef · pi · gpt-5.4
_{Comment /souschef to run again}

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 10.3 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

[pelikhan]

/matt

[github-actions]

⚠️ Security scanning failed for Matt Pocock Skills Reviewer. Review the logs for details.

[github-actions]

Skills-Based Review 🧠

Applied /codebase-design — approving with one minor documentation observation.

📋 Key Themes & Highlights

Summary

The fix is minimal, correct, and well-tested. The guard uses permissions.Get(PermissionContents) which correctly handles both explicit per-scope permissions and shorthands like write-all — a better choice than GetExplicit (which the PR description mistakenly names). See the inline comment for details.

Positive Highlights

• ✅ Regression test added before the fix is merged
• ✅ Accurate comment in source explaining why the guard is needed
• ✅ Minimal, surgical change — only the at-risk branch is guarded
• ✅ Test name reads as a specification and clearly documents the invariant

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 24.5 AIC · ⊞ 7.1K
_{Comment /matt to run again}

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.86.0