fix(plugin-security): a write refused for a caller who cannot read the object answers the same for a hidden row and a missing one

.changeset/write-refusal-read-door-parity.md

@@ -0,0 +1,12 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+fix(plugin-security): a write refused for a caller who cannot read the object answers the same for a hidden row and a missing one
+
+Clause-②: no
+Metadata-protocol: none
+
+The administration gate on the permission-assignment objects (`sys_user_permission_set`, `sys_user_position`, ADR-0090 D12) refuses a write before the object's grants are read, in words written for the write. For a by-id update or delete, a caller whose read of the object is refused now gets that read refusal from it instead — the answer its by-id read gives for every id — so a row and an id naming none answer the same status, code and sentence, with a version token (`If-Match`, `expectedVersion`) and without one.
+
+- **Unchanged:** a caller who reads the object keeps the gate's own sentence (an organization administrator's "never edited in place", the administration hint for a holder of plain grants); a tenant-level administrator writes as before; an insert, a predicate write and a write the platform issues under the caller's context keep the gate's own refusal. Every answer is still a refusal: nothing that was refused is admitted, and nothing admitted is refused.

content/docs/protocol/kernel/http-protocol.mdx

@@ -757,7 +757,11 @@ A version token (`If-Match`, or `expectedVersion`) is compared only against a re
 caller can read: a stale token on such a record is answered `409 CONCURRENT_UPDATE`,
 whether or not the caller may write it. For a record the caller's read does not return,
 and for an id that names no record, a stale token changes none of the answers above. A
-caller refused reading the object gets that read refusal.
+caller refused reading the object gets that read refusal — and so does the same caller's
+update or delete of a permission assignment (`sys_user_permission_set`,
+`sys_user_position`), which is refused before the object's grants are read: for any
+`{record_id}`, with or without a token. A caller who reads the assignment object gets
+the administration refusal's own sentence instead.
 
 ### Delete Record
 

packages/plugins/plugin-security/src/by-id-write-unreadable-not-found.test.ts

@@ -19,6 +19,13 @@
  *     write — are not addressed by the caller, so they keep the answer they
  *     had before: the record-level 403, which names nothing. A not-found there
  *     would carry an id the caller never supplied.
+ *  5. The ADR-0090 D12 administration gate, which refuses a write to the
+ *     permission-assignment objects before any grant is read, answers the
+ *     addressed by-id update and delete of a caller whose read of the object
+ *     is refused with that read refusal, for a row and for an id naming none
+ *     alike. A caller who reads the object, and an insert, keep the gate's own
+ *     sentence; an administrator still writes. (Its own rig, at the foot of
+ *     the file: the two assignment objects and three more permission sets.)
  *
  * The rig is one permission set over two objects: a public parent, and a
  * child whose rows a caller reads when it owns them or they are flagged
@@ -35,6 +42,10 @@ import { recordNotFoundError } from '@objectstack/core';
 import { SecurityPlugin } from './security-plugin.js';
 import { defaultPermissionSets } from './objects/default-permission-sets.js';
 import { installDefaultOrganization } from './default-organization.testkit.js';
+import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js';
+import { SysUserPosition } from './objects/sys-user-position.object.js';
+import { SysUser } from '@objectstack/platform-objects/identity';
+import { ADMIN_FULL_ACCESS } from '@objectstack/spec/identity';
 
 const SYS = { context: { isSystem: true } } as never;
 const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!;
@@ -253,3 +264,176 @@ describe('a by-id write the platform issues under the caller\'s context keeps th
     expect(await r.remove(CHILD, 'c_kid')).toMatchObject({ kind: 'refused', code: 'RECORD_NOT_FOUND', status: 404 });
   });
 });
