fix(tbench2_env): keep verifier assets out of the agent's reach#972
fix(tbench2_env): keep verifier assets out of the agent's reach#972nblintao wants to merge 2 commits into
Conversation
…ask env & workdir - _evaluate_task runs the task's official tests/test.sh (pinned uvx toolchain, /logs/verifier/reward.txt) when the task ships it; the bare pytest fallback (custom task dirs without the canonical harness) prefers uvx with a pinned pytest so it carries its own toolchain, and runs from the same workdir the agent worked in. - Pass timeouts through to every shell_exec call: camel's shell_exec has its own per-call default (20s) and ignores the constructor timeout, so agent commands and evaluations were silently truncated at 20s (circuit-fibsqrt's verifier declares 3600s and was cut mid-test). Agent execs use command_timeout_s (TB2_COMMAND_TIMEOUT_S-overridable); evaluate uses the task's own [verifier].timeout_sec budget. - Shell cwd resolves to the task image's own WORKDIR, parsed from the task's environment/Dockerfile (TB2_TASK_WORKDIR overrides): task images pin their own WORKDIR (fix-git: /app/personal-site, prove-plus-comm: /workspace) and both agents and oracle solutions assume commands run there. Falls back to the task source dir for plain local mode. - Drop install_dependencies=['pytest'] (evaluation carries its own pytest via uvx); with CAMEL_RUNTIME=true camel's .initial_env venv then stops hijacking PATH away from the task image's native python. - Serialize canonical evaluation under a lock so concurrent sessions on one server cannot clobber the official fixed verifier paths (/tests, /logs/verifier/reward.txt).
e763ddf to
d90c1fa
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| with tarfile.open(fileobj=buf, mode="w") as tar: | ||
| tar.add(tests_dir, arcname=".") | ||
| self._SEQUESTERED_TESTS[key] = buf.getvalue() | ||
| shutil.rmtree(tests_dir, ignore_errors=True) |
There was a problem hiding this comment.
Sequester breaks shared task cache
High Severity
With TB2_SEQUESTER_TESTS=1, reset() deletes tests/ from the resolved task dir and keeps a copy only in process memory. When TB2_TASKS_DIR is unset, that dir is the shared auto-download extract, and _download_tb2_repo will not re-extract while the extract root still exists. After a server restart the in-memory copy is gone and the on-disk tests/ never come back, so evaluate fails as missing tests.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d90c1fa. Configure here.
There was a problem hiding this comment.
Documented as an assumption rather than code-fixed: the in-memory cache is process-local, so withhold mode assumes one server process owns the task filesystem for its lifetime — which is exactly the intended per-task-sandbox deployment (one uvicorn worker, one ephemeral sandbox per episode, MAX_CONCURRENT_ENVS=1). It is explicitly not for uvicorn --workers N sharing one on-disk checkout; a note to that effect now lives on the _WITHHELD_TESTS cache. The gate is off by default.
| self._terminal_toolkit = TerminalToolkit( | ||
| timeout=self.command_timeout_s, | ||
| working_directory=str(task_dir), | ||
| working_directory=workdir, |
There was a problem hiding this comment.
Dockerfile WORKDIR can hijack local cwd
High Severity
Local reset() now prefers the task Dockerfile's WORKDIR whenever that path exists as a directory. Many TB2 tasks use /app or /workspace, which already exist in the OpenEnv server container but are not the task tree. The toolkit and canonical evaluate then cd there instead of falling back to task_dir, so agents and scoring run in the wrong tree.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d90c1fa. Configure here.
There was a problem hiding this comment.
By design for the per-task path this recipe targets: the server runs inside the task's own official image, so the Dockerfile WORKDIR (e.g. /app) is the real task tree — the 89-task golden sweep in #966 validates commands and scoring run in the right place. This is #965's local-mode resolution; for generic local mode where the server container isn't the task image, TB2_TASK_WORKDIR overrides. Not changing here.
d90c1fa to
e80e4b9
Compare
c620e95 to
f36e3ba
Compare
Both execution modes let the agent read (and tamper with) the assets the verifier scores against — tests/test_outputs.py spells out the expected outputs and solution/ is the literal answer. Fine for eval plumbing with a cooperative agent; for RL training it is a reward-hacking vector. Align with the official TB2 harness's model instead: the solution never ships, and tests exist in the agent's filesystem only during the verify window. - Local mode: with TB2_WITHHOLD_TESTS=1, reset() reads tests/ into server memory and deletes it (and solution/ — nothing in this env reads it; it exists only for oracle runs) from disk before the agent's first action. Gated off by default because in plain local mode TB2_TASKS_DIR may be a developer's working checkout; deployments that stage per-task copies (e.g. cloud sandboxes) should turn it on. The in-memory copy is process-local, so withhold mode assumes one server process owns the task filesystem for its lifetime (the per-task-sandbox deployment: one worker, one ephemeral sandbox per episode) — documented on the cache. - Evaluation stages a pristine tests/ copy at /tests for exactly the verify window — from server memory when withheld, i.e. a copy the agent never had filesystem access to — and removes it again once scoring returns, so a later episode / concurrent session sharing the filesystem can't read it. Both fixed paths (/tests, /logs/verifier) are recreated from scratch first, symlink included: the previous "mkdir -p && cp -a" merged into whatever already existed, and shutil.rmtree silently no-ops on a symlink, so an agent could pre-plant a /tests/conftest.py (pytest auto-loads it) or symlink a fixed path at a dir it controls and survive into scoring. _hard_remove unlinks the symlink itself. The in-memory tarball is restored with the 3.12 data filter when available, falling back cleanly on 3.10/3.11 (the package targets >=3.10). The bare-pytest fallback runs against the same staged copy. The canonical harness writes its test.sh stdout under /logs/verifier (not a fixed /tmp path) and the verify-window cleanup wipes /logs/verifier alongside /tests, so neither reward.txt nor the pytest -rA log — which can spell out expected values — outlives scoring for a timed-out episode or a later session. The withhold cache tars the realpath of tests/ so a symlinked tests/ caches its target's files rather than a bare link that would restore empty. - Docker mode gets true stage-at-verify: the initial /task copy excludes tests/ and solution/, and _evaluate_docker streams tests in from the host (this server sits outside the container) right before pytest runs, then removes them after scoring — in a finally, because docker-mode step() reports a scoring error without ending the episode, and the agent must not keep its session alongside a staged /task/tests that an errored verify left behind. Residual risk, shared with the official TB2 harness: verification necessarily runs inside the same container the agent controlled, and agent-started background processes must be left alive (many tasks are scored on services the agent set up), so a leftover watcher that copies the staged tests or rewrites the reward file during the verify window — or a root agent that tampers with container binaries — could still fake a pass. Task state (services, git state) is not portable, so verification cannot move to a container the agent never touched. tests/envs/test_tbench2_env.py covers the withhold round-trip, staging hygiene (pre-planted conftest.py and stale reward.txt wiped; symlinked fixed paths and a symlinked tests/ removed without following into the target; source checkout untouched without the gate), post-scoring cleanup, the full evaluate flow from the in-memory copy, and docker mode's copy exclusions, evaluate-time staging order, and cleanup when scoring itself raises (mock container — no docker needed). Also verified end-to-end on a per-task Daytona cloud sandbox (fix-git, gate enabled): the staged task dir carries no tests/solution, a filesystem-wide find turns up neither, pre-solution evaluate scores 0.0 via the canonical test.sh from the in-memory copy, and applying the oracle solution scores 1.0. A static scan of all 89 official tasks' tests/ trees (regular files and dirs only — no symlinks or special modes) confirms the tar round-trip is safe suite-wide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
f36e3ba to
d2b7a24
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d2b7a24. Configure here.
| # test.sh log, whose pytest -rA output can reveal expected | ||
| # values. | ||
| _hard_remove(_VERIFY_TESTS_DIR) | ||
| _hard_remove(_VERIFIER_LOG_DIR) |
There was a problem hiding this comment.
Staging errors skip verify cleanup
High Severity
_stage_tests_for_verify runs outside the try/finally that wipes /tests and /logs/verifier. If staging raises after writing files (partial extractall/copytree, disk errors, filter rejection), the withheld suite can remain on disk. step then catches the error with done still false, so the agent keeps a session that can read expected outputs.
Reviewed by Cursor Bugbot for commit d2b7a24. Configure here.
| logging.warning( | ||
| "failed to remove staged /task/tests after verify", | ||
| exc_info=True, | ||
| ) |
There was a problem hiding this comment.
Docker wipe ignores exec status
Medium Severity
_evaluate_docker ignores the exit code from _exec_in_container for both the pre-stage rm -rf and the post-stage cleanup. A failed wipe still proceeds to put_archive, which merges into leftover agent files such as a planted conftest.py. A failed cleanup also stays silent, so /task/tests can remain readable when scoring errors and done stays false.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d2b7a24. Configure here.
1 participant
Summary
Both execution modes let the agent read — and tamper with — the assets the verifier scores against:
tests/test_outputs.pyspells out the expected outputs, andsolution/is the literal answer. That's fine for evaluation plumbing with a cooperative agent; for RL training it is a reward-hacking vector. This PR aligns both modes with the official TB2 harness's model: the solution never ships, and tests exist in the agent's filesystem only during the verify window.TB2_WITHHOLD_TESTS=1,reset()readstests/into server memory and deletes it (andsolution/— nothing in this env reads it; it exists only for oracle runs) from disk before the agent's first action. Gated off by default because in plain local modeTB2_TASKS_DIRmay be a developer's working checkout that must not be modified; deployments that stage per-task copies (e.g. cloud sandboxes — see the stacked feat(tbench2_env): per-task sandbox image recipe + Daytona materialization #966) turn it on./tests,/logs/verifier) from scratch before scoring. The previousmkdir -p && cp -amerged into whatever already existed, so an agent could pre-plant a/tests/conftest.py(pytest auto-loads it) or a symlinked/logs/verifierand have it survive into scoring. The bare-pytest fallback now runs against the same staged copy./taskcopy excludestests/andsolution/, and_evaluate_dockerstreams tests in from the host — this server sits outside the container, so the staged copy is one the agent never saw — right before pytest runs, and removes them again after scoring (in afinally: docker-modestep()reports a scoring error without ending the episode, so an errored verify must not leave/task/testsbehind for the agent's still-live session).Residual risk, shared with the official TB2 harness: verification necessarily runs inside the same container the agent controlled, and agent-started background processes must be left alive (many tasks are scored on services the agent set up), so a leftover watcher that copies the staged tests or rewrites the reward file during the verify window — or a root agent that tampers with container binaries — could still fake a pass. Task state (services, git state) is not portable, so verification cannot move to a container the agent never touched.
Stacked on #965. This branch is based on #965's, so its commit appears here until it merges — review this PR's own commit,
fix(tbench2_env): keep verifier assets out of the agent's reach(the last commit). The per-task sandbox recipe (#966) now stacks on top of this PR and enables the gate in its server command. (GitHub can't retarget a fork PR's base onto another fork branch, so the base readsmain.)Type of Change
Alignment Checklist
.claude/docs/PRINCIPLES.mdand this PR aligns with our principles (verifier integrity is a precondition for "one env = one trajectory" reward semantics).claude/docs/INVARIANTS.mdand no invariants are violated (no API changes; one new opt-in env var, documented in the README)bash .claude/hooks/lint.sh— the changed files are clean (pre-existing failures in other envs are untouched)RFC Status
Test Plan
tests/envs/test_tbench2_env.py, runs in CI, no camel/docker/daytona needed): withhold round-trip (disk cleared, memory populated,task.toml/instruction.mduntouched, idempotent re-reset); staging wipes a pre-plantedconftest.pyand stalereward.txt; without the gate the source checkout is never touched; full evaluate flow scores from the in-memory copy via the canonical harness; docker mode's copy exclusions, no-duplicate archive, evaluate-time staging order (rm -rf→ fresh copy → pytest →rm -rf), and cleanup when scoring itself raises, via a mock container.fix-git, gate enabled): the staged task dir carries notests/orsolution/; a filesystem-widefindturns up neither anywhere agent-readable;evaluateon the untouched task scores 0.0 via the canonicaltests/test.shrun from the in-memory copy; applying the official oracle solution then scores 1.0.tests/trees contain only regular files and directories (no symlinks or special modes), so the tar round-trip is safe across the whole benchmark.Note
Medium Risk
Changes reward semantics and filesystem side effects during reset/evaluate (including optional deletion of tests/solution on disk); mistakes could break scoring or corrupt dev checkouts if
TB2_WITHHOLD_TESTSis misused, but behavior is gated and heavily tested.Overview
Hardens tbench2 so agents cannot read or tamper with
tests/andsolution/during training, matching the official TB2 model: verifier material exists on the shared filesystem only during scoring.Local mode adds opt-in
TB2_WITHHOLD_TESTS=1: onreset(),tests/is tar-cached in server memory and bothtests/andsolution/are removed from the task dir before the agent acts;evaluatestages a fresh copy at/testsand/logs/verifier, prefers the canonicaltests/test.shharness (reward from/logs/verifier/reward.txt), then wipes those paths. Staging always recreates fixed paths via_hard_remove(symlink-safe) so pre-plantedconftest.pyor symlink redirects cannot survive into scoring. A class-level in-memory cache and_CANONICAL_EVAL_LOCKserialize verify when paths are shared.Docker mode no longer copies
tests/orsolution/into/taskat startup;_evaluate_dockerstages tests from the host only for the verify window (rm -rfbefore copy, cleanup infinallyeven on errors). Tar copy into containers skips excluded top-level dirs and avoids duplicate archive members.Also aligns agent/verify working directory with the task image
WORKDIR(or overrides), passes explicit shell timeouts (including verifier budget fromtask.toml), and documents the new env var in the README. Unit tests cover withholding, staging, canonical evaluate, symlink edge cases, and mocked Docker verify flow.Reviewed by Cursor Bugbot for commit d2b7a24. Bugbot is set up for automated code reviews on this repo. Configure here.