fix(sandbox): make UnixLocal persist_workspace archives restorable by hydrate_workspace

src/agents/sandbox/sandboxes/unix_local.py

@@ -25,7 +25,7 @@
 from contextlib import suppress
 from dataclasses import dataclass, field
 from functools import partial
-from pathlib import Path
+from pathlib import Path, PurePosixPath
 from typing import Literal, cast
 
 from ...logger import log_tool_action_warning
@@ -112,6 +112,65 @@ def _close_fd_quietly(fd: int) -> None:
         os.close(fd)
 
 
+def _restorable_tar_member(ti: tarfile.TarInfo, *, root: Path) -> tarfile.TarInfo | None:
+    """Rewrite one ``persist_workspace`` member so ``hydrate_workspace`` can restore it.
+
+    The strict extractor used for hydrate refuses hardlink members, special files, and
+    absolute symlink targets. A local workspace legitimately contains all three (``uv`` and
+    ``pnpm`` hardlink installed packages, dev servers leave FIFOs behind, ``ln -s "$PWD/x"``
+    makes an absolute link), and archiving them as-is produced a snapshot that could never be
+    restored. Store hardlinks as regular files, drop FIFOs and device nodes, and make an
+    absolute symlink target that stays under the workspace root relative so it survives the
+    root moving between sessions. Absolute targets outside the workspace are kept unchanged.
+    """
+
+    if ti.isfifo() or ti.ischr() or ti.isblk():
+        return None
+    if ti.islnk():
+        # tarfile turns the second occurrence of an inode into a hardlink member with no
+        # payload; ``TarFile.add`` reads the file contents for a regular member instead.
+        ti.type = tarfile.REGTYPE
+        ti.linkname = ""
+        ti.size = os.stat(root / ti.name).st_size
+        return ti
+    if ti.issym() and ti.linkname.startswith("/"):
+        ti.linkname = _rebase_symlink_target(
+            ti.linkname, link_name=ti.name, roots=(root, root.resolve(strict=False))
+        )
+    return ti
+
+
+def _rebase_symlink_target(linkname: str, *, link_name: str, roots: tuple[Path, ...]) -> str:
+    """Rewrite an absolute symlink target under the workspace root as a link-relative one.
+
+    Only the root prefix is replaced; the remaining components are kept verbatim (no
+    normalization), because ``..`` after a symlink component is resolved by the kernel
+    against the link target, so ``<root>/current/../config`` with ``current -> releases/v1``
+    names ``releases/config`` and must stay ``current/../config``. Absolute targets outside
+    the workspace are returned unchanged. A leading ``//`` is collapsed to ``/`` (Linux
+    treats them alike).
+    """
+
+    target = "/" + linkname.lstrip("/")
+    for candidate_root in roots:
+        prefix = candidate_root.as_posix().rstrip("/")
+        if target == prefix:
+            rest = ""
+        elif target.startswith(prefix + "/"):
+            # Consume the whole separator run at the boundary (`<root>//a.txt`), keeping
+            # every later component, including `..`, untouched.
+            rest = target[len(prefix) :].lstrip("/")
+        else:
+            continue
+        # The link's own directory inside the archive holds no symlink components (the
+        # archive validator rejects members beneath a symlink), so climbing it is exact.
+        climb = "/".join([".."] * len(PurePosixPath(link_name).parent.parts))
+        if rest and climb:
+            return f"{climb}/{rest}"
+        return rest or climb or "."
+    return linkname
+
+
 def _restore_pty_child_signal_defaults() -> None:
     for signum in _PTY_CHILD_SIGNAL_DEFAULTS:
         signal.signal(signum, signal.SIG_DFL)
@@ -1097,7 +1156,7 @@ def _archive_workspace() -> None:
                             skip_rel_paths=skip,
                             root_name=None,
                         )
-                        else ti
+                        else _restorable_tar_member(ti, root=root)
                     ),
                 )
 

tests/sandbox/test_unix_local.py

@@ -2,6 +2,7 @@
 
 import asyncio
 import io
+import os
 import signal
 import tarfile
 import threading
