diff --git a/.changeset/21663-readonly-value-shape-refused.md b/.changeset/21663-readonly-value-shape-refused.md new file mode 100644 index 00000000000..a410a1305a3 --- /dev/null +++ b/.changeset/21663-readonly-value-shape-refused.md @@ -0,0 +1,51 @@ +--- +'@objectstack/objectql': minor +--- + +fix(objectql)!: a system write's readonly value is judged for its shape — a seed's `'yesterday'` on a readonly datetime is refused with the sentence any other field gets, never stored (#21663) + +**BREAKING** — a write that keeps a readonly value now has that value's SHAPE +checked. The static readonly strip still exempts a system write (seed replay, +migration, `isSystem` plugin code, a `before*` hook's stamp) and still drops a +non-system caller's readonly value; what changed is that the value the +exemption keeps is no longer stored unjudged. Before, the record validator +skipped every readonly field, so under `isSystem` a malformed readonly value +reached the driver verbatim — a seed's `run_at: 'yesterday'` on a readonly +`datetime`, an unresolved `cel` envelope from a seeder that skips its +resolution, an authored `created_at` the seed now keeps — while the same value +on a non-readonly field was refused. + +Now it is refused the same way: `VALIDATION_FAILED` (400 at an HTTP boundary), +the same field code and the same sentence a non-readonly field gets +(`Run At must be a valid datetime (ISO-8601)`), and a seed counts the row as a +seed error. This holds on insert, on the dry run (`ObjectQL.validate`), and on +both update paths, where the readonly values left after the strip are judged. + +Which checks a readonly value reaches — its type's shape, never a constraint: + +- refused: a `date` / `datetime` / `time` the platform does not read, a + non-number on a number-typed field, a non-boolean on a boolean, a non-array on + a multi-value field, a filter-operator object, and an ADR-0104 reference / + media / structured-JSON shape under the object's own posture (warn-first, as + on any other field, until the deployment's evidence enforces it); +- NOT checked, exactly as before: option membership, `maxLength` / + `minLength`, `valueDomain`, `min` / `max` / `scale` / `precision`, the email / + url / phone formats, and `required`. Option membership stays out on purpose: + `sys_activity.type` is a readonly `select` whose options are the built-in set + of an open vocabulary, and an author-contributed value there is stored. + +A numeric string on a readonly number field is now written as its number, and a +lone scalar on a readonly multi-value field as a one-member list, as on any +other field — the door reads the value the same way it judges it. + +**What moves for consumers.** A seed, migration or `isSystem` write that puts a +malformed value in a readonly field — or a hook that stamps one — is refused +where it was stored. Fix the value at its producer: write an ISO-8601 instant +(or a `Date`) into a readonly `datetime`, resolve a `cel` value before the +write, and stamp numbers and booleans as such. Rows already stored are never +re-read or rewritten. `validateRecord`, as exported, is unchanged: the readonly +scope is the engine write path's own. + +Clause-②: no (narrowing) + + diff --git a/packages/objectql/src/engine-insert-static-readonly-strip.test.ts b/packages/objectql/src/engine-insert-static-readonly-strip.test.ts index 9267fe4352a..ef57e18fcff 100644 --- a/packages/objectql/src/engine-insert-static-readonly-strip.test.ts +++ b/packages/objectql/src/engine-insert-static-readonly-strip.test.ts @@ -382,8 +382,12 @@ describe('#14147 — strictReadonlyWrites refuses before any driver dispatch', ( }); it('strict adds NO second policy — an isSystem write it would not strip is still accepted', async () => { + // A well-formed value: since #21663 a system writer's readonly value is + // judged for its SHAPE (a placeholder like `'x'` in a datetime is refused + // as `invalid_date`), which is a different policy from the one this case + // is about — `strictReadonlyWrites` adding nothing to the strip. const o = await observeInsert( - { title: 'T', completed_at: 'x' }, + { title: 'T', completed_at: '2019-04-01T00:00:00Z' }, { strictReadonlyWrites: true, context: { isSystem: true } }, ); expect(o.refusedCode).toBeNull(); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index be995546c88..4cb5f1345ff 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -262,7 +262,7 @@ import { deriveViewContainerObject } from '@objectstack/metadata/view-container' // registrar and `os validate` both call. import { viewContainerNameRefusal } from './view-container-name-refusal.js'; import { bindHooksToEngine } from './hook-binder.js'; -import { validateRecord, normalizeMultiValueFields, normalizeBlankTypedValues, normalizeNumericStringValues, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js'; +import { validateRecord, validateRecordInScope, normalizeMultiValueFields, normalizeBlankTypedValues, normalizeNumericStringValues, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js'; import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink } from './validation/record-validator.js'; import type { RelatedFieldBinding, RelatedRecordBinding } from './validation/rule-validator.js'; import { collectPredicateRelationships, evaluateValidationRules, optionVisibilityReadsPermissions, readsPermissionPredicate, referentialClearBinding, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, hasParentScopedReadonlyWhenInPayload, hasParentScopedRequiredWhen, stripReadonlyFields, stripRuntimeOwnedFields, staticReadonlyInsertSubject, preserveAuditIgnoredOnInsertWarning } from './validation/rule-validator.js'; @@ -6063,9 +6063,10 @@ export class ObjectQL implements IObjectQLEngine { // so the same declaration behaved differently per datasource. That // split surfaced two ways: a validation-visible field was REJECTED by // the engine's own write validator ("must be a valid datetime"), and a - // `readonly`/`system` field — which `validateRecord` skips, i.e. the - // ~100 `created_at`/`updated_at` platform declarations — silently - // stored the four characters `NOW()`. + // `readonly`/`system` field — which `validateRecord` then skipped, i.e. + // the ~100 `created_at`/`updated_at` platform declarations — silently + // stored the four characters `NOW()`. (Since #21663 a readonly value's + // shape is judged too, so that literal would now be refused.) // // Resolved from the caller's `nowSnapshot`, so every defaulted field // in one insert (and every row of one batch) carries the SAME instant. @@ -9884,12 +9885,19 @@ export class ObjectQL implements IObjectQLEngine { * — and not raw type membership, because the registry INJECTS covered-type * fields into every object it registers: `organization_id` and `owner_id` * (both `system`), plus `created_by` / `updated_by` (both in `SKIP_FIELDS`), - * are all `lookup`s. `validateRecord` skips every one of them before it ever - * reaches the value-shape check, so counting them made this answer `true` for - * literally every object — the dormancy rule above never fired, and this - * cache memoized a constant. Same predicate as the scanner for the same - * reason the scanner imports it: three readings of "a covered field" drifting - * by one clause is how a gate ends up governing fields nothing enforces. + * are all `lookup`s. A caller never writes any of them, so counting them made + * this answer `true` for literally every object — the dormancy rule above + * never fired, and this cache memoized a constant. Same predicate as the + * scanner for the same reason the scanner imports it: three readings of "a + * covered field" drifting by one clause is how a gate ends up governing + * fields nothing enforces. + * + * [#21663] The three that are `readonly` (`organization_id`, `created_by`, + * `updated_by`) DO reach the value-shape check now, on the value a system + * writer, hook or stamp stores. They still do not count here, so an object + * whose only covered fields are those stays warn-first for them: a malformed + * value is admitted, logged and reported, never stored silently. See + * `isScannableValueShapeField` for why widening this test is not the fix. */ private objectHasCoveredValueField(objectSchema: any): boolean { if (!objectSchema?.fields) return false; @@ -12469,9 +12477,11 @@ export class ObjectQL implements IObjectQLEngine { * call that fires side-effecting hooks (mail, outbound calls, writes to * other objects) is the #4052 defect in a new spelling, where a preview * quietly executes. So the gap is documented rather than closed: audit and - * ownership stamps are `system`/`readonly` and are skipped by validation - * anyway, so what remains is the narrow case of a hook deriving a - * *business* field that its object also validates. + * ownership stamps are `system`/`readonly`, so validation never requires + * them, and (#21663) the only thing it asks of a readonly value is its + * shape, which a platform stamp always has — so what remains is the narrow + * case of a hook deriving a *business* field that its object also + * validates. * * Nothing is written, no sequence is consumed, and no driver is touched — * validation is in-process, which is what makes row-by-row dry run of a @@ -12724,7 +12734,11 @@ export class ObjectQL implements IObjectQLEngine { }); }; try { - validateRecord(schemaForValidation, row, mode, { + // [#21663] `'include'` in both modes: the caller-write strips ran + // above, so this is the payload the write stores — and the write + // judges its readonly values' shape (insert in the same call, update + // in a second pass after its own strip). + validateRecordInScope(schemaForValidation, row, mode, 'include', { mediaValueShapeStrict, valueShapeStrict, messages, onAdmittedValueShapeViolation, }); evaluateValidationRules(schemaForValidation as any, row, mode, { @@ -13515,8 +13529,13 @@ export class ObjectQL implements IObjectQLEngine { for (let i = 0; i < rows.length; i++) { if (rowErrors[i] !== undefined) continue; try { - normalizeMultiValueFields(schemaForValidation, rows[i]); - validateRecord(schemaForValidation, rows[i], 'insert', { mediaValueShapeStrict, valueShapeStrict, messages: msgCtx, onAdmittedValueShapeViolation }); + // [#21663] `'include'`: the readonly strip ran above, so every + // readonly value still on the row is one the driver will store — + // a system writer's (seed, migration), a hook's or a stamp — and + // its SHAPE is judged here like any other field's. See + // `ReadonlyValueScope` (record-validator.ts). + normalizeMultiValueFields(schemaForValidation, rows[i], 'include'); + validateRecordInScope(schemaForValidation, rows[i], 'insert', 'include', { mediaValueShapeStrict, valueShapeStrict, messages: msgCtx, onAdmittedValueShapeViolation }); evaluateValidationRules(schemaForValidation as any, rows[i], 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: msgCtx, parent: insertParentForRow?.(rows[i]), related: insertRelatedForRow(rows[i]), permissions: insertPermissionsFor(rows[i]) }); await this.assertReferencesResolve( schemaForValidation, rows[i], suppliedPerRow[i], opCtx.context, msgCtx, @@ -14866,7 +14885,13 @@ export class ObjectQL implements IObjectQLEngine { // secret channel (which carries the secret-arm refusal). this.refuseEmptyPasswordFields(object, hookContext.input.data as Record); await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); - normalizeMultiValueFields(updateSchema, hookContext.input.data as Record); + // [#21663] Scope `'skip'` — the public `validateRecord` IS that + // scope: the readonly strip has NOT run yet, so a readonly value + // here may be a caller's the strip is about to drop — judged, a + // whole-record write-back echoing a legacy stored value would + // become a refusal. Readonly values are judged after the strip + // (`validateRecordInScope(…, 'only')`, below). + normalizeMultiValueFields(updateSchema, hookContext.input.data as Record, 'skip'); validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx, onAdmittedValueShapeViolation }); // [#5284] Demand-driven, and the demand is asked PER OBJECT. // @@ -15051,6 +15076,15 @@ export class ObjectQL implements IObjectQLEngine { // "you sent a read-only field" should not depend on whether some // other field also failed a business rule. assertNoStrictDrops(); + // [#21663] The payload is FINAL here (see the seam below), so + // every readonly value on it is one the driver will store: a + // system writer's (the strip above never ran for it), a hook's, + // or a stamp. Its SHAPE is judged now, by the same arms and + // sentences as the caller-writable fields the first + // `validateRecord` above judged ahead of the strip — `'only'`, + // because those are already judged. See `ReadonlyValueScope`. + normalizeMultiValueFields(updateSchema, hookContext.input.data as Record, 'only'); + validateRecordInScope(updateSchema, hookContext.input.data as Record, 'update', 'only', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx, onAdmittedValueShapeViolation }); // ── [#19989] The post-image seam on the BY-ID path ───────────── // // The by-id twin of the predicate-path call below, placed at the @@ -15186,7 +15220,13 @@ export class ObjectQL implements IObjectQLEngine { // secret channel (which carries the secret-arm refusal). this.refuseEmptyPasswordFields(object, hookContext.input.data as Record); await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); - normalizeMultiValueFields(updateSchema, hookContext.input.data as Record); + // [#21663] Scope `'skip'` — the public `validateRecord` IS that + // scope: the readonly strip has NOT run yet, so a readonly value + // here may be a caller's the strip is about to drop — judged, a + // whole-record write-back echoing a legacy stored value would + // become a refusal. Readonly values are judged after the strip + // (`validateRecordInScope(…, 'only')`, below). + normalizeMultiValueFields(updateSchema, hookContext.input.data as Record, 'skip'); validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx, onAdmittedValueShapeViolation }); // [#2982] The middleware-composed AST — asserted present and // bound to the memoized row read in the pre-phase above, so the @@ -15305,6 +15345,12 @@ export class ObjectQL implements IObjectQLEngine { // caller is told before N rows are written with a column missing // — the failure mode a bulk write makes N times larger. assertNoStrictDrops(); + // [#21663] The predicate-path twin of the by-id second pass, at the + // same point and for the same reason: the readonly values left on + // the final payload are stored, so their SHAPE is judged — before + // N rows are written. + normalizeMultiValueFields(updateSchema, hookContext.input.data as Record, 'only'); + validateRecordInScope(updateSchema, hookContext.input.data as Record, 'update', 'only', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx, onAdmittedValueShapeViolation }); // ── [#19950] The post-image seam on the PREDICATE path ───────── // // An enforcement layer's write `check` must hold for EVERY row a diff --git a/packages/objectql/src/seed-readonly-value-shape.test.ts b/packages/objectql/src/seed-readonly-value-shape.test.ts new file mode 100644 index 00000000000..e9d979a1efa --- /dev/null +++ b/packages/objectql/src/seed-readonly-value-shape.test.ts @@ -0,0 +1,379 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #21663 — a system writer is exempt from the readonly STRIP, never from the +// value-SHAPE check. +// +// ## The defect, as measured on unfixed `main` (72f3c74d60) +// +// field writer value outcome +// ---------------------------- ------------------------- ------------- ----------------------------- +// readonly datetime seed (`isSystem`) 'yesterday' stored verbatim, no error +// NON-readonly datetime seed (`isSystem`) 'yesterday' refused, counted as seed error +// readonly datetime seeder skipping the cel envelope stored verbatim +// `cel` resolution +// +// The record validator skipped every `readonly` field, on the premise that the +// readonly strip had already removed anything a caller sent. The strip exempts +// a system write, so for exactly those writers the premise was false. +// +// ## Ruling (triage on #21663, verbatim from "Ruling" to the pins) +// +// > - Split the branch. The readonly strip keeps its system-context exemption, +// > and the value-shape check runs for every write. +// > - A malformed readonly value is refused loudly with the same message the +// > non-readonly path gives, and a seed counts it as a seed error. +// > - ⛔ No silent coercion. +// > +// > **Pins:** +// > - `'yesterday'` on a readonly datetime in a seed is refused; +// > - a valid ISO value on a readonly field under the seed context is kept; +// > - the non-readonly path is unchanged. +// +// Each refusal asserts the ADR-0112 pair: `code`, and the `status` the HTTP +// boundary assigns (`resolveThrownHttpError` — a `ValidationError` carries no +// `status` of its own by design). The seed loader's own error record carries +// only a sentence, so on the loader the pin asserts the count and that +// sentence, and the code/status pair is asserted on the loader's own call +// shape (an engine write under `SEED_WRITE_EXECUTION_CONTEXT`). + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { cel } from '@objectstack/spec'; +import { SEED_WRITE_EXECUTION_CONTEXT } from '@objectstack/spec/kernel'; +import { SeedLoaderService } from '@objectstack/metadata-protocol'; +import { resolveThrownHttpError } from '@objectstack/types'; +import { ObjectQLPlugin } from './plugin.js'; +import { ObjectQL } from './engine.js'; + +/** `run_at` is READONLY here … */ +const RO = 'seed_ro_case'; +/** … and the SAME field, minus `readonly`, here: the non-readonly control. */ +const RW = 'seed_rw_case'; +const BOOT = '2026-10-03T12:00:00.000Z'; +const ISO = '2026-09-01T12:00:00.000Z'; +/** `daysAgo(5)` at `BOOT`: UTC midnight of the calendar day five days back. */ +const DAYS_AGO_5 = '2026-09-28T00:00:00.000Z'; + +/** A store-backed stub driver: the stored row IS the verdict. */ +function makeStubDriver() { + const stores = new Map>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + const checkOp = (value: any, cond: any): boolean => { + if (cond === null || typeof cond !== 'object' || Array.isArray(cond) || cond instanceof Date) { + return value === cond; + } + return Object.entries(cond).every(([op, target]: [string, any]) => { + switch (op) { + case '$eq': return value === target; + case '$ne': return value !== target; + case '$in': return Array.isArray(target) && target.includes(value); + // REFUSE, never silently match — an unsupported operator answered + // `true` would read as a hit for every row. + default: throw new Error(`stub driver: unsupported operator ${op}`); + } + }); + }; + const matches = (row: any, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]: [string, any]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k === '$or') return (v as any[]).some((w) => matches(row, w)); + if (k.startsWith('$')) throw new Error(`stub driver: unsupported combinator ${k}`); + return checkOp(row?.[k], v); + }); + }; + let n = 0; + const driver: any = { + name: 'seed-store', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async syncSchema() {}, + async find(object: string, ast: any) { + const rows = Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)); + const page = typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + return page.map((r) => ({ ...r })); + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matches(r, ast?.where)) return { ...r }; + return null; + }, + async create(object: string, data: Record) { + n += 1; + const id = (data.id as string) ?? `r_${n}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return { ...row }; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const row = { ...s.get(id), ...data, id }; + s.set(id, row); + return { ...row }; + }, + // The predicate seam's entry point: without it the engine never reaches + // the predicate path, so that seam would go unmeasured. + async updateMany(object: string, ast: any, data: Record) { + const s = storeFor(object); + let changed = 0; + for (const [id, row] of s) { + if (!matches(row, ast?.where)) continue; + s.set(id, { ...row, ...data, id }); + changed += 1; + } + return changed; + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count(object: string, ast: any) { + return Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)).length; + }, + }; + return { driver, storeFor }; +} + +function emptyMetadata() { + return { + getObject: async () => undefined, + listObjects: async () => [], + register: async () => {}, + get: async () => undefined, + list: async () => [], + unregister: async () => {}, + exists: async () => false, + listNames: async () => [], + }; +} + +const quietLogger = { info() {}, warn() {}, error() {}, debug() {} }; + +const LOAD_CONFIG = { + dryRun: false, + haltOnError: false, + multiPass: true, + defaultMode: 'upsert', + batchSize: 1000, + transaction: false, +} as any; + +const seed = (object: string, records: Array>) => ({ + object, + externalId: 'ref', + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records, +}); + +/** The validation sentence a seed error quotes, after its `(ref=…): ` lead. */ +const quotedSentence = (message: string) => message.slice(message.indexOf('): ') + 3); + +/** The thrown refusal, read the way an HTTP boundary reads it. */ +async function refusal(write: () => Promise) { + let thrown: any; + try { + await write(); + } catch (err) { + thrown = err; + } + expect(thrown, 'the write must be refused').toBeDefined(); + const http = resolveThrownHttpError(thrown); + return { thrown, status: http.status, code: http.code, fields: thrown.fields as any[] }; +} + +describe('a system writer is exempt from the readonly strip, never from the value-shape check (#21663)', () => { + let kernel: ObjectKernel; + let objectql: ObjectQL; + let storeFor: ReturnType['storeFor']; + + beforeEach(async () => { + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date(BOOT)); + kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false }); + const stub = makeStubDriver(); + storeFor = stub.storeFor; + await kernel.use({ + name: 'seed-store-plugin', type: 'driver', version: '1.0.0', + init: async (ctx: any) => { ctx.registerService('driver.seed-store', stub.driver); }, + } as any); + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + objectql = kernel.getService('objectql'); + // `created_at` is NOT declared: the registry injects it from + // `AUDIT_FIELD_DEFS` (`readonly`, `system`, and a lifecycle name), as on + // every real object — three skips the old walk applied to it at once. + for (const [name, readonly] of [[RO, true], [RW, false]] as const) { + objectql.registry.registerObject({ + name, + label: name, + datasource: 'seed-store', + fields: { + ref: { name: 'ref', label: 'Ref', type: 'text' }, + subject: { name: 'subject', label: 'Subject', type: 'text' }, + run_at: { name: 'run_at', label: 'Run At', type: 'datetime', readonly }, + // The constraint arms a readonly value does NOT reach (see + // `ReadonlyValueScope`): an option set and a bound. + kind: { name: 'kind', label: 'Kind', type: 'select', options: [{ label: 'Built-in', value: 'builtin' }], readonly }, + score: { name: 'score', label: 'Score', type: 'number', max: 5, readonly }, + }, + } as any, 'test', 'test'); + } + }); + + afterEach(async () => { + vi.useRealTimers(); + vi.restoreAllMocks(); + if (kernel.getState() === 'running') await kernel.shutdown(); + }); + + const loader = () => new SeedLoaderService(objectql as never, emptyMetadata() as never, quietLogger as never); + const load = (...seeds: ReturnType[]) => + loader().load({ seeds: seeds as never, config: LOAD_CONFIG }); + const rows = (object: string, ref: string) => + Array.from(storeFor(object).values()).filter((r) => r.ref === ref); + const stored = (object: string, ref: string) => { + const found = rows(object, ref); + expect(found.length, `exactly one stored ${object} row for ref ${ref}`).toBe(1); + return found[0]; + }; + + // ── Pin 1 ──────────────────────────────────────────────────────────────── + describe("pin 1: 'yesterday' on a readonly datetime in a seed is refused, as the non-readonly path refuses it", () => { + it('the seed loader counts it as a seed error, quotes the non-readonly sentence, and stores no row', async () => { + const result = await load( + seed(RO, [{ ref: 'bad', run_at: 'yesterday' }]), + seed(RW, [{ ref: 'bad', run_at: 'yesterday' }]), + ); + + expect(result.summary.totalErrored).toBe(2); + expect(result.errors).toHaveLength(2); + const [ro, rw] = [RO, RW].map((o) => result.errors.find((e: any) => e.sourceObject === o)!); + expect(quotedSentence(ro.message)).toBe('Run At must be a valid datetime (ISO-8601)'); + expect(quotedSentence(ro.message)).toBe(quotedSentence(rw.message)); + expect(rows(RO, 'bad')).toEqual([]); + expect(rows(RW, 'bad')).toEqual([]); + }); + + it('the replay (UPDATE) of an existing seed row is refused too, and the stored value stands', async () => { + const first = await load(seed(RO, [{ ref: 'r', run_at: ISO }])); + expect(first.errors).toEqual([]); + + const replay = await load(seed(RO, [{ ref: 'r', run_at: 'yesterday' }])); + expect(replay.summary.totalErrored).toBe(1); + expect(quotedSentence(replay.errors[0].message)).toBe('Run At must be a valid datetime (ISO-8601)'); + expect(stored(RO, 'r').run_at).toBe(ISO); + }); + + it('the refusal is VALIDATION_FAILED / 400 on all four write seams, with the non-readonly field envelope', async () => { + const ctx = { context: SEED_WRITE_EXECUTION_CONTEXT }; + const [existing] = await objectql.insert(RO, [{ ref: 'e', run_at: ISO }], ctx); + + const seams: Array<[string, () => Promise]> = [ + ['insert', () => objectql.insert(RO, { ref: 'i', run_at: 'yesterday' }, ctx)], + ['update by id', () => objectql.update(RO, { id: existing.id, run_at: 'yesterday' }, ctx)], + ['update by predicate', () => objectql.update(RO, { run_at: 'yesterday' }, { ...ctx, where: { ref: 'e' }, multi: true } as any)], + ]; + for (const [seam, write] of seams) { + const r = await refusal(write); + expect([seam, r.code, r.status]).toEqual([seam, 'VALIDATION_FAILED', 400]); + expect(r.fields.map((f) => [f.field, f.code, f.message])).toEqual([ + ['run_at', 'invalid_date', 'Run At must be a valid datetime (ISO-8601)'], + ]); + } + // The control: the SAME write on the non-readonly twin answers the same envelope. + const control = await refusal(() => objectql.insert(RW, { ref: 'i', run_at: 'yesterday' }, ctx)); + expect([control.code, control.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(control.fields.map((f) => [f.field, f.code, f.message])).toEqual([ + ['run_at', 'invalid_date', 'Run At must be a valid datetime (ISO-8601)'], + ]); + expect(stored(RO, 'e').run_at).toBe(ISO); + // Positive control on the predicate seam: a valid value lands through it. + await objectql.update(RO, { run_at: DAYS_AGO_5 }, { ...ctx, where: { ref: 'e' }, multi: true } as any); + expect(stored(RO, 'e').run_at).toBe(DAYS_AGO_5); + + // The dry run (fourth seam) reports what the write refuses. + const preview = await objectql.validate(RO, { ref: 'p', run_at: 'yesterday' }, { mode: 'insert', context: SEED_WRITE_EXECUTION_CONTEXT }); + expect(preview.valid).toBe(false); + expect(preview.results[0].errors.map((f: any) => [f.field, f.code])).toEqual([['run_at', 'invalid_date']]); + }); + + it('an unresolved `cel` envelope from a seeder that skips its resolution is refused, and so is a malformed authored `created_at`', async () => { + // `AppPlugin`'s fallback inserts and `@objectstack/verify`'s `seed()` + // hand the row to the engine without `resolveSeedRecord`: this is their + // call shape, single-row and array. + const ctx = { context: SEED_WRITE_EXECUTION_CONTEXT }; + for (const write of [ + () => objectql.insert(RO, { ref: 'c1', run_at: cel`daysAgo(5)` }, ctx), + () => objectql.insert(RO, [{ ref: 'c2', run_at: cel`daysAgo(5)` }], ctx), + ]) { + const r = await refusal(write); + expect([r.code, r.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(r.fields.map((f) => [f.field, f.code])).toEqual([['run_at', 'invalid_date']]); + } + // The injected audit column the seed keeps since #21646. + const audit = await refusal(() => objectql.insert(RO, { ref: 'ca', created_at: 'yesterday' }, ctx)); + expect([audit.code, audit.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(audit.fields.map((f) => [f.field, f.code])).toEqual([['created_at', 'invalid_date']]); + expect(rows(RO, 'c1')).toEqual([]); + expect(rows(RO, 'c2')).toEqual([]); + expect(rows(RO, 'ca')).toEqual([]); + }); + }); + + // ── Pin 2 ──────────────────────────────────────────────────────────────── + it('pin 2: a valid ISO value on a readonly field under the seed context is kept — authored, evaluated from `cel`, or on `created_at`', async () => { + const result = await load(seed(RO, [ + { ref: 'iso', run_at: ISO, created_at: ISO }, + { ref: 'cel', run_at: cel`daysAgo(5)`, created_at: cel`daysAgo(5)` }, + ])); + expect(result.errors, JSON.stringify(result.errors)).toEqual([]); + expect(stored(RO, 'iso').run_at).toBe(ISO); + expect(stored(RO, 'iso').created_at).toBe(ISO); + expect(new Date(stored(RO, 'cel').run_at).toISOString()).toBe(DAYS_AGO_5); + expect(new Date(stored(RO, 'cel').created_at).toISOString()).toBe(DAYS_AGO_5); + + // The replay keeps it too. + const replay = await load(seed(RO, [{ ref: 'iso', subject: 'v2', run_at: ISO, created_at: ISO }])); + expect(replay.errors).toEqual([]); + expect(stored(RO, 'iso').subject).toBe('v2'); + expect(stored(RO, 'iso').run_at).toBe(ISO); + }); + + // ── Pin 3 ──────────────────────────────────────────────────────────────── + describe('pin 3: the non-readonly path is unchanged', () => { + it('a non-readonly datetime takes a valid value and refuses a malformed one, as before', async () => { + const result = await load(seed(RW, [{ ref: 'ok', run_at: ISO }])); + expect(result.errors).toEqual([]); + expect(stored(RW, 'ok').run_at).toBe(ISO); + const r = await refusal(() => objectql.insert(RW, { ref: 'no', run_at: 'yesterday' }, { context: { isSystem: true } })); + expect([r.code, r.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(r.fields.map((f) => [f.field, f.code])).toEqual([['run_at', 'invalid_date']]); + }); + + it('a NON-system caller\'s readonly value is still dropped by the strip, never refused — on insert and on a whole-record write-back', async () => { + const user = { context: { userId: 'user-1' } }; + const [row] = await objectql.insert(RO, [{ ref: 'u', run_at: 'yesterday' }], user); + expect(stored(RO, 'u').run_at).toBeUndefined(); + + // A form round-trip echoes every key it read, a malformed legacy one + // included. The strip drops it; judging it ahead of the strip would turn + // the save into a refusal. + storeFor(RO).get(row.id).run_at = 'legacy text'; + await objectql.update(RO, { id: row.id, subject: 'edited', run_at: 'legacy text' }, user); + expect(stored(RO, 'u').subject).toBe('edited'); + expect(stored(RO, 'u').run_at).toBe('legacy text'); + }); + + it('a readonly value reaches the SHAPE arms only: an undeclared option and an out-of-bound number are stored, as before', async () => { + // The open-vocabulary ruling on `sys_activity.type` (commit 88b9d749a): + // a readonly option set is the built-in set, not a closed enum. + await objectql.insert(RO, { ref: 'k', kind: 'author_value', score: 9 }, { context: SEED_WRITE_EXECUTION_CONTEXT }); + expect(stored(RO, 'k').kind).toBe('author_value'); + expect(stored(RO, 'k').score).toBe(9); + // …while the non-readonly twin refuses both, unchanged. + const r = await refusal(() => objectql.insert(RW, { ref: 'k', kind: 'author_value', score: 9 }, { context: SEED_WRITE_EXECUTION_CONTEXT })); + expect([r.code, r.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(r.fields.map((f) => [f.field, f.code]).sort()).toEqual([['kind', 'invalid_option'], ['score', 'max_value']]); + }); + }); +}); diff --git a/packages/objectql/src/validation/record-validator.number-value.test.ts b/packages/objectql/src/validation/record-validator.number-value.test.ts index 3e92258ff72..13e6641d9ea 100644 --- a/packages/objectql/src/validation/record-validator.number-value.test.ts +++ b/packages/objectql/src/validation/record-validator.number-value.test.ts @@ -271,7 +271,7 @@ describe('normalizeNumericStringValues: an admitted string is written as its num } }); - it('rewrites only what the arm judges: not a text field, not summary, not a system or readonly field, not id, not a non-string', () => { + it('rewrites only what the arm judges: not a text field, not summary, not a non-readonly system field, not id, not a non-string', () => { const schema = { fields: { id: { name: 'id', type: 'number' }, @@ -285,12 +285,16 @@ describe('normalizeNumericStringValues: an admitted string is written as its num }, } as any; const row = { - id: '12', created_at: '12', f_text: '12', f_summary: '12', f_formula: '12', f_system: '12', f_readonly: '12', + id: '12', created_at: '12', f_text: '12', f_summary: '12', f_formula: '12', f_system: '12', f_undeclared: '12', f_number: 12, }; expect(normalizeNumericStringValues(schema, row)).toBe(row); // CONTROL: the same schema does rewrite its judged field when it is a string. expect(normalizeNumericStringValues(schema, { ...row, f_number: '12' }).f_number).toBe(12); + // [#21663] …and a READONLY number field is judged now — its shape, on the + // value a system writer keeps — so its numeric string is written as its + // number too: what the arm judges is what the driver stores. + expect(normalizeNumericStringValues(schema, { ...row, f_readonly: '12' }).f_readonly).toBe(12); }); it('is pure: the caller\'s record is never mutated; one record or an array of them, copied only where changed', () => { diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index 627681bcb71..d57cbd30780 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -67,8 +67,16 @@ * * System-injected fields (`id`, `created_at`, `created_by`, * `updated_at`, `updated_by`, and provenance-flagged `system`/`readonly` - * columns such as an injected `organization_id`) are never validated - * here — the engine and the audit plugin manage them. + * columns such as an injected `organization_id`) are never REQUIRED here — + * the engine and the audit plugin supply them. + * + * [#21663] A `readonly` field's VALUE is still judged for its SHAPE when the + * engine's write path calls {@link validateRecordInScope} with `'include'` or + * `'only'` — see {@link ReadonlyValueScope} for which arms that is and why the + * engine does so only where the payload is final (after the readonly strip). + * The public {@link validateRecord} is unchanged. A `system` + * column that is NOT `readonly` (`owner_id`) and a lifecycle name with no + * `readonly` flag keep the full skip. * * On failure, a `ValidationError` is thrown with `.fields[]` holding * one entry per offending field. REST translates this into a @@ -600,22 +608,95 @@ function valueMayBeAnObject(def: FieldDef): boolean { * nested garbage) is left untouched so that `validateRecord` can reject it with * `invalid_type`. WHICH columns it is applied to is this door's: a declared * multi-valued field (`isMultiValueField`), never a lifecycle column or one the - * engine owns (`system` / `readonly`). + * engine owns (`system` / `readonly`) — except that [#21663] a `readonly` + * field is reached under the same {@link ReadonlyValueScope} the validator is + * handed, so a readonly value is judged in the form a non-readonly one is. + * ⛔ Pass `'include'` / `'only'` only AFTER the readonly strip: the strip keeps + * a key whose value is no longer the caller's, and a wrap here changes the + * value's identity. */ export function normalizeMultiValueFields( objectSchema: { fields?: Record } | undefined | null, data: Record | undefined | null, + readonlyValues: ReadonlyValueScope = 'skip', ): void { if (!objectSchema?.fields || !data) return; for (const [name, value] of Object.entries(data)) { - if (SKIP_FIELDS.has(name)) continue; const def = objectSchema.fields[name]; - if (!def || def.system || def.readonly || !isMultiValueField(def)) continue; + if (!def || !isMultiValueField(def) || !isInReadonlyScope(name, def, readonlyValues)) continue; const stored = multiValueStorageForm(value); if (stored !== value) data[name] = stored; } } +/** + * [#21663] Which `readonly` field values a write-door call reaches, beside the + * fields a caller may write. + * + * ## Why the split exists + * + * The static readonly strip ({@link stripReadonlyFields} in + * `rule-validator.ts`) takes a NON-system caller's value off a readonly field, + * and a system write is exempt from it (seed replay, migration, a hook-owned + * stamp). The record validator skipped every readonly field outright, on the + * premise that the strip had already removed anything a caller sent. That + * premise is false for exactly the writers the strip exempts: under + * `isSystem` a readonly value went to the driver unjudged, so a seed's + * `'yesterday'` on a readonly `datetime` — or an unresolved `cel` envelope — + * was stored verbatim, while the same value on a non-readonly field was + * refused. Triage's ruling on #21663: the strip keeps its system exemption, + * and the value-shape check runs for every write. + * + * ## The three scopes + * + * - `'skip'` — today's walk: a readonly field is not reached. The engine + * passes it where the strip has NOT yet run (the update path's first + * validation), because a caller's readonly value there is about to be + * dropped, not stored: judging it would turn a whole-record write-back that + * echoes a legacy stored value into a refusal. + * - `'include'` — the caller-writable fields AND each readonly field's value, + * for a payload that is FINAL (the insert path and the dry run, both after + * their strips). + * - `'only'` — readonly fields alone, for the update path's second pass after + * its strip, whose caller-writable fields the first pass already judged. + * + * ## Which arms a readonly value reaches — its SHAPE, never its constraints + * + * A readonly value is judged by the per-type arms that ask "is this a value of + * the declared type at all", with the same wire code and the same sentence a + * non-readonly field gets: a `date` / `datetime` / `time` the platform reads, + * a number for a number-typed field, a boolean, an array for a multi-value + * field, no filter-operator object, and the ADR-0104 reference / media / + * structured-JSON shape under the object's own posture (warn-first until the + * deployment's evidence says otherwise). It is never REQUIRED. + * + * ⛔ It does NOT reach the author-declared constraints on top of the type: + * option membership (`invalid_option`), `maxLength` / `minLength`, + * `valueDomain`, `min` / `max` / `scale` / `precision`, or the email / url / + * phone formats (the spec's stored shape for those types is a plain string). + * Option membership is the load-bearing exclusion: `sys_activity.type` is a + * readonly `select` whose declared options are the BUILT-IN set of an open + * vocabulary, and the maintainer ruling recorded at commit 88b9d749a binds that + * an author-contributed value is stored, not refused — enforcing the enum on a + * system-owned write is a direction that ruling did not take. + * + * A `system` column that is not `readonly` (`owner_id`) and a lifecycle name + * with no `readonly` flag are not reached in any scope: neither is the + * system-exempt half of the strip this split repairs. + */ +export type ReadonlyValueScope = 'skip' | 'include' | 'only'; + +/** A field the caller writes and `validateRecord` has always walked. */ +function isCallerWritableField(name: string, def: FieldDef): boolean { + return !SKIP_FIELDS.has(name) && !def.system && !def.readonly; +} + +/** Is `def` reached under `scope`? See {@link ReadonlyValueScope}. */ +function isInReadonlyScope(name: string, def: FieldDef, scope: ReadonlyValueScope): boolean { + if (def.readonly === true) return scope !== 'skip'; + return scope !== 'only' && isCallerWritableField(name, def); +} + /** * [#20308] A BLANK string on a non-string-typed column is that column's typed * blank, `null` — the write door's one reading of it. @@ -716,8 +797,13 @@ function isJudgedNumberType(type: string): boolean { * every other form, and this function pre-decides none of them. * * "Number-typed" is exactly what the arm judges ({@link isJudgedNumberType}), - * on exactly the fields `validateRecord` walks: never a `SKIP_FIELDS` name, a - * `system` or a `readonly` field. A value nobody judges is not rewritten. + * on exactly the fields `validateRecord` walks: never a `SKIP_FIELDS` name or a + * `system` field, unless [#21663] it is `readonly` — whose value's shape is + * judged on every write ({@link ReadonlyValueScope}), so its numeric string is + * written as its number like any other. A value nobody judges is not + * rewritten. Safe ahead of the readonly strip, where this runs: it runs before + * the caller-value snapshot too, so the strip compares the rewritten value with + * itself and still drops a non-system caller's readonly key. * * ## Why the door has to say it * @@ -773,10 +859,10 @@ function normalizeNumericStringRow(fields: Record, row: unknow if (!isPlainRecord(row)) return row; let out: Record | undefined; for (const [name, value] of Object.entries(row)) { - if (typeof value !== 'string' || SKIP_FIELDS.has(name)) continue; + if (typeof value !== 'string') continue; // Own-property: a field name may be `constructor` / `valueOf`. const def = Object.prototype.hasOwnProperty.call(fields, name) ? fields[name] : undefined; - if (!def || def.system || def.readonly || !isJudgedNumberType(def.type)) continue; + if (!def || !isJudgedNumberType(def.type) || !isInReadonlyScope(name, def, 'include')) continue; const n = parseNumericString(value); if (n === undefined) continue; (out ??= { ...row })[name] = n; @@ -902,6 +988,10 @@ function validateOne( ctx?: ValidationMessageContext, valueStrict = false, onAdmitted?: AdmittedValueShapeViolationSink, + // [#21663] A `readonly` field's value: its type's SHAPE arms only, never a + // constraint — see {@link ReadonlyValueScope}. Each `if (shapeOnly) return + // null` below sits where an arm's shape test ends and its constraints begin. + shapeOnly = false, ): FieldValidationError | null { const fail = ( code: FieldErrorCode, @@ -995,6 +1085,9 @@ function validateOne( // in driver-sql (the #11794 invariant). The email/url/phone format checks // below stay per-type conditions inside the branch. if (BOUNDED_STRING_FIELD_TYPES.has(t)) { + // A string type's stored shape is a string; everything below is a bound, + // a domain or a format the author declared on top of it. + if (shapeOnly) return null; const s = typeof value === 'string' ? value : String(value); if (def.maxLength !== undefined && s.length > def.maxLength) { return fail('max_length', { maxLength: def.maxLength, actual: s.length }); @@ -1107,6 +1200,7 @@ function validateOne( if (n === undefined || !Number.isFinite(n)) { return fail('invalid_number'); } + if (shapeOnly) return null; // `min` / `max` bind on every type through this door, `progress` included. // [#20386] `progress` joined the TYPE check above in #20308 and, with this // change, the bounds: `FieldSchema.min` / `max` declare 「Checked on the @@ -1397,6 +1491,9 @@ function validateOne( // question from two authorities that happen to agree today, which is the // drift the one-definition ruling (#17469) closes. if ((t === 'select' || t === 'radio') && !isMultiValueField(def)) { + // Option membership is the whole arm, and it is a constraint: ⛔ never on + // a readonly value (the open-vocabulary ruling, see ReadonlyValueScope). + if (shapeOnly) return null; const allowed = optionValues(def.options); if (picklist !== undefined && allowed.length === 0) return picklistUnresolved(); if (allowed.length > 0 && !allowed.includes(String(value))) { @@ -1420,6 +1517,7 @@ function validateOne( if (!Array.isArray(value)) { return fail('invalid_type', undefined, 'invalid_type_array'); } + if (shapeOnly) return null; // Reference / attachment types carry IDs or storage keys, not options — // reference integrity is handled elsewhere. if (t === 'lookup' || t === 'user' || t === 'file' || t === 'image') return null; @@ -1633,10 +1731,20 @@ export function valueShapeViolation(def: FieldDef, value: unknown): string | nul /** * Is this field one the value-shape scan covers, and one a client may write? - * `system` / `readonly` / lifecycle columns are skipped for the same reason - * `validateRecord` skips them — the engine owns them, so they are never - * validated on a write and must never be counted as blocking a gate that - * governs writes. + * `system` / `readonly` / lifecycle columns are skipped: the engine owns them, + * so a client never writes them and they must never be counted as blocking a + * gate that governs client writes. + * + * [#21663] That is no longer the same thing as "never validated on a write". + * A `readonly` reference / structured-JSON value IS judged on every write now + * ({@link ReadonlyValueScope}) — under the posture its object resolves, and + * this predicate is also what the engine's dormancy test counts, so an object + * whose only covered fields are readonly stays warn-first for them: the value + * is admitted, logged and reported to `onAdmittedValueShapeViolation`, never + * stored silently. ⛔ Widening this predicate to readonly columns is NOT the + * fix for that: every object carries injected readonly lookups (`created_by`, + * `updated_by`, `organization_id`), so it would make every object non-dormant + * — the defect the dormancy test was written to close. */ export function isScannableValueShapeField(name: string, def: FieldDef | undefined): boolean { if (!def || SKIP_FIELDS.has(name) || def.system || def.readonly) return false; @@ -1761,12 +1869,39 @@ export interface ValidateRecordOptions { * `fields` map of `{ [fieldName]: FieldDef }`. * * Returns void on success; throws `ValidationError` on failure. + * + * A `readonly` field is not reached here, exactly as before #21663: this + * public helper has no readonly strip to stand before or after, so it cannot + * say whether a readonly value on `data` is one a write would store. The + * engine's write path asks {@link validateRecordInScope}, which can. */ export function validateRecord( objectSchema: { fields?: Record } | undefined | null, data: Record | undefined | null, mode: Mode, options: ValidateRecordOptions = {}, +): void { + validateRecordInScope(objectSchema, data, mode, 'skip', options); +} + +/** + * [#21663] {@link validateRecord}, told where its payload stands relative to + * the readonly strip — see {@link ReadonlyValueScope} for the three scopes, + * the arms a readonly value reaches, and why it is never one of its + * constraints. + * + * Module-internal on purpose: re-exported from neither package entry, so the + * published `validateRecord` signature is unchanged. The scope is a fact only + * the engine's write path knows. ⛔ It passes `'include'` / `'only'` only + * where the payload is FINAL; a readonly value judged ahead of the strip may + * be one the strip is about to drop. + */ +export function validateRecordInScope( + objectSchema: { fields?: Record } | undefined | null, + data: Record | undefined | null, + mode: Mode, + scope: ReadonlyValueScope, + options: ValidateRecordOptions = {}, ): void { if (!objectSchema?.fields || !data) return; @@ -1781,18 +1916,26 @@ export function validateRecord( // Walk all declared fields — required check applies even when // the caller didn't supply the field at all. for (const [name, def] of Object.entries(fields)) { - if (SKIP_FIELDS.has(name)) continue; - if (def.system || def.readonly) continue; - const err = validateOne(name, def, data[name], false, mediaStrict, messages, valueStrict, onAdmitted); + if (!isInReadonlyScope(name, def, scope)) continue; + // [#21663] A readonly value: its SHAPE, never required (the engine owns + // its presence) and never a constraint — see ReadonlyValueScope. + const shapeOnly = def.readonly === true; + const err = validateOne(name, def, data[name], shapeOnly, mediaStrict, messages, valueStrict, onAdmitted, shapeOnly); if (err) errors.push(err); } } else { // Update — validate only supplied fields; an OMITTED field never 400s. for (const [name, value] of Object.entries(data)) { - if (SKIP_FIELDS.has(name)) continue; const def = fields[name]; if (!def) continue; - if (def.system || def.readonly) continue; + if (!isInReadonlyScope(name, def, scope)) continue; + if (def.readonly === true) { + // [#21663] Same as the insert walk: shape only, so no `required_cleared` + // either — clearing a readonly column is the engine's business. + const err = validateOne(name, def, value, true, mediaStrict, messages, valueStrict, onAdmitted, true); + if (err) errors.push(err); + continue; + } // ADR-0113 non-regression: a PATCH may not null OUT a required field. // The key is in the payload (we are iterating it), so a missing value // is an explicit clear, not an omission — the write would take the diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 1124a069a2a..41ed77fc129 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -1818,7 +1818,7 @@ export function isRuntimeOwnedField(def: { type?: string } | undefined | null): * Strip CALLER-SUPPLIED writes to read-only fields from an UPDATE payload * (#2948). Unlike `readonlyWhen` (conditional, handled above), a * static `readonly` field was never enforced on the server write path: the - * record validator only SKIPS it from validation, so a user-context update + * record validator only SKIPPED it from validation, so a user-context update * could overwrite audit stamps, provenance, or any other read-only column. We * STRIP the change (symmetric with `readonlyWhen`) rather than reject it, for * compatibility. @@ -1845,6 +1845,11 @@ export function isRuntimeOwnedField(def: { type?: string } | undefined | null): * - system context — the caller passes this strip only for NON-system writes; * system-context writes (import, seed replay, approvals, lifecycle hooks — * all `isSystem: true`) legitimately set read-only columns and skip it. + * [#21663] They skip THIS strip and nothing else: the value they keep is + * stored, so the engine judges its SHAPE after the strip point on every + * path (`validateRecordInScope` in `record-validator.ts`, `ReadonlyValueScope`). A + * malformed readonly value from a system writer is refused with the same + * sentence a non-readonly field gets, never stored. * * ### Why `supplied` carries VALUES, not just keys (#5591) *