Skip to content

fix(ci): address Greptile review on Claude Code workflows - #230

Closed
dodeja wants to merge 1 commit into
mainfrom
fix/claude-workflow-permissions
Closed

fix(ci): address Greptile review on Claude Code workflows#230
dodeja wants to merge 1 commit into
mainfrom
fix/claude-workflow-permissions

Conversation

@dodeja

@dodeja dodeja commented May 29, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #229, which was merged before the Greptile/Codex review was addressed. Resolves all four flagged issues.

Changes

# Severity Issue Fix
1 P1 Read-only GITHUB_TOKEN — Claude can authenticate to the AI backend but every attempt to post a comment/review or push code is rejected at runtime pull-requests: write + issues: write in both workflows; contents: write in claude.yml for commits/branches
3 P2 No actor guard — any commenter could trigger claude.yml and consume API quota Restrict to OWNER/MEMBER/COLLABORATOR via author_association on each event type
4 P2 claude-code-action@v1 is a mutable tag — a force-push of a compromised version would run with the OAuth token in scope Pinned to commit 787c5a0 (= v1.0.133) with a version comment, per GitHub's third-party action hardening guide

Deliberately not applied

Issue 2 (P2 — "duplicate actions: read"): kept additional_permissions: actions: read in claude.yml. The job-level permissions block scopes the GITHUB_TOKEN; additional_permissions is claude-code-action's documented opt-in to actually enable CI-reading behavior. They serve different purposes — the upstream example declares both — so removing it would be a regression, not a cleanup.

Verification

  • Both files validated as YAML.

🤖 Generated with Claude Code

Greptile Summary

This PR hardens the two Claude Code GitHub Actions workflows by fixing the GITHUB_TOKEN permission scope, adding an actor guard to the comment-triggered workflow, and pinning the claude-code-action reference to a specific commit hash to prevent supply-chain drift.

  • Permissions: Both workflows now declare pull-requests: write and issues: write; claude.yml additionally gets contents: write so Claude can push commits and open branches.
  • Actor guard (claude.yml): The if condition now gates each event type on author_association being OWNER, MEMBER, or COLLABORATOR, preventing arbitrary commenters from invoking Claude and consuming API quota.
  • Pinning: Both usages of anthropics/claude-code-action are pinned to the full SHA 787c5a0ce96a9a6cfb050ea0c8f4c05f2447c251 (v1.0.133) instead of the mutable @v1 tag.

Confidence Score: 4/5

Safe to merge — the permission and pinning fixes are correct, and the actor guard closes the main quota-abuse vector for comment-triggered workflows.

The actor guard in claude.yml uses github.event.issue.author_association for the issues.assigned event path, which reflects the issue creator's role rather than the person doing the assignment. A trusted collaborator silently fails to route an external user's @claude-tagged issue via assignment. This is a minor logic mismatch with the stated intent of the guard, but it does not open a security hole and workarounds exist (use a comment instead).

.github/workflows/claude.yml — specifically the issues.assigned branch of the if condition.

Important Files Changed

Filename Overview
.github/workflows/claude-code-review.yml Permissions upgraded to pull-requests: write and issues: write, and action pinned to commit hash — changes are correct for enabling PR review comments.
.github/workflows/claude.yml Actor guard added via author_association checks, permissions expanded, and action pinned to commit hash; guard field for issues.assigned events checks the issue author rather than the assigning actor.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GitHub Event] --> B{Event type?}

    B -->|pull_request| C[claude-code-review.yml]
    C --> D[Runs on all repo PRs]
    D --> E[claude-code-action at 787c5a0]
    E --> F[Posts review via pull-requests write]

    B -->|issue_comment or review_comment or review| G{contains @claude?}
    B -->|issues opened or assigned| H{contains @claude?}

    G --> I{author_association trusted?}
    H --> J{issue.author_association trusted?}

    I -->|Yes| K[claude.yml runs]
    I -->|No| L[Skipped]
    J -->|Yes| K
    J -->|No| L

    K --> M[claude-code-action at 787c5a0]
    M --> N[Claude responds with contents write]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/workflows/claude.yml:21
