From e7c99651fb608a2be1692a65c75bb9e68793baaf Mon Sep 17 00:00:00 2001 From: Ben Date: Fri, 29 May 2026 09:42:02 +1000 Subject: [PATCH] fix(mcp): resolve bare npx/npm/node against /usr/local/bin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the Hermes Docker image runs an stdio MCP server configured with an explicit env.PATH that omits /usr/local/bin (a common pattern when users hand-author PATH for sandboxing), the MCP env-filter passes that narrow PATH straight through to the subprocess. _resolve_stdio_command's fallback for bare 'npx' / 'npm' / 'node' commands only checked $HERMES_HOME/node/bin/ and ~/.local/bin/, so execvp() failed with '[Errno 2] No such file or directory: npx' on every Node-based stdio MCP server (Railway, Anthropic, GitHub Copilot, etc.). The naive workaround — symlink /usr/local/bin/npx into the user's PATH — fails one layer deeper because npx's shebang re-execs /usr/bin/env node and node also lives at /usr/local/bin/node. Fix: add /usr/local/bin/ as a third candidate in the fallback list. This is the canonical install location for Node on: - Linux from-source builds - the upstream node:bookworm-slim image, which the Hermes Docker image copies node + npm + corepack from since #4977 (the Node 22 LTS refactor that exposed this) - macOS Homebrew on Intel Because the resolver already calls _prepend_path(resolved_env, command_dir) after locating the command, /usr/local/bin gets prepended to the env's PATH automatically, which also fixes the second-layer shebang failure (npx-cli.js can now find node). Scope is intentionally narrow: the fix activates only when the bare command isn't otherwise locatable through the user's PATH. Users who explicitly narrowed PATH for a non-Node MCP server see no change in behavior. Tested: - tests/tools/test_mcp_tool_issue_948.py: new test test_resolve_stdio_command_falls_back_to_usr_local_bin (mirrors the existing hermes-node-bin fallback test) - Full MCP test suite: 254/254 pass across 7 test files - E2E against a freshly-built Docker image: reproduced the original failure mode (env.PATH=/opt/data/bin:/usr/bin:/bin), confirmed the resolver returns /usr/local/bin/npx and prepends /usr/local/bin to PATH; subprocess.run of the resolved command prints '10.9.8' and exits 0 with empty stderr - Negative E2E on the host (where Node is already on PATH via mise): resolver still hits the mise install dir, /usr/local/bin candidate is not consulted, PATH is unchanged --- tests/tools/test_mcp_tool_issue_948.py | 33 ++++++++++++++++++++++++++ tools/mcp_tool.py | 11 +++++++++ 2 files changed, 44 insertions(+) diff --git a/tests/tools/test_mcp_tool_issue_948.py b/tests/tools/test_mcp_tool_issue_948.py index c3e04220260..f7cc00709e4 100644 --- a/tests/tools/test_mcp_tool_issue_948.py +++ b/tests/tools/test_mcp_tool_issue_948.py @@ -34,6 +34,39 @@ def test_resolve_stdio_command_falls_back_to_hermes_node_bin(tmp_path): assert env["PATH"].split(os.pathsep)[0] == str(node_bin) +def test_resolve_stdio_command_falls_back_to_usr_local_bin(): + """When ``npx`` isn't on the filtered PATH and isn't under ``$HERMES_HOME/node/bin`` + or ``~/.local/bin``, the resolver should still locate it at ``/usr/local/bin/npx``. + + This is the canonical install location for Node on Linux from-source builds, + the upstream ``node:bookworm-slim`` image (which the Hermes Docker image + copies ``node + npm + corepack`` from since #4977), and macOS Homebrew on + Intel. Without this candidate, MCP servers run with an ``env.PATH`` that + omits ``/usr/local/bin`` (common when users hand-author PATH for sandboxing) + fail with ENOENT at ``execvp``. + """ + target = os.path.join(os.sep, "usr", "local", "bin", "npx") + + # Pretend ONLY the /usr/local/bin/npx candidate exists and is executable — + # the other candidates ($HERMES_HOME/node/bin/npx and ~/.local/bin/npx) + # should fail isfile() and the resolver must fall through to /usr/local/bin. + def _fake_isfile(path): + return path == target + + def _fake_access(path, _mode): + return path == target + + with patch("tools.mcp_tool.shutil.which", return_value=None), \ + patch("tools.mcp_tool.os.path.isfile", side_effect=_fake_isfile), \ + patch("tools.mcp_tool.os.access", side_effect=_fake_access): + command, env = _resolve_stdio_command("npx", {"PATH": "/opt/data/bin:/usr/bin:/bin"}) + + assert command == target + # /usr/local/bin must be prepended so npx's shebang (`/usr/bin/env node`) + # can find node in the same directory. + assert env["PATH"].split(os.pathsep)[0] == os.path.dirname(target) + + def test_resolve_stdio_command_respects_explicit_empty_path(): seen_paths = [] diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 157f79c1c52..5a3c7ab63af 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -422,6 +422,17 @@ def _resolve_stdio_command(command: str, env: dict) -> tuple[str, dict]: candidates = [ os.path.join(hermes_home, "node", "bin", resolved_command), os.path.join(os.path.expanduser("~"), ".local", "bin", resolved_command), + # /usr/local/bin is the canonical install location for Node on + # Linux from-source builds, the upstream node:bookworm-slim + # image (which the Hermes Docker image copies node + npm + + # corepack from since #4977), and macOS Homebrew on Intel. + # Without this candidate, any MCP server configured with an + # env.PATH that omits /usr/local/bin (a common pattern when + # users hand-author PATH for sandboxing) fails with ENOENT + # at execvp, and a naive symlink workaround into the user's + # PATH only fails one layer deeper because npx's shebang + # re-execs /usr/bin/env node which needs the same directory. + os.path.join(os.sep, "usr", "local", "bin", resolved_command), ] for candidate in candidates: if os.path.isfile(candidate) and os.access(candidate, os.X_OK):