Vault: one unopenable sealed row permanently bricks unlock #142

New issue

Open

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

lz commented

2026-09-12 19:32:20 +02:00

Owner

Found in the /pr-review pass over feat/mcp (envelope encryption).

The mechanism

upgradeLegacyVault throws when any sealed row fails to open, rolls its transaction back, and rethrows. routes/api/auth/unlock/+server.ts:15 has no try/catch, so the throw becomes SvelteKit's default {"message":"Internal Error"} with status 500.

On a legacy install the upgrade runs on every unlock attempt until it succeeds. So a single undecryptable connections.encrypted_token or repo_env_vars.encrypted_value row means:

the operator can never log in again,
there is no UI route to delete the offending row (that needs a session),
rotation can't help either (it unlocks first),
recovery is hand-editing SQLite.

The throw itself is correct and deliberate — reporting damage as "wrong passphrase" sends the operator to the wrong problem, and vault-envelope.ts documents that choice. The bug is that "throw" currently means "uncaught 500", not "a legible error with a recovery path".

Note the throw protects data that is already unrecoverable: a row that opens under no key is lost whether or not the upgrade proceeds.

How a dead row could arise

Verified there was a real window: c8aa70b (2026-07-16 19:37Z) added repo_env_vars; c472266 (2026-07-17 10:13Z) taught rotateMasterPassphrase to re-seal it. A passphrase rotation in that ~15h window left encrypted_value rows sealed under the pre-rotation key. Both commits are on main.

This instance is not affected — checked before merging feat/mcp: one repo_env_vars row, created 2026-07-17T17:24Z, 7.2 hours after the fix. Filed because the mechanism outlives that particular window (partial restores, manual edits, future bugs).

Options

Skip-and-log the unopenable row instead of throwing — the data is already lost, and this lets the other rows upgrade.
Keep the throw, catch it in the unlock route, and return a distinguishable error naming the table/row id plus a recovery note.
Both: skip, log loudly, and surface a warning once unlocked.

(1)+(3) is probably right, but it changes the "damage must not read as a wrong passphrase" contract, so it wants a deliberate decision rather than a drive-by fix.

/api/auth/unlock is also the only route where vault damage is invisible — the rotation route already returns {ok:false, reason:'error', message} and surfaces it in the modal.

Found in the `/pr-review` pass over `feat/mcp` (envelope encryption). ## The mechanism `upgradeLegacyVault` throws when any sealed row fails to open, rolls its transaction back, and rethrows. `routes/api/auth/unlock/+server.ts:15` has **no try/catch**, so the throw becomes SvelteKit's default `{"message":"Internal Error"}` with status 500. On a legacy install the upgrade runs on *every* unlock attempt until it succeeds. So a single undecryptable `connections.encrypted_token` or `repo_env_vars.encrypted_value` row means: - the operator can never log in again, - there is no UI route to delete the offending row (that needs a session), - rotation can't help either (it unlocks first), - recovery is hand-editing SQLite. The throw itself is **correct and deliberate** — reporting damage as "wrong passphrase" sends the operator to the wrong problem, and `vault-envelope.ts` documents that choice. The bug is that "throw" currently means "uncaught 500", not "a legible error with a recovery path". Note the throw protects data that is *already* unrecoverable: a row that opens under no key is lost whether or not the upgrade proceeds. ## How a dead row could arise Verified there was a real window: `c8aa70b` (2026-07-16 19:37Z) added `repo_env_vars`; `c472266` (2026-07-17 10:13Z) taught `rotateMasterPassphrase` to re-seal it. A passphrase rotation in that ~15h window left `encrypted_value` rows sealed under the pre-rotation key. Both commits are on `main`. **This instance is not affected** — checked before merging `feat/mcp`: one `repo_env_vars` row, created 2026-07-17T17:24Z, 7.2 hours after the fix. Filed because the mechanism outlives that particular window (partial restores, manual edits, future bugs). ## Options 1. Skip-and-log the unopenable row instead of throwing — the data is already lost, and this lets the other rows upgrade. 2. Keep the throw, catch it in the unlock route, and return a distinguishable error naming the table/row id plus a recovery note. 3. Both: skip, log loudly, and surface a warning once unlocked. (1)+(3) is probably right, but it changes the "damage must not read as a wrong passphrase" contract, so it wants a deliberate decision rather than a drive-by fix. ## Related `/api/auth/unlock` is also the only route where vault damage is invisible — the rotation route already returns `{ok:false, reason:'error', message}` and surfaces it in the modal.

lz referenced this issue

2026-09-12 19:35:15 +02:00

Envelope-encrypt the vault: one DEK, per-principal wrapped keys #146

lz referenced this issue

2026-09-15 18:25:48 +02:00

Slice 2: machine principal, create_workspace and start_workspace #159