Skip to content

Fix claude-code-review.yml trust gate: check PR author, not head repo owner - #6

Closed
jnasbyupgrade wants to merge 1 commit into
mainfrom
fix/claude-review-trust-gate-author
Closed

Fix claude-code-review.yml trust gate: check PR author, not head repo owner#6
jnasbyupgrade wants to merge 1 commit into
mainfrom
fix/claude-review-trust-gate-author

Conversation

@jnasbyupgrade

Copy link
Copy Markdown
Contributor

The claude-review job's trust gate checked:

if: >-
  github.event.pull_request.draft == false &&
  github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade'

head.repo.owner.login only identifies who owns the fork for fork-headed
PRs. For an upstream-branch-headed PR (base and head both live in this
repo -- required by gh stack, and also just what you get from gh pr create without a fork), head.repo.owner.login is 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.

Fix: check the PR author instead:

if: >-
  github.event.pull_request.draft == false &&
  github.event.pull_request.user.login == 'jnasbyupgrade'

user.login can't be spoofed by a third party any more than head repo owner
can, and it's the more direct question for this gate's actual purpose:
trusting the person asking for review, not the repository their branch
happens to live in. Works for both fork-headed and upstream-branch-headed
PRs.

Also updated the adjacent SECURITY-CRITICAL comment (which explains why this
check is the only thing making allow-unsafe-pr-checkout: true safe below)
to describe the author check instead of the now-removed owner check.

… owner

github.event.pull_request.head.repo.owner.login only identifies who owns the
fork for fork-headed PRs. For an upstream-branch-headed PR (base and head
both live in this repo, as required by gh stack, or just what gh pr create
produces without a fork), head.repo.owner.login is 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.

Check github.event.pull_request.user.login instead: it identifies the PR
author directly, can't be spoofed by a third party any more than head repo
owner can, and works for both fork-headed and upstream-branch-headed PRs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 429a92bd-fef6-47ea-9b4a-5c7f2584973e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

jnasbyupgrade added a commit that referenced this pull request Aug 7, 2026
Folds in #6.

head.repo.owner.login only distinguishes fork ownership for fork-headed
PRs. For an upstream-branch-headed PR (base and head both in this repo --
what `gh stack` requires, or any `gh pr create` without forking), head.repo
is always this repo itself, so head.repo.owner.login is always the org,
never the actual author -- silently skipping review on PRs that were
legitimately the trusted account's own work (confirmed elsewhere via the
Checks API reporting claude-review as "skipped" on a trusted upstream-branch
PR stack). Check github.event.pull_request.user.login instead: it can't be
spoofed by a third party any more than head repo owner can, and it's the
more direct question for this gate's actual purpose -- trusting the PERSON
asking for a review, not the repository their branch happens to live in.

Applied on top of this PR's already-restructured `if:` (the claude-debug
label clause), rather than as PR #6's standalone diff against the original
file, since both touch the same line and #6 was opened against the
pre-PR-3 version of this workflow.
@jnasbyupgrade

Copy link
Copy Markdown
Contributor Author

Folded into #3 (which had already restructured this same job's if: condition for the claude-debug label toggle, so applying this as its own diff against the original file would've conflicted). Closing in favor of #3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant