Repository navigation
feat(vault): IndexedDB registry and unlock-session service (A2) - #386
Merged
Merged
Conversation
registry.ts: versioned IndexedDB database (steem-wallet-vault v1) with vaults + settings stores. Alias uniqueness = vaultNameComparisonKey comparison inside the same readwrite transaction as the write (IDB writer serialization makes check-then-act atomic; a unique index was rejected because it would persist a non-contract key field). revision/lockEpoch CAS via swapRecord, lockEpoch bumps, rename with settings-pointer coherence, remove with atomic active-pointer clearing, fail-closed VAULT_STORAGE_UNAVAILABLE with no in-memory fallback. createVault runs the shared name/password validators and zeroizes its derived KEK in scope. service.ts: closure-held KEK + decrypted payload. unlock (switch clears first), lock (kek.fill(0) then drop references, lockEpoch+1 persisted, broadcast), getAccounts (public-key-derived summaries only, via the shared validateVaultWif entry point), mutatePayload (fresh IV, whole re-encryption, CAS revision+1, validate-before-write rollback). Cross-tab invalidation: hard line re-reads the registry before every session-bound operation; BroadcastChannel lock/mutate events are acceleration only and degrade gracefully when absent. Tests run on a hand-written fake IndexedDB (tests/mocks/fake-indexeddb.ts, no new dependencies; fidelity boundary documented in its header). 57 new cases (39 registry + 18 service); 13 PBKDF2 derivations total in the service file, ~10s.
…ilure paths (A2 review) Three-party review fixes for PR #386 (1xP1, 2xP2, P3 batch): - P1: ActiveSession and VaultRevisionPointer now carry the record's createdAt, compared by requireFreshSession and by the swapRecord CAS — a same-name remove+recreate (whose counters restart at 0/0) can no longer adopt an old session and overwrite the new vault's ciphertext under the old KEK. Residual same-millisecond recreate window is documented in both files (closing it needs a random per-record id, a VaultRecordV1 type-contract change, out of scope for V1). - P2: mutatePayload's CAS adopt branch uses the LIVE session's KEK, not the await-time snapshot — a destroy + same-password re-unlock while the CAS is in flight would otherwise install the zeroized snapshot bytes and permanently corrupt the vault on the next mutation. The returned status now reports the service's current session truth instead of always "unlocked at the new revision". - P2: lock() broadcasts the acceleration event in a finally around bumpLockEpoch — a persistence failure no longer leaves other tabs silently unlocked (the error still propagates; memory-first order unchanged). - P3: runInTransaction aborts the transaction when `run` throws mid-flight (real IDB would auto-commit the partial writes otherwise); unlock re-checks the record after derivation+decryption and fails with VAULT_SESSION_STALE on concurrent removal/mutation instead of returning a session that is born stale. - P3 cleanups: registry test header derivation count corrected (7, not 4); BroadcastChannel name reuses VAULT_DB_NAME; fake-indexeddb install clears stale databaseStates, drops ~30 lines of dead index code, and exposes abortCount() as the observable for the new abort regression test; vault-service tests import VAULT_DB_NAME instead of hardcoding the name; TODO comment for onversionchange/onblocked left at openDatabase (DB version is fixed at 1). vault-service.test.ts restructured for order independence: every test rebuilds its own fake DB, registry and services in beforeEach and shares only beforeAll-derived KEKs — shuffle-safe (verified across several --sequence.shuffle seeds), 13 cases (the 18 order-dependent cases consolidated + 4 new review regressions), ~17s at 22 PBKDF2 derivations total. Registry tests: +2 cases (recreated-record CAS identity, mid-rename abort). Full suite 1434 green; every regression test verified to fail with its fix reverted.
- Status header and module map now cover registry.ts / service.ts / the fake IndexedDB and the two new test suites (A2, PR #386) - New invariants 11-14: session identity includes createdAt (remove + same-name-recreate collision), IndexedDB transaction discipline (IDB request promises only inside callbacks, abort on throw, comparison key never persisted), two-layer cross-tab invalidation with lock-broadcast in a finally, post-CAS adopt uses the live closure KEK, mutator payload copies stay vault-internal - Pitfalls: jsdom lacks IndexedDB but has Node's BroadcastChannel; fake fidelity bounds and the real-Chromium golden-check precedent; concurrent unlock overwrite semantics; shuffle-stable lifecycle test pattern - Extension checklist: A2 marked landed; A3 import guidance wired to the registry transaction and mutatePayload Part of #381
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #381
Summary
A2 of the vault development kickoff: the persistent storage layer and the in-memory unlock-session layer. Five new files, zero changes to existing files (the A0/A1/A1.1 five vault modules and all existing tests are untouched).
src/lib/vault/registry.ts— IndexedDB storage primitivessteem-wallet-vault(v1), storesvaults(keyPathvaultName, oneVaultRecordV1per vault) andsettings(singletonVaultSettingsV1).readwritetransaction, comparingvaultNameComparisonKeyvalues (fromvalidation.ts— the shared entry points; nothing re-implemented). Design choice: auniqueindex was rejected because it would require persisting a comparison-key field insideVaultRecordV1, which is not part of the type contract; IDB's per-store writer serialization gives check-then-act atomicity. The comparison key is not the vault identity and never enters the envelope/AAD.createVaultvalidates name (VAULT_NAME_INVALID) + password (VAULT_PASSWORD_WEAK) at the primitive level (not left to callers), has a cheap pre-check so duplicate names never pay PBKDF2, and zeroizes its derived KEK inside the call — the registry never retains key material.swapRecord(expected, next): in-transaction CAS on(createdAt, revision, lockEpoch); drift →VAULT_SESSION_STALE, gone →VAULT_NOT_FOUND, malformed envelope →VAULT_ENVELOPE_INVALID, contract violations →TypeError(programmer error, crypto.ts precedent).next.revisionmust beexpected.revision + 1.bumpLockEpoch(lockEpoch+1, revision untouched),renameVault(case-only self-rename allowed, collision with another comparison key rejected, settings active-pointer follows atomically),removeVault(active pointer cleared atomically with the delete),getSettings/putSettings(defaultssigningMode: 'privateKey', corrupt row fails closed withVAULT_PAYLOAD_INVALID).indexedDBglobal, throwingopen, erroring open request, or use-after-close →VAULT_STORAGE_UNAVAILABLE. No in-memory fallback pretending to be persistence.src/lib/vault/service.ts— unlock sessions (createVaultService(registry)factory)getAccounts()returns only public-key-derivedVaultAccountSummaryvalues (via the sharedvalidateVaultWifentry point),getStatus()only non-sensitive counters.unlock(switch clears the previous session completely first — its lockEpoch is persisted),lock(KEKfill(0)BEFORE dropping references, thenlockEpoch+1 persisted, then broadcast). Honest zeroization statement: the KEK bytes can be zeroed; WIF/password strings cannot be physically zeroed — dropping references is the documented limit, the code never claims otherwise. A KEK from a failed unlock is zeroized before the error propagates.mutatePayload: draft copy → validate (failure = rollback by construction: nothing written, session keeps its original payload) → fresh IV → whole-payload re-encryption under the session KEK (salt unchanged) → CAS revision+1.revision/lockEpochbefore EVERY session-bound operation (status queries and mutations today, signing from A4 on) — delayed notifications are never the only defense.BroadcastChannel('steem-wallet-vault')lock/mutate events clear the local session early (acceleration); when the API is absent the service degrades gracefully.Kickoff A2 completion criteria — item by item
Work/work, NFC/NFDCaféconflicts (VAULT_NAME_EXISTS), racing creators with colliding names — exactly one wins (serialized transactions). ✅putSettings/getSettingswithactiveVaultName); the manager-levelactivateVaultorchestration is A4 scope. ✅ (activation = settings primitive only)Test plan
tests/unit/vault-registry.test.ts— 41 cases, ~4.5s (7 PBKDF2 derivations; everything else seeded from the fixed vectors).tests/unit/vault-service.test.ts— 13 cases, ~16-18s (22 derivations total incl. shared beforeAll KEKs; every case rebuilds state in beforeEach — shuffle-stable).pnpm verifygreen — type-check, lint (0 new warnings), 131 files / 1434 tests, coverage thresholds met (global 83.5% stmts / 74% branch). Vault module: registry.ts 95.9% stmts / 86.4% branch, service.ts 91.7% stmts / 75.9% branch / 100% funcs (crypto/envelope/validation still 100%/98.6%/100%).indexedDB(fake required — as briefed); it does have a workingBroadcastChannel(Node's own implementation leaks into the environment — divergence from the task brief). The "no BroadcastChannel" hard-line cases therefore hide the global explicitly; the acceleration cases use the real Node channel (cross-instance delivery, no self-receipt, structured clone verified by probe).Fake IndexedDB fidelity boundary
tests/mocks/fake-indexeddb.ts(hand-written, no new dependencies): single shared state per database name (what the cross-instance "two tabs" cases need), FIFO transaction queue approximating IDB writer serialization, macrotask auto-commit grace, JSON deep-clone on read/write, anabortCount()observability hook. NOT simulated: versionchange/blocked negotiation, abort rollback, cursors/IDBKeyRange, real multi-connection browser semantics. Shared by both test files (atests/mocks/module — chosen over in-file copies so registry and service tests exercise the same fake).Review follow-up (commit e02908c)
Three-way review round 1 found 1×P1 + 2×P2 (session-identity and failure-path defects, each experimentally reproduced), fixed in
e02908c4with regression tests verified to fail with the fix reverted:createdAteverywhere (freshness re-read, CAS pointer, adopt branch); residual same-millisecond window documented in-code (closing it fully needs a random per-record id, aVaultRecordV1change deferred to a future schema version).lock()skipped the BroadcastChannel notification when the lockEpoch persistence failed; the broadcast now runs in afinallyand the error still propagates.runInTransactionaborts on callback throw (no partial writes on real IDB), unlock re-checks the record after derivation (mid-unlock removal →VAULT_SESSION_STALE), service tests rebuilt as per-case independent state (shuffle-stable), fake dead index surface removed, several small hardening items.Round 2 (3-way) confirmed the fixes via adversarial replay, a real-Chromium + real-IndexedDB golden run (43/43 incl. real rollback semantics and cross-page BroadcastChannel) and negative mutation controls (6/6 assertions flip red with fixes reverted).
Not verified / known limits (stated honestly)
getAccountsis mock-driven under vitest (two-layer A1.1 discipline); real-crypto evidence remainstests/node/vault-wif-real-check.mjs(rerun not required — no WIF-handling change).service.tssession-replaced-mid-await (throw locked()in the guard) and the CAS-race cleanup guard inmutatePayload— both exist precisely for interleavings that cannot be triggered deterministically through the public API; registry transactiononerror/onabortbodies and the generic storage wrap likewise never fire on the fake. Above thresholds regardless.openVaultRegistry(dbName)accepts a name override for future test parallelism; default isVAULT_DB_NAME.