From 092bf956d5446b7b5923b809f27b50f18b0966cb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:29:31 +0000 Subject: [PATCH 1/3] fix(security,sharing): materialize the RBAC catalog per organization (#10103) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a walled posture every principal listed ZERO positions, permission sets and sharing rules while the tables held rows: Layer 0's strict `organization_id = :tenant` AND-composes over the driver's `(organization_id = :tenant OR organization_id IS NULL)`, and the conjunction is the strict equality alone. The rows were all organization-less. The wall is untouched at both layers. The rows get an owner: - the four declared/built-in seeders upsert by `(name, organization_id)` and run one pass per organization under a walled posture, built-ins included; - seeding also fires on organization creation, not only at `kernel:ready`; - `single` posture keeps exactly one organization-less pass. Nothing is reaped — grants point at these rows by id. A per-organization pass that meets pre-fix organization-less rows warns loudly, naming the rows and the remedy, and still creates the organization's own copies. That closes the silent no-op where a tenant-threaded pass saw the old row through the driver's compatibility arm and created nothing. The enforcement-plane reads that only become exposures once copies exist are scoped in the same landing: `resolve-authz-context`'s section 6a name-sweep (packages/core) and plugin-security's permission-set dbLoader, whose `limit` was also a truncation once several organizations hold a row per name. Boot reconciliation is O(changed declarations); steady state rides the organization-creation hook. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .changeset/per-organization-rbac-catalog.md | 70 ++++ .../src/security/resolve-authz-context.ts | 45 ++- .../src/bootstrap-builtin-positions.ts | 78 +++- .../src/bootstrap-declared-permissions.ts | 68 +++- .../src/bootstrap-declared-positions.ts | 61 +++- .../src/per-organization-catalog.test.ts | 316 ++++++++++++++++ .../src/per-organization-catalog.ts | 241 ++++++++++++ .../src/permission-set-projection.ts | 23 +- .../plugin-security/src/security-plugin.ts | 342 ++++++++++++++---- .../src/bootstrap-declared-sharing-rules.ts | 97 ++++- .../src/rule-criteria-org-scope.test.ts | 50 ++- .../plugin-sharing/src/sharing-plugin.ts | 136 ++++++- 12 files changed, 1395 insertions(+), 132 deletions(-) create mode 100644 .changeset/per-organization-rbac-catalog.md create mode 100644 packages/plugins/plugin-security/src/per-organization-catalog.test.ts create mode 100644 packages/plugins/plugin-security/src/per-organization-catalog.ts diff --git a/.changeset/per-organization-rbac-catalog.md b/.changeset/per-organization-rbac-catalog.md new file mode 100644 index 0000000000..8adb280bda --- /dev/null +++ b/.changeset/per-organization-rbac-catalog.md @@ -0,0 +1,70 @@ +--- +"@objectstack/plugin-security": minor +"@objectstack/plugin-sharing": minor +"@objectstack/core": patch +--- + +Materialize the RBAC catalog **per organization**, so a walled deployment can +administer positions, permission sets and sharing rules again (#10103). + +On a walled deployment (`group` / `isolated`) every principal — an organization +owner and a platform admin alike — listed **zero** positions, permission sets +and sharing rules while the tables held rows. Nothing could be bound through +Setup, and a declared `hierarchy-security` could never be armed by an operator +however loudly an app declared it. + +Every row in those three tables was organization-less. plugin-security's Layer 0 +composes a strict `organization_id = :tenant` for a walled posture and the +middleware ANDs it into the read AST over the driver's +`(organization_id = :tenant OR organization_id IS NULL)`; the conjunction of the +two is the strict equality alone, so the driver's null arm was annihilated on +every authenticated read. + +**The wall is not changed, at either layer.** The rows get an owner instead: + +- `bootstrapDeclaredPositions`, `bootstrapBuiltinRoles`, + `bootstrapDeclaredPermissions` (plugin-security) and + `bootstrapDeclaredSharingRules` (plugin-sharing) upsert by + `(name, organization_id)` and run **one pass per organization** under a walled + posture — the framework built-ins (`platform_admin`, `org_*`, `everyone`, + `guest`) included, matching `sys_user_position`, which is already + per-organization, and matching both objects' own `unique: 'organization'` name + index. +- Seeding also fires on **organization creation**, not only at `kernel:ready`, so + a tenant created after startup does not administer an empty catalog until the + next restart. +- `single` posture is **unchanged**: exactly one organization-less pass, which is + the correct shape there. + +An organization-less row is now invalid state under a walled posture. Nothing is +reaped — grants (`sys_user_position`, `sys_position_permission_set`, +`sys_user_permission_set`, `sys_record_share`) point at these rows by id, so +deleting them would revoke standing access with no signal at the moment of loss. +Instead a per-organization pass that meets pre-fix organization-less rows for +names it seeds **says so loudly**, naming the rows and the remedy, and still +creates that organization's own copies. The failure this closes is the silent +no-op: a tenant-threaded pass that sees the old row through the driver's +compatibility arm, reads the name as already represented, and creates nothing +while reporting success. + +Two enforcement-plane reads are scoped in the same change, because the exposure +they carry only exists once per-organization copies exist: + +- `resolveUserAuthzContext`'s position name-sweep (`@objectstack/core`) resolved + `sys_position` by name across **every** organization, so the junction read + behind it collected another organization's `everyone` binding — a cross-organization + grant bleed, and an O(organizations) read on the per-request path. It is now + threaded through the driver's tenant chokepoint, keeping per-request resolution + O(the caller's own organization's catalog). +- plugin-security's permission-set `dbLoader` resolved sets by name unscoped, + with a `limit` equal to the number of names — correct while one row existed per + name, a truncation the moment copies exist. It is now scoped to the caller's + organization and its bound widened. + +Boot reconciliation is O(changed declarations): each pass reads what its +organization already has and writes only where a declaration actually differs, so +the common boot performs no writes at all. Steady state rides the +organization-creation hook. + +Cross-links #10119 / PR #10422, whose criteria-sweep scoping makes per-organization +sharing rules cheaper than the unscoped sweep they replace. diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index cd376a0f38..abf4f76b0f 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -118,10 +118,25 @@ function safeJsonParse(s: string, fallback: T): T { try { return JSON.parse(s) as T; } catch { return fallback; } } -async function tryFind(ql: any, object: string, where: any, limit = 100): Promise { +async function tryFind( + ql: any, + object: string, + where: any, + limit = 100, + /** + * Resolve inside ONE organization. Threaded into the execution context, so + * the read routes through `SqlDriver.applyTenantScope` — the governed + * chokepoint — rather than being re-implemented here as a bare equality. + * Omitted keeps the pre-existing installation-wide read, which is what the + * user-keyed reads above want (they are already narrowed by `user_id`) and + * what a `single`-posture deployment wants everywhere. + */ + organizationId?: string, +): Promise { if (!ql || typeof ql.find !== 'function') return []; try { - let rows = await ql.find(object, { where, limit, context: { isSystem: true } } as any); + const context = organizationId ? { isSystem: true, tenantId: organizationId } : { isSystem: true }; + let rows = await ql.find(object, { where, limit, context } as any); if (rows && (rows as any).value) rows = (rows as any).value; return Array.isArray(rows) ? rows : []; } catch { @@ -462,7 +477,31 @@ export async function resolveUserAuthzGrants( // with no `sys_position` row at all (`org_owner`, a membership-derived // role) has no flag to read and is untouched. if (grants.positions.length > 0) { - const positionRows = await tryFind(ql, 'sys_position', { name: { $in: grants.positions } }, 100); + // [#10103] Scoped to the CALLER's organization. `sys_position` spells + // its name index `unique: 'organization'` and its rows are materialized + // per organization, so several organizations hold a row named + // `everyone` (and one named after every declared position). Swept by + // name alone, this read returned EVERY organization's rows, and the + // junction read below then collected another organization's bindings — + // a cross-organization grant bleed, measured reachable from one tenant's + // resolution to another tenant's `everyone` binding. It also made the + // sweep O(organizations) on a table that is read on every request. + // + // Scoped by threading the organization into the context rather than by + // adding an `organization_id` predicate here: the driver's + // `applyTenantScope` is the one governed spelling of this wall, and a + // bare equality written at this call site would be a second, ungoverned + // implementation of it — the exact shape that produced the defect this + // card repairs. Per-request cost stays O(the caller's own organization's + // catalog). + // + // Limit raised with it: the cap has to admit this organization's rows + // alongside any organization-less ones the driver's compatibility arm + // still returns, or a caller silently loses positions. Those + // organization-less rows stay REACHABLE on purpose — they are not + // reaped, and grants point at them by row id, so dropping them here + // would revoke standing access silently. + const positionRows = await tryFind(ql, 'sys_position', { name: { $in: grants.positions } }, 200, tenantId); const deactivatedNames = new Set( positionRows.filter((r) => !isRowActive(r)).map((r) => r.name).filter(Boolean), ); diff --git a/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts b/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts index 9d53040d74..3718da5b23 100644 --- a/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts @@ -12,13 +12,35 @@ * `sys_member.role` for the org_* roles and the unscoped `admin_full_access` * grant for platform_admin — are NEVER changed by this seed. * - * Idempotent upsert-by-name, no prune. Rows are stamped `managed_by = 'platform'` - * (A4 #2920 unified vocab; formerly 'system') so tenants can see (but not - * repurpose) them. Runs on `kernel:ready` alongside the platform-admin and - * declared-role bootstraps. + * Idempotent upsert by `(name, organization_id)`, no prune. Rows are stamped + * `managed_by = 'platform'` (A4 #2920 unified vocab; formerly 'system') so + * tenants can see (but not repurpose) them. Runs on `kernel:ready` alongside the + * platform-admin and declared-role bootstraps, and again for each organization + * as it is created. + * + * ## Per organization under a walled posture + * + * The built-in names are seeded PER ORGANIZATION, copies and all. That is the + * ruled reading of what these rows are: `sys_position` spells its name index + * `unique: 'organization'`, `sys_user_position` assignments are already + * per-organization, and a walled tenant that cannot SEE `everyone` cannot bind + * anything to it. What is not copied is the SOURCE OF TRUTH behind the names — + * `sys_member.role` for the org_* roles and the unscoped `admin_full_access` + * grant for `platform_admin` — exactly as before: this seed remains a catalog + * projection, so per-organization copies of the catalog change no derivation. + * + * A `single`-posture deployment keeps exactly one organization-less pass. See + * `per-organization-catalog.ts` for the doctrine and for the loud guard that + * stands in place of a reap. */ import { BUILTIN_IDENTITY_NAMES, BUILTIN_IDENTITY_METADATA, EVERYONE_POSITION, GUEST_POSITION } from '@objectstack/spec'; +import { + resolveOwnOrganizationRow, + rowMatchesDeclaration, + seedCtx, + warnPreFixOrganizationLessRows, +} from './per-organization-catalog.js'; /** * [ADR-0090 D5/D9] Audience anchors seeded alongside the identity names. @@ -39,29 +61,32 @@ const AUDIENCE_ANCHOR_METADATA: Record { +async function tryFind(ql: any, object: string, where: any, limit = 100, organizationId?: string): Promise { try { - const rows = await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); + const rows = await ql.find(object, { where, limit }, { context: seedCtx(organizationId) }); return Array.isArray(rows) ? rows : []; } catch { return []; } } -async function tryInsert(ql: any, object: string, data: any): Promise { - try { return await ql.insert(object, data, { context: SYSTEM_CTX }); } catch { return null; } +async function tryInsert(ql: any, object: string, data: any, organizationId?: string): Promise { + try { return await ql.insert(object, data, { context: seedCtx(organizationId) }); } catch { return null; } } -async function tryUpdate(ql: any, object: string, data: any): Promise { - try { await ql.update(object, data, { context: SYSTEM_CTX }); return true; } catch { return false; } +async function tryUpdate(ql: any, object: string, data: any, organizationId?: string): Promise { + try { await ql.update(object, data, { context: seedCtx(organizationId) }); return true; } catch { return false; } } interface SeedOptions { logger?: { info: (m: string, meta?: Record) => void; warn: (m: string, meta?: Record) => void }; + /** + * Seed THIS organization's copies. Omitted = the `single`-posture pass, the + * one place an organization-less catalog row is the correct shape. + */ + organizationId?: string; } export async function bootstrapBuiltinRoles( @@ -71,8 +96,11 @@ export async function bootstrapBuiltinRoles( if (!ql || typeof ql.find !== 'function' || typeof ql.insert !== 'function') { return { seeded: 0, updated: 0 }; } + const organizationId = options.organizationId; let seeded = 0; let updated = 0; + let unchanged = 0; + const residue: string[] = []; const rows: Array<[string, { label: string; description: string }]> = [ ...BUILTIN_IDENTITY_NAMES.map((n) => [n, BUILTIN_IDENTITY_METADATA[n]] as [string, { label: string; description: string }]), ...Object.entries(AUDIENCE_ANCHOR_METADATA), @@ -82,16 +110,32 @@ export async function bootstrapBuiltinRoles( // PLATFORM-shipped (formerly stamped 'system'). Re-upserted every boot, so // legacy 'system' rows self-heal to 'platform' on the next kernel:ready. const fields = { label: meta.label, description: meta.description, managed_by: 'platform' }; - const existing = await tryFind(ql, 'sys_position', { name }, 1); - if (existing[0]?.id) { - if (await tryUpdate(ql, 'sys_position', { id: existing[0].id, ...fields })) updated += 1; + // Limit 5, not 1: a tenant-scoped read passes through `applyTenantScope`, + // whose compatibility arm returns organization-less rows alongside this + // organization's own. Asking for one row would hand back whichever the + // driver ordered first — and taking a pre-fix organization-less row as + // "already seeded" is exactly the silent no-op this pass must not perform. + const existing = await tryFind(ql, 'sys_position', { name }, 5, organizationId); + const { own, organizationLessResidue } = resolveOwnOrganizationRow(existing, organizationId); + if (organizationLessResidue) residue.push(name); + if (own?.id) { + // O(changed declarations): an unchanged row costs no write at all. + if (rowMatchesDeclaration(own, fields)) { unchanged += 1; continue; } + if (await tryUpdate(ql, 'sys_position', { id: own.id, ...fields }, organizationId)) updated += 1; } else { const created = await tryInsert(ql, 'sys_position', { id: genId('position'), name, ...fields, active: true, is_default: false, - }); + }, organizationId); if (created) seeded += 1; } } - options.logger?.info?.('[security] built-in identity names + audience anchors seeded into sys_position', { seeded, updated, total: rows.length }); + if (organizationId) { + warnPreFixOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); + } + if (seeded + updated > 0) { + options.logger?.info?.('[security] built-in identity names + audience anchors seeded into sys_position', { + seeded, updated, unchanged, total: rows.length, ...(organizationId ? { organization: organizationId } : {}), + }); + } return { seeded, updated }; } diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index aa861131ca..5fd2ff1229 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -12,7 +12,7 @@ * package's sets, uninstall is undefined, and no provenance axis exists. This * seeder closes that gap: * - * - each declared set is upserted by `name` with `managed_by: 'package'` and + * - each declared set is upserted by `(name, organization_id)` with `managed_by: 'package'` and * `package_id` = the registering package (`_packageId` stamped by the * SchemaRegistry / ADR-0010 `applyProtection`, with the spec-level * `packageId` (ADR-0086 D3) as the author-declared fallback); @@ -33,6 +33,20 @@ * (ADR-0094) — this module is the PACKAGE door only. Both project through the * shared {@link permissionSetRowFields} row shape so they can never hydrate * differently. + * + * ## Per organization under a walled posture + * + * The PACKAGE door runs ONE PASS PER ORGANIZATION when a wall is in force: + * `sys_permission_set` spells its name index `unique: 'organization'`, and an + * organization-less row was measured unreadable by every principal on a walled + * deployment — Layer 0's strict `organization_id = :tenant` AND-composes over + * the driver's compatibility arm and the conjunction is the strict equality + * alone. `single` posture keeps exactly one organization-less pass. + * + * The ENVIRONMENT door is deliberately NOT converted here: its + * organization-less residue (and `bootstrapPlatformAdmin`'s three platform + * defaults) stays outside this change, unreaped and warned about loudly. See + * `per-organization-catalog.ts` for the doctrine and the guard. */ import { @@ -44,11 +58,21 @@ import { type PermissionSeedOutcome, type ProjectionLogger, } from './permission-set-projection.js'; +import { + resolveOwnOrganizationRow, + rowMatchesDeclaration, + warnPreFixOrganizationLessRows, +} from './per-organization-catalog.js'; export type { PermissionSeedOutcome } from './permission-set-projection.js'; interface SeedOptions { logger?: ProjectionLogger; + /** + * Seed THIS organization's copies. Omitted = the `single`-posture pass, the + * one place an organization-less catalog row is the correct shape. + */ + organizationId?: string; } /** @@ -96,8 +120,11 @@ export async function upsertPackagePermissionSet( ps: any, packageId: string | null | undefined, logger?: SeedOptions['logger'], -): Promise { - const out: PermissionSeedOutcome = { seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0 }; + organizationId?: string, +): Promise { + const out: PermissionSeedOutcome & { organizationLessResidue: boolean } = { + seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0, organizationLessResidue: false, + }; if (!ps?.name) return out; // A `managed_by:'package'` row without a `package_id` would make uninstall // undefined again — the exact ambiguity ADR-0086 D3 exists to remove — so a @@ -107,7 +134,15 @@ export async function upsertPackagePermissionSet( return out; } - const existing = (await tryFind(ql, 'sys_permission_set', { name: ps.name }, 1))[0]; + // Limit 5, not 1: a tenant-scoped read passes through `applyTenantScope`, + // whose compatibility arm returns organization-less rows alongside this + // organization's own, and reading a pre-fix organization-less row as + // "already seeded" is the silent no-op the per-organization catalog exists to + // prevent. `resolveOwnOrganizationRow` is what tells the two apart. + const found = await tryFind(ql, 'sys_permission_set', { name: ps.name }, 5, organizationId); + const { own, organizationLessResidue } = resolveOwnOrganizationRow(found, organizationId); + if (organizationLessResidue) out.organizationLessResidue = true; + const existing = own; if (!existing?.id) { const created = await tryInsert(ql, 'sys_permission_set', { id: genId('ps'), @@ -116,7 +151,7 @@ export async function upsertPackagePermissionSet( active: true, package_id: packageId, managed_by: 'package', - }); + }, organizationId); if (created) out.seeded += 1; return out; } @@ -125,7 +160,12 @@ export async function upsertPackagePermissionSet( if (existing.package_id === packageId) { // Our own row — re-seed so the record always reflects the shipped/published // declaration (idempotent; covers version bumps without bookkeeping). - if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...permissionSetRowFields(ps) })) { + // O(changed declarations): a row that already carries every declared + // field costs NO write, so the common boot (nothing declared changed) + // performs reads only, per organization. + const fields = permissionSetRowFields(ps); + if (rowMatchesDeclaration(existing, fields)) return out; + if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...fields }, organizationId)) { out.updated += 1; } } else { @@ -163,20 +203,28 @@ export async function bootstrapDeclaredPermissions( } if (!Array.isArray(sets) || sets.length === 0) return out; + const organizationId = options.organizationId; + const residue: string[] = []; for (const ps of sets) { if (!ps?.name) continue; // Registry provenance first (ADR-0010 `_packageId`), author-declared // spec `packageId` (ADR-0086 D3) as fallback. const packageId: string | undefined = ps._packageId ?? ps.packageId ?? undefined; - const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger); + const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger, organizationId); out.seeded += r.seeded; out.updated += r.updated; out.skippedEnvAuthored += r.skippedEnvAuthored; out.skippedForeign += r.skippedForeign; + if (r.organizationLessResidue) residue.push(ps.name); } - options.logger?.info?.('[security] declared permission sets seeded into sys_permission_set (ADR-0086 D5)', { - ...out, total: sets.length, - }); + if (organizationId) { + warnPreFixOrganizationLessRows(options.logger, 'sys_permission_set', residue, organizationId); + } + if (out.seeded + out.updated > 0) { + options.logger?.info?.('[security] declared permission sets seeded into sys_permission_set (ADR-0086 D5)', { + ...out, total: sets.length, ...(organizationId ? { organization: organizationId } : {}), + }); + } return out; } diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts index f056b50c34..b72356881c 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-positions.ts @@ -6,7 +6,7 @@ * * Reads the validated `position` metadata (registered from the stack's `positions: []` * via `metadataService.list('position')`) and idempotently upserts each into - * `sys_position` by `name`, so the runtime position→permission-set resolution + * `sys_position` by `(name, organization_id)`, so the runtime position→permission-set resolution * (`resolveExecutionContext` → `sys_position` → `sys_position_permission_set`) and * sharing-rule position recipients stop being decorative. Runs on `kernel:ready` * alongside the platform-admin bootstrap. @@ -15,9 +15,22 @@ * HIERARCHY is NOT seeded here: per ADR-0057 D5 the position is a capability * bundle, and "manager sees subordinates" lives on the `sys_business_unit` * tree, not `sys_position.parent`. + * + * Runs ONE PASS PER ORGANIZATION under a walled posture: the object spells its + * name index `unique: 'organization'`, and a row belonging to no organization is + * invalid state there — it was measured unreadable by every principal, because + * Layer 0's strict `organization_id = :tenant` AND-composes over the driver's + * compatibility arm and leaves the strict equality alone. `single` posture keeps + * exactly one organization-less pass. Doctrine, and the loud guard that stands + * in place of a reap: `per-organization-catalog.ts`. */ -const SYSTEM_CTX = { isSystem: true }; +import { + resolveOwnOrganizationRow, + rowMatchesDeclaration, + seedCtx, + warnPreFixOrganizationLessRows, +} from './per-organization-catalog.js'; function genId(prefix: string): string { const rand = Math.random().toString(36).slice(2, 10); @@ -25,21 +38,26 @@ function genId(prefix: string): string { return `${prefix}_${ts}${rand}`; } -async function tryFind(ql: any, object: string, where: any, limit = 100): Promise { +async function tryFind(ql: any, object: string, where: any, limit = 100, organizationId?: string): Promise { try { - const rows = await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); + const rows = await ql.find(object, { where, limit }, { context: seedCtx(organizationId) }); return Array.isArray(rows) ? rows : []; } catch { return []; } } -async function tryInsert(ql: any, object: string, data: any): Promise { - try { return await ql.insert(object, data, { context: SYSTEM_CTX }); } catch { return null; } +async function tryInsert(ql: any, object: string, data: any, organizationId?: string): Promise { + try { return await ql.insert(object, data, { context: seedCtx(organizationId) }); } catch { return null; } } -async function tryUpdate(ql: any, object: string, data: any): Promise { - try { await ql.update(object, data, { context: SYSTEM_CTX }); return true; } catch { return false; } +async function tryUpdate(ql: any, object: string, data: any, organizationId?: string): Promise { + try { await ql.update(object, data, { context: seedCtx(organizationId) }); return true; } catch { return false; } } interface SeedOptions { logger?: { info: (m: string, meta?: Record) => void; warn: (m: string, meta?: Record) => void }; + /** + * Seed THIS organization's copies. Omitted = the `single`-posture pass, the + * one place an organization-less catalog row is the correct shape. + */ + organizationId?: string; } /** @@ -80,21 +98,38 @@ export async function bootstrapDeclaredPositions( } if (!Array.isArray(positions) || positions.length === 0) return { seeded: 0, updated: 0 }; + const organizationId = options.organizationId; let seeded = 0; let updated = 0; + let unchanged = 0; + const residue: string[] = []; for (const r of positions) { if (!r?.name) continue; const fields = { label: r.label ?? r.name, description: r.description ?? null }; - const existing = await tryFind(ql, 'sys_position', { name: r.name }, 1); - if (existing[0]?.id) { - if (await tryUpdate(ql, 'sys_position', { id: existing[0].id, ...fields })) updated += 1; + // Limit 5, not 1 — see `bootstrap-builtin-positions.ts` for why one row is + // the wrong question to ask a tenant-scoped read. + const existing = await tryFind(ql, 'sys_position', { name: r.name }, 5, organizationId); + const { own, organizationLessResidue } = resolveOwnOrganizationRow(existing, organizationId); + if (organizationLessResidue) residue.push(r.name); + if (own?.id) { + // O(changed declarations): an unchanged declaration costs no write. + if (rowMatchesDeclaration(own, fields)) { unchanged += 1; continue; } + if (await tryUpdate(ql, 'sys_position', { id: own.id, ...fields }, organizationId)) updated += 1; } else { const created = await tryInsert(ql, 'sys_position', { id: genId('position'), name: r.name, ...fields, active: true, is_default: false, - }); + }, organizationId); if (created) seeded += 1; } } - options.logger?.info?.('[security] declared positions seeded into sys_position', { seeded, updated, total: positions.length }); + if (organizationId) { + warnPreFixOrganizationLessRows(options.logger, 'sys_position', residue, organizationId); + } + if (seeded + updated > 0) { + options.logger?.info?.('[security] declared positions seeded into sys_position', { + seeded, updated, unchanged, total: positions.length, + ...(organizationId ? { organization: organizationId } : {}), + }); + } return { seeded, updated }; } diff --git a/packages/plugins/plugin-security/src/per-organization-catalog.test.ts b/packages/plugins/plugin-security/src/per-organization-catalog.test.ts new file mode 100644 index 0000000000..b5deb725af --- /dev/null +++ b/packages/plugins/plugin-security/src/per-organization-catalog.test.ts @@ -0,0 +1,316 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10103] The RBAC catalog is materialized PER ORGANIZATION. + * + * ## What was measured, and why these cases exist + * + * On a walled deployment every principal — org owner and platform admin alike — + * listed ZERO positions, permission sets and sharing rules while the tables held + * rows. The rows were all organization-less: this plugin's Layer 0 composes a + * strict `organization_id = :tenant`, the middleware ANDs it over the driver's + * `(organization_id = :tenant OR organization_id IS NULL)`, and the conjunction + * of the two is the strict equality alone. Nothing could be administered and a + * declared `hierarchy-security` could never be armed. + * + * The repair does not touch the wall at either layer. It gives the rows an + * owner. So every case here asserts about ROWS AND THEIR ORGANIZATION, through + * the real shipped seeders, never about a wall predicate. + * + * ## Why a real driver, and a real engine + * + * The whole defect lives in the interaction between a scope the ENGINE threads + * and a predicate the DRIVER emits. A hand-written engine double implements + * neither, so it reports green on a `tenantId` the real stack never applies — + * and it is precisely a double's silence that let the shipped behaviour read as + * correct for as long as it did. These cases therefore run a real `SqlDriver` on + * better-sqlite3 `:memory:` behind a real `ObjectQL`, and call the SHIPPED + * seeder functions rather than re-implementing their upserts. + * + * `case 6` additionally imports the REAL `resolveUserAuthzGrants` from + * `@objectstack/core` — the enforcement-plane half of this change — so the + * cross-organization grant bleed is pinned against the shipped resolver rather + * than a transcription of its shape. That import resolves through the package's + * `exports` to core's `dist/`, so core must be BUILT for this case to mean + * anything; the suite's `pnpm test` run is preceded by a dependency-closure + * build for exactly that reason. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { resolveUserAuthzGrants } from '@objectstack/core'; + +import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; +import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; +import { bootstrapDeclaredPermissions } from './bootstrap-declared-permissions.js'; +import { SysPosition } from './objects/sys-position.object.js'; +import { SysPermissionSet } from './objects/sys-permission-set.object.js'; +import { SysPositionPermissionSet } from './objects/sys-position-permission-set.object.js'; +import { SysUserPosition } from './objects/sys-user-position.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity'; + +const ORG_JIA = 'org_jia'; +const ORG_YI = 'org_yi'; +const ORGS = [ORG_JIA, ORG_YI] as const; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +/** One declared position and one packaged permission set to seed. */ +const STUB_REGISTRY = { + listItems: (type: string) => { + if (type === 'position') return [{ name: 'sales_manager', label: 'Sales Manager' }]; + if (type === 'permission') return [{ name: 'sales_readonly', label: 'Sales RO', _packageId: 'com.acme.crm', objects: {} }]; + return []; + }, +}; + +/** A logger that records what the seeders said, so a LOUD guard can be asserted. */ +function recordingLogger() { + const warns: Array<{ message: string; meta: any }> = []; + const infos: Array<{ message: string; meta: any }> = []; + return { + warns, + infos, + logger: { + info: (message: string, meta?: any) => { infos.push({ message, meta }); }, + warn: (message: string, meta?: any) => { warns.push({ message, meta }); }, + }, + }; +} + +async function boot(): Promise { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.per-org-catalog-10103', + name: 'Per-org catalog', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysPosition, SysPermissionSet, SysPositionPermissionSet, + SysUserPosition, SysUserPermissionSet, SysOrganization, + // Registered so the resolver's own reads SUCCEED. `tryFind` swallows a + // failed read into `[]`, so an unregistered table would make case 6's + // negative assertion pass for the wrong reason — nothing resolving at + // all rather than the sweep being scoped. Its positive control catches + // that too, but a fixture that makes the resolver work is the better + // instrument. + SysUser, SysMember, + ], + } as any); + await engine.syncSchemas(); + engines.push(engine); + for (const org of ORGS) { + await (engine as any).insert('sys_organization', { id: org, name: org }, { context: { isSystem: true } }); + } + return engine; +} + +/** The engine faced with the stub registry (`ObjectQL.registry` is a getter). */ +function withRegistry(engine: any): any { + return { + find: (o: string, q?: any, opt?: any) => engine.find(o, q, opt), + insert: (o: string, d: any, opt?: any) => engine.insert(o, d, opt), + update: (o: string, d: any, opt?: any) => engine.update(o, d, opt), + delete: (o: string, opt?: any) => engine.delete(o, opt), + registry: STUB_REGISTRY, + }; +} + +/** Ground truth: every stored row, straight off knex, past all tenancy. */ +async function stored(engine: ObjectQL, table: string): Promise { + const driver: any = (engine as any).getDriver(table); + return driver.knex(table).select('*'); +} + +/** Run the three catalog seeders for one organization (walled), or none (single). */ +async function seedCatalog(engine: ObjectQL, logger: any, organizationId?: string): Promise { + const ql = withRegistry(engine); + await bootstrapDeclaredPositions(ql, null, { logger, organizationId }); + await bootstrapDeclaredPermissions(ql, null, { logger, organizationId }); + await bootstrapBuiltinRoles(ql, { logger, organizationId }); +} + +const orgOf = (r: any): string | null => (r.organization_id ?? null); + +describe('[#10103] per-organization RBAC catalog materialization', () => { + it('1. walled: every seeded catalog row is STAMPED with its organization, and each organization holds its own copy', async () => { + const engine = await boot(); + const { logger } = recordingLogger(); + for (const org of ORGS) await seedCatalog(engine, logger, org); + + const positions = await stored(engine, 'sys_position'); + const sets = await stored(engine, 'sys_permission_set'); + + // The inversion of the shipped behaviour: no row belongs to nobody. + expect(positions.filter((r) => orgOf(r) === null)).toEqual([]); + expect(sets.filter((r) => orgOf(r) === null)).toEqual([]); + + // Stated per organization so a failure names the tenant that lost its + // catalog rather than printing a set diff. + for (const org of ORGS) { + const names = positions.filter((r) => orgOf(r) === org).map((r) => r.name).sort(); + expect(names).toEqual([ + 'everyone', 'guest', 'org_admin', 'org_member', 'org_owner', 'platform_admin', 'sales_manager', + ]); + expect(sets.filter((r) => orgOf(r) === org).map((r) => r.name)).toEqual(['sales_readonly']); + } + }); + + it('2. walled: the read that measured ZERO over the whole catalog now returns the organization’s own rows', async () => { + const engine = await boot(); + const { logger } = recordingLogger(); + for (const org of ORGS) await seedCatalog(engine, logger, org); + + // The AST plugin-security's Layer 0 AND-composes for `isolated`: a strict + // `organization_id = `, over which the driver adds its own + // predicate. This is the exact read that returned nothing. + const walled = await (engine as any).find('sys_position', { + where: { organization_id: ORG_JIA }, + context: { isSystem: true, tenantId: ORG_JIA }, + }); + const names = (walled as any[]).map((r) => r.name).sort(); + expect(names).toContain('everyone'); + expect(names).toContain('sales_manager'); + expect(names).toHaveLength(7); + + // And the wall still walls: org_yi's copies are not in org_jia's read. + for (const row of walled as any[]) expect(orgOf(row)).toBe(ORG_JIA); + }); + + it('3. THE silent no-op is gone: a per-organization pass over a PRE-FIX organization-less row creates the organization’s own copy AND says so loudly, by name', async () => { + const engine = await boot(); + const { logger, warns } = recordingLogger(); + + // Reproduce a pre-fix deployment exactly: one organization-less pass, the + // shipped behaviour, writing the rows every deployment that booted the old + // code carries. + await seedCatalog(engine, logger, undefined); + const before = await stored(engine, 'sys_position'); + expect(before.every((r) => orgOf(r) === null)).toBe(true); + const everyoneBefore = before.filter((r) => r.name === 'everyone'); + expect(everyoneBefore).toHaveLength(1); + + warns.length = 0; + for (const org of ORGS) await seedCatalog(engine, logger, org); + + // (a) NOT a no-op. The measured failure was: the tenant-threaded pass sees + // the organization-less row through the driver's compatibility arm, reads + // the name as already represented, takes the update branch and creates + // nothing — leaving the deployment as broken as before while reporting + // success. Each organization now has its OWN `everyone`. + const after = await stored(engine, 'sys_position'); + const everyoneAfter = after.filter((r) => r.name === 'everyone'); + // Sorted with `null` spelled explicitly rather than left to the default + // comparator, which stringifies and orders `null` first. + expect(everyoneAfter.map(orgOf).sort((a, b) => `${a}`.localeCompare(`${b}`))) + .toEqual([ORG_JIA, ORG_YI, null].sort((a, b) => `${a}`.localeCompare(`${b}`))); + expect(everyoneAfter.filter((r) => orgOf(r) === ORG_JIA)).toHaveLength(1); + expect(everyoneAfter.filter((r) => orgOf(r) === ORG_YI)).toHaveLength(1); + + // (b) LOUD. Named rows, named remedy — this is what stands in place of the + // reap, so a warning that merely counted would not discharge it. + const guard = warns.filter((w) => w.message.includes('pre-fix organization-less')); + expect(guard.length).toBeGreaterThanOrEqual(2); // at least once per organization + for (const org of ORGS) { + const forOrg = guard.filter((w) => w.meta?.organization === org); + expect(forOrg.length).toBeGreaterThan(0); + const names = forOrg.flatMap((w) => w.meta?.names ?? []); + expect(names).toContain('everyone'); + expect(names).toContain('sales_manager'); + } + // The remedy is spelled out, and the reason nothing is deleted with it. + const text = guard.map((w) => w.message).join(' '); + expect(text).toContain('re-initialize'); + expect(text).toContain('adopt'); + expect(text).toContain('NOT deleted'); + + // (c) and nothing was reaped — the organization-less row that grants point + // at is still there, which is why the guard is a warning and not a delete. + expect(everyoneAfter.filter((r) => orgOf(r) === null)).toHaveLength(1); + }); + + it('4. `single`-posture carve-out: exactly ONE organization-less pass, no copies, and NO warning', async () => { + const engine = await boot(); + const { logger, warns } = recordingLogger(); + + await seedCatalog(engine, logger, undefined); + await seedCatalog(engine, logger, undefined); // idempotent re-run, as at boot + + const positions = await stored(engine, 'sys_position'); + expect(positions.every((r) => orgOf(r) === null)).toBe(true); + expect(positions.filter((r) => r.name === 'everyone')).toHaveLength(1); + // An organization-less row is the CORRECT shape here, so the guard that + // calls it invalid state must stay silent. + expect(warns.filter((w) => w.message.includes('pre-fix organization-less'))).toEqual([]); + }); + + it('5. boot reconciliation is O(changed declarations): a second pass over unchanged declarations writes NOTHING', async () => { + const engine = await boot(); + const { logger } = recordingLogger(); + for (const org of ORGS) await seedCatalog(engine, logger, org); + + // `updated_at` is the observable a blind re-write would move. Snapshot the + // whole row set, re-run every pass, and require byte-equality — a pass that + // re-wrote unchanged rows would fail here even if the VALUES matched. + const snapshot = JSON.stringify((await stored(engine, 'sys_position')).map((r) => ({ ...r })).sort((a, b) => `${a.id}`.localeCompare(`${b.id}`))); + const setSnapshot = JSON.stringify((await stored(engine, 'sys_permission_set')).map((r) => ({ ...r })).sort((a, b) => `${a.id}`.localeCompare(`${b.id}`))); + + for (const org of ORGS) await seedCatalog(engine, logger, org); + + const again = JSON.stringify((await stored(engine, 'sys_position')).map((r) => ({ ...r })).sort((a, b) => `${a.id}`.localeCompare(`${b.id}`))); + const setAgain = JSON.stringify((await stored(engine, 'sys_permission_set')).map((r) => ({ ...r })).sort((a, b) => `${a.id}`.localeCompare(`${b.id}`))); + expect(again).toBe(snapshot); + expect(setAgain).toBe(setSnapshot); + }); + + it('6. the cross-organization grant bleed is closed: the REAL resolver no longer collects another organization’s binding', async () => { + const engine = await boot(); + const { logger } = recordingLogger(); + for (const org of ORGS) await seedCatalog(engine, logger, org); + + const positions = await stored(engine, 'sys_position'); + const everyoneOf = (org: string) => positions.find((r) => r.name === 'everyone' && orgOf(r) === org); + const jiaEveryone = everyoneOf(ORG_JIA); + const yiEveryone = everyoneOf(ORG_YI); + expect(jiaEveryone?.id).toBeTruthy(); + expect(yiEveryone?.id).toBeTruthy(); + expect(jiaEveryone.id).not.toBe(yiEveryone.id); + + // org_jia's admin binds a private set to org_jia's own `everyone`. + await (engine as any).insert('sys_permission_set', + { id: 'ps_jia_secret', name: 'jia_wide_grant', label: 'Jia', active: true }, + { context: { isSystem: true, tenantId: ORG_JIA } }); + await (engine as any).insert('sys_position_permission_set', + { id: 'pps_jia', position_id: jiaEveryone.id, permission_set_id: 'ps_jia_secret' }, + { context: { isSystem: true, tenantId: ORG_JIA } }); + + // A member of org_yi resolves. `everyone` is implicit for every + // authenticated member (ADR-0090 D5), so this is the ordinary path, not a + // contrived one — and it is the path that used to sweep `sys_position` by + // name across every organization and pick up org_jia's binding. + const yiGrants = await resolveUserAuthzGrants(engine as any, 'u_yi', { tenantId: ORG_YI }); + expect(yiGrants.permissions).not.toContain('jia_wide_grant'); + + // Positive control on the same resolver, from the same fixture: org_jia's + // own member DOES get it. Without this the assertion above would pass on a + // resolver that had simply stopped resolving anything. + await (engine as any).insert('sys_user_position', + { id: 'up_jia', user_id: 'u_jia', position: 'everyone' }, + { context: { isSystem: true, tenantId: ORG_JIA } }); + const jiaGrants = await resolveUserAuthzGrants(engine as any, 'u_jia', { tenantId: ORG_JIA }); + expect(jiaGrants.permissions).toContain('jia_wide_grant'); + }); +}); diff --git a/packages/plugins/plugin-security/src/per-organization-catalog.ts b/packages/plugins/plugin-security/src/per-organization-catalog.ts new file mode 100644 index 0000000000..b0d3272c48 --- /dev/null +++ b/packages/plugins/plugin-security/src/per-organization-catalog.ts @@ -0,0 +1,241 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Per-organization materialization of the RBAC catalog — the shared vocabulary + * the four declared/built-in seeders compile against. + * + * ## Why the catalog is materialized per organization + * + * `sys_position`, `sys_permission_set` and `sys_sharing_rule` all declare + * `organization_id` with no tenancy opt-out, and both spell their name index + * `{ fields: ['name'], unique: 'organization' }` — unique PER ORGANIZATION, not + * globally. The seeders nevertheless upserted by bare `name` under a bare + * `{ isSystem: true }` context, which stores `organization_id` NULL, so one row + * stood for every tenant. + * + * Under a walled posture that row is invalid state. It was measured to be + * unreadable by anyone: plugin-security's Layer 0 composes a STRICT + * `organization_id = :tenant` and the middleware ANDs it over the driver's + * `(organization_id = :tenant OR organization_id IS NULL)`, and the conjunction + * of the two is the strict equality alone — so on a walled deployment every + * principal, at every rung, listed ZERO positions, permission sets and sharing + * rules while the tables held rows. + * + * The repair ruled for that measurement does not touch the wall at either + * layer. It gives each organization its own row: upsert by + * `(name, organization_id)`, one pass per organization, so the answer to "which + * organization owns this row" is never NULL and never shared. + * + * ## The doctrine this file implements + * + * An organization-less row is INVALID STATE under a walled posture — refuse or + * warn loudly, never treat it as a platform-wide default. The older reading, in + * which a NULL organization marked a platform row visible to every tenant, + * survives only as DRIVER-LEVEL COMPATIBILITY BEHAVIOUR: `applyTenantScope` + * still emits the `OR organization_id IS NULL` arm, and this module depends on + * that arm being there — it is precisely how a per-organization pass can still + * SEE a pre-fix organization-less row and therefore say something about it. + * + * ## What replaces a reap + * + * #8617 reaped its pre-fix organization-less rows. This catalog does not, and + * the difference is deliberate rather than an omission: + * + * - a fresh walled deployment never mints an organization-less catalog row once + * these seeders run per organization, so there is nothing to migrate; + * - a `single`-posture deployment is where organization-less rows are the + * CORRECT shape, and the carve-out below leaves it byte-for-byte unchanged; + * - the rows a reap would delete are grant TARGETS — `sys_user_position`, + * `sys_position_permission_set` and `sys_user_permission_set` all point at + * them by row id — so deleting them revokes standing access with no signal at + * the moment of loss. #8617's reap could promise "NO grant changes" precisely + * because it never touched a junction table; here the junctions ARE the + * grants. + * + * So the pass says so instead. {@link warnPreFixOrganizationLessRows} names the + * rows and names the remedy, and — this is the load-bearing half — the pass + * still CREATES the organization's own copy. The failure shape it exists to + * prevent is the silent no-op: a tenant-threaded pass sees the pre-fix + * organization-less row through the driver's compatibility arm, reads the name + * as already represented, takes the update branch and creates nothing, leaving + * the deployment exactly as broken as before while reporting success. + * {@link resolveOwnOrganizationRow} is the one read that distinguishes "this + * organization has its row" from "somebody's organization-less row is visible + * here", and every seeder in this catalog routes through it. + */ + +import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security'; + +export type SeedLogger = { + info?: (m: string, meta?: Record) => void; + warn?: (m: string, meta?: Record) => void; +}; + +/** + * How many organizations one boot-time seeding sweep enumerates. + * + * Bounded for the same reason #8617 bounds its own sweep: this runs on + * `kernel:ready` and each organization costs a bounded number of reads. An + * organization past the bound is not left unseeded — the organization-creation + * hook covers every organization minted after this fix, and a redeploy re-runs + * the sweep — but the bound IS reported rather than silently truncating. + */ +export const SEED_ORGANIZATION_SCAN_LIMIT = 500; + +const ORGANIZATION_OBJECT = 'sys_organization'; + +/** + * The system context ONE seeding pass runs under. + * + * `organizationId` present ⇒ a tenant-scoped pass: reads route through + * `SqlDriver.applyTenantScope` and writes are stamped with that organization. + * `organizationId` absent is meaningful and correct in exactly one place — a + * `single`-posture deployment, which has no organization for a row to belong + * to — and is never a fallback for "we could not work out the organization". + */ +export function seedCtx(organizationId?: string): { isSystem: true; tenantId?: string } { + return organizationId ? { isSystem: true, tenantId: organizationId } : { isSystem: true }; +} + +/** Does this posture want per-organization catalog rows? */ +export function catalogIsPerOrganization(posture: TenancyPosture): boolean { + return postureEnforcesWall(posture); +} + +/** + * Enumerate the organizations whose catalog needs seeding. + * + * Returns `null` — not `[]` — when the read FAILS, because the two mean + * opposite things: zero organizations is "nothing to seed", an unreadable + * `sys_organization` is "we do not know". Conflating them turns an outage into + * a silent empty sweep, so a failure is warned and the caller must not proceed + * as though the installation had no tenants. + */ +export async function listSeedOrganizationIds( + ql: any, + logger?: SeedLogger, +): Promise { + let rows: any; + try { + rows = await ql.find(ORGANIZATION_OBJECT, { + fields: ['id'], + limit: SEED_ORGANIZATION_SCAN_LIMIT, + context: seedCtx(), + }); + } catch (e) { + logger?.warn?.( + '[security] could not enumerate organizations — the RBAC catalog was NOT seeded per ' + + 'organization at this call; seeding retries on the next boot and on organization creation', + { object: ORGANIZATION_OBJECT, error: (e as Error)?.message }, + ); + return null; + } + const ids = (Array.isArray(rows) ? rows : []) + .map((r: any) => r?.id) + .filter((id: unknown): id is string => typeof id === 'string' && id !== ''); + if (ids.length >= SEED_ORGANIZATION_SCAN_LIMIT) { + logger?.warn?.( + '[security] organization scan hit its bound — organizations past it are seeded when they are ' + + 'created and on the next boot sweep', + { scanned: ids.length, limit: SEED_ORGANIZATION_SCAN_LIMIT }, + ); + } + return ids; +} + +/** The organization a stored row belongs to, `null` for an organization-less one. */ +export function rowOrganizationId(row: any): string | null { + return (row?.organization_id ?? row?.organizationId) ?? null; +} + +/** + * Resolve THIS organization's own row for a name, out of what a tenant-scoped + * read returned. + * + * A scoped read passes through `applyTenantScope`, whose compatibility arm + * returns the caller's rows AND any organization-less ones. Those two are not + * interchangeable and the seeders must never treat them as such: + * + * - a row stamped with `organizationId` is this organization's — update it; + * - an organization-less row is a PRE-FIX residue that merely happens to be + * visible here. Reading it as "already seeded" is the silent no-op this + * catalog exists to prevent, so it is reported separately and never returned + * as the organization's own row. + * + * Under a `single`-posture pass (`organizationId` undefined) the + * organization-less row IS the row, which is the carve-out, so it is returned + * as `own` and nothing is flagged. + */ +export function resolveOwnOrganizationRow( + rows: any[], + organizationId?: string, +): { own: any | null; organizationLessResidue: any | null } { + const list = Array.isArray(rows) ? rows : []; + if (!organizationId) { + return { own: list[0] ?? null, organizationLessResidue: null }; + } + const own = list.find((r) => rowOrganizationId(r) === organizationId) ?? null; + const residue = list.find((r) => rowOrganizationId(r) === null) ?? null; + return { own, organizationLessResidue: residue }; +} + +/** + * The loud guard that stands in place of a reap. + * + * Called once per pass with everything the pass found, so an operator gets ONE + * actionable line naming the affected rows rather than a warning per name. The + * remedy is named because "invalid state" with no next step is not a diagnosis: + * either re-initialize the deployment (correct while it is pre-launch, which is + * the premise this whole repair was ruled on), or adopt each row by hand by + * stamping it with the organization that should own it. + * + * The pass that emits this has ALREADY created the organization's own copies — + * the warning describes leftovers, never a refusal to seed. + */ +export function warnPreFixOrganizationLessRows( + logger: SeedLogger | undefined, + object: string, + names: string[], + organizationId: string, +): void { + if (names.length === 0) return; + logger?.warn?.( + `[security] pre-fix organization-less ${object} rows are still present for names this ` + + `organization seeds — under a walled posture a row that belongs to no organization is ` + + `invalid state, not a platform-wide default. This organization's own rows WERE created, so ` + + `its catalog is complete; the leftovers below are readable through the driver's ` + + `compatibility arm and belong to nobody. Remedy: re-initialize the deployment, or adopt each ` + + `row by hand by stamping it with the organization that should own it. They are NOT deleted ` + + `automatically — grants (sys_user_position, sys_position_permission_set, ` + + `sys_user_permission_set) point at these row ids, so reaping them would revoke standing ` + + `access with no signal at the moment of loss.`, + { object, organization: organizationId, names: [...names].sort(), count: names.length }, + ); +} + +/** + * Does a stored row already carry every field a pass would write? + * + * The boot sweep is O(CHANGED DECLARATIONS), not O(organizations x rows) of + * blind writes: a pass reads what the organization already has (one bounded + * read), compares it against the declaration, and issues an update only where + * something actually differs. On the overwhelmingly common boot — nothing + * declared changed since the last one — every organization costs its reads and + * ZERO writes. Steady state does not ride this sweep at all; it rides the + * organization-creation hook, which seeds exactly the one new organization. + * + * Compared loosely on purpose: a column absent from a legacy row and a + * declaration that names it as `null`/`undefined` are the same state, and + * treating them as different would make every boot re-write every row, which is + * the cost this predicate exists to avoid. + */ +export function rowMatchesDeclaration(row: any, fields: Record): boolean { + if (!row) return false; + for (const [key, want] of Object.entries(fields)) { + const has = row[key]; + if ((has ?? null) === (want ?? null)) continue; + if (typeof want === 'boolean' && Boolean(has) === want) continue; + return false; + } + return true; +} diff --git a/packages/plugins/plugin-security/src/permission-set-projection.ts b/packages/plugins/plugin-security/src/permission-set-projection.ts index 55bdd0bf00..f5417acee3 100644 --- a/packages/plugins/plugin-security/src/permission-set-projection.ts +++ b/packages/plugins/plugin-security/src/permission-set-projection.ts @@ -68,6 +68,7 @@ */ import { PermissionSetSchema } from '@objectstack/spec/security'; +import { seedCtx } from './per-organization-catalog.js'; export const SYSTEM_CTX = { isSystem: true }; @@ -77,17 +78,27 @@ export function genId(prefix: string): string { return `${prefix}_${ts}${rand}`; } -export async function tryFind(ql: any, object: string, where: any, limit = 100): Promise { +/** + * The three write helpers take an optional `organizationId`, threaded into the + * execution context so reads route through `SqlDriver.applyTenantScope` and + * writes are stamped with that organization. Omitting it reproduces the + * pre-existing organization-less behaviour EXACTLY, which is what the ADR-0094 + * environment door still wants (its residue is deliberately outside the + * per-organization catalog work) and what a `single`-posture deployment wants. + * The PACKAGE door passes an organization under a walled posture — see + * `per-organization-catalog.ts`. + */ +export async function tryFind(ql: any, object: string, where: any, limit = 100, organizationId?: string): Promise { try { - const rows = await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); + const rows = await ql.find(object, { where, limit }, { context: seedCtx(organizationId) }); return Array.isArray(rows) ? rows : []; } catch { return []; } } -export async function tryInsert(ql: any, object: string, data: any): Promise { - try { return await ql.insert(object, data, { context: SYSTEM_CTX }); } catch { return null; } +export async function tryInsert(ql: any, object: string, data: any, organizationId?: string): Promise { + try { return await ql.insert(object, data, { context: seedCtx(organizationId) }); } catch { return null; } } -export async function tryUpdate(ql: any, object: string, data: any): Promise { - try { await ql.update(object, data, { context: SYSTEM_CTX }); return true; } catch { return false; } +export async function tryUpdate(ql: any, object: string, data: any, organizationId?: string): Promise { + try { await ql.update(object, data, { context: seedCtx(organizationId) }); return true; } catch { return false; } } export interface ProjectionLogger { diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 3a2ed20f24..0c0a8136d0 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -46,6 +46,12 @@ import { } from './suggested-audience-bindings.js'; import { cleanupPackagePermissions } from './cleanup-package-permissions.js'; import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; +import { + catalogIsPerOrganization, + listSeedOrganizationIds, + resolveOwnOrganizationRow, + seedCtx, +} from './per-organization-catalog.js'; import { bootstrapSystemCapabilities } from './bootstrap-system-capabilities.js'; import { normalizeManagedByVocab } from './normalize-managed-by.js'; import { bootstrapDeclaredCapabilities } from './bootstrap-declared-capabilities.js'; @@ -720,7 +726,39 @@ export class SecurityPlugin implements Plugin { * cleared on metadata change alongside the other schema-derived caches. */ private readonly objectSecurityMetaCache = new Map(); - private dbLoader?: (names: string[]) => Promise; + /** + * Permission-set loader FACTORY, parameterised by the caller's organization. + * A factory rather than a bare loader because the read is organization-scoped + * and the organization is a per-request fact — see where it is built for why + * an unscoped by-name read stops being correct once the catalog is + * materialized per organization. + */ + private dbLoaderFor?: (organizationId?: string) => (names: string[]) => Promise; + + /** + * The organization a permission-set resolution runs in — the caller's own + * active organization, never a scan across all of them. + * + * Undefined for a `single`-posture caller (and for any context carrying no + * organization at all), which lands on the organization-less surface that + * posture correctly has. Both spellings are read for the same reason the + * sharing service reads both: `tenantId` is the declared envelope field and + * `organizationId` is the cast one, and a hand-built context may carry + * either. + */ + private callerOrganizationId(context: any): string | undefined { + const id = (context?.organizationId ?? context?.tenantId); + return typeof id === 'string' && id !== '' ? id : undefined; + } + + /** + * The permission-set loader for ONE caller. `undefined` when no engine is + * wired, exactly as the un-parameterised loader used to be, so + * `resolvePermissionSets` keeps its "no db source" branch. + */ + private dbLoaderForContext(context: any): ((names: string[]) => Promise) | undefined { + return this.dbLoaderFor?.(this.callerOrganizationId(context)); + } /** * [#10757] Per-EXECUTION-CONTEXT memo for * {@link SecurityPlugin.resolvePermissionSetsForContext}. @@ -984,23 +1022,49 @@ export class SecurityPlugin implements Plugin { ); } - // Construct a dbLoader once that lets resolvePermissionSets + // Construct a dbLoader FACTORY once that lets resolvePermissionSets // surface user-defined permission sets from `sys_permission_set` - // (created via the admin UI) in addition to plugin-registered - // ones. Uses `isSystem` to bypass tenant RLS. - const dbLoader = ql - ? async (names: string[]) => { + // (created via the admin UI) in addition to plugin-registered ones. + // + // Scoped to the CALLER's organization rather than unscoped. Once the + // catalog is materialized per organization, several organizations hold a + // row for the same name, and a by-name `$in` under a bare + // `{ isSystem: true }` resolves whichever copy the driver ordered first — + // one tenant's grants answering another tenant's request. Threading the + // organization routes the read through `SqlDriver.applyTenantScope` (the + // governed chokepoint, unchanged by this work), and + // `resolveOwnOrganizationRow` then prefers this organization's own row over + // any organization-less one still standing. + // + // The `limit` moves with it. `names.length` was exactly right while one row + // existed per name, and is a TRUNCATION the moment copies exist: the driver + // returns this organization's rows AND any organization-less ones, so the + // cap must admit both or a caller silently loses sets. It stays bounded — a + // multiple of the names asked for, never unbounded. + const dbLoaderFor = ql + ? (organizationId?: string) => async (names: string[]) => { let rows: any; try { rows = await ql.find( 'sys_permission_set', - { where: { name: { $in: names } }, limit: names.length }, - { context: { isSystem: true } }, + { where: { name: { $in: names } }, limit: Math.max(names.length * 4, 20) }, + { context: seedCtx(organizationId) }, ); } catch { rows = []; } - const all = Array.isArray(rows) ? rows : rows?.records ?? []; + const fetched = Array.isArray(rows) ? rows : rows?.records ?? []; + // One row per NAME: this organization's own where it has one, an + // organization-less leftover only where it does not. + const byName = new Map(); + for (const name of new Set(names)) { + const own = resolveOwnOrganizationRow( + fetched.filter((r: any) => r?.name === name), + organizationId, + ).own; + if (own) byName.set(name, own); + } + const all = Array.from(byName.values()); // [ADR-0049] A DEACTIVATED set grants nothing. Not defence in depth // that nothing reaches: `resolvePermissionSetsForContext` requests // `context.positions` as permission-set NAMES too (a position name is @@ -1040,7 +1104,7 @@ export class SecurityPlugin implements Plugin { })); } : undefined; - this.dbLoader = dbLoader; + this.dbLoaderFor = dbLoaderFor; // [ADR-0090 D12] Delegated-admin gate shares the SAME permission-set // resolution as the CRUD middleware, so a delegate's authority and their @@ -2782,6 +2846,118 @@ export class SecurityPlugin implements Plugin { // transform is idempotent, but the once-flag avoids re-reading the registry / // re-logging on every runBootstrap re-entry). See managed-object-write-denies.ts. let managedDeniesApplied = false; + + // ── The RBAC catalog, materialized PER ORGANIZATION ────────────────── + // + // `sys_position` and `sys_permission_set` both spell their name index + // `unique: 'organization'`, and both were seeded organization-less. On a + // walled deployment that made the whole catalog unreadable to EVERY + // principal at every rung: this plugin's Layer 0 composes a strict + // `organization_id = :tenant` and the middleware ANDs it over the driver's + // `(organization_id = :tenant OR organization_id IS NULL)`, and the + // conjunction of the two is the strict equality alone. Nothing about the + // wall changes here, at either layer — the rows get an owner instead. + // + // The four steps are named ONCE and driven from two places (the boot sweep + // below, and the organization-creation hook further down) so a newly + // created organization can never be seeded differently from one present at + // boot. See `per-organization-catalog.ts` for the doctrine, the loud guard + // that stands in place of a reap, and why the boot sweep is O(changed + // declarations) rather than O(organizations x rows) of blind writes. + // + // `organizationId` undefined is the `single`-posture pass and reproduces + // the pre-existing organization-less behaviour exactly. + const seedCatalogPositions = async (organizationId?: string): Promise => { + try { + await bootstrapDeclaredPositions(ql, this.metadata, { logger: ctx.logger, organizationId }); + } catch (e) { + ctx.logger.warn('[security] declared-position seeding failed', { error: (e as Error).message, organization: organizationId }); + } + }; + const seedCatalogPermissions = async (organizationId?: string): Promise => { + try { + await bootstrapDeclaredPermissions(ql, this.metadata, { logger: ctx.logger, organizationId }); + } catch (e) { + ctx.logger.warn('[security] declared-permission seeding failed', { error: (e as Error).message, organization: organizationId }); + } + }; + const seedCatalogBuiltins = async (organizationId?: string): Promise => { + try { + await bootstrapBuiltinRoles(ql, { logger: ctx.logger, organizationId }); + } catch (e) { + ctx.logger.warn('[security] built-in role seeding failed', { error: (e as Error).message, organization: organizationId }); + } + }; + // [ADR-0090 D5] Bind the configured baseline set(s) to THIS organization's + // `everyone` anchor. Scoped for the same reason the anchors themselves are: + // once each organization holds its own `everyone` row, an unscoped + // `where: { name: 'everyone' }` would bind whichever copy the driver + // happened to order first — one organization's baseline landing in + // another's catalog. + const bindBaselineToEveryone = async (organizationId?: string): Promise => { + try { + for (const baselineName of this.baselinePermissionSets) { + const boot = this.bootstrapPermissionSets.find((p) => p.name === baselineName); + const offending = boot ? describeHighPrivilegeBits(boot) : null; + if (offending) { + ctx.logger.warn('[security] refusing to bind fallback set to everyone — high-privilege bits', { + set: baselineName, offending, + }); + continue; + } + // Limit 5, not 1: a tenant-scoped read returns organization-less rows + // alongside this organization's own, and the two are not + // interchangeable — `resolveOwnOrganizationRow` is what tells them + // apart. + const everyoneRows = await ql.find('sys_position', { where: { name: 'everyone' }, limit: 5, context: seedCtx(organizationId) }); + const everyone: any = resolveOwnOrganizationRow(Array.isArray(everyoneRows) ? everyoneRows : [], organizationId).own; + const setRows = await ql.find('sys_permission_set', { where: { name: baselineName }, limit: 5, context: seedCtx(organizationId) }); + const set: any = resolveOwnOrganizationRow(Array.isArray(setRows) ? setRows : [], organizationId).own; + if (everyone?.id && set?.id) { + const existing = await ql.find('sys_position_permission_set', { + where: { position_id: everyone.id, permission_set_id: set.id }, limit: 1, context: seedCtx(organizationId), + }); + if (!(Array.isArray(existing) && existing[0])) { + await ql.insert('sys_position_permission_set', { + id: `pps_${Date.now().toString(36)}${Math.random().toString(36).slice(2, 8)}`, + position_id: everyone.id, + permission_set_id: set.id, + }, { context: seedCtx(organizationId) }); + ctx.logger.info('[security] baseline set bound to everyone anchor (ADR-0090 D5)', { + set: baselineName, ...(organizationId ? { organization: organizationId } : {}), + }); + } + } + } + } catch (e) { + ctx.logger.warn('[security] everyone-anchor baseline binding failed (non-fatal)', { error: (e as Error).message }); + } + }; + /** + * Every catalog step for ONE organization, in boot order. Used by the + * organization-creation hook; the boot sweep drives the same four steps + * interleaved with the organization-independent bootstrap work. + */ + const seedCatalogForOrganization = async (organizationId?: string): Promise => { + await seedCatalogPositions(organizationId); + await seedCatalogPermissions(organizationId); + await seedCatalogBuiltins(organizationId); + await bindBaselineToEveryone(organizationId); + }; + /** + * Which passes this boot performs: one per organization under a walled + * posture, exactly one organization-less pass under `single`, and NONE when + * the organization enumeration failed (already warned — an unreadable + * `sys_organization` is "we do not know", never "there are no tenants"). + * A walled deployment with zero organizations yet is also none; each + * organization is seeded as it is created. + */ + const catalogSeedPasses = async (): Promise> => { + if (!catalogIsPerOrganization(this.tenancyPosture)) return [undefined]; + const ids = await listSeedOrganizationIds(ql, ctx.logger); + return ids ?? []; + }; + const runBootstrap = async () => { try { if (!managedDeniesApplied) { @@ -2800,12 +2976,15 @@ export class SecurityPlugin implements Plugin { const report = await bootstrapPlatformAdmin(ql, this.bootstrapPermissionSets, { logger: ctx.logger, }); + // Which organizations this boot seeds. Resolved ONCE per bootstrap run + // and reused by all four catalog steps, so a sweep costs one + // organization enumeration rather than four. + const catalogPasses = await catalogSeedPasses(); // [ADR-0057 D6 / #2077] Seed stack-declared positions into sys_position so they // stop being decorative (position→permission-set resolution + recipients). - try { - await bootstrapDeclaredPositions(ql, this.metadata, { logger: ctx.logger }); - } catch (e) { - ctx.logger.warn('[security] declared-position seeding failed', { error: (e as Error).message }); + // One pass per organization under a walled posture. + for (const organizationId of catalogPasses) { + await seedCatalogPositions(organizationId); } // [ADR-0086 D5] Seed stack-declared permission sets into // sys_permission_set with package provenance (managed_by:'package' + @@ -2813,10 +2992,8 @@ export class SecurityPlugin implements Plugin { // objects, and the admin surface finally sees them. Runs AFTER // bootstrapPlatformAdmin so the platform defaults keep their // insert-once, provenance-less shape (env config, never clobbered). - try { - await bootstrapDeclaredPermissions(ql, this.metadata, { logger: ctx.logger }); - } catch (e) { - ctx.logger.warn('[security] declared-permission seeding failed', { error: (e as Error).message }); + for (const organizationId of catalogPasses) { + await seedCatalogPermissions(organizationId); } // [ADR-0090 D5] The baseline→`everyone` binding runs LATER — after @@ -2927,10 +3104,8 @@ export class SecurityPlugin implements Plugin { } // [ADR-0068 D2] Seed the framework's reserved built-in identity positions // (platform_admin / org_*) so the role catalog is self-describing. - try { - await bootstrapBuiltinRoles(ql, { logger: ctx.logger }); - } catch (e) { - ctx.logger.warn('[security] built-in role seeding failed', { error: (e as Error).message }); + for (const organizationId of catalogPasses) { + await seedCatalogBuiltins(organizationId); } // [ADR-0090 D5] Bind the configured baseline set(s) to the `everyone` // audience anchor (idempotent). This makes the CLI/dev baseline @@ -2951,36 +3126,8 @@ export class SecurityPlugin implements Plugin { // narrower default than the runtime actually applies. The refusal is // PER SET: a high-privilege name is skipped loudly and the rest still // bind (the D5/D9 anchor gate is untouched, and each set faces it). - try { - for (const baselineName of this.baselinePermissionSets) { - const boot = this.bootstrapPermissionSets.find((p) => p.name === baselineName); - const offending = boot ? describeHighPrivilegeBits(boot) : null; - if (offending) { - ctx.logger.warn('[security] refusing to bind fallback set to everyone — high-privilege bits', { - set: baselineName, offending, - }); - continue; - } - const everyoneRows = await ql.find('sys_position', { where: { name: 'everyone' }, limit: 1, context: { isSystem: true } }); - const everyone: any = Array.isArray(everyoneRows) && everyoneRows[0] ? everyoneRows[0] : null; - const setRows = await ql.find('sys_permission_set', { where: { name: baselineName }, limit: 1, context: { isSystem: true } }); - const set: any = Array.isArray(setRows) && setRows[0] ? setRows[0] : null; - if (everyone?.id && set?.id) { - const existing = await ql.find('sys_position_permission_set', { - where: { position_id: everyone.id, permission_set_id: set.id }, limit: 1, context: { isSystem: true }, - }); - if (!(Array.isArray(existing) && existing[0])) { - await ql.insert('sys_position_permission_set', { - id: `pps_${Date.now().toString(36)}${Math.random().toString(36).slice(2, 8)}`, - position_id: everyone.id, - permission_set_id: set.id, - }, { context: { isSystem: true } }); - ctx.logger.info('[security] baseline set bound to everyone anchor (ADR-0090 D5)', { set: baselineName }); - } - } - } - } catch (e) { - ctx.logger.warn('[security] everyone-anchor baseline binding failed (non-fatal)', { error: (e as Error).message }); + for (const organizationId of catalogPasses) { + await bindBaselineToEveryone(organizationId); } // [ADR-0090 D5/D9] Reconcile the suggested-audience-binding surface: // every declared `isDefault: true` set that is not already bound to @@ -3157,10 +3304,80 @@ export class SecurityPlugin implements Plugin { void runOrgAdminBackfill(); } - // Per-organization seed data replay on `sys_organization` insert - // moved to `@objectstack/organizations` (along with - // `claimOrphanOrgRows` / `cloneOrgSeedData`). Install that - // plugin for multi-tenant deployments. + // ── Seed the RBAC catalog for a NEWLY CREATED organization ────────── + // + // The boot sweep covers the organizations that exist when the kernel is + // ready. Every organization minted afterwards needs the same catalog, and + // waiting for the next restart is not a repair: until it runs, that + // tenant's Setup lists no positions, no permission sets and no sharing + // rules — the exact symptom per-organization materialization exists to fix, + // only scoped to the newest tenant. Steady state therefore rides THIS hook, + // which is also why the boot sweep can afford to be a read-mostly + // reconciliation rather than a blind rewrite. + // + // Walled postures only: under `single` there is no organization to seed + // for, and the one organization-less pass at boot is the correct shape. + // Idempotent by construction (every seeder upserts by + // `(name, organization_id)`), so a double-fire is harmless. + ql.registerMiddleware(async (opCtx: any, next: () => Promise) => { + await next(); + if (opCtx?.object !== 'sys_organization') return; + const op = opCtx?.operation; + if (op !== 'insert' && op !== 'create') return; + if (!catalogIsPerOrganization(this.tenancyPosture)) return; + const organizationId = this.extractOrganizationIdFromWrite(opCtx); + if (!organizationId) { + // Never fall back to an organization-less pass here. That is the + // `single`-posture shape, and running it on a walled deployment would + // mint exactly the invalid-state row this change exists to remove. + ctx.logger.warn?.( + '[security] an organization was created but its id could not be read off the write — its ' + + 'RBAC catalog was NOT seeded now; the next boot sweep covers it', + { object: 'sys_organization' }, + ); + return; + } + try { + await seedCatalogForOrganization(organizationId); + ctx.logger.info?.('[security] RBAC catalog seeded for a newly created organization', { + organization: organizationId, + }); + } catch (e) { + ctx.logger.warn?.('[security] catalog seeding for a new organization failed (retried on next boot)', { + organization: organizationId, error: (e as Error).message, + }); + } + }); + + // Per-organization seed DATA replay on `sys_organization` insert (business + // records) lives in `@objectstack/organizations`, along with + // `claimOrphanOrgRows` / `cloneOrgSeedData`. Install that plugin for + // multi-tenant deployments. The hook above is a different thing: the RBAC + // CATALOG, which plugin-security owns and seeds from declarations. + } + + /** + * The id of the organization a `sys_organization` insert just created. + * + * Read from the write's own result first and its input second, because the + * two disagree in the case that matters: an insert that lets the engine mint + * the id carries none on the way in. Returns `undefined` rather than guessing + * — the caller refuses to run an organization-less pass on a walled + * deployment, which is the whole point. + */ + private extractOrganizationIdFromWrite(opCtx: any): string | undefined { + const candidates = [ + opCtx?.result?.id, + Array.isArray(opCtx?.result) ? opCtx.result[0]?.id : undefined, + opCtx?.data?.id, + opCtx?.doc?.id, + opCtx?.record?.id, + opCtx?.id, + ]; + for (const c of candidates) { + if (typeof c === 'string' && c !== '') return c; + } + return undefined; } async destroy(): Promise { @@ -3854,7 +4071,7 @@ export class SecurityPlugin implements Plugin { // only for a caller carrying `userId`, so a guest/anonymous caller lands // here with nothing. Resolve the configured fallback set explicitly — // the same two-step `/auth/me/permissions` performs. - permissionSets = await this.resolveFallbackPermissionSets(); + permissionSets = await this.resolveFallbackPermissionSets(context); } // No sets resolved (e.g. unauthenticated) → no field mask applies, exactly // as the middleware (getFieldPermissions([]) === {} → nothing deleted). @@ -4074,7 +4291,7 @@ export class SecurityPlugin implements Plugin { requested, this.metadata, this.bootstrapPermissionSets, - this.dbLoader, + this.dbLoaderForContext(context), { logger: this.logger }, ); // Post-resolution fallback — closes the fail-open hole where a populated @@ -4091,7 +4308,7 @@ export class SecurityPlugin implements Plugin { baseline, this.metadata, this.bootstrapPermissionSets, - this.dbLoader, + this.dbLoaderForContext(context), { logger: this.logger }, ); } @@ -4134,14 +4351,17 @@ export class SecurityPlugin implements Plugin { * * Returns `[]` when no baseline is configured or none of it resolves. */ - private async resolveFallbackPermissionSets(): Promise { + private async resolveFallbackPermissionSets(context?: any): Promise { const fallback = this.baselinePermissionSets; if (fallback.length === 0) return []; return this.permissionEvaluator.resolvePermissionSets( fallback, this.metadata, this.bootstrapPermissionSets, - this.dbLoader, + // Scoped like every other resolution: a guest of one organization must + // resolve THAT organization's baseline set, not whichever copy of the + // name the driver ordered first. + this.dbLoaderForContext(context), { logger: this.logger }, ); } diff --git a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts index 5f7a0dad71..4078d303b4 100644 --- a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts +++ b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts @@ -23,9 +23,29 @@ * - defensively, any stale pre-built package that still registers an old * `owner`-type / unmapped-recipient shape. * - * Seeding upserts via `SharingRuleService.defineRule` (idempotent by name) and - * MUST run before `listRules()`/`bindRuleHooks` so the lifecycle hooks bind to - * a populated table. + * Seeding upserts via `SharingRuleService.defineRule` and MUST run before + * `listRules()`/`bindRuleHooks` so the lifecycle hooks bind to a populated + * table. + * + * ## Per organization under a walled posture + * + * `defineRule` already keys its upsert on `(name, organization_id)` whenever the + * calling context carries an organization — it is the SEEDER that never + * supplied one, so every declared rule landed organization-less and, on a walled + * deployment, unreadable by every principal (Layer 0's strict + * `organization_id = :tenant` AND-composes over the driver's compatibility arm + * and the conjunction is the strict equality alone). This pass therefore runs + * ONCE PER ORGANIZATION under a walled posture, threading `tenantId` so + * `callerOrgId` resolves and the rule is stamped with its owner. + * + * `single` posture keeps exactly ONE organization-less pass, byte for byte the + * pre-existing behaviour: there an organization-less rule is the correct shape, + * and it is also the platform-global class `deleteRule` guards (#7795). + * + * Nothing is reaped. Pre-fix organization-less rows for names this pass seeds + * are named loudly instead, with their remedy — see + * `plugin-security/src/per-organization-catalog.ts` for why a reap is the wrong + * instrument on tables that grants point at. */ import type { SharingRuleService } from './sharing-rule-service.js'; @@ -35,6 +55,45 @@ import { isMatchAllCriteria } from './rule-criteria.js'; const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; +/** + * The context ONE seeding pass runs under. With an organization it makes + * `SharingRuleService.callerOrgId` resolve, so `defineRule` keys its upsert on + * `(name, organization_id)` and stamps the row with its owner. Without one it is + * `SYSTEM_CTX` unchanged — the `single`-posture pass. + */ +function seedRuleCtx(organizationId?: string): any { + return organizationId ? { ...SYSTEM_CTX, tenantId: organizationId } : SYSTEM_CTX; +} + +/** + * Which of the names this pass seeded ALSO still have a pre-fix + * organization-less row standing. + * + * Read under the organization's own scope on purpose: that read routes through + * `SqlDriver.applyTenantScope`, whose compatibility arm is the only reason an + * organization-less row is visible from inside a tenant at all. A bare + * unscoped read would answer a different question (every organization's rows), + * and a bare equality would answer none. + */ +async function findOrganizationLessRules( + engine: any, + names: string[], + organizationId: string, +): Promise { + if (names.length === 0) return []; + try { + const rows = await engine.find('sys_sharing_rule', { + where: { name: { $in: names } }, + limit: Math.max(names.length * 4, 50), + context: seedRuleCtx(organizationId), + }); + return (Array.isArray(rows) ? rows : []) + .filter((r: any) => ((r?.organization_id ?? r?.organizationId) ?? null) === null) + .map((r: any) => r?.name) + .filter((n: unknown): n is string => typeof n === 'string' && n !== ''); + } catch { return []; } +} + type Logger = { info?: (m: string, meta?: any) => void; warn?: (m: string, meta?: any) => void }; /** Map the spec `sharedWith.type` onto a runtime recipient_type, or null. */ @@ -98,6 +157,11 @@ export async function bootstrapDeclaredSharingRules( metadataService: any, engine: any, logger?: Logger, + /** + * Seed THIS organization's copies. Omitted = the `single`-posture pass, the + * one place an organization-less sharing rule is the correct shape. + */ + organizationId?: string, ): Promise<{ seeded: number; skipped: number }> { let rules: any[] = readDeclared(engine, 'sharing_rule'); if (rules.length === 0) { @@ -110,6 +174,7 @@ export async function bootstrapDeclaredSharingRules( let seeded = 0; let skipped = 0; + const seededNames: string[] = []; for (const r of rules) { if (!r?.name || !r?.object) { skipped += 1; continue; } const recipientType = mapRecipientType(r.sharedWith?.type); @@ -153,13 +218,35 @@ export async function bootstrapDeclaredSharingRules( // pristine rows keep receiving declared updates; admin-authored or // customized rows are never clobbered (defineRule seed-not-clobber). managedBy: 'package', - } as any, SYSTEM_CTX as any); + } as any, seedRuleCtx(organizationId) as any); + seededNames.push(r.name); seeded += 1; } catch (err: any) { logger?.warn?.('[sharing-rule] seed failed', { rule: r.name, error: err?.message }); skipped += 1; } } - logger?.info?.('[sharing-rule] declared rules seeded into sys_sharing_rule', { seeded, skipped, total: rules.length }); + // The loud guard that stands in place of a reap, measured rather than + // inferred. `defineRule` keys its upsert on `(name, organization_id)` once an + // organization is in hand, so a pre-fix organization-less row can never be + // mistaken for this organization's — which is precisely why the leftover is + // INVISIBLE to the pass itself and has to be looked for. Reading the return + // value of `defineRule` instead would be a check that can never fire: the row + // it hands back is always this organization's own. + const residue = organizationId ? await findOrganizationLessRules(engine, seededNames, organizationId) : []; + if (organizationId && residue.length > 0) { + logger?.warn?.( + '[sharing-rule] declared rules did not resolve to this organization\u2019s own rows — a pre-fix ' + + 'organization-less sys_sharing_rule row is standing in for names this organization seeds. Under ' + + 'a walled posture a rule that belongs to no organization is invalid state, not a platform-wide ' + + 'default. Remedy: re-initialize the deployment, or adopt each row by hand by stamping it with ' + + 'the organization that should own it. They are NOT deleted automatically — sys_record_share ' + + 'rows reference these rules by id, so reaping them would revoke standing access with no signal.', + { object: 'sys_sharing_rule', organization: organizationId, names: [...residue].sort(), count: residue.length }, + ); + } + logger?.info?.('[sharing-rule] declared rules seeded into sys_sharing_rule', { + seeded, skipped, total: rules.length, ...(organizationId ? { organization: organizationId } : {}), + }); return { seeded, skipped }; } diff --git a/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts b/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts index 56d280c4e4..b544334839 100644 --- a/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts +++ b/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts @@ -47,11 +47,33 @@ * org-stamped case is stated beside the null-org case it must not become. * * The NULL-org RECORD is here for the third direction: `applyTenantScope` - * emits `field = ? OR field IS NULL` on purpose (#2734 — a bare equality hid - * every platform-seeded row from every tenant). A scope "fixed" with a bare - * equality would pass the cross-org assertions and silently lose the platform - * record, so `deal_p1` is what distinguishes routing through the chokepoint - * from reimplementing a worse copy of it. + * emits `field = ? OR field IS NULL`, so a scope "fixed" with a bare equality + * would pass the cross-org assertions and silently lose that record. `deal_p1` + * is what distinguishes ROUTING THROUGH THE CHOKEPOINT from reimplementing a + * worse copy of it — and that is the whole of what it pins. + * + * ## What `deal_p1` does and does NOT assert (#10103) + * + * It used to be justified as the platform bucket: a NULL organization marks a + * platform row that must stay visible to every tenant (#2734, ADR-0120 D3). + * That reading is HISTORICAL BEHAVIOUR, SUPERSEDED as a general rule. The + * ruling on #10103 makes the single general rule the opposite one — a row + * belonging to no organization is INVALID STATE, to be refused or warned about + * loudly, never a platform-wide default — and demotes NULL-means-global to a + * DRIVER-LEVEL COMPATIBILITY NOTE. The RBAC catalog that reading was invented + * for (#2734's "every tenant admin saw ZERO RBAC rows") is now materialized per + * organization instead, so no NULL row is load-bearing anywhere. + * + * The assertion is nevertheless CORRECT and is kept verbatim, because the wall + * is untouched at both layers: `applyTenantScope` still emits its `OR ... IS + * NULL` arm — that arm IS the compatibility behaviour — so an org-stamped rule + * routed through it still matches an organization-less record. What changed is + * the JUSTIFICATION, not the behaviour: this case pins that the scope goes + * through the driver's chokepoint, and it would be equally load-bearing if the + * arm existed for no reason at all. Reading `deal_p1` as an endorsement of + * NULL-means-global is what is now wrong; deleting it would cost this suite its + * most valuable axis, and weakening it to a bare-equality expectation would + * pin the defect. */ import { describe, it, expect, afterEach } from 'vitest'; @@ -136,8 +158,11 @@ const open: SqlDriver[] = []; * deal_a2 ORG_A stage=lost <- ORG_A's own non-match (criteria still bite) * deal_b1 ORG_B stage=won <- the cross-org match this card is about * deal_b2 ORG_B stage=won <- second one, so a count cannot pass by luck - * deal_p1 (null) stage=won <- platform row: belongs to no tenant, so it - * belongs to no OTHER tenant either (#2734) + * deal_p1 (null) stage=won <- organization-less row, still returned by + * `applyTenantScope`'s compatibility arm. + * [#10103] Kept as the chokepoint-vs-bare- + * equality probe, no longer as a claim that + * NULL means platform-wide — see the header. */ async function boot(): Promise { const driver = new SqlDriver({ @@ -230,9 +255,14 @@ describe('[#10119] sharing-rule criteria sweep is scoped to the rule\'s organiza // failure names them rather than printing a set diff. expect(granted).not.toContain('deal_b1'); expect(granted).not.toContain('deal_b2'); - // ORG_A's own match, plus the null-org platform row the tenant - // chokepoint deliberately keeps visible (#2734). `deal_a2` is absent - // because the CRITERIA still bite — scoping did not replace them. + // ORG_A's own match, plus the organization-less row that `applyTenantScope` + // still returns through its compatibility arm. [#10103] That arm is + // driver-level COMPATIBILITY behaviour, not the platform-bucket rule it + // was once justified as — see this file's header. `deal_p1` earns its + // place here by distinguishing a scope routed through the chokepoint from + // one reimplemented as a bare equality, which is true either way. + // `deal_a2` is absent because the CRITERIA still bite — scoping did not + // replace them. expect(granted).toEqual(['deal_a1', 'deal_p1']); expect(result.matchedRecords).toBe(2); }); diff --git a/packages/plugins/plugin-sharing/src/sharing-plugin.ts b/packages/plugins/plugin-sharing/src/sharing-plugin.ts index 7ef6b327d0..0e38beb6d5 100644 --- a/packages/plugins/plugin-sharing/src/sharing-plugin.ts +++ b/packages/plugins/plugin-sharing/src/sharing-plugin.ts @@ -39,6 +39,69 @@ import { bindPrimaryBuHooks, backfillPrimaryBu } from './primary-bu-projection.j import { bindBusinessUnitTreeRecompute } from './bu-tree-recompute.js'; import { bindRecordShareCascade } from './record-share-cascade.js'; import { bootstrapDeclaredSharingRules } from './bootstrap-declared-sharing-rules.js'; +import { normalizeTenancyPosture, postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security'; + +/** + * How many organizations one boot-time sharing-rule seeding sweep enumerates. + * Bounded for the same reason plugin-security bounds its own: this runs during + * plugin start and each organization costs a bounded number of writes. An + * organization past the bound is covered by the organization-creation hook and + * by the next boot. + */ +const RULE_SEED_ORGANIZATION_SCAN_LIMIT = 500; + +/** + * Which passes one boot performs: exactly one organization-less pass under + * `single`, one per organization under a wall, and NONE when the organization + * enumeration failed — an unreadable `sys_organization` means "we do not know", + * never "there are no tenants", and seeding organization-less on that reading + * would mint precisely the invalid-state rows this change removes. + */ +async function resolveRuleSeedPasses( + engine: any, + posture: TenancyPosture, + logger?: { warn?: (m: string, meta?: any) => void }, +): Promise> { + if (!postureEnforcesWall(posture)) return [undefined]; + let rows: any; + try { + rows = await engine.find('sys_organization', { + fields: ['id'], + limit: RULE_SEED_ORGANIZATION_SCAN_LIMIT, + context: { isSystem: true }, + }); + } catch (e: any) { + logger?.warn?.('SharingServicePlugin: could not enumerate organizations — declared sharing rules ' + + 'were NOT seeded per organization at this boot; seeding retries on the next boot and on ' + + 'organization creation', { error: e?.message }); + return []; + } + return (Array.isArray(rows) ? rows : []) + .map((r: any) => r?.id) + .filter((id: unknown): id is string => typeof id === 'string' && id !== ''); +} + +/** + * The id of the organization a `sys_organization` insert just created. Read + * from the write's result first and its input second, because an insert that + * lets the engine mint the id carries none on the way in. Returns `undefined` + * rather than guessing — the caller refuses to run an organization-less pass on + * a walled deployment, which is the point. + */ +function createdOrganizationId(opCtx: any): string | undefined { + const candidates = [ + opCtx?.result?.id, + Array.isArray(opCtx?.result) ? opCtx.result[0]?.id : undefined, + opCtx?.data?.id, + opCtx?.doc?.id, + opCtx?.record?.id, + opCtx?.id, + ]; + for (const c of candidates) { + if (typeof c === 'string' && c !== '') return c; + } + return undefined; +} export interface SharingPluginOptions { /** Extra object names that bypass sharing entirely. */ @@ -567,14 +630,73 @@ export class SharingServicePlugin implements Plugin { // [ADR-0057 D6 / #2077] Seed stack-declared sharingRules into // sys_sharing_rule BEFORE listRules so the lifecycle hooks bind to a // populated table (previously rules were decorative — ruleCount: 0). - try { - let metadataService: IMetadataService | null = null; - try { metadataService = ctx.getService('metadata'); } catch { /* optional */ } - if (metadataService) { - await bootstrapDeclaredSharingRules(this.ruleService, metadataService, engine, ctx.logger as any); + // + // [#10103] ONE PASS PER ORGANIZATION under a walled posture. A rule + // seeded organization-less was measured unreadable by every principal + // there: plugin-security's Layer 0 composes a strict + // `organization_id = :tenant` and the middleware ANDs it over the + // driver's compatibility arm, so the conjunction is the strict + // equality alone and `sys_sharing_rule` listed ZERO. `single` posture + // keeps exactly one organization-less pass — which is also the + // platform-global rule class `deleteRule` guards (#7795). + // The posture, read EXACTLY the way this plugin already reads it for + // the hierarchy org gate (the `tenancy` service, ADR-0105 D1 / #5859), + // so the two layers can never disagree about whether a wall is in + // force. An unresolvable posture is NOT evidence of `single`: it is + // treated as walled, because seeding organization-less on a walled + // deployment is the failure this change removes, while an unnecessary + // per-organization pass on a `single` deployment costs one wasted read. + const sharingPosture = (): TenancyPosture => { + try { + const probe = ctx.getService('tenancy'); + const normalized = normalizeTenancyPosture(probe?.posture); + if (normalized) return normalized; + if (probe?.isolationActive === false) return 'single'; + } catch { /* absent — fall through to the walled default */ } + return 'isolated'; + }; + const seedDeclaredRules = async (organizationId?: string): Promise => { + try { + let metadataService: IMetadataService | null = null; + try { metadataService = ctx.getService('metadata'); } catch { /* optional */ } + if (metadataService && this.ruleService) { + await bootstrapDeclaredSharingRules( + this.ruleService, metadataService, engine, ctx.logger as any, organizationId, + ); + } + } catch (err: any) { + ctx.logger.warn('SharingServicePlugin: sharing-rule seeding failed', { + error: err?.message, organization: organizationId, + }); } - } catch (err: any) { - ctx.logger.warn('SharingServicePlugin: sharing-rule seeding failed', { error: err?.message }); + }; + for (const organizationId of await resolveRuleSeedPasses(engine, sharingPosture(), ctx.logger as any)) { + await seedDeclaredRules(organizationId); + } + + // Seed a NEWLY CREATED organization's rules too, not only the ones + // present at boot. Without it a tenant created after startup + // administers an empty sharing surface until the next restart — the + // same symptom, scoped to the newest tenant. Idempotent: `defineRule` + // upserts by `(name, organization_id)`. + if (typeof engine.registerMiddleware === 'function') { + engine.registerMiddleware(async (opCtx: any, next: () => Promise) => { + await next(); + if (opCtx?.object !== 'sys_organization') return; + const op = opCtx?.operation; + if (op !== 'insert' && op !== 'create') return; + if (!postureEnforcesWall(sharingPosture())) return; + const organizationId = createdOrganizationId(opCtx); + if (!organizationId) { + // Never fall back to an organization-less pass on a walled + // deployment: that is the `single`-posture shape and would mint + // exactly the invalid-state row this change removes. + ctx.logger.warn('SharingServicePlugin: an organization was created but its id was ' + + 'unreadable — its declared sharing rules were NOT seeded now; the next boot covers it'); + return; + } + await seedDeclaredRules(organizationId); + }); } if (typeof engine.registerHook === 'function' && typeof engine.unregisterHooksByPackage === 'function') { From 10cb75f2b5138353400fed7e9da00f8bd9c9eb15 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:50:38 +0000 Subject: [PATCH 2/3] test(security): pin the per-organization catalog seam through the producer's update dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:engine-double-contract` reads the suite's registry-facing seam as an engine double: it declares update() (and previously delete()) alongside engine siblings, and a delegating pass-through wrapping a real engine is exactly the shape that reads as "not a double" and then admits a call the real engine would reject. - update() now opens with `assertEngineUpdateDispatch(data, options)` from @objectstack/metadata-core, the producer's own predicate; - delete() is dropped entirely — none of the three seeders deletes, so the seam no longer makes a contract nothing exercises. The ledger movement is the TIGHTENING one: one row added to `engine-double-contract.pinned.json` recording new pinned coverage, written by the gate's own `--write`. The shrink-only `engine-double-contract.baseline.json` is untouched — no measured exemption was added. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../src/per-organization-catalog.test.ts | 21 ++++++++++++++++--- scripts/engine-double-contract.pinned.json | 5 +++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/plugins/plugin-security/src/per-organization-catalog.test.ts b/packages/plugins/plugin-security/src/per-organization-catalog.test.ts index b5deb725af..c380624c48 100644 --- a/packages/plugins/plugin-security/src/per-organization-catalog.test.ts +++ b/packages/plugins/plugin-security/src/per-organization-catalog.test.ts @@ -40,6 +40,7 @@ import { describe, it, expect, afterEach } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { resolveUserAuthzGrants } from '@objectstack/core'; +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { bootstrapBuiltinRoles } from './bootstrap-builtin-positions.js'; import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; @@ -118,13 +119,27 @@ async function boot(): Promise { return engine; } -/** The engine faced with the stub registry (`ObjectQL.registry` is a getter). */ +/** + * The REAL engine, faced with the stub registry (`ObjectQL.registry` is a + * getter, so it cannot simply be assigned). + * + * A delegating pass-through, not a fake: every verb reaches the real ObjectQL + * behind it, which is the whole point of this suite. `update` still opens with + * the PRODUCER's own dispatch predicate (#4550 / #5480 / #6277) rather than a + * hand-mirrored guard, because a seam that merely forwards is exactly the shape + * that reads as "not a double" and then admits a call the real engine would + * reject. `delete` is not declared at all — none of the three seeders deletes, + * and a verb no caller exercises would be a contract this seam does not have to + * make. + */ function withRegistry(engine: any): any { return { find: (o: string, q?: any, opt?: any) => engine.find(o, q, opt), insert: (o: string, d: any, opt?: any) => engine.insert(o, d, opt), - update: (o: string, d: any, opt?: any) => engine.update(o, d, opt), - delete: (o: string, opt?: any) => engine.delete(o, opt), + update: (o: string, d: any, opt?: any) => { + assertEngineUpdateDispatch(d, opt); + return engine.update(o, d, opt); + }, registry: STUB_REGISTRY, }; } diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 2cce7bbeb9..53ad86bbf1 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1391,6 +1391,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-security/src/per-organization-catalog.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-security/src/permission-denied-user-copy.test.ts", "verb": "delete", From 1452fe3209bb30e18f7e880295199fa9576630f7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 19:15:30 +0000 Subject: [PATCH 3/3] refactor(security): one spelling of "which catalog row is mine" across the merge The batched seed existence read (#10946) and the per-organization catalog (#10103) landed independently and each grew a resolution for the same question. Two implementations of one question is the shape that produced the defect this scoping repairs, so `seed-name-lookup.ts` now delegates to `resolveOwnOrganizationRow` and only translates its answer into the module's present/absent tri-state. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../src/bootstrap-declared-permissions.ts | 4 +-- .../plugin-security/src/seed-name-lookup.ts | 31 ++++++++----------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index f05653e23e..3a1fc96929 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -46,7 +46,6 @@ */ import { - SYSTEM_CTX, genId, permissionSetRowFields, recordDiffersFromBody, @@ -91,7 +90,8 @@ async function defaultLookup(ql: any, name: string, organizationId?: string): Pr const list = Array.isArray(rows) ? rows : Array.isArray(rows?.records) ? rows.records : null; if (list === null) return { status: 'unknown' }; // [#10103] This organization's own row answers; an organization-less leftover - // is reported beside `absent` and never returned as `present`. + // is reported beside `absent` and never returned as `present`. One spelling of + // that question for the whole catalog — see `per-organization-catalog.ts`. const { own, organizationLessResidue } = resolveOwnOrganizationRow(list, organizationId); if (own) return { status: 'present', row: own }; return organizationLessResidue diff --git a/packages/plugins/plugin-security/src/seed-name-lookup.ts b/packages/plugins/plugin-security/src/seed-name-lookup.ts index ff1736c2b1..98ddaa5320 100644 --- a/packages/plugins/plugin-security/src/seed-name-lookup.ts +++ b/packages/plugins/plugin-security/src/seed-name-lookup.ts @@ -90,17 +90,9 @@ * every respect: nothing is threaded, and the first row is the row. */ -const SYSTEM_CTX = { isSystem: true }; +import { resolveOwnOrganizationRow, seedCtx as lookupCtx } from './per-organization-catalog.js'; -/** The context one scoped read runs under. Bare when no organization applies. */ -function lookupCtx(organizationId?: string): { isSystem: true; tenantId?: string } { - return organizationId ? { isSystem: true, tenantId: organizationId } : SYSTEM_CTX; -} -/** The organization a stored row belongs to, `null` when it belongs to none. */ -function rowOrg(row: any): string | null { - return (row?.organization_id ?? row?.organizationId) ?? null; -} /** Names bound into one `$in` read. See the chunking note in the module header. */ export const NAME_CHUNK_SIZE = 500; @@ -223,20 +215,23 @@ function perItemIndex(ql: any, object: string, organizationId?: string): Existin } /** - * Which of the rows a read returned for ONE name is this organization's. + * Which of the rows a read returned for ONE name is this organization's, + * expressed as this module's tri-state. + * + * The organization split itself is NOT decided here — it delegates to + * {@link resolveOwnOrganizationRow}, the one spelling of that question the + * catalog has. Two spellings of "which row is mine" is exactly the shape that + * produced the defect this scoping repairs (one question, two implementations, + * the ungoverned copy winning), so this function only translates that answer + * into `present` / `absent` + leftover. * * Unscoped (the `single`-posture pass) the first row is the row, exactly as - * before. Scoped, a row stamped with this organization is the answer, and an - * organization-less one is a leftover reported alongside `absent` — never - * returned as `present`. See the module header for the failure that separation - * exists to prevent. + * before. */ function resolveForOrganization(rows: any[], organizationId?: string): ExistingLookupResult { - if (!organizationId) return rows[0] ? { status: 'present', row: rows[0] } : ABSENT; - const own = rows.find((r) => rowOrg(r) === organizationId); + const { own, organizationLessResidue } = resolveOwnOrganizationRow(rows, organizationId); if (own) return { status: 'present', row: own }; - const residue = rows.find((r) => rowOrg(r) === null); - return residue ? { status: 'absent', organizationLessResidue: residue } : ABSENT; + return organizationLessResidue ? { status: 'absent', organizationLessResidue } : ABSENT; } /**