fix: delete a store file whose key is gone, say that it happened, and stop waiting on an owner tab that doesn't answer

This commit is contained in:
Linus Rath
2026-11-03 09:18:41 +01:00
parent 18efb9f4c3
commit 9b3bbc6070
9 changed files with 283 additions and 19 deletions
+2 -1
View File
@@ -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';
+151 -5
View File
@@ -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<Record<string, unknown>> = [];
terminated = false;
@@ -33,7 +35,7 @@ class FakeWorker {
}
postMessage(message: Record<string, unknown>): 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<void>) => { order.push(`lock ${name}`); await run(); order.push('unlock'); } },
value: { request: async (name: string, _options: unknown, run: () => Promise<void>) => { 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();
+36 -3
View File
@@ -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<string, StoreSession | null>();
@@ -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<void> {
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);
+11 -3
View File
@@ -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<string[]> {
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<AccountRecord | null> {
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<AccountRecord[]> {
}
/** 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 };
});
}
+14 -2
View File
@@ -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<void> {
const root = await navigator.storage.getDirectory();
@@ -28,8 +30,18 @@ async function removePool(): Promise<void> {
/** Deletes the pool folder, holding the owner lock so no worker can take the files meanwhile. */
export async function deleteStorePool(): Promise<void> {
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();
}
}
/**
+50 -1
View File
@@ -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<string, Pending>();
private readonly ownerWaiters: Array<() => void> = [];
private ownerWatch: ReturnType<typeof setInterval> | 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<K extends keyof StoreRpc>(method: K, params: Parameters<StoreRpc[K]>[0], timeoutMs = CALL_TIMEOUT_MS): Promise<ReturnType<StoreRpc[K]>> {
await this.waitForOwner();
this.watchOwner();
const id = crypto.randomUUID();
const result = this.track<ReturnType<StoreRpc[K]>>(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<void> {
/** `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<void> {
return this.direct({ kind: 'prune-files', id: crypto.randomUUID(), keep });
}
closeAccount(account: string): Promise<void> {
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);
+9 -2
View File
@@ -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<StorageAdapter>;
remove(file: string): boolean;
@@ -44,7 +49,7 @@ export class StoreHost {
constructor(private readonly options: StoreHostOptions) {}
async openAccount(params: OpenAccountParams): Promise<void> {
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 {
+1
View File
@@ -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 }
+9 -2
View File
@@ -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);