Skip to content

Commit 4b4e2fe

Browse files
committed
feat(auth): thread the real better-auth actor into identity writes for attribution (#4586)
better-auth owns every write to the identity tables and its ObjectQL adapter runs them `isSystem: true` on purpose — the route already authorized the action under better-auth's own ACL. The human who clicked *make admin* was known exactly once, in the hook layer, then discarded, so every `trackHistory` transition on `sys_member` recorded "system" as its actor. W1 — a general seam, not a `sys_member` special case: a request-scoped attribution store opened at `AuthManager.handleRequest`, filled lazily from better-auth's global before-hook, surfaced as `ExecutionContext.attributedUserId` → `HookContext.provenance.attributedUserId` and read by the audit writer. W2 — `auto-org-admin-grant` stamps the attributed human into the `granted_by` column it always wrote null into, plus a machine-provenance `reason` naming the writer and the triggering `sys_member` row. W3 — covered at the real routes (invite-accept, update-member-role, the reconciler bind, demotion) in a dogfood test over the live HTTP stack. ATTRIBUTION ONLY: the threaded actor never becomes the authorization subject. It rides `provenance`, which no security middleware reads; `isSystem` stays the unconditional authorization half. Re-authorizing as the human would open the second adjudication track ADR-0095 D3 closed — pinned by tests at the engine seam, the adapter, and the live route. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
1 parent 73d701f commit 4b4e2fe

23 files changed

Lines changed: 1227 additions & 33 deletions

content/docs/references/data/data-engine.mdx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ const result = BaseEngineOptionsSchema.parse(data);
3939

4040
| Property | Type | Required | Description |
4141
| :--- | :--- | :--- | :--- |
42-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
42+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
4343

4444

4545
---
@@ -52,7 +52,7 @@ Options for DataEngine.aggregate operations
5252

5353
| Property | Type | Required | Description |
5454
| :--- | :--- | :--- | :--- |
55-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
55+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
5656
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |
5757
| **groupBy** | `string[]` | optional | |
5858
| **aggregations** | `{ field: string; method: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct'>; alias?: string }[]` | optional | |
@@ -94,7 +94,7 @@ Options for DataEngine.count operations
9494

9595
| Property | Type | Required | Description |
9696
| :--- | :--- | :--- | :--- |
97-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
97+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
9898
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |
9999

100100

@@ -121,7 +121,7 @@ Options for DataEngine.delete operations
121121

122122
| Property | Type | Required | Description |
123123
| :--- | :--- | :--- | :--- |
124-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
124+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
125125
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |
126126
| **multi** | `boolean` | optional | |
127127

@@ -212,7 +212,7 @@ Options for DataEngine.insert operations
212212

213213
| Property | Type | Required | Description |
214214
| :--- | :--- | :--- | :--- |
215-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
215+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
216216
| **returning** | `boolean` | optional | |
217217

218218

@@ -240,7 +240,7 @@ Query options for IDataEngine.find() operations
240240

241241
| Property | Type | Required | Description |
242242
| :--- | :--- | :--- | :--- |
243-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
243+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
244244
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |
245245
| **select** | `string[]` | optional | |
246246
| **sort** | `Record<string, Enum<'asc' \| 'desc'>> \| Record<string, '1' \| '-1'> \| { field: string; order: Enum<'asc' \| 'desc'> }[]` | optional | Sort order definition |
@@ -428,7 +428,7 @@ Options for DataEngine.update operations
428428

429429
| Property | Type | Required | Description |
430430
| :--- | :--- | :--- | :--- |
431-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
431+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
432432
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |
433433
| **upsert** | `boolean` | optional | |
434434
| **multi** | `boolean` | optional | |
@@ -492,7 +492,7 @@ QueryAST-aligned options for DataEngine.aggregate operations
492492

493493
| Property | Type | Required | Description |
494494
| :--- | :--- | :--- | :--- |
495-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
495+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
496496
| **where** | `Record<string, any> \| any` | optional | |
497497
| **groupBy** | `string[]` | optional | |
498498
| **aggregations** | `{ function: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct' \| 'array_agg' \| 'string_agg'>; field?: string; alias: string; distinct?: boolean; … }[]` | optional | |
@@ -510,7 +510,7 @@ QueryAST-aligned options for DataEngine.count operations
510510

511511
| Property | Type | Required | Description |
512512
| :--- | :--- | :--- | :--- |
513-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
513+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
514514
| **where** | `Record<string, any> \| any` | optional | |
515515

516516

@@ -524,7 +524,7 @@ QueryAST-aligned options for DataEngine.delete operations
524524

525525
| Property | Type | Required | Description |
526526
| :--- | :--- | :--- | :--- |
527-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
527+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
528528
| **where** | `Record<string, any> \| any` | optional | |
529529
| **multi** | `boolean` | optional | |
530530

@@ -539,7 +539,7 @@ QueryAST-aligned query options for IDataEngine.find() operations
539539

540540
| Property | Type | Required | Description |
541541
| :--- | :--- | :--- | :--- |
542-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
542+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
543543
| **where** | `Record<string, any> \| any` | optional | |
544544
| **fields** | `string[]` | optional | |
545545
| **orderBy** | `{ field: string; order: Enum<'asc' \| 'desc'> }[]` | optional | |
@@ -563,7 +563,7 @@ QueryAST-aligned options for DataEngine.update operations
563563

564564
| Property | Type | Required | Description |
565565
| :--- | :--- | :--- | :--- |
566-
| **context** | `{ userId?: string; actor?: string; email?: string; tenantId?: string; … }` | optional | |
566+
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
567567
| **where** | `Record<string, any> \| any` | optional | |
568568
| **upsert** | `boolean` | optional | |
569569
| **multi** | `boolean` | optional | |

content/docs/references/data/hook.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ const result = HookContextSchema.parse(data);
3838
| **result** | `any` | optional | Operation result (After hooks only) |
3939
| **previous** | `Record<string, any>` | optional | Record state before operation |
4040
| **session** | `{ userId?: string; actor?: string; organizationId?: string; roles?: string[]; … }` | optional | Current session context |
41-
| **provenance** | `{ flowRunId?: string }` | optional | Server-stamped write provenance (never client-supplied, never an authorization input) |
41+
| **provenance** | `{ flowRunId?: string; attributedUserId?: string }` | optional | Server-stamped write provenance (never client-supplied, never an authorization input) |
4242
| **transaction** | `any` | optional | Database transaction handle |
4343
| **ql** | `any` || ObjectQL Engine Reference |
4444
| **api** | `any` | optional | Cross-object data access (ScopedContext) |

content/docs/references/kernel/execution-context.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const result = ExecutionContextSchema.parse(data);
4949
| :--- | :--- | :--- | :--- |
5050
| **userId** | `string` | optional | |
5151
| **actor** | `string` | optional | |
52+
| **attributedUserId** | `string` | optional | |
5253
| **email** | `string` | optional | |
5354
| **tenantId** | `string` | optional | |
5455
| **timezone** | `string` | optional | |

packages/objectql/src/engine.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,82 @@ describe('ObjectQL Engine', () => {
632632
});
633633
});
634634

