feat(mcp): first-class MCP tab — catalog, GUI auth/probe/logs, per-tool gating
A Cursor-style MCP manager inside Capabilities, plus the backend it needs. - Server list with brand/favicon avatars + live status dot and a capability summary (N tools, M prompts, K resources); Servers | Catalog views. - Catalog: one-click install of Nous-approved servers with required-env prompts. - GUI OAuth: Authenticate opens the system browser from the TTY-less backend and verifies a token actually lands; header/API-key servers are never pushed down OAuth; a dirty mcp.json can't drop a freshly-persisted auth field. - Full-width mcp.json editor (ecosystem document format) + pinned stdio/agent LogTail; probes cached 5m and keyed by (profile, config) so revisiting never respawns the fleet or shows a stale probe. - Whole-map persistence (PUT /api/mcp/servers) so deletes/toggles actually stick (the generic /api/config deep-merge could not remove keys). - perf: MCP probe/auth no longer hold the global skills lock, so a slow stdio spawn can't stall every other request into a 15s timeout. - per-tool include/exclude gating (lib/mcp-tool-filter) mirroring the CLI loader.
This commit is contained in:
parent
7e6d60aadc
commit
16aa09aca5
12 changed files with 1959 additions and 36 deletions
1481
apps/desktop/src/app/skills/mcp-tab.tsx
Normal file
1481
apps/desktop/src/app/skills/mcp-tab.tsx
Normal file
File diff suppressed because it is too large
Load diff
|
|
@ -73,9 +73,7 @@ describe('Hermes REST session helpers', () => {
|
|||
})
|
||||
|
||||
it('uses a longer timeout for active profile refresh during desktop startup', async () => {
|
||||
api
|
||||
.mockResolvedValueOnce({ current: 'default' })
|
||||
.mockResolvedValueOnce({ profiles: [] })
|
||||
api.mockResolvedValueOnce({ current: 'default' }).mockResolvedValueOnce({ profiles: [] })
|
||||
|
||||
await refreshActiveProfile()
|
||||
|
||||
|
|
|
|||
|
|
@ -22,7 +22,6 @@ import type {
|
|||
LogsResponse,
|
||||
McpCatalogResponse,
|
||||
McpServerSummary,
|
||||
McpServerTestResponse,
|
||||
MemoryProviderConfig,
|
||||
MemoryProviderOAuthStatus,
|
||||
MemoryStatusResponse,
|
||||
|
|
@ -343,6 +342,7 @@ export function getLogs(params: {
|
|||
file?: string
|
||||
level?: string
|
||||
lines?: number
|
||||
search?: string
|
||||
}): Promise<LogsResponse> {
|
||||
const query = new URLSearchParams()
|
||||
|
||||
|
|
@ -362,6 +362,10 @@ export function getLogs(params: {
|
|||
query.set('component', params.component)
|
||||
}
|
||||
|
||||
if (params.search) {
|
||||
query.set('search', params.search)
|
||||
}
|
||||
|
||||
const suffix = query.toString()
|
||||
|
||||
return window.hermesDesktop.api<LogsResponse>({
|
||||
|
|
@ -592,6 +596,49 @@ export function toggleSkill(name: string, enabled: boolean): Promise<{ ok: boole
|
|||
})
|
||||
}
|
||||
|
||||
export interface McpTestResult {
|
||||
ok: boolean
|
||||
error?: string
|
||||
tools: { name: string; description: string }[]
|
||||
/** Capability counts (absent on older backends / failed probes). */
|
||||
prompts?: number
|
||||
resources?: number
|
||||
}
|
||||
|
||||
/** Connect to the server, list its tools, disconnect. Slow (spawns/handshakes
|
||||
* for real) — well past the 15s default fetch timeout. */
|
||||
export function testMcpServer(name: string): Promise<McpTestResult> {
|
||||
return window.hermesDesktop.api<McpTestResult>({
|
||||
...profileScoped(),
|
||||
path: `/api/mcp/servers/${encodeURIComponent(name)}/test`,
|
||||
method: 'POST',
|
||||
timeoutMs: 60_000
|
||||
})
|
||||
}
|
||||
|
||||
/** Replace the whole `mcp_servers` map (the mcp.json editor's save). Unlike
|
||||
* `saveHermesConfig`, this REPLACES rather than deep-merges, so deletes,
|
||||
* re-enables (dropping `enabled: false`), and removed nested fields persist. */
|
||||
export function saveMcpServers(servers: Record<string, Record<string, unknown>>): Promise<{ ok: boolean }> {
|
||||
return window.hermesDesktop.api<{ ok: boolean }>({
|
||||
...profileScoped(),
|
||||
path: '/api/mcp/servers',
|
||||
method: 'PUT',
|
||||
body: { servers }
|
||||
})
|
||||
}
|
||||
|
||||
/** Run the OAuth flow for an HTTP server — opens the system browser and blocks
|
||||
* until the user finishes (or gives up), hence the very generous timeout. */
|
||||
export function authMcpServer(name: string): Promise<McpTestResult> {
|
||||
return window.hermesDesktop.api<McpTestResult>({
|
||||
...profileScoped(),
|
||||
path: `/api/mcp/servers/${encodeURIComponent(name)}/auth`,
|
||||
method: 'POST',
|
||||
timeoutMs: 300_000
|
||||
})
|
||||
}
|
||||
|
||||
export function getToolsets(): Promise<ToolsetInfo[]> {
|
||||
return window.hermesDesktop.api<ToolsetInfo[]>({
|
||||
...profileScoped(),
|
||||
|
|
@ -1033,16 +1080,6 @@ export function listMcpServers(): Promise<{ servers: McpServerSummary[] }> {
|
|||
})
|
||||
}
|
||||
|
||||
export function testMcpServer(name: string): Promise<McpServerTestResponse> {
|
||||
return window.hermesDesktop.api<McpServerTestResponse>({
|
||||
...profileScoped(),
|
||||
path: `/api/mcp/servers/${encodeURIComponent(name)}/test`,
|
||||
method: 'POST',
|
||||
// Connect + list tools can be slow for stdio servers that boot a process.
|
||||
timeoutMs: 60_000
|
||||
})
|
||||
}
|
||||
|
||||
export function setMcpServerEnabled(name: string, enabled: boolean): Promise<{ ok: boolean }> {
|
||||
return window.hermesDesktop.api<{ ok: boolean }>({
|
||||
...profileScoped(),
|
||||
|
|
|
|||
74
apps/desktop/src/lib/mcp-tool-filter.test.ts
Normal file
74
apps/desktop/src/lib/mcp-tool-filter.test.ts
Normal file
|
|
@ -0,0 +1,74 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { countEnabledTools, isToolEnabled, readToolsFilter, toggleToolInServer } from './mcp-tool-filter'
|
||||
|
||||
describe('readToolsFilter', () => {
|
||||
it('returns empty when no tools object', () => {
|
||||
expect(readToolsFilter({ command: 'x' })).toEqual({ exclude: undefined, include: undefined })
|
||||
})
|
||||
|
||||
it('reads include/exclude and ignores non-string entries', () => {
|
||||
expect(readToolsFilter({ tools: { exclude: ['c', 2], include: ['a', 'b', null] } })).toEqual({
|
||||
exclude: ['c'],
|
||||
include: ['a', 'b']
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('isToolEnabled', () => {
|
||||
it('enables everything with no filter', () => {
|
||||
expect(isToolEnabled({ command: 'x' }, 'anything')).toBe(true)
|
||||
})
|
||||
|
||||
it('include wins over exclude', () => {
|
||||
const server = { tools: { exclude: ['a'], include: ['a'] } }
|
||||
expect(isToolEnabled(server, 'a')).toBe(true)
|
||||
expect(isToolEnabled(server, 'b')).toBe(false)
|
||||
})
|
||||
|
||||
it('exclude disables listed tools', () => {
|
||||
const server = { tools: { exclude: ['b'] } }
|
||||
expect(isToolEnabled(server, 'a')).toBe(true)
|
||||
expect(isToolEnabled(server, 'b')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('toggleToolInServer', () => {
|
||||
it('adds a fresh tool to a new exclude denylist when disabled', () => {
|
||||
const next = toggleToolInServer({ command: 'x' }, 'a')
|
||||
expect(next.tools).toEqual({ exclude: ['a'] })
|
||||
})
|
||||
|
||||
it('re-enabling removes the tool and drops the empty exclude/tools', () => {
|
||||
const next = toggleToolInServer({ command: 'x', tools: { exclude: ['a'] } }, 'a')
|
||||
expect(next.tools).toBeUndefined()
|
||||
})
|
||||
|
||||
it('respects include mode: toggling removes from include', () => {
|
||||
const next = toggleToolInServer({ tools: { include: ['a', 'b'] } }, 'a')
|
||||
expect(next.tools).toEqual({ include: ['b'] })
|
||||
})
|
||||
|
||||
it('respects include mode: re-enabling adds back to include', () => {
|
||||
const next = toggleToolInServer({ tools: { include: ['b'] } }, 'a')
|
||||
expect(next.tools).toEqual({ include: ['b', 'a'] })
|
||||
})
|
||||
|
||||
it('preserves sibling tools keys like resources/prompts', () => {
|
||||
const next = toggleToolInServer({ tools: { resources: false } }, 'a')
|
||||
expect(next.tools).toEqual({ exclude: ['a'], resources: false })
|
||||
})
|
||||
|
||||
it('does not mutate the input server', () => {
|
||||
const server = { tools: { exclude: ['a'] } }
|
||||
toggleToolInServer(server, 'b')
|
||||
expect(server.tools.exclude).toEqual(['a'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('countEnabledTools', () => {
|
||||
it('counts enabled tools out of a discovered list', () => {
|
||||
const server = { tools: { exclude: ['b'] } }
|
||||
expect(countEnabledTools(server, ['a', 'b', 'c'])).toBe(2)
|
||||
})
|
||||
})
|
||||
61
apps/desktop/src/lib/mcp-tool-filter.ts
Normal file
61
apps/desktop/src/lib/mcp-tool-filter.ts
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
// Per-tool MCP gating. A server's optional `tools.include` (whitelist) /
|
||||
// `tools.exclude` (denylist) decide which discovered tools the agent registers
|
||||
// — `include` wins, no filter means all. Mirrors `_register_server_tools` in
|
||||
// `tools/mcp_tool.py`.
|
||||
|
||||
export interface McpToolsFilter {
|
||||
exclude?: string[]
|
||||
include?: string[]
|
||||
}
|
||||
|
||||
type ServerConfig = Record<string, unknown>
|
||||
|
||||
const asNames = (value: unknown): string[] | undefined =>
|
||||
Array.isArray(value) ? value.filter((v): v is string => typeof v === 'string') : undefined
|
||||
|
||||
const toolsObject = (server: ServerConfig | null | undefined): Record<string, unknown> => {
|
||||
const tools = server?.tools
|
||||
|
||||
return tools && typeof tools === 'object' && !Array.isArray(tools) ? (tools as Record<string, unknown>) : {}
|
||||
}
|
||||
|
||||
export function readToolsFilter(server: ServerConfig | null | undefined): McpToolsFilter {
|
||||
const tools = toolsObject(server)
|
||||
|
||||
return { exclude: asNames(tools.exclude), include: asNames(tools.include) }
|
||||
}
|
||||
|
||||
export function isToolEnabled(server: ServerConfig | null | undefined, name: string): boolean {
|
||||
const { exclude, include } = readToolsFilter(server)
|
||||
|
||||
return include?.length ? include.includes(name) : !exclude?.includes(name)
|
||||
}
|
||||
|
||||
// Toggle one tool, preserving the config's mode (include if present, else an
|
||||
// exclude denylist). Empty lists — and an emptied `tools` — are dropped.
|
||||
export function toggleToolInServer(server: ServerConfig, name: string): ServerConfig {
|
||||
const { exclude, include } = readToolsFilter(server)
|
||||
const key = include?.length ? 'include' : 'exclude'
|
||||
const current = (key === 'include' ? include : exclude) ?? []
|
||||
const names = current.includes(name) ? current.filter(n => n !== name) : [...current, name]
|
||||
const tools = { ...toolsObject(server) }
|
||||
|
||||
if (names.length) {
|
||||
tools[key] = names
|
||||
} else {
|
||||
delete tools[key]
|
||||
}
|
||||
|
||||
const next = { ...server }
|
||||
|
||||
if (Object.keys(tools).length) {
|
||||
next.tools = tools
|
||||
} else {
|
||||
delete next.tools
|
||||
}
|
||||
|
||||
return next
|
||||
}
|
||||
|
||||
export const countEnabledTools = (server: ServerConfig | null | undefined, names: string[]): number =>
|
||||
names.filter(name => isToolEnabled(server, name)).length
|
||||
|
|
@ -509,9 +509,17 @@ export interface AnalyticsResponse {
|
|||
summary: AnalyticsSkillsSummary
|
||||
top_skills: AnalyticsSkillEntry[]
|
||||
}
|
||||
/** Per-tool-name call counts. Absent on older backends. */
|
||||
tools?: AnalyticsToolEntry[]
|
||||
totals: AnalyticsTotals
|
||||
}
|
||||
|
||||
export interface AnalyticsToolEntry {
|
||||
count: number
|
||||
percentage: number
|
||||
tool: string
|
||||
}
|
||||
|
||||
export interface AnalyticsSkillEntry {
|
||||
last_used_at: null | number
|
||||
manage_count: number
|
||||
|
|
@ -640,6 +648,10 @@ export interface SkillInfo {
|
|||
description: string
|
||||
enabled: boolean
|
||||
name: string
|
||||
/** Total observed activity (use + view + patch). Absent on older backends. */
|
||||
usage?: number
|
||||
/** 'agent' = learned/local (editable), 'bundled' = ships with Hermes, 'hub' = installed. */
|
||||
provenance?: 'agent' | 'bundled' | 'hub'
|
||||
}
|
||||
|
||||
export interface ToolsetInfo {
|
||||
|
|
|
|||
|
|
@ -35,6 +35,9 @@ LOG_FILES = {
|
|||
"gateway": "gateway.log",
|
||||
"gui": "gui.log",
|
||||
"desktop": "desktop.log",
|
||||
# Every stdio MCP subprocess's stderr (tools/mcp_tool.py redirects it
|
||||
# here, with per-server session markers) — the "MCP output channel".
|
||||
"mcp": "mcp-stderr.log",
|
||||
}
|
||||
|
||||
# Log line timestamp regex — matches "2026-04-05 22:35:00,123" or
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ from hermes_cli.config import (
|
|||
from hermes_cli.colors import Colors, color
|
||||
from hermes_constants import display_hermes_home
|
||||
from hermes_cli.mcp_security import validate_mcp_server_entry
|
||||
from tools.mcp_tool import _ENV_VAR_PATTERN
|
||||
from tools.mcp_tool import _ENV_VAR_PATTERN, _env_ref_name
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -117,6 +117,39 @@ def _remove_mcp_server(name: str) -> bool:
|
|||
return True
|
||||
|
||||
|
||||
def _replace_mcp_servers(servers: Dict[str, dict]) -> Tuple[bool, List[str]]:
|
||||
"""Replace the WHOLE ``mcp_servers`` map in config.yaml.
|
||||
|
||||
Unlike ``_save_mcp_server`` (per-key upsert), this sets the entire map so
|
||||
the GUI's mcp.json editor can delete servers, drop an ``enabled: false``
|
||||
flag (re-enable), or remove nested fields and have those *removals* land on
|
||||
disk. A plain ``/api/config`` deep-merge can only add/override keys, never
|
||||
delete them — which is why edits appeared to succeed but the old entry
|
||||
survived (see MCP tab persistence bug).
|
||||
|
||||
Every entry is validated up front; on any suspicious command/args the whole
|
||||
save is rejected (returns ``(False, issues)``) so a bad paste can't be
|
||||
partially applied. An empty map removes the key entirely.
|
||||
"""
|
||||
issues: List[str] = []
|
||||
for name, cfg in servers.items():
|
||||
if not isinstance(cfg, dict):
|
||||
issues.append(f"Server '{name}': expected an object")
|
||||
continue
|
||||
issues.extend(validate_mcp_server_entry(name, cfg))
|
||||
|
||||
if issues:
|
||||
return False, issues
|
||||
|
||||
config = load_config()
|
||||
if servers:
|
||||
config["mcp_servers"] = dict(servers)
|
||||
else:
|
||||
config.pop("mcp_servers", None)
|
||||
save_config(config)
|
||||
return True, []
|
||||
|
||||
|
||||
def _env_key_for_server(name: str) -> str:
|
||||
"""Convert server name to an env-var key like ``MCP_MYSERVER_API_KEY``."""
|
||||
return f"MCP_{name.upper().replace('-', '_')}_API_KEY"
|
||||
|
|
@ -214,12 +247,16 @@ def _resolve_mcp_server_config(config: dict) -> dict:
|
|||
|
||||
|
||||
def _probe_single_server(
|
||||
name: str, config: dict, connect_timeout: float = 30
|
||||
name: str, config: dict, connect_timeout: float = 30, *, details: Optional[dict] = None
|
||||
) -> List[Tuple[str, str]]:
|
||||
"""Temporarily connect to one MCP server, list its tools, disconnect.
|
||||
|
||||
Returns list of ``(tool_name, description)`` tuples.
|
||||
Raises on connection failure.
|
||||
|
||||
``details``: optional dict the probe fills with extra capability counts
|
||||
(``prompts``, ``resources``) — an out-param so the return shape stays
|
||||
stable for existing CLI callers.
|
||||
"""
|
||||
issues = validate_mcp_server_entry(name, config)
|
||||
if issues:
|
||||
|
|
@ -249,6 +286,19 @@ def _probe_single_server(
|
|||
if len(desc) > 80:
|
||||
desc = desc[:77] + "..."
|
||||
tools_found.append((t.name, desc))
|
||||
if details is not None:
|
||||
# Capability probes are best-effort: servers without the
|
||||
# capability raise, which just means "0".
|
||||
try:
|
||||
result = await server.session.list_prompts()
|
||||
details["prompts"] = len(result.prompts)
|
||||
except Exception:
|
||||
pass
|
||||
try:
|
||||
result = await server.session.list_resources()
|
||||
details["resources"] = len(result.resources)
|
||||
except Exception:
|
||||
pass
|
||||
finally:
|
||||
await server.shutdown()
|
||||
|
||||
|
|
@ -632,8 +682,8 @@ def cmd_mcp_test(args):
|
|||
elif headers:
|
||||
for k, v in headers.items():
|
||||
if isinstance(v, str) and ("key" in k.lower() or "auth" in k.lower()):
|
||||
# Mask the value
|
||||
resolved = _ENV_VAR_PATTERN.sub(lambda m: os.getenv(m.group(1), ""), v)
|
||||
# Mask the value (accepts ${VAR} and Cursor-style ${env:VAR})
|
||||
resolved = _ENV_VAR_PATTERN.sub(lambda m: os.getenv(_env_ref_name(m.group(1)), ""), v)
|
||||
if len(resolved) > 8:
|
||||
masked = resolved[:4] + "***" + resolved[-4:]
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -8805,6 +8805,12 @@ class MCPServerCreate(BaseModel):
|
|||
profile: Optional[str] = None
|
||||
|
||||
|
||||
class MCPServersReplace(BaseModel):
|
||||
# Whole-map replace (name → raw server config) for the GUI mcp.json editor.
|
||||
servers: Dict[str, Dict[str, Any]] = {}
|
||||
profile: Optional[str] = None
|
||||
|
||||
|
||||
def _redact_mcp_env(env: Dict[str, Any]) -> Dict[str, str]:
|
||||
"""Mask secret-shaped MCP env values for read responses."""
|
||||
out: Dict[str, str] = {}
|
||||
|
|
@ -8890,6 +8896,25 @@ async def add_mcp_server(body: MCPServerCreate, profile: Optional[str] = None):
|
|||
return _mcp_server_summary(name, server_config)
|
||||
|
||||
|
||||
@app.put("/api/mcp/servers")
|
||||
async def replace_mcp_servers(body: MCPServersReplace, profile: Optional[str] = None):
|
||||
"""Replace the entire ``mcp_servers`` map (the GUI mcp.json editor's save).
|
||||
|
||||
The generic ``/api/config`` endpoint deep-merges maps, so it can never
|
||||
delete a server key, drop an ``enabled: false`` flag, or remove a nested
|
||||
field — edits looked saved but the stale entry survived on disk. This
|
||||
endpoint sets the whole map so removals actually persist. Storage stays
|
||||
the config.yaml ``mcp_servers`` key the CLI/TUI already read.
|
||||
"""
|
||||
from hermes_cli.mcp_config import _replace_mcp_servers
|
||||
|
||||
with _profile_scope(body.profile or profile):
|
||||
ok, issues = _replace_mcp_servers(body.servers)
|
||||
if not ok:
|
||||
raise HTTPException(status_code=400, detail="; ".join(issues))
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@app.delete("/api/mcp/servers/{name}")
|
||||
async def remove_mcp_server(name: str, profile: Optional[str] = None):
|
||||
from hermes_cli.mcp_config import _remove_mcp_server
|
||||
|
|
@ -8911,19 +8936,21 @@ async def test_mcp_server(name: str, profile: Optional[str] = None):
|
|||
if name not in servers:
|
||||
raise HTTPException(status_code=404, detail=f"Server '{name}' not found")
|
||||
|
||||
details: Dict[str, Any] = {}
|
||||
|
||||
def _probe_scoped():
|
||||
# Re-enter the scope INSIDE the worker thread so call-time
|
||||
# resolution during the probe — env-placeholder expansion in
|
||||
# _resolve_mcp_server_config reading the profile's .env — sees the
|
||||
# selected profile, matching the config the server was saved into.
|
||||
# (asyncio.to_thread copies contextvars, but entering explicitly
|
||||
# keeps the lock-protected SKILLS_DIR swap balanced per-thread.)
|
||||
# The probe's dedicated MCP event-loop thread is covered too:
|
||||
# _run_on_mcp_loop wraps scheduled coroutines with the caller's
|
||||
# HERMES_HOME override (see mcp_tool._wrap_with_home_override), so
|
||||
# OAuth token stores resolve against the selected profile as well.
|
||||
with _profile_scope(profile):
|
||||
return _probe_single_server(name, servers[name])
|
||||
# Home-only scope (contextvar), NOT _profile_scope. A probe blocks for
|
||||
# as long as the server takes to spawn/connect — a stdio `npx` cold
|
||||
# start is many seconds — and _profile_scope holds a process-global
|
||||
# skills lock for its ENTIRE body. Holding that across the probe
|
||||
# serialized every other endpoint (config/skills/toolsets all take the
|
||||
# same lock), so a slow server made unrelated requests time out at 15s.
|
||||
# The probe touches no skills globals; it only needs the HERMES_HOME
|
||||
# override for .env interpolation + OAuth token resolution, which the
|
||||
# contextvar provides (copied into this to_thread worker; and
|
||||
# _run_on_mcp_loop re-wraps it onto the MCP event-loop thread).
|
||||
with _config_profile_scope(profile):
|
||||
return _probe_single_server(name, servers[name], details=details)
|
||||
|
||||
try:
|
||||
# Probe blocks on a dedicated MCP event loop — run in a thread so the
|
||||
|
|
@ -8938,9 +8965,103 @@ async def test_mcp_server(name: str, profile: Optional[str] = None):
|
|||
return {
|
||||
"ok": True,
|
||||
"tools": [{"name": t, "description": d} for t, d in tools],
|
||||
"prompts": details.get("prompts", 0),
|
||||
"resources": details.get("resources", 0),
|
||||
}
|
||||
|
||||
|
||||
@app.post("/api/mcp/servers/{name}/auth")
|
||||
async def auth_mcp_server(name: str, profile: Optional[str] = None):
|
||||
"""Run the OAuth flow for an HTTP MCP server (opens the system browser).
|
||||
|
||||
Mirrors ``hermes mcp login``: wipe cached OAuth state so the probe forces
|
||||
a fresh browser flow, connect, then verify a token actually landed on disk
|
||||
(some providers serve tools/list unauthenticated — see
|
||||
``_reauth_oauth_server``). Blocks until the browser flow completes, so it
|
||||
runs in a worker thread. ``auth: oauth`` is persisted only on success.
|
||||
"""
|
||||
from hermes_cli.mcp_config import (
|
||||
_get_mcp_servers,
|
||||
_oauth_tokens_present,
|
||||
_probe_single_server,
|
||||
_save_mcp_server,
|
||||
)
|
||||
|
||||
with _profile_scope(profile):
|
||||
servers = _get_mcp_servers()
|
||||
if name not in servers:
|
||||
raise HTTPException(status_code=404, detail=f"Server '{name}' not found")
|
||||
|
||||
cfg = dict(servers[name])
|
||||
if not cfg.get("url"):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="stdio servers authenticate via env keys, not OAuth",
|
||||
)
|
||||
# A server carrying `headers` uses API-key/bearer auth; a 401 there is a bad
|
||||
# key, not an OAuth prompt. Refuse rather than rewrite it to `auth: oauth`
|
||||
# and corrupt a working header-auth config. (Explicit `auth: oauth` is fine.)
|
||||
if cfg.get("headers") and cfg.get("auth") != "oauth":
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="This server uses header/API-key auth, not OAuth — check its key.",
|
||||
)
|
||||
cfg["auth"] = "oauth"
|
||||
|
||||
def _run():
|
||||
from tools.mcp_oauth import force_interactive_oauth
|
||||
|
||||
# Home-only scope, not _profile_scope: this blocks on the browser flow
|
||||
# for up to minutes; holding the shared skills lock that whole time
|
||||
# would freeze every other endpoint. Config writes here (_save_mcp_server)
|
||||
# resolve HERMES_HOME via the contextvar override, which is all they need.
|
||||
with _config_profile_scope(profile), force_interactive_oauth():
|
||||
try:
|
||||
from tools.mcp_oauth_manager import get_manager
|
||||
|
||||
get_manager().remove(name)
|
||||
except Exception:
|
||||
pass # No cached state to clear — fine.
|
||||
# The default 30s connect timeout would kill the flow while the
|
||||
# user is still looking at the browser consent screen — give the
|
||||
# whole browser round-trip room (the callback itself caps at 300s).
|
||||
tools = _probe_single_server(name, cfg, connect_timeout=240)
|
||||
if not _oauth_tokens_present(name):
|
||||
return {
|
||||
"ok": False,
|
||||
"error": (
|
||||
"The server responded, but no OAuth token was obtained — "
|
||||
"this provider may require a manually-registered OAuth "
|
||||
"client (see `hermes mcp login`)."
|
||||
),
|
||||
"tools": [],
|
||||
}
|
||||
_save_mcp_server(name, cfg)
|
||||
return {
|
||||
"ok": True,
|
||||
"tools": [{"name": t, "description": d} for t, d in tools],
|
||||
}
|
||||
|
||||
try:
|
||||
return await asyncio.to_thread(_run)
|
||||
except Exception as exc:
|
||||
msg = str(exc)
|
||||
# Providers that gate RFC 7591 registration to pre-approved clients
|
||||
# (Figma's MCP catalog, etc.) 403 the register call before any
|
||||
# authorization URL exists — surface what's actually happening
|
||||
# instead of a bare "403 Forbidden".
|
||||
lowered = msg.lower()
|
||||
if "403" in msg and ("regist" in lowered or "forbidden" in lowered):
|
||||
msg = (
|
||||
f"'{name}' only allows pre-approved OAuth clients — it rejected "
|
||||
"client registration (403), so no browser flow can start. "
|
||||
"Options: add a pre-registered client to this server's entry "
|
||||
"(oauth: {client_id: ..., client_secret: ...}), or use the "
|
||||
"provider's stdio / API-key server instead."
|
||||
)
|
||||
return {"ok": False, "error": msg, "tools": []}
|
||||
|
||||
|
||||
class MCPEnabledToggle(BaseModel):
|
||||
enabled: bool
|
||||
profile: Optional[str] = None
|
||||
|
|
@ -11185,12 +11306,31 @@ class SkillToggle(BaseModel):
|
|||
async def get_skills(profile: Optional[str] = None):
|
||||
from tools.skills_tool import _find_all_skills
|
||||
from hermes_cli.skills_config import get_disabled_skills
|
||||
from tools.skill_usage import (
|
||||
_read_bundled_manifest_names,
|
||||
_read_hub_installed_names,
|
||||
activity_count,
|
||||
load_usage,
|
||||
)
|
||||
with _profile_scope(profile):
|
||||
config = load_config()
|
||||
disabled = get_disabled_skills(config)
|
||||
skills = _find_all_skills(skip_disabled=True)
|
||||
usage = load_usage()
|
||||
# Set-based provenance (same classification as skill_usage.provenance,
|
||||
# without a per-skill manifest read): hub > bundled > agent, where
|
||||
# "agent" covers agent-authored AND local hand-made skills — the ones
|
||||
# the user may edit/delete from the UI.
|
||||
bundled_names = _read_bundled_manifest_names()
|
||||
hub_names = _read_hub_installed_names()
|
||||
for s in skills:
|
||||
s["enabled"] = s["name"] not in disabled
|
||||
s["usage"] = activity_count(usage.get(s["name"], {}))
|
||||
s["provenance"] = (
|
||||
"hub" if s["name"] in hub_names
|
||||
else "bundled" if s["name"] in bundled_names
|
||||
else "agent"
|
||||
)
|
||||
return skills
|
||||
|
||||
|
||||
|
|
@ -11908,6 +12048,9 @@ async def get_usage_analytics(days: int = 30, profile: Optional[str] = None):
|
|||
"totals": totals,
|
||||
"period_days": days,
|
||||
"skills": skills,
|
||||
# Per-tool-name call counts (already computed by InsightsEngine);
|
||||
# the desktop Capabilities page aggregates these per toolset.
|
||||
"tools": insights_report.get("tools", []),
|
||||
}
|
||||
finally:
|
||||
db.close()
|
||||
|
|
|
|||
|
|
@ -490,6 +490,27 @@ class TestEnvVarInterpolation:
|
|||
assert _interpolate_env_vars(True) is True
|
||||
assert _interpolate_env_vars(None) is None
|
||||
|
||||
def test_interpolate_cursor_env_prefix(self, monkeypatch):
|
||||
"""Cursor-style ${env:VAR} resolves the same secret as ${VAR}."""
|
||||
monkeypatch.setenv("MY_KEY", "secret123")
|
||||
from tools.mcp_tool import _interpolate_env_vars
|
||||
|
||||
assert _interpolate_env_vars("Bearer ${env:MY_KEY}") == "Bearer secret123"
|
||||
|
||||
def test_interpolate_cursor_env_prefix_missing(self, monkeypatch):
|
||||
"""An unset ${env:VAR} keeps its literal placeholder, like ${VAR}."""
|
||||
monkeypatch.delenv("MISSING_VAR", raising=False)
|
||||
from tools.mcp_tool import _interpolate_env_vars
|
||||
|
||||
assert _interpolate_env_vars("Bearer ${env:MISSING_VAR}") == "Bearer ${env:MISSING_VAR}"
|
||||
|
||||
def test_env_ref_name_strips_prefix(self):
|
||||
from tools.mcp_tool import _env_ref_name
|
||||
|
||||
assert _env_ref_name("env:API_KEY") == "API_KEY"
|
||||
assert _env_ref_name("API_KEY") == "API_KEY"
|
||||
assert _env_ref_name(" env:API_KEY ") == "API_KEY"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests: probe-path env resolution (#37792)
|
||||
|
|
|
|||
|
|
@ -104,6 +104,15 @@ _oauth_interactive_enabled: "contextvars.ContextVar[bool]" = contextvars.Context
|
|||
"_oauth_interactive_enabled", default=True
|
||||
)
|
||||
|
||||
# Forces _is_interactive() past the stdin-TTY check for flows driven from a
|
||||
# GUI (dashboard/desktop REST): the browser + localhost callback server do all
|
||||
# the work there, and the stdin paste fallback degrades harmlessly (EOF is
|
||||
# swallowed by _paste_callback_reader). Suppression still wins — background
|
||||
# discovery must never start a browser flow.
|
||||
_oauth_interactive_forced: "contextvars.ContextVar[bool]" = contextvars.ContextVar(
|
||||
"_oauth_interactive_forced", default=False
|
||||
)
|
||||
|
||||
|
||||
# Skip tokens accepted at the paste prompt — exit OAuth without auth.
|
||||
_SKIP_TOKENS = frozenset({"skip", "cancel", "s", "n", "no", "q", "quit"})
|
||||
|
|
@ -150,12 +159,30 @@ def _is_interactive() -> bool:
|
|||
"""Return True if we can reasonably expect to interact with a user."""
|
||||
if not _oauth_interactive_enabled.get():
|
||||
return False
|
||||
if _oauth_interactive_forced.get():
|
||||
return True
|
||||
try:
|
||||
return sys.stdin.isatty()
|
||||
except (AttributeError, ValueError):
|
||||
return False
|
||||
|
||||
|
||||
@contextmanager
|
||||
def force_interactive_oauth():
|
||||
"""Treat the current execution context as interactive despite no TTY.
|
||||
|
||||
For GUI-driven auth (dashboard/desktop REST endpoint): the user IS present
|
||||
— just not on stdin. Opens the browser + localhost callback flow that the
|
||||
TTY heuristic would otherwise refuse. Same ContextVar propagation story as
|
||||
suppress_interactive_oauth() (#35927).
|
||||
"""
|
||||
token = _oauth_interactive_forced.set(True)
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
_oauth_interactive_forced.reset(token)
|
||||
|
||||
|
||||
@contextmanager
|
||||
def suppress_interactive_oauth():
|
||||
"""Disable stdin-based OAuth prompts for the current execution context.
|
||||
|
|
|
|||
|
|
@ -359,6 +359,19 @@ _CREDENTIAL_PATTERN = re.compile(
|
|||
_ENV_VAR_PATTERN = re.compile(r"\$\{([^}]+)\}")
|
||||
|
||||
|
||||
def _env_ref_name(ref: str) -> str:
|
||||
"""Normalize a ``${...}`` reference body into an env-var name.
|
||||
|
||||
Accepts Cursor-style ``${env:VAR}`` in addition to plain ``${VAR}`` by
|
||||
stripping a leading ``env:`` prefix. The result is the bare variable name
|
||||
to look up in the secret scope / ``os.environ``.
|
||||
"""
|
||||
ref = ref.strip()
|
||||
if ref.startswith("env:"):
|
||||
ref = ref[len("env:"):].strip()
|
||||
return ref
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Security helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -3207,17 +3220,20 @@ def _interrupted_call_result() -> str:
|
|||
def _interpolate_env_vars(value):
|
||||
"""Recursively resolve ``${VAR}`` placeholders.
|
||||
|
||||
Resolves from the active profile's secret scope when multiplexing is on
|
||||
(so an MCP server config's ``${API_KEY}`` picks up the routed profile's
|
||||
value, not the process-global ``os.environ`` which may hold another
|
||||
profile's), falling back to ``os.environ`` otherwise. Unset vars keep the
|
||||
literal ``${VAR}`` placeholder, as before.
|
||||
Both ``${VAR}`` and Cursor-style ``${env:VAR}`` are accepted — the
|
||||
``env:`` prefix is stripped so a doc copied from a Cursor / Claude MCP
|
||||
config resolves the same secret. Resolves from the active profile's secret
|
||||
scope when multiplexing is on (so an MCP server config's ``${API_KEY}``
|
||||
picks up the routed profile's value, not the process-global ``os.environ``
|
||||
which may hold another profile's), falling back to ``os.environ``
|
||||
otherwise. Unset vars keep the literal placeholder, as before.
|
||||
"""
|
||||
from agent.secret_scope import get_secret as _get_secret
|
||||
|
||||
if isinstance(value, str):
|
||||
def _replace(m):
|
||||
return _get_secret(m.group(1), m.group(0)) or m.group(0)
|
||||
name = _env_ref_name(m.group(1))
|
||||
return _get_secret(name, m.group(0)) or m.group(0)
|
||||
return _ENV_VAR_PATTERN.sub(_replace, value)
|
||||
if isinstance(value, dict):
|
||||
return {k: _interpolate_env_vars(v) for k, v in value.items()}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue