fix(plugin-security)!: granted_by is provenance — the delegated-admin gate stamps the writer on every non-system insert, and the column is readonly

.changeset/22201-granted-by-provenance.md

@@ -0,0 +1,26 @@
+---
+'@objectstack/plugin-security': minor
+---
+
+fix(plugin-security)!: `granted_by` on the grant tables is provenance: the platform stamps the writer, and a non-system caller can no longer name the granter
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) An accept-set narrowing at the delegated-admin write gate over an existing column: `sys_user_permission_set.granted_by` and `sys_user_position.granted_by` gain `readonly: true` and keep their name, type, target and storage, so no authorable key, spelling, export or stored shape moves and there is nothing for `objectstack migrate meta` to rewrite. A caller that sent a granter has nothing to change in what it sends; the value it named is replaced by its own user id. Stored rows keep their bytes. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this column and this diff adds none (not registered or already-registered); and the change narrows what a runtime write door stores, not a runtime TypeScript interface or a type surface alone (not runtime-interface-only or type-surface-only). -->
+
+**BREAKING** accept-set narrowing, shipped as `minor` under the repo's launch-window convention for breaking changes.
+
+`granted_by` records who wrote a grant. The platform already treated it that way: the delegated-admin gate stamped the writer, and the organization-admin reconcile writes the attributed human or null (ADR-0118 D1, an id or null and never a sentinel). But the column was a plain lookup, so a non-system caller could store any existing user as the granter, and the integrity audit filed a granter that no longer resolves as a broken business reference, which raised its `[integrity] stored references that resolve to nothing` warning on every run.
+
+**What moves for consumers.**
+
+- **Non-system inserts store the writer.** The delegated-admin gate now stamps the caller's user id into `granted_by` on every `sys_user_permission_set` and `sys_user_position` insert it admits, whatever the payload carried. A tenant-level admin's insert and a delegate's insert are stamped alike. Before, a supplied value was stored as sent, and a tenant-level admin's insert that supplied none stored `null`.
+- **Non-system updates cannot change it.** Both columns are now `readonly: true`, so the engine's update strip drops a caller's `granted_by` and the row keeps its granter. Before, the update landed. A caller that passes `strictReadonlyWrites` is refused instead, as for every read-only column.
+- **The integrity audit files it as provenance.** A `granted_by` that names a user who no longer exists is reported under the audit's `provenance` bucket, which does not raise the warning. A business reference that resolves to nothing is reported under `dangling` and warns, as before.
+
+**Unchanged.**
+
+- System-context writers keep the value they write: invitation placement stamps the issuer, the organization-admin reconcile stamps the attributed human or `null`, and the platform-admin promotion writes `null`. A server-side writer that must record someone other than its caller writes under a system context, as these do.
+- Who may write a grant, and what a grant gives, are unchanged. The gate's authority decision runs first; a refused write is not stamped.
+- Stored rows are not rewritten, and no stored row is refused on read. A row whose `granted_by` already names someone else, or holds a value that names no user, keeps it.
+- `sys_record_share.granted_by` (plugin-sharing) is unchanged: its only writer is the sharing service, which already stamps the acting user under a system context.

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

