From 47aaf9535037d589fab4d494f69f5f6a1e5dcf2f Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 19 Aug 2026 12:19:12 -0700 Subject: [PATCH 1/2] fix(ci): review the PR head, not the default branch The on-demand Claude review checked out with no `ref:`. Neither `issue_comment` nor `pull_request_review_comment` is a PR event, so actions/checkout defaulted to the default branch and every review read `main` instead of the PR. Files a PR added or renamed were invisible, and the job still succeeded, so nothing reported red. Resolve the PR number before checkout (it already branches on `event_name`, since the number lives in `issue.number` for one event and `pull_request.number` for the other) and pin the checkout to `refs/pull//head`. `/head` over `/merge`: it is the tree the author pushed, it matches what `gh pr diff` and the inline-comment line anchors refer to, and it still exists when the PR has conflicts. Security properties are unchanged: no `contents: write`, same maintainer-only `author_association` gate, same `--allowedTools`, still `issue_comment`/`pull_request_review_comment` rather than `pull_request_target`. The header comment now records what the checkout is for, which is why this went unnoticed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3 --- .../claude-code-review-on-demand.yml | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/.github/workflows/claude-code-review-on-demand.yml b/.github/workflows/claude-code-review-on-demand.yml index a088fa36..4f086bef 100644 --- a/.github/workflows/claude-code-review-on-demand.yml +++ b/.github/workflows/claude-code-review-on-demand.yml @@ -13,6 +13,18 @@ name: Claude Code Review (on demand) # - no `contents: write`, so it cannot push commits; # - fires ONLY on a maintainer's comment (author_association gate), so an # outside contributor on a fork can never trigger it. +# +# The checkout MUST be the PR's own ref, never the default one. Neither +# `issue_comment` nor `pull_request_review_comment` is a PR event, so +# actions/checkout with no `ref:` lands on the DEFAULT BRANCH — the review then +# reads `main` while claiming to review the PR. Files a PR adds or renames are +# simply absent, and nothing reports red (the job still succeeds). So the PR +# number is resolved FIRST and the checkout is pinned to it. `refs/pull/N/head` +# (not `/merge`): it is the tree the author actually pushed, it matches what +# `gh pr diff` and the inline-comment line anchors refer to, and unlike `/merge` +# it still exists when the PR has conflicts — a review is exactly what you want +# on a conflicted PR. `contents: read` is all this needs; do NOT "fix" a +# checkout problem by reaching for `pull_request_target` or extra permissions. on: issue_comment: types: [created] @@ -38,13 +50,11 @@ jobs: id-token: write steps: - # Note: claude-code-action adds its own 👀 reaction to the triggering - # comment, so there's no explicit reaction step here. - - name: Checkout repository - uses: actions/checkout@v4 - with: - fetch-depth: 1 - + # Resolved BEFORE checkout: the checkout ref depends on it. The PR number + # lives in a different payload field per event — `issue.number` on + # issue_comment, `pull_request.number` on pull_request_review_comment — + # and each is absent on the other event, so branch on `event_name` rather + # than relying on a `||` fallback over a null. - name: Prepare review context id: prep run: | @@ -60,6 +70,17 @@ jobs: } >> "$GITHUB_OUTPUT" fi + # `fetch-depth: 1` is enough: the prompt forbids running the project's + # build/lint/test, and every allowed tool reads the diff through `gh` + # (the API), not through local history. + - name: Checkout PR head + uses: actions/checkout@v4 + with: + ref: refs/pull/${{ steps.prep.outputs.pr }}/head + fetch-depth: 1 + + # Note: claude-code-action adds its own 👀 reaction to the triggering + # comment, so there's no explicit reaction step here. - name: Run Claude Code Review uses: anthropics/claude-code-action@v1 with: From 250addc17ff2d5da817fead0471ccbe0df5848ea Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 19 Aug 2026 12:47:13 -0700 Subject: [PATCH 2/2] ci: harden the review workflow's checkout and pin its actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from Copilot on #121, all backed by conventions this repo already states elsewhere. `persist-credentials: false` on the checkout. `release.yml` and `vale-binaries.yml` both set it; this workflow did not, and it matters more here than in either of those: the tree being checked out is now contributor-authored PR content, so leaving the token in `.git/config` puts it one step away from anything that later runs in that tree. Pin `anthropics/claude-code-action` to a commit SHA. `release.yml`'s header states the convention — "Action refs are pinned to commit SHAs (supply-chain hardening); the trailing comment records the human-readable tag" — and this file was the exception, while holding a long-lived secret and `id-token: write`. `actions/checkout` is pinned to the same SHA the other workflows use. Correct the header's claim that `contents: read` is all this needs. The job also holds `pull-requests: write`, `issues: write`, and `id-token: write`; those exist for posting comments, not for the checkout. The point being made was that a checkout problem is not a permissions problem — now said that way, rather than in a form that reads as a description of the whole job. --- .../workflows/claude-code-review-on-demand.yml | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/claude-code-review-on-demand.yml b/.github/workflows/claude-code-review-on-demand.yml index 4f086bef..74fa95e8 100644 --- a/.github/workflows/claude-code-review-on-demand.yml +++ b/.github/workflows/claude-code-review-on-demand.yml @@ -23,8 +23,11 @@ name: Claude Code Review (on demand) # (not `/merge`): it is the tree the author actually pushed, it matches what # `gh pr diff` and the inline-comment line anchors refer to, and unlike `/merge` # it still exists when the PR has conflicts — a review is exactly what you want -# on a conflicted PR. `contents: read` is all this needs; do NOT "fix" a -# checkout problem by reaching for `pull_request_target` or extra permissions. +# on a conflicted PR. Reading a PR ref needs no more than the `contents: read` +# this job already has — the write scopes below exist for posting comments, not +# for the checkout. So do NOT "fix" a checkout problem by reaching for +# `pull_request_target` or by widening permissions: neither was ever the +# blocker, and both trade a read problem for a write capability. on: issue_comment: types: [created] @@ -74,15 +77,19 @@ jobs: # build/lint/test, and every allowed tool reads the diff through `gh` # (the API), not through local history. - name: Checkout PR head - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: ref: refs/pull/${{ steps.prep.outputs.pr }}/head fetch-depth: 1 + # Nothing here writes to git, and this checkout is contributor-authored + # PR content — leaving the token in `.git/config` would put it a step + # away from anything that later runs in this tree. + persist-credentials: false # Note: claude-code-action adds its own 👀 reaction to the triggering # comment, so there's no explicit reaction step here. - name: Run Claude Code Review - uses: anthropics/claude-code-action@v1 + uses: anthropics/claude-code-action@d40ddef4c030e508327d6e35a9c45f3368482c50 # v1 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} # Single tracking comment (in-progress → results), updated in place.