QA pass on the encrypted mail index found two real gaps beyond what the
prior end-to-end fix pass caught:
1. store.ts's MailIndex.open() only wrapped SOME of the post-key pragma
calls in a try/catch before this: the first attempt's `key` pragma and
assertEncrypted() ran outside any try at all, and the wrong-key retry
repeated the same gap. Any pragma throwing there (SQLITE_BUSY, a full
disk on the first WAL write) leaked the native SQLite handle instead of
closing it. Factored the open+key+verify+pragma sequence into openKeyed(),
which guarantees a close before rethrowing on any failure, and reused it
for both the first attempt and the retry.
2. reindex.ts never removed a deleted contact or file from the index. The
`removed` field exists in the API and is fully tested at the store layer,
but nothing in the renderer populates it, so a deleted contact/file stayed
searchable - and retrievable by the AI feature - indefinitely. Mail and
calendar can't use the same fix (their queries are date-windowed, so an id
missing from one fetch may just be outside the window), but contacts/files
have no date filter - a catch-up fetch that comes back under its cap IS
the complete set, so anything locally indexed but absent from it is safely
known to be deleted. Added strayIdsAfterCatchUp() and wired it into the
catch-up path for those two types only.
Also read binding.ts, key.ts, paths.ts, jmap.ts, extract.ts, the FTS5
query builder, and both /api/offline/{search,reindex} routes end to end;
no other concrete bugs found there. Full findings reported separately.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
308 lines
13 KiB
TypeScript
308 lines
13 KiB
TypeScript
import fs from 'node:fs';
|
|
import os from 'node:os';
|
|
import path from 'node:path';
|
|
import { randomBytes } from 'node:crypto';
|
|
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
|
import { isSqlcipherAvailable } from '../binding';
|
|
import type { SqlcipherConstructor, SqlcipherDatabase, SqlcipherStatement } from '../binding';
|
|
import { accountFileToken, getStoreDir, indexDbPath, STORE_DIR_ENV } from '../paths';
|
|
import { MailIndex, openKeyed, toFtsMatchQuery, type IndexDoc } from '../store';
|
|
|
|
describe('toFtsMatchQuery', () => {
|
|
it('quotes every token so FTS5 operators in user input cannot break the query', () => {
|
|
// FTS5's MATCH grammar is NOT protected by SQL parameter binding: a bare
|
|
// quote or a stray NEAR/AND/* raises `fts5: syntax error`, which would turn
|
|
// a search box into a 500.
|
|
expect(toFtsMatchQuery('a" OR b')).toBe('"a" AND "OR" AND "b"');
|
|
// No trailing `*` here: the final token is one character, below the
|
|
// prefix-match threshold (see the next test).
|
|
expect(toFtsMatchQuery('NEAR(x y)')).toBe('"NEAR" AND "x" AND "y"');
|
|
expect(toFtsMatchQuery('NEAR(x yes)')).toBe('"NEAR" AND "x" AND "yes"*');
|
|
expect(toFtsMatchQuery('foo*')).toBe('"foo"*');
|
|
expect(toFtsMatchQuery('a AND NOT b')).toContain('"NOT"');
|
|
});
|
|
|
|
it('prefix-matches only the final token, and only when it is long enough', () => {
|
|
expect(toFtsMatchQuery('zurich lea')).toBe('"zurich" AND "lea"*');
|
|
// Two characters would match too much of a mailbox to be useful.
|
|
expect(toFtsMatchQuery('zurich le')).toBe('"zurich" AND "le"');
|
|
});
|
|
|
|
it('keeps unicode letters, emails and hyphenated words', () => {
|
|
expect(toFtsMatchQuery('Müller')).toBe('"Müller"*');
|
|
expect(toFtsMatchQuery('東京')).toBe('"東京"');
|
|
expect(toFtsMatchQuery('a@b.com')).toBe('"a@b.com"*');
|
|
expect(toFtsMatchQuery("O'Brien-Smith")).toBe('"O\'Brien-Smith"*');
|
|
});
|
|
|
|
it('returns null for input with no usable tokens', () => {
|
|
expect(toFtsMatchQuery('')).toBeNull();
|
|
expect(toFtsMatchQuery(' ')).toBeNull();
|
|
expect(toFtsMatchQuery('***')).toBeNull();
|
|
expect(toFtsMatchQuery(undefined as unknown as string)).toBeNull();
|
|
});
|
|
|
|
it('bounds the token count', () => {
|
|
const many = Array.from({ length: 100 }, (_, i) => `w${i}`).join(' ');
|
|
expect((toFtsMatchQuery(many) ?? '').split(' AND ')).toHaveLength(24);
|
|
});
|
|
});
|
|
|
|
describe('paths', () => {
|
|
const original = process.env[STORE_DIR_ENV];
|
|
afterEach(() => {
|
|
if (original === undefined) delete process.env[STORE_DIR_ENV];
|
|
else process.env[STORE_DIR_ENV] = original;
|
|
});
|
|
|
|
it('is disabled unless the env var is set - the hosted-deployment gate', () => {
|
|
delete process.env[STORE_DIR_ENV];
|
|
expect(getStoreDir()).toBeNull();
|
|
process.env[STORE_DIR_ENV] = '';
|
|
expect(getStoreDir()).toBeNull();
|
|
});
|
|
|
|
it('rejects a relative path, which would resolve against the server cwd', () => {
|
|
process.env[STORE_DIR_ENV] = 'offline';
|
|
expect(getStoreDir()).toBeNull();
|
|
process.env[STORE_DIR_ENV] = '/abs/offline';
|
|
expect(getStoreDir()).toBe('/abs/offline');
|
|
});
|
|
|
|
it('hashes the filename so the directory is not an account inventory', () => {
|
|
const token = accountFileToken('linus@example.com');
|
|
expect(token).toMatch(/^[0-9a-f]{32}$/);
|
|
expect(token).not.toContain('linus');
|
|
expect(indexDbPath('/s', 'linus@example.com')).toBe(`/s/index/${token}.db`);
|
|
// Deterministic - the same account must resolve to the same file forever.
|
|
expect(accountFileToken('linus@example.com')).toBe(token);
|
|
});
|
|
});
|
|
|
|
/** A fake `SqlcipherDatabase` whose `pragma()` is driven by the given handler. */
|
|
function fakeSqlcipherCtor(
|
|
pragmaHandler: (source: string) => unknown,
|
|
): { ctor: SqlcipherConstructor; instances: Array<{ closeCalls: number }> } {
|
|
const instances: Array<{ closeCalls: number }> = [];
|
|
function FakeDatabase(this: unknown, _path?: string): SqlcipherDatabase {
|
|
const state = { closeCalls: 0 };
|
|
instances.push(state);
|
|
const stmt: SqlcipherStatement = { run: () => ({ changes: 0, lastInsertRowid: 0 }), get: () => undefined, all: () => [] };
|
|
const db: SqlcipherDatabase = {
|
|
exec: () => {},
|
|
prepare: () => stmt,
|
|
pragma: pragmaHandler,
|
|
close: () => { state.closeCalls += 1; },
|
|
};
|
|
return db;
|
|
}
|
|
return { ctor: FakeDatabase as unknown as SqlcipherConstructor, instances };
|
|
}
|
|
|
|
describe('openKeyed', () => {
|
|
it('closes the connection before rethrowing when a pragma AFTER assertEncrypted fails', () => {
|
|
// The bug this guards: `journal_mode = WAL` (or any pragma after the key
|
|
// check) throwing must not leak the native handle - a `try` that only
|
|
// wrapped SOME of these calls previously let exactly this escape.
|
|
const { ctor, instances } = fakeSqlcipherCtor((source) => {
|
|
if (source === 'cipher_version') return [{ cipher_version: 'fake-4.5.0' }];
|
|
if (source === 'journal_mode = WAL') throw new Error('simulated pragma failure');
|
|
return undefined;
|
|
});
|
|
expect(() => openKeyed(ctor, '/fake/path.db', randomBytes(32))).toThrow(/simulated pragma failure/);
|
|
expect(instances).toHaveLength(1);
|
|
expect(instances[0].closeCalls).toBe(1);
|
|
});
|
|
|
|
it('tolerates the extra close when assertEncrypted itself is the failure (its own close is idempotent)', () => {
|
|
// assertEncrypted closes on its own failure before throwing; openKeyed's
|
|
// catch then calls close again. That second call must be harmless, not a
|
|
// new crash - hence >= 1 rather than a fixed count.
|
|
const { ctor, instances } = fakeSqlcipherCtor((source) => (source === 'cipher_version' ? [] : undefined));
|
|
expect(() => openKeyed(ctor, '/fake/path.db', randomBytes(32))).toThrow(/no SQLCipher support/);
|
|
expect(instances[0].closeCalls).toBeGreaterThanOrEqual(1);
|
|
});
|
|
});
|
|
|
|
function doc(overrides: Partial<IndexDoc> = {}): IndexDoc {
|
|
return {
|
|
jmapAccountId: 'acc1',
|
|
contentType: 'mail',
|
|
id: 'M1',
|
|
title: 'Quarterly budget review',
|
|
people: 'Sophie Müller sophie@example.com',
|
|
body: 'The Zurich office lease renewal needs a decision before September.',
|
|
occurredAt: '2026-08-01T10:00:00Z',
|
|
metadata: { threadId: 'T1' },
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
// The native binding is an OPTIONAL dependency, so these skip rather than fail
|
|
// on a platform with no prebuild (e.g. Alpine/musl in CI containers).
|
|
describe.skipIf(!isSqlcipherAvailable())('MailIndex (real SQLCipher)', () => {
|
|
let storeDir: string;
|
|
const accountId = 'linus@example.com';
|
|
const key = randomBytes(32);
|
|
|
|
beforeEach(() => {
|
|
storeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mail-index-test-'));
|
|
});
|
|
afterEach(() => {
|
|
fs.rmSync(storeDir, { recursive: true, force: true });
|
|
});
|
|
|
|
const open = () => MailIndex.open({ storeDir, accountId, key });
|
|
|
|
it('writes an ENCRYPTED file - no plaintext recoverable from the raw bytes', () => {
|
|
const index = open();
|
|
index.upsert([doc()]);
|
|
index.close();
|
|
|
|
const bytes = fs.readFileSync(indexDbPath(storeDir, accountId));
|
|
// The canary check, not just a header check: this is the assertion that
|
|
// would have caught `PRAGMA key` being a silent no-op.
|
|
expect(bytes.includes('Zurich office lease')).toBe(false);
|
|
expect(bytes.includes('Quarterly budget')).toBe(false);
|
|
expect(bytes.subarray(0, 15).toString('latin1')).not.toBe('SQLite format 3');
|
|
});
|
|
|
|
it('rejects a wrong key and rebuilds instead of throwing at the caller', () => {
|
|
const index = open();
|
|
index.upsert([doc()]);
|
|
index.close();
|
|
|
|
// A different key cannot read the data; the store recreates the file rather
|
|
// than surfacing an unrecoverable error, because the index is derived data
|
|
// and the key was never a user secret.
|
|
const other = MailIndex.open({ storeDir, accountId, key: randomBytes(32) });
|
|
expect(other.search({ query: 'Zurich' })).toHaveLength(0);
|
|
other.close();
|
|
});
|
|
|
|
it('refuses a key of the wrong length', () => {
|
|
expect(() => MailIndex.open({ storeDir, accountId, key: randomBytes(16) })).toThrow(/32 bytes/);
|
|
});
|
|
|
|
it('finds documents by body, title and people', () => {
|
|
const index = open();
|
|
index.upsert([doc()]);
|
|
expect(index.search({ query: 'Zurich' }).map((h) => h.id)).toEqual(['M1']);
|
|
expect(index.search({ query: 'quarterly' }).map((h) => h.id)).toEqual(['M1']);
|
|
expect(index.search({ query: 'sophie@example.com' }).map((h) => h.id)).toEqual(['M1']);
|
|
expect(index.search({ query: 'nonexistentword' })).toHaveLength(0);
|
|
index.close();
|
|
});
|
|
|
|
it('returns a snippet for use as LLM context', () => {
|
|
const index = open();
|
|
index.upsert([doc()]);
|
|
const [hit] = index.search({ query: 'Zurich' });
|
|
expect(hit.snippet).toContain('[Zurich]');
|
|
expect(hit.metadata.threadId).toBe('T1');
|
|
index.close();
|
|
});
|
|
|
|
it('upserting the same id REPLACES the FTS row rather than duplicating it', () => {
|
|
const index = open();
|
|
index.upsert([doc()]);
|
|
index.upsert([doc({ body: 'Completely different content about Geneva.' })]);
|
|
|
|
// One row, and the OLD text must no longer match - the classic
|
|
// stale-FTS-row bug when the index is maintained by hand.
|
|
expect(index.search({ query: 'Geneva' })).toHaveLength(1);
|
|
expect(index.search({ query: 'Zurich' })).toHaveLength(0);
|
|
expect(index.stats().find((s) => s.contentType === 'mail')?.count).toBe(1);
|
|
index.close();
|
|
});
|
|
|
|
it('scopes rows by JMAP account, so delegated accounts cannot merge', () => {
|
|
const index = open();
|
|
index.upsert([
|
|
doc({ jmapAccountId: 'acc1', id: 'X', body: 'shared secret alpha' }),
|
|
// Same JMAP id under a different account - legal, since JMAP ids are only
|
|
// unique within an account (see namespaceMailboxIds in lib/jmap/client.ts).
|
|
doc({ jmapAccountId: 'acc2', id: 'X', body: 'shared secret beta' }),
|
|
]);
|
|
expect(index.stats().find((s) => s.contentType === 'mail')?.count).toBe(2);
|
|
const hits = index.search({ query: 'secret' });
|
|
expect(hits).toHaveLength(2);
|
|
expect(new Set(hits.map((h) => h.jmapAccountId))).toEqual(new Set(['acc1', 'acc2']));
|
|
index.close();
|
|
});
|
|
|
|
it('filters by content type and searches across all four by default', () => {
|
|
const index = open();
|
|
index.upsert([
|
|
doc({ contentType: 'mail', id: 'm', title: 'Zurich mail' }),
|
|
doc({ contentType: 'calendar', id: 'c', title: 'Zurich meeting' }),
|
|
doc({ contentType: 'contact', id: 'k', title: 'Zurich person', occurredAt: null }),
|
|
doc({ contentType: 'file', id: 'f', title: 'Zurich file' }),
|
|
]);
|
|
expect(index.search({ query: 'Zurich' })).toHaveLength(4);
|
|
expect(index.search({ query: 'Zurich', types: ['calendar'] }).map((h) => h.id)).toEqual(['c']);
|
|
expect(new Set(index.search({ query: 'Zurich', types: ['mail', 'file'] }).map((h) => h.id)))
|
|
.toEqual(new Set(['m', 'f']));
|
|
index.close();
|
|
});
|
|
|
|
it('weights a title hit above a body-only hit', () => {
|
|
const index = open();
|
|
index.upsert([
|
|
doc({ id: 'body-only', title: 'unrelated', body: 'mentions lease once' }),
|
|
doc({ id: 'in-title', title: 'lease renewal', body: 'unrelated text' }),
|
|
]);
|
|
// bm25 is negative and lower is better, so the title hit must come first.
|
|
expect(index.search({ query: 'lease' })[0].id).toBe('in-title');
|
|
index.close();
|
|
});
|
|
|
|
it('removes documents and their FTS rows', () => {
|
|
const index = open();
|
|
index.upsert([doc()]);
|
|
expect(index.remove('acc1', 'mail', ['M1'])).toBe(1);
|
|
expect(index.search({ query: 'Zurich' })).toHaveLength(0);
|
|
expect(index.remove('acc1', 'mail', ['does-not-exist'])).toBe(0);
|
|
index.close();
|
|
});
|
|
|
|
it('prunes by date without touching newer rows', () => {
|
|
const index = open();
|
|
index.upsert([
|
|
doc({ id: 'old', occurredAt: '2020-01-01T00:00:00Z', body: 'ancient lease' }),
|
|
doc({ id: 'new', occurredAt: '2026-08-01T00:00:00Z', body: 'current lease' }),
|
|
]);
|
|
expect(index.pruneOlderThan('acc1', 'mail', '2026-01-01T00:00:00Z')).toBe(1);
|
|
expect(index.search({ query: 'lease' }).map((h) => h.id)).toEqual(['new']);
|
|
index.close();
|
|
});
|
|
|
|
it('reports existing ids and per-type stats', () => {
|
|
const index = open();
|
|
index.upsert([doc({ id: 'a' }), doc({ id: 'b' }), doc({ contentType: 'file', id: 'f' })]);
|
|
expect(index.existingIds('acc1', 'mail')).toEqual(new Set(['a', 'b']));
|
|
const stats = index.stats();
|
|
expect(stats.find((s) => s.contentType === 'mail')?.count).toBe(2);
|
|
expect(stats.find((s) => s.contentType === 'file')?.count).toBe(1);
|
|
index.close();
|
|
});
|
|
|
|
it('survives reopening and keeps the data', () => {
|
|
const first = open();
|
|
first.upsert([doc()]);
|
|
first.close();
|
|
const second = open();
|
|
expect(second.search({ query: 'Zurich' })).toHaveLength(1);
|
|
second.close();
|
|
});
|
|
|
|
it('tolerates a hostile query string end to end', () => {
|
|
const index = open();
|
|
index.upsert([doc()]);
|
|
for (const q of ['"', '*', 'a" OR "b', 'NEAR(', ')', 'AND', '^', ':', '-']) {
|
|
expect(() => index.search({ query: q })).not.toThrow();
|
|
}
|
|
index.close();
|
|
});
|
|
});
|