diff --git a/hermes_cli/mcp_config.py b/hermes_cli/mcp_config.py index 378de219a..377c6b20b 100644 --- a/hermes_cli/mcp_config.py +++ b/hermes_cli/mcp_config.py @@ -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 `` → 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}") diff --git a/scripts/release.py b/scripts/release.py index 648a16c8c..35d54ad51 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -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", diff --git a/tests/hermes_cli/test_mcp_config.py b/tests/hermes_cli/test_mcp_config.py index d52d3ed0e..581724117 100644 --- a/tests/hermes_cli/test_mcp_config.py +++ b/tests/hermes_cli/test_mcp_config.py @@ -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 `` 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 # ---------------------------------------------------------------------------