|
| 1 | +# SECURITY NOTE |
| 2 | +# ───────────── |
| 3 | +# This workflow uses `pull_request_target` rather than `pull_request` so that |
| 4 | +# fork PRs from org members can be reviewed by Claude. `pull_request_target` |
| 5 | +# runs in the context of the BASE repository, which means: |
| 6 | +# |
| 7 | +# 1. `secrets.ANTHROPIC_API_KEY` is available even when the PR comes from a |
| 8 | +# fork. With `pull_request` this secret would be absent on fork PRs and |
| 9 | +# the action would fail. |
| 10 | +# 2. The workflow definition that runs is the one on the base branch, NOT |
| 11 | +# the version in the PR. A fork therefore cannot edit this file to |
| 12 | +# bypass the author-association gate or exfiltrate secrets by editing |
| 13 | +# the workflow itself. |
| 14 | +# |
| 15 | +# However, `pull_request_target` is also the source of GitHub's well-known |
| 16 | +# "pwn request" class of vulnerabilities. The risks specific to this file: |
| 17 | +# |
| 18 | +# A. We check out `github.event.pull_request.head.sha` below into an |
| 19 | +# isolated `pr-head/` subdirectory, NOT the workspace root. The base |
| 20 | +# ref is checked out at the workspace root instead. This follows |
| 21 | +# claude-code-action's recommended pattern for `pull_request_target` |
| 22 | +# (see https://github.com/anthropics/claude-code-action/blob/main/docs/security.md): |
| 23 | +# tools that consult repo-local config (.git/config, .git/hooks, |
| 24 | +# .gitignore, .npmrc, Makefile, pre-commit hooks, etc.) at the |
| 25 | +# workspace root see only trusted base-branch files, while Claude |
| 26 | +# can still read the PR's files via `--add-dir pr-head`. |
| 27 | +# |
| 28 | +# The PR head is still attacker-controlled code, so any future step |
| 29 | +# that executes, sources, or interprets files from `pr-head/` (build |
| 30 | +# scripts, package install hooks, test runners) would run with |
| 31 | +# access to the secrets injected into this job. Treat any new step |
| 32 | +# added below that touches `pr-head/` as if it were running attacker |
| 33 | +# code with secrets in scope. |
| 34 | +# |
| 35 | +# B. The mitigation is the `if:` gate: only PRs whose author_association |
| 36 | +# is MEMBER (member of the org that owns this repo) or OWNER (the repo |
| 37 | +# owner) get this far. `author_association` is set by GitHub from the |
| 38 | +# author's relationship to the repo at event time and cannot be forged |
| 39 | +# from the PR. A compromised org-member account would defeat this |
| 40 | +# gate, which is the residual risk we are accepting — same trust |
| 41 | +# boundary as merge access. |
| 42 | +# |
| 43 | +# C. COLLABORATOR (outside collaborators invited to this repo) and |
| 44 | +# CONTRIBUTOR (anyone who has previously had a commit merged) are |
| 45 | +# intentionally NOT allowed. CONTRIBUTOR in particular is dangerous: |
| 46 | +# a single merged typo fix would otherwise grant a stranger the |
| 47 | +# ability to run code with secrets. |
| 48 | +# |
| 49 | +# D. The PR head is re-evaluated on every `synchronize` event, so an |
| 50 | +# org member cannot open a benign PR, get it approved for review, |
| 51 | +# and then push malicious commits afterward — each push re-runs |
| 52 | +# the gate. But note: the gate is on the AUTHOR, not the pusher. |
| 53 | +# If a malicious actor gains write access to a fork owned by an |
| 54 | +# org member, they can push to that fork's PR branch and trigger |
| 55 | +# this workflow. This is the same trust model as the rest of CI. |
| 56 | +# |
| 57 | +# E. `permissions:` is scoped to the minimum needed (contents: read, |
| 58 | +# pull-requests: write, id-token: write). Do not broaden without |
| 59 | +# reconsidering the threat model — `contents: write` here would |
| 60 | +# let attacker-controlled code in the head ref push to the base |
| 61 | +# repo. |
| 62 | +# |
| 63 | +# Before adding ANY new step to this job, ask: does it execute, source, |
| 64 | +# or interpret files from the checked-out PR head? If yes, the secrets |
| 65 | +# in this job's environment are exposed to whatever that step does. |
| 66 | +name: Claude Review |
| 67 | +on: |
| 68 | + pull_request_target: |
| 69 | + types: [opened, synchronize, ready_for_review, reopened] |
| 70 | + |
| 71 | +# Revoke all default GITHUB_TOKEN permissions at the workflow level. Each job |
| 72 | +# below must explicitly opt in to whatever it needs. This is defense in depth: |
| 73 | +# if a future job is added without its own `permissions:` block it inherits |
| 74 | +# nothing, rather than whatever the repo or org default happens to be. |
| 75 | +permissions: {} |
| 76 | + |
| 77 | +concurrency: |
| 78 | + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} |
| 79 | + cancel-in-progress: true |
| 80 | + |
| 81 | +jobs: |
| 82 | + review: |
| 83 | + if: | |
| 84 | + github.event.pull_request.draft == false && |
| 85 | + (github.event.pull_request.head.repo.fork == false || |
| 86 | + github.event.pull_request.author_association == 'MEMBER' || |
| 87 | + github.event.pull_request.author_association == 'OWNER') |
| 88 | + runs-on: ubuntu-latest |
| 89 | + permissions: |
| 90 | + contents: read |
| 91 | + pull-requests: write |
| 92 | + id-token: write |
| 93 | + steps: |
| 94 | + # Check out the BASE ref at the workspace root. This is trusted code from |
| 95 | + # the base branch, so it's safe for the action to operate against (e.g. |
| 96 | + # reading .git/config, .git/hooks, etc. that the action and its tools |
| 97 | + # consult). Do NOT check out the PR head here — see security note above. |
| 98 | + - uses: actions/checkout@v6 |
| 99 | + with: |
| 100 | + fetch-depth: 1 |
| 101 | + |
| 102 | + # Check out the PR head into an isolated subdirectory. The action is told |
| 103 | + # about it via `--add-dir` below so Claude can read the PR's files, but |
| 104 | + # any attacker-controlled config (.git/config, .git/hooks, etc.) inside |
| 105 | + # this subdirectory is NOT picked up by tools running at the workspace |
| 106 | + # root. See: https://github.com/anthropics/claude-code-action/blob/main/docs/security.md |
| 107 | + - uses: actions/checkout@v6 |
| 108 | + with: |
| 109 | + fetch-depth: 1 |
| 110 | + ref: ${{ github.event.pull_request.head.sha }} |
| 111 | + path: pr-head |
| 112 | + |
| 113 | + - uses: anthropics/claude-code-action@v1 |
| 114 | + with: |
| 115 | + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} |
| 116 | + track_progress: true |
| 117 | + prompt: | |
| 118 | + REPO: ${{ github.repository }} |
| 119 | + PR NUMBER: ${{ github.event.pull_request.number }} |
| 120 | +
|
| 121 | + The PR's checked-out files are in the `pr-head/` subdirectory. |
| 122 | +
|
| 123 | + Please review this pull request with a focus on: |
| 124 | + - Code quality and best practices |
| 125 | + - Potential bugs or issues |
| 126 | + - Security implications |
| 127 | + - Performance considerations |
| 128 | +
|
| 129 | + Provide detailed feedback using inline comments for specific issues. |
| 130 | +
|
| 131 | + # --max-turns caps how many tool-use cycles Claude can run, which |
| 132 | + # bounds token spend per invocation. The allowed `gh pr` commands are |
| 133 | + # scoped to this PR's number so a misfire can't reach into another PR. |
| 134 | + claude_args: | |
| 135 | + --add-dir pr-head |
| 136 | + --max-turns 30 |
| 137 | + --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 }}:*)" |
0 commit comments