Skip to content

Commit da5d1b4

Browse files
baozhoutaoclaude
andauthored
fix(runtime): ctx.user.name carries the display name on all three dispatch paths (#5372) (#5518)
The REST /actions dispatcher hardcoded `name: ec.userId` — a declared key delivering a plausible WRONG value, undetectable by any consumer-side fallback. The MCP and AI-route dispatchers read `ec.userName` / `ec.userDisplayName`, neither declared on ExecutionContextSchema nor ever assigned, so their `??` chains also landed on the id; the AI route additionally read `ec.userEmail` (declared field: `ec.email`), leaving `user.email` permanently undefined. One shared producer (security/actor-user.ts) now builds the envelope for all three paths plus the AI routes' second producer. `name` comes from `sys_user.name`, resolved once per request (memo keyed on the ExecutionContext; ~0.22ms per cold read against real SQLite) and falling back to the id quietly — so `name === id` means exactly "no display name", which is what lets an app-side workaround self-retire. [ADR-0068 D1] The identity core is built through the spec's own `createEvalUser`, the same factory the predicate surface mounts under `ctx.user`, so a body and the predicate beside it see one shape. The transport keys (`userId`, `displayName`, `roles`, `permissions`, `systemPermissions`) sit on top; nothing was removed. Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh Co-authored-by: Claude <noreply@anthropic.com>
1 parent 308c709 commit da5d1b4

8 files changed

Lines changed: 721 additions & 45 deletions

File tree

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): `ctx.user.name` is the acting user's display name, on every dispatch path (#5372)
6+
7+
An action body reading `ctx.user.name` got the raw user id back — a *declared*
8+
key delivering a plausible **wrong value**, which is the failure mode
9+
"declared = enforced" exists to prevent. Nothing downstream can detect it: the
10+
value is a perfectly good string, so no `??` chain and no consumer-side guard
11+
tells it apart from a real name. Apps that trusted the declaration wrote opaque
12+
ids into user-facing surfaces (an activity timeline rendering
13+
`usr_01j…` as its actor for every logged activity).
14+
15+
Three dispatchers built the caller's `user` object three different ways, and
16+
all three landed on the id:
17+
18+
- **REST `/actions`** hardcoded `name: ec.userId`.
19+
- **MCP `run_action`** read `ec.userName ?? ec.userDisplayName ?? ec.userId`.
20+
Neither alias is declared on `ExecutionContextSchema` and nothing ever
21+
assigned either, so the chain's only reachable arm was the id.
22+
- **The AI routes** spelled the key `displayName` (same dead chain behind it)
23+
and read the caller's address off `ec.userEmail` — the declared field is
24+
`ec.email` — so `req.user.email` there was permanently `undefined`.
25+
26+
**What changes.** One shared producer builds the user envelope for all three
27+
paths. `name` now carries `sys_user.name`, the platform's own profile
28+
display-name column, resolved once per request (a memo keyed on the request's
29+
ExecutionContext, so N action dispatches in one request cost one indexed read
30+
~0.22 ms measured against real SQLite — and nothing is cached across
31+
requests, so a rename takes effect on the user's next request).
32+
33+
Resolution is **quiet**: no `sys_user` row, no engine, a failing read or a blank
34+
name falls back to the id. A missing display name never fails an action. So
35+
`name === id` now means exactly one thing — *this user has no resolvable display
36+
name* — which is what makes the fix detectable from application code: any
37+
workaround of the form "if `ctx.user.name` differs from `ctx.user.id`, trust
38+
it; otherwise look the name up myself" **self-retires** the moment this lands,
39+
with no coordinated deploy.
40+
41+
**One shape, and it is the spec's.** [ADR-0068 D1] declares `EvalUser` as the
42+
one user-context contract, mounted under `current_user` / `user` / `ctx.user`
43+
on the predicate surface — with `name` on it, meaning "display name". The
44+
dispatch envelope's identity core is now built through that same
45+
`createEvalUser` factory, so an action's `visible` predicate and its `body`
46+
both spelled `ctx.user` — see one object: `id`, `name`, `email`, `positions`,
47+
`isPlatformAdmin`, `organizationId`. On top of that core the dispatch surfaces
48+
keep publishing what they already published: `userId` and `displayName`
49+
(aliases of `id` / `name`, same values), `roles` (the pre-ADR-0090 alias of
50+
`positions`), and the two authority channels `permissions` (permission-set
51+
names) and `systemPermissions` (capabilities), still side by side and never
52+
merged. Additive for every existing reader; no key was removed.
53+
54+
The AI routes' second `req.user` producer (the concrete per-route mounts) is
55+
built by the same function, so the two can no longer drift apart by hand. Its
56+
display name comes from the session's own `user.name`, needing no extra read;
57+
its former `?? user.email` middle arm is gone so that `name === id` means the
58+
same thing on every producer — the address is still served under `email`.
59+
60+
`buildActionSandboxContext` is unchanged: it passed the user through verbatim
61+
all along, and was never where the name was lost.
Lines changed: 314 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,314 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5372] `ctx.user.name` is the acting user's DISPLAY NAME, on every path.
5+
*
6+
* The defect these tests pin is not a missing key — it is a declared key
7+
* delivering a plausible WRONG value, which is strictly worse: `ctx.user.name`
8+
* read as a perfectly good string, so no consumer-side `??` could tell that it
9+
* was the raw user id, and app code that trusted the declaration wrote opaque
10+
* ids into user-facing surfaces (objectstack-ai/hotcrm#673's activity
11+
* timeline).
12+
*
13+
* Three dispatchers built the user object three different ways:
14+
*
15+
* - REST `/actions` hardcoded `name: ec.userId`;
16+
* - MCP `run_action` read `ec.userName ?? ec.userDisplayName ?? ec.userId`
17+
* — neither alias is declared on `ExecutionContextSchema` and nothing in
18+
* the repo ever assigned either, so the only reachable arm was the id;
19+
* - the AI routes spelled the key `displayName` (same dead chain) and read
20+
* the caller's address off `ec.userEmail`, which is not the declared field
21+
* (`ec.email`), so `user.email` there was permanently `undefined`.
22+
*
23+
* So the assertions come in three families:
24+
* 1. the VALUE — `name` is `sys_user.name`, and `name === id` happens if and
25+
* only if the user has no resolvable display name (both directions);
26+
* 2. the SHAPE — all three paths, plus the AI routes' second producer, emit
27+
* ONE key set, so a body/handler never branches on which door it came in;
28+
* 3. the FAILURE MODE — a name that cannot be resolved is quiet: the
29+
* dispatch still succeeds and `name` falls back to the id. A display name
30+
* is not worth failing an action over.
31+
*/
32+
33+
import { describe, it, expect, vi } from 'vitest';
34+
35+
import { HttpDispatcher } from './http-dispatcher.js';
36+
import { invokeBusinessAction } from './action-execution.js';
37+
import { handleAIRequest } from './domains/ai.js';
38+
import { actionBodyRunnerFactory } from './sandbox/body-runner.js';
39+
import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js';
40+
import type { DomainHandlerDeps } from './domain-handler-registry.js';
41+
import type { HttpProtocolContext } from './http-dispatcher.js';
42+
43+
const ACTION = {
44+
name: 'close_case',
45+
label: 'Close',
46+
objectName: 'crm_case',
47+
type: 'script',
48+
target: 'closeCase',
49+
ai: { exposed: true, description: 'Close a case.' },
50+
};
51+
const OBJECT_DEF = { name: 'crm_case', actions: [ACTION] };
52+
53+
/** The acting principal, as `resolveExecutionContext` actually builds one. */
54+
function makeEc(overrides: Record<string, unknown> = {}) {
55+
return {
56+
userId: 'usr_admin',
57+
email: 'admin@objectos.ai',
58+
tenantId: 'org_1',
59+
positions: ['platform_admin'],
60+
permissions: ['admin_full_access'],
61+
systemPermissions: ['manage_metadata'],
62+
...overrides,
63+
};
64+
}
65+
66+
/**
67+
* An engine whose `sys_user` read answers with `row`. `undefined` = the row is
68+
* not there at all (a service principal, a deleted account); `throws: true` =
69+
* the read itself fails.
70+
*/
71+
function makeQl(row: Record<string, unknown> | undefined, opts: { throws?: boolean } = {}) {
72+
const executeAction = vi.fn(async () => ({ ok: true }));
73+
const userReads: any[] = [];
74+
const schemaOf = (n: string) => (n === OBJECT_DEF.name ? OBJECT_DEF : undefined);
75+
const ql: any = {
76+
executeAction,
77+
userReads,
78+
getSchema: schemaOf,
79+
registry: { getObject: schemaOf, getItem: () => undefined },
80+
find: vi.fn(async (object: string, options?: any) => {
81+
if (object === 'sys_user') {
82+
userReads.push(options);
83+
if (opts.throws) throw new Error('sys_user unavailable');
84+
return row ? [row] : [];
85+
}
86+
return [{ id: 'case_1', status: 'open' }];
87+
}),
88+
insert: vi.fn(), update: vi.fn(), delete: vi.fn(),
89+
};
90+
return ql;
91+
}
92+
93+
/** REST — `POST /actions/crm_case/close_case/case_1`. Returns the body ctx. */
94+
async function dispatchRest(ec: any, ql: any, context?: HttpProtocolContext) {
95+
const kernel: any = {
96+
context: { getService: (n: string) => (n === 'objectql' || n === 'data' ? ql : null) },
97+
};
98+
const ctx = context ?? ({ request: {}, environmentId: 'platform', executionContext: ec } as any);
99+
const res: any = await new HttpDispatcher(kernel).handleActions(
100+
'/crm_case/close_case/case_1', 'POST', {}, ctx,
101+
);
102+
return { response: res.response, actionCtx: ql.executeAction.mock.calls[0]?.[2] };
103+
}
104+
105+
/** MCP — `run_action`. Returns the body ctx. */
106+
async function dispatchMcp(ec: any, ql: any) {
107+
const deps: any = { resolveService: async () => null, getObjectQL: async () => ql };
108+
await invokeBusinessAction(deps, { request: {} } as any, 'close_case', { recordId: 'case_1' }, {
109+
driver: undefined,
110+
envId: 'platform',
111+
ec,
112+
getMeta: () => ({ listObjects: async () => [OBJECT_DEF] }),
113+
callData: async () => ({ record: { id: 'case_1' } }),
114+
});
115+
return { actionCtx: ql.executeAction.mock.calls[0]?.[2] };
116+
}
117+
118+
const AI_ROUTE = '/api/v1/ai/tools/:toolName/execute';
119+
120+
/** AI route — `POST /ai/tools/create_object/execute`. Returns the handler's `req.user`. */
121+
async function dispatchAi(ec: any, ql: any) {
122+
const seen: { req?: any } = {};
123+
const deps = {
124+
resolveService: (async (_c: any, name: string) => (name === 'ai' ? { chat: async () => ({}) } : undefined)) as any,
125+
getObjectQL: async () => ql,
126+
getRegisteredAiRoutes: () => [{
127+
method: 'POST', path: AI_ROUTE, auth: true,
128+
handler: async (req: any) => { seen.req = req; return { status: 200, body: { success: true, data: {} } }; },
129+
}],
130+
success: (data: any) => ({ status: 200, body: { success: true, data } }),
131+
error: (message: string, httpStatus = 500) => ({ status: httpStatus, body: { success: false, error: { message } } }),
132+
routeNotFound: (route: string) => ({ status: 404, body: { success: false, error: { route } } }),
133+
} as unknown as DomainHandlerDeps;
134+
await handleAIRequest(
135+
deps, '/ai/tools/create_object/execute', 'POST', {}, {},
136+
{ executionContext: ec } as unknown as HttpProtocolContext,
137+
);
138+
return seen.req?.user;
139+
}
140+
141+
const DEV_ADMIN = { id: 'usr_admin', name: 'Dev Admin', email: 'admin@objectos.ai' };
142+
143+
describe('#5372 — the VALUE: ctx.user.name is sys_user.name, not the id', () => {
144+
it('REST /actions — the path that was hardcoded to the id', async () => {
145+
const { actionCtx } = await dispatchRest(makeEc(), makeQl(DEV_ADMIN));
146+
147+
expect(actionCtx.user.name).toBe('Dev Admin');
148+
expect(actionCtx.user.name).not.toBe(actionCtx.user.id);
149+
expect(actionCtx.user.id).toBe('usr_admin');
150+
// The alias carries the SAME value — one name, two spellings, never two
151+
// different answers.
152+
expect(actionCtx.user.displayName).toBe('Dev Admin');
153+
});
154+
155+
it('MCP run_action — same value through the other dispatcher', async () => {
156+
const { actionCtx } = await dispatchMcp(makeEc(), makeQl(DEV_ADMIN));
157+
158+
expect(actionCtx.user.name).toBe('Dev Admin');
159+
expect(actionCtx.user.displayName).toBe('Dev Admin');
160+
expect(actionCtx.user.id).toBe('usr_admin');
161+
});
162+
163+
it('AI route req.user — same value again', async () => {
164+
const user = await dispatchAi(makeEc(), makeQl(DEV_ADMIN));
165+
166+
expect(user.name).toBe('Dev Admin');
167+
expect(user.displayName).toBe('Dev Admin');
168+
// `email` used to read `ec.userEmail`, a field ExecutionContext does
169+
// not declare — permanently undefined. It reads the declared one now.
170+
expect(user.email).toBe('admin@objectos.ai');
171+
});
172+
173+
it('a real sandboxed body reads it — end to end, dispatcher → VM', async () => {
174+
// `buildActionSandboxContext` was never where the name was lost (it
175+
// passes `actionCtx.user` through verbatim), so this closes the loop
176+
// on the OTHER end: what an author actually writes in a body.
177+
const ql = makeQl(DEV_ADMIN);
178+
const { actionCtx } = await dispatchRest(makeEc(), ql);
179+
const fn = actionBodyRunnerFactory(new QuickJSScriptRunner(), { ql, appId: 'crm' })({
180+
name: 'close_case',
181+
object: 'crm_case',
182+
type: 'script',
183+
body: { language: 'js', source: 'return ctx.user.name;', capabilities: [] },
184+
} as any);
185+
186+
await expect(fn!(actionCtx)).resolves.toBe('Dev Admin');
187+
}, 60_000);
188+
});
189+
190+
describe('#5372 — the VALUE, other direction: name === id iff there is no display name', () => {
191+
it('a sys_user row with no name falls back to the id', async () => {
192+
const { actionCtx } = await dispatchRest(makeEc(), makeQl({ id: 'usr_admin', email: 'a@b.c' }));
193+
194+
expect(actionCtx.user.name).toBe('usr_admin');
195+
expect(actionCtx.user.name).toBe(actionCtx.user.id);
196+
});
197+
198+
it('a blank/whitespace name is no display name', async () => {
199+
const { actionCtx } = await dispatchRest(makeEc(), makeQl({ id: 'usr_admin', name: ' ' }));
200+
201+
expect(actionCtx.user.name).toBe('usr_admin');
202+
});
203+
204+
it('no sys_user row at all (a principal with no profile) falls back to the id', async () => {
205+
const { actionCtx } = await dispatchRest(makeEc(), makeQl(undefined));
206+
207+
expect(actionCtx.user.name).toBe('usr_admin');
208+
});
209+
210+
it('an anonymous / self-invoked dispatch is the `system` principal, unchanged (#2701)', async () => {
211+
const { actionCtx } = await dispatchRest(undefined, makeQl(DEV_ADMIN));
212+
213+
expect(actionCtx.user.id).toBe('system');
214+
expect(actionCtx.user.name).toBe('system');
215+
// …and it still carries the empty authority arrays, so a body reads
216+
// "holds nothing" rather than needing a `?? []`.
217+
expect(actionCtx.user.permissions).toEqual([]);
218+
expect(actionCtx.user.systemPermissions).toEqual([]);
219+
});
220+
});
221+
222+
describe('#5372 — the FAILURE MODE: an unresolvable name is quiet', () => {
223+
it('a failing sys_user read falls back to the id and the action still runs', async () => {
224+
const ql = makeQl(DEV_ADMIN, { throws: true });
225+
const { response, actionCtx } = await dispatchRest(makeEc(), ql);
226+
227+
expect(response.status).toBe(200);
228+
expect(actionCtx.user.name).toBe('usr_admin');
229+
});
230+
231+
it('an engine with no `find` at all does not break the dispatch', async () => {
232+
const ql = makeQl(DEV_ADMIN);
233+
delete (ql as any).find;
234+
// The record pre-load needs `find` too, so this also proves the name
235+
// resolution is not what turns a degraded engine into a 500.
236+
const { response, actionCtx } = await dispatchRest(makeEc(), ql);
237+
238+
expect(response.status).toBe(200);
239+
expect(actionCtx.user.name).toBe('usr_admin');
240+
});
241+
242+
it('the read is system-elevated — resolving WHO the caller is cannot depend on their own grants', async () => {
243+
const ql = makeQl(DEV_ADMIN);
244+
await dispatchRest(makeEc(), ql);
245+
246+
expect(ql.userReads[0]).toMatchObject({
247+
where: { id: 'usr_admin' }, limit: 1, context: { isSystem: true },
248+
});
249+
});
250+
251+
it('resolves ONCE per request, however many actions the request dispatches', async () => {
252+
const ql = makeQl(DEV_ADMIN);
253+
const ec = makeEc();
254+
// One ExecutionContext object = one inbound request. The memo is keyed
255+
// on its identity, so nothing is cached across requests and a renamed
256+
// user is correct on their very next one.
257+
const context: any = { request: {}, environmentId: 'platform', executionContext: ec };
258+
await dispatchRest(ec, ql, context);
259+
await dispatchRest(ec, ql, context);
260+
261+
expect(ql.userReads.length).toBe(1);
262+
expect(ql.executeAction.mock.calls.length).toBe(2);
263+
});
264+
});
265+
266+
describe('#5372 — the SHAPE: one key set across every producer', () => {
267+
it('REST, MCP and the AI route agree key-for-key', async () => {
268+
const ec = makeEc();
269+
const rest = (await dispatchRest(ec, makeQl(DEV_ADMIN))).actionCtx.user;
270+
const mcp = (await dispatchMcp(makeEc(), makeQl(DEV_ADMIN))).actionCtx.user;
271+
const ai = await dispatchAi(makeEc(), makeQl(DEV_ADMIN));
272+
273+
const keys = (u: any) => Object.keys(u).sort();
274+
expect(keys(mcp)).toEqual(keys(rest));
275+
expect(keys(ai)).toEqual(keys(rest));
276+
// The EvalUser core (ADR-0068 D1: id/name/email/positions/
277+
// isPlatformAdmin/organizationId) plus the two transport channels and
278+
// the id/name aliases the dispatch surfaces already published.
279+
expect(keys(rest)).toEqual([
280+
'displayName', 'email', 'id', 'isPlatformAdmin', 'name', 'organizationId',
281+
'permissions', 'positions', 'roles', 'systemPermissions', 'userId',
282+
]);
283+
});
284+
285+
it('and value-for-value, for one and the same caller', async () => {
286+
const rest = (await dispatchRest(makeEc(), makeQl(DEV_ADMIN))).actionCtx.user;
287+
const mcp = (await dispatchMcp(makeEc(), makeQl(DEV_ADMIN))).actionCtx.user;
288+
const ai = await dispatchAi(makeEc(), makeQl(DEV_ADMIN));
289+
290+
expect(mcp).toEqual(rest);
291+
expect(ai).toEqual(rest);
292+
expect(rest).toEqual({
293+
id: 'usr_admin',
294+
userId: 'usr_admin',
295+
name: 'Dev Admin',
296+
displayName: 'Dev Admin',
297+
email: 'admin@objectos.ai',
298+
positions: ['platform_admin'],
299+
roles: ['platform_admin'],
300+
// Derived by `createEvalUser`, never stored — ADR-0068 D2.
301+
isPlatformAdmin: true,
302+
permissions: ['admin_full_access'],
303+
systemPermissions: ['manage_metadata'],
304+
organizationId: 'org_1',
305+
});
306+
});
307+
308+
it('the ADR-0090 position aliases stay in lockstep (`roles` is `positions`)', async () => {
309+
const { actionCtx } = await dispatchRest(makeEc({ positions: ['sales_rep'] }), makeQl(DEV_ADMIN));
310+
311+
expect(actionCtx.user.positions).toEqual(['sales_rep']);
312+
expect(actionCtx.user.roles).toEqual(actionCtx.user.positions);
313+
});
314+
});

0 commit comments

Comments
 (0)