Merge PR: fix(account): adding a read-only npub no longer clobbers a signing account

Merges nostr proposal e283c388 into main (replaces the closed e05208d9 with a
minimal guard). Adding/scanning your own read-only npub for a pubkey you already
hold the nsec for no longer downgrades the signing account — on Android it
wiped cached lists + disabled notifications, on desktop it orphaned the key.
Guarded at the single persistence point on each platform; desktop regression
test verified failing without the guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vitor Pamplona
2026-06-26 19:24:01 -04:00
co-authored by Claude Opus 4.8
4 changed files with 79 additions and 4 deletions
@@ -355,7 +355,27 @@ object LocalPreferences {
}
}
suspend fun setDefaultAccount(accountSettings: AccountSettings) {
/**
* Make [accountSettings] the current account, persisting + caching it. Returns the settings that
* actually became current — normally [accountSettings] itself, but see the downgrade guard below.
*/
suspend fun setDefaultAccount(accountSettings: AccountSettings): AccountSettings {
val npub = accountSettings.keyPair.pubKey.toNpub()
// Downgrade guard: adding a read-only npub for a pubkey we already hold a SIGNING account for
// must not clobber that account. Accounts dedup by npub, so saving fresh read-only settings
// here would overwrite the signing account's per-npub file — wiping its cached follow/relay/
// mute lists and flipping hasPrivKey off, which silently disables its push notifications. A
// signing account already does everything the read-only one would, so keep it and just make
// it current instead of degrading it.
if (!accountSettings.isWriteable()) {
val existing = loadAccountConfigFromEncryptedStorage(npub)
if (existing != null && existing.isWriteable()) {
setCurrentAccount(existing)
return existing
}
}
// Save the per-npub file before emitting onto the savedAccounts flow.
// Otherwise a collector (e.g. AlwaysOnNotificationServiceManager) can race in
// and call loadAccountConfigFromEncryptedStorage(npub) before NOSTR_PUBKEY is
@@ -363,9 +383,9 @@ object LocalPreferences {
// rest of the session — making every later switch to this account land on
// LoggedOff instead of LoggedIn.
saveToEncryptedStorage(accountSettings)
val npub = accountSettings.keyPair.pubKey.toNpub()
mutex.withLock { cachedAccounts.put(npub, accountSettings) }
setCurrentAccount(accountSettings)
return accountSettings
}
suspend fun allSavedAccounts(): List<AccountInfo> = savedAccounts()
@@ -175,9 +175,11 @@ class AccountSessionManager(
}
}
localPreferences.setDefaultAccount(accountSettings)
// setDefaultAccount may keep an existing signing account instead of this one when a
// read-only npub is added for a pubkey we already sign for — show whichever became current.
val current = localPreferences.setDefaultAccount(accountSettings)
startUI(accountSettings)
startUI(current)
}
fun startUI(
@@ -508,6 +508,18 @@ class AccountManager internal constructor(
suspend fun saveCurrentAccount(): Result<Unit> {
val current = currentAccount() ?: return Result.failure(Exception("No account logged in"))
// Downgrade guard (mirrors Android LocalPreferences.setDefaultAccount): never persist a
// read-only account over an existing SIGNING account for the same pubkey. Accounts key by
// npub, so this would overwrite signerType to ViewOnly and orphan the stored key, routing
// every later switch through read-only. A signing account already subsumes a read-only one,
// so keep it and switch to it instead of degrading it.
if (current.isReadOnly) {
val existing = accountStorage.loadAccounts().firstOrNull { it.npub == current.npub }
if (existing != null && existing.signerType !is SignerType.ViewOnly) {
return switchAccount(current.npub).map { }
}
}
// Bunker accounts: private key saved during loginWithBunker
if (current.signerType is SignerType.Remote) {
// Still ensure multi-account storage is updated
@@ -159,4 +159,45 @@ class AccountManagerKeyLoginTest {
val accounts = manager.accountStorage.loadAccounts()
assertTrue(accounts.any { it.npub == keyPair.pubKey.toNpub() })
}
/**
* Regression: adding the read-only npub for a pubkey we already hold a SIGNING account for must
* NOT downgrade it. Accounts key by npub, so persisting the ViewOnly entry would overwrite the
* Internal signerType and orphan the stored key, routing every later switch through read-only.
* The signing account must survive and become current instead.
*/
@Test
fun addingReadOnlyNpubDoesNotDowngradeExistingSigningAccount() =
runTest {
val keyPair = KeyPair()
val npub = keyPair.pubKey.toNpub()
// 1. Sign in with the nsec and persist it as a full signing account.
manager.loginWithKey(keyPair.privKey!!.toNsec())
manager.saveCurrentAccount()
assertEquals(
SignerType.Internal,
manager.accountStorage
.loadAccounts()
.first { it.npub == npub }
.signerType,
)
assertTrue(keyStore.containsKey(npub))
// 2. Add the read-only npub for the SAME pubkey and try to save it.
manager.loginWithKey(npub)
assertTrue((manager.accountState.value as AccountState.LoggedIn).isReadOnly)
val result = manager.saveCurrentAccount()
assertTrue(result.isSuccess)
// 3. The signing account survives: storage stays Internal, the key is kept,
// and the current account was switched back to signing (not downgraded).
val stored = manager.accountStorage.loadAccounts().first { it.npub == npub }
assertEquals(SignerType.Internal, stored.signerType)
assertTrue(keyStore.containsKey(npub))
val finalState = manager.accountState.value
assertIs<AccountState.LoggedIn>(finalState)
assertFalse(finalState.isReadOnly)
assertEquals(SignerType.Internal, finalState.signerType)
}
}