diff --git a/hermes_cli/web_routers/mcp.py b/hermes_cli/web_routers/mcp.py index 0c9c6134eb..9a3bf37029 100644 --- a/hermes_cli/web_routers/mcp.py +++ b/hermes_cli/web_routers/mcp.py @@ -149,7 +149,39 @@ async def test_mcp_server(name: str, profile: Optional[str] = None): """Connect to the server, list its tools, disconnect.""" from hermes_cli.mcp_config import _get_mcp_servers, _oauth_tokens_present, _probe_single_server - servers = await scoped_to_thread(profile, _get_mcp_servers) + def _secret_scoped(fn): + # Home + secret scope for BOTH the config read and the probe: config.yaml's + # `${VAR}` expansion (config._env_ref_lookup) and the probe's own + # interpolation resolve against plain os.environ while no scope is + # installed — the dashboard process's own environment, i.e. the DEFAULT + # profile's values (or nothing at all) on a shared remote dashboard. A + # secondary profile whose credential comes only from an external secret + # source (Bitwarden/1Password) then never resolves and the probe sends the + # literal placeholder (#109901). Home-only scope (contextvar), NOT + # _profile_scope: both stages can block for seconds and _profile_scope + # holds the process-global skills lock for its whole body, serializing + # every other endpoint. External sources hydrate per-home (once, cached); + # a scope miss still falls back to os.environ outside multiplexing, so + # shell-injected keys keep working. + def _run(): + from pathlib import Path + + from agent.secret_scope import build_profile_secret_scope, reset_secret_scope, set_secret_scope + from hermes_constants import get_hermes_home + from hermes_cli.env_loader import hydrate_profile_secret_sources + + with _config_profile_scope(profile): + home = Path(get_hermes_home()) + hydrate_profile_secret_sources(home) # first call may block on the source's fetch + scope_token = set_secret_scope(build_profile_secret_scope(home)) + try: + return fn() + finally: + reset_secret_scope(scope_token) + + return _run + + servers = await asyncio.to_thread(_secret_scoped(_get_mcp_servers)) if name not in servers: raise HTTPException(status_code=404, detail=f"Server '{name}' not found") @@ -158,17 +190,12 @@ async def test_mcp_server(name: str, profile: Optional[str] = None): # with no token — a false green. Require a token on disk, matching /auth. needs_oauth_token = servers[name].get("auth") == "oauth" - def _probe_scoped(): - # Home-only scope (contextvar), NOT _profile_scope: a probe can block for - # seconds (stdio `npx` cold start) and _profile_scope holds the - # process-global skills lock for its whole body, serializing every other - # endpoint. The probe only needs HERMES_HOME for .env + token resolution. - with _config_profile_scope(profile): - tools = _probe_single_server(name, servers[name], details=details) - return tools, (_oauth_tokens_present(name) if needs_oauth_token else True) + def _probe(): + tools = _probe_single_server(name, servers[name], details=details) + return tools, (_oauth_tokens_present(name) if needs_oauth_token else True) try: # probe blocks on a dedicated MCP event loop — keep it off the FastAPI loop - tools, token_present = await asyncio.to_thread(_probe_scoped) + tools, token_present = await asyncio.to_thread(_secret_scoped(_probe)) except Exception as exc: from hermes_cli.mcp_config import redact_mcp_probe_text diff --git a/tests/hermes_cli/test_web_server_profile_unification.py b/tests/hermes_cli/test_web_server_profile_unification.py index 64562cb10b..1d20060b93 100644 --- a/tests/hermes_cli/test_web_server_profile_unification.py +++ b/tests/hermes_cli/test_web_server_profile_unification.py @@ -7,7 +7,9 @@ reads/writes land in the REQUESTED profile, the dashboard's own profile stays untouched, and the chat PTY env is scoped via HERMES_HOME. """ import json +import os from contextlib import contextmanager +from pathlib import Path import pytest import yaml @@ -227,6 +229,55 @@ class TestProfileScopedMcp: assert resp.status_code == 200 assert resp.json()["tools"] == [{"name": "tool-a", "description": "desc"}] + def test_mcp_test_resolves_profile_secret_source_scope( + self, client, isolated_profiles, monkeypatch + ): + """The probe's `${VAR}` interpolation must resolve from the REQUESTED + profile's secret scope, not the dashboard process's os.environ: a + secondary profile whose credential comes from an external secret source + (Bitwarden/1Password) never has it in the shared process env, so the + probe used to send the literal placeholder — or the default profile's + value of the same name — and the server answered 400 (#109901).""" + import hermes_cli.env_loader as env_loader + import hermes_cli.mcp_config as mcp_config + + worker_home = isolated_profiles["worker_beta"] + (worker_home / "config.yaml").write_text( + "mcp_servers:\n bw-srv:\n url: http://x/mcp\n" + " headers:\n Authorization: Bearer ${GITHUB_PERSONAL_ACCESS_TOKEN}\n", + encoding="utf-8", + ) + # The shared dashboard process carries the DEFAULT profile's value of the + # same env name — the probe must not use it. + os.environ["GITHUB_PERSONAL_ACCESS_TOKEN"] = "default-profile-token" + + def _worker_sources(hermes_home): + if Path(hermes_home).resolve() == worker_home.resolve(): + return {"GITHUB_PERSONAL_ACCESS_TOKEN": "bw-worker-token"} + return {} + + monkeypatch.setattr(env_loader, "get_secret_source_values", _worker_sources) + + resolved_headers = {} + + def fake_probe(name, config, connect_timeout=30, details=None): + resolved = mcp_config._resolve_mcp_server_config(config) + resolved_headers.update(resolved.get("headers", {})) + return [("tool-a", "desc")] + + monkeypatch.setattr(mcp_config, "_probe_single_server", fake_probe) + + try: + resp = client.post( + "/api/mcp/servers/bw-srv/test", params={"profile": "worker_beta"} + ) + finally: + os.environ.pop("GITHUB_PERSONAL_ACCESS_TOKEN", None) + + assert resp.status_code == 200 + assert resp.json()["ok"] is True + assert resolved_headers["Authorization"] == "Bearer bw-worker-token" + class TestProfileScopedModel: @pytest.fixture(autouse=True)