feat(evaluate): grade a driver: docker row inside a container of its own image - #161
Merged
Conversation
…s own image
Detached grading refused `driver: docker` outright, on the reasoning that
"grading cannot start a container (there is no agent to run in it)". The premise
was wrong. `DockerRunner` starts a container that runs `_run-task-internal`,
which builds an in-container Orchestrator — and a grade is that same Orchestrator
with `prior_result` set and no agent. The mechanism existed; it was never wired.
why it matters, measured
------------------------
`tasks/byod_smoke_test.yaml` asserts `test -f /opt/byod_marker`, a file baked
into its image. The IDENTICAL row:
graded in a container -> SUCCESS 1.000 (no stamp)
graded on the host -> FAILURE 0.000 (graded_on_host: True)
The host is answering "is that marker on THIS machine", which nobody asked. That
is the general shape for any container task: its criteria address the image's
paths and toolchain. So the refusal was protecting against a real hazard by
making the feature unusable for the tasks that most need isolation.
the shape
---------
`_should_grade_in_container` -> `_grade_in_container` -> `DockerRunner(
prior_result=, grade_workspace=)`. The container gets TWO mounts, and their
separation is the whole design:
* the grading pass's own fresh run_dir at CONTAINER_OUTPUT_DIR, whose task.json
the host folds back into the row — preserving task.execute.json exactly as on
the host path;
* the executed workspace at CONTAINER_GRADE_WORKSPACE, read-WRITE and NOT a
copy, adopted rather than written over. Read-write because criteria
legitimately mutate what they grade; not a copy because that is what the host
path already proved wrong (the template filter drops node_modules / dist /
build / .venv, so a criterion reading those fails as a copying artifact).
The container half reuses the same `regrade_in_place` rather than restating it —
`evaluate`'s run-dir mode and `run --resume` drifted apart once already, and that
is why that function exists. It is driven by `context.json`'s `regrade` flag plus
a staged `prior.json`, both coerced on arrival: a truthy `"false"` would re-RUN
the agent over the workspace the operator asked only to grade.
A container-graded row carries NO `graded_on_host` stamp, so it is
indistinguishable from a `run` row. That parity is the point.
`--allow-host-grading` survives as the escape hatch (no docker here; criteria
known host-portable) and still stamps.
gated on the env var, never the driver
--------------------------------------
`IN_CONTAINER_ENV`, because the in-container entry point rewrites
`docker` -> `tempdir` before building its Orchestrator — a driver-based test
reads an already-changed value, and a grading container would dispatch a grading
container. That env var had four literal spellings; it now has one definition in
`models/container_paths.py`.
CE052 matched only the literal, so the migration made it read a constant-based
gate as NO gate — a rule instructing the author to paste the literal back,
arguing against the SSOT it exists to reinforce. It now accepts both spellings.
CE021 also caught the new `prior.json` parse; it degrades to a named message
rather than surfacing as "container exited without producing task.json".
verified
--------
End to end against real docker, both entry points:
- `evaluate <run_dir>`: hello_world_docker 3/3 SUCCESS, byod 1/1 SUCCESS,
no flag, no stamp, driver still recorded as docker, duration preserved to
the digit, post_run run once in the grading phase.
- `run --resume`: byod SUCCESS, exit 0, unstamped.
- negative control: same byod row with --allow-host-grading -> FAILURE 0.000,
stamped.
11 new tests (routing truth table incl. the recursion gate, dispatch payload,
both refusals, wire format, and controls asserting an ordinary run stages and
mounts neither).
ruff clean, pyright 0 errors, 455 lint rules pass,
5406 passed / 6 skipped at CI's -n 2, 92.54% coverage.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…grading An 8-axis review of the container-grading commit found two score-altering defects, one security regression and four correctness bugs. Fixes, in order of blast radius: **The dispatch was outside the trust gate.** A run directory is a shareable artifact, so its recorded config is untrusted input — but grading a `driver: docker` row DISPATCHES A CONTAINER built from the recorded sandbox block. The record names the image; a container of it runs with the default credential allowlist (ANTHROPIC_API_KEY, UIPATH_ACCESS_TOKEN, AWS_BEARER_TOKEN_BEDROCK ...) forwarded in, a copy of ~/.claude mounted, and a pinned --entrypoint the image supplies. `embedded_commands` walked only success_criteria and post_run, so a run dir whose criteria were all `file_exists` reached `docker run` with no flags — where the previous release refused. That is the identical blind spot the function's own docstring already describes for `--copy` provisioning, one layer up. `include_container_dispatch` now scans it on the in-place path, as post_run is. **Version skew turned a grade into a fresh agent run.** `regrade` crosses the boundary only through context.json; an image predating this feature ignores the key, ignores prior.json, and runs the agent — and the host folded that fabricated trajectory back as the recorded row's verdict. `_assert_regrade_honored` is the exact sibling of `_assert_grade_honored`, keyed on `started_at` moving, and quarantines the record rather than refusing only in memory. **The record named a path that exists on no host.** A container run recorded `source_file` as `/work/task_dir/task.yaml`, so `_grade_in_container`'s `task_file is None` guard passed and `_prepare_task_dir_mount`'s `if not source.is_dir(): return` then mounted nothing — every `$TASK_DIR` criterion silently resolving against the wrong tree. `Orchestrator.recorded_task_file` is the path twin of `recorded_task`; the host forwards its own path. **The grading container inherited the caller's run_dir.** `run --resume` passes the executed row's OWN directory, where `_parse_result_or_raise` (keyed on task.json existing, discarding returncode) read a dead container's stale pre-grade record back as a successful grade, and where docker.log was truncated. It now gets a scratch dir and the verdict is folded back. **The workspace mount skipped `grant_container_access`.** The only framework-owned mount to do so, while DAC_OVERRIDE/DAC_READ_SEARCH are dropped — so a host-owned workspace failed EACCES and booked a gating 0.0 that reads as an agent failure. Also: `pre_run` is not re-run in the second container, so a criterion depending on out-of-workspace state scores 0.000 for a trajectory `run` scores 1.000 (3d-scan-calc symlinks /root/mass_report.json in pre_run and its verifier asserts it). Re-running would trade it for the deliverable-clobbering bug `_skip_pre_run_for_adopted` prevents, so it is warned at dispatch and documented. Plus CE056 (no bare CODER_EVAL_IN_CONTAINER literal outside container_paths — the migration converted four readers and left the single WRITER, so a rename would have disarmed the reference anti-cheat window, the reference mount, the recursion guard and the watchdog together, silently), stale docstrings/help/guide that still asserted "grading cannot start a container", RegradeError reaching the user as a traceback, OSError on the prior.json read, and tests for the container half (was 48.73% covered), the dispatch ordering, and the fail-closed baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akshaylive
requested review from
CarlesUIPath,
bai-uipath,
tmatup and
uipreliga
as code owners
September 9, 2026 21:43
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
uipreliga
self-requested a review
September 9, 2026 23:45
uipreliga
approved these changes
Sep 9, 2026
uipreliga
left a comment
uipreliga
left a comment
Collaborator
There was a problem hiding this comment.
Fix what you agree with and 🚢
…ed grading Addresses the 8-axis review on #161: 6 blockers, 10 non-blocking findings and 7 nits. The two that change verdicts: `recorded_task_file` was not threaded through the in-container regrade branch (`_grade_recorded_run` / `regrade_in_place`), so every container-graded row re-recorded `/work/task_dir/task.yaml` as its `source_file` — the exact defect that parameter was added to fix, reintroduced one caller down, and enough to make a later `evaluate <run_dir>` refuse or mount the wrong task dir. The two known equivalence gaps are now STAMPED on the row, not only logged: `graded_without_pre_run` and `graded_with_rebuilt_image`. `stamp_host_grading`'s own docstring already gave the argument — a console warning does not travel with task.json into run.json, the reports or the evalboard — and 3 of the 10 in-tree docker tasks hit the pre_run case with no flags via `execute` -> `run --resume`. The dockerfile_path warning the user guide promised did not exist; it does now. Diagnosability and host safety on the grading container: - logs are folded out of the scratch dir in a `finally`, not only on success. A failed container deleted `docker.log` while its own error said "See {path}". `grade.log` — a documented run-layout artifact — was never folded out at all, so a `driver: docker` row was the one shape a detached grade left without one. - both log copies refuse a symlinked destination (`shutil.copy2` follows one; the sibling verdict write uses `write_text_atomic` for exactly that reason). - the verdict write raises RegradeError, never a bare OSError: it sits outside the dispatch `try`, where `evaluate` let it escape into Typer after a SUCCESSFUL grade and `run --resume` reported a correct verdict as a failure. - `grant_container_access` returns what it widened and `run()` restores it. The graded workspace is the caller's tree; an operator-supplied `--workspace` was left world-writable permanently. - a container grade emits its own `CoderEval.Task.End`, mirroring batch.py. Every container runs `TELEMETRY_ENABLED=false` under "container silent, host emits once"; the grading path had inherited only the silent half. Consent prompt: - the dispatch renders as ONE command. Argv fragments were appended as separate entries, so one `docker build` was reported as "4 shell command(s)". - it now names every host path exposed, not just `sandbox.docker.*`: the task directory copied from the recorded `source_file`'s parent, the auto-mounted plugins / template dirs / system_prompt_file, and the writable ~/.claude copy. - `include_setup_phase` + `grade_in_place` collapse to one parameter. They were exact complements at every call site, with nothing rejecting the incoherent pairings — on a flag that gates a security disclosure. Also: CE053 widened to the run-log filenames (docker.log was three unrelated literals across two packages, and its consumer skips silently when absent); `_quarantine_record` extracted from the two identical skew refusals; the stale "instead of refusing" text corrected in both USER_GUIDE flag tables and the run CLI help; the duplicated --allow-host-grading paragraph and mis-pointed "rely on it" fixed; Rich markup escaped on the four handlers that render recorded strings; the Sandbox/prior Optionals removed in favour of real narrowing; test doubles bound to DockerRunner's real signature. Tests: the in-container regrade branch is driven end to end (deleting either `recorded_task` or `recorded_task_file` now fails, verified by mutation), plus the failure-path log rescue, the symlink refusal, the OSError wrap, both stamps, the one-command rendering, the host-path disclosure, telemetry parity and the mode restore. 5447 passed, 6 skipped; ruff and pyright clean. Not done, deliberately: pinning the resolved image by digest needs the RUN path to record it first, so the row says the rebuild happened rather than the guide claiming a control that does not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…py + test_detached_grading_boundaries.py CodeQL flagged 9 open alerts (all `py/import-and-import-from`, note severity) introduced by the last commit's new test functions: each does a local `import coder_eval.X as alias` for monkeypatching a module attribute, while the same file also has a module-level `from coder_eval.X import (...)` for the same fully-qualified module — confusing, per the rule's own rationale, even though nothing here is a correctness bug. Fixed by switching the local imports to the "from parent import submodule as alias" form (`from coder_eval.isolation import docker_runner as dr`, `from coder_eval.orchestration import regrade as rg`, `from coder_eval import models`), which still binds the real module object — required for `monkeypatch.setattr(dr, "DockerRunner", ...)` to affect the module's own `from ... import DockerRunner` lookups at call time — without colliding with the top-level `from coder_eval.X import (...)` imports of the same modules. No behavior change: 109/109 tests in the two files still pass, ruff/pyright clean. Co-Authored-By: Claude Sonnet 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
3 participants
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
coder-eval evaluate <run_dir>andrun --resumenow grade adriver: dockerrow inside a container of the task's own image, instead of on the host.
That is the only place its criteria mean what they meant during the run.
tasks/byod_smoke_test.yamlassertstest -f /opt/byod_marker, a file bakedinto its image: the identical row scores
SUCCESS 1.000graded in a containerand
FAILURE 0.000graded on the host — the host answering a question nobodyasked. A container-graded row carries no
graded_on_hoststamp, so it isindistinguishable from a
runrow, which is the parity that makes theexecute/evaluate split honest.
--allow-host-gradingsurvives as the escape hatch (no docker here; criteriaknown to be host-portable) and still stamps.
So the resulting model is:
executeevaluate/run --resumedriver: tempdirdriver: dockerReview
The first commit was reviewed across 8 axes and iterated on twice. The review
found 2 score-altering defects, 1 security regression and 4 correctness bugs;
all are fixed in
03f79f4e. The highest-value ones:The dispatch was outside the trust gate. A run dir is a shareable artifact,
so its recorded config is untrusted — but the dispatch builds a container from
the recorded
sandbox.dockerblock. The record names the image, and a containerof it runs with the default credential allowlist forwarded in and a copy of
~/.claudemounted.embedded_commandswalked onlysuccess_criteriaandpost_run, so a run dir whose criteria were allfile_existsreacheddocker runwith no flags — where the previous release refused outright.This is the same blind spot the function's own docstring already describes for
--copyprovisioning, one layer up. Now scanned on the in-place path, exactlyas
post_runis.Version skew turned a grade into a fresh agent run. An image predating this
change ignores the unknown
regradekey and runs the agent; the host folded thatfabricated trajectory back as the recorded row's verdict, and billed for it.
_assert_regrade_honoredis the exact sibling of_assert_grade_honored.The record named a path that exists on no host. A container run recorded
source_fileas/work/task_dir/task.yaml, so thetask_file is Noneguardpassed and
_prepare_task_dir_mountsilently mounted nothing — every$TASK_DIRcriterion resolving against the wrong tree, with no error.
The grading container inherited the caller's
run_dir. Onrun --resumethat is the executed row's own directory, where a dead grading container's stale
pre-grade
task.jsonwas read back as a successful grade, anddocker.logwastruncated.
The workspace mount skipped
grant_container_access— the onlyframework-owned mount to do so while the DAC caps are dropped, so a host-owned
workspace failed EACCES and booked a gating
0.0that reads as an agent failure.Known limitation (documented, warned at dispatch)
The grading container is a second, fresh container. Only the workspace
crosses, and
pre_runis not re-run — so a criterion depending on statepre_runput outside the workspace does not see it.tasks/samples/skillsbench/3d-scan-calcsymlinks/root/mass_report.jsoninpre_runand its verifier asserts that path, so it would score 0.000 for atrajectory
runscores 1.000. Re-runningpre_runwould trade this for thedeliverable-clobbering bug
_skip_pre_run_for_adoptedexists to prevent, so itis warned loudly rather than silently wrong. Same for
dockerfile_pathimagedrift between the two phases — there is no
reference_digest-style pin on theimage yet.
Lint
CE056 — no bare
CODER_EVAL_IN_CONTAINERliteral outsidemodels/container_paths.py. The migration converted all four readers and leftthe single writer, so a rename would have disarmed the reference anti-cheat
window, the reference mount, the grading-container recursion guard and the
watchdog together, with nothing failing. CE052 can't catch it — that rule
inspects
ifguards, and the writer is not one.Verification
ruff check+ruff format --check: cleanpyright: 0 errorspytest: 5432 passed, 6 skippedthe skew guard, the trust gate, and the fail-closed baseline
🤖 Generated with Claude Code