diff --git a/.changeset/readonly-strip-warning-truthful.md b/.changeset/readonly-strip-warning-truthful.md new file mode 100644 index 0000000000..caf93547eb --- /dev/null +++ b/.changeset/readonly-strip-warning-truthful.md @@ -0,0 +1,40 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): the read-only strip's warning stops promising a commit that `strictReadonlyWrites` refused, and offers `preserveAudit` only where it works (#8214) + +Two claims `readonlyStripWarning` (and its insert-side twin +`runtimeOwnedStripWarning`) were making were not true of the call in front of +them. The level is unchanged — both lines stay at `warn`, so real forgery +attempts stay visible — and both still name the field, the consequence and a +remedy. + +**1. "COMMITTED WITHOUT IT" under `strictReadonlyWrites`.** The strip logs from +inside `stripReadonlyFields` / `stripRuntimeOwnedFields`, while the refusal +throws afterwards and before any driver call. A strict caller was told in prose +that the write had been committed without the field while nothing had been +written at all — a reader debugging from the log alone went hunting for a row +that was never touched. Measured on a real `ObjectQL` plus a recording driver: + +| | refused code | driver writes | warn lines | claimed a commit | +|---|---|---|---|---| +| update, strict | `ERR_READONLY_FIELD_REJECTED` | 0 | 1 | yes | +| insert, strict | `ERR_READONLY_FIELD_REJECTED` | 0 | 1 | yes | + +The strip now learns the flag and reports the refusal instead — naming +`ERR_READONLY_FIELD_REJECTED`, and pointing at dropping `strictReadonlyWrites` +rather than at `onFieldsDropped`, which is the remedy that applies in that mode. +Default (non-strict) writes are byte-identical: there the commit really happens, +and the sentence was always true. + +**2. The remedy named `isSystem` but never `preserveAudit`.** `stripReadonlyFields` +honours `context.preserveAudit`, a whitelist narrower than `isSystem` by +construction, but the message offered only the blanket exemption — so an import +that forgot the flag was steered to the strictly worse posture. The narrower +remedy is now offered **per field, derived from the same predicate the strip +consults**, never from a prose description of the whitelist: a field the flag +would really have kept gets the sentence, and a non-audit `system` column such +as `organization_id` — which `preserveAudit` strips anyway — does not. + +The write's own address still logs nothing (#8141), in strict mode as well. diff --git a/packages/objectql/src/engine-readonly-strip-signal.test.ts b/packages/objectql/src/engine-readonly-strip-signal.test.ts index 4a47a83836..e7061f9d3e 100644 --- a/packages/objectql/src/engine-readonly-strip-signal.test.ts +++ b/packages/objectql/src/engine-readonly-strip-signal.test.ts @@ -217,7 +217,7 @@ describe('static `readonly` write strip — caller-facing signal (#4903)', () => probe, ); expect(calls.map(([level]) => level)).toEqual(['warn']); - expect(calls[0][1]).toBe(readonlyStripWarning('work_duration', 'attendance')); + expect(calls[0][1]).toBe(readonlyStripWarning('work_duration', 'attendance', { preserveAuditApplies: true })); }); it('omits the object clause when the schema carries no name', () => { diff --git a/packages/objectql/src/engine-strict-readonly-warning-truthful.test.ts b/packages/objectql/src/engine-strict-readonly-warning-truthful.test.ts new file mode 100644 index 0000000000..c89d220d3a --- /dev/null +++ b/packages/objectql/src/engine-strict-readonly-warning-truthful.test.ts @@ -0,0 +1,336 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #8214 — the strip's WARN line must not promise a commit that +// `strictReadonlyWrites` refuses. +// +// The strip logs from inside `stripReadonlyFields` / `stripRuntimeOwnedFields`; +// `assertNoStrictDrops()` (update) and the `ReadonlyFieldRejectedError` throw +// (insert) come AFTERWARDS, before any driver call. So a strict caller was told +// in prose that the write had been "COMMITTED WITHOUT IT" while nothing at all +// had been written — the #4632 shape inverted, sending a reader debugging from +// the log alone to hunt for a row that was never touched. +// +// Measured on `origin/main` @ 29488ccae, real `ObjectQL` + the recording driver +// below, before the fix: +// +// UPDATE { strictReadonlyWrites: true }, caller forges `locked_note` +// refusedCode ERR_READONLY_FIELD_REJECTED +// driverWrites 0 +// warnLines 1 +// claimsCommitted true ← the line said "COMMITTED WITHOUT IT" +// +// INSERT { strictReadonlyWrites: true }, caller seeds `account_number` +// refusedCode ERR_READONLY_FIELD_REJECTED +// driverCreates 0 +// warnLines 1 +// claimsCommitted true ← same claim, `runtimeOwnedStripWarning` +// +// The insert half was filed UNVERIFIED and is verified here: it reproduces, on +// the same sequencing, so it is the same defect and not a lookalike. +// +// ⛔ What this suite must never be read as licence for: DROPPING the line in +// strict mode. The level argument in `readonlyStripWarning`'s own docblock is +// that `warn` exists so real forgery attempts stay visible, and a forged write +// that got refused is the case that most deserves a line. Every case below that +// asserts the claim is gone has a sibling asserting the LINE is still there, at +// `warn`, naming the field. "Stopped lying" and "stopped warning" are the two +// outcomes this file exists to tell apart. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { readonlyStripWarning, runtimeOwnedStripWarning } from './validation/rule-validator.js'; + +function makeCapturingLogger() { + const lines: Array<{ level: string; msg: string }> = []; + const logger: any = { + lines, + trace() {}, fatal() {}, + debug(msg: string) { lines.push({ level: 'debug', msg: String(msg) }); }, + info(msg: string) { lines.push({ level: 'info', msg: String(msg) }); }, + warn(msg: string) { lines.push({ level: 'warn', msg: String(msg) }); }, + error(msg: string) { lines.push({ level: 'error', msg: String(msg) }); }, + child() { return logger; }, + }; + return logger; +} + +/** Records every payload that actually reaches the driver — `0` is the point. */ +function makeRecordingDriver() { + const writes: Array<{ fn: string; data: Record }> = []; + const row = { id: 'rec_1', value: 'v0', locked_note: 'n0', title: 't0' }; + 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 [{ ...row }]; }, + async findOne() { return { ...row }; }, + async create(_o: string, data: Record) { + writes.push({ fn: 'create', data: { ...data } }); + return { id: 'rec_1', ...data }; + }, + async update(_o: string, id: string, data: Record) { + writes.push({ fn: 'update', data: { ...data } }); + return { ...row, ...data, id }; + }, + async updateMany(_o: string, _ast: unknown, data: Record) { + writes.push({ fn: 'updateMany', data: { ...data } }); + return 2; + }, + async delete() { return true; }, + async deleteMany() { return 0; }, + async count() { return 1; }, + async bulkCreate(o: string, rows: Record[]) { + return Promise.all(rows.map((r) => driver.create(o, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, writes }; +} + +/** + * `id` is `readonly` and `locked_note` is an author-declared business readonly + * field, mirroring every platform object (`sys_user_preference`'s `id` is + * `Field.text({ …, readonly: true })`). `organization_id` is the `system` + * tenancy column — the field `preserveAudit` does NOT rescue, which is what + * makes it the control for the remedy half. + */ +async function makeEngine() { + const logger = makeCapturingLogger(); + const engine = new ObjectQL({ logger }); + const { driver, writes } = makeRecordingDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ + name: 'pref', + fields: { + id: { name: 'id', type: 'text', primaryKey: true, readonly: true }, + value: { name: 'value', type: 'text' }, + locked_note: { name: 'locked_note', type: 'text', readonly: true }, + organization_id: { name: 'organization_id', type: 'text', readonly: true, system: true }, + account_number: { name: 'account_number', type: 'autonumber' }, + title: { name: 'title', type: 'text' }, + }, + } as any, 'test'); + return { engine, writes, logger }; +} + +interface Observed { + readonly refusedCode: string | null; + readonly driverWrites: number; + readonly warns: string[]; + /** Every non-debug/info line, level included. */ + readonly lines: Array<{ level: string; msg: string }>; +} + +/** + * The level of the STRIP's own line, isolated from the engine's pre-existing + * `'Update operation failed'` / `'Insert operation failed'` ERROR — which a + * strict refusal legitimately emits, because the caller really was handed a + * failure. The two lines are different facts and this suite must not conflate + * them: the strip's line stays at `warn` (its docblock argues that level), and + * the operation-level error is nobody's business here. + */ +function stripLineLevels(o: Observed): string[] { + return o.lines + .filter((l) => !/^(Update|Insert) operation failed$/.test(l.msg)) + .map((l) => l.level); +} + +async function observeUpdate(data: unknown, options: Record): Promise { + const { engine, writes, logger } = await makeEngine(); + let refusedCode: string | null = null; + try { + await engine.update('pref', data as any, options as any); + } catch (e: any) { + refusedCode = e?.code ?? e?.name ?? null; + } + return { + refusedCode, + driverWrites: writes.length, + warns: logger.lines.filter((l: any) => l.level === 'warn').map((l: any) => l.msg), + lines: logger.lines.filter((l: any) => l.level !== 'debug' && l.level !== 'info'), + }; +} + +async function observeInsert(data: unknown, options: Record = {}): Promise { + const { engine, writes, logger } = await makeEngine(); + let refusedCode: string | null = null; + try { + await engine.insert('pref', data as any, options as any); + } catch (e: any) { + refusedCode = e?.code ?? e?.name ?? null; + } + return { + refusedCode, + driverWrites: writes.filter((w) => w.fn === 'create').length, + warns: logger.lines.filter((l: any) => l.level === 'warn').map((l: any) => l.msg), + lines: logger.lines.filter((l: any) => l.level !== 'debug' && l.level !== 'info'), + }; +} + +describe('#8214 — a strict refusal is not reported as a commit (UPDATE)', () => { + it('THE REPRO, inverted: refused, nothing written, and the line no longer claims a commit', async () => { + const o = await observeUpdate( + { id: 'rec_1', value: 'v1', locked_note: 'forged' }, + { where: { id: 'rec_1' }, strictReadonlyWrites: true }, + ); + // The three facts the log has to agree with, unchanged by this card. + expect(o.refusedCode).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(o.driverWrites).toBe(0); + // …and the fourth, which is what moved. + expect(o.warns).toHaveLength(1); + expect(o.warns[0]).not.toContain('COMMITTED WITHOUT IT'); + expect(o.warns[0]).toContain('REFUSED ENTIRELY'); + expect(o.warns[0]).toContain('ERR_READONLY_FIELD_REJECTED'); + // Byte-identical to the exported composer, so the wording lives in ONE + // place and a future edit cannot drift the log away from its own pin. + expect(o.warns[0]).toBe( + readonlyStripWarning('locked_note', 'pref', { strict: true, preserveAuditApplies: true }), + ); + }); + + it('THE COUNTER-CASE: strict did not silence the forgery signal', async () => { + // Without this, "the message stopped lying" is indistinguishable from "the + // message stopped warning" — the failure the card explicitly forbids. + const o = await observeUpdate( + { id: 'rec_1', value: 'v1', locked_note: 'forged' }, + { where: { id: 'rec_1' }, strictReadonlyWrites: true }, + ); + expect(o.warns).toHaveLength(1); + expect(stripLineLevels(o)).toEqual(['warn']); // the level its docblock argues + // …and the refusal itself is reported separately, at `error`, by the + // engine — two lines, two facts, neither pretending to be the other. + expect(o.lines.some((l) => l.level === 'error' && l.msg === 'Update operation failed')).toBe(true); + expect(o.warns[0]).toContain("Field 'locked_note'"); // the field, named + expect(o.warns[0]).toContain('{ context: { isSystem: true } }'); // a remedy + }); + + it('the DEFAULT strip is untouched — "COMMITTED WITHOUT IT" is true there and stays', async () => { + const o = await observeUpdate( + { id: 'rec_1', value: 'v1', locked_note: 'forged' }, + { where: { id: 'rec_1' } }, + ); + expect(o.refusedCode).toBeNull(); + expect(o.driverWrites).toBe(1); + expect(o.warns).toEqual([ + readonlyStripWarning('locked_note', 'pref', { preserveAuditApplies: true }), + ]); + expect(o.warns[0]).toContain('COMMITTED WITHOUT IT'); + }); + + it('the MULTI branch reports strict the same way', async () => { + // The multi branch runs its own strip call and its own `assertNoStrictDrops` + // — a fix threaded through only the by-id call site would leave this one + // still claiming a commit, so it is pinned separately rather than assumed. + const o = await observeUpdate( + { locked_note: 'forged', value: 'v1' }, + { multi: true, where: { title: 't0' }, strictReadonlyWrites: true }, + ); + expect(o.refusedCode).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(o.driverWrites).toBe(0); + expect(o.warns).toEqual([ + readonlyStripWarning('locked_note', 'pref', { strict: true, preserveAuditApplies: true }), + ]); + }); +}); + +describe('#8214 — the INSERT side carries the identical defect (the card marked it UNVERIFIED)', () => { + it('REPRODUCED then FIXED: refused, zero creates, and no claim of a commit', async () => { + const o = await observeInsert({ title: 't', account_number: 'ACC-888888' }, { strictReadonlyWrites: true }); + expect(o.refusedCode).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(o.driverWrites).toBe(0); + expect(o.warns).toHaveLength(1); + expect(o.warns[0]).not.toContain('COMMITTED WITHOUT IT'); + expect(o.warns[0]).toBe( + runtimeOwnedStripWarning('account_number', 'autonumber', 'pref', { + strict: true, preserveAuditApplies: true, + }), + ); + expect(stripLineLevels(o)).toEqual(['warn']); + expect(o.lines.some((l) => l.level === 'error' && l.msg === 'Insert operation failed')).toBe(true); + }); + + it('the DEFAULT insert strip still says COMMITTED WITHOUT IT — and the row really does commit', async () => { + // The control that makes the strict case meaningful: here the claim is TRUE + // (the record lands, holding the sequence value, not the caller's), so the + // wording must not have moved. + const { engine, writes, logger } = await makeEngine(); + const row: any = await engine.insert('pref', { title: 't', account_number: 'ACC-888888' } as any); + const warns = logger.lines.filter((l: any) => l.level === 'warn').map((l: any) => l.msg); + expect(writes.filter((w) => w.fn === 'create')).toHaveLength(1); + expect(row.account_number).toBe('0001'); + expect(warns).toEqual([ + runtimeOwnedStripWarning('account_number', 'autonumber', 'pref', { preserveAuditApplies: true }), + ]); + expect(warns[0]).toContain('COMMITTED WITHOUT IT'); + }); +}); + +describe('#8214 — #8141 stays closed, in strict mode too', () => { + it('a by-id ADDRESS write logs nothing under strictReadonlyWrites either', async () => { + // The card's hard fence: if the addressing `id` starts printing again for a + // by-id address write, the change went wrong. `addressKey` is consulted + // before the message is composed, so the mode cannot reach it — pinned + // rather than argued, and pinned in BOTH modes. + const o = await observeUpdate( + { value: 'v1', id: 'rec_1' }, + { where: { id: 'rec_1' }, strictReadonlyWrites: true }, + ); + expect(o.warns).toEqual([]); + // …and it is not refused either: the address is excluded from the report + // channel by the same predicate, so both channels still agree (#8141). + expect(o.refusedCode).toBeNull(); + expect(o.driverWrites).toBe(1); + }); + + it('a forgery riding along with the address still logs, and still refuses', async () => { + const o = await observeUpdate( + { value: 'v1', id: 'rec_1', locked_note: 'forged' }, + { where: { id: 'rec_1' }, strictReadonlyWrites: true }, + ); + expect(o.refusedCode).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(o.warns).toHaveLength(1); + expect(o.warns[0]).not.toContain("Field 'id'"); + expect(o.warns[0]).toContain("Field 'locked_note'"); + }); +}); + +describe('#8214 — the preserveAudit remedy is offered per FIELD, never as a blanket', () => { + it('offers it for a field it would really have kept (locked_note)', async () => { + const o = await observeUpdate( + { id: 'rec_1', value: 'v1', locked_note: 'forged' }, + { where: { id: 'rec_1' } }, + ); + expect(o.warns[0]).toContain('preserveAudit'); + expect(o.warns[0]).toContain('{ context: { preserveAudit: true } }'); + }); + + it('WITHHOLDS it for a field it would NOT have kept (organization_id — tenancy)', async () => { + // The case that makes "targeted" a correctness property rather than a + // preference. `organization_id` is `system` and outside the audit family, + // so `preserveAudit` strips it anyway (no tenancy-forging backdoor, #3493) + // — advertising the flag here would repeat #8141's defect one exemption + // over: a caller steered to a posture that does not help. + const o = await observeUpdate( + { id: 'rec_1', value: 'v1', organization_id: 'org_forged' }, + { where: { id: 'rec_1' } }, + ); + expect(o.warns).toEqual([readonlyStripWarning('organization_id', 'pref')]); + expect(o.warns[0]).not.toContain('preserveAudit'); + // …while `isSystem`, which WOULD have kept it, is still offered. + expect(o.warns[0]).toContain('{ context: { isSystem: true } }'); + }); + + it('the remedy the line offers really works — following it makes the line stop', async () => { + // The strongest form of "true for that case": take the advice and measure. + // `locked_note` survives the strip under `preserveAudit`, so the warning it + // was named in disappears and the value reaches the driver. + const o = await observeUpdate( + { id: 'rec_1', value: 'v1', locked_note: 'restored' }, + { where: { id: 'rec_1' }, context: { preserveAudit: true } }, + ); + expect(o.warns).toEqual([]); + expect(o.driverWrites).toBe(1); + }); +}); diff --git a/packages/objectql/src/engine-update-addressing-id-no-warn.test.ts b/packages/objectql/src/engine-update-addressing-id-no-warn.test.ts index 7e72ee7972..73920223dc 100644 --- a/packages/objectql/src/engine-update-addressing-id-no-warn.test.ts +++ b/packages/objectql/src/engine-update-addressing-id-no-warn.test.ts @@ -206,7 +206,7 @@ describe('#8141 — the addressed row\'s primary key is not logged as a forged c { value: 'v1', locked_note: 'forged', id: 'rec_1' }, { where: { id: 'rec_1' } }, ); - expect(warns).toEqual([readonlyStripWarning('locked_note', 'pref')]); + expect(warns).toEqual([readonlyStripWarning('locked_note', 'pref', { preserveAuditApplies: true })]); expect(lines.filter((l: any) => l.level !== 'debug' && l.level !== 'info').map((l: any) => l.level)) .toEqual(['warn']); // The line still carries the consequence and both remedies (#4903), and @@ -233,7 +233,7 @@ describe('#8141 — the addressed row\'s primary key is not logged as a forged c ); expect(warns).toHaveLength(2); expect(warns.some((w: string) => w.includes("dropped 'id' from the write payload"))).toBe(true); - expect(warns).toContain(readonlyStripWarning('locked_note', 'pref')); + expect(warns).toContain(readonlyStripWarning('locked_note', 'pref', { preserveAuditApplies: true })); expect(events).toEqual([ { object: 'pref', fields: ['id'], reason: 'primary_key' }, { object: 'pref', fields: ['locked_note'], reason: 'readonly' }, @@ -249,7 +249,7 @@ describe('#8141 — the addressed row\'s primary key is not logged as a forged c { locked_note: 'forged', value: 'v1' }, { multi: true, where: { title: 't0' } }, ); - expect(warns).toEqual([readonlyStripWarning('locked_note', 'pref')]); + expect(warns).toEqual([readonlyStripWarning('locked_note', 'pref', { preserveAuditApplies: true })]); expect(writes.map((w) => w.fn)).toEqual(['updateMany']); expect(writes[0].data).toEqual({ value: 'v1' }); }); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 58b1f34f5d..a74eaceac6 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -7723,8 +7723,15 @@ export class ObjectQL implements IObjectQLEngine { const preserveAudit = opCtx.context?.preserveAudit === true; for (let i = 0; i < rows.length; i++) { if (rowErrors[i] !== undefined) continue; + // [#8214] The insert side carries the same claim and the same + // sequencing — this pass logs, the `ReadonlyFieldRejectedError` + // below throws before any driver dispatch. Measured on + // `origin/main`: `driverCreates 0` while the line said the write + // was "COMMITTED WITHOUT IT". The card marked this half UNVERIFIED; + // it reproduces, so the flag is threaded here too. const stripped = stripRuntimeOwnedFields( - schemaForValidation as any, rows[i], suppliedPerRow[i] ?? {}, this.logger, { preserveAudit }, + schemaForValidation as any, rows[i], suppliedPerRow[i] ?? {}, this.logger, + { preserveAudit, strictReadonlyWrites: options?.strictReadonlyWrites === true }, ) as Record; if (stripped === rows[i]) continue; for (const k of Object.keys(rows[i])) { @@ -8709,7 +8716,15 @@ export class ObjectQL implements IObjectQLEngine { // insert-side sibling), which is what keeps those byte-identical. if (!opCtx.context?.isSystem) { const preRo = hookContext.input.data as Record; - hookContext.input.data = stripReadonlyFields(updateSchema as any, preRo, suppliedValues, this.logger, { preserveAudit: opCtx.context?.preserveAudit === true, addressKey: idAddressesThisRow ? 'id' : undefined }) as any; + // [#8214] `strictReadonlyWrites` is threaded INTO the strip + // rather than consulted only at `assertNoStrictDrops()` + // below: the strip logs from inside, the refusal happens + // afterwards, and until the strip knew the flag its line + // told a refused caller the update had been "COMMITTED + // WITHOUT IT" while `driverWrites` was 0. The seam that + // composes the sentence has to know the mode the sentence + // describes; nothing else here can tell it. + hookContext.input.data = stripReadonlyFields(updateSchema as any, preRo, suppliedValues, this.logger, { preserveAudit: opCtx.context?.preserveAudit === true, addressKey: idAddressesThisRow ? 'id' : undefined, strictReadonlyWrites }) as any; reportDroppedFields(preRo, hookContext.input.data as Record, 'readonly'); } // [#5126] Both strip passes are done; refuse now if the caller @@ -8856,7 +8871,11 @@ export class ObjectQL implements IObjectQLEngine { // rejected upstream by the tenant write wall, #2946). if (!opCtx.context?.isSystem) { const preRoMulti = hookContext.input.data as Record; - hookContext.input.data = stripReadonlyFields(updateSchema as any, preRoMulti, suppliedValues, this.logger, { preserveAudit: opCtx.context?.preserveAudit === true }) as any; + // [#8214] Same threading as the by-id branch; the multi + // branch still passes no `addressKey` (nothing addresses a + // row by key here), which is what keeps it byte-identical + // to #8141 in every other respect. + hookContext.input.data = stripReadonlyFields(updateSchema as any, preRoMulti, suppliedValues, this.logger, { preserveAudit: opCtx.context?.preserveAudit === true, strictReadonlyWrites }) as any; reportDroppedFields(preRoMulti, hookContext.input.data as Record, 'readonly'); } // [#5126] Same refusal on the predicate path. A bulk strip is diff --git a/packages/objectql/src/validation/rule-validator.test.ts b/packages/objectql/src/validation/rule-validator.test.ts index 5bef6e471e..85be5e43c7 100644 --- a/packages/objectql/src/validation/rule-validator.test.ts +++ b/packages/objectql/src/validation/rule-validator.test.ts @@ -890,7 +890,13 @@ describe('stripReadonlyFields — implicit readonly on autonumber (#5503)', () = { warn: (m: string) => warns.push(m) } as any, ); expect(warns).toHaveLength(2); - expect(warns.some((m) => m === runtimeOwnedStripWarning('account_number', 'autonumber', 'an_account'))).toBe(true); + // [#8214] `preserveAuditApplies` is now stated by the pin rather than + // assumed: `account_number` is an `autonumber` with no `system: true`, so + // {@link isPreservableUnderAudit} keeps it and the remedy really is on + // offer. The clause the old text printed unconditionally is unchanged FOR + // THIS FIELD — what moved is that the strip now proves it applies before + // saying so. + expect(warns.some((m) => m === runtimeOwnedStripWarning('account_number', 'autonumber', 'an_account', { preserveAuditApplies: true }))).toBe(true); // The message must say WHY (runtime-issued) and name BOTH exempt writer // paths — an author who never wrote `readonly: true` gets no help from a // bare "this field is read-only". @@ -927,7 +933,7 @@ function stripWithWarns( schema: unknown, data: Record, supplied: Record, - options?: { preserveAudit?: boolean; addressKey?: string }, + options?: { preserveAudit?: boolean; addressKey?: string; strictReadonlyWrites?: boolean }, ) { const warns: string[] = []; const levels: string[] = []; @@ -959,7 +965,7 @@ describe('stripReadonlyFields — addressKey silences the LOG, never the strip ( const supplied = { id: 'rec_1', value: 'v1' }; const { out, warns, levels } = stripWithWarns(addressedFields, { ...supplied }, supplied); expect(out).toEqual({ value: 'v1' }); - expect(warns).toEqual([readonlyStripWarning('id', 'pref')]); + expect(warns).toEqual([readonlyStripWarning('id', 'pref', { preserveAuditApplies: true })]); expect(levels).toEqual(['warn']); }); @@ -972,7 +978,7 @@ describe('stripReadonlyFields — addressKey silences the LOG, never the strip ( addressedFields, { ...supplied }, supplied, { addressKey: 'id' }, ); expect(out).toEqual({ value: 'v1' }); - expect(warns).toEqual([readonlyStripWarning('locked_note', 'pref')]); + expect(warns).toEqual([readonlyStripWarning('locked_note', 'pref', { preserveAuditApplies: true })]); expect(levels).toEqual(['warn']); expect(warns[0]).toContain('COMMITTED WITHOUT IT'); expect(warns[0]).toContain('{ context: { isSystem: true } }'); @@ -994,7 +1000,7 @@ describe('stripReadonlyFields — addressKey silences the LOG, never the strip ( addressedFields, { ...supplied }, supplied, { addressKey: 'value' }, ); expect(out).toEqual({ value: 'v1' }); - expect(warns).toEqual([readonlyStripWarning('locked_note', 'pref')]); + expect(warns).toEqual([readonlyStripWarning('locked_note', 'pref', { preserveAuditApplies: true })]); }); it('composes with preserveAudit rather than overriding it', () => { @@ -1014,7 +1020,13 @@ describe('stripReadonlyFields — addressKey silences the LOG, never the strip ( // reinstated by the same whitelist; `organization_id` is a non-audit system // column, so it is still stripped and still loud. expect(out).toEqual({ id: 'rec_1', created_at: '2020-01-01T00:00:00Z' }); + // [#8214] NO `preserveAuditApplies` here, and that is the load-bearing + // half: `organization_id` is `system` and outside the audit family, so + // `preserveAudit` does NOT rescue it — this very call had the flag ON and + // stripped it anyway. A blanket "use preserveAudit" sentence would be a + // lie exactly here, which is why the remedy is derived per field. expect(warns).toEqual([readonlyStripWarning('organization_id', 'pref')]); + expect(warns[0]).not.toContain('preserveAudit'); }); it('silences the RUNTIME-OWNED message for the same key, on the same ground', () => { @@ -1033,7 +1045,137 @@ describe('stripReadonlyFields — addressKey silences the LOG, never the strip ( const supplied = { id: 'ACC-1', account_number: 'ACC-888888', value: 'v1' }; const { out, warns } = stripWithWarns(numberedId, { ...supplied }, supplied, { addressKey: 'id' }); expect(out).toEqual({ value: 'v1' }); - expect(warns).toEqual([runtimeOwnedStripWarning('account_number', 'autonumber', 'pref')]); + expect(warns).toEqual([runtimeOwnedStripWarning('account_number', 'autonumber', 'pref', { preserveAuditApplies: true })]); + }); +}); + +// #8214 — the two claims the strip messages were making that were not true of +// the call in front of them. The engine-level measurement lives in +// `engine-strict-readonly-warning-truthful.test.ts`; this block pins the +// composer and the strip's derivation of what it may say. +describe('#8214 — strictReadonlyWrites: the line reports a REFUSAL, not a commit', () => { + it('the strip threads the flag into the message it emits', () => { + const supplied = { id: 'rec_1', value: 'v1', locked_note: 'forged' }; + const { warns, levels } = stripWithWarns( + addressedFields, { ...supplied }, supplied, + { addressKey: 'id', strictReadonlyWrites: true }, + ); + expect(warns).toEqual([ + readonlyStripWarning('locked_note', 'pref', { strict: true, preserveAuditApplies: true }), + ]); + // ⛔ Not silenced, and not demoted: the forgery signal is the whole reason + // this line exists at `warn`, and a refused forged write is the case that + // most deserves it. + expect(levels).toEqual(['warn']); + expect(warns[0]).toContain("Field 'locked_note'"); + }); + + it('and the ADDRESS is still silent under strict — #8141 is not re-opened', () => { + const supplied = { id: 'rec_1', value: 'v1' }; + const { out, warns } = stripWithWarns( + addressedFields, { ...supplied }, supplied, + { addressKey: 'id', strictReadonlyWrites: true }, + ); + expect(warns).toEqual([]); + expect(out).toEqual({ value: 'v1' }); // …and still STRIPPED, as ever + }); + + it('omitting the flag is identical to passing it false — the default is the old text', () => { + expect(readonlyStripWarning('f', 'o')).toBe(readonlyStripWarning('f', 'o', { strict: false })); + expect(runtimeOwnedStripWarning('f', 'autonumber', 'o')) + .toBe(runtimeOwnedStripWarning('f', 'autonumber', 'o', { strict: false })); + }); + + it('the strict text swaps the CONSEQUENCE and the observe-instead remedy, nothing else', () => { + const plain = readonlyStripWarning('f', 'o'); + const strict = readonlyStripWarning('f', 'o', { strict: true }); + expect(plain).toContain('COMMITTED WITHOUT IT'); + expect(strict).not.toContain('COMMITTED WITHOUT IT'); + expect(strict).toContain('REFUSED ENTIRELY'); + expect(strict).toContain('ERR_READONLY_FIELD_REJECTED'); + // Under strict, "pass onFieldsDropped" alone is not the remedy — dropping + // strict is. Same direction the refusal error's own message points. + expect(strict).toContain('drop options.strictReadonlyWrites'); + // Everything the #4903 contract requires is still in both. + for (const m of [plain, strict]) { + expect(m).toContain("Field 'f' on 'o' is read-only"); + expect(m).toContain('{ context: { isSystem: true } }'); + expect(m).toContain('onFieldsDropped'); + } + }); + + it('the runtime-owned twin does the same, keeping its own WHY', () => { + const strict = runtimeOwnedStripWarning('code', 'autonumber', 'o', { strict: true }); + expect(strict).not.toContain('COMMITTED WITHOUT IT'); + expect(strict).toContain('REFUSED ENTIRELY'); + // The clause that makes it distinct from the readonly message must survive: + // the author never wrote `readonly: true` here, so the line has to say who + // owns the value. + expect(strict).toContain('the runtime issues this value from its sequence'); + }); +}); + +// #8214 (2) — the remedy set. `stripReadonlyFields` honours `preserveAudit` +// (#3493), a WHITELIST narrower than `isSystem` by construction, while the +// message offered only the blanket exemption — steering an import that forgot +// the flag to the strictly worse posture, the identical failure #8141 fixed one +// remedy over. It is now offered per field, DERIVED from the same predicate the +// strip consults, so it can never advertise an exemption that would not have +// worked — and cannot drift when #8215 narrows that predicate. +describe('#8214 — the preserveAudit remedy is derived per field, not described', () => { + it('offers it for a field the whitelist would have kept', () => { + const supplied = { closed_at: '2021-03-01T00:00:00Z' }; + const { warns } = stripWithWarns(historicalFields, { ...supplied }, supplied); + expect(warns).toEqual([ + readonlyStripWarning('closed_at', undefined, { preserveAuditApplies: true }), + ]); + expect(warns[0]).toContain('{ context: { preserveAudit: true } }'); + }); + + it('WITHHOLDS it for a non-audit system column the whitelist strips anyway', () => { + const supplied = { organization_id: 'org_forged' }; + const { warns } = stripWithWarns(historicalFields, { ...supplied }, supplied); + expect(warns).toEqual([readonlyStripWarning('organization_id')]); + expect(warns[0]).not.toContain('preserveAudit'); + expect(warns[0]).toContain('{ context: { isSystem: true } }'); + }); + + it('the audit family itself is offered it — that is the case #3493 exists for', () => { + const supplied = { created_at: '2020-01-01T00:00:00Z' }; + const { warns } = stripWithWarns(historicalFields, { ...supplied }, supplied); + expect(warns[0]).toContain('{ context: { preserveAudit: true } }'); + }); + + it('a line that PRINTS with preserveAudit already on never offers it', () => { + // The derivation's safety property, stated as a test rather than trusted: + // a field kept by `preserveAudit` never reaches the log at all, so any + // field that DOES reach it while the flag is on is one the whitelist + // refused — and the remedy is correctly withheld. + const supplied = { organization_id: 'org_forged', closed_at: '2021-03-01T00:00:00Z' }; + const { out, warns } = stripWithWarns( + historicalFields, { ...supplied }, supplied, { preserveAudit: true }, + ); + expect(out).toEqual({ closed_at: '2021-03-01T00:00:00Z' }); + expect(warns).toEqual([readonlyStripWarning('organization_id')]); + expect(warns[0]).not.toContain('preserveAudit'); + }); + + it('the two sibling messages now agree — one predicate, two texts', () => { + // The disagreement the card was filed about: the runtime-owned twin named + // both exemptions unconditionally while the readonly one named neither. + // Both are now gated on the same fact, so a field that is preservable gets + // the sentence from either message and a field that is not gets it from + // neither. + for (const applies of [true, false]) { + const ro = readonlyStripWarning('f', 'o', { preserveAuditApplies: applies }); + const rt = runtimeOwnedStripWarning('f', 'autonumber', 'o', { preserveAuditApplies: applies }); + expect(ro.includes('preserveAudit')).toBe(applies); + expect(rt.includes('preserveAudit')).toBe(applies); + // `isSystem` is unconditional in both — it exempts the whole strip, so it + // is true for every field either message can name. + expect(ro).toContain('isSystem'); + expect(rt).toContain('isSystem'); + } }); }); diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index c2d3d49b8a..c51b4e08fe 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -958,6 +958,14 @@ export function isRuntimeOwnedField(def: { type?: string } | undefined | null): * level. Changing this means giving `ExecutionContext` a real origin marker * first — not guessing from the absence of a principal. * + * [#8214] The level did not move; two of the message's CLAIMS did. Under + * `options.strictReadonlyWrites` the write is refused outright, so the line no + * longer says the update was committed without the field, and the + * `preserveAudit` remedy is now offered per-field, derived from + * {@link isPreservableUnderAudit} rather than described in prose. Both are + * argued on {@link StripWarningOptions}; both keep the line naming the field, + * the consequence, and a remedy that is TRUE for the case that printed. + * * ### ...and the one key it does NOT log: the write's ADDRESS (#8141) * * `options.addressKey` names the key that carries the ADDRESS of the row this @@ -1006,12 +1014,13 @@ export function stripReadonlyFields( data: Record | undefined | null, supplied: Readonly>, logger?: EvaluateRulesOptions['logger'], - options?: { preserveAudit?: boolean; addressKey?: string }, + options?: { preserveAudit?: boolean; addressKey?: string; strictReadonlyWrites?: boolean }, ): Record | undefined | null { const fields = objectSchema?.fields; if (!fields || !data) return data; const preserveAudit = options?.preserveAudit === true; const addressKey = options?.addressKey; + const strict = options?.strictReadonlyWrites === true; let result = data; for (const [name, def] of Object.entries(fields)) { // [#5503] `readonly: true` is the AUTHOR-declared lock; a runtime-owned @@ -1043,10 +1052,19 @@ export function stripReadonlyFields( // rejection on stores with immutable primary keys, #6435), and only the // caller can know a key is an address — this helper never guesses one. if (addressKey !== undefined && name === addressKey) continue; + // [#8214] Both facts the message is allowed to state, computed HERE from + // what this pass actually knows. `preserveAuditApplies` is the same + // predicate the `continue` above consults — and reaching this line proves + // the flag was OFF (a preservable field under `preserveAudit` never gets + // here), so offering it is a remedy that would really have worked. + const warnOptions: StripWarningOptions = { + strict, + preserveAuditApplies: isPreservableUnderAudit(name, def), + }; logger?.warn?.( def?.readonly - ? readonlyStripWarning(name, objectSchema?.name) - : runtimeOwnedStripWarning(name, String(def?.type), objectSchema?.name), + ? readonlyStripWarning(name, objectSchema?.name, warnOptions) + : runtimeOwnedStripWarning(name, String(def?.type), objectSchema?.name, warnOptions), ); } return result; @@ -1118,11 +1136,12 @@ export function stripRuntimeOwnedFields( data: Record | undefined | null, supplied: Readonly>, logger?: EvaluateRulesOptions['logger'], - options?: { preserveAudit?: boolean }, + options?: { preserveAudit?: boolean; strictReadonlyWrites?: boolean }, ): Record | undefined | null { const fields = objectSchema?.fields; if (!fields || !data) return data; const preserveAudit = options?.preserveAudit === true; + const strict = options?.strictReadonlyWrites === true; let result = data; for (const [name, def] of Object.entries(fields)) { if (!isRuntimeOwnedField(def)) continue; @@ -1142,11 +1161,111 @@ export function stripRuntimeOwnedFields( if (preserveAudit && isPreservableUnderAudit(name, def)) continue; // historical import reinstates it if (result === data) result = { ...data }; delete (result as Record)[name]; - logger?.warn?.(runtimeOwnedStripWarning(name, String(def?.type), objectSchema?.name)); + // [#8214] The insert side carried the same false "COMMITTED WITHOUT IT" + // claim under `strictReadonlyWrites`, and the card marked that UNVERIFIED. + // Measured on `origin/main`, real `ObjectQL` + a recording driver, one + // `autonumber` seeded by the caller: `refusedCode + // ERR_READONLY_FIELD_REJECTED`, `driverCreates 0`, `warnLines 1`, + // `claimsCommitted true`. It reproduces — `engine.insert` throws after this + // pass and before any driver dispatch, exactly as `update` does. + logger?.warn?.( + runtimeOwnedStripWarning(name, String(def?.type), objectSchema?.name, { + strict, + preserveAuditApplies: isPreservableUnderAudit(name, def), + }), + ); } return result; } +/** + * What the strip knows about the write at the moment it composes a line (#8214). + * + * Both members exist for the same reason and follow the same rule: **a strip + * message may state only what is true of the call in front of it.** Neither is + * a wording preference — each replaces a sentence that was measurably false for + * a real caller. + * + * Both default to `false`, and the default is the CONSERVATIVE reading, not the + * common one: an unset flag never claims a commit that may not happen and never + * advertises a remedy that may not work. A caller that knows better says so. + * #8141 is the standing reason — its defect was a line offering `isSystem` to a + * caller for whom that exemption was the strictly worse posture, and a message + * that guesses optimistically reproduces it. + */ +export interface StripWarningOptions { + /** + * The write passed `options.strictReadonlyWrites` (#5126), so the drop this + * line reports will REFUSE the whole write instead of shrinking it. + * + * Measured on `origin/main` before this option existed (real `ObjectQL` + a + * recording driver, by-id update of an object with a `readonly` field, + * `{ strictReadonlyWrites: true }`): `refusedCode ERR_READONLY_FIELD_REJECTED`, + * `driverWrites 0`, `warnLines 1`, and the one line said the update was + * "COMMITTED WITHOUT IT". Nothing was written and the log promised a partial + * commit — the #4632 shape inverted, sending a reader to hunt for a row that + * was never touched. The INSERT side reproduced identically + * (`runtimeOwnedStripWarning`, `driverCreates 0`), which is why this option + * is shared by both messages rather than being an update-path patch. + * + * ⚠️ The strip is NOT the seam that decides the refusal, so this flag is the + * caller's assertion, not a derivation: `assertNoStrictDrops()` in `engine.ts` + * throws afterwards. It is safe to state as fact here because the two are + * driven off the SAME drop — a field this function is called for is a field + * `reportDroppedFields` records and `assertNoStrictDrops` then refuses on, + * including the `addressKey` case, which is excluded from both channels by + * one predicate (#8141). Adding a drop path that skips one channel and not + * the other would falsify this line; keep the two fed from one fact. + */ + readonly strict?: boolean; + /** + * `{ context: { preserveAudit: true } }` (#3493) would have KEPT this exact + * field — so naming it is a remedy the reader can act on. + * + * DERIVED from {@link isPreservableUnderAudit} at the call site, never from a + * hand-written description of the whitelist. That is the whole point: the + * whitelist is `AUDIT_TIMELINE_FIELDS` plus any non-`system` field, and its + * second limb is under active revision (#8215 — it currently sweeps in a + * platform object's own primary key). A sentence DESCRIBING the whitelist + * becomes false the day that lands; a sentence gated on the predicate narrows + * with it, automatically and with no second declaration to forget. + * + * It is also why the remedy is targeted rather than blanket. `preserveAudit` + * does not help every stripped `readonly` field: a non-audit `system` column + * (`organization_id` — tenancy) is stripped under `preserveAudit` too, and + * measured, today's blanket-free wording is right about that one by accident. + * Offering the flag there would repeat #8141's defect one exemption over. + */ + readonly preserveAuditApplies?: boolean; +} + +/** + * The `preserveAudit` remedy sentence, emitted only when the flag would really + * have kept the field this line names. See {@link StripWarningOptions}. + */ +function preserveAuditRemedySentence(options?: StripWarningOptions): string { + if (options?.preserveAuditApplies !== true) return ''; + return ( + ` A historical import restoring this record's own earlier values does NOT need that blanket ` + + `exemption: pass the narrower historical-import context { context: { preserveAudit: true } } ` + + `(#3493), which reinstates THIS field while the rest of the strip stays in force.` + ); +} + +/** + * The "observe instead of reading this log" sentence. Under strict the write is + * refused, so "detect drops programmatically" is not the remedy on offer — + * dropping strict is. Same sentence the refusal error itself ends on, so the + * log line and `ReadonlyFieldRejectedError` point the same way. + */ +function observeInsteadSentence(options?: StripWarningOptions): string { + return options?.strict === true + ? ` To let the strip happen and merely observe it instead of refusing the write, drop ` + + `options.strictReadonlyWrites and pass options.onFieldsDropped (#3407).` + : ` To detect drops programmatically instead of reading ` + + `this log, pass options.onFieldsDropped (#3407).`; +} + /** * The message the runtime-owned strip logs per dropped field (#5503). Exported * so the pin test asserts the CONTRACT of this text rather than its wording. @@ -1158,19 +1277,47 @@ export function stripRuntimeOwnedFields( * writer paths still may set it. Same `warn` level, for the same reason spelled * out on {@link readonlyStripWarning}: this seam cannot tell a hostile forged * body from a trusted server-side writer that simply forgot to declare itself. + * + * [#8214] Its `preserveAudit` clause is now gated on + * {@link StripWarningOptions.preserveAuditApplies} rather than printed + * unconditionally. The clause was RIGHT for every field measured — a declared + * `autonumber` is not `system`, so the whitelist keeps it — and it stays + * printed for those. What changed is that the two sibling messages now derive + * their remedy set from ONE predicate instead of each carrying its own opinion, + * which is the disagreement #8214 was filed about. */ -export function runtimeOwnedStripWarning(field: string, type: string, object?: string): string { +export function runtimeOwnedStripWarning( + field: string, + type: string, + object?: string, + options?: StripWarningOptions, +): string { const on = object ? ` on '${object}'` : ''; + const audit = options?.preserveAuditApplies === true; + const consequence = + options?.strict === true + ? `DROPPED and the write is being REFUSED ENTIRELY — the runtime issues this value from its ` + + `sequence, and this write passed options.strictReadonlyWrites, so NOTHING is written: not ` + + `this column, and not the fields that would have survived the strip. The call throws ` + + `ERR_READONLY_FIELD_REJECTED rather than returning success (#5126).` + : `DROPPED and the write is being COMMITTED WITHOUT IT — the runtime issues this value from its ` + + `sequence, so the call returns success while the column holds the generated number, not the one ` + + `sent (#5503).`; return ( `Field '${field}'${on} is a runtime-owned '${type}' field: the caller-supplied value was ` + - `DROPPED and the write is being COMMITTED WITHOUT IT — the runtime issues this value from its ` + - `sequence, so the call returns success while the column holds the generated number, not the one ` + - `sent (#5503). Server-side code that legitimately sets it (seed replay, a migration) must ` + - `declare itself trusted by passing { context: { isSystem: true } }; a data import reinstating ` + - `legacy record numbers uses the historical-import context ({ context: { preserveAudit: true } }, ` + - `#3493). A beforeInsert/beforeUpdate hook does NOT need either — hook-written keys are not ` + - `caller-supplied. To detect drops programmatically instead of reading this log, pass ` + - `options.onFieldsDropped (#3407). Forged record numbers from untrusted client input are ` + + consequence + + ` Server-side code that legitimately sets it (seed replay, a migration) must ` + + `declare itself trusted by passing { context: { isSystem: true } }` + + (audit + ? `; a data import reinstating ` + + `legacy record numbers uses the historical-import context ({ context: { preserveAudit: true } }, ` + + `#3493), which reinstates THIS field while the rest of the strip stays in force. ` + + `A beforeInsert/beforeUpdate hook does NOT need either — hook-written keys are not ` + + `caller-supplied.` + : `. A beforeInsert/beforeUpdate hook does NOT need it — hook-written keys are not ` + + `caller-supplied.`) + + observeInsteadSentence(options) + + ` Forged record numbers from untrusted client input are ` + `expected here and need no action.` ); } @@ -1179,17 +1326,55 @@ export function runtimeOwnedStripWarning(field: string, type: string, object?: s * The message {@link stripReadonlyFields} logs per dropped field (#4903). * Exported so the pin test asserts the CONTRACT of this text — consequence, * `isSystem` remedy, `onFieldsDropped` remedy — rather than its wording. + * + * ### [#8214] Two claims that were false, and are now conditioned on the fact + * + * 1. **The commit.** "COMMITTED WITHOUT IT" is the DEFAULT strip's consequence + * and stays byte-identical there. Under `strictReadonlyWrites` it was a lie: + * nothing is written at all. See {@link StripWarningOptions.strict} for the + * measurement. The line is NOT dropped in strict mode — the level argument + * below is that `warn` exists so real forgery attempts stay visible, and a + * forged write that got REFUSED is the case that most deserves a line. Only + * the claim of a commit goes. + * 2. **The remedy.** The message named `{ context: { isSystem: true } }`, the + * BLANKET exemption from the whole strip, and never `preserveAudit` (#3493), + * the narrower whitelist this strip actually honours three lines up. An + * import that forgot the flag read this line and was steered to the strictly + * worse posture — the identical failure #8141 fixed one remedy over. It is + * offered per-field rather than always, and derived rather than described; + * {@link StripWarningOptions.preserveAuditApplies} argues both. + * + * What did NOT change, deliberately: the level (`warn`), and the requirement + * that every surviving line name the FIELD, the CONSEQUENCE and a REMEDY THAT + * IS TRUE FOR THAT CASE. And the write's own address still logs nothing at all + * (#8141) — in strict mode as well, since `addressKey` is consulted before this + * message is composed in either mode. */ -export function readonlyStripWarning(field: string, object?: string): string { +export function readonlyStripWarning( + field: string, + object?: string, + options?: StripWarningOptions, +): string { const on = object ? ` on '${object}'` : ''; + const consequence = + options?.strict === true + ? `the caller-supplied value was DROPPED and the update is being REFUSED ENTIRELY — this ` + + `write passed options.strictReadonlyWrites, so NOTHING is written: not this column, and ` + + `not the fields that would have survived the strip. The call throws ` + + `ERR_READONLY_FIELD_REJECTED rather than returning success (#5126).` + : `the caller-supplied value was DROPPED and the update ` + + `is being COMMITTED WITHOUT IT — the call returns success while this column keeps its stored ` + + `value (#2948).`; return ( - `Field '${field}'${on} is read-only: the caller-supplied value was DROPPED and the update ` + - `is being COMMITTED WITHOUT IT — the call returns success while this column keeps its stored ` + - `value (#2948). Server-side code that legitimately writes read-only columns (a plugin, a cron / ` + + `Field '${field}'${on} is read-only: ` + + consequence + + ` Server-side code that legitimately writes read-only columns (a plugin, a cron / ` + `background job persisting a system-computed value) must declare itself trusted by passing ` + `{ context: { isSystem: true } } on the write; a beforeUpdate hook does NOT need this because ` + - `hook-written keys are not caller-supplied. To detect drops programmatically instead of reading ` + - `this log, pass options.onFieldsDropped (#3407). Forged read-only keys from untrusted client ` + + `hook-written keys are not caller-supplied.` + + preserveAuditRemedySentence(options) + + observeInsteadSentence(options) + + ` Forged read-only keys from untrusted client ` + `input are expected here and need no action.` ); }