Harden Claude workflow permission gate#776
Conversation
WalkthroughThe Claude workflow now authorizes both the event actor and event requester through repository collaborator permissions before running the Claude job. ChangesClaude authorization flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Event as GitHub event
participant Authorization as authorize_claude_actor
participant API as GitHub collaborator API
participant Claude as claude job
Event->>Authorization: provide actor and requester
Authorization->>API: check actor permission
Authorization->>API: check requester permission
API-->>Authorization: return permission levels
Authorization->>Claude: authorize when both checks pass
Claude-->>Event: execute Claude workflow
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR adds a permission gate before the Claude workflow runs. The main changes are:
Confidence Score: 4/5The
.github/workflows/claude.yml
|
| Filename | Overview |
|---|---|
| .github/workflows/claude.yml | Adds the Claude authorization job and downstream gate, with a remaining requester mismatch on assigned issue events. |
Reviews (1): Last reviewed commit: "Harden Claude workflow permission gate" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (context.eventName === 'issues') { | ||
| return context.payload.issue?.user?.login; | ||
| } |
There was a problem hiding this comment.
When an issues: assigned event fires, this returns the original issue author while the gate checks the current issue title/body for @claude. If a read-only user can edit an issue that was opened by a write/admin user, then a later assignment by a write/admin user can pass both permission checks and run Claude on instructions supplied by the read-only editor.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| steps: | ||
| - name: Verify actor can run Claude | ||
| id: permission-gate | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 |
There was a problem hiding this comment.
Nice to pin actions/github-script to a full commit SHA rather than a mutable tag — good supply-chain hygiene. Worth double-checking (outside this sandbox, which blocked outbound gh api calls to other repos) that 3a2844b7e9c422d3c10d287c895573f7108da1b3 is actually the commit tagged v9 upstream, since a stale/incorrect comment here would be misleading if this pin is ever rotated by hand later.
| if (!(await hasWriteAccessFor(actor))) { | ||
| core.setOutput('authorized', 'false'); | ||
| core.setFailed(`Unable to run Claude because ${actor} does not have write/admin repository permission.`); | ||
| return; | ||
| } | ||
|
|
||
| if (!(await hasWriteAccessFor(requester))) { | ||
| core.setOutput('authorized', 'false'); | ||
| core.setFailed( | ||
| `Unable to run Claude because requester ${requester} does not have write/admin repository permission.` | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
core.setFailed marks authorize_claude_actor (and thus the overall workflow run) as failed, not skipped, for every unauthorized @claude mention. On a public repo like this one, any external/non-collaborator who tags @claude in an issue/PR comment will produce a red ❌ workflow run visible in the Actions tab and as a check — which can look like a broken CI job rather than "expected access-denied". If that's intentional (e.g. for auditability), fine, but consider whether core.warning + a clean skip (leaving the job "successful" with authorized=false) would give a less noisy signal for the common case of well-meaning outside contributors.
Review: Harden Claude workflow permission gateOverviewSplits the single Code quality
Suggestions (left as inline comments)
Risks / other notes
Overall: solid, low-risk security hardening. The two notes above are minor/non-blocking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/claude.yml (1)
85-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAuthorization logic is sound; consider not failing the run on denial.
The dual actor+requester gate, fail-closed error handling, and SHA-pinned
github-scriptall look good. One operational nit: callingcore.setFailedon every unauthorized@claudemention marks the whole workflow run as failed (red ❌). On a high-traffic public repo, drive-by mentions from non-collaborators will each produce a failed run and notifications. Since theclaudejob is already gated onauthorized == 'true', you can deny by setting the output tofalseand logging, without failing the job.Note:
getCollaboratorPermissionLevelreturnsnonefor GitHub App bots (they authenticate via installation tokens, not as collaborators), so any bot-authored@clauderequest will be denied here — expected for a human-review gate, but worth confirming no bot-initiated flow relies on it.♻️ Optional: deny without failing the run
if (!requester) { core.setOutput('authorized', 'false'); - core.setFailed('Unable to run Claude because the requester could not be determined.'); + core.warning('Skipping Claude because the requester could not be determined.'); return; } if (!(await hasWriteAccessFor(actor))) { core.setOutput('authorized', 'false'); - core.setFailed(`Unable to run Claude because ${actor} does not have write/admin repository permission.`); + core.warning(`Skipping Claude because ${actor} does not have write/admin repository permission.`); return; } if (!(await hasWriteAccessFor(requester))) { core.setOutput('authorized', 'false'); - core.setFailed( - `Unable to run Claude because requester ${requester} does not have write/admin repository permission.` - ); + core.warning( + `Skipping Claude because requester ${requester} does not have write/admin repository permission.` + ); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/claude.yml around lines 85 - 106, Replace the core.setFailed calls in the requester-missing and unauthorized actor/requester branches with non-failing core.warning or core.info logs while preserving authorized=false and early returns. Keep the existing dual authorization checks and successful authorized output unchanged; confirm bot-initiated flows are not expected to pass this human-review gate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/claude.yml:
- Around line 85-106: Replace the core.setFailed calls in the requester-missing
and unauthorized actor/requester branches with non-failing core.warning or
core.info logs while preserving authorized=false and early returns. Keep the
existing dual authorization checks and successful authorized output unchanged;
confirm bot-initiated flows are not expected to pass this human-review gate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd5ad90b-7c44-438b-a165-607fdd425a53
📒 Files selected for processing (1)
.github/workflows/claude.yml
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
✅ Review App DeletedReview app for PR #776 is deleted |
1 participant
Summary
authorize_claude_actorpermission gate before Claude Code runsChanged workflows
.github/workflows/claude.ymlValidation
actionlintpassed for the changed workflow file(s)Rollout context
This mirrors the verified gate from
shakacode/react_on_rails#4533.Positive path was exercised on
react_on_rails#4534; negative read-only actor attempts onreact_on_rails#4540blocked before the secret-bearing Claude job.Summary by CodeRabbit