Repository navigation
ci: drop the redundant PR-head checkout from claude.yml (code scanning alert 71) #2069
Description
Summary
Code scanning alert #71 (actions/untrusted-checkout/high, CodeQL) is open against main at .github/workflows/claude.yml:91 — the Checkout PR branch step. That is the step #1966 added the is_fork == 'false' guard to, so the alert is current, not stale.
The alert is a false positive on its own terms: after #1966 there is no path where a fork head is checked out. But investigating it surfaced a real cleanup — the flagged step is redundant, and removing it clears the alert legitimately rather than by dismissal.
Why the step is redundant
anthropics/claude-code-action checks out the PR branch itself. From src/github/operations/branch.ts at the pinned v1.0.190:
// Handle open PR: Checkout the PR branch
execGit(["fetch", "origin", ...depthArgs, branchName]);
execGit(["checkout", branchName, "--"]);The action's own docs/security.md names a bare base-ref checkout (no ref:) as the preferred pattern, and upstream's own .github/workflows/claude.yml is exactly that: one actions/checkout, fetch-depth: 1, no PR lookup and no head checkout. Our Get PR details → Checkout PR branch pair is vestigial from the v1 example workflow inherited in #1869; the action re-checks-out the same branch moments later regardless.
Why CodeQL can't be satisfied any other way
The rule is syntactic: privileged trigger (issue_comment) + actions/checkout with a ref: derived from PR data ⇒ high. It does not evaluate steps.pr.outputs.is_fork (a runtime step output), the job-level author_association gate, or the repo's pull_request_creation_policy: collaborators_only (verified against the API). All three mitigations are invisible to it, so no amount of hardening around the step clears the alert while the step exists.
Proposed change
- Delete the
Checkout PR branchstep (the flagged line). - Make
Checkout repositoryunconditional — base ref, noref:input. - Drop the now-unused
shaoutput fromGet PR details. - Keep
Get PR details,Decline fork PR, and theis_forkgate onRun Claude Code, unchanged.
That last point is load-bearing. The action will check out a fork PR itself via refs/pull/N/head (if (prData.isCrossRepository) in the same source file), so the is_fork gate is what stops untrusted code reaching the workspace at all. This change removes our untrusted-ref checkout; it must not remove the gate.
Notes
- Workflows run from the default branch, so this is inert until
v2/mainreachesmainat the next milestone merge; the alert clears on the CodeQL run after that. - No test surface — workflow file only.
Activity
daichunghy commentedon Aug 22, 2026
Still true on current
main(86d5f58ec8c3)..github/workflows/claude.ymlstill hasCheckout PR branchwithref: ${{ steps.pr.outputs.sha }}. At the pinned action SHA (5ef2e550)setupBranchinsrc/github/operations/branch.tsthen fetches the PR again; the fork path usespull/${n}/head.On
v2/mainthat checkout step is already gone. Theis_forkgate is what still keeps the action from doing that fetch.cliffhall commentedon Aug 23, 2026
Confirmed, and that's the expected state — it's the "Notes" caveat in the issue body playing out.
The fix landed on
v2/mainin #2070. Workflows execute from the default branch, andmainonly receivesv2/mainat a milestone merge, somainkeeps the oldCheckout PR branchstep (and the alert stays open) until the v2.4.0 merge. Nothing further to do on the branch side; the CodeQL alert should clear on the first scan after that merge.Two things worth putting on the record while
mainstill carries the step:The alert remains a false positive on
mainin the meantime.Checkout PR branchthere is already gated onsteps.pr.outputs.is_fork == 'false'(#1882), so a fork head is never the checked-out ref. What CodeQL flags is the syntax — privileged trigger plus aref:derived from PR data — and it can't see the runtime step output, the job-levelauthor_associationgate, or the repo'scollaborators_onlyfork-PR policy. So the window is a stale-alert window, not an exposure window.And yes — the
is_forkgate is the load-bearing half, which is why #2070 deliberately kept it. With our own head checkout gone onv2/main, that gate is the only thing standing between a fork PR andsetupBranch'spull/${n}/headfetch. Tracing the conditions onv2/mainafter the change, all four states resolve correctly:Get PR detailsstateoutcomeis_forkRun Claude Codeskippedsuccess'false'head.reponull)success'true'failureThe fork row fails both disjuncts, so the action never runs and never fetches. The failure row is excluded twice over — the condition carries no status-check function, so GitHub applies an implicit
success()regardless of what the comparisons say.The remaining
Checkout repositorystep is unconditional but takes noref:, so it resolves to the base repo's default branch viaGITHUB_REF.fetch-depth: 0doesn't widen that either —refs/pull/*isn't in checkout's default refspec, so a fork head isn't in the local clone at all.daichunghy commentedon Aug 23, 2026
That answers it. I’ll treat
mainas a stale-alert window, not an active fork-head checkout, and use thev2/mainmilestone merge as the evidence point. Nothing else needed on my side.ci: drop the redundant PR-head checkout from claude.yml