Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions hooks/pre-tool-use.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,36 @@ def extract_diff_info(tool_name: str, tool_input: dict[str, Any]) -> dict[str, A
file=sys.stderr,
)
raw_path: str = str(tool_input.get("file_path", ""))
normalized: str = os.path.normpath(raw_path) if raw_path else raw_path
if raw_path and (
os.path.isabs(normalized)
or raw_path.startswith("/")
or normalized.startswith("..")
):
print(
f"[bench hook] path traversal blocked in fallback: {raw_path!r}",
file=sys.stderr,
)
normalized = "[PATH_TRAVERSAL_BLOCKED]"
normalized: str = raw_path
if raw_path:
# Mirror utils.diff._normalize_path: resolve against the project root and
# allow in-root paths (returned project-relative, nameable for
# governance), blocking only genuine escapes. Rejecting every absolute
# path would garble every edit, since Write/Edit always pass absolute
# paths — the bug this mirrors the fix for. Use the file-derived repo
# root (_REPO_ROOT), not os.getcwd(), which can be a subdir at runtime.
root: str = os.path.realpath(str(_REPO_ROOT))
candidate: str = os.path.realpath(os.path.join(root, raw_path))
try:
normalized = os.path.relpath(candidate, root)
except ValueError as exc:
# Different drive on Windows: cannot be inside the project root.
print(
f"[bench hook] path traversal blocked in fallback "
f"(cross-drive) {raw_path!r}: {exc}",
file=sys.stderr,
)
normalized = "[PATH_TRAVERSAL_BLOCKED]"
else:
if normalized == os.pardir or normalized.startswith(
os.pardir + os.sep
):
print(
f"[bench hook] path traversal blocked in fallback "
f"(escapes project root) {raw_path!r}",
file=sys.stderr,
)
normalized = "[PATH_TRAVERSAL_BLOCKED]"
if tool_name == "Write":
return {
"file_path": normalized,
Expand Down
2,536 changes: 2,536 additions & 0 deletions ledger/bench-ledger.json

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions ledger/ledger-meta.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"entry_count": 70,
"latest_hash": "c313554adedb683734b70be879b46093a45219b910ab86d14e45ae4ce1b3b7aa",
"entry_count": 91,
"latest_hash": "1dcf00fb09ee80d63f2cdab73c3b13ba29509610da3b3fd26179ef4bea323c2f",
"created": "2026-04-22T04:00:32.627154+00:00",
"last_updated": "2026-06-26T05:01:48.596076+00:00"
"last_updated": "2026-06-26T07:46:35.462007+00:00"
}
65 changes: 48 additions & 17 deletions tests/test_api_claude_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,21 +113,26 @@ def test_envelope_not_object_raises(self, _which, run) -> None:

@mock.patch("utils.api.subprocess.run")
@mock.patch("utils.api.shutil.which", return_value="/usr/bin/claude")
def test_sets_subprocess_env_and_disallowed_tools(self, _which, run) -> None:
def test_sets_subprocess_env_and_no_tools(self, _which, run) -> None:
run.return_value = _completed(stdout=_OK_ENVELOPE)
_claude_cli_call("claude-opus-4-7", "sys", _msgs(), 4096)
_claude_cli_call("claude-opus-4-7", "SYSPROMPT_SENTINEL", _msgs(), 4096)
args, kwargs = run.call_args
cmd = args[0] if args else kwargs["args"]
self.assertIn("--disallowedTools", cmd)
self.assertIn("Write", cmd)
self.assertIn("Edit", cmd)
# Security: the judge gets NO tools (--tools ""), not just a Write/Edit
# deny list, since the child bypasses Bench's own hook.
self.assertEqual(cmd[cmd.index("--tools") + 1], "")
self.assertIn("--strict-mcp-config", cmd)
self.assertNotIn("--disallowedTools", cmd)
self.assertIn("--model", cmd)
self.assertIn("claude-opus-4-7", cmd)
self.assertEqual(kwargs["env"].get("BENCH_SUBPROCESS"), "1")
self.assertFalse(kwargs.get("shell", False))
self.assertNotIn("--system-prompt", cmd)
self.assertEqual(kwargs.get("encoding"), "utf-8")
self.assertTrue(kwargs["input"].startswith("sys"))
# System prompt goes to --system-prompt-file, never onto the stdin
# payload or the argv (which would lose system priority / get truncated).
self.assertIn("--system-prompt-file", cmd)
self.assertNotIn("SYSPROMPT_SENTINEL", kwargs["input"])
self.assertNotIn("SYSPROMPT_SENTINEL", cmd)

