fix(mcp): resolve ${ENV} in discovery probe so header auth works (#38571)

`hermes mcp add --auth header` built `Authorization: Bearer ${MCP_X_API_KEY}`
and passed it straight to the discovery probe without interpolation, so the
probe sent the literal placeholder and auth-requiring servers (e.g. n8n)
returned 401. Runtime tool loading worked because `_load_mcp_config()`
interpolates, but the four CLI probe call sites (add/test/login/configure)
all used unresolved config.

Resolve `${ENV}` inside `_probe_single_server` via a new
`_resolve_mcp_server_config()` (load_hermes_dotenv + _interpolate_env_vars),
mirroring runtime loading. This covers all four call sites, not just add.

Also strip a leading `Bearer ` from pasted tokens before saving to
`MCP_*_API_KEY`, so a token pasted with the prefix doesn't produce
`Bearer Bearer <jwt>` (also a 401).

Reported with a precise root-cause analysis in #37792.

Co-authored-by: ThyFriendlyFox <116314616+ThyFriendlyFox@users.noreply.github.com>
This commit is contained in:
Teknium
2026-06-03 17:49:39 -07:00
committed by GitHub
parent 5a22cd427d
commit b0a52d74ac
3 changed files with 136 additions and 0 deletions

View File

@ -109,6 +109,21 @@ def _env_key_for_server(name: str) -> str:
return f"MCP_{name.upper().replace('-', '_')}_API_KEY"
def _strip_bearer_prefix(token: str) -> str:
"""Strip a leading ``Bearer `` from a pasted token.
The header template stores ``Authorization: Bearer ${MCP_X_API_KEY}``, so
if a user pastes a token that already includes the ``Bearer `` prefix the
server receives ``Bearer Bearer <jwt>`` → 401. Normalize on save. (#37792)
"""
if not isinstance(token, str):
return token
stripped = token.strip()
if stripped[:7].lower() == "bearer ":
return stripped[7:].strip()
return stripped
def _parse_env_assignments(raw_env: Optional[List[str]]) -> Dict[str, str]:
"""Parse ``KEY=VALUE`` strings from CLI args into an env dict."""
parsed: Dict[str, str] = {}
@ -164,6 +179,27 @@ def _apply_mcp_preset(
# ─── Discovery (temporary connect) ───────────────────────────────────────────
def _resolve_mcp_server_config(config: dict) -> dict:
"""Resolve ``${ENV}`` placeholders in a server config before connecting.
Mirrors ``_load_mcp_config()`` in ``tools/mcp_tool.py``: load
``~/.hermes/.env`` into ``os.environ`` and recursively interpolate any
``${VAR}`` placeholders. The CLI builds header templates like
``Authorization: Bearer ${MCP_X_API_KEY}`` but the probe path never
resolved them, so the discovery probe sent the literal placeholder and
auth-requiring servers (e.g. n8n) returned 401 — while runtime tool
loading worked because it interpolates. (#37792)
"""
from tools.mcp_tool import _interpolate_env_vars
try:
from hermes_cli.env_loader import load_hermes_dotenv
load_hermes_dotenv()
except Exception: # pragma: no cover — defensive
pass
return _interpolate_env_vars(config)
def _probe_single_server(
name: str, config: dict, connect_timeout: float = 30
) -> List[Tuple[str, str]]:
@ -179,6 +215,8 @@ def _probe_single_server(
_stop_mcp_loop,
)
config = _resolve_mcp_server_config(config)
_ensure_mcp_loop()
tools_found: List[Tuple[str, str]] = []
@ -340,6 +378,7 @@ def cmd_mcp_add(args):
else:
api_key = _prompt("API key / Bearer token", password=True)
if api_key:
api_key = _strip_bearer_prefix(api_key)
save_env_value(env_key, api_key)
_success(f"Saved to {display_hermes_home()}/.env as {env_key}")

View File

@ -46,6 +46,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json"
# Auto-extracted from noreply emails + manual overrides
AUTHOR_MAP = {
"prostoandrei9@gmail.com": "vladkvlchk",
"116314616+ThyFriendlyFox@users.noreply.github.com": "ThyFriendlyFox",
"liliangjya@gmail.com": "truenorth-lj",
"16943149+nepenth@users.noreply.github.com": "nepenth",
"ben.bartholomew@vectorize.io": "benfrank241",

View File

@ -483,6 +483,102 @@ class TestEnvVarInterpolation:
assert _interpolate_env_vars(None) is None
# ---------------------------------------------------------------------------
# Tests: probe-path env resolution (#37792)
# ---------------------------------------------------------------------------
class TestProbeEnvResolution:
"""The probe path must resolve ``${ENV}`` before connecting, so the
discovery probe behaves like runtime tool loading. Regression for #37792
where `hermes mcp add --auth header` sent a literal
``Authorization: Bearer ${MCP_X_API_KEY}`` and got 401."""
def test_resolve_interpolates_header(self, monkeypatch):
from hermes_cli.mcp_config import _resolve_mcp_server_config
monkeypatch.setenv("MCP_N8N_API_KEY", "jwt-token-xyz")
resolved = _resolve_mcp_server_config({
"url": "http://localhost:5678/mcp-server/http",
"headers": {"Authorization": "Bearer ${MCP_N8N_API_KEY}"},
})
assert resolved["headers"]["Authorization"] == "Bearer jwt-token-xyz"
def test_resolve_leaves_unset_var_literal(self, monkeypatch):
from hermes_cli.mcp_config import _resolve_mcp_server_config
monkeypatch.delenv("MCP_UNSET_API_KEY", raising=False)
resolved = _resolve_mcp_server_config({
"headers": {"Authorization": "Bearer ${MCP_UNSET_API_KEY}"},
})
# Unresolved placeholder stays literal (no crash) — matches
# _interpolate_env_vars semantics.
assert resolved["headers"]["Authorization"] == "Bearer ${MCP_UNSET_API_KEY}"
def test_probe_resolves_before_connect(self, monkeypatch):
"""_probe_single_server must pass the RESOLVED config to _connect_server."""
import hermes_cli.mcp_config as mc
monkeypatch.setenv("MCP_N8N_API_KEY", "jwt-token-xyz")
seen = {}
class _FakeTool:
name = "do_thing"
description = "a tool"
class _FakeServer:
_tools = [_FakeTool()]
async def shutdown(self):
return None
async def _fake_connect(name, config):
seen["config"] = config
return _FakeServer()
monkeypatch.setattr("tools.mcp_tool._connect_server", _fake_connect)
tools = mc._probe_single_server("n8n", {
"url": "http://localhost:5678/mcp-server/http",
"headers": {"Authorization": "Bearer ${MCP_N8N_API_KEY}"},
})
assert tools == [("do_thing", "a tool")]
assert seen["config"]["headers"]["Authorization"] == "Bearer jwt-token-xyz"
class TestStripBearerPrefix:
"""Pasted tokens that already include ``Bearer `` would otherwise produce
``Bearer Bearer <jwt>`` once the header template adds its own prefix."""
def test_bare_token_unchanged(self):
from hermes_cli.mcp_config import _strip_bearer_prefix
assert _strip_bearer_prefix("eyJabc123") == "eyJabc123"
def test_strips_bearer_prefix(self):
from hermes_cli.mcp_config import _strip_bearer_prefix
assert _strip_bearer_prefix("Bearer eyJabc123") == "eyJabc123"
def test_strips_case_insensitive_and_whitespace(self):
from hermes_cli.mcp_config import _strip_bearer_prefix
assert _strip_bearer_prefix("bearer eyJabc123") == "eyJabc123"
assert _strip_bearer_prefix(" Bearer eyJabc123 ") == "eyJabc123"
def test_does_not_strip_without_space(self):
from hermes_cli.mcp_config import _strip_bearer_prefix
# "BearerToken" is a token that happens to start with "Bearer", not a prefix.
assert _strip_bearer_prefix("BearerToken") == "BearerToken"
def test_non_string_passthrough(self):
from hermes_cli.mcp_config import _strip_bearer_prefix
assert _strip_bearer_prefix(None) is None # type: ignore[arg-type]
# ---------------------------------------------------------------------------
# Tests: config helpers
# ---------------------------------------------------------------------------