Skip to content

Commit 56bffad

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

68 files changed

Lines changed: 5167 additions & 1116 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/pr-checks.yml‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,10 @@ jobs:
452452
e2e-smoke:
453453
name: E2E Smoke Tests (Real API)
454454
runs-on: uipath-ubuntu-latest
455-
timeout-minutes: 10
455+
# 15 (was 10): the bucket now includes anti_cheat_reference, a driver: docker
456+
# task that spins its own container on top of the two image builds this job
457+
# already does. Headroom, not an expected duration.
458+
timeout-minutes: 15
456459
# Skip on fork PRs where secrets aren't available
457460
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
458461

@@ -466,12 +469,16 @@ jobs:
466469
AWS_BEARER_TOKEN_BEDROCK: ${{ secrets.AWS_BEARER_TOKEN_BEDROCK }}
467470
AWS_REGION: ${{ secrets.AWS_REGION }}
468471
BEDROCK_MODEL: ${{ secrets.BEDROCK_MODEL }}
469-
# tasks_run for --tags smoke-pass. 6 task files (hello_date, dataset_example,
470-
# smoke_llm_judge, smoke_agent_judge, byod_smoke_test, agentless_smoke_test);
471-
# dataset_example fans out to 2 inline rows, so 7 sub-tasks. If you add/remove a
472-
# smoke-pass task or change the dataset row count, bump these.
473-
EXPECTED_SMOKE_PASS_RUN: "7"
474-
EXPECTED_SMOKE_PASS_SUCCEEDED: "7"
472+
# tasks_run for --tags smoke-pass. 7 task files (hello_date, dataset_example,
473+
# smoke_llm_judge, smoke_agent_judge, byod_smoke_test, agentless_smoke_test,
474+
# anti_cheat_reference); dataset_example fans out to 2 inline rows, so 8
475+
# sub-tasks. If you add/remove a smoke-pass task or change the dataset row
476+
# count, bump these.
477+
#
478+
# anti_cheat_reference lives in a SUBDIRECTORY, which `tasks/*.yaml` does not
479+
# match — the smoke-pass step names its path explicitly. Keep that in sync.
480+
EXPECTED_SMOKE_PASS_RUN: "8"
481+
EXPECTED_SMOKE_PASS_SUCCEEDED: "8"
475482
# smoke-fail bucket: three tasks expected to fail.
476483
# 1. smoke_negative_path: file_contains criterion is unsatisfiable
477484
# (sentinel-string regression detection for success-checker).
@@ -538,9 +545,13 @@ jobs:
538545
# that Bedrock rejects with 400 (no such cross-region profile). Falling
539546
# back to BEDROCK_MODEL — a valid pre-formatted Bedrock profile id — is
540547
# the same pattern live-tests uses (see test_claude_settings_enforcement_live._model_for_env).
548+
# `tasks/*.yaml` is NOT recursive, so subdirectory tasks are listed
549+
# explicitly. anti_cheat_reference is the adversarial probe that the agent
550+
# cannot read the reference solution during its turn; it needs the
551+
# coder-eval-agent image built above (it is a driver: docker task).
541552
- name: Run smoke-pass bucket (expect all to succeed)
542553
run: |
543-
.venv/bin/coder-eval run tasks/*.yaml \
554+
.venv/bin/coder-eval run tasks/*.yaml tasks/anti_cheat_reference/*.yaml \
544555
--tags smoke-pass \
545556
--run-dir runs/ci-smoke-pass
546557

‎CLAUDE.md‎

Lines changed: 5 additions & 1 deletion
Large diffs are not rendered by default.

‎Makefile‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,21 @@ clean: ## Clean build artifacts and cache
9595
rm -rf runs/2025-* runs/latest
9696
find . -type d -name __pycache__ -exec rm -rf {} + 2>/dev/null || true
9797

98+
# Task globs. `tasks/*.yaml` does NOT recurse, so every subdirectory holding a
99+
# task must be listed. Kept identical to the CI e2e-smoke job's globs
100+
# (.github/workflows/pr-checks.yml) and pinned there by
101+
# tests/test_tags.py::TestCiSmokePassContract -- when the two drifted, `make
102+
# test-smoke` silently skipped tasks CI was gating on.
103+
TASK_GLOBS := tasks/*.yaml tasks/agents/*.yaml tasks/anti_cheat_reference/*.yaml
104+
SMOKE_GLOBS := tasks/*.yaml tasks/anti_cheat_reference/*.yaml
105+
98106
run: ## Run coder-eval on all tasks with 8 parallel jobs
99-
uv run coder-eval run tasks/*.yaml tasks/agents/*.yaml -j 8
107+
uv run coder-eval run $(TASK_GLOBS) -j 8
100108

101109
test-smoke: ## Run e2e smoke tests with real API (mirrors CI "E2E Smoke Tests" job)
102-
uv run coder-eval run tasks/*.yaml --tags smoke-pass --model claude-haiku-4-5-20251001
110+
uv run coder-eval run $(SMOKE_GLOBS) --tags smoke-pass --model claude-haiku-4-5-20251001
103111
@echo "--- now running smoke-fail bucket (expected to exit non-zero) ---"
104-
! uv run coder-eval run tasks/*.yaml --tags smoke-fail --model claude-haiku-4-5-20251001
112+
! uv run coder-eval run $(SMOKE_GLOBS) --tags smoke-fail --model claude-haiku-4-5-20251001
105113

106114
docker-image: ## Build the coder-eval-agent image (core + both agents baked in; no creds needed)
107115
@VERSION=$$($(VERSION_CMD)); \

‎comparison.md‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
# Closing the grading-material leak: two approaches compared
2+
3+
> **Scope note.** This is a design-decision document about *sandbox isolation*. It is unrelated to
4+
> `docs/comparison.md`, which compares coder_eval to other eval frameworks (SWE-bench, Harbor, …).
5+
6+
- **Approach A** — [PR #88](https://github.com/UiPath/coder_eval/pull/88),
7+
`fix/docker-isolation-copy-prune`: **COPY/PRUNE + GRADE-OUTSIDE**. Open, draft, 8 commits,
8+
+5,754 / −214 across 35 files. Implemented and verified.
9+
- **Approach B** — **Declarative trust split + DAC permissions**: `input_dir` / `reference_dir` on
10+
`TaskDefinition`, `$INPUT_DIR` / `$REFERENCE_DIR` substitution, agent runs as an unprivileged uid,
11+
`reference_dir` is root-owned `0700`, `task.yaml` itself lives in `reference_dir`. A design, not
12+
yet built.
13+
14+
---
15+
16+
## The problem
17+
18+
When coder_eval grades an agent, the grading material sits on the same filesystem the agent is
19+
working in — **including under `--driver docker`, which is the mode users assume is safe**.
20+
21+
On `main`, an agent in the container can reach all of the following:
22+
23+
| Surface | Where it lives today |
24+
|---|---|
25+
| Full `success_criteria` | `/work/input/task.yaml`, staged verbatim (`docker_runner.py:605-620`) |
26+
| Raw task YAML text | `context.json.source_yaml` |
27+
| Reference solution | `reference.file` / `reference.directory`, auto-mounted (`:1253-1254`) |
28+
| Task-dir graders (`check_*.py`, `RESOLUTION.md`, reference agents) | host task dir bind-mounted at its own host path (`:1176`) |
29+
| Signposts pointing at all of the above | `TASK_DIR` / `SKILLS_REPO_PATH` in the agent's env |
30+
31+
The agent and the harness run as the **same user** in the container — both root, since neither
32+
`docker/Dockerfile` nor `docker/Dockerfile.runtime` sets `USER` and `_build_argv` passes no
33+
`--user`. So there is no permission barrier of any kind: a single `grep -r` reaches the answer key.
34+
35+
**This is measured, not theoretical.** PR #88 reports a nightly audit in which **~2.4% of replicates
36+
read the suite instead of solving the task**, and an adversarial "check `/work`" prompt drives that
37+
to **~100%**.
38+
39+
A separate but adjacent defect: `~/.uipath` (including `.auth`) was mounted **read-write from the
40+
host original**, so an agent could overwrite operator credentials.
41+
42+
The root cause is structural: **the harness lives inside the sandbox it is supposed to be isolating
43+
from.** The in-container process runs the agent *and* grades it (`run_task_internal_command.py:190-209`),
44+
so the rubric has to be shipped into the container at `t=0`.
45+
46+
---
47+
48+
## Approach A — COPY/PRUNE + GRADE-OUTSIDE (PR #88)
49+
50+
**Closes the leak by absence, not by permission.** The container never contains grading material.
51+
52+
Three coordinated moves:
53+
54+
1. **COPY/PRUNE the agent's inputs.** The staged `task.yaml` is criteria-stripped via
55+
`agent_safe_dump()` (`success_criteria: []`, `reference: null`) and `context.json.source_yaml` is
56+
nulled. Plugins are projected to a *sanitized bundle copy* (`project_plugin_for_agent`, allowlist
57+
`PLUGIN_AGENT_ALLOWED_SUBDIRS` = `skills`/`commands`/`agents`/`hooks`/`.claude-plugin`) mounted
58+
`:ro` at `/work/skills`. The raw checkout, the reference, and the host task dir are **not mounted
59+
at all**. A **grader-dir overlap guard** hard-errors any `template_sources` / `system_prompt_file`
60+
/ `extra_mounts` whose source contains the task dir.
61+
2. **GRADE OUTSIDE.** The container runs the agent only. After it exits, the host grades the
62+
copied-out artifacts via `regrade_on_host`, using the full unstripped `TaskDefinition` it still
63+
holds, with `TASK_DIR` pointing at the real host task dir. Re-grade is restricted to an allowlist
64+
of final statuses (`SUCCESS` / `FAILURE` / `MAX_TURNS_EXHAUSTED`).
65+
3. **Copy-then-mount host state.** `~/.uipath` and `~/.claude` are forwarded as throwaway rw
66+
**copies**; `~/.claude/jobs` (operator session timelines) is denylisted.
67+
68+
Plus **harness-outside**: `pre_run`/`post_run` invoke helper scripts under the skills-repo `tests/`
69+
tree, which is no longer mounted. `post_run` becomes host-only; `pre_run` gains a per-command
70+
`runs_in: host|agent` (default `host`, seeding the workspace through a `:ro` `/work/seed` staging
71+
mount). The PR body flags this as a known blocker affecting **243 of 1,147 downstream tasks (~21%)**;
72+
the branch implements the fix, with only the ~7 `uv sync` tasks needing `runs_in: agent`.
73+
74+
**Verification shipped with it:** Detector A (host-unchanged — daemon-less mount-shape proxy in CI
75+
plus an `-m live` byte-and-metadata-identical real run), Detector B (zero-grading-material in the
76+
agent mount view, with a positive control so "staged nothing" can't pass vacuously), a baked-image
77+
answer scan, and live confirmation that an adversarial "read your grader" task fails on gpt / claude
78+
/ gemini / kimi.
79+
80+
**Stated costs:** early stop is **disabled under docker** (criteria are gone, so the in-container
81+
watcher can't arm — a warning is logged, verdict unaffected), and the design doc states plainly that
82+
**allowlist-by-absence has no DAC backstop** — if the prune boundary is wrong, nothing else stops the
83+
read.
84+
85+
## Approach B — Declarative trust split + DAC permissions
86+
87+
**Closes the leak by permission, with an author-declared boundary.**
88+
89+
- `TaskDefinition` gains `input_dir` (agent-visible fixtures) and `reference_dir` (answer key),
90+
substituted as `$INPUT_DIR` / `$REFERENCE_DIR` inside the task file.
91+
- `task.yaml` itself lives in `reference_dir`, so the rubric is protected by the same single rule as
92+
everything else — no special case.
93+
- In the container, the agent runs as an unprivileged uid; `reference_dir` is root-owned `0700`;
94+
`input_dir` and the workspace are chowned to the agent uid.
95+
- Effectively this **splits the existing `$TASK_DIR` token into its two halves by trust level** —
96+
which is precisely why the leak exists today, since one token points at a directory mixing both.
97+
98+
Refinements this needs to be sound:
99+
100+
- **Scope-aware substitution.** `$REFERENCE_DIR` must be a hard validation error in agent-facing
101+
fields (`initial_prompt`, `system_prompt`, mutations) — resolving it there advertises the path and
102+
leaks it into logs and reports. `$INPUT_DIR` substitutes anywhere.
103+
- **Copy, don't mount, `reference_dir`.** The host bind mount is `:ro`, so `chmod` fails `EROFS`;
104+
making it writable would mutate the user's real repo permissions with no crash-safe restore. Stage
105+
a container-local root-owned copy — which also lets the symmetric same-host-path mount disappear.
106+
- **Static, not scoped, permissions.** A `with block_access(): …` context manager that flips modes
107+
on entry and restores on exit is not crash-safe (SIGKILL between enter and exit leaves them
108+
flipped). With two identities, the perms are simply set once at stage time and never restored.
109+
- **Dataset files default to `reference_dir`** — for activation suites the JSONL rows carry
110+
`expected_skill`, which *is* the answer key.
111+
- The `~/.claude` credential copy must stay agent-writable, so `0700`-root cannot be applied
112+
blanket — it is per-path.
113+
114+
---
115+
116+
## Pros and cons
117+
118+
| | Pros | Cons |
119+
|---|---|---|
120+
| **A — COPY/PRUNE + GRADE-OUTSIDE** (PR #88) | Closes the leak **even against a root agent** — no identity work needed. Fails safe: a file that isn't there can't be read. Also closes surfaces B doesn't touch: host-credential overwrite (`~/.uipath` rw), `~/.claude/jobs` exposure, env signposts, baked-image content. **Built, tested, and empirically verified** (detectors in CI + live adversarial probe across four models). Requires **no per-task authoring changes** for the leak fix itself. Removes the harness from the sandbox — the structural root cause. | **Early stop disabled under docker** — a shipped feature silently degrades on one driver. Forced `pre_run`/`post_run` host-side, touching **~21% of downstream tasks** and adding a new `runs_in` concept. **No DAC backstop** (the PR says so): the prune allowlist is "a coder_eval-side guess about what is answer-free", so a plugin putting answers in `skills/` defeats it. Detector B catches known sentinels, not an unknown golden file. Large blast radius: **+5,754 LOC / 35 files**, restructuring `orchestrator.py`. Nothing protects `tempdir`. |
121+
| **B — Trust split + DAC permissions** | Supplies exactly the **DAC backstop A lacks** — a second, independent barrier. The boundary is **author-declared**, not framework-guessed, replacing A's residual risk #1 with an explicit contract. **Preserves early stop** (criteria stay in container memory, which the agent can't read) and **keeps `pre_run`/`post_run` in-container** (they run as root and can still read `reference_dir`) — no `runs_in`, no 243-task migration. Much smaller runtime change: a uid on spawn plus chowns at setup. Conceptually simple: one rule, "`reference_dir` is harness-only". | **Depends entirely on identity separation that doesn't exist yet** — both processes are root today, so `chmod 0700` is a *no-op* until the agent runs as a separate uid. Fails **open and silently**: one missed chown, one new mount, and there's no absence to fall back on. Real friction running the agent CLI unprivileged (HOME, npm cache, the `~/.claude` copy must stay writable). Pushes classification onto **1,147 downstream task authors** unless the default is "protected unless declared input". Does **not** address host-credential overwrite, baked-image content, or env signposts. **Also nothing for `tempdir`** (no second identity on the host). Unbuilt and unverified. |
122+
123+
### Head-to-head
124+
125+
| Axis | A — Absence | B — Permissions |
126+
|---|---|---|
127+
| Works against a root agent | ✅ | ❌ (requires uid split first) |
128+
| Failure mode | Fails safe (nothing to read) | Fails open (silently, if a chown is missed) |
129+
| Boundary defined by | Framework allowlist (a guess) | Task author (a declaration) |
130+
| Early stop under docker | ❌ disabled | ✅ preserved |
131+
| `pre_run` / `post_run` | Moved host-side; `runs_in` added; ~21% of tasks affected | Unchanged, in-container |
132+
| Host credential / `~/.claude/jobs` / baked image | ✅ covered | ❌ out of scope |
133+
| `tempdir` driver | ❌ | ❌ |
134+
| Per-task migration | None for the leak fix | `input_dir` / `reference_dir` across the suite |
135+
| Framework blast radius | +5,754 LOC, 35 files | Smaller runtime change; new schema + token rules |
136+
| Status | Implemented, CI-gated, live-verified | Design only |
137+
138+
---
139+
140+
## Assessment
141+
142+
**These are not competing designs — B is the missing layer under A.**
143+
144+
PR #88's own residual-risk list names its weakest point: *"Allowlist-by-absence has no DAC backstop
145+
… `PLUGIN_AGENT_ALLOWED_SUBDIRS` is a coder_eval-side guess about what is answer-free."* That is
146+
precisely the gap Approach B fills, in two independent ways:
147+
148+
1. **As a mechanism** — a uid split gives the second barrier, so a prune-boundary miss is no longer
149+
game over.
150+
2. **As a contract** — and this matters more. `input_dir`/`reference_dir` replaces the framework's
151+
guess with an author declaration, which is the durable fix PR #88 already identifies as a
152+
cross-repo follow-up ("push the agent-bundle boundary into the skills repo — a manifest declaring
153+
the agent-safe surface"). Approach B *is* that manifest, expressed in the task schema.
154+
155+
If only one ships, it should be **A**: it is built, measured against real leak rates, gated by
156+
detectors in CI, and it closes several surfaces B never touches. B's mechanism half is also blocked
157+
on identity work (non-root agent, credential-copy ownership) that A doesn't need.
158+
159+
The sequencing that gets the most value:
160+
161+
1. **Land A.** It is the structural fix — it removes the harness from the sandbox.
162+
2. **Adopt B's declarative half next**, as the durable replacement for the prune allowlist. It is
163+
the higher-value part of B and is independent of the permission mechanism.
164+
3. **Add B's uid split as defense-in-depth** where material must remain in-container.
165+
4. **Revisit early stop.** A disables it under docker; the leak-free restoration is a host-side
166+
watcher over the live event stream — the host already receives every `ToolStartEvent` /
167+
`ToolEndEvent` with full `CommandTelemetry` (`streaming/wire.py`), and can already signal the
168+
container through the heartbeat channel. Worth first confirming the feature earns its keep: every
169+
`stop_early` usage in this repo is a fixture for testing early stop itself.
170+
5. **State plainly that `tempdir` is not cheat-resistant** under either approach. Neither has a
171+
boundary there; it should be documented as a development driver.

0 commit comments

Comments
 (0)