diff --git a/hermes_cli/web_routers/mcp.py b/hermes_cli/web_routers/mcp.py index 9a3bf37029..8537496b0c 100644 --- a/hermes_cli/web_routers/mcp.py +++ b/hermes_cli/web_routers/mcp.py @@ -7,6 +7,8 @@ web_server — reached via the late-binding seam so tests that mutate import asyncio import hashlib +from contextlib import contextmanager +from pathlib import Path import re import secrets import threading @@ -38,6 +40,37 @@ _MCP_DASHBOARD_OAUTH_TTL = 15 * 60 _MAX_PENDING_MCP_OAUTH_FLOWS = 8 +@contextmanager +def _profile_secret_scope(profile: Optional[str]): + """Home + secret scope for a probe-class request: config.yaml's ``${VAR}`` expansion + (``config._env_ref_lookup``) and the probe's own interpolation read plain ``os.environ`` + while no scope is installed — the dashboard process's env, i.e. the DEFAULT profile's + values — so a secondary profile whose credential lives only in Bitwarden/1Password sent + the literal placeholder or the default's token (#109901). Same wrapping as the OAuth + worker (``_run_dashboard_mcp_oauth``). Home-only ``_config_profile_scope``, NOT + ``_profile_scope``: the body can block for seconds and the latter holds the process-global + skills lock. A scope miss still falls through to ``os.environ`` outside multiplexing.""" + from agent.secret_scope import build_profile_secret_scope, reset_secret_scope, set_secret_scope + from hermes_cli.env_loader import hydrate_profile_secret_sources + from hermes_constants import get_hermes_home + + with _config_profile_scope(profile): + home = Path(get_hermes_home()) + hydrate_profile_secret_sources(home) # first call may block on the source's fetch + token = set_secret_scope(build_profile_secret_scope(home)) + try: + yield + finally: + reset_secret_scope(token) + + +def _secret_scoped(profile: Optional[str], fn): + def _run(): + with _profile_secret_scope(profile): + return fn() + return _run + + def _gc_mcp_oauth_flows() -> None: cutoff = time.time() - _MCP_DASHBOARD_OAUTH_TTL with _mcp_oauth_flows_lock: @@ -77,7 +110,8 @@ def _mcp_install_action_name(name: str) -> str: async def list_mcp_servers(profile: Optional[str] = None): from hermes_cli.mcp_config import _get_mcp_servers - servers = await scoped_to_thread(profile, _get_mcp_servers) + # ``url`` may carry a ``${VAR}`` ref — expand it against the requested profile, not this process. + servers = await asyncio.to_thread(_secret_scoped(profile, _get_mcp_servers)) return {"servers": [_mcp_server_summary(name, cfg) for name, cfg in sorted(servers.items())]} @@ -149,39 +183,7 @@ 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 - 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)) + servers = await asyncio.to_thread(_secret_scoped(profile, _get_mcp_servers)) if name not in servers: raise HTTPException(status_code=404, detail=f"Server '{name}' not found") @@ -195,7 +197,7 @@ async def test_mcp_server(name: str, profile: Optional[str] = None): 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(_secret_scoped(_probe)) + tools, token_present = await asyncio.to_thread(_secret_scoped(profile, _probe)) except Exception as exc: from hermes_cli.mcp_config import redact_mcp_probe_text @@ -236,7 +238,7 @@ async def auth_mcp_server(name: str, request: Request, profile: Optional[str] = process_home = _home() def _read(): - with _profile_scope(profile): + with _profile_secret_scope(profile): return _get_mcp_servers(), _home() servers, flow_home = await asyncio.to_thread(_read) diff --git a/tests/hermes_cli/test_web_server_profile_unification.py b/tests/hermes_cli/test_web_server_profile_unification.py index 1d20060b93..cfb57f2049 100644 --- a/tests/hermes_cli/test_web_server_profile_unification.py +++ b/tests/hermes_cli/test_web_server_profile_unification.py @@ -7,7 +7,6 @@ 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 @@ -249,7 +248,7 @@ class TestProfileScopedMcp: ) # 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" + monkeypatch.setenv("GITHUB_PERSONAL_ACCESS_TOKEN", "default-profile-token") def _worker_sources(hermes_home): if Path(hermes_home).resolve() == worker_home.resolve(): @@ -267,17 +266,34 @@ class TestProfileScopedMcp: 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) + resp = client.post("/api/mcp/servers/bw-srv/test", params={"profile": "worker_beta"}) assert resp.status_code == 200 assert resp.json()["ok"] is True assert resolved_headers["Authorization"] == "Bearer bw-worker-token" + def test_mcp_list_expands_url_ref_from_profile_secret_scope( + self, client, isolated_profiles, monkeypatch + ): + """Same class for the read endpoint: a ``${VAR}`` in a secondary profile's server + ``url`` must expand from THAT profile's secret scope, never the dashboard process env.""" + import hermes_cli.env_loader as env_loader + + worker_home = isolated_profiles["worker_beta"] + (worker_home / "config.yaml").write_text( + "mcp_servers:\n bw-srv:\n url: ${MCP_GH_URL}\n", encoding="utf-8" + ) + monkeypatch.setenv("MCP_GH_URL", "http://default-profile/mcp") + monkeypatch.setattr( + env_loader, "get_secret_source_values", + lambda hermes_home: {"MCP_GH_URL": "http://worker/mcp"} + if Path(hermes_home).resolve() == worker_home.resolve() else {}, + ) + + resp = client.get("/api/mcp/servers", params={"profile": "worker_beta"}) + assert resp.status_code == 200 + assert [s["url"] for s in resp.json()["servers"]] == ["http://worker/mcp"] + class TestProfileScopedModel: @pytest.fixture(autouse=True)