From 412f6caf73f9a3318bc89826847ab702b426f65f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 05:52:41 +0000 Subject: [PATCH 1/2] fix(plugin-security): an on-behalf-of by-id write never exceeds the delegator's own write ADR-0090 D10: a delegated principal never exceeds its delegator. Each delegated row gate of a by-id write (the step 2.7 pre-image, the step 3.6 post-image check and the master gate's delegator pass) now ANDs the delegator's own direct composition of that write beside the composition the delegated path already used. The AND only narrows. Explain's record verdict composes the same two filters for the delegator. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- .../plugin-security/src/explain-engine.ts | 44 +++++- .../plugin-security/src/security-plugin.ts | 143 +++++++++++++++++- 2 files changed, 175 insertions(+), 12 deletions(-) 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 { From dfa8fac7f3aaec1d926809a6010906605635071b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 05:57:35 +0000 Subject: [PATCH 2/2] test(plugin-security): pin that an on-behalf-of by-id write never exceeds the delegator's own write Real-stack pins under `isolated` and `single`: the delegated by-id update, delete, stored row and master-gated detail write are refused wherever the delegator's own write is, with the ADR-0112 envelope and the delegator's own request as the control; rows the delegator's own write admits and the delegated write has always refused stay refused. Explain's record verdict is asserted beside the door, and the parity table gains the matching rig 4 rows. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- ...delegated-write-never-exceeds-delegator.md | 17 + ...ated-write-never-exceeds-delegator.test.ts | 371 ++++++++++++++++++ .../src/explain-enforce-parity.test.ts | 46 ++- 3 files changed, 426 insertions(+), 8 deletions(-) create mode 100644 .changeset/24746-delegated-write-never-exceeds-delegator.md create mode 100644 packages/plugins/plugin-security/src/delegated-write-never-exceeds-delegator.test.ts 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) => ({