Skip to content

Commit b80c554

Browse files
os-zhuangclaude
andcommitted
feat(plugin-security): ADR-0066 D2/D3 — enforce private posture, requiredPermissions, posture-gated RLS bypass
- D2 CRUD: a `private` object is not covered by a non-super-user '*' wildcard grant (resolveObjectPermission); access needs an explicit per-object grant OR the View/Modify All Data super-user wildcard. Explicit grants always honored. - D3 capability AND-gate: object `requiredPermissions` checked before the CRUD grant (caller's systemPermissions union); missing capability → deny. - ① posture-gated super-user RLS bypass: on a private/platform-global object, viewAllRecords bypasses read RLS and modifyAllRecords bypasses write RLS (incl. pre-image + post-image check) — so a platform admin who is also an org admin sees all rows, while the bypass never leaks cross-tenant data on ordinary tenant business objects. - New getObjectSecurityMeta cache (private/tenancy/requiredPermissions), invalidated on metadata change. Evaluator: getSystemPermissions, hasSuperuserRead/WriteBypass; checkObjectPermission/getEffectiveScope take an isPrivate option (backward-compatible). - 10 new tests (evaluator units + full-middleware integration). Full DTS green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e7e1ab9 commit b80c554

3 files changed

Lines changed: 326 additions & 9 deletions

File tree

packages/plugins/plugin-security/src/permission-evaluator.ts

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,30 @@ const OPERATION_TO_PERMISSION: Record<string, keyof ObjectPermission> = {
2626
*/
2727
const DESTRUCTIVE_OPERATIONS = new Set<string>(['transfer', 'restore', 'purge']);
2828

29+
/**
30+
* [ADR-0066 D2] Resolve the object permission a permission set contributes for
31+
* `objectName`, honouring the secure-by-default posture:
32+
*
33+
* - an EXPLICIT per-object grant (`ps.objects[objectName]`) always applies;
34+
* - the `'*'` wildcard applies to a `public` object (today's allow-by-default);
35+
* - for a `private` object the `'*'` wildcard applies ONLY when it carries the
36+
* super-user bypass bits (`viewAllRecords`/`modifyAllRecords` — the Salesforce
37+
* "View/Modify All Data" power). A plain `'*': {allowRead:true}` does NOT cover
38+
* a private object; access then requires an explicit per-object grant.
39+
*/
40+
function resolveObjectPermission(
41+
ps: PermissionSet,
42+
objectName: string,
43+
isPrivate: boolean,
44+
): ObjectPermission | undefined {
45+
const explicit = ps.objects?.[objectName];
46+
if (explicit) return explicit;
47+
const wild = ps.objects?.['*'];
48+
if (!wild) return undefined;
49+
if (!isPrivate) return wild;
50+
return wild.viewAllRecords || wild.modifyAllRecords ? wild : undefined;
51+
}
52+
2953
/**
3054
* PermissionEvaluator
3155
*
@@ -40,7 +64,9 @@ export class PermissionEvaluator {
4064
checkObjectPermission(
4165
operation: string,
4266
objectName: string,
43-
permissionSets: PermissionSet[]
67+
permissionSets: PermissionSet[],
68+
/** [ADR-0066 D2] When the object is `private`, the `'*'` wildcard only covers it if it is a super-user grant. */
69+
opts: { isPrivate?: boolean } = {},
4470
): boolean {
4571
const permKey = OPERATION_TO_PERMISSION[operation];
4672
if (!permKey) {
@@ -51,10 +77,10 @@ export class PermissionEvaluator {
5177
}
5278

5379
for (const ps of permissionSets) {
54-
// Honour the `'*'` wildcard sentinel — admin permission sets typically
55-
// grant blanket access via a single `objects: { '*': … }` entry rather
56-
// than enumerating every system object.
57-
const objPerm = ps.objects?.[objectName] ?? ps.objects?.['*'];
80+
// [ADR-0066 D2] Honour the `'*'` wildcard sentinel — admin permission
81+
// sets grant blanket access via a single `objects: { '*': … }` entry
82+
// but a `private` object is excluded from a non-super-user wildcard.
83+
const objPerm = resolveObjectPermission(ps, objectName, opts.isPrivate ?? false);
5884
if (objPerm) {
5985
// Check if modifyAllRecords is set (super-user bypass for write ops)
6086
if (['allowEdit', 'allowDelete'].includes(permKey) && objPerm.modifyAllRecords) {
@@ -87,13 +113,14 @@ export class PermissionEvaluator {
87113
opClass: 'read' | 'write',
88114
objectName: string,
89115
permissionSets: PermissionSet[],
116+
opts: { isPrivate?: boolean } = {},
90117
): 'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org' {
91118
const RANK = { own: 0, own_and_reports: 1, unit: 2, unit_and_below: 3, org: 4 } as const;
92119
const ORDER = ['own', 'own_and_reports', 'unit', 'unit_and_below', 'org'] as const;
93120
let widest = -1;
94121
let matched = false;
95122
for (const ps of permissionSets) {
96-
const op: any = ps.objects?.[objectName] ?? ps.objects?.['*'];
123+
const op: any = resolveObjectPermission(ps, objectName, opts.isPrivate ?? false);
97124
if (!op) continue;
98125
matched = true;
99126
if (opClass === 'read' && (op.viewAllRecords || op.modifyAllRecords)) return 'org';
@@ -106,6 +133,51 @@ export class PermissionEvaluator {
106133
return ORDER[widest < 0 ? 0 : widest];
107134
}
108135

136+
/**
137+
* [ADR-0066 D3] Union of `systemPermissions` (capabilities) the caller holds
138+
* across the resolved permission sets — used to enforce a resource's
139+
* `requiredPermissions` AND-gate.
140+
*/
141+
getSystemPermissions(permissionSets: PermissionSet[]): Set<string> {
142+
const out = new Set<string>();
143+
for (const ps of permissionSets) {
144+
for (const cap of ps.systemPermissions ?? []) out.add(cap);
145+
}
146+
return out;
147+
}
148+
149+
/**
150+
* [ADR-0066 D2 / ①] Does any resolved set grant the super-user READ bypass
151+
* (`viewAllRecords`/`modifyAllRecords`, the "View All Data" power) for the
152+
* object? Honours the private posture (see {@link resolveObjectPermission}).
153+
* The security plugin uses this to skip wildcard RLS on private/platform-global
154+
* objects so a platform admin sees all rows.
155+
*/
156+
hasSuperuserReadBypass(
157+
objectName: string,
158+
permissionSets: PermissionSet[],
159+
opts: { isPrivate?: boolean } = {},
160+
): boolean {
161+
for (const ps of permissionSets) {
162+
const op = resolveObjectPermission(ps, objectName, opts.isPrivate ?? false);
163+
if (op && (op.viewAllRecords || op.modifyAllRecords)) return true;
164+
}
165+
return false;
166+
}
167+
168+
/** [ADR-0066 D2 / ①] Super-user WRITE bypass (`modifyAllRecords`) for the object. */
169+
hasSuperuserWriteBypass(
170+
objectName: string,
171+
permissionSets: PermissionSet[],
172+
opts: { isPrivate?: boolean } = {},
173+
): boolean {
174+
for (const ps of permissionSets) {
175+
const op = resolveObjectPermission(ps, objectName, opts.isPrivate ?? false);
176+
if (op && op.modifyAllRecords) return true;
177+
}
178+
return false;
179+
}
180+
109181
/**
110182
* Get the merged field permissions for an object.
111183
* Returns a map of field names to their effective permissions.

packages/plugins/plugin-security/src/security-plugin.test.ts

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,102 @@ describe('SecurityPlugin', () => {
389389
expect(opCtx.ast.where).toEqual({ organization_id: 'org-1' });
390390
});
391391

392+
// -------------------------------------------------------------------------
393+
// ADR-0066 D2/D3 — private posture + requiredPermissions (full middleware)
394+
// -------------------------------------------------------------------------
395+
describe('ADR-0066 private posture + requiredPermissions (middleware)', () => {
396+
const memberSet: PermissionSet = {
397+
name: 'member_default', label: 'Member', isProfile: true,
398+
objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } },
399+
} as any;
400+
// Platform super-admin: super-user wildcard + the capability + a tenant_isolation
401+
// RLS policy (to prove the posture-gated bypass actually short-circuits it).
402+
const adminSet: PermissionSet = {
403+
name: 'admin_full_access', label: 'Admin', isProfile: true,
404+
objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true } },
405+
systemPermissions: ['manage_platform_settings'],
406+
rowLevelSecurity: [
407+
{ name: 'tenant_isolation', object: '*', operation: 'all', using: 'organization_id = current_user.organization_id' },
408+
],
409+
} as any;
410+
411+
it('DENIES a non-admin (plain wildcard) on a private object — wildcard does not cover it', async () => {
412+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
413+
const harness = makeMiddlewareCtx({
414+
permissionSets: [memberSet],
415+
objectFields: ['id', 'organization_id', 'signed_token'],
416+
schemaExtra: { access: { default: 'private' } },
417+
orgScoping: true,
418+
});
419+
await plugin.init(harness.ctx);
420+
await plugin.start(harness.ctx);
421+
const opCtx: any = {
422+
object: 'task', operation: 'find', ast: { where: undefined },
423+
context: { userId: 'u1', tenantId: 'org-1', roles: [], permissions: [] },
424+
};
425+
await expect(harness.run(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' });
426+
});
427+
428+
it('DENIES a caller missing the required capability (D3 AND-gate)', async () => {
429+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
430+
const harness = makeMiddlewareCtx({
431+
permissionSets: [memberSet],
432+
objectFields: ['id', 'organization_id', 'signed_token'],
433+
schemaExtra: { requiredPermissions: ['manage_platform_settings'] },
434+
});
435+
await plugin.init(harness.ctx);
436+
await plugin.start(harness.ctx);
437+
const opCtx: any = {
438+
object: 'task', operation: 'find', ast: { where: undefined },
439+
context: { userId: 'u1', tenantId: 'org-1', roles: [], permissions: [] },
440+
};
441+
await expect(harness.run(opCtx)).rejects.toMatchObject({
442+
name: 'PermissionDeniedError',
443+
message: expect.stringContaining('requires capability'),
444+
});
445+
});
446+
447+
it('ALLOWS the platform admin and BYPASSES wildcard RLS on a private object (read)', async () => {
448+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'admin_full_access' });
449+
const harness = makeMiddlewareCtx({
450+
permissionSets: [adminSet],
451+
objectFields: ['id', 'organization_id', 'signed_token'],
452+
schemaExtra: { access: { default: 'private' }, requiredPermissions: ['manage_platform_settings'] },
453+
orgScoping: true,
454+
});
455+
await plugin.init(harness.ctx);
456+
await plugin.start(harness.ctx);
457+
const opCtx: any = {
458+
object: 'task', operation: 'find', ast: { where: undefined },
459+
context: { userId: 'admin', tenantId: 'org-1', roles: ['admin_full_access'], permissions: [] },
460+
};
461+
await expect(harness.run(opCtx)).resolves.toBeDefined();
462+
// Without the bypass this would be { organization_id: 'org-1' } and the
463+
// platform admin would miss null-/cross-org rows (the sys_license bug).
464+
expect(opCtx.ast.where).toBeUndefined();
465+
});
466+
467+
it('BYPASSES the write pre-image check for a modifyAll admin on a private object', async () => {
468+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'admin_full_access' });
469+
const harness = makeMiddlewareCtx({
470+
permissionSets: [adminSet],
471+
objectFields: ['id', 'organization_id', 'signed_token'],
472+
schemaExtra: { access: { default: 'private' }, requiredPermissions: ['manage_platform_settings'] },
473+
orgScoping: true,
474+
findOneImpl: () => null, // would DENY if the pre-image check ran
475+
});
476+
await plugin.init(harness.ctx);
477+
await plugin.start(harness.ctx);
478+
const opCtx: any = {
479+
object: 'task', operation: 'update',
480+
data: { id: 'r1', signed_token: 'x' }, options: { where: { id: 'r1' } },
481+
context: { userId: 'admin', tenantId: 'org-1', roles: ['admin_full_access'], permissions: [] },
482+
};
483+
await expect(harness.run(opCtx)).resolves.toBeDefined();
484+
expect(harness.findOne).not.toHaveBeenCalled();
485+
});
486+
});
487+
392488
// -------------------------------------------------------------------------
393489
// getReadFilter service (ADR-0021 D-C) — the reusable READ scope the
394490
// analytics raw-SQL path bridges to. Must produce the SAME FilterCondition
@@ -891,6 +987,63 @@ describe('PermissionEvaluator', () => {
891987
});
892988
});
893989

990+
// ---------------------------------------------------------------------------
991+
// PermissionEvaluator — ADR-0066 D2 (private posture) + D3 (capabilities)
992+
// ---------------------------------------------------------------------------
993+
describe('PermissionEvaluator — ADR-0066 posture + capabilities', () => {
994+
const ps = (name: string, objects: PermissionSet['objects'] = {}, systemPermissions?: string[]): PermissionSet =>
995+
({ name, objects, ...(systemPermissions ? { systemPermissions } : {}) });
996+
const plainWildcard = ps('member', { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } });
997+
const superWildcard = ps('admin', { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true } });
998+
const explicitGrant = ps('license_reader', { sys_license: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false } });
999+
1000+
it('private object is NOT covered by a plain (non-superuser) wildcard', () => {
1001+
const ev = new PermissionEvaluator();
1002+
expect(ev.checkObjectPermission('find', 'sys_license', [plainWildcard], { isPrivate: true })).toBe(false);
1003+
// ...but a public object still is (today's allow-by-default).
1004+
expect(ev.checkObjectPermission('find', 'crm_account', [plainWildcard])).toBe(true);
1005+
expect(ev.checkObjectPermission('find', 'crm_account', [plainWildcard], { isPrivate: false })).toBe(true);
1006+
});
1007+
1008+
it('private object IS covered by a super-user wildcard (View/Modify All Data)', () => {
1009+
const ev = new PermissionEvaluator();
1010+
expect(ev.checkObjectPermission('find', 'sys_license', [superWildcard], { isPrivate: true })).toBe(true);
1011+
expect(ev.checkObjectPermission('insert', 'sys_license', [superWildcard], { isPrivate: true })).toBe(true);
1012+
expect(ev.checkObjectPermission('update', 'sys_license', [superWildcard], { isPrivate: true })).toBe(true);
1013+
});
1014+
1015+
it('private object IS covered by an explicit per-object grant (no superuser bit needed)', () => {
1016+
const ev = new PermissionEvaluator();
1017+
expect(ev.checkObjectPermission('find', 'sys_license', [explicitGrant], { isPrivate: true })).toBe(true);
1018+
expect(ev.checkObjectPermission('update', 'sys_license', [explicitGrant], { isPrivate: true })).toBe(false);
1019+
});
1020+
1021+
it('getSystemPermissions unions capabilities across sets', () => {
1022+
const ev = new PermissionEvaluator();
1023+
const a = ps('a', {}, ['manage_users', 'export_data']);
1024+
const b = ps('b', {}, ['export_data', 'manage_platform_settings']);
1025+
expect([...ev.getSystemPermissions([a, b])].sort()).toEqual(['export_data', 'manage_platform_settings', 'manage_users']);
1026+
expect([...ev.getSystemPermissions([plainWildcard])]).toEqual([]);
1027+
});
1028+
1029+
it('hasSuperuserReadBypass honours the private posture', () => {
1030+
const ev = new PermissionEvaluator();
1031+
// plain wildcard: bypasses on a public object, NOT on a private one.
1032+
expect(ev.hasSuperuserReadBypass('crm_account', [plainWildcard], { isPrivate: false })).toBe(false); // no viewAll bit
1033+
expect(ev.hasSuperuserReadBypass('sys_license', [superWildcard], { isPrivate: true })).toBe(true);
1034+
expect(ev.hasSuperuserReadBypass('sys_license', [plainWildcard], { isPrivate: true })).toBe(false);
1035+
// explicit grant without viewAll → no read bypass.
1036+
expect(ev.hasSuperuserReadBypass('sys_license', [explicitGrant], { isPrivate: true })).toBe(false);
1037+
});
1038+
1039+
it('hasSuperuserWriteBypass requires modifyAllRecords', () => {
1040+
const ev = new PermissionEvaluator();
1041+
const viewOnly = ps('vo', { '*': { allowRead: true, viewAllRecords: true } });
1042+
expect(ev.hasSuperuserWriteBypass('sys_license', [superWildcard], { isPrivate: true })).toBe(true);
1043+
expect(ev.hasSuperuserWriteBypass('sys_license', [viewOnly], { isPrivate: true })).toBe(false);
1044+
});
1045+
});
1046+
8941047
// ---------------------------------------------------------------------------
8951048
// FieldMasker
8961049
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)