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 <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
Duncan
2026-08-28 13:57:29 -04:00
parent 0d9b8d7933
commit 950b8f6247
6 changed files with 254 additions and 136 deletions

View File

@@ -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);
});

View File

@@ -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;
}

View File

@@ -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,

View File

@@ -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;
}

View File

@@ -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);
});

View File

@@ -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;
}