fix(review-only): enforce Claude sandbox boundary hook

[deadczarvc]

Summary

• Add a scoped review-only-boundary-hook.mjs PreToolUse guard to read-only sandbox settings.
• Fail closed on write-class tools and mutating shell commands when OS sandboxing is unavailable.
• Cover hook wiring plus the patcher read-only matrix: --dry-run, --status, --verify allowed; apply/plain/path-spoof blocked.

Verification

• node --check scripts/lib/claude-cli.mjs
• node --check hooks/review-only-boundary-hook.mjs
• node --check tests/review-only-boundary-hook.test.mjs
• node --test tests/review-only-boundary-hook.test.mjs
• node --test tests/sandbox-modes.test.mjs
• npm run test:cross-platform
• npm run check:version-sync && npm run check:changelog
• npm ci
• npm run lint
• npm run typecheck

Note

Full npm test was attempted on Windows and did not pass because of existing Windows-specific failures outside this patch scope: fake claude fixture lookup in hook tests, /bin/bash wrapper path handling, and /proc process identity assumptions.

Co-Authored-By: LLM+TUI-IDE-based agent Codex Gopota
Directive: code produced under human supervisory control signal (ТСАУ). DM @dead_imperator · lawrenncecharlotte@gmail.com

[sf-jin-ku]

Thanks for the contribution. Closing in favor of a different approach we're shipping in the next version.

Why we're not merging this PR:

1. Empirical: we measured what --allowedTools actually enforces against Claude CLI. The Bash(<sub-pattern>:*) entries in SANDBOX_READ_ONLY_BASH_TOOLS are not strict — once Bash appears in the allowlist with any pattern, the entire Bash tool opens up. The filesystem.allowWrite setting in SANDBOX_SETTINGS["read-only"] also was not enforcing OS-level writes in our macOS test. So the foundational claim that this hook is the "missing piece on top of working allowlist + OS sandbox" doesn't match what's running.

2. The regex shell-command filter is structurally bypassable: bash -c "...", eval, $(...), backticks, ;/&& chains, heredocs, function definitions, alias indirection, sh /tmp/script.sh, xargs, env-prefix tricks, node -e "fs.unlinkSync(...)", gh pr create, docker run, etc. all sail through. The hook stops easy/accidental mutations but provides no defense against a model or hostile input that puts a token of indirection in front of the dangerous command. The cost of carrying that regex layer + an extra Node subprocess per tool call doesn't match the value.

3. The approach scales by adding more regex. Every new shell construct or new package manager becomes another entry. We don't want to take ownership of a permanent regex maintenance surface.

What's landing in the next version instead (commit 0d9bf09 on main):

• Review/adversarial-review now run inside an ephemeral git worktree that is created at the start of the run and force-removed at the end (nested try/finally so every failure path still cleans up). Any mutation that slips through is contained and dies with the worktree.
• A bundled read-only git MCP server (scripts/lib/mcp-git.mjs) exposes structured diff/log/show/blame/status/grep/ls_files tools. MCP tool names are strictly enforced by --allowedTools unlike Bash sub-patterns, and argv is constructed by the server with spawnSync so command injection is impossible by construction.
• The allowlist becomes Read, Glob, Grep, WebSearch, WebFetch, mcp__gitReview__* — no Bash entry at all. Empirical tool-usage measurements on real review runs showed ~99% of Bash usage is git, fully covered by the MCP surface.
• The read-only sandbox preset stops blocking network so WebFetch/WebSearch and the Claude CLI's API path keep working; safety comes from removing Bash, not from blocking network.

Appreciate the time you put into the patch.