fix(ci): restore e2e fork PR checkout after actions/checkout@v7 bump - #2503
Conversation
actions/checkout@v7 refuses fork PR head checkouts on pull_request_target unless allow-unsafe-pr-checkout is set. The e2e workflow intentionally checks out authorized PR code after the gate job; restore that behavior after the actions version bump in fullsend-ai#2457. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
PR Summary by QodoFix e2e fork PR checkout under pull_request_target for actions/checkout@v7 Description
Diagram
High-Level Assessment
Files changed (1)
|
Site previewPreview: https://029936fe-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 1:54 PM UTC · Completed 2:12 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsHigh
Low
Labels: PR modifies .github/workflows/e2e.yml — e2e CI workflow fix |
| with: | ||
| ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }} | ||
| persist-credentials: false | ||
| # checkout@v7 blocks fork PR head checkouts on pull_request_target by default. |
There was a problem hiding this comment.
[low] comment-formatting
The new comment appears inside the with: block directly above the allow-unsafe-pr-checkout parameter it describes. Other security comments in this file (lines 42-44, 71-73) appear before entire jobs, but those describe job-level concerns. Parameter-level placement is idiomatic YAML — no change needed.
|
🤖 Finished Retro · ✅ Success · Started 2:31 PM UTC · Completed 2:36 PM UTC |
Retro: PR #2503 —
|
| Time (UTC) | Event |
|---|---|
| 13:50 | PR opened by ifireball — 3-line fix to .github/workflows/e2e.yml |
| 13:51–13:52 | Qodo bot posts summary and review (no issues found) |
| 13:54 | Review agent posts "started" status |
| 14:10 | ralphbean approves with "LGTM" (before review agent finishes) |
| 14:12 | Review agent submits CHANGES_REQUESTED — HIGH [protected-path] (missing linked issue), LOW [comment-formatting] (no action needed) |
| 14:26 | PR merged despite bot's CHANGES_REQUESTED |
Assessment
This workflow completed successfully with appropriate human judgment. The review agent's HIGH finding was procedural (missing linked issue for a protected path change), not technical. The human reviewer correctly assessed the 3-line fix as safe and merged it. The 18-minute review time for a 3-line diff was disproportionate but not blocking since the human approved independently.
Existing issues already cover observed patterns
The improvement opportunities from this PR are well-covered by existing open issues:
- Protected-path severity for human PRs: #1551 proposes downgrading protected-path severity for human-authored PRs
- Missing-authorization severity cap: #2200 proposes capping missing-authorization severity at Low when the PR body is descriptive
- COMMENT verdict for human PRs: #2115 proposes using COMMENT instead of CHANGES_REQUESTED for human-authored PRs with only medium/low findings
- Governance-only findings: #1068 proposes not filing CHANGES_REQUESTED for findings the fix agent cannot resolve
- Fast path for trivial changes: #1900 proposes a fast review path for small diffs
- PR description context: #2380 proposes fetching URLs linked in PR descriptions to validate change necessity
No new proposals are warranted — implementing the existing issues (particularly #1551, #2200, and #2115) would have prevented the false-positive CHANGES_REQUESTED on this PR.
…7-unsafe-pr-opt-in fix(ci): restore e2e fork PR checkout after actions/checkout@v7 bump
Summary
actions/checkout@v7(merged in chore: update obsolete GitHub Actions to latest major versions #2457) blocks fork PR head checkouts onpull_request_targetunlessallow-unsafe-pr-checkoutis setpull_request.head.shaunderpull_request_targetContext
After #2457 landed, authorized fork PRs pass the e2e gate job but fail at checkout with:
This is expected checkout@v7 behavior. The e2e split gate/e2e design already mitigates pwn-request risk: the gate job authorizes on base-branch code only, and the e2e job has no
pull-requests: write.Test plan
allow-unsafe-pr-checkoutis false outsidepull_request_target)Made with Cursor