diff --git a/.changeset/undeclared-field-preflight.md b/.changeset/undeclared-field-preflight.md new file mode 100644 index 0000000000..3b1aaaf5ab --- /dev/null +++ b/.changeset/undeclared-field-preflight.md @@ -0,0 +1,11 @@ +--- +"@objectstack/objectql": patch +--- + +Refuse undeclared fields on insert at the schema, and keep bound values out of the write-path logs (#8682) + +**A single mistyped field name in a client request no longer writes an entire row's values to disk.** A driver-level write fault is logged by prefixing the fully bound SQL statement — values inlined — to the database's own message, and the logger serializes both `message` and `stack`, so the statement was written twice at ERROR level. Confirmed with planted canaries: the row's values landed in the log alongside the organization id and the acting user id. The insert, update and delete loggers now write the database's own diagnostic — which still names the failing column and the object — with the statement and its bound values cut from both fields. The level, the message and the entry itself are unchanged: a driver fault nobody can debug would be a worse outcome than one logged too loudly. + +**An undeclared field is now refused by the object's field map, before anything runs for a request that was already going to be refused.** Previously an unknown key was caught only at the very end, by the driver, after an id, an auto-number, a normalized name, owner/creator resolution, the column defaults and the app's `beforeInsert` hooks had all been produced for it. The auto-number was the durable damage: the refused request consumed a sequence value and left a permanent gap in a document number an end user reads. `insertMany` now culls such a row per row instead of letting it fail the whole batch. + +The client-facing answer is deliberately unchanged — the same `400 INVALID_FIELD`, with the same message and the same `field` / `object` — and the rethrown error is untouched, so only what reaches the log has moved. Objects whose field map is absent or empty get no verdict at all, and `id` / `created_at` / `updated_at` stay accepted even when a declaration omits them, matching what the read path already tolerates; in every one of those cases the driver remains the backstop it has always been. diff --git a/packages/objectql/src/driver-fault-redaction.test.ts b/packages/objectql/src/driver-fault-redaction.test.ts new file mode 100644 index 0000000000..8153f030e2 --- /dev/null +++ b/packages/objectql/src/driver-fault-redaction.test.ts @@ -0,0 +1,251 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #8682 half B — a driver-level write fault is logged WITHOUT the caller's +// values. +// +// Measured on `origin/main` @ 3508678 with planted canaries, a single insert +// carrying one misspelled field name: +// +// message carries SENSITIVE-CANARY-9f3a2b true +// stack carries SENSITIVE-CANARY-9f3a2b true ← the same statement twice +// +// `Logger.error(msg, error, meta)` serializes exactly `error.message` and +// `error.stack` (`ObjectLogger.write`, and both `@objectstack/observability` +// loggers do the same), so those two fields ARE the exposure — redacting one +// and not the other would have moved the leak rather than closed it. +// +// ⛔ What this suite must never be read as licence for: LOWERING the level or +// DROPPING the entry. Every case below that asserts a value is gone has a +// sibling asserting the line is still there, at `error`, still saying `Insert +// operation failed`, still naming the object AND the failing column the +// database itself named. "Stopped leaking" and "stopped reporting" are the two +// outcomes this file exists to tell apart — a driver fault nobody can debug is +// the tolerant-fallback direction, not the loud one. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { redactBoundStatement, redactStatementFromMessage } from './driver-fault-redaction.js'; + +/** The canaries the card planted, kept verbatim so a leak is unmistakable. */ +const SECRET = 'SENSITIVE-CANARY-9f3a2b'; +const DESCRIPTION = 'DESCRIPTION-VALUE-CANARY'; + +/** knex's shape: the fully bound statement, ` - `, then the database's own words. */ +const BOUND_INSERT = + 'insert into `crm_account` (`description`, `name`, `zzz_secret_field`) values ' + + `('${DESCRIPTION}', 'P689 probe', '${SECRET}')` + + ' returning * - table crm_account has no column named zzz_secret_field'; + +describe('redactStatementFromMessage', () => { + it('keeps the database`s diagnostic and drops the bound statement', () => { + const out = redactStatementFromMessage(BOUND_INSERT); + + expect(out).toContain('table crm_account has no column named zzz_secret_field'); + expect(out).not.toContain(SECRET); + expect(out).not.toContain(DESCRIPTION); + expect(out).not.toContain('insert into'); + }); + + it('cuts at the LAST separator, so a value containing " - " leaves no fragment', () => { + // The reason the cut is the last separator and not the first: cutting early + // would leave the tail of the value standing in what we then log as "the + // diagnostic". + const out = redactStatementFromMessage( + "insert into `t` (`label`) values ('2026 - Q3 secret plan') returning * - table t has no column named label", + ); + + expect(out).toContain('table t has no column named label'); + expect(out).not.toContain('Q3 secret plan'); + }); + + it.each([ + ['postgres', 'insert into "t" ("c") values (\'boundvalue\') - column "c" of relation "t" does not exist', 'column "c" of relation "t" does not exist'], + ['mysql via knex', "insert into `t` (`c`) values ('boundvalue') - Unknown column 'c' in 'field list'", "Unknown column 'c' in 'field list'"], + ['update — values ride the `set` clause', "update `t` set `c` = 'boundvalue' where `id` = 'r1' - table t has no column named c", 'table t has no column named c'], + ['delete — values ride the `where` clause', "delete from `t` where `email` = 'boundvalue@example.com' - no such column: email", 'no such column: email'], + ])('%s', (_dialect, message, diagnostic) => { + const out = redactStatementFromMessage(message); + + expect(out).toContain(diagnostic); + // The bound literal, and the statement that carried it, are both gone. + expect(out).not.toContain('boundvalue'); + expect(out).not.toMatch(/insert into|update `|delete from/); + }); + + it('leaves a driver dump that carries no statement exactly as it was', () => { + // Nothing to cut: these are already the diagnostic, and the column they + // name is the operator's whole answer. + for (const message of [ + 'UNIQUE constraint failed: sys_user.email', + 'NOT NULL constraint failed: sys_team.organization_id', + 'SQLITE_CONSTRAINT_NOTNULL: NOT NULL constraint failed: t.c', + ]) { + expect(redactStatementFromMessage(message)).toBe(message); + } + }); + + it('leaves ordinary business and validation prose alone, dashes included', () => { + // The verdict comes from `looksLikeInternalErrorLeak`, which is pinned from + // both directions in `@objectstack/types`. A hook's own message is not a + // driver dump however it is punctuated, and mangling it would replace a + // real answer with a redaction notice. + for (const message of [ + 'name is required', + 'Order 4711 - cannot be closed while lines are open', + "Object 'ghost' is not registered", + '删除被阻断:该客户下仍有未结订单', + ]) { + expect(redactStatementFromMessage(message)).toBe(message); + } + }); +}); + +describe('redactBoundStatement', () => { + it('redacts `stack` too — the statement opened it a second time', () => { + const original = new Error(BOUND_INSERT); + original.name = 'SqliteError'; + original.stack = `SqliteError: ${BOUND_INSERT}\n at Database.prepare (/x/better-sqlite3.js:1:1)\n at create (/x/driver.js:2:2)`; + + const redacted = redactBoundStatement(original) as Error; + + expect(redacted.message).not.toContain(SECRET); + expect(redacted.stack).not.toContain(SECRET); + expect(redacted.stack).not.toContain(DESCRIPTION); + // The frames survive: the redaction narrows WHAT is written, it does not + // take the operator's stack away. + expect(redacted.stack).toContain('at Database.prepare (/x/better-sqlite3.js:1:1)'); + expect(redacted.stack).toContain('at create (/x/driver.js:2:2)'); + expect(redacted.stack).toContain('SqliteError: table crm_account has no column named zzz_secret_field'); + }); + + it('rebuilds a header that spanned several lines', () => { + // A bound statement can contain newlines, so the header is not reliably one + // line — which is why the frames are found rather than the message matched. + const original = new Error("insert into `t`\n(`c`)\nvalues ('multi\nline secret') - table t has no column named c"); + original.name = 'SqliteError'; + original.stack = `SqliteError: ${original.message}\n at only (/x/y.js:1:1)`; + + const redacted = redactBoundStatement(original) as Error; + + expect(redacted.stack).not.toContain('line secret'); + expect(redacted.stack).toBe('SqliteError: table t has no column named c [statement and bound values redacted]\n at only (/x/y.js:1:1)'); + }); + + it('returns the SAME error when there is nothing to redact', () => { + // Identity, not equality: a validation failure, a hook's business error and + // a bare `Error` from our own code must reach the log untouched, and the + // cheapest proof of that is that no new object was made. + const untouched = new Error('name is required'); + + expect(redactBoundStatement(untouched)).toBe(untouched); + }); + + it('passes a non-Error through unchanged', () => { + expect(redactBoundStatement('a thrown string')).toBe('a thrown string'); + expect(redactBoundStatement(undefined)).toBe(undefined); + }); +}); + +/** + * The engine-level half: the SAME insert the card measured, but on a DECLARED + * field whose physical column is missing — schema drift, the one shape that + * still reaches the driver after half A's door closed the undeclared-key route. + * This is the case the redaction exists for, and the case where the ERROR line + * must survive intact. + */ +describe('#8682 half B — the write-path loggers', () => { + function makeCapturingLogger() { + const lines: Array<{ level: string; msg: string; err?: any; meta?: any }> = []; + const logger: any = { + lines, + trace() {}, fatal() {}, + debug() {}, info() {}, + warn(msg: string) { lines.push({ level: 'warn', msg: String(msg) }); }, + error(msg: string, err?: any, meta?: any) { lines.push({ level: 'error', msg: String(msg), err, meta }); }, + child() { return logger; }, + }; + return logger; + } + + async function insertAgainstADriftedColumn() { + const logger = makeCapturingLogger(); + const engine = new ObjectQL({ logger }); + const driver: any = { + name: 'drifted', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return []; }, + async findOne() { return null; }, + async create(object: string, data: Record) { + const cols = Object.keys(data).sort(); + const stmt = `insert into \`${object}\` (${cols.map((c) => `\`${c}\``).join(', ')}) values (${cols.map((c) => `'${String(data[c])}'`).join(', ')}) returning *`; + const e: any = new Error(`${stmt} - table ${object} has no column named secret_note`); + e.name = 'SqliteError'; + e.code = 'SQLITE_ERROR'; + e.stack = `SqliteError: ${stmt} - table ${object} has no column named secret_note\n at Database.prepare (/x/better-sqlite3.js:1:1)`; + throw e; + }, + async update() { return {}; }, async updateMany() { return 0; }, + async delete() { return true; }, async deleteMany() { return 0; }, async count() { return 0; }, + async bulkCreate() { return []; }, async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ + name: 'crm_account', + fields: { + id: { name: 'id', type: 'text', primaryKey: true, readonly: true }, + name: { name: 'name', type: 'text' }, + description: { name: 'description', type: 'text' }, + // DECLARED — and the table does not have it. The door in half A cannot + // see this and must not: it is drift, not a caller mistake. + secret_note: { name: 'secret_note', type: 'text' }, + }, + } as any, 'test'); + + let thrown: any = null; + try { + await engine.insert('crm_account', { + name: 'P689 probe', description: DESCRIPTION, secret_note: SECRET, + } as any); + } catch (e) { thrown = e; } + const line = logger.lines.find((l: any) => l.msg === 'Insert operation failed'); + return { line, thrown }; + } + + it('the entry survives — same level, same message, same object', async () => { + const { line } = await insertAgainstADriftedColumn(); + + expect(line).toBeDefined(); + expect(line!.level).toBe('error'); + expect(line!.meta).toEqual({ object: 'crm_account' }); + }); + + it('the failing column is still named — the fault stays debuggable', async () => { + const { line } = await insertAgainstADriftedColumn(); + + expect(String(line!.err?.message)).toContain('has no column named secret_note'); + expect(String(line!.err?.stack)).toContain('at Database.prepare'); + }); + + it('neither `message` nor `stack` carries a caller value', async () => { + const { line } = await insertAgainstADriftedColumn(); + + for (const field of [String(line!.err?.message), String(line!.err?.stack)]) { + expect(field).not.toContain(SECRET); + expect(field).not.toContain(DESCRIPTION); + expect(field).not.toContain('insert into'); + } + }); + + it('the RETHROWN error is untouched — the caller`s 400 must not move', async () => { + // `mapDataError` reads the driver's raw message to extract the failing + // field and answer `400 INVALID_FIELD`. Redacting what we THROW would break + // that answer; the redaction is one argument at one call site. + const { thrown } = await insertAgainstADriftedColumn(); + + expect(String(thrown?.message)).toContain('insert into'); + expect(String(thrown?.message)).toContain(SECRET); + }); +}); diff --git a/packages/objectql/src/driver-fault-redaction.ts b/packages/objectql/src/driver-fault-redaction.ts new file mode 100644 index 0000000000..3fdda839f4 --- /dev/null +++ b/packages/objectql/src/driver-fault-redaction.ts @@ -0,0 +1,153 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8682] Keep the CALLER'S VALUES out of the server log when a driver-level + * write fault is logged. + * + * ## The exposure this closes + * + * A SQL driver builds its error message by prefixing the offending statement — + * fully bound, values inlined — to the database's own diagnostic. knex's shape + * is ` - `. The engine's write paths log that error, + * and `Logger` serializes exactly two of its fields (`message`, `stack`), so a + * single mistyped field name in a client request wrote an entire row's values + * to disk at ERROR level, twice: + * + * ``` + * ERROR Insert operation failed {"object":"crm_account","error":{"message": + * "insert into `crm_account` (`account_number`, …, `zzz_nonexistent_field`) + * values ('ACC-000011', …, 'SENSITIVE-CANARY-9f3a2b') returning * + * - table crm_account has no column named zzz_nonexistent_field", "stack": + * "SqliteError: insert into `crm_account` (…) values (…) returning * - …"}} + * ``` + * + * Measured with planted canaries: the row's values, the organization id and + * the acting user id all landed in the log, and `stack` re-opened with the + * whole statement a second time — so redacting `message` alone would have + * moved the leak rather than closed it. Both fields are rebuilt here. + * + * ## What is KEPT, deliberately + * + * The driver's own diagnostic — the tail — is the half that names the failing + * column and the condition (`table crm_account has no column named + * zzz_nonexistent_field`, `NOT NULL constraint failed: sys_team.name`). It is + * kept verbatim, and the log site keeps the `object` it already carried. ⛔ The + * remedy for this exposure is NOT to lower the level or drop the entry: a + * driver-level fault that logs nothing is a fault nobody can debug, which is + * strictly worse than one logged too loudly. This narrows WHAT is written; the + * level, the message and the entry are untouched. + * + * ## Why the cut is at the separator, and not at a statement keyword + * + * "Is this a driver dump?" already has ONE owner in this repo — + * {@link looksLikeInternalErrorLeak} in `@objectstack/types`, applied by both + * HTTP boundaries and tested from both directions (it catches dialect codes, + * bare statements and constraint dumps; it deliberately does NOT fire on + * ordinary business or validation prose). Re-deriving a second statement-head + * keyword list here is exactly the drift that module exists to prevent, so this + * asks it the verdict and adds only the one thing it does not answer: WHERE the + * statement ends. + * + * That answer is structural, not lexical. Every value a driver inlines sits in + * the statement, and the statement always comes FIRST — so the last ` - ` in a + * message the shared predicate has already called a driver dump separates the + * part that may carry values from the part that may not. The cut takes the + * LAST separator rather than the first on purpose: a bound value may itself + * contain ` - ` (`'2026 - Q3 plan'`), and cutting at the first would leave a + * fragment of that value standing in the tail. Cutting at the last can only + * ever discard MORE than necessary, which is the safe direction — and when a + * native message legitimately contains ` - `, what is lost is a prefix of the + * diagnostic, never the failing identifier the database names at the end. + * + * A driver dump with no separator carries no statement to cut (`UNIQUE + * constraint failed: sys_user.email`, `SQLITE_CONSTRAINT_NOTNULL: …`) and is + * returned untouched — there is nothing there but the diagnostic already. + * + * ## Not a change to the thrown error + * + * This never mutates and never replaces what the engine rethrows. The REST + * boundary reads the driver's raw message to answer `400 INVALID_FIELD` with + * the failing field name (`mapDataError`), and that answer is correct and must + * stay byte-identical. The redaction applies to the LOG SLOT only — one + * argument at one call site — so the caller's answer and the operator's log + * diverge exactly where they should. + */ + +import { looksLikeInternalErrorLeak } from '@objectstack/types'; + +/** + * knex joins the bound statement to the database's own message with this + * separator. It is the only structural marker the shape offers, and it is + * stable across the dialects this repo runs (`driver-sql`, `driver-turso`, + * `driver-sqlite-wasm` all reach knex). + */ +const STATEMENT_SEPARATOR = ' - '; + +/** What replaces a statement that carried nothing but values. */ +const REDACTED_STATEMENT = '[statement and bound values redacted]'; + +/** + * The first stack FRAME line (` at …`). Everything above it is the header + * that repeats `name: message` — and therefore repeats the statement. + */ +const FIRST_STACK_FRAME = /^[ \t]*at\s/m; + +/** + * Strip the bound statement from a driver error's `message` and `stack`, + * keeping the database's own diagnostic. + * + * Returns the input UNCHANGED (same reference) when there is nothing to + * redact — not a driver dump, or a dump that carries no statement — so a + * validation error, a hook's business error and a bare `Error` from our own + * code all reach the log exactly as before, stack frames included. + * + * Otherwise returns a NEW `Error` carrying the redacted text and the original + * frames. It must be a real `Error`: `ObjectLogger.error` routes its second + * argument by `instanceof Error`, and a plain object would silently land in + * the meta slot instead. + * + * @param error - the thrown value, of any shape. + */ +export function redactBoundStatement(error: unknown): unknown { + if (!(error instanceof Error)) return error; + const redactedMessage = redactStatementFromMessage(error.message); + if (redactedMessage === error.message) return error; + + const redacted = new Error(redactedMessage); + redacted.name = error.name; + redacted.stack = redactStack(error.stack, error.name, redactedMessage); + return redacted; +} + +/** + * The message half, exported for the cases that pin the cut directly. + * Returns the input string unchanged when nothing is cut. + */ +export function redactStatementFromMessage(message: string): string { + if (!message || !looksLikeInternalErrorLeak(message)) return message; + const cut = message.lastIndexOf(STATEMENT_SEPARATOR); + if (cut === -1) return message; + const diagnostic = message.slice(cut + STATEMENT_SEPARATOR.length).trim(); + // A dump whose tail is empty still had its head removed: report the + // redaction rather than an empty message, so the entry never reads as a + // fault with no detail at all. + return diagnostic.length > 0 + ? `${diagnostic} ${REDACTED_STATEMENT}` + : REDACTED_STATEMENT; +} + +/** + * Rebuild `stack` so its header carries the redacted message instead of the + * statement, keeping every frame. + * + * The header is located by the first FRAME line, not by matching the message: + * a bound statement can contain newlines, so the header is not reliably one + * line and a `replace(message, …)` would leave the remainder standing. + */ +function redactStack(stack: string | undefined, name: string, redactedMessage: string): string | undefined { + const header = `${name}: ${redactedMessage}`; + if (stack === undefined) return undefined; + const frame = FIRST_STACK_FRAME.exec(stack); + if (!frame) return header; + return `${header}\n${stack.slice(frame.index)}`; +} diff --git a/packages/objectql/src/engine-insert-many.test.ts b/packages/objectql/src/engine-insert-many.test.ts index 07326115dc..2ac4c12f49 100644 --- a/packages/objectql/src/engine-insert-many.test.ts +++ b/packages/objectql/src/engine-insert-many.test.ts @@ -88,7 +88,14 @@ describe('engine.insertMany — partial-success batch insert (framework#3172)', }, { object: 'task' }); const outcomes = await engine.insertMany('task', [ - { name: 'good1' }, { slug: 'no-name' }, { name: 'good2' }, + // [#8682] The middle row's defect is the MISSING REQUIRED `name` — which + // is what this case is about, and what makes the hook run for it. It used + // to be spelled `{ slug: 'no-name' }`, which carried a second, undeclared + // defect nobody meant to test: `slug` is not a field of `task`, so the + // declared-field door now refuses that row BEFORE the hooks, and the + // assertion below would be measuring the wrong refusal. An empty row says + // exactly the one thing intended. + { name: 'good1' }, {}, { name: 'good2' }, ]); // The #3152 acceptance criterion: hooks fired ONCE per row. diff --git a/packages/objectql/src/engine-undeclared-field-preflight.test.ts b/packages/objectql/src/engine-undeclared-field-preflight.test.ts new file mode 100644 index 0000000000..294ca9028e --- /dev/null +++ b/packages/objectql/src/engine-undeclared-field-preflight.test.ts @@ -0,0 +1,248 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #8682 half A — an undeclared field must be refused by the SCHEMA, before the +// insert path produces anything for a request that is already refused. +// +// The client answer was never the defect: a mistyped field name has always come +// back as a clean `400 INVALID_FIELD`, because the DRIVER refused the statement +// and `mapDataError` translated `table X has no column named c` into that +// envelope. What this suite is about is everything that ran BEHIND that answer. +// +// Measured on `origin/main` @ 3508678, real `ObjectQL` + the recording driver +// below, one mistyped key on a single insert: +// +// autonumber before the bad request 0001 +// the bad request refused — SQLITE_ERROR, from the driver +// autonumber after the bad request 0003 ← gap of 1, permanently +// beforeInsert hooks fired ok-1, bad, ok-2 ← ran for the refused row +// driver create() calls 3 ← the refused row reached the driver +// +// After the door: `0001` → refused → `0002`, hooks `ok-1, ok-2`, two creates. +// +// ## Why the AUTONUMBER is the pin and the log is not +// +// The card's own strongest evidence is the sequence gap, and it is the only +// observable here that does not depend on log contents — which matters +// especially in this PR, whose other half REWRITES what the write-path loggers +// emit. A pin that grepped log text would have rotted the moment half B landed, +// in the same commit that made it pass. So the refusal is asserted on the +// caller's error envelope (ADR-0112 `code` + `status`) and on three +// side-effect counters, and never on a log line. +// +// A sequence gap is not bookkeeping: `ACC-000014` is a document number an end +// user reads on an invoice, and nothing ever reissues it. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; + +/** Records everything that reached the driver — the counts are the point. */ +function makeRecordingDriver(missingColumns: readonly string[] = []) { + const writes: Array<{ fn: string; data: Record }> = []; + const driver: any = { + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return []; }, + async findOne() { return null; }, + async create(object: string, data: Record) { + writes.push({ fn: 'create', data: { ...data } }); + const bad = missingColumns.find((c) => c in data); + // The shape knex produces: the bound statement, then ` - `, then the + // database's own diagnostic. + if (bad) throw new Error(`insert into \`${object}\` (\`${bad}\`) values ('v') returning * - table ${object} has no column named ${bad}`); + return { id: `rec_${writes.length}`, ...data }; + }, + async update(_o: string, id: string, data: Record) { writes.push({ fn: 'update', data: { ...data } }); return { id, ...data }; }, + async updateMany() { return 0; }, + async delete() { return true; }, + async deleteMany() { return 0; }, + async count() { return 0; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => driver.create(object, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, writes }; +} + +/** + * `account_number` is the autonumber whose gap is this suite's observable, and + * the `beforeInsert` hook DERIVES a value the caller never sent — the + * `period_label = 'Q3 2026'` shape the card measured on `crm_forecast`, which + * is what makes "the hook ran" a fact about app behaviour and not just a + * counter. + */ +async function makeEngine(options: { missingColumns?: readonly string[]; registerObject?: boolean } = {}) { + const engine = new ObjectQL({ logger: silentLogger() }); + const { driver, writes } = makeRecordingDriver(options.missingColumns ?? []); + engine.registerDriver(driver, true); + await engine.init(); + if (options.registerObject !== false) { + engine.registry.registerObject({ + name: 'acct', + fields: { + id: { name: 'id', type: 'text', primaryKey: true, readonly: true }, + name: { name: 'name', type: 'text' }, + description: { name: 'description', type: 'text' }, + account_number: { name: 'account_number', type: 'autonumber' }, + }, + } as any, 'test'); + } + const hookRuns: string[] = []; + engine.registerHook('beforeInsert', (ctx: any) => { + hookRuns.push(String(ctx.input.data?.name ?? '?')); + ctx.input.data.description = `derived-for-${ctx.input.data?.name}`; + }, { object: 'acct' }); + return { engine, writes, hookRuns }; +} + +function silentLogger() { + const logger: any = { + trace() {}, debug() {}, info() {}, warn() {}, error() {}, fatal() {}, + child() { return logger; }, + }; + return logger; +} + +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e; + } + return null; +} + +describe('#8682 half A — the declared-field door', () => { + it('REPRODUCED then FIXED: the rejected request consumes no autonumber', async () => { + const { engine } = await makeEngine(); + + const before: any = await engine.insert('acct', { name: 'ok-1' }); + await refusalOf(() => engine.insert('acct', { name: 'bad', zzz_nonexistent_field: 'x' } as any)); + const after: any = await engine.insert('acct', { name: 'ok-2' }); + + // On `origin/main` this read 0001 → 0003. The refused request took 0002 + // into the void; the gap was permanent. + expect(before.account_number).toBe('0001'); + expect(after.account_number).toBe('0002'); + }); + + it('the app`s beforeInsert hook does not run for the refused row', async () => { + const { engine, hookRuns } = await makeEngine(); + + await engine.insert('acct', { name: 'ok-1' }); + await refusalOf(() => engine.insert('acct', { name: 'bad', zzz_nonexistent_field: 'x' } as any)); + await engine.insert('acct', { name: 'ok-2' }); + + // `'bad'` between them on `origin/main`: the hook computed a derived value + // for a request the server had already decided to refuse. + expect(hookRuns).toEqual(['ok-1', 'ok-2']); + }); + + it('nothing reaches the driver', async () => { + const { engine, writes } = await makeEngine(); + + await refusalOf(() => engine.insert('acct', { name: 'bad', zzz_nonexistent_field: 'x' } as any)); + + expect(writes).toHaveLength(0); + }); + + it('refuses in the ADR-0112 envelope, with the wire answer unchanged', async () => { + const { engine } = await makeEngine(); + + const refusal = await refusalOf(() => engine.insert('acct', { name: 'bad', zzz_nonexistent_field: 'x' } as any)); + + // `code` AND `status` — the envelope, not merely "it threw". + expect(refusal?.code).toBe('INVALID_FIELD'); + expect(refusal?.status).toBe(400); + expect(refusal?.field).toBe('zzz_nonexistent_field'); + expect(refusal?.object).toBe('acct'); + // Byte-identical to the message `mapDataError`'s driver-string branch + // produced before this door existed — the refusal moved, the answer did + // not. `@objectstack/rest` re-emits this verbatim as the 400 body's + // `error`, so a change here is a change to the public wire answer. + expect(refusal?.message).toBe("Unknown field 'zzz_nonexistent_field' on object 'acct'"); + }); + + it('names every undeclared key, not only the first', async () => { + const { engine } = await makeEngine(); + + const refusal = await refusalOf(() => engine.insert('acct', { + name: 'bad', zzz_one: 1, zzz_two: 2, + } as any)); + + expect(refusal?.field).toBe('zzz_one'); + expect(refusal?.fields).toEqual(['zzz_one', 'zzz_two']); + }); + + it('a key holding `undefined` is still an undeclared key', async () => { + // A spread of a partial object is the common way this arrives from code + // rather than from JSON, and a mistyped key is a mistyped key whatever it + // holds — the AI-authoring case this door is for. Declared fields holding + // `undefined` are untouched: only the NAME is judged here. + const { engine } = await makeEngine(); + + const refusal = await refusalOf(() => engine.insert('acct', { name: 'bad', zzz_typo: undefined } as any)); + + expect(refusal?.code).toBe('INVALID_FIELD'); + expect(refusal?.field).toBe('zzz_typo'); + }); + + it('a declared field whose physical column is missing still reaches the driver', async () => { + // The door's scope is the SCHEMA's field map, so schema drift — declared + // here, absent in the table — is invisible to it by construction and stays + // the driver's to refuse, exactly as before. `mapDataError`'s driver-string + // branch is what turns that into the caller's 400, and it is still needed. + const { engine, writes } = await makeEngine({ missingColumns: ['description'] }); + + const refusal = await refusalOf(() => engine.insert('acct', { name: 'ok', description: 'v' } as any)); + + expect(writes).toHaveLength(1); + expect(String(refusal?.message)).toContain('has no column named description'); + }); + + it('a batch refuses on the first bad row and writes none of it', async () => { + const { engine, writes, hookRuns } = await makeEngine(); + + const refusal = await refusalOf(() => engine.insert('acct', [ + { name: 'a' }, { name: 'b', zzz_nonexistent_field: 'x' }, { name: 'c' }, + ] as any)); + + expect(refusal?.code).toBe('INVALID_FIELD'); + expect(refusal?.status).toBe(400); + expect(writes).toHaveLength(0); + expect(hookRuns).toEqual([]); + }); + + it('insertMany culls the bad row and writes the good ones', async () => { + // Partial-success mode's whole contract is that one bad row costs the + // others nothing — and before this door an undeclared key defeated it + // completely: the row travelled to `bulkCreate` with the rest and the + // driver failed the WHOLE batch that partial mode exists to protect. + const { engine, writes, hookRuns } = await makeEngine(); + + const outcomes = await engine.insertMany('acct', [ + { name: 'a' }, { name: 'b', zzz_nonexistent_field: 'x' }, { name: 'c' }, + ] as any); + + expect(outcomes.map((o: any) => o.ok)).toEqual([true, false, true]); + expect((outcomes[1] as any).error?.code).toBe('INVALID_FIELD'); + expect((outcomes[1] as any).error?.status).toBe(400); + // The culled row ran no hook and consumed no sequence value: the two good + // rows took 0001 and 0002 with nothing skipped between them. + expect(hookRuns).toEqual(['a', 'c']); + expect(writes.map((w) => w.data.account_number)).toEqual(['0001', '0002']); + }); + + it('a registry-less host gets no verdict — the driver stays the backstop', async () => { + // Same discipline as the read path's doors: a door that cannot see the + // field map must not invent an opinion about it. + const { engine, writes } = await makeEngine({ registerObject: false, missingColumns: ['zzz_nonexistent_field'] }); + + const refusal = await refusalOf(() => engine.insert('acct', { name: 'x', zzz_nonexistent_field: 'x' } as any)); + + expect(writes).toHaveLength(1); + expect(String(refusal?.message)).toContain('has no column named zzz_nonexistent_field'); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 98dd40fad9..0b817c26c3 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -112,6 +112,8 @@ import { isMissingTableError } from '@objectstack/metadata/errors'; // engine is the consumer-side tolerant parsing PD #12 forbids and precedent // #5841 retired. import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; +// [#8682] The write-path loggers' redaction — bound values never reach the log. +import { redactBoundStatement } from './driver-fault-redaction.js'; /** * Per-row outcome of {@link ObjectQL.insertMany} (framework#3172). One entry @@ -977,6 +979,115 @@ function assertProjectionHasNoDottedPaths( throw err; } +/** + * [#8682] The DECLARED-FIELD DOOR on the insert path: a key the object does not + * declare is refused by the SCHEMA, before anything is produced for a request + * that is about to be refused anyway. + * + * ## What ran before this door existed + * + * Nothing rejected an undeclared write key. The row travelled the whole insert + * path and was refused at the very end by the DRIVER — `table crm_account has + * no column named zzz_nonexistent_field` — which `mapDataError` then translated + * into the (correct, unchanged) `400 INVALID_FIELD` the caller sees. The client + * answer was never the defect; what happened behind it was. Measured on + * `origin/main` for one mistyped key: + * + * - an id, a normalized name, owner/creator resolution and the column + * defaults were all produced; + * - the app's `beforeInsert` hooks RAN, and their derived values reached the + * statement (`period_label = 'Q3 2026'`, computed for a request nobody + * would ever store); + * - an AUTONUMBER was issued and CONSUMED — the log-independent observable, + * and the one this door is pinned on: a valid create before the bad request + * numbered `ACC-000013`, the bad request took `ACC-000014` into the void, + * and the next valid create came back `ACC-000015`. A sequence gap is + * permanent and visible to end users on a document number. + * + * A hook is not a pure function — it writes rows, calls out, stamps ledgers — + * so "it ran and we threw the result away" is not a no-op, it is a side effect + * of a request the server had already decided to refuse. + * + * ## Why HERE and not one step later + * + * This runs as the first act inside the middleware body: after middleware (a + * middleware may legitimately rewrite `data`, and judging the caller's keys + * before it ran would refuse rows a sanitizing middleware had already fixed), + * and before `applyFieldDefaults` — hence before the defaults, the summary + * seeding, the hooks, the strips, validation, the autonumber and the statement. + * The same placement, and the same argument, as {@link assertWriteAllowed} and + * `enforceTransactionOrigin` one level up: a refusal this early costs nothing. + * Like those two it throws without logging — the caller is told exactly what is + * wrong, and a client typo is not a server fault to record at ERROR. + * + * ## The verdict is the SCHEMA's field map, and the wire answer is unchanged + * + * `INVALID_FIELD` + 400, and the message is byte-identical to the one + * `mapDataError`'s driver-string branch produced (`Unknown field 'x' on object + * 'y'`) — so this moves WHERE the refusal is decided without moving what the + * caller reads. One condition keeps one wire shape however it was noticed, the + * same rule the read path's dotted-projection and unmaterializable-sort doors + * record. The driver-string branch stays where it is: it still backstops schema + * drift (a DECLARED field whose physical column is missing), which this door by + * construction cannot see. + * + * ## Where this door deliberately has NO opinion + * + * A door that cannot see the field map must not invent an opinion about it — + * the read-path doors' rule, and it decides two cases here: + * + * - **`schema.fields` absent** (a registry-less host), and **`schema.fields` + * EMPTY**. An empty map is indistinguishable from an unpopulated one: a real + * registered object always carries at least its primary key, and the + * registry injects the tenancy and audit columns on top, so zero declared + * fields means the host did not fill the map in — not that the object + * forbids every key. Refusing everything on that reading would be a verdict + * made from an absence, which is what this clause exists to prevent. + * - **`id` / `created_at` / `updated_at`**, which are tolerated even when the + * map does not list them. This mirrors `find()` / `findOne()`, which add + * exactly these three to their known set for exactly this reason: they are + * provisioned by the platform rather than authored, so an object may legally + * omit them from its declaration while the physical table has them. Same + * three names, same rationale, so a key accepted by a read is not refused by + * a write. + * + * In every one of those cases the driver remains the backstop it has always + * been — nothing is widened, the refusal simply moves back to where it was. + * + * @returns one entry per row: an `Error` for a row carrying undeclared keys, + * `undefined` for a row that passes. Per row rather than throw-on-first + * because the partial-success path (`insertMany`) reports per row and must + * cull the bad rows instead of failing the batch around them. + */ +const PLATFORM_PROVISIONED_COLUMNS = ['id', 'created_at', 'updated_at'] as const; + +function undeclaredInsertFieldErrors( + object: string, + schema: { fields?: unknown } | undefined, + rows: readonly unknown[], +): Array { + const out: Array = new Array(rows.length); + const fields = schema?.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return out; + const declared = new Set(Object.keys(fields as Record)); + if (declared.size === 0) return out; + for (const provisioned of PLATFORM_PROVISIONED_COLUMNS) declared.add(provisioned); + for (let i = 0; i < rows.length; i++) { + const row = rows[i]; + if (!row || typeof row !== 'object' || Array.isArray(row)) continue; + const undeclared = Object.keys(row as Record).filter((key) => !declared.has(key)); + if (undeclared.length === 0) continue; + const err: any = new Error(`Unknown field '${undeclared[0]}' on object '${object}'`); + err.status = 400; + err.code = 'INVALID_FIELD'; + err.field = undeclared[0]; + err.fields = undeclared; + err.object = object; + out[i] = err as Error; + } + return out; +} + /** * Evaluate formula virtual fields against the raw rows a driver handed back — * the read path (`find` / `findOne`) and, since #5504, the write path's @@ -7959,6 +8070,23 @@ export class ObjectQL implements IObjectQLEngine { // untouched, hooks run after and may override. const nowSnap = new Date(); const isBatch = Array.isArray(opCtx.data); + // [#8682] The declared-field door — see `undeclaredInsertFieldErrors` for + // what used to run below it for a request that was already refused. + // FIRST, so nothing downstream (defaults, summary seeding, the hooks, the + // secret writes, validation, the autonumber) happens for a row the schema + // rejects. Non-partial callers get the refusal thrown here; the + // partial-success path (`insertMany`) carries it per row into `rowErrors` + // below, where the culled rows also skip the hooks and every producer. + const undeclaredPerRow = undeclaredInsertFieldErrors( + object, + this._registry.getObject(object) as { fields?: unknown } | undefined, + isBatch ? (opCtx.data as unknown[]) : [opCtx.data], + ); + const partialRowMode = isBatch && (options as any)?.__partialRowErrors === true; + if (!partialRowMode) { + const refusal = undeclaredPerRow.find((e) => e !== undefined); + if (refusal) throw refusal; + } // [#4441] The RAW caller payload per row — before `applyFieldDefaults` // resolves any `defaultValue` / `current_user` token and before the // beforeInsert hooks stamp `owner_id` / `organization_id` / @@ -8027,8 +8155,14 @@ export class ObjectQL implements IObjectQLEngine { ql: this, }), ); - for (const rowCtx of rowHookContexts) { - await this.triggerHooks('beforeInsert', rowCtx); + // [#8682] A row the declared-field door refused runs NO hook. In + // non-partial mode the throw above already returned, so this skip only + // ever fires for a culled row of a partial batch — where the whole point + // is that one bad row costs the others nothing, and a hook that stamps a + // ledger or calls out must not fire for a row that will never be written. + for (let i = 0; i < rowHookContexts.length; i++) { + if (undeclaredPerRow[i] !== undefined) continue; + await this.triggerHooks('beforeInsert', rowHookContexts[i]); } // Thread the open transaction (if any) into the driver-facing // options so that knex's `.transacting(trx)` is honoured. Without @@ -8058,9 +8192,16 @@ export class ObjectQL implements IObjectQLEngine { // whole-batch degradation that re-runs beforeInsert hooks on the good // rows. rowErrors[i] set = row i is dead; only live rows reach the // driver / afterInsert / summaries. - const partialMode = isBatch && (options as any)?.__partialRowErrors === true; + const partialMode = partialRowMode; const rowErrors: (unknown | undefined)[] = new Array(rows.length); + // [#8682] Rows the declared-field door refused are already dead on + // arrival — seeded BEFORE the credential loop below, which is the first + // pass with a real side effect (`encryptSecretFields` writes a + // `sys_secret` row), so a culled row cannot mint a secret for a write + // that will never happen. + for (let i = 0; i < rows.length; i++) rowErrors[i] = undeclaredPerRow[i]; for (let i = 0; i < rows.length; i++) { + if (rowErrors[i] !== undefined) continue; try { // [#8559] Both halves of the credential write door, per row: the // password refusal at its own seam, then the secret channel (which @@ -8378,7 +8519,14 @@ export class ObjectQL implements IObjectQLEngine { if (summaryFailures.length > 0) throw new SummaryRecomputeError(summaryFailures, written); return written; } catch (e) { - this.logger.error('Insert operation failed', e as Error, { object }); + // [#8682] Same message, same ERROR level, same `object` — only the + // driver's inlined statement and its bound values are cut, from BOTH + // `message` and `stack` (the logger serializes exactly those two, and + // the stack re-opened with the statement a second time). What the + // database itself said, including the failing column, is kept; the + // error rethrown below is untouched, so the caller's answer does not + // move. See `redactBoundStatement`. + this.logger.error('Insert operation failed', redactBoundStatement(e) as Error, { object }); throw e; } }); @@ -9441,7 +9589,12 @@ export class ObjectQL implements IObjectQLEngine { if (summaryFailures.length > 0) throw new SummaryRecomputeError(summaryFailures, hookContext.result); return hookContext.result; } catch (e) { - this.logger.error('Update operation failed', e as Error, { object }); + // [#8682] The insert logger's twin, and measured to carry the same + // exposure: an UPDATE statement inlines the caller's values in its + // `set` clause exactly as an INSERT does in its `values` list. Same + // redaction, same one argument — the message, the level and the + // `object` are unchanged. + this.logger.error('Update operation failed', redactBoundStatement(e) as Error, { object }); throw e; } }); @@ -10193,8 +10346,14 @@ export class ObjectQL implements IObjectQLEngine { // `deleteBehavior:'cascade'` remedy) would reach no channel at all, // and the server log of a zh-CN deployment would read in Chinese. An // error that carries no `developerMessage` logs exactly as before. + // [#8682] Same redaction as the insert/update loggers. A DELETE's + // statement carries the caller's values in its `where` clause — which + // is how a record is addressed, so it is the same class of payload. + // `developerMessage` is read off the ORIGINAL error: it is written by + // our own throw sites (#7307), never by a driver, so it carries no + // statement and the redaction has no opinion about it. const devDetail = (e as any)?.developerMessage; - this.logger.error('Delete operation failed', e as Error, { + this.logger.error('Delete operation failed', redactBoundStatement(e) as Error, { object, ...(typeof devDetail === 'string' && devDetail.length > 0 ? { developerMessage: devDetail } : {}), }); diff --git a/packages/objectql/src/protocol-recorded-by-null.test.ts b/packages/objectql/src/protocol-recorded-by-null.test.ts index 0f6acf66ff..27bfc776ee 100644 --- a/packages/objectql/src/protocol-recorded-by-null.test.ts +++ b/packages/objectql/src/protocol-recorded-by-null.test.ts @@ -42,6 +42,16 @@ const sysMetadataObject = { type: { name: 'type', label: 'Type', type: 'text' as const, required: true }, name: { name: 'name', label: 'Name', type: 'text' as const, required: true }, organization_id: { name: 'organization_id', label: 'Org', type: 'text' as const }, + // [#8682] Declared on the real `sys_metadata` + // (`metadata-core/src/objects/sys-metadata.object.ts`), where it is part + // of the row's uniqueness key `(type, name, organization_id, + // package_id)`, and written by `SysMetadataRepository` — but omitted by + // this minimal stub. Nothing noticed while an undeclared write key just + // travelled on to the driver; the declared-field door judges the payload + // against this map, so the omission now shows up as the fixture defect + // it always was. (`sys_metadata_history` genuinely carries none, so its + // sibling stub below is left alone.) + package_id: { name: 'package_id', label: 'Package', type: 'text' as const }, metadata: { name: 'metadata', label: 'Body', type: 'longtext' as const }, checksum: { name: 'checksum', label: 'Checksum', type: 'text' as const, maxLength: 71 }, state: { name: 'state', label: 'State', type: 'text' as const }, diff --git a/packages/objectql/src/protocol-registry-shadow.test.ts b/packages/objectql/src/protocol-registry-shadow.test.ts index 688190a7fe..e302e2a0e6 100644 --- a/packages/objectql/src/protocol-registry-shadow.test.ts +++ b/packages/objectql/src/protocol-registry-shadow.test.ts @@ -38,6 +38,16 @@ const sysMetadataObject = { type: { name: 'type', label: 'Type', type: 'text' as const, required: true }, name: { name: 'name', label: 'Name', type: 'text' as const, required: true }, organization_id: { name: 'organization_id', label: 'Org', type: 'text' as const }, + // [#8682] Declared on the real `sys_metadata` + // (`metadata-core/src/objects/sys-metadata.object.ts`), where it is part + // of the row's uniqueness key `(type, name, organization_id, + // package_id)`, and written by `SysMetadataRepository` — but omitted by + // this minimal stub. Nothing noticed while an undeclared write key just + // travelled on to the driver; the declared-field door judges the payload + // against this map, so the omission now shows up as the fixture defect + // it always was. (`sys_metadata_history` genuinely carries none, so its + // sibling stub below is left alone.) + package_id: { name: 'package_id', label: 'Package', type: 'text' as const }, metadata: { name: 'metadata', label: 'Body', type: 'longtext' as const }, checksum: { name: 'checksum', label: 'Checksum', type: 'text' as const, maxLength: 71 }, state: { name: 'state', label: 'State', type: 'text' as const }, diff --git a/packages/objectql/src/protocol-save-meta-repo-path-real-engine.test.ts b/packages/objectql/src/protocol-save-meta-repo-path-real-engine.test.ts index 2248fd4435..4a5f406083 100644 --- a/packages/objectql/src/protocol-save-meta-repo-path-real-engine.test.ts +++ b/packages/objectql/src/protocol-save-meta-repo-path-real-engine.test.ts @@ -22,6 +22,13 @@ const sysMetadataObject = { type: { name: 'type', label: 'Type', type: 'text' as const, required: true }, name: { name: 'name', label: 'Name', type: 'text' as const, required: true }, organization_id: { name: 'organization_id', label: 'Org', type: 'text' as const }, + // [#8682] The real `sys_metadata` carries this — it is part of the + // row's uniqueness key `(type, name, organization_id, package_id)` and + // `SysMetadataRepository` writes it — but this minimal stub had omitted + // it. Nothing noticed while an undeclared write key simply travelled to + // the driver; the declared-field door judges the payload against this + // map, so the omission now shows up as the fixture defect it always was. + package_id: { name: 'package_id', label: 'Package', type: 'text' as const }, metadata: { name: 'metadata', label: 'Body', type: 'longtext' as const }, checksum: { name: 'checksum', label: 'Checksum', type: 'text' as const, maxLength: 71 }, state: { name: 'state', label: 'State', type: 'text' as const }, diff --git a/packages/objectql/src/publish-meta-response-conformance.test.ts b/packages/objectql/src/publish-meta-response-conformance.test.ts index 3cfe11a590..9cbfd42cb4 100644 --- a/packages/objectql/src/publish-meta-response-conformance.test.ts +++ b/packages/objectql/src/publish-meta-response-conformance.test.ts @@ -43,6 +43,16 @@ const sysMetadataObject: ServiceObject = { type: { name: 'type', label: 'Type', type: 'text' as const, required: true }, name: { name: 'name', label: 'Name', type: 'text' as const, required: true }, organization_id: { name: 'organization_id', label: 'Org', type: 'text' as const }, + // [#8682] Declared on the real `sys_metadata` + // (`metadata-core/src/objects/sys-metadata.object.ts`), where it is part + // of the row's uniqueness key `(type, name, organization_id, + // package_id)`, and written by `SysMetadataRepository` — but omitted by + // this minimal stub. Nothing noticed while an undeclared write key just + // travelled on to the driver; the declared-field door judges the payload + // against this map, so the omission now shows up as the fixture defect + // it always was. (`sys_metadata_history` genuinely carries none, so its + // sibling stub below is left alone.) + package_id: { name: 'package_id', label: 'Package', type: 'text' as const }, metadata: { name: 'metadata', label: 'Body', type: 'textarea' as const }, checksum: { name: 'checksum', label: 'Checksum', type: 'text' as const, maxLength: 71 }, state: { name: 'state', label: 'State', type: 'text' as const }, diff --git a/packages/objectql/src/save-meta-response-conformance.test.ts b/packages/objectql/src/save-meta-response-conformance.test.ts index 0325e6510a..a17c48cc64 100644 --- a/packages/objectql/src/save-meta-response-conformance.test.ts +++ b/packages/objectql/src/save-meta-response-conformance.test.ts @@ -56,6 +56,16 @@ const sysMetadataObject: ServiceObject = { type: { name: 'type', label: 'Type', type: 'text' as const, required: true }, name: { name: 'name', label: 'Name', type: 'text' as const, required: true }, organization_id: { name: 'organization_id', label: 'Org', type: 'text' as const }, + // [#8682] Declared on the real `sys_metadata` + // (`metadata-core/src/objects/sys-metadata.object.ts`), where it is part + // of the row's uniqueness key `(type, name, organization_id, + // package_id)`, and written by `SysMetadataRepository` — but omitted by + // this minimal stub. Nothing noticed while an undeclared write key just + // travelled on to the driver; the declared-field door judges the payload + // against this map, so the omission now shows up as the fixture defect + // it always was. (`sys_metadata_history` genuinely carries none, so its + // sibling stub below is left alone.) + package_id: { name: 'package_id', label: 'Package', type: 'text' as const }, metadata: { name: 'metadata', label: 'Body', type: 'textarea' as const }, checksum: { name: 'checksum', label: 'Checksum', type: 'text' as const, maxLength: 71 }, state: { name: 'state', label: 'State', type: 'text' as const },