Skip to content

Commit 504c8d5

Browse files
os-warrenclaude
andauthored
fix(security,sharing): materialize the RBAC catalog per organization (#11121)
* fix(security,sharing): materialize the RBAC catalog per organization (#10103) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx * test(security): pin the per-organization catalog seam through the producer's update dispatch `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5fdd463 commit 504c8d5

14 files changed

Lines changed: 1525 additions & 149 deletions
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
"@objectstack/plugin-sharing": minor
4+
"@objectstack/core": patch
5+
---
6+
7+
Materialize the RBAC catalog **per organization**, so a walled deployment can
8+
administer positions, permission sets and sharing rules again (#10103).
9+
10+
On a walled deployment (`group` / `isolated`) every principal — an organization
11+
owner and a platform admin alike — listed **zero** positions, permission sets
12+
and sharing rules while the tables held rows. Nothing could be bound through
13+
Setup, and a declared `hierarchy-security` could never be armed by an operator
14+
however loudly an app declared it.
15+
16+
Every row in those three tables was organization-less. plugin-security's Layer 0
17+
composes a strict `organization_id = :tenant` for a walled posture and the
18+
middleware ANDs it into the read AST over the driver's
19+
`(organization_id = :tenant OR organization_id IS NULL)`; the conjunction of the
20+
two is the strict equality alone, so the driver's null arm was annihilated on
21+
every authenticated read.
22+
23+
**The wall is not changed, at either layer.** The rows get an owner instead:
24+
25+
- `bootstrapDeclaredPositions`, `bootstrapBuiltinRoles`,
26+
`bootstrapDeclaredPermissions` (plugin-security) and
27+
`bootstrapDeclaredSharingRules` (plugin-sharing) upsert by
28+
`(name, organization_id)` and run **one pass per organization** under a walled
29+
posture — the framework built-ins (`platform_admin`, `org_*`, `everyone`,
30+
`guest`) included, matching `sys_user_position`, which is already
31+
per-organization, and matching both objects' own `unique: 'organization'` name
32+
index.
33+
- Seeding also fires on **organization creation**, not only at `kernel:ready`, so
34+
a tenant created after startup does not administer an empty catalog until the
35+
next restart.
36+
- `single` posture is **unchanged**: exactly one organization-less pass, which is
37+
the correct shape there.
38+
39+
An organization-less row is now invalid state under a walled posture. Nothing is
40+
reaped — grants (`sys_user_position`, `sys_position_permission_set`,
41+
`sys_user_permission_set`, `sys_record_share`) point at these rows by id, so
42+
deleting them would revoke standing access with no signal at the moment of loss.
43+
Instead a per-organization pass that meets pre-fix organization-less rows for
44+
names it seeds **says so loudly**, naming the rows and the remedy, and still
45+
creates that organization's own copies. The failure this closes is the silent
46+
no-op: a tenant-threaded pass that sees the old row through the driver's
47+
compatibility arm, reads the name as already represented, and creates nothing
48+
while reporting success.
49+
50+
Two enforcement-plane reads are scoped in the same change, because the exposure
51+
they carry only exists once per-organization copies exist:
52+
53+
- `resolveUserAuthzContext`'s position name-sweep (`@objectstack/core`) resolved
54+
`sys_position` by name across **every** organization, so the junction read
55+
behind it collected another organization's `everyone` binding — a cross-organization
56+
grant bleed, and an O(organizations) read on the per-request path. It is now
57+
threaded through the driver's tenant chokepoint, keeping per-request resolution
58+
O(the caller's own organization's catalog).
59+
- plugin-security's permission-set `dbLoader` resolved sets by name unscoped,
60+
with a `limit` equal to the number of names — correct while one row existed per
61+
name, a truncation the moment copies exist. It is now scoped to the caller's
62+
organization and its bound widened.
63+
64+
Boot reconciliation is O(changed declarations): each pass reads what its
65+
organization already has and writes only where a declaration actually differs, so
66+
the common boot performs no writes at all. Steady state rides the
67+
organization-creation hook.
68+
69+
Cross-links #10119 / PR #10422, whose criteria-sweep scoping makes per-organization
70+
sharing rules cheaper than the unscoped sweep they replace.

packages/core/src/security/resolve-authz-context.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,25 @@ function safeJsonParse<T>(s: string, fallback: T): T {
118118
try { return JSON.parse(s) as T; } catch { return fallback; }
119119
}
120120

121-
async function tryFind(ql: any, object: string, where: any, limit = 100): Promise<any[]> {
121+
async function tryFind(
122+
ql: any,
123+
object: string,
124+
where: any,
125+
limit = 100,
126+
/**
127+
* Resolve inside ONE organization. Threaded into the execution context, so
128+
* the read routes through `SqlDriver.applyTenantScope` — the governed
129+
* chokepoint — rather than being re-implemented here as a bare equality.
130+
* Omitted keeps the pre-existing installation-wide read, which is what the
131+
* user-keyed reads above want (they are already narrowed by `user_id`) and
132+
* what a `single`-posture deployment wants everywhere.
133+
*/
134+
organizationId?: string,
135+
): Promise<any[]> {
122136
if (!ql || typeof ql.find !== 'function') return [];
123137
try {
124-
let rows = await ql.find(object, { where, limit, context: { isSystem: true } } as any);
138+
const context = organizationId ? { isSystem: true, tenantId: organizationId } : { isSystem: true };
139+
let rows = await ql.find(object, { where, limit, context } as any);
125140
if (rows && (rows as any).value) rows = (rows as any).value;
126141
return Array.isArray(rows) ? rows : [];
127142
} catch {
@@ -462,7 +477,31 @@ export async function resolveUserAuthzGrants(
462477
// with no `sys_position` row at all (`org_owner`, a membership-derived
463478
// role) has no flag to read and is untouched.
464479
if (grants.positions.length > 0) {
465-
const positionRows = await tryFind(ql, 'sys_position', { name: { $in: grants.positions } }, 100);
480+
// [#10103] Scoped to the CALLER's organization. `sys_position` spells
481+
// its name index `unique: 'organization'` and its rows are materialized
482+
// per organization, so several organizations hold a row named
483+
// `everyone` (and one named after every declared position). Swept by
484+
// name alone, this read returned EVERY organization's rows, and the
485+
// junction read below then collected another organization's bindings —
486+
// a cross-organization grant bleed, measured reachable from one tenant's
487+
// resolution to another tenant's `everyone` binding. It also made the
488+
// sweep O(organizations) on a table that is read on every request.
489+
//
490+
// Scoped by threading the organization into the context rather than by
491+
// adding an `organization_id` predicate here: the driver's
492+
// `applyTenantScope` is the one governed spelling of this wall, and a
493+
// bare equality written at this call site would be a second, ungoverned
494+
// implementation of it — the exact shape that produced the defect this
495+
// card repairs. Per-request cost stays O(the caller's own organization's
496+
// catalog).
497+
//
498+
// Limit raised with it: the cap has to admit this organization's rows
499+
// alongside any organization-less ones the driver's compatibility arm
500+
// still returns, or a caller silently loses positions. Those
501+
// organization-less rows stay REACHABLE on purpose — they are not
502+
// reaped, and grants point at them by row id, so dropping them here
503+
// would revoke standing access silently.
504+
const positionRows = await tryFind(ql, 'sys_position', { name: { $in: grants.positions } }, 200, tenantId);
466505
const deactivatedNames = new Set<string>(
467506
positionRows.filter((r) => !isRowActive(r)).map((r) => r.name).filter(Boolean),
468507
);

packages/plugins/plugin-security/src/bootstrap-builtin-positions.ts

Lines changed: 61 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,35 @@
1212
* `sys_member.role` for the org_* roles and the unscoped `admin_full_access`
1313
* grant for platform_admin — are NEVER changed by this seed.
1414
*
15-
* Idempotent upsert-by-name, no prune. Rows are stamped `managed_by = 'platform'`
16-
* (A4 #2920 unified vocab; formerly 'system') so tenants can see (but not
17-
* repurpose) them. Runs on `kernel:ready` alongside the platform-admin and
18-
* declared-role bootstraps.
15+
* Idempotent upsert by `(name, organization_id)`, no prune. Rows are stamped
16+
* `managed_by = 'platform'` (A4 #2920 unified vocab; formerly 'system') so
17+
* tenants can see (but not repurpose) them. Runs on `kernel:ready` alongside the
18+
* platform-admin and declared-role bootstraps, and again for each organization
19+
* as it is created.
20+
*
21+
* ## Per organization under a walled posture
22+
*
23+
* The built-in names are seeded PER ORGANIZATION, copies and all. That is the
24+
* ruled reading of what these rows are: `sys_position` spells its name index
25+
* `unique: 'organization'`, `sys_user_position` assignments are already
26+
* per-organization, and a walled tenant that cannot SEE `everyone` cannot bind
27+
* anything to it. What is not copied is the SOURCE OF TRUTH behind the names —
28+
* `sys_member.role` for the org_* roles and the unscoped `admin_full_access`
29+
* grant for `platform_admin` — exactly as before: this seed remains a catalog
30+
* projection, so per-organization copies of the catalog change no derivation.
31+
*
32+
* A `single`-posture deployment keeps exactly one organization-less pass. See
33+
* `per-organization-catalog.ts` for the doctrine and for the loud guard that
34+
* stands in place of a reap.
1935
*/
2036

2137
import { BUILTIN_IDENTITY_NAMES, BUILTIN_IDENTITY_METADATA, EVERYONE_POSITION, GUEST_POSITION } from '@objectstack/spec';
38+
import {
39+
resolveOwnOrganizationRow,
40+
rowMatchesDeclaration,
41+
seedCtx,
42+
warnPreFixOrganizationLessRows,
43+
} from './per-organization-catalog.js';
2244

2345
/**
2446
* [ADR-0090 D5/D9] Audience anchors seeded alongside the identity names.
@@ -39,29 +61,32 @@ const AUDIENCE_ANCHOR_METADATA: Record<string, { label: string; description: str
3961
},
4062
};
4163

42-
const SYSTEM_CTX = { isSystem: true };
43-
4464
function genId(prefix: string): string {
4565
const rand = Math.random().toString(36).slice(2, 10);
4666
const ts = Date.now().toString(36);
4767
return `${prefix}_${ts}${rand}`;
4868
}
4969

50-
async function tryFind(ql: any, object: string, where: any, limit = 100): Promise<any[]> {
70+
async function tryFind(ql: any, object: string, where: any, limit = 100, organizationId?: string): Promise<any[]> {
5171
try {
52-
const rows = await ql.find(object, { where, limit }, { context: SYSTEM_CTX });
72+
const rows = await ql.find(object, { where, limit }, { context: seedCtx(organizationId) });
5373
return Array.isArray(rows) ? rows : [];
5474
} catch { return []; }
5575
}
56-
async function tryInsert(ql: any, object: string, data: any): Promise<any | null> {
57-
try { return await ql.insert(object, data, { context: SYSTEM_CTX }); } catch { return null; }
76+
async function tryInsert(ql: any, object: string, data: any, organizationId?: string): Promise<any | null> {
77+
try { return await ql.insert(object, data, { context: seedCtx(organizationId) }); } catch { return null; }
5878
}
59-
async function tryUpdate(ql: any, object: string, data: any): Promise<boolean> {
60-
try { await ql.update(object, data, { context: SYSTEM_CTX }); return true; } catch { return false; }
79+
async function tryUpdate(ql: any, object: string, data: any, organizationId?: string): Promise<boolean> {
80+
try { await ql.update(object, data, { context: seedCtx(organizationId) }); return true; } catch { return false; }
6181
}
6282

6383
interface SeedOptions {
6484
logger?: { info: (m: string, meta?: Record<string, any>) => void; warn: (m: string, meta?: Record<string, any>) => void };
85+
/**
86+
* Seed THIS organization's copies. Omitted = the `single`-posture pass, the
87+
* one place an organization-less catalog row is the correct shape.
88+
*/
89+
organizationId?: string;
6590
}
6691

6792
export async function bootstrapBuiltinRoles(
@@ -71,8 +96,11 @@ export async function bootstrapBuiltinRoles(
7196
if (!ql || typeof ql.find !== 'function' || typeof ql.insert !== 'function') {
7297
return { seeded: 0, updated: 0 };
7398
}
99+
const organizationId = options.organizationId;
74100
let seeded = 0;
75101
let updated = 0;
102+
let unchanged = 0;
103+
const residue: string[] = [];
76104
const rows: Array<[string, { label: string; description: string }]> = [
77105
...BUILTIN_IDENTITY_NAMES.map((n) => [n, BUILTIN_IDENTITY_METADATA[n]] as [string, { label: string; description: string }]),
78106
...Object.entries(AUDIENCE_ANCHOR_METADATA),
@@ -82,16 +110,32 @@ export async function bootstrapBuiltinRoles(
82110
// PLATFORM-shipped (formerly stamped 'system'). Re-upserted every boot, so
83111
// legacy 'system' rows self-heal to 'platform' on the next kernel:ready.
84112
const fields = { label: meta.label, description: meta.description, managed_by: 'platform' };
85-
const existing = await tryFind(ql, 'sys_position', { name }, 1);
86-
if (existing[0]?.id) {
87-
if (await tryUpdate(ql, 'sys_position', { id: existing[0].id, ...fields })) updated += 1;
113+
// Limit 5, not 1: a tenant-scoped read passes through `applyTenantScope`,
114+
// whose compatibility arm returns organization-less rows alongside this
115+
// organization's own. Asking for one row would hand back whichever the
116+
// driver ordered first — and taking a pre-fix organization-less row as
117+
// "already seeded" is exactly the silent no-op this pass must not perform.
118+
const existing = await tryFind(ql, 'sys_position', { name }, 5, organizationId);
119+
const { own, organizationLessResidue } = resolveOwnOrganizationRow(existing, organizationId);
120+
if (organizationLessResidue) residue.push(name);
121+
if (own?.id) {
122+
// O(changed declarations): an unchanged row costs no write at all.
123+
if (rowMatchesDeclaration(own, fields)) { unchanged += 1; continue; }
124+
if (await tryUpdate(ql, 'sys_position', { id: own.id, ...fields }, organizationId)) updated += 1;
88125
} else {
89126
const created = await tryInsert(ql, 'sys_position', {
90127
id: genId('position'), name, ...fields, active: true, is_default: false,
91-
});
128+
}, organizationId);
92129
if (created) seeded += 1;
93130
}
94131
}
95-
options.logger?.info?.('[security] built-in identity names + audience anchors seeded into sys_position', { seeded, updated, total: rows.length });
132+
if (organizationId) {
133+
warnPreFixOrganizationLessRows(options.logger, 'sys_position', residue, organizationId);
134+
}
135+
if (seeded + updated > 0) {
136+
options.logger?.info?.('[security] built-in identity names + audience anchors seeded into sys_position', {
137+
seeded, updated, unchanged, total: rows.length, ...(organizationId ? { organization: organizationId } : {}),
138+
});
139+
}
96140
return { seeded, updated };
97141
}

0 commit comments

Comments
 (0)