635+
/**
636+
* #4586 — the ATTRIBUTED human rides provenance, never the session.
637+
*
638+
* better-auth owns every write to the identity tables and runs them
639+
* `isSystem: true` ON PURPOSE: the route already authorized the action
640+
* under better-auth's own ACL. Threading the real human through so
641+
* `sys_member` history stops saying "system" must therefore change exactly
642+
* one thing — who the write is CREDITED to — and nothing about who it is
643+
* AUTHORIZED as. Re-authorizing as the human would open a second
644+
* adjudication track at the boundary ADR-0095 D3 closed.
645+
*
646+
* These are the pins for that constraint at the engine seam, where the
647+
* context is split into the envelopes hooks and middleware actually read.
648+
*/
649+
describe('attributed actor is attribution, never authorization (#4586)', () => {
650+
beforeEach(async () => {
651+
engine.registerDriver(mockDriver, true);
652+
await engine.init();
653+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} } as any);
654+
});
655+
656+
const capture = () => {
657+
const seen: { session?: any; provenance?: any; user?: any } = {};
658+
engine.registerHook('beforeInsert', async (ctx: any) => {
659+
seen.session = ctx.session;
660+
seen.provenance = ctx.provenance;
661+
seen.user = ctx.user;
662+
}, { object: 'task' });
663+
return seen;
664+
};
665+
666+
it('a better-auth write surfaces the human on provenance and stays a SYSTEM session', async () => {
667+
const seen = capture();
668+
669+
await engine.insert('task', { title: 'grade change' }, {
670+
context: { isSystem: true, attributedUserId: 'usr_admin' } as any,
671+
});
672+
673+
expect(seen.provenance).toEqual({ attributedUserId: 'usr_admin' });
674+
// The authorization half is untouched: still system, still no caller.
675+
expect(seen.session).toMatchObject({ isSystem: true });
676+
expect(seen.session.userId).toBeUndefined();
677+
// And the attributed human must NOT leak into any channel that a
678+
// hook or middleware reads as "the acting user".
679+
expect(seen.session).not.toHaveProperty('attributedUserId');
680+
expect(seen.user).toBeUndefined();
681+
});
682+
683+
it('attribution ALONE authorizes exactly like no context at all (ADR-0118 D2)', async () => {
684+
// "Absence is never system": a context that names only who to credit
685+
// establishes no principal, so it must not become one. Anything else
686+
// would make forgetting `isSystem` an accidental elevation.
687+
const seen = capture();
688+
689+
await engine.insert('task', { title: 'no authority' }, {
690+
context: { attributedUserId: 'usr_admin' } as any,
691+
});
692+
693+
expect(seen.provenance).toEqual({ attributedUserId: 'usr_admin' });
694+
expect(seen.session).toBeUndefined();
695+
expect(seen.user).toBeUndefined();
696+
});
697+
698+
it('a real caller keeps their own session; the two envelopes never merge', async () => {
699+
const seen = capture();
700+
701+
await engine.insert('task', { title: 'both' }, {
702+
context: { userId: 'u1', attributedUserId: 'usr_admin', flowRunId: 'run_9' } as any,
703+
});
704+
705+
expect(seen.session).toMatchObject({ userId: 'u1' });
706+
expect(seen.user).toMatchObject({ id: 'u1' });
707+
expect(seen.provenance).toEqual({ flowRunId: 'run_9', attributedUserId: 'usr_admin' });
708+
});
709+
});
710+
635711
describe('execution context via the trailing options arg (read methods)', () => {
636712
// Regression: reads took context inside the query while writes took it in
637713
// a trailing options arg — so `find(obj, q, { context })` silently dropped

packages/objectql/src/engine.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,17 +1181,28 @@ export class ObjectQL implements IObjectQLEngine {
11811181
}
11821182

11831183
/**
1184-
* Build the HookContext.provenance envelope — WHAT produced this write.
1184+
* Build the HookContext.provenance envelope — WHERE this write came from.
11851185
*
11861186
* Deliberately separate from {@link buildSession}: provenance is server-
11871187
* stamped, evaluated by no security middleware, and can exist with no
11881188
* identity beside it. A schedule-triggered flow run resolves no principal
11891189
* yet still owns its writes, and that is the case the approvals record lock
11901190
* needs to recognize (#3456 / #3712).
1191+
*
1192+
* `attributedUserId` rides the SAME envelope for the same reason (#4586):
1193+
* a better-auth-originated write authorizes as the system, so the human who
1194+
* triggered it must reach the audit writer WITHOUT appearing in `session` —
1195+
* where every caller-gating hook would read them as the caller. Attribution
1196+
* here, authorization in `session`/`isSystem`, never the two mixed.
11911197
*/
11921198
private buildProvenance(execCtx?: ExecutionContextInput): HookContext['provenance'] {
11931199
const flowRunId = (execCtx as any)?.flowRunId;
1194-
return flowRunId ? { flowRunId: String(flowRunId) } : undefined;
1200+
const attributedUserId = (execCtx as any)?.attributedUserId;
1201+
if (!flowRunId && !attributedUserId) return undefined;
1202+
return {
1203+
...(flowRunId ? { flowRunId: String(flowRunId) } : {}),
1204+
...(attributedUserId ? { attributedUserId: String(attributedUserId) } : {}),
1205+
};
11951206
}
11961207

11971208
/**

packages/plugins/plugin-audit/src/audit-writers.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,63 @@ describe('audit writers — actor attribution (ADR-0014 D2, cloud#340)', () => {
183183
expect(audit?.row.actor).toBeNull();
184184
expect(audit?.row.user_id).toBeNull();
185185
});
186+
187+
/**
188+
* [#4586] The `sys_member` case the issue is about: better-auth authorizes
189+
* identity writes as the SYSTEM on purpose, so the session names no caller
190+
* and every grade change used to record as "system". The human arrives on
191+
* PROVENANCE instead — attribution, never authorization.
192+
*/
193+
it('credits the attributed human when the write authorized as the system', async () => {
194+
const { engine, fire, created } = makeEngine(SINGLE_TENANT);
195+
installAuditWriters(engine as any, 'test.audit');
196+
await fire('afterUpdate', {
197+
object: 'sys_member',
198+
input: { id: 'mem-1' },
199+
__previous: { id: 'mem-1', role: 'member' },
200+
result: { id: 'mem-1', role: 'admin' },
201+
// Exactly the envelope `withSystemContext` produces for an
202+
// `organization/update-member-role` call.
203+
session: { isSystem: true },
204+
provenance: { attributedUserId: 'user-admin' },
205+
});
206+
const audit = created.find((c) => c.object === 'sys_audit_log');
207+
expect(audit?.row.action).toBe('update');
208+
// WHO changed the grade — a real sys_user id, so the lookup still joins
209+
// (ADR-0118 D1: an id or null, never a sentinel like 'system').
210+
expect(audit?.row.user_id).toBe('user-admin');
211+
expect(audit?.row.actor).toBe('user-admin');
212+
});
213+
214+
it('a genuinely machine-originated write still records as the system (null)', async () => {
215+
// Boot sync / migration / the kernel:ready backfill: no scope, no actor.
216+
// Absence must stay absence — never upgraded into some ambient user.
217+
const { engine, fire, created } = makeEngine(SINGLE_TENANT);
218+
installAuditWriters(engine as any, 'test.audit');
219+
await fire('afterInsert', {
220+
object: 'sys_member',
221+
input: { id: 'mem-2' },
222+
result: { id: 'mem-2', role: 'member' },
223+
session: { isSystem: true },
224+
});
225+
const audit = created.find((c) => c.object === 'sys_audit_log');
226+
expect(audit?.row.user_id).toBeNull();
227+
expect(audit?.row.actor).toBeNull();
228+
});
229+
230+
it('a real caller outranks attribution — the session subject wins', async () => {
231+
const { engine, fire, created } = makeEngine(SINGLE_TENANT);
232+
installAuditWriters(engine as any, 'test.audit');
233+
await fire('afterInsert', {
234+
object: 'crm_lead',
235+
input: { id: 'lead-3' },
236+
result: { id: 'lead-3', name: 'Gamma' },
237+
session: { userId: 'user-7' },
238+
provenance: { attributedUserId: 'user-admin' },
239+
});
240+
const audit = created.find((c) => c.object === 'sys_audit_log');
241+
expect(audit?.row.user_id).toBe('user-7');
242+
});
186243
});
187244

