diff --git a/src/agents/sandbox/sandboxes/docker.py b/src/agents/sandbox/sandboxes/docker.py index 3bc69a043b..ebf5d84bf0 100644 --- a/src/agents/sandbox/sandboxes/docker.py +++ b/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: diff --git a/src/agents/sandbox/util/tar_utils.py b/src/agents/sandbox/util/tar_utils.py index 55adbd77e4..c558262a70 100644 --- a/src/agents/sandbox/util/tar_utils.py +++ b/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() diff --git a/tests/sandbox/test_docker.py b/tests/sandbox/test_docker.py index e4c7cc812f..5c91520e52 100644 --- a/tests/sandbox/test_docker.py +++ b/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, diff --git a/tests/sandbox/test_tar_utils.py b/tests/sandbox/test_tar_utils.py index 50402557c6..b3cdd5c10e 100644 --- a/tests/sandbox/test_tar_utils.py +++ b/tests/sandbox/test_tar_utils.py @@ -1,21 +1,25 @@ from __future__ import annotations +import errno import io import os import stat import sys import tarfile from dataclasses import dataclass -from pathlib import Path +from pathlib import Path, PurePosixPath import pytest +from typing_extensions import Buffer +from agents.sandbox.util import tar_utils from agents.sandbox.util.tar_utils import ( UnsafeTarMemberError, safe_extract_tarfile, safe_tar_member_rel_path, strip_tar_member_prefix, validate_tar_bytes, + validate_tarfile, ) @@ -179,6 +183,367 @@ def test_strip_tar_member_prefix_returns_workspace_relative_archive() -> None: assert tar.getnames() == [".", "pkg", "pkg/main.py", "pkg/python"] +@pytest.mark.parametrize("absolute_link", [False, True]) +def test_strip_tar_member_prefix_retains_only_one_archive_payload( + monkeypatch: pytest.MonkeyPatch, absolute_link: bool +) -> None: + payload = b"workspace content\n" * 65536 + entries = [_dir("workspace")] + if absolute_link: + # The target follows the link in the input stream. + entries.append(_symlink("workspace/link", "/workspace/data.txt")) + entries.append(_file("workspace/data.txt", payload)) + raw = _tar_bytes(*entries) + storage: list[io.BytesIO] = [] + + class BudgetedArchive(io.BytesIO): + def write(self, data: Buffer) -> int: + written = super().write(data) + retained = sum(len(stream.getbuffer()) for stream in storage if not stream.closed) + if retained > len(raw) + tarfile.RECORDSIZE: + raise OSError(errno.ENOSPC, "archive storage budget exceeded") + return written + + def temporary_file() -> io.BytesIO: + stream = BudgetedArchive() + storage.append(stream) + return stream + + monkeypatch.setattr(tar_utils.tempfile, "TemporaryFile", temporary_file) + source = io.BytesIO(raw) + with strip_tar_member_prefix( + source, prefix="workspace", relativize_symlinks_under="/workspace" + ) as normalized: + assert source.closed + with tarfile.open(fileobj=normalized, mode="r:*") as archive: + validate_tarfile(archive, allow_external_symlink_targets=False) + restored = archive.extractfile("data.txt") + assert restored is not None + with restored: + assert restored.read() == payload + if absolute_link: + assert archive.getmember("link").linkname == "data.txt" + assert all(stream.closed for stream in storage) + + +@pytest.mark.parametrize("failure", ["read", "write", "validation"]) +def test_strip_tar_member_prefix_closes_streams_on_failure( + monkeypatch: pytest.MonkeyPatch, failure: str +) -> None: + entries = [_dir("workspace"), _file("workspace/data.txt")] + if failure == "validation": + entries.append(_file("workspace/data.txt", b"duplicate")) + + class Source(io.BytesIO): + def read(self, size: int | None = -1) -> bytes: + if failure == "read": + raise OSError("source read failed") + return super().read(size) + + class Output(io.BytesIO): + def write(self, data: Buffer) -> int: + if failure == "write": + raise OSError("archive write failed") + return super().write(data) + + source = Source(_tar_bytes(*entries)) + outputs: list[Output] = [] + + def temporary_file() -> Output: + output = Output() + outputs.append(output) + return output + + monkeypatch.setattr(tar_utils.tempfile, "TemporaryFile", temporary_file) + error = UnsafeTarMemberError if failure == "validation" else OSError + message = "duplicate archive path" if failure == "validation" else f"{failure} failed" + with pytest.raises(error, match=message): + strip_tar_member_prefix(source, prefix="workspace", relativize_symlinks_under="/workspace") + assert source.closed + assert outputs and all(output.closed for output in outputs) + + +def _prefixed_workspace_archive( + *, external_symlink: bool, link_through_alias: bool = True +) -> 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") + add_symlink(tar, "workspace/alias", "sub/deep") + if link_through_alias: + # `alias/..` resolves against the alias target (sub/deep): where this lands + # depends on another symlink, so the rewrite must leave it absolute. + 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. + nested = "workspace" + for _ in range(5): + nested += "/deeply-nested-directory" + add_dir(tar, nested) + add_file(tar, nested + "/target.txt", b"deep") + 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. + # Depends on how `alias` resolves: left absolute for strict hydration to refuse. + assert members["abs_alias"].linkname == "/workspace/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, link_through_alias=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) + + # Inspect the restored link metadata rather than reading through the links: the + # targets are POSIX paths that only a POSIX host resolves the way the sandbox does. + assert os.readlink(tmp_path / "sub" / "abs_up") == "../a.txt" + assert os.readlink(tmp_path / "alias") == "sub/deep" + assert (tmp_path / "sub" / "data.txt").read_bytes() == b"right" + assert not os.path.lexists(tmp_path / "dev.fifo") + + +def test_strip_tar_member_prefix_output_with_alias_link_is_refused_by_strict_hydrate() -> None: + """The archive keeps `/workspace/alias/../data.txt` absolute, and the strict hydrate + validation is what refuses it: this rewrite never guesses through another link.""" + 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: + assert tar.getmember("abs_alias").linkname == "/workspace/alias/../data.txt" + with pytest.raises(UnsafeTarMemberError, match="absolute symlink target not allowed"): + validate_tarfile(tar, allow_external_symlink_targets=False) + + +@pytest.mark.parametrize( + ("members", "victim", "target"), + [ + pytest.param( + (_dir("workspace/a"), _symlink("workspace/a/link", "..")), + "workspace/victim", + "/workspace/a/link/../tmp", + id="dot-dot after a link that climbs out", + ), + pytest.param( + (_symlink("workspace/outside", "/usr"),), + "workspace/victim", + "/workspace/outside/../x", + id="dot-dot after an external link", + ), + pytest.param( + (_file("workspace/secret", b"s"),), + "workspace/victim", + "/workspace/alias/../secret", + id="dot-dot through a component the archive does not create", + ), + pytest.param( + (), + "workspace/victim", + "/workspace/missing.txt", + id="leaf the archive does not create", + ), + pytest.param( + (_file("workspace/notes.txt", b"n"),), + "workspace/victim", + "/workspace/notes.txt/secret", + id="descends through a regular file", + ), + pytest.param( + (_symlink("workspace/loop", "loop"),), + "workspace/victim", + "/workspace/loop/x", + id="descends through a symlink member", + ), + pytest.param( + (_symlink("workspace/alias", "sub"), _dir("workspace/sub")), + "workspace/victim", + "/workspace/alias", + id="leaf is a symlink member", + ), + pytest.param( + (_file("workspace/a.txt", b"a"),), + "workspace/victim", + "/workspace/a.txt/", + id="trailing separator after a regular file (ENOTDIR on the source)", + ), + pytest.param( + (_dir("workspace/sub"),), + "workspace/victim", + "/workspace/sub/", + id="trailing separator after a directory", + ), + pytest.param( + (_dir("workspace/sub"),), + "workspace/victim", + "/workspace/sub", + id="directory target", + ), + pytest.param((), "workspace/victim", "/workspace", id="workspace root"), + pytest.param((), "workspace/victim", "/workspace/", id="workspace root with separator"), + pytest.param( + (_file("workspace/implied/deep/data.txt", b"d"),), + "workspace/victim", + "/workspace/implied/deep/data.txt", + id="directories only implied by a file path", + ), + pytest.param( + ( + _dir("workspace/a"), + _dir("workspace/sub"), + _file("workspace/data.txt", b"root"), + _symlink("workspace/a/link2", "../sub"), + _symlink("workspace/b", "a/link2"), + ), + "workspace/sub/victim", + "/workspace/b/../data.txt", + id="would resolve inside, but only by interpreting two links", + ), + ], +) +def test_strip_tar_member_prefix_leaves_non_simple_targets_absolute( + members: tuple[bytes, ...], victim: str, target: str +) -> None: + """Targets whose destination depends on another symlink, or that walk through a path + the archive does not establish as an ordinary directory, are not rewritten. They stay + absolute so the strict hydrate validation refuses them instead of a rewrite guessing.""" + raw = _tar_bytes(_dir("workspace"), *members, _symlink(victim, target)) + + stripped = strip_tar_member_prefix( + io.BytesIO(raw), prefix="workspace", relativize_symlinks_under="/workspace" + ) + + with tarfile.open(fileobj=stripped, mode="r:*") as tar: + name = victim.removeprefix("workspace/") + assert tar.getmember(name).linkname == target + with pytest.raises(UnsafeTarMemberError, match="absolute symlink target not allowed"): + validate_tarfile(tar, allow_external_symlink_targets=False) + + +def test_strip_tar_member_prefix_rebases_simple_targets_established_by_the_archive() -> None: + """Every directory the relative target walks through is an explicit directory member + and the leaf is a regular file, so the rewrite is exact.""" + raw = _tar_bytes( + _dir("workspace"), + _dir("workspace/sub"), + _dir("workspace/sub/deep"), + _file("workspace/sub/deep/data.txt", b"d"), + _dir("workspace/other"), + _symlink("workspace/other/victim", "/workspace/sub/deep/data.txt"), + ) + + stripped = strip_tar_member_prefix( + io.BytesIO(raw), prefix="workspace", relativize_symlinks_under="/workspace" + ) + + with tarfile.open(fileobj=stripped, mode="r:*") as tar: + assert tar.getmember("other/victim").linkname == "../sub/deep/data.txt" + validate_tarfile(tar, allow_external_symlink_targets=False) + + +def test_strip_tar_member_prefix_leaves_link_under_unestablished_parent_absolute() -> None: + """The link's own parent directory is only implied, so the `..` climb cannot be trusted + either: hydration could find a symlink there in the destination.""" + raw = _tar_bytes( + _dir("workspace"), + _file("workspace/data.txt", b"d"), + _symlink("workspace/implied/victim", "/workspace/data.txt"), + ) + + stripped = strip_tar_member_prefix( + io.BytesIO(raw), prefix="workspace", relativize_symlinks_under="/workspace" + ) + + with tarfile.open(fileobj=stripped, mode="r:*") as tar: + assert tar.getmember("implied/victim").linkname == "/workspace/data.txt" + + +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"