-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use shared claude-review reusable workflow #5270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
leighmcculloch
wants to merge
1
commit into
stellar:master
Choose a base branch
from
leighmcculloch:claude-review-reusable
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+8
−128
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,137 +1,17 @@ | ||
| # SECURITY NOTE | ||
| # ───────────── | ||
| # This workflow uses `pull_request_target` rather than `pull_request` so that | ||
| # fork PRs from org members can be reviewed by Claude. `pull_request_target` | ||
| # runs in the context of the BASE repository, which means: | ||
| # | ||
| # 1. `secrets.ANTHROPIC_API_KEY` is available even when the PR comes from a | ||
| # fork. With `pull_request` this secret would be absent on fork PRs and | ||
| # the action would fail. | ||
| # 2. The workflow definition that runs is the one on the base branch, NOT | ||
| # the version in the PR. A fork therefore cannot edit this file to | ||
| # bypass the author-association gate or exfiltrate secrets by editing | ||
| # the workflow itself. | ||
| # | ||
| # However, `pull_request_target` is also the source of GitHub's well-known | ||
| # "pwn request" class of vulnerabilities. The risks specific to this file: | ||
| # | ||
| # A. We check out `github.event.pull_request.head.sha` below into an | ||
| # isolated `pr-head/` subdirectory, NOT the workspace root. The base | ||
| # ref is checked out at the workspace root instead. This follows | ||
| # claude-code-action's recommended pattern for `pull_request_target` | ||
| # (see https://github.com/anthropics/claude-code-action/blob/main/docs/security.md): | ||
| # tools that consult repo-local config (.git/config, .git/hooks, | ||
| # .gitignore, .npmrc, Makefile, pre-commit hooks, etc.) at the | ||
| # workspace root see only trusted base-branch files, while Claude | ||
| # can still read the PR's files via `--add-dir pr-head`. | ||
| # | ||
| # The PR head is still attacker-controlled code, so any future step | ||
| # that executes, sources, or interprets files from `pr-head/` (build | ||
| # scripts, package install hooks, test runners) would run with | ||
| # access to the secrets injected into this job. Treat any new step | ||
| # added below that touches `pr-head/` as if it were running attacker | ||
| # code with secrets in scope. | ||
| # | ||
| # B. The mitigation is the `if:` gate: only PRs whose author_association | ||
| # is MEMBER (member of the org that owns this repo) or OWNER (the repo | ||
| # owner) get this far. `author_association` is set by GitHub from the | ||
| # author's relationship to the repo at event time and cannot be forged | ||
| # from the PR. A compromised org-member account would defeat this | ||
| # gate, which is the residual risk we are accepting — same trust | ||
| # boundary as merge access. | ||
| # | ||
| # C. COLLABORATOR (outside collaborators invited to this repo) and | ||
| # CONTRIBUTOR (anyone who has previously had a commit merged) are | ||
| # intentionally NOT allowed. CONTRIBUTOR in particular is dangerous: | ||
| # a single merged typo fix would otherwise grant a stranger the | ||
| # ability to run code with secrets. | ||
| # | ||
| # D. The PR head is re-evaluated on every `synchronize` event, so an | ||
| # org member cannot open a benign PR, get it approved for review, | ||
| # and then push malicious commits afterward — each push re-runs | ||
| # the gate. But note: the gate is on the AUTHOR, not the pusher. | ||
| # If a malicious actor gains write access to a fork owned by an | ||
| # org member, they can push to that fork's PR branch and trigger | ||
| # this workflow. This is the same trust model as the rest of CI. | ||
| # | ||
| # E. `permissions:` is scoped to the minimum needed (contents: read, | ||
| # pull-requests: write, id-token: write). Do not broaden without | ||
| # reconsidering the threat model — `contents: write` here would | ||
| # let attacker-controlled code in the head ref push to the base | ||
| # repo. | ||
| # | ||
| # Before adding ANY new step to this job, ask: does it execute, source, | ||
| # or interpret files from the checked-out PR head? If yes, the secrets | ||
| # in this job's environment are exposed to whatever that step does. | ||
| name: Claude Review | ||
|
|
||
| on: | ||
| pull_request_target: | ||
| types: [opened, synchronize, ready_for_review, reopened] | ||
|
|
||
| # Revoke all default GITHUB_TOKEN permissions at the workflow level. Each job | ||
| # below must explicitly opt in to whatever it needs. This is defense in depth: | ||
| # if a future job is added without its own `permissions:` block it inherits | ||
| # nothing, rather than whatever the repo or org default happens to be. | ||
| permissions: {} | ||
| types: [ready_for_review, synchronize] | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number }} | ||
| group: claude-review-${{ github.event.pull_request.number }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: {} | ||
|
|
||
| jobs: | ||
| review: | ||
| if: | | ||
| github.event.pull_request.draft == false && | ||
| (github.event.pull_request.head.repo.fork == false || | ||
| github.event.pull_request.author_association == 'MEMBER' || | ||
| github.event.pull_request.author_association == 'OWNER') | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| id-token: write | ||
| steps: | ||
| # Check out the BASE ref at the workspace root. This is trusted code from | ||
| # the base branch, so it's safe for the action to operate against (e.g. | ||
| # reading .git/config, .git/hooks, etc. that the action and its tools | ||
| # consult). Do NOT check out the PR head here — see security note above. | ||
| - uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 1 | ||
|
|
||
| # Check out the PR head into an isolated subdirectory. The action is told | ||
| # about it via `--add-dir` below so Claude can read the PR's files, but | ||
| # any attacker-controlled config (.git/config, .git/hooks, etc.) inside | ||
| # this subdirectory is NOT picked up by tools running at the workspace | ||
| # root. See: https://github.com/anthropics/claude-code-action/blob/main/docs/security.md | ||
| - uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 1 | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| path: pr-head | ||
|
|
||
| - uses: anthropics/claude-code-action@v1 | ||
| with: | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| track_progress: true | ||
| prompt: | | ||
| REPO: ${{ github.repository }} | ||
| PR NUMBER: ${{ github.event.pull_request.number }} | ||
|
|
||
| The PR's checked-out files are in the `pr-head/` subdirectory. | ||
|
|
||
| Please review this pull request with a focus on: | ||
| - Code quality and best practices | ||
| - Potential bugs or issues | ||
| - Security implications | ||
| - Performance considerations | ||
|
|
||
| Provide detailed feedback using inline comments for specific issues. | ||
|
|
||
| # --max-turns caps how many tool-use cycles Claude can run, which | ||
| # bounds token spend per invocation. The allowed `gh pr` commands are | ||
| # scoped to this PR's number so a misfire can't reach into another PR. | ||
| claude_args: | | ||
| --add-dir pr-head | ||
| --max-turns 30 | ||
| --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment ${{ github.event.pull_request.number }}:*),Bash(gh pr diff ${{ github.event.pull_request.number }}:*),Bash(gh pr view ${{ github.event.pull_request.number }}:*)" | ||
| uses: stellar/actions/.github/workflows/claude-review.yml@main | ||
| secrets: | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Limiting
pull_request_targettoready_for_reviewandsynchronizestops Claude from running on the first review cycle for PRs that are opened as non-draft (and on PRs that are reopened without new commits).ready_for_reviewonly fires when converting a draft PR, so many normal PR openings will get no review until another push happens.Useful? React with 👍 / 👎.