diff --git a/eval/tests/test_comparator_reuse.py b/eval/tests/test_comparator_reuse.py index 57f327ba4..d56a09a1d 100644 --- a/eval/tests/test_comparator_reuse.py +++ b/eval/tests/test_comparator_reuse.py @@ -8,6 +8,7 @@ from pathlib import Path import pytest +from workflow_bench import comparator_reuse from workflow_bench.comparator_reuse import ( ComparatorReuseExpectation, TaskReuseBinding, @@ -227,3 +228,29 @@ def test_a_changed_sandbox_dependency_is_not_the_same_baseline(): ) is False # A row that predates the field is not evidence of agreement either. assert row_is_reusable_comparator(_row(sandbox_dependency_manifest_digest=None), _expected()) is False + + +def test_reuse_directories_allow_a_symlinked_parent_but_not_a_symlinked_leaf(tmp_path: Path): + """Pins a deliberate difference from the sandbox's mount-root check. + + proposer_sandbox refuses every symlink hop because a hop changes what an + untrusted session is handed. A reuse directory is data, and every file + inside it is validated on its own, so a symlinked parent is allowed - + rejecting it would break a symlinked artifacts directory or macOS's /var + for no gain. The leaf itself must still be a real directory. + """ + + real = tmp_path / "real" + real.mkdir() + (real / "inner").mkdir() + linked_parent = tmp_path / "linked" + linked_parent.symlink_to(real, target_is_directory=True) + + # Reached through a symlinked parent: allowed, and resolved to the real path. + assert comparator_reuse._resolved_directory( + linked_parent / "inner", label="probe" + ) == (real / "inner").resolve() + + # The leaf itself being a symlink is still refused. + with pytest.raises(SandboxError, match="must be a real directory"): + comparator_reuse._resolved_directory(linked_parent, label="probe") diff --git a/eval/workflow_bench/comparator_reuse.py b/eval/workflow_bench/comparator_reuse.py index 3878f8fef..71d1d15cd 100644 --- a/eval/workflow_bench/comparator_reuse.py +++ b/eval/workflow_bench/comparator_reuse.py @@ -246,8 +246,8 @@ def materialize_reused_row( ) -> dict[str, Any]: """Copy digest-bound artifacts into this sweep's evidence dir and stamp reuse.""" - source = _real_directory(source_dir, label="reuse source") - dest = _real_directory(dest_dir, label="reuse destination") + source = _resolved_directory(source_dir, label="reuse source") + dest = _resolved_directory(dest_dir, label="reuse destination") if source == dest: raise SandboxError("comparator reuse cannot read and write the same results directory") @@ -332,7 +332,22 @@ def _transcript_metadata(metadata: Any) -> tuple[str, str, int]: return relative, digest, size -def _real_directory(path: Path, *, label: str) -> Path: +def _resolved_directory(path: Path, *, label: str) -> Path: + """An existing, non-symlink directory, resolved through its parents. + + Deliberately weaker than proposer_sandbox's same-shaped helper, which + refuses every symlink hop in the path. That one guards a MOUNT ROOT, where + a hop changes what an untrusted session is handed. This one guards a DATA + directory whose contents are validated individually anyway - every file + read goes through ``_regular_file`` (lstat, symlinks rejected) and every + write through ``O_NOFOLLOW`` - so a symlinked parent grants nothing those + guards do not already cover, while refusing one would reject ordinary + setups such as a symlinked artifacts directory or macOS's /var. + + Separately named because they make different promises. Do not merge them + without first deciding which promise the reuse path should make. + """ + resolved = path.expanduser() try: metadata = resolved.lstat()