fix: harden Windows exec shell default
Maintainer edit: prefer pwsh when available so single-line commands support modern PowerShell operators, add -NonInteractive, and parse explicit cmd.exe paths with Windows path semantics for cross-platform tests.
This commit is contained in:
@@ -9,7 +9,7 @@ import shutil
|
|||||||
import sys
|
import sys
|
||||||
from contextlib import suppress
|
from contextlib import suppress
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path, PureWindowsPath
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from loguru import logger
|
from loguru import logger
|
||||||
@@ -471,9 +471,9 @@ class ExecTool(Tool):
|
|||||||
# Default to PowerShell so single-line and multi-line commands
|
# Default to PowerShell so single-line and multi-line commands
|
||||||
# share the same shell semantics. cmd.exe is reachable via the
|
# share the same shell semantics. cmd.exe is reachable via the
|
||||||
# explicit shell="cmd" parameter (see _resolve_shell).
|
# explicit shell="cmd" parameter (see _resolve_shell).
|
||||||
default_program = shutil.which("powershell") or "powershell"
|
default_program = shutil.which("pwsh") or shutil.which("powershell") or "powershell"
|
||||||
program = shell_program or default_program
|
program = shell_program or default_program
|
||||||
program_name = Path(program).name.lower()
|
program_name = PureWindowsPath(program).name.lower()
|
||||||
if program_name in ("cmd", "cmd.exe"):
|
if program_name in ("cmd", "cmd.exe"):
|
||||||
return await asyncio.create_subprocess_shell(
|
return await asyncio.create_subprocess_shell(
|
||||||
command,
|
command,
|
||||||
@@ -484,7 +484,7 @@ class ExecTool(Tool):
|
|||||||
env=env,
|
env=env,
|
||||||
)
|
)
|
||||||
return await asyncio.create_subprocess_exec(
|
return await asyncio.create_subprocess_exec(
|
||||||
program, "-NoProfile", "-Command", command,
|
program, "-NoProfile", "-NonInteractive", "-Command", command,
|
||||||
stdin=stdin,
|
stdin=stdin,
|
||||||
stdout=asyncio.subprocess.PIPE,
|
stdout=asyncio.subprocess.PIPE,
|
||||||
stderr=asyncio.subprocess.PIPE,
|
stderr=asyncio.subprocess.PIPE,
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ platform-specific binaries (all subprocess calls are mocked).
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import shutil
|
||||||
import sys
|
import sys
|
||||||
from unittest.mock import AsyncMock, patch
|
from unittest.mock import AsyncMock, patch
|
||||||
|
|
||||||
@@ -127,8 +128,9 @@ class TestSpawnWindows:
|
|||||||
await ExecTool._spawn("dir", r"C:\work", env)
|
await ExecTool._spawn("dir", r"C:\work", env)
|
||||||
|
|
||||||
args = mock_exec.call_args[0]
|
args = mock_exec.call_args[0]
|
||||||
assert "powershell" in args[0].lower()
|
assert any(shell in args[0].lower() for shell in ("pwsh", "powershell"))
|
||||||
assert "-NoProfile" in args
|
assert "-NoProfile" in args
|
||||||
|
assert "-NonInteractive" in args
|
||||||
assert "-Command" in args
|
assert "-Command" in args
|
||||||
assert "dir" in args[-1]
|
assert "dir" in args[-1]
|
||||||
|
|
||||||
@@ -161,8 +163,9 @@ class TestSpawnWindows:
|
|||||||
await ExecTool._spawn('python -c "print(1)\nprint(2)"', r"C:\work", env)
|
await ExecTool._spawn('python -c "print(1)\nprint(2)"', r"C:\work", env)
|
||||||
|
|
||||||
args = mock_exec.call_args[0]
|
args = mock_exec.call_args[0]
|
||||||
assert "powershell" in args[0].lower()
|
assert any(shell in args[0].lower() for shell in ("pwsh", "powershell"))
|
||||||
assert "-NoProfile" in args
|
assert "-NoProfile" in args
|
||||||
|
assert "-NonInteractive" in args
|
||||||
assert "-Command" in args
|
assert "-Command" in args
|
||||||
assert "print(1)" in args[-1]
|
assert "print(1)" in args[-1]
|
||||||
assert "print(2)" in args[-1]
|
assert "print(2)" in args[-1]
|
||||||
@@ -189,6 +192,28 @@ class TestSpawnWindows:
|
|||||||
kwargs = mock_shell.call_args[1]
|
kwargs = mock_shell.call_args[1]
|
||||||
assert kwargs["cwd"] == r"C:\work"
|
assert kwargs["cwd"] == r"C:\work"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_prefers_pwsh_when_available(self):
|
||||||
|
env = {"PATH": ""}
|
||||||
|
|
||||||
|
def fake_which(command):
|
||||||
|
if command == "pwsh":
|
||||||
|
return r"C:\Program Files\PowerShell\7\pwsh.exe"
|
||||||
|
if command == "powershell":
|
||||||
|
return r"C:\Windows\system32\WindowsPowerShell\v1.0\powershell.exe"
|
||||||
|
return None
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("nanobot.agent.tools.shell._IS_WINDOWS", True),
|
||||||
|
patch("nanobot.agent.tools.shell.shutil.which", side_effect=fake_which),
|
||||||
|
patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec,
|
||||||
|
):
|
||||||
|
mock_exec.return_value = AsyncMock()
|
||||||
|
await ExecTool._spawn("dir", r"C:\work", env)
|
||||||
|
|
||||||
|
args = mock_exec.call_args[0]
|
||||||
|
assert "pwsh" in args[0].lower()
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# path_append
|
# path_append
|
||||||
@@ -532,7 +557,7 @@ class TestWindowsMultilineExec:
|
|||||||
assert "2" in result
|
assert "2" in result
|
||||||
assert "Exit code: 0" in result
|
assert "Exit code: 0" in result
|
||||||
args = mock_exec.call_args[0]
|
args = mock_exec.call_args[0]
|
||||||
assert "powershell" in args[0].lower()
|
assert any(shell in args[0].lower() for shell in ("pwsh", "powershell"))
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_multiline_node_uses_powershell(self):
|
async def test_multiline_node_uses_powershell(self):
|
||||||
@@ -551,7 +576,7 @@ class TestWindowsMultilineExec:
|
|||||||
|
|
||||||
assert "1" in result
|
assert "1" in result
|
||||||
args = mock_exec.call_args[0]
|
args = mock_exec.call_args[0]
|
||||||
assert "powershell" in args[0].lower()
|
assert any(shell in args[0].lower() for shell in ("pwsh", "powershell"))
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_single_line_uses_powershell(self):
|
async def test_single_line_uses_powershell(self):
|
||||||
@@ -615,6 +640,7 @@ class TestResolveShellWindows:
|
|||||||
assert "hello" in result
|
assert "hello" in result
|
||||||
args = mock_exec.call_args[0]
|
args = mock_exec.call_args[0]
|
||||||
assert "powershell" in args[0].lower()
|
assert "powershell" in args[0].lower()
|
||||||
|
assert "-NonInteractive" in args
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_shell_cmd_accepted(self):
|
async def test_shell_cmd_accepted(self):
|
||||||
@@ -644,3 +670,18 @@ class TestResolveShellWindows:
|
|||||||
|
|
||||||
assert "Error: unsupported shell" in result
|
assert "Error: unsupported shell" in result
|
||||||
assert "Allowed: powershell, pwsh, cmd" in result
|
assert "Allowed: powershell, pwsh, cmd" in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.skipif(
|
||||||
|
sys.platform != "win32" or shutil.which("pwsh") is None,
|
||||||
|
reason="requires Windows with PowerShell 7",
|
||||||
|
)
|
||||||
|
class TestWindowsRealExec:
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_single_line_and_separator_uses_pwsh(self):
|
||||||
|
result = await ExecTool(timeout=10).execute(command="echo before && echo after")
|
||||||
|
|
||||||
|
assert "before" in result
|
||||||
|
assert "after" in result
|
||||||
|
assert "Exit code: 0" in result
|
||||||
|
|||||||
Reference in New Issue
Block a user