mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-31 06:51:29 +00:00
fix(mcp): resolve bare npx/npm/node against /usr/local/bin
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/<cmd> 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
This commit is contained in:
parent
fb51253620
commit
e7c99651fb
2 changed files with 44 additions and 0 deletions
|
|
@ -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 = []
|
||||
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue