From e903e24f70e0fda290e64370d314a2249f94f18f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 03:40:39 +0000 Subject: [PATCH 1/3] =?UTF-8?q?fix(core):=20the=20effective=20map's=20supe?= =?UTF-8?q?r-user=20fold=20is=20per=20set=20only=20=E2=80=94=20no=20merged?= =?UTF-8?q?-bypass=20fold=20over=20a=20set's=20own=20narrower=20entry,=20n?= =?UTF-8?q?o=20create=20on=20modifyAllRecords=20alone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- .../security/effective-object-permissions.ts | 83 +++++++++++-------- 1 file changed, 49 insertions(+), 34 deletions(-) diff --git a/packages/core/src/security/effective-object-permissions.ts b/packages/core/src/security/effective-object-permissions.ts index da81b3192bb..d3208e7cf25 100644 --- a/packages/core/src/security/effective-object-permissions.ts +++ b/packages/core/src/security/effective-object-permissions.ts @@ -43,8 +43,9 @@ import { objectPermissionGrants, type EffectiveObjectPermission } from '@objects /** * Does the `'*'` entry carry the super-user READ bypass? * - * ONE reading of that question for this whole file — {@link foldWildcardSuperUser} - * asks it to decide whose `allowRead` it pulls true, and + * ONE reading of that question for this whole file — {@link foldSuperUserWildcardGrants} + * asks it of each set to decide which wildcards it folds (and the standalone + * {@link foldWildcardSuperUser} of the merged map), and * {@link seedSuperUserRestrictedObjects} asks it to decide whom it seeds for, so * the seed can never materialise an entry for a principal the fold leaves false. * It is the same bypass the server itself applies: `PermissionEvaluator`'s @@ -80,21 +81,23 @@ function wildcardGrantsSuperRead(objects: Record): boolean { * The super-user grant covers private/managed objects on the server, so the * read and write bypass reach every entry here. * - * [#20134] This reads the MERGED wildcard, so it knows only the two bypass - * bits, and only four entry bits. The per-set half, - * {@link foldSuperUserWildcardGrants}, carries what that reading cannot: the - * `transfer` the write bypass grants, and a super-user wildcard's own plain - * bits (`'*': { viewAllRecords, allowEdit }` edits). Two cells here are known - * to be BROADER than enforcement, and are left exactly as they are for the - * over-grant card of this family (#20136): - * - the merged bypass is folded into an entry the super-user set itself names - * narrower — `resolveObjectPermission` answers that set with its explicit - * entry, so the walled `organization_admin` is granted edit on - * `sys_position` here and refused it by the server; - * - `allowCreate` is pulled on the write bypass, which the spec's - * `objectPermissionGrants` deliberately gives no create cell: a - * `'*': { modifyAllRecords: true }` without `allowCreate` is granted create - * here and refused it by the server. + * [#20136] ⛔ NOT a step of {@link buildEffectiveObjectPermissions}, and not + * what the effective map is built from. It reads the MERGED wildcard, and a + * merged map no longer says which set names which object, so no fold over it + * alone can match the server. The map's super-user fold is + * {@link foldSuperUserWildcardGrants}, which reads each set on its own. This + * helper stays exported, body unchanged, for the callers of the released + * `@objectstack/plugin-hono-server` export of the same name. Against + * `PermissionEvaluator.checkObjectPermission` its answer is: + * - BROADER where the super-user set itself names the object narrower: + * `resolveObjectPermission` answers that set with its explicit entry, so + * the walled `organization_admin` would be granted edit on `sys_position` + * here and is refused it by the server; + * - BROADER on create: `allowCreate` is pulled on the write bypass, which the + * spec's `objectPermissionGrants` gives no create cell; + * - NARROWER on `transfer` and on a super-user wildcard's own plain bits, + * which it never sets. + * Build a map with {@link buildEffectiveObjectPermissions}, never with this. */ export function foldWildcardSuperUser(objects: Record): void { const wild = objects?.['*']; @@ -114,10 +117,10 @@ export function foldWildcardSuperUser(objects: Record): void { } /** - * [#20134] The per-set half of the super-user fold: each set's SUPER-USER - * `'*'` puts every bit it grants on each entry that set does not name, mutating - * the map in place. Runs after the seed (so every registered object has an - * entry) and BEFORE the managed-write clamp. + * [#20134] The super-user fold: each set's SUPER-USER `'*'` puts every bit it + * grants on each entry that set does not name, mutating the map in place. + * Runs after the seed (so every registered object has an entry) and BEFORE the + * managed-write clamp. * * Without it the map held only the four bits {@link foldWildcardSuperUser} * pulls, so `current_user.can(object, 'transfer')` answered `false` for @@ -126,6 +129,15 @@ export function foldWildcardSuperUser(objects: Record): void { * `true` through `modifyAllRecords`, and a super-read wildcard lost its own * plain bits (`'*': { viewAllRecords, allowEdit }` read only). * + * [#20136] And it is the WHOLE super-user fold: the merged + * {@link foldWildcardSuperUser} no longer runs before it. That pass put the + * merged bypass on every entry, the ones a super-user set names itself + * included, and pulled `allowCreate` on `modifyAllRecords` alone, so the map + * granted cells the server refuses — edit on `sys_position` for the walled + * `organization_admin`, create for a `'*': { modifyAllRecords: true }`, read on + * an entry a super-user set names as `{}`. Every bit that pass set that the + * server grants, this one sets too, per set. + * * The bits are DERIVED, not listed: for each grant bit, the spec's * `objectPermissionGrants` — the one fold `checkObjectPermission` itself asks * — is read on the set's wildcard. For a super-user wildcard its read cell is @@ -216,7 +228,8 @@ const GUARDED_WRITE_BUCKETS: ReadonlySet = new Set(['better-auth', 'engi * object opted the write affordance in via `userActions.{create,edit,delete}` * (e.g. sys_user opens `edit` for its profile fields). * - * Without this clamp, {@link foldWildcardSuperUser} would report `allowEdit:true` + * Without this clamp, the super-user fold ({@link foldSuperUserWildcardGrants}) + * would report `allowEdit:true` * for a platform admin on tables the guard actually blocks (sys_member, * sys_automation_run, …) — a false-POSITIVE that mirrors, inverted, the * false-negative the fold fixes. The real effective answer for a user-context @@ -317,7 +330,7 @@ function grantsAnyVerb(entry: Record): boolean { * left out: an absent entry and an all-`false` one read the same. * * A super-user wildcard is NOT materialised here: {@link seedSuperUserRestrictedObjects} - * and {@link foldWildcardSuperUser} carry it, and this pass leaves their answer + * and {@link foldSuperUserWildcardGrants} carry it, and this pass leaves their answer * byte-for-byte as it was for every subject holding no plain wildcard. */ function materializePlainWildcardCoverage( @@ -365,10 +378,10 @@ function materializePlainWildcardCoverage( * * A super-user's grant is usually the `'*'` wildcard, not explicit per-object * entries — so the objects it reaches never appear in the merged `objects` map. - * Seeding a `{allow*: false}` entry lets {@link foldWildcardSuperUser} and - * {@link foldSuperUserWildcardGrants} pull its grants true and lets + * Seeding a `{allow*: false}` entry lets {@link foldSuperUserWildcardGrants} + * pull its grants true and lets * {@link annotateEffectiveApiOperations} attach the effective operation set - * where the object narrows it. Runs BEFORE both folds. + * where the object narrows it. Runs BEFORE the fold. * * [#18990] Admitted by {@link wildcardGrantsSuperRead} — the READ bypass, so * BOTH super-user classes are seeded, and a plain wildcard grant carrying @@ -537,9 +550,11 @@ export interface EffectiveObjectPermissionsInputSet { * 3. {@link materializePlainWildcardCoverage} — [#20083] each set's plain * `'*'` onto the registered objects it covers for that set, so the map is * as broad as `checkObjectPermission` there; guarded like (2); - * 4. {@link foldWildcardSuperUser}, then {@link foldSuperUserWildcardGrants} — - * [#20134] each set's super-user `'*'` onto the entries it covers for that - * set, every bit it grants; + * 4. {@link foldSuperUserWildcardGrants} — [#20134] each set's super-user + * `'*'` onto the entries it covers for that set, every bit it grants; + * [#20136] and nothing more: the merged {@link foldWildcardSuperUser} is + * not a step, so an entry a super-user set names itself keeps that set's + * explicit answer, and `modifyAllRecords` alone grants no create; * 5. {@link clampManagedObjectWrites}; * 6. {@link annotateEffectiveApiOperations} — guarded like (2). * @@ -593,12 +608,12 @@ export function buildEffectiveObjectPermissions( } // Make the per-object map reflect the server's ACTUAL effective enforcement // = permission-set grant ∩ identity write guard (ADR-0057 D10, cited as an - // attribution, #9628): (1) fold the `'*'` super-user grant into every object - // so an admin's wildcard is not shadowed by another set's explicit deny — - // the merged bypass bits, then [#20134] every bit each set's super-user - // wildcard grants, `transfer` and its own plain bits included; + // attribution, #9628): (1) fold each set's `'*'` super-user grant into every + // entry that set does not name, so an admin's wildcard is not shadowed by + // another set's explicit deny — [#20134] every bit it grants, `transfer` and + // its own plain bits included; [#20136] per set only, never the merged + // bypass, which would also override the super-user set's OWN narrower entry; // (2) re-clamp guarded managed objects by their write affordance. - foldWildcardSuperUser(objects); foldSuperUserWildcardGrants(objects, sets); clampManagedObjectWrites(objects, schemaOf); // [#3391] Annotate the per-object effective API operation set. Guarded: on From 4c86410b3289c928d6336c5ee90df6852b65de6e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 03:47:58 +0000 Subject: [PATCH 2/3] =?UTF-8?q?test:=20hold=20every=20super-user=20shape?= =?UTF-8?q?=20at=200=20over-granted=20cells=20=E2=80=94=20the=20KNOWN=5FOV?= =?UTF-8?q?ER=5FGRANT=20rows=20flip,=20rows=20b,=20d-h=20join=20the=20pari?= =?UTF-8?q?ty=20table?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- .../effective-object-permissions.test.ts | 97 +++++++++++++++- ...t-user-endpoints-effective-objects.test.ts | 36 ++++-- .../src/fold-wildcard-superuser.test.ts | 7 ++ .../get-effective-object-permissions.test.ts | 108 ++++++++++-------- 4 files changed, 186 insertions(+), 62 deletions(-) diff --git a/packages/core/src/security/effective-object-permissions.test.ts b/packages/core/src/security/effective-object-permissions.test.ts index cd0367c6311..e6d092bedfd 100644 --- a/packages/core/src/security/effective-object-permissions.test.ts +++ b/packages/core/src/security/effective-object-permissions.test.ts @@ -9,9 +9,12 @@ * `effective-api-operations.test.ts`, which exercise them through that * package's unchanged re-exports). What is pinned HERE is the composition: * the merge rule, the order, the guards, and that the result aliases nothing. + * [#20136] `foldWildcardSuperUser` is no longer one of the steps composed: + * the super-user fold is per set (the `[#20136]` block below). */ import { describe, it, expect } from 'vitest'; +import { objectPermissionGrants } from '@objectstack/spec/security'; import { buildEffectiveObjectPermissions } from './effective-object-permissions.js'; describe('buildEffectiveObjectPermissions', () => { @@ -42,11 +45,16 @@ describe('buildEffectiveObjectPermissions', () => { sys_member: { name: 'sys_member', managedBy: 'better-auth' }, }; const map: any = buildEffectiveObjectPermissions( - [{ objects: { '*': { viewAllRecords: true, modifyAllRecords: true }, sys_member: { allowRead: true } } }], + [ + { objects: { '*': { viewAllRecords: true, modifyAllRecords: true } } }, + // [#20136] Named by ANOTHER set, so the super-user wildcard reaches it and the clamp has something to narrow. + { objects: { sys_member: { allowRead: true } } }, + ], { allSchemas: () => Object.values(schemas), schemaOf: (n) => schemas[n] }, ); - // Seed → fold: an entry nobody named, pulled true by the super-user bits. - expect(map.report).toMatchObject({ allowRead: true, allowEdit: true, allowCreate: true, allowDelete: true }); + // Seed → fold: an entry nobody named, pulled true by the super-user bits — + // [#20136] every bit they grant and no other: `modifyAllRecords` grants no create. + expect(map.report).toMatchObject({ allowRead: true, allowEdit: true, allowDelete: true, allowTransfer: true, allowCreate: false }); // Fold → clamp: the guard has the last word on a managed object's writes. expect(map.sys_member).toMatchObject({ allowRead: true, allowEdit: false, allowCreate: false, allowDelete: false }); // Annotate runs last, over the final entries. @@ -89,9 +97,11 @@ describe('buildEffectiveObjectPermissions', () => { it('with no schema source at all it is the bare merge plus the fold', () => { const map: any = buildEffectiveObjectPermissions([ - { objects: { '*': { modifyAllRecords: true }, deal: { allowRead: false } } }, + { objects: { '*': { modifyAllRecords: true } } }, + { objects: { deal: { allowRead: false } } }, ]); - expect(map.deal).toMatchObject({ allowRead: true, allowEdit: true, allowCreate: true, allowDelete: true }); + expect(map.deal).toMatchObject({ allowRead: true, allowEdit: true, allowDelete: true, allowTransfer: true }); + expect(map.deal.allowCreate).not.toBe(true); expect(map.deal.apiOperations).toBeUndefined(); }); }); @@ -284,6 +294,83 @@ describe('[#20134] super-user wildcard: every bit it grants, per set', () => { }); }); +/** + * [#20136] The super-user fold is per set, and it is the only one: an entry a + * super-user set names ITSELF is that set's whole answer for the object + * (`resolveObjectPermission`), and a wildcard lends only the bits the spec's + * `objectPermissionGrants` says it grants — `modifyAllRecords` grants no + * create. The merged-bypass fold that used to run first granted both, so the + * map answered `true` where `PermissionEvaluator.checkObjectPermission` + * refuses. Each case reads the entry the way `current_user.can()` does. + * + * The enumeration over every super-user shape — shipped sets and authored + * rows, every verb, the real `can()` against the real evaluator — is pinned + * in plugin-security's `get-effective-object-permissions.test.ts`. + */ +describe('[#20136] a super-user set\'s own explicit entry is its whole answer', () => { + const SCHEMAS: Record = { + crm_account: { name: 'crm_account' }, + crm_lead: { name: 'crm_lead' }, + sys_position: { name: 'sys_position' }, + }; + const source = { allSchemas: () => Object.values(SCHEMAS), schemaOf: (n: string) => SCHEMAS[n] }; + const SUPER = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true }; + const grants = (entry: unknown) => + (['allowRead', 'allowCreate', 'allowEdit', 'allowDelete', 'allowTransfer', 'allowExport'] as const) + .filter((bit) => objectPermissionGrants(entry as any, bit)); + + it('the walled org admin shape: a read-only entry in the super-user set stays read-only', () => { + const map: any = buildEffectiveObjectPermissions( + [{ objects: { '*': SUPER, sys_position: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false } } }], + source, + ); + expect(grants(map.sys_position)).toEqual(['allowRead']); + // The objects the set does NOT name still take its wildcard, every bit it grants. + expect(grants(map.crm_account)).toEqual(['allowRead', 'allowCreate', 'allowEdit', 'allowDelete', 'allowTransfer']); + }); + + it('an EMPTY entry `{}` in the super-user set grants nothing — read included', () => { + const map: any = buildEffectiveObjectPermissions([{ objects: { '*': SUPER, crm_account: {} } }], source); + expect(grants(map.crm_account)).toEqual([]); + }); + + it('a super-read wildcard over its own export-only entry: no read, so no export, and `apiOperations` withholds it', () => { + const map: any = buildEffectiveObjectPermissions( + [{ objects: { '*': { viewAllRecords: true }, crm_account: { allowExport: true } } }], + source, + ); + expect(grants(map.crm_account)).toEqual([]); + expect(map.crm_account.apiOperations).not.toContain('export'); + }); + + it('`modifyAllRecords` alone grants no create — on a seeded entry, and on one another set names', () => { + const map: any = buildEffectiveObjectPermissions( + [ + { objects: { '*': { modifyAllRecords: true } } }, + { objects: { crm_lead: { allowRead: true } } }, + ], + source, + ); + for (const name of Object.keys(SCHEMAS)) { + expect(grants(map[name]), name).toEqual(['allowRead', 'allowEdit', 'allowDelete', 'allowTransfer']); + } + }); + + it('ANOTHER set\'s super-user wildcard still widens that entry — create only where its own wildcard grants it', () => { + const map: any = buildEffectiveObjectPermissions( + [ + { objects: { '*': SUPER, crm_account: { allowRead: true } } }, + { objects: { '*': { modifyAllRecords: true } } }, + ], + source, + ); + // The second set does not name `crm_account`: its write bypass reaches it, and grants no create. + expect(grants(map.crm_account)).toEqual(['allowRead', 'allowEdit', 'allowDelete', 'allowTransfer']); + // Where the first set's wildcard reaches, its own `allowCreate` does. + expect(grants(map.crm_lead)).toContain('allowCreate'); + }); +}); + /** * [#20135] `apiOperations` is what the REST door SERVES this subject — the * door's own two questions, asked per entry: diff --git a/packages/plugins/plugin-hono-server/src/current-user-endpoints-effective-objects.test.ts b/packages/plugins/plugin-hono-server/src/current-user-endpoints-effective-objects.test.ts index ab113b718ac..801dce58be0 100644 --- a/packages/plugins/plugin-hono-server/src/current-user-endpoints-effective-objects.test.ts +++ b/packages/plugins/plugin-hono-server/src/current-user-endpoints-effective-objects.test.ts @@ -23,17 +23,19 @@ import { registerCurrentUserEndpoints } from './current-user-endpoints'; const ME_PERMISSIONS = '/api/v1/auth/me/permissions'; const USER = 'usr_admin'; -/** The sets the resolver hands back — a super-user wildcard beside an explicit deny, and a plain grant. */ +/** + * The sets the resolver hands back — a super-user wildcard, and a second set whose explicit denies it + * folds over. [#20136] The denies sit in the OTHER set on purpose: a super-user set's own explicit + * entry is that set's whole answer for the object, so the fold never reaches it. + */ +const SUPER_WILDCARD = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true }; const RESOLVED = [ + { name: 'ops_admin', objects: { '*': SUPER_WILDCARD }, fields: {} }, { - name: 'ops_admin', - objects: { - '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true }, - sys_member: { allowRead: true, allowEdit: false }, - }, + name: 'sales', + objects: { deal: { allowRead: true, allowEdit: false }, sys_member: { allowRead: true, allowEdit: false } }, fields: {}, }, - { name: 'sales', objects: { deal: { allowRead: true, allowEdit: false } }, fields: {} }, ]; /** @@ -146,6 +148,26 @@ describe('[#18783] /auth/me/permissions `objects` is the one effective-map funct expect(body.objects.report.apiOperations).toContain('export'); }); + it('[#20136] a super-user set\'s OWN narrower entry is served as that set\'s answer — the same bytes', async () => { + // The walled org admin's shape: the set carrying the super-user `'*'` names `report` read-only itself. + const walled = [ + { name: 'org_admin', objects: { '*': SUPER_WILDCARD, report: { allowRead: true, allowEdit: false } }, fields: {} }, + ]; + const body: any = await (await mount(walled).request(`http://localhost${ME_PERMISSIONS}`)).json(); + const expected = buildEffectiveObjectPermissions(walled, { + allSchemas: () => ql.registry.getAllObjects(), + schemaOf: (name) => ql.getSchema(name), + }); + expect(JSON.stringify(body.objects)).toBe(JSON.stringify(expected)); + // The server answers that set with its explicit entry, so no write is granted on it… + expect(body.objects.report).toMatchObject({ allowRead: true, allowEdit: false }); + expect(body.objects.report.allowCreate).not.toBe(true); + expect(body.objects.report.allowDelete).not.toBe(true); + expect(body.objects.report.allowTransfer).not.toBe(true); + // …while an object it does not name takes its wildcard, every bit the wildcard grants. + expect(body.objects.deal).toMatchObject({ allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, allowTransfer: true }); + }); + it('keeps the rest of the envelope on its own merges', async () => { const body: any = await (await mount().request(`http://localhost${ME_PERMISSIONS}`)).json(); expect(body.permissionSets).toEqual(['ops_admin', 'sales']); diff --git a/packages/plugins/plugin-hono-server/src/fold-wildcard-superuser.test.ts b/packages/plugins/plugin-hono-server/src/fold-wildcard-superuser.test.ts index 2a135536590..b54633409a5 100644 --- a/packages/plugins/plugin-hono-server/src/fold-wildcard-superuser.test.ts +++ b/packages/plugins/plugin-hono-server/src/fold-wildcard-superuser.test.ts @@ -10,6 +10,13 @@ import { foldWildcardSuperUser, clampManagedObjectWrites, type ManagedSchemaLike * mirror the server's actual enforcement, which grants writes via a `'*'` * modifyAll super-user bypass regardless of another set's explicit per-object * deny (most-permissive merge, no deny-wins). + * + * [#20136] These cases pin the standalone released export, body unchanged. It + * is no longer a step of the effective map (`buildEffectiveObjectPermissions` + * in `@objectstack/core`), whose super-user fold reads each set on its own: + * over a merged map this helper cannot tell a set's OWN narrower entry from + * another set's, and it pulls `allowCreate` on `modifyAllRecords`, which the + * server does not grant. */ describe('foldWildcardSuperUser', () => { it('lifts an explicit per-object deny when the wildcard is a modifyAll super-user grant', () => { diff --git a/packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts b/packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts index 0ea90cbfed6..844f14aa909 100644 --- a/packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts +++ b/packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts @@ -309,12 +309,15 @@ describe('[#18783] the engine is handed the same producer', () => { * wildcard's own plain bits, and the `allowExport` shape had no entry at all for * an unrestricted object; each row is now 0 under-granted. * - * The super-user fold is also BROADER than the evaluator in two known places. - * Both belong to this family's over-grant card (#20136), and both are - * pre-registered below cell for cell in `KNOWN_OVER_GRANT` — never absorbed - * into the table's rule: the merged bypass folded into an entry the super-user - * set itself names narrower, and `allowCreate` pulled on `modifyAllRecords` - * alone, which the spec's `objectPermissionGrants` gives no create cell. + * [#20136] …and 0 over-granted. The map used to be BROADER than the evaluator + * wherever a merged-bypass fold ran over it: into an entry the super-user set + * itself names narrower (the walled org admin's read-only RBAC rows, an empty + * `{}` entry, an export-only entry), and `allowCreate` pulled on + * `modifyAllRecords` alone, which the spec's `objectPermissionGrants` gives no + * create cell. Each shape that went wrong is a row of its own below, tagged + * with its enumeration letter, so a regression names its row. There is no + * allowance for a known divergence in either direction: the only cells the + * table forgives are the managed-write clamp's. */ describe('[#20083] parity: can() over the member\'s map answers what checkObjectPermission answers', () => { const REGISTERED: Record = { @@ -361,8 +364,8 @@ describe('[#20083] parity: can() over the member\'s map answers what checkObject 'platform admin who is also a wall-less org admin': [ shipped('admin_full_access'), shipped('organization_admin_no_bypass'), shipped('member_default'), ], - 'walled org admin': [shipped('organization_admin'), shipped('member_default')], - 'a bare modify-all wildcard': [authored('modify_all', { '*': { modifyAllRecords: true } })], + '[#20136 row a] walled org admin': [shipped('organization_admin'), shipped('member_default')], + '[#20136 row c] a bare modify-all wildcard': [authored('modify_all', { '*': { modifyAllRecords: true } })], 'a super-read wildcard carrying plain bits': [ authored('view_all_editor', { '*': { viewAllRecords: true, allowEdit: true, allowTransfer: true } }), ], @@ -381,12 +384,36 @@ describe('[#20083] parity: can() over the member\'s map answers what checkObject authored('reader', { crm_account: { allowRead: true } }), authored('super', { '*': { viewAllRecords: true, allowTransfer: true, allowExport: true } }), ], - 'one set: a super-user wildcard AND a narrower explicit entry': [ + '[#20136] one set: a super-user wildcard AND a narrower explicit entry': [ authored('same_super', { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true }, crm_account: { allowRead: true }, }), ], + // [#20136] The rest of the over-grant enumeration, one row per shape. + '[#20136 row b] walled org admin WITHOUT member_default': [shipped('organization_admin')], + '[#20136 row d] an EMPTY explicit entry inside a super-user set': [ + authored('empty_entry', { + '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true }, + crm_account: {}, + }), + ], + '[#20136 row e] a super-read wildcard over its own export-only entry': [ + authored('export_no_read', { '*': { viewAllRecords: true }, crm_account: { allowExport: true } }), + ], + '[#20136 row f] a super-read wildcard over its own narrower entry': [ + authored('view_all_own', { '*': { viewAllRecords: true }, crm_account: { allowEdit: true } }), + ], + '[#20136 row g] a modify-all-only wildcard beside a set naming the object narrower': [ + authored('named_narrow', { + '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, viewAllRecords: true, modifyAllRecords: true }, + crm_account: { allowRead: true }, + }), + authored('modify_only', { '*': { modifyAllRecords: true } }), + ], + '[#20136 row h] platform admin who is also a walled org admin': [ + shipped('admin_full_access'), shipped('organization_admin'), shipped('member_default'), + ], // [#20135] The export slot's own shapes. 'platform admin beside a plain export-only wildcard': [shipped('admin_full_access'), authored('exporter', { '*': { allowExport: true } })], 'one set: an exporting wildcard AND an explicit entry without the grant': [ @@ -394,37 +421,6 @@ describe('[#20083] parity: can() over the member\'s map answers what checkObject ], }; - /** - * [#20134] KNOWN DIVERGENCE, pre-registered — the super-user fold's two - * over-grants, both owned by #20136 and both left exactly as they were: - * - * - the merged bypass folded into an entry the super-user set ITSELF names - * narrower (`resolveObjectPermission` answers that set with its explicit - * entry): the walled org admin's read-only RBAC rows, and the one-set row; - * - `allowCreate` pulled on `modifyAllRecords` alone. - * - * The assertion below is EXACT, both ways: a cell missing here fails the row, - * and a cell listed here that no longer diverges fails it too. ⛔ FLIP TRIGGER: - * when #20136 lands, these rows go red; delete each entry it empties — never - * widen one to absorb a new cell. - */ - const WRITE_VERBS = ['create', 'delete', 'edit', 'import', 'remove', 'update', 'write'] as const; - const CREATE_VERBS = ['create', 'import'] as const; - const cellsOf = (objects: readonly string[], verbs: readonly string[]) => - objects.flatMap((object) => verbs.map((verb) => `${object}.${verb}`)); - const KNOWN_OVER_GRANT: Record = { - // `organization_admin` names its RBAC rows read-only; its own `'*'` is folded over them. - 'walled org admin': cellsOf([SysPosition.name, SysPermissionSet.name], WRITE_VERBS), - // The same shape, authored: the set's `crm_account` entry is its whole answer there. - 'one set: a super-user wildcard AND a narrower explicit entry': cellsOf(['crm_account'], WRITE_VERBS), - // `modifyAllRecords` has no create cell; the fold pulls `allowCreate` on every unguarded entry - // (the clamp takes it back on a guarded one). - 'a bare modify-all wildcard': cellsOf( - ['crm_account', 'crm_lead', 'crm_secret', 'crm_hidden', SysAttachment.name, SysUserPreference.name, SysPosition.name, SysPermissionSet.name], - CREATE_VERBS, - ), - }; - const OPERATION: Record = { allowRead: 'find', allowCreate: 'insert', allowEdit: 'update', allowDelete: 'delete', allowTransfer: 'transfer', allowExport: 'export', @@ -457,7 +453,6 @@ describe('[#20083] parity: can() over the member\'s map answers what checkObject const permissions = toEvalPermissions(await svc.getEffectiveObjectPermissions!({ userId: USER.id })); const wrong: string[] = []; - const overGranted: string[] = []; let cells = 0; for (const schema of Object.values(REGISTERED)) { const isPrivate = schema.access?.default === 'private'; @@ -470,23 +465,36 @@ describe('[#20083] parity: can() over the member\'s map answers what checkObject if (map === server) continue; // The managed-write clamp may only NARROW, and only on its own verbs. if (clamped && CLAMPED.has(target) && server && !map) continue; - // [#20134] A pre-registered over-grant is collected, then held EXACTLY below. - if (map && (KNOWN_OVER_GRANT[label] ?? []).includes(`${schema.name}.${verb}`)) { - overGranted.push(`${schema.name}.${verb}`); - continue; - } wrong.push(`${schema.name}.${verb}: can()=${map} checkObjectPermission=${server}`); } } expect(cells).toBe(Object.keys(REGISTERED).length * OBJECT_PERMISSION_VERB_NAMES.length); expect(wrong).toEqual([]); - // The flip trigger: the known divergence is exactly what it was registered as. - expect([...overGranted].sort()).toEqual([...(KNOWN_OVER_GRANT[label] ?? [])].sort()); }); } - it('[#20134] every pre-registered over-grant names a row of the table', () => { - for (const label of Object.keys(KNOWN_OVER_GRANT)) expect(SUBJECTS, label).toHaveProperty([label]); + it('[#20136] the reported case, spelled out: the walled org admin may not write its RBAC rows, and can() says so', async () => { + const context = { userId: USER.id }; + for (const label of ['[#20136 row a] walled org admin', '[#20136 row b] walled org admin WITHOUT member_default']) { + const sets = SUBJECTS[label]; + const { svc, plugin } = await locate({ schemas: REGISTERED }); + vi.spyOn(plugin as any, 'resolvePermissionSetsForContext').mockResolvedValue(sets); + const permissions = toEvalPermissions(await svc.getEffectiveObjectPermissions!(context)); + for (const [verb, operation] of [['edit', 'update'], ['create', 'insert'], ['delete', 'delete']] as const) { + expect(evaluator.checkObjectPermission(operation, SysPosition.name, sets), `${label} ${verb}`).toBe(false); + expect(can(permissions, SysPosition.name, verb), `${label} ${verb}`).toBe(false); + } + // …and still reads them, on both sides. + expect(evaluator.checkObjectPermission('find', SysPosition.name, sets)).toBe(true); + expect(can(permissions, SysPosition.name, 'read')).toBe(true); + } + // `sys_user` edit: `member_default` grants it, `organization_admin` alone names it write-denied. + const alone = SUBJECTS['[#20136 row b] walled org admin WITHOUT member_default']; + const { svc, plugin } = await locate({ schemas: REGISTERED }); + vi.spyOn(plugin as any, 'resolvePermissionSetsForContext').mockResolvedValue(alone); + const permissions = toEvalPermissions(await svc.getEffectiveObjectPermissions!(context)); + expect(evaluator.checkObjectPermission('update', SysUser.name, alone)).toBe(false); + expect(can(permissions, SysUser.name, 'edit')).toBe(false); }); it('[#20134] the reported case, spelled out: the platform admin may transfer, and can() says so', async () => { From 6527062a22436a391ac47ce36efcd8b08d4fd2ac Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 03:50:10 +0000 Subject: [PATCH 3/3] chore(changeset): 20136 patch note; one-clause corrections to the pending 18783, 18931 and 20134 notes the fix made false Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- .../18783-server-can-option-visibility.md | 2 +- ...missions-unrestricted-export-annotation.md | 2 +- .../20134-super-user-entries-every-bit.md | 2 +- .changeset/20136-super-user-fold-per-set.md | 20 +++++++++++++++++++ 4 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 .changeset/20136-super-user-fold-per-set.md diff --git a/.changeset/18783-server-can-option-visibility.md b/.changeset/18783-server-can-option-visibility.md index 29a6ba2e551..b2357e1e42d 100644 --- a/.changeset/18783-server-can-option-visibility.md +++ b/.changeset/18783-server-can-option-visibility.md @@ -34,6 +34,6 @@ stage: Field.select({ - If the security service cannot resolve the map, a write that needs it is refused with the resolution's own error — fail closed. It is never read as "no grants". - With no security plugin, or an engine older than the seam, there is no permission data. The gate stays unevaluable and the value is admitted with the same `warn` as before, which names the missing input. The security plugin logs one `warn` at start when the engine lacks the seam. -**Plain-wildcard coverage, closed in this release.** `can()` reads only the per-object entries of the map. Before #20083, `/auth/me/permissions` listed an object for a `'*'` wildcard grant only when that grant carried a super-user bit, so a subject whose access to an object came only from a plain wildcard — for example `organization_admin_no_bypass`, which a deployment without an organization wall grants to organization owners and admins — got `false` from `current_user.can()` for that object, although the data plane admits the write, and was refused on a `can`-gated option. That gap is closed in this same release by #20083 (`.changeset/20083-effective-map-plain-wildcard.md`): `buildEffectiveObjectPermissions` now puts each set's plain `'*'` on the registered public objects that set does not name, so that population's map — and any client that answers `can()` from the same `/auth/me/permissions` map — carries an entry for each object the wildcard covers, with the wildcard's grants, narrowed on a guarded managed object by the same managed-write clamp as every other entry. The map still differs from `PermissionEvaluator.checkObjectPermission` for subjects holding a super-user wildcard: an entry the super-user set itself names narrower can read as granted. Its missing `transfer` is closed in this same release (`.changeset/20134-super-user-entries-every-bit.md`). +**Plain-wildcard coverage, closed in this release.** `can()` reads only the per-object entries of the map. Before #20083, `/auth/me/permissions` listed an object for a `'*'` wildcard grant only when that grant carried a super-user bit, so a subject whose access to an object came only from a plain wildcard — for example `organization_admin_no_bypass`, which a deployment without an organization wall grants to organization owners and admins — got `false` from `current_user.can()` for that object, although the data plane admits the write, and was refused on a `can`-gated option. That gap is closed in this same release by #20083 (`.changeset/20083-effective-map-plain-wildcard.md`): `buildEffectiveObjectPermissions` now puts each set's plain `'*'` on the registered public objects that set does not name, so that population's map — and any client that answers `can()` from the same `/auth/me/permissions` map — carries an entry for each object the wildcard covers, with the wildcard's grants, narrowed on a guarded managed object by the same managed-write clamp as every other entry. The map also differed from `PermissionEvaluator.checkObjectPermission` for subjects holding a super-user wildcard: an entry the super-user set itself names narrower read as granted, which is closed in this same release (`.changeset/20136-super-user-fold-per-set.md`). Its missing `transfer` is closed in this same release (`.changeset/20134-super-user-entries-every-bit.md`). **No spec key, route or config key is added or removed.** diff --git a/.changeset/18931-me-permissions-unrestricted-export-annotation.md b/.changeset/18931-me-permissions-unrestricted-export-annotation.md index d49f56e33e1..99f02f2b7a1 100644 --- a/.changeset/18931-me-permissions-unrestricted-export-annotation.md +++ b/.changeset/18931-me-permissions-unrestricted-export-annotation.md @@ -12,5 +12,5 @@ For that principal an unrestricted object got no entry, so annotate never saw it - **The seed and annotate cannot diverge again.** Since #20134 the seed places an entry for every registered object a super-user wildcard reaches that the merge left without one, and `annotateEffectiveApiOperations` alone decides which entries carry `apiOperations`: an object that is unrestricted **and** keeps `export` gets none — unless its `enable.apiEnabled` is `false`, which is annotated `[]` since #20135 because the REST door answers 404 for every verb on it. - **The export axis is the only axis this reaches.** Measured across the `enable` shapes an unrestricted object can carry: withholding `export` subtracts `export` and nothing else, and `mode` stays `unrestricted` either way — which is why the old `mode`-only guard could not tell the two cases apart. The CRUD axis needed no annotation and still gets none. -- **What the response gains**: for such a principal, one entry per unrestricted object, each the full closure minus `export`. Its CRUD bits are folded `true` — the same answer the client already computed by falling back to `'*'`, now stated explicitly rather than inherited. +- **What the response gains**: for such a principal, one entry per unrestricted object, each the full closure minus `export`. Its CRUD bits are folded to what its wildcard grants (all four for the built-in admin sets) — the same answer the client already computed by falling back to `'*'`, now stated explicitly rather than inherited. - **Denial is unchanged.** `enforceExportPermission` → `security.canExport` still answers `403 EXPORT_NOT_PERMITTED`, and no request that was refused is now accepted. This is the affordance half: the channel that is supposed to tell the client now does. diff --git a/.changeset/20134-super-user-entries-every-bit.md b/.changeset/20134-super-user-entries-every-bit.md index d2ee6a58648..ae28819cf4b 100644 --- a/.changeset/20134-super-user-entries-every-bit.md +++ b/.changeset/20134-super-user-entries-every-bit.md @@ -14,4 +14,4 @@ fix(core): an effective-map entry reached through a super-user `'*'` carries eve **What a reader of `/auth/me/permissions` sees.** For a subject holding a super-user wildcard, entries gain `true` bits (`allowTransfer`, and the wildcard's own plain and export bits). Where that subject's wildcards also grant `allowExport`, the map gains an entry for each registered unrestricted object that had none. That entry carries no `apiOperations` — unless the object declares `enable.apiEnabled: false`, which is annotated `[]` since #20135 — so for every other such object the operation channel says what it said before and a client's default-allow path is unchanged. Nothing is removed and no `true` bit turns `false`. The response is byte-identical for a subject holding no super-user wildcard: `member_default` alone, and `viewer_readonly` or `organization_admin_no_bypass` beside it. The response shape, its keys and the route are unchanged. On the write path, a `can(object, 'transfer')`-gated option or default is now admitted for these subjects wherever the server grants `transfer`. -**Still broader than the server, unchanged here.** The fold still folds the merged super-user bits into an entry the super-user set itself names narrower. It also still pulls `allowCreate` on `modifyAllRecords` alone, which the server does not grant. Both over-grants are left exactly as they were. +**Still broader than the server, unchanged here.** The fold still folds the merged super-user bits into an entry the super-user set itself names narrower. It also still pulls `allowCreate` on `modifyAllRecords` alone, which the server does not grant. Both over-grants are left exactly as they were by this change, and both are closed in this same release (`.changeset/20136-super-user-fold-per-set.md`). diff --git a/.changeset/20136-super-user-fold-per-set.md b/.changeset/20136-super-user-fold-per-set.md new file mode 100644 index 00000000000..cb968bbd050 --- /dev/null +++ b/.changeset/20136-super-user-fold-per-set.md @@ -0,0 +1,20 @@ +--- +'@objectstack/core': patch +--- + +fix(core): the effective object-permission map grants no cell the server refuses for a super-user subject — a super-user set's own narrower entry is that set's answer, and `modifyAllRecords` alone grants no create (#20136) + +`buildEffectiveObjectPermissions` builds the `objects` slot of `GET /auth/me/permissions` (`@objectstack/plugin-hono-server`) and the map `ISecurityService.getEffectiveObjectPermissions` returns (`@objectstack/plugin-security`), which the engine hands to `current_user.can(object, verb)` on the write path. For a subject holding a super-user wildcard — a `'*'` carrying `viewAllRecords` or `modifyAllRecords` — the map granted cells that `PermissionEvaluator.checkObjectPermission` refuses. Before building each set's own contribution, it ran a fold over the MERGED map that put the merged bypass bits on every entry: + +- **Into an entry the super-user set names itself.** The server answers each set with its explicit entry for the object when it has one, so the set's wildcard never reaches that object. The walled `organization_admin` names `sys_position`, `sys_permission_set`, `sys_position_permission_set`, `sys_user_permission_set` and `sys_user_position` read-only, and the identity tables write-denied; the map granted create, edit and delete on the first five anyway, and edit on `sys_organization`. With `member_default` that was 38 cells, and 44 without it (edit on `sys_user` and `sys_api_key` as well). An explicit `{}` entry read as readable and writable. An explicit entry granting `allowExport` without read read as readable and exportable, so `apiOperations` offered `export` where the export door answers `403 EXPORT_NOT_PERMITTED`. +- **`allowCreate` on `modifyAllRecords` alone.** The spec's `objectPermissionGrants` gives the write bypass no create cell, and the server grants none. A `'*': { modifyAllRecords: true }` without `allowCreate` read `create` and `import` as granted on every object the managed-write clamp does not cover. + +On the write path this failed OPEN: an option gated on `current_user.can('sys_position', 'edit')` was admitted for the walled `organization_admin`, whom the server refuses that edit. + +Clause-②: no + +**What changes.** The merged fold is no longer a step of `buildEffectiveObjectPermissions`. The super-user fold is the per-set one alone: each set's super-user `'*'` puts on every entry that set does not name exactly the bits the spec's `objectPermissionGrants` says that wildcard grants. A set that names an object keeps its explicit entry as its whole answer for that object, and another set's super-user wildcard still widens that entry bit by bit, as `checkObjectPermission` combines sets. The seed, the plain-wildcard coverage, the managed-write clamp and the `apiOperations` annotation are unchanged. `checkObjectPermission` and every route are unchanged, so no request changes its answer on the server. + +**What a reader of `/auth/me/permissions` sees.** For a subject whose super-user set names an object narrower than its wildcard, or whose only create grant was `modifyAllRecords`, the entry reads what the server enforces: `allowCreate`, `allowEdit`, `allowDelete` or `allowRead` turn from `true` to `false` on those cells, and an entry whose export no longer holds gains an `apiOperations` list without `export`. No entry is added or removed and no bit turns from `false` to `true`. The response is byte-identical for every subject whose super-user sets name no object narrower and grant `allowCreate` wherever they carry `modifyAllRecords` — `admin_full_access` alone or beside `member_default` — and for every subject holding no super-user wildcard. The response shape, its keys and the route are unchanged. On the write path, a `can()`-gated option for those cells is now refused and a `can()` default reads `false`, as the server refuses the write they describe. + +**For a caller of the exported helper.** `foldWildcardSuperUser` keeps its name, signature and body, and `@objectstack/plugin-hono-server` still re-exports it. It is no longer what the map is built from: over a merged map it cannot tell a set's own entry from another set's. Build the map with `buildEffectiveObjectPermissions`.