+
+
+// ── The delegated-administration gate (ADR-0090 D12) ─────────────────────────
+//
+// The gate refuses a write to the permission-assignment objects on the caller
+// and the object alone, before any grant is read, in its own words. For a
+// by-id update or delete the caller addressed, a caller whose read of the
+// object is refused gets the read door's refusal from it instead: the answer
+// that caller's by-id read gives for every id, so a row and an id naming none
+// answer alike. A caller who reads the object keeps the gate's own sentence,
+// an insert (which names no row) keeps it too, and an administrator still
+// writes.
+
+const ASSIGNMENT_OBJECTS = ['sys_user_permission_set', 'sys_user_position'] as const;
+type AssignmentObject = (typeof ASSIGNMENT_OBJECTS)[number];
+const SEEDED_ROW: Record<AssignmentObject, string> = { sys_user_permission_set: 'ups_seeded', sys_user_position: 'upos_seeded' };
+const NOWHERE = 'zz_nowhere_0001';
+const GATE_SENTENCE = 'requires tenant-level administration or a delegated adminScope';
+const ADMIN_SET = defaultPermissionSets.find((p) => p.name === ADMIN_FULL_ACCESS)!;
+
+/** Holds no grant on either object (its one set names the two business objects above). */
+const NO_GRANT = { userId: 'usr_nf_nogrant', positions: [], permissions: ['qa_nf_guard'], posture: 'MEMBER' };
+/** May create, update and delete both objects, and read neither. */
+const WRITE_NO_READ = { userId: 'usr_nf_writer', positions: [], permissions: ['qa_nf_blind_writer'], posture: 'MEMBER' };
+/** Reads both objects, and holds nothing else on them. */
+const READER = { userId: 'usr_nf_reader', positions: [], permissions: ['qa_nf_reader'], posture: 'MEMBER' };
+const ADMIN = { userId: 'usr_nf_admin', positions: [], permissions: [ADMIN_FULL_ACCESS], posture: 'PLATFORM_ADMIN' };
+
+const gateEngines: ObjectQL[] = [];
+afterEach(async () => {
+  for (const engine of gateEngines.splice(0)) {
+    try { await engine.destroy(); } catch { /* noop */ }
+  }
+});
+
+type Read = { kind: 'read'; row: unknown } | ({ kind: 'refused' } & Refusal);
+
+interface GateRig {
+  read: (caller: object, object: string, id: string) => Promise<Read>;
+  write: (caller: object, verb: 'update' | 'delete', object: string, id: string) => Promise<Outcome>;
+  insert: (caller: object, object: string, row: Record<string, unknown>) => Promise<Outcome>;
+  stored: (object: string, id: string) => Promise<Record<string, unknown> | null>;
+}
+
+async function bootGateRig(): Promise<GateRig> {
+  const engine = new ObjectQL();
+  engine.registerDriver(
+    new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as never,
+    true,
+  );
+  await engine.init();
+  engine.registerApp({
+    id: 'com.objectstack.qa.by-id-write-administration-gate',
+    name: 'By-id write: the administration gate answers a caller who cannot read',
+    version: '1.0.0',
+    type: 'plugin',
+    scope: 'system',
+    objects: [SysUserPermissionSet, SysUserPosition],
+  } as never);
+  await engine.syncSchemas();
+  // `sys_user` registered and unprovisioned, as in the sibling suites: the
+  // assignment rows' title formula reads through it.
+  if (!engine.registry.getObject(SysUser.name)) engine.registry.registerObject(SysUser as never, 'qa.by-id-write-administration-gate');
+  await installDefaultOrganization(engine);
+  await engine.insert('sys_user_permission_set', { id: SEEDED_ROW.sys_user_permission_set, user_id: OTHER, permission_set: 'qa_nf_guard' }, SYS);
+  await engine.insert('sys_user_position', { id: SEEDED_ROW.sys_user_position, user_id: OTHER, position: 'qa_pos' }, SYS);
+  gateEngines.push(engine);
+
+  const grant = (allowRead: boolean, write: boolean) =>
+    Object.fromEntries(ASSIGNMENT_OBJECTS.map((o) => [o, { allowRead, allowCreate: write, allowEdit: write, allowDelete: write }]));
+  const sets = [
+    MEMBER_DEFAULT,
+    ADMIN_SET,
+    PermissionSetSchema.parse({
+      name: 'qa_nf_guard',
+      objects: {
+        [PARENT]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true },
+        [CHILD]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true },
+      },
+    }),
+    PermissionSetSchema.parse({ name: 'qa_nf_blind_writer', objects: grant(false, true) }),
+    PermissionSetSchema.parse({ name: 'qa_nf_reader', objects: grant(true, false) }),
+  ];
+  const services: Record<string, unknown> = {
+    manifest: { register: vi.fn() },
+    objectql: engine,
+    metadata: {
+      get: async (type: string, name: string) =>
+        type === 'permission' ? sets.find((s) => s.name === name) ?? null : engine.getSchema(name) ?? null,
+      list: async () => sets,
+    },
+  };
+  const ctx = {
+    logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
+    registerService: vi.fn(),
+    hook: () => undefined,
+    getService: (name: string) => {
+      if (!(name in services)) throw new Error(`service not registered: ${name}`);
+      return services[name];
+    },
+  };
+  const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
+  await plugin.init(ctx as never);
+  await plugin.start(ctx as never);
+
+  return {
+    read: (caller, object, id) =>
+      engine.findOne(object, { where: { id }, context: { ...caller } } as never).then(
+        (row): Read => ({ kind: 'read', row }),
+        (e: any): Read => ({ kind: 'refused', code: e?.code, status: e?.statusCode ?? e?.status, message: String(e?.message), name: e?.name }),
+      ),
+    write: (caller, verb, object, id) =>
+      outcome(
+        verb === 'update'
+          ? engine.update(object, { reason: 'renamed' }, { where: { id }, context: { ...caller } } as never)
+          : engine.delete(object, { where: { id }, context: { ...caller } } as never),
+      ),
+    insert: (caller, object, row) => outcome(engine.insert(object, row, { context: { ...caller } } as never)),
+    stored: async (object, id) =>
+      ((await engine.findOne(object, { where: { id }, context: { isSystem: true } } as never)) ?? null) as Record<string, unknown> | null,
+  };
+}
+
+describe('the administration gate answers a by-id write from a caller who cannot read the object with that read\'s refusal', () => {
+  for (const object of ASSIGNMENT_OBJECTS) {
+    for (const verb of ['update', 'delete'] as const) {
+      for (const [label, caller] of [['no grant on the object', NO_GRANT], ['a write grant without a read grant', WRITE_NO_READ]] as const) {
+        it(`${object} ${verb}, ${label}: the row and an id naming none get the read door's refusal`, async () => {
+          const r = await bootGateRig();
+          const id = SEEDED_ROW[object];
+          const before = await r.stored(object, id);
+          // The precondition, read off the read door: it refuses this caller.
+          const read = await r.read(caller, object, id);
+          expect(read).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 });
+
+          const row = await r.write(caller, verb, object, id);
+          const none = await r.write(caller, verb, object, NOWHERE);
+
+          expect(row, 'the row').toEqual(read);
+          expect(none, 'an id naming none').toEqual(read);
+          expect(await r.stored(object, id), 'refused means untouched').toEqual(before);
+        });
+      }
+
+      it(`${object} ${verb}, a caller who reads the object: the row and an id naming none keep the gate's own refusal`, async () => {
+        const r = await bootGateRig();
+        const id = SEEDED_ROW[object];
+        expect(await r.read(READER, object, id)).toMatchObject({ kind: 'read', row: { id } });
+
+        const row = await r.write(READER, verb, object, id);
+        const none = await r.write(READER, verb, object, NOWHERE);
+
+        expect(row).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 });
+        expect(row.kind === 'refused' && row.message).toContain(GATE_SENTENCE);
+        expect(withoutId(none, NOWHERE)).toEqual(withoutId(row, id));
+      });
+    }
+  }
+
+  it('an administrator still updates an assignment row, and gets not-found for an id naming none', async () => {
+    const r = await bootGateRig();
+    expect(await r.write(ADMIN, 'update', 'sys_user_permission_set', SEEDED_ROW.sys_user_permission_set)).toEqual({ kind: 'landed' });
+    expect((await r.stored('sys_user_permission_set', SEEDED_ROW.sys_user_permission_set))?.reason).toBe('renamed');
+    expect(await r.write(ADMIN, 'update', 'sys_user_permission_set', NOWHERE)).toMatchObject({ kind: 'refused', code: 'RECORD_NOT_FOUND', status: 404 });
+  });
+
+  it('an insert names no row, so a caller who cannot read the object keeps the gate\'s own refusal', async () => {
+    const r = await bootGateRig();
+    const got = await r.insert(NO_GRANT, 'sys_user_permission_set', { user_id: OTHER, permission_set: 'qa_nf_guard' });
+    expect(got).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 });
+    expect(got.kind === 'refused' && got.message).toContain(GATE_SENTENCE);
+  });
+});

packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts

@@ -7,7 +7,9 @@
  * through. The handler retires the permission-set memo on a write, refuses a
  * flow run's identity-assignment writes, lets every other system operation
  * through, and serves the ADR-0056 public-form grant. It then runs the
- * engine-owned write guard and the ADR-0090 D12 delegated-administration gate,
+ * engine-owned write guard and the ADR-0090 D12 delegated-administration gate
+ * (whose refusal of a by-id write answers a caller the read door refuses with
+ * that read refusal, `readDoorAnswerForGateRefusal`),
  * resolves the operation's principals and the object's posture, and runs the
  * numbered steps in the order they stand: the `requiredPermissions` AND-gate
  * (1.5), the CRUD check (2), the access depth and an external principal's
@@ -97,6 +99,7 @@ import { renderOperationMessage } from '@objectstack/spec/system';
 import { resolveEngineUpdateDispatch, type EngineUpdateDispatchData } from '@objectstack/metadata-core';
 import type { II18nService, IMetadataService, IObjectQLEngine, SharingWriteVerdict } from '@objectstack/spec/contracts';
 import { crossFieldClassRefusalCarriedBy, findCrossFieldClassRefusal } from '@objectstack/formula';
+import { declaredHttpStatus } from '@objectstack/types';
 import type { SecurityPlugin } from './security-plugin.js';
 import { isGuestEnvelope, requiredCapsForOperation } from './security-plugin.js';
 import { assertFlowSystemIdentityWriteAllowed } from './flow-system-identity-write-refusal.js';
