From 01a11b39803cd106e40a747d5be03ca2987defab Mon Sep 17 00:00:00 2001 From: yu-xin-c <2182712990@qq.com> Date: Thu, 18 Jun 2026 23:19:00 +0800 Subject: [PATCH] feat(exec): allow extra bwrap bind roots --- docs/configuration.md | 2 + nanobot/agent/tools/sandbox.py | 54 ++++++++++++++++++++-- nanobot/agent/tools/shell.py | 45 +++++++++++++++++- tests/tools/test_exec_platform.py | 30 ++++++++++++ tests/tools/test_exec_security.py | 71 +++++++++++++++++++++++++++++ tests/tools/test_sandbox.py | 51 +++++++++++++++++++++ tests/tools/test_tool_validation.py | 16 +++++++ 7 files changed, 263 insertions(+), 6 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index ed80cf62..3d4be10d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1995,6 +1995,8 @@ For API keys, tokens, and other secrets, see [Environment Variables for Secrets] | `tools.exec.timeout` | `60` | Default hard timeout in seconds for shell commands. Config values may exceed the per-call tool cap; set `0` to disable the hard timeout for trusted long-running commands. | | `tools.exec.pathPrepend` | `""` | Extra directories to prepend to `PATH` when running shell commands. Use this when configured tools should win executable lookup precedence, such as a Python virtual environment's `bin` or `Scripts` directory. | | `tools.exec.pathAppend` | `""` | Extra directories to append to `PATH` when running shell commands (e.g. `/usr/sbin` for `ufw`). | +| `tools.exec.sandboxRoBinds` | `[]` | Extra absolute paths to read-only bind into the `"bwrap"` sandbox with `--ro-bind-try`, such as `/home/user/.local/bin` or `/home/user/.cargo/bin` when those paths are also in `pathPrepend`/`pathAppend`. These roots are also accepted by the shell absolute-path guard only while bwrap is active. | +| `tools.exec.sandboxRwBinds` | `[]` | Extra absolute paths to read-write bind into the `"bwrap"` sandbox with `--bind-try`, for trusted tool caches or scratch directories. Use sparingly: paths listed here are intentionally writable by shell commands inside the sandbox. | | `tools.webuiAllowRemotePackageInstall` | `false` | When `false`, the WebUI can install missing optional packages only from a browser opened on the same machine as nanobot. Set to `true` only when a trusted remote admin is allowed to install Python packages into this nanobot environment. | | `tools.ssrfWhitelist` | `[]` | CIDR ranges exempted from the shared SSRF guard used by web fetches and HTTP/SSE MCP connections. Prefer exact host CIDRs such as `192.168.1.50/32`; broad ranges increase SSRF exposure. | | `channels.*.allowFrom` | omitted | Access control per channel. Omit to use pairing-only mode; set `["*"]` to allow everyone; or list specific user IDs. See [Pairing](#pairing) for details. | diff --git a/nanobot/agent/tools/sandbox.py b/nanobot/agent/tools/sandbox.py index d1f771b2..0d602abf 100644 --- a/nanobot/agent/tools/sandbox.py +++ b/nanobot/agent/tools/sandbox.py @@ -5,13 +5,40 @@ To add a new backend, implement a function with the signature: and register it in _BACKENDS below. """ +import os import shlex from pathlib import Path +from typing import Iterable from nanobot.config.paths import get_media_dir -def _bwrap(command: str, workspace: str, cwd: str) -> str: +def _normalize_bind_paths(paths: Iterable[str] | None) -> list[str]: + out: list[str] = [] + seen: set[str] = set() + for raw in paths or []: + value = str(raw).strip() + if not value: + continue + path = Path(os.path.expandvars(value)).expanduser() + if not path.is_absolute(): + continue + resolved = str(path.resolve(strict=False)) + if resolved in seen: + continue + seen.add(resolved) + out.append(resolved) + return out + + +def _bwrap( + command: str, + workspace: str, + cwd: str, + *, + sandbox_ro_binds: Iterable[str] | None = None, + sandbox_rw_binds: Iterable[str] | None = None, +) -> str: """Wrap command in a bubblewrap sandbox (requires bwrap in container). Only the workspace is bind-mounted read-write; its parent dir (which holds @@ -51,17 +78,34 @@ def _bwrap(command: str, workspace: str, cwd: str) -> str: "--dir", str(ws), # recreate workspace mount point "--bind", str(ws), str(ws), "--ro-bind-try", str(media), str(media), # read-only access to media - "--chdir", sandbox_cwd, - "--", "sh", "-c", command, ] + for p in _normalize_bind_paths(sandbox_ro_binds): + args += ["--ro-bind-try", p, p] + for p in _normalize_bind_paths(sandbox_rw_binds): + args += ["--bind-try", p, p] + args += ["--chdir", sandbox_cwd, "--", "sh", "-c", command] return shlex.join(args) _BACKENDS = {"bwrap": _bwrap} -def wrap_command(sandbox: str, command: str, workspace: str, cwd: str) -> str: +def wrap_command( + sandbox: str, + command: str, + workspace: str, + cwd: str, + *, + sandbox_ro_binds: Iterable[str] | None = None, + sandbox_rw_binds: Iterable[str] | None = None, +) -> str: """Wrap *command* using the named sandbox backend.""" if backend := _BACKENDS.get(sandbox): - return backend(command, workspace, cwd) + return backend( + command, + workspace, + cwd, + sandbox_ro_binds=sandbox_ro_binds, + sandbox_rw_binds=sandbox_rw_binds, + ) raise ValueError(f"Unknown sandbox backend {sandbox!r}. Available: {list(_BACKENDS)}") diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index 9fa639a7..be3e5605 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -84,6 +84,8 @@ class ExecToolConfig(Base): path_prepend: str = "" path_append: str = "" sandbox: str = "" + sandbox_ro_binds: list[str] = Field(default_factory=list) + sandbox_rw_binds: list[str] = Field(default_factory=list) allowed_env_keys: list[str] = Field(default_factory=list) allow_patterns: list[str] = Field(default_factory=list) deny_patterns: list[str] = Field(default_factory=list) @@ -187,6 +189,8 @@ class ExecTool(Tool): sandbox=cfg.sandbox, path_prepend=cfg.path_prepend, path_append=cfg.path_append, + sandbox_ro_binds=cfg.sandbox_ro_binds, + sandbox_rw_binds=cfg.sandbox_rw_binds, allowed_env_keys=cfg.allowed_env_keys, allow_patterns=cfg.allow_patterns, deny_patterns=cfg.deny_patterns, @@ -205,6 +209,8 @@ class ExecTool(Tool): sandbox: str = "", path_prepend: str = "", path_append: str = "", + sandbox_ro_binds: list[str] | None = None, + sandbox_rw_binds: list[str] | None = None, allowed_env_keys: list[str] | None = None, session_manager: Any | None = None, ): @@ -237,6 +243,8 @@ class ExecTool(Tool): self.webui_allow_local_service_access = webui_allow_local_service_access self.path_prepend = path_prepend self.path_append = path_append + self.sandbox_ro_binds = self._normalize_bind_roots(sandbox_ro_binds) + self.sandbox_rw_binds = self._normalize_bind_roots(sandbox_rw_binds) self.allowed_env_keys = allowed_env_keys or [] self._session_manager = session_manager or DEFAULT_EXEC_SESSION_MANAGER @@ -464,7 +472,14 @@ class ExecTool(Tool): ) else: workspace = workspace_root or cwd - command = wrap_command(self.sandbox, command, workspace, cwd) + command = wrap_command( + self.sandbox, + command, + workspace, + cwd, + sandbox_ro_binds=[str(p) for p in self.sandbox_ro_binds], + sandbox_rw_binds=[str(p) for p in self.sandbox_rw_binds], + ) cwd = str(Path(workspace).resolve()) effective_timeout = self._resolve_timeout(timeout) @@ -794,6 +809,7 @@ class ExecTool(Tool): if workspace_root else None ) + sandbox_bind_roots = self._active_sandbox_bind_roots() for raw in self._extract_absolute_paths(cmd): try: @@ -817,6 +833,8 @@ class ExecTool(Tool): ) if not allowed and resolved_workspace is not None: allowed = is_path_within(p, resolved_workspace) + if not allowed and sandbox_bind_roots: + allowed = any(is_path_within(p, root) for root in sandbox_bind_roots) if p.is_absolute() and not allowed: return ToolResult.error( "Error: Command blocked by safety guard (path outside working dir)" @@ -921,3 +939,28 @@ class ExecTool(Tool): posix_paths = re.findall(r"(?:^|[\s|>='\"])(/[^\s\"'>;|<]+)", command) # POSIX: /absolute only home_paths = re.findall(r"(?:^|[\s>='\"])(~[/+][^\s\"'>;|<]*)", command) # POSIX/Windows home shortcut: ~/ or ~+ return win_paths + posix_paths + home_paths + + @staticmethod + def _normalize_bind_roots(paths: list[str] | None) -> list[Path]: + roots: list[Path] = [] + seen: set[str] = set() + for raw in paths or []: + value = str(raw).strip() + if not value: + continue + path = Path(os.path.expandvars(value)).expanduser() + if not path.is_absolute(): + continue + with suppress(OSError, RuntimeError, ValueError): + resolved = path.resolve(strict=False) + key = os.path.normcase(os.fspath(resolved)) + if key in seen: + continue + seen.add(key) + roots.append(resolved) + return roots + + def _active_sandbox_bind_roots(self) -> list[Path]: + if self.sandbox != "bwrap" or _IS_WINDOWS: + return [] + return [*self.sandbox_ro_binds, *self.sandbox_rw_binds] diff --git a/tests/tools/test_exec_platform.py b/tests/tools/test_exec_platform.py index 514ccd95..4a6277b2 100644 --- a/tests/tools/test_exec_platform.py +++ b/tests/tools/test_exec_platform.py @@ -8,6 +8,7 @@ platform-specific binaries (all subprocess calls are mocked). import asyncio import shutil import sys +from pathlib import Path from unittest.mock import AsyncMock, patch import pytest @@ -473,6 +474,35 @@ class TestSandboxPlatform: spawned_cmd = mock_spawn.call_args[0][0] assert "bwrap" in spawned_cmd + @pytest.mark.asyncio + async def test_bwrap_receives_configured_bind_roots(self): + """Configured bwrap bind roots should be forwarded to the sandbox wrapper.""" + mock_proc = AsyncMock() + mock_proc.communicate.return_value = (b"sandboxed", b"") + mock_proc.returncode = 0 + + with ( + patch("nanobot.agent.tools.shell._IS_WINDOWS", False), + patch("nanobot.agent.tools.shell.wrap_command", return_value="bwrap -- sh -c ls") as mock_wrap, + patch.object(ExecTool, "_spawn", return_value=mock_proc), + patch.object(ExecTool, "_guard_command", return_value=None), + ): + tool = ExecTool( + sandbox="bwrap", + working_dir="/workspace", + sandbox_ro_binds=["/home/user/.local/bin"], + sandbox_rw_binds=["/home/user/.cache/uv"], + ) + await tool.execute(command="ls") + + kwargs = mock_wrap.call_args.kwargs + assert kwargs["sandbox_ro_binds"] == [ + str(Path("/home/user/.local/bin").resolve(strict=False)) + ] + assert kwargs["sandbox_rw_binds"] == [ + str(Path("/home/user/.cache/uv").resolve(strict=False)) + ] + # --------------------------------------------------------------------------- # end-to-end (mocked subprocess, full execute path) diff --git a/tests/tools/test_exec_security.py b/tests/tools/test_exec_security.py index 73a58ce2..6e3a6f8a 100644 --- a/tests/tools/test_exec_security.py +++ b/tests/tools/test_exec_security.py @@ -314,6 +314,77 @@ def test_exec_still_blocks_real_outside_path_via_redirect(tmp_path): assert "path outside working dir" in blocked +def test_exec_allows_absolute_path_inside_bwrap_ro_bind(tmp_path): + workspace = tmp_path / "workspace" + workspace.mkdir() + tool_bin = tmp_path / "home" / ".local" / "bin" + tool_bin.mkdir(parents=True) + uv = tool_bin / "uv" + uv.write_text("#!/bin/sh\n") + tool = ExecTool( + working_dir=str(workspace), + restrict_to_workspace=True, + sandbox="bwrap", + sandbox_ro_binds=[str(tool_bin)], + ) + + blocked = tool._guard_command( + f"{uv} --version", + str(workspace), + restrict_to_workspace=True, + workspace_root=str(workspace), + ) + + assert blocked is None + + +def test_exec_allows_absolute_path_inside_bwrap_rw_bind(tmp_path): + workspace = tmp_path / "workspace" + workspace.mkdir() + cache_dir = tmp_path / "cache" + cache_dir.mkdir() + tool = ExecTool( + working_dir=str(workspace), + restrict_to_workspace=True, + sandbox="bwrap", + sandbox_rw_binds=[str(cache_dir)], + ) + + blocked = tool._guard_command( + f"touch {cache_dir / 'stamp'}", + str(workspace), + restrict_to_workspace=True, + workspace_root=str(workspace), + ) + + assert blocked is None + + +def test_exec_bind_roots_do_not_widen_guard_without_bwrap(tmp_path): + workspace = tmp_path / "workspace" + workspace.mkdir() + tool_bin = tmp_path / "home" / ".local" / "bin" + tool_bin.mkdir(parents=True) + uv = tool_bin / "uv" + uv.write_text("#!/bin/sh\n") + tool = ExecTool( + working_dir=str(workspace), + restrict_to_workspace=True, + sandbox="", + sandbox_ro_binds=[str(tool_bin)], + ) + + blocked = tool._guard_command( + f"{uv} --version", + str(workspace), + restrict_to_workspace=True, + workspace_root=str(workspace), + ) + + assert blocked is not None + assert "path outside working dir" in blocked + + # --- format command blocking ----------------------------------------------- diff --git a/tests/tools/test_sandbox.py b/tests/tools/test_sandbox.py index 57495a31..75271351 100644 --- a/tests/tools/test_sandbox.py +++ b/tests/tools/test_sandbox.py @@ -150,6 +150,57 @@ class TestBwrapBackend: try_pairs = {(tokens[i + 1], tokens[i + 2]) for i in try_indices} assert (str(fake_media), str(fake_media)) in try_pairs + def test_custom_read_only_binds_use_ro_bind_try(self, tmp_path): + ws = tmp_path / "project" + tool_bin = tmp_path / "home" / ".local" / "bin" + + result = wrap_command( + "bwrap", + "uv --version", + str(ws), + str(ws), + sandbox_ro_binds=[str(tool_bin)], + ) + tokens = _parse(result) + + try_indices = [i for i, t in enumerate(tokens) if t == "--ro-bind-try"] + try_pairs = {(tokens[i + 1], tokens[i + 2]) for i in try_indices} + assert (str(tool_bin.resolve(strict=False)), str(tool_bin.resolve(strict=False))) in try_pairs + + def test_custom_read_write_binds_use_bind_try(self, tmp_path): + ws = tmp_path / "project" + cache_dir = tmp_path / "cache" + + result = wrap_command( + "bwrap", + "touch cache/file", + str(ws), + str(ws), + sandbox_rw_binds=[str(cache_dir)], + ) + tokens = _parse(result) + + bind_try_indices = [i for i, t in enumerate(tokens) if t == "--bind-try"] + bind_try_pairs = {(tokens[i + 1], tokens[i + 2]) for i in bind_try_indices} + resolved = str(cache_dir.resolve(strict=False)) + assert (resolved, resolved) in bind_try_pairs + + def test_custom_relative_bind_paths_are_ignored(self, tmp_path): + ws = tmp_path / "project" + + result = wrap_command( + "bwrap", + "ls", + str(ws), + str(ws), + sandbox_ro_binds=["relative/bin"], + sandbox_rw_binds=["relative/cache"], + ) + tokens = _parse(result) + + assert "relative/bin" not in tokens + assert "relative/cache" not in tokens + class TestUnknownBackend: def test_raises_value_error(self, tmp_path): diff --git a/tests/tools/test_tool_validation.py b/tests/tools/test_tool_validation.py index e05c7de4..8ffd83e4 100644 --- a/tests/tools/test_tool_validation.py +++ b/tests/tools/test_tool_validation.py @@ -714,6 +714,22 @@ def test_exec_config_timeout_uncapped_and_zero() -> None: ExecToolConfig(timeout=-1) +def test_exec_config_accepts_bwrap_bind_aliases() -> None: + cfg = ExecToolConfig.model_validate( + { + "sandboxRoBinds": ["/home/user/.local/bin"], + "sandboxRwBinds": ["/home/user/.cache/uv"], + } + ) + + dumped = cfg.model_dump(by_alias=True) + + assert cfg.sandbox_ro_binds == ["/home/user/.local/bin"] + assert cfg.sandbox_rw_binds == ["/home/user/.cache/uv"] + assert dumped["sandboxRoBinds"] == ["/home/user/.local/bin"] + assert dumped["sandboxRwBinds"] == ["/home/user/.cache/uv"] + + def test_resolve_timeout_config_uncapped_and_unlimited() -> None: """Config timeout drives the hard timeout uncapped; 0 means no limit (#3595).""" assert ExecTool(timeout=3600)._resolve_timeout(None) == 3600