diff --git a/.claude/notes/isolation.md b/.claude/notes/isolation.md index 03dc8bc8e..5e507ef73 100644 --- a/.claude/notes/isolation.md +++ b/.claude/notes/isolation.md @@ -516,9 +516,10 @@ the `~/.claude` copy the target can be an arbitrary path outside the staging tre `writable=False` is not cosmetic. The container only ever *reads* and `chmod`s the reference copy, so withholding the write bit keeps `_verify_reference_integrity` from being the sole guard against tampering during the gaps between windows. The task-dir copy is -read-only for the same reason: criteria read fixtures there, nothing legitimately writes -them, and withholding `o+w` keeps an agent from rewriting the expectations it is graded -against. The `~/.claude` copy IS writable — the CLI rewrites settings and state in place, +read-only too: criteria read fixtures there and nothing legitimately writes them. That +does not stop an agent from rewriting the expectations it is graded against: root keeps +`CAP_FOWNER` and can chmod the copy back, and `_verify_reference_integrity` does not cover +it. The `~/.claude` copy IS writable — the CLI rewrites settings and state in place, `copytree` preserves host modes, and `~/.claude` is routinely 0700 with 0600 files, which without DAC_OVERRIDE is unreadable to the container, so the agent cannot authenticate. diff --git a/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml b/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml index 46cfef5b5..2433241dc 100644 --- a/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml +++ b/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml @@ -58,9 +58,14 @@ run_limits: turn_timeout: 300 success_criteria: - # Run the skillsbench verifier verbatim: copy it from the task dir into a - # writable /verifier, run its test.sh (writes /logs/verifier/reward.txt), and - # emit that reward (0.0-1.0) as the criterion score. + # Run the skillsbench verifier (patched locally to grade against fixed + # expected values): copy it from the task dir into a writable /verifier, run + # its test.sh (writes /logs/verifier/reward.txt), and emit that reward + # (0.0-1.0) as the criterion score. + # The patch closes only the agent-writable scan_data.stl vector (#233). + # Grading still runs in the agent's container, so the pytest on PATH, + # auto-loaded pytest11 plugins, site-packages and the $TASK_DIR verifier + # copy remain tamperable; see the follow-up issue #238. - type: run_command command: | set -e diff --git a/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py b/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py index d12da4c2e..5f1ff6f40 100644 --- a/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py +++ b/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py @@ -1,17 +1,26 @@ +# Modified from benchflow-ai/skillsbench@95b3e60: grade against fixed EXPECTED_* (issue #233). import collections import json import math import os import struct +# Expected values for the shipped environment/scan_data.stl, computed once with +# _get_ground_truth() below. They are fixed here because the agent can write to its +# working copy of the STL, so grading must not read it. +# tests/test_skillsbench_3d_scan_calc_verifier.py checks they match the shipped file. +EXPECTED_MASS = 34648.04123490179 # coordinates read as cm +EXPECTED_VOL = 6242.890312594918 +EXPECTED_ID = 42 + class TestOutputs: def test_file_exists(self): """Check output file was created.""" assert os.path.exists("/root/mass_report.json"), "Output file not found" - def _get_ground_truth(self): - filepath = "/root/scan_data.stl" + def _get_ground_truth(self, filepath): + """Derive (mass, vol, id) from an STL. Not used for grading; see EXPECTED_*.""" triangles = [] # Binary Parse with Attribute @@ -85,7 +94,8 @@ def quantize(v): def test_values_correct(self): """Validate submission against ground truth.""" - expected_mass, expected_vol, expected_id = self._get_ground_truth() + # Local aliases so the upstream lines below stay unchanged (keeps the vendored diff small). + expected_mass, expected_vol, expected_id = EXPECTED_MASS, EXPECTED_VOL, EXPECTED_ID with open("/root/mass_report.json") as f: submission = json.load(f) diff --git a/tasks/samples/skillsbench/README.md b/tasks/samples/skillsbench/README.md index 1d819b934..0584d5d7b 100644 --- a/tasks/samples/skillsbench/README.md +++ b/tasks/samples/skillsbench/README.md @@ -8,7 +8,18 @@ They are vendored from [**benchflow-ai/skillsbench**](https://github.com/benchfl (Apache-2.0), pinned at commit [`95b3e60`](https://github.com/benchflow-ai/skillsbench/tree/95b3e60de7a075090e2f9137f32e0d24da1a289b). Each sample keeps the upstream prompt, inputs, and verifier; only the container -base and the coder_eval task wiring were added. +base and the coder_eval task wiring were added. The one local change is to the +`3d-scan-calc` verifier, which is patched to grade against fixed expected +values instead of re-deriving them from the agent-writable `scan_data.stl` +(issue #233). The other verifiers are unchanged and can still read +agent-writable inputs: `dialogue-parser` takes part of its ground truth from +`/app/script.txt`. + +That patch closes only the `scan_data.stl` vector. Grading still runs in the +agent's container, so the `pytest` on `PATH`, auto-loaded `pytest11` plugins, +site-packages, and the `$TASK_DIR` verifier copy remain tamperable by the +agent. See the follow-up issue +[#238](https://github.com/UiPath/coder_eval/issues/238). ## What they do @@ -44,8 +55,9 @@ coder-eval run tasks/samples/skillsbench/*/*.yaml ``` Each task builds its container, runs the agent (Claude Sonnet 4.6 by default) in -an isolated sandbox, then scores the result with the upstream verifier. Budget -roughly 1–3 minutes per task. +an isolated sandbox, then scores the result with the upstream verifier (for +`3d-scan-calc`, the locally patched one described above). Budget roughly 1–3 +minutes per task. ## Interpreting the results @@ -89,5 +101,6 @@ is non-deterministic, so exact numbers vary from run to run: These tasks are derived from benchflow-ai/skillsbench, licensed under Apache-2.0. The prompts, input data, and verifiers are the work of their original authors -(credited in each upstream task's `task.md`). See the pinned commit above for the -source of truth. +(credited in each upstream task's `task.md`). The `3d-scan-calc` verifier is +patched locally to grade against fixed expected values. See the pinned commit +above for the upstream source of truth. diff --git a/tests/test_skillsbench_3d_scan_calc_verifier.py b/tests/test_skillsbench_3d_scan_calc_verifier.py new file mode 100644 index 000000000..9af437440 --- /dev/null +++ b/tests/test_skillsbench_3d_scan_calc_verifier.py @@ -0,0 +1,83 @@ +"""Regression tests for the vendored ``skillsbench/3d-scan-calc`` sample verifier.""" + +from __future__ import annotations + +import importlib.util +import json +import struct +from pathlib import Path +from types import ModuleType +from typing import IO, Any + +import pytest + + +_TASK_DIR = Path(__file__).resolve().parents[1] / "tasks" / "samples" / "skillsbench" / "3d-scan-calc" +_SHIPPED_STL_PATH = _TASK_DIR / "environment" / "scan_data.stl" + + +def _load_verifier() -> ModuleType: + spec = importlib.util.spec_from_file_location("scan_calc_verifier", _TASK_DIR / "verifier" / "test_outputs.py") + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +_VERIFIER = _load_verifier() +_CORRECT_MASS: float = _VERIFIER.EXPECTED_MASS +_CORRECT_ID: int = _VERIFIER.EXPECTED_ID +_CORRECT = {"main_part_mass": _CORRECT_MASS, "material_id": _CORRECT_ID} +_MASS_ZERO = {"main_part_mass": 0, "material_id": _CORRECT_ID} +# One all-zero triangle whose attribute (material ID) is the expected one: volume 0. +_ZERO_STL = b"\0" * 80 + struct.pack(" None: + """Run ``test_values_correct`` with the verifier's ``/root/...`` paths redirected to ``tmp_path``. + + ``tmp_path`` stands in for the container's ``/root``: it holds the agent's + ``mass_report.json`` (*report*) and, if *stl* is given, an agent-written ``scan_data.stl``. + """ + module = _load_verifier() + if stl is not None: + (tmp_path / "scan_data.stl").write_bytes(stl) + (tmp_path / "mass_report.json").write_text(json.dumps(report)) + + def _open(path: str, *args: Any, **kwargs: Any) -> IO[Any]: + return open(str(path).replace("/root/", f"{tmp_path}/"), *args, **kwargs) + + monkeypatch.setattr(module, "open", _open, raising=False) + module.TestOutputs().test_values_correct() + + +@pytest.mark.parametrize("mass", [_CORRECT_MASS, _CORRECT_MASS / 1000.0], ids=["cm", "mm"]) +def test_correct_answer_passes(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, mass: float) -> None: + _grade(tmp_path, monkeypatch, {"main_part_mass": mass, "material_id": _CORRECT_ID}) + + +@pytest.mark.parametrize( + ("report", "message"), + [ + ({"main_part_mass": _CORRECT_MASS, "material_id": _CORRECT_ID + 1}, "Expected Material ID"), + ({"main_part_mass": _CORRECT_MASS * 1.002, "material_id": _CORRECT_ID}, "Mass mismatch"), + ], + ids=["wrong-material-id", "mass-just-outside-tolerance"], +) +def test_wrong_answer_fails( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, report: dict[str, float], message: str +) -> None: + with pytest.raises(AssertionError, match=message): + _grade(tmp_path, monkeypatch, report) + + +def test_overwritten_stl_does_not_change_the_expected_values(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + with pytest.raises(AssertionError, match="Mass mismatch"): + _grade(tmp_path, monkeypatch, _MASS_ZERO, stl=_ZERO_STL) + _grade(tmp_path, monkeypatch, _CORRECT, stl=_ZERO_STL) + + +def test_expected_values_match_the_shipped_stl() -> None: + module = _load_verifier() + derived = module.TestOutputs()._get_ground_truth(str(_SHIPPED_STL_PATH)) + assert derived == (module.EXPECTED_MASS, module.EXPECTED_VOL, module.EXPECTED_ID)