From 9f185ffaf586e509428e2f8f4b34159ad6015e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joshua=20Kr=C3=BCger?= Date: Mon, 1 Jun 2026 13:57:03 +0200 Subject: [PATCH 1/4] fix(wallet): fully purge seed + key on wallet delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user-facing 'Delete Wallet' only cleared walletAccountInfos via WalletStorage.deleteWallet — the walletInfos row (the AES-GCM-encrypted seed) and the mnemonic encryption key in secure storage both survived, leaving the full mnemonic recoverable after a delete (resale / right-to-erasure risk). Add a distinct purge primitive used only by the user-facing delete: - WalletStorage.deleteWalletCompletely (accounts + the seed row, in a tx) - SecureStorage.deleteMnemonicKey - WalletRepository.purgeWallet = deleteWalletCompletely + deleteMnemonicKey - WalletService.deleteCurrentWallet now calls purgeWallet The account-only deleteWallet is left untouched for the onboarding-regenerate flow (which relies on the seed row surviving). Tests: purgeWallet removes the row AND the key; deleteWallet still leaves both; the service delete now verifies purgeWallet (never deleteWallet). Refs #612 (S2) --- .../repository/wallet_repository.dart | 8 +++++ lib/packages/service/wallet_service.dart | 4 ++- lib/packages/storage/secure_storage.dart | 6 ++++ lib/packages/storage/wallet_storage.dart | 10 +++++++ .../repository/wallet_repository_test.dart | 29 +++++++++++++++++++ .../packages/service/wallet_service_test.dart | 6 +++- 6 files changed, 61 insertions(+), 2 deletions(-) diff --git a/lib/packages/repository/wallet_repository.dart b/lib/packages/repository/wallet_repository.dart index 13b9d582..1b00710e 100644 --- a/lib/packages/repository/wallet_repository.dart +++ b/lib/packages/repository/wallet_repository.dart @@ -44,6 +44,14 @@ class WalletRepository { Future deleteWallet(int id) => _appDatabase.deleteWallet(id); + /// Full purge for the user-facing delete: removes the encrypted-seed row AND + /// the AES-GCM mnemonic key, so no recoverable seed material remains on + /// device. [deleteWallet] (account-only) is preserved for onboarding regen. + Future purgeWallet(int id) async { + await _appDatabase.deleteWalletCompletely(id); + await _secureStorage.deleteMnemonicKey(); + } + Future _decryptWalletInfo(WalletInfo info) async { final key = await _secureStorage.getOrCreateMnemonicKey(); final decryptedSeed = SecureStorage.decryptSeed(key, info.seed); diff --git a/lib/packages/service/wallet_service.dart b/lib/packages/service/wallet_service.dart index 02322e93..052546b5 100644 --- a/lib/packages/service/wallet_service.dart +++ b/lib/packages/service/wallet_service.dart @@ -274,7 +274,9 @@ class WalletService { Future deleteCurrentWallet() async { final id = _settingsRepository.currentWalletId!; - await _repository.deleteWallet(id); + // Full purge (seed row + mnemonic key), not the account-only deleteWallet + // used by onboarding-regenerate — so no recoverable seed survives delete. + await _repository.purgeWallet(id); await _settingsRepository.removeCurrentWalletId(); } diff --git a/lib/packages/storage/secure_storage.dart b/lib/packages/storage/secure_storage.dart index 8472bae4..d89412ef 100644 --- a/lib/packages/storage/secure_storage.dart +++ b/lib/packages/storage/secure_storage.dart @@ -165,6 +165,12 @@ class SecureStorage { return key; } + /// Removes the AES-GCM key that decrypts stored seeds. Call only from the + /// user-facing wallet delete (purge) — once gone, any surviving encrypted + /// seed is permanently undecryptable. The onboarding-regenerate account-only + /// delete must NOT call this. A fresh key is lazily minted on next creation. + Future deleteMnemonicKey() => _secureStorage.delete(key: _mnemonicEncryptionKey); + static String encryptSeed(Uint8List key, String plaintext) { final iv = _secureRandomBytes(12); final cipher = GCMBlockCipher(AESEngine()) diff --git a/lib/packages/storage/wallet_storage.dart b/lib/packages/storage/wallet_storage.dart index 5eb8c83a..d0882603 100644 --- a/lib/packages/storage/wallet_storage.dart +++ b/lib/packages/storage/wallet_storage.dart @@ -28,6 +28,16 @@ extension WalletStorage on AppDatabase { Future deleteWallet(int walletId) => (delete(walletAccountInfos)..where((row) => row.wallet.equals(walletId))).go(); + /// Deletes the `walletInfos` row itself (the encrypted-seed record) after + /// clearing its dependent `walletAccountInfos` rows (FK in + /// [WalletAccountInfos.wallet]). Distinct from [deleteWallet], which clears + /// only accounts and deliberately leaves the seed row for the + /// onboarding-regenerate flow. + Future deleteWalletCompletely(int walletId) => transaction(() async { + await (delete(walletAccountInfos)..where((row) => row.wallet.equals(walletId))).go(); + await (delete(walletInfos)..where((row) => row.id.equals(walletId))).go(); + }); + Future get hasWallet => select(walletInfos).get().then((result) => result.isNotEmpty); } diff --git a/test/packages/repository/wallet_repository_test.dart b/test/packages/repository/wallet_repository_test.dart index f049bca4..f0cb7f54 100644 --- a/test/packages/repository/wallet_repository_test.dart +++ b/test/packages/repository/wallet_repository_test.dart @@ -145,5 +145,34 @@ void main() { final afterAccounts = await db.getWalletAccounts(walletId); expect(afterAccounts, isEmpty); }); + + test('purgeWallet removes the walletInfos seed row AND the mnemonic key', () async { + // Regression for #612 S2: the user-facing delete must leave no + // recoverable seed material — neither the encrypted row nor the AES key. + when(() => secureStorage.deleteMnemonicKey()).thenAnswer((_) async {}); + + final walletId = await repo.createWallet(walletName, WalletType.software, seed, address); + await db.insertWalletAccount(walletId, 'acc-0', 0); + expect(await db.getWalletById(walletId), isNotNull); + + await repo.purgeWallet(walletId); + + expect(await db.getWalletById(walletId), isNull); // encrypted seed row gone + expect(await db.getWalletAccounts(walletId), isEmpty); // accounts gone + verify(() => secureStorage.deleteMnemonicKey()).called(1); // AES key removed + }); + + test('deleteWallet (account-only) leaves the seed row and mnemonic key intact', () async { + // Onboarding-regenerate contract: the account-only primitive must NOT + // wipe the seed row or the AES key. + final walletId = await repo.createWallet(walletName, WalletType.software, seed, address); + await db.insertWalletAccount(walletId, 'acc-0', 0); + + await repo.deleteWallet(walletId); + + expect(await db.getWalletById(walletId), isNotNull); // row survives + expect(await db.getWalletAccounts(walletId), isEmpty); // accounts gone + verifyNever(() => secureStorage.deleteMnemonicKey()); // key untouched + }); }); } diff --git a/test/packages/service/wallet_service_test.dart b/test/packages/service/wallet_service_test.dart index 91ea64d9..27175999 100644 --- a/test/packages/service/wallet_service_test.dart +++ b/test/packages/service/wallet_service_test.dart @@ -57,6 +57,7 @@ void main() { when(() => settings.saveCurrentWalletId(any())).thenAnswer((_) async => true); when(() => settings.removeCurrentWalletId()).thenAnswer((_) async => true); when(() => repo.deleteWallet(any())).thenAnswer((_) async {}); + when(() => repo.purgeWallet(any())).thenAnswer((_) async {}); when(() => repo.updateAddress(any(), any())).thenAnswer((_) async {}); }); @@ -440,7 +441,10 @@ void main() { await service.deleteCurrentWallet(); - verify(() => repo.deleteWallet(8)).called(1); + // User-facing delete must fully purge (seed row + mnemonic key), not the + // account-only deleteWallet used by onboarding-regenerate (#612 S2). + verify(() => repo.purgeWallet(8)).called(1); + verifyNever(() => repo.deleteWallet(any())); verify(() => settings.removeCurrentWalletId()).called(1); }); }); From d1efe4bffeb943852641dc1d245e43197b6fbe8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joshua=20Kr=C3=BCger?= Date: Mon, 1 Jun 2026 14:46:17 +0200 Subject: [PATCH 2/4] test(storage): cover SecureStorage.deleteMnemonicKey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repository test mocks SecureStorage, so deleteMnemonicKey's real body was never executed → scoped line coverage fell to 99.9% and the 100% Coverage Floor Gate failed. Add a direct SecureStorage unit test that invokes it. --- test/packages/storage/secure_storage_test.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/packages/storage/secure_storage_test.dart b/test/packages/storage/secure_storage_test.dart index 19508ca6..96df3098 100644 --- a/test/packages/storage/secure_storage_test.dart +++ b/test/packages/storage/secure_storage_test.dart @@ -120,6 +120,12 @@ void main() { verify(() => mockStorage.delete(key: 'pin.salt')).called(1); }); + test('deleteMnemonicKey deletes the mnemonic encryption key', () async { + await secureStorage.deleteMnemonicKey(); + + verify(() => mockStorage.delete(key: 'wallet.mnemonic.encryption.key')).called(1); + }); + test('getPinSalt returns null when no salt is stored', () async { when( () => mockStorage.read(key: 'pin.salt'), From 99b7cc8739db139974f0cbaaf9069a81741352c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joshua=20Kr=C3=BCger?= Date: Mon, 1 Jun 2026 15:27:46 +0200 Subject: [PATCH 3/4] fix(wallet): drop issue refs and caller/sibling-method refs from delete-purge comments --- lib/packages/repository/wallet_repository.dart | 2 +- lib/packages/service/wallet_service.dart | 4 ++-- lib/packages/storage/secure_storage.dart | 7 +++---- lib/packages/storage/wallet_storage.dart | 4 +--- test/packages/repository/wallet_repository_test.dart | 4 ++-- test/packages/service/wallet_service_test.dart | 4 ++-- 6 files changed, 11 insertions(+), 14 deletions(-) diff --git a/lib/packages/repository/wallet_repository.dart b/lib/packages/repository/wallet_repository.dart index 1b00710e..e2ffbce7 100644 --- a/lib/packages/repository/wallet_repository.dart +++ b/lib/packages/repository/wallet_repository.dart @@ -46,7 +46,7 @@ class WalletRepository { /// Full purge for the user-facing delete: removes the encrypted-seed row AND /// the AES-GCM mnemonic key, so no recoverable seed material remains on - /// device. [deleteWallet] (account-only) is preserved for onboarding regen. + /// device. Future purgeWallet(int id) async { await _appDatabase.deleteWalletCompletely(id); await _secureStorage.deleteMnemonicKey(); diff --git a/lib/packages/service/wallet_service.dart b/lib/packages/service/wallet_service.dart index 052546b5..a0a0fa70 100644 --- a/lib/packages/service/wallet_service.dart +++ b/lib/packages/service/wallet_service.dart @@ -274,8 +274,8 @@ class WalletService { Future deleteCurrentWallet() async { final id = _settingsRepository.currentWalletId!; - // Full purge (seed row + mnemonic key), not the account-only deleteWallet - // used by onboarding-regenerate — so no recoverable seed survives delete. + // Full purge (seed row + mnemonic key), not an account-only delete — so no + // recoverable seed survives delete. await _repository.purgeWallet(id); await _settingsRepository.removeCurrentWalletId(); } diff --git a/lib/packages/storage/secure_storage.dart b/lib/packages/storage/secure_storage.dart index d89412ef..3f38fd1b 100644 --- a/lib/packages/storage/secure_storage.dart +++ b/lib/packages/storage/secure_storage.dart @@ -165,10 +165,9 @@ class SecureStorage { return key; } - /// Removes the AES-GCM key that decrypts stored seeds. Call only from the - /// user-facing wallet delete (purge) — once gone, any surviving encrypted - /// seed is permanently undecryptable. The onboarding-regenerate account-only - /// delete must NOT call this. A fresh key is lazily minted on next creation. + /// Removes the AES-GCM key that decrypts stored seeds. Once gone, any + /// surviving encrypted seed is permanently undecryptable; a fresh key is + /// lazily minted on next creation. Future deleteMnemonicKey() => _secureStorage.delete(key: _mnemonicEncryptionKey); static String encryptSeed(Uint8List key, String plaintext) { diff --git a/lib/packages/storage/wallet_storage.dart b/lib/packages/storage/wallet_storage.dart index d0882603..81317d76 100644 --- a/lib/packages/storage/wallet_storage.dart +++ b/lib/packages/storage/wallet_storage.dart @@ -30,9 +30,7 @@ extension WalletStorage on AppDatabase { /// Deletes the `walletInfos` row itself (the encrypted-seed record) after /// clearing its dependent `walletAccountInfos` rows (FK in - /// [WalletAccountInfos.wallet]). Distinct from [deleteWallet], which clears - /// only accounts and deliberately leaves the seed row for the - /// onboarding-regenerate flow. + /// [WalletAccountInfos.wallet]). Future deleteWalletCompletely(int walletId) => transaction(() async { await (delete(walletAccountInfos)..where((row) => row.wallet.equals(walletId))).go(); await (delete(walletInfos)..where((row) => row.id.equals(walletId))).go(); diff --git a/test/packages/repository/wallet_repository_test.dart b/test/packages/repository/wallet_repository_test.dart index f0cb7f54..b9ab2ded 100644 --- a/test/packages/repository/wallet_repository_test.dart +++ b/test/packages/repository/wallet_repository_test.dart @@ -147,8 +147,8 @@ void main() { }); test('purgeWallet removes the walletInfos seed row AND the mnemonic key', () async { - // Regression for #612 S2: the user-facing delete must leave no - // recoverable seed material — neither the encrypted row nor the AES key. + // The user-facing delete must leave no recoverable seed material — + // neither the encrypted row nor the AES key. when(() => secureStorage.deleteMnemonicKey()).thenAnswer((_) async {}); final walletId = await repo.createWallet(walletName, WalletType.software, seed, address); diff --git a/test/packages/service/wallet_service_test.dart b/test/packages/service/wallet_service_test.dart index 27175999..24f19cfa 100644 --- a/test/packages/service/wallet_service_test.dart +++ b/test/packages/service/wallet_service_test.dart @@ -441,8 +441,8 @@ void main() { await service.deleteCurrentWallet(); - // User-facing delete must fully purge (seed row + mnemonic key), not the - // account-only deleteWallet used by onboarding-regenerate (#612 S2). + // User-facing delete must fully purge (seed row + mnemonic key), not + // the account-only delete. verify(() => repo.purgeWallet(8)).called(1); verifyNever(() => repo.deleteWallet(any())); verify(() => settings.removeCurrentWalletId()).called(1); From d132e0ab5c1abcd44416b92a19529c87a218dd49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joshua=20Kr=C3=BCger?= Date: Mon, 1 Jun 2026 16:03:54 +0200 Subject: [PATCH 4/4] docs(storage): annotate deleteMnemonicKey platform path with @no-integration-test Per CONTRIBUTING.md, new secure-storage (platform-channel) code paths must carry the // @no-integration-test annotation while no integration_test/ dir exists; the unit test only mocks the plugin. --- lib/packages/storage/secure_storage.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/packages/storage/secure_storage.dart b/lib/packages/storage/secure_storage.dart index 3f38fd1b..55c78b27 100644 --- a/lib/packages/storage/secure_storage.dart +++ b/lib/packages/storage/secure_storage.dart @@ -168,6 +168,9 @@ class SecureStorage { /// Removes the AES-GCM key that decrypts stored seeds. Once gone, any /// surviving encrypted seed is permanently undecryptable; a fresh key is /// lazily minted on next creation. + // @no-integration-test: forwards to FlutterSecureStorage (Android Keystore / + // iOS Keychain) over a platform channel; real keystore removal is only + // verifiable on-device — the unit test mocks the plugin. Future deleteMnemonicKey() => _secureStorage.delete(key: _mnemonicEncryptionKey); static String encryptSeed(Uint8List key, String plaintext) {