@mock.patch("utils.api.subprocess.run")
@mock.patch("utils.api.shutil.which", return_value="/usr/bin/claude")
Expand All @@ -138,22 +143,48 @@ def test_multi_turn_flattening(self, _which, run) -> None:
{"role": "assistant", "content": "bad output"},
{"role": "user", "content": "respond with JSON only"},
]
_claude_cli_call("claude-sonnet-4-6", "sys", msgs, 4096)
_claude_cli_call("claude-sonnet-4-6", "SYS_SENTINEL", msgs, 4096)
_args, kwargs = run.call_args
sent = kwargs["input"]
self.assertIn("ASSISTANT: bad output", sent)
self.assertIn("USER: respond with JSON only", sent)
# The system prompt must not leak onto stdin in the multi-turn path
# either — it goes to --system-prompt-file.
self.assertNotIn("SYS_SENTINEL", sent)

@mock.patch("utils.api.subprocess.run")
@mock.patch("utils.api.shutil.which", return_value="/usr/bin/claude")
def test_folds_system_prompt_into_stdin(self, _which, run) -> None:
run.return_value = _completed(stdout=_OK_ENVELOPE)
_claude_cli_call("claude-sonnet-4-6", "SYSTEM_RULES", _msgs(), 4096)
args, kwargs = run.call_args
cmd = args[0] if args else kwargs["args"]
self.assertTrue(kwargs["input"].startswith("SYSTEM_RULES"))
self.assertNotIn("--system-prompt", cmd)
self.assertNotIn("SYSTEM_RULES", cmd)
def test_system_prompt_written_to_file_not_stdin(self, _which) -> None:
captured: dict[str, str] = {}

def fake_run(cmd, **kwargs):
# Read the system-prompt-file content while it still exists, before
# _claude_cli_call removes it in its finally block.
path = cmd[cmd.index("--system-prompt-file") + 1]
captured["file"] = Path(path).read_text(encoding="utf-8")
captured["input"] = kwargs.get("input", "")
captured["path"] = path
return _completed(stdout=_OK_ENVELOPE)

with mock.patch("utils.api.subprocess.run", side_effect=fake_run):
_claude_cli_call("claude-sonnet-4-6", "STRICT_JUDGE_RULES", _msgs(), 4096)

# System prompt reaches the file (system priority), not the stdin payload.
self.assertEqual(captured["file"], "STRICT_JUDGE_RULES")
self.assertNotIn("STRICT_JUDGE_RULES", captured["input"])
# The temp file is cleaned up after the call.
self.assertFalse(Path(captured["path"]).exists())

@mock.patch("utils.api.shutil.which", return_value="/usr/bin/claude")
def test_temp_file_write_failure_raises_provider_error(self, _which) -> None:
# If the system-prompt temp file cannot be written, the helper must
# raise the typed _ProviderError, never a bare OSError that would break
# call_model's never-raises contract.
with mock.patch(
"utils.api.tempfile.NamedTemporaryFile",
side_effect=OSError("disk full"),
):
with self.assertRaises(_ProviderError):
_claude_cli_call("claude-sonnet-4-6", "sys", _msgs(), 4096)

@mock.patch("utils.api.subprocess.run")
@mock.patch("utils.api.shutil.which", return_value="/usr/bin/claude")
Expand Down
32 changes: 32 additions & 0 deletions tests/test_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from utils.diff import ( # noqa: E402
BINARY_LABEL,
MAX_DIFF_LINES,
_PROJECT_ROOT,
_is_binary,
_normalize_path,
_truncate_preserving,
Expand Down Expand Up @@ -257,6 +258,37 @@ def test_build_diff_info_normalizes_absolute(self) -> None:
)
self.assertEqual(result["file_path"], "[PATH_TRAVERSAL_BLOCKED]")

def test_in_root_absolute_path_allowed(self) -> None:
# Write/Edit always pass ABSOLUTE paths, so an in-root absolute path must
# resolve to its project-relative form, not be blocked. Regression guard:
# the old code blocked every absolute path, which garbled every edit.
abs_in_root: str = os.path.join(_PROJECT_ROOT, "utils", "diff.py")
result = _normalize_path(abs_in_root)
self.assertNotEqual(result, "[PATH_TRAVERSAL_BLOCKED]")
self.assertEqual(result, os.path.join("utils", "diff.py"))

