fix(#2504): allow checkout@v7 to check out fork PR code in e2e workflow

[ralphbean]

Summary

• PR chore: update obsolete GitHub Actions to latest major versions #2457 upgraded actions/checkout to v7, which blocks fork PR checkouts in pull_request_target workflows by default
• This breaks e2e CI for all external contributor PRs (e.g., PR docs(security): add audit log integrity to threat model #2010)
• The gate job already requires the ok-to-test label from a maintainer before the e2e job runs, so opting in with allow-unsafe-pr-checkout: true is safe

Fixes #2504

Test plan

• Verify this PR's own e2e check passes (it's from an upstream branch, so the fix isn't strictly needed here, but it shouldn't regress)
• After merge, verify a fork PR (e.g., docs(security): add audit log integrity to threat model #2010) gets a passing e2e check

[ralphbean]

Duplicate of #2503 — @ifireball got there first.

[qodo-code-review]

PR Summary by Qodo

Allow fork PR checkout in e2e workflow with actions/checkout@v7
🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Description

• Opt in to fork PR checkouts under pull_request_target for actions/checkout@v7.
• Unblocks e2e CI for external contributors while keeping the existing ok-to-test gate.
• Documents the safety rationale inline next to the opt-in flag.

Diagram

graph TD
  evt{{"pull_request_target"}} --> wf["e2e.yml workflow"] --> gate["Gate: ok-to-test"] --> job["E2E job"] --> co["checkout@v7 (unsafe on)"] --> sha["PR head SHA"] --> run["Run e2e"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Move e2e to `pull_request` (avoid `pull_request_target`)

• ➕ Eliminates the need for unsafe checkout opt-in
• ➕ Reduces risk of elevated-context execution from fork code
• ➖ Likely loses access to required secrets/permissions for e2e infrastructure
• ➖ May require substantial rework (OIDC, ephemeral creds, or reduced test coverage)

2. Use a separate trusted runner workflow (`workflow_run`)

• ➕ Keeps privileged execution in a trusted workflow triggered after validation
• ➕ Can enforce additional policy checks before running e2e
• ➖ More complex plumbing across workflows and artifacts
• ➖ Longer feedback loop and harder debugging (two-step pipeline)

Recommendation: The current approach is appropriate given the existing maintainer-controlled ok-to-test gate: it restores fork PR e2e coverage with minimal churn while making the security trade-off explicit. The main alternatives either remove privileged context (at the cost of e2e capability) or add significant workflow complexity.

Files changed (1) +1 / -0

Other (1) +1 / -0

e2e.ymlEnable unsafe fork PR checkout for checkout@v7 in e2e job +1/-0

Enable unsafe fork PR checkout for checkout@v7 in e2e job

• Adds 'allow-unsafe-pr-checkout: true' to the 'actions/checkout@v7' step so fork PR code can be checked out when the workflow runs under 'pull_request_target'. Includes an inline comment documenting why this is considered safe given the existing maintainer 'ok-to-test' gate.

.github/workflows/e2e.yml

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 e2e: actions/checkout@v7 rejects fork PR checkouts in pull_request_target workflow

✅ Compliance rules (platform): 51 rules

✅ Skills: writing-user-docs, writing-adrs

1. Misleading safety comment 🐞 Bug ⚙ Maintainability

Description

The added comment says the unsafe checkout is safe because the gate requires an ok-to-test label,
but the gate also authorizes trusted authors without that label. This mismatch can mislead
maintainers about what conditions actually protect the unsafe checkout.

Code

.github/workflows/e2e.yml[110]

+          allow-unsafe-pr-checkout: true  # Safe: gate job requires ok-to-test label from maintainer before this runs

Relevance

⭐⭐⭐ High

Team previously accepted security-invariant clarifying comments/guards in e2e workflow (gate vs
checkout) in PR #2106.

PR-#2106
PR-#2158
PR-#2398

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The workflow gates pull_request_target runs on needs.gate.outputs.authorized == 'true', but the
authorization script sets authorized=true for trusted authors OR for a fresh ok-to-test label;
therefore the new comment is incomplete/inaccurate.

.github/workflows/e2e.yml[70-78]
scripts/check-e2e-authorization.sh[4-6]
scripts/check-e2e-authorization.sh[65-76]
.github/workflows/e2e.yml[105-110]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The comment on `allow-unsafe-pr-checkout: true` states safety is based on requiring an `ok-to-test` label, but the actual authorization logic also allows trusted authors (OWNER/MEMBER/COLLABORATOR) to run without that label.

### Issue Context
This is a documentation/maintainability mismatch (runtime behavior is unchanged), but it can cause incorrect assumptions during future edits to this security-sensitive workflow.

### Fix Focus Areas
- .github/workflows/e2e.yml[105-110]

### Suggested fix
Update the inline comment to describe the real condition, e.g.:
- `# Safe: e2e job is gated by needs.gate.outputs.authorized (trusted author or fresh ok-to-test).`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:13 PM UTC · Completed 2:24 PM UTC
Commit: d79f081 · View workflow run →

[fullsend-ai-review]

Review skipped — this PR is already closed.

The /fs-review command only reviews open pull requests.

_{Posted by fullsend pre-review check}

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 2:14 PM UTC · Completed 2:26 PM UTC
Commit: d79f081 · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review skipped — this PR is already closed.

The /fs-review command only reviews open pull requests.

_{Posted by fullsend post-review check}

[fullsend-ai-retro]

Retro: PR #2505

What happened: Issue #2504 was filed by ralphbean at 14:00 UTC documenting a regression from the checkout v4→v7 upgrade (PR #2457) that broke e2e CI for fork PRs. The triage agent ran successfully (~2 min), correctly identified the root cause and recommended fix, and applied ready-to-code. The code agent then ran and correctly implemented the 1-line fix (allow-unsafe-pr-checkout: true in e2e.yml), but failed to push because the GitHub App token lacks the workflows permission required to modify .github/workflows/ files. Meanwhile, PR #2503 had already been opened by ifireball at 13:50 UTC with an equivalent fix — 10 minutes before the issue was even filed. ralphbean then opened PR #2505 with the same fix, immediately self-closed it as a duplicate of #2503 (within 10 seconds), and the review agent dispatched but correctly skipped review on the closed PR.

Key observations:

1. Code agent push failure on workflow files — The agent did the right work but couldn't deliver it due to missing workflows permission. Already tracked in #470. A complementary approach — having triage detect likely workflow modifications and skip code dispatch — is tracked in #1888.
2. Review dispatched on already-closed PR — Wasted a dispatch cycle, though the agent handled it gracefully. Already covered by #1870, #1439, and #885.
3. No duplicate/existing-PR detection — The triage agent didn't notice PR fix(ci): restore e2e fork PR checkout after actions/checkout@v7 bump #2503 already existed before marking the issue ready-to-code. Already covered by #2184 and #1757.

No new proposals filed — all improvement opportunities are already tracked by existing open issues. The triage agent performed well (correct root cause, good labels). The code agent's implementation was correct; only the delivery mechanism failed due to a known permission gap.