Uh oh!
There was an error while loading. Please reload this page.
CI: fix claude-review failing at checkout on every fork PR - #21
Closed
jnasbyupgrade wants to merge 3 commits into
Closed
CI: fix claude-review failing at checkout on every fork PR#21jnasbyupgrade wants to merge 3 commits into
jnasbyupgrade wants to merge 3 commits into
Conversation
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.
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 |
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.
…d 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.
This was referenced Aug 7, 2026
jnasbyupgrade
commented
Aug 7, 2026
ContributorAuthor
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
claude-reviewcurrently fails for every PR from this fork at the checkout step (actions/checkout's "pwn request" refusal onpull_request_target).Updated (the original fix here was wrong):
allow-unsafe-pr-checkout: trueplus keeping therepository:/ref:override that redirectsoriginto the fork silences the checkout refusal but trades it for a different failure —claude-code-actioninternally fetchesrefs/pull/<n>/headfrom this repo to read the PR's actual content, and that ref doesn't exist on the fork onceoriginis redirected there.Postgres-Extensions/extension_tools#28hit and fixed the identical mistake.The actual fix: remove the checkout override entirely.
claude-code-actionfetches and reads the PR's content itself; this step only needs to check out the base branch (noref:/repository:, noallow-unsafe-pr-checkout).with:overrides.if:condition — still load-bearing defense-in-depth even though the checkout itself is now safe by construction.Same
pull_request_targetself-verification limitation as before: this PR's ownclaude-reviewcheck runs the OLD workflow frommasterand can't demonstrate the fix on itself. Verification happens on a subsequent PR after this merges.🤖 Generated with Claude Code
Likely trivial conflict with #15: both touch the same checkout step (#15 bumps
@v4->@v7; this PR removes thewith:block below it). Resolve by keeping@v7with nowith:overrides.Folded in #26: also fixes the trust gate itself to check
user.login(PR author) instead ofhead.repo.owner.login. The latter only identifies the fork for fork-headed PRs -- for an upstream-branch-headed PR (base and head both in this repo, e.g. the several in this same effort pushed directly toPostgres-Extensions/object_reference), it's always this repo's own org, never the actual author, so review was being silently skipped on every one of those regardless of who opened it.user.loginisn't a weaker check -- it's GitHub's own authenticated record of who opened the PR, not attacker-spoofable -- and it's correct for both fork-headed and upstream-headed PRs. #26 is superseded by this and can be closed.