Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 62 additions & 1 deletion tests/tools/test_mcp_tool_issue_948.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@

import pytest

from tools.mcp_tool import MCPServerTask, _format_connect_error, _resolve_stdio_command, _MCP_AVAILABLE
from tools.mcp_tool import (
MCPServerTask,
_MCP_AVAILABLE,
_format_connect_error,
_resolve_stdio_command,
_resolve_stdio_cwd,
)

# Ensure the mcp module symbols exist for patching even when the SDK isn't installed
if not _MCP_AVAILABLE:
Expand Down Expand Up @@ -49,6 +55,29 @@ def _fake_which(_cmd, path=None):
assert seen_paths == [""]


def test_resolve_stdio_cwd_uses_windows_mount_for_wsl_windows_exe():
with patch("tools.mcp_tool.is_wsl", return_value=True), \
patch.dict("os.environ", {"USERNAME": "Administrator"}, clear=False), \
patch("tools.mcp_tool.Path.is_dir", autospec=True, side_effect=lambda self: str(self) == "/mnt/c/Users/Administrator"):
cwd = _resolve_stdio_cwd("cmd.exe", None)

assert cwd == "/mnt/c/Users/Administrator"


def test_resolve_stdio_cwd_respects_explicit_config():
with patch("tools.mcp_tool.is_wsl", return_value=True):
cwd = _resolve_stdio_cwd("cmd.exe", "/mnt/c/workspace")

assert cwd == "/mnt/c/workspace"


def test_resolve_stdio_cwd_skips_non_windows_commands():
with patch("tools.mcp_tool.is_wsl", return_value=True):
cwd = _resolve_stdio_cwd("npx", None)

assert cwd is None


