Vault: a lost vault_keys row routes the operator to /setup, which conceals the loss #143

Open
opened 2026-09-12 19:32:32 +02:00 by lz · 0 comments
Owner

Found in the /pr-review pass over feat/mcp.

isSetupComplete (auth/vault-store.ts) answers yes if a master principal exists, else falls back to meta.kdf_salt to recognise a pre-envelope install. That fallback was added deliberately (commit 2081490) to stop /setup writing a fresh DEK over a legacy vault.

But a fresh envelope install writes no meta rows at allsetupMasterPassphrase only writes the principal. So the guard covers legacy installs and not envelope ones. If the single vault_keys row is lost (restore from a backup predating migration 0020, a manual DELETE, 0020.down(), a partial table restore), isSetupComplete returns false, /api/state reports needs-setup, PHASE_ROUTE forces /setup, and setup succeeds silently with a brand-new random DEK.

Severity: this conceals data loss, it does not cause it

Worth being precise, because the first framing overstated it. wrapped_dek is the only copy of the data key — the moment that row vanishes, every encrypted_* value is already unrecoverable. /setup re-keying destroys nothing that was still readable.

What it costs is the diagnostic and the restore window: the operator is told they have a fresh install, reconnects providers, re-enters env vars, and never learns that restoring vault_keys from a backup would have recovered everything.

Suggested fix

Make the predicate "sealed data exists but no principal does" an explicit refusal rather than letting it fall through to setup:

  • if any connections or repo_env_vars row exists and no principal does → refuse setup with "this instance holds sealed data but its vault key is missing; restore vault_keys from a backup".

That generalises the meta.kdf_salt fallback, which only approximates the same condition for legacy installs.

Found in the `/pr-review` pass over `feat/mcp`. `isSetupComplete` (`auth/vault-store.ts`) answers yes if a `master` principal exists, else falls back to `meta.kdf_salt` to recognise a pre-envelope install. That fallback was added deliberately (commit `2081490`) to stop `/setup` writing a fresh DEK over a legacy vault. But a **fresh envelope install writes no `meta` rows at all** — `setupMasterPassphrase` only writes the principal. So the guard covers legacy installs and not envelope ones. If the single `vault_keys` row is lost (restore from a backup predating migration 0020, a manual `DELETE`, `0020.down()`, a partial table restore), `isSetupComplete` returns **false**, `/api/state` reports `needs-setup`, `PHASE_ROUTE` forces `/setup`, and setup succeeds silently with a brand-new random DEK. ## Severity: this conceals data loss, it does not cause it Worth being precise, because the first framing overstated it. `wrapped_dek` is the **only** copy of the data key — the moment that row vanishes, every `encrypted_*` value is already unrecoverable. `/setup` re-keying destroys nothing that was still readable. What it costs is the diagnostic and the restore window: the operator is told they have a fresh install, reconnects providers, re-enters env vars, and never learns that restoring `vault_keys` from a backup would have recovered everything. ## Suggested fix Make the predicate "sealed data exists but no principal does" an explicit refusal rather than letting it fall through to setup: - if any `connections` or `repo_env_vars` row exists and no principal does → refuse setup with "this instance holds sealed data but its vault key is missing; restore `vault_keys` from a backup". That generalises the `meta.kdf_salt` fallback, which only approximates the same condition for legacy installs.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lz/agent-nexus#143
No description provided.