fix(audit): name the impersonating administrator on identity writes and on an approval decided while impersonating

.changeset/30851-impersonation-identity-and-decider.md

@@ -0,0 +1,18 @@
+---
+'@objectstack/spec': patch
+'@objectstack/plugin-auth': patch
+'@objectstack/plugin-approvals': patch
+'@objectstack/service-automation': patch
+---
+
+fix(audit): name the impersonating administrator on identity writes and on an approval decided while impersonating
+
+Clause-②: yes (widening)
+Metadata-protocol: none
+
+Two writes an administrator makes while impersonating a user still read as the user's own in the audit ledger. Both now carry the same attribution-only `impersonatedBy` a record write from that session already carries, recorded as `sys_audit_log.metadata.impersonated_by`. `user_id`, `actor` and every permission stay the impersonated user's.
+
+- **Identity-table writes (`@objectstack/plugin-auth`).** A write better-auth makes on the user's behalf (renaming the profile, changing an email, a membership change, an admin route such as set-user-manager) runs as the system and is credited to the user through `attributedUserId`. On an impersonation session it now also names the administrator, read from the session row's `impersonatedBy`, never from the request body. `authSystemWriteContext()` builds it, and the adapter's `withSystemContext` writes now use that same builder. `resolveAttributedActor()` (new) answers the user and the administrator together; `runAttributedToUser()` also accepts the admitted actor object, whose new optional `impersonatedBy` the platform-admin gate fills from the session.
+- **The approval decider's segment (`@objectstack/spec`, `@objectstack/plugin-approvals`, `@objectstack/service-automation`).** `ResumeSignal.impersonatedBy` (new, optional) names the administrator when the decider decided from an impersonation session. It is honoured only beside `resumedBy`, on a signal carrying `RESUME_AUTHORITY_SERVICE`, so an HTTP body cannot set it. The approvals decision door fills it from the decision request's context, and a stranded decision's journal keeps it for the replay. The nodes after the decision now carry exactly the decision's administrator, and none when the decider acted in their own name. Before this, a trigger user deciding their own run kept the stamp of the write that started it, and a decider deciding while impersonated carried none.
+
+No schema column and no migration.

packages/plugins/plugin-approvals/src/approval-service.ts

