Skip to content

feat: pre-job authorization#193

Merged
louisliu2048 merged 1 commit intomainfrom
vui-chee/prevent-outside-trigger
Mar 12, 2026
Merged

feat: pre-job authorization#193
louisliu2048 merged 1 commit intomainfrom
vui-chee/prevent-outside-trigger

Conversation

@Vui-Chee
Copy link
Contributor

No description provided.

@louisliu2048 louisliu2048 merged commit a87bb2f into main Mar 12, 2026
4 checks passed
@Vui-Chee Vui-Chee deleted the vui-chee/prevent-outside-trigger branch March 12, 2026 09:54
Comment on lines +18 to +24
run: |
ASSOCIATION="${{ github.event.comment.author_association || github.event.review.author_association || github.event.issue.author_association }}"
echo "Author association: $ASSOCIATION"
if [[ "$ASSOCIATION" != "OWNER" && "$ASSOCIATION" != "MEMBER" && "$ASSOCIATION" != "COLLABORATOR" ]]; then
echo "Unauthorized: actor '${{ github.actor }}' has association '$ASSOCIATION'"
exit 1
fi
Copy link

Choose a reason for hiding this comment

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

🔴 For issues: assigned events, github.event.issue.author_association reflects the original issue creator's association, not the person who triggered the assignment (github.event.sender). If a team member assigns an issue opened by an external user mentioning @claude, the authorize job sees the external user's association (e.g., NONE) and incorrectly rejects the action. Consider using the GitHub API to resolve github.event.sender.login's association for issues events, or handle assigned separately.

Extended reasoning...

What the bug is

The new authorize job determines the actor's association using a fallback chain:

github.event.comment.author_association || github.event.review.author_association || github.event.issue.author_association

For issues: assigned events, there is no comment or review in the payload, so both of those fields evaluate to empty strings. The expression falls through to github.event.issue.author_association, which is the association of the issue creator, not the person who performed the assignment.

Step-by-step proof

  1. An external user (association = NONE) opens an issue with @claude in the body.
  2. A team MEMBER sees the issue and assigns it (triggering the issues: assigned event).
  3. The GitHub webhook payload for this event has:
    • github.event.sender.login = the team member (the assigner)
    • github.event.issue.author_association = NONE (the external user who created the issue)
    • github.event.comment = null (no comment involved)
    • github.event.review = null (no review involved)
  4. The authorize job evaluates: '' || '' || 'NONE' which resolves to ASSOCIATION='NONE'
  5. The if condition checks NONE != OWNER && NONE != MEMBER && NONE != COLLABORATOR - all true, so exit 1
  6. The authorize job fails, blocking the downstream claude job even though the actual actor is a trusted team member.

Why existing code doesn't prevent this

The fallback chain comment.author_association || review.author_association || issue.author_association correctly handles issue_comment, pull_request_review_comment, and pull_request_review events because in those cases author_association on the comment/review reflects the sender. For issues: opened, it also happens to work because the issue author IS the sender. But for issues: assigned, the sender (assigner) is a different person than the issue author, and there is no field in the fallback chain that captures the assigner's association.

Impact

This bug means that the common workflow of an external contributor opening an issue mentioning @claude and then a team member assigning it will be blocked by the authorization check. The team member's legitimate action is rejected because the wrong principal's association is checked.

How to fix

The fix should resolve the sender's association rather than relying on issue.author_association for issues events. Options include:

  • Using the GitHub API (GET /repos/{owner}/{repo}/collaborators/{username}/permission) to check github.event.sender.login's actual association.
  • Adding a conditional branch that handles the assigned event type differently, e.g., checking github.actor against a known collaborator list or API call.
  • Using github.event.sender information which is always the person who triggered the event.

Vui-Chee added a commit that referenced this pull request Mar 13, 2026
* main: (25 commits)
  fix: bump quinn-proto to 0.11.14 to patch CVE-2026-31812 DoS vuln (#183)
  pre-job authorization (#193)
  fix: trigger review skill failed to ack (#192)
  feat: trigger skill review separately (#191)
  feat: add Claude skills and CLAUDE.md for AI-assisted development (#190)
  rename ext (#185)
  supply workflow for claude (#184)
  feat(builder): incremental trie cache optimization for flashblocks state root (#163)
  chore(flashblocks-rpc): migrate op-reth flashblocks into xlayer-reth (#175)
  Feat(chainspec): adding xlayer-devnet chainspec (#167)
  chore(builder): flatten flashblocks builder, remove unnecessary trait interfaces (#172)
  rpc: remove unnecessary trait bounds and dependencies from XlayerRpcExtApiServer impl (#171)
  fix fmt in bin/tools/gen_genesis.rs (#170)
  fix(builder): Resolve bugs on upstream flashblocks timing scheduler (#169)
  Feat(tools): Add a tool to generate a custom genesis file based on a template and existing chain data (#159)
  feat(flashblocks): Add flashblocks sequence persistence logic on RPC and sequence replay flashblock builder (#162)
  chore(builder): remove unused custom-engine-api feature flag in tests (#168)
  fix: p2p test hang due to hang on port (#165)
  fix: update testcontainers to v0.27.0 to remediate CVE-2025-62518 (#164)
  chore(builder): further clean up builder crate (#161)
  ...
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.

2 participants