Skip to content

Commit 57b8fe0

Browse files
os-zhuangclaude
andauthored
fix(runtime): resolve session identity for action body ctx.user (#2701) (#2725)
* chore: bump objectui to 7a68d78f2a0c chore: release packages (#2304) objectui@7a68d78f2a0c3c1f99dafd39b75b4f117a24917b * fix(runtime): resolve session identity for action body ctx.user (#2701) The POST /actions/:object/:action route called handleActions directly, bypassing dispatch() → resolveExecutionContext. The action body sandbox's ctx.user was therefore hard-coded to { id: 'system' }, so handlers could not branch on the operator's identity/roles or enforce server-side ownership. - dispatcher-plugin: action routes now dispatch through dispatch() (like the automation/AI routes), resolving the session identity + per-project kernel per request before the action body runs. - http-dispatcher: handleActions builds ctx.user from the request's ExecutionContext (id, email, positions/roles, permissions, tenantId), matching the MCP runAction and record-change trigger paths; falls back to a system principal only for a genuinely anonymous / self-invoked call. - Tests: 4 regression cases incl. an end-to-end api-key → dispatch → action pipeline asserting the principal reaches ctx.user (verified red without the fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent bd39dc5 commit 57b8fe0

4 files changed

Lines changed: 166 additions & 14 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): action body `ctx.user` now reflects the session operator, not `system`
6+
7+
The `POST /actions/:object/:action` route called `handleActions` directly,
8+
bypassing `dispatcher.dispatch()` — so `resolveExecutionContext` never ran and
9+
the action handler's `ctx.user` was hard-coded to `{ id: 'system' }`. Handlers
10+
could not branch on the operator's identity or business roles, nor enforce
11+
server-side ownership. (#2701)
12+
13+
- The action routes now dispatch through `dispatch()` like the automation/AI
14+
routes, so the per-request pipeline resolves the session identity (and swaps
15+
to the per-project kernel) before the action body runs.
16+
- `handleActions` builds `ctx.user` from the resolved `ExecutionContext`,
17+
exposing `id`, `email`, `roles`/`positions` (ADR-0090 business roles),
18+
`permissions`, and `tenantId` — matching the MCP `runAction` and
19+
record-change trigger paths. It falls back to a `system` principal only for a
20+
genuinely anonymous / self-invoked call.
21+
22+
No authoring change is required: action handlers that previously always saw
23+
`ctx.user.id === 'system'` will now see the real caller.

packages/runtime/src/dispatcher-plugin.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -983,22 +983,25 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu
983983
// ── Actions (server-registered handlers, e.g. CRM convertLead) ───
984984
// Bridges UI `script` / `modal` actions to ObjectQL handlers
985985
// registered via `engine.registerAction(object, action, fn)`.
986+
// Dispatched through `dispatcher.dispatch()` (like automation/AI) so
987+
// the per-request pipeline resolves the session into `executionContext`
988+
// and swaps to the per-project kernel BEFORE the action body sandbox
989+
// runs — otherwise the handler's `ctx.user` sees no session and falls
990+
// back to `system` (#2701). The scoped-URL `:environmentId` rides on
991+
// `req.params` and is picked up by `prepareResolverHints`, exactly as
992+
// the automation routes handle it.
986993
const registerActionRoutes = (base: string) => {
987994
server!.post(`${base}/actions/:object/:action`, async (req: any, res: any) => {
988995
try {
989-
const ctx: any = { request: req };
990-
if (req.params?.environmentId) ctx.environmentId = req.params.environmentId;
991-
const result = await dispatcher.handleActions(`/${req.params.object}/${req.params.action}`, 'POST', req.body, ctx);
996+
const result = await dispatcher.dispatch('POST', `/actions/${req.params.object}/${req.params.action}`, req.body, req.query, { request: req });
992997
sendResult(result, res);
993998
} catch (err: any) {
994999
errorResponse(err, res);
9951000
}
9961001
});
9971002
server!.post(`${base}/actions/:object/:action/:recordId`, async (req: any, res: any) => {
9981003
try {
999-
const ctx: any = { request: req };
1000-
if (req.params?.environmentId) ctx.environmentId = req.params.environmentId;
1001-
const result = await dispatcher.handleActions(`/${req.params.object}/${req.params.action}/${req.params.recordId}`, 'POST', req.body, ctx);
1004+
const result = await dispatcher.dispatch('POST', `/actions/${req.params.object}/${req.params.action}/${req.params.recordId}`, req.body, req.query, { request: req });
10021005
sendResult(result, res);
10031006
} catch (err: any) {
10041007
errorResponse(err, res);

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2202,6 +2202,113 @@ describe('HttpDispatcher — ADR-0066 D4 action requiredPermissions gate', () =>
22022202
});
22032203
});
22042204

2205+
describe('HttpDispatcher — action body ctx.user identity (#2701)', () => {
2206+
// The action body sandbox must see the SESSION operator (id + business
2207+
// roles), resolved from the request's ExecutionContext — the same envelope
2208+
// `dispatch()` populates and that the MCP / record-change paths already read.
2209+
// Pre-#2701 the fallback chain read `_context.user` / `_context.userId`
2210+
// (fields HttpProtocolContext never carries) and hard-fell to `system`, so
2211+
// every action ran blind to who invoked it.
2212+
const captureCtx = (execCtx: any) => {
2213+
const executeAction = vi.fn(async () => ({ ok: true }));
2214+
const schemaOf = (name: string) => ({
2215+
name,
2216+
actions: [{ name: 'convert', label: 'Convert', type: 'script', execute: 'true' }],
2217+
});
2218+
const ql: any = {
2219+
executeAction,
2220+
getSchema: schemaOf,
2221+
registry: { getObject: schemaOf },
2222+
find: vi.fn().mockResolvedValue([]),
2223+
insert: vi.fn(), update: vi.fn(), delete: vi.fn(),
2224+
};
2225+
const kernel: any = { context: { getService: (n: string) => (n === 'objectql' ? ql : null) } };
2226+
const dispatcher = new HttpDispatcher(kernel);
2227+
const ctx: any = { request: {}, environmentId: 'platform', executionContext: execCtx };
2228+
return { dispatcher, executeAction, ctx };
2229+
};
2230+
2231+
const actionUser = (executeAction: any) => executeAction.mock.calls[0]?.[2]?.user;
2232+
2233+
it('forwards the session user id + business roles to the action body (not `system`)', async () => {
2234+
const { dispatcher, executeAction, ctx } = captureCtx({
2235+
userId: 'user_42',
2236+
positions: ['sales_rep', 'org_member'],
2237+
permissions: ['convert_lead'],
2238+
email: 'rep@acme.test',
2239+
tenantId: 'org_acme',
2240+
});
2241+
await dispatcher.handleActions('/lead/convert', 'POST', {}, ctx);
2242+
const user = actionUser(executeAction);
2243+
expect(user.id).toBe('user_42');
2244+
expect(user.roles).toEqual(['sales_rep', 'org_member']);
2245+
expect(user.positions).toEqual(['sales_rep', 'org_member']);
2246+
expect(user.permissions).toEqual(['convert_lead']);
2247+
expect(user.email).toBe('rep@acme.test');
2248+
expect(user.tenantId).toBe('org_acme');
2249+
});
2250+
2251+
it('falls back to a `system` principal only when the request is anonymous', async () => {
2252+
const { dispatcher, executeAction, ctx } = captureCtx(undefined);
2253+
await dispatcher.handleActions('/lead/convert', 'POST', {}, ctx);
2254+
const user = actionUser(executeAction);
2255+
expect(user.id).toBe('system');
2256+
expect(user.roles).toEqual([]);
2257+
expect(user.positions).toEqual([]);
2258+
});
2259+
2260+
it('sources identity from executionContext, ignoring a stray `_context.user` (regression guard)', async () => {
2261+
// HttpProtocolContext carries no `user`/`userId`; a caller must not be able
2262+
// to spoof identity by stuffing one on. The resolved session is the one source.
2263+
const { dispatcher, executeAction, ctx } = captureCtx({ userId: 'ec_user', positions: ['viewer'] });
2264+
(ctx as any).user = { id: 'spoofed' };
2265+
(ctx as any).userId = 'spoofed_2';
2266+
await dispatcher.handleActions('/lead/convert', 'POST', {}, ctx);
2267+
expect(actionUser(executeAction).id).toBe('ec_user');
2268+
});
2269+
2270+
it('resolves the session end-to-end: dispatch(/actions/…) threads the authenticated principal into ctx.user', async () => {
2271+
// Full pipeline: an api-key request → dispatch() → resolveExecutionContext →
2272+
// handleActions. This is the path registerActionRoutes now takes (it calls
2273+
// `dispatch('POST', '/actions/…')`) — the identity resolution that was
2274+
// bypassed pre-#2701, when the action route called handleActions directly.
2275+
const rows: any[] = [];
2276+
const executeAction = vi.fn(async () => ({ ok: true }));
2277+
const schemaOf = (name: string) => ({ name, actions: [{ name: 'convert', label: 'C', type: 'script', execute: 'true' }] });
2278+
const ql: any = {
2279+
executeAction,
2280+
getSchema: schemaOf,
2281+
registry: { getObject: schemaOf },
2282+
insert: async (_o: string, data: any) => { const id = `key_${rows.length + 1}`; rows.push({ id, ...data }); return { id }; },
2283+
find: async (obj: string, opts: any) => {
2284+
const where = opts?.where ?? {};
2285+
if (obj !== 'sys_api_key') return [];
2286+
return rows.filter((r) => Object.entries(where).every(([k, v]) => r[k] === v));
2287+
},
2288+
update: async () => ({}), delete: async () => ({}),
2289+
};
2290+
const kernel: any = {
2291+
getService: (n: string) => (n === 'objectql' ? ql : undefined),
2292+
getServiceAsync: async (n: string) => (n === 'objectql' ? ql : undefined),
2293+
context: { getService: (n: string) => (n === 'objectql' ? ql : undefined) },
2294+
};
2295+
const dispatcher = new HttpDispatcher(kernel, undefined, { enforceProjectMembership: false });
2296+
2297+
// Mint an api key bound to `user_9`, then invoke an action authenticated by it.
2298+
const mint = await dispatcher.handleKeys('POST', { name: 'agent' }, {
2299+
request: { headers: {} }, executionContext: { userId: 'user_9', positions: [], permissions: [] },
2300+
} as any);
2301+
const raw = mint.response.body.data.key;
2302+
2303+
await dispatcher.dispatch('POST', '/actions/lead/convert', {}, {}, {
2304+
request: { headers: { 'x-api-key': raw } },
2305+
} as any);
2306+
2307+
const user = executeAction.mock.calls[0]?.[2]?.user;
2308+
expect(user?.id).toBe('user_9'); // was `system` before the fix — the route bypassed dispatch()
2309+
});
2310+
});
2311+
22052312
describe('HttpDispatcher — MCP action bridge (list_actions / run_action)', () => {
22062313
// A `todo_task` object with declarative actions, mirroring examples/app-todo:
22072314
// - complete_task: script bound to the `completeTask` handler (row-context)

packages/runtime/src/http-dispatcher.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3385,12 +3385,14 @@ export class HttpDispatcher {
33853385
if (def?.environmentId) _context.environmentId = def.environmentId;
33863386
}
33873387

3388-
// Replicate the kernel swap that `dispatcher.handle()` does for
3389-
// data/meta/automation routes. Action routes are registered on the
3390-
// raw HTTP server and skip the `handle()` chain, so without this
3391-
// swap `getObjectQLService` would resolve the control-plane kernel
3392-
// (where the CRM bundle's actions are NOT registered). Routed via the
3393-
// host's KernelResolver (ADR-0006 Phase 5) — same seam as handle().
3388+
// Kernel-resolution fallback for the per-project kernel. HTTP action
3389+
// routes now flow through `dispatcher.dispatch()` (like data/meta/
3390+
// automation), which already swapped to the project kernel and resolved
3391+
// `executionContext` before reaching here — so on that path this block
3392+
// re-resolves idempotently (a no-op in single-kernel mode). Kept for
3393+
// DIRECT `handleActions` callers (unit tests / internal dispatch) so the
3394+
// call still lands on the kernel where the bundle's actions are
3395+
// registered, not the control-plane kernel (ADR-0006 Phase 5).
33943396
let projectQl: any = null;
33953397
if (this.kernelResolver && _context.environmentId && _context.environmentId !== 'platform') {
33963398
try {
@@ -3480,8 +3482,25 @@ export class HttpDispatcher {
34803482
},
34813483
};
34823484

3483-
const userIdFromAuth = (_context as any)?.user?.id ?? (_context as any)?.userId ?? 'system';
3484-
const userFromAuth = (_context as any)?.user ?? { id: userIdFromAuth, name: userIdFromAuth };
3485+
// Resolve the caller identity from the request's ExecutionContext — the
3486+
// single source `dispatch()` populates via `resolveExecutionContext`,
3487+
// the same envelope the MCP `runAction` and record-change trigger paths
3488+
// read. The action body sandbox receives the operator's id and business
3489+
// roles (ADR-0090 `positions`, formerly `roles`) so a handler can branch
3490+
// on identity and enforce ownership. Falls back to a `system` principal
3491+
// only for a genuinely anonymous / self-invoked call (#2701).
3492+
const ec: any = _context?.executionContext;
3493+
const userFromAuth = ec?.userId
3494+
? {
3495+
id: ec.userId,
3496+
name: ec.userId,
3497+
email: ec.email,
3498+
roles: Array.isArray(ec.positions) ? ec.positions : [],
3499+
positions: Array.isArray(ec.positions) ? ec.positions : [],
3500+
permissions: Array.isArray(ec.permissions) ? ec.permissions : [],
3501+
tenantId: ec.tenantId,
3502+
}
3503+
: { id: 'system', name: 'system', roles: [], positions: [], permissions: [] };
34853504

34863505
const actionContext: any = {
34873506
record,

0 commit comments

Comments
 (0)