fix: restore authorization check on Claude workflow caller#46
fix: restore authorization check on Claude workflow caller#46danielewood merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restores authorization checks on the Claude workflow to prevent unauthorized users from triggering workflows that expose OAuth token secrets. It also adds a missing CHANGELOG entry for PR #45 which centralized CI workflows.
Changes:
- Add authorization gate checking for OWNER/MEMBER/COLLABORATOR roles in the Claude workflow
- Add CHANGELOG entry and reference link for PR #45
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/claude.yml |
Adds authorization condition to restrict Claude workflow to repository owner, members, and collaborators |
CHANGELOG.md |
Adds missing entry for PR #45 (CI centralization) and corresponding reference link |
| github.event.comment.author_association == 'OWNER' || | ||
| github.event.comment.author_association == 'MEMBER' || | ||
| github.event.comment.author_association == 'COLLABORATOR' |
There was a problem hiding this comment.
The authorization check does not properly handle all event types that trigger this workflow. The condition references github.event.comment.author_association but this field is not available for all event types:
- For
issuesevents, the sender is ingithub.event.issue.userand there is no comment object - For
pull_request_reviewevents, the review author association should be checked viagithub.event.review.author_association
This means the authorization check will always evaluate to false for issues and pull_request_review events (where there's no github.event.comment object), potentially blocking legitimate users while still exposing the OAuth token through the workflow execution context.
The condition should check the appropriate author_association field for each event type, or use a more robust authorization mechanism.
| github.event.comment.author_association == 'OWNER' || | |
| github.event.comment.author_association == 'MEMBER' || | |
| github.event.comment.author_association == 'COLLABORATOR' | |
| ( | |
| (github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') && | |
| ( | |
| github.event.comment.author_association == 'OWNER' || | |
| github.event.comment.author_association == 'MEMBER' || | |
| github.event.comment.author_association == 'COLLABORATOR' | |
| ) | |
| ) || | |
| ( | |
| github.event_name == 'pull_request_review' && | |
| ( | |
| github.event.review.author_association == 'OWNER' || | |
| github.event.review.author_association == 'MEMBER' || | |
| github.event.review.author_association == 'COLLABORATOR' | |
| ) | |
| ) || | |
| ( | |
| github.event_name == 'issues' && | |
| ( | |
| github.event.issue.author_association == 'OWNER' || | |
| github.event.issue.author_association == 'MEMBER' || | |
| github.event.issue.author_association == 'COLLABORATOR' | |
| ) | |
| ) |
This comment has been minimized.
This comment has been minimized.
cba2da8 to
e29978a
Compare
Defense-in-depth: gate secret transmission behind OWNER/MEMBER/ COLLABORATOR check at the caller level, not just the reusable workflow. Check author_association on the correct event object for each trigger type (comment, review, issue) — previously only comment-based events were checked, silently blocking members on review and issue events. Also adds missing CHANGELOG entries for #45 and #46. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e29978a to
f09f3b4
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Notes (non-blocking):
|
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
| github.event.issue.author_association == 'OWNER' || | ||
| github.event.issue.author_association == 'MEMBER' || | ||
| github.event.issue.author_association == 'COLLABORATOR' |
There was a problem hiding this comment.
The authorization check for the issues event type assigned is incorrect. When an issue is assigned (not opened), github.event.sender is the person performing the assignment, but the code checks github.event.issue.author_association, which is the association of the person who originally created the issue. This means an unauthorized user could assign an existing issue that contains @claude in its body/title and trigger the workflow with the OAuth token.
To fix this security issue, you should check github.event.sender's authorization directly. However, GitHub Actions doesn't expose sender.author_association directly. The safest approach is to either:
- Remove the
assignedtrigger type from theissuesevent, keeping onlyopened - Use a separate API call to check the sender's association (more complex)
- Rely only on the
github.event.sender.login == github.repository_ownercheck combined with explicit allowlist checking
Alternatively, if the assigned trigger is not actually needed for the Claude workflow functionality (since Claude is typically triggered by opening issues or commenting, not by assignment), remove it entirely.
| (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || | ||
| (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) | ||
| ) && ( | ||
| github.event.sender.login == github.repository_owner || |
There was a problem hiding this comment.
The check github.event.sender.login == github.repository_owner may not work as intended for organization-owned repositories. For repositories owned by an organization (like sensiblebit/certkit), github.repository_owner is the organization name (e.g., "sensiblebit"), not a user login. A user's login will never equal the organization name, so this check will always evaluate to false for organization repositories.
If the intent is to check if the sender is the repository owner (for user-owned repos) OR an organization member/owner (for org-owned repos), this check only handles the first case. For organization repositories, you should rely on the author_association checks (OWNER, MEMBER, COLLABORATOR) which correctly identify organization members and owners.
Consider removing this redundant check, or document that it's only for potential user-owned forks and has no effect on the main organization repository.
| github.event.sender.login == github.repository_owner || |
Summary
OWNER/MEMBER/COLLABORATORauthorization gate on the Claude workflow caller — prevents OAuth token secret from being transmitted to the reusable workflow for unauthorized usersFlagged by Claude code review on #45.
Test plan
@claudemention from org member still triggers Claude🤖 Generated with Claude Code