Skip to content

Commit 2b4e67b

Browse files
committed
fix(plugin-security): an organization-less permission-set read resolves organization-less rows only
With no active organization the permission-set loader read sys_permission_set by name with no tenant, which the driver treats as an unscoped path: every organization's row of each requested name came back and the first one won. The requested names include the caller's positions, which with no active organization still carry every membership's role and the everyone anchor. The read now asks for organization-less rows only when no organization is active, the rule the grants resolver already applies to grant rows. Scoped reads are unchanged. The measurement probe becomes the pin. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
1 parent bf72f62 commit 2b4e67b

2 files changed

Lines changed: 212 additions & 72 deletions

File tree

Lines changed: 180 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,51 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* [#20555] MEASUREMENT PROBE — what an organization-less principal's
5-
* position-name fold resolves, over a real engine.
4+
* [#20555] A principal with NO active organization resolves permission sets
5+
* from the organization-less rows only — never from a set some organization
6+
* authored under a name the principal happens to hold as a position.
67
*
7-
* Probe stage: records readings only (to the file named by
8-
* `OS_TEST_PROBE_20555_OUT` when set). Converted into the pin once measured.
8+
* ## The seam
9+
*
10+
* `resolvePermissionSetsForContextUnmemoized` requests
11+
* `[...positions, ...permissions]` as permission-set NAMES, and anything not
12+
* declared in metadata or bootstrap is read from `sys_permission_set` by name
13+
* through `dbLoaderFor(callerOrganizationId(context))`. With no active
14+
* organization, `resolveUserAuthzGrants` still lists every current
15+
* membership's role (`org_member`, …) and the `everyone` anchor in
16+
* `positions`, and the by-name read carried no tenant — so it returned every
17+
* organization's row of each name, and the loader kept whichever came first.
18+
*
19+
* Measured on `main` at 1c761c0d, over this file's rig: a member of
20+
* `ORG_HOME` only, with no organization active, resolved the sets `ORG_OTHER`
21+
* had authored as `org_member` and `everyone` — their `systemPermissions` and
22+
* their object map (view/modify-all included) reached the resolved sets and
23+
* `getEffectiveObjectPermissions`. The same principal's GLOBAL grant to
24+
* `platform_ops` resolved `ORG_OTHER`'s same-named copy instead of the global
25+
* row it names. With `ORG_HOME` active the read was scoped and none of this
26+
* happened.
27+
*
28+
* ## The rule pinned here
29+
*
30+
* A row scoped to an organization applies only while that organization is
31+
* active; an organization-less row applies everywhere — the rule
32+
* `resolveUserAuthzGrants` already applies to grant rows. So the negative
33+
* pins below are paired with preservation pins: the global grants an
34+
* organization-less principal really holds (a global position folded onto a
35+
* global same-named set, a global user grant) keep resolving, and an
36+
* organization's own set still reaches its own members while it is active.
37+
*
38+
* ## The rig
39+
*
40+
* A real `ObjectQL` over a real `SqlDriver` (better-sqlite3 `:memory:`), the
41+
* real platform object definitions, the real `SecurityPlugin` resolved through
42+
* the `security` service it registers, and principals built by
43+
* `buildContextForUser` — the same `resolveUserAuthzGrants` the request path
44+
* runs. The plugin's `kernel:ready` bootstraps are collected and never fired:
45+
* the loader under test does not depend on them, and a bootstrap left running
46+
* would race the engine teardown.
947
*/
1048

11-
import { writeFileSync } from 'node:fs';
1249
import { describe, it, expect, afterEach } from 'vitest';
1350
import { ObjectQL } from '@objectstack/objectql';
1451
import { SqlDriver } from '@objectstack/driver-sql';
@@ -29,11 +66,16 @@ import { SecurityPlugin } from './security-plugin.js';
2966
import { buildContextForUser } from './explain-engine.js';
3067

3168
const SYS = { context: { isSystem: true } } as any;
32-
const ORG_A = 'org_a_author';
33-
const ORG_B = 'org_b_home';
34-
const USER_B = 'usr_b_member';
35-
/** Org-less principal holding GLOBAL grants only (A4: what must keep resolving). */
36-
const USER_G = 'usr_g_global';
69+
/** The organization that authors the colliding sets. Nobody below is its member except `USER_OTHER`. */
70+
const ORG_OTHER = 'org_other';
71+
/** The principals' own organization. */
72+
const ORG_HOME = 'org_home';
73+
/** A member of `ORG_HOME` only. */
74+
const USER_HOME = 'usr_home_member';
75+
/** A member of `ORG_HOME` only, holding two GLOBAL grants. */
76+
const USER_GLOBAL = 'usr_home_global';
77+
/** A member of `ORG_OTHER` — the control that the authored set resolves at all. */
78+
const USER_OTHER = 'usr_other_member';
3779

3880
const PROBE_OBJECT: any = {
3981
name: 'probe_ledger',
@@ -45,7 +87,10 @@ const PROBE_OBJECT: any = {
4587
},
4688
};
4789

48-
const FULL = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true };
90+
const FULL = {
91+
allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true,
92+
viewAllRecords: true, modifyAllRecords: true,
93+
};
4994

5095
const engines: ObjectQL[] = [];
5196
afterEach(async () => {
@@ -58,7 +103,7 @@ async function insertSet(e: any, id: string, name: string, organizationId: strin
58103
await e.insert('sys_permission_set', {
59104
id,
60105
name,
61-
label: `${organizationId ?? 'GLOBAL'} ${name}`,
106+
label: `${organizationId ?? 'global'} ${name}`,
62107
organization_id: organizationId,
63108
managed_by: 'admin',
64109
active: true,
@@ -75,8 +120,8 @@ async function boot(): Promise<ObjectQL> {
75120
);
76121
await engine.init();
77122
engine.registerApp({
78-
id: 'com.objectstack.orgless-position-fold-20555',
79-
name: 'Org-less position fold',
123+
id: 'com.objectstack.qa.orgless-position-name-fold',
124+
name: 'Organization-less position-name fold',
80125
version: '1.0.0',
81126
type: 'plugin',
82127
scope: 'system',
@@ -89,32 +134,43 @@ async function boot(): Promise<ObjectQL> {
89134
await engine.syncSchemas();
90135
engines.push(engine);
91136
const e = engine as any;
92-
await e.insert('sys_organization', { id: ORG_A, name: ORG_A, slug: 'a' }, SYS);
93-
await e.insert('sys_organization', { id: ORG_B, name: ORG_B, slug: 'b' }, SYS);
94-
await e.insert('sys_user', { id: USER_B, name: 'b', email: 'b@example.test' }, SYS);
95-
await e.insert('sys_user', { id: USER_G, name: 'g', email: 'g@example.test' }, SYS);
96-
// Both principals: members of ORG_B only — never of ORG_A.
97-
await e.insert('sys_member', { id: 'mem_b', user_id: USER_B, organization_id: ORG_B, role: 'member' }, SYS);
98-
await e.insert('sys_member', { id: 'mem_g', user_id: USER_G, organization_id: ORG_B, role: 'member' }, SYS);
99-
// ORG_A authors permission sets whose NAMES spell names the principals hold as positions.
100-
await insertSet(e, 'ps_a_org_member', 'org_member', ORG_A, ['manage_metadata', 'probe.cap.a_org_member']);
101-
await insertSet(e, 'ps_a_everyone', 'everyone', ORG_A, ['manage_metadata', 'probe.cap.a_everyone']);
102-
103-
// A4 — the GLOBAL grants USER_G really holds:
104-
// (1) a global position assignment folded onto a global same-named set;
105-
await e.insert('sys_user_position', { id: 'up_g_ops', user_id: USER_G, position: 'ops_lead', organization_id: null }, SYS);
106-
await insertSet(e, 'ps_global_ops_lead', 'ops_lead', null, ['probe.cap.global_ops_lead']);
107-
// (2) a global user grant to a global DB-authored set — but ORG_A holds a
108-
// same-named copy, inserted FIRST, so an unordered read meets it first.
109-
await insertSet(e, 'ps_a_platform_ops', 'platform_ops', ORG_A, ['probe.cap.a_platform_ops']);
110-
await insertSet(e, 'ps_global_platform_ops', 'platform_ops', null, ['probe.cap.global_platform_ops']);
137+
await e.insert('sys_organization', { id: ORG_OTHER, name: ORG_OTHER, slug: 'other' }, SYS);
138+
await e.insert('sys_organization', { id: ORG_HOME, name: ORG_HOME, slug: 'home' }, SYS);
139+
for (const id of [USER_HOME, USER_GLOBAL, USER_OTHER]) {
140+
await e.insert('sys_user', { id, name: id, email: `${id}@example.test` }, SYS);
141+
}
142+
await e.insert('sys_member', { id: 'mem_home', user_id: USER_HOME, organization_id: ORG_HOME, role: 'member' }, SYS);
143+
await e.insert('sys_member', { id: 'mem_global', user_id: USER_GLOBAL, organization_id: ORG_HOME, role: 'member' }, SYS);
144+
await e.insert('sys_member', { id: 'mem_other', user_id: USER_OTHER, organization_id: ORG_OTHER, role: 'member' }, SYS);
145+
146+
// ORG_OTHER authors sets named after what every member holds as a position.
147+
await insertSet(e, 'ps_other_org_member', 'org_member', ORG_OTHER, ['manage_metadata', 'probe.other_org_member']);
148+
await insertSet(e, 'ps_other_everyone', 'everyone', ORG_OTHER, ['manage_metadata', 'probe.other_everyone']);
149+
150+
// USER_GLOBAL's GLOBAL grants — what an organization-less principal really holds:
151+
// (1) a global position assignment, folded onto a global same-named set;
152+
await e.insert('sys_user_position', {
153+
id: 'up_global_ops_lead', user_id: USER_GLOBAL, position: 'ops_lead', organization_id: null,
154+
}, SYS);
155+
await insertSet(e, 'ps_global_ops_lead', 'ops_lead', null, ['probe.global_ops_lead']);
156+
// (2) a global user grant to a global set, beside ORG_OTHER's same-named
157+
// copy inserted FIRST — an unordered read meets the copy first.
158+
await insertSet(e, 'ps_other_platform_ops', 'platform_ops', ORG_OTHER, ['probe.other_platform_ops']);
159+
await insertSet(e, 'ps_global_platform_ops', 'platform_ops', null, ['probe.global_platform_ops']);
111160
await e.insert('sys_user_permission_set', {
112-
id: 'ups_g_ops', user_id: USER_G, permission_set_id: 'ps_global_platform_ops', organization_id: null,
161+
id: 'ups_global_platform_ops', user_id: USER_GLOBAL, permission_set_id: 'ps_global_platform_ops', organization_id: null,
113162
}, SYS);
114163
return engine;
115164
}
116165

117-
function observed(engine: any, reads: any[]): any {
166+
/** One `sys_permission_set` by-name read the loader issued, and the rows it got. */
167+
interface LoaderRead { rows: Array<{ id: string; name: string; organization_id: string | null }> }
168+
169+
/**
170+
* The real engine, `find` OBSERVED and forwarded verbatim. The dispatch-shaped
171+
* verbs open with the producer's own predicates (`check:engine-double-contract`).
172+
*/
173+
function observed(engine: any, reads: LoaderRead[]): any {
118174
return {
119175
registry: engine.registry,
120176
registerMiddleware: (...a: any[]) => engine.registerMiddleware?.(...a),
@@ -123,8 +179,6 @@ function observed(engine: any, reads: any[]): any {
123179
const r = await engine.find(o, q, opt);
124180
if (o === 'sys_permission_set' && q?.where?.name?.$in) {
125181
reads.push({
126-
where: q.where,
127-
context: opt?.context ?? q?.context,
128182
rows: (r as any[]).map((x) => ({ id: x.id, name: x.name, organization_id: x.organization_id ?? null })),
129183
});
130184
}
@@ -146,58 +200,114 @@ function observed(engine: any, reads: any[]): any {
146200
};
147201
}
148202

149-
async function securityServiceOver(engine: any, reads: any[]) {
203+
interface Rig {
204+
engine: ObjectQL;
205+
reads: LoaderRead[];
206+
security: any;
207+
}
208+
209+
async function rig(): Promise<Rig> {
210+
const engine = await boot();
211+
const reads: LoaderRead[] = [];
150212
const plugin = new SecurityPlugin();
151-
const svc = observed(engine, reads);
213+
const ql = observed(engine, reads);
152214
const services: Record<string, any> = {};
153215
const ctx: any = {
154216
logger: { info: () => {}, warn: () => {}, debug: () => {}, error: () => {} },
155217
registerService: (n: string, s: any) => { services[n] = s; },
156218
registerMiddleware: () => {},
219+
hook: () => { /* collected, never fired — see the file header */ },
157220
getService: (n: string) => {
158-
if (n === 'objectql') return svc;
221+
if (n === 'objectql') return ql;
159222
if (n === 'metadata') return { list: async () => [] };
160223
if (n === 'manifest') return { register: () => {} };
161224
return services[n];
162225
},
163226
};
164227
await plugin.init(ctx);
165228
await plugin.start(ctx);
166-
expect(services.security, 'security service never registered').toBeTruthy();
167-
return services.security;
229+
expect(services.security, 'the security service was never registered').toBeTruthy();
230+
return { engine, reads, security: services.security };
168231
}
169232

170-
async function reading(security: any, reads: any[], base: any, organizationId: string | undefined) {
171-
const ctx = organizationId ? { ...base, tenantId: organizationId, organizationId } : { ...base, tenantId: undefined };
172-
reads.length = 0;
173-
const sets = await security.resolvePermissionSetsForContext(ctx);
174-
const loaderReads = [...reads];
175-
const eff = await security.getEffectiveObjectPermissions({ ...ctx });
233+
/** What a principal resolves, read back through the registered `security` service. */
234+
async function resolve(r: Rig, userId: string, organizationId?: string) {
235+
const base = await buildContextForUser(r.engine, userId, Date.now(), organizationId);
236+
const context = organizationId
237+
? { ...base, tenantId: organizationId, organizationId }
238+
: { ...base, tenantId: undefined };
239+
r.reads.length = 0;
240+
const sets = await r.security.resolvePermissionSetsForContext(context);
241+
const loaderRows = r.reads.flatMap((read) => read.rows);
242+
const effective = await r.security.getEffectiveObjectPermissions(context);
243+
const byName = new Map<string, any>(sets.map((s: any) => [s.name, s]));
176244
return {
177-
positions: base.positions,
178-
permissions: base.permissions,
179-
coreSystemPermissions: base.systemPermissions,
180-
posture: base.posture,
181-
loaderReads,
182-
sets: sets.map((s: any) => ({ name: s.name, label: s.label, systemPermissions: s.systemPermissions })),
183-
effectiveProbeLedger: eff?.probe_ledger ?? null,
245+
positions: base.positions as string[],
246+
permissions: base.permissions as string[],
247+
loaderRows,
248+
setNames: sets.map((s: any) => s.name) as string[],
249+
byName,
250+
capabilities: new Set<string>(sets.flatMap((s: any) => s.systemPermissions ?? [])),
251+
ledger: effective?.probe_ledger ?? null,
184252
};
185253
}
186254

187-
describe('[#20555] PROBE', () => {
188-
it('records the org-less resolution', async () => {
189-
const engine = await boot();
190-
const reads: any[] = [];
191-
const security = await securityServiceOver(engine, reads);
192-
193-
const now = Date.now();
194-
const out = {
195-
userB_orgless: await reading(security, reads, await buildContextForUser(engine, USER_B, now, undefined), undefined),
196-
userB_home: await reading(security, reads, await buildContextForUser(engine, USER_B, now, ORG_B), ORG_B),
197-
userG_orgless: await reading(security, reads, await buildContextForUser(engine, USER_G, now, undefined), undefined),
198-
userG_home: await reading(security, reads, await buildContextForUser(engine, USER_G, now, ORG_B), ORG_B),
199-
};
200-
const file = process.env.OS_TEST_PROBE_20555_OUT;
201-
if (file) writeFileSync(file, JSON.stringify(out, null, 2));
255+
describe('[#20555] with no active organization, another organization\'s set named like a position does not reach the principal', () => {
256+
it('precondition: the organization-less principal really holds the colliding names as positions', async () => {
257+
const r = await rig();
258+
const orgless = await resolve(r, USER_HOME);
259+
// Without these two names in `positions` the pins below would pass for want
260+
// of a collision, not because the read is scoped.
261+
expect(orgless.positions).toEqual(expect.arrayContaining(['org_member', 'everyone']));
262+
}, 120_000);
263+
264+
it('the by-name read returns no other organization\'s row', async () => {
265+
const r = await rig();
266+
const orgless = await resolve(r, USER_HOME);
267+
expect(orgless.loaderRows.filter((row) => row.organization_id !== null)).toEqual([]);
268+
}, 120_000);
269+
270+
it('the authored sets\' capabilities and object map reach neither the resolved sets nor the effective map', async () => {
271+
const r = await rig();
272+
const orgless = await resolve(r, USER_HOME);
273+
expect(orgless.setNames).not.toContain('org_member');
274+
expect(orgless.setNames).not.toContain('everyone');
275+
expect([...orgless.capabilities].filter((c) => c === 'manage_metadata' || c.startsWith('probe.'))).toEqual([]);
276+
expect(orgless.ledger).toBeNull();
277+
}, 120_000);
278+
279+
it('a GLOBAL grant resolves the global row it names, not another organization\'s same-named copy', async () => {
280+
const r = await rig();
281+
const orgless = await resolve(r, USER_GLOBAL);
282+
expect(orgless.permissions).toContain('platform_ops');
283+
expect(orgless.byName.get('platform_ops')?.systemPermissions).toEqual(['probe.global_platform_ops']);
284+
expect(orgless.capabilities.has('probe.other_platform_ops')).toBe(false);
285+
}, 120_000);
286+
});
287+
288+
describe('[#20555] what the rule keeps', () => {
289+
it('a global position folded onto a global same-named set still resolves with no active organization', async () => {
290+
const r = await rig();
291+
const orgless = await resolve(r, USER_GLOBAL);
292+
expect(orgless.positions).toContain('ops_lead');
293+
expect(orgless.byName.get('ops_lead')?.systemPermissions).toEqual(['probe.global_ops_lead']);
294+
// …while the colliding names stay unresolved for this principal too.
295+
expect(orgless.setNames).not.toContain('org_member');
296+
expect(orgless.setNames).not.toContain('everyone');
297+
}, 120_000);
298+
299+
it('the global grants resolve identically with the principal\'s own organization active', async () => {
300+
const r = await rig();
301+
const home = await resolve(r, USER_GLOBAL, ORG_HOME);
302+
expect(home.byName.get('ops_lead')?.systemPermissions).toEqual(['probe.global_ops_lead']);
303+
expect(home.byName.get('platform_ops')?.systemPermissions).toEqual(['probe.global_platform_ops']);
304+
expect(home.setNames).not.toContain('org_member');
305+
}, 120_000);
306+
307+
it('CONTROL · the authoring organization\'s own member, with it active, does resolve its set — the set is resolvable at all', async () => {
308+
const r = await rig();
309+
const other = await resolve(r, USER_OTHER, ORG_OTHER);
310+
expect(other.byName.get('org_member')?.systemPermissions).toEqual(['manage_metadata', 'probe.other_org_member']);
311+
expect(other.ledger).toMatchObject({ viewAllRecords: true, modifyAllRecords: true });
202312
}, 120_000);
203313
});

0 commit comments

Comments
 (0)