From 819cba63c9a4cec0cde4f0edc69d4a68c63329bc Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Mon, 14 Sep 2026 17:25:33 +0100 Subject: [PATCH 1/7] =?UTF-8?q?feat(sandbox):=201/3=20=E2=80=94=20delete?= =?UTF-8?q?=20staged=20task.yaml=20in-container=20after=20load=20(anti-che?= =?UTF-8?q?at)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix A: make /work/input a writable mount (drop :ro; grant input dir writable) and delete the staged task.yaml immediately after load_task in the in-container entry point, gated on IN_CONTAINER_ENV. The staged file is the post-override TaskDefinition with success_criteria, and the agent runs in the same container, so leaving it readable hands over the grading answer key. It is read exactly once; grading reads criteria from memory. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../cli/run_task_internal_command.py | 26 +++++++ src/coder_eval/isolation/docker_runner.py | 20 ++++-- tests/test_docker_runner_mounts.py | 71 ++++++++++++++++++- tests/test_run_task_internal.py | 58 +++++++++++++++ 4 files changed, 170 insertions(+), 5 deletions(-) create mode 100644 tests/test_run_task_internal.py diff --git a/src/coder_eval/cli/run_task_internal_command.py b/src/coder_eval/cli/run_task_internal_command.py index 786ddba9..145974da 100644 --- a/src/coder_eval/cli/run_task_internal_command.py +++ b/src/coder_eval/cli/run_task_internal_command.py @@ -18,6 +18,7 @@ import contextlib import json import logging +import os from pathlib import Path import typer @@ -46,6 +47,26 @@ logger = logging.getLogger(__name__) +def _scrub_staged_task_yaml(task_yaml: Path) -> None: + """Delete the staged ``task.yaml`` after it has been loaded (anti-cheat). + + The post-override :class:`TaskDefinition` (``success_criteria`` included) was + staged at ``/work/input`` for THIS load only. The agent runs in this same + container; leaving the file readable hands it the grading answer key. It is + read exactly once (by ``load_task``) -- grading reads criteria from the + in-memory task, never from disk -- so delete it now. + + Gated on ``IN_CONTAINER_ENV``: docker-only by construction (a host/tempdir + invocation shares our uid and has no filesystem isolation, so there is + nothing to protect and nothing to delete). ``missing_ok`` so a re-entrant or + host call never crashes on an absent file. ``context.json``/``prior.json`` + are deliberately NOT deleted -- they carry no criteria answer key and + ``prior.json`` is read after this point on the regrade path. + """ + if os.environ.get(IN_CONTAINER_ENV) == "1": + task_yaml.unlink(missing_ok=True) + + def heartbeat_is_alive(current: str, last_counter: str, current_mtime: float, last_mtime: float) -> bool: """True when the heartbeat shows a fresh signal of life. @@ -248,6 +269,11 @@ def run_task_internal_command( # `TASK_DIR` env exposed to `run_command` criteria -- resolves to the # original host task directory rather than `/work/input/`. task, source_yaml = load_task(task_yaml) + # ANTI-CHEAT: load_task is task.yaml's sole reader (both the normal and the + # regrade paths run it before their dispatch, and prior.json is read INSIDE + # _grade_recorded_run, strictly after this point). Delete it now so the agent + # in this same container cannot read its own grading criteria back. + _scrub_staged_task_yaml(task_yaml) if host_source_yaml is not None: source_yaml = host_source_yaml # The path below is never re-read; it only seeds Orchestrator's TASK_DIR. diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index 76dd0f76..c45a2405 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -734,9 +734,16 @@ async def run(self) -> EvaluationResult: await asyncio.to_thread(self._prepare_task_dir_mount, staging) # AFTER staging, BEFORE the container starts: the DAC caps are # dropped, so every framework-owned mount must be reachable through - # its `other` bits. Read-only for the inputs the container merely - # consumes; writable only for the run dir it must produce into. - await asyncio.to_thread(grant_container_access, input_dir, writable=False) + # its `other` bits. The input dir is writable, not merely consumed: + # ANTI-CHEAT, the in-container entry point DELETES the staged + # task.yaml (the post-override TaskDefinition, success_criteria + # included) right after loading it, so the agent -- which runs in this + # same container -- can never read its own grading answer key back. + # `unlink` needs `other`-write on the input DIRECTORY through the + # dropped DAC caps, so grant it writable like the run dir it produces + # into. The whole staging tree is destroyed host-side in run()'s + # finally regardless, so a writable input dir strands nothing. + await asyncio.to_thread(grant_container_access, input_dir, writable=True) await asyncio.to_thread(grant_container_access, output_dir, writable=True) if self.grade_workspace is not None: # The graded workspace is a framework-owned mount like any other, @@ -1647,7 +1654,12 @@ def _build_argv( # Explicit value (not name-only) so it overrides any inherited/baked value. argv += ["--env", "TELEMETRY_ENABLED=false"] - argv += ["-v", f"{input_dir.resolve()}:{CONTAINER_INPUT_DIR}:ro"] + # Read-WRITE, not `:ro`: the in-container entry point deletes the staged + # task.yaml right after loading it (ANTI-CHEAT -- see the grant above and + # run_task_internal_command._scrub_staged_task_yaml), and both `rm` and + # `chmod` fail with EROFS on a `:ro` bind mount. The host destroys the + # whole staging tree in run()'s finally, so nothing is stranded. + argv += ["-v", f"{input_dir.resolve()}:{CONTAINER_INPUT_DIR}"] # Mount the host run_dir to the container's standard output location # so the in-container Orchestrator writes task.json/task.log/etc. # directly to the host filesystem via bind-mount. diff --git a/tests/test_docker_runner_mounts.py b/tests/test_docker_runner_mounts.py index dae7d344..32c72bed 100644 --- a/tests/test_docker_runner_mounts.py +++ b/tests/test_docker_runner_mounts.py @@ -35,7 +35,13 @@ _validate_extra_mount, grant_container_access, ) -from coder_eval.models import FileExistsCriterion, ReferenceSource, SandboxConfig, TaskDefinition +from coder_eval.models import ( + CONTAINER_INPUT_DIR, + FileExistsCriterion, + ReferenceSource, + SandboxConfig, + TaskDefinition, +) # DockerRunner targets Linux containers from POSIX hosts. On Windows the test @@ -1053,6 +1059,55 @@ async def fake_exec(*argv, **kwargs): "and DAC_OVERRIDE is dropped" ) + async def test_run_grants_input_dir_writable(self, tmp_path: Path, monkeypatch): + """ANTI-CHEAT: the in-container entry point DELETES the staged task.yaml. + + `unlink` needs `other`-write on the input DIRECTORY through the dropped + DAC caps, so run() must grant the input dir writable (like the run dir), + not read-only. A read-only grant leaves the delete failing EACCES and the + agent able to `cat` its own grading criteria. + """ + monkeypatch.setenv("CODER_EVAL_NO_CLAUDE_MOUNT", "1") + run_dir = tmp_path / "run" + run_dir.mkdir(mode=0o755) + task = TaskDefinition( + task_id="grant-input", + description="test task", + initial_prompt="test", + sandbox=SandboxConfig(), + success_criteria=[FileExistsCriterion(description="c", path="t.txt")], + ) + rt = MagicMock() + rt.task = task + rt.run_dir = run_dir + rt.replicate_index = 0 + rt.variant_id = "default" + rt.config_lineage = {} + rt.source_yaml = "# task" + rt.task_file = tmp_path / "task.yaml" + rt.task_file.write_text("# task", encoding="utf-8") + runner = DockerRunner(rt) + + seen: dict[str, list] = {"grants": []} + + def fake_grant(root, *, writable): + seen["grants"].append((Path(root).name, writable)) + return [] + + async def fake_exec(*argv, **kwargs): + raise FileNotFoundError("docker not present in this test") + + monkeypatch.setattr("coder_eval.isolation.docker_runner.grant_container_access", fake_grant) + monkeypatch.setattr("asyncio.create_subprocess_exec", fake_exec) + with pytest.raises(Exception): # noqa: B017 - the launch failure itself is not under test + await runner.run() + + # The input dir (a mkdtemp under staging; basename is "input") must be + # granted writable=True so the in-container delete of task.yaml succeeds. + input_grants = [w for name, w in seen["grants"] if name == "input"] + assert input_grants, "run() must grant the input dir container access" + assert all(input_grants), "the input dir must be granted writable (the container deletes task.yaml from it)" + class TestTaskDirCopyMount: """$TASK_DIR is a shielded copy at a fixed container path, not the host tree. @@ -1177,3 +1232,17 @@ def test_no_task_file_emits_no_mount(self, tmp_path: Path): runner._prepare_task_dir_mount(staging) assert runner._task_dir_mount_src is None + + def test_input_mount_is_read_write(self, tmp_path: Path): + """ANTI-CHEAT (Fix A): the container deletes the staged task.yaml. + + A `:ro` input mount rejects `rm` with EROFS, so the mount must be + read-write. The output mount is already writable for symmetry. + """ + argv, _ = self._prepared_argv(tmp_path) + + input_specs = [m for m in self._mounts(argv) if m.endswith(CONTAINER_INPUT_DIR)] + assert len(input_specs) == 1 + assert not input_specs[0].endswith(":ro"), "the input mount must be writable so task.yaml can be deleted" + output_specs = [m for m in self._mounts(argv) if m.endswith(CONTAINER_OUTPUT_DIR)] + assert output_specs and not output_specs[0].endswith(":ro") diff --git a/tests/test_run_task_internal.py b/tests/test_run_task_internal.py new file mode 100644 index 00000000..1807aec9 --- /dev/null +++ b/tests/test_run_task_internal.py @@ -0,0 +1,58 @@ +"""Tests for the in-container entry point (`coder-eval _run-task-internal`). + +CE048: the command itself is a Typer command whose parameter defaults are +`OptionInfo` sentinels, so it must never be invoked in-process. These tests +target the extracted, pure `_scrub_staged_task_yaml` helper instead. +""" + +from __future__ import annotations + +from pathlib import Path + +from coder_eval.cli.run_task_internal_command import _scrub_staged_task_yaml +from coder_eval.models import IN_CONTAINER_ENV + + +class TestScrubStagedTaskYaml: + """Anti-cheat: the staged task.yaml (with success_criteria) is deleted after + load so the in-container agent cannot read its own grading answer key.""" + + def test_deletes_when_in_container(self, tmp_path: Path, monkeypatch): + monkeypatch.setenv(IN_CONTAINER_ENV, "1") + task_yaml = tmp_path / "task.yaml" + task_yaml.write_text("task_id: x\nsuccess_criteria: []\n", encoding="utf-8") + + _scrub_staged_task_yaml(task_yaml) + + assert not task_yaml.exists() + + def test_survives_when_not_in_container(self, tmp_path: Path, monkeypatch): + # A host/tempdir invocation shares our uid and has no filesystem + # isolation; the gate keeps the delete docker-only by construction. + monkeypatch.delenv(IN_CONTAINER_ENV, raising=False) + task_yaml = tmp_path / "task.yaml" + task_yaml.write_text("task_id: x\n", encoding="utf-8") + + _scrub_staged_task_yaml(task_yaml) + + assert task_yaml.exists() + + def test_env_var_set_but_not_one_leaves_file(self, tmp_path: Path, monkeypatch): + # Only the exact "1" sentinel arms the delete, mirroring the reference + # window gate (Sandbox.enforces_permission_windows). + monkeypatch.setenv(IN_CONTAINER_ENV, "0") + task_yaml = tmp_path / "task.yaml" + task_yaml.write_text("task_id: x\n", encoding="utf-8") + + _scrub_staged_task_yaml(task_yaml) + + assert task_yaml.exists() + + def test_missing_file_does_not_raise(self, tmp_path: Path, monkeypatch): + monkeypatch.setenv(IN_CONTAINER_ENV, "1") + task_yaml = tmp_path / "does-not-exist.yaml" + + # missing_ok=True: a re-entrant call must not crash on an absent file. + _scrub_staged_task_yaml(task_yaml) + + assert not task_yaml.exists() From 4875369e8d85e239cc6d3ea83c00fb21baa04818 Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Mon, 14 Sep 2026 17:30:58 +0100 Subject: [PATCH 2/7] =?UTF-8?q?feat(sandbox):=202/3=20=E2=80=94=20allowlis?= =?UTF-8?q?t=20tmpfs-mask=20of=20auto-mounted=20plugin=20trees?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix B: default-deny mask over an auto-mounted Claude-plugin root. The whole root stays :ro-mounted so the plugin loads, but every child dir outside the keep-set (.claude-plugin + manifest-declared skill dirs) is masked with an empty tmpfs. Colocated eval material — sibling task YAMLs, reference solutions, tests/, node_modules/ — is masked by default so an unknown layout can never leak. New pure helper eval_material.mask_dirs; shared skill-dir resolver manifest_skill_dirs (renamed public in agents/_skills.py) is the SSOT for "what is a skill dir". Co-Authored-By: Claude Opus 4.8 (1M context) --- src/coder_eval/agents/_skills.py | 4 +- src/coder_eval/isolation/docker_runner.py | 21 ++++ src/coder_eval/isolation/eval_material.py | 94 +++++++++++++++ tests/test_docker_runner_mounts.py | 121 +++++++++++++++++++ tests/test_eval_material.py | 137 ++++++++++++++++++++++ 5 files changed, 375 insertions(+), 2 deletions(-) create mode 100644 src/coder_eval/isolation/eval_material.py create mode 100644 tests/test_eval_material.py diff --git a/src/coder_eval/agents/_skills.py b/src/coder_eval/agents/_skills.py index 60bdb11b..03f30eaf 100644 --- a/src/coder_eval/agents/_skills.py +++ b/src/coder_eval/agents/_skills.py @@ -29,7 +29,7 @@ _SKILL_FILE = "SKILL.md" -def _manifest_skill_dirs(root: Path) -> list[Path]: +def manifest_skill_dirs(root: Path) -> list[Path]: """Skill directories a Claude-plugin root declares, in manifest order. Reads the ``skills`` field of ``/.claude-plugin/plugin.json`` (a string @@ -89,7 +89,7 @@ def _plugin_skill_dirs( hint, ) continue - candidates = [directory for directory in _manifest_skill_dirs(root) if directory.is_dir()] + candidates = [directory for directory in manifest_skill_dirs(root) if directory.is_dir()] # A path that is ALREADY a bare skills directory (//SKILL.md) # has no `skills/` subdir, so use it as-is. Deliberately not a fallback for # a root that HAS one: `skills.paths` is scanned recursively and a repo diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index c45a2405..b58d26e8 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -162,6 +162,9 @@ def _rewrite_loopback_for_container(url: str) -> str | None: # the same guard on the orchestrator's post-run subprocesses. STDOUT_LINE_LIMIT_BYTES = 64 * 1024 * 1024 # 64 MiB +# Logged once per masked child of an auto-mounted plugin root (Fix B allowlist). +_MASK_WARNING = "Masking non-skill path %s under plugin root %s (anti-cheat: only skills stay readable)." + async def _heartbeat_loop(heartbeat_path: Path) -> None: """Write a monotonic counter to ``heartbeat_path`` every interval until cancelled. @@ -1731,6 +1734,11 @@ def _build_argv( # `~/.aws/config`). The warning surfaces the surprise. sensitive_sources = self._sensitive_source_paths() + # Lazy import: eval_material -> agents._skills triggers agents/__init__, + # which imports back into this module (opencode_agent). Importing it here, + # after this module is fully initialised, breaks that cycle. + from coder_eval.isolation.eval_material import mask_dirs + def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: if not raw_path: return @@ -1749,6 +1757,19 @@ def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: break mounted.add(target) argv.extend(["-v", f"{target}:{target}:ro"]) + # ANTI-CHEAT (allowlist / default-deny): if `target` is a Claude-plugin + # root, the plugin stays mounted whole (:ro, above) so it still loads, + # but every child dir that is NOT the plugin surface (.claude-plugin + + # the manifest-declared skill dirs) is masked with an empty tmpfs. This + # closes the whole-suite channel: sibling task YAMLs, reference + # solutions, and test fixtures colocated under the tree are masked by + # default. `mask_dirs` returns [] for a non-plugin root, so a plain + # template dir / system_prompt_file parent is untouched. Docker applies + # mounts by target-path depth, so the deeper --tmpfs wins over the :ro + # bind regardless of argv order. + for masked_dir in mask_dirs(target): + argv.extend(["--tmpfs", str(masked_dir)]) + logger.warning(_MASK_WARNING, masked_dir, target) plugins = (self.rt.task.agent.plugins if self.rt.task.agent else None) or [] for plugin in plugins: diff --git a/src/coder_eval/isolation/eval_material.py b/src/coder_eval/isolation/eval_material.py new file mode 100644 index 00000000..a3b9a2b2 --- /dev/null +++ b/src/coder_eval/isolation/eval_material.py @@ -0,0 +1,94 @@ +"""Allowlist (default-deny) mask for auto-mounted Claude-plugin trees. + +A ``driver: docker`` run auto-mounts an ``agent.plugins[].path`` (or a +``TemplateDirSource.path`` that is itself a plugin root) at its host path, ``:ro``, +so the plugin loads. That same tree often holds eval material as siblings of the +skills dir -- sibling task YAMLs, reference solutions, test fixtures -- which the +agent under evaluation could then read as its own answer key. + +This module answers the one question the docker runner needs: *given a mounted +plugin root, which child directories must be tmpfs-masked so that only the plugin +surface (``.claude-plugin`` + the manifest-declared skill dirs) stays readable?* +It is a **security** control, so the posture is default-deny: keep the skill +surface, mask everything else. An unknown or new eval layout is masked by +default and can never leak. + +Pure and dependency-light: a shallow ``iterdir`` + keep-set. No ``os.walk``, no +YAML parsing, no prune-set. The caller (``DockerRunner._build_argv``) emits a +``--tmpfs`` over each returned path and logs it. +""" + +from __future__ import annotations + +from pathlib import Path + +from coder_eval.agents._skills import manifest_skill_dirs + + +_PLUGIN_MANIFEST_RELPATH = (".claude-plugin", "plugin.json") + + +def mask_dirs(root: Path) -> list[Path]: + """Directories under a mounted plugin ``root`` to tmpfs-mask. + + Returns ``[]`` unless ``root`` is a plugin root (has + ``.claude-plugin/plugin.json``) -- a plain template dir or the + ``system_prompt_file`` parent carries no skill/eval convention, so its author + controls it and nothing is masked. + + Otherwise default-deny: the keep-set is ``.claude-plugin`` + the + manifest-declared skill dirs (via the shared ``manifest_skill_dirs`` + resolver -- one SSOT for "what is a skill dir"; ``"skills"`` is never + hardcoded). Every other child directory is masked. For a NESTED skills path + (e.g. declared ``src/skills``) the mask is applied at the granularity needed + to keep exactly the declared skill dir(s) + ``.claude-plugin`` -- siblings are + masked at each level down to the skills dir, so ``src/`` is not over-exposed. + + Symlinked children are skipped: a symlink is not a valid tmpfs mountpoint, + and a symlink inside the mounted tree resolves against the CONTAINER's + filesystem (which holds no eval material), not the host's -- so it is not a + leak. + """ + root = root.resolve() + if not (root / Path(*_PLUGIN_MANIFEST_RELPATH)).is_file(): + return [] + + keep = {(root / ".claude-plugin").resolve(), *manifest_skill_dirs(root)} + + # "Protected" = every kept path AND every ancestor of a kept path down to + # (but excluding) the root. A protected dir is never masked; instead we + # descend into it and mask ITS non-protected children. This is what keeps a + # nested `src/skills` from over-exposing all of `src/`. + protected: set[Path] = set() + for kept in keep: + protected.add(kept) + for ancestor in kept.parents: + if ancestor == root: + break + if root in ancestor.parents: + protected.add(ancestor) + + # Directories to descend into: the root plus every protected ANCESTOR (a + # protected path that is not itself a kept leaf). We never descend into a + # kept skill dir -- everything inside it stays readable, including the + # skill's own supporting assets. + descend = {root, *(p for p in protected if p not in keep)} + + masked: set[Path] = set() + for directory in descend: + # Only descend into a real (non-symlink) directory. + if not directory.is_dir() or directory.is_symlink(): + continue + for child in directory.iterdir(): + if child.is_symlink(): + # Not a valid tmpfs mountpoint; resolves against the container fs + # (no host eval material). Skip -- not a leak. + continue + if not child.is_dir(): + continue + resolved = child.resolve() + if resolved in protected: + continue + masked.add(resolved) + + return sorted(masked) diff --git a/tests/test_docker_runner_mounts.py b/tests/test_docker_runner_mounts.py index 32c72bed..d824de6c 100644 --- a/tests/test_docker_runner_mounts.py +++ b/tests/test_docker_runner_mounts.py @@ -721,6 +721,127 @@ def test_container_paths_reexported_from_docker_runner(self): assert CONTAINER_OUTPUT_DIR == "/work/output" +class TestAutoMountAllowlistMask: + """Auto-mounted plugin trees are default-deny masked (Fix B). + + A plugin root is mounted whole (:ro, so it loads), but every child dir that + is not the plugin surface (.claude-plugin + declared skill dirs) is masked + with an empty tmpfs so colocated eval material (sibling task YAMLs, reference + solutions, tests/, node_modules/) can never be read by the agent. + """ + + import json as _json + + @staticmethod + def _mounts(argv: list[str]) -> list[str]: + return [argv[i + 1] for i, a in enumerate(argv) if a == "-v"] + + @staticmethod + def _tmpfs(argv: list[str]) -> list[str]: + return [argv[i + 1] for i, a in enumerate(argv) if a == "--tmpfs"] + + def _plugin_root(self, root: Path, *, skills=None) -> Path: + (root / ".claude-plugin").mkdir(parents=True) + payload = {"name": "demo"} + if skills is not None: + payload["skills"] = skills + (root / ".claude-plugin" / "plugin.json").write_text(self._json.dumps(payload), encoding="utf-8") + return root + + def _runner(self, tmp_path: Path, *, plugins=None, template_sources=None) -> DockerRunner: + from coder_eval.models import DockerDriverConfig + + agent_kwargs: dict = {"type": "claude-code"} + if plugins is not None: + agent_kwargs["plugins"] = plugins + sandbox_kwargs: dict = {"driver": "docker", "docker": DockerDriverConfig()} + if template_sources is not None: + sandbox_kwargs["template_sources"] = template_sources + + task = TaskDefinition( + task_id="test", + description="test task", + initial_prompt="test", + agent=agent_kwargs, # dict -> validated into the AgentConfig union + sandbox=SandboxConfig(**sandbox_kwargs), + success_criteria=[FileExistsCriterion(description="c", path="t.txt")], + ) + rt = MagicMock() + rt.task = task + rt.run_dir = tmp_path / "run" + rt.task_file = None + return DockerRunner(rt) + + def _argv(self, runner: DockerRunner, tmp_path: Path) -> list[str]: + input_dir = tmp_path / "input" + output_dir = tmp_path / "output" + input_dir.mkdir(exist_ok=True) + output_dir.mkdir(exist_ok=True) + return runner._build_argv(input_dir, output_dir, container_name="c", image="img") + + def test_plugin_root_masks_non_skill_children(self, tmp_path: Path): + root = self._plugin_root(tmp_path / "plugin") + (root / "skills" / "demo").mkdir(parents=True) + (root / "tests").mkdir() + (root / "reference").mkdir() + (root / "node_modules").mkdir() + + runner = self._runner(tmp_path, plugins=[{"type": "local", "path": str(root)}]) + argv = self._argv(runner, tmp_path) + + tmpfs = self._tmpfs(argv) + # The whole root is :ro-mounted so the plugin loads. + assert f"{root.resolve()}:{root.resolve()}:ro" in self._mounts(argv) + # Non-skill children masked; skill surface + manifest not. + assert str((root / "tests").resolve()) in tmpfs + assert str((root / "reference").resolve()) in tmpfs + assert str((root / "node_modules").resolve()) in tmpfs + assert str((root / "skills").resolve()) not in tmpfs + assert str((root / ".claude-plugin").resolve()) not in tmpfs + + def test_tmpfs_targets_are_under_mounted_host_root(self, tmp_path: Path): + # Codex symlinks / Antigravity search paths dereference the ORIGINAL + # mounted host path, so the mask must sit on that path, not a copy. + root = self._plugin_root(tmp_path / "plugin") + (root / "skills" / "demo").mkdir(parents=True) + (root / "tests").mkdir() + + runner = self._runner(tmp_path, plugins=[{"type": "local", "path": str(root)}]) + argv = self._argv(runner, tmp_path) + + for masked in self._tmpfs(argv): + assert Path(masked).is_relative_to(root.resolve()) + + def test_template_source_plugin_root_is_masked(self, tmp_path: Path): + from coder_eval.models import TemplateDirSource + + root = self._plugin_root(tmp_path / "tpl") + (root / "skills" / "demo").mkdir(parents=True) + (root / "tests").mkdir() + + runner = self._runner(tmp_path, template_sources=[TemplateDirSource(path=str(root))]) + argv = self._argv(runner, tmp_path) + + assert str((root / "tests").resolve()) in self._tmpfs(argv) + + def test_non_plugin_template_dir_emits_no_mask(self, tmp_path: Path): + from coder_eval.models import TemplateDirSource + + plain = tmp_path / "plain_tpl" + (plain / "some_files").mkdir(parents=True) + + runner = self._runner(tmp_path, template_sources=[TemplateDirSource(path=str(plain))]) + argv = self._argv(runner, tmp_path) + + assert self._tmpfs(argv) == [] + + def test_no_plugins_no_mask(self, tmp_path: Path): + runner = self._runner(tmp_path) + argv = self._argv(runner, tmp_path) + + assert self._tmpfs(argv) == [] + + class TestReferenceMountAntiCheat: """The reference must reach the harness but never the agent under evaluation.""" diff --git a/tests/test_eval_material.py b/tests/test_eval_material.py new file mode 100644 index 00000000..a700c535 --- /dev/null +++ b/tests/test_eval_material.py @@ -0,0 +1,137 @@ +"""Unit tests for the allowlist (default-deny) mask over auto-mounted plugin trees. + +`eval_material.mask_dirs(root)` returns the child dirs to tmpfs-mask so that only +`.claude-plugin` + the manifest-declared skill dirs stay readable under a mounted +Claude-plugin root. Everything else (sibling task YAMLs, reference solutions, +`tests/`, `node_modules/`) is masked by default so it can never leak. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest + +from coder_eval.isolation.eval_material import mask_dirs + + +def _plugin_root(root: Path, *, skills: str | list[str] | None = None) -> Path: + """Create a `.claude-plugin/plugin.json`, optionally declaring a skills path.""" + manifest_dir = root / ".claude-plugin" + manifest_dir.mkdir(parents=True) + payload: dict = {"name": "demo"} + if skills is not None: + payload["skills"] = skills + (manifest_dir / "plugin.json").write_text(json.dumps(payload), encoding="utf-8") + return root + + +class TestMaskDirs: + def test_non_plugin_root_returns_empty(self, tmp_path: Path): + # No .claude-plugin/plugin.json -> not a plugin root -> nothing masked. + (tmp_path / "some_dir").mkdir() + assert mask_dirs(tmp_path) == [] + + def test_default_skills_layout_masks_non_skill_children(self, tmp_path: Path): + root = _plugin_root(tmp_path) + (root / "skills" / "demo").mkdir(parents=True) + (root / "skills" / "demo" / "SKILL.md").write_text("x", encoding="utf-8") + (root / "skills" / "demo" / "helper.py").write_text("x", encoding="utf-8") + (root / "tests").mkdir() + (root / "reference").mkdir() + (root / "node_modules").mkdir() + (root / "fixtures").mkdir() + + masked = set(mask_dirs(root)) + + assert masked == { + (root / "tests").resolve(), + (root / "reference").resolve(), + (root / "node_modules").resolve(), + (root / "fixtures").resolve(), + } + # Skill surface and manifest stay readable. + assert (root / "skills").resolve() not in masked + assert (root / ".claude-plugin").resolve() not in masked + + def test_skill_internal_assets_stay_readable(self, tmp_path: Path): + root = _plugin_root(tmp_path) + (root / "skills" / "demo" / "assets").mkdir(parents=True) + (root / "tests").mkdir() + + masked = set(mask_dirs(root)) + + # A skill's own subdirs are inside a kept dir -> never masked. + assert (root / "skills" / "demo" / "assets").resolve() not in masked + assert (root / "tests").resolve() in masked + + def test_nested_skills_path_does_not_over_expose_src(self, tmp_path: Path): + root = _plugin_root(tmp_path, skills="src/skills") + (root / "src" / "skills" / "demo").mkdir(parents=True) + (root / "src" / "secret_lib").mkdir(parents=True) + (root / "tests").mkdir() + + masked = set(mask_dirs(root)) + + # src/skills is kept; src/secret_lib (a sibling under src) is masked; src + # itself is NOT masked (it is an ancestor of the kept skills dir). + assert (root / "src" / "skills").resolve() not in masked + assert (root / "src").resolve() not in masked + assert (root / "src" / "secret_lib").resolve() in masked + assert (root / "tests").resolve() in masked + + def test_multiple_declared_skill_dirs_all_kept(self, tmp_path: Path): + root = _plugin_root(tmp_path, skills=["skills", "extra_skills"]) + (root / "skills" / "a").mkdir(parents=True) + (root / "extra_skills" / "b").mkdir(parents=True) + (root / "tests").mkdir() + + masked = set(mask_dirs(root)) + + assert (root / "skills").resolve() not in masked + assert (root / "extra_skills").resolve() not in masked + assert (root / "tests").resolve() in masked + + def test_symlinked_child_is_skipped(self, tmp_path: Path): + root = _plugin_root(tmp_path) + (root / "skills" / "demo").mkdir(parents=True) + outside = tmp_path.parent / "outside_target" + outside.mkdir(exist_ok=True) + os.symlink(outside, root / "evil_link") + + masked = set(mask_dirs(root)) + + # A symlinked child is not a valid tmpfs mountpoint and resolves against + # the container fs -> not masked. + assert (root / "evil_link") not in masked + assert outside.resolve() not in masked + + def test_reference_dir_under_root_is_masked(self, tmp_path: Path): + root = _plugin_root(tmp_path) + (root / "skills" / "demo").mkdir(parents=True) + (root / "solution").mkdir() # a reference dir sibling of skills + + assert (root / "solution").resolve() in set(mask_dirs(root)) + + def test_files_at_root_are_not_masked(self, tmp_path: Path): + # Only directories can be tmpfs mountpoints; a stray file is left alone. + root = _plugin_root(tmp_path) + (root / "skills" / "demo").mkdir(parents=True) + (root / "README.md").write_text("x", encoding="utf-8") + + assert (root / "README.md").resolve() not in set(mask_dirs(root)) + + +class TestSharedResolver: + def test_uses_shared_manifest_skill_dirs(self): + # SSOT: eval_material and CE065 must agree on "what is a skill dir". + from coder_eval.agents._skills import manifest_skill_dirs + from coder_eval.isolation import eval_material + + assert eval_material.manifest_skill_dirs is manifest_skill_dirs + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(pytest.main([__file__, "-v"])) From 75367a24dd2ef298cf969b32c2bdeb0679da5054 Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Mon, 14 Sep 2026 17:34:28 +0100 Subject: [PATCH 3/7] =?UTF-8?q?feat(lint):=203/3=20=E2=80=94=20CE065=20fla?= =?UTF-8?q?gs=20eval=20material=20inside=20a=20skill=20dir?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix C: a static lint rule (pytest class, CE055 template) that flags the one residual the Fix B allowlist cannot close — a task_id: YAML or a resolved reference.directory colocated INSIDE a skill dir reached through an in-repo agent.plugins[].path / sandbox.template_sources[].path. Masking a skill dir would hide the skill, so such material stays readable and leaks the grading answer key under driver: docker. Reuses the shared manifest_skill_dirs resolver (SSOT, import-identity asserted) with positive+negative sensors. Documents both docker anti-cheat fixes in docs/DOCKER_ISOLATION.md and the CE065 entry in CLAUDE.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 2 +- docs/DOCKER_ISOLATION.md | 33 +++++++ tests/test_custom_lint.py | 178 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 212 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 857fb447..3e6ed3d6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -236,7 +236,7 @@ make plugin-reference # the plugin's bundled criteria reference from the models Editing `src/coder_eval/pricing.py` means editing `evalboard/lib/pricing.ts` too — it is a hand-copied mirror, and `evalboard/lib/__tests__/pricing-parity.test.ts` fails the build on drift in either direction. -Recent additions, each traceable to a shipped defect: **CE064** (in `src/coder_eval/agents/`, a module that imports `TurnClock` must pass an explicit `timestamp=` to `AgentStartEvent` and `AgentEndEvent` — the turn's OUTER bounds, which no other rule looks at, since CE058-CE061 all scope to `AssistantMessage` and the bracket is not one. `timing.decompose_turn` produces `harness_startup_ms` / `harness_teardown_ms` by subtracting a generation-window bound from a bracket timestamp, so the two must share a basis; all three clocked harnesses derived their bounds from the `TurnClock` and let the bracket fall back to `StreamEvent.timestamp`'s `default_factory=datetime.now`, putting a monotonic-derived stamp and a raw wall stamp inside one subtraction — the exact split `TurnClock` exists to remove, reintroduced at the one seam the clock did not own. Measured on a live antigravity turn: an `AgentEndEvent` stamped **17 us BEFORE its own last message finished**, which cannot happen (the event is constructed strictly after the final flush), and `decompose_turn` clamped that negative and published `0.0` — "measured, and instant", the CE058 confusion arrived at from the other direction — for a harness whose real tail is ~0.1 ms; it now records 0.035 ms. It surfaced on one harness only because the drift is tens of microseconds and antigravity is the only one that holds its process across turns, so nothing happens between its last flush and its end event; every other harness books a tail of 7-543 ms, where the drift is invisible rather than absent — which is why the fix is at every clocked site rather than at that one. SCOPE IS DERIVED, never a harness list: codex and opencode take their spans from the CLI's own epoch stamps, deliberately have no `TurnClock`, and are correctly invisible to the rule — a raw bracket is CONSISTENT with their bounds — and the day either adopts a clock the rule starts applying with no edit. BLIND SPOT, in the rule's docstring: presence, not correctness. It cannot tell `self.clock.now()` from a `datetime.now()` spelled out at the call site, because the three harnesses legitimately reach their clock three ways; the guard for the SOURCE is behavioural (`tests/_bracket_clock.py` injects a stand-in anchored a year from real time, so a reverted argument fails by a year rather than by the microseconds that separate the two clocks), which is the division of labour CE060 states — a rule removes the SILENT case, a default nobody chose), **CE063** (no module in `src/coder_eval/agents/` may import `busy_ms` — tool execution comes out of a generation window in exactly ONE place, `timing.py::subtract_tool_time`. Five reducers used to do it themselves while the head and tail were already computed centrally at the same seam, and that asymmetry is where every timing defect on this branch lived — none of them in the arithmetic, all of them in the bookkeeping AROUND it: when to reset a per-step span list (clearing it at `step_start` wiped a span before the flush could subtract it, a 100% overstatement of that window), when to clear a spent start stamp (a second flush with no intervening start republished the previous span — 3000 ms of generation for a 2000 ms turn), when to advance the mark. A sixth harness reaching for `busy_ms` rebuilds that, and its tool time is then subtracted TWICE — by the reducer and again by the collector — under-reporting generation on one harness only, which takes a corpus comparison to notice. A separate id from CE061 rather than a rebody: CE061 asks where a window's ARITHMETIC came from and four reducers still call `close_window`, so its property is live and unsuperseded; this asks whether a reducer subtracts at all. It deliberately does NOT reuse CE061's `_imports_the_helper`, whose bare-module-import branch exists so `timing.close_window(...)` counts as reaching the helper — inverted into a ban that branch flags four of the five reducers. CE061 is now **exemption-free**: claude-code was its one permanent `# noqa` and, with the subtraction moved, calls the shrunken `close_window` like the other four), **CE060** (in `src/coder_eval/agents/`, every `AssistantMessage(...)` must pass `message_id` explicitly — an identity invariant, which is why it is its own id rather than a second arm of CE058/CE059, both of which are about timing. Antigravity omitted the kwarg, so the field defaulted to `None` on every message it ever recorded, and the evalboard — which groups assistant emissions by `message_id` and falls back to a `SAME_EMISSION_GAP_MS` wall-clock gap when either side lacks one — collapsed a whole turn's generations into ONE timeline row as soon as the harness's generation windows became contiguous (the gap is then exactly 0 ms, always). Nothing failed: the consumer SUMS the group, so the totals and the reconciliation invariant stayed right, and the golden snapshots had ratified the `null` on the day they were written — a snapshot is regenerated from whatever the code currently does, so it catches a later change and never an initial omission. The damage was not confined to the timeline, which is why "only granularity is lost" was the wrong way to describe it: a grouped emission is one API call to the evalboard's thinking-cost simulator, whose prompt-cache cascade is quadratic in that count, so a single-shot Antigravity run had every cascade coefficient pinned at zero; the `Messages` count and the 10 s slow-generation bar were per-turn too. Unlike its two siblings it **derives its constructor set from each module's own `coder_eval.models` imports** instead of hardcoding the spelling, which closes exactly the blind spot CE058's clause below concedes: `claude_code_agent.py` binds only `AssistantMessage as AssistantMessageTelemetry`, so a name list guards that file's two construction sites purely by coincidence, and an arbitrary `as Msg` is missed outright. Widening CE058/CE059 the same way is recorded in `.claude/harness-candidates.md`. BLIND SPOT, in the rule's docstring: the runtime `None` — the kwarg must be PRESENT, not statically non-`None`, because OpenCode's `messageID` and Pi's `responseId` legitimately evaluate to `None` when the CLI omits them, and passing a fallback expression *is* deciding), **CE058** (in `src/coder_eval/`, an unknown timing value may not become a numeric literal — `duration_ms is None` means *never timed* and `0.0` means *timed and instant*, so writing the literal publishes the second while meaning the first. One invariant, one id, five syntactic forms — a zero constructor keyword, `x or 0`, `x if x is not None else 0.0`, `if x.duration_ms is None: x.duration_ms = 0.0`, and a `model_copy(update={...})` dict (the shape the Antigravity DONE path writes through, which a keyword-only rule cannot see). Antigravity constructed EVERY message with `generation_duration_ms=0.0`, so the task page's Generation cell read `0ms` and its breakdown rendered `0%` for months with nothing failing; Codex published the SDK's `0.0` as a measured command duration, so `avg_command_time_ms` divided real milliseconds by a command count of which 70 of 211 in one nightly had never been timed. The fourth form is the one no existing rule shape covered and is where a live instance was hiding — `claude_code_agent._finalize_commands` set `0.0` on every command force-closed without a tool result, in the one harness a timing audit had called healthy. BLIND SPOT, stated in the rule's docstring: form 1 keys on the callee's spelling, so renaming the `AssistantMessageTelemetry` import alias silently disarms it there), **CE059** (in `src/coder_eval/agents/`, an `AssistantMessage` may not receive the same `ast.Name` for both `started_at` and `completed_at` — the Antigravity reducer read `datetime.now()` once and passed it as both bounds, so `started_at == completed_at` on 368 of 368 sampled messages. A separate id from CE058 because it is a separate invariant, a zero-length window whatever the duration field says, and one invariant per id is what makes a `# noqa` mean one thing. It does NOT fire when the same call passes `generation_duration_ms=None`: a call that says, in the field built to say it, that no window was measurable is not claiming one — that exemption is what keeps the rule pointed at the misleading case instead of accumulating four permanent suppressions on the rollout-rebuild and sub-agent-synthesis sites), **CE056** (no bare `CODER_EVAL_IN_CONTAINER` literal outside `models/container_paths.py` — the CE053 shape again: a rename-safety constant that shipped beside the literal it replaced, and the straggler was the single WRITER, so a rename would have disarmed four security/correctness gates at once with nothing failing; CE052 cannot catch it because that rule inspects `if` guards and the writer is not one), **CE055** (a criterion `path:` in `tasks/` must be sandbox-relative — an absolute path is joined onto the sandbox root, which DISCARDS the root, so containment refuses it and the criterion can never match whatever the agent does; two in-tree tasks were broken this way and the pair is the argument for a static rule on top of the runtime `CheckerMisuseError`: `byod_smoke_test` IS in a CI bucket and produced only `Results: 7/8 succeeded` plus a gating 0.0 reading "file does not exist" for a file that existed, while `dockerfile_build_example` is in NO bucket, so nothing ran it and no runtime guard was ever reached — the fix is never to relax containment but to say what the criterion means, `run_command: test -f /opt/marker`, a claim about the container IMAGE rather than about the agent's workspace), **CE054** (an `environment_info` key that is READ must be WRITTEN somewhere in `src/` — the bag is `dict[str, Any]`, so nothing connects reader to writer, and the `reference_digest` anti-cheat guard shipped as a read with no writer anywhere: `.get()` returned `None`, the guard took its early return, and CLAUDE.md plus the user guide both described it as protection it never provided), **CE048** (never call a Typer command function in process — its parameter defaults are `OptionInfo` sentinels, not values, and the sentinel is TRUTHY, so `in_place=None` silently selected the wrong branch; the fix is the `run_pipeline` / `run_evaluation` / `run_plan` split, and this rule is the one that also scans `tests/`, since that is the only place the defect occurs), **CE049** (never coalesce a possibly-unmeasured score to a numeric literal — `score or 0.0` publishes "measured and scored zero" while meaning "never measured", which is how an ungraded night reached four unfiltered `avg(Score)` dashboards as a real zero), **CE050** (no untyped `getattr` probe for a discriminated-union field — pyright cannot see the string, so a rename degrades the guard to a permanent no-op; scoped to criterion-shaped receivers because `command`/`tool`/`prompt` are far too common to flag on their own), **CE051** (a sandbox driver may not be rewritten silently — the driver IS the isolation boundary, so a downgrade must be an explicit, stamped, operator-visible decision), **CE053** (no bare run-record or run-LOG filename literal outside `path_utils` — widened to `docker.log` / `grade.docker.log` / `task.log` / `grade.log` after the same shape recurred: `docker.log` was produced in `isolation/` and consumed in `orchestration/` as three unrelated literals, and because the consumer guards its copy with `is_file()`, a rename would have silently discarded the only record of why a grading container failed — `TASK_JSON_FILENAME` shipped with a rename-safety rationale while twelve exact literals stayed unmigrated, including all three `rglob("task.json")` sites the constant's own comment cites as its reason to exist, so it created the second source of truth it argues against), **CE052** (an `os._exit` must sit inside a branch testing `CODER_EVAL_IN_CONTAINER` — it is the right primitive only for reaping the container's own disposable main process, and `run_task_internal_command` armed its heartbeat watchdog, a daemon thread whose whole authority is `os._exit(137)`, unconditionally: a test that invoked the command in-process left the pytest worker holding that thread, which exited the worker 40s later inside an unrelated test file, naming a different test on each run and on each platform with no traceback — and the dead worker's lost coverage data then failed the gate as `65.13 < 80.00`, naming neither the test nor the cause), **CE037** (no unreferenced module-level private helper in `src/` — a helper whose docstring documents a bug the live code still has is worse than none), **CE038** (in an `@asynccontextmanager`, the acquire must sit INSIDE the `try` whose `finally` releases it — `asyncio.shield` protects the inner task, NOT the await, so a cancel on `__aenter__` skips the unwind while the work completes), **CE039** (a criterion checker must not return a gating `score=0.0` from an `except OSError` over a path the *task author* named — that books an eval-config error as an agent failure; raise `CheckerMisuseError` instead, and `# noqa: CE039` the cases that really are the agent's), **CE047** (every onboarding/marketing surface — README, `docs/index.md`, `docs/comparison.md`, `docs/llms.txt`, `mkdocs.yml`'s `site_description`, the Pages stub, and pyproject's `description`/`keywords` — must name every built-in `AgentKind`; OpenCode shipped while four of those seven still listed three harnesses, and nothing failed), **CE057** (a module copied into the recorder directory beside a generated sandbox shim — `models.sandbox.SIDECAR_MODULES`, currently `argv_match.py` — may import stdlib only. The failure is silent: the sidecar runs where `coder_eval` and its dependencies are not installed, so one package import makes every shadowed CLI die with an ImportError the agent reads as "the tool is broken", costing a whole run to diagnose. The rule derives its target set from that exported tuple and a test asserts it matches a file that exists — a lint rule guarding zero files must fail, not pass). +Recent additions, each traceable to a shipped defect: **CE065** (in `tasks/`, no `task_id:`-bearing YAML and no resolved `reference.directory` may live INSIDE a skill dir reached through an in-repo `agent.plugins[].path` / `sandbox.template_sources[].path` — the ONE residual the docker anti-cheat allowlist (`isolation/eval_material.py::mask_dirs`) cannot close. Under `driver: docker` the whole plugin root is auto-mounted `:ro` so it loads, and every child dir OUTSIDE the keep-set (`.claude-plugin` + the manifest-declared skill dirs) is tmpfs-masked by default, so sibling task YAMLs / reference solutions / `tests/` are masked for free; but masking a skill dir would hide the skill, so an eval def or reference COLOCATED inside a skill dir stays readable and hands the agent under evaluation its own grading answer key. The rule reuses the SAME `manifest_skill_dirs` resolver (`agents/_skills.py`) the runtime mask uses — one SSOT for "what is a skill dir", asserted by an import-identity test so the guardrail and the control cannot drift — and, like CE055, is a pytest class scanning `tasks/**` with positive+negative in-memory sensors so an empty glob cannot pass silently. The fix is never to relax the mask but to move the eval def / reference OUT of the skill dir, e.g. to a sibling `tests/`, where the allowlist masks it without hiding the skill), **CE064** (in `src/coder_eval/agents/`, a module that imports `TurnClock` must pass an explicit `timestamp=` to `AgentStartEvent` and `AgentEndEvent` — the turn's OUTER bounds, which no other rule looks at, since CE058-CE061 all scope to `AssistantMessage` and the bracket is not one. `timing.decompose_turn` produces `harness_startup_ms` / `harness_teardown_ms` by subtracting a generation-window bound from a bracket timestamp, so the two must share a basis; all three clocked harnesses derived their bounds from the `TurnClock` and let the bracket fall back to `StreamEvent.timestamp`'s `default_factory=datetime.now`, putting a monotonic-derived stamp and a raw wall stamp inside one subtraction — the exact split `TurnClock` exists to remove, reintroduced at the one seam the clock did not own. Measured on a live antigravity turn: an `AgentEndEvent` stamped **17 us BEFORE its own last message finished**, which cannot happen (the event is constructed strictly after the final flush), and `decompose_turn` clamped that negative and published `0.0` — "measured, and instant", the CE058 confusion arrived at from the other direction — for a harness whose real tail is ~0.1 ms; it now records 0.035 ms. It surfaced on one harness only because the drift is tens of microseconds and antigravity is the only one that holds its process across turns, so nothing happens between its last flush and its end event; every other harness books a tail of 7-543 ms, where the drift is invisible rather than absent — which is why the fix is at every clocked site rather than at that one. SCOPE IS DERIVED, never a harness list: codex and opencode take their spans from the CLI's own epoch stamps, deliberately have no `TurnClock`, and are correctly invisible to the rule — a raw bracket is CONSISTENT with their bounds — and the day either adopts a clock the rule starts applying with no edit. BLIND SPOT, in the rule's docstring: presence, not correctness. It cannot tell `self.clock.now()` from a `datetime.now()` spelled out at the call site, because the three harnesses legitimately reach their clock three ways; the guard for the SOURCE is behavioural (`tests/_bracket_clock.py` injects a stand-in anchored a year from real time, so a reverted argument fails by a year rather than by the microseconds that separate the two clocks), which is the division of labour CE060 states — a rule removes the SILENT case, a default nobody chose), **CE063** (no module in `src/coder_eval/agents/` may import `busy_ms` — tool execution comes out of a generation window in exactly ONE place, `timing.py::subtract_tool_time`. Five reducers used to do it themselves while the head and tail were already computed centrally at the same seam, and that asymmetry is where every timing defect on this branch lived — none of them in the arithmetic, all of them in the bookkeeping AROUND it: when to reset a per-step span list (clearing it at `step_start` wiped a span before the flush could subtract it, a 100% overstatement of that window), when to clear a spent start stamp (a second flush with no intervening start republished the previous span — 3000 ms of generation for a 2000 ms turn), when to advance the mark. A sixth harness reaching for `busy_ms` rebuilds that, and its tool time is then subtracted TWICE — by the reducer and again by the collector — under-reporting generation on one harness only, which takes a corpus comparison to notice. A separate id from CE061 rather than a rebody: CE061 asks where a window's ARITHMETIC came from and four reducers still call `close_window`, so its property is live and unsuperseded; this asks whether a reducer subtracts at all. It deliberately does NOT reuse CE061's `_imports_the_helper`, whose bare-module-import branch exists so `timing.close_window(...)` counts as reaching the helper — inverted into a ban that branch flags four of the five reducers. CE061 is now **exemption-free**: claude-code was its one permanent `# noqa` and, with the subtraction moved, calls the shrunken `close_window` like the other four), **CE060** (in `src/coder_eval/agents/`, every `AssistantMessage(...)` must pass `message_id` explicitly — an identity invariant, which is why it is its own id rather than a second arm of CE058/CE059, both of which are about timing. Antigravity omitted the kwarg, so the field defaulted to `None` on every message it ever recorded, and the evalboard — which groups assistant emissions by `message_id` and falls back to a `SAME_EMISSION_GAP_MS` wall-clock gap when either side lacks one — collapsed a whole turn's generations into ONE timeline row as soon as the harness's generation windows became contiguous (the gap is then exactly 0 ms, always). Nothing failed: the consumer SUMS the group, so the totals and the reconciliation invariant stayed right, and the golden snapshots had ratified the `null` on the day they were written — a snapshot is regenerated from whatever the code currently does, so it catches a later change and never an initial omission. The damage was not confined to the timeline, which is why "only granularity is lost" was the wrong way to describe it: a grouped emission is one API call to the evalboard's thinking-cost simulator, whose prompt-cache cascade is quadratic in that count, so a single-shot Antigravity run had every cascade coefficient pinned at zero; the `Messages` count and the 10 s slow-generation bar were per-turn too. Unlike its two siblings it **derives its constructor set from each module's own `coder_eval.models` imports** instead of hardcoding the spelling, which closes exactly the blind spot CE058's clause below concedes: `claude_code_agent.py` binds only `AssistantMessage as AssistantMessageTelemetry`, so a name list guards that file's two construction sites purely by coincidence, and an arbitrary `as Msg` is missed outright. Widening CE058/CE059 the same way is recorded in `.claude/harness-candidates.md`. BLIND SPOT, in the rule's docstring: the runtime `None` — the kwarg must be PRESENT, not statically non-`None`, because OpenCode's `messageID` and Pi's `responseId` legitimately evaluate to `None` when the CLI omits them, and passing a fallback expression *is* deciding), **CE058** (in `src/coder_eval/`, an unknown timing value may not become a numeric literal — `duration_ms is None` means *never timed* and `0.0` means *timed and instant*, so writing the literal publishes the second while meaning the first. One invariant, one id, five syntactic forms — a zero constructor keyword, `x or 0`, `x if x is not None else 0.0`, `if x.duration_ms is None: x.duration_ms = 0.0`, and a `model_copy(update={...})` dict (the shape the Antigravity DONE path writes through, which a keyword-only rule cannot see). Antigravity constructed EVERY message with `generation_duration_ms=0.0`, so the task page's Generation cell read `0ms` and its breakdown rendered `0%` for months with nothing failing; Codex published the SDK's `0.0` as a measured command duration, so `avg_command_time_ms` divided real milliseconds by a command count of which 70 of 211 in one nightly had never been timed. The fourth form is the one no existing rule shape covered and is where a live instance was hiding — `claude_code_agent._finalize_commands` set `0.0` on every command force-closed without a tool result, in the one harness a timing audit had called healthy. BLIND SPOT, stated in the rule's docstring: form 1 keys on the callee's spelling, so renaming the `AssistantMessageTelemetry` import alias silently disarms it there), **CE059** (in `src/coder_eval/agents/`, an `AssistantMessage` may not receive the same `ast.Name` for both `started_at` and `completed_at` — the Antigravity reducer read `datetime.now()` once and passed it as both bounds, so `started_at == completed_at` on 368 of 368 sampled messages. A separate id from CE058 because it is a separate invariant, a zero-length window whatever the duration field says, and one invariant per id is what makes a `# noqa` mean one thing. It does NOT fire when the same call passes `generation_duration_ms=None`: a call that says, in the field built to say it, that no window was measurable is not claiming one — that exemption is what keeps the rule pointed at the misleading case instead of accumulating four permanent suppressions on the rollout-rebuild and sub-agent-synthesis sites), **CE056** (no bare `CODER_EVAL_IN_CONTAINER` literal outside `models/container_paths.py` — the CE053 shape again: a rename-safety constant that shipped beside the literal it replaced, and the straggler was the single WRITER, so a rename would have disarmed four security/correctness gates at once with nothing failing; CE052 cannot catch it because that rule inspects `if` guards and the writer is not one), **CE055** (a criterion `path:` in `tasks/` must be sandbox-relative — an absolute path is joined onto the sandbox root, which DISCARDS the root, so containment refuses it and the criterion can never match whatever the agent does; two in-tree tasks were broken this way and the pair is the argument for a static rule on top of the runtime `CheckerMisuseError`: `byod_smoke_test` IS in a CI bucket and produced only `Results: 7/8 succeeded` plus a gating 0.0 reading "file does not exist" for a file that existed, while `dockerfile_build_example` is in NO bucket, so nothing ran it and no runtime guard was ever reached — the fix is never to relax containment but to say what the criterion means, `run_command: test -f /opt/marker`, a claim about the container IMAGE rather than about the agent's workspace), **CE054** (an `environment_info` key that is READ must be WRITTEN somewhere in `src/` — the bag is `dict[str, Any]`, so nothing connects reader to writer, and the `reference_digest` anti-cheat guard shipped as a read with no writer anywhere: `.get()` returned `None`, the guard took its early return, and CLAUDE.md plus the user guide both described it as protection it never provided), **CE048** (never call a Typer command function in process — its parameter defaults are `OptionInfo` sentinels, not values, and the sentinel is TRUTHY, so `in_place=None` silently selected the wrong branch; the fix is the `run_pipeline` / `run_evaluation` / `run_plan` split, and this rule is the one that also scans `tests/`, since that is the only place the defect occurs), **CE049** (never coalesce a possibly-unmeasured score to a numeric literal — `score or 0.0` publishes "measured and scored zero" while meaning "never measured", which is how an ungraded night reached four unfiltered `avg(Score)` dashboards as a real zero), **CE050** (no untyped `getattr` probe for a discriminated-union field — pyright cannot see the string, so a rename degrades the guard to a permanent no-op; scoped to criterion-shaped receivers because `command`/`tool`/`prompt` are far too common to flag on their own), **CE051** (a sandbox driver may not be rewritten silently — the driver IS the isolation boundary, so a downgrade must be an explicit, stamped, operator-visible decision), **CE053** (no bare run-record or run-LOG filename literal outside `path_utils` — widened to `docker.log` / `grade.docker.log` / `task.log` / `grade.log` after the same shape recurred: `docker.log` was produced in `isolation/` and consumed in `orchestration/` as three unrelated literals, and because the consumer guards its copy with `is_file()`, a rename would have silently discarded the only record of why a grading container failed — `TASK_JSON_FILENAME` shipped with a rename-safety rationale while twelve exact literals stayed unmigrated, including all three `rglob("task.json")` sites the constant's own comment cites as its reason to exist, so it created the second source of truth it argues against), **CE052** (an `os._exit` must sit inside a branch testing `CODER_EVAL_IN_CONTAINER` — it is the right primitive only for reaping the container's own disposable main process, and `run_task_internal_command` armed its heartbeat watchdog, a daemon thread whose whole authority is `os._exit(137)`, unconditionally: a test that invoked the command in-process left the pytest worker holding that thread, which exited the worker 40s later inside an unrelated test file, naming a different test on each run and on each platform with no traceback — and the dead worker's lost coverage data then failed the gate as `65.13 < 80.00`, naming neither the test nor the cause), **CE037** (no unreferenced module-level private helper in `src/` — a helper whose docstring documents a bug the live code still has is worse than none), **CE038** (in an `@asynccontextmanager`, the acquire must sit INSIDE the `try` whose `finally` releases it — `asyncio.shield` protects the inner task, NOT the await, so a cancel on `__aenter__` skips the unwind while the work completes), **CE039** (a criterion checker must not return a gating `score=0.0` from an `except OSError` over a path the *task author* named — that books an eval-config error as an agent failure; raise `CheckerMisuseError` instead, and `# noqa: CE039` the cases that really are the agent's), **CE047** (every onboarding/marketing surface — README, `docs/index.md`, `docs/comparison.md`, `docs/llms.txt`, `mkdocs.yml`'s `site_description`, the Pages stub, and pyproject's `description`/`keywords` — must name every built-in `AgentKind`; OpenCode shipped while four of those seven still listed three harnesses, and nothing failed), **CE057** (a module copied into the recorder directory beside a generated sandbox shim — `models.sandbox.SIDECAR_MODULES`, currently `argv_match.py` — may import stdlib only. The failure is silent: the sidecar runs where `coder_eval` and its dependencies are not installed, so one package import makes every shadowed CLI die with an ImportError the agent reads as "the tool is broken", costing a whole run to diagnose. The rule derives its target set from that exported tuple and a test asserts it matches a file that exists — a lint rule guarding zero files must fail, not pass). When fixing a bug, ask: *could a custom lint rule have prevented this?* If the root cause is a mechanically detectable pattern (e.g., "always import from `coder_eval.models`", "never call blocking IO in async"), add a rule to `tests/lint/rules/` following the CE001+ pattern and wire it up in `tests/lint/runner.py`. This turns a one-time fix into permanent enforcement. See `tests/test_custom_lint.py` for how rules are tested. (Doc-surface / whole-tree rules that reason over Markdown/YAML or the entire `src/` tree rather than one `.py` AST at a time — CE026–CE031, CE033–CE036 — are not `BaseRule`s in the runner; they are wired as dedicated `@pytest.mark.lint` test classes. CE036 enforces the `live_verdict` determinism + monotonicity contract (`criteria/base.py`) that `EarlyStopWatcher`'s latching, deferred fail-stop, and flip-attribution silently depend on: monotonicity over arbitrary Python is undecidable, so instead of a static check it REPLAYS each live criterion against every prefix of recorded trajectories (`tests/lint/live_verdict_contract.py::CASES`) — on the authored ordering AND under seeded shuffles (`permuted_violations`, which catch order-sensitive bugs the authored walk misses) — and asserts the property directly, plus registry-derived coverage — every `LiveSuccessCriterion` in the union must have cases, and every polarity its instances claim via `live_decidable_polarities()` must actually be reached by one (otherwise a single always-`undecided` fixture would "cover" a type while proving nothing). Adding a live criterion therefore means adding `ContractCase`s in the same change. CE035 resolves every `steps..outputs.` / `needs..outputs.` reference in `.github/workflows/**` to a writer that actually produces that key — GitHub expands an unwritten output to the empty string, so a typo degrades a gate silently and actionlint models `steps.*.outputs` as an open string map. CE034 scans `tasks/` and forces an armed, live-*passable* `command_executed` to set `require_success` — a crashed invocation would otherwise latch a live PASS, fire `on_pass: stop`, and let FIRED-ONLY armed gating report SUCCESS without ever consulting the unarmed criteria (negative assertions are fail-only and are exempt). CE033 keeps the plugin's bundled `reference/criteria.md` in parity with the `SuccessCriterion` union that generates it (`make plugin-reference` writes it; the rule re-renders and diffs — never hand-edit the file). CE031 guards against dead config: a behavior-driving field on `SimulationConfig`/`RunLimits`/`Dataset` that no code reads by name. CE026 keeps the GitHub Action's onboarding surfaces honest — `README.md`, `docs/CI_GATE.md`, `docs/tutorials/02-ci-pipeline.md`, and the plugin's `ci` skill, whose emitted workflow users copy into their own repos: a page's *first* Action snippet must show the agent-runtime prerequisite steps (pinned to the `action-dogfood` job that proves them in CI), a zero-install absolute next to such a snippet must name the channel it means, every `github.com/marketplace/actions/` link plus the shields badge label must match `action.yml`'s `name:`, and every `with:` key on a snippet's action step must be a real `action.yml` input (GitHub ignores unknown inputs, so a rename would silently degrade every copied workflow). Renaming an action input or changing its runtime prerequisites therefore means updating the skill too.) diff --git a/docs/DOCKER_ISOLATION.md b/docs/DOCKER_ISOLATION.md index 14850092..a2eef133 100644 --- a/docs/DOCKER_ISOLATION.md +++ b/docs/DOCKER_ISOLATION.md @@ -337,6 +337,39 @@ If the task declares a `reference:` block, a throwaway **copy** of its directory See [Reference Solutions](TASK_DEFINITION_GUIDE.md#reference-solutions). +### Two more passive-read anti-cheat blocks + +These are `driver: docker` only. `driver: tempdir` shares the host uid and has no +filesystem isolation, so neither applies there (nor can — there is nothing to +mask). Both are defense-in-depth passive-read blocks, consistent with the +reference window's posture above; neither contains an adversarial agent. + +- **The staged task definition is deleted after load.** The host stages the + post-override `TaskDefinition` (with `success_criteria`) at `/work/input/task.yaml` + for the in-container orchestrator to load once at startup. The agent runs in the + same container, so leaving that file readable would hand it the grading answer + key. The in-container entry point deletes it immediately after `load_task` + (gated on `CODER_EVAL_IN_CONTAINER`). It is read exactly once — grading reads + criteria from the in-memory task, never from disk. The `/work/input` mount is + therefore read-write (a `:ro` mount rejects `rm` with EROFS). `context.json` + and `prior.json` are kept: they carry no criteria and `prior.json` is read + later on the regrade path. + +- **Auto-mounted plugin trees are default-deny masked.** An `agent.plugins[].path` + (or a `TemplateDirSource.path` that is itself a plugin root) is auto-mounted at + its host path `:ro` so the plugin loads. Eval material colocated under that tree + as siblings of the skills dir — sibling task YAMLs, reference solutions, test + fixtures — would otherwise be readable. So the runner keeps the whole root + mounted but layers an empty `--tmpfs` over every child dir OUTSIDE the keep-set + (`.claude-plugin` + the manifest-declared skill dirs). Everything that is not + the plugin surface is masked by default, so an unknown or new eval layout can + never leak; `tests/`, `node_modules/`, and reference solutions are masked for + free. A root agent cannot `umount` a tmpfs (`CAP_SYS_ADMIN` is not in Docker's + default set), so this mask is *stronger* than the mode-000 reference window. The + one residual — an eval def or reference COLOCATED inside a skill dir, which + cannot be masked without hiding the skill — is caught by lint rule CE065 (keep + eval material out of skill dirs; put it under a sibling `tests/`). + Inside the container, the entrypoint invokes `coder-eval _run-task-internal` (hidden subcommand), which loads the staged YAML + context, runs the standard in-process Orchestrator (driver auto-coerced back to `tempdir`), and writes `task.json` to the output mount. Host reads it and feeds the existing aggregation pipeline. A `result_kind` discriminator on `CriterionResult` ensures `ClassificationCriterionResult` subclasses survive the JSON round-trip — without it, host-side aggregation would silently lose `observed_label`/`expected_label`. diff --git a/tests/test_custom_lint.py b/tests/test_custom_lint.py index d158a1ad..76a5a153 100644 --- a/tests/test_custom_lint.py +++ b/tests/test_custom_lint.py @@ -4980,3 +4980,181 @@ def test_is_suppressible(self, tmp_path): encoding="utf-8", ) assert not check_file(target, [TurnBracketOnTheClock]) + + +class TestCE065NoEvalMaterialInsideSkillDir: + """CE065 — no eval definition / reference dir INSIDE a skill dir. + + The Fix B allowlist auto-masks eval material anywhere under an auto-mounted + plugin root EXCEPT inside a kept skill dir — masking those would hide the + skill itself. So a `task_id:`-bearing YAML (or a resolved `reference.directory`) + colocated INSIDE a skill dir is the one leak the runtime allowlist cannot + close: under `driver: docker` the agent reads its own grading answer key + straight out of the readable skill surface. + + This static rule flags that layout in-repo so it can never recur silently. + It reuses the SAME `manifest_skill_dirs` resolver the runtime allowlist uses + (one SSOT for "what is a skill dir"). The fix is to move the eval def / + reference OUT of the skill dir (e.g. to a sibling `tests/`), where the + allowlist masks it without hiding the skill. + """ + + ROOT = Path(__file__).parent.parent + + @staticmethod + def _skill_dirs_for_task(task, task_file: Path) -> list[Path]: + """In-repo skill dirs reachable from a task's plugin / template roots. + + Plugin roots (`agent.plugins[].path`) and `TemplateDirSource.path` roots + are resolved relative to the task file's dir (mirroring + `resolve_host_reference_dir`), then each plugin root's skill dirs come + from the shared `manifest_skill_dirs` resolver. + """ + import os + + from coder_eval.agents._skills import manifest_skill_dirs + from coder_eval.models import TemplateDirSource + + roots: list[Path] = [] + agent = task.agent + for plugin in (agent.plugins if agent else None) or []: + raw = plugin.get("path") if isinstance(plugin, dict) else None + if not raw: + continue + expanded = Path(os.path.expandvars(os.path.expanduser(str(raw)))) + root = expanded if expanded.is_absolute() else (task_file.parent / expanded) + roots.append(root.resolve()) + sandbox = task.sandbox + for source in (sandbox.template_sources if sandbox else None) or []: + if not isinstance(source, TemplateDirSource): + continue + expanded = Path(os.path.expandvars(os.path.expanduser(str(source.path)))) + root = expanded if expanded.is_absolute() else (task_file.parent / expanded) + roots.append(root.resolve()) + + skill_dirs: list[Path] = [] + for root in roots: + # Only a real plugin root declares skills; a plain template dir has none. + if not (root / ".claude-plugin" / "plugin.json").is_file(): + continue + skill_dirs.extend(manifest_skill_dirs(root)) + return skill_dirs + + @classmethod + def _offenders(cls, task, task_file: Path) -> list[str]: + from coder_eval.orchestration.evaluation import resolve_host_reference_dir + + skill_dirs = cls._skill_dirs_for_task(task, task_file) + if not skill_dirs: + return [] + + offenders: list[str] = [] + + # A resolved reference dir colocated inside a skill dir. + ref_dir = resolve_host_reference_dir(task, task_file) + if ref_dir is not None: + for skill in skill_dirs: + if ref_dir == skill or skill in ref_dir.parents: + offenders.append(f"reference.directory -> {ref_dir}") + break + + # A `task_id:`-bearing YAML inside a skill dir. + for skill in skill_dirs: + if not skill.is_dir(): + continue + for yaml_file in skill.rglob("*.yaml"): + if yaml_file.name == "metadata.yaml": + continue + try: + text = yaml_file.read_text(encoding="utf-8") + except OSError: + continue + # Cheap key test: a task definition declares a top-level task_id. + if re.search(r"(?m)^task_id\s*:", text): + offenders.append(f"task def inside skill dir -> {yaml_file}") + return offenders + + @pytest.mark.parametrize( + "path", + sorted(p for p in (Path(__file__).parent.parent / "tasks").rglob("*.yaml") if p.name != "metadata.yaml"), + ids=lambda p: p.relative_to(Path(__file__).parent.parent).as_posix(), + ) + def test_repo_tasks_keep_eval_material_out_of_skill_dirs(self, path: Path): + from coder_eval.orchestration.task_loader import load_task + + task, _ = load_task(path) + offenders = self._offenders(task, path) + assert not offenders, ( + f"{path}: {offenders} live inside a skill dir under an auto-mounted plugin/template root. " + "The Fix B allowlist keeps skill dirs readable, so it cannot mask eval material there — the " + "agent under `driver: docker` reads its own grading answer key. Move the eval def / reference " + "OUT of the skill dir (e.g. to a sibling `tests/`), where the allowlist masks it." + ) + + def _synthetic_plugin_task(self, tmp_path: Path, *, place_task_inside_skill: bool): + from coder_eval.models import TaskDefinition + + plugin_root = tmp_path / "plugin" + (plugin_root / ".claude-plugin").mkdir(parents=True) + (plugin_root / ".claude-plugin" / "plugin.json").write_text(json.dumps({"name": "demo"}), encoding="utf-8") + skill = plugin_root / "skills" / "demo" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("# skill", encoding="utf-8") + + if place_task_inside_skill: + (skill / "leak.yaml").write_text("task_id: leaked\n", encoding="utf-8") + else: + sibling = plugin_root / "tests" / "tasks" + sibling.mkdir(parents=True) + (sibling / "ok.yaml").write_text("task_id: fine\n", encoding="utf-8") + + task_file = tmp_path / "task.yaml" + task_file.write_text("# task", encoding="utf-8") + task = TaskDefinition( + task_id="host", + description="d", + initial_prompt="p", + agent={"type": "claude-code", "plugins": [{"type": "local", "path": str(plugin_root)}]}, + success_criteria=[], + ) + return task, task_file + + def test_detects_a_task_def_inside_a_skill_dir(self, tmp_path): + """Positive sensor — without it an empty `tasks/` glob would 'pass'.""" + task, task_file = self._synthetic_plugin_task(tmp_path, place_task_inside_skill=True) + offenders = self._offenders(task, task_file) + assert offenders and any("skill dir" in o for o in offenders), offenders + + def test_allows_eval_material_outside_skill_dirs(self, tmp_path): + """Negative sensor — a task def under a sibling `tests/` is masked at runtime.""" + task, task_file = self._synthetic_plugin_task(tmp_path, place_task_inside_skill=False) + assert self._offenders(task, task_file) == [] + + def test_detects_a_reference_dir_inside_a_skill_dir(self, tmp_path): + from coder_eval.models import ReferenceSource, TaskDefinition + + plugin_root = tmp_path / "plugin" + (plugin_root / ".claude-plugin").mkdir(parents=True) + (plugin_root / ".claude-plugin" / "plugin.json").write_text(json.dumps({"name": "demo"}), encoding="utf-8") + ref = plugin_root / "skills" / "demo" / "solution" + ref.mkdir(parents=True) + + task_file = tmp_path / "task.yaml" + task_file.write_text("# task", encoding="utf-8") + task = TaskDefinition( + task_id="host", + description="d", + initial_prompt="p", + agent={"type": "claude-code", "plugins": [{"type": "local", "path": str(plugin_root)}]}, + reference=ReferenceSource(directory="plugin/skills/demo/solution"), + success_criteria=[], + ) + offenders = self._offenders(task, task_file) + assert offenders and any("reference" in o for o in offenders), offenders + + def test_shares_manifest_skill_dirs_with_the_runtime_allowlist(self): + """SSOT: CE065 and Fix B (eval_material.mask_dirs) must agree on skill dirs.""" + from coder_eval.agents._skills import manifest_skill_dirs + from coder_eval.isolation import eval_material + + assert eval_material.manifest_skill_dirs is manifest_skill_dirs From dc71add6cdc54f988635b0c139afb94a7c81f1bd Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Mon, 14 Sep 2026 18:00:17 +0100 Subject: [PATCH 4/7] fix: also delete context.json in-container (source_yaml leaked criteria) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review found Fix A closed only /work/input/task.yaml but left the identical success_criteria readable in /work/input/context.json via source_yaml (top-level AND inside every config_lineage entry / ConfigLineageEntry.source_yaml) — in a dir this change makes world-readable+writable. The agent could cat context.json to recover its grading answer key. Delete both staged files after they are read into memory (_scrub_staged_task_yaml -> _scrub_staged_inputs); correct the docstring and docs/DOCKER_ISOLATION.md (the 'context.json carries no criteria' claim was false). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/DOCKER_ISOLATION.md | 22 ++++---- .../cli/run_task_internal_command.py | 48 +++++++++++------ src/coder_eval/isolation/docker_runner.py | 5 +- tests/test_run_task_internal.py | 53 ++++++++++++------- 4 files changed, 82 insertions(+), 46 deletions(-) diff --git a/docs/DOCKER_ISOLATION.md b/docs/DOCKER_ISOLATION.md index a2eef133..1891ea73 100644 --- a/docs/DOCKER_ISOLATION.md +++ b/docs/DOCKER_ISOLATION.md @@ -344,16 +344,20 @@ filesystem isolation, so neither applies there (nor can — there is nothing to mask). Both are defense-in-depth passive-read blocks, consistent with the reference window's posture above; neither contains an adversarial agent. -- **The staged task definition is deleted after load.** The host stages the +- **The staged grading inputs are deleted after load.** The host stages the post-override `TaskDefinition` (with `success_criteria`) at `/work/input/task.yaml` - for the in-container orchestrator to load once at startup. The agent runs in the - same container, so leaving that file readable would hand it the grading answer - key. The in-container entry point deletes it immediately after `load_task` - (gated on `CODER_EVAL_IN_CONTAINER`). It is read exactly once — grading reads - criteria from the in-memory task, never from disk. The `/work/input` mount is - therefore read-write (a `:ro` mount rejects `rm` with EROFS). `context.json` - and `prior.json` are kept: they carry no criteria and `prior.json` is read - later on the regrade path. + **and** a `context.json` whose `source_yaml` is the raw task text — criteria + verbatim, both at the top level and inside every `config_lineage` entry — for the + in-container orchestrator to load once at startup. The agent runs in the same + container, so leaving *either* readable would hand it the grading answer key + (deleting only `task.yaml` leaves the identical criteria one file over in + `context.json`). The in-container entry point deletes **both** immediately after + they are consumed — `context.json` is parsed into memory in the command body and + `task.yaml` by `load_task`, both before the delete (gated on + `CODER_EVAL_IN_CONTAINER`). They are read exactly once — grading reads criteria + from the in-memory task, never from disk. The `/work/input` mount is therefore + read-write (a `:ro` mount rejects `rm` with EROFS). `prior.json` is kept: it is + read later on the regrade path, and a regrade runs no agent so it is not a leak. - **Auto-mounted plugin trees are default-deny masked.** An `agent.plugins[].path` (or a `TemplateDirSource.path` that is itself a plugin root) is auto-mounted at diff --git a/src/coder_eval/cli/run_task_internal_command.py b/src/coder_eval/cli/run_task_internal_command.py index 145974da..6ba56d7a 100644 --- a/src/coder_eval/cli/run_task_internal_command.py +++ b/src/coder_eval/cli/run_task_internal_command.py @@ -47,24 +47,35 @@ logger = logging.getLogger(__name__) -def _scrub_staged_task_yaml(task_yaml: Path) -> None: - """Delete the staged ``task.yaml`` after it has been loaded (anti-cheat). +def _scrub_staged_inputs(task_yaml: Path, context_json: Path) -> None: + """Delete the staged grading inputs after they are loaded (anti-cheat). - The post-override :class:`TaskDefinition` (``success_criteria`` included) was - staged at ``/work/input`` for THIS load only. The agent runs in this same - container; leaving the file readable hands it the grading answer key. It is - read exactly once (by ``load_task``) -- grading reads criteria from the - in-memory task, never from disk -- so delete it now. + ``/work/input`` holds BOTH copies of the grading answer key: + + * ``task.yaml`` -- the post-override :class:`TaskDefinition`, ``success_criteria`` + included. + * ``context.json`` -- whose ``source_yaml`` is the RAW task YAML *text* + (``success_criteria`` verbatim), present at the top level AND inside every + ``config_lineage`` entry (``ConfigLineageEntry.source_yaml``). Deleting only + ``task.yaml`` would leave the identical criteria one file over -- and this + driver makes ``/work/input`` readable+writable, so the agent in this same + container could ``cat /work/input/context.json`` to recover them. + + Both are read exactly once at startup (``context.json`` into memory in the + command body before this call; ``task.yaml`` by ``load_task``) and never again + during the agent turn or grading -- grading reads criteria from the in-memory + task, never from disk -- so delete both now. Gated on ``IN_CONTAINER_ENV``: docker-only by construction (a host/tempdir - invocation shares our uid and has no filesystem isolation, so there is - nothing to protect and nothing to delete). ``missing_ok`` so a re-entrant or - host call never crashes on an absent file. ``context.json``/``prior.json`` - are deliberately NOT deleted -- they carry no criteria answer key and - ``prior.json`` is read after this point on the regrade path. + invocation shares our uid and has no filesystem isolation, so there is nothing + to protect and nothing to delete). ``missing_ok`` so a re-entrant or host call + never crashes on an absent file. ``prior.json`` is deliberately left in place: + it is read after this point on the regrade path, and a regrade runs no agent, + so it is not a leak. """ if os.environ.get(IN_CONTAINER_ENV) == "1": task_yaml.unlink(missing_ok=True) + context_json.unlink(missing_ok=True) def heartbeat_is_alive(current: str, last_counter: str, current_mtime: float, last_mtime: float) -> bool: @@ -269,11 +280,14 @@ def run_task_internal_command( # `TASK_DIR` env exposed to `run_command` criteria -- resolves to the # original host task directory rather than `/work/input/`. task, source_yaml = load_task(task_yaml) - # ANTI-CHEAT: load_task is task.yaml's sole reader (both the normal and the - # regrade paths run it before their dispatch, and prior.json is read INSIDE - # _grade_recorded_run, strictly after this point). Delete it now so the agent - # in this same container cannot read its own grading criteria back. - _scrub_staged_task_yaml(task_yaml) + # ANTI-CHEAT: both task.yaml and context.json have been fully consumed by this + # point -- context.json was parsed into memory above (source_yaml/config_lineage + # already extracted into `context`), and load_task is task.yaml's sole reader. + # Both the normal and the regrade paths reach here before their dispatch, and + # prior.json is read INSIDE _grade_recorded_run (strictly after this). Delete + # both staged files now so the agent in this same container cannot read its own + # grading criteria back -- from task.yaml OR from context.json's source_yaml. + _scrub_staged_inputs(task_yaml, context_json) if host_source_yaml is not None: source_yaml = host_source_yaml # The path below is never re-read; it only seeds Orchestrator's TASK_DIR. diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index b58d26e8..c1f5039a 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -1658,8 +1658,9 @@ def _build_argv( argv += ["--env", "TELEMETRY_ENABLED=false"] # Read-WRITE, not `:ro`: the in-container entry point deletes the staged - # task.yaml right after loading it (ANTI-CHEAT -- see the grant above and - # run_task_internal_command._scrub_staged_task_yaml), and both `rm` and + # task.yaml AND context.json right after loading them (ANTI-CHEAT -- both + # carry success_criteria; see the grant above and + # run_task_internal_command._scrub_staged_inputs), and both `rm` and # `chmod` fail with EROFS on a `:ro` bind mount. The host destroys the # whole staging tree in run()'s finally, so nothing is stranded. argv += ["-v", f"{input_dir.resolve()}:{CONTAINER_INPUT_DIR}"] diff --git a/tests/test_run_task_internal.py b/tests/test_run_task_internal.py index 1807aec9..271fcc48 100644 --- a/tests/test_run_task_internal.py +++ b/tests/test_run_task_internal.py @@ -2,57 +2,74 @@ CE048: the command itself is a Typer command whose parameter defaults are `OptionInfo` sentinels, so it must never be invoked in-process. These tests -target the extracted, pure `_scrub_staged_task_yaml` helper instead. +target the extracted, pure `_scrub_staged_inputs` helper instead. """ from __future__ import annotations from pathlib import Path -from coder_eval.cli.run_task_internal_command import _scrub_staged_task_yaml +from coder_eval.cli.run_task_internal_command import _scrub_staged_inputs from coder_eval.models import IN_CONTAINER_ENV -class TestScrubStagedTaskYaml: - """Anti-cheat: the staged task.yaml (with success_criteria) is deleted after - load so the in-container agent cannot read its own grading answer key.""" +def _stage(tmp_path: Path) -> tuple[Path, Path]: + """Write a staged task.yaml + context.json, both carrying the criteria.""" + task_yaml = tmp_path / "task.yaml" + task_yaml.write_text("task_id: x\nsuccess_criteria: []\n", encoding="utf-8") + context_json = tmp_path / "context.json" + # source_yaml is the raw task text (criteria verbatim) -- the second copy the + # scrub must remove, at the top level and inside config_lineage. + context_json.write_text( + '{"variant_id": "v", "source_yaml": "task_id: x\\nsuccess_criteria: [SECRET]\\n"}', + encoding="utf-8", + ) + return task_yaml, context_json - def test_deletes_when_in_container(self, tmp_path: Path, monkeypatch): + +class TestScrubStagedInputs: + """Anti-cheat: BOTH staged grading-answer-key copies (task.yaml's criteria AND + context.json's source_yaml) are deleted after load so the in-container agent + cannot read its own criteria back from either file.""" + + def test_deletes_both_when_in_container(self, tmp_path: Path, monkeypatch): monkeypatch.setenv(IN_CONTAINER_ENV, "1") - task_yaml = tmp_path / "task.yaml" - task_yaml.write_text("task_id: x\nsuccess_criteria: []\n", encoding="utf-8") + task_yaml, context_json = _stage(tmp_path) - _scrub_staged_task_yaml(task_yaml) + _scrub_staged_inputs(task_yaml, context_json) assert not task_yaml.exists() + assert not context_json.exists(), "context.json (source_yaml has criteria) must be deleted too" def test_survives_when_not_in_container(self, tmp_path: Path, monkeypatch): # A host/tempdir invocation shares our uid and has no filesystem # isolation; the gate keeps the delete docker-only by construction. monkeypatch.delenv(IN_CONTAINER_ENV, raising=False) - task_yaml = tmp_path / "task.yaml" - task_yaml.write_text("task_id: x\n", encoding="utf-8") + task_yaml, context_json = _stage(tmp_path) - _scrub_staged_task_yaml(task_yaml) + _scrub_staged_inputs(task_yaml, context_json) assert task_yaml.exists() + assert context_json.exists() - def test_env_var_set_but_not_one_leaves_file(self, tmp_path: Path, monkeypatch): + def test_env_var_set_but_not_one_leaves_files(self, tmp_path: Path, monkeypatch): # Only the exact "1" sentinel arms the delete, mirroring the reference # window gate (Sandbox.enforces_permission_windows). monkeypatch.setenv(IN_CONTAINER_ENV, "0") - task_yaml = tmp_path / "task.yaml" - task_yaml.write_text("task_id: x\n", encoding="utf-8") + task_yaml, context_json = _stage(tmp_path) - _scrub_staged_task_yaml(task_yaml) + _scrub_staged_inputs(task_yaml, context_json) assert task_yaml.exists() + assert context_json.exists() - def test_missing_file_does_not_raise(self, tmp_path: Path, monkeypatch): + def test_missing_files_do_not_raise(self, tmp_path: Path, monkeypatch): monkeypatch.setenv(IN_CONTAINER_ENV, "1") task_yaml = tmp_path / "does-not-exist.yaml" + context_json = tmp_path / "also-missing.json" # missing_ok=True: a re-entrant call must not crash on an absent file. - _scrub_staged_task_yaml(task_yaml) + _scrub_staged_inputs(task_yaml, context_json) assert not task_yaml.exists() + assert not context_json.exists() From d10778d35da6f6bbad0023156381b6229094a99c Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Mon, 14 Sep 2026 18:07:30 +0100 Subject: [PATCH 5/7] docs(harness): log deferred Low findings from anti-cheat review Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/harness-candidates.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/.claude/harness-candidates.md b/.claude/harness-candidates.md index fcfb02b4..bd347653 100644 --- a/.claude/harness-candidates.md +++ b/.claude/harness-candidates.md @@ -873,3 +873,26 @@ re-derive from scratch. timing work (the function is zero lines of its diff) and not a guardrail candidate — a small real bug needing its own change. Caught in: the turn-timing consolidation final review (gpt-5.6-sol). + +## docker anti-cheat auto-mount allowlist (fix/docker-anti-cheat-leaks) — deferred Low findings + +Surfaced by the code review of the Fix A/B/C branch. All Low; the Critical (context.json +source_yaml leak) was fixed in-branch (commit dc71add6). These are follow-ups, each +needing its own review: + +- **L1 — loose `task_id:` YAML *file* at a plugin root is unmasked.** `eval_material.mask_dirs` + masks child *directories* only (a `--tmpfs` needs a dir mountpoint), and CE065 only scans + *inside* skill dirs — so a bare `answers.yaml`/`criteria.yaml` directly under the plugin root + falls through both. Narrow (eval material is conventionally directory-shaped; UiPath/skills has + no such loose file). Fix options: extend CE065 to also flag a `task_id:` YAML file anywhere + under a masked plugin root (catches in-repo layouts), or per-file mask via `-v /dev/null::ro`. +- **L2 — CE065 vs runtime resolve plugin paths on divergent bases.** CE065 resolves + `agent.plugins[].path` relative to the task-file dir; `docker_runner._auto_mount` resolves the + raw path vs CWD. Benign today (in-repo tasks use absolute/env-var plugin paths, and the runtime + couples the mask to the very tree it mounts, so no runtime leak escapes). Fix: absolutize + `agent.plugins[].path` in `load_task` the way `TemplateDirSource.path` already is, or share one + plugin-path resolver between CE065 and `_auto_mount`. +- **L3 — manifest `skills: "."` collapses the keep-set.** If a manifest declares its skills dir as + the root, `mask_dirs` descends `root` and masks every child except `.claude-plugin` — denial, not + a leak (consistent with fail-safe default-deny), but it silently breaks such a plugin. Add a guard + (don't descend `root` when it is itself a kept leaf) + a test if that layout is supported. From 49252c6248cccb7414986b1e27ba0eeff2c50114 Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Mon, 14 Sep 2026 20:21:04 +0100 Subject: [PATCH 6/7] fix(anti-cheat): apply review Low findings L1/L2/L3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L1 — CE065 now flags ANY task_id: YAML or reference dir left readable under a plugin root (not just inside a skill dir): it reuses the runtime mask_dirs (SSOT), walks the readable surface, and catches a loose task_id: YAML *file* at the plugin root too (a tmpfs masks a dir, not a single file). Renamed the class to TestCE065EvalMaterialReadableUnderPluginRoot; added a loose-root-file sensor. L2 — docker_runner resolves a relative agent.plugins[].path against the task-file dir (new _resolve_mount_path), not CWD, matching reference/template/CE065 resolution so the static rule and the runtime inspect the same tree. L3 — eval_material.mask_dirs no longer descends a kept path, so a degenerate manifest skills: '.' masks nothing (fail-safe) instead of hiding the whole skill surface; added a test. Updated CLAUDE.md CE065 entry and docs. make verify green (the one failure is a pre-existing flaky wall-clock datetime test in test_agent_telemetry, passes on rerun, unrelated to these files). Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 2 +- src/coder_eval/isolation/docker_runner.py | 17 ++- src/coder_eval/isolation/eval_material.py | 11 +- tests/test_custom_lint.py | 169 ++++++++++++---------- tests/test_eval_material.py | 10 ++ 5 files changed, 129 insertions(+), 80 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3e6ed3d6..cbc03c56 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -236,7 +236,7 @@ make plugin-reference # the plugin's bundled criteria reference from the models Editing `src/coder_eval/pricing.py` means editing `evalboard/lib/pricing.ts` too — it is a hand-copied mirror, and `evalboard/lib/__tests__/pricing-parity.test.ts` fails the build on drift in either direction. -Recent additions, each traceable to a shipped defect: **CE065** (in `tasks/`, no `task_id:`-bearing YAML and no resolved `reference.directory` may live INSIDE a skill dir reached through an in-repo `agent.plugins[].path` / `sandbox.template_sources[].path` — the ONE residual the docker anti-cheat allowlist (`isolation/eval_material.py::mask_dirs`) cannot close. Under `driver: docker` the whole plugin root is auto-mounted `:ro` so it loads, and every child dir OUTSIDE the keep-set (`.claude-plugin` + the manifest-declared skill dirs) is tmpfs-masked by default, so sibling task YAMLs / reference solutions / `tests/` are masked for free; but masking a skill dir would hide the skill, so an eval def or reference COLOCATED inside a skill dir stays readable and hands the agent under evaluation its own grading answer key. The rule reuses the SAME `manifest_skill_dirs` resolver (`agents/_skills.py`) the runtime mask uses — one SSOT for "what is a skill dir", asserted by an import-identity test so the guardrail and the control cannot drift — and, like CE055, is a pytest class scanning `tasks/**` with positive+negative in-memory sensors so an empty glob cannot pass silently. The fix is never to relax the mask but to move the eval def / reference OUT of the skill dir, e.g. to a sibling `tests/`, where the allowlist masks it without hiding the skill), **CE064** (in `src/coder_eval/agents/`, a module that imports `TurnClock` must pass an explicit `timestamp=` to `AgentStartEvent` and `AgentEndEvent` — the turn's OUTER bounds, which no other rule looks at, since CE058-CE061 all scope to `AssistantMessage` and the bracket is not one. `timing.decompose_turn` produces `harness_startup_ms` / `harness_teardown_ms` by subtracting a generation-window bound from a bracket timestamp, so the two must share a basis; all three clocked harnesses derived their bounds from the `TurnClock` and let the bracket fall back to `StreamEvent.timestamp`'s `default_factory=datetime.now`, putting a monotonic-derived stamp and a raw wall stamp inside one subtraction — the exact split `TurnClock` exists to remove, reintroduced at the one seam the clock did not own. Measured on a live antigravity turn: an `AgentEndEvent` stamped **17 us BEFORE its own last message finished**, which cannot happen (the event is constructed strictly after the final flush), and `decompose_turn` clamped that negative and published `0.0` — "measured, and instant", the CE058 confusion arrived at from the other direction — for a harness whose real tail is ~0.1 ms; it now records 0.035 ms. It surfaced on one harness only because the drift is tens of microseconds and antigravity is the only one that holds its process across turns, so nothing happens between its last flush and its end event; every other harness books a tail of 7-543 ms, where the drift is invisible rather than absent — which is why the fix is at every clocked site rather than at that one. SCOPE IS DERIVED, never a harness list: codex and opencode take their spans from the CLI's own epoch stamps, deliberately have no `TurnClock`, and are correctly invisible to the rule — a raw bracket is CONSISTENT with their bounds — and the day either adopts a clock the rule starts applying with no edit. BLIND SPOT, in the rule's docstring: presence, not correctness. It cannot tell `self.clock.now()` from a `datetime.now()` spelled out at the call site, because the three harnesses legitimately reach their clock three ways; the guard for the SOURCE is behavioural (`tests/_bracket_clock.py` injects a stand-in anchored a year from real time, so a reverted argument fails by a year rather than by the microseconds that separate the two clocks), which is the division of labour CE060 states — a rule removes the SILENT case, a default nobody chose), **CE063** (no module in `src/coder_eval/agents/` may import `busy_ms` — tool execution comes out of a generation window in exactly ONE place, `timing.py::subtract_tool_time`. Five reducers used to do it themselves while the head and tail were already computed centrally at the same seam, and that asymmetry is where every timing defect on this branch lived — none of them in the arithmetic, all of them in the bookkeeping AROUND it: when to reset a per-step span list (clearing it at `step_start` wiped a span before the flush could subtract it, a 100% overstatement of that window), when to clear a spent start stamp (a second flush with no intervening start republished the previous span — 3000 ms of generation for a 2000 ms turn), when to advance the mark. A sixth harness reaching for `busy_ms` rebuilds that, and its tool time is then subtracted TWICE — by the reducer and again by the collector — under-reporting generation on one harness only, which takes a corpus comparison to notice. A separate id from CE061 rather than a rebody: CE061 asks where a window's ARITHMETIC came from and four reducers still call `close_window`, so its property is live and unsuperseded; this asks whether a reducer subtracts at all. It deliberately does NOT reuse CE061's `_imports_the_helper`, whose bare-module-import branch exists so `timing.close_window(...)` counts as reaching the helper — inverted into a ban that branch flags four of the five reducers. CE061 is now **exemption-free**: claude-code was its one permanent `# noqa` and, with the subtraction moved, calls the shrunken `close_window` like the other four), **CE060** (in `src/coder_eval/agents/`, every `AssistantMessage(...)` must pass `message_id` explicitly — an identity invariant, which is why it is its own id rather than a second arm of CE058/CE059, both of which are about timing. Antigravity omitted the kwarg, so the field defaulted to `None` on every message it ever recorded, and the evalboard — which groups assistant emissions by `message_id` and falls back to a `SAME_EMISSION_GAP_MS` wall-clock gap when either side lacks one — collapsed a whole turn's generations into ONE timeline row as soon as the harness's generation windows became contiguous (the gap is then exactly 0 ms, always). Nothing failed: the consumer SUMS the group, so the totals and the reconciliation invariant stayed right, and the golden snapshots had ratified the `null` on the day they were written — a snapshot is regenerated from whatever the code currently does, so it catches a later change and never an initial omission. The damage was not confined to the timeline, which is why "only granularity is lost" was the wrong way to describe it: a grouped emission is one API call to the evalboard's thinking-cost simulator, whose prompt-cache cascade is quadratic in that count, so a single-shot Antigravity run had every cascade coefficient pinned at zero; the `Messages` count and the 10 s slow-generation bar were per-turn too. Unlike its two siblings it **derives its constructor set from each module's own `coder_eval.models` imports** instead of hardcoding the spelling, which closes exactly the blind spot CE058's clause below concedes: `claude_code_agent.py` binds only `AssistantMessage as AssistantMessageTelemetry`, so a name list guards that file's two construction sites purely by coincidence, and an arbitrary `as Msg` is missed outright. Widening CE058/CE059 the same way is recorded in `.claude/harness-candidates.md`. BLIND SPOT, in the rule's docstring: the runtime `None` — the kwarg must be PRESENT, not statically non-`None`, because OpenCode's `messageID` and Pi's `responseId` legitimately evaluate to `None` when the CLI omits them, and passing a fallback expression *is* deciding), **CE058** (in `src/coder_eval/`, an unknown timing value may not become a numeric literal — `duration_ms is None` means *never timed* and `0.0` means *timed and instant*, so writing the literal publishes the second while meaning the first. One invariant, one id, five syntactic forms — a zero constructor keyword, `x or 0`, `x if x is not None else 0.0`, `if x.duration_ms is None: x.duration_ms = 0.0`, and a `model_copy(update={...})` dict (the shape the Antigravity DONE path writes through, which a keyword-only rule cannot see). Antigravity constructed EVERY message with `generation_duration_ms=0.0`, so the task page's Generation cell read `0ms` and its breakdown rendered `0%` for months with nothing failing; Codex published the SDK's `0.0` as a measured command duration, so `avg_command_time_ms` divided real milliseconds by a command count of which 70 of 211 in one nightly had never been timed. The fourth form is the one no existing rule shape covered and is where a live instance was hiding — `claude_code_agent._finalize_commands` set `0.0` on every command force-closed without a tool result, in the one harness a timing audit had called healthy. BLIND SPOT, stated in the rule's docstring: form 1 keys on the callee's spelling, so renaming the `AssistantMessageTelemetry` import alias silently disarms it there), **CE059** (in `src/coder_eval/agents/`, an `AssistantMessage` may not receive the same `ast.Name` for both `started_at` and `completed_at` — the Antigravity reducer read `datetime.now()` once and passed it as both bounds, so `started_at == completed_at` on 368 of 368 sampled messages. A separate id from CE058 because it is a separate invariant, a zero-length window whatever the duration field says, and one invariant per id is what makes a `# noqa` mean one thing. It does NOT fire when the same call passes `generation_duration_ms=None`: a call that says, in the field built to say it, that no window was measurable is not claiming one — that exemption is what keeps the rule pointed at the misleading case instead of accumulating four permanent suppressions on the rollout-rebuild and sub-agent-synthesis sites), **CE056** (no bare `CODER_EVAL_IN_CONTAINER` literal outside `models/container_paths.py` — the CE053 shape again: a rename-safety constant that shipped beside the literal it replaced, and the straggler was the single WRITER, so a rename would have disarmed four security/correctness gates at once with nothing failing; CE052 cannot catch it because that rule inspects `if` guards and the writer is not one), **CE055** (a criterion `path:` in `tasks/` must be sandbox-relative — an absolute path is joined onto the sandbox root, which DISCARDS the root, so containment refuses it and the criterion can never match whatever the agent does; two in-tree tasks were broken this way and the pair is the argument for a static rule on top of the runtime `CheckerMisuseError`: `byod_smoke_test` IS in a CI bucket and produced only `Results: 7/8 succeeded` plus a gating 0.0 reading "file does not exist" for a file that existed, while `dockerfile_build_example` is in NO bucket, so nothing ran it and no runtime guard was ever reached — the fix is never to relax containment but to say what the criterion means, `run_command: test -f /opt/marker`, a claim about the container IMAGE rather than about the agent's workspace), **CE054** (an `environment_info` key that is READ must be WRITTEN somewhere in `src/` — the bag is `dict[str, Any]`, so nothing connects reader to writer, and the `reference_digest` anti-cheat guard shipped as a read with no writer anywhere: `.get()` returned `None`, the guard took its early return, and CLAUDE.md plus the user guide both described it as protection it never provided), **CE048** (never call a Typer command function in process — its parameter defaults are `OptionInfo` sentinels, not values, and the sentinel is TRUTHY, so `in_place=None` silently selected the wrong branch; the fix is the `run_pipeline` / `run_evaluation` / `run_plan` split, and this rule is the one that also scans `tests/`, since that is the only place the defect occurs), **CE049** (never coalesce a possibly-unmeasured score to a numeric literal — `score or 0.0` publishes "measured and scored zero" while meaning "never measured", which is how an ungraded night reached four unfiltered `avg(Score)` dashboards as a real zero), **CE050** (no untyped `getattr` probe for a discriminated-union field — pyright cannot see the string, so a rename degrades the guard to a permanent no-op; scoped to criterion-shaped receivers because `command`/`tool`/`prompt` are far too common to flag on their own), **CE051** (a sandbox driver may not be rewritten silently — the driver IS the isolation boundary, so a downgrade must be an explicit, stamped, operator-visible decision), **CE053** (no bare run-record or run-LOG filename literal outside `path_utils` — widened to `docker.log` / `grade.docker.log` / `task.log` / `grade.log` after the same shape recurred: `docker.log` was produced in `isolation/` and consumed in `orchestration/` as three unrelated literals, and because the consumer guards its copy with `is_file()`, a rename would have silently discarded the only record of why a grading container failed — `TASK_JSON_FILENAME` shipped with a rename-safety rationale while twelve exact literals stayed unmigrated, including all three `rglob("task.json")` sites the constant's own comment cites as its reason to exist, so it created the second source of truth it argues against), **CE052** (an `os._exit` must sit inside a branch testing `CODER_EVAL_IN_CONTAINER` — it is the right primitive only for reaping the container's own disposable main process, and `run_task_internal_command` armed its heartbeat watchdog, a daemon thread whose whole authority is `os._exit(137)`, unconditionally: a test that invoked the command in-process left the pytest worker holding that thread, which exited the worker 40s later inside an unrelated test file, naming a different test on each run and on each platform with no traceback — and the dead worker's lost coverage data then failed the gate as `65.13 < 80.00`, naming neither the test nor the cause), **CE037** (no unreferenced module-level private helper in `src/` — a helper whose docstring documents a bug the live code still has is worse than none), **CE038** (in an `@asynccontextmanager`, the acquire must sit INSIDE the `try` whose `finally` releases it — `asyncio.shield` protects the inner task, NOT the await, so a cancel on `__aenter__` skips the unwind while the work completes), **CE039** (a criterion checker must not return a gating `score=0.0` from an `except OSError` over a path the *task author* named — that books an eval-config error as an agent failure; raise `CheckerMisuseError` instead, and `# noqa: CE039` the cases that really are the agent's), **CE047** (every onboarding/marketing surface — README, `docs/index.md`, `docs/comparison.md`, `docs/llms.txt`, `mkdocs.yml`'s `site_description`, the Pages stub, and pyproject's `description`/`keywords` — must name every built-in `AgentKind`; OpenCode shipped while four of those seven still listed three harnesses, and nothing failed), **CE057** (a module copied into the recorder directory beside a generated sandbox shim — `models.sandbox.SIDECAR_MODULES`, currently `argv_match.py` — may import stdlib only. The failure is silent: the sidecar runs where `coder_eval` and its dependencies are not installed, so one package import makes every shadowed CLI die with an ImportError the agent reads as "the tool is broken", costing a whole run to diagnose. The rule derives its target set from that exported tuple and a test asserts it matches a file that exists — a lint rule guarding zero files must fail, not pass). +Recent additions, each traceable to a shipped defect: **CE065** (in `tasks/`, no `task_id:`-bearing YAML and no resolved `reference.directory` reached through an in-repo `agent.plugins[].path` / `sandbox.template_sources[].path` may be left READABLE by the docker anti-cheat allowlist (`isolation/eval_material.py::mask_dirs`). Under `driver: docker` the whole plugin root is auto-mounted `:ro` so it loads, and every child dir OUTSIDE the keep-set (`.claude-plugin` + the manifest-declared skill dirs) is tmpfs-masked by default, so sibling task YAMLs / reference solutions / `tests/` are masked for free. Two spots the mask cannot cover and the rule flags: (a) an eval def or reference COLOCATED inside a skill dir — masking it would hide the skill; and (b) a `task_id:` YAML **file** loose at the plugin root — a tmpfs masks a directory, not a single file. Either hands the agent under evaluation its own grading answer key. The rule reuses the SAME `mask_dirs` the runtime applies (walking the plugin root, pruning the masked subtrees, flagging any `task_id:` YAML or reference dir left unmasked) — one SSOT, so the guardrail and the control cannot diverge; `manifest_skill_dirs` (`agents/_skills.py`) is the shared skill-dir resolver, asserted by an import-identity test. Plugin/template paths resolve relative to the task-file dir, matching the runtime auto-mount (`docker_runner` now resolves a relative `plugins[].path` against `task_file.parent`, not CWD, so the static rule and the runtime inspect the same tree). Like CE055 it is a pytest class scanning `tasks/**` with positive (inside-skill-dir + loose-root-file) and negative in-memory sensors so an empty glob cannot pass silently. The fix is never to relax the mask but to move the eval def / reference under a sibling `tests/` (or any non-kept dir), where the allowlist masks it), **CE064** (in `src/coder_eval/agents/`, a module that imports `TurnClock` must pass an explicit `timestamp=` to `AgentStartEvent` and `AgentEndEvent` — the turn's OUTER bounds, which no other rule looks at, since CE058-CE061 all scope to `AssistantMessage` and the bracket is not one. `timing.decompose_turn` produces `harness_startup_ms` / `harness_teardown_ms` by subtracting a generation-window bound from a bracket timestamp, so the two must share a basis; all three clocked harnesses derived their bounds from the `TurnClock` and let the bracket fall back to `StreamEvent.timestamp`'s `default_factory=datetime.now`, putting a monotonic-derived stamp and a raw wall stamp inside one subtraction — the exact split `TurnClock` exists to remove, reintroduced at the one seam the clock did not own. Measured on a live antigravity turn: an `AgentEndEvent` stamped **17 us BEFORE its own last message finished**, which cannot happen (the event is constructed strictly after the final flush), and `decompose_turn` clamped that negative and published `0.0` — "measured, and instant", the CE058 confusion arrived at from the other direction — for a harness whose real tail is ~0.1 ms; it now records 0.035 ms. It surfaced on one harness only because the drift is tens of microseconds and antigravity is the only one that holds its process across turns, so nothing happens between its last flush and its end event; every other harness books a tail of 7-543 ms, where the drift is invisible rather than absent — which is why the fix is at every clocked site rather than at that one. SCOPE IS DERIVED, never a harness list: codex and opencode take their spans from the CLI's own epoch stamps, deliberately have no `TurnClock`, and are correctly invisible to the rule — a raw bracket is CONSISTENT with their bounds — and the day either adopts a clock the rule starts applying with no edit. BLIND SPOT, in the rule's docstring: presence, not correctness. It cannot tell `self.clock.now()` from a `datetime.now()` spelled out at the call site, because the three harnesses legitimately reach their clock three ways; the guard for the SOURCE is behavioural (`tests/_bracket_clock.py` injects a stand-in anchored a year from real time, so a reverted argument fails by a year rather than by the microseconds that separate the two clocks), which is the division of labour CE060 states — a rule removes the SILENT case, a default nobody chose), **CE063** (no module in `src/coder_eval/agents/` may import `busy_ms` — tool execution comes out of a generation window in exactly ONE place, `timing.py::subtract_tool_time`. Five reducers used to do it themselves while the head and tail were already computed centrally at the same seam, and that asymmetry is where every timing defect on this branch lived — none of them in the arithmetic, all of them in the bookkeeping AROUND it: when to reset a per-step span list (clearing it at `step_start` wiped a span before the flush could subtract it, a 100% overstatement of that window), when to clear a spent start stamp (a second flush with no intervening start republished the previous span — 3000 ms of generation for a 2000 ms turn), when to advance the mark. A sixth harness reaching for `busy_ms` rebuilds that, and its tool time is then subtracted TWICE — by the reducer and again by the collector — under-reporting generation on one harness only, which takes a corpus comparison to notice. A separate id from CE061 rather than a rebody: CE061 asks where a window's ARITHMETIC came from and four reducers still call `close_window`, so its property is live and unsuperseded; this asks whether a reducer subtracts at all. It deliberately does NOT reuse CE061's `_imports_the_helper`, whose bare-module-import branch exists so `timing.close_window(...)` counts as reaching the helper — inverted into a ban that branch flags four of the five reducers. CE061 is now **exemption-free**: claude-code was its one permanent `# noqa` and, with the subtraction moved, calls the shrunken `close_window` like the other four), **CE060** (in `src/coder_eval/agents/`, every `AssistantMessage(...)` must pass `message_id` explicitly — an identity invariant, which is why it is its own id rather than a second arm of CE058/CE059, both of which are about timing. Antigravity omitted the kwarg, so the field defaulted to `None` on every message it ever recorded, and the evalboard — which groups assistant emissions by `message_id` and falls back to a `SAME_EMISSION_GAP_MS` wall-clock gap when either side lacks one — collapsed a whole turn's generations into ONE timeline row as soon as the harness's generation windows became contiguous (the gap is then exactly 0 ms, always). Nothing failed: the consumer SUMS the group, so the totals and the reconciliation invariant stayed right, and the golden snapshots had ratified the `null` on the day they were written — a snapshot is regenerated from whatever the code currently does, so it catches a later change and never an initial omission. The damage was not confined to the timeline, which is why "only granularity is lost" was the wrong way to describe it: a grouped emission is one API call to the evalboard's thinking-cost simulator, whose prompt-cache cascade is quadratic in that count, so a single-shot Antigravity run had every cascade coefficient pinned at zero; the `Messages` count and the 10 s slow-generation bar were per-turn too. Unlike its two siblings it **derives its constructor set from each module's own `coder_eval.models` imports** instead of hardcoding the spelling, which closes exactly the blind spot CE058's clause below concedes: `claude_code_agent.py` binds only `AssistantMessage as AssistantMessageTelemetry`, so a name list guards that file's two construction sites purely by coincidence, and an arbitrary `as Msg` is missed outright. Widening CE058/CE059 the same way is recorded in `.claude/harness-candidates.md`. BLIND SPOT, in the rule's docstring: the runtime `None` — the kwarg must be PRESENT, not statically non-`None`, because OpenCode's `messageID` and Pi's `responseId` legitimately evaluate to `None` when the CLI omits them, and passing a fallback expression *is* deciding), **CE058** (in `src/coder_eval/`, an unknown timing value may not become a numeric literal — `duration_ms is None` means *never timed* and `0.0` means *timed and instant*, so writing the literal publishes the second while meaning the first. One invariant, one id, five syntactic forms — a zero constructor keyword, `x or 0`, `x if x is not None else 0.0`, `if x.duration_ms is None: x.duration_ms = 0.0`, and a `model_copy(update={...})` dict (the shape the Antigravity DONE path writes through, which a keyword-only rule cannot see). Antigravity constructed EVERY message with `generation_duration_ms=0.0`, so the task page's Generation cell read `0ms` and its breakdown rendered `0%` for months with nothing failing; Codex published the SDK's `0.0` as a measured command duration, so `avg_command_time_ms` divided real milliseconds by a command count of which 70 of 211 in one nightly had never been timed. The fourth form is the one no existing rule shape covered and is where a live instance was hiding — `claude_code_agent._finalize_commands` set `0.0` on every command force-closed without a tool result, in the one harness a timing audit had called healthy. BLIND SPOT, stated in the rule's docstring: form 1 keys on the callee's spelling, so renaming the `AssistantMessageTelemetry` import alias silently disarms it there), **CE059** (in `src/coder_eval/agents/`, an `AssistantMessage` may not receive the same `ast.Name` for both `started_at` and `completed_at` — the Antigravity reducer read `datetime.now()` once and passed it as both bounds, so `started_at == completed_at` on 368 of 368 sampled messages. A separate id from CE058 because it is a separate invariant, a zero-length window whatever the duration field says, and one invariant per id is what makes a `# noqa` mean one thing. It does NOT fire when the same call passes `generation_duration_ms=None`: a call that says, in the field built to say it, that no window was measurable is not claiming one — that exemption is what keeps the rule pointed at the misleading case instead of accumulating four permanent suppressions on the rollout-rebuild and sub-agent-synthesis sites), **CE056** (no bare `CODER_EVAL_IN_CONTAINER` literal outside `models/container_paths.py` — the CE053 shape again: a rename-safety constant that shipped beside the literal it replaced, and the straggler was the single WRITER, so a rename would have disarmed four security/correctness gates at once with nothing failing; CE052 cannot catch it because that rule inspects `if` guards and the writer is not one), **CE055** (a criterion `path:` in `tasks/` must be sandbox-relative — an absolute path is joined onto the sandbox root, which DISCARDS the root, so containment refuses it and the criterion can never match whatever the agent does; two in-tree tasks were broken this way and the pair is the argument for a static rule on top of the runtime `CheckerMisuseError`: `byod_smoke_test` IS in a CI bucket and produced only `Results: 7/8 succeeded` plus a gating 0.0 reading "file does not exist" for a file that existed, while `dockerfile_build_example` is in NO bucket, so nothing ran it and no runtime guard was ever reached — the fix is never to relax containment but to say what the criterion means, `run_command: test -f /opt/marker`, a claim about the container IMAGE rather than about the agent's workspace), **CE054** (an `environment_info` key that is READ must be WRITTEN somewhere in `src/` — the bag is `dict[str, Any]`, so nothing connects reader to writer, and the `reference_digest` anti-cheat guard shipped as a read with no writer anywhere: `.get()` returned `None`, the guard took its early return, and CLAUDE.md plus the user guide both described it as protection it never provided), **CE048** (never call a Typer command function in process — its parameter defaults are `OptionInfo` sentinels, not values, and the sentinel is TRUTHY, so `in_place=None` silently selected the wrong branch; the fix is the `run_pipeline` / `run_evaluation` / `run_plan` split, and this rule is the one that also scans `tests/`, since that is the only place the defect occurs), **CE049** (never coalesce a possibly-unmeasured score to a numeric literal — `score or 0.0` publishes "measured and scored zero" while meaning "never measured", which is how an ungraded night reached four unfiltered `avg(Score)` dashboards as a real zero), **CE050** (no untyped `getattr` probe for a discriminated-union field — pyright cannot see the string, so a rename degrades the guard to a permanent no-op; scoped to criterion-shaped receivers because `command`/`tool`/`prompt` are far too common to flag on their own), **CE051** (a sandbox driver may not be rewritten silently — the driver IS the isolation boundary, so a downgrade must be an explicit, stamped, operator-visible decision), **CE053** (no bare run-record or run-LOG filename literal outside `path_utils` — widened to `docker.log` / `grade.docker.log` / `task.log` / `grade.log` after the same shape recurred: `docker.log` was produced in `isolation/` and consumed in `orchestration/` as three unrelated literals, and because the consumer guards its copy with `is_file()`, a rename would have silently discarded the only record of why a grading container failed — `TASK_JSON_FILENAME` shipped with a rename-safety rationale while twelve exact literals stayed unmigrated, including all three `rglob("task.json")` sites the constant's own comment cites as its reason to exist, so it created the second source of truth it argues against), **CE052** (an `os._exit` must sit inside a branch testing `CODER_EVAL_IN_CONTAINER` — it is the right primitive only for reaping the container's own disposable main process, and `run_task_internal_command` armed its heartbeat watchdog, a daemon thread whose whole authority is `os._exit(137)`, unconditionally: a test that invoked the command in-process left the pytest worker holding that thread, which exited the worker 40s later inside an unrelated test file, naming a different test on each run and on each platform with no traceback — and the dead worker's lost coverage data then failed the gate as `65.13 < 80.00`, naming neither the test nor the cause), **CE037** (no unreferenced module-level private helper in `src/` — a helper whose docstring documents a bug the live code still has is worse than none), **CE038** (in an `@asynccontextmanager`, the acquire must sit INSIDE the `try` whose `finally` releases it — `asyncio.shield` protects the inner task, NOT the await, so a cancel on `__aenter__` skips the unwind while the work completes), **CE039** (a criterion checker must not return a gating `score=0.0` from an `except OSError` over a path the *task author* named — that books an eval-config error as an agent failure; raise `CheckerMisuseError` instead, and `# noqa: CE039` the cases that really are the agent's), **CE047** (every onboarding/marketing surface — README, `docs/index.md`, `docs/comparison.md`, `docs/llms.txt`, `mkdocs.yml`'s `site_description`, the Pages stub, and pyproject's `description`/`keywords` — must name every built-in `AgentKind`; OpenCode shipped while four of those seven still listed three harnesses, and nothing failed), **CE057** (a module copied into the recorder directory beside a generated sandbox shim — `models.sandbox.SIDECAR_MODULES`, currently `argv_match.py` — may import stdlib only. The failure is silent: the sidecar runs where `coder_eval` and its dependencies are not installed, so one package import makes every shadowed CLI die with an ImportError the agent reads as "the tool is broken", costing a whole run to diagnose. The rule derives its target set from that exported tuple and a test asserts it matches a file that exists — a lint rule guarding zero files must fail, not pass). When fixing a bug, ask: *could a custom lint rule have prevented this?* If the root cause is a mechanically detectable pattern (e.g., "always import from `coder_eval.models`", "never call blocking IO in async"), add a rule to `tests/lint/rules/` following the CE001+ pattern and wire it up in `tests/lint/runner.py`. This turns a one-time fix into permanent enforcement. See `tests/test_custom_lint.py` for how rules are tested. (Doc-surface / whole-tree rules that reason over Markdown/YAML or the entire `src/` tree rather than one `.py` AST at a time — CE026–CE031, CE033–CE036 — are not `BaseRule`s in the runner; they are wired as dedicated `@pytest.mark.lint` test classes. CE036 enforces the `live_verdict` determinism + monotonicity contract (`criteria/base.py`) that `EarlyStopWatcher`'s latching, deferred fail-stop, and flip-attribution silently depend on: monotonicity over arbitrary Python is undecidable, so instead of a static check it REPLAYS each live criterion against every prefix of recorded trajectories (`tests/lint/live_verdict_contract.py::CASES`) — on the authored ordering AND under seeded shuffles (`permuted_violations`, which catch order-sensitive bugs the authored walk misses) — and asserts the property directly, plus registry-derived coverage — every `LiveSuccessCriterion` in the union must have cases, and every polarity its instances claim via `live_decidable_polarities()` must actually be reached by one (otherwise a single always-`undecided` fixture would "cover" a type while proving nothing). Adding a live criterion therefore means adding `ContractCase`s in the same change. CE035 resolves every `steps..outputs.` / `needs..outputs.` reference in `.github/workflows/**` to a writer that actually produces that key — GitHub expands an unwritten output to the empty string, so a typo degrades a gate silently and actionlint models `steps.*.outputs` as an open string map. CE034 scans `tasks/` and forces an armed, live-*passable* `command_executed` to set `require_success` — a crashed invocation would otherwise latch a live PASS, fire `on_pass: stop`, and let FIRED-ONLY armed gating report SUCCESS without ever consulting the unarmed criteria (negative assertions are fail-only and are exempt). CE033 keeps the plugin's bundled `reference/criteria.md` in parity with the `SuccessCriterion` union that generates it (`make plugin-reference` writes it; the rule re-renders and diffs — never hand-edit the file). CE031 guards against dead config: a behavior-driving field on `SimulationConfig`/`RunLimits`/`Dataset` that no code reads by name. CE026 keeps the GitHub Action's onboarding surfaces honest — `README.md`, `docs/CI_GATE.md`, `docs/tutorials/02-ci-pipeline.md`, and the plugin's `ci` skill, whose emitted workflow users copy into their own repos: a page's *first* Action snippet must show the agent-runtime prerequisite steps (pinned to the `action-dogfood` job that proves them in CI), a zero-install absolute next to such a snippet must name the channel it means, every `github.com/marketplace/actions/` link plus the shields badge label must match `action.yml`'s `name:`, and every `with:` key on a snippet's action step must be a real `action.yml` input (GitHub ignores unknown inputs, so a rename would silently degrade every copied workflow). Renaming an action input or changing its runtime prerequisites therefore means updating the skill too.) diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index c1f5039a..36a53318 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -1522,6 +1522,21 @@ def _reference_mount_args(self) -> list[str]: # EROFS. See _prepare_reference_mount. return ["-v", f"{self._reference_mount_src}:{CONTAINER_REFERENCE_DIR}"] + def _resolve_mount_path(self, raw_path: str) -> Path: + """Resolve an auto-mount source path to an absolute host path. + + A RELATIVE path resolves against the task-file dir (matching + reference / template / CE065 resolution), NOT the process CWD. + ``agent.plugins[].path`` is the one auto-mounted field not absolutized at + load, so a bare ``.resolve()`` mounted a CWD-relative tree while CE065 + inspected the task-file-relative one -- they could disagree. + (``template_sources[].path`` is already absolute by the time it gets here.) + """ + expanded = Path(os.path.expandvars(os.path.expanduser(raw_path))) + if not expanded.is_absolute() and self.rt.task_file is not None: + expanded = self.rt.task_file.parent / expanded + return expanded.resolve() + def _build_argv( self, input_dir: Path, output_dir: Path, *, container_name: str, image: str | None = None ) -> list[str]: @@ -1743,7 +1758,7 @@ def _build_argv( def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: if not raw_path: return - resolved = Path(os.path.expandvars(os.path.expanduser(raw_path))).resolve() + resolved = self._resolve_mount_path(raw_path) # File paths get mounted as the parent dir so a single -v covers # the file; container-side reads still resolve at the same path. target = resolved if (dir_only or resolved.is_dir()) else resolved.parent diff --git a/src/coder_eval/isolation/eval_material.py b/src/coder_eval/isolation/eval_material.py index a3b9a2b2..83fce568 100644 --- a/src/coder_eval/isolation/eval_material.py +++ b/src/coder_eval/isolation/eval_material.py @@ -70,9 +70,14 @@ def mask_dirs(root: Path) -> list[Path]: # Directories to descend into: the root plus every protected ANCESTOR (a # protected path that is not itself a kept leaf). We never descend into a - # kept skill dir -- everything inside it stays readable, including the - # skill's own supporting assets. - descend = {root, *(p for p in protected if p not in keep)} + # kept path -- everything inside a kept skill dir (or `.claude-plugin`) stays + # readable, including the skill's own supporting assets. Excluding kept paths + # from `descend` also covers the degenerate manifest `skills: "."`: `root` + # then IS a kept leaf, so it is not descended and nothing is masked -- the + # whole plugin root is the skill surface, and masking its children would + # hide the skill. Fail-safe (no leak vs the plugin, at the cost of no masking + # for that unusual layout; CE065 still flags in-repo colocated task defs). + descend = {p for p in ({root} | protected) if p not in keep} masked: set[Path] = set() for directory in descend: diff --git a/tests/test_custom_lint.py b/tests/test_custom_lint.py index 76a5a153..59a8ffb7 100644 --- a/tests/test_custom_lint.py +++ b/tests/test_custom_lint.py @@ -4982,96 +4982,104 @@ def test_is_suppressible(self, tmp_path): assert not check_file(target, [TurnBracketOnTheClock]) -class TestCE065NoEvalMaterialInsideSkillDir: - """CE065 — no eval definition / reference dir INSIDE a skill dir. - - The Fix B allowlist auto-masks eval material anywhere under an auto-mounted - plugin root EXCEPT inside a kept skill dir — masking those would hide the - skill itself. So a `task_id:`-bearing YAML (or a resolved `reference.directory`) - colocated INSIDE a skill dir is the one leak the runtime allowlist cannot - close: under `driver: docker` the agent reads its own grading answer key - straight out of the readable skill surface. - - This static rule flags that layout in-repo so it can never recur silently. - It reuses the SAME `manifest_skill_dirs` resolver the runtime allowlist uses - (one SSOT for "what is a skill dir"). The fix is to move the eval def / - reference OUT of the skill dir (e.g. to a sibling `tests/`), where the - allowlist masks it without hiding the skill. +class TestCE065EvalMaterialReadableUnderPluginRoot: + """CE065 — no eval material left READABLE under an auto-mounted plugin root. + + Under `driver: docker` the Fix B allowlist keeps `.claude-plugin` + the + manifest-declared skill dirs readable and `--tmpfs`-masks every other child + dir (`eval_material.mask_dirs`). Two spots the mask cannot cover, where the + agent under evaluation would read its own grading answer key straight off the + readable surface: + + * a `task_id:`-bearing YAML (or a resolved `reference.directory`) INSIDE a + kept skill dir — masking it would hide the skill; + * a `task_id:` YAML FILE loose at the plugin root (or any other unmasked + spot) — a `--tmpfs` masks a directory, not a single file. + + This static rule flags both in-repo so the layout can never recur silently. + It reuses the SAME resolver AND the SAME `mask_dirs` the runtime allowlist + uses (one SSOT): anything the runtime does NOT mask must not be eval material. + The fix is to move the eval def / reference under a sibling `tests/` (or any + non-kept dir), where the allowlist masks it. """ ROOT = Path(__file__).parent.parent @staticmethod - def _skill_dirs_for_task(task, task_file: Path) -> list[Path]: - """In-repo skill dirs reachable from a task's plugin / template roots. - - Plugin roots (`agent.plugins[].path`) and `TemplateDirSource.path` roots - are resolved relative to the task file's dir (mirroring - `resolve_host_reference_dir`), then each plugin root's skill dirs come - from the shared `manifest_skill_dirs` resolver. + def _plugin_roots_for_task(task, task_file: Path) -> list[Path]: + """Real plugin roots reachable from a task's plugin / template paths. + + `agent.plugins[].path` and `TemplateDirSource.path` are resolved relative + to the task-file dir — matching the runtime auto-mount (`docker_runner` + resolves a relative `plugins[].path` against `task_file.parent`) and + reference/template resolution. Only real plugin roots + (`.claude-plugin/plugin.json`) are kept; a plain template dir declares no + skills and is not masked, so it is out of scope. """ import os - from coder_eval.agents._skills import manifest_skill_dirs from coder_eval.models import TemplateDirSource - roots: list[Path] = [] + raws: list[str] = [] agent = task.agent for plugin in (agent.plugins if agent else None) or []: raw = plugin.get("path") if isinstance(plugin, dict) else None - if not raw: - continue - expanded = Path(os.path.expandvars(os.path.expanduser(str(raw)))) - root = expanded if expanded.is_absolute() else (task_file.parent / expanded) - roots.append(root.resolve()) + if raw: + raws.append(str(raw)) sandbox = task.sandbox for source in (sandbox.template_sources if sandbox else None) or []: - if not isinstance(source, TemplateDirSource): - continue - expanded = Path(os.path.expandvars(os.path.expanduser(str(source.path)))) - root = expanded if expanded.is_absolute() else (task_file.parent / expanded) - roots.append(root.resolve()) - - skill_dirs: list[Path] = [] - for root in roots: - # Only a real plugin root declares skills; a plain template dir has none. - if not (root / ".claude-plugin" / "plugin.json").is_file(): - continue - skill_dirs.extend(manifest_skill_dirs(root)) - return skill_dirs + if isinstance(source, TemplateDirSource): + raws.append(str(source.path)) + + roots: list[Path] = [] + for raw in raws: + expanded = Path(os.path.expandvars(os.path.expanduser(raw))) + root = (expanded if expanded.is_absolute() else task_file.parent / expanded).resolve() + if (root / ".claude-plugin" / "plugin.json").is_file(): + roots.append(root) + return roots @classmethod def _offenders(cls, task, task_file: Path) -> list[str]: - from coder_eval.orchestration.evaluation import resolve_host_reference_dir + import os - skill_dirs = cls._skill_dirs_for_task(task, task_file) - if not skill_dirs: - return [] + from coder_eval.isolation.eval_material import mask_dirs + from coder_eval.orchestration.evaluation import resolve_host_reference_dir offenders: list[str] = [] - - # A resolved reference dir colocated inside a skill dir. ref_dir = resolve_host_reference_dir(task, task_file) - if ref_dir is not None: - for skill in skill_dirs: - if ref_dir == skill or skill in ref_dir.parents: - offenders.append(f"reference.directory -> {ref_dir}") - break - - # A `task_id:`-bearing YAML inside a skill dir. - for skill in skill_dirs: - if not skill.is_dir(): - continue - for yaml_file in skill.rglob("*.yaml"): - if yaml_file.name == "metadata.yaml": - continue - try: - text = yaml_file.read_text(encoding="utf-8") - except OSError: - continue - # Cheap key test: a task definition declares a top-level task_id. - if re.search(r"(?m)^task_id\s*:", text): - offenders.append(f"task def inside skill dir -> {yaml_file}") + + for root in cls._plugin_roots_for_task(task, task_file): + # The exact set the runtime tmpfs-masks. Anything NOT under one of + # these stays readable to the agent -- so if it is eval material, it + # leaks. + masked = {Path(m).resolve() for m in mask_dirs(root)} + + def _is_masked(p: Path, _masked: set[Path] = masked) -> bool: + rp = p.resolve() + return any(m == rp or m in rp.parents for m in _masked) + + # Walk the readable surface only: prune masked subtrees so we neither + # waste time nor false-flag a task def the runtime already hides. + for dirpath, dirnames, filenames in os.walk(root): + here = Path(dirpath) + dirnames[:] = [d for d in dirnames if not _is_masked(here / d)] + for fn in filenames: + if not (fn.endswith(".yaml") or fn.endswith(".yml")) or fn == "metadata.yaml": + continue + yaml_file = here / fn + try: + text = yaml_file.read_text(encoding="utf-8") + except OSError: + continue + # Cheap key test: a task definition declares a top-level task_id. + if re.search(r"(?m)^task_id\s*:", text): + offenders.append(f"readable task def under plugin root -> {yaml_file}") + + # A resolved reference dir under this root that the runtime leaves readable. + if ref_dir is not None and (ref_dir == root or root in ref_dir.parents) and not _is_masked(ref_dir): + offenders.append(f"readable reference.directory -> {ref_dir}") + return offenders @pytest.mark.parametrize( @@ -5079,16 +5087,17 @@ def _offenders(cls, task, task_file: Path) -> list[str]: sorted(p for p in (Path(__file__).parent.parent / "tasks").rglob("*.yaml") if p.name != "metadata.yaml"), ids=lambda p: p.relative_to(Path(__file__).parent.parent).as_posix(), ) - def test_repo_tasks_keep_eval_material_out_of_skill_dirs(self, path: Path): + def test_repo_tasks_keep_eval_material_masked(self, path: Path): from coder_eval.orchestration.task_loader import load_task task, _ = load_task(path) offenders = self._offenders(task, path) assert not offenders, ( - f"{path}: {offenders} live inside a skill dir under an auto-mounted plugin/template root. " - "The Fix B allowlist keeps skill dirs readable, so it cannot mask eval material there — the " - "agent under `driver: docker` reads its own grading answer key. Move the eval def / reference " - "OUT of the skill dir (e.g. to a sibling `tests/`), where the allowlist masks it." + f"{path}: {offenders} stay READABLE under an auto-mounted plugin root. The Fix B allowlist " + "masks non-skill child dirs, but cannot mask eval material inside a skill dir (would hide the " + "skill) or a loose YAML file at the root (a tmpfs masks a dir, not a file) — so the agent under " + "`driver: docker` reads its own grading answer key. Move the eval def / reference under a sibling " + "`tests/` (or any non-kept dir), where the allowlist masks it." ) def _synthetic_plugin_task(self, tmp_path: Path, *, place_task_inside_skill: bool): @@ -5120,10 +5129,20 @@ def _synthetic_plugin_task(self, tmp_path: Path, *, place_task_inside_skill: boo return task, task_file def test_detects_a_task_def_inside_a_skill_dir(self, tmp_path): - """Positive sensor — without it an empty `tasks/` glob would 'pass'.""" + """Positive sensor — a task def inside a kept skill dir stays readable.""" task, task_file = self._synthetic_plugin_task(tmp_path, place_task_inside_skill=True) offenders = self._offenders(task, task_file) - assert offenders and any("skill dir" in o for o in offenders), offenders + assert offenders and any("readable task def" in o for o in offenders), offenders + + def test_detects_a_loose_task_def_file_at_plugin_root(self, tmp_path): + """Positive sensor (L1) — a `task_id:` YAML FILE loose at the plugin root + is unmasked (a tmpfs masks a dir, not a single file) and must be flagged.""" + task, task_file = self._synthetic_plugin_task(tmp_path, place_task_inside_skill=False) + # add a loose criteria file directly at the plugin root (not in any masked dir) + plugin_root = tmp_path / "plugin" + (plugin_root / "answers.yaml").write_text("task_id: loose_leak\nsuccess_criteria: []\n", encoding="utf-8") + offenders = self._offenders(task, task_file) + assert offenders and any("answers.yaml" in o for o in offenders), offenders def test_allows_eval_material_outside_skill_dirs(self, tmp_path): """Negative sensor — a task def under a sibling `tests/` is masked at runtime.""" diff --git a/tests/test_eval_material.py b/tests/test_eval_material.py index a700c535..8f2d75ff 100644 --- a/tests/test_eval_material.py +++ b/tests/test_eval_material.py @@ -56,6 +56,16 @@ def test_default_skills_layout_masks_non_skill_children(self, tmp_path: Path): assert (root / "skills").resolve() not in masked assert (root / ".claude-plugin").resolve() not in masked + def test_manifest_skills_at_root_masks_nothing(self, tmp_path: Path): + # Degenerate layout: the manifest declares the plugin ROOT as its skills + # dir (`skills: "."`). The whole root is then the skill surface, so masking + # any child would hide the skill -> mask nothing (fail-safe: denial, not a + # leak). Guards against collapsing the keep-set and breaking plugin load. + root = _plugin_root(tmp_path, skills=".") + (root / "some_dir").mkdir() + (root / "tests").mkdir() + assert mask_dirs(root) == [] + def test_skill_internal_assets_stay_readable(self, tmp_path: Path): root = _plugin_root(tmp_path) (root / "skills" / "demo" / "assets").mkdir(parents=True) From 0de0c48ba14c3787f4393a07bf68ec17dbfd3505 Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Mon, 14 Sep 2026 20:43:06 +0100 Subject: [PATCH 7/7] fix(anti-cheat): apply multi-model review findings M1/M2/M3 + doc drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multi-model review (Opus + Fable + Sonnet) found no Critical/High and no leak bypass; addressed the 3 Mediums and the doc drift: M2 — nested plugin roots (plugin B under plugin A) produced a duplicate Docker mount destination: A's mask emitted --tmpfs /B while B's own mount bound /B:ro. Defer mask emission until all binds are known and drop any mask whose path is also a bind (the bind wins so B loads and masks its own children). Extracted the auto-mount block into _append_auto_mounts to stay under the per-method lint limits. Added a nested-plugin test. M1 — added direct tests for _resolve_mount_path (relative->task-file dir, absolute unchanged, task_file None->cwd, env-var expansion); the L2 fix had no coverage. M3 — warn when a plugin root's mask stands down (manifest skills: '.') so the anti-cheat mask never voids silently. Added a test. Doc drift (L-a/L-b/L-c): corrected the now-false 'task.yaml readable at /work/input' / 'hiding criteria is unsolved' claims in orchestrator.py, fs_permissions.py and CLAUDE.md (Fix A deletes task.yaml + context.json after load); widened DOCKER_ISOLATION.md's residual note to include the loose-root-file case; marked the harness-candidates entry RESOLVED (nothing deferred). make verify green (5758 passed). Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/harness-candidates.md | 30 +--- CLAUDE.md | 2 +- docs/DOCKER_ISOLATION.md | 10 +- src/coder_eval/fs_permissions.py | 5 +- src/coder_eval/isolation/docker_runner.py | 178 +++++++++++++--------- src/coder_eval/orchestrator.py | 10 +- tests/test_docker_runner_mounts.py | 66 ++++++++ 7 files changed, 193 insertions(+), 108 deletions(-) diff --git a/.claude/harness-candidates.md b/.claude/harness-candidates.md index bd347653..04797d3e 100644 --- a/.claude/harness-candidates.md +++ b/.claude/harness-candidates.md @@ -874,25 +874,11 @@ re-derive from scratch. candidate — a small real bug needing its own change. Caught in: the turn-timing consolidation final review (gpt-5.6-sol). -## docker anti-cheat auto-mount allowlist (fix/docker-anti-cheat-leaks) — deferred Low findings - -Surfaced by the code review of the Fix A/B/C branch. All Low; the Critical (context.json -source_yaml leak) was fixed in-branch (commit dc71add6). These are follow-ups, each -needing its own review: - -- **L1 — loose `task_id:` YAML *file* at a plugin root is unmasked.** `eval_material.mask_dirs` - masks child *directories* only (a `--tmpfs` needs a dir mountpoint), and CE065 only scans - *inside* skill dirs — so a bare `answers.yaml`/`criteria.yaml` directly under the plugin root - falls through both. Narrow (eval material is conventionally directory-shaped; UiPath/skills has - no such loose file). Fix options: extend CE065 to also flag a `task_id:` YAML file anywhere - under a masked plugin root (catches in-repo layouts), or per-file mask via `-v /dev/null::ro`. -- **L2 — CE065 vs runtime resolve plugin paths on divergent bases.** CE065 resolves - `agent.plugins[].path` relative to the task-file dir; `docker_runner._auto_mount` resolves the - raw path vs CWD. Benign today (in-repo tasks use absolute/env-var plugin paths, and the runtime - couples the mask to the very tree it mounts, so no runtime leak escapes). Fix: absolutize - `agent.plugins[].path` in `load_task` the way `TemplateDirSource.path` already is, or share one - plugin-path resolver between CE065 and `_auto_mount`. -- **L3 — manifest `skills: "."` collapses the keep-set.** If a manifest declares its skills dir as - the root, `mask_dirs` descends `root` and masks every child except `.claude-plugin` — denial, not - a leak (consistent with fail-safe default-deny), but it silently breaks such a plugin. Add a guard - (don't descend `root` when it is itself a kept leaf) + a test if that layout is supported. +## docker anti-cheat auto-mount allowlist (fix/docker-anti-cheat-leaks) — RESOLVED in-branch + +Surfaced by the code review of the Fix A/B/C branch. The Critical (context.json source_yaml +leak) was fixed in commit dc71add6; the three Low follow-ups (L1 loose task_id file at plugin +root, L2 CE065-vs-runtime path-resolution divergence, L3 manifest `skills: "."` collapsing the +keep-set) and the multi-model review's M1/M2/M3 (test coverage for `_resolve_mount_path`, nested- +plugin duplicate-mount crash, silent mask stand-down) were all fixed in the same branch. Nothing +deferred. Left here only as a pointer to the branch history. diff --git a/CLAUDE.md b/CLAUDE.md index cbc03c56..e82f8d8d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -159,7 +159,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Per-criterion aggregation**: Each `BaseCriterion` subclass exposes `aggregate(criterion, per_row_results) -> CriterionAggregate | None`. Default emits `count / mean / median / std / min / max` so every criterion is suite-thresholdable for free. Classification-style criteria return `ClassificationCriterionResult` (subclass of `CriterionResult`) and layer accuracy / P/R/F1 / confusion via the shared `overlay_classification_metrics` utility. `BaseSuccessCriterion.suite_thresholds` gates the suite on those metrics; CLI exits non-zero on any gate failure. - **Sub-agent token accounting**: There is NO separate per-sub-agent field. Every sub-agent generation is captured as a `parent_tool_use_id`-tagged `AssistantMessage` in the turn transcript, so per-sub-agent usage is derived by grouping those messages on that id (the evalboard's `aggregateSubAgentUsage` does exactly this). Claude bubbles its sub-agent's intermediate generations into the parent stream natively, and the **terminal** generation (delivered as the Agent tool result, never streamed) is synthesized into one via `_synthesize_subagent_terminal_message` from `tool_use_result.usage`. Codex reconstructs all child generations from the child rollout (`_recover_subagent_tool_calls`). The turn total already includes sub-agent cost — Claude via the SDK's cumulative `model_usage`; Codex via `_fold_subagent_tokens`, which folds the child messages (their real per-generation tokens) into the parent total. `CommandTelemetry.result_summary` is stored **untruncated** (no 200-char cap) so sub-agent returns are preserved whole. Set `CODER_EVAL_RAW_SDK_LOG=1` to dump every raw SDK event to the task log for inspection. - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The LiteLLM open-weight actual-cost join (`litellm_cost.apply_actual_cost`) deliberately writes cost at the TURN level only (`token_usage.total_cost_usd` = the real OpenRouter bill) plus the per-call `TurnRecord.provider_call_costs` audit record; it does NOT touch the message token buckets, so `EventCollector` stays the single writer and this invariant holds on every backend. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. -- **Reference solutions are directory-only, and shielded (partially) from the agent**: `task.reference` is a single required `directory:` (relative to the task YAML) — the inline `code:` / single-file `file:` forms are gone, because a directory is the only shape that can be permission-gated as a unit; a `model_validator(mode="before")` gives the removed forms a migration error. The orchestrator stages a **per-run private copy** (`orchestration/evaluation.py::stage_reference_dir`, symlinks stripped) into a tempdir, removed in `_cleanup` via `path_utils.rmtree_restrictive` (keyed on `_reference_staging_root`, recorded BEFORE the copy so a failed copy still cleans up; `rmtree(ignore_errors=True)` silently declines on a tree left at 000) and deliberately never preserved into `run_dir/artifacts`. That copy is held at mode `000` for the whole of every `agent.communicate` call via **`Sandbox.set_permissions`**, the driver-aware wrapper over `fs_permissions.py::set_permissions`. Windows **stack**: exiting restores the *enclosing* window's mode, only the outermost exit restores the pre-window mode — that is what makes a mid-turn re-grant (`mode=READ_ONLY_MODE`) expressible, and it covers two windows at the same mode so no refcount is needed. The window is enforced **only inside a docker container** (`Sandbox.enforces_permission_windows`) and is a no-op on the host, where the agent shares our uid. **That gate keys on the `CODER_EVAL_IN_CONTAINER` env var, NOT `sandbox.driver`** — `run_task_internal_command` rewrites `driver: docker` → `tempdir` before building the in-container Orchestrator, so a driver-based gate would silently disable the anti-cheat on exactly the path that needs it (regression-guarded by `TestSandboxDriverGate`); `resolve_reference_dir` gates its `/work/references` branch on the same var for the same reason. The task directory is **not** shielded (`:ro` mount → EROFS, and the same YAML is readable at `/work/input`). Criteria address reference files with the `$REFERENCE_DIR` token (same resolver as `$TASK_DIR`) and the `REFERENCE_DIR` env var for `run_command`; `reference_comparison` names one file via `reference_file`. Docker mounts a throwaway **read-write** copy at `/work/references` (a `:ro` mount cannot be chmod'd — EROFS), masks the in-task-dir original with an empty tmpfs, and drops `DAC_OVERRIDE`/`DAC_READ_SEARCH`. `FOWNER`/`CHOWN` are deliberately **NOT** dropped: the in-container orchestrator that applies the window is the same root process with the same caps, so dropping `FOWNER` breaks *the harness's own* chmod wherever the bind mount preserves a non-root owner (native Linux — verified: `chmod: Operation not permitted`), i.e. exactly where the drop would otherwise bite. A window that cannot be applied is now a hard error, not a warning: `Sandbox.set_permissions` passes `strict=True` whenever it enforces, so an unprotected run fails instead of producing a normal-looking score. **KNOWN GAP — this is defense-in-depth, not a boundary**: (a) `chmod(2)` is gated on owner-or-`CAP_FOWNER` and the container runs as root owning the copy, so a deliberate `chmod 755 /work/references` restores access; (b) the window spans `agent.communicate` only, and nothing reaps agent child processes at turn end, so a backgrounded read loop succeeds once the window closes. The **write** half of (b) is closed — `path_utils.digest_tree` hashes the tree at staging and `Orchestrator._verify_reference_integrity` re-checks before grading, raising `ReferenceTamperedError` (→ `FinalStatus.ERROR`) on a mismatch so an agent cannot overwrite the reference to drive `reference_comparison` to 1.0. Passive reads are blocked; an adversarial agent is not. Full containment requires running the agent as a non-root uid AND holding the window for the agent's whole lifetime — follow-up. `tasks/anti_cheat_reference` probes the passive-read half. +- **Reference solutions are directory-only, and shielded (partially) from the agent**: `task.reference` is a single required `directory:` (relative to the task YAML) — the inline `code:` / single-file `file:` forms are gone, because a directory is the only shape that can be permission-gated as a unit; a `model_validator(mode="before")` gives the removed forms a migration error. The orchestrator stages a **per-run private copy** (`orchestration/evaluation.py::stage_reference_dir`, symlinks stripped) into a tempdir, removed in `_cleanup` via `path_utils.rmtree_restrictive` (keyed on `_reference_staging_root`, recorded BEFORE the copy so a failed copy still cleans up; `rmtree(ignore_errors=True)` silently declines on a tree left at 000) and deliberately never preserved into `run_dir/artifacts`. That copy is held at mode `000` for the whole of every `agent.communicate` call via **`Sandbox.set_permissions`**, the driver-aware wrapper over `fs_permissions.py::set_permissions`. Windows **stack**: exiting restores the *enclosing* window's mode, only the outermost exit restores the pre-window mode — that is what makes a mid-turn re-grant (`mode=READ_ONLY_MODE`) expressible, and it covers two windows at the same mode so no refcount is needed. The window is enforced **only inside a docker container** (`Sandbox.enforces_permission_windows`) and is a no-op on the host, where the agent shares our uid. **That gate keys on the `CODER_EVAL_IN_CONTAINER` env var, NOT `sandbox.driver`** — `run_task_internal_command` rewrites `driver: docker` → `tempdir` before building the in-container Orchestrator, so a driver-based gate would silently disable the anti-cheat on exactly the path that needs it (regression-guarded by `TestSandboxDriverGate`); `resolve_reference_dir` gates its `/work/references` branch on the same var for the same reason. The task directory copy IS shielded by the same window (`self.sandbox.task_dir` is in the `set_permissions` list); the post-override YAML is also staged at `/work/input`, but the in-container entry point DELETES `/work/input/task.yaml` **and** `context.json` (whose `source_yaml` carries the criteria top-level and per `config_lineage` entry) after load via `_scrub_staged_inputs`, so the agent cannot read its criteria from there either. Criteria address reference files with the `$REFERENCE_DIR` token (same resolver as `$TASK_DIR`) and the `REFERENCE_DIR` env var for `run_command`; `reference_comparison` names one file via `reference_file`. Docker mounts a throwaway **read-write** copy at `/work/references` (a `:ro` mount cannot be chmod'd — EROFS), masks the in-task-dir original with an empty tmpfs, and drops `DAC_OVERRIDE`/`DAC_READ_SEARCH`. `FOWNER`/`CHOWN` are deliberately **NOT** dropped: the in-container orchestrator that applies the window is the same root process with the same caps, so dropping `FOWNER` breaks *the harness's own* chmod wherever the bind mount preserves a non-root owner (native Linux — verified: `chmod: Operation not permitted`), i.e. exactly where the drop would otherwise bite. A window that cannot be applied is now a hard error, not a warning: `Sandbox.set_permissions` passes `strict=True` whenever it enforces, so an unprotected run fails instead of producing a normal-looking score. **KNOWN GAP — this is defense-in-depth, not a boundary**: (a) `chmod(2)` is gated on owner-or-`CAP_FOWNER` and the container runs as root owning the copy, so a deliberate `chmod 755 /work/references` restores access; (b) the window spans `agent.communicate` only, and nothing reaps agent child processes at turn end, so a backgrounded read loop succeeds once the window closes. The **write** half of (b) is closed — `path_utils.digest_tree` hashes the tree at staging and `Orchestrator._verify_reference_integrity` re-checks before grading, raising `ReferenceTamperedError` (→ `FinalStatus.ERROR`) on a mismatch so an agent cannot overwrite the reference to drive `reference_comparison` to 1.0. Passive reads are blocked; an adversarial agent is not. Full containment requires running the agent as a non-root uid AND holding the window for the agent's whole lifetime — follow-up. `tasks/anti_cheat_reference` probes the passive-read half. - **Harness run-limit parity**: a shared `BaseAgentConfig` field must mean the same thing on every backend, so a divergence is either fixed or documented — never silent. **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool calls, read live off the shared `EventCollector.visible_turn_count`, the same list `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, so a native counter would clamp at 1; claude-code keeps its native SDK cap, whose unit (an agent-loop turn) absorbs arbitrarily many parallel calls — the same number is NOT the same budget across harnesses. OpenCode likewise keeps a native unit — the CLI streams a real multi-step loop per `communicate()` (`step_start`/`step_finish`), so `max_turns: N` allows N complete steps and cuts cleanly when step N+1 begins. **Pi** is the same shape — the CLI (`pi -p --mode json`) streams a real multi-step loop per `communicate()` (`turn_start`/`turn_end`), so `max_turns: N` counts native `turn_start` steps; Pi retries transient/provider errors INTERNALLY (`agent_end.willRetry`), and the reducer finalizes once at `agent_settled`/EOF (not the first `agent_end`), folding the retry cycles into one turn. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry); on Antigravity that boundary lives in `_drain()`, so the background-work poll loop honors it too. Known unfixed divergences: `permission_mode` on Codex, Antigravity, and Pi (all run unconfined — the sandbox driver is the isolation boundary), `disallowed_tools` on Codex (forwarded, not SDK-enforced), `allowed_tools`/`disallowed_tools` on Antigravity (not read at all), `turn_timeout` on Antigravity (bounded by an earlier internal poll deadline at 80% of it), `allowed_tools`/`disallowed_tools`/`system_prompt` on OpenCode (no CLI knob; warned at `start()`, not enforced), `allowed_tools`/`disallowed_tools` on Pi (its built-in tool names are lowercase — `bash`/`read`/… — and cannot map to the Claude-namespaced config default, so forwarding them would strip the agent of ALL tools; warned+ignored like the three agents above) — Pi DOES enforce `system_prompt` (`--append-system-prompt`, a small win over OpenCode) and DOES honor `plugins` for skills (each resolved skills dir → a `--skill ` arg via the shared `_plugin_skill_dirs` resolver, recorded as `pi_skill_paths`, so it CAN run activation suites) but does NOT read `system_prompt_file`, and **`agent.plugins[].path` depth** — claude-code REQUIRES a plugin root holding `skills/` and silently loads NOTHING from a bare skills directory, while Codex and Antigravity scan both depths and accept either. That is the costly direction: the wrong depth produces no error, every positive row of an activation suite scores 0, and the suite reports recall 0.0, which reads exactly like a skill that never triggers. Held to the plugin-root shape (for `SKILL_SOURCE_PATH` only) by lint rule CE045. `plugins` on OpenCode is **honored for skills**: each local plugin root is mapped to the `skills` dir its `.claude-plugin/plugin.json` declares (default `/skills`, never the root itself — `skills.paths` is scanned recursively and a plugin root can hold a self-referential symlink) and injected via `OPENCODE_CONFIG_CONTENT`, which `--pure` does not suppress; a plugin's agents/hooks/commands/MCP servers are still dropped. Full table + rationale: docs/agents/HARNESS_PARITY.md. - **sandbox isolation**: Tasks that don't need MCP servers should set `setting_sources: []` in their `agent:` block to isolate the sandbox from the host project's CLAUDE.md and settings. Without this, the host project's CLAUDE.md (often 20 KB+) is injected into every API call, inflating cache-creation tokens and cost significantly. - **Execute vs. run (the grading switch)**: `coder-eval execute` is `coder-eval run` with grading removed — the agent runs and the full trajectory is captured, but no criterion is checked, `weighted_score` is `None` (never `0.0`, which would be indistinguishable from "graded and scored zero"), and the row finalizes as **`FinalStatus.NOT_GRADED`**, whose `category` is a **fourth** bucket, `"ungraded"`. Ungraded rows leave BOTH sides of every rate: `RunSummary.pass_rate` / `error_share` and `VariantAggregate.pass_rate` divide by `tasks_graded` (`tasks_run - tasks_not_graded`), and `tasks_not_graded` is part of the sum-to-`tasks_run` invariant, not a `tasks_failed` sub-counter. **Only SUCCESS/FAILURE collapse into it** — `ERROR`, `TIMEOUT`, `BUILD_FAILED`, `MAX_TURNS_EXHAUSTED` and the budget stops are facts about the *run*, not about grading, and still apply (so `execute` still exits non-zero on a crash). The switch is `BatchRunConfig.grade` → `Orchestrator(grade=...)` → the **four** grading call sites (single-shot, evaluate-only, the simulation dialog check, and post-failure diagnostics); it crosses the docker boundary in `context.json` (defaulting to `True` in-container, so a host predating `execute` keeps grading). It is **deliberately not a task-config field** — no 5-layer merge, no `-D` path — because a task YAML must never declare itself ungraded; only the invoking command decides. `run` and `execute` share one body (`run_command.run_pipeline`) and differ solely in that flag, so there is no third code path. Three things are refused rather than degraded: `--junit-xml` (a report of verdicts, and there are none — though `reports_junit` still emits `` for an ungraded row it encounters), `--allow-host-grading` (it decides how an ungraded row is GRADED, and `execute` grades nothing), and simulation tasks (their turn-continuation logic reads criteria results, so an ungraded dialog would silently change its own stopping behavior). `stop_early:` blocks are inert under `execute` for the same reason the kill switch exists: the full trajectory is the deliverable. Motivating consumer: an external harness (Harbor / Terminal-Bench 2.0) that builds its own container, calls coder-eval as the agent, and grades with its own tests. diff --git a/docs/DOCKER_ISOLATION.md b/docs/DOCKER_ISOLATION.md index 1891ea73..6c86628b 100644 --- a/docs/DOCKER_ISOLATION.md +++ b/docs/DOCKER_ISOLATION.md @@ -369,10 +369,12 @@ reference window's posture above; neither contains an adversarial agent. the plugin surface is masked by default, so an unknown or new eval layout can never leak; `tests/`, `node_modules/`, and reference solutions are masked for free. A root agent cannot `umount` a tmpfs (`CAP_SYS_ADMIN` is not in Docker's - default set), so this mask is *stronger* than the mode-000 reference window. The - one residual — an eval def or reference COLOCATED inside a skill dir, which - cannot be masked without hiding the skill — is caught by lint rule CE065 (keep - eval material out of skill dirs; put it under a sibling `tests/`). + default set), so this mask is *stronger* than the mode-000 reference window. Two + residuals the mask cannot cover — an eval def or reference COLOCATED inside a + skill dir (masking it would hide the skill), and a `task_id:` YAML **file** loose + at the plugin root (a tmpfs masks a directory, not a single file) — are caught by + lint rule CE065 (keep eval material out of skill dirs and off the plugin root; + put it under a sibling `tests/`). Inside the container, the entrypoint invokes `coder-eval _run-task-internal` (hidden subcommand), which loads the staged YAML + context, runs the standard in-process Orchestrator (driver auto-coerced back to `tempdir`), and writes `task.json` to the output mount. Host reads it and feeds the existing aggregation pipeline. diff --git a/src/coder_eval/fs_permissions.py b/src/coder_eval/fs_permissions.py index 499c8320..6b3fe731 100644 --- a/src/coder_eval/fs_permissions.py +++ b/src/coder_eval/fs_permissions.py @@ -19,7 +19,10 @@ This shields grading MATERIAL that happens to live in the task directory (a ``reference/`` subdirectory, fixtures), not the task DEFINITION: ``task.yaml`` -is separately staged at ``/work/input``, which the agent can still read. +(and ``context.json``'s ``source_yaml``) are separately staged at ``/work/input`` +and DELETED after load by the in-container entry point +(``run_task_internal_command._scrub_staged_inputs``), so the agent cannot read +the criteria from there. Windows **stack**, which is what makes a mid-turn re-grant expressible: code that runs inside the turn but is not the agent can open a narrower window to diff --git a/src/coder_eval/isolation/docker_runner.py b/src/coder_eval/isolation/docker_runner.py index 36a53318..02f47756 100644 --- a/src/coder_eval/isolation/docker_runner.py +++ b/src/coder_eval/isolation/docker_runner.py @@ -164,6 +164,11 @@ def _rewrite_loopback_for_container(url: str) -> str | None: # Logged once per masked child of an auto-mounted plugin root (Fix B allowlist). _MASK_WARNING = "Masking non-skill path %s under plugin root %s (anti-cheat: only skills stay readable)." +_MASK_STANDDOWN_WARNING = ( + "Anti-cheat mask stood down for plugin root %s: its whole tree is the declared skill surface " + '(e.g. manifest `skills: "."`), so nothing is masked. Any eval material colocated here is READABLE ' + "to the agent — move it outside the plugin root." +) async def _heartbeat_loop(heartbeat_path: Path) -> None: @@ -1537,6 +1542,102 @@ def _resolve_mount_path(self, raw_path: str) -> Path: expanded = self.rt.task_file.parent / expanded return expanded.resolve() + def _append_auto_mounts(self, argv: list[str]) -> None: + """Bind-mount the host paths a task references, at their same host path. + + Covers Claude-Code plugin dirs (``agent.plugins[].path``) and + ``TemplateDirSource.path`` roots so they resolve inside the container at + the same path they have on the host. Each mount is ``:ro``; a plugin root + additionally gets an anti-cheat allowlist mask (see below). The reference + is deliberately NOT here -- it has its own ``CONTAINER_REFERENCE_DIR`` + mount and is masked out of the task_dir mount (see ``_reference_mount_args``). + """ + # ``mounted`` dedupes overlapping bind entries. + mounted: set[Path] = set() + # Sources that look like credential / secret dirs get a loud warning: + # `plugin.path` / `template_sources` are user-controlled strings and a typo + # (or a hostile suite) can silently expose `~/.ssh`. Warn, not hard-fail -- + # legitimate uses exist (a task that does want `~/.aws/config`). + sensitive_sources = self._sensitive_source_paths() + + # Lazy import: eval_material -> agents._skills triggers agents/__init__, + # which imports back into this module (opencode_agent). Importing it here, + # after this module is fully initialised, breaks that cycle. + from coder_eval.isolation.eval_material import mask_dirs + + # ANTI-CHEAT masks, COLLECTED here and emitted AFTER every bind is known. + # Deferred so a nested auto-mounted plugin root (plugin B under plugin A) + # is reconciled: A's mask would `--tmpfs /B` while B's own mount does + # `-v /B:...:ro` -- an identical Docker mount destination, which the + # daemon rejects ("Duplicate mount point"). The bind must win (so B loads + # and masks its OWN non-skill children), so a mask whose path is also a + # bind is dropped below. Maps masked dir -> its plugin root (for logging). + mask_targets: dict[Path, Path] = {} + + def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: + if not raw_path: + return + resolved = self._resolve_mount_path(raw_path) + # File paths get mounted as the parent dir so a single -v covers + # the file; container-side reads still resolve at the same path. + target = resolved if (dir_only or resolved.is_dir()) else resolved.parent + if target in mounted or not target.is_dir(): + return + for sensitive in sensitive_sources: + if target == sensitive or sensitive in target.parents: + logger.warning( + "Auto-mounting sensitive host path %s into container; fix task YAML if unintended.", + target, + ) + break + mounted.add(target) + argv.extend(["-v", f"{target}:{target}:ro"]) + # ANTI-CHEAT (allowlist / default-deny): if `target` is a Claude-plugin + # root, the plugin stays mounted whole (:ro, above) so it still loads, + # but every child dir that is NOT the plugin surface (.claude-plugin + + # the manifest-declared skill dirs) is masked with an empty tmpfs. This + # closes the whole-suite channel: sibling task YAMLs, reference + # solutions, and test fixtures colocated under the tree are masked by + # default. `mask_dirs` returns [] for a non-plugin root, so a plain + # template dir / system_prompt_file parent is untouched. + masks = mask_dirs(target) + if not masks and (target / ".claude-plugin" / "plugin.json").is_file(): + # A plugin root whose whole tree is the skill surface (e.g. a + # manifest declaring `skills: "."`) stands the mask down. Say so, + # or the anti-cheat mask voids with no operator-visible signal. + logger.warning(_MASK_STANDDOWN_WARNING, target) + for masked_dir in masks: + mask_targets.setdefault(masked_dir, target) + + plugins = (self.rt.task.agent.plugins if self.rt.task.agent else None) or [] + for plugin in plugins: + _auto_mount(plugin.get("path") if isinstance(plugin, dict) else None) + + from coder_eval.models import TemplateDirSource + + sandbox_cfg = self.rt.task.sandbox + for source in (sandbox_cfg.template_sources or []) if sandbox_cfg else []: + if isinstance(source, TemplateDirSource): + _auto_mount(source.path) + + # Defensive: system_prompt_file is normally inlined into system_prompt by + # load_task / experiment resolution, but a variant could inject an absolute + # path that survives. Cover it so the in-container Orchestrator can read it. + agent_cfg = self.rt.task.agent + if agent_cfg and agent_cfg.system_prompt_file: + _auto_mount(agent_cfg.system_prompt_file, dir_only=False) + + # Emit the anti-cheat masks now that every bind is known. Docker applies + # mounts by target-path depth, so a deeper --tmpfs wins over the enclosing + # :ro bind regardless of argv order. Skip a mask whose path is ALSO a bind + # (a nested auto-mounted plugin root, see mask_targets above): the bind + # wins so the nested plugin loads and masks its own non-skill children. + for masked_dir, root in sorted(mask_targets.items()): + if masked_dir in mounted: + continue + argv.extend(["--tmpfs", str(masked_dir)]) + logger.warning(_MASK_WARNING, masked_dir, root) + def _build_argv( self, input_dir: Path, output_dir: Path, *, container_name: str, image: str | None = None ) -> list[str]: @@ -1729,82 +1830,7 @@ def _build_argv( host_claude_dir = Path.home() / ".claude" argv += ["-v", f"{self._claude_mount_src}:{host_claude_dir}"] - # Auto-mount host paths the task references so they resolve inside - # the container at the *same* path they have on the host. - # Includes: - # - Claude Code plugin dirs (`agent.plugins[].path`) - # - Template directories (`sandbox.template_sources[].path` for - # TemplateDirSource entries -- already absolute after - # resolve_template_paths runs on the host). - # `run_command` criteria that use `$TASK_DIR/...` are covered by the - # symmetric task_dir mount above. The reference is deliberately NOT here: - # it gets its own mount at CONTAINER_REFERENCE_DIR and is masked out of - # the task_dir mount (see _reference_mount_args). ``mounted`` dedupes overlapping entries. - mounted: set[Path] = set() - # Auto-mount sources that look like credential / secret dirs get a - # loud warning. Task YAMLs typically come from in-house suite authors, - # but the `plugin.path` / `reference.directory` / `template_sources` - # fields are user-controlled strings, and a typo (or a hostile suite) - # can silently expose `~/.ssh` etc. Warning, not hard fail, because - # legitimate uses exist (a task that does in fact want to read - # `~/.aws/config`). The warning surfaces the surprise. - sensitive_sources = self._sensitive_source_paths() - - # Lazy import: eval_material -> agents._skills triggers agents/__init__, - # which imports back into this module (opencode_agent). Importing it here, - # after this module is fully initialised, breaks that cycle. - from coder_eval.isolation.eval_material import mask_dirs - - def _auto_mount(raw_path: str | None, *, dir_only: bool = True) -> None: - if not raw_path: - return - resolved = self._resolve_mount_path(raw_path) - # File paths get mounted as the parent dir so a single -v covers - # the file; container-side reads still resolve at the same path. - target = resolved if (dir_only or resolved.is_dir()) else resolved.parent - if target in mounted or not target.is_dir(): - return - for sensitive in sensitive_sources: - if target == sensitive or sensitive in target.parents: - logger.warning( - "Auto-mounting sensitive host path %s into container; fix task YAML if unintended.", - target, - ) - break - mounted.add(target) - argv.extend(["-v", f"{target}:{target}:ro"]) - # ANTI-CHEAT (allowlist / default-deny): if `target` is a Claude-plugin - # root, the plugin stays mounted whole (:ro, above) so it still loads, - # but every child dir that is NOT the plugin surface (.claude-plugin + - # the manifest-declared skill dirs) is masked with an empty tmpfs. This - # closes the whole-suite channel: sibling task YAMLs, reference - # solutions, and test fixtures colocated under the tree are masked by - # default. `mask_dirs` returns [] for a non-plugin root, so a plain - # template dir / system_prompt_file parent is untouched. Docker applies - # mounts by target-path depth, so the deeper --tmpfs wins over the :ro - # bind regardless of argv order. - for masked_dir in mask_dirs(target): - argv.extend(["--tmpfs", str(masked_dir)]) - logger.warning(_MASK_WARNING, masked_dir, target) - - plugins = (self.rt.task.agent.plugins if self.rt.task.agent else None) or [] - for plugin in plugins: - _auto_mount(plugin.get("path") if isinstance(plugin, dict) else None) - - from coder_eval.models import TemplateDirSource - - sandbox_cfg = self.rt.task.sandbox - for source in (sandbox_cfg.template_sources or []) if sandbox_cfg else []: - if isinstance(source, TemplateDirSource): - _auto_mount(source.path) - - # Defensive: system_prompt_file is normally inlined into - # system_prompt by load_task / experiment resolution, but a variant - # could conceivably inject an absolute path that survives. Cover - # that path so the in-container Orchestrator can read it. - agent_cfg = self.rt.task.agent - if agent_cfg and agent_cfg.system_prompt_file: - _auto_mount(agent_cfg.system_prompt_file, dir_only=False) + self._append_auto_mounts(argv) # NOTE: task.reference.directory is deliberately NOT auto-mounted at its # host path here. A copy of it gets a single dedicated read-write mount diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 51f8fb7e..e03414dd 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -2244,10 +2244,12 @@ async def _communicate_attempt() -> TurnRecord: # and -- for a task laid out flat, whose parent is the whole `tasks/` # tree -- every SIBLING task's reference solution. # - # What this does NOT do is hide the task DEFINITION. `task.yaml` is also - # staged at /work/input for the in-container orchestrator to read, and - # that mount is untouched by this window. Hiding the criteria from the - # agent remains a separate, unsolved problem. + # This window shields grading MATERIAL in the task dir (reference/, + # fixtures). The task DEFINITION itself -- `task.yaml` plus `context.json`'s + # `source_yaml`, both carrying `success_criteria` -- is staged at + # /work/input, not covered by this window, but DELETED after load by the + # in-container entry point (`run_task_internal_command._scrub_staged_inputs`), + # so the agent cannot read its own criteria from there either. assert self.sandbox is not None async with self.sandbox.set_permissions([self._reference_dir, self.sandbox.task_dir]): turn_record = await execute_with_retry( diff --git a/tests/test_docker_runner_mounts.py b/tests/test_docker_runner_mounts.py index d824de6c..f33d125d 100644 --- a/tests/test_docker_runner_mounts.py +++ b/tests/test_docker_runner_mounts.py @@ -841,6 +841,72 @@ def test_no_plugins_no_mask(self, tmp_path: Path): assert self._tmpfs(argv) == [] + def test_nested_plugin_root_bind_wins_over_mask(self, tmp_path: Path): + # M2: plugin B nested under plugin A. A's mask would `--tmpfs /nested_b` + # while B's mount does `-v /nested_b:...:ro` -- an identical Docker mount + # destination the daemon rejects. The bind must win (B loads + masks its + # own children), so A's mask of B is dropped. + a = self._plugin_root(tmp_path / "plugin_a") + (a / "skills" / "demo").mkdir(parents=True) + (a / "tests").mkdir() + b = self._plugin_root(a / "nested_b") + (b / "skills" / "demo").mkdir(parents=True) + (b / "tests").mkdir() + + runner = self._runner( + tmp_path, + plugins=[{"type": "local", "path": str(a)}, {"type": "local", "path": str(b)}], + ) + argv = self._argv(runner, tmp_path) + mounts, tmpfs = self._mounts(argv), self._tmpfs(argv) + + # B is bind-mounted (so it loads) and NOT tmpfs-masked (no duplicate dest). + assert f"{b.resolve()}:{b.resolve()}:ro" in mounts + assert str(b.resolve()) not in tmpfs + # A's own non-skill child is still masked; B masks its own. + assert str((a / "tests").resolve()) in tmpfs + assert str((b / "tests").resolve()) in tmpfs + # No --tmpfs target collides with a bind destination (the M2 crash). + bind_dests = {m.split(":")[1] for m in mounts if m.count(":") >= 2} + assert not (set(tmpfs) & bind_dests) + + def test_skills_at_root_logs_mask_standdown(self, tmp_path: Path, caplog): + # M3: a plugin root whose whole tree is the skill surface (manifest + # `skills: "."`) voids the mask -- warn so it isn't silent. + root = self._plugin_root(tmp_path / "plugin", skills=".") + (root / "tests").mkdir() + + runner = self._runner(tmp_path, plugins=[{"type": "local", "path": str(root)}]) + with caplog.at_level("WARNING"): + argv = self._argv(runner, tmp_path) + + assert self._tmpfs(argv) == [] # nothing masked + assert any("stood down" in r.getMessage() for r in caplog.records), [r.getMessage() for r in caplog.records] + + # --- M1: _resolve_mount_path (the L2 relative-vs-CWD resolution) --- + + def test_resolve_mount_path_relative_uses_task_file_dir(self, tmp_path: Path): + runner = self._runner(tmp_path) + runner.rt.task_file = tmp_path / "suite" / "task.yaml" + (tmp_path / "suite" / "plugin").mkdir(parents=True) + assert runner._resolve_mount_path("plugin") == (tmp_path / "suite" / "plugin").resolve() + + def test_resolve_mount_path_absolute_is_unchanged(self, tmp_path: Path): + runner = self._runner(tmp_path) + runner.rt.task_file = tmp_path / "suite" / "task.yaml" + abs_path = tmp_path / "elsewhere" + assert runner._resolve_mount_path(str(abs_path)) == abs_path.resolve() + + def test_resolve_mount_path_none_task_file_falls_back_to_cwd(self, tmp_path: Path): + runner = self._runner(tmp_path) + runner.rt.task_file = None + assert runner._resolve_mount_path("rel") == (Path.cwd() / "rel").resolve() + + def test_resolve_mount_path_expands_env_vars(self, tmp_path: Path, monkeypatch): + monkeypatch.setenv("MY_PLUGIN_DIR", str(tmp_path / "pdir")) + runner = self._runner(tmp_path) + assert runner._resolve_mount_path("$MY_PLUGIN_DIR") == (tmp_path / "pdir").resolve() + class TestReferenceMountAntiCheat: """The reference must reach the harness but never the agent under evaluation."""