From 908e73593345c882ef6bd9827006f8f225374927 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Tue, 15 Sep 2026 21:32:06 -0400 Subject: [PATCH] fix(security): logout's search-index wipe coordinates across tabs deleteSearchCacheDatabase() resolved after a 3 s "blocked" timeout while another tab still held the DB, so decrypted rows could survive logout. It now broadcasts lotus-logout first; every tab closes its handle and refuses to reopen, then the delete proceeds. A boot with no session re-runs the wipe once in case a race was still lost. Unit-tested. Fixes #45 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA --- src/app/utils/searchCache.test.ts | 66 ++++++++++++++++++++++++++++- src/app/utils/searchCache.ts | 70 +++++++++++++++++++++++++++++++ src/client/initMatrix.ts | 17 +++++++- src/index.tsx | 7 ++++ 4 files changed, 157 insertions(+), 3 deletions(-) diff --git a/src/app/utils/searchCache.test.ts b/src/app/utils/searchCache.test.ts index 6067b1aba..317db990e 100644 --- a/src/app/utils/searchCache.test.ts +++ b/src/app/utils/searchCache.test.ts @@ -12,6 +12,7 @@ import { clearAll, deleteRow, deleteSearchCacheDatabase, + shouldRunBootCleanup, SearchCacheRow, } from './searchCache'; @@ -126,7 +127,11 @@ test('searchCache IDB round-trip', { skip: !hasIdb }, async () => { assert.equal((await queryRoom('!r1')).length, 0); assert.equal((await queryRoom('!r2')).length, 1); - await deleteSearchCacheDatabase(); + // clearAll() (not deleteSearchCacheDatabase()) so later tests in this file + // can still reopen the DB — deleteSearchCacheDatabase() now permanently + // closes this module's handle (Gitea #45), which in the real app is fine + // because logout always reloads the page right after. + await clearAll(); }); test('deleteRow: removes only the targeted [roomId, eventId] row', { skip: !hasIdb }, async () => { @@ -151,7 +156,7 @@ test('deleteRow: removes only the targeted [roomId, eventId] row', { skip: !hasI // Deleting a row that doesn't exist is a silent no-op. await assert.doesNotReject(deleteRow('!r1', '$does-not-exist')); - await deleteSearchCacheDatabase(); + await clearAll(); }); test('resilient helpers never throw when IDB is unavailable', { skip: hasIdb }, async () => { @@ -168,3 +173,60 @@ test('resilient helpers never throw when IDB is unavailable', { skip: hasIdb }, await assert.doesNotReject(clearAll()); await assert.doesNotReject(deleteSearchCacheDatabase()); }); + +// --- Gitea #45: boot-time completion wipe decision -------------------------- + +test('shouldRunBootCleanup: only when signed out and not already run this boot', () => { + assert.equal(shouldRunBootCleanup(false, false), true); // no session, first time + assert.equal(shouldRunBootCleanup(true, false), false); // signed in — never wipe a live cache + assert.equal(shouldRunBootCleanup(false, true), false); // already ran this boot + assert.equal(shouldRunBootCleanup(true, true), false); +}); + +// --- Gitea #45: closed-flag gating ------------------------------------------ +// +// A minimal fake `indexedDB` that only supports what `deleteSearchCacheDatabase()` +// and the `openDb()` open path touch, so this test doesn't need a full +// IDBTransaction mock: it only needs to prove that once +// `deleteSearchCacheDatabase()` has run, nothing calls `indexedDB.open()` +// again in this module instance — the actual guarantee the fix provides. +// +// Note: `closed` is process-wide module state, and other tests above already +// call `deleteSearchCacheDatabase()` (harmlessly, since `indexedDB` was +// `undefined` when they ran), so this module may already be "closed" by the +// time this test runs. That's fine — the property under test doesn't depend +// on the prior state: after (another) `deleteSearchCacheDatabase()` call, +// `indexedDB.open()` must never fire again, however many callers still try. +test('closed-flag gating: deleteSearchCacheDatabase() stops the DB from reopening', async () => { + let openCalls = 0; + const previousIdb = (globalThis as { indexedDB?: unknown }).indexedDB; + (globalThis as { indexedDB?: unknown }).indexedDB = { + open: () => { + openCalls += 1; + const req: Record = { result: {} }; + // openDb() assigns onupgradeneeded/onsuccess/onerror/onblocked + // synchronously after calling open(); fire success on the next tick so + // those handlers are already attached. + setTimeout(() => (req.onsuccess as (() => void) | undefined)?.(), 0); + return req; + }, + deleteDatabase: () => { + const req: Record = {}; + setTimeout(() => (req.onsuccess as (() => void) | undefined)?.(), 0); + return req; + }, + }; + + try { + await deleteSearchCacheDatabase(); + openCalls = 0; // isolate: only count opens attempted AFTER closing + + // Every subsequent read must short-circuit to a cache-miss without ever + // calling indexedDB.open() again. + assert.deepEqual(await queryRoom('!gate'), []); + await putRows([{ roomId: '!gate', eventId: '$1', ts: 1, sender: '@a', body: 'x' }]); + assert.equal(openCalls, 0); + } finally { + (globalThis as { indexedDB?: unknown }).indexedDB = previousIdb; + } +}); diff --git a/src/app/utils/searchCache.ts b/src/app/utils/searchCache.ts index 1e6b9eea6..51968e873 100644 --- a/src/app/utils/searchCache.ts +++ b/src/app/utils/searchCache.ts @@ -55,7 +55,50 @@ const roomRange = (roomId: string): IDBKeyRange => IDBKeyRange.bound([roomId], [ let dbPromise: Promise | null = null; +// Gitea #45 — `deleteSearchCacheDatabase()` used to resolve as soon as its +// bounded `onblocked` wait elapsed, even though another tab still had the DB +// open, so the delete stayed silently queued while that tab could keep +// `put`-ing fresh decrypted rows (or immediately reopen the DB once the +// delete eventually landed). `closed` is broadcast to every tab the moment a +// wipe starts: once set, `openDb()` refuses to (re)open a handle — including +// for an already in-flight `saveRoomIndex()` awaiting its next IDB +// round-trip — so nothing can repopulate the DB out from under the delete. +let closed = false; + +const LOGOUT_CHANNEL_NAME = 'lotus-logout'; +const LOGOUT_MESSAGE = 'lotus-logout'; + +/** Stop touching the DB and release our handle. Idempotent, never throws. */ +const handleLogoutSignal = (): void => { + closed = true; + const pending = dbPromise; + dbPromise = null; + if (pending) { + pending.then((db) => db?.close()).catch(() => undefined); + } +}; + +const logoutChannel: BroadcastChannel | null = (() => { + try { + if (typeof BroadcastChannel === 'undefined') return null; + const channel = new BroadcastChannel(LOGOUT_CHANNEL_NAME); + channel.onmessage = (ev: MessageEvent) => { + if (ev.data === LOGOUT_MESSAGE) handleLogoutSignal(); + }; + // Node's BroadcastChannel (unlike the browser's) keeps the event loop + // alive while open, which would hang `node --test`. unref() is a + // Node-only extension — no-op via optional chaining in the browser. + (channel as unknown as { unref?: () => void }).unref?.(); + return channel; + } catch { + return null; + } +})(); + const openDb = (): Promise => { + // Once logout has broadcast a wipe, this module must never reopen the DB — + // otherwise a write racing the delete could recreate it right after. + if (closed) return Promise.resolve(null); if (dbPromise) return dbPromise; dbPromise = new Promise((resolve) => { try { @@ -335,8 +378,23 @@ export const clearAll = async (): Promise => { * Drop the entire on-disk database. Wired into the logout path by the * coordinator (initMatrix) so no decrypted plaintext lingers after sign-out. * Closes any open handle first so the delete is not blocked. Never throws. + * + * Gitea #45 — before touching IDB at all, broadcast the wipe on + * `lotus-logout` so every other tab closes its handle and stops writing + * (`handleLogoutSignal`/`closed` above); that tab's in-flight + * `saveRoomIndex()` short-circuits on its next IDB call instead of + * repopulating a DB we're about to delete or racing back in right after. The + * bounded `onblocked` wait stays as a last-resort fallback (a tab that hasn't + * processed the broadcast yet, or a browser without BroadcastChannel), not + * the primary coordination mechanism. */ export const deleteSearchCacheDatabase = async (): Promise => { + closed = true; + try { + logoutChannel?.postMessage(LOGOUT_MESSAGE); + } catch { + // ignore + } try { const existing = dbPromise ? await dbPromise : null; if (existing) existing.close(); @@ -373,3 +431,15 @@ export const deleteSearchCacheDatabase = async (): Promise => { } }); }; + +/** + * Pure decision for the boot-time completion wipe (Gitea #45): a delete that + * lost the `onblocked` race leaves `lotus-search-cache` on disk even though + * the user is signed out. Re-run the wipe next boot, but only when there is + * no session (never nuke a live, signed-in cache) and only once per boot + * (the caller's own re-checks — e.g. route changes — must not repeat it). + * Exported for testing without a DOM/session; the actual boot wiring lives in + * initMatrix.ts. + */ +export const shouldRunBootCleanup = (hasSession: boolean, alreadyRan: boolean): boolean => + !hasSession && !alreadyRan; diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index b1c248ab4..ca33ebd39 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -6,7 +6,7 @@ import { getFallbackSession, removeFallbackSession, Session } from '../app/state import { LotusOidcTokenRefresher } from './oidcTokenRefresher'; import { revokeOidcTokens } from './oidcLogout'; import { pushSessionToSW } from '../sw-session'; -import { deleteSearchCacheDatabase } from '../app/utils/searchCache'; +import { deleteSearchCacheDatabase, shouldRunBootCleanup } from '../app/utils/searchCache'; import { clearPlaintextCaches } from '../app/state/plaintextCaches'; import { clearSoundboardClipCache } from '../app/utils/soundboardClips'; @@ -141,6 +141,21 @@ export const logoutClient = async (mx: MatrixClient) => { window.location.reload(); }; +// Gitea #45 — deleteSearchCacheDatabase()'s onblocked handling is a bounded +// best-effort wait; if another tab held `lotus-search-cache` open through it +// (the pre-broadcast case, or a browser without BroadcastChannel), the delete +// stayed queued and plaintext survived on disk despite logout "succeeding". +// Complete it on the next boot with no session, once per boot — call this +// wherever the app decides there is no session to hydrate (e.g. alongside the +// existing getFallbackSession() checks in src/index.tsx). +let bootCleanupRan = false; +export const cleanupSearchCacheIfSignedOut = async (): Promise => { + const hasSession = !!getFallbackSession(); + if (!shouldRunBootCleanup(hasSession, bootCleanupRan)) return; + bootCleanupRan = true; + await deleteSearchCacheDatabase(); +}; + export const clearLoginData = async () => { const dbs = await window.indexedDB.databases(); diff --git a/src/index.tsx b/src/index.tsx index 0ebabbf03..926408b19 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -17,6 +17,7 @@ import App from './app/pages/App'; import './app/i18n'; import { pushSessionToSW } from './sw-session'; import { getFallbackSession } from './app/state/sessions'; +import { cleanupSearchCacheIfSignedOut } from './client/initMatrix'; document.body.classList.add(configClass, varsClass); @@ -51,6 +52,12 @@ if ('serviceWorker' in navigator) { // already exists" upload storm and E2EE breakage. Only ask for sessions worth // protecting (skip anonymous/landing visitors to avoid a needless Firefox // prompt); check persisted() first so we don't re-prompt. Best-effort. +// [Gitea #45] If a logout's search-index wipe lost the multi-tab race (another +// tab still held the DB), finish it now that no session exists. +if (!getFallbackSession()) { + cleanupSearchCacheIfSignedOut().catch(() => undefined); +} + if (navigator.storage?.persist && getFallbackSession()) { navigator.storage .persisted()