diff --git a/src/mcp/cli/cli.py b/src/mcp/cli/cli.py index 62334a4a2c..573d432842 100644 --- a/src/mcp/cli/cli.py +++ b/src/mcp/cli/cli.py @@ -3,6 +3,7 @@ import importlib.metadata import importlib.util import os +import shutil import subprocess import sys from pathlib import Path @@ -39,15 +40,18 @@ ) -def _get_npx_command(): +def _get_npx_command() -> str | None: """Get the correct npx command for the current platform.""" if sys.platform == "win32": # Try both npx.cmd and npx.exe on Windows for cmd in ["npx.cmd", "npx.exe", "npx"]: + npx_path = shutil.which(cmd) + if not npx_path: + continue try: - subprocess.run([cmd, "--version"], check=True, capture_output=True, shell=True) - return cmd - except subprocess.CalledProcessError: + subprocess.run([npx_path, "--version"], check=True, capture_output=True) + return npx_path + except (OSError, subprocess.CalledProcessError): continue return None return "npx" # On Unix-like systems, just use npx @@ -271,12 +275,9 @@ def dev( ) sys.exit(1) - # Run the MCP Inspector command with shell=True on Windows - shell = sys.platform == "win32" process = subprocess.run( [npx_cmd, "@modelcontextprotocol/inspector"] + uv_cmd, check=True, - shell=shell, env=dict(os.environ.items()), # Convert to list of tuples for env update ) sys.exit(process.returncode) diff --git a/tests/cli/test_utils.py b/tests/cli/test_utils.py index 44f4ab4d31..189fb44af0 100644 --- a/tests/cli/test_utils.py +++ b/tests/cli/test_utils.py @@ -5,6 +5,7 @@ import pytest +from mcp.cli import cli as cli_module from mcp.cli.cli import _build_uv_command, _get_npx_command, _parse_file_path # type: ignore[reportPrivateUsage] @@ -78,24 +79,69 @@ def test_get_npx_unix_like(monkeypatch: pytest.MonkeyPatch): def test_get_npx_windows(monkeypatch: pytest.MonkeyPatch): """Should return one of the npx candidates on Windows.""" candidates = ["npx.cmd", "npx.exe", "npx"] + located = {cmd: f"C:\\Node\\{cmd}" for cmd in candidates} def fake_run(cmd: list[str], **kw: Any) -> subprocess.CompletedProcess[bytes]: - if cmd[0] in candidates: + assert kw.get("shell") is not True + assert cmd[0] in located.values() + if Path(cmd[0]).name in candidates: return subprocess.CompletedProcess(cmd, 0) else: # pragma: no cover raise subprocess.CalledProcessError(1, cmd[0]) monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setattr("shutil.which", located.get) monkeypatch.setattr(subprocess, "run", fake_run) - assert _get_npx_command() in candidates + assert _get_npx_command() in located.values() + + +def test_get_npx_windows_skips_failed_candidates(monkeypatch: pytest.MonkeyPatch): + """Should keep checking candidates if one exists but fails.""" + located = {"npx.cmd": "C:\\Node\\npx.cmd", "npx.exe": "C:\\Node\\npx.exe"} + + def fake_run(cmd: list[str], **kw: Any) -> subprocess.CompletedProcess[bytes]: + assert kw.get("shell") is not True + if cmd[0].endswith("npx.cmd"): + raise subprocess.CalledProcessError(1, cmd) + return subprocess.CompletedProcess(cmd, 0) + + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setattr("shutil.which", located.get) + monkeypatch.setattr(subprocess, "run", fake_run) + + assert _get_npx_command() == located["npx.exe"] def test_get_npx_returns_none_when_npx_missing(monkeypatch: pytest.MonkeyPatch): """Should give None if every candidate fails.""" monkeypatch.setattr(sys, "platform", "win32", raising=False) + monkeypatch.setattr("shutil.which", lambda cmd: None) + assert _get_npx_command() is None - def always_fail(*args: Any, **kwargs: Any) -> subprocess.CompletedProcess[bytes]: - raise subprocess.CalledProcessError(1, args[0]) - monkeypatch.setattr(subprocess, "run", always_fail) - assert _get_npx_command() is None +def test_dev_runs_inspector_without_shell(monkeypatch: pytest.MonkeyPatch): + """mcp dev should not route file paths or args through a platform shell.""" + calls: list[tuple[list[str], dict[str, Any]]] = [] + + class Server: + dependencies = ["server-dep"] + + def fake_run(cmd: list[str], **kw: Any) -> subprocess.CompletedProcess[str]: + calls.append((cmd, kw)) + return subprocess.CompletedProcess(cmd, 0) + + monkeypatch.setattr(cli_module, "_parse_file_path", lambda file_spec: (Path("server&calc.py"), None)) + monkeypatch.setattr(cli_module, "_import_server", lambda file, server_object: Server()) + monkeypatch.setattr(cli_module, "_build_uv_command", lambda file_spec, with_editable, with_packages: ["uv", "run"]) + monkeypatch.setattr(cli_module, "_get_npx_command", lambda: "C:\\Node\\npx.cmd") + monkeypatch.setattr(subprocess, "run", fake_run) + + with pytest.raises(SystemExit) as exc_info: + cli_module.dev("server&calc.py", with_packages=["cli-dep"]) + + assert exc_info.value.code == 0 + assert len(calls) == 1 + cmd, kwargs = calls[0] + assert cmd == ["C:\\Node\\npx.cmd", "@modelcontextprotocol/inspector", "uv", "run"] + assert kwargs["check"] is True + assert "shell" not in kwargs