@@ -206,6 +206,14 @@ export interface ApprovalResumeSurface {
      * decided (the SLA sweep), and then the run keeps the identity it had.
      */
     resumedBy?: string;
+    /**
+     * The administrator impersonating {@link resumedBy} when the decision came
+     * from an impersonation session (`ResumeSignal.impersonatedBy`), read off
+     * the decision request's context ({@link impersonatorOf}); the engine
+     * stamps it on the decider's segment. Absent when the decider acted in
+     * their own name, and never sent without `resumedBy`.
+     */
+    impersonatedBy?: string;
     /**
      * #3801: the engine refuses a resume of an `approval` suspension unless
      * the signal carries this marker — the proof that the resume is the tail
@@ -960,6 +968,13 @@ export interface StrandedContinuationSignal {
    * original carried none.
    */
   resumedBy?: string;
+  /**
+   * The administrator the original resume named as impersonating
+   * {@link resumedBy} (`ResumeSignal.impersonatedBy`), so a replay stamps the
+   * decider's segment exactly as the decision did. Absent when the original
+   * carried none.
+   */
+  impersonatedBy?: string;
 }
 
 /**
@@ -1110,6 +1125,18 @@ function actingUserId(context: ExecutionContext | undefined): string | null {
   return typeof userId === 'string' && userId ? userId : null;
 }
 
+/**
+ * The administrator a request's context names as impersonating its user — the
+ * session-resolved, attribution-only `ExecutionContext.impersonatedBy` — as a
+ * non-blank string, or `undefined`. Never read off a caller's input.
+ */
+function impersonatorOf(context: ExecutionContext | undefined): string | undefined {
+  const by = context?.impersonatedBy;
+  if (typeof by !== 'string') return undefined;
+  const trimmed = by.trim();
+  return trimmed ? trimmed : undefined;
+}
+
 /**
  * The PERSON an action records (ADR-0118 D1): a `sys_user` id, or `null` —
  * never the slot the action took (that is `acted_as`), never an address the
@@ -5268,7 +5295,7 @@ export class ApprovalService implements IApprovalService {
    */
   private async serviceResume(
     runId: string,
-    signal: { output?: Record<string, unknown>; branchLabel?: string; resumedBy?: string },
+    signal: { output?: Record<string, unknown>; branchLabel?: string; resumedBy?: string; impersonatedBy?: string },
   ): Promise<{ runId: string; repairable: boolean; error: string } | undefined> {
     const result = await this.automation!.resume!(runId, { ...signal, [RESUME_AUTHORITY_SERVICE]: true });
     const reported = result as
@@ -5485,7 +5512,7 @@ export class ApprovalService implements IApprovalService {
     runId: string,
     requestId: string,
     what: string,
-    signal: { output?: Record<string, unknown>; branchLabel?: string; resumedBy?: string },
+    signal: { output?: Record<string, unknown>; branchLabel?: string; resumedBy?: string; impersonatedBy?: string },
     decision: string,
   ): Promise<{ resumed: boolean; resumeError?: string; resumeFailure?: ResumeFailureReport }> {
     const missing = this.missingRunCapability(runId, requestId, what, 'resume');
@@ -5546,6 +5573,7 @@ export class ApprovalService implements IApprovalService {
         await this.journalStrandedContinuation(requestId, {
           branchLabel: signal.branchLabel, output: signal.output, decision, what,
           ...(signal.resumedBy ? { resumedBy: signal.resumedBy } : {}),
+          ...(signal.resumedBy && signal.impersonatedBy ? { impersonatedBy: signal.impersonatedBy } : {}),
         });
       }
       throw strandedDecisionFailure(
@@ -5618,6 +5646,11 @@ export class ApprovalService implements IApprovalService {
       // vouches for nobody, so its auto-decision names no decider and the run
       // keeps the identity it had.
       const decider = actingUserId(context);
+      // The administrator at the keyboard when this decision came from an
+      // impersonation session, off the decision request's own context (the
+      // session), never off the input: the decider's segment names them, as
+      // the decision's own audit row does. Only beside a decider.
+      const impersonatedBy = decider ? impersonatorOf(context) : undefined;
       const outcome = await this.resumeRecordedOutcome(
         result.runId, requestId, `the ${result.decision} decision`,
         {
@@ -5629,6 +5662,7 @@ export class ApprovalService implements IApprovalService {
           // whitelist already rejects them; this is defense in depth).
           output: { ...(result.outputs ?? {}), decision: result.decision, requestId },
           ...(decider ? { resumedBy: decider } : {}),
+          ...(impersonatedBy ? { impersonatedBy } : {}),
         },
         result.decision,
       );
@@ -8025,6 +8059,11 @@ export class ApprovalService implements IApprovalService {
         // The journalled decider (ruling A): the replayed nodes after the
         // decision run as the person who decided, not as the run's trigger.
         ...(typeof signal.resumedBy === 'string' && signal.resumedBy ? { resumedBy: signal.resumedBy } : {}),
+        // The journalled impersonator beside that decider: the replayed
+        // segment is stamped exactly as the decision's own resume would have.
+        ...(typeof signal.resumedBy === 'string' && signal.resumedBy
+          && typeof signal.impersonatedBy === 'string' && signal.impersonatedBy.trim()
+          ? { impersonatedBy: signal.impersonatedBy.trim() } : {}),
       },
       signal.decision,
     );

packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts

@@ -948,7 +948,7 @@ describe('ruling A — a decision resumes the run as the person who decided', ()
 
   let data: ReturnType<typeof makeFakeEngine>;
   let service: ApprovalService;
-  let seen: Record<string, { userId?: string; permissions?: string[]; isSystem?: boolean }>;
+  let seen: Record<string, { userId?: string; permissions?: string[]; isSystem?: boolean; impersonatedBy?: string }>;
   let rejectThrowsOnce: boolean;
 
   function boot() {
@@ -963,7 +963,12 @@ describe('ruling A — a decision resumes the run as the person who decided', ()
           throw new Error(DOWNSTREAM_FAILURE);
         }
         const ctx = resolveRunDataContext(context);
-        seen[node.id] = { userId: ctx?.userId, permissions: ctx?.permissions, ...(ctx?.isSystem ? { isSystem: true } : {}) };
+        seen[node.id] = {
+          userId: ctx?.userId, permissions: ctx?.permissions,
+          ...(ctx?.isSystem ? { isSystem: true } : {}),
+          // Present only on a write the audit writer would stamp `impersonated_by`.
+          ...((ctx as any)?.impersonatedBy !== undefined ? { impersonatedBy: (ctx as any).impersonatedBy } : {}),
+        };
         return { success: true };
       },
     } as never);
@@ -1034,6 +1039,45 @@ describe('ruling A — a decision resumes the run as the person who decided', ()
     expect(out.source).toBe('journal');
     expect(seen.after_reject, 'the replay fell back to the trigger user').toEqual({ userId: 'u1', permissions: ['deal_manager'] });
   });
+
+  // #30851: an administrator impersonating the decider made THIS decision, so
+  // the decider's segment names them; read off the decision request's context
+  // (the session), never off the input.
+  const IMPERSONATED_DECIDER_CTX = { ...DECIDER_CTX, impersonatedBy: 'adm' } as any;
+
+  it.each(['approve', 'reject'] as const)('%s while impersonated: the nodes after the decision carry the administrator beside the decider', async (decision) => {
+    const automation = boot();
+    const req = await parked(automation);
+    const out = await service.decide(req.id, { decision, actorId: 'u1', comment: decision }, IMPERSONATED_DECIDER_CTX);
+    expect(out.resumed, JSON.stringify(out)).toBe(true);
+    expect(seen[`after_${decision}`]).toEqual({ userId: 'u1', permissions: ['deal_manager'], impersonatedBy: 'adm' });
+  });
+
+  it('CONTROL: an administrator named in the decision INPUT is not the session, and stamps nothing', async () => {
+    const automation = boot();
+    const req = await parked(automation);
+    const out = await service.decide(
+      req.id, { decision: 'approve', actorId: 'u1', comment: 'ok', impersonatedBy: 'forged' } as any, DECIDER_CTX,
+    );
+    expect(out.resumed, JSON.stringify(out)).toBe(true);
+    expect(seen.after_approve).toEqual({ userId: 'u1', permissions: ['deal_manager'] });
+  });
+
+  it('a stranded impersonated decision is replayed with the same administrator beside the decider', async () => {
+    const automation = boot();
+    const req = await parked(automation);
+    rejectThrowsOnce = true;
+    const err = await service.decide(req.id, { decision: 'reject', actorId: 'u1', comment: 'no' }, IMPERSONATED_DECIDER_CTX)
+      .then(() => null, (e: Error) => e);
+    expect(err?.message).toMatch(/^RESUME_FAILED/);
+    const journal = JSON.parse((await rowOf(req.id)).node_config_json).__strandedContinuation;
+    expect(journal).toMatchObject({ decision: 'reject', resumedBy: 'u1', impersonatedBy: 'adm' });
+
+    expect((await automation.restoreConsumedSuspension(req.flow_run_id)).restored).toBe(true);
+    const out = await service.continueRestoredRun(req.id, { requestedBy: 'ops', reason: 'fixed the record' });
+    expect(out.resumed, JSON.stringify(out)).toBe(true);
+    expect(seen.after_reject).toEqual({ userId: 'u1', permissions: ['deal_manager'], impersonatedBy: 'adm' });
+  });
 });
 
 /**

packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts

@@ -284,6 +284,58 @@ describe.concurrent('ADR-0068 D2 — /admin/impersonate-user admits the ObjectSt
   });
 });
 
+// ───────────────────────────────────────────────────────────────────────────
+// #30851 — an identity write made from the impersonation session names the
+// administrator beside the impersonated user it is credited to. The engine's
+// `buildProvenance` copies `impersonatedBy` onto the hook context, where the
+// audit writer records it as `sys_audit_log.metadata.impersonated_by`.
+describe.concurrent('an identity write made while impersonating names the administrator', () => {
+  /** Every `sys_user` update's context, as the better-auth adapter hands it to the engine. */
+  const recordUserWrites = (engine: any) => {
+    const writes: Array<{ data: any; context: any }> = [];
+    const update = engine.update.bind(engine);
+    engine.update = async (name: string, data: any, options?: any) => {
+      if (name === 'sys_user') writes.push({ data, context: options?.context });
+      return update(name, data, options);
+    };
+    return writes;
+  };
+  const updateUser = (manager: AuthManager, bearer: string, body: Record<string, unknown>) =>
+    manager.handleRequest(
+      new Request(`${BASE}/update-user`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json', authorization: `Bearer ${bearer}` },
+        body: JSON.stringify(body),
+      }),
+    );
+
+  it('a rename from the impersonation session carries the administrator, read off the session and not the body', async () => {
+    const { engine, manager, adminBearer, adminId, targetId } = await arrange();
+    const writes = recordUserWrites(engine);
+    const started = await impersonate(manager, adminBearer, targetId);
+    expect(started.status).toBe(200);
+
+    const renamed = await updateUser(manager, bearerFrom(started), { name: 'Renamed by admin', impersonatedBy: 'usr_forged' });
+    expect(renamed.status).toBe(200);
+    const write = writes.find((w) => w.data?.name === 'Renamed by admin');
+    expect(write, JSON.stringify(writes)).toBeDefined();
+    expect(write!.context).toMatchObject({ isSystem: true, attributedUserId: targetId, impersonatedBy: adminId });
+    expect(write!.context.userId, 'the write still authorizes as the system').toBeUndefined();
+  });
+
+  it("CONTROL: the administrator's own rename names no administrator", async () => {
+    const { engine, manager, adminBearer, adminId } = await arrange();
+    const writes = recordUserWrites(engine);
+
+    const renamed = await updateUser(manager, adminBearer, { name: 'Renamed self' });
+    expect(renamed.status).toBe(200);
+    const write = writes.find((w) => w.data?.name === 'Renamed self');
+    expect(write, JSON.stringify(writes)).toBeDefined();
+    expect(write!.context).toMatchObject({ isSystem: true, attributedUserId: adminId });
+    expect('impersonatedBy' in write!.context).toBe(false);
+  });
+});
+
 // ───────────────────────────────────────────────────────────────────────────
 describe.concurrent('the other direction — a non-entitled caller is still refused', () => {
   it('a signed-in plain member is refused 403 YOU_ARE_NOT_ALLOWED_TO_IMPERSONATE_USERS', async () => {

packages/plugins/plugin-auth/src/auth-actor-attribution.test.ts

@@ -16,8 +16,10 @@ import { describe, it, expect, vi } from 'vitest';
 import type { IDataEngine } from '@objectstack/core';
 import {
   runWithAuthActorScope,
+  runAttributedToUser,
   setAuthActorResolver,
   resolveAttributedUserId,
+  resolveAttributedActor,
   authSystemWriteContext,
 } from './auth-actor-attribution';
 import { withSystemContext } from './objectql-adapter';
@@ -204,3 +206,68 @@ describe('withSystemContext carries attribution on WRITES only (#4586)', () => {
     });
   });
 });
