From 73011af5578e65f5b02dbb6a11dc79e4712a6e72 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:26:33 +0000 Subject: [PATCH 01/26] fix(objectql)!: a non-system lookup or user write names only a record the caller can read The write's reference check now asks the caller's own read door for every `lookup` and `user` value a non-system caller supplies, exactly as it already did for a `master_detail` header: a target the read door does not return answers `reference_not_found`, the answer of a target that exists nowhere. System writes keep their exemption; a value the row already stores is not judged again. Claude-Session: https://claude.ai/code/session_01ADHkmSZhbvtSoFM7YTkq6B Co-authored-by: Claude --- .../src/engine-referential-integrity.ts | 25 +- ...ne-unreadable-master-detail-header.test.ts | 18 +- ...engine-unreadable-reference-target.test.ts | 296 ++++++++++++++++++ packages/objectql/src/engine.ts | 73 +++-- 4 files changed, 364 insertions(+), 48 deletions(-) create mode 100644 packages/objectql/src/engine-unreadable-reference-target.test.ts diff --git a/packages/objectql/src/engine-referential-integrity.ts b/packages/objectql/src/engine-referential-integrity.ts index ff48466164e..78e70270c6e 100644 --- a/packages/objectql/src/engine-referential-integrity.ts +++ b/packages/objectql/src/engine-referential-integrity.ts @@ -24,7 +24,7 @@ * that call a module helper `engine.ts` still needs elsewhere * (`settleMasterDetailLanding`, `resolvePreviewParents`, * `resolvePredicateRelated`, `resolveOneHopRelated`); and - * `masterDetailHeaderExists`, `materializeParentHeader` and + * `callerReadableReferenceExists`, `materializeParentHeader` and * `masterDeclaredFields`, whose bodies are no longer than the host literal * moving them would cost. * @@ -36,7 +36,7 @@ * 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 + * they call (`find`, `callerReadableReferenceExists`, `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. * @@ -62,7 +62,7 @@ import type { ObjectQL } from './engine.js'; export interface ReferentialIntegrityHost { readonly logger: ObjectQL['logger']; find: ObjectQL['find']; - masterDetailHeaderExists: ObjectQL['masterDetailHeaderExists']; + callerReadableReferenceExists: ObjectQL['callerReadableReferenceExists']; referenceExists: ObjectQL['referenceExists']; } @@ -79,9 +79,19 @@ function isEmptyReferenceValue(v: unknown): boolean { return false; } +/** + * [#29029] The reference types whose target a non-system write may name only + * when the caller's own read door returns it: `master_detail` (#22519), and + * `lookup` and `user` with it. A target the caller cannot read answers + * `reference_not_found`, exactly as a target that exists nowhere. `tree`, the + * one other reference type, keeps the elevated existence probe: the ruling + * that tightened this set (record 6573561091) names `lookup` and `user` only. + */ +const CALLER_READ_REFERENCE_TYPES: ReadonlySet = new Set(['master_detail', 'lookup', 'user']); + /** The body of `ObjectQL#assertReferencesResolve`; the method's docblock there is its contract. */ export async function assertReferencesResolve( - this: Pick, + this: Pick, schema: any, data: Record | null | undefined, supplied: Record | null | undefined, @@ -152,9 +162,10 @@ export async function assertReferencesResolve( 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) + // [#22519, #29029] A master-detail header, a lookup target and a user + // must also be one the caller can read. + const resolved = CALLER_READ_REFERENCE_TYPES.has(def?.type) + ? await this.callerReadableReferenceExists(target, v, context) : await this.referenceExists(target, v, context); if (resolved === false) refused.push({ name, def, value: v, target }); } diff --git a/packages/objectql/src/engine-unreadable-master-detail-header.test.ts b/packages/objectql/src/engine-unreadable-master-detail-header.test.ts index 8d11957bab3..0182129b51d 100644 --- a/packages/objectql/src/engine-unreadable-master-detail-header.test.ts +++ b/packages/objectql/src/engine-unreadable-master-detail-header.test.ts @@ -45,8 +45,9 @@ * the mechanism: the read door first, then the elevated read; * CONTROLS: a readable header unchanged (its stored values judged, a masked * column included); a nonexistent id; a `controlled_by_parent` detail - * unchanged; a system caller unchanged (no read door asked); a plain `lookup` - * to the same object keeps its elevated probe (not this rule's). + * unchanged; a system caller unchanged (no read door asked). [#29029] A plain + * `lookup` to the same object takes the same rule since ruling record + * 6573561091 (its own pins: `engine-unreadable-reference-target.test.ts`). */ import { describe, it, expect, beforeEach } from 'vitest'; @@ -119,7 +120,7 @@ const CBP_LINE_OBJECT = { fields: { ...REQ_LINE_OBJECT.fields, invoice: { ...invoiceField, required: true } }, }; -/** A plain `lookup` to the same object: its probe is not this rule's. */ +/** [#29029] A plain `lookup` to the same object: the same rule since ruling record 6573561091. */ const LOOKUP_NOTE_OBJECT = { name: LOOKUP_NOTE, label: 'Lookup note', @@ -338,13 +339,14 @@ describe('[#22519] an unreadable master-detail header answers what a nonexistent expect([...storeFor(PLAIN_LINE).values()].some((r) => r.invoice === 'h_mine_locked')).toBe(true); }); - it('control: a plain lookup to the same object keeps its elevated probe — not this rule\'s', async () => { - await engine.insert(LOOKUP_NOTE, { invoice_ref: 'h_other_open' }, { context: ALICE } as any); - expect([...storeFor(LOOKUP_NOTE).values()].map((r) => r.invoice_ref)).toEqual(['h_other_open']); + it('[#29029] a plain lookup to the same object takes the same rule: an unreadable target is refused as a nonexistent one is', async () => { const nowhere = await refusalOf(() => engine.insert(LOOKUP_NOTE, { invoice_ref: NOWHERE }, { context: ALICE } as any)); expect(envelopeOf(nowhere)).toEqual({ ...NOT_FOUND, fields: [{ field: 'invoice_ref', code: 'reference_not_found' }] }); - // No read door was asked for it. - expect(headerReads.filter((r) => !r.elevated)).toEqual([]); + const unreadable = await refusalOf(() => engine.insert(LOOKUP_NOTE, { invoice_ref: 'h_other_open' }, { context: ALICE } as any)); + expect(envelopeOf(unreadable)).toEqual(envelopeOf(nowhere)); + expect(storeFor(LOOKUP_NOTE).size).toBe(0); + // The caller's read door was asked for it. + expect(headerReads.some((r) => !r.elevated)).toBe(true); }); }); diff --git a/packages/objectql/src/engine-unreadable-reference-target.test.ts b/packages/objectql/src/engine-unreadable-reference-target.test.ts new file mode 100644 index 00000000000..3bca0035a95 --- /dev/null +++ b/packages/objectql/src/engine-unreadable-reference-target.test.ts @@ -0,0 +1,296 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#29029] A non-system write may name, in a `lookup` or `user` field, only a + * record the caller's own read door returns — the rule #22519 (ruling A) set + * for a `master_detail` header, made the default for `lookup` and `user` by + * ruling record 6573561091, with no field-level opt-out. A target the caller + * cannot read answers `reference_not_found`, exactly what a target that exists + * nowhere answers. A system write keeps its exemption. + * + * ## The rig + * + * A store-backed driver, and a synthetic read scope shaped like the + * security/sharing middlewares (the one the #22519 pins use): a non-system + * caller reads only the target rows it owns, on both target objects (a plain + * object and `sys_user`). A second caller's read of either target object is + * refused outright, as an object-level denial is. + * + * ## Pins + * + * - a `lookup` and a `user` value: insert, batch insert, update by id and + * bulk update — a target the caller cannot read answers what a target that + * exists nowhere answers (`code`, `status`, field code and the sentence); + * - a refused read of the target object answers the same; + * - a multi-valued field: one unreadable member refuses the whole write; + * - the mechanism: the caller's read door first, then the elevated probe; + * CONTROLS: a readable target lands; a system write naming an unreadable + * target lands and asks no read door; an update that carries a stored value + * back is not judged again (#29062), as for a `master_detail` header. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { resolveThrownHttpError } from '@objectstack/types'; +import { ObjectQL } from './engine.js'; +import { ValidationError } from './validation/record-validator.js'; + +const TARGET = 'urt_duty'; +const USER = 'sys_user'; +const ITEM = 'urt_task'; + +const ownedTarget = (name: string, label: string) => ({ + name, + label, + fields: { + name: { name: 'name', label: 'Name', type: 'text' }, + owner_name: { name: 'owner_name', label: 'Owner', type: 'text' }, + }, +}); + +const ITEM_OBJECT = { + name: ITEM, + label: 'Task', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + duty: { name: 'duty', label: 'Duty', type: 'lookup', reference: TARGET }, + duties: { name: 'duties', label: 'Duties', type: 'lookup', reference: TARGET, multiple: true }, + assignee: { name: 'assignee', label: 'Assignee', type: 'user' }, + watchers: { name: 'watchers', label: 'Watchers', type: 'user', multiple: true }, + }, +}; + +/** A store-backed driver: what is stored is what a later read answers. */ +function makeStoreDriver() { + const stores = new Map>>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + const matches = (row: any, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]: [string, any]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k.startsWith('$')) throw new Error(`store driver: unsupported combinator ${k}`); + if (v !== null && typeof v === 'object' && !Array.isArray(v)) { + return Object.entries(v).every(([op, target]) => { + if (op === '$eq') return row?.[k] === target; + if (op === '$in') return Array.isArray(target) && target.includes(row?.[k]); + throw new Error(`store driver: unsupported operator ${op}`); + }); + } + return row?.[k] === v; + }); + }; + let n = 0; + const driver: any = { + name: 'urt-store', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async syncSchema() {}, + async find(o: string, ast: any) { + const hits = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + return (typeof ast?.limit === 'number' ? hits.slice(0, ast.limit) : hits).map((r) => ({ ...r })); + }, + async findOne(o: string, ast: any) { + for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return { ...r }; + return null; + }, + async create(o: string, data: Record) { + n += 1; + const id = (data.id as string) ?? `rec_${n}`; + const row = { ...data, id }; + storeFor(o).set(id, row); + return { ...row }; + }, + async update(o: string, id: string, data: Record) { + const row = { ...storeFor(o).get(id), ...data, id }; + storeFor(o).set(id, row); + return { ...row }; + }, + async updateMany(o: string, ast: any, data: Record) { + const hits = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + for (const r of hits) storeFor(o).set(r.id, { ...r, ...data, id: r.id }); + return hits.length; + }, + async delete(o: string, id: string) { return storeFor(o).delete(id); }, + async count(o: string, ast: any) { + return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)).length; + }, + }; + return { driver, storeFor }; +} + +/** The refusal an object-level read denial throws — `PermissionDeniedError`'s declared envelope. */ +const readRefusal = () => Object.assign(new Error('[Security] Access denied: read on the target object'), { + name: 'PermissionDeniedError', code: 'PERMISSION_DENIED', status: 403, statusCode: 403, +}); + +/** Every target read, with whether it ran under the caller's own context or elevated. */ +type TargetRead = { object: string; operation: string; elevated: boolean }; + +function makeEngine() { + const store = makeStoreDriver(); + const engine = new ObjectQL(); + engine.registerDriver(store.driver, true); + engine.registerApp({ + id: 'urt', + name: 'Unreadable reference target', + objects: [ownedTarget(TARGET, 'Duty'), ownedTarget(USER, 'User'), ITEM_OBJECT], + } as any); + const targetReads: TargetRead[] = []; + engine.registerMiddleware(async (ctx: any, next: () => Promise) => { + if ([TARGET, USER].includes(ctx.object) && ['find', 'findOne'].includes(ctx.operation)) { + const elevated = ctx.context?.isSystem === true; + targetReads.push({ object: ctx.object, operation: ctx.operation, elevated }); + const userId = ctx.context?.userId; + if (!elevated && userId === 'carol') throw readRefusal(); + // Read scope shaped like the security/sharing middlewares: a non-system + // caller reads only the target rows it owns. + if (!elevated && userId) { + const scoped = { owner_name: userId }; + const ast: any = ctx.ast ?? { object: ctx.object }; + ast.where = ast.where ? { $and: [ast.where, scoped] } : scoped; + ctx.ast = ast; + } + } + await next(); + }); + for (const object of [TARGET, USER]) { + const rows = store.storeFor(object); + rows.set('t_mine', { id: 't_mine', name: 'mine', owner_name: 'alice' }); + rows.set('t_mine_2', { id: 't_mine_2', name: 'mine too', owner_name: 'alice' }); + rows.set('t_other', { id: 't_other', name: 'other', owner_name: 'bob' }); + } + return { engine, targetReads, ...store }; +} + +/** Alice may write every task and reads only the target rows she owns. */ +const ALICE = { userId: 'alice' }; +/** Carol's read of either target object is refused outright. */ +const CAROL = { userId: 'carol' }; +/** Seed replay / boot. */ +const SYSTEM = { isSystem: true, userId: 'alice' }; + +const NOWHERE = 't_nowhere'; +const UNREADABLE = 't_other'; + +type Envelope = { status: number; code: string; fields: Array<{ field: string; code: string }> }; + +/** The refusal as every HTTP door reads it (ADR-0112), plus its field codes. */ +function envelopeOf(err: unknown): Envelope { + const thrown = resolveThrownHttpError(err); + const fields = ((err as any)?.fields ?? []).map((f: any) => ({ field: f.field, code: f.code })); + return { status: thrown.status, code: thrown.code, fields }; +} + +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e; + } + throw new Error('expected the write to be refused, but it succeeded'); +} + +/** The message with the target id taken out: what else a caller could compare. */ +const sentence = (err: unknown, id: string) => String((err as Error).message).split(id).join('ID'); + +const notFound = (field: string): Envelope => ({ + status: 400, code: 'VALIDATION_FAILED', fields: [{ field, code: 'reference_not_found' }], +}); + +type Door = 'insert' | 'batch insert' | 'update by id' | 'bulk update'; +const DOORS: Door[] = ['insert', 'batch insert', 'update by id', 'bulk update']; +const FIELDS = [ + { field: 'duty', kind: 'lookup', target: TARGET }, + { field: 'assignee', kind: 'user', target: USER }, +] as const; + +describe('[#29029] a non-system lookup or user write may name only a record the caller can read', () => { + let engine: ObjectQL; + let targetReads: TargetRead[]; + let storeFor: (o: string) => Map>; + + beforeEach(() => { + ({ engine, targetReads, storeFor } = makeEngine()); + storeFor(ITEM).set('k1', { id: 'k1', title: 'stored' }); + }); + + const write = (door: Door, payload: Record, context: Record = ALICE) => { + if (door === 'insert') return engine.insert(ITEM, { title: 'new', ...payload }, { context } as any); + if (door === 'batch insert') { + return engine.insert(ITEM, [{ title: 'first' }, { title: 'second', ...payload }], { context } as any); + } + if (door === 'update by id') return engine.update(ITEM, payload, { where: { id: 'k1' }, context } as any); + return engine.update(ITEM, payload, { where: { id: { $in: ['k1'] } }, multi: true, context } as any); + }; + + const storedItems = () => [...storeFor(ITEM).values()]; + + describe.each(FIELDS)('a $kind field', ({ field }) => { + it.each(DOORS)('%s — a target the caller cannot read is refused as a target that exists nowhere is', async (door) => { + const nowhere = await refusalOf(() => write(door, { [field]: NOWHERE })); + expect(nowhere).toBeInstanceOf(ValidationError); + expect(envelopeOf(nowhere)).toEqual(notFound(field)); + const unreadable = await refusalOf(() => write(door, { [field]: UNREADABLE })); + expect(envelopeOf(unreadable)).toEqual(notFound(field)); + expect(sentence(unreadable, UNREADABLE)).toBe(sentence(nowhere, NOWHERE)); + // Nothing written: the stored task is untouched, and no task was added. + expect(storedItems()).toEqual([{ id: 'k1', title: 'stored' }]); + }); + + it.each(DOORS)('%s — a refused read of the target object answers as a target that exists nowhere', async (door) => { + const nowhere = envelopeOf(await refusalOf(() => write(door, { [field]: NOWHERE }, CAROL))); + expect(nowhere).toEqual(notFound(field)); + expect(envelopeOf(await refusalOf(() => write(door, { [field]: 't_mine' }, CAROL)))).toEqual(nowhere); + }); + + it.each(DOORS)('%s — control: a target the caller can read lands', async (door) => { + await write(door, { [field]: 't_mine' }); + expect(storedItems().some((r) => r[field] === 't_mine')).toBe(true); + }); + + it.each(DOORS)('%s — control: a system write naming a target the caller cannot read lands, and asks no read door', async (door) => { + await write(door, { [field]: UNREADABLE }, SYSTEM); + expect(storedItems().some((r) => r[field] === UNREADABLE)).toBe(true); + expect(targetReads.length).toBeGreaterThan(0); + expect(targetReads.filter((r) => !r.elevated)).toEqual([]); + }); + + it.each(['update by id', 'bulk update'] as const)('%s — control: a stored value carried back is not judged again', async (door) => { + storeFor(ITEM).set('k1', { id: 'k1', title: 'stored', [field]: UNREADABLE }); + await write(door, { title: 'renamed', [field]: UNREADABLE }); + expect(storedItems()).toEqual([{ id: 'k1', title: 'renamed', [field]: UNREADABLE }]); + }); + + it('the mechanism: the caller\'s read door first, then the elevated probe', async () => { + await write('insert', { [field]: 't_mine' }); + expect(targetReads.map(({ operation, elevated }) => ({ operation, elevated }))).toEqual([ + { operation: 'find', elevated: false }, + { operation: 'findOne', elevated: true }, + ]); + }); + }); + + describe.each([ + { field: 'duties', kind: 'lookup' }, + { field: 'watchers', kind: 'user' }, + ] as const)('a multi-valued $kind field', ({ field }) => { + it.each(['insert', 'update by id'] as const)('%s — one member the caller cannot read refuses the whole write', async (door) => { + const err = await refusalOf(() => write(door, { [field]: ['t_mine', UNREADABLE, 't_mine_2'] })); + expect(envelopeOf(err)).toEqual(notFound(field)); + expect(storedItems()).toEqual([{ id: 'k1', title: 'stored' }]); + }); + + it.each(['insert', 'update by id'] as const)('%s — control: every member readable lands', async (door) => { + await write(door, { [field]: ['t_mine', 't_mine_2'] }); + expect(storedItems().some((r) => Array.isArray(r[field]) && r[field].length === 2)).toBe(true); + }); + + it('update — control: a stored member carried back beside a new readable one is not judged again', async () => { + storeFor(ITEM).set('k1', { id: 'k1', title: 'stored', [field]: [UNREADABLE] }); + await write('update by id', { [field]: [UNREADABLE, 't_mine'] }); + expect(storeFor(ITEM).get('k1')?.[field]).toEqual([UNREADABLE, 't_mine']); + }); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 899a02084e6..730c0513d11 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -10426,15 +10426,14 @@ export class ObjectQL implements IObjectQLEngine { * to answer for. A value new to any of the rows is refused as before, and * an insert has no stored rows, so every value it supplies is judged. * - * ## Why the probe bypasses RLS — and why it keeps the tenant wall + * ## Why the existence probe bypasses RLS — and why it keeps the tenant wall * * Existence is a fact about the database, not about the caller's ROW-LEVEL - * visibility — the same distinction the #4435 existence probe turns on. A - * probe under the caller's own context would refuse a link to a permission - * set the caller cannot READ, which is ordinary in an RLS-scoped deployment - * and would make the platform's own admin flows fail. Whether the caller may - * create the binding at all is the RBAC/RLS layer's decision, made where it - * already is. So the probe is elevated: `isSystem` bypasses RBAC, RLS and FLS. + * visibility — the same distinction the #4435 existence probe turns on. So + * the existence probe ({@link referenceExists}) is elevated: `isSystem` + * bypasses RBAC, RLS and FLS. Whether a NON-system caller may name the row is + * the separate question the next section answers, and it is asked first, of + * the caller's own read door, for every reference type but `tree`. * * That argument is about visibility, never about TENANCY, and the elevation * is therefore `sudo()`-shaped — `{ ...context, isSystem: true }`, see @@ -10453,17 +10452,25 @@ export class ObjectQL implements IObjectQLEngine { * tenant's (ADR-0131 D1): the driver's scope is the organization equality, * with no `IS NULL` arm, so an org-bound caller's probe does not find one. * - * ## …except a master-detail header, which the caller must be able to read (#22519) - * - * A `master_detail` field names the detail's HEADER, and a write of a detail - * whose header the caller cannot read answers exactly what a write naming a - * header that exists nowhere answers. Nothing upstream of this check settles - * that: ADR-0055 §3 gates a detail write on its header (master edit) only for - * a `controlled_by_parent` detail, so an elevated probe alone would tell such - * a caller "exists" apart from "exists nowhere". So for a `master_detail` - * field the caller's read door is asked first ({@link masterDetailHeaderExists}), - * and a header it does not return is `reference_not_found`. ⛔ A plain - * `lookup` keeps the elevated probe above, unchanged. + * ## A non-system caller may name only a row its own read door returns (#22519, #29029) + * + * A `master_detail`, `lookup` or `user` value a non-system caller supplies + * must name a row the caller's own read door returns; a write naming a row + * the caller cannot read answers exactly what a write naming a row that + * exists nowhere answers: `reference_not_found`. So for those three types the + * caller's read door is asked first ({@link callerReadableReferenceExists}), + * then the elevated, tenant-kept existence probe for a row it returned. A + * multi-valued field is judged per member: one unreadable member refuses the + * write. Nothing upstream of this check settles it: ADR-0055 §3 gates a + * detail write on its header (master edit) only for a `controlled_by_parent` + * detail, and nothing gates a lookup on its target, so an elevated probe + * alone would let a caller bind its row to a row it has no access to, and + * tell "exists" apart from "exists nowhere". #22519 (ruling A) set the rule + * for `master_detail`; ruling record 6573561091 extended it to `lookup` and + * `user` as the default, with no field-level opt-out. A system caller keeps + * the exemption above (its read door returns every row), and a value the + * row already stores is not judged again (#29062). `tree` keeps the + * elevated probe alone. * * Fails OPEN when the target cannot be checked (unregistered object, no * driver, a probe that throws): an integrity check that cannot run must not @@ -10483,7 +10490,7 @@ export class ObjectQL implements IObjectQLEngine { ): Promise { // [#25522 cut 10] The body moved to `engine-referential-integrity.ts` (see `ReferentialIntegrityHost`). const host: ThisParameterType = { - masterDetailHeaderExists: (...a) => this.masterDetailHeaderExists(...a), + callerReadableReferenceExists: (...a) => this.callerReadableReferenceExists(...a), referenceExists: (...a) => this.referenceExists(...a), }; return assertReferencesResolve.call(host, schema, data, supplied, context, msgCtx, storedRows); @@ -10526,24 +10533,24 @@ export class ObjectQL implements IObjectQLEngine { } /** - * [#22519] The write's reference check for a `master_detail` field: does `id` - * name a header THIS CALLER can read? The write's two header reads, as - * {@link resolveMasterDetailParent} reads them: the caller's read door - * ({@link masterDetailHeadersCallerReads}), then the elevated, tenant-kept - * {@link referenceExists} for a header it returned. A header the read door - * does not return is `false` — `reference_not_found`, exactly as a header - * that exists nowhere. + * [#22519, #29029] The write's reference check for a `master_detail`, + * `lookup` or `user` field: does `id` name a row THIS CALLER can read? The + * two reads {@link resolveMasterDetailParent} reads a header with: the + * caller's read door ({@link masterDetailHeadersCallerReads}), then the + * elevated, tenant-kept {@link referenceExists} for a row it returned. A row + * the read door does not return is `false` — `reference_not_found`, exactly + * as a row that exists nowhere. * * `null` (fail-open) only where {@link referenceExists} answers it, and for - * an unregistered master, which has no read door to ask either. A SYSTEM + * an unregistered target, which has no read door to ask either. A SYSTEM * caller reaches it only as a flow run's elevated write, or a write that - * write sets off (#28881): its read door returns every header - * ({@link masterDetailHeadersCallerReads}), so the header's existence in the + * write sets off (#28881): its read door returns every row + * ({@link masterDetailHeadersCallerReads}), so the row's existence in the * caller's tenant is what is asked. * - * ⛔ `master_detail` only — the plain `lookup` probe is not this rule's. + * ⛔ Not `tree` — its value keeps the elevated existence probe alone. */ - private async masterDetailHeaderExists( + private async callerReadableReferenceExists( target: string, id: unknown, context: ExecutionContext | undefined, @@ -10565,7 +10572,7 @@ export class ObjectQL implements IObjectQLEngine { * * A header it does not return binds as a header that exists nowhere binds: * the reference check answers `reference_not_found` for it - * ({@link masterDetailHeaderExists}), and the write's header resolution + * ({@link callerReadableReferenceExists}), and the write's header resolution * leaves `parent` unbound for it ({@link resolveMasterDetailParent}, * {@link resolveMasterDetailParents}). A read that fails — a refusal of the * read itself included — returned no header, so every header binds as @@ -10696,7 +10703,7 @@ export class ObjectQL implements IObjectQLEngine { * not return binds as a missing header binds: `null`, so a parent-scoped * `requiredWhen` / `readonlyWhen` meets an unbound `parent`, and the * reference check answers `reference_not_found` for it - * ({@link masterDetailHeaderExists}). + * ({@link callerReadableReferenceExists}). * 2. WHAT THE HEADER HOLDS — the write's own ELEVATED read below, for a * header read 1 returned only. The lock is a data-integrity property of * the header's state, so a header the caller can read is judged on its From 836064c41c4682df4112a71ebb9bdc61a1707e44 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:30:09 +0000 Subject: [PATCH 02/26] test(objectql): a system write asks no read door and no probe Claude-Session: https://claude.ai/code/session_01ADHkmSZhbvtSoFM7YTkq6B Co-authored-by: Claude --- .../objectql/src/engine-unreadable-reference-target.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/objectql/src/engine-unreadable-reference-target.test.ts b/packages/objectql/src/engine-unreadable-reference-target.test.ts index 3bca0035a95..b1d4a1c7113 100644 --- a/packages/objectql/src/engine-unreadable-reference-target.test.ts +++ b/packages/objectql/src/engine-unreadable-reference-target.test.ts @@ -253,7 +253,7 @@ describe('[#29029] a non-system lookup or user write may name only a record the it.each(DOORS)('%s — control: a system write naming a target the caller cannot read lands, and asks no read door', async (door) => { await write(door, { [field]: UNREADABLE }, SYSTEM); expect(storedItems().some((r) => r[field] === UNREADABLE)).toBe(true); - expect(targetReads.length).toBeGreaterThan(0); + // The system caller's exemption: no read door asked (nor a probe at all). expect(targetReads.filter((r) => !r.elevated)).toEqual([]); }); From 4517600e6eb0e9b8b27749f9d93cfddb107a7092 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:38:15 +0000 Subject: [PATCH 03/26] fix(objectql): an unresolvable lookup target fails open; forced test edits for the caller-read rule Claude-Session: https://claude.ai/code/session_01ADHkmSZhbvtSoFM7YTkq6B Co-authored-by: Claude --- ...ne-predicate-related-master-detail-header.test.ts | 12 +++++++++--- .../src/engine-predicate-relationship.test.ts | 11 +++++++++-- packages/objectql/src/engine.ts | 10 +++++++++- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/objectql/src/engine-predicate-related-master-detail-header.test.ts b/packages/objectql/src/engine-predicate-related-master-detail-header.test.ts index 0d0456ef566..523fec6ba81 100644 --- a/packages/objectql/src/engine-predicate-related-master-detail-header.test.ts +++ b/packages/objectql/src/engine-predicate-related-master-detail-header.test.ts @@ -87,6 +87,7 @@ const LOOKUP_LINE_OBJECT = { label: 'Lookup line', fields: { invoice_ref: { name: 'invoice_ref', label: 'Invoice', type: 'lookup', reference: HEADER }, + memo: { name: 'memo', label: 'Memo', type: 'text' }, }, validations: [{ name: 'no_refs_to_locked', type: 'script', severity: 'error', @@ -368,10 +369,15 @@ describe('[#22892] a predicate rule reading its master through the master-detail }); it('a plain lookup traversal to the same object is unchanged: the header\'s stored state is judged, and no read door is asked', async () => { - const err = await refusalOf(() => engine.insert(LOOKUP_LINE, { invoice_ref: 'h_other_locked' }, { context: ALICE } as any)); + // [#29029] A non-system write may no longer NAME a header the caller + // cannot read through a lookup either, so the rows that already store + // one are seeded and the write edits another column of them. + storeFor(LOOKUP_LINE).set('ll_locked', { id: 'll_locked', invoice_ref: 'h_other_locked', memo: 'a' }); + storeFor(LOOKUP_LINE).set('ll_open', { id: 'll_open', invoice_ref: 'h_other_open', memo: 'a' }); + const err = await refusalOf(() => engine.update(LOOKUP_LINE, { memo: 'b' }, { where: { id: 'll_locked' }, context: ALICE } as any)); expectRuleVerdict(answerOf(err, 'h_other_locked')); - await engine.insert(LOOKUP_LINE, { invoice_ref: 'h_other_open' }, { context: ALICE } as any); - expect([...storeFor(LOOKUP_LINE).values()].map((r) => r.invoice_ref)).toEqual(['h_other_open']); + await engine.update(LOOKUP_LINE, { memo: 'b' }, { where: { id: 'll_open' }, context: ALICE } as any); + expect([...storeFor(LOOKUP_LINE).values()].map((r) => [r.invoice_ref, r.memo])).toEqual([['h_other_locked', 'a'], ['h_other_open', 'b']]); expect(headerReads.length).toBeGreaterThan(0); expect(headerReads.filter((r) => !r.elevated)).toEqual([]); }); diff --git a/packages/objectql/src/engine-predicate-relationship.test.ts b/packages/objectql/src/engine-predicate-relationship.test.ts index bdbb5843026..660ac4dfd12 100644 --- a/packages/objectql/src/engine-predicate-relationship.test.ts +++ b/packages/objectql/src/engine-predicate-relationship.test.ts @@ -138,7 +138,14 @@ describe('#18682 — engine-produced relationship bindings', () => { d.storeFor('crm_account').set('acc_empty', { id: 'acc_empty', name: 'E', secret: 's' }); }); - const relatedReads = () => d.calls.filter((c) => c.object === 'crm_account'); + // [#29029] The write's reference check asks the caller's read door for each + // lookup value it names, projected to `id` alone: that read is the + // reference check's, not a rule's related read, so it is not counted here. + const isReferenceCheckRead = (c: { ast: any }) => { + const fields = c.ast?.fields; + return Array.isArray(fields) && fields.length === 1 && fields[0] === 'id'; + }; + const relatedReads = () => d.calls.filter((c) => c.object === 'crm_account' && !isReferenceCheckRead(c)); it('projects id plus only the fields the rules name — not the whole row', async () => { await engine.insert('crm_opportunity', { name: 'A', amount: 10, account: 'acc_d' }, { context: ACTING } as any); @@ -488,7 +495,7 @@ describe('#18682 — engine-produced relationship bindings', () => { } as any, 'test-package'); d.calls.length = 0; await engine.insert('plain', { name: 'A', account: 'acc_p' }, { context: ACTING } as any); - expect(d.calls.filter((c) => c.object === 'crm_account')).toHaveLength(0); + expect(relatedReads()).toHaveLength(0); }); }); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 730c0513d11..01931826a3a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -10555,7 +10555,15 @@ export class ObjectQL implements IObjectQLEngine { id: unknown, context: ExecutionContext | undefined, ): Promise { - const master = this.resolveObjectName(target); + // [#29029] An unresolvable target fails open here exactly as it does in + // {@link referenceExists}: an integrity check that cannot run invents no + // rejection. + let master: string; + try { + master = this.resolveObjectName(target); + } catch { + return null; + } if (!this._registry.getObject(master)) return null; const readable = await this.masterDetailHeadersCallerReads(master, [id], context); if (!readable.has(String(id))) return false; From 60e40025b35676e4cfe5798565b5c82d650beb95 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:38:55 +0000 Subject: [PATCH 04/26] chore(changeset): objectql lookup and user writes name only a readable record Claude-Session: https://claude.ai/code/session_01ADHkmSZhbvtSoFM7YTkq6B Co-authored-by: Claude --- .changeset/29029-lookup-user-caller-read.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .changeset/29029-lookup-user-caller-read.md diff --git a/.changeset/29029-lookup-user-caller-read.md b/.changeset/29029-lookup-user-caller-read.md new file mode 100644 index 00000000000..14871b38c5b --- /dev/null +++ b/.changeset/29029-lookup-user-caller-read.md @@ -0,0 +1,20 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql)!: a non-system write may name, in a `lookup` or `user` field, only a record the caller can read (#29029) + +Clause-②: yes (narrowing) — a non-system write whose `lookup` or `user` value names a record the caller's own read door does not return is refused with `reference_not_found`, which it admitted before. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the write's reference check (`@objectstack/objectql`), served by every write door (REST, MCP, flows run as the user, `ctx.api` in hooks). + +After the upgrade, an insert or update that carries no `isSystem` is refused when a `lookup` or `user` value it supplies names a record the caller cannot read: the answer is `400 VALIDATION_FAILED` with the field's `reference_not_found`, localized, exactly the answer for a record that exists nowhere. This is the rule `master_detail` already follows. + +- **Each value is judged.** A multi-valued `lookup` or `user` field is refused when any one of its values names a record the caller cannot read. +- **Unchanged.** System writes (`isSystem`: seed replay, boot, package install) stay exempt, as before; a flow run's elevated write keeps its existence check. An update that carries back a value the row already stores is not judged again. An empty value is not a reference. A `readonly` field is not the caller's to answer for. A `tree` parent keeps the existence check alone. +- **Who sees it.** An app whose users write a reference to a record their own access does not return. A user can still reference every record they can read. + +**The remedy.** None for an app that meant its read access as the boundary. An app that wrote its own `beforeInsert` / `beforeUpdate` hook to refuse a reference to an unreadable record can delete it: the engine now refuses it, localized. A flow that must link records the acting user cannot read runs with `runAs: 'system'`. From 61bba86aa789ee381ec2d264bed8d3773bdd1995 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 17:12:20 +0000 Subject: [PATCH 05/26] fix(objectql)!: the reference check runs before the record's validation rules A reference a caller cannot read answers reference_not_found on every write door (insert, update by id, bulk update, and the validate preview), whatever a validation rule would answer. The one-hop predicate read's docblock states the rule that replaces the retired write-without-reading-the-parent persona. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- .../src/engine-master-detail-related.ts | 44 ++++++++++++------- packages/objectql/src/engine-validate.ts | 22 +++++----- packages/objectql/src/engine.ts | 31 +++++++------ 3 files changed, 57 insertions(+), 40 deletions(-) diff --git a/packages/objectql/src/engine-master-detail-related.ts b/packages/objectql/src/engine-master-detail-related.ts index 21490a3e228..ef2d03d2428 100644 --- a/packages/objectql/src/engine-master-detail-related.ts +++ b/packages/objectql/src/engine-master-detail-related.ts @@ -354,11 +354,19 @@ export async function resolvePreviewParents( * The VALUES are read as SYSTEM: a validation rule's output is a pass/fail the * SYSTEM enforces, not data handed to the caller — categorically unlike an * access-control rule, which is why RLS predicates are excluded from this - * capability altogether. Reading the columns as the acting user instead made - * the rule unauthorable for exactly the persona it exists to constrain. What - * bounds that elevation is the PROJECTION: only the columns the predicate - * names, intersected with the related object's declared fields. ⛔ Never the - * whole row. + * capability altogether. What bounds that elevation is the PROJECTION: only + * the columns the predicate names, intersected with the related object's + * declared fields. ⛔ Never the whole row. + * + * ⛔ No caller writes a row naming a related row it cannot read (#29029, + * ruling records 6573561091 and 6663849489). A non-system write may name, in + * a `master_detail`, `lookup` or `user` field, only a row the caller's own + * read door returns, and that reference check runs BEFORE the rules + * (`assertReferencesResolve`), so a rule never answers for a reference the + * caller supplied and cannot read: the write answers `reference_not_found`. + * The elevated value read therefore serves a related row the caller can read, + * a value the row already stores (#29062), a `tree` parent, and a system + * caller. * * Which ROWS a caller that is not SYSTEM may have read is a different * question, answered before any value is read: @@ -375,8 +383,9 @@ export async function resolvePreviewParents( * stored values, as the write judges it. * - On a related object no organization wall scopes, the own read of the * caller bounds the rows, and any other id is `unreadable`, stored or not. - * - ⛔ A plain `lookup` to a walled object keeps the tenant-kept elevated read - * alone: no ruling extends the header rule to it. + * - A plain `lookup` to a walled object keeps the tenant-kept elevated read + * alone HERE: a value the caller supplies and cannot read was already + * refused by the reference check before this read is judged (above). * * A SYSTEM caller (seed replay, boot) asks no read door: the elevated read * alone, as before. @@ -592,10 +601,12 @@ export async function resolveOneHopRelated( // A validation rule's output is a pass/fail the SYSTEM enforces, not // data handed to the caller — categorically unlike an access-control // rule, which is why RLS predicates are excluded from this capability - // altogether. Reading as the acting user instead made the rule - // unauthorable for exactly the persona it exists to constrain: a member - // with CRUD on the child and no read on the parent faulted on every - // write, so a legitimate business rule could not ship. + // altogether. [#29029] A reference the caller supplies and cannot read + // never reaches a rule: the write's reference check refuses it first + // (`reference_not_found`), so no caller writes a row naming a related + // row it cannot read. What this read still serves without a read door + // is a row the caller can read, a value the row already stores, and a + // system caller. // // What bounds the elevation is the PROJECTION: only the // columns this predicate names, intersected with the related object's @@ -604,11 +615,12 @@ export async function resolveOneHopRelated( // The accepted cost, recorded so nobody widens it by accident: a caller // can INFER a value they cannot see by observing which writes refuse — // a column hidden from them on a row they may read, or a column of a - // row a plain `lookup` names on a walled object. The value itself never - // appears — not in the row handed to CEL beyond the predicate's own use - // of it, and not in the refusal text, which names the field and the - // rule and never the value. [#22892] ⛔ Never what a master-detail - // header the caller cannot read holds: the door below answers first. + // row a plain `lookup` the row already stores names on a walled object. + // The value itself never appears — not in the row handed to CEL beyond + // the predicate's own use of it, and not in the refusal text, which + // names the field and the rule and never the value. [#22892] ⛔ Never + // what a master-detail header the caller cannot read holds: the door + // below answers first. // // ⛔ No organization wall scopes a related object with no tenant column // (e.g. `sys_user`), `tenancy.enabled: false` or `external`, so for a diff --git a/packages/objectql/src/engine-validate.ts b/packages/objectql/src/engine-validate.ts index ccdef5f4bc5..3975ceb0c2d 100644 --- a/packages/objectql/src/engine-validate.ts +++ b/packages/objectql/src/engine-validate.ts @@ -574,6 +574,17 @@ export async function validate( object, schemaForValidation, mode, row, mode === 'insert' ? row : judgedViews[i], options?.context, storedIds[i], ); } + // [#26684] The insert's admit step, run as `insert()` runs it: before + // the rules (#29029), the reference check on the ids the caller supplied + // (`rawRows`, before the defaults — the write's `suppliedPerRow`), so + // a row the write refuses on a dangling or unreadable reference is + // refused here with the same finding, and is not counted toward a later + // row's parent summary. ⛔ Behind the same write-gate probe as the + // related read above: a caller the probe refuses learns nothing about + // which ids exist. + if (mode === 'insert' && mayWrite) { + await this.assertReferencesResolve(schemaForValidation, row, rawRows[i], options?.context, messages); + } warnings.push(...evaluateValidationRules(schemaForValidation as any, row, mode, { // [#22445] The stored row, when this row named one: the by-id // update's `previous: priorRecord`. Absent, the record is the patch. @@ -587,17 +598,6 @@ export async function validate( related: batchSummaries ? batchSummaries.overlay(previewRelatedForRow(judgedViews[i])) : previewRelatedForRow(judgedViews[i]), permissions: previewPermissionsFor(row), })); - // [#26684] The insert's admit step, run as `insert()` runs it: after - // the rules, the reference check on the ids the caller supplied - // (`rawRows`, before the defaults — the write's `suppliedPerRow`), so - // a row the write refuses on a dangling reference is refused here - // with the same finding, and is not counted toward a later row's - // parent summary. ⛔ Behind the same write-gate probe as the related - // read above: the probe on a `lookup` target is elevated, so a caller - // the probe refuses learns nothing about which ids exist. - if (mode === 'insert' && mayWrite) { - await this.assertReferencesResolve(schemaForValidation, row, rawRows[i], options?.context, messages); - } } catch (e) { if (e instanceof ValidationError) { return { valid: false, errors: e.fields.map((f) => ({ ...f })), warnings }; diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index bf94bdcd7c1..be05e48804a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -6492,10 +6492,13 @@ export class ObjectQL implements IObjectQLEngine { // [ADR-0144 D8 rule 4] A `record`-mode amount is stored with its // currency code, or the row is refused (`ERR_CURRENCY_CODE_REQUIRED`). assertRecordCurrencyCodes(object, (schemaForValidation as { fields?: unknown } | undefined)?.fields, rows[i], 'insert'); - emitValidationAdvisories(options?.onValidationAdvisory, evaluateValidationRules(schemaForValidation as any, rows[i], 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: msgCtx, parent: insertParentForRow?.(rows[i]), related: batchSummaries ? batchSummaries.overlay(insertRelatedForRow(rows[i])) : insertRelatedForRow(rows[i]), permissions: insertPermissionsFor(rows[i]) }), this.logger, object); + // [#29029] The reference check runs BEFORE the record's validation + // rules: a reference the caller cannot read answers + // `reference_not_found`, whatever a rule would answer. await this.assertReferencesResolve( schemaForValidation, rows[i], suppliedPerRow[i], opCtx.context, msgCtx, ); + emitValidationAdvisories(options?.onValidationAdvisory, evaluateValidationRules(schemaForValidation as any, rows[i], 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: msgCtx, parent: insertParentForRow?.(rows[i]), related: batchSummaries ? batchSummaries.overlay(insertRelatedForRow(rows[i])) : insertRelatedForRow(rows[i]), permissions: insertPermissionsFor(rows[i]) }), this.logger, object); // [#25579] Admitted: a later row of this batch reads it in the summary. batchSummaries?.accept(rows[i]); } catch (e) { @@ -8322,15 +8325,16 @@ export class ObjectQL implements IObjectQLEngine { // resolution failure fails this write closed right here. const updatePayload = hookContext.input.data as Record; const permissionsForUpdate = (await this.resolveOptionPermissions(updateSchema, [updatePayload], permissionResolution))(updatePayload); - emitValidationAdvisories(options?.onValidationAdvisory, evaluateValidationRules(updateSchema as any, hookContext.input.data as Record, 'update', { previous: priorRecord, logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: updateMsgCtx, parent: roWhenParent, previousParent: roWhenPreviousParent, related: relatedForUpdate, permissions: permissionsForUpdate }), this.logger, object); // [#4441] A repoint is as capable of dangling as an initial link. // [#29062] …but a value the row already stores is not a repoint: // the prior row is what the check forgives a refused value against. + // [#29029] Before the record's validation rules, as on insert. await this.assertReferencesResolve( updateSchema, hookContext.input.data as Record, opCtx.data as Record, opCtx.context, updateMsgCtx, async () => [priorRecord], ); + emitValidationAdvisories(options?.onValidationAdvisory, evaluateValidationRules(updateSchema as any, hookContext.input.data as Record, 'update', { previous: priorRecord, logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: updateMsgCtx, parent: roWhenParent, previousParent: roWhenPreviousParent, related: relatedForUpdate, permissions: permissionsForUpdate }), this.logger, object); // [commit 9d7f7259f] The by-id driver exit — where a driver's refusal // leaves this door, and where a recognised unique violation // stops being the driver's error. `envelopeUniqueViolation` @@ -8735,6 +8739,18 @@ export class ObjectQL implements IObjectQLEngine { // set — the patch is shared, so either every row picks a // `can`-gated option or none does. Never per row. const bulkPermissions = (await this.resolveOptionPermissions(updateSchema, [bulkPatch], permissionResolution))(bulkPatch); + // [#4441] The bulk call site too — a guard wired into single-id + // writes only is still a hole one call site over (AGENTS.md + // PD #10's own worked example, #3106). + // [#29062] A refused value is forgiven only when EVERY matched row + // already stores it (the `readPriorRows` memo, read only then); + // a row it is new to makes it this write's reference. + // [#29029] Before the record's validation rules, as on insert. + await this.assertReferencesResolve( + updateSchema, hookContext.input.data as Record, + opCtx.data as Record, opCtx.context, updateMsgCtx, + readPriorRows, + ); if (rulesNeedRows) { for (const row of priorRows ?? []) { try { @@ -8756,17 +8772,6 @@ export class ObjectQL implements IObjectQLEngine { // rule families that never read a header. emitValidationAdvisories(options?.onValidationAdvisory, evaluateValidationRules(updateSchema as any, hookContext.input.data as Record, 'update', { previous: null, logger: this.logger, currentUser: bulkEvalUser, skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: updateMsgCtx, permissions: bulkPermissions }), this.logger, object); } - // [#4441] The bulk call site too — a guard wired into single-id - // writes only is still a hole one call site over (AGENTS.md - // PD #10's own worked example, #3106). - // [#29062] A refused value is forgiven only when EVERY matched row - // already stores it (the `readPriorRows` memo, read only then); - // a row it is new to makes it this write's reference. - await this.assertReferencesResolve( - updateSchema, hookContext.input.data as Record, - opCtx.data as Record, opCtx.context, updateMsgCtx, - readPriorRows, - ); // `updateMany` presence is part of the ladder verdict resolved above. // [commit 9d7f7259f] The predicate driver exit, enveloped on the same // terms as the by-id exit above. A multi-row write names no From c9cb46aa5b8cbdfab8c66c29eb5542d9ae90d0e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 17:17:06 +0000 Subject: [PATCH 06/26] feat(plugin-security): a delegate reads the business units its admin scope covers A delegation's admin scope over a business-unit subtree (ADR-0090 D12) also grants read on the units inside that scope, so delegated administration can name the units it administers. The read is admitted only for a read of sys_business_unit the caller's sets do not grant, on the caller's own behalf, and is bounded to the union of the subtrees its scopes cover. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- .../src/delegated-admin-gate.ts | 61 ++++++++++ ...legated-admin-own-name-set.posture.test.ts | 105 ++++++++++++++++-- .../src/security-plugin-authz-middleware.ts | 18 ++- 3 files changed, 175 insertions(+), 9 deletions(-) diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.ts index 8174d6484be..7bf56e43050 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.ts @@ -423,6 +423,47 @@ export function isUnreadableAddressedRowRefusal(refusal: unknown): boolean { return refusal instanceof UnreadableAddressedRowRefusal; } +/** The object a delegation's scope grants read on: the business-unit tree it is drawn over. */ +const DELEGATED_READ_OBJECT = 'sys_business_unit'; +/** The operations that read; the middleware dispatches no other read verb. */ +const DELEGATED_READ_OPERATIONS: ReadonlySet = new Set(['find', 'findOne', 'count', 'aggregate']); + +/** + * [ADR-0090 D12, ruling of record 6663849489] A delegation's admin scope over + * a business-unit subtree also grants read on the units inside that scope, so + * delegated administration can name the units it administers (an assignment's + * `business_unit_id` is a reference the caller must be able to read). + * + * The authz middleware asks this only for an operation the caller's sets do + * NOT grant, and admits it only on a non-null answer: the `where` scope + * `id IN ` it then ANDs into the read. Null — the ordinary + * refusal stands — unless every condition holds: a read of + * `sys_business_unit` carrying a query, on the caller's own behalf (no + * on-behalf-of delegator), with the gate composed, and at least one covered + * unit ({@link DelegatedAdminGate.scopedBusinessUnitIds}). Nothing wider: no + * other object, no write, and no unit outside the scopes the caller holds; a + * caller whose sets already grant the read is untouched. A scope that cannot + * be resolved admits nothing (fail closed). + */ +export async function delegatedUnitReadScope( + gate: Pick | null | undefined, + opCtx: { object?: string; operation?: string; ast?: unknown; context?: any }, + permissionSets: PermissionSet[], + delegatorSets: PermissionSet[] | null | undefined, +): Promise | null> { + if (!gate || delegatorSets) return null; + if (opCtx.object !== DELEGATED_READ_OBJECT || !DELEGATED_READ_OPERATIONS.has(String(opCtx.operation)) || !opCtx.ast) { + return null; + } + let ids: ReadonlySet; + try { + ids = await gate.scopedBusinessUnitIds(permissionSets, opCtx.context); + } catch { + return null; + } + return ids.size > 0 ? { id: { $in: [...ids] } } : null; +} + /** * [ADR-0090 D12] A row an administrator's write targets — the delegated walk's * and the organization branch's alike — with what the caller may be told about @@ -655,6 +696,26 @@ export class DelegatedAdminGate { return false; } + /** + * [ADR-0090 D12, ruling of record 6663849489] The business units a caller's + * delegated scopes cover: the union of the subtrees of every `adminScope` + * its resolved sets carry, resolved in the caller's own organization + * ({@link callerOrganizationId}), exactly as the write path resolves them. + * A delegation's scope also grants read on these units, so delegated + * administration can name the units it administers; the authz middleware + * admits that read on this answer alone. Any held scope counts, as for + * {@link scopesCoverUser}. + * + * Fail closed: no scope, or a scope that resolves to no unit of the + * caller's organization, covers nothing — an empty set. + */ + async scopedBusinessUnitIds(sets: PermissionSet[], callerContext?: any): Promise> { + const held = await this.resolveHeldScopes(sets, callerOrganizationId(callerContext)); + const ids = new Set(); + for (const s of held) for (const id of s.subtree) ids.add(id); + return ids; + } + /** * [ADR-0090 D12 / ADR-0105 D8] Describe what the caller may DELEGATE — * the read half of this gate. diff --git a/packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts b/packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts index 8ba29096821..08fe246298b 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts @@ -43,6 +43,9 @@ const eastScope = (allow: string[]) => ({ assignablePermissionSets: allow, }); const CRUD = { sys_user_position: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }; +/** [#29029] A plain object whose `unit` lookup names a business unit. */ +const UNIT_NOTE = 'qa_unit_note'; +const NOTE_CRUD = { allowRead: true, allowCreate: true, allowEdit: true }; /** * The catalog. `field_rep` and `field_lead` each list only `field_user`, and @@ -60,7 +63,9 @@ const PERMISSIONS = [ // Carries an adminScope equal to the wide delegate's own: not strictly contained by it. { name: 'field_lead', label: 'Field lead (own name)', objects: CRUD, adminScope: eastScope(['field_user', 'field_rep', 'field_lead']) }, // Narrow: allowlists the sets the positions LIST, not their own-name sets. - { name: 'east_admin', label: 'East delegate', objects: CRUD, adminScope: eastScope(['field_user']) }, + { name: 'east_admin', label: 'East delegate', objects: { ...CRUD, [UNIT_NOTE]: NOTE_CRUD }, adminScope: eastScope(['field_user']) }, + // [#29029] A writer of the unit note with no admin scope at all. + { name: 'note_writer', label: 'Note writer', objects: { [UNIT_NOTE]: NOTE_CRUD } }, // Wide: also allowlists the own-name sets. { name: 'east_admin_wide', label: 'East delegate (wide)', objects: CRUD, adminScope: eastScope(['field_user', 'field_rep', 'field_lead']) }, ]; @@ -74,8 +79,10 @@ const POSITIONS = [ type Posture = 'single' | 'group' | 'isolated'; const POSTURES: Posture[] = ['single', 'group', 'isolated']; +/** The users of `ORG_A`, as the runtime resolves them onto every session's context from `sys_member`. */ +const ORG_USER_IDS = ['u_delegate', 'u_delegate_wide', 'u_target', 'u_writer', 'u_org_admin']; const delegateCtx = (userId: string, set: string) => - ({ userId, positions: [], permissions: [set], tenantId: ORG_A, accessible_org_ids: [ORG_A] }); + ({ userId, positions: [], permissions: [set], tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_USER_IDS }); const NARROW = delegateCtx('u_delegate', 'east_admin'); const WIDE = delegateCtx('u_delegate_wide', 'east_admin_wide'); @@ -100,11 +107,22 @@ async function boot(posture: Posture) { version: '1.0.0', type: 'plugin', scope: 'system', - objects: [SysUserPosition, SysUserPermissionSet, SysMember, SysBusinessUnit, SysBusinessUnitMember], + // [#29029] `sys_user` provisioned: an assignment's `user_id` names a user + // the caller must be able to read, so the table the read door asks exists. + objects: [ + SysUser, SysUserPosition, SysUserPermissionSet, SysMember, SysBusinessUnit, SysBusinessUnitMember, + { + name: UNIT_NOTE, + label: 'Unit note', + sharingModel: 'public_read_write', + fields: { + name: { name: 'name', type: 'text' }, + unit: { name: 'unit', type: 'lookup', reference: 'sys_business_unit' }, + }, + }, + ], } as any); await engine.syncSchemas(); - // `sys_user` registered and unprovisioned, as in the sibling posture suite. - if (!engine.registry.getObject(SysUser.name)) engine.registry.registerObject(SysUser as never, 'qa.delegated-own-name'); engines.push(engine); const services: Record = { @@ -140,8 +158,10 @@ async function boot(posture: Posture) { { id: 'bu_east', name: 'east', parent_business_unit_id: 'bu_hq', organization_id: ORG_A }, { id: 'bu_east_field', name: 'east_field', parent_business_unit_id: 'bu_east', organization_id: ORG_A }, ], sys); - for (const userId of ['u_delegate', 'u_delegate_wide', 'u_target']) { - await engine.insert('sys_member', { id: `m_${userId}`, user_id: userId, organization_id: ORG_A, role: 'member' }, sys); + for (const userId of ORG_USER_IDS) { + await engine.insert('sys_user', { id: userId, email: `${userId}@example.test`, name: userId }, sys); + const role = userId === 'u_org_admin' ? 'admin' : 'member'; + await engine.insert('sys_member', { id: `m_${userId}`, user_id: userId, organization_id: ORG_A, role }, sys); } await engine.insert('sys_business_unit_member', { id: 'bum_target', user_id: 'u_target', business_unit_id: 'bu_east_field', organization_id: ORG_A }, sys); @@ -236,3 +256,74 @@ describe('[ADR-0090 D12, amended 2026-10-17; #24757] a delegate is judged on exa }); } }); + +/** [#29029] Every unit id `context` reads, sorted, or the refusal's status and code. */ +async function unitsRead(h: Awaited>, context: any): Promise { + try { + const rows = await h.engine.find('sys_business_unit', { context } as any); + return (Array.isArray(rows) ? rows : []).map((r: any) => String(r.id)).sort(); + } catch (e) { + const r = resolveThrownHttpError(e); + return { status: r.status, code: r.code }; + } +} + +/** [#29029] A unit note naming `unit`, written as `context`: the outcome, and the refused field's code. */ +async function noteNaming(h: Awaited>, context: any, unit: string) { + try { + await h.engine.insert(UNIT_NOTE, { name: `note on ${unit}`, unit }, { context } as any); + return { status: 200 }; + } catch (e) { + const r = resolveThrownHttpError(e); + const fields = ((e as any)?.fields ?? []) as Array<{ field?: string; code?: string }>; + return { status: r.status, code: r.code, fields: fields.map((f) => `${f.field}:${f.code}`) }; + } +} + +async function storedNotes(h: Awaited>): Promise { + const rows = await h.engine.find(UNIT_NOTE, { context: SYS }); + return (Array.isArray(rows) ? rows : []).map((r: any) => String(r.unit)).sort(); +} + +describe('[ADR-0090 D12, ruling of record 6663849489; #29029] a delegate reads the business units its scope covers, and no other — three postures', () => { + const WRITER = delegateCtx('u_writer', 'note_writer'); + const ORG_ADMIN = delegateCtx('u_org_admin', 'organization_admin'); + + for (const posture of POSTURES) { + describe(`posture '${posture}'`, () => { + it('the delegate reads exactly the units its scope covers: the scope root and its subtree, not the unit above it', async () => { + const h = await boot(posture); + expect(await unitsRead(h, NARROW)).toEqual(['bu_east', 'bu_east_field']); + const above = await h.engine.findOne('sys_business_unit', { where: { id: 'bu_hq' }, context: NARROW } as any); + expect(above ?? null).toBeNull(); + expect(await h.engine.count('sys_business_unit', { context: NARROW } as any)).toBe(2); + }); + + it('the delegate assigns a position in a unit its scope covers, and the assignment is stored', async () => { + const h = await boot(posture); + expect(await assign(h, NARROW, 'field_rep')).toEqual({ status: 200 }); + expect(await storedPositions(h)).toEqual(['field_rep']); + }); + + it('a unit outside the scope is a unit the delegate cannot read: a reference to it answers reference_not_found, and one inside is admitted', async () => { + const h = await boot(posture); + expect(await noteNaming(h, NARROW, 'bu_hq')).toEqual({ status: 400, code: 'VALIDATION_FAILED', fields: ['unit:reference_not_found'] }); + expect(await noteNaming(h, NARROW, 'bu_east_field')).toEqual({ status: 200 }); + expect(await storedNotes(h)).toEqual(['bu_east_field']); + }); + + it('CONTROL: a caller holding no scope still reads no unit, and cannot name one', async () => { + const h = await boot(posture); + expect(await unitsRead(h, WRITER)).toEqual({ status: 403, code: 'PERMISSION_DENIED' }); + expect(await noteNaming(h, WRITER, 'bu_east')).toEqual({ status: 400, code: 'VALIDATION_FAILED', fields: ['unit:reference_not_found'] }); + expect(await storedNotes(h)).toEqual([]); + }); + + it('CONTROL: an organization administrator reads and names every unit of its organization, as before', async () => { + const h = await boot(posture); + expect(await unitsRead(h, ORG_ADMIN)).toEqual(['bu_east', 'bu_east_field', 'bu_hq']); + expect(await noteNaming(h, ORG_ADMIN, 'bu_hq')).toEqual({ status: 200 }); + }); + }); + } +}); diff --git a/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts b/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts index f3a6b3d705a..de0f4db534f 100644 --- a/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts +++ b/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts @@ -104,7 +104,7 @@ import { declaredHttpStatus } from '@objectstack/types'; import type { SecurityPlugin } from './security-plugin.js'; import { isGuestEnvelope, readUnlessRefused, requiredCapsForOperation } from './security-plugin.js'; import { assertFlowSystemIdentityWriteAllowed } from './flow-system-identity-write-refusal.js'; -import { isUnreadableAddressedRowRefusal } from './delegated-admin-gate.js'; +import { delegatedUnitReadScope, isUnreadableAddressedRowRefusal } from './delegated-admin-gate.js'; import { isRowWriteProbe, logGateRefusal, type AuthzMiddlewareFn } from './record-row-write-probe.js'; import { intersectDelegatedScope, @@ -840,6 +840,14 @@ export function registerAuthzMiddleware( // guard this replaced admitted that caller to every object no set // grants. A principal-less context never reaches this line — it was // refused above ({@link isPrincipalLessContext}, [#21908] ADR-0096 D5). + // + // [ADR-0090 D12, ruling of record 6663849489] One admission beside the + // grant: a delegate reads the business units its admin scopes cover. For + // a read of `sys_business_unit` no set grants, `delegatedUnitReadScope` + // answers the `id IN ` scope step 3 ANDs into the read, or + // null — and null keeps the refusal below. Every condition and why it is + // no wider is on that function. + let delegatedUnitScope: Record | null = null; if (!referentialFieldClearWrite) { const allowed = this.permissionEvaluator.checkObjectPermission( opCtx.operation, @@ -847,8 +855,11 @@ export function registerAuthzMiddleware( permissionSets, { isPrivate: secMeta.isPrivate }, ); - if (!allowed) { + delegatedUnitScope = await delegatedUnitReadScope(this.delegatedAdminGate, opCtx, permissionSets, delegatorSets); + } + + if (!allowed && !delegatedUnitScope) { // [#7414] TWO messages, two audiences — the split #7307 made for // `DELETE_RESTRICTED`, reached from the authorization side and through // the SAME catalog. @@ -2351,6 +2362,9 @@ export function registerAuthzMiddleware( ); const rlsFilter = andComposeLayers(layered.layer0, layered.layer1); if (rlsFilter) extra.push(rlsFilter); + // [ADR-0090 D12] The delegated unit read admitted at step 2 sees the + // units its scopes cover, and only those. + if (delegatedUnitScope) extra.push(delegatedUnitScope); let tenantLayer0Verdict = layered.tenantLayer0Verdict; // ADR-0055: a controlled_by_parent object derives its read scope from the // master record — `masterFK IN ()`, From 129000b5614aefb3587ee78c1fbbabe9f4f51708 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 17:37:53 +0000 Subject: [PATCH 07/26] fix(objectql): a system write keeps its validation rules before the reference check A write that is not a system write has its references checked before the record's validation rules; a system write keeps the order it had, so its answer is unchanged. One predicate, referencesCheckedBeforeRules, is read by insert, update by id, bulk update and the validate preview. Pins: each door answers reference_not_found for an unreadable reference a rule would trip on, and for one that exists nowhere, while a readable one still answers the rule; a system write and a flow run's system write keep the rule's answer; a read of the target that fails is refused, and a reference to an unregistered object is admitted. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- .../src/engine-referential-integrity.ts | 14 ++ ...engine-unreadable-reference-target.test.ts | 145 +++++++++++++++++- packages/objectql/src/engine-validate.ts | 11 +- packages/objectql/src/engine.ts | 31 ++-- 4 files changed, 189 insertions(+), 12 deletions(-) diff --git a/packages/objectql/src/engine-referential-integrity.ts b/packages/objectql/src/engine-referential-integrity.ts index b35d88b14d2..694ea14a431 100644 --- a/packages/objectql/src/engine-referential-integrity.ts +++ b/packages/objectql/src/engine-referential-integrity.ts @@ -95,6 +95,20 @@ function isEmptyReferenceValue(v: unknown): boolean { */ const CALLER_READ_REFERENCE_TYPES: ReadonlySet = new Set(['master_detail', 'lookup', 'user']); +/** + * [#29029] Whether this write's reference check ({@link assertReferencesResolve}) + * runs BEFORE the record's validation rules. A write that is not a system + * write: yes — a reference its caller cannot read answers + * `reference_not_found`, whatever a rule would answer. A system write keeps + * the order it had, the rules first and then the check, so its answer is the + * one it gave before (ruling of record 6663849489). The one predicate every + * write door and the validate preview read, so the order cannot differ + * between them. + */ +export function referencesCheckedBeforeRules(context: unknown): boolean { + return !(context as { isSystem?: unknown } | null | undefined)?.isSystem; +} + /** * [#4441] Referential integrity on the WRITE path: a `lookup` (or any * reference-typed field) may not be given an id that exists in no row of the diff --git a/packages/objectql/src/engine-unreadable-reference-target.test.ts b/packages/objectql/src/engine-unreadable-reference-target.test.ts index b1d4a1c7113..357b34a4ff4 100644 --- a/packages/objectql/src/engine-unreadable-reference-target.test.ts +++ b/packages/objectql/src/engine-unreadable-reference-target.test.ts @@ -31,6 +31,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { resolveThrownHttpError } from '@objectstack/types'; +import { markFlowWriteReferenceCheck } from '@objectstack/core'; import { ObjectQL } from './engine.js'; import { ValidationError } from './validation/record-validator.js'; @@ -59,6 +60,32 @@ const ITEM_OBJECT = { }, }; +/** + * [#29029] A task whose validation rules read each reference one hop: each + * trips on a target named `tripping`. The reference check runs BEFORE them. + */ +const RULED = 'urt_ruled_task'; +const TRIPPED = 'URT29029: the named target trips the rule'; +const TITLE_TRIPPED = 'URT29029: the title trips the rule'; +const RULED_OBJECT = { + name: RULED, + label: 'Ruled task', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + duty: { name: 'duty', label: 'Duty', type: 'lookup', reference: TARGET }, + assignee: { name: 'assignee', label: 'Assignee', type: 'user' }, + header: { name: 'header', label: 'Header', type: 'master_detail', reference: TARGET }, + }, + validations: (['duty', 'assignee', 'header'] as const).map((field) => ({ + // Checked only while the reference is set, as an author guards a traversal. + name: `urt_${field}_when_set`, type: 'conditional', severity: 'error', message: 'unused', + when: `record.${field} != null`, + then: { name: `urt_${field}_trips`, type: 'script', severity: 'error', message: TRIPPED, condition: `record.${field}.name == 'tripping'` }, + })).concat([ + { name: 'urt_title_trips', type: 'script', severity: 'error', message: TITLE_TRIPPED, condition: "record.title == 'trip'" } as any, + ]), +}; + /** A store-backed driver: what is stored is what a later read answers. */ function makeStoreDriver() { const stores = new Map>>(); @@ -135,7 +162,7 @@ function makeEngine() { engine.registerApp({ id: 'urt', name: 'Unreadable reference target', - objects: [ownedTarget(TARGET, 'Duty'), ownedTarget(USER, 'User'), ITEM_OBJECT], + objects: [ownedTarget(TARGET, 'Duty'), ownedTarget(USER, 'User'), ITEM_OBJECT, RULED_OBJECT], } as any); const targetReads: TargetRead[] = []; engine.registerMiddleware(async (ctx: any, next: () => Promise) => { @@ -160,6 +187,9 @@ function makeEngine() { rows.set('t_mine', { id: 't_mine', name: 'mine', owner_name: 'alice' }); rows.set('t_mine_2', { id: 't_mine_2', name: 'mine too', owner_name: 'alice' }); rows.set('t_other', { id: 't_other', name: 'other', owner_name: 'bob' }); + // [#29029] A readable and an unreadable target that trip the ruled task's rules. + rows.set('t_trip_mine', { id: 't_trip_mine', name: 'tripping', owner_name: 'alice' }); + rows.set('t_trip_other', { id: 't_trip_other', name: 'tripping', owner_name: 'bob' }); } return { engine, targetReads, ...store }; } @@ -293,4 +323,117 @@ describe('[#29029] a non-system lookup or user write may name only a record the expect(storeFor(ITEM).get('k1')?.[field]).toEqual([UNREADABLE, 't_mine']); }); }); + + it.each(DOORS)('%s — control: a flow run\'s system write keeps its reference check: a target that exists nowhere is refused', async (door) => { + const flowRun = markFlowWriteReferenceCheck({ ...SYSTEM }); + expect(envelopeOf(await refusalOf(() => write(door, { duty: NOWHERE }, flowRun)))).toEqual(notFound('duty')); + expect(storedItems()).toEqual([{ id: 'k1', title: 'stored' }]); + }); +}); + +describe('[#29029] an unreadable reference answers reference_not_found before the record\'s validation rules', () => { + let engine: ObjectQL; + let storeFor: (o: string) => Map>; + + beforeEach(() => { + ({ engine, storeFor } = makeEngine()); + storeFor(RULED).set('r1', { id: 'r1', title: 'stored' }); + }); + + const write = (door: Door, payload: Record, context: Record = ALICE) => { + if (door === 'insert') return engine.insert(RULED, { title: 'new', ...payload }, { context } as any); + if (door === 'batch insert') { + return engine.insert(RULED, [{ title: 'first' }, { title: 'second', ...payload }], { context } as any); + } + if (door === 'update by id') return engine.update(RULED, payload, { where: { id: 'r1' }, context } as any); + return engine.update(RULED, payload, { where: { id: { $in: ['r1'] } }, multi: true, context } as any); + }; + + const storedRuled = () => [...storeFor(RULED).values()]; + const ruleRefusal = (err: unknown) => ({ + code: resolveThrownHttpError(err).code, + messages: ((err as any)?.fields ?? []).map((f: any) => String(f.message ?? '')), + }); + + describe.each([ + { field: 'duty', kind: 'lookup' }, + { field: 'assignee', kind: 'user' }, + { field: 'header', kind: 'master_detail' }, + ] as const)('a $kind field', ({ field }) => { + it.each(DOORS)('%s — a target the caller cannot read that a rule would trip on answers reference_not_found, not the rule', async (door) => { + const err = await refusalOf(() => write(door, { [field]: 't_trip_other' })); + expect(envelopeOf(err)).toEqual(notFound(field)); + expect(String((err as Error).message)).not.toContain(TRIPPED); + expect(storedRuled()).toEqual([{ id: 'r1', title: 'stored' }]); + }); + + it.each(DOORS)('%s — control: a readable target the rule trips on answers the rule', async (door) => { + const err = await refusalOf(() => write(door, { [field]: 't_trip_mine' })); + expect(ruleRefusal(err).code).toBe('VALIDATION_FAILED'); + expect(ruleRefusal(err).messages.some((m: string) => m.includes(TRIPPED))).toBe(true); + expect(storedRuled()).toEqual([{ id: 'r1', title: 'stored' }]); + }); + + it.each(DOORS)('%s — control: a system write naming that target answers the rule, as before', async (door) => { + const err = await refusalOf(() => write(door, { [field]: 't_trip_other' }, SYSTEM)); + expect(ruleRefusal(err).messages.some((m: string) => m.includes(TRIPPED))).toBe(true); + expect(envelopeOf(err).fields.map((f) => f.code)).not.toContain('reference_not_found'); + }); + }); + + it.each(DOORS)('%s — a reference to a record that exists nowhere answers reference_not_found before a rule the record trips', async (door) => { + const err = await refusalOf(() => write(door, { title: 'trip', duty: 't_missing' })); + expect(envelopeOf(err)).toEqual(notFound('duty')); + expect(String((err as Error).message)).not.toContain(TITLE_TRIPPED); + }); + + it.each(DOORS)('%s — control: a flow run\'s system write keeps the rules first — the rule its record trips answers, not the reference check', async (door) => { + const flowRun = markFlowWriteReferenceCheck({ ...SYSTEM }); + const err = await refusalOf(() => write(door, { title: 'trip', duty: 't_missing' }, flowRun)); + expect(ruleRefusal(err).messages.some((m: string) => m.includes(TITLE_TRIPPED))).toBe(true); + expect(envelopeOf(err).fields.map((f) => f.code)).not.toContain('reference_not_found'); + }); + + it('the validate preview answers as the insert does: reference_not_found for the unreadable target, the rule for the readable one', async () => { + const unreadable = (await engine.validate(RULED, { title: 'preview', duty: 't_trip_other' }, { mode: 'insert', context: ALICE } as any)).results?.[0]; + expect(unreadable?.valid).toBe(false); + expect((unreadable?.errors ?? []).map((e: any) => ({ field: e.field, code: e.code }))).toEqual([{ field: 'duty', code: 'reference_not_found' }]); + expect(JSON.stringify(unreadable?.errors ?? [])).not.toContain(TRIPPED); + const readable = (await engine.validate(RULED, { title: 'preview', duty: 't_trip_mine' }, { mode: 'insert', context: ALICE } as any)).results?.[0]; + expect(readable?.valid).toBe(false); + expect((readable?.errors ?? []).some((e: any) => String(e.message).includes(TRIPPED))).toBe(true); + }); +}); + +describe('[#29029] which failures of the reference check fail open, and which fail closed', () => { + it('a read door that fails for a reason other than access fails CLOSED: the target is not one the caller read', async () => { + const { engine, storeFor } = makeEngine(); + engine.registerMiddleware(async (ctx: any, next: () => Promise) => { + if (ctx.object === TARGET && ctx.operation === 'find' && !ctx.context?.isSystem) { + throw new Error('no such table: urt_duty'); + } + await next(); + }); + const err = await refusalOf(() => engine.insert(ITEM, { title: 'new', duty: 't_mine' }, { context: ALICE } as any)); + expect(envelopeOf(err)).toEqual(notFound('duty')); + expect([...storeFor(ITEM).values()]).toEqual([]); + }); + + it('a reference to an object the registry does not hold fails OPEN: there is no read door to ask, as the existence probe already answered', async () => { + const { engine, storeFor } = makeEngine(); + engine.registerApp({ + id: 'urt_unregistered', + name: 'A reference to an unregistered object', + objects: [{ + name: 'urt_loose_task', + label: 'Loose task', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + loose: { name: 'loose', label: 'Loose', type: 'lookup', reference: 'urt_not_registered' }, + }, + }], + } as any); + await engine.insert('urt_loose_task', { title: 'new', loose: 'x_1' }, { context: ALICE } as any); + expect([...storeFor('urt_loose_task').values()].map((r) => r.loose)).toEqual(['x_1']); + }); }); diff --git a/packages/objectql/src/engine-validate.ts b/packages/objectql/src/engine-validate.ts index 3975ceb0c2d..4f77b46dd85 100644 --- a/packages/objectql/src/engine-validate.ts +++ b/packages/objectql/src/engine-validate.ts @@ -52,6 +52,7 @@ import { evaluateValidationRules, hasParentScopedRequiredWhen, stripReadonlyFiel import { createBatchSummaryAccumulator } from './summary-batch-accumulation.js'; import { assertRecordCurrencyCodes, RecordCurrencyCodeRequiredError } from './validation/record-currency-code.js'; import { isHomeAmountRefusal } from './home-amount-projection.js'; +import { referencesCheckedBeforeRules } from './engine-referential-integrity.js'; import { resolveEngineUpdateDispatch, type EngineUpdateDispatchData } from './engine-update-dispatch.js'; import { droppedFieldEvents, @@ -575,14 +576,17 @@ export async function validate( ); } // [#26684] The insert's admit step, run as `insert()` runs it: before - // the rules (#29029), the reference check on the ids the caller supplied + // the rules unless this is a system write (#29029, + // `referencesCheckedBeforeRules`), the reference check on the ids the caller supplied // (`rawRows`, before the defaults — the write's `suppliedPerRow`), so // a row the write refuses on a dangling or unreadable reference is // refused here with the same finding, and is not counted toward a later // row's parent summary. ⛔ Behind the same write-gate probe as the // related read above: a caller the probe refuses learns nothing about // which ids exist. - if (mode === 'insert' && mayWrite) { + const previewReferences = mode === 'insert' && mayWrite; + const referencesFirst = referencesCheckedBeforeRules(options?.context); + if (previewReferences && referencesFirst) { await this.assertReferencesResolve(schemaForValidation, row, rawRows[i], options?.context, messages); } warnings.push(...evaluateValidationRules(schemaForValidation as any, row, mode, { @@ -598,6 +602,9 @@ export async function validate( related: batchSummaries ? batchSummaries.overlay(previewRelatedForRow(judgedViews[i])) : previewRelatedForRow(judgedViews[i]), permissions: previewPermissionsFor(row), })); + if (previewReferences && !referencesFirst) { + await this.assertReferencesResolve(schemaForValidation, row, rawRows[i], options?.context, messages); + } } catch (e) { if (e instanceof ValidationError) { return { valid: false, errors: e.fields.map((f) => ({ ...f })), warnings }; diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index be05e48804a..7d80ebbf852 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -101,7 +101,7 @@ import { buildEvalUser, buildProvenance, buildSession, buildUser } from './engin // [#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, type StoredReferenceRows } from './engine-referential-integrity.js'; +import { assertReferencesResolve, masterDetailHeadersCallerReads, referencesCheckedBeforeRules, type StoredReferenceRows } from './engine-referential-integrity.js'; // [#25522 cut 11] File-field hydration on read; the methods of the same names // below hand each body the members it reads (`FileReferenceHydrationHost`). import { callerMayReadSysFile, readParentDerivedFiles, resolveFileReferences, servableFileRows, serveRefusedFileHydration } from './engine-file-reference-hydration.js'; @@ -6492,13 +6492,18 @@ export class ObjectQL implements IObjectQLEngine { // [ADR-0144 D8 rule 4] A `record`-mode amount is stored with its // currency code, or the row is refused (`ERR_CURRENCY_CODE_REQUIRED`). assertRecordCurrencyCodes(object, (schemaForValidation as { fields?: unknown } | undefined)?.fields, rows[i], 'insert'); - // [#29029] The reference check runs BEFORE the record's validation - // rules: a reference the caller cannot read answers - // `reference_not_found`, whatever a rule would answer. - await this.assertReferencesResolve( + // [#29029] A write that is not a system write has its references + // checked BEFORE the record's validation rules: a reference the + // caller cannot read answers `reference_not_found`, whatever a rule + // would answer. A system write keeps the rules first + // (`referencesCheckedBeforeRules`). + const referencesFirst = referencesCheckedBeforeRules(opCtx.context); + const checkReferences = () => this.assertReferencesResolve( schemaForValidation, rows[i], suppliedPerRow[i], opCtx.context, msgCtx, ); + if (referencesFirst) await checkReferences(); emitValidationAdvisories(options?.onValidationAdvisory, evaluateValidationRules(schemaForValidation as any, rows[i], 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: msgCtx, parent: insertParentForRow?.(rows[i]), related: batchSummaries ? batchSummaries.overlay(insertRelatedForRow(rows[i])) : insertRelatedForRow(rows[i]), permissions: insertPermissionsFor(rows[i]) }), this.logger, object); + if (!referencesFirst) await checkReferences(); // [#25579] Admitted: a later row of this batch reads it in the summary. batchSummaries?.accept(rows[i]); } catch (e) { @@ -8328,13 +8333,17 @@ export class ObjectQL implements IObjectQLEngine { // [#4441] A repoint is as capable of dangling as an initial link. // [#29062] …but a value the row already stores is not a repoint: // the prior row is what the check forgives a refused value against. - // [#29029] Before the record's validation rules, as on insert. - await this.assertReferencesResolve( + // [#29029] Before the record's validation rules unless this is a + // system write, as on insert (`referencesCheckedBeforeRules`). + const referencesFirst = referencesCheckedBeforeRules(opCtx.context); + const checkReferences = () => this.assertReferencesResolve( updateSchema, hookContext.input.data as Record, opCtx.data as Record, opCtx.context, updateMsgCtx, async () => [priorRecord], ); + if (referencesFirst) await checkReferences(); emitValidationAdvisories(options?.onValidationAdvisory, evaluateValidationRules(updateSchema as any, hookContext.input.data as Record, 'update', { previous: priorRecord, logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: updateMsgCtx, parent: roWhenParent, previousParent: roWhenPreviousParent, related: relatedForUpdate, permissions: permissionsForUpdate }), this.logger, object); + if (!referencesFirst) await checkReferences(); // [commit 9d7f7259f] The by-id driver exit — where a driver's refusal // leaves this door, and where a recognised unique violation // stops being the driver's error. `envelopeUniqueViolation` @@ -8745,12 +8754,15 @@ export class ObjectQL implements IObjectQLEngine { // [#29062] A refused value is forgiven only when EVERY matched row // already stores it (the `readPriorRows` memo, read only then); // a row it is new to makes it this write's reference. - // [#29029] Before the record's validation rules, as on insert. - await this.assertReferencesResolve( + // [#29029] Before the record's validation rules unless this is a + // system write, as on insert (`referencesCheckedBeforeRules`). + const referencesFirst = referencesCheckedBeforeRules(opCtx.context); + const checkReferences = () => this.assertReferencesResolve( updateSchema, hookContext.input.data as Record, opCtx.data as Record, opCtx.context, updateMsgCtx, readPriorRows, ); + if (referencesFirst) await checkReferences(); if (rulesNeedRows) { for (const row of priorRows ?? []) { try { @@ -8772,6 +8784,7 @@ export class ObjectQL implements IObjectQLEngine { // rule families that never read a header. emitValidationAdvisories(options?.onValidationAdvisory, evaluateValidationRules(updateSchema as any, hookContext.input.data as Record, 'update', { previous: null, logger: this.logger, currentUser: bulkEvalUser, skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: updateMsgCtx, permissions: bulkPermissions }), this.logger, object); } + if (!referencesFirst) await checkReferences(); // `updateMany` presence is part of the ladder verdict resolved above. // [commit 9d7f7259f] The predicate driver exit, enveloped on the same // terms as the by-id exit above. A multi-row write names no From 6602814468c44726f0acc8b1737182a1b06b8389 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 17:37:53 +0000 Subject: [PATCH 08/26] test(plugin-security): suites name users their callers read The engine's reference check now reads a user or lookup target through the caller's own read door, so suites whose subject is another predicate on the same write provision sys_user with a row for each user they name, and give their callers the org_user_ids the runtime fills from sys_member. The write-preview parity controls read the related account, and a new block drives the editor who cannot: both doors answer the lookup's reference_not_found, never the rule. The tenant-scope suite counts the rule's related reads alone, and a user the caller cannot read is now answered on the reference check. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- ...cope-no-identity-positions.posture.test.ts | 27 ++++- .../grant-holder-membership-refusal.test.ts | 15 ++- .../src/grant-permission-set-name.test.ts | 10 ++ ...-admin-assignment-boundary.posture.test.ts | 45 +++++--- .../src/position-catalog-refusal.test.ts | 14 +++ ...redicate-related-read-tenant-scope.test.ts | 24 ++-- .../plugin-security/src/rig-users.testkit.ts | 28 +++++ ...s-user-permission-set-window-order.test.ts | 10 ++ .../write-preview-field-gate-parity.test.ts | 104 ++++++++++++++---- 9 files changed, 227 insertions(+), 50 deletions(-) create mode 100644 packages/plugins/plugin-security/src/rig-users.testkit.ts diff --git a/packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts b/packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts index 06d0760017e..af175ef13d3 100644 --- a/packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts +++ b/packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts @@ -37,6 +37,7 @@ import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.j import { RESERVED_IDENTITY_NAMES } from './objects/reserved-identity-names.js'; import { defaultPermissionSets } from './objects/index.js'; import { BUILTIN_IDENTITY_ORG_EXTERNAL } from '@objectstack/spec'; +import { provisionRigUsers } from './rig-users.testkit.js'; const SYS = { isSystem: true } as const; const ORG_A = 'org_a'; @@ -53,7 +54,12 @@ const PERMISSIONS = [ { name: 'east_admin', label: 'East delegate', - objects: { sys_user_position: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, + // [#29029] With read on `sys_user`, the companion grant a delegate holds + // to pick people: the write names a holder its caller reads. + objects: { + sys_user_position: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + sys_user: { allowRead: true }, + }, adminScope: { businessUnit: 'east', includeSubtree: true, manageAssignments: true, assignablePermissionSets: ['field_user'] }, }, ]; @@ -62,20 +68,26 @@ const POSITIONS = [{ name: CONTROL, label: 'Field rep', permissionSets: ['field_ type Posture = 'single' | 'group' | 'isolated'; const POSTURES: Posture[] = ['single', 'group', 'isolated']; +/** + * [#29029] Organization A's users as `org_user_ids`, which the runtime fills + * from `sys_member`: what the shipped sets' `sys_user` read rule reads. + */ +const ORG_USERS = ['u_delegate', 'u_org_admin', 'u_platform', 'u_target']; + /** One caller per branch of the report, and how the report must name the branch it took. */ const BRANCHES = { 'tenant-level': { - context: { userId: 'u_platform', positions: [], permissions: ['admin_full_access'], posture: 'PLATFORM_ADMIN', tenantId: ORG_A, accessible_org_ids: [ORG_A] }, + context: { userId: 'u_platform', positions: [], permissions: ['admin_full_access'], posture: 'PLATFORM_ADMIN', tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_USERS }, isTenantAdmin: true, scopes: [] as string[], }, 'organization-level': { - context: { userId: 'u_org_admin', positions: [], permissions: ['organization_admin'], tenantId: ORG_A, accessible_org_ids: [ORG_A] }, + context: { userId: 'u_org_admin', positions: [], permissions: ['organization_admin'], tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_USERS }, isTenantAdmin: false, scopes: [] as string[], }, delegated: { - context: { userId: 'u_delegate', positions: [], permissions: ['east_admin'], tenantId: ORG_A, accessible_org_ids: [ORG_A] }, + context: { userId: 'u_delegate', positions: [], permissions: ['east_admin'], tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_USERS }, isTenantAdmin: false, scopes: ['east_admin'], }, @@ -89,6 +101,9 @@ const MEMBERSHIPS = [ { id: 'm_target', user_id: 'u_target', role: 'member' }, ]; +/** [#29029] Every user this suite's writes name or act as: each gets a `sys_user` row. */ +const RIG_USERS = ORG_USERS; + const engines: ObjectQL[] = []; afterEach(async () => { vi.restoreAllMocks(); @@ -113,8 +128,10 @@ async function boot(posture: Posture) { objects: [SysUserPosition, SysUserPermissionSet, SysMember, SysBusinessUnit, SysBusinessUnitMember], } as any); await engine.syncSchemas(); - // `sys_user` registered and unprovisioned, as in the sibling posture suites. + // [#29029] `sys_user` provisioned with a row for every user the writes name, + // as in the sibling posture suites. if (!engine.registry.getObject(SysUser.name)) engine.registry.registerObject(SysUser as never, 'qa.delegable-scope-identity'); + await provisionRigUsers(engine, RIG_USERS); engines.push(engine); const services: Record = { diff --git a/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts b/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts index 59fe67d8cfe..ad1bf69bcbd 100644 --- a/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts +++ b/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts @@ -40,6 +40,7 @@ import { registerGrantHolderMembershipRefusal, } from './grant-holder-membership-refusal.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; +import { provisionRigUsers } from './rig-users.testkit.js'; // --------------------------------------------------------------------------- // Fixture @@ -95,6 +96,12 @@ const MEMBERSHIPS = [ { id: 'm_both_b', user_id: 'u_both', organization_id: ORG_B, role: 'member' }, ]; +/** [#29029] Every user this suite's writes name or act as: each gets a `sys_user` row. */ +const RIG_USERS = [ + 'u_admin', 'u_admin_a', 'u_both', 'u_member_a', 'u_member_a2', 'u_member_b', 'u_nobody', + 'u_platform', 'u_ro_a', +]; + const engines: ObjectQL[] = []; afterEach(async () => { vi.restoreAllMocks(); @@ -143,10 +150,12 @@ async function boot(opts: BootOptions = {}) { // The organization-less holdings and rows below exist only on a store built // before 18.x (ADR-0131 D1 makes a fresh tenant column NOT NULL): this is it. await relaxTenantColumns(engine, ['sys_position', 'sys_user_position', 'sys_user_permission_set']); - // `sys_user` stays registered and unprovisioned, as in the sibling suites: - // the engine's own `user_id` existence probe then cannot run and stands - // down, so what is measured here is the membership predicate alone. + // [#29029] `sys_user` is provisioned with a row for every user the writes + // name: the engine's own `user_id` reference check reads the holder through + // the caller's read door and finds it, so what is measured here is the + // membership predicate alone. if (!engine.registry.getObject(SysUser.name)) engine.registry.registerObject(SysUser as never, 'qa.authz-read-set'); + await provisionRigUsers(engine, RIG_USERS); if (opts.members === 'unprovisioned') engine.registry.registerObject(SysMember as never, 'qa.authz-read-set'); engines.push(engine); diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts index 26f7d60c633..889f777e893 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts @@ -44,6 +44,7 @@ import { registerGrantPermissionSetNameHooks, } from './grant-permission-set-name.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; +import { provisionRigUsers } from './rig-users.testkit.js'; // --------------------------------------------------------------------------- // Fixture @@ -70,6 +71,12 @@ const CATALOG = [MEMBER_DEFAULT, QA_ADMIN, QA_ALPHA, QA_BETA]; const SYS = { isSystem: true } as const; const ADMIN = { userId: 'u_admin', positions: [], permissions: ['qa_admin'] }; +/** [#29029] Every user this suite's writes name or act as: each gets a `sys_user` row. */ +const RIG_USERS = [ + 'u_1', 'u_2', 'u_3', 'u_3b', 'u_4', 'u_5', 'u_6', 'u_7', 'u_admin', 'u_b1', 'u_c1', 'u_c2', + 'u_c3', 'u_c4', 'u_d1', 'u_p1', 'u_r1', 'u_s1', 'u_s2', 'u_u1', +]; + const engines: ObjectQL[] = []; afterEach(async () => { vi.restoreAllMocks(); @@ -103,6 +110,9 @@ async function boot() { for (const o of [SysUser, SysMember]) { if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as never, 'qa.authz-read-set'); } + // [#29029] Each grant's holder is a user the writer reads, so the grant-name + // check is what is measured. + await provisionRigUsers(engine, RIG_USERS); engines.push(engine); const services: Record = { diff --git a/packages/plugins/plugin-security/src/org-admin-assignment-boundary.posture.test.ts b/packages/plugins/plugin-security/src/org-admin-assignment-boundary.posture.test.ts index 83abe90249a..acb1ebea007 100644 --- a/packages/plugins/plugin-security/src/org-admin-assignment-boundary.posture.test.ts +++ b/packages/plugins/plugin-security/src/org-admin-assignment-boundary.posture.test.ts @@ -44,6 +44,7 @@ import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { defaultPermissionSets } from './objects/index.js'; import { maskFieldValue } from './field-masker.js'; +import { provisionRigUsers } from './rig-users.testkit.js'; const SYS = { isSystem: true } as const; const ORG_A = 'org_a'; @@ -109,11 +110,30 @@ const PERMISSIONS = [...defaultPermissionSets, QA_IN_SET, ...DELEGATE_SETS, ORG_ type Posture = 'single' | 'group' | 'isolated'; const POSTURES: Posture[] = ['single', 'group', 'isolated']; +/** [#29029] Every user this suite's writes name or act as: each gets a `sys_user` row. */ +const RIG_USERS = [ + 'u_delegate', 'u_holder_hidden_delegate', 'u_in_0', 'u_in_1', 'u_in_2', 'u_in_3', 'u_in_4', + 'u_in_5', 'u_member', 'u_org_admin', 'u_org_admin_columns_hidden', 'u_org_admin_delegate', + 'u_org_admin_hiding', 'u_org_admin_nb', 'u_org_admin_organization_hidden', + 'u_org_admin_reads_masked', 'u_other_org', 'u_platform', 'u_position_hidden_delegate', + 'u_seeded', 'u_set_hidden_delegate', 'u_target', 'u_unit_hidden_delegate', + 'u_write_only_delegate', +]; + +/** + * [#29029] The users organization A seats — every rig user but the one of + * organization B — as `org_user_ids`, which the runtime fills from + * `sys_member`: an organization-A caller's `sys_user` read returns them, as + * the shipped sets' `id in current_user.org_user_ids` rule answers in a + * deployment. + */ +const ORG_A_USERS = RIG_USERS.filter((id) => id !== 'u_other_org'); + const CALLERS = { - platform: { userId: 'u_platform', positions: [], permissions: ['admin_full_access'], posture: 'PLATFORM_ADMIN', tenantId: ORG_A, accessible_org_ids: [ORG_A] }, - orgAdmin: { userId: 'u_org_admin', positions: [], permissions: ['organization_admin'], tenantId: ORG_A, accessible_org_ids: [ORG_A] }, - orgAdminNoBypass: { userId: 'u_org_admin_nb', positions: [], permissions: ['organization_admin_no_bypass'], tenantId: ORG_A, accessible_org_ids: [ORG_A] }, - member: { userId: 'u_member', positions: [], permissions: ['member_default'], tenantId: ORG_A, accessible_org_ids: [ORG_A] }, + platform: { userId: 'u_platform', positions: [], permissions: ['admin_full_access'], posture: 'PLATFORM_ADMIN', tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_A_USERS }, + orgAdmin: { userId: 'u_org_admin', positions: [], permissions: ['organization_admin'], tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_A_USERS }, + orgAdminNoBypass: { userId: 'u_org_admin_nb', positions: [], permissions: ['organization_admin_no_bypass'], tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_A_USERS }, + member: { userId: 'u_member', positions: [], permissions: ['member_default'], tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_A_USERS }, } as const; type Caller = keyof typeof CALLERS; @@ -171,10 +191,12 @@ async function boot(posture: Posture, options: BootOptions = {}) { ], } as any); await engine.syncSchemas(); - // `sys_user` registered and unprovisioned, as in the sibling suites: the - // engine's `user_id` existence probe stands down and the membership hook is - // what judges the holder. + // [#29029] `sys_user` is provisioned with a row for every user the writes + // name: the engine's `user_id` reference check reads the holder through the + // caller's read door and finds it, and the membership hook is what judges + // the holder. if (!engine.registry.getObject(SysUser.name)) engine.registry.registerObject(SysUser as never, 'qa.org-admin-boundary'); + await provisionRigUsers(engine, RIG_USERS); engines.push(engine); const services: Record = { @@ -328,11 +350,8 @@ describe('[ADR-0090 D12, amended 2026-10-17] the organization administrator assi it('the delegable-scope report offers the org admin its in-boundary positions only, and explain admits it for its member', async () => { const h = await boot(posture); const security = h.plugin as any; - // [#26641] Explain answers only for a user a `sys_user` row carries, so - // the explained member gets one here (the boot leaves the table - // unprovisioned for the write cases above). - await h.engine.syncObjectSchema(SysUser.name); - await h.engine.insert('sys_user', { id: 'u_target', name: 'Target', email: 'u_target@example.test' }, { context: { isSystem: true } } as any); + // [#26641] Explain answers only for a user a `sys_user` row carries; + // the boot gives the explained member one (`RIG_USERS`). for (const context of [CALLERS.orgAdmin, CALLERS.orgAdminNoBypass]) { const sets = await security.resolvePermissionSetsForContext(context); const report = await security.delegatedAdminGate.describeDelegableScope(sets, context); @@ -511,7 +530,7 @@ describe("[ADR-0090 D12; ruling of record 5985885287] an organization administra // ── The delegated branch ──────────────────────────────────────────────────── const delegateContext = (userId: string, permissions: string[]) => - ({ userId, positions: [] as string[], permissions, tenantId: ORG_A, accessible_org_ids: [ORG_A] }); + ({ userId, positions: [] as string[], permissions, tenantId: ORG_A, accessible_org_ids: [ORG_A], org_user_ids: ORG_A_USERS }); type DelegateContext = ReturnType; /** Three principal classes the delegated walk judges, each holding the `a_ops` scope of organization A. */ diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts index a8529e1d90e..81b7bfbc636 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -37,6 +37,7 @@ import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { namesWithoutCatalogRow, positionNotInCatalogMessage } from './position-catalog-refusal.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; +import { provisionRigUsers } from './rig-users.testkit.js'; // --------------------------------------------------------------------------- // Fixture @@ -104,6 +105,14 @@ const ADMIN_IN_GRANT_ORG = { ...ADMIN, tenantId: GRANT_ORG }; /** A position record id — the spelling the measured authors wrote where the position's NAME belongs. */ const AUDITOR_POSITION_ID = 'position_mtur8hz5jp7ptm3r'; +/** [#29029] Every user this suite's writes name or act as: each gets a `sys_user` row. */ +const RIG_USERS = [ + 'u_admin', 'u_b1', 'u_b2', 'u_bx', 'u_echo', 'u_fossil', 'u_holder', 'u_member', 'u_multi', + 'u_named', 'u_ns', 'u_nsu', 'u_ob', 'u_ob2', 'u_op', 'u_org_admin', 'u_plain', 'u_reg', + 'u_retired', 'u_sys', 'u_tok', 'u_tok_ok', 'u_up', 'u_wa', 'u_wb', 'u_wc', 'u_wf', 'u_wm', + 'u_wo', 'u_x', +]; + const engines: ObjectQL[] = []; afterEach(async () => { vi.restoreAllMocks(); @@ -143,6 +152,11 @@ async function boot(opts: { walled?: boolean } = {}) { for (const o of [SysUser, SysMember, SysUserPosition, SysUserPermissionSet]) { if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as never, 'qa.authz-read-set'); } + // [#29029] …except `sys_user`, which is provisioned with a row for every + // holder the writes name: the engine's `user_id` reference check reads the + // holder through the caller's read door, so the position check is what is + // measured. + await provisionRigUsers(engine, RIG_USERS); engines.push(engine); const warn = vi.fn(); diff --git a/packages/plugins/plugin-security/src/predicate-related-read-tenant-scope.test.ts b/packages/plugins/plugin-security/src/predicate-related-read-tenant-scope.test.ts index 7c651c72921..6b3dfb3d161 100644 --- a/packages/plugins/plugin-security/src/predicate-related-read-tenant-scope.test.ts +++ b/packages/plugins/plugin-security/src/predicate-related-read-tenant-scope.test.ts @@ -15,6 +15,13 @@ * real middleware: a caller bound to org X writes a reference to org Y's row, * once with that row `secret` and once `public`. The two must end identically, * the read must return nothing, and the org X control must reach the rule. + * + * [#29029] A write that is not a system write may name only a record its + * caller can read, and that reference check runs before the record's rules + * (ruling records 6573561091 and 6663849489): a reference to org Y's row, or to + * a user the caller cannot read, is answered the lookup's `reference_not_found`. + * The reads counted below are the rule's related reads alone — a read that + * selects a column the predicate names; the reference check reads `id` only. */ import { describe, it, expect, afterEach, vi } from 'vitest'; @@ -173,7 +180,9 @@ async function boot(kind: 'secret' | 'public', posture?: 'single' | 'group' | 'i const find = driver.find.bind(driver); vi.spyOn(driver, 'find').mockImplementation(async (object, ast, options) => { const rows = await find(object, ast, options); - if (object === 'qa_line') readsOfLine.push(rows as unknown[]); + const columns = (ast as { fields?: unknown[] }).fields; + const ruleRead = Array.isArray(columns) && columns.some((column) => column !== 'id'); + if (object === 'qa_line' && ruleRead) readsOfLine.push(rows as unknown[]); if (object === 'sys_user') userColumnsRead.push(...((ast as { fields?: unknown[] }).fields ?? ['*'])); return rows; }); @@ -303,12 +312,13 @@ describe('#18682 — under `isolated`, an org-less writer of an unwalled object /** * `sys_user` has no tenant column, so no organization wall scopes the related * read; its own wall is the shipped `member_default` row-level security. A user - * the caller's own read of `sys_user` does not return is NOT READABLE: the rule - * faults loudly, is never evaluated on that user, and never reads its columns. + * the caller's own read of `sys_user` does not return may not be named + * (#29029): the write answers the field's `reference_not_found`, before the + * rule, which is never evaluated on that user and never reads its columns. */ -describe('#18682 — a related user the caller cannot read makes the rule fault loudly', () => { +describe('#18682, #29029 — a user the caller cannot read cannot be named', () => { const PEERS = { ...CALLER, org_user_ids: ['u_x', 'u_x2'] }; - const NOT_READABLE = "could not read 'sys_user'"; + const NOT_READABLE = 'reviewer: no sys_user record has id "u_y"'; it('CONTROL: the caller’s own read of sys_user returns org X’s users, never org Y’s', async () => { const h = await boot('secret', 'isolated'); @@ -379,12 +389,12 @@ describe('#18682 — a caller with no userId that is not system is bound like a expect(secret.readsOfLine).toEqual([]); }); - it('a user only org Y holds, under no wall: every door refuses, whatever its value — the write as not readable, the preview at its write gate', async () => { + it('a user only org Y holds, under no wall: every door refuses, whatever its value — the write on the reference check, the preview at its write gate', async () => { const banned = await observe('secret', 'u_y', PUBLIC_FORM('qa_review'), 'single', 'qa_review'); const clear = await observe('public', 'u_y', PUBLIC_FORM('qa_review'), 'single', 'qa_review'); expect(clear.seen).toEqual(banned.seen); - expect(banned.seen.refusal).toMatchObject({ code: 'VALIDATION_FAILED', message: expect.stringContaining("could not read 'sys_user'") }); + expect(banned.seen.refusal).toMatchObject({ code: 'VALIDATION_FAILED', message: expect.stringContaining('reviewer: no sys_user record has id "u_y"') }); expect(banned.seen.committed).toBe(0); // [#21079] The preview asks the engine's write gate before it reads anything // related. This grant context names a set the deployment does not register, diff --git a/packages/plugins/plugin-security/src/rig-users.testkit.ts b/packages/plugins/plugin-security/src/rig-users.testkit.ts new file mode 100644 index 00000000000..8b3ffdf22fd --- /dev/null +++ b/packages/plugins/plugin-security/src/rig-users.testkit.ts @@ -0,0 +1,28 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Provision `sys_user` and give each user a suite's writes name a row. + * + * [#29029] A write that is not a system write may name, in a `user` or + * `lookup` field, only a record its caller's own read door returns (ruling + * records 6573561091 and 6663849489), and a read door that fails — a + * registered `sys_user` with no table behind it among them — answers + * `reference_not_found`. A suite whose subject is another predicate on the + * same write (a membership, a catalog name, a boundary) provisions the users + * its writes name, so that predicate is the one it measures. + * + * Call it after `sys_user` is registered. The rows are written as the system; + * each suite's callers read them through their own grants, as in a deployment. + */ +export async function provisionRigUsers( + engine: { + syncObjectSchema(object: string): Promise; + insert(object: string, data: Record, options: unknown): Promise; + }, + ids: readonly string[], +): Promise { + await engine.syncObjectSchema('sys_user'); + for (const id of new Set(ids)) { + await engine.insert('sys_user', { id, name: id, email: `${id}@example.test` }, { context: { isSystem: true } }); + } +} diff --git a/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts b/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts index 33fde7eecaf..1cb93d95612 100644 --- a/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts +++ b/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts @@ -44,6 +44,7 @@ import { SysPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js' import { SysPositionPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; +import { provisionRigUsers } from './rig-users.testkit.js'; const MEMBER_DEFAULT = { name: 'member_default', label: 'Member', objects: {} } as unknown as PermissionSet; const QA_ADMIN = { @@ -65,6 +66,12 @@ const T0 = '2026-11-01T00:00:00.000Z'; const T1 = '2026-12-01T00:00:00.000Z'; const T2 = '2027-01-01T00:00:00.000Z'; +/** [#29029] Every user this suite's writes name or act as: each gets a `sys_user` row. */ +const RIG_USERS = [ + 'u_admin', 'u_b1', 'u_b2', 'u_eq', 'u_from', 'u_g_from', 'u_g_m1', 'u_g_m2', 'u_g_move', + 'u_g_until', 'u_inv', 'u_legacy', 'u_none', 'u_ok', 'u_until', +]; + const engines: ObjectQL[] = []; afterEach(async () => { vi.restoreAllMocks(); @@ -93,6 +100,9 @@ async function boot() { for (const o of [SysUser, SysMember]) { if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as never, 'qa.authz-read-set'); } + // [#29029] Each grant's holder is a user the writer reads, so the window + // order is what is measured. + await provisionRigUsers(engine, RIG_USERS); engines.push(engine); const services: Record = { diff --git a/packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts b/packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts index 8bf66a26a47..8f2d0b653ce 100644 --- a/packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts +++ b/packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts @@ -43,8 +43,17 @@ * * Both controls run on the same harness, so this cannot pass by refusing * everything: the same caller with the column `editable` is admitted by both - * doors, pays exactly ONE related read on each, and DOES receive the rule's - * verdict. + * doors, pays exactly ONE related read for the rule on each (beside the + * reference check's id-only reads), and DOES receive the rule's verdict. + * + * ## …and the caller who cannot read the related row (#29029) + * + * A write that is not a system write may name, in a lookup, only a record its + * caller can read, and that check runs before the record's validation rules + * (ruling records 6573561091 and 6663849489). So every caller here reads + * `crm_account`, and one block drives the editor who may edit the column but + * holds no read on the account: both doors answer the lookup's + * `reference_not_found`, never the rule's verdict. * * ## …and the two PRE-RESOLUTION classes, which are a different shape * @@ -161,7 +170,7 @@ const USER_POSITION = { const FLS_LOCKED: PermissionSet = { name: 'member_default', label: 'Opportunity editor, locked out of the lookup column', - objects: { crm_opportunity: { allowRead: true, allowCreate: true, allowEdit: true } }, + objects: { crm_opportunity: { allowRead: true, allowCreate: true, allowEdit: true }, crm_account: { allowRead: true } }, fields: { 'crm_opportunity.account': { readable: true, editable: false } }, } as unknown as PermissionSet; @@ -169,6 +178,14 @@ const FLS_LOCKED: PermissionSet = { const FLS_OPEN: PermissionSet = { name: 'member_default', label: 'Opportunity editor who may repoint the account', + objects: { crm_opportunity: { allowRead: true, allowCreate: true, allowEdit: true }, crm_account: { allowRead: true } }, + fields: { 'crm_opportunity.account': { readable: true, editable: true } }, +} as unknown as PermissionSet; + +/** [#29029] The control's grant WITHOUT read on the account: it may not name one. */ +const FLS_OPEN_NO_ACCOUNT_READ: PermissionSet = { + name: 'member_default', + label: 'Opportunity editor who may repoint the account, and reads no account', objects: { crm_opportunity: { allowRead: true, allowCreate: true, allowEdit: true } }, fields: { 'crm_opportunity.account': { readable: true, editable: true } }, } as unknown as PermissionSet; @@ -185,6 +202,7 @@ const FULL_CRUD: PermissionSet = { eng_log: { allowRead: true, allowCreate: true, allowEdit: true }, eng_log_amendable: { allowRead: true, allowCreate: true, allowEdit: true }, sys_user_position: { allowRead: true, allowCreate: true, allowEdit: true }, + crm_account: { allowRead: true }, }, fields: { 'eng_log.account': { readable: true, editable: true }, @@ -197,7 +215,7 @@ const FULL_CRUD: PermissionSet = { const TASK_EDITOR: PermissionSet = { name: 'member_default', label: 'Task editor', - objects: { crm_task: { allowRead: true, allowCreate: true, allowEdit: true } }, + objects: { crm_task: { allowRead: true, allowCreate: true, allowEdit: true }, crm_account: { allowRead: true } }, fields: { 'crm_task.account': { readable: true, editable: true } }, } as unknown as PermissionSet; @@ -324,8 +342,10 @@ async function boot(sets: PermissionSet[], opts: { orgScoping?: boolean } = {}) // [ADR-0131 D3/D4] The position the assignment rows name, declared in the // security catalog the assignment refusal reads. engine.registry.registerItem('position', { name: 'sales', label: 'Sales' } as any, 'name' as any, 'test-package'); - d.storeFor('crm_account').set('acc_p', { id: 'acc_p', name: 'P', type: 'partner' }); - d.storeFor('crm_account').set('acc_d', { id: 'acc_d', name: 'D', type: 'direct' }); + // [#29029] In the callers' organization, so a caller with an active + // organization reads them under the organization wall too. + d.storeFor('crm_account').set('acc_p', { id: 'acc_p', name: 'P', type: 'partner', organization_id: 'org-1' }); + d.storeFor('crm_account').set('acc_d', { id: 'acc_d', name: 'D', type: 'direct', organization_id: 'org-1' }); const services: Record = { // The `isolated` posture, as the plugin resolves it with no `tenancy` @@ -369,7 +389,14 @@ async function boot(sets: PermissionSet[], opts: { orgScoping?: boolean } = {}) d.calls.length = 0; return { engine, + /** Every read of the related object, the reference check's included. */ relatedReads: () => d.calls.filter((c) => c.object === 'crm_account').length, + /** + * [#29029] The rule's related reads alone: a read that selects a column the + * predicate names. The reference check reads `id` only. + */ + ruleReads: () => d.calls.filter((c) => c.object === 'crm_account' + && Array.isArray(c.ast?.fields) && c.ast.fields.some((f: string) => f !== 'id')).length, reset: () => { d.calls.length = 0; }, }; } @@ -435,20 +462,20 @@ describe('#18682 — the preview answers nobody the FLS write gate refuses', () let h: Awaited>; beforeEach(async () => { h = await boot([FLS_OPEN]); }); - it('insert() reaches the rule and refuses with the rule, after ONE related read', async () => { + it('insert() reaches the rule and refuses with the rule, after ONE related read for the rule', async () => { const outcome = await attempt( () => h.engine.insert('crm_opportunity', { ...PARTNER_PAYLOAD }, { context: CALLER } as any), ); expect(outcome.ok).toBe(false); expect(outcome.message).toContain(RULE_MESSAGE); - expect(h.relatedReads()).toBe(1); + expect(h.ruleReads()).toBe(1); }); - it('validate() agrees with it, after ONE related read', async () => { + it('validate() agrees with it, after ONE related read for the rule', async () => { const preview = await h.engine.validate( 'crm_opportunity', { ...PARTNER_PAYLOAD }, { mode: 'insert', context: CALLER } as any, ); - expect(h.relatedReads()).toBe(1); + expect(h.ruleReads()).toBe(1); expect(preview.results?.[0]?.valid).toBe(false); expect(JSON.stringify(preview.results?.[0]?.errors ?? [])).toContain(RULE_MESSAGE); }); @@ -466,6 +493,39 @@ describe('#18682 — the preview answers nobody the FLS write gate refuses', () }); }); +/** + * [#29029] The editor who may edit the column and reads no account: it may not + * name one. Both doors answer the lookup's `reference_not_found`, before the + * record's rules, so the rule's verdict never reaches it, for a partner account + * and a direct one alike. + */ +describe('#29029 — a caller who cannot read the related row cannot name it', () => { + let h: Awaited>; + beforeEach(async () => { h = await boot([FLS_OPEN_NO_ACCOUNT_READ]); }); + + for (const account of ['acc_p', 'acc_d']) { + it(`insert() answers reference_not_found on the lookup naming ${account}, never the rule, and stores nothing`, async () => { + const outcome = await attempt( + () => h.engine.insert('crm_opportunity', { name: 'A', amount: 50000, account }, { context: CALLER } as any), + ); + expect(outcome.ok).toBe(false); + expect(outcome.code).toBe('VALIDATION_FAILED'); + expect(outcome.message).toMatch(/^account: /); + expect(outcome.message).not.toContain(RULE_MESSAGE); + }); + + it(`validate() agrees with it for ${account}`, async () => { + const preview = await h.engine.validate( + 'crm_opportunity', { name: 'A', amount: 50000, account }, { mode: 'insert', context: CALLER } as any, + ); + expect(preview.results?.[0]?.valid).toBe(false); + expect((preview.results?.[0]?.errors ?? []).map((e: any) => ({ field: e.field, code: e.code }))) + .toEqual([{ field: 'account', code: 'reference_not_found' }]); + expect(JSON.stringify(preview.results?.[0]?.errors ?? [])).not.toContain(RULE_MESSAGE); + }); + } +}); + /** * ⭐ [#18682] The two PRE-RESOLUTION caller-class refusals, on the same composed * runtime. They are a different shape from W6 above and the difference is the @@ -528,20 +588,20 @@ describe('#18682 — the preview answers nobody the two pre-resolution gates ref let h: Awaited>; beforeEach(async () => { h = await boot([FULL_CRUD]); }); - it('insert() reaches the rule and refuses with the rule, after ONE related read', async () => { + it('insert() reaches the rule and refuses with the rule, after ONE related read for the rule', async () => { const outcome = await attempt( () => h.engine.insert('eng_log_amendable', { ...PARTNER_PAYLOAD }, { context: CALLER } as any), ); expect(outcome.ok).toBe(false); expect(outcome.message).toContain(RULE_MESSAGE); - expect(h.relatedReads()).toBe(1); + expect(h.ruleReads()).toBe(1); }); - it('validate() agrees with it, after ONE related read', async () => { + it('validate() agrees with it, after ONE related read for the rule', async () => { const preview = await h.engine.validate( 'eng_log_amendable', { ...PARTNER_PAYLOAD }, { mode: 'insert', context: CALLER } as any, ); - expect(h.relatedReads()).toBe(1); + expect(h.ruleReads()).toBe(1); expect(preview.results?.[0]?.valid).toBe(false); expect(JSON.stringify(preview.results?.[0]?.errors ?? [])).toContain(RULE_MESSAGE); }); @@ -597,20 +657,20 @@ describe('#18682 — the preview answers nobody the two pre-resolution gates ref let h: Awaited>; beforeEach(async () => { h = await boot([TENANT_ADMIN]); }); - it('insert() reaches the rule and refuses with the rule, after ONE related read', async () => { + it('insert() reaches the rule and refuses with the rule, after ONE related read for the rule', async () => { const outcome = await attempt( () => h.engine.insert('sys_user_position', { ...RBAC_PARTNER_PAYLOAD }, { context: ADMIN_CALLER } as any), ); expect(outcome.ok).toBe(false); expect(outcome.message).toContain(RULE_MESSAGE); - expect(h.relatedReads()).toBe(1); + expect(h.ruleReads()).toBe(1); }); - it('validate() agrees with it, after ONE related read', async () => { + it('validate() agrees with it, after ONE related read for the rule', async () => { const preview = await h.engine.validate( 'sys_user_position', { ...RBAC_PARTNER_PAYLOAD }, { mode: 'insert', context: ADMIN_CALLER } as any, ); - expect(h.relatedReads()).toBe(1); + expect(h.ruleReads()).toBe(1); expect(preview.results?.[0]?.valid).toBe(false); expect(JSON.stringify(preview.results?.[0]?.errors ?? [])).toContain(RULE_MESSAGE); }); @@ -664,20 +724,20 @@ describe('#18682 — the preview answers nobody the organization wall refuses', let h: Awaited>; beforeEach(async () => { h = await boot([TASK_EDITOR], { orgScoping: true }); }); - it('insert() reaches the rule and refuses with the rule, after ONE related read', async () => { + it('insert() reaches the rule and refuses with the rule, after ONE related read for the rule', async () => { const outcome = await attempt( () => h.engine.insert('crm_task', { ...PARTNER_PAYLOAD }, { context: CALLER } as any), ); expect(outcome.ok).toBe(false); expect(outcome.message).toContain(RULE_MESSAGE); - expect(h.relatedReads()).toBe(1); + expect(h.ruleReads()).toBe(1); }); - it('validate() agrees with it, after ONE related read', async () => { + it('validate() agrees with it, after ONE related read for the rule', async () => { const preview = await h.engine.validate( 'crm_task', { ...PARTNER_PAYLOAD }, { mode: 'insert', context: CALLER } as any, ); - expect(h.relatedReads()).toBe(1); + expect(h.ruleReads()).toBe(1); expect(preview.results?.[0]?.valid).toBe(false); expect(JSON.stringify(preview.results?.[0]?.errors ?? [])).toContain(RULE_MESSAGE); }); From eda7ee248615f7eab7ec3907be6888ce561be363 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 17:37:53 +0000 Subject: [PATCH 09/26] docs(qa): retire the write-without-reading-the-parent persona The records-forms predicate-relationship-traversal item takes revision 7: a member who cannot read the lookup parent may not name it, and is answered reference_not_found for a tripping and a non-tripping parent alike. The dogfood case is rewritten to that rule, and the changeset states the refused writes and the delegate's business-unit read. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- .changeset/29029-lookup-user-caller-read.md | 15 +++---- .../areas/records-forms.json | 22 ++++++---- ...ate-relationship-traversal.dogfood.test.ts | 41 ++++++++++++------- 3 files changed, 46 insertions(+), 32 deletions(-) diff --git a/.changeset/29029-lookup-user-caller-read.md b/.changeset/29029-lookup-user-caller-read.md index 14871b38c5b..e8434a70740 100644 --- a/.changeset/29029-lookup-user-caller-read.md +++ b/.changeset/29029-lookup-user-caller-read.md @@ -1,20 +1,17 @@ --- '@objectstack/objectql': patch +'@objectstack/plugin-security': patch --- fix(objectql)!: a non-system write may name, in a `lookup` or `user` field, only a record the caller can read (#29029) -Clause-②: yes (narrowing) — a non-system write whose `lookup` or `user` value names a record the caller's own read door does not return is refused with `reference_not_found`, which it admitted before. Nothing is widened. +Clause-②: yes (narrowing) — the write's reference check narrows: a non-system `lookup` or `user` value naming a record the caller cannot read is refused. The business-unit read widens: a delegated admin reads the business units inside the admin scope it holds, and no other. Metadata-protocol: none - + -**BREAKING** accept-set narrowing at the write's reference check (`@objectstack/objectql`), served by every write door (REST, MCP, flows run as the user, `ctx.api` in hooks). +**BREAKING** accept-set narrowing at the write's reference check (`@objectstack/objectql`), served by every write door. -After the upgrade, an insert or update that carries no `isSystem` is refused when a `lookup` or `user` value it supplies names a record the caller cannot read: the answer is `400 VALIDATION_FAILED` with the field's `reference_not_found`, localized, exactly the answer for a record that exists nowhere. This is the rule `master_detail` already follows. +After the upgrade, an insert or update that is not a system write is refused with `400 VALIDATION_FAILED` and the field's localized `reference_not_found` when a `lookup` or `user` value it supplies names a record the caller cannot read, before the record's validation rules run. -- **Each value is judged.** A multi-valued `lookup` or `user` field is refused when any one of its values names a record the caller cannot read. -- **Unchanged.** System writes (`isSystem`: seed replay, boot, package install) stay exempt, as before; a flow run's elevated write keeps its existence check. An update that carries back a value the row already stores is not judged again. An empty value is not a reference. A `readonly` field is not the caller's to answer for. A `tree` parent keeps the existence check alone. -- **Who sees it.** An app whose users write a reference to a record their own access does not return. A user can still reference every record they can read. - -**The remedy.** None for an app that meant its read access as the boundary. An app that wrote its own `beforeInsert` / `beforeUpdate` hook to refuse a reference to an unreadable record can delete it: the engine now refuses it, localized. A flow that must link records the acting user cannot read runs with `runAs: 'system'`. +A delegated admin whose admin scope covers a business-unit subtree can read the business units in that subtree, and no business unit outside it (`@objectstack/plugin-security`). diff --git a/docs/qa/platform-checklist/areas/records-forms.json b/docs/qa/platform-checklist/areas/records-forms.json index fee8a9bec68..f8c5cf5290e 100644 --- a/docs/qa/platform-checklist/areas/records-forms.json +++ b/docs/qa/platform-checklist/areas/records-forms.json @@ -5546,12 +5546,12 @@ "title": "A validation rule reads a parent field one hop through a lookup", "since": "v17", "status": "active", - "revision": 6, + "revision": 7, "priority": "P1", "surface": "api", "personas": [ "seeded admin (admin@objectos.ai / admin123)", - "a member with create/edit on a child object and NO read on its lookup parent — the persona the rule exists to constrain. ⚠ No stock showcase persona is one (fixtures.knownGaps): acceptance[3] is scored from the engine pin or on a scratch pair" + "a member with create/edit on a child object and NO read on its lookup parent — who may not name that parent at all: a non-system write may name, in a lookup, only a record the caller's own read door returns. ⚠ No stock showcase persona is one (fixtures.knownGaps): acceptance[3] is scored on a scratch pair" ], "fixtures": { "app": "showcase", @@ -5561,14 +5561,14 @@ "at least one churned account and one non-churned account to invoice against" ], "knownGaps": [ - "No stock persona has showcase_invoice CRUD WITHOUT showcase_account read, and none can be made from the stock showcase objects: showcase_member_default, the everyone baseline every member holds, grants showcase_account allowRead (examples/app-showcase/src/security/permission-sets.ts), and permission sets only ever ADD grants, so no extra set takes it away (QA run #21720, parked on #22017 row 14). An earlier revision of this item said the opposite, reading packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts. That test's persona is its own injected fallback set, not a stock persona, and it never writes against a churned account. So acceptance[3] is scored from the ENGINE pin in `automated`, which proves the mechanism the clause rests on: the related row is read under SYSTEM authority with the acting identity carried, and the write path accepts and refuses by the parent field. Or it is scored live on the scratch substitute #21720 measured, which is runtime-only on a stock boot (QA run #27438 built and drove it at c4c515ad): POST /api/v1/packages with a declared namespace; PUT /api/v1/meta/object for a parent object, and for a child object carrying a one-hop script validation over its lookup to the parent; PUT /api/v1/meta/permission for a set granting the member the CHILD only, bound with POST /api/v1/data/sys_user_permission_set; then, as that member, write against a tripping and a non-tripping parent. The control is the member's GET of a parent row, which must answer 403: without it the leg proves nothing about an unreadable parent" + "No stock persona has showcase_invoice CRUD WITHOUT showcase_account read, and none can be made from the stock showcase objects: showcase_member_default, the everyone baseline every member holds, grants showcase_account allowRead (examples/app-showcase/src/security/permission-sets.ts), and permission sets only ever ADD grants, so no extra set takes it away (QA run #21720, parked on #22017 row 14). An earlier revision of this item said the opposite, reading packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts. That test's persona is its own injected fallback set, not a stock persona, and it never writes against a churned account. So acceptance[3] is scored on a scratch pair: the dogfood pin in `automated` builds one at boot, or it is scored live on the scratch substitute #21720 measured, which is runtime-only on a stock boot (QA run #27438 built and drove it at c4c515ad): POST /api/v1/packages with a declared namespace; PUT /api/v1/meta/object for a parent object, and for a child object carrying a one-hop script validation over its lookup to the parent; PUT /api/v1/meta/permission for a set granting the member the CHILD only, bound with POST /api/v1/data/sys_user_permission_set; then, as that member, write against a tripping and a non-tripping parent. The control is the member's GET of a parent row, which must answer 403: without it the leg proves nothing about an unreadable parent" ] }, "steps": [ "POST /api/v1/data/showcase_invoice with `account` pointing at a NON-churned account", "POST /api/v1/data/showcase_invoice with `account` pointing at a CHURNED account", "PATCH an existing invoice to REPOINT it at a churned account (the update path reads the FK off the merged record, not the patch alone)", - "as a persona with NO read on the parent (none exists on stock — fixtures.knownGaps): run the engine pin, or on the scratch substitute write against a non-tripping parent, then a tripping one" + "as a member with NO read on the parent (none exists on stock — fixtures.knownGaps), on the scratch pair: write against a non-tripping parent, then a tripping one" ], "acceptance": [ { @@ -5590,23 +5590,23 @@ "evidence": "the patch response" }, { - "clause": "a rule whose parent the acting user cannot read still ENFORCES, and still lets a legitimate write through — the read is system-authority, bounded by the fields the predicate names", + "clause": "a member who cannot read the parent cannot name it: the write answers reference_not_found on the lookup, before the record's rules, whether or not the parent trips one", "oracle": "api", - "verify": "on stock showcase no such persona exists (fixtures.knownGaps), so score this from the engine pin in `automated` or live on the scratch substitute. The engine-pin verdict is a layer exception this item declares (fixtures.knownGaps), not a RUNNER rule 6 pass of this api clause: the run record says the verdict rests on the engine layer. With such a persona, the non-churned write is 2xx and the churned write is 400 carrying the AUTHORED message; an unevaluable-rule envelope for this persona is the defect this clause exists to catch, and so is a 2xx on the churned account", + "verify": "on stock showcase no such persona exists (fixtures.knownGaps), so score this on the scratch pair. For the member who cannot read the parent, the write naming the non-tripping parent and the write naming the tripping parent are each 400 VALIDATION_FAILED with exactly one fields[] entry, { field: , code: 'reference_not_found' } — the answer for a parent that exists nowhere — and nothing is stored. Neither answer carries the rule's authored message or the unevaluable-rule fault, and the member's own GET of the parent answers 403 (the control)", "evidence": "both responses for the restricted persona" } ], "negative": [ "a 2xx on the churned-account create is a FAIL — the rule did not run", "a rejection whose text is the unevaluable-rule fault on the READABLE-parent cases is a FAIL: the related row was not preloaded and the rule is rejecting every write, which is the pre-capability behaviour", - "a 400 unevaluable-rule envelope for the persona who cannot read the parent is a FAIL: it means the related read was not system-authority, and the rule cannot be authored for the persona it exists to constrain. A 2xx on the CHURNED account is also a FAIL — the rule did not enforce" + "for the member who cannot read the parent, a 2xx is a FAIL — it named a record it cannot read — and so is a refusal carrying the rule's authored message or the unevaluable-rule fault instead of the lookup's reference_not_found: the write answered for a parent its caller cannot read" ], "traps": [ "seed-data-thin" ], "automated": { "kind": "dogfood", - "ref": "packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts, at the REST data door of a booted stack: on the showcase as shipped, acceptance[0] (an invoice on an active account is 2xx and reads back, no unevaluable fault), acceptance[1] and negative[0] (an invoice on a churned account is 400 VALIDATION_FAILED with one _record rule_violation entry carrying the authored message, and nothing is stored), acceptance[2] (a PATCH carrying only the repointed account is refused and the stored account is unchanged) and negative[1] (no readable-parent case answers the unevaluable fault); on a scratch pair, acceptance[3] and negative[2] for the persona no stock showcase persona is (fixtures.knownGaps): a member with create/edit on the child and no grant on the parent, whose parent read is the control (403 PERMISSION_DENIED), writes 2xx against a non-tripping parent and 400 with the authored message against a tripping one. packages/objectql/src/engine-predicate-relationship.test.ts stays engine-layer evidence of the mechanism ('reads the related row under SYSTEM authority' and its two siblings)" + "ref": "packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts, at the REST data door of a booted stack: on the showcase as shipped, acceptance[0] (an invoice on an active account is 2xx and reads back, no unevaluable fault), acceptance[1] and negative[0] (an invoice on a churned account is 400 VALIDATION_FAILED with one _record rule_violation entry carrying the authored message, and nothing is stored), acceptance[2] (a PATCH carrying only the repointed account is refused and the stored account is unchanged) and negative[1] (no readable-parent case answers the unevaluable fault); on a scratch pair, acceptance[3] and negative[2] for a member no stock showcase persona is (fixtures.knownGaps): a member with create/edit on the child and no grant on the parent, whose parent read is the control (403 PERMISSION_DENIED), is answered 400 VALIDATION_FAILED with the lookup's reference_not_found against a non-tripping and a tripping parent alike, never the authored message, and nothing is stored. packages/objectql/src/engine-predicate-relationship.test.ts stays engine-layer evidence of the mechanism ('reads the related row under SYSTEM authority' and its two siblings)" }, "source": [ "examples/app-showcase/src/data/objects/invoice.object.ts (no_invoice_for_churned_account)", @@ -5649,6 +5649,12 @@ "date": "2026-10-31", "change": "fixtures.knownGaps[0] no longer says the restricted persona cannot be provisioned. From QA run #27438 (W3-C2b, at c4c515ad; wave anchor #27377, wave-3 accuracy slice S1). It still holds that no STOCK persona has invoice CRUD without account read. But the scratch substitute is runtime-only on a stock boot: the run installed a package, authored a parent and a child (a one-hop script validation on the child), granted the member the child alone and bound the set, with the member's 403 on a parent read as the control. The gap now spells that sequence and the control. acceptance[3] keeps both of its routes (the engine pin, or the live substitute); no clause changed", "ref": "#27438" + }, + { + "revision": 7, + "date": "2026-11-05", + "change": "the persona that wrote a child without reading its lookup parent is retired: a non-system write may name, in a lookup or user field, only a record the caller's own read door returns, and that reference check runs before the record's validation rules (ruling records 6573561091 and 6663849489). acceptance[3], its verify, negative[2], step 4, the second persona and automated.ref now state that rule: the member who cannot read the parent is answered reference_not_found against a tripping and a non-tripping parent alike. acceptance[0]–[2] are unchanged", + "ref": "#29029" } ] }, diff --git a/packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts b/packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts index 2ea512a737b..d290a9755c5 100644 --- a/packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts +++ b/packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts @@ -14,14 +14,16 @@ // invoice against each (acceptance[0], acceptance[1], negative[0], // negative[1]) and repoints a stored invoice at a churned account with a // patch that carries no status (acceptance[2]). -// - A scratch pair for the persona the rule exists to constrain, which no -// stock showcase persona is (the item's fixtures.knownGaps): a member with -// create/edit on the child object and NO grant at all on the lookup's -// parent. The member writes against a non-tripping and a tripping parent -// (acceptance[3], negative[2]); a read of the parent as that member is the -// control that the persona really cannot read it. +// - A scratch pair for a member with create/edit on the child object and NO +// grant at all on the lookup's parent, which no stock showcase persona is +// (the item's fixtures.knownGaps). Such a member may not name the parent at +// all (#29029, revision 7): a write naming a parent the caller's own read +// door does not return answers `reference_not_found` on the lookup, before +// the record's rules, whether or not the parent trips one (acceptance[3], +// negative[2]). A read of the parent as that member is the control that the +// member really cannot read it. // -// The refusal is scored on the envelope — 400 VALIDATION_FAILED, the +// The rule's refusal is scored on the envelope — 400 VALIDATION_FAILED, the // `_record` / `rule_violation` entry — and on the AUTHORED message, which is // the one place the item names wording as the contract. A refusal that reads // "could not be evaluated" is the pre-capability fault, and every case below @@ -54,6 +56,14 @@ function expectRuleRefusal(answer: Answer, message: string) { expect(JSON.stringify(answer.json)).not.toContain(UNEVALUABLE); } +/** The refusal acceptance[3] names: the lookup's own `reference_not_found`, and no rule entry. */ +function expectReferenceRefusal(answer: Answer, field: string) { + expect(answer.status, JSON.stringify(answer.json)).toBe(400); + expect(answer.json?.code, JSON.stringify(answer.json)).toBe('VALIDATION_FAILED'); + expect(ruleEntries(answer.json).map((f) => ({ field: f.field, code: f.code }))).toEqual([{ field, code: 'reference_not_found' }]); + expect(JSON.stringify(answer.json)).not.toContain(UNEVALUABLE); +} + function expectAdmitted(answer: Answer) { expect(answer.status, JSON.stringify(answer.json)).toBeLessThan(300); expect(JSON.stringify(answer.json)).not.toContain(UNEVALUABLE); @@ -122,7 +132,7 @@ describe('records-forms.predicate-relationship-traversal — the showcase invoic }); }); -// ── The persona the rule exists to constrain ─────────────────────────────── +// ── A member who cannot read the parent ──────────────────────────────────── const PARENT = 'prt_parent'; const CHILD = 'prt_child'; @@ -162,7 +172,7 @@ const prtStack = defineStack({ ], } as never); -describe('records-forms.predicate-relationship-traversal — a member who cannot read the parent', () => { +describe('records-forms.predicate-relationship-traversal — a member who cannot read the parent cannot name it', () => { let stack: VerifyStack; let member = ''; let openId = ''; @@ -196,14 +206,15 @@ describe('records-forms.predicate-relationship-traversal — a member who cannot expect(read.json.code).toBe('PERMISSION_DENIED'); }); - it('acceptance[3]: a legitimate write against a non-tripping parent goes through', async () => { - const created = await call('POST', `/data/${CHILD}`, { name: 'prt-open', parent: openId }); - expectAdmitted(created); - expect(await stack.rows(CHILD, { name: 'prt-open' })).toHaveLength(1); + it('acceptance[3]: a write naming a non-tripping parent the member cannot read answers reference_not_found, and stores nothing', async () => { + expectReferenceRefusal(await call('POST', `/data/${CHILD}`, { name: 'prt-open', parent: openId }), 'parent'); + expect(await stack.rows(CHILD, { name: 'prt-open' })).toHaveLength(0); }); - it('acceptance[3], negative[2]: a write against a tripping parent is refused with the authored message, never the unevaluable fault', async () => { - expectRuleRefusal(await call('POST', `/data/${CHILD}`, { name: 'prt-blocked', parent: blockedId }), BLOCKED_MESSAGE); + it('acceptance[3], negative[2]: a write naming a tripping parent the member cannot read answers the same, never the rule\'s message', async () => { + const answer = await call('POST', `/data/${CHILD}`, { name: 'prt-blocked', parent: blockedId }); + expectReferenceRefusal(answer, 'parent'); + expect(JSON.stringify(answer.json)).not.toContain(BLOCKED_MESSAGE); expect(await stack.rows(CHILD, { name: 'prt-blocked' })).toHaveLength(0); }); }); From 2e53abd8d55b1291f3098978e56d9fff6fdd0e93 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 17:43:14 +0000 Subject: [PATCH 10/26] test(plugin-audit): count the audit summary's reads, not the write's reference check The write's reference check reads each lookup or user value it names through the caller's read door, projected to id alone. The read-count pins measure what the audit summary reads, so that read is left out of their counters. The log line for a failed caller read names a referenced record rather than a master-detail header, as the read now serves lookup and user values too. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- packages/objectql/src/engine-referential-integrity.ts | 2 +- .../plugins/plugin-audit/src/audit-lookup-summary.test.ts | 7 ++++++- .../plugin-audit/src/audit-milestone-summary.test.ts | 7 ++++++- .../plugin-audit/src/audit-option-label-summary.test.ts | 7 ++++++- .../src/implicit-reference-target-summary.test.ts | 7 ++++++- 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/objectql/src/engine-referential-integrity.ts b/packages/objectql/src/engine-referential-integrity.ts index 694ea14a431..d34ca83c39c 100644 --- a/packages/objectql/src/engine-referential-integrity.ts +++ b/packages/objectql/src/engine-referential-integrity.ts @@ -384,7 +384,7 @@ export async function masterDetailHeadersCallerReads( } return served; } catch (err) { - this.logger?.warn?.('the caller\'s read of the master-detail header failed — the header binds as a missing header', { + this.logger?.warn?.('the caller\'s read of the referenced record failed — it answers as a record that does not exist', { object: master, error: err, }); return new Set(); diff --git a/packages/plugins/plugin-audit/src/audit-lookup-summary.test.ts b/packages/plugins/plugin-audit/src/audit-lookup-summary.test.ts index d2e46f1770c..bab96ed469d 100644 --- a/packages/plugins/plugin-audit/src/audit-lookup-summary.test.ts +++ b/packages/plugins/plugin-audit/src/audit-lookup-summary.test.ts @@ -174,7 +174,12 @@ function makeCountingDriver() { async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, async find(object: string, ast: any) { - reads.findOn[object] = (reads.findOn[object] ?? 0) + 1; + // [#29029] The write's own reference check reads each lookup / user + // value it names through the caller's read door, projected to `id` + // alone: that read is the write's, not this plugin's, so it is not + // counted here. + const referenceCheckRead = Array.isArray(ast?.fields) && ast.fields.length === 1 && ast.fields[0] === 'id'; + if (!referenceCheckRead) reads.findOn[object] = (reads.findOn[object] ?? 0) + 1; return Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)).map(copy); }, async findOne(object: string, ast: any) { diff --git a/packages/plugins/plugin-audit/src/audit-milestone-summary.test.ts b/packages/plugins/plugin-audit/src/audit-milestone-summary.test.ts index af4f6196991..2f9d84b1338 100644 --- a/packages/plugins/plugin-audit/src/audit-milestone-summary.test.ts +++ b/packages/plugins/plugin-audit/src/audit-milestone-summary.test.ts @@ -223,7 +223,12 @@ function makeCountingDriver() { async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, async find(object: string, ast: any) { - reads.findOn[object] = (reads.findOn[object] ?? 0) + 1; + // [#29029] The write's own reference check reads each lookup / user + // value it names through the caller's read door, projected to `id` + // alone: that read is the write's, not this plugin's, so it is not + // counted here. + const referenceCheckRead = Array.isArray(ast?.fields) && ast.fields.length === 1 && ast.fields[0] === 'id'; + if (!referenceCheckRead) reads.findOn[object] = (reads.findOn[object] ?? 0) + 1; return Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)).map(copy); }, async findOne(object: string, ast: any) { diff --git a/packages/plugins/plugin-audit/src/audit-option-label-summary.test.ts b/packages/plugins/plugin-audit/src/audit-option-label-summary.test.ts index df74d2ad48a..d796a48ff90 100644 --- a/packages/plugins/plugin-audit/src/audit-option-label-summary.test.ts +++ b/packages/plugins/plugin-audit/src/audit-option-label-summary.test.ts @@ -247,7 +247,12 @@ function makeCountingDriver() { async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, async find(object: string, ast: any) { - reads.findOn[object] = (reads.findOn[object] ?? 0) + 1; + // [#29029] The write's own reference check reads each lookup / user + // value it names through the caller's read door, projected to `id` + // alone: that read is the write's, not this plugin's, so it is not + // counted here. + const referenceCheckRead = Array.isArray(ast?.fields) && ast.fields.length === 1 && ast.fields[0] === 'id'; + if (!referenceCheckRead) reads.findOn[object] = (reads.findOn[object] ?? 0) + 1; return Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)).map(copy); }, async findOne(object: string, ast: any) { diff --git a/packages/plugins/plugin-audit/src/implicit-reference-target-summary.test.ts b/packages/plugins/plugin-audit/src/implicit-reference-target-summary.test.ts index 56ce425dcac..da7627d34cb 100644 --- a/packages/plugins/plugin-audit/src/implicit-reference-target-summary.test.ts +++ b/packages/plugins/plugin-audit/src/implicit-reference-target-summary.test.ts @@ -194,8 +194,13 @@ function makeCountingDriver() { // non-function declaration it cannot supply — `reads` among them — and // arithmetic on that stub throws before the body can be judged at all, // which files the double as UNJUDGED rather than as conforming. + // [#29029] The write's own reference check reads each lookup / user + // value it names through the caller's read door, projected to `id` + // alone: that read is the write's, not this plugin's, so it is not + // counted here. + const referenceCheckRead = Array.isArray(ast?.fields) && ast.fields.length === 1 && ast.fields[0] === 'id'; const prior = reads.findOn[object]; - reads.findOn[object] = (typeof prior === 'number' ? prior : 0) + 1; + if (!referenceCheckRead) reads.findOn[object] = (typeof prior === 'number' ? prior : 0) + 1; const matched = Array.from(storeFor(object).values()) .filter((r) => matches(r, ast?.where)); // The caller's bound: applied by PRESENCE (so `limit: 0` returns nothing), From 9af142fc18be87439b7d2869befd72f9b1d85be2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 17:43:15 +0000 Subject: [PATCH 11/26] docs: a write names only a record its caller can read; a delegate reads its scope's units The error catalog's reference callout and the field reference's referential integrity section state that a non-system write may name, in a lookup, master_detail or user field, only a record its caller can read, before the record's validation rules. The delegated administration page states that an admin scope grants read on the business units inside its subtree. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- content/docs/api/error-catalog.mdx | 16 ++++++++++------ content/docs/data-modeling/fields.mdx | 5 ++++- .../permissions/delegated-administration.mdx | 5 +++++ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index c24afb9dbff..9cbcffc653d 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -316,12 +316,16 @@ object is rejected with `400 VALIDATION_FAILED`, and the specifics ride in } ``` -The check covers create, update and bulk update. Four cases are deliberately -*not* rejections: an empty value (`null` / `""` / `[]`) means "no link", a -`isSystem` write is exempt (seed replay and package install legitimately write -in an order that only resolves once the batch completes), a target that -cannot be checked at all — an unregistered object, an unreachable datasource — -fails **open** rather than inventing a rejection, and an update that carries back +The check covers create, update and bulk update. A write that is not a system +write may name, in a `lookup`, `master_detail` or `user` field, only a record +the caller can read: a record the caller cannot read is answered exactly as one +that exists nowhere, before the record's validation rules run, and so is one +whose read by the caller fails. Four cases are deliberately *not* rejections: an +empty value (`null` / `""` / `[]`) means "no link", a `isSystem` write is exempt +(seed replay and package install legitimately write in an order that only +resolves once the batch completes), a target that cannot be checked at all — an +unregistered object, or an unreachable datasource on a check that does not read +as the caller — fails **open** rather than inventing a rejection, and an update that carries back a value (or a member of a multi-valued field) the row already stores is not judged again, because the update does not add it; on a bulk update that holds only when every matched row already stores it. diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index f1193ac4da0..378cd07253f 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/content/docs/data-modeling/fields.mdx @@ -283,7 +283,10 @@ The key's full contract is in the [Field reference](/docs/references/data/field) `reference` is **enforced on write**. A create or update that sets a `lookup` / `master_detail` to an id with no matching row in the target object is rejected with `400 VALIDATION_FAILED` and a `fields[]` entry whose `code` is -`reference_not_found` (see the [error catalog](/docs/api/error-catalog)). The +`reference_not_found` (see the [error catalog](/docs/api/error-catalog)). A +write that is not a system write may name, in a `lookup`, `master_detail` or +`user` field, only a record its caller can read: one the caller cannot read is +rejected the same way, before the record's validation rules run. The same check runs on bulk updates. An update judges only the references it adds: a value the row already stores, carried back unchanged, is not judged again, so a deleted user's id left in a `user` field does not block later edits of the record. diff --git a/content/docs/permissions/delegated-administration.mdx b/content/docs/permissions/delegated-administration.mdx index 4c5ad992fe5..4a1f0aa0fba 100644 --- a/content/docs/permissions/delegated-administration.mdx +++ b/content/docs/permissions/delegated-administration.mdx @@ -56,6 +56,11 @@ delegate holds. In the permission set editor (Studio's **All metadata** area), t block lists the five, marks which ones the set already grants, and adds the missing ones to the set in one click. +The scope itself grants read on the business units inside its subtree, and on +no unit outside it, so a delegate can pick the units it administers without a +`sys_business_unit` grant. A `sys_business_unit` read grant the delegate holds +reads as that grant says, as before. + ## What the runtime gate enforces Writes to the governed tables (`sys_user_position`, `sys_user_permission_set`, From d2b3757e66f278b842cbf877023c48f5f9f85480 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 18:17:19 +0000 Subject: [PATCH 12/26] test(objectql): a header the write names answers the reference check first With the reference check before the record's validation rules, a non-system write that names a master-detail header it cannot read, or one that exists nowhere, answers the field's reference_not_found rather than the rule's unevaluable refusal; a header the row already stores is still bound by the rules, and a system write keeps the rules first. The header-read counts add the reference check's read door, and the stored-metadata suites count only reads that ask for more than id. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- ...e-formula-hop-stored-metadata-body.test.ts | 22 ++++++++++--- ...ine-hydration-stored-metadata-body.test.ts | 14 +++++++-- ...icate-related-master-detail-header.test.ts | 29 +++++++++++++---- .../src/engine-reference-tenant-scope.test.ts | 31 ++++++++++--------- ...ne-unreadable-master-detail-header.test.ts | 9 ++++-- 5 files changed, 75 insertions(+), 30 deletions(-) diff --git a/packages/objectql/src/engine-formula-hop-stored-metadata-body.test.ts b/packages/objectql/src/engine-formula-hop-stored-metadata-body.test.ts index 35c38eda55b..7a6a40835a1 100644 --- a/packages/objectql/src/engine-formula-hop-stored-metadata-body.test.ts +++ b/packages/objectql/src/engine-formula-hop-stored-metadata-body.test.ts @@ -119,6 +119,13 @@ for (const family of STORED_METADATA_BODY_OBJECTS) { let d: ReturnType; let warns: ReturnType['warns']; const familyReads = () => d.calls.filter((c) => c.object === family).length; + /** + * [#29029] Reads of the family that ask for more than `id`: a member's + * write that names a family row has it checked through its read door, + * projected to `id` alone, and that read serves no body. + */ + const familyBodyReads = () => d.calls.filter((c) => c.object === family + && !(Array.isArray(c.ast?.fields) && c.ast.fields.length === 1 && c.ast.fields[0] === 'id')).length; const faultsOn = (field: string) => warns.filter((w) => w.meta?.object === 'probe_carrier' && w.meta?.field === field); const servesNothing = (value: unknown) => { const text = JSON.stringify(value); @@ -200,13 +207,20 @@ for (const family of STORED_METADATA_BODY_OBJECTS) { } as any, 'test-package'); for (const context of [MEMBER, SYSTEM]) { const err = await engine.insert('probe_ruled', { meta: 'm1' }, { context }).then(() => undefined, (e) => e); - expect(err).toMatchObject({ code: 'VALIDATION_FAILED', fields: [expect.objectContaining({ field: '_record', code: 'rule_violation' })] }); - // No verdict over the body: the refusal is the unevaluable one, never the rule's own. + // No verdict over the body: never the rule's own refusal. expect(String(err.message)).not.toBe('probe hit'); - expect(String(err.message)).toContain(`could not read '${family}'`); servesNothing(err.message); + if (context === MEMBER) { + // [#29029] A member's write names a record its read door does not + // return: the reference check answers, before the record's rules. + expect(err).toMatchObject({ code: 'VALIDATION_FAILED', fields: [expect.objectContaining({ field: 'meta', code: 'reference_not_found' })] }); + continue; + } + // A system write keeps the rules first: the refusal is the unevaluable one. + expect(err).toMatchObject({ code: 'VALIDATION_FAILED', fields: [expect.objectContaining({ field: '_record', code: 'rule_violation' })] }); + expect(String(err.message)).toContain(`could not read '${family}'`); } - expect(familyReads()).toBe(0); + expect(familyBodyReads()).toBe(0); }); }); } diff --git a/packages/objectql/src/engine-hydration-stored-metadata-body.test.ts b/packages/objectql/src/engine-hydration-stored-metadata-body.test.ts index c93eb73dbe9..2bb80f85933 100644 --- a/packages/objectql/src/engine-hydration-stored-metadata-body.test.ts +++ b/packages/objectql/src/engine-hydration-stored-metadata-body.test.ts @@ -187,9 +187,17 @@ for (const family of STORED_METADATA_BODY_OBJECTS) { const holding = await failure(engine.insert('probe_detail', { hdr: 'm1' }, { context })); const control = await failure(engine.insert('probe_detail', { hdr: 'm2' }, { context })); expect(holding).toMatchObject({ code: 'VALIDATION_FAILED' }); - expect(holding?.message).toContain("reads 'parent'"); - // The verdict does not depend on the row: the bit the body would decide is not served. - expect(control).toEqual(holding); + if (context === MEMBER) { + // [#29029] A member's write names a header its read door does not + // return: the reference check answers, before the record's rules, + // alike for either header. + expect(holding?.message).toBe(`hdr: no ${family} record has id "m1"`); + expect(control).toEqual({ ...holding, message: `hdr: no ${family} record has id "m2"` }); + } else { + expect(holding?.message).toContain("reads 'parent'"); + // The verdict does not depend on the row: the bit the body would decide is not served. + expect(control).toEqual(holding); + } servesNothing([holding, control]); expect(familyBodyReads()).toHaveLength(0); }); diff --git a/packages/objectql/src/engine-predicate-related-master-detail-header.test.ts b/packages/objectql/src/engine-predicate-related-master-detail-header.test.ts index 523fec6ba81..ff4204f1696 100644 --- a/packages/objectql/src/engine-predicate-related-master-detail-header.test.ts +++ b/packages/objectql/src/engine-predicate-related-master-detail-header.test.ts @@ -233,6 +233,11 @@ async function refusalOf(run: () => Promise): Promise { /** ADR-0137 D2: the rule has no verdict over a header that is not there. */ const UNEVALUABLE = { status: 400, code: 'VALIDATION_FAILED', fields: [{ field: '_record', code: 'rule_violation' }] }; +/** + * [#29029] A header the write itself names is judged by the reference check + * before the record's rules: the answer is the master-detail field's. + */ +const NOT_FOUND = { status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'invoice', code: 'reference_not_found' }] }; /** The rule's own verdict: it read the header's stored state and refused (the bulk path adds the row id). */ function expectRuleVerdict(answer: Answer): void { @@ -251,6 +256,8 @@ describe('[#22892] a predicate rule reading its master through the master-detail type Door = 'insert' | 'batch insert' | 'update by id' | 'repoint by id' | 'bulk update'; const DOORS: Door[] = ['insert', 'batch insert', 'update by id', 'repoint by id', 'bulk update']; + /** [#29029] The doors whose write names the header; the other two carry a stored one. */ + const namesHeader = (door: Door) => door === 'insert' || door === 'batch insert' || door === 'repoint by id'; /** One write through `door` that lands the line under `header`. */ const write = (door: Door, header: string, context: Record = ALICE) => { @@ -275,8 +282,12 @@ describe('[#22892] a predicate rule reading its master through the master-detail const before = [...storeFor(LINE).values()]; const nowhere = await refusalOf(() => write(door, NOWHERE)); expect(nowhere).toBeInstanceOf(ValidationError); - expect(answerOf(nowhere, NOWHERE)).toMatchObject(UNEVALUABLE); - expect(answerOf(nowhere, NOWHERE).message).toContain('the related record was not found'); + if (namesHeader(door)) { + expect(answerOf(nowhere, NOWHERE)).toMatchObject(NOT_FOUND); + } else { + expect(answerOf(nowhere, NOWHERE)).toMatchObject(UNEVALUABLE); + expect(answerOf(nowhere, NOWHERE).message).toContain('the related record was not found'); + } for (const header of UNREADABLE) { expect(answerOf(await refusalOf(() => write(door, header)), header)).toEqual(answerOf(nowhere, NOWHERE)); } @@ -289,9 +300,13 @@ describe('[#22892] a predicate rule reading its master through the master-detail it.each(DOORS)('%s — a refused read of the header object answers as a header that exists nowhere', async (door) => { const nowhere = answerOf(await refusalOf(() => write(door, NOWHERE, CAROL)), NOWHERE); - expect(nowhere).toMatchObject(UNEVALUABLE); + expect(nowhere).toMatchObject(namesHeader(door) ? NOT_FOUND : UNEVALUABLE); // Alice's headers exist; Carol's read door refuses the object, whatever they hold. - for (const header of ['h_mine_open', 'h_mine_locked']) { + // [#29029] The batch's first row and the repoint's stored line are + // `h_mine_open` itself, the door's own fixture: those doors take the + // locked header only. + const headers = door === 'batch insert' || door === 'repoint by id' ? ['h_mine_locked'] : ['h_mine_open', 'h_mine_locked']; + for (const header of headers) { expect(answerOf(await refusalOf(() => write(door, header, CAROL)), header)).toEqual(nowhere); } }); @@ -302,7 +317,7 @@ describe('[#22892] a predicate rule reading its master through the master-detail return (out.results?.[0]?.errors ?? []).map((e: any) => ({ field: e.field, code: e.code, message: String(e.message).split(header).join('ID') })); }; const nowhere = await preview(NOWHERE); - expect(nowhere).toEqual([expect.objectContaining({ field: '_record', code: 'rule_violation' })]); + expect(nowhere).toEqual([expect.objectContaining({ field: 'invoice', code: 'reference_not_found' })]); for (const header of UNREADABLE) expect(await preview(header)).toEqual(nowhere); // CONTROL: a readable header is judged on its stored state. expect(await preview('h_mine_locked')).toEqual([{ field: '_record', code: 'rule_violation', message: RULE_MESSAGE }]); @@ -320,7 +335,7 @@ describe('[#22892] a predicate rule reading its master through the master-detail expect(out.map((o) => o.ok)).toEqual([false, false, false, false, true]); const errors = out.map((o) => (o.ok ? undefined : o.error)); const nowhere = answerOf(errors[2], NOWHERE); - expect(nowhere).toMatchObject(UNEVALUABLE); + expect(nowhere).toMatchObject(NOT_FOUND); expect(answerOf(errors[0], 'h_other_open')).toEqual(nowhere); expect(answerOf(errors[1], 'h_other_locked')).toEqual(nowhere); expectRuleVerdict(answerOf(errors[3], 'h_mine_locked')); @@ -329,8 +344,10 @@ describe('[#22892] a predicate rule reading its master through the master-detail it('the mechanism: the caller\'s read door first, and no elevated read of a header it did not return', async () => { await refusalOf(() => engine.insert(LINE, { invoice: 'h_other_locked', quantity: 1 }, { context: ALICE } as any)); + // The rule's binding read and the reference check's [#29029], each through the read door. expect(headerReads).toEqual([ { operation: 'find', elevated: false, where: { id: 'h_other_locked' } }, + { operation: 'find', elevated: false, where: { id: 'h_other_locked' } }, ]); headerReads.length = 0; await engine.insert(LINE, { invoice: 'h_mine_open', quantity: 1 }, { context: ALICE } as any); diff --git a/packages/objectql/src/engine-reference-tenant-scope.test.ts b/packages/objectql/src/engine-reference-tenant-scope.test.ts index 5edc9d13cbf..5cef4993488 100644 --- a/packages/objectql/src/engine-reference-tenant-scope.test.ts +++ b/packages/objectql/src/engine-reference-tenant-scope.test.ts @@ -546,9 +546,10 @@ describe('[#19837] the master-detail parent binding is scoped to the caller\'s o const open = await refusalOf(() => insertLine('hy_open')); const nowhere = await refusalOf(() => insertLine('h_nowhere')); - // ADR-0137 D2: the unbound header leaves `note`'s requirement without a - // verdict, and that refusal precedes the reference guard's. - const expected = { status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'note', code: 'rule_violation' }] }; + // [#29029] The header the write names is judged by the reference check + // before the record's rules: an unreadable header answers as one that + // exists nowhere. + const expected = { status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'header', code: 'reference_not_found' }] }; expect(locked).toBeInstanceOf(ValidationError); expect(envelopeOf(locked)).toEqual(expected); expect(envelopeOf(open)).toEqual(expected); @@ -566,8 +567,8 @@ describe('[#19837] the master-detail parent binding is scoped to the caller\'s o const locked = await refusalOf(() => repoint('hy_locked')); const open = await refusalOf(() => repoint('hy_open')); - // ADR-0137 D2: see the describe's docblock — alike, and the rule's refusal. - const expected = { status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'note', code: 'rule_violation' }] }; + // [#29029] Alike, and the reference check's refusal: the repoint names the header. + const expected = { status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'header', code: 'reference_not_found' }] }; expect(envelopeOf(locked)).toEqual(expected); expect(envelopeOf(open)).toEqual(expected); expect(storeFor('pb_line').get('ln_x')?.header).toBe('hx_open'); @@ -662,10 +663,11 @@ describe('[#19837] the master-detail parent binding is scoped to the caller\'s o const global = await refusalOf(() => insertLine('gh_locked', 'pb_gline')); expect(envelopeOf(global)).toEqual({ status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'note', code: 'required' }] }); // Bound because the engine withheld the tenant for this master (ADR-0066), - // on both of the write's header reads (#22519: the caller's read door, then - // the elevated read). + // on each of the write's header reads: the binding's two (#22519: the + // caller's read door, then the elevated read) and the reference check's + // read door, which runs before the record's rules (#29029). const read = observed.filter((c) => c.object === 'pb_gheader' && c.method === 'find'); - expect(read).toHaveLength(2); + expect(read).toHaveLength(3); for (const r of read) expect(r.options?.tenantId).toBeUndefined(); }); @@ -681,20 +683,21 @@ describe('[#19837] the master-detail parent binding is scoped to the caller\'s o engine.insert('pb_line', { id: 'ln_grp', header: 'hy_locked' }, { context: GROUP_MEMBER } as any)); expect(envelopeOf(inside)).toEqual({ status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'note', code: 'required' }] }); - // Both of the write's header reads (#22519: the caller's read door, then - // the elevated read) reach the union. + // Each of the write's header reads reaches the union: the binding's two + // (#22519: the caller's read door, then the elevated read) and the + // reference check's read door (#29029). const read = observed.filter((c) => c.object === 'pb_header' && c.method === 'find'); - expect(read).toHaveLength(2); + expect(read).toHaveLength(3); for (const r of read) expect([...(r.options?.tenantIds ?? [])].sort()).toEqual([ORG_X, ORG_Y].sort()); // Lit control: a member whose set is org X alone cannot see the org-Y - // header, so it binds absent and the write answers as an absent header does - // (ADR-0137 D2: `note`'s requirement has no verdict, and is refused). + // header, so the write answers as an absent header does (#29029: the + // reference check, before the record's rules). const ONLY_X = { ...MEMBER_X, accessible_org_ids: [ORG_X] } as unknown as ExecutionContext; const outside = await refusalOf(() => engine.insert('pb_line', { id: 'ln_x_only', header: 'hy_locked' }, { context: ONLY_X } as any)); - expect(envelopeOf(outside)).toEqual({ status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'note', code: 'rule_violation' }] }); + expect(envelopeOf(outside)).toEqual({ status: 400, code: 'VALIDATION_FAILED', fields: [{ field: 'header', code: 'reference_not_found' }] }); }); it('the header read asks the caller\'s read door first, then runs ELEVATED and TENANT-SCOPED — the two halves of `{ ...context, isSystem: true }`', async () => { diff --git a/packages/objectql/src/engine-unreadable-master-detail-header.test.ts b/packages/objectql/src/engine-unreadable-master-detail-header.test.ts index 0182129b51d..c1702b784a0 100644 --- a/packages/objectql/src/engine-unreadable-master-detail-header.test.ts +++ b/packages/objectql/src/engine-unreadable-master-detail-header.test.ts @@ -353,7 +353,8 @@ describe('[#22519] an unreadable master-detail header answers what a nonexistent describe('(b) requiredWhen over parent', () => { it('insert — an unreadable header in either state answers what a nonexistent one does', async () => { const nowhere = await refusalOf(() => engine.insert(REQ_LINE, { invoice: NOWHERE, quantity: 1 }, { context: ALICE } as any)); - expect(envelopeOf(nowhere)).toEqual(UNEVALUABLE); + // [#29029] The header the write names is judged before the record's rules. + expect(envelopeOf(nowhere)).toEqual(NOT_FOUND); for (const header of UNREADABLE) { const err = await refusalOf(() => engine.insert(REQ_LINE, { invoice: header, quantity: 1 }, { context: ALICE } as any)); expect(envelopeOf(err)).toEqual(envelopeOf(nowhere)); @@ -386,7 +387,8 @@ describe('[#22519] an unreadable master-detail header answers what a nonexistent }; for (const bulk of [false, true]) { const nowhere = await repoint(NOWHERE, bulk); - expect(envelopeOf(nowhere)).toEqual(UNEVALUABLE); + // [#29029] The header the repoint names is judged before the record's rules. + expect(envelopeOf(nowhere)).toEqual(NOT_FOUND); for (const header of UNREADABLE) { const err = await repoint(header, bulk); expect(envelopeOf(err)).toEqual(envelopeOf(nowhere)); @@ -449,7 +451,8 @@ describe('[#22519] an unreadable master-detail header answers what a nonexistent it('a controlled_by_parent detail takes the same rule under an unreadable header', async () => { const nowhere = envelopeOf(await refusalOf(() => engine.insert(CBP_LINE, { invoice: NOWHERE, quantity: 1 }, { context: ALICE } as any))); - expect(nowhere).toEqual(UNEVALUABLE); + // [#29029] The header the write names is judged before the record's rules. + expect(nowhere).toEqual(NOT_FOUND); for (const header of UNREADABLE) { expect(envelopeOf(await refusalOf(() => engine.insert(CBP_LINE, { invoice: header, quantity: 1 }, { context: ALICE } as any)))).toEqual(nowhere); } From c614b03c06165cf56a82fb54ca8d41b72f1d8fb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 18:36:07 +0000 Subject: [PATCH 13/26] test(plugin-security): the rig-users testkit lives under __tests__, and the delegate-read pins type their options The system-context page's reference-check row names referencesCheckedBeforeRules, and its site register is regenerated. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- content/docs/permissions/system-context.mdx | 3 ++- .../src/{ => __tests__}/rig-users.testkit.ts | 0 .../delegable-scope-no-identity-positions.posture.test.ts | 2 +- .../src/delegated-admin-own-name-set.posture.test.ts | 8 ++++---- .../src/grant-holder-membership-refusal.test.ts | 2 +- .../plugin-security/src/grant-permission-set-name.test.ts | 2 +- .../src/org-admin-assignment-boundary.posture.test.ts | 2 +- .../plugin-security/src/position-catalog-refusal.test.ts | 2 +- .../src/sys-user-permission-set-window-order.test.ts | 2 +- 9 files changed, 12 insertions(+), 11 deletions(-) rename packages/plugins/plugin-security/src/{ => __tests__}/rig-users.testkit.ts (100%) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 5c3f0736cf3..14268fbd703 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -134,7 +134,7 @@ bugs — it is the protection or the side effect 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 | `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 | `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 | `insert`, `packages/objectql/src/readonly-strict-errors.ts#READONLY_CLASS_REASONS` | -| 23 | **Referential-integrity check skipped** — except a `runAs: 'system'` flow's record writes, which keep it | objectql | Get: writes proceed against unreachable/unresolvable targets — a seed replay, an import, a restore or a cascade may store a reference to a row that does not exist yet. Lose: an `isSystem` caller can write a **dangling reference**. A flow run's elevated context (`resolveRunDataContext`) carries an internal mark that keeps the check, so a flow's `create_record` / `update_record` naming a row that does not exist is refused `400 VALIDATION_FAILED` / `reference_not_found`, as on the user path; its authorization elevation is unchanged. A write the flow's write sets off (a hook's `ctx.api`, its `sudo()`, a cascade leg of a flow's delete) spreads the run's context and is checked as the flow's own, as on the user path. The mark is a symbol no flow definition or request can spell, and `ExecutionContextSchema.parse` drops it | `assertReferencesResolve` | +| 23 | **Referential-integrity check skipped** — except a `runAs: 'system'` flow's record writes, which keep it | objectql | Get: writes proceed against unreachable/unresolvable targets — a seed replay, an import, a restore or a cascade may store a reference to a row that does not exist yet. Lose: an `isSystem` caller can write a **dangling reference**. A flow run's elevated context (`resolveRunDataContext`) carries an internal mark that keeps the check, so a flow's `create_record` / `update_record` naming a row that does not exist is refused `400 VALIDATION_FAILED` / `reference_not_found`, as on the user path; its authorization elevation is unchanged. A write the flow's write sets off (a hook's `ctx.api`, its `sudo()`, a cascade leg of a flow's delete) spreads the run's context and is checked as the flow's own, as on the user path. The mark is a symbol no flow definition or request can spell, and `ExecutionContextSchema.parse` drops it. Where a system write runs the check (a flow run's), it runs after the record's validation rules, as before; every other write runs it before them | `assertReferencesResolve`, `referencesCheckedBeforeRules` | | 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` | @@ -539,6 +539,7 @@ and CI refuses one that differs from what the tree produces. | `packages/objectql/src/engine-master-detail-related.ts#resolveOneHopRelated` | 1 | 29b | | `packages/objectql/src/engine-referential-integrity.ts#assertReferencesResolve` | 1 | 23 | | `packages/objectql/src/engine-referential-integrity.ts#masterDetailHeadersCallerReads` | 1 | 29c | +| `packages/objectql/src/engine-referential-integrity.ts#referencesCheckedBeforeRules` | 1 | 23 | | `packages/objectql/src/engine-secret-internal-fields.ts#encryptSecretFields` | 1 | 23e | | `packages/objectql/src/engine-secret-internal-fields.ts#stripSearchCompanionFromRead` | 1 | 27 | | `packages/objectql/src/engine-validate.ts#validate` | 1 | 21b | diff --git a/packages/plugins/plugin-security/src/rig-users.testkit.ts b/packages/plugins/plugin-security/src/__tests__/rig-users.testkit.ts similarity index 100% rename from packages/plugins/plugin-security/src/rig-users.testkit.ts rename to packages/plugins/plugin-security/src/__tests__/rig-users.testkit.ts diff --git a/packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts b/packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts index af175ef13d3..18570649bac 100644 --- a/packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts +++ b/packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts @@ -37,7 +37,7 @@ import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.j import { RESERVED_IDENTITY_NAMES } from './objects/reserved-identity-names.js'; import { defaultPermissionSets } from './objects/index.js'; import { BUILTIN_IDENTITY_ORG_EXTERNAL } from '@objectstack/spec'; -import { provisionRigUsers } from './rig-users.testkit.js'; +import { provisionRigUsers } from './__tests__/rig-users.testkit.js'; const SYS = { isSystem: true } as const; const ORG_A = 'org_a'; diff --git a/packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts b/packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts index 08fe246298b..f074c4df683 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts @@ -258,9 +258,9 @@ describe('[ADR-0090 D12, amended 2026-10-17; #24757] a delegate is judged on exa }); /** [#29029] Every unit id `context` reads, sorted, or the refusal's status and code. */ -async function unitsRead(h: Awaited>, context: any): Promise { +async function unitsRead(h: Awaited>, context: ReturnType): Promise { try { - const rows = await h.engine.find('sys_business_unit', { context } as any); + const rows = await h.engine.find('sys_business_unit', { context }); return (Array.isArray(rows) ? rows : []).map((r: any) => String(r.id)).sort(); } catch (e) { const r = resolveThrownHttpError(e); @@ -294,9 +294,9 @@ describe('[ADR-0090 D12, ruling of record 6663849489; #29029] a delegate reads t it('the delegate reads exactly the units its scope covers: the scope root and its subtree, not the unit above it', async () => { const h = await boot(posture); expect(await unitsRead(h, NARROW)).toEqual(['bu_east', 'bu_east_field']); - const above = await h.engine.findOne('sys_business_unit', { where: { id: 'bu_hq' }, context: NARROW } as any); + const above = await h.engine.findOne('sys_business_unit', { where: { id: 'bu_hq' }, context: NARROW }); expect(above ?? null).toBeNull(); - expect(await h.engine.count('sys_business_unit', { context: NARROW } as any)).toBe(2); + expect(await h.engine.count('sys_business_unit', { context: NARROW })).toBe(2); }); it('the delegate assigns a position in a unit its scope covers, and the assignment is stored', async () => { diff --git a/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts b/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts index ad1bf69bcbd..8f8914c446c 100644 --- a/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts +++ b/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts @@ -40,7 +40,7 @@ import { registerGrantHolderMembershipRefusal, } from './grant-holder-membership-refusal.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; -import { provisionRigUsers } from './rig-users.testkit.js'; +import { provisionRigUsers } from './__tests__/rig-users.testkit.js'; // --------------------------------------------------------------------------- // Fixture diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts index 889f777e893..8c0dc76b223 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts @@ -44,7 +44,7 @@ import { registerGrantPermissionSetNameHooks, } from './grant-permission-set-name.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; -import { provisionRigUsers } from './rig-users.testkit.js'; +import { provisionRigUsers } from './__tests__/rig-users.testkit.js'; // --------------------------------------------------------------------------- // Fixture diff --git a/packages/plugins/plugin-security/src/org-admin-assignment-boundary.posture.test.ts b/packages/plugins/plugin-security/src/org-admin-assignment-boundary.posture.test.ts index acb1ebea007..2f3ca240d9c 100644 --- a/packages/plugins/plugin-security/src/org-admin-assignment-boundary.posture.test.ts +++ b/packages/plugins/plugin-security/src/org-admin-assignment-boundary.posture.test.ts @@ -44,7 +44,7 @@ import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { defaultPermissionSets } from './objects/index.js'; import { maskFieldValue } from './field-masker.js'; -import { provisionRigUsers } from './rig-users.testkit.js'; +import { provisionRigUsers } from './__tests__/rig-users.testkit.js'; const SYS = { isSystem: true } as const; const ORG_A = 'org_a'; diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts index 81b7bfbc636..d806868de68 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -37,7 +37,7 @@ import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { namesWithoutCatalogRow, positionNotInCatalogMessage } from './position-catalog-refusal.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; -import { provisionRigUsers } from './rig-users.testkit.js'; +import { provisionRigUsers } from './__tests__/rig-users.testkit.js'; // --------------------------------------------------------------------------- // Fixture diff --git a/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts b/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts index 1cb93d95612..dab4848adfd 100644 --- a/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts +++ b/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts @@ -44,7 +44,7 @@ import { SysPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js' import { SysPositionPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; -import { provisionRigUsers } from './rig-users.testkit.js'; +import { provisionRigUsers } from './__tests__/rig-users.testkit.js'; const MEMBER_DEFAULT = { name: 'member_default', label: 'Member', objects: {} } as unknown as PermissionSet; const QA_ADMIN = { From 75a7c44d96a6c28c96258279cd516dcd7b1ea0eb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 20:44:21 +0000 Subject: [PATCH 14/26] test(plugin-security): the rule's peer carries an expired ban, so the rule is what answers it A non-system write now meets the reference check, and with it the ban check, before the record's validation rules; the control's peer keeps `banned` true for the rule to read, with a ban no longer in force. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- .../src/predicate-related-read-tenant-scope.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-security/src/predicate-related-read-tenant-scope.test.ts b/packages/plugins/plugin-security/src/predicate-related-read-tenant-scope.test.ts index 6b3dfb3d161..9ca4d371860 100644 --- a/packages/plugins/plugin-security/src/predicate-related-read-tenant-scope.test.ts +++ b/packages/plugins/plugin-security/src/predicate-related-read-tenant-scope.test.ts @@ -168,9 +168,11 @@ async function boot(kind: 'secret' | 'public', posture?: 'single' | 'group' | 'i { id: 'line_y', kind, organization_id: 'org_y' }, ]); // The caller u_x and its peer u_x2 (org X); u_y belongs to org Y only. + // [#30205] u_x2's ban has expired: `banned` still reads true, which is what + // the rule reads, and the write's ban check (a ban in force) leaves it be. await table('sys_user').insert([ { id: 'u_x', name: 'X', email: 'x@x.test', banned: false }, - { id: 'u_x2', name: 'X2', email: 'x2@x.test', banned: true }, + { id: 'u_x2', name: 'X2', email: 'x2@x.test', banned: true, ban_expires: '2020-01-01T00:00:00.000Z' }, { id: 'u_y', name: 'Y', email: 'y@y.test', banned: kind === 'secret' }, ]); From 0c2b9851b6ee0eb79ca4b66f9d90f5f9d51a019d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 22:11:02 +0000 Subject: [PATCH 15/26] docs(qa): the checklist item and the pins name the rule alone Revision 7 of records-forms.predicate-relationship-traversal, the dogfood case and the objectql pins say what the write answers: a reference the caller cannot read is reference_not_found, before the record's validation rules. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- .../areas/records-forms.json | 14 ++--- ...engine-unreadable-reference-target.test.ts | 52 +++++++++---------- ...ate-relationship-traversal.dogfood.test.ts | 7 ++- 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/docs/qa/platform-checklist/areas/records-forms.json b/docs/qa/platform-checklist/areas/records-forms.json index f8c5cf5290e..f9c5fa5d6ee 100644 --- a/docs/qa/platform-checklist/areas/records-forms.json +++ b/docs/qa/platform-checklist/areas/records-forms.json @@ -5561,14 +5561,14 @@ "at least one churned account and one non-churned account to invoice against" ], "knownGaps": [ - "No stock persona has showcase_invoice CRUD WITHOUT showcase_account read, and none can be made from the stock showcase objects: showcase_member_default, the everyone baseline every member holds, grants showcase_account allowRead (examples/app-showcase/src/security/permission-sets.ts), and permission sets only ever ADD grants, so no extra set takes it away (QA run #21720, parked on #22017 row 14). An earlier revision of this item said the opposite, reading packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts. That test's persona is its own injected fallback set, not a stock persona, and it never writes against a churned account. So acceptance[3] is scored on a scratch pair: the dogfood pin in `automated` builds one at boot, or it is scored live on the scratch substitute #21720 measured, which is runtime-only on a stock boot (QA run #27438 built and drove it at c4c515ad): POST /api/v1/packages with a declared namespace; PUT /api/v1/meta/object for a parent object, and for a child object carrying a one-hop script validation over its lookup to the parent; PUT /api/v1/meta/permission for a set granting the member the CHILD only, bound with POST /api/v1/data/sys_user_permission_set; then, as that member, write against a tripping and a non-tripping parent. The control is the member's GET of a parent row, which must answer 403: without it the leg proves nothing about an unreadable parent" + "No stock persona has showcase_invoice CRUD WITHOUT showcase_account read, and none can be made from the stock showcase objects: showcase_member_default, the everyone baseline every member holds, grants showcase_account allowRead (examples/app-showcase/src/security/permission-sets.ts), and permission sets only ever ADD grants, so no extra set takes it away (QA run #21720, parked on #22017 row 14). An earlier revision of this item said the opposite, reading packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts. That test's persona is its own injected fallback set, not a stock persona, and it never writes against a churned account. So acceptance[3] is scored on a scratch pair: the dogfood pin in `automated` builds one at boot, or it is scored live on the scratch substitute #21720 measured, which is runtime-only on a stock boot (QA run #27438 built and drove it at c4c515ad): POST /api/v1/packages with a declared namespace; PUT /api/v1/meta/object for a parent object, and for a child object carrying a one-hop script validation over its lookup to the parent; PUT /api/v1/meta/permission for a set granting the member the CHILD only, bound with POST /api/v1/data/sys_user_permission_set; then, as that member, write naming a parent row. The control is the member's GET of a parent row, which must answer 403: without it the leg proves nothing about an unreadable parent" ] }, "steps": [ "POST /api/v1/data/showcase_invoice with `account` pointing at a NON-churned account", "POST /api/v1/data/showcase_invoice with `account` pointing at a CHURNED account", "PATCH an existing invoice to REPOINT it at a churned account (the update path reads the FK off the merged record, not the patch alone)", - "as a member with NO read on the parent (none exists on stock — fixtures.knownGaps), on the scratch pair: write against a non-tripping parent, then a tripping one" + "as a member with NO read on the parent (none exists on stock — fixtures.knownGaps), on the scratch pair: write naming a parent row" ], "acceptance": [ { @@ -5590,10 +5590,10 @@ "evidence": "the patch response" }, { - "clause": "a member who cannot read the parent cannot name it: the write answers reference_not_found on the lookup, before the record's rules, whether or not the parent trips one", + "clause": "a member who cannot read the parent cannot name it: the write answers reference_not_found on the lookup, before the record's validation rules", "oracle": "api", - "verify": "on stock showcase no such persona exists (fixtures.knownGaps), so score this on the scratch pair. For the member who cannot read the parent, the write naming the non-tripping parent and the write naming the tripping parent are each 400 VALIDATION_FAILED with exactly one fields[] entry, { field: , code: 'reference_not_found' } — the answer for a parent that exists nowhere — and nothing is stored. Neither answer carries the rule's authored message or the unevaluable-rule fault, and the member's own GET of the parent answers 403 (the control)", - "evidence": "both responses for the restricted persona" + "verify": "on stock showcase no such persona exists (fixtures.knownGaps), so score this on the scratch pair. For the member who cannot read the parent, the write naming it is 400 VALIDATION_FAILED with exactly one fields[] entry, { field: , code: 'reference_not_found' } — the answer for a parent that exists nowhere — and nothing is stored. The answer carries neither the rule's authored message nor the unevaluable-rule fault, and the member's own GET of the parent answers 403 (the control)", + "evidence": "the restricted persona's responses" } ], "negative": [ @@ -5606,7 +5606,7 @@ ], "automated": { "kind": "dogfood", - "ref": "packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts, at the REST data door of a booted stack: on the showcase as shipped, acceptance[0] (an invoice on an active account is 2xx and reads back, no unevaluable fault), acceptance[1] and negative[0] (an invoice on a churned account is 400 VALIDATION_FAILED with one _record rule_violation entry carrying the authored message, and nothing is stored), acceptance[2] (a PATCH carrying only the repointed account is refused and the stored account is unchanged) and negative[1] (no readable-parent case answers the unevaluable fault); on a scratch pair, acceptance[3] and negative[2] for a member no stock showcase persona is (fixtures.knownGaps): a member with create/edit on the child and no grant on the parent, whose parent read is the control (403 PERMISSION_DENIED), is answered 400 VALIDATION_FAILED with the lookup's reference_not_found against a non-tripping and a tripping parent alike, never the authored message, and nothing is stored. packages/objectql/src/engine-predicate-relationship.test.ts stays engine-layer evidence of the mechanism ('reads the related row under SYSTEM authority' and its two siblings)" + "ref": "packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts, at the REST data door of a booted stack: on the showcase as shipped, acceptance[0] (an invoice on an active account is 2xx and reads back, no unevaluable fault), acceptance[1] and negative[0] (an invoice on a churned account is 400 VALIDATION_FAILED with one _record rule_violation entry carrying the authored message, and nothing is stored), acceptance[2] (a PATCH carrying only the repointed account is refused and the stored account is unchanged) and negative[1] (no readable-parent case answers the unevaluable fault); on a scratch pair, acceptance[3] and negative[2] for a member no stock showcase persona is (fixtures.knownGaps): a member with create/edit on the child and no grant on the parent, whose parent read is the control (403 PERMISSION_DENIED), is answered 400 VALIDATION_FAILED with the lookup's reference_not_found, before the record's validation rules, and nothing is stored. packages/objectql/src/engine-predicate-relationship.test.ts stays engine-layer evidence of the mechanism ('reads the related row under SYSTEM authority' and its two siblings)" }, "source": [ "examples/app-showcase/src/data/objects/invoice.object.ts (no_invoice_for_churned_account)", @@ -5653,7 +5653,7 @@ { "revision": 7, "date": "2026-11-05", - "change": "the persona that wrote a child without reading its lookup parent is retired: a non-system write may name, in a lookup or user field, only a record the caller's own read door returns, and that reference check runs before the record's validation rules (ruling records 6573561091 and 6663849489). acceptance[3], its verify, negative[2], step 4, the second persona and automated.ref now state that rule: the member who cannot read the parent is answered reference_not_found against a tripping and a non-tripping parent alike. acceptance[0]–[2] are unchanged", + "change": "the persona that wrote a child without reading its lookup parent is retired: a non-system write may name, in a lookup or user field, only a record the caller's own read door returns, and that reference check runs before the record's validation rules (ruling records 6573561091 and 6663849489). acceptance[3], its verify, negative[2], step 4, the second persona and automated.ref now state that rule: the member who cannot read the parent is answered reference_not_found, before the record's validation rules. acceptance[0]–[2] are unchanged", "ref": "#29029" } ] diff --git a/packages/objectql/src/engine-unreadable-reference-target.test.ts b/packages/objectql/src/engine-unreadable-reference-target.test.ts index 357b34a4ff4..0cef7139d55 100644 --- a/packages/objectql/src/engine-unreadable-reference-target.test.ts +++ b/packages/objectql/src/engine-unreadable-reference-target.test.ts @@ -62,11 +62,11 @@ const ITEM_OBJECT = { /** * [#29029] A task whose validation rules read each reference one hop: each - * trips on a target named `tripping`. The reference check runs BEFORE them. + * refuses a target named `flagged`. The reference check runs BEFORE them. */ const RULED = 'urt_ruled_task'; -const TRIPPED = 'URT29029: the named target trips the rule'; -const TITLE_TRIPPED = 'URT29029: the title trips the rule'; +const RULE_MESSAGE = 'URT29029: the rule refuses the named target'; +const TITLE_RULE_MESSAGE = 'URT29029: the rule refuses the title'; const RULED_OBJECT = { name: RULED, label: 'Ruled task', @@ -80,9 +80,9 @@ const RULED_OBJECT = { // Checked only while the reference is set, as an author guards a traversal. name: `urt_${field}_when_set`, type: 'conditional', severity: 'error', message: 'unused', when: `record.${field} != null`, - then: { name: `urt_${field}_trips`, type: 'script', severity: 'error', message: TRIPPED, condition: `record.${field}.name == 'tripping'` }, + then: { name: `urt_${field}_rule`, type: 'script', severity: 'error', message: RULE_MESSAGE, condition: `record.${field}.name == 'flagged'` }, })).concat([ - { name: 'urt_title_trips', type: 'script', severity: 'error', message: TITLE_TRIPPED, condition: "record.title == 'trip'" } as any, + { name: 'urt_title_rule', type: 'script', severity: 'error', message: TITLE_RULE_MESSAGE, condition: "record.title == 'flagged'" } as any, ]), }; @@ -187,9 +187,9 @@ function makeEngine() { rows.set('t_mine', { id: 't_mine', name: 'mine', owner_name: 'alice' }); rows.set('t_mine_2', { id: 't_mine_2', name: 'mine too', owner_name: 'alice' }); rows.set('t_other', { id: 't_other', name: 'other', owner_name: 'bob' }); - // [#29029] A readable and an unreadable target that trip the ruled task's rules. - rows.set('t_trip_mine', { id: 't_trip_mine', name: 'tripping', owner_name: 'alice' }); - rows.set('t_trip_other', { id: 't_trip_other', name: 'tripping', owner_name: 'bob' }); + // [#29029] A readable and an unreadable target the ruled task's rules refuse. + rows.set('t_flagged_mine', { id: 't_flagged_mine', name: 'flagged', owner_name: 'alice' }); + rows.set('t_flagged_other', { id: 't_flagged_other', name: 'flagged', owner_name: 'bob' }); } return { engine, targetReads, ...store }; } @@ -360,48 +360,48 @@ describe('[#29029] an unreadable reference answers reference_not_found before th { field: 'assignee', kind: 'user' }, { field: 'header', kind: 'master_detail' }, ] as const)('a $kind field', ({ field }) => { - it.each(DOORS)('%s — a target the caller cannot read that a rule would trip on answers reference_not_found, not the rule', async (door) => { - const err = await refusalOf(() => write(door, { [field]: 't_trip_other' })); + it.each(DOORS)('%s — a target the caller cannot read answers reference_not_found, before the record\'s validation rules', async (door) => { + const err = await refusalOf(() => write(door, { [field]: 't_flagged_other' })); expect(envelopeOf(err)).toEqual(notFound(field)); - expect(String((err as Error).message)).not.toContain(TRIPPED); + expect(String((err as Error).message)).not.toContain(RULE_MESSAGE); expect(storedRuled()).toEqual([{ id: 'r1', title: 'stored' }]); }); - it.each(DOORS)('%s — control: a readable target the rule trips on answers the rule', async (door) => { - const err = await refusalOf(() => write(door, { [field]: 't_trip_mine' })); + it.each(DOORS)('%s — control: a readable target reaches the record\'s validation rules', async (door) => { + const err = await refusalOf(() => write(door, { [field]: 't_flagged_mine' })); expect(ruleRefusal(err).code).toBe('VALIDATION_FAILED'); - expect(ruleRefusal(err).messages.some((m: string) => m.includes(TRIPPED))).toBe(true); + expect(ruleRefusal(err).messages.some((m: string) => m.includes(RULE_MESSAGE))).toBe(true); expect(storedRuled()).toEqual([{ id: 'r1', title: 'stored' }]); }); it.each(DOORS)('%s — control: a system write naming that target answers the rule, as before', async (door) => { - const err = await refusalOf(() => write(door, { [field]: 't_trip_other' }, SYSTEM)); - expect(ruleRefusal(err).messages.some((m: string) => m.includes(TRIPPED))).toBe(true); + const err = await refusalOf(() => write(door, { [field]: 't_flagged_other' }, SYSTEM)); + expect(ruleRefusal(err).messages.some((m: string) => m.includes(RULE_MESSAGE))).toBe(true); expect(envelopeOf(err).fields.map((f) => f.code)).not.toContain('reference_not_found'); }); }); - it.each(DOORS)('%s — a reference to a record that exists nowhere answers reference_not_found before a rule the record trips', async (door) => { - const err = await refusalOf(() => write(door, { title: 'trip', duty: 't_missing' })); + it.each(DOORS)('%s — a reference to a record that exists nowhere answers reference_not_found, before the record\'s validation rules', async (door) => { + const err = await refusalOf(() => write(door, { title: 'flagged', duty: 't_missing' })); expect(envelopeOf(err)).toEqual(notFound('duty')); - expect(String((err as Error).message)).not.toContain(TITLE_TRIPPED); + expect(String((err as Error).message)).not.toContain(TITLE_RULE_MESSAGE); }); - it.each(DOORS)('%s — control: a flow run\'s system write keeps the rules first — the rule its record trips answers, not the reference check', async (door) => { + it.each(DOORS)('%s — control: a flow run\'s system write keeps the validation rules first', async (door) => { const flowRun = markFlowWriteReferenceCheck({ ...SYSTEM }); - const err = await refusalOf(() => write(door, { title: 'trip', duty: 't_missing' }, flowRun)); - expect(ruleRefusal(err).messages.some((m: string) => m.includes(TITLE_TRIPPED))).toBe(true); + const err = await refusalOf(() => write(door, { title: 'flagged', duty: 't_missing' }, flowRun)); + expect(ruleRefusal(err).messages.some((m: string) => m.includes(TITLE_RULE_MESSAGE))).toBe(true); expect(envelopeOf(err).fields.map((f) => f.code)).not.toContain('reference_not_found'); }); it('the validate preview answers as the insert does: reference_not_found for the unreadable target, the rule for the readable one', async () => { - const unreadable = (await engine.validate(RULED, { title: 'preview', duty: 't_trip_other' }, { mode: 'insert', context: ALICE } as any)).results?.[0]; + const unreadable = (await engine.validate(RULED, { title: 'preview', duty: 't_flagged_other' }, { mode: 'insert', context: ALICE } as any)).results?.[0]; expect(unreadable?.valid).toBe(false); expect((unreadable?.errors ?? []).map((e: any) => ({ field: e.field, code: e.code }))).toEqual([{ field: 'duty', code: 'reference_not_found' }]); - expect(JSON.stringify(unreadable?.errors ?? [])).not.toContain(TRIPPED); - const readable = (await engine.validate(RULED, { title: 'preview', duty: 't_trip_mine' }, { mode: 'insert', context: ALICE } as any)).results?.[0]; + expect(JSON.stringify(unreadable?.errors ?? [])).not.toContain(RULE_MESSAGE); + const readable = (await engine.validate(RULED, { title: 'preview', duty: 't_flagged_mine' }, { mode: 'insert', context: ALICE } as any)).results?.[0]; expect(readable?.valid).toBe(false); - expect((readable?.errors ?? []).some((e: any) => String(e.message).includes(TRIPPED))).toBe(true); + expect((readable?.errors ?? []).some((e: any) => String(e.message).includes(RULE_MESSAGE))).toBe(true); }); }); diff --git a/packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts b/packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts index d290a9755c5..0688b698c08 100644 --- a/packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts +++ b/packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts @@ -19,8 +19,7 @@ // (the item's fixtures.knownGaps). Such a member may not name the parent at // all (#29029, revision 7): a write naming a parent the caller's own read // door does not return answers `reference_not_found` on the lookup, before -// the record's rules, whether or not the parent trips one (acceptance[3], -// negative[2]). A read of the parent as that member is the control that the +// the record's validation rules (acceptance[3], negative[2]). A read of the parent as that member is the control that the // member really cannot read it. // // The rule's refusal is scored on the envelope — 400 VALIDATION_FAILED, the @@ -206,12 +205,12 @@ describe('records-forms.predicate-relationship-traversal — a member who cannot expect(read.json.code).toBe('PERMISSION_DENIED'); }); - it('acceptance[3]: a write naming a non-tripping parent the member cannot read answers reference_not_found, and stores nothing', async () => { + it('acceptance[3]: a write naming a parent the member cannot read answers reference_not_found, and stores nothing', async () => { expectReferenceRefusal(await call('POST', `/data/${CHILD}`, { name: 'prt-open', parent: openId }), 'parent'); expect(await stack.rows(CHILD, { name: 'prt-open' })).toHaveLength(0); }); - it('acceptance[3], negative[2]: a write naming a tripping parent the member cannot read answers the same, never the rule\'s message', async () => { + it('acceptance[3], negative[2]: the reference check answers before the record\'s validation rules, never a rule\'s message', async () => { const answer = await call('POST', `/data/${CHILD}`, { name: 'prt-blocked', parent: blockedId }); expectReferenceRefusal(answer, 'parent'); expect(JSON.stringify(answer.json)).not.toContain(BLOCKED_MESSAGE); From 3ec1f3e3e85a3aadc67999eae76143e240946785 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 22:33:15 +0000 Subject: [PATCH 16/26] test(plugin-security): the agent-ceiling grant suite names users its writer reads A grant's holder is a `user` value, so a non-system write naming it must name a user its caller can read; the suite provisions its three users, as the sibling grant suites do, so the set the row names is what it measures. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- .../src/agent-ceiling-grant-refusal.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/plugins/plugin-security/src/agent-ceiling-grant-refusal.test.ts b/packages/plugins/plugin-security/src/agent-ceiling-grant-refusal.test.ts index 2394b3b7b2e..be4d40620d2 100644 --- a/packages/plugins/plugin-security/src/agent-ceiling-grant-refusal.test.ts +++ b/packages/plugins/plugin-security/src/agent-ceiling-grant-refusal.test.ts @@ -30,6 +30,7 @@ import { SysUser } from '@objectstack/platform-objects/identity'; import { SecurityPlugin } from './security-plugin.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { provisionRigUsers } from './__tests__/rig-users.testkit.js'; import { AGENT_CEILING_GRANT_HOOK_PACKAGE, isAgentCeilingPermissionSet, @@ -91,11 +92,12 @@ async function boot() { objects: [SysUserPermissionSet], } as any); await engine.syncSchemas(); - // `sys_user` registered and unprovisioned, as in the sibling grant suites: the - // engine's `user_id` existence probe stands down, and no `sys_member` object - // is registered, so the holder-membership refusal stands down too. What is - // measured here is the set the row names, alone. + // `sys_user` registered, and no `sys_member` object, so the holder-membership + // refusal stands down. [#29029] Each grant's holder is a user the writer + // reads, as in the sibling grant suites. What is measured here is the set + // the row names, alone. if (!engine.registry.getObject(SysUser.name)) engine.registry.registerObject(SysUser as never, 'qa.agent-ceiling'); + await provisionRigUsers(engine, ['u_person', 'u_person_2', 'u_platform']); engines.push(engine); const services: Record = { From 4ba91079fb120a94b2557e75eff3e84bfabb9659 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 22:52:12 +0000 Subject: [PATCH 17/26] fix(verify): the parent-write-reach personas read the records a create probe names A write may name, in a lookup or user field, only a record the writer can read (#29029). The verifier's permission sets now read every record of each object a probed detail's required reference names outside its master chain, and the fixture's manager position reads the people a contract's signer names. Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT Co-authored-by: Claude --- .changeset/29029-lookup-user-caller-read.md | 3 +++ packages/verify/src/parent-write-reach.test.ts | 4 +++- packages/verify/src/parent-write-reach.ts | 18 ++++++++++++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/.changeset/29029-lookup-user-caller-read.md b/.changeset/29029-lookup-user-caller-read.md index e8434a70740..659283e2907 100644 --- a/.changeset/29029-lookup-user-caller-read.md +++ b/.changeset/29029-lookup-user-caller-read.md @@ -1,6 +1,7 @@ --- '@objectstack/objectql': patch '@objectstack/plugin-security': patch +'@objectstack/verify': patch --- fix(objectql)!: a non-system write may name, in a `lookup` or `user` field, only a record the caller can read (#29029) @@ -15,3 +16,5 @@ Metadata-protocol: none After the upgrade, an insert or update that is not a system write is refused with `400 VALIDATION_FAILED` and the field's localized `reference_not_found` when a `lookup` or `user` value it supplies names a record the caller cannot read, before the record's validation rules run. A delegated admin whose admin scope covers a business-unit subtree can read the business units in that subtree, and no business unit outside it (`@objectstack/plugin-security`). + +The parent-write-reach verifier's personas read every record of each object a probed detail's required reference names outside its master chain, so the create probe names records its writer reads (`@objectstack/verify`). diff --git a/packages/verify/src/parent-write-reach.test.ts b/packages/verify/src/parent-write-reach.test.ts index dbd50c76ff9..f328f1410e5 100644 --- a/packages/verify/src/parent-write-reach.test.ts +++ b/packages/verify/src/parent-write-reach.test.ts @@ -48,6 +48,8 @@ const manager = { pwr_account: { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true }, // Edit and create a contract, never delete one. pwr_contract: { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: false }, + // Every person: a contract's signer names a record its writer reads (#29029). + pwr_person: { allowRead: true, viewAllRecords: true }, pwr_quote: { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }, pwr_quote_line: { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }, }, @@ -75,7 +77,7 @@ const source = { sharingModel: 'controlled_by_parent', fields: { account: { type: 'master_detail', required: true, label: 'Account', reference: 'pwr_account' }, - // A second required reference, to an object the personas hold no grant on. + // A second required reference, outside the master chain, to an object the personas only read. signer: { type: 'lookup', label: 'Signer', reference: 'pwr_person', required: true }, title: { type: 'text', label: 'Title' }, }, diff --git a/packages/verify/src/parent-write-reach.ts b/packages/verify/src/parent-write-reach.ts index 8fbf606b706..cf79cf2d46b 100644 --- a/packages/verify/src/parent-write-reach.ts +++ b/packages/verify/src/parent-write-reach.ts @@ -366,20 +366,34 @@ const OWNER_EMAIL = 'verify-parent-write-owner@objectstack.test'; * details read / create / edit: an edit and a create are expected allowed, * and a delete refused by the detail's own permission (the master's * all-records grant widens which rows, never which verbs). + * + * Every set also reads every record of each object a probed detail's + * required reference names outside its master chain: a write may name, in a + * `lookup` or `user` field, only a record the writer can read (#29029), and + * the create probe fills those references with records another principal + * owns. */ function parentWriteReachPermissionSets(config: any): PermissionSet[] { const details = new Set(); const roots = new Set(); - for (const c of deriveParentWriteReachCases(config)) { - if (c.excluded || c.notDerivable) continue; + const cases = deriveParentWriteReachCases(config).filter((c) => !c.excluded && !c.notDerivable); + for (const c of cases) { details.add(c.object); c.hops.forEach((h, i) => (i === c.hops.length - 1 ? roots.add(h.master) : details.add(h.master))); } + const crud = new Map(deriveCrudCases(config).map((c) => [c.object, c])); + const referenced = new Set(); + for (const c of cases) { + for (const ref of crud.get(c.object)?.relationalRefs ?? []) { + if (ref.required && ref.field !== c.hops[0]!.field && !details.has(ref.target) && !roots.has(ref.target)) referenced.add(ref.target); + } + } const set = (name: string, label: string, detail: Record, root: Record) => ({ name, label, objects: { + ...Object.fromEntries([...referenced].map((o) => [o, { allowRead: true, viewAllRecords: true }])), ...Object.fromEntries([...details].map((o) => [o, { ...detail }])), ...Object.fromEntries([...roots].map((o) => [o, { ...root }])), }, From 0ae1bab1914ffe33511c96306568c8af5b778511 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 15:12:13 +0000 Subject: [PATCH 18/26] fix(objectql): the update's reference forgiveness compares only stored values the caller may read A refused reference value an update carries back is forgiven only when every row the write lands on stores it as the caller's own read door serves that row. A column the caller cannot read holds no stored value for it, so it forgives nothing and the value is judged like any other. A caller who reads the column keeps the forgiveness; a system write compares the rows as stored, as before. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../src/engine-referential-integrity.ts | 82 ++++++++++++++++++- packages/objectql/src/engine.ts | 2 + 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/packages/objectql/src/engine-referential-integrity.ts b/packages/objectql/src/engine-referential-integrity.ts index 3f00a21b1f6..6fc9b11c36b 100644 --- a/packages/objectql/src/engine-referential-integrity.ts +++ b/packages/objectql/src/engine-referential-integrity.ts @@ -54,6 +54,7 @@ import { referenceTargetOf, type EngineQueryOptions } from '@objectstack/spec/da import type { ExecutionContext } from '@objectstack/spec/kernel'; import { isUserBanInForce, keepsFlowWriteReferenceCheck, USER_BAN_FIELDS } from '@objectstack/core'; import { ValidationError, buildFieldError } from './validation/record-validator.js'; +import { servedAsStored } from './engine-top-level.js'; import type { ObjectQL } from './engine.js'; /** @@ -166,6 +167,11 @@ export function referencesCheckedBeforeRules(context: unknown): boolean { * value in place, `DELETE_SEMANTICS_REFERENCE_TYPES`) is not the editor's * to answer for. A value new to any of the rows is refused as before, and * an insert has no stored rows, so every value it supplies is judged. + * [#29029] The stored rows are compared as the caller may read them + * ({@link storedReferencesCallerReads}): a column the caller cannot read + * holds no stored value for it, so it forgives nothing, and the answer is + * the same whatever the column holds. A system write compares them as + * stored, as before. * * ## Why the existence probe bypasses RLS — and why it keeps the tenant wall * @@ -223,7 +229,7 @@ export function referencesCheckedBeforeRules(context: unknown): boolean { * (see {@link masterDetailHeadersCallerReads}). */ export async function assertReferencesResolve( - this: Pick, + this: Pick, schema: any, data: Record | null | undefined, supplied: Record | null | undefined, @@ -328,7 +334,14 @@ export async function assertReferencesResolve( // [#29062] A refused value every row this write lands on already stores // is not a reference the write ADDS, so it is not this caller's to answer // for. The stored rows are read only now, once a probe has refused. - const stored = storedRows ? (await storedRows() ?? []).filter(isRow) : []; + // [#29029] …and only as this caller may read them + // ({@link storedReferencesCallerReads}): the update's reference forgiveness + // compares only stored values the caller may read, so a column it cannot + // read forgives nothing, and its value is refused like any other. + const storedAsStored = storedRows ? (await storedRows() ?? []).filter(isRow) : []; + const stored = storedAsStored.length === 0 + ? storedAsStored + : await storedReferencesCallerReads.call(this, schema?.name, storedAsStored, [...new Set(refused.map((r) => r.name))], context); const failures = refused .filter((r) => stored.length === 0 || !stored.every((row) => storedReferenceIds(row[r.name]).has(String(r.value)))) .map((r) => buildFieldError( @@ -378,6 +391,71 @@ function isRow(row: unknown): row is Record { return row !== null && typeof row === 'object'; } +/** + * [#29029] The stored rows an update's reference forgiveness compares against, + * as THIS caller may read them: for each of `rows`, only the `fields` the + * caller's own read door serves for that row exactly as stored + * ({@link servedAsStored}). One `find` for the batch, under the caller's own + * context, so every enforcement layer in the middleware chain (RBAC, RLS, + * sharing, field-level security) answers it, as it answers + * {@link masterDetailHeadersCallerReads}. + * + * A column the caller cannot read — hidden, masked or rewritten for it — is + * absent from the row returned here, so it holds no stored id and forgives + * nothing: the value the write names is judged like any other, and the + * answer does not depend on what the column holds. A row the read door does + * not return, or a read that fails, keeps no column either; ⛔ never the row + * as stored. A caller who can read the column keeps the forgiveness: an + * unchanged stored reference is still accepted. + * + * A SYSTEM caller (a flow run's elevated write, the one system write the + * reference check judges) is answered with `rows` unchanged, as before. + */ +export async function storedReferencesCallerReads( + this: Pick, + object: unknown, + rows: ReadonlyArray>, + fields: ReadonlyArray, + context: ExecutionContext | undefined, +): Promise>> { + if (context?.isSystem) return rows; + const ids = new Map(); + for (const row of rows) { + const id = row.id; + if (typeof id === 'string' || typeof id === 'number') ids.set(String(id), id); + } + const served = new Map>(); + if (typeof object === 'string' && object && ids.size > 0) { + const values = [...ids.values()]; + try { + const found = await this.find(object, { + where: { id: values.length === 1 ? values[0] : { $in: values } }, + fields: ['id', ...fields], + context, + } as EngineQueryOptions) as Array>; + for (const row of Array.isArray(found) ? found : []) { + if (isRow(row) && row.id != null && ids.has(String(row.id))) served.set(String(row.id), row); + } + } catch (err) { + this.logger?.warn?.('the caller\'s read of the stored record failed — its stored references forgive nothing', { + object, error: err, + }); + } + } + return rows.map((row) => { + const view = row.id != null ? served.get(String(row.id)) : undefined; + const kept: Record = {}; + if (!view) return kept; + for (const field of fields) { + if (Object.prototype.hasOwnProperty.call(view, field) && Object.prototype.hasOwnProperty.call(row, field) + && servedAsStored(view[field], row[field])) { + kept[field] = row[field]; + } + } + return kept; + }); +} + /** The ids a stored reference value holds: each member of a multi-valued one, or the one scalar. */ function storedReferenceIds(value: unknown): ReadonlySet { const members = Array.isArray(value) ? value : [value]; diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 86688866e6c..b6926bf2429 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -2783,6 +2783,8 @@ export class ObjectQL implements IObjectQLEngine { const readLogger = () => this.logger; const host: ThisParameterType = { get logger() { return readLogger(); }, + // [#29029] The stored rows a refused value is forgiven against are read through the caller's own door. + find: (...a) => this.find(...a), callerReadableReferenceExists: (...a) => this.callerReadableReferenceExists(...a), callerReadableReferenceRow: (...a) => this.callerReadableReferenceRow(...a), referenceExists: (...a) => this.referenceExists(...a), From 1c9158f6cce49336c1349968bbe4253c04d77b68 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 15:28:43 +0000 Subject: [PATCH 19/26] test(objectql,plugin-security): the update's reference forgiveness compares only stored values the caller may read For master_detail, lookup and user, by id and in bulk: a column the caller may edit but not read answers a value the rows store exactly as a value they do not. Controls: a caller who reads the column keeps the forgiveness, the same grant edits another column, and a system write keeps its exemption. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- ...engine-unreadable-reference-target.test.ts | 61 ++++ ...erence-forgiveness-readable-column.test.ts | 269 ++++++++++++++++++ 2 files changed, 330 insertions(+) create mode 100644 packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts diff --git a/packages/objectql/src/engine-unreadable-reference-target.test.ts b/packages/objectql/src/engine-unreadable-reference-target.test.ts index 0cef7139d55..5fa2bfc3b19 100644 --- a/packages/objectql/src/engine-unreadable-reference-target.test.ts +++ b/packages/objectql/src/engine-unreadable-reference-target.test.ts @@ -437,3 +437,64 @@ describe('[#29029] which failures of the reference check fail open, and which fa expect([...storeFor('urt_loose_task').values()].map((r) => r.loose)).toEqual(['x_1']); }); }); + +/** + * [#29029, #29062] The update's reference forgiveness compares only stored + * values the caller may read: the stored rows as the caller's own read door + * serves them. A middleware here withholds the task's reference columns from + * Alice's reads of the task, as a field-level read denial does. + */ +describe('[#29029] the update\'s reference forgiveness compares only stored values the caller may read', () => { + const HIDDEN_FROM_ALICE = ['duty', 'assignee']; + let engine: ObjectQL; + let storeFor: (o: string) => Map>; + + const boot = (itemRead: 'hide columns' | 'fail' | 'serve') => { + ({ engine, storeFor } = makeEngine()); + engine.registerMiddleware(async (ctx: any, next: () => Promise) => { + const own = ctx.object === ITEM && ['find', 'findOne'].includes(ctx.operation) && ctx.context?.userId === 'alice' && !ctx.context?.isSystem; + if (own && itemRead === 'fail') throw new Error('the task read failed'); + await next(); + if (own && itemRead === 'hide columns') { + const strip = (row: any) => { if (row && typeof row === 'object') for (const f of HIDDEN_FROM_ALICE) delete row[f]; }; + if (Array.isArray(ctx.result)) ctx.result.forEach(strip); else strip(ctx.result); + } + }); + for (const id of ['k1', 'k2']) storeFor(ITEM).set(id, { id, title: 'stored', duty: UNREADABLE, assignee: UNREADABLE }); + }; + const write = (door: 'update by id' | 'bulk update', payload: Record, context: Record = ALICE) => + door === 'update by id' + ? engine.update(ITEM, payload, { where: { id: 'k1' }, context } as any) + : engine.update(ITEM, payload, { where: { id: { $in: ['k1', 'k2'] } }, multi: true, context } as any); + const answer = (err: unknown, id: string) => JSON.stringify({ ...envelopeOf(err), sentence: sentence(err, id) }); + const untouched = () => ['k1', 'k2'].map((id) => ({ id, title: 'stored', duty: UNREADABLE, assignee: UNREADABLE })); + + describe.each(FIELDS)('a $kind column', ({ field }) => { + it.each(['update by id', 'bulk update'] as const)('%s — a column the caller cannot read forgives nothing: the stored value answers as another unreadable one', async (door) => { + boot('hide columns'); + const storedValue = await refusalOf(() => write(door, { [field]: UNREADABLE })); + expect(envelopeOf(storedValue)).toEqual(notFound(field)); + const otherValue = await refusalOf(() => write(door, { [field]: 't_flagged_other' })); + expect(answer(storedValue, UNREADABLE)).toBe(answer(otherValue, 't_flagged_other')); + expect([...storeFor(ITEM).values()]).toEqual(untouched()); + }); + + it('update by id — a read of the stored row that fails forgives nothing', async () => { + boot('fail'); + expect(envelopeOf(await refusalOf(() => write('update by id', { [field]: UNREADABLE })))).toEqual(notFound(field)); + }); + + it.each(['update by id', 'bulk update'] as const)('%s — control: a column the caller reads keeps the forgiveness', async (door) => { + boot('serve'); + await write(door, { title: 'renamed', [field]: UNREADABLE }); + expect(storeFor(ITEM).get('k1')).toEqual({ id: 'k1', title: 'renamed', duty: UNREADABLE, assignee: UNREADABLE }); + }); + + it.each(['update by id', 'bulk update'] as const)('%s — control: a flow run\'s system write compares the rows as stored', async (door) => { + boot('hide columns'); + const flowRun = markFlowWriteReferenceCheck({ ...SYSTEM }); + await write(door, { title: 'renamed', [field]: UNREADABLE }, flowRun); + expect(storeFor(ITEM).get('k1')?.title).toBe('renamed'); + }); + }); +}); diff --git a/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts b/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts new file mode 100644 index 00000000000..095dfde0adf --- /dev/null +++ b/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts @@ -0,0 +1,269 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#29029, #29062] The update's reference forgiveness compares only stored + * values the caller may read. + * + * A non-system write may name, in a `lookup`, `user` or `master_detail` + * field, only a record its caller can read; a value it cannot read answers + * `reference_not_found`. An update that carries back a value the row already + * stores is not judged again (#29062) — and that comparison reads the stored + * row as the caller may read it. A column the caller may edit but not read + * (`{ readable: false, editable: true }`, which the field-write gate admits) + * holds no stored value for it, so it forgives nothing: the value is refused + * like any other unreadable reference, and the answer is the same whatever + * the column holds. + * + * ## The rig + * + * The real engine and the real security plugin, on a store-backed driver. + * The caller may read and edit tasks and holds no read on the target object + * or on `sys_user`, so every target id below is one it cannot read. Two + * grants differ only in the reference columns' `readable` bit. + * + * ## Pins + * + * - for each of the three types, by id and in bulk: a column the caller may + * edit but not read answers a value it already stores exactly as it + * answers a value it does not (status, code, field and sentence), and the + * rows are untouched; + * CONTROLS: the same grant edits another column of the same rows; a caller + * who can read the column keeps the forgiveness (the stored value carried + * back lands) and is refused a new unreadable value; a system write keeps + * its exemption. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { resolveThrownHttpError } from '@objectstack/types'; +import type { PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin } from './security-plugin.js'; + +import '@objectstack/spec'; +import '@objectstack/formula'; + +const TARGET = 'rfc_target'; +const ITEM = 'rfc_item'; + +const ITEM_OBJECT = { + name: ITEM, + fields: { + title: { type: 'text' }, + duty: { type: 'lookup', reference: TARGET }, + assignee: { type: 'user' }, + header: { type: 'master_detail', reference: TARGET }, + }, +}; + +const REFERENCE_FIELDS = ['duty', 'assignee', 'header'] as const; + +/** Edits tasks, and may edit but not read their reference columns; reads no target. */ +const HIDDEN_COLUMNS: PermissionSet = { + name: 'member_default', + label: 'Task editor who may not read the reference columns', + objects: { [ITEM]: { allowRead: true, allowCreate: true, allowEdit: true } }, + fields: Object.fromEntries(REFERENCE_FIELDS.map((f) => [`${ITEM}.${f}`, { readable: false, editable: true }])), +} as unknown as PermissionSet; + +/** The same grant with the reference columns readable. */ +const READABLE_COLUMNS: PermissionSet = { + name: 'member_default', + label: 'Task editor who may read the reference columns', + objects: { [ITEM]: { allowRead: true, allowCreate: true, allowEdit: true } }, + fields: Object.fromEntries(REFERENCE_FIELDS.map((f) => [`${ITEM}.${f}`, { readable: true, editable: true }])), +} as unknown as PermissionSet; + +const CALLER = { userId: 'u_editor', tenantId: 'org-1', positions: [], permissions: [], posture: 'MEMBER' }; +const SYS_CTX = { isSystem: true, userId: 'usr_system' }; + +/** What the rows store, per field, and a second id of the same object the caller cannot read either. */ +const STORED: Record<(typeof REFERENCE_FIELDS)[number], string> = { duty: 't_stored', assignee: 'u_stored', header: 't_stored' }; +const OTHER: Record<(typeof REFERENCE_FIELDS)[number], string> = { duty: 't_other', assignee: 'u_other', header: 't_other' }; + +function makeDriver() { + const stores = new Map>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + const matches = (row: any, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]: [string, any]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k === '$or') return (v as any[]).some((w) => matches(row, w)); + const cond = v as any; + if (cond && typeof cond === 'object' && !Array.isArray(cond)) { + if ('$in' in cond) return Array.isArray(cond.$in) && cond.$in.includes(row?.[k]); + if ('$eq' in cond) return row?.[k] === cond.$eq; + } + return row?.[k] === cond; + }); + }; + let n = 0; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(object: string, ast: any) { + const rows = Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)); + const bounded = typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + const fields: string[] | undefined = ast?.fields; + if (!fields) return bounded.map((r) => ({ ...r })); + return bounded.map((r) => { + const out: any = {}; + for (const f of fields) if (r[f] !== undefined) out[f] = r[f]; + return out; + }); + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matches(r, ast?.where)) return { ...r }; + return null; + }, + async create(object: string, data: Record) { + n += 1; + const id = (data.id as string) ?? `r_${n}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return { ...row }; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const row = { ...s.get(id), ...data, id }; + s.set(id, row); + return { ...row }; + }, + async updateMany(object: string, ast: any, data: Record) { + const hits = Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)); + for (const r of hits) storeFor(object).set(r.id, { ...r, ...data, id: r.id }); + return hits.length; + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count(object: string, ast: any) { + return Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)).length; + }, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, storeFor }; +} + +async function boot(set: PermissionSet) { + const engine = new ObjectQL(); + const d = makeDriver(); + engine.registerDriver(d.driver, true); + await engine.init(); + engine.registry.registerObject({ name: TARGET, fields: { name: { type: 'text' } } } as any, 'test-package'); + engine.registry.registerObject({ name: 'sys_user', fields: { name: { type: 'text' }, email: { type: 'text' } } } as any, 'test-package'); + engine.registry.registerObject(ITEM_OBJECT as any, 'test-package'); + for (const id of ['t_stored', 't_other']) d.storeFor(TARGET).set(id, { id, name: id }); + for (const id of ['u_stored', 'u_other']) d.storeFor('sys_user').set(id, { id, name: id, email: `${id}@example.test` }); + + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [set], + }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as any); + await plugin.start(ctx as any); + + // Two tasks the caller may edit, each storing the same unreadable ids. + for (const id of ['k1', 'k2']) { + await engine.insert(ITEM, { id, title: 'stored', ...STORED }, { context: SYS_CTX } as any); + } + return { engine, storeFor: d.storeFor }; +} + +type Door = 'update by id' | 'bulk update'; +const DOORS: Door[] = ['update by id', 'bulk update']; + +const write = (engine: ObjectQL, door: Door, payload: Record, context: Record = CALLER) => + door === 'update by id' + ? engine.update(ITEM, payload, { where: { id: 'k1' }, context } as any) + : engine.update(ITEM, payload, { where: { id: { $in: ['k1', 'k2'] } }, multi: true, context } as any); + +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e; + } + throw new Error('expected the write to be refused, but it succeeded'); +} + +/** + * The answer as every HTTP door reads it (ADR-0112) — status, code, each field + * finding and the sentence — with the id the caller sent written as `ID`, so + * two answers to two ids compare whole. + */ +function answerOf(err: unknown, sentId: string): string { + const thrown = resolveThrownHttpError(err); + const out = (s: unknown) => String(s ?? '').split(sentId).join('ID'); + return JSON.stringify({ + status: thrown.status, + code: thrown.code, + message: out((err as Error).message), + fields: ((err as any)?.fields ?? []).map((f: any) => ({ field: f.field, code: f.code, message: out(f.message) })), + }); +} + +const storedItems = (storeFor: (o: string) => Map) => + [...storeFor(ITEM).values()].map((r) => ({ id: r.id, title: r.title, duty: r.duty, assignee: r.assignee, header: r.header })); + +const UNTOUCHED = ['k1', 'k2'].map((id) => ({ id, title: 'stored', ...STORED })); + +describe('the update\'s reference forgiveness compares only stored values the caller may read', () => { + describe.each(REFERENCE_FIELDS)('the %s column, which the caller may edit but not read', (field) => { + it.each(DOORS)('%s — a value the rows store answers exactly as a value they do not', async (door) => { + const { engine, storeFor } = await boot(HIDDEN_COLUMNS); + const storedValue = await refusalOf(() => write(engine, door, { [field]: STORED[field] })); + const otherValue = await refusalOf(() => write(engine, door, { [field]: OTHER[field] })); + const answer = JSON.parse(answerOf(storedValue, STORED[field])); + expect(answer.status).toBe(400); + expect(answer.code).toBe('VALIDATION_FAILED'); + expect(answer.fields.map((f: any) => ({ field: f.field, code: f.code }))).toEqual([{ field, code: 'reference_not_found' }]); + expect(answerOf(storedValue, STORED[field])).toBe(answerOf(otherValue, OTHER[field])); + expect(storedItems(storeFor)).toEqual(UNTOUCHED); + }); + + it.each(DOORS)('%s — control: the same grant edits another column of the same rows', async (door) => { + const { engine, storeFor } = await boot(HIDDEN_COLUMNS); + await write(engine, door, { title: 'renamed' }); + expect(storedItems(storeFor).find((r) => r.id === 'k1')).toEqual({ id: 'k1', title: 'renamed', ...STORED }); + }); + }); + + describe.each(REFERENCE_FIELDS)('control: the %s column, which the caller can read', (field) => { + it.each(DOORS)('%s — the value the rows store, carried back, lands', async (door) => { + const { engine, storeFor } = await boot(READABLE_COLUMNS); + await write(engine, door, { title: 'renamed', [field]: STORED[field] }); + expect(storedItems(storeFor).find((r) => r.id === 'k1')).toEqual({ id: 'k1', title: 'renamed', ...STORED }); + }); + + it.each(DOORS)('%s — a value the rows do not store is refused as one the caller cannot read', async (door) => { + const { engine, storeFor } = await boot(READABLE_COLUMNS); + const err = await refusalOf(() => write(engine, door, { [field]: OTHER[field] })); + const answer = JSON.parse(answerOf(err, OTHER[field])); + expect({ status: answer.status, code: answer.code }).toEqual({ status: 400, code: 'VALIDATION_FAILED' }); + expect(answer.fields.map((f: any) => ({ field: f.field, code: f.code }))).toEqual([{ field, code: 'reference_not_found' }]); + expect(storedItems(storeFor)).toEqual(UNTOUCHED); + }); + }); + + it.each(DOORS)('%s — control: a system write keeps its exemption', async (door) => { + const { engine, storeFor } = await boot(HIDDEN_COLUMNS); + await write(engine, door, { duty: OTHER.duty }, SYS_CTX); + expect(storedItems(storeFor).find((r) => r.id === 'k1')?.duty).toBe(OTHER.duty); + }); +}); From 341c43a4865c0eaec91a90ff62880ff9c748c2ce Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 15:29:37 +0000 Subject: [PATCH 20/26] docs(objectql): the update's reference forgiveness compares only stored values the caller may read The changeset and the two pages that describe the forgiveness say that a column the caller may edit but not read holds no stored value for it. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .changeset/29029-lookup-user-caller-read.md | 4 +++- content/docs/api/error-catalog.mdx | 4 +++- content/docs/data-modeling/fields.mdx | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.changeset/29029-lookup-user-caller-read.md b/.changeset/29029-lookup-user-caller-read.md index 659283e2907..a67ba7aed70 100644 --- a/.changeset/29029-lookup-user-caller-read.md +++ b/.changeset/29029-lookup-user-caller-read.md @@ -6,7 +6,7 @@ fix(objectql)!: a non-system write may name, in a `lookup` or `user` field, only a record the caller can read (#29029) -Clause-②: yes (narrowing) — the write's reference check narrows: a non-system `lookup` or `user` value naming a record the caller cannot read is refused. The business-unit read widens: a delegated admin reads the business units inside the admin scope it holds, and no other. +Clause-②: yes (narrowing) — the write's reference check narrows: a non-system `lookup` or `user` value naming a record the caller cannot read is refused, and an update's forgiveness of a value the row already stores compares only stored values the caller may read. The business-unit read widens: a delegated admin reads the business units inside the admin scope it holds, and no other. Metadata-protocol: none @@ -15,6 +15,8 @@ Metadata-protocol: none After the upgrade, an insert or update that is not a system write is refused with `400 VALIDATION_FAILED` and the field's localized `reference_not_found` when a `lookup` or `user` value it supplies names a record the caller cannot read, before the record's validation rules run. +An update that carries back a `lookup`, `master_detail` or `user` value the row already stores is still not judged again, but that comparison reads only stored values the caller may read, by id and in bulk alike. In a column the caller may edit but not read (a field permission `{ readable: false, editable: true }`), every value is judged as new: one naming a record the caller cannot read is refused with `reference_not_found` even when the row already stores it. A caller who can read the column keeps the forgiveness, and a system write is unchanged. + A delegated admin whose admin scope covers a business-unit subtree can read the business units in that subtree, and no business unit outside it (`@objectstack/plugin-security`). The parent-write-reach verifier's personas read every record of each object a probed detail's required reference names outside its master chain, so the create probe names records its writer reads (`@objectstack/verify`). diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index 9cbcffc653d..10a1d40dbad 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -328,7 +328,9 @@ unregistered object, or an unreachable datasource on a check that does not read as the caller — fails **open** rather than inventing a rejection, and an update that carries back a value (or a member of a multi-valued field) the row already stores is not judged again, because the update does not add it; on a bulk update that holds -only when every matched row already stores it. +only when every matched row already stores it. That comparison reads only stored +values the caller may read: a column the caller may edit but not read holds no +stored value for it, so a value written there is judged like any other. ### `DUPLICATE_VALUE` diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index cee9be42a53..e6042bc1619 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/content/docs/data-modeling/fields.mdx @@ -290,6 +290,8 @@ rejected the same way, before the record's validation rules run. The same check runs on bulk updates. An update judges only the references it adds: a value the row already stores, carried back unchanged, is not judged again, so a deleted user's id left in a `user` field does not block later edits of the record. +That comparison reads only stored values the caller may read: in a column the +caller may edit but not read, every value is judged as new. Clearing a relationship is not a dangling reference: `null`, `""` and `[]` all mean "no link". Which of them `deleteBehavior: 'set_null'` writes when the referenced From 798dbc53aad56f79a8e49e222f30af0228908dc4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:20:18 +0000 Subject: [PATCH 21/26] docs(permissions): the system-context page's reference-check row names the forgiveness's system-write read Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- content/docs/permissions/system-context.mdx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index af0959e3492..3d45ba60caf 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -134,7 +134,7 @@ bugs — it is the protection or the side effect 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 | `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 | `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 | `insert`, `packages/objectql/src/readonly-strict-errors.ts#READONLY_CLASS_REASONS` | -| 23 | **Referential-integrity check skipped** — except a `runAs: 'system'` flow's record writes, which keep it | objectql | Get: writes proceed against unreachable/unresolvable targets — a seed replay, an import, a restore or a cascade may store a reference to a row that does not exist yet. Lose: an `isSystem` caller can write a **dangling reference**. A flow run's elevated context (`resolveRunDataContext`) carries an internal mark that keeps the check, so a flow's `create_record` / `update_record` naming a row that does not exist is refused `400 VALIDATION_FAILED` / `reference_not_found`, as on the user path; its authorization elevation is unchanged. A write the flow's write sets off (a hook's `ctx.api`, its `sudo()`, a cascade leg of a flow's delete) spreads the run's context and is checked as the flow's own, as on the user path. The mark is a symbol no flow definition or request can spell, and `ExecutionContextSchema.parse` drops it. Where a system write runs the check (a flow run's), it runs after the record's validation rules, as before; every other write runs it before them | `assertReferencesResolve`, `referencesCheckedBeforeRules` | +| 23 | **Referential-integrity check skipped** — except a `runAs: 'system'` flow's record writes, which keep it | objectql | Get: writes proceed against unreachable/unresolvable targets — a seed replay, an import, a restore or a cascade may store a reference to a row that does not exist yet. Lose: an `isSystem` caller can write a **dangling reference**. A flow run's elevated context (`resolveRunDataContext`) carries an internal mark that keeps the check, so a flow's `create_record` / `update_record` naming a row that does not exist is refused `400 VALIDATION_FAILED` / `reference_not_found`, as on the user path; its authorization elevation is unchanged. A write the flow's write sets off (a hook's `ctx.api`, its `sudo()`, a cascade leg of a flow's delete) spreads the run's context and is checked as the flow's own, as on the user path. The mark is a symbol no flow definition or request can spell, and `ExecutionContextSchema.parse` drops it. Where a system write runs the check (a flow run's), it runs after the record's validation rules, as before; every other write runs it before them. An update's check does not judge again a value every row it lands on already stores; a system write compares those rows as stored, every other write only as the caller may read them, so a column the caller cannot read forgives nothing | `assertReferencesResolve`, `referencesCheckedBeforeRules`, `storedReferencesCallerReads` | | 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` | @@ -542,6 +542,7 @@ and CI refuses one that differs from what the tree produces. | `packages/objectql/src/engine-referential-integrity.ts#assertReferencesResolve` | 1 | 23 | | `packages/objectql/src/engine-referential-integrity.ts#masterDetailHeadersCallerReads` | 1 | 29c | | `packages/objectql/src/engine-referential-integrity.ts#referencesCheckedBeforeRules` | 1 | 23 | +| `packages/objectql/src/engine-referential-integrity.ts#storedReferencesCallerReads` | 1 | 23 | | `packages/objectql/src/engine-secret-internal-fields.ts#encryptSecretFields` | 1 | 23e | | `packages/objectql/src/engine-secret-internal-fields.ts#stripSearchCompanionFromRead` | 1 | 27 | | `packages/objectql/src/engine-validate.ts#validate` | 1 | 21b | From 2ae4d980c234c0137467eef86e249ac5b2f9d7f6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 17:56:29 +0000 Subject: [PATCH 22/26] test(verify): the parent-write-reach suite pays the harness's two lazy service loads at module top Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- packages/verify/src/parent-write-reach.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/verify/src/parent-write-reach.test.ts b/packages/verify/src/parent-write-reach.test.ts index f328f1410e5..33821be98ca 100644 --- a/packages/verify/src/parent-write-reach.test.ts +++ b/packages/verify/src/parent-write-reach.test.ts @@ -20,6 +20,9 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { defineStack } from '@objectstack/spec'; +// The harness loads these two inside a function body; paid here, during collection, not in a clocked window. +import '@objectstack/service-datasource'; +import '@objectstack/service-automation'; import { bootStack, type VerifyStack } from './harness.js'; import { warmUpBoot } from './boot-warm-up.testkit.js'; From cf174c5c91d440fcdb317a74daee1fec80f4fe7b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 20:19:49 +0000 Subject: [PATCH 23/26] fix(objectql): an update compares a sent value only against the stored row as the caller reads it A non-system write's secret reference is kept only where the caller's own read door serves the column as stored, and a master-detail repoint's prior header is read off the stored foreign key only as the caller reads it. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../src/engine-master-detail-related.ts | 46 +++++++++++++ .../src/engine-secret-internal-fields.ts | 58 +++++++++++----- packages/objectql/src/engine.ts | 66 +++++++++++++++---- 3 files changed, 143 insertions(+), 27 deletions(-) diff --git a/packages/objectql/src/engine-master-detail-related.ts b/packages/objectql/src/engine-master-detail-related.ts index ef2d03d2428..b0922ad75a0 100644 --- a/packages/objectql/src/engine-master-detail-related.ts +++ b/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'; /** @@ -338,6 +344,46 @@ export async function resolvePreviewParents( }; } +/** + * 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, + schema: unknown, + rows: ReadonlyArray | null | undefined>, + context: ExecutionContext | undefined, +): Promise> { + 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. diff --git a/packages/objectql/src/engine-secret-internal-fields.ts b/packages/objectql/src/engine-secret-internal-fields.ts index fe8ad2a06ca..2c8afad5a92 100644 --- a/packages/objectql/src/engine-secret-internal-fields.ts +++ b/packages/objectql/src/engine-secret-internal-fields.ts @@ -27,7 +27,11 @@ * may only seal a new value or keep its own record's ref. `encryptSecretFields` * takes the write's own stored rows ({@link StoredRowsReader}) and refuses any * other ref with {@link SecretRefWriteError}; the system principal keeps the - * pass-through. + * pass-through. The stored rows a non-system write is compared against are + * the rows as that caller's own read door serves them + * ({@link storedReferencesCallerReads}), which serves a `secret` column + * masked, so no stored ref is one a non-system write keeps: every ref it + * sends gets the same refusal, whatever the column holds. * * ## How the moved bodies reach the class * @@ -40,7 +44,7 @@ * (`_registry`, `cryptoProvider`, `txStore`, `logger`) go as GETTERS, so each * read happens when the body makes it: `setCryptoProvider` replaces the * provider at any time, and a body sees whatever is installed at the moment it - * reads, as it did. The six methods they call (`getDriver`, + * reads, as it did. The seven methods they call (`find`, `getDriver`, * `transactionCoversDriverFor`, `isClosedUnitHandle`, `omitInternalFields`, * `privilegedReadDriverOptions`, `resolveSecret`) go as late-bound closures * over the instance, so a double or spy set on the engine — tests replace @@ -74,6 +78,7 @@ import { SECRET_MASK, } from './secret-fields.js'; import { isSearchCompanionRequested, stripSearchCompanion } from './search-companion.js'; +import { storedReferencesCallerReads } from './engine-referential-integrity.js'; import type { ObjectQL } from './engine.js'; /** @@ -89,6 +94,7 @@ export interface SecretInternalFieldsHost { readonly cryptoProvider: ObjectQL['cryptoProvider']; readonly txStore: ObjectQL['txStore']; readonly logger: ObjectQL['logger']; + find: ObjectQL['find']; getDriver: ObjectQL['getDriver']; transactionCoversDriverFor: ObjectQL['transactionCoversDriverFor']; isClosedUnitHandle: ObjectQL['isClosedUnitHandle']; @@ -124,7 +130,9 @@ export interface SecretHandleSlot { * insert (a new record holds no ref of its own), the pre-image for a by-id * update, the matched rows for a predicate update — and the reader is called * only when a non-system payload carries a ref, so every other write reads - * nothing more than it did. + * nothing more than it did. The rows are compared only as the caller's own + * read door serves them ({@link storedReferencesCallerReads}), never as the + * call site read them. */ export type StoredRowsReader = () => Promise | null | undefined)[] | null>; @@ -140,9 +148,12 @@ export const SECRET_REF_WRITE_REFUSAL_STATUS = 400; /** * [#27582] Refusal of a `secret:` ref on a non-system write that is not the - * value the record's own column already holds. Fail closed, with one answer: - * the message names the column and never the ref, and nothing about the handle - * the ref names is looked up, so the answer is the same whatever it names. + * value the record's own column holds as its caller reads it. Fail closed, with + * one answer: the message names the column and never the ref, and nothing about + * the handle the ref names is looked up, so the answer is the same whatever it + * names. The read door serves a `secret` column masked to every caller, so a + * non-system write that sends a ref always meets this refusal, whatever the + * column holds. */ export class SecretRefWriteError extends Error { readonly code = SECRET_REF_WRITE_REFUSAL_CODE; @@ -154,9 +165,10 @@ export class SecretRefWriteError extends Error { constructor(object: string, field: string) { super( `Stored-reference value refused for secret field "${object}.${field}": a value that starts with "secret:" ` - + 'is read as a reference to a credential already stored, not as a credential, and a write may only keep ' - + 'the reference this record already holds. To set the secret, send its value; to clear it, write null; ' - + 'to leave it unchanged, omit the field. Nothing was written.', + + 'is read as a reference to a credential already stored, not as a credential, and a write keeps a ' + + 'reference only where it is the value its caller reads in this column, which serves a secret masked. ' + + 'To set the secret, send its value; to clear it, write null; to leave it unchanged, omit the field or ' + + 'send back the mask it reads. Nothing was written.', ); this.name = 'SecretRefWriteError'; this.object = object; @@ -200,18 +212,21 @@ export class SecretRefWriteError extends Error { * a new value or keep its own record's ref. From the system principal * (`context.isSystem === true`) it is left as-is, as it always was. From * any other caller it is kept only when every row the write lands on - * already holds exactly that ref in that column — read from `storedRows`, - * which each call site supplies (none for an insert, the pre-image for a - * by-id update, the matched rows for a predicate update) — and then the - * key is dropped, as the echoed mask is. Any other ref is REFUSED with + * holds exactly that ref in that column AS THE CALLER'S OWN READ DOOR + * SERVES IT — `storedRows`, which each call site supplies (none for an + * insert, the pre-image for a by-id update, the matched rows for a + * predicate update), projected by {@link storedReferencesCallerReads} — + * and then the key is dropped, as the echoed mask is. The read door serves + * a `secret` column masked ({@link maskSecretFields}), never as stored, so + * no stored ref is kept: every ref is REFUSED with * {@link SecretRefWriteError} (ADR-0112 `VALIDATION_ERROR`/400), one - * answer whatever the ref names. + * answer whatever the ref names and whatever the column holds. * - **Fail-closed:** any other secret value with no CryptoProvider registered, * or no reachable `sys_secret` store, THROWS — never persists cleartext. * (A `password` field needs no CryptoProvider — it is stored as-is.) */ export async function encryptSecretFields( - this: Pick, + this: Pick, object: string, row: Record, context: ExecutionContext | undefined, @@ -256,7 +271,18 @@ export async function encryptSecretFields( // predicate that matches nothing) has nothing to keep. Kept, the key is // dropped, as the echoed mask is: the column is not rewritten at all, so // the verdict cannot go stale before the statement runs. - ownRows ??= (await storedRows()) ?? []; + // ⛔ The rows are compared only as this caller's own read door serves + // them, never as stored: the answer to a column the caller cannot read + // as stored must not depend on what it holds. The door serves a + // `secret` column masked, so no ref survives the projection and every + // ref this caller sends is refused alike. + ownRows ??= await storedReferencesCallerReads.call( + this, + object, + ((await storedRows()) ?? []).map((own) => (own && typeof own === 'object' ? own : {})), + secretFields, + context, + ); const keepsOwn = ownRows.length > 0 && ownRows.every((own) => own?.[field] === value); if (!keepsOwn) throw new SecretRefWriteError(object, field); delete row[field]; diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index a9b6fefb533..febd1b0624f 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -311,7 +311,7 @@ import { // hand each body the members it reads (`MasterDetailRelatedHost`). Imported after // every module those bodies import, so the engine's module evaluation order // is what it was. -import { resolveOneHopRelated, resolvePredicateRelated, resolvePreviewParents, settleMasterDetailLanding } from './engine-master-detail-related.js'; +import { priorMasterIdsCallerReads, resolveOneHopRelated, resolvePredicateRelated, resolvePreviewParents, settleMasterDetailLanding } from './engine-master-detail-related.js'; // [#25522 cut 15] The `ReadableIds` chain composition, the controlled-by-parent // boot screen, and the permission and formula resolutions an operation shares; // the methods of the same names below hand each body the members it reads @@ -2842,6 +2842,20 @@ export class ObjectQL implements IObjectQLEngine { return resolvePreviewParents.call(host, schema, views, storedRows, context); } + private async priorMasterIdsCallerReads( + schema: unknown, + rows: ReadonlyArray | null | undefined>, + context: ExecutionContext | undefined, + ): Promise> { + // The body lives in `engine-master-detail-related.ts`, its docblock with it (see `MasterDetailRelatedHost`). + const readLogger = () => this.logger; + const host: ThisParameterType = { + find: (...a) => this.find(...a), + get logger() { return readLogger(); }, + }; + return priorMasterIdsCallerReads.call(host, schema, rows, context); + } + private async resolvePredicateRelated( schema: any, rows: ReadonlyArray | undefined | null>, @@ -3108,10 +3122,14 @@ export class ObjectQL implements IObjectQLEngine { // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const readRegistry = () => this._registry; const readCryptoProvider = () => this.cryptoProvider; + const readLogger = () => this.logger; const host: ThisParameterType = { get _registry() { return readRegistry(); }, get cryptoProvider() { return readCryptoProvider(); }, + get logger() { return readLogger(); }, getDriver: (...a) => this.getDriver(...a), + // A ref on a non-system write is compared against its own rows as the caller's read door serves them. + find: (...a) => this.find(...a), }; return encryptSecretFields.call(host, object, row, context, driverOptions, storedRows); } @@ -7710,15 +7728,29 @@ export class ObjectQL implements IObjectQLEngine { // and `evaluateValidationRules` reuses `parent` for both. // [#19853] "Actually moves" is asked of the view the write // stores: a repoint that never lands moves nothing. + // ⛔ And of the stored FK only as the caller's own read door + // serves it (`priorMasterIdsCallerReads`), asked only when the + // write names the FK: a FK the caller cannot read as stored + // leaves the prior master UNKNOWN, bound as the unresolved + // header (`null`), so whether the pre-check reads a second + // header, and which, never depends on what the column holds. + // A FK the caller reads, and a system write, are judged as + // before. const mdRel = schemaHasParentRequiredWhen ? resolveMasterDetailRelation(updateSchema as any) : null; - const priorMasterId = mdRel ? masterIdOf(mdRel.fk, null, priorRecord) : undefined; + const landingView = landing?.view ?? preRoWhen; + const namesMaster = mdRel != null && landingView != null && typeof landingView === 'object' && mdRel.fk in landingView; + const [priorMaster] = namesMaster + ? await this.priorMasterIdsCallerReads(updateSchema, [priorRecord], opCtx.context) + : [undefined]; const repointsMaster = mdRel != null && - priorMasterId != null && - masterIdOf(mdRel.fk, landing?.view ?? preRoWhen, priorRecord) !== priorMasterId; - const roWhenPreviousParent = repointsMaster - ? await this.resolveMasterDetailParent(updateSchema, null, priorRecord, opCtx.context) - : undefined; + priorMaster?.id != null && + masterIdOf(mdRel.fk, landingView, null) !== priorMaster.id; + const roWhenPreviousParent = priorMaster === null + ? null + : repointsMaster + ? await this.resolveMasterDetailParent(updateSchema, null, { [mdRel!.fk]: priorMaster!.id }, opCtx.context) + : undefined; // [#9107] `suppliedValues` — the SAME entry snapshot the static // strip below consumes, now feeding the conditional one too, so // both strips answer "did the CALLER write this key?" from one @@ -8101,11 +8133,23 @@ export class ObjectQL implements IObjectQLEngine { // resolved only when the payload REPOINTS the matched rows at // another master (see the single-id branch for why the stored // row's own header is the one that question needs). + // ⛔ Each matched row's stored FK only as the caller's own read + // door serves it, as on the by-id branch: a row whose FK the + // caller cannot read as stored binds the unresolved header + // (`null`), and its stored master is never read. const mdRelMulti = schemaHasParentRequiredWhenMulti ? resolveMasterDetailRelation(updateSchema as any) : null; - const previousParentForRow = - mdRelMulti != null && masterIdOf(mdRelMulti.fk, landingMulti?.view ?? preRoWhenMulti, undefined) != null - ? await this.resolveMasterDetailParents(updateSchema, null, priorRows, opCtx.context) - : undefined; + let previousParentForRow: ((row: Record | undefined) => Record | null) | undefined; + if (mdRelMulti != null && masterIdOf(mdRelMulti.fk, landingMulti?.view ?? preRoWhenMulti, undefined) != null) { + const matched = priorRows ?? []; + const priorMasters = await this.priorMasterIdsCallerReads(updateSchema, matched, opCtx.context); + const known = matched.flatMap((row, i) => { + const prior = priorMasters[i]; + return prior ? [{ row, stored: { [mdRelMulti.fk]: prior.id } }] : []; + }); + const headerOf = await this.resolveMasterDetailParents(updateSchema, null, known.map((k) => k.stored), opCtx.context); + const previousByRow = new Map(known.map((k) => [k.row, headerOf(k.stored)])); + previousParentForRow = (row) => (row ? previousByRow.get(row) ?? null : null); + } if (payloadHasReadonlyWhen) { // [#9107] Same entry snapshot, same authorship gate as the // by-id branch above — "both call sites" is the #3106 / From b5d1b51a8e5d83a1fd133027e11b61eaab2aaa1a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 20:27:14 +0000 Subject: [PATCH 24/26] test(objectql,plugin-security): an update's answer does not depend on a stored value the caller cannot read Pins the secret-reference keep and the master-detail repoint pre-check, by id and in bulk, with the real security plugin; rewrites the own-reference suites to the rule that a secret column reads masked. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- ...-row-caller-read-secret-ref-and-repoint.md | 16 ++ content/docs/permissions/system-context.mdx | 2 +- .../src/secret-field-own-ref-write.test.ts | 68 ++++-- ...erence-forgiveness-readable-column.test.ts | 225 +++++++++++++++++- ...secret-field-own-ref-write.dogfood.test.ts | 27 ++- 5 files changed, 299 insertions(+), 39 deletions(-) create mode 100644 .changeset/update-stored-row-caller-read-secret-ref-and-repoint.md diff --git a/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md b/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md new file mode 100644 index 00000000000..cae9f3ce9e1 --- /dev/null +++ b/.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 + + + +**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. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 06f95ab9505..f0278872e85 100644 --- a/content/docs/permissions/system-context.mdx +++ b/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` | diff --git a/packages/objectql/src/secret-field-own-ref-write.test.ts b/packages/objectql/src/secret-field-own-ref-write.test.ts index 6b825ebe586..da5927b1ebd 100644 --- a/packages/objectql/src/secret-field-own-ref-write.test.ts +++ b/packages/objectql/src/secret-field-own-ref-write.test.ts @@ -2,15 +2,17 @@ /** * [#27582] A non-system write may only seal a new value or keep its own - * record's ref. + * record's ref — and "its own" is the value its caller reads in that column. * * A `secret` field's stored value is an opaque `secret:` ref. On a write that - * is not the system principal's, a value already spelled as a ref is accepted - * only when it is the ref that same record's same column already holds, and - * then it changes nothing (like the echoed read mask). Any other ref is - * refused with the ADR-0112 `VALIDATION_ERROR` / 400 pair, before any - * credential row or record is written, and with one answer whatever the ref - * names. A system write keeps the pass-through. + * is not the system principal's, a value already spelled as a ref is compared + * with the rows the write lands on only as the caller's own read door serves + * them, and the read door serves a `secret` column masked to every caller. So + * no ref is one such a write keeps: every ref is refused with the ADR-0112 + * `VALIDATION_ERROR` / 400 pair, before any credential row or record is + * written, with one answer whatever the ref names and whatever the column + * holds — the ref the row itself holds included. The echoed read mask is + * still a no-op, and a system write keeps the pass-through. * * Driven through a real {@link ObjectQL} over an in-memory stub driver, on * every engine write door the credential pass sits on: insert (single, batch @@ -122,7 +124,7 @@ const USER = { context: { isSystem: false, userId: 'u1' } } as any; /** A ref spelled like a stored one, naming no `sys_secret` row at all. */ const UNKNOWN_REF = makeSecretRef('sec_none'); -describe('[#27582] a non-system write may only seal a new value or keep its own record\'s ref', () => { +describe('[#27582] a non-system write may only seal a new value, never send a ref', () => { let engine: ObjectQL; let d: ReturnType; let crypto: ReturnType; @@ -210,22 +212,25 @@ describe('[#27582] a non-system write may only seal a new value or keep its own }); }); - describe('by-id update: only the ref this record already holds', () => { - it('keeping its own ref is accepted and changes nothing', async () => { + describe('by-id update: a ref is answered the same whatever the column holds', () => { + it('the ref this record holds is refused exactly as another record\'s, and nothing is written', async () => { const own = await seed('r1', 'mine'); - const before = secrets().map((s) => s.id); - await engine.update(VAULT, { id: 'r1', name: 'renamed', token: own }, USER); - expect(stored('r1')).toMatchObject({ name: 'renamed', token: own }); - // Kept, not rewritten: the statement leaves the column alone, as for the echoed mask. - expect(d.lastStatement(VAULT)).toMatchObject({ name: 'renamed' }); - expect(d.lastStatement(VAULT)).not.toHaveProperty('token'); - expect(secrets().map((s) => s.id)).toEqual(before); + const theirs = await seed('r2', 'theirs'); + const before = { secrets: secrets().map((s) => s.id), statement: d.lastStatement(VAULT) }; + const kept = await caught(engine.update(VAULT, { id: 'r1', name: 'renamed', token: own }, USER)); + const named = await caught(engine.update(VAULT, { id: 'r1', name: 'renamed', token: theirs }, USER)); + expect(kept).toMatchObject(refusal()); + expect(named).toMatchObject(refusal()); + expect(kept!.message).toBe(named!.message); + expect(stored('r1')).toMatchObject({ name: 'r1', token: own }); + expect(d.lastStatement(VAULT)).toBe(before.statement); + expect(secrets().map((s) => s.id)).toEqual(before.secrets); expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); }); - it('keeping its own ref as the only key is accepted too', async () => { + it('the ref this record holds, as the only key, is refused too', async () => { const own = await seed('r1', 'mine'); - await engine.update(VAULT, { id: 'r1', token: own }, USER); + expect(await caught(engine.update(VAULT, { id: 'r1', token: own }, USER))).toMatchObject(refusal()); expect(refOf('r1')).toBe(own); expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); }); @@ -236,11 +241,21 @@ describe('[#27582] a non-system write may only seal a new value or keep its own expect(refOf('r1')).toBe(own); }); - it('a beforeUpdate hook that writes the stored row back keeps its own ref', async () => { + it('a beforeUpdate hook that writes the stored row back is judged on the ref it carries, as a caller is', async () => { const own = await seed('r1', 'mine'); engine.registerHook('beforeUpdate', (ctx: any) => { ctx.input.data = { ...ctx.previous, ...ctx.input.data }; }, { object: VAULT }); + expect(await caught(engine.update(VAULT, { id: 'r1', name: 'renamed' }, USER))).toMatchObject(refusal()); + expect(stored('r1')).toMatchObject({ name: 'r1', token: own }); + }); + + it('a beforeUpdate hook that writes the stored row back without its secret column lands', async () => { + const own = await seed('r1', 'mine'); + engine.registerHook('beforeUpdate', (ctx: any) => { + const { token: _held, ...rest } = ctx.previous; + ctx.input.data = { ...rest, ...ctx.input.data }; + }, { object: VAULT }); await engine.update(VAULT, { id: 'r1', name: 'renamed' }, USER); expect(stored('r1')).toMatchObject({ name: 'renamed', token: own }); expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); @@ -282,18 +297,21 @@ describe('[#27582] a non-system write may only seal a new value or keep its own }); }); - describe('predicate update: only a ref every matched record already holds', () => { + describe('predicate update: a ref is answered the same whatever the matched rows hold', () => { const multi = (where: Row) => ({ ...USER, where, multi: true }); - it('a ref every matched row already holds is accepted and changes nothing', async () => { + it('a ref every matched row holds is refused exactly as one none holds, and nothing is written', async () => { await seed('r1', 'one'); await seed('r2', 'two'); await engine.update(VAULT, { token: 'shared' }, { ...SYSTEM, where: { tier: 'gold' }, multi: true }); const shared = refOf('r1'); expect(refOf('r2')).toBe(shared); - await engine.update(VAULT, { name: 'renamed', token: shared }, multi({ tier: 'gold' })); - expect(d.rowsOf(VAULT).map((r) => [r.name, r.token])).toEqual([['renamed', shared], ['renamed', shared]]); - expect(d.lastStatement(VAULT)).not.toHaveProperty('token'); + const held = await caught(engine.update(VAULT, { name: 'renamed', token: shared }, multi({ tier: 'gold' }))); + const unknown = await caught(engine.update(VAULT, { name: 'renamed', token: UNKNOWN_REF }, multi({ tier: 'gold' }))); + expect(held).toMatchObject(refusal()); + expect(unknown).toMatchObject(refusal()); + expect(held!.message).toBe(unknown!.message); + expect(d.rowsOf(VAULT).map((r) => [r.name, r.token])).toEqual([['r1', shared], ['r2', shared]]); expect(await engine.resolveSecretField(VAULT, 'r2', 'token')).toBe('shared'); }); diff --git a/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts b/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts index 095dfde0adf..40c98d03e04 100644 --- a/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts +++ b/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts @@ -31,10 +31,15 @@ * who can read the column keeps the forgiveness (the stored value carried * back lands) and is refused a new unreadable value; a system write keeps * its exemption. + * + * Two more update seams that compare a sent value with the stored row are + * pinned on the same rule at the end of this file: the stored header a + * parent-scoped `requiredWhen` pre-check reads, and the `secret` reference a + * write may keep. */ import { describe, it, expect, vi } from 'vitest'; -import { ObjectQL } from '@objectstack/objectql'; +import { ObjectQL, SECRET_MASK } from '@objectstack/objectql'; import { resolveThrownHttpError } from '@objectstack/types'; import type { PermissionSet } from '@objectstack/spec/security'; import { SecurityPlugin } from './security-plugin.js'; @@ -267,3 +272,221 @@ describe('the update\'s reference forgiveness compares only stored values the ca expect(storedItems(storeFor).find((r) => r.id === 'k1')?.duty).toBe(OTHER.duty); }); }); + +/** + * The same rule on the two other update seams that compare a sent value with + * the stored row: the master-detail header the ADR-0113 pre-check of a + * parent-scoped `requiredWhen` reads, and the `secret` reference a write may + * keep. Each is answered from the stored row only as the caller's own read + * door serves it, so for a column the caller may edit but not read the answer + * is the same whatever the column holds. + * + * The rig is the one above with its own objects: a header the caller can + * read, a detail line whose `header` FK carries the grant's `readable` bit, + * and a vault whose `token` secret does. Rows are seeded straight into the + * store, so a line can hold a stored state no write would produce (a legacy + * line that already lacks its required note). + */ +const HEADER = 'rfc_header'; +const LINE = 'rfc_line'; +const VAULT = 'rfc_vault'; + +const HEADER_OBJECT = { name: HEADER, fields: { name: { type: 'text' }, status: { type: 'text' } } }; +const LINE_OBJECT = { + name: LINE, + fields: { + title: { type: 'text' }, + header: { type: 'master_detail', reference: HEADER }, + note: { type: 'text', requiredWhen: "parent.status == 'paid'" }, + }, +}; +const SYS_SECRET_OBJECT = { + name: 'sys_secret', systemFields: { tenant: false }, + fields: { + namespace: { type: 'text' }, key: { type: 'text' }, kms_key_id: { type: 'text' }, alg: { type: 'text' }, + version: { type: 'number' }, ciphertext: { type: 'text' }, created_at: { type: 'datetime' }, + }, +}; +const VAULT_OBJECT = { name: VAULT, fields: { name: { type: 'text' }, token: { type: 'secret' } } }; + +/** Reads headers; edits lines and vaults; `readable` is the one bit two grants differ in. */ +function columnGrant(readable: boolean): PermissionSet { + return { + name: 'member_default', + label: readable ? 'Editor who may read the column' : 'Editor who may edit but not read the column', + objects: { + [HEADER]: { allowRead: true }, + [LINE]: { allowRead: true, allowCreate: true, allowEdit: true }, + [VAULT]: { allowRead: true, allowCreate: true, allowEdit: true }, + }, + fields: { + [`${LINE}.header`]: { readable, editable: true }, + [`${VAULT}.token`]: { readable, editable: true }, + }, + } as unknown as PermissionSet; +} + +function makeCrypto() { + let n = 0; + return { + async encrypt(plain: string) { + n += 1; + return { id: `sec_${n}`, kmsKeyId: 'local', alg: 'test-b64', version: 1, ciphertext: Buffer.from(plain).toString('base64') }; + }, + async decrypt(handle: { ciphertext: string }) { return Buffer.from(handle.ciphertext, 'base64').toString('utf8'); }, + async rotateKey(handle: unknown) { return handle; }, + digest(plain: string) { return `d:${plain.length}`; }, + async keyedDigest(plain: string) { return `k:${plain.length}`; }, + }; +} + +async function bootStored(set: PermissionSet) { + const engine = new ObjectQL(); + const d = makeDriver(); + engine.registerDriver(d.driver, true); + await engine.init(); + for (const object of [HEADER_OBJECT, LINE_OBJECT, SYS_SECRET_OBJECT, VAULT_OBJECT]) { + engine.registry.registerObject(object as any, 'test-package'); + } + engine.setCryptoProvider(makeCrypto() as never); + d.storeFor(HEADER).set('h_paid', { id: 'h_paid', name: 'paid', status: 'paid' }); + d.storeFor(HEADER).set('h_open', { id: 'h_open', name: 'open', status: 'open' }); + + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [set], + }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as any); + await plugin.start(ctx as any); + return { engine, storeFor: d.storeFor }; +} + +/** The answer a write gets, whole: `ok`, or the status, code, sentence and field findings (ADR-0112). */ +async function outcomeOf(run: () => Promise): Promise { + try { + await run(); + return 'ok'; + } catch (err) { + const thrown = resolveThrownHttpError(err); + return JSON.stringify({ + status: thrown.status, + code: thrown.code, + message: (err as Error).message, + fields: ((err as any)?.fields ?? []).map((f: any) => ({ field: f.field, code: f.code, message: f.message })), + }); + } +} + +describe('a parent-scoped requiredWhen pre-check reads the stored header only through a FK the caller may read', () => { + /** Two lines that already lack the note, each hanging off `stored`. */ + async function rig(readable: boolean, stored: 'h_paid' | 'h_open') { + const booted = await bootStored(columnGrant(readable)); + for (const id of ['l1', 'l2']) booted.storeFor(LINE).set(id, { id, title: 'stored', header: stored, note: null }); + return booted; + } + const writeLines = (engine: ObjectQL, door: Door, payload: Record, context: Record = CALLER) => + door === 'update by id' + ? engine.update(LINE, payload, { where: { id: 'l1' }, context } as any) + : engine.update(LINE, payload, { where: { id: { $in: ['l1', 'l2'] } }, multi: true, context } as any); + const linesOf = (storeFor: (o: string) => Map) => + [...storeFor(LINE).values()].map((r) => ({ id: r.id, title: r.title, header: r.header })); + + it.each(DOORS)('%s — naming a header with the FK hidden answers the same whatever the line stores', async (door) => { + const answers: string[] = []; + for (const stored of ['h_paid', 'h_open'] as const) { + const { engine, storeFor } = await rig(false, stored); + answers.push(await outcomeOf(() => writeLines(engine, door, { title: 'renamed', header: 'h_paid' }))); + // Refused before the statement: the lines are as stored. + expect(linesOf(storeFor)).toEqual(['l1', 'l2'].map((id) => ({ id, title: 'stored', header: stored }))); + } + expect(answers[0]).toBe(answers[1]); + const answer = JSON.parse(answers[0]); + expect({ status: answer.status, code: answer.code }).toEqual({ status: 400, code: 'VALIDATION_FAILED' }); + expect(answer.fields.map((f: any) => ({ field: f.field, code: f.code }))).toEqual([{ field: 'note', code: 'required' }]); + }); + + it.each(DOORS)('%s — control: with the FK hidden, an edit that does not name it is judged as before', async (door) => { + const { engine, storeFor } = await rig(false, 'h_paid'); + await writeLines(engine, door, { title: 'renamed' }); + expect(linesOf(storeFor).find((r) => r.id === 'l1')).toEqual({ id: 'l1', title: 'renamed', header: 'h_paid' }); + }); + + it.each(DOORS)('%s — control: a caller who reads the FK keeps the legacy rest, and a repoint is judged as before', async (door) => { + const resting = await rig(true, 'h_paid'); + await writeLines(resting.engine, door, { title: 'renamed', header: 'h_paid' }); + expect(linesOf(resting.storeFor).find((r) => r.id === 'l1')).toEqual({ id: 'l1', title: 'renamed', header: 'h_paid' }); + + const repointed = await rig(true, 'h_open'); + const answer = JSON.parse(await outcomeOf(() => writeLines(repointed.engine, door, { title: 'renamed', header: 'h_paid' }))); + expect(answer.fields.map((f: any) => ({ field: f.field, code: f.code }))).toEqual([{ field: 'note', code: 'required' }]); + expect(linesOf(repointed.storeFor).find((r) => r.id === 'l1')).toEqual({ id: 'l1', title: 'stored', header: 'h_open' }); + }); + + it.each(DOORS)('%s — control: a system write is judged on the stored row as before', async (door) => { + const resting = await rig(false, 'h_paid'); + await writeLines(resting.engine, door, { title: 'renamed', header: 'h_paid' }, SYS_CTX); + expect(linesOf(resting.storeFor).find((r) => r.id === 'l1')).toEqual({ id: 'l1', title: 'renamed', header: 'h_paid' }); + + const repointed = await rig(false, 'h_open'); + const answer = JSON.parse(await outcomeOf(() => writeLines(repointed.engine, door, { title: 'renamed', header: 'h_paid' }, SYS_CTX))); + expect(answer.fields.map((f: any) => ({ field: f.field, code: f.code }))).toEqual([{ field: 'note', code: 'required' }]); + }); +}); + +describe('a secret reference on a non-system update is answered the same whatever the column holds', () => { + /** Two vault rows sealed by a system write; `v1` and `v2` hold different refs. */ + async function rig(readable: boolean) { + const booted = await bootStored(columnGrant(readable)); + for (const id of ['v1', 'v2']) await booted.engine.insert(VAULT, { id, name: id, token: `${id}-value` }, { context: SYS_CTX } as any); + const refOf = (id: string) => booted.storeFor(VAULT).get(id)?.token as string; + return { ...booted, refOf }; + } + const writeVault = (engine: ObjectQL, door: Door, payload: Record, context: Record = CALLER) => + door === 'update by id' + ? engine.update(VAULT, payload, { where: { id: 'v1' }, context } as any) + : engine.update(VAULT, payload, { where: { id: { $in: ['v1'] } }, multi: true, context } as any); + + describe.each([false, true])('the caller reads the column: %s', (readable) => { + it.each(DOORS)('%s — the ref the row holds, another row\'s and one naming nothing get one answer', async (door) => { + const { engine, storeFor, refOf } = await rig(readable); + const before = [...storeFor(VAULT).values()].map((r) => ({ ...r })); + const own = await outcomeOf(() => writeVault(engine, door, { name: 'renamed', token: refOf('v1') })); + const other = await outcomeOf(() => writeVault(engine, door, { name: 'renamed', token: refOf('v2') })); + const none = await outcomeOf(() => writeVault(engine, door, { name: 'renamed', token: 'secret:sec_none' })); + expect(own).toBe(other); + expect(own).toBe(none); + expect(JSON.parse(own)).toMatchObject({ status: 400, code: 'VALIDATION_ERROR' }); + expect([...storeFor(VAULT).values()]).toEqual(before); + }); + + it.each(DOORS)('%s — control: a new value is sealed, and the echoed mask leaves the column alone', async (door) => { + const { engine, storeFor, refOf } = await rig(readable); + const held = refOf('v1'); + await writeVault(engine, door, { name: 'masked', token: SECRET_MASK }); + expect(storeFor(VAULT).get('v1')).toMatchObject({ name: 'masked', token: held }); + await writeVault(engine, door, { token: 'v1-next' }); + expect(refOf('v1')).toMatch(/^secret:/); + expect(refOf('v1')).not.toBe(held); + expect(await engine.resolveSecretField(VAULT, 'v1', 'token')).toBe('v1-next'); + }); + }); + + it.each(DOORS)('%s — control: a system write keeps the pass-through', async (door) => { + const { engine, refOf } = await rig(false); + await writeVault(engine, door, { token: refOf('v2') }, SYS_CTX); + expect(refOf('v1')).toBe(refOf('v2')); + }); +}); diff --git a/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts b/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts index 8371108c030..7fe3bb03310 100644 --- a/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts +++ b/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts @@ -1,7 +1,9 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // // [#27582] A non-system write may only seal a new value or keep its own -// record's ref — pinned at the REST data door of a booted stack with a crypto +// record's ref, and its own is the value its caller reads in that column — a +// `secret` column reads masked, so no ref is one it keeps. Pinned at the REST +// data door of a booted stack with a crypto // provider wired (`bootStack` wires one LocalCryptoProvider, as // `objectstack dev` / `serve` do). Every HTTP request is assembled as a // non-system caller. `packages/objectql/src/secret-field-own-ref-write.test.ts` @@ -10,10 +12,11 @@ // // - a create whose `secret` field holds a ref is refused (`VALIDATION_ERROR` / // 400), on the single-record door, the bulk door and the import door; -// - an update that keeps the record's own value is accepted and changes -// nothing — the read mask the form echoes, and the record's own ref; -// - an update naming any other ref is refused with one answer, whatever that -// ref names, and the stored value stays; +// - an update that echoes the read mask the form shows is accepted and +// changes nothing; +// - an update naming any ref is refused with one answer, whatever that ref +// names and whatever the column holds (the record's own ref included), and +// the stored value stays; // - the system principal keeps the pass-through. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -42,7 +45,7 @@ const sorStack = defineStack({ type Answer = { status: number; json: any; text: string }; -describe('[#27582] secret field: a non-system write may only seal a new value or keep its own record\'s ref — over REST', () => { +describe('[#27582] secret field: a non-system write may only seal a new value, never send a ref — over REST', () => { let stack: VerifyStack; let ql: any; let token = ''; @@ -109,7 +112,7 @@ describe('[#27582] secret field: a non-system write may only seal a new value or expect(await stack.rows(VAULT, { name: 'sor-import' })).toHaveLength(0); }); - it('an update that echoes the read mask, or the record\'s own ref, is accepted and changes nothing', async () => { + it('an update that echoes the read mask is accepted and changes nothing', async () => { const id = await create('sor-own', 'sor-own-value'); const own = String(await storedRef(id)); const before = await secretRowCount(); @@ -120,16 +123,13 @@ describe('[#27582] secret field: a non-system write may only seal a new value or const masked = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-own-renamed', token: echoed }); expect(masked.status, masked.text).toBe(200); - const kept = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-own-again', token: own }); - expect(kept.status, kept.text).toBe(200); - expect(await storedRef(id)).toBe(own); expect(await secretRowCount()).toBe(before); expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sor-own-value'); - expect((await stack.rows(VAULT, { id }))[0].name).toBe('sor-own-again'); + expect((await stack.rows(VAULT, { id }))[0].name).toBe('sor-own-renamed'); }); - it('an update naming any other ref is refused with one answer, and the stored value stays', async () => { + it('an update naming any ref — the record\'s own included — is refused with one answer, and the stored value stays', async () => { const id = await create('sor-keep', 'sor-keep-value'); const own = String(await storedRef(id)); const before = await secretRowCount(); @@ -142,6 +142,9 @@ describe('[#27582] secret field: a non-system write may only seal a new value or const unknown = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-keep-renamed', token: unknownRef }); expect(answerOf(unknown)).toEqual(answerOf(named)); + const kept = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-keep-renamed', token: own }); + expect(answerOf(kept)).toEqual(answerOf(named)); + expect(await storedRef(id)).toBe(own); expect(await secretRowCount()).toBe(before); expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sor-keep-value'); From f49cf38154f4e1964424902df99528fda32d04f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 22:28:24 +0000 Subject: [PATCH 25/26] fix(objectql): the update preview binds a prior header the caller is not served as the write binds it When the patch names the master-detail FK over a stored FK the caller's read door did not serve as stored, the preview binds the prior header as unresolved, as the write does, so preview and write give one verdict. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../src/engine-master-detail-related.ts | 17 ++++++- packages/objectql/src/engine-validate.ts | 4 +- .../src/validate-preview-parent.test.ts | 47 +++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/packages/objectql/src/engine-master-detail-related.ts b/packages/objectql/src/engine-master-detail-related.ts index b0922ad75a0..609f4c40dee 100644 --- a/packages/objectql/src/engine-master-detail-related.ts +++ b/packages/objectql/src/engine-master-detail-related.ts @@ -254,6 +254,14 @@ export async function settleMasterDetailLanding( * 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 @@ -296,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(); landingIds.forEach((id) => { if (id != null) ids.add(String(id)); }); priorIds.forEach((id, i) => { if (repoints[i]) ids.add(String(id)); }); @@ -340,7 +355,7 @@ 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)), }; } diff --git a/packages/objectql/src/engine-validate.ts b/packages/objectql/src/engine-validate.ts index 4f77b46dd85..84a7cdf7b3e 100644 --- a/packages/objectql/src/engine-validate.ts +++ b/packages/objectql/src/engine-validate.ts @@ -442,7 +442,9 @@ export async function validate( // payload-FK-first reading (`masterIdOf`): the row's own FK on an insert, // and on an update the patch's FK, else the stored row's. A repointing // update also binds the header the stored row hangs off as - // `previousParent`, the header the write's ADR-0113 pre-check reads. + // `previousParent`, the header the write's ADR-0113 pre-check reads — and, + // as the write binds it, the unresolved header (`null`) when the patch + // names the FK over a stored FK this caller was not served as stored. // // ⛔ Read under the caller's access, as the write reads it since #22519: // see {@link resolvePreviewParents}. A header the caller cannot read binds diff --git a/packages/objectql/src/validate-preview-parent.test.ts b/packages/objectql/src/validate-preview-parent.test.ts index c69478c8865..983c0dbbd83 100644 --- a/packages/objectql/src/validate-preview-parent.test.ts +++ b/packages/objectql/src/validate-preview-parent.test.ts @@ -42,6 +42,10 @@ // writes in `engine.ts`, the preview in `engine-validate.ts`). // // (a), (b) and (e) were red before the fix. +// +// A later describe pins the repoint over a stored FK the caller is not served +// as stored: the preview binds the prior header as unresolved, as the write +// does, so both give one verdict whatever header the row hung off. import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { readFileSync } from 'node:fs'; @@ -382,6 +386,49 @@ describe('#22474 — the preview binds the master-detail header the write binds' }); }); +describe('a repoint over a stored FK the caller is not served: the preview binds the prior header as the write does', () => { + /** + * A field-level read rule on the line's `invoice` FK for one caller, shaped + * like the security middleware's result mask (`hide` deletes the key). The + * write's projection of the stored FK and the preview's stored-row read both + * go through the read door, so both meet it. + */ + function hideInvoiceFrom(ql: ObjectQL, userId: string) { + ql.registerMiddleware(async (ctx: any, next: () => Promise) => { + await next(); + if (ctx.object !== LINE || ctx.context?.isSystem || ctx.context?.userId !== userId) return; + if (!['find', 'findOne'].includes(ctx.operation)) return; + const apply = (r: any) => { if (r && typeof r === 'object') delete r.invoice; }; + if (Array.isArray(ctx.result)) ctx.result.forEach(apply); + else apply(ctx.result); + }); + } + + /** The preview's verdict and then the write's, for a line lacking its description that hangs off `stored`. */ + async function bothVerdicts(stored: string, context: Record) { + const { engine, storeFor } = makeEngine(); + hideInvoiceFrom(engine, 'frank'); + storeFor(LINE).set('l1', { id: 'l1', name: 'l', invoice: stored, quantity: 1 }); + const patch = { invoice: 'h_sent', id: 'l1' }; + const previewed = await preview(engine, LINE, patch, 'update', context); + const written = await write(() => engine.update(LINE, patch, { context } as any)); + return { previewed, written }; + } + + it('a caller not served the FK gets one verdict whatever header the line hangs off, in the preview and in the write', async () => { + const frank = { userId: 'frank' }; + for (const stored of ['h_sent', 'h_draft']) { + expect(await bothVerdicts(stored, frank)).toEqual({ previewed: REQUIRED, written: REQUIRED }); + } + }); + + it('control: a caller served the FK keeps the legacy rest and the repoint refusal, in both', async () => { + const alice = { userId: 'alice' }; + expect(await bothVerdicts('h_sent', alice)).toEqual({ previewed: ADMITTED, written: ADMITTED }); + expect(await bothVerdicts('h_draft', alice)).toEqual({ previewed: REQUIRED, written: REQUIRED }); + }); +}); + describe('#22474 (e) — the import dry run of a detail row, through the protocol', () => { let kernel: ObjectKernel; let objectql: ObjectQL; From 82a9a6bbdad7b753e92d22009da814e295f8d138 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 22:38:18 +0000 Subject: [PATCH 26/26] docs(changeset): the update preview predicts the repoint pre-check's answer Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc --- .../update-stored-row-caller-read-secret-ref-and-repoint.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md b/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md index cae9f3ce9e1..89e5482aa75 100644 --- a/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md +++ b/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md @@ -13,4 +13,4 @@ Metadata-protocol: none **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. +**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.