From e776122fca8d7af6aa9d3ac0cbcce657969aeb75 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 08:03:51 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(plugin-security)!:=20a=20batch=20create?= =?UTF-8?q?=20of=20a=20controlled=5Fby=5Fparent=20detail=20asks=20for=20ed?= =?UTF-8?q?it=20access=20on=20every=20row's=20master,=20and=20an=20update?= =?UTF-8?q?=20no=20longer=20empties=20the=20master=20reference=20(ADR-0055?= =?UTF-8?q?=20=C2=A71,=20=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The master-detail write gate read a create's master only from a body that is a single record, so a batch create (the engine's array insert and insertMany, and the doors that call them) asked about no master. Every master the rows name is now asked by the one chain walk, once per master, in row order, before anything is written; the empty-master branch then answers the batch as before. An update whose body sets the master reference to null is answered by the empty-master rule: handed to required-field validation where it judges the field, refused 422 otherwise, after the stored masters' question. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- .changeset/25049-cbp-every-row-master.md | 31 ++ ...trolled-by-parent-every-row-master.test.ts | 511 ++++++++++++++++++ .../plugin-security/src/security-plugin.ts | 73 ++- 3 files changed, 607 insertions(+), 8 deletions(-) create mode 100644 .changeset/25049-cbp-every-row-master.md create mode 100644 packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts diff --git a/.changeset/25049-cbp-every-row-master.md b/.changeset/25049-cbp-every-row-master.md new file mode 100644 index 00000000000..75633584d31 --- /dev/null +++ b/.changeset/25049-cbp-every-row-master.md @@ -0,0 +1,31 @@ +--- +'@objectstack/plugin-security': minor +--- + +fix(plugin-security)!: a batch create of a `controlled_by_parent` detail asks for edit access on the master of every row it names, and an update no longer empties a detail's master reference (ADR-0055 §1, §3) + +ADR-0055 §3 says creating, editing or deleting a detail requires edit access to its master, and it reads a create's master from the master id in the incoming body. A batch create carries one body per row. The master-detail write gate read a create's master only from a body that is a single record, so a batch create asked about no master at all. That covers the engine's array insert and `insertMany`, and the bulk-create and import doors that call them. The gate now asks about the master of every row the batch names, with the same check a single create runs, once per master and in row order. One master the caller may not edit refuses the whole batch, and nothing is written. On a write made on a user's behalf, both principals are asked, as on every other detail write. + +ADR-0055 §1 derives a detail's access from exactly one required master. For a master reference that required-field validation does not judge, an update whose body set the reference to null landed. It stored a detail that no principal's read can match. The gate now refuses it as it already refuses a create that names no master. + +Clause-②: no (narrowing) + + + +**BREAKING** for a caller that relied on either write landing. Shipped as `minor` under the launch-window convention for accept-set narrowings. + +- **Before.** + - A batch create of a detail landed whatever masters its rows named, including masters the caller may not edit. + - An update that set a master reference, of a kind required-field validation does not judge, to null landed and stored the null. +- **Now.** + - That batch create answers the refusal a single create of the offending row answers: `PERMISSION_DENIED` (403), naming the detail and the master-edit reason. The partial-success `insertMany` refuses the whole call rather than reporting that row. + - A batch whose rows' masters the caller may all edit lands as before and writes the same rows. + - That update answers `MISSING_REQUIRED_FIELD` (422) and writes nothing. Where required-field validation does judge the reference, it still answers the update, as before. +- **Unchanged.** + - Single creates, and every update and delete that does not empty the master reference. + - A row naming no master, which the same rules as before answer. + - System contexts, the detail's read scope and the platform ownership floor. + - Every object that does not declare `controlled_by_parent`. + - The import door degrades a refused batch to one create per row. There, the rows under masters the caller may edit still land, and the other row is refused on its own. + +**What to do.** Grant the caller edit access on every master a batch names, or keep each batch within masters the caller may edit. To move a detail to another master, write that master's id, never null. diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts new file mode 100644 index 00000000000..60d69515879 --- /dev/null +++ b/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts @@ -0,0 +1,511 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0055 §1 · §3] Every row a write stores in a `controlled_by_parent` + * detail is held to its master: the caller must be able to edit the master of + * EACH row a create names, and a write never empties the master reference the + * detail's access derives from. + * + * §3 reads a create's master from "the master id in the incoming body". A + * batch create carries one body per row, so it involves the master each of its + * rows names. §1 derives a detail's access from exactly one required master, so + * a row whose master reference a write empties has no master to derive its + * access from — no principal's read can match it. + * + * Pinned here: + * + * - a batch create, through every door that hands the engine an array of + * rows (its array insert and partial-success `insertMany`, and the + * protocol's `createManyData` and `insertManyData` that the bulk-create and + * import doors call), is refused when ONE row names a master the caller may + * not edit, with the refusal a single create of that row gets, and nothing + * is written: on the row-level leg for a member, an org admin and an agent + * acting for a member, and on a master's own master for a member and an org + * admin; + * - a batch create whose rows' masters the caller may all edit lands as it + * did, each row under the master it names; + * - each master a batch names is asked once, in row order; + * - a row naming no master, under a reference required-field validation + * judges, is answered by validation as before; + * - under a reference required-field validation does not judge, a batch + * create naming a master the caller may not edit is refused on that master, + * and every other batch create is still refused whole, as before; + * - an update that empties the master reference, under a reference + * required-field validation does not judge, is refused — addressed by + * `where`, in its body and by predicate — with the empty-master refusal, and + * nothing is written; under a reference validation judges, validation's + * answer is unchanged; and an update that leaves the reference out lands as + * it did. + * + * Real stack: `ObjectQL` over better-sqlite3, the real `SecurityPlugin`, the + * real sharing service and middleware, the real protocol, and contexts + * assembled by `resolveAuthzContext` and `assembleExecutionContext` — the + * agent's with the `mcp_agent_data_write` OAuth ceiling. `@objectstack/core` and + * `@objectstack/plugin-sharing` resolve through their built `dist/`, as this + * package's other suites read them. + */ + +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import { AsyncLocalStorage } from 'node:async_hooks'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { resolveAuthzContext, assembleExecutionContext } from '@objectstack/core'; +import { SharingService, buildSharingMiddleware, SysRecordShare } from '@objectstack/plugin-sharing'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity'; + +import { SysPosition, SysPermissionSet, SysPositionPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js'; +import { SysUserPosition } from './objects/sys-user-position.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +const SYS = { context: { isSystem: true } } as never; +const ORG = 'org_cbp_erm'; +const POSTURE = 'isolated' as const; + +/** Holds the org's `member` role, so the platform ownership floor binds it. */ +const MEMBER = 'u_cbp_erm_member'; +/** Holds the org's `admin` role, outside the floor's domain. */ +const ADMIN = 'u_cbp_erm_admin'; +/** Owns the masters. */ +const OTHER = 'u_cbp_erm_other'; + +/** A master whose own row-level policy decides, and a master whose own master decides. */ +const M = { rls: 'qa_cbp_erm_master_rls', chain: 'qa_cbp_erm_master_chain' } as const; +/** The master of `M.chain`. */ +const TOP = 'qa_cbp_erm_top'; +type Leg = keyof typeof M; +const D = { rls: 'qa_cbp_erm_detail_rls', chain: 'qa_cbp_erm_detail_chain' } as const; +/** + * A detail under `M.rls` whose master reference required-field validation does + * not judge (`validateRecord` skips a provenance-flagged field), so the gate is + * the one thing that holds the reference populated. + */ +const UNJUDGED = 'qa_cbp_erm_detail_unjudged'; + +/** On every leg: the master row the caller may not edit, and two it may. */ +const held = (leg: Leg) => `${M[leg]}_held`; +const free = (leg: Leg, n: 1 | 2 = 1) => `${M[leg]}_free${n}`; + +type Outcome = + | { kind: 'landed'; value: unknown } + | { kind: 'refused'; code?: string; status?: number; message: string }; + +const outcomeOf = (p: Promise): Promise => + p.then( + (value) => ({ kind: 'landed' as const, value }), + (e: unknown) => { + const x = e as { code?: unknown; status?: unknown; statusCode?: unknown; message?: unknown } | null; + const status = x?.statusCode ?? x?.status; + return { + kind: 'refused' as const, + code: x?.code == null ? undefined : String(x.code), + status: status == null ? undefined : Number(status), + message: String(x?.message), + }; + }, + ); + +/** The refusal as the envelope it is: code, status and the refusal itself. */ +const envelope = (o: Outcome) => (o.kind === 'refused' ? { code: o.code, status: o.status, message: o.message } : o); + +/** See the explain-enforce-parity suite: a torn-down rig's async storages are released once its last row is judged. */ +const releaseAsyncContexts = (...owners: object[]): void => { + for (const owner of owners) { + for (const value of Object.values(owner)) if (value instanceof AsyncLocalStorage) value.disable(); + } +}; + +const masterFields = { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + owner_id: { name: 'owner_id', type: 'text' }, + created_by: { name: 'created_by', type: 'text' }, +}; +const detailOf = (name: string, master: string, fk = 'master', flags: Record = {}) => ({ + name, + label: name, + sharingModel: 'controlled_by_parent', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + note: { name: 'note', type: 'text' }, + name: { name: 'name', type: 'text' }, + owner_id: { name: 'owner_id', type: 'text' }, + created_by: { name: 'created_by', type: 'text' }, + [fk]: { name: fk, type: 'master_detail', required: true, reference: master, ...flags }, + }, +}); + +async function boot() { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as never, + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.cbp-every-row-master', + name: 'Every row a write stores in a controlled_by_parent detail is held to its master', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPosition, SysUserPermissionSet, + SysOrganization, SysUser, SysMember, SysRecordShare, + { name: M.rls, label: 'Master', sharingModel: 'public_read_write', fields: masterFields }, + { name: TOP, label: 'Top', sharingModel: 'public_read_write', fields: masterFields }, + { ...detailOf(M.chain, TOP, 'top'), label: 'Master' }, + detailOf(D.rls, M.rls), + detailOf(D.chain, M.chain), + detailOf(UNJUDGED, M.rls, 'master', { system: true }), + ], + } as never); + await engine.syncSchemas(); + + const e = engine as unknown as { + insert: (o: string, row: Record, opts: never) => Promise; + getDriver: (o: string) => { create: (o: string, row: Record) => Promise }; + logger: { warn: () => void }; + }; + await e.insert('sys_organization', { id: ORG, name: ORG, slug: ORG }, SYS); + for (const [id, role] of [[MEMBER, 'member'], [ADMIN, 'admin'], [OTHER, 'member']] as const) { + await e.insert('sys_user', { id, name: id, email: `${id}@example.test` }, SYS); + await e.insert('sys_member', { id: `mem_${id}`, user_id: id, organization_id: ORG, role }, SYS); + } + const CRUD = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } as const; + const WRITER = 'qa_cbp_erm_writer'; + const heldByName = (name: string, object: string) => + ({ name, object, operation: 'update', using: "name != 'held'" }) as const; + const sets = [ + ...defaultPermissionSets, + PermissionSetSchema.parse({ + name: WRITER, + objects: { + [M.rls]: { allowRead: true, allowEdit: true }, + [TOP]: { allowRead: true, allowEdit: true }, + [M.chain]: CRUD, + [D.rls]: CRUD, + [D.chain]: CRUD, + [UNJUDGED]: CRUD, + }, + rowLevelSecurity: [heldByName('qa_cbp_erm_master_updates', M.rls), heldByName('qa_cbp_erm_top_updates', TOP)], + }), + ]; + await e.getDriver('sys_permission_set').create('sys_permission_set', { + id: `ps_${WRITER}`, name: WRITER, label: WRITER, organization_id: null, managed_by: 'admin', active: true, + object_permissions: JSON.stringify({}), system_permissions: JSON.stringify([]), + }); + for (const user of [MEMBER, ADMIN]) { + await e.insert('sys_user_permission_set', { id: `ups_${user}`, user_id: user, permission_set: WRITER, organization_id: ORG }, SYS); + } + const master = (object: string, id: string, name: string, extra: Record = {}) => + e.insert(object, { id, name, owner_id: OTHER, created_by: OTHER, organization_id: ORG, ...extra }, SYS); + await master(M.rls, held('rls'), 'held'); + await master(M.rls, free('rls', 1), 'free'); + await master(M.rls, free('rls', 2), 'free'); + await master(TOP, `${TOP}_held`, 'held'); + await master(TOP, `${TOP}_free`, 'free'); + // The middle masters carry no rule of their own: the top each sits under decides. + await master(M.chain, held('chain'), 'held', { top: `${TOP}_held` }); + await master(M.chain, free('chain', 1), 'free', { top: `${TOP}_free` }); + await master(M.chain, free('chain', 2), 'free', { top: `${TOP}_free` }); + + let security: unknown; + let sharing: SharingService | undefined; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (type: string, name: string) => + type === 'permission' ? sets.find((s: { name?: string }) => s.name === name) ?? null : engine.getSchema(name) ?? null, + list: async () => sets, + }, + tenancy: { posture: POSTURE }, + get sharing() { return sharing; }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: (name: string, svc: unknown) => { + if (name === 'security') security = svc; + else services[name] = svc; + }, + hook: () => undefined, + 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 never); + await plugin.start(ctx as never); + sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); + engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); + // The row gates log the writes they refuse. + vi.spyOn(e.logger, 'warn').mockImplementation(() => undefined); + + /** The protocol the bulk-create (`createManyData`) and import (`insertManyData`) doors call. */ + const protocol = new ObjectStackProtocolImplementation(engine as never, () => new Map(), undefined) as unknown as { + createManyData(req: { object: string; records: unknown[]; context?: unknown }): Promise; + insertManyData(req: { object: string; records: unknown[]; context?: unknown }): Promise; + }; + + /** The caller's context as a request assembles it: the user's session, and for an agent its OAuth ceiling. */ + const contextOf = async (userId: string, onBehalf: boolean): Promise> => { + const authz = await resolveAuthzContext({ + userAuthGates: undefined, + apiKeyLifecycle: undefined, + ql: engine, + headers: {}, + getSession: async () => ({ + user: { id: userId }, + session: { id: `sess_${userId}`, userId, activeOrganizationId: ORG }, + }), + tenancyPosture: POSTURE, + } as never); + return { + ...(assembleExecutionContext({ + authz, + oauth: onBehalf + ? { userId, scopes: ['data:write'], clientId: 'qa_agent_client', scopePermissions: ['mcp_agent_data_write'], delegatesActions: false } + : undefined, + localization: undefined, + requestLocale: undefined, + accessToken: undefined, + } as never) as Record), + }; + }; + + let n = 0; + return { + /** The master-edit walk the gate runs once per master it asks about, for counting. */ + walk: () => + vi.spyOn(plugin as unknown as { assertControlledByParentChainEditable: () => Promise }, 'assertControlledByParentChainEditable'), + contextOf, + /** Fresh row ids, one per master given, each row naming that master. */ + rows: (masters: readonly (string | undefined)[]) => { + const tag = `t${++n}`; + return masters.map((m, i) => ({ id: `${tag}_${i}`, note: tag, ...(m === undefined ? {} : { master: m }) })); + }, + /** + * A row written as SYSTEM under `under`, created by `creator`: a selecting + * write by a floor-bound caller reaches only the rows it created. + */ + seed: async (detail: string, under: string, creator: string) => { + const id = `s${++n}`; + await e.getDriver(detail).create(detail, { id, note: 'seed', created_by: creator, master: under, organization_id: ORG }); + return id; + }, + /** Each row as stored — its master and its note — or `null` when it is absent. */ + stored: async (detail: string, ids: readonly string[]) => { + const out: Record = {}; + for (const id of ids) { + const row = (await engine.findOne(detail, { where: { id }, context: { isSystem: true } } as never)) as + | Record + | null; + out[id] = row ? { master: row.master, note: row.note } : null; + } + return out; + }, + insert: (detail: string, row: Record, context: Record) => + outcomeOf(engine.insert(detail, row, { context: { ...context } } as never)), + /** The doors that hand the engine an array of rows. */ + batch: { + 'the engine array insert': (detail: string, rows: Record[], context: Record) => + outcomeOf(engine.insert(detail, rows, { context: { ...context } } as never)), + 'the engine insertMany': (detail: string, rows: Record[], context: Record) => + outcomeOf((engine as unknown as { insertMany: (o: string, r: unknown[], x: never) => Promise }).insertMany(detail, rows, { context: { ...context } } as never)), + 'the protocol createManyData': (detail: string, rows: Record[], context: Record) => + outcomeOf(protocol.createManyData({ object: detail, records: rows, context: { ...context } })), + 'the protocol insertManyData': (detail: string, rows: Record[], context: Record) => + outcomeOf(protocol.insertManyData({ object: detail, records: rows, context: { ...context } })), + }, + /** The three update shapes, each writing `data` to the one row `id` names. */ + update: { + 'addressed by where': (detail: string, id: string, data: Record, context: Record) => + outcomeOf(engine.update(detail, data, { where: { id }, context: { ...context } } as never)), + 'addressed in its body': (detail: string, id: string, data: Record, context: Record) => + outcomeOf(engine.update(detail, { id, ...data }, { context: { ...context } } as never)), + selecting: (detail: string, id: string, data: Record, context: Record) => + outcomeOf(engine.update(detail, data, { where: { id: { $in: [id] } }, multi: true, context: { ...context } } as never)), + }, + teardown: async () => { + try { await engine.destroy(); } catch { /* noop */ } + releaseAsyncContexts(engine, plugin); + }, + }; +} + +type Rig = Awaited>; +type BatchDoor = keyof Rig['batch']; +const DOORS: readonly BatchDoor[] = ['the engine array insert', 'the engine insertMany', 'the protocol createManyData', 'the protocol insertManyData']; +type UpdateShape = keyof Rig['update']; +const UPDATES: readonly UpdateShape[] = ['addressed by where', 'addressed in its body', 'selecting']; + +const CALLERS = { + 'a member': { user: MEMBER, onBehalf: false }, + 'an org admin': { user: ADMIN, onBehalf: false }, + 'an agent acting for a member': { user: MEMBER, onBehalf: true }, +} as const; +type Caller = keyof typeof CALLERS; + +const LEGS: ReadonlyArray<{ leg: Leg; name: string; callers: readonly Caller[] }> = [ + { leg: 'rls', name: "the master's own row-level policy", callers: ['a member', 'an org admin', 'an agent acting for a member'] }, + { leg: 'chain', name: "the master's own master", callers: ['a member', 'an org admin'] }, +]; + +const ids = (rows: readonly { id: string }[]) => rows.map((r) => r.id); + +describe('ADR-0055 §1 · §3 — every row a write stores in a controlled_by_parent detail is held to its master', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(); }); + afterAll(async () => { await rig.teardown(); }); + + describe('a batch create naming one master the caller may not edit is refused as a single create of that row is, and writes nothing', () => { + for (const { leg, name, callers } of LEGS) { + for (const caller of callers) { + it(`${name}, ${caller}: through every door that hands the engine an array of rows`, async () => { + const { user, onBehalf } = CALLERS[caller]; + const context = await rig.contextOf(user, onBehalf); + // The precondition: a single create of that row is refused on the master-edit question. + const [single] = rig.rows([held(leg)]); + const refusal = await rig.insert(D[leg], single!, context); + expect(refusal).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + expect(await rig.stored(D[leg], [single!.id])).toEqual({ [single!.id]: null }); + + for (const door of DOORS) { + for (const masters of [[held(leg)], [free(leg), held(leg)], [held(leg), free(leg)]]) { + const rows = rig.rows(masters); + const outcome = await rig.batch[door](D[leg], rows, context); + expect({ door, masters, outcome: envelope(outcome) }).toEqual({ door, masters, outcome: envelope(refusal) }); + const after = await rig.stored(D[leg], ids(rows)); + for (const id of ids(rows)) expect({ door, id, row: after[id] }).toEqual({ door, id, row: null }); + } + } + }); + } + } + }); + + describe('a batch create whose rows’ masters the caller may all edit lands as it did', () => { + for (const { leg, name, callers } of LEGS) { + for (const caller of callers) { + it(`${name}, ${caller}: each row under the master it names`, async () => { + const { user, onBehalf } = CALLERS[caller]; + const context = await rig.contextOf(user, onBehalf); + for (const door of DOORS) { + const rows = rig.rows([free(leg, 1), free(leg, 2), free(leg, 1)]); + const outcome = await rig.batch[door](D[leg], rows, context); + expect({ door, landed: outcome.kind }).toEqual({ door, landed: 'landed' }); + const after = await rig.stored(D[leg], ids(rows)); + for (const row of rows) { + expect({ door, id: row.id, row: after[row.id] }).toEqual({ door, id: row.id, row: { master: row.master, note: row.note } }); + } + } + }); + } + } + }); + + it('each master a batch names is asked once, in row order, and a single create is asked once', async () => { + const context = await rig.contextOf(ADMIN, false); + const walk = rig.walk(); + try { + const count = async (p: Promise) => { + walk.mockClear(); + const outcome = await p; + return { landed: outcome.kind, asked: walk.mock.calls.map((c) => (c as unknown[])[3]) }; + }; + const [f1, f2, h] = [free('rls', 1), free('rls', 2), held('rls')]; + expect(await count(rig.insert(D.rls, rig.rows([f1])[0]!, context))).toEqual({ landed: 'landed', asked: [f1] }); + for (const door of DOORS) { + expect({ door, ...(await count(rig.batch[door](D.rls, rig.rows([f2, f1, f2, f1]), context))) }) + .toEqual({ door, landed: 'landed', asked: [f2, f1] }); + // The first master the caller may not edit is the answer; no master after it is asked. + expect({ door, ...(await count(rig.batch[door](D.rls, rig.rows([f1, h, f2]), context))) }) + .toEqual({ door, landed: 'refused', asked: [f1, h] }); + } + } finally { + walk.mockRestore(); + } + }); + + it('a row naming no master, under a reference validation judges, is answered by validation as before', async () => { + const context = await rig.contextOf(MEMBER, false); + const single = await rig.insert(D.rls, rig.rows([undefined])[0]!, context); + expect(single).toMatchObject({ kind: 'refused', code: 'VALIDATION_FAILED' }); + const rows = rig.rows([free('rls'), undefined]); + const batch = await rig.batch['the engine array insert'](D.rls, rows, context); + expect(batch).toMatchObject({ kind: 'refused', code: 'VALIDATION_FAILED' }); + expect(await rig.stored(D.rls, ids(rows))).toEqual(Object.fromEntries(ids(rows).map((id) => [id, null]))); + // The partial-success door answers that row alone, and lands the row whose master the caller may edit. + const partial = rig.rows([free('rls'), undefined]); + const outcome = await rig.batch['the engine insertMany'](D.rls, partial, context); + expect(outcome.kind).toBe('landed'); + const perRow = (outcome as { value: Array<{ ok: boolean; error?: { code?: string } }> }).value; + expect(perRow.map((r) => ({ ok: r.ok, code: r.error?.code }))).toEqual([{ ok: true, code: undefined }, { ok: false, code: 'VALIDATION_FAILED' }]); + expect(await rig.stored(D.rls, ids(partial))).toEqual({ + [partial[0]!.id]: { master: free('rls'), note: partial[0]!.note }, + [partial[1]!.id]: null, + }); + }); + + it('under a reference validation does not judge: a batch create naming a master the caller may not edit is refused on that master, and every other batch create is still refused whole', async () => { + for (const caller of Object.keys(CALLERS) as Caller[]) { + const { user, onBehalf } = CALLERS[caller]; + const context = await rig.contextOf(user, onBehalf); + const refusal = await rig.insert(UNJUDGED, rig.rows([held('rls')])[0]!, context); + expect(refusal).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + for (const door of DOORS) { + const refused = rig.rows([free('rls'), held('rls')]); + expect({ caller, door, outcome: envelope(await rig.batch[door](UNJUDGED, refused, context)) }) + .toEqual({ caller, door, outcome: envelope(refusal) }); + const whole = rig.rows([free('rls'), free('rls', 2)]); + expect({ caller, door, outcome: await rig.batch[door](UNJUDGED, whole, context) }) + .toMatchObject({ caller, door, outcome: { kind: 'refused', code: 'MISSING_REQUIRED_FIELD', status: 422 } }); + const after = await rig.stored(UNJUDGED, [...ids(refused), ...ids(whole)]); + for (const [id, row] of Object.entries(after)) expect({ caller, door, id, row }).toEqual({ caller, door, id, row: null }); + } + } + }); + + describe('an update never empties the master reference a detail derives its access from', () => { + for (const caller of Object.keys(CALLERS) as Caller[]) { + it(`${caller}: under a reference validation does not judge, it is refused on every update shape, and nothing is written`, async () => { + const { user, onBehalf } = CALLERS[caller]; + const context = await rig.contextOf(user, onBehalf); + for (const shape of UPDATES) { + const id = await rig.seed(UNJUDGED, free('rls'), user); + const outcome = await rig.update[shape](UNJUDGED, id, { master: null, note: 'written' }, context); + expect({ shape, outcome }).toMatchObject({ shape, outcome: { kind: 'refused', code: 'MISSING_REQUIRED_FIELD', status: 422 } }); + expect({ shape, row: (await rig.stored(UNJUDGED, [id]))[id] }).toEqual({ shape, row: { master: free('rls'), note: 'seed' } }); + // The control: the same update leaving the reference out lands. + const control = await rig.update[shape](UNJUDGED, id, { note: 'written' }, context); + expect({ shape, landed: control.kind }).toEqual({ shape, landed: 'landed' }); + expect({ shape, row: (await rig.stored(UNJUDGED, [id]))[id] }).toEqual({ shape, row: { master: free('rls'), note: 'written' } }); + } + }); + } + + it('under a reference validation judges, validation answers it as before', async () => { + const context = await rig.contextOf(MEMBER, false); + for (const shape of UPDATES) { + const id = await rig.seed(D.rls, free('rls'), MEMBER); + const outcome = await rig.update[shape](D.rls, id, { master: null, note: 'written' }, context); + expect({ shape, outcome }).toMatchObject({ shape, outcome: { kind: 'refused', code: 'VALIDATION_FAILED' } }); + expect({ shape, row: (await rig.stored(D.rls, [id]))[id] }).toEqual({ shape, row: { master: free('rls'), note: 'seed' } }); + } + }); + + it('the master the row is stored under is asked first: one the caller may not edit refuses on it', async () => { + const context = await rig.contextOf(MEMBER, false); + for (const shape of UPDATES) { + const id = await rig.seed(UNJUDGED, held('rls'), MEMBER); + const outcome = await rig.update[shape](UNJUDGED, id, { master: null }, context); + expect({ shape, outcome }).toMatchObject({ shape, outcome: { kind: 'refused', code: 'PERMISSION_DENIED', status: 403 } }); + expect({ shape, row: (await rig.stored(UNJUDGED, [id]))[id] }).toEqual({ shape, row: { master: held('rls'), note: 'seed' } }); + } + }); + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index c55ac28bdeb..878d500975d 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -9171,6 +9171,24 @@ export class SecurityPlugin implements Plugin { * asked is not asked again; and a predicate write that selects no row the * caller can read involves no master at all. * + * [#25049] …a BATCH create's included. Its body is one record per row and + * each row names its own master, so the same walk is asked of every master + * the rows name — once per master, in row order — before anything is + * written. The first refusal is the answer, in the by-id leg's own envelope, + * and nothing is written. The empty-master branch then answers the batch + * exactly as it did before, since it reads no master off an array: it + * stands down where required-field validation judges each row that names + * none, and refuses the batch whole otherwise. + * + * [#25049] And a write never EMPTIES the master reference. §1 derives a + * detail's access from exactly one required master, so a row whose + * reference an update sets to null has no master to derive it from, and no + * principal's read can match it. That is the empty-master condition below, + * met on an update's body, and it is answered the same way: handed to + * required-field validation where validation judges the same field, and + * refused `422` otherwise — after the stored masters' question, so a master + * the caller may not edit is still the answer there. + * * [#7474] SIX conditions refuse a write here, and they are NOT one verdict. * Three are genuine authorization answers (no object-level `update` on the * master, the master row outside the caller's write RLS, no `edit`-level @@ -9184,8 +9202,8 @@ export class SecurityPlugin implements Plugin { * INVALID_METADATA` ({@link MasterDetailRelationMissingError}) * - the target row does not exist → `404 RECORD_NOT_FOUND` * ({@link DetailRecordNotFoundError}) - * - the detail's master reference is empty → `422 MISSING_REQUIRED_FIELD` - * ({@link MasterReferenceMissingError}) + * - the detail's master reference is empty, or an update's body empties + * it → `422 MISSING_REQUIRED_FIELD` ({@link MasterReferenceMissingError}) * * [#8688] The last of those three has since NARROWED, by the maintainer * ruling of 2026-08-15: on an `insert` whose master FK is absent, this gate @@ -9195,6 +9213,8 @@ export class SecurityPlugin implements Plugin { * platform's ordinary `400 VALIDATION_FAILED` with `fields[]` instead. It * keeps answering `422` for the three shapes validation does not cover, and * for the stored-row shape on any other by-id write. See the branch itself. + * An update whose body empties the reference is the insert shape's twin and + * is answered by the same rule (#25049). * * [#7505] A seventh outcome is not a leg of this gate at all: if the store * cannot be read, the engine's own error propagates (typically @@ -9258,15 +9278,28 @@ export class SecurityPlugin implements Plugin { // so once the masters `judged` (the stored ones) have been asked, the SAME // walk is asked of the body's — unless it is one of them, which would ask // one master twice. Read as an OWN property, never through the prototype - // chain. A body that names no master (no such key, or a null) leaves this - // question nothing to ask about; that is not a master-edit verdict, and - // nothing here answers it. + // chain. A body without the key names no master and leaves this question + // nothing to ask about. + // + // [ADR-0055 §1 · #25049] A body WITH the key and a null value names no + // master either, but it is not silent: on an update it empties the + // reference the row's access derives from. That is the empty-master + // condition the branch below answers for an insert, answered by the same + // rule — handed to required-field validation where it provably judges the + // same field ({@link CbpRelation.omissionRefusedByValidation}: its update + // pass refuses a required field the body sets to null), and refused here + // otherwise, before anything is written. An insert never reaches here + // with an empty master: the branch below has answered it. const askBodyNamedMaster = async (judged: readonly unknown[]): Promise => { const data = opCtx.data; if (data == null || typeof data !== 'object' || Array.isArray(data)) return; if (!Object.prototype.hasOwnProperty.call(data, rel.fk)) return; const named = (data as Record)[rel.fk]; - if (named == null || judged.includes(named)) return; + if (named == null) { + if (operation !== 'update' || rel.omissionRefusedByValidation) return; + throw new MasterReferenceMissingError(object, operation, rel.fk); + } + if (judged.includes(named)) return; await this.assertControlledByParentChainEditable( permissionSets, object, rel, named, context, opCtx, masterEditDenied, denyMasterEdit, ); @@ -9278,6 +9311,26 @@ export class SecurityPlugin implements Plugin { let detailRecordId: unknown; if (operation === 'insert') { const data = opCtx.data; + if (Array.isArray(data)) { + // [ADR-0055 §3 · #25049] A BATCH create: one body per row, and the + // middleware chain runs once for all of them, so every master the rows + // name is asked here — the same walk a single create runs, once per + // master, in row order — before anything is written. A row is read + // exactly as a single create's body is read below. A row that names no + // master is left to the empty-master branch, which this batch then + // meets exactly as it did before (`masterId` stays undefined for an + // array): it stands down where required-field validation judges each + // such row, and refuses the batch whole otherwise. + const asked: unknown[] = []; + for (const row of data as unknown[]) { + const rowMasterId = row && typeof row === 'object' && !Array.isArray(row) ? (row as any)[rel.fk] : undefined; + if (rowMasterId == null || asked.includes(rowMasterId)) continue; + asked.push(rowMasterId); + await this.assertControlledByParentChainEditable( + permissionSets, object, rel, rowMasterId, context, opCtx, masterEditDenied, denyMasterEdit, + ); + } + } masterId = data && typeof data === 'object' && !Array.isArray(data) ? (data as any)[rel.fk] : undefined; } else { const targetId = this.extractSingleId(opCtx); @@ -9426,6 +9479,9 @@ export class SecurityPlugin implements Plugin { // whose persisted FK is null) keeps its 422: the caller sent no such // field, so a `fields[]` naming it would name a field that was never in // the request, and there is no payload the caller could send to fix it. + // [#25049] An update whose BODY empties the reference is the insert + // shape's twin — the caller did send the field — and is answered by the + // same rule, in `askBodyNamedMaster` above. if (operation === 'insert' && rel.omissionRefusedByValidation) return; throw new MasterReferenceMissingError(object, operation, rel.fk, detailRecordId); } @@ -9449,8 +9505,9 @@ export class SecurityPlugin implements Plugin { * ⚠️ Extraction, not a rewrite — the body is the walk the gate ran inline, * moved verbatim so that the gate can run it once per master: once for an * insert or a by-id write, once for each master a predicate write selects - * rows under ({@link controlledByParentPredicateWriteMasters}), and once more - * for a master the write's body names that is not among those (#25044). + * rows under ({@link controlledByParentPredicateWriteMasters}), once more + * for a master the write's body names that is not among those (#25044), and + * once for each master a batch create's rows name (#25049). * There must stay exactly one walk, for the reason * {@link assertMasterRowEditable} gives for its legs. * From 1b27f979b4218c5c6d31aa4453ff5dd8e2dacb37 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 08:09:49 +0000 Subject: [PATCH 2/2] test(plugin-security): on a user's behalf the agent's own pass answers an unjudged-reference batch first Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- .../controlled-by-parent-every-row-master.test.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts index 60d69515879..db6ae60217f 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts @@ -28,7 +28,8 @@ * - a row naming no master, under a reference required-field validation * judges, is answered by validation as before; * - under a reference required-field validation does not judge, a batch - * create naming a master the caller may not edit is refused on that master, + * create naming a master the caller may not edit is refused on that master + * (for an agent, whose own pass runs first, on the empty-master answer), * and every other batch create is still refused whole, as before; * - an update that empties the master reference, under a reference * required-field validation does not judge, is refused — addressed by @@ -459,8 +460,16 @@ describe('ADR-0055 §1 · §3 — every row a write stores in a controlled_by_pa expect(refusal).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); for (const door of DOORS) { const refused = rig.rows([free('rls'), held('rls')]); - expect({ caller, door, outcome: envelope(await rig.batch[door](UNJUDGED, refused, context)) }) - .toEqual({ caller, door, outcome: envelope(refusal) }); + const outcome = await rig.batch[door](UNJUDGED, refused, context); + if (onBehalf) { + // The agent's own pass runs first and whole: its sets may edit both + // masters, so it meets the batch's empty-master answer before the + // member's pass asks the master the member may not edit. Refused all + // the same, and nothing is written. + expect({ caller, door, outcome }).toMatchObject({ caller, door, outcome: { kind: 'refused', code: 'MISSING_REQUIRED_FIELD', status: 422 } }); + } else { + expect({ caller, door, outcome: envelope(outcome) }).toEqual({ caller, door, outcome: envelope(refusal) }); + } const whole = rig.rows([free('rls'), free('rls', 2)]); expect({ caller, door, outcome: await rig.batch[door](UNJUDGED, whole, context) }) .toMatchObject({ caller, door, outcome: { kind: 'refused', code: 'MISSING_REQUIRED_FIELD', status: 422 } });