Address #128 review: unopenable journals, repository binding, and the node:sqlite guard

mobile/jest/in-memory-sqlite.test.ts

@@ -0,0 +1,57 @@
+// The guard in in-memory-sqlite.ts only earns its place if it actually runs on
+// a Node without node:sqlite. It did not: a static `import ... from
+// 'node:sqlite'` is resolved before any module code, so the two SQLite-backed
+// suites errored out on Node below 22.5 instead of skipping - issue #133, on a
+// version `engines` still permits.
+//
+// CI runs 22.x, so nothing here would notice a regression. Making the require
+// throw is the only way to test the old-Node path from a new Node.
+
+// doMock registrations live for the whole file, so each test states which world
+// it wants rather than inheriting the previous one's.
+beforeEach(() => {
+  jest.resetModules();
+  jest.dontMock('node:sqlite');
+});
+
+function withoutNodeSqlite() {
+  jest.doMock('node:sqlite', () => {
+    throw new Error('No such built-in module: node:sqlite');
+  });
+}
+
+describe('on a Node without node:sqlite', () => {
+  it('reports the feature as missing rather than throwing at import', () => {
+    withoutNodeSqlite();
+
+    // The import itself is the assertion: before the fix it threw here.
+    const { HAS_NODE_SQLITE } = require('./in-memory-sqlite');
+
+    expect(HAS_NODE_SQLITE).toBe(false);
+  });
+
+  it('refuses to build a database, and says which Node it needs', () => {
+    withoutNodeSqlite();
+
+    const { createInMemoryDatabase } = require('./in-memory-sqlite');
+
+    expect(() => createInMemoryDatabase()).toThrow(/Node 22.5 or newer/);
+  });
+});
+
+describe('on a Node that has it', () => {
+  it('reports the feature as present and builds a working database', async () => {
+    const { HAS_NODE_SQLITE, createInMemoryDatabase } = require('./in-memory-sqlite');
+
+    expect(HAS_NODE_SQLITE).toBe(true);
+
+    const db = createInMemoryDatabase();
+    try {
+      await db.execAsync('CREATE TABLE t (id TEXT)');
+      await db.runAsync('INSERT INTO t (id) VALUES (?)', 'a');
+      await expect(db.getFirstAsync('SELECT id FROM t')).resolves.toEqual({ id: 'a' });
+    } finally {
+      await db.closeAsync();
+    }
+  });
+});

mobile/jest/in-memory-sqlite.ts

@@ -10,18 +10,29 @@
 // cannot tell you is anything about SQLCipher, which has no Node build and
 // needs a device. Encryption is verified on hardware (issue #101), not here.
 
-import { DatabaseSync } from 'node:sqlite';
+import type { DatabaseSync } from 'node:sqlite';
 import type { SQLiteDatabase } from 'expo-sqlite';
 
