Skip to content

Harden Claude workflow permission gate - #32

Merged
justin808 merged 1 commit into
masterfrom
codex/harden-claude-permission-gate
Jul 11, 2026
Merged

Harden Claude workflow permission gate#32
justin808 merged 1 commit into
masterfrom
codex/harden-claude-permission-gate

Conversation

@justin808

Copy link
Copy Markdown
Member

Summary

  • add a workflow-level authorize_claude_actor permission gate before Claude Code runs
  • require both the workflow actor and original Claude requester to have write/admin repository permission
  • keep the existing Claude trigger condition on the authorization job and only run the secret-bearing Claude job when authorized

Changed workflows

  • .github/workflows/claude.yml

Validation

  • actionlint passed 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 on react_on_rails#4540 blocked before the secret-bearing Claude job.

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@justin808, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a4731bbd-ae2f-4cbe-9872-ff1a82638016

📥 Commits

Reviewing files that changed from the base of the PR and between 6594da2 and 317ef4c.

📒 Files selected for processing (1)
  • .github/workflows/claude.yml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/harden-claude-permission-gate

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.

@greptile-apps

greptile-apps Bot commented Jul 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a workflow permission gate before Claude runs. The main changes are:

  • A new authorize_claude_actor job before the Claude job.
  • Repository write/admin checks for the workflow actor and inferred requester.
  • A gated Claude job that only runs when authorization succeeds.

Confidence Score: 4/5

The changed workflow has a contained authorization regression on assigned issue events.

  • The secret-bearing Claude job is gated and fails closed.
  • Assigned issue events can use the original issue author as the requester instead of the maintainer who triggered the assignment.
  • That can block legitimate maintainer-triggered Claude runs on issues opened by external users.

.github/workflows/claude.yml

Important Files Changed

Filename Overview
.github/workflows/claude.yml Adds a pre-flight authorization job and gates the Claude job on its result, with one requester-inference bug on assigned issue events.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Event contains @claude] --> B[authorize_claude_actor]
  B --> C[Infer requester]
  C --> D{Actor has write/admin?}
  D -->|No| E[Block Claude]
  D -->|Yes| F{Requester has write/admin?}
  F -->|No| E
  F -->|Yes| G[authorized = true]
  G --> H[Run Claude job]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
  A[Event contains @claude] --> B[authorize_claude_actor]
  B --> C[Infer requester]
  C --> D{Actor has write/admin?}
  D -->|No| E[Block Claude]
  D -->|Yes| F{Requester has write/admin?}
  F -->|No| E
  F -->|Yes| G[authorized = true]
  G --> H[Run Claude job]
Loading

Reviews (1): Last reviewed commit: "Harden Claude workflow permission gate" | Re-trigger Greptile


if (context.eventName === 'issues') {
return context.payload.issue?.user?.login;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Assigned Issues Use Original Author

When an issues: assigned event fires, context.actor is the assigner but this branch makes the requester the original issue author. If a maintainer adds @claude or assigns an external user's issue to run Claude, the actor can pass while the requester check fails, so the workflow blocks a maintainer-triggered Claude run.

Context Used: CLAUDE.md (source)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review: Harden Claude workflow permission gate

Overview: Adds a new authorize_claude_actor job that runs before the secret-bearing claude job, requiring both the triggering actor and the original @claude requester to have write/admin repo permission via getCollaboratorPermissionLevel. The claude job now gates on needs.authorize_claude_actor.outputs.authorized == 'true'. Mirrors a pattern already validated in shakacode/react_on_rails#4533/#4534/#4540.

Strengths

  • Fail-safe default: hasWriteAccessFor catches API errors (e.g. non-collaborator → 404) and treats them as "no access," so failures deny rather than accidentally allow.
  • Checks both actor and requester: correctly handles the issues: assigned case, where the person assigning (actor) could differ from the original issue author (requester) whose body text is what actually gets executed as a prompt — preventing a low-privilege user's issue body from running just because a maintainer assigned it.
  • Least privilege: the new job only requests contents: read, and the secret-bearing claude job/checkout is now unreachable unless authorization succeeds — so unauthorized triggers never reach the step with CLAUDE_CODE_OAUTH_TOKEN.
  • Job dependency wiring (needs + if: needs...outputs.authorized == 'true') is correct: a skipped authorize_claude_actor (condition not met) still resolves to authorized != 'true', so claude is skipped too, preserving the original trigger-gating behavior.

Suggestions / minor issues

  1. Sequential awaits could be parallelized (.github/workflows/claude.yml:91-103): hasWriteAccessFor(actor) and hasWriteAccessFor(requester) run sequentially. Not a real performance concern at this scale (two quick REST calls), but Promise.all would shave one round trip and read a bit cleaner.
  2. Verify the pinned SHA before merging (.github/workflows/claude.yml:28): actions/github-script@3a2844b7... is pinned by full SHA, which is good practice (and better than the tag-only pinning used elsewhere in this repo's other workflows), but the # v9 comment is only a human-readable label — GitHub doesn't validate it matches the tag. Worth double-checking the SHA resolves to the real actions/github-script v9 release before merging, since this is exactly the kind of security-hardening PR where a subtly wrong pin would be easy to miss.
  3. core.setFailed on every unauthorized attempt (.github/workflows/claude.yml:87-102): this marks the job/workflow run as failed (red ❌) for any non-collaborator who mentions @claude, which is fine for the security goal but may be noisy for repos with many external contributors (failure notifications/status checks on unrelated issues/PRs). Consider whether a neutral/skipped outcome (e.g. core.notice + non-failing exit) is preferable to a hard failure for the common "external contributor tagged @claude" case — purely a UX/noise consideration, not a security issue.

No test coverage exists for this logic (understandably, since it's a GitHub Actions script), but the PR description notes it was validated against a live positive/negative case in react_on_rails, which is reasonable given the constraints.

Overall this is a solid, well-reasoned hardening change with correct fail-safe semantics.

Comment on lines +91 to +103
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;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: these two permission checks run sequentially (await hasWriteAccessFor(actor) then await hasWriteAccessFor(requester)). Not a real problem at this scale, but Promise.all([hasWriteAccessFor(actor), hasWriteAccessFor(requester)]) would save a round trip when actor !== requester.

steps:
- name: Verify actor can run Claude
id: permission-gate
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good practice pinning by full commit SHA rather than tag. Please double check this SHA actually resolves to actions/github-script v9 before merging — the # v9 suffix is just a comment and isn't validated by GitHub, and this workflow is specifically the security-hardening path where a wrong/stale pin would be easy to overlook.

@justin808
justin808 merged commit d9c9dc6 into master Jul 11, 2026
16 of 18 checks passed
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