Harden claude_code judge (system-priority prompt, no tools) + fix path sanitizer blocking all edits#8
Merged
Conversation
…ction Two coupled fixes to the claude_code provider path; the second addresses both Codex P1 review comments on #6. utils/diff.py (CRITICAL): _normalize_path rejected EVERY absolute path as traversal, replacing it with the [PATH_TRAVERSAL_BLOCKED] sentinel. Since Write/Edit always pass absolute paths, this made the Oracle veto (or fail to verify) every edit the moment a real provider was active -- the fail-open had masked it all session. Now resolves the path against the project root and allows in-root paths (returned project-relative, nameable for governance), blocking only genuine escapes. Drops the os.path.isabs anti-pattern. NOTE: applied via a direct write (bypassing the PreToolUse hook), because the broken sanitizer was vetoing its own repair. One-time break-glass, approved, to restore governance. Every subsequent edit in this change was governed by Bench normally -- and it correctly vetoed a docstring that described the hardening before the code implemented it. utils/api.py (Codex P1 x2): the claude_code judge now (1) loads the stage system prompt from a temp file via --system-prompt-file so it keeps SYSTEM priority over the untrusted diff on stdin -- a prompt-injection diff can no longer override the role/schema instructions -- instead of folding it into the same stdin text; and (2) runs with --tools "" (no tools) rather than only --disallowedTools Write/Edit, so an injected diff cannot drive the judge to mutate files (the child bypasses Bench's own hook via BENCH_SUBPROCESS=1). The temp file is ephemeral model input, removed in a finally block. Tests: test_api_claude_cli updated for the new behavior, plus a test proving the system prompt reaches the file (not stdin) and is cleaned up. 93 tests pass. Verified live on the subscription: a multi-line Oracle prompt rejected an in-diff injection attempt (verdict stayed PASS, not INJECTED). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial audit (6 confirmed findings of 10) follow-ups: - injection (P? -> real): the judge now also passes --strict-mcp-config so it has NO MCP tools, not just no built-ins. --tools "" alone drops only built-in tools; MCP-namespaced tools and the inherited settings allow-list survived it, so "no tools" was overstated. (--bare would isolate further but strips the subscription auth -> "Not logged in", so it is unusable here.) Docstring updated to match. - temp-file orphan: record the temp path before f.write so a write/flush failure still cleans up the file (and unlink it inline on that error path). - hook fallback (hooks/pre-tool-use.py): the inline fallback used when utils.diff fails to import still blocked every absolute path, reintroducing the garble-every-edit bug in degraded mode. Now mirrors _normalize_path's resolve-against-root containment, with C-001-compliant inline logging on each blocked branch (Bench vetoed the first attempt for deferring the log). - tests: the core allow-in-root-absolute behavior had zero coverage (a revert would have stayed green) -- added _normalize_path and build_diff_info regression tests, plus hook-fallback in-root/escape tests, a multi-turn stdin-absence assertion, a --strict-mcp-config assertion, and a temp-write OSError -> _ProviderError test. 288 tests pass. Every fix here was governed by Bench itself and passed (it correctly vetoed two intermediate attempts -- a docstring ahead of its code, and a C-001 deferred log -- both corrected). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a97a55742
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 on #8: _normalize_path used os.path.realpath(os.getcwd()) as the project root. When the hook runs with a CWD below the repo root (e.g. a subdir), an in-repo absolute path resolves to ../... and is wrongly blocked as traversal, making a legitimate governed edit unnameable in the ledger. Now derive the root from this module's location (utils/diff.py -> repo root) via a _PROJECT_ROOT constant, mirroring the hook's existing _REPO_ROOT pattern; the inline hook fallback uses _REPO_ROOT for the same reason. CWD-independent. Tests: in-root absolute tests now key off _PROJECT_ROOT, plus a regression test that chdir's into a subdir and asserts an in-repo absolute path still resolves. 289 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dburks-svg
added a commit
that referenced
this pull request
Jul 12, 2026
…ider docs - CLAUDE.md: python -m bench never existed (no bench package); the real entry point is python -m cli. Structure diagram now shows .claude/settings.json and the previously undocumented utils/viewer.py, utils/stats.py, and viewer CLI command. - README.md: Quick Start cp path corrected to .claude/settings.json; the self-governance receipt is ledger entry #13 (hook fallback-coverage veto), not #8 (Oracle constitution-replacement veto). - utils/api.py: module docstring now documents all three providers, including claude_code. - cli/commands.py: docstring counts all five commands. - .gitignore: replaced the stock Node/JS template with a lean Python set; no tracked files affected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
What
Addresses both Codex P1 review comments on #6, and fixes a critical latent bug that the dogfooding surfaced along the way.
The Codex P1s (both verified live)
1. System-priority prompt (#6 comment 1). The claude_code judge previously folded the stage system prompt into the same stdin text as the untrusted diff, so a prompt-injection diff could override the role/schema instructions and earn an unearned PASS. The judge now writes the system prompt to a temp file and loads it via
--system-prompt-file, keeping it at SYSTEM priority over the diff (which stays on stdin), without the multi-line-argv truncationcmd.exeinflicts on--system-promptfor.cmdshims. Verified live: an in-diff "ignore your instructions" injection is rejected (verdict stays PASS, not INJECTED).2. No tools for the judge (#6 comment 2). The judge ran with only
--disallowedTools Write Edit, but it executes withBENCH_SUBPROCESS=1(Bench's own hook is bypassed), so an injected diff could drive it to mutate files via other tools. It now runs with--tools ""(no built-in tools) and--strict-mcp-config(no MCP servers). The audit caught that--tools ""alone leaves MCP tools available;--barewould isolate further but strips the subscription auth, so it is not usable.Critical bug found while fixing the above
utils/diff.py:_normalize_pathrejected every absolute path as traversal. Since Write/Edit always pass absolute paths, this made the Oracle veto/garble every edit the moment a real provider was active (the fail-open had masked it). Now resolves against the project root and allows in-root paths (project-relative, nameable for governance), blocking only genuine escapes; the same fix is mirrored in the hook's inline fallback. Drops theos.path.isabsanti-pattern.This one fix was applied via a direct write (a one-time, approved break-glass) because the broken sanitizer was vetoing its own repair. Every other change in this PR was governed by Bench itself and passed — and Bench correctly vetoed two intermediate attempts (a docstring describing hardening before the code implemented it, and a catch block that deferred its log, a C-001 violation), both corrected.
Tests
288 pass under
python -m unittest discover -s tests. New coverage: in-root absolute paths (the core fix had zero coverage — a revert would have stayed green), hook-fallback in-root/escape, multi-turn stdin-absence,--strict-mcp-configpresence, and temp-writeOSError -> _ProviderError.Closes the review feedback on #6.
🤖 Generated with Claude Code