Skip to content

Commit 5aaa6fc

Browse files
baozhoutaoclaude
andauthored
fix(runtime): deny anonymous callers on /actions and /automation (#5519) (#5569)
The dispatcher's `/actions/*` and `/automation/*` routes carried no anonymity check, while `@objectstack/rest`'s `/data` and the dispatcher's own `/meta`, `/ai` and `/security` have denied anonymous callers 401 UNAUTHENTICATED since #3963 made that a platform promise (the `api.requireAuth` opt-out is a tombstone). Two registration paths, one gated. `/actions` was the expensive half: a script action's body runs with `isSystem: true` forced on (buildActionExecutionContext), so an unauthenticated POST bought an RLS/FLS-bypassing SYSTEM write. The only gate ahead of it was ADR-0066 D4's requiredPermissions, which allows every action declaring none. Measured on a real showcase boot before the fix, with no credential: POST /actions/showcase_task/showcase_mark_done/:id -> 200 {ok:true} POST /automation/showcase_reassign_wizard/trigger -> 200 {runId: run_...} GET /automation -> 200 (full inventory) DELETE /automation/showcase_inquiry_janitor -> 200 {deleted:true} ...while /data on the same process answered 401. Both domains now call the shared `shouldDenyAnonymous` decision before anything dispatches, in the same envelope every other seam returns. Finer authorization is unchanged below the floor. Internal dispatch paths never enter these HTTP handlers (MCP run_action, the declarative endpoint executor, engine-internal triggers), and `isSystem` contexts pass untouched. Tests: a handler-level file pinning the decision and a real-socket integration file pinning that the MOUNTED routes reach the gated handler. Collateral in existing suites was triaged per case: route-behaviour tests that were only incidentally anonymous gained a session; the four whose SUBJECT was the anonymous path were replaced rather than re-spelled, because their assertions would otherwise have read off a call that never happens. Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh Co-authored-by: Claude <noreply@anthropic.com>
1 parent ddc2527 commit 5aaa6fc

11 files changed

Lines changed: 886 additions & 59 deletions

.changeset/thick-pumas-judge.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
Deny anonymous callers on the `/actions` and `/automation` dispatch routes (#5519)
6+
7+
`@objectstack/rest`'s `/data` and the dispatcher's `/meta`, `/ai` and `/security`
8+
have answered an unauthenticated caller 401 `UNAUTHENTICATED` since #3963 made
9+
"anonymous access is always denied" a platform promise (the `api.requireAuth`
10+
opt-out is a tombstone). The dispatcher's own `/actions/*` and `/automation/*`
11+
routes — mounted by `dispatcher-plugin.ts` onto the host server, a different
12+
registration path from the REST one — carried no anonymity check at all.
13+
14+
`/actions` was the expensive half: a `script` action's body executes with
15+
`isSystem: true` forced on (`buildActionExecutionContext`), so an
16+
unauthenticated POST bought an RLS/FLS-bypassing SYSTEM write. The only gate
17+
ahead of it was ADR-0066 D4's `requiredPermissions`, which allows every action
18+
that declares none — i.e. most authored actions. On `/automation`, anonymous
19+
callers could trigger a flow run, list every flow, register one, and
20+
unregister one.
21+
22+
Both domains now call the shared `shouldDenyAnonymous` decision before anything
23+
dispatches, returning the same 401 envelope every other seam returns. Finer
24+
authorization is unchanged and still runs for callers who clear the floor —
25+
`requiredPermissions` (ADR-0066 D4), `ai.exposed`, the ADR-0104 param contract.
26+
27+
**What passes unchanged:** any authenticated caller (session, API key or OAuth
28+
principal), and internal `isSystem` contexts. CORS preflight (`OPTIONS`) is
29+
exempt as always. Internal dispatch paths never enter these HTTP handlers and
30+
are untouched — the MCP `run_action` bridge, the declarative endpoint executor
31+
(a `type: 'flow'` endpoint keeps its own `authRequired` gate, so an explicit
32+
`authRequired: false` endpoint stays public), and engine-internal record-change
33+
and schedule triggers.
34+
35+
**Behaviour change to expect:** an unauthenticated call that previously got 200
36+
(or 403 on a `requiredPermissions` action, or 405/501) now gets 401. If a
37+
deployment relied on unauthenticated action or flow invocation, the supported
38+
replacement is a declared endpoint with `authRequired: false`, a public-form
39+
grant, or a share-link token — never an anonymous `/actions` POST.

packages/runtime/src/action-body-identity.test.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,12 +217,26 @@ describe('#3914 — REST /actions dispatch binds ctx.api and ctx.engine', () =>
217217
});
218218
});
219219

