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

src/agents/sandbox/sandboxes/docker.py

@@ -1374,7 +1374,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,51 @@ 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.
+    Only *simple* targets are rebased: every component is a plain name and every directory
+    the relative target walks through (the link's own parents and the target's parents) is
+    a directory member of the archive, with a regular-file leaf. Targets that depend on
+    another symlink (``alias/../x``), walk through directories absent from the archive, or
+    name directories stay absolute for strict hydration to refuse.
     """
 
     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()
+    members: dict[str, tarfile.TarInfo] = {}
+    candidates: list[tuple[tarfile.TarInfo, str]] = []
     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 +168,14 @@ 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)
+                        members[stripped_name] = rewritten
+                        if rewritten.issym() and symlink_root is not None:
+                            rebased = rebase_symlink_target(
+                                rewritten.linkname, link_name=stripped_name, root=symlink_root
+                            )
+                            if rebased != rewritten.linkname:
+                                candidates.append((rewritten, rebased))
+                                continue
                         if member.isreg():
                             fileobj = src.extractfile(member)
                             if fileobj is None:
@@ -155,16 +190,107 @@ def strip_tar_member_prefix(data: io.IOBase, *, prefix: str | Path) -> io.IOBase
                         else:
                             dst.addfile(rewritten)
 
+                    # Only symlink headers need the complete member inventory. Keep file
+                    # payloads in one output archive instead of copying them a second time.
+                    for member, candidate in candidates:
+                        if symlink_root is not None:
+                            parts = _simple_rebase_components(member.linkname, root=symlink_root)
+                            if parts is not None and _archive_establishes(
+                                parts, link_name=member.name, members=members
+                            ):
+                                member.linkname = candidate
+                                # A long source target lives in a PAX "linkpath" record
+                                # that would otherwise override the rewritten linkname.
+                                cast(dict[str, str], member.pax_headers).pop("linkpath", None)
+                        dst.addfile(member)
         out.seek(0)
         with tarfile.open(fileobj=out, mode="r:*") as tar:
             validate_tarfile(tar)
         out.seek(0)
         return cast(io.IOBase, out)
-    except Exception:
+    except BaseException:
         out.close()
         raise
 
 
+def _simple_rebase_components(linkname: str, *, root: str) -> tuple[str, ...] | None:
+    """Return the root-relative components of a *simple* absolute target under `root`.
+
+    Simple means every component after the root is a plain name: no ``.``, ``..``, or
+    empty segment, and no trailing separator. A leading ``//`` is collapsed to ``/`` and
+    the whole separator run at the root boundary is consumed (``/workspace//a.txt``). The
+    root itself (``/workspace`` or ``/workspace/``), targets outside the root, and
+    targets whose meaning depends on how a symlink component resolves (``alias/../x``),
+    return ``None`` and are left absolute for the strict hydrate check to refuse.
+    """
+
+    if not linkname.startswith("/"):
+        return None
+    target = "/" + linkname.lstrip("/")
+    prefix = "/" + root.strip("/")
+    if target == prefix:
+        rest = ""
+    elif target.startswith(prefix + "/"):
+        rest = target[len(prefix) :].lstrip("/")
+    else:
+        return None
+    if not rest or rest.endswith("/"):
+        # `/workspace/a.txt/` fails with ENOTDIR when `a.txt` is a file; dropping the
+        # separator would turn it into a working link, so it is not a simple target.
+        return None
+    parts = tuple(part for part in rest.split("/") if part)
+    if any(part in (".", "..") for part in parts):
+        return None
+    return parts
+
+
+def rebase_symlink_target(linkname: str, *, link_name: str, root: str) -> str:
+    """Rewrite a simple absolute symlink target under `root` as a target relative to the link.
+
+    Only targets whose components are all plain names are rewritten (see
+    `_simple_rebase_components`): ``/workspace/sub/data.txt`` from ``sub/abs_up`` becomes
+    ``../sub/data.txt``. Anything that would need a symlink component resolved to know
+    where it lands (``/workspace/alias/../data.txt``) is returned unchanged, so the strict
+    hydrate validation refuses it as an absolute target instead of this function guessing.
+    Whether the components are established by the archive itself is checked by
+    `strip_tar_member_prefix`, which sees every member.
+    """
+
+    parts = _simple_rebase_components(linkname, root=root)
+    if parts is None:
+        return linkname
+    climb = "/".join([".."] * len(PurePosixPath(link_name).parent.parts))
+    rest = "/".join(parts)
+    return f"{climb}/{rest}" if climb else rest
+
+
+def _archive_establishes(
+    parts: tuple[str, ...], *, link_name: str, members: dict[str, tarfile.TarInfo]
+) -> bool:
+    """Whether every component a rebased target walks through is an ordinary archive path.
+
+    Hydration extracts into an existing root, so a directory the archive does not create
+    could already be a symlink in the destination and redirect the restored link. Every
+    directory the relative target climbs out of (the link's own parents) and every
+    directory it descends into must therefore be a directory member of the archive, and
+    the leaf must be a regular file. Directory targets stay absolute: another relative
+    link can traverse a directory link followed by ``..``, so proving the rewritten link
+    alone is insufficient to preserve strict hydration's boundary.
+    """
+
+    link_parents = PurePosixPath(link_name).parent.parts
+    for depth in range(1, len(link_parents) + 1):
+        parent = members.get("/".join(link_parents[:depth]))
+        if parent is None or not parent.isdir():
+            return False
+    for depth in range(1, len(parts)):
+        intermediate = members.get("/".join(parts[:depth]))
+        if intermediate is None or not intermediate.isdir():
+            return False
+    leaf = members.get("/".join(parts))
+    return leaf is not None and leaf.isreg()
+
+
 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,106 @@ 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 whose target is a plain path the archive establishes comes back relative
+    (`/workspace/sub/data.txt` from `sub/abs_up` as `../sub/data.txt`); a target whose
+    destination depends on another link (`/workspace/alias/../data.txt`) is left absolute
+    and refused by strict hydration, see the sibling test."""
+    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 / "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"]
+        # The container runs GNU tar, which restores link targets verbatim. Spell that out
+        # instead of relying on `extractall`, whose default filter rewrites symlink targets
+        # on Python 3.14.
+        root = restored._host_path(cmd[3])
+        with tarfile.open(fileobj=stream, mode="r|*") as tar:
+            for member in tar:
+                dest = root / member.name
+                if member.isdir():
+                    dest.mkdir(parents=True, exist_ok=True)
+                elif member.issym():
+                    os.symlink(member.linkname, dest)
+                elif member.isreg():
+                    payload = tar.extractfile(member)
+                    assert payload is not None
+                    with payload:
+                        dest.write_bytes(payload.read())
+                else:
+                    raise AssertionError(f"unexpected member type: {member.name}")
+
+    restored._stream_into_exec = _extract_like_tar  # type: ignore[method-assign]
+    await restored.hydrate_workspace(archive)
+
+    # Assert on the restored link metadata: the targets are POSIX paths that only the
+    # sandbox's own filesystem resolves the way these assertions describe.
+    restored_workspace = restored_host_root / "workspace"
+    assert os.readlink(restored_workspace / "sub" / "abs_up") == "../sub/data.txt"
+    assert os.readlink(restored_workspace / "alias") == "sub/deep"
+    assert (restored_workspace / "sub" / "data.txt").read_text(encoding="utf-8") == "right"
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "target", ["/workspace/alias/../data.txt", "/workspace/sub/deep", "/workspace"]
+)
+async def test_docker_persist_keeps_unsupported_symlink_targets_absolute_for_hydrate(
+    tmp_path: Path,
+    target: str,
+) -> None:
+    """Directory and link-dependent targets stay absolute for strict hydration to refuse."""
+    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(target)
+    session = _HostBackedDockerSession(host_root=host_root, manifest=Manifest(root="/workspace"))
+
+    archive = await session.persist_workspace()
+
+    with tarfile.open(fileobj=archive, mode="r:*") as tar:
+        assert tar.getmember("abs_alias").linkname == target
+        assert tar.getmember("alias").linkname == "sub/deep"
+    archive.seek(0)
+
+    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")
+    )
+    with pytest.raises(WorkspaceArchiveWriteError):
+        await restored.hydrate_workspace(archive)
+
+
 @pytest.mark.asyncio
 async def test_docker_persist_workspace_closes_archive_http_response_after_normalization(
     tmp_path: Path,