diff --git a/.changeset/claim-seed-ownership-skip-external.md b/.changeset/claim-seed-ownership-skip-external.md new file mode 100644 index 0000000000..f5b8dec9f9 --- /dev/null +++ b/.changeset/claim-seed-ownership-skip-external.md @@ -0,0 +1,7 @@ +--- +"@objectstack/plugin-security": patch +--- + +`claimSeedOwnership` now skips **external (federated) objects** — those with an `external` remote-table binding (ADR-0015) — the same way it already skips `managedBy` and `sys_*` objects. + +The seed-ownership backfill walks every registered object that exposes an `owner_id` column and re-owns its unowned rows to the first admin. Federated objects get `owner_id` auto-injected into their schema, so they passed the filter and the backfill issued `select id from where owner_id is null` against a read-only remote datasource whose table may not be provisioned yet at boot — producing startup errors like `Find operation failed … no such table: customers`. External objects are read-only (DDL forbidden, writes double-opt-in) and their ownership is not the platform's to reassign, so they are excluded from the scan entirely. diff --git a/.changeset/wasm-sqlite-durability.md b/.changeset/wasm-sqlite-durability.md new file mode 100644 index 0000000000..1b7f3919cd --- /dev/null +++ b/.changeset/wasm-sqlite-durability.md @@ -0,0 +1,9 @@ +--- +"@objectstack/driver-sqlite-wasm": patch +--- + +Harden the wasm SQLite driver (the dev fallback used when native `better-sqlite3` has an ABI mismatch) against three failure modes that spammed `pnpm dev`: + +- **Atomic flushes.** The database was persisted with a plain `writeFile` that truncates-then-streams in place, so a process killed mid-write (a dev-server restart, Ctrl-C, or crash — likely under `on-write`, where every dispatcher tick flushes) left a torn file that sql.js rejected on the next boot with `database disk image is malformed`. Flushes now write to a sibling temp file, `fsync`, and atomically `rename()` over the target, so a reader always sees a complete image. +- **Corruption self-heal.** When an on-disk image is already corrupt, the driver now detects it at open (via `PRAGMA quick_check`), quarantines the bad file to `.corrupt-`, and boots on a fresh database — instead of failing every query forever with no path to recovery. +- **`undefined` bindings.** A raw `undefined` binding made sql.js `throw` a plain string (`Wrong API use : tried to bind a value of an unknown type (undefined).`), which aborted the write and logged as a garbled char-indexed object. `undefined` is now coerced to SQL `NULL`, matching the driver's `useNullAsDefault` semantics and the native better-sqlite3 path. diff --git a/packages/plugins/driver-sqlite-wasm/src/knex-wasm-dialect.ts b/packages/plugins/driver-sqlite-wasm/src/knex-wasm-dialect.ts index 1183eb2a84..9fc8b39c8b 100644 --- a/packages/plugins/driver-sqlite-wasm/src/knex-wasm-dialect.ts +++ b/packages/plugins/driver-sqlite-wasm/src/knex-wasm-dialect.ts @@ -65,10 +65,20 @@ export interface WasmSqliteConnectionSettings { /** * Coerce JS values that sql.js cannot bind directly. Mirrors * `Client_BetterSQLite3._formatBindings`. + * + * `undefined` is mapped to `null`: sql.js's binder only accepts + * string/number/bigint/boolean/null (and array/blob) and `throw`s a *raw + * string* — `"Wrong API use : tried to bind a value of an unknown type + * (undefined)."` — for anything else. Because it throws a string rather than + * an `Error`, it logs as a garbled char-indexed object and aborts the whole + * write. Mapping to `null` matches the `useNullAsDefault` semantics the + * dialect is configured with, so a missing/undefined value persists as SQL + * `NULL` exactly as it would through better-sqlite3. */ function formatBindings(bindings: unknown[] | undefined): unknown[] { if (!bindings) return []; return bindings.map((b) => { + if (b === undefined) return null; if (b instanceof Date) return b.valueOf(); if (typeof b === 'boolean') return Number(b); return b; diff --git a/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-durability.test.ts b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-durability.test.ts new file mode 100644 index 0000000000..0643aace84 --- /dev/null +++ b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-durability.test.ts @@ -0,0 +1,123 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, afterEach } from 'vitest'; +import { + mkdtempSync, + rmSync, + writeFileSync, + readFileSync, + readdirSync, + existsSync, + statSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { SqliteWasmDriver } from '../src/index.js'; + +/** + * Durability + resilience regressions for the wasm SQLite driver — the dev + * fallback used whenever native better-sqlite3 has an ABI mismatch. + * + * 1. Flushes must be ATOMIC. A non-atomic overwrite torn by a mid-write kill + * left "database disk image is malformed" on the next boot. + * 2. An already-corrupt on-disk image must SELF-HEAL: quarantine the bad file + * and boot on a fresh database instead of failing every query forever. + * 3. `undefined` bindings must persist as SQL NULL rather than throwing sql.js's + * raw-string "Wrong API use : tried to bind a value of an unknown type". + */ +describe('SqliteWasmDriver durability & resilience', () => { + const dirs: string[] = []; + const drivers: SqliteWasmDriver[] = []; + + function newDir(): string { + const dir = mkdtempSync(join(tmpdir(), 'wasm-dur-')); + dirs.push(dir); + return dir; + } + + function track(d: SqliteWasmDriver): SqliteWasmDriver { + drivers.push(d); + return d; + } + + afterEach(async () => { + await Promise.all(drivers.splice(0).map((d) => d.disconnect().catch(() => {}))); + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); + }); + + it('writes the database file atomically (no leftover temp files)', async () => { + const dir = newDir(); + const file = join(dir, 'atomic.db'); + const driver = track(new SqliteWasmDriver({ filename: file, persist: 'on-write' })); + await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]); + + await (driver as any).knex('acct').insert({ id: 'a1', name: 'A' }); + await (driver as any).flush(); + + expect(existsSync(file)).toBe(true); + expect(statSync(file).size).toBeGreaterThan(0); + // The temp file used for the atomic rename must not survive a clean flush. + const leftovers = readdirSync(dir).filter((f) => f.includes('.tmp-')); + expect(leftovers).toEqual([]); + + // The written file is a valid, re-readable SQLite image. + const reopened = track( + new SqliteWasmDriver({ filename: file, persist: 'on-disconnect' }), + ); + const rows = await (reopened as any).knex('acct'); + expect(rows.length).toBe(1); + expect(rows[0].name).toBe('A'); + }); + + it('self-heals a corrupt database image by quarantining it and booting fresh', async () => { + const dir = newDir(); + const file = join(dir, 'corrupt.db'); + // A file that starts with the SQLite magic header but is otherwise garbage: + // sql.js opens it, then blows up on the first page read ("malformed"). + const junk = Buffer.concat([ + Buffer.from('SQLite format 3\0', 'binary'), + Buffer.alloc(4096, 0xab), + ]); + writeFileSync(file, junk); + + const warnings: string[] = []; + const driver = track( + new SqliteWasmDriver({ + filename: file, + persist: 'on-write', + logger: { warn: (m: string) => warnings.push(m) }, + }), + ); + + // Boot must succeed and queries must work on the fresh database. + await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]); + await (driver as any).knex('acct').insert({ id: 'a1', name: 'healed' }); + const rows = await (driver as any).knex('acct'); + expect(rows.length).toBe(1); + + // The corrupt bytes were preserved in a sibling quarantine file, and a + // warning was surfaced. + const quarantined = readdirSync(dir).filter((f) => f.includes('.corrupt-')); + expect(quarantined.length).toBe(1); + expect(readFileSync(join(dir, quarantined[0]))).toEqual(junk); + expect(warnings.some((w) => w.includes('corrupt'))).toBe(true); + }); + + it('persists an undefined binding as SQL NULL instead of throwing', async () => { + const driver = track(new SqliteWasmDriver({ filename: ':memory:' })); + await driver.initObjects([ + { name: 'acct', fields: { name: { type: 'string' }, note: { type: 'string' } } }, + ]); + const knex = (driver as any).knex; + + // A raw binding of `undefined` is exactly what tripped sql.js's binder. + await expect( + knex('acct').insert({ id: 'a1', name: 'A', note: undefined }), + ).resolves.not.toThrow(); + + const rows = await knex('acct').where('id', 'a1'); + expect(rows.length).toBe(1); + expect(rows[0].note).toBeNull(); + }); +}); diff --git a/packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts b/packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts index 384fa99872..826395381c 100644 --- a/packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts +++ b/packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts @@ -97,6 +97,13 @@ async function loadSqlJs( * configurable persistence strategy so the on-disk file stays in sync. */ export class WasmSqliteConnection { + /** + * Process-wide counter making each atomic-write temp filename unique, so + * concurrent connections (or overlapping flushes) never target the same + * temp path. Combined with `process.pid` for cross-process uniqueness. + */ + private static tmpSeq = 0; + readonly filename: string; readonly persist: PersistMode; readonly isEphemeral: boolean; @@ -177,7 +184,71 @@ export class WasmSqliteConnection { if (e?.code !== 'ENOENT') throw e; } - this.db = bytes ? new SQL.Database(bytes) : new SQL.Database(); + if (!bytes) { + this.db = new SQL.Database(); + return; + } + + // Open the on-disk bytes, but guard against a corrupt image. A torn write + // (process killed mid-flush before atomic writes existed) or otherwise + // damaged file makes `new SQL.Database(bytes)` either throw ("file is not a + // database") or open a handle whose every query fails with "database disk + // image is malformed" — which, for a background dispatcher on a tick loop, + // means the same error spammed forever with no path to recovery. Detect it + // once at open, quarantine the bad file, and start fresh so the dev server + // becomes usable again instead of wedging. + try { + const candidate = new SQL.Database(bytes); + this.assertReadable(candidate); + this.db = candidate; + } catch (err) { + await this.quarantineCorruptFile(err); + this.db = new SQL.Database(); + } + } + + /** + * Force sql.js to actually read a page so a malformed image surfaces now + * rather than on the first business query. `PRAGMA quick_check` walks the + * b-tree structure without the full-scan cost of `integrity_check`; a healthy + * database returns a single `ok` row. Any thrown error (raw string or Error) + * or a non-`ok` result is treated as corruption. + */ + private assertReadable(db: Database): void { + const res = db.exec('PRAGMA quick_check(1)'); + const first = res?.[0]?.values?.[0]?.[0]; + if (typeof first === 'string' && first.toLowerCase() !== 'ok') { + throw new Error(`sqlite quick_check failed: ${first}`); + } + } + + /** + * Move a corrupt database file aside so its bytes are preserved for + * post-mortem while a fresh, empty database takes its place. Best-effort: + * failures here must not prevent the server from booting on a clean DB. + */ + private async quarantineCorruptFile(cause: unknown): Promise { + if (!this.fs) return; + const reason = + typeof cause === 'string' ? cause : (cause as Error)?.message ?? String(cause); + const backup = `${this.filename}.corrupt-${Date.now()}`; + try { + await this.fs.rename(this.filename, backup); + this.logger.warn( + `[driver-sqlite-wasm] Database image at ${this.filename} is corrupt ` + + `(${reason}). Quarantined to ${backup} and starting from an empty ` + + `database so the server can boot.`, + ); + } catch (renameErr) { + // Could not move it aside (e.g. permissions) — overwrite is still better + // than looping forever on a malformed image. Warn loudly and continue. + this.logger.warn( + `[driver-sqlite-wasm] Database image at ${this.filename} is corrupt ` + + `(${reason}) and could not be quarantined (${String(renameErr)}). ` + + `Starting from an empty database; the corrupt file will be overwritten ` + + `on the next flush.`, + ); + } } /** @@ -271,8 +342,8 @@ export class WasmSqliteConnection { try { const exported = this.db.export(); // sql.js returns a Uint8Array; Buffer.from on it shares memory but - // works fine for the synchronous writeFile enqueue below. - await this.fs!.writeFile(this.filename, Buffer.from(exported)); + // works fine for the atomic write below. + await this.atomicWriteFile(Buffer.from(exported)); } catch (err) { this.dirty = true; // let a later flush retry throw err; @@ -284,6 +355,50 @@ export class WasmSqliteConnection { await step; } + /** + * Write the database bytes to disk atomically: write to a sibling temp file, + * fsync it, then `rename()` it over the target. + * + * A plain `writeFile(this.filename, …)` truncates the target and streams the + * new bytes in place, so a process killed mid-write (a dev-server restart, + * Ctrl-C, or crash — likely under `on-write`, where every dispatcher tick + * flushes) leaves a half-written file. sql.js then rejects that file on the + * next boot with "database disk image is malformed". `rename(2)` is atomic + * within a filesystem, so a reader always sees either the complete old file + * or the complete new one — never a torn image. The temp file lives in the + * same directory as the target so the rename stays intra-filesystem. + */ + private async atomicWriteFile(data: Buffer): Promise { + if (!this.fs) return; + const tmp = `${this.filename}.tmp-${process.pid}-${(WasmSqliteConnection.tmpSeq += 1)}`; + let handle: import('node:fs/promises').FileHandle | undefined; + try { + handle = await this.fs.open(tmp, 'w'); + await handle.writeFile(data); + // Flush the bytes to the platter before the rename so a crash can't leave + // a renamed-but-empty file behind on filesystems that reorder the two. + await handle.sync(); + await handle.close(); + handle = undefined; + await this.fs.rename(tmp, this.filename); + } catch (err) { + if (handle) { + try { + await handle.close(); + } catch { + /* ignore */ + } + } + // Clean up the temp file so a failed flush doesn't litter the data dir. + try { + await this.fs.unlink(tmp); + } catch { + /* ignore */ + } + throw err; + } + } + /** Close the database, flushing any pending writes first. */ async close(): Promise { if (this.destroyed) return; diff --git a/packages/plugins/plugin-security/src/claim-seed-ownership.test.ts b/packages/plugins/plugin-security/src/claim-seed-ownership.test.ts index a7c08492aa..2fbc8fcc8d 100644 --- a/packages/plugins/plugin-security/src/claim-seed-ownership.test.ts +++ b/packages/plugins/plugin-security/src/claim-seed-ownership.test.ts @@ -55,6 +55,25 @@ describe('claimSeedOwnership', () => { expect(updates).toHaveLength(0); }); + it('skips external (federated) objects even when they expose owner_id', async () => { + // Federated read-only objects (ADR-0015) bind to a remote table; the + // platform must not scan or re-own them — and the remote table may not even + // exist at boot, so a scan would error with "no such table". + const schemas = [ + { + name: 'showcase_ext_customer', + external: { remoteName: 'customers' }, + fields: [{ name: 'owner_id' }], + }, + ]; + const { ql, updates } = makeQL(schemas, { + showcase_ext_customer: [{ id: 'c1', owner_id: null }], + }); + expect(await claimSeedOwnership(ql, ADMIN)).toEqual([]); + expect(ql.find).not.toHaveBeenCalled(); + expect(updates).toHaveLength(0); + }); + it('skips objects without an owner_id field', async () => { const schemas = [{ name: 'crm_pricebook', fields: [{ name: 'name' }] }]; const { ql, updates } = makeQL(schemas, { crm_pricebook: [{ id: 'p1' }] }); diff --git a/packages/plugins/plugin-security/src/claim-seed-ownership.ts b/packages/plugins/plugin-security/src/claim-seed-ownership.ts index 845efc1bc5..2ceff135b3 100644 --- a/packages/plugins/plugin-security/src/claim-seed-ownership.ts +++ b/packages/plugins/plugin-security/src/claim-seed-ownership.ts @@ -54,7 +54,9 @@ function hasOwnerField(schema: ServiceObject): boolean { * Walks `ql.registry.getAllObjects()`, filters to schemas that * (a) are not `managedBy` (skip sys_/auth/platform tables), * (b) are not `sys_*`-namespaced, - * (c) declare an `owner_id` field, + * (c) are not `external` (federated remote-table bindings — read-only, DDL + * forbidden, and their `owner_id` is not ours to reassign), + * (d) declare an `owner_id` field, * and updates the unowned rows as `isSystem`. Returns a per-object summary. */ export async function claimSeedOwnership( @@ -80,6 +82,13 @@ export async function claimSeedOwnership( if (!schema?.name) continue; if ((schema as any).managedBy) continue; if (schema.name.startsWith('sys_')) continue; + // External (federated) objects bind to a remote table on another datasource + // (ADR-0015): reads are remapped, DDL is forbidden, and writes need a double + // opt-in. Their `owner_id` — if the remote even has the column — is not the + // platform's to reassign, and the remote table may not be provisioned when + // this runs at boot (e.g. a fixture that seeds later), so a scan errors with + // "no such table". Skip them entirely. + if ((schema as any).external) continue; if (!hasOwnerField(schema)) continue; try {