Skip to content

Commit 475d428

Browse files
fix(metadata-protocol): a metadata save's audit row names the user who saved it (#27568)
Fixes #27539 Part of #27414 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:元数据保存(`/meta`、`/automation`、删除)写 `sys_audit_log` 时,审计行的 `user_id` / `actor` 现在记下保存人,不再是空。 - 为什么改:ADR-0073 D3 要求归属必须具体;此前管理员每次保存元数据,审计日志都显示"无人"。 - 风险与回滚:只改 `SysMetadataRepository.put` / `delete` 写入时带的上下文,权限判定仍按系统身份;回滚即还原本 PR 一个文件。 - 先看:`packages/metadata-protocol/src/sys-metadata-repository.ts` 的 `writeContext` 两处(put 与 delete)。 - 两分钟验证:`pnpm --filter @objectstack/metadata-protocol exec vitest run src/sys-metadata-repository.recorded-by.test.ts`,预期 15/15 通过。 - 需要你做的:无,席位落地。 ## What changes `SysMetadataRepository.put` and `delete` ran every `sys_metadata` / `sys_metadata_history` write under `{ ...ctx, isSystem: true }`. That context carries no `session.userId` and no `attributedUserId`, the only two sources the audit writer (`plugin-audit` `audit-writers.ts`) reads for `sys_audit_log.user_id`. So every metadata save's audit row named nobody, on every door, `/meta` included, even though the save's `actor` already reached `created_by` / `updated_by` / `recorded_by`. Both verbs now build one `writeContext` per transaction: `isSystem: true` as before, plus `attributedUserId: opts.actor` when the save names an actor. `attributedUserId` is the existing attribution channel (`ExecutionContext.attributedUserId`, copied by the engine into `HookContext.provenance.attributedUserId`). The write is still authorized as the system, and the actor is never placed in `userId`, so no caller-gating hook reads it as the caller. With no actor the key is absent, so engine self-writes and seeds stay unattributed, as before. Reads are unchanged. No new mechanism, no spec change. ## Tests - New pins in `sys-metadata-repository.recorded-by.test.ts` (the file's existing engine double now records each write verb's context; no new double): - actor case: create, update and delete with three different actors. Every write (`sys_metadata` insert/update/delete plus the history insert) carries `isSystem: true`, `attributedUserId` set to that write's actor, and no `userId`. - control: with no actor, all six writes carry `isSystem: true` and no `attributedUserId` key. - A real-store pin with the audit writer installed cannot live in this package. `@objectstack/metadata-protocol` has no driver, objectql or plugin-audit dependency, and objectql depends on this package, so importing it would create a cycle. The engine-to-audit-writer leg was measured end to end instead (Verification below). <details><summary>Package suite and typecheck at 9bc0b58</summary> `pnpm --filter @objectstack/metadata-protocol typecheck && pnpm --filter @objectstack/metadata-protocol test` under the verify lock: `VERDICT command-exit 0`, Test Files 279 passed, 4 skipped (283). Tests 21783 passed, 26 skipped. </details> <details><summary>Ablation (fix committed first, src import so no rebuild needed)</summary> `scripts/ablation-replace.mjs` removed the `attributedUserId` spread from `put`'s `writeContext`. Anchor 1 -> 0, blob 5b8de92 -> 3ee7c3a8b2eb. Result: 1 failed, 14 passed. The failure was the actor pin; the control stayed green. Restore: blob == HEAD 5b8de92, `git diff HEAD` empty. </details> ## Acceptance notes - The `/automation` doors pass no `actor` until #27538 lands. Before that PR, their audit rows stay null for the same reason their `created_by` does. - `pnpm check:system-context-census` passes with no register edit. The change spreads `isSystem: true` exactly as before, and the census does not count that as a new elevation read site. ## Verification <details><summary>Live, stock showcase, fresh DB, signed in as the seeded admin</summary> Branch build at this PR's head, plus #27538's one-file runtime change applied locally and not committed. I read `GET /api/v1/data/sys_audit_log` filtered to `object_name = sys_metadata`. Every door answered 200: - `PUT /api/v1/meta/flow/qa_meta_27539` gave a create row with `user_id` and `actor` set to the admin id. - `POST /api/v1/automation` gave a create row with `user_id` and `actor` set to the admin id. - `PUT /api/v1/automation/qa_auto_27539` gave an update row with `user_id` and `actor` set to the admin id. - `DELETE /api/v1/meta/flow/qa_meta_27539` gave a delete row with `user_id` and `actor` set to the admin id. Earlier run without #27538's change: the `/meta` create and delete rows held the admin id. The `POST /api/v1/automation` create row had `user_id` null and `actor` null. That door passes no actor yet, so this is the no-actor control. </details> <details><summary>Gates at 9bc0b58</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 60 families, and all 60 exited 0. Reconciliation `--ran`: "60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN". `test:repo` and repo-wide lint are left to CI. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_017xRBcScm27Q3R5mkYLPWdB)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 33ab543 commit 475d428

3 files changed