220-
it('still elevates for an anonymous / self-invoked call', async () => {
221-
const { dispatcher, executeAction, ql, ctx } = makeDispatcher(undefined);
220+
it('still elevates for a SELF-INVOKED call — and the anonymous half is 401 now (#5519)', async () => {
221+
// REPLACED, not re-spelled. Driven with NO execution context this used
222+
// to be the "anonymous" case; #5519 puts the platform anonymous-deny
223+
// baseline in front of `/actions`, so an anonymous POST never reaches
224+
// the body and `executeAction.mock.calls[0]` would be `undefined` —
225+
// the assertions below would have been reading nothing.
226+
//
227+
// The elevation claim survives intact for the caller that can still
228+
// get here without a `userId`: a self-invoked `isSystem` context.
229+
const { dispatcher, executeAction, ql, ctx } = makeDispatcher({ isSystem: true });
222230
await dispatcher.handleActions('/crm_case/close_case', 'POST', {}, ctx);
223231
const actionCtx = executeAction.mock.calls[0]?.[2];
224232
await actionCtx.engine.update('crm_case', 'case_1', { status: 'closed' });
225233
expect(ql.writes.find((w: any) => w.op === 'update').context).toMatchObject({ isSystem: true });
234+
235+
// The anonymous door is shut — stated, not implied.
236+
const anon = makeDispatcher(undefined);
237+
const denied: any = await anon.dispatcher.handleActions('/crm_case/close_case', 'POST', {}, anon.ctx);
238+
expect(denied.response.status).toBe(401);
239+
expect(anon.executeAction).not.toHaveBeenCalled();
226240
});
227241
});
228242

packages/runtime/src/action-ctx-user-shape.test.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,16 @@ describe('#5372 — the VALUE, other direction: name === id iff there is no disp
207207
expect(actionCtx.user.name).toBe('usr_admin');
208208
});
209209

