diff --git a/strix/runtime/local_dir_staging.py b/strix/runtime/local_dir_staging.py new file mode 100644 index 00000000..4758aa5c --- /dev/null +++ b/strix/runtime/local_dir_staging.py @@ -0,0 +1,120 @@ +"""Symlink-safe staging for ``LocalDir`` manifest uploads. + +The sandbox SDK's ``LocalDir`` walker refuses to copy symlinks at all — it +raises ``LocalDirReadError(reason="symlink_not_supported")`` on the first one +as a path-escape / TOCTOU safeguard. Real source trees (especially JS/TS +monorepos with workspace or shared-config links) routinely commit symlinks, so +handing such a tree straight to ``LocalDir`` aborts the upload before the agent +even starts. + +:func:`stage_symlink_safe_dir` returns a path that is always safe to hand to +``LocalDir``: + +* a tree with no symlinks is used as-is (no copy); +* otherwise the tree is copied into a temp directory with symlinks resolved: + + - a link whose target stays inside the tree is *dereferenced* (its target + content is materialized in place), so the agent still sees the file; + - a link that escapes the tree, dangles, or forms a cycle is *dropped* and + never followed. Refusing to follow out-of-tree links preserves the walker's + path-escape safety and keeps host/out-of-tree content from leaking into the + (hostile) sandbox. + +Regular files are hard-linked when possible (falling back to a copy across +devices), so the staged tree adds negligible disk for the non-symlink bulk. +""" + +from __future__ import annotations + +import logging +import os +import shutil +import tempfile +from pathlib import Path + + +logger = logging.getLogger(__name__) + +_STAGING_PREFIX = "strix-localdir-" + + +def _is_within(target: Path, root: Path) -> bool: + """Return whether ``target`` is ``root`` itself or nested under it.""" + if target == root: + return True + try: + target.relative_to(root) + except ValueError: + return False + return True + + +def tree_has_symlink(root: Path) -> bool: + """Return whether ``root`` contains any symlink (file or directory).""" + for dirpath, dirnames, filenames in os.walk(root, followlinks=False): + base = Path(dirpath) + for name in (*dirnames, *filenames): + if (base / name).is_symlink(): + return True + return False + + +def _link_or_copy(src: Path, dst: Path) -> None: + """Hard-link ``src`` to ``dst``, falling back to a content copy.""" + try: + os.link(src, dst) + except OSError: + shutil.copy2(src, dst, follow_symlinks=True) + + +def _stage_dir(src: Path, dst: Path, root: Path, seen: frozenset[Path]) -> None: + dst.mkdir(parents=True, exist_ok=True) + for entry in os.scandir(src): + entry_path = Path(entry.path) + dest_path = dst / entry.name + + if entry.is_symlink(): + target = Path(os.path.realpath(entry_path)) + if not _is_within(target, root): + logger.warning("staging: dropping out-of-tree symlink %s -> %s", entry_path, target) + continue + if not target.exists(): + logger.warning("staging: dropping dangling symlink %s", entry_path) + continue + if target in seen: + logger.warning("staging: dropping cyclic symlink %s -> %s", entry_path, target) + continue + if target.is_dir(): + _stage_dir(target, dest_path, root, seen | {target}) + else: + _link_or_copy(target, dest_path) + elif entry.is_dir(follow_symlinks=False): + _stage_dir(entry_path, dest_path, root, seen) + elif entry.is_file(follow_symlinks=False): + _link_or_copy(entry_path, dest_path) + else: + # Sockets, FIFOs, devices — not part of a source tree; skip. + logger.debug("staging: skipping non-regular entry %s", entry_path) + + +def stage_symlink_safe_dir(src_root: Path) -> tuple[Path, Path | None]: + """Return ``(upload_path, staged_temp)`` for uploading ``src_root``. + + ``upload_path`` is safe to hand to ``LocalDir``. When the tree contains no + symlinks it is ``src_root`` itself and ``staged_temp`` is ``None``. + Otherwise a symlink-safe copy is materialized in a temp directory and both + returned values point at it; the caller owns removing ``staged_temp`` once + the upload completes. + """ + root = src_root.resolve() + if not tree_has_symlink(root): + return root, None + + staged = Path(tempfile.mkdtemp(prefix=_STAGING_PREFIX)) + try: + _stage_dir(root, staged, root, frozenset({root})) + except OSError: + shutil.rmtree(staged, ignore_errors=True) + raise + logger.info("staging: materialized symlink-safe copy of %s at %s", root, staged) + return staged, staged diff --git a/strix/runtime/session_manager.py b/strix/runtime/session_manager.py index b16e79ba..5aa71423 100644 --- a/strix/runtime/session_manager.py +++ b/strix/runtime/session_manager.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging +import shutil from pathlib import Path from typing import Any @@ -12,6 +13,7 @@ from agents.sandbox.manifest import Environment, Manifest from strix.config import load_settings from strix.runtime.backends import get_backend from strix.runtime.caido_bootstrap import bootstrap_caido +from strix.runtime.local_dir_staging import stage_symlink_safe_dir logger = logging.getLogger(__name__) @@ -29,16 +31,20 @@ _WORKSPACE_ROOT = "/workspace" def build_session_entries( local_sources: list[dict[str, Any]], -) -> tuple[dict[str | Path, BaseEntry], list[dict[str, Any]]]: +) -> tuple[dict[str | Path, BaseEntry], list[dict[str, Any]], list[Path]]: """Split local sources into copied manifest entries and host bind mounts. Sources flagged ``mount`` are bind-mounted read-only at ``/workspace/`` (not added to the manifest, so the SDK does not stream them in file-by-file). Every other source becomes a - ``LocalDir`` entry copied into the container as before. + ``LocalDir`` entry copied into the container as before. Trees containing + symlinks (which the SDK's ``LocalDir`` walker refuses outright) are first + staged into a symlink-safe temp copy; those temp dirs are returned so the + caller can remove them once the upload completes. """ entries: dict[str | Path, BaseEntry] = {} bind_mounts: list[dict[str, Any]] = [] + staged_dirs: list[Path] = [] for src in local_sources: ws_subdir = src.get("workspace_subdir") or "" host_path = src.get("source_path") or "" @@ -54,8 +60,11 @@ def build_session_entries( } ) else: - entries[ws_subdir] = LocalDir(src=resolved) - return entries, bind_mounts + upload_path, staged = stage_symlink_safe_dir(resolved) + if staged is not None: + staged_dirs.append(staged) + entries[ws_subdir] = LocalDir(src=upload_path) + return entries, bind_mounts, staged_dirs async def create_or_reuse( @@ -75,7 +84,7 @@ async def create_or_reuse( logger.info("Reusing existing sandbox session for scan %s", scan_id) return cached - entries, bind_mounts = build_session_entries(local_sources) + entries, bind_mounts, staged_dirs = build_session_entries(local_sources) # Caido runs as an in-container sidecar; HTTP(S) traffic from any # process started via ``session.exec`` (the SDK's Shell tool, etc.) @@ -106,12 +115,16 @@ async def create_or_reuse( backend_name, image, ) - client, session = await backend( - image=image, - manifest=manifest, - exposed_ports=(_CONTAINER_CAIDO_PORT,), - bind_mounts=bind_mounts, - ) + try: + client, session = await backend( + image=image, + manifest=manifest, + exposed_ports=(_CONTAINER_CAIDO_PORT,), + bind_mounts=bind_mounts, + ) + finally: + for staged in staged_dirs: + shutil.rmtree(staged, ignore_errors=True) caido_endpoint = await session.resolve_exposed_port(_CONTAINER_CAIDO_PORT) scheme = "https" if caido_endpoint.tls else "http" diff --git a/tests/test_local_dir_staging.py b/tests/test_local_dir_staging.py new file mode 100644 index 00000000..89a60221 --- /dev/null +++ b/tests/test_local_dir_staging.py @@ -0,0 +1,108 @@ +"""Tests for symlink-safe LocalDir staging.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from strix.runtime.local_dir_staging import stage_symlink_safe_dir, tree_has_symlink + + +if TYPE_CHECKING: + from pathlib import Path + + +def _make_repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + (repo / "pkg").mkdir(parents=True) + (repo / "pkg" / "mod.py").write_text("x = 1\n") + (repo / "README.md").write_text("readme\n") + return repo + + +def test_tree_without_symlinks_used_as_is(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + + upload_path, staged = stage_symlink_safe_dir(repo) + + assert staged is None + assert upload_path == repo.resolve() + assert not tree_has_symlink(repo) + + +def test_in_tree_file_symlink_is_dereferenced(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + (repo / "link.py").symlink_to(repo / "pkg" / "mod.py") + + upload_path, staged = stage_symlink_safe_dir(repo) + + assert staged is not None + assert upload_path == staged + assert not (staged / "link.py").is_symlink() + assert (staged / "link.py").read_text() == "x = 1\n" + assert (staged / "pkg" / "mod.py").read_text() == "x = 1\n" + assert not tree_has_symlink(staged) + + +def test_in_tree_relative_dir_symlink_is_dereferenced(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + (repo / "pkg_alias").symlink_to("pkg") + + _upload, staged = stage_symlink_safe_dir(repo) + + assert staged is not None + assert (staged / "pkg_alias" / "mod.py").read_text() == "x = 1\n" + assert not tree_has_symlink(staged) + + +def test_out_of_tree_symlink_is_dropped(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + outside = tmp_path / "outside.txt" + outside.write_text("secret\n") + (repo / "escape.txt").symlink_to(outside) + (repo / "abs_escape").symlink_to("/etc") + + _upload, staged = stage_symlink_safe_dir(repo) + + assert staged is not None + assert not (staged / "escape.txt").exists() + assert not (staged / "abs_escape").exists() + assert (staged / "README.md").exists() + + +def test_dangling_symlink_is_dropped(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + (repo / "dangling").symlink_to(repo / "does-not-exist") + + _upload, staged = stage_symlink_safe_dir(repo) + + assert staged is not None + assert not (staged / "dangling").exists() + assert not (staged / "dangling").is_symlink() + + +def test_cyclic_symlink_terminates(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + (repo / "self").symlink_to(repo) + (repo / "pkg" / "up").symlink_to("..") + + _upload, staged = stage_symlink_safe_dir(repo) + + assert staged is not None + assert (staged / "README.md").exists() + assert not tree_has_symlink(staged) + + +def test_nested_symlinks_inside_linked_dir(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + shared = repo / "shared" + shared.mkdir() + (shared / "conf.json").write_text("{}\n") + (shared / "escape").symlink_to("/etc/passwd") + (repo / "pkg" / "shared_link").symlink_to(shared) + + _upload, staged = stage_symlink_safe_dir(repo) + + assert staged is not None + assert (staged / "pkg" / "shared_link" / "conf.json").read_text() == "{}\n" + assert not (staged / "pkg" / "shared_link" / "escape").exists() + assert not (staged / "shared" / "escape").exists() diff --git a/tests/test_session_entries.py b/tests/test_session_entries.py index 8288c1d0..e08c59aa 100644 --- a/tests/test_session_entries.py +++ b/tests/test_session_entries.py @@ -18,15 +18,18 @@ def _source(subdir: str, path: str, *, mount: bool = False) -> dict[str, Any]: def test_copied_source_becomes_localdir_entry(tmp_path: Path) -> None: - entries, bind_mounts = build_session_entries([_source("repo", str(tmp_path))]) + entries, bind_mounts, staged_dirs = build_session_entries([_source("repo", str(tmp_path))]) assert bind_mounts == [] + assert staged_dirs == [] assert isinstance(entries["repo"], LocalDir) assert entries["repo"].src == tmp_path.resolve() def test_mounted_source_becomes_bind_mount(tmp_path: Path) -> None: - entries, bind_mounts = build_session_entries([_source("repo", str(tmp_path), mount=True)]) + entries, bind_mounts, _staged = build_session_entries( + [_source("repo", str(tmp_path), mount=True)] + ) assert entries == {} assert bind_mounts == [ @@ -44,7 +47,7 @@ def test_mixed_sources_split_correctly(tmp_path: Path) -> None: copied.mkdir() mounted.mkdir() - entries, bind_mounts = build_session_entries( + entries, bind_mounts, _staged = build_session_entries( [ _source("copied", str(copied)), _source("mounted", str(mounted), mount=True), @@ -57,7 +60,7 @@ def test_mixed_sources_split_correctly(tmp_path: Path) -> None: def test_incomplete_sources_are_skipped() -> None: - entries, bind_mounts = build_session_entries( + entries, bind_mounts, staged_dirs = build_session_entries( [ {"source_path": "", "workspace_subdir": "x"}, {"source_path": "/p", "workspace_subdir": ""}, @@ -65,3 +68,20 @@ def test_incomplete_sources_are_skipped() -> None: ) assert entries == {} assert bind_mounts == [] + assert staged_dirs == [] + + +def test_symlink_tree_is_staged(tmp_path: Path) -> None: + repo = tmp_path / "repo" + repo.mkdir() + (repo / "real.txt").write_text("content") + (repo / "link.txt").symlink_to(repo / "real.txt") + + entries, _mounts, staged_dirs = build_session_entries([_source("repo", str(repo))]) + + assert len(staged_dirs) == 1 + entry = entries["repo"] + assert isinstance(entry, LocalDir) + assert entry.src == staged_dirs[0] + assert not (staged_dirs[0] / "link.txt").is_symlink() + assert (staged_dirs[0] / "link.txt").read_text() == "content"