[jimmy-holdout]

Fixes #233

What

The skillsbench-3d-scan-calc verifier now grades against fixed expected values: EXPECTED_MASS, EXPECTED_VOL and EXPECTED_ID, stored in verifier/test_outputs.py. They were computed once by running the verifier's existing parse (_get_ground_truth(), unchanged apart from taking the path as an argument) on the shipped environment/scan_data.stl. test_values_correct no longer reads /root/scan_data.stl. The skillsbench sample README now says this verifier is patched locally.

Why

The verifier has no stored answer. It recomputes the expected mass and material ID from the scan_data.stl in the container at grading time, and the agent can write to that file. If the input is replaced by a single zero-volume triangle with attribute 42, the expected answer becomes mass 0 / ID 42, so the report {"main_part_mass": 0, "material_id": 42} scores 1.0.

Answer	Working scan_data.stl	Before	After
Correct mass, ID 42	shipped	1	1
Correct mass, ID 42	shipped, 80-byte header changed	1	1
Mass 0, ID 42	shipped	0	0
Mass 0, ID 42	replaced with one zero-volume triangle	1	0

Tests

New tests/test_skillsbench_3d_scan_calc_verifier.py loads the real verifier and points its /root/... paths at a temp dir. It has 5 tests:

• The correct answer passes, once for the cm reading and once for the mm reading.
• The correct answer passes when the working STL's header differs from the shipped file.
• With the working STL overwritten by the zero-volume file, mass 0 fails and the correct answer still passes. On main this test fails with DID NOT RAISE.
• _get_ground_truth() run on the shipped environment/scan_data.stl returns exactly the stored constants. This fails if the STL changes without the constants.

Commands:

• ruff format --check and ruff check on src/ tests/ .github/scripts/ pass. The verifier itself is under tasks/samples, which is excluded from ruff.
• make typecheck passes with the extras CI installs.
• tests/test_custom_lint.py passes (735 tests), and so does python -m tests.lint.prose_budget.
• pytest tests/ -n 6 -m "not live and not lint": 6209 passed, 14 skipped and 14 failed. The same 14 tests fail on main in this environment (6204 passed): 10 in tests/test_judge_litellm.py, where litellm is not importable, the 2 TestOutputMountWidenedBeforeLaunch tests, test_every_exemption_names_a_registered_command_and_back and test_stream_invalid_value_rejected.

The repro script from #233 runs the verifier with pytest and scores it as test.sh does. Before:

no-op (empty answer)               answer={}  reward=0
honest solution                    answer={"main_part_mass": 34648.04123490179, "material_id": 42}  reward=1
mass 0, input untouched            answer={"main_part_mass": 0, "material_id": 42}  reward=0
mass 0, scan_data.stl overwritten  answer={"main_part_mass": 0, "material_id": 42}  reward=1

After:

no-op (empty answer)               answer={}  reward=0
honest solution                    answer={"main_part_mass": 34648.04123490179, "material_id": 42}  reward=1
mass 0, input untouched            answer={"main_part_mass": 0, "material_id": 42}  reward=0
mass 0, scan_data.stl overwritten  answer={"main_part_mass": 0, "material_id": 42}  reward=0

Notes

• A custom lint rule doesn't fit here: this is a one-off in a vendored sample verifier under tasks/samples.
• After this change, the expected values depend only on the verifier source. The verifier is copied from $TASK_DIR at grading time. The harness mounts a copy of the task directory. The mount itself is writable; access is restricted by permissions: the copy gets a read-only grant for the container, and it is held at mode 000 during agent turns. Docker Isolation says this window, described there for the reference copy, does not stop a deliberate chmod from inside the container, or a process that outlasts the window. This PR does not change that.
• Tampering with the container's own pytest setup is outside this issue.
• If scan_data.stl is regenerated, the EXPECTED_* constants must be updated in the same change. The new test fails until they are.
• This is a local change to a vendored sample. Upstream SkillsBench computes the answer the same way.