210-
it('an anonymous / self-invoked dispatch is the `system` principal, unchanged (#2701)', async () => {
211-
const { actionCtx } = await dispatchRest(undefined, makeQl(DEV_ADMIN));
210+
it('a SELF-INVOKED dispatch is the `system` principal, unchanged (#2701); anonymous is 401 (#5519)', async () => {
211+
// REPLACED, not re-spelled: driven with `undefined` this was the
212+
// ANONYMOUS shape, and #5519 denies that at the door — `executeAction`
213+
// is never called, so `actionCtx` would be `undefined` and every
214+
// assertion below would read off nothing.
215+
//
216+
// The `system`-principal shape #2701 pinned is still real for the
217+
// caller that reaches the body without a `userId`: the self-invoked
218+
// `isSystem` context.
219+
const { actionCtx } = await dispatchRest({ isSystem: true }, makeQl(DEV_ADMIN));
212220

213221
expect(actionCtx.user.id).toBe('system');
214222
expect(actionCtx.user.name).toBe('system');
@@ -217,6 +225,14 @@ describe('#5372 — the VALUE, other direction: name === id iff there is no disp
217225
expect(actionCtx.user.permissions).toEqual([]);
218226
expect(actionCtx.user.systemPermissions).toEqual([]);
219227
});
228+
229+
it('a genuinely ANONYMOUS dispatch never reaches the body at all (#5519)', async () => {
230+
const ql = makeQl(DEV_ADMIN);
231+
const { response } = await dispatchRest(undefined, ql);
232+
233+
expect(response.status).toBe(401);
234+
expect(ql.executeAction).not.toHaveBeenCalled();
235+
});
220236
});
221237

222238
describe('#5372 — the FAILURE MODE: an unresolvable name is quiet', () => {
Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #5519 — the `/actions` and `/automation` anonymous baseline, through a REAL
5+
* boot and over a REAL socket.
6+
*
7+
* `domains/anonymous-gate-actions-automation.test.ts` pins the DECISION (which
8+
* caller the handler denies, and that nothing dispatches behind it). This file
9+
* pins the WIRING, and the distinction is the whole reason it exists: the gate
10+
* lives in the domain handler, but the routes are mounted by
11+
* `dispatcher-plugin.ts` straight onto the host `IHttpServer` — a separate
12+
* registration path from the one `@objectstack/rest` uses for `/data`, and
13+
* precisely the seam whose divergence #5519 is about. A unit test that calls
14+
* `handleActions()` directly cannot tell you that the MOUNTED route reaches the
15+
* gated handler; only a socket can. AGENTS.md states the rule flatly: "who
16+
* serves this path" is a question about the composed, provisioned runtime —
17+
* boot it or do not claim an answer. #3913 is the standing proof, where
18+
* `POST /actions//:action` was correct in the domain and unreachable on the
19+
* wire for exactly this reason.
20+
*
21+
* The pre-fix behaviour these replace, measured on a real showcase boot:
22+
* POST /actions/showcase_task/showcase_mark_done/:id → 200 {ok:true}
23+
* POST /automation/showcase_reassign_wizard/trigger → 200 {runId: run_…}
24+
* GET /automation → 200 (full inventory)
25+
* DELETE /automation/showcase_inquiry_janitor → 200 {deleted:true}
26+
* …all with no credential of any kind, while `/data` on the same process
27+
* answered 401.
28+
*/
29+
30+
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
31+
import { LiteKernel, Plugin, PluginContext } from '@objectstack/core';
32+
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
33+
import type { IHttpServer } from '@objectstack/spec/contracts';
34+
35+
import { createDispatcherPlugin } from './dispatcher-plugin.js';
36+
37+
const SESSION_HEADER = 'x-test-session';
38+
39+
const executeAction = vi.fn(async () => ({ ok: true, wrote: 'system-elevated' }));
40+
const automationExecute = vi.fn(async () => ({ success: true, status: 'paused', runId: 'run_1' }));
41+
const unregisterFlow = vi.fn();
42+
const listFlows = vi.fn(async () => ['crm_escalation_flow']);
43+
44+
/** One `script` action, declared on the object and carrying NO `requiredPermissions`. */
45+
const scriptAction = {
46+
name: 'mark_primary',
47+
objectName: 'crm_contact',
48+
type: 'script',
49+
body: { language: 'js', source: 'return { ok: true };', capabilities: ['api.write'] },
50+
};
51+
const objectDef = { name: 'crm_contact', actions: [scriptAction] };
52+
53+
/**
54+
* `auth` slot in the shape `resolveExecutionContext` actually reads
55+
* (`authService.api.getSession({ headers })`). It answers a session only when
56+
* the request carries `x-test-session`, so ONE boot serves both the anonymous
57+
* and the authenticated case and the difference on the wire is a header —
58+
* which is exactly the difference the gate is supposed to key off.
59+
*/
60+
function servicesPlugin(): Plugin {
61+
return {
62+
name: 'com.objectstack.test.services-5519',
63+
version: '1.0.0',
64+
init: async (ctx: PluginContext) => {
65+
ctx.registerService('objectql', {
66+
executeAction,
67+
getSchema: (n: string) => (n === objectDef.name ? objectDef : undefined),
68+
registry: { getObject: (n: string) => (n === objectDef.name ? objectDef : undefined), getItem: () => undefined },
69+
find: async () => [],
70+
insert: async () => ({}), update: async () => ({}), delete: async () => ({}),
71+
});
72+
ctx.registerService('automation', {
73+
execute: automationExecute,
74+
unregisterFlow,
75+
listFlows,
76+
registerFlow: () => { /* unused */ },
77+
handlerReady: true,
78+
});
79+
ctx.registerService('auth', {
80+
api: {
81+
getSession: async ({ headers }: any) =>
82+
(headers?.get?.(SESSION_HEADER) ? { user: { id: 'u_socket' } } : undefined),
83+
},
84+
});
85+
},
86+
};
87+
}
88+
89+
async function boot() {
90+
const kernel = new LiteKernel();
91+
kernel.use(new HonoServerPlugin({ port: 0, cors: false }));
92+
kernel.use(servicesPlugin());
93+
kernel.use(createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }));
94+
await kernel.bootstrap();
95+
const httpServer = kernel.getService<IHttpServer>('http.server');
96+
return { kernel, baseUrl: `http://127.0.0.1:${httpServer.getPort!()}` };
97+
}
98+
99+
describe('#5519 — the mounted /actions and /automation routes deny anonymous callers on the wire', () => {
100+
let kernel: LiteKernel;
101+
let baseUrl: string;
102+
103+
beforeAll(async () => { ({ kernel, baseUrl } = await boot()); }, 60_000);
104+
afterAll(async () => {
105+
await Promise.race([kernel?.shutdown(), new Promise<void>((r) => setTimeout(r, 10_000))]);
106+
}, 30_000);
107+
108+
const post = (path: string, body: unknown, session = false) =>
109+
fetch(`${baseUrl}/api/v1${path}`, {
110+
method: 'POST',
111+
headers: { 'Content-Type': 'application/json', ...(session ? { [SESSION_HEADER]: '1' } : {}) },
112+
body: JSON.stringify(body),
113+
});
114+
115+
// ── /actions ────────────────────────────────────────────────────────────
116+
117+
it('anonymous POST /api/v1/actions/:object/:action/:recordId → 401, body never runs', async () => {
118+
executeAction.mockClear();
119+
const res = await post('/actions/crm_contact/mark_primary/c1', { params: {} });
120+
121+
expect(res.status).toBe(401);
122+
const body: any = await res.json();
123+
expect(body?.error?.code ?? body?.error?.details?.code).toBe('UNAUTHENTICATED');
124+
expect(body?.error?.message).toBe('Authentication is required to access this endpoint.');
125+
// The script body would have run system-elevated. It did not.
126+
expect(executeAction).not.toHaveBeenCalled();
127+
}, 60_000);
128+
129+
it('anonymous POST /api/v1/actions/:object/:action (no recordId) → 401', async () => {
130+
executeAction.mockClear();
131+
const res = await post('/actions/crm_contact/mark_primary', {});
132+
133+
expect(res.status).toBe(401);
134+
expect(executeAction).not.toHaveBeenCalled();
135+
}, 60_000);
136+
137+
it('the SAME request with a session is served — the deny targets anonymity, not the route', async () => {
138+
executeAction.mockClear();
139+
const res = await post('/actions/crm_contact/mark_primary/c1', { params: {} }, true);
140+
141+
expect(res.status).toBe(200);
142+
expect((await res.json())?.data).toMatchObject({ ok: true });
143+
expect(executeAction).toHaveBeenCalledTimes(1);
144+
}, 60_000);
145+
146+
// ── /automation ─────────────────────────────────────────────────────────
147+
148+
it('anonymous POST /api/v1/automation/:name/trigger → 401, no run started', async () => {
149+
automationExecute.mockClear();
150+
const res = await post('/automation/crm_escalation_flow/trigger', { recordId: 'c1' });
151+
152+
expect(res.status).toBe(401);
153+
expect(automationExecute).not.toHaveBeenCalled();
154+
}, 60_000);
155+
156+
it('anonymous POST /api/v1/automation/trigger/:name (the legacy SDK shape) → 401', async () => {
157+
automationExecute.mockClear();
158+
const res = await post('/automation/trigger/crm_escalation_flow', { recordId: 'c1' });
159+
160+
expect(res.status).toBe(401);
161+
expect(automationExecute).not.toHaveBeenCalled();
162+
}, 60_000);
163+
164+
it('anonymous GET /api/v1/automation → 401, the flow inventory stays private', async () => {
165+
listFlows.mockClear();
166+
const res = await fetch(`${baseUrl}/api/v1/automation`);
167+
168+
expect(res.status).toBe(401);
169+
expect(listFlows).not.toHaveBeenCalled();
170+
}, 60_000);
171+
172+
it('anonymous DELETE /api/v1/automation/:name → 401 — the destructive one', async () => {
173+
unregisterFlow.mockClear();
174+
const res = await fetch(`${baseUrl}/api/v1/automation/crm_escalation_flow`, { method: 'DELETE' });
175+
176+
expect(res.status).toBe(401);
177+
expect(unregisterFlow).not.toHaveBeenCalled();
178+
}, 60_000);
179+
180+
it('the same trigger with a session is served', async () => {
181+
automationExecute.mockClear();
182+
const res = await post('/automation/crm_escalation_flow/trigger', { recordId: 'c1' }, true);
183+
184+
expect(res.status).toBe(200);
185+
expect(automationExecute).toHaveBeenCalledTimes(1);
186+
// Identity forwarding survives the gate — a `runAs: 'user'` flow still
187+
// learns who triggered it (#4127).
188+
expect(automationExecute.mock.calls[0]?.[1]).toMatchObject({ userId: 'u_socket' });
189+
}, 60_000);
190+
191+
// ── one answer, one shape ───────────────────────────────────────────────
192+
193+
it('both newly-gated domains answer in the IDENTICAL envelope, byte for byte', async () => {
194+
// The cross-surface contrast that made this a p0 — `/data` answering
195+
// 401 while `/actions` answered 200 in the SAME process — is not
196+
// provable on this boot: `@objectstack/rest` owns `/data` and `/meta`
197+
// and the dispatcher plugin mounts neither, so there is no second
198+
// surface here to compare against. It was measured instead on a real
199+
// showcase boot (recorded in the PR body), and asserting a lookalike
200+
// here would be a weaker claim wearing the stronger one's clothes.
201+
//
202+
// What THIS boot can prove is the half that would actually regress
203+
// unnoticed: the two domains gated by this change share one envelope
204+
// and cannot drift into two dialects of "unauthenticated".
205+
const fromActions = await (await post('/actions/crm_contact/mark_primary/c1', {})).json();
206+
const fromAutomation = await (await post('/automation/crm_escalation_flow/trigger', {})).json();
207+
208+
expect(fromActions).toEqual(fromAutomation);
209+
expect((fromActions as any)?.error?.code ?? (fromActions as any)?.error?.details?.code).toBe('UNAUTHENTICATED');
210+
}, 60_000);
211+
});

packages/runtime/src/domain-handler-registry.test.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,9 +540,19 @@ describe('HttpDispatcher extracted domains (PR-5: packages)', () => {
540540
// ---------------------------------------------------------------------------
541541

542542
describe('HttpDispatcher extracted domains (PR-6: automation)', () => {
543+
/**
544+
* [#5519] `/automation` stands on the platform anonymous-deny baseline now.
545+
* The cases below go through the REAL `dispatch()`, which re-resolves
546+
* identity off the mock kernel, so an `auth` slot that answers with a
547+
* session is what keeps each of them testing ROUTING (which service method
548+
* a path reaches) instead of quietly re-testing the auth floor. Anonymity
549+
* itself is pinned in `domains/anonymous-gate-actions-automation.test.ts`.
550+
*/
551+
const auth = { api: { getSession: async () => ({ user: { id: 'u_test' } }) } };
552+
543553
it('GET /automation lists flows via the automation service', async () => {
544554
const automation = { listFlows: vi.fn().mockResolvedValue(['flow-a', 'flow-b']) };
545-
const result = await makeDispatcher({ automation }).dispatch('GET', '/automation', undefined, {}, {} as any);
555+
const result = await makeDispatcher({ automation, auth }).dispatch('GET', '/automation', undefined, {}, {} as any);
546556
expect(result.response?.status).toBe(200);
547557
expect(result.response?.body?.data?.total).toBe(2);
548558
});
@@ -556,7 +566,7 @@ describe('HttpDispatcher extracted domains (PR-6: automation)', () => {
556566
{ name: 'a2', source: 'plugin', paradigms: ['workflow'] },
557567
]),
558568
};
559-
const result = await makeDispatcher({ automation }).dispatch('GET', '/automation/actions', undefined, { source: 'plugin' }, {} as any);
569+
const result = await makeDispatcher({ automation, auth }).dispatch('GET', '/automation/actions', undefined, { source: 'plugin' }, {} as any);
560570
expect(result.response?.status).toBe(200);
561571
expect(result.response?.body?.data?.actions).toHaveLength(1);
562572
// The /:name→getFlow catch-all must NOT have shadowed the guard route.
@@ -631,7 +641,7 @@ describe('HttpDispatcher extracted domains (PR-6: automation)', () => {
631641
const execute = vi.fn().mockResolvedValue({ success: true });
632642
const automation = { trigger, execute, listFlows: vi.fn(), getFlow: vi.fn() };
633643

634-
const result = await makeDispatcher({ automation })
644+
const result = await makeDispatcher({ automation, auth })
635645
.dispatch('POST', '/automation/trigger/nurture', {}, {}, {} as any);
636646

637647
expect(result.response?.status).toBe(200);
@@ -653,7 +663,7 @@ describe('HttpDispatcher extracted domains (PR-6: automation)', () => {
653663
{ name: 'nurture', enabled: true, bound: false, status: 'active', triggerType: 'on_create', object: 'sales_lead' },
654664
]),
655665
};
656-
const result = await makeDispatcher({ automation }).dispatch('GET', '/automation/_status', undefined, {}, {} as any);
666+
const result = await makeDispatcher({ automation, auth }).dispatch('GET', '/automation/_status', undefined, {}, {} as any);
657667
expect(result.response?.status).toBe(200);
658668
expect(result.response?.body?.data?.flows?.[0]).toEqual({
659669
name: 'nurture', enabled: true, bound: false,

0 commit comments

Comments
 (0)