Add Claude GitHub Actions workflows #2
Conversation
- claude-code-review.yml: Automatic PR code review for org members - claude.yml: Responds to @claude mentions in issues and PR comments Co-Authored-By: Claude Opus 4.5 <[email protected]>
ajpallares
left a comment
ajpallares
left a comment
There was a problem hiding this comment.
I like it! One question though, to be extra safe 🔒
.github/workflows/claude.yml
Outdated
| github.event.issue.author_association == 'MEMBER' || | ||
| github.event.issue.author_association == 'OWNER' || |
There was a problem hiding this comment.
Does this mean that anyone outside the org can trigger a claude code review on a PR created by anyone in the org?
I wonder, should this condition be under &&?
github.event.issue.author_association == 'MEMBER' ||
github.event.issue.author_association == 'OWNER'
There was a problem hiding this comment.
Not sure I follow. Just updated this to be only MEMBER, which btw means member of the org. Owner is owner of the repository.
How can someone trigger a code review?
There was a problem hiding this comment.
I was wondering: say I open a PR:
github.event.issue.author_association == 'MEMBER' --> `true`
Then someone outside of the org posts a comment in that PR mentioning @claude:
github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude'))` --> `true`
Which means the @claude review is triggered.
Wouldn't it be better if we did:
(
github.event.issue.author_association == 'MEMBER' &&
(
github.event.comment.author_association == 'MEMBER' ||
github.event.review.author_association == 'MEMBER'
)
) && (
(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) ||
(github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) ||
(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')))
To make sure that case above can't happen? Maybe I'm not understanding it right
There was a problem hiding this comment.
Ah I see. I think what would happen in a comment is that github.event.issue doesn't exist. The content of event depends on the type of event that triggers the workflow. So in a comment by an external contributor, github.event.comment.author_association == 'MEMBER' won't be true and it won't even start anything
There was a problem hiding this comment.
but let me confirm, because issue_comment might actually have both issue and comment?
There was a problem hiding this comment.
in any case I believe this covers us. https://github.com/anthropics/claude-code-action/blob/main/docs/security.md#access-control
Repository Access: The action can only be triggered by users with write access to the repository
⚠️ Non-Write User Access (RISKY): The allowed_non_write_users parameter allows bypassing the write permission requirement. This is a significant security risk and should only be used for workflows with extremely limited permissions (e.g., issue labeling workflows that only have issues: write permission).
There was a problem hiding this comment.
43cb87d added this to be extra safe, this change prevents issue_comments events from outsiders from evaluating to true
tonidero
left a comment
tonidero
left a comment
There was a problem hiding this comment.
I think this looks great! Excited to get this running 🙌
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, ready_for_review, reopened] |
There was a problem hiding this comment.
Should we only ask for reviews when it's ready_for_review?
There was a problem hiding this comment.
also, I believe ready_for_review is only triggered when changed from draft to ready
There was a problem hiding this comment.
I filtered draft PRs further down
| claude-review: | ||
| # Only allow org members and owners to trigger Claude reviews | ||
| if: | | ||
| github.event.pull_request.author_association == 'MEMBER' |
There was a problem hiding this comment.
I know before we were also checking whether the author was an "OWNER". Since this seems to be comparing a string directly, would it also work for owners with this? I guess we can always merge this and test it, and if it doesn't work add it back.
There was a problem hiding this comment.
That was my thinking for having both. But by my searches it looks like Owners are also Members, but you're right this is just a string comparison. In any case it's owners of the repository, which is mostly no one that actively works on the repositories.
ajpallares
left a comment
ajpallares
left a comment
There was a problem hiding this comment.
Very cool! Looking forward to using this in our PRs! 🤖
Moved from RevenueCat/purchases-android#3011