diff --git a/hermes_cli/mcp_config.py b/hermes_cli/mcp_config.py index 304cfcc61b23..76dcc7e7e9c1 100644 --- a/hermes_cli/mcp_config.py +++ b/hermes_cli/mcp_config.py @@ -171,6 +171,31 @@ def _strip_bearer_prefix(token: str) -> str: return stripped +def _bearer_auth_headers(name: str) -> Dict[str, str]: + """Build the persisted Authorization header for a named MCP server. + + The secret itself lives in the active profile's ``.env`` file. Keeping + this template construction beside ``_env_key_for_server`` ensures the CLI + and Dashboard produce byte-equivalent MCP configuration. + """ + env_key = _env_key_for_server(name) + return {"Authorization": f"Bearer ${{{env_key}}}"} + + +def _save_bearer_auth_token(name: str, token: str) -> Dict[str, str]: + """Persist a Bearer token in the active profile and return safe headers. + + ``token`` is a one-time provisioning value. It is normalized and written + only to ``.env``; callers persist the returned interpolation template in + ``config.yaml``. + """ + normalized = _strip_bearer_prefix(token) + if not normalized or normalized.lower() == "bearer": + raise ValueError("Bearer token is required") + save_env_value(_env_key_for_server(name), normalized) + return _bearer_auth_headers(name) + + 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] = {} @@ -492,19 +517,17 @@ def cmd_mcp_add(args): existing_key = get_env_value(env_key) if existing_key: _success(f"{env_key}: already configured") - api_key = existing_key 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) + server_config["headers"] = _save_bearer_auth_token( + name, api_key + ) _success(f"Saved to {display_hermes_home()}/.env as {env_key}") # Set header with env var interpolation - if api_key or existing_key: - server_config["headers"] = { - "Authorization": f"Bearer ${{{env_key}}}" - } + if existing_key: + server_config["headers"] = _bearer_auth_headers(name) # ── Discovery: connect and list tools ───────────────────────────── diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 6c4b16982a8b..cb818f54f8c3 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -99,7 +99,7 @@ try: from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, HTMLResponse, JSONResponse, Response from fastapi.staticfiles import StaticFiles - from pydantic import BaseModel + from pydantic import BaseModel, SecretStr from starlette.concurrency import run_in_threadpool except ImportError: # First try lazy-installing the dashboard extras. Only the user actually @@ -115,7 +115,7 @@ except ImportError: from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, HTMLResponse, JSONResponse, Response from fastapi.staticfiles import StaticFiles - from pydantic import BaseModel + from pydantic import BaseModel, SecretStr from starlette.concurrency import run_in_threadpool except Exception: raise SystemExit( @@ -10943,8 +10943,10 @@ class MCPServerCreate(BaseModel): args: List[str] = [] # env: KEY=VALUE map for stdio servers (API keys, etc.) env: Dict[str, str] = {} - # auth: "oauth" | "header" | None + # auth: "none" | "oauth" | "header" | None auth: Optional[str] = None + # One-time provisioning input; persisted only to the profile's .env. + bearer_token: Optional[SecretStr] = None profile: Optional[str] = None @@ -10967,6 +10969,12 @@ def _redact_mcp_env(env: Dict[str, Any]) -> Dict[str, str]: def _mcp_server_summary(name: str, cfg: Dict[str, Any]) -> Dict[str, Any]: transport = "http" if cfg.get("url") else ("stdio" if cfg.get("command") else "unknown") + auth = cfg.get("auth") + headers = cfg.get("headers") or {} + if not auth and isinstance(headers, dict) and any( + str(key).lower() == "authorization" for key in headers + ): + auth = "header" return { "name": name, "transport": transport, @@ -10974,7 +10982,7 @@ def _mcp_server_summary(name: str, cfg: Dict[str, Any]) -> Dict[str, Any]: "command": cfg.get("command"), "args": list(cfg.get("args") or []), "env": _redact_mcp_env(cfg.get("env") or {}), - "auth": cfg.get("auth"), + "auth": auth, "enabled": cfg.get("enabled", True) is not False, # Tool selection: list of enabled tool names, or None = all. "tools": cfg.get("tools"), @@ -10996,35 +11004,78 @@ async def list_mcp_servers(profile: Optional[str] = None): @app.post("/api/mcp/servers") async def add_mcp_server(body: MCPServerCreate, profile: Optional[str] = None): - from hermes_cli.mcp_config import _get_mcp_servers, _save_mcp_server + from hermes_cli.mcp_config import ( + _get_mcp_servers, + _save_bearer_auth_token, + _save_mcp_server, + ) name = (body.name or "").strip() if not name: raise HTTPException(status_code=400, detail="Server name is required") + + url = (body.url or "").strip() + command = (body.command or "").strip() + auth = (body.auth or "none").strip().lower() + bearer_token = ( + body.bearer_token.get_secret_value() + if body.bearer_token is not None + else None + ) + + if bool(url) == bool(command): + raise HTTPException( + status_code=400, + detail="Provide exactly one of URL (HTTP/SSE) or command (stdio)", + ) + if auth not in {"none", "header", "oauth"}: + raise HTTPException(status_code=400, detail=f"Unsupported auth mode: {auth}") + + if url: + if body.args: + raise HTTPException( + status_code=400, + detail="Arguments are only supported for stdio MCP servers", + ) + if body.env: + raise HTTPException( + status_code=400, + detail="Environment variables are only supported for stdio MCP servers", + ) + if auth == "header" and bearer_token is None: + raise HTTPException(status_code=400, detail="Bearer token is required") + if auth != "header" and body.bearer_token is not None: + raise HTTPException( + status_code=400, + detail="Bearer token requires header authentication", + ) + else: + if auth != "none" or body.bearer_token is not None: + raise HTTPException( + status_code=400, + detail="HTTP authentication is not supported for stdio MCP servers", + ) + with _profile_scope(body.profile or profile): existing = _get_mcp_servers() if name in existing: raise HTTPException(status_code=409, detail=f"Server '{name}' already exists") - if not body.url and not body.command: - raise HTTPException( - status_code=400, - detail="Provide either a URL (HTTP/SSE server) or a command (stdio server)", - ) - server_config: Dict[str, Any] = {} - if body.url: - server_config["url"] = body.url.strip() - if body.command: - server_config["command"] = body.command.strip() + if url: + server_config["url"] = url + if auth == "oauth": + server_config["auth"] = "oauth" + else: + server_config["command"] = command if body.args: server_config["args"] = list(body.args) - if body.env: - server_config["env"] = dict(body.env) - if body.auth: - server_config["auth"] = body.auth + if body.env: + server_config["env"] = dict(body.env) try: with _profile_scope(body.profile or profile): + if auth == "header" and bearer_token is not None: + server_config["headers"] = _save_bearer_auth_token(name, bearer_token) if not _save_mcp_server(name, server_config): raise HTTPException( status_code=400, diff --git a/tests/hermes_cli/test_dashboard_admin_endpoints.py b/tests/hermes_cli/test_dashboard_admin_endpoints.py index 14744a217d0d..049ed460880a 100644 --- a/tests/hermes_cli/test_dashboard_admin_endpoints.py +++ b/tests/hermes_cli/test_dashboard_admin_endpoints.py @@ -65,6 +65,98 @@ class TestMcpEndpoints: srv = self.client.get("/api/mcp/servers").json()["servers"][0] assert srv["env"]["API_KEY"] != "sk-secret-1234567890" + def test_http_bearer_auth_separates_secret_from_config( + self, _isolate_hermes_home + ): + from hermes_constants import get_hermes_home + + secret = "dashboard-secret-value" + response = self.client.post( + "/api/mcp/servers", + json={ + "name": "Bearer Server", + "url": "https://example.com/mcp", + "auth": "header", + "bearer_token": f"Bearer {secret}", + }, + ) + + assert response.status_code == 200 + assert response.json()["auth"] == "header" + assert "bearer_token" not in response.json() + + hermes_home = get_hermes_home() + config_text = (hermes_home / "config.yaml").read_text() + env_text = (hermes_home / ".env").read_text() + assert secret not in config_text + assert "Bearer ${MCP_BEARER_SERVER_API_KEY}" in config_text + assert f"MCP_BEARER_SERVER_API_KEY={secret}" in env_text + + def test_http_oauth_mode_is_persisted_for_existing_auth_flow(self): + response = self.client.post( + "/api/mcp/servers", + json={ + "name": "oauth-server", + "url": "https://example.com/mcp", + "auth": "oauth", + }, + ) + + assert response.status_code == 200 + assert response.json()["auth"] == "oauth" + + from hermes_cli.mcp_config import _get_mcp_servers + + assert _get_mcp_servers()["oauth-server"]["auth"] == "oauth" + + @pytest.mark.parametrize( + ("payload", "error"), + [ + ( + {"name": "bad", "url": "https://x/mcp", "env": {"KEY": "value"}}, + "only supported for stdio", + ), + ( + {"name": "bad", "url": "https://x/mcp", "args": ["ignored"]}, + "only supported for stdio", + ), + ( + {"name": "bad", "command": "npx", "auth": "oauth"}, + "not supported for stdio", + ), + ( + {"name": "bad", "url": "https://x/mcp", "auth": "header"}, + "Bearer token is required", + ), + ( + { + "name": "bad", + "url": "https://x/mcp", + "auth": "header", + "bearer_token": "Bearer ", + }, + "Bearer token is required", + ), + ( + {"name": "bad", "url": "https://x/mcp", "bearer_token": "secret"}, + "requires header authentication", + ), + ( + {"name": "bad", "url": "https://x/mcp", "command": "npx"}, + "exactly one", + ), + ( + {"name": "bad", "url": "https://x/mcp", "auth": "unknown"}, + "Unsupported auth mode", + ), + ], + ) + def test_transport_auth_contract_is_enforced(self, payload, error): + response = self.client.post("/api/mcp/servers", json=payload) + + assert response.status_code == 400 + assert error in response.json()["detail"] + def test_duplicate_rejected(self): self.client.post("/api/mcp/servers", json={"name": "dup", "url": "u"}) r = self.client.post("/api/mcp/servers", json={"name": "dup", "url": "u"}) diff --git a/tests/hermes_cli/test_mcp_config.py b/tests/hermes_cli/test_mcp_config.py index e88f04960406..64e24c42470f 100644 --- a/tests/hermes_cli/test_mcp_config.py +++ b/tests/hermes_cli/test_mcp_config.py @@ -739,6 +739,25 @@ class TestStripBearerPrefix: assert _strip_bearer_prefix(None) is None # type: ignore[arg-type] +class TestBearerAuthPersistence: + def test_secret_and_header_are_persisted_separately(self): + from hermes_cli.config import get_env_value + from hermes_cli.mcp_config import _save_bearer_auth_token + + headers = _save_bearer_auth_token("My Server", "Bearer secret-value") + + assert headers == { + "Authorization": "Bearer ${MCP_MY_SERVER_API_KEY}", + } + assert get_env_value("MCP_MY_SERVER_API_KEY") == "secret-value" + + def test_empty_token_is_rejected(self): + from hermes_cli.mcp_config import _save_bearer_auth_token + + with pytest.raises(ValueError, match="Bearer token is required"): + _save_bearer_auth_token("empty", "Bearer ") + + # --------------------------------------------------------------------------- # Tests: config helpers # --------------------------------------------------------------------------- diff --git a/tests/hermes_cli/test_web_server_profile_unification.py b/tests/hermes_cli/test_web_server_profile_unification.py index 260892000411..14141a815362 100644 --- a/tests/hermes_cli/test_web_server_profile_unification.py +++ b/tests/hermes_cli/test_web_server_profile_unification.py @@ -161,6 +161,30 @@ class TestProfileScopedMcp: listing = client.get("/api/mcp/servers").json() assert not any(s["name"] == "scoped-srv" for s in listing["servers"]) + def test_mcp_bearer_secret_is_profile_scoped(self, client, isolated_profiles): + secret = "worker-only-secret" + response = client.post( + "/api/mcp/servers", + params={"profile": "worker_beta"}, + json={ + "name": "profile-bearer", + "url": "https://example.com/mcp", + "auth": "header", + "bearer_token": secret, + }, + ) + + assert response.status_code == 200 + worker_cfg = _cfg(isolated_profiles["worker_beta"]) + assert worker_cfg["mcp_servers"]["profile-bearer"]["headers"] == { + "Authorization": "Bearer ${MCP_PROFILE_BEARER_API_KEY}", + } + assert secret in (isolated_profiles["worker_beta"] / ".env").read_text() + assert not (isolated_profiles["default"] / ".env").exists() + assert "profile-bearer" not in _cfg(isolated_profiles["default"]).get( + "mcp_servers", {} + ) + def test_mcp_enabled_toggle_scoped(self, client, isolated_profiles): (isolated_profiles["worker_beta"] / "config.yaml").write_text( "mcp_servers:\n srv1:\n url: http://x/sse\n", encoding="utf-8" diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 2d1521454bfd..d65a50153909 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -995,6 +995,11 @@ export const api = { headers: { "Content-Type": "application/json" }, body: JSON.stringify(body), }), + authMcpServer: (name: string) => + fetchJSON( + `/api/mcp/servers/${encodeURIComponent(name)}/auth`, + { method: "POST" }, + ), removeMcpServer: (name: string) => fetchJSON<{ ok: boolean }>(`/api/mcp/servers/${encodeURIComponent(name)}`, { method: "DELETE", @@ -1407,7 +1412,7 @@ export interface McpServer { command: string | null; args: string[]; env: Record; - auth: string | null; + auth: "header" | "oauth" | null; enabled: boolean; tools: string[] | null; } @@ -1442,13 +1447,16 @@ export interface McpCatalogDiagnostic { } +export type McpHttpAuth = "none" | "header" | "oauth"; + export interface McpServerCreate { name: string; url?: string; command?: string; args?: string[]; env?: Record; - auth?: string; + auth?: McpHttpAuth; + bearer_token?: string; } export interface McpTestResult { diff --git a/web/src/pages/McpPage.tsx b/web/src/pages/McpPage.tsx index 0f808211bca0..cc34e00a8be1 100644 --- a/web/src/pages/McpPage.tsx +++ b/web/src/pages/McpPage.tsx @@ -1,5 +1,5 @@ import { useCallback, useEffect, useLayoutEffect, useState } from "react"; -import { Package, Power, Server, Trash2, X, Zap } from "lucide-react"; +import { KeyRound, Package, Power, Server, Trash2, X, Zap } from "lucide-react"; import { Badge } from "@nous-research/ui/ui/components/badge"; import { Button } from "@nous-research/ui/ui/components/button"; import { Select, SelectOption } from "@nous-research/ui/ui/components/select"; @@ -9,6 +9,7 @@ import { api } from "@/lib/api"; import type { McpCatalogDiagnostic, McpCatalogEntry, + McpHttpAuth, McpServer, McpServerCreate, McpTestResult, @@ -76,11 +77,16 @@ export default function McpPage() { const [name, setName] = useState(""); const [transport, setTransport] = useState("http"); const [url, setUrl] = useState(""); + const [httpAuth, setHttpAuth] = useState("none"); + const [bearerToken, setBearerToken] = useState(""); const [command, setCommand] = useState(""); const [args, setArgs] = useState(""); const [env, setEnv] = useState(""); const [creating, setCreating] = useState(false); - const closeCreateModal = useCallback(() => setCreateModalOpen(false), []); + const closeCreateModal = useCallback(() => { + setBearerToken(""); + setCreateModalOpen(false); + }, []); const createModalRef = useModalBehavior({ open: createModalOpen, onClose: closeCreateModal, @@ -88,6 +94,7 @@ export default function McpPage() { // Test results keyed by server name const [testing, setTesting] = useState(null); + const [authenticating, setAuthenticating] = useState(null); const [testResults, setTestResults] = useState< Record >({}); @@ -140,6 +147,10 @@ export default function McpPage() { showToast("URL required", "error"); return; } + if (transport === "http" && httpAuth === "header" && !bearerToken.trim()) { + showToast("Bearer token required", "error"); + return; + } if (transport === "stdio" && !command.trim()) { showToast("Command required", "error"); return; @@ -149,18 +160,27 @@ export default function McpPage() { const body: McpServerCreate = { name: name.trim() }; if (transport === "http") { body.url = url.trim(); + if (httpAuth !== "none") body.auth = httpAuth; + if (httpAuth === "header") body.bearer_token = bearerToken; } else { body.command = command.trim(); const argList = parseArgs(args); if (argList.length) body.args = argList; + const envMap = parseEnv(env); + if (Object.keys(envMap).length) body.env = envMap; } - const envMap = parseEnv(env); - if (Object.keys(envMap).length) body.env = envMap; await api.addMcpServer(body); - showToast("Add ✓", "success"); + showToast( + transport === "http" && httpAuth === "oauth" + ? "Added — authenticate with OAuth" + : "Add ✓", + "success", + ); setName(""); setUrl(""); + setHttpAuth("none"); + setBearerToken(""); setCommand(""); setArgs(""); setEnv(""); @@ -191,6 +211,23 @@ export default function McpPage() { } }; + const handleAuthenticate = async (server: McpServer) => { + setAuthenticating(server.name); + try { + const result = await api.authMcpServer(server.name); + setTestResults((prev) => ({ ...prev, [server.name]: result })); + if (result.ok) { + showToast(`${server.name}: OAuth authentication complete`, "success"); + } else { + showToast(`${server.name}: ${result.error ?? "OAuth failed"}`, "error"); + } + } catch (e) { + showToast(`OAuth error: ${e}`, "error"); + } finally { + setAuthenticating(null); + } + }; + const handleToggleEnabled = async (server: McpServer) => { const next = !server.enabled; setTogglingName(server.name); @@ -335,9 +372,7 @@ export default function McpPage() {
- e.target === e.currentTarget && setCreateModalOpen(false) - } + onClick={(e) => e.target === e.currentTarget && closeCreateModal()} role="dialog" aria-modal="true" aria-labelledby="create-mcp-title" @@ -351,7 +386,7 @@ export default function McpPage() {
+ <> +
+ + setUrl(e.target.value)} + /> +
+
+ + +
+ {httpAuth === "header" && ( +
+ + setBearerToken(e.target.value)} + /> +

+ Stored in this profile's .env; config.yaml keeps only + an environment-variable reference. +

+
+ )} + {httpAuth === "oauth" && ( +

+ Add the server, then use Authenticate. Hermes opens the + OAuth browser on the machine running the Dashboard backend. +

+ )} + ) : ( <>
@@ -421,20 +501,21 @@ export default function McpPage() { onChange={(e) => setArgs(e.target.value)} />
+
+ +