diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml
index cda601c..eb4699a 100644
--- a/.github/workflows/claude-review.yml
+++ b/.github/workflows/claude-review.yml
@@ -1,3 +1,22 @@
+# Reusable AI PR-review workflow. Callers invoke it from a `pull_request`
+# workflow (see each repo's .github/workflows/claude.yml).
+#
+# Permission model: the review gets a full read shell (Read/Glob/Grep/LS plus
+# Bash) so it never wastes turns on denied commands; scope and safety are
+# enforced by separate mechanisms rather than a command allow-list:
+# - scope (don't run builds/tests): a narrow deny-list of package managers,
+# build/test runners, and network tools (in claude_args below)
+# - safety: a read-only contents token; denies on the git add/commit/rm/push
+# commands claude-code-action injects by default (deny beats allow from
+# every source); a harden-runner egress step (audit); the ephemeral runner
+#
+# ADOPTION PRECONDITION (cannot be enforced from here — callers must honor it):
+# invoke this only on `pull_request`, never `pull_request_target`, and do not
+# enable "send secrets to fork PRs". That trigger model is the real safety
+# boundary — it keeps secret-bearing runs limited to same-repo (push-access)
+# authors; fork and read-only-collaborator PRs get no secrets, so the review
+# can't authenticate and never runs. The deny-list and egress control are
+# defense-in-depth on top of it.
name: Claude Code – PR Review
on:
workflow_call:
@@ -27,7 +46,10 @@ on:
required: true
permissions:
- contents: write
+ # read-only: the review reads the checkout and posts comments, never pushes.
+ # (The git add/commit/rm/push denies in claude_args below close the mutation
+ # path claude-code-action's injected allows would otherwise leave open.)
+ contents: read
pull-requests: write
issues: write
actions: read
@@ -42,6 +64,25 @@ jobs:
runs-on: ubuntu-latest
timeout-minutes: ${{ inputs.timeout_minutes }}
steps:
+ # Network egress control, audit mode. MUST be the job's first step so
+ # the monitor is installed before anything else runs. In audit mode
+ # harden-runner logs every outbound connection (an egress audit trail —
+ # how we see what the reviewer's runner actually contacts) and blocks
+ # known-compromised C2/exfil endpoints from its built-in threat feed
+ # regardless of policy. It does NOT block egress to a novel host in
+ # audit mode. Staying in audit is deliberate: the real safety boundary
+ # is the trigger model (see the adoption precondition at the top of this
+ # file), and egress filtering — like the command deny-list, which can't
+ # fence bash /dev/tcp or python sockets — is defense-in-depth. Switching
+ # egress-policy to `block` with an `allowed-endpoints` list would stop
+ # novel-host exfil rather than just log it, at the cost of maintaining
+ # that list against endpoint drift; capture the baseline from a real
+ # run's audit log first.
+ - name: Harden runner egress
+ uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0
+ with:
+ egress-policy: audit
+
- name: Checkout PR
uses: actions/checkout@v5
with:
@@ -61,34 +102,27 @@ jobs:
## Environment constraints
- This review is read-only: the Read, Glob, and Grep tools plus read-only
- shell (`git log`/`git diff`/`git show`, `grep`, `cat`, `head`, `tail`,
- `wc`, `ls`, `jq`, `diff`) are available. Use `jq` to inspect or query JSON (it is
- read-only) — `python`, `node`, and other interpreters are NOT available.
- To locate or list files, use the Glob and LS tools (or `ls`) — `find` is
- NOT available.
- It cannot run builds, linters, or test suites, install dependencies, edit
- files, or reach the network — so do not attempt repo setup, linting, or
- test commands. Review primarily from the
- diff (`git diff origin/...HEAD`), opening additional files only when
- a finding needs cross-file confirmation. If something genuinely requires a
- disallowed tool, note it once in the final comment instead of retrying.
+ This review is read-only. You have the Read, Glob, and Grep tools plus a
+ shell for read-only inspection — git (log/diff/show), grep, find, sed, awk,
+ jq, python3 one-liners, pipes, and loops all work. Batch repetitive
+ per-file checks into a single command (e.g. one loop, or one
+ process-substitution diff) instead of one command per file.
+
+ Do NOT edit files, run builds, linters, or test suites, install
+ dependencies, or access the network. Package managers, build tools, and
+ network commands are blocked, and executing the project is out of scope
+ regardless — the job is reading and reasoning about the diff. If something
+ genuinely requires a blocked tool, note it once in the final comment
+ instead of retrying. The checkout already contains full history and
+ `origin/` — do not `git fetch` or `git pull`.
+
+ Review primarily from the diff (`git diff origin/...HEAD`), opening
+ additional files only when a finding needs cross-file confirmation.
The review rubric below already incorporates the organization-wide code
review guidelines. Apply it IN ADDITION to any repository-specific
CLAUDE.md guidelines.
- ## Repeated changes across files
-
- When the same change repeats across files (a migration applied to several
- modules, a field added to many definitions, a symbol renamed across
- callers), review it once on one representative file, then confirm the
- other occurrences are identical rather than re-reading each — e.g. `diff
- <(git diff origin/...HEAD -- fileA) <(git diff origin/...HEAD
- -- fileB)`. Focus the review on the distinct changes and on any file that
- does not match the pattern — a deviation from an otherwise-uniform change
- is the highest-signal finding.
-
# Review rubric (Claude)
## Must-fix
@@ -189,5 +223,22 @@ jobs:
5. Provide detailed feedback on code review
${{ inputs.extra_prompt }}
- claude_args: "--model ${{ inputs.model }} --max-turns ${{ inputs.max_turns }} --allowedTools 'Read,Glob,Grep,LS,Bash(git log:*),Bash(git diff:*),Bash(git show:*),Bash(grep:*),Bash(cat:*),Bash(head:*),Bash(tail:*),Bash(wc:*),Bash(jq:*),Bash(ls:*),Bash(diff:*)'"
+ # --allowedTools: bare `Bash` (plus the read tools) replaces the old
+ # command enumeration — the model drives a full shell, so every
+ # construct not on an allow-list (find, sed, awk, python3, pipes,
+ # loops) used to cost a turn on a denial + retry. Safety does NOT rest
+ # on this list; see the deny-list and the header.
+ # --disallowedTools: scope guards + defense-in-depth (best-effort, not
+ # a boundary — it can't fence bash /dev/tcp or python sockets). git
+ # add/commit/rm/push: claude-code-action injects allows for exactly
+ # these + its git-push.sh helper; deny beats allow, closing the push
+ # path even for the action's own token. git fetch/pull: checkout is
+ # complete already. Plus network tools and package/build/test runners
+ # (executing the project is out of scope). Bash(...) patterns ONLY —
+ # never an MCP tool name, or the sticky-comment tool breaks.
+ claude_args: >-
+ --model ${{ inputs.model }}
+ --max-turns ${{ inputs.max_turns }}
+ --allowedTools 'Read,Glob,Grep,LS,Bash'
+ --disallowedTools 'Bash(git push:*),Bash(git commit:*),Bash(git add:*),Bash(git rm:*),Bash(git fetch:*),Bash(git pull:*),Bash(gh:*),Bash(curl:*),Bash(wget:*),Bash(nc:*),Bash(ncat:*),Bash(ssh:*),Bash(scp:*),Bash(rsync:*),Bash(npm:*),Bash(npx:*),Bash(yarn:*),Bash(pnpm:*),Bash(pip:*),Bash(pip3:*),Bash(poetry:*),Bash(uv:*),Bash(bundle:*),Bash(gem:*),Bash(pod:*),Bash(brew:*),Bash(apt:*),Bash(apt-get:*),Bash(sudo:*),Bash(make:*),Bash(cmake:*),Bash(gradle:*),Bash(./gradlew:*),Bash(xcodebuild:*),Bash(swift:*),Bash(pytest:*),Bash(tox:*),Bash(tsc:*),Bash(cargo:*),Bash(go:*),Bash(docker:*),Bash(terraform:*)'
track_progress: true