fix: take the shell away from the claude reviewer - #123
Merged
Conversation
run_claude passed --allowedTools Read Grep Glob. That grants permission within whatever tool set exists; it does not restrict the set. The reviewer still had Bash and used it: of 427 traced tool calls across 20 rubric runs, 295 were Bash and 278 succeeded. Reproduced directly in the shape reviewer_env builds, where 'claude -p --disable-slash-commands --allowedTools Read Grep Glob' answers SHELL_IS_AVAILABLE. The rest of this module is built on that not being so. reviewer_env calls the isolation load-bearing 'with public transcripts and no redaction gate', and names its residual as a reviewer reading its own key from /proc/self/environ; a shell plus the egress a reviewer needs to reach its provider turns that residual into a direct exfiltration path. run_pi already states the intended property for the OpenRouter reviewers: 'a read-only tool set (PI_TOOLS, no bash) means it has no shell to leak it with.' Pass --tools as well, which restricts the built-in set. --disallowedTools was the other candidate and is worse: it blocks Bash but leaves Write and Edit. Verified end to end that a reuse review still works on the restricted set, in seven turns using Grep, Glob and Read. The test asserts the SET, not a flag's spelling, so a rewrite that keeps the tools read-only passes and one that quietly readmits a shell does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T1zpkPwUh41oU3mtSQGDVU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR takes the shell away from the claude reviewer, which it turned out to have all along.
run_claudepassed--allowedTools Read Grep Glob. That flag grants permission within whatever tool set exists; it does not restrict the set. The reviewer therefore still hadBash, and used it heavily. The tool trace added in #118 shows it plainly, across 20 rubric runs:Sixty-nine percent of its tool calls were
Bash, and 278 of them succeeded. Reproduced directly, in the environment shapereviewer_envbuilds:claude -p --disable-slash-commands --allowedTools Read Grep GlobanswersSHELL_IS_AVAILABLE.The rest of this module is built on that not being the case.
reviewer_envcalls the isolation load-bearing "with public transcripts and no redaction gate, a prompt-injected reviewer must have nothing worth leaking", and names its residual as a reviewer reading its own key from/proc/self/environ, something that "needs a proxy or uid-separation to close". A shell, plus the network egress a reviewer needs to reach its provider, turns that residual into a direct exfiltration path rather than a documented limitation.run_pistates the intended property outright for the OpenRouter reviewers: "a read-only tool set (PI_TOOLS, no bash) means it has no shell to leak it with."--toolsrestricts the built-in set, so both flags are now passed: one removes the tool, the other keeps the remaining three from prompting a headless round to a halt.--disallowedToolswas the other candidate and is worse, since it blocksBashbut leavesWriteandEditin the set.Verified end to end that a review still works on the restricted set: a realistic
reusequestion resolved in seven turns usingGrep,GlobandRead, correctly.tests/test_reviewer_no_shell.pyasserts the resulting tool SET rather than a flag's spelling, so a later rewrite that keeps the reviewer read-only passes, and one that quietly readmits a shell does not.🤖 Prepared with Claude Code