Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,28 @@ jobs:
ref: ${{ steps.pr.outputs.sha }}
persist-credentials: false
fetch-depth: 1
# actions/checkout hard-refuses a fork PR head under
# pull_request_target unless this opt-in is set — it is guarding
# against "pwn request", where a workflow checks out fork code and
# then *executes* it (npm ci, build, test, a lifecycle script) with the
# base repo's write-scoped token and secrets. That guard landed in a
# v7 patch release, so every fork PR — including known contributors,
# who are the ones the `if:` above lets through — started failing this
# step with no change on our side.
#
# This job does none of the things the guard protects against: it never
# installs dependencies, never runs a build or test, and never executes
# anything from the checkout. The head is only ever *read* — by Claude's
# Read/Grep/Glob, which is the entire point of checking it out. The
# token is kept out of .git/config by persist-credentials above, out of
# the agent's subprocesses by the action's env scrub (see the header),
# and the agent has no Bash tool with which to run checked-out code
# even if it wanted to. So we opt in deliberately.
#
# 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.


# Claude cannot fetch the diff itself (no token in its subprocesses — see
# the header). Stage it on disk instead. This runs AFTER the checkout and
Expand Down
Loading