Skip to content

fix(ci): restrict @claude mention workflow to trusted authors - #174

Merged
MaryWylde merged 1 commit into
devfrom
chore/gate-claude-mention-workflow
Aug 11, 2026
Merged

fix(ci): restrict @claude mention workflow to trusted authors#174
MaryWylde merged 1 commit into
devfrom
chore/gate-claude-mention-workflow

Conversation

@MaryWylde

Copy link
Copy Markdown
Contributor

The problem

claude.yml fired on any text containing @claude. There was no check on who wrote it — and this is our only public repo, with issues enabled.

The part that makes it matter: a fork's pull_request run gets no secrets, which is why the review workflow is safe. But issues and issue_comment events run in this repository's context and do receive repository secrets. So those runs had CLAUDE_CODE_OAUTH_TOKEN sitting in the environment.

And this isn't the usual prompt-injection story. That workflow is built to carry out the instructions in the comment that tagged it. A stranger writing @claude <instructions> isn't sneaking past anything — they're using it as designed. They just aren't us.

Anyone with a GitHub account could open an issue titled @claude ... and get a job running against our Claude subscription, with the token in reach.

The fix

Gates on author_associationOWNER, MEMBER, COLLABORATOR. Those are people with org membership or explicit repo access. CONTRIBUTOR is deliberately excluded: it only means someone had a single commit merged, which is not the same as being trusted with a credential.

The association is checked per event, against the account that actually triggered it. This is the subtle bit — my first attempt was wrong. On issue_comment the payload carries both comment.author_association (the commenter) and issue.author_association (whoever opened the thread). A single shared OR across event types would have let an untrusted account comment @claude ... on an issue one of us opened, and pass the gate. Each event branch now checks its own actor.

Also included

  • timeout-minutes: 15 — there was no timeout, so a wedged run could bill against the Max subscription for the full 6-hour default.
  • A concurrency group per issue/PR, without cancel-in-progress. Unlike a stale PR review, a superseded @claude task is still work somebody asked for, so these queue rather than get killed.
  • Drops id-token: write. That's only needed for OIDC federation; on the static-token path it's dead weight.

Was it abused?

No. I checked every run of this workflow — all legitimate internal use, most recently 2026-06-23. Repo has 0 forks. This is a hole that was open, not one that was used.

Still worth doing separately

  • The same ungated claude.yml is on keepsimple-cms-new, uxcat-api, and library. Those are private and everyone with access is trusted, so the exposure is very different — but the same patch applies.
  • The review workflow's token is shared with other services. Moving CI to a dedicated credential would mean a leak costs a CI token rather than everything else too. Tracked separately.

Test plan

  • This PR triggers the review workflow (pull_request, same-repo) — confirm it still runs green.
  • After merge, comment @claude here as an OWNER and confirm the job still fires.
  • Confirm a CONTRIBUTOR/NONE mention does not start a run.

This repo is public with issues enabled. The job only checked that the text
contained "@claude", so any GitHub user could trigger it by opening an issue.
Unlike a fork's pull_request run, `issues` and `issue_comment` events execute
in this repo's context and DO receive repository secrets, so those runs had
CLAUDE_CODE_OAUTH_TOKEN in the environment. And because the action executes
whatever instructions are in the comment that tagged it, reaching that token
needed no injection trick — just a comment.

Gates on author_association (OWNER / MEMBER / COLLABORATOR) per event, checked
against the account that actually triggered it. CONTRIBUTOR is excluded: it
only means someone had a commit merged once.

The check is deliberately not one shared OR across event types. On
issue_comment the payload carries both comment.author_association and
issue.author_association, so a flat OR would have let an untrusted account
comment on a trusted author's issue and pass.

Also:
- timeout-minutes: 15, so a wedged run cannot bill the subscription for the
  6h default.
- concurrency group per issue/PR, without cancel-in-progress — a superseded
  @claude task is still work someone asked for.
- drops id-token: write, which is only needed for OIDC federation and is dead
  weight on the static-token path.

No evidence of abuse: every run of this workflow to date was a legitimate
internal one, the most recent on 2026-06-23.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown

Claude finished @MaryWylde's task in 1m 14s —— View job


Reviewing claude.yml gating fix

  • Read PR diff (.github/workflows/claude.yml, +38/-5) and prior version
  • Trace if: gating logic per event type (issue_comment, pull_request_review_comment, pull_request_review, issues)
  • Verify concurrency.group expression resolves correctly for every event type
  • Check permissions block (id-token: write removal, timeout-minutes)
  • Post final review

Summary

This is a scoped, correct fix — only .github/workflows/claude.yml changes, so the usual TS/React/AGENTS.md checklist doesn't apply here; this review is about the workflow logic itself.

Verified correct:

  • Each event branch checks its own actor's author_association (comment.author_association for issue_comment/pull_request_review_comment, review.author_association for pull_request_review, issue.author_association for issues) — no cross-event OR that could let an untrusted commenter ride on a trusted issue author's association, as called out in the PR description.
  • contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), …) correctly excludes CONTRIBUTOR/NONE/FIRST_TIME_CONTRIBUTOR.
  • Parens in the multi-branch if: block balance correctly across all four clauses — traced each one.
  • concurrency.group: claude-mention-${{ github.event.issue.number || github.event.pull_request.number }} resolves correctly for all four event payloads: issues/issue_comment carry event.issue.number; pull_request_review/pull_request_review_comment carry event.pull_request.number (no top-level issue on those), so the || fallback lands correctly either way. cancel-in-progress: false matches the stated intent (queue, don't kill, superseded @claude work).
  • timeout-minutes: 15 and dropping id-token: write (unused — auth is via the static CLAUDE_CODE_OAUTH_TOKEN secret, not OIDC) are both reasonable hardening with no downside spotted.
  • permissions: block still grants exactly what claude-code-action needs (contents/pull-requests/issues: read, actions: read for CI status) — nothing over-scoped.

No bugs or security gaps found in the gating logic. No inline comments needed.

One thing worth confirming in the test plan rather than in review: author_association is evaluated from the webhook payload at trigger time, so if someone's collaborator access is revoked between posting a comment and the job actually starting, the stale association would still pass — a narrow TOCTOU window, not something this PR needs to solve, just worth knowing it's not airtight against that edge case.

@MaryWylde
MaryWylde merged commit 4242572 into dev Aug 11, 2026
2 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