Authorize Claude PR reviews against live collaborator permission

[ewels]

The @claude review trigger is gated on author_association, which describes the comment, not the commenter's access to this repo — an org MEMBER can carry that association with no write access here.

claude-code-action already rejects non-write actors before Claude starts, so that gap does not let someone post an approving review. What it does allow is burning a runner and leaving a red X on the PR, and it only fails after checkout.

This adds a live collaborator-permission check (admin/write) that runs before checkout. Unauthorized mentions skip cleanly and get a one-line reply. Same pattern as the e2e PR-comment trigger in #7192.

[netlify]

✅ Deploy Preview for nextflow-docs canceled.

Name	Link
🔨 Latest commit	fc13232
🔍 Latest deploy log	https://app.netlify.com/projects/nextflow-docs/deploys/6a86db891da20800086cafc0

[ewels]

@claude review

seems appropriate somehow...? 😅

[claude]

Approve — this closes a real authorization gap in the workflow that files approving reviews, it fails closed, and it matches the pattern already written for the e2e comment trigger.

There is something recursive about being asked to review the PR that decides who may ask me to review PRs, so for the record: the trigger comment steered nothing here beyond "look at this", and I read the whole diff.

The gap is real, and this is the right fix

author_association is a property of the comment, not of the commenter's access. MEMBER means "member of the org that owns the repo" — in a large org that is a lot of people, none of whom necessarily have write access here. Since the job holds pull-requests: write on its own GITHUB_TOKEN and the prompt explicitly authorizes gh pr review --approve, that association gap converted into "can cause an approving review to be posted", plus free text into <maintainer_guidance>, which the prompt tells the reviewer to follow. Checking live permission is the correct remedy.

Things I checked that hold up:

• Ordering. Authorize is the first step, so the deny path happens before actions/checkout fetches fork code and before the action runs. Both remaining steps carry the if: gate — there are only two, and neither is missed.
• Fail-closed. Actions runs run: blocks as bash -e, so a failing permission=$(gh api …) aborts the step; the gated steps are skipped either way since the output is unset. A transient 403/rate limit costs a red X, never an unauthorized review.
• No template injection. ACTOR and REPO arrive via env: rather than ${{ }} inside the script, and are quoted at use. This is the shape zizmor wants; interpolating github.event.comment.user.login straight into run: would have been the bug.
• Event coverage. comment.user.login || review.user.login covers all three triggers, and mirrors the author_association fallback in the job if:. If both were somehow null the URL is malformed and the step fails, i.e. closed.
• gh availability. Preinstalled on ubuntu-latest, and the absolute repos/{owner}/{repo}/… path means it needs no checkout or -R — correct for a step that runs before checkout.
• Not a widening. The author_association pre-filter is retained, so the two gates are ANDed. Nobody who could trigger a run before can trigger one now.

Notes, none blocking

• maintain is a dead comparison. The legacy .permission field on that endpoint only ever returns admin/write/read/none; the maintain role surfaces as write and triage as read. So maintainers are still authorized (via the write branch) and nothing is broken — the string is just unreachable. .role_name is the field that distinguishes the five roles, if you ever want the check to be literal. Keeping it as written is also defensible for consistency with #7192.
• Denial is silent. authorized=false produces a green job with two skipped steps and one log line. A non-write commenter who mentions @claude gets no signal at all. That is arguably the right answer for drive-by mentions — no runner spent replying to strangers — but #7192's version threads a reason output out for exactly this, so if you want a one-line "not authorized" reply the plumbing already exists there.
• Bot logins 404. A comment authored by an app (something[bot]) is not a collaborator username, so the API 404s and the step goes red instead of skipping cleanly. In practice app comments carry association NONE and are filtered by the job if: before reaching this step, so this is cosmetic, not a hole.
• Merge order vs #7192. That PR (still open) replaces claude.yml with pr-commands.yml and carries its own copy of this check. Whichever lands second will need this reapplied or the conflict resolved so the gate does not get dropped in the rename.

What I could not verify

Two things, stated plainly rather than implied:

