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

src/agents/sandbox/sandboxes/docker.py

@@ -1371,7 +1371,11 @@ async def persist_workspace(self) -> io.IOBase:
                 staging_workspace,
                 cleanup_path=staging_parent,
             )
-            return strip_tar_member_prefix(root_prefixed_archive, prefix=staging_workspace.name)
+            return strip_tar_member_prefix(
+                root_prefixed_archive,
+                prefix=staging_workspace.name,
+                relativize_symlinks_under=root,
+            )
         except docker.errors.NotFound as e:
             raise WorkspaceArchiveReadError(path=error_root, cause=e, retryable=False) from e
         except docker.errors.APIError as e:

src/agents/sandbox/util/tar_utils.py

@@ -7,7 +7,7 @@
 import tarfile
 import tempfile
 from collections.abc import Iterable
-from pathlib import Path, PurePosixPath, PureWindowsPath
+from pathlib import Path, PurePath, PurePosixPath, PureWindowsPath
 from typing import cast
 
 
@@ -100,24 +100,44 @@ def safe_tar_member_rel_path(
     return Path(*rel.parts)
 
 
-def strip_tar_member_prefix(data: io.IOBase, *, prefix: str | Path) -> io.IOBase:
+def strip_tar_member_prefix(
+    data: io.IOBase,
+    *,
+    prefix: str | Path,
+    relativize_symlinks_under: str | PurePath | None = None,
+) -> io.IOBase:
     """Return a seekable tar stream after replacing a leading member prefix with `.`.
 
     For example, Docker archives a workspace copied to `/tmp/stage/workspace`
     as `workspace/...`; portable workspace snapshots should store the same
     files as `.` and `...`, independent of the source backend's root name.
+
+    The strict hydrate extractor refuses FIFOs, device nodes, and absolute symlink
+    targets, and a staged workspace copy (`cp -R`) legitimately carries the first two
+    kinds and absolute links into the workspace. FIFOs and device nodes are dropped, and
+    when `relativize_symlinks_under` names the workspace root, an absolute symlink target
+    under it is rebased onto the link's own directory with its components kept verbatim.
     """
 
     prefix_rel = _normalize_rel(prefix)
     if prefix_rel == Path():
         raise ValueError("tar member prefix must not be empty")
+    symlink_root: str | None = None
+    if relativize_symlinks_under is not None:
+        symlink_root = (
+            relativize_symlinks_under.as_posix()
+            if isinstance(relativize_symlinks_under, PurePath)
+            else relativize_symlinks_under
+        )
 
     out = tempfile.TemporaryFile()
     try:
         with data:
             with tarfile.open(fileobj=data, mode="r|*") as src:
                 with tarfile.open(fileobj=out, mode="w|") as dst:
                     for member in src:
+                        if member.isfifo() or member.ischr() or member.isblk():
+                            continue
                         rel_path = safe_tar_member_rel_path(
                             member,
                             allow_symlinks=True,
@@ -141,6 +161,13 @@ def strip_tar_member_prefix(data: io.IOBase, *, prefix: str | Path) -> io.IOBase
                         rewritten.name = stripped_name
                         rewritten.pax_headers = dict(member.pax_headers)
                         rewritten.pax_headers.pop("path", None)
+                        if rewritten.issym() and symlink_root is not None:
+                            rewritten.linkname = rebase_symlink_target(
+                                rewritten.linkname, link_name=stripped_name, root=symlink_root
+                            )
+                            # A long source target lives in a PAX "linkpath" record that
+                            # would otherwise override the rewritten linkname.
+                            rewritten.pax_headers.pop("linkpath", None)
                         if member.isreg():
                             fileobj = src.extractfile(member)
                             if fileobj is None:
@@ -165,6 +192,37 @@ def strip_tar_member_prefix(data: io.IOBase, *, prefix: str | Path) -> io.IOBase
         raise
 
 
+def rebase_symlink_target(linkname: str, *, link_name: str, root: str) -> str:
+    """Rewrite an absolute symlink target under `root` as a target relative to the link.
+
+    Only the root prefix is replaced by the climb out of the link's own archive directory;
+    the remaining components are kept verbatim, because the kernel resolves ``..`` after a
+    symlink component against that link's target: with ``alias -> sub/deep``,
+    ``/workspace/alias/../data.txt`` names ``sub/data.txt`` and must stay
+    ``alias/../data.txt``. Absolute targets outside the root are returned unchanged. A
+    leading ``//`` is collapsed to ``/`` (Linux treats them alike).
+    """
+
+    if not linkname.startswith("/"):
+        return linkname
+    target = "/" + linkname.lstrip("/")
+    prefix = "/" + root.strip("/")
+    if target == prefix:
+        rest = ""
+    elif target.startswith(prefix + "/"):
+        # Consume the whole separator run at the boundary (`/workspace//a.txt`), keeping
+        # every later component, including `..`, untouched.
+        rest = target[len(prefix) :].lstrip("/")
+    else:
+        return linkname
+    # Members beneath a symlink are rejected by the archive validator, so the link's
+    # archive directory holds no symlink components and 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 "."
+
+
 def _normalize_rel(prefix: str | Path) -> Path:
     rel = prefix if isinstance(prefix, Path) else Path(prefix)
     posix = rel.as_posix()

tests/sandbox/test_docker.py

@@ -4,6 +4,7 @@
 import builtins
 import errno
 import io
+import os
 import queue
 import shutil
 import socket
@@ -501,9 +502,10 @@ async def _exec_internal(
             src = self._host_path(cmd[3])
             dst = self._host_path(cmd[4])
             if src.is_dir():
-                shutil.copytree(src, dst)
+                # Like `cp -R`, keep symlinks as symlinks instead of following them.
+                shutil.copytree(src, dst, symlinks=True)
             else:
-                shutil.copy2(src, dst)
+                shutil.copy2(src, dst, follow_symlinks=False)
             return ExecResult(stdout=b"", stderr=b"", exit_code=0)
         if cmd[:2] == ["cat", "--"]:
             src = self._host_path(cmd[2])
@@ -751,6 +753,54 @@ async def test_docker_persist_workspace_stages_copy_before_get_archive(
     assert not any(name == "workspace" or name.startswith("workspace/") for name in names)
 
 
+@pytest.mark.asyncio
+async def test_docker_persist_and_hydrate_keep_absolute_workspace_symlinks_resolving(
+    tmp_path: Path,
+) -> None:
+    """Persist stages the workspace with `cp -R`, the daemon archives the copy, and the
+    archive is normalized for the strict hydrate extractor. An absolute in-workspace
+    symlink must come back relative *with its components intact*: `alias -> sub/deep`
+    makes `/workspace/alias/../data.txt` name `sub/data.txt`, not `data.txt`."""
+    host_root = tmp_path / "container"
+    workspace = host_root / "workspace"
+    (workspace / "sub" / "deep").mkdir(parents=True)
+    (workspace / "data.txt").write_text("wrong", encoding="utf-8")
+    (workspace / "sub" / "data.txt").write_text("right", encoding="utf-8")
+    (workspace / "alias").symlink_to("sub/deep")
+    (workspace / "abs_alias").symlink_to("/workspace/alias/../data.txt")
+    (workspace / "sub" / "abs_up").symlink_to("/workspace/sub/data.txt")
+    session = _HostBackedDockerSession(host_root=host_root, manifest=Manifest(root="/workspace"))
+
+    archive = await session.persist_workspace()
+
+    restored_host_root = tmp_path / "restored-container"
+    (restored_host_root / "workspace").mkdir(parents=True)
+    restored = _HostBackedDockerSession(
+        host_root=restored_host_root, manifest=Manifest(root="/workspace")
+    )
+
+    async def _extract_like_tar(
+        *,
+        cmd: list[str],
+        stream: io.IOBase,
+        error_path: Path,
+        user: object = None,
+    ) -> None:
+        _ = (error_path, user)
+        assert cmd[:3] == ["tar", "-x", "-C"]
+        with tarfile.open(fileobj=stream, mode="r|*") as tar:
+            tar.extractall(restored._host_path(cmd[3]))
+
+    restored._stream_into_exec = _extract_like_tar  # type: ignore[method-assign]
+    await restored.hydrate_workspace(archive)
+
+    restored_workspace = restored_host_root / "workspace"
+    assert os.readlink(restored_workspace / "abs_alias") == "alias/../data.txt"
+    assert os.readlink(restored_workspace / "sub" / "abs_up") == "../sub/data.txt"
+    assert (restored_workspace / "abs_alias").read_text(encoding="utf-8") == "right"
+    assert (restored_workspace / "sub" / "abs_up").read_text(encoding="utf-8") == "right"
+
+
 @pytest.mark.asyncio
 async def test_docker_persist_workspace_closes_archive_http_response_after_normalization(
     tmp_path: Path,

tests/sandbox/test_tar_utils.py

@@ -6,7 +6,7 @@
 import sys
 import tarfile
 from dataclasses import dataclass
-from pathlib import Path
+from pathlib import Path, PurePosixPath
 
 import pytest
 
@@ -16,6 +16,7 @@
     safe_tar_member_rel_path,
     strip_tar_member_prefix,
     validate_tar_bytes,
+    validate_tarfile,
 )
 
 
@@ -179,6 +180,117 @@ def test_strip_tar_member_prefix_returns_workspace_relative_archive() -> None:
         assert tar.getnames() == [".", "pkg", "pkg/main.py", "pkg/python"]
 
 
+def _prefixed_workspace_archive(*, external_symlink: bool) -> io.BytesIO:
+    """A `workspace/...` archive shaped like Docker's staged copy, with members that the
+    strict hydrate extractor refuses as-is."""
+
+    def add_dir(tar: tarfile.TarFile, name: str) -> None:
+        info = tarfile.TarInfo(name)
+        info.type = tarfile.DIRTYPE
+        tar.addfile(info)
+
+    def add_file(tar: tarfile.TarFile, name: str, payload: bytes) -> None:
+        info = tarfile.TarInfo(name)
+        info.size = len(payload)
+        tar.addfile(info, io.BytesIO(payload))
+
+    def add_symlink(tar: tarfile.TarFile, name: str, target: str) -> None:
+        info = tarfile.TarInfo(name)
+        info.type = tarfile.SYMTYPE
+        info.linkname = target
+        tar.addfile(info)
+
+    buf = io.BytesIO()
+    with tarfile.open(fileobj=buf, mode="w") as tar:
+        add_dir(tar, "workspace")
+        add_dir(tar, "workspace/sub")
+        add_dir(tar, "workspace/sub/deep")
+        add_file(tar, "workspace/a.txt", b"shared")
+        add_file(tar, "workspace/data.txt", b"wrong")
+        add_file(tar, "workspace/sub/data.txt", b"right")
+        fifo = tarfile.TarInfo("workspace/dev.fifo")
+        fifo.type = tarfile.FIFOTYPE
+        tar.addfile(fifo)
+        add_symlink(tar, "workspace/sub/abs_up", "/workspace/a.txt")
+        add_symlink(tar, "workspace/rel", "a.txt")
+        add_symlink(tar, "workspace/double_slash", "//workspace/a.txt")
+        add_symlink(tar, "workspace/double_sep", "/workspace//a.txt")
+        # `alias/..` resolves against the alias target (sub/deep), so this names sub/data.txt.
+        add_symlink(tar, "workspace/alias", "sub/deep")
+        add_symlink(tar, "workspace/abs_alias", "/workspace/alias/../data.txt")
+        # Longer than the 100-byte ustar field, so tarfile records it in a PAX linkpath.
+        long_target = "/workspace/" + "/".join(["deeply-nested-directory"] * 5) + "/target.txt"
+        add_symlink(tar, "workspace/long_link", long_target)
+        if external_symlink:
+            add_symlink(tar, "workspace/outside", "/usr/bin/python3")
+    buf.seek(0)
+    return buf
+
+
+def test_strip_tar_member_prefix_rewrites_members_hydrate_refuses() -> None:
+    stripped = strip_tar_member_prefix(
+        _prefixed_workspace_archive(external_symlink=True),
+        prefix="workspace",
+        relativize_symlinks_under="/workspace",
+    )
+
+    with tarfile.open(fileobj=stripped, mode="r:*") as tar:
+        members = {member.name: member for member in tar.getmembers()}
+        assert "dev.fifo" not in members
+        assert members["sub/abs_up"].issym()
+        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"
+        # Components after the root prefix are kept verbatim; `..` is not collapsed.
+        assert members["abs_alias"].linkname == "alias/../data.txt"
+        long_link = members["long_link"]
+        assert long_link.linkname == "/".join(["deeply-nested-directory"] * 5) + "/target.txt"
+        assert "linkpath" not in long_link.pax_headers or (
+            long_link.pax_headers["linkpath"] == long_link.linkname
+        )
+        # External absolute targets are left for hydrate's policy to decide.
+        assert members["outside"].linkname == "/usr/bin/python3"
+
+
+def test_strip_tar_member_prefix_output_passes_strict_hydrate_validation(
+    tmp_path: Path,
+) -> None:
+    stripped = strip_tar_member_prefix(
+        _prefixed_workspace_archive(external_symlink=False),
+        prefix="workspace",
+        relativize_symlinks_under=PurePosixPath("/workspace"),
+    )
+
+    with tarfile.open(fileobj=stripped, mode="r:*") as tar:
+        validate_tarfile(tar, allow_external_symlink_targets=False)
+        safe_extract_tarfile(tar, root=tmp_path, allow_external_symlink_targets=False)
+
+    assert (tmp_path / "sub" / "abs_up").read_bytes() == b"shared"
+    assert (tmp_path / "abs_alias").read_bytes() == b"right"
+    assert not (tmp_path / "dev.fifo").exists()
+
+
+def test_strip_tar_member_prefix_keeps_absolute_symlinks_without_a_root() -> None:
+    stripped = strip_tar_member_prefix(
+        _prefixed_workspace_archive(external_symlink=False), prefix="workspace"
+    )
+
+    with tarfile.open(fileobj=stripped, mode="r:*") as tar:
+        assert tar.getmember("sub/abs_up").linkname == "/workspace/a.txt"
+
+
+def test_strip_tar_member_prefix_still_rejects_hardlink_members() -> None:
+    raw = _tar_bytes(
+        _dir("workspace"),
+        _file("workspace/a.txt", b"x"),
+        _hardlink("workspace/b.txt", "workspace/a.txt"),
+    )
+
+    with pytest.raises(UnsafeTarMemberError, match="hardlink member not allowed"):
+        strip_tar_member_prefix(io.BytesIO(raw), prefix="workspace")
+
+
 def test_strip_tar_member_prefix_rewrites_pax_path_headers() -> None:
     long_name = "workspace/" + ("a" * 120) + ".txt"
     payload = b"payload"