perf(vault): reuse the verified key during unlock

infrastructure/services/EncryptionService.ts

@@ -385,21 +385,27 @@ export const unlockMasterKey = async (
   password: string,
   config: MasterKeyConfig
 ): Promise<UnlockedMasterKey | null> => {
-  const isValid = await verifyPassword(password, config);
-  if (!isValid) return null;
+  try {
+    const salt = base64ToUint8Array(config.salt);
+    const derivedKey = await deriveKey(
+      password,
+      salt,
+      config.kdfIterations || SYNC_CONSTANTS.PBKDF2_ITERATIONS
+    );
+    const hash = await createVerificationHash(derivedKey);
+    if (hash !== config.verificationHash) return null;
 
-  const salt = base64ToUint8Array(config.salt);
-  const derivedKey = await deriveKey(
-    password,
-    salt,
-    config.kdfIterations || SYNC_CONSTANTS.PBKDF2_ITERATIONS
-  );
-
-  return {
-    derivedKey,
-    salt,
-    unlockedAt: Date.now(),
-  };
+    // Return the exact key that passed verification, rather than deriving the
+    // same password/salt key again. Ownership remains with the unlock caller.
+    return {
+      derivedKey,
+      salt,
+      unlockedAt: Date.now(),
+    };
+  } catch {
+    // Match verifyPassword's failure contract for malformed data/crypto errors.
+    return null;
+  }
 };
 
 /**

infrastructure/services/EncryptionService.unlock.test.ts

@@ -0,0 +1,118 @@
+import assert from 'node:assert/strict';
+import test, { type TestContext } from 'node:test';
+import { SYNC_CONSTANTS, type MasterKeyConfig } from '../../domain/sync';
+import { unlockMasterKey } from './EncryptionService';
+
+// These fakes prove call counts and verified-key ownership, not cryptography or
+// timing. Real encryption/rotation tests remain the integration gate.
+function installCryptoFixture(t: TestContext) {
+  const derivations: Array<{ key: CryptoKey; params: Pbkdf2Params }> = [];
+  const exportedKeys: CryptoKey[] = [];
+  const rawKeys = new Map<CryptoKey, ArrayBuffer>();
+  const encoder = new TextEncoder();
+  const salt = new Uint8Array([1, 2, 3, 4]);
+  const identity = (password: string, bytes: Uint8Array, iterations: number) =>
+    JSON.stringify([password, [...bytes], iterations]);
+  const bytesOf = (value: BufferSource): Uint8Array => ArrayBuffer.isView(value)
+    ? new Uint8Array(value.buffer, value.byteOffset, value.byteLength)
+    : new Uint8Array(value);
+  const base64 = (value: string) => btoa(String.fromCharCode(...encoder.encode(value)));
+
+  t.mock.method(crypto.subtle, 'importKey', async (
+    format: string, raw: BufferSource, algorithm: string, extractable: boolean, usages: string[],
+  ) => {
+    assert.equal(format, 'raw');
+    assert.equal(algorithm, 'PBKDF2');
+    assert.equal(extractable, false);
+    assert.deepEqual(usages, ['deriveBits', 'deriveKey']);
+    return { fixturePassword: new TextDecoder().decode(bytesOf(raw)) } as unknown as CryptoKey;
+  });
+  t.mock.method(crypto.subtle, 'deriveKey', async (
+    params: Pbkdf2Params, material: CryptoKey, algorithm: AesKeyGenParams,
+    extractable: boolean, usages: string[],
+  ) => {
+    assert.equal(params.name, 'PBKDF2');
+    assert.equal(params.hash, 'SHA-256');
+    assert.deepEqual(algorithm, { name: 'AES-GCM', length: 256 });
+    assert.equal(extractable, true);
+    assert.deepEqual(usages, ['encrypt', 'decrypt']);
+    const key = { fixtureSequence: derivations.length } as unknown as CryptoKey;
+    const password = (material as unknown as { fixturePassword: string }).fixturePassword;
+    rawKeys.set(key, encoder.encode(identity(password, bytesOf(params.salt), params.iterations)).buffer as ArrayBuffer);
+    derivations.push({ key, params });
+    return key;
+  });
+  t.mock.method(crypto.subtle, 'exportKey', async (format: string, key: CryptoKey) => {
+    assert.equal(format, 'raw');
+    exportedKeys.push(key);
+    assert.ok(rawKeys.has(key));
+    return rawKeys.get(key)!;
+  });
+  t.mock.method(crypto.subtle, 'digest', async (algorithm: string, data: BufferSource) => {
+    assert.equal(algorithm, 'SHA-256');
+    // Identity digest is deliberately not a crypto implementation.
+    return bytesOf(data).slice().buffer;
+  });
+  const makeConfig = (storedIterations?: number): MasterKeyConfig => ({
+    salt: btoa(String.fromCharCode(...salt)),
+    verificationHash: base64(identity('fixture-password', salt, storedIterations || SYNC_CONSTANTS.PBKDF2_ITERATIONS)),
+    kdf: 'PBKDF2',
+    kdfIterations: storedIterations as number,
+    createdAt: 1,
+  });
+  return { derivations, exportedKeys, salt, makeConfig };
+}
+
+test('unlock derives once and returns the exact verified key without caching across calls', async (t) => {
+  const fixture = installCryptoFixture(t);
+  const config = fixture.makeConfig();
+  const first = await unlockMasterKey('fixture-password', config);
+  assert.ok(first);
+  assert.equal(fixture.derivations.length, 1);
+  assert.equal(first.derivedKey, fixture.exportedKeys[0]);
+  assert.equal(first.derivedKey, fixture.derivations[0].key);
+  assert.deepEqual(first.salt, fixture.salt);
+  assert.equal(typeof first.unlockedAt, 'number');
+  const second = await unlockMasterKey('fixture-password', config);
+  assert.ok(second);
+  assert.equal(fixture.derivations.length, 2);
+  assert.notEqual(second.derivedKey, first.derivedKey);
+  assert.equal(second.derivedKey, fixture.exportedKeys[1]);
+});
+
+for (const iterations of [undefined, 0, 123456, SYNC_CONSTANTS.PBKDF2_ITERATIONS]) {
+  test(`unlock preserves stored iteration/fallback behavior: ${iterations}`, async (t) => {
+    const fixture = installCryptoFixture(t);
+    const result = await unlockMasterKey('fixture-password', fixture.makeConfig(iterations));
+    assert.ok(result);
+    assert.equal(fixture.derivations.length, 1);
+    assert.equal(fixture.derivations[0].params.iterations, iterations || SYNC_CONSTANTS.PBKDF2_ITERATIONS);
+  });
+}
+
+for (const scenario of ['password', 'hash', 'salt', 'iterations'] as const) {
+  test(`unlock rejects mismatched ${scenario} without returning a key`, async (t) => {
+    const fixture = installCryptoFixture(t);
+    const config = fixture.makeConfig();
+    if (scenario === 'hash') config.verificationHash = 'tampered';
+    if (scenario === 'salt') config.salt = btoa('other-salt');
+    if (scenario === 'iterations') config.kdfIterations = (config.kdfIterations || SYNC_CONSTANTS.PBKDF2_ITERATIONS) + 1;
+    const result = await unlockMasterKey(scenario === 'password' ? 'wrong-password' : 'fixture-password', config);
+    assert.equal(result, null);
+    assert.equal(fixture.derivations.length, 1);
+  });
+}
+
+test('unlock preserves null on malformed base64 salt', async (t) => {
+  const fixture = installCryptoFixture(t);
+  assert.equal(await unlockMasterKey('fixture-password', { ...fixture.makeConfig(), salt: '%%%' }), null);
+  assert.equal(fixture.derivations.length, 0);
+});
+
+for (const method of ['importKey', 'deriveKey', 'exportKey', 'digest'] as const) {
+  test(`unlock preserves null when ${method} rejects`, async (t) => {
+    const fixture = installCryptoFixture(t);
+    t.mock.method(crypto.subtle, method, async () => { throw new Error('fixture crypto rejection'); });
+    assert.equal(await unlockMasterKey('fixture-password', fixture.makeConfig()), null);
+  });
+}

infrastructure/services/cloudSync/convergentSyncStorageMethods.test.ts

@@ -256,3 +256,37 @@ test('failed local re-encryption leaves the active master key unchanged', async
     EncryptionService.unlockMasterKey = oldUnlock;
   }
 });
+
+test('real verified unlock keys decrypt existing records and survive master-key rotation', async () => {
+  const oldConfig = await EncryptionService.createMasterKeyConfig('old-integration-fixture');
+  const oldUnlocked = await EncryptionService.unlockMasterKey('old-integration-fixture', oldConfig);
+  assert.ok(oldUnlocked);
+  const record = { fixture: 'private sync baseline', revision: 7 };
+  const storage = new Map<string, unknown>();
+  storage.set(SYNC_STORAGE_KEYS.MASTER_KEY_CONFIG, oldConfig);
+  const originalCiphertext = await encryptLocalStorageValue(record, oldUnlocked.derivedKey);
+  storage.set(SYNC_STORAGE_KEYS.CONVERGENT_REPLICA, originalCiphertext);
+  const subject = {
+    ...manager(storage, oldUnlocked.derivedKey),
+    state: {
+      masterKeyConfig: oldConfig, securityState: 'UNLOCKED',
+      unlockedKey: oldUnlocked, autoSyncEnabled: false,
+    },
+    masterPassword: 'old-integration-fixture',
+    bumpSyncSecurityGeneration: () => undefined,
+    emit: () => undefined,
+    async reencryptSyncStorage(oldKey: CryptoKey, newKey: CryptoKey, nextConfig: MasterKeyConfig) {
+      assert.deepEqual(await decryptLocalStorageValue(originalCiphertext, oldKey), record);
+      await reencryptSyncStorageImpl.call(this, oldKey, newKey, nextConfig);
+    },
+  };
+  assert.equal(await changeMasterKeyImpl.call(subject, 'old-integration-fixture', 'new-integration-fixture'), true);
+  const rotatedCiphertext = storage.get(SYNC_STORAGE_KEYS.CONVERGENT_REPLICA) as string;
+  assert.notEqual(rotatedCiphertext, originalCiphertext);
+  assert.deepEqual(await decryptLocalStorageValue(rotatedCiphertext, subject.state.unlockedKey.derivedKey), record);
+  await assert.rejects(decryptLocalStorageValue(rotatedCiphertext, oldUnlocked.derivedKey));
+  const reopened = await EncryptionService.unlockMasterKey('new-integration-fixture', subject.state.masterKeyConfig);
+  assert.ok(reopened);
+  assert.deepEqual(await decryptLocalStorageValue(rotatedCiphertext, reopened.derivedKey), record);
+  assert.equal(await EncryptionService.unlockMasterKey('old-integration-fixture', subject.state.masterKeyConfig), null);
+});

infrastructure/services/cloudSync/unlockStaleCompletion.test.ts

@@ -28,3 +28,66 @@ test('a wrong password for the current configuration remains an ordinary verific
   assert.equal(await unlockImpl.call(manager, 'wrong-fixture-password'), false);
   assert.equal(manager.state.unlockedKey, null);
 });
+
+// No real KDF in these ownership cases. The service's per-call key reuse must
+// not change the manager's existing generation/config checks after await.
+for (const outcome of ['valid', 'invalid'] as const) {
+  for (const change of ['rotate', 'remove', 'generation'] as const) {
+    test(`mocked pending unlock cannot publish after ${change}: ${outcome}`, async (t) => {
+      const config = { verificationHash: 'fixture', salt: 'AQ==', kdf: 'PBKDF2' as const, kdfIterations: 600000, createdAt: 1 };
+      const key = { derivedKey: {} as CryptoKey, salt: new Uint8Array([1]), unlockedAt: 1 };
+      let resolve!: (value: typeof key | null) => void;
+      t.mock.method(EncryptionService, 'unlockMasterKey', () => new Promise<typeof key | null>((done) => { resolve = done; }));
+      let generation = 0;
+      let events = 0;
+      let syncStarts = 0;
+      const manager = {
+        state: {
+          masterKeyConfig: config as typeof config | null,
+          securityState: 'LOCKED', unlockedKey: null as typeof key | null, autoSyncEnabled: true,
+        },
+        masterPassword: null as string | null,
+        getSyncSecurityGeneration: () => generation,
+        emit: () => { events += 1; },
+        startAutoSync: () => { syncStarts += 1; },
+      };
+      const pending = unlockImpl.call(manager, 'fixture-password');
+      if (change === 'rotate') manager.state.masterKeyConfig = { ...config, salt: 'Ag==' };
+      if (change === 'remove') {
+        manager.state.masterKeyConfig = null;
+        manager.state.securityState = 'NO_KEY';
+      }
+      if (change === 'generation') generation += 1;
+      resolve(outcome === 'valid' ? key : null);
+      await assert.rejects(pending, /changed while unlocking/i);
+      assert.equal(manager.state.unlockedKey, null);
+      assert.equal(manager.masterPassword, null);
+      assert.equal(events, 0);
+      assert.equal(syncStarts, 0);
+    });
+  }
+}
+
+test('mocked concurrent same-config unlock keeps the already-installed winner', async (t) => {
+  const config = { verificationHash: 'fixture', salt: 'AQ==', kdf: 'PBKDF2' as const, kdfIterations: 600000, createdAt: 1 };
+  const winner = { derivedKey: {} as CryptoKey, salt: new Uint8Array([1]), unlockedAt: 1 };
+  const loser = { ...winner, derivedKey: {} as CryptoKey };
+  let resolve!: (value: typeof loser) => void;
+  const unlock = t.mock.method(EncryptionService, 'unlockMasterKey', () => new Promise<typeof loser>((done) => { resolve = done; }));
+  const manager = {
+    state: { masterKeyConfig: config, securityState: 'LOCKED', unlockedKey: null as typeof winner | null },
+    masterPassword: null as string | null,
+    getSyncSecurityGeneration: () => 0,
+    emit: () => { assert.fail('losing unlock must not emit'); },
+  };
+  const pending = unlockImpl.call(manager, 'fixture-password');
+  manager.state.securityState = 'UNLOCKED';
+  manager.state.unlockedKey = winner;
+  manager.masterPassword = 'winner-password';
+  resolve(loser);
+  assert.equal(await pending, true);
+  assert.equal(manager.state.unlockedKey, winner);
+  assert.equal(manager.masterPassword, 'winner-password');
+  assert.equal(await unlockImpl.call(manager, 'another-password'), true);
+  assert.equal(unlock.mock.callCount(), 1, 'already-unlocked requests must not derive');
+});