mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-06-13 22:34:06 +00:00
fix(exec): bypass cmd.exe for multi-line python -c commands on Windows
On Windows, cmd.exe /c treats newlines as command separators, silently dropping code after the first line in `python -c "..."` commands. This causes multi-line inline Python to produce no output with exit code 0. Detect multi-line `python -c` commands on Windows, parse them into exec args via `_split_python_c_args`, and use `create_subprocess_exec` to bypass cmd.exe entirely. Same principle as Codex's Rust `Command::args()`. Applied to both the direct execution path and the session spawn path. Added unit tests for the parser and the exec-vs-shell branching logic.
This commit is contained in:
parent
8e421eb976
commit
7c86223643
@ -3,17 +3,20 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
import shutil
|
|
||||||
import time
|
import time
|
||||||
import uuid
|
import uuid
|
||||||
from contextlib import suppress
|
from contextlib import suppress
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from nanobot.agent.tools.context import current_request_session_key
|
|
||||||
from nanobot.agent.tools.base import Tool, tool_parameters
|
from nanobot.agent.tools.base import Tool, tool_parameters
|
||||||
from nanobot.agent.tools.schema import BooleanSchema, IntegerSchema, StringSchema, tool_parameters_schema
|
from nanobot.agent.tools.context import current_request_session_key
|
||||||
|
from nanobot.agent.tools.schema import (
|
||||||
|
BooleanSchema,
|
||||||
|
IntegerSchema,
|
||||||
|
StringSchema,
|
||||||
|
tool_parameters_schema,
|
||||||
|
)
|
||||||
|
|
||||||
DEFAULT_YIELD_MS = 1000
|
DEFAULT_YIELD_MS = 1000
|
||||||
MAX_YIELD_MS = 30_000
|
MAX_YIELD_MS = 30_000
|
||||||
@ -289,29 +292,11 @@ class ExecSessionManager:
|
|||||||
shell_program: str | None,
|
shell_program: str | None,
|
||||||
login: bool,
|
login: bool,
|
||||||
) -> asyncio.subprocess.Process:
|
) -> asyncio.subprocess.Process:
|
||||||
from nanobot.agent.tools import shell
|
from nanobot.agent.tools.shell import ExecTool
|
||||||
|
|
||||||
if shell._IS_WINDOWS:
|
return await ExecTool._spawn(
|
||||||
return await asyncio.create_subprocess_shell(
|
command, cwd, env, shell_program, login,
|
||||||
command,
|
|
||||||
stdin=asyncio.subprocess.PIPE,
|
|
||||||
stdout=asyncio.subprocess.PIPE,
|
|
||||||
stderr=asyncio.subprocess.PIPE,
|
|
||||||
cwd=cwd,
|
|
||||||
env=env,
|
|
||||||
)
|
|
||||||
shell_program = shell_program or shutil.which("bash") or "/bin/bash"
|
|
||||||
args = [shell_program]
|
|
||||||
if login and shell_program.rsplit("/", 1)[-1] in {"bash", "zsh"}:
|
|
||||||
args.append("-l")
|
|
||||||
args.extend(["-c", command])
|
|
||||||
return await asyncio.create_subprocess_exec(
|
|
||||||
*args,
|
|
||||||
stdin=asyncio.subprocess.PIPE,
|
stdin=asyncio.subprocess.PIPE,
|
||||||
stdout=asyncio.subprocess.PIPE,
|
|
||||||
stderr=asyncio.subprocess.PIPE,
|
|
||||||
cwd=cwd,
|
|
||||||
env=env,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@ -16,21 +16,26 @@ from loguru import logger
|
|||||||
from pydantic import Field
|
from pydantic import Field
|
||||||
|
|
||||||
from nanobot.agent.tools.base import Tool, tool_parameters
|
from nanobot.agent.tools.base import Tool, tool_parameters
|
||||||
|
from nanobot.agent.tools.context import current_request_session_key
|
||||||
from nanobot.agent.tools.exec_session import (
|
from nanobot.agent.tools.exec_session import (
|
||||||
|
DEFAULT_EXEC_SESSION_MANAGER,
|
||||||
DEFAULT_MAX_OUTPUT_CHARS,
|
DEFAULT_MAX_OUTPUT_CHARS,
|
||||||
DEFAULT_YIELD_MS,
|
DEFAULT_YIELD_MS,
|
||||||
DEFAULT_EXEC_SESSION_MANAGER,
|
|
||||||
MAX_OUTPUT_CHARS,
|
MAX_OUTPUT_CHARS,
|
||||||
MAX_YIELD_MS,
|
MAX_YIELD_MS,
|
||||||
clamp_session_int,
|
clamp_session_int,
|
||||||
format_session_poll,
|
format_session_poll,
|
||||||
)
|
)
|
||||||
from nanobot.agent.tools.context import current_request_session_key
|
|
||||||
from nanobot.agent.tools.sandbox import wrap_command
|
from nanobot.agent.tools.sandbox import wrap_command
|
||||||
from nanobot.agent.tools.schema import BooleanSchema, IntegerSchema, StringSchema, tool_parameters_schema
|
from nanobot.agent.tools.schema import (
|
||||||
from nanobot.security.workspace_access import current_scope_allows_loopback, current_tool_workspace
|
BooleanSchema,
|
||||||
|
IntegerSchema,
|
||||||
|
StringSchema,
|
||||||
|
tool_parameters_schema,
|
||||||
|
)
|
||||||
from nanobot.config.paths import get_media_dir
|
from nanobot.config.paths import get_media_dir
|
||||||
from nanobot.config.schema import Base
|
from nanobot.config.schema import Base
|
||||||
|
from nanobot.security.workspace_access import current_scope_allows_loopback, current_tool_workspace
|
||||||
from nanobot.security.workspace_policy import is_path_within
|
from nanobot.security.workspace_policy import is_path_within
|
||||||
|
|
||||||
_IS_WINDOWS = sys.platform == "win32"
|
_IS_WINDOWS = sys.platform == "win32"
|
||||||
@ -431,16 +436,23 @@ class ExecTool(Tool):
|
|||||||
command: str, cwd: str, env: dict[str, str],
|
command: str, cwd: str, env: dict[str, str],
|
||||||
shell_program: str | None = None,
|
shell_program: str | None = None,
|
||||||
login: bool = True,
|
login: bool = True,
|
||||||
|
*,
|
||||||
|
stdin: int = asyncio.subprocess.DEVNULL,
|
||||||
) -> asyncio.subprocess.Process:
|
) -> asyncio.subprocess.Process:
|
||||||
"""Launch *command* in a platform-appropriate shell."""
|
"""Launch *command* in a platform-appropriate shell."""
|
||||||
if _IS_WINDOWS:
|
if _IS_WINDOWS:
|
||||||
# create_subprocess_exec re-quotes args via list2cmdline, which
|
if "\n" in command:
|
||||||
# breaks commands containing paths with spaces (e.g. "D:\Program
|
return await asyncio.create_subprocess_exec(
|
||||||
# Files\python.exe" "script.py"). create_subprocess_shell passes
|
"powershell", "-NoProfile", "-Command", command,
|
||||||
# the raw command string to COMSPEC without re-quoting.
|
stdin=stdin,
|
||||||
|
stdout=asyncio.subprocess.PIPE,
|
||||||
|
stderr=asyncio.subprocess.PIPE,
|
||||||
|
cwd=cwd,
|
||||||
|
env=env,
|
||||||
|
)
|
||||||
return await asyncio.create_subprocess_shell(
|
return await asyncio.create_subprocess_shell(
|
||||||
command,
|
command,
|
||||||
stdin=asyncio.subprocess.DEVNULL,
|
stdin=stdin,
|
||||||
stdout=asyncio.subprocess.PIPE,
|
stdout=asyncio.subprocess.PIPE,
|
||||||
stderr=asyncio.subprocess.PIPE,
|
stderr=asyncio.subprocess.PIPE,
|
||||||
cwd=cwd,
|
cwd=cwd,
|
||||||
@ -454,7 +466,7 @@ class ExecTool(Tool):
|
|||||||
args.extend(["-c", command])
|
args.extend(["-c", command])
|
||||||
return await asyncio.create_subprocess_exec(
|
return await asyncio.create_subprocess_exec(
|
||||||
*args,
|
*args,
|
||||||
stdin=asyncio.subprocess.DEVNULL,
|
stdin=stdin,
|
||||||
stdout=asyncio.subprocess.PIPE,
|
stdout=asyncio.subprocess.PIPE,
|
||||||
stderr=asyncio.subprocess.PIPE,
|
stderr=asyncio.subprocess.PIPE,
|
||||||
cwd=cwd,
|
cwd=cwd,
|
||||||
|
|||||||
@ -116,7 +116,7 @@ class TestSpawnUnix:
|
|||||||
class TestSpawnWindows:
|
class TestSpawnWindows:
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_uses_create_subprocess_shell(self):
|
async def test_single_line_uses_shell(self):
|
||||||
env = {"COMSPEC": r"C:\Windows\system32\cmd.exe", "PATH": ""}
|
env = {"COMSPEC": r"C:\Windows\system32\cmd.exe", "PATH": ""}
|
||||||
with (
|
with (
|
||||||
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
||||||
@ -132,7 +132,7 @@ class TestSpawnWindows:
|
|||||||
assert kwargs["stdin"] == asyncio.subprocess.DEVNULL
|
assert kwargs["stdin"] == asyncio.subprocess.DEVNULL
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_passes_cwd_and_env(self):
|
async def test_single_line_passes_cwd_and_env(self):
|
||||||
env = {"PATH": "/usr/bin"}
|
env = {"PATH": "/usr/bin"}
|
||||||
with (
|
with (
|
||||||
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
||||||
@ -145,6 +145,27 @@ class TestSpawnWindows:
|
|||||||
assert kwargs["cwd"] == r"C:\work"
|
assert kwargs["cwd"] == r"C:\work"
|
||||||
assert kwargs["env"] == env
|
assert kwargs["env"] == env
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_multiline_uses_powershell(self):
|
||||||
|
env = {"PATH": ""}
|
||||||
|
with (
|
||||||
|
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
||||||
|
patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec,
|
||||||
|
):
|
||||||
|
mock_exec.return_value = AsyncMock()
|
||||||
|
await ExecTool._spawn('python -c "print(1)\nprint(2)"', r"C:\work", env)
|
||||||
|
|
||||||
|
args = mock_exec.call_args[0]
|
||||||
|
assert args[0] == "powershell"
|
||||||
|
assert "-NoProfile" in args
|
||||||
|
assert "-Command" in args
|
||||||
|
assert "print(1)" in args[-1]
|
||||||
|
assert "print(2)" in args[-1]
|
||||||
|
|
||||||
|
kwargs = mock_exec.call_args[1]
|
||||||
|
assert kwargs["cwd"] == r"C:\work"
|
||||||
|
assert kwargs["env"] == env
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# path_append
|
# path_append
|
||||||
@ -352,3 +373,85 @@ class TestExtractAbsolutePaths:
|
|||||||
cmd = "echo hello"
|
cmd = "echo hello"
|
||||||
paths = ExecTool._extract_absolute_paths(cmd)
|
paths = ExecTool._extract_absolute_paths(cmd)
|
||||||
assert paths == []
|
assert paths == []
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Windows multi-line command PowerShell fallback
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
class TestWindowsMultilineExec:
|
||||||
|
"""Verify multi-line commands on Windows route through PowerShell."""
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_multiline_python_uses_powershell(self):
|
||||||
|
mock_proc = AsyncMock()
|
||||||
|
mock_proc.communicate.return_value = (b"1\n2\n", b"")
|
||||||
|
mock_proc.returncode = 0
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
||||||
|
patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec,
|
||||||
|
patch.object(ExecTool, "_guard_command", return_value=None),
|
||||||
|
):
|
||||||
|
mock_exec.return_value = mock_proc
|
||||||
|
tool = ExecTool()
|
||||||
|
result = await tool.execute(command='python -c "print(1)\nprint(2)"')
|
||||||
|
|
||||||
|
assert "1" in result
|
||||||
|
assert "2" in result
|
||||||
|
assert "Exit code: 0" in result
|
||||||
|
args = mock_exec.call_args[0]
|
||||||
|
assert args[0] == "powershell"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_multiline_node_uses_powershell(self):
|
||||||
|
mock_proc = AsyncMock()
|
||||||
|
mock_proc.communicate.return_value = (b"1\n", b"")
|
||||||
|
mock_proc.returncode = 0
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
||||||
|
patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec,
|
||||||
|
patch.object(ExecTool, "_guard_command", return_value=None),
|
||||||
|
):
|
||||||
|
mock_exec.return_value = mock_proc
|
||||||
|
tool = ExecTool()
|
||||||
|
result = await tool.execute(command='node -e "console.log(1)\nconsole.log(2)"')
|
||||||
|
|
||||||
|
assert "1" in result
|
||||||
|
args = mock_exec.call_args[0]
|
||||||
|
assert args[0] == "powershell"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_single_line_uses_shell(self):
|
||||||
|
mock_proc = AsyncMock()
|
||||||
|
mock_proc.communicate.return_value = (b"1\n", b"")
|
||||||
|
mock_proc.returncode = 0
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
||||||
|
patch.object(ExecTool, "_spawn", return_value=mock_proc) as mock_spawn,
|
||||||
|
patch.object(ExecTool, "_guard_command", return_value=None),
|
||||||
|
):
|
||||||
|
tool = ExecTool()
|
||||||
|
result = await tool.execute(command='python -c "print(1)"')
|
||||||
|
|
||||||
|
assert "1" in result
|
||||||
|
mock_spawn.assert_called_once()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_unix_unchanged(self):
|
||||||
|
mock_proc = AsyncMock()
|
||||||
|
mock_proc.communicate.return_value = (b"1\n2\n", b"")
|
||||||
|
mock_proc.returncode = 0
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("nanobot.agent.tools.shell._IS_WINDOWS", False),
|
||||||
|
patch.object(ExecTool, "_spawn", return_value=mock_proc) as mock_spawn,
|
||||||
|
patch.object(ExecTool, "_guard_command", return_value=None),
|
||||||
|
):
|
||||||
|
tool = ExecTool()
|
||||||
|
result = await tool.execute(command='python -c "print(1)\nprint(2)"')
|
||||||
|
|
||||||
|
assert "1" in result
|
||||||
|
mock_spawn.assert_called_once()
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user