Lines changed: 95 additions & 8 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): a metadata save's audit row names the user who saved it
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A signed-in user's metadata save (`PUT /api/v1/meta/:type/:name`, the `/automation` definition doors, a delete) wrote its `sys_audit_log` row on `sys_metadata` with `user_id` and `actor` both null, although the save named its actor and the `sys_metadata` row's `created_by` / `updated_by` recorded it. `SysMetadataRepository.put` and `delete` run their writes as the system, so the audit writer saw no session user. They now pass the save's actor as `ExecutionContext.attributedUserId`, the existing attribution channel the audit writer reads into `user_id`. The write is still authorized as the system; the actor is never presented as the caller. A save with no actor (boot sync, seeds, engine self-writes) stays unattributed, as before.

‎packages/metadata-protocol/src/sys-metadata-repository.recorded-by.test.ts‎

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ interface Row { [k: string]: unknown }
3737
function makeFakeEngine() {
3838
const rows = new Map<string, Row>();
3939
const historyRows: Row[] = [];
40+
// [#27539] The context each WRITE verb was handed, in call order — what the
41+
// engine turns into the hook's `session` / `provenance`.
42+
const writes: Array<{ verb: string; table: string; context: Record<string, unknown> | undefined }> = [];
4043

4144
const keyOf = (w: Record<string, unknown>) =>
4245
`${String(w.type)}|${String(w.name)}|${String(w.organization_id ?? 'null')}|${String(w.state ?? 'active')}`;
@@ -60,6 +63,7 @@ function makeFakeEngine() {
6063
return {
6164
rows,
6265
historyRows,
66+
writes,
6367
async find(table: string, opts: { where: Record<string, unknown> }) {
6468
if (table === 'sys_metadata_history') return historyRows.filter((h) => matchesHistory(h, opts.where));
6569
return Array.from(rows.values()).filter((r) => {
@@ -74,7 +78,8 @@ function makeFakeEngine() {
7478
if (table === 'sys_metadata_history') return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null;
7579
return findRow(opts.where)?.row ?? null;
7680
},
77-
async insert(table: string, data: Record<string, unknown>) {
81+
async insert(table: string, data: Record<string, unknown>, opts?: { context?: Record<string, unknown> }) {
82+
writes.push({ verb: 'insert', table, context: opts?.context });
7883
if (table === 'sys_metadata_history') {
7984
const h: Row = { ...data };
8085
if (!h.id) h.id = `h_${historyRows.length + 1}`;
@@ -86,15 +91,17 @@ function makeFakeEngine() {
8691
rows.set(k, row);
8792
return { id: row.id as string };
8893
},
89-
async update(_t: string, data: Record<string, unknown>, opts: { where: Record<string, unknown> }) {
94+
async update(_t: string, data: Record<string, unknown>, opts: { where: Record<string, unknown>; context?: Record<string, unknown> }) {
9095
assertEngineUpdateDispatch(data, opts);
96+
writes.push({ verb: 'update', table: _t, context: opts.context });
9197
const found = findRow(opts.where);
9298
if (!found) throw new Error('not found');
9399
rows.set(found.key, { ...found.row, ...data });
94100
return { id: found.row.id as string };
95101
},
96-
async delete(_t: string, opts: { where: Record<string, unknown> }) {
102+
async delete(_t: string, opts: { where: Record<string, unknown>; context?: Record<string, unknown> }) {
97103
assertEngineDeleteDispatch(opts);
104+
writes.push({ verb: 'delete', table: _t, context: opts.context });
98105
const found = findRow(opts.where);
99106
if (!found) return { deleted: 0 };
100107
rows.delete(found.key);
@@ -260,3 +267,55 @@ describe('#23033 — the sys_metadata row records who created it and who last ch
260267
expect(drafts.map((d) => [d.name, d.updatedBy])).toEqual([['case_grid', 'usr_carol']]);
261268
});
262269
});
270+
271+
/**
272+
* [#27539, ADR-0073 D3] The save's actor reaches the AUDIT row too. Every
273+
* `sys_metadata` write runs under `isSystem` (the door already authorized it),
274+
* so the engine's hook carries no `session.userId`, and the audit writer's only
275+
* other channel for `sys_audit_log.user_id` is `provenance.attributedUserId`,
276+
* which the engine copies from `ExecutionContext.attributedUserId`. So the
277+
* repository hands every write verb of `put` and `delete` the actor on that
278+
* key, and never as `userId` (that would make every caller-gating hook read
279+
* the actor as the caller).
280+
*
281+
* This fake records the context it is handed, so these pins prove the
282+
* repository HANDS the engine the attribution. That the engine copies it into
283+
* `provenance` and the audit writer reads it into `user_id` / `actor` is the
284+
* engine's and the writer's own contract, measured end to end on the PR.
285+
*/
286+
describe('#27539 — put and delete credit the actor to the audit writer through attributedUserId', () => {
287+
let engine: ReturnType<typeof makeFakeEngine>;
288+
let repo: SysMetadataRepository;
289+
const ref = { org: 'system', type: 'view' as const, name: 'case_grid' };
290+
const contexts = () => engine.writes.map((w) => [w.verb, w.table, w.context?.isSystem, w.context?.attributedUserId, w.context?.userId]);
291+
292+
beforeEach(() => {
293+
engine = makeFakeEngine();
294+
repo = new SysMetadataRepository({ engine });
295+
});
296+
297+
it('create, update and delete with an actor credit that actor on every write, as the system', async () => {
298+
const first = await repo.put(ref, view('A'), { parentVersion: null, actor: 'usr_alice' });
299+
const second = await repo.put(ref, view('B'), { parentVersion: first.version, actor: 'usr_bob' });
300+
await repo.delete(ref, { parentVersion: second.version, actor: 'usr_carol' });
301+
expect(contexts()).toEqual([
302+
['insert', 'sys_metadata', true, 'usr_alice', undefined],
303+
['insert', 'sys_metadata_history', true, 'usr_alice', undefined],
304+
['update', 'sys_metadata', true, 'usr_bob', undefined],
305+
['insert', 'sys_metadata_history', true, 'usr_bob', undefined],
306+
['delete', 'sys_metadata', true, 'usr_carol', undefined],
307+
['insert', 'sys_metadata_history', true, 'usr_carol', undefined],
308+
]);
309+
});
310+
311+
it('CONTROL: with no actor the writes carry no attributedUserId key at all — unattributed, as before', async () => {
312+
const first = await repo.put(ref, view('A'), { parentVersion: null, actor: null });
313+
const second = await repo.put(ref, view('B'), { parentVersion: first.version, actor: null });
314+
await repo.delete(ref, { parentVersion: second.version, actor: null });
315+
expect(engine.writes).toHaveLength(6);
316+
for (const w of engine.writes) {
317+
expect(w.context?.isSystem).toBe(true);
318+
expect(w.context !== undefined && 'attributedUserId' in w.context).toBe(false);
319+
}
320+
});
321+
});

‎packages/metadata-protocol/src/sys-metadata-repository.ts‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,6 +1147,17 @@ export class SysMetadataRepository implements MetadataRepository {
11471147
const version = await this.nextItemVersion(ref, ctx);
11481148
// Per-org monotonic event log cursor.
11491149
const eventSeq = await this.nextEventSeq(ctx);
1150+
// [#27539, ADR-0073 D3] The context every write below runs under. The
1151+
// write is AUTHORIZED as the system (the door already authorized the
1152+
// save), so the actor rides as `attributedUserId` — the attribution
1153+
// channel the audit writer reads into `sys_audit_log.user_id` — and
1154+
// never as `userId`, which every caller-gating hook would read as the
1155+
// caller. No actor → no key: the write stays unattributed, as before.
1156+
const writeContext = {
1157+
...ctx,
1158+
isSystem: true,
1159+
...(opts.actor ? { attributedUserId: opts.actor } : {}),
1160+
};
11501161

11511162
const parentRowData: Record<string, unknown> = {
11521163
type: ref.type,
@@ -1203,14 +1214,14 @@ export class SysMetadataRepository implements MetadataRepository {
12031214
}
12041215
await this.engine.update('sys_metadata', parentRowData, {
12051216
where: { id: existingId },
1206-
context: { ...ctx, isSystem: true },
1217+
context: writeContext,
12071218
});
12081219
} else {
12091220
parentRowData.created_at = now;
12101221
// [#23033] Who created the row: the first write's actor, set on the
12111222
// insert only — an update never names `created_by`, so it keeps it.
12121223
parentRowData.created_by = opts.actor ?? null;
1213-
await this.engine.insert('sys_metadata', parentRowData, { context: { ...ctx, isSystem: true } });
1224+
await this.engine.insert('sys_metadata', parentRowData, { context: writeContext });
12141225
}
12151226

12161227
// Durable history append — same transaction, so the parent write
@@ -1235,7 +1246,7 @@ export class SysMetadataRepository implements MetadataRepository {
12351246
recorded_by: opts.actor ?? null,
12361247
recorded_at: now,
12371248
},
1238-
{ context: { ...ctx, isSystem: true } },
1249+
{ context: writeContext },
12391250
);
12401251

12411252
const item: MetadataItem = {
@@ -1356,9 +1367,16 @@ export class SysMetadataRepository implements MetadataRepository {
13561367
eventSeq = await this.nextEventSeq(ctx);
13571368
}
13581369

1370+
// [#27539, ADR-0073 D3] As `put`'s: authorized as the system, the
1371+
// actor credited through `attributedUserId`; no actor → no key.
1372+
const writeContext = {
1373+
...ctx,
1374+
isSystem: true,
1375+
...(opts.actor ? { attributedUserId: opts.actor } : {}),
1376+
};
13591377
await this.engine.delete('sys_metadata', {
13601378
where: { id: existingId },
1361-
context: { ...ctx, isSystem: true },
1379+
context: writeContext,
13621380
});
13631381

13641382
if (state === 'active') {
@@ -1383,7 +1401,7 @@ export class SysMetadataRepository implements MetadataRepository {
13831401
recorded_by: opts.actor ?? null,
13841402
recorded_at: now,
13851403
},
1386-
{ context: { ...ctx, isSystem: true } },
1404+
{ context: writeContext },
13871405
);
13881406
}
13891407

0 commit comments

Comments
 (0)