diff --git a/.changeset/audit-binder-created-at-unconditional-on-create.md b/.changeset/audit-binder-created-at-unconditional-on-create.md new file mode 100644 index 0000000000..88376bef26 --- /dev/null +++ b/.changeset/audit-binder-created-at-unconditional-on-create.md @@ -0,0 +1,21 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): the audit binder stamps `created_at` from the system clock on an ordinary create, so a caller-supplied value no longer survives a plain `POST` (#15964) + +The `beforeInsert` audit stamp was `record.created_at = record.created_at ?? now` — client-preferred on every insert, with no flag and no privilege required. Since the static-`readonly` strip moved INSIDE `engine.insert` (2026-09-03 ruling, option C) it runs AFTER the before-phase hooks, and its guard treats a key a `beforeInsert` hook ASSIGNED as the hook's write rather than a caller forgery. The `??` therefore laundered the caller's bytes past that strip: a normal authenticated `POST /api/v1/data/OBJECT` carrying `created_at: '1999-01-01T00:00:00.000Z'` stored exactly that on an object declaring `created_at` as `readonly: true`, while `id`, `updated_at` and every other author-declared readonly datetime in the same payload were taken. `created_at` is the audit anchor, so a forgeable one makes after-the-fact attribution untrustworthy. + +The stamp now takes the same shape as `updated_at`: + +```ts +record.created_at = preserveAudit ? (record.created_at ?? now) : now; +``` + +**What changes for a caller.** An ordinary create no longer preserves a supplied `created_at` — the value is overwritten with the server instant rather than deleted, so the column is still a real stamp and no `defaultValue` re-derivation is involved. This narrows the accept set to the `readonly` contract the field already documents; no exported symbol, schema or config key moves. + +**The historical-import channel is unchanged and pinned.** `runImport({ treatAsHistorical: true })` sets `preserveAudit: true` on the write context (`@objectstack/rest`), and that branch still reinstates an original `created_at`, exactly as it has reinstated `updated_at`/`updated_by` since #3493. This is why the fix is the `preserveAudit` ternary rather than a bare `= now`. The create-side strip still does not read `preserveAudit` (2026-08-08 ruling, untouched): the preservation is the audit binder's, and it always was. + +**A creator that back-dated rows through the old `??` must now ask for it.** Any insert path that supplied a historical `created_at` without `preserveAudit` now gets the server instant. The remedy is one context key on the write (`preserveAudit: true`), the same one `treatAsHistorical` sets. + +Ruled by the maintainer on 2026-09-06 (decision batch #54, option A). diff --git a/packages/objectql/src/plugin-audit-created-at-create-side.test.ts b/packages/objectql/src/plugin-audit-created-at-create-side.test.ts new file mode 100644 index 0000000000..f181ab4146 --- /dev/null +++ b/packages/objectql/src/plugin-audit-created-at-create-side.test.ts @@ -0,0 +1,233 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #15964 — on an ORDINARY create the audit binder stamps `created_at` from the +// system clock, so a caller-supplied value never survives a plain REST `POST`. +// +// ## The hole, and why it needed the three controls below +// +// Measured on a live rig (framework `e581457baaaf`): a normal authenticated +// caller `POST /api/v1/data/OBJECT` with a forged `created_at` kept that value +// on the stored row, on objects that declare `created_at` as `readonly: true`. +// The same request, same path, same object family: +// +// field declaration sent stored verdict +// --------------------- ------------------- ------------ --------- -------- +// id readonly: true forged minted stripped +// run_at readonly, datetime 1999-01-01 now stripped +// updated_at readonly, datetime 1999-01-01 now stripped +// created_at readonly, datetime 1999-01-01 1999-01-01 KEPT +// +// The three stripped rows are the reading's in-experiment controls: they prove +// the create-side strip IS running on this path and DOES take other +// author-declared `readonly` datetimes, so `created_at` surviving is "the strip +// ran and spared exactly this one", never "the strip did not run". +// +// ## Mechanism +// +// Since #15395 the static-`readonly` strip runs INSIDE `engine.insert`, AFTER +// the `beforeInsert` hooks, and its #14259 guard treats a key a hook ASSIGNED +// as the hook's write rather than a caller forgery (`rowHookWrittenKeys`). The +// audit binder stamped `record.created_at = record.created_at ?? now`, so on a +// forged payload the hook "wrote" a value whose bytes came entirely from the +// caller — and the strip spared it. `updated_at`'s `preserveAudit ? (… ?? now) +// : now` overwrote the forgery first, which is why it is a control here rather +// than a second symptom. +// +// ## Maintainer ruling, 2026-09-06 (decision batch #54, option A), verbatim +// 「同意」 +// +// - the beforeInsert stamp for `created_at` takes the SAME SHAPE as +// `updated_at` — the system clock wins on an ordinary create; +// - the historical-import channel KEEPS working: `treatAsHistorical` sets +// `preserveAudit` on the write context (`packages/rest/src/import-runner.ts`), +// and that branch still reinstates an original `created_at`. That is the +// third case below, and it is the reason the ruled shape is the +// `preserveAudit`-branching one rather than a bare `= now`. +// +// Consistent with the 2026-08-08 ruling that narrowed `preserveAudit` to the +// UPDATE path: `created_at` is preserved here by the audit binder's own +// `preserveAudit` branch, which is where the flag has always been read on this +// path — the create-side strip still does not read it (#14147). + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQLPlugin } from './plugin.js'; +import { ObjectQL } from './engine.js'; + +const FORGED_AT = '1999-01-01T00:00:00.000Z'; +const FORGED_ID = 'conv_REST_FORGED'; + +describe('audit binder: create-side `created_at` (#15964)', () => { + let kernel: ObjectKernel; + + beforeEach(() => { + kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false }); + }); + + afterEach(async () => { + if (kernel.getState() === 'running') await kernel.shutdown(); + }); + + /** + * Boots the REAL ingress this card is about: `ObjectQLPlugin` binds its + * `sys_stamp_audit_insert` hook through `bindHooksToEngine`, and + * `engine.insert` runs the static-readonly strip after it. What reaches the + * driver's `create` IS the stored row, so the payload is the verdict. + */ + async function boot(objectName: string) { + const captured: Record[] = []; + const mockDriver = { + name: 'audit-capture', version: '1.0.0', + connect: async () => {}, disconnect: async () => {}, + find: async () => [], findOne: async () => null, + create: async (_o: string, d: any) => { + captured.push({ ...d }); + return { id: d.id ?? 'minted_id', ...d }; + }, + update: async (_o: string, i: any, d: any) => ({ id: i, ...d }), + delete: async () => true, syncSchema: async () => {}, + }; + await kernel.use({ + name: 'audit-capture-plugin', type: 'driver', version: '1.0.0', + init: async (ctx: any) => { ctx.registerService('driver.audit-capture', mockDriver); }, + } as any); + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + + const objectql = kernel.getService('objectql'); + // `created_at` / `updated_at` are NOT declared here: the registry injects + // them from `AUDIT_FIELD_DEFS`, where both are `readonly: true` datetimes — + // the same declaration the card's three live objects carry. + const schema = { + name: objectName, + label: 'Repro Object', + datasource: 'audit-capture', + fields: { + id: { name: 'id', label: 'Id', type: 'text', readonly: true }, + title: { name: 'title', label: 'Title', type: 'text' }, + run_at: { name: 'run_at', label: 'Run At', type: 'datetime', readonly: true }, + }, + } as any; + objectql.registry.registerObject(schema, 'test', 'test'); + return { objectql, captured }; + } + + const forgedPayload = () => ({ + id: FORGED_ID, + title: 'x', + run_at: FORGED_AT, + created_at: FORGED_AT, + updated_at: FORGED_AT, + }); + + /** Prints the card's four-field table for the row that reached the driver. */ + function printTable(label: string, row: Record) { + const verdict = (v: unknown, forged: unknown) => (v === forged ? 'KEPT (forged)' : 'stripped/overwritten'); + // eslint-disable-next-line no-console + console.log( + `\n[#15964 ${label}]\n` + + ` id sent=${FORGED_ID} stored=${String(row.id)} -> ${verdict(row.id, FORGED_ID)}\n` + + ` run_at sent=${FORGED_AT} stored=${String(row.run_at)} -> ${verdict(row.run_at, FORGED_AT)}\n` + + ` updated_at sent=${FORGED_AT} stored=${String(row.updated_at)} -> ${verdict(row.updated_at, FORGED_AT)}\n` + + ` created_at sent=${FORGED_AT} stored=${String(row.created_at)} -> ${verdict(row.created_at, FORGED_AT)}\n`, + ); + } + + it('an ordinary create: the caller-supplied `created_at` does NOT survive, and the three controls stay stripped', async () => { + const { objectql, captured } = await boot('repro_conversations'); + + await objectql.insert('repro_conversations', forgedPayload(), { + context: { userId: 'user-1' }, + }); + + expect(captured.length).toBe(1); + const row = captured[0]; + printTable('after', row); + + // The three in-experiment controls — each was already stripped BEFORE this + // change, and a fix that closes `created_at` while opening any of them is a + // regression on a security card. + expect(row.id).not.toBe(FORGED_ID); + expect(row.run_at).not.toBe(FORGED_AT); + expect(row.updated_at).not.toBe(FORGED_AT); + + // The card's row. Overwritten by the binder rather than deleted, which is + // why `created_at` is still present and still a real stamp. + expect(row.created_at).not.toBe(FORGED_AT); + expect(typeof row.created_at).toBe('string'); + expect(Date.parse(row.created_at)).toBeGreaterThan(Date.parse('2020-01-01T00:00:00.000Z')); + // …and the two audit timestamps agree on a create, as they did before. + expect(row.updated_at).toBe(row.created_at); + }); + + it('a create that sends no `created_at` is still stamped (the binder keeps doing its job)', async () => { + const { objectql, captured } = await boot('repro_plain'); + + await objectql.insert('repro_plain', { title: 'x' }, { context: { userId: 'user-1' } }); + + const row = captured[0]; + expect(typeof row.created_at).toBe('string'); + expect(Date.parse(row.created_at)).toBeGreaterThan(Date.parse('2020-01-01T00:00:00.000Z')); + }); + + // [#15964] `isSystem` on its own is NOT a back-dating channel, and never was. + // It exempts the engine's readonly STRIP; the audit binder's stamp is not + // gated on it at all. Before this change a system-context seed kept its + // supplied `created_at` because of the `??`, not because of its elevation — + // which is how a legitimate back-dating fixture came to depend on the hole + // (`packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts` seeds a + // timezone-boundary instant exactly this way). Both halves are pinned here so + // the next such seed is told which flag it actually needs. + it('`isSystem` alone does NOT preserve it, and `isSystem` + `preserveAudit` does', async () => { + const { objectql, captured } = await boot('repro_system_seed'); + + await objectql.insert('repro_system_seed', forgedPayload(), { + context: { isSystem: true }, + }); + await objectql.insert('repro_system_seed', forgedPayload(), { + context: { isSystem: true, preserveAudit: true }, + }); + + const [elevatedOnly, historical] = captured; + printTable('isSystem only', elevatedOnly); + printTable('isSystem + preserveAudit', historical); + + // The system context skips the strip, so `id` and `run_at` DO survive here — + // that is the control proving the elevation really took effect, and it is + // what makes the `created_at` row below a statement about the binder alone. + expect(elevatedOnly.id).toBe(FORGED_ID); + expect(elevatedOnly.run_at).toBe(FORGED_AT); + // …and the binder still stamps, because it is not gated on `isSystem`. + expect(elevatedOnly.created_at).not.toBe(FORGED_AT); + expect(elevatedOnly.updated_at).not.toBe(FORGED_AT); + + // The explicit channel — `ExecutionContext.preserveAudit`, what REST's + // `treatAsHistorical` import sets — reaches the binder through + // `buildSession` independently of `isSystem`, so a back-dated seed works. + expect(historical.created_at).toBe(FORGED_AT); + expect(historical.updated_at).toBe(FORGED_AT); + }); + + // The ruled control: the historical-import channel is EXPLICIT and still + // works. `runImport({ treatAsHistorical: true })` puts `preserveAudit: true` + // on the write context (`packages/rest/src/import-runner.ts`), which is + // exactly the context asserted here. + it('`preserveAudit` (what `treatAsHistorical` sets) still reinstates the original `created_at`', async () => { + const { objectql, captured } = await boot('repro_historical'); + + await objectql.insert('repro_historical', forgedPayload(), { + context: { userId: 'user-1', preserveAudit: true }, + }); + + const row = captured[0]; + printTable('preserveAudit control', row); + + expect(row.created_at).toBe(FORGED_AT); + // Symmetric with `updated_at`, which has had this branch since #3493. + expect(row.updated_at).toBe(FORGED_AT); + // …and the exemption is the audit binder's, not the strip's: a non-audit + // readonly field is still taken on the create side (2026-08-08 ruling, + // unchanged by this card). + expect(row.run_at).not.toBe(FORGED_AT); + }); +}); diff --git a/packages/objectql/src/plugin.ts b/packages/objectql/src/plugin.ts index b261d3cf78..e3b3bdda7f 100644 --- a/packages/objectql/src/plugin.ts +++ b/packages/objectql/src/plugin.ts @@ -1100,13 +1100,27 @@ export class ObjectQLPlugin implements Plugin { ) => { const now = stamp(); // A "historical" import (#3493) reinstates the ORIGINAL timeline, so a - // client-supplied updated_at/updated_by is CLIENT-PREFERRED here — - // symmetric with created_at/created_by on insert — instead of being - // overwritten with the import instant. Opt-in and server-set only; a - // normal write leaves `preserveAudit` unset and still stamps now. + // client-supplied created_at/updated_at/updated_by is CLIENT-PREFERRED + // under `preserveAudit` — instead of being overwritten with the import + // instant. Opt-in and server-set only; a normal write leaves + // `preserveAudit` unset and still stamps now. + // + // [#15964] `created_at` takes the SAME SHAPE as `updated_at`, on the + // maintainer ruling of 2026-09-06 (decision batch #54, option A). It was + // `record.created_at ?? now` on EVERY insert — client-preferred with no + // flag at all — and since #15395 the static-`readonly` strip runs INSIDE + // `engine.insert`, AFTER this hook, where #14259's guard reads a key a + // `beforeInsert` hook ASSIGNED as the hook's write rather than a caller + // forgery (`rowHookWrittenKeys`). The `??` therefore LAUNDERED the + // caller's bytes past that strip: an ordinary authenticated POST kept a + // forged `created_at` on an object declaring it `readonly: true`, while + // `id`, `updated_at` and every other author-declared readonly datetime in + // the same payload were taken. Under `preserveAudit` the preservation is + // DECLARED, so the same keep is the ruled historical-import channel and + // stays — which is why the fix is this ternary and not a bare `= now`. const preserveAudit = session?.preserveAudit === true; if (isInsert) { - record.created_at = record.created_at ?? now; + record.created_at = preserveAudit ? (record.created_at ?? now) : now; } record.updated_at = preserveAudit ? (record.updated_at ?? now) : now; if (session?.userId) { diff --git a/packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts b/packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts index 76a19aef9f..4c5767aac7 100644 --- a/packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts +++ b/packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts @@ -42,14 +42,34 @@ describe('dogfood: org timezone drives analytics date bucketing (#1982/#2018)', stack = await bootStack(crmStack); // Deterministic fixture: N leads pinned to the tz-boundary instant, inserted - // as system so the write path's defaults/validation don't fight the setup. + // as system so the write path's defaults/validation don't fight the setup, + // and with `preserveAudit` because a BACK-DATED `created_at` is exactly what + // that flag exists to permit. + // + // [#15964] `preserveAudit` is REQUIRED here and is not decoration. The audit + // binder's `beforeInsert` stamp used to be `record.created_at ?? now` — a + // caller-supplied value won on EVERY insert, with no flag — which is the + // hole a plain REST `POST` reached to forge the audit anchor. It is now the + // same shape as `updated_at`, `preserveAudit ? (… ?? now) : now`, on the + // maintainer ruling of 2026-09-06 (decision batch #54, option A). This + // fixture is a LEGITIMATE back-dating consumer of the old behaviour — the + // first one measured — so it moves to the explicit channel the same ruling + // preserved rather than the accident it used to ride on. + // + // ⚠️ `isSystem` alone does NOT do this and never did: it exempts the engine's + // readonly STRIP, not the audit binder's stamp, which is why this fixture + // used to depend on the `??` rather than on its own elevation. Measured + // both ways in `@objectstack/objectql`'s + // `plugin-audit-created-at-create-side.test.ts`. `preserveAudit` is the flag + // REST's `treatAsHistorical` import sets on the write context + // (`packages/rest/src/import-runner.ts`), reaching this same hook. // eslint-disable-next-line @typescript-eslint/no-explicit-any const ql = await stack.kernel.getServiceAsync('objectql'); for (let i = 0; i < N_LEADS; i++) { await ql.insert( 'crm_lead', { name: `tz-lead-${i}`, status: 'new', created_at: BOUNDARY }, - { context: { isSystem: true } }, + { context: { isSystem: true, preserveAudit: true } }, ); } // Sanity: confirm created_at actually persisted as the boundary instant