ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 11 Packages Projects Releases Wiki Activity

fix(dashboard): every MCP router site that expands ${VAR} refs runs under the requested profile's secret scope

Browse Source 
Follow-up to the #109930 salvage (#109901). The probe endpoint was the reported
site, but the same class covers every router path that expands a secondary
profile's `${VAR}` refs while only a home override is installed:
`GET /api/mcp/servers` (a `${VAR}` in `url` expanded from this process's env)
and the `/auth` config read, whose expanded entry is handed to the OAuth worker.
Hoist the PR's inline wrapper into one `_profile_secret_scope` context manager
(mirrors `_run_dashboard_mcp_oauth`'s wrapping) and use it at all three sites.
Policy unchanged: scope miss still falls through to os.environ outside
multiplexing; under multiplexing a miss is a miss, never another profile's value.

Tests: the salvaged probe test now uses monkeypatch.setenv (no raw os.environ
mutation); one invariant test for the list endpoint, red on origin/main.
This commit is contained in:
teknium1
2026-09-13 12:38:07 -07:00
committed by Teknium
parent 13e9e32fd8
commit 668f7278de
2 changed files with 62 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hermes_cli/web_routers/mcp.py
+38 -36
View File
@@ -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)
tests/hermes_cli/test_web_server_profile_unification.py
+24 -8
View File
@@ -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)