-/** True when the running Node has node:sqlite (22.5+). */
-export const HAS_NODE_SQLITE = (() => {
+/**
+ * node:sqlite's `DatabaseSync`, or null on a Node that does not have it.
+ *
+ * The require is lazy on purpose. A static `import { DatabaseSync } from
+ * 'node:sqlite'` is resolved before any code in this module runs, so on Node
+ * below 22.5 the module throws at import time and no guard written here can
+ * catch it - the dependent suites then error out instead of skipping. That is
+ * issue #133, and `engines` currently permits 20.19.4.
+ */
+const DatabaseSyncClass: typeof DatabaseSync | null = (() => {
   try {
-    return typeof DatabaseSync === 'function';
+    return (require('node:sqlite') as typeof import('node:sqlite')).DatabaseSync;
   } catch {
-    return false;
+    return null;
   }
 })();
 
+/** True when the running Node has node:sqlite (22.5+). */
+export const HAS_NODE_SQLITE = DatabaseSyncClass !== null;
+
 /**
  * expo-sqlite accepts either `run(sql, a, b)` or `run(sql, [a, b])`, and this
  * codebase uses both. Collapse them into one array.
@@ -34,7 +45,13 @@ function normaliseParams(params: unknown[]): unknown[] {
 export type InMemoryDatabase = SQLiteDatabase & { readonly raw: DatabaseSync };
 
 export function createInMemoryDatabase(): InMemoryDatabase {
-  const raw = new DatabaseSync(':memory:');
+  if (!DatabaseSyncClass) {
+    throw new Error(
+      'node:sqlite is unavailable - this helper needs Node 22.5 or newer. Guard the suite with HAS_NODE_SQLITE.'
+    );
+  }
+
+  const raw = new DatabaseSyncClass(':memory:');
 
   const db = {
     raw,

mobile/src/components/unlock-gate.test.tsx

@@ -3,14 +3,23 @@ import { AppState, Platform, Text as RNText } from 'react-native';
 
 import { UnlockGate } from './unlock-gate';
 import { checkUnlockAvailability, requestUnlock } from '@/lib/auth/unlock';
+import { closeJournalDatabase } from '@/lib/db/database';
 
 jest.mock('@/lib/auth/unlock', () => ({
   checkUnlockAvailability: jest.fn(),
   requestUnlock: jest.fn(),
 }));
 
+// The gate closes the database when it re-locks. Mocked rather than exercised:
+// the real one needs expo-sqlite and a device, and what matters here is only
+// that the gate asks.
+jest.mock('@/lib/db/database', () => ({
+  closeJournalDatabase: jest.fn(async () => undefined),
+}));
+
 const availability = checkUnlockAvailability as jest.Mock;
 const unlock = requestUnlock as jest.Mock;
+const closeDatabase = closeJournalDatabase as jest.Mock;
 
 const originalOS = Platform.OS;
 function setPlatform(os: typeof Platform.OS) {
@@ -148,9 +157,26 @@ describe('UnlockGate', () => {
     await waitFor(() => expect(screen.queryByText('Journal contents')).toBeNull());
     expect(screen.getByText('Your journal is locked')).toBeVisible();
 
+    // Locking the view is only half of it. While the handle stays open the key
+    // is still in memory and any caller can still read, which makes 0015's
+    // WHEN_UNLOCKED_THIS_DEVICE_ONLY a first-launch property and nothing more.
+    expect(closeDatabase).toHaveBeenCalled();
+
     jest.restoreAllMocks();
   });
 
+  it('does not close a database the web build never opened', async () => {
+    setPlatform('web');
+
+    await render(
+      <UnlockGate>
+        <Journal />
+      </UnlockGate>
+    );
+
+    expect(closeDatabase).not.toHaveBeenCalled();
+  });
+
   it('does not gate the web build, which holds no journal data', async () => {
     setPlatform('web');
 

mobile/src/components/unlock-gate.tsx

@@ -16,6 +16,7 @@ import { ActivityIndicator, AppState, Platform, View } from 'react-native';
 import { Button } from '@/components/ui/button';
 import { Text } from '@/components/ui/text';
 import { checkUnlockAvailability, requestUnlock } from '@/lib/auth/unlock';
+import { closeJournalDatabase } from '@/lib/db/database';
 
 type GateState =
   | { status: 'checking' }
@@ -105,6 +106,18 @@ export function UnlockGate({ children, skip = false }: UnlockGateProps) {
     const subscription = AppState.addEventListener('change', (next) => {
       if (next === 'background') {
         setState({ status: 'locked', message: null });
+
+        // Close the database too, not just the view. The gate is the only thing
+        // that knows the app is meant to be locked, and locking only the UI
+        // leaves db/database.ts holding a decrypted handle and the key in memory
+        // for the life of the process - at which point 0015's
+        // WHEN_UNLOCKED_THIS_DEVICE_ONLY is a property of the first launch and
+        // nothing after it, because the keychain is never asked again. Dropping
+        // the handle here is what makes that option mean something.
+        //
+        // Failures are swallowed on purpose: this runs on the way out of the
+        // foreground, there is nobody to tell, and the next unlock re-opens.
+        void closeJournalDatabase().catch(() => undefined);
       }
     });
 

mobile/src/lib/db/database.test.ts

@@ -6,6 +6,7 @@ import {
   DatabaseUnavailableError,
   destroyJournalDatabase,
   getJournalDatabase,
+  UnrecoverableJournalError,
 } from './database';
 import { deleteDatabaseKey, getOrCreateDatabaseKey } from './key';
 import { migrate } from './migrations';
@@ -57,9 +58,13 @@ beforeEach(async () => {
   setPlatform('ios');
   db = fakeDatabase();
   openDatabaseAsync.mockResolvedValue(db);
-  getKey.mockResolvedValue(KEY);
+  getKey.mockResolvedValue({ key: KEY, created: false });
   migrateMock.mockResolvedValue(0);
   deleteDatabaseAsync.mockResolvedValue(undefined);
+  // Has to resolve, not return undefined: the code under test chains .catch()
+  // onto it, and a bare jest.fn() would throw a TypeError that swallows the
+  // error the caller was actually meant to see.
+  deleteKey.mockResolvedValue(undefined);
 });
 
 afterEach(async () => {
@@ -124,6 +129,59 @@ describe('getJournalDatabase', () => {
   });
 });
 
+describe('a journal that no key can open', () => {
+  /** A key we just minted cannot fail against a file we just created. */
+  function restoredFromAnotherPhone() {
+    getKey.mockResolvedValue({ key: KEY, created: true });
+    db.getFirstAsync.mockRejectedValue(new Error('file is not a database'));
+  }
+
+  it('is reported as its own error rather than a generic open failure', async () => {
+    // Restoring a backup onto a new phone brings journal.db back but not the
+    // key, which 0015 keeps THIS_DEVICE_ONLY on purpose. Saying "could not open
+    // the database" there is true and useless; this is the case that has to be
+    // nameable so something can eventually offer to start over.
+    restoredFromAnotherPhone();
+
+    await expect(getJournalDatabase()).rejects.toThrow(UnrecoverableJournalError);
+  });
+
+  it('takes the useless key back out, so the next launch reaches the same branch', async () => {
+    // Leaving it stored would make created=false next time, and the diagnosis
+    // would degrade to a generic failure that explains nothing.
+    restoredFromAnotherPhone();
+
+    await expect(getJournalDatabase()).rejects.toThrow(UnrecoverableJournalError);
+    expect(deleteKey).toHaveBeenCalled();
+  });
+
+  it('closes the handle it could not read', async () => {
+    restoredFromAnotherPhone();
+
+    await expect(getJournalDatabase()).rejects.toThrow(UnrecoverableJournalError);
+    expect(db.closeAsync).toHaveBeenCalled();
+  });
+
+  it('does not blame a stored key for a decrypt failure, or delete it', async () => {
+    // Same symptom, different cause: the key was already there, so this is
+    // corruption or something else - not a journal from another phone. Deleting
+    // the key here would destroy a database that might still be readable.
+    getKey.mockResolvedValue({ key: KEY, created: false });
+    db.getFirstAsync.mockRejectedValue(new Error('file is not a database'));
+
+    await expect(getJournalDatabase()).rejects.toThrow(DatabaseUnavailableError);
+    expect(deleteKey).not.toHaveBeenCalled();
+  });
+
+  it('leaves a genuine first run alone', async () => {
+    // Fresh install: key minted, empty file, decrypt fine. Nothing to report.
+    getKey.mockResolvedValue({ key: KEY, created: true });
+
+    await expect(getJournalDatabase()).resolves.toBeDefined();
+    expect(deleteKey).not.toHaveBeenCalled();
+  });
+});
+
 describe('closeJournalDatabase', () => {
   it('closes an open handle', async () => {
     await getJournalDatabase();

mobile/src/lib/db/database.ts

@@ -25,6 +25,32 @@ export class DatabaseUnavailableError extends Error {
   }
 }
 
+/**
+ * There is a journal on this device and no key that opens it. The data is gone.
+ *
+ * The path here is not exotic - it is the ordinary one. 0015 stores the key
+ * WHEN_UNLOCKED_THIS_DEVICE_ONLY precisely so it does *not* travel in an iCloud
+ * backup, but `journal.db` lives in Documents and does. So restoring a backup
+ * onto a new phone - which is what people do when they replace a handset -
+ * brings back the file without the key.
+ *
+ * Treating that as a first run and minting a fresh key is the worst available
+ * response: every read then fails, and the app is bricked on every launch after
+ * with a message that explains nothing. So it is called out as its own error.
+ * The journal genuinely cannot be recovered - 0007 commits to telling people
+ * that during onboarding - but "your journal was made on a different phone and
+ * cannot be opened here" is a true thing to say, and it leaves the user
+ * somewhere to go. `destroyJournalDatabase` is the way to start over.
+ *
+ * The UI for that is not built; this is the mechanism it needs.
+ */
+export class UnrecoverableJournalError extends Error {
+  constructor(message: string, options?: { cause?: unknown }) {
+    super(message, options);
+    this.name = 'UnrecoverableJournalError';
+  }
+}
+
 /**
  * Cached as the in-flight promise, not the resolved handle, so that two screens
  * mounting at once share one open rather than racing to create two. Cleared on
@@ -45,17 +71,14 @@ async function open(): Promise<SQLiteDatabase> {
     );
   }
 
-  const key = await getOrCreateDatabaseKey();
+  const { key, created } = await getOrCreateDatabaseKey();
   const db = await SQLite.openDatabaseAsync(DATABASE_NAME);
 
   try {
     // Has to be the first statement executed against the connection.
     await db.execAsync(rawKeyPragma(key));
 
-    // Touch the schema to force SQLCipher to actually decrypt a page. Without
-    // this, a wrong key is not discovered until the first real query, which
-    // could be several screens away from the thing that caused it.
-    await db.getFirstAsync('SELECT count(*) FROM sqlite_master');
+    await assertReadable(db, created);
 
     await db.execAsync('PRAGMA journal_mode = WAL');
     await db.execAsync('PRAGMA foreign_keys = ON');
@@ -64,12 +87,45 @@ async function open(): Promise<SQLiteDatabase> {
   } catch (cause) {
     // Do not leave a half-configured handle behind for the next caller.
     await db.closeAsync().catch(() => undefined);
+    if (cause instanceof UnrecoverableJournalError) throw cause;
     throw new DatabaseUnavailableError('Could not open the journal database.', { cause });
   }
 
   return db;
 }
 
+/**
+ * Reads a page, so a key that does not fit this file is discovered here rather
+ * than several screens away at the first real query.
+ *
+ * What this does not catch: on a build without SQLCipher, `PRAGMA key` is
+ * silently ignored - SQLite ignores unknown pragmas - and this read succeeds
+ * against a plaintext file. Detecting that needs `PRAGMA cipher_version`; see
+ * issue #130.
+ *
+ * What it does catch, given `keyWasCreated`, is the restored-backup case. A key
+ * we just minted cannot fail to read a database we just created, so if it fails
+ * the file was already there and belonged to a key that is gone.
+ */
+async function assertReadable(db: SQLiteDatabase, keyWasCreated: boolean): Promise<void> {
+  try {
+    await db.getFirstAsync('SELECT count(*) FROM sqlite_master');
+  } catch (cause) {
+    if (!keyWasCreated) throw cause;
+
+    // Take the useless key back out. Leaving it would turn a diagnosable state
+    // into an undiagnosable one: the next launch would read a stored key, and
+    // this branch - the only thing that knows what actually happened - would
+    // never run again.
+    await deleteDatabaseKey().catch(() => undefined);
+
+    throw new UnrecoverableJournalError(
+      'There is a journal on this device that no key can open. It was almost certainly restored from a backup of another phone, which does not carry the key. The journal cannot be recovered; starting a new one means erasing it.',
+      { cause }
+    );
+  }
+}
+
 /** The shared database handle, opening and migrating it on first call. */
 export function getJournalDatabase(): Promise<SQLiteDatabase> {
   openPromise ??= open().catch((error: unknown) => {