fix(plugin-security): an organization administrator's assignment removal is judged on the stored row, and its refusal names only values the caller can read

.changeset/org-admin-removal-caller-read.md

@@ -1,3 +1,3 @@
 ---
 '@objectstack/plugin-security': patch
 ---
@@ -7,8 +7,8 @@
 Clause-②: no
 Metadata-protocol: none
 
-An organization administrator (`organization_admin` or `organization_admin_no_bypass`) removes an assignment row (`sys_user_position` or `sys_user_permission_set`) by id. The delegated-administration gate (ADR-0090 D12) now judges that row as the administrator itself reads it, in its own context.
+An organization administrator (`organization_admin` or `organization_admin_no_bypass`) removes an assignment row (`sys_user_position` or `sys_user_permission_set`) by id. The delegated-administration gate (ADR-0090 D12) now asks the administrator's own read, in its own context, whether that row is there to remove.
 
-- **A row the administrator cannot read** is answered exactly as an id that names no row is: the read door's `404 RECORD_NOT_FOUND` for that object and id.
-- **A row the administrator can read** keeps the answer it had, including the guidance that names the organization boundary.
+- **The removal of a row the administrator cannot read** is answered exactly as the removal of an id that names no row is: the read door's `404 RECORD_NOT_FOUND` for that object and id.
+- **A row the administrator can read** is judged on the stored row. A refusal names the organization boundary, and names the row's organization, position or permission set only when the administrator's own read returns that value as stored.
 - **Unchanged.** An edit in place of a row it reads is refused as before, every removal that was admitted is still admitted, and a tenant-level administrator is judged as before.

.changeset/org-admin-removal-stored-row-verdict.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+fix(plugin-security): an organization administrator's assignment removal is judged on the stored row, and its refusal names only values the caller can read
+
+Clause-②: no
+Metadata-protocol: none
+
+An organization administrator (`organization_admin` or `organization_admin_no_bypass`) removes an assignment row (`sys_user_position` or `sys_user_permission_set`) by id. Once its own read returns the row, the delegated-administration gate (ADR-0090 D12) judges the removal on the stored row, as it judges a delegated administrator's write.
+
+- **The verdict is the stored row's.** A field rule or a masking rule on the row's `position` or `permission_set` does not change it: the removal of a row inside the organization boundary is admitted, and the removal of a row outside it is refused.
+- **A refusal names a value only when the caller can read it.** The row's organization, its position and the sets that position carries, or its permission set are named only when the caller's own read returned that column at its stored value. Otherwise the sentence leaves the value out, for example "the assignment's position is not inside the organization boundary", and `details` leaves it out too.
+- **Unchanged.** A row the caller cannot read, and an id that names no row, still answer the read door's `404 RECORD_NOT_FOUND`. An edit in place is refused as before. Inserts, tenant-level administrators and delegated administrators are judged as before.

packages/plugins/plugin-security/src/delegated-admin-gate.test.ts

@@ -3,7 +3,7 @@
 
 import { describe, it, expect, beforeEach } from 'vitest';
 import { mapDataError } from '@objectstack/types';
-import { DelegatedAdminGate } from './delegated-admin-gate';
+import { DelegatedAdminGate, isUnreadableAddressedRowRefusal } from './delegated-admin-gate';
 import { isPermissionDeniedError } from './errors.js';
 import { bindCatalogFromTables, bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js';
 
@@ -1389,6 +1389,98 @@ describe('DelegatedAdminGate — organization administrators (ADR-0090 D12 as am
   });
 });
 
