Repository navigation
feat(vault): backup export, independent import and merge migration (A3) - #387
Merged
Merged
Conversation
…oc precision (A3 review) - exportVaultBackup JSDoc: the lastBackupAt CAS bump invalidates unlocked sessions for this vault in ANY tab, including the caller's own service instance (next operation -> VAULT_SESSION_STALE); A4/A5 wiring note to tolerate re-unlocking (or lock before exporting). - importVaultBackup JSDoc: document the input contract — text backups go through parseVaultEnvelopeJson first (raw-JSON 1 MiB gate); this function takes an already-parsed VaultEnvelopeV1 object. - tests: end-to-end illegal-role import case (real encrypted envelope with role 'superuser' -> VAULT_PAYLOAD_INVALID, nothing created); export -> import chained roundtrip (envelope bytes survive the CAS marker move, new vault decrypts to the source payload under the same password); removeVault rollback-failure injection (primary error propagates, the residual is an unlockable EMPTY vault, locked against drift). - whole-file PBKDF2 budget comment updated (24 -> 34 derivations).
…nstance (A3 review r2) P2: the independent import's create->unlock gap spans a ~1s PBKDF2 derivation, during which another tab may remove the just-created vault and recreate the same name. The unlock looks the vault up by name and VaultServiceStatus exposes no createdAt, so the session could adopt the replacement record (writing the source payload into the other tab's vault) and the bare-name rollback could delete it. The import path is now instance-bound end to end: - adoption check after unlock, before the payload write: re-read the record and require the created instance (same name + createdAt), else reject VAULT_SESSION_STALE and roll back without writing; - rollback re-reads and deletes ONLY the created instance; a same-name replacement is left untouched (stand down, primary error propagates); absent records still get the best-effort removal (NOT_FOUND swallowed); - the final read-back also verifies the instance before returning it. P3 follow-ups from the same review round: - mergeVaultBackup JSDoc: empty/no-change merges are still rewrites (fresh IV + revision +1), no equality short-circuit by design; - tests: empty-source import and empty-source merge semantics locked; - tests: header PBKDF2 budget corrected (the bad-role case was both listed separately and counted in 'nine') and rebudgeted for the new cases (49 derivations total).
- Status header: A3 landed (PR #387); A4-A6 remain - Module map gains backup.ts and vault-backup.test.ts entries - A3 extension checklist rewritten as wiring rules: text backups through parseVaultEnvelopeJson, single-password contract, instance-bound import adoption/rollback (with the documented sub-ms residual window), target-first merge semantics incl. empty-source rewrite, export's revision bump invalidating sessions in any tab 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
A3 task package:
src/lib/vault/backup.ts(new, 431 lines) +tests/unit/vault-backup.test.ts(new, 27 cases). Zero diff on all existing files — the seven landed vault modules and every existing test are untouched; the new code composes the A1.1 shared validators and the A2 registry/service primitives only.Kickoff A3 implementation rules — how each is met
exportVaultBackupextracts the envelope viavaultRecordToEnvelope(fail-closed re-validation included); revision/lockEpoch/timestamps never enter a backup. Reading existing ciphertext needs no new IV (asserted byte-identical salt/iv/ciphertext).lastBackupAtis updated throughswapRecordCAS;VAULT_SESSION_STALEpropagates per the registry contract (tested with an injected concurrent lockEpoch bump).createVault(fresh salt/IV minted inside, password policy enforced) and the payload written under a second fresh IV.settings.activeVaultNameis never touched (asserted).(accountName, role, publicKey)dedupe — all through the sharedvalidateVaultWif/validateVaultPayloadTextentry points (never re-implemented here); fully offline; backup contents are never treated as on-chain authority proof.preMergeBackup(the internal safety net; memory-only, deliberately does NOT updatelastBackupAt— that marker belongs to user-initiated exports). Merging happens insidemutatePayload(fresh IV + CAS, A2 primitive); target entries win(accountName, role, publicKey)conflicts (target-first iteration, target's raw spelling preserved); target entries are re-validated throughvalidateVaultWif— an invalid target entry aborts withVAULT_KEY_INVALIDand nothing is written.validateVaultEnvelope, payload caps invalidateVaultPayloadText(insidemutatePayloadfor merges, so an over-limit merged result leaves the target byte-identical). No bounds are re-derived at call sites.Kickoff A3 completion criteria — evidence
['ciphertext','iv','salt','vaultName','version']keys and byte-equal crypto material.(accountName, role, publicKey): union + conflict case asserts the target entry (rawAlicespelling) wins and the source-only entry is added normalized; target-internal duplicates also collapse.toEqual(before); a failed import removes the just-created vault (tested — no empty vault left behind).Design decisions worth review attention
createVaultcomposition (signature gap, not worked around by editing A2): the A2createVault({vaultName, password})takes no payload and always creates an empty vault. Independent import therefore composescreateVault(atomic alias uniqueness + fresh salt/IV + policy) → in-scopeservice.unlock→mutatePayload(payload under a second fresh IV, revision 1, CAS). Cost: one extra PBKDF2 derivation per import. Write path still goes exclusively through the registry's atomic transaction and the service mutator — no direct IndexedDB access anywhere.VAULT_PAYLOAD_INVALID, notVAULT_KEY_INVALID: role legality is enforced by the sharedvalidateVaultPayloadText(the pipeline's step 2, before per-entry WIF checks), and the kickoff assignsVAULT_KEY_INVALIDto WIF/public-key/account failures. An explicit per-entry role check would be unreachable dead code; the test documents the composed behavior.VAULT_LIMIT_EXCEEDED. The payload-layer branch is exercised for real on the in-memory merge path (401-key merged result) and asserted directly againstvalidateVaultPayloadText.exportVaultBackupbumpsrevision:swapRecord's contract mandates revision+1 per write, so recordinglastBackupAtalso invalidates other tabs' unlocked sessions for that vault — documented in the JSDoc as the accepted cost of the only mutating primitive.{ status, preMergeBackup }: the auto-backup safety net is returned to the vault-internal caller (A4/A5) instead of dangling as an unused local; it stays memory-only and never toucheslastBackupAt.importVaultBackuptakes the parsed envelope object per the A0ImportVaultParamscontract; the raw-text gate (parseVaultEnvelopeJson, same shared constants) is what A4/A5 callers compose before this call — covered by a test and noted for A4.Test plan
tests/unit/vault-backup.test.ts: 27/27 passed, 49 PBKDF2 derivations total (2 shared inbeforeAll), tests-time 15.5–17.6 s (budget <20 s), shuffle-stable across seeds 1/3/7/11/42/99.pnpm test).pnpm type-checkandpnpm lintclean (strict TS; no new dependencies).pnpm test:coverage):src/lib/vault97.57% stmts / 92.26% branch;backup.ts97.5% stmts / 92.85% branch / 97.06% lines (100% funcs); global 83.78% / 74.49% (thresholds 80/60). Uncovered lines 248 and 377 are defensive post-hard-line branches (the import final read-back instance guardVAULT_SESSION_STALE, and merge'sVAULT_NOT_FOUNDrace guard).node tests/node/vault-wif-real-check.mjs: 9/9 PASS (rerun discipline; this PR does not change WIF logic — expected and confirmed).Review rounds
VAULT_PAYLOAD_INVALIDper the A1.1 contract;lastBackupAtCAS side-effect acceptable). Follow-upd15675d1: JSDoc precision (export invalidates sessions in ANY tab incl. the caller's own; text-backup parse contract) + 3 tests (end-to-end bad-role, export->import roundtrip, rollback-failure injection).3cbb8df2: instance-bound adoption check + instance-bound rollback + instance-bound final read-back, with two race regression tests verified to fail pre-fix; plus empty-source cases and budget-note fixes.Not verified here (honest declarations)
tests/mocks/steem-js.ts:isWifoverridable, fakegetPublicKey) — branch/error-code coverage only; real checksum/derivation evidence is the plain-Node script above (two-layer A1.1 rule).