diff --git a/.changeset/adr-0095-d1-tenant-layer-0.md b/.changeset/adr-0095-d1-tenant-layer-0.md new file mode 100644 index 0000000000..bef1902c23 --- /dev/null +++ b/.changeset/adr-0095-d1-tenant-layer-0.md @@ -0,0 +1,39 @@ +--- +"@objectstack/plugin-security": minor +--- + +ADR-0095 D1: tenant isolation is now **Layer 0** — an independent, always-first, +AND-composed filter (`tenant-layer.ts`), no longer a wildcard `tenant_isolation` +RLS policy OR-merged with business RLS. The effective row filter is +`Layer0(tenant) AND Layer1(business RLS)`; the two share no compiler, merge step, +or bypass bit. The superuser bypass now exempts the tenant wall only as a Layer 0 +rule (platform-admin posture + object posture permits: private / platform-global +/ better-auth-managed), never via a business-RLS short-circuit. + +**BREAKING (multi-org `tenancy.mode = 'multi'` deployments only; `single` mode is +inert and unchanged).** Retiring the OR-merged tenant policy resolves four +behavior deltas, all toward stronger/correcter isolation: + +- **(a) Cross-tenant read leak closed.** A permissive business RLS policy (e.g. + `status == 'public'`) no longer OR-widens tenant scope; a foreign-org row it + matched is now invisible. +- **(b) Member by-id writes narrow to owner-only.** The OR-merge silently widened + `owner_only_writes` (`created_by == me`) back to org-wide, so a member could + by-id update/delete *any* record in their org. Writes are now owner-scoped as + authored. **Migration:** if your deployment intentionally relied on members + editing each other's records org-wide, grant an explicit per-object edit + permission set (position-distributed) where that is wanted — the baseline + `member_default` no longer permits it. +- **(c) Global-catalog objects visible to members.** On a `tenancy.enabled:false` + object, members were scoped by a phantom `organization_id` filter (a column + such objects lack); Layer 0 correctly treats them as non-tenant, so the global + catalog is visible. +- **(e) No-active-org writes fail closed.** A write by a principal with no active + organization on a tenant object is now denied (was owner-scoped only). + +`tenant_isolation` is retired from the seeded `organization_admin` / +`member_default` / `viewer_readonly` sets; the `_self` / `_org` identity-table +carve-outs and `owner_only_writes/deletes` are unchanged. Customized seeded sets +keep their overlays (ADR-0094). The driver-level `applyTenantScope` seam is +untouched. See ADR-0095 and framework#2936 (the `extractTargetField` `==` blind +spot this exposes, tracked separately). diff --git a/docs/adr/0095-authz-kernel-tenant-layer-and-posture-ladder.md b/docs/adr/0095-authz-kernel-tenant-layer-and-posture-ladder.md index f060b7322d..6a3a3bd550 100644 --- a/docs/adr/0095-authz-kernel-tenant-layer-and-posture-ladder.md +++ b/docs/adr/0095-authz-kernel-tenant-layer-and-posture-ladder.md @@ -136,12 +136,43 @@ compiler: than by auditing every policy. The driver-level `applyTenantScope` seam is unchanged (defense in depth, now behind a real first line). -**Behavior contract.** The extraction is behavior-preserving under the matrix -gate, with one deliberate exception: where the current OR-merge (W1) would -admit a cross-org row that the documented AND semantics forbid, Layer 0 -enforces the documented semantics. That is a **security fix**, not drift; the -matrix suite gains explicit rows for it (a permissive business policy × a -foreign-org row → invisible). +**Behavior contract (as built).** The extraction is behavior-preserving under +the matrix gate — every `role × object × operation` cell is identical or a +same-visibility filter simplification (duplicate-OR dedup; dead org-clause +removal on non-tenant objects) — **except four deliberate deltas**. All four are +the same structural defect (the tenant policy sharing the OR-merge / a +`==`-blind field net with business RLS) resolving toward stronger, more correct +isolation; the matrix suite (`plugin-security/authz-matrix-gate.test.ts`) gains +an explicit, annotated cell for each: + +- **(a) W1 read.** A permissive business policy (e.g. `status == 'public'`) no + longer OR-widens tenant scope: a foreign-org public row becomes **invisible** + (`Layer0(org) AND Layer1(status==public)`). *The* headline W1 fix. +- **(b) W1 write (its twin).** The same OR-merge silently widened a *restrictive* + write policy: `owner_only_writes` (`created_by == me`) was OR'd with the tenant + policy, so a member's by-id write resolved to `org OR created_by` = **any row + in their org**. Layer 0 AND-composes the tenant scope, so the write narrows to + **owner-only**, as authored. (Release-notes BREAKING — see Consequences.) +- **(c) Platform-global object.** A member reading a `tenancy.enabled:false` + global object was scoped by a *phantom* `organization_id` filter — the seeded + tenant policy uses canonical `==`, which the field-existence/tenancy-disabled + net (`extractTargetField`, single-`=`/`IN` only) cannot parse, so the + tenancy-disabled skip never fired and the policy compiled against a column the + object lacks. Layer 0 decides "tenant object?" **directly** from the field set + + posture (not via `extractTargetField`), so a global object correctly + contributes nothing and its **catalog is visible**. (The broader + `extractTargetField` `==` blind spot still affects Layer-1 *business* policies + and is tracked separately — out of scope for D1.) +- **(e) No active organization.** A write by a principal with no active org on a + tenant object is now **fail-closed by Layer 0** (was owner-scoped only). + +**Not touched.** `computeWriteCheckFilter` gains no tenant post-image check: the +seeded `tenant_isolation` carried only a `using` clause (never a `check`), so +there is no post-image tenant behavior to preserve; insert tenant placement is +owned by the enterprise auto-stamp, and update/delete targeting is enforced by +the Layer 0 pre-image path. Adding a mandatory post-image tenant check would +risk denying legitimate inserts before the auto-stamp runs — a regression, not a +fix — so it is deliberately excluded from D1. ### D2 — A monotonic posture ladder, resolved once @@ -224,12 +255,18 @@ each step lands only behind the snapshot gate: need, instead of under portal-feature deadline pressure. **Negative / accepted.** -- The seeded permission sets change (the `tenant_isolation` policy retires). - Environments that customized seeded sets keep their overlays (ADR-0094 - semantics), but the diff is visible; release notes must call it out. -- W1's resolution is a behavior change in deliberately-misconfigured corners - (a business policy that was silently widening tenant scope stops doing so). - This is the point, and it ships loudly. +- The seeded permission sets change (the `tenant_isolation` policy retires from + `organization_admin` / `member_default` / `viewer_readonly`). Environments that + customized seeded sets keep their overlays (ADR-0094 semantics), but the diff + is visible; release notes must call it out. +- **Four behavior deltas ship (release-notes BREAKING), all W1-class** — see the + D1 behavior contract for the enumerated cells. The load-bearing one for + operators is **(b)**: a deployment that relied on *members editing each other's + records within the org* (the OR-merge silently permitting it) will find member + by-id writes now restricted to records they own (`created_by`); grant an + explicit per-object edit set where org-wide editing is intended. Deltas (a) W1 + read, (c) global-catalog visibility for members, and (e) fail-closed no-org + writes are each toward stronger/correcter isolation. These ship loudly. - `resolveAuthzContext` gains one more derived field; hot-path cost is one enum computation over already-loaded grants (negligible). diff --git a/packages/dogfood/test/authz-conformance.matrix.ts b/packages/dogfood/test/authz-conformance.matrix.ts index 6157af55be..2ae661c721 100644 --- a/packages/dogfood/test/authz-conformance.matrix.ts +++ b/packages/dogfood/test/authz-conformance.matrix.ts @@ -41,7 +41,7 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ { id: 'controlled-by-parent', summary: 'master-detail controlled_by_parent', state: 'enforced', enforcement: 'plugin-security/security-plugin.ts computeControlledByParentFilter + assertControlledByParentWrite', proof: 'controlled-by-parent.dogfood.test.ts' }, { id: 'multi-tenant', summary: 'organization isolation', state: 'enforced', - enforcement: '@objectstack/organizations (enterprise) + wildcard tenant RLS', proof: 'rls-multitenant.dogfood.test.ts' }, + enforcement: '@objectstack/organizations (enterprise) + Layer 0 tenant wall (plugin-security/tenant-layer.ts, AND-composed ahead of business RLS — ADR-0095 D1)', proof: 'rls-multitenant.dogfood.test.ts' }, { id: 'anonymous-deny', summary: 'secure-by-default anonymous posture (capability)', state: 'enforced', enforcement: 'rest/rest-server.ts enforceAuth (requireAuth)', proof: 'showcase-anonymous-deny.dogfood.test.ts' }, { id: 'default-profile', summary: 'app-declared default profile (isDefault)', state: 'enforced', diff --git a/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts b/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts new file mode 100644 index 0000000000..45a252f664 --- /dev/null +++ b/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts @@ -0,0 +1,283 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// ── Unit-layer authorization matrix gate (ADR-0095 D1) ────────────────────── +// +// ADR-0095 makes tenant isolation a refactor validated by a `role × object × +// expected-visible-rows` snapshot: every cell must match across the Layer 0 +// extraction EXCEPT the four deltas the architect accepted (all W1-class, +// all toward stronger/correcter isolation — see below). The existing +// conformance matrix (`packages/dogfood/test/authz-conformance.matrix.ts`) +// proves this end-to-end through a real app boot (minutes). This file is the +// UNIT-LAYER equivalent so the loop is seconds: it drives the real +// SecurityPlugin CRUD middleware with the real seeded permission sets and +// snapshots the *effective RLS filter* each (role × object × operation) cell +// produces — the compiled filter the engine ANDs onto a read, or verifies the +// by-id write target against. Two filters selecting the same rows are the same +// visibility; the snapshot is that filter, verbatim. +// +// This file adds NO production code; it is the gate the extraction landed behind. +// The four accepted, ADR-authorized deltas (post-extraction values below): +// (a) [W1 read] a permissive business policy no longer OR-widens tenant scope +// — a cross-org `public` row becomes INVISIBLE (Layer0 AND Layer1). +// (b) [W1 write] `owner_only_writes` is no longer OR-diluted by the tenant +// policy — a member's by-id write narrows to OWNER-only (was org-wide). +// (c) [tenancy-disabled] a member reading a `tenancy.enabled:false` global +// object is no longer scoped by a phantom org filter — the global catalog +// is VISIBLE (Layer 0 correctly treats it as a non-tenant object; this +// also retires the `extractTargetField` `==` blind spot for tenant scope). +// (e) [no active org] a write by a principal with no active organization on a +// tenant object is FAIL-CLOSED by Layer 0 (was owner-scoped only). +// Every OTHER change vs the pre-extraction snapshot is a same-visibility filter +// simplification (duplicate-OR dedup; dead org-clause removal on non-tenant +// objects) and is annotated inline. + +import { describe, it, expect, vi } from 'vitest'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; +import { RLS_DENY_FILTER } from './rls-compiler.js'; +import type { PermissionSet } from '@objectstack/spec/security'; + +// A permissive, admin-authored business RLS policy (ADR-0095 W1's worked +// example): "everyone may read rows whose status is public". At the RLS layer +// this is OR-merged with the wildcard tenant policy today — so it is, by itself, +// sufficient to admit a row from ANOTHER organization. Modeled here as a custom +// set because W1 is about ANY permissive business policy, not a seeded one. +const publicReader: PermissionSet = { + name: 'public_reader', + label: 'Public Reader (permissive business RLS)', + objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true } }, + rowLevelSecurity: [ + { name: 'public_read', object: '*', operation: 'select', using: "status == 'public'" }, + ], +} as any; + +const ALL_SETS: PermissionSet[] = [...defaultPermissionSets, publicReader]; +const DENY = RLS_DENY_FILTER.id; // the fail-closed sentinel's marker value + +// ── Minimal middleware harness ────────────────────────────────────────────── +// Drives the REAL security CRUD middleware against a single-object schema whose +// posture (public / private / tenancy-disabled / better-auth-managed) and field +// set are configurable, so one helper covers the whole object axis. +function makeHarness(opts: { + objectName: string; + objectFields: string[]; + schemaExtra?: Record; + orgScoping?: boolean; + findOneImpl?: (q: any) => any; +}) { + const fields: Record = {}; + for (const f of opts.objectFields) fields[f] = { name: f }; + const baseSchema: any = { name: opts.objectName, fields, ...(opts.schemaExtra ?? {}) }; + let middleware: any; + const findOne = vi.fn(async (_o: string, q: any) => (opts.findOneImpl ? opts.findOneImpl(q) : null)); + const ql = { + registerMiddleware: (mw: any) => { if (!middleware) middleware = mw; }, + getSchema: () => baseSchema, + findOne, + }; + const metadata = { get: async () => baseSchema, list: () => ALL_SETS }; + const services: Record = { manifest: { register: vi.fn() }, objectql: ql, metadata }; + // Multi-org isolation active iff org-scoping is wired (ADR-0093 D4 — the exact + // signal SecurityPlugin probes). `tenancy` service is absent here, so the + // plugin falls back to the `org-scoping` probe (same as production baseline). + if (opts.orgScoping) services['org-scoping'] = { name: 'org-scoping' }; + const ctx: any = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`no service: ${name}`); + return services[name]; + }, + }; + return { ctx, findOne, run: async (opCtx: any) => { await middleware(opCtx, async () => {}); return opCtx; } }; +} + +/** Effective READ filter the engine would AND onto a `find` (the visible-row set). */ +async function readFilter(cell: any, roleCtx: any): Promise { + const plugin = new SecurityPlugin(); + const h = makeHarness({ ...cell, orgScoping: cell.orgScoping ?? true }); + await plugin.init(h.ctx); await plugin.start(h.ctx); + const opCtx: any = { object: cell.objectName, operation: 'find', ast: { where: undefined }, context: roleCtx }; + try { await h.run(opCtx); } catch (e: any) { return `CRUD_DENY:${e?.name ?? 'err'}`; } + return opCtx.ast.where ?? null; +} + +/** + * Effective WRITE filter used by the by-id update pre-image check — the row the + * caller is allowed to mutate must satisfy it. Returned as the array of RLS + * parts ANDed with the `{id}` guard (that guard is stripped). `BYPASS` = no + * write filter (superuser). `CRUD_DENY` = blocked before the pre-image check. + */ +async function writeFilter(cell: any, roleCtx: any): Promise { + const plugin = new SecurityPlugin(); + const h = makeHarness({ ...cell, orgScoping: cell.orgScoping ?? true, findOneImpl: () => null }); + await plugin.init(h.ctx); await plugin.start(h.ctx); + const opCtx: any = { + object: cell.objectName, operation: 'update', + data: { id: 'r1', name: 'x' }, options: { where: { id: 'r1' } }, context: roleCtx, + }; + let threw: any = null; + try { await h.run(opCtx); } catch (e: any) { threw = e; } + if (h.findOne.mock.calls.length === 0) { + return threw ? `CRUD_DENY:${threw?.name ?? 'err'}` : 'BYPASS(no-write-filter)'; + } + return h.findOne.mock.calls[0][1].where.$and.slice(1); +} + +// ── Axes ───────────────────────────────────────────────────────────────────── +const OBJECTS = { + // Ordinary tenant business object: has organization_id, public posture. + task: { objectName: 'task', objectFields: ['id', 'organization_id', 'created_by', 'status', 'name'] }, + // Private object (access.default: private) — plain wildcard grant does NOT cover it (ADR-0066 ④). + private_obj: { objectName: 'crm_secret', objectFields: ['id', 'organization_id', 'created_by', 'name'], schemaExtra: { access: { default: 'private' } } }, + // Platform-global object (tenancy.enabled: false), no organization_id column. + platform_global: { objectName: 'sys_package', objectFields: ['id', 'name', 'visibility'], schemaExtra: { tenancy: { enabled: false } } }, + // Better-auth-managed identity table (managedBy: 'better-auth'); writes flow through better-auth. + better_auth: { objectName: 'sys_user', objectFields: ['id', 'email', 'name'], schemaExtra: { managedBy: 'better-auth' } }, +}; + +const ROLES = { + // Platform admin: holds admin_full_access (viewAllRecords/modifyAllRecords) — the superuser bypass evidence. + platform_admin: { userId: 'padmin', tenantId: 'org-1', positions: ['platform_admin'], permissions: ['admin_full_access'] }, + // Org admin: holds organization_admin (also viewAll/modifyAll, but tenant-scoped by its RLS). + org_admin: { userId: 'oadmin', tenantId: 'org-1', positions: ['org_admin'], permissions: ['organization_admin'] }, + // Rank-and-file member: only the additive member_default baseline; org_member gates owner_only_*. + member: { userId: 'u1', tenantId: 'org-1', positions: ['org_member'], permissions: [] }, + // Authenticated user with NO active organization → tenant scoping cannot resolve → fail-closed. + no_org_member: { userId: 'u2', positions: ['org_member'], permissions: [] }, +}; + +// The locked snapshot of POST-EXTRACTION behavior (Layer0 AND Layer1). Read the +// annotations against ADR-0095. Cells tagged [D1 accepted delta] are the four +// authorized changes; all others are unchanged or same-visibility simplifications. +const EXPECTED_MATRIX: Record> = { + task: { + // [posture-gate] Public business object → superuser bypass withheld, admin stays org-scoped (Layer 0). + platform_admin: { read: { organization_id: 'org-1' }, write: [{ organization_id: 'org-1' }] }, + // Simplification: the pre-extraction duplicate-OR (`{$or:[{org},{org}]}` from + // organization_admin + baseline) collapses to `{org}` — Layer 0 is the single owner. + org_admin: { + read: { organization_id: 'org-1' }, + write: [{ organization_id: 'org-1' }], + }, + // [D1 accepted delta (b)] WRITE narrows from `org OR created_by` (org-wide) to + // `org AND created_by` (owner-only) — owner_only_writes finally enforces. + member: { + read: { organization_id: 'org-1' }, + write: [{ $and: [{ organization_id: 'org-1' }, { created_by: 'u1' }] }], + }, + // [D1 accepted delta (e)] no active org: Layer 0 fail-closes the WRITE + // (was owner-scoped only). Read stays deny-sentinel (unchanged). + no_org_member: { read: { id: DENY }, write: [{ $and: [{ id: DENY }, { created_by: 'u2' }] }] }, + }, + private_obj: { + // [W2] Layer 0 exemption + Layer 1 short-circuit both fire for superuser on a private object → null. + platform_admin: { read: null, write: 'BYPASS(no-write-filter)' }, + org_admin: { read: null, write: 'BYPASS(no-write-filter)' }, + // A member's plain wildcard grant does not cover a private object → denied at the CRUD gate, before RLS. + member: { read: 'CRUD_DENY:PermissionDeniedError', write: 'CRUD_DENY:PermissionDeniedError' }, + no_org_member: { read: 'CRUD_DENY:PermissionDeniedError', write: 'CRUD_DENY:PermissionDeniedError' }, + }, + platform_global: { + // [W2] superuser bypass fires on tenancy-disabled posture → null. + platform_admin: { read: null, write: 'BYPASS(no-write-filter)' }, + org_admin: { read: null, write: 'BYPASS(no-write-filter)' }, + // [D1 accepted delta (c)] A member reading a `tenancy.enabled:false` global + // object: Layer 0 treats it as a NON-tenant object (no phantom org filter) → + // the global catalog is VISIBLE (read: null). The write drops the dead + // org-disjunct to plain owner scope (same visibility, no org column exists). + member: { read: null, write: [{ created_by: 'u1' }] }, + // [D1 accepted delta (c)] no-org member likewise sees the global catalog (was deny-sentinel). + no_org_member: { read: null, write: [{ created_by: 'u2' }] }, + }, + better_auth: { + // [W2] better-auth-managed posture → superuser read bypass → null. + platform_admin: { read: null, write: 'BYPASS(no-write-filter)' }, + // Simplification: `sys_user` has no organization_id, so the pre-extraction + // dead org-disjuncts drop; the duplicated `_self` policies remain (self only). + org_admin: { + read: { $or: [{ id: 'oadmin' }, { id: 'oadmin' }] }, + write: 'CRUD_DENY:PermissionDeniedError', + }, + // Member: self only (dead org-clause removed); writes denied (better-auth door). + member: { read: { id: 'u1' }, write: 'CRUD_DENY:PermissionDeniedError' }, + // No-org member: self only; writes denied. + no_org_member: { read: { id: 'u2' }, write: 'CRUD_DENY:PermissionDeniedError' }, + }, +}; + +describe('authz Layer-0 matrix gate — ADR-0095 D1 (post-extraction)', () => { + it('locks the role × object × {read,write} effective-filter matrix', async () => { + const actual: Record> = {}; + for (const [oName, cell] of Object.entries(OBJECTS)) { + actual[oName] = {}; + for (const [rName, role] of Object.entries(ROLES)) { + actual[oName][rName] = { read: await readFilter(cell, role), write: await writeFilter(cell, role) }; + } + } + expect(actual).toEqual(EXPECTED_MATRIX); + }); + + // ── W1: cross-tenant read leak, CLOSED by Layer 0 ───────────────────────── + // [ADR-0095 D1 accepted delta (a)] A user holding a permissive business RLS + // policy (`status == 'public'`) reads a tenant object. Pre-extraction the + // wildcard tenant policy was OR-merged with it (`tenant OR status==public`), so + // a foreign-org public row matched the second disjunct and was VISIBLE. Layer 0 + // now AND-composes the tenant wall ahead of business RLS, so the effective read + // is `Layer0(org) AND Layer1(status==public)` — the foreign-org public row is + // INVISIBLE. This is the W1 fix. + it('[W1] permissive business RLS is AND-composed under Layer 0 (cross-org public row INVISIBLE)', async () => { + const filter = await readFilter(OBJECTS.task, { + userId: 'u3', tenantId: 'org-1', positions: ['org_member'], permissions: ['public_reader'], + }); + // Post-D1: tenant wall AND business policy — no OR-widening. + expect(filter).toEqual({ $and: [{ organization_id: 'org-1' }, { status: 'public' }] }); + // A foreign-org public row now FAILS the tenant conjunct → invisible. + const foreignPublicRow: Record = { organization_id: 'org-2', status: 'public' }; + const andClauses = (filter as any).$and as Array>; + const visible = andClauses.every((c) => + Object.entries(c).every(([k, v]) => foreignPublicRow[k] === v)); + expect(visible).toBe(false); // ← [ADR-0095 W1 fix] the leak is closed. + }); + + // ── W1's write-side twin: owner_only now enforces (delta b) ─────────────── + // [ADR-0095 D1 accepted delta (b)] The same OR-merge that leaked reads also + // WIDENED a restrictive write policy: a member's `owner_only_writes` + // (created_by == me) was OR'd with the tenant policy, so the by-id write + // pre-image resolved to `org OR created_by` = any row in the member's org. + // Layer 0 now AND-composes the tenant scope, so the pre-image is + // `Layer0(org) AND Layer1(created_by == me)` = owner-only, as authored. + it('[W1-write] member by-id write narrows to owner-only under Layer 0', async () => { + const wf = await writeFilter(OBJECTS.task, ROLES.member); + expect(wf).toEqual([{ $and: [{ organization_id: 'org-1' }, { created_by: 'u1' }] }]); + }); + + // ── W2: the superuser bypass short-circuits BOTH layers via one bit ──────── + // On private / platform-global / better-auth objects a superuser-bit holder + // skips ALL wildcard RLS — tenant wall included — through a single check. On a + // PUBLIC business object the posture gate withholds the bypass, so the admin + // stays org-scoped. Both facets locked. + it('[W2] superuser bypass fires on private/platform-global/better-auth, withheld on public business objects', async () => { + expect(await readFilter(OBJECTS.private_obj, ROLES.platform_admin)).toBeNull(); + expect(await readFilter(OBJECTS.platform_global, ROLES.platform_admin)).toBeNull(); + expect(await readFilter(OBJECTS.better_auth, ROLES.platform_admin)).toBeNull(); + // Withheld on a public tenant object → admin remains org-scoped (the posture gate). + expect(await readFilter(OBJECTS.task, ROLES.platform_admin)).toEqual({ organization_id: 'org-1' }); + }); + + // ── Fail-closed: an authenticated user with no active org sees no tenant rows ─ + it('[fail-closed] no active organization → tenant read denies via the sentinel', async () => { + expect(await readFilter(OBJECTS.task, ROLES.no_org_member)).toEqual({ id: DENY }); + }); + + // ── Single-org mode: Layer 0 is inert; tenant policy stripped (parity today) ─ + // With org-scoping absent, collectRLSPolicies strips the wildcard tenant policy + // entirely, so a member's read carries NO tenant where and the write keeps only + // the owner scope. ADR-0095: in single mode Layer 0 contributes nothing — this + // cell must NOT move after the extraction. + it('[single-mode] tenant policy stripped when org-scoping is absent', async () => { + const single = { ...OBJECTS.task, orgScoping: false }; + expect(await readFilter(single, ROLES.member)).toBeNull(); + expect(await writeFilter(single, ROLES.member)).toEqual([{ created_by: 'u1' }]); + }); +}); diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts index 10fb289d26..e3386ba817 100644 --- a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts +++ b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts @@ -110,7 +110,7 @@ export const defaultPermissionSets: PermissionSet[] = [ // // Third tier between platform admin (`admin_full_access`) and rank-and-file // member. Lives at the *organization* scope: full CRUD on business - // objects within their org (governed by `tenant_isolation` RLS), plus + // objects within their org (governed by the Layer 0 tenant wall, ADR-0095 D1), plus // `setup.access` so the Setup app shell is reachable. // // **Deliberately withheld** vs `admin_full_access`: @@ -157,12 +157,12 @@ export const defaultPermissionSets: PermissionSet[] = [ }, systemPermissions: ['manage_org_users', 'setup.access', 'setup.write'], rowLevelSecurity: [ - { - name: 'tenant_isolation', - object: '*', - operation: 'all', - using: 'organization_id == current_user.organization_id', - }, + // [ADR-0095 D1] The wildcard `tenant_isolation` policy RETIRED here — the + // tenant wall is now Layer 0 (`tenant-layer.ts`), AND-composed ahead of and + // independently of business RLS. Keeping it as an OR-merged RLS policy is + // what let a permissive business policy widen tenant scope (W1). The + // per-object `_org` / `_self` carve-outs below are NOT tenant walls — they + // are identity-table scoping and stay. // ── better-auth system tables that lack `organization_id` and would // otherwise be denied by the wildcard policy. Same self-only // carve-outs as `member_default` — an org admin does not get to @@ -247,8 +247,8 @@ export const defaultPermissionSets: PermissionSet[] = [ }, // OAuth applications a user has registered themselves (self-service // developer flow exposed in the Account app's Developer section). - // `sys_oauth_application` has no `organization_id` so the wildcard - // `tenant_isolation` policy would otherwise deny every row. + // `sys_oauth_application` has no `organization_id`; this `_self` carve-out + // is its Layer 1 scoping (Layer 0 is inert on a non-tenant object). { name: 'sys_oauth_application_self', object: 'sys_oauth_application', @@ -298,12 +298,11 @@ export const defaultPermissionSets: PermissionSet[] = [ ...denyWritesOnManagedObjects(), }, rowLevelSecurity: [ - { - name: 'tenant_isolation', - object: '*', - operation: 'all', - using: 'organization_id == current_user.organization_id', - }, + // [ADR-0095 D1] The wildcard `tenant_isolation` policy RETIRED here — the + // tenant wall is now Layer 0 (`tenant-layer.ts`). Its old OR-merge with the + // owner-scoped policies below is exactly what widened a member's by-id write + // back to org-wide (W1's write-side twin); Layer 0 now AND-composes the + // tenant scope, so `owner_only_writes/deletes` finally narrow as intended. // Owner-scoped writes/deletes for rank-and-file members: you may modify // and delete the records you created, not other users'. Keyed on // `created_by` — the column the engine stamps on EVERY record — rather @@ -436,8 +435,8 @@ export const defaultPermissionSets: PermissionSet[] = [ }, // OAuth applications a user has registered themselves (Account → // Developer → OAuth Applications). `sys_oauth_application` has no - // `organization_id`, so without this carve-out the wildcard - // `tenant_isolation` policy returns zero rows even for the owner. + // `organization_id`; this `_self` carve-out is its Layer 1 scoping + // (Layer 0 is inert on a non-tenant object). { name: 'sys_oauth_application_self', object: 'sys_oauth_application', @@ -462,12 +461,9 @@ export const defaultPermissionSets: PermissionSet[] = [ ...denyWritesOnManagedObjects(), }, rowLevelSecurity: [ - { - name: 'tenant_isolation', - object: '*', - operation: 'select', - using: 'organization_id == current_user.organization_id', - }, + // [ADR-0095 D1] The wildcard `tenant_isolation` policy RETIRED here — the + // tenant wall is now Layer 0 (`tenant-layer.ts`). The `_self` carve-outs + // below are identity-table scoping, not tenant walls, and stay. { name: 'sys_organization_self', object: 'sys_organization', diff --git a/packages/plugins/plugin-security/src/objects/rbac-objects.test.ts b/packages/plugins/plugin-security/src/objects/rbac-objects.test.ts index 446fab8ee2..8a8e8d68ad 100644 --- a/packages/plugins/plugin-security/src/objects/rbac-objects.test.ts +++ b/packages/plugins/plugin-security/src/objects/rbac-objects.test.ts @@ -85,9 +85,12 @@ describe('default permission sets', () => { expect(wildcard.modifyAllRecords).toBe(true); }); - it('member_default ships tenant + owner RLS policies plus better-auth system table guards', () => { + it('member_default ships owner + better-auth self RLS policies; tenant wall is Layer 0 (ADR-0095 D1)', () => { const member = defaultPermissionSets.find((p) => p.name === 'member_default')!; const policyNames = (member.rowLevelSecurity ?? []).map((p) => p.name).sort(); + // [ADR-0095 D1] `tenant_isolation` RETIRED from the seed — the tenant wall is + // now Layer 0 (`tenant-layer.ts`), not an OR-merged RLS policy. The remaining + // policies are business RLS: owner-scoped writes + better-auth `_self` carve-outs. expect(policyNames).toEqual([ 'owner_only_deletes', 'owner_only_writes', @@ -105,10 +108,8 @@ describe('default permission sets', () => { 'sys_user_org_members', 'sys_user_preference_self', 'sys_user_self', - 'tenant_isolation', ]); - const tenantPolicy = (member.rowLevelSecurity ?? []).find((p) => p.name === 'tenant_isolation')!; - expect(tenantPolicy.using).toBe('organization_id == current_user.organization_id'); + expect(policyNames).not.toContain('tenant_isolation'); const orgSelf = (member.rowLevelSecurity ?? []).find((p) => p.name === 'sys_organization_self')!; expect(orgSelf.object).toBe('sys_organization'); expect(orgSelf.using).toBe('id == current_user.organization_id'); diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index c0bab76bf2..69577e0057 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -203,7 +203,11 @@ describe('SecurityPlugin', () => { context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] }, }; await harness.run(opCtx); - expect(opCtx.ast.where).toEqual({ organization_id: 'org-1' }); + // [ADR-0095 D1] Layer 0 (the tenant wall) AND the fixture's own legacy + // tenant_isolation policy (Layer 1) — same rows. Production seeds no longer + // carry the Layer-1 tenant policy, so in production this is Layer 0's + // `{organization_id}` alone; this fixture keeps it to exercise composition. + expect(opCtx.ast.where).toEqual({ $and: [{ organization_id: 'org-1' }, { organization_id: 'org-1' }] }); }); // Regression: when a schema explicitly opts out of tenancy @@ -414,7 +418,9 @@ describe('SecurityPlugin', () => { context: { userId: 'u1', tenantId: 'org-1', roles: ['owner'], permissions: [] }, }; await harness.run(opCtx); - expect(opCtx.ast.where).toEqual({ organization_id: 'org-1' }); + // [ADR-0095 D1] Tenant scope is now guaranteed by Layer 0 regardless of which + // Layer-1 sets resolve; ANDed with the fixture's legacy tenant policy (same rows). + expect(opCtx.ast.where).toEqual({ $and: [{ organization_id: 'org-1' }, { organization_id: 'org-1' }] }); }); // ------------------------------------------------------------------------- @@ -556,8 +562,11 @@ describe('SecurityPlugin', () => { context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] }, }; await harness.run(opCtx); - // The member is still tenant-scoped — the managedBy bypass is admin-only. - expect(opCtx.ast.where).toEqual({ organization_id: 'org-1' }); + // The member is still tenant-scoped — the managedBy superuser bypass is + // admin-only (Layer 0's exemption requires the platform-admin bit). Here + // the object HAS organization_id, so Layer 0 applies the wall; ANDed with + // the fixture's legacy tenant policy (Layer 1) — same rows, no leak. + expect(opCtx.ast.where).toEqual({ $and: [{ organization_id: 'org-1' }, { organization_id: 'org-1' }] }); }); }); @@ -666,7 +675,9 @@ describe('SecurityPlugin', () => { await plugin.start(harness.ctx); const ctx = { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] }; const filter = await plugin.getReadFilter('task', ctx); - expect(filter).toEqual({ organization_id: 'org-1' }); + // [ADR-0095 D1] Same Layer0-AND-Layer1 composition as the find path — the + // point of this test (getReadFilter parity) holds by construction. + expect(filter).toEqual({ $and: [{ organization_id: 'org-1' }, { organization_id: 'org-1' }] }); }); it('returns undefined (no scope) when tenant_isolation is stripped (org-scoping off)', async () => { diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 3db056fab7..bd6dae0b10 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -32,6 +32,7 @@ import { cleanupPackagePermissions } from './cleanup-package-permissions.js'; import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; import { bootstrapSystemCapabilities } from './bootstrap-system-capabilities.js'; import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; +import { computeTenantLayer0Filter, andComposeLayers } from './tenant-layer.js'; import { matchesFilterCondition } from '@objectstack/formula'; import { FieldMasker } from './field-masker.js'; import { assertReadableQueryFields } from './predicate-guard.js'; @@ -1868,12 +1869,18 @@ export class SecurityPlugin implements Plugin { } /** - * Compile the applicable RLS policies for (object, operation) into a single - * `FilterCondition`, applying the field-existence safety net (wildcard - * policies targeting a column the object lacks fail-closed to the deny - * sentinel, unless the object explicitly opted out of tenancy). Shared by the - * engine middleware and {@link getReadFilter}. Returns `null` when no policy - * applies (caller adds no filter). + * [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[], @@ -1881,50 +1888,72 @@ export class SecurityPlugin implements Plugin { operation: string, context: any, ): Promise | null> { - // [ADR-0066 ①] Posture-gated super-user RLS bypass. On a `private` or - // platform-global object, a caller with the super-user bypass bit - // (viewAllRecords for reads, modifyAllRecords for writes) skips wildcard RLS - // entirely — so a platform admin (incl. one who is also an org admin whose - // tenant_isolation would otherwise narrow the result) sees all rows. The - // posture gate ensures this never grants cross-tenant visibility on ordinary - // tenant business objects. + // [ADR-0095 D1] Effective filter = Layer0(tenant) AND Layer1(business RLS). + // The two are computed independently and never share a compiler, a merge + // step, or a bypass bit (closes W1 by construction, W2 structurally). const meta = await this.getObjectSecurityMeta(object); - if (meta.isPrivate || meta.tenancyDisabled || meta.isBetterAuthManaged) { - const isWrite = operation === 'insert' || operation === 'update' || operation === 'delete'; - const bypass = isWrite - ? this.permissionEvaluator.hasSuperuserWriteBypass(object, permissionSets, { isPrivate: meta.isPrivate }) - : this.permissionEvaluator.hasSuperuserReadBypass(object, permissionSets, { isPrivate: meta.isPrivate }); - if (bypass) return null; - } - const allRlsPolicies = this.collectRLSPolicies(permissionSets, object, operation, (context?.positions ?? []) as string[]); - if (allRlsPolicies.length === 0) return null; - // Field-existence safety: wildcard policies (`object: '*'`) target fields - // like `organization_id` that may not exist on every object. Treat such a - // policy as a *deny* contribution (fail-closed) rather than dropping it — - // unless the object explicitly opted out of tenancy, where it's "not - // applicable" and skipped silently. When the schema lookup itself fails we - // keep all policies (drivers surface column errors clearly at compile time). + const isWrite = operation === 'insert' || operation === 'update' || operation === 'delete'; + // Posture permits a platform admin to cross the tenant wall (ADR-0066 ①): + // private / platform-global / better-auth-managed objects. Public tenant + // business objects do NOT permit it, so a platform admin stays org-scoped. + const posturePermits = meta.isPrivate || meta.tenancyDisabled || meta.isBetterAuthManaged; + const platformAdminBypass = posturePermits + ? (isWrite + ? this.permissionEvaluator.hasSuperuserWriteBypass(object, permissionSets, { isPrivate: meta.isPrivate }) + : this.permissionEvaluator.hasSuperuserReadBypass(object, permissionSets, { isPrivate: meta.isPrivate })) + : false; + + // Field set drives BOTH the Layer 1 field-existence net and the Layer 0 + // "is this a tenant object?" check. const objectFields = await this.getObjectFieldNames(this.metadata, object, this.ql); - const tenancyDisabled = this.tenancyDisabledCache.get(object) === true; - let dropped = 0; - const compilable = objectFields - ? allRlsPolicies.filter((p) => { - const targetField = this.extractTargetField(p.using); - if (!targetField) return true; - if (objectFields.has(targetField)) return true; - if (tenancyDisabled && targetField === 'organization_id') { - return false; - } - dropped++; - return false; - }) - : allRlsPolicies; - let rlsFilter = this.rlsCompiler.compileFilter(compilable, context); - // Every applicable policy dropped for a missing field → deny sentinel. - if (rlsFilter == null && dropped > 0) { - rlsFilter = { ...RLS_DENY_FILTER }; + const tenancyDisabled = this.tenancyDisabledCache.get(object) === true || meta.tenancyDisabled; + + // ── Layer 1: business RLS (ownership / unit depth / sharing / _self carve-outs). + // The wildcard tenant policy has LEFT this layer (retired from the seeds), so + // this superuser short-circuit now governs BUSINESS RLS only — it can no + // longer skip the tenant wall (that is Layer 0's own exemption, below). + let layer1: Record | null = null; + if (!(posturePermits && platformAdminBypass)) { + const allRlsPolicies = this.collectRLSPolicies(permissionSets, object, operation, (context?.positions ?? []) as string[]); + if (allRlsPolicies.length > 0) { + // Field-existence safety: a wildcard policy targeting a column the object + // lacks is a *deny* contribution (fail-closed), unless the object opted + // out of tenancy (skip). Schema-lookup failure keeps all policies. + let dropped = 0; + const compilable = objectFields + ? allRlsPolicies.filter((p) => { + const targetField = this.extractTargetField(p.using); + if (!targetField) return true; + if (objectFields.has(targetField)) return true; + if (tenancyDisabled && targetField === 'organization_id') { + return false; + } + dropped++; + return false; + }) + : allRlsPolicies; + layer1 = this.rlsCompiler.compileFilter(compilable, context); + // Every applicable policy dropped for a missing field → deny sentinel. + if (layer1 == null && dropped > 0) { + layer1 = { ...RLS_DENY_FILTER }; + } + } } - return rlsFilter; + + // ── Layer 0: tenant isolation — independent, always-first, AND-composed. + // Decides "tenant object?" directly from the field set + tenancy posture (NOT + // via extractTargetField's `=`-only shape match), so a `tenancy.enabled:false` + // global object correctly contributes nothing (ADR-0095 delta c). + const layer0 = computeTenantLayer0Filter({ + isolationActive: this.orgScopingEnabled, + organizationId: context?.tenantId, + objectHasOrgIdField: objectFields ? objectFields.has('organization_id') : undefined, + tenancyDisabled, + posturePermitsCrossTenant: posturePermits, + platformAdminBypass, + }); + + return andComposeLayers(layer0, layer1); } /** @@ -2183,13 +2212,12 @@ export class SecurityPlugin implements Plugin { // (`managedBy: 'better-auth'`: sys_user, sys_account, sys_session, // sys_oauth_application, sys_sso_provider, …). Their rows are written by // better-auth's own adapter with no tenant context, so `organization_id` - // is never stamped and the wildcard `tenant_isolation` RLS denies them — - // making a platform admin's `viewAllRecords` see an empty list. Treat - // them like the private/non-tenant posture for the SUPERUSER BYPASS ONLY - // (so the platform super-admin sees all identity rows env-wide). This does - // NOT relax member RLS (members never trigger the bypass; their `_self` - // carve-outs / tenant_isolation still apply) and is NOT used for the - // wildcard-policy drop below, so it can never leak rows to non-admins. + // is never stamped (and most such tables have no such column at all). + // [ADR-0095 D1] `posturePermitsCrossTenant` uses this flag so a platform + // admin's Layer 0 exemption lets `viewAllRecords` see all identity rows + // env-wide. This does NOT relax member scoping (members never satisfy the + // exemption; their `_self` carve-outs are their Layer 1 scoping, and Layer + // 0 stays inert on a column-less table), so it can never leak to non-admins. isBetterAuthManaged: (obj as any)?.managedBy === 'better-auth', requiredPermissions: normalizeRequiredPermissions((obj as any)?.requiredPermissions), fieldRequiredPermissions, diff --git a/packages/plugins/plugin-security/src/tenant-layer.ts b/packages/plugins/plugin-security/src/tenant-layer.ts new file mode 100644 index 0000000000..d983da4a3c --- /dev/null +++ b/packages/plugins/plugin-security/src/tenant-layer.ts @@ -0,0 +1,108 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { RLS_DENY_FILTER } from './rls-compiler.js'; + +/** + * ── Layer 0: tenant isolation (ADR-0095 D1) ───────────────────────────────── + * + * The tenant wall lives HERE — outside the RLS (business-policy) compiler — as + * an independent, always-first, AND-composed filter. It shares no compiler, no + * merge step, and no bypass bit with Layer 1 (business RLS), so a business-RLS + * change can never weaken tenant isolation (W1), and the superuser business-RLS + * bypass can never cross the tenant wall (W2). + * + * Effective read/write filter = `Layer0(tenant) AND Layer1(business RLS)`. + * + * This module deliberately does NOT reuse `security-plugin`'s `extractTargetField` + * (which recognizes only the legacy single-`=`/`IN` shape, not canonical `==`); + * it decides "is this a tenant object?" directly from the object's field set and + * tenancy posture. That is what makes ADR-0095 delta (c) — a member reading a + * `tenancy.enabled:false` global object — resolve correctly (see tracking issue + * for the broader `extractTargetField` `==` blind spot, which still affects + * Layer-1 business policies and is out of scope for D1). + */ + +/** Inputs the Layer 0 decision needs — all cheap, already-loaded facts. */ +export interface TenantLayer0Input { + /** + * True iff multi-org isolation is actually active (ADR-0093 `tenancy.mode === + * 'multi'` / `tenancy.isolationActive`). In `single` mode Layer 0 is inert. + */ + isolationActive: boolean; + /** The resolved authz context's active organization id (`ExecutionContext.tenantId`). */ + organizationId?: string; + /** + * Whether the object carries an `organization_id` column. `undefined` = the + * schema/field-set could not be resolved yet (boot); treated as "assume tenant + * object" so a not-yet-registered object still fails safe toward isolation + * (parity with the pre-extraction path, which kept the wildcard tenant policy + * when the field set was unavailable). + */ + objectHasOrgIdField?: boolean; + /** + * Whether the object opted out of tenancy (`tenancy.enabled === false` or + * `systemFields.tenant === false`) — a platform-global object. Such objects + * are NOT tenant objects; Layer 0 contributes nothing. + */ + tenancyDisabled: boolean; + /** + * Whether the object's POSTURE permits a platform admin to cross the tenant + * wall — `private`, platform-global (`tenancy.enabled:false`), or + * better-auth-managed (exactly the ADR-0066 ① gate). Business (public) tenant + * objects do NOT permit it, so a platform admin stays org-scoped on them. + */ + posturePermitsCrossTenant: boolean; + /** + * Platform-admin evidence for THIS operation side — the same capability the + * superuser bypass already trusts (`viewAllRecords` on reads / + * `modifyAllRecords` on writes). This is the D1 stand-in for the full posture + * enum (B2); it is NOT the better-auth role. + */ + platformAdminBypass: boolean; +} + +/** + * Compute the Layer 0 (tenant) filter to AND onto a read/write. + * + * - `null` → Layer 0 contributes nothing (single mode; non-tenant object; or an + * exempt platform admin). The caller applies only Layer 1. + * - `{ organization_id }` → the tenant wall, AND-composed unconditionally. + * - {@link RLS_DENY_FILTER} → multi mode on a tenant object but the context has + * no active organization → fail closed (zero rows / write denied). + */ +export function computeTenantLayer0Filter( + input: TenantLayer0Input, +): Record | null { + // `single` mode / isolation absent → parity with today's policy stripping. + if (!input.isolationActive) return null; + + // Not a tenant object: platform-global (tenancy disabled) or simply carries no + // `organization_id` column (e.g. better-auth identity tables like `sys_user`). + // Layer 0 contributes nothing — the object's own business RLS (Layer 1, e.g. + // `_self` carve-outs) is its only scoping. + if (input.tenancyDisabled) return null; + if (input.objectHasOrgIdField === false) return null; + + // Exemption is a Layer 0 rule (W2 fix): only a PLATFORM_ADMIN-posture caller on + // an object whose posture permits it crosses the wall. Layer 1's bypass no + // longer implies Layer 0's. + if (input.platformAdminBypass && input.posturePermitsCrossTenant) return null; + + // Enforce the wall. Missing active org in multi mode → fail closed. + if (!input.organizationId) return { ...RLS_DENY_FILTER }; + return { organization_id: input.organizationId }; +} + +/** + * AND-compose Layer 0 onto Layer 1. Layer 0 is placed FIRST (outermost) so the + * tenant predicate is always evaluated and can never be widened by a Layer 1 + * disjunction. `null` on either side means "no contribution". + */ +export function andComposeLayers( + layer0: Record | null, + layer1: Record | null, +): Record | null { + if (layer0 == null) return layer1; + if (layer1 == null) return layer0; + return { $and: [layer0, layer1] }; +}