+/**
+ * The organization harness's caller reads served through a field rule on
+ * `column`: absent from the row (`hidden`) or replaced by a partial mask
+ * (`masked`), as the read door serves them. System reads see the stored row.
+ * Returns the objects the gate read by the system, in order.
+ */
+function serveCallerReadsThrough(o: ReturnType<typeof makeOrganizationHarness>, column: string, how: 'hidden' | 'masked'): string[] {
+  const storedFindOne = o.ql.findOne.bind(o.ql);
+  const systemReads: string[] = [];
+  o.ql.findOne = async (object: string, opts: any) => {
+    const row = await storedFindOne(object, opts);
+    if (opts?.context?.isSystem) {
+      systemReads.push(object);
+      return row;
+    }
+    if (!row || !Object.prototype.hasOwnProperty.call(row, column)) return row;
+    const seen = { ...row };
+    if (how === 'hidden') delete seen[column];
+    else seen[column] = `${String(row[column]).slice(0, 2)}****`;
+    return seen;
+  };
+  return systemReads;
+}
+
+describe("[ADR-0090 D12; ruling of record 6649064874] an organization administrator's removal is judged on the stored row, and its refusal names only values the caller reads", () => {
+  let o: ReturnType<typeof makeOrganizationHarness>;
+  beforeEach(() => { o = makeOrganizationHarness(); });
+
+  const remove = (principal: string, object: string, id: string) =>
+    o.gate.assert({ object, operation: 'delete', options: { where: { id } }, context: o.ctxOf(principal) });
+
+  /** One refusal, serialized whole: its sentence and every detail it carries. */
+  const whole = (err: any) => JSON.stringify({ message: err.message, details: err.details });
+
+  /**
+   * Per column a removal is judged on: the in-boundary rows the stored-row
+   * verdict admits, the rows it refuses, the stored values each refusal
+   * leaves out, and the sentence it says instead.
+   */
+  const CASES = [
+    { column: 'position', object: 'sys_user_position', admitted: ['up_a'],
+      refused: [{ id: 'up_a_mixed', unnamed: ['mixed_pos', 'wild_set', 'in_set'], says: /the assignment's position is not inside the organization boundary/ }] },
+    { column: 'permission_set', object: 'sys_user_permission_set', admitted: ['ups_a'],
+      refused: [{ id: 'ups_a_wild', unnamed: ['wild_set'], says: /the grant's permission set is not inside the organization boundary/ }] },
+    { column: 'organization_id', object: 'sys_user_position', admitted: ['up_a'],
+      refused: [
+        { id: 'up_b', unnamed: ['org_b'], says: /the row does not belong to the organization you administer/ },
+        { id: 'up_orgless', unnamed: ['carries no organization'], says: /the row does not belong to the organization you administer/ },
+      ] },
+    { column: 'organization_id', object: 'sys_user_permission_set', admitted: ['ups_a'], refused: [] },
+  ] as const;
+
+  for (const principal of ['org_admin', 'org_admin_nb']) {
+    for (const how of ['hidden', 'masked'] as const) {
+      for (const c of CASES) {
+        it(`${principal}: with ${c.object}.${c.column} ${how} from its reads, an in-boundary row is removed, and a row outside is refused without the stored value`, async () => {
+          serveCallerReadsThrough(o, c.column, how);
+          for (const id of c.admitted) {
+            await expect(remove(principal, c.object, id), id).resolves.toBeUndefined();
+          }
+          for (const r of c.refused) {
+            const err = await deniedOf(() => remove(principal, c.object, r.id));
+            expect(err.message, r.id).toMatch(/organization boundary/);
+            expect(err.message, r.id).toMatch(r.says);
+            for (const value of r.unnamed) expect(whole(err), `${r.id}: '${value}'`).not.toContain(value);
+            expect(whole(err), `${r.id}: not the masked value either`).not.toContain('****');
+          }
+        });
+      }
+    }
+
+    it(`${principal}: a column it does read keeps the stored value in the guidance — the position and its set, beside an organization it does not read`, async () => {
+      serveCallerReadsThrough(o, 'organization_id', 'hidden');
+      const err = await deniedOf(() => remove(principal, 'sys_user_position', 'up_a_mixed'));
+      expect(err.message).toMatch(/position 'mixed_pos' carries permission set 'wild_set'.*condition \(a\)/);
+      expect(err.details).toMatchObject({ position: 'mixed_pos', permissionSet: 'wild_set' });
+    });
+
+    it(`${principal}: a row its own read does not return, and an id naming no row, are refused as no row — with no read of the stored row`, async () => {
+      const systemReads = serveCallerReadsThrough(o, 'position', 'hidden');
+      const servedFindOne = o.ql.findOne;
+      o.ql.findOne = async (object: string, opts: any) =>
+        (!opts?.context?.isSystem && opts?.where?.id === 'up_a') ? null : servedFindOne(object, opts);
+      for (const id of ['up_a', 'zz_no_such_row']) {
+        const err = await deniedOf(() => remove(principal, 'sys_user_position', id));
+        expect(isUnreadableAddressedRowRefusal(err), id).toBe(true);
+      }
+      expect(systemReads.filter((object) => object === 'sys_user_position'), 'no system read of the addressed row').toEqual([]);
+    });
+  }
+});
+
 // ---------------------------------------------------------------------------
 // [ADR-0090 D12, amended 2026-10-17; #24757] The delegated walk judges the
 // listed sets, never a set that only shares the position's name

packages/plugins/plugin-security/src/delegated-admin-gate.ts

@@ -74,7 +74,7 @@ import {
   rowOrganizationId,
   seedCtx as organizationScopedCtx,
 } from './organization-scope.js';
-import { grantSetNameOf } from './grant-permission-set-name.js';
+import { GRANT_SET_NAME_FIELD, grantSetNameOf } from './grant-permission-set-name.js';
 import { isReservedIdentityName } from './objects/reserved-identity-names.js';
 
 const SYSTEM_CTX = { isSystem: true } as const;
@@ -424,10 +424,11 @@ export function isUnreadableAddressedRowRefusal(refusal: unknown): boolean {
 }
 
 /**
- * [ADR-0090 D12] A row a delegated write targets, with what the caller may be
- * told about it: `prev` the stored row a by-id write addresses, `next` the row
- * an insert or an update writes, and the two column sets a refusal's guidance
- * reads ({@link mayName}).
+ * [ADR-0090 D12] A row an administrator's write targets — the delegated walk's
+ * and the organization branch's alike — with what the caller may be told about
+ * it: `prev` the stored row a by-id write addresses, `next` the row an insert
+ * or an update writes, and the two column sets a refusal's guidance reads
+ * ({@link mayName}).
  */
 interface DelegatedTarget {
   next: any | null;
@@ -445,7 +446,7 @@ interface DelegatedTarget {
 }
 
 /**
- * [ruling of record 5985885287, sub-item B] The columns of the stored row
+ * [ruling of record 6649064874] The columns of the stored row
  * `prev` that the caller's own read `seen` returned at their stored value.
  *
  * The read is the read door's answer to this caller, field rules and masking
@@ -463,7 +464,7 @@ function columnsReadAsStored(seen: any | null, prev: any | null): Set<string> {
 }
 
 /**
- * [ruling of record 5985885287, sub-item B] May a refusal of `t` name the
+ * [ruling of record 6649064874] May a refusal of `t` name the
  * value of `column` on its `side`?
  *
  * Only a value the caller already has: one it wrote (an insert's row, a
@@ -847,13 +848,18 @@ export class DelegatedAdminGate {
    * ({@link organizationBoundaryFailure}); for a position, every set it carries
    * ({@link setsCarriedByPosition}). One set outside puts the row outside.
    *
-   * The row a removal targets is judged as the CALLER reads it
+   * Whether a removal has a row to judge is asked of the CALLER's own read
    * ({@link organizationTargets}), so a row the caller cannot read is answered
-   * exactly as an id naming no row is (ruling of record 5985885287). An edit
-   * in place is a verb the right never holds, so it is refused before any row
-   * is looked up, whatever the id names: a row and no row get the same
-   * refusal (the update-by-id refusal order). The not-found is the answer
-   * only for a verb the caller holds, judged on the row.
+   * exactly as an id naming no row is (ruling of record 5985885287). A row the
+   * caller reads is judged on the STORED row, as the delegated branch judges
+   * it (ruling of record 6649064874): the caller's read is served through its
+   * field rules and masking, and a verdict taken on what a rule hid would be
+   * taken on less than the row. A refusal names a value only when the caller
+   * reads it at its stored value or wrote it ({@link mayName}). An edit in
+   * place is a verb the right never holds, so it is refused before any row is
+   * looked up, whatever the id names: a row and no row get the same refusal
+   * (the update-by-id refusal order). The not-found is the answer only for a
+   * verb the caller holds, judged on the row.
    */
   private async organizationAdminFailure(
     opCtx: any,
@@ -877,13 +883,20 @@ export class DelegatedAdminGate {
     }
 
     for (const t of await this.organizationTargets(opCtx)) {
+      const side = t.next ? 'next' : 'prev';
       const row = t.next ?? t.prev;
       // No row answers an id naming none and a row the caller cannot read
       // alike: one refusal, and never a fall-through to the engine.
       if (!row) return { reason: 'the assignment row to remove could not be read', meta: {}, unreadable: op === 'delete' };
 
+      // [ruling of record 6649064874] The verdict is the stored row's; each
+      // value below is named only when the caller wrote or reads it
+      // ({@link mayName}).
       const rowOrg = rowOrganizationId(row);
       if (t.next ? rowOrg !== null && rowOrg !== organizationId : rowOrg !== organizationId) {
+        if (!mayName(t, side, 'organization_id')) {
+          return { reason: 'the row does not belong to the organization you administer', meta: {} };
+        }
         return {
           reason: rowOrg
             ? `the row belongs to organization '${rowOrg}', not the one you administer`
@@ -902,13 +915,22 @@ export class DelegatedAdminGate {
         }
         const positionName = String(row.position ?? '');
         const failure = await this.positionBoundaryFailure(positionName);
-        if (failure) return { reason: failure.reason, meta: { position: positionName, permissionSet: failure.permissionSet } };
+        if (failure) {
+          // A set is named only beside the position it is read from.
+          return mayName(t, side, 'position')
+            ? { reason: failure.reason, meta: { position: positionName, permissionSet: failure.permissionSet } }
+            : { reason: "the assignment's position is not inside the organization boundary", meta: {} };
+        }
       } else {
         const setName = grantSetNameOf(row) ?? '';
         const reason = setName
           ? await this.organizationBoundaryFailure(setName)
           : 'the grant names no permission set';
-        if (reason) return { reason, meta: { permissionSet: setName } };
+        if (reason) {
+          return mayName(t, side, GRANT_SET_NAME_FIELD)
+            ? { reason, meta: { permissionSet: setName } }
+            : { reason: "the grant's permission set is not inside the organization boundary", meta: {} };
+        }
       }
     }
 
@@ -923,21 +945,30 @@ export class DelegatedAdminGate {
 
   /**
    * [ADR-0090 D12; ruling of record 5985885287] The rows an organization
-   * administrator's insert or removal targets: an insert's payload rows, or the
-   * one row a removal addresses, as the CALLER reads it.
+   * administrator's insert or removal targets, with what the caller may be
+   * told of each ({@link DelegatedTarget}): an insert's payload rows, every
+   * value the caller's own; or the one row a removal addresses.
    *
-   * The pre-image is the caller's own by-id read through the engine — the read
-   * door's answer to this caller, every read visibility and wall included —
-   * never a system read: a row the caller cannot read answers what an id naming
-   * no row answers. A row it reads is judged on what it read, so its guidance
-   * stays. A read refusal or a store fault propagates as raised: an outage is
-   * neither an answer nor a refusal, and the by-id write door answers a caller
-   * the read door refuses with that refusal.
+   * Whether a removal has a row is asked of the caller's own by-id read
+   * ({@link callerRead}) — the read door's answer to this caller, every read
+   * visibility and wall included: a row the caller cannot read answers what an
+   * id naming no row answers, and no system read is made for it. A row it
+   * reads is then judged on the stored row ({@link storedRow}), never on the
+   * caller's read of it, which is served through the caller's field rules and
+   * masking (ruling of record 6649064874); `reads` holds the columns that read
+   * returned at their stored value ({@link columnsReadAsStored}). A read
+   * refusal or a store fault, in either read, propagates as raised: an outage
+   * is neither an answer nor a refusal, and the by-id write door answers a
+   * caller the read door refuses with that refusal.
    */
-  private async organizationTargets(opCtx: any): Promise<Array<{ next: any | null; prev: any | null }>> {
-    if (opCtx.operation === 'insert') return rowsOf(opCtx).map((r) => ({ next: r, prev: null }));
+  private async organizationTargets(opCtx: any): Promise<DelegatedTarget[]> {
+    if (opCtx.operation === 'insert') {
+      return rowsOf(opCtx).map((r) => ({ next: r, prev: null, wrote: null, reads: new Set<string>() }));
+    }
     const id = extractSingleId(opCtx);
-    return [{ next: null, prev: id != null ? await this.callerRead(opCtx, id) : null }];
+    const seen = id != null ? await this.callerRead(opCtx, id) : null;
+    const prev = seen && id != null ? await this.storedRow(opCtx.object, id) : null;
+    return [{ next: null, prev, wrote: new Set<string>(), reads: columnsReadAsStored(seen, prev) }];
   }
 
   /**
@@ -1284,7 +1315,7 @@ export class DelegatedAdminGate {
       const positionSide = t.next && (t.next.position != null || !t.prev) ? 'next' : 'prev';
       const positionName = String(t.next?.position ?? t.prev?.position ?? '');
       const boundSets = positionName ? await this.setsBoundToPosition(positionName) : [];
-      // [ruling of record 5985885287, sub-item B] Each value below is named
+      // [ruling of record 6649064874] Each value below is named
       // only when the caller wrote or reads it ({@link mayName}); the verdict
       // is the stored row's either way.
       const positionNamed = mayName(t, positionSide, 'position');
@@ -1350,7 +1381,7 @@ export class DelegatedAdminGate {
       const { setRow, setName, unresolved, setNamed } = await this.directGrantSet(t, row);
       const targetUserId = row.user_id ? String(row.user_id) : null;
       const userBUs = targetUserId ? await this.businessUnitsOfUser(targetUserId) : new Set<string>();
-      // [ruling of record 5985885287, sub-item B] Named only when the caller
+      // [ruling of record 6649064874] Named only when the caller
       // wrote or reads it ({@link mayName}); the verdict is the stored row's.
       const userNamed = mayName(t, side, 'user_id');
 
@@ -1613,10 +1644,11 @@ export class DelegatedAdminGate {
    * ({@link callerRead}): a row the caller cannot read answers exactly what an
    * id naming no row answers — one refusal, here at the gate, never a
    * fall-through to the engine. A row the caller reads is judged on the stored
-   * row, as before, never on the caller's read of it: that read is served
-   * through the caller's field-level security, and a judgement made on what a
-   * field rule hid would be made on less than the row. So a row the caller
-   * reads keeps its verdict and its guidance.
+   * row, never on the caller's read of it (ruling of record 6649064874): that
+   * read is served through the caller's field-level security, and a judgement
+   * made on what a field rule hid would be made on less than the row. So a row
+   * the caller reads keeps its verdict, and its guidance names only what the
+   * caller may be told ({@link mayName}).
    *
    * A read the read door REFUSES (a declared 4xx — the object's read grant
    * withheld) hides no row: the stored row is judged as before, and a refusal