feat(sandbox): per-source template exclude_patterns and host-side grading convention - #91
feat(sandbox): per-source template exclude_patterns and host-side grading convention#91dmorosanu wants to merge 1 commit into
Conversation
uipreliga
left a comment
There was a problem hiding this comment.
Review: coder_eval — pr:91
Scope: pr:91 · branch feat/template-exclude-patterns · 3b31224 · 2026-08-06T17:49Z · workflow variant
Change class: complex — adds a new per-source template exclude_patterns field with terminal precedence semantics over include/ignore patterns, changes the template-copy control flow in Sandbox._apply_template_dir_source, and rewrites a task's grading command to execute a host-side pristine test copy; correctness requires reasoning about pattern matching precedence and host/sandbox boundary.
Architecture, error handling, and harness design are excellent (10/10 across three axes) and the new exclude_patterns feature is well-tested in the shapes it covers, but the grading boundary this PR exists to establish is not actually airtight — a sandbox-writable pytest.py can force a passing verdict, a bare-directory exclude silently copies the whole grading tree in anyway, and the docs oversell both TASK_DIR and docker "containment" — so the bottom line is a healthy codebase with a handful of fail-open trust-boundary gaps that must be closed before task authors rely on the documented guarantees.
Summary
| Axis | Score | 🔴 | 🟠 | 🟡 | 🔵 | Top Issue |
|---|---|---|---|---|---|---|
| 1. Code Quality & Style | 9 / 10 | 0 | 0 | 2 | 0 | Docs' exclude_patterns grading-oracle example **/*.expected cannot match template-root files under fnmatch, contradicting the semantics stated at docs:584 and leaking top-level grading oracles |
| 2. Type Safety | 9.9 / 10 | 0 | 0 | 0 | 1 | info.field_name (`str |
| 3. Test Health | 9.5 / 10 | 0 | 0 | 1 | 0 | exclude_patterns test coverage gaps in tests/test_sandbox_templates.py: only the both-patterns form is exercised (directory-only pattern and non-default mount_point interaction untested) |
| 4. Security | 8 / 10 | 0 | 2 | 0 | 0 | Grading command's import pytest resolves from the agent-writable sandbox cwd, so a dropped pytest.py stub forces a passing verdict (and the 'graded against the pristine host copy' contract is untested) |
| 5. Architecture & Design | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 6. Error Handling & Resilience | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 7. API Surface & Maintainability | 7.9 / 10 | 0 | 2 | 0 | 1 | Docs claim TASK_DIR is set for pre_run / post_run, but _run_command_list spawns them with no env= (orchestrator.py:2165) |
| 8. Evaluation Harness Quality | 10 / 10 | 0 | 0 | 0 | 0 | — |
Overall Score: 9.3 / 10 · Weakest Axis: API Surface & Maintainability at 7.9 / 10
Totals: 🔴 0 · 🟠 4 · 🟡 3 · 🔵 2 across 8 axes.
Blockers
- [Axis 4] Grading command's
import pytestresolves from the agent-writable sandbox cwd, so a droppedpytest.pystub forces a passing verdict (and the 'graded against the pristine host copy' contract is untested) (tasks/fibonacci_with_template.yaml:42) — Line 42-44 iscommand: >-/python -B -c "import os, pathlib, sys, pytest; ... sys.exit(pytest.main([...]))".sandbox.run_commandexecutes it withcwd=self.sandbox_dir(src/coder_eval/sandbox.py:1079) andpython -cprepends''(cwd) tosys.path, soimport pytestresolves to<sandbox>/pytest.pyif the agent wrote one. Reproduced: withsrc/main.pyreturning 999 and a 5-line<sandbox>/pytest.pydefiningdef main(args=None): return 0, the exact command exits 0 (criterion passes); removing the shim gives3 failed. This defeats the stated guarantee in the YAML comment on line 31-32 ("so edits to the sandbox copy cannot move the bar") and lets agent-controlled code run in the grading process, which inherits the parent environment (_build_run_command_env, src/coder_eval/sandbox.py:941: "Inherit parent env (so agent tools / credentials remain reachable)"). Fix: keep cwd off the front ofsys.path— runpython -P -B -c "import os, pathlib, sys; sys.path.append(os.getcwd()); import pytest; ..."(append, so the agent'ssrc/is still importable but stdlib/pytest are not shadowable), and setPYTEST_DISABLE_PLUGIN_AUTOLOAD=1so a plugin pip-installed by the agent into the sandbox.venvcannot register apytest11hook that forces a pass. CVSS:3.1/AV:L/AC:L/PR:N/UI:N/S:C/C:N/I:H/A:N - [Axis 4]
exclude_patternshas no subtree-prune semantics: a bare directory pattern withholds only the directory entry while its entire contents (grading material) still copy into the sandbox (src/coder_eval/sandbox.py:361) — Line 361 isif self._matches_template_exclude_pattern(rel_path, source.exclude_patterns): continue, and_matches_template_exclude_pattern(line 629) delegates to_matches_template_include_pattern, which only doesfnmatch.fnmatchcase(rel_path.as_posix(), pattern)on the full relative path. Becausetemplate_path.rglob("*")(line 354) enumerates descendants independently of their parent, skipping thegradingdirectory entry does not skipgrading/expected.json— anddest_path.parent.mkdir(parents=True, exist_ok=True)(line 415) recreates the directory anyway. Reproduced againstSandbox.setup():exclude_patterns=['grading']→ sandbox containsgrading/expected.jsonandgrading/fixtures/oracle.txt;['grading/*']→ nothing leaks. Every comparable tool (gitignore, .dockerignore,rsync --exclude) prunes the subtree, soexclude_patterns: ['grading']is the natural authoring shape and it fails open silently with no warning. Note the PR's own test at tests/test_sandbox_templates.py:352 has to passexclude_patterns=["grading", "grading/*"]to make the assertion hold — the belt-and-braces pattern is masking the defect rather than covering it. Fix: after an exclude match on a directory entry, prune the subtree (matchrel_pathand every ancestor against the exclude list, e.g.any(self._matches_template_exclude_pattern(p, pats) for p in [rel_path, *rel_path.parents] if p != Path('.'))), and add a test assertingexclude_patterns=['grading']alone withholds the descendants. CVSS:3.1/AV:L/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:N - [Axis 7] Docs claim
TASK_DIRis set forpre_run/post_run, but_run_command_listspawns them with noenv=(orchestrator.py:2165) (docs/TASK_DEFINITION_GUIDE.md:664) — The new table row asserts: "|run_commandcriteria (andpre_run/post_run) | TheTASK_DIRenvironment variable is set for every command, so\"$TASK_DIR/verifier/check.py\"resolves host-side …|".TASK_DIRis exported in exactly one place —Sandbox._build_run_command_env(src/coder_eval/sandbox.py:986,env["TASK_DIR"] = str(self.task_dir)), which is consumed only bySandbox.run_command(sandbox.py:1069,env = self._build_run_command_env()).pre_run/post_rundo NOT go through it:Orchestrator._run_command_listspawns them withawait asyncio.create_subprocess_shell(cmd.command, cwd=str(sandbox_dir), stdout=…, stderr=…, limit=…)(src/coder_eval/orchestrator.py:2180-2186) with noenv=argument, so they inherit the orchestrator'sos.environ, and nothing in orchestrator.py or sandbox.py ever assignsos.environ["TASK_DIR"]. Fix the docs: restrict the row torun_commandcriteria, or (preferred, since the doc is teaching a convention) plumb the sandbox env into_run_command_listby passingenv=self.sandbox._build_run_command_env()(promoting it to a public accessor) so the documented contract actually holds for all three surfaces. Note the failure mode is silent, not loud: undersh,$TASK_DIRexpands to the empty string, so a documented"$TASK_DIR/verifier/check.py"becomes/verifier/check.py— and forpre_run, whosefail_on_errordefaults to True, that lands the whole run asFinalStatus.ERRORwith a misleading reason. - [Axis 7] docs:689 'Containment' claim is false: the docker driver bind-mounts the task dir and every template_dir (unfiltered by exclude_patterns) read-only at identical host paths and ships no UID/GID isolation (
docs/TASK_DEFINITION_GUIDE.md:689) — Line 689 asserts: "- Containment - thedockerdriver's UID/GID isolation, which makes host paths genuinely unreachable rather than merely unadvertised." and line 660 asserts "The task directory is never copied into the sandbox, so the agent has no path to it". Under thedockerdriver both grading-asset locations this new section is about are bind-mounted into the agent's own container at their identical host paths: the task dir at src/coder_eval/isolation/docker_runner.py:1176 (argv += ["-v", f"{host_task_dir}:{host_task_dir}:ro"]) and every template dir at docker_runner.py:1233-1236 (for source in … if isinstance(source, TemplateDirSource): _auto_mount(source.path)→argv.extend(["-v", f"{target}:{target}:ro"])at line 1226). The consequence for the new public field is concrete:exclude_patternswithholds a path only from the sandbox copy made inSandbox._apply_template_dir_source, while the whole unfiltered template tree — including the excluded grading files — stays readable by the agent at the template's absolute host path inside the container. So the field's own description ("Use it to keep grading material (expected outputs, reference solutions, verifier fixtures) out of the agent's working copy", src/coder_eval/models/templates.py:59-67) is honoured only ontempdir, and the docs point the reader at exactly the driver where the boundary is weakest. Either scope the claims ("underdockerthe task dir and everytemplate_dirpath are mounted read-only at their host paths, so grading assets there ARE reachable — docker isolates writes and UID, not reads of these mounts"), or stop auto-mounting the template source dir when the source declaresexclude_patterns(copy the filtered tree into the container instead).
Non-blocking, but please consider before merge
- [Axis 1] Docs'
exclude_patternsgrading-oracle example**/*.expectedcannot match template-root files under fnmatch, contradicting the semantics stated at docs:584 and leaking top-level grading oracles (docs/TASK_DEFINITION_GUIDE.md:594) — Line 588 states the rule: "Both take template-relative glob patterns matched withfnmatch, where*does not stop at/". Line 594 then givesexclude_patterns: ["grading", "grading/*", "**/*.expected"]. Underfnmatch,**is just two*s and the literal/in the pattern is still required, so**/*.expectedmatches only paths that contain a slash. Verified directly:fnmatch.fnmatchcase('report.expected', '**/*.expected')→False;fnmatch.fnmatchcase('a/report.expected', '**/*.expected')→True;fnmatch.translate('**/*.expected')→(?s:(?>.*?/).*\.expected)\z. A task author who copies this example leaves every top-level*.expectedgrading file in the sandbox — the exact failure the section exists to prevent. Under the documented semantics the correct recursive form is simply*.expected(fnmatch.fnmatchcase('report.expected', '*.expected')→True). Change the example toexclude_patterns: ["grading", "grading/*", "*.expected"]and add one sentence saying gitignore's**has no special meaning here. (tests/test_sandbox_templates.py:379propagates the same**idiom ininclude_patterns=["tools/*/dist", "tools/*/dist/**"]; that one happens to work, but it teaches the same wrong mental model.) - [Axis 1] mock_path_dirs anti-shortcut/anti-gaming rationale is restated across four shipped files and overstates what the mocks actually enforce (the scripts ship into the sandbox, so the asserted receipts are statically derivable) (
tasks/mock_path_dirs_template_dir/mock-cli-bins/mocks/say_hello:2) — The same paragraph is now restated in four places:mocks/say_hello:2-5,mocks/echo_args:2-4,mock-cli-bins/README.md:22-25, andtask.yaml:32-35— four copies of one rationale that must now be kept in sync. The claim itself is also stronger than the code supports.say_hello:4-5asserts "Reading the source is not a substitute for executing the mock", but the receipt is fully deterministic and derivable from the shipped source: line 11 isprintf 'MOCK_PATH_OK say_hello argc=%s argchars=%s\n' "$argc" "$argchars", andargc/argcharsare plain$#/${#arg}arithmetic over the arguments the prompt itself specifies (say_hello world→argc=1 argchars=5), so the asserted string is reconstructible by reading alone.mock-cli-bins/README.mdis copied into the sandbox root (task.yaml:14-16copies the whole./mock-cli-binstree), so its new paragraph hands the agent the composition rule as well. Keep one authoritative copy of the rationale (thetask.yamlcomment, which never enters the sandbox), delete the other three, and either soften the claim to what is true ("the expected text does not appear verbatim") or use theexclude_patternsthis PR just added to withholdREADME.mdfrom the copy. - [Axis 3] exclude_patterns test coverage gaps in tests/test_sandbox_templates.py: only the both-patterns form is exercised (directory-only pattern and non-default mount_point interaction untested) (
tests/test_sandbox_templates.py:353) — All four new tests use pattern forms that happen to work, so the feature's principal footgun is masked.test_template_exclude_patterns_withhold_nested_pathsat line 353 writesexclude_patterns=["grading", "grading/*"]— the both-patterns form. Nothing covers the natural single-pattern form a task author will write first.
Verified against the PR state with a real Sandbox.setup(): exclude_patterns=["grading"] yields ['grading/expected.json', 'grading/fixtures/oracle.txt', 'src/main.py'] — the entire grading tree lands in the sandbox. The continue at src/coder_eval/sandbox.py:361-362 skips only the directory entry; rglob("*") still yields its children and dest_path.parent.mkdir(parents=True, exist_ok=True) (line 418) recreates the directory. The one failure mode the feature exists to prevent therefore succeeds silently, with a green test suite.
Second instance of the same gap: the doc example added at docs/TASK_DEFINITION_GUIDE.md:594, exclude_patterns: ["grading", "grading/*", "**/*.expected"], is never exercised. fnmatch.fnmatchcase("foo.expected", "**/*.expected") is False (only a/foo.expected matches), so a top-level answers.expected copies through despite the documented pattern.
Fix: add a test asserting the directory-only form's actual behavior (either pin the leak as known-and-documented, or make a directory exclude prune its subtree and assert that), plus a test that walks the exact pattern list from the docs example over a fixture containing both answers.expected and nested/answers.expected. Pattern semantics that decide whether grading material reaches the agent should be pinned by assertion, not by prose.
Nits
- [Axis 2]
info.field_name(str | None) is interpolated unnarrowed into three validator messages, and no test pins the field name (src/coder_eval/models/templates.py:85) —src/coder_eval/models/templates.py:85binds the optional directly and lines 88/90/92 interpolate it:
def _validate_patterns(cls, v: list[str], info: ValidationInfo) -> list[str]:
field = info.field_name
...
raise ValueError(f"{field} entries must not be empty")ValidationInfo.field_name is annotated str | None (confirmed by reading the installed pydantic 2.12.5 pydantic_core.core_schema.ValidationInfo protocol), and field is never narrowed. Runtime is correct today — I executed the model at PR state and got include_patterns entries must not be empty / exclude_patterns entries must be relative, got: '/x' — but the type system permits None entries must not be empty, and no static check can catch that (f-strings accept object; pyright reports 0 diagnostics on this file in standard mode, and strict would not differ). The two validator tests, tests/test_sandbox_templates.py:331 and :418, match only on "must be relative" / "must not be empty" / "must not contain '\.\.', so the interpolated prefix is unasserted and a regression to None would ship green.
Fix: narrow at the binding — field = info.field_name or "pattern" — and tighten one assertion per field to include the prefix, e.g. pytest.raises(ValueError, match="exclude_patterns entries must be relative"). That converts the untestable Optional into a checked contract and makes the generalized two-field validator's whole point (naming the offending field) actually load-bearing.
2. [Axis 7] Public exclude_patterns field description points at a private method name task authors cannot see (src/coder_eval/models/templates.py:66) — The field description ends "copy. \` does not stop at `/`, see `_matches_template_exclude_pattern`."(line 66). This description is the user-facing surface — it is what shows up in the generated JSON schema and in any YAML-schema tooling a task author uses — and it dead-ends on a privateSandboxmethod that is not importable, not documented, and (per the new docstring at src/coder_eval/sandbox.py:629-637) just delegates to_matches_template_include_pattern. Replace the pointer with the fact itself (e.g. "matched with fnmatch, so crosses/and**has no special meaning; a directory name does not imply its contents") and/or link the doc sectiondocs/TASK_DEFINITION_GUIDE.md#per-source-include_patterns-and-exclude_patterns. The same nit exists on include_patterns(line 56,see `_matches_template_include_pattern`.`) and is pre-existing, but this PR replicates it into a second field rather than fixing it.
What's Missing
Parallel paths:
- 🟠 The new
exclude_patternsfilter has exactly one consumer —Sandbox._apply_template_dir_source(sandbox.py:361). The docker driver's parallel path was not updated:docker_runner.py:1234-1237auto-mounts everyTemplateDirSource.path(and the task dir at :1176) read-only at its identical host path, so underdriver: dockerthe unfiltered template tree — excluded grading oracles included — stays readable in-container even though the sandbox copy is filtered. Either skip the auto-mount for sources declaringexclude_patterns(copying the filtered tree in instead) or scope the field description/docs totempdir. (trigger: src/coder_eval/sandbox.py) (restates: Axis 7: docs:689 'Containment' claim is false — docker bind-mounts task dir and template dirs unfiltered) - 🟡
exclude_patternswas added only toTemplateDirSource; the siblingRepoSourcestill clones wholesale via_apply_repo_source(sandbox.py:249-268,git clonewith no ignore/include/exclude filtering at all), andStarterFilesSourcehas no equivalent. The new docs sentence "Where a template mixes working files and grading files in one tree,exclude_patternswithholds the grading paths" (TASK_DEFINITION_GUIDE.md:681) is therefore unimplementable for arepo-sourced task — the most likely shape for a real project that carries its own tests/answers. Either add the same filter toRepoSourceor scope the guidance totemplate_direxplicitly. (trigger: src/coder_eval/models/templates.py) - 🟡 The anti-shortcut rewrite was applied to
mock_path_dirs_template_dirbut not to its siblingtasks/mock_path_dirs_smoke.yaml, which still ships an inlinestarter_filesmock whose body isecho "MOCK_PATH_OK from say_hello, args=$*"while the criterion assertsincludes: ["MOCK_PATH_OK from say_hello"]— the criterion string sits verbatim in a file the agent can read, i.e. exactly the gaming path this PR closed in the template_dir twin. Apply the same runtime-computed receipt (or drop the claim that this shape is hardened). (trigger: tasks/mock_path_dirs_template_dir/task.yaml) - 🔵
tasks/fibonacci_with_template.yamlnow names the template location twice in two idioms —path: "../templates/fibonacci-starter"(line 20, resolved relative to the task YAML) andpathlib.Path(os.environ['TASK_DIR']).parent / 'templates' / 'fibonacci-starter'(line 43, re-derived by hand). Renaming or moving the template updates one and silently leaves the grader pointing elsewhere. If this convention is to be reused, expose the resolved template root torun_command(e.g. aTEMPLATE_DIR-style env or a documented helper) instead of duplicating the path arithmetic in every task that adopts it. (trigger: tasks/fibonacci_with_template.yaml)
Tests:
- 🟠 The PR's headline invariant — "grading runs the PRISTINE host copy … so edits to the sandbox copy cannot move the bar" (tasks/fibonacci_with_template.yaml:29-32) — is asserted only in a YAML comment and exercised only by a live agent run. No test drives the new command over a synthetic sandbox to show that (a) a correct
src/main.pypasses, (b) a wrong one fails, and (c) rewriting the sandbox'stests/test_main.pytoassert Truedoes not flip the verdict. That missing test is precisely the one that would have caught the grader-shadowing hole. (trigger: tasks/fibonacci_with_template.yaml) (restates: Axis 4: Grading command'simport pytestresolves from the agent-writable sandbox cwd) - 🟡 Nothing in CI or pytest executes the two rewritten mocks, yet six hand-computed literals now live in
task.yaml(MOCK_PATH_OK say_hello argc=1 argchars=5,ECHO_ARGS_OK argc=3 argchars=11,arg[3]=one,arg[3]=two,arg[5]=three). I verified they match today by running the scripts, but any edit tosay_hello/echo_args(or to the prompt's argv) silently desynchronizes criteria from producers —pr-checks.ymlonly runs--tags smoke-pass|smoke-fail|smoke-variants, and these tasks are taggedmock, template-dir(andgolden, basicfor fibonacci), so neither is executed anywhere. Cheap fix: a unit test thatsh-executes both mocks with the prompt's arguments and asserts everyincludes:string parsed out oftask.yaml. (trigger: tasks/mock_path_dirs_template_dir/mock-cli-bins/mocks/echo_args) - 🟡 No test covers
exclude_patterns× symlinks, and the interaction defeats the documented "exclusion is terminal" guarantee. Reproduced againstSandbox.setup()at PR state: a template containingoracle_abs -> <template>/grading/expected.jsonwithexclude_patterns=["grading", "grading/*"]yields a sandbox containingoracle_abswhoseread_text()returns the excluded oracle (the symlink branch at sandbox.py:376-408 copiesos.readlinkverbatim, by design for host passthrough). The relative-link variant merely dangles, so only the absolute form leaks — but nothing warns, tests, or documents it. (trigger: tests/test_sandbox_templates.py) (restates: Axis 4:exclude_patternshas no subtree-prune semantics) - 🟡 The four new tests only exercise pattern shapes that happen to work: no test for the bare-directory form
exclude_patterns=["grading"](which leaks the whole subtree), none for the exact pattern list the new docs example teaches (**/*.expected, which misses top-level files), and none forexclude_patternscombined with a non-defaultmount_point. Pattern semantics that decide whether grading material reaches the agent should be pinned by assertion, not by prose. (trigger: tests/test_sandbox_templates.py) (restates: Axis 3: exclude_patterns test coverage gaps (directory-only pattern, mount_point interaction)) - 🟡 The new grading-surface table claims
TASK_DIRis set forpre_run/post_runas well asrun_commandcriteria, but test coverage stops atrun_command(tests/test_sandbox.py:124set /:142absent). There is no test asserting apre_runcommand can see$TASK_DIR— which is why the docs row could ship stating a contractOrchestrator._run_command_listdoes not implement. (trigger: docs/TASK_DEFINITION_GUIDE.md) _(restates: Axis 7: Docs claim TASK_DIR is set for pre_run / post_run, but run_command_list spawns them with no env=) - 🔵 The new criterion's comment justifies its
python -cshape by cross-shell portability ("behaves identically under sh and cmd.exe"), but the Windows CI job runs onlytasks/hello_date.yaml(pr-checks.yml:306), so the claim is never exercised. Either addfibonacci_with_templateto the Windows smoke step or soften the comment to "intended to". (trigger: tasks/fibonacci_with_template.yaml)
Display & mapping dicts:
- 🟡 Exclusion is invisible on every output surface: the
continueat sandbox.py:361 emits no log line, no warning when an exclude pattern matches zero paths, and no copy manifest inrun.json/ the report — compare the adjacent overwrite logging at sandbox.py:419-424. For a safety-relevant, fail-open filter this means a typo'd pattern (grading/vsgrading/*, or the docs' own**/*.expected) is indistinguishable from a working one: the task still runs green while the oracle sits in the sandbox. Add at least a debug log of withheld paths plus a warning for patterns that matched nothing. (trigger: src/coder_eval/sandbox.py)
Downstream consumers:
- 🔵
tasks/README.mdwas not updated alongside the reshaped task: rows 29 and 65 still describemock_path_dirs_template_dir/as a "Template dir (mock CLI bins) consumed bymock_path_dirs_smoke", when it is a standalone task with its own criteria (andmock_path_dirs_smokeuses inlinestarter_files). The index that points authors at the worked examples now mis-describes the one this PR made canonical. (trigger: tasks/mock_path_dirs_template_dir/task.yaml) - 🔵
exclude_patternsis a new user-authored YAML field, butTemplateDirSourceis outside the set CE030's doc-schema-parity lint tracks (TaskDefinition,RunLimits,Dataset,SimulationConfig— tests/lint/doc_schema_parity.py), so the docs update here is voluntary and the next template-source field can ship undocumented withmake lintgreen. Extending the tracked-model list to the template-source models is the mechanical guard. (trigger: src/coder_eval/models/templates.py)
Daily/nightly:
- 🟡 The PR states no docker-driver blast radius for the new host-side grading convention. It works in-container only because
docker_runnermounts the task dir (:1176) and everyTemplateDirSource.path(:1234-1237) at identical host paths, soTASK_DIR.parent/templates/fibonacci-starterhappens to resolve;fibonacci_with_template.yamlpinsdriver: tempdir, so that coupling is never exercised, and the natural fix for the containment finding (stop auto-mounting a source that declaresexclude_patterns) would break exactly this pattern. State the interaction and add a docker-driver run of the task to the nightly/CI matrix before recommending the convention to task authors. (trigger: tasks/fibonacci_with_template.yaml) - 🔵 The documented convention assumes
TASK_DIRis always present, butSandbox._build_run_command_envsets it only whentask_diris not None (sandbox.py:985-986), i.e. only when anOrchestratorwas constructed with atask_file. Any consumer that drivesOrchestratorprogrammatically (cross-repo eval-runner /coder-eval-uipath-style embedding) getsos.environ['TASK_DIR']→KeyErrorand a traceback in place of a grading verdict. Docs should say the convention is CLI-entry-point-only, and the example should useos.environ.get(...)with an explicit error. (trigger: docs/TASK_DEFINITION_GUIDE.md)
Harness & Lint Improvements
Static checks (lint / type):
- [ce-lint] CE032 — template glob patterns must be fnmatch-honest. Promote
TemplateDirSource._validate_patterns(src/coder_eval/models/templates.py:81-96) to a module-levelvalidate_template_pattern(field, pattern)(the model keeps calling it), and have it additionally reject the two shapes that silently fail underfnmatch: (a) any entry containing**(fnmatch has no recursive-glob semantics —fnmatch.translate('**/*.expected')still requires a literal/, so the correct recursive form is plain*.expected); (b) a wildcard-free entry that names a directory, which withholds only the directory entry and not its contents. Then add a whole-tree lint test classTestCE032TemplateGlobPatternsin tests/test_custom_lint.py (CE027/CE029/CE030 precedent — it scans Markdown/YAML, so it is not aBaseRulein tests/lint/runner.py), reusingtests/lint/doc_examples.extract_yaml_blocksto walk every yaml fence indocs/**/*.mdincluding fragments (CE029 skips fragments today, which is exactly why docs:594 shipped), plustasks/**/*.yaml,experiments/**/*.yaml, andinclude_patterns=/exclude_patterns=list literals undertests/, running each entry through the shared validator. Add a<!-- lint-skip: template-glob -->escape mirroring CE029'sSKIP_MARKER. Prevents: F1 (docs/TASK_DEFINITION_GUIDE.md:594exclude_patterns: ["grading", "grading/*", "**/*.expected"], which leaks every template-root*.expectedgrading oracle); the authoring half of F5 (a baregradingentry withholds only the directory entry whilegrading/expected.jsonandgrading/fixtures/oracle.txtstill copy in); and the propagated**idiom at tests/test_sandbox_templates.py:266, 319, 380 (tools/*/dist/**) that teaches the same wrong mental model. - [ce-lint] CE033 — a grading command must not let the agent-writable sandbox cwd shadow its imports. New whole-tree lint test scanning every task-authored command string (
success_criteria[].commandfortype: run_command, pluspre_run[].command/post_run[].command) intasks/**/*.yamland in docs yaml fences; flag any command invokingpython/python3with-cor-mthat lacks-P/-I(orPYTHONSAFEPATH=1in the same string). Escalate when the command also mentionspytest: requirePYTEST_DISABLE_PLUGIN_AUTOLOAD=1, since_build_run_command_envprepends the sandbox venv's scripts dir, so apytest11entry point the agent pip-installs into<sandbox>/.venvis autoloaded into the grading process. Rule docstring rationale:Sandbox.run_commandexecutes withcwd=self.sandbox_dir(src/coder_eval/sandbox.py:1079) andpython -cprepends''tosys.path, so any import in a grading command resolves from a directory the agent fully controls. Prevents: F4 (tasks/fibonacci_with_template.yaml:42-44 — a 5-line agent-authored<sandbox>/pytest.pydefiningdef main(args=None): return 0makes the criterion exit 0 against a deliberately wrongsrc/main.py, defeating that file's own "edits to the sandbox copy cannot move the bar" comment at lines 29-32). Also pre-empts the same shadow in the skillsbench verifier tasks that shell out topython. - [ce-lint] Extend CE015 (tests/lint/rules/ce015_create_subprocess_limit.py) to also require an explicit
env=kwarg onasyncio.create_subprocess_exec/create_subprocess_shell, renaming it to "explicit stream limit and explicit environment". There are exactly two spawn sites insrc/(orchestrator.py, docker_runner.py) and the rule already visits both, so the churn is two lines. Effect: every spawn of a task-authored command must state whether it receives the sandbox environment (Sandbox._build_run_command_env, the sole assigner ofTASK_DIRat sandbox.py:986) or deliberately inheritsos.environ(with# noqa: CE015plus a reason). Env inheritance stops being an invisible default that documentation can drift away from. Prevents: F6 (docs/TASK_DEFINITION_GUIDE.md:664 documentsTASK_DIRas "set for every command" forpre_run/post_run, butOrchestrator._run_command_listspawns them at orchestrator.py:2165-2170 with noenv=; undershthe unset$TASK_DIRexpands to empty, so a documented"$TASK_DIR/verifier/check.py"becomes/verifier/check.py— and sincePreRunCommand.fail_on_errordefaults True, the run silently lands asFinalStatus.ERRORwith a misleading reason). - [ce-lint] CE034 — a documented isolation/containment mechanism must be emitted by the code that would implement it. Direct sibling of CE027's doc↔code env-var parity, implemented as
tests/lint/doc_mechanism_parity.py+TestCE034DocContainmentParity. Keep a small explicit term→evidence map (e.g. "UID/GID isolation" / "runs as a non-root user" → requires--useror--usernsin the argv built bysrc/coder_eval/isolation/docker_runner.py, or aUSERdirective indocker/Dockerfile; "host paths … unreachable" → requires that docker_runner does not bind-mount the task dir or template dirs). Fail when a doc asserts a term whose evidence token is absent fromsrc/. A small explicit registry keeps false positives near zero, exactly as CE027 does. Prevents: F7 (docs/TASK_DEFINITION_GUIDE.md:689 credits "thedockerdriver's UID/GID isolation, which makes host paths genuinely unreachable" —git grep -nE '\"--user\"|--user=|userns'oversrc/anddocker/returns zero hits and the Dockerfile has noUSER, while docker_runner.py:1176 and :1226/1234-1237 bind-mount the task dir and everyTemplateDirSource.pathread-only at their identical host paths, unfiltered byexclude_patterns). Also guards the related false claim at docs:660 ("the agent has no path to it"). - [ce-lint] CE035 — a user-facing
Field(description=...)must not dead-end on a private symbol. A per-fileBaseRulein tests/lint/rules/ce035_field_description_no_private_ref.py, wired intoALL_RULESin tests/lint/runner.py: for anyField(...)call inside aBaseModelsubclass undersrc/coder_eval/models/, flag adescription=string literal containing a backticked_-prefixed identifier (regex`_\w+`). Field descriptions are the JSON-schema surface a task author reads in YAML tooling; a pointer to a private, non-importable, undocumentedSandboxmethod is unresolvable for them. Suggested fix shape in the message: state the fact ("matched withfnmatch, so*crosses/and**has no special meaning") or link the doc anchor. Prevents: F8 (src/coder_eval/models/templates.py:66see \_matches_template_exclude_pattern`on the new publicexclude_patternsfield, plus the pre-existinginclude_patternsinstance at line 56 that this PR replicated rather than fixed). Second-order: that private pointer is precisely what made the**` / bare-directory semantics behind F1 and F5 unlearnable from the schema. - [pyright] Tighten the validator-message idiom so the existing pyright gate does the work: annotate the local as
str. InTemplateDirSource._validate_patterns(src/coder_eval/models/templates.py:85) writefield: str = info.field_name or "pattern"instead offield = info.field_name.ValidationInfo.field_nameisstr | None; with the explicit annotation, any revert to the bare optional becomes a pyright error (str | Nonenot assignable tostr) in the existing standard-mode run — no new rule needed. If the idiom recurs across validators, promote it to a one-lineBaseRule(next free number after CE035) flagginginfo.field_namereaching an f-string orValueErrormessage without anorfallback or astr-annotated binding. Record the boundary explicitly: neither ruff nor pyright flags an optional interpolated into an f-string (f-strings acceptobject), so the annotation is the check. Prevents: F3 ({field} entries must not be empty/… must be relativeat templates.py:88/90/92 can render asNone entries must not be empty; the validator tests at tests/test_sandbox_templates.py:331 and :418 match only the suffix, so a regression toNoneships green — pair the annotation with tightening one assertion per field tomatch="exclude_patterns entries must be relative"). - [bandit-codeql] Custom bandit plugin / CodeQL query: flag a subprocess spawned with the full parent environment into an attacker-writable working directory. Pattern: a
subprocess.*/asyncio.create_subprocess_*call whoseenvderives fromos.environ(directly or via a helper such asSandbox._build_run_command_env, whose own comment at sandbox.py:941 says it inherits the parent env "so agent tools / credentials remain reachable") and whosecwdis a sandbox path. Report it as an information-flow sink so the credential exposure of the grading path is a reviewed, annotated decision; the remediation the query text should suggest is an allowlisted env projection for grading commands (PATH + TASK_DIR + explicitly named vars) instead of wholesale inheritance. Prevents: The escalation half of F4 — once an agent-authoredpytest.py(orsitecustomize.py, orconftest.py) executes inside the grading process, it runs with the evaluator's inherited credentials, turning a scoring-integrity bug into credential exposure (the finding's scope-changeS:Cvector). Also coversagent_judge, which the criterion docstring already flags as running with evaluator credentials.
Harness improvements (not statically reachable):
- Sandbox manifest, materialized and pinned. Add
coder-eval plan --print-sandbox-manifest <task>that runs the real template resolution (ignore_patterns → include_patterns → exclude_patterns → mount_point) over the actual template trees and prints the exact sorted path list the agent will receive; then add a snapshot test asserting the manifest for every in-repo task declaring atemplate_dirwithinclude_patterns/exclude_patterns. Any change to matcher semantics or to a task's patterns must be re-approved as a visible diff of agent-reachable files. Why not static: The leak is emergent from the interaction oftemplate_path.rglob("*")(sandbox.py:354) enumerating descendants independently of their parent, the per-entrycontinueat sandbox.py:361, anddest_path.parent.mkdir(parents=True, exist_ok=True)at sandbox.py:418 recreating the excluded directory. No single source token is wrong — only the resulting file set is, and producing it requires walking a real tree. Prevents: F5 (exclude_patterns=['grading']alone yieldsgrading/expected.jsonandgrading/fixtures/oracle.txtinside the sandbox, fail-open with no warning); F2 (the manifest showsmock-cli-bins/README.md, carrying the mocks' composition rule, landing in the agent's working copy of tasks/mock_path_dirs_template_dir); F1's downstream effect. - Authoring-shape matrix test for include/exclude patterns. Parametrize one test over the shapes a task author actually writes —
grading,grading/,grading/*,grading/**,*.expected,**/*.expected,tools/*/dist— against a fixture tree containing both a root-level and a nested oracle, and assert the exact resulting sandbox file set for each. Pin today's behavior explicitly (including the shapes that leak, if subtree pruning is not implemented) so the semantics live in assertions rather than in a docstring. Why not static:fnmatchsemantics only manifest against a concrete path set; whether**/*.expectedwithholdsreport.expectedis a runtime property of the copy loop, not of the pattern string in isolation. Prevents: The Test-Health finding (all four new tests at tests/test_sandbox_templates.py:353/381/407/423 use the belt-and-braces["grading", "grading/*"]form, masking rather than covering the defect; no test exercises the bare-directory form or any**pattern); F5; F1. - Grader-tamper regression suite. For each in-repo task with a
run_commandgrading criterion, run the criterion twice against a sandbox seeded with a knowingly-wrong solution: once clean (must fail) and once with hostile shims dropped in the sandbox —pytest.py,conftest.py,sitecustomize.py, and a fakepytest11entry point installed into the sandbox venv. The criterion must fail in both. Make this the standing acceptance test for any task that grades against a pristine host copy. Why not static: Whether a shim actually wins depends on the interpreter's resolvedsys.path[0], the venv onPATHfrom_build_run_command_env, and pytest's plugin autoload — all runtime resolution. A lint rule can require-P(CE033); only execution proves the grader is genuinely unshadowable. Prevents: F4, including the variants CE033's flag-check cannot see (aconftest.pyin the graded rootdir, or a plugin autoloaded from the sandbox venv). - Cross-driver reachability parity test (
tempdirvsdocker). One test that, for a task declaringexclude_patterns, asserts the set of paths the agent process can read is identical under both drivers — no excluded template path re-exposed by a bind mount, and the task dir unreadable where the docs say it is. Pair with a code-side guard that refuses (or loudly warns) whendocker_runner._auto_mountwould mount aTemplateDirSourcethat declaresexclude_patterns, since the filtering applies only to the copy. Why not static: The exposure is the composition of a filtered copy (sandbox.py:361) with an unfiltered read-only bind mount at the identical host path (docker_runner.py:1226 / 1234-1237) — it exists only in the running container's filesystem view, not in either source site alone. Prevents: F7 (the docs point readers atdockeras the stronger boundary precisely whereexclude_patternsis honoured least); the driver-dependent half of F5. - A
TASK_DIRcontract smoke task in CI. A tiny in-repo task that reads$TASK_DIRfrom apre_runcommand, apost_runcommand, and arun_commandcriterion, each asserting the variable is non-empty and resolves to a real host file, wired into the existing CI task-smoke job. It either proves the documented contract on all three surfaces or fails the moment one diverges. Why not static: The failure mode is shell expansion of an unset variable to the empty string — no exception, no log line, just a wrong path. Nothing in the source text is detectably wrong; only spawning the shell and observing the expansion reveals it. Prevents: F6 (documented at docs:664 forpre_run/post_run, implemented only forSandbox.run_command). Corroborating signal the test would have made loud: every existing$TASK_DIRconsumer in the repo is arun_commandcriterion, and the skillsbenchpre_runblocks all use$PWDinstead. - Make doc examples behaviorally true, not merely schema-valid. Extend
tests/lint/doc_examples.pywith a second mode for glob-bearing fragments: when a docs yaml fence containsinclude_patterns/exclude_patterns, execute it against a small fixture tree and assert the outcome the surrounding prose claims ("the grading oracle stays on the host"). CE029 today only checks that a complete task/experiment document parses, which is why a schema-valid but semantically broken example shipped. Why not static: The doc's claim is behavioral ("the oracle stays on the host"), not structural; verifying it requires running the copy path. CE032 catches the specific**token; this catches the general case of an example whose stated effect and real effect diverge. Prevents: F1, and the class it belongs to — a documented pattern list whose prose promise the code does not keep. - Answer-leak scan as a
coder-eval planwarning. Cross-check every literal a task's criteria assert (file_contains.contains, plain-stringfile_matches_regex.pattern,classification_matchexpected labels,reference.filecontents) against the materialized sandbox manifest from item 1, and warn when an asserted literal appears verbatim in a file that ships into the sandbox. Add a task-authoring checklist line: anti-gaming rationale belongs in the task YAML comment (never copied), not in assets that ship — and prefer withholding such assets withexclude_patternsover restating the rationale inside them. Why not static: Needs the materialized file set (the manifest) plus the resolved criterion literals after dataset${row.*}substitution — neither is available from a single file's AST or from the YAML text alone. Prevents: The shipped-into-sandbox half of F2 (mock-cli-bins/README.md:22-25states the receipt composition rule inside the agent's working copy) and the general class of tasks that ship their own answer key. The duplicated-prose half of F2 (four paraphrases of one rationale across mocks/say_hello, mocks/echo_args, README.md, task.yaml) is judgment, not mechanically detectable — the checklist line is the honest fallback there.
Top 5 Priority Actions
- Close the grader import shadow in tasks/fibonacci_with_template.yaml:42 —
python -cputs the agent-writable sandbox cwd atsys.path[0], so a 5-linepytest.pystub makes the run_command criterion exit 0 with a wrong solution (verified); switch topython -P -B -c "import os, sys; sys.path.append(os.getcwd()); import pytest; ..."and setPYTEST_DISABLE_PLUGIN_AUTOLOAD=1so an agent-installedpytest11plugin cannot force a pass either. - Make
exclude_patternsprune subtrees in src/coder_eval/sandbox.py:361 — a bareexclude_patterns: ['grading'](the natural authoring shape, and the one every ignore tool prunes) skips only the directory entry whilerglobstill copiesgrading/expected.jsonand line 418 recreates the directory, leaking grading oracles into the agent's working copy with no warning; matchrel_pathplus every ancestor against the exclude list. - Fix the
TASK_DIRclaim at docs/TASK_DEFINITION_GUIDE.md:664 —_run_command_listspawnspre_run/post_runwith noenv=(src/coder_eval/orchestrator.py:2165-2170), so"$TASK_DIR/verifier/check.py"expands to/verifier/check.pyand, sincepre_run.fail_on_errordefaults True, silently aborts the run asFinalStatus.ERROR; either scope the row torun_commandcriteria or plumb_build_run_command_env()into_run_command_list. - Correct the broken worked example at docs/TASK_DEFINITION_GUIDE.md:594 — under the
fnmatchsemantics stated ten lines above at :584,**/*.expectedrequires a literal/and therefore misses every top-level*.expectedoracle (plain*.expectedis the correct recursive form), and add the two missing tests to tests/test_sandbox_templates.py (bare-directory exclude, and the exact documented pattern list over a fixture with both top-level and nested.expectedfiles) so pattern semantics that decide oracle exposure are pinned by assertion rather than prose. - Scope or retract the containment claim at docs/TASK_DEFINITION_GUIDE.md:689 — the docker driver bind-mounts the task dir (src/coder_eval/isolation/docker_runner.py:1176) and every unfiltered
template_dir(docker_runner.py:1226/1234-1237) read-only at identical host paths, and ships no--user/USERUID-GID isolation at all, so excluded grading files stay readable in-container; either state that docker isolates writes not reads, or stop auto-mounting a template source that declaresexclude_patterns.
Stats: 0 🔴 · 4 🟠 · 3 🟡 · 2 🔵 across 8 axes reviewed.
What
Closes the template path by which grading material reaches an evaluated agent's sandbox.
TemplateDirSource.exclude_patterns- a new per-source list of template-relative globs that are never copied into the sandbox. Exclusion is terminal: it beatsinclude_patternsand beats a!-negatedignore_patternsentry, so an excluded path cannot be brought back by either. Matching mirrorsinclude_patternsexactly (fnmatchcaseon the posix form,*spans/, leading./stripped); the validator is now shared between the two fields and rejects absolute paths,..segments, and empty entries.Task-authoring convention docs - new "Grading Assets and the Sandbox Boundary" section in
docs/TASK_DEFINITION_GUIDE.md: grading assets belong next to the task YAML (never copied into the sandbox) and are reached host-side viaTASK_DIRinrun_command/pre_run/post_run,$TASK_DIR/entries inllm_judge/agent_judgefiles:, andreference:. Includes a "Test-data separation and its limits" note stating plainly that under thetempdirdriver the agent runs as the host user, so these mechanisms are hygiene that removes every path the agent is pointed at, not containment (containment is the docker UID/GID work; transcript-level detection is separate).Two in-repo example tasks fixed:
tasks/fibonacci_with_template.yamlgraded by runningpython -m pytest tests/inside the sandbox, so the oracle was agent-writable. It now runs the pristine host copy of the same tests (resolved underTASK_DIR) against the sandbox'ssrc/.tasks/mock_path_dirs_template_dir/task.yamlasserted strings whose template was literally present in the mock scripts copied into the sandbox. The mocks now compose their receipt at runtime from the invocation (argument count and argument lengths), so no asserted line exists verbatim in anything that ships, and the prompt no longer points the agent at the mock sources.Why
Evaluated agents read grading material when it ships into their sandbox, and a criterion that can be satisfied by reading rather than doing stops measuring the skill the task was written for. Sibling PRs cover plugin exposure (#89) and mock fixtures (#90); this one covers templates.
Notes on the fibonacci fix
templates/fibonacci-starter/pytest.ini(new, empty[pytest]) pins the template as its own pytest rootdir. Without it, pointing pytest at the host tests makes it discover the enclosing repo'spyproject.tomland fail on-n auto/ unknown ini options.TASK_DIRis read viaos.environinsidepython -crather than as shell$TASK_DIR.run_commandusesshell=True, which iscmd.exeon Windows where$TASK_DIRdoes not expand - the shell form silently graded every run as a failure there. The env-read form behaves identically undershandcmd.exe.-Band-p no:cacheproviderstop the grading run from writing bytecode or a cache directory into the host template.Validation
tests/test_sandbox_templates.py: exclude beats include, exclude beats!-negated default-ignore, plain nested exclusion, validator rejection of absolute/../empty patterns.Sandbox(setup,run_command) on Windows/cmd.exe and again undersh, across four states: untouched stub (fail), sandbox tests neutered plus wrong implementation (still fail - proves the host copy is what grades), correct implementation with sandbox tests neutered (pass), correct implementation with the sandboxtests/deleted outright (pass). No files written into the repo working tree by the grading run.sh; output matches the asserted strings exactly.task_loader.load_task;tests/test_yaml_migration.py(loads everytasks/**/*.yaml) passes.ruff format --check,ruff check,pyright(0 errors; 1 pre-existing warning inantigravity_agent.py), custom lint (171 passed), full suite3925 passed, 102 skipped, coverage 90.39% (gate 80%). The 2 failures are the known pre-existing Windows symlink-privilege cases (test_build_run_command_env_preserves_external_plugin_tools_dir,test_capture_to_copies_and_tolerates_dangling_symlink).