**`issues.assigned` guard checks the wrong actor**

For `issues` events with type `assigned`, `github.event.issue.author_association` is the association of the **issue creator**, not the person performing the assignment (`github.actor`). In practice, only collaborators can assign issues on GitHub, so the risk is low — but the intent of the guard ("restrict who can invoke Claude") is better expressed by checking the actor who triggered the assignment. If a trusted collaborator tries to route an external user's issue (which happens to contain `@claude`) to Claude via assignment, the workflow silently does nothing because the issue author is `NONE`/`CONTRIBUTOR`. Using `github.actor` and checking it against the trusted-association list (or against team membership) would match the declared intent of the guard.

Reviews (1): Last reviewed commit: "fix(ci): address Greptile review on Clau..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Follow-up to #229, which merged before the review was addressed.

- Grant write scopes so Claude can actually post back via GITHUB_TOKEN:
  pull-requests/issues:write in both workflows, plus contents:write in
  claude.yml for commits/branches (P1).
- Restrict claude.yml to trusted actors (OWNER/MEMBER/COLLABORATOR) so
  outside accounts can't invoke Claude and burn API quota (P2).
- Pin claude-code-action from the mutable @v1 tag to commit SHA
  787c5a0 (v1.0.133) per GitHub's third-party action hardening guide (P2).

Kept `additional_permissions: actions: read` in claude.yml: it is the
action's documented opt-in for reading CI results, not a true duplicate
of the job-level scope (upstream's example declares both).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented May 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api Ready Ready Preview, Comment May 29, 2026 7:32pm

Request Review

(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude') && contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association)) ||
(github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude') && contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association)) ||
(github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude') && contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.review.author_association)) ||
(github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')) && contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.issue.author_association))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 issues.assigned guard checks the wrong actor

For issues events with type assigned, github.event.issue.author_association is the association of the issue creator, not the person performing the assignment (github.actor). In practice, only collaborators can assign issues on GitHub, so the risk is low — but the intent of the guard ("restrict who can invoke Claude") is better expressed by checking the actor who triggered the assignment. If a trusted collaborator tries to route an external user's issue (which happens to contain @claude) to Claude via assignment, the workflow silently does nothing because the issue author is NONE/CONTRIBUTOR. Using github.actor and checking it against the trusted-association list (or against team membership) would match the declared intent of the guard.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/claude.yml
Line: 21

Comment:
**`issues.assigned` guard checks the wrong actor**

For `issues` events with type `assigned`, `github.event.issue.author_association` is the association of the **issue creator**, not the person performing the assignment (`github.actor`). In practice, only collaborators can assign issues on GitHub, so the risk is low — but the intent of the guard ("restrict who can invoke Claude") is better expressed by checking the actor who triggered the assignment. If a trusted collaborator tries to route an external user's issue (which happens to contain `@claude`) to Claude via assignment, the workflow silently does nothing because the issue author is `NONE`/`CONTRIBUTOR`. Using `github.actor` and checking it against the trusted-association list (or against team membership) would match the declared intent of the guard.

How can I resolve this? If you propose a fix, please make it concise.

@dodeja

dodeja commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Closing: superseded by #228, which rewrote both Claude workflows on main and already addresses the Greptile P1/P2 findings this PR targeted:

  • contents/pull-requests/issues: write permissions (P1)
  • author_association actor guard restricting triggers to OWNER/MEMBER/COLLABORATOR (P2)
  • plus a claude[bot] guard, concurrency, and an auth-secret check.

This PR was also based on a stale main and conflicts with #228's structure. The one Greptile item #228 didn't cover — pinning claude-code-action@v1 to a commit SHA — will be handled in a fresh PR cut from current main.

@dodeja dodeja closed this May 29, 2026
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