Skip to content

ci: drop the redundant PR-head checkout from claude.yml (code scanning alert 71) #2069

Description

@cliffhall

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 branch step (the flagged line).
  • Make Checkout repository unconditional — base ref, no ref: input.
  • Drop the now-unused sha output from Get PR details.
  • Keep Get PR details, Decline fork PR, and the is_fork gate on Run 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/main reaches main at the next milestone merge; the alert clears on the CodeQL run after that.
  • No test surface — workflow file only.

Activity

  1. added this to the v2.4.0 milestone on Aug 20, 2026
  2. added
    v2Issues and PRs for v2
    choreMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior change
    on Aug 20, 2026
  3. self-assigned this
    on Aug 22, 2026
  4. daichunghy commented on Aug 22, 2026

    @daichunghy

    Still true on current main (86d5f58ec8c3).

    .github/workflows/claude.yml still has Checkout PR branch with ref: ${{ steps.pr.outputs.sha }}. At the pinned action SHA (5ef2e550) setupBranch in src/github/operations/branch.ts then fetches the PR again; the fork path uses pull/${n}/head.

    On v2/main that checkout step is already gone. The is_fork gate is what still keeps the action from doing that fetch.

  5. cliffhall commented on Aug 23, 2026

    @cliffhall
    MemberAuthor

    Confirmed, and that's the expected state — it's the "Notes" caveat in the issue body playing out.

    The fix landed on v2/main in #2070. Workflows execute from the default branch, and main only receives v2/main at a milestone merge, so main keeps the old Checkout PR branch step (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 main still carries the step:

    The alert remains a false positive on main in the meantime. Checkout PR branch there is already gated on steps.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 a ref: derived from PR data — and it can't see the runtime step output, the job-level author_association gate, or the repo's collaborators_only fork-PR policy. So the window is a stale-alert window, not an exposure window.

    And yes — the is_fork gate is the load-bearing half, which is why #2070 deliberately kept it. With our own head checkout gone on v2/main, that gate is the only thing standing between a fork PR and setupBranch's pull/${n}/head fetch. Tracing the conditions on v2/main after the change, all four states resolve correctly:

    Get PR details state outcome is_fork Run Claude Code
    skipped (issue / non-PR comment) skipped (unset) runs
    same-repo PR success 'false' runs
    fork PR (incl. deleted fork → head.repo null) success 'true' skipped
    lookup failed failure (unset) skipped

    The 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 repository step is unconditional but takes no ref:, so it resolves to the base repo's default branch via GITHUB_REF. fetch-depth: 0 doesn'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.

  6. daichunghy commented on Aug 23, 2026

    @daichunghy

    That answers it. I’ll treat main as a stale-alert window, not an active fork-head checkout, and use the v2/main milestone merge as the evidence point. Nothing else needed on my side.

  7. added a commit that references this issue on Aug 26, 2026
    f8e97c9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

choreMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior changev2Issues and PRs for v2

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions