From 81dba2597caaec798055662cdb6f0f4b8e90c7d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 13:28:00 +0000 Subject: [PATCH 1/2] fix(plugin-security): explain's record write verdict uses the by-id write path's inputs security/explain answered record.visible: false for update/delete on rows the by-id PATCH/DELETE admits, on private-OWD objects: - M1: Layer 1 kept the platform ownership floor the pre-image gate drops when the sharing write verdict is allow. The gate's floor decision moves into resolvePreImageFloorDrop, and explain's computeLayeredRlsFilter binding asks it for update/delete (behaviour of the gate unchanged). - M2: canEditRecord / canDeleteRecord asked plugin-sharing with the bare context. They now stamp __writeScope from resolveWriteScopeForSharing, overwritten as the middleware's step 2.6 overwrites it. The on-behalf-of path keeps its previous inputs on both halves. Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude --- .../19963-explain-write-verdict-inputs.md | 21 + .../src/explain-write-verdict-inputs.test.ts | 395 ++++++++++++++++++ .../plugin-security/src/security-plugin.ts | 125 ++++-- 3 files changed, 511 insertions(+), 30 deletions(-) create mode 100644 .changeset/19963-explain-write-verdict-inputs.md create mode 100644 packages/plugins/plugin-security/src/explain-write-verdict-inputs.test.ts diff --git a/.changeset/19963-explain-write-verdict-inputs.md b/.changeset/19963-explain-write-verdict-inputs.md new file mode 100644 index 00000000000..758e34f3d9b --- /dev/null +++ b/.changeset/19963-explain-write-verdict-inputs.md @@ -0,0 +1,21 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security): `security/explain` computes a record's `update` / `delete` verdict from the by-id write path's own inputs, so `record.visible` matches what the by-id PATCH / DELETE does (#19963) + +Clause-②: no + +`POST /api/v1/security/explain` with `{ object, operation: 'update' | 'delete', recordId }` answered `decision.record.visible: false` (`decidedBy: 'rls'` or `'sharing'`) on rows that the by-id `PATCH` / `DELETE /api/v1/data/{object}/{id}` then admitted for the same caller. It happened on every object whose OWD is private (set explicitly, or left unset). A console that gates Edit on `record.visible` hid Edit and inline edit from users who were allowed to edit. Two inputs differed from the write path: + +- **The platform ownership floor.** The by-id write gate drops `owner_only_writes` / `owner_only_deletes` (`created_by == current_user.id`) when the sharing service answers `allow` for the row. Explain kept the floor, so it excluded every row the caller did not create. That covers a row shared to them with `edit` access, a row they own but did not create, and a row an `org`-depth writer may edit. +- **The write depth.** The write path hands the sharing service's per-record gate (`canEdit` / `canDelete`) the caller's effective write depth. Explain asked the same gate without it, so a caller with `org` or unit write depth was judged owner-only. + +Explain now asks the same floor decision the write gate asks. It also passes the same write depth to the per-record gate. + +Unchanged: + +- Enforcement: the by-id write gate admits and refuses exactly what it did before. Its floor decision moved into one method that both paths call. +- Reads (`operation: 'read'`) and object-level explanations (no `recordId`). +- A caller acting on behalf of another user (`onBehalfOf`): its record-level write explanation uses the same inputs as before. +- Objects whose OWD is `public_read_write` already matched and still do. diff --git a/packages/plugins/plugin-security/src/explain-write-verdict-inputs.test.ts b/packages/plugins/plugin-security/src/explain-write-verdict-inputs.test.ts new file mode 100644 index 00000000000..95772f2def8 --- /dev/null +++ b/packages/plugins/plugin-security/src/explain-write-verdict-inputs.test.ts @@ -0,0 +1,395 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `security/explain` for a WRITE must be computed from the by-id write path's +// own inputs, so its `record.visible` equals what the by-id PATCH / DELETE does. +// +// The measured divergence: on a private-OWD object, explain answered +// `record.visible: false` (`decidedBy: 'rls'`) for `update` on rows the by-id +// PATCH then ADMITTED for the same caller, through the same stack. Every +// consumer that gates Edit on `record.visible` hid it from permitted users. +// Two independent mechanisms, one per explain dependency: +// +// M1 — the platform ownership floor. The pre-image gate (step 2.7) drops +// `owner_only_writes` / `owner_only_deletes` (`created_by == +// current_user.id`, bound to `org_member`) when the sharing write +// verdict is `allow`. Explain composed Layer 1 with no options, so the +// floor stayed and excluded every row the caller did not CREATE. +// M2 — the write depth. The middleware (step 2.6) stamps `__writeScope` +// before plugin-sharing's `canEdit` / `canDelete` read it. Explain asked +// the same gate with the bare context, so an `org` writer was judged +// owner-only. +// +// This file wires the REAL SecurityPlugin (middleware + explain entry point), +// the REAL SharingService and sharing middleware, and the REAL platform +// `member_default` seed (the floor's source) over one in-memory engine, and +// asserts one property per cell: explain's `record.visible` equals the write's +// outcome. A cell that asserts only one side would pin nothing about the +// agreement this file exists for. +// +// ⚠️ The double's nested reads are not scoped by plugin-sharing's READ filter +// (see `row-write-widener-composition.test.ts`, "WHAT THIS FILE CANNOT SEE"). +// Every cell below whose write is admitted is one whose row the caller can +// also READ on the real stack — `org` read depth, an edit share (which grants +// read), or ownership — so no verdict here rests on the double's looseness. +import { describe, it, expect, vi } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { SharingService, buildSharingMiddleware } from '@objectstack/plugin-sharing'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import type { PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +// ── metadata ─────────────────────────────────────────────────────────────── + +const OBJECT = 'kpi_entry_line'; + +const fieldsOf = (...names: string[]) => + Object.fromEntries(names.map((n) => [n, { name: n }])); + +/** + * The reported object: `sharingModel` UNSET, which ADR-0090 D1 resolves to + * `private` (fail-closed default), and an ordinary access posture, so the + * Layer-1 superuser short-circuit is withheld and the floor is in play. + */ +const PRIVATE_SCHEMA = { + name: OBJECT, + fields: fieldsOf('id', 'name', 'value', 'owner_id', 'created_by', 'organization_id'), +}; + +/** The OWD control: the object's own write model opens row writes. */ +const PUBLIC_RW_SCHEMA = { ...PRIVATE_SCHEMA, sharingModel: 'public_read_write' }; + +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; +const ADMIN_FULL_ACCESS = defaultPermissionSets.find((p) => p.name === 'admin_full_access')!; + +/** Read/write depth `org` — an HR reviewer who may correct any line. */ +const HR_REVIEWER: PermissionSet = PermissionSetSchema.parse({ + name: 'hr_reviewer', + objects: { + [OBJECT]: { + allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, + readScope: 'org', writeScope: 'org', + }, + }, +}); + +/** + * Depth `own` — widened only by ownership or an `edit` share. The DELETE bit + * is granted so a refused delete below is the ROW gate refusing, never CRUD. + */ +const DEPT_REPORTER: PermissionSet = PermissionSetSchema.parse({ + name: 'dept_reporter', + objects: { + [OBJECT]: { + allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, + readScope: 'own', writeScope: 'own', + }, + }, +}); + +const PERMISSION_SETS: PermissionSet[] = [MEMBER_DEFAULT, ADMIN_FULL_ACCESS, HR_REVIEWER, DEPT_REPORTER]; + +// ── principals and rows ──────────────────────────────────────────────────── + +const U_ADMIN = 'u_admin'; +const U_HR = 'u_hr'; +const U_REPORTER = 'u_reporter'; + +/** + * `org_member` is the applicability domain of the platform floor. Every + * member holds it, which is why the floor used to override their grants. + */ +const ctxFor = (userId: string, permission: string, positions: string[] = ['org_member']) => ({ + userId, tenantId: 'org1', positions, permissions: [permission], +}); + +const ADMIN_CTX = ctxFor(U_ADMIN, 'admin_full_access'); +const HR_CTX = ctxFor(U_HR, 'hr_reviewer'); +const REPORTER_CTX = ctxFor(U_REPORTER, 'dept_reporter'); +/** The floor taken out of play by its own domain — isolates M2. */ +const HR_NO_FLOOR_CTX = ctxFor(U_HR, 'hr_reviewer', []); +const REPORTER_NO_FLOOR_CTX = ctxFor(U_REPORTER, 'dept_reporter', []); + +type RowKey = 'shared' | 'owned' | 'unshared'; + +/** Every row is CREATED by the admin, as in the report. */ +const ROWS: Record> = { + /** Owned by the admin; the reporter holds an `edit` share on it. */ + shared: { id: 'kle_shared', name: 'Shared', value: 1, owner_id: U_ADMIN, created_by: U_ADMIN, organization_id: 'org1' }, + /** `owner_id` moved to the reporter; `created_by` is still the admin. */ + owned: { id: 'kle_owned', name: 'Owned', value: 1, owner_id: U_REPORTER, created_by: U_ADMIN, organization_id: 'org1' }, + /** Neither shared with nor owned by the reporter — the negative control. */ + unshared: { id: 'kle_unshared', name: 'Unshared', value: 1, owner_id: U_ADMIN, created_by: U_ADMIN, organization_id: 'org1' }, +}; + +const EDIT_SHARE = { + id: 'shr_edit', object_name: OBJECT, record_id: ROWS.shared.id, + recipient_type: 'user', recipient_id: U_REPORTER, access_level: 'edit', source: 'manual', +}; + +// ── in-memory engine ─────────────────────────────────────────────────────── + +function makeEngine(schema: Record, rowOverrides: Partial>>) { + const tables: Record = { + [OBJECT]: (Object.keys(ROWS) as RowKey[]).map((k) => ({ ...ROWS[k], ...(rowOverrides[k] ?? {}) })), + sys_record_share: [{ ...EDIT_SHARE }], + }; + // `$or` / `$and` conjoin WITH their sibling keys, the way a real driver ANDs + // them (#7620). + const matches = (row: any, filter: any): boolean => { + if (!filter || typeof filter !== 'object') return true; + if (Array.isArray(filter.$or) && !filter.$or.some((f: any) => matches(row, f))) return false; + if (Array.isArray(filter.$and) && !filter.$and.every((f: any) => matches(row, f))) return false; + for (const [k, v] of Object.entries(filter)) { + if (k === '$or' || k === '$and') continue; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(row[k])) return false; + continue; + } + if (row[k] !== v) return false; + } + return true; + }; + const middlewares: any[] = []; + return { + _tables: tables, + _middlewares: middlewares, + registerMiddleware: (mw: any) => middlewares.push(mw), + getSchema: (name: string) => (name === OBJECT ? schema : undefined), + async find(object: string, options: any = {}) { + const rows = (tables[object] ??= []); + return rows.filter((r) => matches(r, options.filter ?? options.where)).slice(0, options.limit ?? 1000); + }, + async findOne(object: string, options: any = {}) { + assertEngineFindOnePredicate(object, options); + const rows = await this.find(object, { ...options, limit: 1 }); + return rows[0] ?? null; + }, + async insert(object: string, data: any) { + (tables[object] ??= []).push({ ...data }); + return data; + }, + // Both write verbs open with the PRODUCER's dispatch predicate (#6277), so + // a call shape `ObjectQL` would refuse fails loudly instead of collecting + // a green from gates that never ran. + async update(object: string, data: any, options?: any) { + const dispatch = assertEngineUpdateDispatch(data, options); + const rows = (tables[object] ??= []); + const targets = dispatch.kind === 'by-id' + ? rows.filter((r) => r.id === dispatch.id) + : rows.filter((r) => matches(r, options?.where)); + for (const r of targets) Object.assign(r, data); + return dispatch.kind === 'by-id' ? (targets[0] ?? null) : targets.length; + }, + async delete(object: string, options?: any) { + const dispatch = assertEngineDeleteDispatch(options); + const rows = (tables[object] ??= []); + const targets = dispatch.kind === 'by-id' + ? rows.filter((r) => r.id === dispatch.id) + : rows.filter((r) => matches(r, options?.where)); + tables[object] = rows.filter((r) => !targets.includes(r)); + return dispatch.kind === 'by-id' ? targets.length > 0 : targets.length; + }, + }; +} + +// ── the stack ────────────────────────────────────────────────────────────── + +interface WriteOutcome { + ok: boolean; + /** ADR-0112 envelope of a refusal — asserted, never a bare `toThrow()`. */ + code?: string; + status?: number; + message: string; +} + +interface Stack { + explain: (operation: 'update' | 'delete', rowKey: RowKey, context: any) => Promise; + write: (operation: 'update' | 'delete', rowKey: RowKey, context: any) => Promise; + row: (rowKey: RowKey) => Record | undefined; +} + +async function makeStack( + opts: { schema?: Record; rows?: Partial>> } = {}, +): Promise { + const schema = opts.schema ?? PRIVATE_SCHEMA; + const engine = makeEngine(schema, opts.rows ?? {}); + const metadata = { + get: async (_type: string, name: string) => (name === OBJECT ? schema : null), + list: async () => PERMISSION_SETS, + }; + let security: any; + let sharing: SharingService; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata, + // Org scoping active, so Layer 0 contributes a real tenant predicate. + 'org-scoping': { name: 'org-scoping' }, + get sharing() { return sharing; }, + }; + const ctx: any = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: (name: string, impl: any) => { if (name === 'security') security = impl; }, + 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); + await plugin.start(ctx); + if (!security) throw new Error('SecurityPlugin did not register the security service'); + + sharing = new SharingService({ engine: engine as any, securityService: () => security }); + const sharingMw = buildSharingMiddleware(sharing, ctx.logger); + const securityMw = engine._middlewares[0]; + const idOf = (rowKey: RowKey) => String(ROWS[rowKey].id); + + return { + explain: (operation, rowKey, context) => + security.explain({ object: OBJECT, operation, recordId: idOf(rowKey) }, { ...context }), + row: (rowKey) => engine._tables[OBJECT].find((r) => r.id === idOf(rowKey)), + async write(operation, rowKey, context) { + const recordId = idOf(rowKey); + const opCtx: any = { + object: OBJECT, + operation, + context: { ...context }, + ...(operation === 'update' + ? { data: { id: recordId, value: 2 } } + : { options: { where: { id: recordId } } }), + }; + let reached = false; + try { + await securityMw(opCtx, async () => { + await sharingMw(opCtx, async () => { + if (operation === 'delete') await engine.delete(opCtx.object, opCtx.options); + else await engine.update(opCtx.object, opCtx.data, opCtx.options); + reached = true; + }); + }); + } catch (e: any) { + // Security's `PermissionDeniedError` carries `statusCode`; plugin-sharing's + // refusal carries `status`. Both are the one ADR-0112 403. + return { ok: false, code: e?.code, status: e?.statusCode ?? e?.status, message: String(e?.message ?? e) }; + } + return reached ? { ok: true, message: 'written' } : { ok: false, message: 'middleware swallowed the write' }; + }, + }; +} + +/** + * One cell: explain FIRST (it must not see the write's effect), then the + * write, then the agreement — and, for an admitted write, the row really + * changed; for a refused one, the ADR-0112 envelope of the refusal, whose + * code names the gate that refused: `PERMISSION_DENIED` is the security + * pre-image gate (step 2.7), `FORBIDDEN` is plugin-sharing's per-record gate. + */ +async function expectCell( + stack: Stack, + operation: 'update' | 'delete', + rowKey: RowKey, + context: any, + expected: boolean, + refusedWith: 'PERMISSION_DENIED' | 'FORBIDDEN' = 'PERMISSION_DENIED', +): Promise { + const decision = await stack.explain(operation, rowKey, context); + const write = await stack.write(operation, rowKey, context); + const cell = `${context.userId} × ${rowKey} × ${operation}`; + expect(write.ok, `${cell}: the by-id write (${write.message})`).toBe(expected); + expect( + decision.record?.visible, + `${cell}: explain record.visible (decidedBy ${decision.record?.decidedBy}) must equal the write's outcome`, + ).toBe(write.ok); + if (expected) { + if (operation === 'update') expect(stack.row(rowKey)?.value, `${cell}: row really changed`).toBe(2); + else expect(stack.row(rowKey), `${cell}: row really gone`).toBeUndefined(); + } else { + expect(write.code, `${cell}: ADR-0112 code`).toBe(refusedWith); + expect(write.status, `${cell}: ADR-0112 status`).toBe(403); + expect(stack.row(rowKey)?.value, `${cell}: row untouched`).toBe(1); + } + return decision; +} + +// ─────────────────────────────────────────────────────────────────────────── + +describe('explain(update) agrees with the by-id PATCH, cell by cell (private OWD)', () => { + const cells: Array<[string, RowKey, any, boolean]> = [ + ['builtin admin', 'shared', ADMIN_CTX, true], + ['hr_reviewer (org depth)', 'shared', HR_CTX, true], + ['hr_reviewer (org depth)', 'owned', HR_CTX, true], + ['hr_reviewer (org depth)', 'unshared', HR_CTX, true], + ['dept_reporter via edit share', 'shared', REPORTER_CTX, true], + ['dept_reporter as owner (not creator)', 'owned', REPORTER_CTX, true], + ['dept_reporter, unshared and not owned', 'unshared', REPORTER_CTX, false], + ]; + it.each(cells)('%s × %s row', async (_label, rowKey, context, expected) => { + await expectCell(await makeStack(), 'update', rowKey, context, expected); + }); + + it('negative control: the refused cell is refused by BOTH gates explain reports', async () => { + const decision = await expectCell(await makeStack(), 'update', 'unshared', REPORTER_CTX, false); + // The floor stays (the sharing verdict is `deny`, so nothing replaces it) + // and the sharing gate refuses: explain names the first of the two. + expect(decision.record.decidedBy).toBe('rls'); + expect(decision.layers.find((l: any) => l.layer === 'sharing').record.outcome).toBe('excluded'); + }); +}); + +describe('explain(delete) agrees with the by-id DELETE, cell by cell (private OWD)', () => { + const cells: Array<[string, RowKey, any, boolean]> = [ + ['hr_reviewer (org depth)', 'unshared', HR_CTX, true], + ['dept_reporter as owner (not creator)', 'owned', REPORTER_CTX, true], + // ADR-0111 D3: an `edit` share widens update, never delete. + ['dept_reporter via edit share', 'shared', REPORTER_CTX, false], + ['dept_reporter, unshared and not owned', 'unshared', REPORTER_CTX, false], + ]; + it.each(cells)('%s × %s row', async (_label, rowKey, context, expected) => { + await expectCell(await makeStack(), 'delete', rowKey, context, expected); + }); +}); + +describe('M1 — the platform floor: move ONLY created_by, the verdict must not move', () => { + // Same principal, same row, same share, same owner. Before the fix the + // not-created-by leg read `false` (`decidedBy: 'rls'`) while PATCH admitted. + it.each([ + ['created by the admin', U_ADMIN], + ['created by the reporter', U_REPORTER], + ])('dept_reporter × shared row, %s', async (_label, createdBy) => { + const stack = await makeStack({ rows: { shared: { created_by: createdBy } } }); + await expectCell(stack, 'update', 'shared', REPORTER_CTX, true); + }); +}); + +describe('M2 — the write depth: explain stamps it the way step 2.6 does', () => { + // Without `org_member` the floor is outside its domain, so only the sharing + // gate's depth input is under test. Before the fix this cell read `false` + // (`decidedBy: 'sharing'`) while PATCH admitted. + it('an org-depth writer is admitted on an unshared, not-owned row', async () => { + await expectCell(await makeStack(), 'update', 'unshared', HR_NO_FLOOR_CTX, true); + }); + + it('a depth the CALLER brings does not decide — explain computes it, as step 2.6 overwrites it', async () => { + // One variable: `__writeScope` stamped on the explained context. The + // write path overwrites any such key before plugin-sharing reads it, so + // it refuses; explain must not be widened by it either. + // No floor in play, so the refusal is plugin-sharing's own. + const forged = { ...REPORTER_NO_FLOOR_CTX, __writeScope: 'org' }; + await expectCell(await makeStack(), 'update', 'unshared', forged, false, 'FORBIDDEN'); + }); +}); + +describe("OWD control: `public_read_write` — the object's own model opens writes", () => { + const principals: Array<[string, any]> = [ + ['builtin admin', ADMIN_CTX], + ['hr_reviewer', HR_CTX], + ['dept_reporter', REPORTER_CTX], + ]; + const rows: RowKey[] = ['shared', 'owned', 'unshared']; + const cells = principals.flatMap(([label, context]) => rows.map((r) => [label, r, context] as const)); + it.each(cells)('%s × %s row: both admit', async (_label, rowKey, context) => { + await expectCell(await makeStack({ schema: PUBLIC_RW_SCHEMA }), 'update', rowKey, context, true); + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index be9f98572d0..be403e07a5a 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -2505,33 +2505,17 @@ export class SecurityPlugin implements Plugin { opCtx.operation === 'purge' ? 'delete' : opCtx.operation === 'transfer' || opCtx.operation === 'restore' ? 'update' : opCtx.operation; - // [#5492] Ask the write authority ONLY when the platform floor is - // actually in play for this (principal, object, operation) — no floor, - // nothing to replace, and no reason to spend a sharing probe. - // - // [ADR-0090 D10] The on-behalf-of path is deliberately EXCLUDED. The - // bypass predicate the verdict folds through already fails closed for a - // 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. - const floorApplies = - !delegatorSets && - this.collectRLSPolicies( - permissionSets, - opCtx.object, - rlsOperation, - (opCtx.context?.positions ?? []) as string[], - ).some(isPlatformOwnershipFloorPolicy); - const dropPlatformOwnershipFloor = floorApplies - ? (await this.resolveSharingWriteVerdict( - rlsOperation, - opCtx.object, - String(targetId), - opCtx.context, - permissionSets, - )) === 'allow' - : false; + // [#5492] The floor decision. Its clauses and their reasons (floor in + // play, `allow` only, the on-behalf-of exclusion) live on + // {@link resolvePreImageFloorDrop}, which `security/explain` asks too. + const dropPlatformOwnershipFloor = await this.resolvePreImageFloorDrop( + rlsOperation, + opCtx.object, + String(targetId), + opCtx.context, + permissionSets, + !!delegatorSets, + ); // [#8757] The COVERAGE VOUCH for ADR-0055 details. This gate and the // 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 @@ -4363,6 +4347,38 @@ export class SecurityPlugin implements Plugin { // record path still works, the sharing layer simply reports not_evaluated. const sharing = recordId ? (this.resolveKernelService?.('sharing') as any) : undefined; + // [#19963] A record-grained WRITE verdict is computed from the by-id write + // path's own inputs, so `record.visible` for `update` / `delete` is the + // answer the by-id PATCH / DELETE gives: + // - Layer 1 carries the pre-image gate's floor decision (step 2.7, + // {@link resolvePreImageFloorDrop}). Not `masterGateCoversThisWrite`: + // that knob VOUCHES that ADR-0055's master gate runs after the filter, + // and explain runs no master gate, so it has nothing to vouch with. + // - plugin-sharing's per-record gate is asked with `__writeScope` stamped + // as the middleware stamps it (step 2.6), always overwritten, so an + // `org` / unit writer is not judged owner-only. + // The on-behalf-of path keeps its previous inputs on both halves. The gate + // itself excludes it from the floor decision, and the middleware's + // delegated write depth (the agent-leg intersection, plus a second gate + // call as the delegator) is not modelled on explain's record path. + const delegatedWrite = (c: any): boolean => !!c?.onBehalfOf?.userId; + const recordWriteFloorOptions = async ( + sets: PermissionSet[], + o: string, + engineOp: string, + c: any, + ): Promise => { + if (!recordId || (engineOp !== 'update' && engineOp !== 'delete')) return undefined; + // Fail toward the floor: an unanswerable decision keeps it standing, + // which is what the gate's own verdict does on a failed probe. + const dropPlatformOwnershipFloor = await this.resolvePreImageFloorDrop( + engineOp, o, recordId, c, sets, delegatedWrite(c), + ).catch(() => false); + return { dropPlatformOwnershipFloor }; + }; + const withWriteScope = async (o: string, c: any): Promise => + delegatedWrite(c) ? c : { ...c, __writeScope: await this.resolveWriteScopeForSharing(o, c) }; + return explainAccess( { ql: this.ql, @@ -4390,7 +4406,10 @@ export class SecurityPlugin implements Plugin { }, baselinePermissionSets: this.baselinePermissionSets, // ── record-grained deps (only consulted when recordId is present) ── - computeLayeredRlsFilter: (sets, o, engineOp, c) => this.computeLayeredRlsFilter(sets as any, o, engineOp, c), + computeLayeredRlsFilter: async (sets, o, engineOp, c) => + this.computeLayeredRlsFilter( + sets as any, o, engineOp, c, await recordWriteFloorOptions(sets as any, o, engineOp, c), + ), fetchRecord: async (o: string, rid: string) => { try { const finder = this.ql?.findOne @@ -4416,12 +4435,12 @@ export class SecurityPlugin implements Plugin { ? { listRecordShares: (o: string, rid: string) => sharing.listShares(o, rid, { isSystem: true }) } : {}), ...(sharing && typeof sharing.canEdit === 'function' - ? { canEditRecord: (o: string, rid: string, c: any) => sharing.canEdit(o, rid, c) } + ? { canEditRecord: async (o: string, rid: string, c: any) => sharing.canEdit(o, rid, await withWriteScope(o, c)) } : {}), // [ADR-0111 D3] The narrower delete gate — an edit share opens update // but not delete, so a delete explanation must consult this. ...(sharing && typeof sharing.canDelete === 'function' - ? { canDeleteRecord: (o: string, rid: string, c: any) => sharing.canDelete(o, rid, c) } + ? { canDeleteRecord: async (o: string, rid: string, c: any) => sharing.canDelete(o, rid, await withWriteScope(o, c)) } : {}), }, { object, operation, context: targetContext, recordId }, @@ -4636,6 +4655,52 @@ export class SecurityPlugin implements Plugin { } } + /** + * [#5492] The by-id write pre-image gate's floor decision (step 2.7): does + * the platform ownership floor yield to the declared write authority for + * THIS principal, row and write class? It is the value of + * {@link RlsFilterOptions.dropPlatformOwnershipFloor} on that path. + * + * One method, two readers: the gate, and `security/explain`'s record-grained + * Layer 1 for `update` / `delete` (#19963). Explain used to compose Layer 1 + * with the floor always standing, so it answered `record.visible: false` + * (`decidedBy: 'rls'`) on every row the caller did not CREATE, while the gate + * dropped the floor and the by-id PATCH admitted the same row. + * + * - The write authority is asked ONLY when the floor is actually in play for + * this (principal, object, operation): no floor, nothing to replace, and + * no reason to spend a sharing probe. + * - `allow` alone drops it; `abstain` and `deny` leave it standing + * ({@link resolveSharingWriteVerdict} has the reasons). + * - [ADR-0090 D10] The on-behalf-of path is deliberately EXCLUDED. The + * bypass predicate the verdict folds through already fails closed for a + * 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. + */ + private async resolvePreImageFloorDrop( + rlsOperation: string, + object: string, + recordId: string, + context: any, + permissionSets: PermissionSet[], + delegated: boolean, + ): Promise { + if (delegated) return false; + const floorApplies = this.collectRLSPolicies( + permissionSets, + object, + rlsOperation, + (context?.positions ?? []) as string[], + ).some(isPlatformOwnershipFloorPolicy); + if (!floorApplies) return false; + return ( + (await this.resolveSharingWriteVerdict(rlsOperation, object, recordId, context, permissionSets)) === + 'allow' + ); + } + /** * [#5493 / ADR-0105 D3] `ISecurityService.checkAuthoredRowWrite` — does an * APP-AUTHORED row-level policy admit this row for this write operation, on From aa9013ffb9712f779d8fc11bcb4a39b0f1906c49 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 13:52:13 +0000 Subject: [PATCH 2/2] chore(scripts): record the new explain-write-verdict test's pinned engine doubles node scripts/check-engine-double-contract.mjs --write: three added rows (delete, findOne, update) for the new test file, 0 seams added or lost. Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude --- scripts/engine-double-contract.pinned.json | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index f44d5c7c242..b363a707d9f 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2856,6 +2856,21 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/plugins/plugin-security/src/explain-write-verdict-inputs.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-security/src/explain-write-verdict-inputs.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-security/src/explain-write-verdict-inputs.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-security/src/field-masking-rule.test.ts", "verb": "findOne",