Skip to content

Commit 04b5cbb

Browse files
committed
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 <noreply@anthropic.com>
1 parent 5dd16ba commit 04b5cbb

4 files changed

Lines changed: 42 additions & 16 deletions

File tree

‎.changeset/30440-managed-clamp-modify-all.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22
'@objectstack/core': patch
33
---
44

5-
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
5+
fix(core): effective object permissions on a guarded managed object grant no edit, delete or transfer the write guard refuses, `modifyAllRecords` included
66

77
Clause-②: yes (widening)
88
Metadata-protocol: none
99

10-
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.
10+
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:
1111

12-
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.
12+
- **`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.
13+
- **`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.
1314

1415
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.

‎packages/core/src/security/effective-object-permissions.test.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -591,24 +591,34 @@ describe('[#30440] a guarded managed object grants no verb its write guard refus
591591
expect(can(map.sys_feed_note)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true });
592592
});
593593

594+
it('a plain allowTransfer on an object not opting edit in answers no transfer; opting edit in keeps it', () => {
595+
const map: any = buildEffectiveObjectPermissions(
596+
[{ objects: { sys_automation_run: { allowRead: true, allowTransfer: true }, sys_user: { allowRead: true, allowTransfer: true } } }],
597+
source,
598+
);
599+
expect(objectPermissionGrants(map.sys_automation_run, 'allowTransfer')).toBe(false);
600+
expect(objectPermissionGrants(map.sys_automation_run, 'allowRead')).toBe(true);
601+
expect(objectPermissionGrants(map.sys_user, 'allowTransfer')).toBe(true);
602+
});
603+
594604
it('CONTROL: the same entry on an unguarded object keeps every verb modifyAllRecords grants', () => {
595605
const map: any = buildEffectiveObjectPermissions([{ objects: { crm_lead: { modifyAllRecords: true } } }], source);
596606
expect(map.crm_lead.modifyAllRecords).toBe(true);
597607
expect(can(map.crm_lead)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true });
598608
});
599609

600-
it('CONTROL: the super-user wildcard fold answers as before', () => {
610+
it('CONTROL: the super-user wildcard fold answers as before, transfer on the object opting nothing in aside', () => {
601611
const map: any = buildEffectiveObjectPermissions(
602612
[{ objects: { '*': { allowRead: true, viewAllRecords: true, modifyAllRecords: true } } }],
603613
source,
604614
);
605615
expect(can(map.crm_lead)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true });
606616
expect(can(map.sys_user)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false, allowTransfer: true });
607617
expect(can(map.sys_feed_note)).toEqual({ allowRead: true, allowCreate: false, allowEdit: true, allowDelete: true, allowTransfer: true });
608-
// The fold copies the bits the wildcard grants (transfer among them), never
609-
// modifyAllRecords itself, so the lowering never reaches a folded entry.
610-
expect(can(map.sys_automation_run)).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, allowTransfer: true });
618+
// The fold copies the bits the wildcard grants, never modifyAllRecords itself.
611619
for (const name of Object.keys(SCHEMAS)) expect(map[name], name).not.toHaveProperty('modifyAllRecords');
620+
// The one answer that moves: the folded transfer bit, on the object whose guard refuses the update.
621+
expect(can(map.sys_automation_run)).toEqual({ allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false, allowTransfer: false });
612622
});
613623
});
614624

‎packages/core/src/security/effective-object-permissions.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -240,13 +240,17 @@ const GUARDED_WRITE_BUCKETS: ReadonlySet<string> = new Set(['better-auth', 'engi
240240
* stands (an admin CAN write them via the data API, and the hint must not
241241
* under-report that).
242242
*
243-
* [#30440] `modifyAllRecords` grants edit, delete and transfer on its own (the
244-
* spec's `objectPermissionGrants`), so no `allow*: false` beside it takes any of
245-
* them away. Where the guard refuses `edit` or `delete` it is lowered by
246-
* {@link lowerModifyAllRecords}: to `viewAllRecords`, the read it implies, plus
247-
* the plain bit of each verb the object still opts in — `allowTransfer` with
248-
* `edit`, since an owner change is an update. Where `edit` and `delete` are
249-
* both opted in it grants nothing the guard refuses, and stays.
243+
* [#30440] Every verb the guard refuses is refused whatever grants it:
244+
* - `allowTransfer` follows `edit` — an owner change is an update, which the
245+
* guard refuses unless `edit` is opted in; the super-user wildcard fold
246+
* copies the transfer bit onto every entry it covers;
247+
* - `modifyAllRecords` grants edit, delete and transfer on its own (the
248+
* spec's `objectPermissionGrants`), so no `allow*: false` beside it takes
249+
* any of them away. Where the guard refuses `edit` or `delete` it is lowered
250+
* by {@link lowerModifyAllRecords}: to `viewAllRecords`, the read it
251+
* implies, plus the plain bit of each verb the object still opts in. Where
252+
* `edit` and `delete` are both opted in it grants nothing the guard
253+
* refuses, and stays.
250254
*/
251255
export function clampManagedObjectWrites(
252256
objects: Record<string, any>,
@@ -265,6 +269,7 @@ export function clampManagedObjectWrites(
265269
// would clamp away a legitimate `{ enabled: true, visibleWhen: … }`.
266270
if (!isWriteOptedIn(ua.create)) acc.allowCreate = false;
267271
if (!del) acc.allowDelete = false;
272+
if (!edit) acc.allowTransfer = false;
268273
lowerModifyAllRecords(acc, { allowEdit: edit, allowDelete: del, allowTransfer: edit });
269274
}
270275
}

‎packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,8 @@ describe('[#18783] the engine is handed the same producer', () => {
354354
* the create / edit / delete verbs may read NARROWER than the evaluator: that
355355
* is the managed-write clamp, the engine write guard the permission sets do not
356356
* model. There the pin holds the refuse direction only — the map never grants
357-
* what the evaluator refuses.
357+
* what the evaluator refuses. [#30440] `transfer` is one of them: an owner
358+
* change is an update, which the guard refuses unless the object opts `edit` in.
358359
*
359360
* [#20134] Subjects whose sets carry a SUPER-USER wildcard are held by the same
360361
* 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
473474
shipped('admin_full_access'), shipped('organization_admin'), shipped('member_default'),
474475
],
475476
// [#20135] The export slot's own shapes.
477+
// [#30440] An explicit modify-all entry on guarded managed objects: one opting
478+
// `edit` in (sys_user), one opting nothing in (sys_member), one private (sys_secret).
479+
'[#30440] explicit modifyAllRecords entries on guarded managed objects': [
480+
authored('managed_modify_all', {
481+
[SysUser.name]: { modifyAllRecords: true },
482+
[SysMember.name]: { modifyAllRecords: true },
483+
[SysSecret.name]: { modifyAllRecords: true },
484+
}),
485+
],
476486
'platform admin beside a plain export-only wildcard': [shipped('admin_full_access'), authored('exporter', { '*': { allowExport: true } })],
477487
'one set: an exporting wildcard AND an explicit entry without the grant': [
478488
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
484494
allowTransfer: 'transfer', allowExport: 'export',
485495
};
486496
const GUARDED = new Set(['better-auth', 'engine-owned', 'append-only']);
487-
const CLAMPED = new Set(['allowCreate', 'allowEdit', 'allowDelete']);
497+
const CLAMPED = new Set(['allowCreate', 'allowEdit', 'allowDelete', 'allowTransfer']);
488498
const USER = { id: 'u_parity', positions: [] as string[] };
489499
const evaluator = new PermissionEvaluator();
490500

0 commit comments

Comments
 (0)