Skip to content

ci: gate claude.yml on same-repo PRs and pin actions to commit SHAs - #1966

Merged
cliffhall merged 1 commit into
v2/mainfrom
v2/ci/harden-claude-workflow
Aug 11, 2026
Merged

ci: gate claude.yml on same-repo PRs and pin actions to commit SHAs#1966
cliffhall merged 1 commit into
v2/mainfrom
v2/ci/harden-claude-workflow

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #1882

Hardens .github/workflows/claude.yml in two ways, and documents a third finding that turned out to need no code change.

The issue was originally filed as a fork-PR concern raised by Copilot on #1869. The repo runs pull_request_creation_policy: collaborators_only (verified against the API), which retires most of that threat model — so this is defense in depth, not remediation, and #1882 was re-scored Low to match. It is still worth landing: the mitigation currently lives entirely in a repo setting, flippable from a UI with no review and no diff, rather than in the file.

1. Same-repo gating

Get PR details now emits an is_fork output, and both the PR-head checkout and Run Claude Code are gated on it:

Trigger steps.pr.outcome is_fork Behavior
Issue / non-PR comment skipped Check out base repo, run Claude
Same-repo PR success false Check out PR head, run Claude
Fork PR success true No checkout, no Claude, reason in the step summary
PR lookup failed failure No checkout, no Claude

Declining a fork PR outright is deliberate. The alternative floated in the issue — fall through to a "metadata-only" review against the base tree — trades an untrusted-code problem for a wrong-tree one, which is the other half of what Copilot flagged. Better to decline visibly than to post a confident review of a tree that isn't the one under review.

A deleted fork counts as a fork: head.repo is null in that case, so the comparison treats it as not-ours rather than throwing.

The head checkout also drops its repository: input. Only a same-repo head reaches that step now, and checkout's default is github.repository — a value no PR can influence. That is a stronger position than passing a PR-derived repo name that happens to have been checked.

2. Actions pinned to commit SHAs

actions/checkout, actions/github-script, and anthropics/claude-code-action were all on mutable major tags, in a job that holds ANTHROPIC_API_KEY and grants the agent Bash. A force-moved tag on any of the three is an unreviewed code change inside a secret-holding job.

Each is now pinned to a full commit SHA with the trailing # vX.Y.Z comment Dependabot reads, so pinning costs no upgrade automation:

Action Pin Version
actions/checkout 3d3c42e5aac5ba805825da76410c181273ba90b1 v7.0.1
actions/github-script 3a2844b7e9c422d3c10d287c895573f7108da1b3 v9.0.0
anthropics/claude-code-action 5ef2e550a465a721f4f45e4a7d3c340c873e1dcc v1.0.190 (== v1)

Interaction with #1922. The open Dependabot PR bumps checkout v6→v7 and github-script v8→v9 in this same file. The pins above are the v7 / v9 SHAs, so this does not regress those bumps — Dependabot will rebase #1922 and drop its now-redundant claude.yml hunks, leaving its main.yml changes intact.

3. The failure fallback — no change needed

Copilot's mirror-image finding was that a failed PR lookup falls back to the base checkout but still runs Claude, reviewing the wrong tree while spending secrets and minutes. On inspection this doesn't happen: Checkout repository was guarded by if: steps.pr.outcome != 'success', which contains no status-check function, so GitHub applies an implicit success() and skips the step after a failed prior step regardless of the outcome comparison.

The condition is now the explicit steps.pr.outcome == 'skipped', with a comment saying why the failure case is excluded — so the next reader doesn't have to re-derive it.

Out of scope

Copilot's other comment on #1869 — that issues: read / pull-requests: read prevents @claude from posting — is incorrect and is untouched here. claude-code-action mints its own GitHub App token via id-token: write; the permissions: block scopes GITHUB_TOKEN, not that app token. claude[bot] posted on #1825 under exactly these permissions. The read-only grants are correct and must not be widened.

Testing

