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
7 changes: 7 additions & 0 deletions .changeset/claim-seed-ownership-skip-external.md
Original file line number Diff line number Diff line change
@@ -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 <remote_table> 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.
9 changes: 9 additions & 0 deletions .changeset/wasm-sqlite-durability.md
Original file line number Diff line number Diff line change
@@ -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 `<db>.corrupt-<timestamp>`, 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.
10 changes: 10 additions & 0 deletions packages/plugins/driver-sqlite-wasm/src/knex-wasm-dialect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
});
});
121 changes: 118 additions & 3 deletions packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<void> {
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.`,
);
}
}

/**
Expand Down Expand Up @@ -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;
Expand All @@ -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<void> {
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<void> {
if (this.destroyed) return;
Expand Down
19 changes: 19 additions & 0 deletions packages/plugins/plugin-security/src/claim-seed-ownership.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' }] });
Expand Down
11 changes: 10 additions & 1 deletion packages/plugins/plugin-security/src/claim-seed-ownership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 {
Expand Down