From 00469dae5cca95c7db5797e411fbcc7dde818f25 Mon Sep 17 00:00:00 2001 From: William Grzybowski Date: Fri, 14 Aug 2026 11:54:20 -0300 Subject: [PATCH 1/3] Tune the review allowlist on the first round of denial evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the denial notices and full transcript of a real run: - Write(.claude-review/**): the reviewer drafts its summary there — the scratch dir exists for exactly this, and the scope cannot touch the tree under review. - Bash(gh pr checks:*): the make/go-vet denials were the reviewer asking "does this compile?" — CI already answers that in jobs that do not hold this job's write token. - extra-allowed-tools input: repo-specific tools (go vet on Go repos) belong in the caller, as that repo's own recorded decision. - Prompt: point the reviewer at `gh pr diff` for the change set; the transcript shows it using `git diff HEAD~1`, which silently misses every commit but the newest. Read-only shell (grep, sed -n, ls) auto-approves in the sandbox, so none of it needs allowlisting; interpreters and builds stay denied. --- .github/workflows/claude-review.yml | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index c9c10d8..618d060 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -77,6 +77,14 @@ on: description: 'Checkout depth. Needs to cover the PR range for the diff.' type: number default: 10 + extra-allowed-tools: + description: >- + Comma-separated permission rules appended to --allowedTools, for + repo-specific needs like `Bash(go vet:*)`. Anything that executes + repo code (make, go generate, package scripts) runs PR-controlled + code next to this job's write token — allow that knowingly. + type: string + default: '' tooling-ref: description: >- Ref of this repository to take the schema, rubric and scripts from. @@ -305,13 +313,15 @@ jobs: # nothing itself, hence gh and the inline-comment tool below. # # allowedTools is the auto-approve list, not the whole toolbox — - # Read/Grep/Glob need no approval. Listed: posting, the PR diff, and - # read-only git history. Nothing broader (`gh api`, arbitrary Bash): - # those can write or exfiltrate with this job's token. + # Read/Grep/Glob and provably read-only Bash (grep, sed -n, ls) need + # no approval. Listed: posting, PR reads (diff/view/checks), read-only + # git history, and Write scoped to the scratch dir. Nothing broader + # (`gh api`, arbitrary Bash): those can write or exfiltrate with this + # job's token. claude_args: | --model ${{ inputs.model }} --json-schema '${{ steps.schema.outputs.json }}' - --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*),Bash(git diff:*)" + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr checks:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*),Bash(git diff:*),Write(.claude-review/**)${{ inputs.extra-allowed-tools != '' && format(',{0}', inputs.extra-allowed-tools) || '' }}" # No use_sticky_comment either — it only applies to tag mode. prompt: | REPO: ${{ github.repository }} @@ -320,6 +330,10 @@ jobs: Please review this pull request using the guidelines below. It should be already checked out in the current directory. + The authoritative change set is `gh pr diff` — the checkout's + `HEAD~1` is only the newest commit, not the PR range, and a review + of it silently misses every earlier commit in the PR. + Put each finding that has a file and line on that line, as an inline comment. Use one top-level comment for the summary and anything not tied to a specific line, posted with: From 064c97158078960ab94a66a7835bcff5e8c27907 Mon Sep 17 00:00:00 2001 From: William Grzybowski Date: Fri, 14 Aug 2026 12:40:18 -0300 Subject: [PATCH 2/3] Scope the reviewer's Write to a scratch subdir, not .claude-review/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (HIGH): .claude-review/ also holds the tooling checkout, so Write(.claude-review/**) covered the gate script a later step executes with node — a reviewer steered by PR content could rewrite the gate to exit 0. Grant .claude-review/scratch/ instead, create it up front, and tell the prompt it is the one writable place. Also document extra-allowed-tools in the README input table (LOW). --- .github/workflows/claude-review.yml | 14 ++++++++------ README.md | 1 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 618d060..4f97aa2 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -203,7 +203,7 @@ jobs: # from the workspace root. - name: Reserve the tooling directory run: | - mkdir -p .claude-review + mkdir -p .claude-review/scratch echo "/.claude-review/" >> .git/info/exclude # Fail here rather than let the run proceed: a review with no guidelines @@ -315,13 +315,14 @@ jobs: # allowedTools is the auto-approve list, not the whole toolbox — # Read/Grep/Glob and provably read-only Bash (grep, sed -n, ls) need # no approval. Listed: posting, PR reads (diff/view/checks), read-only - # git history, and Write scoped to the scratch dir. Nothing broader - # (`gh api`, arbitrary Bash): those can write or exfiltrate with this - # job's token. + # git history, and Write scoped to scratch/ — NOT .claude-review/ + # itself, which holds the gate scripts later steps execute. Nothing + # broader (`gh api`, arbitrary Bash): those can write or exfiltrate + # with this job's token. claude_args: | --model ${{ inputs.model }} --json-schema '${{ steps.schema.outputs.json }}' - --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr checks:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*),Bash(git diff:*),Write(.claude-review/**)${{ inputs.extra-allowed-tools != '' && format(',{0}', inputs.extra-allowed-tools) || '' }}" + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr checks:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*),Bash(git diff:*),Write(.claude-review/scratch/**)${{ inputs.extra-allowed-tools != '' && format(',{0}', inputs.extra-allowed-tools) || '' }}" # No use_sticky_comment either — it only applies to tag mode. prompt: | REPO: ${{ github.repository }} @@ -361,7 +362,8 @@ jobs: `.claude-review/` holds the files this run generated for you. It is tooling, not part of the change under review — do not review it, and - do not comment on it. + do not comment on it. If you need to draft a file, write it under + `.claude-review/scratch/` — the one place you can write. The tool you are running in also moves this repository's `.claude/` directory aside to `.claude-pr/.claude/` before you start, so diff --git a/README.md b/README.md index f48ce26..de2f1ce 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,7 @@ either, so granting it in a caller has no effect on the token the job runs with. | `skip-label` | `skip-claude` | | | `timeout-minutes` | `20` | | | `fetch-depth` | `10` | Must cover the PR range | +| `extra-allowed-tools` | `''` | Comma-separated permission rules appended to the reviewer's `--allowedTools`, e.g. `Bash(go vet:*)`. Empty by default on purpose: anything that executes repo code runs PR-controlled code next to the job's write token, so each repo opts in as its own recorded decision | | `tooling-ref` | `master` | Ref this repo's `review/` assets come from; see below | The secret is named, not inherited, because the repos call it different things From 21fbce2cdb9aa0c112a6b5d5b255238350b13069 Mon Sep 17 00:00:00 2001 From: William Grzybowski Date: Fri, 14 Aug 2026 13:02:08 -0300 Subject: [PATCH 3/3] Withdraw the Write grant; the sandbox refuses it below --allowedTools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding, verified from inside its own run: the scoped Write rule never matched, and even Bash redirects into the workspace are refused by a layer under --allowedTools — no allowlist entry can deliver the write, so drop the rule and the prompt's promise. Comment bodies go via stdin, which already works. Also tell the reviewer never to use `gh pr checks --watch`: this review is one of the checks being watched, so it deadlocks until the job timeout. --- .github/workflows/claude-review.yml | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 4f97aa2..9b6bb5c 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -203,7 +203,7 @@ jobs: # from the workspace root. - name: Reserve the tooling directory run: | - mkdir -p .claude-review/scratch + mkdir -p .claude-review echo "/.claude-review/" >> .git/info/exclude # Fail here rather than let the run proceed: a review with no guidelines @@ -314,15 +314,16 @@ jobs: # # allowedTools is the auto-approve list, not the whole toolbox — # Read/Grep/Glob and provably read-only Bash (grep, sed -n, ls) need - # no approval. Listed: posting, PR reads (diff/view/checks), read-only - # git history, and Write scoped to scratch/ — NOT .claude-review/ - # itself, which holds the gate scripts later steps execute. Nothing - # broader (`gh api`, arbitrary Bash): those can write or exfiltrate - # with this job's token. + # no approval. Listed: posting, PR reads (diff/view/checks), and + # read-only git history. No Write rule: the sandbox refuses workspace + # writes at a layer below --allowedTools (verified in-run on PR #8), + # so a grant here is a promise that layer breaks. Nothing broader + # (`gh api`, arbitrary Bash): those can write or exfiltrate with this + # job's token. claude_args: | --model ${{ inputs.model }} --json-schema '${{ steps.schema.outputs.json }}' - --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr checks:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*),Bash(git diff:*),Write(.claude-review/scratch/**)${{ inputs.extra-allowed-tools != '' && format(',{0}', inputs.extra-allowed-tools) || '' }}" + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr checks:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*),Bash(git diff:*)${{ inputs.extra-allowed-tools != '' && format(',{0}', inputs.extra-allowed-tools) || '' }}" # No use_sticky_comment either — it only applies to tag mode. prompt: | REPO: ${{ github.repository }} @@ -335,6 +336,10 @@ jobs: `HEAD~1` is only the newest commit, not the PR range, and a review of it silently misses every earlier commit in the PR. + CI results are `gh pr checks ` — never `--watch`, which + waits on every check while this review is itself one of them, and + so deadlocks until the job timeout. + Put each finding that has a file and line on that line, as an inline comment. Use one top-level comment for the summary and anything not tied to a specific line, posted with: @@ -362,8 +367,8 @@ jobs: `.claude-review/` holds the files this run generated for you. It is tooling, not part of the change under review — do not review it, and - do not comment on it. If you need to draft a file, write it under - `.claude-review/scratch/` — the one place you can write. + do not comment on it. You cannot write files anywhere — compose + comment bodies on stdin, as the posting command above does. The tool you are running in also moves this repository's `.claude/` directory aside to `.claude-pr/.claude/` before you start, so