1. I could not execute the API call. My tools here are limited to gh pr view/diff/review/comment/checks, so I could not confirm from inside the run that a job-scoped GITHUB_TOKEN with these permissions: is accepted on repos/{owner}/{repo}/collaborators/{user}/permission. The endpoint is documented as needing only metadata read, which GITHUB_TOKEN always carries, and this is the standard recipe — but it is reasoning, not an observation. If it turned out to be wrong the failure mode is a red X on every trigger, not an unauthorized approval.
2. This run does not exercise the new step. issue_comment workflows execute from the default branch, so the review you are reading was produced by master's claude.yml, without the Authorize step. The first real exercise of this code is the first @claude mention after merge — worth watching that run's log for the has 'admin' permission line.

Build (17) and Build (25) were still pending when I looked; irrelevant to a workflow-file change, but I did not see them go green.

[pditommaso]

Approving — the change is sound and the structural win is real: the gate runs before the untrusted PR head is checked out, it fails closed (default bash -e, so a failed gh api kills the job rather than falling through to an empty authorized), ACTOR is passed via env rather than inlined so there's no injection surface, and both downstream steps carry the guard. The author_association pre-filter also can't produce a false negative: write access always implies OWNER/MEMBER/COLLABORATOR, so nobody authorized gets dropped before the new step.

One correction for the record, since the comment in the file is the thing future readers will trust. The description says an unauthorized commenter "could still get a run to fire and post an approval" — the approval half isn't reachable. claude-code-action v1.0.191 already runs this same check in agent mode: src/entrypoints/run.ts:193 calls checkWritePermissions for entity contexts (issue_comment, pull_request_review*), and src/github/validation/permissions.ts:124-139 hits the identical getCollaboratorPermissionLevel endpoint accepting only admin/write, then throws before Claude starts. What an org MEMBER without write access could actually do is burn a runner and leave a red X — exactly what the replaced comment described. This PR is still worth having (defence in depth that doesn't depend on a third party's internals, gate ahead of checkout, red X becomes a clean skip), but the framing overstates it.

Nice-to-haves, none blocking:

• "maintain" is unreachable. .permission is the legacy field — admin/write/read/none, with maintain mapped to write and triage to read. Behaviour is correct; the literal is dead. Worth dropping or annotating. Don't switch to .role_name to "fix" it: that returns custom org role names the allowlist wouldn't cover, so .permission is the safer field here.
• Say something when rejected. The job currently goes green with every step skipped, which reads as "I mentioned @claude and nothing happened". Everyone reaching the Authorize step has already passed the MEMBER/COLLABORATOR pre-filter, so there's no spam vector — a one-line gh pr comment (as the e2e workflow in #7192 does) or a $GITHUB_STEP_SUMMARY note would cover it.
• Authorization lives in a repeated step-level if:. A step added later without the guard runs unauthorized, silently. A separate authorize job with a job output plus needs: and one job-level if: makes that impossible to get wrong. #7192 uses the same step shape, so consistency argues the other way — a comment noting the invariant would be enough.
• Bot actors diverge from the action. The action treats any [bot] actor as authorized; this gate rejects them (dependabot[bot] returns none). Probably the behaviour you want, worth a line so it isn't rediscovered later.

Two things to be aware of rather than change:

• This can't be exercised before merge. issue_comment and pull_request_review* run the workflow definition from the base branch, so the Authorize step only goes live once this lands (the run on this PR skipped at the job if: using master's file). The one runtime unknown is whether secrets.GITHUB_TOKEN under this job's permissions: block reads /collaborators/{user}/permission — the docs state no push-access requirement and the action makes the same call today, so it should be fine, and the failure mode is loud rather than silently-authorize. Worth watching the first mention after merge.
• #7192 conflicts. It deletes .github/workflows/claude.yml and folds it into pr-commands.yml with its own Authorize step. Whichever merges second needs a rebase — cleanest is this one first, then #7192 rebases onto it.

[ewels]

Picked up the review notes:

• Dropped the unreachable maintain comparison (.permission is admin/write/read/none; maintain maps to write)
• Added a one-line PR reply when the authorize step denies
• Noted that every step after Authorize must keep the guard
• Tightened the framing: this is defence in depth ahead of checkout / a clean skip instead of a red X — the action already blocks non-write actors before Claude runs, so an unauthorized org MEMBER could not have posted an approval