@@ -228,6 +231,58 @@ function userFacingDenialMessage(
   return renderOperationMessage({ messageKey }, { locale, translate });
 }
 
+/**
+ * [ADR-0090 D12; ruling of record 5985885287] What a by-id write the
+ * delegated-administration gate refused answers a caller the read door
+ * refuses: that door's refusal.
+ *
+ * The gate judges the caller and the object before any grant is read and
+ * before any row is, and words its refusal for the write. The by-id write
+ * doors answer a caller who cannot read a row what they answer for an id
+ * naming no row (ruling of record 5985885287), and where a write is answered
+ * from the caller's own read of its row — the by-id version comparison answers
+ * a mismatched token with that read's refusal (`assertVersionOf`,
+ * `@objectstack/metadata-protocol`) — a caller the read door refuses is
+ * answered that refusal. So the gate answers that caller the same way: the
+ * read door's own question is asked of the id the caller addressed
+ * ({@link SecurityPlugin.getCallerPreImage}: the caller's by-id read through
+ * the engine, never a second evaluator), and
+ *
+ *  - the read is REFUSED (a declared 4xx): that refusal is the answer. It is
+ *    the read door's answer to this caller for any id, so the row and an id
+ *    naming none answer alike, with a version token and without;
+ *  - the read ANSWERS (a row, or none): the caller reads the object, and the
+ *    gate's own refusal stands, its guidance intact;
+ *  - the read FAULTS: the fault propagates as raised (#7505) — an outage is
+ *    neither an answer nor a refusal.
+ *
+ * Asked only of a refusal (a declared 4xx; anything else the gate throws stays
+ * what it is), only for the by-id update or delete the caller addressed (an
+ * insert, a predicate write and a nested write name no row of the caller's —
+ * {@link SecurityPlugin.addressedByIdWriteId}), and never for a row-write
+ * probe, whose verdict is admitted or refused and nothing more.
+ */
+async function readDoorAnswerForGateRefusal(
+  this: Pick<AuthzMiddlewareHost, 'addressedByIdWriteId' | 'getCallerPreImage'>,
+  opCtx: any,
+  refusal: unknown,
+  nestedInOperation: boolean,
+): Promise<unknown> {
+  const status = declaredHttpStatus(refusal);
+  if (status === undefined || status >= 500 || isRowWriteProbe(opCtx)) return refusal;
+  const referentialFieldClear = opCtx.operation === 'update' && opCtx.context?.__referentialFieldClear === true;
+  const addressedId = this.addressedByIdWriteId(opCtx, nestedInOperation, referentialFieldClear);
+  if (addressedId === undefined) return refusal;
+  try {
+    await this.getCallerPreImage(opCtx, addressedId);
+  } catch (readRefusal) {
+    const readStatus = declaredHttpStatus(readRefusal);
+    if (readStatus !== undefined && readStatus < 500) return readRefusal;
+    throw readRefusal;
+  }
+  return refusal;
+}
+
 /**
  * The `SecurityPlugin` members the function below reads — every one of them,
  * and no other; the function's `this` is the `Pick` of the members it reads.
@@ -485,8 +540,16 @@ export function registerAuthzMiddleware(
     // else — including holders of plain CRUD grants on these tables — is
     // denied. Runs BEFORE the principal-less refusal below so RBAC
     // tables fail CLOSED for principal-less non-system contexts.
+    //
+    // It judges before any grant is read, in its own words; on the by-id
+    // update or delete the caller addressed, a caller the read door refuses
+    // is answered that read refusal instead — {@link readDoorAnswerForGateRefusal}.
     if (this.delegatedAdminGate) {
-      await this.delegatedAdminGate.assert(opCtx);
+      try {
+        await this.delegatedAdminGate.assert(opCtx);
+      } catch (refusal) {
+        throw await readDoorAnswerForGateRefusal.call(this, opCtx, refusal, nestedInOperation);
+      }
     }
 
     const positions = opCtx.context?.positions ?? [];