Skip to content

Commit 1c07e6a

Browse files
authored
fix(app-shell)!: the server-action URL identifies an action by name, not target (ADR-0110 D1) (#2970)
serverActionHandler posted `action.target || action.name` — the handler's registration KEY — to /api/v1/actions/:object/:action. The server resolves the declaration by NAME, so for a target-bound action it resolved none and silently skipped both the ADR-0066 D4 capability gate and the ADR-0104 param contract: a button correctly hidden from users without the capability posted to an endpoint that accepted anyone (objectstack#3935). target is a binding expression — a handler key here, a flow id for type:'flow', a URL for type:'url', interpolatable, and legally non-unique — so it cannot identify a declaration. The URL now carries action.name and the server derives the handler key from what it resolved. An action with no name is refused rather than falling back to target. apiHandler and flowHandler are unchanged: their target genuinely is the endpoint / flow id they dispatch on. Requires the framework half, objectstack#3958 (merged).
1 parent 0ded602 commit 1c07e6a

3 files changed

Lines changed: 80 additions & 6 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@object-ui/app-shell': major
3+
---
4+
5+
**[ADR-0110 D1] The server-action URL identifies an action by `name`, not `target`.**
6+
7+
`serverActionHandler` posted `action.target || action.name` — the handler's
8+
registration KEY — to `/api/v1/actions/:object/:action`. For a target-bound
9+
action (`{ name: 'complete_task', target: 'completeTask' }`) the server resolves
10+
the declaration by name, so posting the target meant it resolved **no**
11+
declaration and silently skipped both the ADR-0066 D4 capability gate and the
12+
ADR-0104 param contract: a Console button correctly hidden from users without
13+
the capability posted to an endpoint that accepted anyone (framework#3935).
14+
15+
`target` is a binding expression — a handler key here, a flow id for
16+
`type: 'flow'`, a URL for `type: 'url'`, `${param.X}`-interpolatable, and
17+
legitimately non-unique — so it can never identify a declaration. The URL now
18+
carries `action.name`, and the server derives the handler key from the
19+
declaration it resolves. An action with no `name` is refused rather than
20+
falling back to `target`.
21+
22+
`apiHandler` and `flowHandler` are unchanged: their `target` genuinely is the
23+
endpoint / flow id they dispatch on.
24+
25+
Requires a framework with the ADR-0110 handler-key rotation (protocol 17); the
26+
two ship in lockstep.

packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,45 @@ describe('useConsoleActionRuntime — authenticated handlers', () => {
282282
expect(res).toMatchObject({ success: true });
283283
});
284284

285+
// [ADR-0110 D1] The action URL identifies the action by `name`. It used to
286+
// post `target || name` — the handler's REGISTRATION KEY — so for a
287+
// target-bound action the server resolved no declaration and silently
288+
// skipped the ADR-0066 D4 capability gate and the ADR-0104 param contract
289+
// (framework#3935). `target` is a binding expression, not an identity.
290+
it('serverActionHandler posts the action NAME, not its target', async () => {
291+
authFetchSpy.mockResolvedValue({ ok: true, json: async () => ({ success: true, data: {} }) });
292+
const { result } = renderHook(() =>
293+
useConsoleActionRuntime({ dataSource: {}, objects: [], objectName: 'todo_task' }),
294+
);
295+
296+
await act(async () => {
297+
// app-todo's real shape — declarative name ≠ handler registration key.
298+
await result.current.serverActionHandler(
299+
{ type: 'script', name: 'complete_task', target: 'completeTask' } as any,
300+
{ selectedRecords: [{ id: 'task_1' }] } as any,
301+
);
302+
});
303+
304+
const url = String(authFetchSpy.mock.calls[0][0]);
305+
expect(url).toContain('/api/v1/actions/todo_task/complete_task');
306+
expect(url).not.toContain('completeTask');
307+
});
308+
309+
it('serverActionHandler refuses an action with no name rather than falling back to target', async () => {
310+
const { result } = renderHook(() =>
311+
useConsoleActionRuntime({ dataSource: {}, objects: [], objectName: 'todo_task' }),
312+
);
313+
314+
let res: any;
315+
await act(async () => {
316+
res = await result.current.serverActionHandler({ type: 'script', target: 'completeTask' } as any);
317+
});
318+
319+
expect(res).toMatchObject({ success: false });
320+
expect(String(res.error)).toMatch(/no name/i);
321+
expect(authFetchSpy).not.toHaveBeenCalled();
322+
});
323+
285324
it('serverActionHandler returns a failed action error WITHOUT toasting it (the ActionRunner owns the error toast — no double toast)', async () => {
286325
// A script action that throws (e.g. lead_apply_convert validation) returns
287326
// { success:false, error } from the server. The handler must NOT toast it —

packages/app-shell/src/hooks/useConsoleActionRuntime.tsx

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -494,9 +494,18 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons
494494
// `context` is the shared ActionRunner context (registered handlers are
495495
// invoked as `handler(action, runnerContext)`).
496496
const serverActionHandler = useCallback(async (action: ActionDef, context?: ActionContext): Promise<ActionResult> => {
497-
const targetName = action.target || action.name;
498-
if (!targetName) {
499-
return { success: false, error: 'No action target provided' };
497+
// [ADR-0110 D1] The URL identifies the action by its declarative `name`,
498+
// never by `target`. `target` is a BINDING EXPRESSION — the handler's
499+
// registration key here, but a URL / flow id / FormView name for other
500+
// types, `${param.X}`-interpolatable, and legitimately non-unique — so it
501+
// cannot identify a declaration. Posting it (the old `target || name`)
502+
// meant the server resolved NO declaration for a target-bound action and
503+
// silently skipped both the ADR-0066 D4 capability gate and the ADR-0104
504+
// param contract (#3935). The server derives the handler key from the
505+
// declaration it resolves by name.
506+
const actionName = action.name;
507+
if (!actionName) {
508+
return { success: false, error: 'Action has no name — a server action must declare one' };
500509
}
501510
const params = (action.params && !Array.isArray(action.params))
502511
? { ...(action.params as Record<string, unknown>) }
@@ -523,7 +532,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons
523532
}
524533

525534
// Re-entrancy guard.
526-
const inflightKey = `${targetName}:${resolvedRecordId ?? ''}`;
535+
const inflightKey = `${actionName}:${resolvedRecordId ?? ''}`;
527536
if (serverActionInFlight.current.has(inflightKey)) {
528537
return { success: false, error: 'Action already in progress' };
529538
}
@@ -580,7 +589,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons
580589
}
581590
const obj = action.objectName || objApiName || 'global';
582591
const res = await authFetch(
583-
`${baseUrl}/api/v1/actions/${encodeURIComponent(obj)}/${encodeURIComponent(targetName)}`,
592+
`${baseUrl}/api/v1/actions/${encodeURIComponent(obj)}/${encodeURIComponent(actionName)}`,
584593
{
585594
method: 'POST',
586595
headers: { 'Content-Type': 'application/json' },
@@ -591,7 +600,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons
591600
// Single source for the `/actions` envelope rule — shared with
592601
// RecordDetailView, whose copy of this handler drifted from it and caused
593602
// objectstack#3913's console symptom. See utils/actionResponse.
594-
const outcome = interpretActionResponse(res, json, `Action "${targetName}"`);
603+
const outcome = interpretActionResponse(res, json, `Action "${actionName}"`);
595604
if (!outcome.ok) {
596605
if (preOpenedTab) { try { preOpenedTab.close(); } catch { /* ignore */ } }
597606
// Don't toast here — the ActionRunner's post-execution hook surfaces

0 commit comments

Comments
 (0)