SecretBackendRotatorLoop had zero tests, despite being the detector added
in e51b924 specifically so a re-initialised OpenBao surfaces the moment
mcpd boots rather than 24h later when the scheduled rotation finally
fires. The class already injects setTimeout/clearTimeout and a logger, so
this needed no production change.
Nine cases, weighted to what actually breaks: the boot health check runs
per rotatable backend; a dead token emits kind BACKEND_TOKEN_DEAD through
the injected logger (the point of the earlier console.error removal — a
bare console call never reaches ErrorLogBuffer, so `mcpctl errors` could
not see it); a throwing health check does not abort start(); overdue
backends rotate immediately and still get scheduled; the 60s floor holds
across 50 adversarial-jitter draws; and stop() both clears timers and
trips the `stopped` guard against rescheduling, which had never been
exercised.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
Two of these four assertions currently FAIL against the live cluster, which
is the point: mcpd there still renders `Secrets: bao-k8s* ✓` from the
rotation field, and its JSON status carries no live/ready pair. Per the
project rule the fix is to deploy, not to relax the assertion.
The status check specifically rejects a bare "name ✓" with no qualifier —
that string is the old rendering and proves the probe was never consulted.
Deliberately does not take the real OpenBao down. Simulating an outage
against shared infrastructure to satisfy a test would be worse than the
bug it covers; the outage paths are unit-tested with an injected clock.
Docs: adds a Reliability section (request hardening, the stale-while-error
table, the cold-cache gap and why persisting values is not the answer,
live-vs-ready) and replaces the stale "Kubernetes ServiceAccount auth is
not shipped yet" note — it shipped in 5152066, and it is what the live
backend has used since June.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
`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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
mcpd re-read the secret backend on every use — server env resolution, LLM
api keys, chat, git providers, code repos, webhooks — with no value cache,
no request timeout, and a retry that only fired on HTTP 403. A few seconds
of OpenBao unavailability therefore turned into minutes of degraded
service: instances that restarted during the blip failed env resolution,
got marked ERROR, and entered the 30s x5 then 5min backoff.
Three changes, in dependency order:
1. Typed errors. `SecretNotFoundError` (definitive) vs
`SecretBackendUnavailableError` (transport). The distinction has to be
typed rather than string-matched — a mis-classified "not found" would
resurrect deleted secrets, and a mis-classified auth failure would
paper over revoked grants, which is how an upstream re-init once broke
every secret write for four days (e51b924).
2. Driver resilience. Every request now carries an AbortSignal timeout
(there was none, so an unreachable backend hung its caller) and retries
5xx/429/network with full-jitter backoff — 503 is what a sealed
OpenBao returns and used to be an immediate hard failure. The 403
purge-and-retry stays single-shot and outside the retry budget: it is a
credential refresh, not a backend-unavailable condition, and looping on
it would hide a genuinely revoked grant.
`healthCheck()` no longer routes through the authenticated path, so an
expired role stops reporting as "OpenBao is down"; it maps OpenBao's
status codes (sealed/standby/uninitialised) instead. New `authCheck()`
covers the readiness half via list(), which exercises the capability we
actually depend on — unlike lookup-self, which only proves the token
exists.
3. CachingSecretBackendDriver. Fresh reads inside the TTL never touch the
network; past it we always try the backend, and on a transport failure
serve the last known-good value instead of throwing. That is what stops
the ERROR storm. Deleted secrets evict and rethrow — serving those
stale would resurrect a revoked credential, strictly worse than an
outage — and non-transport errors rethrow untouched. plaintext is not
wrapped: its read() is an identity function over the row handed in.
A cold cache during an outage still fails, loudly and by design (e6cd735).
Also routes BACKEND_TOKEN_DEAD / BACKEND_ROTATION_FAILED through pino
rather than bare console.error. They bypassed the multistream feeding
ErrorLogBuffer, so the one failure `mcpctl errors` exists to surface was
the one it never showed.
Tests: 20 new. The two load-bearing guards (never serve a deleted secret
stale; never serve stale for a non-transport error) were confirmed to fail
against deliberately broken code before being kept. The 403 purge-retry
path had no coverage at all until now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
