Skip to content

fix(ci): opt in to fork PR checkout so Claude review stops failing - #1715

Merged
giswqs merged 1 commit into
mainfrom
fix/claude-review-fork-checkout
Aug 5, 2026
Merged

fix(ci): opt in to fork PR checkout so Claude review stops failing#1715
giswqs merged 1 commit into
mainfrom
fix/claude-review-fork-checkout

Conversation

@giswqs

@giswqs giswqs commented Aug 5, 2026

Copy link
Copy Markdown
Member

Problem

Every Claude Code Review run on an external PR fails within seconds at the checkout step:

Refusing to check out fork pull request code from a pull_request_target workflow. […] To opt in, review the risks at https://gh.io/securely-using-pull_request_target and set allow-unsafe-pr-checkout: true on the actions/checkout step.

Example: run 31022945517 on #1714.

Nothing changed in this repo. A recent actions/checkout v7 patch release added this guard, and @v7 is a mutable tag, so it moved under us. Because every external PR is cross-repository, this hits all of them — including known CONTRIBUTOR authors, who are precisely the ones the job's if: gate is meant to review automatically.

Fix

Set allow-unsafe-pr-checkout: true on that one checkout step.

Why that is safe here

The guard exists to stop "pwn request": a workflow that checks out fork code and then executes it — npm ci, a build, a test, an npm lifecycle script — with the base repository's write-scoped GITHUB_TOKEN and secrets.

This job does none of that. It never installs dependencies, never builds, never tests, and never runs anything from the checkout. The head is only ever read, by Claude's Read/Grep/Glob — which is the entire reason it is checked out (the base ref alone would hide added files and show pre-change context). Layered on top of that, already in place before this PR:

  • persist-credentials: false keeps the token out of .git/config.
  • The action's subprocess env scrub (auto-enabled by allowed_non_write_users) strips GITHUB_TOKEN/GH_TOKEN from every subprocess the agent spawns.
  • --allowedTools grants no Bash tool, so the agent has no way to execute the checked-out code even if a prompt injection told it to.

The step comment records the invariant this rests on: it holds only while no step in this job executes the checkout. Build/install/test steps belong in ci.yml, which runs under pull_request and gets no secrets.

Verification

The workflow parses and the input name matches actions/checkout's action.yml:

Checkout PR head for review context (read-only; never executed)
{ "ref": "...", "persist-credentials": false, "fetch-depth": 1, "allow-unsafe-pr-checkout": true }

pre-commit run --files .github/workflows/claude-code-review.yml passes. End-to-end confirmation is the next fork PR's review run going green — this PR is same-repo, so it does not exercise the fork path itself.

Note: pr-preview.yml is the repo's only other pull_request_target workflow, and it is unaffected — it checks out the default branch with no ref:, so it never touches fork code.

Summary by CodeRabbit

  • Chores
    • Updated pull request review workflow handling to support contributions from forked repositories.
    • Added safeguards and documentation clarifying that checked-out code is read-only and must not be executed, installed, built, or tested.

A recent actions/checkout v7 patch made the action hard-refuse a fork PR
head under `pull_request_target` unless `allow-unsafe-pr-checkout: true`.
Since every external PR is cross-repository, the review job started
failing at the checkout step within seconds — including for known
CONTRIBUTOR authors, who are exactly the ones the job's `if:` gate lets
through automatically. Nothing changed on our side; the mutable `v7` tag
moved under us.

The guard targets "pwn request": checking out fork code and then
executing it with the base repo's write-scoped token and secrets. This
job never installs, builds, tests, or otherwise executes the checkout —
it only reads it via Claude's Read/Grep/Glob, which is the whole reason
the head is checked out. The token is already kept out of .git/config
(persist-credentials: false) and out of the agent's subprocesses (the
action's env scrub), and the agent has no Bash tool at all. So opt in
explicitly, and record in a comment that this holds only while no step
in this job executes the checkout.
Copilot AI lite review requested due to automatic review settings August 5, 2026 16:07
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bfe915f1-2a74-47c3-9bcc-a268ce9b9107

📥 Commits

Reviewing files that changed from the base of the PR and between 03e907f and d39d826.

📒 Files selected for processing (1)
  • .github/workflows/claude-code-review.yml

📝 Walkthrough

Walkthrough

The review workflow now checks out fork pull-request heads with allow-unsafe-pr-checkout: true. Comments state that Claude may read the checkout but must not execute, install, build, or test the checked-out code.

Changes

Review workflow checkout

Layer / File(s) Summary
Configure read-only fork checkout
.github/workflows/claude-code-review.yml
The checkout step enables allow-unsafe-pr-checkout: true and documents read-only handling restrictions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: copilot

