ci: gate claude.yml on author_association to skip unauthorized invokes

[cliffhall]

Summary

Tighten the trigger conditions for .github/workflows/claude.yml so the job is skipped at workflow evaluation time when the invoking user does not have write access, rather than starting and later failing when the action tries to perform privileged operations.

Background

We observed a case where someone left an @claude comment on a PR and the workflow started running, then errored out partway through because the commenter didn't have the repo permissions Claude needs to act on their behalf. That's the intended security posture (only trusted users should be able to drive Claude in this repo), but the failure mode wasn't great:

• Red workflow runs cluttered the Actions tab for what is really an authorization rejection, not a real failure.
• Runner minutes were spent spinning up a job that was never going to succeed.
• The user got a confusing in-progress run followed by an opaque error, instead of a clean no-op.

Change

Add an author_association check to each of the four trigger branches (issue_comment, pull_request_review_comment, pull_request_review, issues). The job's if: now requires the author to be OWNER, MEMBER, or COLLABORATOR in addition to the existing event-type and @claude body checks.

author_association is supplied by GitHub on the event payload itself, so this check happens during workflow evaluation — before a runner is allocated. Comments from users without write access become silent no-ops instead of failed runs.

This doesn't change who is allowed to drive Claude (the action itself already enforces this); it just moves the rejection earlier in the pipeline and makes it quiet.

Test plan

• Comment @claude ... from an account with write access on an issue or PR — workflow runs as before.
• Comment @claude ... from an account without write access — workflow is skipped (no run appears in Actions), instead of starting and failing.
• Confirm all four trigger paths (issue_comment, pull_request_review_comment, pull_request_review, issues) behave consistently.

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 57s —— View job

---

Code Review

• Gather context
• Read changed file and analyze diff
• Post review feedback

---

Overall: This is a well-reasoned, correct change. The approach — checking author_association in the job-level if: — is exactly right for moving the rejection earlier in the pipeline. No functional issues found. A few observations below.

---

What's good

• Correct field paths for each event type:
  • issue_comment → github.event.comment.author_association ✓
  • pull_request_review_comment → github.event.comment.author_association ✓
  • pull_request_review → github.event.review.author_association ✓
  • issues → github.event.issue.author_association ✓
• Job-level if: means the runner is never allocated for unauthorized invocations — exactly as described in the PR summary.
• Not including CONTRIBUTOR / FIRST_TIME_CONTRIBUTOR / NONE is the right conservative default.

---

Minor observations

1. issues + assigned edge case

For the assigned event type, github.event.issue.author_association reflects the issue author's association, not the person who performed the assignment. If a non-member opens an issue and a collaborator later assigns it to claude, the job would be skipped even though the assigning action came from a trusted user.

This is arguably acceptable (and the assignee_trigger: "claude" mechanism still works for the opened case), but it's worth being aware of the subtle mismatch.

2. Style: contains(fromJSON(...), ...) is more concise

The three-way OR for each event branch could be replaced with a single contains() call:

contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association)

This reduces duplication across four trigger branches and makes the allowed-roles list easier to update in one place. Not a blocker, just a readability suggestion. Fix this →

3. No test for null/empty author_association

If the author_association field were somehow absent (unlikely in practice — GitHub always populates it), the condition would silently evaluate to false, which is a safe fail-closed behavior. No action needed, just noting it's already safe by accident.

---

Summary

The change is correct and achieves its goal cleanly. The two minor notes (edge case on assigned events, and the contains(fromJSON(...)) style) are non-blocking. Happy to approve as-is.

Note: I'm unable to submit a formal GitHub PR approval due to permission constraints. A human reviewer will need to approve.

[cliffhall]

Thanks for the review. Responding to the three notes:

1. issues + assigned edge case

Good catch, and I think the conservative behavior is actually what we want here. For assigned events, the workflow effectively runs on behalf of the issue's context, not the assigner — so gating on the issue author's association is the safer read. If a non-member opened an issue and a collaborator assigned claude to it, having the job skip is a feature: we don't want to drive Claude through a low-trust issue body just because a trusted user clicked assign. The opened-by-trusted-user path still works as expected, and that's the primary trigger.

I'd leave this branch as-is, but happy to revisit if anyone hits a real workflow they want to enable.

2. contains(fromJSON(...)) style

Agreed this is cleaner — single source of truth for the allowed-roles list, and four fewer repeated || chains. I'll push a follow-up commit on this branch refactoring to:

contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association)

(or the equivalent field per event). Holding off until there's a 👍 on point 1 so I'm not churning the diff.

3. null/empty author_association

Yep — fail-closed is the right default for an auth gate, even if accidental. No change needed.

[BobDickinson]

LGTM