CI: fix claude-code-review by removing the fork-checkout step entirely - #28
Conversation
PR #15's allow-unsafe-pr-checkout fix silenced the checkout-refusal error but was fixing the wrong problem. anthropics/claude-code-action's own docs/security.md calls our checkout step's pattern (checking out the untrusted PR ref into the workspace root) the exact anti-pattern to avoid, and shows the fix: a plain checkout of the base ref, nothing more. The action fetches and reads the PR's actual content itself, safely, via GitHub's refs/pull/<n>/head ref on THIS repo -- which is also why our redirecting `origin` to the fork broke the action's own internal fetch ("couldn't find remote ref pull/10/head": that ref only exists on the base repo, not the fork). Confirmed by reading the action's actual source (src/github/operations/branch.ts).
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
@claude pls review |
|
Claude finished @jnasbyupgrade's task in 1m 24s —— View job Reviewing PR #28
ReviewThis is a solid fix and I agree with the root-cause analysis. Correctness of the fix
Caveats
No changes requested — this looks correct and the comments accurately describe the new behavior. |
…-checkout fix now on master)
…-checkout fix now on master)
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/<n>/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.
…ion fetch the PR itself The previous commit's allow-unsafe-pr-checkout fix solved the checkout-refusal error but traded it for a different, worse one: anthropics/claude-code-action fetches and reads a PR's actual content itself (src/github/operations/ branch.ts: for a fork PR it fetches origin's refs/pull/<n>/head, a ref GitHub maintains on the BASE repo for any PR, fork or not). Redirecting the checkout step's `origin` to the fork (via repository:/ref:) breaks that internal fetch, since refs/pull/<n>/head doesn't exist on the fork's own remote -- `fatal: couldn't find remote ref pull/<n>/head`. Caught before it ever hit CI here by checking Postgres-Extensions/extension_tools#28, which hit and fixed the exact same mistake (their PR #15 was the same allow-unsafe-pr-checkout approach; #28 corrected it). anthropics/claude-code-action's own docs/security.md names this checkout pattern (checking out the PR's own untrusted ref into the workspace) as the anti-pattern to avoid in the first place; its preferred pattern is a plain checkout of the base ref, nothing more. Fix: remove the repository:/ref:/allow-unsafe-pr-checkout overrides entirely -- just `uses: actions/checkout@v7` with no inputs, checking out this repo's own base branch. The if: trust gate is unchanged and still load-bearing defense-in-depth, even though the checkout itself is now safe by construction regardless of that check. Same self-verification limitation as before: this PR's own claude-review check runs the OLD workflow from the base branch and can't demonstrate this on itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oggle Three fixes to claude-code-review.yml, folded into this PR alongside the --comment fix since they all touch the review workflow: - The existing checkout step redirected `origin` to the PR's fork via `repository:`/`ref:` + `allow-unsafe-pr-checkout: true`. That breaks anthropics/claude-code-action's own internal PR fetch (it runs `git fetch origin pull/<N>/head`, a ref that only exists on the base repo) with "couldn't find remote ref pull/<N>/head" -- the same bug root-caused and fixed in Postgres-Extensions/extension_tools#28. Fix: drop the override entirely and just check out the base branch; the action fetches the actual PR head itself. - Add `track_progress: true` so a long review posts a live-updating tracking comment instead of staying silent until the whole run finishes (cat_tools PR #69). - Add a `claude-debug` PR label toggle: skips the cost gate and turns on `show_full_output` for a fast, fully-verbose debug iteration instead of a 5-20+ minute wait per attempt (cat_tools PR #64). Ported from cat_tools' current claude-code-review.yml, which already has all three fixes live.
…ion fetch the PR itself The previous commit's allow-unsafe-pr-checkout fix solved the checkout-refusal error but traded it for a different, worse one: anthropics/claude-code-action fetches and reads a PR's actual content itself (src/github/operations/ branch.ts: for a fork PR it fetches origin's refs/pull/<n>/head, a ref GitHub maintains on the BASE repo for any PR, fork or not). Redirecting the checkout step's `origin` to the fork (via repository:/ref:) breaks that internal fetch, since refs/pull/<n>/head doesn't exist on the fork's own remote -- `fatal: couldn't find remote ref pull/<n>/head`. Caught before it ever hit CI here by checking Postgres-Extensions/extension_tools#28, which hit and fixed the exact same mistake (their PR #15 was the same allow-unsafe-pr-checkout approach; #28 corrected it). anthropics/claude-code-action's own docs/security.md names this checkout pattern (checking out the PR's own untrusted ref into the workspace) as the anti-pattern to avoid in the first place; its preferred pattern is a plain checkout of the base ref, nothing more. Fix: remove the repository:/ref:/allow-unsafe-pr-checkout overrides entirely -- just `uses: actions/checkout@v7` with no inputs, checking out this repo's own base branch. The if: trust gate is unchanged and still load-bearing defense-in-depth, even though the checkout itself is now safe by construction regardless of that check. Same self-verification limitation as before: this PR's own claude-review check runs the OLD workflow from the base branch and can't demonstrate this on itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ion fetch the PR itself The previous commit's allow-unsafe-pr-checkout fix solved the checkout-refusal error but traded it for a different, worse one: anthropics/claude-code-action fetches and reads a PR's actual content itself (src/github/operations/ branch.ts: for a fork PR it fetches origin's refs/pull/<n>/head, a ref GitHub maintains on the BASE repo for any PR, fork or not). Redirecting the checkout step's `origin` to the fork (via repository:/ref:) breaks that internal fetch, since refs/pull/<n>/head doesn't exist on the fork's own remote -- `fatal: couldn't find remote ref pull/<n>/head`. Caught before it ever hit CI here by checking Postgres-Extensions/extension_tools#28, which hit and fixed the exact same mistake (their PR #15 was the same allow-unsafe-pr-checkout approach; #28 corrected it). anthropics/claude-code-action's own docs/security.md names this checkout pattern (checking out the PR's own untrusted ref into the workspace) as the anti-pattern to avoid in the first place; its preferred pattern is a plain checkout of the base ref, nothing more. Fix: remove the repository:/ref:/allow-unsafe-pr-checkout overrides entirely -- just `uses: actions/checkout@v7` with no inputs, checking out this repo's own base branch. The if: trust gate is unchanged and still load-bearing defense-in-depth, even though the checkout itself is now safe by construction regardless of that check. Same self-verification limitation as before: this PR's own claude-review check runs the OLD workflow from the base branch and can't demonstrate this on itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR #15's
allow-unsafe-pr-checkout: truefix silenced the checkout-refusalerror but was solving the wrong problem, and traded it for a new one:
after merging, review runs on #10 and #16 failed with a different error,
fatal: couldn't find remote ref pull/10/head.Root cause:
anthropics/claude-code-action's owndocs/security.mdexplicitly names our checkout step's pattern (checking out the PR's own
untrusted ref, from the fork, into the workspace root) as the anti-pattern
to avoid, and its "preferred" fix is a plain checkout of the base ref with
no override. The action fetches and reads the PR's actual content itself
-- confirmed by reading its source (
src/github/operations/branch.ts): fora fork PR it runs
git fetch origin ... pull/<n>/head, a ref GitHubmaintains on the BASE repo for any PR (fork or not), so it never needs
direct access to the fork's remote. Our step redirecting
originto thefork broke that internal fetch, since
refs/pull/<n>/headdoesn't existthere.
Fix: remove the
repository:/ref:/allow-unsafe-pr-checkoutoverridesentirely -- just
uses: actions/checkout@v7with no inputs, checking outthis repo's own base branch. Updated the surrounding comments (the
top-of-file SECURITY note and the job's trust-check warning) to match --
they previously described the now-removed manual fork-checkout.
The
if:trust gate (head.repo.owner.login == 'jnasbyupgrade') isunchanged and still load-bearing defense-in-depth, even though the
checkout itself is now safe by construction regardless of that check.
Confirmed via
gh api search/code+ reading the action's actual source,not guessed. Same
pull_request_targetself-verification limitation asbefore: this PR's own
claude-reviewcheck runs the OLD workflow frommasterand can't demonstrate the fix on itself -- needs a subsequent PRafter merge to confirm.