diff --git a/neode-ui/src/composables/useCachedResource.ts b/neode-ui/src/composables/useCachedResource.ts index 804984d7..c217415f 100644 --- a/neode-ui/src/composables/useCachedResource.ts +++ b/neode-ui/src/composables/useCachedResource.ts @@ -32,9 +32,13 @@ export interface CachedResourceOptions { fetcher: (signal: AbortSignal) => Promise /** Data older than this triggers a background revalidate (default 30s). */ ttlMs?: number - /** Snapshot to sessionStorage so reloads paint instantly (default true). - * Disable for large payloads. */ - persist?: boolean + /** Snapshot to sessionStorage so reloads paint instantly. REQUIRED (no + * default) — `persist ?? true` was the exact footgun CR-01 hit (a wallet + * resource omitted this and silently persisted balances to + * sessionStorage), so every call site must make the decision explicitly. + * Sensitive data (money, identity, peer-identity/DID/pubkey payloads) + * MUST be `false`; static/aggregate/non-identifying data may be `true`. */ + persist: boolean /** Revalidate (if stale) when the window regains focus (default true). */ revalidateOnFocus?: boolean /** Fetch on first use (default true). Set false for lazy resources. */ @@ -61,7 +65,7 @@ export interface CachedResource { export function useCachedResource(opts: CachedResourceOptions): CachedResource { const store = useResourcesStore() const ttlMs = opts.ttlMs ?? 30_000 - const persist = opts.persist ?? true + const persist = opts.persist const entry = store.entry(opts.key, persist) const aborter = new AbortController() diff --git a/neode-ui/src/stores/__tests__/resources.test.ts b/neode-ui/src/stores/__tests__/resources.test.ts index 32275a57..e352a722 100644 --- a/neode-ui/src/stores/__tests__/resources.test.ts +++ b/neode-ui/src/stores/__tests__/resources.test.ts @@ -18,7 +18,7 @@ describe('resources store — stale-while-revalidate semantics', () => { const store = useResourcesStore() const e = store.entry('k1', true) expect(e.loadState).toBe('idle') - const p = store.refresh('k1', async () => 'hello') + const p = store.refresh('k1', async () => 'hello', { persist: true }) expect(e.loadState).toBe('loading') await p expect(e.loadState).toBe('ready') @@ -28,9 +28,9 @@ describe('resources store — stale-while-revalidate semantics', () => { it('sticky-ready: refresh never regresses ready → loading', async () => { const store = useResourcesStore() - await store.refresh('k2', async () => 1) + await store.refresh('k2', async () => 1, { persist: true }) const e = store.entry('k2', true) - const p = store.refresh('k2', async () => 2) + const p = store.refresh('k2', async () => 2, { persist: true }) expect(e.loadState).toBe('refreshing') await p expect(e.loadState).toBe('ready') @@ -39,11 +39,11 @@ describe('resources store — stale-while-revalidate semantics', () => { it('keeps last-known data on refresh error (ready + error set)', async () => { const store = useResourcesStore() - await store.refresh('k3', async () => 'good') + await store.refresh('k3', async () => 'good', { persist: true }) const e = store.entry('k3', true) await store.refresh('k3', async () => { throw new Error('boom') - }) + }, { persist: true }) expect(e.data).toBe('good') expect(e.loadState).toBe('ready') expect(e.error).toBe('boom') @@ -53,7 +53,7 @@ describe('resources store — stale-while-revalidate semantics', () => { const store = useResourcesStore() await store.refresh('k4', async () => { throw new Error('down') - }) + }, { persist: true }) const e = store.entry('k4', true) expect(e.loadState).toBe('error') expect(e.data).toBeNull() @@ -62,15 +62,15 @@ describe('resources store — stale-while-revalidate semantics', () => { it('dedups concurrent refreshes for the same key', async () => { const store = useResourcesStore() const fetcher = vi.fn(async () => 'once') - const p1 = store.refresh('k5', fetcher) - const p2 = store.refresh('k5', fetcher) + const p1 = store.refresh('k5', fetcher, { persist: true }) + const p2 = store.refresh('k5', fetcher, { persist: true }) await Promise.all([p1, p2]) expect(fetcher).toHaveBeenCalledTimes(1) }) it('hydrates a new entry from the sessionStorage snapshot', async () => { const store = useResourcesStore() - await store.refresh('k6', async () => ({ n: 42 })) + await store.refresh('k6', async () => ({ n: 42 }), { persist: true }) // Fresh pinia = fresh memory cache, same sessionStorage. setActivePinia(createPinia()) const store2 = useResourcesStore() @@ -81,7 +81,7 @@ describe('resources store — stale-while-revalidate semantics', () => { it('optimistic update applies immediately and rollback restores', async () => { const store = useResourcesStore() - await store.refresh('k7', async () => ['a']) + await store.refresh('k7', async () => ['a'], { persist: true }) const e = store.entry('k7', true) const rollback = store.optimistic('k7', (cur) => [...(cur ?? []), 'b'], true) expect(e.data).toEqual(['a', 'b']) @@ -91,7 +91,7 @@ describe('resources store — stale-while-revalidate semantics', () => { it('invalidate marks stale and debounce-runs subscribers', async () => { const store = useResourcesStore() - await store.refresh('k8', async () => 1) + await store.refresh('k8', async () => 1, { persist: true }) const revalidate = vi.fn() store.subscribe('k8', revalidate) store.invalidate('k8') @@ -110,7 +110,7 @@ describe('useCachedResource composable', () => { it('fetches immediately when stale and exposes reactive views', async () => { const fetcher = vi.fn(async () => 'data') - const r = useCachedResource({ key: 'c1', fetcher, revalidateOnFocus: false }) + const r = useCachedResource({ key: 'c1', fetcher, revalidateOnFocus: false, persist: true }) await r.refresh() expect(fetcher).toHaveBeenCalled() expect(r.data.value).toBe('data') @@ -120,11 +120,11 @@ describe('useCachedResource composable', () => { it('does not refetch within TTL (instant render from cache)', async () => { const fetcher = vi.fn(async () => 'v1') - const r1 = useCachedResource({ key: 'c2', fetcher, ttlMs: 60_000, revalidateOnFocus: false }) + const r1 = useCachedResource({ key: 'c2', fetcher, ttlMs: 60_000, revalidateOnFocus: false, persist: true }) await r1.refresh() // Second component using the same key inside the TTL: no new fetch. const fetcher2 = vi.fn(async () => 'v2') - const r2 = useCachedResource({ key: 'c2', fetcher: fetcher2, ttlMs: 60_000, revalidateOnFocus: false }) + const r2 = useCachedResource({ key: 'c2', fetcher: fetcher2, ttlMs: 60_000, revalidateOnFocus: false, persist: true }) expect(r2.data.value).toBe('v1') expect(fetcher2).not.toHaveBeenCalled() }) diff --git a/neode-ui/src/stores/__tests__/resourcesClear.test.ts b/neode-ui/src/stores/__tests__/resourcesClear.test.ts index 78fb3fd0..a7320f73 100644 --- a/neode-ui/src/stores/__tests__/resourcesClear.test.ts +++ b/neode-ui/src/stores/__tests__/resourcesClear.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { setActivePinia, createPinia } from 'pinia' -import { useResourcesStore } from '../resources' +import { useResourcesStore, SCHEMA_MARKER_KEY, CURRENT_SCHEMA_VERSION } from '../resources' vi.mock('@/api/rpc-client', () => ({ rpcClient: { @@ -21,8 +21,8 @@ describe('resources store — clearAll (T-02-02 logout purge)', () => { it('drops every in-memory entry', async () => { const store = useResourcesStore() - await store.refresh('k1', async () => 'a') - await store.refresh('k2', async () => 'b') + await store.refresh('k1', async () => 'a', { persist: true }) + await store.refresh('k2', async () => 'b', { persist: true }) expect(store.entries.size).toBe(2) store.clearAll() expect(store.entries.size).toBe(0) @@ -30,7 +30,7 @@ describe('resources store — clearAll (T-02-02 logout purge)', () => { it('removes every resource:-prefixed sessionStorage key and leaves unrelated keys untouched', async () => { const store = useResourcesStore() - await store.refresh('k1', async () => 'a') + await store.refresh('k1', async () => 'a', { persist: true }) sessionStorage.setItem('unrelated-app-setting', 'keep-me') expect(sessionStorage.getItem('resource:k1')).not.toBeNull() @@ -44,7 +44,7 @@ describe('resources store — clearAll (T-02-02 logout purge)', () => { it('cancels pending invalidate timers so a scheduled revalidate never fires post-logout', async () => { const store = useResourcesStore() - await store.refresh('k1', async () => 'a') + await store.refresh('k1', async () => 'a', { persist: true }) const revalidate = vi.fn() store.subscribe('k1', revalidate) store.invalidate('k1') // schedules a debounced revalidate @@ -60,7 +60,7 @@ describe('resources store — clearAll (T-02-02 logout purge)', () => { const store = useResourcesStore() let resolveOld!: (v: string) => void const oldFetch = new Promise((res) => { resolveOld = res }) - const p = store.refresh('k2', () => oldFetch) + const p = store.refresh('k2', () => oldFetch, { persist: true }) store.clearAll() // The key was dropped from `entries` by clearAll — a fresh lookup must @@ -80,7 +80,7 @@ describe('resources store — clearAll (T-02-02 logout purge)', () => { it('does not throw when sessionStorage.removeItem throws', async () => { const store = useResourcesStore() - await store.refresh('k1', async () => 'a') + await store.refresh('k1', async () => 'a', { persist: true }) const spy = vi.spyOn(Storage.prototype, 'removeItem').mockImplementation(() => { throw new Error('sessionStorage unavailable') }) @@ -90,7 +90,7 @@ describe('resources store — clearAll (T-02-02 logout purge)', () => { it('does not throw when sessionStorage.key/length throw', async () => { const store = useResourcesStore() - await store.refresh('k1', async () => 'a') + await store.refresh('k1', async () => 'a', { persist: true }) const spy = vi.spyOn(Storage.prototype, 'key').mockImplementation(() => { throw new Error('sessionStorage inaccessible') }) @@ -111,7 +111,7 @@ describe('auth store — logout purges the cache even when the RPC rejects', () vi.mocked(rpcClient.logout).mockRejectedValueOnce(new Error('network down')) const resources = useResourcesStore() - await resources.refresh('some.cached.key', async () => 'sensitive-data') + await resources.refresh('some.cached.key', async () => 'sensitive-data', { persist: true }) expect(resources.entries.size).toBe(1) const { useAuthStore } = await import('../auth') @@ -127,7 +127,7 @@ describe('auth store — logout purges the cache even when the RPC rejects', () vi.mocked(rpcClient.logout).mockResolvedValueOnce(undefined as never) const resources = useResourcesStore() - await resources.refresh('another.key', async () => 'data') + await resources.refresh('another.key', async () => 'data', { persist: true }) expect(resources.entries.size).toBe(1) const { useAuthStore } = await import('../auth') @@ -137,3 +137,121 @@ describe('auth store — logout purges the cache even when the RPC rejects', () expect(resources.entries.size).toBe(0) }) }) + +describe('resources store — one-time schema migration purge (CR-01 follow-up, T-02-01)', () => { + beforeEach(() => { + sessionStorage.clear() + localStorage.clear() + }) + + it('purges a legacy resource: snapshot written with no marker at all (pre-migration tab)', () => { + // Simulates the exact incident scenario: a tab that was open before this + // migration shipped, holding a snapshot the OLD bundle wrote under the + // OLD (now-incorrect) persist decision — e.g. CR-01's web5.lnd-info. + sessionStorage.setItem('resource:web5.lnd-info', JSON.stringify({ + data: { balance_sats: 50000, channel_balance_sats: 25000, synced_to_chain: true }, + fetchedAt: Date.now(), + })) + + setActivePinia(createPinia()) + useResourcesStore() // store setup() runs the migration check synchronously + + expect(sessionStorage.getItem('resource:web5.lnd-info')).toBeNull() + }) + + it('purges every resource: snapshot when the marker is present but stale (persist contract changed since)', () => { + sessionStorage.setItem(SCHEMA_MARKER_KEY, 'some-older-version-string') + sessionStorage.setItem('resource:some-key', JSON.stringify({ data: 'stale-contract-data', fetchedAt: Date.now() })) + + setActivePinia(createPinia()) + useResourcesStore() + + expect(sessionStorage.getItem('resource:some-key')).toBeNull() + }) + + it('writes the current marker after purging, so this is a one-time cost per tab session', () => { + sessionStorage.setItem('resource:legacy', JSON.stringify({ data: 'x', fetchedAt: Date.now() })) + + setActivePinia(createPinia()) + useResourcesStore() + + expect(sessionStorage.getItem(SCHEMA_MARKER_KEY)).toBe(CURRENT_SCHEMA_VERSION) + }) + + it('does NOT purge — a snapshot written under the CURRENT marker survives a later store init (one-time, not every-boot)', () => { + // Marker already matches CURRENT_SCHEMA_VERSION, as if this same tab + // already ran the migration once (e.g. an earlier navigation/reload). + sessionStorage.setItem(SCHEMA_MARKER_KEY, CURRENT_SCHEMA_VERSION) + const snapshot = JSON.stringify({ data: 'keep-me', fetchedAt: Date.now() }) + sessionStorage.setItem('resource:already-migrated-key', snapshot) + + // A second store "init" — e.g. a fresh Pinia instance, standing in for + // another reload of the same tab (sessionStorage persists across it). + setActivePinia(createPinia()) + useResourcesStore() + + // Byte-for-byte untouched — the migration was a no-op this time. + expect(sessionStorage.getItem('resource:already-migrated-key')).toBe(snapshot) + }) + + it('a persist:false key never writes a sessionStorage snapshot in the first place', async () => { + setActivePinia(createPinia()) + const store = useResourcesStore() + await store.refresh('web5.lnd-info', async () => ({ balance_sats: 1, channel_balance_sats: 2 }), { persist: false }) + + expect(sessionStorage.getItem('resource:web5.lnd-info')).toBeNull() + }) + + it('migration purge is strictly bounded to the resource: prefix — every other storage key (localStorage auth, onboarding seed scratch, PWA/splash flags, hand-rolled view caches) survives untouched', () => { + // A representative sample of this app's real non-`resource:` storage + // usage, seeded BEFORE a legacy snapshot forces the purge to actually + // run — proving the purge cannot log anyone out or destroy unrelated + // state (Dorian's blast-radius requirement). + localStorage.setItem('neode-auth', 'true') // auth session flag (localStorage, not sessionStorage — untouched by construction) + sessionStorage.setItem('archipelago_from_boot', '1') + sessionStorage.setItem('archipelago_from_splash', '1') + sessionStorage.setItem('_seed_words', JSON.stringify(['abandon', 'ability'])) + sessionStorage.setItem('_seed_challenge_indices', JSON.stringify([1, 2, 3])) + sessionStorage.setItem('archipelago.web5.identities.v1', JSON.stringify({ v: 1, savedAt: Date.now(), identities: [] })) + sessionStorage.setItem('archipelago.fleet.cache.v1', JSON.stringify({ v: 1 })) + sessionStorage.setItem('unrelated-app-setting', 'keep-me') + // The thing that actually triggers the purge this run: + sessionStorage.setItem('resource:legacy-key', JSON.stringify({ data: 'x', fetchedAt: Date.now() })) + + setActivePinia(createPinia()) + useResourcesStore() + + // The purge fired (proves the test actually exercised the purge path). + expect(sessionStorage.getItem('resource:legacy-key')).toBeNull() + + // Everything else survives byte-for-byte. + expect(localStorage.getItem('neode-auth')).toBe('true') + expect(sessionStorage.getItem('archipelago_from_boot')).toBe('1') + expect(sessionStorage.getItem('archipelago_from_splash')).toBe('1') + expect(sessionStorage.getItem('_seed_words')).toBe(JSON.stringify(['abandon', 'ability'])) + expect(sessionStorage.getItem('_seed_challenge_indices')).toBe(JSON.stringify([1, 2, 3])) + expect(sessionStorage.getItem('archipelago.web5.identities.v1')).not.toBeNull() + expect(sessionStorage.getItem('archipelago.fleet.cache.v1')).not.toBeNull() + expect(sessionStorage.getItem('unrelated-app-setting')).toBe('keep-me') + }) + + it('the migration cannot race an in-flight fetch — it runs synchronously at store setup, before any entry()/refresh() call is possible, and a refresh immediately afterward behaves normally', async () => { + sessionStorage.setItem('resource:legacy-key', JSON.stringify({ data: 'x', fetchedAt: Date.now() })) + + setActivePinia(createPinia()) + const store = useResourcesStore() + + // Nothing was, or could have been, in flight during the migration: the + // store's internal bookkeeping is still pristine immediately after + // construction (entries empty, no leaked in-flight promise). + expect(store.entries.size).toBe(0) + + // A normal refresh right after store construction is unaffected by the + // migration having just run — proves no corruption/wedged state. + await store.refresh('fresh-key', async () => 'fresh-data', { persist: true }) + const e = store.entry('fresh-key', true) + expect(e.data).toBe('fresh-data') + expect(e.loadState).toBe('ready') + expect(sessionStorage.getItem('resource:fresh-key')).not.toBeNull() + }) +}) diff --git a/neode-ui/src/stores/resources.ts b/neode-ui/src/stores/resources.ts index f7fe2ac1..11a14cfe 100644 --- a/neode-ui/src/stores/resources.ts +++ b/neode-ui/src/stores/resources.ts @@ -30,6 +30,33 @@ export interface ResourceEntry { const SNAPSHOT_PREFIX = 'resource:' +// One-time legacy-snapshot purge (CR-01 follow-up, T-02-01). Bumping a cache +// key's persist decision (e.g. Web5.vue's web5.lnd-info going true → false) +// only stops FUTURE writes — a tab that was already open before the update +// ships reloads in-place onto the new bundle and keeps whatever the OLD +// bundle already wrote to sessionStorage under the old decision, forever +// (nothing else ever purges a `resource:` snapshot except clearAll() on +// logout). This marker/version pair is what detects that case: +// - marker present + matches CURRENT_SCHEMA_VERSION: this tab's snapshots +// were written under the CURRENT persist contract — trust them, no-op. +// - marker absent (tab predates this migration) or stale (persist +// contract changed since this tab last checked): every `resource:` +// snapshot in this session is untrustworthy — purge all of them, then +// write the current marker so this is a one-time cost per tab session, +// not a per-navigation/per-reload one (sessionStorage — and this +// marker — survive reloads of the same tab; only a brand-new tab starts +// with no marker at all, and a brand-new tab has nothing to purge). +// +// CONTRACT: bump CURRENT_SCHEMA_VERSION any time a cache key's `persist` +// value changes (true -> false, or a newly-added key defaults the wrong +// way) so the next deploy automatically purges any snapshot written under +// the old, now-incorrect decision. Forgetting to bump this is exactly how +// CR-01 stayed exposed for updating users even after the code fix landed. +// Exported so tests can assert against the real marker key/version instead +// of a hardcoded duplicate that could silently drift from this file. +export const SCHEMA_MARKER_KEY = 'resource:__schema' +export const CURRENT_SCHEMA_VERSION = '1' + function readSnapshot(key: string): { data: T; fetchedAt: number } | null { try { const raw = sessionStorage.getItem(SNAPSHOT_PREFIX + key) @@ -96,16 +123,21 @@ export const useResourcesStore = defineStore('resources', () => { } /** Run `fetcher` for `key` with sticky-ready + keep-last-value semantics. - * Concurrent calls for the same key share one in-flight fetch. */ + * Concurrent calls for the same key share one in-flight fetch. `persist` + * is REQUIRED (no default) for the same reason as entry()/optimistic() + * (WR-04/T-02-01): `opts.persist ?? true` was the exact footgun that let + * CR-01 happen (Web5.vue's wallet resources omitted it and silently + * persisted), so no call site can opt a key into sessionStorage by + * omission anymore — every caller must make the decision explicitly. */ function refresh( key: string, fetcher: () => Promise, - opts: { persist?: boolean } = {}, + opts: { persist: boolean }, ): Promise { const existing = inflight.get(key) if (existing) return existing const startGeneration = generation - const e = entry(key, opts.persist ?? true) + const e = entry(key, opts.persist) e.loadState = e.loadState === 'ready' || e.loadState === 'refreshing' ? 'refreshing' : 'loading' const p = (async () => { try { @@ -117,7 +149,7 @@ export const useResourcesStore = defineStore('resources', () => { e.error = null e.fetchedAt = Date.now() e.loadState = 'ready' - if (opts.persist ?? true) writeSnapshot(key, data, e.fetchedAt) + if (opts.persist) writeSnapshot(key, data, e.fetchedAt) } catch (err) { if (generation !== startGeneration) return e.error = err instanceof Error ? err.message : String(err) @@ -192,18 +224,19 @@ export const useResourcesStore = defineStore('resources', () => { } } - /** Purge every cached resource — memory and sessionStorage — so no payload - * from this session outlives a logout or identity switch (T-02-02). Cancels - * pending invalidate timers and drops in-flight/revalidator bookkeeping - * first, so a fetch that resolves after this call cannot repopulate the - * cache with data from the ending session. */ - function clearAll(): void { - generation++ - for (const timer of invalidateTimers.values()) clearTimeout(timer) - invalidateTimers.clear() - inflight.clear() - revalidators.clear() - entries.clear() + /** Remove every `resource:`-prefixed sessionStorage key — snapshots AND + * the schema marker itself (whichever caller triggered this always + * rewrites its own marker/state right after, so there's no window where + * a missing marker is user-visible). This is the ONLY place that + * enumerates/removes `resource:` keys; clearAll() (logout, T-02-02) and + * the one-time schema migration (CR-01 follow-up, T-02-01, below) both + * call it rather than duplicating the loop. Strictly bounded to the + * `resource:` prefix — every other sessionStorage/localStorage key this + * app uses (auth's `neode-auth` in localStorage, onboarding/seed-entry + * scratch keys, PWA/splash flags, the hand-rolled per-view caches like + * `archipelago.web5.identities.v1`) uses a different prefix or storage + * entirely and is untouched by this loop. */ + function purgeAllSnapshots(): void { try { const keys: string[] = [] for (let i = 0; i < sessionStorage.length; i++) { @@ -216,5 +249,39 @@ export const useResourcesStore = defineStore('resources', () => { } } + /** Purge every cached resource — memory and sessionStorage — so no payload + * from this session outlives a logout or identity switch (T-02-02). Cancels + * pending invalidate timers and drops in-flight/revalidator bookkeeping + * first, so a fetch that resolves after this call cannot repopulate the + * cache with data from the ending session. */ + function clearAll(): void { + generation++ + for (const timer of invalidateTimers.values()) clearTimeout(timer) + invalidateTimers.clear() + inflight.clear() + revalidators.clear() + entries.clear() + purgeAllSnapshots() + } + + // One-time legacy-snapshot migration (see CURRENT_SCHEMA_VERSION's comment + // above). Runs synchronously here, at store setup — i.e. once per Pinia + // instance (once per tab's page load), and strictly BEFORE this store is + // returned to any caller, so no entry()/refresh()/optimistic() call can + // possibly be in flight yet: `entries`/`inflight` are still the fresh + // empty Maps declared above. There is nothing for this purge to race. + // A plain in-place reload of an already-migrated tab reads a matching + // marker and returns immediately — the whole point is that this is cheap + // (one sessionStorage read) on every init and only actually purges once, + // on the specific pre-update tab that still holds legacy data. + try { + if (sessionStorage.getItem(SCHEMA_MARKER_KEY) !== CURRENT_SCHEMA_VERSION) { + purgeAllSnapshots() + sessionStorage.setItem(SCHEMA_MARKER_KEY, CURRENT_SCHEMA_VERSION) + } + } catch { + /* sessionStorage unavailable — nothing was ever persisted, nothing to migrate */ + } + return { entries, entry, refresh, invalidate, subscribe, optimistic, evict, clearAll } })