fix(objectql)!: an update compares a value the caller sends only against the stored row as the caller reads it

.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md

@@ -0,0 +1,16 @@
+---
+'@objectstack/objectql': patch
+---
+
+fix(objectql)!: an update compares a value the caller sends only against the stored row as the caller's own read door serves it — a `secret` reference and a master-detail repoint
+
+Clause-②: yes (narrowing) — a non-system update that sends a `secret:` reference is refused even when the record already holds it, and a non-system update naming the master-detail FK of a detail with a parent-scoped `requiredWhen`, in a FK column it may edit but not read, no longer lets a row that already lacks a required value rest under the header it names.
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes. Both refusals are answers the engine already gives — VALIDATION_ERROR for a secret reference, the field's required finding for requiredWhen — so there is nothing for a migration to rewrite and no tombstone. -->
+
+**BREAKING** accept-set narrowing at two update seams of `@objectstack/objectql`, served by every write door, by id and in bulk alike. A system write is unchanged at both.
+
+**A `secret` reference.** A non-system update keeps a `secret:` reference only where every record it lands on serves that reference, in that column, to the caller's own read door. The read path serves a `secret` column masked to every caller, so after the upgrade every non-system update that sends a reference is refused with `400 VALIDATION_ERROR` and nothing is written, with one answer whatever the reference names and whatever the column holds — the record's own reference included, which was accepted and kept before. The same holds for a reference a `beforeUpdate` hook writes onto the payload, such as one that spreads the stored row (`ctx.previous`) back into it. To leave a secret unchanged, omit the field or send back the read mask; a hook that writes the stored row back omits its `secret` columns. A new value is still sealed, `null` still clears, and the system principal keeps the pass-through.
+
+**A master-detail repoint.** On a detail whose fields declare a parent-scoped `requiredWhen`, the ADR-0113 pre-check of an update that names the master-detail FK reads the header the row hung off from the stored FK only as the caller's own read door serves it. Where the caller may edit the FK but not read it (a field permission `{ readable: false, editable: true }`), the prior header is unknown to it and is bound as an unresolved one, so the pre-check cannot prove a violation was already stored: a row that already lacks a value its requirement asks for, and that such a caller names a header for, is refused with the field's `required` finding whatever header the row hung off — including the header it already hangs off, which used to rest. A caller who can read the FK is judged as before, and an update that does not name the FK is unchanged. The `validate()` preview of such an update binds the prior header the same way, so it predicts the same `required` refusal instead of admitting the row.

content/docs/permissions/system-context.mdx

@@ -138,7 +138,7 @@ bugs — it is the protection or the side effect that silently does not happen.
 | 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 | `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`, `packages/plugins/plugin-security/src/grant-permission-set-name.ts#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`, `packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts#onUpdate` |
-| 23e | **A `secret` field takes a stored reference as given** — a value already spelled `secret:` is stored unchanged | objectql | Get: a system writer may copy a `secret:` reference onto a record as it stands (seed replay, a migration). Every other caller may only seal a new value or keep its own record's ref: on a create any such value is refused, and on an update it is accepted only when every record the write lands on already holds exactly that reference in that column, which is then left as it is, as an echoed read mask is. Any other reference is refused `400 VALIDATION_ERROR` before anything is written, with one answer whatever it names. Lose: the check that the reference is the record's own — a system writer that copies one is trusted to mean it. Inbound HTTP never carries the flag | `encryptSecretFields` |
+| 23e | **A `secret` field takes a stored reference as given** — a value already spelled `secret:` is stored unchanged | objectql | Get: a system writer may copy a `secret:` reference onto a record as it stands (seed replay, a migration). Every other caller may only seal a new value or keep its own record's ref, and its own is the value it reads in that column: on a create any such value is refused, and on an update it would be kept only where every record the write lands on serves exactly that reference to this caller's own read door. The read path serves a `secret` column masked to every caller, so every reference such a caller sends is refused `400 VALIDATION_ERROR` before anything is written, with one answer whatever it names and whatever the column holds; an echoed read mask still leaves the column as it is. Lose: the check that the reference is the record's own — a system writer that copies one is trusted to mean it. Inbound HTTP never carries the flag | `encryptSecretFields` |
 | 23f | **Agent ceiling grant check skipped** — a `sys_user_permission_set` write naming an MCP agent ceiling set is not refused | plugin-security | Get: a system writer is not judged. The platform-admin promotion and the organization-admin reconcile name their sets in code, and neither is a ceiling; the self-registration grant writes the configured `audience.selfRegistrationPermissionSet`, which the audience validation (`assertAudienceConfig`, plugin-auth) refuses when it names an agent ceiling set. Lose: seed replay and the development-only demo-staff seed write the sets their stack declares, unjudged, so a seed that names an agent ceiling set stores it. A non-system insert, or a non-system update whose resulting row names one, is refused `400 VALIDATION_FAILED` / `invalid_value` at `permission_set`, for every caller, a platform administrator included; a delete is not judged (ADR-0090 D10) | `packages/plugins/plugin-security/src/agent-ceiling-grant-refusal.ts#onInsert`, `packages/plugins/plugin-security/src/agent-ceiling-grant-refusal.ts#onUpdate` |
 | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `buildDriverOptions` |
 | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `packages/plugins/plugin-security/src/system-write-guard.ts#assertEngineOwnedWriteAllowed` |

packages/objectql/src/engine-master-detail-related.ts

