Skip to content

Commit 72fb11f

Browse files
os-zhuangclaude
andcommitted
feat(audit): record a service/non-user principal on sys_audit_log.actor (ADR-0014 D2, cloud#340)
A non-user-authenticated write (e.g. a service token) left `sys_audit_log.user_id` = null and was therefore UNATTRIBUTABLE — exactly how the prod env-delete of `os-790m7q` went unaccountable (a service-token call with no user/IP). `user_id` is a strict `sys_user` lookup, so a service principal can't be stuffed there. This opens the mechanism (the host wires the value — see cloud#340 cloud half): - `spec` ExecutionContext: add an optional `actor` field — a stable principal label for attribution when there's no `userId` (e.g. `svc:<name>`). Additive, backward-compatible; public API surface unchanged. - `plugin-audit` sys_audit_log: add a first-class `actor` text field (the principal label), independent of the `user_id` lookup. `user_id` stays user-only. - audit writer: record `actor = userId ?? session.actor ?? null`. Conditionally stamped (same pattern as organization_id) so audit tables that predate the column keep working. No behavior change until a host sets `ExecutionContext.actor` (cloud control plane will, in service mode). Tests: real user → actor=user id; service token (no userId) → actor='svc:cloud-control', user_id stays null; neither → null. spec 6599/6599 + api-surface ✓; plugin-audit 21/21. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 26d7df4 commit 72fb11f

4 files changed

Lines changed: 90 additions & 2 deletions

File tree

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

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ function makeEngine(
7676

7777
const SINGLE_TENANT = {
7878
// No `organization_id` — single-tenant stacks skip the auto-injection.
79-
sys_audit_log: ['id', 'action', 'user_id', 'object_name', 'record_id', 'old_value', 'new_value', 'tenant_id'],
79+
sys_audit_log: ['id', 'action', 'user_id', 'actor', 'object_name', 'record_id', 'old_value', 'new_value', 'tenant_id'],
8080
sys_activity: ['id', 'type', 'timestamp', 'summary', 'actor_id', 'object_name', 'record_id', 'record_label', 'metadata'],
8181
};
8282

@@ -126,6 +126,55 @@ describe('audit writers — organization_id stamping (#1532)', () => {
126126
});
127127
});
128128

