diff --git a/.changeset/meta-read-audit-field-governance.md b/.changeset/meta-read-audit-field-governance.md new file mode 100644 index 0000000000..e1df029a14 --- /dev/null +++ b/.changeset/meta-read-audit-field-governance.md @@ -0,0 +1,63 @@ +--- +"@objectstack/metadata-core": patch +"@objectstack/metadata-protocol": patch +"@objectstack/objectql": patch +--- + +fix(meta): `/meta` object reads stop reporting `readonly: false` on fields the write path refuses (#4513) + +`#4447` made the audit-provenance family (`created_at`, `created_by`, +`updated_at`, `updated_by`) engine-owned on the **write** path: the registry's +`applySystemFields` forces `{ readonly: true, system: true }` over a *declared* +audit field, and `ObjectQL.update` strips a non-system caller's write to it. + +The **read** path never learned it. A `/meta` object read resolves through +`sys_metadata` overlay → MetadataService → SchemaRegistry, and only the last of +those three has been through `applySystemFields` — so an object whose built +artifact ships a materialized `created_at` carrying FieldSchema defaults +(`readonly: false`) reported that value to every client while writes to that +same field were being refused. Measured before the fix, all of the read exits +agreed with each other and disagreed with the engine: + +``` +single read: {"type":"datetime","label":"Created At","readonly":false} +list read: {"type":"datetime","label":"Created At","readonly":false} +cached read: {"type":"datetime","label":"Created At","readonly":false} +layered read: {"type":"datetime","label":"Created At","readonly":false} +``` + +One field, two answers — and the machine-readable one, the only face a client +or an AI author writing code off `/meta` can see, was the wrong one. + +**What changes.** Every `/meta` object read exit now reports the audit family +the way the engine enforces it. That covers the single-item read (both the +singular and plural type spelling), the list read, the cached/ETag branch, the +`?preview=draft` and `?state=draft` reads, and the layered read's `effective` +layer. `GET` bodies for objects that declare an audit field will show +`readonly: true, system: true` where they previously showed `readonly: false` +or omitted the keys; nothing else about the document changes, and the ETag for +such an object changes once. + +**What deliberately does not change.** + +- The layered read's `code` and `overlay` layers stay raw — showing the + package's declaration beside the governed `effective` value is the + diagnostic's whole point. +- `sys_metadata` still stores exactly what the author saved; the correction is + applied on the way out, so no phantom customization appears in the diff. +- An object that opts out of the audit family (`systemFields: false`, + `systemFields.audit: false`, `managedBy: 'better-auth'`) is untouched — the + engine enforces nothing there, so a read that claimed otherwise would be the + same lie pointing the other way. +- Only `readonly` and `system` are forced. Every other key an author writes — + `label`, `description`, `hidden`, `group`, and `type` for an external object + mapping a differently-typed remote column — stays theirs. + +The governance table moved from `packages/objectql/src/registry.ts` to +`@objectstack/metadata-core` (`AUDIT_FIELD_GOVERNANCE`, plus the +`applyAuditFieldGovernance` normalizer the read path applies), by the same +criterion and for the same cycle as the `#5619` engine-dispatch predicates: +`@objectstack/objectql` depends on `@objectstack/metadata-protocol`, so the +read path cannot import the table from the registry that enforces it, and a +second copy would agree only until someone edited one side. `objectql` +re-exports the symbol from its original path, so its public API is unchanged. diff --git a/packages/metadata-core/src/audit-field-governance.ts b/packages/metadata-core/src/audit-field-governance.ts new file mode 100644 index 0000000000..6b58641d13 --- /dev/null +++ b/packages/metadata-core/src/audit-field-governance.ts @@ -0,0 +1,148 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The **one** answer to "which keys on an object's audit-provenance columns are + * platform-owned rather than authorable?" — and the normalizer that applies + * that answer to a metadata document (objectstack#4513, from objectstack#4447). + * + * ## What #4447 established, and the half it left open + * + * `applySystemFields` (`@objectstack/objectql`) injects the + * {@link AUDIT_PROVENANCE_FIELDS} family and, since #4447, **forces** + * `readonly: true` / `system: true` over a *declared* audit field as well: + * `fields: { ...additions, ...schema.fields, ...overrides }`. That is what the + * write path enforces — `ObjectQL.update` strips a non-system caller's write to + * a statically-`readonly` field off the registry's post-injection schema, so a + * forged `created_at` is refused whatever the author declared. + * + * The **read** path never learned it. `GET /api/v1/meta/objects/:name` answers + * from `sys_metadata` (the stored overlay / build-artifact body) first and + * consults the registry only as a fallback, so an object whose artifact ships a + * materialized `created_at` carrying FieldSchema DEFAULTS (`readonly: false`) + * reported `readonly: false` to every client while writes to that same field + * were being refused. Measured on `origin/main` before this module existed, all + * four protocol read exits agreed with each other and disagreed with the + * engine: + * + * ``` + * single read: {"type":"datetime","label":"Created At","readonly":false} + * list read: {"type":"datetime","label":"Created At","readonly":false} + * cached read: {"type":"datetime","label":"Created At","readonly":false} + * layered read: {"type":"datetime","label":"Created At","readonly":false} + * ``` + * + * One field, two answers, and the machine-readable one was the wrong one — the + * face that clients, and AI authors writing code against `/meta`, are the only + * ones able to see. + * + * ## Why this module lives in `@objectstack/metadata-core` + * + * The same criterion `engine-delete-dispatch.ts` records, for the same cycle: + * `@objectstack/objectql` **depends on** `@objectstack/metadata-protocol`, so + * the read path cannot import the governance table from the registry that owns + * it. When a reverse import is impossible, the only honest way out is to sink + * the contract into a package **both sides already depend on** — and this + * package's own dependencies are `{ @objectstack/spec, zod }`, so there is no + * new edge and no new cycle. + * + * The alternative — a second governance table inside the read path — is exactly + * the drift this repo keeps paying for: the read would agree with the write + * only until someone edited one side, which is the state #4513 records. + * + * ## What it deliberately does NOT do + * + * - **It governs only DECLARED audit fields.** `applySystemFields` *injects* an + * absent one; this normalizer does not, because the served document is the + * authored metadata document and injecting columns into it would rewrite what + * a `GET` → `PUT` round-trip persists (the #4326 invariant) and what the + * layered read reports as "customised". An absent field does not claim + * `readonly: false`, so it is not the lie #4513 names. + * - **It governs only the audit family.** A declared `organization_id` / + * `owner_id` / `owning_business_unit_id` is the author's field and the + * registry lets it win (those are `additions`, not `overrides`), so reporting + * the author's value for them already agrees with what the write path + * enforces. Forcing them here would create the mismatch in the other + * direction. + */ + +import { + AUDIT_PROVENANCE_FIELDS, + resolveInjectedSystemColumns, + type AuditProvenanceField, +} from '@objectstack/spec/data'; + +/** + * The subset of an audit column's definition that is NOT authorable — the keys + * that decide **who may write** the column. + * + * Only `readonly` / `system` travel: everything else an author writes — + * `label`, `description`, `hidden`, `group`, and even `type` for an external + * object mapping a differently-typed remote column — stays theirs. Narrower is + * the point: this overrides an author, so it takes only what the defect + * requires (#4447). + * + * Keyed by the spec's {@link AUDIT_PROVENANCE_FIELDS} tuple, so a name added + * there without an entry here — or an entry for a name the spec dropped — is a + * compile error rather than a silently diverging copy. + */ +export const AUDIT_FIELD_GOVERNANCE: Record> = + Object.fromEntries( + AUDIT_PROVENANCE_FIELDS.map((name) => [name, { readonly: true, system: true }]), + ) as unknown as Record>; + +/** Does this field definition already carry every governance key at its governed value? */ +function isGoverned(declared: unknown, governance: Record): boolean { + if (!declared || typeof declared !== 'object' || Array.isArray(declared)) return false; + const rec = declared as Record; + for (const [key, value] of Object.entries(governance)) { + if (rec[key] !== value) return false; + } + return true; +} + +/** + * Force {@link AUDIT_FIELD_GOVERNANCE} onto every audit-provenance field the + * document declares, so what a reader is told about who may write the column + * matches what the engine enforces. + * + * Pure and total, with the same tolerance contract as + * {@link resolveInjectedSystemColumns}: any input may be handed to it, + * including a bare record that has never been through Zod. Objects that opt out + * of the audit family (`systemFields: false`, `systemFields.audit: false`, + * `managedBy: 'better-auth'`) carry no platform governance and are returned + * untouched — the same rows `applySystemFields` skips. + * + * Returns the **same reference** when nothing needed forcing, so a read path + * that already agrees with the engine (a registry-sourced document, which went + * through `applySystemFields` at registration) pays one comparison and no copy. + * + * @param doc An object metadata document, or any bare record shaped like one. + */ +export function applyAuditFieldGovernance(doc: T): T { + if (!doc || typeof doc !== 'object' || Array.isArray(doc)) return doc; + const rec = doc as unknown as Record; + const fields = rec.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return doc; + + // WHICH columns this object carries is the spec's derivation — the same one + // `applySystemFields` consumes. Re-deriving the opt-out conditions here is + // precisely the drift this module exists to prevent. + if (!resolveInjectedSystemColumns(rec).audit) return doc; + + const declaredFields = fields as Record; + let governed: Record | undefined; + for (const name of AUDIT_PROVENANCE_FIELDS) { + const declared = declaredFields[name]; + // Absent is not a lie — see the module header. Only a DECLARED audit field + // can claim a writability the engine refuses. + if (declared === undefined || declared === null) continue; + if (isGoverned(declared, AUDIT_FIELD_GOVERNANCE[name])) continue; + governed ??= { ...declaredFields }; + governed[name] = typeof declared === 'object' && !Array.isArray(declared) + ? { ...(declared as Record), ...AUDIT_FIELD_GOVERNANCE[name] } + : { ...AUDIT_FIELD_GOVERNANCE[name] }; + } + + if (governed === undefined) return doc; + return { ...rec, fields: governed } as unknown as T; +} diff --git a/packages/metadata-core/src/index.ts b/packages/metadata-core/src/index.ts index 1c09f5ed3a..aba553c99a 100644 --- a/packages/metadata-core/src/index.ts +++ b/packages/metadata-core/src/index.ts @@ -26,3 +26,12 @@ export * from './objects/index.js'; // See `scripts/check-engine-double-contract.mjs` — the gate over the doubles. export * from './engine-delete-dispatch.js'; export * from './engine-update-dispatch.js'; + +// [#4513] The audit-family GOVERNANCE table (#4447) and its normalizer, sunk +// here for the same reason and by the same criterion as the two dispatch +// predicates above: the `/meta` READ path lives in +// `@objectstack/metadata-protocol`, which `@objectstack/objectql` depends on, +// so it cannot import the table from the registry that enforces it. The read +// surface and the write path now derive one answer from one table instead of +// reporting two. +export * from './audit-field-governance.js'; diff --git a/packages/metadata-protocol/src/protocol.audit-field-governance.test.ts b/packages/metadata-protocol/src/protocol.audit-field-governance.test.ts new file mode 100644 index 0000000000..8f27658548 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.audit-field-governance.test.ts @@ -0,0 +1,336 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#4513] Every `/meta` object read exit reports the audit family the way the + * ENGINE enforces it — `readonly: true, system: true` — and never the + * `readonly: false` a stored artifact/overlay body happens to declare. + * + * ## The defect, measured on `origin/main` before the fix + * + * `#4447` made the audit family engine-owned on the WRITE path: the registry's + * `applySystemFields` forces `{ readonly: true, system: true }` over a declared + * `created_at`, and `ObjectQL.update` strips a non-system caller's write to it. + * The READ path never learned it — a `/meta` object read resolves through + * `sys_metadata` overlay → MetadataService → SchemaRegistry, and only the last + * of those three has ever been through `applySystemFields`. With the showcase's + * materialized `created_at` (FieldSchema defaults, so `readonly: false`) stored + * as the overlay body, all four exits answered: + * + * ``` + * single read: {"type":"datetime","label":"Created At","readonly":false} + * list read: {"type":"datetime","label":"Created At","readonly":false} + * cached read: {"type":"datetime","label":"Created At","readonly":false} + * layered read: {"type":"datetime","label":"Created At","readonly":false} + * ``` + * + * while a PATCH to that same field was being refused. One field, two answers, + * and the machine-readable one — the only face a client, or an AI author + * writing code off `/meta`, can see — was the wrong one. + * + * ## Why one case table and not one file per exit + * + * The exits are several faces of ONE contract, and the defect is that they can + * answer differently. A per-exit file lets a future divergence hide in the file + * nobody added a case to; driving every exit from one table means a new exit + * that disagrees fails here, and the table is where you add it. + * + * `_diagnostics.valid` is asserted alongside every row on purpose: the rule + * judges a VALUE (who may write the column), so its fixture has to be a + * document the spec fully accepts — otherwise the row would be pinning the + * governance of something no author could have written in the first place. + */ + +import { describe, expect, it } from 'vitest'; +// [#5619] The producer's OWN write-verb dispatch decisions, so the fake engine +// below cannot accept a call ObjectQL refuses. From `@objectstack/metadata-core` +// and not `@objectstack/objectql`: objectql DEPENDS ON this package, so that +// import would close a cycle turbo rejects outright. +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data'; +import { ObjectStackProtocolImplementation } from './index.js'; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; + checksum?: string; + version?: number; +} + +function matches(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; +} + +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +function makeStubEngine() { + const rows = new Map(); + let nextId = 0; + const findRow = (w: Record) => { + for (const [k, r] of rows) if (matches(r, w)) return { key: k, row: r }; + return null; + }; + const engine: any = { + async findOne(_t: string, opts: { where: Record }) { + return findRow(opts.where)?.row ?? null; + }, + async find(_t: string, opts: { where: Record }) { + return Array.from(rows.values()).filter((r) => matches(r, opts.where)); + }, + async insert(table: string, data: Record) { + if (table !== 'sys_metadata') return { id: 'side_table' }; + const row = { id: `r_${++nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(table: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + if (table !== 'sys_metadata') return { id: null }; + const found = findRow(opts.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as any) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, opts?: Record) { + assertEngineDeleteDispatch(opts); + return { deleted: 0 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + async syncObjectSchema() { /* no DDL in this stub */ }, + registry: { + listItems: () => [], + isPackageDisabled: () => false, + getItem: () => undefined, + registerItem: () => {}, + registerObject: () => {}, + getPackage: () => undefined, + }, + }; + return { engine, rows }; +} + +/** + * The showcase's built object body, verbatim in the shape that caused #4447 — + * a MATERIALIZED audit field carrying only FieldSchema defaults. `readonly` is + * spelled `false` on two of the four and left ABSENT on the other two, because + * absent defaults to false as well and the read must not be right only for the + * spelling that happens to be explicit. + */ +const artifactObject = (name: string) => ({ + name, + label: 'Invoice', + fields: { + amount: { type: 'currency', label: 'Amount' }, + created_at: { + label: 'Created At', type: 'datetime', required: false, + searchable: false, multiple: false, unique: false, + deleteBehavior: 'set_null', hidden: false, + readonly: false, sortable: true, externalId: false, + }, + created_by: { type: 'lookup', reference: 'sys_user', label: 'Created By' }, + updated_at: { type: 'datetime', label: 'Updated At', readonly: false }, + updated_by: { type: 'lookup', reference: 'sys_user', label: 'Updated By' }, + }, +}); + +/** One `/meta` read exit: how a client gets an object document out of the protocol. */ +interface Exit { + /** The route this exit serves, so a failure names the surface a client would hit. */ + readonly label: string; + /** Seeded before the read (draft exits need a draft row rather than an active one). */ + readonly mode?: 'draft'; + read( + protocol: ObjectStackProtocolImplementation, + name: string, + ): Promise | undefined>; +} + +const EXITS: readonly Exit[] = [ + { + label: 'GET /meta/object/:name — single item', + async read(p, name) { + return (await p.getMetaItem({ type: 'object', name })).item as any; + }, + }, + { + label: 'GET /meta/objects/:name — the PLURAL spelling of the same route (#4432)', + async read(p, name) { + return (await (p as any).getMetaItem({ type: 'objects', name })).item; + }, + }, + { + label: 'GET /meta/objects — list', + async read(p, name) { + const list: any = await p.getMetaItems({ type: 'object' }); + return (list.items as any[]).find((i) => i?.name === name); + }, + }, + { + label: 'GET /meta/object/:name — cached / ETag branch', + async read(p, name) { + const cached: any = await p.getMetaItemCached({ type: 'object', name }); + return cached?.data; + }, + }, + { + label: 'GET /meta/object/:name?layers=1 — the `effective` layer', + async read(p, name) { + const layered: any = await (p as any).getMetaItemLayered({ type: 'object', name }); + return layered?.effective; + }, + }, + { + label: 'GET /meta/object/:name?preview=draft — draft overlaid on active', + mode: 'draft', + async read(p, name) { + return (await p.getMetaItem({ type: 'object', name, previewDrafts: true })).item as any; + }, + }, + { + label: 'GET /meta/object/:name?state=draft — the strict draft read', + mode: 'draft', + async read(p, name) { + return (await p.getMetaItem({ type: 'object', name, state: 'draft' })).item as any; + }, + }, +]; + +async function seed(mode: 'active' | 'draft', name: string) { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await protocol.saveMetaItem({ + type: 'object', name, item: artifactObject(name), + ...(mode === 'draft' ? { mode: 'draft' as const } : {}), + }); + return { protocol, rows }; +} + +describe('[#4513] the /meta read surface agrees with the enforced write behaviour', () => { + for (const exit of EXITS) { + describe(exit.label, () => { + it('reports every audit-provenance field as engine-owned', async () => { + const { protocol } = await seed(exit.mode ?? 'active', 'crm_invoice'); + const served = await exit.read(protocol, 'crm_invoice'); + expect(served, 'the exit must actually resolve the item').toBeDefined(); + + for (const field of AUDIT_PROVENANCE_FIELDS) { + expect( + served!.fields[field], + `${field} is stripped from a non-system caller's write; the read must say so`, + ).toMatchObject({ readonly: true, system: true }); + } + }); + + it('keeps every non-governance key the author declared', async () => { + const { protocol } = await seed(exit.mode ?? 'active', 'crm_invoice'); + const served = await exit.read(protocol, 'crm_invoice'); + // Governance is `readonly` + `system` and nothing else — the + // author's presentation and storage shape survive untouched. + expect(served!.fields.created_at).toMatchObject({ + label: 'Created At', type: 'datetime', sortable: true, required: false, + }); + expect(served!.fields.created_by).toMatchObject({ + type: 'lookup', reference: 'sys_user', label: 'Created By', + }); + // …and an ordinary business field is not touched at all. + expect(served!.fields.amount).toEqual({ type: 'currency', label: 'Amount' }); + }); + }); + } + + it('the governed document is one the spec fully accepts', async () => { + // The rule judges a VALUE, so the fixture has to parse green — otherwise + // every row above would be pinning the writability of a document no + // author could have written. `_diagnostics` is the product's own + // `safeParse` over exactly the body it just served. + const { protocol } = await seed('active', 'crm_invoice'); + const served: any = (await protocol.getMetaItem({ type: 'object', name: 'crm_invoice' })).item; + expect(served._diagnostics).toEqual({ valid: true }); + }); + + it('an object that opts OUT of the audit family is left exactly as declared', async () => { + // The mirror row. `systemFields: { audit: false }` makes + // `applySystemFields` skip the family entirely, so the engine enforces + // NOTHING on this object's `created_at` — and a read that forced + // `readonly: true` here would be the same lie pointing the other way. + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + const optedOut = { + ...artifactObject('legacy_ledger'), + systemFields: { audit: false }, + }; + await protocol.saveMetaItem({ type: 'object', name: 'legacy_ledger', item: optedOut }); + + const served: any = (await protocol.getMetaItem({ type: 'object', name: 'legacy_ledger' })).item; + expect(served.fields.created_at.readonly).toBe(false); + expect(served.fields.created_at.system).toBeUndefined(); + expect(served.fields.created_by.readonly).toBeUndefined(); + }); + + it('a non-object metadata type is never touched', async () => { + // The governance is an OBJECT-schema contract, so the normalizer is + // gated on the metadata TYPE and not on "this document happens to have + // a `fields` key" — nothing stops another type from carrying one, and + // rewriting it there would be inventing a rule the engine does not have. + // + // Seeded straight into `sys_metadata` rather than through + // `saveMetaItem`: the subject here is the READ path, and a `page` body + // shaped like this is (correctly) refused by the save path's schema, so + // going through it would only measure that refusal. + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + const stored = { + name: 'invoice_page', + fields: { created_at: { type: 'datetime', label: 'Created At', readonly: false } }, + }; + await engine.insert('sys_metadata', { + type: 'page', name: 'invoice_page', organization_id: null, package_id: null, + state: 'active', metadata: JSON.stringify(stored), + }); + + const served: any = (await protocol.getMetaItem({ type: 'page', name: 'invoice_page' })).item; + expect(served.fields.created_at).toEqual({ + type: 'datetime', label: 'Created At', readonly: false, + }); + }); +}); + +describe('[#4513] the served correction does not become a phantom customization', () => { + it('the overlay row keeps what the author actually stored', async () => { + // The read reports the EFFECTIVE value; `sys_metadata` keeps the + // author's declaration. Collapsing the two would make the layered + // read's `code` vs `overlay` diff report a customization nobody made. + const { protocol, rows } = await seed('active', 'crm_invoice'); + await protocol.getMetaItem({ type: 'object', name: 'crm_invoice' }); + + const stored = JSON.parse(Array.from(rows.values()).find((r) => r.name === 'crm_invoice')!.metadata); + expect(stored.fields.created_at.readonly).toBe(false); + expect(stored.fields.created_at.system).toBeUndefined(); + }); + + it('the layered read still shows the raw `code`/`overlay` layers beside the governed `effective`', async () => { + const { protocol } = await seed('active', 'crm_invoice'); + const layered: any = await (protocol as any).getMetaItemLayered({ + type: 'object', name: 'crm_invoice', + }); + // The diagnostic's whole point is seeing the layers as they are… + expect(layered.overlay.fields.created_at.readonly).toBe(false); + // …while `effective` keeps its documented promise of being what + // `getMetaItem` would return. + expect(layered.effective.fields.created_at).toMatchObject({ readonly: true, system: true }); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8031298444..b9f7d217c5 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -8,7 +8,7 @@ import { readEnvWithDeprecation } from '@objectstack/types'; import type { MetadataHostEngine } from './host-engine.js'; import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js'; import { SysMetadataRepository, type SysMetadataEngine } from './sys-metadata-repository.js'; -import { ConflictError, assertProtocolCompat, type MetadataItem } from '@objectstack/metadata-core'; +import { ConflictError, assertProtocolCompat, applyAuditFieldGovernance, type MetadataItem } from '@objectstack/metadata-core'; // [#5532] One vocabulary of "which driver read errors are benign", shared with // `sys-metadata-repository.ts` in this package and with `DatabaseLoader` in // `@objectstack/metadata` (#5108). See `rethrowUnlessMetadataStoreUnprovisioned`. @@ -130,6 +130,33 @@ function canonicalizeMetaRequestType(request: T): T return type === request.type ? request : { ...request, type }; } +/** + * [#4513] The last thing every `/meta` read does to an OBJECT document before + * it leaves this service: make the field metadata it reports agree with what + * the engine enforces on the write path. + * + * The mismatch this closes is structural, not incidental. A `/meta` object read + * resolves through `sys_metadata` overlay → MetadataService → SchemaRegistry, + * and only the last of those three has been through `applySystemFields` — so + * the two stored layers answered with whatever the artifact/overlay body + * happened to declare, while `ObjectQL.update` was stripping caller writes to + * the audit family off the registry's post-injection schema. `created_at` read + * `readonly: false` and wrote as read-only, on the same field, at the same + * moment, from the one face a client can actually see (#4447 fixed the write + * half; this is the read half). + * + * Applied per EXIT rather than inside `decorateMetadataItem`: decoration is a + * diagnostics concern whose output `stripReadDecorations` deliberately removes + * again on write, and governance is neither — it is what the document means. + * + * `applyAuditFieldGovernance` returns its input by reference when nothing needed + * forcing, so the registry-sourced path (already governed at registration) and + * every non-object type pay a comparison and no copy. + */ +function governServedItem(type: string, item: T): T { + return canonicalMetaType(type) === 'object' ? applyAuditFieldGovernance(item) : item; +} + /** * [#5206 step 2] Where an author of THIS path sets the namespace the ADR-0121 * D2 gate demands. @@ -3544,7 +3571,11 @@ export class ObjectStackProtocolImplementation implements (it as any)?.name, packageId ?? ((it as any)?._packageId as string | undefined), ); - return mergeArtifactProtection(it, a) as any; + // [#4513] Same governance as the single-item read — the list + // is the other exit a client reads field metadata from, and + // an overlay row wins over the (already-governed) registry + // entry in the merge above, so it carries the same lie. + return governServedItem(request.type, mergeArtifactProtection(it, a)) as any; }), ), }; @@ -3605,7 +3636,11 @@ export class ObjectStackProtocolImplementation implements if (recPkg && (draftItem as any)._packageId === undefined) (draftItem as any)._packageId = recPkg; (draftItem as any)._draft = true; } - return { type: request.type, name: request.name, item: decorateMetadataItem(request.type, draftItem) }; + return { + type: request.type, + name: request.name, + item: decorateMetadataItem(request.type, governServedItem(request.type, draftItem)), + }; } } catch (error) { // [#5532] Falling through to the active read here would answer @@ -3695,7 +3730,11 @@ export class ObjectStackProtocolImplementation implements err.status = 404; throw err; } - return { type: request.type, name: request.name, item: decorateMetadataItem(request.type, item) }; + return { + type: request.type, + name: request.name, + item: decorateMetadataItem(request.type, governServedItem(request.type, item)), + }; } // 2. MetadataService (runtime-registered items: HMR-updated view/page/ @@ -3790,7 +3829,7 @@ export class ObjectStackProtocolImplementation implements const artifactItem = this.lookupArtifactItem(request.type, request.name, request.packageId); let decorated = decorateMetadataItem( request.type, - mergeArtifactProtection(item, artifactItem), + governServedItem(request.type, mergeArtifactProtection(item, artifactItem)), ); // ADR-0047 — list views additionally get reference-integrity // diagnostics (userFilters/tabs fields must exist on the source @@ -4051,7 +4090,16 @@ export class ObjectStackProtocolImplementation implements this.rethrowUnlessMetadataStoreUnprovisioned(error); } - const effective: unknown | null = overlay ?? code; + // [#4513] `effective` is documented above as "what `getMetaItem` would + // return", and the response's `_diagnostics` is computed from it — so it + // carries the same audit-family governance that read now applies, or the + // sentence stops being true the moment the overlay declares a writable + // `created_at`. `code` and `overlay` are deliberately left RAW: they are + // the diagnostic's whole point (what the package shipped vs what was + // customised), and a Studio diff showing `code`'s declaration next to + // `effective`'s governed value is the platform override made visible, + // not a defect. + const effective: unknown | null = governServedItem(request.type, overlay ?? code); const _diagnostics = effective !== null && effective !== undefined diff --git a/packages/objectql/src/engine-audit-anchor-write.test.ts b/packages/objectql/src/engine-audit-anchor-write.test.ts index 778b2b2b1d..cd9451d02f 100644 --- a/packages/objectql/src/engine-audit-anchor-write.test.ts +++ b/packages/objectql/src/engine-audit-anchor-write.test.ts @@ -11,6 +11,11 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; +// [#4513] The `/meta` read path's share of the governance table this file's +// write-path assertions measure. Imported from `@objectstack/metadata-core` +// because that is where it lives for BOTH consumers — see the last describe. +import { applyAuditFieldGovernance } from '@objectstack/metadata-core'; +import { AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data'; import { ObjectQL } from './engine.js'; const taskObject = { @@ -304,3 +309,103 @@ describe('[#4447] a declared audit field cannot loosen the platform posture', () }); }); }); + +// --------------------------------------------------------------------------- +// [#4513] The other half of the same contract: what the `/meta` READ surface +// tells a client about these fields. +// +// #4447 closed the write path off the registry's post-injection schema — a +// place `@objectstack/metadata-protocol` structurally cannot reach (`objectql` +// depends on IT, so the import would close a turbo-rejected cycle). The read +// path therefore answered from the stored artifact/overlay body and reported +// `readonly: false` on the very field every assertion above proves is refused. +// +// `applyAuditFieldGovernance` is the read path's share of one table, and this +// block is where the two are held against each other with BOTH sides live: the +// engine's refusal measured on a real write, and the normalizer's answer +// measured on the same document. A future key added to the governance the +// registry forces, but not to the normalizer, fails here rather than in a +// production `/meta` body. +// --------------------------------------------------------------------------- +describe('[#4513] the read-path normalizer answers what this engine enforces', () => { + const shadowed = { + name: 'audit_readface', + label: 'Shadowed', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + // The same materialized artifact field as the block above. + created_at: { + label: 'Created At', type: 'datetime' as const, required: false, + readonly: false, sortable: true, + }, + }, + }; + + let engine: ObjectQL; + beforeEach(async () => { + engine = new ObjectQL(); + const { driver } = makeStubDriver(); + engine.registerDriver(driver, true); + await engine.init(); + // [#4311] Typed call site: `packageId` is required, and the ledger over + // this package's hidden test layer is a shrink-only ratchet — a new test + // pays its own way rather than raising the frozen number. + engine.registry.registerObject(shadowed as any, 'test'); + }); + + it('the normalizer reproduces the registry\'s governance, field for field', () => { + // Ground truth: what the engine will enforce, read off the live registry. + const enforced: any = engine.registry.getObject('audit_readface')?.fields; + // What a `/meta` read is allowed to say, computed from the AUTHORED + // document — the body the read path actually has in hand. + const reported: any = (applyAuditFieldGovernance(shadowed) as any).fields; + + for (const name of AUDIT_PROVENANCE_FIELDS) { + if (!reported[name]) continue; // the read path governs declared fields only + expect(reported[name].readonly, `${name}.readonly`).toBe(enforced[name].readonly); + expect(reported[name].system, `${name}.system`).toBe(enforced[name].system); + } + expect(reported.created_at).toMatchObject({ readonly: true, system: true }); + }); + + it('the refusal and the reported value are measured on the same field', async () => { + const row: any = await engine.insert( + 'audit_readface', { title: 'T' }, { context: { userId: 'u1' } } as any, + ); + const real = row.created_at; + await engine.update( + 'audit_readface', + { title: 'T2', created_at: FORGED }, + { where: { id: row.id }, context: { userId: 'u1' } } as any, + ); + // [#4918] No `as any` on the options bag — this call is ON contract, and + // the signature infers it. + const after: any = await engine.findOne('audit_readface', { where: { id: row.id } }); + + // The write was refused … + expect(after.created_at).toBe(real); + expect(after.created_at).not.toBe(FORGED); + // … so the read is not permitted to advertise it as writable. + expect((applyAuditFieldGovernance(shadowed) as any).fields.created_at.readonly).toBe(true); + }); + + it('leaves an audit-opted-out object alone, because the engine does too', () => { + const optedOut = { + name: 'audit_optout', + label: 'Opted out', + systemFields: { audit: false }, + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + created_at: { label: 'Created At', type: 'datetime' as const, readonly: false }, + }, + }; + engine.registry.registerObject(optedOut as any, 'test'); + + const enforced: any = engine.registry.getObject('audit_optout')?.fields.created_at; + const reported: any = (applyAuditFieldGovernance(optedOut) as any).fields.created_at; + expect(enforced.readonly).toBe(false); + expect(reported.readonly).toBe(false); + expect(reported.system).toBeUndefined(); + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index a72fb1dcc4..5b3ce386ff 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -1,6 +1,9 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary, resolveCrudAffordances, resolveInjectedSystemColumns, LEGACY_API_METHODS, AUDIT_PROVENANCE_FIELDS, type AuditProvenanceField } from '@objectstack/spec/data'; +// [#4513] The audit-family governance table — see the re-export below for why +// it lives in a package both `objectql` and `metadata-protocol` depend on. +import { AUDIT_FIELD_GOVERNANCE } from '@objectstack/metadata-core'; import { SystemFieldName } from '@objectstack/spec/system'; import { resolveTenancyPosture, resolveSearchPinyinEnabled } from '@objectstack/types'; import { postureEnforcesWall } from '@objectstack/spec/security'; @@ -300,16 +303,20 @@ const AUDIT_FIELD_DEFS = { * Only `readonly` / `system` travel: everything else an author writes — * `label`, `description`, `hidden`, `group`, and even `type` for an * external object mapping a differently-typed remote column — stays theirs. + * `type` and `reference` are deliberately NOT forced: an external/federated + * object legitimately maps its audit column to a differently-typed remote + * column, and #4447 is about writability, not storage shape. Narrower is the + * point — this overrides an author, so it takes only what the defect requires. + * + * [#4513] The table itself now lives in `@objectstack/metadata-core`, because + * the `/meta` READ surface has to report the same answer this injection + * enforces and could not reach it here: `@objectstack/objectql` depends on + * `@objectstack/metadata-protocol`, so the import a read-side pin needs would + * close a turbo-rejected cycle. Same criterion, same package, and for the same + * reason as the #5619 dispatch predicates. Re-exported here so the symbol + * still resolves from `@objectstack/objectql`. */ -const AUDIT_FIELD_GOVERNANCE: Record> = - Object.fromEntries( - // ONLY the keys that decide WHO MAY WRITE the column. `type` and - // `reference` are deliberately NOT forced: an external/federated object - // legitimately maps its audit column to a differently-typed remote column, - // and #4447 is about writability, not storage shape. Narrower is the point - // — this overrides an author, so it takes only what the defect requires. - AUDIT_PROVENANCE_FIELDS.map((name) => [name, { readonly: true, system: true }]), - ) as unknown as Record>; +export { AUDIT_FIELD_GOVERNANCE }; /** * [ADR-0117 D1] The injected BU-ownership column name, spelled out so it greps