diff --git a/mobile/jest/in-memory-sqlite.test.ts b/mobile/jest/in-memory-sqlite.test.ts new file mode 100644 index 0000000..3c6f149 --- /dev/null +++ b/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(); + } + }); +}); diff --git a/mobile/jest/in-memory-sqlite.ts b/mobile/jest/in-memory-sqlite.ts index 8498874..aa976e3 100644 --- a/mobile/jest/in-memory-sqlite.ts +++ b/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, diff --git a/mobile/src/components/unlock-gate.test.tsx b/mobile/src/components/unlock-gate.test.tsx index 62b5bb2..c536d57 100644 --- a/mobile/src/components/unlock-gate.test.tsx +++ b/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( + + + + ); + + expect(closeDatabase).not.toHaveBeenCalled(); + }); + it('does not gate the web build, which holds no journal data', async () => { setPlatform('web'); diff --git a/mobile/src/components/unlock-gate.tsx b/mobile/src/components/unlock-gate.tsx index ba53a1f..7ed26ae 100644 --- a/mobile/src/components/unlock-gate.tsx +++ b/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); } }); diff --git a/mobile/src/lib/db/database.test.ts b/mobile/src/lib/db/database.test.ts index 9807630..5f24c26 100644 --- a/mobile/src/lib/db/database.test.ts +++ b/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(); diff --git a/mobile/src/lib/db/database.ts b/mobile/src/lib/db/database.ts index 402a9a5..f246310 100644 --- a/mobile/src/lib/db/database.ts +++ b/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 { ); } - 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 { } 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 { + 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 { openPromise ??= open().catch((error: unknown) => { diff --git a/mobile/src/lib/db/key.test.ts b/mobile/src/lib/db/key.test.ts index 21351a6..476e5d7 100644 --- a/mobile/src/lib/db/key.test.ts +++ b/mobile/src/lib/db/key.test.ts @@ -37,7 +37,7 @@ describe('getOrCreateDatabaseKey', () => { it('generates, stores and returns a key the first time', async () => { getItemAsync.mockResolvedValue(null); - await expect(getOrCreateDatabaseKey()).resolves.toBe(HEX); + await expect(getOrCreateDatabaseKey()).resolves.toEqual({ key: HEX, created: true }); expect(getRandomBytesAsync).toHaveBeenCalledWith(32); expect(setItemAsync).toHaveBeenCalledWith('journal.database.key', HEX, expect.any(Object)); @@ -46,12 +46,23 @@ describe('getOrCreateDatabaseKey', () => { it('returns the stored key without generating a new one', async () => { getItemAsync.mockResolvedValue(HEX); - await expect(getOrCreateDatabaseKey()).resolves.toBe(HEX); + await expect(getOrCreateDatabaseKey()).resolves.toEqual({ key: HEX, created: false }); expect(getRandomBytesAsync).not.toHaveBeenCalled(); expect(setItemAsync).not.toHaveBeenCalled(); }); + it('reports whether it minted the key, because null is ambiguous', async () => { + // SecureStore returns null both for "nothing stored yet" and for "the entry + // was invalidated". Only database.ts can tell those apart - by finding out + // whether the key opens the file - so this flag has to reach it. + getItemAsync.mockResolvedValue(null); + await expect(getOrCreateDatabaseKey()).resolves.toMatchObject({ created: true }); + + getItemAsync.mockResolvedValue(HEX); + await expect(getOrCreateDatabaseKey()).resolves.toMatchObject({ created: false }); + }); + it('stores the key without requireAuthentication, and device-only', async () => { // This is the decision documented at length in key.ts. If someone turns // requireAuthentication on, adding a fingerprint silently destroys every diff --git a/mobile/src/lib/db/key.ts b/mobile/src/lib/db/key.ts index 7d01ca9..8ea9b5f 100644 --- a/mobile/src/lib/db/key.ts +++ b/mobile/src/lib/db/key.ts @@ -47,9 +47,26 @@ const KEYCHAIN_ENTRY = 'journal.database.key'; * loss, this is the better side of the trade - but it is a product decision as * much as a technical one and it deserves a signed-off ADR of its own. * - * THIS_DEVICE_ONLY additionally keeps the key out of iCloud/Google backups, so - * a restored backup on a new phone cannot decrypt a copied database file. That - * is the same boundary 0003 draws and what issue #115 asks for. + * THIS_DEVICE_ONLY keeps the key out of iCloud backups, so a restored backup on + * a new phone cannot decrypt a copied database file. That is the same boundary + * 0003 draws and what issue #115 asks for. Note that database.ts has to handle + * the other side of that: the file does come back from a backup, so a restored + * phone finds a journal it cannot read and has to say so rather than mint a new + * key over it. + * + * This option is iOS-only - `keychainAccessible` is @platform ios in + * expo-secure-store, so it does nothing on Android. The Android equivalent + * comes from the expo-secure-store config plugin, whose backup rules exclude + * the SecureStore shared preferences from Auto Backup. That makes the bare + * "expo-secure-store" entry in app.json load-bearing rather than registration. + * + * WHEN_PASSCODE_SET_THIS_DEVICE_ONLY is the upgrade that looks free and is not. + * It is stronger, and unlike requireAuthentication it does not bind to + * biometric enrollment - but expo documents it as "the user must have set a + * passcode in order to store an entry. If the user removes their passcode, the + * entry will be deleted." It therefore cannot store a key at all on the devices + * 0018 is about, and it turns removing a passcode into a data-loss event: the + * same catastrophe as the fingerprint case, with a rarer trigger. */ const KEYCHAIN_OPTIONS: SecureStore.SecureStoreOptions = { keychainAccessible: SecureStore.WHEN_UNLOCKED_THIS_DEVICE_ONLY, @@ -67,13 +84,30 @@ function toHex(bytes: Uint8Array): string { return Array.from(bytes, (b) => b.toString(16).padStart(2, '0')).join(''); } +export type DatabaseKey = { + /** Raw key as hex, not a passphrase - see `rawKeyPragma`. */ + readonly key: string; + /** + * True when this call minted the key rather than reading a stored one. + * + * The caller needs this, and the reason is worth stating. `getItemAsync` + * resolves to null "if there is no entry for the given key **or if the key + * has been invalidated**" - expo's own wording. One value, two very different + * situations, and only one of them is a first run. Nothing at this level can + * tell them apart, because that takes knowing whether a database file already + * exists; database.ts can, by trying to decrypt it. So this flag hands the + * question up rather than guessing here. + */ + readonly created: boolean; +}; + /** * Returns the database key, generating and storing one the first time. * * The hex string this returns is a raw key, not a passphrase - see * `rawKeyPragma` for why that distinction matters at the PRAGMA. */ -export async function getOrCreateDatabaseKey(): Promise { +export async function getOrCreateDatabaseKey(): Promise { let existing: string | null; try { existing = await SecureStore.getItemAsync(KEYCHAIN_ENTRY, KEYCHAIN_OPTIONS); @@ -88,7 +122,7 @@ export async function getOrCreateDatabaseKey(): Promise { }); } - if (existing) return existing; + if (existing) return { key: existing, created: false }; const key = toHex(await Crypto.getRandomBytesAsync(KEY_BYTES)); @@ -100,7 +134,7 @@ export async function getOrCreateDatabaseKey(): Promise { }); } - return key; + return { key, created: true }; } /** diff --git a/mobile/src/lib/db/repository.test.ts b/mobile/src/lib/db/repository.test.ts index 08ff407..b9deec8 100644 --- a/mobile/src/lib/db/repository.test.ts +++ b/mobile/src/lib/db/repository.test.ts @@ -101,6 +101,31 @@ describeSql('createRepository', () => { expect(stored?.phone).toBeNull(); }); + it('returns exactly what find() reads back', async () => { + // The two used to disagree: an omitted field was stored as NULL and + // returned as absent, so a screen rendering `created` saw something the + // database did not contain. + const created = await contacts.create({ + name: 'Sam Okafor', + relationship: 'Neighbour', + } as Contact); + + await expect(contacts.find(created.id)).resolves.toEqual(created); + }); + + it('does not hand back keys that were never columns', async () => { + // Undeclared keys are dropped on insert, so echoing them makes the return + // value look like a saved record when it is not. + const created = await contacts.create({ + name: 'Sam', + relationship: 'Friend', + phone: null, + nickname: 'Sammy', + } as never); + + expect(created).not.toHaveProperty('nickname'); + }); + it('treats SQL in a value as text, not as SQL', async () => { const created = await contacts.create({ name: "Robert'); DROP TABLE contacts;--", @@ -189,6 +214,25 @@ describeSql('createRepository', () => { it('throws for an id that is not there', async () => { await expect(contacts.update('nope', { name: 'X' })).rejects.toThrow(RepositoryError); }); + + it('still works when pulled off the repository', async () => { + // `const { update } = repo` and `onPress={repo.update}` are both ordinary + // React. While update reached its sibling through `this`, both threw + // TypeError at runtime, and TypeScript had nothing to say about it. + const created = await contacts.create({ name: 'Alex', relationship: 'Sister', phone: null }); + const { update } = contacts; + + await expect(update(created.id, { name: 'Alexis' })).resolves.toMatchObject({ + name: 'Alexis', + }); + }); + + it('still works detached on the no-op path, which reads a row back too', async () => { + const created = await contacts.create({ name: 'Alex', relationship: 'Sister', phone: null }); + const { update } = contacts; + + await expect(update(created.id, {})).resolves.toMatchObject({ name: 'Alex' }); + }); }); describe('remove', () => { diff --git a/mobile/src/lib/db/repository.ts b/mobile/src/lib/db/repository.ts index abd1d4e..c0b5328 100644 --- a/mobile/src/lib/db/repository.ts +++ b/mobile/src/lib/db/repository.ts @@ -104,36 +104,58 @@ export function createRepository( // millisecond keep a stable order between renders instead of swapping around. const ordering = 'ORDER BY created_at ASC, id ASC'; + // A plain function rather than a method, deliberately. `update` needs to read + // a row back, and reaching a sibling through `this` breaks the moment anyone + // writes `const { update } = repo` or passes `repo.update` as a callback - + // both ordinary in React, neither caught by TypeScript, and the failure is a + // TypeError at runtime. + async function find(id: string): Promise | null> { + const db = await getDatabase(); + const row = await db.getFirstAsync>( + `SELECT ${selection} FROM ${table} WHERE id = ?`, + id + ); + return row ?? null; + } + + /** + * The declared fields, and only those, with anything the caller left out + * turned into the NULL that will actually be stored. + * + * This is what `create` returns rather than the caller's own object, so that + * the entry it hands back matches the row `find` reads. Spreading the input + * instead lets an omitted field come back as `undefined` while the database + * holds `null`, and lets keys that were never columns travel onwards as + * though they had been saved. + */ + function normalise(values: Partial): TFields { + return Object.fromEntries(fields.map((field) => [field, values[field] ?? null])) as TFields; + } + return { async list() { const db = await getDatabase(); return db.getAllAsync>(`SELECT ${selection} FROM ${table} ${ordering}`); }, - async find(id) { - const db = await getDatabase(); - const row = await db.getFirstAsync>( - `SELECT ${selection} FROM ${table} WHERE id = ?`, - id - ); - return row ?? null; - }, + find, async create(values) { const db = await getDatabase(); const id = Crypto.randomUUID(); const now = new Date().toISOString(); + const stored = normalise(values); const columns = ['id', 'created_at', 'updated_at', ...fields]; const placeholders = columns.map(() => '?').join(', '); - const bound = [id, now, now, ...fields.map((field) => values[field] ?? null)]; + const bound = [id, now, now, ...fields.map((field) => stored[field])]; await db.runAsync( `INSERT INTO ${table} (${columns.join(', ')}) VALUES (${placeholders})`, - bound + bound as FieldValue[] ); - return { ...values, id, createdAt: now, updatedAt: now }; + return { ...stored, id, createdAt: now, updatedAt: now }; }, async update(id, changes) { @@ -143,7 +165,7 @@ export function createRepository( if (changed.length === 0) { // Nothing to write. Returning the row as-is beats touching updatedAt // for an edit that changed nothing. - const current = await this.find(id); + const current = await find(id); if (!current) throw new RepositoryError(`No ${table} entry with id ${id}.`); return current; } @@ -161,7 +183,7 @@ export function createRepository( throw new RepositoryError(`No ${table} entry with id ${id}.`); } - const updated = await this.find(id); + const updated = await find(id); if (!updated) throw new RepositoryError(`No ${table} entry with id ${id}.`); return updated; },