From 36061da89c4ecf43c2092ff82e938a5366a30042 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 18:15:24 +0000 Subject: [PATCH 1/2] feat(security): fail-closed authored-row-write verdict on ISecurityService (#5493 step 1) (#6735) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ISecurityService` gains an optional, verdict-shaped, by-id method `checkAuthoredRowWrite(object, recordId, operation, context)` returning `'admit' | 'abstain'`, plus `AuthoredRowWriteVerdict` / `AuthoredRowWriteOperation`. The question it answers is the one no existing surface could: does an APP-AUTHORED row-level policy admit this row for this write, on its own, with the platform's ownership floor taken out by PROVENANCE? Every other method reports the COMPOSED RLS verdict, and inside that composition sits the platform's own wildcard write floor (`created_by == current_user.id`, on the `member_default` baseline every authenticated member resolves). #5493 probe E-A measured the gap: a creator who is no longer the owner is admitted by the floor and refused by sharing with a byte-identical envelope, so a deferral keyed on the composed answer hands transferred records back to their former creators. Implementation reuses the #6684 provenance split — the SAME `computeLayeredRlsFilter` the middleware enforces with, driven by the SAME `dropPlatformOwnershipFloor` knob. No second RLS evaluator. A null Layer 1 is read as `abstain` (nothing authored is gating the write), and Layer 0 stays AND-ed in so the tenant wall holds. Fail-closed in the `abstain` direction throughout: the method is OPTIONAL, so a deployment without it behaves byte-for-byte as today; a principal-less or on-behalf-of context, an unresolvable probe and a thrown lookup all return `abstain`, and nothing throws outward. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --- .changeset/authored-row-write-verdict.md | 41 ++ .../src/authored-row-write-verdict.test.ts | 479 ++++++++++++++++++ .../plugin-security/src/security-plugin.ts | 122 ++++- .../src/contracts/security-service.test.ts | 85 +++- .../spec/src/contracts/security-service.ts | 108 ++++ 5 files changed, 833 insertions(+), 2 deletions(-) create mode 100644 .changeset/authored-row-write-verdict.md create mode 100644 packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts diff --git a/.changeset/authored-row-write-verdict.md b/.changeset/authored-row-write-verdict.md new file mode 100644 index 0000000000..9e1adabfdb --- /dev/null +++ b/.changeset/authored-row-write-verdict.md @@ -0,0 +1,41 @@ +--- +'@objectstack/spec': minor +'@objectstack/plugin-security': minor +--- + +security: add a fail-closed authored-row-write verdict to `ISecurityService` + +`ISecurityService` gains an optional, verdict-shaped, by-id method: + +```ts +checkAuthoredRowWrite?( + object: string, + recordId: string, + operation: AuthoredRowWriteOperation, // 'update' | 'delete' + context?: SecurityContext, +): Promise< AuthoredRowWriteVerdict >; // 'admit' | 'abstain' +``` + +It answers one question no existing surface could: does an **app-authored** +row-level security policy admit this row for this write, on its own, with the +platform's ownership floor taken out by provenance? + +Every other method reports the **composed** RLS verdict, and sitting inside that +composition is the platform's own wildcard write floor (`created_by == +current_user.id`, shipped on the `member_default` baseline every authenticated +member resolves additively). So "the composed RLS admits this row" is true for +the row's CREATOR whether or not any app policy mentions it — which makes it a +measurably different question, not a cheaper spelling of the same one. A caller +deferring to the composed answer would hand transferred records back to their +former creators. + +`admit` iff at least one applicable, non-floor policy matches the row for the +operation. `abstain` in every other case — no authored policy, no match, an +unreadable or cross-tenant row, a principal-less or on-behalf-of context, or any +internal failure. The method never throws outward, and it is **optional**: a +deployment whose security service omits it behaves byte-for-byte as before, +because callers feature-detect and read absence as `abstain`. + +`@objectstack/plugin-security` implements it on the registered `security` +service, reading the verdict off the same layered RLS computation the middleware +enforces with — no second RLS evaluator. diff --git a/packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts b/packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts new file mode 100644 index 0000000000..9fc77044d5 --- /dev/null +++ b/packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts @@ -0,0 +1,479 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#5493 step 1] `ISecurityService.checkAuthoredRowWrite` — "does an APP-AUTHORED +// row-level policy admit this row for this write, on its own?" +// +// The whole reason this method exists rather than the caller asking a cheaper +// question is a MEASURED hole, and the cases below are that measurement turned +// into pins. +// +// #5493's fix needs the sharing middleware to defer, before hard-refusing a +// by-id write, to "a declared, app-authored RLS update-widener admits this row". +// The obvious proxy — "the COMPOSED RLS admits this row" — is not a cheaper +// spelling of that question. Sitting inside the composed answer is the +// platform's OWN wildcard write floor (`owner_only_writes` / +// `owner_only_deletes`, `created_by == current_user.id`), shipped on +// `member_default`, the additive baseline every authenticated member resolves. +// So the composed answer is `true` for a row's CREATOR whether or not any app +// policy mentions the row at all. +// +// Probe E-A (#5493 comment 5226364929) is where that costs something real: a +// **creator who is no longer the owner** — a record transferred away from them — +// is admitted by the platform floor and refused by sharing with an envelope +// byte-identical to #5493's own. A deferral keyed on the composed answer would +// hand transferred records back to their former creators. +// +// `TRANSFERRED_UPDATE_IS_ADMITTED_BY_THE_COMPOSED_PATH` below is not decoration: +// it drives the REAL middleware and proves the composed path really does admit +// the very row this method abstains on. Without it the E-A pins would be +// asserting `abstain` against a row nothing admits, which is no pin at all. +// +// Everything here runs the REAL SecurityPlugin, the REAL RLS compiler and the +// REAL `member_default` seed — the floor has to be the shipped one, because the +// separation under test is PROVENANCE (`platform-ownership-policies.ts`), not +// shape: an app policy spelling the identical predicate is app-authored and must +// keep counting. +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +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 ─────────────────────────────────────────────────────────────── + +/** + * An ORDINARY tenant business object: `sharingModel: 'private'` but NO + * `access.default: 'private'`. That omission is load-bearing — it is what + * withholds the ADR-0066 ① Layer-1 superuser short-circuit, so the platform + * ownership floor is really in play for every principal here. + */ +const OPPORTUNITY_SCHEMA = { + name: 'crm_opportunity', + sharingModel: 'private', + fields: { + id: { name: 'id' }, + name: { name: 'name' }, + next_step: { name: 'next_step' }, + stage: { name: 'stage' }, + owner_id: { name: 'owner_id' }, + created_by: { name: 'created_by' }, + organization_id: { name: 'organization_id' }, + }, +}; + +/** + * A PRIVATE-posture object — `access.default: 'private'` — which is exactly the + * posture that ENABLES the ADR-0066 ① superuser short-circuit. Used for the one + * case that pins "Layer 1 came back null" as an abstention rather than an + * admission. + */ +const VAULT_SCHEMA = { + name: 'sys_vault_entry', + sharingModel: 'private', + access: { default: 'private' }, + fields: { + id: { name: 'id' }, + label: { name: 'label' }, + stage: { name: 'stage' }, + created_by: { name: 'created_by' }, + organization_id: { name: 'organization_id' }, + }, +}; + +const SCHEMAS: Record = { + crm_opportunity: OPPORTUNITY_SCHEMA, + sys_vault_entry: VAULT_SCHEMA, +}; + +/** The platform seed under test — the source of `owner_only_writes/deletes`. */ +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; + +/** Ordinary CRUD, no authored RLS at all, no bypass of any kind. */ +const CRM_REP: PermissionSet = PermissionSetSchema.parse({ + name: 'crm_rep', + objects: { + crm_opportunity: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, +}); + +/** + * The same profile PLUS an app-authored RLS update-widener: "anyone may update + * an opportunity still in `prospecting`". Declared for `update` ONLY — the verb + * boundary case below reads that directly. + */ +const CRM_REP_WIDENED: PermissionSet = PermissionSetSchema.parse({ + name: 'crm_rep_widened', + objects: { + crm_opportunity: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, + rowLevelSecurity: [ + { + name: 'app_open_stage_updates', + object: 'crm_opportunity', + operation: 'update', + using: "stage == 'prospecting'", + }, + ], +}); + +/** + * A Modify-All profile on the PRIVATE-posture object, carrying an authored + * policy so the provenance pre-check passes and the case really reaches the + * compiler — where the superuser short-circuit then withholds Layer 1. + */ +const VAULT_ADMIN: PermissionSet = PermissionSetSchema.parse({ + name: 'vault_admin', + objects: { + sys_vault_entry: { + allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, + viewAllRecords: true, modifyAllRecords: true, + }, + }, + rowLevelSecurity: [ + { + name: 'app_vault_open_stage', + object: 'sys_vault_entry', + operation: 'update', + using: "stage == 'prospecting'", + }, + ], +}); + +const PERMISSION_SETS: PermissionSet[] = [MEMBER_DEFAULT, CRM_REP, CRM_REP_WIDENED, VAULT_ADMIN]; + +// ── rows ─────────────────────────────────────────────────────────────────── + +const U_CREATOR = 'u_creator'; +const U_OTHER = 'u_other'; +const U_VAULT_ADMIN = 'u_vault_admin'; + +/** + * ⭐ The E-A row. Created by `U_CREATOR`, since TRANSFERRED to `U_OTHER`, and in + * a stage the authored widener does NOT name. So: + * • the platform floor matches it (`created_by == U_CREATOR`) + * • the authored policy does not (`stage != 'prospecting'`) + * An implementation that reads the composed RLS answer says "admitted"; the + * ruled semantics says `abstain`. + */ +const OPP_TRANSFERRED = { + id: 'opp_transferred', name: 'Transferred', next_step: 'call', stage: 'closed_won', + owner_id: U_OTHER, created_by: U_CREATOR, organization_id: 'org1', +}; + +/** + * The positive control: nobody's creation, nobody's property, but in the stage + * the authored policy names — so the ONLY thing that can admit it is the + * app-authored declaration. `admit` here cannot come from the floor. + */ +const OPP_OPEN = { + id: 'opp_open', name: 'Open', next_step: 'call', stage: 'prospecting', + owner_id: U_OTHER, created_by: U_OTHER, organization_id: 'org1', +}; + +/** Same shape as OPP_OPEN but in ANOTHER tenant — the Layer 0 case. */ +const OPP_OTHER_TENANT = { + id: 'opp_other_tenant', name: 'Elsewhere', next_step: 'call', stage: 'prospecting', + owner_id: U_OTHER, created_by: U_OTHER, organization_id: 'org2', +}; + +/** The private-posture row for the superuser short-circuit case. */ +const VAULT_ROW = { + id: 'vault_1', label: 'secret', stage: 'prospecting', + created_by: U_OTHER, organization_id: 'org1', +}; + +// ── in-memory engine ─────────────────────────────────────────────────────── + +function makeEngine(opts: { findOneThrows?: boolean } = {}) { + const tables: Record = { + crm_opportunity: [{ ...OPP_TRANSFERRED }, { ...OPP_OPEN }, { ...OPP_OTHER_TENANT }], + sys_vault_entry: [{ ...VAULT_ROW }], + }; + const matches = (row: any, filter: any): boolean => { + if (!filter || typeof filter !== 'object') return true; + if (Array.isArray(filter.$or)) return filter.$or.some((f: any) => matches(row, f)); + if (Array.isArray(filter.$and)) return filter.$and.every((f: any) => matches(row, f)); + 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) => SCHEMAS[name], + 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 = {}) { + // The fail-closed case drives the probe itself into a throw — the method + // must swallow it into `abstain` and never reject outward. + if (opts.findOneThrows) throw new Error('engine exploded'); + 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 own dispatch predicate + // (#4550 / #5480 / #6277), never a hand-mirrored guard. + 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 Stack { + security: any; + engine: ReturnType; + /** Drive a by-id UPDATE through the REAL security middleware. */ + update: (object: string, recordId: string, context: any) => Promise<{ ok: boolean; message: string }>; +} + +async function makeStack(engineOpts: { findOneThrows?: boolean } = {}): Promise { + const engine = makeEngine(engineOpts); + const metadata = { + get: async (_type: string, name: string) => SCHEMAS[name] ?? null, + list: async () => PERMISSION_SETS, + }; + let security: any; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata, + // Org scoping active, exactly as a multi-tenant deployment wires it — so + // Layer 0 contributes a real tenant predicate this verdict must preserve. + 'org-scoping': { name: 'org-scoping' }, + }; + 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'); + + const securityMw = engine._middlewares[0]; + return { + security, + engine, + async update(object, recordId, context) { + const opCtx: any = { + object, + operation: 'update', + context: { ...context }, + data: { id: recordId, next_step: 'updated' }, + }; + let reached = false; + try { + await securityMw(opCtx, async () => { + await engine.update(opCtx.object, opCtx.data, opCtx.options); + reached = true; + }); + } catch (e: any) { + return { ok: false, message: String(e?.message ?? e) }; + } + return reached + ? { ok: true, message: 'written' } + : { ok: false, message: 'middleware swallowed the write' }; + }, + }; +} + +/** + * The execution-context shape `resolveAuthzContext` hands the middleware. The + * `org_member` position is not decoration: it is the applicability domain of + * `owner_only_writes` / `owner_only_deletes`, i.e. the platform floor these + * cases have to see. + */ +const ctxFor = (userId: string, ...permissions: string[]) => ({ + userId, tenantId: 'org1', positions: ['org_member'], permissions, +}); + +/** The creator of the transferred row, holding an APP-AUTHORED widener. */ +const CREATOR_WIDENED_CTX = ctxFor(U_CREATOR, 'crm_rep_widened'); +/** The same creator, holding NO authored policy at all. */ +const CREATOR_PLAIN_CTX = ctxFor(U_CREATOR, 'crm_rep'); +/** A stranger holding the authored widener. */ +const OUTSIDER_WIDENED_CTX = ctxFor('u_outsider', 'crm_rep_widened'); +const VAULT_ADMIN_CTX = ctxFor(U_VAULT_ADMIN, 'vault_admin'); + +// ─────────────────────────────────────────────────────────────────────────── + +describe('[#5493] checkAuthoredRowWrite is registered on the security service', () => { + it('is present as a function, so a consumer feature-detecting it finds it', async () => { + const stack = await makeStack(); + expect(typeof stack.security.checkAuthoredRowWrite).toBe('function'); + }); +}); + +describe('[#5493 probe E-A] the platform ownership floor is NOT an authored admission', () => { + let stack: Stack; + beforeEach(async () => { stack = await makeStack(); }); + + it('TRANSFERRED_UPDATE_IS_ADMITTED_BY_THE_COMPOSED_PATH — the control that gives E-A its teeth', async () => { + // The creator of a since-transferred row still passes the REAL by-id write + // pre-image gate, because the platform floor (`created_by == + // current_user.id`) matches. This is the composed answer a naive deferral + // would key on — measured here, not assumed, so the `abstain` pins below + // are known to be refusing something that is genuinely on offer. + const out = await stack.update('crm_opportunity', OPP_TRANSFERRED.id, CREATOR_WIDENED_CTX); + expect(out.ok, out.message).toBe(true); + }); + + it('⭐ a creator-but-no-longer-owner whose authored policy does NOT match the row → abstain', async () => { + // The E-A hole itself. The floor admits (proved above); the app policy + // `stage == 'prospecting'` says nothing about a `closed_won` row. Only the + // authored half may count, so the verdict is `abstain` — never `admit`. + await expect( + stack.security.checkAuthoredRowWrite( + 'crm_opportunity', OPP_TRANSFERRED.id, 'update', CREATOR_WIDENED_CTX, + ), + ).resolves.toBe('abstain'); + }); + + it('⭐ a creator holding NO authored policy at all → abstain (nothing could admit by declaration)', async () => { + // Same row, same creator, a profile that authors no RLS whatsoever. The + // floor is the ONLY applicable policy, and a floor is not a declaration. + await expect( + stack.security.checkAuthoredRowWrite( + 'crm_opportunity', OPP_TRANSFERRED.id, 'update', CREATOR_PLAIN_CTX, + ), + ).resolves.toBe('abstain'); + // …and the composed path still admits that same principal on that same row, + // so this case too is refusing something genuinely on offer. + const out = await stack.update('crm_opportunity', OPP_TRANSFERRED.id, CREATOR_PLAIN_CTX); + expect(out.ok, out.message).toBe(true); + }); +}); + +describe('[#5493] an app-authored policy that DOES match is the one thing that admits', () => { + let stack: Stack; + beforeEach(async () => { stack = await makeStack(); }); + + it('admit — a stranger to the row, admitted purely by the declared widener', async () => { + // Neither owner nor creator of OPP_OPEN, so the floor contributes nothing: + // an `admit` here can only be the app-authored `stage == 'prospecting'`. + await expect( + stack.security.checkAuthoredRowWrite( + 'crm_opportunity', OPP_OPEN.id, 'update', OUTSIDER_WIDENED_CTX, + ), + ).resolves.toBe('admit'); + }); + + it('abstain — the same policy, the same row, the OTHER verb (a widener widens rows, not verbs)', async () => { + // `app_open_stage_updates` is declared `operation: 'update'`. Delete is not + // in its applicability domain, so nothing authored applies at all. + await expect( + stack.security.checkAuthoredRowWrite( + 'crm_opportunity', OPP_OPEN.id, 'delete', OUTSIDER_WIDENED_CTX, + ), + ).resolves.toBe('abstain'); + }); + + it('abstain — the matching row in ANOTHER tenant (Layer 0 stays AND-ed in)', async () => { + // OPP_OTHER_TENANT satisfies `stage == 'prospecting'` exactly as OPP_OPEN + // does. Only the tenant wall separates them, and this surface must not be + // the one place in the plugin that answers across it. + await expect( + stack.security.checkAuthoredRowWrite( + 'crm_opportunity', OPP_OTHER_TENANT.id, 'update', OUTSIDER_WIDENED_CTX, + ), + ).resolves.toBe('abstain'); + }); + + it('abstain — a superuser short-circuit is not an authored admission either', async () => { + // ADR-0066 ①: on a PRIVATE posture a Modify-All holder skips business RLS + // wholesale, so Layer 1 comes back null even though the caller authors a + // policy that would match this row. Null Layer 1 means "no authored + // predicate is gating this write" — reading it as `admit` would re-open + // E-A from the other side, on the very objects that need it least. + await expect( + stack.security.checkAuthoredRowWrite( + 'sys_vault_entry', VAULT_ROW.id, 'update', VAULT_ADMIN_CTX, + ), + ).resolves.toBe('abstain'); + }); +}); + +describe('[#5493] fail-closed: every failure is `abstain`, and nothing throws outward', () => { + it('a probe that THROWS resolves to abstain rather than rejecting', async () => { + // The method is consumed by a gate composing a refusal; a rejection there + // would be an outage, and a rejection read as "no opinion" would be a + // widening. It returns a value. + const stack = await makeStack({ findOneThrows: true }); + await expect( + stack.security.checkAuthoredRowWrite( + 'crm_opportunity', OPP_OPEN.id, 'update', OUTSIDER_WIDENED_CTX, + ), + ).resolves.toBe('abstain'); + }); + + it('an unknown record id → abstain', async () => { + const stack = await makeStack(); + await expect( + stack.security.checkAuthoredRowWrite( + 'crm_opportunity', 'no_such_row', 'update', OUTSIDER_WIDENED_CTX, + ), + ).resolves.toBe('abstain'); + }); + + it('a principal-less context → abstain', async () => { + const stack = await makeStack(); + await expect( + stack.security.checkAuthoredRowWrite('crm_opportunity', OPP_OPEN.id, 'update', {}), + ).resolves.toBe('abstain'); + }); + + it('an on-behalf-of context → abstain (ADR-0090 D10: no delegator intersection on this path)', async () => { + // Same fail-closed stance `hasWriteBypass` and `resolveWriteScope` take on + // a delegated context: an answer computed here would be resolved against + // the wrong identity, so there is no answer. + const stack = await makeStack(); + await expect( + stack.security.checkAuthoredRowWrite('crm_opportunity', OPP_OPEN.id, 'update', { + ...OUTSIDER_WIDENED_CTX, + onBehalfOf: { userId: 'u_delegator' }, + }), + ).resolves.toBe('abstain'); + }); + + it('an unknown object → abstain', async () => { + const stack = await makeStack(); + await expect( + stack.security.checkAuthoredRowWrite('no_such_object', 'r1', 'update', OUTSIDER_WIDENED_CTX), + ).resolves.toBe('abstain'); + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 05c97e2089..ca83fc562b 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -56,6 +56,8 @@ import { type IRlsMembershipResolver, type ISecurityService, type SharingWriteVerdict, + type AuthoredRowWriteVerdict, + type AuthoredRowWriteOperation, } from '@objectstack/spec/contracts'; import { matchesFilterCondition } from '@objectstack/formula'; import { FieldMasker } from './field-masker.js'; @@ -736,6 +738,20 @@ export class SecurityPlugin implements Plugin { return 'own'; } }, + // [#5493 / ADR-0105 D3] Authored-row-write evidence: does an + // APP-AUTHORED (non-floor) RLS policy admit this row for this write, + // with the platform's `created_by` ownership floor taken out by + // provenance? The composed RLS answer cannot stand in for it — the + // floor admits every row's CREATOR, so a caller deferring to "composed + // RLS admits" hands transferred records back to their former creators + // (#5493 probe E-A). Verdict-shaped and fail-closed to `abstain`; see + // the method for why a null Layer 1 is an abstention. + checkAuthoredRowWrite: ( + object: string, + recordId: string, + operation: AuthoredRowWriteOperation, + context?: any, + ) => this.checkAuthoredRowWrite(object, recordId, operation, context), // [ADR-0046 §6.7] Effective permission-set NAMES for a caller — the // primitive the REST read layer needs to evaluate a permission-set- // gated book/doc audience ({ permissionSet: '…' }). Same resolution @@ -796,7 +812,7 @@ export class SecurityPlugin implements Plugin { this.getMetadataReadableFields(object, context), }); ctx.registerService('security', registeredSecurityService); - ctx.logger.info('[security] registered "security" service (getReadFilter, getReadableFields, getMetadataReadableFields, canExport, explain, audience-binding suggestions) — ADR-0021 D-C / ADR-0090 D5/D6/D9 / ADR-0106 D7 / #3544 / #3547'); + ctx.logger.info('[security] registered "security" service (getReadFilter, getReadableFields, getMetadataReadableFields, canExport, checkAuthoredRowWrite, explain, audience-binding suggestions) — ADR-0021 D-C / ADR-0090 D5/D6/D9 / ADR-0106 D7 / #3544 / #3547 / #5493'); } catch (e) { ctx.logger.warn?.('[security] failed to register "security" service', { error: (e as Error).message, @@ -2592,6 +2608,110 @@ export class SecurityPlugin implements Plugin { } } + /** + * [#5493 / ADR-0105 D3] `ISecurityService.checkAuthoredRowWrite` — does an + * APP-AUTHORED row-level policy admit this row for this write operation, on + * its own, with the platform's ownership floor taken out? + * + * The question exists because the composed RLS answer cannot stand in for it. + * `member_default` — the additive baseline every authenticated member + * resolves — ships `owner_only_writes` / `owner_only_deletes` + * (`created_by == current_user.id`, see `platform-ownership-policies.ts`), so + * "the composed RLS admits this row" is true for the row's CREATOR whether or + * not any app policy mentions it. #5493's probe E-A measured the gap: a + * creator who is no longer the owner (a record transferred away) is admitted + * by the floor and refused by sharing with a byte-identical envelope, so a + * deferral keyed on the composed answer would hand transferred records back + * to their former creators. Provenance is the only thing that separates the + * two, and it is private to this package by design. + * + * **No second RLS evaluator.** The verdict is read off the SAME + * {@link computeLayeredRlsFilter} the middleware enforces with, driven by the + * SAME `dropPlatformOwnershipFloor` knob #6684 landed for the by-id write + * pre-image gate — the floor is removed by provenance, everything else + * compiles exactly as it would on the enforcement path. Two consequences + * worth naming, because both are load-bearing: + * + * - `layer1 == null` is read as `abstain`, never as "admitted". Layer 1 is + * null precisely when no authored predicate is actually gating this write: + * the applicable set was empty, or the ADR-0066 ① posture-gated superuser + * short-circuit skipped business RLS wholesale. A superuser bypass is not + * an authored admission, and reporting it as one would re-open E-A from + * the other side. (The field-existence net's deny sentinel is NOT null, so + * it flows through the probe and matches nothing — also `abstain`.) + * - Layer 0 (the tenant wall) stays AND-ed in. A row in another tenant is + * admitted by nothing, and dropping the wall here would make this the one + * surface in the plugin that answers across it. + * + * **Fail-closed in the `abstain` direction** — the caller uses `admit` to + * WIDEN, so every failure must be the answer that changes nothing. No throw + * ever escapes: a principal-less context, an on-behalf-of context (ADR-0090 + * D10 — the delegator intersection is not computed on this path, exactly as + * {@link hasWriteBypass} and {@link resolveWriteScope} fail closed on it), an + * unresolvable probe and a thrown lookup all return `abstain`. + * + * The pre-image read is the same `findOne` shape the by-id write gate uses, + * with the caller's own context — so a row the caller cannot READ is not + * "admitted by declaration" here either, which is the non-widening direction + * and matches what the enforcement path already does with the same read. + */ + async checkAuthoredRowWrite( + object: string, + recordId: string, + operation: AuthoredRowWriteOperation, + context?: any, + ): Promise { + try { + if (!object || recordId == null || recordId === '') return 'abstain'; + if (operation !== 'update' && operation !== 'delete') return 'abstain'; + if (!this.ql) return 'abstain'; + // No principal, or a delegated identity this path cannot intersect — + // both are "cannot measure", which is `abstain` (never `admit`). + if (!context?.userId) return 'abstain'; + if (context?.onBehalfOf?.userId) return 'abstain'; + + const permissionSets = await this.resolvePermissionSetsForContext(context); + if (permissionSets.length === 0) return 'abstain'; + + // Cheap provenance pre-check: if the caller holds NO app-authored policy + // applicable to (object, operation), there is nothing that could admit by + // declaration — answer without spending a database round-trip. This is + // the same collection the compiler consumes, filtered by the same + // provenance predicate, so the two cannot disagree about what "authored" + // means. + const authored = this.collectRLSPolicies( + permissionSets, + object, + operation, + (context?.positions ?? []) as string[], + ).filter((p) => !isPlatformOwnershipFloorPolicy(p)); + if (authored.length === 0) return 'abstain'; + + const { layer0, layer1 } = await this.computeLayeredRlsFilter( + permissionSets, + object, + operation, + context, + { dropPlatformOwnershipFloor: true }, + ); + // See the doc above: a null Layer 1 means no authored predicate is + // gating this write, which is an abstention and not an admission. + if (layer1 == null) return 'abstain'; + + const parts = [{ id: recordId }, ...(layer0 ? [layer0] : []), layer1]; + const row = await this.ql.findOne(object, { where: { $and: parts }, context }); + return row ? 'admit' : 'abstain'; + } catch (e) { + this.logger.warn?.( + `[security] checkAuthoredRowWrite could not resolve an authored-policy verdict for ` + + `'${object}' record '${recordId}' (${operation}, user ${context?.userId ?? 'unknown'}) — ` + + `abstaining (fail-closed, #5493)`, + e instanceof Error ? e : new Error(String(e)), + ); + return 'abstain'; + } + } + /** * The read scope for `object` under `context` — the filter the analytics / * raw-SQL path ANDs into its query, being the one surface that bypasses the diff --git a/packages/spec/src/contracts/security-service.test.ts b/packages/spec/src/contracts/security-service.test.ts index 257b30964d..8f5f00f799 100644 --- a/packages/spec/src/contracts/security-service.test.ts +++ b/packages/spec/src/contracts/security-service.test.ts @@ -1,7 +1,11 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import type { ISecurityService } from './security-service'; +import type { + ISecurityService, + AuthoredRowWriteVerdict, + AuthoredRowWriteOperation, +} from './security-service'; /** * These tests pin the two things a consumer of the `security` service reasons @@ -102,6 +106,85 @@ describe('Security Service Contract', () => { expect(typeof partial.getReadableFields).toBe('undefined'); }); + it('[#5493] AuthoredRowWriteVerdict names exactly admit / abstain (compile-time)', () => { + const everyVerdict: AuthoredRowWriteVerdict[] = ['admit', 'abstain']; + // Deliberately NO `deny`. This surface is evidence, not a gate: the caller + // already holds a refusal and asks only whether a declared widener speaks + // for the row. "No evidence" and "evidence against" are the same + // instruction to that caller — keep refusing — so a third state would be + // one nobody could act on differently. + // @ts-expect-error `deny` is not a state this contract defines + const notAVerdict: AuthoredRowWriteVerdict = 'deny'; + expect(everyVerdict).toHaveLength(2); + expect(notAVerdict).toBe('deny'); + + // The operation axis is the RLS WRITE vocabulary, not the engine verb list: + // a caller maps `purge`/`transfer`/`restore` onto its nearest write class + // itself, so a new lifecycle verb cannot acquire a widening path here just + // by being spelled into a wider union. + const everyOperation: AuthoredRowWriteOperation[] = ['update', 'delete']; + // @ts-expect-error `select` is a read class — this surface answers writes only + const notAWriteOperation: AuthoredRowWriteOperation = 'select'; + expect(everyOperation).toHaveLength(2); + expect(notAWriteOperation).toBe('select'); + }); + + it('[#5493] checkAuthoredRowWrite is OPTIONAL — absence is the fail-closed default (compile-time)', () => { + // THE structural pin behind "a deployment without this method behaves + // byte-for-byte as today". Declaring it optional is what makes that a + // property of the TYPE rather than a promise in prose: a security service + // that predates the method still satisfies the contract, and TypeScript + // forces every consumer to handle the absent case instead of calling into + // `undefined` at runtime. + const withoutIt: ISecurityService = makeService(); + expect(typeof withoutIt.checkAuthoredRowWrite).toBe('undefined'); + + // …and a consumer cannot forget: the unguarded call does not compile. + // Never invoked — its only job is to make the COMPILER prove the point + // (invoking it would merely prove that JavaScript throws on `undefined()`, + // which is the runtime symptom this declaration exists to prevent). + const mustNotCompileWithoutAGuard = () => + // @ts-expect-error possibly undefined — a consumer must feature-detect first + withoutIt.checkAuthoredRowWrite('deal', 'r1', 'update', {}); + expect(typeof mustNotCompileWithoutAGuard).toBe('function'); + + // The guarded form is the one that compiles, and it degrades to `undefined` + // — which the caller reads as `abstain` (see the case below). + expect(withoutIt.checkAuthoredRowWrite?.('deal', 'r1', 'update', {})).toBeUndefined(); + }); + + it('[#5493] admit is a positive measurement; every other outcome is abstain', async () => { + // `admit` means "an app-authored, non-floor policy matches this row for + // this operation" — it never means "the write is permitted" (CRUD, the + // tenant wall, sharing and the post-image check all still apply), and it is + // never reported for a reason the implementation did not measure. + const admitting = makeService({ + checkAuthoredRowWrite: async (_object, recordId) => + recordId === 'r_open' ? 'admit' : 'abstain', + }); + await expect(admitting.checkAuthoredRowWrite?.('deal', 'r_open', 'update', { userId: 'u1' })) + .resolves.toBe('admit'); + // The #5493 probe E-A shape: a row a platform-floor policy would admit, but + // no authored policy names. The verdict is `abstain`, never `admit`. + await expect(admitting.checkAuthoredRowWrite?.('deal', 'r_transferred', 'update', { userId: 'u1' })) + .resolves.toBe('abstain'); + + // Fail-closed, and it is the INVERSE of SharingWriteVerdict's: there a + // failed lookup must be `deny` because `abstain` hands the decision on; + // here the caller uses `admit` to WIDEN, so the answer that changes nothing + // is `abstain`. The method returns a verdict rather than throwing. + const failing = makeService({ checkAuthoredRowWrite: async () => 'abstain' }); + await expect(failing.checkAuthoredRowWrite?.('deal', 'r_open', 'update', { userId: 'u1' })) + .resolves.toBe('abstain'); + + // Absence and `abstain` are ONE instruction to the caller, which is what + // lets a consumer collapse feature detection and the verdict into a single + // non-widening branch. + const absent = makeService(); + const verdict = (await absent.checkAuthoredRowWrite?.('deal', 'r_open', 'update', {})) ?? 'abstain'; + expect(verdict).toBe('abstain'); + }); + it('explain accepts a record-scoped request and an explicit target user', async () => { const seen: unknown[] = []; const service = makeService({ diff --git a/packages/spec/src/contracts/security-service.ts b/packages/spec/src/contracts/security-service.ts index 7ef8a494fe..4a542dce38 100644 --- a/packages/spec/src/contracts/security-service.ts +++ b/packages/spec/src/contracts/security-service.ts @@ -34,6 +34,10 @@ * the object schema cannot be resolved it returns `undefined`, meaning * "no answer — use your own fallback", NOT "no fields are readable". An empty * array is a real answer and means the opposite: nothing is readable. + * - **Verdicts fail to ABSTENTION.** {@link ISecurityService.checkAuthoredRowWrite} + * answers a question a composing caller may use to WIDEN, so its failure mode + * is the one that changes nothing: `abstain`. It never reports `admit` for a + * reason it did not measure, and it never throws outward. * * That distinction is load-bearing: the field projection is only ever a * cosmetic narrowing on top of enforcement that already happened (the read path @@ -138,6 +142,52 @@ export interface AudienceBindingSuggestionSync { */ export type AudienceBindingSuggestion = Record; +/** + * [#5493 / ADR-0105 D3] The two-state answer of + * {@link ISecurityService.checkAuthoredRowWrite}. + * + * - `admit` — at least one **applicable, app-authored** row-level security + * policy matches this row for this operation. A positive, measured fact. + * - `abstain` — everything else: no authored policy applies, none of the + * applicable ones matches the row, the probe could not be resolved, or the + * implementation declines to answer. **Never** a statement that the write is + * refused — this surface has no `deny` because it is not a gate. + * + * **Why only two states, and why the missing one is not `deny`.** Its sibling + * `SharingWriteVerdict` (`./sharing-service.js`) is a *gate's* verdict, so it + * needs `deny` to end a decision. This one is an *evidence* + * probe: the caller already holds a refusal and is asking whether a declared, + * app-authored widener speaks for this row before it fires. "No evidence" and + * "evidence against" are the same instruction to that caller — keep your + * refusal — so collapsing them removes a state nobody could act on differently. + * + * **`abstain` is the FAIL-CLOSED direction here, and that is the inverse of + * `SharingWriteVerdict`'s.** There, a failed lookup must be `deny` because + * `abstain` hands the decision on. Here the caller uses `admit` to WIDEN, so + * the answer that changes nothing is `abstain`: a deployment whose security + * service omits {@link ISecurityService.checkAuthoredRowWrite} entirely, or + * whose probe throws, behaves byte-for-byte as one that never asked. Read + * either verdict's fail direction off *what the caller does with it*, never off + * the state's name. + * + * @see ISecurityService.checkAuthoredRowWrite + */ +export type AuthoredRowWriteVerdict = 'admit' | 'abstain'; + +/** + * The row-level WRITE operations {@link ISecurityService.checkAuthoredRowWrite} + * answers for — the RLS write vocabulary, not the engine's verb list. + * + * A caller holding a destructive lifecycle verb maps it onto its nearest write + * class itself (`purge` destroys like `delete`; `transfer` / `restore` mutate + * like `update`), which is the same mapping the engine's own by-id write + * pre-image gate applies before it collects policies. Keeping the mapping on + * the caller's side is deliberate: this contract then names exactly the two + * classes an RLS policy can declare, and a new lifecycle verb cannot silently + * acquire a widening path here by being spelled into a wider union. + */ +export type AuthoredRowWriteOperation = 'update' | 'delete'; + /** * Public contract for the `security` service. * @@ -258,6 +308,64 @@ export interface ISecurityService { context?: SecurityContext, ): Promise<'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org'>; + /** + * [#5493 / ADR-0105 D3] Does an **app-authored** row-level security policy + * admit `recordId` for `operation` — by declaration, on its own, without the + * platform's ownership floor? + * + * The primitive a composing caller needs before it lets a *declared* widener + * defer a hard refusal. `getReadFilter` cannot answer it and neither can any + * composition of the other methods here, because every one of them reports + * the **composed** RLS verdict, and sitting inside that composition is the + * platform's own wildcard write floor (`created_by == current_user.id`, + * shipped on the `member_default` baseline every authenticated member + * resolves additively). Deferring to "the composed RLS admits this row" is + * therefore not a cheaper spelling of this question — it is a measurably + * different one, and the difference is a security hole: + * + * > #5493's probe E-A measured a **creator who is no longer the owner** — + * > a record transferred away from them — being admitted by the platform + * > floor while an authored policy said nothing about the row at all. A + * > deferral keyed on the composed answer hands transferred records back to + * > their former creators. + * + * Separating the two needs policy PROVENANCE (which policies the platform + * shipped vs. which the app declared), and provenance is deliberately private + * to the implementation — an authorable "this is a floor" flag would hand + * authors a switch that turns their own policy off. Hence this method, and + * hence it lives on the service rather than being re-derived by consumers. + * + * **`admit` iff** at least one applicable, **non-floor** policy matches the + * row for this operation. `abstain` in **every** other case, including: + * the caller holds no authored policy for `(object, operation)`; the + * authored policies apply but none matches this row; the row is unreadable, + * absent, or in another tenant; the context carries no principal; the context + * is on-behalf-of (ADR-0090 D10 — the delegator intersection is not computed + * on this path, so an answer here would be resolved against the wrong + * identity); or any internal probe fails. + * + * **Fail-closed by construction, in both halves.** The method itself never + * throws outward — an internal failure becomes `abstain`. And the method is + * OPTIONAL: a deployment whose security service predates it, or omits it, + * behaves byte-for-byte as today, because a caller that cannot find it must + * read the absence as `abstain` too. Callers therefore feature-detect + * (`typeof svc.checkAuthoredRowWrite === 'function'`) and treat every + * non-`admit` outcome identically. + * + * **This is evidence, not authorization.** `admit` says a declared policy + * speaks for this row; it does NOT say the write is permitted — object-level + * CRUD, the tenant wall, sharing, and the post-image `check` clause all still + * apply, and the caller composes this answer with them rather than replacing + * them. Nothing here may be used to *narrow*: `abstain` is "no evidence", + * never "denied". + */ + checkAuthoredRowWrite?( + object: string, + recordId: string, + operation: AuthoredRowWriteOperation, + context?: SecurityContext, + ): Promise; + /** * Explain WHY access is granted or denied — the decision plus the layers that * produced it (permission sets, object permissions, RLS, sharing, field mask). From 0a74774ae137e94dd5bafd33aa2066eef5e18d4d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 23:42:13 +0000 Subject: [PATCH 2/2] chore(spec): regenerate the contracts API-surface snapshot for the new verdict types (#6735) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:api-surface` reported 0 breaking / 2 added — the expected additive product of declaring `AuthoredRowWriteVerdict` and `AuthoredRowWriteOperation` on the security-service contract. Regenerated with `pnpm --filter @objectstack/spec gen:api-surface`; the delta is exactly those two entries. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --- packages/spec/api-surface/contracts.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index cb29295c52..4f51224719 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -43,6 +43,8 @@ "AuthSession (interface)", "AuthSessionApi (interface)", "AuthUser (interface)", + "AuthoredRowWriteOperation (type)", + "AuthoredRowWriteVerdict (type)", "AutomationContext (interface)", "AutomationResult (interface)", "CacheStats (interface)",