Add protected mock service and plugin bundle projection - #95
Conversation
…isolation wrapper execs it
…served-path assertion
…e isolation default
…dir and skip POSIX-only relative symlinks on Windows
feat(protected-mock): add subset fixture matching and harden mockd startup
|
Claude finished @dmorosanu's task in 1m 58s —— View job Code Review in Progress
|
1dd19ec to
7a2c59a
Compare
uipreliga
left a comment
uipreliga
left a comment
There was a problem hiding this comment.
Review: coder_eval — pr:95
Scope: pr:95 · branch feat/protected-mockd · b855fd9 · 2026-08-11T09:59Z · workflow variant
Change class: complex — introduces a privilege-separated container security boundary (dedicated agent UID/GID, capability drop, protected grader paths), a new fixture-backed mockd Unix-socket RPC service, and manifest-verified plugin-bundle projection; correctness requires reasoning about trust boundaries, symlink/path handling, control flow, and schema semantics.
The isolation design itself is solid and well-documented — type safety, architecture, and test structure all hold at 8+ — but Evaluation Harness Quality (4.4) is the real risk: a default-on, fail-closed agent_isolation shipped with zero task-YAML migration turns all nine in-repo driver: docker tasks into score-0 ERROR rows, and several grading-trust seams (root post_run executing agent-authored files in the agent-owned sandbox, agent-writable cli_called evidence, UID-2000 processes still alive while criteria are scored) plus silently truncated plugin bundles mean the harness can change a task's score or final_status for identical agent output, so this should not land until the default-on breakage and those trust seams are closed.
Summary
| Axis | Score | 🔴 | 🟠 | 🟡 | 🔵 | Top Issue |
|---|---|---|---|---|---|---|
| 1. Code Quality & Style | 7.9 / 10 | 0 | 1 | 2 | 1 | docker_runner complexity blowout: DockerRunner grows to a ~1070-line god class and _build_argv reaches CC F(52) under an unratcheted noqa |
| 2. Type Safety | 8.4 / 10 | 0 | 1 | 1 | 1 | Fixture JSON is hand-parsed with no unknown-key rejection, so a typo'd key silently degrades the served response |
| 3. Test Health | 8.4 / 10 | 0 | 1 | 1 | 1 | New PR surfaces ship effectively untested: protected-mock wire path, agent_identity UID-drop, docker_runner fail-closed gates, plugin_bundle fail-closed arms, validate_tool_name, resolve_protected_mock_paths |
| 4. Security | 7 / 10 | 0 | 2 | 2 | 0 | Protected mode blocks privileged dynamic criteria but still runs task post_run shell as root in the agent-owned sandbox (and the docs' rejection list omits post_run) |
| 5. Architecture & Design | 8.5 / 10 | 0 | 1 | 1 | 0 | Antigravity pops CODER_EVAL_AGENT_ISOLATION from os.environ before the agent_isolation_enabled() check, so the protected-mode HOME=/home/agent override never fires |
| 6. Error Handling & Resilience | 5.4 / 10 | 1 | 1 | 1 | 1 | Per-task Docker staging tempdir leaks whenever a task stages a plugin bundle or protected-mock fixtures — 0o555/0o444 trees defeat shutil.rmtree(ignore_errors=True) (leaked content is the sanitized bundle + mock fixture copies, NOT the ~/.claude credential copy) |
| 7. API Surface & Maintainability | 8 / 10 | 0 | 2 | 0 | 0 | agent_isolation gates on a hardcoded agent allowlist (docker_runner.py:716) with no registry/SPI capability seam, so third-party agents hard-fail under driver: docker by default |
| 8. Evaluation Harness Quality | 4.4 / 10 | 1 | 2 | 1 | 1 | agent_isolation defaults to true and fails closed, but zero in-repo task YAMLs were migrated, so every driver: docker task aborts at preflight |
Overall Score: 7.3 / 10 · Weakest Axis: Evaluation Harness Quality at 4.4 / 10
Totals: 🔴 2 · 🟠 11 · 🟡 9 · 🔵 5 across 8 axes.
Blockers
-
[Axis 1] docker_runner complexity blowout: DockerRunner grows to a ~1070-line god class and _build_argv reaches CC F(52) under an unratcheted noqa (
src/coder_eval/isolation/docker_runner.py:1330) — This PR pushed the repo's worst function further over the line and papered it with the debt marker rather than extracting. Verified numbers: merge-base_build_argv= radon E (34) and ruff-clean; PR HEAD = F (52), and with the newly added marker stripped ruff reportsPLR0912 Too many branches (38 > 25)andPLR0915 Too many statements (102 > 80)(pyproject setsmax-branches = 25,max-statements = 80). The added line is literally:def _build_argv( # noqa: PLR0912, PLR0915 - one ordered rendering pipeline mirrors docker-run argv
The cost is not abstract: the isolation logic is threaded through the body as inverted guards on pre-existing loops —
if not cfg.agent_isolation:
for plugin in plugins:
_auto_mount(plugin.get("path") if isinstance(plugin, dict) else None)
...
if not cfg.agent_isolation and agent_cfg and agent_cfg.system_prompt_file:
...
if not cfg.agent_isolation and reference is not None:
so a single forgotten not cfg.agent_isolation re-mounts a raw host source tree at an agent-readable path with no lint or test signal. Same theme, same commit: the new _prepare_isolated_sources (line 745) lands at D (24) doing four unrelated jobs in one method (plugin bundling, template mounts, reference mounts, mock-fixture staging). Extract the isolation argv rendering into a _isolation_argv(...) -> list[str] helper (and split _prepare_isolated_sources on its four responsibilities) so the marker can come back off; docker_runner is the container entrypoint for every nightly run, which is exactly where an unreadable function is most expensive.
2. [Axis 2] Fixture JSON is hand-parsed with no unknown-key rejection, so a typo'd key silently degrades the served response (src/coder_eval/protected_mock/server.py:87) — fixture: files are user-authored task material (docs/TASK_DEFINITION_GUIDE.md shows fixture: ./fixtures/uip-troubleshoot.json), yet unlike every sibling config surface in the repo they get no Pydantic model and no extra="forbid" (rubric item 18). The parser reads only the keys it knows, defaulting everything else away:
87: exit_code = raw.get("exit_code", 0)
88: stdout = raw.get("stdout", "")
89: stderr = raw.get("stderr", "")
124: match_mode = entry.get("match_mode", "exact")
I ran _load_tool on a fixture containing "match_modes": "subset", "stdou": "PAYLOAD\n" and a stray top-level "responsess". It loaded without complaint and produced CommandResponse(exit_code=0, stdout='', stderr='') under an exact match key — the subset rule silently vanished (subset rules: []) and the payload silently became empty. The agent then observes an empty successful uip call, and the task scores wrong with no diagnostic on the host or in the container.
Fix: define Pydantic models for the fixture envelope and its response entries (argv, match_mode, exit_code, stdout, stderr) with model_config = ConfigDict(extra="forbid"), and validate the fixture host-side in orchestration/task_loader.py::resolve_protected_mock_paths (which today only checks fixture.is_file()), so a typo fails at task load rather than degrading a run silently. Add a test asserting an unknown key in a fixture entry raises.
3. [Axis 3] New PR surfaces ship effectively untested: protected-mock wire path, agent_identity UID-drop, docker_runner fail-closed gates, plugin_bundle fail-closed arms, validate_tool_name, resolve_protected_mock_paths (src/coder_eval/protected_mock/server.py:259) — Every test in tests/test_protected_mock.py calls the unbound method with a fake self (ProtectedMockServer.dispatch(fake_server, "uip", [...]), e.g. line 111 fake_server = MagicMock()), so ProtectedMockServer is never constructed and ProtectedMockHandler never runs. Confirmed uncovered in server.py: 259-280 (handle() — including the caller-identity gate if self._peer_uid() not in {0, AGENT_UID}: ... CommandResponse(77, ...) at 259-261 and the "protected mock: invalid request size" / "protected mock: invalid request" rejections at 262-278), 283-288 (_peer_uid SO_PEERCRED unpack), 291-317 (_write, including the MAX_RESPONSE_BYTES truncation at 304-316), 321-336 (serve(), including chown(socket_path, geteuid(), MOCK_RPC_GID) and socket_path.chmod(0o660) — the access-control gate), 340-343 (main). The client mirror is equally dark: client.py 44.79%, missing 36-48 (_receive_line) and 72-83/89-94 (send, envelope validation, stdout/stderr emission) — i.e. no test exercises a successful invoke(). runtime.py 76 and 84 (if socket_path.exists(): break and the yield) are also uncovered, so running_mock_server never reaches a running server, and 38-39 (if config_path is None: yield; return) — the path taken by every non-protected container run — is untested too. A client/server envelope mismatch would only surface in the docker live tests, which SKIP here. Add one in-process round-trip test: bind a ProtectedMockServer on a tmp socket in a thread, monkeypatch client.SOCKET_PATH/protocol.SOCKET_PATH and AGENT_UID (or the _peer_uid result) to the test process's uid, and assert exit code + stdout for a matched argv, a budget-exhausted call, an oversized request, and a malformed envelope. Also give _fake_server (line 68) a MagicMock(spec=ProtectedMockServer) so a future attribute read on self fails instead of silently returning a Mock.
4. [Axis 4] Protected mode blocks privileged dynamic criteria but still runs task post_run shell as root in the agent-owned sandbox (and the docs' rejection list omits post_run) (src/coder_eval/isolation/docker_runner.py:736) — _validate_agent_isolation_compatibility fails closed only on criterion types — docker_runner.py:732-736 reads:
unsupported_criteria = sorted(
{
criterion.type
for criterion in self.rt.task.success_criteria
if criterion.type in {"agent_judge", "run_command", "uipath_eval"}
}
)
with the rationale at docker_runner.py:729-731: "These criterion implementations can execute … arbitrary task-authored commands in the privileged harness. If that execution imports candidate-controlled code, it can act as a confused deputy and publish hidden grader bytes." task.pre_run / task.post_run are that exact primitive and are not rejected. Orchestrator._run_command_list (orchestrator.py:2178-2184) runs them as root:
proc = await asyncio.create_subprocess_shell(
cmd.command,
cwd=str(sandbox_dir),
...
) # nosec B602,B604 - commands come from task YAML, not user input
and sandbox_dir was handed to UID 2000 by the new await self._grant_current_sandbox_to_agent() at orchestrator.py:479 and :1085. Failure scenario: a task with post_run: [{command: "pytest -q"}] (or make check, npm test, ./verify.sh) — the agent, running as UID 2000, drops a conftest.py / edits Makefile / rewrites the script in its own workspace; root then executes it at orchestrator.py:2178 and can read /opt/coder-eval/grader/{input,task_dir,references,output} (0700 root) and rewrite task.json. Fix: extend the guard to reject non-empty task.pre_run/task.post_run under agent_isolation (or run them through CONTAINER_DROP_SHIM), and update the now-stale # nosec B602,B604 justification — the command string is still trusted, but the working directory no longer is. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:H
5. [Axis 4] passthrough_argv_prefixes proxying: only the argv prefix is checked, and the real tool is spawned with mockd's full inherited environment (no env=), with stdout/stderr returned verbatim to the agent (src/coder_eval/protected_mock/server.py:236) — ProtectedMockServer.dispatch (server.py:223) only checks the prefix: if any(tuple(argv[: len(prefix)]) == prefix for prefix in state.passthrough_prefixes) — everything after the prefix is attacker-supplied. It is then executed verbatim at server.py:236-245:
result = subprocess.run(
[state.passthrough_executable, *argv],
stdin=subprocess.DEVNULL,
capture_output=True,
...
)
with no env= argument, so the child inherits mockd's environment, which itself inherits the root harness environment (protected_mock/runtime.py:49 spawns mockd via subprocess.Popen([SERVER_LAUNCHER, ...]) with no env=, and docker/coder_eval_mockd.sh exec setpriv … -- "$@" preserves it). That environment carries ANTHROPIC_API_KEY, UIPATH_ACCESS_TOKEN, LITELLM_AUTH_TOKEN, and AWS_BEARER_TOKEN_BEDROCK (models/sandbox.py:218-230 env_passthrough defaults) — including the one utils.py:AGENT_ENV_SCRUB_VARS deliberately strips from the agent ("Left inherited it reaches the dropped agent process…"). Failure scenario: a task configures passthrough_argv_prefixes: [[docsai, ask]]; the agent runs uip docsai ask <anything> <any-flag>; mockd (UID 2100, which can read the 0500 /opt/coder-eval/mock/fixtures) executes the real tool with any file-reading/config/output flag the tool exposes and returns its stdout to the agent at server.py:247 — recovering the protected fixture and/or the credentials the scrub was meant to withhold. Fix: pass an explicit minimal env= to subprocess.run (drop every credential not required by the proxied tool) and require the fixture to declare an allowlist for the argv tail, not just the prefix (e.g. a per-prefix max-arity plus a permitted-flag set), instead of relying on the docs' advisory "Do not use a broad prefix". CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:L/A:N
6. [Axis 5] Antigravity pops CODER_EVAL_AGENT_ISOLATION from os.environ before the agent_isolation_enabled() check, so the protected-mode HOME=/home/agent override never fires (src/coder_eval/agents/antigravity_agent.py:436) — The isolation switch is transported as a process-global env var (CODER_EVAL_AGENT_ISOLATION) read live at 16 call sites via agent_identity.agent_isolation_enabled() instead of being plumbed through the config objects the rest of the framework uses. In _harness_spawn_env() that ambient design bites immediately: lines 424-429 pop every CODER_EVAL_* name out of os.environ —
scrubbed = {
name: os.environ.pop(name)
for name in list(os.environ)
if name in AGENT_ENV_SCRUB_VARS
or (name.startswith(AGENT_ENV_SCRUB_PREFIXES) and name not in AGENT_ENV_PASSTHROUGH_VARS)
}AGENT_ENV_SCRUB_PREFIXES = ("CODER_EVAL_",) and AGENT_ENV_PASSTHROUGH_VARS = ("CODER_EVAL_AGENT_ALLOW_RPC",) (src/coder_eval/utils.py:102-104), so CODER_EVAL_AGENT_ISOLATION is popped. Ten lines later the code branches on the value it just deleted:
if agent_isolation_enabled(): # line 436 — always False here
os.environ["HOME"] = AGENT_HOMEagent_isolation_enabled() re-reads os.environ.get(AGENT_ISOLATION_ENV) (isolation/agent_identity.py:22), so under protected mode this is always False and the Antigravity harness spawns with the container's root HOME=/root — which the dropped UID 2000 process cannot read (install -d -o root -g root -m 0700 …, and docker_runner.py:1383-1384 deliberately stops forwarding HOME). The symmetric restore at line 447 happens after os.environ.update(scrubbed) at line 441, so it evaluates True — the asymmetry is what hides the bug. Fix: hoist isolated = agent_isolation_enabled() above the pop (or better, pass isolation in as constructor state rather than re-reading a global that this same function mutates), and add a test asserting the spawn-window env has HOME == AGENT_HOME for Antigravity under isolation — tests/test_docker_identity_isolation.py only asserts the Codex login-shell case (assert f"export HOME={AGENT_HOME}" in content, line 115). The scattered if agent_isolation_enabled(): pattern is also a mutable-global coupling across concurrently-running tasks: on the host, this unconditional pop removes every CODER_EVAL_* var (including CODER_EVAL_IN_CONTAINER) for the whole spawn window, which other tasks in the same run_batch process read concurrently.
7. [Axis 6] Per-task Docker staging tempdir leaks whenever a task stages a plugin bundle or protected-mock fixtures — 0o555/0o444 trees defeat shutil.rmtree(ignore_errors=True) (leaked content is the sanitized bundle + mock fixture copies, NOT the ~/.claude credential copy) (src/coder_eval/isolation/docker_runner.py:666) — run()'s cleanup is await asyncio.to_thread(shutil.rmtree, staging, ignore_errors=True) (docker_runner.py:666). Under agent_isolation (default true), _prepare_isolated_sources and stage_bundle now write read-only trees INSIDE that same staging dir: plugin_bundle.py:178 bundle_dir.chmod(0o555) / :184 child.chmod(0o555) / :188 child.chmod(0o444), and docker_runner.py:830 destination.chmod(0o444) / :843 config_path.chmod(0o444) / :844 mock_root.chmod(0o555). Unlinking a file requires write permission on its parent directory, so rmtree cannot delete anything under a 0o555 dir; ignore_errors=True swallows the failure silently and no log line is emitted. Reproduced at PR HEAD: staging a one-file plugin bundle then shutil.rmtree(staging, ignore_errors=True) leaves agent-skills/plugin-0/skills/demo/SKILL.md and all its parents behind (staging survived: True). Because _prepare_host_mounts puts the lean copy of ~/.claude at staging/claude-home — and its docstring at docker_runner.py:1183 explicitly relies on this rmtree ("The copy lives under staging, which run() removes in its finally, so there is no extra cleanup to track") — every isolated task with a plugin or a protected mock leaves a full copy of the evaluator's Claude OAuth state plus staged task.yaml in /tmp/coder_eval_docker_* forever. This compounds across every task of every run. Fix: do not chmod host-side staging at all (the container reads these through a :ro bind mount; agent-readability is governed by the mount and the parent dir's mode, not by 0444/0555 on the source), or restore write bits before teardown, e.g. replace line 666 with an onexc/onerror handler that os.chmod(path, 0o700) on the parent and retries — and drop ignore_errors=True in favour of logging a warning so a future cleanup failure is visible.
8. [Axis 6] mockd passthrough budget (60 s) exceeds the protected-mock client socket timeout (5 s), so slow passthroughs surface to the agent as a fabricated exit 125 (src/coder_eval/protected_mock/server.py:42) — PASSTHROUGH_TIMEOUT_SECONDS = 60 (server.py:42) is used as timeout=PASSTHROUGH_TIMEOUT_SECONDS on the proxied subprocess.run (server.py:244), but the agent-side client sets connection.settimeout(CLIENT_TIMEOUT_SECONDS) with CLIENT_TIMEOUT_SECONDS = 5.0 (protocol.py:13, client.py:70) and that timeout covers the _receive_line recv. Any passthrough that takes longer than 5 s — which is the norm for the documented [['docsai','ask']] network-backed example — makes the client's recv raise socket.timeout (an OSError), caught by the broad handler at client.py:83, which writes protected mock client: service unavailable or invalid response: ... and returns exit 125. Meanwhile the budget was already spent: dispatch does state.remaining -= 1 at server.py:208, BEFORE the fixture match and before self._passthrough(state, argv) at server.py:222. So the agent sees a bogus infrastructure failure, and a retry spends a second budget unit; only an identical argv benefits from state.passthrough_cache, so a varying command (e.g. docsai ask "<question>") fails on every attempt until max_requests is exhausted. There is no test covering this (grep of tests/test_protected_mock.py finds no reference to CLIENT_TIMEOUT_SECONDS). Fix: make the client deadline strictly larger than the server's worst case (e.g. CLIENT_TIMEOUT_SECONDS = PASSTHROUGH_TIMEOUT_SECONDS + slack, or lower PASSTHROUGH_TIMEOUT_SECONDS below 5 s), and refund the budget when the response is an infrastructure error rather than a fixture answer.
9. [Axis 7] agent_isolation gates on a hardcoded agent allowlist (docker_runner.py:716) with no registry/SPI capability seam, so third-party agents hard-fail under driver: docker by default (src/coder_eval/isolation/docker_runner.py:716) — _validate_agent_isolation_compatibility gates on a literal set: supported_agents = { AgentKind.CLAUDE_CODE.value, AgentKind.CODEX.value, AgentKind.ANTIGRAVITY.value, AgentKind.NONE.value } (docker_runner.py:716-722), raising f"docker.agent_isolation has no verified UID-drop launch seam for agent type {agent_type!r}" (line 724) for anything else. That directly contradicts the extension point the project documents: AgentKind's own docstring says "These are NOT the closed set of valid agent types: agent.type is an open string validated against the AgentRegistry (which plugins extend via the coder_eval.plugins entry point)" (models/enums.py:102-105), and CLAUDE.md names coder_eval_uipath's Delegate agent as the worked out-of-tree example. Since agent_isolation defaults to true, every third-party SPI agent stops working under --driver docker with no code change on their side. The message also names neither the YAML path nor the escape hatch. Fix: make UID-drop launch support a property the agent declares (e.g. an Agent class attribute / registry capability flag) rather than a name list in the docker runner, so a conforming plugin agent opts in; until then, at minimum extend the message to ... set sandbox.docker.agent_isolation: false (not a security boundary) or register a launch seam — see docs/DOCKER_ISOLATION.md.
10. [Axis 7] protected_mocks client generator skips guards its record_cli twin enforces (mock_path_dirs collision scan, reserved tool names), silently shadowing a task's own mock (src/coder_eval/sandbox.py:576) — _generate_protected_mock_clients (sandbox.py:564-591) writes into the SAME PATH-prepended directory as record_cli (RECORD_CLI_DIR, appended first in resolved_mock_path_dirs, sandbox.py:461-465) but drops each hardening step its twin documents. (1) Stale log: it does log_path.touch(exist_ok=True) (line 576), whereas _generate_cli_recorders wipes — # Wipe rather than reuse: DIRECT_WRITE (the docker default) does not clear the target dir, so a reused --run-dir would leave a previous run's log to be scored as this run's (sandbox.py:518-520) followed by log_path.write_text("", encoding="utf-8") (line 530). A protected-mocks-only task re-run into the same --run-dir therefore scores the previous run's calls.jsonl entries under cli_called. (2) PATH collision: the only check is if wrapper.exists() inside the generated dir (line 580); the mock_path_dirs cross-check that _generate_cli_recorders performs (sandbox.py:492-514, 'Remove the record_cli entry to keep your own mock…') has no protected-mocks equivalent, so a protected wrapper silently shadows a task's own mocks/uip. (3) Reserved/valid names: ProtectedMockConfig.tool (models/sandbox.py:419) is a bare str with no pattern=, while RecordedCli.tool carries pattern=r"^[A-Za-z0-9._+-]+$" (models/sandbox.py:356) plus RECORD_CLI_RESERVED_TOOLS (models/sandbox.py:331-333, 394-401) whose comment records a real incident — a shim that re-resolved its own interpreter and 'spins to the task timeout'. tool: bash is accepted today and reproduces it: the wrapper execs /usr/local/bin/coder_eval_mock_client, whose shebang is #!/usr/bin/env bash (docker/coder_eval_mock_client:1), and env resolves bash through the PATH that now starts with cli_mocks/. Fix: truncate the log (write_text("")), reuse the mock_path_dirs collision check for protected tools, and give ProtectedMockConfig.tool the same pattern= + reserved-name validator as RecordedCli.
11. [Axis 8] agent_isolation defaults to true and fails closed, but zero in-repo task YAMLs were migrated, so every driver: docker task aborts at preflight (src/coder_eval/models/sandbox.py:198) — agent_isolation: bool = Field(default=True, ...) (models/sandbox.py:198-205) turns on a mode that hard-rejects, at DockerRunner.run() time: if criterion.type in {"agent_judge", "run_command", "uipath_eval"} → raise DockerRunError("docker.agent_isolation rejects privileged dynamic criteria...") (docker_runner.py:736-743); "docker.agent_isolation does not yet support docker.working_dir" (docker_runner.py:583); "docker.agent_isolation rejects extra_mounts" (docker_runner.py:588); and if capability != "uid-gid-v1": raise DockerRunError(f"image {image!r} does not declare org.coder-eval.agent-isolation=uid-gid-v1") (docker_runner.py:288-292). Every driver: docker task in this repo trips one of these: tasks/agents/claude_hello_world_docker.yaml:44, tasks/agents/antigravity_hello_world_docker.yaml:44, tasks/samples/skillsbench/dialogue-parser/dialogue-parser.yaml:65, 3d-scan-calc.yaml:64, court-form-filling.yaml:52 (all - type: run_command); tasks/dockerfile_build_example/working_dir_auto_example.yaml:35 (working_dir: auto) and working_dir_concrete_example.yaml:35 (working_dir: /app); tasks/byod_smoke_test.yaml:25 (image: byod-custom-image:0.1.0, which cannot carry the new label). dockerfile_build_example.yaml survives only if its base image is rebuilt. The PR changes 0 task YAMLs. Blast radius: the DockerRunError is converted by build_error_result (docker_runner.py:1571-1600) into EvaluationResult(final_status=FinalStatus.ERROR, iteration_count=0), so a task that scored 1.0 yesterday becomes a score-0 ERROR row today — loud, but it wipes the whole docker half of the nightly, and the external coder-eval-uipath / eval-runner pipeline that consumes task.json sees a suite-wide ERROR spike rather than a config change. It is also caught only inside DockerRunner.run(), i.e. after image build/pull — coder-eval plan and load_task report these tasks as valid, so there is no pre-flight way to find out. Fix: either default agent_isolation to false for this release (opt-in) with a deprecation window, or land the task-YAML migration in the same PR and add the compatibility check to plan/load time so it fails before a container starts. Also document the knob in docs/TASK_DEFINITION_GUIDE.md — it appears nowhere there, and ### run_command (line 845) still reads as universally available.
12. [Axis 8] SKILLS_REPO_PATH is skipped unconditionally in the env passthrough loop but re-added only under agent_isolation, dropping the var in the escape-hatch path (src/coder_eval/isolation/docker_runner.py:1381) — Line 1381 is if env_var in ("LITELLM_BASE_URL", "LITELLM_COST_LOG", "SKILLS_REPO_PATH"): continue — ungated by cfg.agent_isolation, so the var is removed from the name-only passthrough loop for every docker run, including the documented agent_isolation: false migration escape hatch. It is re-added only at 1388-1394, and only when self._host_to_private_paths.get(resolved_skills) is non-None: if cfg.agent_isolation and (skills_repo := os.environ.get("SKILLS_REPO_PATH")): ... else: logger.debug("Not forwarding unstaged SKILLS_REPO_PATH into protected container: %s", resolved_skills). _host_to_private_paths is keyed on the task dir, individual plugin dirs, template source dirs and reference dirs (docker_runner.py:790, 803, 830, 838) — never on the skills-repo root itself — so a task whose plugin path is $SKILLS_REPO_PATH/plugins/x registers /host/skills/plugins/x, not /host/skills, the lookup misses, and the var is dropped with a DEBUG-level log. In-container, expand_env_vars then leaves the literal $SKILLS_REPO_PATH, and both agents only warn: self._log.warning(f"Plugin skills path did not resolve: {path_str!r} → {expanded_path!r} ({hint}); no skills linked from it") (codex_agent.py:1006-1010, antigravity_agent.py:277-280). The agent runs with no skills linked, skill_triggered scores 0, and the failure is indistinguishable from the agent simply not using the skill. Fix: gate the continue on cfg.agent_isolation, and under isolation resolve SKILLS_REPO_PATH by longest-prefix match against _host_to_private_paths (or register the skills root as a private mount) instead of exact-key lookup; raise instead of logger.debug when the var is set but unmappable.
13. [Axis 8] Plugin bundle projection silently drops all plugin-root material outside 5 hardcoded subdirs under a default-on flag (src/coder_eval/plugin_bundle.py:22) — PLUGIN_AGENT_ALLOWED_SUBDIRS = frozenset({"skills", "commands", "agents", ".claude-plugin", "hooks"}) (line 22) is the entire projection: build_manifest iterates only for name in sorted(PLUGIN_AGENT_ALLOWED_SUBDIRS) (line 118) and everything else at the plugin root is dropped with no error and no log — the only message emitted is the success line logger.info("Prepared agent-visible plugin bundle %s -> %s (%d files, digest %s)", ...) (docker_runner.py:795-801). Real installed plugins carry surfaces outside that set: /Users/religa/.claude/plugins/cache/claude-code-lsps/pyright/1.0.0/ contains .lsp.json; /Users/religa/.claude/plugins/cache/uipath-claude-marketplace/security/1.6.2/ contains .codex-plugin; several contain top-level README.md/LICENSE. A plugin's MCP servers (.mcp.json), scripts/ referenced by hooks via ${CLAUDE_PLUGIN_ROOT}, and this repo's own reference/ convention all vanish under the default docker configuration, so the same plugin gives the agent different capabilities inside a protected run than outside — a score change that reads as an agent failure. Separately, HIDDEN_MATERIAL_FILE_PATTERNS: tuple[str, ...] = ("resolution.md", "check_*.py") (line 23) is matched against every file in the allowed subtrees and raises PluginBundleError(f"hidden grading material appears inside an agent-visible plugin subtree: {relative}") (lines 111-114), so a skill that legitimately ships skills/foo/check_env.py turns the whole task into a DockerRunError/ERROR. Fix: copy the full plugin tree minus an explicit exclusion list (or warn loudly, per excluded top-level entry, naming what was dropped) so the truncation is visible, and scope the check_*.py denylist to directories that are actually grader-adjacent rather than the whole projection.
Non-blocking, but please consider before merge
-
[Axis 1] Hardcoded
_NOISE_VALUE_FLAGS = {"--output"}strips the flag fromnormalized/subsetkeys with no per-fixture override; twosubsetrules differing only in--outputvalue silently collapse (second unreachable) (src/coder_eval/protected_mock/server.py:43) — A generic mock server hardcodes one CLI's flag name:_NOISE_VALUE_FLAGS = frozenset({"--output"})
and _expand_argv_tokens deletes it and its value from every comparison (server.py:53-56 for the --flag=value form, server.py:68-73 for the split form). Fixture authors get no way to see or override this list from the task schema. Concrete consequence: two subset rules that differ only in output format — ["rpa","get-errors","--output","json"] and ["rpa","get-errors","--output","table"] — both expand to the token set {rpa, get-errors}, so the second is unreachable and every invocation gets the first rule's stdout regardless of the format requested. This is the "hardcoded magic-string special-case where a generic mechanism fits" shape: either drop the concept entirely (see the match-mode finding — exact matching needs no noise list) or make it declarative, e.g. an optional per-fixture ignore_value_flags: ["--output"] so the behavior is visible where the fixture is authored.
2. [Axis 1] container_paths/protocol SSOT constants are unreferenced while docker_runner hardcodes the same container paths at multiple sites (src/coder_eval/models/container_paths.py:35) — AGENT_USERNAME = "agent" (container_paths.py:35), MOCKD_USERNAME = "mockd" (:39) and MOCK_RPC_GROUP = "uip-rpc" (:41) are added and re-exported from the public coder_eval.models surface (models/init.py:25/40/37 and __all__ entries at :287/:302/:299) with zero references anywhere in the repo; CONTAINER_FIXTURE_DIR = "/opt/coder-eval/mock/fixtures" (protected_mock/protocol.py:10) is likewise unreferenced. Meanwhile docker_runner.py — whose header comment for this module is "leaf constants; re-exported so consumers obey CE001" — bypasses the SSOT with six raw literals: line 704 "/opt/coder-eval/mock/fixtures/mock-config.json", line 803 f"/opt/coder-eval/grader/templates/source-{template_index}", line 810 "/opt/coder-eval/grader/references/directory", line 815 "/opt/coder-eval/grader/references/file-parent", line 831 f"/opt/coder-eval/mock/fixtures/{filename}", line 1474 :/opt/coder-eval/mock/fixtures:ro. Note CONTAINER_GRADER_DIR is not even in docker_runner's import list (lines 28-44), so the grader-root constant and the actual mounts can diverge silently. Fix: import and use CONTAINER_GRADER_DIR / CONTAINER_FIXTURE_DIR at those six sites, and either wire the three identity-name constants into the Dockerfile drift guard (tests/test_docker_identity_isolation.py currently asserts only the numeric ARG AGENT_UID=/ARG MOCKD_UID=/ARG MOCK_RPC_GID= lines, :32-36) or delete them rather than freezing them into the public models API. Lint-rule candidate: a CE rule forbidding string literals starting with /opt/coder-eval or /work outside models/container_paths.py.
3. [Axis 2] getattr-based platform-API access erases pyright checking, including an Any base class that disables all attribute checking on ProtectedMockServer (one of the six sites — socket.SO_PEERCRED — is genuinely required and must stay) (src/coder_eval/protected_mock/server.py:189) — 189: _UnixStreamServer: Any = getattr(socketserver, "UnixStreamServer", object) followed by 192: class ProtectedMockServer(socketserver.ThreadingMixIn, _UnixStreamServer): makes the whole isolation-boundary server class derive from Any, and forces the unjustified suppression on 199: super().__init__(path, ProtectedMockHandler) # pyright: ignore[reportCallIssue].
I verified both halves with pyright 1.1.408 under this project's config (typeCheckingMode = "standard", no pythonPlatform pin): with the getattr base, a body containing self.totally_bogus_attribute_xyz() reports 0 errors; with the plain socketserver.UnixStreamServer base it reports Cannot access attribute "totally_bogus_attribute_xyz" for class "S2*". The same probe showed reveal_type(socket.AF_UNIX) → Literal[AddressFamily.AF_UNIX] and reveal_type(os.chown) → the full typed signature, whereas getattr(os, "chown", None) → Any | None. So the getattr dance buys nothing under the configured type checker and costs all checking at these sites:
src/coder_eval/protected_mock/server.py:283peer_cred = getattr(socket, "SO_PEERCRED", None)src/coder_eval/protected_mock/server.py:327-328chown = getattr(os, "chown", None)/geteuid = getattr(os, "geteuid", None)src/coder_eval/protected_mock/client.py:66af_unix = getattr(socket, "AF_UNIX", None)src/coder_eval/isolation/agent_identity.py:48chown = getattr(os, "chown", None)(the subsequentchown(candidate, AGENT_UID, AGENT_GID, follow_symlinks=False)argument list is completely unchecked)src/coder_eval/isolation/agent_identity.py:104sigkill = getattr(signal, "SIGKILL", signal.SIGTERM)
Fix: reference these APIs directly (typeshed already guards them with sys.platform != "win32"), keeping the existing explicit require_isolation_runtime() / sys.platform != "linux" runtime guards as the portability story; drop the now-unnecessary # pyright: ignore[reportCallIssue].
4. [Axis 3] New tests leave 0o555/0o444 temp trees that silently abort pytest's tmp_path cleanup, stranding whole garbage- roots* (tests/test_plugin_bundle.py:58) — stage_bundle hardens its output (bundle_dir.chmod(0o555) / files 0o444, plugin_bundle.py:178-188) and the tests never undo it, so pytest's tmp_path retention sweep cannot remove the directories. Running the suite in the PR worktree emits dozens of PytestWarning: (rm_rf) error removing .../test_bundle_preserves_safe_rel0/bundle / OSError: [Errno 66] Directory not empty, and find /private/var/folders/.../pytest-of-religa -type d ! -perm -u+w | wc -l reports 374 undeletable leaked dirs across 17 accumulated garbage-* roots. Leak sites: tests/test_plugin_bundle.py:58 (manifest = stage_bundle(source, bundle)), :107, :120 (test_bundle_verification_detects_drift relaxes only staged.parent/staged at lines 126-127, not bundle itself), :165, and tests/test_protected_mock.py:621 (runner._prepare_isolated_sources(staging), which does mock_root.chmod(0o555)). Add an autouse fixture in both modules that walks tmp_path in a finally and restores 0o700/0o600 (the authors already recognised the problem inline at test_plugin_bundle.py:123-127 — generalise it).
5. [Axis 4] Protected-mock invocation evidence (cli_mocks/calls.jsonl, the log cli_called grades on) is written by the agent-side client into the agent-owned sandbox; the trusted mockd server (ProtectedMockServer.dispatch, server.py:201) persists no record (src/coder_eval/sandbox.py:586) — Sandbox._generate_protected_mock_clients writes the wrapper at sandbox.py:585-589:
wrapper.write_text(
"#!/bin/sh\n"
+ f"CODER_EVAL_MOCK_CALL_LOG={shlex.quote(str(log_path))} "
+ f'exec {shlex.quote(CLIENT_EXECUTABLE)} {shlex.quote(spec.tool)} "$@"\n',
with log_path = self.sandbox_dir / RECORD_CLI_LOG (sandbox.py:575), i.e. cli_mocks/calls.jsonl — the exact file the cli_called criterion parses (models/sandbox.py:324, models/criteria.py:570). The record is produced by protected_mock/client.py:_record (client.py:21-33), which runs as the agent and appends to a file inside the sandbox tree that Orchestrator._grant_current_sandbox_to_agent (orchestrator.py:1085) chowns to UID 2000. ProtectedMockServer.dispatch (server.py:198) sees every authoritative request but persists nothing. Failure scenario: a task pairs sandbox.protected_mocks with a cli_called criterion; the agent never calls the tool (or calls it wrongly), then appends hand-crafted {"ts":…,"tool":"uip","argv":[…],"exit":0} lines to cli_mocks/calls.jsonl (or deletes lines proving a forbidden call) and the criterion scores it as satisfied. Fix: have mockd append the authoritative JSONL record itself into a root-only path under /opt/coder-eval/grader/ and have cli_called read that file when the tool is a protected_mocks tool; the client-side _record should then be diagnostic only. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:N
6. [Axis 4] Agent UID-2000 processes are only reaped in the teardown finally (orchestrator.py:608), after check_all_async has already scored the live sandbox (orchestrator.py:1572) (src/coder_eval/orchestrator.py:608) — await self._stop_isolated_agent_processes() sits at orchestrator.py:608, inside the teardown finally block, with the comment "Protected Docker runs stop the SDK process and kill any same-UID descendants before post-run commands, capture, or task.json publication" — but await self.success_checker.check_all_async(...) already ran at orchestrator.py:1572, inside _evaluation_loop(), which is awaited at orchestrator.py:502 well before that finally. DOCKER_ISOLATION.md's claim ("Before trusted post-run/finalization begins, the harness stops the SDK, repeatedly kills every remaining UID-2000 process") therefore does not cover the scoring step. Failure scenario: the agent backgrounds a detached process during its turn (nohup sh -c 'sleep 20; while :; do mv a b; mv b a; done' &, which survives the SDK turn because only --init/terminate_agent_processes reaps it, and both run later); that process flips a file between two criteria checks so a task whose criteria are mutually exclusive (e.g. file_exists: out.json plus file_contains … should_not_contain) passes both, or writes the expected artifact after the turn "ended". Fix: call _stop_isolated_agent_processes() at the end of _evaluation_loop, immediately before check_all_async, so grading always reads a frozen trajectory and an emptied agent UID. CVSS:3.1/AV:L/AC:H/PR:L/UI:N/S:U/C:N/I:H/A:N
7. [Axis 5] Vendor-specific names and matching semantics baked into the vendor-neutral core (uip.sock, uip-rpc, and a global --output noise flag that also silently rewrites subset rules) (src/coder_eval/protected_mock/protocol.py:7) — The protected-mock mechanism is generic — one socket serves every configured tool, and the request envelope carries "tool" (server.py:270, dispatch(tool, argv)) — yet it is named after one vendor's CLI throughout the importable core that is going OSS-public: SOCKET_PATH = "/run/coder-eval/uip.sock" (protocol.py:7), MOCK_RPC_GROUP = "uip-rpc" in the models layer (models/container_paths.py:41), and the same string re-hardcoded in docker/coder_eval_drop_privilege.sh:14 (GROUP_ARGS=(--groups=uip-rpc)), docker/coder_eval_mockd.sh:12 and docker/Dockerfile:38-43. Worse than naming, one vendor's flag vocabulary is baked into the shared matcher semantics: _NOISE_VALUE_FLAGS = frozenset({"--output"}) (server.py:43) makes normalized matching drop --output <fmt> for every tool a user ever mocks, a per-CLI rule that belongs in the per-tool fixture (e.g. a noise_flags key on the fixture, defaulting to empty) rather than as a global constant. Rename to neutral identifiers (mockd.sock, coder-eval-rpc) and move the noise-flag list into the fixture schema; the UiPath framing can stay in the docs example.
8. [Axis 6] ProtectedMockHandler sets no per-connection read timeout and the threading server is unbounded, so a stalled peer pins a thread/fd and can exhaust --pids-limit 512 (src/coder_eval/protected_mock/server.py:262) — ProtectedMockHandler (server.py:257) does not set the timeout class attribute that socketserver.StreamRequestHandler.setup() would turn into self.connection.settimeout(...), so line = self.rfile.readline(MAX_REQUEST_BYTES + 1) at server.py:262 blocks indefinitely on a peer that connects and never sends a newline. ProtectedMockServer is a ThreadingMixIn with daemon_threads = True (server.py:194) and no cap on concurrent handlers, so each such connection permanently consumes one OS thread and one fd in the mockd process. Docker's pids cgroup counts threads as tasks, and this PR adds --pids-limit 512 as the isolation default (docker_runner.py:1365-1366, DEFAULT_AGENT_ISOLATION_MAX_PIDS = 512) — that limit is container-wide, so an agent looping for i in $(seq 600); do exec 3<>/run/coder-eval/uip.sock & done exhausts it and the root harness's own post-run commands and grading subprocesses then fail to fork. Note the per-run max_requests budget does not help: the budget is only consulted inside dispatch, which a never-completed request never reaches. Fix: set timeout = CLIENT_TIMEOUT_SECONDS on ProtectedMockHandler (StreamRequestHandler applies it to the connection in setup()) so a stalled peer is dropped, and bound concurrency (a semaphore around handle, or a small request_queue_size plus a non-threading server since requests are sub-millisecond).
9. [Axis 8] Protected-mock client drops invocation-log records without the .error sentinel that cli_called relies on, so a failed log write reads as "the agent never ran it" (the budget-exhaustion half of the finding is overstated — it is recorded) (src/coder_eval/protected_mock/server.py:207) — if state.remaining <= 0: return CommandResponse(75, "", "protected mock: request budget exhausted\n") (lines 206-207) silently converts every subsequent tool call into a failure once max_requests (default 100, ProtectedMockConfig.max_requests, models/sandbox.py:421) is spent. Nothing records that the budget was hit — no telemetry dimension, no criterion, no field on EvaluationResult — so a suite where one task exhausts its budget scores 0 and is indistinguishable from an agent that simply failed. Compounding it, the client's log write degrades more quietly than the record_cli shim it is schema-compatible with: except OSError as exc: sys.stderr.write(f"protected mock client: invocation log failed: {exc!r}\n") (protected_mock/client.py:31-32) drops the record, whereas the shim template explicitly writes a LOG_ERROR_PATH sentinel precisely because "a dropped record reads exactly like 'the agent never ran it'" (invocation_log.py:76-81) — and a dropped record makes any cli_called criterion under-count and fail. Note protected_mock/server.py is at 68.39 % coverage with the whole serve/handler tail (259-343) uncovered and client.py at 44.79 %. Fix: mirror the .error sentinel in client.py::_record, and surface budget exhaustion as a first-class signal (a warning in task.log plus a counter on the result) so the reports can distinguish it from an agent failure.
Nits
-
[Axis 1] load_config hand-validates a harness-generated file that ProtectedMockConfig already owns (
src/coder_eval/protected_mock/server.py:161) —load_configre-implements, in ~25 lines of isinstance/range/uniqueness checks (radon C(20)), validation thatProtectedMockConfig(models/sandbox.py:409-455) already performs withextra="forbid"plusvalidate_tool_name/validate_passthrough_prefixes:if not isinstance(tool, str) or not tool or tool in loaded:
raise ValueError("mock config tools must have unique non-empty names")
if not isinstance(fixture, str) or not isinstance(max_requests, int) or max_requests < 1:
Unlike the fixture file (task-authored, so _load_tool's defensive parsing is warranted), mock-config.json is written by the trusted root harness in the same container from those already-validated models (docker_runner.py:842-844, json.dumps({"version": 1, "tools": tools})). The duplication is the drift risk the repo's "Single Source of Truth: Schema models are the authoritative source" principle exists to prevent: add a field to ProtectedMockConfig, docker_runner serializes it, and load_config silently ignores it with no failing check. Parse the config with the Pydantic model (ProtectedMockConfig.model_validate) and keep the hand-rolled validation only for the untrusted fixture payload.
2. [Axis 2] ProtectedMockConfig.fixture carries no validator while its sibling tool does, so empty/whitespace paths pass model validation and fail later with a misleading message (src/coder_eval/models/sandbox.py:420) — 420: fixture: str = Field(description="Path to the protected exact-command response fixture") has no constraint, whereas tool (line 419) gets a strict validate_tool_name. I confirmed with the built model that ProtectedMockConfig(tool="uip", fixture="") and fixture=" " both validate successfully. An empty value then reaches task_loader.resolve_protected_mock_paths, where Path(os.path.expandvars("")) becomes ., resolves to the task directory, and raises Protected mock fixture not found: <task dir> — pointing at a directory the author never wrote. Add min_length=1 plus a validator rejecting blank/whitespace-only values (mirroring validate_tool_name) so the error names the real problem at load. Separately, tool accepts interior whitespace (ProtectedMockConfig(tool="my tool", fixture="f.json") validates) even though the value must be an invocable bare command name and becomes a wrapper filename in sandbox.py:583; consider rejecting whitespace outright rather than only leading/trailing.
3. [Axis 3] Privilege-launcher drift guards assert substring presence, not behavior (tests/test_docker_identity_isolation.py:43) — test_privilege_launchers_clear_capabilities_and_set_no_new_privs only checks that strings appear anywhere in the script text (assert "--clear-groups" in script at line 50, assert "CODER_EVAL_AGENT_ALLOW_RPC" in script at line 51). It cannot detect an inversion of docker/coder_eval_drop_privilege.sh:13-15 — flipping if [[ "${CODER_EVAL_AGENT_ALLOW_RPC:-}" == "1" ]] so every agent gets --groups=uip-rpc by default leaves both substrings present and the test green. Since the docker live tests SKIP in this environment, this is the only guard on that logic. Make it behavioral: put a stub setpriv that echoes its argv on PATH and run the script under bash with and without CODER_EVAL_AGENT_ALLOW_RPC=1, asserting the exact rendered argv in each case.
4. [Axis 6] Antigravity drop-shim tempdir leaks when start() fails before _teardown, and each retried start() overwrites _drop_shim_dir without removing the previous one (src/coder_eval/agents/antigravity_agent.py:348) — start() calls self._env_path_prepend.insert(0, str(self._stage_localharness_drop_shim())) at antigravity_agent.py:348, and _stage_localharness_drop_shim (line 238) does shim_dir = Path(tempfile.mkdtemp(prefix="antigravity-drop-")) then self._drop_shim_dir = shim_dir with no removal of any prior value. The mkdtemp happens BEFORE the SDK import block, whose except ImportError as e: at line 355 raises RuntimeError("Antigravity SDK not installed...") without calling await self._teardown() (only the later except Exception at line 406 does), so that dir is orphaned. Because agent.start() is driven through execute_with_retry (orchestrator.py:1111), each retry mints a fresh mkdtemp and overwrites self._drop_shim_dir, so only the last one is ever removed by _teardown() (line 626-629). Fix: rmtree any existing self._drop_shim_dir at the top of _stage_localharness_drop_shim, and move the shim staging inside the try: whose except Exception already calls _teardown().
5. [Axis 8] protected_mocks + agent_isolation: false is only rejected inside DockerRunner.run(), not by the model validator that already checks the driver (src/coder_eval/isolation/docker_runner.py:593) — raise DockerRunError("sandbox.protected_mocks requires docker.agent_isolation: true") sits at docker_runner.py:593, inside run() and after image resolution/build. The sibling constraint is already enforced at load time by the SandboxConfig model validator — if self.driver != "docker": raise ValueError("sandbox.protected_mocks requires driver: docker") (models/sandbox.py:558-560) — so the two halves of the same rule are enforced at different times: a task with protected_mocks plus agent_isolation: false loads cleanly, passes coder-eval plan, pulls/builds an image, and only then errors. Fix: move the agent_isolation half into the same validate_template_sources model validator (it can read self.docker.agent_isolation) so both fail at task load with one consistent message.
What's Missing
Parallel paths:
- 🟠
docker/Dockerfilegained the agent/mockd identities, theuip-rpcgroup, the setpriv launchers andLABEL org.coder-eval.agent-isolation="uid-gid-v1"(lines 38-59, 133), but the siblingdocker/Dockerfile.runtime(the runtime-kit / inject-mode image built by the samemake docker-images) got none of them and still only carriesorg.coder-eval.version. The two framework images now have divergent capability, and every inject-mode task is dead under the default; the runtime Dockerfile's own header comment ("org.coder-eval.version on the injected image -- that's the label the host asserts") is stale now that the host asserts a second label. (trigger: docker/Dockerfile) - 🟠 The four new fail-closed gates (agent-type allowlist, privileged-criteria rejection,
working_dir/extra_mountsrejection, image-label preflight) all live insideDockerRunner.run(); the pre-flight surfaces were not taught any of them.cli/plan_command.pycontains zeroagent_isolationreferences andorchestration/task_loader.pyonly gained fixture-path resolution, socoder-eval planstill reports every now-incompatible task as valid and the failure only appears after a container starts. (trigger: src/coder_eval/isolation/docker_runner.py) (restates: Axis 8: agent_isolation defaults to true and fails closed with no task migration) - 🟡 Three agents each grew their own ad-hoc UID-drop seam with no shared contract: Claude passes
cli_path=CONTAINER_CLAUDE_SHIM(claude_code_agent.py:1210), Codex prepends_drop_privilege_launch_args()to its argv, and Antigravity stages a temp wrapper dir and mutatesos.environacross the spawn. Nothing on theAgentABC or the registry expresses "this agent supports the UID drop", which is both whydocker_runner.py:716needs a hardcoded name list and why the third implementation (Antigravity) is the one that silently broke. (trigger: src/coder_eval/agents/antigravity_agent.py) (restates: Axis 5: Antigravity pops CODER_EVAL_AGENT_ISOLATION before the isolation check) - 🟡
protected_mock/server.pyadds a second, independent argv normalizer (_expand_argv_tokens+_NOISE_VALUE_FLAGS, lines 43-81) for the same invocation stream thatcriteria/cli_called.pyalready normalizes with its own splitter (ignore_flags/value_flags, cli_called.py:21-90). The mock decides what an invocation "is" for serving, andcli_calleddecides it again for grading, with no shared code and no shared config — so a fixture that matches can still fail the criterion, and vice versa. (trigger: src/coder_eval/protected_mock/server.py) _(restates: Axis 1: Hardcoded NOISE_VALUE_FLAGS strips --output with no per-fixture override) - 🟡 CE030's
DOCUMENTED_MODELS(tests/lint/doc_schema_parity.py:44-48) still tracks onlyTaskDefinition/RunLimits/Dataset/SimulationConfigand explicitly excludesSandboxConfig, so the PR could adddocker.agent_isolation(a default-on behavior flip) and the wholeProtectedMockConfigblock withoutmake lintnoticing thatagent_isolationnever appears indocs/TASK_DEFINITION_GUIDE.md. The parity list needed to grow with the surface it now guards. (trigger: src/coder_eval/models/sandbox.py) - 🟡 Only
DOCKER_ISOLATION.mdand theprotected_mockssection of the task guide were updated.docs/tutorials/06-use-docker-isolation.md— the page that teachesdriver: docker— still tells users to point at anyimage: my-custom:tag, to use runtime-kit inject-mode for foreign bases, and never mentions the label requirement or thatrun_command/working_dir/extra_mountsare now rejected;### run_command(TASK_DEFINITION_GUIDE.md:845) still reads as universally available. (trigger: docs/TASK_DEFINITION_GUIDE.md) (restates: Axis 8: agent_isolation defaults to true and fails closed with no task migration)
Tests:
- 🟠
tests/test_docker_runner_mounts.py(678 lines, the argv/mount suite) has zeroagent_isolationreferences after this PR, yet the isolation behavior is threaded through_build_argvas invertedif not cfg.agent_isolation:guards on the pre-existing plugin /template_sources/reference/system_prompt_fileauto-mounts (docker_runner.py:1516, 1523, 1533, 1541). No test asserts those raw host paths are absent from the rendered argv under isolation — nor that they are still present with isolation off — so dropping anotsilently re-exposes a host source tree at an agent-readable path. (trigger: tests/test_docker_runner_mounts.py) _(restates: Axis 1: docker_runner complexity blowout, build_argv at CC F(52) under an unratcheted noqa) - 🟠 Of the three agents wired for UID drop, only Codex gets an isolation test (
test_isolated_codex_profiles_never_restore_root_harness_home). No test anywhere referencesCONTAINER_CLAUDE_SHIM(the Claude launch seam at claude_code_agent.py:1210) or Antigravity's_stage_localharness_drop_shim/HOME=/home/agentspawn-window override — which is precisely why the AntigravityHOMEdefect shipped green. (trigger: tests/test_docker_identity_isolation.py) (restates: Axis 5: Antigravity pops CODER_EVAL_AGENT_ISOLATION before the isolation check) - 🟡 The workspace root moved from
/workto/work/agentunder isolation (docker_runner.py:591), butTestWorkspaceDirwas only updated for the grader-dir constant (CONTAINER_OUTPUT_DIR == "/opt/coder-eval/grader/output"); no test asserts_workspace_dir == CONTAINER_AGENT_WORK_DIRor that the rendered--workdir/ capture path follows it.CONTAINER_AGENT_WORK_DIRappears nowhere undertests/. (trigger: tests/test_docker_runner_mounts.py) - 🟠 No test ever constructs
ProtectedMockServeror runsProtectedMockHandler; every dispatch test passes a bareMagicMock()asself, so the socket handler,_peer_uidgate,_writetruncation,serve()chown/chmod, and the entire successfulclient.invoke()path are uncovered (server.py 68%, client.py 45%). One in-process bind-and-round-trip test would cover the wire contract that currently has no coverage on any surface. (trigger: tests/test_protected_mock.py) (restates: Axis 3: New PR surfaces ship effectively untested (protected mock, agent_identity, fail-closed gates)) - 🟡 No test asserts that a run's per-task staging tempdir is actually gone after
DockerRunner.run()finishes when a plugin bundle or protected-mock fixture was staged into it — the case wherestage_bundle's 0o555/0o444 hardening defeatsshutil.rmtree(..., ignore_errors=True). The same missing assertion is why the test suite itself now strands undeletablegarbage-*roots under pytest's basetemp. (trigger: tests/test_plugin_bundle.py) (restates: Axis 6: Per-task Docker staging tempdir leaks when a plugin bundle or mock fixtures are staged)
Downstream consumers:
- 🟠 Nothing records that a run was protected:
EvaluationResulthas no isolation provenance field, and neithertask.jsonnorrun.jsoncarriesagent_isolation, the protected-mock tools in effect, or the plugin-bundle digest. Since the flag defaults to true and materially changes what the agent can see (bundle-projected skills only,/work/agent, mocked CLIs), every downstream consumer — reports, the evalboard, the external eval-runner pipeline, and any before/after score comparison — will read a config-driven score change as an agent regression with no way to tell them apart. (trigger: src/coder_eval/orchestrator.py) - 🟡
verify_bundle(plugin_bundle.py:208) andmanifest_path_for(:142) are exercised only bytests/test_plugin_bundle.py— no production caller exists. The manifest digest is computed host-side and merely logged (docker_runner.py:786-792); nothing re-verifies the mounted bundle at container start, which is the moment the drift check the API was written for would actually mean something. (trigger: src/coder_eval/plugin_bundle.py) - 🟡
cli_mocks/calls.jsonlnow has a second writer (protected_mock/client.py::_record) feeding the samecli_calledcriterion as therecord_clishim, but the criterion's contract wasn't extended to it: the mock client never writes the<log>.errorsentinel thatcriteria/cli_called.py:231-243looks for, so a failed append degrades into a silent under-count instead of the hard "verdict cannot be trusted" error. (trigger: src/coder_eval/sandbox.py) (restates: Axis 8: Protected-mock client drops invocation-log records without the .error sentinel)
Display & mapping dicts:
- 🟡 The "Troubleshooting custom images" table (DOCKER_ISOLATION.md:114-119) was not extended for the new hard failure
image <x> does not declare org.coder-eval.agent-isolation=uid-gid-v1— the most likely first error any existing user hits. Worse, its existinghas no org.coder-eval.version labelrow still advises "or use the runtime kit", a remedy protected mode now rejects outright. (trigger: docs/DOCKER_ISOLATION.md) - 🔵 The three new agent-visible failure codes — 75 (request budget exhausted), 77 (caller identity rejected), 125 (client could not reach mockd) — are mapped nowhere outside the mock itself: no telemetry dimension, no
FinalStatus, no report row, no note in the run summary. They surface only as ordinary non-zero tool exits in the transcript, so a harness-side mock failure is indistinguishable from the agent misusing the CLI. (trigger: src/coder_eval/protected_mock/server.py) (restates: Axis 8: Protected-mock client drops invocation-log records without the .error sentinel)
Daily/nightly:
- 🟠 The PR says nothing about the nightly, and CI will not catch it: the only
driver: dockertask in any smoke bucket istasks/byod_smoke_test.yaml(taggedsmoke-pass, criteria arefile_existsonly, image freshly built from the labeled base in the same job), sopr-checks.ymlstays green while all 5run_commanddocker tasks and bothworking_direxamples abort at preflight. The breakage lands first in the nightly / users' runs, as a suite-wide ERROR spike rather than a config change. (trigger: src/coder_eval/models/sandbox.py) (restates: Axis 8: agent_isolation defaults to true and fails closed with no task migration) - 🟠 No image-republish or version story:
pyproject.tomlis untouched, so anycoder-eval-agent:<same-version>image already cached locally or published bydocker-publish.ymlpredatesLABEL org.coder-eval.agent-isolationand now fails the preflight hard — while the pre-existing_preflight_image_versiononly warns on a stale image, so "stale local image" is the realistic default state. The PR should state that every runner (nightly host, CI, downstream users) must rebuild/repull before it merges. (trigger: docker/Dockerfile) - 🟡 Protected runs now get an implicit
--pids-limit 512wheneverlimits.max_pidsis unset (docker_runner.py:1363-1366) — previously there was no limit at all. That is a container-wide resource-envelope change for every nightly docker task (builds,pytest -n auto, npm installs share the cap with the harness), and it is documented only in DOCKER_ISOLATION.md; themax_pidsline in TASK_DEFINITION_GUIDE.md:509 still implies the cap only exists when you set it. (trigger: src/coder_eval/isolation/docker_runner.py)
Harness & Lint Improvements
Static checks (lint / type):
- [ruff] Add
C901to[tool.ruff.lint] selectwith[tool.ruff.lint.mccabe] max-complexity = 30. MEASURED: at 30 the current tree has zero violations (worst isDockerRunner._build_argvat C901 29), while PR HEAD's_build_argvis C901 41 — so this ratchet fails the PR with no migration cost. Crucially C901 is a distinct code from PLR0912/PLR0915, so the PR's new# noqa: PLR0912, PLR0915marker does NOT suppress it: regrowth would require a second, separately-visible# noqa: C901. (Setting it to 20 — the axis-1 anchor — costs exactly 4 pre-existing fixes:claude_code_agent.communicate21,orchestrator._simulation_dialog_loop22,reports_experiment.generate_variant_report24,_build_argv29; a 30-now/20-later ratchet is the cheap path.) Verify:.venv/bin/ruff check --select C901 --config "lint.mccabe.max-complexity = 30" src/coder_eval/. Prevents: A1/A5 high —_build_argvgrowing radon E(34)→F(52) / mccabe 29→41 under an unratcheted noqa, and_prepare_isolated_sourceslanding at D(24). - [ce-lint] New CE035 — PLR debt markers must be ratcheted (
tests/lint/rules/ce035_plr_noqa_ratchet.py, wired intests/lint/runner.py). Generalize the CE022 precedent (which pins exactly one function's statement count) into a registry: any# noqa: PLR0912/PLR0915insrc/must have an entry in a{module}::{qualname} -> (max_statements, max_branches)table pinned at the value measured when the marker was added; the rule re-counts withastand fails on regrowth, and fails on a marker with no registry entry.tests/test_custom_lint.py:415already records the intent ("a different oversized function is ruff's job, not CE022's") — this closes the gap that lets a NEW marker ship unbounded. Prevents: A1/A5 high — the new_build_argvmarker ships with no ratchet, unlike its only in-repo precedent (_simulation_dialog_loop, capped by CE022), so 102 statements / 38 branches can now grow without signal. - [pyright] In
[tool.pyright]: (1) addpythonPlatform = "Linux"— MEASURED with pyright 1.1.408: 0 new errors on the current tree, and it makessocketserver.UnixStreamServer,os.chown,os.geteuid,socket.AF_UNIX,signal.SIGKILLANDsocket.SO_PEERCREDall resolve directly on macOS dev machines, removing the entire justification for the sixgetattr(module, "NAME", default)sites; (2) addreportUnnecessaryTypeIgnoreComment = "error"so the stale# pyright: ignore[reportCallIssue]is flagged the moment theAnybase is removed (MEASURED cost: 13 pre-existing stale suppressions inagents/,models/results.py,models/telemetry.py). Do NOT proposereportUntypedBaseClass = "error"for this — I tested it and pyright does not flag an explicitlyAny-annotated base, so it would not have caught_UnixStreamServer: Any. Prevents: A2 medium —getattr-based platform-API access erasing pyright at 6 sites, plus the unjustified# pyright: ignore[reportCallIssue]. - [ce-lint] New CE036 — no type-erasing class bases or stdlib
getattrprobes. Two AST patterns insrc/: (a) a class whose base is a module-level name annotatedAny(_UnixStreamServer: Any = getattr(...)→class ProtectedMockServer(..., _UnixStreamServer)), which silently disables ALL attribute checking on the subclass — confirmed: pyright reports 0 errors forself.totally_bogus_attribute_xyz()under that base and an error under the direct base, and no pyright setting catches it; (b)getattr(<imported module>, "<str literal>", <default>)— platform-API probing that yieldsAny | Nonewhere the direct reference is fully typed. Ruff's B009 does not fire because these calls pass a default. Prevents: A2 medium — theAnybase class on the isolation-boundary server plus the five othergetattrerasure sites inprotected_mock/andisolation/agent_identity.py. - [ce-lint] New CE037 — container-path literals must come from the SSOT. Forbid any string literal (including f-strings) beginning with
/opt/coder-evalor/workanywhere insrc/exceptmodels/container_paths.pyandprotected_mock/protocol.py. MEASURED: zero violations on the current tree (grep -rn '"/work' src/coder_eval/ | grep -v container_paths.py→ 0), so it is free today and would have fired on all six newdocker_runner.pyliterals (704, 803, 810, 815, 831, 1474). Concrete risk it closes: host-side mounts spell the grader root as a literal while the container side derives it fromCONTAINER_GRADER_DIR(cli/run_task_internal_command.py:96). Prevents: A1/A2/A5/A7 medium —CONTAINER_GRADER_DIR/CONTAINER_FIXTURE_DIRbypassed by six raw literals;CONTAINER_GRADER_DIRnot even imported indocker_runner. - [ce-lint] New CE038 — no dead public exports: every name in
coder_eval/models/__init__.py::__all__must be referenced at least once outsidesrc/coder_eval/models/(source, tests, or docs). MEASURED cost on the current tree: 8 names need an allowlist entry with a stated reason (ResolvedAgentConfig,PermissionMode,JudgeTransport,BaseTemplateSource,validate_template_sources_list,MERGE_STRATEGY_KEY,APPEND_ORDER_KEY,CriteriaCheckTiming) — a one-time ~8-line allowlist matching the CE030 EXEMPT-with-reason convention. Ruff cannot do this: F401 is silenced precisely by__all__membership. Prevents: A1/A5 medium/low —AGENT_USERNAME,MOCKD_USERNAME,MOCK_RPC_GROUPfrozen into the public models API with zero references anywhere, whiletests/test_docker_identity_isolation.pyre-hardcodes the same string values. - [ce-lint] New CE039 — no closed agent-type allowlists outside the registry (direct sibling of CE018, which already bans denylist membership tests against
FinalStatusmember names). Forbidin/not inmembership tests against a set/tuple/list literal built from ≥2AgentKind.*.value(or equivalent agent-type string literals) outside the agent registry module. Capability must be declared by the agent (a class attribute / registry capability flag) so a plugin agent registered via thecoder_eval.pluginsSPI can opt in — which is whatAgentKind's own docstring (models/enums.py:99-105) promises. Pair with a rule-doc note that the raise message must name the escape hatch, mirroring the adjacent criteria rejection atdocker_runner.py:741-744. Prevents: A5/A7 high —_validate_agent_isolation_compatibility's hardcodedsupported_agentsset (docker_runner.py:716-721) hard-failing every third-party SPI agent under the now-default-onagent_isolation. - [ce-lint] New CE040 — in-repo task corpus must satisfy driver preflight (a whole-tree derived check wired as a
@pytest.mark.lintclass like CE026–CE031/CE033, not aBaseRule). Load everytasks/**/*.yamland, for eachdriver: dockertask, assert it satisfies the same predicatesDockerRunner._validate_agent_isolation_compatibilityenforces at runtime — norun_command/uipath_eval/agent_judgecriteria, nodocker.working_dir, nodocker.extra_mounts— unless the task setsagent_isolation: falseexplicitly. There is currently NO test anywhere that loads the in-repo task corpus, so a default flip can (and did) break 9/9 docker tasks withmake verifygreen. Prevents: A8 critical —agent_isolationdefaulting to true and failing closed with zero task-YAML migration; everydriver: dockertask aborts at container preflight whilecoder-eval planreports them valid. - [ce-lint] New CE041 — task-authored files must be parsed through a pydantic model. In the modules that read task-declared paths (
protected_mock/,criteria/,orchestration/task_loader.py), forbid.get(walking of ajson.load/json.loadsresult; require the payload to go through<Model>.model_validate(...)where the model setsextra="forbid". This extends theyaml_models_forbid_extraspremise from YAML-backed models to on-disk JSON fixtures. Feasible in-container:protected_mock/server.pyalready importscoder_eval.models, so the model can be shared rather than duplicated. Prevents: A2 high / A7 medium — fixture JSON hand-parsed withraw.get(...)defaults, so"stdou"/"match_modes"typos silently yield an empty successful response and drop the subset rule, contradicting the guide's documented "fail loudly" guarantee. Also coversload_config's hand-rolled re-validation of a model the harness already owns (A1 low). - [ce-lint] New CE042 —
shutil.rmtree(..., ignore_errors=True)must carry a failure signal: require either anonexc=/onerror=handler or a post-condition existence check +logger.warning(in-repo precedent:sandbox.py:932-935, "MST-9795 remediation: %s still present after rmtree"). MEASURED: ~8 existing sites insrc/, one already compliant; the rest take a# noqa: CE042with a one-line reason (the codebase already has a# noqa: CE002precedent atsimulation/user_simulator.py:245). Rationale:ignore_errors=Truecannot fail — an unlinkable 0o555 subtree is swallowed with no log line at all. Prevents: A6 critical —docker_runner.py:666silently failing to remove the 0o555/0o444 plugin-bundle and mock-fixture subtrees from every per-taskmkdtempstaging dir, leaking/tmp/coder_eval_docker_*roots unboundedly across runs. - [ce-lint] New CE043 — isolation-boundary spawns must pass explicit
env=: anysubprocess.run/Popen/create_subprocess_*insidesrc/coder_eval/protected_mock/must pass anenv=kwarg. Scope it narrowly — a blanket rule would hit 21 spawn sites repo-wide with only 6 currently passingenv=; insideprotected_mock/it is 2 sites, both defective. Same mechanical shape as the existing CE015 (create_subprocess_*must passlimit=). Prevents: A4 high — the passthrough proxy spawning the real tool with mockd's fully inherited environment (AWS_BEARER_TOKEN_BEDROCK,TASK_DIR,SKILLS_REPO_PATH,CODER_EVAL_*) and returning its stdout verbatim to the agent, from a UID that can read the 0500 fixture dir. - [ce-lint] New CE044 — protected-mock timeout constants live in
protocol.pyand are derived, not independently literal: forbid bare numeric*_TIMEOUT_SECONDSassignments inprotected_mock/outsideprotocol.py, and require the client deadline to be expressed asPASSTHROUGH_TIMEOUT_SECONDS + SLACKrather than an unrelated literal. Co-locating both constants makes the ordering inversion visible at the point of edit (the ordering assertion itself belongs in a test — see the harness bucket). Prevents: A1/A6 high —CLIENT_TIMEOUT_SECONDS = 5.0(protocol.py:13) bounding aPASSTHROUGH_TIMEOUT_SECONDS = 60(server.py:42) server budget, so the documented[docsai, ask]passthrough surfaces to the agent as a fabricated exit 125 after the request budget was already decremented. - [ce-lint] New CE045 — identity/path constants must match their Docker assets (whole-tree asset-parity check, same family as CE026/CE028/CE033). Assert that
AGENT_UID/AGENT_GID/MOCKD_UID/MOCKD_GID/MOCK_RPC_GID/AGENT_USERNAME/MOCKD_USERNAME/MOCK_RPC_GROUP/SOCKET_PATHinmodels/container_paths.py+protected_mock/protocol.pyare the values actually spelled indocker/Dockerfile,docker/coder_eval_drop_privilege.shanddocker/coder_eval_mockd.sh— parsed from the assets, not re-typed as literals in the test (which is whattests/test_docker_identity_isolation.py:37,53,58-60does today). Prevents: A1/A5 medium —uip-rpc/agent/mockdhardcoded in 4+ shell/Dockerfile sites while the Python constants holding the same values are unreferenced, so the two can drift silently. - [bandit-codeql] Add a CodeQL (or custom bandit) taint query: shell execution whose
cwdis agent-writable. Flagasyncio.create_subprocess_shell/subprocess.run(shell=True)where thecwdargument flows from a path that was passed togrant_agent_workspace/ chowned toAGENT_UID. Secondarily, gate# noseccomments so a justification naming only the command source (orchestrator.py:2184: "commands come from task YAML, not user input") is flagged when the same call'scwdis tainted — the trusted-input claim covers the wrong argument. Prevents: A4 high (CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:H) — root-executedpost_runshell commands (pytest -q,make check,./verify.sh) running in a directory the UID-2000 agent owns, letting a droppedconftest.py/Makefileread root-only/opt/coder-eval/grader/**and rewritetask.json, while the equivalentrun_commandcriterion is rejected.
Harness improvements (not statically reachable):
- Add an in-process protected-mock round-trip test: bind a real
ProtectedMockServeron a tmp socket in a thread, monkeypatchclient.SOCKET_PATH(an existing test already does this attests/test_protected_mock.py:461) and patch_peer_uid(not justAGENT_UID—socket.SO_PEERCREDdoes not exist on macOS), then assert exit code + stdout for a matched argv, a budget-exhausted call, an oversized request, a malformed envelope, and a passthrough slower thanCLIENT_TIMEOUT_SECONDS. Also give_fake_serveraMagicMock(spec=ProtectedMockServer)so a future attribute read onselffails instead of silently returning a Mock. Why not static: A client/server envelope or deadline mismatch is a runtime protocol property — no AST rule can prove the two sides agree. Today 8 of 22 tests callProtectedMockServer.dispatch(fake_server, ...)unbound, so the server is never constructed andProtectedMockHandlernever runs; measured coverage is server.py 68.39% (259-343 entirely dark) and client.py 44.79% (no successfulinvoke()at all), and there is no live docker test for the mock either — a mismatch surfaces only in production. Prevents: A3 high (untested wire path), A6 high (5s/60s deadline inversion), A6 medium (no per-connection read timeout / unbounded threading). - Add a session-scoped autouse conftest guard for permission-hardened temp trees: walk
tmp_pathin afinallyrestoring 0o700/0o600, and assert at session end that the pytest basetemp contains no non-writable directories. The authors already hit this and patched it locally attests/test_plugin_bundle.py:123-127— generalize it. Why not static: The leak is runtime filesystem state produced by the interaction of a production chmod (plugin_bundle.py:178-188,docker_runner.py:830-844) with pytest's cleanup; nothing in the test source is syntactically wrong. It is also silent — thePermissionErrorfromos.rmdirescapes pytest'son_rm_rf_errorhandler and is swallowed bymaybe_delete_a_numbered_dir's bareexcept OSError: return, so no warning is emitted. Current real basetemp state: 27 abandonedgarbage-*roots / 537 non-writable dirs, including unrelated tests' dirs in the same worker. Prevents: A3 medium (0o555/0o444 test trees stranding wholegarbage-*roots); it is the test-side twin of the A6 critical staging-dir leak. - Make
coder-eval planrun the driver preflight (image-label check, isolation criterion/mount/working_dir compatibility) and add a CI job that plans the whole in-repo task corpus. Pair it with a release-note requirement for any default flip on a fail-closed flag: the PR must state what happens to the nightly docker half. Why not static: CE040 can prove the in-repo corpus is compatible, but the image-label preflight (org.coder-eval.agent-isolation=uid-gid-v1) needs a real Docker daemon and a built/pulled image, and external suites (coder-eval-uipath/ eval-runner) have task corpora this repo cannot see — the gate must be reachable as a command they can run, not just a repo-local lint. Prevents: A8 critical — every check lives insideDockerRunner.run(), so tasks load clean, passplan, build/pull an image, then becomeFinalStatus.ERRORscore-0 rows; the external pipeline sees a suite-wide ERROR spike rather than a config change. - Add a diff-coverage gate to CI (e.g.
diff-coveragainst the merge base, or a per-new-module--cov-fail-under) alongside the existing global 80% threshold. Why not static: Coverage is by definition a runtime measurement. The global gate is the loophole: this PR addedprotected_mock/server.pyat 68.39%,client.pyat 44.79% andruntime.pyat 76.47% — every fail-closed gate, the UID-drop path, and the whole serve/handler tail — whilemake verifystayed green because the repo-wide average absorbed it. Prevents: A3 high — new fail-closed/runtime code (protected-mock wire path,agent_identityUID drop, docker_runner isolation gates,plugin_bundlefail-closed arms,validate_tool_name,resolve_protected_mock_paths) landing with zero or near-zero coverage. - Convert the privilege-launcher drift guards from substring assertions to behavioral ones: put a stub
setprivthat echoes its argv on PATH, rundocker/coder_eval_drop_privilege.shunder bash with and withoutCODER_EVAL_AGENT_ALLOW_RPC=1, and assert the exact rendered argv in each case. Same treatment forcoder_eval_mockd.sh. Why not static: The defect class is a semantic inversion of a shell conditional (if [[ "${CODER_EVAL_AGENT_ALLOW_RPC:-}" == "1" ]]flipped so every agent joinsuip-rpcby default) — every substring the current test greps for is still present after the inversion, and no parser can decide which branch is intended. It needs execution. Prevents: A3 low (substring-only launcher guards) and, by extension, the silent-capability-grant shape behind the A4 findings, since the docker live tests SKIP in this environment. - Grade against a frozen workspace: reap UID-2000 processes (or snapshot the sandbox) immediately before the FINAL
check_all_async, not in the teardownfinally, and add a determinism test that runs the criteria set twice against a workspace with a background writer flipping a file, asserting identicalscore/final_status. The reap must skip the simulation path's per-turn_run_dialog_criteria_check(orchestrator.py:1669), which would otherwise kill the agent mid-dialog. Why not static: This is an ordering property between two runtime phases plus live child processes;_stop_isolated_agent_processes()at line 608 andcheck_all_asyncat line 1572 are both well-formed calls in isolation. Deciding that one must precede the other needs the call graph AND the concurrency model. It is also the axis-8 blocker class: same agent output, different score. Prevents: A4 medium — detached UID-2000 processes mutating the workspace between sequentially-awaited criteria (a real TOCTOU:SuccessChecker.check_all_asyncfully awaits each criterion in order, and anllm_judgereads whatever the background writer last wrote). - Move invocation evidence to the trusted side and make it self-diagnosing: have mockd append the authoritative JSONL record into a root-only path under
/opt/coder-eval/grader/, havecli_calledprefer that file forprotected_mockstools, and mirror the.errorsentinel inprotected_mock/client.py::_record(the sentinel has a real consumer atcriteria/cli_called.py:231-243, which fails the criterion loudly rather than under-counting). Why not static: Whether a given file is agent-writable at grade time is a runtime property of the chown sweep (grant_agent_workspacewalks and chowns the whole sandbox tree), not a syntactic one — a lint rule cannot tellsandbox_dir / "cli_mocks/calls.jsonl"from a trusted path. The.error-sentinel half COULD be a narrow CE rule ("every writer toRECORD_CLI_LOGmust writeLOG_ERROR_PATHonOSError") and is worth adding as one; the trust-boundary move cannot. Prevents: A4 medium (agent-forgeablecli_calledevidence — the agent can append or delete lines in its own chowned tree and the criterion grades the forgery) and A8 medium (a dropped log record reading exactly like "the agent never ran it"). - Record the agent-visible capability surface in
task.json: emit one warning per dropped top-level plugin entry fromstage_bundle(todayplugin_bundle.pyhas no logger at all — the only message is docker_runner's success line), and persist the bundle manifest + digest into the run record so a score delta traceable to a missing plugin surface is visible in cross-run comparison rather than reading as agent failure. Why not static: The allowlist itself is intentional and already lint-locked (test_bundle_includes_only_plugin_discovery_subtrees); the gap is that a SPECIFIC plugin's material falls outside it, which depends on that plugin's on-disk layout at run time. Sharpest case:hooks/is allow-listed butscripts/is not, sohooks/hooks.jsoninvoking${CLAUDE_PLUGIN_ROOT}/scripts/x.shships the hook and drops its payload — the plugin arrives partially wired, and only underagent_isolation(default true). Prevents: A7/A8 high — silent plugin-bundle truncation producing different agent capabilities inside vs. outside a protected run, with no diagnostic naming what was dropped. - Add a config-invariant test module for cross-module numeric relationships that CE044 can co-locate but not verify — starting with
CLIENT_TIMEOUT_SECONDS > PASSTHROUGH_TIMEOUT_SECONDS, plusMAX_RESPONSE_BYTESvs the//2passthrough cap, and the mockdSTARTUP_TIMEOUT_SECONDSvs the client deadline. Why not static: The invariant is arithmetic between constants whose meaning ("client deadline must exceed the server's worst-case work") is semantic, not structural — a lint rule can force both constants into one module (CE044) but cannot know which one bounds the other. Prevents: A6 high — the 5s-client / 60s-server inversion, currently untested becausetest_passthrough_is_prefix_limited_and_cachedcalls_passthroughdirectly against a fake server and never crosses the socket.
Top 5 Priority Actions
- Stop the default-on breakage first: either flip
agent_isolationto opt-in for this release or land the task-YAML migration in the same change (src/coder_eval/models/sandbox.py:198), and move the four fail-closed rejections (src/coder_eval/isolation/docker_runner.py:736, :583, :588, :288) intoplan/load time so ninedriver: dockertasks stop silently becoming score-0 ERROR rows only after an image build. - Close the three grading-trust holes that let the agent influence its own score: reject or drop-privilege non-empty
post_rununder isolation, since root executes agent-writtenconftest.py/Makefile/scripts in the chowned sandbox with read access to/opt/coder-eval/grader(src/coder_eval/orchestrator.py:2178, gate at src/coder_eval/isolation/docker_runner.py:736); reap UID-2000 processes beforecheck_all_asyncrather than in the teardownfinally, so criteria grade a frozen workspace (src/coder_eval/orchestrator.py:608 vs :1572); and have mockd write the authoritative invocation record into root-only grader space instead of trusting the agent-ownedcli_mocks/calls.jsonlthatcli_calledparses (src/coder_eval/sandbox.py:586). - Fix the two silent capability losses that read as agent failures: hoist
isolated = agent_isolation_enabled()above theCODER_EVAL_*env pop so Antigravity's protected-modeHOME=/home/agentoverride actually fires instead of leaving the dropped UID with root's unreadableHOME(src/coder_eval/agents/antigravity_agent.py:436), and make the plugin-bundle projection log (or copy) what it drops, sincehooks/ships without its${CLAUDE_PLUGIN_ROOT}/scripts/payload and root.mcp.jsonvanishes with no diagnostic (src/coder_eval/plugin_bundle.py:22). - Harden the protected-mock wire path so infrastructure faults stop masquerading as task failures: make the client deadline strictly exceed the server's passthrough budget (5 s vs 60 s today — src/coder_eval/protected_mock/protocol.py:13, src/coder_eval/protected_mock/server.py:42) and refund the budget decremented at server.py:208 on infrastructure errors, reject unknown fixture keys instead of defaulting the payload away (src/coder_eval/protected_mock/server.py:87), pass an explicit minimal
env=plus an argv-tail allowlist to the passthroughsubprocess.run(src/coder_eval/protected_mock/server.py:236), and mirrorrecord_cli's.errorsentinel in the client's_record(src/coder_eval/protected_mock/client.py:31). - Backfill the missing safety net around the new code: add an in-process client/server round-trip test (the whole handler/serve tail at src/coder_eval/protected_mock/server.py:259-343 and client.py's success path are uncovered, with no live docker test either), fix the
shutil.rmtree(..., ignore_errors=True)staging leak that 0o555/0o444 trees defeat (src/coder_eval/isolation/docker_runner.py:666) along with the same pattern stranding pytest tmp dirs (tests/test_plugin_bundle.py:58), and make the privilege-launcher guard behavioral rather than substring-based (tests/test_docker_identity_isolation.py:43).
Stats: 2 🔴 · 11 🟠 · 9 🟡 · 5 🔵 across 8 axes reviewed.
2 participants
This PR carries the protected mock service and the plugin bundle projection, split out of #87 so that PR stays scoped to the UID/GID architecture; it stacks on
codex/uid-gid-agent-isolationbecause both features build on the agent/grader identity boundary (mockd authorizes callers by the agent UID and needs the image users and groups).Fixture matching behavior is unchanged from the previously merged sub-PR #93.
Protected mock service (mockd)
How it works. Fixtures live in
/opt/coder-eval/mock/fixtures(mode0500, owned by themockduser), so they are not readable by the agent identity. The agent instead gets thin client wrappers named as the mocked CLI (for exampleuip) onPATH, generated bySandbox._generate_protected_mock_clients. The wrappers talk to mockd over a Unix domain socket, so fixture bytes never enter the agent-readable filesystem.How it starts.
docker_runnerstages fixtures and writesmock-config.jsoninto the private grader tree.run_task_internalwraps the turn inrunning_mock_server(...).coder_eval_mockd.sh, whichexecssetpriv --reuid=mockd --regid=mockd --groups=uip-rpc(UID/GID 2100) with all capabilities dropped and--no-new-privs./run/coder-eval/uip.sock, chowns it to theuip-rpcgroup and chmods it0660.The agent user is a member of
uip-rpconly whenprotected_mocksis configured (CODER_EVAL_AGENT_ALLOW_RPC). Startup failures abort loudly, carrying the child's exit code and a tail of its stderr, rather than binding a socket over a partially loaded fixture set.Algorithm (one request).
argv(size-capped atMAX_REQUEST_BYTES; oversized requests are refused before any connection is attempted).SO_PEERCRED- root or the agent UID only.match_mode: subsetrules in fixture-file order (unchanged from feat(protected-mock): add subset fixture matching and harden mockd startup #93).[docsai, ask]), run the real command with no shell, bounded execution time and output size, and an in-memory response cache.Every call is appended to
calls.jsonl.flowchart LR A["agent<br/>UID 2000"] --> W["CLI wrapper<br/>(uip)"] W --> S["unix socket<br/>/run/coder-eval/uip.sock<br/>0660, group uip-rpc"] S --> M["mockd<br/>UID 2100"] M --> F["fixtures<br/>0500, mockd-only"] M -.->|"typed passthrough<br/>(allowlisted prefixes)"| R["real CLI"] A -->|denied| FPlugin bundle projection
How it works. Local plugins are projected into sanitized read-only copies at
/opt/coder-eval/agent-skills/plugin-Ninstead of raw host mounts.agent.plugins[].pathin the task payload is rewritten to point at the projection.How it starts. During
_prepare_isolated_sources, each local plugin source is staged viastage_bundlebefore the container starts. The bundle directory is mounted read-only, and the manifest is kept beside the bundle rather than inside it.Algorithm.
build_manifestwalks only the allowed top-level subtrees (skills,commands,agents,.claude-plugin,hooks), sha256-hashes every file, and validates that symlinks resolve inside the allowed subtrees (absolute links are rejected, as are broken or looping ones).resolution.md,check_*.py) appear inside an agent-visible subtree.stage_bundlerequires an empty destination, copies exactly the manifest-listed files, and re-verifies the digest after copying.flowchart LR H["host plugin checkout"] --> B["build_manifest<br/>allowlist + hashes<br/>+ symlink validation"] B --> S["stage_bundle<br/>copy + digest re-verify"] S --> P["/opt/coder-eval/agent-skills/plugin-N<br/>read-only mount"] P --> A["agent"] H -->|"no raw mount"| AValidation
Ran the full
make verifysequence locally on Windows (invoking the underlying commands directly):ruff format --check src/ tests/ .github/scripts/ruff check src/ tests/ .github/scripts/pyrightantigravity_agent.py:394)pytest tests/test_custom_lint.py(CE001+)pytest tests/ -n auto -m "not live and not lint" --cov-fail-under=80Known-acceptable results:
tests/test_sandbox.py(test_build_run_command_env_preserves_external_plugin_tools_dir,test_capture_to_copies_and_tolerates_dangling_symlink) fail withOSError: [WinError 1314] A required privilege is not held by the client. These are pre-existing environmental failures - Windows symlink creation needs admin or Developer Mode - and are unrelated to this change.TestCE028DocIndexParity::test_every_published_doc_is_in_the_nav, is caused entirely by five untracked local scratch files in my working tree that are not part of this branch. The onlydocs/pages this commit touches -DOCKER_ISOLATION.mdandTASK_DEFINITION_GUIDE.md- are both already in the mkdocs nav, so CE028 is clean for this change and will pass in CI.The diff of this branch against the pre-split base (
dcc73456) is exactly the added test coverage intests/test_protected_mock.pyand nothing else, confirming the split restored the stripped content byte for byte.