feat(reference)!: directory-only references + anti-cheat permission window - #106
feat(reference)!: directory-only references + anti-cheat permission window#106akshaylive wants to merge 8 commits into
Conversation
…indow
Reference solutions were readable by the agent under evaluation, which shares
a filesystem with the harness — an agent could grep the task directory for the
answer instead of solving the task.
- `task.reference` is now directory-only. `code:`/`file:` are removed (a
directory is the only shape that can be permission-gated as a unit); the
removed keys raise a migration error naming the replacement.
`reference_comparison` gains a required `reference_file`.
- New `orchestration/permissions.py::set_permissions` — an async context
manager that chmods paths for the duration of a block. Windows STACK, so a
nested re-grant restores the enclosing mode rather than the original.
Crash-safe (finally + shield + atexit + SIGINT/SIGTERM).
- `Sandbox.set_permissions` wraps it and enforces only inside a container,
gated on CODER_EVAL_IN_CONTAINER — NOT on sandbox.driver, which the
in-container entrypoint rewrites to "tempdir".
- Docker mounts a throwaway read-write copy of the reference at
/work/references (`:ro` cannot be chmod'd — EROFS), masks its in-task-dir
original with an empty tmpfs, and drops DAC_OVERRIDE/DAC_READ_SEARCH/
FOWNER/CHOWN.
- Criteria address the reference via `$REFERENCE_DIR` (judge `files:`) and the
REFERENCE_DIR env var (`run_command`). `reference_code` is removed from the
criteria SPI; `CheckContext.reference_dir` supersedes it.
- `tasks/anti_cheat_reference` is an adversarial probe wired into CI smoke.
KNOWN GAP (documented in CLAUDE.md, docs/DOCKER_ISOLATION.md and the module):
this is defense-in-depth, not a boundary. chmod(2) is gated on
owner-or-CAP_FOWNER and the container runs as root owning the copy, so an
agent that deliberately runs `chmod 755 /work/references` regains access
(verified on Docker Desktop even with all four caps dropped). Passive reads
are blocked; an adversarial agent is not. Full containment requires running
the agent as a non-root uid — follow-up.
BREAKING CHANGE: `reference: {code: ...}` and `reference: {file: ...}` are no
longer accepted; use `reference: {directory: <dir>}`. `reference_comparison`
now requires `reference_file`. Third-party criteria must drop the
`reference_code` parameter from `_check_impl`/`_check_impl_async`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review fixes: - Move permissions.py to a top-level leaf (fs_permissions.py): sandbox.py importing from orchestration/ was a layering inversion that would become a real cycle the moment the module needed anything from coder_eval. - Hard-fail when in-container with a declared reference but no /work/references mount. The old fallback resolved to the UN-masked reference under the :ro task-dir bind, which the window then cannot chmod (EROFS) — so the run would complete with the solution readable, reporting a normal pass/fail. - Scrub keys now match what the judge was actually shown: render_reference_dir truncates per file but collect_reference_secrets returned the untruncated text, so any file over max_file_chars was echoed verbatim into persisted transcripts and CriterionResult.details. - Bound the inlined reference block (200k chars) with an explicit omission marker; unbounded, a large tree blew the judge's context into a 0.0 score. - One token-matching rule (`path_uses_token`) shared by the judge resolver and the load-time validator; they disagreed, so `$REFERENCE_DIRECTORY/x` was a sandbox path to one and a reference consumer to the other. - Validator now also catches `$REFERENCE_DIR` in a run_command `command`. - `check`/`check_async` take turn_records/context keyword-only, matching `_check_impl*` — an untyped caller could otherwise bind a str to turn_records. - Drop the stale `reference_code` parameter from ~12 test overrides, including one that forwarded it positionally into a now keyword-only base (latent TypeError, unreached only because that checker never runs). - Set the crash-handler installed flag only after a successful install; drop the sub_agent alias; cache the resolved reference source instead of re-stat-ing it; fix the stale `task.reference.file` comment and the docs/EXTENDING.md SPI exemplar. CodeQL: - Replace `pytest.raises` with explicit try/except in three tests — CodeQL cannot model it as catching, so it reported the following asserts as unreachable and their variables as unused (alerts 77/78/80). - Use 0o700 instead of group-readable 0o750 in the mode-preservation test (75). - `await task` no-effect alert cleared by the same restructure (76). - `_handlers_installed` write is now the last statement in the guarded block (79). New coverage (+15): `_setup` actually arms the feature (mutation-verified); `$REFERENCE_DIR` resolver incl. the `$REFERENCE_DIRECTORY` lookalike and the no-reference case; REFERENCE_DIR env var set/unset; chmod-refusal skips its pop; unresolvable paths warn; out-of-order release of differing modes; deterministic render ordering and per-file truncation; rmtree of a tree left at mode 000; and a CI drift guard asserting EXPECTED_SMOKE_PASS_RUN and the smoke-pass globs match the tagged task set (both mutation-verified). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Move the install-once flag from a module-level global onto _PermissionStack. A mutable global read only by its own writer reads as dead to static analysis (py/unused-global-variable), and the state belongs with the registry whose entries the handlers restore. - Use string-target monkeypatch in the two new tests instead of re-importing coder_eval.fs_permissions, which the module already imports with `from ... import` (py/import-and-import-from). Also adds the crash-safety test the review flagged as missing: asserts the atexit hook and signal handlers install on the first push, do not re-install on the second, and that restore_all actually restores. Mutation-verified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Windows Smoke Test job runs the full suite on a Windows runner, where `chmod` honours only the read-only bit — so mode 000 never takes and 17 of the new assertions read back 0o555. `os.geteuid` also does not exist there. Skips the host-side POSIX-mode tests on win32 (matching the existing idiom in test_docker_runner_mounts.py) and replaces the geteuid root check with a portable helper. This is NOT a coverage gap for Windows users: the window is enforced only when CODER_EVAL_IN_CONTAINER=1, which only DockerRunner sets, and Docker Desktop on Windows runs LINUX containers — so the in-container orchestrator that performs the chmod is on Linux and behaves exactly as these tests assert. A Windows host only sees the window under `driver: tempdir`, where it is a deliberate no-op on every platform. The real behaviour stays covered by the Linux jobs and by tasks/anti_cheat_reference, which runs inside the container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…anti-cheat
Works through the pr:106 review (1 critical, 7 high, 15 medium, 11 low) plus
the outstanding CodeQL alerts. The theme is the layer that decides scores.
Scoring correctness (an eval-config error must not read as an agent failure):
* reference_comparison now raises CheckerMisuseError (-> FinalStatus.ERROR) for
a typo'd/unreadable/empty reference_file instead of returning a gating 0.0,
which was counted against the agent's pass rate and silently zeroed every row
of a dataset-fanned suite. reference_file also gained a load-time validator
(non-empty, relative, no ..), and the orchestrator pre-flights it during
_stage_reference so it fails before the agent burns a token.
* Reference integrity: the tree is hashed at staging and re-verified before
grading. The window is per-turn and the docker mount must be writable, so an
agent-backgrounded writer could previously overwrite the reference and drive
reference_comparison to 1.0. Mismatch now raises ReferenceTamperedError.
* Judge scrubbing is keyed on what reached the prompt (JudgeContext records the
reference bytes it attached), not on include_reference. The documented
`include_reference: false` + `files: [$REFERENCE_DIR/rubric.md]` combination
was persisting the solution verbatim into the archived judge transcript.
agent_judge now also passes max_file_chars, so the key matches truncated text.
Anti-cheat, fail closed:
* Stop dropping FOWNER/CHOWN. The in-container orchestrator that APPLIES the
window is the same root process with the same caps, so dropping FOWNER breaks
the harness's own chmod wherever the bind mount preserves a non-root owner
(native Linux). Verified: container root + uid-1000-owned dir + FOWNER dropped
-> "Operation not permitted". The drop only bit where it also disabled the
control. The re-chmod hole stays the documented KNOWN GAP (needs a non-root
agent uid, not a smaller capability set).
* A window that cannot be applied is now a hard error (strict=True whenever
Sandbox actually enforces), not a warning — an unprotected run must not be
indistinguishable from a protected one downstream.
fs_permissions:
* Crash handlers install from the event-loop thread. They were installed from
push(), which only runs on an asyncio.to_thread worker where signal.signal
raises ValueError into a swallowing except — so SIGTERM had NO restore, and
the flag latched anyway. Install now reports success and is retried if it
fails; a failed install logs WARNING.
* The acquire moved inside the try. asyncio.shield protects the inner task, not
the await, so a cancel on __aenter__ skipped the finally while every chmod
completed — leaving the path at 000 with no matching pop and a stale registry
entry that poisoned the next window.
* pop() keeps its entry when the restoring chmod fails, so restore_all still
holds the pre-window mode.
* Precise signal typing removes the blanket `# type: ignore`; SIG_IGN is handled.
Cleanup / dedupe:
* rmtree_restrictive moved to path_utils and WIRED IN — it had no production
caller, while both live cleanup sites used the swallowing rmtree its own
docstring rejects, orphaning mode-000 reference trees.
* _cleanup keys on _reference_staging_root, recorded before the copy, so a
copytree that raises does not leak a partial copy of the solution.
* Reference resolution and the copytree ignore list are shared between the two
drivers (resolve_host_reference_dir, REFERENCE_COPY_IGNORE).
API / validators:
* SuccessChecker.check/check_all/check_all_async take trailing args keyword-only
(reference_code was removed from the middle, so positional callers misbound
silently). scrub_reference is list[str] | None — str satisfied the old union.
* The reference-consumer validator narrows with isinstance instead of untyped
getattr, and matches ${REFERENCE_DIR} via a new command_uses_token seam.
* Sandbox gained a reference_dir constructor kwarg, mirroring task_dir.
Probe:
* verdict.txt/command_executed are weight 0 — `weight` does not soften a strict
AND gate, and this task blocks the e2e-smoke bucket.
* Step 3 used $TASK_DIR, which is NOT in the agent's environment, so it expanded
to empty and the tmpfs-mask check was inert. It now hunts the path with find.
* Verified live against a rebuilt container: 1/1, all six criteria, agent denied.
Lint (each traceable to a defect above): CE033 no unreferenced private helper in
src/, CE034 acquire inside the try of an async CM, CE035 no gating 0.0 from an
except OSError in a checker. All three verified to fire on the original code.
Also: 100% coverage on the in-container branch (was 0%, and docker is the only
driver the feature runs on); fs_permissions 89% -> 98%; assert the dropped-cap
set exactly; pin the probe's canary to its own detector; Makefile smoke globs
pinned to CI's; migration + score-comparability + image-lockstep notes; removed
an inert `# nosec` (bandit does not flag asyncio.create_subprocess_shell).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o wire The module docstring said "No in-tree caller needs the inner form today ... The stack exists so that adding one does not require reworking this module", which reads as textbook speculative generality — and the PR review duly filed it as one, recommending the stack and READ_ONLY_MODE be deleted. They should not be. The re-grant is a designed seam for live success criteria: early-stop verdicts run mid-turn, inside the 000 window, so a live criterion that consults the reference has to read it exactly while the agent cannot. A flat set/restore cannot express that and a refcount actively breaks it. Records that, plus the remaining work and the three non-obvious constraints found while scoping it (all deliberately NOT implemented here): * the window belongs around the watcher's verdict LOOP — tightest placement, which matters because a chmod is global state and the agent runs concurrently, so the re-grant is visible to it for as long as it is open; * that loop is a sync StreamCallback, so it needs a sync twin pushing onto the same registry; * live_verdict gains no parameter — it reads a per-task accessor, which must be a ContextVar and not os.environ, because `run_batch -j 8` shares one process and a process-global would leak one task's reference into a sibling's verdict under parallelism only. (REFERENCE_DIR today is set only in the env= dict for run_command subprocesses, so it is not readable in-process.) Also scopes it: only the reference is shielded, never the sandbox. Reading the static reference mid-turn cannot break LiveVerdict monotonicity; reading the half-written sandbox can, and is the end-state peeking live_verdict rules out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review addressed — all 8 blockers, 15 non-blocking, 11 nitsPushed as Blockers
On #6 — reproduced the failure directlyThe in-container orchestrator that applies the window is the same root process with the same caps as the agent, so dropping On #2 — declined, with the underlying doc defect fixedThe reviewer's facts were right ( What misled the review was the docstring, which said "No in-tree caller needs the inner form today... The stack exists so that adding one does not require reworking this module." Also note deleting the signal chaining (as #2 proposed) would have deleted the fix for #5. Beyond the listed findings
Verification
Ran the probe live against a rebuilt container: 1/1, all six criteria, agent denied at every step. Step 3's |
| """Dropped reference files change what the judge grades against, so the | ||
| omission must be visible in the prompt rather than read as 'the reference | ||
| doesn't implement that'.""" | ||
| import coder_eval.evaluation.judge_context as jc |
| await started.wait() | ||
| task.cancel() | ||
| with contextlib.suppress(asyncio.CancelledError): | ||
| await task |
…the task dir
The `--cap-drop DAC_OVERRIDE --cap-drop DAC_READ_SEARCH` added for the
reference anti-cheat broke EVERY `driver: docker` task. The container runs as
root but does not own the framework-owned bind mounts -- on native Linux they
preserve the uid that ran coder-eval -- so all its access is an "other" access
that only ever worked via the capability. The in-container orchestrator died on
its first `open('/work/output/task.log', 'w')` with EACCES, taking
byod_smoke_test (which has nothing to do with references) down with it. macOS
Docker Desktop hid this: virtiofs reports the mount as root-owned.
`grant_container_access` widens the framework-owned mounts host-side, so access
goes through the `other` bits instead of a capability. `chmod -R o+rwX`
semantics; read-only for what the container merely consumes. The drop and the
widening are counterparts -- drop without widening kills every docker task,
widen without dropping makes the mode-000 window a no-op.
Also replaces the symmetric `-v <host task dir>:<host task dir>:ro` mount with
a shielded COPY at /work/task_dir, held at mode 000 for every agent turn
alongside the reference. Verified against a real container: `:ro` makes the
window inexpressible (`chmod: Read-only file system`), and read-write without a
copy chmods the operator's own `tasks/` tree (host dir came back 0600; cleanup
then failed with Permission denied). This retires the `--tmpfs` mask and closes
a leak it could not reach -- a flat `tasks/foo.yaml` has parent `tasks/`, so the
old mount exposed every sibling task's reference solution.
Symmetry was never load-bearing: run_task_internal_command uses --task-dir only
to seed TASK_DIR, and never re-reads the path. TASK_DIR is exposed solely to
run_command criteria, so the agent loses nothing legitimate.
Does NOT hide the task definition: task.yaml is also staged at /work/input, and
that mount is untouched by the window.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reference solutions were readable by the agent under evaluation — it shares a filesystem with the harness, so an agent could grep the task directory for the answer instead of solving the task. This closes the passive-read path and adds an adversarial probe to CI.
Read this first: known gap
This is defense-in-depth, not a boundary.
chmod(2)is gated on owner-or-CAP_FOWNER, and the container runs as root, which owns the bind-mounted reference copy. An agent that deliberately runschmod 755 /work/referencesregains access:Verified on Docker Desktop even with
DAC_OVERRIDE,DAC_READ_SEARCH,FOWNERandCHOWNall dropped — the owner check passes with no capability at all. Dropping those caps does close it where the mount preserves a non-root owner (native Linux), so they are kept.Passive reads are blocked; an adversarial agent is not. Full containment requires running the agent as a non-root uid that does not own the reference — deliberately left as follow-up rather than bolted on here. Documented in
CLAUDE.md,docs/DOCKER_ISOLATION.md,docs/TASK_DEFINITION_GUIDE.md, the module docstring, and the probe task itself, so nobody reads a green probe as "the reference is unreachable".What changed
task.referenceis directory-only.code:/file:removed — a directory is the only shape that can be permission-gated as a unit. The removed keys raise a migration error naming the replacement.reference_comparisongains a requiredreference_file.orchestration/permissions.py::set_permissions— async CM that chmods paths for a block. Windows stack, so a nested re-grant (mode=READ_ONLY_MODE) restores the enclosing mode, not the original; the stack also subsumes what a refcount would do. Crash-safe:finally+asyncio.shieldon both halves +atexit+ chained SIGINT/SIGTERM.Sandbox.set_permissionsenforces only inside a container, gated onCODER_EVAL_IN_CONTAINER— not onsandbox.driver, which the in-container entrypoint rewrites totempdir(a driver-based gate would silently disable the feature on exactly the path that needs it; regression-guarded)./work/references(:rocannot be chmod'd — EROFS), empty tmpfs masking the in-task-dir original, four caps dropped.$REFERENCE_DIRtoken in judgefiles:+REFERENCE_DIRenv forrun_command.reference_coderemoved from the criteria SPI;CheckContext.reference_dirsupersedes it.tasks/anti_cheat_reference— adversarial probe, taggedsmoke-pass, wired intoe2e-smoke(EXPECTED_SMOKE_PASS_RUN7→8; the glob names the subdir explicitly becausetasks/*.yamlis not recursive).The task directory is deliberately not shielded: under docker it is a
:romount (EROFS) and the same YAML is readable at/work/inputregardless, so it only produced a per-turn warning.How the bugs were found
The probe task caught a real bug on its first live run — the agent read
/work/referencesin full, because a:romount can't be chmod'd so the code was shielding an in-container staged copy while the real mount stayed readable. Every unit test passed; only an agent reaching for the actual path exposed it. A subsequent 8-axis review found thechmod-restore bypass above, plus:task_dirshielding was inert under docker,resolve_reference_dirkeyed container detection on a bare/work/referencesprobe (would hijack any host with that path), the acquire half of the window wasn't cancellation-shielded,_stage_referenceleaked its tempdir on copy failure, andagent_judgesilently dropped$REFERENCE_DIRentries wheninclude_reference: false. All fixed here.Verification
make check/make lint(177) /make test(3955 passed, 8 skipped) green.make typecheckunchanged at 3 pre-existingopenai_codeximport errors (confirmed identical on a clean checkout).tasks_run=8matchesEXPECTED_SMOKE_PASS_RUN._setupwiring, and the stack semantics.Breaking changes
reference: {code:}/{file:}→reference: {directory:}.reference_comparisonrequiresreference_file. Third-party criteria must dropreference_codefrom_check_impl/_check_impl_async. Judge prompts change shape for reference-bearing tasks (agent_judgemounts instead of inlining;llm_judgeinlines labelled per-file blocks), so such suites need re-baselining.🤖 Generated with Claude Code