Skip to content

Commit bdc8e70

Browse files
hotlongclaude
andauthored
fix(service-automation): stamp org/owner/creator on runAs:'system' create_record per ADR-0118 (#5494) (#6153)
resolveRunDataContext's system branch discarded the trigger's userId/ tenantId, and every platform stamp for the three columns keys on exactly what was discarded: created_by on the write context's userId (ObjectQL audit hook), owner_id on the acting user in the security middleware (whole chain short-circuits on isSystem), organization_id on the context tenantId (driver-level tenant machinery). Rows a user-triggered system sweep created were born with all three NULL - outside the org partition and untouchable even by the triggering member (#5494's admin-403). Fix, at the writer's seam (runAs is authorization posture, not identity, ADR-0073 D2; elevation is not anonymity, #3783): - the system branch now carries the trigger's userId + tenantId through, the same { ...caller, isSystem: true } envelope the action-body seam ships (the hotcrm#548-family fix): created_by/updated_by stamp again, audit rows credit the human, the driver fills organization_id on born rows, and record-change cascades keep the triggering identity; - create_record fills owner_id (fill-only, schema-guarded) for system runs with a known acting user - the anchor's platform stamp is behind the isSystem short-circuit, so the payload is the only channel; the value is the acting user, never a system identity (ADR-0118 D6, ADR-0073 D3), and flow-authored fields always win; - genuinely user-less runs (schedules) stamp nothing: no acting user exists, and ADR-0118 D1 bans sentinel/pseudo-user stand-ins; the svc:flow actor label + flowRunId remain the provenance channels. Acceptance replays the issue's step-3->5 flip against the real SecurityPlugin: the same member context is denied on the NULL-born row and admitted on the stamped row - row content, not caller, decides. Fixes #5494 Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD Co-authored-by: Claude <noreply@anthropic.com>
1 parent 36fc938 commit bdc8e70

10 files changed

Lines changed: 533 additions & 27 deletions
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
---
4+
5+
fix(service-automation): runAs:'system' 的 create_record 按 ADR-0118 染全三列——组织、属主、创建者禁 NULL (#5494)
6+
7+
修的是缺陷,不是新语义——契约是 ADR-0118(#4608)既有的:显式 `isSystem`、fail-closed、
8+
禁 NULL 歧义;`runAs` 声明的是授权姿态而非身份(ADR-0073 D2),提权不等于匿名。
9+
10+
根因:`resolveRunDataContext` 的 system 分支把触发上下文的 `userId` / `tenantId` 整个丢弃,
11+
而三列的平台盖章恰好全部键在被丢弃的信息上——`created_by` 键在写上下文的 `userId`
12+
(ObjectQL 审计钩子)、`owner_id` 键在安全中间件的 acting user(而整条中间件含盖章步骤在
13+
`isSystem` 上短路)、`organization_id` 键在上下文 `tenantId`(驱动层租户机制)。于是用户
14+
触发的 system 清扫流程建出的每一行三列全 NULL:落在组织分区之外(唯一索引跨 NULL 不生效、
15+
org 作用域查询看不见),也落在所有 owner/creator 作用域授权之外——issue 里"admin 都
16+
403"的由来。
17+
18+
修复(writer 侧,`packages/services/service-automation`):
19+
20+
- system 分支把触发身份原样带过去(`userId` + `tenantId`),与 action-body 缝的
21+
`{ ...caller, isSystem: true }` 信封(hotcrm#548 同族修复)同形:`isSystem` 独自决定
22+
授权(中间件在读到 `userId` 之前就短路),身份只驱动归因盖章(`created_by`/`updated_by`
23+
审计 actor)、驱动层的 `organization_id` 填充,以及下游 record-change 级联的触发身份;
24+
- `create_record` 对 system 运行补 `owner_id` 填充(fill-only、schema 存在才染):所有权锚
25+
的平台盖章在 `isSystem` 上被短路,payload 是唯一通道;染的是 acting user——与同一触发在
26+
`runAs:'user'` 下会得到的默认一致,不是把系统身份塞进 owner(ADR-0118 D6 / ADR-0073 D3);
27+
- 流程 `fields` 显式给值一律优先;真正无用户的运行(schedule)保持三列不染——没有 acting
28+
user 时按 ADR-0118 D1,哨兵串与伪用户都是被禁的替代品,`svc:flow:*` actor 标签 +
29+
`flowRunId` 继续承担溯源。
30+
31+
行为变化:`runAs:'system'` 且触发上下文带 org 的运行,其数据操作在驱动层按
32+
`(org = 触发 org OR org IS NULL)` 作用域——与 action-body 缝一致的姿态;schedule 触发的
33+
运行不带 org,行为不变。

packages/services/service-automation/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
"@objectstack/spec": "workspace:*"
2424
},
2525
"devDependencies": {
26+
"@objectstack/driver-sql": "workspace:*",
2627
"@objectstack/objectql": "workspace:*",
28+
"@objectstack/plugin-security": "workspace:*",
2729
"@types/node": "^26.1.2",
2830
"typescript": "^6.0.3",
2931
"vitest": "^4.1.10"

packages/services/service-automation/src/builtin/crud-nodes.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import type { AutomationEngine } from '../engine.js';
2020
import { interpolate, interpolateFilter, type VariableMap } from './template.js';
2121
import { refuseNode } from '../guard-refusal.js';
2222
import { parseNodeConfig } from './parse-config.js';
23-
import { resolveRunDataContext } from '../runtime-identity.js';
23+
import { resolveRunDataContext, stampSystemInsertOwner } from '../runtime-identity.js';
2424

2525
/**
2626
* A filter condition that an author WROTE but that interpolation erased
@@ -300,7 +300,17 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
300300
}
301301

302302
// #1888 — honor flow.runAs (system → RLS-bypassing; user → trigger user).
303+
// #5494 — a BORN row must not escape the platform stamps. The run
304+
// context now carries the trigger's user + org even under system
305+
// elevation (so the audit hook stamps `created_by` and the driver's
306+
// tenant machinery fills `organization_id`, exactly like a user-path
307+
// insert); the ownership anchor has no such engine-side channel for
308+
// system writes — the security middleware that stamps it
309+
// short-circuits on `isSystem` — so the writer fills it here.
310+
// Fill-only — flow-authored `fields` win. Policy + rationale live
311+
// beside `resolveRunDataContext` in runtime-identity.ts.
303312
const dataCtx = resolveRunDataContext(context);
313+
stampSystemInsertOwner(fields, dataCtx, data, objectName);
304314
try {
305315
// #3407 — symmetric with update_record. Today the engine's
306316
// insert path strips nothing (INSERT is readonly-exempt and

packages/services/service-automation/src/builtin/crud-runas.test.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,24 @@ describe('flow.runAs identity enforcement at the data layer (#1888)', () => {
8181
engine.registerFlow('sys', allOpsFlow('sys', 'system'));
8282

8383
// Triggered by a normal user — `runAs:'system'` must still elevate.
84-
const res = await engine.execute('sys', { userId: 'u1' });
84+
const res = await engine.execute('sys', { userId: 'u1', tenantId: 'org1' });
8585
expect(res.success).toBe(true);
8686

8787
expect(calls.map((c) => c.op).sort()).toEqual(['delete', 'find', 'findOne', 'insert', 'update']);
8888
for (const c of calls) {
8989
expect(c.ctx, `${c.op} got no context`).toBeTruthy();
9090
expect(c.ctx.isSystem, `${c.op} not elevated`).toBe(true);
91-
// An elevated run is NOT attributed to the triggering user…
92-
expect(c.ctx.userId).toBeUndefined();
93-
// …but it IS attributed to the flow, so audit rows never read
94-
// "Unknown user" (ADR-0014 D2, #4366).
91+
// #5494 — elevation is not anonymity: the triggering user and org are
92+
// CARRIED THROUGH (attribution: created_by/updated_by stamps, audit
93+
// actor, downstream record-change identity; the org drives the driver's
94+
// organization_id fill on born rows), while `isSystem` alone decides
95+
// authorization. Dropping them here is what inserted rows with all
96+
// three platform columns NULL — untouchable even by the triggering
97+
// member, and outside the org partition.
98+
expect(c.ctx.userId, `${c.op} lost the acting user (#5494)`).toBe('u1');
99+
expect(c.ctx.tenantId, `${c.op} lost the trigger org (#5494)`).toBe('org1');
100+
// …and it stays attributed to the flow as well, so audit rows name
101+
// WHICH automation wrote them (ADR-0014 D2, #4366; ADR-0118 D5).
95102
expect(c.ctx.actor, `${c.op} lost the service-principal label`).toBe('svc:flow:sys');
96103
}
97104
});
@@ -174,19 +181,22 @@ describe('flow.runAs identity enforcement at the data layer (#1888)', () => {
174181
edges: [{ id: 'e1', source: 'start', target: 'mk' }, { id: 'e2', source: 'mk', target: 'end' }],
175182
} as any);
176183

177-
// Trigger as a restricted user. If the engine ignored runAs, the insert would
178-
// carry that user's identity (or none) instead of the elevated principal.
184+
// Trigger as a restricted user. If the engine ignored runAs, the insert
185+
// would run under that user's AUTHORIZATION (isSystem false) instead of the
186+
// elevated principal. Since #5494 the user still rides the context — as
187+
// attribution, which grants nothing under the isSystem short-circuit — so
188+
// the regression tell is the `isSystem` flag, never the userId's absence.
179189
await engine.execute('reg', { userId: 'restricted' });
180190
const insert = calls.find((c) => c.op === 'insert');
181191
expect(insert?.ctx?.isSystem, 'runAs:system did not elevate the data op (#1888 regressed)').toBe(true);
182-
expect(insert?.ctx?.userId).not.toBe('restricted');
192+
expect(insert?.ctx?.userId, 'the acting user must ride the elevated context (#5494)').toBe('restricted');
183193
});
184194
});
185195

186196
describe('resolveRunDataContext (#1888 unit)', () => {
187-
it("maps runAs:'system' to an elevated context attributed to the flow (#4366)", () => {
197+
it("maps runAs:'system' to an elevated context attributed to the flow AND the acting user (#4366, #5494)", () => {
188198
expect(resolveRunDataContext({ runAs: 'system', userId: 'u1', flowName: 'mirror_status' })).toEqual({
189-
isSystem: true, actor: 'svc:flow:mirror_status', positions: [], permissions: [],
199+
isSystem: true, actor: 'svc:flow:mirror_status', userId: 'u1', positions: [], permissions: [],
190200
});
191201
});
192202

packages/services/service-automation/src/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ export type { AutomationServicePluginOptions } from './plugin.js';
7474
// outright (#3760). Exported for hosts building custom data nodes: call
7575
// `resolveRunDataContext` and let the error propagate, so a custom node inherits
7676
// the same posture as the built-ins instead of re-opening the fail-open.
77-
export { resolveRunDataContext, UnscopedRunDataAccessError } from './runtime-identity.js';
77+
export {
78+
resolveRunDataContext,
79+
stampSystemInsertOwner,
80+
UnscopedRunDataAccessError,
81+
} from './runtime-identity.js';
7882
export type { RunDataContext, RunIdentityContext, RunProvenanceContext } from './runtime-identity.js';
7983

8084
// Built-in node executors (ADR-0018). These are seeded by AutomationServicePlugin

packages/services/service-automation/src/record-lookup-expand.integration.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ describe('record-change lookup expansion (#3475)', () => {
109109

110110
const read = crud.find((c) => c.op === 'findOne' && c.obj === 'lead');
111111
expect(read!.ctx?.isSystem).toBe(true);
112-
expect(read!.ctx?.userId).toBeUndefined();
112+
// #5494 — the acting user rides the elevated context as attribution; what
113+
// makes this read ELEVATED is `isSystem` (the middleware short-circuits
114+
// before any gate reads `userId`), not the absence of a user.
115+
expect(read!.ctx?.userId).toBe('u1');
113116

114117
await kernel.shutdown();
115118
});

packages/services/service-automation/src/runas-grant-resolution.integration.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,13 @@ describe("AutomationServicePlugin bridges the runAs:'user' grant resolver (#3356
119119
await automation.execute('sys', { userId: 'u1', params: { noteId: 'n1' } });
120120
const update = crud.find((c) => c.op === 'update' && c.obj === 'runas_thing');
121121
expect(update!.ctx.isSystem).toBe(true);
122-
expect(update!.ctx.userId).toBeUndefined();
122+
// #5494 — the acting user rides the elevated context as ATTRIBUTION (it
123+
// drives the created_by/updated_by stamps and the audit actor; the
124+
// isSystem short-circuit precedes every gate that reads it). What proves
125+
// "the resolver is not consulted" is the untouched authz envelope:
126+
expect(update!.ctx.userId).toBe('u1');
127+
expect(update!.ctx.positions).toEqual([]);
128+
expect(update!.ctx.permissions).toEqual([]);
123129

124130
await kernel.shutdown();
125131
});

0 commit comments

Comments
 (0)