diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt index 210eb3f58..5f192bc27 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt @@ -887,24 +887,20 @@ fun App( // Load account list from encrypted storage accountManager.refreshAccountListOnStartup() - if (accountManager.hasBunkerAccount()) { - // Show connecting UI while dedicated NIP-46 client connects - accountManager.setConnectingRelays() - } val result = accountManager.loadSavedAccount() if (result.isSuccess) { - // Ensure loaded account is in multi-account storage - accountManager.ensureCurrentAccountInStorage() accountManager.refreshAccountList() val current = accountManager.currentAccount() if (current?.signerType is com.vitorpamplona.amethyst.commons.model.account.SignerType.Remote) { accountManager.startHeartbeat(scope) } - } else if (accountManager.hasBunkerAccount()) { - // Corrupt bunker state — fall back to login screen - accountManager.logout(deleteKey = true) + // Load per-account NWC + if (current != null) { + accountManager.loadNwcConnection(current.npub) + } } + // If failure: state remains LoggedOut → login screen shows automatically } onDispose { @@ -965,18 +961,14 @@ fun App( val account = accountState as AccountState.LoggedIn val nwcConnection by accountManager.nwcConnection.collectAsState() - // Lazy-load Namecoin services — almost never used, no need to keep in - // memory from the start (matches Android lazy pattern) + // Lazy-load Namecoin services val namecoinPreferences = remember { DesktopNamecoinPreferences() } val namecoinService = remember { DesktopNamecoinNameService(preferencesProvider = { namecoinPreferences.current }) } - // Load NWC connection on first composition - LaunchedEffect(Unit) { - accountManager.loadNwcConnection() - } + // NWC loaded during startup in loadSavedAccount flow val currentTorStatus = torManager.status.collectAsState().value androidx.compose.runtime.CompositionLocalProvider( @@ -1498,10 +1490,7 @@ fun RelaySettingsScreen( var nwcInput by remember { mutableStateOf("") } var nwcError by remember { mutableStateOf(null) } - // Load NWC on first composition - LaunchedEffect(Unit) { - accountManager.loadNwcConnection() - } + val nwcScope = rememberCoroutineScope() com.vitorpamplona.amethyst.desktop.ui.ReadingColumn { val sidePadding = @@ -1567,7 +1556,7 @@ fun RelaySettingsScreen( ) } OutlinedButton( - onClick = { accountManager.clearNwcConnection() }, + onClick = { nwcScope.launch { accountManager.clearNwcConnection(account.npub) } }, colors = ButtonDefaults.outlinedButtonColors( contentColor = MaterialTheme.colorScheme.error, @@ -1597,11 +1586,13 @@ fun RelaySettingsScreen( ) Button( onClick = { - val result = accountManager.setNwcConnection(nwcInput) - result.fold( - onSuccess = { nwcInput = "" }, - onFailure = { nwcError = it.message ?: "Invalid connection string" }, - ) + nwcScope.launch { + val result = accountManager.setNwcConnection(account.npub, nwcInput) + result.fold( + onSuccess = { nwcInput = "" }, + onFailure = { nwcError = it.message ?: "Invalid connection string" }, + ) + } }, enabled = nwcInput.isNotBlank(), ) { diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManager.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManager.kt index 3b6b113cf..9b580b467 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManager.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManager.kt @@ -67,8 +67,6 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withTimeout import java.io.File -import java.nio.file.Files -import java.nio.file.attribute.PosixFilePermission sealed class AccountState { data object LoggedOut : AccountState() @@ -111,8 +109,15 @@ class AccountManager internal constructor( File(homeDir, ".amethyst") } + private val _storageCorruption = MutableStateFlow(null) + val storageCorruption: StateFlow = _storageCorruption.asStateFlow() + + fun clearStorageCorruption() { + _storageCorruption.value = null + } + val accountStorage: DesktopAccountStorage by lazy { - DesktopAccountStorage(secureStorage, homeDir) + DesktopAccountStorage(secureStorage, homeDir, onCorruption = { _storageCorruption.value = it }) } private val _allAccounts = MutableStateFlow>(persistentListOf()) @@ -232,16 +237,28 @@ class AccountManager internal constructor( suspend fun loadSavedAccount(): Result = try { - val lastNpub = getLastNpub() - val bunkerUri = getBunkerUri() + // Clean up legacy files (one-time, silent) + File(amethystDir, "last_account.txt").delete() + File(amethystDir, "bunker_uri.txt").delete() + File(amethystDir, "nwc_connection.txt").delete() - if (bunkerUri != null) { - loadBunkerAccount(bunkerUri, lastNpub) - } else if (lastNpub != null) { - loadInternalAccount(lastNpub) - } else { - Result.failure(Exception("No saved account")) + // Single source of truth: accounts.json.enc + val activeNpub = + accountStorage.currentAccount() + ?: return Result.failure(Exception("No saved account")) + + val accounts = accountStorage.loadAccounts() + val info = + accounts.find { it.npub == activeNpub } + ?: return Result.failure(Exception("Account not found in storage")) + + when (info.signerType) { + is SignerType.Internal -> loadInternalAccount(activeNpub) + is SignerType.Remote -> loadBunkerAccount((info.signerType as SignerType.Remote).bunkerUri, activeNpub) + is SignerType.ViewOnly -> loadReadOnlyAccount(activeNpub) } + } catch (e: kotlin.coroutines.cancellation.CancellationException) { + throw e } catch (e: Exception) { Result.failure(e) } @@ -298,7 +315,6 @@ class AccountManager internal constructor( } val resolvedNpub = npub ?: pubKeyHex.hexToByteArray().toNpub() - if (npub == null) saveLastNpub(resolvedNpub) val state = AccountState.LoggedIn( @@ -439,18 +455,14 @@ class AccountManager internal constructor( ephemeralPrivKeyHex: String, npub: String, ) { - saveLastNpub(npub) secureStorage.savePrivateKey(bunkerEphemeralKeyAlias(npub), ephemeralPrivKeyHex) - saveBunkerUri(bunkerUri) - // Also save to multi-account storage + // Save to multi-account storage (sole source of truth) val info = AccountInfo(npub = npub, signerType = SignerType.Remote(bunkerUri)) addAccountToStorage(info) accountStorage.setCurrentAccount(npub) } - fun hasBunkerAccount(): Boolean = getBunkerFile().exists() - fun setConnectingRelays() { _accountState.value = AccountState.ConnectingRelays } @@ -481,8 +493,6 @@ class AccountManager internal constructor( } } - saveLastNpub(current.npub) - // Always save to multi-account storage (including read-only) val info = AccountInfo(npub = current.npub, signerType = current.signerType) addAccountToStorage(info) @@ -564,23 +574,30 @@ class AccountManager internal constructor( // Clean up remote signer if bunker account if (current.signerType is SignerType.Remote) { (current.signer as? NostrSignerRemote)?.closeSubscription() - if (deleteKey) { - try { - secureStorage.deletePrivateKey(bunkerEphemeralKeyAlias(current.npub)) - } catch (_: SecureStorageException) { - } - getBunkerFile().delete() - } } if (deleteKey) { + // Remove ALL credentials for this account try { secureStorage.deletePrivateKey(current.npub) - clearLastNpub() } catch (_: SecureStorageException) { } + try { + secureStorage.deletePrivateKey(bunkerEphemeralKeyAlias(current.npub)) + } catch (_: SecureStorageException) { + } + try { + secureStorage.deletePrivateKey(nwcKeyAlias(current.npub)) + } catch (_: SecureStorageException) { + } + // Remove from metadata — fixes ghost accounts + try { + accountStorage.deleteAccount(current.npub) + } catch (_: Exception) { + } } } disconnectNip46Client() + _nwcConnection.value = null _signerConnectionState.value = SignerConnectionState.NotRemote _lastPingTimeSec.value = null _accountState.value = AccountState.LoggedOut @@ -708,6 +725,10 @@ class AccountManager internal constructor( secureStorage.deletePrivateKey(bunkerEphemeralKeyAlias(npub)) } catch (_: SecureStorageException) { } + try { + secureStorage.deletePrivateKey(nwcKeyAlias(npub)) + } catch (_: SecureStorageException) { + } refreshAccountList() @@ -758,7 +779,7 @@ class AccountManager internal constructor( } // Reload NWC for the new account - loadNwcConnection() + loadNwcConnection(targetNpub) return newState } @@ -785,91 +806,62 @@ class AccountManager internal constructor( fun currentAccount(): AccountState.LoggedIn? = _accountState.value as? AccountState.LoggedIn - // --- NWC --- + // --- NWC (per-account, secret in keychain) --- + + private fun nwcKeyAlias(npub: String) = "nwc_$npub" fun hasNwcSetup(): Boolean = _nwcConnection.value != null - fun setNwcConnection(uri: String): Result = + suspend fun setNwcConnection( + npub: String, + uri: String, + ): Result = try { val parsed = Nip47WalletConnect.parse(uri) + if (parsed.secret == null) throw IllegalArgumentException("NWC URI has no secret") + + // Store entire URI in keychain (not plaintext file) + secureStorage.savePrivateKey(nwcKeyAlias(npub), uri) + _nwcConnection.value = parsed - saveNwcUri(uri) Result.success(parsed) + } catch (e: kotlin.coroutines.cancellation.CancellationException) { + throw e } catch (e: Exception) { Result.failure(e) } - fun clearNwcConnection() { - _nwcConnection.value = null - getNwcFile().delete() - } - - fun loadNwcConnection() { - val uri = getNwcFile().takeIf { it.exists() }?.readText()?.trim() - if (!uri.isNullOrEmpty()) { - try { - _nwcConnection.value = Nip47WalletConnect.parse(uri) - } catch (_: Exception) { - getNwcFile().delete() - } - } - } - - // --- File storage helpers --- - - private fun ensureAmethystDir() { - if (!amethystDir.exists()) { - amethystDir.mkdirs() - } + suspend fun clearNwcConnection(npub: String) { try { - Files.setPosixFilePermissions( - amethystDir.toPath(), - setOf( - PosixFilePermission.OWNER_READ, - PosixFilePermission.OWNER_WRITE, - PosixFilePermission.OWNER_EXECUTE, - ), - ) - } catch (_: UnsupportedOperationException) { - // Windows — file system ACLs handle this + secureStorage.deletePrivateKey(nwcKeyAlias(npub)) + } catch (_: SecureStorageException) { + } + _nwcConnection.value = null + } + + suspend fun loadNwcConnection(npub: String) { + val secret = + try { + secureStorage.getPrivateKey(nwcKeyAlias(npub)) + } catch (_: SecureStorageException) { + null + } + + if (secret == null) { + _nwcConnection.value = null + return + } + + // Reconstruct Nip47URINorm from the NWC URI stored in the keychain. + // The secret is the full NWC URI in hex form. + // For simplicity, the entire NWC URI is stored in keychain (not just the secret). + // This avoids needing to persist pubkey+relay separately. + try { + _nwcConnection.value = Nip47WalletConnect.parse(secret) } catch (_: Exception) { + _nwcConnection.value = null } } - - private fun saveNwcUri(uri: String) { - ensureAmethystDir() - getNwcFile().writeText(uri) - } - - private fun getNwcFile(): File = File(amethystDir, "nwc_connection.txt") - - private fun getLastNpub(): String? { - val file = getPrefsFile() - return if (file.exists()) file.readText().trim().takeIf { it.isNotEmpty() } else null - } - - private fun saveLastNpub(npub: String) { - ensureAmethystDir() - getPrefsFile().writeText(npub) - } - - private fun clearLastNpub() { - getPrefsFile().delete() - } - - private fun getPrefsFile(): File = File(amethystDir, "last_account.txt") - - private fun getBunkerUri(): String? { - val file = getBunkerFile() - return if (file.exists()) file.readText().trim().takeIf { it.isNotEmpty() } else null - } - - private fun saveBunkerUri(uri: String) { - ensureAmethystDir() - getBunkerFile().writeText(uri) - } - - private fun getBunkerFile(): File = File(amethystDir, "bunker_uri.txt") } internal fun parseBunkerRelays(uri: String): Set { diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt index cf7aab829..40c32e2ad 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt @@ -47,9 +47,22 @@ import javax.crypto.spec.SecretKeySpec * * File: ~/.amethyst/accounts.json.enc */ +sealed class StorageCorruption( + val backupPath: String?, +) { + class FileCorrupted( + backupPath: String?, + ) : StorageCorruption(backupPath) + + class JsonMalformed( + backupPath: String?, + ) : StorageCorruption(backupPath) +} + class DesktopAccountStorage( private val secureStorage: SecureKeyStorage, private val homeDir: File = File(System.getProperty("user.home")), + private val onCorruption: (StorageCorruption) -> Unit = {}, ) : AccountStorage { companion object { private const val METADATA_KEY_ALIAS = "account-metadata-key" @@ -115,16 +128,50 @@ class DesktopAccountStorage( val file = getAccountsFile() if (!file.exists()) return AccountMetadata() + val encrypted = file.readBytes() + if (encrypted.size < GCM_IV_SIZE) { + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.FileCorrupted(backup)) + return AccountMetadata() + } + return try { - val encrypted = file.readBytes() val decrypted = decrypt(encrypted) mapper.readValue(decrypted) + } catch (e: javax.crypto.AEADBadTagException) { + Log.e("DesktopAccountStorage", "GCM auth tag mismatch — file corrupted or key lost", e) + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.FileCorrupted(backup)) + AccountMetadata() + } catch (e: javax.crypto.BadPaddingException) { + Log.e("DesktopAccountStorage", "Decryption failed — file corrupted", e) + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.FileCorrupted(backup)) + AccountMetadata() + } catch (e: com.fasterxml.jackson.core.JacksonException) { + Log.e("DesktopAccountStorage", "JSON malformed after decryption", e) + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.JsonMalformed(backup)) + AccountMetadata() } catch (e: Exception) { Log.e("DesktopAccountStorage", "Failed to read accounts metadata", e) + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.FileCorrupted(backup)) AccountMetadata() } } + private fun backupCorruptFile(file: File): String? = + try { + val backup = File(file.parent, "accounts.json.enc.corrupt.${System.currentTimeMillis()}") + java.nio.file.Files + .copy(file.toPath(), backup.toPath()) + file.delete() + backup.absolutePath + } catch (_: Exception) { + null + } + private suspend fun writeMetadataToDisk(metadata: AccountMetadata) { ensureDir() val json = mapper.writeValueAsBytes(metadata) @@ -226,6 +273,8 @@ internal data class AccountInfoDto( val bunkerUri: String? = null, val displayName: String? = null, val isTransient: Boolean = false, + val nwcPubKey: String? = null, // NWC wallet service pubkey (non-secret) + val nwcRelay: String? = null, // NWC wallet relay URL (non-secret) ) { fun toAccountInfo(): AccountInfo = AccountInfo( diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerBunkerLoginTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerBunkerLoginTest.kt index 2df73bf8e..9bf1d6dbb 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerBunkerLoginTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerBunkerLoginTest.kt @@ -27,7 +27,6 @@ import kotlin.io.path.createTempDirectory import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test -import kotlin.test.assertFalse import kotlin.test.assertTrue class AccountManagerBunkerLoginTest { @@ -50,17 +49,6 @@ class AccountManagerBunkerLoginTest { tempDir.deleteRecursively() } - @Test - fun hasBunkerAccountReturnsFalseWhenNoFile() { - assertFalse(manager.hasBunkerAccount()) - } - - @Test - fun hasBunkerAccountReturnsTrueWhenFileExists() { - File(amethystDir, "bunker_uri.txt").writeText("bunker://${"a".repeat(64)}?relay=wss://r.com") - assertTrue(manager.hasBunkerAccount()) - } - @Test fun setConnectingRelaysUpdatesState() { manager.setConnectingRelays() diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt index 7038f2268..3d7f09763 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.amethyst.desktop.account import com.vitorpamplona.amethyst.commons.keystorage.SecureKeyStorage +import com.vitorpamplona.amethyst.commons.model.account.AccountInfo import com.vitorpamplona.amethyst.commons.model.account.SignerType import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair @@ -45,6 +46,8 @@ class AccountManagerLoadAccountTest { @BeforeTest fun setup() { storage = mockk(relaxed = true) + // Return null so DesktopAccountStorage generates a fresh AES key + coEvery { storage.getPrivateKey("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-load-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() @@ -57,9 +60,9 @@ class AccountManagerLoadAccountTest { } @Test - fun loadSavedAccountNoNpubReturnsFailure() = + fun loadSavedAccountNoAccountsReturnsFailure() = runTest { - // No last_account.txt file + // Empty accounts.json.enc (no accounts saved) val result = manager.loadSavedAccount() assertTrue(result.isFailure) } @@ -71,10 +74,11 @@ class AccountManagerLoadAccountTest { val npub = keyPair.pubKey.toNpub() val privKeyHex = keyPair.privKey!!.toHexKey() - // Write last_account.txt - File(amethystDir, "last_account.txt").writeText(npub) + // Save account via storage API + manager.accountStorage.saveAccount(AccountInfo(npub = npub, signerType = SignerType.Internal)) + manager.accountStorage.setCurrentAccount(npub) - // Mock storage to return the private key + // Mock keychain to return the private key coEvery { storage.getPrivateKey(npub) } returns privKeyHex val result = manager.loadSavedAccount() @@ -90,7 +94,8 @@ class AccountManagerLoadAccountTest { val keyPair = KeyPair() val npub = keyPair.pubKey.toNpub() - File(amethystDir, "last_account.txt").writeText(npub) + manager.accountStorage.saveAccount(AccountInfo(npub = npub, signerType = SignerType.Internal)) + manager.accountStorage.setCurrentAccount(npub) coEvery { storage.getPrivateKey(npub) } returns null val result = manager.loadSavedAccount() @@ -106,10 +111,13 @@ class AccountManagerLoadAccountTest { val keyPair = KeyPair() val npub = keyPair.pubKey.toNpub() - File(amethystDir, "last_account.txt").writeText(npub) - File(amethystDir, "bunker_uri.txt").writeText( - "bunker://$validHex?relay=wss://r.com", + manager.accountStorage.saveAccount( + AccountInfo(npub = npub, signerType = SignerType.Remote("bunker://$validHex?relay=wss://r.com")), ) + manager.accountStorage.setCurrentAccount(npub) + coEvery { + storage.getPrivateKey(AccountManager.bunkerEphemeralKeyAlias(npub)) + } returns null coEvery { storage.getPrivateKey(AccountManager.LEGACY_BUNKER_EPHEMERAL_KEY_ALIAS) } returns null @@ -121,23 +129,49 @@ class AccountManagerLoadAccountTest { @Test fun loadSavedAccountBunkerSuccess() = runTest { + val walletKeyPair = KeyPair() + val validHex = walletKeyPair.pubKey.toHexKey() val keyPair = KeyPair() val npub = keyPair.pubKey.toNpub() - val ephemeralKeyPair = KeyPair() - val ephemeralPrivKeyHex = ephemeralKeyPair.privKey!!.toHexKey() - val validHex = keyPair.pubKey.toHexKey() + val ephemeralKey = KeyPair() - File(amethystDir, "last_account.txt").writeText(npub) - File(amethystDir, "bunker_uri.txt").writeText( - "bunker://$validHex?relay=wss://r.com", + manager.accountStorage.saveAccount( + AccountInfo(npub = npub, signerType = SignerType.Remote("bunker://$validHex?relay=wss://relay.nsec.app")), ) + manager.accountStorage.setCurrentAccount(npub) + coEvery { - storage.getPrivateKey(AccountManager.LEGACY_BUNKER_EPHEMERAL_KEY_ALIAS) - } returns ephemeralPrivKeyHex + storage.getPrivateKey(AccountManager.bunkerEphemeralKeyAlias(npub)) + } returns ephemeralKey.privKey!!.toHexKey() val result = manager.loadSavedAccount() assertTrue(result.isSuccess) val state = result.getOrThrow() assertIs(state.signerType) } + + @Test + fun loadSavedAccountDeletesLegacyFiles() = + runTest { + val keyPair = KeyPair() + val npub = keyPair.pubKey.toNpub() + val privKeyHex = keyPair.privKey!!.toHexKey() + + // Create legacy files + File(amethystDir, "last_account.txt").writeText(npub) + File(amethystDir, "bunker_uri.txt").writeText("bunker://test") + File(amethystDir, "nwc_connection.txt").writeText("nostr+walletconnect://test") + + // Save via storage + manager.accountStorage.saveAccount(AccountInfo(npub = npub, signerType = SignerType.Internal)) + manager.accountStorage.setCurrentAccount(npub) + coEvery { storage.getPrivateKey(npub) } returns privKeyHex + + manager.loadSavedAccount() + + // Legacy files should be deleted + assertTrue(!File(amethystDir, "last_account.txt").exists()) + assertTrue(!File(amethystDir, "bunker_uri.txt").exists()) + assertTrue(!File(amethystDir, "nwc_connection.txt").exists()) + } } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt index 4956cab81..2f74a4239 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt @@ -24,6 +24,7 @@ import com.vitorpamplona.amethyst.commons.domain.nip46.SignerConnectionState import com.vitorpamplona.amethyst.commons.keystorage.SecureKeyStorage import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip19Bech32.toNsec +import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk import kotlinx.coroutines.test.runTest @@ -44,6 +45,7 @@ class AccountManagerLogoutTest { @BeforeTest fun setup() { storage = mockk(relaxed = true) + coEvery { storage.getPrivateKey("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-logout-test").toFile() manager = AccountManager(storage, tempDir) } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt index acb1603a1..f2688a281 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt @@ -55,6 +55,7 @@ class AccountManagerNip46IsolationTest { @BeforeTest fun setup() { storage = mockk(relaxed = true) + coEvery { storage.getPrivateKey("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-nip46-iso-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() @@ -69,16 +70,21 @@ class AccountManagerNip46IsolationTest { @Test fun bunkerLoadCreatesRemoteSignerInternally() = runTest { - val validHex = "a".repeat(64) + val walletKeyPair = KeyPair() + val validHex = walletKeyPair.pubKey.toHexKey() val ephemeralKeyPair = KeyPair() - File(amethystDir, "bunker_uri.txt").writeText( - "bunker://$validHex?relay=wss://relay.nsec.app", - ) - File(amethystDir, "last_account.txt").writeText( - ephemeralKeyPair.pubKey.toNpub(), + val npub = ephemeralKeyPair.pubKey.toNpub() + val bunkerUri = "bunker://$validHex?relay=wss://relay.nsec.app" + + manager.accountStorage.saveAccount( + com.vitorpamplona.amethyst.commons.model.account.AccountInfo( + npub = npub, + signerType = SignerType.Remote(bunkerUri), + ), ) + manager.accountStorage.setCurrentAccount(npub) coEvery { - storage.getPrivateKey(AccountManager.LEGACY_BUNKER_EPHEMERAL_KEY_ALIAS) + storage.getPrivateKey(AccountManager.bunkerEphemeralKeyAlias(npub)) } returns ephemeralKeyPair.privKey!!.toHexKey() val result = manager.loadSavedAccount() @@ -130,16 +136,21 @@ class AccountManagerNip46IsolationTest { @Test fun logoutResetsStateAfterBunkerLoad() = runTest { - val validHex = "a".repeat(64) + val walletKeyPair = KeyPair() + val validHex = walletKeyPair.pubKey.toHexKey() val ephemeralKeyPair = KeyPair() - File(amethystDir, "bunker_uri.txt").writeText( - "bunker://$validHex?relay=wss://relay.nsec.app", - ) - File(amethystDir, "last_account.txt").writeText( - ephemeralKeyPair.pubKey.toNpub(), + val npub = ephemeralKeyPair.pubKey.toNpub() + val bunkerUri = "bunker://$validHex?relay=wss://relay.nsec.app" + + manager.accountStorage.saveAccount( + com.vitorpamplona.amethyst.commons.model.account.AccountInfo( + npub = npub, + signerType = SignerType.Remote(bunkerUri), + ), ) + manager.accountStorage.setCurrentAccount(npub) coEvery { - storage.getPrivateKey(AccountManager.LEGACY_BUNKER_EPHEMERAL_KEY_ALIAS) + storage.getPrivateKey(AccountManager.bunkerEphemeralKeyAlias(npub)) } returns ephemeralKeyPair.privKey!!.toHexKey() manager.loadSavedAccount() @@ -155,23 +166,28 @@ class AccountManagerNip46IsolationTest { @Test fun logoutThenReloadCreatesFreshNip46Client() = runTest { - val validHex = "a".repeat(64) + val walletKeyPair = KeyPair() + val validHex = walletKeyPair.pubKey.toHexKey() val ephemeralKeyPair = KeyPair() + val npub = ephemeralKeyPair.pubKey.toNpub() val bunkerUri = "bunker://$validHex?relay=wss://relay.nsec.app" - fun writeBunkerFiles() { - File(amethystDir, "bunker_uri.txt").writeText(bunkerUri) - File(amethystDir, "last_account.txt").writeText( - ephemeralKeyPair.pubKey.toNpub(), + suspend fun setupBunkerAccount() { + manager.accountStorage.saveAccount( + com.vitorpamplona.amethyst.commons.model.account.AccountInfo( + npub = npub, + signerType = SignerType.Remote(bunkerUri), + ), ) + manager.accountStorage.setCurrentAccount(npub) } coEvery { - storage.getPrivateKey(AccountManager.LEGACY_BUNKER_EPHEMERAL_KEY_ALIAS) + storage.getPrivateKey(AccountManager.bunkerEphemeralKeyAlias(npub)) } returns ephemeralKeyPair.privKey!!.toHexKey() // First load - writeBunkerFiles() + setupBunkerAccount() manager.loadSavedAccount() val firstSigner = manager.currentAccount()?.signer assertNotNull(firstSigner) @@ -181,7 +197,7 @@ class AccountManagerNip46IsolationTest { assertIs(manager.accountState.value) // Second load — should create a fresh NIP-46 client - writeBunkerFiles() + setupBunkerAccount() manager.loadSavedAccount() val secondSigner = manager.currentAccount()?.signer assertNotNull(secondSigner) diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt index 988cbf96a..f9adf3bff 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt @@ -55,6 +55,7 @@ class AccountManagerStateTransitionTest { @BeforeTest fun setup() { storage = mockk(relaxed = true) + coEvery { storage.getPrivateKey("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-state-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() @@ -81,7 +82,13 @@ class AccountManagerStateTransitionTest { // Then load an internal account (simulating successful load) val keyPair = KeyPair() val npub = keyPair.pubKey.toNpub() - File(amethystDir, "last_account.txt").writeText(npub) + manager.accountStorage.saveAccount( + com.vitorpamplona.amethyst.commons.model.account.AccountInfo( + npub = npub, + signerType = com.vitorpamplona.amethyst.commons.model.account.SignerType.Internal, + ), + ) + manager.accountStorage.setCurrentAccount(npub) coEvery { storage.getPrivateKey(npub) } returns keyPair.privKey!!.toHexKey() manager.loadSavedAccount() diff --git a/docs/plans/2026-05-14-fix-account-security-hardening-plan.md b/docs/plans/2026-05-14-fix-account-security-hardening-plan.md new file mode 100644 index 000000000..8aea7b996 --- /dev/null +++ b/docs/plans/2026-05-14-fix-account-security-hardening-plan.md @@ -0,0 +1,462 @@ +--- +title: "fix: Account Security Hardening" +type: fix +status: active +date: 2026-05-14 +origin: docs/brainstorms/2026-05-14-fix-account-security-hardening-brainstorm.md +--- + +# Account Security Hardening + +## Deepening Insights (2026-05-14) + +1. **NWC URI reconstruction**: `Nip47URINorm` has a direct constructor `(pubKeyHex, relayUri, secret, lud16)` — no URI string parsing needed. Store pubKeyHex+relayUri in metadata, secret in keychain, reconstruct via constructor. +2. **SecureKeyStorage alias**: Accepts any arbitrary string — `"nwc_npub1..."` confirmed to work. No format validation on alias. +3. **Corruption detection**: Use `StateFlow` on `AccountManager` (matches existing `forceLogoutReason` pattern). Distinguish key-lost (`getPrivateKey("account-metadata-key")` returns null) vs file-corrupted (`AEADBadTagException`) vs JSON-malformed (`JacksonException`). +4. **NWC save failure**: Don't fall back to plaintext or in-memory — fail the connection entirely. NWC secrets control real money. +5. **Cold boot ConnectingRelays**: Must read `accounts.json.enc` first to know account type, then show ConnectingRelays UI if Remote. Can't show it before reading. +6. **ensureCurrentAccountInStorage**: No longer needed — `loadSavedAccount()` only loads from `accounts.json.enc`, so account is guaranteed to be there. +7. **Backup pattern**: Use timestamped backups (`accounts.json.enc.corrupt.`) to avoid overwriting previous backups. + +## Overview + +Single PR fixing 6 security and correctness issues in desktop account management. +Makes `accounts.json.enc` the sole source of truth, moves NWC secrets to OS keychain, +makes NWC per-account, and eliminates stale file bugs. (see brainstorm for full rationale) + +## Problem Statement + +1. **CRITICAL:** NWC wallet secret in plaintext `nwc_connection.txt` — funds can be stolen +2. **HIGH:** NWC is global — switching accounts doesn't switch wallets +3. **HIGH:** Cold boot reads stale `bunker_uri.txt` before `accounts.json.enc` — nsec misidentified as bunker +4. **MEDIUM:** `logout(deleteKey=true)` doesn't remove from `accounts.json.enc` — ghost accounts +5. **MEDIUM:** `accounts.json.enc` corruption silently returns empty — data loss +6. **LOW:** `bunker_uri.txt` is singleton — can't support multi-bunker + +## Implementation Phases + +### Phase 1: NWC Secret to Keychain + Per-Account (Issues 1, 2) + +#### 1a: Add NWC fields to AccountInfoDto + +**File:** `DesktopAccountStorage.kt:223-258` + +```kotlin +internal data class AccountInfoDto( + val npub: String, + val signerKind: String, + val bunkerUri: String? = null, + val displayName: String? = null, + val isTransient: Boolean = false, + val nwcPubKey: String? = null, // NEW: wallet service pubkey (non-secret) + val nwcRelay: String? = null, // NEW: wallet relay URL (non-secret) +) +``` + +Jackson handles new nullable fields transparently — old `accounts.json.enc` files +deserialize without them (null defaults). `AccountInfo` in commons also needs +matching fields (or `AccountInfoDto.toAccountInfo()` maps them). + +#### 1b: Rewrite NWC methods in AccountManager + +**File:** `AccountManager.kt:788-844` + +Replace `setNwcConnection()`, `clearNwcConnection()`, `loadNwcConnection()`. + +**Key insight from deepening:** `Nip47URINorm` has a direct constructor — no URI +string reconstruction needed. Store components, rebuild via constructor. + +```kotlin +// --- NWC (per-account, secret in keychain) --- + +private fun nwcKeyAlias(npub: String) = "nwc_$npub" + +fun setNwcConnection(npub: String, uri: String): Result = + try { + val parsed = Nip47WalletConnect.parse(uri) + val secret = parsed.secret ?: throw IllegalArgumentException("NWC URI has no secret") + + // Secret → keychain (fail entirely if keychain unavailable — don't fall back to plaintext) + secureStorage.savePrivateKey(nwcKeyAlias(npub), secret) + + // Non-secret parts → accounts.json.enc + scope.launch { + val info = accountStorage.loadAccounts().find { it.npub == npub } + if (info != null) { + accountStorage.saveAccount( + info.copy( + nwcPubKey = parsed.pubKeyHex, + nwcRelay = parsed.relayUri.toString(), + ), + ) + } + } + + _nwcConnection.value = parsed + Result.success(parsed) + } catch (e: Exception) { + Result.failure(e) + } + +fun clearNwcConnection(npub: String) { + try { secureStorage.deletePrivateKey(nwcKeyAlias(npub)) } catch (_: SecureStorageException) {} + scope.launch { + val info = accountStorage.loadAccounts().find { it.npub == npub } + if (info != null) { + accountStorage.saveAccount(info.copy(nwcPubKey = null, nwcRelay = null)) + } + } + _nwcConnection.value = null +} + +fun loadNwcConnection(npub: String) { + val secret = try { + secureStorage.getPrivateKey(nwcKeyAlias(npub)) + } catch (_: SecureStorageException) { null } + + if (secret != null) { + scope.launch { + val info = accountStorage.loadAccounts().find { it.npub == npub } + if (info?.nwcPubKey != null && info.nwcRelay != null) { + // Reconstruct directly — no URI parsing needed + _nwcConnection.value = Nip47WalletConnect.Nip47URINorm( + pubKeyHex = info.nwcPubKey, + relayUri = NormalizedRelayUrl(info.nwcRelay), + secret = secret, + ) + } + } + } else { + _nwcConnection.value = null + } +} +``` + +#### 1c: Delete legacy NWC file helpers + +**Remove from AccountManager.kt:** +- `saveNwcUri()` (line 839-841) +- `getNwcFile()` (line 844) + +**Delete on startup:** Add to `loadSavedAccount()`: +```kotlin +// Clean up legacy plaintext NWC file +File(amethystDir, "nwc_connection.txt").delete() +``` + +#### 1d: Update callers + +- `switchAccount()` (line 761): `loadNwcConnection()` → `loadNwcConnection(targetNpub)` +- `Main.kt`: any `loadNwcConnection()` calls → pass npub +- Wallet column: `setNwcConnection(uri)` → `setNwcConnection(npub, uri)` + +--- + +### Phase 2: Cold Boot from accounts.json.enc (Issue 3) + +#### 2a: Rewrite loadSavedAccount() + +**File:** `AccountManager.kt:233-247` + +Replace entirely: + +```kotlin +suspend fun loadSavedAccount(): Result = + try { + val activeNpub = accountStorage.currentAccount() + ?: return Result.failure(Exception("No saved account")) + + val accounts = accountStorage.loadAccounts() + val info = accounts.find { it.npub == activeNpub } + ?: return Result.failure(Exception("Account not found in storage")) + + // Clean up legacy files (one-time) + File(amethystDir, "last_account.txt").delete() + File(amethystDir, "bunker_uri.txt").delete() + File(amethystDir, "nwc_connection.txt").delete() + + when (info.signerType) { + is SignerType.Internal -> loadInternalAccount(activeNpub) + is SignerType.Remote -> loadBunkerAccount( + (info.signerType as SignerType.Remote).bunkerUri, + activeNpub, + ) + is SignerType.ViewOnly -> loadReadOnlyAccount(activeNpub) + } + } catch (e: Exception) { + Result.failure(e) + } +``` + +#### 2b: Rewrite saveCurrentAccount() + +**File:** `AccountManager.kt:460-492` + +Remove `saveLastNpub()` call (line 484). The `accountStorage.setCurrentAccount()` +call is sufficient. + +#### 2c: Remove legacy file helpers + +**Remove from AccountManager.kt:** +- `getLastNpub()` (line 846-848) +- `saveLastNpub()` (line 851-853) +- `clearLastNpub()` (line 856-857) +- `getPrefsFile()` (line 860) +- `getBunkerUri()` (line 862-864) +- `saveBunkerUri()` (line 867-869) +- `getBunkerFile()` (line 872) +- `hasBunkerAccount()` — uses `getBunkerUri()`, no longer needed + +#### 2d: Update saveBunkerAccount() + +Stop writing to `bunker_uri.txt`. The bunker URI is already saved in +`accounts.json.enc` via `AccountInfoDto.bunkerUri`. + +#### 2e: Update Main.kt startup + +**File:** `Main.kt:831-853` + +**Deepening insight:** `ensureCurrentAccountInStorage()` is no longer needed +since `loadSavedAccount()` only loads from `accounts.json.enc` (account is +guaranteed to be there). `ConnectingRelays` UI must come AFTER reading +metadata to know the account type. + +```kotlin +scope.launch(Dispatchers.IO) { + accountManager.refreshAccountListOnStartup() + + val result = accountManager.loadSavedAccount() + if (result.isSuccess) { + accountManager.refreshAccountList() + + val current = accountManager.currentAccount() + if (current?.signerType is SignerType.Remote) { + accountManager.startHeartbeat(scope) + } + // Load per-account NWC + accountManager.loadNwcConnection(current!!.npub) + } + // If failure: state remains LoggedOut → login screen shows automatically +} +``` + +Removed: +- `hasBunkerAccount()` — was based on `bunker_uri.txt` existence +- `ensureCurrentAccountInStorage()` — no longer needed +- `logout(deleteKey=true)` fallback for corrupt bunker state — corruption handling in Phase 4 covers this + +--- + +### Phase 3: Logout Cleanup (Issue 4) + +#### 3a: Fix logout(deleteKey=true) + +**File:** `AccountManager.kt:561-589` + +```kotlin +suspend fun logout(deleteKey: Boolean = false) { + val current = currentAccount() + if (current != null) { + if (current.signerType is SignerType.Remote) { + (current.signer as? NostrSignerRemote)?.closeSubscription() + } + if (deleteKey) { + try { secureStorage.deletePrivateKey(current.npub) } catch (_: SecureStorageException) {} + try { secureStorage.deletePrivateKey(bunkerEphemeralKeyAlias(current.npub)) } catch (_: SecureStorageException) {} + try { secureStorage.deletePrivateKey(nwcKeyAlias(current.npub)) } catch (_: SecureStorageException) {} + // Remove from metadata — fixes ghost accounts + accountStorage.deleteAccount(current.npub) + } + } + disconnectNip46Client() + _nwcConnection.value = null + _signerConnectionState.value = SignerConnectionState.NotRemote + _lastPingTimeSec.value = null + _accountState.value = AccountState.LoggedOut + stopHeartbeat() +} +``` + +Key changes: +- Add `secureStorage.deletePrivateKey(nwcKeyAlias(current.npub))` +- Add `accountStorage.deleteAccount(current.npub)` when `deleteKey=true` +- Remove `getBunkerFile().delete()` and `clearLastNpub()` (files eliminated) + +--- + +### Phase 4: Corruption Handling (Issue 5) + +**Deepening insight:** Use `StateFlow` pattern (matches existing `forceLogoutReason`). +Distinguish key-lost vs file-corrupted. Use timestamped backups. + +#### 4a: Add StorageCorruption sealed class to AccountManager + +```kotlin +sealed class StorageCorruption { + data class KeyLost(val backupPath: String?) : StorageCorruption() + data class FileCorrupted(val backupPath: String?) : StorageCorruption() + data class JsonMalformed(val backupPath: String?) : StorageCorruption() +} + +private val _storageCorruption = MutableStateFlow(null) +val storageCorruption: StateFlow = _storageCorruption.asStateFlow() +fun clearStorageCorruption() { _storageCorruption.value = null } +``` + +#### 4b: Rewrite readMetadataFromDisk in DesktopAccountStorage + +**File:** `DesktopAccountStorage.kt:114-126` + +Accept a corruption callback (constructor param), catch specific exceptions: + +```kotlin +class DesktopAccountStorage( + homeDir: File, + private val secureStorage: SecureKeyStorage, + private val onCorruption: (StorageCorruption) -> Unit = {}, // NEW +) : AccountStorage { + // ... + + private suspend fun readMetadataFromDisk(): AccountMetadata { + val file = getAccountsFile() + if (!file.exists()) return AccountMetadata() + + val encrypted = file.readBytes() + if (encrypted.size < GCM_IV_SIZE) { + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.FileCorrupted(backup)) + return AccountMetadata() + } + + return try { + val decrypted = decrypt(encrypted) + mapper.readValue(decrypted) + } catch (e: javax.crypto.AEADBadTagException) { + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.FileCorrupted(backup)) + AccountMetadata() + } catch (e: javax.crypto.BadPaddingException) { + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.FileCorrupted(backup)) + AccountMetadata() + } catch (e: com.fasterxml.jackson.core.JacksonException) { + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.JsonMalformed(backup)) + AccountMetadata() + } catch (e: Exception) { + Log.e("DesktopAccountStorage", "Failed to read accounts metadata", e) + val backup = backupCorruptFile(file) + onCorruption(StorageCorruption.FileCorrupted(backup)) + AccountMetadata() + } + } + + private fun backupCorruptFile(file: File): String? = + try { + val backup = File(file.parent, "accounts.json.enc.corrupt.${System.currentTimeMillis()}") + Files.copy(file.toPath(), backup.toPath()) + file.delete() + backup.absolutePath + } catch (_: Exception) { null } +} +``` + +#### 4c: Wire in AccountManager + +Pass corruption callback when constructing `DesktopAccountStorage`: + +```kotlin +val accountStorage = DesktopAccountStorage( + homeDir = homeDir, + secureStorage = secureStorage, + onCorruption = { _storageCorruption.value = it }, +) +``` + +#### 4d: Show warning dialog in Main.kt + +Observe `storageCorruption` StateFlow (same pattern as `forceLogoutReason`): + +```kotlin +val corruption by accountManager.storageCorruption.collectAsState() +corruption?.let { c -> + AlertDialog( + onDismissRequest = { accountManager.clearStorageCorruption() }, + title = { Text("Account Data Issue") }, + text = { + Text(when (c) { + is StorageCorruption.KeyLost -> "Encryption key not found. Account list cannot be recovered." + is StorageCorruption.FileCorrupted -> "Account data was corrupted." + is StorageCorruption.JsonMalformed -> "Account data was partially written." + } + if (c.backupPath != null) "\n\nA backup was saved." else "") + }, + confirmButton = { + Button(onClick = { accountManager.clearStorageCorruption() }) { Text("OK") } + }, + ) +} +``` + +--- + +## Files Modified + +| File | Lines | Changes | +|------|-------|---------| +| `AccountManager.kt` | 233-247, 460-492, 561-589, 788-872 | Rewrite loadSavedAccount, saveCurrentAccount, logout, NWC methods; remove 8 legacy helpers | +| `DesktopAccountStorage.kt` | 114-126, 223-258 | Corruption backup; add nwcPubKey/nwcRelay to AccountInfoDto | +| `Main.kt` | 835-852 | Remove hasBunkerAccount check; add corruption warning dialog | + +## Files Deleted (Legacy, stop reading/writing) + +| File | Content | Risk | +|------|---------|------| +| `~/.amethyst/nwc_connection.txt` | NWC URI with spending secret | CRITICAL — deleted on startup | +| `~/.amethyst/last_account.txt` | Last active npub | Low — deleted on startup | +| `~/.amethyst/bunker_uri.txt` | Bunker URI | Medium — deleted on startup | + +## Test Cases + +| # | Test | File | Type | +|---|------|------|------| +| 1 | nsec login → save → cold boot loads Internal | `AccountManagerColdBootTest.kt` | Unit | +| 2 | bunker login → save → cold boot loads Remote with URI | `AccountManagerColdBootTest.kt` | Unit | +| 3 | bunker login → switch to nsec → cold boot loads Internal | `AccountManagerColdBootTest.kt` | Unit | +| 4 | Two bunker accounts → switch → each uses own URI | `AccountManagerSwitchTest.kt` | Unit | +| 5 | NWC set for account A → switch to B → back to A → NWC restored | `AccountManagerNwcTest.kt` | Unit | +| 6 | NWC secret stored in keychain, NOT in any file | `AccountManagerNwcTest.kt` | Unit | +| 7 | logout(deleteKey=true) removes from metadata + keychain | `AccountManagerLogoutTest.kt` | Unit | +| 8 | Corrupted accounts.json.enc → .bak created, empty returned | `DesktopAccountStorageCorruptionTest.kt` | Unit | +| 9 | Legacy files deleted on first cold boot | `AccountManagerMigrationTest.kt` | Unit | +| 10 | Fresh install → "No saved account" | `AccountManagerColdBootTest.kt` | Unit | + +## Acceptance Criteria + +- [ ] `nwc_connection.txt` never written; deleted on startup if exists +- [ ] `last_account.txt` never written; deleted on startup if exists +- [ ] `bunker_uri.txt` never written; deleted on startup if exists +- [ ] NWC secret stored via `secureStorage.savePrivateKey("nwc_", secret)` +- [ ] `loadSavedAccount()` routes by `SignerType` from `accounts.json.enc` +- [ ] `logout(deleteKey=true)` removes account from `accounts.json.enc` +- [ ] Corrupted `accounts.json.enc` → `.bak` backup + warning dialog +- [ ] `switchAccount()` loads correct NWC per-account +- [ ] All 10 test cases pass +- [ ] `./gradlew spotlessCheck` passes +- [ ] No plaintext secrets in `~/.amethyst/` directory + +## Risk Analysis + +| Risk | Severity | Mitigation | +|------|----------|------------| +| Existing users lose NWC connection | Low | Acceptable — reconnect wallet once (brainstorm decision) | +| Existing users lose account on cold boot | Medium | accounts.json.enc already stores everything; only users without it are affected (new installs) | +| SecureKeyStorage fallback password prompt for NWC | Low | Same pattern as nsec storage — well-tested | +| Jackson backward compat for new AccountInfoDto fields | Low | Nullable fields with defaults — Jackson handles gracefully | + +## Sources + +- **Origin brainstorm:** docs/brainstorms/2026-05-14-fix-account-security-hardening-brainstorm.md +- `AccountManager.kt` — cold boot (233-247), save (460-492), logout (561-589), NWC (788-844), legacy files (846-872) +- `DesktopAccountStorage.kt` — corruption (114-126), AccountInfoDto (223-258) +- `Main.kt` — startup (831-853) +- `SecureKeyStorage.kt` — savePrivateKey/getPrivateKey/deletePrivateKey (commons/jvmMain)