diff --git a/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs b/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs index bb4fabb357..5bb59dfb17 100644 --- a/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs +++ b/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs @@ -640,3 +640,62 @@ test("readChannelMutesStore: scoped key takes precedence over legacy key", () => writeChannelMutesStore(pubkey, scoped, relay); assert.deepEqual(readChannelMutesStore(pubkey, relay), scoped); }); + +// Migration failure safety (Thufir r6 IMPORTANT): legacy data must be exposed +// to bootstrap ONLY once the legacy key is provably gone. If the scoped write +// throws, or the legacy delete does not take, the reader must roll back and +// return DEFAULT_STORE so no relay publishes legacy prefs while the legacy key +// still holds them (which would let a second relay import the same value). + +test("readChannelMutesStore: scoped-write failure returns DEFAULT and leaves neither relay able to seed the legacy value", () => { + const pubkey = "pk-mute-migrate-writefail"; + const relayA = "wss://relay-a-writefail.example.com"; + const relayB = "wss://relay-b-writefail.example.com"; + const legacy = makeMuteStore({ chw: MUTE_E(true, 500, 2) }); + writeChannelMutesStore(pubkey, legacy); + const scopedA = storageKey(pubkey, relayA); + const origSet = window.localStorage.setItem; + window.localStorage.setItem = (k, v) => { + if (k === scopedA) throw new Error("QuotaExceededError"); + return origSet.call(window.localStorage, k, v); + }; + try { + // Relay A cannot claim the legacy value — it must NOT be exposed to publish. + assert.deepEqual(readChannelMutesStore(pubkey, relayA), DEFAULT_STORE); + // Rolled back: no partial scoped copy left behind for A. + assert.equal(window.localStorage.getItem(scopedA), null); + } finally { + window.localStorage.setItem = origSet; + } + // Legacy key survived (migration is retryable), and relay B likewise cannot + // be seeded — the value is claimed by whichever relay first deletes it. + assert.notEqual(window.localStorage.getItem(storageKey(pubkey)), null); + assert.deepEqual(readChannelMutesStore(pubkey, relayB), legacy); + // Once B claims it, the legacy key is gone and A can no longer seed. + assert.equal(window.localStorage.getItem(storageKey(pubkey)), null); + assert.deepEqual(readChannelMutesStore(pubkey, relayA), DEFAULT_STORE); +}); + +test("readChannelMutesStore: a legacy delete that does not take rolls back and returns DEFAULT", () => { + const pubkey = "pk-mute-migrate-delfail"; + const relay = "wss://relay-delfail.example.com"; + const legacy = makeMuteStore({ chd: MUTE_E(true, 700, 3) }); + writeChannelMutesStore(pubkey, legacy); + const legacyKey = storageKey(pubkey); + const scoped = storageKey(pubkey, relay); + const origRemove = window.localStorage.removeItem; + window.localStorage.removeItem = (k) => { + if (k === legacyKey) return; // silently no-op: delete "does not take" + return origRemove.call(window.localStorage, k); + }; + try { + assert.deepEqual(readChannelMutesStore(pubkey, relay), DEFAULT_STORE); + // Partial scoped copy rolled back so the migration stays retryable. + assert.equal(window.localStorage.getItem(scoped), null); + } finally { + window.localStorage.removeItem = origRemove; + } + // With the delete working again, the migration completes cleanly. + assert.deepEqual(readChannelMutesStore(pubkey, relay), legacy); + assert.equal(window.localStorage.getItem(legacyKey), null); +}); diff --git a/desktop/src/features/sidebar/lib/channelMutesStorage.ts b/desktop/src/features/sidebar/lib/channelMutesStorage.ts index 27b80603a2..407ab5f8d3 100644 --- a/desktop/src/features/sidebar/lib/channelMutesStorage.ts +++ b/desktop/src/features/sidebar/lib/channelMutesStorage.ts @@ -118,11 +118,21 @@ function parseRaw(raw: string | null): ChannelMuteStore | null { * Read the mute store for `pubkey` scoped to `relayUrl`. * * On first access for a scoped key, migrates any existing data from the legacy - * pubkey-only key so users don't lose their mutes on upgrade. After a - * successful migration write the legacy key is deleted, making this a globally - * one-time operation — a subsequent empty scoped-key read on a DIFFERENT relay - * won't see legacy data, so a non-empty store from relay A can never seed onto - * a first-visited relay B (Carl P1). + * pubkey-only key so users don't lose their mutes on upgrade. Migrated data is + * exposed to the caller (and thus to publish) ONLY once the legacy key is + * provably gone: we write the scoped key, delete the legacy key, and confirm + * the delete took. On any write/delete failure we roll back the partial scoped + * copy and return `DEFAULT_STORE`, leaving the migration safely retryable — so + * legacy preferences can never be exposed while still importable by a second + * relay, which would let relay A's mutes seed-publish onto relay B (Carl P1). + * + * This makes the migration first-scope-only, not merely "usually deletes + * afterward." Concurrent claimants are safe: every window (main and huddle) + * resolves the same app-wide active community (`buzz-active-community-id`), so + * at any instant they scope to the same relay and any race migrates the + * identical legacy value to the identical scoped key (idempotent). A second + * relay only enters play after a community switch, which happens strictly after + * the first scoped read consumed the legacy key. */ export function readChannelMutesStore( pubkey: string, @@ -143,15 +153,7 @@ export function readChannelMutesStore( const legacyKey = storageKey(pubkey); const migrated = parseRaw(window.localStorage.getItem(legacyKey)); if (migrated && Object.keys(migrated.channels).length > 0) { - // Persist under the scoped key and remove the legacy key so this - // migration cannot fire again for any other relay scope. - try { - window.localStorage.setItem(key, JSON.stringify(migrated)); - window.localStorage.removeItem(legacyKey); - } catch { - // Ignore write failures — we still return the migrated value. - } - return migrated; + return migrateLegacyStore(key, legacyKey, migrated); } } @@ -161,6 +163,38 @@ export function readChannelMutesStore( } } +/** + * Claim the legacy value for the scoped key: persist it, delete the legacy key, + * and expose it only if the legacy key is confirmed gone. Any failure rolls + * back the partial scoped copy and returns `DEFAULT_STORE` so the migration is + * retryable and no relay imports legacy data the legacy key still holds. + */ +function migrateLegacyStore( + key: string, + legacyKey: string, + migrated: ChannelMuteStore, +): ChannelMuteStore { + try { + window.localStorage.setItem(key, JSON.stringify(migrated)); + window.localStorage.removeItem(legacyKey); + if (window.localStorage.getItem(legacyKey) !== null) { + // Delete did not take — do not expose data another relay could import. + window.localStorage.removeItem(key); + return DEFAULT_STORE; + } + return migrated; + } catch { + // Scoped write or legacy delete threw — roll back the partial scoped copy + // so the scoped key stays empty and the migration can retry later. + try { + window.localStorage.removeItem(key); + } catch { + // Best effort; a stuck partial copy self-heals on the next read. + } + return DEFAULT_STORE; + } +} + export function boundMuteStore( store: ChannelMuteStore, preservedKey?: string, diff --git a/desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs b/desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs index f28108e5b5..cdf0a60eef 100644 --- a/desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs +++ b/desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs @@ -357,6 +357,71 @@ test("readChannelSectionsStore: scoped key takes precedence over legacy key afte assert.deepEqual(result, newStore); }); +// Migration failure safety (Thufir r6 IMPORTANT): legacy data must be exposed +// to bootstrap ONLY once the legacy key is provably gone. If the scoped write +// throws, or the legacy delete does not take, the reader must roll back and +// return DEFAULT_STORE so no relay publishes legacy prefs while the legacy key +// still holds them (which would let a second relay import the same value). + +test("readChannelSectionsStore: scoped-write failure returns DEFAULT and leaves neither relay able to seed the legacy value", () => { + const pubkey = "pk-migrate-writefail"; + const relayA = "wss://relay-a-writefail.example.com"; + const relayB = "wss://relay-b-writefail.example.com"; + const legacy = makeStore({ + sections: [makeSection({ id: "sw", name: "Legacy", order: 0 })], + assignments: {}, + }); + writeChannelSectionsStore(pubkey, legacy); + const scopedA = storageKey(pubkey, relayA); + const origSet = window.localStorage.setItem; + window.localStorage.setItem = (k, v) => { + if (k === scopedA) throw new Error("QuotaExceededError"); + return origSet.call(window.localStorage, k, v); + }; + try { + // Relay A cannot claim the legacy value — it must NOT be exposed to publish. + assert.deepEqual(readChannelSectionsStore(pubkey, relayA), DEFAULT_STORE); + // Rolled back: no partial scoped copy left behind for A. + assert.equal(window.localStorage.getItem(scopedA), null); + } finally { + window.localStorage.setItem = origSet; + } + // Legacy key survived (migration is retryable), and relay B likewise cannot + // be seeded — the value is claimed by whichever relay first deletes it. + assert.notEqual(window.localStorage.getItem(storageKey(pubkey)), null); + assert.deepEqual(readChannelSectionsStore(pubkey, relayB), legacy); + // Once B claims it, the legacy key is gone and A can no longer seed. + assert.equal(window.localStorage.getItem(storageKey(pubkey)), null); + assert.deepEqual(readChannelSectionsStore(pubkey, relayA), DEFAULT_STORE); +}); + +test("readChannelSectionsStore: a legacy delete that does not take rolls back and returns DEFAULT", () => { + const pubkey = "pk-migrate-delfail"; + const relay = "wss://relay-delfail.example.com"; + const legacy = makeStore({ + sections: [makeSection({ id: "sd", name: "Legacy", order: 0 })], + assignments: {}, + }); + writeChannelSectionsStore(pubkey, legacy); + const legacyKey = storageKey(pubkey); + const scoped = storageKey(pubkey, relay); + const origRemove = window.localStorage.removeItem; + window.localStorage.removeItem = (k) => { + if (k === legacyKey) return; // silently no-op: delete "does not take" + return origRemove.call(window.localStorage, k); + }; + try { + assert.deepEqual(readChannelSectionsStore(pubkey, relay), DEFAULT_STORE); + // Partial scoped copy rolled back so the migration stays retryable. + assert.equal(window.localStorage.getItem(scoped), null); + } finally { + window.localStorage.removeItem = origRemove; + } + // With the delete working again, the migration completes cleanly. + assert.deepEqual(readChannelSectionsStore(pubkey, relay), legacy); + assert.equal(window.localStorage.getItem(legacyKey), null); +}); + test("parseChannelSectionPayload: preserves icon field when present", () => { const payload = { version: 1, diff --git a/desktop/src/features/sidebar/lib/channelSectionsStorage.ts b/desktop/src/features/sidebar/lib/channelSectionsStorage.ts index 9d37866b2c..c538098405 100644 --- a/desktop/src/features/sidebar/lib/channelSectionsStorage.ts +++ b/desktop/src/features/sidebar/lib/channelSectionsStorage.ts @@ -141,12 +141,22 @@ function parseRaw(raw: string | null): ChannelSectionStore | null { /** * Read the section store for `pubkey` scoped to `relayUrl`. * - * On first access for a scoped key, migrates any existing data from the - * legacy pubkey-only key so users don't lose their sections on upgrade. - * After a successful migration write the legacy key is deleted, making - * this a globally one-time operation — subsequent empty scoped-key reads - * (i.e. a different relay) won't see legacy data and won't trigger - * cross-relay contamination via the seed-publish path. + * On first access for a scoped key, migrates any existing data from the legacy + * pubkey-only key so users don't lose their sections on upgrade. Migrated data + * is exposed to the caller (and thus to publish) ONLY once the legacy key is + * provably gone: we write the scoped key, delete the legacy key, and confirm + * the delete took. On any write/delete failure we roll back the partial scoped + * copy and return `DEFAULT_STORE`, leaving the migration safely retryable — so + * legacy sections can never be exposed while still importable by a second + * relay, which would let relay A's sections seed-publish onto relay B (Carl P1). + * + * This makes the migration first-scope-only, not merely "usually deletes + * afterward." Concurrent claimants are safe: every window (main and huddle) + * resolves the same app-wide active community (`buzz-active-community-id`), so + * at any instant they scope to the same relay and any race migrates the + * identical legacy value to the identical scoped key (idempotent). A second + * relay only enters play after a community switch, which happens strictly after + * the first scoped read consumed the legacy key. */ export function readChannelSectionsStore( pubkey: string, @@ -168,15 +178,7 @@ export function readChannelSectionsStore( const legacyRaw = window.localStorage.getItem(legacyKey); const migrated = parseRaw(legacyRaw); if (migrated && migrated.sections.length > 0) { - // Persist under the scoped key and remove the legacy key so this - // migration cannot fire again for any other relay scope. - try { - window.localStorage.setItem(key, JSON.stringify(migrated)); - window.localStorage.removeItem(legacyKey); - } catch { - // Ignore write failures — we still return the migrated value. - } - return migrated; + return migrateLegacyStore(key, legacyKey, migrated); } } @@ -186,6 +188,38 @@ export function readChannelSectionsStore( } } +/** + * Claim the legacy value for the scoped key: persist it, delete the legacy key, + * and expose it only if the legacy key is confirmed gone. Any failure rolls + * back the partial scoped copy and returns `DEFAULT_STORE` so the migration is + * retryable and no relay imports legacy data the legacy key still holds. + */ +function migrateLegacyStore( + key: string, + legacyKey: string, + migrated: ChannelSectionStore, +): ChannelSectionStore { + try { + window.localStorage.setItem(key, JSON.stringify(migrated)); + window.localStorage.removeItem(legacyKey); + if (window.localStorage.getItem(legacyKey) !== null) { + // Delete did not take — do not expose data another relay could import. + window.localStorage.removeItem(key); + return DEFAULT_STORE; + } + return migrated; + } catch { + // Scoped write or legacy delete threw — roll back the partial scoped copy + // so the scoped key stays empty and the migration can retry later. + try { + window.localStorage.removeItem(key); + } catch { + // Best effort; a stuck partial copy self-heals on the next read. + } + return DEFAULT_STORE; + } +} + export function writeChannelSectionsStore( pubkey: string, store: ChannelSectionStore, diff --git a/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs b/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs index 2875e4821b..70b00454d6 100644 --- a/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs +++ b/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs @@ -643,3 +643,62 @@ test("readChannelStarsStore: scoped key takes precedence over legacy key", () => writeChannelStarsStore(pubkey, scoped, relay); assert.deepEqual(readChannelStarsStore(pubkey, relay), scoped); }); + +// Migration failure safety (Thufir r6 IMPORTANT): legacy data must be exposed +// to bootstrap ONLY once the legacy key is provably gone. If the scoped write +// throws, or the legacy delete does not take, the reader must roll back and +// return DEFAULT_STORE so no relay publishes legacy prefs while the legacy key +// still holds them (which would let a second relay import the same value). + +test("readChannelStarsStore: scoped-write failure returns DEFAULT and leaves neither relay able to seed the legacy value", () => { + const pubkey = "pk-star-migrate-writefail"; + const relayA = "wss://relay-a-writefail.example.com"; + const relayB = "wss://relay-b-writefail.example.com"; + const legacy = makeStarStore({ chw: STAR_E(true, 500, 2) }); + writeChannelStarsStore(pubkey, legacy); + const scopedA = storageKey(pubkey, relayA); + const origSet = window.localStorage.setItem; + window.localStorage.setItem = (k, v) => { + if (k === scopedA) throw new Error("QuotaExceededError"); + return origSet.call(window.localStorage, k, v); + }; + try { + // Relay A cannot claim the legacy value — it must NOT be exposed to publish. + assert.deepEqual(readChannelStarsStore(pubkey, relayA), DEFAULT_STORE); + // Rolled back: no partial scoped copy left behind for A. + assert.equal(window.localStorage.getItem(scopedA), null); + } finally { + window.localStorage.setItem = origSet; + } + // Legacy key survived (migration is retryable), and relay B likewise cannot + // be seeded — the value is claimed by whichever relay first deletes it. + assert.notEqual(window.localStorage.getItem(storageKey(pubkey)), null); + assert.deepEqual(readChannelStarsStore(pubkey, relayB), legacy); + // Once B claims it, the legacy key is gone and A can no longer seed. + assert.equal(window.localStorage.getItem(storageKey(pubkey)), null); + assert.deepEqual(readChannelStarsStore(pubkey, relayA), DEFAULT_STORE); +}); + +test("readChannelStarsStore: a legacy delete that does not take rolls back and returns DEFAULT", () => { + const pubkey = "pk-star-migrate-delfail"; + const relay = "wss://relay-delfail.example.com"; + const legacy = makeStarStore({ chd: STAR_E(true, 700, 3) }); + writeChannelStarsStore(pubkey, legacy); + const legacyKey = storageKey(pubkey); + const scoped = storageKey(pubkey, relay); + const origRemove = window.localStorage.removeItem; + window.localStorage.removeItem = (k) => { + if (k === legacyKey) return; // silently no-op: delete "does not take" + return origRemove.call(window.localStorage, k); + }; + try { + assert.deepEqual(readChannelStarsStore(pubkey, relay), DEFAULT_STORE); + // Partial scoped copy rolled back so the migration stays retryable. + assert.equal(window.localStorage.getItem(scoped), null); + } finally { + window.localStorage.removeItem = origRemove; + } + // With the delete working again, the migration completes cleanly. + assert.deepEqual(readChannelStarsStore(pubkey, relay), legacy); + assert.equal(window.localStorage.getItem(legacyKey), null); +}); diff --git a/desktop/src/features/sidebar/lib/channelStarsStorage.ts b/desktop/src/features/sidebar/lib/channelStarsStorage.ts index b6f15783cc..164b4d93de 100644 --- a/desktop/src/features/sidebar/lib/channelStarsStorage.ts +++ b/desktop/src/features/sidebar/lib/channelStarsStorage.ts @@ -118,11 +118,21 @@ function parseRaw(raw: string | null): ChannelStarStore | null { * Read the star store for `pubkey` scoped to `relayUrl`. * * On first access for a scoped key, migrates any existing data from the legacy - * pubkey-only key so users don't lose their stars on upgrade. After a - * successful migration write the legacy key is deleted, making this a globally - * one-time operation — a subsequent empty scoped-key read on a DIFFERENT relay - * won't see legacy data, so a non-empty store from relay A can never seed onto - * a first-visited relay B (Carl P1). + * pubkey-only key so users don't lose their stars on upgrade. Migrated data is + * exposed to the caller (and thus to publish) ONLY once the legacy key is + * provably gone: we write the scoped key, delete the legacy key, and confirm + * the delete took. On any write/delete failure we roll back the partial scoped + * copy and return `DEFAULT_STORE`, leaving the migration safely retryable — so + * legacy preferences can never be exposed while still importable by a second + * relay, which would let relay A's stars seed-publish onto relay B (Carl P1). + * + * This makes the migration first-scope-only, not merely "usually deletes + * afterward." Concurrent claimants are safe: every window (main and huddle) + * resolves the same app-wide active community (`buzz-active-community-id`), so + * at any instant they scope to the same relay and any race migrates the + * identical legacy value to the identical scoped key (idempotent). A second + * relay only enters play after a community switch, which happens strictly after + * the first scoped read consumed the legacy key. */ export function readChannelStarsStore( pubkey: string, @@ -143,15 +153,7 @@ export function readChannelStarsStore( const legacyKey = storageKey(pubkey); const migrated = parseRaw(window.localStorage.getItem(legacyKey)); if (migrated && Object.keys(migrated.channels).length > 0) { - // Persist under the scoped key and remove the legacy key so this - // migration cannot fire again for any other relay scope. - try { - window.localStorage.setItem(key, JSON.stringify(migrated)); - window.localStorage.removeItem(legacyKey); - } catch { - // Ignore write failures — we still return the migrated value. - } - return migrated; + return migrateLegacyStore(key, legacyKey, migrated); } } @@ -161,6 +163,38 @@ export function readChannelStarsStore( } } +/** + * Claim the legacy value for the scoped key: persist it, delete the legacy key, + * and expose it only if the legacy key is confirmed gone. Any failure rolls + * back the partial scoped copy and returns `DEFAULT_STORE` so the migration is + * retryable and no relay imports legacy data the legacy key still holds. + */ +function migrateLegacyStore( + key: string, + legacyKey: string, + migrated: ChannelStarStore, +): ChannelStarStore { + try { + window.localStorage.setItem(key, JSON.stringify(migrated)); + window.localStorage.removeItem(legacyKey); + if (window.localStorage.getItem(legacyKey) !== null) { + // Delete did not take — do not expose data another relay could import. + window.localStorage.removeItem(key); + return DEFAULT_STORE; + } + return migrated; + } catch { + // Scoped write or legacy delete threw — roll back the partial scoped copy + // so the scoped key stays empty and the migration can retry later. + try { + window.localStorage.removeItem(key); + } catch { + // Best effort; a stuck partial copy self-heals on the next read. + } + return DEFAULT_STORE; + } +} + export function boundStarStore( store: ChannelStarStore, preservedKey?: string,