No test surface — this is a workflow file, and workflows only run from the default branch, so the change isn't exercised until v2/main reaches main at the next milestone merge.

  • YAML parses; every step's resolved condition was checked against the table above.
  • npm run verify:format-coverage passes (939 tracked source files gated; .yml is outside the format globs, so nothing else in the gate applies to this diff).
  • Every pinned SHA was resolved from the upstream tag via the GitHub API (annotated tags dereferenced to their commit), not copied from a third party.

After it reaches main: verify with a real @claude invocation on a same-repo PR (must still check out the PR head and review it), and confirm a fork PR is declined with the summary message rather than silently reviewing the wrong tree.

No UI change, so no screenshots.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt

…1882)

Two pieces of hardening on the Claude Code workflow, plus a comment for a
finding that needed no change.

Same-repo gating. `Get PR details` now emits an `is_fork` output, and the
PR-head checkout and the `Run Claude Code` step are both gated on it. A fork
PR is declined outright — no checkout, no agent run, with the reason written
to the step summary — rather than falling through to a "metadata-only" review
of the base tree, which would trade an untrusted-code problem for a
wrong-tree one. A deleted fork (`head.repo` null) counts as a fork.

The head checkout also drops its `repository:` input: only a same-repo head
reaches that step now, and checkout's default `github.repository` is a value
no PR can influence.

SHA pins. actions/checkout, actions/github-script, and claude-code-action
were all on mutable major tags in a job that holds ANTHROPIC_API_KEY and
grants the agent Bash. Each is now pinned to a full commit SHA with the
trailing `# vX.Y.Z` comment Dependabot reads, so upgrade automation is
unaffected. Pinned to the v7 / v9 SHAs so this does not regress the bumps in
the open Dependabot PR #1922.

The failure fallback Copilot flagged turns out not to exist: `Checkout
repository` carries no status-check function, so GitHub applies an implicit
`success()` and skips it after a failed lookup regardless of the `outcome`
comparison. Its condition is now the explicit `== 'skipped'` and says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
@cliffhall cliffhall added v2 Issues and PRs for v2 chore Maintenance: deps, build tooling, CI, cleanup — no user-facing behavior change labels Aug 11, 2026
@cliffhall
cliffhall requested a balanced review from Copilot August 11, 2026 03:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens the Claude workflow against fork PR execution and mutable action tags.

Changes:

  • Declines fork PRs before checkout or Claude execution.
  • Pins all actions to verified commit SHAs.
  • Clarifies safe checkout behavior for non-PR and failed-lookup paths.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@cliffhall

Copy link
Copy Markdown
Member Author

Copilot review round 1 — no findings

Copilot's review (4902693976) posted an overview with no inline comments and no suppressed comments — verified against the raw /pulls/1966/reviews and /pulls/1966/comments payloads, not just the rendered page, since suppressed findings are easy to miss visually.

Its summary reads the diff correctly on all three axes:

  • declines fork PRs before both the checkout and the Claude run,
  • pins every action to a commit SHA,
  • clarifies the non-PR and failed-lookup checkout paths.

Nothing to respond to and nothing to change, so no follow-up review round is needed.

For the record, the two things a reviewer would most reasonably push back on, and why they are deliberate:

Declining a fork PR outright rather than degrading to a base-tree review. The metadata-only fallback floated in #1882 would swap an untrusted-code problem for a wrong-tree one — a confident review of a tree that isn't the one under review. Declining visibly, with the reason in the step summary, is the honest failure mode.

Dropping repository: from the head checkout rather than asserting it. Only a same-repo head reaches that step now, and actions/checkout defaults to github.repository, which no PR can influence. Not passing a PR-derived value at all is stronger than passing one that has been checked.

@cliffhall cliffhall linked an issue Aug 11, 2026 that may be closed by this pull request
@cliffhall
cliffhall merged commit eee482e into v2/main Aug 11, 2026
4 checks passed
@cliffhall
cliffhall deleted the v2/ci/harden-claude-workflow branch August 11, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance: deps, build tooling, CI, cleanup — no user-facing behavior change v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: gate claude.yml on same-repo PRs and pin actions to commit SHAs

2 participants