[uipreliga]

Review: coder_eval — pr:237 (4 files, +91/-9) — fix(samples): grade 3d-scan-calc against fixed expected values (author jimmy-holdout, fixes #233)

Scope: pr:237 (4 files, +91/-9) — fix(samples): grade 3d-scan-calc against fixed expected values (author jimmy-holdout, fixes #233) · branch fix/3d-scan-calc-input-tamper (pr-237) · d17a0de · 2026-10-25T23:46Z · workflow variant

Change class: simple — a vendored sample verifier swaps a runtime-recomputed expected value for stored constants, plus a regression test and README note; no src/ code path changes

The PR closes the agent-writable scan_data.stl reward-hack vector (issue #233) with clean, well-tested code (9.9/10, and Type Safety, Architecture, Error Handling and Harness Quality are all 10/10), but grading still runs in the agent's root container, where tampering with the pytest entry point, a pytest11 plugin or the task-dir verifier can still force reward 1.0 for the same agent output, so the change is ready to merge after the remaining vectors are documented and given a follow-up issue.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	9.6 / 10	0	0	0	4	Header-variation test is vacuous because the patched grader never reads scan_data.stl
2. Type Safety	10 / 10	0	0	0	0	—
3. Test Health	9.9 / 10	0	0	0	1	The material_id gate of the patched verifier is never tripped by the new regression tests
4. Security	9.5 / 10	0	0	1	0	Grading still runs agent-writable tooling and the task-dir verifier in the agent's container with no tamper check; other reward-hack vectors remain
5. Architecture & Design	10 / 10	0	0	0	0	—
6. Error Handling & Resilience	10 / 10	0	0	0	0	—
7. API Surface & Maintainability	9.9 / 10	0	0	0	1	Locally patched vendored verifier does not say in the file that it differs from upstream 95b3e60
8. Evaluation Harness Quality	10 / 10	0	0	0	0	—

Overall Score: 9.9 / 10 · Weakest Axis: Security at 9.5 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 1 · 🔵 6 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

1. [Axis 4] Grading still runs agent-writable tooling and the task-dir verifier in the agent's container with no tamper check; other reward-hack vectors remain (tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml:69) — The PR stops grading against /root/scan_data.stl (verifier/test_outputs.py:96 expected_mass, expected_vol, expected_id = EXPECTED_MASS, EXPECTED_VOL, EXPECTED_ID). The grading criterion still runs in the agent's own container. The orchestrator runs inside that container, and per docs/DOCKER_ISOLATION.md the container runs as root. The criterion runs rm -rf /verifier && cp -r "$TASK_DIR/verifier" /verifier (yaml:67) and then bash /verifier/test.sh > /logs/verifier/console.log 2>&1 || true (yaml:69). test.sh (unchanged) runs pytest --ctrf /logs/verifier/ctrf.json /verifier/test_outputs.py -rA -v from PATH and then does if [ $? -eq 0 ]; then echo 1 > /logs/verifier/reward.txt. Its line pip3 install --break-system-packages pytest==8.4.1 ... does not reinstall a package that is already present. The agent's allowed tools include Bash, so the agent can still force reward 1.0 without the STL. Two vectors remain. (a) Overwrite the pytest entry point on PATH with exit 0, or pip-install an auto-loaded pytest11 plugin, or edit site-packages. None of these needs a chmod, so they are as easy as the STL overwrite that this PR fixes. (b) Edit $TASK_DIR/verifier/test_outputs.py to change EXPECTED_MASS = 34648.04123490179. The task dir is held at mode 000 only during communicate (orchestrator.py:1964 async with self.sandbox.set_permissions([self._reference_dir, self.sandbox.task_dir])). Root owns the copy and can chmod it back, and a writer started in the background can act after the window closes. The task dir has no pre-grade digest check like _verify_reference_integrity (orchestrator.py:1370), which covers only the reference dir. These gaps existed before the PR and affect every skillsbench sample. This PR's stated goal is to close this reward-hack class, so the remaining vectors need to be recorded. Fix options: (1) Grade outside the agent-controlled container, or extend the digest check to sandbox.task_dir and run the verifier with a known-good interpreter, e.g. PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /path/to/baked/python -I -m pytest --noconftest .... (2) At minimum, state in README.md and the yaml comment that the patch closes only the STL vector, and link a follow-up issue for the shared verifier-runtime trust gap. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:N

Nits

1. [Axis 1] Header-variation test is vacuous because the patched grader never reads scan_data.stl (tests/test_skillsbench_3d_scan_calc_verifier.py:55) — After this PR, test_values_correct opens only /root/mass_report.json. It reads constants and never opens the STL (verifier line 96: expected_mass, expected_vol, expected_id = EXPECTED_MASS, EXPECTED_VOL, EXPECTED_ID). That makes test_correct_answer_passes_when_the_stl_header_differs (lines 55-57: # The 80-byte header is not used by the computation, so changing it must not matter. / _grade(tmp_path, monkeypatch, b"x" * 80 + _SHIPPED_STL[80:], _CORRECT)) the same test as test_correct_answer_passes[cm]. Its comment also points to a 'computation' that grading no longer does, so it misleads the reader. The module-level _SHIPPED_STL = _SHIPPED_STL_PATH.read_bytes() (line 17) is only passed as the stl argument that the grader ignores (line 52). Delete the header test. Write an STL in _grade only from the overwrite test, which is the one test where the STL content is meaningful, for example as an optional stl: bytes | None = None argument. You can then delete _SHIPPED_STL.
2. [Axis 1] Test hardcodes the expected mass literal instead of reading the verifier's EXPECTED_MASS (tests/test_skillsbench_3d_scan_calc_verifier.py:20) — _CORRECT_MASS = 34648.04123490179 (test line 20) copies EXPECTED_MASS = 34648.04123490179 # coordinates read as cm (verifier line 11). test_expected_values_match_the_shipped_stl already ties module.EXPECTED_MASS to the shipped STL, so the second literal adds no independent check. It is one more site to update by hand if the STL changes. Derive it from the loaded module (_load_verifier().EXPECTED_MASS, e.g. in a fixture) so the value has one source. Do the same for the repeated 42 material ID on lines 21, 22 and 52, which repeats EXPECTED_ID.
3. [Axis 1] Redundant tuple re-alias of module constants in test_values_correct (tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py:96) — expected_mass, expected_vol, expected_id = EXPECTED_MASS, EXPECTED_VOL, EXPECTED_ID only copies three module constants into locals. The goal is probably to keep the vendored diff small, since the lines below still use the lowercase names. This is acceptable for a vendored file, but a reader sees an indirection with no purpose. Either use the constants directly in lines 105-116, or add a short note that the aliasing is there to keep the upstream diff small. Non-blocking.
4. [Axis 1] Edited YAML comment line is 123 chars and breaks the block's wrap (tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml:61) — Line 61 ( # Run the skillsbench verifier (patched locally to grade against fixed expected values): copy it from the task dir into a) is 123 characters long. Lines 62-63 are 79 and 54 characters long. The parenthetical was added without rewrapping the comment block. Rewrap the comment so the line lengths match the rest of the block.
5. [Axis 3] The material_id gate of the patched verifier is never tripped by the new regression tests (tests/test_skillsbench_3d_scan_calc_verifier.py:60) — All five tests submit "material_id": 42 (_CORRECT, _MASS_ZERO, and the inline dict at line 52). So the scoring gate at verifier/test_outputs.py:105, assert submission["material_id"] == expected_id, ..., is never reached with a mismatch. Mutation check: if you delete that assert, or set EXPECTED_ID to any value that the constants-parity test still accepts, all 5 tests still pass. The only negative case is pytest.raises(AssertionError, match="Mass mismatch") at line 61, which trips the mass gate only. The 0.1% tolerance boundary (rel_tol=0.001, verifier line 113) is also untested. Add a parametrized negative test that grades {"main_part_mass": _CORRECT_MASS, "material_id": 43} and expects match="Expected Material ID". Optionally add a mass just outside the tolerance (_CORRECT_MASS * 1.002) and expect match="Mass mismatch". Low: the id check is unchanged upstream logic, and the new contract (grading ignores the agent-writable STL) is covered.
6. [Axis 7] Locally patched vendored verifier does not say in the file that it differs from upstream 95b3e60 (tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py:7) — Lines 7-10 explain the constants ("# Expected values for the shipped environment/scan_data.stl, computed once with" ... "# tests/test_skillsbench_3d_scan_calc_verifier.py checks they match the shipped file."). They do not say that this file is a local modification of the upstream verifier. Only README.md says that the vendored tree has been changed. If someone re-vendors from benchflow-ai/skillsbench and copies verifier/ over this file, the reward-hack fix (issue #233) is silently reverted. The regression test would then fail with an AttributeError on module.EXPECTED_MASS, but nothing tells the person re-vendoring why. Apache-2.0 §4(b) also asks for a prominent notice in each modified file. Fix: add one line at the top, e.g. # Modified from benchflow-ai/skillsbench@95b3e60: grade against fixed EXPECTED_* (issue #233).

What's Missing

Parallel paths:

• 🔵 The sibling sample dialogue-parser has the same class of defect, and this PR leaves it unpatched. In verifier/test_outputs.py, test_content_integrity[dynamic_script_sample] and [first_node_check] read their ground truth from /app/script.txt. That file is in the agent's working tree and the agent can write to it. If the agent rewrites the file (for example, it removes every Speaker: text -> Target line and keeps [Start] first), the anti-hardcoding check passes with no work. The README now calls 3d-scan-calc "the one exception" to the verbatim verifiers, so a reader concludes that the other verifiers do not read agent-writable inputs. Pin the expected values the same way, or say in the README that only the STL vector was closed. The court-form-filling comparison with the agent-writable /root/sc100-blank.pdf is harmless, because it can only make the 'differs from input' check easier to pass. (trigger: tasks/samples/skillsbench/README.md)
• 🔵 .claude/notes/isolation.md (about line 518) still says that the read-only task-dir copy "keeps an agent from rewriting the expectations it is graded against". The new EXPECTED_* constants live in exactly that task-dir copy, and the container keeps CAP_FOWNER, so root can chmod the copy and edit them. The note was not updated together with the change that moved the grading truth into the task dir. (trigger: tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py) (restates: Axis 4: Grading still runs agent-writable tooling and the task-dir verifier in the agent's container with no tamper check)

Tests:

• 🔵 The new regression test calls TestOutputs().test_values_correct() in-process with a monkeypatched open. Nothing runs the patched verifier through its real entry point (test.sh -> pytest /verifier/test_outputs.py in the task image, which writes reward.txt). A collection-time or import-time break in the patched file passes make test and appears only as a 0.0 reward in a docker run. A cheap guard: run pytest --collect-only on the vendored file, or add a marked docker smoke test that checks reward 1.0 for a correct mass_report.json. (trigger: tests/test_skillsbench_3d_scan_calc_verifier.py)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] Add a new rule, CE070 (CE067 is retired and an in-flight worktree claims CE069), named vendored-provenance. It is a @pytest.mark.lint whole-tree class in tests/test_custom_lint.py, because it reasons over a directory and a manifest, not one AST. Each vendored tree (tasks/samples/skillsbench/<task>/) gets a VENDORED.json that records the upstream repo, the commit (95b3e60) and the sha256 of each upstream file. The rule hashes each file under verifier/ and environment/. When a digest differs from the upstream digest, the first 5 lines of the file must hold Modified from <repo>@<sha>, and the manifest must have a modified: [<path>: <issue>] entry. When the header is present but the digest matches upstream, the rule also fails. That case is a re-vendor that silently reverted the local patch. No network is needed: the manifest is the SSOT. Prevents: A7 low, verifier/test_outputs.py:7: the patched vendored verifier does not say in the file that it differs from upstream, and a re-vendor silently reverts the issue-233 fix. It also covers the Apache-2.0 §4(b) per-file modification notice for every vendored sample, not only 3d-scan-calc.
• [ce-lint] Add a new rule, CE071 (duplicated-magic-literal), as an AST rule in tests/lint/rules/ce071_no_duplicated_magic_literal.py, wired in tests/lint/runner.py. It collects float literals with 8 or more significant digits from module-level NAME = <literal> assignments in src/ and tasks//*.py. It then flags an identical float literal in any tests//*.py file. The fix it requires is to import or load the constant from the module under test. Integers are out of scope, because small ints like 42 give too many false positives. The rule docstring records that blind spot. Prevents: A1/A5/A7 low, tests/test_skillsbench_3d_scan_calc_verifier.py:20: _CORRECT_MASS = 34648.04123490179 copies the verifier's EXPECTED_MASS. The test then has a second hand-maintained source that adds no independent check.
• [ci-gate] Add yamllint to the dev dependencies and run yamllint -d '{extends: relaxed, rules: {line-length: {max: 120}}}' tasks/ experiments/ in make check, which make verify runs. Excluding vendored environment/ trees is acceptable. ruff cannot do this: it does not read YAML, and pyproject.toml line 242 also excludes tasks/samples from ruff. Prevents: A1/A7 low, 3d-scan-calc.yaml:61: an edited comment line is 123 characters and breaks the block's wrap. It also catches any other over-long task-YAML line.
• [ce-lint] Add a new rule, CE072 (verifier-runtime-pinned), as a whole-tree lint class. For each run_command criterion in tasks/**/*.yaml whose command runs a script or pytest inside the agent's container (it matches bash .*test\.sh, pytest or python -m pytest), it resolves the invoked test.sh and requires these three things. (1) The pytest invocation sets PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 and --noconftest. (2) It uses an absolute interpreter path with -I, not a bare pytest resolved from PATH. (3) The reward line reads the exit status of that pinned call. This makes vector (a) of the security finding a build failure. It does not close vector (b), task-dir tampering, which needs the runtime harness fix. Prevents: A4/A8, 3d-scan-calc.yaml:67-69: grading runs the agent-writable pytest from PATH, so a pytest11 plugin or a stub entry point forces reward 1.0. The same pattern is in the court-form-filling and dialogue-parser samples.

Harness improvements (not statically reachable):

• Extend the pre-grade digest check _verify_reference_integrity in src/coder_eval/orchestrator.py (line 1370) so that it also covers sandbox.task_dir. Take the digest before communicate, verify it before check_all_async, and on a mismatch finalize the row as tampered, not graded. The better long-term fix is to run the grading criteria outside the agent-controlled container, or in a fresh container from the pristine image. Also correct .claude/notes/isolation.md near line 518. It says the read-only task-dir copy stops the agent from rewriting its expectations, but root keeps CAP_FOWNER and can chmod the copy back. Why not static: Tampering happens at run time inside a root container: a background writer, a chmod after the window closes, or an edit to site-packages. Only a digest taken at run time, or a different grading process boundary, can detect it. Static analysis of the task YAML cannot. Prevents: A4/A8 medium, then downgraded to low, 3d-scan-calc.yaml:69: the agent edits $TASK_DIR/verifier/test_outputs.py (EXPECTED_MASS), or poisons the verifier runtime, to force reward 1.0 on every skillsbench sample.
• Add a docker red-team regression test (marked live/docker) for reward hacking. A scripted fake agent does each known hack in turn: overwrite the agent-writable input (scan_data.stl), replace pytest on PATH with exit 0, pip-install a pytest11 plugin, and edit the $TASK_DIR verifier through a background writer. The test asserts that each row grades to 0 or is flagged as tampered. Run it on every skillsbench sample. Why not static: It needs a live container, root privileges and real filesystem state across the communicate and grade boundary. Whether a hack succeeds is an emergent property of the runtime. Prevents: A4/A8: the remaining reward-hack vectors after the issue-233 STL fix. It also stops the STL vector from coming back.
• Add a make mutate-verifiers target, run in CI on changes under tasks/**/verifier/. It runs a mutation tool (mutmut or cosmic-ray) restricted to the assert statements and EXPECTED_* constants of the locally patched verifiers, against their regression tests in tests/test_skillsbench_*_verifier.py. It fails when a mutant survives. A surviving mutant shows either a vacuous test or an untested scoring gate. Why not static: To prove that a test is vacuous you must know that the grader never reads a file the test writes, which is dataflow across the filesystem. To prove that a gate is untested you must execute mutated code. Neither is AST-detectable: the stl argument is used (written to disk), so an unused-argument lint does not fire. Prevents: A1/A3 low, test_skillsbench_3d_scan_calc_verifier.py:55: the header-variation test is vacuous because the grader never opens scan_data.stl. A3 low, test file line 60: the material_id assert and the rel_tol=0.001 boundary can be deleted or changed and all 5 tests still pass.
• No rule for the redundant tuple re-alias (expected_mass, expected_vol, expected_id = EXPECTED_MASS, EXPECTED_VOL, EXPECTED_ID). No ruff rule exists for it, tasks/samples is excluded from ruff, and in a vendored file the aliasing can be deliberate, to keep the upstream diff small. Instead, the CE070 provenance header gives a place to say that, and the reviewer judges the rest. This is a deliberate decision not to add a static check. Why not static: A stylistic re-alias is only a defect when it has no purpose. Keeping a vendored diff small is a valid purpose, and only semantic judgment can tell the two cases apart. Prevents: A1 low, verifier/test_outputs.py:96: the redundant constant re-alias. It is recorded so that the boundary of static coverage is explicit.

Top 5 Priority Actions

1. Record the remaining reward-hack vectors at tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml:67-69 (the agent can change the pytest on PATH, install a pytest11 plugin, or edit $TASK_DIR/verifier/test_outputs.py; there is no task_dir digest check like orchestrator.py:1370): state in README.md and the yaml comment that the patch closes only the STL vector, and open a follow-up to grade outside the agent container or to digest-check sandbox.task_dir and run PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 <baked python> -I -m pytest --noconftest, and also fix the wrong read-only claim in .claude/notes/isolation.md (~line 518).
2. In tests/test_skillsbench_3d_scan_calc_verifier.py:60, add negative tests that submit material_id 43 (expect 'Expected Material ID') and mass _CORRECT_MASS * 1.002 (expect 'Mass mismatch'), so that the material-id gate at verifier/test_outputs.py:105 and the rel_tol=0.001 boundary at line 113 are under test.
3. Add a modification notice at the top of tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py:7 (e.g. '# Modified from benchflow-ai/skillsbench@95b3e60: grade against fixed EXPECTED_* (issue #233).'), so that a re-vendor does not silently undo the reward-hack fix and the file meets Apache-2.0 §4(b).
4. Delete the vacuous test_correct_answer_passes_when_the_stl_header_differs at tests/test_skillsbench_3d_scan_calc_verifier.py:55 together with the module-level _SHIPPED_STL (line 17), and give _grade an optional stl argument for the overwrite test only; also derive _CORRECT_MASS (line 20) and the repeated 42 (lines 21, 22, 52) from the loaded verifier's EXPECTED_MASS/EXPECTED_ID.
5. Do the cosmetic cleanup: remove the redundant tuple re-alias at verifier/test_outputs.py:96 or give its reason in a comment, and rewrap the 123-character comment at 3d-scan-calc.yaml:61 to match the rest of the block.

---

Stats: 0 🔴 · 0 🟠 · 1 🟡 · 6 🔵 across 8 axes reviewed.