fix(core): lower modifyAllRecords in the managed write clamp where the write guard refuses edit or delete

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

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

@@ -555,6 +555,73 @@ 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<string, any> = {
+    // 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('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, 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 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 });
+  });
+});
+
 // ── [#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.

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

@@ -239,6 +239,18 @@ const GUARDED_WRITE_BUCKETS: ReadonlySet<string> = 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, 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<string, any>,
@@ -249,15 +261,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<string, any>,
+  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 +590,7 @@ const EXTERNAL_WRITE_GATED_OPERATIONS: ReadonlySet<ApiOperation> = new Set<ApiOp
  * answers edit, delete and transfer `true` on it alone, so an entry keeping it
  * would still answer `current_user.can(object, 'edit')` `true`. It is lowered
  * to `viewAllRecords`, the read half it implies, so the entry keeps exactly
- * the read it had.
+ * the read it had ({@link lowerModifyAllRecords}, keeping no verb).
  *
  * An object the map does not mention already grants nothing, and is left out.
  */
@@ -563,10 +602,7 @@ function clampExternalReadOnlyWrites(objects: Record<string, any>, 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 });
   }
 }
 

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();