diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 6be924c3eae..62d9409b697 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -128,7 +128,7 @@ that silently does not happen. | 21 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped. A second read in the same symbol gates the pass that withholds those caller values from `beforeInsert` before the strip, the same exemption, so a system caller's hook still sees what it sent | `packages/objectql/src/engine.ts#insert` | | 21b | **`readonly` strip bypassed — the write PREVIEW (`ObjectQL.validate`)** | objectql | Same as 19–21, in the preview: it runs the write's caller-write strips under the write's own gate, so a system preview reports no `readonly` drop and judges the value a system write would store. The `computed` strip of a `formula` value is NOT gated here or on either write verb — no driver has a column for it, so it runs for every caller | `packages/objectql/src/engine.ts#validate` | | 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing. Two exceptions, both judged in every context: a `formula` value, which the `computed` strip takes from every caller, and a value for a stored field that declares `expression` when it differs from the value the engine computes in its place (`stored_expression`), so strict still refuses both | `packages/objectql/src/engine.ts#insert`, `packages/objectql/src/readonly-strict-errors.ts#READONLY_CLASS_REASONS` | -| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `packages/objectql/src/engine.ts#assertReferencesResolve` | +| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `packages/objectql/src/engine-referential-integrity.ts#assertReferencesResolve` | | 23b | **Position-catalog check skipped** — a `sys_user_position` write whose `position` names no position in the security catalog is not refused | plugin-security | Get: a system writer can store an assignment naming no position — the seed loader writes `stack.data` from `AppPlugin.start()` under a system context, as invitation acceptance and the platform's own bootstraps do. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `position`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no position of that name: the environment registry (ADR-0131 D3), one per deployment, which takes no organization, so a name only a `sys_position` row carries — any organization's — is refused like one nobody carries. It is the same stand-down as the engine's referential-integrity check for a lookup column | `packages/plugins/plugin-security/src/position-catalog-refusal.ts#assertPositionNamesCatalogRow` | | 23c | **Grant-name catalog check skipped** — a `sys_user_permission_set` write whose `permission_set` names no permission set in the security catalog is not refused | plugin-security | Get: a system writer can store a grant naming a set not yet registered — seed replay and boot provisioning may write the grant before the definition it names is registered. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `permission_set`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no set by that name; a grant that names no set at all is refused `required` for every caller. The grant names its set by name alone, and nothing derives the name from `permission_set_id` (ADR-0131 D4, D10) | `packages/plugins/plugin-security/src/grant-permission-set-name.ts#onInsert`, `#onUpdate` | | 23d | **Grant-holder membership check skipped** — a `sys_user_position` or `sys_user_permission_set` write naming a user with no `sys_member` row in the row's organization is not refused | plugin-security | Get: a system writer can store such a grant — invitation acceptance writes the membership and its placement together, and seed replay, the organization-admin reconcile and the platform bootstraps write grants as the system. Lose: such a row takes effect once its user is admitted to the organization. A non-system insert, or a non-system update that moves `user_id` or `organization_id`, is refused `400 VALIDATION_FAILED` / `reference_not_found` at `user_id` instead, for every caller, a platform administrator included; the membership is read as `{ isSystem: true, tenantId }` with `tenantId` the row's own organization, never another one. A row stored with no organization is not judged. Under `single`, 18.0 stamps a new grant with the Default Organization, so such a row is one an earlier release wrote | `packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts#onInsert`, `#onUpdate` | @@ -140,7 +140,7 @@ that silently does not happen. | 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `packages/objectql/src/engine.ts#dependentCountIsDisclosable` | | 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `packages/objectql/src/engine.ts#recordReferenceCheckElevation` | | 29b | A traversing validation rule's related read skips the **caller's bounds** | objectql | Get: no row bound from the caller's own read on a related object no organization wall scopes, no read door on a header the rule reads through a `master_detail` field, and a related read even with no organization under a walled posture (a `tenantId` on the context still scopes it). Every caller that is not system is held to all three: a header its read door does not return is unavailable to the rule exactly as a header that exists nowhere is, whatever it holds, and a header it can read is judged on its stored values. A plain `lookup` to an object an organization wall scopes asks no read door, system caller or not | `packages/objectql/src/engine.ts#resolveOneHopRelated` | -| 29c | A detail write's master-detail header read asks **no read door** | objectql | Get: the write binds the header a parent-scoped `requiredWhen` / `readonlyWhen` reads as `parent` from the header's stored row whoever can read it, as before. Every caller that is not system meets its own read door first: a header that read does not return answers as a header that exists nowhere — an unbound `parent`, and `reference_not_found` on the reference check (which a system caller's write skips altogether) | `packages/objectql/src/engine.ts#masterDetailHeadersCallerReads` | +| 29c | A detail write's master-detail header read asks **no read door** | objectql | Get: the write binds the header a parent-scoped `requiredWhen` / `readonlyWhen` reads as `parent` from the header's stored row whoever can read it, as before. Every caller that is not system meets its own read door first: a header that read does not return answers as a header that exists nowhere — an unbound `parent`, and `reference_not_found` on the reference check (which a system caller's write skips altogether) | `packages/objectql/src/engine-referential-integrity.ts#masterDetailHeadersCallerReads` | | 30 | **Bulk data event `organizationId` OMITTED** — the batch is published "not asserted" | plugin-security | Get: nothing — the `data.records.*` event still publishes. Lose: the per-organization attribution: this exit is taken before the security middleware composes any tenant wall, so it records no Layer 0 verdict on the operation (`OperationContext.tenantLayer0Verdict`, #15813), and the engine's bulk producer — which reads that recorded verdict and nothing else — omits the key rather than filling it from the caller's `tenantId`; a tenant-scoped consumer then does not deliver the event inside an organization wall (#15225) | `packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts#registerAuthzMiddleware` | | 30b | **The import doors admit a control opt-out** — `runAutomations: false` and `treatAsHistorical: true` on `POST /data/:object/import` and `/import/jobs` | rest | Get: an import whose rows are written with `skipAutomations` (no metadata-bound hook, no record-change trigger: an app's approvals and record freezes among them) or with `skipStateMachine` and `preserveAudit`. Every other caller, every signed-in user and administrator included, is refused `403 PERMISSION_DENIED` before the body is parsed, so nothing is written and no job is created. Inbound HTTP never carries the flag, so this is an in-process caller's door. Note: the flag still suppresses none of those controls by itself (see below); it only admits the opt-outs that do | `packages/rest/src/import-prepare.ts#importControlOptOutRefusal` | | 30c | **The import undo keeps the control skips** — `POST /data/import/jobs/:jobId/undo` | rest | Get: an undo whose deletes and restores are written with `skipAutomations` and `skipStateMachine`, so no metadata-bound hook, record-change trigger or `state_machine` rule refuses a reversal. Every other caller, every signed-in user and administrator included, undoes under those controls: a reversal one of them refuses is reported per row in the undo answer's `failures` and kept in the job's undo log, and the job is stamped reverted only once every row is. A historical job's `preserveAudit` belongs to the job, not the caller, and rides every undo. Inbound HTTP never carries the flag, so this is an in-process caller's door | `packages/rest/src/data-action-routes.ts#importUndoWriteContext` | diff --git a/packages/core/src/security/second-object-read-exposure.pin.test.ts b/packages/core/src/security/second-object-read-exposure.pin.test.ts index 3a09ee59ef8..98249f38966 100644 --- a/packages/core/src/security/second-object-read-exposure.pin.test.ts +++ b/packages/core/src/security/second-object-read-exposure.pin.test.ts @@ -138,7 +138,6 @@ const CALLERS: Record = { read: "the expansion sub-read (`ObjectQL.expandRelatedRecords`), for the data door's `$expand`", decidedIn: 'packages/metadata-protocol/src/protocol-query-validation.ts', also: - 'the write-path reference-existence probe (`referenceExists`: whether a stored id resolves, no field) and ' + 'the validation-predicate reads (`resolvePredicateRelated`: a pass/fail verdict, the value never served). ' + "The engine's privileged callers (a flow's `config.expand`, a hook's read) never pass the door, by design.", }, @@ -210,6 +209,14 @@ const CALLERS: Record = { kind: 'not-a-served-read', reason: "resolves a seed's references at install, under the system identity; serves nothing to a caller", }, + // [#25522 cut 10] The write-path reference check, moved out of engine.ts. + 'packages/objectql/src/engine-referential-integrity.ts': { + kind: 'not-a-served-read', + reason: + 'the write-path reference-existence check (`assertReferencesResolve`): it asks whether a caller-supplied id ' + + "resolves on the target (the engine's `referenceExists` probe) and answers `reference_not_found` or nothing, " + + 'never a field of the target', + }, 'packages/objectql/src/integrity/dangling-reference-audit.ts': { kind: 'not-a-served-read', reason: 'an integrity audit: it probes whether a stored id resolves and reports ids, never a field of the target', diff --git a/packages/objectql/src/engine-referential-integrity.ts b/packages/objectql/src/engine-referential-integrity.ts new file mode 100644 index 00000000000..63ba1e8b6a2 --- /dev/null +++ b/packages/objectql/src/engine-referential-integrity.ts @@ -0,0 +1,200 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25522 cut 10] Two referential-integrity bodies of `ObjectQL`, moved out + * of `engine.ts` whole: `assertReferencesResolve` (the write path's #4441 + * reference check: a caller-supplied id in a reference field must name a row + * of its target) and `masterDetailHeadersCallerReads` (#22519: which + * master-detail headers the caller's own read door returns), with + * `isEmptyReferenceValue`, the module helper only the first of them calls. + * + * A MOVE, not a redesign. Each exported function below is the body of the + * `ObjectQL` method of the same name as it stood, re-indented one level and + * otherwise unedited: the same system-caller exit, the same `readonly` and + * caller-supplied skips, the same master-detail branch, the same one-`find` + * read door and the same warning when that read fails. Each method's + * docblock stays on the method in `engine.ts`, which is still the contract. + * `isEmptyReferenceValue` moves with its docblock, byte for byte, and is not + * exported. + * + * What stays in the class: `referenceExists`, `resolveMasterDetailParent` and + * `resolveMasterDetailParents`, which read the private static + * `referenceCheckContext` and hold an erased query-options site; the bodies + * that call a module helper `engine.ts` still needs elsewhere + * (`settleMasterDetailLanding`, `resolvePreviewParents`, + * `resolvePredicateRelated`, `resolveOneHopRelated`); and + * `masterDetailHeaderExists`, `materializeParentHeader` and + * `masterDeclaredFields`, whose bodies are no longer than the host literal + * moving them would cost. + * + * ## How the moved bodies reach the class + * + * The shape #23050 set for `rest-server.ts`, applied per method as + * `engine-compatibility-api.ts` applies it. The class keeps both methods with + * name, modifiers, signature, docblock and position, and each body is one + * literal of exactly the {@link ReferentialIntegrityHost} members that method + * reads, plus a `.call`. The one field a body reads (`logger`) goes as a + * GETTER, so the read happens when the body makes it, as it did. The methods + * they call (`find`, `masterDetailHeaderExists`, `referenceExists`) go as + * late-bound closures over the instance, so a double or spy set on the engine + * is still the one a call reaches. + * + * This module imports the engine only as a TYPE, so there is no module cycle + * at run time, and nothing re-exports it, so the published surface does not + * move. + */ + +import { referenceTargetOf, type EngineQueryOptions } from '@objectstack/spec/data'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { ValidationError, buildFieldError } from './validation/record-validator.js'; +import type { ObjectQL } from './engine.js'; + +/** + * The `ObjectQL` members the functions below read — every one of them, and no + * other; each function's `this` is the `Pick` of the members IT reads. Each is + * typed AS the class member (an indexed access on the class), so a change to a + * member's type on the class is a change here too, and naming a member the + * class does not have is a compile error. See the module header for which + * member is a getter. + */ +export interface ReferentialIntegrityHost { + readonly logger: ObjectQL['logger']; + find: ObjectQL['find']; + masterDetailHeaderExists: ObjectQL['masterDetailHeaderExists']; + referenceExists: ObjectQL['referenceExists']; +} + +/** + * [#4441] "The caller did not name a record here." + * + * `null` / `undefined` / `''` mean NO LINK — exactly what + * `deleteBehavior: 'set_null'` writes — and an empty array is the multi-value + * spelling of the same thing. None of them is an id to resolve. + */ +function isEmptyReferenceValue(v: unknown): boolean { + if (v === null || v === undefined || v === '') return true; + if (Array.isArray(v)) return v.length === 0 || v.every((e) => e === null || e === undefined || e === ''); + return false; +} + +/** The body of `ObjectQL#assertReferencesResolve`; the method's docblock there is its contract. */ +export async function assertReferencesResolve( + this: Pick, + schema: any, + data: Record | null | undefined, + supplied: Record | null | undefined, + context: any, + msgCtx?: { locale?: string; translate?: any; objectName?: string }, +): Promise { + if (context?.isSystem) return; + const fields = schema?.fields; + if (!fields || !data) return; + + const failures: any[] = []; + for (const name of Object.keys(fields)) { + // [#4441, narrowing kept by #4743] A `readonly` field is never the + // caller's to answer for — BY CONSTRUCTION, not by exemption. + // + // This check answers for exactly one thing: "the reference the CALLER + // named". `stripReadonlyFields` removes a non-system caller's value from + // a readonly field before the write — on BOTH write paths since #14147 + // moved the create-side strip into this engine and deleted the boundary + // copy it used to live in. So a value still sitting in one at + // this point was minted by the PLATFORM — outside this check's own stated + // scope, whatever it happens to hold. That argument stands on its own and + // depends on no particular field: deleting the `continue` would start + // rejecting the platform's own writes, which is why it is recorded here + // (AGENTS.md PD #13) rather than left to be re-derived. + // + // Historical note, kept because it is how the narrowing was FOUND (the + // dogfood gate, not reasoning) and because the audit next door re-scoped + // itself on its removal: `sys_metadata_history.recorded_by` is a + // `Field.lookup('sys_user', { readonly: true })` that the metadata + // repository once filled with `actor ?? 'system'` — a SENTINEL STRING, + // not a user id, on a write carrying no `isSystem`; checking it rejected + // ordinary metadata authoring (package create / publish / clone). #4556 + // replaced that sentinel with NULL, so no platform write puts a non-id in + // a reference column any more. The narrowing survives it unchanged + // because it never rested on it — but #4551's blanket audit skip did, and + // #4743/#5719 duly re-scoped that one (see `inspectDanglingReferences`). + // + // This does NOT weaken #4441: the fields the issue names — + // `sys_position_permission_set.permission_set_id` and + // `showcase_task.project` — are ordinary author-facing lookups with no + // `readonly`, and both stay enforced (pinned in the unit suite). + if (fields[name]?.readonly === true) continue; + // Only a value the CALLER actually supplied is theirs to answer for. + // + // Key presence is not enough: a form serializes an unpicked control as + // an explicit `null`, and `applyFieldDefaults` then fills it from + // `defaultValue` — including the `current_user` token (#2706). The key is + // in the payload, but the ID that lands is the PLATFORM's, so validating + // it would report a server-derived value as the caller's bad reference + // (and reject a perfectly ordinary insert against a driver that has no + // `sys_user` row for the acting principal). + // + // So the value read for the check comes from the post-normalization + // `data` (multi-value strings are already split by then), while WHETHER + // to check is decided by the caller's own raw value being non-empty. + if (!supplied || isEmptyReferenceValue((supplied as Record)[name])) continue; + if (!(name in data)) continue; + const def = fields[name]; + const target = referenceTargetOf(def); + if (!target) continue; + const raw = (data as Record)[name]; + const values = Array.isArray(raw) ? raw : [raw]; + for (const v of values) { + if (v === null || v === undefined || v === '') continue; + if (typeof v === 'object') continue; + // [#22519] A master-detail header must also be one the caller can read. + const resolved = def?.type === 'master_detail' + ? await this.masterDetailHeaderExists(target, v, context) + : await this.referenceExists(target, v, context); + if (resolved === false) { + failures.push(buildFieldError( + { + field: name, + code: 'reference_not_found', + def, + value: String(v), + constraint: { target }, + }, + msgCtx as any, + )); + } + } + } + if (failures.length > 0) throw new ValidationError(failures); +} + +/** The body of `ObjectQL#masterDetailHeadersCallerReads`; the method's docblock there is its contract. */ +export async function masterDetailHeadersCallerReads( + this: Pick, + master: string, + ids: ReadonlyArray, + context: ExecutionContext | undefined, +): Promise> { + const asked = new Map(); + for (const id of ids) if (id != null) asked.set(String(id), id); + if (context?.isSystem || asked.size === 0) return new Set(asked.keys()); + // One id is asked by equality, the predicate shape the elevated probe + // already sends; a batch by `$in`, as read 2 of the batch reads it. + const values = [...asked.values()]; + try { + const rows = await this.find(master, { + where: { id: values.length === 1 ? values[0] : { $in: values } }, + fields: ['id'], + context, + } as EngineQueryOptions) as Array>; + const served = new Set(); + for (const row of Array.isArray(rows) ? rows : []) { + if (row?.id != null && asked.has(String(row.id))) served.add(String(row.id)); + } + return served; + } catch (err) { + this.logger?.warn?.('the caller\'s read of the master-detail header failed — the header binds as a missing header', { + object: master, error: err, + }); + return new Set(); + } +} diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index c9773e3ac5f..0603fb25caa 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -142,6 +142,10 @@ import { getConfigs, introspectDatasource, registerObject, removePackage, syncOb // [#25522 cut 9] The hook-context caller-envelope builders; the methods of the // same names below call them, and none of the bodies reads a member. import { buildEvalUser, buildProvenance, buildSession, buildUser } from './engine-execution-plumbing.js'; +// [#25522 cut 10] The write path's reference check and the caller's read of a +// master-detail header; the methods of the same names below hand each body the +// members it reads (`ReferentialIntegrityHost`). +import { assertReferencesResolve, masterDetailHeadersCallerReads } from './engine-referential-integrity.js'; import { ExecutionContext, ExecutionContextSchema, type InstalledPackageSource } from '@objectstack/spec/kernel'; import { isStoredMetadataBodyObject } from '@objectstack/spec/kernel'; import type { FlowFunctionEffect } from '@objectstack/spec/automation'; @@ -320,7 +324,7 @@ import { deriveViewContainerObject } from '@objectstack/metadata/view-container' import { viewContainerNameRefusal } from './view-container-name-refusal.js'; import { bindHooksToEngine, type BindHooksOptions } from './hook-binder.js'; import { declaredHookCondition } from './hook-wrappers.js'; -import { validateRecord, validateRecordInScope, normalizeMultiValueFields, normalizeBlankTypedValues, normalizeNumericStringValues, coerceBooleanFields, ValidationError, type FieldValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js'; +import { validateRecord, validateRecordInScope, normalizeMultiValueFields, normalizeBlankTypedValues, normalizeNumericStringValues, coerceBooleanFields, ValidationError, type FieldValidationError, 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, collectReferenceTraversals, referenceTraversalConflicts, evaluateValidationRules, emitValidationAdvisories, optionVisibilityReadsPermissions, readsPermissionPredicate, referentialClearBinding, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, hasParentScopedReadonlyWhenInPayload, hasParentScopedRequiredWhen, stripReadonlyFields, stripRuntimeOwnedFields, staticReadonlyInsertSubject, preserveAuditIgnoredOnInsertWarning } from './validation/rule-validator.js'; @@ -4327,19 +4331,6 @@ function resolveMetadataItemName(key: string, item: any): string | undefined { // on every build, so the seven consumer-local surface declarations the contract // replaced can never silently drift from the engine again. IObjectQLEngine // extends IDataEngine, so the old claim rides along. -/** - * [#4441] "The caller did not name a record here." - * - * `null` / `undefined` / `''` mean NO LINK — exactly what - * `deleteBehavior: 'set_null'` writes — and an empty array is the multi-value - * spelling of the same thing. None of them is an id to resolve. - */ -function isEmptyReferenceValue(v: unknown): boolean { - if (v === null || v === undefined || v === '') return true; - if (Array.isArray(v)) return v.length === 0 || v.every((e) => e === null || e === undefined || e === ''); - return false; -} - /** * [#4889] The master id a detail row's `parent` binding resolves against: * the write's own value for the FK when it carries one (a REPOINT must be @@ -9929,85 +9920,12 @@ export class ObjectQL implements IObjectQLEngine { context: any, msgCtx?: { locale?: string; translate?: any; objectName?: string }, ): Promise { - if (context?.isSystem) return; - const fields = schema?.fields; - if (!fields || !data) return; - - const failures: any[] = []; - for (const name of Object.keys(fields)) { - // [#4441, narrowing kept by #4743] A `readonly` field is never the - // caller's to answer for — BY CONSTRUCTION, not by exemption. - // - // This check answers for exactly one thing: "the reference the CALLER - // named". `stripReadonlyFields` removes a non-system caller's value from - // a readonly field before the write — on BOTH write paths since #14147 - // moved the create-side strip into this engine and deleted the boundary - // copy it used to live in. So a value still sitting in one at - // this point was minted by the PLATFORM — outside this check's own stated - // scope, whatever it happens to hold. That argument stands on its own and - // depends on no particular field: deleting the `continue` would start - // rejecting the platform's own writes, which is why it is recorded here - // (AGENTS.md PD #13) rather than left to be re-derived. - // - // Historical note, kept because it is how the narrowing was FOUND (the - // dogfood gate, not reasoning) and because the audit next door re-scoped - // itself on its removal: `sys_metadata_history.recorded_by` is a - // `Field.lookup('sys_user', { readonly: true })` that the metadata - // repository once filled with `actor ?? 'system'` — a SENTINEL STRING, - // not a user id, on a write carrying no `isSystem`; checking it rejected - // ordinary metadata authoring (package create / publish / clone). #4556 - // replaced that sentinel with NULL, so no platform write puts a non-id in - // a reference column any more. The narrowing survives it unchanged - // because it never rested on it — but #4551's blanket audit skip did, and - // #4743/#5719 duly re-scoped that one (see `inspectDanglingReferences`). - // - // This does NOT weaken #4441: the fields the issue names — - // `sys_position_permission_set.permission_set_id` and - // `showcase_task.project` — are ordinary author-facing lookups with no - // `readonly`, and both stay enforced (pinned in the unit suite). - if (fields[name]?.readonly === true) continue; - // Only a value the CALLER actually supplied is theirs to answer for. - // - // Key presence is not enough: a form serializes an unpicked control as - // an explicit `null`, and `applyFieldDefaults` then fills it from - // `defaultValue` — including the `current_user` token (#2706). The key is - // in the payload, but the ID that lands is the PLATFORM's, so validating - // it would report a server-derived value as the caller's bad reference - // (and reject a perfectly ordinary insert against a driver that has no - // `sys_user` row for the acting principal). - // - // So the value read for the check comes from the post-normalization - // `data` (multi-value strings are already split by then), while WHETHER - // to check is decided by the caller's own raw value being non-empty. - if (!supplied || isEmptyReferenceValue((supplied as Record)[name])) continue; - if (!(name in data)) continue; - const def = fields[name]; - const target = referenceTargetOf(def); - if (!target) continue; - const raw = (data as Record)[name]; - const values = Array.isArray(raw) ? raw : [raw]; - for (const v of values) { - if (v === null || v === undefined || v === '') continue; - if (typeof v === 'object') continue; - // [#22519] A master-detail header must also be one the caller can read. - const resolved = def?.type === 'master_detail' - ? await this.masterDetailHeaderExists(target, v, context) - : await this.referenceExists(target, v, context); - if (resolved === false) { - failures.push(buildFieldError( - { - field: name, - code: 'reference_not_found', - def, - value: String(v), - constraint: { target }, - }, - msgCtx as any, - )); - } - } - } - if (failures.length > 0) throw new ValidationError(failures); + // [#25522 cut 10] The body moved to `engine-referential-integrity.ts` (see `ReferentialIntegrityHost`). + const host: ThisParameterType = { + masterDetailHeaderExists: (...a) => this.masterDetailHeaderExists(...a), + referenceExists: (...a) => this.referenceExists(...a), + }; + return assertReferencesResolve.call(host, schema, data, supplied, context, msgCtx); } /** @@ -10098,29 +10016,13 @@ export class ObjectQL implements IObjectQLEngine { ids: ReadonlyArray, context: ExecutionContext | undefined, ): Promise> { - const asked = new Map(); - for (const id of ids) if (id != null) asked.set(String(id), id); - if (context?.isSystem || asked.size === 0) return new Set(asked.keys()); - // One id is asked by equality, the predicate shape the elevated probe - // already sends; a batch by `$in`, as read 2 of the batch reads it. - const values = [...asked.values()]; - try { - const rows = await this.find(master, { - where: { id: values.length === 1 ? values[0] : { $in: values } }, - fields: ['id'], - context, - } as EngineQueryOptions) as Array>; - const served = new Set(); - for (const row of Array.isArray(rows) ? rows : []) { - if (row?.id != null && asked.has(String(row.id))) served.add(String(row.id)); - } - return served; - } catch (err) { - this.logger?.warn?.('the caller\'s read of the master-detail header failed — the header binds as a missing header', { - object: master, error: err, - }); - return new Set(); - } + // [#25522 cut 10] The body moved to `engine-referential-integrity.ts` (see `ReferentialIntegrityHost`). + const readLogger = () => this.logger; + const host: ThisParameterType = { + find: (...a) => this.find(...a), + get logger() { return readLogger(); }, + }; + return masterDetailHeadersCallerReads.call(host, master, ids, context); } /** diff --git a/packages/objectql/src/federated-injected-column-readers.test.ts b/packages/objectql/src/federated-injected-column-readers.test.ts index b76881c04fd..24908f43341 100644 --- a/packages/objectql/src/federated-injected-column-readers.test.ts +++ b/packages/objectql/src/federated-injected-column-readers.test.ts @@ -248,7 +248,7 @@ const READERS: Record = { }, // ── Seams that are not a storage read of a federated object's column ───── - 'engine.ts#assertReferencesResolve :: referenceTargetOf()': { + 'engine-referential-integrity.ts#assertReferencesResolve :: referenceTargetOf()': { disposition: 'target-by-id', why: "the id a caller wrote into a reference field is looked up on the TARGET object", },