From a1a3cc86f727354aba1be86825118ad3b5384169 Mon Sep 17 00:00:00 2001 From: Rodaddy Date: Thu, 11 Jun 2026 21:08:39 -0400 Subject: [PATCH 1/5] feat: harden CT216 hub discovery and auth Add authenticated remote service discovery and remote-only CLI routing for hub-hosted services. Add sanitized private registry import/export, Vaultwarden secret refs, safe config/token watch reloads, caller-aware metrics/audit logging, token refresh/rotation, and macOS upgrade docs. Validation: bun run typecheck; bun test; git diff --check; ggshield secret scan repo . --- README.md | 47 +- src/cli/commands/direct-service.ts | 13 + src/cli/commands/remote-routing.ts | 10 + src/cli/commands/schema.ts | 49 +- src/cli/commands/service-help.ts | 55 ++- src/cli/commands/services.ts | 33 +- src/cli/commands/tool-call.ts | 93 +++- src/config/loader.ts | 255 ++++++++++- src/config/schema.ts | 13 +- src/credentials/credential-manager.ts | 4 + src/daemon/auth-provider.ts | 153 ++++++- src/daemon/auth.ts | 3 + src/daemon/config-manager.ts | 89 ++++ src/daemon/file-watch.ts | 100 ++++ src/daemon/index.ts | 10 + src/daemon/metrics.ts | 132 +++++- src/daemon/pool.ts | 46 +- src/daemon/server.ts | 74 ++- src/logger/audit.ts | 6 + src/process/client.ts | 172 +++++-- src/process/index.ts | 9 + src/process/remote-discovery.ts | 103 +++++ src/secrets/index.ts | 7 + src/secrets/refs.ts | 207 +++++++++ tests/cli/remote-routing.test.ts | 70 +++ tests/config/loader.test.ts | 503 ++++++++++++++++++++- tests/config/schema.test.ts | 59 +++ tests/daemon/api.test.ts | 162 +++++++ tests/daemon/auth-provider.test.ts | 140 ++++++ tests/daemon/file-watch.test.ts | 152 +++++++ tests/daemon/observability.test.ts | 174 ++++++- tests/daemon/pool.test.ts | 38 +- tests/daemon/timeout.test.ts | 181 ++++---- tests/fixtures/mock-remote-daemon.ts | 42 ++ tests/fixtures/mock-vaultwarden-command.ts | 24 + tests/integration/cli-binary.test.ts | 127 ++++++ tests/logger/audit.test.ts | 4 + tests/process/client-auth-refresh.test.ts | 136 ++++++ tests/process/client-remote.test.ts | 229 ++++++++++ tests/secrets/refs.test.ts | 117 +++++ tests/test-helpers/run-cli.ts | 13 +- 41 files changed, 3641 insertions(+), 213 deletions(-) create mode 100644 src/cli/commands/direct-service.ts create mode 100644 src/cli/commands/remote-routing.ts create mode 100644 src/daemon/file-watch.ts create mode 100644 src/process/remote-discovery.ts create mode 100644 src/secrets/index.ts create mode 100644 src/secrets/refs.ts create mode 100644 tests/cli/remote-routing.test.ts create mode 100644 tests/daemon/file-watch.test.ts create mode 100644 tests/fixtures/mock-remote-daemon.ts create mode 100644 tests/fixtures/mock-vaultwarden-command.ts create mode 100644 tests/process/client-auth-refresh.test.ts create mode 100644 tests/secrets/refs.test.ts diff --git a/README.md b/README.md index 9cffebd..ad2e7bc 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,25 @@ bun run build The compiled binary lands at `dist/mcp2cli`. Add it to your PATH or symlink it. +### macOS Binary Upgrades + +On macOS, do not overwrite an existing compiled binary in place with `cp new dist/mcp2cli`. Replacing the contents of the same inode can invalidate the ad-hoc code signature and cause the next exec to be killed with `SIGKILL` / exit code 137. + +Use a fresh inode instead: + +```bash +rm dist/mcp2cli +cp /path/to/new/mcp2cli dist/mcp2cli +``` + +If the local UI daemon is managed by launchd, restart it after replacing the binary: + +```bash +launchctl kickstart -k gui/501/com.mcp2cli.local-ui +``` + +The already-running daemon keeps the old inode open until restart, so this is safe to do while the daemon is live. + ## Configuration ### Service Registry @@ -196,6 +215,7 @@ fi |----------|---------|-------------| | `MCP2CLI_LOG_LEVEL` | `silent` | Log verbosity: `silent`, `error`, `warn`, `info`, `debug` | | `MCP2CLI_IDLE_TIMEOUT` | `60` | Daemon idle timeout in seconds | +| `MCP2CLI_STARTUP_TIMEOUT` | `10000` | CLI wait time for daemon startup readiness in milliseconds | | `MCP2CLI_TOOL_TIMEOUT` | `30000` | Tool call timeout in milliseconds | | `MCP2CLI_POOL_MAX` | `50` | Max concurrent MCP connections in the pool | | `MCP2CLI_LOG_DIR` | `~/.cache/mcp2cli/logs` | Directory for stderr capture logs | @@ -270,13 +290,15 @@ mcp2cli supports multi-user RBAC via `~/.config/mcp2cli/tokens.json`. Each user "role": "admin", "description": "Full admin access", "username": "rico", - "password": "your-web-ui-password" + "password": "your-web-ui-password", + "expiresAt": "2026-07-01T00:00:00.000Z" }, { "id": "skippy", "token": "your-agent-token-here", "role": "agent", - "description": "AI agent - tools + read, no config mutations" + "description": "AI agent - tools + read, no config mutations", + "expiresAt": "2026-07-01T00:00:00.000Z" }, { "id": "viewer01", @@ -303,6 +325,8 @@ Generate secure tokens: `openssl rand -base64 32` The `username`/`password` fields enable web UI login at the daemon's root URL. Token-based auth (Bearer header) works for all API and CLI access. +The optional `expiresAt` field enables token expiry and refresh. Expired tokens are rejected. Near-expiry tokens from `tokens.json` can be rotated through `POST /api/auth/refresh`; the daemon writes the new token back to `tokens.json` and hot-reloads token file edits. Local CLI clients proactively refresh near-expiry admin tokens before daemon API calls. + ### Fallback Behavior - **No tokens.json, no env token:** Auth disabled, all requests treated as admin (backward compatible) @@ -637,7 +661,16 @@ Manual edits inside `MANUAL:START` / `MANUAL:END` markers are preserved across r |----------|---------|-------------| | `MCP2CLI_CACHE_DIR` | `~/.cache/mcp2cli` | Base directory for schema cache and circuit breaker state | | `MCP2CLI_TOKENS_FILE` | `~/.config/mcp2cli/tokens.json` | Path to multi-user token config | +| `MCP2CLI_TOKEN_REFRESH_WINDOW_MS` | `86400000` | Refresh window for expiring tokens (default 24h) | +| `MCP2CLI_TOKEN_TTL_MS` | `2592000000` | Lifetime for refreshed tokens (default 30d) | | `MCP2CLI_CREDENTIALS_FILE` | `~/.config/mcp2cli/credentials.json` | Path to per-identity credential mappings | +| `MCP2CLI_IMPORT_TOKEN` | (unset) | Bearer token used only for `importUrl` config imports | +| `MCP2CLI_IMPORT_ALLOWED_HOSTS` | (required for `importUrl`) | Comma-separated allowlist for `importUrl` hostnames | +| `MCP2CLI_IMPORT_ALLOW_DNS` | (unset) | Set to `1` to permit DNS hostnames for non-private `importUrl` targets | +| `MCP2CLI_IMPORT_ALLOW_HTTP` | (unset) | Set to `1` to permit non-HTTPS `importUrl` targets | +| `MCP2CLI_IMPORT_ALLOW_PRIVATE` | (unset) | Set to `1` to permit loopback/private/link-local `importUrl` targets | +| `MCP2CLI_METRICS_INCLUDE_CALLER` | (unset) | Set to `1` to include raw caller labels on public `/metrics`; disabled by default | +| `MCP2CLI_VAULTWARDEN_TIMEOUT_MS` | `10000` | Timeout for Vaultwarden-backed `${secret:...}` lookups | ## Network Deployment @@ -688,8 +721,8 @@ The daemon supports two auth modes (see [Multi-User Authentication](#multi-user- All token comparisons use timing-safe equality to prevent timing attacks. **Auth-exempt paths** -- these skip authentication so load balancers and monitoring can probe without credentials: -- `GET /health` -- health check with uptime, memory, and pool status -- `GET /metrics` -- Prometheus metrics endpoint +- `GET /health` -- health check with uptime, memory, and active connection count +- `GET /metrics` -- Prometheus metrics endpoint with aggregate service/tool metrics ### Prometheus Metrics @@ -708,6 +741,12 @@ The daemon exposes metrics at `GET /metrics` in Prometheus text exposition forma | `mcp2cli_process_uptime_seconds` | gauge | Daemon uptime | | `mcp2cli_process_memory_rss_bytes` | gauge | Resident set size | +Raw caller labels on public `/metrics` are disabled by default to avoid exposing user IDs to unauthenticated scrapers. Set `MCP2CLI_METRICS_INCLUDE_CALLER=1` only in trusted monitoring environments to add `{service, tool, caller}` series. + +For a quick JSON breakdown by identity, call `GET /api/metrics/user/:userId` with a bearer token that has `status` permission. Non-admin callers can only read their own user metrics; admins can read any user. + +Remote clients discover the daemon's service inventory through authenticated `GET /api/services/discovery`, not the public `/health` probe. + Add to your Prometheus config: ```yaml diff --git a/src/cli/commands/direct-service.ts b/src/cli/commands/direct-service.ts new file mode 100644 index 0000000..8919bad --- /dev/null +++ b/src/cli/commands/direct-service.ts @@ -0,0 +1,13 @@ +import type { ServiceConfig } from "../../config/index.ts"; +import { resolveServiceSecretRefs, VaultwardenSecretResolver } from "../../secrets/index.ts"; + +export async function resolveDirectServiceConfig( + serviceName: string, + service: ServiceConfig, +): Promise { + return resolveServiceSecretRefs( + serviceName, + service, + new VaultwardenSecretResolver(), + ); +} diff --git a/src/cli/commands/remote-routing.ts b/src/cli/commands/remote-routing.ts new file mode 100644 index 0000000..e44072e --- /dev/null +++ b/src/cli/commands/remote-routing.ts @@ -0,0 +1,10 @@ +import { getRemoteServiceAvailability } from "../../process/index.ts"; + +export async function shouldRouteMissingServiceToRemote( + serviceName: string, + daemonEnabled: boolean, +): Promise { + if (!daemonEnabled) return false; + const availability = await getRemoteServiceAvailability(serviceName); + return availability === "hosted"; +} diff --git a/src/cli/commands/schema.ts b/src/cli/commands/schema.ts index ada3866..a9a6997 100644 --- a/src/cli/commands/schema.ts +++ b/src/cli/commands/schema.ts @@ -4,7 +4,7 @@ * ADV-01: Checks cache first, falls back to live fetch, caches result. * Supports --fresh flag to bypass cache for one call. */ -import { loadConfig } from "../../config/index.ts"; +import { ConfigError, loadConfig } from "../../config/index.ts"; import { connectToService } from "../../connection/index.ts"; import { connectToHttpService } from "../../connection/http-transport.ts"; import { connectToWebSocketService } from "../../connection/websocket-transport.ts"; @@ -24,6 +24,8 @@ import { validateIdentifier } from "../../validation/pipelines.ts"; import { printError } from "../errors.ts"; import { EXIT_CODES } from "../../types/index.ts"; import type { CommandHandler } from "../../types/index.ts"; +import { resolveDirectServiceConfig } from "./direct-service.ts"; +import { shouldRouteMissingServiceToRemote } from "./remote-routing.ts"; export const handleSchema: CommandHandler = async (args: string[]) => { // Extract --fresh flag before parsing positional args @@ -72,10 +74,38 @@ export const handleSchema: CommandHandler = async (args: string[]) => { } // Load config and resolve service - const config = await loadConfig(); - const service = config.services[serviceName]; + const daemonEnabled = !process.env.MCP2CLI_NO_DAEMON; + let config: Awaited> | null = null; + try { + config = await loadConfig(); + } catch (err) { + if (!(err instanceof ConfigError) || err.code !== "CONFIG_NOT_FOUND" || !daemonEnabled) { + throw err; + } + } + const service = config?.services[serviceName]; if (!service) { + if (await shouldRouteMissingServiceToRemote(serviceName, daemonEnabled)) { + const result = await getSchemaViaDaemon({ service: serviceName, tool: toolName }); + + if (result.success) { + const schemaOutput = result.result as SchemaOutput; + console.log(formatSchemaOutput(schemaOutput)); + await cacheSchemaResult(serviceName, schemaOutput); + process.exitCode = EXIT_CODES.SUCCESS; + } else { + printError({ + error: true, + code: result.error.code, + message: result.error.message, + ...(result.error.reason ? { reason: result.error.reason } : {}), + }); + process.exitCode = EXIT_CODES.VALIDATION; + } + return; + } + printError({ error: true, code: "UNKNOWN_COMMAND", @@ -120,8 +150,6 @@ export const handleSchema: CommandHandler = async (args: string[]) => { } } - const daemonEnabled = !process.env.MCP2CLI_NO_DAEMON; - if (daemonEnabled) { // Daemon path: get full SchemaOutput via daemon's /schema endpoint const result = await getSchemaViaDaemon({ service: serviceName, tool: toolName }); @@ -147,11 +175,12 @@ export const handleSchema: CommandHandler = async (args: string[]) => { // Direct path (MCP2CLI_NO_DAEMON=1): legacy direct connection // Connect and get schema (stdio, http, or websocket) - const connection = service.backend === "http" - ? await connectToHttpService(service) - : service.backend === "websocket" - ? await connectToWebSocketService(service) - : await connectToService(service); + const directService = await resolveDirectServiceConfig(serviceName, service); + const connection = directService.backend === "http" + ? await connectToHttpService(directService) + : directService.backend === "websocket" + ? await connectToWebSocketService(directService) + : await connectToService(directService); try { const result = await getToolSchema(connection.client, toolName, serviceName); diff --git a/src/cli/commands/service-help.ts b/src/cli/commands/service-help.ts index 6ca33eb..9b5c29d 100644 --- a/src/cli/commands/service-help.ts +++ b/src/cli/commands/service-help.ts @@ -2,7 +2,7 @@ * Handle `mcp2cli --help` -- list available tools for a service. * Routes through daemon by default. Set MCP2CLI_NO_DAEMON=1 for direct connection. */ -import { loadConfig } from "../../config/index.ts"; +import { ConfigError, loadConfig } from "../../config/index.ts"; import { connectToService } from "../../connection/index.ts"; import { connectToHttpService } from "../../connection/http-transport.ts"; import { connectToWebSocketService } from "../../connection/websocket-transport.ts"; @@ -13,16 +13,52 @@ import { filterTools, extractPolicy } from "../../access/index.ts"; import { isAiMode } from "../help.ts"; import { printError } from "../errors.ts"; import { EXIT_CODES } from "../../types/index.ts"; +import { resolveDirectServiceConfig } from "./direct-service.ts"; +import { shouldRouteMissingServiceToRemote } from "./remote-routing.ts"; export async function handleServiceHelp( serviceName: string, args: string[], ): Promise { // Load config and resolve service - const config = await loadConfig(); - const service = config.services[serviceName]; + const daemonEnabled = !process.env.MCP2CLI_NO_DAEMON; + let config: Awaited> | null = null; + try { + config = await loadConfig(); + } catch (err) { + if (!(err instanceof ConfigError) || err.code !== "CONFIG_NOT_FOUND" || !daemonEnabled) { + throw err; + } + } + const service = config?.services[serviceName]; if (!service) { + if (await shouldRouteMissingServiceToRemote(serviceName, daemonEnabled)) { + const result = await listToolsViaDaemon({ service: serviceName }); + + if (result.success) { + const listing: ToolListing = { + service: serviceName, + description: "(remote only)", + tools: result.result as ToolSummary[], + usage: `mcp2cli ${serviceName} [--params '{}']`, + }; + + const aiMode = isAiMode(args); + console.log(formatToolListing(listing, aiMode)); + process.exitCode = EXIT_CODES.SUCCESS; + } else { + printError({ + error: true, + code: result.error.code, + message: result.error.message, + ...(result.error.reason ? { reason: result.error.reason } : {}), + }); + process.exitCode = EXIT_CODES.CONNECTION; + } + return; + } + printError({ error: true, code: "UNKNOWN_COMMAND", @@ -32,8 +68,6 @@ export async function handleServiceHelp( return; } - const daemonEnabled = !process.env.MCP2CLI_NO_DAEMON; - if (daemonEnabled) { // Daemon path: route through persistent daemon process const result = await listToolsViaDaemon({ service: serviceName }); @@ -67,11 +101,12 @@ export async function handleServiceHelp( // Direct path (MCP2CLI_NO_DAEMON=1): legacy direct connection // Connect and introspect (stdio, http, or websocket) - const connection = service.backend === "http" - ? await connectToHttpService(service) - : service.backend === "websocket" - ? await connectToWebSocketService(service) - : await connectToService(service); + const directService = await resolveDirectServiceConfig(serviceName, service); + const connection = directService.backend === "http" + ? await connectToHttpService(directService) + : directService.backend === "websocket" + ? await connectToWebSocketService(directService) + : await connectToService(directService); try { const allTools = await listToolsCached(connection.client, serviceName); diff --git a/src/cli/commands/services.ts b/src/cli/commands/services.ts index 9aa8056..6c9f1e2 100644 --- a/src/cli/commands/services.ts +++ b/src/cli/commands/services.ts @@ -1,5 +1,7 @@ import type { CommandHandler } from "../../types/index.ts"; -import { loadConfig } from "../../config/index.ts"; +import { ConfigError, loadConfig } from "../../config/index.ts"; +import { getRemoteServiceNames } from "../../process/index.ts"; +import { getRemoteConfig } from "../../daemon/paths.ts"; /** * List configured MCP services from services.json. @@ -7,14 +9,39 @@ import { loadConfig } from "../../config/index.ts"; * Errors propagate to the main catch block as ConfigError. */ export const handleServices: CommandHandler = async (_args: string[]) => { - const config = await loadConfig(); + const [config, remoteNames] = await Promise.all([ + loadConfig().catch((err) => { + if (err instanceof ConfigError && err.code === "CONFIG_NOT_FOUND" && getRemoteConfig()) { + return null; + } + throw err; + }), + getRemoteServiceNames(), + ]); - const services = Object.entries(config.services).map(([name, svc]) => ({ + const services: Array<{ + name: string; + description: string; + backend: string; + status: "configured" | "remote-configured"; + }> = Object.entries(config?.services ?? {}).map(([name, svc]) => ({ name, description: svc.description ?? "(no description)", backend: svc.backend, status: "configured" as const, })); + const localNames = new Set(services.map((svc) => svc.name)); + for (const name of remoteNames) { + if (!localNames.has(name)) { + services.push({ + name, + description: "(remote only)", + backend: "remote", + status: "remote-configured" as const, + }); + } + } + console.log(JSON.stringify({ services })); }; diff --git a/src/cli/commands/tool-call.ts b/src/cli/commands/tool-call.ts index b208532..7b8fd15 100644 --- a/src/cli/commands/tool-call.ts +++ b/src/cli/commands/tool-call.ts @@ -6,7 +6,7 @@ import { applyFieldMask, } from "../../invocation/index.ts"; import { validationResultToCliError } from "../../validation/pipelines.ts"; -import { loadConfig } from "../../config/index.ts"; +import { ConfigError, loadConfig } from "../../config/index.ts"; import { connectToService } from "../../connection/index.ts"; import { connectToHttpService } from "../../connection/http-transport.ts"; import { connectToWebSocketService } from "../../connection/websocket-transport.ts"; @@ -19,6 +19,8 @@ import { EXIT_CODES } from "../../types/index.ts"; import type { ErrorCode } from "../../types/index.ts"; import type { SchemaOutput } from "../../schema/types.ts"; import { formatOutput } from "../../format/index.ts"; +import { resolveDirectServiceConfig } from "./direct-service.ts"; +import { shouldRouteMissingServiceToRemote } from "./remote-routing.ts"; /** * Map daemon error codes to semantic exit codes. @@ -65,10 +67,80 @@ export async function handleToolCall(args: string[]): Promise { } // 3. Load config and resolve service - const config = await loadConfig(); - const service = config.services[parsed.value.serviceName]; + const daemonEnabled = !process.env.MCP2CLI_NO_DAEMON; + let config: Awaited> | null = null; + try { + config = await loadConfig(); + } catch (err) { + if (!(err instanceof ConfigError) || err.code !== "CONFIG_NOT_FOUND" || !daemonEnabled) { + throw err; + } + } + const service = config?.services[parsed.value.serviceName]; if (!service) { + if (await shouldRouteMissingServiceToRemote(parsed.value.serviceName, daemonEnabled)) { + if (parsed.value.dryRun) { + const schemaResponse = await getSchemaViaDaemon({ + service: parsed.value.serviceName, + tool: parsed.value.toolName, + }); + + if (!schemaResponse.success) { + printError({ + error: true, + code: schemaResponse.error.code, + message: schemaResponse.error.message, + ...(schemaResponse.error.reason ? { reason: schemaResponse.error.reason } : {}), + }); + process.exitCode = mapErrorCodeToExit(schemaResponse.error.code); + return; + } + + const schema = schemaResponse.result as SchemaOutput; + const preview = formatDryRunPreview({ + service: parsed.value.serviceName, + tool: parsed.value.toolName, + params: parsed.value.params, + toolDescription: schema.description, + inputSchema: schema.inputSchema, + fields: parsed.value.fields, + }); + + console.log(JSON.stringify(preview)); + process.exitCode = EXIT_CODES.DRY_RUN; + return; + } + + const result = await callViaDaemon({ + service: parsed.value.serviceName, + tool: parsed.value.toolName, + params: parsed.value.params, + }); + + if (result.success) { + let outputData = result.result; + if (parsed.value.fields.length > 0) { + const { masked, missing } = applyFieldMask(result.result, parsed.value.fields); + for (const field of missing) { + process.stderr.write(`warning: field "${field}" not found in response\n`); + } + outputData = masked; + } + console.log(formatOutput(outputData, parsed.value.format)); + process.exitCode = EXIT_CODES.SUCCESS; + } else { + printError({ + error: true, + code: result.error.code, + message: result.error.message, + ...(result.error.reason ? { reason: result.error.reason } : {}), + }); + process.exitCode = mapErrorCodeToExit(result.error.code); + } + return; + } + printError({ error: true, code: "UNKNOWN_COMMAND", @@ -91,8 +163,6 @@ export async function handleToolCall(args: string[]): Promise { return; } - const daemonEnabled = !process.env.MCP2CLI_NO_DAEMON; - if (daemonEnabled) { // Daemon path: route through persistent daemon process @@ -165,11 +235,12 @@ export async function handleToolCall(args: string[]): Promise { // Direct path (MCP2CLI_NO_DAEMON=1): legacy direct connection // 4. Connect to MCP server (stdio, http, or websocket) - const connection = service.backend === "http" - ? await connectToHttpService(service) - : service.backend === "websocket" - ? await connectToWebSocketService(service) - : await connectToService(service); + const directService = await resolveDirectServiceConfig(parsed.value.serviceName, service); + const connection = directService.backend === "http" + ? await connectToHttpService(directService) + : directService.backend === "websocket" + ? await connectToWebSocketService(directService) + : await connectToService(directService); const directStartTime = performance.now(); let directSuccess = false; @@ -177,7 +248,7 @@ export async function handleToolCall(args: string[]): Promise { let directError: string | undefined; let directResolvedTool: string | undefined; let dryRun = false; - const directTransport = service.backend; + const directTransport = directService.backend; try { // Dry-run interception (inside try/finally so connection closes) diff --git a/src/config/loader.ts b/src/config/loader.ts index 9ad8e30..eeaf72f 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -1,6 +1,11 @@ import { ServicesConfigSchema } from "./schema.ts"; import { ConfigError } from "./errors.ts"; import type { ServicesConfig } from "./schema.ts"; +import { createLogger } from "../logger/index.ts"; +import { isIP } from "node:net"; +import { lookup } from "node:dns/promises"; + +const log = createLogger("config-loader"); /** * Resolve the config file path. @@ -74,5 +79,253 @@ export async function loadConfig(configPath?: string): Promise { ); } - return result.data; + return maybeImportConfig(result.data, path); +} + +async function maybeImportConfig( + localConfig: ServicesConfig, + localPath: string, +): Promise { + if (!localConfig.importUrl) return localConfig; + + if (localConfig.importTtlSeconds !== undefined) { + const ttlSeconds = localConfig.importTtlSeconds; + const state = await readImportState(localPath); + const ageMs = state ? Date.now() - state.importedAt : Number.POSITIVE_INFINITY; + if (ttlSeconds > 0 && state?.url === localConfig.importUrl && ageMs < ttlSeconds * 1000) { + try { + await validateImportUrl(localConfig.importUrl); + } catch (err) { + log.warn("config_import_cache_rejected", { + url: redactImportUrl(localConfig.importUrl), + error: err instanceof Error ? err.message : String(err), + }); + return localConfig; + } + return mergeImportedConfig(localConfig, state.importedConfig); + } + } + + try { + const response = await fetchImportUrl(localConfig.importUrl); + if (!response.ok) { + throw new Error(`${response.status} ${response.statusText}`); + } + const importedRaw = await response.json(); + const imported = ServicesConfigSchema.safeParse(importedRaw); + if (!imported.success) { + const issues = imported.error.issues.map((i) => `${i.path.join(".")}: ${i.message}`).join(", "); + throw new Error(`validation failed: ${issues}`); + } + const merged = mergeImportedConfig(localConfig, imported.data); + log.info("config_imported_on_load", { + url: redactImportUrl(localConfig.importUrl), + localServices: Object.keys(localConfig.services).length, + importedServices: Object.keys(imported.data.services).length, + mergedServices: Object.keys(merged.services).length, + }); + await writeImportState(localPath, localConfig.importUrl, imported.data); + return merged; + } catch (err) { + log.warn("config_import_failed", { + url: redactImportUrl(localConfig.importUrl), + error: err instanceof Error ? err.message : String(err), + }); + return localConfig; + } +} + +async function fetchImportUrl(rawUrl: string): Promise { + const originalUrl = new URL(rawUrl); + let currentUrl = rawUrl; + for (let redirectCount = 0; redirectCount <= 5; redirectCount++) { + await validateImportUrl(currentUrl); + const headers = buildImportHeaders(new URL(currentUrl).origin === originalUrl.origin); + const response = await fetch(currentUrl, { + method: "GET", + headers, + redirect: "manual", + signal: AbortSignal.timeout(10_000), + }); + if (![301, 302, 303, 307, 308].includes(response.status)) { + return response; + } + const location = response.headers.get("location"); + if (!location) { + throw new Error("importUrl redirect missing Location header"); + } + currentUrl = new URL(location, currentUrl).toString(); + } + throw new Error("importUrl exceeded redirect limit"); +} + +function buildImportHeaders(includeAuthorization: boolean): Record { + const headers: Record = { Accept: "application/json" }; + const token = process.env.MCP2CLI_IMPORT_TOKEN; + if (token && includeAuthorization) { + headers.Authorization = `Bearer ${token}`; + } + return headers; +} + +async function validateImportUrl(rawUrl: string): Promise { + let url: URL; + try { + url = new URL(rawUrl); + } catch { + throw new Error("invalid importUrl"); + } + + const allowHttp = process.env.MCP2CLI_IMPORT_ALLOW_HTTP === "1"; + if (url.protocol !== "https:" && !(allowHttp && url.protocol === "http:")) { + throw new Error("importUrl must use https"); + } + + const allowedHosts = parseCsvEnv(process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS); + if (allowedHosts.size === 0) { + throw new Error("MCP2CLI_IMPORT_ALLOWED_HOSTS is required for importUrl"); + } + if (!allowedHosts.has(url.hostname)) { + throw new Error(`importUrl host is not allowed: ${url.hostname}`); + } + + if (process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE !== "1" && isPrivateImportHost(url.hostname)) { + throw new Error(`importUrl host is private or local: ${url.hostname}`); + } + if ( + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE !== "1" + && process.env.MCP2CLI_IMPORT_ALLOW_DNS !== "1" + && !isIP(normalizeImportHostname(url.hostname)) + ) { + throw new Error("DNS importUrl hosts require MCP2CLI_IMPORT_ALLOW_DNS=1"); + } + await validateResolvedImportHost(url.hostname); +} + +function parseCsvEnv(raw: string | undefined): Set { + return new Set((raw ?? "").split(",").map((part) => part.trim()).filter(Boolean)); +} + +function isPrivateImportHost(hostname: string): boolean { + let lower = normalizeImportHostname(hostname); + const mappedIpv4 = lower.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/); + if (mappedIpv4?.[1]) lower = mappedIpv4[1]; + const mappedIpv4Hex = lower.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/); + if (mappedIpv4Hex?.[1] && mappedIpv4Hex[2]) { + const high = Number.parseInt(mappedIpv4Hex[1], 16); + const low = Number.parseInt(mappedIpv4Hex[2], 16); + lower = [ + (high >> 8) & 0xff, + high & 0xff, + (low >> 8) & 0xff, + low & 0xff, + ].join("."); + } + if (lower === "localhost" || lower.endsWith(".localhost")) return true; + if (lower === "metadata.google.internal") return true; + + const family = isIP(lower); + if (family === 4) { + const parts = lower.split(".").map((part) => Number.parseInt(part, 10)); + const [a, b] = parts; + if (a === 10 || a === 127 || a === 0) return true; + if (a === 169 && b === 254) return true; + if (a === 172 && b !== undefined && b >= 16 && b <= 31) return true; + if (a === 192 && b === 168) return true; + if (a === 100 && b !== undefined && b >= 64 && b <= 127) return true; + if (a !== undefined && a >= 224) return true; + if (lower === "100.100.100.200") return true; + } + if (family === 6) { + if (lower === "::1") return true; + if (lower.startsWith("fe80:") || lower.startsWith("fc") || lower.startsWith("fd")) return true; + if (lower.startsWith("ff")) return true; + } + return false; +} + +function normalizeImportHostname(hostname: string): string { + const lower = hostname.toLowerCase(); + if (lower.startsWith("[") && lower.endsWith("]")) { + return lower.slice(1, -1); + } + return lower; +} + +async function validateResolvedImportHost(hostname: string): Promise { + if (process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE === "1") return; + if (isIP(hostname)) return; + const addresses = await lookup(hostname, { all: true, verbatim: true }); + for (const address of addresses) { + if (isPrivateImportHost(address.address)) { + throw new Error(`importUrl host resolves to private or local address: ${hostname}`); + } + } +} + +async function readImportState( + localPath: string, +): Promise<{ url: string; importedAt: number; importedConfig: ServicesConfig } | null> { + try { + const raw = await Bun.file(importStatePath(localPath)).json(); + if (!raw || typeof raw !== "object") return null; + const obj = raw as Record; + if (typeof obj.url !== "string" || typeof obj.importedAt !== "number") return null; + const imported = ServicesConfigSchema.safeParse(obj.importedConfig); + if (!imported.success) return null; + return { url: obj.url, importedAt: obj.importedAt, importedConfig: imported.data }; + } catch { + return null; + } +} + +async function writeImportState( + localPath: string, + url: string, + importedConfig: ServicesConfig, +): Promise { + try { + await Bun.write( + importStatePath(localPath), + JSON.stringify({ url, importedAt: Date.now(), importedConfig }, null, 2) + "\n", + ); + } catch (err) { + log.warn("config_import_state_write_failed", { + error: err instanceof Error ? err.message : String(err), + }); + } +} + +function importStatePath(localPath: string): string { + return `${localPath}.import-state.json`; +} + +function redactImportUrl(raw: string): string { + try { + const url = new URL(raw); + url.username = ""; + url.password = ""; + url.search = ""; + url.hash = ""; + return url.toString(); + } catch { + return "[invalid-url]"; + } +} + +export function mergeImportedConfig( + localConfig: ServicesConfig, + importedConfig: ServicesConfig, +): ServicesConfig { + const services: ServicesConfig["services"] = structuredClone(localConfig.services); + + for (const [name, importedService] of Object.entries(importedConfig.services)) { + services[name] = structuredClone(importedService); + services[name].source = importedService.source ?? "remote"; + } + + return { + ...localConfig, + services, + }; } diff --git a/src/config/schema.ts b/src/config/schema.ts index 99f754d..6f4fa41 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -10,6 +10,11 @@ import { z } from "zod"; export const SourceSchema = z.enum(["local", "remote", "remote-local"]).optional(); export type ServiceSource = z.infer; +const SecretRefUrlSchema = z.string().refine( + (value) => (/^\$\{secret:[^}]+\}$/.test(value)) || z.string().url().safeParse(value).success, + { message: "Invalid url" }, +); + /** * Tool access control fields shared by all service backends. * allowTools: glob patterns for tools to include (whitelist). If set, only matching tools are visible. @@ -21,6 +26,8 @@ const accessControlFields = { blockTools: z.array(z.string()).optional(), /** Per-service tool call timeout in milliseconds. Overrides MCP2CLI_TOOL_TIMEOUT env var. */ timeout: z.number().int().positive().optional(), + /** OS platforms where this service can run locally. Values match process.platform. */ + platforms: z.array(z.string().min(1)).optional(), source: SourceSchema, }; @@ -55,7 +62,7 @@ export const StdioFallbackSchema = z.object({ export const HttpServiceSchema = z.object({ description: z.string().optional(), backend: z.literal("http"), - url: z.string().url(), + url: SecretRefUrlSchema, headers: z.record(z.string(), z.string()).optional().default({}), fallback: StdioFallbackSchema.optional(), ...accessControlFields, @@ -69,7 +76,7 @@ export const HttpServiceSchema = z.object({ export const WebSocketServiceSchema = z.object({ description: z.string().optional(), backend: z.literal("websocket"), - url: z.string().url(), + url: SecretRefUrlSchema, headers: z.record(z.string(), z.string()).optional().default({}), fallback: StdioFallbackSchema.optional(), ...accessControlFields, @@ -90,6 +97,8 @@ export const ServiceSchema = z.discriminatedUnion("backend", [ * Requires at least one service to be configured. */ export const ServicesConfigSchema = z.object({ + importUrl: z.string().url().optional(), + importTtlSeconds: z.number().int().nonnegative().optional(), services: z .record(z.string(), ServiceSchema) .refine((obj) => Object.keys(obj).length > 0, { diff --git a/src/credentials/credential-manager.ts b/src/credentials/credential-manager.ts index 835557b..6a1b70c 100644 --- a/src/credentials/credential-manager.ts +++ b/src/credentials/credential-manager.ts @@ -25,6 +25,10 @@ export class CredentialManager { this.configPath = configPath; } + get configFilePath(): string { + return this.configPath; + } + /** * Load credentials from disk. Returns empty config if file doesn't exist. */ diff --git a/src/daemon/auth-provider.ts b/src/daemon/auth-provider.ts index ddca65c..7261db1 100644 --- a/src/daemon/auth-provider.ts +++ b/src/daemon/auth-provider.ts @@ -2,6 +2,9 @@ * Pluggable auth provider interface and token-based RBAC implementation. * Designed for provider swap: token-based now, OAuth/OIDC later. */ +import { mkdir, open, rename } from "node:fs/promises"; +import { dirname, basename, join } from "node:path"; +import { randomBytes } from "node:crypto"; import { createLogger } from "../logger/index.ts"; const log = createLogger("auth-provider"); @@ -50,6 +53,7 @@ export interface TokenEntry { description?: string; username?: string; password?: string; + expiresAt?: string; } export interface TokensConfig { @@ -102,6 +106,13 @@ function validateTokensConfig(raw: unknown): TokensConfig { if (e.password !== undefined && typeof e.password !== "string") { throw new Error(`Token entry ${i} ('${e.id}') has non-string password`); } + let expiresAt: string | undefined; + if (e.expiresAt !== undefined) { + if (typeof e.expiresAt !== "string" || Number.isNaN(Date.parse(e.expiresAt))) { + throw new Error(`Token entry ${i} ('${e.id}') has invalid expiresAt`); + } + expiresAt = e.expiresAt; + } tokens.push({ id: e.id, @@ -110,6 +121,7 @@ function validateTokensConfig(raw: unknown): TokensConfig { description: typeof e.description === "string" ? e.description : undefined, username: typeof e.username === "string" ? e.username : undefined, password: typeof e.password === "string" ? e.password : undefined, + expiresAt, }); } @@ -127,27 +139,44 @@ function validateTokensConfig(raw: unknown): TokensConfig { * Uses timing-safe comparison to prevent timing attacks. */ export class TokenAuthProvider implements AuthProvider { - private tokenMap: Map; + private tokenMap: Map; /** Map username -> { password, token, context } for basic auth login */ - private userMap: Map; - readonly enabled: boolean; + private userMap: Map; + private entries: TokenEntry[]; + private readonly tokensPath?: string; + private refreshQueue: Promise = Promise.resolve(); + enabled: boolean; + + constructor(entries: TokenEntry[], opts: { tokensPath?: string } = {}) { + this.entries = entries.map((entry) => ({ ...entry })); + this.tokensPath = opts.tokensPath; + this.tokenMap = new Map(); + this.userMap = new Map(); + this.enabled = false; + this.rebuildMaps(); + } - constructor(entries: TokenEntry[]) { + private rebuildMaps(): void { this.tokenMap = new Map(); this.userMap = new Map(); - for (const entry of entries) { - this.tokenMap.set(entry.token, { userId: entry.id, role: entry.role }); + for (const entry of this.entries) { + this.tokenMap.set(entry.token, { userId: entry.id, role: entry.role, expiresAt: entry.expiresAt }); if (entry.username && entry.password) { this.userMap.set(entry.username, { password: entry.password, token: entry.token, ctx: { userId: entry.id, role: entry.role }, + expiresAt: entry.expiresAt, }); } } this.enabled = this.tokenMap.size > 0; } + get configFilePath(): string | null { + return this.tokensPath ?? null; + } + authenticate(req: Request): AuthContext | null { if (!this.enabled) return null; @@ -160,14 +189,15 @@ export class TokenAuthProvider implements AuthProvider { const provided = match[1]!; // Timing-safe: check against ALL tokens to prevent timing leaks - let found: AuthContext | null = null; + let found: (AuthContext & { expiresAt?: string }) | null = null; for (const [token, ctx] of this.tokenMap) { if (timingSafeEqual(provided, token)) { found = ctx; } } - return found; + if (!found || isExpired(found.expiresAt)) return null; + return { userId: found.userId, role: found.role }; } /** @@ -177,17 +207,107 @@ export class TokenAuthProvider implements AuthProvider { */ authenticateBasic(username: string, password: string): { ctx: AuthContext; token: string } | null { // Timing-safe: always iterate all entries to prevent user-enumeration timing leaks - let found: { ctx: AuthContext; token: string } | null = null; + let found: { ctx: AuthContext; token: string; expiresAt?: string } | null = null; for (const [uname, entry] of this.userMap) { const nameMatch = timingSafeEqual(username, uname); const passMatch = timingSafeEqual(password, entry.password); if (nameMatch && passMatch) { - found = { ctx: entry.ctx, token: entry.token }; + found = { ctx: entry.ctx, token: entry.token, expiresAt: entry.expiresAt }; } } + if (found && isExpired(found.expiresAt)) return null; return found; } + async reloadFromDisk(): Promise { + if (!this.tokensPath) { + throw new Error("Token provider has no tokens file to reload"); + } + const raw = await Bun.file(this.tokensPath).json(); + const config = validateTokensConfig(raw); + this.entries = config.tokens.map((entry) => ({ ...entry })); + this.rebuildMaps(); + } + + async refreshBearerToken( + provided: string, + opts: { now?: Date; refreshWindowMs?: number; ttlMs?: number } = {}, + ): Promise< + | { ok: true; token: string; expiresAt: string; userId: string; role: Role } + | { ok: false; status: number; message: string } + > { + return this.withRefreshLock(async () => { + if (!this.tokensPath) { + return { ok: false, status: 501, message: "Token refresh requires a tokens.json provider" }; + } + + await this.reloadFromDisk(); + const now = opts.now ?? new Date(); + const refreshWindowMs = opts.refreshWindowMs ?? resolveDurationEnv("MCP2CLI_TOKEN_REFRESH_WINDOW_MS", 24 * 60 * 60 * 1000); + const ttlMs = opts.ttlMs ?? resolveDurationEnv("MCP2CLI_TOKEN_TTL_MS", 30 * 24 * 60 * 60 * 1000); + let matchIndex = -1; + + for (const [index, entry] of this.entries.entries()) { + if (timingSafeEqual(provided, entry.token)) { + matchIndex = index; + } + } + + if (matchIndex < 0) { + return { ok: false, status: 401, message: "Invalid token" }; + } + + const entry = this.entries[matchIndex]!; + if (!entry.expiresAt) { + return { ok: false, status: 400, message: "Token has no expiry and is not refreshable" }; + } + const expiresAtMs = Date.parse(entry.expiresAt); + if (expiresAtMs <= now.getTime()) { + return { ok: false, status: 401, message: "Token is expired" }; + } + if (expiresAtMs - now.getTime() > refreshWindowMs) { + return { ok: false, status: 400, message: "Token is not near expiry" }; + } + + const token = `mcp2cli_${randomBytes(32).toString("base64url")}`; + const expiresAt = new Date(now.getTime() + ttlMs).toISOString(); + this.entries[matchIndex] = { ...entry, token, expiresAt }; + await this.persistTokens(); + this.rebuildMaps(); + + return { ok: true, token, expiresAt, userId: entry.id, role: entry.role }; + }); + } + + private async withRefreshLock(fn: () => Promise): Promise { + const previous = this.refreshQueue; + let release!: () => void; + this.refreshQueue = new Promise((resolve) => { + release = resolve; + }); + await previous.catch(() => {}); + try { + return await fn(); + } finally { + release(); + } + } + + private async persistTokens(): Promise { + if (!this.tokensPath) throw new Error("Token provider has no tokens file to write"); + const dir = dirname(this.tokensPath); + await mkdir(dir, { recursive: true }); + const tempPath = join(dir, `.${basename(this.tokensPath)}.${process.pid}.${Date.now()}.tmp`); + const fd = await open(tempPath, "wx", 0o600); + try { + await fd.writeFile(JSON.stringify({ tokens: this.entries }, null, 2) + "\n"); + await fd.sync(); + } finally { + await fd.close(); + } + await rename(tempPath, this.tokensPath); + } + /** * Load from legacy single-token env var. * The token gets admin role with userId "default". @@ -212,7 +332,7 @@ export class TokenAuthProvider implements AuthProvider { const raw = await file.json(); const config = validateTokensConfig(raw); log.info("tokens_loaded", { path, count: config.tokens.length }); - return new TokenAuthProvider(config.tokens); + return new TokenAuthProvider(config.tokens, { tokensPath: path }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); log.error("tokens_load_failed", { path, error: msg }); @@ -242,6 +362,17 @@ function getTokensPath(): string { return `${home}/.config/mcp2cli/tokens.json`; } +function isExpired(expiresAt: string | undefined): boolean { + return expiresAt !== undefined && Date.parse(expiresAt) <= Date.now(); +} + +function resolveDurationEnv(name: string, fallback: number): number { + const value = process.env[name]; + if (!value) return fallback; + const parsed = parseInt(value, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; +} + /** Timing-safe string comparison. */ function timingSafeEqual(a: string, b: string): boolean { const encoder = new TextEncoder(); diff --git a/src/daemon/auth.ts b/src/daemon/auth.ts index f9e6bc3..2400893 100644 --- a/src/daemon/auth.ts +++ b/src/daemon/auth.ts @@ -86,12 +86,15 @@ const PATH_PERMISSIONS: Array<{ pattern: RegExp; method: string; permission: str { pattern: /^\/shutdown$/, method: "POST", permission: "shutdown" }, // Management API { pattern: /^\/api\/services$/, method: "GET", permission: "list" }, + { pattern: /^\/api\/services\/discovery$/, method: "GET", permission: "list" }, { pattern: /^\/api\/services$/, method: "POST", permission: "add" }, + { pattern: /^\/api\/services\/export$/, method: "GET", permission: "import" }, { pattern: /^\/api\/services\/reload$/, method: "POST", permission: "reload" }, { pattern: /^\/api\/services\/import$/, method: "POST", permission: "import" }, { pattern: /^\/api\/services\/[^/]+$/, method: "PUT", permission: "update" }, { pattern: /^\/api\/services\/[^/]+$/, method: "DELETE", permission: "remove" }, { pattern: /^\/api\/services\/[^/]+\/status$/, method: "GET", permission: "status" }, + { pattern: /^\/api\/metrics\/user\/[^/]+$/, method: "GET", permission: "status" }, // Credential management API { pattern: /^\/api\/credentials$/, method: "GET", permission: "credentials-write" }, { pattern: /^\/api\/credentials\/resolve$/, method: "GET", permission: "credentials-read" }, diff --git a/src/daemon/config-manager.ts b/src/daemon/config-manager.ts index ebb877b..245cd63 100644 --- a/src/daemon/config-manager.ts +++ b/src/daemon/config-manager.ts @@ -21,6 +21,10 @@ export class ConfigManager { this.configPath = configPath ?? getConfigPath(); } + get configFilePath(): string { + return this.configPath; + } + /** Attach pool reference for connection lifecycle management. */ setPool(pool: ConnectionPool): void { this.pool = pool; @@ -31,6 +35,11 @@ export class ConfigManager { return structuredClone(this.config); } + /** Get a sanitized services config safe for client import/export. */ + getSanitizedServices(): ServicesConfig { + return sanitizeServicesConfig(this.config); + } + /** Get a single service config by name, or null if not found. */ getService(name: string): ServiceConfig | null { return this.config.services[name] ?? null; @@ -248,3 +257,83 @@ export class ConfigManagerError extends Error { this.name = "ConfigManagerError"; } } + +export function sanitizeServicesConfig(config: ServicesConfig): ServicesConfig { + const sanitized: ServicesConfig = { services: structuredClone(config.services) }; + + for (const svc of Object.values(sanitized.services)) { + svc.source = "remote"; + if ("url" in svc) { + svc.url = sanitizeUrl(svc.url); + } + if ("args" in svc && svc.args) { + svc.args = sanitizeArgs(svc.args); + } + if ("headers" in svc) { + svc.headers = {}; + } + if ("env" in svc) { + svc.env = {}; + } + if ("fallback" in svc && svc.fallback) { + if ("args" in svc.fallback && svc.fallback.args) { + svc.fallback.args = sanitizeArgs(svc.fallback.args); + } + if ("env" in svc.fallback) { + svc.fallback.env = {}; + } + } + } + + return sanitized; +} + +const SENSITIVE_ARG_PATTERN = /token|secret|password|api[_-]?key|auth|bearer|credential|private[_-]?key|access[_-]?key|session[_-]?id|cookie|passphrase/i; +const SENSITIVE_ARG_VALUE_PATTERN = /(?:authorization|cookie|set-cookie|x-api-key|api[_-]?key|token|bearer|password|secret|session[_-]?id)\s*[:=]\s*\S+|bearer\s+\S+/i; +const HEADER_ARG_VALUE_PATTERN = /(?:authorization|cookie|set-cookie|x-api-key)\s*:/i; + +function sanitizeUrl(raw: string): string { + try { + const url = new URL(raw); + url.username = ""; + url.password = ""; + url.search = ""; + url.hash = ""; + return url.toString(); + } catch { + return raw; + } +} + +function sanitizeArgs(args: string[]): string[] { + const sanitized: string[] = []; + let redactNext = false; + for (const arg of args) { + if (redactNext) { + sanitized.push("[REDACTED]"); + redactNext = false; + continue; + } + if (HEADER_ARG_VALUE_PATTERN.test(arg) || /\bbearer\s+\S+/i.test(arg)) { + sanitized.push("[REDACTED]"); + continue; + } + if (SENSITIVE_ARG_PATTERN.test(arg)) { + if (arg.includes("=")) { + sanitized.push(arg.replace(/=.*/, "=[REDACTED]")); + } else if (SENSITIVE_ARG_VALUE_PATTERN.test(arg)) { + sanitized.push("[REDACTED]"); + } else { + sanitized.push(arg); + redactNext = true; + } + continue; + } + if (SENSITIVE_ARG_VALUE_PATTERN.test(arg)) { + sanitized.push("[REDACTED]"); + continue; + } + sanitized.push(sanitizeUrl(arg)); + } + return sanitized; +} diff --git a/src/daemon/file-watch.ts b/src/daemon/file-watch.ts new file mode 100644 index 0000000..723d61a --- /dev/null +++ b/src/daemon/file-watch.ts @@ -0,0 +1,100 @@ +import { watch, type FSWatcher } from "node:fs"; +import { basename, dirname } from "node:path"; +import { createLogger } from "../logger/index.ts"; +import type { ConfigManager } from "./config-manager.ts"; +import type { CredentialManager } from "../credentials/index.ts"; +import type { TokenAuthProvider } from "./auth-provider.ts"; + +const log = createLogger("daemon:file-watch"); +const DEFAULT_DEBOUNCE_MS = 100; + +export interface FileWatchHandle { + close(): void; +} + +export function startConfigFileWatchers(opts: { + configManager: ConfigManager; + credentialManager: CredentialManager; + authProvider?: TokenAuthProvider; + debounceMs?: number; +}): FileWatchHandle { + const debounceMs = opts.debounceMs ?? DEFAULT_DEBOUNCE_MS; + const watchers: FSWatcher[] = []; + const timers = new Map>(); + + const schedule = (key: string, fn: () => Promise) => { + const existing = timers.get(key); + if (existing) clearTimeout(existing); + const timer = setTimeout(() => { + timers.delete(key); + fn().catch((err) => { + log.warn("auto_reload_failed", { + target: key, + error: err instanceof Error ? err.message : String(err), + }); + }); + }, debounceMs); + timers.set(key, timer); + }; + + const servicesWatcher = tryWatchPath(opts.configManager.configFilePath, () => { + schedule("services", async () => { + const diff = await opts.configManager.reloadFromDisk(); + log.info("services_auto_reloaded", diff); + }); + }); + if (servicesWatcher) watchers.push(servicesWatcher); + + const credentialsWatcher = tryWatchPath(opts.credentialManager.configFilePath, () => { + schedule("credentials", async () => { + await opts.credentialManager.reloadFromDisk(); + log.info("credentials_auto_reloaded"); + }); + }); + if (credentialsWatcher) watchers.push(credentialsWatcher); + + const tokenPath = opts.authProvider?.configFilePath; + if (tokenPath) { + const tokensWatcher = tryWatchPath(tokenPath, () => { + schedule("tokens", async () => { + await opts.authProvider!.reloadFromDisk(); + log.info("tokens_auto_reloaded"); + }); + }); + if (tokensWatcher) watchers.push(tokensWatcher); + } + + return { + close() { + for (const timer of timers.values()) { + clearTimeout(timer); + } + timers.clear(); + for (const watcher of watchers) { + watcher.close(); + } + }, + }; +} + +function watchPath(path: string, onChange: () => void): FSWatcher { + const dir = dirname(path); + const file = basename(path); + return watch(dir, (_eventType, changedName) => { + if (!changedName || changedName.toString() === file) { + onChange(); + } + }); +} + +function tryWatchPath(path: string, onChange: () => void): FSWatcher | null { + try { + return watchPath(path, onChange); + } catch (err) { + log.warn("file_watch_unavailable", { + path, + error: err instanceof Error ? err.message : String(err), + }); + return null; + } +} diff --git a/src/daemon/index.ts b/src/daemon/index.ts index 7944f93..91e1c65 100644 --- a/src/daemon/index.ts +++ b/src/daemon/index.ts @@ -16,6 +16,8 @@ import { MetricsCollector } from "./metrics.ts"; import { ConfigManager } from "./config-manager.ts"; import { TokenAuthProvider } from "./auth-provider.ts"; import { CredentialManager } from "../credentials/index.ts"; +import { startConfigFileWatchers } from "./file-watch.ts"; +import type { FileWatchHandle } from "./file-watch.ts"; const log = createLogger("daemon"); @@ -91,6 +93,7 @@ export async function startDaemon(): Promise { // Graceful shutdown function let isShuttingDown = false; + let fileWatchers: FileWatchHandle | null = null; const gracefulShutdown = async (): Promise => { if (isShuttingDown) return; isShuttingDown = true; @@ -104,6 +107,7 @@ export async function startDaemon(): Promise { try { // Stop accepting new connections, drain in-flight requests server.stop(); + fileWatchers?.close(); // Close all MCP connections (reuses McpTransport.close() multi-step shutdown) await pool.closeAll(); @@ -142,6 +146,12 @@ export async function startDaemon(): Promise { metrics, }); + fileWatchers = startConfigFileWatchers({ + configManager, + credentialManager, + authProvider, + }); + // Install signal handlers process.on("SIGTERM", () => void gracefulShutdown()); process.on("SIGINT", () => void gracefulShutdown()); diff --git a/src/daemon/metrics.ts b/src/daemon/metrics.ts index 6af7e30..dc802ed 100644 --- a/src/daemon/metrics.ts +++ b/src/daemon/metrics.ts @@ -18,12 +18,28 @@ interface RequestMetric { buckets: number[]; } +export interface UserMetricSummary { + userId: string; + totalRequests: number; + errorCount: number; + totalDurationMs: number; + requests: Array<{ + service: string; + tool: string; + count: number; + errorCount: number; + totalDurationMs: number; + avgDurationMs: number; + }>; +} + /** * Centralized metrics collector. * Thread-safe for single-threaded Bun runtime. */ export class MetricsCollector { private readonly requests = new Map(); + private readonly callerRequests = new Map(); private activeRequests = 0; private peakActiveRequests = 0; private totalRequests = 0; @@ -43,20 +59,60 @@ export class MetricsCollector { tool: string, success: boolean, durationMs: number, + caller?: string, ): void { this.activeRequests = Math.max(0, this.activeRequests - 1); this.totalRequests++; - const key = `${service}.${tool}`; - let metric = this.requests.get(key); + this.recordMetric(this.requests, metricKey(service, tool), success, durationMs); + if (caller) { + this.recordMetric(this.callerRequests, metricKey(service, tool, caller), success, durationMs); + } + } + + getUserBreakdown(userId: string): UserMetricSummary { + const requests: UserMetricSummary["requests"] = []; + let totalRequests = 0; + let errorCount = 0; + let totalDurationMs = 0; + + for (const [key, metric] of this.callerRequests) { + const parsed = parseMetricKey(key); + if (!parsed.caller || parsed.caller !== userId) continue; + totalRequests += metric.count; + errorCount += metric.errorCount; + totalDurationMs += metric.totalDurationMs; + requests.push({ + service: parsed.service, + tool: parsed.tool, + count: metric.count, + errorCount: metric.errorCount, + totalDurationMs: metric.totalDurationMs, + avgDurationMs: metric.count === 0 ? 0 : Math.round(metric.totalDurationMs / metric.count), + }); + } + + requests.sort((a, b) => b.count - a.count || a.service.localeCompare(b.service) || a.tool.localeCompare(b.tool)); + + return { + userId, + totalRequests, + errorCount, + totalDurationMs, + requests, + }; + } + + private recordMetric( + map: Map, + key: string, + success: boolean, + durationMs: number, + ): void { + let metric = map.get(key); if (!metric) { - metric = { - count: 0, - errorCount: 0, - totalDurationMs: 0, - buckets: new Array(DURATION_BUCKETS.length).fill(0) as number[], - }; - this.requests.set(key, metric); + metric = newMetric(); + map.set(key, metric); } metric.count++; @@ -82,7 +138,11 @@ export class MetricsCollector { * @param poolSize Current connection pool size * @param poolServices List of connected service names */ - render(poolSize: number, poolServices: string[]): string { + render( + poolSize: number, + poolServices: string[], + opts: { includeCallerMetrics?: boolean } = {}, + ): string { const lines: string[] = []; // -- Process metrics -- @@ -131,19 +191,29 @@ export class MetricsCollector { lines.push("# TYPE mcp2cli_requests_duration_ms_sum counter"); for (const [key, metric] of this.requests) { - const [service, tool] = key.split(".", 2); - const labels = `service="${service}",tool="${tool}"`; + const { service, tool } = parseMetricKey(key); + const labels = `service="${escapeLabelValue(service)}",tool="${escapeLabelValue(tool)}"`; lines.push(`mcp2cli_requests_total{${labels}} ${metric.count}`); lines.push(`mcp2cli_requests_errors_total{${labels}} ${metric.errorCount}`); lines.push(`mcp2cli_requests_duration_ms_sum{${labels}} ${metric.totalDurationMs.toFixed(0)}`); } + if (opts.includeCallerMetrics) { + for (const [key, metric] of this.callerRequests) { + const { service, tool, caller } = parseMetricKey(key); + const labels = `service="${escapeLabelValue(service)}",tool="${escapeLabelValue(tool)}",caller="${escapeLabelValue(caller ?? "unknown")}"`; + lines.push(`mcp2cli_requests_total{${labels}} ${metric.count}`); + lines.push(`mcp2cli_requests_errors_total{${labels}} ${metric.errorCount}`); + lines.push(`mcp2cli_requests_duration_ms_sum{${labels}} ${metric.totalDurationMs.toFixed(0)}`); + } + } + // -- Request duration histogram -- lines.push("# HELP mcp2cli_request_duration_ms Request duration histogram"); lines.push("# TYPE mcp2cli_request_duration_ms histogram"); for (const [key, metric] of this.requests) { - const [service, tool] = key.split(".", 2); - const labels = `service="${service}",tool="${tool}"`; + const { service, tool } = parseMetricKey(key); + const labels = `service="${escapeLabelValue(service)}",tool="${escapeLabelValue(tool)}"`; for (let i = 0; i < DURATION_BUCKETS.length; i++) { lines.push(`mcp2cli_request_duration_ms_bucket{${labels},le="${DURATION_BUCKETS[i]}"} ${metric.buckets[i]}`); } @@ -151,6 +221,18 @@ export class MetricsCollector { lines.push(`mcp2cli_request_duration_ms_count{${labels}} ${metric.count}`); lines.push(`mcp2cli_request_duration_ms_sum{${labels}} ${metric.totalDurationMs.toFixed(0)}`); } + if (opts.includeCallerMetrics) { + for (const [key, metric] of this.callerRequests) { + const { service, tool, caller } = parseMetricKey(key); + const labels = `service="${escapeLabelValue(service)}",tool="${escapeLabelValue(tool)}",caller="${escapeLabelValue(caller ?? "unknown")}"`; + for (let i = 0; i < DURATION_BUCKETS.length; i++) { + lines.push(`mcp2cli_request_duration_ms_bucket{${labels},le="${DURATION_BUCKETS[i]}"} ${metric.buckets[i]}`); + } + lines.push(`mcp2cli_request_duration_ms_bucket{${labels},le="+Inf"} ${metric.count}`); + lines.push(`mcp2cli_request_duration_ms_count{${labels}} ${metric.count}`); + lines.push(`mcp2cli_request_duration_ms_sum{${labels}} ${metric.totalDurationMs.toFixed(0)}`); + } + } // -- Auth metrics -- lines.push("# HELP mcp2cli_auth_failures_total Total auth failures"); @@ -167,3 +249,25 @@ export class MetricsCollector { } } + +function newMetric(): RequestMetric { + return { + count: 0, + errorCount: 0, + totalDurationMs: 0, + buckets: new Array(DURATION_BUCKETS.length).fill(0) as number[], + }; +} + +function metricKey(service: string, tool: string, caller?: string): string { + return JSON.stringify(caller ? [service, tool, caller] : [service, tool]); +} + +function parseMetricKey(key: string): { service: string; tool: string; caller?: string } { + const parsed = JSON.parse(key) as [string, string, string?]; + return { service: parsed[0], tool: parsed[1], caller: parsed[2] }; +} + +function escapeLabelValue(value: string): string { + return value.replace(/\\/g, "\\\\").replace(/\n/g, "\\n").replace(/"/g, '\\"'); +} diff --git a/src/daemon/pool.ts b/src/daemon/pool.ts index 7262cc1..553d8e0 100644 --- a/src/daemon/pool.ts +++ b/src/daemon/pool.ts @@ -11,6 +11,8 @@ import { ConnectionError } from "../connection/errors.ts"; import type { McpConnection } from "../connection/types.ts"; import type { ServicesConfig, HttpService, WebSocketService } from "../config/index.ts"; import { createLogger } from "../logger/index.ts"; +import { resolveServiceSecretRefs, VaultwardenSecretResolver } from "../secrets/index.ts"; +import type { SecretResolver } from "../secrets/index.ts"; import { checkDriftOnConnect } from "./drift-hook.ts"; import { extractPolicy } from "../access/filter.ts"; import { @@ -33,6 +35,7 @@ interface PoolEntry { interface PoolOptions { maxSize?: number; healthCheckTimeoutMs?: number; + secretResolver?: SecretResolver; } /** @@ -45,11 +48,13 @@ export class ConnectionPool { private pending = new Map>(); private readonly _maxSize: number; private readonly _healthCheckTimeoutMs: number; + private readonly secretResolver: SecretResolver; constructor(options?: PoolOptions) { const envMax = parseInt(process.env.MCP2CLI_POOL_MAX ?? "", 10); this._maxSize = options?.maxSize ?? (Number.isNaN(envMax) ? DEFAULT_POOL_MAX : envMax); this._healthCheckTimeoutMs = options?.healthCheckTimeoutMs ?? DEFAULT_HEALTH_CHECK_TIMEOUT_MS; + this.secretResolver = options?.secretResolver ?? new VaultwardenSecretResolver(); } /** Maximum number of concurrent connections allowed. */ @@ -110,8 +115,8 @@ export class ConnectionPool { } // Use override if provided, otherwise look up by service name - const serviceConfig = serviceConfigOverride ?? config.services[baseServiceName]; - if (!serviceConfig) { + const rawServiceConfig = serviceConfigOverride ?? config.services[baseServiceName]; + if (!rawServiceConfig) { throw new ConnectionError( `Service not found in config: ${baseServiceName}`, "service_not_configured", @@ -119,22 +124,29 @@ export class ConnectionPool { } // Create connection promise and store in pending map log.info("connecting", { service: poolKey }); - let connectFn: () => Promise; - if (serviceConfig.backend === "http") { - connectFn = () => this.connectHttpWithFallback(poolKey, baseServiceName, serviceConfig); - } else if (serviceConfig.backend === "websocket") { - connectFn = () => this.connectWebSocketWithFallback(poolKey, baseServiceName, serviceConfig); - } else if (serviceConfig.backend === "stdio") { - connectFn = () => connectToService(serviceConfig); - } else { - const backend = (serviceConfig as { backend: string }).backend; - throw new ConnectionError( - `Unsupported backend for service ${poolKey}: ${backend}`, - "unsupported_backend", + const connectPromise = (async () => { + const serviceConfig = await resolveServiceSecretRefs( + baseServiceName, + rawServiceConfig, + this.secretResolver, ); - } - const connectPromise = connectFn().then( - (connection) => { + let connection: McpConnection; + if (serviceConfig.backend === "http") { + connection = await this.connectHttpWithFallback(poolKey, baseServiceName, serviceConfig); + } else if (serviceConfig.backend === "websocket") { + connection = await this.connectWebSocketWithFallback(poolKey, baseServiceName, serviceConfig); + } else if (serviceConfig.backend === "stdio") { + connection = await connectToService(serviceConfig); + } else { + const backend = (serviceConfig as { backend: string }).backend; + throw new ConnectionError( + `Unsupported backend for service ${poolKey}: ${backend}`, + "unsupported_backend", + ); + } + return { connection, serviceConfig }; + })().then( + ({ connection, serviceConfig }) => { this.connections.set(poolKey, { connection, connectedAt: Date.now(), diff --git a/src/daemon/server.ts b/src/daemon/server.ts index 42328f1..3f042ae 100644 --- a/src/daemon/server.ts +++ b/src/daemon/server.ts @@ -152,6 +152,9 @@ export function createDaemonServer(opts: DaemonServerOptions) { idleTimer.onRequestStart(); metrics.onRequestStart(); const startTime = performance.now(); + const caller = authCtx + ? { userId: authCtx.userId, role: authCtx.role } + : undefined; let callService = "unknown"; let callTool = "unknown"; let success = false; @@ -168,6 +171,7 @@ export function createDaemonServer(opts: DaemonServerOptions) { // 1/4: Request received from client reqLog.info("request_in", { + ...caller, service: callService, tool: callTool, params: sanitizeParams(body.params ?? {}), @@ -210,6 +214,7 @@ export function createDaemonServer(opts: DaemonServerOptions) { // 2/4: Forwarding request to MCP server reqLog.info("mcp_call_start", { + ...caller, service: callService, tool: resolvedTool, transport: callTransport, @@ -240,6 +245,7 @@ export function createDaemonServer(opts: DaemonServerOptions) { // 3/4: Response received from MCP server reqLog.info("mcp_call_end", { + ...caller, service: callService, tool: resolvedTool, mcpDuration, @@ -262,6 +268,7 @@ export function createDaemonServer(opts: DaemonServerOptions) { // 3/4 (error path): MCP server error reqLog.warn("mcp_call_end", { + ...caller, service: callService, tool: callResolvedTool ?? callTool, success: false, @@ -278,6 +285,7 @@ export function createDaemonServer(opts: DaemonServerOptions) { // 4/4: Response sent to client reqLog.info("response_out", { + ...caller, service: callService, tool: callTool, totalDuration: duration, @@ -285,9 +293,11 @@ export function createDaemonServer(opts: DaemonServerOptions) { ...(callError ? { error: callError } : {}), }); - metrics.onRequestEnd(callService, callTool, success, duration); + metrics.onRequestEnd(callService, callTool, success, duration, caller?.userId); auditToolCall({ path: "daemon", + userId: caller?.userId, + role: caller?.role, service: callService, tool: callTool, resolvedTool: callResolvedTool, @@ -349,15 +359,10 @@ export function createDaemonServer(opts: DaemonServerOptions) { // GET /health -- health check (auth-exempt) if (path === "/health" && req.method === "GET") { const mem = process.memoryUsage(); - const currentConfig = getConfig(); - const configuredServices = Object.keys(currentConfig.services); - const connectedServices = pool.baseServiceNames; return Response.json({ status: "ok", version: pkg.version, uptime: process.uptime(), - configuredServices, - connectedServices, activeConnections: pool.size, memory: { rss: mem.rss, @@ -367,14 +372,36 @@ export function createDaemonServer(opts: DaemonServerOptions) { }); } + // GET /api/services/discovery -- authenticated lightweight service inventory for remote clients + if (path === "/api/services/discovery" && req.method === "GET") { + const currentConfig = getConfig(); + return Response.json({ + success: true, + version: pkg.version, + configuredServices: Object.keys(currentConfig.services), + connectedServices: pool.baseServiceNames, + }); + } + // GET /metrics -- Prometheus metrics (auth-exempt) if (path === "/metrics" && req.method === "GET") { - const body = metrics.render(pool.size, pool.baseServiceNames); + const body = metrics.render(pool.size, pool.baseServiceNames, { + includeCallerMetrics: process.env.MCP2CLI_METRICS_INCLUDE_CALLER === "1", + }); return new Response(body, { headers: { "Content-Type": "text/plain; version=0.0.4; charset=utf-8" }, }); } + const userMetricsMatch = path.match(/^\/api\/metrics\/user\/([^/]+)$/); + if (userMetricsMatch && req.method === "GET") { + const userId = decodeURIComponent(userMetricsMatch[1]!); + if (authCtx && authCtx.role !== "admin" && authCtx.userId !== userId) { + return errorResponse("AUTH_ERROR", "Permission denied: cannot read metrics for another user", undefined, 403); + } + return Response.json({ success: true, ...metrics.getUserBreakdown(userId) }); + } + // POST /shutdown -- graceful shutdown if (path === "/shutdown" && req.method === "POST") { // Return response FIRST, then schedule shutdown @@ -408,6 +435,28 @@ export function createDaemonServer(opts: DaemonServerOptions) { } } + // POST /api/auth/refresh -- rotate a valid near-expiry token from tokens.json + if (path === "/api/auth/refresh" && req.method === "POST") { + if (!(authProvider instanceof TokenAuthProvider)) { + return errorResponse("AUTH_ERROR", "Token refresh not supported with current auth provider", undefined, 501); + } + const token = extractBearerToken(req); + if (!token) { + return errorResponse("AUTH_ERROR", "Missing bearer token", undefined, 401); + } + const result = await authProvider.refreshBearerToken(token); + if (!result.ok) { + return errorResponse("AUTH_ERROR", result.message, undefined, result.status); + } + return Response.json({ + success: true, + token: result.token, + expiresAt: result.expiresAt, + userId: result.userId, + role: result.role, + }); + } + // GET /api/auth/me -- returns current user identity and role if (path === "/api/auth/me" && req.method === "GET") { return Response.json({ @@ -438,6 +487,11 @@ export function createDaemonServer(opts: DaemonServerOptions) { return Response.json({ success: true, services }); } + // GET /api/services/export -- sanitized services.json for remote clients + if (path === "/api/services/export" && req.method === "GET") { + return Response.json(configManager.getSanitizedServices()); + } + // POST /api/services -- add a service { name, config } if (path === "/api/services" && req.method === "POST") { try { @@ -580,6 +634,12 @@ export function createDaemonServer(opts: DaemonServerOptions) { }); } +function extractBearerToken(req: Request): string | null { + const authHeader = req.headers.get("authorization"); + const match = authHeader?.match(/^Bearer\s+(.+)$/i); + return match?.[1] ?? null; +} + /** Shared error handler for /call, /list-tools, /schema endpoints */ function handleEndpointError( err: unknown, diff --git a/src/logger/audit.ts b/src/logger/audit.ts index fa23a88..243bdb1 100644 --- a/src/logger/audit.ts +++ b/src/logger/audit.ts @@ -41,6 +41,8 @@ const SENSITIVE_PATTERNS = new RegExp(`(?:${SENSITIVE_KEYS})`, "i"); export interface AuditEntry { timestamp: string; path: "daemon" | "cli"; + userId?: string; + role?: string; service: string; tool: string; resolvedTool?: string; @@ -262,6 +264,8 @@ export function writeAuditEntry(entry: AuditEntry): void { */ export function auditToolCall(opts: { path: "daemon" | "cli"; + userId?: string; + role?: string; service: string; tool: string; resolvedTool?: string; @@ -275,6 +279,8 @@ export function auditToolCall(opts: { writeAuditEntry({ timestamp: new Date().toISOString(), path: opts.path, + userId: opts.userId, + role: opts.role, service: opts.service, tool: opts.tool, resolvedTool: opts.resolvedTool, diff --git a/src/process/client.ts b/src/process/client.ts index 12282fc..ff820fb 100644 --- a/src/process/client.ts +++ b/src/process/client.ts @@ -10,6 +10,7 @@ import { constants } from "node:fs"; import { getDaemonPaths, getRemoteConfig } from "../daemon/paths.ts"; import { ConnectionError } from "../connection/errors.ts"; import { getDaemonStatus, cleanStaleDaemon } from "./liveness.ts"; +import { getRemoteServiceAvailability } from "./remote-discovery.ts"; import type { ServiceSource, ServicesConfig } from "../config/index.ts"; import type { DaemonPaths } from "../daemon/types.ts"; import type { @@ -19,7 +20,12 @@ import type { DaemonResponse, } from "../daemon/types.ts"; -const STARTUP_TIMEOUT_MS = 10_000; +function readPositiveIntEnv(name: string, fallback: number): number { + const parsed = parseInt(process.env[name] ?? "", 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; +} + +const STARTUP_TIMEOUT_MS = readPositiveIntEnv("MCP2CLI_STARTUP_TIMEOUT", 10_000); const STARTUP_POLL_MS = 50; const REQUEST_TIMEOUT_MS = 60_000; const REMOTE_CONNECT_TIMEOUT_MS = 10_000; @@ -27,6 +33,7 @@ const STALE_LOCK_THRESHOLD_MS = 30_000; /** Cached local token to avoid re-reading tokens.json on every request. */ let cachedLocalToken: string | undefined; +let cachedLocalTokenExpiresAt: string | undefined; let localTokenResolved = false; /** @@ -45,6 +52,7 @@ async function getLocalToken(): Promise { const envToken = process.env.MCP2CLI_AUTH_TOKEN ?? process.env.MCP_TOKEN; if (envToken && !process.env.MCP2CLI_REMOTE_URL && !process.env.MCP_HOST) { cachedLocalToken = envToken; + cachedLocalTokenExpiresAt = undefined; return cachedLocalToken; } @@ -54,10 +62,11 @@ async function getLocalToken(): Promise { try { const file = Bun.file(tokensPath); if (await file.exists()) { - const raw = await file.json() as { tokens?: Array<{ token: string; role: string }> }; - // Use the first admin token for local socket auth - const adminEntry = raw.tokens?.find((t) => t.role === "admin"); + const raw = await file.json() as { tokens?: Array<{ token: string; role: string; expiresAt?: string }> }; + // Use the first non-expired admin token for local socket auth. + const adminEntry = raw.tokens?.find((t) => t.role === "admin" && !isExpiredToken(t.expiresAt)); cachedLocalToken = adminEntry?.token; + cachedLocalTokenExpiresAt = adminEntry?.expiresAt; } } catch { // tokens.json missing or malformed -- auth may be disabled @@ -65,6 +74,70 @@ async function getLocalToken(): Promise { return cachedLocalToken; } +function isExpiredToken(expiresAt: string | undefined): boolean { + if (!expiresAt) return false; + const expiresAtMs = Date.parse(expiresAt); + return Number.isNaN(expiresAtMs) || expiresAtMs <= Date.now(); +} + +async function buildLocalHeaders(): Promise> { + const headers: Record = { + "Content-Type": "application/json", + }; + const localToken = await getLocalToken(); + if (localToken) { + headers["Authorization"] = `Bearer ${localToken}`; + } + return headers; +} + +async function refreshLocalToken(paths: DaemonPaths, token: string): Promise { + const response = await fetch("http://localhost/api/auth/refresh", { + unix: paths.socketPath, + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${token}`, + }, + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }).catch(() => null); + if (!response?.ok) { + return reloadLocalTokenIfChanged(token); + } + const body = await response.json().catch(() => null) as { token?: unknown; expiresAt?: unknown } | null; + if (typeof body?.token !== "string") return reloadLocalTokenIfChanged(token); + cachedLocalToken = body.token; + cachedLocalTokenExpiresAt = typeof body.expiresAt === "string" ? body.expiresAt : undefined; + localTokenResolved = true; + return true; +} + +async function reloadLocalTokenIfChanged(previousToken: string): Promise { + clearLocalTokenCache(); + const latestToken = await getLocalToken(); + return latestToken !== undefined && latestToken !== previousToken; +} + +async function refreshLocalTokenIfNearExpiry(paths: DaemonPaths): Promise { + const token = await getLocalToken(); + if (!token || !cachedLocalTokenExpiresAt) return; + const expiresAtMs = Date.parse(cachedLocalTokenExpiresAt); + if (Number.isNaN(expiresAtMs)) return; + const refreshWindowMs = parseInt(process.env.MCP2CLI_TOKEN_REFRESH_WINDOW_MS ?? String(24 * 60 * 60 * 1000), 10); + const windowMs = Number.isFinite(refreshWindowMs) && refreshWindowMs > 0 + ? refreshWindowMs + : 24 * 60 * 60 * 1000; + if (expiresAtMs - Date.now() <= windowMs) { + await refreshLocalToken(paths, token); + } +} + +export function clearLocalTokenCache(): void { + cachedLocalToken = undefined; + cachedLocalTokenExpiresAt = undefined; + localTokenResolved = false; +} + /** * Start the daemon as a background process. * Detects dev vs compiled mode for correct spawn args. @@ -195,6 +268,10 @@ export async function ensureDaemon(paths: DaemonPaths): Promise { */ let cachedConfig: ServicesConfig | null = null; +export function clearClientConfigCache(): void { + cachedConfig = null; +} + async function getLocalConfig(): Promise { if (!cachedConfig) { try { @@ -213,11 +290,23 @@ async function getLocalConfig(): Promise { * - "remote-local" when MCP2CLI_REMOTE_URL is set * - "local" when no remote URL */ -async function resolveSource(serviceName: string | undefined): Promise { +export async function resolveSource(serviceName: string | undefined): Promise { if (!serviceName) return undefined; const config = await getLocalConfig(); const svc = config?.services[serviceName]; - return svc?.source; + if (svc?.source) return svc.source; + + if (svc?.platforms && svc.platforms.length > 0) { + if (svc.platforms.includes(process.platform)) return "local"; + const availability = await getRemoteServiceAvailability(serviceName); + return availability === "hosted" ? "remote" : "local"; + } + + const availability = await getRemoteServiceAvailability(serviceName); + if (availability === "hosted") return svc ? undefined : "remote"; + if (svc || availability === "not-hosted") return "local"; + + return undefined; } async function fetchLocal( @@ -226,13 +315,8 @@ async function fetchLocal( ): Promise { const paths = getDaemonPaths(); await ensureDaemon(paths); - const headers: Record = { - "Content-Type": "application/json", - }; - const localToken = await getLocalToken(); - if (localToken) { - headers["Authorization"] = `Bearer ${localToken}`; - } + await refreshLocalTokenIfNearExpiry(paths); + const headers = await buildLocalHeaders(); const response = await fetch(`http://localhost${path}`, { unix: paths.socketPath, method: "POST", @@ -240,6 +324,23 @@ async function fetchLocal( body: body !== undefined ? JSON.stringify(body) : undefined, signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), }); + + const localToken = headers.Authorization?.replace(/^Bearer\s+/i, ""); + if (response.status === 401 && localToken && path !== "/api/auth/refresh") { + const refreshed = await refreshLocalToken(paths, localToken); + if (refreshed) { + const retryHeaders = await buildLocalHeaders(); + const retry = await fetch(`http://localhost${path}`, { + unix: paths.socketPath, + method: "POST", + headers: retryHeaders, + body: body !== undefined ? JSON.stringify(body) : undefined, + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }); + return (await retry.json()) as DaemonResponse; + } + } + return (await response.json()) as DaemonResponse; } @@ -299,11 +400,11 @@ async function fetchRemote( /** * Shared fetch helper with per-service routing. - * Routes based on the service's "source" field in local config: - * - "local": always use local daemon - * - "remote": always use remote daemon - * - "remote-local": try remote, fall back to local on failure - * Default (no source set): "remote-local" if MCP2CLI_REMOTE_URL is set, else "local". + * Routes based on explicit source, platform support, and remote discovery. + * Missing local services are remote only when discovery positively hosts them. + * Platform-disallowed services use remote only when hosted remotely. + * "remote-local" falls back to local for connection failures, but never for + * remote auth failures. */ async function fetchDaemon( path: string, @@ -329,7 +430,10 @@ async function fetchDaemon( // "remote-local": try remote, fall back to local try { return await fetchRemote(path, body); - } catch { + } catch (err) { + if (isRemoteAuthError(err)) { + throw err; + } return await fetchLocal(path, body); } } catch (err) { @@ -345,6 +449,10 @@ async function fetchDaemon( } } +function isRemoteAuthError(err: unknown): boolean { + return err instanceof Error && err.message.startsWith("Remote auth failed"); +} + /** * Send a tool call request to the daemon. */ @@ -384,13 +492,8 @@ export async function fetchDaemonApi( ): Promise { const paths = getDaemonPaths(); await ensureDaemon(paths); - const headers: Record = { - "Content-Type": "application/json", - }; - const localToken = await getLocalToken(); - if (localToken) { - headers["Authorization"] = `Bearer ${localToken}`; - } + await refreshLocalTokenIfNearExpiry(paths); + const headers = await buildLocalHeaders(); const response = await fetch(`http://localhost${path}`, { unix: paths.socketPath, method, @@ -398,5 +501,22 @@ export async function fetchDaemonApi( body: body !== undefined ? JSON.stringify(body) : undefined, signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), }); + + const localToken = headers.Authorization?.replace(/^Bearer\s+/i, ""); + if (response.status === 401 && localToken && path !== "/api/auth/refresh") { + const refreshed = await refreshLocalToken(paths, localToken); + if (refreshed) { + const retryHeaders = await buildLocalHeaders(); + const retry = await fetch(`http://localhost${path}`, { + unix: paths.socketPath, + method, + headers: retryHeaders, + body: body !== undefined ? JSON.stringify(body) : undefined, + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }); + return await retry.json(); + } + } + return await response.json(); } diff --git a/src/process/index.ts b/src/process/index.ts index 1ec7ccb..661d3c1 100644 --- a/src/process/index.ts +++ b/src/process/index.ts @@ -3,6 +3,8 @@ export { listToolsViaDaemon, getSchemaViaDaemon, fetchDaemonApi, + clearClientConfigCache, + resolveSource, } from "./client.ts"; export { getDaemonStatus, @@ -10,4 +12,11 @@ export { cleanStaleDaemon, checkRemoteHealth, } from "./liveness.ts"; +export { + clearRemoteServiceCache, + getRemoteServiceAvailability, + getRemoteServiceNames, + getRemoteServiceSnapshot, +} from "./remote-discovery.ts"; +export type { RemoteServiceAvailability } from "./remote-discovery.ts"; export type { DaemonStatus } from "./types.ts"; diff --git a/src/process/remote-discovery.ts b/src/process/remote-discovery.ts new file mode 100644 index 0000000..c89126d --- /dev/null +++ b/src/process/remote-discovery.ts @@ -0,0 +1,103 @@ +import { getRemoteConfig } from "../daemon/paths.ts"; + +const DEFAULT_REMOTE_SERVICE_CACHE_TTL_MS = 60_000; + +export type RemoteServiceAvailability = + | "hosted" + | "not-hosted" + | "unknown" + | "no-remote"; + +interface RemoteServiceSnapshot { + success?: boolean; + configuredServices?: string[]; + version?: string; +} + +let cachedSnapshot: + | { + cacheKey: string; + expiresAt: number; + snapshot: RemoteServiceSnapshot | null; + } + | null = null; + +function cacheTtlMs(): number { + const raw = process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS; + if (!raw) return DEFAULT_REMOTE_SERVICE_CACHE_TTL_MS; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed >= 0 + ? parsed + : DEFAULT_REMOTE_SERVICE_CACHE_TTL_MS; +} + +function parseSnapshot(data: unknown): RemoteServiceSnapshot | null { + if (!data || typeof data !== "object") return null; + const obj = data as Record; + const success = typeof obj.success === "boolean" ? obj.success : undefined; + const configuredServices = Array.isArray(obj.configuredServices) + ? obj.configuredServices.filter((svc): svc is string => typeof svc === "string") + : undefined; + const version = typeof obj.version === "string" ? obj.version : undefined; + return { success, configuredServices, version }; +} + +export function clearRemoteServiceCache(): void { + cachedSnapshot = null; +} + +export async function getRemoteServiceSnapshot(): Promise { + const remote = getRemoteConfig(); + if (!remote) return null; + + const cacheKey = `${remote.url}\0${remote.token ?? ""}`; + const now = Date.now(); + if (cachedSnapshot && cachedSnapshot.cacheKey === cacheKey && cachedSnapshot.expiresAt > now) { + return cachedSnapshot.snapshot; + } + + let snapshot: RemoteServiceSnapshot | null = null; + try { + const headers: Record = {}; + if (remote.token) { + headers.Authorization = `Bearer ${remote.token}`; + } + const response = await fetch(`${remote.url.replace(/\/$/, "")}/api/services/discovery`, { + method: "GET", + headers, + signal: AbortSignal.timeout(5_000), + }); + if (response.ok) { + snapshot = parseSnapshot(await response.json()); + } + } catch { + snapshot = null; + } + + if (!snapshot?.configuredServices) { + cachedSnapshot = null; + return null; + } + + cachedSnapshot = { + cacheKey, + expiresAt: now + cacheTtlMs(), + snapshot, + }; + return snapshot; +} + +export async function getRemoteServiceNames(): Promise { + const snapshot = await getRemoteServiceSnapshot(); + return snapshot?.configuredServices ?? []; +} + +export async function getRemoteServiceAvailability( + serviceName: string | undefined, +): Promise { + if (!serviceName) return "unknown"; + if (!getRemoteConfig()) return "no-remote"; + const snapshot = await getRemoteServiceSnapshot(); + if (!snapshot?.configuredServices) return "unknown"; + return snapshot.configuredServices.includes(serviceName) ? "hosted" : "not-hosted"; +} diff --git a/src/secrets/index.ts b/src/secrets/index.ts new file mode 100644 index 0000000..eff5096 --- /dev/null +++ b/src/secrets/index.ts @@ -0,0 +1,7 @@ +export { + hasSecretRefs, + resolveServiceSecretRefs, + SecretResolutionError, + VaultwardenSecretResolver, +} from "./refs.ts"; +export type { SecretResolver } from "./refs.ts"; diff --git a/src/secrets/refs.ts b/src/secrets/refs.ts new file mode 100644 index 0000000..34393c1 --- /dev/null +++ b/src/secrets/refs.ts @@ -0,0 +1,207 @@ +import { createLogger } from "../logger/index.ts"; +import type { ServiceConfig } from "../config/index.ts"; + +const log = createLogger("secret-refs"); +const SECRET_REF_PATTERN = /\$\{secret:([^}]+)\}/g; + +export interface SecretResolver { + resolve(ref: string): Promise; +} + +export function hasSecretRefs(value: unknown): boolean { + if (typeof value === "string") { + return value.includes("${secret:"); + } + if (Array.isArray(value)) { + return value.some(hasSecretRefs); + } + if (value && typeof value === "object") { + return Object.values(value).some(hasSecretRefs); + } + return false; +} + +export async function resolveServiceSecretRefs( + serviceName: string, + serviceConfig: ServiceConfig, + resolver: SecretResolver, +): Promise { + if (!hasSecretRefs(serviceConfig)) return serviceConfig; + const resolved = await resolveValue(serviceConfig, resolver) as ServiceConfig; + log.info("secret_refs_resolved", { service: serviceName }); + return resolved; +} + +async function resolveValue(value: unknown, resolver: SecretResolver): Promise { + if (typeof value === "string") { + return resolveString(value, resolver); + } + if (Array.isArray(value)) { + return Promise.all(value.map((item) => resolveValue(item, resolver))); + } + if (value && typeof value === "object") { + const entries = await Promise.all( + Object.entries(value).map(async ([key, nested]) => [key, await resolveValue(nested, resolver)] as const), + ); + return Object.fromEntries(entries); + } + return value; +} + +async function resolveString(value: string, resolver: SecretResolver): Promise { + const matches = [...value.matchAll(SECRET_REF_PATTERN)]; + if (value.includes("${secret:") && matches.length === 0) { + throw new SecretResolutionError("Malformed secret reference"); + } + if (matches.length === 0) return value; + + let resolved = value; + for (const match of matches) { + const full = match[0]; + const ref = match[1]?.trim(); + if (!ref) { + throw new SecretResolutionError("Empty secret reference"); + } + const secretValue = await resolver.resolve(ref); + resolved = resolved.replace(full, () => secretValue); + } + return resolved; +} + +export class VaultwardenSecretResolver implements SecretResolver { + private readonly cache = new Map(); + + async resolve(ref: string): Promise { + const cached = this.cache.get(ref); + if (cached !== undefined) return cached; + + const parsed = parseSecretRef(ref); + const credential = await fetchVaultwardenCredential(parsed.query); + const value = extractSecretValue(credential, parsed.field); + if (typeof value !== "string" || value.length === 0) { + throw new SecretResolutionError(`Vaultwarden secret ref did not resolve a string value: ${redactRef(ref)}`); + } + this.cache.set(ref, value); + return value; + } +} + +export class SecretResolutionError extends Error { + constructor(message: string) { + super(message); + this.name = "SecretResolutionError"; + } +} + +function parseSecretRef(ref: string): { query: string; field?: string } { + const [queryPart, fieldPart] = ref.split("#", 2); + const query = queryPart?.trim(); + const field = fieldPart?.trim(); + if (!query) { + throw new SecretResolutionError("Secret reference is missing a query"); + } + return field ? { query, field } : { query }; +} + +async function fetchVaultwardenCredential(query: string): Promise { + const command = process.env.MCP2CLI_VAULTWARDEN_COMMAND ?? "mcp2cli"; + const commandArgs = parseCommandArgs(process.env.MCP2CLI_VAULTWARDEN_COMMAND_ARGS); + const timeoutMs = resolveTimeoutMs(); + const proc = Bun.spawn([ + command, + ...commandArgs, + "vaultwarden-secrets", + "get_credential", + "--params", + JSON.stringify({ query }), + ], { + stdout: "pipe", + stderr: "pipe", + env: { + ...process.env, + MCP2CLI_NO_DAEMON: process.env.MCP2CLI_VAULTWARDEN_USE_DAEMON === "1" ? "" : "1", + }, + }); + let timedOut = false; + const timeout = setTimeout(() => { + timedOut = true; + proc.kill("SIGKILL"); + }, timeoutMs); + + const [stdout, , exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + clearTimeout(timeout); + + if (exitCode !== 0) { + throw new SecretResolutionError( + `Vaultwarden lookup failed for ${redactRef(query)}${timedOut ? " (timeout)" : ""}`, + ); + } + + try { + const parsed = JSON.parse(stdout); + return unwrapMcpResult(parsed); + } catch { + throw new SecretResolutionError(`Vaultwarden lookup returned non-JSON output for ${redactRef(query)}`); + } +} + +function parseCommandArgs(raw: string | undefined): string[] { + if (!raw?.trim()) return []; + try { + const parsed = JSON.parse(raw) as unknown; + if (Array.isArray(parsed) && parsed.every((item) => typeof item === "string")) { + return parsed; + } + } catch { + // Fall through to whitespace split for simple local use. + } + return raw.split(/\s+/).filter(Boolean); +} + +function resolveTimeoutMs(): number { + const raw = process.env.MCP2CLI_VAULTWARDEN_TIMEOUT_MS; + if (!raw) return 10_000; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : 10_000; +} + +function unwrapMcpResult(value: unknown): unknown { + if (value && typeof value === "object" && "success" in value && "result" in value) { + return (value as { result: unknown }).result; + } + return value; +} + +function extractSecretValue(credential: unknown, field: string | undefined): unknown { + if (field) { + return getPath(credential, field); + } + + if (credential && typeof credential === "object") { + const obj = credential as Record; + for (const key of ["value", "token", "password", "secret", "apiKey", "api_key"]) { + if (typeof obj[key] === "string") return obj[key]; + } + if (obj.fields && typeof obj.fields === "object") { + return extractSecretValue(obj.fields, undefined); + } + } + + if (typeof credential === "string") return credential; + return undefined; +} + +function getPath(value: unknown, path: string): unknown { + return path.split(".").reduce((current, segment) => { + if (!current || typeof current !== "object") return undefined; + return (current as Record)[segment]; + }, value); +} + +function redactRef(ref: string): string { + return ref.length <= 4 ? "***" : `${ref.slice(0, 4)}***`; +} diff --git a/tests/cli/remote-routing.test.ts b/tests/cli/remote-routing.test.ts new file mode 100644 index 0000000..a47a63b --- /dev/null +++ b/tests/cli/remote-routing.test.ts @@ -0,0 +1,70 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { shouldRouteMissingServiceToRemote } from "../../src/cli/commands/remote-routing.ts"; +import { clearRemoteServiceCache } from "../../src/process/remote-discovery.ts"; + +describe("shouldRouteMissingServiceToRemote", () => { + const originalUrl = process.env.MCP2CLI_REMOTE_URL; + const originalTtl = process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS; + + afterEach(() => { + clearRemoteServiceCache(); + if (originalUrl === undefined) { + delete process.env.MCP2CLI_REMOTE_URL; + } else { + process.env.MCP2CLI_REMOTE_URL = originalUrl; + } + if (originalTtl === undefined) { + delete process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS; + } else { + process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS = originalTtl; + } + }); + + test("does not route without daemon mode", async () => { + await expect(shouldRouteMissingServiceToRemote("missing", false)).resolves.toBe(false); + }); + + test("does not route when no remote is configured", async () => { + delete process.env.MCP2CLI_REMOTE_URL; + await expect(shouldRouteMissingServiceToRemote("missing", true)).resolves.toBe(false); + }); + + test("routes only when discovery positively hosts the service", async () => { + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/api/services/discovery") { + return Response.json({ success: true, configuredServices: ["hosted"] }); + } + return new Response("Not Found", { status: 404 }); + }, + }); + try { + process.env.MCP2CLI_REMOTE_URL = `http://localhost:${server.port}`; + process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS = "0"; + clearRemoteServiceCache(); + await expect(shouldRouteMissingServiceToRemote("hosted", true)).resolves.toBe(true); + await expect(shouldRouteMissingServiceToRemote("missing", true)).resolves.toBe(false); + } finally { + server.stop(true); + } + }); + + test("does not route on discovery failure", async () => { + const server = Bun.serve({ + port: 0, + fetch() { + return new Response("temporary failure", { status: 500 }); + }, + }); + try { + process.env.MCP2CLI_REMOTE_URL = `http://localhost:${server.port}`; + process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS = "0"; + clearRemoteServiceCache(); + await expect(shouldRouteMissingServiceToRemote("missing", true)).resolves.toBe(false); + } finally { + server.stop(true); + } + }); +}); diff --git a/tests/config/loader.test.ts b/tests/config/loader.test.ts index 335c46e..ec85ba2 100644 --- a/tests/config/loader.test.ts +++ b/tests/config/loader.test.ts @@ -1,13 +1,20 @@ import { describe, test, expect, afterEach } from "bun:test"; import { resolve } from "path"; import { tmpdir } from "os"; -import { loadConfig } from "../../src/config/loader.ts"; +import { mkdtemp, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { loadConfig, mergeImportedConfig } from "../../src/config/loader.ts"; import { ConfigError } from "../../src/config/errors.ts"; +import type { ServicesConfig } from "../../src/config/index.ts"; const FIXTURES_DIR = resolve(import.meta.dir, "../fixtures"); afterEach(() => { delete process.env.MCP2CLI_CONFIG; + delete process.env.MCP2CLI_IMPORT_TOKEN; + delete process.env.MCP2CLI_IMPORT_ALLOW_HTTP; + delete process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE; + delete process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS; }); describe("loadConfig", () => { @@ -87,4 +94,498 @@ describe("loadConfig", () => { const { unlink } = await import("fs/promises"); await unlink(customPath).catch(() => {}); }); + + test("importUrl merges remote services when local file is stale", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-test-")); + const configPath = join(dir, "services.json"); + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/api/services/export") { + return Response.json({ + services: { + remote: { + backend: "http", + url: "http://ct216.example/mcp", + source: "remote", + }, + }, + }); + } + return new Response("Not Found", { status: 404 }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "localhost"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${server.port}/api/services/export`, + importTtlSeconds: 0, + services: { + local: { + backend: "stdio", + command: "echo", + }, + }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services).sort()).toEqual(["local", "remote"]); + expect(config.services.remote?.source).toBe("remote"); + } finally { + server.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl failure keeps local config", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-fail-test-")); + const configPath = join(dir, "services.json"); + const server = Bun.serve({ + port: 0, + fetch() { + return new Response("nope", { status: 500 }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "localhost"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${server.port}/api/services/export`, + importTtlSeconds: 0, + services: { + local: { + backend: "stdio", + command: "echo", + }, + }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services)).toEqual(["local"]); + } finally { + server.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl imports on first load when ttl is not explicitly set", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-default-test-")); + const configPath = join(dir, "services.json"); + const server = Bun.serve({ + port: 0, + fetch() { + return Response.json({ + services: { + remote: { + backend: "http", + url: "http://ct216.example/mcp", + }, + }, + }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "localhost"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${server.port}/api/services/export`, + services: { + local: { + backend: "stdio", + command: "echo", + }, + }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services).sort()).toEqual(["local", "remote"]); + } finally { + server.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("explicit positive import ttl skips fresh local file", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-ttl-test-")); + const configPath = join(dir, "services.json"); + let requests = 0; + const server = Bun.serve({ + port: 0, + fetch() { + requests++; + return Response.json({ + services: { + remote: { + backend: "http", + url: "http://ct216.example/mcp", + }, + }, + }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "localhost"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${server.port}/api/services/export`, + importTtlSeconds: 3600, + services: { + local: { + backend: "stdio", + command: "echo", + }, + }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services).sort()).toEqual(["local", "remote"]); + const secondLoad = await loadConfig(configPath); + expect(Object.keys(secondLoad.services).sort()).toEqual(["local", "remote"]); + expect(requests).toBe(1); + } finally { + server.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("cached import state still requires current importUrl policy", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-cache-policy-test-")); + const configPath = join(dir, "services.json"); + const server = Bun.serve({ + port: 0, + fetch() { + return Response.json({ + services: { + remote: { + backend: "http", + url: "http://ct216.example/mcp", + }, + }, + }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "localhost"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${server.port}/api/services/export`, + importTtlSeconds: 3600, + services: { local: { backend: "stdio", command: "echo" } }, + }), + ); + + const first = await loadConfig(configPath); + expect(Object.keys(first.services).sort()).toEqual(["local", "remote"]); + + delete process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS; + const second = await loadConfig(configPath); + expect(Object.keys(second.services)).toEqual(["local"]); + } finally { + server.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl sends bearer token when MCP2CLI_IMPORT_TOKEN is set", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-auth-test-")); + const configPath = join(dir, "services.json"); + let authHeader: string | null = null; + const server = Bun.serve({ + port: 0, + fetch(req) { + authHeader = req.headers.get("authorization"); + return Response.json({ + services: { + remote: { + backend: "http", + url: "http://ct216.example/mcp", + }, + }, + }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "localhost"; + process.env.MCP2CLI_IMPORT_TOKEN = "import-token"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${server.port}/api/services/export`, + importTtlSeconds: 0, + services: { + local: { + backend: "stdio", + command: "echo", + }, + }, + }), + ); + + await loadConfig(configPath); + expect(authHeader as string | null).toBe("Bearer import-token"); + } finally { + server.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl token requires an explicit host allowlist", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-auth-allowlist-test-")); + const configPath = join(dir, "services.json"); + let requests = 0; + const server = Bun.serve({ + port: 0, + fetch() { + requests++; + return Response.json({ services: { remote: { backend: "http", url: "http://ct216.example/mcp" } } }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_TOKEN = "import-token"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${server.port}/api/services/export`, + importTtlSeconds: 0, + services: { local: { backend: "stdio", command: "echo" } }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services)).toEqual(["local"]); + expect(requests).toBe(0); + } finally { + server.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl strips bearer token on cross-origin redirects", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-redirect-test-")); + const configPath = join(dir, "services.json"); + let redirectedAuthHeader: string | null = null; + const target = Bun.serve({ + port: 0, + fetch(req) { + redirectedAuthHeader = req.headers.get("authorization"); + return Response.json({ services: { remote: { backend: "http", url: "http://ct216.example/mcp" } } }); + }, + }); + const origin = Bun.serve({ + port: 0, + fetch() { + return new Response(null, { + status: 302, + headers: { Location: `http://127.0.0.1:${target.port}/api/services/export` }, + }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "localhost,127.0.0.1"; + process.env.MCP2CLI_IMPORT_TOKEN = "import-token"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${origin.port}/api/services/export`, + importTtlSeconds: 0, + services: { local: { backend: "stdio", command: "echo" } }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services).sort()).toEqual(["local", "remote"]); + expect(redirectedAuthHeader as string | null).toBeNull(); + } finally { + origin.stop(true); + target.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl does not send general daemon auth token", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-no-leak-test-")); + const configPath = join(dir, "services.json"); + let authHeader: string | null = null; + const originalAuth = process.env.MCP2CLI_AUTH_TOKEN; + const server = Bun.serve({ + port: 0, + fetch(req) { + authHeader = req.headers.get("authorization"); + return Response.json({ services: { remote: { backend: "http", url: "http://ct216.example/mcp" } } }); + }, + }); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOW_PRIVATE = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "localhost"; + process.env.MCP2CLI_AUTH_TOKEN = "daemon-token"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: `http://localhost:${server.port}/api/services/export`, + importTtlSeconds: 0, + services: { local: { backend: "stdio", command: "echo" } }, + }), + ); + + await loadConfig(configPath); + expect(authHeader as string | null).toBeNull(); + } finally { + if (originalAuth === undefined) { + delete process.env.MCP2CLI_AUTH_TOKEN; + } else { + process.env.MCP2CLI_AUTH_TOKEN = originalAuth; + } + server.stop(true); + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl blocks private hosts unless explicitly allowed", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-private-test-")); + const configPath = join(dir, "services.json"); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: "http://localhost:65535/api/services/export", + importTtlSeconds: 0, + services: { local: { backend: "stdio", command: "echo" } }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services)).toEqual(["local"]); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl blocks IPv4-mapped IPv6 loopback hosts", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-mapped-ipv6-test-")); + const configPath = join(dir, "services.json"); + try { + process.env.MCP2CLI_IMPORT_ALLOW_HTTP = "1"; + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "[::ffff:7f00:1]"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: "http://[::ffff:127.0.0.1]:65535/api/services/export", + importTtlSeconds: 0, + services: { local: { backend: "stdio", command: "echo" } }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services)).toEqual(["local"]); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("importUrl blocks DNS hosts unless explicitly allowed", async () => { + const dir = await mkdtemp(join(tmpdir(), "mcp2cli-import-dns-test-")); + const configPath = join(dir, "services.json"); + try { + process.env.MCP2CLI_IMPORT_ALLOWED_HOSTS = "example.com"; + await Bun.write( + configPath, + JSON.stringify({ + importUrl: "https://example.com/api/services/export", + importTtlSeconds: 0, + services: { local: { backend: "stdio", command: "echo" } }, + }), + ); + + const config = await loadConfig(configPath); + expect(Object.keys(config.services)).toEqual(["local"]); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); + +describe("mergeImportedConfig", () => { + test("imported service wins and source comes from imported config", () => { + const local = { + services: { + shared: { + backend: "stdio", + command: "old", + args: [], + env: {}, + source: "local", + }, + }, + } satisfies ServicesConfig; + const imported = { + services: { + shared: { + backend: "http", + url: "http://ct216.example/mcp", + headers: {}, + source: "remote", + }, + }, + } satisfies ServicesConfig; + + const merged = mergeImportedConfig(local, imported); + const shared = merged.services.shared!; + expect(shared.backend).toBe("http"); + expect(shared.source).toBe("remote"); + }); + + test("imported services default to remote source", () => { + const local = { + services: { + local: { + backend: "stdio", + command: "echo", + args: [], + env: {}, + }, + }, + } satisfies ServicesConfig; + const imported = { + services: { + remote: { + backend: "stdio", + command: "remote-command", + args: [], + env: {}, + }, + }, + } satisfies ServicesConfig; + + const merged = mergeImportedConfig(local, imported); + expect(merged.services.remote?.source).toBe("remote"); + }); }); diff --git a/tests/config/schema.test.ts b/tests/config/schema.test.ts index f787bf9..59bca22 100644 --- a/tests/config/schema.test.ts +++ b/tests/config/schema.test.ts @@ -46,6 +46,18 @@ describe("StdioServiceSchema", () => { } }); + test("optional platforms field is accepted", () => { + const result = StdioServiceSchema.safeParse({ + backend: "stdio", + command: "npx", + platforms: ["darwin"], + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.platforms).toEqual(["darwin"]); + } + }); + test("missing command fails", () => { const result = StdioServiceSchema.safeParse({ backend: "stdio", @@ -77,6 +89,22 @@ describe("HttpServiceSchema", () => { } }); + test("valid http service accepts full url secret ref", () => { + const result = HttpServiceSchema.safeParse({ + backend: "http", + url: "${secret:open-brain#url}", + }); + expect(result.success).toBe(true); + }); + + test("http service rejects partial url secret ref", () => { + const result = HttpServiceSchema.safeParse({ + backend: "http", + url: "https://${secret:host}/mcp", + }); + expect(result.success).toBe(false); + }); + test("optional headers missing defaults to empty", () => { const result = HttpServiceSchema.safeParse({ backend: "http", @@ -156,6 +184,37 @@ describe("ServicesConfigSchema", () => { } }); + test("valid config accepts importUrl and importTtlSeconds", () => { + const result = ServicesConfigSchema.safeParse({ + importUrl: "http://localhost:9500/api/services/export", + importTtlSeconds: 0, + services: { + n8n: { + backend: "stdio", + command: "npx", + }, + }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.importUrl).toBe("http://localhost:9500/api/services/export"); + expect(result.data.importTtlSeconds).toBe(0); + } + }); + + test("invalid importUrl fails validation", () => { + const result = ServicesConfigSchema.safeParse({ + importUrl: "not-a-url", + services: { + n8n: { + backend: "stdio", + command: "npx", + }, + }, + }); + expect(result.success).toBe(false); + }); + test("valid mixed services (stdio + http) passes", () => { const result = ServicesConfigSchema.safeParse({ services: { diff --git a/tests/daemon/api.test.ts b/tests/daemon/api.test.ts index c705de9..350d552 100644 --- a/tests/daemon/api.test.ts +++ b/tests/daemon/api.test.ts @@ -5,6 +5,11 @@ import { tmpdir } from "node:os"; import { ConfigManager } from "../../src/daemon/config-manager.ts"; import type { ServicesConfig } from "../../src/config/index.ts"; import { isAuthExempt } from "../../src/daemon/auth.ts"; +import { createDaemonServer } from "../../src/daemon/server.ts"; +import { ConnectionPool } from "../../src/daemon/pool.ts"; +import { IdleTimer } from "../../src/daemon/idle.ts"; +import { TokenAuthProvider } from "../../src/daemon/auth-provider.ts"; +import { MetricsCollector } from "../../src/daemon/metrics.ts"; const STDIO_SERVICE = { backend: "stdio" as const, @@ -38,6 +43,10 @@ describe("Management API auth exemptions", () => { expect(isAuthExempt("/api/services/import")).toBe(false); }); + test("/api/services/export requires auth", () => { + expect(isAuthExempt("/api/services/export")).toBe(false); + }); + test("/api/services/myservice/status requires auth", () => { expect(isAuthExempt("/api/services/myservice/status")).toBe(false); }); @@ -142,4 +151,157 @@ describe("ConfigManager API operations", () => { expect(svc).not.toBeNull(); expect(svc!.backend).toBe("websocket"); }); + + test("sanitized export strips env, headers, URL credentials, and sensitive args", async () => { + const exportConfig = makeConfig({ + local: { + backend: "stdio", + command: "/usr/bin/env", + source: "local", + args: [ + "--api-key", + "do-not-export", + "--header", + "Authorization: Bearer do-not-export", + "Cookie: session=do-not-export", + "--mode=ok", + ], + env: { SECRET_TOKEN: "do-not-export" }, + }, + remote: { + backend: "http", + url: "http://user:pass@example.test/mcp?token=do-not-export#frag", + headers: { Authorization: "Bearer do-not-export" }, + fallback: { + command: "/usr/bin/env", + args: ["--api-key=do-not-export", "--safe", "value"], + env: { API_KEY: "do-not-export" }, + }, + }, + }); + exportConfig.importUrl = "https://user:pass@example.test/export?token=do-not-export#frag"; + exportConfig.importTtlSeconds = 3600; + const exportMgr = new ConfigManager(exportConfig, configPath); + const pool = new ConnectionPool(); + const server = createDaemonServer({ + listenConfig: { mode: "unix", socketPath: join(tmpDir, "export.sock") }, + pool, + config: exportConfig, + configManager: exportMgr, + idleTimer: new IdleTimer(60000, () => {}), + onShutdown: () => {}, + authProvider: new TokenAuthProvider([]), + metrics: new MetricsCollector(), + }); + + try { + const res = await server.fetch( + new Request("http://localhost/api/services/export", { method: "GET" }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as ServicesConfig; + expect("importUrl" in body).toBe(false); + expect("importTtlSeconds" in body).toBe(false); + const local = body.services.local!; + expect(local.backend).toBe("stdio"); + if (local.backend === "stdio") { + expect(local.source).toBe("remote"); + expect(local.args).toEqual([ + "--api-key", + "[REDACTED]", + "--header", + "[REDACTED]", + "[REDACTED]", + "--mode=ok", + ]); + expect(local.env).toEqual({}); + } + const remote = body.services.remote!; + expect(remote.backend).toBe("http"); + if (remote.backend === "http") { + expect(remote.url).toBe("http://example.test/mcp"); + expect(remote.headers).toEqual({}); + expect(remote.fallback).toEqual({ command: "/usr/bin/env", args: ["--api-key=[REDACTED]", "--safe", "value"], env: {} }); + } + } finally { + server.stop(true); + await pool.closeAll(); + } + }); +}); + +describe("Auth refresh API", () => { + let tmpDir: string; + let tokensPath: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), "mcp2cli-auth-api-test-")); + tokensPath = join(tmpDir, "tokens.json"); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + test("POST /api/auth/refresh rotates a valid near-expiry token", async () => { + const now = Date.now(); + await Bun.write(tokensPath, JSON.stringify({ + tokens: [{ + id: "skippy", + token: "old-token", + role: "agent", + expiresAt: new Date(now + 5 * 60 * 1000).toISOString(), + }], + })); + process.env.MCP2CLI_TOKEN_REFRESH_WINDOW_MS = String(10 * 60 * 1000); + process.env.MCP2CLI_TOKEN_TTL_MS = String(60 * 60 * 1000); + + const pool = new ConnectionPool(); + const authProvider = await TokenAuthProvider.load(tokensPath); + const server = createDaemonServer({ + listenConfig: { mode: "unix", socketPath: join(tmpDir, "auth-refresh.sock") }, + pool, + config: makeConfig({ echo: STDIO_SERVICE }), + idleTimer: new IdleTimer(60000, () => {}), + onShutdown: () => {}, + authProvider, + metrics: new MetricsCollector(), + }); + + try { + const res = await server.fetch( + new Request("http://localhost/api/auth/refresh", { + method: "POST", + headers: { Authorization: "Bearer old-token" }, + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { success: boolean; token: string; userId: string; role: string }; + expect(body.success).toBe(true); + expect(body.token).not.toBe("old-token"); + expect(body.userId).toBe("skippy"); + expect(body.role).toBe("agent"); + + const meOld = await server.fetch( + new Request("http://localhost/api/auth/me", { + method: "GET", + headers: { Authorization: "Bearer old-token" }, + }), + ); + expect(meOld.status).toBe(401); + + const meNew = await server.fetch( + new Request("http://localhost/api/auth/me", { + method: "GET", + headers: { Authorization: `Bearer ${body.token}` }, + }), + ); + expect(meNew.status).toBe(200); + } finally { + delete process.env.MCP2CLI_TOKEN_REFRESH_WINDOW_MS; + delete process.env.MCP2CLI_TOKEN_TTL_MS; + server.stop(true); + await pool.closeAll(); + } + }); }); diff --git a/tests/daemon/auth-provider.test.ts b/tests/daemon/auth-provider.test.ts index d3400a2..ee12cd4 100644 --- a/tests/daemon/auth-provider.test.ts +++ b/tests/daemon/auth-provider.test.ts @@ -90,6 +90,20 @@ describe("TokenAuthProvider", () => { }); expect(provider.authenticate(req)).not.toBeNull(); }); + + test("rejects expired bearer tokens", () => { + const provider = new TokenAuthProvider([ + { ...ADMIN_TOKEN, expiresAt: "2020-01-01T00:00:00.000Z" }, + ]); + expect(provider.authenticate(makeReq("/call", "POST", "admin-secret"))).toBeNull(); + }); + + test("accepts unexpired bearer tokens", () => { + const provider = new TokenAuthProvider([ + { ...ADMIN_TOKEN, expiresAt: "2999-01-01T00:00:00.000Z" }, + ]); + expect(provider.authenticate(makeReq("/call", "POST", "admin-secret"))?.userId).toBe("rico"); + }); }); describe("TokenAuthProvider.authenticateBasic", () => { @@ -243,6 +257,132 @@ describe("TokenAuthProvider.load", () => { process.env.MCP2CLI_TOKENS_FILE = tokensPath; expect(TokenAuthProvider.load()).rejects.toThrow("invalid role"); }); + + test("rejects invalid expiresAt", async () => { + const tokensPath = join(tmpDir, "tokens.json"); + await Bun.write(tokensPath, JSON.stringify({ + tokens: [{ id: "x", token: "t", role: "admin", expiresAt: "not-a-date" }], + })); + process.env.MCP2CLI_TOKENS_FILE = tokensPath; + expect(TokenAuthProvider.load()).rejects.toThrow("invalid expiresAt"); + }); +}); + +describe("TokenAuthProvider token refresh", () => { + let tmpDir: string; + let tokensPath: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), "mcp2cli-auth-refresh-test-")); + tokensPath = join(tmpDir, "tokens.json"); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + test("refreshes a valid near-expiry token, persists it, and rejects the old token", async () => { + const now = new Date(); + await Bun.write(tokensPath, JSON.stringify({ + tokens: [{ + id: "skippy", + token: "old-token", + role: "agent", + expiresAt: new Date(now.getTime() + 5 * 60 * 1000).toISOString(), + }], + })); + + const provider = await TokenAuthProvider.load(tokensPath); + const result = await provider.refreshBearerToken("old-token", { + now, + refreshWindowMs: 10 * 60 * 1000, + ttlMs: 60 * 60 * 1000, + }); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error(result.message); + expect(result.token).not.toBe("old-token"); + expect(provider.authenticate(makeReq("/call", "POST", "old-token"))).toBeNull(); + expect(provider.authenticate(makeReq("/call", "POST", result.token))?.userId).toBe("skippy"); + + const onDisk = await Bun.file(tokensPath).json() as { tokens: Array<{ token: string; expiresAt: string }> }; + expect(onDisk.tokens[0]!.token).toBe(result.token); + expect(onDisk.tokens[0]!.expiresAt).toBe(result.expiresAt); + }); + + test("does not refresh tokens that are not near expiry", async () => { + const now = new Date(); + await Bun.write(tokensPath, JSON.stringify({ + tokens: [{ + id: "skippy", + token: "still-fresh", + role: "agent", + expiresAt: new Date(now.getTime() + 60 * 60 * 1000).toISOString(), + }], + })); + + const provider = await TokenAuthProvider.load(tokensPath); + const result = await provider.refreshBearerToken("still-fresh", { + now, + refreshWindowMs: 10 * 60 * 1000, + }); + + expect(result).toEqual({ + ok: false, + status: 400, + message: "Token is not near expiry", + }); + }); + + test("serializes concurrent refresh requests for the same token", async () => { + const now = new Date(); + await Bun.write(tokensPath, JSON.stringify({ + tokens: [{ + id: "skippy", + token: "old-token", + role: "agent", + expiresAt: new Date(now.getTime() + 5 * 60 * 1000).toISOString(), + }], + })); + + const provider = await TokenAuthProvider.load(tokensPath); + const results = await Promise.all([ + provider.refreshBearerToken("old-token", { + now, + refreshWindowMs: 10 * 60 * 1000, + ttlMs: 60 * 60 * 1000, + }), + provider.refreshBearerToken("old-token", { + now, + refreshWindowMs: 10 * 60 * 1000, + ttlMs: 60 * 60 * 1000, + }), + ]); + + expect(results.filter((result) => result.ok)).toHaveLength(1); + const onDisk = await Bun.file(tokensPath).json() as { tokens: Array<{ token: string }> }; + const successful = results.find((result) => result.ok); + expect(successful?.ok).toBe(true); + if (successful?.ok) { + expect(onDisk.tokens[0]!.token).toBe(successful.token); + } + }); + + test("reloadFromDisk picks up edited token files", async () => { + await Bun.write(tokensPath, JSON.stringify({ + tokens: [{ id: "skippy", token: "before", role: "agent" }], + })); + const provider = await TokenAuthProvider.load(tokensPath); + expect(provider.authenticate(makeReq("/call", "POST", "before"))?.userId).toBe("skippy"); + + await Bun.write(tokensPath, JSON.stringify({ + tokens: [{ id: "rico", token: "after", role: "admin" }], + })); + await provider.reloadFromDisk(); + + expect(provider.authenticate(makeReq("/call", "POST", "before"))).toBeNull(); + expect(provider.authenticate(makeReq("/call", "POST", "after"))?.userId).toBe("rico"); + }); }); describe("hasPermission", () => { diff --git a/tests/daemon/file-watch.test.ts b/tests/daemon/file-watch.test.ts new file mode 100644 index 0000000..b9f6df4 --- /dev/null +++ b/tests/daemon/file-watch.test.ts @@ -0,0 +1,152 @@ +import { describe, test, expect, afterEach } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { ConfigManager } from "../../src/daemon/config-manager.ts"; +import { CredentialManager } from "../../src/credentials/index.ts"; +import { startConfigFileWatchers, type FileWatchHandle } from "../../src/daemon/file-watch.ts"; +import { TokenAuthProvider } from "../../src/daemon/auth-provider.ts"; +import type { ServicesConfig } from "../../src/config/index.ts"; + +const ECHO_SERVICE = { + backend: "stdio" as const, + command: "/usr/bin/echo", + args: [], + env: {}, +}; + +describe("config file watchers", () => { + let tempDir: string | null = null; + let watcher: FileWatchHandle | null = null; + + afterEach(async () => { + watcher?.close(); + watcher = null; + if (tempDir) { + await rm(tempDir, { recursive: true, force: true }); + tempDir = null; + } + }); + + async function setup() { + tempDir = await mkdtemp(join(tmpdir(), "mcp2cli-watch-test-")); + const configPath = join(tempDir, "services.json"); + const credentialsPath = join(tempDir, "credentials.json"); + const tokensPath = join(tempDir, "tokens.json"); + const initialConfig: ServicesConfig = { + services: { + echo: ECHO_SERVICE, + }, + }; + await Bun.write(configPath, JSON.stringify(initialConfig, null, 2)); + await Bun.write(credentialsPath, JSON.stringify({ groups: {}, credentials: {}, defaults: {} }, null, 2)); + await Bun.write(tokensPath, JSON.stringify({ tokens: [{ id: "skippy", token: "before", role: "agent" }] }, null, 2)); + const configManager = new ConfigManager(initialConfig, configPath); + const credentialManager = await CredentialManager.load(credentialsPath); + const authProvider = await TokenAuthProvider.load(tokensPath); + watcher = startConfigFileWatchers({ + configManager, + credentialManager, + authProvider, + debounceMs: 10, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + return { configPath, credentialsPath, tokensPath, configManager, credentialManager, authProvider }; + } + + test("auto-reloads services.json after a valid file edit", async () => { + const { configPath, configManager } = await setup(); + await Bun.write( + configPath, + JSON.stringify({ + services: { + echo: ECHO_SERVICE, + cat: { + backend: "stdio", + command: "/bin/cat", + args: [], + env: {}, + }, + }, + }, null, 2), + ); + + await waitFor(() => configManager.getService("cat") !== null); + expect(configManager.serviceNames.sort()).toEqual(["cat", "echo"]); + }); + + test("keeps previous services config when edited file is invalid", async () => { + const { configPath, configManager } = await setup(); + await Bun.write(configPath, JSON.stringify({ services: {} }, null, 2)); + + await new Promise((resolve) => setTimeout(resolve, 150)); + expect(configManager.serviceNames).toEqual(["echo"]); + }); + + test("auto-reloads credentials.json after a valid file edit", async () => { + const { credentialsPath, credentialManager } = await setup(); + await Bun.write( + credentialsPath, + JSON.stringify({ + groups: {}, + credentials: {}, + defaults: { + echo: { + env: { ECHO_TOKEN: "changed" }, + }, + }, + }, null, 2), + ); + + await waitFor(() => credentialManager.resolve("anyone", "echo")?.env?.ECHO_TOKEN === "changed"); + expect(credentialManager.resolve("anyone", "echo")?.env?.ECHO_TOKEN).toBe("changed"); + }); + + test("auto-reloads tokens.json after a valid file edit", async () => { + const { tokensPath, authProvider } = await setup(); + expect( + authProvider.authenticate( + new Request("http://localhost/call", { + method: "POST", + headers: { Authorization: "Bearer before" }, + }), + )?.userId, + ).toBe("skippy"); + + await Bun.write( + tokensPath, + JSON.stringify({ + tokens: [{ id: "rico", token: "after", role: "admin" }], + }, null, 2), + ); + + await waitFor(() => + authProvider.authenticate( + new Request("http://localhost/call", { + method: "POST", + headers: { Authorization: "Bearer after" }, + }), + )?.userId === "rico", + ); + expect( + authProvider.authenticate( + new Request("http://localhost/call", { + method: "POST", + headers: { Authorization: "Bearer before" }, + }), + ), + ).toBeNull(); + }); +}); + +async function waitFor( + predicate: () => boolean, + timeoutMs = 1000, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (predicate()) return; + await new Promise((resolve) => setTimeout(resolve, 20)); + } + throw new Error("Timed out waiting for condition"); +} diff --git a/tests/daemon/observability.test.ts b/tests/daemon/observability.test.ts index 4c3bee7..2d85173 100644 --- a/tests/daemon/observability.test.ts +++ b/tests/daemon/observability.test.ts @@ -105,7 +105,10 @@ describe("Daemon Observability", () => { }); /** Create a daemon server bound to a unique unix socket in tempDir */ - function makeServer(pool: InstanceType) { + function makeServer( + pool: InstanceType, + authProvider = new TokenAuthProvider([]), + ) { const socketPath = join(tempDir, `test-${Date.now()}.sock`); const idleTimer = new IdleTimer(60000, () => {}); const server = createDaemonServer({ @@ -114,7 +117,7 @@ describe("Daemon Observability", () => { config: testConfig, idleTimer, onShutdown: () => {}, - authProvider: new TokenAuthProvider([]), + authProvider, metrics: new MetricsCollector(), }); servers.push(server); @@ -132,8 +135,8 @@ describe("Daemon Observability", () => { expect(body.status).toBe("ok"); expect(body.uptime).toBeDefined(); - expect(body.configuredServices).toBeDefined(); - expect(body.connectedServices).toBeDefined(); + expect(body.configuredServices).toBeUndefined(); + expect(body.connectedServices).toBeUndefined(); expect(body.activeConnections).toBeDefined(); // Memory stats @@ -177,6 +180,169 @@ describe("Daemon Observability", () => { await pool.closeAll(); }); + test("authenticated /call logs caller identity and role", async () => { + setLogLevel("info"); + + const pool = new ConnectionPool(); + const server = makeServer( + pool, + new TokenAuthProvider([{ id: "skippy", token: "agent-token", role: "agent" }]), + ); + + const lines = await captureStderrAsync(async () => { + const req = new Request("http://localhost/call", { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer agent-token", + }, + body: JSON.stringify({ + service: "test-svc", + tool: "my-tool", + params: {}, + }), + }); + await server.fetch(req); + }); + + const responseLines = parseLogEntries(lines).filter( + (e) => + e.component === "daemon:request" && e.message === "response_out", + ); + + expect(responseLines.length).toBeGreaterThanOrEqual(1); + const entry = responseLines[0]!; + expect(entry.data?.userId).toBe("skippy"); + expect(entry.data?.role).toBe("agent"); + + await pool.closeAll(); + }); + + test("per-caller metrics are hidden from public /metrics by default and exposed through user API", async () => { + const pool = new ConnectionPool(); + const server = makeServer( + pool, + new TokenAuthProvider([{ id: "skippy", token: "agent-token", role: "agent" }]), + ); + + const callReq = new Request("http://localhost/call", { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer agent-token", + }, + body: JSON.stringify({ + service: "test-svc", + tool: "my-tool", + params: {}, + }), + }); + await server.fetch(callReq); + + const metricsRes = await server.fetch(new Request("http://localhost/metrics", { method: "GET" })); + const metricsBody = await metricsRes.text(); + expect(metricsBody).toContain('mcp2cli_requests_total{service="test-svc",tool="my-tool"} 1'); + expect(metricsBody).not.toContain('caller="skippy"'); + + const userRes = await server.fetch( + new Request("http://localhost/api/metrics/user/skippy", { + method: "GET", + headers: { Authorization: "Bearer agent-token" }, + }), + ); + expect(userRes.status).toBe(200); + const userBody = (await userRes.json()) as { + success: boolean; + userId: string; + totalRequests: number; + errorCount: number; + requests: Array<{ service: string; tool: string; count: number }>; + }; + expect(userBody.success).toBe(true); + expect(userBody.userId).toBe("skippy"); + expect(userBody.totalRequests).toBe(1); + expect(userBody.errorCount).toBe(0); + expect(userBody.requests).toEqual([ + expect.objectContaining({ service: "test-svc", tool: "my-tool", count: 1 }), + ]); + + await pool.closeAll(); + }); + + test("per-caller metrics can be explicitly enabled for /metrics", async () => { + const original = process.env.MCP2CLI_METRICS_INCLUDE_CALLER; + process.env.MCP2CLI_METRICS_INCLUDE_CALLER = "1"; + const pool = new ConnectionPool(); + const server = makeServer( + pool, + new TokenAuthProvider([{ id: "skippy", token: "agent-token", role: "agent" }]), + ); + + try { + await server.fetch(new Request("http://localhost/call", { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer agent-token", + }, + body: JSON.stringify({ + service: "test-svc", + tool: "my-tool", + params: {}, + }), + })); + + const metricsRes = await server.fetch(new Request("http://localhost/metrics", { method: "GET" })); + const metricsBody = await metricsRes.text(); + expect(metricsBody).toContain('mcp2cli_requests_total{service="test-svc",tool="my-tool",caller="skippy"} 1'); + expect(metricsBody).toContain('mcp2cli_request_duration_ms_count{service="test-svc",tool="my-tool",caller="skippy"} 1'); + } finally { + if (original === undefined) { + delete process.env.MCP2CLI_METRICS_INCLUDE_CALLER; + } else { + process.env.MCP2CLI_METRICS_INCLUDE_CALLER = original; + } + await pool.closeAll(); + } + }); + + test("non-admin callers cannot read another user's metrics", async () => { + const pool = new ConnectionPool(); + const server = makeServer( + pool, + new TokenAuthProvider([ + { id: "skippy", token: "agent-token", role: "agent" }, + { id: "rico", token: "admin-token", role: "admin" }, + ]), + ); + + const denied = await server.fetch( + new Request("http://localhost/api/metrics/user/rico", { + method: "GET", + headers: { Authorization: "Bearer agent-token" }, + }), + ); + expect(denied.status).toBe(403); + + const allowed = await server.fetch( + new Request("http://localhost/api/metrics/user/skippy", { + method: "GET", + headers: { Authorization: "Bearer agent-token" }, + }), + ); + expect(allowed.status).toBe(200); + + const adminAllowed = await server.fetch( + new Request("http://localhost/api/metrics/user/skippy", { + method: "GET", + headers: { Authorization: "Bearer admin-token" }, + }), + ); + expect(adminAllowed.status).toBe(200); + + await pool.closeAll(); + }); + test("successful /call logs response_out with service, tool, totalDuration, success=true", async () => { setLogLevel("info"); diff --git a/tests/daemon/pool.test.ts b/tests/daemon/pool.test.ts index 47027e0..4863ee1 100644 --- a/tests/daemon/pool.test.ts +++ b/tests/daemon/pool.test.ts @@ -72,12 +72,21 @@ const testConfig: ServicesConfig = { }, }; +const testResolver = { + resolve: mock(async (ref: string) => { + if (ref === "svc#token") return "resolved-token"; + if (ref === "svc#url") return "http://resolved.example/mcp"; + throw new Error(`unexpected ref ${ref}`); + }), +}; + describe("ConnectionPool", () => { let pool: InstanceType; beforeEach(() => { - pool = new ConnectionPool(); + pool = new ConnectionPool({ secretResolver: testResolver }); mockConnectToService.mockClear(); + testResolver.resolve.mockClear(); }); test("getConnection creates new connection (mock called once)", async () => { @@ -130,6 +139,33 @@ describe("ConnectionPool", () => { expect(mockConnectToService).toHaveBeenCalledTimes(1); }); + test("resolves secret refs before opening a connection", async () => { + const config: ServicesConfig = { + services: { + secreted: { + backend: "http", + url: "${secret:svc#url}", + headers: { Authorization: "Bearer ${secret:svc#token}" }, + }, + }, + }; + + await pool.getConnection("secreted", config); + + expect(mockConnectToService).toHaveBeenCalledTimes(1); + const calls = mockConnectToService.mock.calls as unknown as Array<[unknown]>; + expect(calls[0]![0]).toEqual({ + backend: "http", + url: "http://resolved.example/mcp", + headers: { Authorization: "Bearer resolved-token" }, + }); + const original = config.services.secreted!; + expect(original.backend).toBe("http"); + if (original.backend === "http") { + expect(original.url).toBe("${secret:svc#url}"); + } + }); + test("closeServicePattern only closes entries for the matching base service", async () => { const unrelated = await pool.getConnection("svc::with-delimiter", testConfig); const credentialed = await pool.getConnection( diff --git a/tests/daemon/timeout.test.ts b/tests/daemon/timeout.test.ts index b0a5f84..e98af67 100644 --- a/tests/daemon/timeout.test.ts +++ b/tests/daemon/timeout.test.ts @@ -1,120 +1,121 @@ /** * MEM-02: Tool call timeout tests. - * Tests that tool calls exceeding MCP2CLI_TOOL_TIMEOUT return structured TOOL_TIMEOUT errors. - * - * Integration test: exercises the full CLI -> daemon -> slow-mcp-server path. + * Exercises the daemon /call path against a slow MCP fixture without relying on + * CLI-managed daemon startup timing. */ -import { describe, test, expect, afterEach, beforeEach } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdtemp, rm } from "node:fs/promises"; -import { join } from "node:path"; import { tmpdir } from "node:os"; -import { resolve } from "path"; - -const PROJECT_ROOT = resolve(import.meta.dir, "../.."); - -interface CliResult { - stdout: string; - stderr: string; - exitCode: number; -} +import { join } from "node:path"; +import { createDaemonServer } from "../../src/daemon/server.ts"; +import { ConnectionPool } from "../../src/daemon/pool.ts"; +import { IdleTimer } from "../../src/daemon/idle.ts"; +import { TokenAuthProvider } from "../../src/daemon/auth-provider.ts"; +import { MetricsCollector } from "../../src/daemon/metrics.ts"; +import type { ServicesConfig } from "../../src/config/index.ts"; let tempDir: string; +let pool: ConnectionPool; +let server: ReturnType; -function runCli( - args: string[], - extraEnv?: Record, - timeoutMs = 30_000, -): CliResult { - const configPath = join(tempDir, "config.json"); - - const env: Record = { - ...(process.env as Record), - MCP2CLI_CONFIG: configPath, - MCP2CLI_PID_FILE: join(tempDir, "daemon.pid"), - MCP2CLI_SOCKET_PATH: join(tempDir, "daemon.sock"), - MCP2CLI_IDLE_TIMEOUT: "10", - MCP2CLI_CACHE_DIR: join(tempDir, "schemas"), - MCP2CLI_TOKENS_FILE: join(tempDir, "nonexistent-tokens.json"), - MCP2CLI_AUTH_TOKEN: "", - ...extraEnv, - }; - - const proc = Bun.spawnSync( - ["bun", "run", "src/cli/index.ts", ...args], - { - cwd: PROJECT_ROOT, - env, - stdout: "pipe", - stderr: "pipe", - timeout: timeoutMs, - }, - ); - +function makeConfig(): ServicesConfig { return { - stdout: proc.stdout.toString().trim(), - stderr: proc.stderr.toString().trim(), - exitCode: proc.exitCode, + services: { + slowOk: { + backend: "stdio", + command: "fake", + args: [], + env: {}, + timeout: 30_000, + }, + slowTimeout: { + backend: "stdio", + command: "fake", + args: [], + env: {}, + timeout: 500, + }, + }, }; } -function shutdownDaemon() { - try { - runCli(["shutdown"], undefined, 5000); - } catch { - // May not be running +class FakePool { + async getConnection() { + return { + client: { + async callTool(request: { arguments?: { delay_ms?: number } }) { + const delay = request.arguments?.delay_ms ?? 0; + await new Promise((resolveDelay) => setTimeout(resolveDelay, delay)); + return { + content: [ + { + type: "text", + text: JSON.stringify({ status: "ok", delayed: delay }), + }, + ], + }; + }, + }, + }; } + + async closeAll() {} +} + +async function callTool(service: string, delayMs: number): Promise { + const response = await server.fetch( + new Request("http://localhost/call", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + service, + tool: "slow_tool", + params: { delay_ms: delayMs }, + }), + }), + ); + + return response.json(); } describe("MEM-02: Tool call timeout", () => { beforeEach(async () => { tempDir = await mkdtemp(join(tmpdir(), "mcp2cli-timeout-")); - const slowServer = resolve(PROJECT_ROOT, "tests/fixtures/slow-mcp-server.ts"); - - const config = { - services: { - slow: { - backend: "stdio", - command: "bun", - args: [slowServer], - }, - }, - }; - await Bun.write(join(tempDir, "config.json"), JSON.stringify(config)); + const config = makeConfig(); + pool = new FakePool() as unknown as ConnectionPool; + server = createDaemonServer({ + listenConfig: { mode: "unix", socketPath: join(tempDir, "daemon.sock") }, + pool, + config, + idleTimer: new IdleTimer(60_000, () => {}), + onShutdown: () => {}, + authProvider: new TokenAuthProvider([]), + metrics: new MetricsCollector(), + }); }); afterEach(async () => { - shutdownDaemon(); - await new Promise((r) => setTimeout(r, 500)); - if (tempDir) { - await rm(tempDir, { recursive: true, force: true }).catch(() => {}); - } + server.stop(true); + await pool.closeAll(); + await rm(tempDir, { recursive: true, force: true }); }); test("tool call within timeout succeeds normally", async () => { - // Use --params JSON format (the CLI's supported param syntax) - const result = runCli( - ["slow", "slow_tool", "--params", '{"delay_ms":100}'], - { MCP2CLI_TOOL_TIMEOUT: "30000" }, - ); + const parsed = await callTool("slowOk", 10) as { success?: boolean; result?: { delayed?: number } }; - const parsed = JSON.parse(result.stdout); expect(parsed.success).toBe(true); - expect(parsed.result.delayed).toBe(100); - }, 30_000); + expect(parsed.result?.delayed).toBe(10); + }); test("tool call exceeding timeout returns TOOL_TIMEOUT error", async () => { - // Use very short timeout (500ms) with long delay (60s) - const result = runCli( - ["slow", "slow_tool", "--params", '{"delay_ms":60000}'], - { MCP2CLI_TOOL_TIMEOUT: "500" }, - ); + const parsed = await callTool("slowTimeout", 2_000) as { + success?: boolean; + error?: { code?: string; message?: string }; + }; - // Should fail with TOOL_TIMEOUT - expect(result.exitCode).not.toBe(0); - const parsed = JSON.parse(result.stdout); - expect(parsed.error).toBe(true); - expect(parsed.code).toBe("TOOL_TIMEOUT"); - expect(parsed.message).toContain("timed out"); - expect(parsed.message).toContain("500ms"); - }, 30_000); + expect(parsed.success).toBe(false); + expect(parsed.error?.code).toBe("TOOL_TIMEOUT"); + expect(parsed.error?.message).toContain("timed out"); + expect(parsed.error?.message).toContain("500ms"); + }); }); diff --git a/tests/fixtures/mock-remote-daemon.ts b/tests/fixtures/mock-remote-daemon.ts new file mode 100644 index 0000000..57c4b3a --- /dev/null +++ b/tests/fixtures/mock-remote-daemon.ts @@ -0,0 +1,42 @@ +const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/health") { + return Response.json({ + status: "ok", + }); + } + if (url.pathname === "/api/services/discovery") { + return Response.json({ + success: true, + configuredServices: ["n8n", "yt-dlp"], + }); + } + if (url.pathname === "/call" && req.method === "POST") { + if (process.env.MOCK_REMOTE_AUTH_FAIL === "1") { + return new Response("unauthorized", { status: 401 }); + } + return Response.json({ + success: true, + result: { + routed: "remote", + request: await req.json(), + }, + }); + } + return new Response("Not Found", { status: 404 }); + }, +}); + +console.log(JSON.stringify({ port: server.port })); + +const shutdown = () => { + server.stop(true); + process.exit(0); +}; + +process.on("SIGTERM", shutdown); +process.on("SIGINT", shutdown); + +await new Promise(() => {}); diff --git a/tests/fixtures/mock-vaultwarden-command.ts b/tests/fixtures/mock-vaultwarden-command.ts new file mode 100644 index 0000000..f866d3d --- /dev/null +++ b/tests/fixtures/mock-vaultwarden-command.ts @@ -0,0 +1,24 @@ +const paramsIndex = Bun.argv.indexOf("--params"); +const paramsRaw = paramsIndex >= 0 ? Bun.argv[paramsIndex + 1] : "{}"; +const params = JSON.parse(paramsRaw ?? "{}") as { query?: string }; + +if (params.query === "fixture") { + console.log(JSON.stringify({ + success: true, + result: { + fields: { + token: "fixture-token", + }, + }, + })); + process.exit(0); +} + +if (params.query === "slow") { + await new Promise((resolve) => setTimeout(resolve, 5_000)); + console.log(JSON.stringify({ success: true, result: "too-late" })); + process.exit(0); +} + +console.error("missing fixture"); +process.exit(1); diff --git a/tests/integration/cli-binary.test.ts b/tests/integration/cli-binary.test.ts index ba41c36..052d4ba 100644 --- a/tests/integration/cli-binary.test.ts +++ b/tests/integration/cli-binary.test.ts @@ -3,6 +3,41 @@ import { resolve } from "path"; import { runCli } from "../test-helpers/run-cli.ts"; const PROJECT_ROOT = resolve(import.meta.dir, "../.."); +const REMOTE_DAEMON_FIXTURE = resolve( + PROJECT_ROOT, + "tests/fixtures/mock-remote-daemon.ts", +); + +async function startMockRemoteDaemon(env?: Record): Promise<{ + url: string; + stop: () => Promise; +}> { + const proc = Bun.spawn(["bun", "run", REMOTE_DAEMON_FIXTURE], { + cwd: PROJECT_ROOT, + env: { ...process.env, ...env }, + stdout: "pipe", + stderr: "pipe", + }); + const reader = proc.stdout.getReader(); + const decoder = new TextDecoder(); + let buffer = ""; + while (!buffer.includes("\n")) { + const chunk = await reader.read(); + if (chunk.done) { + throw new Error("mock remote daemon exited before reporting its port"); + } + buffer += decoder.decode(chunk.value); + } + const firstLine = buffer.split("\n")[0]!; + const { port } = JSON.parse(firstLine) as { port: number }; + return { + url: `http://localhost:${port}`, + stop: async () => { + proc.kill("SIGTERM"); + await proc.exited; + }, + }; +} describe("CLI dispatch", () => { test("no args: exitCode 0, stdout contains help text", () => { @@ -84,6 +119,98 @@ describe("services command - config integration", () => { expect(names).toContain("vault"); }); + test("services command merges remote-only services from daemon discovery", async () => { + const remote = await startMockRemoteDaemon(); + try { + const configPath = resolve( + PROJECT_ROOT, + "tests/fixtures/valid-config.json", + ); + const result = runCli(["services"], { + MCP2CLI_CONFIG: configPath, + MCP2CLI_REMOTE_URL: remote.url, + }); + expect(result.exitCode).toBe(0); + const data = JSON.parse(result.stdout); + const remoteOnly = data.services.find((s: { name: string }) => s.name === "yt-dlp"); + expect(remoteOnly).toMatchObject({ + name: "yt-dlp", + backend: "remote", + status: "remote-configured", + }); + } finally { + await remote.stop(); + } + }); + + test("services command lists remote-only services when local config is missing", async () => { + const remote = await startMockRemoteDaemon(); + try { + const result = runCli(["services"], { + MCP2CLI_CONFIG: "/tmp/nonexistent-mcp2cli-remote-config.json", + MCP2CLI_REMOTE_URL: remote.url, + }); + expect(result.exitCode).toBe(0); + const data = JSON.parse(result.stdout); + expect(data.services).toContainEqual(expect.objectContaining({ + name: "yt-dlp", + backend: "remote", + status: "remote-configured", + })); + } finally { + await remote.stop(); + } + }); + + test("remote-only service invocation routes to remote daemon", async () => { + const remote = await startMockRemoteDaemon(); + try { + const configPath = resolve( + PROJECT_ROOT, + "tests/fixtures/valid-config.json", + ); + const result = runCli(["yt-dlp", "download"], { + MCP2CLI_CONFIG: configPath, + MCP2CLI_NO_DAEMON: "", + MCP2CLI_REMOTE_RETRIES: "1", + MCP2CLI_REMOTE_URL: remote.url, + }); + expect(result.exitCode).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + success: true, + result: { + routed: "remote", + request: { service: "yt-dlp", tool: "download", params: {} }, + }, + }); + } finally { + await remote.stop(); + } + }); + + test("remote auth failure does not fall back to local daemon execution", async () => { + const remote = await startMockRemoteDaemon({ MOCK_REMOTE_AUTH_FAIL: "1" }); + try { + const configPath = resolve( + PROJECT_ROOT, + "tests/fixtures/valid-config.json", + ); + const result = runCli(["n8n", "list"], { + MCP2CLI_CONFIG: configPath, + MCP2CLI_NO_DAEMON: "", + MCP2CLI_REMOTE_RETRIES: "1", + MCP2CLI_REMOTE_URL: remote.url, + }); + + expect(result.exitCode).toBe(5); + const error = JSON.parse(result.stdout); + expect(error.code).toBe("CONNECTION_ERROR"); + expect(error.message).toContain("Remote auth failed"); + } finally { + await remote.stop(); + } + }); + test("missing config exits 1 with CONFIG_NOT_FOUND", () => { const result = runCli(["services"], { MCP2CLI_CONFIG: "/tmp/nonexistent-mcp2cli-config.json", diff --git a/tests/logger/audit.test.ts b/tests/logger/audit.test.ts index 153eb2d..4c9fd3e 100644 --- a/tests/logger/audit.test.ts +++ b/tests/logger/audit.test.ts @@ -337,6 +337,8 @@ describe("auditToolCall", () => { test("includes resolvedTool and transport when provided", async () => { auditToolCall({ path: "daemon", + userId: "skippy", + role: "agent", service: "test-svc", tool: "short_name", resolvedTool: "test-svc_short_name", @@ -348,6 +350,8 @@ describe("auditToolCall", () => { await flushAuditQueue(); const entries = await readAuditFile(); + expect(entries[0]!.userId).toBe("skippy"); + expect(entries[0]!.role).toBe("agent"); expect(entries[0]!.resolvedTool).toBe("test-svc_short_name"); expect(entries[0]!.transport).toBe("stdio"); }); diff --git a/tests/process/client-auth-refresh.test.ts b/tests/process/client-auth-refresh.test.ts new file mode 100644 index 0000000..f991ba7 --- /dev/null +++ b/tests/process/client-auth-refresh.test.ts @@ -0,0 +1,136 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { createDaemonServer } from "../../src/daemon/server.ts"; +import { ConnectionPool } from "../../src/daemon/pool.ts"; +import { IdleTimer } from "../../src/daemon/idle.ts"; +import { MetricsCollector } from "../../src/daemon/metrics.ts"; +import { TokenAuthProvider } from "../../src/daemon/auth-provider.ts"; +import { clearLocalTokenCache, fetchDaemonApi } from "../../src/process/client.ts"; +import type { ServicesConfig } from "../../src/config/index.ts"; + +const EMPTY_CONFIG: ServicesConfig = { services: {} }; + +describe("client token refresh", () => { + let tempDir: string; + let originalEnv: Record; + let servers: ReturnType[] = []; + let pools: ConnectionPool[] = []; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "mcp2cli-client-refresh-test-")); + originalEnv = { + MCP2CLI_PID_FILE: process.env.MCP2CLI_PID_FILE, + MCP2CLI_SOCKET_PATH: process.env.MCP2CLI_SOCKET_PATH, + MCP2CLI_TOKENS_FILE: process.env.MCP2CLI_TOKENS_FILE, + MCP2CLI_TOKEN_REFRESH_WINDOW_MS: process.env.MCP2CLI_TOKEN_REFRESH_WINDOW_MS, + MCP2CLI_TOKEN_TTL_MS: process.env.MCP2CLI_TOKEN_TTL_MS, + MCP2CLI_AUTH_TOKEN: process.env.MCP2CLI_AUTH_TOKEN, + MCP_TOKEN: process.env.MCP_TOKEN, + }; + delete process.env.MCP2CLI_AUTH_TOKEN; + delete process.env.MCP_TOKEN; + clearLocalTokenCache(); + }); + + afterEach(async () => { + for (const server of servers) server.stop(true); + for (const pool of pools) await pool.closeAll(); + servers = []; + pools = []; + for (const [key, value] of Object.entries(originalEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + clearLocalTokenCache(); + await rm(tempDir, { recursive: true, force: true }); + }); + + test("refreshes a near-expiry local token before retryable daemon API calls", async () => { + const socketPath = join(tempDir, "daemon.sock"); + const pidFile = join(tempDir, "daemon.pid"); + const tokensPath = join(tempDir, "tokens.json"); + const oldExpiresAt = new Date(Date.now() + 5 * 60 * 1000).toISOString(); + await Bun.write(pidFile, String(process.pid)); + await Bun.write(tokensPath, JSON.stringify({ + tokens: [{ + id: "rico", + token: "old-token", + role: "admin", + expiresAt: oldExpiresAt, + }], + }, null, 2)); + process.env.MCP2CLI_PID_FILE = pidFile; + process.env.MCP2CLI_SOCKET_PATH = socketPath; + process.env.MCP2CLI_TOKENS_FILE = tokensPath; + process.env.MCP2CLI_TOKEN_REFRESH_WINDOW_MS = String(10 * 60 * 1000); + process.env.MCP2CLI_TOKEN_TTL_MS = String(60 * 60 * 1000); + + const pool = new ConnectionPool(); + pools.push(pool); + const server = createDaemonServer({ + listenConfig: { mode: "unix", socketPath }, + pool, + config: EMPTY_CONFIG, + idleTimer: new IdleTimer(60000, () => {}), + onShutdown: () => {}, + authProvider: await TokenAuthProvider.load(tokensPath), + metrics: new MetricsCollector(), + }); + servers.push(server); + + const result = await fetchDaemonApi("GET", "/api/auth/me") as { success: boolean; userId: string; role: string }; + expect(result).toEqual({ success: true, userId: "rico", role: "admin" }); + + const onDisk = await Bun.file(tokensPath).json() as { tokens: Array<{ token: string; expiresAt: string }> }; + expect(onDisk.tokens[0]!.token).not.toBe("old-token"); + expect(onDisk.tokens[0]!.expiresAt).not.toBe(oldExpiresAt); + }); + + test("skips expired admin tokens when selecting local daemon credentials", async () => { + const socketPath = join(tempDir, "daemon.sock"); + const pidFile = join(tempDir, "daemon.pid"); + const tokensPath = join(tempDir, "tokens.json"); + await Bun.write(pidFile, String(process.pid)); + await Bun.write(tokensPath, JSON.stringify({ + tokens: [ + { + id: "expired-admin", + token: "expired-token", + role: "admin", + expiresAt: new Date(Date.now() - 60_000).toISOString(), + }, + { + id: "valid-admin", + token: "valid-token", + role: "admin", + expiresAt: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + }, + ], + }, null, 2)); + process.env.MCP2CLI_PID_FILE = pidFile; + process.env.MCP2CLI_SOCKET_PATH = socketPath; + process.env.MCP2CLI_TOKENS_FILE = tokensPath; + process.env.MCP2CLI_TOKEN_REFRESH_WINDOW_MS = String(10 * 60 * 1000); + + const pool = new ConnectionPool(); + pools.push(pool); + const server = createDaemonServer({ + listenConfig: { mode: "unix", socketPath }, + pool, + config: EMPTY_CONFIG, + idleTimer: new IdleTimer(60000, () => {}), + onShutdown: () => {}, + authProvider: await TokenAuthProvider.load(tokensPath), + metrics: new MetricsCollector(), + }); + servers.push(server); + + const result = await fetchDaemonApi("GET", "/api/auth/me") as { success: boolean; userId: string; role: string }; + expect(result).toEqual({ success: true, userId: "valid-admin", role: "admin" }); + }); +}); diff --git a/tests/process/client-remote.test.ts b/tests/process/client-remote.test.ts index e678981..59d7d05 100644 --- a/tests/process/client-remote.test.ts +++ b/tests/process/client-remote.test.ts @@ -1,6 +1,15 @@ import { describe, test, expect, beforeEach, afterEach } from "bun:test"; import { getRemoteConfig } from "../../src/daemon/paths.ts"; import { checkRemoteHealth } from "../../src/process/liveness.ts"; +import { + clearRemoteServiceCache, + getRemoteServiceAvailability, + getRemoteServiceNames, +} from "../../src/process/remote-discovery.ts"; +import { clearClientConfigCache, resolveSource } from "../../src/process/client.ts"; +import { join } from "node:path"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; describe("getRemoteConfig", () => { const originalUrl = process.env.MCP2CLI_REMOTE_URL; @@ -110,3 +119,223 @@ describe("checkRemoteHealth", () => { expect(result.status).toBe("ok"); }); }); + +describe("remote service discovery", () => { + const originalUrl = process.env.MCP2CLI_REMOTE_URL; + const originalToken = process.env.MCP2CLI_AUTH_TOKEN; + const originalTtl = process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS; + let server: ReturnType; + let baseUrl: string; + + beforeEach(() => { + clearRemoteServiceCache(); + process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS = "0"; + server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/api/services/discovery") { + return Response.json({ + success: true, + version: "0.3.3", + configuredServices: ["yt-dlp", "stealth-browser"], + }); + } + return new Response("Not Found", { status: 404 }); + }, + }); + baseUrl = `http://localhost:${server.port}`; + process.env.MCP2CLI_REMOTE_URL = baseUrl; + delete process.env.MCP2CLI_AUTH_TOKEN; + }); + + afterEach(() => { + server.stop(true); + clearRemoteServiceCache(); + if (originalUrl !== undefined) { + process.env.MCP2CLI_REMOTE_URL = originalUrl; + } else { + delete process.env.MCP2CLI_REMOTE_URL; + } + if (originalToken !== undefined) { + process.env.MCP2CLI_AUTH_TOKEN = originalToken; + } else { + delete process.env.MCP2CLI_AUTH_TOKEN; + } + if (originalTtl !== undefined) { + process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS = originalTtl; + } else { + delete process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS; + } + }); + + test("lists remote configured services from authenticated daemon discovery", async () => { + await expect(getRemoteServiceNames()).resolves.toEqual(["yt-dlp", "stealth-browser"]); + }); + + test("distinguishes hosted and non-hosted services", async () => { + await expect(getRemoteServiceAvailability("yt-dlp")).resolves.toBe("hosted"); + await expect(getRemoteServiceAvailability("king-secrets")).resolves.toBe("not-hosted"); + }); + + test("returns no-remote without MCP2CLI_REMOTE_URL", async () => { + delete process.env.MCP2CLI_REMOTE_URL; + clearRemoteServiceCache(); + await expect(getRemoteServiceAvailability("yt-dlp")).resolves.toBe("no-remote"); + }); + + test("does not cache failed discovery snapshots", async () => { + let fail = true; + server.stop(true); + clearRemoteServiceCache(); + server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/api/services/discovery") { + if (fail) { + fail = false; + return new Response("temporary failure", { status: 500 }); + } + return Response.json({ + success: true, + configuredServices: ["recovered"], + }); + } + return new Response("Not Found", { status: 404 }); + }, + }); + process.env.MCP2CLI_REMOTE_URL = `http://localhost:${server.port}`; + process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS = "60000"; + + await expect(getRemoteServiceAvailability("recovered")).resolves.toBe("unknown"); + await expect(getRemoteServiceAvailability("recovered")).resolves.toBe("hosted"); + }); +}); + +describe("remote-aware source resolution", () => { + const originalUrl = process.env.MCP2CLI_REMOTE_URL; + const originalConfig = process.env.MCP2CLI_CONFIG; + const originalTtl = process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS; + let server: ReturnType; + let testDir: string; + let configPath: string; + + beforeEach(async () => { + clearRemoteServiceCache(); + clearClientConfigCache(); + process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS = "0"; + testDir = await mkdtemp(join(tmpdir(), "mcp2cli-remote-source-test-")); + configPath = join(testDir, "services.json"); + server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/api/services/discovery") { + return Response.json({ + success: true, + configuredServices: ["remote-only", "hosted-local"], + }); + } + return new Response("Not Found", { status: 404 }); + }, + }); + process.env.MCP2CLI_REMOTE_URL = `http://localhost:${server.port}`; + process.env.MCP2CLI_CONFIG = configPath; + }); + + afterEach(async () => { + server.stop(true); + clearRemoteServiceCache(); + clearClientConfigCache(); + await rm(testDir, { recursive: true, force: true }); + if (originalUrl !== undefined) { + process.env.MCP2CLI_REMOTE_URL = originalUrl; + } else { + delete process.env.MCP2CLI_REMOTE_URL; + } + if (originalConfig !== undefined) { + process.env.MCP2CLI_CONFIG = originalConfig; + } else { + delete process.env.MCP2CLI_CONFIG; + } + if (originalTtl !== undefined) { + process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS = originalTtl; + } else { + delete process.env.MCP2CLI_REMOTE_SERVICE_CACHE_TTL_MS; + } + }); + + async function writeServices(services: Record): Promise { + await Bun.write(configPath, JSON.stringify({ services }, null, 2)); + clearClientConfigCache(); + } + + test("uses remote for remote-only services discovered via authenticated discovery", async () => { + await writeServices({ + local: { backend: "stdio", command: "echo" }, + }); + await expect(resolveSource("remote-only")).resolves.toBe("remote"); + }); + + test("uses local when the remote daemon does not host an unpinned local service", async () => { + await writeServices({ + local: { backend: "stdio", command: "echo" }, + }); + await expect(resolveSource("local")).resolves.toBe("local"); + }); + + test("uses local for configured services when remote discovery is unavailable", async () => { + server.stop(true); + clearRemoteServiceCache(); + await writeServices({ + local: { backend: "stdio", command: "echo" }, + }); + await expect(resolveSource("local")).resolves.toBe("local"); + }); + + test("explicit source wins over platform and remote discovery inference", async () => { + await writeServices({ + local: { + backend: "stdio", + command: "echo", + source: "remote-local", + platforms: ["plan9"], + }, + }); + await expect(resolveSource("local")).resolves.toBe("remote-local"); + }); + + test("platforms prefer local when current OS is allowed", async () => { + await writeServices({ + local: { + backend: "stdio", + command: "echo", + platforms: [process.platform], + }, + }); + await expect(resolveSource("local")).resolves.toBe("local"); + }); + + test("platforms prefer remote when current OS is not allowed", async () => { + await writeServices({ + local: { + backend: "stdio", + command: "echo", + platforms: ["not-this-platform"], + }, + }); + await expect(resolveSource("local")).resolves.toBe("local"); + }); + + test("platforms use remote only when unsupported service is hosted remotely", async () => { + await writeServices({ + "hosted-local": { + backend: "stdio", + command: "echo", + platforms: ["not-this-platform"], + }, + }); + await expect(resolveSource("hosted-local")).resolves.toBe("remote"); + }); +}); diff --git a/tests/secrets/refs.test.ts b/tests/secrets/refs.test.ts new file mode 100644 index 0000000..da6fd92 --- /dev/null +++ b/tests/secrets/refs.test.ts @@ -0,0 +1,117 @@ +import { afterEach, describe, test, expect, mock } from "bun:test"; +import { + hasSecretRefs, + resolveServiceSecretRefs, + SecretResolutionError, + VaultwardenSecretResolver, +} from "../../src/secrets/index.ts"; +import type { SecretResolver } from "../../src/secrets/index.ts"; +import type { ServiceConfig } from "../../src/config/index.ts"; +import { resolve } from "node:path"; + +describe("secret refs", () => { + test("detects nested secret refs", () => { + expect(hasSecretRefs({ headers: { Authorization: "Bearer ${secret:token}" } })).toBe(true); + expect(hasSecretRefs({ headers: { Authorization: "Bearer token" } })).toBe(false); + }); + + test("resolves secret refs in service config without mutating original", async () => { + const resolver: SecretResolver = { + resolve: mock(async (ref: string) => { + if (ref === "open-brain#url") return "http://ct216.example/mcp"; + if (ref === "open-brain#token") return "resolved-token"; + throw new Error(`unexpected ref ${ref}`); + }), + }; + const service = { + backend: "http", + url: "${secret:open-brain#url}", + headers: { Authorization: "Bearer ${secret:open-brain#token}" }, + } satisfies ServiceConfig; + + const resolved = await resolveServiceSecretRefs("open-brain", service, resolver); + expect(resolved).toEqual({ + backend: "http", + url: "http://ct216.example/mcp", + headers: { Authorization: "Bearer resolved-token" }, + }); + expect(service.url).toBe("${secret:open-brain#url}"); + }); + + test("throws on empty secret ref", async () => { + const resolver: SecretResolver = { resolve: mock(async () => "unused") }; + const service = { + backend: "stdio", + command: "echo", + args: ["${secret:}"], + env: {}, + } satisfies ServiceConfig; + await expect(resolveServiceSecretRefs("bad", service, resolver)).rejects.toThrow(SecretResolutionError); + }); + + test("resolves secret values containing replacement metacharacters literally", async () => { + const resolver: SecretResolver = { + resolve: mock(async () => "$&-$1-$$"), + }; + const service = { + backend: "stdio", + command: "echo", + args: ["value=${secret:literal}"], + env: {}, + } satisfies ServiceConfig; + + const resolved = await resolveServiceSecretRefs("literal", service, resolver); + expect(resolved.backend).toBe("stdio"); + if (resolved.backend === "stdio") { + expect(resolved.args).toEqual(["value=$&-$1-$$"]); + } + }); +}); + +describe("VaultwardenSecretResolver", () => { + const originalCommand = process.env.MCP2CLI_VAULTWARDEN_COMMAND; + const originalArgs = process.env.MCP2CLI_VAULTWARDEN_COMMAND_ARGS; + const originalTimeout = process.env.MCP2CLI_VAULTWARDEN_TIMEOUT_MS; + + afterEach(() => { + if (originalCommand !== undefined) { + process.env.MCP2CLI_VAULTWARDEN_COMMAND = originalCommand; + } else { + delete process.env.MCP2CLI_VAULTWARDEN_COMMAND; + } + if (originalArgs !== undefined) { + process.env.MCP2CLI_VAULTWARDEN_COMMAND_ARGS = originalArgs; + } else { + delete process.env.MCP2CLI_VAULTWARDEN_COMMAND_ARGS; + } + if (originalTimeout !== undefined) { + process.env.MCP2CLI_VAULTWARDEN_TIMEOUT_MS = originalTimeout; + } else { + delete process.env.MCP2CLI_VAULTWARDEN_TIMEOUT_MS; + } + }); + + test("extracts field paths from wrapped mcp2cli JSON output", async () => { + process.env.MCP2CLI_VAULTWARDEN_COMMAND = Bun.argv[0]!; + process.env.MCP2CLI_VAULTWARDEN_COMMAND_ARGS = JSON.stringify([ + "run", + resolve(import.meta.dir, "../fixtures/mock-vaultwarden-command.ts"), + ]); + const resolver = new VaultwardenSecretResolver(); + const value = await resolver.resolve( + `fixture#fields.token`, + ); + expect(value).toBe("fixture-token"); + }); + + test("times out stalled Vaultwarden lookups", async () => { + process.env.MCP2CLI_VAULTWARDEN_COMMAND = Bun.argv[0]!; + process.env.MCP2CLI_VAULTWARDEN_COMMAND_ARGS = JSON.stringify([ + "run", + resolve(import.meta.dir, "../fixtures/mock-vaultwarden-command.ts"), + ]); + process.env.MCP2CLI_VAULTWARDEN_TIMEOUT_MS = "25"; + const resolver = new VaultwardenSecretResolver(); + await expect(resolver.resolve("slow")).rejects.toThrow(SecretResolutionError); + }); +}); diff --git a/tests/test-helpers/run-cli.ts b/tests/test-helpers/run-cli.ts index a8f40af..9c1d8ff 100644 --- a/tests/test-helpers/run-cli.ts +++ b/tests/test-helpers/run-cli.ts @@ -19,7 +19,18 @@ export function runCli( const proc = Bun.spawnSync(["bun", "run", "src/cli/index.ts", ...args], { cwd: projectRoot, - env: { ...process.env, MCP2CLI_NO_DAEMON: "1", ...env }, + env: { + ...process.env, + MCP2CLI_NO_DAEMON: "1", + MCP2CLI_REMOTE_URL: "", + MCP_HOST: "", + MCP2CLI_PID_FILE: "", + MCP2CLI_SOCKET_PATH: "", + MCP2CLI_TOKENS_FILE: "", + MCP2CLI_AUTH_TOKEN: "", + MCP_TOKEN: "", + ...env, + }, stdout: "pipe", stderr: "pipe", }); From 1ba3924396ffe90426c9bfb2dfecefb27aa2def3 Mon Sep 17 00:00:00 2001 From: Rodaddy Date: Mon, 15 Jun 2026 18:55:49 -0400 Subject: [PATCH 2/5] fix: wire MCP client capabilities --- _agents.md | 88 ++++++++++ agents.md | 239 ++++++++++++++++++++++++++ src/connection/capabilities.ts | 31 ++++ src/connection/client.ts | 5 +- src/connection/http-transport.ts | 7 +- src/connection/websocket-transport.ts | 5 +- tests/connection/capabilities.test.ts | 17 ++ 7 files changed, 382 insertions(+), 10 deletions(-) create mode 100644 _agents.md create mode 100644 agents.md create mode 100644 src/connection/capabilities.ts create mode 100644 tests/connection/capabilities.test.ts diff --git a/_agents.md b/_agents.md new file mode 100644 index 0000000..a11396a --- /dev/null +++ b/_agents.md @@ -0,0 +1,88 @@ +# mcp2cli Agent Quick Router + +Read `agents.md` first. This file is the compact repo-local checklist for future +sessions in `/Volumes/ThunderBolt/Development/mcp2cli`. + +## Required Policy Refresh + +Before task action after compaction, resume, checkpoint, phase change, or +long-running continuation, reread: + +- `~/.codex/AGENTS.md` +- `/Volumes/ThunderBolt/Development/AGENTS.md` +- `agents.md` +- this `_agents.md` +- triggered SOPs in `/Volumes/ThunderBolt/Development/_DOCS` + +Do not continue from memory alone. + +## Triggered SOPs + +- Code edits: `_DOCS/CODING_STANDARDS.md` +- Git, branch, commit, PR, issue, label, milestone, or project work: + `_DOCS/GIT_STANDARDS.md` +- GitHub Project board mutation: `_DOCS/BOARD_FIELDS.md` plus any repo-local map +- Goal run, fanout, swarm, manager, board-manager, or long-running work: + `_DOCS/AGENT_WORKFLOW.md` +- Infra, host, IP, SSH, LXC, VM, Proxmox, runner, DNS, Caddy, Authentik, mounts, + or deploy work: `_DOCS/INFRASTRUCTURE_SOP.md` +- Domain language, PAI terms, infra names, shorthand, or workflow terms: + `_DOCS/GLOSSARY.md` + +## Non-Negotiables + +- This is Codex, not Claude Code. Use Codex-native tools and config. +- Skills first when a skill matches the task. +- Use `/opt/homebrew/bin/bash` when bash is needed. +- Use `/Volumes/ThunderBolt/_tmp` for temp files, scratch clones, generated + patches, downloads, review checkouts, and temporary worktrees. +- Clean temp work with `/Users/rico/.codex/tools/codex-clean-tmp `. +- Do not use raw `rm -rf` for `/Volumes/ThunderBolt/_tmp` cleanup. +- Do not create hidden Codex-owned folders or `.reports/`. +- Use visible durable artifact paths such as `reports/`, `docs/`, or + user-requested locations. +- Never put secrets in git, logs, reports, fixtures, PRs, issues, screenshots, + or generated artifacts. +- Do not edit, stage, or commit on `main` unless explicitly approved. +- Never use `claude --worktree` directly. +- Temporary/review worktrees belong under `/Volumes/ThunderBolt/_tmp`, not under + `/Volumes/ThunderBolt/Development`. +- Use shared todo stores through `add-todo` and `check-todos`; do not create a + Codex-only todo database. + +## MCP And Service Access + +- Direct Codex MCP servers are intentionally absent. +- Use `mcp2cli` for MCP-backed tools. +- Use `mcp2cli services`, `mcp2cli --help`, and + `mcp2cli schema .` for discovery. +- Use one `mcp2cli vaultwarden-secrets get_credential --params + '{"query":""}'` call for credentials; never chain credential lookups. +- For n8n workflow ops, use MCP/mcp2cli-backed tools only unless the user + explicitly says to use SSH. + +## Goal Runs + +- Establish a controller before broad implementation continues. +- Workers provide evidence; they do not declare board state, PR readiness, merge + readiness, issue closure, or goal completion. +- Verify worker claims against live repo, GitHub, board, CI, and validation + evidence before acting. +- Continue through recoverable failures; report only real blockers with + evidence. + +## Infra + +- Never guess IPs, container IDs, service locations, hostnames, runner targets, + or credentials. +- Use direct IPs for SSH after verifying host inventory. +- Hostmap sources: + - macOS: `/Volumes/collab/hostmap.json` + - Linux/LXC: `/mnt/collab/hostmap.json` + +## Persona And Hooks + +- Default persona is Skippy; `ACTIVE_PERSONA=skippy` is expected. +- Hooks are safety rails only. +- No wildcard PreToolUse or PostToolUse hooks. +- No read-only Bash hook gauntlet. diff --git a/agents.md b/agents.md new file mode 100644 index 0000000..a13f0ae --- /dev/null +++ b/agents.md @@ -0,0 +1,239 @@ +# mcp2cli Local Agent Policy + +This repo is part of the canonical live Codex workspace under +`/Volumes/ThunderBolt/Development/mcp2cli`. + +Future sessions working in this repo must treat this file as the repo-local +router, then apply the global and Development-wide routers: + +- Global: `~/.codex/AGENTS.md` +- Development-wide: `/Volumes/ThunderBolt/Development/AGENTS.md` +- Repo-local quick alias: `_agents.md` + +If a higher-level policy is stricter, follow the stricter rule. If this file is +more specific to `mcp2cli`, follow this file. On this case-insensitive volume, +opening `AGENTS.md` resolves to this same `agents.md` file. + +## Runtime Policy + +- This is Codex, not Claude Code. Prefer Codex-native tools, slash commands, and + Codex config. +- PAI skills are workflow and context docs, not Codex slash commands. +- Use natural language triggers such as "search brain", "session wrap", + "critical-mode", "add todo", and "check todos". +- Do not load Claude Code hooks, command status lines, marketplaces, or direct + MCP server blocks. +- Use `/opt/homebrew/bin/bash`, never `/bin/bash`, for shell workflows when a + bash shell is needed. +- Prefer `bun` for Node/TypeScript, `uv` for Python, and `brew` for macOS + packages. +- Use `rg` and `rg --files` before slower search tools. + +## PAI LAWs For Codex + +- Skills first: if a Codex skill matches the task, read and use it instead of + rebuilding the workflow manually. +- Critical thinking: challenge weak plans, security risks, scaling problems, and + maintenance traps. Use `critical-mode` when stronger pushback is needed. +- Explain before major changes: for broad, risky, or multi-file work, state the + plan and scope before editing. +- Verify significant implementation: run relevant tests, build, typecheck, or a + focused manual check before calling work done. +- Shared todo stores: Codex and Claude use the same files, not separate todo + databases. +- No secrets in git, logs, reports, fixtures, PRs, issues, screenshots, or + generated artifacts. Use env files or `vaultwarden-secrets` through `mcp2cli`; + never invent credentials. +- Protected branches: do not commit directly to main, master, or protected + branches unless explicitly approved. +- Search memory when it matters: use `pai-brain` or + `mcp2cli open-brain search_all` when prior decisions or indexed context are + likely relevant; do not force memory before every exact-path read. +- Subagents are optional: use them only when explicitly requested or when + parallel work is genuinely useful under Codex rules. +- Ask clearly: use Codex `request_user_input` only when available; otherwise ask + concise plain-text questions. + +## Policy Refresh And Compaction Recovery + +- After any context compaction, resume, checkpoint, or long-running goal-run + continuation, task action is blocked until the agent rereads: + - `~/.codex/AGENTS.md` + - `/Volumes/ThunderBolt/Development/AGENTS.md` + - this repo-local `agents.md` + - `_agents.md` + - every mandatory SOP triggered by the next action +- During long goal runs, refresh policy at every meaningful phase change or + after roughly 8-10 worker/manager rounds, whichever comes first. +- For goal runs, the refresh must include `_DOCS/AGENT_WORKFLOW.md`. +- If board work is active, also read `_DOCS/BOARD_FIELDS.md` and any repo-local + board map. +- If branch, commit, PR, issue, label, milestone, or project work is active, + also read `_DOCS/GIT_STANDARDS.md`. +- If host, IP, SSH, LXC, VM, Proxmox, runner, DNS, Caddy, Authentik, mounts, or + deploy facts are involved, also read `_DOCS/INFRASTRUCTURE_SOP.md`. +- If domain language, PAI terms, infra names, shorthand, or workflow terms + matter, read `_DOCS/GLOSSARY.md` and only the routed topic glossary that is + needed. +- Acting after compaction, resume, or phase change without refreshing policy is + a protocol violation. Stop the drift, reread the router and relevant SOP, + restate the controlling rule in one sentence, correct worker prompts or + replace drifting workers, then continue unless there is a real approval gate + or blocker. + +## Mandatory SOP Routing + +Detailed shared SOPs live in `/Volumes/ThunderBolt/Development/_DOCS`. + +- Creating or modifying code: read `_DOCS/CODING_STANDARDS.md`. +- Branch, commit, PR, issue, label, milestone, or project-board work: read + `_DOCS/GIT_STANDARDS.md`. +- GitHub Project board mutations: read `_DOCS/BOARD_FIELDS.md` and any + repo-local board map first. +- Goal run, fanout, review swarm, manager/board-manager workflow, or + long-running work: read `_DOCS/AGENT_WORKFLOW.md`. +- Infrastructure, hosts, IPs, SSH, LXC, VM, Proxmox, TrueNAS, runners, DNS, + Caddy, Authentik, mounts, or deployments: read + `_DOCS/INFRASTRUCTURE_SOP.md`. +- Domain language or shorthand: read `_DOCS/GLOSSARY.md`. + +Repo-local docs, context, glossary, and standards override these SOPs when they +are stricter or more specific. + +## Temporary Work Policy + +- Use `/Volumes/ThunderBolt/_tmp` for all Codex temporary files, downloaded + artifacts, generated patches, scratch clones, review checkouts, PR-review + worktrees, and one-off test homes. +- Do not create temporary `codex-*`, review, scratch, or experiment directories + in `/Volumes/ThunderBolt/Development` or this repo root. +- Do not use `/tmp` or `/private/tmp` for Codex-controlled temp work when + `/Volumes/ThunderBolt/_tmp` is available. +- If a tool hard-requires system temp, keep only tool-managed transient files + there and put all named artifacts under `/Volumes/ThunderBolt/_tmp`. +- Clean up your own temporary files under `/Volumes/ThunderBolt/_tmp` without + asking by using `/Users/rico/.codex/tools/codex-clean-tmp `. +- Do not use raw `rm -rf` for `/Volumes/ThunderBolt/_tmp` cleanup in agent + workflows. +- Before starting a PR review or temporary worktree, create a clearly named + directory under `/Volumes/ThunderBolt/_tmp` and run Codex with `-C ` + when launching a nested session. + +## Worktree And Git Policy + +- Never use `claude --worktree` directly. +- Use `gwt "name"` or `gwt "name" codex|pi|claude` only for durable, + user-approved worktrees. +- PR review worktrees, review clones, scratch worktrees, and temporary + `codex-*` work must live under `/Volumes/ThunderBolt/_tmp`, not beside repos + in `/Volumes/ThunderBolt/Development`. +- Reading, status checks, and fetches on `main` are allowed. +- Editing, staging, or committing on `main` is not allowed unless explicitly + approved. +- For implementation work, create a focused branch from clean current `main` + when required by the task. +- In dedicated implementation worktrees, commit scoped agent changes before + handoff unless the user explicitly says review-only or no-commit. + +## Local Artifact Policy + +- Do not create new Codex-owned hidden folders in this repo or under + `/Volumes/ThunderBolt/Development`. +- For durable project artifacts, use visible paths such as `reports/`, `docs/`, + `planning/`, `fixtures/`, or a user-requested location. +- For session wrap, prefer direct OpenBrain capture through + `mcp2cli open-brain`. +- Only write visible repo files for session wrap when OpenBrain is unavailable + or the user explicitly asks for local files. +- Never write session-wrap output to `.reports/`. + +## Shared Todo Stores + +- Project todos: `.planning/todos/pending/` +- Global active ideas: `/Volumes/ThunderBolt/Development/.claude_ideas/active/` +- Someday ideas: `/Volumes/ThunderBolt/Development/.claude_ideas/someday/` +- Use the `add-todo` and `check-todos` skills for todo work. +- Do not create a Codex-only todo store. + +## MCP Policy + +- Direct Codex MCP servers are intentionally absent here. +- MCP-backed tools go through `mcp2cli`. +- Run `mcp2cli services` for the full service list. +- For any service, `mcp2cli --help` lists tools. +- For tool params, use `mcp2cli schema .`. +- Credentials: use one `mcp2cli vaultwarden-secrets get_credential --params + '{"query":""}'` call. Never chain multiple credential lookups. + +Key services and wrappers: + +| Service | Wrapper | Usage | +| --- | --- | --- | +| `open-brain` | `pai-brain` | `pai-brain search_all --params '{"query":"..."}'` | +| `vaultwarden-secrets` | direct | `mcp2cli vaultwarden-secrets get_credential --params '{"query":"..."}'` | +| `qmd` | direct | `mcp2cli qmd search --params '{"query":"..."}'` | +| `proxmox` | direct | `mcp2cli proxmox list_containers --params '{}'` | +| `home-assistant` | direct | `mcp2cli home-assistant --params '{}'` | +| `n8n` | direct | `mcp2cli n8n --params '{}'` | +| `unifi` | direct | `mcp2cli unifi --params '{}'` | + +## n8n Workflow Ops + +- Use MCP/mcp2cli-backed n8n tools only. +- If auth or MCP is broken, stop and fix that path unless the user explicitly + says to use SSH. + +## Goal-Run Execution + +- Goal runs must have a controller role. +- Worker/implementation sessions are not allowed to self-declare board state, PR + readiness, merge readiness, issue closure, or goal completion. +- Worker output is evidence, not authority. +- The controller owns live board state, issue/PR state, worker routing, + CI/check status, review gates, final readiness, and merge/closure decisions. +- Material worker claims must be verified against live repo, GitHub, board, CI, + and validation evidence before acting on them. +- Work through recoverable issues. A failed command, stale checkout, queued CI, + missing local dependency, worker stall, or temporary artifact cleanup is not a + stop condition by itself. +- Do not spawn workers merely to fill capacity. Each worker needs a concrete + deliverable that advances the run. + +## Host And Infra Policy + +- Never guess IPs, container IDs, service locations, hostnames, runner targets, + or credentials. +- Use direct IP addresses for SSH in homelab/workspace tasks after verifying the + target through host inventory. +- Current generated HOSTMAP source of truth: + - macOS: `/Volumes/collab/hostmap.json` + - Linux/LXC: `/mnt/collab/hostmap.json` +- Repo-local `HOSTMAP.md` or `HOSTMAP.json` files may be stale snapshots unless + repo-local instructions say otherwise. + +## Persona + +- Default persona: Skippy. +- `ACTIVE_PERSONA=skippy` is injected through Codex config. +- To switch tracked persona from the shell, run `pai-persona skippy`, + `pai-persona bob`, `pai-persona clarisa`, or `pai-persona april`. + +## Hook Policy + +- Hooks are safety rails only. +- No wildcard PreToolUse or PostToolUse hooks. +- No read-only Bash hook gauntlet. +- Keep blocking checks for destructive git, protected branch push/commit, + secrets/security, SSH target verification, LiteLLM self-surgery, and direct + `claude --worktree` usage. + +## Session Wrap + +Before ending or checkpointing a session: + +- Review whether `_DOCS/GLOSSARY.md`, repo-local context, glossary files, SOPs, + or this file need updates from terms or protocols learned during the session. +- Update the relevant SOP, skill, or repo-local agent file when the user + clarifies durable protocols. +- Prefer OpenBrain capture through `mcp2cli open-brain`; avoid local wrap files + unless OpenBrain is unavailable or the user asks for them. diff --git a/src/connection/capabilities.ts b/src/connection/capabilities.ts new file mode 100644 index 0000000..d0d0063 --- /dev/null +++ b/src/connection/capabilities.ts @@ -0,0 +1,31 @@ +/** + * Shared MCP client factory with protocol capability handlers. + * Configures elicitation (auto-confirm) so servers that require + * write confirmation don't hang waiting for a response. + */ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { ElicitRequestSchema } from "@modelcontextprotocol/sdk/types.js"; +import { createLogger } from "../logger/index.ts"; +import pkg from "../../package.json" with { type: "json" }; + +const log = createLogger("capabilities"); + +/** + * Create an MCP Client with elicitation support. + * Auto-accepts all elicitation requests; the caller has already + * passed RBAC before reaching the MCP layer. + */ +export function createMcpClient(): Client { + const client = new Client( + { name: "mcp2cli", version: pkg.version }, + { capabilities: { elicitation: {} } }, + ); + + client.setRequestHandler(ElicitRequestSchema, async (request) => { + const message = request.params?.message ?? "(no message)"; + log.info("elicitation_auto_accepted", { message }); + return { action: "accept" as const, content: {} }; + }); + + return client; +} diff --git a/src/connection/client.ts b/src/connection/client.ts index 4b622be..ecc9b47 100644 --- a/src/connection/client.ts +++ b/src/connection/client.ts @@ -1,10 +1,9 @@ -import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { McpTransport } from "./transport.ts"; import { ConnectionError } from "./errors.ts"; +import { createMcpClient } from "./capabilities.ts"; import type { ConnectionOptions, McpConnection } from "./types.ts"; import type { StdioService } from "../config/schema.ts"; import { createLogger } from "../logger/index.ts"; -import pkg from "../../package.json" with { type: "json" }; const log = createLogger("connection"); @@ -27,7 +26,7 @@ export async function connectToService( }; const transport = new McpTransport(connOpts); - const client = new Client({ name: "mcp2cli", version: pkg.version }); + const client = createMcpClient(); const cmdLabel = `${service.command} ${service.args.join(" ")}`; diff --git a/src/connection/http-transport.ts b/src/connection/http-transport.ts index 4e7cdab..1f03e62 100644 --- a/src/connection/http-transport.ts +++ b/src/connection/http-transport.ts @@ -3,14 +3,13 @@ * Tries StreamableHTTPClientTransport first (modern, spec 2025-03-26), * falls back to SSEClientTransport (deprecated, spec 2024-11-05). */ -import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"; import { ConnectionError } from "./errors.ts"; +import { createMcpClient } from "./capabilities.ts"; import type { McpConnection } from "./types.ts"; import type { HttpService } from "../config/schema.ts"; import { createLogger } from "../logger/index.ts"; -import pkg from "../../package.json" with { type: "json" }; const log = createLogger("http-transport"); @@ -37,7 +36,7 @@ export async function connectToHttpService( const streamableTransport = new StreamableHTTPClientTransport(url, { requestInit, }); - const client = new Client({ name: "mcp2cli", version: pkg.version }); + const client = createMcpClient(); await Promise.race([ client.connect(streamableTransport), @@ -70,7 +69,7 @@ export async function connectToHttpService( requestInit, }); - const client = new Client({ name: "mcp2cli", version: pkg.version }); + const client = createMcpClient(); await Promise.race([ client.connect(sseTransport), diff --git a/src/connection/websocket-transport.ts b/src/connection/websocket-transport.ts index 01551c2..7e39681 100644 --- a/src/connection/websocket-transport.ts +++ b/src/connection/websocket-transport.ts @@ -2,13 +2,12 @@ * WebSocket transport for connecting to remote MCP servers. * Uses the SDK's built-in WebSocketClientTransport. */ -import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { WebSocketClientTransport } from "@modelcontextprotocol/sdk/client/websocket.js"; import { ConnectionError } from "./errors.ts"; +import { createMcpClient } from "./capabilities.ts"; import type { McpConnection } from "./types.ts"; import type { WebSocketService } from "../config/schema.ts"; import { createLogger } from "../logger/index.ts"; -import pkg from "../../package.json" with { type: "json" }; const log = createLogger("websocket-transport"); @@ -26,7 +25,7 @@ export async function connectToWebSocketService( try { log.info("connecting_websocket", { url: service.url }); const transport = new WebSocketClientTransport(url); - const client = new Client({ name: "mcp2cli", version: pkg.version }); + const client = createMcpClient(); await Promise.race([ client.connect(transport), diff --git a/tests/connection/capabilities.test.ts b/tests/connection/capabilities.test.ts new file mode 100644 index 0000000..44d4167 --- /dev/null +++ b/tests/connection/capabilities.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, test } from "bun:test"; +import { createMcpClient } from "../../src/connection/capabilities.ts"; + +interface ClientInternals { + _capabilities?: { + elicitation?: object; + }; +} + +describe("createMcpClient", () => { + test("declares elicitation client capabilities", () => { + const client = createMcpClient(); + const internals = client as unknown as ClientInternals; + + expect(internals._capabilities?.elicitation).toEqual({}); + }); +}); From af1af28a12de6422af97a926aaae9c521eaeb017 Mon Sep 17 00:00:00 2001 From: Rodaddy Date: Mon, 15 Jun 2026 23:55:25 -0400 Subject: [PATCH 3/5] docs: align local agent quick router name --- agents.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agents.md b/agents.md index a13f0ae..d28bc62 100644 --- a/agents.md +++ b/agents.md @@ -8,7 +8,7 @@ router, then apply the global and Development-wide routers: - Global: `~/.codex/AGENTS.md` - Development-wide: `/Volumes/ThunderBolt/Development/AGENTS.md` -- Repo-local quick alias: `_agents.md` +- Repo-local quick alias: `_AGENTS.md` If a higher-level policy is stricter, follow the stricter rule. If this file is more specific to `mcp2cli`, follow this file. On this case-insensitive volume, From fc23980803e2795f05b0e05c3bd6add65e098aaa Mon Sep 17 00:00:00 2001 From: Rodaddy Date: Sat, 27 Jun 2026 23:57:38 -0400 Subject: [PATCH 4/5] feat: slim generated SKILL.md to a group routing index generate-skills inlined a full per-tool description table in the front SKILL.md, pushing large services well over the 300-token target (Open Brain was ~1100 tokens). The per-tool detail already lived in references/*.md, so the front skill was duplicating a flat index. generateSkillMd now emits a progressive-disclosure front skill: a routing index of tool groups, each linking to its reference file, with a flat tool-name fallback when no grouping is available. parseExistingTools reads names from both new formats (legacy quick-reference table still parsed for migration) so --diff and drift detection keep working. schema_hash in frontmatter remains the drift signal. Bump 0.3.3 -> 0.3.4. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 +++ VERSION | 2 +- package.json | 2 +- src/cli/commands/generate-skills.ts | 1 + src/generation/diff.ts | 58 +++++++++++++++++------ src/generation/templates.ts | 45 +++++++++++++----- src/generation/types.ts | 2 + tests/generation/diff.test.ts | 40 ++++++++++++++++ tests/generation/templates.test.ts | 26 +++++++--- tests/integration/generate-skills.test.ts | 5 +- 10 files changed, 151 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0222d8f..658fba7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.3.4] - 2026-06-28 + +### Changed +- `generate-skills` now emits a slim front `SKILL.md` (progressive disclosure): a routing index of tool *groups* linking to `references/*.md`, instead of inlining a full per-tool description table. Keeps the front skill under the 300-token target for large services (Open Brain dropped from ~1100 tokens). Per-tool detail remains in the reference files. +- `parseExistingTools` reads tool names from the new group-index and flat-fallback formats (legacy quick-reference table still parsed for migration), so `--diff` and drift detection keep working. + ## [0.3.1] - 2026-06-09 ### Added diff --git a/VERSION b/VERSION index 1c09c74..42045ac 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.3 +0.3.4 diff --git a/package.json b/package.json index acf37f8..87151c6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "mcp2cli", - "version": "0.3.3", + "version": "0.3.4", "description": "CLI bridge that wraps MCP servers as bash-invokable commands, recovering ~11K tokens of context window per session", "type": "module", "license": "MIT", diff --git a/src/cli/commands/generate-skills.ts b/src/cli/commands/generate-skills.ts index 59708f1..ce857ce 100644 --- a/src/cli/commands/generate-skills.ts +++ b/src/cli/commands/generate-skills.ts @@ -262,6 +262,7 @@ export const handleGenerateSkills = async (args: string[]): Promise => { description: `MCP tools for ${serviceName}`, tools, triggerKeywords, + groups, generatedAt, schemaHash, toolCount: tools.length, diff --git a/src/generation/diff.ts b/src/generation/diff.ts index e4ea528..2de2fd4 100644 --- a/src/generation/diff.ts +++ b/src/generation/diff.ts @@ -27,54 +27,84 @@ export interface SkillDiffResult { } /** - * Parse tool names and descriptions from an existing SKILL.md file. - * Extracts from the quick reference table (| Tool | Description |). + * Parse tool names from an existing SKILL.md front skill. + * + * The slim front skill lists tools two ways, both supported here: + * - a "Tool Groups" table whose middle column is a comma-separated tool list + * (`| Group | Tools | Reference |`), or + * - a flat fallback bullet list (`- tool_name`) when no grouping was available. + * + * Descriptions live in `references/*.md`, not the front skill, so they are not + * recovered here -- name-level add/remove diffing plus the frontmatter + * `schema_hash` are the drift signals for the front skill. + * + * The legacy `| Tool | Description |` quick-reference table is still parsed so + * diffs against pre-existing generated skills keep working during migration. */ export function parseExistingTools(skillContent: string): ToolSummary[] { const tools: ToolSummary[] = []; const lines = skillContent.split("\n"); - let inTable = false; + // Mode: parsing rows of a markdown table we recognized via its header. + type TableKind = "legacy" | "groups" | null; + let tableKind: TableKind = null; let headerSeen = false; for (const line of lines) { const trimmed = line.trim(); - // Detect table start by header row (exact match on "| Tool |" pattern) + // Legacy quick-reference table header: | Tool | Description | if ( (trimmed.startsWith("| Tool |") || trimmed.startsWith("| tool |")) && trimmed.includes("Description") ) { - inTable = true; + tableKind = "legacy"; + headerSeen = false; + continue; + } + + // New group-index table header: | Group | Tools | Reference | + if (trimmed.startsWith("| Group |") && trimmed.includes("Tools")) { + tableKind = "groups"; headerSeen = false; continue; } // Skip separator row (|------|...) - if (inTable && !headerSeen && trimmed.startsWith("|---")) { + if (tableKind && !headerSeen && trimmed.startsWith("|---")) { headerSeen = true; continue; } // Parse data rows - if (inTable && headerSeen && trimmed.startsWith("|")) { + if (tableKind && headerSeen && trimmed.startsWith("|")) { const cells = trimmed .split("|") .map((c) => c.trim()) .filter((c) => c.length > 0); - if (cells.length >= 2) { - tools.push({ - name: cells[0]!, - description: cells[1]!, - }); + if (tableKind === "legacy" && cells.length >= 2) { + tools.push({ name: cells[0]!, description: cells[1]! }); + } else if (tableKind === "groups" && cells.length >= 2) { + // Middle column is a comma-separated list of tool names. + for (const name of cells[1]!.split(",").map((n) => n.trim())) { + if (name.length > 0) tools.push({ name, description: "" }); + } } continue; } // End of table - if (inTable && headerSeen && !trimmed.startsWith("|")) { - inTable = false; + if (tableKind && headerSeen && !trimmed.startsWith("|")) { + tableKind = null; + } + + // Flat fallback list: "- tool_name" (no table, no spaces in the name). + if (!tableKind && trimmed.startsWith("- ")) { + const name = trimmed.slice(2).trim(); + if (name.length > 0 && !name.includes(" ")) { + tools.push({ name, description: "" }); + } } } diff --git a/src/generation/templates.ts b/src/generation/templates.ts index e1aeb4a..7554f92 100644 --- a/src/generation/templates.ts +++ b/src/generation/templates.ts @@ -20,8 +20,14 @@ const MANUAL_START = ""; const MANUAL_END = ""; /** - * Generate a slim SKILL.md file with YAML frontmatter, tool table, and invoke pattern. - * Stays under 300 tokens for typical services. + * Generate a slim SKILL.md front skill: YAML frontmatter, a routing index of + * tool *groups* (each linking to its reference file), and the invoke pattern. + * + * Progressive-disclosure shape (per PAI skill-creator standard): the front skill + * carries only routing -- the per-tool detail lives in `references/*.md`. The flat + * per-tool listing is intentionally NOT inlined here; it would blow the token + * budget for large services (a 46-tool service was ~1100 tokens with the old + * inline table). Falls back to a flat tool list only when no groups are supplied. */ export function generateSkillMd(input: SkillTemplateInput): string { const lines: string[] = []; @@ -56,15 +62,30 @@ export function generateSkillMd(input: SkillTemplateInput): string { lines.push(MARKER_START); lines.push(""); - // Quick reference tool table - lines.push("## Quick Reference"); - lines.push(""); - lines.push("| Tool | Description |"); - lines.push("|------|-------------|"); - for (const tool of input.tools) { - lines.push(`| ${tool.name} | ${tool.description.replace(/\|/g, "\\|")} |`); + const groups = input.groups ?? []; + + if (groups.length > 0) { + // Routing index: one row per group, linking to its reference file. + lines.push("## Tool Groups"); + lines.push(""); + lines.push("| Group | Tools | Reference |"); + lines.push("|-------|-------|-----------|"); + for (const group of groups) { + const names = group.tools.map((t) => t.tool).join(", "); + lines.push( + `| ${group.label} | ${names.replace(/\|/g, "\\|")} | [${group.filename}](references/${group.filename}) |`, + ); + } + lines.push(""); + } else { + // Fallback (no grouping available): flat tool name list, no descriptions. + lines.push("## Tools"); + lines.push(""); + for (const tool of input.tools) { + lines.push(`- ${tool.name}`); + } + lines.push(""); } - lines.push(""); // Invoke pattern lines.push("## Usage"); @@ -75,7 +96,9 @@ export function generateSkillMd(input: SkillTemplateInput): string { lines.push(""); // References pointer - lines.push("See `references/` for detailed parameter docs per tool."); + lines.push( + "See `references/` for per-tool parameters, types, and examples.", + ); lines.push(""); lines.push(MARKER_END); diff --git a/src/generation/types.ts b/src/generation/types.ts index 8404be5..ff3eb0b 100644 --- a/src/generation/types.ts +++ b/src/generation/types.ts @@ -10,6 +10,8 @@ export interface SkillTemplateInput { description: string; tools: ToolSummary[]; triggerKeywords: string[]; + /** Tool groups used to build the slim front-skill reference index. */ + groups?: ToolGroup[]; generatedAt?: string; schemaHash?: string; toolCount?: number; diff --git a/tests/generation/diff.test.ts b/tests/generation/diff.test.ts index 38764c5..9773b57 100644 --- a/tests/generation/diff.test.ts +++ b/tests/generation/diff.test.ts @@ -67,6 +67,46 @@ describe("parseExistingTools", () => { expect(tools[0]!.name).toBe("tool_a"); expect(tools[1]!.name).toBe("tool_b"); }); + + test("extracts tool names from the slim group-index table", () => { + const content = [ + "## Tool Groups", + "", + "| Group | Tools | Reference |", + "|-------|-------|-----------|", + "| Item Operations | list_items, create_item | [item-ops.md](references/item-ops.md) |", + "| Tier Operations | set_tier, bulk_set_tier | [tier-ops.md](references/tier-ops.md) |", + "", + "## Usage", + ].join("\n"); + + const tools = parseExistingTools(content); + expect(tools.map((t) => t.name)).toEqual([ + "list_items", + "create_item", + "set_tier", + "bulk_set_tier", + ]); + }); + + test("extracts tool names from the flat fallback list", () => { + const content = [ + "## Tools", + "", + "- json_tool", + "- error_tool", + "- create_item", + "", + "## Usage", + ].join("\n"); + + const tools = parseExistingTools(content); + expect(tools.map((t) => t.name)).toEqual([ + "json_tool", + "error_tool", + "create_item", + ]); + }); }); // -- computeSkillDiff -- diff --git a/tests/generation/templates.test.ts b/tests/generation/templates.test.ts index 9760f46..7504e88 100644 --- a/tests/generation/templates.test.ts +++ b/tests/generation/templates.test.ts @@ -100,14 +100,26 @@ describe("generateSkillMd", () => { expect(md).toContain("test"); }); - test("includes quick reference tool table", () => { + test("falls back to a flat tool name list when no groups supplied", () => { const md = generateSkillMd(mockInput); - expect(md).toContain("json_tool"); - expect(md).toContain("error_tool"); - expect(md).toContain("create_item"); - // Table headers - expect(md).toContain("Tool"); - expect(md).toContain("Description"); + expect(md).toContain("## Tools"); + expect(md).toContain("- json_tool"); + expect(md).toContain("- error_tool"); + expect(md).toContain("- create_item"); + // No description column in the slim front skill. + expect(md).not.toContain("Description"); + }); + + test("emits a group index linking to reference files when groups supplied", () => { + const md = generateSkillMd({ ...mockInput, groups: [mockGroup] }); + expect(md).toContain("## Tool Groups"); + expect(md).toContain("| Group | Tools | Reference |"); + expect(md).toContain("Data Operations"); + // Group row carries tool names and a link to its reference file. + expect(md).toContain("json_tool, create_item"); + expect(md).toContain("[data-ops.md](references/data-ops.md)"); + // Detail (descriptions) stays out of the front skill. + expect(md).not.toContain("Returns a JSON object"); }); test("includes generic invoke pattern", () => { diff --git a/tests/integration/generate-skills.test.ts b/tests/integration/generate-skills.test.ts index b488201..0275bf0 100644 --- a/tests/integration/generate-skills.test.ts +++ b/tests/integration/generate-skills.test.ts @@ -96,11 +96,12 @@ describe("generate-skills integration", () => { expect(skillContent).toContain("description:"); expect(skillContent).toContain("triggers:"); - // Tool table with all 3 mock tools + // Slim group index lists all 3 mock tools and links to reference files expect(skillContent).toContain("json_tool"); expect(skillContent).toContain("error_tool"); expect(skillContent).toContain("create_item"); - expect(skillContent).toContain("| Tool | Description |"); + expect(skillContent).toContain("| Group | Tools | Reference |"); + expect(skillContent).toContain("(references/"); // Invoke pattern expect(skillContent).toContain("mcp2cli mock-server"); From 0d3ee472a1af3d2f19ad0c932a71d2b35c291d6f Mon Sep 17 00:00:00 2001 From: Rodaddy Date: Sun, 28 Jun 2026 15:57:13 -0400 Subject: [PATCH 5/5] fix(generation): scope skill diff to auto-generated block + emit groups on auto-regen Review-swarm findings on #57 (both reviewers converged on the diff bug): - parseExistingTools scanned the WHOLE SKILL.md, so YAML frontmatter `triggers:` bullets (and single-word Notes bullets) parsed as phantom tools. Round-tripping a generated grouped skill back through the diff reported the trigger keywords as "removed" and -- because the slim group index carries no per-tool descriptions -- flagged every real tool as "modified". `--diff` was useless for the exact format this PR introduces. Fix: scope the parse to the AUTO-GENERATED block (markers now exported from templates.ts), with a whole-file fallback when markers are absent; and skip the description comparison when the existing description is empty (empty == "unknown", not "changed to empty"). - Added a generate -> parse -> diff round-trip regression test (grouped and flat fallback). The existing diff tests used hand-crafted snippets without frontmatter, so the bug shipped green; the round-trip test would have caught it. - auto-regen.ts (drift-hook regeneration) computed `groups` but called generateSkillMd before that, without passing them, so the drift hook emitted the flat fallback while a manual generate-skills emitted the grouped index. Moved grouping ahead of input construction and pass `groups` so both paths produce the same slim routing index. Verified: bun run typecheck clean; full suite 1055 pass / 0 fail (deterministic across repeat runs). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/generation/auto-regen.ts | 46 +++++++++++++------- src/generation/diff.ts | 23 +++++++++- src/generation/templates.ts | 8 ++-- tests/generation/diff.test.ts | 82 +++++++++++++++++++++++++++++------ 4 files changed, 124 insertions(+), 35 deletions(-) diff --git a/src/generation/auto-regen.ts b/src/generation/auto-regen.ts index 1c233af..75ba482 100644 --- a/src/generation/auto-regen.ts +++ b/src/generation/auto-regen.ts @@ -7,9 +7,17 @@ import type { ToolSummary } from "../schema/types.ts"; import type { AccessPolicy } from "../access/types.ts"; import type { SkillTemplateInput } from "./types.ts"; import { filterTools } from "../access/filter.ts"; -import { generateSkillMd, generateReferenceMd, estimateTokens } from "./templates.ts"; +import { + generateSkillMd, + generateReferenceMd, + estimateTokens, +} from "./templates.ts"; import { detectPrefixGroups } from "./grouping.ts"; -import { resolveOutputDir, planFileWrites, executeFileWrites } from "./file-manager.ts"; +import { + resolveOutputDir, + planFileWrites, + executeFileWrites, +} from "./file-manager.ts"; import { extractManualSections, injectManualSections } from "./preserve.ts"; import { computeSchemaHash } from "./skill-hash.ts"; import { createLogger } from "../logger/index.ts"; @@ -70,23 +78,42 @@ export async function autoRegenerateSkills( const resolvedDir = outputDir ?? resolveOutputDir(serviceName); const existingSkillPath = join(resolvedDir, "SKILL.md"); const existingFile = Bun.file(existingSkillPath); - const existingContent = await existingFile.exists() ? await existingFile.text() : null; + const existingContent = (await existingFile.exists()) + ? await existingFile.text() + : null; // Reuse existing timestamp if hash hasn't changed let generatedAt = new Date().toISOString(); if (existingContent) { const existingHashMatch = existingContent.match(/^schema_hash:\s*(\S+)/m); if (existingHashMatch?.[1] === schemaHash) { - const existingAtMatch = existingContent.match(/^generated_at:\s*(\S+)/m); + const existingAtMatch = existingContent.match( + /^generated_at:\s*(\S+)/m, + ); generatedAt = existingAtMatch?.[1] ?? generatedAt; } } + // Build schemas for grouping + reference file generation. + // Use minimal SchemaOutput objects from ToolSummary data. + const schemas = filteredTools.map((t) => ({ + tool: t.name, + description: t.description, + inputSchema: {} as object, + usage: `mcp2cli ${serviceName} ${t.name}`, + })); + + const groups = detectPrefixGroups(schemas, serviceName); + const input: SkillTemplateInput = { serviceName, description: `MCP tools for ${serviceName}`, tools: filteredTools, triggerKeywords: [serviceName], + // Pass groups so auto-regen emits the same slim group-routing index as a + // manual `generate-skills` run; without it the drift hook regenerated the + // flat fallback format, diverging from the manual path. + groups, generatedAt, schemaHash, toolCount: filteredTools.length, @@ -104,17 +131,6 @@ export async function autoRegenerateSkills( } } - // Build schemas for reference file generation - // Use minimal SchemaOutput objects from ToolSummary data - const schemas = filteredTools.map((t) => ({ - tool: t.name, - description: t.description, - inputSchema: {} as object, - usage: `mcp2cli ${serviceName} ${t.name}`, - })); - - const groups = detectPrefixGroups(schemas, serviceName); - // Collect generated files const generated = new Map(); generated.set("SKILL.md", skillMd); diff --git a/src/generation/diff.ts b/src/generation/diff.ts index 2de2fd4..f3e4970 100644 --- a/src/generation/diff.ts +++ b/src/generation/diff.ts @@ -4,6 +4,7 @@ * to produce a human-readable diff showing added, removed, and modified tools. */ import type { ToolSummary } from "../schema/types.ts"; +import { MARKER_START, MARKER_END } from "./templates.ts"; /** Classification of a single tool change */ export interface ToolChange { @@ -43,7 +44,19 @@ export interface SkillDiffResult { */ export function parseExistingTools(skillContent: string): ToolSummary[] { const tools: ToolSummary[] = []; - const lines = skillContent.split("\n"); + + // Only parse the auto-generated block. Outside it live YAML frontmatter + // (whose `triggers:` bullets look like flat tool entries) and the manual + // Notes section -- scanning those produced phantom tools and made every + // `--diff` report spurious removals. If the markers are absent (e.g. a + // pre-marker hand-authored file), fall back to scanning the whole content. + const startIdx = skillContent.indexOf(MARKER_START); + const endIdx = skillContent.indexOf(MARKER_END); + const scoped = + startIdx !== -1 && endIdx !== -1 && endIdx > startIdx + ? skillContent.slice(startIdx + MARKER_START.length, endIdx) + : skillContent; + const lines = scoped.split("\n"); // Mode: parsing rows of a markdown table we recognized via its header. type TableKind = "legacy" | "groups" | null; @@ -139,7 +152,13 @@ export function computeSkillDiff( const existing = existingMap.get(name); if (!existing) { added.push({ tool: name, type: "added" }); - } else if (existing.description !== newTool.description) { + } else if ( + // The slim group-index front skill stores no per-tool descriptions, so a + // parsed empty description means "unknown", not "changed to empty". Only + // flag a real description change to avoid every tool showing as modified. + existing.description !== "" && + existing.description !== newTool.description + ) { modified.push({ tool: name, type: "modified", diff --git a/src/generation/templates.ts b/src/generation/templates.ts index 7554f92..81b9776 100644 --- a/src/generation/templates.ts +++ b/src/generation/templates.ts @@ -12,8 +12,8 @@ export function estimateTokens(text: string): number { } /** Auto-generated section markers */ -const MARKER_START = ""; -const MARKER_END = ""; +export const MARKER_START = ""; +export const MARKER_END = ""; /** Manual (user-editable) section markers */ const MANUAL_START = ""; @@ -96,9 +96,7 @@ export function generateSkillMd(input: SkillTemplateInput): string { lines.push(""); // References pointer - lines.push( - "See `references/` for per-tool parameters, types, and examples.", - ); + lines.push("See `references/` for per-tool parameters, types, and examples."); lines.push(""); lines.push(MARKER_END); diff --git a/tests/generation/diff.test.ts b/tests/generation/diff.test.ts index 9773b57..57b4e1e 100644 --- a/tests/generation/diff.test.ts +++ b/tests/generation/diff.test.ts @@ -4,6 +4,7 @@ import { computeSkillDiff, formatDiffPreview, } from "../../src/generation/diff.ts"; +import { generateSkillMd } from "../../src/generation/templates.ts"; // -- parseExistingTools -- @@ -113,9 +114,7 @@ describe("parseExistingTools", () => { describe("computeSkillDiff", () => { test("detects added tools", () => { - const existing = [ - { name: "tool_a", description: "Tool A" }, - ]; + const existing = [{ name: "tool_a", description: "Tool A" }]; const newTools = [ { name: "tool_a", description: "Tool A" }, { name: "tool_b", description: "Tool B" }, @@ -134,9 +133,7 @@ describe("computeSkillDiff", () => { { name: "tool_a", description: "Tool A" }, { name: "tool_b", description: "Tool B" }, ]; - const newTools = [ - { name: "tool_a", description: "Tool A" }, - ]; + const newTools = [{ name: "tool_a", description: "Tool A" }]; const diff = computeSkillDiff("test", existing, newTools); expect(diff.hasChanges).toBe(true); @@ -146,12 +143,8 @@ describe("computeSkillDiff", () => { }); test("detects modified tools (description changed)", () => { - const existing = [ - { name: "tool_a", description: "Old description" }, - ]; - const newTools = [ - { name: "tool_a", description: "New description" }, - ]; + const existing = [{ name: "tool_a", description: "Old description" }]; + const newTools = [{ name: "tool_a", description: "New description" }]; const diff = computeSkillDiff("test", existing, newTools); expect(diff.hasChanges).toBe(true); @@ -261,7 +254,10 @@ describe("formatDiffPreview", () => { const diff = computeSkillDiff( "my-service", [{ name: "a", description: "A" }], - [{ name: "a", description: "A" }, { name: "b", description: "B" }], + [ + { name: "a", description: "A" }, + { name: "b", description: "B" }, + ], ); const output = formatDiffPreview(diff); expect(output).toContain("Existing: 1 tools"); @@ -269,3 +265,63 @@ describe("formatDiffPreview", () => { expect(output).toContain("my-service"); }); }); + +// -- round-trip: generated SKILL.md must diff clean against its own tools -- +// Regression guard: parseExistingTools previously scanned the whole file, +// picking up YAML `triggers:` bullets as phantom tools (spurious "removed"), +// and the slim group index has no descriptions, which flagged every tool as +// "modified". Both made --diff useless for the format this generator emits. + +describe("generate -> parse -> diff round-trip", () => { + const tools = [ + { name: "ob_search", description: "Search the brain" }, + { name: "ob_write", description: "Write to the brain" }, + ]; + + test("grouped front skill diffs clean against its own tool list", () => { + const md = generateSkillMd({ + serviceName: "open-brain", + description: "Open Brain KB", + tools, + triggerKeywords: ["open-brain", "search", "knowledge"], + groups: [ + { + prefix: "ob", + label: "Ob Operations", + tools: tools.map((t) => ({ + tool: t.name, + description: t.description, + inputSchema: {}, + usage: "", + })), + filename: "ob.md", + }, + ], + }); + + const parsed = parseExistingTools(md); + expect(parsed.map((t) => t.name).sort()).toEqual(["ob_search", "ob_write"]); + // trigger keywords must NOT leak in as tools + expect(parsed.some((t) => t.name === "search")).toBe(false); + + const diff = computeSkillDiff("open-brain", parsed, tools); + expect(diff.hasChanges).toBe(false); + expect(diff.removed).toHaveLength(0); + expect(diff.added).toHaveLength(0); + expect(diff.modified).toHaveLength(0); + }); + + test("flat fallback front skill diffs clean against its own tool list", () => { + const md = generateSkillMd({ + serviceName: "svc", + description: "Flat service", + tools, + triggerKeywords: ["svc", "flat"], + }); + + const parsed = parseExistingTools(md); + expect(parsed.map((t) => t.name).sort()).toEqual(["ob_search", "ob_write"]); + const diff = computeSkillDiff("svc", parsed, tools); + expect(diff.hasChanges).toBe(false); + }); +});