fix(runtime): skip unreadable local source entries

This commit is contained in:
Ousama Ben Younes 2026-07-27 19:04:54 +00:00
parent ac0014fe65
commit 96b0ef2c7c
2 changed files with 123 additions and 31 deletions

View file

@ -36,6 +36,8 @@ from pathlib import Path
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()

View file

@ -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 test_staged_path_has_no_symlink_ancestor(tmp_path: Path, monkeypatch) -> Non
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 test_staged_path_has_no_symlink_ancestor(tmp_path: Path, monkeypatch) -> Non
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