From bcbf621eaebefdd315b4bdbefd43bea5f51a661b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 03:24:42 +0000 Subject: [PATCH 1/9] fix(plugin-security): a USING-only RLS policy now gates the write post-image When no applicable policy declares `check`, the write check compiles each applicable policy's `using` as its check, matching the published RowLevelSecurityPolicySchema.check default. Declared-check composition is unchanged; the platform ownership floor stays a pre-image construct. Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- .../plugin-security/src/rls-compiler.ts | 11 ++- .../plugin-security/src/security-plugin.ts | 86 +++++++++++++++---- 2 files changed, 79 insertions(+), 18 deletions(-) diff --git a/packages/plugins/plugin-security/src/rls-compiler.ts b/packages/plugins/plugin-security/src/rls-compiler.ts index f048998f9d1..8bd29fb9f34 100644 --- a/packages/plugins/plugin-security/src/rls-compiler.ts +++ b/packages/plugins/plugin-security/src/rls-compiler.ts @@ -524,9 +524,16 @@ export class RLSCompiler { for (const policy of policies) { // [ADR-0058 D4] On a WRITE (check) pass, the post-image is validated against - // the `check` clause, defaulting to `using` when omitted. Reads use `using`. + // the `check` clause, defaulting to `using` when the policy declares no + // `check` (the published `RowLevelSecurityPolicySchema.check` default). + // Reads use `using`. The default is REACHED: the write gate + // (`SecurityPlugin.computeWriteCheckFilter`, through its + // `writeCheckPolicies` selector) hands USING-only policies to this pass + // when no applicable policy declares a `check`. "Declares" is + // `policyDeclaresClause`, the same test the selector applies, so a blank + // `check` string defaults here too instead of skipping the policy. const predicate = clause === 'check' - ? ((policy as { check?: string }).check ?? policy.using) + ? (policyDeclaresClause(policy, 'check') ? (policy as { check?: string }).check : policy.using) : policy.using; // A policy that carries no predicate for THIS clause (e.g. a check-only // policy on the `using` read pass) is not applicable here — skip it diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index b476ce432c9..56ca4b50be6 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -829,6 +829,54 @@ function permissionSetPageOrRefuse(rows: unknown, names: readonly string[]): any return rows; } +/** + * [ADR-0058 D4] The policies whose predicate the write post-image check + * compiles, out of the policies that apply to this principal, object and write + * operation (`insert` / `update`, `all` included). + * + * The published contract is `RowLevelSecurityPolicySchema.check`: "defaults to + * USING clause if not specified". That is also PostgreSQL's rule: a policy + * without `WITH CHECK` holds new rows to its `USING`. Before this selector the + * runtime compiled only policies that declared `check`, so a USING-only policy + * never gated an INSERT or an UPDATE's new row. An author writing + * `record.status != 'closed'` could store a closed row they could then not see. + * + * The composition, stated exactly: + * + * 1. **Some applicable policy declares `check`** → only those policies take + * part, OR-combined by `compileFilter`. That is today's result, byte for + * byte. A USING-only sibling in the same set does NOT add its `using` to + * the OR. Under PostgreSQL's permissive-OR composition it would, and that + * could only WIDEN the check. A grant spelled `id != null` would then erase + * every declared `check` beside it. Taking the narrower of the two readings + * is the fail-closed choice: a declared check never admits more than it did. + * 2. **No applicable policy declares `check`** → every applicable policy with a + * `using` takes part, its `using` compiled as its check (`compileFilter` + * with `clause: 'check'` reads `using` for a policy that declares no + * `check`), OR-combined. This is the half that changed: those writes were + * unchecked, and a row outside every applicable `using` is now refused. + * 3. **The platform's ownership floor never takes part in case 2** + * (`owner_only_writes`, recognised by provenance through + * {@link isPlatformOwnershipFloorPolicy}). The floor is a PRE-image + * construct. The by-id write gate lets a declared write authority replace it + * (a record share at edit depth, a `public_read_write` OWD, the + * controlled-by-parent master gate), and each of those writes targets a row + * whose `created_by` is someone else. Defaulting the floor's `using` onto + * the post-image would refuse exactly those writes again, one step later. + * The floor keeps its pre-image enforcement unchanged. An app-authored + * policy with the same predicate is not the floor, and it takes part. + * + * Layers compose as before: this is one principal's check. The delegator's + * check (ADR-0090 D10) and the Layer 0 tenant post-image check are AND-ed with + * it by the caller. The `modifyAllRecords` bypass on private / platform-global + * objects still returns before this selector runs. + */ +function writeCheckPolicies(applicable: RowLevelSecurityPolicy[]): RowLevelSecurityPolicy[] { + const declared = applicable.filter((p) => policyDeclaresClause(p, 'check')); + if (declared.length > 0) return declared; + return applicable.filter((p) => !isPlatformOwnershipFloorPolicy(p) && policyDeclaresClause(p, 'using')); +} + export class SecurityPlugin implements Plugin { name = 'com.objectstack.security'; /** @@ -2886,8 +2934,10 @@ export class SecurityPlugin implements Plugin { // match the resolved FilterCondition against the post-image in-memory // (the single-record backend for the same filter shape, ADR-0058 D6). A // row that fails the check is DENIED (fail closed, D5) — never silently - // written. Scoped to policies that EXPLICITLY declare `check`, so an - // object governed only by `using` is unaffected. + // written. A policy that declares no `check` is held to its `using` + // (the published default), except when a sibling declares one — the + // exact composition, and the ownership-floor exception, live on + // `writeCheckPolicies`. // // ── [#16608] WHICH IMAGE, on an INSERT ──────────────────────────────── // @@ -3037,8 +3087,8 @@ export class SecurityPlugin implements Plugin { // • UPDATE (Finding 1 / BLOCKER): the pre-image check (step 2.7) validates // only that the caller may touch the EXISTING row (old org == A); it // never sees the NEW value. `organization_id` is auto-stamp-insert-only, - // FLS doesn't protect it, server-side `readonly` isn't enforced, and the - // RLS `check` fires only for explicit policies — so a member owning a + // FLS doesn't protect it, server-side `readonly` isn't enforced, and + // business RLS may not govern the column at all — so a member owning a // row R in org A could `update` R with `{organization_id: victim org B}` // and MOVE the row into another tenant, where it becomes visible. This // is a cross-tenant write by any member. @@ -6452,10 +6502,12 @@ export class SecurityPlugin implements Plugin { /** * [ADR-0058 D4] Compile the WRITE `check` predicate for a post-image - * validation. Scoped to applicable policies that EXPLICITLY declare a `check` - * clause — an object governed only by `using` (the pre-image path) yields no - * check filter and is unaffected. The compiled FilterCondition is matched - * against the post-image record by the caller (fail closed). + * validation. Which applicable policies take part is decided by + * {@link writeCheckPolicies}: the ones that declare `check` when any does, + * otherwise every applicable policy's `using` standing in as its check — the + * published `RowLevelSecurityPolicySchema.check` default. The compiled + * FilterCondition is matched against the post-image record by the caller + * (fail closed). */ private async computeWriteCheckFilter( permissionSets: PermissionSet[], @@ -6485,12 +6537,14 @@ export class SecurityPlugin implements Plugin { // either once site 1 had let it past the row gate. The domain still // decides: a non-holder is outside it and the policy still does not apply // to them. - const withCheck = this.collectRLSPolicies( - permissionSets, - object, - operation, - (context?.positions ?? []) as string[], - ).filter((p) => policyDeclaresClause(p, 'check')); + const withCheck = writeCheckPolicies( + this.collectRLSPolicies( + permissionSets, + object, + operation, + (context?.positions ?? []) as string[], + ), + ); if (withCheck.length === 0) return null; // [ADR-0105 D11 / #16607] Stage the app-resolved membership sets on THIS // context before the `check` clause compiles — the same staging the read @@ -6540,8 +6594,8 @@ export class SecurityPlugin implements Plugin { * "is this a tenant object?" field/posture test, same platform-admin posture * exemption, same fail-closed deny sentinel when the context has no active * organization. Only `layer0` is returned — business RLS (`layer1`) is NOT - * applied to the write post-image (that path is governed by explicit `check` - * clauses via {@link computeWriteCheckFilter}). + * applied to the write post-image by this method (that path is governed by + * {@link computeWriteCheckFilter}). */ /** * [ADR-0105 D11] Populate `context.rlsMembership` from the registered From 000e7d7fca2772c7ea48c72cb8526ca76c582ffe Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 03:31:03 +0000 Subject: [PATCH 2/9] test(plugin-security): pin the USING-as-check default on insert and update Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- .../src/rls-check-defaults-to-using.test.ts | 284 ++++++++++++++++++ 1 file changed, 284 insertions(+) create mode 100644 packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts diff --git a/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts b/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts new file mode 100644 index 00000000000..d7b79e49be0 --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts @@ -0,0 +1,284 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0058 D4] A policy that declares no `check` holds the write post-image to + * its `using`, which is what `RowLevelSecurityPolicySchema.check` publishes: + * "defaults to USING clause if not specified". It is also PostgreSQL's rule for + * a policy without `WITH CHECK`. + * + * ## What was measured before the change + * + * On driver-sql, through the real `SecurityPlugin` and engine, a policy carrying + * only `using: "record.status != 'closed'"` did not stop an INSERT of + * `status = 'closed'`. The row was admitted and stored, and it was a row the + * caller could then not read. The write gate compiled only the policies that + * declared `check`, so with none declared it compiled nothing. + * + * ## What this file pins, on both SQL driver families + * + * - the repro: the USING-only policy refuses the INSERT of a row outside it, + * on the ADR-0112 envelope, and stores nothing; it admits a row inside it; + * - the UPDATE new row: the same policy refuses an update that moves the row + * outside it, and the stored row does not move; + * - a tenant-style USING-only policy refuses an INSERT that names another + * organization. The Layer 0 wall is not armed in this harness (no + * organizations plugin), so the refusal can only come from the defaulted + * check; + * - the control: a policy that declares `check` is judged on its `check`, not + * on its `using`, exactly as before; + * - the composition: when any applicable policy declares `check`, a USING-only + * sibling adds nothing to the check (see `writeCheckPolicies` in + * `security-plugin.ts` for why the narrower reading was taken). + * + * Ground truth is read off the table under a system context: "the gate + * refused" and "nothing was stored" are separate facts, and both are asserted. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +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'; + +const OWN_ORG = 'org_a'; +const OTHER_ORG = 'org_b'; + +const OBJECTS = [ + { + name: 'qa_ticket', + label: 'Ticket', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + title: { name: 'title', type: 'text' }, + status: { name: 'status', type: 'text' }, + organization_id: { name: 'organization_id', type: 'text' }, + }, + }, +]; + +/** The repro's predicate, verbatim. */ +const NOT_CLOSED = "record.status != 'closed'"; +/** A tenant-isolation predicate, the shape the triage names. */ +const OWN_TENANT = 'organization_id == current_user.organization_id'; + +type Policy = { name: string; operation: string; using?: string; check?: string }; + +function permissionSet(policies: Policy[]): PermissionSet { + return PermissionSetSchema.parse({ + name: 'qa_writer', + objects: { qa_ticket: { allowRead: true, allowCreate: true, allowEdit: true } }, + rowLevelSecurity: policies.map((p) => ({ object: 'qa_ticket', ...p })), + }); +} + +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; +const SYS_CTX = { isSystem: true, userId: 'usr_system' }; +const CALLER = { + userId: 'usr_a', + email: 'a@e.example', + tenantId: OWN_ORG, + positions: ['writer'], + permissions: ['qa_writer'], + posture: 'MEMBER', +}; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(makeDriver: () => unknown, ps: PermissionSet): Promise { + const engine = new ObjectQL(); + engine.registerDriver(makeDriver() as never, true); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.rls-check-defaults-to-using', + name: 'RLS check defaults to using', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: OBJECTS, + } as never); + await engine.syncSchemas(); + engines.push(engine); + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [MEMBER_DEFAULT, ps], + }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as never); + await plugin.start(ctx as never); + // The expected refusals log at WARN through the engine's own logger. + vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined); + return engine; +} + +const DRIVERS: Array<[string, () => unknown]> = [ + ['driver-sql (better-sqlite3 :memory:)', + () => new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as never)], + ['driver-sqlite-wasm (:memory:)', () => new SqliteWasmDriver({ filename: ':memory:' } as never)], +]; + +interface Outcome { ok: boolean; code?: string; status?: number } + +const attempt = async (run: () => Promise): Promise => { + try { + await run(); + return { ok: true }; + } catch (e) { + const err = e as { code?: string; statusCode?: number; status?: number }; + return { ok: false, code: err.code, status: err.statusCode ?? err.status }; + } +}; + +/** ⚠️ A SINGLE object, never an array: step 3.6 skips a bulk payload. */ +const insert = (engine: ObjectQL, row: Record) => + engine.insert('qa_ticket', { id: 't1', title: 't', ...row } as never, { context: CALLER } as never); + +const storedRow = async (engine: ObjectQL, id = 't1') => { + const rows = (await engine.find('qa_ticket', { where: { id }, context: SYS_CTX } as never)) as Array< + Record + >; + return rows[0]; +}; + +const expectCheckRefusal = (outcome: Outcome) => { + // The ADR-0112 envelope, never a bare throw: a driver raising a raw `Error` + // would satisfy `toThrow()` and prove nothing. + expect(outcome.ok).toBe(false); + expect(outcome.code).toBe('PERMISSION_DENIED'); + expect(outcome.status).toBe(403); +}; + +for (const [driverName, makeDriver] of DRIVERS) { + describe(`a USING-only policy gates INSERT — ${driverName}`, () => { + const usingOnly = () => permissionSet([{ name: 'not_closed', operation: 'all', using: NOT_CLOSED }]); + + it("the repro: status 'closed' is refused on the check envelope, and nothing is stored", async () => { + const engine = await boot(makeDriver, usingOnly()); + expectCheckRefusal(await attempt(() => insert(engine, { status: 'closed' }))); + expect(await storedRow(engine)).toBeUndefined(); + }); + + it("⭐ status 'open' is admitted and stored — the over-fix control", async () => { + const engine = await boot(makeDriver, usingOnly()); + expect((await attempt(() => insert(engine, { status: 'open' }))).ok).toBe(true); + expect((await storedRow(engine))?.status).toBe('open'); + }); + + it('an insert-class USING-only policy gates INSERT the same way', async () => { + const engine = await boot( + makeDriver, + permissionSet([{ name: 'not_closed_insert', operation: 'insert', using: NOT_CLOSED }]), + ); + expectCheckRefusal(await attempt(() => insert(engine, { status: 'closed' }))); + expect(await storedRow(engine)).toBeUndefined(); + }); + }); + + describe(`a USING-only policy gates the UPDATE new row — ${driverName}`, () => { + const seedOpen = (engine: ObjectQL) => + engine.insert('qa_ticket', { id: 't1', title: 't', status: 'open' } as never, { context: SYS_CTX } as never); + + it("moving an in-scope row to 'closed' is refused, and the stored row does not move", async () => { + const engine = await boot( + makeDriver, + permissionSet([{ name: 'not_closed_update', operation: 'update', using: NOT_CLOSED }]), + ); + await seedOpen(engine); + expectCheckRefusal( + await attempt(() => engine.update('qa_ticket', { id: 't1', status: 'closed' } as never, { context: CALLER } as never)), + ); + expect((await storedRow(engine))?.status).toBe('open'); + }); + + it("⭐ an update that stays in scope is admitted — the over-fix control", async () => { + const engine = await boot( + makeDriver, + permissionSet([{ name: 'not_closed_update', operation: 'update', using: NOT_CLOSED }]), + ); + await seedOpen(engine); + const outcome = await attempt(() => + engine.update('qa_ticket', { id: 't1', status: 'pending' } as never, { context: CALLER } as never), + ); + expect(outcome.ok).toBe(true); + expect((await storedRow(engine))?.status).toBe('pending'); + }); + }); + + describe(`a tenant-style USING-only policy refuses a cross-organization INSERT — ${driverName}`, () => { + const tenantOnly = () => permissionSet([{ name: 'own_tenant', operation: 'all', using: OWN_TENANT }]); + + it('an insert naming another organization is refused, and nothing is stored', async () => { + const engine = await boot(makeDriver, tenantOnly()); + expectCheckRefusal(await attempt(() => insert(engine, { status: 'open', organization_id: OTHER_ORG }))); + expect(await storedRow(engine)).toBeUndefined(); + }); + + it("⭐ an insert naming the caller's own organization is admitted", async () => { + const engine = await boot(makeDriver, tenantOnly()); + expect((await attempt(() => insert(engine, { status: 'open', organization_id: OWN_ORG }))).ok).toBe(true); + expect((await storedRow(engine))?.organization_id).toBe(OWN_ORG); + }); + }); + + describe(`a declared check is judged exactly as before — ${driverName}`, () => { + it('the policy is held to its check, not its using', async () => { + const engine = await boot( + makeDriver, + permissionSet([ + { name: 'declared', operation: 'all', using: NOT_CLOSED, check: "record.status != 'archived'" }, + ]), + ); + // Outside `using`, inside `check`: admitted — `check` is the write rule. + expect((await attempt(() => insert(engine, { status: 'closed' }))).ok).toBe(true); + expect((await storedRow(engine))?.status).toBe('closed'); + // Outside `check`: refused. + expectCheckRefusal( + await attempt(() => + engine.insert('qa_ticket', { id: 't2', title: 't', status: 'archived' } as never, { context: CALLER } as never), + ), + ); + expect(await storedRow(engine, 't2')).toBeUndefined(); + }); + + it('a USING-only sibling adds nothing to a declared check (the composition)', async () => { + const engine = await boot( + makeDriver, + permissionSet([ + { name: 'declared', operation: 'insert', check: "record.status != 'archived'" }, + { name: 'using_only', operation: 'insert', using: NOT_CLOSED }, + ]), + ); + // The declared check alone decides: 'closed' passes it, so it is admitted + // even though it is outside the sibling's `using` — today's answer. + expect((await attempt(() => insert(engine, { status: 'closed' }))).ok).toBe(true); + // …and the sibling's `using` does not widen it: 'archived' is inside the + // sibling's `using` and outside the declared check, and it is refused. + expectCheckRefusal( + await attempt(() => + engine.insert('qa_ticket', { id: 't2', title: 't', status: 'archived' } as never, { context: CALLER } as never), + ), + ); + expect(await storedRow(engine, 't2')).toBeUndefined(); + }); + }); +} From 2fdda29c00c1ceaed9d711daa628a96cbfcc0c59 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 03:35:23 +0000 Subject: [PATCH 3/9] fix(plugin-security): the defaulted write check composes the floor the pre-image kept Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- .../plugin-security/src/security-plugin.ts | 93 +++++++++++++++---- 1 file changed, 73 insertions(+), 20 deletions(-) diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 56ca4b50be6..145ddda33b8 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -855,26 +855,37 @@ function permissionSetPageOrRefuse(rows: unknown, names: readonly string[]): any * with `clause: 'check'` reads `using` for a policy that declares no * `check`), OR-combined. This is the half that changed: those writes were * unchecked, and a row outside every applicable `using` is now refused. - * 3. **The platform's ownership floor never takes part in case 2** - * (`owner_only_writes`, recognised by provenance through - * {@link isPlatformOwnershipFloorPolicy}). The floor is a PRE-image - * construct. The by-id write gate lets a declared write authority replace it - * (a record share at edit depth, a `public_read_write` OWD, the - * controlled-by-parent master gate), and each of those writes targets a row - * whose `created_by` is someone else. Defaulting the floor's `using` onto - * the post-image would refuse exactly those writes again, one step later. - * The floor keeps its pre-image enforcement unchanged. An app-authored - * policy with the same predicate is not the floor, and it takes part. + * 3. **The platform's ownership floor takes part in case 2 exactly when the + * pre-image gate kept it** (`owner_only_writes`, recognised by provenance + * through {@link isPlatformOwnershipFloorPolicy}; `keepOwnershipFloor`). + * The by-id write gate lets a declared write authority replace the floor + * (an `allow` from the sharing service, a `public_read_write` OWD, a + * covering controlled-by-parent master gate). Each of those writes targets + * a row whose `created_by` is someone else, so the floor's `using` on the + * post-image would refuse them again one step later; there it stays out. + * Where the pre-image kept it, it stays in: an app-authored USING-only + * update policy is a WIDENER OR-ed with the floor at the pre-image + * (`stage == 'prospecting'`: anyone may edit an open deal), and leaving + * the floor out of the check would turn that widener into a restriction on + * the creator's own rows. With the floor in, the check is the pre-image's + * write-class OR applied to the new row — PostgreSQL's reading. An + * app-authored policy with the floor's predicate is not the floor; it + * always takes part. * * Layers compose as before: this is one principal's check. The delegator's * check (ADR-0090 D10) and the Layer 0 tenant post-image check are AND-ed with * it by the caller. The `modifyAllRecords` bypass on private / platform-global * objects still returns before this selector runs. */ -function writeCheckPolicies(applicable: RowLevelSecurityPolicy[]): RowLevelSecurityPolicy[] { +function writeCheckPolicies( + applicable: RowLevelSecurityPolicy[], + opts: { keepOwnershipFloor: boolean }, +): RowLevelSecurityPolicy[] { const declared = applicable.filter((p) => policyDeclaresClause(p, 'check')); if (declared.length > 0) return declared; - return applicable.filter((p) => !isPlatformOwnershipFloorPolicy(p) && policyDeclaresClause(p, 'using')); + return applicable.filter( + (p) => policyDeclaresClause(p, 'using') && (opts.keepOwnershipFloor || !isPlatformOwnershipFloorPolicy(p)), + ); } export class SecurityPlugin implements Plugin { @@ -2034,6 +2045,13 @@ export class SecurityPlugin implements Plugin { // additive baseline would resurrect access for a non-existent user). let delegatorSets: PermissionSet[] | null = null; let delegatorContext: any = null; + // [ADR-0058 D4] What the by-id pre-image gate (step 2.7) decided about the + // platform ownership floor for THIS write. The post-image check (step + // 3.6) reads it so that a USING-only policy set is held, on the new row, + // to the same write-class policies the pre-image admitted the caller by. + // Null until step 2.7 runs; step 3.6 judges an update post-image only on + // a path where it did (same single id, same guard triple). + let preImageFloorOpts: RlsFilterOptions | null = null; if (permissionSets.length > 0 && opCtx.context?.onBehalfOf?.userId) { const del = await resolveDelegatorContext(this.ql, opCtx.context); if (del.kind === 'missing') { @@ -2534,6 +2552,7 @@ export class SecurityPlugin implements Plugin { // BOTH floors, exactly as before, and that residual is recorded on the // card rather than resolved by a guess. const masterGateCoversThisWrite = !delegatorSets; + preImageFloorOpts = { dropPlatformOwnershipFloor, masterGateCoversThisWrite }; const writeFilter = await this.computeRlsFilter( permissionSets, opCtx.object, @@ -2987,6 +3006,7 @@ export class SecurityPlugin implements Plugin { opCtx.object, opCtx.operation, opCtx.context, + preImageFloorOpts ?? undefined, ); // [ADR-0090 D10] The post-image must satisfy the delegator's CHECK too — // an on-behalf-of write may not produce a row the delegator itself @@ -6008,6 +6028,31 @@ export class SecurityPlugin implements Plugin { * Shared by the engine middleware (read + by-id write pre-image) and * {@link getReadFilter}. Returns `null` when neither layer contributes. */ + /** + * Does the object's own write model replace the platform ownership floor + * for this operation? True for a `public_read_write` OWD on `update` (#8023) + * and for a `controlled_by_parent` detail whose master gate the caller + * vouches covers this write (#8757). The reasons are recorded where + * {@link computeLayeredRlsFilter} applies the drop. + * + * One predicate, two readers: the pre-image Layer 1 composition and the + * post-image check's floor decision (`computeWriteCheckFilter`). Keeping it + * in one place keeps the two from disagreeing about when the floor applies. + */ + private platformFloorYieldsToObjectWriteModel( + meta: { owdOpensRowWrites?: boolean }, + object: string, + operation: string, + opts?: RlsFilterOptions, + ): boolean { + if (meta.owdOpensRowWrites && owdOpenWritesCoversOperation(operation)) return true; + return ( + !!opts?.masterGateCoversThisWrite && + masterGateCoversOperation(operation) && + this.declaresControlledByParent(object) + ); + } + private async computeRlsFilter( permissionSets: PermissionSet[], object: string, @@ -6151,9 +6196,12 @@ export class SecurityPlugin implements Plugin { // that also meant delete was removed for being wider than it. // Layer 0 (the tenant wall) is untouched — a `public_read_write` object is // org-wide open, never cross-tenant open. - if (meta.owdOpensRowWrites && owdOpenWritesCoversOperation(operation)) { - collected = collected.filter((p) => !isPlatformOwnershipFloorPolicy(p)); - } + // + // Both drops below ask ONE predicate, + // {@link platformFloorYieldsToObjectWriteModel}, which the post-image + // check (`computeWriteCheckFilter`) asks too, so the floor a defaulted + // check composes is the floor this pre-image composed. + // // [#8757] An ADR-0055 `controlled_by_parent` detail does not inherit the // platform's wildcard write ownership floor either — for a DIFFERENT // reason from the OWD above, and under one extra condition. @@ -6194,11 +6242,7 @@ export class SecurityPlugin implements Plugin { // (`masterGateCoversOperation`), which is every class the floor ships. // Layer 0 (the tenant wall) is untouched, and so is the read path: the // floor is a write-side construct that never applied to `select`. - if ( - opts?.masterGateCoversThisWrite && - masterGateCoversOperation(operation) && - this.declaresControlledByParent(object) - ) { + if (this.platformFloorYieldsToObjectWriteModel(meta, object, operation, opts)) { collected = collected.filter((p) => !isPlatformOwnershipFloorPolicy(p)); } // [#7665] The write-visibility floor: a write target must be inside the @@ -6514,6 +6558,7 @@ export class SecurityPlugin implements Plugin { object: string, operation: string, context: any, + preImageOpts?: RlsFilterOptions, ): Promise | null> { // [ADR-0066 ①] modifyAllRecords bypasses write-side RLS (incl. the post-image // check) on private/platform-global objects. @@ -6537,6 +6582,13 @@ export class SecurityPlugin implements Plugin { // either once site 1 had let it past the row gate. The domain still // decides: a non-holder is outside it and the policy still does not apply // to them. + // The platform ownership floor takes part in a defaulted check exactly + // when the pre-image gate kept it: the same three replacements drop it here + // (an `allow` from the declared write authority, a `public_read_write` OWD, + // a covering master gate), and nothing else does. + const floorReplaced = + !!preImageOpts?.dropPlatformOwnershipFloor || + this.platformFloorYieldsToObjectWriteModel(meta, object, operation, preImageOpts); const withCheck = writeCheckPolicies( this.collectRLSPolicies( permissionSets, @@ -6544,6 +6596,7 @@ export class SecurityPlugin implements Plugin { operation, (context?.positions ?? []) as string[], ), + { keepOwnershipFloor: !floorReplaced }, ); if (withCheck.length === 0) return null; // [ADR-0105 D11 / #16607] Stage the app-resolved membership sets on THIS From cc9025a124466e65330f07233718a4b3681aeaad Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 03:37:51 +0000 Subject: [PATCH 4/9] test(plugin-security): triage the tenant fixture and pin the floor composition Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- .../src/rls-check-defaults-to-using.test.ts | 54 ++++++++++++++++++- .../src/security-plugin.test.ts | 12 +++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts b/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts index d7b79e49be0..0b0857b19b4 100644 --- a/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts +++ b/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts @@ -58,6 +58,18 @@ const OBJECTS = [ organization_id: { name: 'organization_id', type: 'text' }, }, }, + // An object whose OWD says nothing about writes, so `member_default`'s + // ownership floor (`owner_only_writes`, `created_by == current_user.id`, + // positions `org_member`) applies to an `org_member` caller. + { + name: 'qa_deal', + label: 'Deal', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + stage: { name: 'stage', type: 'text' }, + created_by: { name: 'created_by', type: 'text' }, + }, + }, ]; /** The repro's predicate, verbatim. */ @@ -67,10 +79,13 @@ const OWN_TENANT = 'organization_id == current_user.organization_id'; type Policy = { name: string; operation: string; using?: string; check?: string }; -function permissionSet(policies: Policy[]): PermissionSet { +function permissionSet(policies: Array): PermissionSet { return PermissionSetSchema.parse({ name: 'qa_writer', - objects: { qa_ticket: { allowRead: true, allowCreate: true, allowEdit: true } }, + objects: { + qa_ticket: { allowRead: true, allowCreate: true, allowEdit: true }, + qa_deal: { allowRead: true, allowCreate: true, allowEdit: true }, + }, rowLevelSecurity: policies.map((p) => ({ object: 'qa_ticket', ...p })), }); } @@ -281,4 +296,39 @@ for (const [driverName, makeDriver] of DRIVERS) { expect(await storedRow(engine, 't2')).toBeUndefined(); }); }); + + describe(`a USING-only update widener composes with the ownership floor — ${driverName}`, () => { + // "Anyone may update a deal still open" — an app-authored widener, OR-ed at + // the pre-image with the platform floor `created_by == current_user.id`. + // The defaulted check is that same OR on the new row. + const widener = () => + permissionSet([{ name: 'open_deals', object: 'qa_deal', operation: 'update', using: "stage == 'open'" }]); + const MEMBER = { ...CALLER, positions: ['writer', 'org_member'] }; + const seed = (engine: ObjectQL, id: string, stage: string, createdBy: string) => + engine.insert('qa_deal', { id, stage, created_by: createdBy } as never, { context: SYS_CTX } as never); + const dealStage = async (engine: ObjectQL, id: string) => + ((await engine.find('qa_deal', { where: { id }, context: SYS_CTX } as never)) as Array>)[0] + ?.stage; + + it("⭐ the creator still updates their own deal out of 'open' — the floor stays in the check", async () => { + const engine = await boot(makeDriver, widener()); + await seed(engine, 'd_mine', 'open', MEMBER.userId); + const outcome = await attempt(() => + engine.update('qa_deal', { id: 'd_mine', stage: 'won' } as never, { context: MEMBER } as never), + ); + expect(outcome.ok).toBe(true); + expect(await dealStage(engine, 'd_mine')).toBe('won'); + }); + + it("a non-creator admitted only by the widener cannot move the deal out of 'open'", async () => { + const engine = await boot(makeDriver, widener()); + await seed(engine, 'd_theirs', 'open', 'usr_other'); + expectCheckRefusal( + await attempt(() => + engine.update('qa_deal', { id: 'd_theirs', stage: 'won' } as never, { context: MEMBER } as never), + ), + ); + expect(await dealStage(engine, 'd_theirs')).toBe('open'); + }); + }); } diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index 7fc8e5228ca..dd95311f13a 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -334,13 +334,19 @@ describe('SecurityPlugin', () => { const harness = makeMiddlewareCtx({ permissionSets: [tenantPolicySet] }); await plugin.init(harness.ctx); await plugin.start(harness.ctx); + // The row carries the caller's own organization, as the org-scoping + // auto-stamp writes it in `beforeInsert` ahead of the post-image check. The + // `tenant_isolation` policy declares no `check`, so its `using` is the + // insert check (ADR-0058 D4 default) and a row with no organization would + // be refused. This double runs no hooks, so the payload stands in for them. const opCtx: any = { - object: 'task', operation: 'insert', data: { name: 'A' }, + object: 'task', operation: 'insert', data: { name: 'A', organization_id: 'org-1' }, context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] }, }; await harness.run(opCtx); - // SecurityPlugin no longer touches organization_id — that's plugin-org-scoping's job. - expect(opCtx.data.organization_id).toBeUndefined(); + // SecurityPlugin does not touch organization_id — that's plugin-org-scoping's + // job — so the value arrives exactly as sent. + expect(opCtx.data.organization_id).toBe('org-1'); expect(opCtx.data.owner_id).toBe('u1'); }); From f3fa2cdb78dbe3cd57c4c4a7ecf97466c26eddac Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 04:00:50 +0000 Subject: [PATCH 5/9] test(dogfood): a smuggled re-owner of an API key is refused by the defaulted check Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- .../test/api-key-owner-revoke.dogfood.test.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/qa/dogfood/test/api-key-owner-revoke.dogfood.test.ts b/packages/qa/dogfood/test/api-key-owner-revoke.dogfood.test.ts index 689d6758ecc..7b965a2e918 100644 --- a/packages/qa/dogfood/test/api-key-owner-revoke.dogfood.test.ts +++ b/packages/qa/dogfood/test/api-key-owner-revoke.dogfood.test.ts @@ -250,15 +250,30 @@ describe('#8053: a member revokes their OWN sys_api_key', () => { const originalOwner = before.row.user_id; expect(originalOwner).toBeTruthy(); + // Re-owning the key is refused before the whitelist ever strips it: the + // `sys_api_key_self` policy declares no `check`, so its `using` + // (`user_id == current_user.id`) is the post-image check (ADR-0058 D4 + // default), and a new row owned by someone else fails it. The whole patch, + // `revoked` included, is refused on the ADR-0112 envelope. + const reowned = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { + revoked: true, + user_id: 'usr_someone_else', + }); + expect(reowned.status).toBe(403); + const reownedBody: any = await reowned.json(); + expect(reownedBody.code ?? reownedBody.error?.code).toBe('PERMISSION_DENIED'); + const untouched = await readKey(memberToken, id); + expect(untouched.row.user_id, 're-owning a key is privilege transfer').toBe(originalOwner); + expect(untouched.row.revoked).toBeFalsy(); + const res = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { revoked: true, key: 'forged-hash-value', - user_id: 'usr_someone_else', }); expect(res.status).toBe(200); const after = await readKey(memberToken, id); - expect(after.row.user_id, 're-owning a key is privilege transfer').toBe(originalOwner); + expect(after.row.user_id).toBe(originalOwner); expect(after.row.revoked).toBe(true); // The decisive proof that `key` was stripped: the ORIGINAL secret is still From 19236d91ad40d066e1f808f48620961cf14d6f62 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 04:01:40 +0000 Subject: [PATCH 6/9] chore(changeset): plugin-security holds USING-only policies to their using on writes Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- .changeset/rls-check-defaults-to-using.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .changeset/rls-check-defaults-to-using.md diff --git a/.changeset/rls-check-defaults-to-using.md b/.changeset/rls-check-defaults-to-using.md new file mode 100644 index 00000000000..2841376bc2c --- /dev/null +++ b/.changeset/rls-check-defaults-to-using.md @@ -0,0 +1,19 @@ +--- +"@objectstack/plugin-security": patch +--- + +A row-level security policy that declares no `check` now holds INSERTs and UPDATEs to its `using` (#19942). + +The published contract has always said this. `RowLevelSecurityPolicySchema.check` reads "defaults to USING clause if not specified", and PostgreSQL treats a policy without `WITH CHECK` the same way. The write gate did not do it. It compiled only the policies that declared `check`, so a policy with `using` alone never checked a write. With `using: "record.status != 'closed'"`, a caller could INSERT a closed row, and the row was stored even though the same caller could not then read it. + +**Writes that are now refused.** A single-row INSERT, or a by-id UPDATE, whose resulting row falls outside the `using` of every applicable write-class policy (`insert`, `update` or `all`), when none of those policies declares a `check`. The refusal is the existing row-level CHECK denial: `403 PERMISSION_DENIED`, and nothing is stored. There is no transition switch. If a write should be allowed to leave a policy's scope, declare a `check` on that policy. + +The platform's own permission sets are affected in one way. The `_self` policies with `operation: 'all'` (`user_id == current_user.id`) now refuse a write that sets `user_id` to another user, on the self-service tables a member may write (`sys_user_preference`, and the revoke patch on `sys_api_key`). For example, a member can no longer create a preference row for someone else, or re-own their API key by adding `user_id` to a revoke patch. Before this change, both writes were admitted, although the second one had `user_id` stripped later. + +**What does not change.** + +- If any applicable policy declares `check`, only the declared checks decide, exactly as before. A policy with `using` alone that sits beside them adds nothing to the check. +- The platform's ownership floor (`owner_only_writes`) takes part in a defaulted check only when the by-id write gate kept it for that write. A record share at edit depth, a `public_read_write` object and a covering controlled-by-parent master gate still replace the floor, so those writes are not refused again on the new row. +- `select` policies never gate a write's new row. +- Bulk updates without a single id are still scoped by the `using` where clause and are not checked row by row. +- The `modifyAllRecords` bypass on private and platform-global objects still skips the check. From 62af5581f92a7ef0e365357276ddfe358689f7f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 04:36:22 +0000 Subject: [PATCH 7/9] chore(changeset): declare the USING-as-check default a narrowing (minor, BREAKING, ADR-0087 disposition) Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- .changeset/rls-check-defaults-to-using.md | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/.changeset/rls-check-defaults-to-using.md b/.changeset/rls-check-defaults-to-using.md index 2841376bc2c..4866e1ef947 100644 --- a/.changeset/rls-check-defaults-to-using.md +++ b/.changeset/rls-check-defaults-to-using.md @@ -1,19 +1,28 @@ --- -"@objectstack/plugin-security": patch +"@objectstack/plugin-security": minor --- -A row-level security policy that declares no `check` now holds INSERTs and UPDATEs to its `using` (#19942). +fix(plugin-security)!: a row-level security policy that declares no `check` now holds INSERTs and UPDATEs to its `using` (#19942) -The published contract has always said this. `RowLevelSecurityPolicySchema.check` reads "defaults to USING clause if not specified", and PostgreSQL treats a policy without `WITH CHECK` the same way. The write gate did not do it. It compiled only the policies that declared `check`, so a policy with `using` alone never checked a write. With `using: "record.status != 'closed'"`, a caller could INSERT a closed row, and the row was stored even though the same caller could not then read it. +Clause-②: no (narrowing) -**Writes that are now refused.** A single-row INSERT, or a by-id UPDATE, whose resulting row falls outside the `using` of every applicable write-class policy (`insert`, `update` or `all`), when none of those policies declares a `check`. The refusal is the existing row-level CHECK denial: `403 PERMISSION_DENIED`, and nothing is stored. There is no transition switch. If a write should be allowed to leave a policy's scope, declare a `check` on that policy. + -The platform's own permission sets are affected in one way. The `_self` policies with `operation: 'all'` (`user_id == current_user.id`) now refuse a write that sets `user_id` to another user, on the self-service tables a member may write (`sys_user_preference`, and the revoke patch on `sys_api_key`). For example, a member can no longer create a preference row for someone else, or re-own their API key by adding `user_id` to a revoke patch. Before this change, both writes were admitted, although the second one had `user_id` stripped later. +**BREAKING**: this narrows the set of writes the write gate accepts. A write that is admitted today can be refused after this change. It ships as `minor` under the launch-window convention, the same way the insert-side `check` reorder did (#16805). + +The published contract has always said this. `RowLevelSecurityPolicySchema.check` reads "defaults to USING clause if not specified", and PostgreSQL treats a policy without `WITH CHECK` the same way. The write gate did not do it. It compiled only the policies that declared `check`, so a policy with only a `using` never checked a write. With `using: "record.status != 'closed'"`, a caller could INSERT a closed row. The row was stored even though the same caller could not read it afterwards. + +**Writes that are now refused.** Each refusal is the existing row-level CHECK denial, `403 PERMISSION_DENIED`, and nothing is stored. There is no transition switch. + +- **Any policy with only a `using`.** A single-row INSERT, or a by-id UPDATE, is refused when its resulting row falls outside the `using` of every applicable write-class policy (`insert`, `update` or `all`) and none of those policies declares a `check`. To let a write move a row outside a policy's scope, declare a `check` on that policy. +- **The platform's `_self` policies.** These have `operation: 'all'` and `using: user_id == current_user.id`. They now refuse a write that sets `user_id` to another user, on the self-service tables a member may write: `sys_user_preference`, and the revoke patch on `sys_api_key`. A member can no longer create a preference row for someone else. A member can no longer re-own their API key by adding `user_id` to a revoke patch. Before this change both writes were admitted, and the second one had `user_id` stripped later. +- **Re-pointing `created_by` under the ownership floor.** This is a by-id UPDATE that changes `created_by` while the floor still applies to that write. The readonly strip used to remove the new value, and the update was admitted. It is now refused with 403. The new row is judged before that strip runs (#16790). +- **A `using` that does not compile.** This applies to an `insert` or `all` policy that has only a `using`. That `using` is now also the insert check, and the policy fails closed: every insert it governs is refused. Before this change the insert was admitted, because no check ran. **What does not change.** -- If any applicable policy declares `check`, only the declared checks decide, exactly as before. A policy with `using` alone that sits beside them adds nothing to the check. -- The platform's ownership floor (`owner_only_writes`) takes part in a defaulted check only when the by-id write gate kept it for that write. A record share at edit depth, a `public_read_write` object and a covering controlled-by-parent master gate still replace the floor, so those writes are not refused again on the new row. +- If any applicable policy declares `check`, only the declared checks decide, exactly as before. A policy with only a `using` alongside them adds nothing to the check. +- The platform's ownership floor (`owner_only_writes`) is part of a defaulted check only when the by-id write gate kept it for that write. A record share at edit depth, a `public_read_write` object, or a covering controlled-by-parent master gate still replaces the floor. Those writes are not refused again on the new row. - `select` policies never gate a write's new row. - Bulk updates without a single id are still scoped by the `using` where clause and are not checked row by row. - The `modifyAllRecords` bypass on private and platform-global objects still skips the check. From ecea3b4714045a07589217c5b2934500f4a7cfa1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 04:56:31 +0000 Subject: [PATCH 8/9] fix(plugin-security,plugin-auth): cite the resolving record for the master-gate ruling; pin the post-image own-row read Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- .../src/sys-user-self-service-route.test.ts | 31 +++++++++++++---- .../plugin-security/src/security-plugin.ts | 34 ++++++++++--------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts b/packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts index d1aed3c796f..98d69aa98ae 100644 --- a/packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts +++ b/packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts @@ -125,7 +125,16 @@ interface RouteResult { error: any; /** The payload as it stands after every layer ran (the guard strips in place). */ data: Record; - /** Every `where` the pre-image re-read was called with, in order. */ + /** + * Every `where` the engine was asked to read with, in order. On an admitted + * own-row update there are two: the by-id pre-image re-read (step 2.7, the + * `{id}` address AND-ed with the compiled write-RLS filter) and then the + * caller's own-row read (step 3.6, a bare `{id}`) that the post-image check + * merges the change set onto. That second read exists because + * `sys_user_self` (`operation: 'all'`, `using: id == current_user.id`) + * declares no `check`, so its `using` is the write check (ADR-0058 D4 + * default, `RowLevelSecurityPolicySchema.check`). + */ preImageWheres: unknown[]; /** The secondary-storage writes the ADR-0092 D6 companion hook performed. */ snapshotWrites: Array<{ key: string; value: any }>; @@ -269,8 +278,15 @@ describe('sys_user self-service — the row scope the write actually composes', // the compiled RLS parts. Asserted verbatim because every other assertion in // this file leans on `matchesFilter` reading it correctly, and because it is // the security property in one line: the caller's id, and nothing wider. - expect(r.preImageWheres).toHaveLength(1); + // + // Then exactly one more read: the caller's OWN row by id, which step 3.6 + // merges the change set onto to judge the post-image against the defaulted + // `sys_user_self` check (`id == current_user.id`). It is a read of the row + // already admitted above, never a scope: it cannot widen what the member may + // write, and the check it feeds can only refuse more. + expect(r.preImageWheres).toHaveLength(2); expect(r.preImageWheres[0]).toEqual({ $and: [{ id: ME }, { id: ME }] }); + expect(r.preImageWheres[1]).toEqual({ id: ME }); }); it('the org-peer READ scope does not appear in the write filter', async () => { @@ -285,7 +301,9 @@ describe('sys_user self-service — the row scope the write actually composes', // an object-gate refusal produces exactly that. Pin the write got as far as // composing a filter before asserting what is not in it. expect(r.refusedBy).toBeNull(); - expect(r.preImageWheres).toHaveLength(1); + // The pre-image re-read plus the post-image check's own-row read (see the + // `preImageWheres` field). The absences below hold over BOTH. + expect(r.preImageWheres).toHaveLength(2); expect(JSON.stringify(r.preImageWheres)).not.toContain('$in'); expect(JSON.stringify(r.preImageWheres)).not.toContain(PEER); }); @@ -354,9 +372,10 @@ describe('sys_user self-service — the four pins, each attributed to a layer', // while the guard was dead code. const r = await route(ME, { email: 'attacker@example.com' }); expect(r.refusedBy).toBe('identity-guard'); - // The pre-image re-read ran and SUCCEEDED (the row was visible) — proof the - // first two layers admitted the write before the guard refused it. - expect(r.preImageWheres).toEqual([{ $and: [{ id: ME }, { id: ME }] }]); + // The pre-image re-read ran and SUCCEEDED (the row was visible), and the + // post-image check read the caller's own row and passed — proof the first + // two layers admitted the write before the guard refused it. + expect(r.preImageWheres).toEqual([{ $and: [{ id: ME }, { id: ME }] }, { id: ME }]); // ADR-0112 envelope, both discriminators (the REST boundary derives 403 from // `status`, and `mapDataError` keys on `code`). expect(r.error?.code).toBe('PERMISSION_DENIED'); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 145ddda33b8..17c14f37b95 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -6014,26 +6014,14 @@ export class SecurityPlugin implements Plugin { return row; } - /** - * [ADR-0095 D1] Compute the effective row filter for (object, operation) as - * `Layer0(tenant) AND Layer1(business RLS)`. - * - * - **Layer 0** (tenant isolation) is computed by {@link computeTenantLayer0Filter} - * from the tenancy mode + the object's field set/posture — independent of the - * RLS compiler, always first, unconditionally AND-composed. - * - **Layer 1** (business RLS) is the applicable per-policy compile (ownership, - * depth, sharing, `_self` carve-outs), with the field-existence safety net and - * the posture-gated superuser bypass — which now governs BUSINESS RLS only. - * - * Shared by the engine middleware (read + by-id write pre-image) and - * {@link getReadFilter}. Returns `null` when neither layer contributes. - */ /** * Does the object's own write model replace the platform ownership floor * for this operation? True for a `public_read_write` OWD on `update` (#8023) * and for a `controlled_by_parent` detail whose master gate the caller - * vouches covers this write (#8757). The reasons are recorded where - * {@link computeLayeredRlsFilter} applies the drop. + * vouches covers this write. That ruling landed in PR #8869 (commit + * 6feac910b6); its card number, cited elsewhere in this file, no longer + * resolves. The reasons are recorded where {@link computeLayeredRlsFilter} + * applies the drop. * * One predicate, two readers: the pre-image Layer 1 composition and the * post-image check's floor decision (`computeWriteCheckFilter`). Keeping it @@ -6053,6 +6041,20 @@ export class SecurityPlugin implements Plugin { ); } + /** + * [ADR-0095 D1] Compute the effective row filter for (object, operation) as + * `Layer0(tenant) AND Layer1(business RLS)`. + * + * - **Layer 0** (tenant isolation) is computed by {@link computeTenantLayer0Filter} + * from the tenancy mode + the object's field set/posture — independent of the + * RLS compiler, always first, unconditionally AND-composed. + * - **Layer 1** (business RLS) is the applicable per-policy compile (ownership, + * depth, sharing, `_self` carve-outs), with the field-existence safety net and + * the posture-gated superuser bypass — which now governs BUSINESS RLS only. + * + * Shared by the engine middleware (read + by-id write pre-image) and + * {@link getReadFilter}. Returns `null` when neither layer contributes. + */ private async computeRlsFilter( permissionSets: PermissionSet[], object: string, From 62d12779800c9baa147fc5e3367fffca38b0aac3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 05:08:02 +0000 Subject: [PATCH 9/9] docs(plugin-security): cite the landed commit for the master-gate ruling, not an unresolvable number Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude --- packages/plugins/plugin-security/src/security-plugin.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 17c14f37b95..be9f98572d0 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -6018,9 +6018,11 @@ export class SecurityPlugin implements Plugin { * Does the object's own write model replace the platform ownership floor * for this operation? True for a `public_read_write` OWD on `update` (#8023) * and for a `controlled_by_parent` detail whose master gate the caller - * vouches covers this write. That ruling landed in PR #8869 (commit - * 6feac910b6); its card number, cited elsewhere in this file, no longer - * resolves. The reasons are recorded where {@link computeLayeredRlsFilter} + * vouches covers this write. That ruling landed in commit 6feac910b6 + * ("the master gate is the sole row-write authority for a + * controlled_by_parent detail"); the card number cited for it elsewhere in + * this file no longer resolves on the board, and that commit is the live + * record. The reasons are recorded where {@link computeLayeredRlsFilter} * applies the drop. * * One predicate, two readers: the pre-image Layer 1 composition and the