def test_format_connect_error_unwraps_exception_group():
error = ExceptionGroup(
"unhandled errors in a TaskGroup",
Expand Down Expand Up @@ -91,6 +120,38 @@ async def _test():
call_kwargs = mock_params.call_args.kwargs
assert call_kwargs["command"] == str(npx_path)
assert call_kwargs["env"]["PATH"].split(os.pathsep)[0] == str(node_bin)
assert call_kwargs["cwd"] is None

await server.shutdown()

asyncio.run(_test())


def test_run_stdio_uses_safe_windows_cwd_on_wsl(tmp_path):
mock_session = MagicMock()
mock_session.initialize = AsyncMock()
mock_session.list_tools = AsyncMock(return_value=SimpleNamespace(tools=[]))

mock_stdio_cm = MagicMock()
mock_stdio_cm.__aenter__ = AsyncMock(return_value=(object(), object()))
mock_stdio_cm.__aexit__ = AsyncMock(return_value=False)

mock_session_cm = MagicMock()
mock_session_cm.__aenter__ = AsyncMock(return_value=mock_session)
mock_session_cm.__aexit__ = AsyncMock(return_value=False)

async def _test():
with patch("tools.mcp_tool.is_wsl", return_value=True), \
patch("tools.mcp_tool._resolve_stdio_cwd", return_value="/mnt/c/Users/Administrator"), \
patch("tools.mcp_tool.StdioServerParameters") as mock_params, \
patch("tools.mcp_tool.stdio_client", return_value=mock_stdio_cm), \
patch("tools.mcp_tool.ClientSession", return_value=mock_session_cm):
server = MCPServerTask("srv")
await server.start({"command": "cmd.exe", "args": ["/c", "echo", "ok"]})

call_kwargs = mock_params.call_args.kwargs
assert call_kwargs["command"].endswith("cmd.exe")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main wraps POSIX stdio commands through sys.executable tools/mcp_stdio_watchdog.py ... -- <real-command> at tools/mcp_tool.py:2066. On WSL this assertion should instead verify the wrapper command and assert that cmd.exe appears after --; keep the cwd assertion to cover this fix.

assert call_kwargs["cwd"] == "/mnt/c/Users/Administrator"

await server.shutdown()

Expand Down
45 changes: 45 additions & 0 deletions tools/mcp_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,11 @@
import shutil
import threading
import time
from pathlib import Path
from typing import Any, Dict, List, Optional

from hermes_constants import is_wsl

logger = logging.getLogger(__name__)

# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -267,6 +270,45 @@ def _resolve_stdio_command(command: str, env: dict) -> tuple[str, dict]:
return resolved_command, resolved_env


def _looks_like_windows_executable(command: str) -> bool:
"""Return True when *command* appears to be a Windows executable name/path."""
lowered = os.path.basename(str(command).strip()).lower()
return lowered.endswith((".exe", ".cmd", ".bat", ".com"))


def _resolve_stdio_cwd(command: str, config_cwd: Optional[str]) -> Optional[str]:
"""Resolve a safe working directory for stdio MCP subprocesses.

On WSL, launching ``cmd.exe`` (or another Windows executable) from a Linux
cwd like ``/root`` makes Windows emit a UNC-path warning on stdout. That
corrupts stdio-based MCP traffic. When no explicit cwd is configured, route
Windows executables through a Windows-mounted path instead.
"""
if config_cwd:
return os.path.expanduser(str(config_cwd))

if not is_wsl() or not _looks_like_windows_executable(command):
return None

candidates = []
for username in (
os.environ.get("USERNAME"),
os.environ.get("WIN_USERNAME"),
):
if username:
candidates.append(Path("/mnt/c/Users") / username)

candidates.extend([
Path("/mnt/c/Users/Administrator"),
Path("/mnt/c/Users"),
Path("/mnt/c"),
])
for candidate in candidates:
if candidate.is_dir():
return str(candidate)
return None


def _format_connect_error(exc: BaseException) -> str:
"""Render nested MCP connection errors into an actionable short message."""

Expand Down Expand Up @@ -825,6 +867,7 @@ async def _run_stdio(self, config: dict):
command = config.get("command")
args = config.get("args", [])
user_env = config.get("env")
config_cwd = config.get("cwd")

if not command:
raise ValueError(
Expand All @@ -833,6 +876,7 @@ async def _run_stdio(self, config: dict):

safe_env = _build_safe_env(user_env)
command, safe_env = _resolve_stdio_command(command, safe_env)
resolved_cwd = _resolve_stdio_cwd(command, config_cwd)

# Check package against OSV malware database before spawning
from tools.osv_check import check_package_for_malware
Expand All @@ -846,6 +890,7 @@ async def _run_stdio(self, config: dict):
command=command,
args=args,
env=safe_env if safe_env else None,
cwd=resolved_cwd,
)

sampling_kwargs = self._sampling.session_kwargs() if self._sampling else {}
Expand Down
38 changes: 38 additions & 0 deletions website/docs/user-guide/features/mcp.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ Hermes reads MCP config from `~/.hermes/config.yaml` under `mcp_servers`.
| `command` | string | Executable for a stdio MCP server |
| `args` | list | Arguments for the stdio server |
| `env` | mapping | Environment variables passed to the stdio server |
| `cwd` | string | Working directory for stdio server subprocesses |
| `url` | string | HTTP MCP endpoint |
| `headers` | mapping | HTTP headers for remote servers |
| `timeout` | number | Tool call timeout |
Expand Down Expand Up @@ -311,6 +312,43 @@ For stdio servers, Hermes does not blindly pass your full shell environment.

Only explicitly configured `env` plus a safe baseline are passed through. This reduces accidental secret leakage.

### Stdio cwd control

For stdio servers, you can also set an explicit working directory:

```yaml
mcp_servers:
chrome_devtools:
command: "cmd.exe"
args: ["/c", "npx -y chrome-devtools-mcp@latest --autoConnect"]
cwd: "/mnt/c/Users/Administrator"
```

This is especially useful on WSL when the stdio server command is a Windows executable such as `cmd.exe` or `powershell.exe`.

If Hermes is launched from a Linux-only WSL path like `/root` or `/home/user`, Windows may treat that launch directory as a `\\wsl.localhost\...` UNC path and print a warning before the MCP server starts. That extra stdout text corrupts the stdio MCP protocol stream and makes the server appear to fail immediately.

Hermes now automatically picks a Windows-mounted working directory for Windows stdio executables on WSL when `cwd` is not set, but you can still override it explicitly if needed.

### WSL troubleshooting for Windows-backed stdio servers

If an MCP server works in `hermes mcp test` but shows as failed when Hermes starts on WSL, check where you launched Hermes from.

Avoid launching Hermes from Linux-only WSL directories when the MCP command is a Windows executable:

- bad: `/root`, `/home/...`
- good: `/mnt/c/Users/<you>`, `/mnt/c/workspace/...`

Typical failure symptom:

```text
'\\wsl.localhost\Ubuntu\root'
CMD.EXE was started with the above path as the current directory.
UNC paths are not supported. Defaulting to Windows directory.
```

That message comes from `cmd.exe`, not from the MCP server itself.

### Config-level exposure control

The new filtering support is also a security control:
Expand Down