@@ -54,6 +54,11 @@
  * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`),
  * and the class keeps none for it; a public or protected method's docblock
  * stays on the class, the contract, and its function keeps the placeholder.
+ *
+ * One body was written here rather than moved: `priorMasterIdsCallerReads`,
+ * the stored master an update's pre-check reads, as the caller's read door
+ * serves the FK. Its private method on the class is a delegation of the same
+ * shape.
  */
 
 import { referenceTargetOf } from '@objectstack/spec/data';
@@ -70,6 +75,7 @@ import type { RelatedFieldBinding, RelatedRecordBinding } from './validation/rul
 import { resolveMasterDetailRelation } from './master-detail.js';
 import { isFederatedObject } from './federated-object.js';
 import { masterIdOf, servedAsStored, staticReadonlyStoredView, undeclaredRelatedFields } from './engine.js';
+import { storedReferencesCallerReads } from './engine-referential-integrity.js';
 import type { ObjectQL } from './engine.js';
 
 /**
@@ -248,6 +254,14 @@ export async function settleMasterDetailLanding<H>(
  * merged with the patch on an update, so the patch's FK wins over the stored
  * one; `storedRows[i]` names the header a repoint leaves.
  *
+ * ⛔ `storedRows[i]` holds only the columns the read door served this caller
+ * as stored, so a stored FK it was not served is absent there. When the patch
+ * names the FK over such a row, the header the row leaves is UNKNOWN to this
+ * caller, and `previousParents[i]` is `null`, the unresolved header — the one
+ * fixed binding the write gives it ({@link priorMasterIdsCallerReads}), so the
+ * pre-check's verdict is the write's and never depends on what the column
+ * holds. A patch that does not name the FK reads no second header.
+ *
  * ## Read under the caller's access, not elevated
  *
  * A header read elevated here would answer a predicate over a row the caller
@@ -290,6 +304,13 @@ export async function resolvePreviewParents(
   const priorIds = storedRows.map((stored) => (rel && stored ? masterIdOf(rel.fk, null, stored) : undefined));
   // The write resolves a second header only for a repoint (`repointsMaster`).
   const repoints = priorIds.map((prior, i) => prior != null && landingIds[i] !== prior);
+  // The patch names the FK over a stored row whose FK this caller was not
+  // served as stored: the prior header is unknown, as the write binds it.
+  const priorUnknown = storedRows.map((stored, i) => {
+    const view = views[i];
+    return rel != null && stored != null && !Object.prototype.hasOwnProperty.call(stored, rel.fk)
+      && view != null && typeof view === 'object' && rel.fk in view;
+  });
   const ids = new Set<string>();
   landingIds.forEach((id) => { if (id != null) ids.add(String(id)); });
   priorIds.forEach((id, i) => { if (repoints[i]) ids.add(String(id)); });
@@ -334,10 +355,50 @@ export async function resolvePreviewParents(
   const headerOf = (id: string | number | undefined) => (id == null ? null : (headers.get(String(id)) ?? null));
   return {
     parents: landingIds.map(headerOf),
-    previousParents: priorIds.map((id, i) => (repoints[i] ? headerOf(id) : undefined)),
+    previousParents: priorIds.map((id, i) => (priorUnknown[i] ? null : repoints[i] ? headerOf(id) : undefined)),
   };
 }
 
+/**
+ * The master each of an update's stored rows hangs off, as THIS caller's own
+ * read door serves the master-detail FK: the header the ADR-0113 pre-check of
+ * a parent-scoped `requiredWhen` reads as `previousParent` when the write
+ * names the FK.
+ *
+ * `{ id }` for a row whose FK the read door serves this caller exactly as
+ * stored ({@link storedReferencesCallerReads}, the update's one projection of
+ * stored rows onto the caller's read): `id` is the stored master, or
+ * `undefined` for a row that hangs off none. `null` for a row whose FK it does
+ * not — hidden, or served transformed, or a row the door does not return: the
+ * prior master is UNKNOWN to this caller, and the write binds one fixed header
+ * for it, the unresolved one, so neither whether the pre-check reads a second
+ * header nor which one it reads depends on what the column holds. A SYSTEM
+ * caller is answered from the rows as stored, as before.
+ *
+ * A stored row that carries no FK key at all is read as holding no master, as
+ * `masterIdOf` reads it: the key is spelled out as empty before the
+ * projection, so a system caller's answer for it is `{ id: undefined }`.
+ */
+export async function priorMasterIdsCallerReads(
+  this: Pick<MasterDetailRelatedHost, 'find' | 'logger'>,
+  schema: unknown,
+  rows: ReadonlyArray<Record<string, unknown> | null | undefined>,
+  context: ExecutionContext | undefined,
+): Promise<Array<{ readonly id: string | number | undefined } | null>> {
+  const rel = resolveMasterDetailRelation(schema as any);
+  if (!rel) return rows.map(() => null);
+  const stored = rows.map((row) => {
+    const own = row && typeof row === 'object' ? row : {};
+    return Object.prototype.hasOwnProperty.call(own, rel.fk) ? own : { ...own, [rel.fk]: undefined };
+  });
+  const served = await storedReferencesCallerReads.call(
+    this, (schema as { name?: unknown } | null | undefined)?.name, stored, [rel.fk], context,
+  );
+  return served.map((view) => (
+    view && Object.prototype.hasOwnProperty.call(view, rel.fk) ? { id: masterIdOf(rel.fk, null, view) } : null
+  ));
+}
+
 /**
  * [#18682] Resolve the related rows this object's PREDICATE rules read one
  * hop through a reference field, for a whole batch of rows at once.