@@ -112,7 +112,7 @@ const insertAssignment = (ctx: any, row: any) => h.gate.assert({
 });
 
 describe('DelegatedAdminGate — tenant admins and outsiders', () => {
-  it('tenant-level admin (superuser wildcard) passes untouched', async () => {
+  it('tenant-level admin (superuser wildcard) passes — the ordinary CRUD/RLS checks decide', async () => {
     await expect(insertAssignment(h.ctxOf('tenant_admin'), {
       user_id: 'u_west_1', position: 'mixed_pos', business_unit_id: 'bu_west',
     })).resolves.toBeUndefined();
@@ -979,3 +979,72 @@ describe('DelegatedAdminGate — a position is picked by organization, not by ar
     })).rejects.toThrow(/position 'closer' distributes permission set 'finance_user', which is not in the scope's allowlist/);
   });
 });
+
+// ── granted_by is the writer on every admitted insert ─────────────────────
+//
+// `granted_by` is provenance (`readonly` on both grant tables): the gate stamps
+// the writer on every non-system insert it admits, whatever the payload
+// carried — a caller-supplied granter is overwritten, not kept, and a
+// tenant-level admin's grant is stamped too. What is stored on the real engine
+// is pinned in `granted-by-writer-provenance.test.ts`; this block pins the
+// gate's own half, path by path.
+describe('DelegatedAdminGate — granted_by is the writer on every admitted insert', () => {
+  const grantRow = (extra: Record<string, unknown> = {}): any => ({ user_id: 'u_east_1', permission_set_id: 'ps_sales', ...extra });
+  const assignmentRow = (extra: Record<string, unknown> = {}): any => ({ user_id: 'u_east_1', position: 'sales_rep', business_unit_id: 'bu_es', ...extra });
+
+  for (const [object, row] of [
+    ['sys_user_position', assignmentRow],
+    ['sys_user_permission_set', grantRow],
+  ] as const) {
+    for (const principal of ['tenant_admin', 'delegate'] as const) {
+      it(`${object}: ${principal} naming another user as granter is stamped with the writer`, async () => {
+        const data = row({ granted_by: 'u_west_1' });
+        await expect(h.gate.assert({ object, operation: 'insert', data, context: h.ctxOf(principal) })).resolves.toBeUndefined();
+        expect(data.granted_by).toBe(`u_${principal}`);
+      });
+
+      it(`${object}: ${principal} naming no granter is stamped with the writer`, async () => {
+        const data = row();
+        await expect(h.gate.assert({ object, operation: 'insert', data, context: h.ctxOf(principal) })).resolves.toBeUndefined();
+        expect(data.granted_by).toBe(`u_${principal}`);
+      });
+    }
+
+    it(`${object}: every row of a batch insert is stamped`, async () => {
+      const data = [row({ granted_by: 'u_west_1' }), row()];
+      await expect(h.gate.assert({ object, operation: 'insert', data, context: h.ctxOf('tenant_admin') })).resolves.toBeUndefined();
+      expect(data.map((r: any) => r.granted_by)).toEqual(['u_tenant_admin', 'u_tenant_admin']);
+    });
+
+    it(`${object}: a refused insert is not stamped`, async () => {
+      const data = row({ granted_by: 'u_west_1' });
+      await expect(h.gate.assert({ object, operation: 'insert', data, context: h.ctxOf('crud_only') })).rejects.toThrow(/delegated adminScope/);
+      expect(data.granted_by).toBe('u_west_1');
+    });
+
+    it(`${object}: an update is not stamped — the column is readonly, so the engine's update strip owns it`, async () => {
+      const data = { id: 'a_prev', granted_by: 'u_west_1' };
+      await expect(h.gate.assert({ object, operation: 'update', data, context: h.ctxOf('tenant_admin') })).resolves.toBeUndefined();
+      expect(data.granted_by).toBe('u_west_1');
+    });
+  }
+
+  it('a self-delegation naming another user as granter is stamped with the delegator', async () => {
+    const d = makeDelegationHarness();
+    const row: any = {
+      user_id: 'u_deleg', position: 'approver', delegated_from: 'u_boss', valid_until: iso(T0 + 10 * DAY),
+      reason: 'vacation stand-in', granted_by: 'u_relay',
+    };
+    await expect(d.delegate('u_boss', row)).resolves.toBeUndefined();
+    expect(row.granted_by).toBe('u_boss');
+  });
+
+  it('a governed object with no granted_by column is never given one', async () => {
+    const member: any = { user_id: 'u_x', organization_id: 'org_1', role: 'admin' };
+    await expect(h.gate.assert({ object: 'sys_member', operation: 'insert', data: member, context: h.ctxOf('tenant_admin') })).resolves.toBeUndefined();
+    const binding: any = { position_id: 'pos_sales', permission_set_id: 'ps_sales' };
+    await expect(h.gate.assert({ object: 'sys_position_permission_set', operation: 'insert', data: binding, context: h.ctxOf('tenant_admin') })).resolves.toBeUndefined();
+    expect('granted_by' in member).toBe(false);
+    expect('granted_by' in binding).toBe(false);
+  });
+});

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

@@ -148,6 +148,40 @@ const GOVERNED_OBJECTS = new Set([
 const GOVERNED_OPERATIONS = new Set(['insert', 'update', 'delete', 'transfer', 'restore', 'purge']);
 const ANCHOR_POSITIONS = new Set(['everyone', 'guest']);
 
+/**
+ * The governed objects whose rows record their writer in `granted_by` — a
+ * `readonly` provenance column on both (ADR-0090 D12's dual audit: `granted_by`
+ * = writer, `delegated_from` = authority source, `reason` = why).
+ *
+ * The writer is the value, so a caller does not get to name one: every
+ * non-system insert that this gate admits is stamped with its own `userId`,
+ * whatever the payload carried — tenant-level admins, delegates and
+ * self-delegations alike. The stamp is assigned in place on the payload rows
+ * BEFORE the engine's insert pipeline snapshots them; the engine's create-side
+ * `readonly` strip does not judge `sys_` objects, so the stamp is what lands.
+ * A non-system UPDATE gets no stamp: the column is `readonly`, so the engine's
+ * update strip drops a caller's value and the row keeps its writer.
+ *
+ * System writers never reach this gate (`isSystem` short-circuits the security
+ * middleware first) and keep the id or null they wrote: invitation placement
+ * writes its issuer, the organization-admin reconcile the attributed human or
+ * null, the platform-admin promotion null (ADR-0118 D1 — never a sentinel).
+ */
+const GRANTED_BY_OBJECTS = new Set(['sys_user_position', 'sys_user_permission_set']);
+
+/**
+ * Stamp the writer into `granted_by` on every row of an admitted non-system
+ * insert into a {@link GRANTED_BY_OBJECTS} table. Unconditional on purpose:
+ * keeping a supplied value would let a caller name any user as the granter.
+ */
+function stampWriter(opCtx: any, userId: unknown): void {
+  if (opCtx?.operation !== 'insert' || !GRANTED_BY_OBJECTS.has(opCtx?.object)) return;
+  if (userId == null || userId === '') return;
+  for (const row of rowsOf(opCtx)) {
+    if (row && typeof row === 'object') row.granted_by = userId;
+  }
+}
+
 export interface DelegatedAdminGateDeps {
   /** ObjectQL engine handle (system-context reads for pre-images/lookups). */
   ql: any;
@@ -242,6 +276,14 @@ export class DelegatedAdminGate {
     if (!GOVERNED_OBJECTS.has(opCtx?.object)) return;
     if (!GOVERNED_OPERATIONS.has(opCtx?.operation)) return;
 
+    // Authority first: a refused write throws here and is never stamped.
+    await this.authorize(opCtx);
+    // Then provenance, on every admitted path — see {@link GRANTED_BY_OBJECTS}.
+    stampWriter(opCtx, opCtx.context?.userId);
+  }
+
+  /** The authority decision for a governed write: returns when admitted, throws when refused. */
+  private async authorize(opCtx: any): Promise<void> {
     const ctx = opCtx.context ?? {};
     // Every business-unit anchor this call resolves is resolved in the
     // caller's own organization — see {@link callerOrganizationId}.
@@ -346,9 +388,9 @@ export class DelegatedAdminGate {
 
     switch (opCtx.object) {
       case 'sys_user_position':
-        return this.assertAssignmentWrite(opCtx, ctx, held, organizationId);
+        return this.assertAssignmentWrite(opCtx, held, organizationId);
       case 'sys_user_permission_set':
-        return this.assertDirectGrantWrite(opCtx, ctx, held);
+        return this.assertDirectGrantWrite(opCtx, held);
       case 'sys_position_permission_set':
         return this.assertBindingWrite(opCtx, held);
       case 'sys_permission_set':
@@ -536,8 +578,9 @@ export class DelegatedAdminGate {
    *  7. [cloud#830 follow-up] if the delegation carries a `business_unit_id`
    *     anchor, that anchor falls inside the delegator's OWN effective anchor
    *     for the position — a delegation may NARROW visibility, never widen it.
-   * The writer is stamped into `granted_by` (dual audit: `granted_by` = writer,
-   * `delegated_from` = authority source).
+   * The writer is stamped into `granted_by` by {@link DelegatedAdminGate.assert}
+   * once this returns (dual audit: `granted_by` = writer, `delegated_from` =
+   * authority source).
    */
   private async assertSelfDelegation(opCtx: any, ctx: any, organizationId?: string): Promise<void> {
     const now = this.now();
@@ -640,9 +683,6 @@ export class DelegatedAdminGate {
           deny(`position '${positionName}' distributes the admin set '${b.name}' — administration cannot be self-delegated (D12 containment)`, { position: positionName, permissionSet: b.name });
         }
       }
-
-      // Dual audit: stamp the writer (never overwrite an explicit value).
-      if (row.granted_by == null) row.granted_by = ctx.userId;
     }
   }
 
@@ -737,7 +777,6 @@ export class DelegatedAdminGate {
 
   private async assertAssignmentWrite(
     opCtx: any,
-    ctx: any,
     held: HeldScope[],
     organizationId?: string,
   ): Promise<void> {
@@ -777,16 +816,12 @@ export class DelegatedAdminGate {
           { operation: opCtx.operation, object: opCtx.object, position: positionName },
         );
       }
-      // Audit stamp: who granted this (insert only; never overwrite an explicit value).
-      if (opCtx.operation === 'insert' && t.next && t.next.granted_by == null && ctx.userId) {
-        t.next.granted_by = ctx.userId;
-      }
     }
   }
 
   // ── sys_user_permission_set: direct user ↔ set grants ───────────────
 
-  private async assertDirectGrantWrite(opCtx: any, ctx: any, held: HeldScope[]): Promise<void> {
+  private async assertDirectGrantWrite(opCtx: any, held: HeldScope[]): Promise<void> {
     const targets = await this.materializeTargets(opCtx, 'sys_user_permission_set');
     for (const t of targets) {
       const row = t.next ?? t.prev ?? {};
@@ -817,9 +852,6 @@ export class DelegatedAdminGate {
           { operation: opCtx.operation, object: opCtx.object, permissionSet: setName },
         );
       }
-      if (opCtx.operation === 'insert' && t.next && t.next.granted_by == null && ctx.userId) {
-        t.next.granted_by = ctx.userId;
-      }
     }
   }