From dadb35af49c7d5efb9d423a23e692c5f120a3ecd Mon Sep 17 00:00:00 2001 From: chengyongru Date: Wed, 10 Jun 2026 15:50:24 +0800 Subject: [PATCH] feat(exec): add path prepend config --- docs/configuration.md | 1 + nanobot/agent/tools/shell.py | 33 ++++++++-- nanobot/webui/settings_api.py | 1 + tests/tools/test_exec_env.py | 22 +++++++ tests/tools/test_exec_platform.py | 85 ++++++++++++++++++++++++++ tests/tools/test_tool_loader.py | 8 ++- tests/webui/test_settings_api.py | 18 ++++++ webui/src/lib/types.ts | 1 + webui/src/tests/app-layout.test.tsx | 3 + webui/src/tests/settings-view.test.tsx | 1 + webui/src/tests/thread-shell.test.tsx | 1 + 11 files changed, 169 insertions(+), 5 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 5cfdcda4d..dd11eb3aa 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1727,6 +1727,7 @@ For API keys, tokens, and other secrets, see [Environment Variables for Secrets] | `tools.exec.sandbox` | `""` | Sandbox backend for shell commands. Set to `"bwrap"` to wrap exec calls in a [bubblewrap](https://github.com/containers/bubblewrap) sandbox — the process can only see the workspace (read-write) and media directory (read-only); config files and API keys are hidden. Automatically enables `restrictToWorkspace` for file tools. **Linux only** — requires `bwrap` installed (`apt install bubblewrap`; pre-installed in the Docker image). Not available on macOS or Windows (bwrap depends on Linux kernel namespaces). | | `tools.exec.enable` | `true` | When `false`, the shell `exec` tool is not registered at all. Use this to completely disable shell command execution. | | `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.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/shell.py b/nanobot/agent/tools/shell.py index 0ecfadc00..b4960e8e0 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -55,6 +55,7 @@ class ExecToolConfig(Base): """Shell exec tool configuration.""" enable: bool = True timeout: int = Field(default=60, ge=0) # Hard timeout (s); 0 = no limit. Not capped by the per-call max. + path_prepend: str = "" path_append: str = "" sandbox: str = "" allowed_env_keys: list[str] = Field(default_factory=list) @@ -150,6 +151,7 @@ class ExecTool(Tool): restrict_to_workspace=ctx.config.restrict_to_workspace, webui_allow_local_service_access=ctx.config.webui_allow_local_service_access, sandbox=cfg.sandbox, + path_prepend=cfg.path_prepend, path_append=cfg.path_append, allowed_env_keys=cfg.allowed_env_keys, allow_patterns=cfg.allow_patterns, @@ -166,6 +168,7 @@ class ExecTool(Tool): webui_allow_local_service_access: bool = True, allow_local_preview_access: bool | None = None, sandbox: str = "", + path_prepend: str = "", path_append: str = "", allowed_env_keys: list[str] | None = None, session_manager: Any | None = None, @@ -197,6 +200,7 @@ class ExecTool(Tool): if allow_local_preview_access is not None: webui_allow_local_service_access = allow_local_preview_access self.webui_allow_local_service_access = webui_allow_local_service_access + self.path_prepend = path_prepend self.path_append = path_append self.allowed_env_keys = allowed_env_keys or [] self._session_manager = session_manager or DEFAULT_EXEC_SESSION_MANAGER @@ -411,12 +415,11 @@ class ExecTool(Tool): effective_timeout = self._resolve_timeout(timeout) env = self._build_env() - if self.path_append: + if self.path_prepend or self.path_append: if _IS_WINDOWS: - env["PATH"] = env.get("PATH", "") + os.pathsep + self.path_append + env["PATH"] = self._compose_path(env.get("PATH", "")) else: - env["NANOBOT_PATH_APPEND"] = self.path_append - command = f'export PATH="$PATH{os.pathsep}$NANOBOT_PATH_APPEND"; {command}' + command = self._wrap_path_export(command, env) shell_program, shell_error = self._resolve_shell(shell) if shell_error: @@ -431,6 +434,28 @@ class ExecTool(Tool): login=True if login is None else login, ) + def _compose_path(self, current_path: str) -> str: + parts = [] + if self.path_prepend: + parts.append(self.path_prepend) + if current_path: + parts.append(current_path) + if self.path_append: + parts.append(self.path_append) + return os.pathsep.join(parts) + + def _wrap_path_export(self, command: str, env: dict[str, str]) -> str: + segments = [] + if self.path_prepend: + env["NANOBOT_PATH_PREPEND"] = self.path_prepend + segments.append("$NANOBOT_PATH_PREPEND") + segments.append("$PATH") + if self.path_append: + env["NANOBOT_PATH_APPEND"] = self.path_append + segments.append("$NANOBOT_PATH_APPEND") + path_expr = os.pathsep.join(segments) + return f'export PATH="{path_expr}"; {command}' + @staticmethod async def _spawn( command: str, cwd: str, env: dict[str, str], diff --git a/nanobot/webui/settings_api.py b/nanobot/webui/settings_api.py index cbd5e4e13..1f663a121 100644 --- a/nanobot/webui/settings_api.py +++ b/nanobot/webui/settings_api.py @@ -801,6 +801,7 @@ def settings_payload( "mcp_server_count": len(config.tools.mcp_servers), "exec_enabled": exec_config.enable, "exec_sandbox": exec_config.sandbox or None, + "exec_path_prepend_set": bool(exec_config.path_prepend), "exec_path_append_set": bool(exec_config.path_append), }, "requires_restart": requires_restart, diff --git a/tests/tools/test_exec_env.py b/tests/tools/test_exec_env.py index b9567f29d..1d749a078 100644 --- a/tests/tools/test_exec_env.py +++ b/tests/tools/test_exec_env.py @@ -45,6 +45,28 @@ async def test_exec_path_append_preserves_system_path(): assert "Exit code: 0" in result +@_UNIX_ONLY +@pytest.mark.asyncio +async def test_exec_path_prepend_takes_lookup_precedence(tmp_path): + """pathPrepend should win over pathAppend for executable lookup.""" + preferred = tmp_path / "preferred" + fallback = tmp_path / "fallback" + preferred.mkdir() + fallback.mkdir() + preferred_tool = preferred / "pathprobe" + fallback_tool = fallback / "pathprobe" + preferred_tool.write_text("#!/bin/sh\necho preferred\n", encoding="utf-8") + fallback_tool.write_text("#!/bin/sh\necho fallback\n", encoding="utf-8") + preferred_tool.chmod(0o755) + fallback_tool.chmod(0o755) + + tool = ExecTool(path_prepend=str(preferred), path_append=str(fallback)) + result = await tool.execute(command="pathprobe") + + assert "preferred" in result + assert "fallback" not in result + + @_UNIX_ONLY @pytest.mark.asyncio async def test_exec_allowed_env_keys_passthrough(monkeypatch): diff --git a/tests/tools/test_exec_platform.py b/tests/tools/test_exec_platform.py index e09838492..a72b06e36 100644 --- a/tests/tools/test_exec_platform.py +++ b/tests/tools/test_exec_platform.py @@ -202,6 +202,65 @@ class TestPathAppendPlatform: assert captured_env["NANOBOT_PATH_APPEND"] == "/opt/bin; echo INJECTED" assert "INJECTED" not in captured_cmd + @pytest.mark.asyncio + async def test_unix_path_prepend_uses_env_var_in_fixed_export(self): + """On Unix, path_prepend must not be interpolated into shell source.""" + mock_proc = AsyncMock() + mock_proc.communicate.return_value = (b"ok", b"") + mock_proc.returncode = 0 + + captured_cmd = None + captured_env = {} + + async def capture_spawn(cmd, cwd, env, shell_program=None, login=True, *, stdin=None): + nonlocal captured_cmd + captured_cmd = cmd + captured_env.update(env) + return mock_proc + + with ( + patch("nanobot.agent.tools.shell._IS_WINDOWS", False), + patch("nanobot.agent.tools.shell.os.pathsep", ":"), + patch.object(ExecTool, "_spawn", side_effect=capture_spawn), + patch.object(ExecTool, "_guard_command", return_value=None), + ): + tool = ExecTool(path_prepend="/venv/bin; echo INJECTED") + await tool.execute(command="python --version") + + assert captured_cmd == 'export PATH="$NANOBOT_PATH_PREPEND:$PATH"; python --version' + assert captured_env["NANOBOT_PATH_PREPEND"] == "/venv/bin; echo INJECTED" + assert "INJECTED" not in captured_cmd + + @pytest.mark.asyncio + async def test_unix_path_prepend_and_append_order(self): + mock_proc = AsyncMock() + mock_proc.communicate.return_value = (b"ok", b"") + mock_proc.returncode = 0 + + captured_cmd = None + captured_env = {} + + async def capture_spawn(cmd, cwd, env, shell_program=None, login=True, *, stdin=None): + nonlocal captured_cmd + captured_cmd = cmd + captured_env.update(env) + return mock_proc + + with ( + patch("nanobot.agent.tools.shell._IS_WINDOWS", False), + patch("nanobot.agent.tools.shell.os.pathsep", ":"), + patch.object(ExecTool, "_spawn", side_effect=capture_spawn), + patch.object(ExecTool, "_guard_command", return_value=None), + ): + tool = ExecTool(path_prepend="/venv/bin", path_append="/usr/sbin") + await tool.execute(command="python --version") + + assert captured_cmd == ( + 'export PATH="$NANOBOT_PATH_PREPEND:$PATH:$NANOBOT_PATH_APPEND"; python --version' + ) + assert captured_env["NANOBOT_PATH_PREPEND"] == "/venv/bin" + assert captured_env["NANOBOT_PATH_APPEND"] == "/usr/sbin" + @pytest.mark.asyncio async def test_windows_modifies_env(self): """On Windows, path_append is appended to PATH in the env dict.""" @@ -226,6 +285,32 @@ class TestPathAppendPlatform: assert captured_env["PATH"].endswith(r";C:\tools\bin") + @pytest.mark.asyncio + async def test_windows_path_prepend_and_append_order(self): + mock_proc = AsyncMock() + mock_proc.communicate.return_value = (b"ok", b"") + mock_proc.returncode = 0 + + captured_env = {} + + async def capture_spawn(cmd, cwd, env, shell_program=None, login=True, *, stdin=None): + captured_env.update(env) + return mock_proc + + with ( + patch("nanobot.agent.tools.shell._IS_WINDOWS", True), + patch("nanobot.agent.tools.shell.os.pathsep", ";"), + patch.object(ExecTool, "_build_env", return_value={"PATH": r"C:\Windows\System32"}), + patch.object(ExecTool, "_spawn", side_effect=capture_spawn), + patch.object(ExecTool, "_guard_command", return_value=None), + ): + tool = ExecTool(path_prepend=r"C:\venv\Scripts", path_append=r"C:\tools\bin") + await tool.execute(command="python --version") + + assert captured_env["PATH"] == ( + r"C:\venv\Scripts;C:\Windows\System32;C:\tools\bin" + ) + # --------------------------------------------------------------------------- # sandbox diff --git a/tests/tools/test_tool_loader.py b/tests/tools/test_tool_loader.py index 4d6f128f1..7c6cd8727 100644 --- a/tests/tools/test_tool_loader.py +++ b/tests/tools/test_tool_loader.py @@ -244,6 +244,7 @@ def test_exec_tool_create(): mock_config.exec.enable = True mock_config.exec.timeout = 120 mock_config.exec.sandbox = "" + mock_config.exec.path_prepend = "/venv/bin" mock_config.exec.path_append = "" mock_config.exec.allowed_env_keys = [] mock_config.exec.allow_patterns = [] @@ -252,6 +253,7 @@ def test_exec_tool_create(): ctx = ToolContext(config=mock_config, workspace="/tmp") tool = ExecTool.create(ctx) assert isinstance(tool, ExecTool) + assert tool.path_prepend == "/venv/bin" def test_web_tools_config_cls(): @@ -360,7 +362,7 @@ def test_config_round_trip(): config_dict = { "tools": { "web": {"enable": True, "search": {"provider": "brave", "api_key": "test"}}, - "exec": {"enable": False, "timeout": 120}, + "exec": {"enable": False, "timeout": 120, "pathPrepend": "/venv/bin"}, "my": {"allowSet": True}, "imageGeneration": {"enabled": True, "provider": "openrouter"}, } @@ -370,8 +372,10 @@ def test_config_round_trip(): assert dumped["tools"]["my"]["allowSet"] is True assert dumped["tools"]["imageGeneration"]["enabled"] is True + assert dumped["tools"]["exec"]["pathPrepend"] == "/venv/bin" assert config.tools.exec.enable is False assert config.tools.exec.timeout == 120 + assert config.tools.exec.path_prepend == "/venv/bin" assert config.tools.web.search.provider == "brave" @@ -382,6 +386,7 @@ def test_config_defaults(): config = Config.model_validate({}) assert config.tools.exec.enable is True assert config.tools.exec.timeout == 60 + assert config.tools.exec.path_prepend == "" assert config.tools.web.enable is True assert config.tools.web.search.provider == "duckduckgo" assert config.tools.my.enable is True @@ -403,6 +408,7 @@ def test_loader_registers_same_tools_as_old_hardcoded(): mock_config.exec.enable = True mock_config.exec.timeout = 60 mock_config.exec.sandbox = "" + mock_config.exec.path_prepend = "" mock_config.exec.path_append = "" mock_config.exec.allowed_env_keys = [] mock_config.exec.allow_patterns = [] diff --git a/tests/webui/test_settings_api.py b/tests/webui/test_settings_api.py index 76518c576..8c3c5889f 100644 --- a/tests/webui/test_settings_api.py +++ b/tests/webui/test_settings_api.py @@ -244,6 +244,24 @@ def test_settings_payload_includes_network_safety_fields( assert payload["advanced"]["ssrf_whitelist_count"] == 1 +def test_settings_payload_includes_exec_path_flags( + tmp_path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + config_path = tmp_path / "config.json" + config = Config() + config.tools.exec.path_prepend = "/venv/bin" + config.tools.exec.path_append = "/usr/sbin" + save_config(config, config_path) + monkeypatch.setattr("nanobot.config.loader._current_config_path", config_path) + monkeypatch.setattr("nanobot.webui.workspaces.get_webui_dir", lambda: tmp_path / "webui") + + payload = settings_payload() + + assert payload["advanced"]["exec_path_prepend_set"] is True + assert payload["advanced"]["exec_path_append_set"] is True + + def test_settings_payload_includes_effective_transcription_config( tmp_path, monkeypatch: pytest.MonkeyPatch, diff --git a/webui/src/lib/types.ts b/webui/src/lib/types.ts index 438373a1f..c9dc4164d 100644 --- a/webui/src/lib/types.ts +++ b/webui/src/lib/types.ts @@ -480,6 +480,7 @@ export interface SettingsPayload { mcp_server_count: number; exec_enabled: boolean; exec_sandbox?: string | null; + exec_path_prepend_set: boolean; exec_path_append_set: boolean; }; requires_restart: boolean; diff --git a/webui/src/tests/app-layout.test.tsx b/webui/src/tests/app-layout.test.tsx index 845efa8ab..3fa3e8124 100644 --- a/webui/src/tests/app-layout.test.tsx +++ b/webui/src/tests/app-layout.test.tsx @@ -125,6 +125,7 @@ function baseSettingsPayload() { mcp_server_count: 0, exec_enabled: true, exec_sandbox: null, + exec_path_prepend_set: false, exec_path_append_set: false, }, requires_restart: false, @@ -1023,6 +1024,7 @@ describe("App layout", () => { mcp_server_count: 0, exec_enabled: true, exec_sandbox: null, + exec_path_prepend_set: false, exec_path_append_set: false, }, requires_restart: false, @@ -1349,6 +1351,7 @@ describe("App layout", () => { mcp_server_count: 0, exec_enabled: true, exec_sandbox: null, + exec_path_prepend_set: false, exec_path_append_set: false, }, requires_restart: false, diff --git a/webui/src/tests/settings-view.test.tsx b/webui/src/tests/settings-view.test.tsx index 4987fb96c..15d0dbc54 100644 --- a/webui/src/tests/settings-view.test.tsx +++ b/webui/src/tests/settings-view.test.tsx @@ -93,6 +93,7 @@ function settingsPayload(): SettingsPayload { mcp_server_count: 0, exec_enabled: true, exec_sandbox: null, + exec_path_prepend_set: false, exec_path_append_set: false, }, requires_restart: false, diff --git a/webui/src/tests/thread-shell.test.tsx b/webui/src/tests/thread-shell.test.tsx index f80640056..c1efd1df3 100644 --- a/webui/src/tests/thread-shell.test.tsx +++ b/webui/src/tests/thread-shell.test.tsx @@ -212,6 +212,7 @@ function modelSettings(model: string, provider: string): SettingsPayload { mcp_server_count: 0, exec_enabled: true, exec_sandbox: null, + exec_path_prepend_set: false, exec_path_append_set: false, }, requires_restart: false,