From 950b8f624764b8d8921fe26b71ef7556e050ae96 Mon Sep 17 00:00:00 2001 From: Duncan Date: Fri, 28 Aug 2026 13:57:29 -0400 Subject: [PATCH] fix(sync): prove legacy migration ownership at read time The migration wrote the scoped key before deleting the legacy key. If the legacy delete threw and the rollback delete also threw, the scoped copy survived and the next read gave it precedence, exposing legacy prefs to seed-publish while the legacy key was still importable by another relay. Move the ownership check into the reader: a scoped copy is exposed only while no importable legacy key remains, so a half-migrated copy can never be read as owned. The claim completes inline once storage recovers. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../sidebar/lib/channelMutesStorage.test.mjs | 31 ++++++ .../sidebar/lib/channelMutesStorage.ts | 98 ++++++++++--------- .../lib/channelSectionsStorage.test.mjs | 34 +++++++ .../sidebar/lib/channelSectionsStorage.ts | 98 ++++++++++--------- .../sidebar/lib/channelStarsStorage.test.mjs | 31 ++++++ .../sidebar/lib/channelStarsStorage.ts | 98 ++++++++++--------- 6 files changed, 254 insertions(+), 136 deletions(-) diff --git a/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs b/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs index 5bb59dfb17..50ab38409a 100644 --- a/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs +++ b/desktop/src/features/sidebar/lib/channelMutesStorage.test.mjs @@ -699,3 +699,34 @@ test("readChannelMutesStore: a legacy delete that does not take rolls back and r assert.deepEqual(readChannelMutesStore(pubkey, relay), legacy); assert.equal(window.localStorage.getItem(legacyKey), null); }); + +test("readChannelMutesStore: legacy delete + rollback both throw — every read returns DEFAULT until storage recovers, never seeds", () => { + const pubkey = "pk-mute-migrate-delrollbackthrow"; + const relay = "wss://relay-delrollbackthrow.example.com"; + const legacy = makeMuteStore({ chr: MUTE_E(true, 900, 4) }); + writeChannelMutesStore(pubkey, legacy); + const legacyKey = storageKey(pubkey); + const scoped = storageKey(pubkey, relay); + const origRemove = window.localStorage.removeItem; + // removeItem throws globally: the legacy delete AND the scoped rollback both + // throw, so a scoped copy the migration wrote survives on disk. + window.localStorage.removeItem = () => { + throw new Error("SecurityError"); + }; + try { + // First read: scoped write took, legacy delete threw, rollback threw. + assert.deepEqual(readChannelMutesStore(pubkey, relay), DEFAULT_STORE); + // A stuck scoped copy survives, but the reader must NOT expose it while the + // legacy key still holds importable data — the seam Thufir found. + assert.deepEqual(readChannelMutesStore(pubkey, relay), DEFAULT_STORE); + // Legacy key is intact, so the claim stays retryable rather than lost. + assert.notEqual(window.localStorage.getItem(legacyKey), null); + } finally { + window.localStorage.removeItem = origRemove; + } + // Storage recovers: the next read finishes the claim from the stuck scoped + // copy, deletes the legacy key, and only then exposes the value. + assert.deepEqual(readChannelMutesStore(pubkey, relay), legacy); + assert.equal(window.localStorage.getItem(legacyKey), null); + assert.notEqual(window.localStorage.getItem(scoped), null); +}); diff --git a/desktop/src/features/sidebar/lib/channelMutesStorage.ts b/desktop/src/features/sidebar/lib/channelMutesStorage.ts index 407ab5f8d3..a86edae07d 100644 --- a/desktop/src/features/sidebar/lib/channelMutesStorage.ts +++ b/desktop/src/features/sidebar/lib/channelMutesStorage.ts @@ -117,22 +117,26 @@ 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. 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). + * Enforces one invariant on every read: a scoped copy is exposed only while no + * importable legacy (pubkey-only) key remains. If a non-empty legacy key still + * exists — whether because this is the first scoped read or because a prior + * migration wrote the scoped copy but could not delete the legacy key — we + * finish the claim inline: write the scoped copy if absent, delete the legacy + * key, and expose the value only once that delete provably took. On any storage + * failure we return `DEFAULT_STORE`, which cannot seed-publish (bootstrap gates + * on a non-empty local store), leaving the claim to complete on a later read + * once storage recovers. This makes "scoped data is owned only when the legacy + * key is gone" a property the reader proves every read, not a promise a + * one-shot migration hopes to keep under failing storage — so relay A's mutes + * can never seed-publish onto a first-visited 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. + * Concurrent windows are safe without an atomic claim. Each window (main and + * huddle) takes its scope from the app-wide active community at mount, so the + * first scoped read for a live legacy key runs before any second relay scope + * can exist; that read either claims the legacy key or hits the failure path + * above. A race migrates the identical legacy value to the identical scoped key + * (idempotent), and the loser's post-delete confirmation fails and exposes + * `DEFAULT_STORE`. */ export function readChannelMutesStore( pubkey: string, @@ -140,56 +144,60 @@ export function readChannelMutesStore( ): ChannelMuteStore { try { const key = storageKey(pubkey, relayUrl); - const raw = window.localStorage.getItem(key); + const scoped = parseRaw(window.localStorage.getItem(key)); - // Scoped key already has data — use it directly. - if (raw !== null) { - return parseRaw(raw) ?? DEFAULT_STORE; - } + // Unscoped read: the key IS the legacy key, so there is nothing to claim. + if (!relayUrl) return scoped ?? DEFAULT_STORE; - // No scoped data yet. If we were given a relay scope, attempt a one-time - // migration from the legacy pubkey-only key. - if (relayUrl) { - const legacyKey = storageKey(pubkey); - const migrated = parseRaw(window.localStorage.getItem(legacyKey)); - if (migrated && Object.keys(migrated.channels).length > 0) { - return migrateLegacyStore(key, legacyKey, migrated); - } - } + const legacyKey = storageKey(pubkey); + const legacy = parseRaw(window.localStorage.getItem(legacyKey)); + const legacyHasData = + legacy !== null && Object.keys(legacy.channels).length > 0; - return DEFAULT_STORE; + // No importable legacy value remains — an existing scoped copy is proven + // owned. This short-circuits every read after a completed claim. + if (!legacyHasData) return scoped ?? DEFAULT_STORE; + + // A consumable legacy key still coexists with (or would seed) the scoped + // key; neither is safe to expose until that legacy key is provably gone. + return claimLegacy(key, legacyKey, scoped ?? legacy, scoped !== null); } catch { return DEFAULT_STORE; } } /** - * 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. + * Enforce the ownership invariant: expose `owned` only once the legacy key is + * provably gone. Write the scoped copy when it is not already present, delete + * the legacy key, and confirm the delete took. On any failure return + * `DEFAULT_STORE`, rolling back only a scoped copy WE just wrote — never a + * pre-existing one, whose deletion could itself throw and lose owned data. An + * unproven copy is thus never exposed while the legacy key can still be + * imported by another relay; the next read re-attempts once storage recovers. */ -function migrateLegacyStore( +function claimLegacy( key: string, legacyKey: string, - migrated: ChannelMuteStore, + owned: ChannelMuteStore, + scopedExists: boolean, ): ChannelMuteStore { try { - window.localStorage.setItem(key, JSON.stringify(migrated)); + if (!scopedExists) window.localStorage.setItem(key, JSON.stringify(owned)); 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); + if (!scopedExists) window.localStorage.removeItem(key); return DEFAULT_STORE; } - return migrated; + return owned; } 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. + if (!scopedExists) { + try { + window.localStorage.removeItem(key); + } catch { + // Best effort. A stuck scoped copy is re-exposed only by a future read + // that proves the legacy key gone, so it can never seed early. + } } return DEFAULT_STORE; } diff --git a/desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs b/desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs index cdf0a60eef..3a71328472 100644 --- a/desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs +++ b/desktop/src/features/sidebar/lib/channelSectionsStorage.test.mjs @@ -422,6 +422,40 @@ test("readChannelSectionsStore: a legacy delete that does not take rolls back an assert.equal(window.localStorage.getItem(legacyKey), null); }); +test("readChannelSectionsStore: legacy delete + rollback both throw — every read returns DEFAULT until storage recovers, never seeds", () => { + const pubkey = "pk-migrate-delrollbackthrow"; + const relay = "wss://relay-delrollbackthrow.example.com"; + const legacy = makeStore({ + sections: [makeSection({ id: "sr", name: "Legacy", order: 0 })], + assignments: {}, + }); + writeChannelSectionsStore(pubkey, legacy); + const legacyKey = storageKey(pubkey); + const scoped = storageKey(pubkey, relay); + const origRemove = window.localStorage.removeItem; + // removeItem throws globally: the legacy delete AND the scoped rollback both + // throw, so a scoped copy the migration wrote survives on disk. + window.localStorage.removeItem = () => { + throw new Error("SecurityError"); + }; + try { + // First read: scoped write took, legacy delete threw, rollback threw. + assert.deepEqual(readChannelSectionsStore(pubkey, relay), DEFAULT_STORE); + // A stuck scoped copy survives, but the reader must NOT expose it while the + // legacy key still holds importable data — the seam Thufir found. + assert.deepEqual(readChannelSectionsStore(pubkey, relay), DEFAULT_STORE); + // Legacy key is intact, so the claim stays retryable rather than lost. + assert.notEqual(window.localStorage.getItem(legacyKey), null); + } finally { + window.localStorage.removeItem = origRemove; + } + // Storage recovers: the next read finishes the claim from the stuck scoped + // copy, deletes the legacy key, and only then exposes the value. + assert.deepEqual(readChannelSectionsStore(pubkey, relay), legacy); + assert.equal(window.localStorage.getItem(legacyKey), null); + assert.notEqual(window.localStorage.getItem(scoped), 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 c538098405..ee239d450c 100644 --- a/desktop/src/features/sidebar/lib/channelSectionsStorage.ts +++ b/desktop/src/features/sidebar/lib/channelSectionsStorage.ts @@ -141,22 +141,26 @@ 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. 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). + * Enforces one invariant on every read: a scoped copy is exposed only while no + * importable legacy (pubkey-only) key remains. If a non-empty legacy key still + * exists — whether because this is the first scoped read or because a prior + * migration wrote the scoped copy but could not delete the legacy key — we + * finish the claim inline: write the scoped copy if absent, delete the legacy + * key, and expose the value only once that delete provably took. On any storage + * failure we return `DEFAULT_STORE`, which cannot seed-publish (bootstrap gates + * on a non-empty local store), leaving the claim to complete on a later read + * once storage recovers. This makes "scoped data is owned only when the legacy + * key is gone" a property the reader proves every read, not a promise a + * one-shot migration hopes to keep under failing storage — so relay A's + * sections can never seed-publish onto a first-visited 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. + * Concurrent windows are safe without an atomic claim. Each window (main and + * huddle) takes its scope from the app-wide active community at mount, so the + * first scoped read for a live legacy key runs before any second relay scope + * can exist; that read either claims the legacy key or hits the failure path + * above. A race migrates the identical legacy value to the identical scoped key + * (idempotent), and the loser's post-delete confirmation fails and exposes + * `DEFAULT_STORE`. */ export function readChannelSectionsStore( pubkey: string, @@ -164,57 +168,59 @@ export function readChannelSectionsStore( ): ChannelSectionStore { try { const key = storageKey(pubkey, relayUrl); - const raw = window.localStorage.getItem(key); + const scoped = parseRaw(window.localStorage.getItem(key)); - // Scoped key already has data — use it directly. - if (raw !== null) { - return parseRaw(raw) ?? DEFAULT_STORE; - } + // Unscoped read: the key IS the legacy key, so there is nothing to claim. + if (!relayUrl) return scoped ?? DEFAULT_STORE; - // No scoped data yet. If we were given a relay scope, attempt a - // one-time migration from the legacy pubkey-only key. - if (relayUrl) { - const legacyKey = storageKey(pubkey); - const legacyRaw = window.localStorage.getItem(legacyKey); - const migrated = parseRaw(legacyRaw); - if (migrated && migrated.sections.length > 0) { - return migrateLegacyStore(key, legacyKey, migrated); - } - } + const legacyKey = storageKey(pubkey); + const legacy = parseRaw(window.localStorage.getItem(legacyKey)); + const legacyHasData = legacy !== null && legacy.sections.length > 0; - return DEFAULT_STORE; + // No importable legacy value remains — an existing scoped copy is proven + // owned. This short-circuits every read after a completed claim. + if (!legacyHasData) return scoped ?? DEFAULT_STORE; + + // A consumable legacy key still coexists with (or would seed) the scoped + // key; neither is safe to expose until that legacy key is provably gone. + return claimLegacy(key, legacyKey, scoped ?? legacy, scoped !== null); } catch { return DEFAULT_STORE; } } /** - * 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. + * Enforce the ownership invariant: expose `owned` only once the legacy key is + * provably gone. Write the scoped copy when it is not already present, delete + * the legacy key, and confirm the delete took. On any failure return + * `DEFAULT_STORE`, rolling back only a scoped copy WE just wrote — never a + * pre-existing one, whose deletion could itself throw and lose owned data. An + * unproven copy is thus never exposed while the legacy key can still be + * imported by another relay; the next read re-attempts once storage recovers. */ -function migrateLegacyStore( +function claimLegacy( key: string, legacyKey: string, - migrated: ChannelSectionStore, + owned: ChannelSectionStore, + scopedExists: boolean, ): ChannelSectionStore { try { - window.localStorage.setItem(key, JSON.stringify(migrated)); + if (!scopedExists) window.localStorage.setItem(key, JSON.stringify(owned)); 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); + if (!scopedExists) window.localStorage.removeItem(key); return DEFAULT_STORE; } - return migrated; + return owned; } 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. + if (!scopedExists) { + try { + window.localStorage.removeItem(key); + } catch { + // Best effort. A stuck scoped copy is re-exposed only by a future read + // that proves the legacy key gone, so it can never seed early. + } } return DEFAULT_STORE; } diff --git a/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs b/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs index 70b00454d6..11087ec774 100644 --- a/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs +++ b/desktop/src/features/sidebar/lib/channelStarsStorage.test.mjs @@ -702,3 +702,34 @@ test("readChannelStarsStore: a legacy delete that does not take rolls back and r assert.deepEqual(readChannelStarsStore(pubkey, relay), legacy); assert.equal(window.localStorage.getItem(legacyKey), null); }); + +test("readChannelStarsStore: legacy delete + rollback both throw — every read returns DEFAULT until storage recovers, never seeds", () => { + const pubkey = "pk-star-migrate-delrollbackthrow"; + const relay = "wss://relay-delrollbackthrow.example.com"; + const legacy = makeStarStore({ chr: STAR_E(true, 900, 4) }); + writeChannelStarsStore(pubkey, legacy); + const legacyKey = storageKey(pubkey); + const scoped = storageKey(pubkey, relay); + const origRemove = window.localStorage.removeItem; + // removeItem throws globally: the legacy delete AND the scoped rollback both + // throw, so a scoped copy the migration wrote survives on disk. + window.localStorage.removeItem = () => { + throw new Error("SecurityError"); + }; + try { + // First read: scoped write took, legacy delete threw, rollback threw. + assert.deepEqual(readChannelStarsStore(pubkey, relay), DEFAULT_STORE); + // A stuck scoped copy survives, but the reader must NOT expose it while the + // legacy key still holds importable data — the seam Thufir found. + assert.deepEqual(readChannelStarsStore(pubkey, relay), DEFAULT_STORE); + // Legacy key is intact, so the claim stays retryable rather than lost. + assert.notEqual(window.localStorage.getItem(legacyKey), null); + } finally { + window.localStorage.removeItem = origRemove; + } + // Storage recovers: the next read finishes the claim from the stuck scoped + // copy, deletes the legacy key, and only then exposes the value. + assert.deepEqual(readChannelStarsStore(pubkey, relay), legacy); + assert.equal(window.localStorage.getItem(legacyKey), null); + assert.notEqual(window.localStorage.getItem(scoped), null); +}); diff --git a/desktop/src/features/sidebar/lib/channelStarsStorage.ts b/desktop/src/features/sidebar/lib/channelStarsStorage.ts index 164b4d93de..b3431fcbe8 100644 --- a/desktop/src/features/sidebar/lib/channelStarsStorage.ts +++ b/desktop/src/features/sidebar/lib/channelStarsStorage.ts @@ -117,22 +117,26 @@ 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. 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). + * Enforces one invariant on every read: a scoped copy is exposed only while no + * importable legacy (pubkey-only) key remains. If a non-empty legacy key still + * exists — whether because this is the first scoped read or because a prior + * migration wrote the scoped copy but could not delete the legacy key — we + * finish the claim inline: write the scoped copy if absent, delete the legacy + * key, and expose the value only once that delete provably took. On any storage + * failure we return `DEFAULT_STORE`, which cannot seed-publish (bootstrap gates + * on a non-empty local store), leaving the claim to complete on a later read + * once storage recovers. This makes "scoped data is owned only when the legacy + * key is gone" a property the reader proves every read, not a promise a + * one-shot migration hopes to keep under failing storage — so relay A's stars + * can never seed-publish onto a first-visited 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. + * Concurrent windows are safe without an atomic claim. Each window (main and + * huddle) takes its scope from the app-wide active community at mount, so the + * first scoped read for a live legacy key runs before any second relay scope + * can exist; that read either claims the legacy key or hits the failure path + * above. A race migrates the identical legacy value to the identical scoped key + * (idempotent), and the loser's post-delete confirmation fails and exposes + * `DEFAULT_STORE`. */ export function readChannelStarsStore( pubkey: string, @@ -140,56 +144,60 @@ export function readChannelStarsStore( ): ChannelStarStore { try { const key = storageKey(pubkey, relayUrl); - const raw = window.localStorage.getItem(key); + const scoped = parseRaw(window.localStorage.getItem(key)); - // Scoped key already has data — use it directly. - if (raw !== null) { - return parseRaw(raw) ?? DEFAULT_STORE; - } + // Unscoped read: the key IS the legacy key, so there is nothing to claim. + if (!relayUrl) return scoped ?? DEFAULT_STORE; - // No scoped data yet. If we were given a relay scope, attempt a one-time - // migration from the legacy pubkey-only key. - if (relayUrl) { - const legacyKey = storageKey(pubkey); - const migrated = parseRaw(window.localStorage.getItem(legacyKey)); - if (migrated && Object.keys(migrated.channels).length > 0) { - return migrateLegacyStore(key, legacyKey, migrated); - } - } + const legacyKey = storageKey(pubkey); + const legacy = parseRaw(window.localStorage.getItem(legacyKey)); + const legacyHasData = + legacy !== null && Object.keys(legacy.channels).length > 0; - return DEFAULT_STORE; + // No importable legacy value remains — an existing scoped copy is proven + // owned. This short-circuits every read after a completed claim. + if (!legacyHasData) return scoped ?? DEFAULT_STORE; + + // A consumable legacy key still coexists with (or would seed) the scoped + // key; neither is safe to expose until that legacy key is provably gone. + return claimLegacy(key, legacyKey, scoped ?? legacy, scoped !== null); } catch { return DEFAULT_STORE; } } /** - * 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. + * Enforce the ownership invariant: expose `owned` only once the legacy key is + * provably gone. Write the scoped copy when it is not already present, delete + * the legacy key, and confirm the delete took. On any failure return + * `DEFAULT_STORE`, rolling back only a scoped copy WE just wrote — never a + * pre-existing one, whose deletion could itself throw and lose owned data. An + * unproven copy is thus never exposed while the legacy key can still be + * imported by another relay; the next read re-attempts once storage recovers. */ -function migrateLegacyStore( +function claimLegacy( key: string, legacyKey: string, - migrated: ChannelStarStore, + owned: ChannelStarStore, + scopedExists: boolean, ): ChannelStarStore { try { - window.localStorage.setItem(key, JSON.stringify(migrated)); + if (!scopedExists) window.localStorage.setItem(key, JSON.stringify(owned)); 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); + if (!scopedExists) window.localStorage.removeItem(key); return DEFAULT_STORE; } - return migrated; + return owned; } 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. + if (!scopedExists) { + try { + window.localStorage.removeItem(key); + } catch { + // Best effort. A stuck scoped copy is re-exposed only by a future read + // that proves the legacy key gone, so it can never seed early. + } } return DEFAULT_STORE; }