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..db6ae60217f --- /dev/null +++ b/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts @@ -0,0 +1,520 @@ +// 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 + * (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 + * `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')]); + 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 } }); + 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. *