188245
describe('audit writers — declarative trackHistory activity (ADR-0052 §5b)', () => {

packages/plugins/plugin-audit/src/audit-writers.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,26 @@ export function installAuditWriters(
476476
if (recordId !== undefined) recordId = String(recordId);
477477

478478
const sess: any = (ctx as any).session ?? {};
479-
const userId: string | undefined = sess.userId;
479+
// [#4586] Two channels can name a human, and they mean different things:
480+
//
481+
// session.userId — the subject the write was AUTHORIZED as.
482+
// provenance.attributedUserId — the human CREDITED for a write the
483+
// system authorized on their behalf.
484+
//
485+
// The second exists because better-auth owns every identity-table write
486+
// and runs them `isSystem` on purpose (the route already authorized under
487+
// its own ACL), which left every `sys_member` grade change recorded as
488+
// "system". Reading it here is what makes the history row name the admin
489+
// who clicked *make admin*. The session subject still WINS when present —
490+
// attribution never overrides who actually acted — and neither channel
491+
// widens what the write may touch (no security middleware reads
492+
// provenance).
493+
const attributedUserId: string | undefined =
494+
typeof (ctx as any).provenance?.attributedUserId === 'string' &&
495+
(ctx as any).provenance.attributedUserId
496+
? (ctx as any).provenance.attributedUserId
497+
: undefined;
498+
const userId: string | undefined = sess.userId ?? attributedUserId;
480499
// Principal label for attribution. Prefer the real user id; otherwise fall
481500
// back to a service/automation principal the host put on the context
482501
// (`ExecutionContext.actor`, e.g. `svc:<name>`). This is what makes a

0 commit comments

Comments
 (0)