diff --git a/.changeset/24746-delegated-write-never-exceeds-delegator.md b/.changeset/24746-delegated-write-never-exceeds-delegator.md new file mode 100644 index 00000000000..e68890d2efa --- /dev/null +++ b/.changeset/24746-delegated-write-never-exceeds-delegator.md @@ -0,0 +1,17 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security): an on-behalf-of by-id write never admits a row its delegator's own by-id write refuses (ADR-0090 D10) + +Clause-②: no + +ADR-0090 D10 says a delegated principal, such as an OAuth agent acting for a user, never exceeds its delegator. The by-id write already asked every row gate as the delegator too. But it composed the delegator's row-level write filter differently from the filter the delegator's own direct write of the same row gets. Where an app-authored row-level write policy bounds the delegator, a write on the delegator's behalf could therefore be admitted for a row the delegator's own write is refused. + +Each gate that judges a delegated by-id write now also composes the delegator's filter as the delegator's own direct write gets it, and requires both: + +- the pre-image check of a by-id `update` or `delete`; +- the post-image check of the row an `update` stores; +- the master gate of a `controlled_by_parent` detail write, on the delegator's pass. + +The change only narrows. A write is admitted only where it was admitted before AND the delegator's own write of that row is admitted. Nothing refused before is admitted now: where the delegator's own write admits a row that the delegated write has always refused, that row stays refused. `security.explain`'s record verdict for an on-behalf-of `update` or `delete` composes the same two filters, so it still gives the answer the door gives. Writes that are not delegated are unchanged. diff --git a/packages/plugins/plugin-security/src/delegated-write-never-exceeds-delegator.test.ts b/packages/plugins/plugin-security/src/delegated-write-never-exceeds-delegator.test.ts new file mode 100644 index 00000000000..d87113708fc --- /dev/null +++ b/packages/plugins/plugin-security/src/delegated-write-never-exceeds-delegator.test.ts @@ -0,0 +1,371 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0090 D10] An on-behalf-of by-id write never admits a row its + * delegator's own by-id write refuses. + * + * An OAuth agent acting for a user carries a ceiling of its own and is bounded + * by that user: every row gate a by-id write meets asks the delegator as well. + * The bound pinned here is the one the delegator's OWN direct write sets, at + * each gate that judges a delegated by-id write: + * + * - the by-id `update` and `delete` of a row; + * - the row an `update` would store; + * - a `controlled_by_parent` detail's write, judged through its master. + * + * Every refusal stands beside the delegator's own request as its control, and + * every row-level verdict beside explain's record verdict for the same caller, + * under a walled (`isolated`) and a wall-less (`single`) posture. + * + * The bound only narrows. Two shapes the delegator's own write admits stay + * refused on its behalf, exactly as they were: making the delegated write + * match the delegator's own there would WIDEN it, which is not what this rule + * says, so those rows pin the refusal as unchanged. + * + * Real stack: `ObjectQL` over better-sqlite3, the real `SecurityPlugin`, the + * real sharing service and middleware, 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, afterAll } from 'vitest'; +import { AsyncLocalStorage } from 'node:async_hooks'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { resolveAuthzContext, assembleExecutionContext } from '@objectstack/core'; +import { SharingService, buildSharingMiddleware, SysRecordShare } from '@objectstack/plugin-sharing'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import type { ExplainDecision, TenancyPosture } 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 ALPHA = 'org_alpha'; + +/** A delegator whose own writes app-authored row-level policies bound. */ +const BOUNDED = 'u_bounded_writer'; +/** A delegator with an `org` write grant and no authored policy. */ +const PLAIN = 'u_plain_writer'; +/** Owns and creates the rows nobody else here does. */ +const OTHER = 'u_someone_else'; + +/** An outcome with its ADR-0112 envelope: what a request answered. */ +type Outcome = { kind: 'admitted' } | { kind: 'refused'; code?: string; status?: number }; +const ADMITTED: Outcome = { kind: 'admitted' }; +const DENIED: Outcome = { kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }; + +const outcomeOf = (p: Promise): Promise => + p.then( + () => ADMITTED, + (e: unknown) => { + const x = e as { code?: unknown; status?: unknown; statusCode?: 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), + }; + }, + ); + +/** 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(); + } +}; + +async function boot(posture: TenancyPosture) { + const OBJ = 'qa_bounded_record'; + const MASTER = 'qa_bounded_master'; + const DETAIL = 'qa_bounded_detail'; + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as never, + true, + ); + await engine.init(); + const fields = { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + note: { name: 'note', type: 'text' }, + owner_id: { name: 'owner_id', type: 'text' }, + created_by: { name: 'created_by', type: 'text' }, + }; + engine.registerApp({ + id: `com.objectstack.qa.delegated-write-bound-${posture}`, + name: 'An on-behalf-of write is bounded by its delegator', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPosition, SysUserPermissionSet, + SysOrganization, SysUser, SysMember, SysRecordShare, + { name: OBJ, label: 'Record', sharingModel: 'private', fields }, + { name: MASTER, label: 'Master', sharingModel: 'private', fields }, + { + name: DETAIL, + label: 'Detail', + sharingModel: 'controlled_by_parent', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + note: { name: 'note', type: 'text' }, + created_by: { name: 'created_by', type: 'text' }, + master: { name: 'master', type: 'master_detail', required: true, reference: MASTER }, + }, + }, + ], + } 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: ALPHA, name: ALPHA, slug: ALPHA }, SYS); + for (const id of [BOUNDED, PLAIN, OTHER]) { + 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: ALPHA, role: 'member' }, SYS); + } + const org = { allowRead: true, allowEdit: true, allowDelete: true, readScope: 'org', writeScope: 'org' } as const; + const sets = [ + ...defaultPermissionSets, + PermissionSetSchema.parse({ + name: 'qa_bounded_writer', + objects: { [OBJ]: org, [MASTER]: org, [DETAIL]: { allowRead: true, allowEdit: true, allowDelete: true } }, + rowLevelSecurity: [ + { name: 'qa_bounded_updates', object: OBJ, operation: 'update', using: "name != 'held'" }, + { name: 'qa_bounded_deletes', object: OBJ, operation: 'delete', using: "name != 'held'" }, + { name: 'qa_bounded_master_updates', object: MASTER, operation: 'update', using: "name != 'held'" }, + ], + }), + PermissionSetSchema.parse({ name: 'qa_plain_writer', objects: { [OBJ]: org } }), + ]; + for (const name of ['qa_bounded_writer', 'qa_plain_writer']) { + await e.getDriver('sys_permission_set').create('sys_permission_set', { + id: `ps_${name}`, name, label: name, organization_id: null, managed_by: 'admin', active: true, + object_permissions: JSON.stringify({}), system_permissions: JSON.stringify([]), + }); + } + for (const [user, set] of [[BOUNDED, 'qa_bounded_writer'], [PLAIN, 'qa_plain_writer']] as const) { + await e.insert('sys_user_permission_set', { + id: `ups_${user}`, user_id: user, permission_set: set, organization_id: ALPHA, + }, SYS); + } + const row = (id: string, name: string, owner: string, creator: string) => + ({ id, name, note: '', owner_id: owner, created_by: creator, organization_id: ALPHA }); + for (const r of [ + row('held_1', 'held', BOUNDED, BOUNDED), + row('held_2', 'held', OTHER, BOUNDED), + row('free_1', 'free', BOUNDED, BOUNDED), + row('held_del_direct', 'held', BOUNDED, BOUNDED), + row('held_del_agent', 'held', BOUNDED, BOUNDED), + row('free_del_direct', 'free', BOUNDED, BOUNDED), + row('free_del_agent', 'free', BOUNDED, BOUNDED), + row('store_direct', 'free', BOUNDED, BOUNDED), + row('store_agent', 'free', BOUNDED, BOUNDED), + row('plain_1', 'x', OTHER, OTHER), + row('plain_2', 'x', PLAIN, OTHER), + ]) await e.insert(OBJ, r, SYS); + await e.insert(MASTER, row('m_held', 'held', BOUNDED, BOUNDED), SYS); + await e.insert(MASTER, row('m_free', 'free', BOUNDED, BOUNDED), SYS); + for (const [id, master] of [ + ['d_held', 'm_held'], ['d_free', 'm_free'], ['d_held_del_direct', 'm_held'], ['d_held_del_agent', 'm_held'], + ] as const) { + await e.insert(DETAIL, { id, note: '', created_by: BOUNDED, master, organization_id: ALPHA }, SYS); + } + + let security: { explain: (input: unknown, ctx: unknown) => Promise } | undefined; + 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 }, + 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 as typeof security; + 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 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) => { + const authz = await resolveAuthzContext({ + userAuthGates: undefined, + apiKeyLifecycle: undefined, + ql: engine, + headers: {}, + getSession: async () => ({ + user: { id: userId }, + session: { id: `sess_${userId}_${posture}`, userId, activeOrganizationId: ALPHA }, + }), + tenancyPosture: posture, + }); + 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)!; + }; + const objects = { OBJ, DETAIL } as const; + type Obj = keyof typeof objects; + let n = 0; + + return { + /** Explain's record verdict for the caller's own by-id write of `object/id`. */ + explainVisible: async (userId: string, onBehalf: boolean, operation: 'update' | 'delete', object: Obj, id: string) => + (await security!.explain({ object: objects[object], operation, recordId: id }, { ...(await contextOf(userId, onBehalf)) })) + .record?.visible, + update: async (userId: string, onBehalf: boolean, object: Obj, id: string) => + outcomeOf(engine.update(objects[object], { note: `n_${++n}` }, { + where: { id }, context: { ...(await contextOf(userId, onBehalf)) }, + } as never)), + /** A by-id update whose stored row carries a value the delegator's own update may not store. */ + storeHeld: async (userId: string, onBehalf: boolean, id: string) => + outcomeOf(engine.update(OBJ, { name: 'held' }, { + where: { id }, context: { ...(await contextOf(userId, onBehalf)) }, + } as never)), + remove: async (userId: string, onBehalf: boolean, object: Obj, id: string) => + outcomeOf(engine.delete(objects[object], { where: { id }, context: { ...(await contextOf(userId, onBehalf)) } } as never)), + teardown: async () => { + try { await engine.destroy(); } catch { /* noop */ } + releaseAsyncContexts(engine, plugin); + }, + }; +} + +type Rig = Awaited>; +const rigs = new Map>(); +const rigFor = (posture: TenancyPosture): Promise => { + let rig = rigs.get(posture); + if (!rig) { + rig = boot(posture); + rigs.set(posture, rig); + } + return rig; +}; +afterAll(async () => { + for (const rig of rigs.values()) await (await rig.catch(() => undefined))?.teardown(); +}); + +const word = (o: Outcome): string => (o.kind === 'admitted' ? 'admitted' : `refused ${o.code} / ${o.status}`); + +/** + * One by-id write, asked by the delegator itself and by an agent on its + * behalf. `rows` names the row each asks about (a delete that lands removes + * its row, so each delete asker has its own). + */ +interface Cell { + delegator: string; + verb: 'update' | 'delete'; + object: 'OBJ' | 'DETAIL'; + rows: { direct: string; agent: string }; + shape: string; + direct: Outcome; + agent: Outcome; +} + +const CELLS: Cell[] = [ + { + delegator: BOUNDED, verb: 'update', object: 'OBJ', rows: { direct: 'held_1', agent: 'held_1' }, + shape: 'a row the delegator\'s own update refuses', direct: DENIED, agent: DENIED, + }, + { + delegator: BOUNDED, verb: 'update', object: 'OBJ', rows: { direct: 'held_2', agent: 'held_2' }, + shape: 'a second row the delegator\'s own update refuses', direct: DENIED, agent: DENIED, + }, + { + delegator: BOUNDED, verb: 'update', object: 'OBJ', rows: { direct: 'free_1', agent: 'free_1' }, + shape: 'the control: a row the delegator\'s own update admits', direct: ADMITTED, agent: ADMITTED, + }, + { + delegator: BOUNDED, verb: 'delete', object: 'OBJ', rows: { direct: 'held_del_direct', agent: 'held_del_agent' }, + shape: 'a row the delegator\'s own delete refuses', direct: DENIED, agent: DENIED, + }, + { + delegator: BOUNDED, verb: 'delete', object: 'OBJ', rows: { direct: 'free_del_direct', agent: 'free_del_agent' }, + shape: 'the control: a row the delegator\'s own delete admits', direct: ADMITTED, agent: ADMITTED, + }, + { + delegator: BOUNDED, verb: 'update', object: 'DETAIL', rows: { direct: 'd_held', agent: 'd_held' }, + shape: 'a detail whose master refuses the delegator\'s own detail update', direct: DENIED, agent: DENIED, + }, + { + delegator: BOUNDED, verb: 'delete', object: 'DETAIL', rows: { direct: 'd_held_del_direct', agent: 'd_held_del_agent' }, + shape: 'a detail whose master refuses the delegator\'s own detail delete', direct: DENIED, agent: DENIED, + }, + { + delegator: BOUNDED, verb: 'update', object: 'DETAIL', rows: { direct: 'd_free', agent: 'd_free' }, + shape: 'the control: a detail whose master admits the delegator\'s own detail update', direct: ADMITTED, agent: ADMITTED, + }, + // The bound only narrows: rows the delegator's own write admits and the + // delegated write has always refused stay refused. + { + delegator: PLAIN, verb: 'update', object: 'OBJ', rows: { direct: 'plain_1', agent: 'plain_1' }, + shape: 'unchanged: a row the delegator\'s own update admits and the delegated update refuses', direct: ADMITTED, agent: DENIED, + }, + { + delegator: PLAIN, verb: 'update', object: 'OBJ', rows: { direct: 'plain_2', agent: 'plain_2' }, + shape: 'unchanged: a second row the delegator\'s own update admits and the delegated update refuses', direct: ADMITTED, agent: DENIED, + }, +]; + +describe('an on-behalf-of by-id write never admits a row its delegator\'s own by-id write refuses (ADR-0090 D10)', () => { + for (const posture of ['isolated', 'single'] as const) { + for (const c of CELLS) { + it(`\`${posture}\` · ${c.verb} · ${c.shape} → the delegator's own: ${word(c.direct)}; on its behalf: ${word(c.agent)}`, async () => { + const rig = await rigFor(posture); + const ask = (onBehalf: boolean, id: string) => + c.verb === 'update' ? rig.update(c.delegator, onBehalf, c.object, id) : rig.remove(c.delegator, onBehalf, c.object, id); + // Explain first: a delete that lands removes the row it explains. + const explainedDirect = await rig.explainVisible(c.delegator, false, c.verb, c.object, c.rows.direct); + const explainedAgent = await rig.explainVisible(c.delegator, true, c.verb, c.object, c.rows.agent); + const direct = await ask(false, c.rows.direct); + const agent = await ask(true, c.rows.agent); + expect(direct, 'the delegator\'s own write').toStrictEqual(c.direct); + expect(agent, 'the write on its behalf').toStrictEqual(c.agent); + // Explain's record verdict is the door's, for both callers. + expect(explainedDirect, 'explain, the delegator\'s own write').toBe(c.direct.kind === 'admitted'); + expect(explainedAgent, 'explain, the write on its behalf').toBe(c.agent.kind === 'admitted'); + }, 60_000); + } + + it(`\`${posture}\` · update · a row the delegator's own update may not store → refused on its behalf as for the delegator`, async () => { + const rig = await rigFor(posture); + expect(await rig.storeHeld(BOUNDED, false, 'store_direct'), 'the delegator\'s own write').toStrictEqual(DENIED); + expect(await rig.storeHeld(BOUNDED, true, 'store_agent'), 'the write on its behalf').toStrictEqual(DENIED); + }, 60_000); + } +}); diff --git a/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts b/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts index 05b51fa168e..549dc1d756b 100644 --- a/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts +++ b/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts @@ -751,6 +751,8 @@ const D_ORG = 'u_d_org'; const D_OWN = 'u_d_own'; /** Owns the rows nobody the rig explains owns. */ const D_OTHER = 'u_d_other'; +/** Write depth `org`, its own writes bounded by app-authored row-level policies: the delegator #24746 is about. */ +const D_BOUNDED = 'u_d_bounded'; /** * #24646 ruling item 3: for an ON-BEHALF-OF caller — an OAuth agent acting for @@ -771,7 +773,9 @@ const D_OTHER = 'u_d_other'; * - `p_org_mine_created_by_other`: owned by `D_ORG`, created by someone else; * - `p_theirs`, `p_org_mine`, `p_own_mine`: the controls; * - `p_beta`: another organization's row, which only the wall-less posture lets through; - * - `p_del_*`: the same two shapes again, for the delete rows (a delete that lands removes its row). + * - `p_del_*`: the same two shapes again, for the delete rows (a delete that lands removes its row); + * - `p_bounded_*`, `p_del_bounded_*`, `m_bounded_*`: rows `D_BOUNDED`'s own authored + * policies refuse (`held`) or admit (`free`), and a detail under each master (#24746). * * One rig per posture, shared by its rows: an admitted update writes only * `note`, which no policy reads, and every delete row has its own record. @@ -822,7 +826,7 @@ async function bootDelegated(posture: TenancyPosture) { const e = engine as unknown as { insert: (o: string, row: Record, opts: never) => Promise }; for (const id of [ALPHA, BETA]) await e.insert('sys_organization', { id, name: id, slug: id }, SYS); - for (const id of [D_ORG, D_OWN, D_OTHER]) { + for (const id of [D_ORG, D_OWN, D_OTHER, D_BOUNDED]) { await e.insert('sys_user', { id, name: id, email: `${id}@example.test` }, SYS); await e.insert('sys_member', { id: `mem_${id}_${n}`, user_id: id, organization_id: ALPHA, role: 'member' }, SYS); } @@ -838,15 +842,24 @@ async function bootDelegated(posture: TenancyPosture) { name: 'qa_d_own', objects: { [PARENT]: own, [MASTER]: { ...own, readScope: 'org' }, [DETAIL]: detail }, }), + PermissionSetSchema.parse({ + name: 'qa_d_bounded', + objects: { [PARENT]: org, [MASTER]: org, [DETAIL]: { ...detail, allowDelete: true } }, + rowLevelSecurity: [ + { name: 'qa_d_bounded_updates', object: PARENT, operation: 'update', using: "name != 'held'" }, + { name: 'qa_d_bounded_deletes', object: PARENT, operation: 'delete', using: "name != 'held'" }, + { name: 'qa_d_bounded_master_updates', object: MASTER, operation: 'update', using: "name != 'held'" }, + ], + }), ]; // A catalogue row with no organization, landed through the driver as in rig 2. - for (const name of ['qa_d_org', 'qa_d_own']) { + for (const name of ['qa_d_org', 'qa_d_own', 'qa_d_bounded']) { await (engine as any).getDriver('sys_permission_set').create('sys_permission_set', { id: `ps_${name}_${n}`, name, label: name, organization_id: null, managed_by: 'admin', active: true, object_permissions: JSON.stringify({}), system_permissions: JSON.stringify([]), }); } - for (const [user, set] of [[D_ORG, 'qa_d_org'], [D_OWN, 'qa_d_own']] as const) { + for (const [user, set] of [[D_ORG, 'qa_d_org'], [D_OWN, 'qa_d_own'], [D_BOUNDED, 'qa_d_bounded']] as const) { await e.insert('sys_user_permission_set', { id: `ups_${user}_${n}`, user_id: user, permission_set: set, organization_id: ALPHA, }, SYS); @@ -862,11 +875,19 @@ async function bootDelegated(posture: TenancyPosture) { row('p_beta', D_ORG, D_ORG, BETA), row('p_del_theirs_created_by_org', D_OTHER, D_ORG), row('p_del_org_mine_created_by_other', D_ORG, D_OTHER), + { ...row('p_bounded_held', D_BOUNDED, D_BOUNDED), name: 'held' }, + { ...row('p_bounded_free', D_BOUNDED, D_BOUNDED), name: 'free' }, + { ...row('p_del_bounded_held', D_BOUNDED, D_BOUNDED), name: 'held' }, ]) await e.insert(PARENT, r, SYS); await e.insert(MASTER, row('m_org', D_ORG, D_ORG), SYS); await e.insert(MASTER, row('m_theirs', D_OTHER, D_OTHER), SYS); + await e.insert(MASTER, { ...row('m_bounded_held', D_BOUNDED, D_BOUNDED), name: 'held' }, SYS); + await e.insert(MASTER, { ...row('m_bounded_free', D_BOUNDED, D_BOUNDED), name: 'free' }, SYS); await e.insert(DETAIL, { id: 'd_under_org', note: '', created_by: D_ORG, master: 'm_org', organization_id: ALPHA }, SYS); await e.insert(DETAIL, { id: 'd_under_theirs', note: '', created_by: D_ORG, master: 'm_theirs', organization_id: ALPHA }, SYS); + for (const [id, master] of [['d_under_bounded_held', 'm_bounded_held'], ['d_under_bounded_free', 'm_bounded_free']] as const) { + await e.insert(DETAIL, { id, note: '', created_by: D_BOUNDED, master, organization_id: ALPHA }, SYS); + } let security: any; let sharing: SharingService | undefined; @@ -1200,7 +1221,7 @@ const CLAIM_KEPT_UNDER_A_WALL = * caller cannot read). */ function delegatedRows(posture: TenancyPosture, label: string, walled: boolean): Row[] { - type Cell = [who: string, onBehalf: boolean, op: 'update' | 'delete', object: 'PARENT' | 'DETAIL', id: string, shape: string, enforced: Enforced | 'admitted']; + type Cell = [who: string, onBehalf: boolean, op: 'update' | 'delete', object: 'PARENT' | 'DETAIL', id: string, shape: string, enforced: Enforced | 'admitted', card?: string]; const cells: Cell[] = [ // The delegated path's two legs, each deciding a row. [D_ORG, true, 'update', 'PARENT', 'p_theirs_created_by_org', 'a row owned by someone else that it created', 'admitted'], @@ -1219,10 +1240,19 @@ function delegatedRows(posture: TenancyPosture, label: string, walled: boolean): [D_ORG, false, 'update', 'DETAIL', 'd_under_theirs', "a detail under someone else's master", 'admitted'], [D_ORG, false, 'update', 'PARENT', 'p_beta', 'its own row in another organization', walled ? REFUSED_NOT_FOUND : 'admitted'], [D_OWN, false, 'update', 'DETAIL', 'd_under_org', "a detail it reads under someone else's master", REFUSED_DENIED], + // #24746: an on-behalf-of write never admits a row its delegator's own write refuses. + [D_BOUNDED, true, 'update', 'PARENT', 'p_bounded_held', 'a row its own authored update policy refuses', REFUSED_DENIED, '#24746'], + [D_BOUNDED, false, 'update', 'PARENT', 'p_bounded_held', 'a row its own authored update policy refuses', REFUSED_DENIED, '#24746'], + [D_BOUNDED, true, 'update', 'PARENT', 'p_bounded_free', 'a row its own authored update policy admits', 'admitted', '#24746'], + [D_BOUNDED, true, 'delete', 'PARENT', 'p_del_bounded_held', 'a row its own authored delete policy refuses', REFUSED_DENIED, '#24746'], + [D_BOUNDED, true, 'update', 'DETAIL', 'd_under_bounded_held', 'a detail under a master its own authored update policy refuses', REFUSED_DENIED, '#24746'], + [D_BOUNDED, false, 'update', 'DETAIL', 'd_under_bounded_held', 'a detail under a master its own authored update policy refuses', REFUSED_DENIED, '#24746'], + [D_BOUNDED, true, 'update', 'DETAIL', 'd_under_bounded_free', 'a detail under a master its own authored update policy admits', 'admitted', '#24746'], ]; - return cells.map(([who, onBehalf, op, object, id, shape, enforced]) => ({ - card: '#24646 item 3', - shape: `${label}: ${onBehalf ? `an agent on behalf of the \`${who === D_ORG ? 'org' : 'own'}\` writer` : `the \`${who === D_ORG ? 'org' : 'own'}\` writer`}, ${shape}, ${op}`, + const depthOf = (who: string): string => (who === D_ORG ? 'org' : who === D_OWN ? 'own' : 'bounded'); + return cells.map(([who, onBehalf, op, object, id, shape, enforced, card]) => ({ + card: card ?? '#24646 item 3', + shape: `${label}: ${onBehalf ? `an agent on behalf of the \`${depthOf(who)}\` writer` : `the \`${depthOf(who)}\` writer`}, ${shape}, ${op}`, position: 'record.visible' as const, enforced, run: withDelegated(posture, async (r) => ({ diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 69b8c4dfead..c788b207abe 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -57,6 +57,7 @@ import { superuserBypassBitForOperation } from './permission-evaluator.js'; import { rlsOperationForVerb } from './lifecycle-verb-rls-operation.js'; import { ExplainObjectNotFoundError } from './errors.js'; import { RLS_DENY_FILTER, compiledObjectDefaultRowScopeOf, compiledPolicyNameOf } from './rls-compiler.js'; +import { andComposeLayers } from './tenant-layer.js'; import { declaredComparisonColumns } from './declared-comparison-columns.js'; import { declaredJsonStoredColumns, @@ -1414,6 +1415,41 @@ interface RecordAttributionContext { delegator?: { sets: PermissionSet[]; context: any }; } +/** + * [ADR-0090 D10] The delegator's write filter an on-behalf-of by-id write's + * pre-image re-read (step 2.7) ANDs beside the principal's, as the door + * composes it — two compositions of the delegator's own sets and context, + * ANDed: + * + * - {@link ExplainEngineDeps.computeRlsFilter}: the delegator's filter as the + * delegated path composes it (the floor standing); + * - {@link ExplainEngineDeps.computeLayeredRlsFilter}: the delegator's OWN + * record-grained write composition — the one the delegator's own + * explanation of this record is judged on, carrying the floor decision the + * delegator's own by-id write of the record gets. + * + * So an on-behalf-of record write verdict never admits a row the delegator's + * own record write verdict refuses. The AND only narrows. An engine that + * cannot split the layers composes the first alone, as its record story does + * for every principal. + */ +async function delegatorRecordWriteFilter( + deps: ExplainEngineDeps, + delegator: { sets: PermissionSet[]; context: any }, + object: string, + rlsOp: string, +): Promise | null> { + const parts: Record[] = []; + const delegated = await deps.computeRlsFilter(delegator.sets, object, rlsOp, delegator.context); + if (delegated) parts.push(delegated); + const own = deps.computeLayeredRlsFilter + ? await deps.computeLayeredRlsFilter(delegator.sets, object, rlsOp, delegator.context) + : undefined; + const ownFilter = own ? andComposeLayers(own.layer0, own.layer1) : null; + if (ownFilter) parts.push(ownFilter); + return parts.length === 0 ? null : parts.length === 1 ? parts[0] : { $and: parts }; +} + /** * [C2 / ADR-0095] Fill the per-record row story onto the pipeline. Prepends the * `tenant_isolation` Layer 0, tags every layer with its `kernelTier`, attaches a @@ -1478,13 +1514,13 @@ async function applyRecordAttribution( const layer1 = layered?.layer1; // [#24646 / ADR-0090 D10] An on-behalf-of by-id write may only touch a row the // delegator could touch: step 2.7 composes the DELEGATOR's write filter (its - // own sets, its own context, no floor option) and ANDs it into the same - // pre-image re-read as the principal's. Composed here by that same call, for - // the verbs step 2.7 judges, and judged on the `rls` layer with the + // own sets, its own context) and ANDs it into the same pre-image re-read as + // the principal's — {@link delegatorRecordWriteFilter} is that composition, + // for the verbs step 2.7 judges, judged on the `rls` layer with the // principal's Layer 1. A rejection fails the write, so it is reported fail // closed, as a rejection of the principal's own composition is. const delegatorWriteOrFault = delegator && recordExists && (rlsOp === 'update' || rlsOp === 'delete') - ? await settle(deps.computeRlsFilter(delegator.sets, object, rlsOp, delegator.context)) + ? await settle(delegatorRecordWriteFilter(deps, delegator, object, rlsOp)) : undefined; const delegatorWriteFault = delegatorWriteOrFault === DEPENDENCY_FAULT; const delegatorWriteFilter = delegatorWriteOrFault === DEPENDENCY_FAULT ? undefined : (delegatorWriteOrFault ?? undefined); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index ebd640e30fc..ec081e79d21 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -2571,6 +2571,12 @@ export class SecurityPlugin implements Plugin { // floor on exactly the terms its `where` scope does: step 3 composes the // bulk scope with no floor options either. let preImageFloorOpts: RlsFilterOptions | null = null; + // [ADR-0090 D10] Its twin for the DELEGATOR of an on-behalf-of write: the + // floor decision and coverage vouch the delegator's OWN direct by-id write + // of this row would get ({@link delegatorOwnWriteFloorOptions}). Null on + // the ordinary path and until step 2.7 runs; step 3.6 reads it so the + // post-image is held to the delegator's own check as well. + let delegatorPreImageFloorOpts: RlsFilterOptions | null = null; // [ADR-0066 D2/D3] Resolve the object's security posture (private flag, // platform-global flag, capability contract) once for the checks below. @@ -3046,7 +3052,9 @@ export class SecurityPlugin implements Plugin { // would drop one principal's floor and not the other's: a half-state // no measurement on this card covers. The delegated by-id write keeps // BOTH floors, exactly as before, and that residual is recorded on the - // card rather than resolved by a guess. + // card rather than resolved by a guess. What bounds it by the + // delegator is the delegator's own composition ANDed below, never a + // dropped floor here. const masterGateCoversThisWrite = !delegatorSets; preImageFloorOpts = { dropPlatformOwnershipFloor, masterGateCoversThisWrite }; const writeFilter = await this.computeRlsFilter( @@ -3064,7 +3072,37 @@ export class SecurityPlugin implements Plugin { const delWriteFilter = delegatorSets ? await this.computeRlsFilter(delegatorSets, opCtx.object, rlsOperation, delegatorContext) : null; - const writeParts = [writeFilter, delWriteFilter].filter(Boolean) as Record[]; + // [ADR-0090 D10] ...and the delegator's OWN direct by-id write of this + // row must admit it as well. The filter above is composed with the + // delegated path's floor exclusion, which is not the composition the + // delegator's own write of this row gets, so it alone does not bound + // the on-behalf-of write by what the delegator may write. The + // delegator's own composition — its own floor decision and coverage + // vouch, {@link delegatorOwnWriteFloorOptions} — is ANDed beside it: + // the row must satisfy both, so this only ever narrows, and an + // on-behalf-of write never admits a row the delegator's own write + // refuses. The vouch holds on this path too: step 2.8 asks the master + // gate for the delegator below. + if (delegatorSets) { + delegatorPreImageFloorOpts = await this.delegatorOwnWriteFloorOptions( + rlsOperation, + opCtx.object, + String(targetId), + delegatorContext, + delegatorSets, + true, + ); + } + const delOwnWriteFilter = delegatorSets && delegatorPreImageFloorOpts + ? await this.computeRlsFilter( + delegatorSets, + opCtx.object, + rlsOperation, + delegatorContext, + delegatorPreImageFloorOpts, + ) + : null; + const writeParts = [writeFilter, delWriteFilter, delOwnWriteFilter].filter(Boolean) as Record[]; // [#21771] Ruling A on the write doors: a row the caller cannot READ // is a row that does not exist. For the by-id write the caller // addressed (see {@link addressedByIdWriteId}), "can you read it?" is @@ -3603,7 +3641,21 @@ export class SecurityPlugin implements Plugin { const delCheckFilter = delegatorSets ? await this.computeWriteCheckFilter(delegatorSets, opCtx.object, opCtx.operation, delegatorContext) : null; - const checkParts = [checkFilter, delCheckFilter].filter(Boolean) as Record[]; + // [ADR-0090 D10] ...and the check the delegator's OWN direct by-id write + // of this row is held to, on the floor decision its own pre-image got + // (step 2.7, `delegatorPreImageFloorOpts`). ANDed beside the check + // above, so it only narrows: an on-behalf-of write never stores a row + // the delegator's own write could not. + const delOwnCheckFilter = delegatorSets && delegatorPreImageFloorOpts + ? await this.computeWriteCheckFilter( + delegatorSets, + opCtx.object, + opCtx.operation, + delegatorContext, + delegatorPreImageFloorOpts, + ) + : null; + const checkParts = [checkFilter, delCheckFilter, delOwnCheckFilter].filter(Boolean) as Record[]; if (checkParts.length > 0) { // The ONE refusal, shared by every shape — so a write judged inside // the engine (an insert, a predicate update) and a by-id update @@ -4985,8 +5037,12 @@ export class SecurityPlugin implements Plugin { // delegated principal at its agent-leg depth, then, if that admits, once // for the delegator at the delegator's own depth. The delegator's own // write filter, which step 2.7 ANDs into the pre-image re-read, is composed - // by the engine. The floor decision still excludes this path, as the gate - // does. The delegated READ depths are not modelled on explain's record + // by the engine, as the gate composes it: once with the floor standing + // (`computeRlsFilter`), once as the delegator's own record composition + // (`computeLayeredRlsFilter` below, asked with the delegator's context, so + // its floor decision is the delegator's own). The floor decision still + // excludes the delegated context itself, as the gate's does. The delegated + // READ depths are not modelled on explain's record // path: the read filter is asked with the delegated context as it stands. const actsOnBehalfOf = (c: any): boolean => !!c?.onBehalfOf?.userId; const recordWriteFloorOptions = async ( @@ -5413,7 +5469,9 @@ export class SecurityPlugin implements Plugin { * delegated context (`hasWriteBypass`: "no D10 delegator intersection on * this path", ADR-0111 D2), so composing here could only produce a verdict * resolved against the wrong identity. The delegated write keeps both - * principals' floors, exactly as before. + * principals' floors, exactly as before — and beside the delegator's + * floor-standing filter it also ANDs the delegator's own composition, + * asked here AS the delegator ({@link delegatorOwnWriteFloorOptions}). */ private async resolvePreImageFloorDrop( rlsOperation: string, @@ -5438,6 +5496,48 @@ export class SecurityPlugin implements Plugin { ); } + /** + * [ADR-0090 D10] The composition knobs the DELEGATOR's own direct by-id + * write of `recordId` would get: its floor decision + * ({@link resolvePreImageFloorDrop}, asked as the delegator — its own sets, + * its own context, not delegated) and the coverage vouch the caller makes + * for that write. + * + * An on-behalf-of principal never exceeds its delegator. Every delegated + * row gate composes the delegator's filter with the floor standing (the + * delegated path's exclusion), which is a different composition from the one + * the delegator's own write of the same row gets, so on its own it does not + * bound the delegated write by what the delegator may write. Each such gate + * therefore also composes the delegator's filter with THESE options and ANDs + * the two: the by-id pre-image (step 2.7), its post-image check (step 3.6) + * and the master gate's delegator pass ({@link assertMasterRowEditable}). + * The AND only ever narrows — a row passes only where today's composition + * AND the delegator's own both admit it — so nothing refused before is + * admitted by it. + * + * `masterGateCoversThisWrite` is the vouch the caller's own direct + * composition makes, passed rather than assumed, because coverage is a fact + * about the call site ({@link RlsFilterOptions.masterGateCoversThisWrite}). + */ + private async delegatorOwnWriteFloorOptions( + rlsOperation: string, + object: string, + recordId: string, + delegatorContext: any, + delegatorSets: PermissionSet[], + masterGateCoversThisWrite: boolean, + ): Promise { + const dropPlatformOwnershipFloor = await this.resolvePreImageFloorDrop( + rlsOperation, + object, + recordId, + delegatorContext, + delegatorSets, + false, + ); + return { dropPlatformOwnershipFloor, masterGateCoversThisWrite }; + } + /** * [#5493 / ADR-0105 D3] `ISecurityService.checkAuthoredRowWrite` — does an * APP-AUTHORED row-level policy admit this row for this write operation, on @@ -9028,7 +9128,34 @@ export class SecurityPlugin implements Plugin { dropPlatformOwnershipFloor, masterGateCoversThisWrite: !delegatedWrite && this.declaresControlledByParent(rel.master), }); - if (masterWriteFilter) { + // [ADR-0090 D10] On the DELEGATOR's pass of a delegated write — `context` + // is the delegator's own (no delegation link) while `opCtx` carries the + // link — the master must also be editable under the composition the + // delegator's OWN direct write of this detail gets on this hop: its own + // floor decision on the master row, and the vouch the direct hop makes + // ({@link delegatorOwnWriteFloorOptions}). ANDed beside the filter above, + // so it only narrows: an on-behalf-of detail write never passes a master + // the delegator's own detail write is refused on. The agent's pass is + // unchanged. + const delegatorPass = delegatedWrite && !context?.onBehalfOf?.userId; + const delegatorOwnMasterWriteFilter = delegatorPass + ? await this.computeRlsFilter( + permissionSets, + rel.master, + 'update', + context, + await this.delegatorOwnWriteFloorOptions( + 'update', + rel.master, + String(masterId), + context, + permissionSets, + this.declaresControlledByParent(rel.master), + ), + ) + : null; + const masterWriteParts = [masterWriteFilter, delegatorOwnMasterWriteFilter].filter(Boolean) as Record[]; + if (masterWriteParts.length > 0) { let visible: unknown = null; // [#7505] This catch STAYS, and the difference from the existence probe // above is the whole per-caller distinction the ruling asks for. That @@ -9041,7 +9168,7 @@ export class SecurityPlugin implements Plugin { // not as a site to change. try { visible = await this.ql.findOne(rel.master, { - where: { $and: [{ id: masterId }, masterWriteFilter] }, + where: { $and: [{ id: masterId }, ...masterWriteParts] }, context, }); } catch {