Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions apps/mobile/src/lib/persist/encrypted-kv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ let failNextProbe = false;
let failEveryProbe = false;
let failNextPragma = false;
let failEveryPragma = false;
// `PRAGMA journal_mode = WAL` returns the mode the connection switched to.
// node:sqlite's `:memory:` database cannot use WAL, so the fake answers with
// this seam and the test can force a rejected switch.
let journalMode = 'wal';
// Simulates a native handle that will not close. A leaked handle must not be
// replaced by a second connection to the same file.
let failClose = false;
// Simulates a native build without SQLCipher: `PRAGMA cipher_version` returns
// no row, exactly as plain SQLite does for an unrecognized pragma.
let hasSQLCipher = true;
Expand Down Expand Up @@ -124,6 +131,9 @@ function createFakeDatabase(): FakeDatabase {
cipherProbedAtSqlCount = sqlLog.length - 1;
return hasSQLCipher ? { cipher_version: '4.5.5 community' } : null;
}
if (source.startsWith('PRAGMA journal_mode')) {
return { journal_mode: journalMode };
}
return native.prepare(source).get() ?? null;
},
// PRAGMA failure seam: the key call can fail after the handle opened.
Expand All @@ -141,6 +151,9 @@ function createFakeDatabase(): FakeDatabase {
},
closeAsync: async () => {
closeCallCount += 1;
if (failClose) {
throw new Error('handle is stuck open');
}
native.close();
},
};
Expand Down Expand Up @@ -198,6 +211,8 @@ beforeEach(() => {
failEveryProbe = false;
failNextPragma = false;
failEveryPragma = false;
journalMode = 'wal';
failClose = false;
hasSQLCipher = true;
cipherProbedAtSqlCount = -1;
closeCallCount = 0;
Expand Down Expand Up @@ -348,6 +363,35 @@ describe('single-flight open contract', () => {
});
});

describe('connection configuration', () => {
it('configures busy_timeout and WAL after the key and before the first schema read', async () => {
await setItem('s', 'a', 'x');
const cipherIndex = sqlLog.findIndex(source => source.startsWith('PRAGMA cipher_version'));
const keyIndex = sqlLog.findIndex(source => source.startsWith('PRAGMA key'));
const busyIndex = sqlLog.findIndex(source => source.startsWith('PRAGMA busy_timeout'));
const walIndex = sqlLog.findIndex(source => source.startsWith('PRAGMA journal_mode'));
const probeIndex = sqlLog.findIndex(source => source.includes('sqlite_master'));

// The full SQL order: cipher_version, key, busy_timeout, WAL, then the probe.
expect(cipherIndex).toBe(0);
expect(keyIndex).toBeGreaterThan(cipherIndex);
expect(busyIndex).toBeGreaterThan(keyIndex);
expect(walIndex).toBeGreaterThan(busyIndex);
expect(probeIndex).toBeGreaterThan(walIndex);
});

it('sets a non-zero busy timeout value on the connection', async () => {
await setItem('s', 'a', 'x');
const busy = sqlLog.find(source => source.startsWith('PRAGMA busy_timeout'));
expect(busy).toBe('PRAGMA busy_timeout = 5000');
});

it('fails the open when journal_mode does not switch to WAL', async () => {
journalMode = 'delete';
await expect(setItem('s', 'a', 'x')).rejects.toThrow(/journal_mode did not switch to WAL/);
});
});

