feat(secrets): survive OpenBao outages and report real backend health #115

Merged
michal merged 4 commits from feat/openbao-resilience into main 2026-08-20 21:37:58 +00:00
Showing only changes of commit 545e7745da - Show all commits

View File

@@ -0,0 +1,155 @@
/**
* SecretBackendRotatorLoop had no coverage at all, despite being the boot-time
* detector added after an upstream OpenBao re-init silently broke every secret
* write for four days (e51b924). These pin the behaviours that matter when that
* recurs: the boot health check fires, it reports through the injected logger
* (so `mcpctl errors` sees it), and stop() genuinely stops.
*/
import { describe, it, expect, vi } from 'vitest';
import type { SecretBackend } from '@prisma/client';
import { SecretBackendRotatorLoop } from '../src/services/secret-backend-rotator-loop.js';
import type { SecretBackendService } from '../src/services/secret-backend.service.js';
import type { SecretBackendRotator } from '../src/services/secret-backend-rotator.service.js';
function backend(overrides: Partial<SecretBackend> = {}): SecretBackend {
return {
id: 'b1', name: 'bao', type: 'openbao',
config: { url: 'http://bao.example:8200', rotation: { enabled: true, tokenRole: 'r', intervalHours: 24 } },
isDefault: true, description: '', version: 1,
createdAt: new Date(), updatedAt: new Date(),
...overrides,
} as SecretBackend;
}
interface Harness {
loop: SecretBackendRotatorLoop;
rotator: { isRotatable: ReturnType<typeof vi.fn>; isOverdue: ReturnType<typeof vi.fn>; healthCheck: ReturnType<typeof vi.fn>; rotateOne: ReturnType<typeof vi.fn> };
logs: { info: string[]; warn: string[]; error: Array<{ obj: Record<string, unknown>; msg: string }> };
timers: Array<{ cb: () => void; ms: number }>;
cleared: number;
}
function harness(opts: {
rows?: SecretBackend[];
rotatable?: boolean;
overdue?: boolean;
health?: { ok: boolean; message?: string } | Error;
} = {}): Harness {
const rows = opts.rows ?? [backend()];
const logs: Harness['logs'] = { info: [], warn: [], error: [] };
const timers: Harness['timers'] = [];
const state = { cleared: 0 };
const rotator = {
isRotatable: vi.fn(() => opts.rotatable ?? true),
isOverdue: vi.fn(() => opts.overdue ?? false),
healthCheck: vi.fn(async () => {
if (opts.health instanceof Error) throw opts.health;
return opts.health ?? { ok: true };
}),
rotateOne: vi.fn(async () => ({})),
};
const loop = new SecretBackendRotatorLoop({
backends: {
list: async () => rows,
getById: async (id: string) => rows.find((r) => r.id === id) ?? rows[0]!,
} as unknown as SecretBackendService,
rotator: rotator as unknown as SecretBackendRotator,
setTimeout: ((cb: () => void, ms: number) => { timers.push({ cb, ms }); return { id: timers.length } as unknown as NodeJS.Timeout; }),
clearTimeout: (() => { state.cleared++; }),
log: {
info: (m) => { logs.info.push(m); },
warn: (m) => { logs.warn.push(m); },
error: (obj, msg) => { logs.error.push({ obj, msg }); },
},
});
return { loop, rotator, logs, timers, get cleared() { return state.cleared; } } as Harness;
}
/** The boot health check is fire-and-forget; let its microtasks settle. */
const settle = async (): Promise<void> => { await Promise.resolve(); await Promise.resolve(); await Promise.resolve(); };
describe('SecretBackendRotatorLoop', () => {
it('stays idle when nothing is rotatable', async () => {
const h = harness({ rotatable: false });
await h.loop.start();
expect(h.logs.info.join(' ')).toMatch(/no rotatable backends/);
expect(h.timers).toHaveLength(0);
expect(h.rotator.healthCheck).not.toHaveBeenCalled();
});
it('runs a boot-time health check for every rotatable backend', async () => {
const h = harness({ rows: [backend(), backend({ id: 'b2', name: 'bao2' })] });
await h.loop.start();
await settle();
expect(h.rotator.healthCheck).toHaveBeenCalledTimes(2);
});
it('emits BACKEND_TOKEN_DEAD through the logger, not console', async () => {
// The regression that made `mcpctl errors` blind to it: this used to be a
// bare console.error, which bypasses the pino stream feeding ErrorLogBuffer.
const h = harness({ health: { ok: false, message: 'token rejected' } });
await h.loop.start();
await settle();
expect(h.logs.error).toHaveLength(1);
expect(h.logs.error[0]?.obj).toMatchObject({ kind: 'BACKEND_TOKEN_DEAD', backend: 'bao' });
expect(h.logs.error[0]?.msg).toBe('token rejected');
});
it('does not log a fatal when the backend is healthy', async () => {
const h = harness({ health: { ok: true } });
await h.loop.start();
await settle();
expect(h.logs.error).toHaveLength(0);
});
it('survives a health check that throws', async () => {
const h = harness({ health: new Error('network down') });
await expect(h.loop.start()).resolves.toBeUndefined();
await settle();
expect(h.logs.warn.join(' ')).toMatch(/health check threw: network down/);
});
it('rotates immediately when a backend is overdue, and still schedules', async () => {
const h = harness({ overdue: true });
await h.loop.start();
await settle();
expect(h.rotator.rotateOne).toHaveBeenCalledWith('b1');
expect(h.timers).toHaveLength(1);
});
it('does not rotate on boot when not overdue', async () => {
const h = harness({ overdue: false });
await h.loop.start();
await settle();
expect(h.rotator.rotateOne).not.toHaveBeenCalled();
expect(h.timers).toHaveLength(1);
});
it('never schedules sooner than the 60s floor, even with adversarial jitter', async () => {
// intervalHours tiny + default jitter would otherwise produce a negative delay.
const rows = [backend({ config: { url: 'u', rotation: { enabled: true, tokenRole: 'r', intervalHours: 0.0001 } } } as Partial<SecretBackend>)];
for (let i = 0; i < 50; i++) {
const h = harness({ rows });
await h.loop.start();
expect(h.timers[0]?.ms).toBeGreaterThanOrEqual(60_000);
}
});
it('stop() clears timers and suppresses further scheduling', async () => {
const h = harness();
await h.loop.start();
expect(h.timers).toHaveLength(1);
h.loop.stop();
expect(h.cleared).toBeGreaterThan(0);
// The `stopped` guard has never been exercised: a firing timer must not
// reschedule after stop().
const before = h.timers.length;
await h.loop.rotateNow('b1').catch(() => undefined);
expect(h.timers).toHaveLength(before);
});
});