Skip to content

Commit fe3d74f

Browse files
Trumpclaude
andauthored
fix(runtime): the enablement door refuses in its own words (#11666) (#12139)
* fix(runtime): the enablement door refuses in its own words (#11666) A caller without `manage_metadata` that hit `POST /automation/:name/toggle` was answered with the refusal the three definition writes share — "Authoring automation flows requires the `manage_metadata` capability." They were disabling a flow, not authoring one: accurate about the policy #10243 ruled, and naming a verb the caller did not use. Adds a second refusal constant for the enablement arm, shaped on this file's own precedent (`SCREEN_READ_DENY_MESSAGE` beside `RUN_READ_DENY_MESSAGE`, #7968): a second constant for a second question, rather than a reworded shared one. The question "is this request the enablement door?" is extracted into `isFlowEnablementWrite` so the gate and the sentence read the SAME answer — this file's own rule that a question spelled at two call sites is two questions that happen to agree today. Copy only. The accept set is bit-identical, `PERMISSION_DENIED` / 403 is unchanged on every arm, and #10243's policy classification is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ * refactor(runtime): keep each docblock next to the function it documents Pure move, no text changed. `isFlowEnablementWrite` and its `[#11666]` docblock were inserted BETWEEN the `[#10145]` docblock and `isFlowAuthoringWrite`, which left two doc comments stacked before one declaration: the nearer one wins, so the helper took the adjacent block and `isFlowAuthoringWrite` was left with none — while the `[#10145]` route-coverage table, where #10243's toggle arm and the `trigger` exclusion are justified, sat above a function it does not describe. That table is exactly what the next author reads before touching the gate. The helper now sits above the `[#10145]` block, so each docblock is adjacent to its own subject and the helper is still declared before its callers. Verified as a pure relocation: the file's multiset of lines is unchanged, and the `[#10145]` block is byte-identical before and after (md5 244ddce7…). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 40a93b5 commit fe3d74f

3 files changed

Lines changed: 428 additions & 4 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): a refused `POST /automation/:name/toggle` is told what it attempted (#11666)
6+
7+
The enablement door refuses in its own words now. A caller without
8+
`manage_metadata` that hit `POST /api/v1/automation/:name/toggle` was answered
9+
with the refusal the three definition writes share:
10+
11+
```text
12+
before: Authoring automation flows requires the `manage_metadata` capability.
13+
after: Enabling or disabling an automation flow requires the `manage_metadata` capability.
14+
```
15+
16+
They were disabling a flow, not authoring one. The sentence was accurate about
17+
the policy — #10243's ruling classified toggle into the `manage_metadata`
18+
authoring write set — and it named a verb the caller did not use.
19+
20+
⛔ **Copy only; no policy moved.** The accept set is bit-identical: the same
21+
callers are refused on the same four routes, `POST /` / `PUT /:name` /
22+
`DELETE /:name` keep the shared sentence they read correctly with, and the
23+
envelope is untouched — `PERMISSION_DENIED` / **403** on every arm, as #11660's
24+
pins and the ADR-0112 vocabulary assert. Nothing becomes newly accepted or
25+
newly rejected.
26+
27+
Shaped on this domain's own precedent (`SCREEN_READ_DENY_MESSAGE` beside
28+
`RUN_READ_DENY_MESSAGE`, #7968): a second constant for a second question,
29+
rather than a reworded shared one. Rewording the shared sentence to cover both
30+
was considered and declined — it would degrade the message for the three
31+
definition writes in order to fix one arm. Both sentences still satisfy #7450:
32+
each names the capability that would admit any caller, and nothing about this
33+
one.
34+
35+
A client branching on the human-readable prose of a 403 (rather than on
36+
`error.code`) is the only thing that can notice.
Lines changed: 328 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,328 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#11666] A refused `POST /automation/:name/toggle` is told what IT attempted.
5+
*
6+
* ## The defect, exactly
7+
*
8+
* #10243's ruling put the enablement door into the `manage_metadata` authoring
9+
* write set, and #11660 landed it by adding one arm to `isFlowAuthoringWrite`.
10+
* The refusal that arm reached was the shared one:
11+
*
12+
* > `Authoring automation flows requires the \`manage_metadata\` capability.`
13+
*
14+
* A caller DISABLING a shipped flow was not authoring one. The sentence is
15+
* accurate about the policy — the ruling classified toggle as an authoring
16+
* write — and it names a verb the caller did not use.
17+
*
18+
* ## Why a pin on the SENTENCE, and why it could not be a pin on the envelope
19+
*
20+
* ⭐ The defect ships a 403 today. Every status- and code-only assertion in
21+
* `automation-write-capability-gate.test.ts` passes over it, in both the
22+
* before and the after state, which is precisely why this file exists and
23+
* asserts the prose. Those assertions are not weakened here; they are the
24+
* envelope half, and this is the copy half.
25+
*
26+
* ⛔ `code` and `status` do NOT move — `PERMISSION_DENIED` / 403 is what
27+
* #11660's pins and the ADR-0112 vocabulary assert — and they are re-asserted
28+
* on every case below so that a future edit to the copy cannot drag the
29+
* envelope with it.
30+
*
31+
* ⛔ The POLICY is untouched. The accept set is bit-identical: the same callers
32+
* are refused on the same four routes. Nothing here is allowed to become newly
33+
* accepted or newly rejected, and the `still refused / still admitted` cases at
34+
* the foot of this file are that guard, not decoration.
35+
*
36+
* ## Both directions, or the change is unpinned where it matters
37+
*
38+
* A one-sided pin ("toggle says the new thing") would sit green if someone
39+
* later reworded the SHARED constant to match — option C, which was considered
40+
* and declined because it degrades the sentence for the authoring writes that
41+
* read correctly today. So each arm asserts its own sentence AND the
42+
* absence of the other's.
43+
*
44+
* ## Driven through the registered route, not the shortcut
45+
*
46+
* Every case goes through `dispatcher.dispatch('POST', '/automation/…')`, which
47+
* resolves the domain out of the registry `createAutomationDomain` registers —
48+
* the path a real request takes, including the prefix slicing. Identity is
49+
* supplied by stubbing `timedResolveExecutionContext`, the seam `dispatch`
50+
* itself writes `context.executionContext` from
51+
* (`automation-resume-envelope.test.ts` drives the same seam the same way).
52+
*/
53+
54+
import { describe, it, expect, vi } from 'vitest';
55+
56+
import { HttpDispatcher } from '../http-dispatcher.js';
57+
import type { HttpProtocolContext } from '../http-dispatcher.js';
58+
59+
const FLOW = 'lead_auto_assignment';
60+
const CAPABILITY = 'manage_metadata';
61+
62+
/** A legal flow definition — so nothing below is refused for its shape. */
63+
const DEFINITION = { name: FLOW, label: 'Lead Auto Assignment', type: 'autolaunched', nodes: [], edges: [] };
64+
65+
/** The two sentences, spelled out here rather than imported — a pin that reads
66+
* its expectation from the module under test cannot see the module change. */
67+
const AUTHORING_SENTENCE = 'Authoring automation flows requires the `manage_metadata` capability.';
68+
const ENABLEMENT_SENTENCE = 'Enabling or disabling an automation flow requires the `manage_metadata` capability.';
69+
70+
/** The filer's principal: authenticated, an org owner, and NOT an author. */
71+
const UNENTITLED = {
72+
userId: 'u_northwind_owner',
73+
positions: ['organization_admin'],
74+
permissions: ['org_admin'],
75+
systemPermissions: [] as string[],
76+
};
77+
78+
/** A metadata author — the positive control. */
79+
const AUTHOR = { userId: 'u_author', systemPermissions: [CAPABILITY] };
80+
81+
interface Harness {
82+
dispatch: (method: string, path: string, body?: unknown) => Promise<any>;
83+
registerFlow: ReturnType<typeof vi.fn>;
84+
unregisterFlow: ReturnType<typeof vi.fn>;
85+
toggleFlow: ReturnType<typeof vi.fn>;
86+
seed: (name: string) => void;
87+
}
88+
89+
function boot(principal: Record<string, unknown> = UNENTITLED): Harness {
90+
const flows = new Map<string, unknown>([[FLOW, { ...DEFINITION }]]);
91+
92+
const registerFlow = vi.fn((name: string, definition: unknown) => { flows.set(name, definition); });
93+
const unregisterFlow = vi.fn((name: string) => { flows.delete(name); });
94+
const toggleFlow = vi.fn(async () => undefined);
95+
const getFlow = vi.fn(async (name: string) => flows.get(name));
96+
const listFlows = vi.fn(async () => [...flows.keys()]);
97+
const execute = vi.fn(async () => ({ success: true, runId: 'run_1', status: 'completed' }));
98+
99+
const services: Record<string, unknown> = {
100+
automation: { handlerReady: true, registerFlow, unregisterFlow, toggleFlow, getFlow, listFlows, execute },
101+
};
102+
const resolve = (name: string): unknown => services[name];
103+
const kernel: any = {
104+
getService: resolve,
105+
getServiceAsync: async (name: string) => resolve(name),
106+
context: { getService: resolve },
107+
};
108+
109+
const dispatcher = new HttpDispatcher(kernel);
110+
// The seam `dispatch` resolves identity through; a context handed in is
111+
// overwritten by it, so the principal is supplied here.
112+
(dispatcher as any).timedResolveExecutionContext = async () => ({ ...principal });
113+
114+
return {
115+
dispatch: (method: string, path: string, body?: unknown) =>
116+
dispatcher.dispatch(method, path, body, {}, { request: {} } as HttpProtocolContext),
117+
registerFlow, unregisterFlow, toggleFlow,
118+
seed: (name: string) => { flows.set(name, { ...DEFINITION, name }); },
119+
};
120+
}
121+
122+
const statusOf = (r: any): unknown => r?.response?.status;
123+
const codeOf = (r: any): unknown => r?.response?.body?.error?.code ?? r?.response?.body?.error?.details?.code;
124+
const messageOf = (r: any): string => String(r?.response?.body?.error?.message ?? '');
125+
126+
/** The three DEFINITION writes — the arm whose sentence must NOT move. */
127+
const DEFINITION_WRITES = [
128+
{
129+
name: 'POST /automation (createFlow)',
130+
drive: (h: Harness) => h.dispatch('POST', '/automation', { ...DEFINITION, name: 'probe_flow_x' }),
131+
spy: (h: Harness) => h.registerFlow,
132+
},
133+
{
134+
name: 'PUT /automation/:name (updateFlow)',
135+
drive: (h: Harness) => h.dispatch('PUT', `/automation/${FLOW}`, { ...DEFINITION, label: 'clobbered' }),
136+
spy: (h: Harness) => h.registerFlow,
137+
},
138+
{
139+
name: 'DELETE /automation/:name (deleteFlow)',
140+
drive: (h: Harness) => h.dispatch('DELETE', `/automation/${FLOW}`),
141+
spy: (h: Harness) => h.unregisterFlow,
142+
},
143+
] as const;
144+
145+
describe('#11666 — the enablement door refuses in its own words', () => {
146+
describe('the arm that was refused: POST /:name/toggle', () => {
147+
it('names enabling/disabling, not authoring', async () => {
148+
const h = boot();
149+
const result = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: false });
150+
151+
// THE POINT of this file.
152+
expect(messageOf(result)).toBe(ENABLEMENT_SENTENCE);
153+
// ⛔ The defect's sentence, gone from this arm — asserted rather
154+
// than implied, because `toBe` above would also pass if the shared
155+
// constant had merely been reworded in place (option C).
156+
expect(messageOf(result)).not.toContain('Authoring');
157+
expect(messageOf(result)).not.toBe(AUTHORING_SENTENCE);
158+
});
159+
160+
it('⛔ carries the same envelope it always did — 403 PERMISSION_DENIED', async () => {
161+
const h = boot();
162+
const result = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: false });
163+
164+
expect(statusOf(result)).toBe(403);
165+
expect(codeOf(result)).toBe('PERMISSION_DENIED');
166+
});
167+
168+
it('⛔ still refuses — the copy change admits nobody new', async () => {
169+
const h = boot();
170+
const result = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: false });
171+
172+
expect(statusOf(result)).toBe(403);
173+
expect(h.toggleFlow).not.toHaveBeenCalled();
174+
});
175+
176+
it('says the same thing in both directions — enabling and disabling', async () => {
177+
// #10243's measurement was symmetric, and a caller switching a flow
178+
// ON is no more "authoring" than one switching it off.
179+
const h = boot();
180+
181+
const off = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: false });
182+
const on = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: true });
183+
184+
expect(messageOf(off)).toBe(ENABLEMENT_SENTENCE);
185+
expect(messageOf(on)).toBe(ENABLEMENT_SENTENCE);
186+
});
187+
188+
it('answers before the body is read, in its own words', async () => {
189+
// The gate is ahead of #3899's body checks, so `{ enable: false }`
190+
// is a 403 rather than the 400 that names the key — and the 403 it
191+
// gets is still the enablement sentence, not the shared one.
192+
const h = boot();
193+
const result = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enable: false });
194+
195+
expect(statusOf(result)).toBe(403);
196+
expect(messageOf(result)).toBe(ENABLEMENT_SENTENCE);
197+
expect(h.toggleFlow).not.toHaveBeenCalled();
198+
});
199+
200+
it('a deeper spelling is gated AND told the same thing — `/:name/toggle/anything`', async () => {
201+
// The router's toggle arm has no depth bound, so this path still
202+
// reaches `toggleFlow`; the gate matches it, and the sentence must
203+
// follow the gate rather than a narrower reading of the path.
204+
const h = boot();
205+
const result = await h.dispatch('POST', `/automation/${FLOW}/toggle/x`, { enabled: false });
206+
207+
expect(statusOf(result)).toBe(403);
208+
expect(messageOf(result)).toBe(ENABLEMENT_SENTENCE);
209+
expect(h.toggleFlow).not.toHaveBeenCalled();
210+
});
211+
});
212+
213+
describe('the arms that were NOT refused here keep the sentence they read correctly with', () => {
214+
for (const route of DEFINITION_WRITES) {
215+
it(`${route.name}: still "Authoring automation flows …"`, async () => {
216+
const h = boot();
217+
const result = await route.drive(h);
218+
219+
expect(messageOf(result)).toBe(AUTHORING_SENTENCE);
220+
// The other direction of the same pin: the enablement wording
221+
// must not bleed onto a definition write.
222+
expect(messageOf(result)).not.toContain('Enabling or disabling');
223+
expect(statusOf(result)).toBe(403);
224+
expect(codeOf(result)).toBe('PERMISSION_DENIED');
225+
expect(route.spy(h)).not.toHaveBeenCalled();
226+
});
227+
}
228+
229+
it('a flow literally NAMED `toggle` is still a definition write on PUT /automation/toggle', async () => {
230+
// The sentence is chosen by the OPERATION, never by a substring of
231+
// the flow name: `parts[1]` is what the enablement arm reads, and a
232+
// one-segment PUT has no `parts[1]` at all.
233+
const h = boot();
234+
h.seed('toggle');
235+
const result = await h.dispatch('PUT', '/automation/toggle', { ...DEFINITION, name: 'toggle' });
236+
237+
expect(statusOf(result)).toBe(403);
238+
expect(messageOf(result)).toBe(AUTHORING_SENTENCE);
239+
expect(h.registerFlow).not.toHaveBeenCalled();
240+
});
241+
242+
it('POST /:name/clone — the arm that arrived AFTER this card — still reads "Authoring …"', async () => {
243+
// [#12156] The clone door joined `isFlowAuthoringWrite` on `main`
244+
// while this branch held the file. This case could not have existed
245+
// when the ones above were written; the merge created the
246+
// population, so the merge owes the pin.
247+
//
248+
// A clone AUTHORS a flow — it registers a new definition under a new
249+
// name — so the shared sentence is the correct one for it. It is also
250+
// the arm most able to inherit the WRONG one: like the enablement
251+
// door it is a two-segment POST whose verb lives in `parts[1]`, so a
252+
// future loosening of `isFlowEnablementWrite` (dropping
253+
// `parts[1] === 'toggle'`, or reading only the method and the depth)
254+
// would hand it "Enabling or disabling an automation flow …" — this
255+
// card's own defect, reproduced on the arm that arrived after it.
256+
const h = boot();
257+
const result = await h.dispatch('POST', `/automation/${FLOW}/clone`, {
258+
name: 'lead_auto_assignment_copy',
259+
label: 'Lead Auto Assignment (copy)',
260+
});
261+
262+
expect(messageOf(result)).toBe(AUTHORING_SENTENCE);
263+
// The other direction, as every arm here does it: the enablement
264+
// wording must not bleed onto an authoring write.
265+
expect(messageOf(result)).not.toContain('Enabling or disabling');
266+
expect(messageOf(result)).not.toBe(ENABLEMENT_SENTENCE);
267+
expect(statusOf(result)).toBe(403);
268+
expect(codeOf(result)).toBe('PERMISSION_DENIED');
269+
expect(h.registerFlow).not.toHaveBeenCalled();
270+
});
271+
});
272+
273+
describe('what both sentences must go on doing (#7450)', () => {
274+
it('each names the capability that would admit ANY caller', async () => {
275+
const h = boot();
276+
277+
const toggle = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: false });
278+
const write = await h.dispatch('DELETE', `/automation/${FLOW}`);
279+
280+
expect(messageOf(toggle)).toContain(CAPABILITY);
281+
expect(messageOf(write)).toContain(CAPABILITY);
282+
});
283+
284+
it('neither answers the caller\'s own authorization topology', async () => {
285+
const h = boot();
286+
const result = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: false });
287+
288+
const serialized = JSON.stringify(result?.response?.body);
289+
expect(serialized).not.toContain('organization_admin');
290+
expect(serialized).not.toContain('u_northwind_owner');
291+
expect(serialized).not.toContain('org_admin');
292+
});
293+
});
294+
295+
describe('⛔ the policy classification is untouched — the accept set is bit-identical', () => {
296+
it('an entitled caller still toggles, in both directions', async () => {
297+
const h = boot(AUTHOR);
298+
299+
const off = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: false });
300+
expect(statusOf(off)).toBe(200);
301+
expect(h.toggleFlow).toHaveBeenLastCalledWith(FLOW, false);
302+
303+
const on = await h.dispatch('POST', `/automation/${FLOW}/toggle`, { enabled: true });
304+
expect(statusOf(on)).toBe(200);
305+
expect(h.toggleFlow).toHaveBeenLastCalledWith(FLOW, true);
306+
});
307+
308+
it('an entitled caller still writes definitions', async () => {
309+
const h = boot(AUTHOR);
310+
const result = await h.dispatch('DELETE', `/automation/${FLOW}`);
311+
312+
expect(statusOf(result)).toBe(200);
313+
expect(h.unregisterFlow).toHaveBeenCalledWith(FLOW);
314+
});
315+
316+
it('the legacy EXECUTION door stays out of the gate — even for a flow named `toggle`', async () => {
317+
// `POST /automation/trigger/:name` is the run door. The enablement
318+
// helper excludes `parts[0] === 'trigger'` for exactly this path,
319+
// and extracting that helper must not have moved the exclusion.
320+
const h = boot();
321+
h.seed('toggle');
322+
const result = await h.dispatch('POST', '/automation/trigger/toggle', {});
323+
324+
expect(statusOf(result)).not.toBe(403);
325+
expect(h.toggleFlow).not.toHaveBeenCalled();
326+
});
327+
});
328+
});

0 commit comments

Comments
 (0)