From 47fe789a85cba632b4ac8e850bf19e3db3a2a802 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 12:22:52 +0000 Subject: [PATCH 1/3] fix(core): the managed write clamp lowers modifyAllRecords and clears transfer where the guard refuses the update clampManagedObjectWrites cleared allowEdit/allowCreate/allowDelete on a guarded managed object that does not opt the verb in, but left modifyAllRecords (which grants edit, delete and transfer on its own) and allowTransfer (an owner change is an update). It now lowers modifyAllRecords to viewAllRecords plus the plain bit of each verb the object still opts in, through one helper shared with the external write clamp, and clears allowTransfer unless edit is opted in. Claude-Session: https://claude.ai/code/session_01Utp6CZUqByp4kJMhCQDWZ2 Co-authored-by: Claude --- .changeset/30440-managed-clamp-modify-all.md | 15 +++++ .../effective-object-permissions.test.ts | 63 +++++++++++++++++++ .../security/effective-object-permissions.ts | 48 +++++++++++--- 3 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 .changeset/30440-managed-clamp-modify-all.md diff --git a/.changeset/30440-managed-clamp-modify-all.md b/.changeset/30440-managed-clamp-modify-all.md new file mode 100644 index 00000000000..6186e9bd1b0 --- /dev/null +++ b/.changeset/30440-managed-clamp-modify-all.md @@ -0,0 +1,15 @@ +--- +'@objectstack/core': patch +--- + +fix(core): effective object permissions on a guarded managed object grant no edit, delete or transfer the write guard refuses, `modifyAllRecords` included + +Clause-②: yes (widening) +Metadata-protocol: none + +The managed write clamp behind `/auth/me/permissions` and `ISecurityService.getEffectiveObjectPermissions` cleared `allowEdit`, `allowCreate` and `allowDelete` on an object in a guarded `managedBy` bucket (`better-auth`, `engine-owned`, `append-only`) that does not opt the verb in through `userActions`, but left two grants the write guard refuses: + +- **`modifyAllRecords`**, which grants edit, delete and transfer on its own. A permission set naming such an object with `{ modifyAllRecords: true }` made `current_user.can(object, 'edit' | 'delete' | 'transfer')` answer `true` while every one of those writes was refused. It is now lowered to `viewAllRecords` (the read it implies, so read is unchanged), and each verb the object still opts in is kept as its plain bit: an object opting `edit` in keeps edit and transfer and loses delete. An object opting both `edit` and `delete` in keeps `modifyAllRecords`. +- **`allowTransfer`**, which now follows `edit`: an owner change is an update, refused unless `edit` is opted in. This also corrects a super-user wildcard admin, who was answered `can(object, 'transfer')` `true` on every guarded object. + +No write changes: the guard refused each of these before and still does. Only the served capability answer, and the console buttons that read it, now match it. diff --git a/packages/core/src/security/effective-object-permissions.test.ts b/packages/core/src/security/effective-object-permissions.test.ts index a83f1b2fe98..46b1fb882dc 100644 --- a/packages/core/src/security/effective-object-permissions.test.ts +++ b/packages/core/src/security/effective-object-permissions.test.ts @@ -555,6 +555,69 @@ describe('[#30370] an object whose external write gate is closed grants no write }); }); +describe('[#30440] a guarded managed object grants no verb its write guard refuses, modifyAllRecords included', () => { + const SCHEMAS: Record = { + // engine-owned, nothing opted in: the guard refuses every user-context write. + sys_automation_run: { name: 'sys_automation_run', managedBy: 'engine-owned' }, + // better-auth with only `edit` opted in (the sys_user shape). + sys_user: { name: 'sys_user', managedBy: 'better-auth', userActions: { edit: true } }, + // append-only with both row verbs opted in (object form for delete). + sys_feed_note: { name: 'sys_feed_note', managedBy: 'append-only', userActions: { edit: true, delete: { enabled: true } } }, + // no guard covers it. + crm_lead: { name: 'crm_lead' }, + }; + const source = { allSchemas: () => Object.values(SCHEMAS), schemaOf: (name: string) => SCHEMAS[name] }; + const ROW_VERBS = ['allowEdit', 'allowDelete', 'allowTransfer'] as const; + const can = (entry: any) => Object.fromEntries( + (['allowRead', 'allowCreate', ...ROW_VERBS] as const).map((verb) => [verb, objectPermissionGrants(entry, verb)]), + ); + + it('an explicit { modifyAllRecords: true } on an object opting nothing in answers no edit, delete or transfer, and the same read', () => { + const map: any = buildEffectiveObjectPermissions([{ objects: { sys_automation_run: { modifyAllRecords: true } } }], source); + expect(can(map.sys_automation_run)).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, allowTransfer: false }); + expect(map.sys_automation_run).toMatchObject({ modifyAllRecords: false, viewAllRecords: true }); + }); + + it('an object opting edit in keeps edit and transfer through it, and loses delete', () => { + const map: any = buildEffectiveObjectPermissions([{ objects: { sys_user: { modifyAllRecords: true } } }], source); + expect(can(map.sys_user)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false, allowTransfer: true }); + expect(map.sys_user).toMatchObject({ modifyAllRecords: false, viewAllRecords: true, allowEdit: true, allowTransfer: true }); + }); + + it('an object opting edit and delete in keeps modifyAllRecords as granted', () => { + const map: any = buildEffectiveObjectPermissions([{ objects: { sys_feed_note: { modifyAllRecords: true } } }], source); + expect(map.sys_feed_note.modifyAllRecords).toBe(true); + expect(map.sys_feed_note).not.toHaveProperty('viewAllRecords'); + expect(can(map.sys_feed_note)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); + }); + + it('an explicit allowTransfer on an object not opting edit in answers no transfer', () => { + const map: any = buildEffectiveObjectPermissions([{ objects: { sys_automation_run: { allowRead: true, allowTransfer: true } } }], source); + expect(objectPermissionGrants(map.sys_automation_run, 'allowTransfer')).toBe(false); + expect(objectPermissionGrants(map.sys_automation_run, 'allowRead')).toBe(true); + }); + + it('CONTROL: the same entry on an unguarded object keeps every verb modifyAllRecords grants', () => { + const map: any = buildEffectiveObjectPermissions([{ objects: { crm_lead: { modifyAllRecords: true } } }], source); + expect(map.crm_lead.modifyAllRecords).toBe(true); + expect(can(map.crm_lead)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); + }); + + it('CONTROL: the super-user wildcard fold answers as before, transfer on the object opting nothing in aside', () => { + const map: any = buildEffectiveObjectPermissions( + [{ objects: { '*': { allowRead: true, viewAllRecords: true, modifyAllRecords: true } } }], + source, + ); + expect(can(map.crm_lead)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); + expect(can(map.sys_user)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false, allowTransfer: true }); + expect(can(map.sys_feed_note)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); + // The fold copies the bits the wildcard grants, never modifyAllRecords itself. + for (const name of Object.keys(SCHEMAS)) expect(map[name], name).not.toHaveProperty('modifyAllRecords'); + // The one answer that moves: the guard refuses the update a transfer is. + expect(can(map.sys_automation_run)).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, allowTransfer: false }); + }); +}); + // ── [#25866 / ruling of record 6408558941] A field permission entry's row // condition (`when`): the ONE vote per verdict, and the row-blind merge behind // `/auth/me/permissions`' `fields` slot. diff --git a/packages/core/src/security/effective-object-permissions.ts b/packages/core/src/security/effective-object-permissions.ts index 0c7fdba6e4e..27c0c88256d 100644 --- a/packages/core/src/security/effective-object-permissions.ts +++ b/packages/core/src/security/effective-object-permissions.ts @@ -239,6 +239,16 @@ const GUARDED_WRITE_BUCKETS: ReadonlySet = new Set(['better-auth', 'engi * objects are NOT clamped — no guard covers them, so their permission-set result * stands (an admin CAN write them via the data API, and the hint must not * under-report that). + * + * [#30440] Every verb the guard refuses is refused whatever grants it: + * - `allowTransfer` follows `edit` — an owner change is an update, and the + * guard refuses the update unless `edit` is opted in; + * - `modifyAllRecords` grants edit, delete and transfer on its own (the + * spec's `objectPermissionGrants`), so where the guard refuses one of them + * it is lowered by {@link lowerModifyAllRecords}: to `viewAllRecords`, the + * read it implies, plus the plain bit of each verb the object still opts in. + * Where `edit` and `delete` are both opted in it grants nothing the guard + * refuses, and stays. */ export function clampManagedObjectWrites( objects: Record, @@ -249,15 +259,42 @@ export function clampManagedObjectWrites( const schema = schemaOf(obj); if (!schema?.managedBy || !GUARDED_WRITE_BUCKETS.has(schema.managedBy)) continue; const ua = schema.userActions ?? {}; - if (!isWriteOptedIn(ua.edit)) acc.allowEdit = false; + const edit = isWriteOptedIn(ua.edit); + const del = isWriteOptedIn(ua.delete); + if (!edit) acc.allowEdit = false; // `create` reads through the same opt-in helper as edit/delete since // #7692 widened it to the object form — a bare `ua.create !== true` // would clamp away a legitimate `{ enabled: true, visibleWhen: … }`. if (!isWriteOptedIn(ua.create)) acc.allowCreate = false; - if (!isWriteOptedIn(ua.delete)) acc.allowDelete = false; + if (!del) acc.allowDelete = false; + if (!edit) acc.allowTransfer = false; + lowerModifyAllRecords(acc, { allowEdit: edit, allowDelete: del, allowTransfer: edit }); } } +/** + * Lower an entry's `modifyAllRecords` where a server-side layer refuses a verb + * it grants. The spec's `objectPermissionGrants` answers edit, delete and + * transfer `true` on `modifyAllRecords` alone, so no `allow*: false` beside it + * takes any of them away: the bit itself has to go. It becomes + * `viewAllRecords` — the read it implies, so the entry keeps exactly the read + * it had — and each verb `kept` names is carried over as its plain bit, so a + * verb the layer still admits is not lost with it. An entry the layer admits + * every one of those verbs on is left as it is. + */ +function lowerModifyAllRecords( + acc: Record, + kept: { allowEdit: boolean; allowDelete: boolean; allowTransfer: boolean }, +): void { + if (acc.modifyAllRecords !== true) return; + if (kept.allowEdit && kept.allowDelete && kept.allowTransfer) return; + acc.modifyAllRecords = false; + acc.viewAllRecords = true; + if (kept.allowEdit) acc.allowEdit = true; + if (kept.allowDelete) acc.allowDelete = true; + if (kept.allowTransfer) acc.allowTransfer = true; +} + /** The API-exposure-relevant slice of a registered object schema. */ export interface ApiExposureSchemaLike { name?: string; @@ -551,7 +588,7 @@ const EXTERNAL_WRITE_GATED_OPERATIONS: ReadonlySet = new Set, forbidden: Re acc.allowEdit = false; acc.allowDelete = false; acc.allowTransfer = false; - if (acc.modifyAllRecords === true) { - acc.modifyAllRecords = false; - acc.viewAllRecords = true; - } + lowerModifyAllRecords(acc, { allowEdit: false, allowDelete: false, allowTransfer: false }); } } From 5dd16ba3c6c1e982dedaa13681169950ee838c08 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 12:29:25 +0000 Subject: [PATCH 2/3] fix(core): keep the managed clamp to modifyAllRecords; leave a plain allowTransfer as it was Clearing a plain allowTransfer on every guarded object also moved the super-user wildcard admin's transfer answer, which the plugin-security parity table (map vs evaluator) does not forgive. That half goes back to the seat; this change keeps to the card: modifyAllRecords is lowered where the guard refuses edit or delete. Claude-Session: https://claude.ai/code/session_01Utp6CZUqByp4kJMhCQDWZ2 Co-authored-by: Claude --- .changeset/30440-managed-clamp-modify-all.md | 7 +++---- .../effective-object-permissions.test.ts | 14 ++++---------- .../security/effective-object-permissions.ts | 17 +++++++---------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/.changeset/30440-managed-clamp-modify-all.md b/.changeset/30440-managed-clamp-modify-all.md index 6186e9bd1b0..62014ec01e4 100644 --- a/.changeset/30440-managed-clamp-modify-all.md +++ b/.changeset/30440-managed-clamp-modify-all.md @@ -2,14 +2,13 @@ '@objectstack/core': patch --- -fix(core): effective object permissions on a guarded managed object grant no edit, delete or transfer the write guard refuses, `modifyAllRecords` included +fix(core): effective object permissions on a guarded managed object no longer grant edit, delete or transfer through `modifyAllRecords` where the write guard refuses them Clause-②: yes (widening) Metadata-protocol: none -The managed write clamp behind `/auth/me/permissions` and `ISecurityService.getEffectiveObjectPermissions` cleared `allowEdit`, `allowCreate` and `allowDelete` on an object in a guarded `managedBy` bucket (`better-auth`, `engine-owned`, `append-only`) that does not opt the verb in through `userActions`, but left two grants the write guard refuses: +The managed write clamp behind `/auth/me/permissions` and `ISecurityService.getEffectiveObjectPermissions` cleared `allowEdit`, `allowCreate` and `allowDelete` on an object in a guarded `managedBy` bucket (`better-auth`, `engine-owned`, `append-only`) that does not opt the verb in through `userActions`, but left `modifyAllRecords`, which grants edit, delete and transfer on its own. A permission set naming such an object with `{ modifyAllRecords: true }` therefore made `current_user.can(object, 'edit' | 'delete' | 'transfer')` answer `true` while the write guard refused each of those writes. -- **`modifyAllRecords`**, which grants edit, delete and transfer on its own. A permission set naming such an object with `{ modifyAllRecords: true }` made `current_user.can(object, 'edit' | 'delete' | 'transfer')` answer `true` while every one of those writes was refused. It is now lowered to `viewAllRecords` (the read it implies, so read is unchanged), and each verb the object still opts in is kept as its plain bit: an object opting `edit` in keeps edit and transfer and loses delete. An object opting both `edit` and `delete` in keeps `modifyAllRecords`. -- **`allowTransfer`**, which now follows `edit`: an owner change is an update, refused unless `edit` is opted in. This also corrects a super-user wildcard admin, who was answered `can(object, 'transfer')` `true` on every guarded object. +Where the guard refuses `edit` or `delete`, `modifyAllRecords` is now lowered to `viewAllRecords` (the read it implies, so read is unchanged), and each verb the object still opts in is kept as its plain bit: an object opting `edit` in (such as `sys_user`) keeps edit and transfer and loses delete; an object opting nothing in keeps read only. An object opting both `edit` and `delete` in keeps `modifyAllRecords`. The external write clamp lowers it through the same helper. No write changes: the guard refused each of these before and still does. Only the served capability answer, and the console buttons that read it, now match it. diff --git a/packages/core/src/security/effective-object-permissions.test.ts b/packages/core/src/security/effective-object-permissions.test.ts index 46b1fb882dc..079029b38c7 100644 --- a/packages/core/src/security/effective-object-permissions.test.ts +++ b/packages/core/src/security/effective-object-permissions.test.ts @@ -591,19 +591,13 @@ describe('[#30440] a guarded managed object grants no verb its write guard refus expect(can(map.sys_feed_note)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); }); - it('an explicit allowTransfer on an object not opting edit in answers no transfer', () => { - const map: any = buildEffectiveObjectPermissions([{ objects: { sys_automation_run: { allowRead: true, allowTransfer: true } } }], source); - expect(objectPermissionGrants(map.sys_automation_run, 'allowTransfer')).toBe(false); - expect(objectPermissionGrants(map.sys_automation_run, 'allowRead')).toBe(true); - }); - it('CONTROL: the same entry on an unguarded object keeps every verb modifyAllRecords grants', () => { const map: any = buildEffectiveObjectPermissions([{ objects: { crm_lead: { modifyAllRecords: true } } }], source); expect(map.crm_lead.modifyAllRecords).toBe(true); expect(can(map.crm_lead)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); }); - it('CONTROL: the super-user wildcard fold answers as before, transfer on the object opting nothing in aside', () => { + it('CONTROL: the super-user wildcard fold answers as before', () => { const map: any = buildEffectiveObjectPermissions( [{ objects: { '*': { allowRead: true, viewAllRecords: true, modifyAllRecords: true } } }], source, @@ -611,10 +605,10 @@ describe('[#30440] a guarded managed object grants no verb its write guard refus expect(can(map.crm_lead)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); expect(can(map.sys_user)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false, allowTransfer: true }); expect(can(map.sys_feed_note)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); - // The fold copies the bits the wildcard grants, never modifyAllRecords itself. + // The fold copies the bits the wildcard grants (transfer among them), never + // modifyAllRecords itself, so the lowering never reaches a folded entry. + expect(can(map.sys_automation_run)).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, allowTransfer: true }); for (const name of Object.keys(SCHEMAS)) expect(map[name], name).not.toHaveProperty('modifyAllRecords'); - // The one answer that moves: the guard refuses the update a transfer is. - expect(can(map.sys_automation_run)).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, allowTransfer: false }); }); }); diff --git a/packages/core/src/security/effective-object-permissions.ts b/packages/core/src/security/effective-object-permissions.ts index 27c0c88256d..616043bce1a 100644 --- a/packages/core/src/security/effective-object-permissions.ts +++ b/packages/core/src/security/effective-object-permissions.ts @@ -240,15 +240,13 @@ const GUARDED_WRITE_BUCKETS: ReadonlySet = new Set(['better-auth', 'engi * stands (an admin CAN write them via the data API, and the hint must not * under-report that). * - * [#30440] Every verb the guard refuses is refused whatever grants it: - * - `allowTransfer` follows `edit` — an owner change is an update, and the - * guard refuses the update unless `edit` is opted in; - * - `modifyAllRecords` grants edit, delete and transfer on its own (the - * spec's `objectPermissionGrants`), so where the guard refuses one of them - * it is lowered by {@link lowerModifyAllRecords}: to `viewAllRecords`, the - * read it implies, plus the plain bit of each verb the object still opts in. - * Where `edit` and `delete` are both opted in it grants nothing the guard - * refuses, and stays. + * [#30440] `modifyAllRecords` grants edit, delete and transfer on its own (the + * spec's `objectPermissionGrants`), so no `allow*: false` beside it takes any of + * them away. Where the guard refuses `edit` or `delete` it is lowered by + * {@link lowerModifyAllRecords}: to `viewAllRecords`, the read it implies, plus + * the plain bit of each verb the object still opts in — `allowTransfer` with + * `edit`, since an owner change is an update. Where `edit` and `delete` are + * both opted in it grants nothing the guard refuses, and stays. */ export function clampManagedObjectWrites( objects: Record, @@ -267,7 +265,6 @@ export function clampManagedObjectWrites( // would clamp away a legitimate `{ enabled: true, visibleWhen: … }`. if (!isWriteOptedIn(ua.create)) acc.allowCreate = false; if (!del) acc.allowDelete = false; - if (!edit) acc.allowTransfer = false; lowerModifyAllRecords(acc, { allowEdit: edit, allowDelete: del, allowTransfer: edit }); } } From 04b5cbbebd76fae78aeebe51681fe262654477be Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 13:02:07 +0000 Subject: [PATCH 3/3] fix(core): the managed clamp also clears a plain allowTransfer where the guard refuses the update The super-user wildcard fold copies the transfer bit onto every guarded object it covers, so the platform admin was served can(object, 'transfer') true where the guard maps transfer to edit and refuses it. The clamp now clears allowTransfer unless edit is opted in. The plugin-security agreement table forgives transfer as one of the clamp's own verbs, and gains a row with explicit modifyAllRecords entries on guarded objects. Claude-Session: https://claude.ai/code/session_01Utp6CZUqByp4kJMhCQDWZ2 Co-authored-by: Claude --- .changeset/30440-managed-clamp-modify-all.md | 7 ++++--- .../effective-object-permissions.test.ts | 18 ++++++++++++++---- .../security/effective-object-permissions.ts | 19 ++++++++++++------- .../get-effective-object-permissions.test.ts | 14 ++++++++++++-- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/.changeset/30440-managed-clamp-modify-all.md b/.changeset/30440-managed-clamp-modify-all.md index 62014ec01e4..a78d2b5400f 100644 --- a/.changeset/30440-managed-clamp-modify-all.md +++ b/.changeset/30440-managed-clamp-modify-all.md @@ -2,13 +2,14 @@ '@objectstack/core': patch --- -fix(core): effective object permissions on a guarded managed object no longer grant edit, delete or transfer through `modifyAllRecords` where the write guard refuses them +fix(core): effective object permissions on a guarded managed object grant no edit, delete or transfer the write guard refuses, `modifyAllRecords` included Clause-②: yes (widening) Metadata-protocol: none -The managed write clamp behind `/auth/me/permissions` and `ISecurityService.getEffectiveObjectPermissions` cleared `allowEdit`, `allowCreate` and `allowDelete` on an object in a guarded `managedBy` bucket (`better-auth`, `engine-owned`, `append-only`) that does not opt the verb in through `userActions`, but left `modifyAllRecords`, which grants edit, delete and transfer on its own. A permission set naming such an object with `{ modifyAllRecords: true }` therefore made `current_user.can(object, 'edit' | 'delete' | 'transfer')` answer `true` while the write guard refused each of those writes. +The managed write clamp behind `/auth/me/permissions` and `ISecurityService.getEffectiveObjectPermissions` cleared `allowEdit`, `allowCreate` and `allowDelete` on an object in a guarded `managedBy` bucket (`better-auth`, `engine-owned`, `append-only`) that does not opt the verb in through `userActions`, but left two grants the write guard refuses: -Where the guard refuses `edit` or `delete`, `modifyAllRecords` is now lowered to `viewAllRecords` (the read it implies, so read is unchanged), and each verb the object still opts in is kept as its plain bit: an object opting `edit` in (such as `sys_user`) keeps edit and transfer and loses delete; an object opting nothing in keeps read only. An object opting both `edit` and `delete` in keeps `modifyAllRecords`. The external write clamp lowers it through the same helper. +- **`modifyAllRecords`**, which grants edit, delete and transfer on its own. A permission set naming such an object with `{ modifyAllRecords: true }` made `current_user.can(object, 'edit' | 'delete' | 'transfer')` answer `true` while every one of those writes was refused. Where the guard refuses `edit` or `delete`, it is now lowered to `viewAllRecords` (the read it implies, so read is unchanged), and each verb the object still opts in is kept as its plain bit: an object opting `edit` in (such as `sys_user`) keeps edit and transfer and loses delete; an object opting nothing in keeps read only. An object opting both `edit` and `delete` in keeps `modifyAllRecords`. The external write clamp lowers it through the same helper. +- **`allowTransfer`**, which now follows `edit`: an owner change is an update, refused unless `edit` is opted in. This also corrects a super-user wildcard admin, who was answered `can(object, 'transfer')` `true` on every guarded object that does not opt `edit` in. No write changes: the guard refused each of these before and still does. Only the served capability answer, and the console buttons that read it, now match it. diff --git a/packages/core/src/security/effective-object-permissions.test.ts b/packages/core/src/security/effective-object-permissions.test.ts index 079029b38c7..3b9ec5bd108 100644 --- a/packages/core/src/security/effective-object-permissions.test.ts +++ b/packages/core/src/security/effective-object-permissions.test.ts @@ -591,13 +591,23 @@ describe('[#30440] a guarded managed object grants no verb its write guard refus expect(can(map.sys_feed_note)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); }); + it('a plain allowTransfer on an object not opting edit in answers no transfer; opting edit in keeps it', () => { + const map: any = buildEffectiveObjectPermissions( + [{ objects: { sys_automation_run: { allowRead: true, allowTransfer: true }, sys_user: { allowRead: true, allowTransfer: true } } }], + source, + ); + expect(objectPermissionGrants(map.sys_automation_run, 'allowTransfer')).toBe(false); + expect(objectPermissionGrants(map.sys_automation_run, 'allowRead')).toBe(true); + expect(objectPermissionGrants(map.sys_user, 'allowTransfer')).toBe(true); + }); + it('CONTROL: the same entry on an unguarded object keeps every verb modifyAllRecords grants', () => { const map: any = buildEffectiveObjectPermissions([{ objects: { crm_lead: { modifyAllRecords: true } } }], source); expect(map.crm_lead.modifyAllRecords).toBe(true); expect(can(map.crm_lead)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); }); - it('CONTROL: the super-user wildcard fold answers as before', () => { + it('CONTROL: the super-user wildcard fold answers as before, transfer on the object opting nothing in aside', () => { const map: any = buildEffectiveObjectPermissions( [{ objects: { '*': { allowRead: true, viewAllRecords: true, modifyAllRecords: true } } }], source, @@ -605,10 +615,10 @@ describe('[#30440] a guarded managed object grants no verb its write guard refus expect(can(map.crm_lead)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); expect(can(map.sys_user)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false, allowTransfer: true }); expect(can(map.sys_feed_note)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true }); - // The fold copies the bits the wildcard grants (transfer among them), never - // modifyAllRecords itself, so the lowering never reaches a folded entry. - expect(can(map.sys_automation_run)).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, allowTransfer: true }); + // The fold copies the bits the wildcard grants, never modifyAllRecords itself. for (const name of Object.keys(SCHEMAS)) expect(map[name], name).not.toHaveProperty('modifyAllRecords'); + // The one answer that moves: the folded transfer bit, on the object whose guard refuses the update. + expect(can(map.sys_automation_run)).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, allowTransfer: false }); }); }); diff --git a/packages/core/src/security/effective-object-permissions.ts b/packages/core/src/security/effective-object-permissions.ts index 616043bce1a..83183d154da 100644 --- a/packages/core/src/security/effective-object-permissions.ts +++ b/packages/core/src/security/effective-object-permissions.ts @@ -240,13 +240,17 @@ const GUARDED_WRITE_BUCKETS: ReadonlySet = new Set(['better-auth', 'engi * stands (an admin CAN write them via the data API, and the hint must not * under-report that). * - * [#30440] `modifyAllRecords` grants edit, delete and transfer on its own (the - * spec's `objectPermissionGrants`), so no `allow*: false` beside it takes any of - * them away. Where the guard refuses `edit` or `delete` it is lowered by - * {@link lowerModifyAllRecords}: to `viewAllRecords`, the read it implies, plus - * the plain bit of each verb the object still opts in — `allowTransfer` with - * `edit`, since an owner change is an update. Where `edit` and `delete` are - * both opted in it grants nothing the guard refuses, and stays. + * [#30440] Every verb the guard refuses is refused whatever grants it: + * - `allowTransfer` follows `edit` — an owner change is an update, which the + * guard refuses unless `edit` is opted in; the super-user wildcard fold + * copies the transfer bit onto every entry it covers; + * - `modifyAllRecords` grants edit, delete and transfer on its own (the + * spec's `objectPermissionGrants`), so no `allow*: false` beside it takes + * any of them away. Where the guard refuses `edit` or `delete` it is lowered + * by {@link lowerModifyAllRecords}: to `viewAllRecords`, the read it + * implies, plus the plain bit of each verb the object still opts in. Where + * `edit` and `delete` are both opted in it grants nothing the guard + * refuses, and stays. */ export function clampManagedObjectWrites( objects: Record, @@ -265,6 +269,7 @@ export function clampManagedObjectWrites( // would clamp away a legitimate `{ enabled: true, visibleWhen: … }`. if (!isWriteOptedIn(ua.create)) acc.allowCreate = false; if (!del) acc.allowDelete = false; + if (!edit) acc.allowTransfer = false; lowerModifyAllRecords(acc, { allowEdit: edit, allowDelete: del, allowTransfer: edit }); } } 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 14bffb0cc3a..6811596d655 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 @@ -354,7 +354,8 @@ describe('[#18783] the engine is handed the same producer', () => { * the create / edit / delete verbs may read NARROWER than the evaluator: that * is the managed-write clamp, the engine write guard the permission sets do not * model. There the pin holds the refuse direction only — the map never grants - * what the evaluator refuses. + * what the evaluator refuses. [#30440] `transfer` is one of them: an owner + * change is an update, which the guard refuses unless the object opts `edit` in. * * [#20134] Subjects whose sets carry a SUPER-USER wildcard are held by the same * table, one row per shape: the shipped platform admin and walled org admin, a @@ -473,6 +474,15 @@ describe('[#20083] parity: can() over the member\'s map answers what checkObject shipped('admin_full_access'), shipped('organization_admin'), shipped('member_default'), ], // [#20135] The export slot's own shapes. + // [#30440] An explicit modify-all entry on guarded managed objects: one opting + // `edit` in (sys_user), one opting nothing in (sys_member), one private (sys_secret). + '[#30440] explicit modifyAllRecords entries on guarded managed objects': [ + authored('managed_modify_all', { + [SysUser.name]: { modifyAllRecords: true }, + [SysMember.name]: { modifyAllRecords: true }, + [SysSecret.name]: { modifyAllRecords: true }, + }), + ], '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': [ authored('same_export', { '*': { allowRead: true, allowExport: true }, crm_lead: { allowRead: true } }), @@ -484,7 +494,7 @@ describe('[#20083] parity: can() over the member\'s map answers what checkObject allowTransfer: 'transfer', allowExport: 'export', }; const GUARDED = new Set(['better-auth', 'engine-owned', 'append-only']); - const CLAMPED = new Set(['allowCreate', 'allowEdit', 'allowDelete']); + const CLAMPED = new Set(['allowCreate', 'allowEdit', 'allowDelete', 'allowTransfer']); const USER = { id: 'u_parity', positions: [] as string[] }; const evaluator = new PermissionEvaluator();