diff --git a/strix/interface/cli_args.py b/strix/interface/cli_args.py index 6c672437..5601b990 100644 --- a/strix/interface/cli_args.py +++ b/strix/interface/cli_args.py @@ -362,16 +362,21 @@ def _load_resume_state(args: argparse.Namespace, parser: argparse.ArgumentParser if not getattr(args, "user_instruction", None): args.user_instruction = state.get("user_instruction") or None args.local_sources = collect_local_sources(args.targets_info) - # Remount the workspace the run was started with. The user already confirmed - # this directory, so the target mount guard does not apply to it; it only has - # to still be there. + # Remount the workspace the run was started with. Exact $HOME remains + # allowed (operator already confirmed it), but system/credential paths are + # re-checked so a poisoned run.json cannot rehydrate a refused mount. args.workspace_mount = workspace_mount if workspace_mount: - if not Path(workspace_mount).expanduser().is_dir(): + mount_path = Path(workspace_mount).expanduser() + if not mount_path.is_dir(): parser.error( f"--resume {args.resume}: the working directory {workspace_mount} " f"is missing. Restore it before resuming, or start a fresh run." ) + try: + check_mountable_dir(mount_path, allow_home=True) + except ValueError as exc: + parser.error(f"--resume {args.resume}: {exc}") attach_workspace_mount(args) if state.get("diff_scope"): args.diff_scope = state.get("diff_scope") diff --git a/strix/interface/scan_setup.py b/strix/interface/scan_setup.py index 1e795a5c..647bd8f4 100644 --- a/strix/interface/scan_setup.py +++ b/strix/interface/scan_setup.py @@ -12,12 +12,14 @@ from __future__ import annotations import asyncio import logging from datetime import UTC, datetime +from pathlib import Path from typing import TYPE_CHECKING, Any from strix.config import Settings, codex, load_settings from strix.core.paths import run_dir_for from strix.interface.utils import ( assign_workspace_subdirs, + check_mountable_dir, clone_repository, collect_local_sources, dedupe_local_targets, @@ -206,10 +208,16 @@ def attach_workspace_mount(args: argparse.Namespace) -> None: it stays out of ``targets_info``, so it carries no authorized scope, and it is attached after diff-scope resolution so it contributes no diff context. The instruction is the only source of truth for what to do with it. + + Still runs the mount admission check (with ``allow_home=True``): system + trees and credential directories must not reach the sandbox even when the + operator confirmed a working directory. Exact ``$HOME`` remains allowed + for the target-less TUI flow. """ mount = getattr(args, "workspace_mount", None) if not mount: return + check_mountable_dir(Path(mount).expanduser(), allow_home=True) args.workspace_subdir = derive_local_base_name(mount) local_sources = list(getattr(args, "local_sources", None) or []) local_sources.append( diff --git a/strix/interface/utils.py b/strix/interface/utils.py index 8dc950d2..ca29b209 100644 --- a/strix/interface/utils.py +++ b/strix/interface/utils.py @@ -1382,7 +1382,16 @@ def _is_within(path: Path, ancestor: Path) -> bool: return path_parts[: len(ancestor_parts)] == ancestor_parts -def check_mountable_dir(path: Path) -> None: +def check_mountable_dir(path: Path, *, allow_home: bool = False) -> None: + """Refuse system trees and credential directories for sandbox bind-mounts. + + Scan targets (`--target`) pass ``allow_home=False`` (default), so the + operator's exact ``$HOME`` is also refused. Workspace mounts used by the + target-less TUI / ``--resume`` flow pass ``allow_home=True``: the operator + already confirmed that working directory, and it may legitimately be + ``$HOME``, but system trees and credential directories (``.ssh``, ``.aws``, …) + must still be refused. + """ resolved = path.resolve() if not resolved.is_dir(): raise ValueError(f"'{path}' is not an existing directory.") @@ -1391,7 +1400,8 @@ def check_mountable_dir(path: Path) -> None: # /private/etc symlink, and only the resolved path is compared below. exact = {str(Path(root)).casefold() for root in _FORBIDDEN_MOUNT_ROOTS} exact |= {str(Path(root).resolve()).casefold() for root in _FORBIDDEN_MOUNT_ROOTS} - exact.add(str(Path.home().resolve()).casefold()) + if not allow_home: + exact.add(str(Path.home().resolve()).casefold()) tree_roots = set(_FORBIDDEN_MOUNT_TREES) if os.name == "nt": drive = Path(resolved.anchor) diff --git a/tests/test_local_sources.py b/tests/test_local_sources.py index 9984bef6..8428c88b 100644 --- a/tests/test_local_sources.py +++ b/tests/test_local_sources.py @@ -86,8 +86,8 @@ def test_workspace_mount_is_mounted_without_becoming_a_target( ) -> None: """A workspace mount reaches the sandbox but carries no target semantics. - It is the directory the agent works in, so it is exempt from the guard that - refuses home directories for scan targets, and it never enters targets_info. + Exact ``$HOME`` remains allowed for workspace mounts (unlike scan targets), + but the mount never enters targets_info. """ home = tmp_path / "home" home.mkdir() @@ -110,6 +110,23 @@ def test_workspace_mount_is_mounted_without_becoming_a_target( ) +def test_workspace_mount_still_rejects_credential_dirs(tmp_path: Path) -> None: + ssh_dir = tmp_path / ".ssh" + ssh_dir.mkdir() + args = argparse.Namespace(targets_info=[], local_sources=[], workspace_mount=str(ssh_dir)) + with pytest.raises(ValueError, match="Refusing to mount"): + attach_workspace_mount(args) + + +def test_workspace_mount_still_rejects_system_trees() -> None: + etc = Path("/etc") + if not etc.is_dir(): + pytest.skip("no /etc on this platform") + args = argparse.Namespace(targets_info=[], local_sources=[], workspace_mount=str(etc)) + with pytest.raises(ValueError, match="Refusing to mount"): + attach_workspace_mount(args) + + def test_workspace_mount_absent_leaves_local_sources_alone() -> None: args = argparse.Namespace(targets_info=[], local_sources=[], workspace_mount=None)