fix(sandbox): validate LocalFile sources before hashing

Signed-off-by: matthewflint <277024436+matthewflint@users.noreply.github.com>

Author: matthewflint

src/agents/sandbox/entries/artifacts.py

@@ -23,7 +23,6 @@
 )
 from ..materialization import MaterializedFile, gather_in_order
 from ..types import ExecResult, User
-from ..util.checksums import sha256_file
 from .base import BaseEntry
 
 if TYPE_CHECKING:
@@ -114,11 +113,6 @@ async def apply(
         local_dir = LocalDir(src=self.src.parent)
         rel_child = Path(self.src.name)
         fd: int | None = None
-        try:
-            checksum = sha256_file(src)
-        except OSError as e:
-            raise LocalChecksumError(src=src, cause=e) from e
-        await session.mkdir(Path(dest).parent, parents=True)
         try:
             src_root = local_dir._resolve_local_dir_src_root(base_dir)
             fd = local_dir._open_local_dir_file_for_copy(
@@ -128,6 +122,12 @@ async def apply(
             )
             with os.fdopen(fd, "rb") as f:
                 fd = None
+                try:
+                    checksum = _sha256_handle(f)
+                    f.seek(0)
+                except OSError as e:
+                    raise LocalChecksumError(src=src, cause=e) from e
+                await session.mkdir(Path(dest).parent, parents=True)
                 await session.write(dest, f)
         except LocalDirReadError as e:
             context = dict(e.context)

tests/sandbox/test_entries.py

@@ -168,6 +168,25 @@ async def test_local_file_rejects_symlinked_source_leaf(tmp_path: Path) -> None:
     assert session.writes == {}
 
 
+@pytest.mark.asyncio
+async def test_local_file_rejects_symlinked_source_before_checksum(tmp_path: Path) -> None:
+    target_dir = tmp_path / "secret-dir"
+    target_dir.mkdir()
+    _symlink_or_skip(tmp_path / "link.txt", target_dir, target_is_directory=True)
+    session = _RecordingSession()
+
+    with pytest.raises(LocalFileReadError) as excinfo:
+        await LocalFile(src=Path("link.txt")).apply(
+            session,
+            Path("/workspace/copied.txt"),
+            tmp_path,
+        )
+
+    assert excinfo.value.context["reason"] == "symlink_not_supported"
+    assert excinfo.value.context["child"] == "link.txt"
+    assert session.writes == {}
+
+
 @pytest.mark.asyncio
 async def test_local_dir_copy_falls_back_when_safe_dir_fd_open_unavailable(
     monkeypatch: pytest.MonkeyPatch,
@@ -200,6 +219,9 @@ async def test_local_dir_copy_revalidates_swapped_paths_during_open(
     monkeypatch: pytest.MonkeyPatch,
     tmp_path: Path,
 ) -> None:
+    if not artifacts_module._OPEN_SUPPORTS_DIR_FD or not artifacts_module._HAS_O_DIRECTORY:
+        pytest.skip("safe dir_fd open pinning is unavailable on this platform")
+
     src_root = tmp_path / "src"
     src_root.mkdir()
     src_file = src_root / "safe.txt"
@@ -221,7 +243,7 @@ def swap_then_open(
         nonlocal swapped
         if path == "safe.txt" and not swapped:
             src_file.unlink()
-            src_file.symlink_to(secret)
+            _symlink_or_skip(src_file, secret)
             swapped = True
         if dir_fd is None:
             return original_open(path, flags, mode)
@@ -251,6 +273,9 @@ async def test_local_dir_copy_pins_parent_directories_during_open(
     monkeypatch: pytest.MonkeyPatch,
     tmp_path: Path,
 ) -> None:
+    if not artifacts_module._OPEN_SUPPORTS_DIR_FD or not artifacts_module._HAS_O_DIRECTORY:
+        pytest.skip("safe dir_fd open pinning is unavailable on this platform")
+
     src_root = tmp_path / "src"
     src_root.mkdir()
     nested_dir = src_root / "nested"
@@ -275,7 +300,7 @@ def swap_parent_then_open(
         nonlocal swapped
         if path == "safe.txt" and not swapped:
             (src_root / "nested").rename(src_root / "nested-original")
-            (src_root / "nested").symlink_to(secret_dir, target_is_directory=True)
+            _symlink_or_skip(src_root / "nested", secret_dir, target_is_directory=True)
             swapped = True
         if dir_fd is None:
             return original_open(path, flags, mode)
@@ -300,6 +325,9 @@ async def test_local_dir_apply_rejects_source_root_swapped_to_symlink_after_vali
     monkeypatch: pytest.MonkeyPatch,
     tmp_path: Path,
 ) -> None:
+    if not artifacts_module._OPEN_SUPPORTS_DIR_FD or not artifacts_module._HAS_O_DIRECTORY:
+        pytest.skip("safe dir_fd open pinning is unavailable on this platform")
+
     src_root = tmp_path / "src"
     src_root.mkdir()
     (src_root / "safe.txt").write_text("safe", encoding="utf-8")
@@ -321,7 +349,7 @@ def swap_root_then_open(
         nonlocal swapped
         if path == "src" and dir_fd is not None and not swapped:
             src_root.rename(tmp_path / "src-original")
-            (tmp_path / "src").symlink_to(secret_dir, target_is_directory=True)
+            _symlink_or_skip(tmp_path / "src", secret_dir, target_is_directory=True)
             swapped = True
         if dir_fd is None:
             return original_open(path, flags, mode)
@@ -393,7 +421,7 @@ async def test_local_dir_rejects_symlinked_source_ancestors(tmp_path: Path) -> N
     nested_dir = target_dir / "sub"
     nested_dir.mkdir()
     (nested_dir / "secret.txt").write_text("secret", encoding="utf-8")
-    (tmp_path / "link").symlink_to(target_dir, target_is_directory=True)
+    _symlink_or_skip(tmp_path / "link", target_dir, target_is_directory=True)
     session = _RecordingSession()
 
     with pytest.raises(LocalDirReadError) as excinfo:
@@ -409,7 +437,7 @@ async def test_local_dir_rejects_symlinked_source_root(tmp_path: Path) -> None:
     target_dir = tmp_path / "secret-dir"
     target_dir.mkdir()
     (target_dir / "secret.txt").write_text("secret", encoding="utf-8")
-    (tmp_path / "src").symlink_to(target_dir, target_is_directory=True)
+    _symlink_or_skip(tmp_path / "src", target_dir, target_is_directory=True)
     session = _RecordingSession()
 
     with pytest.raises(LocalDirReadError) as excinfo:
@@ -427,7 +455,7 @@ async def test_local_dir_rejects_symlinked_files(tmp_path: Path) -> None:
     (src_root / "safe.txt").write_text("safe", encoding="utf-8")
     secret = tmp_path / "secret.txt"
     secret.write_text("secret", encoding="utf-8")
-    (src_root / "link.txt").symlink_to(secret)
+    _symlink_or_skip(src_root / "link.txt", secret)
     session = _RecordingSession()
 
     with pytest.raises(LocalDirReadError) as excinfo:
@@ -446,7 +474,7 @@ async def test_local_dir_rejects_symlinked_directories(tmp_path: Path) -> None:
     target_dir = tmp_path / "secret-dir"
     target_dir.mkdir()
     (target_dir / "secret.txt").write_text("secret", encoding="utf-8")
-    (src_root / "linked-dir").symlink_to(target_dir, target_is_directory=True)
+    _symlink_or_skip(src_root / "linked-dir", target_dir, target_is_directory=True)
     session = _RecordingSession()
 
     with pytest.raises(LocalDirReadError) as excinfo:
@@ -492,10 +520,11 @@ async def test_git_repo_uses_fetch_checkout_path_for_commit_refs() -> None:
 @pytest.mark.asyncio
 async def test_dir_metadata_strips_file_type_bits_before_chmod() -> None:
     session = _RecordingSession()
+    dest = Path("/workspace/dir")
 
-    await Dir()._apply_metadata(session, Path("/workspace/dir"))
+    await Dir()._apply_metadata(session, dest)
 
-    assert ("chmod", "0755", "/workspace/dir") in session.exec_calls
+    assert ("chmod", "0755", str(dest)) in session.exec_calls
 
 
 @pytest.mark.asyncio
@@ -526,5 +555,5 @@ async def test_apply_manifest_raises_on_chgrp_failure() -> None:
     with pytest.raises(ExecNonZeroError):
         await session.apply_manifest()
 
-    assert ("chgrp", "sandbox-user", "/workspace/copied.txt") in session.exec_calls
+    assert ("chgrp", "sandbox-user", str(Path("/workspace/copied.txt"))) in session.exec_calls
     assert not any(call[0] == "chmod" for call in session.exec_calls)