diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index c9c10d8..9b6bb5c 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,17 @@ 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), 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(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:*)${{ 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 +332,14 @@ 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. + + 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: @@ -347,7 +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. + 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 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