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

michal · 2026-08-20T21:37:50Z

michal commented

2026-08-20 21:37:50 +00:00

Why

Two gaps, both verified against the live cluster.

OpenBao outages cascaded. mcpd re-read the backend on every secret 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 sealed OpenBao answers 503, which was an immediate hard failure; an unreachable one hung its caller indefinitely. So a few seconds of unavailability became minutes of instance ERROR backoff (30s x5, then 5min forever).

mcpctl status could not tell you. The Secrets: verdict came solely from tokenMeta.lastRotationError, and the rotator writes that field only for auth: 'token' backends. The live bao-k8s backend is auth: kubernetes, so it never wrote it — the tick was structurally incapable of going red, and reported OpenBao healthy while it was unreachable.

What changed

CachingSecretBackendDriver — TTL cache with stale-while-error. On a transport failure it serves last-known-good rather than throwing, which is what stops the ERROR storm. A deleted secret evicts and rethrows: serving that stale would resurrect a revoked credential. plaintext is not wrapped.
Typed errors — SecretNotFoundError vs SecretBackendUnavailableError. Typed rather than string-matched, because a mis-classification either resurrects deleted secrets or papers over revoked grants.
Driver hardening — AbortSignal timeout on every call including login (which had none), full-jitter retry on 5xx/429/network. The 403 purge-retry stays single-shot and outside the retry budget so a genuinely revoked grant still fails loudly.
GET /api/v1/secretbackends/:id/health — live (unauthenticated) vs ready (real read). live && !ready is the re-initialised-OpenBao case; one boolean cannot express it. No RBAC mapping needed.
Honest mcpctl status — four states, rotation error demoted to a trailing clause. A failed probe renders ? unknown, never green.
BACKEND_TOKEN_DEAD through pino — it was a bare console.error, bypassing the multistream feeding ErrorLogBuffer, so mcpctl errors never showed the one failure it exists for.
Boot cache warm — closes the cold-cache case for outages that begin after startup. Deliberately partial: if the backend is down at boot, instances still fail loudly.

Tests

39 new; 1048 mcpd + 730 cli green. The load-bearing guards were confirmed to fail against deliberately broken code before being kept — the 5 status assertions all fail against the old rotation-only logic, and the cache's never-serve-a-deleted-secret guard fails if the typed-error branch is removed.

SecretBackendRotatorLoop had zero coverage despite being the boot-time dead-token detector added after an upstream re-init broke every secret write for four days; it now has nine cases.

Known state

src/mcplocal/tests/smoke/secret-resilience.smoke.test.ts fails until this is deployed — it asserts the probed rendering and the live cluster still returns the old Secrets: bao-k8s* ✓. That is the intended signal, not a broken test.

Cold cache during an outage still fails, by design (e6cd735): booting a server with an empty credential is worse than failing loudly.

## Why Two gaps, both verified against the live cluster. **OpenBao outages cascaded.** mcpd re-read the backend on every secret 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 sealed OpenBao answers 503, which was an immediate hard failure; an unreachable one hung its caller indefinitely. So a few seconds of unavailability became minutes of instance ERROR backoff (30s x5, then 5min forever). **`mcpctl status` could not tell you.** The `Secrets:` verdict came solely from `tokenMeta.lastRotationError`, and the rotator writes that field only for `auth: 'token'` backends. The live `bao-k8s` backend is `auth: kubernetes`, so it never wrote it — the tick was structurally incapable of going red, and reported OpenBao healthy while it was unreachable. ## What changed - **`CachingSecretBackendDriver`** — TTL cache with stale-while-error. On a transport failure it serves last-known-good rather than throwing, which is what stops the ERROR storm. A deleted secret evicts and rethrows: serving that stale would resurrect a revoked credential. `plaintext` is not wrapped. - **Typed errors** — `SecretNotFoundError` vs `SecretBackendUnavailableError`. Typed rather than string-matched, because a mis-classification either resurrects deleted secrets or papers over revoked grants. - **Driver hardening** — AbortSignal timeout on every call including login (which had none), full-jitter retry on 5xx/429/network. The 403 purge-retry stays single-shot and outside the retry budget so a genuinely revoked grant still fails loudly. - **`GET /api/v1/secretbackends/:id/health`** — `live` (unauthenticated) vs `ready` (real read). `live && !ready` is the re-initialised-OpenBao case; one boolean cannot express it. No RBAC mapping needed. - **Honest `mcpctl status`** — four states, rotation error demoted to a trailing clause. A failed probe renders `? unknown`, never green. - **`BACKEND_TOKEN_DEAD` through pino** — it was a bare `console.error`, bypassing the multistream feeding `ErrorLogBuffer`, so `mcpctl errors` never showed the one failure it exists for. - **Boot cache warm** — closes the cold-cache case for outages that begin after startup. Deliberately partial: if the backend is down at boot, instances still fail loudly. ## Tests 39 new; 1048 mcpd + 730 cli green. The load-bearing guards were confirmed to fail against deliberately broken code before being kept — the 5 status assertions all fail against the old rotation-only logic, and the cache's never-serve-a-deleted-secret guard fails if the typed-error branch is removed. `SecretBackendRotatorLoop` had zero coverage despite being the boot-time dead-token detector added after an upstream re-init broke every secret write for four days; it now has nine cases. ## Known state `src/mcplocal/tests/smoke/secret-resilience.smoke.test.ts` fails until this is deployed — it asserts the probed rendering and the live cluster still returns the old `Secrets: bao-k8s* ✓`. That is the intended signal, not a broken test. Cold cache during an outage still fails, by design (e6cd735): booting a server with an empty credential is worse than failing loudly.

michal added 4 commits 2026-08-20 21:37:51 +00:00

feat(secrets): survive OpenBao outages instead of cascading them bd3e1134c9

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

feat(secrets): report real backend health instead of a hard-coded tick 0fbfc72d68

`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

test(secrets): smoke-cover backend health honesty + cache correctness fe9987cefd

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

test(secrets): cover the rotator loop's boot-time dead-token detection

CI/CD / typecheck (pull_request) Successful in 1m25s

Details

CI/CD / lint (pull_request) Successful in 2m34s

Details

CI/CD / test (pull_request) Successful in 1m32s

Details

CI/CD / smoke (pull_request) Failing after 3m3s

Details

CI/CD / build (pull_request) Successful in 2m26s

Details

CI/CD / publish (pull_request) Has been skipped

Details

545e7745da

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

michal merged commit eb3e558a44 into main

2026-08-20 21:37:58 +00:00

michal referenced this issue from a commit

2026-08-20 21:37:59 +00:00

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

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: michal/mcpctl#115