diff --git a/.changeset/readonly-static-insert-strip.md b/.changeset/readonly-static-insert-strip.md new file mode 100644 index 0000000000..fdd40f79e7 --- /dev/null +++ b/.changeset/readonly-static-insert-strip.md @@ -0,0 +1,49 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/spec": patch +--- + +fix(metadata-protocol): strip static `readonly` on INSERT at the data-write ingress (#3043) + +#2948/#3003 made static `readonly: true` fields server-enforced on UPDATE (a +non-system PATCH forging `approval_status: 'approved'` is silently stripped in +the engine), but INSERT was exempt. For approval/status/verdict columns that +exemption was the *shorter* attack: instead of the #3003 draft-then-PATCH move, a +non-system caller could `POST` a record already `approval_status: 'approved'` in +one step — and the UPDATE-only strip never reached it. + +The strip now also runs on INSERT, but at the **external data-write ingress** +(`DataProtocol.createData` / `createManyData` / `batchData` / `cloneData`) rather +than in the engine. That seam is the single point every external programmatic +create funnels through — the REST CRUD route, the GraphQL/MCP dispatcher +(`bridge.create` → `callData` → `createData`), and bulk import — while **trusted +internal writers** (better-auth's adapter, the metadata repository, the seed +loader) call `engine.insert` directly and bypass it. Enforcing at the ingress +protects every caller/agent path at once without stripping the internal writers +that legitimately seed read-only columns on create (identity provisioning, +provenance stamps, event-log cursors) — the blast radius an engine-level insert +strip would have. + +- **Caller-forged only, at the ingress.** The payload here is raw caller input + (the security middleware stamps `owner_id` / `organization_id` later, inside + `engine.insert`), so only keys the caller actually sent are dropped; server + stamps are added afterwards and are unaffected. +- **Re-derives the default.** A stripped field falls back to its declared + `defaultValue` in the engine (a forged `approval_status` becomes `draft`, not + NULL). +- **System-context exempt.** `isSystem` writes still seed read-only columns. +- **Silent** (HTTP 2xx), per-row on batch/import. `readonlyWhen` stays + INSERT-exempt (a conditional lock needs a prior record). +- **Author-defined business objects only.** Platform objects (`managedBy` set, + or the `sys_` namespace) carry their own field-write governance that a silent + strip must not pre-empt — e.g. ADR-0086 REJECTS (403) a forged + `managed_by:'package'` on `sys_permission_set`, and #3004 rejects a forged + `owner_id`; several of those columns are `readonly`, so stripping them here + would swallow the payload the guard is meant to reject. The #3043 threat is app + approval/status fields, never `sys_` — the same boundary `applySystemFields` + uses for ownership. + +Behavior change: a non-system create through the data API (REST / GraphQL / MCP / +import) can no longer seed a `readonly` column from the payload. Flows that +legitimately write read-only columns at creation must run with a system context +(`isSystem`), the same requirement the UPDATE strip already imposes. diff --git a/content/docs/references/data/field.mdx b/content/docs/references/data/field.mdx index 3253e89243..789d9b7eb4 100644 --- a/content/docs/references/data/field.mdx +++ b/content/docs/references/data/field.mdx @@ -150,7 +150,7 @@ const result = Address.parse(data); | **conditionalRequired** | `string \| { dialect: Enum<'cel' \| 'js' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is required when TRUE. Alias of `requiredWhen`. | | **widget** | `string` | optional | Form widget override — names a registered field component (resolved as `field:`) to render this field instead of the `type` default. Degrades to the `type` renderer when unregistered. e.g. "object-ref", "filter-condition", "recipient-picker". | | **hidden** | `boolean` | optional | Hidden from default UI | -| **readonly** | `boolean` | optional | Read-only — never editable in forms, AND server-enforced on UPDATE: a non-system write to this field is silently dropped from the payload (#2948/#3003; symmetric with `readonlyWhen`). INSERT may still seed it (defaultValue, import). | +| **readonly** | `boolean` | optional | Read-only — never editable in forms, AND server-enforced on BOTH write paths: a non-system write to this field is silently dropped from the payload on UPDATE (#2948/#3003) and on INSERT (#3043; a create can no longer directly seed e.g. `approval_status: "approved"`), symmetric with `readonlyWhen`. A stripped INSERT field still falls back to its `defaultValue`; system-context writes (import, seed replay, migration) are exempt. | | **requiredPermissions** | `string[]` | optional | [ADR-0066 D3] Capabilities required to read/edit this field (mask on read, deny on write; AND-gate). | | **system** | `boolean` | optional | Auto-injected system/audit field (e.g. created_at, updated_by, organization_id). Tools that surface system fields separately from author-declared business fields should branch on this flag. | | **sortable** | `boolean` | optional | Whether field is sortable in list views | diff --git a/packages/metadata-protocol/src/protocol.readonly-insert.test.ts b/packages/metadata-protocol/src/protocol.readonly-insert.test.ts new file mode 100644 index 0000000000..6e05540c3a --- /dev/null +++ b/packages/metadata-protocol/src/protocol.readonly-insert.test.ts @@ -0,0 +1,117 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #3043 — static `readonly: true` fields must not be SEEDABLE via a non-system +// create through the external data API. The strip lives at the DataProtocol +// ingress (createData / createManyData / batchData / cloneData) — the seam every +// external REST/GraphQL/MCP create funnels through — while trusted internal +// writers call engine.insert directly and are unaffected. It runs BEFORE +// engine.insert, so a stripped field falls back to its defaultValue (re-derived +// by the engine, which the mock stands in for). System context is exempt. + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +const SCHEMA = { + name: 'approval_case', + fields: { + title: { name: 'title', type: 'text' }, + // readonly approval column — the #3003 attack target + approval_status: { name: 'approval_status', type: 'text', readonly: true, defaultValue: 'draft' }, + // readonly provenance stamp with no default + source: { name: 'source', type: 'text', readonly: true }, + }, +}; + +function makeProtocol() { + const inserts: Array<{ object: string; data: any; options: any }> = []; + const engine = { + registry: { getObject: (n: string) => (n === 'approval_case' ? SCHEMA : undefined) }, + insert: vi.fn(async (object: string, data: any, options?: any) => { + inserts.push({ object, data, options }); + const rows = Array.isArray(data) ? data : [data]; + const out = rows.map((r, i) => ({ id: `rec-${i + 1}`, ...r })); + return Array.isArray(data) ? out : out[0]; + }), + }; + const p = new ObjectStackProtocolImplementation(engine as any); + return { p, engine, inserts }; +} + +describe('createData — static readonly INSERT strip (#3043)', () => { + it('drops a non-system caller forging a readonly field; editable sibling lands', async () => { + const { p, inserts } = makeProtocol(); + await p.createData({ + object: 'approval_case', + data: { title: 'Case A', approval_status: 'approved' }, + context: { userId: 'u1' }, + }); + expect(inserts).toHaveLength(1); + expect(inserts[0].data).toEqual({ title: 'Case A' }); // approval_status stripped + expect(inserts[0].data).not.toHaveProperty('approval_status'); + }); + + it('ALLOWS a system-context caller to seed the readonly field', async () => { + const { p, inserts } = makeProtocol(); + await p.createData({ + object: 'approval_case', + data: { title: 'Seed', approval_status: 'approved' }, + context: { isSystem: true }, + }); + expect(inserts[0].data.approval_status).toBe('approved'); + }); + + it('strips a forged readonly field even when no context is supplied (non-system default)', async () => { + const { p, inserts } = makeProtocol(); + await p.createData({ object: 'approval_case', data: { title: 'X', source: 'attacker' } }); + expect(inserts[0].data).not.toHaveProperty('source'); + }); + + it('does NOT strip a PLATFORM object — defers to its own field guards (ADR-0086 / #3004)', async () => { + // A `sys_`/managedBy object carries dedicated write governance (e.g. the + // ADR-0086 provenance guard REJECTS a forged managed_by/package_id with 403); + // the generic silent strip must not pre-empt that. Proven with both markers. + const platformSchema = { + name: 'sys_permission_set', + fields: { managed_by: { name: 'managed_by', type: 'select', readonly: true } }, + }; + const managedSchema = { + name: 'crm_thing', managedBy: 'package', + fields: { locked: { name: 'locked', type: 'text', readonly: true } }, + }; + const inserts: any[] = []; + const engine = { + registry: { getObject: (n: string) => (n === 'sys_permission_set' ? platformSchema : managedSchema) }, + insert: vi.fn(async (object: string, data: any) => { inserts.push({ object, data }); return { id: 'x', ...data }; }), + }; + const p = new ObjectStackProtocolImplementation(engine as any); + await p.createData({ object: 'sys_permission_set', data: { managed_by: 'package' }, context: { userId: 'u1' } }); + await p.createData({ object: 'crm_thing', data: { locked: 'forged' }, context: { userId: 'u1' } }); + expect(inserts[0].data.managed_by, 'sys_ object: readonly field passed through to its guard').toBe('package'); + expect(inserts[1].data.locked, 'managedBy object: readonly field passed through to its guard').toBe('forged'); + }); +}); + +describe('createManyData / batchData — per-row readonly INSERT strip (#3043)', () => { + it('createManyData strips the forged readonly column on every row', async () => { + const { p, inserts } = makeProtocol(); + await p.createManyData({ + object: 'approval_case', + records: [ + { title: 'A', approval_status: 'approved' }, + { title: 'B', approval_status: 'approved' }, + ], + context: { userId: 'u1' }, + }); + expect(inserts[0].data).toEqual([{ title: 'A' }, { title: 'B' }]); + }); + + it('batchData create strips the forged readonly column', async () => { + const { p, inserts } = makeProtocol(); + await p.batchData({ + object: 'approval_case', + request: { operation: 'create', records: [{ data: { title: 'A', approval_status: 'approved' } }] } as any, + }); + expect(inserts).toHaveLength(1); + expect(inserts[0].data).toEqual({ title: 'A' }); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 5a83e8f5ba..5466f80cde 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -446,6 +446,61 @@ const CLONE_STRIP_FIELDS: readonly string[] = [ 'id', 'created_at', 'created_by', 'updated_at', 'updated_by', ]; +/** + * [#3043] Drop caller-supplied writes to statically `readonly: true` fields from + * an INSERT payload, at the external DATA-WRITE INGRESS. + * + * #2948/#3003 made static `readonly` server-enforced on UPDATE (the engine strips + * a non-system caller's write). INSERT was left exempt — but for approval/status + * columns that exemption is the SHORTER attack: instead of the #3003 + * draft-then-PATCH move, a non-system caller can POST a record already + * `approval_status: 'approved'` in one step. This closes it symmetrically, but at + * the INGRESS rather than in the engine: every EXTERNAL programmatic create — the + * REST CRUD route, the GraphQL/MCP dispatcher (`bridge.create` → `callData` → + * here), and bulk import — lands in the DataProtocol, while TRUSTED internal + * writers (better-auth's adapter, the metadata repository, the seed loader) call + * `engine.insert` DIRECTLY and never pass through here. Keeping the strip at the + * ingress therefore protects every agent/caller path at once WITHOUT stripping + * the internal writers that legitimately seed read-only columns on create + * (identity provisioning, provenance stamps, event-log cursors) — the blast + * radius an engine-level insert strip would have. + * + * Silent by contract (like the UPDATE / `readonlyWhen` strips): the forged key is + * dropped, the create still succeeds, and the engine re-derives the field's + * `defaultValue` (a forged `approval_status` becomes `draft`, the enforced + * initial state, not NULL). `isSystem` writes are exempt. `readonlyWhen` stays + * INSERT-exempt (a conditional lock needs a prior record, which a create lacks). + * Handles a single record or a batch array. + * + * SCOPE — author-defined business objects only. PLATFORM objects (`managedBy` + * set, or the reserved `sys_` namespace) carry their OWN field-write governance + * that a silent strip must not pre-empt: e.g. ADR-0086 REJECTS (403) a forged + * `managed_by:'package'` / `package_id` on `sys_permission_set`, and #3004 + * rejects a forged `owner_id` anchor — several of those columns are `readonly`, + * so stripping them here would silently swallow the payload the guard is meant to + * reject. The #3043 threat is app approval/status/verdict fields (the issue's + * `sporadic_application` / `assessment`), never `sys_`; this is the same + * platform-vs-authored boundary `applySystemFields` uses for ownership. + */ +function stripReadonlyForInsert(schema: any, data: any, context: any): any { + if (context?.isSystem) return data; + if (!schema || schema.managedBy || String(schema.name ?? '').startsWith('sys_')) return data; + const fields = schema?.fields; + if (!fields || data == null) return data; + const stripRow = (row: any): any => { + if (row == null || typeof row !== 'object') return row; + let out = row; + for (const name of Object.keys(fields)) { + if (!fields[name]?.readonly) continue; + if (!(name in out)) continue; + if (out === row) out = { ...row }; + delete out[name]; + } + return out; + }; + return Array.isArray(data) ? data.map(stripRow) : stripRow(data); +} + /** * Service Configuration for Discovery * Maps service names to their routes and plugin providers. @@ -2685,9 +2740,16 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { } async createData(request: { object: string, data: any, context?: any }) { + // [#3043] Ingress-level static-`readonly` strip — a non-system caller + // cannot seed a read-only column (e.g. `approval_status`) on create. + const data = stripReadonlyForInsert( + this.engine.registry?.getObject(request.object), + request.data, + request.context, + ); const result = await this.engine.insert( request.object, - request.data, + data, request.context !== undefined ? { context: request.context } as any : undefined, ); return { @@ -2765,7 +2827,13 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { Object.assign(data, request.overrides); } - const result = await this.engine.insert(request.object, data, ctxOpt as any); + // [#3043] A clone is a create: a non-system caller must not carry over (or + // override in) a read-only column — copying the source's `approval_status` + // or forging one via `overrides` would mint an approved record. Strip them + // so the insert re-derives their `defaultValue`, symmetric with createData. + const insertData = stripReadonlyForInsert(schema, data, ctx); + + const result = await this.engine.insert(request.object, insertData, ctxOpt as any); return { object: request.object, id: result.id, @@ -3101,11 +3169,15 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { let succeeded = 0; let failed = 0; + // [#3043] The batch endpoint is an external ingress and threads no + // context, so its creates are non-system: strip forged read-only columns. + const batchSchema = this.engine.registry?.getObject(object); + for (const record of records) { try { switch (operation) { case 'create': { - const created = await this.engine.insert(object, record.data || record); + const created = await this.engine.insert(object, stripReadonlyForInsert(batchSchema, record.data || record, undefined)); results.push({ id: created.id, success: true, record: created }); succeeded++; break; @@ -3175,9 +3247,16 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { } async createManyData(request: { object: string, records: any[], context?: any }): Promise { + // [#3043] Ingress-level static-`readonly` strip (per row) — mirrors + // createData for the bulk-create / import surface. + const rows = stripReadonlyForInsert( + this.engine.registry?.getObject(request.object), + request.records, + request.context, + ); const records = await this.engine.insert( request.object, - request.records, + rows, request.context !== undefined ? { context: request.context } as any : undefined, ); return { diff --git a/packages/qa/dogfood/test/authz-conformance.matrix.ts b/packages/qa/dogfood/test/authz-conformance.matrix.ts index 12add1f56e..31ba832c9a 100644 --- a/packages/qa/dogfood/test/authz-conformance.matrix.ts +++ b/packages/qa/dogfood/test/authz-conformance.matrix.ts @@ -91,10 +91,10 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ note: 'Surface posture: system (trusted-implicit), pre-wiring — no end-user transport exists (handleUpgrade unimplemented, no REST subscribe route, client RealtimeAPI is a placeholder); the only subscribers are server-internal plugins (webhook auto-enqueuer, knowledge sync). Structural defect: Subscription carries no principal, matchesSubscription filters only by object+eventTypes (RealtimeSubscriptionOptions.filter is declared but never read), and the engine publishes the FULL after-row — so any future external subscriber would receive record bodies cross-tenant that its own find would hide. ADMISSION REQUIREMENT before any WebSocket/SSE/subscribe transport ships: per-recipient RLS/FLS/tenant re-check on delivery (subscription carries the subscriber ExecutionContext) OR id-only payload + client re-fetch. The transport tripwire probes in authz-conformance.test.ts turn a wired transport into an UNCLASSIFIED surface → red CI until this row is upgraded with the enforcement site.' }, { id: 'default-profile', summary: 'app-declared default profile (isDefault)', state: 'enforced', enforcement: 'plugin-security/security-plugin.ts fallback resolution', proof: 'showcase-default-profile.dogfood.test.ts' }, - { id: 'readonly-static-write', summary: 'static `readonly: true` stripped from non-system UPDATE payloads (#2948 / #3003 — a direct PATCH cannot forge approval/status/amount columns the UI never renders)', state: 'enforced', - enforcement: 'objectql/engine.ts update — stripReadonlyFields on both the single-id and multi-row paths (caller-supplied keys only, so audit-hook/middleware server stamps survive; isSystem exempt; symmetric with the readonlyWhen strip)', + { id: 'readonly-static-write', summary: 'static `readonly: true` stripped from non-system UPDATE (#2948 / #3003) AND INSERT (#3043) payloads — neither a direct PATCH nor a direct POST can forge approval/status/amount columns the UI never renders', state: 'enforced', + enforcement: 'UPDATE: objectql/engine.ts stripReadonlyFields on the single-id + multi-row paths (#2948, caller-supplied keys only so server stamps survive). INSERT: metadata-protocol/protocol.ts strips read-only keys at the DataProtocol create INGRESS (createData / createManyData / batchData / cloneData) — the single seam every external REST/GraphQL/MCP create funnels through, while trusted internal engine.insert writers (better-auth adapter, metadata repo, seed loader) bypass it; stripped before the engine so the field re-derives its defaultValue. isSystem exempt on both; symmetric with the readonlyWhen strip', proof: 'showcase-static-readonly.dogfood.test.ts', - note: 'The #3003 field report: `readonly: true` used to be UI-only, so a logged-in non-admin self-approved a 4-stage approval (approval_status/approval_stage/confirmed_total) with one same-session REST PATCH on a draft record — RECORD_LOCKED only guards pending flows, and the draft never entered one. The strip is SILENT (HTTP 200, persisted value kept — reject-vs-strip decided in #2948 for readonlyWhen symmetry). INSERT is deliberately exempt (create may seed a readonly column: defaultValue, import, migration), also symmetric with readonlyWhen. Engine-level unit/integration proof in objectql/plugin.integration.test.ts (#2948 suite: forge stripped, server stamp survives, system context allowed).' }, + note: 'The #3003 field report: `readonly: true` used to be UI-only, so a logged-in non-admin self-approved a 4-stage approval (approval_status/approval_stage/confirmed_total) with one same-session REST PATCH on a draft record — RECORD_LOCKED only guards pending flows, and the draft never entered one. #3043 is the INSERT face: the same non-admin could skip the draft entirely and POST a record already `approval_status:"approved"` — a step SHORTER than #3003, and one the UPDATE strip never reached. Enforced at the DATA-WRITE INGRESS (not the engine) so it covers every external caller — REST, the GraphQL/MCP dispatcher, bulk import — without stripping the internal writers that legitimately seed readonly columns on create (identity provisioning, provenance, event-log cursors). The strip is SILENT on both paths (HTTP 2xx, forged value dropped; a stripped INSERT field falls back to its defaultValue). `readonlyWhen` stays INSERT-exempt (a conditional lock needs a prior record). System-context writes (import, seed replay, migration) still seed readonly columns. Ingress unit proof in metadata-protocol protocol.readonly-insert.test.ts (forge stripped, default re-seeded, system context allowed, batch rows covered, internal engine.insert unaffected).' }, // ── ADR-0057 — ERP authorization core (enforced + e2e proven) ────────── { id: 'scope-depth', summary: 'permission-grant access DEPTH (own/own_and_reports/unit/unit_and_below/org)', state: 'enforced', diff --git a/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts b/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts index 17f05fd442..812ed9451d 100644 --- a/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts @@ -2,17 +2,25 @@ // // @proof: readonly-static-write // -// #2948 / #3003 — static `readonly: true` is SERVER-enforced on UPDATE, not a -// UI-only affordance. The #3003 field report: an approval-flow object declared -// `approval_status` / `approval_stage` as `readonly: true`, the create/edit -// forms never rendered them — and a logged-in, non-admin user forged all of -// them (plus an amount column) with one direct REST PATCH from the same -// session, self-approving a 4-stage approval. The strip added for #2948 -// (`stripReadonlyFields`, objectql/engine.ts) closes exactly that: on a -// non-system UPDATE, caller-supplied writes to statically-readonly fields are -// silently dropped (HTTP 200, persisted value kept) — symmetric with the -// `readonlyWhen` strip. INSERT is deliberately exempt (a create may seed a -// readonly column: defaultValue, import, migration), matching `readonlyWhen`. +// #2948 / #3003 (UPDATE) + #3043 (INSERT) — static `readonly: true` is +// SERVER-enforced on BOTH write paths, not a UI-only affordance. The #3003 +// field report: an approval-flow object declared `approval_status` / +// `approval_stage` as `readonly: true`, the create/edit forms never rendered +// them — and a logged-in, non-admin user forged all of them (plus an amount +// column) with one direct REST PATCH from the same session, self-approving a +// 4-stage approval. #3043 is the INSERT face of the same gap: the create path +// used to be EXEMPT, so the same non-admin could skip the draft entirely and +// POST a record already `approval_status:'approved'` — a step SHORTER than +// #3003, and one the UPDATE strip never reached. It is now closed on both +// paths: UPDATE in the engine (`stripReadonlyFields`, objectql/engine.ts, +// #2948) and INSERT at the DataProtocol create INGRESS +// (metadata-protocol/protocol.ts `stripReadonlyForInsert`, #3043 — the single +// seam every external REST/GraphQL/MCP create funnels through, while trusted +// internal engine.insert writers are untouched). On a non-system INSERT or +// UPDATE, caller-supplied writes to statically-readonly fields are silently +// dropped (HTTP 2xx; a stripped INSERT field falls back to its `defaultValue`). +// System-context writes (import, seed replay, migration) stay exempt, as does +// `readonlyWhen` on INSERT (a conditional lock needs a prior record). // // Proven here on the REAL showcase app over HTTP: `showcase_contact.lead_score` // is the stand-in for the #3003 approval/status/amount columns — readonly, @@ -26,7 +34,7 @@ const OBJ = '/data/showcase_contact'; const idOf = (b: any) => b?.id ?? b?.record?.id ?? b?.data?.id ?? b?.recordId; const recordOf = (b: any) => b?.record ?? b?.data ?? b; -describe('showcase: static readonly write enforcement (#2948 / #3003)', () => { +describe('showcase: static readonly write enforcement (#2948 / #3003 / #3043)', () => { let stack: VerifyStack; let token: string; let contactId: string; @@ -36,25 +44,30 @@ describe('showcase: static readonly write enforcement (#2948 / #3003)', () => { await stack.signIn(); token = await stack.signUp('ro-worker@verify.test'); - // INSERT exemption (documented contract, symmetric with `readonlyWhen`): - // a create MAY seed a readonly column — the scoring pipeline, an import, - // or a migration legitimately writes the initial value. + // #3043: a non-system INSERT that forges the readonly column is admitted + // (HTTP 2xx, silent — like the UPDATE strip) but the forged value must NOT + // persist. The create itself still succeeds — only the readonly key is + // dropped from the payload, so the editable fields land. const created = await stack.apiAs(token, 'POST', OBJ, { name: 'Readonly Probe', email: 'ro-probe@verify.test', - lead_score: 10, + lead_score: 10, // forged: the UI never renders this on the create form }); - expect(created.status).toBeLessThan(300); + expect(created.status, 'create succeeds; the readonly key is dropped, not rejected').toBeLessThan(300); contactId = idOf(await created.json()); expect(contactId).toBeTruthy(); }, 60_000); afterAll(async () => { await stack?.stop(); }); - it('INSERT may seed the readonly field (documented exemption)', async () => { + it('INSERT forging the readonly field is silently stripped — the forged value never persists (#3043)', async () => { const res = await stack.apiAs(token, 'GET', `${OBJ}/${contactId}`); expect(res.status).toBe(200); - expect(recordOf(await res.json()).lead_score, 'insert-seeded value persisted').toBe(10); + const rec = recordOf(await res.json()); + expect(rec.name, 'editable field from the create payload landed').toBe('Readonly Probe'); + // No `defaultValue` is declared for `lead_score`, so the stripped field is + // simply absent — the caller-forged 10 is gone. + expect(rec.lead_score ?? null, 'insert-forged readonly value must NOT persist').toBeNull(); }); it('a direct PATCH forging the readonly field is silently stripped — sibling editable fields still land', async () => { @@ -68,7 +81,7 @@ describe('showcase: static readonly write enforcement (#2948 / #3003)', () => { expect(forge.status, 'strip is silent — the request succeeds').toBe(200); const after = recordOf(await (await stack.apiAs(token, 'GET', `${OBJ}/${contactId}`)).json()); - expect(after.lead_score, 'forged readonly value must NOT persist').toBe(10); + expect(after.lead_score ?? null, 'forged readonly value must NOT persist').toBeNull(); expect(after.notes, 'editable field from the same payload still lands').toBe( 'legitimate edit in the same payload', ); @@ -79,6 +92,6 @@ describe('showcase: static readonly write enforcement (#2948 / #3003)', () => { expect(forge.status).toBe(200); const after = recordOf(await (await stack.apiAs(token, 'GET', `${OBJ}/${contactId}`)).json()); - expect(after.lead_score, 'readonly value survives an all-forged payload').toBe(10); + expect(after.lead_score ?? null, 'readonly value unchanged by an all-forged payload').toBeNull(); }); }); diff --git a/packages/spec/liveness/field.json b/packages/spec/liveness/field.json index 1bf7dd678f..244a2fa92c 100644 --- a/packages/spec/liveness/field.json +++ b/packages/spec/liveness/field.json @@ -87,9 +87,9 @@ }, "readonly": { "status": "live", - "evidence": "packages/objectql/src/validation/rule-validator.ts", + "evidence": "packages/objectql/src/validation/rule-validator.ts (UPDATE strip); packages/metadata-protocol/src/protocol.ts (INSERT ingress strip)", "proof": "packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts#readonly-static-write", - "note": "renderer + server write path: stripReadonlyFields drops non-system UPDATE writes to the field (#2948/#3003 — was renderer-only, i.e. false compliance for approval/status columns); INSERT exempt, symmetric with readonlyWhen." + "note": "renderer + server write path: a non-system write to the field is silently dropped on BOTH UPDATE (#2948/#3003 — engine stripReadonlyFields; was renderer-only, i.e. false compliance for approval/status columns) and INSERT (#3043 — stripped at the DataProtocol createData/import ingress so external REST/GraphQL/MCP creates can't seed approval_status:'approved' a step shorter than #3003, while trusted internal engine writers that legitimately seed readonly columns are unaffected); a stripped INSERT field falls back to its defaultValue; symmetric with readonlyWhen (which stays INSERT-exempt, needing a prior record)." }, "hidden": { "status": "live", diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index b2b379beab..8433bdd312 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -613,7 +613,7 @@ export const FieldSchema = lazySchema(() => z.object({ /** Security & Visibility */ hidden: z.boolean().default(false).describe('Hidden from default UI'), - readonly: z.boolean().default(false).describe('Read-only — never editable in forms, AND server-enforced on UPDATE: a non-system write to this field is silently dropped from the payload (#2948/#3003; symmetric with `readonlyWhen`). INSERT may still seed it (defaultValue, import).'), + readonly: z.boolean().default(false).describe('Read-only — never editable in forms, AND server-enforced on BOTH write paths: a non-system write to this field is silently dropped from the payload on UPDATE (#2948/#3003) and on INSERT (#3043; a create can no longer directly seed e.g. `approval_status: "approved"`), symmetric with `readonlyWhen`. A stripped INSERT field still falls back to its `defaultValue`; system-context writes (import, seed replay, migration) are exempt.'), /** * [ADR-0066 D3] Capabilities required to READ/EDIT this field. A field