mirror of
https://github.com/usestrix/strix.git
synced 2026-08-28 05:25:00 +00:00
fix(interface): apply mount admission to workspace mounts
check_mountable_dir already refuses system trees and credential directories for --target paths, but attach_workspace_mount (TUI working-directory mount and --resume rehydration) skipped it. Keep the documented exact-$HOME exemption for workspace mounts (allow_home=True) while still refusing /etc, .ssh, .aws, and other credential/system paths. Re-check on --resume so a stored workspace_mount cannot bypass admission. Fixes #1054
This commit is contained in:
parent
7cc9fa9faa
commit
253c1eea3a
4 changed files with 48 additions and 8 deletions
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue