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/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)", 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).