def test_build_diff_info_allows_in_root_absolute(self) -> None:
# The real-world input shape: build_diff_info given an absolute in-root
# file_path must return the nameable project-relative path.
abs_in_root: str = os.path.join(_PROJECT_ROOT, "utils", "api.py")
result = build_diff_info(
"Write", {"file_path": abs_in_root, "content": "x = 1"}
)
self.assertEqual(result["file_path"], os.path.join("utils", "api.py"))

def test_in_root_absolute_allowed_when_cwd_is_subdir(self) -> None:
# Regression for the os.getcwd() root bug (Codex P2 on #8): the hook can
# run with a CWD below the repo root, but an in-repo absolute path must
# still resolve — the root is derived from the module location, not CWD.
target: str = os.path.join(_PROJECT_ROOT, "utils", "diff.py")
original_cwd: str = os.getcwd()
try:
os.chdir(os.path.join(_PROJECT_ROOT, "tests"))
result = _normalize_path(target)
finally:
os.chdir(original_cwd)
self.assertEqual(result, os.path.join("utils", "diff.py"))


class MalformedInputTests(unittest.TestCase):
def test_multi_edit_with_non_list_edits_field(self) -> None:
Expand Down
38 changes: 38 additions & 0 deletions tests/test_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,5 +212,43 @@ def test_bench_subprocess_short_circuits_before_parsing_stdin(
mock_pipeline.assert_not_called()


class ExtractDiffInfoFallbackTests(unittest.TestCase):
"""The inline fallback (used when utils.diff fails to import) must use the
same project-root containment as utils.diff._normalize_path, not block every
absolute path (Write/Edit always supply absolute paths)."""

def test_fallback_allows_in_root_absolute_path(self) -> None:
import os

abs_in_root: str = os.path.join(os.getcwd(), "utils", "api.py")
original = _hook_module._build_diff_info_hardened
try:
_hook_module._build_diff_info_hardened = None # force fallback
info: dict = _hook_module.extract_diff_info(
"Edit",
{"file_path": abs_in_root, "old_string": "a", "new_string": "b"},
)
finally:
_hook_module._build_diff_info_hardened = original
self.assertNotEqual(info["file_path"], "[PATH_TRAVERSAL_BLOCKED]")
self.assertEqual(info["file_path"], os.path.join("utils", "api.py"))

def test_fallback_blocks_escape(self) -> None:
original = _hook_module._build_diff_info_hardened
try:
_hook_module._build_diff_info_hardened = None
info: dict = _hook_module.extract_diff_info(
"Edit",
{
"file_path": "../../../etc/passwd",
"old_string": "a",
"new_string": "b",
},
)
finally:
_hook_module._build_diff_info_hardened = original
self.assertEqual(info["file_path"], "[PATH_TRAVERSAL_BLOCKED]")


if __name__ == "__main__":
unittest.main()
100 changes: 73 additions & 27 deletions utils/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import shutil
import subprocess
import sys
import tempfile
from typing import Any

import anthropic
Expand Down Expand Up @@ -327,16 +328,19 @@ def _claude_cli_call(
"""One call via the local `claude` CLI in headless print mode.

Routes the stage through `claude -p` so it rides the user's Claude Code
subscription instead of an ANTHROPIC_API_KEY. The system prompt and user
content are folded into a single stdin payload (not passed as --system-prompt,
which truncates multi-line values on the .cmd shim path); the reply returns
as a single JSON envelope whose "result" field is the assistant text.
Returns (text, in_tok, out_tok).

Reentrancy: the child inherits this repo's .claude/settings.json, hence
Bench's own PreToolUse hook. BENCH_SUBPROCESS=1 in the child env makes that
hook fail open immediately (see hooks/pre-tool-use.py); --disallowedTools is
a second layer in case the nested agent attempts an edit anyway.
subscription instead of an ANTHROPIC_API_KEY. The stage system prompt is
written to a temp file and loaded via --system-prompt-file so it keeps
SYSTEM priority over the untrusted diff (which goes on stdin) and avoids the
multi-line-argv truncation cmd.exe inflicts on --system-prompt for a .cmd/.bat
shim. The reply returns as a single JSON envelope whose "result" field is the
assistant text. Returns (text, in_tok, out_tok).

Hardening: the call runs tool-less -- --tools "" drops the built-in tools
and --strict-mcp-config (no --mcp-config) drops every MCP server -- so a
prompt-injected diff cannot drive the judge to run Bash/Edit/MCP/etc. This
matters because the child runs with BENCH_SUBPROCESS=1, which makes Bench's
own PreToolUse hook fail open (see hooks/pre-tool-use.py); the env guard
still prevents recursion.

max_tokens is accepted for signature parity with the other providers; the
CLI manages its own output cap.
Expand All @@ -348,9 +352,11 @@ def _claude_cli_call(
if binary is None:
raise _ProviderError("claude_code: `claude` binary not found on PATH")

# Flatten messages into a single text body. Single-turn calls pass the user
# content as-is; the parse-retry path (user/assistant/user) is rendered with
# role labels so the prior reply and the JSON nudge both survive.
# Flatten messages into a single text body for stdin. Single-turn calls pass
# the user content as-is; the parse-retry path (user/assistant/user) is
# rendered with role labels so the prior reply and the JSON nudge survive.
# The system prompt is NOT folded in here — it goes to --system-prompt-file
# below so it keeps system priority over this (untrusted) payload.
if len(messages) == 1:
body: str = messages[0].get("content", "")
else:
Expand All @@ -359,13 +365,6 @@ def _claude_cli_call(
for m in messages
)

# Fold the system prompt into the stdin payload rather than passing it as
# --system-prompt. Bench's stage prompts are multi-line, and when `claude`
# resolves to a .cmd/.bat shim a multi-line argv is truncated at the first
# newline by cmd.exe, silently gutting the prompt. stdin carries arbitrary
# text safely on every platform.
prompt: str = f"{system_prompt}\n\n{body}" if system_prompt else body

timeout: float = _DEFAULT_CLAUDE_CLI_TIMEOUT
timeout_raw: str = os.environ.get("BENCH_CLAUDE_TIMEOUT", "")
if timeout_raw:
Expand All @@ -390,25 +389,62 @@ def _claude_cli_call(
child_env: dict[str, str] = dict(os.environ)
child_env["BENCH_SUBPROCESS"] = "1"

# --disallowedTools is variadic; keep it last so it does not swallow other
# flags. "MultiEdit" is rejected as unknown by some CLI versions, so the
# env-var guard above is the load-bearing protection against recursion.
# Write the system prompt to a temp file loaded via --system-prompt-file so
# the stage's role/schema instructions keep SYSTEM priority over the
# untrusted diff on stdin (a prompt-injection diff cannot override a
# system-priority prompt), without the multi-line-argv truncation cmd.exe
# inflicts on --system-prompt for a .cmd/.bat shim. The file is ephemeral
# (model input only, never a governed project file) and removed in finally.
sys_prompt_path: str | None = None
if system_prompt:
try:
with tempfile.NamedTemporaryFile(
mode="w", suffix=".txt", delete=False, encoding="utf-8"
) as f:
# Record the path before writing: the file already exists on disk
# once NamedTemporaryFile is opened, so if the write (or the
# close-time flush) raises, the cleanup below still finds it.
sys_prompt_path = f.name
f.write(system_prompt)
except OSError as e:
if sys_prompt_path is not None:
try:
os.unlink(sys_prompt_path)
except OSError as cleanup_err:
print(
"[bench api] failed to remove temp system-prompt file "
f"after write error: {cleanup_err}",
file=sys.stderr,
)
raise _ProviderError(
"claude_code: failed to write system prompt file: "
f"{type(e).__name__}: {_sanitize_error_detail(str(e))}"
) from e

# Give the judge NO tools at all: --tools "" removes the built-in tools and
# --strict-mcp-config (with no --mcp-config) removes every MCP server, so an
# injected diff cannot make the agent run Bash/Edit/MCP/etc. This matters
# because the child runs with BENCH_SUBPROCESS=1 (Bench's own hook is
# bypassed). Note --tools "" alone drops only built-ins, not MCP tools, and
# --bare would isolate further but strips the subscription auth (unusable).
cmd: list[str] = [
binary,
"-p",
"--output-format",
"json",
"--model",
model,
"--disallowedTools",
"Write",
"Edit",
"--tools",
"",
"--strict-mcp-config",
]
if sys_prompt_path is not None:
cmd += ["--system-prompt-file", sys_prompt_path]

try:
completed = subprocess.run(
cmd,
input=prompt,
input=body,
capture_output=True,
text=True,
encoding="utf-8",
Expand All @@ -429,6 +465,16 @@ def _claude_cli_call(
f"claude_code: failed to run `claude`: {type(e).__name__}: "
f"{_sanitize_error_detail(str(e))}"
) from e
finally:
if sys_prompt_path is not None:
try:
os.unlink(sys_prompt_path)
except OSError as cleanup_err:
print(
"[bench api] failed to remove temp system-prompt file "
f"{sys_prompt_path!r}: {cleanup_err}",
file=sys.stderr,
)

if completed.returncode != 0:
detail: str = _sanitize_error_detail(
Expand Down
Loading
Loading