diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index dda5e15b..ccbe98bd 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -89,7 +89,7 @@ class _PreparedCommand: maximum=600, ), shell=StringSchema( - "Optional shell binary to launch. On Unix, supports sh, bash, or zsh.", + "Optional shell binary to launch. Unix: sh, bash, zsh. Windows: powershell, pwsh, cmd.", nullable=True, ), login=BooleanSchema( @@ -468,17 +468,23 @@ class ExecTool(Tool): ) -> asyncio.subprocess.Process: """Launch *command* in a platform-appropriate shell.""" if _IS_WINDOWS: - if "\n" in command: - return await asyncio.create_subprocess_exec( - "powershell", "-NoProfile", "-Command", command, + # Default to PowerShell so single-line and multi-line commands + # share the same shell semantics. cmd.exe is reachable via the + # explicit shell="cmd" parameter (see _resolve_shell). + default_program = shutil.which("powershell") or "powershell" + program = shell_program or default_program + program_name = Path(program).name.lower() + if program_name in ("cmd", "cmd.exe"): + return await asyncio.create_subprocess_shell( + command, stdin=stdin, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=cwd, env=env, ) - return await asyncio.create_subprocess_shell( - command, + return await asyncio.create_subprocess_exec( + program, "-NoProfile", "-Command", command, stdin=stdin, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, @@ -504,10 +510,33 @@ class ExecTool(Tool): def _resolve_shell(shell: str | None) -> tuple[str | None, str | None]: if not shell: return None, None - if _IS_WINDOWS: - return None, ToolResult.error("Error: shell parameter is not supported on Windows") if "\0" in shell or "\n" in shell or "\r" in shell: return None, ToolResult.error("Error: shell contains invalid characters") + if _IS_WINDOWS: + win_allowed = {"powershell", "powershell.exe", "pwsh", "pwsh.exe", "cmd", "cmd.exe"} + path = Path(shell).expanduser() + if path.is_absolute(): + name = path.name.lower() + if name not in win_allowed: + return None, ToolResult.error( + f"Error: unsupported shell {shell!r}. " + "Allowed: powershell, pwsh, cmd" + ) + if not path.is_file(): + return None, ToolResult.error(f"Error: shell is not found: {shell}") + return str(path), None + if "/" in shell or "\\" in shell: + return None, ToolResult.error("Error: shell must be a shell name or absolute path") + if shell.lower() not in win_allowed: + return None, ToolResult.error( + f"Error: unsupported shell {shell!r}. " + "Allowed: powershell, pwsh, cmd" + ) + if shell.lower() in ("cmd", "cmd.exe"): + resolved = os.environ.get("COMSPEC") or shutil.which("cmd") or "cmd" + return resolved, None + resolved = shutil.which(shell) or shell + return resolved, None allowed = {"sh", "bash", "zsh"} path = Path(shell).expanduser() if path.is_absolute(): diff --git a/tests/tools/test_exec_platform.py b/tests/tools/test_exec_platform.py index 0a5337f6..eacbf9da 100644 --- a/tests/tools/test_exec_platform.py +++ b/tests/tools/test_exec_platform.py @@ -116,32 +116,37 @@ class TestSpawnUnix: class TestSpawnWindows: @pytest.mark.asyncio - async def test_single_line_uses_shell(self): + async def test_single_line_uses_powershell(self): + """Single-line commands on Windows now route through PowerShell.""" env = {"COMSPEC": r"C:\Windows\system32\cmd.exe", "PATH": ""} with ( patch("nanobot.agent.tools.shell._IS_WINDOWS", True), - patch("asyncio.create_subprocess_shell", new_callable=AsyncMock) as mock_shell, + patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec, ): - mock_shell.return_value = AsyncMock() + mock_exec.return_value = AsyncMock() await ExecTool._spawn("dir", r"C:\work", env) - args = mock_shell.call_args[0] - assert "dir" in args + args = mock_exec.call_args[0] + assert "powershell" in args[0].lower() + assert "-NoProfile" in args + assert "-Command" in args + assert "dir" in args[-1] - kwargs = mock_shell.call_args[1] + kwargs = mock_exec.call_args[1] assert kwargs["stdin"] == asyncio.subprocess.DEVNULL @pytest.mark.asyncio async def test_single_line_passes_cwd_and_env(self): + """PowerShell should receive cwd and env from the caller.""" env = {"PATH": "/usr/bin"} with ( patch("nanobot.agent.tools.shell._IS_WINDOWS", True), - patch("asyncio.create_subprocess_shell", new_callable=AsyncMock) as mock_shell, + patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec, ): - mock_shell.return_value = AsyncMock() + mock_exec.return_value = AsyncMock() await ExecTool._spawn("echo hi", r"C:\work", env) - kwargs = mock_shell.call_args[1] + kwargs = mock_exec.call_args[1] assert kwargs["cwd"] == r"C:\work" assert kwargs["env"] == env @@ -156,7 +161,7 @@ class TestSpawnWindows: await ExecTool._spawn('python -c "print(1)\nprint(2)"', r"C:\work", env) args = mock_exec.call_args[0] - assert args[0] == "powershell" + assert "powershell" in args[0].lower() assert "-NoProfile" in args assert "-Command" in args assert "print(1)" in args[-1] @@ -166,6 +171,24 @@ class TestSpawnWindows: assert kwargs["cwd"] == r"C:\work" assert kwargs["env"] == env + @pytest.mark.asyncio + async def test_explicit_cmd_shell_uses_create_subprocess_shell(self): + """Explicit shell='cmd' should use create_subprocess_shell.""" + env = {"COMSPEC": r"C:\Windows\system32\cmd.exe", "PATH": ""} + with ( + patch("nanobot.agent.tools.shell._IS_WINDOWS", True), + patch("asyncio.create_subprocess_shell", new_callable=AsyncMock) as mock_shell, + ): + mock_shell.return_value = AsyncMock() + await ExecTool._spawn( + "dir", r"C:\work", env, + shell_program=r"C:\Windows\system32\cmd.exe", + ) + + mock_shell.assert_called_once() + kwargs = mock_shell.call_args[1] + assert kwargs["cwd"] == r"C:\work" + # --------------------------------------------------------------------------- # path_append @@ -488,7 +511,7 @@ class TestExtractAbsolutePaths: # --------------------------------------------------------------------------- class TestWindowsMultilineExec: - """Verify multi-line commands on Windows route through PowerShell.""" + """Verify commands on Windows route through PowerShell (now the default).""" @pytest.mark.asyncio async def test_multiline_python_uses_powershell(self): @@ -509,7 +532,7 @@ class TestWindowsMultilineExec: assert "2" in result assert "Exit code: 0" in result args = mock_exec.call_args[0] - assert args[0] == "powershell" + assert "powershell" in args[0].lower() @pytest.mark.asyncio async def test_multiline_node_uses_powershell(self): @@ -528,10 +551,11 @@ class TestWindowsMultilineExec: assert "1" in result args = mock_exec.call_args[0] - assert args[0] == "powershell" + assert "powershell" in args[0].lower() @pytest.mark.asyncio - async def test_single_line_uses_shell(self): + async def test_single_line_uses_powershell(self): + """Single-line commands also route through PowerShell now.""" mock_proc = AsyncMock() mock_proc.communicate.return_value = (b"1\n", b"") mock_proc.returncode = 0 @@ -563,3 +587,60 @@ class TestWindowsMultilineExec: assert "1" in result mock_spawn.assert_called_once() + + +# --------------------------------------------------------------------------- +# _resolve_shell — Windows support +# --------------------------------------------------------------------------- + +class TestResolveShellWindows: + """shell parameter is now accepted on Windows.""" + + @pytest.mark.asyncio + async def test_shell_powershell_accepted(self): + """shell='powershell' should resolve and route through PowerShell.""" + mock_proc = AsyncMock() + mock_proc.communicate.return_value = (b"hello\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="echo hello", shell="powershell") + + assert "hello" in result + args = mock_exec.call_args[0] + assert "powershell" in args[0].lower() + + @pytest.mark.asyncio + async def test_shell_cmd_accepted(self): + """shell='cmd' should use create_subprocess_shell.""" + mock_proc = AsyncMock() + mock_proc.communicate.return_value = (b"hello\n", b"") + mock_proc.returncode = 0 + + with ( + patch("nanobot.agent.tools.shell._IS_WINDOWS", True), + patch("asyncio.create_subprocess_shell", new_callable=AsyncMock) as mock_shell, + patch.object(ExecTool, "_guard_command", return_value=None), + ): + mock_shell.return_value = mock_proc + tool = ExecTool() + result = await tool.execute(command="echo hello", shell="cmd") + + assert "hello" in result + mock_shell.assert_called_once() + + @pytest.mark.asyncio + async def test_shell_bash_rejected_on_windows(self): + """shell='bash' should still be rejected on Windows.""" + with patch("nanobot.agent.tools.shell._IS_WINDOWS", True): + tool = ExecTool() + result = await tool.execute(command="echo hello", shell="bash") + + assert "Error: unsupported shell" in result + assert "Allowed: powershell, pwsh, cmd" in result