diff --git a/lib/sign-out-cleanup.ts b/lib/sign-out-cleanup.ts index f7ca42832..0effb91ac 100644 --- a/lib/sign-out-cleanup.ts +++ b/lib/sign-out-cleanup.ts @@ -38,9 +38,10 @@ const SIGNED_OUT_STORAGE_KEYS = [ 'bulwark-offline-accounts', // Which accounts' data waits for a new sign-in, and until when. 'bulwark-offline-evicted', + 'bulwark-store-opened', ]; -const LOCAL_STORE_KEYS = new Set(['bulwark-offline-accounts', 'bulwark-offline-evicted']); +const LOCAL_STORE_KEYS = new Set(['bulwark-offline-accounts', 'bulwark-offline-evicted', 'bulwark-store-opened']); const SIGNED_OUT_SIGNAL = 'bulwark:signed-out'; diff --git a/lib/store/__tests__/page-side.test.ts b/lib/store/__tests__/page-side.test.ts index ede57905f..fbd336b69 100644 --- a/lib/store/__tests__/page-side.test.ts +++ b/lib/store/__tests__/page-side.test.ts @@ -1,9 +1,10 @@ import 'fake-indexeddb/auto'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { configureStore, getStore, setStoreBackend } from '../client/api'; -import { getAccountRecord } from '../client/keys'; +import { configureStore, getStore, setStoreBackend, STORE_OPENED_KEY } from '../client/api'; +import { forgetAccountKey, getAccountRecord } from '../client/keys'; import { purgeStoreFiles } from '../client/purge'; -import { StoreClient } from '../client/store-client'; +import { StoreClient, StoreOwnerGoneError } from '../client/store-client'; +import { storeFilesToPrune } from '../worker/host'; import { FAKE_SESSION } from './helpers/fake-jmap'; /** Everything on one page: a BroadcastChannel that also reaches other objects here, as in a browser. */ @@ -25,6 +26,7 @@ class FakeChannel { class FakeWorker { static all: FakeWorker[] = []; + static openResult: unknown; onmessage: ((event: MessageEvent) => void) | null = null; posted: Array> = []; terminated = false; @@ -33,7 +35,7 @@ class FakeWorker { } postMessage(message: Record): void { this.posted.push(message); - if (message.kind === 'open') queueMicrotask(() => this.onmessage?.({ data: { kind: 'result', id: message.id } } as MessageEvent)); + if (message.kind === 'open' || message.kind === 'prune-files') queueMicrotask(() => this.onmessage?.({ data: { kind: 'result', id: message.id, result: message.kind === 'open' ? FakeWorker.openResult : undefined } } as MessageEvent)); } terminate(): void { this.terminated = true; @@ -107,6 +109,47 @@ describe('a tab that starts after the owner', () => { }); }); +describe('an owner that stopped answering', () => { + afterEach(() => vi.useRealTimers()); + + async function clientWithOwner(answers: () => boolean) { + vi.useFakeTimers(); + const owner = new FakeChannel('bulwark-store-rpc'); + owner.onmessage = (event) => { + const data = event.data as { kind: string }; + if (data.kind === 'who-owns' && answers()) owner.postMessage({ kind: 'owner-changed', owner: 'elsewhere' }); + }; + const client = new StoreClient({ basePath: '' }); + client.start(); + await vi.advanceTimersByTimeAsync(0); + return client; + } + + it('stops its calls waiting, long before their own time limit', async () => { + let answering = true; + const client = await clientWithOwner(() => answering); + const call = client.sync('a@x'); + const failed = vi.fn(); + void call.catch(failed); + await vi.advanceTimersByTimeAsync(0); + answering = false; + await vi.advanceTimersByTimeAsync(39_000); + expect(failed).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(11_000); + expect(failed.mock.calls[0][0]).toBeInstanceOf(StoreOwnerGoneError); + client.halt(); + }); + + it('leaves calls alone while the owner answers', async () => { + const client = await clientWithOwner(() => true); + const failed = vi.fn(); + void client.sync('a@x').catch(failed); + await vi.advanceTimersByTimeAsync(10 * 60_000); + expect(failed).not.toHaveBeenCalled(); + client.halt(); + }); +}); + describe('the owner tab calling its own worker', () => { it('waits for the worker on the channel, since a call posted before that is lost', async () => { const worker = new FakeChannel('bulwark-store-rpc'); @@ -165,7 +208,7 @@ describe('purgeStoreFiles', () => { const order: string[] = []; Object.defineProperty(window.navigator, 'locks', { configurable: true, - value: { request: async (name: string, run: () => Promise) => { order.push(`lock ${name}`); await run(); order.push('unlock'); } }, + value: { request: async (name: string, _options: unknown, run: () => Promise) => { order.push(`lock ${name}`); await run(); order.push('unlock'); } }, }); storage(async (name, options) => { order.push(`remove ${name} ${JSON.stringify(options)}`); @@ -176,6 +219,25 @@ describe('purgeStoreFiles', () => { expect((await indexedDB.databases()).map((d) => d.name)).not.toContain('bulwark-store'); }); + it('deletes what it can when another tab never lets go of the owner lock', async () => { + const remove = vi.fn(async () => {}); + storage(remove); + let waited: AbortSignal | undefined; + Object.defineProperty(window.navigator, 'locks', { + configurable: true, + value: { + // The browser gives up when the signal says so. + request: async (_name: string, options: { signal: AbortSignal }) => { + waited = options.signal; + throw new DOMException('Took too long', 'TimeoutError'); + }, + }, + }); + await purgeStoreFiles(); + expect(waited).toBeInstanceOf(AbortSignal); + expect(remove).toHaveBeenCalledTimes(1); + }); + it('waits out a worker that still holds the files, and shrugs at a pool that isn\'t there', async () => { const remove = vi.fn() .mockRejectedValueOnce(Object.assign(new Error('busy'), { name: 'NoModificationAllowedError' })) @@ -189,6 +251,12 @@ describe('purgeStoreFiles', () => { }); }); +describe('which files have no key', () => { + it('names store files with no key, and leaves anything else alone', () => { + expect(storeFilesToPrune(['/a01.db', '/a02.db', '/a02.db-journal', '/other', '/ab.DB'], ['/a02.db'])).toEqual(['/a01.db']); + }); +}); + describe('the page\'s store', () => { function allowEngine() { configureStore({ @@ -214,6 +282,84 @@ describe('the page\'s store', () => { expect(worker.terminated).toBe(true); }); + describe('when the key or the file of a store is gone', () => { + const lost = vi.fn(); + const open = async () => { + configureStore({ + inputs: () => ({ session: FAKE_SESSION, auth: 'Basic x', invertedKeywordSort: true }), + refreshAuth: async () => null, + onEvent: () => {}, + onStoreLost: lost, + }); + const store = getStore(); + await store.ensureOpen('lost@x'); + const worker = FakeWorker.all[FakeWorker.all.length - 1]; + worker.becomeOwner(); + await vi.waitFor(() => expect(worker.posted.some((m) => m.kind === 'open')).toBe(true)); + await vi.waitFor(() => expect(localStorage.getItem(STORE_OPENED_KEY)).toContain('lost@x')); + store.stop(); + return worker; + }; + + beforeEach(() => { + lost.mockClear(); + localStorage.clear(); + }); + + afterEach(async () => { + await forgetAccountKey('lost@x'); + }); + + it('stays quiet about a first open, and removes nothing a key still names', async () => { + const worker = await open(); + expect(lost).not.toHaveBeenCalled(); + const prune = worker.posted.find((m) => m.kind === 'prune-files') as unknown as { keep: string[] }; + const record = await getAccountRecord('lost@x'); + expect(prune.keep).toContain(record!.file); + }); + + it('deletes the orphan and tells the page when the key went', async () => { + await open(); + const before = await getAccountRecord('lost@x'); + await forgetAccountKey('lost@x'); + const worker = await open(); + const after = await getAccountRecord('lost@x'); + expect(after!.file).not.toBe(before!.file); + const prune = worker.posted.find((m) => m.kind === 'prune-files') as unknown as { keep: string[] }; + expect(prune.keep).toContain(after!.file); + expect(prune.keep).not.toContain(before!.file); + expect(lost).toHaveBeenCalledWith('lost@x'); + }); + + it('stays quiet on a normal reopen', async () => { + await open(); + const worker = await open(); + expect(lost).not.toHaveBeenCalled(); + expect(worker.posted.some((m) => m.kind === 'prune-files')).toBe(false); + }); + + it('tells the page when the file went and the key stayed', async () => { + await open(); + FakeWorker.openResult = { created: true }; + try { + await open(); + } finally { + FakeWorker.openResult = undefined; + } + expect(lost).toHaveBeenCalledWith('lost@x'); + }); + + it('doesn\'t remember an account that was removed', async () => { + await open(); + vi.spyOn(StoreClient.prototype, 'removeAccount').mockResolvedValue(true as never); + await getStore().remove('lost@x'); + getStore().stop(); + vi.restoreAllMocks(); + await open(); + expect(lost).not.toHaveBeenCalled(); + }); + }); + it('runs the client\'s refresh when the worker needs auth, and hands the result over', async () => { allowEngine(); const store = getStore(); diff --git a/lib/store/client/api.ts b/lib/store/client/api.ts index 09cb18af2..83cb83655 100644 --- a/lib/store/client/api.ts +++ b/lib/store/client/api.ts @@ -7,7 +7,7 @@ import { DEFAULT_SCOPE, type StoreEvent, type StoreScope } from '../area'; import type { JmapResponse, MethodCall, StoreSession } from '../engine/transport'; -import { forgetAccountKey, getAccountRecord, openAccountKey, type WrapLayer } from './keys'; +import { forgetAccountKey, getAccountRecord, knownStoreFiles, openAccountKey, type WrapLayer } from './keys'; import { StoreClient } from './store-client'; export interface StoreAccountInputs { @@ -25,6 +25,8 @@ export interface StorePageHooks { onEvent(event: StoreEvent): void; /** Wrap layers on top of the WebCrypto one (desktop keychain, app lock). */ keyLayers?(): readonly WrapLayer[]; + /** An account this device kept mail for has an empty store now: its key or its file went. */ + onStoreLost?(account: string): void; } /** What lib/offline uses. A seam, so its tests run the real engine on node:sqlite. */ @@ -49,6 +51,30 @@ export interface StoreBackend { const OPEN_RETRIES = 25; const OPEN_RETRY_MS = 200; +/** + * localStorage: accounts that have had a store here. Holds nothing but the + * account's id, and it outlives what a browser may clear of the rest. + */ +export const STORE_OPENED_KEY = 'bulwark-store-opened'; + +function readOpenedAccounts(): string[] { + try { + const parsed = JSON.parse(localStorage.getItem(STORE_OPENED_KEY) ?? '[]') as unknown; + return Array.isArray(parsed) ? parsed as string[] : []; + } catch { + return []; + } +} + +function rememberOpened(account: string, opened: boolean): void { + try { + const others = readOpenedAccounts().filter((id) => id !== account); + localStorage.setItem(STORE_OPENED_KEY, JSON.stringify(opened ? [...others, account] : others)); + } catch { + // Without it a lost store goes unnoticed, and the store still works. + } +} + class PageStore implements StoreBackend { private client: StoreClient | null = null; private readonly wanted = new Map(); @@ -104,10 +130,12 @@ class PageStore implements StoreBackend { if (running) return running; const run = (async () => { const client = this.started(); - const { keyHex, record } = await openAccountKey(account, this.hooks()?.keyLayers?.() ?? []); + const { keyHex, record, fresh } = await openAccountKey(account, this.hooks()?.keyLayers?.() ?? []); + // The old file, if the key alone went, can never be read again. + if (fresh) await client.pruneFiles(await knownStoreFiles()).catch(() => {}); const inputs = this.hooks()?.inputs(account) ?? { session: null, auth: null, invertedKeywordSort: null }; if (inputs.scope) this.sentScope.set(account, JSON.stringify(inputs.scope)); - await client.openAccount({ + const opened = await client.openAccount({ account, file: record.file, keyHex, @@ -116,6 +144,10 @@ class PageStore implements StoreBackend { invertedKeywordSort: inputs.invertedKeywordSort, auth: inputs.auth, }); + if (fresh || opened?.created) { + if (readOpenedAccounts().includes(account)) this.hooks()?.onStoreLost?.(account); + } + rememberOpened(account, true); })(); this.opening.set(account, run); run.catch(() => this.opening.delete(account)); @@ -183,6 +215,7 @@ class PageStore implements StoreBackend { async remove(account: string): Promise { const record = await getAccountRecord(account); await forgetAccountKey(account); + rememberOpened(account, false); this.wanted.delete(account); this.opening.delete(account); if (record && !this.broken) await this.started().removeAccount(account, record.file); diff --git a/lib/store/client/keys.ts b/lib/store/client/keys.ts index ecd4fe4c2..9723c6710 100644 --- a/lib/store/client/keys.ts +++ b/lib/store/client/keys.ts @@ -83,6 +83,14 @@ export function newStoreFileName(): string { return `/a${toHex(crypto.getRandomValues(new Uint8Array(8)))}.db`; } +/** The file of every account with a key. Any other file in the pool can't be read by anyone. */ +export async function knownStoreFiles(): Promise { + return withDb(async (db) => { + const records = await promisify(db.transaction(ACCOUNTS, 'readonly').objectStore(ACCOUNTS).getAll()) as AccountRecord[]; + return records.map((record) => record.file); + }); +} + export async function getAccountRecord(account: string): Promise { return withDb(async (db) => { const found = await promisify(db.transaction(ACCOUNTS, 'readonly').objectStore(ACCOUNTS).get(account)) as AccountRecord | undefined; @@ -95,7 +103,7 @@ export async function listAccountRecords(): Promise { } /** The account's key in hex, created on first use. Returns the record too, for the file name. */ -export async function openAccountKey(account: string, layers: readonly WrapLayer[] = []): Promise<{ keyHex: string; record: AccountRecord }> { +export async function openAccountKey(account: string, layers: readonly WrapLayer[] = []): Promise<{ keyHex: string; record: AccountRecord; fresh: boolean }> { return withDb(async (db) => { const wrapping = await wrapKey(db); const found = await promisify(db.transaction(ACCOUNTS, 'readonly').objectStore(ACCOUNTS).get(account)) as AccountRecord | undefined; @@ -109,7 +117,7 @@ export async function openAccountKey(account: string, layers: readonly WrapLayer const raw = new Uint8Array(await crypto.subtle.decrypt({ name: 'AES-GCM', iv: asBuffer(found.iv) }, wrapping, asBuffer(data))); const keyHex = toHex(raw); raw.fill(0); - return { keyHex, record: found }; + return { keyHex, record: found, fresh: false }; } const raw = crypto.getRandomValues(new Uint8Array(32)); const iv = crypto.getRandomValues(new Uint8Array(12)); @@ -126,7 +134,7 @@ export async function openAccountKey(account: string, layers: readonly WrapLayer await promisify(db.transaction(ACCOUNTS, 'readwrite').objectStore(ACCOUNTS).put(record)); const keyHex = toHex(raw); raw.fill(0); - return { keyHex, record }; + return { keyHex, record, fresh: true }; }); } diff --git a/lib/store/client/purge.ts b/lib/store/client/purge.ts index c2356dd34..10fb04dfb 100644 --- a/lib/store/client/purge.ts +++ b/lib/store/client/purge.ts @@ -10,6 +10,8 @@ import { deleteKeyDatabase } from './keys'; /** A worker just stopped may hold the files for a moment longer. */ const REMOVE_TRIES = 10; const REMOVE_WAIT_MS = 200; +/** A sign-out doesn't wait for a tab that never lets go of the owner lock. */ +const POOL_LOCK_WAIT_MS = 5_000; async function removePool(): Promise { const root = await navigator.storage.getDirectory(); @@ -28,8 +30,18 @@ async function removePool(): Promise { /** Deletes the pool folder, holding the owner lock so no worker can take the files meanwhile. */ export async function deleteStorePool(): Promise { if (typeof navigator === 'undefined' || typeof navigator.storage?.getDirectory !== 'function') return; - if (navigator.locks) await navigator.locks.request(OWNER_LOCK, removePool); - else await removePool(); + if (!navigator.locks) { + await removePool(); + return; + } + try { + await navigator.locks.request(OWNER_LOCK, { signal: AbortSignal.timeout(POOL_LOCK_WAIT_MS) }, removePool); + } catch (error) { + if ((error as Error)?.name !== 'TimeoutError' && (error as Error)?.name !== 'AbortError') throw error; + // A tab that won't let go, stuck or asleep: the key is gone, so what can be + // deleted goes now, and what it still holds can't be read anyway. + await removePool(); + } } /** diff --git a/lib/store/client/store-client.ts b/lib/store/client/store-client.ts index 13b4af661..66e7c3b92 100644 --- a/lib/store/client/store-client.ts +++ b/lib/store/client/store-client.ts @@ -21,6 +21,9 @@ import { const CALL_TIMEOUT_MS = 60_000; const OWNER_WAIT_MS = 15_000; +/** Calls waiting on the owner ask it this often whether it's still there. */ +const OWNER_PING_MS = 10_000; +const OWNER_PING_MISSES = 3; /** A first sync of a large account runs a long time, and its promise is what ends "syncing". */ const SYNC_TIMEOUT_MS = 30 * 60_000; @@ -58,6 +61,8 @@ export class StoreClient { private isOwner = false; private readonly pending = new Map(); private readonly ownerWaiters: Array<() => void> = []; + private ownerWatch: ReturnType | null = null; + private ownerAnswered: (() => void) | null = null; private lifecycleBound = false; constructor(private readonly options: StoreClientOptions = {}) {} @@ -150,6 +155,7 @@ export class StoreClient { break; case 'owner-changed': this.owner = message.owner; + if (message.owner) this.ownerAnswered?.(); if (message.owner) this.flushOwnerWaiters(); break; case 'shutdown': @@ -175,6 +181,41 @@ export class StoreClient { } } + /** + * A tab that's frozen or stuck keeps the lock, and nothing else would tell. + * While calls wait, the owner is asked to answer; when it doesn't for a while, + * they stop waiting instead of running into their long time limits. + */ + private watchOwner(): void { + if (this.ownerWatch) return; + let unanswered = 0; + this.ownerAnswered = () => { unanswered = 0; }; + this.ownerWatch = setInterval(() => { + if (this.pending.size === 0) { + this.stopWatchingOwner(); + return; + } + if (unanswered >= OWNER_PING_MISSES) { + this.owner = null; + this.stopWatchingOwner(); + for (const [id, pending] of this.pending) { + this.pending.delete(id); + clearTimeout(pending.timer); + pending.reject(new StoreOwnerGoneError()); + } + return; + } + unanswered += 1; + this.channel?.postMessage({ kind: 'who-owns' } satisfies ChannelMessage); + }, OWNER_PING_MS); + } + + private stopWatchingOwner(): void { + if (this.ownerWatch) clearInterval(this.ownerWatch); + this.ownerWatch = null; + this.ownerAnswered = null; + } + private flushOwnerWaiters(): void { for (const waiter of this.ownerWaiters.splice(0)) waiter(); } @@ -202,6 +243,7 @@ export class StoreClient { private async rpc(method: K, params: Parameters[0], timeoutMs = CALL_TIMEOUT_MS): Promise> { await this.waitForOwner(); + this.watchOwner(); const id = crypto.randomUUID(); const result = this.track>(id, timeoutMs); this.channel?.postMessage({ kind: 'rpc', id, from: this.clientId, method, params } as ChannelMessage); @@ -220,10 +262,16 @@ export class StoreClient { return this.isOwner; } - openAccount(params: OpenAccountParams): Promise { + /** `created`: there was no database under this name. */ + openAccount(params: OpenAccountParams): Promise<{ created?: boolean } | undefined> { return this.direct({ kind: 'open', id: crypto.randomUUID(), params }); } + /** Deletes every store file but these. */ + pruneFiles(keep: string[]): Promise { + return this.direct({ kind: 'prune-files', id: crypto.randomUUID(), keep }); + } + closeAccount(account: string): Promise { return this.direct({ kind: 'close-account', id: crypto.randomUUID(), account }); } @@ -266,6 +314,7 @@ export class StoreClient { this.channel = null; this.isOwner = false; this.owner = null; + this.stopWatchingOwner(); for (const [id, pending] of this.pending) { this.pending.delete(id); clearTimeout(pending.timer); diff --git a/lib/store/worker/host.ts b/lib/store/worker/host.ts index 9b56b594b..3a2a7036f 100644 --- a/lib/store/worker/host.ts +++ b/lib/store/worker/host.ts @@ -9,6 +9,11 @@ import { FetchTransport, type StoreSession } from '../engine/transport'; import type { StorageAdapter } from '../storage'; import type { OpenAccountParams, StoreRpc, StoreStatus } from './protocol'; +/** Files in the pool that belong to no key. A name that isn't a store file's is left alone. */ +export function storeFilesToPrune(files: readonly string[], keep: readonly string[]): string[] { + return files.filter((file) => /^\/a[0-9a-f]+\.db$/.test(file) && !keep.includes(file)); +} + export interface HostStorage { open(file: string, keyHex: string): Promise; remove(file: string): boolean; @@ -44,7 +49,7 @@ export class StoreHost { constructor(private readonly options: StoreHostOptions) {} - async openAccount(params: OpenAccountParams): Promise { + async openAccount(params: OpenAccountParams): Promise<{ created: boolean }> { const existing = this.accounts.get(params.account); if (existing) { if (params.session) this.setSession(params.account, params.session); @@ -52,9 +57,10 @@ export class StoreHost { existing.engine.setScope(params.scope); if (params.invertedKeywordSort !== null) existing.engine.setInvertedKeywordSort(params.invertedKeywordSort); void existing.engine.requestSync(); - return; + return { created: false }; } const db = await this.options.storage.open(params.file, params.keyHex); + const created = !db.get("SELECT 1 AS yes FROM sqlite_master WHERE name = 'schema_migration'"); const session = params.session ?? AccountEngine.storedSession(db); if (!session) { db.close(); @@ -85,6 +91,7 @@ export class StoreHost { account.engine.setOnline(this.online); this.accounts.set(params.account, account); void account.engine.requestSync(); + return { created }; } setAuth(account: string, auth: string | null): void { diff --git a/lib/store/worker/protocol.ts b/lib/store/worker/protocol.ts index eefd34f96..5fefc7e0f 100644 --- a/lib/store/worker/protocol.ts +++ b/lib/store/worker/protocol.ts @@ -75,6 +75,7 @@ export type PageToWorker = | { kind: 'init'; clientId: string; moduleUrl: string } | { kind: 'open'; id: string; params: OpenAccountParams } | { kind: 'close-account'; id: string; account: string } + | { kind: 'prune-files'; id: string; keep: string[] } | { kind: 'set-auth'; account: string; auth: string | null } | { kind: 'set-inverted'; account: string; inverted: boolean } | { kind: 'set-session'; account: string; session: StoreSession } diff --git a/lib/store/worker/worker.ts b/lib/store/worker/worker.ts index 52e1c3688..a41ee1405 100644 --- a/lib/store/worker/worker.ts +++ b/lib/store/worker/worker.ts @@ -9,7 +9,7 @@ import { WasmStorage } from '../adapters/wasm'; import { STORE_AREAS } from '../areas'; import { RPC_CHANNEL, OWNER_LOCK, type ChannelMessage, type PageToWorker, type WorkerToPage } from './protocol'; -import { StoreHost } from './host'; +import { StoreHost, storeFilesToPrune } from './host'; const scope = self as unknown as { postMessage(message: WorkerToPage): void; @@ -110,7 +110,14 @@ scope.addEventListener('message', async (event) => { break; case 'open': try { - await requireHost().openAccount(message.params); + reply(message.id, await requireHost().openAccount(message.params)); + } catch (error) { + replyError(message.id, error); + } + break; + case 'prune-files': + try { + for (const file of storeFilesToPrune(storage?.files() ?? [], message.keep)) storage?.remove(file); reply(message.id); } catch (error) { replyError(message.id, error);