Poem

A rabbit checks out code with care,
Reads the files but leaves them there.
No builds, installs, or tests take flight,
Forked heads stay read-only tonight.
Hop, hop—safe review in sight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the workflow change that enables fork pull-request checkout for Claude review.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/claude-review-fork-checkout

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Claude Code Review GitHub Actions workflow to opt in to checking out fork PR heads under pull_request_target, unblocking automated reviews for external contributors after a recent actions/checkout@v7 patch introduced a safety guard.

Changes:

  • Set allow-unsafe-pr-checkout: true on the actions/checkout@v7 step that checks out the PR head SHA for review context.
  • Add an in-file security rationale documenting why the opt-in is considered safe for this specific job and what invariants must remain true (no executing code from the checkout).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@giswqs
giswqs merged commit 8c7ac62 into main Aug 5, 2026
11 of 12 checks passed
@giswqs
giswqs deleted the fix/claude-review-fork-checkout branch August 5, 2026 16:09
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🔍 Cloudflare PR preview

Item Value
Site https://b54d141e.geolibre-preview.pages.dev
Demo app https://b54d141e.geolibre-preview.pages.dev/demo/
Commit d39d826

# This stays true only as long as no step in this job executes the
# checkout. Do not add a build, install, or test step here — put it in
# ci.yml, which runs under `pull_request` and gets no secrets.
allow-unsafe-pr-checkout: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Security (medium confidence): The safety argument above (no npm ci/build/test/lifecycle-script execution, no Bash tool) covers the classic "pwn request" path, but there's a second execution path it doesn't address: Claude Code's own hook mechanism. If the checked-out fork head (which is exactly what this opt-in now allows onto disk) contains a .claude/settings.json or .claude/settings.local.json with PreToolUse/PostToolUse hooks matching Read/Grep/Glob, those hooks run as shell commands automatically — hook execution isn't gated by --allowedTools, so excluding Bash from the agent's tool list doesn't stop it. A PR could add such a file in the very diff being reviewed and get arbitrary command execution in the runner during this step, even though GITHUB_TOKEN itself is scrubbed from subprocess env.

Worth confirming (and ideally noting in the comment here) that claude-code-action either ignores repo-local hook config or runs Claude Code with an isolated settings scope when reviewing untrusted fork content — otherwise this residual vector isn't covered by the reasoning given.

Also worth double-checking (lower confidence): that allow-unsafe-pr-checkout is genuinely a supported input on the pinned actions/checkout@v7 release. If the input name is off, GitHub Actions will typically just warn about an unrecognized input rather than fail the step, so the original checkout refusal would silently persist rather than being fixed — the PR's own verification notes it hasn't yet been confirmed against a fork PR run.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • None found in the diff itself — it's a single boolean input addition plus a comment block; no logic changes. (High confidence)

Security

  • The safety justification (no build/install/test/execution of fork code, no Bash tool) doesn't address Claude Code's own hook mechanism: a malicious PR could ship a .claude/settings.json with PreToolUse/PostToolUse hooks on Read/Grep/Glob that run shell commands automatically, independent of --allowedTools. Since this checkout now puts the fork's head (including newly added files) on disk before the review step runs, that's a residual "pwn request" path the reasoning doesn't cover. Worth confirming claude-code-action ignores/sandboxes repo-local hook config for untrusted checkouts. (Medium confidence — flagged inline on line 152)
  • Whether allow-unsafe-pr-checkout is actually a valid input on the pinned actions/checkout@v7 release is unverified (no network access here, and the PR body itself says end-to-end confirmation is still pending). If the input name is wrong, GitHub Actions would likely just warn and the original failure would silently persist rather than being fixed. (Low-medium confidence — flagged inline on line 152)

Performance

  • No performance-relevant changes; this only adds a step input and comments. (High confidence — not applicable)

Quality

  • The PR description states "pr-preview.yml is the repo's only other pull_request_target workflow," but pr-preview.yml actually triggers on pull_request, not pull_request_target (its only mention of pull_request_target is a comment warning against switching to it). claude-code-review.yml appears to be the only workflow in the repo using pull_request_target. This doesn't affect the code change, but it's a factual inaccuracy in the PR's own verification notes for future readers. (High confidence — not tied to a diff line, so not posted inline)
  • The added 22-line comment duplicates some ground already covered by the file's existing header comment (subprocess env scrub, no-Bash guarantee, "pwn request" framing), but that repetition matches this file's established documentation-heavy style, so not flagged as an issue. (Low confidence / stylistic only)

CLAUDE.md

  • No violations identified; this change is scoped to .github/workflows/claude-code-review.yml and doesn't touch any area CLAUDE.md gives specific conventions for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants