From 06a935098bd4a8f86b18cd0ad7f6f0cc614c1033 Mon Sep 17 00:00:00 2001 From: Leigh <351529+leighmcculloch@users.noreply.github.com> Date: Wed, 13 May 2026 20:05:40 +0000 Subject: [PATCH] add claude-review reusable workflow --- .github/workflows/claude-review.yml | 242 ++++++++++++++++++++++++++++ README.md | 7 + 2 files changed, 249 insertions(+) create mode 100644 .github/workflows/claude-review.yml diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml new file mode 100644 index 0000000..ff7200b --- /dev/null +++ b/.github/workflows/claude-review.yml @@ -0,0 +1,242 @@ +# # Claude Review reusable workflow +# +# Runs `anthropics/claude-code-action` to review a pull request. +# +# ## Usage +# +# Call this reusable workflow from a caller workflow in another stellar +# repo. Trigger on `pull_request` (**recommended** for safety). Use +# `pull_request_target` only if your repo strictly requires reviewing +# fork-based contributions — see [Security](#security) below for the +# threat model of each option. +# +# ### Minimal caller (recommended) +# +# For repos that don't accept fork contributions or don't need Claude to +# review them: +# +# ```yaml +# name: Claude Review +# on: +# pull_request: +# types: [ready_for_review, synchronize] +# concurrency: +# group: claude-review-${{ github.event.pull_request.number }} +# cancel-in-progress: true +# permissions: {} +# jobs: +# review: +# uses: stellar/actions/.github/workflows/claude-review.yml@main +# secrets: +# anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} +# ``` +# +# ### Caller for repos requiring fork-based contribution review +# +# Only change is the trigger event: +# +# ```yaml +# on: +# pull_request_target: +# types: [ready_for_review, synchronize] +# ``` +# +# ### Customizing the prompt +# +# Pass a `prompt` input on the `uses:` job: +# +# ```yaml +# jobs: +# review: +# uses: stellar/actions/.github/workflows/claude-review.yml@main +# with: +# prompt: | +# Custom review instructions. The PR head is in `pr-head/`. +# secrets: +# anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} +# ``` +# +# ### Inputs +# +# - `prompt` _(optional)_ — Review prompt. The default works for most repos. +# +# ### Secrets +# +# - `anthropic_api_key` _(required)_ — Anthropic API key for Claude. +# +# ## Security +# +# This workflow supports two trigger modes with very different threat +# models. +# +# ### `pull_request` (recommended — safer default) +# +# - Runs in the context of the PR head. `secrets.ANTHROPIC_API_KEY` is +# **not** available for PRs from forks (GitHub strips secrets for fork +# PRs on this event), so fork PRs cannot be reviewed in this mode — +# the action will fail on them. This is the safe default for repos +# that don't accept fork PRs (or don't need Claude to review them). +# - Same-repo PRs in `pull_request` mode run with the workflow file **as +# it exists on the PR**, so a contributor with push access can edit +# the workflow on a branch. The author-association gate below is +# therefore not a security boundary in this mode — it only filters +# which PRs are reviewed. That's acceptable because same-repo +# contributors already have push access. +# +# ### `pull_request_target` (highly discouraged unless required) +# +# Use **only** if your repo strictly requires reviewing contributions +# via forks from org members (e.g. a repo where the core team +# contributes via personal forks rather than branches). +# +# - Runs in the context of the BASE repository, so secrets **are** +# available even for fork PRs, and the workflow definition that runs +# is the one on the base branch (a fork cannot edit the caller +# workflow to bypass the gate or exfiltrate secrets by editing the +# workflow itself). +# - This is also the source of GitHub's well-known "pwn request" class +# of vulnerabilities. The mitigations below (author-association gate, +# isolated `pr-head/` checkout, minimal permissions) exist precisely +# because the threat surface is larger. +# +# #### Risks specific to `pull_request_target` +# +# 1. **Isolated PR head checkout.** We check out +# `github.event.pull_request.head.sha` 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 ): +# 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. +# +# 2. **Author-association gate.** Only PRs whose `author_association` is +# `MEMBER` (member of the org that owns the caller 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 being accepted +# — same trust boundary as merge access. +# +# 3. **Excluded associations.** `COLLABORATOR` (outside collaborators +# invited to the 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. +# +# 4. **Re-evaluation on push.** 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. +# +# 5. **Minimal permissions.** `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: + workflow_call: + inputs: + prompt: + description: >- + The prompt for Claude. The PR's checked-out files are in the + `pr-head/` subdirectory. + type: string + required: false + default: | + 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. + + When using GitHub CLI commands always include the PR number explicitly, + e.g. `gh pr diff ${{ github.event.pull_request.number }}`, + `gh pr view ${{ github.event.pull_request.number }}`, + `gh pr comment ${{ github.event.pull_request.number }} --body "..."`. + Do not rely on the current directory context to infer the PR number. + secrets: + anthropic_api_key: + required: true + +# Revoke all default GITHUB_TOKEN permissions at the workflow level. The 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: {} + +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. + # Fetch full history so the agent can use `git log` / `git blame` for + # context. + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + # 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: 0 + 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: ${{ inputs.prompt }} + + # --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 }}:*)" diff --git a/README.md b/README.md index aed153f..e711544 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,12 @@ workflows. | ---- | ---- | ----------- | | [update-completed-sprint-on-issue-closed] | Workflow | Updates the CompletedSprint project field when an issue/PR is closed. | +### Code Review + +| Name | Type | Description | +| ---- | ---- | ----------- | +| [claude-review] | Workflow | Runs Claude to review a pull request. Callers should trigger on `pull_request` (recommended for safety). Use `pull_request_target` only for repos that strictly require reviewing fork-based contributions. See the workflow's security note. | + [@stellar]: https://github.com/stellar [rust-cache]: ./rust-cache @@ -71,6 +77,7 @@ workflows. [rust-publish-dry-run-v2]: ./.github/workflows/rust-publish-dry-run-v2.yml [rust-publish]: ./.github/workflows/rust-publish.yml [update-completed-sprint-on-issue-closed]: ./.github/workflows/update-completed-sprint-on-issue-closed.yml +[claude-review]: ./.github/workflows/claude-review.yml [disk-cleanup]: ./disk-cleanup [sdf-ecr-login]: ./sdf-ecr-login/action.yml [README-rust-release.md]: README-rust-release.md