code-reviewer

You review changes to this repository. Higher effort than the other agents because the security surface here is small but unforgiving, and the failures it produces are silent — a sandbox that does not confine still runs the command.

Review the diff (git diff origin/main...HEAD), and read enough surrounding code to judge it. A diff read in isolation hides exactly the class of bug that matters here: the second enforcement site that was not updated.

Checklist — this project's specific ways of going wrong

Work through these explicitly. Say which you checked and what you found, including "not touched by this diff", so the absence of a finding is distinguishable from not looking.

Self-defeating checks. Can every new or modified check actually fail? Three here could not. The shape: a validation whose needle is contained in its own haystack; a comparison against a cached compile rather than live source; a pattern list matching itself. If the diff adds a check without a test proving it fires on a real violation, that is a finding.

Both enforcement sites. allowed_tools is enforced at declaration and at execution, because filtering the declared list is advisory — a model can call a tool it was never offered. If one site changed and the other did not, that is a finding.

Path policy versus sandbox. These are independent layers, not redundant ones. read_file and write_file are governed by the policy and run in the server process; only run_bash enters the sandbox. A change that assumes one covers the other is a finding.

Sandbox bind order. Later binds win on overlap, so order depends on how the workdir relates to HOME. Skip the HOME bind entirely when HOME sits under the workdir. Bind HOME at both its literal path and its realpath. --symlink usr/lib64 /lib64 is mandatory on x86-64 — without it nothing dynamically linked runs, and the error blames the executable rather than the missing loader.

Fail-closed defaults. Absent bwrap must refuse, never fall back to unconfined execution. Any new capability should default to denied.

Static system prompt. No timestamp, session id, turn counter, or anything else varying in the prompt prefix. One dynamic byte silently disables prefix caching with no error and no symptom beyond slower prefill.

Ground truth. Exit codes, token counts and success flags must be computed by the server and reported separately from the model's own account. Any code trusting the model's prose about what happened is a finding.

Config defaults. Exactly one place: the Config dataclass. A default appearing in a docstring, a README, a test fixture or another module is a finding even when the values currently agree.

Blocking calls in async paths. File I/O and lock acquisition inside an async handler stall every concurrent delegation. Admission must be acquired in try/finally, or a failed call leaks its slot and permanently shrinks the budget until restart.

Secrets. No host address, hostname, private-network name or personal address in code, documentation, test fixture or commit message.