fix(security): serialise vault writes and make KDF upgrades crash-safe (#54)

KS-2: route every whole-collection read-modify-write in StorageService through
a per-storage-key FIFO mutex, acquiring multiple keys in a fixed order. Two
interleaved mutations (import while a save is in flight, autosave plus manual
save, a save during a rekey) can no longer overwrite each other from the same
in-memory snapshot. A queued mutation rebuilds from the committed cache and a
re-encryption excludes concurrent writes on the collections it rewrites.

KS-1: record the new KDF metadata as a pending recovery record before
re-encrypting the vault and commit the active metadata with a single
SecureStore write. The next master-password login derives both candidates,
inspects which key decrypts the stored collections, and either promotes the
pending metadata or discards the record and keeps the previous password
working. A process kill between the vault write and the metadata commit no
longer leaves ciphertext whose salt was never stored. Biometric unlock defers
to the master password while a recovery record exists, and the login messages
state whether the new or the previous password is required. Crypto parameters
and error checks are unchanged.

Tests: interleaved saves with a delayed AsyncStorage mock; simulated crash
between the vault write and the metadata commit for both the legacy KDF
upgrade and the master-password change, covering the new-password, old-then-new
password and pre-vault-write rollback states. Docs and CHANGELOG updated.

Co-authored-by: Michael Gasperini <michael@mikesoft.it>

Author: TheStreamCode

CHANGELOG.md

@@ -4,7 +4,22 @@ All notable project changes are documented here.
 
 ## [Unreleased]
 
-No unreleased changes.
+### Security
+
+- Made transparent KDF upgrades and master-password changes crash-safe. The vault is
+  re-encrypted only after a pending recovery record describing the new KDF metadata is
+  persisted alongside the still-active previous metadata, and the new metadata commit is a
+  single SecureStore write. A process kill between the vault write and the metadata commit
+  no longer leaves ciphertext encrypted under a salt that was never stored: the next
+  password login inspects the vault records and either promotes the pending metadata or
+  discards it, keeping the previous password usable when the vault was not re-encrypted
+  yet. Biometric unlock defers to the master password while a recovery record exists, and
+  the login screen reports whether the new or the previous password is required.
+- Serialised whole-vault writes with a per-storage-key async mutex. Two mutations that
+  start from the same in-memory snapshot (for example an import while a save is in flight,
+  or autosave plus a manual save) can no longer overwrite each other; a queued mutation
+  always rebuilds from the committed cache and re-encryptions run before any concurrent
+  write on the same collection.
 
 ## [3.3.1] - 2026-08-09
 

docs/architecture.md

@@ -109,11 +109,16 @@ A top-level error boundary (`src/components/ErrorBoundary.tsx`) wraps the naviga
 3. Biometric login asks SecureStore for the stored biometric vault key with device authentication, verifies it against the same master-key verifier, and then loads it into memory.
 4. `StorageService` initializes the encrypted vault cache with the active key.
 5. `AuthContext` completes the shared post-login pipeline: authentication state, login notification, post-auth migration, notification settings, clipboard timeout, and auto-lock timeout.
-6. PIN setup, PIN rotation, and transparent KDF upgrades treat ciphertext and verifier
-   metadata as one authenticated transaction. If rollback also fails, the session and
-   in-memory key are cleared instead of continuing against uncertain storage.
-7. Password and note writes are serialized, encrypted with KS1, persisted to AsyncStorage,
-   and only then committed to the decrypted in-memory cache.
+6. PIN setup, PIN rotation, and transparent KDF upgrades write a pending recovery record
+   before touching the vault, re-encrypt the vault in one batched write, and commit the new
+   verifier metadata with a single SecureStore write. A process kill between the vault write
+   and the metadata commit is reconciled on the next master-password login; biometric unlock
+   waits for that login. If rollback also fails, the session and in-memory key are cleared
+   instead of continuing against uncertain storage, and the recovery record is kept so the
+   next login can complete the promotion.
+7. Whole-collection writes are queued per storage key, encrypted with KS1, persisted to
+   AsyncStorage, and only then committed to the decrypted in-memory cache, so two concurrent
+   mutations cannot overwrite each other from the same snapshot.
 8. Logout and auto-lock clear authentication state and the in-memory key.
 
 ## Storage Boundaries

docs/security.md

@@ -42,7 +42,18 @@ Derived keys are normalized to 64-character lowercase hex strings. Invalid deriv
 
 ### Transparent KDF upgrade on login
 
-Vaults created with legacy parameters — either PBKDF2 (`memory = 0`) or the old heavy Argon2id (`64 MiB / t = 3`) — are transparently upgraded to the current Argon2id parameters on the next successful password login, while the vault is decrypted in memory. The upgrade re-derives a new key (new salt and verifier), precomputes both encrypted collections, persists them in one batched storage call, and uses explicit rollback if the following verifier update fails. Recoverable failures restore the previous working key and retry on the next login. If both verifier persistence and rollback fail, Keysoft closes the authenticated session and clears the in-memory key rather than allowing further writes against uncertain storage. Biometric logins do not trigger the upgrade (no password is available); the next password login performs it.
+Vaults created with legacy parameters — either PBKDF2 (`memory = 0`) or the old heavy Argon2id (`64 MiB / t = 3`) — are transparently upgraded to the current Argon2id parameters on the next successful password login, while the vault is decrypted in memory. The same crash-safe sequence is used when the master password changes:
+
+1. A pending recovery record containing the new KDF metadata (salt, iterations, memory, verifier) is written to SecureStore while the active metadata still describes the previous key.
+2. Both encrypted collections are re-encrypted and persisted in one batched AsyncStorage write.
+3. The new metadata is committed with a single SecureStore write, which is the atomic switch point.
+4. The recovery record is removed.
+
+A process kill between the vault write and the metadata commit would previously leave ciphertext encrypted with a key whose salt was never stored — permanent vault loss. With the recovery record, the next master-password login derives both candidates, inspects which key decrypts the stored collections, and either promotes the pending metadata (vault already re-encrypted) or discards the record and keeps the previous password working (vault not re-encrypted yet). If the password only matches metadata whose key no longer decrypts the vault, login fails with a diagnostic naming the password that is required and keeps the recovery record untouched. A crash after the metadata commit leaves only a stale record, which the next login removes. Biometric unlock cannot derive the pending key: it refuses to run while a record exists and does not disable biometrics, and the following password login refreshes the biometric key best-effort.
+
+Recoverable failures during the regular flow restore the previous working key and retry on a later login. If both the metadata commit and the rollback fail, Keysoft closes the authenticated session and clears the in-memory key rather than allowing further writes against uncertain storage; the recovery record is kept so the next password login can complete the promotion instead of losing the vault. The recovery record holds no key material, only the same class of KDF metadata already stored in SecureStore.
+
+This safety relies on two platform guarantees: a single SecureStore key write is atomic, and `AsyncStorage.multiSet` persists both collections in one transaction on Android (the supported release platform). The residual risk is the same as any password change: once the vault has been re-encrypted, only the new password can open it, so a user who forgets the new password after a crash cannot fall back to the previous one.
 
 When creating or changing the PIN, the verifier metadata and the vault key are produced from the same KDF result. This avoids a duplicate KDF pass for the same new PIN while preserving the configured KDF cost and the 64-character derived-key requirement.
 
@@ -71,6 +82,8 @@ Vault data uses the KS1 format:
 
 Decryption verifies the MAC before trusting plaintext output.
 
+Each collection is stored as one whole encrypted record. Mutations rebuild that record from the decrypted in-memory cache, so writes are queued per storage key: two concurrent mutations cannot both start from the same snapshot and silently drop each other's record, and a re-encryption of the whole vault excludes concurrent saves on the collections it rewrites.
+
 ### Key separation
 
 KS1 derives a single 256-bit key and uses that same value both as the AES-256-CBC

src/__tests__/services/AuthService.test.ts

@@ -60,6 +60,10 @@ describe('AuthService', () => {
 
     (StorageService.saveMasterKeyInfo as jest.Mock).mockResolvedValue(undefined);
     (StorageService.deleteMasterKeyInfo as jest.Mock).mockResolvedValue(undefined);
+    (StorageService.savePendingMasterKeyMigration as jest.Mock).mockResolvedValue(undefined);
+    (StorageService.getPendingMasterKeyMigration as jest.Mock).mockResolvedValue(null);
+    (StorageService.deletePendingMasterKeyMigration as jest.Mock).mockResolvedValue(undefined);
+    (StorageService.canDecryptVaultWithKey as jest.Mock).mockResolvedValue(true);
     (StorageService.setEncryptionKey as jest.Mock).mockImplementation(() => {});
     (StorageService.initDatabase as jest.Mock).mockResolvedValue(undefined);
     (StorageService.getMasterKeyInfo as jest.Mock).mockResolvedValue(mockMasterKeyInfo);
@@ -589,6 +593,115 @@ describe('AuthService', () => {
     });
   });
 
+  describe('Crash-safe KDF transaction', () => {
+    const newInfo: UserMasterKey = {
+      salt: 'new-salt',
+      iterations: 2,
+      memory: 19456,
+      verifier: 'new-verifier',
+    };
+    const newKey = 'aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899';
+
+    beforeEach(() => {
+      (CryptoService.createMasterKeyInfoWithDerivedKey as jest.Mock).mockResolvedValue({
+        masterKeyInfo: newInfo,
+        derivedKey: newKey,
+      });
+      (StorageService.getMasterKeyInfo as jest.Mock).mockResolvedValue(mockMasterKeyInfo);
+      (StorageService.getEncryptionKey as jest.Mock).mockReturnValue(mockEncryptionKey);
+    });
+
+    it('writes the recovery record before the vault and commits metadata last', async () => {
+      (CryptoService.isNativeKdfAvailable as jest.Mock).mockReturnValue(true);
+      (CryptoService.isLegacyKdf as jest.Mock).mockReturnValue(true);
+
+      const callOrder: string[] = [];
+      (StorageService.savePendingMasterKeyMigration as jest.Mock).mockImplementation(async () => {
+        callOrder.push('write-recovery-record');
+      });
+      (StorageService.reEncryptAllData as jest.Mock).mockImplementation(async () => {
+        callOrder.push('vault-write');
+      });
+      (StorageService.saveMasterKeyInfo as jest.Mock).mockImplementation(async () => {
+        callOrder.push('metadata-commit');
+      });
+      (StorageService.deletePendingMasterKeyMigration as jest.Mock).mockImplementation(async () => {
+        callOrder.push('clear-recovery-record');
+      });
+
+      await expect(AuthService.loginWithMasterPassword(mockMasterPassword)).resolves.toBe(true);
+
+      expect(callOrder).toEqual([
+        'write-recovery-record',
+        'vault-write',
+        'metadata-commit',
+        'clear-recovery-record',
+      ]);
+      expect(StorageService.savePendingMasterKeyMigration).toHaveBeenCalledWith(
+        expect.objectContaining({ masterKeyInfo: newInfo }),
+      );
+    });
+
+    it('keeps the recovery record when the metadata commit and the rollback both fail', async () => {
+      (CryptoService.isNativeKdfAvailable as jest.Mock).mockReturnValue(true);
+      (CryptoService.isLegacyKdf as jest.Mock).mockReturnValue(true);
+      (StorageService.reEncryptAllData as jest.Mock)
+        .mockResolvedValueOnce(undefined)
+        .mockRejectedValueOnce(new Error('rollback failed'));
+      (StorageService.saveMasterKeyInfo as jest.Mock).mockRejectedValueOnce(
+        new Error('SecureStore unavailable'),
+      );
+
+      await expect(AuthService.loginWithMasterPassword(mockMasterPassword)).resolves.toBe(false);
+
+      expect(StorageService.savePendingMasterKeyMigration).toHaveBeenCalled();
+      expect(StorageService.deletePendingMasterKeyMigration).not.toHaveBeenCalled();
+      expect(AuthService.getLastAuthFailure()).toEqual(
+        expect.objectContaining({ reason: 'kdf_upgrade_rollback_failed' }),
+      );
+    });
+
+    it('writes the recovery record when rotating the master password', async () => {
+      await AuthService.loginWithMasterPassword(mockMasterPassword);
+      (CryptoService.isLegacyKdf as jest.Mock).mockReturnValue(false);
+
+      await expect(AuthService.updateMasterPassword('new-password')).resolves.toBe(true);
+
+      expect(StorageService.savePendingMasterKeyMigration).toHaveBeenCalledWith(
+        expect.objectContaining({ masterKeyInfo: newInfo }),
+      );
+      const pendingWriteOrder = (StorageService.savePendingMasterKeyMigration as jest.Mock).mock
+        .invocationCallOrder[0];
+      const vaultWriteOrder = (StorageService.reEncryptAllData as jest.Mock).mock
+        .invocationCallOrder[0];
+      const metadataOrder = (StorageService.saveMasterKeyInfo as jest.Mock).mock
+        .invocationCallOrder[0];
+      const pendingClearOrders = (StorageService.deletePendingMasterKeyMigration as jest.Mock).mock
+        .invocationCallOrder;
+      expect(pendingWriteOrder).toBeLessThan(vaultWriteOrder);
+      expect(vaultWriteOrder).toBeLessThan(metadataOrder);
+      expect(Math.max(...pendingClearOrders)).toBeGreaterThan(metadataOrder);
+    });
+
+    it('refuses biometric login while a KDF recovery record is pending', async () => {
+      (StorageService.getPendingMasterKeyMigration as jest.Mock).mockResolvedValue({
+        masterKeyInfo: newInfo,
+        startedAt: 1,
+      });
+      (StorageService.getUserPreferences as jest.Mock).mockResolvedValue({
+        biometricsEnabled: true,
+      });
+
+      await expect(AuthService.loginWithBiometrics()).resolves.toBe(false);
+
+      expect(StorageService.getBiometricKey).not.toHaveBeenCalled();
+      expect(StorageService.deleteBiometricKey).not.toHaveBeenCalled();
+      expect(AuthService.getLastAuthFailure()).toEqual(
+        expect.objectContaining({ reason: 'kdf_recovery_pending' }),
+      );
+    });
+  });
+
   describe('verifyMasterPassword', () => {
     it('should return true for correct password without modifying auth state', async () => {
       // beforeEach -> logout calls setEncryptionKey(''). Reset mocks so