describe('open failure recovery', () => {
it.each([
{
Expand Down Expand Up @@ -435,6 +479,36 @@ describe('open failure recovery', () => {
await expect(setItem('s', 'a', 'x')).resolves.toBeUndefined();
}
);

it('aborts the reset without opening a second connection when the previous handle does not close', async () => {
failNextProbe = true;
failClose = true;
await expect(setItem('s', 'a', 'x')).rejects.toThrow('file is not a database');

// The probed handle never closed, so recovery must not delete the file or
// open a second connection to it.
expect(SQLite.openDatabaseSync).toHaveBeenCalledTimes(1);
expect(SQLite.deleteDatabaseAsync).not.toHaveBeenCalled();
// The original open error is reported under the existing subsystem tag.
expect(Sentry.captureException).toHaveBeenCalledTimes(1);
expect(Sentry.captureException).toHaveBeenCalledWith(expect.any(Error), {
level: 'error',
tags: { 'error.subsystem': 'encrypted-kv', 'error.operation': 'reset' },
});
});

it('aborts the reset when the failed open could not close its handle', async () => {
failNextPragma = true;
failClose = true;
await expect(setItem('s', 'a', 'x')).rejects.toThrow('PRAGMA key failed');

expect(SQLite.openDatabaseSync).toHaveBeenCalledTimes(1);
expect(SQLite.deleteDatabaseAsync).not.toHaveBeenCalled();
expect(Sentry.captureException).toHaveBeenCalledWith(expect.any(Error), {
level: 'error',
tags: { 'error.subsystem': 'encrypted-kv', 'error.operation': 'reset' },
});
});
});

describe('database key validation', () => {
Expand Down
74 changes: 67 additions & 7 deletions apps/mobile/src/lib/persist/encrypted-kv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ import { kv } from './schema';
const DATABASE_NAME = 'kilo-persist.db';
const KEY_BYTE_COUNT = 32;

// How long a statement waits for another connection's lock before it fails
// with SQLITE_BUSY. Without it, SQLite's default rollback-journal behaviour
// fails a synchronous write the moment a lock is held, which is the reported
// `database is locked` rejection (KILO-APP-7K / KILO-APP-5J / KILO-APP-5H).
const BUSY_TIMEOUT_MS = 5000;

// SQLCipher key format: exactly 64 lowercase hex chars (32 bytes). A stored
// key that does not match is treated as tampered: it must never reach PRAGMA
// interpolation, and the open is routed into delete-and-recreate recovery.
Expand Down Expand Up @@ -128,12 +134,42 @@ function assertSQLCipher(client: SQLite.SQLiteDatabase): void {
}
}

/** Closes the native handle, best-effort: a failed close must not mask the cause. */
async function closeQuietly(client: SQLite.SQLiteDatabase): Promise<void> {
/**
* Open failures whose native handle could not be closed. Delete-and-recreate
* must not run over such a handle, so recovery reports the cause and rethrows
* it instead of opening a second connection to the same file. The error
* identity is the signal, so no wrapper type is needed.
*/
const unclosedOpenErrors = new WeakSet<Error>();

/** Closes the native handle, best-effort. The return value is load-bearing: a
* handle that did not close must not be replaced by a second connection to the
* same file, because that is the `old connection still open` shape behind the
* reported `database is locked` rejection. */
async function closeQuietly(client: SQLite.SQLiteDatabase): Promise<boolean> {
try {
await client.closeAsync();
return true;
} catch {
// Close is best-effort; the delete in the recovery path removes the file.
// Close is best-effort here; the caller decides whether it can proceed.
return false;
}
}

/**
* Configures the single connection to wait out a lock and to use WAL. Both
* pragmas must run after `PRAGMA key` and before any statement reads the
* schema, so a concurrent writer waits instead of failing immediately.
* SQLCipher supports WAL, and `PRAGMA journal_mode = WAL` returns the mode it
* switched to, so the switch is verified, not assumed.
*/
function configureConnection(client: SQLite.SQLiteDatabase): void {
client.execSync(`PRAGMA busy_timeout = ${BUSY_TIMEOUT_MS}`);
const row = client.getFirstSync<{ journal_mode?: string }>('PRAGMA journal_mode = WAL');
if (row?.journal_mode?.toLowerCase() !== 'wal') {
throw new Error(
`encrypted-kv: journal_mode did not switch to WAL (got ${String(row?.journal_mode)})`
);
}
}

Expand All @@ -153,11 +189,15 @@ async function openWithKey(key: string): Promise<KVDatabase> {
try {
assertSQLCipher(client);
client.execSync(`PRAGMA key = "x'${key}'"`);
configureConnection(client);
return drizzle(client);
} catch (error) {
// The PRAGMA failed after the handle opened. Close it before rethrowing
// so the delete-and-recreate recovery never runs with an open handle.
await closeQuietly(client);
// The setup failed after the handle opened. Close it before rethrowing so
// the delete-and-recreate recovery never runs with an open handle. If the
// handle will not close, mark the error: recovery must not replace it.
if (!(await closeQuietly(client)) && error instanceof Error) {
unclosedOpenErrors.add(error);
}
throw error;
}
}
Expand All @@ -170,6 +210,18 @@ async function probeAndMigrate(db: KVDatabase): Promise<void> {
await migrate(db, migrations);
}

/**
* Reports an open failure that could not be recovered, because the previous
* handle is still open, then rethrows the original error.
*/
function reportAbortedReset(cause: unknown): never {
Sentry.captureException(cause, {
level: 'error',
tags: { 'error.subsystem': 'encrypted-kv', 'error.operation': 'reset' },
});
throw cause;
}

async function openEncryptedDatabase(): Promise<KVDatabase> {
const key = await readOrCreateKey();
let db: KVDatabase | undefined = undefined;
Expand All @@ -192,7 +244,15 @@ async function openEncryptedDatabase(): Promise<KVDatabase> {
// recoverable losses. Close, delete the file, regenerate the key, and
// reopen (DEC-01 step 4).
if (db) {
await closeQuietly(db.$client);
// The probe or migration failed after the handle opened. A handle that
// will not close must not be replaced by a second connection to the same
// file: abort the reset and report the original open error instead.
if (!(await closeQuietly(db.$client))) {
reportAbortedReset(openError);
}
} else if (openError instanceof Error && unclosedOpenErrors.has(openError)) {
// The failed open never returned a handle, and its handle is still open.
reportAbortedReset(openError);
}
let reopened: KVDatabase | undefined = undefined;
try {
Expand Down