Merge pull request 'feat(secrets): survive OpenBao outages and report real backend health' (#115) from feat/openbao-resilience into main

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<ServerLlmHealth>;
   /** Fetch SecretBackends from mcpd to surface backend health. Null on error. */
   fetchSecretBackends: (mcpdUrl: string, token: string | null) => Promise<SecretBackendInfo[] | null>;
+  probeSecretBackend: (mcpdUrl: string, id: string, token: string | null) => Promise<SecretBackendHealth | null>;
   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<SecretBackendHealth | null> {
+  return new Promise((resolve) => {
+    let req: http.ClientRequest;
+    const headers: Record<string, string> = { 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/<name>/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<StatusCommandDeps>): 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<StatusCommandDeps>): 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<StatusCommandDeps>): 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<StatusCommandDeps>): 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<StatusCommandDeps>): 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<StatusCommandDeps>): Command
   async function renderSecretBackendsSection(
     backendsPromise: Promise<SecretBackendInfo[] | null>,
     ansi: boolean,
+    mcpdUrl: string,
+    token: string | null,
   ): Promise<void> {
     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

src/cli/tests/commands/status.test.ts

@@ -30,6 +30,7 @@ function baseDeps(overrides?: Partial<StatusCommandDeps>): Partial<StatusCommand
     fetchServerLlms: async () => 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' });