fix(security): logout's search-index wipe coordinates across tabs
CI / Build & Quality Checks (push) Successful in 1m40s
CI / Docker image build & smoke test (push) Skipped
CI / Secret scan (gitleaks) (push) Successful in 7s
CI / Trigger Desktop Build (push) Successful in 6s
CI / Playwright smoke (e2e) (push) Successful in 2m7s
CI / Build & Quality Checks (push) Successful in 1m40s
CI / Docker image build & smoke test (push) Skipped
CI / Secret scan (gitleaks) (push) Successful in 7s
CI / Trigger Desktop Build (push) Successful in 6s
CI / Playwright smoke (e2e) (push) Successful in 2m7s
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA
This commit is contained in:
@@ -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<string, unknown> = { 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<string, unknown> = {};
|
||||
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;
|
||||
}
|
||||
});
|
||||
|
||||
@@ -55,7 +55,50 @@ const roomRange = (roomId: string): IDBKeyRange => IDBKeyRange.bound([roomId], [
|
||||
|
||||
let dbPromise: Promise<IDBDatabase | null> | 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<IDBDatabase | null> => {
|
||||
// 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<IDBDatabase | null>((resolve) => {
|
||||
try {
|
||||
@@ -335,8 +378,23 @@ export const clearAll = async (): Promise<void> => {
|
||||
* 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<void> => {
|
||||
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<void> => {
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
/**
|
||||
* 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;
|
||||
|
||||
@@ -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<void> => {
|
||||
const hasSession = !!getFallbackSession();
|
||||
if (!shouldRunBootCleanup(hasSession, bootCleanupRan)) return;
|
||||
bootCleanupRan = true;
|
||||
await deleteSearchCacheDatabase();
|
||||
};
|
||||
|
||||
export const clearLoginData = async () => {
|
||||
const dbs = await window.indexedDB.databases();
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user