diff --git a/amethyst/plans/2026-09-23-encrypted-storage-retirement.md b/amethyst/plans/2026-09-23-encrypted-storage-retirement.md index 4a6ff1ab2f..ceb220068c 100644 --- a/amethyst/plans/2026-09-23-encrypted-storage-retirement.md +++ b/amethyst/plans/2026-09-23-encrypted-storage-retirement.md @@ -1,15 +1,17 @@ # Retiring EncryptedStorage -Status: **blocked** — the legacy files cannot be deleted yet, and the reader -can never be. +Status: **migrated, not yet deleted.** Every key has a home in the new stores. +The legacy files are still written, so they are still there — and the reader +can never go. ## The constraint Every migration in the preference layer is *lazy*: it reads the legacy store -when it runs, not when the app is installed. Nine come through -`EncryptedStorage` — the seven `CopyOnceMigration`s in `LocalPreferences` plus -the key, secret and roster stores. Only the Cashu counters and calendar -reminders read a plain (non-encrypted) source and would survive its removal. +when it runs, not when the app is installed. Ten come through +`EncryptedStorage` — the eight `LegacyKeyTable` copies on the per-account +DataStore plus the key, secret and roster stores. Only the Cashu counters and +calendar reminders read a plain (non-encrypted) source and would survive its +removal. So deleting `EncryptedStorage` does not merely affect installs that have not upgraded yet. It strands anyone who **skips** the release introducing the new @@ -25,54 +27,89 @@ from a backup all skip releases. an unmaintained-library risk, not an active vulnerability, and a much smaller cost than stranding users. -## Outstanding before any legacy file is deleted +## Where each key went -Deletion is only safe for an account whose every key has a new home. These do -not yet, and are still read from the legacy files: +`LegacyKeyCoverageTest` holds this to being exhaustive: every constant in +`PrefKeys` is either claimed by a migration table, one of the secrets, on the +accepted-loss list, or a key of the global file. A key added to `PrefKeys` and +to none of those fails that test at the commit that adds it. -| key | scope | if deleted today | +| group | destination | legacy write | |---|---|---| -| `NOSTR_PUBKEY` | per-account | **fatal** — `loadAccountConfigFromEncryptedStorage` returns null without it, so the account disappears even though its private key migrated | -| `LOGIN_WITH_EXTERNAL_SIGNER` | per-account | external-signer accounts stop resolving their signer | -| `SIGNER_PACKAGE_NAME` | per-account | as above | -| `HAS_BACKED_UP_KEYS` | per-account | the key-backup nag returns for everyone | -| `LOCAL_RELAY_SERVERS` | per-account | silently lost | -| `OPEN_BACKUP_CONFLICTS` | per-account | silently lost | -| `SHARED_SETTINGS` | global | UI settings reset | +| follow lists, cached events, upload, dialogs, relay auth, feed visibility, notifications | the account's plain DataStore | already retired | +| identity — pubkey, signer, local relays, backup conflicts, backup flag | the account's plain DataStore | **kept** | +| private key | `SecureKeyStorage` | **kept** | +| NIP-46 material, wallets, payment source | the account's encrypted DataStore | **kept** | +| current account, saved accounts | the encrypted roster store | **kept** | +| UI settings (`shared_settings`) | `UiSharedPreferences`' own DataStore | none left | + +The three stores that still mirror are the ones whose loss is not an +annoyance: an account that cannot be listed, signed with, or paid from. They +keep the rollback window open until the device pass below has happened. + +The UI settings copy is **guarded** where the others are not. That store has +been the real home of these settings for a while, so most installs already +have a populated one and copying the old blob over it would undo every UI +change since. The copy only runs into a store that has never been saved +(`ui.theme` absent, which `save` always writes). ## Deliberately not migrated -Three keys are accepted losses rather than outstanding work — the cost of -losing them is one-off and small, and carrying them is not worth the code: - | key | what is lost | |---|---| | `PENDING_ATTESTATIONS` | queued OTS attestations are not published | | `NOTIF_GLOBAL_TO_CURATED_MIGRATED` | the one-shot notification filter migration runs once more | | `LAST_READ_PER_ROUTE` | every feed reads as unread once | +| `USE_PROXY`, `PROXY_PORT` | nothing — only ever removed, never read | +| `TOR_SETTINGS` | nothing — no reader left anywhere | -`NotificationPrefsStore` was given `hasRunGlobalToCuratedMigration`, -`markGlobalToCuratedMigrated`, `lastReadPerRoute` and `saveLastReadPerRoute` -for the last two of these. Nothing ever called them, and now nothing will; -they have been removed rather than left looking like a feature. +These are listed in `LegacyAccountKeys.accepted`, which is what lets the +cleanup treat any *other* unclaimed key as a reason to keep the file. -`USE_PROXY` and `PROXY_PORT` need nothing either: they are only ever `remove`d, -being cleaned up rather than read. +## Deleting a legacy file + +`LegacyPreferenceCleanup` runs after every successful account load and deletes +that account's file only when it can prove nothing would be lost: + +1. **Every key in the file is accounted for** — claimed by a table, one of the + secrets, or on the accepted list. Driven from the file's own keys, not from + a checklist, because a checklist fails silently in the one direction that + matters. +2. **Every copy that had something to copy has run.** Marker-based, not a value + comparison: those groups stopped being legacy-written when they moved, so + the file is a frozen snapshot and the two are *expected* to diverge as soon + as the user changes a setting. `CopyOnceMigration` commits the values and + its marker as one `Preferences`, so the marker cannot be set without them. +3. **The secrets and the private key read back identical** from the current + stores. Those *are* still dual-written, so the stronger question is + available and is asked. +4. A store that cannot be read is a reason, never a pass. + +It refuses today, and says so, because of the fifth condition: +`LEGACY_WRITES_RETIRED` is false. While the app still mirrors into the file, +deleting it achieves nothing — the next save recreates it — and would look +like it had worked. ## Order of work -1. Migrate the seven keys above, on the same dual-store terms as the rest. -2. Add a per-account completeness check — every key present in the new stores — - and only then delete that account's legacy file, after reading back what was - written. -3. Retire the legacy writes once (2) holds for every account on a device. -4. Keep the reader, and the dependency, indefinitely. +1. ~~Migrate the remaining keys.~~ Done. +2. ~~Gate deletion on a per-account read-back.~~ Done. +3. Do the device pass below. +4. Flip `LEGACY_WRITES_RETIRED` and drop the legacy writes for the identity, + key, secret and roster stores. This ends the rollback window, so it is a + release of its own. +5. Keep the reader, and `androidx.security.crypto`, indefinitely. ## Verification this needs and has not had None of the AndroidKeyStore paths have executed: this environment has no device or emulator, and `commons` has no Robolectric. What is tested is the decision -logic against fakes. On a real device, before any deletion ships: upgrade an -install holding accounts and confirm they all list; open one and sign; -force-stop and relaunch; add and remove an account; pair a NIP-46 signer; pay -from a wallet. +logic against fakes — which is why step 3 is not optional, and why deletion is +the one irreversible step in the whole series. + +On a real device, before step 4 ships: upgrade an install holding accounts and +confirm they all list; open one and sign; force-stop and relaunch; add and +remove an account; pair a NIP-46 signer; pay from a wallet; check the +key-backup nudge stays dismissed; confirm UI settings survive the upgrade. +Then let the cleanup run with the flag flipped, and confirm the files are gone +and everything above still holds on the next cold start. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountKeyStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountKeyStore.kt index 71ac28878d..5ef8b4af68 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountKeyStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountKeyStore.kt @@ -168,6 +168,13 @@ class AccountKeyStore( } } + /** + * What the current store holds, with no fallback to the legacy value. + * + * For [LegacyPreferenceCleanup]; [read] deliberately hides this distinction. + */ + suspend fun stored(npub: String): String? = vault.get(npub) + /** Drops the key from the new store; the caller clears the legacy file itself. */ suspend fun delete(npub: String) { try { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountSecretsStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountSecretsStore.kt index 6e3c3bcf12..e60aaa48cf 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountSecretsStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountSecretsStore.kt @@ -90,6 +90,14 @@ class AccountSecretsStore( } } + /** + * What the current store holds, with no fallback to the legacy file. + * + * For [LegacyPreferenceCleanup], which has to tell "migrated" from + * "falling back and looking migrated" — the read above deliberately cannot. + */ + suspend fun stored(npub: String): AccountSecrets? = stores.loadSecrets(npub) + suspend fun delete(npub: String) { try { stores.removeAccount(npub) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanup.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanup.kt new file mode 100644 index 0000000000..fc7973abe6 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanup.kt @@ -0,0 +1,302 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst + +import androidx.datastore.preferences.core.Preferences +import com.vitorpamplona.amethyst.commons.model.preferences.AccountIdentityStore +import com.vitorpamplona.amethyst.commons.model.preferences.AccountSecrets +import com.vitorpamplona.amethyst.commons.model.preferences.DialogDismissalStore +import com.vitorpamplona.amethyst.commons.model.preferences.FeedVisibilityStore +import com.vitorpamplona.amethyst.commons.model.preferences.LatestEventCacheStore +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyAccountSecretNames +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyKeyTable +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyPreferenceSource +import com.vitorpamplona.amethyst.commons.model.preferences.NotificationPrefsStore +import com.vitorpamplona.amethyst.commons.model.preferences.RelayAuthStore +import com.vitorpamplona.amethyst.commons.model.preferences.TopNavFollowListStore +import com.vitorpamplona.amethyst.commons.model.preferences.UploadSettingsStore +import com.vitorpamplona.amethyst.commons.model.preferences.readLegacyAccountSecrets +import com.vitorpamplona.quartz.utils.Log + +/** + * How every key that can appear in a `secret_keeper_` file is accounted + * for. + * + * Together with [LegacyAccountSecretNames], these two lists are what let + * [LegacyPreferenceCleanup] treat any *other* key in the file as a reason not + * to delete it. `LegacyKeyCoverageTest` holds them to covering all of + * `PrefKeys`, so a key added later cannot quietly fall outside both. + */ +internal object LegacyAccountKeys { + /** + * The one-shot copies out of the account's legacy file. + * + * Each store owns the table of legacy names it came from, so the copy and + * the check that the copy happened read the same list — see [LegacyKeyTable]. + */ + val tables = + listOf( + TopNavFollowListStore.legacyTable, + LatestEventCacheStore.legacyTable, + UploadSettingsStore.legacyTable, + DialogDismissalStore.legacyTable, + RelayAuthStore.legacyTable, + FeedVisibilityStore.legacyTable, + NotificationPrefsStore.legacyTable, + AccountIdentityStore.legacyTable, + ) + + /** + * Keys that are deliberately not carried across. + * + * Each costs something once and nothing after, and none is worth the code + * to move it: queued attestations go unpublished, the one-shot + * Global -> Curated notification rewrite runs one more time, and every feed + * reads as unread once. `use_proxy` and `proxy_port` are only ever removed, + * never read, and `tor_settings` has no reader left at all. + */ + val accepted = + setOf( + PrefKeys.PENDING_ATTESTATIONS, + PrefKeys.NOTIF_GLOBAL_TO_CURATED_MIGRATED, + PrefKeys.LAST_READ_PER_ROUTE, + PrefKeys.USE_PROXY, + PrefKeys.PROXY_PORT, + PrefKeys.TOR_SETTINGS, + ) +} + +/** What [LegacyPreferenceCleanup] did, and why. */ +sealed interface LegacyCleanupResult { + /** There was no legacy file for this account. */ + data object NothingToDelete : LegacyCleanupResult + + data object Deleted : LegacyCleanupResult + + /** Nothing was touched. Each reason names one thing that would have been lost. */ + data class Kept( + val reasons: List, + ) : LegacyCleanupResult +} + +/** The per-account legacy file, as this needs it. */ +interface LegacyAccountFiles { + fun source(npub: String): LegacyPreferenceSource + + fun exists(npub: String): Boolean + + /** Returns false when there was nothing to delete. */ + suspend fun delete(npub: String): Boolean +} + +/** What the current, encrypted stores hold for an account. */ +interface MigratedSecrets { + /** Null when this account has not been copied across yet. */ + suspend fun secrets(npub: String): AccountSecrets? + + /** Null only when the account genuinely has no private key. Throws when the store is unreadable. */ + suspend fun privateKey(npub: String): String? +} + +/** + * Deletes an account's `secret_keeper_` file, but only once it can prove + * nothing in it would be lost. + * + * # Why the check is not one rule + * + * The two halves of the migration are in different states, and asking the same + * question of both would give the wrong answer for one of them. + * + * The plain per-account groups — settings, dialogs, feeds, cached events — + * stopped being written to the legacy file when they moved, so that file is a + * frozen snapshot of the day they migrated. Comparing values would flag every + * setting the user has changed since. What is actually being asked of them is + * "did the copy run", and [LegacyKeyTable.hasRun] answers it exactly: + * `CopyOnceMigration` writes the values and its marker as a single + * `Preferences`, committed atomically, so the marker cannot be set without them. + * + * The secrets and the private key are still written to *both* stores on every + * save, so for those the stronger question is available and is asked: read both + * back and require them to agree. + * + * # Why an unrecognised key blocks + * + * A list of keys to check, maintained by hand, fails silently in the one + * direction that matters: a key added later that no migration carries. So the + * check runs the other way round — every key *in the file* must be claimed by + * a table, be one of the secrets, or be on [accepted], the short list of + * deliberate losses. Anything else stops the deletion and says so by name. + * + * # Cost + * + * [LegacyPreferenceSource.keys] goes through `EncryptedSharedPreferences.all`, + * which decrypts every value in the file — there is no keys-only API. It is + * called from the one place an account load is not already cached, so it costs + * at most once per account per process, and nothing at all while + * [legacyWritesRetired] is false. + * + * # Why deletion also waits on the legacy writes + * + * [legacyWritesRetired] is the other half. While the app still mirrors into + * this file on every save, deleting it achieves nothing — the next save + * recreates it, with a subset of what was there. Worse, it would look like it + * had worked. So the file is only removed once it is no longer being written, + * which is a separate release from this one. + */ +class LegacyPreferenceCleanup( + private val tables: List, + private val accepted: Set, + private val files: LegacyAccountFiles, + private val currentStore: suspend (String) -> Preferences, + private val secrets: MigratedSecrets, + private val legacyWritesRetired: Boolean, +) { + companion object { + private const val TAG = "LegacyPreferenceCleanup" + + const val STILL_WRITTEN = "the legacy file is still written on every save" + } + + private val claimed: Set = tables.flatMapTo(mutableSetOf()) { it.legacyNames } + LegacyAccountSecretNames.all + + /** + * Everything that would be lost by deleting this account's legacy file. + * Empty means nothing would be. + * + * A store that cannot be read is a reason, never a pass: the whole point is + * to be sure, and "the check itself failed" is not sure. + */ + suspend fun verify(npub: String): List { + val legacy = + try { + files.source(npub) + } catch (e: Exception) { + Log.w(TAG, "Could not open the legacy file for $npub", e) + return listOf("the legacy file could not be read") + } + + val reasons = mutableListOf() + + val present = legacy.keys() + (present - claimed - accepted).sorted().forEach { + reasons += "no migration claims '$it'" + } + + val current = + try { + currentStore(npub) + } catch (e: Exception) { + Log.w(TAG, "Could not read the current store for $npub", e) + return reasons + "the current store could not be read" + } + + tables.forEach { table -> + // A table whose keys the file never held has nothing to prove. + if (present.none { it in table.legacyNames }) return@forEach + if (!table.hasRun(current)) reasons += "the '${table.markerName}' copy has not run" + } + + reasons += secretMismatches(npub, legacy) + + return reasons + } + + private suspend fun secretMismatches( + npub: String, + legacy: LegacyPreferenceSource, + ): List { + val expected = readLegacyAccountSecrets(legacy) + val reasons = mutableListOf() + + try { + val stored = secrets.secrets(npub) + when { + stored == null -> reasons += "the secrets have not been copied across" + // Field names only. These values are bunker secrets and wallet + // connection strings; a log line is the last place for them. + stored != expected -> reasons += "the stored secrets differ from the legacy file: ${differingFields(expected, stored)}" + } + } catch (e: Exception) { + Log.w(TAG, "Could not read the secrets store for $npub", e) + reasons += "the secrets store could not be read" + } + + val legacyKey = legacy.getString(LegacyAccountSecretNames.NOSTR_PRIVKEY) + if (legacyKey != null) { + try { + when (secrets.privateKey(npub)) { + null -> reasons += "the private key has not been copied across" + legacyKey -> Unit + else -> reasons += "the stored private key differs from the legacy file" + } + } catch (e: Exception) { + Log.w(TAG, "Could not read the key store for $npub", e) + reasons += "the key store could not be read" + } + } + + return reasons + } + + private fun differingFields( + expected: AccountSecrets, + stored: AccountSecrets, + ): String = + listOfNotNull( + "nip46SignerEnabled".takeIf { expected.nip46SignerEnabled != stored.nip46SignerEnabled }, + "nip46BunkerSecret".takeIf { expected.nip46BunkerSecret != stored.nip46BunkerSecret }, + "nip46TransportKey".takeIf { expected.nip46TransportKey != stored.nip46TransportKey }, + "nip46SeenRequestIds".takeIf { expected.nip46SeenRequestIds != stored.nip46SeenRequestIds }, + "nwcWallets".takeIf { expected.nwcWalletsJson != stored.nwcWalletsJson }, + "clinkDebitWallets".takeIf { expected.clinkDebitWalletsJson != stored.clinkDebitWalletsJson }, + "defaultPaymentSourceId".takeIf { expected.defaultPaymentSourceId != stored.defaultPaymentSourceId }, + "defaultNwcWalletId".takeIf { expected.legacyDefaultNwcWalletId != stored.legacyDefaultNwcWalletId }, + "zapPaymentServer".takeIf { expected.legacyZapPaymentRequestServer != stored.legacyZapPaymentRequestServer }, + ).joinToString() + + /** + * Deletes the account's legacy file if — and only if — [verify] comes back + * empty and the app has stopped writing to it. + */ + suspend fun deleteIfVerified(npub: String): LegacyCleanupResult { + if (!files.exists(npub)) return LegacyCleanupResult.NothingToDelete + + if (!legacyWritesRetired) return LegacyCleanupResult.Kept(listOf(STILL_WRITTEN)) + + val reasons = verify(npub) + if (reasons.isNotEmpty()) { + Log.i(TAG) { "Keeping the legacy file for $npub: ${reasons.joinToString("; ")}" } + return LegacyCleanupResult.Kept(reasons) + } + + return try { + if (files.delete(npub)) { + Log.i(TAG) { "Deleted the migrated legacy file for $npub" } + LegacyCleanupResult.Deleted + } else { + LegacyCleanupResult.NothingToDelete + } + } catch (e: Exception) { + Log.w(TAG, "Could not delete the legacy file for $npub", e) + LegacyCleanupResult.Kept(listOf("the legacy file could not be deleted")) + } + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt index 37e72033e3..9df3f2a2f5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt @@ -34,6 +34,8 @@ import com.vitorpamplona.amethyst.commons.model.mediaServers.ServerName import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupViewMode import com.vitorpamplona.amethyst.commons.model.nip47WalletConnect.NwcWalletEntry import com.vitorpamplona.amethyst.commons.model.nip47WalletConnect.NwcWalletEntryNorm +import com.vitorpamplona.amethyst.commons.model.preferences.AccountIdentity +import com.vitorpamplona.amethyst.commons.model.preferences.AccountIdentityStore import com.vitorpamplona.amethyst.commons.model.preferences.AccountPreferenceStores import com.vitorpamplona.amethyst.commons.model.preferences.AccountSecrets import com.vitorpamplona.amethyst.commons.model.preferences.CopyOnceMigration @@ -44,6 +46,7 @@ import com.vitorpamplona.amethyst.commons.model.preferences.FeedVisibilityStore import com.vitorpamplona.amethyst.commons.model.preferences.FollowListSlot import com.vitorpamplona.amethyst.commons.model.preferences.LatestEventCacheStore import com.vitorpamplona.amethyst.commons.model.preferences.LatestEventSlot +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyPreferenceSource import com.vitorpamplona.amethyst.commons.model.preferences.NotificationPrefs import com.vitorpamplona.amethyst.commons.model.preferences.NotificationPrefsStore import com.vitorpamplona.amethyst.commons.model.preferences.RelayAuth @@ -51,12 +54,15 @@ import com.vitorpamplona.amethyst.commons.model.preferences.RelayAuthStore import com.vitorpamplona.amethyst.commons.model.preferences.TopNavFollowListStore import com.vitorpamplona.amethyst.commons.model.preferences.UploadSettings import com.vitorpamplona.amethyst.commons.model.preferences.UploadSettingsStore +import com.vitorpamplona.amethyst.commons.model.preferences.orIfUnusable +import com.vitorpamplona.amethyst.commons.model.preferences.readLegacyAccountSecrets import com.vitorpamplona.amethyst.commons.model.topNavFeeds.TopFilter import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy import com.vitorpamplona.amethyst.model.AccountSettings import com.vitorpamplona.amethyst.model.UiSettings import com.vitorpamplona.amethyst.model.backups.BackupConflictStorage import com.vitorpamplona.amethyst.model.nip60Cashu.CashuPreferences +import com.vitorpamplona.amethyst.model.preferences.UiSharedPreferences import com.vitorpamplona.amethyst.service.checkNotInMainThread import com.vitorpamplona.quartz.concord.cord02Community.ConcordCommunityListEvent import com.vitorpamplona.quartz.experimental.ephemChat.list.EphemeralChatListEvent @@ -98,6 +104,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -120,7 +127,7 @@ data class AccountInfo( val isTransient: Boolean = false, ) -private object PrefKeys { +internal object PrefKeys { const val CURRENT_ACCOUNT = "currently_logged_in_account" // Global (non-account) master switch for the always-on notification service. @@ -268,12 +275,13 @@ object LocalPreferences { private val cachedAccounts: MutableMap = mutableMapOf() /** - * The per-account DataStore, and the top-nav filter selections inside it. + * The per-account DataStore: every non-secret setting this account has. * - * Each account's store carries a [CopyOnceMigration] that lifts the filters - * out of that account's legacy encrypted SharedPreferences the first time - * the store is read. The copy leaves the legacy keys in place, so a build - * that reads the old location still works — see [CopyOnceMigration]. + * Each account's store carries one [CopyOnceMigration] per group in + * [LegacyAccountKeys.tables], lifting that group out of the account's legacy + * encrypted SharedPreferences the first time the store is read. The copies + * leave the legacy keys in place, so a build that reads the old location + * still works — see [CopyOnceMigration]. */ private val accountStores: AccountPreferenceStores by lazy { AccountPreferenceStores( @@ -282,8 +290,7 @@ object LocalPreferences { .toOkioPath() }, migrations = { npub -> - listOf(followListMigration(npub), latestEventMigration(npub)) + - listOf(uploadSettingsMigration(npub), dialogDismissalMigration(npub), relayAuthMigration(npub), feedVisibilityMigration(npub), notificationPrefsMigration(npub)) + LegacyAccountKeys.tables.map { it.migration { legacySource(npub) } } }, ) } @@ -302,68 +309,64 @@ object LocalPreferences { private fun notificationPrefsStore(npub: String) = NotificationPrefsStore(accountStores.getDataStore(npub)) - private fun uploadSettingsMigration(npub: String) = - CopyOnceMigration("migrated.uploadSettings") { out -> - withContext(Dispatchers.IO) { - val legacy = encryptedPreferences(npub) - if (legacy.contains(PrefKeys.STRIP_LOCATION_ON_UPLOAD)) out[UploadSettingsStore.stripLocationOnUpload] = legacy.getBoolean(PrefKeys.STRIP_LOCATION_ON_UPLOAD, false) - if (legacy.contains(PrefKeys.OPTIMIZE_MEDIA_ON_UPLOAD)) out[UploadSettingsStore.optimizeMediaOnUpload] = legacy.getBoolean(PrefKeys.OPTIMIZE_MEDIA_ON_UPLOAD, false) - if (legacy.contains(PrefKeys.MIRROR_UPLOADS_TO_ALL_SERVERS)) out[UploadSettingsStore.mirrorUploadsToAllServers] = legacy.getBoolean(PrefKeys.MIRROR_UPLOADS_TO_ALL_SERVERS, false) - if (legacy.contains(PrefKeys.USE_LOCAL_BLOSSOM_CACHE)) out[UploadSettingsStore.useLocalBlossomCache] = legacy.getBoolean(PrefKeys.USE_LOCAL_BLOSSOM_CACHE, false) - if (legacy.contains(PrefKeys.LOCAL_BLOSSOM_CACHE_PROFILE_PICTURES_ONLY)) out[UploadSettingsStore.localBlossomCacheProfilePicturesOnly] = legacy.getBoolean(PrefKeys.LOCAL_BLOSSOM_CACHE_PROFILE_PICTURES_ONLY, false) - legacy.getString(PrefKeys.DEFAULT_FILE_SERVER, null)?.let { out[UploadSettingsStore.defaultFileServerJson] = it } - } - } + private fun identityStore(npub: String) = AccountIdentityStore(accountStores.getDataStore(npub)) - private fun dialogDismissalMigration(npub: String) = - CopyOnceMigration("migrated.dialogDismissal") { out -> - withContext(Dispatchers.IO) { - val legacy = encryptedPreferences(npub) - if (legacy.contains(PrefKeys.HIDE_DELETE_REQUEST_DIALOG)) out[DialogDismissalStore.hideDeleteRequestDialog] = legacy.getBoolean(PrefKeys.HIDE_DELETE_REQUEST_DIALOG, false) - if (legacy.contains(PrefKeys.HIDE_BLOCK_ALERT_DIALOG)) out[DialogDismissalStore.hideBlockAlertDialog] = legacy.getBoolean(PrefKeys.HIDE_BLOCK_ALERT_DIALOG, false) - if (legacy.contains(PrefKeys.HIDE_NIP_17_WARNING_DIALOG)) out[DialogDismissalStore.hideNip17WarningDialog] = legacy.getBoolean(PrefKeys.HIDE_NIP_17_WARNING_DIALOG, false) - if (legacy.contains(PrefKeys.HIDE_COMMUNITY_RULES_VIOLATIONS)) out[DialogDismissalStore.hideCommunityRulesViolations] = legacy.getBoolean(PrefKeys.HIDE_COMMUNITY_RULES_VIOLATIONS, false) - legacy.getStringSet(PrefKeys.DISMISSED_POLL_NOTE_IDS, null)?.let { out[DialogDismissalStore.dismissedPollNoteIds] = it } - legacy.getStringSet(PrefKeys.DISMISSED_CHANNEL_INVITES, null)?.let { out[DialogDismissalStore.dismissedChannelInvites] = it } - legacy.getStringSet(PrefKeys.MUTED_PUBLIC_CHATS, null)?.let { out[DialogDismissalStore.mutedPublicChats] = it } - legacy.getStringSet(PrefKeys.HAS_DONATED_IN_VERSION, null)?.let { out[DialogDismissalStore.hasDonatedInVersion] = it } - legacy.getString(PrefKeys.VIEWED_POLL_RESULT_NOTE_IDS, null)?.let { out[DialogDismissalStore.viewedPollResultNoteIdsJson] = it } - } - } + private fun legacySource(npub: String): LegacyPreferenceSource = LegacySharedPreferences(encryptedPreferences(npub)) - private fun relayAuthMigration(npub: String) = - CopyOnceMigration("migrated.relayAuth") { out -> - withContext(Dispatchers.IO) { - val legacy = encryptedPreferences(npub) - legacy.getString(PrefKeys.DEFAULT_RELAY_AUTH_POLICY, null)?.let { out[RelayAuthStore.policyName] = it } - if (legacy.contains(PrefKeys.RELAY_AUTH_TRUST_MY_RELAYS)) out[RelayAuthStore.trustMyRelays] = legacy.getBoolean(PrefKeys.RELAY_AUTH_TRUST_MY_RELAYS, false) - if (legacy.contains(PrefKeys.RELAY_AUTH_TRUST_READ_FOLLOWS)) out[RelayAuthStore.trustReadFollows] = legacy.getBoolean(PrefKeys.RELAY_AUTH_TRUST_READ_FOLLOWS, false) - if (legacy.contains(PrefKeys.RELAY_AUTH_TRUST_MESSAGE_FOLLOWS)) out[RelayAuthStore.trustMessageFollows] = legacy.getBoolean(PrefKeys.RELAY_AUTH_TRUST_MESSAGE_FOLLOWS, false) - if (legacy.contains(PrefKeys.RELAY_AUTH_TRUST_MESSAGE_STRANGERS)) out[RelayAuthStore.trustMessageStrangers] = legacy.getBoolean(PrefKeys.RELAY_AUTH_TRUST_MESSAGE_STRANGERS, false) - } - } + /** + * Whether the app has stopped mirroring into `secret_keeper_`. + * + * False, and deliberately so: the private key, the secrets and the identity + * group are all still written there, so that a build rolled back to reading + * only the legacy file still finds a complete account. Deleting the file + * while that is true would achieve nothing — the next save recreates it — + * so [legacyCleanup] refuses to. + * + * Flipping this is a release of its own, and it ends the rollback window. + * It waits on the device pass in + * `amethyst/plans/2026-09-23-encrypted-storage-retirement.md`. + */ + private const val LEGACY_WRITES_RETIRED = false - private fun feedVisibilityMigration(npub: String) = - CopyOnceMigration("migrated.feedVisibility") { out -> - withContext(Dispatchers.IO) { - val legacy = encryptedPreferences(npub) - legacy.getString(PrefKeys.DISABLED_CHAT_FEEDS, null)?.let { out[FeedVisibilityStore.disabledChatFeeds] = it } - legacy.getString(PrefKeys.DISABLED_HOME_FEED_TYPES, null)?.let { out[FeedVisibilityStore.disabledHomeFeedTypes] = it } - legacy.getString(PrefKeys.RELAY_GROUP_VIEW_MODE, null)?.let { out[FeedVisibilityStore.relayGroupViewMode] = it } - legacy.getString(PrefKeys.CONCORD_VIEW_MODE, null)?.let { out[FeedVisibilityStore.concordViewMode] = it } - if (legacy.contains(PrefKeys.CALLS_ENABLED)) out[FeedVisibilityStore.callsEnabled] = legacy.getBoolean(PrefKeys.CALLS_ENABLED, false) - } - } + private val legacyCleanup: LegacyPreferenceCleanup by lazy { + LegacyPreferenceCleanup( + tables = LegacyAccountKeys.tables, + accepted = LegacyAccountKeys.accepted, + files = + object : LegacyAccountFiles { + override fun source(npub: String) = legacySource(npub) - private fun notificationPrefsMigration(npub: String) = - CopyOnceMigration("migrated.notificationPrefs") { out -> - withContext(Dispatchers.IO) { - val legacy = encryptedPreferences(npub) - if (legacy.contains(PrefKeys.ALWAYS_ON_NOTIFICATION_SERVICE)) out[NotificationPrefsStore.alwaysOnService] = legacy.getBoolean(PrefKeys.ALWAYS_ON_NOTIFICATION_SERVICE, false) - if (legacy.contains(PrefKeys.SHOW_MESSAGES_IN_NOTIFICATIONS)) out[NotificationPrefsStore.showMessagesInNotifications] = legacy.getBoolean(PrefKeys.SHOW_MESSAGES_IN_NOTIFICATIONS, false) - if (legacy.contains(PrefKeys.SPLIT_NOTIFICATIONS_ENABLED)) out[NotificationPrefsStore.splitNotificationsEnabled] = legacy.getBoolean(PrefKeys.SPLIT_NOTIFICATIONS_ENABLED, false) - } - } + override fun exists(npub: String) = legacyAccountFile(npub).exists() + + override suspend fun delete(npub: String): Boolean { + // Clear before unlinking, as deleteAccount does: the live + // SharedPreferences still holds the values in memory and + // would write them straight back out. + encryptedPreferences(npub).edit(commit = true) { clear() } + return legacyAccountFile(npub).delete() + } + }, + currentStore = { npub -> accountStores.getDataStore(npub).data.first() }, + secrets = + object : MigratedSecrets { + override suspend fun secrets(npub: String) = accountSecretsStore.stored(npub) + + override suspend fun privateKey(npub: String) = accountKeyStore.stored(npub) + }, + legacyWritesRetired = LEGACY_WRITES_RETIRED, + ) + } + + /** + * The file behind [encryptedPreferences], following the same branch it + * does — a name taken from the other side of that `if` would have the + * cleanup checking for, and deleting, a file that is not the one being + * read. + */ + private fun legacyAccountFile(npub: String): File { + val name = if (BuildConfig.DEBUG && DEBUG_PLAINTEXT_PREFERENCES) "${DEBUG_PREFERENCES_NAME}_$npub" else EncryptedStorage.prefsFileName(npub) + return File(prefsDirPath, "$name.xml") + } /** * Everything the account's DataStore holds, read in one hop. @@ -375,6 +378,7 @@ object LocalPreferences { * over. One call, one state. */ private class AccountStoreData( + val identity: AccountIdentity, val followLists: Map, val latestEvents: Map, val uploadSettings: UploadSettings, @@ -384,36 +388,19 @@ object LocalPreferences { val notificationPrefs: NotificationPrefs, ) - private suspend fun loadAccountStores(npub: String) = - AccountStoreData( - followLists = followListStore(npub).load(), - latestEvents = latestEventStore(npub).load(), - uploadSettings = uploadSettingsStore(npub).load(), - dialogDismissal = dialogDismissalStore(npub).load(), - relayAuth = relayAuthStore(npub).load(), - feedVisibility = feedVisibilityStore(npub).load(), - notificationPrefs = notificationPrefsStore(npub).load(), - ) - - private fun followListMigration(npub: String) = - CopyOnceMigration("migrated.followLists") { out -> - withContext(Dispatchers.IO) { - val legacy = encryptedPreferences(npub) - FollowListSlot.entries.forEach { slot -> - legacy.getString(slot.prefKey, null)?.let { out[slot.key] = it } - } - } - } - - private fun latestEventMigration(npub: String) = - CopyOnceMigration("migrated.latestEvents") { out -> - withContext(Dispatchers.IO) { - val legacy = encryptedPreferences(npub) - LatestEventSlot.entries.forEach { slot -> - legacy.getString(slot.prefKey, null)?.let { out[slot.key] = it } - } - } - } + private suspend fun loadAccountStores( + npub: String, + legacyIdentity: () -> AccountIdentity, + ) = AccountStoreData( + identity = identityStore(npub).load().orIfUnusable(legacyIdentity), + followLists = followListStore(npub).load(), + latestEvents = latestEventStore(npub).load(), + uploadSettings = uploadSettingsStore(npub).load(), + dialogDismissal = dialogDismissalStore(npub).load(), + relayAuth = relayAuthStore(npub).load(), + feedVisibility = feedVisibilityStore(npub).load(), + notificationPrefs = notificationPrefsStore(npub).load(), + ) // NOT migrated to DataStore, and cannot be: DataStore is suspend-only, while // NotificationRelayService.isEnabled(context) is a synchronous Boolean read @@ -796,6 +783,18 @@ object LocalPreferences { privKeyHex = settings.keyPair.privKey?.toHexKey(), ) } + // Mirrored, not moved: NOSTR_PUBKEY is the one key whose loss empties + // the app, so the legacy write above stays until a release has + // proved this one — see [EncryptedStorage]. + identityStore(settings.keyPair.pubKey.toNpub()).save( + AccountIdentity( + pubKeyHex = settings.keyPair.pubKey.toHexKey(), + loginWithExternalSigner = settings.externalSignerPackageName != null, + externalSignerPackageName = settings.externalSignerPackageName, + localRelayServers = settings.localRelayServers.value, + openBackupConflictsJson = settings.openBackupConflicts().takeIf { it.isNotEmpty() }?.let { BackupConflictStorage.encode(it) }, + ), + ) uploadSettingsStore(settings.keyPair.pubKey.toNpub()).save( UploadSettings( stripLocationOnUpload = settings.stripLocationOnUpload, @@ -915,16 +914,15 @@ object LocalPreferences { suspend fun loadAccountConfigFromEncryptedStorage(): AccountSettings? = currentAccount()?.let { loadAccountConfigFromEncryptedStorage(it) } - fun saveSharedSettings( - sharedSettings: UiSettings, - prefs: SharedPreferences = encryptedPreferences(), - ) { - Log.d("LocalPreferences", "Saving to shared settings") - prefs.edit { - putString(PrefKeys.SHARED_SETTINGS, JsonMapper.toJson(sharedSettings)) - } - } - + /** + * The UI settings as the global `secret_keeper` file holds them. + * + * A migration source only: [UiSharedPreferences] owns these now and writes + * them to its own DataStore, which carries a one-shot copy out of this blob + * for installs that predate it. Nothing writes here any more — the matching + * `saveSharedSettings` was removed once it had no callers — but the read + * stays for good, like every other legacy reader; see [EncryptedStorage]. + */ fun loadSharedSettings(prefs: SharedPreferences = encryptedPreferences()): UiSettings? { Log.d("LocalPreferences", "Load shared settings") with(prefs) { @@ -953,10 +951,9 @@ object LocalPreferences { private suspend fun hasBackedUpKeysFlow(npub: String): MutableStateFlow = hasBackedUpKeysMutex.withLock { hasBackedUpKeysFlows.getOrPut(npub) { - val stored = - withContext(Dispatchers.IO) { - encryptedPreferences(npub).getBoolean(PrefKeys.HAS_BACKED_UP_KEYS, true) - } + // Absent reads as true in both stores, so a store that cannot be + // read leaves the nudge off rather than showing it to everyone. + val stored = withContext(Dispatchers.IO) { identityStore(npub).hasBackedUpKeys() } MutableStateFlow(stored) } } @@ -969,7 +966,10 @@ object LocalPreferences { npub: String, ) { withContext(Dispatchers.IO) { + // Legacy write kept alongside the new one, as for the rest of the + // identity group — see [EncryptedStorage]. encryptedPreferences(npub).edit { putBoolean(PrefKeys.HAS_BACKED_UP_KEYS, value) } + identityStore(npub).setHasBackedUpKeys(value) } hasBackedUpKeysFlow(npub).value = value } @@ -991,6 +991,12 @@ object LocalPreferences { // raced in before the per-npub file finished being written. if (accountSettings != null) { cachedAccounts.put(npub, accountSettings) + + // Everything this account has is now migrated and just been + // read back, which is the only moment the legacy file can be + // shown to be redundant. It will not be, yet — see + // [LEGACY_WRITES_RETIRED]. + legacyCleanup.deleteIfVerified(npub) } return@withContext accountSettings @@ -998,26 +1004,52 @@ object LocalPreferences { } } - private suspend fun innerLoadCurrentAccountFromEncryptedStorage(npub: String?): AccountSettings? { + private suspend fun innerLoadCurrentAccountFromEncryptedStorage(npub: String): AccountSettings? { Log.d("LocalPreferences") { "Load account from file $npub" } val startedAtMs = TimeUtils.nowMillis() val result = withContext(Dispatchers.IO) { return@withContext with(encryptedPreferences(npub)) { Log.d("LocalPreferences") { "Load account from file $npub - opened file" } - // pubKey first: the key store is keyed by npub, which is derived - // from it, and this is the same npub the save side writes under. - val pubKey = getString(PrefKeys.NOSTR_PUBKEY, null) ?: return@with null + // Every store this account has, read in one hop — including + // the identity the rest of this function is derived from, so + // that read does not cost its own state in the generated + // coroutine state machine (see [AccountStoreData]). + // + // Keyed by the npub handed in, which is the npub the save side + // writes under and the name of the legacy file just opened. The + // identity falls back to that file when its store cannot + // produce a pubkey: an account without one vanishes from the + // app entirely, private key intact. + val stores = + loadAccountStores(npub) { + AccountIdentity( + pubKeyHex = getString(PrefKeys.NOSTR_PUBKEY, null), + loginWithExternalSigner = getBoolean(PrefKeys.LOGIN_WITH_EXTERNAL_SIGNER, false), + externalSignerPackageName = getString(PrefKeys.SIGNER_PACKAGE_NAME, null), + localRelayServers = getStringSet(PrefKeys.LOCAL_RELAY_SERVERS, null) ?: setOf(), + openBackupConflictsJson = getString(PrefKeys.OPEN_BACKUP_CONFLICTS, null), + ) + } + val identity = stores.identity + val pubKey = identity.pubKeyHex ?: return@with null val privKey = accountKeyStore.read( npub = pubKey.hexToByteArray().toNpub(), legacyValue = getString(PrefKeys.NOSTR_PRIVKEY, null), ) - val externalSignerPackageName = getString(PrefKeys.SIGNER_PACKAGE_NAME, null) ?: if (getBoolean(PrefKeys.LOGIN_WITH_EXTERNAL_SIGNER, false)) "com.greenart7c3.nostrsigner" else null + val externalSignerPackageName = identity.externalSignerPackageName ?: if (identity.loginWithExternalSigner) "com.greenart7c3.nostrsigner" else null val keyPair = KeyPair(privKey = privKey?.hexToByteArray(), pubKey = pubKey.hexToByteArray()) - val stores = loadAccountStores(keyPair.pubKey.toNpub()) + // The npub handed in names the file just read, and the save + // side writes every store under the npub derived from the + // pubkey inside it, so the two are the same by construction. + // Say so if they ever are not: it would mean this load is + // reading stores that a save never wrote. + if (keyPair.pubKey.toNpub() != npub) { + Log.e("LocalPreferences", "Account file $npub holds pubkey ${keyPair.pubKey.toNpub()}; its stores were read under the file's name", null) + } Log.d("LocalPreferences") { "Load account from file $npub - keys ready" } @@ -1043,7 +1075,7 @@ object LocalPreferences { val dismissedChannelInvites = stores.dialogDismissal.dismissedChannelInvites val mutedPublicChats = stores.dialogDismissal.mutedPublicChats val viewedPollResultNoteIdsStr = stores.dialogDismissal.viewedPollResultNoteIdsJson - val localRelayServers = getStringSet(PrefKeys.LOCAL_RELAY_SERVERS, null) ?: setOf() + val localRelayServers = identity.localRelayServers val followListPrefs = toFollowListPrefs(stores.followLists) @@ -1052,18 +1084,9 @@ object LocalPreferences { val secrets = accountSecretsStore.read( npub = keyPair.pubKey.toNpub(), - legacy = - AccountSecrets( - nip46SignerEnabled = getBoolean(PrefKeys.NIP46_SIGNER_ENABLED, false), - nip46BunkerSecret = getString(PrefKeys.NIP46_BUNKER_SECRET, "") ?: "", - nip46TransportKey = getString(PrefKeys.NIP46_TRANSPORT_KEY, "") ?: "", - nip46SeenRequestIds = getStringSet(PrefKeys.NIP46_SEEN_IDS, null) ?: setOf(), - nwcWalletsJson = getString(PrefKeys.NWC_WALLETS, null), - clinkDebitWalletsJson = getString(PrefKeys.CLINK_DEBIT_WALLETS, null), - defaultPaymentSourceId = getString(PrefKeys.DEFAULT_PAYMENT_SOURCE_ID, null), - legacyDefaultNwcWalletId = getString(PrefKeys.DEFAULT_NWC_WALLET_ID, null), - legacyZapPaymentRequestServer = getString(PrefKeys.ZAP_PAYMENT_REQUEST_SERVER, null), - ), + // Through the shared reader, so the loader and the check + // that gates deleting this file read the same keys. + legacy = readLegacyAccountSecrets(LegacySharedPreferences(this)), ) val nip46SignerEnabled = secrets.nip46SignerEnabled val nip46BunkerSecret = secrets.nip46BunkerSecret @@ -1077,7 +1100,7 @@ object LocalPreferences { val defaultFileServerStr = stores.uploadSettings.defaultFileServerJson val pendingAttestationsStr = getString(PrefKeys.PENDING_ATTESTATIONS, null) - val openBackupConflictsStr = getString(PrefKeys.OPEN_BACKUP_CONFLICTS, null) + val openBackupConflictsStr = identity.openBackupConflictsJson val latestUserMetadataStr = stores.latestEvents[LatestEventSlot.USER_METADATA] val latestContactListStr = stores.latestEvents[LatestEventSlot.CONTACT_LIST] val latestDmRelayListStr = stores.latestEvents[LatestEventSlot.DM_RELAY_LIST] diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyKeyCoverageTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyKeyCoverageTest.kt new file mode 100644 index 0000000000..e218c8be32 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyKeyCoverageTest.kt @@ -0,0 +1,117 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst + +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyAccountSecretNames +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.lang.reflect.Modifier + +/** + * Every key the app has ever written to a `secret_keeper` file has to be + * accounted for somewhere, and this is what says so. + * + * [LegacyPreferenceCleanup] refuses to delete a file holding a key it does not + * recognise, which is the right runtime behaviour but a slow way to find out. + * A key added to `PrefKeys` and to neither a migration table nor the accepted + * list fails here instead — at the commit that adds it, naming it. + */ +class LegacyKeyCoverageTest { + /** Read off the object rather than restated, so the test cannot go stale. */ + private val allPrefKeys: Set = + PrefKeys::class.java.declaredFields + .filter { Modifier.isStatic(it.modifiers) && it.type == String::class.java } + .map { + it.isAccessible = true + it.get(null) as String + }.toSet() + + /** + * Keys of the *global* `secret_keeper` file, which has no per-account + * counterpart and is not what the cleanup deletes. + * `notification_service_enabled` is not even in it — it lives in a plain + * file, deliberately, because it is read synchronously in fresh processes. + */ + private val globalFileKeys = + setOf( + PrefKeys.CURRENT_ACCOUNT, + PrefKeys.SAVED_ACCOUNTS, + PrefKeys.ALL_ACCOUNT_INFO, + PrefKeys.SHARED_SETTINGS, + PrefKeys.NOTIFICATION_SERVICE_ENABLED, + ) + + @Test + fun thePrefKeysListWasActuallyRead() { + assertTrue(allPrefKeys.size.toString(), allPrefKeys.size > 100) + assertTrue(PrefKeys.NOSTR_PUBKEY in allPrefKeys) + } + + @Test + fun everyLegacyKeyIsEitherMigratedOrDeliberatelyDropped() { + val migrated = LegacyAccountKeys.tables.flatMapTo(mutableSetOf()) { it.legacyNames } + val classified = migrated + LegacyAccountSecretNames.all + LegacyAccountKeys.accepted + globalFileKeys + + assertEquals( + "Unclassified legacy keys. Add each to a migration table, or to LegacyAccountKeys.accepted if losing it is deliberate.", + emptySet(), + allPrefKeys - classified, + ) + } + + /** + * The reverse direction: a table claiming a key `PrefKeys` no longer has + * means the copy is reading a name nothing writes. + */ + @Test + fun noTableClaimsAKeyThatNoLongerExists() { + val migrated = LegacyAccountKeys.tables.flatMapTo(mutableSetOf()) { it.legacyNames } + + assertEquals(emptySet(), migrated - allPrefKeys) + assertEquals(emptySet(), LegacyAccountSecretNames.all - allPrefKeys) + assertEquals(emptySet(), LegacyAccountKeys.accepted - allPrefKeys) + } + + /** + * The seven that were still read only from the legacy file. Named + * individually because `nostr_pubkey` is the one whose loss empties the + * app: without it the loader returns null and the account disappears, with + * its private key sitting safe and unreachable in the key store. + */ + @Test + fun theLastSevenKeysAreMigrated() { + val migrated = LegacyAccountKeys.tables.flatMapTo(mutableSetOf()) { it.legacyNames } + + listOf( + PrefKeys.NOSTR_PUBKEY, + PrefKeys.LOGIN_WITH_EXTERNAL_SIGNER, + PrefKeys.SIGNER_PACKAGE_NAME, + PrefKeys.HAS_BACKED_UP_KEYS, + PrefKeys.LOCAL_RELAY_SERVERS, + PrefKeys.OPEN_BACKUP_CONFLICTS, + ).forEach { assertTrue(it, it in migrated) } + + // The seventh is global, and moved into the UI settings DataStore + // rather than a per-account one. + assertTrue(PrefKeys.SHARED_SETTINGS in globalFileKeys) + } +} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanupTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanupTest.kt new file mode 100644 index 0000000000..c79704c29a --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanupTest.kt @@ -0,0 +1,321 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst + +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.booleanPreferencesKey +import androidx.datastore.preferences.core.emptyPreferences +import androidx.datastore.preferences.core.mutablePreferencesOf +import androidx.datastore.preferences.core.stringPreferencesKey +import com.vitorpamplona.amethyst.commons.model.preferences.AccountSecrets +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyBooleanKey +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyKeyTable +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyPreferenceSource +import com.vitorpamplona.amethyst.commons.model.preferences.LegacyStringKey +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +private const val NPUB = "npub1test" + +private class MapSource( + private val values: Map, +) : LegacyPreferenceSource { + override fun keys() = values.keys + + override fun getBoolean(name: String) = values[name] as Boolean? + + override fun getString(name: String) = values[name] as String? + + @Suppress("UNCHECKED_CAST") + override fun getStringSet(name: String) = values[name] as Set? +} + +private class FakeFiles( + private val values: Map, +) : LegacyAccountFiles { + var deleted = false + private set + + var present = true + + override fun source(npub: String) = MapSource(values) + + override fun exists(npub: String) = present + + override suspend fun delete(npub: String): Boolean { + deleted = true + present = false + return true + } +} + +private class FakeSecrets( + private val stored: AccountSecrets? = AccountSecrets(), + private val key: String? = null, + private val throws: Boolean = false, +) : MigratedSecrets { + override suspend fun secrets(npub: String): AccountSecrets? { + if (throws) throw IllegalStateException("keystore unavailable") + return stored + } + + override suspend fun privateKey(npub: String): String? { + if (throws) throw IllegalStateException("keystore unavailable") + return key + } +} + +/** + * The gate in front of deleting an account's `secret_keeper_` file. + * + * Every test here is a way the deletion could destroy something, so the + * assertions are mostly that it did *not* happen. + */ +class LegacyPreferenceCleanupTest { + private val flag = booleanPreferencesKey("flag") + private val text = stringPreferencesKey("text") + + private val table = + LegacyKeyTable( + "migrated.group", + listOf(LegacyBooleanKey("legacy_flag", flag), LegacyStringKey("legacy_text", text)), + ) + + private val migrated = mutablePreferencesOf().also { it[booleanPreferencesKey("migrated.group")] = true } + + private fun cleanup( + values: Map, + current: Preferences = migrated, + secrets: MigratedSecrets = FakeSecrets(), + files: FakeFiles = FakeFiles(values), + retired: Boolean = true, + accepted: Set = setOf("pending_attestations"), + ) = files to + LegacyPreferenceCleanup( + tables = listOf(table), + accepted = accepted, + files = files, + currentStore = { current }, + secrets = secrets, + legacyWritesRetired = retired, + ) + + @Test + fun deletesOnceEverythingIsAccountedFor() = + runTest { + val (files, subject) = cleanup(mapOf("legacy_flag" to true, "pending_attestations" to "[]")) + + assertEquals(emptyList(), subject.verify(NPUB)) + assertEquals(LegacyCleanupResult.Deleted, subject.deleteIfVerified(NPUB)) + assertTrue(files.deleted) + } + + /** + * The hole a hand-maintained checklist leaves: a key added later that no + * migration carries. The check runs from the file's own keys so that it + * cannot be missed. + */ + @Test + fun anUnrecognisedKeyStopsTheDeletion() = + runTest { + val (files, subject) = cleanup(mapOf("legacy_flag" to true, "something_new" to "value")) + + val result = subject.deleteIfVerified(NPUB) + + assertEquals(LegacyCleanupResult.Kept(listOf("no migration claims 'something_new'")), result) + assertTrue(!files.deleted) + } + + @Test + fun aCopyThatHasNotRunStopsTheDeletion() = + runTest { + val (files, subject) = cleanup(mapOf("legacy_flag" to true), current = emptyPreferences()) + + val result = subject.deleteIfVerified(NPUB) + + assertEquals(LegacyCleanupResult.Kept(listOf("the 'migrated.group' copy has not run")), result) + assertTrue(!files.deleted) + } + + /** + * A file that never held a group's keys has nothing for that copy to prove, + * so an account predating a setting is not held back by it forever. + */ + @Test + fun aGroupTheFileNeverHeldDoesNotBlock() = + runTest { + val (_, subject) = cleanup(mapOf("pending_attestations" to "[]"), current = emptyPreferences()) + + assertEquals(emptyList(), subject.verify(NPUB)) + } + + @Test + fun secretsThatHaveNotBeenCopiedStopTheDeletion() = + runTest { + val (files, subject) = cleanup(mapOf("legacy_flag" to true), secrets = FakeSecrets(stored = null)) + + val result = subject.deleteIfVerified(NPUB) + + assertEquals(LegacyCleanupResult.Kept(listOf("the secrets have not been copied across")), result) + assertTrue(!files.deleted) + } + + /** Both stores are still written, so a disagreement means a write was lost. */ + @Test + fun secretsThatDisagreeStopTheDeletion() = + runTest { + val (files, subject) = + cleanup( + mapOf("legacy_flag" to true, "nip46BunkerSecret" to "from-the-file"), + secrets = FakeSecrets(stored = AccountSecrets(nip46BunkerSecret = "stale")), + ) + + val result = subject.deleteIfVerified(NPUB) + + assertEquals( + LegacyCleanupResult.Kept(listOf("the stored secrets differ from the legacy file: nip46BunkerSecret")), + result, + ) + assertTrue(!files.deleted) + } + + /** The values themselves are bunker secrets and wallet strings. */ + @Test + fun aSecretsMismatchNamesTheFieldAndNotTheValue() = + runTest { + val (_, subject) = + cleanup( + mapOf("nwcWallets" to "nostr+walletconnect://deadbeef?secret=hunter2"), + secrets = FakeSecrets(stored = AccountSecrets()), + ) + + val reason = subject.verify(NPUB).single() + + assertTrue(reason, !reason.contains("hunter2")) + assertTrue(reason, reason.contains("nwcWallets")) + } + + @Test + fun aPrivateKeyThatHasNotBeenCopiedStopsTheDeletion() = + runTest { + val (files, subject) = + cleanup( + mapOf("nostr_privkey" to "abc123"), + secrets = FakeSecrets(key = null), + ) + + val result = subject.deleteIfVerified(NPUB) + + assertEquals(LegacyCleanupResult.Kept(listOf("the private key has not been copied across")), result) + assertTrue(!files.deleted) + } + + @Test + fun aPrivateKeyThatDisagreesStopsTheDeletion() = + runTest { + val (_, subject) = cleanup(mapOf("nostr_privkey" to "abc123"), secrets = FakeSecrets(key = "def456")) + + assertEquals(listOf("the stored private key differs from the legacy file"), subject.verify(NPUB)) + } + + @Test + fun aMatchingPrivateKeyPasses() = + runTest { + val (_, subject) = cleanup(mapOf("nostr_privkey" to "abc123"), secrets = FakeSecrets(key = "abc123")) + + assertEquals(emptyList(), subject.verify(NPUB)) + } + + /** + * An external-signer account has no private key in either store, and must + * not be held back for the one it never had. + */ + @Test + fun anAccountWithNoPrivateKeyIsNotHeldBack() = + runTest { + val (_, subject) = cleanup(mapOf("legacy_flag" to true), secrets = FakeSecrets(key = null)) + + assertEquals(emptyList(), subject.verify(NPUB)) + } + + /** "The check itself failed" is not "the check passed". */ + @Test + fun aStoreThatCannotBeReadStopsTheDeletion() = + runTest { + val (files, subject) = cleanup(mapOf("nostr_privkey" to "abc123"), secrets = FakeSecrets(throws = true)) + + val result = subject.deleteIfVerified(NPUB) + + assertEquals( + LegacyCleanupResult.Kept(listOf("the secrets store could not be read", "the key store could not be read")), + result, + ) + assertTrue(!files.deleted) + } + + /** + * While the app still mirrors into this file, deleting it achieves nothing + * — the next save recreates it — and would look like it had worked. + */ + @Test + fun nothingIsDeletedWhileTheLegacyFileIsStillWritten() = + runTest { + val (files, subject) = cleanup(mapOf("legacy_flag" to true), retired = false) + + val result = subject.deleteIfVerified(NPUB) + + assertEquals(LegacyCleanupResult.Kept(listOf(LegacyPreferenceCleanup.STILL_WRITTEN)), result) + assertTrue(!files.deleted) + } + + @Test + fun anAccountWithNoLegacyFileIsAlreadyDone() = + runTest { + val files = FakeFiles(emptyMap()).also { it.present = false } + val (_, subject) = cleanup(emptyMap(), files = files) + + assertEquals(LegacyCleanupResult.NothingToDelete, subject.deleteIfVerified(NPUB)) + } + + /** Every reason is reported, so one fix does not merely reveal the next. */ + @Test + fun everyReasonIsReportedAtOnce() = + runTest { + val (_, subject) = + cleanup( + mapOf("legacy_flag" to true, "mystery" to "x", "nostr_privkey" to "abc123"), + current = emptyPreferences(), + secrets = FakeSecrets(stored = null, key = null), + ) + + assertEquals( + listOf( + "no migration claims 'mystery'", + "the 'migrated.group' copy has not run", + "the secrets have not been copied across", + "the private key has not been copied across", + ), + subject.verify(NPUB), + ) + } +}