diff --git a/strix/runtime/local_dir_staging.py b/strix/runtime/local_dir_staging.py index 600d11250..a497b0c4e 100644 --- a/strix/runtime/local_dir_staging.py +++ b/strix/runtime/local_dir_staging.py @@ -36,6 +36,8 @@ logger = logging.getLogger(__name__) _STAGING_PREFIX = "strix-localdir-" +_DIR_ACCESS_MODE = os.R_OK | os.X_OK +_FILE_ACCESS_MODE = os.R_OK def _is_within(target: Path, root: Path) -> bool: @@ -59,6 +61,29 @@ def tree_has_symlink(root: Path) -> bool: return False +def _needs_safe_staging(root: Path) -> bool: + try: + with os.scandir(root) as entries: + for entry in entries: + if _entry_needs_safe_staging(entry): + return True + except OSError: + return True + return False + + +def _entry_needs_safe_staging(entry: os.DirEntry[str]) -> bool: + entry_path = Path(entry.path) + try: + if entry.is_symlink(): + return True + if entry.is_dir(follow_symlinks=False): + return not os.access(entry_path, _DIR_ACCESS_MODE) or _needs_safe_staging(entry_path) + return entry.is_file(follow_symlinks=False) and not os.access(entry_path, _FILE_ACCESS_MODE) + except OSError: + return True + + def _link_or_copy(src: Path, dst: Path) -> None: """Hard-link ``src`` to ``dst``, falling back to a content copy.""" try: @@ -69,32 +94,68 @@ def _link_or_copy(src: Path, dst: Path) -> None: 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 + try: + entries = list(os.scandir(src)) + except OSError as exc: + logger.warning("staging: skipping unreadable directory %s: %s", src, exc) + return - 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) + for entry in entries: + _stage_entry(entry, dst, root, seen) + + +def _stage_entry(entry: os.DirEntry[str], dst: Path, root: Path, seen: frozenset[Path]) -> None: + entry_path = Path(entry.path) + dest_path = dst / entry.name + + try: + is_symlink = entry.is_symlink() + is_dir = entry.is_dir(follow_symlinks=False) + is_file = entry.is_file(follow_symlinks=False) + except OSError as exc: + logger.warning("staging: skipping unreadable entry %s: %s", entry_path, exc) + return + + if is_symlink: + _stage_symlink(entry_path, dest_path, root, seen) + elif is_dir: + _stage_child_dir(entry_path, dest_path, root, seen) + elif is_file: + _stage_file(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(entry_path: Path, dest_path: Path, root: Path, seen: frozenset[Path]) -> None: + 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) + return + if not target.exists(): + logger.warning("staging: dropping dangling symlink %s", entry_path) + return + if target in seen: + logger.warning("staging: dropping cyclic symlink %s -> %s", entry_path, target) + return + if target.is_dir(): + _stage_dir(target, dest_path, root, seen | {target}) + return + _stage_file(target, dest_path) + + +def _stage_child_dir(entry_path: Path, dest_path: Path, root: Path, seen: frozenset[Path]) -> None: + if not os.access(entry_path, _DIR_ACCESS_MODE): + logger.warning("staging: skipping unreadable directory %s", entry_path) + return + _stage_dir(entry_path, dest_path, root, seen) + + +def _stage_file(entry_path: Path, dest_path: Path) -> None: + if not os.access(entry_path, _FILE_ACCESS_MODE): + logger.warning("staging: skipping unreadable file %s", entry_path) + return + _link_or_copy(entry_path, dest_path) def stage_symlink_safe_dir(src_root: Path) -> tuple[Path, Path | None]: @@ -107,7 +168,7 @@ def stage_symlink_safe_dir(src_root: Path) -> tuple[Path, Path | None]: the upload completes. """ root = src_root.resolve() - if not tree_has_symlink(root): + if not _needs_safe_staging(root): return root, None staged = Path(tempfile.mkdtemp(prefix=_STAGING_PREFIX)).resolve() diff --git a/tests/test_local_dir_staging.py b/tests/test_local_dir_staging.py index 25e293915..b0cd22f6d 100644 --- a/tests/test_local_dir_staging.py +++ b/tests/test_local_dir_staging.py @@ -2,13 +2,21 @@ from __future__ import annotations +import os +from pathlib import Path from typing import TYPE_CHECKING +from agents.sandbox.entries import LocalDir + from strix.runtime.local_dir_staging import stage_symlink_safe_dir, tree_has_symlink if TYPE_CHECKING: - from pathlib import Path + import pytest + + +UNREADABLE_MODE = 0 +RESTORED_DIR_MODE = 0o755 def _make_repo(tmp_path: Path) -> Path: @@ -108,7 +116,9 @@ def test_nested_symlinks_inside_linked_dir(tmp_path: Path) -> None: assert not (staged / "shared" / "escape").exists() -def test_staged_path_has_no_symlink_ancestor(tmp_path: Path, monkeypatch) -> None: # noqa: ANN001 +def test_staged_path_has_no_symlink_ancestor( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: """The staging directory itself must never sit behind a symlink. ``tempfile.mkdtemp()`` honors ``$TMPDIR``, and on macOS the default @@ -131,9 +141,7 @@ def fake_mkdtemp(prefix: str = "") -> str: real_dir.mkdir() return str(symlinked_tmp_root / real_dir.name) - monkeypatch.setattr( - "strix.runtime.local_dir_staging.tempfile.mkdtemp", fake_mkdtemp - ) + monkeypatch.setattr("strix.runtime.local_dir_staging.tempfile.mkdtemp", fake_mkdtemp) upload_path, staged = stage_symlink_safe_dir(repo) @@ -141,3 +149,26 @@ def fake_mkdtemp(prefix: str = "") -> str: assert upload_path == staged for path in (staged, *staged.parents): assert not path.is_symlink(), f"staged path has a symlink ancestor: {path}" + + +def test_unreadable_child_is_skipped_before_localdir_upload(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + unreadable = repo / "unreadable" + unreadable.mkdir() + (unreadable / "secret.txt").write_text("secret\n") + + try: + unreadable.chmod(UNREADABLE_MODE) + upload_path, staged = stage_symlink_safe_dir(repo) + local_dir = LocalDir(src=upload_path) + + local_files = local_dir._list_local_dir_files( + base_dir=Path(os.curdir), + src_root=upload_path, + ) + finally: + unreadable.chmod(RESTORED_DIR_MODE) + + assert staged is not None + assert Path("README.md") in local_files + assert Path("unreadable/secret.txt") not in local_files