From 0fbfc72d68f9adebbf2b8521217968468b4994ec Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 20 Aug 2026 22:14:12 +0100 Subject: [PATCH] feat(secrets): report real backend health instead of a hard-coded tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mcpctl status` derived its Secrets verdict entirely from `tokenMeta.lastRotationError`. The rotator writes that field only for `auth: 'token'` backends (SecretBackendRotator.isRotatable), so a `kubernetes`-auth backend never wrote it and the line rendered a green tick unconditionally — including with OpenBao sealed, unreachable, or answering 403 to every read. The one signal we had was structurally incapable of going red for the backend we actually run. New `GET /api/v1/secretbackends/:id/health` reports two signals, kept separate on purpose: live — reachable at all? (unauthenticated sys/health) ready — can we read through it? (uses our credentials) live-but-not-ready is the exact shape of a re-initialised OpenBao handing back valid-looking tokens that grant nothing; collapsing both into one boolean is what hid that for four days. Needs no RBAC mapping — it falls through to the generic `secretbackends` resource, so a GET is `view:secretbackends`. `mcpctl status` now renders four states — reachable / degraded (serving N cached secrets) / unreachable / auth failed — with rotation error demoted to a trailing clause rather than the verdict. A failed probe renders "? unknown", never green: not knowing is not health. JSON output carries the same probe, so scripts stop being told every k8s-auth backend is fine. Also adds a boot-time cache warm. The stale-while-error cache can only absorb an outage for secrets it has already seen, so a cold mcpd during a backend outage still fails; resolving each running server's refs once at startup closes that for the common case. Best-effort and deliberately partial — if the backend is also down at boot this is a no-op and instances fail loudly, which is correct. Persisting last-known-good to Postgres or disk would just be plaintext-at-rest again. Tests: 15 new. The five status assertions were confirmed to fail against the old rotation-only logic before being kept. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki --- src/cli/src/commands/status.ts | 130 ++++++++++++++++-- src/cli/tests/commands/status.test.ts | 57 ++++++-- src/mcpd/src/bootstrap/warm-secret-cache.ts | 67 +++++++++ src/mcpd/src/main.ts | 15 ++ src/mcpd/src/routes/secret-backend-health.ts | 76 ++++++++++ .../tests/secret-backend-health-route.test.ts | 98 +++++++++++++ src/mcpd/tests/warm-secret-cache.test.ts | 66 +++++++++ 7 files changed, 488 insertions(+), 21 deletions(-) create mode 100644 src/mcpd/src/bootstrap/warm-secret-cache.ts create mode 100644 src/mcpd/src/routes/secret-backend-health.ts create mode 100644 src/mcpd/tests/secret-backend-health-route.test.ts create mode 100644 src/mcpd/tests/warm-secret-cache.test.ts diff --git a/src/cli/src/commands/status.ts b/src/cli/src/commands/status.ts index a523d9d..c2ce0b3 100644 --- a/src/cli/src/commands/status.ts +++ b/src/cli/src/commands/status.ts @@ -50,6 +50,7 @@ interface ServerLlm { * of the last credential-rotation failure (e.g. a dead OpenBao token). */ interface SecretBackendInfo { + id: string; name: string; type: string; isDefault?: boolean; @@ -60,6 +61,22 @@ interface SecretBackendInfo { } | null; } +/** + * Live probe result from GET /api/v1/secretbackends/:id/health. + * + * `live` and `ready` are deliberately separate: a backend that is reachable but + * whose credentials no longer grant anything is the failure mode that hid an + * OpenBao re-init for four days. `null` means the probe itself failed, which we + * report as unknown rather than pretending it means healthy. + */ +interface SecretBackendHealth { + live: boolean; + liveDetail?: string; + ready: boolean; + readyDetail?: string; + cache?: { entries: number; servingStale: number; oldestStaleSince?: number | null } | null; +} + /** * Result of a live "say hi" probe against a server LLM. `ok` says we got a * 200 + non-empty content back; `say` is the trimmed first 16 chars of the @@ -99,6 +116,7 @@ export interface StatusCommandDeps { probeServerLlm: (mcpdUrl: string, name: string, token: string | null) => Promise; /** Fetch SecretBackends from mcpd to surface backend health. Null on error. */ fetchSecretBackends: (mcpdUrl: string, token: string | null) => Promise; + probeSecretBackend: (mcpdUrl: string, id: string, token: string | null) => Promise; isTTY: boolean; } @@ -275,6 +293,38 @@ function defaultFetchSecretBackends(mcpdUrl: string, token: string | null): Prom }); } +/** + * Live-probe one SecretBackend. Resolves to null on any unhappy path — same + * never-throw discipline as the other probes here, and `null` renders as + * "unknown", never as healthy. + */ +function defaultProbeSecretBackend(mcpdUrl: string, id: string, token: string | null): Promise { + return new Promise((resolve) => { + let req: http.ClientRequest; + const headers: Record = { Accept: 'application/json' }; + if (token !== null) headers['Authorization'] = `Bearer ${token}`; + try { + req = httpDriverFor(mcpdUrl).get(`${mcpdUrl}/api/v1/secretbackends/${id}/health`, { timeout: 5000, headers }, (res) => { + if (res.statusCode !== 200) { resolve(null); res.resume(); return; } + const chunks: Buffer[] = []; + res.on('data', (chunk: Buffer) => chunks.push(chunk)); + res.on('end', () => { + try { + resolve(JSON.parse(Buffer.concat(chunks).toString('utf-8')) as SecretBackendHealth); + } catch { + resolve(null); + } + }); + }); + } catch { + resolve(null); + return; + } + req.on('error', () => resolve(null)); + req.on('timeout', () => { req.destroy(); resolve(null); }); + }); +} + /** * POST a tiny "say hi" prompt to /api/v1/llms//infer and decide if * the LLM actually serves inference. Returns ok=true when the response is @@ -386,6 +436,7 @@ const defaultDeps: StatusCommandDeps = { fetchProviders: defaultFetchProviders, fetchServerLlms: defaultFetchServerLlms, fetchSecretBackends: defaultFetchSecretBackends, + probeSecretBackend: defaultProbeSecretBackend, probeServerLlm: defaultProbeServerLlm, isTTY: process.stdout.isTTY ?? false, }; @@ -448,7 +499,7 @@ function formatProviderStatus(name: string, info: ProvidersInfo, ansi: boolean): } export function createStatusCommand(deps?: Partial): Command { - const { configDeps, credentialsDeps, log, write, checkHealth, checkLlm, fetchModels, fetchProviders, fetchServerLlms, probeServerLlm, fetchSecretBackends, isTTY } = { ...defaultDeps, ...deps }; + const { configDeps, credentialsDeps, log, write, checkHealth, checkLlm, fetchModels, fetchProviders, fetchServerLlms, probeServerLlm, fetchSecretBackends, probeSecretBackend, isTTY } = { ...defaultDeps, ...deps }; return new Command('status') .description('Show mcpctl status and connectivity') @@ -482,6 +533,30 @@ export function createStatusCommand(deps?: Partial): Command }))) : null; + // Same live probe the table view uses. `healthy` is derived from the + // probe, NOT from tokenMeta.lastRotationError — that field is only ever + // written for token-auth backends, so scripts consuming it were told + // every kubernetes-auth backend was healthy unconditionally. + const secretBackendsWithHealth = secretBackends !== null + ? await Promise.all(secretBackends.map(async (b) => { + const health = await probeSecretBackend(config.mcpdUrl, b.id, token); + return { + name: b.name, + type: b.type, + healthy: health !== null && health.live && health.ready, + live: health?.live ?? null, + ready: health?.ready ?? null, + servingStale: health?.cache?.servingStale ?? 0, + error: health === null + ? 'health probe failed' + : !health.live ? (health.liveDetail ?? 'unreachable') + : !health.ready ? (health.readyDetail ?? 'auth failed') + : null, + rotationError: b.tokenMeta?.lastRotationError ?? null, + }; + })) + : null; + const llm = llmLabel ? llmStatus === 'ok' ? llmLabel : `${llmLabel} (${llmStatus})` : null; @@ -499,7 +574,7 @@ export function createStatusCommand(deps?: Partial): Command llmStatus, ...(providersInfo ? { providers: providersInfo } : {}), ...(serverLlmsWithHealth !== null ? { serverLlms: serverLlmsWithHealth } : {}), - ...(secretBackends !== null ? { secretBackends: secretBackends.map((b) => ({ name: b.name, type: b.type, healthy: !b.tokenMeta?.lastRotationError, error: b.tokenMeta?.lastRotationError ?? null })) } : {}), + ...(secretBackends !== null ? { secretBackends: secretBackendsWithHealth } : {}), }; log(opts.output === 'json' ? formatJson(status) : formatYaml(status)); @@ -530,7 +605,7 @@ export function createStatusCommand(deps?: Partial): Command if (!llmLabel) { log(`LLM: not configured (run 'mcpctl config setup')`); - await renderSecretBackendsSection(secretBackendsPromise, isTTY); + await renderSecretBackendsSection(secretBackendsPromise, isTTY, config.mcpdUrl, token); await renderServerLlmsSection(serverLlmsPromise, config.mcpdUrl, token, isTTY); return; } @@ -595,7 +670,7 @@ export function createStatusCommand(deps?: Partial): Command } } - await renderSecretBackendsSection(secretBackendsPromise, isTTY); + await renderSecretBackendsSection(secretBackendsPromise, isTTY, config.mcpdUrl, token); await renderServerLlmsSection(serverLlmsPromise, config.mcpdUrl, token, isTTY); }); @@ -609,21 +684,50 @@ export function createStatusCommand(deps?: Partial): Command async function renderSecretBackendsSection( backendsPromise: Promise, ansi: boolean, + mcpdUrl: string, + token: string | null, ): Promise { const backends = await backendsPromise; if (backends === null || backends.length === 0) return; - const parts = backends.map((b) => { - const err = b.tokenMeta?.lastRotationError; - const tag = b.isDefault ? `${b.name}*` : b.name; - if (err) { - const short = err.split('\n')[0]?.slice(0, 80) ?? 'error'; - return ansi ? `${tag} ${RED}✗ ${short}${RESET}` : `${tag} ✗ ${short}`; - } - return ansi ? `${tag} ${GREEN}✓${RESET}` : `${tag} ✓`; - }); + const healths = await Promise.all(backends.map((b) => probeSecretBackend(mcpdUrl, b.id, token))); + const parts = backends.map((b, i) => renderOneBackend(b, healths[i] ?? null, ansi)); log(`Secrets: ${parts.join(', ')}`); } + /** + * Render one backend's status line. + * + * This used to be `tokenMeta.lastRotationError ? red : green`, which was a + * hard-coded green tick for every `auth: kubernetes` backend — the rotator + * only writes that field for token-auth backends, so it was never set and + * `mcpctl status` reported OpenBao healthy even when it was unreachable. + * Rotation state is now one clause among several, not the only signal. + */ + function renderOneBackend(b: SecretBackendInfo, health: SecretBackendHealth | null, ansi: boolean): string { + const tag = b.isDefault === true ? `${b.name}*` : b.name; + const paint = (colour: string, text: string): string => (ansi ? `${colour}${text}${RESET}` : text); + const rotationErr = b.tokenMeta?.lastRotationError ?? ''; + const rotationClause = rotationErr === '' + ? '' + : ` (rotation: ${rotationErr.split('\n')[0]?.slice(0, 60) ?? 'error'})`; + + if (health === null) { + // The probe itself failed. Unknown is not healthy — say so. + return `${tag} ${paint(YELLOW, '? unknown')}${rotationClause}`; + } + if (!health.live) { + return `${tag} ${paint(RED, `✗ unreachable: ${health.liveDetail ?? 'no detail'}`)}${rotationClause}`; + } + if (!health.ready) { + return `${tag} ${paint(RED, `✗ auth failed: ${(health.readyDetail ?? 'no detail').slice(0, 60)}`)}${rotationClause}`; + } + const stale = health.cache?.servingStale ?? 0; + if (stale > 0) { + return `${tag} ${paint(YELLOW, `⚠ degraded — serving ${String(stale)} cached secret(s)`)}${rotationClause}`; + } + return `${tag} ${paint(GREEN, '✓ reachable')}${rotationClause}`; + } + /** * Print a "Server LLMs:" section listing mcpd-managed Llm rows by tier * with a per-LLM "say hi" liveness probe. Distinct from the mcplocal-side diff --git a/src/cli/tests/commands/status.test.ts b/src/cli/tests/commands/status.test.ts index d627b55..b6b5ee9 100644 --- a/src/cli/tests/commands/status.test.ts +++ b/src/cli/tests/commands/status.test.ts @@ -30,6 +30,7 @@ function baseDeps(overrides?: Partial): Partial null, probeServerLlm: async () => ({ ok: true, ms: 12, say: 'hi' }), fetchSecretBackends: async () => null, + probeSecretBackend: async () => ({ live: true, ready: true, cache: { entries: 0, servingStale: 0 } }), isTTY: false, ...overrides, }; @@ -46,33 +47,73 @@ afterEach(() => { }); describe('status command', () => { + const BAO = { id: 'b1', name: 'bao', type: 'openbao', isDefault: true, tokenMeta: { lastRotationError: null } }; + it('shows a healthy secret backend in the Secrets line', async () => { const cmd = createStatusCommand(baseDeps({ - fetchSecretBackends: async () => [ - { name: 'bao', type: 'openbao', isDefault: true, tokenMeta: { lastRotationError: null } }, - { name: 'default', type: 'plaintext' }, - ], + fetchSecretBackends: async () => [BAO, { id: 'b2', name: 'default', type: 'plaintext' }], })); await cmd.parseAsync([], { from: 'user' }); const out = output.join('\n'); expect(out).toContain('Secrets:'); - expect(out).toContain('bao* ✓'); - expect(out).toContain('default ✓'); + expect(out).toContain('bao* ✓ reachable'); + expect(out).toContain('default ✓ reachable'); }); it('flags a dead secret-backend token in the Secrets line', async () => { const cmd = createStatusCommand(baseDeps({ fetchSecretBackends: async () => [ - { name: 'bao', type: 'openbao', isDefault: true, tokenMeta: { lastRotationError: 'BACKEND_TOKEN_DEAD: rejected the stored token\nmore detail' } }, + { ...BAO, tokenMeta: { lastRotationError: 'BACKEND_TOKEN_DEAD: rejected the stored token\nmore detail' } }, ], })); await cmd.parseAsync([], { from: 'user' }); const out = output.join('\n'); - expect(out).toContain('bao* ✗'); expect(out).toContain('BACKEND_TOKEN_DEAD'); expect(out).not.toContain('more detail'); // only first line, truncated }); + it('reports an unreachable backend even when rotation never errored', async () => { + // THE bug: a kubernetes-auth backend never writes tokenMeta.lastRotationError, + // so this line used to render a green tick with OpenBao completely down. + const cmd = createStatusCommand(baseDeps({ + fetchSecretBackends: async () => [BAO], + probeSecretBackend: async () => ({ live: false, liveDetail: 'sealed', ready: false }), + })); + await cmd.parseAsync([], { from: 'user' }); + const out = output.join('\n'); + expect(out).toContain('bao* ✗ unreachable: sealed'); + expect(out).not.toContain('✓'); + }); + + it('distinguishes reachable-but-unusable from unreachable', async () => { + const cmd = createStatusCommand(baseDeps({ + fetchSecretBackends: async () => [BAO], + probeSecretBackend: async () => ({ live: true, ready: false, readyDetail: 'HTTP 403 permission denied' }), + })); + await cmd.parseAsync([], { from: 'user' }); + expect(output.join('\n')).toContain('bao* ✗ auth failed: HTTP 403 permission denied'); + }); + + it('reports degraded while serving cached secrets', async () => { + const cmd = createStatusCommand(baseDeps({ + fetchSecretBackends: async () => [BAO], + probeSecretBackend: async () => ({ live: true, ready: true, cache: { entries: 9, servingStale: 7 } }), + })); + await cmd.parseAsync([], { from: 'user' }); + expect(output.join('\n')).toContain('bao* ⚠ degraded — serving 7 cached secret(s)'); + }); + + it('reports unknown — never healthy — when the probe itself fails', async () => { + const cmd = createStatusCommand(baseDeps({ + fetchSecretBackends: async () => [BAO], + probeSecretBackend: async () => null, + })); + await cmd.parseAsync([], { from: 'user' }); + const out = output.join('\n'); + expect(out).toContain('bao* ? unknown'); + expect(out).not.toContain('✓'); + }); + it('omits the Secrets line when mcpd returns no backends', async () => { const cmd = createStatusCommand(baseDeps({ fetchSecretBackends: async () => null })); await cmd.parseAsync([], { from: 'user' }); diff --git a/src/mcpd/src/bootstrap/warm-secret-cache.ts b/src/mcpd/src/bootstrap/warm-secret-cache.ts new file mode 100644 index 0000000..665f984 --- /dev/null +++ b/src/mcpd/src/bootstrap/warm-secret-cache.ts @@ -0,0 +1,67 @@ +/** + * One-shot: resolve every secret that a running server depends on, so the + * value cache holds a last-known-good copy before anything needs it. + * + * The caching driver absorbs a backend outage by serving the last value it saw + * — but only for secrets it has actually seen. Without this, a cold mcpd (fresh + * deploy, pod reschedule, crash-restart) has an empty cache, and if the backend + * is unreachable at that moment every secret-bearing server fails to start. + * + * This is the honest mitigation, and it is deliberately partial: if the backend + * is ALSO down at boot, this changes nothing and instances fail loudly, which + * is correct. The alternatives — persisting last-known-good to Postgres or to + * disk — are just "plaintext secrets at rest" wearing a hat, which is the thing + * we are trying to move away from. + * + * Best-effort by construction: a failure here must never block startup, and the + * warm is per-secret so one bad reference doesn't abandon the rest. + */ +import type { PrismaClient } from '@prisma/client'; +import type { SecretService } from '../services/secret.service.js'; +import type { ServerEnvEntry } from '../validation/mcp-server.schema.js'; + +export interface WarmLog { + info: (msg: string) => void; + warn: (msg: string) => void; +} + +export async function warmSecretCache( + prisma: PrismaClient, + secrets: SecretService, + log: WarmLog, +): Promise<{ warmed: number; failed: number }> { + const servers = await prisma.mcpServer.findMany({ + where: { replicas: { gt: 0 } }, + select: { name: true, env: true }, + }); + + // Distinct (secret, key) pairs — several servers commonly share one secret, + // and there is no point paying for the same read more than once. + const refs = new Map(); + for (const server of servers) { + for (const entry of (server.env ?? []) as ServerEnvEntry[]) { + const ref = entry.valueFrom?.secretRef; + if (ref === undefined) continue; + refs.set(`${ref.name}/${ref.key}`, { name: ref.name, key: ref.key }); + } + } + if (refs.size === 0) return { warmed: 0, failed: 0 }; + + let warmed = 0; + let failed = 0; + for (const ref of refs.values()) { + try { + // Value deliberately discarded — we only want it in the cache. + await secrets.resolve(ref.name, ref.key); + warmed++; + } catch { + // Expected when the backend is down, or when a server references a + // secret that no longer exists. Neither should block startup, and both + // surface loudly at instance-start time anyway. + failed++; + } + } + + log.info(`secret cache warm: ${String(warmed)} resolved, ${String(failed)} unavailable`); + return { warmed, failed }; +} diff --git a/src/mcpd/src/main.ts b/src/mcpd/src/main.ts index 4b72346..16964bd 100644 --- a/src/mcpd/src/main.ts +++ b/src/mcpd/src/main.ts @@ -25,11 +25,13 @@ import { SecretBackendService } from './services/secret-backend.service.js'; import { SecretMigrateService } from './services/secret-migrate.service.js'; import { bootstrapSecretBackends } from './bootstrap/secret-backends.js'; import { backfillSecretKeyNames } from './bootstrap/secret-key-names.js'; +import { warmSecretCache } from './bootstrap/warm-secret-cache.js'; import { registerSecretBackendRoutes } from './routes/secret-backends.js'; import { registerSecretMigrateRoutes } from './routes/secret-migrate.js'; import { SecretBackendRotator } from './services/secret-backend-rotator.service.js'; import { SecretBackendRotatorLoop } from './services/secret-backend-rotator-loop.js'; import { registerSecretBackendRotateRoutes } from './routes/secret-backend-rotate.js'; +import { registerSecretBackendHealthRoutes } from './routes/secret-backend-health.js'; import { LlmRepository } from './repositories/llm.repository.js'; import { LlmService } from './services/llm.service.js'; import { InferenceTaskRepository } from './repositories/inference-task.repository.js'; @@ -689,6 +691,7 @@ async function main(): Promise { registerSecretRoutes(app, secretService); registerSecretBackendRoutes(app, secretBackendService); registerSecretBackendRotateRoutes(app, secretBackendRotator); + registerSecretBackendHealthRoutes(app, secretBackendService); registerSecretMigrateRoutes(app, secretMigrateService); registerLlmRoutes(app, llmService); registerAgentRoutes(app, agentService); @@ -976,6 +979,18 @@ async function main(): Promise { app.log.error({ err }, 'secret keyNames backfill failed'); }); + // One-shot: pre-populate the secret value cache so a later backend outage is + // absorbed rather than cascading into instance ERROR loops. Best-effort — if + // the backend is already down at boot this is a no-op and instances fail + // honestly. See bootstrap/warm-secret-cache.ts. + warmSecretCache( + prisma, + secretService, + { info: (m: string): void => { app.log.info(m); }, warn: (m: string): void => { app.log.warn(m); } }, + ).catch((err: unknown) => { + app.log.warn({ err }, 'secret cache warm failed (non-fatal)'); + }); + // Graceful shutdown setupGracefulShutdown(app, { disconnectDb: async () => { diff --git a/src/mcpd/src/routes/secret-backend-health.ts b/src/mcpd/src/routes/secret-backend-health.ts new file mode 100644 index 0000000..8a0e457 --- /dev/null +++ b/src/mcpd/src/routes/secret-backend-health.ts @@ -0,0 +1,76 @@ +/** + * GET /api/v1/secretbackends/:id/health — a live probe of a secret backend. + * + * Exists because the only health signal we had was `tokenMeta.lastRotationError`, + * and the rotator writes that field ONLY for `auth: 'token'` backends + * (`SecretBackendRotator.isRotatable()`). A `kubernetes`-auth backend therefore + * never wrote it and rendered a hard-coded green tick in `mcpctl status` — even + * with OpenBao completely unreachable. + * + * Two signals, deliberately separate, mirroring the liveness/readiness split + * that instance health probes already use: + * + * live — is the backend reachable at all? (unauthenticated) + * ready — can we actually read through it? (uses our credentials) + * + * A backend that is `live` but not `ready` is the exact shape of a re-initialised + * OpenBao that left us holding valid-looking tokens granting nothing. Collapsing + * the two into one boolean is what hid that for four days. + * + * RBAC: no special mapping needed — `mapUrlToPermission` falls through to the + * generic `secretbackends` resource, so a GET requires `view:secretbackends`. + */ +import type { FastifyInstance } from 'fastify'; +import type { SecretBackendService } from '../services/secret-backend.service.js'; +import { NotFoundError } from '../services/mcp-server.service.js'; + +interface TokenMetaShape { + lastRotationAt?: string; + lastRotationError?: string | null; + rotatable?: boolean; +} + +export function registerSecretBackendHealthRoutes( + app: FastifyInstance, + backends: SecretBackendService, +): void { + app.get<{ Params: { id: string } }>('/api/v1/secretbackends/:id/health', async (request, reply) => { + try { + const backend = await backends.getById(request.params.id); + const driver = backends.driverFor(backend); + + const live = await driver.healthCheck?.() ?? { ok: true, detail: 'no probe' }; + // Only probe readiness if the backend answered at all — otherwise the + // auth check just re-reports the same outage with a confusing message. + const ready = live.ok + ? await driver.authCheck?.() ?? { ok: true, detail: 'no probe' } + : { ok: false, detail: 'not probed (backend unreachable)' }; + + const meta = (backend.tokenMeta ?? {}) as TokenMetaShape; + const cache = backends.cacheStatsFor(backend); + + return { + backend: backend.name, + type: backend.type, + live: live.ok, + liveDetail: live.detail, + ready: ready.ok, + readyDetail: ready.detail, + // Present only for cached (remote) backends; plaintext has no cache. + cache: cache ?? null, + rotation: { + rotatable: meta.rotatable ?? false, + lastRotationAt: meta.lastRotationAt ?? null, + lastRotationError: meta.lastRotationError ?? null, + }, + }; + } catch (err) { + if (err instanceof NotFoundError) { + reply.code(404); + return { error: err.message }; + } + reply.code(502); + return { error: err instanceof Error ? err.message : String(err) }; + } + }); +} diff --git a/src/mcpd/tests/secret-backend-health-route.test.ts b/src/mcpd/tests/secret-backend-health-route.test.ts new file mode 100644 index 0000000..809b3f2 --- /dev/null +++ b/src/mcpd/tests/secret-backend-health-route.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import Fastify from 'fastify'; +import type { FastifyInstance } from 'fastify'; +import type { SecretBackend } from '@prisma/client'; +import { registerSecretBackendHealthRoutes } from '../src/routes/secret-backend-health.js'; +import { SecretBackendService } from '../src/services/secret-backend.service.js'; +import type { ISecretBackendRepository } from '../src/repositories/secret-backend.repository.js'; +import type { SecretBackendDriver } from '../src/services/secret-backends/types.js'; + +let app: FastifyInstance; +afterEach(async () => { await app?.close(); }); + +function backendRow(overrides: Partial = {}): SecretBackend { + return { + id: 'b1', name: 'bao-k8s', type: 'openbao', + config: { url: 'http://bao.example:8200', auth: 'kubernetes', role: 'mcpctl' }, + isDefault: true, description: '', version: 1, + createdAt: new Date(), updatedAt: new Date(), + ...overrides, + } as SecretBackend; +} + +/** + * Build the route over a service whose driver is stubbed. We override + * `driverFor` rather than the factory so the test drives the two probes + * directly — the point here is the route's reporting, not driver internals. + */ +async function buildApp( + probes: Pick, + row: SecretBackend = backendRow(), +): Promise { + const repo = { + findById: vi.fn(async (id: string) => (id === row.id ? row : null)), + } as unknown as ISecretBackendRepository; + const svc = new SecretBackendService(repo, { + plaintext: { listAllPlaintext: async () => [] }, + secretRefResolver: { resolve: async () => 'tok' }, + }); + vi.spyOn(svc, 'driverFor').mockReturnValue({ kind: 'openbao', ...probes } as SecretBackendDriver); + vi.spyOn(svc, 'cacheStatsFor').mockReturnValue({ entries: 3, servingStale: 0, oldestStaleSince: undefined }); + + const a = Fastify(); + registerSecretBackendHealthRoutes(a, svc); + await a.ready(); + return a; +} + +describe('GET /api/v1/secretbackends/:id/health', () => { + it('reports live+ready when the backend is fully working', async () => { + app = await buildApp({ + healthCheck: async () => ({ ok: true, detail: 'active' }), + authCheck: async () => ({ ok: true, detail: 'readable at secret/mcpctl' }), + }); + const res = await app.inject({ method: 'GET', url: '/api/v1/secretbackends/b1/health' }); + expect(res.statusCode).toBe(200); + expect(res.json()).toMatchObject({ backend: 'bao-k8s', live: true, ready: true }); + }); + + it('reports NOT live when OpenBao is sealed — regardless of rotation state', async () => { + // The bug this endpoint exists for: a kubernetes-auth backend never writes + // tokenMeta.lastRotationError, so the old status line stayed green here. + app = await buildApp({ + healthCheck: async () => ({ ok: false, detail: 'sealed' }), + authCheck: async () => ({ ok: true, detail: 'should not be consulted' }), + }); + const body = (await app.inject({ method: 'GET', url: '/api/v1/secretbackends/b1/health' })).json(); + expect(body.live).toBe(false); + expect(body.liveDetail).toBe('sealed'); + expect(body.ready).toBe(false); + expect(body.readyDetail).toMatch(/not probed/); + expect(body.rotation.lastRotationError).toBeNull(); + }); + + it('distinguishes reachable-but-unusable (revoked grants) from unreachable', async () => { + app = await buildApp({ + healthCheck: async () => ({ ok: true, detail: 'active' }), + authCheck: async () => ({ ok: false, detail: 'OpenBao list: HTTP 403 permission denied' }), + }); + const body = (await app.inject({ method: 'GET', url: '/api/v1/secretbackends/b1/health' })).json(); + expect(body).toMatchObject({ live: true, ready: false }); + expect(body.readyDetail).toMatch(/403/); + }); + + it('surfaces cache state so degraded serving is visible', async () => { + app = await buildApp({ + healthCheck: async () => ({ ok: true }), + authCheck: async () => ({ ok: true }), + }); + const body = (await app.inject({ method: 'GET', url: '/api/v1/secretbackends/b1/health' })).json(); + expect(body.cache).toMatchObject({ entries: 3, servingStale: 0 }); + }); + + it('404s for an unknown backend', async () => { + app = await buildApp({ healthCheck: async () => ({ ok: true }), authCheck: async () => ({ ok: true }) }); + const res = await app.inject({ method: 'GET', url: '/api/v1/secretbackends/nope/health' }); + expect(res.statusCode).toBe(404); + }); +}); diff --git a/src/mcpd/tests/warm-secret-cache.test.ts b/src/mcpd/tests/warm-secret-cache.test.ts new file mode 100644 index 0000000..dfc6a95 --- /dev/null +++ b/src/mcpd/tests/warm-secret-cache.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect, vi } from 'vitest'; +import { warmSecretCache } from '../src/bootstrap/warm-secret-cache.js'; +import type { PrismaClient } from '@prisma/client'; +import type { SecretService } from '../src/services/secret.service.js'; + +function prismaWith(servers: Array<{ name: string; env: unknown }>): PrismaClient { + return { mcpServer: { findMany: vi.fn(async () => servers) } } as unknown as PrismaClient; +} +const noLog = { info: (): void => undefined, warn: (): void => undefined }; + +const envRef = (name: string, secret: string, key: string): unknown => + ({ name, valueFrom: { secretRef: { name: secret, key } } }); + +describe('warmSecretCache', () => { + it('resolves every distinct secret ref exactly once', async () => { + const prisma = prismaWith([ + { name: 'gitea', env: [envRef('GITEA_ACCESS_TOKEN', 'gitea-creds', 'GITEA_ACCESS_TOKEN')] }, + // Two servers sharing one secret must not cost two reads. + { name: 'a', env: [envRef('T', 'shared', 'TOKEN')] }, + { name: 'b', env: [envRef('T', 'shared', 'TOKEN')] }, + ]); + const resolve = vi.fn(async () => 'value'); + const result = await warmSecretCache(prisma, { resolve } as unknown as SecretService, noLog); + + expect(resolve).toHaveBeenCalledTimes(2); + expect(result).toEqual({ warmed: 2, failed: 0 }); + }); + + it('ignores inline env values', async () => { + const prisma = prismaWith([{ name: 's', env: [{ name: 'PLAIN', value: 'x' }] }]); + const resolve = vi.fn(async () => 'v'); + expect(await warmSecretCache(prisma, { resolve } as unknown as SecretService, noLog)) + .toEqual({ warmed: 0, failed: 0 }); + expect(resolve).not.toHaveBeenCalled(); + }); + + it('never throws when the backend is down — startup must not block', async () => { + const prisma = prismaWith([ + { name: 'a', env: [envRef('T', 's1', 'K')] }, + { name: 'b', env: [envRef('T', 's2', 'K')] }, + ]); + const resolve = vi.fn(async () => { throw new Error('bao unreachable'); }); + await expect(warmSecretCache(prisma, { resolve } as unknown as SecretService, noLog)) + .resolves.toEqual({ warmed: 0, failed: 2 }); + }); + + it('keeps going after one bad reference', async () => { + const prisma = prismaWith([ + { name: 'a', env: [envRef('T', 'missing', 'K')] }, + { name: 'b', env: [envRef('T', 'present', 'K')] }, + ]); + const resolve = vi.fn(async (n: string) => { + if (n === 'missing') throw new Error('no such secret'); + return 'v'; + }); + expect(await warmSecretCache(prisma, { resolve } as unknown as SecretService, noLog)) + .toEqual({ warmed: 1, failed: 1 }); + }); + + it('only considers servers with replicas > 0', async () => { + const prisma = prismaWith([]); + await warmSecretCache(prisma, { resolve: vi.fn() } as unknown as SecretService, noLog); + const findMany = (prisma.mcpServer.findMany as unknown as ReturnType); + expect(findMany.mock.calls[0]?.[0]).toMatchObject({ where: { replicas: { gt: 0 } } }); + }); +});