Skip to content

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

Description

@cliffhall

Rewritten 2026-08-10. This issue was originally filed as a fork-PR sandbox-escape concern raised by GitHub Copilot during review of #1869. The repo has since been confirmed to run pull_request_creation_policy: collaborators_only, which retires most of that threat model. The surviving work is narrower and is what this issue now tracks. The original framing is preserved in the collapsed section at the bottom.

What this issue is now

Two pieces of hardening on .github/workflows/claude.yml, plus one finding that turned out to need no code change.

1. Gate the PR-head checkout and the Claude run on same-repo PRs

.github/workflows/claude.yml checks out the PR head repo:

- name: Checkout PR branch
  if: steps.pr.outcome == 'success'
  uses: actions/checkout@v6
  with:
    ref: ${{ steps.pr.outputs.sha }}
    repository: ${{ steps.pr.outputs.repo }}
    fetch-depth: 0

steps.pr.outputs.repo is pr.data.head.repo.full_name — for a fork PR, the fork. Run Claude Code then executes in that checkout with ANTHROPIC_API_KEY in the environment and --allowedTools "Bash,mcp__mcp-docs".

Why this is no longer the emergency it was filed as. Two settings stand between an outside contributor and that checkout:

  1. pull_request_creation_policy: collaborators_only (verified against the repo API on 2026-08-10) — non-collaborators cannot open a PR here at all, so the untrusted fork PR mostly does not exist. Note allow_forking is still true; forks exist, they just can't open PRs.
  2. Author-association gate — every trigger arm requires OWNER / MEMBER / COLLABORATOR, so a PR author cannot self-trigger the workflow unless they are already in that set.

The residual path, and why it is still worth closing. The trigger gate keys on author_association, and MEMBER means member of the modelcontextprotocol org — a materially larger set than "has write on this repo." If GitHub counts org members as collaborators for the purposes of collaborators_only, an org member with no write access here could open a fork PR and self-trigger @claude on it. That has not been tested, and the setting is new enough that it should be tested rather than assumed.

Either way, the mitigation currently lives entirely in a repo setting rather than in the workflow. A setting can be flipped by anyone with admin, in a UI with no review and no diff. The gate belongs in the file.

Proposed shape — have the Get PR details step emit an is_fork output, then:

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 written to the step summary
PR lookup failed failure No checkout, no Claude

Refusing outright on a fork PR is deliberate. The alternative floated in the original write-up — fall through to a "metadata-only" review against the base tree — trades an untrusted-code problem for a wrong-tree problem, 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.

2. Pin the actions to commit SHAs

actions/checkout, actions/github-script, and anthropics/claude-code-action are all referenced by mutable major tags. This is independent of the fork question — it is ordinary supply-chain hygiene on a workflow that handles ANTHROPIC_API_KEY and grants Bash. A compromised or force-moved tag on any of the three is an unreviewed code change inside a secret-holding job.

Pin each to a full commit SHA with a trailing # vX.Y.Z comment, which is the form Dependabot reads and keeps updated — so pinning does not cost us upgrade automation.

⚠️ Coordinate with #1922. The open Dependabot PR bumps actions/checkout v6 → v7 and actions/github-script v8 → v9 in this same file. Pin to the v7 / v9 SHAs so this doesn't 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 concern — no change needed

Copilot's mirror-image finding was that when the PR-lookup step fails, the workflow falls back to checking out the base repo but still runs Claude, reviewing the wrong tree while spending secrets and minutes.

On inspection this does not happen. Checkout repository is guarded by if: steps.pr.outcome != 'success', which contains no status-check function, so GitHub applies an implicit success() — when a prior step has failed, the step is skipped regardless of the outcome comparison. The failure path already declines to run. Worth stating explicitly in the workflow so the next reader doesn't re-derive it, but there is no bug to fix.

Verification

The workflow only runs from the default branch, so a change here is not exercised until it reaches main at the next milestone merge. After that, verify with a real @claude invocation on a same-repo PR (must still check out the PR head and review it) and, if a fork PR can be produced, that it is declined with the summary message rather than silently reviewing the wrong tree.

Out of scope

Copilot's other comment on #1869 — that issues: read / pull-requests: read prevents @claude from posting — is factually incorrect and needs no action. 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 on 2026-07-27 under exactly these permissions, ten days after ac3c1a12 (2026-07-17) last touched the file. The read-only grants are correct and must not be widened.

Priority

Re-scored Low (5/16), down from Medium (8/16). The original score rested on the fork exposure being live; collaborators_only makes this hardening rather than remediation.

Axis Score Reasoning
Severity / impact 2/5 Defense in depth. The actor the original score assumed — an outside contributor opening a fork PR — cannot open a PR here. What remains is an unverified org-member edge and a mitigation that lives in a flippable setting instead of in the file.
Urgency / staleness 2/5 Nothing waits on it, and no exposure is believed live. Wanted, not scheduled-critical.
Signal bonuses +1 assigned (+1)
Total 5 Low band (≤5).
Original issue text, as filed 2026-08-01

Raised by GitHub Copilot during review of #1869 (the byte-for-byte restore of .github/workflows/claude.yml). Deliberately not fixed there — that PR's contract is a faithful restore of a known-working file, so a behavior change belongs in its own PR.

The observation

.github/workflows/claude.yml checks out the PR head repo:

- name: Checkout PR branch
  if: steps.pr.outcome == 'success'
  uses: actions/checkout@v6
  with:
    ref: ${{ steps.pr.outputs.sha }}
    repository: ${{ steps.pr.outputs.repo }}
    fetch-depth: 0

steps.pr.outputs.repo is pr.data.head.repo.full_name, which for a fork PR is the fork. The Run Claude Code step then runs in that checkout with ANTHROPIC_API_KEY in the environment and --allowedTools "Bash,mcp__mcp-docs". So a fork's package.json scripts, tool configs, or repo-local agent instructions are in reach of a tool-enabled agent run holding a secret.

Copilot also flagged the mirror-image case: when the PR lookup step is skipped or fails, the workflow falls back to checking out the base repo but still runs Claude — so it can review the wrong tree while still spending secrets and minutes.

Why it is not urgent

Two structural mitigations are already in place:

  1. Author-association gate. Every trigger arm requires the commenting/opening user to be OWNER / MEMBER / COLLABORATOR. An outside contributor cannot self-trigger the workflow on their own PR.
  2. pull_request_creation_policy: collaborators_only. The repo does not accept PRs from non-collaborators at all (consistent with the issues-only policy in CONTRIBUTORS.md, go-live 6/9: restrict PRs to collaborators; CONTRIBUTORS.md + SECURITY.md #1820), so untrusted fork PRs largely do not exist here.

The residual path is a maintainer deliberately running @claude against a collaborator's fork branch — collaborators already hold repo access, so the marginal exposure is small.

Possible hardening

Worth evaluating, none obviously free:

  • Gate the PR-head checkout on head.repo.full_name == github.repository, letting fork PRs fall through to a metadata-only review (no code checkout).
  • Alternatively gate the Run Claude Code step itself on same-repo, so the fallback branch cannot review the wrong tree.
  • Consider pinning actions/checkout, actions/github-script, and anthropics/claude-code-action to commit SHAs rather than mutable major tags.

Any change here should be verified against a real @claude invocation afterward, since the workflow is only exercised from the default branch.

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