From 325b37c97eb25e33e6624d7a450de0ff50751979 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 5 Aug 2026 15:48:57 -0500 Subject: [PATCH 1/3] CI: fix claude-review failing at checkout on every fork PR actions/checkout now refuses, by default, to check out a fork PR's head under pull_request_target (a "pwn request" guard) -- this job has always been safe to opt out of that guard (trusted-fork gate + read-only use), it just started failing when the guard shipped. --- .github/workflows/claude-code-review.yml | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 6585d24..8bf3e8e 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -21,8 +21,19 @@ concurrency: jobs: claude-review: - # Trusted fork only, and skip drafts (don't spend API/CI on unfinished PRs). - # To add more trusted owners, extend the head-owner check. + # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE head.repo.owner.login + # CHECK BELOW !!! It is the ONLY thing standing between an arbitrary external + # fork's PR and this job's write-capable GITHUB_TOKEN, CLAUDE_CODE_OAUTH_TOKEN, + # and -- now that the checkout step below sets allow-unsafe-pr-checkout: true -- + # a checked-out copy of that fork's own code running in this trusted context. + # Drop or loosen this check and the checkout step's "safe because the job is + # already gated to a trusted fork" justification stops being true, turning this + # into a textbook "pwn request" vulnerability. To trust an additional fork, + # EXTEND this condition explicitly (e.g. `|| ... == 'other-trusted-account'`) -- + # never replace it with something broader (a wildcard, a check on PR author + # instead of head repo owner, etc.). + # + # Skip drafts too (don't spend API/CI on unfinished PRs). if: >- github.event.pull_request.draft == false && github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade' @@ -84,6 +95,14 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 1 persist-credentials: false + # actions/checkout refuses, by default, to check out a fork PR's head + # under pull_request_target (a "pwn request" guard). Safe to override + # here specifically because BOTH halves of the two-layer defense hold: + # the job above is gated to the trusted-fork check, and this checkout + # is never built/run/sourced -- only fed to the read-only review step + # below. Opting in without both of those would be unsafe; see + # https://gh.io/securely-using-pull_request_target. + allow-unsafe-pr-checkout: true - name: Run Claude Code Review if: steps.gate.outputs.decision == 'run' From f65714d0b17ffe701f4a44e1d18414822bff555e Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 5 Aug 2026 16:51:56 -0500 Subject: [PATCH 2/3] Fix checkout pattern: don't redirect origin to the fork allow-unsafe-pr-checkout: true plus a repository:/ref: override checking out the fork directly is the wrong fix -- it silences the checkout refusal but breaks claude-code-action's own internal fetch of refs/pull//head (which only exists on this repo, not the fork), per Postgres-Extensions/extension_tools#28 hitting and fixing the identical mistake. The action already fetches and reads the PR's actual content itself; this step only needs to check out the base branch. --- .github/workflows/claude-code-review.yml | 45 ++++++++++++------------ 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 8bf3e8e..7a7c2c4 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -9,8 +9,10 @@ name: Claude Code Review # write-capable token. The job is gated to PRs from the trusted `jnasbyupgrade` # fork only — an arbitrary external fork can never trigger this secret-bearing # job. The workflow file always comes from the base branch (master), so a PR -# cannot modify the reviewer that runs on it. We check out the PR head only for -# read context (persist-credentials: false) and never build or execute PR code. +# cannot modify the reviewer that runs on it. This workflow never checks out +# the PR's own ref into the workspace (see the checkout step below) -- +# claude-code-action fetches and reads the PR's content itself, safely, and +# never builds or executes it. on: pull_request_target: types: [opened, synchronize, reopened, ready_for_review] @@ -23,12 +25,9 @@ jobs: claude-review: # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE head.repo.owner.login # CHECK BELOW !!! It is the ONLY thing standing between an arbitrary external - # fork's PR and this job's write-capable GITHUB_TOKEN, CLAUDE_CODE_OAUTH_TOKEN, - # and -- now that the checkout step below sets allow-unsafe-pr-checkout: true -- - # a checked-out copy of that fork's own code running in this trusted context. - # Drop or loosen this check and the checkout step's "safe because the job is - # already gated to a trusted fork" justification stops being true, turning this - # into a textbook "pwn request" vulnerability. To trust an additional fork, + # fork's PR and this job's write-capable GITHUB_TOKEN and + # CLAUDE_CODE_OAUTH_TOKEN. Drop or loosen this check and any fork can trigger + # a job that runs with this repo's secrets. To trust an additional fork, # EXTEND this condition explicitly (e.g. `|| ... == 'other-trusted-account'`) -- # never replace it with something broader (a wildcard, a check on PR author # instead of head repo owner, etc.). @@ -85,24 +84,26 @@ jobs: echo "decision=$decision" >> "$GITHUB_OUTPUT" echo "gate decision: $decision" - - name: Check out PR head (read-only context) + - name: Check out base branch if: steps.gate.outputs.decision == 'run' + # Deliberately NO ref:/repository: override -- this checks out this + # repo's own base branch (master), not the PR's fork/ref. Checking + # out an untrusted PR ref into the workspace root before this action + # is exactly the anti-pattern anthropics/claude-code-action's own + # docs/security.md warns against; its "preferred" pattern is a plain + # checkout of the base ref, nothing more. claude-code-action fetches + # and reviews the PR's actual content itself, from ITS OWN internal + # logic (see its src/github/operations/branch.ts): for a fork PR it + # fetches origin's refs/pull//head -- a ref GitHub maintains on + # THIS repo for any PR, fork or not, so it never needs direct access + # to the fork's own remote at all. That's why this step must leave + # `origin` pointing at this repo (the default) rather than being + # redirected to the fork: an earlier version of this step did that, + # which broke the action's own internal fetch ("couldn't find remote + # ref pull//head") since that ref doesn't exist on the fork. # Intentionally tracks the major-version tag (not a pinned SHA) so # upstream fixes are picked up automatically. uses: actions/checkout@v4 - with: - repository: ${{ github.event.pull_request.head.repo.full_name }} - ref: ${{ github.event.pull_request.head.sha }} - fetch-depth: 1 - persist-credentials: false - # actions/checkout refuses, by default, to check out a fork PR's head - # under pull_request_target (a "pwn request" guard). Safe to override - # here specifically because BOTH halves of the two-layer defense hold: - # the job above is gated to the trusted-fork check, and this checkout - # is never built/run/sourced -- only fed to the read-only review step - # below. Opting in without both of those would be unsafe; see - # https://gh.io/securely-using-pull_request_target. - allow-unsafe-pr-checkout: true - name: Run Claude Code Review if: steps.gate.outputs.decision == 'run' From e4a126decf5ee322fab58c4b1d19b4044376b8de Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 19:19:55 -0500 Subject: [PATCH 3/3] Fold in #26: check PR author (user.login), not head repo owner head.repo.owner.login only identifies the fork for fork-headed PRs; for an upstream-branch-headed PR (base and head both in this repo) it's always this repo's own org, never the actual author, so the gate silently skipped review on every such PR regardless of who opened it. user.login is GitHub's own authenticated record of who opened the PR and isn't attacker-spoofable, so this isn't a weaker check -- it's the more correct one, and covers both fork-headed and upstream-headed PRs. --- .github/workflows/claude-code-review.yml | 29 ++++++++++++++++-------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 7a7c2c4..cb2d201 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -23,19 +23,30 @@ concurrency: jobs: claude-review: - # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE head.repo.owner.login - # CHECK BELOW !!! It is the ONLY thing standing between an arbitrary external - # fork's PR and this job's write-capable GITHUB_TOKEN and - # CLAUDE_CODE_OAUTH_TOKEN. Drop or loosen this check and any fork can trigger - # a job that runs with this repo's secrets. To trust an additional fork, - # EXTEND this condition explicitly (e.g. `|| ... == 'other-trusted-account'`) -- - # never replace it with something broader (a wildcard, a check on PR author - # instead of head repo owner, etc.). + # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE user.login CHECK + # BELOW !!! It is the ONLY thing standing between an arbitrary external + # PR and this job's write-capable GITHUB_TOKEN and CLAUDE_CODE_OAUTH_TOKEN. + # Drop or loosen this check and any PR can trigger a job that runs with + # this repo's secrets. To trust an additional person, EXTEND this + # condition explicitly (e.g. `|| ... == 'other-trusted-account'`) -- + # never replace it with something broader (a wildcard, a check on a + # label or a comment -- those ARE attacker-controlled on their own PR, + # unlike `user.login`, which is GitHub's own authenticated record of who + # actually opened the PR and can't be spoofed by anyone else). + # + # Deliberately checks the PR AUTHOR (user.login), not + # head.repo.owner.login: the latter only identifies "who owns the fork" + # for fork-headed PRs -- for an upstream-branch-headed PR (base and head + # both in this repo, e.g. from `gh stack`, or `gh pr create` without a + # fork), it's always this repo's own org, never the actual author, so + # that check silently skipped review on every such PR regardless of who + # opened it. `user.login` works correctly for both fork-headed and + # upstream-branch-headed PRs. # # Skip drafts too (don't spend API/CI on unfinished PRs). if: >- github.event.pull_request.draft == false && - github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade' + github.event.pull_request.user.login == 'jnasbyupgrade' runs-on: ubuntu-latest timeout-minutes: 60 permissions: