From 607f6d19e9c823389e6e6816a9acd7b671f9d9a0 Mon Sep 17 00:00:00 2001 From: coderdailyone Date: Wed, 2 Sep 2026 16:39:49 +0000 Subject: [PATCH 1/4] fix(sandbox): make UnixLocal persist_workspace archives restorable UnixLocalSandboxSession.persist_workspace() archived the workspace with tarfile.add() unchanged, while hydrate_workspace() extracts with the strict policy that refuses hardlink members, FIFOs/device nodes and absolute symlink targets. Ordinary workspaces hit all three: uv and pnpm hardlink installed packages, dev servers leave FIFOs behind, and `ln -s "$PWD/file" link` writes an absolute target. The snapshot was taken successfully and then could never be restored ("hardlink member not allowed", "unsupported member type", "absolute symlink target not allowed: /tmp/sandbox-local-.../file"). Rewrite members while archiving: store hardlinks as regular files, drop FIFOs and device nodes, and turn an absolute symlink target that stays under the workspace root into a relative one so it also survives the root moving between sessions. Absolute targets outside the workspace are left unchanged; hydrate keeps rejecting them by design. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01BN4v25msJgrjNgac97g1Az --- src/agents/sandbox/sandboxes/unix_local.py | 38 +++++++++++++++- tests/sandbox/test_unix_local.py | 53 ++++++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index ea7f83e9d8..e819ede410 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/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,40 @@ 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 PurePosixPath(ti.linkname).is_absolute(): + normalized_target = Path(os.path.normpath(ti.linkname)) + link_dir = PurePosixPath(ti.name).parent + for candidate_root in (root, root.resolve(strict=False)): + try: + target_rel = normalized_target.relative_to(candidate_root) + except ValueError: + continue + ti.linkname = os.path.relpath(target_rel.as_posix() or ".", start=link_dir.as_posix()) + break + return ti + + def _restore_pty_child_signal_defaults() -> None: for signum in _PTY_CHILD_SIGNAL_DEFAULTS: signal.signal(signum, signal.SIG_DFL) @@ -1097,7 +1131,7 @@ def _archive_workspace() -> None: skip_rel_paths=skip, root_name=None, ) - else ti + else _restorable_tar_member(ti, root=root) ), ) diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index fb13be3c51..e4b18effbd 100644 --- a/tests/sandbox/test_unix_local.py +++ b/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,58 @@ 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 / "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["outside"].linkname == str(tmp_path / "elsewhere.txt") + + @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, From 570b70e2064986a99244b0abc1f835c884cfe6a1 Mon Sep 17 00:00:00 2001 From: coderdailyone Date: Fri, 4 Sep 2026 16:12:56 +0000 Subject: [PATCH 2/4] fix(sandbox): relativize in-workspace symlink targets that start with a double slash os.path.normpath keeps two leading slashes, so ///a.txt was not recognized as under the workspace root and stayed absolute; Linux resolves // as /, so collapse it before the containment check. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFSMrLZZ3oq1dKmJFAofz6 --- src/agents/sandbox/sandboxes/unix_local.py | 3 ++- tests/sandbox/test_unix_local.py | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index e819ede410..5f8eecedee 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/src/agents/sandbox/sandboxes/unix_local.py @@ -134,7 +134,8 @@ def _restorable_tar_member(ti: tarfile.TarInfo, *, root: Path) -> tarfile.TarInf ti.size = os.stat(root / ti.name).st_size return ti if ti.issym() and PurePosixPath(ti.linkname).is_absolute(): - normalized_target = Path(os.path.normpath(ti.linkname)) + # normpath keeps a leading "//"; Linux resolves it as "/", so collapse it first. + normalized_target = Path("/" + os.path.normpath(ti.linkname).lstrip("/")) link_dir = PurePosixPath(ti.name).parent for candidate_root in (root, root.resolve(strict=False)): try: diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index e4b18effbd..eea0933f28 100644 --- a/tests/sandbox/test_unix_local.py +++ b/tests/sandbox/test_unix_local.py @@ -484,6 +484,7 @@ def _workspace(tmp_path: Path) -> Path: (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 / "outside").symlink_to(tmp_path / "elsewhere.txt") return workspace @@ -504,6 +505,7 @@ async def test_persist_emits_restorable_members(self, tmp_path: Path) -> None: 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["outside"].linkname == str(tmp_path / "elsewhere.txt") @pytest.mark.asyncio From 1e1806d47cabd6d5fbd9bd1669ebdc3a2e1781ab Mon Sep 17 00:00:00 2001 From: coderdailyone Date: Sat, 5 Sep 2026 07:55:12 +0000 Subject: [PATCH 3/4] fix(sandbox): rebase absolute symlink targets without collapsing their components Replacing the workspace-root prefix of an absolute symlink target went through normpath(), which collapses `..` lexically. The kernel resolves `..` after a symlink component against the link target, so `/current/../config` with `current -> releases/v1` names `releases/config`, and the normalized `config` silently retargeted the restored link. Keep the target's components verbatim and only climb out of the link's own archive directory. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFSMrLZZ3oq1dKmJFAofz6 --- src/agents/sandbox/sandboxes/unix_local.py | 44 ++++++++++++++++------ tests/sandbox/test_unix_local.py | 29 ++++++++++++++ 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index 5f8eecedee..a44bddfce3 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/src/agents/sandbox/sandboxes/unix_local.py @@ -133,20 +133,42 @@ def _restorable_tar_member(ti: tarfile.TarInfo, *, root: Path) -> tarfile.TarInf ti.linkname = "" ti.size = os.stat(root / ti.name).st_size return ti - if ti.issym() and PurePosixPath(ti.linkname).is_absolute(): - # normpath keeps a leading "//"; Linux resolves it as "/", so collapse it first. - normalized_target = Path("/" + os.path.normpath(ti.linkname).lstrip("/")) - link_dir = PurePosixPath(ti.name).parent - for candidate_root in (root, root.resolve(strict=False)): - try: - target_rel = normalized_target.relative_to(candidate_root) - except ValueError: - continue - ti.linkname = os.path.relpath(target_rel.as_posix() or ".", start=link_dir.as_posix()) - break + 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 ``/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 + "/"): + rest = target[len(prefix) + 1 :] + 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) diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index eea0933f28..ea731e0a0f 100644 --- a/tests/sandbox/test_unix_local.py +++ b/tests/sandbox/test_unix_local.py @@ -508,6 +508,35 @@ async def test_persist_emits_restorable_members(self, tmp_path: Path) -> None: assert members["double_slash"].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: + """`/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) From 7133097a3671917f0121e233e0fdaeff145604c0 Mon Sep 17 00:00:00 2001 From: coderdailyone Date: Sun, 6 Sep 2026 03:57:36 +0000 Subject: [PATCH 4/4] fix(sandbox): consume the whole separator run after the workspace root when rebasing symlinks Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFSMrLZZ3oq1dKmJFAofz6 --- src/agents/sandbox/sandboxes/unix_local.py | 4 +++- tests/sandbox/test_unix_local.py | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index a44bddfce3..cb79d58184 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/src/agents/sandbox/sandboxes/unix_local.py @@ -157,7 +157,9 @@ def _rebase_symlink_target(linkname: str, *, link_name: str, roots: tuple[Path, if target == prefix: rest = "" elif target.startswith(prefix + "/"): - rest = target[len(prefix) + 1 :] + # Consume the whole separator run at the boundary (`//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 diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index ea731e0a0f..6741ff5191 100644 --- a/tests/sandbox/test_unix_local.py +++ b/tests/sandbox/test_unix_local.py @@ -485,6 +485,7 @@ def _workspace(tmp_path: Path) -> Path: (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 @@ -506,6 +507,7 @@ async def test_persist_emits_restorable_members(self, tmp_path: Path) -> None: 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