Skip to content

ci: drop the redundant PR-head checkout from claude.yml - #2070

Open
cliffhall wants to merge 1 commit into
v2/mainfrom
v2/ci/2069-drop-redundant-pr-checkout
Open

ci: drop the redundant PR-head checkout from claude.yml#2070
cliffhall wants to merge 1 commit into
v2/mainfrom
v2/ci/2069-drop-redundant-pr-checkout

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #2069

Code scanning alert #71 (actions/untrusted-checkout/high) has stayed open against main at .github/workflows/claude.yml:91 — the Checkout PR branch step, i.e. the step #1966 added the is_fork == 'false' guard to. The alert is current, not a stale pre-#1966 instance:

ref:  refs/heads/main @ 86d5f58e     (current head)
path: .github/workflows/claude.yml:91

Why it can't be argued away

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 of #1966's mitigations are invisible to it, so no further hardening around the step clears the alert while the step exists. The choice was dismiss-as-false-positive or remove the step.

The step was 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, "--"]);

Its 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, no PR lookup, no head checkout. Our Get PR detailsCheckout PR branch pair is vestigial from the v1 example workflow inherited in #1869; the action re-checked-out the same branch moments later on every PR run.

So this removes the alert legitimately rather than by dismissal, and drops one redundant checkout plus one github-script round trip per PR run.

What changed

  • Deleted Checkout PR branch (the flagged step).
  • Checkout repository is now unconditional — base ref, no ref: input, so nothing a PR controls selects what is checked out. (It still carries an implicit success(), so a failed lookup skips it, same as before.)
  • Dropped the now-unused sha output from Get PR details; is_fork is its only consumer.
  • Kept Get PR details, Decline fork PR, and the is_fork gate on Run Claude Code, unchanged.

Resolved conditions:

Trigger steps.pr.outcome is_fork Checkout Claude
Issue / non-PR comment skipped base ref runs
Same-repo PR success false base ref, then action checks out PR branch runs
Fork PR success true base ref skipped, reason in step summary
PR lookup failed failure skipped (implicit success()) skipped

The gate that must not go

Keeping the is_fork gate is load-bearing, and more so after this change, not less. The action fetches a cross-repository head on its own via refs/pull/N/head:

if (prData.isCrossRepository) {
  execGit(["fetch", "origin", ...depthArgs, `pull/${entityNumber}/head:${branchName}`]);
}

So skipping Run Claude Code is now the only thing keeping fork code out of the workspace. The comment on Decline fork PR says this explicitly so nobody removes it as dead weight once the visible checkout is gone.

Testing

No test surface — workflow file only. Workflows run from the default branch, so this is inert until v2/main reaches main at the next milestone merge; alert 71 clears on the CodeQL run after that.

  • npm run ci passes.
  • YAML parses; every step's resolved condition checked against the table above.
  • Action SHA pins are untouched (checkout v7.0.1, github-script v9.0.0, claude-code-action v1.0.190).

No UI change, so no screenshots.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H

Code scanning alert 71 (actions/untrusted-checkout/high) has stayed open
against main at the `Checkout PR branch` step — the same step #1966 added
the `is_fork == 'false'` guard to. The rule is syntactic, so it cannot see
that guard, the job-level author_association check, or the repo's
`collaborators_only` fork-PR policy; no hardening around the step clears it
while the step exists.

The step turns out to be redundant anyway. claude-code-action fetches and
checks out the PR branch itself, and its own docs name a bare base-ref
checkout as the preferred pattern — which is what upstream's own claude.yml
does. Our lookup-then-checkout pair is vestigial from the v1 example
workflow inherited in #1869.

So: delete the PR-head checkout, make the base checkout unconditional, and
drop the now-unused `sha` output. The `is_fork` gate on Run Claude Code
stays, and is now the only thing keeping fork code out of the workspace —
the action fetches a cross-repository head via refs/pull/N/head on its own.

Closes #2069

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant