From 139d1bbf93d38fb86ad16fbb222cc5408865e234 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 18:53:51 -0500 Subject: [PATCH 1/2] Fix claude-code-review.yml trust gate: check PR author, not head repo owner head.repo.owner.login only identifies the fork owner for fork-headed PRs. For an upstream-branch-headed PR (base and head both in this repo, as required by gh stack or produced by a plain gh pr create without a fork), it's always this repo's own org, never the actual PR author -- so the gate silently skipped review on every such PR regardless of who opened it. Switch to github.event.pull_request.user.login, which is the PR's actual author and can't be spoofed any more than head repo owner can, and covers both fork-headed and upstream-branch-headed PRs correctly. Also updates the SECURITY-CRITICAL comment above the condition so it references the new field instead of describing the old, now-wrong one. --- .github/workflows/claude-code-review.yml | 45 +++++++++++++++--------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index f8aff0a..800e63f 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -6,13 +6,15 @@ name: Claude Code Review # `pull_request` version never worked for fork PRs. # # SECURITY: pull_request_target runs in the BASE repo with secrets and a -# 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. 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. +# write-capable token. The job is gated to PRs authored by jnasbyupgrade only +# — github.event.pull_request.user.login is the PR's original author and +# can't be spoofed by PR content, so this check holds regardless of whether +# the PR head lives in this repo or an external fork. The workflow file +# always comes from the base branch (master), so a PR 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,19 +25,28 @@ concurrency: jobs: claude-review: - # Trusted fork only, and skip drafts (don't spend API/CI on unfinished PRs). + # jnasbyupgrade's own PRs only, and skip drafts (don't spend API/CI on + # unfinished PRs). # - # !!! 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 + # actor's 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. NOTE: this used to + # check head.repo.owner.login (the owner of the fork the PR head lives + # in), but that only distinguishes forks -- for an upstream-branch-headed + # PR (base and head both in this repo, e.g. from `gh stack` or a plain + # `gh pr create` without a fork) it's always this repo's own org, + # regardless of who actually opened the PR, so it silently skipped review + # on every such PR. github.event.pull_request.user.login is the PR's + # actual author and can't be spoofed by PR content either, and it + # correctly covers both fork-headed and upstream-branch-headed PRs. To + # trust an additional author, EXTEND this condition explicitly (e.g. + # `|| ... == 'other-trusted-account'`) -- never replace it with something + # broader (a wildcard, etc.). 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: From fc1d1b9c2d7ba472f6ae51d1e07bee9e281637f3 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 18:57:55 -0500 Subject: [PATCH 2/2] CI: restore persist-credentials: false on claude-code-review.yml's checkout step The fork-checkout fix (removing the repository:/ref: override so this step checks out the base branch instead of an untrusted PR ref) was applied by deleting the whole with: block under actions/checkout, which also silently dropped persist-credentials: false. This job's permissions include pull-requests: write, a real write-capable credential; nothing here legitimately runs git push (review comments post via the API/ claude-code-action, not git), so there's no reason to leave that credential sitting in .git/config for the rest of the job to misuse if anything later goes wrong. --- .github/workflows/claude-code-review.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 800e63f..60d8f98 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -120,6 +120,14 @@ jobs: # Intentionally tracks the major-version tag (not a pinned SHA) so # upstream fixes are picked up automatically. uses: actions/checkout@v7 + with: + # This job's permissions include pull-requests: write, a real + # write-capable credential -- nothing here legitimately runs `git + # push` (review comments post via the API/claude-code-action, not + # git), so there's no reason to leave that credential sitting in + # .git/config for the rest of the job to misuse if anything later + # goes wrong. + persist-credentials: false - name: Run Claude Code Review if: steps.gate.outputs.decision == 'run'