@@ -470,6 +471,91 @@ async def test_rm_as_user_checks_permissions_then_uses_local_fs(
         assert not any(part.startswith("rm ") for part in session.exec_commands[0])
 
 
+class TestUnixLocalPersistWorkspaceRestorable:
+    """persist_workspace must only emit members that the strict hydrate extractor accepts."""
+
+    @staticmethod
+    def _workspace(tmp_path: Path) -> Path:
+        workspace = tmp_path / "workspace"
+        (workspace / "sub").mkdir(parents=True)
+        (workspace / "a.txt").write_text("shared", encoding="utf-8")
+        os.link(workspace / "a.txt", workspace / "sub" / "hardlink.txt")
+        os.mkfifo(workspace / "dev.fifo")
+        (workspace / "abs_inside").symlink_to(workspace / "a.txt")
+        (workspace / "sub" / "abs_up").symlink_to(workspace / "a.txt")
+        (workspace / "rel").symlink_to("a.txt")
+        (workspace / "double_slash").symlink_to("/" + str(workspace / "a.txt"))
+        (workspace / "double_sep").symlink_to(str(workspace) + "//a.txt")
+        (workspace / "outside").symlink_to(tmp_path / "elsewhere.txt")
+        return workspace
+
+    @pytest.mark.asyncio
+    async def test_persist_emits_restorable_members(self, tmp_path: Path) -> None:
+        workspace = self._workspace(tmp_path)
+        session = _RecordingUnixLocalSession(workspace)
+
+        blob = await session.persist_workspace()
+
+        with tarfile.open(fileobj=cast(io.BytesIO, blob), mode="r:*") as tar:
+            members = {member.name.removeprefix("./"): member for member in tar.getmembers()}
+            assert "dev.fifo" not in members
+            hardlink = members["sub/hardlink.txt"]
+            assert hardlink.isreg() and hardlink.size == len("shared")
+            extracted = tar.extractfile(hardlink)
+            assert extracted is not None and extracted.read() == b"shared"
+            assert members["abs_inside"].linkname == "a.txt"
+            assert members["sub/abs_up"].linkname == "../a.txt"
+            assert members["rel"].linkname == "a.txt"
+            assert members["double_slash"].linkname == "a.txt"
+            assert members["double_sep"].linkname == "a.txt"
+            assert members["outside"].linkname == str(tmp_path / "elsewhere.txt")
+
+    @pytest.mark.asyncio
+    async def test_rebased_symlink_keeps_parent_steps_after_symlink_components(
+        self,
+        tmp_path: Path,
+    ) -> None:
+        """`<root>/current/../config` with `current -> releases/v1` names `releases/config`;
+        collapsing the `..` lexically would silently retarget the restored link."""
+        workspace = tmp_path / "workspace"
+        (workspace / "releases" / "v1").mkdir(parents=True)
+        (workspace / "releases" / "config").write_text("right", encoding="utf-8")
+        (workspace / "config").write_text("wrong", encoding="utf-8")
+        (workspace / "current").symlink_to("releases/v1")
+        (workspace / "abs_config").symlink_to(workspace / "current" / ".." / "config")
+        (workspace / "releases" / "v1" / "abs_up").symlink_to(
+            workspace / "current" / ".." / "config"
+        )
+        assert (workspace / "abs_config").read_text(encoding="utf-8") == "right"
+
+        blob = await _RecordingUnixLocalSession(workspace).persist_workspace()
+        restored_root = tmp_path / "restored"
+        await _RecordingUnixLocalSession(restored_root).hydrate_workspace(blob)
+
+        assert os.readlink(restored_root / "abs_config") == "current/../config"
+        assert (
+            os.readlink(restored_root / "releases" / "v1" / "abs_up") == "../../current/../config"
+        )
+        assert (restored_root / "abs_config").read_text(encoding="utf-8") == "right"
+        assert (restored_root / "releases" / "v1" / "abs_up").read_text(encoding="utf-8") == "right"
+
+    @pytest.mark.asyncio
+    async def test_persisted_workspace_hydrates_into_a_new_root(self, tmp_path: Path) -> None:
+        workspace = self._workspace(tmp_path)
+        (workspace / "outside").unlink()  # hydrate rejects external targets by design
+        blob = await _RecordingUnixLocalSession(workspace).persist_workspace()
+
+        restored_root = tmp_path / "restored"
+        restored = _RecordingUnixLocalSession(restored_root)
+        await restored.hydrate_workspace(blob)
+
+        assert (restored_root / "sub" / "hardlink.txt").read_text(encoding="utf-8") == "shared"
+        assert not (restored_root / "dev.fifo").exists()
+        assert os.readlink(restored_root / "abs_inside") == "a.txt"
+        assert (restored_root / "abs_inside").read_text(encoding="utf-8") == "shared"
+        assert (restored_root / "sub" / "abs_up").read_text(encoding="utf-8") == "shared"
+
+
 @pytest.mark.asyncio
 async def test_hydrate_workspace_cancellation_waits_for_the_extracting_worker(
     tmp_path: Path,