From 131b8cfa2c3d7f2afefebbb011f43bfe2c49f8c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:40:25 +0000 Subject: [PATCH 1/5] =?UTF-8?q?fix(plugin-security):=20the=20controlled=5F?= =?UTF-8?q?by=5Fparent=20write=20gate=20asks=20the=20master-edit=20questio?= =?UTF-8?q?n=20on=20every=20write=20shape=20(ADR-0055=20=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The master-detail write gate decided one write shape of a controlled_by_parent detail without asking whether the caller may edit the masters involved. It now judges that shape per master, through the same chain walk a by-id write runs, and refuses with the by-id leg's envelope. The chain walk is extracted verbatim so both legs run one copy. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- .../plugin-security/src/security-plugin.ts | 168 +++++++++++++++++- 1 file changed, 162 insertions(+), 6 deletions(-) diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 4cb6e250600..cdc1192e522 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -3230,7 +3230,9 @@ export class SecurityPlugin implements Plugin { // path by checking the master instead — and the master's own write // filter, since #7665, derives from its SELECT narrowing when no // write-class policy applies, so a select-only master gates its details - // by visibility too. + // by visibility too. [#25012] The same gate answers a write that + // addresses no single row: the masters of the rows it selects are each + // judged, and one the caller may not edit refuses the whole write. if ( ['insert', 'update', 'delete', 'transfer', 'restore', 'purge'].includes(opCtx.operation) && permissionSets.length > 0 && @@ -9011,9 +9013,20 @@ export class SecurityPlugin implements Plugin { * ⛔ Not a blanket refusal for chained declarations: the single-level case is * the loop's first iteration and its answer is byte-for-byte the old one. * - * v1 scope: single-id writes. Bulk writes flow through the AST and are already - * scoped by the controlled-by-parent READ filter (to readable masters) — which - * since the ADR-0055 amendment is itself chain-composed, so the two faces still agree. + * [#25012] EVERY write shape asks it. A write that addresses no single row — + * the engine's predicate path, which `extractSingleId` cannot name a target + * for — is judged per MASTER: the gate collects the masters of the detail + * rows the caller's own predicate selects and the caller can read + * ({@link controlledByParentPredicateWriteMasters}), and runs the same chain + * walk on each ({@link assertControlledByParentChainEditable}). The first + * refusal is the answer, in the by-id leg's own envelope, and nothing is + * written. It is a VERDICT, never a scope: no master set is composed into the + * write's predicate (ADR-0055 §2's master id set is the READ scope, and its + * mechanism is decided on its own record), so a write that selects one such + * row writes none of them. A write whose rows all sit under masters the caller + * may edit lands exactly as before: the read scope, the platform ownership + * floor and the detail's own write policy still decide which of them it + * writes, and nothing here vouches the floor away. * * [#7474] SIX conditions refuse a write here, and they are NOT one verdict. * Three are genuine authorization answers (no object-level `update` on the @@ -9106,7 +9119,28 @@ export class SecurityPlugin implements Plugin { masterId = data && typeof data === 'object' && !Array.isArray(data) ? (data as any)[rel.fk] : undefined; } else { const targetId = this.extractSingleId(opCtx); - if (targetId == null) return; // bulk write — scoped by the read filter on the AST + if (targetId == null) { + // [#25012] No single target row. Either the engine refuses this shape + // itself before anything is written (its `reject` verdict — nothing to + // judge here), or it is the engine's predicate path, read from the + // engine's own dispatch predicates exactly as the write doors read them + // ({@link addressedPredicateWrite}), and every master the write would + // write details under is judged by the same walk a by-id write runs. + const predicatePath = + operation === 'update' + ? opCtx.data != null && + typeof opCtx.data === 'object' && + resolveEngineUpdateDispatch(opCtx.data as EngineUpdateDispatchData, opCtx.options).kind === 'multi' + : operation === 'delete' && + resolveEngineDeleteDispatch(opCtx.options as EngineDeleteDispatchInput | undefined).kind === 'multi'; + if (!predicatePath) return; + for (const predicateMasterId of await this.controlledByParentPredicateWriteMasters(object, operation, rel, opCtx)) { + await this.assertControlledByParentChainEditable( + permissionSets, object, rel, predicateMasterId, context, opCtx, masterEditDenied, denyMasterEdit, + ); + } + return; + } detailRecordId = targetId; const row = await this.readRowById(object, targetId, { isSystem: true }); if (!row) throw new DetailRecordNotFoundError(object, operation, targetId); @@ -9230,6 +9264,41 @@ export class SecurityPlugin implements Plugin { throw new MasterReferenceMissingError(object, operation, rel.fk, detailRecordId); } + await this.assertControlledByParentChainEditable( + permissionSets, object, rel, masterId, context, opCtx, masterEditDenied, denyMasterEdit, + ); + } + + /** + * [ADR-0055 amendment / #25012] The chain walk of + * {@link assertControlledByParentWrite}: the three master-edit legs + * ({@link assertMasterRowEditable}) run on ONE master row, then on that + * master's own master, hop by hop, until a master that governs its own rows + * is reached — with the cycle guard and the depth bound + * ({@link CBP_MAX_CHAIN_DEPTH}), each failing closed. + * + * ⚠️ 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}). There must + * stay exactly one walk, for the reason {@link assertMasterRowEditable} gives + * for its legs. + * + * `masterEditDenied` / `denyMasterEdit` are the gate's own refusal builders, + * passed in rather than rebuilt so every refusal, at every hop and for every + * write shape, names the CALLER's object and operation (#7474's envelope) + * and records its leg ({@link MASTER_EDIT_DENIAL_LEG}). + */ + private async assertControlledByParentChainEditable( + permissionSets: PermissionSet[], + object: string, + rel: CbpRelation, + masterId: unknown, + context: any, + opCtx: any, + masterEditDenied: (leg: ControlledByParentWriteDenialLeg, reason: string, recordId?: unknown) => PermissionDeniedError, + denyMasterEdit: (leg: ControlledByParentWriteDenialLeg, reason: string, recordId?: unknown) => never, + ): Promise { // [ADR-0055 amendment] Walk the `controlled_by_parent` chain, one hop at a time, and run // the SAME three master-edit legs on every hop. // @@ -9293,7 +9362,7 @@ export class SecurityPlugin implements Plugin { ); } // Read as SYSTEM — we only need this row's master FK, exactly as the - // detail's own FK was read above. A throw here is a store fault and + // gate reads the detail's own. A throw here is a store fault and // propagates (#7505): an outage must not be reported as an access verdict. const masterRow = await this.readRowById(masterObject, hopMasterId, { isSystem: true }); if (!masterRow) { @@ -9317,6 +9386,93 @@ export class SecurityPlugin implements Plugin { } } + /** + * [#25012 / ADR-0055 §3] The masters a write that addresses no single row + * would write details under — what {@link assertControlledByParentWrite} + * judges on the engine's predicate path, one chain walk per master. + * + * WHICH ROWS. The detail rows the caller's own predicate selects AND the + * caller can read, asked through the read door under the request's context — + * the read question the write doors narrow a predicate write's matched set to + * ({@link readableRowsScopeForPredicateWrite}), so every row that write can + * reach is among them. They can be MORE than it writes: the write-class + * scopes (the platform ownership floor, the detail's own write policy) still + * leave some of them unwritten, and a row among them under a master the + * caller may not edit refuses the write all the same. The caller named that + * row and can read it, and its master too (the detail's read scope is the + * readable-master set), so the refusal discloses nothing the read door does + * not. A row the caller cannot read is never judged: it is not matched. + * + * Three outcomes: + * + * - the distinct master ids of those rows — an empty list when the predicate + * selects no row the caller can read, and nothing is refused; + * - ABOVE {@link PREDICATE_WRITE_READABLE_ID_CAP} rows, the refusal the read + * narrowing answers for the same count ({@link predicateWriteReadableCapError}): + * masters of rows that were never enumerated cannot be judged; + * - the read door's own refusal, PROPAGATED as raised — a declared 4xx (the + * detail's read grant withheld, a predicate the read guard or the driver + * refuses) or a store fault. ⛔ Never read as "no rows": a write whose rows + * cannot be enumerated cannot have their masters judged, and admitting it + * would be the unjudged write this leg exists to close. + * + * Each row's master reference is then read as SYSTEM, as the by-id leg reads + * its one target row's: only the reference's value is needed, and a field + * rule hiding it from the caller must not hide the master from this gate. A + * row whose reference is empty answers the by-id leg's stored-row refusal + * ({@link MasterReferenceMissingError}, `422`). + */ + private async controlledByParentPredicateWriteMasters( + object: string, + operation: string, + rel: CbpRelation, + opCtx: any, + ): Promise { + if (typeof this.ql?.find !== 'function') { + // Unreachable in a real deployment — `start()` registers no security + // middleware at all without a query engine — but a non-conforming engine + // double fails closed rather than admitting an unjudged write. + throw new Error( + `[Security] Cannot judge the ${operation} on '${object}' against the masters of the records it ` + + `selects: no query engine is available to read them.`, + ); + } + const callerWhere = (opCtx.options as { where?: unknown } | undefined)?.where; + const readable = await this.ql.find(object, { + ...(callerWhere != null ? { where: copyOfCallerPredicate(callerWhere) } : {}), + fields: ['id'], + limit: PREDICATE_WRITE_READABLE_ID_CAP + 1, + // The request's envelope minus the operation-private keys, as the read + // narrowing asks it: on a delegated write that is the agent's context with + // the delegation link on it, so the rows are the ones both may read. + context: withoutOperationPrivateKeys((opCtx.context ?? {}) as Record), + }); + const rows: unknown[] = Array.isArray(readable) ? readable : []; + if (rows.length > PREDICATE_WRITE_READABLE_ID_CAP) { + throw predicateWriteReadableCapError(object, operation); + } + const ids = rows + .map((r) => (r && typeof r === 'object' ? (r as { id?: unknown }).id : undefined)) + .filter((id) => id !== undefined && id !== null); + if (ids.length === 0) return []; + const stored = await this.ql.find(object, { + where: { id: { $in: ids } }, + fields: ['id', rel.fk], + limit: ids.length, + context: { isSystem: true }, + }); + const masterIds = new Set(); + for (const row of Array.isArray(stored) ? stored : []) { + if (!row || typeof row !== 'object') continue; + const masterId = (row as Record)[rel.fk]; + if (masterId == null) { + throw new MasterReferenceMissingError(object, operation, rel.fk, (row as { id?: unknown }).id); + } + masterIds.add(masterId); + } + return [...masterIds]; + } + /** * [#5386 / commit 498f4e884 / #8679] The three legs that decide whether ONE principal may * EDIT ONE master row — extracted verbatim from From 9f08525a6a2b44e30b34cce8a379f95b49bca79f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:43:10 +0000 Subject: [PATCH 2/5] =?UTF-8?q?test(plugin-security):=20pin=20the=20master?= =?UTF-8?q?-edit=20verdict=20on=20every=20controlled=5Fby=5Fparent=20detai?= =?UTF-8?q?l=20write=20shape=20(ADR-0055=20=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- ...-by-parent-write-shape-master-edit.test.ts | 430 ++++++++++++++++++ 1 file changed, 430 insertions(+) create mode 100644 packages/plugins/plugin-security/src/controlled-by-parent-write-shape-master-edit.test.ts diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-write-shape-master-edit.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-write-shape-master-edit.test.ts new file mode 100644 index 00000000000..0f1869bb5b9 --- /dev/null +++ b/packages/plugins/plugin-security/src/controlled-by-parent-write-shape-master-edit.test.ts @@ -0,0 +1,430 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0055 §3] Editing or deleting a `controlled_by_parent` detail requires + * edit access to its master, and the rule names no write shape as exempt. + * + * The master-edit gate answers every write the engine routes: one that names + * its target row, and one that names none and selects its rows instead. For + * the second, the gate judges the masters of the detail rows the caller's own + * selection reaches and the caller can read, one chain walk per master, and + * pinned here is what that judgement answers: + * + * - a master the caller may not edit refuses the whole write, with the very + * refusal the by-id door gives the same caller for the same master, and + * nothing is written: for each refusal leg, for a member, an org admin and + * an agent acting for a member, for an update and a delete; + * - one such master among editable ones refuses the whole write too; + * - a write whose rows all sit under masters the caller may edit lands as it + * did, and writes exactly the rows it wrote — the platform ownership floor + * still binds a member; + * - a row the caller cannot read is not judged and is not written; + * - a caller whose read of the detail is refused is refused the write with + * that read's own refusal, and nothing is written. + * + * 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, beforeAll, 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 { 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_wsme'; +const POSTURE = 'isolated' as const; + +/** Holds the org's `member` role, so the platform ownership floor binds it. */ +const MEMBER = 'u_cbp_wsme_member'; +/** Holds the org's `admin` role, outside the floor's domain. */ +const ADMIN = 'u_cbp_wsme_admin'; +/** Holds a grant on the detail that withholds its read. */ +const UNREAD = 'u_cbp_wsme_unread'; +/** Owns the masters and creates the rows nobody else here does. */ +const OTHER = 'u_cbp_wsme_other'; + +/** One master per refusal leg, one the caller may edit, and one the caller cannot read. */ +const M = { + object: 'qa_cbp_wsme_master_object', + sharing: 'qa_cbp_wsme_master_sharing', + rls: 'qa_cbp_wsme_master_rls', + hidden: 'qa_cbp_wsme_master_hidden', +} as const; +const D = { + object: 'qa_cbp_wsme_detail_object', + sharing: 'qa_cbp_wsme_detail_sharing', + rls: 'qa_cbp_wsme_detail_rls', + hidden: 'qa_cbp_wsme_detail_hidden', +} as const; +type Shape = keyof typeof M; + +/** The master row the caller may not edit, and (on the rls shape) the one it may. */ +const refusingMaster = (shape: Shape) => `${M[shape]}_held`; +const EDITABLE_MASTER = `${M.rls}_free`; + +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), + }; + }, + ); + +/** 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) => ({ + name, + label: name, + 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 }, + }, +}); + +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-write-shape-master-edit', + name: 'A controlled_by_parent detail is written only under masters the caller may edit, on every write shape', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPosition, SysUserPermissionSet, + SysOrganization, SysUser, SysMember, SysRecordShare, + { name: M.object, label: 'Master', sharingModel: 'public_read_write', fields: masterFields }, + { name: M.sharing, label: 'Master', sharingModel: 'private', fields: masterFields }, + { name: M.rls, label: 'Master', sharingModel: 'public_read_write', fields: masterFields }, + { name: M.hidden, label: 'Master', sharingModel: 'public_read_write', fields: masterFields }, + detailOf(D.object, M.object), + detailOf(D.sharing, M.sharing), + detailOf(D.rls, M.rls), + detailOf(D.hidden, M.hidden), + ], + } 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'], [UNREAD, '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_wsme_writer'; + const UNREAD_WRITER = 'qa_cbp_wsme_unread_writer'; + const sets = [ + ...defaultPermissionSets, + PermissionSetSchema.parse({ + name: WRITER, + objects: { + [M.object]: { allowRead: true }, + [M.sharing]: { allowRead: true, allowEdit: true, readScope: 'org', writeScope: 'own' }, + [M.rls]: { allowRead: true, allowEdit: true }, + [D.object]: CRUD, + [D.sharing]: CRUD, + [D.rls]: CRUD, + [D.hidden]: CRUD, + }, + rowLevelSecurity: [{ name: 'qa_cbp_wsme_master_updates', object: M.rls, operation: 'update', using: "name != 'held'" }], + }), + PermissionSetSchema.parse({ + name: UNREAD_WRITER, + objects: { [M.rls]: { allowRead: true, allowEdit: true }, [D.rls]: { allowEdit: true, allowDelete: true } }, + }), + ]; + for (const name of [WRITER, UNREAD_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 [[MEMBER, WRITER], [ADMIN, WRITER], [UNREAD, UNREAD_WRITER]] as const) { + await e.insert('sys_user_permission_set', { id: `ups_${user}`, user_id: user, permission_set: set, organization_id: ORG }, SYS); + } + for (const master of Object.values(M)) { + await e.insert(master, { id: `${master}_held`, name: 'held', owner_id: OTHER, created_by: OTHER, organization_id: ORG }, SYS); + } + await e.insert(M.rls, { id: EDITABLE_MASTER, name: 'free', owner_id: OTHER, created_by: OTHER, organization_id: ORG }, SYS); + + 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 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 { + contextOf, + /** Two rows the caller created under `master` and one somebody else did, written as SYSTEM; all carry one tag. */ + seed: async (detail: string, master: string, creator: string, tag = `t${++n}`) => { + const rows = { own: [`${tag}_own_1`, `${tag}_own_2`], others: [`${tag}_others_1`] }; + for (const [id, by] of [[rows.own[0], creator], [rows.own[1], creator], [rows.others[0], OTHER]] as const) { + await e.getDriver(detail).create(detail, { id, note: tag, created_by: by, master, organization_id: ORG }); + } + return { tag, ...rows, all: [...rows.own, ...rows.others] }; + }, + /** Each row's stored `note`, or `null` once it is gone. */ + notes: 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 ? row.note : null; + } + return out; + }, + byIdUpdate: (detail: string, id: string, context: Record) => + outcomeOf(engine.update(detail, { note: 'written' }, { where: { id }, context: { ...context } } as never)), + byIdDelete: (detail: string, id: string, context: Record) => + outcomeOf(engine.delete(detail, { where: { id }, context: { ...context } } as never)), + selectUpdate: (detail: string, where: Record, context: Record) => + outcomeOf(engine.update(detail, { note: 'written' }, { where, multi: true, context: { ...context } } as never)), + selectDelete: (detail: string, where: Record, context: Record) => + outcomeOf(engine.delete(detail, { where, multi: true, context: { ...context } } as never)), + read: (detail: string, where: Record, context: Record) => + outcomeOf(engine.find(detail, { where, context: { ...context } } as never)), + teardown: async () => { + try { await engine.destroy(); } catch { /* noop */ } + releaseAsyncContexts(engine, plugin); + }, + }; +} + +type Rig = Awaited>; + +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; + +/** The by-id 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); + +const LEGS: ReadonlyArray<{ leg: string; shape: Shape; callers: readonly Caller[] }> = [ + { leg: 'the object-permission leg', shape: 'object', callers: ['a member', 'an org admin', 'an agent acting for a member'] }, + { leg: 'the record-level leg of a private master', shape: 'sharing', callers: ['a member', 'an org admin'] }, + { leg: "the master's own row-level policy", shape: 'rls', callers: ['a member', 'an org admin', 'an agent acting for a member'] }, +]; + +describe("ADR-0055 §3 — a controlled_by_parent detail is written only under masters the caller may edit, on every write shape", () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(); }); + afterAll(async () => { await rig.teardown(); }); + + describe("a master the caller may not edit refuses the write, with the by-id door's own refusal", () => { + for (const { leg, shape, callers } of LEGS) { + for (const caller of callers) { + it(`${leg}, ${caller}: the update and the delete are refused as the by-id door refuses them, and nothing is written`, async () => { + const { user, onBehalf } = CALLERS[caller]; + const context = await rig.contextOf(user, onBehalf); + const byId = await rig.seed(D[shape], refusingMaster(shape), user); + const byIdUpdate = await rig.byIdUpdate(D[shape], byId.own[0], context); + const byIdDelete = await rig.byIdDelete(D[shape], byId.own[1], context); + // The precondition: the by-id door refuses this caller on the master-edit question. + expect(byIdUpdate).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + expect(byIdDelete).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + + const updated = await rig.seed(D[shape], refusingMaster(shape), user); + const update = await rig.selectUpdate(D[shape], { note: updated.tag }, context); + const deleted = await rig.seed(D[shape], refusingMaster(shape), user); + const del = await rig.selectDelete(D[shape], { note: deleted.tag }, context); + + expect(envelope(update)).toEqual(envelope(byIdUpdate)); + expect(envelope(del)).toEqual(envelope(byIdDelete)); + expect(Object.values(await rig.notes(D[shape], updated.all))).toEqual([updated.tag, updated.tag, updated.tag]); + expect(Object.values(await rig.notes(D[shape], deleted.all))).toEqual([deleted.tag, deleted.tag, deleted.tag]); + }); + } + } + + it('whatever the selection names — the master reference, another field, or a list of ids — the answer is the same refusal', async () => { + const context = await rig.contextOf(MEMBER, false); + const byId = await rig.seed(D.rls, refusingMaster('rls'), MEMBER); + const byIdUpdate = await rig.byIdUpdate(D.rls, byId.own[0], context); + const byIdDelete = await rig.byIdDelete(D.rls, byId.own[1], context); + expect(byIdUpdate).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + const selections: ReadonlyArray<(rows: { tag: string; all: string[] }) => Record> = [ + () => ({ master: refusingMaster('rls') }), + (rows) => ({ note: rows.tag }), + (rows) => ({ id: { $in: rows.all } }), + ]; + for (const where of selections) { + const updated = await rig.seed(D.rls, refusingMaster('rls'), MEMBER); + expect(envelope(await rig.selectUpdate(D.rls, where(updated), context))).toEqual(envelope(byIdUpdate)); + const deleted = await rig.seed(D.rls, refusingMaster('rls'), MEMBER); + expect(envelope(await rig.selectDelete(D.rls, where(deleted), context))).toEqual(envelope(byIdDelete)); + expect(Object.values(await rig.notes(D.rls, [...updated.all, ...deleted.all])).every((v) => v !== null && v !== 'written')).toBe(true); + } + }); + + it('one such master among masters the caller may edit refuses the whole write, and nothing is written under either', async () => { + const context = await rig.contextOf(ADMIN, false); + const tag = 'mixed'; + const editable = await rig.seed(D.rls, EDITABLE_MASTER, ADMIN, `${tag}_a`); + const refusing = await rig.seed(D.rls, refusingMaster('rls'), ADMIN, `${tag}_b`); + const ids = [...editable.all, ...refusing.all]; + const byIdUpdate = await rig.byIdUpdate(D.rls, refusing.own[0], context); + expect(byIdUpdate).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + + expect(envelope(await rig.selectUpdate(D.rls, { id: { $in: ids } }, context))).toEqual(envelope(byIdUpdate)); + const before = await rig.notes(D.rls, ids); + expect(Object.values(before).includes('written')).toBe(false); + const del = await rig.selectDelete(D.rls, { id: { $in: ids } }, context); + expect(del).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + expect(Object.values(await rig.notes(D.rls, ids)).includes(null)).toBe(false); + }); + }); + + describe('a write whose rows all sit under masters the caller may edit lands as it did', () => { + const WRITES: Record = { + // The platform ownership floor binds a member, on its own write and an agent's alike. + 'a member': 'own', + 'an agent acting for a member': 'own', + 'an org admin': 'all', + }; + for (const caller of Object.keys(CALLERS) as Caller[]) { + it(`${caller}: the update and the delete land, and write ${WRITES[caller] === 'own' ? 'only the rows it created' : 'every row'}`, async () => { + const { user, onBehalf } = CALLERS[caller]; + const context = await rig.contextOf(user, onBehalf); + const updated = await rig.seed(D.rls, EDITABLE_MASTER, user); + const update = await rig.selectUpdate(D.rls, { note: updated.tag }, context); + const deleted = await rig.seed(D.rls, EDITABLE_MASTER, user); + const del = await rig.selectDelete(D.rls, { note: deleted.tag }, context); + const reached = (rows: { own: string[]; all: string[] }) => (WRITES[caller] === 'own' ? rows.own : rows.all); + + expect(update).toEqual({ kind: 'landed', value: reached(updated).length }); + expect(del).toEqual({ kind: 'landed', value: reached(deleted).length }); + const updatedNotes = await rig.notes(D.rls, updated.all); + const deletedNotes = await rig.notes(D.rls, deleted.all); + for (const id of updated.all) expect(updatedNotes[id]).toBe(reached(updated).includes(id) ? 'written' : updated.tag); + for (const id of deleted.all) expect(deletedNotes[id]).toBe(reached(deleted).includes(id) ? null : deleted.tag); + }); + } + }); + + it('a row the caller cannot read is not judged and is not written', async () => { + const context = await rig.contextOf(ADMIN, false); + const rows = await rig.seed(D.hidden, refusingMaster('hidden'), ADMIN); + // The precondition: the caller's read door does not return these rows. + expect(await rig.read(D.hidden, { note: rows.tag }, context)).toEqual({ kind: 'landed', value: [] }); + expect(await rig.selectUpdate(D.hidden, { note: rows.tag }, context)).toEqual({ kind: 'landed', value: 0 }); + expect(await rig.selectDelete(D.hidden, { note: rows.tag }, context)).toEqual({ kind: 'landed', value: 0 }); + expect(Object.values(await rig.notes(D.hidden, rows.all))).toEqual([rows.tag, rows.tag, rows.tag]); + }); + + it("a caller whose read of the detail is refused is refused the write with that read's own refusal, and nothing is written", async () => { + const context = await rig.contextOf(UNREAD, false); + const rows = await rig.seed(D.rls, EDITABLE_MASTER, UNREAD); + const read = await rig.read(D.rls, { note: rows.tag }, context); + expect(read).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + expect(envelope(await rig.selectUpdate(D.rls, { note: rows.tag }, context))).toEqual(envelope(read)); + expect(envelope(await rig.selectDelete(D.rls, { note: rows.tag }, context))).toEqual(envelope(read)); + expect(Object.values(await rig.notes(D.rls, rows.all))).toEqual([rows.tag, rows.tag, rows.tag]); + }); +}); From 44eef53692a7ff83d3eb62a5a410e343996d50bd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 02:46:33 +0000 Subject: [PATCH 3/5] chore(changeset): plugin-security master-edit question on every controlled_by_parent detail write shape Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- .../25012-cbp-write-shape-master-edit.md | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .changeset/25012-cbp-write-shape-master-edit.md diff --git a/.changeset/25012-cbp-write-shape-master-edit.md b/.changeset/25012-cbp-write-shape-master-edit.md new file mode 100644 index 00000000000..86071ec07d8 --- /dev/null +++ b/.changeset/25012-cbp-write-shape-master-edit.md @@ -0,0 +1,20 @@ +--- +'@objectstack/plugin-security': minor +--- + +fix(plugin-security)!: every write to a `controlled_by_parent` detail asks for edit access to its master (ADR-0055 §3) + +Clause-②: no (narrowing) + + + +ADR-0055 §3 says editing or deleting a detail requires edit access to its master, and it names no write shape as exempt. The master-detail write gate decided one shape of write to a `controlled_by_parent` detail without asking that question. It now asks it: the gate collects the masters of the detail rows the write would reach, as the caller's own read returns them, and runs on each master the check a by-id write runs. One master the caller may not edit refuses the whole write, and nothing is written. On a write made on a user's behalf, both principals are asked, as on a by-id write. + +**BREAKING** for a caller that relied on that shape to write details under a master it may not edit. Shipped as `minor` under the launch-window convention for accept-set narrowings. + +- **Before.** That shape of write changed or removed details under a master the caller may not edit, bounded only by what the caller could read and, for an organization member, by the platform ownership floor. +- **Now.** It answers the refusal a by-id write of the same detail answers: `PERMISSION_DENIED` (403), naming the detail and the master-edit reason. A write whose rows all sit under masters the caller may edit lands as before and writes the same rows. A row the caller cannot read is not judged and is not written. +- **Also refused.** A caller whose read of the detail is itself refused gets that read's refusal for this shape of write, because the masters of rows it cannot read cannot be established. A write reaching more than 10,000 readable rows answers `INVALID_FILTER` (400), the refusal the same count already answers. +- **Unchanged.** Inserts and by-id updates and deletes; system contexts; the detail's read scope; the platform ownership floor; and every object that does not declare `controlled_by_parent`. + +**What to do.** Grant the caller edit access on the master, or narrow the write to details under masters the caller may edit. From 033fc75a09f8205f05a060cd29b05e449c9afea0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 03:48:29 +0000 Subject: [PATCH 4/5] docs(plugin-security): two docblock sentences describe the master-edit gate as it now answers every write shape Comment-only: the RlsFilterOptions coverage-vouch bullet and step 2.7's superset parenthetical no longer say the gate returns early when no single row is addressed. No code token changes. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- .../plugins/plugin-security/src/security-plugin.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index cdc1192e522..8c460c1b37f 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -485,8 +485,11 @@ interface RlsFilterOptions { * the walk covers that master's write is stated once, at that call site's * `masterGateCoversThisWrite` bullet — not restated here; * • the BULK write path does NOT, and must not: with no single id, - * `assertControlledByParentWrite` returns early and nothing replaces the - * floor. Reading the posture alone here would have widened bulk writes on + * `assertControlledByParentWrite` judges every master the write reaches + * and refuses on the first the caller may not edit, but that is a + * verdict, not a row scope, so the floor stays — dropping it here would + * widen the rows a member writes, a separate decision. Reading the + * posture alone here would have widened bulk writes on * every `controlled_by_parent` detail to "any row under a READABLE * master", which no gate had authorized; * • `getReadFilter` and `checkAuthoredRowWrite` do not, and need not: the @@ -3071,7 +3074,8 @@ export class SecurityPlugin implements Plugin { // master gate (step 2.8, immediately below) run on the same middleware // pass, and 2.8's condition is a strict SUPERSET of this one: the same // `permissionSets.length > 0 && userId && this.ql` triple, the same - // single target id (`extractSingleId`, or it returns early), and one + // single target id (`extractSingleId`; with none, 2.8 judges every + // master the write reaches instead), and one // additional operation (`insert`, which has no pre-image and so no // floor to drop). Reaching this line therefore PROVES the master gate // will answer the same write — the maintainer's precondition, held From e71bf1e5764a9e4c0715bb773551ad0cda59dff1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 04:31:16 +0000 Subject: [PATCH 5/5] chore(changeset): scope the plugin-security headline and ADR-0087 note to update and delete Changeset text only; no code, no test. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude --- .changeset/25012-cbp-write-shape-master-edit.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/25012-cbp-write-shape-master-edit.md b/.changeset/25012-cbp-write-shape-master-edit.md index 86071ec07d8..04181a401ac 100644 --- a/.changeset/25012-cbp-write-shape-master-edit.md +++ b/.changeset/25012-cbp-write-shape-master-edit.md @@ -2,11 +2,11 @@ '@objectstack/plugin-security': minor --- -fix(plugin-security)!: every write to a `controlled_by_parent` detail asks for edit access to its master (ADR-0055 §3) +fix(plugin-security)!: an update or delete of a `controlled_by_parent` detail asks for edit access to its master on every write shape (ADR-0055 §3) Clause-②: no (narrowing) - + ADR-0055 §3 says editing or deleting a detail requires edit access to its master, and it names no write shape as exempt. The master-detail write gate decided one shape of write to a `controlled_by_parent` detail without asking that question. It now asks it: the gate collects the masters of the detail rows the write would reach, as the caller's own read returns them, and runs on each master the check a by-id write runs. One master the caller may not edit refuses the whole write, and nothing is written. On a write made on a user's behalf, both principals are asked, as on a by-id write.