129+
describe('audit writers — actor attribution (ADR-0014 D2, cloud#340)', () => {
130+
it('records a real user id on actor + user_id', async () => {
131+
const { engine, fire, created } = makeEngine(SINGLE_TENANT);
132+
installAuditWriters(engine as any, 'test.audit');
133+
await fire('afterInsert', {
134+
object: 'crm_lead',
135+
input: { id: 'lead-1' },
136+
result: { id: 'lead-1', name: 'Acme' },
137+
session: { userId: 'user-7' },
138+
});
139+
const audit = created.find((c) => c.object === 'sys_audit_log');
140+
expect(audit?.row.user_id).toBe('user-7');
141+
expect(audit?.row.actor).toBe('user-7');
142+
});
143+
144+
it('attributes a service-token write (no userId) via session.actor → actor, user_id stays null', async () => {
145+
const { engine, fire, created } = makeEngine(SINGLE_TENANT);
146+
installAuditWriters(engine as any, 'test.audit');
147+
// The os-790m7q class: a service-token delete with no real user.
148+
await fire('afterDelete', {
149+
object: 'sys_environment',
150+
input: { id: 'os-790m7q' },
151+
__previous: { id: 'os-790m7q', name: 'test' },
152+
result: { id: 'os-790m7q' },
153+
session: { actor: 'svc:cloud-control' },
154+
});
155+
const audit = created.find((c) => c.object === 'sys_audit_log');
156+
expect(audit?.row.action).toBe('delete');
157+
// user_id (sys_user lookup) stays null — a service principal isn't a user…
158+
expect(audit?.row.user_id).toBeNull();
159+
// …but the action is now ATTRIBUTABLE on actor.
160+
expect(audit?.row.actor).toBe('svc:cloud-control');
161+
});
162+
163+
it('leaves actor null when neither a user nor a service principal is present', async () => {
164+
const { engine, fire, created } = makeEngine(SINGLE_TENANT);
165+
installAuditWriters(engine as any, 'test.audit');
166+
await fire('afterInsert', {
167+
object: 'crm_lead',
168+
input: { id: 'lead-2' },
169+
result: { id: 'lead-2', name: 'Beta' },
170+
session: {},
171+
});
172+
const audit = created.find((c) => c.object === 'sys_audit_log');
173+
expect(audit?.row.actor).toBeNull();
174+
expect(audit?.row.user_id).toBeNull();
175+
});
176+
});
177+
129178
describe('audit writers — declarative trackHistory activity (ADR-0052 §5b)', () => {
130179
// crm_opportunity with a tracked select field (Stage) carrying option labels.
131180
const SCHEMA = {

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,14 @@ export function installAuditWriters(
352352

353353
const sess: any = (ctx as any).session ?? {};
354354
const userId: string | undefined = sess.userId;
355+
// Principal label for attribution. Prefer the real user id; otherwise fall
356+
// back to a service/automation principal the host put on the context
357+
// (`ExecutionContext.actor`, e.g. `svc:<name>`). This is what makes a
358+
// non-user-authenticated write attributable instead of a null actor — the
359+
// os-790m7q env-delete class (ADR-0014 D2). `user_id` stays user-only (it's
360+
// a strict sys_user lookup); the service principal lands on `actor`.
361+
const actorLabel: string | null =
362+
userId ?? (typeof sess.actor === 'string' && sess.actor.trim() ? sess.actor.trim() : null);
355363
// Prefer the active session tenant, but fall back to the audited
356364
// record's own `organization_id`. This matters in two cases:
357365
// 1. Background jobs / unauthenticated sudo paths where the
@@ -406,6 +414,11 @@ export function installAuditWriters(
406414
if (objectHasField('sys_audit_log', 'organization_id')) {
407415
auditRow.organization_id = tenantId ?? null;
408416
}
417+
// First-class principal label (ADR-0014 D2). Conditionally stamped — same
418+
// rationale as organization_id: older audit tables predate the column.
419+
if (objectHasField('sys_audit_log', 'actor')) {
420+
auditRow.actor = actorLabel;
421+
}
409422

410423
const label = recordLabel(after ?? before, recordId ?? '');
411424
let summary: string;

packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,26 @@ export const SysAuditLog = ObjectSchema.create({
100100
),
101101

102102
user_id: Field.lookup('sys_user', {
103+
label: 'User',
104+
required: false,
105+
readonly: true,
106+
searchable: true,
107+
description: 'User who performed the action (null for non-user / service actions — see actor)',
108+
group: 'Event',
109+
}),
110+
111+
// First-class principal label, independent of the sys_user lookup. Records
112+
// WHO acted even when there is no real user row: a user id, a service-token
113+
// principal (`svc:<name>`), or null/'system'. `user_id` stays a strict
114+
// sys_user lookup (a service principal can't be stuffed there), so this is
115+
// the field that makes service-token writes attributable (ADR-0014 D2).
116+
actor: Field.text({
103117
label: 'Actor',
104118
required: false,
105119
readonly: true,
106120
searchable: true,
107-
description: 'User who performed the action (null for system actions)',
121+
maxLength: 255,
122+
description: 'Principal that performed the action: a user id, svc:<name>, or null',
108123
group: 'Event',
109124
}),
110125

packages/spec/src/kernel/execution-context.zod.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,17 @@ export const ExecutionContextSchema = lazySchema(() => z.object({
2222
/** Current user ID (resolved from session) */
2323
userId: z.string().optional(),
2424

25+
/**
26+
* Stable principal label for AUDIT ATTRIBUTION when the operation is not a
27+
* real user — e.g. a service token (`svc:<name>`) or an automation. The audit
28+
* writer records this on `sys_audit_log.actor` so a non-user-authenticated
29+
* write (the class that left `user_id` null and made the os-790m7q env-delete
30+
* unattributable) is still attributable. `userId` takes precedence when set;
31+
* this is the fallback. The runtime/host sets it (e.g. the control plane's
32+
* service-mode auth) — the framework only defines + records the contract.
33+
*/
34+
actor: z.string().optional(),
35+
2536
/**
2637
* Current user's unique email (resolved from session, falling back to a
2738
* `sys_user` lookup). Exposed to RLS as `current_user.email` for seedable,

0 commit comments

Comments
 (0)