+
+// The real-pipeline pin (an impersonated `/update-user` on better-auth) lives
+// with the impersonation harness, in `admin-impersonate-endpoint.test.ts`.
+describe('an impersonation session names its administrator on identity writes (#30851)', () => {
+  it('the context carries the administrator beside the impersonated user, attribution only', async () => {
+    const ctx = await runWithAuthActorScope(async () => {
+      setAuthActorResolver(async () => ({ userId: 'usr_target', impersonatedBy: 'usr_admin' }));
+      return authSystemWriteContext();
+    });
+    expect(ctx).toEqual({ isSystem: true, attributedUserId: 'usr_target', impersonatedBy: 'usr_admin' });
+    expect(ctx).not.toHaveProperty('userId');
+  });
+
+  it('CONTROL: a session that is not an impersonation carries no administrator', async () => {
+    for (const answer of ['usr_target', { userId: 'usr_target' }, { userId: 'usr_target', impersonatedBy: null }, { userId: 'usr_target', impersonatedBy: '  ' }]) {
+      const ctx = await runWithAuthActorScope(async () => {
+        setAuthActorResolver(async () => answer);
+        return authSystemWriteContext();
+      });
+      expect(ctx, JSON.stringify(answer)).toEqual({ isSystem: true, attributedUserId: 'usr_target' });
+    }
+  });
+
+  it('CONTROL: an administrator with no user beside it attributes nothing', async () => {
+    const actor = await runWithAuthActorScope(async () => {
+      setAuthActorResolver(async () => ({ userId: null, impersonatedBy: 'usr_admin' }));
+      return resolveAttributedActor();
+    });
+    expect(actor).toBeUndefined();
+  });
+
+  it('withSystemContext stamps it on insert / update / delete, never on reads', async () => {
+    const engine = {
+      insert: vi.fn().mockResolvedValue({ id: '1' }),
+      update: vi.fn().mockResolvedValue({ id: '1' }),
+      delete: vi.fn().mockResolvedValue(undefined),
+      find: vi.fn().mockResolvedValue([]),
+      findOne: vi.fn().mockResolvedValue({ id: '1' }),
+      count: vi.fn().mockResolvedValue(0),
+    } as unknown as IDataEngine;
+    await runWithAuthActorScope(async () => {
+      setAuthActorResolver(async () => ({ userId: 'usr_target', impersonatedBy: 'usr_admin' }));
+      const e = withSystemContext(engine);
+      await e.insert('sys_user', { id: 'u1' } as any);
+      await e.update('sys_user', { id: 'u1' } as any);
+      await e.delete('sys_user', { where: { id: 'u1' } } as any);
+      await e.find('sys_user', { where: {} });
+    });
+    const expected = { context: { attributedUserId: 'usr_target', impersonatedBy: 'usr_admin', isSystem: true } };
+    expect(engine.insert).toHaveBeenCalledWith('sys_user', { id: 'u1' }, expected);
+    expect(engine.update).toHaveBeenCalledWith('sys_user', { id: 'u1' }, expected);
+    expect(engine.delete).toHaveBeenCalledWith('sys_user', expect.objectContaining(expected));
+    expect(engine.find).toHaveBeenCalledWith('sys_user', expect.objectContaining({ context: { isSystem: true } }));
+  });
+
+  it('a framework route that gated the admitted actor carries the administrator it names', async () => {
+    const impersonated = await runAttributedToUser({ id: 'usr_target', impersonatedBy: 'usr_admin' }, () => authSystemWriteContext());
+    expect(impersonated).toEqual({ isSystem: true, attributedUserId: 'usr_target', impersonatedBy: 'usr_admin' });
+    const plain = await runAttributedToUser({ id: 'usr_admin' }, () => authSystemWriteContext());
+    expect(plain).toEqual({ isSystem: true, attributedUserId: 'usr_admin' });
+    const bare = await runAttributedToUser('usr_admin', () => authSystemWriteContext());
+    expect(bare).toEqual({ isSystem: true, attributedUserId: 'usr_admin' });
+  });
+});
+