Skip to content

Commit d5c202d

Browse files
committed
fix(trigger-api,service-automation): refuse an api flow with no per-flow secret at arm time and at registration
ADR-0041's trigger-api acceptance criteria name a per-flow secret and HMAC verification. ApiTrigger.start() now throws, naming the flow and config.secret, before it stores a hook or subscribes a consumer; the armed hook's secret is required by its type, so handleRequest verifies every post. AutomationEngine.registerFlow refuses the same binding at the publish seam, reading the binding the engine would hand the trigger. Claude-Session: https://claude.ai/code/session_017B6YKCGu8CTY2KBWgwaHAs Co-authored-by: Claude <noreply@anthropic.com>
1 parent 288611e commit d5c202d

8 files changed

Lines changed: 324 additions & 38 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
'@objectstack/trigger-api': minor
3+
'@objectstack/service-automation': minor
4+
---
5+
6+
fix(trigger-api,service-automation): an `api` flow with no per-flow secret is refused, at arm time and at registration (#20529)
7+
8+
Clause-②: no (narrowing)
9+
10+
**BREAKING** — shipped as `minor` under the launch-window convention
11+
(`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by
12+
this banner and the ADR-0087 disposition below, never by the level).
13+
14+
ADR-0041's `trigger-api` acceptance criteria name a per-flow secret and HMAC
15+
signature verification. The trigger used to arm a flow's inbound hook without a
16+
secret, with only a warning, and that hook skipped signature verification. An
17+
`api` flow whose start node carries no non-blank `config.secret` is now refused
18+
in two places:
19+
20+
- **At registration** (`@objectstack/service-automation`). `registerFlow` refuses
21+
a flow whose binding resolves to the `api` trigger (`type: 'api'`, or a start
22+
node with `triggerType: 'api'`) when the start node declares no non-blank
23+
`config.secret`, whatever the flow's `status`. The error names the flow and
24+
`config.secret`. The `/automation` create, update and clone doors answer it as
25+
`400 VALIDATION_FAILED`, like every other registration refusal. At boot the
26+
flow is skipped and the existing `[Automation] failed to register flow` warning
27+
names it.
28+
- **At arm time** (`@objectstack/trigger-api`). `ApiTrigger.start()` throws,
29+
naming the flow and `config.secret`, before it stores a hook or subscribes a
30+
queue consumer. The engine logs `Failed to bind flow` and the flow stays
31+
unbound. This covers a host that binds the trigger without the engine. The
32+
arm-time `armed WITHOUT a secret` warning is gone, since that state no longer
33+
exists. Every armed hook verifies the signature on every post.
34+
35+
**Fix.** Give the flow's start node a non-blank `config.secret` and sign each
36+
post with it, as the `x-objectstack-signature` header already documents. A flow
37+
that is only ever started explicitly (`engine.execute()`, or the `/automation`
38+
trigger route) and is not meant to receive inbound posts is an `autolaunched`
39+
flow. Declare it `type: 'autolaunched'`, with no `triggerType: 'api'` on its
40+
start node, and it needs no secret.
41+
42+
Unchanged: a flow that already carries a secret registers, arms and verifies
43+
exactly as before.
44+
45+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable changes spelling or type: `packages/spec` is untouched, the start node's `config` stays the open record it was, and `hookId` and `secret` are read from it exactly as before. What changes is runtime behaviour for one authored shape, an `api`-kind flow whose start node carries no `config.secret`, which is now refused at registration and at arm time. `objectstack migrate meta` could not rewrite that shape even in principle, because the missing value is a shared secret only the author and the sending system can supply. The other categories are closed on facts: both packages publish (not `unpublished`); no ADR-0087 id covers this shape and none is minted here (not `registered` / `already-registered`); and the change is runtime behaviour, not a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* ADR-0041 — `trigger-api`'s acceptance criteria name "a per-flow secret; HMAC
5+
* signature verification". A flow whose binding resolves to the `api` trigger
6+
* and whose start node carries no non-blank `config.secret` is refused at
7+
* REGISTRATION — the publish seam — so its author learns before deploying.
8+
* (`@objectstack/trigger-api`'s own `start()` refuses the same binding for a
9+
* host that binds without this engine; its tests pin that half.)
10+
*
11+
* Every refusal case asserts the substance, not only the throw: the flow is
12+
* absent from the engine afterwards and the api trigger was never started. The
13+
* contrast cases pin what stays legal — a signed `api` flow registers and its
14+
* trigger receives that secret, and a flow that binds no `api` trigger needs
15+
* none.
16+
*/
17+
18+
import { describe, it, expect, beforeEach } from 'vitest';
19+
import { AutomationEngine } from './engine.js';
20+
import type { FlowTrigger, FlowTriggerBinding } from './engine.js';
21+
22+
function createTestLogger() {
23+
return { debug() {}, info() {}, warn() {}, error() {} } as any;
24+
}
25+
26+
/** A minimal registrable flow whose start node carries `config`. */
27+
function flowWith(
28+
name: string,
29+
config: Record<string, unknown>,
30+
type: string = 'api',
31+
extra: Record<string, unknown> = {},
32+
) {
33+
return {
34+
name,
35+
label: name,
36+
type,
37+
status: 'active',
38+
...extra,
39+
nodes: [
40+
{ id: 'start', type: 'start', label: 'Start', config },
41+
{ id: 'end', type: 'end', label: 'End' },
42+
],
43+
edges: [{ id: 'e1', source: 'start', target: 'end' }],
44+
};
45+
}
46+
47+
describe('ADR-0041 — an api flow registers only with its per-flow secret', () => {
48+
let engine: AutomationEngine;
49+
let started: FlowTriggerBinding[];
50+
let stopped: string[];
51+
52+
beforeEach(() => {
53+
engine = new AutomationEngine(createTestLogger());
54+
started = [];
55+
stopped = [];
56+
// A recording `api` trigger, registered BEFORE the flows, so a flow that
57+
// got past registration would be started on it at once.
58+
const trigger: FlowTrigger = {
59+
type: 'api',
60+
start: (binding) => {
61+
started.push(binding);
62+
},
63+
stop: (flowName) => {
64+
stopped.push(flowName);
65+
},
66+
};
67+
engine.registerTrigger(trigger);
68+
});
69+
70+
const refused: Array<{ label: string; flow: ReturnType<typeof flowWith> }> = [
71+
{ label: "a `type: 'api'` flow with no secret", flow: flowWith('no_secret', {}) },
72+
{ label: "a `type: 'api'` flow with a blank secret", flow: flowWith('blank_secret', { secret: ' ' }) },
73+
{ label: "a `type: 'api'` flow with a non-string secret", flow: flowWith('numeric_secret', { secret: 42 }) },
74+
{
75+
label: "a start-node `triggerType: 'api'` flow with no secret",
76+
flow: flowWith('token_no_secret', { triggerType: 'api', hookId: 'intake' }, 'autolaunched'),
77+
},
78+
{
79+
// Status-agnostic, like every other registration refusal: an
80+
// `obsolete` flow is re-enabled by a toggle, not by re-registering.
81+
label: "an obsolete `type: 'api'` flow with no secret",
82+
flow: flowWith('obsolete_no_secret', {}, 'api', { status: 'obsolete' }),
83+
},
84+
];
85+
86+
for (const row of refused) {
87+
it(`refuses ${row.label} at registration, naming the flow and config.secret, and arms nothing`, async () => {
88+
const name = row.flow.name;
89+
expect(() => engine.registerFlow(name, row.flow as never)).toThrow(
90+
new RegExp(`Flow '${name}' rejected: .*\`api\` trigger.*config\\.secret`, 's'),
91+
);
92+
93+
// Not registered: nothing to read back, nothing to run, nothing armed.
94+
expect(await engine.getFlow(name)).toBeNull();
95+
expect(engine.getFlowRuntimeStates().map((s) => s.name)).not.toContain(name);
96+
expect(started).toEqual([]);
97+
});
98+
}
99+
100+
it('registers and binds an api flow that carries its secret, handing the trigger that secret', async () => {
101+
engine.registerFlow('signed_hook', flowWith('signed_hook', { hookId: 'intake', secret: 's3cret' }) as never);
102+
103+
expect(await engine.getFlow('signed_hook')).not.toBeNull();
104+
expect(started).toHaveLength(1);
105+
expect(started[0].flowName).toBe('signed_hook');
106+
expect(started[0].config).toMatchObject({ hookId: 'intake', secret: 's3cret' });
107+
const state = engine.getFlowRuntimeStates().find((s) => s.name === 'signed_hook');
108+
expect(state?.triggerType).toBe('api');
109+
});
110+
111+
it('requires nothing of a flow that binds no api trigger — an autolaunched flow registers without a secret', async () => {
112+
engine.registerFlow('manual', flowWith('manual', {}, 'autolaunched') as never);
113+
114+
expect(await engine.getFlow('manual')).not.toBeNull();
115+
expect(started).toEqual([]);
116+
expect((await engine.execute('manual')).success).toBe(true);
117+
});
118+
119+
it('refuses a re-registration that drops the secret, and the registered signed version stays armed', async () => {
120+
engine.registerFlow('signed_hook', flowWith('signed_hook', { secret: 's3cret' }) as never);
121+
expect(started).toHaveLength(1);
122+
123+
expect(() => engine.registerFlow('signed_hook', flowWith('signed_hook', {}) as never)).toThrow(
124+
/Flow 'signed_hook' rejected: .*config\.secret/s,
125+
);
126+
127+
// The refused definition never replaced the stored one, and the trigger
128+
// was neither stopped nor re-started with the unsigned binding.
129+
const stored = await engine.getFlow('signed_hook');
130+
const start = stored?.nodes.find((n) => n.type === 'start');
131+
expect((start?.config as Record<string, unknown> | undefined)?.secret).toBe('s3cret');
132+
expect(started).toHaveLength(1);
133+
expect(stopped).toEqual([]);
134+
});
135+
});

‎packages/services/service-automation/src/engine.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1306,7 +1306,9 @@ describe('AutomationEngine - Execution History', () => {
13061306
const simpleFlow = {
13071307
name: 'test_flow',
13081308
label: 'Test Flow',
1309-
type: 'api' as const,
1309+
// Started explicitly (`engine.execute`), never by an inbound post —
1310+
// an `api` flow is an inbound hook and needs a `config.secret` (ADR-0041).
1311+
type: 'autolaunched' as const,
13101312
nodes: [
13111313
{ id: 'start', type: 'start' as const, label: 'Start' },
13121314
{ id: 'end', type: 'end' as const, label: 'End' },

‎packages/services/service-automation/src/engine.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3433,6 +3433,19 @@ export class AutomationEngine implements IAutomationService {
34333433
): { triggerType: string; binding: FlowTriggerBinding } | undefined {
34343434
const flow = this.flows.get(flowName);
34353435
if (!flow) return undefined;
3436+
return this.deriveTriggerBinding(flowName, flow);
3437+
}
3438+
3439+
/**
3440+
* {@link resolveTriggerBinding}'s body, over a flow that need not be
3441+
* registered yet — so {@link validateApiTriggerSecret} judges, at
3442+
* registration, the very binding {@link activateFlowTrigger} would hand the
3443+
* trigger, rather than a second reading of the start node.
3444+
*/
3445+
private deriveTriggerBinding(
3446+
flowName: string,
3447+
flow: FlowParsed,
3448+
): { triggerType: string; binding: FlowTriggerBinding } | undefined {
34363449
const startNode = flow.nodes.find(n => n.type === 'start');
34373450
const config = (startNode?.config ?? {}) as Record<string, unknown>;
34383451
const condition = (config.condition as FlowTriggerBinding['condition']) ?? undefined;
@@ -4191,6 +4204,12 @@ export class AutomationEngine implements IAutomationService {
41914204
// safe to run.
41924205
this.validateFlowExpressions(name, parsed);
41934206

4207+
// ADR-0041 — an `api` flow's inbound hook requires a per-flow secret.
4208+
// Refused here, at the publish seam, so the author learns before
4209+
// deploying; `trigger-api`'s own `start()` refuses the same binding for
4210+
// a host that binds without this engine.
4211+
this.validateApiTriggerSecret(name, parsed);
4212+
41944213
// Version history management
41954214
const history = this.flowVersionHistory.get(name) ?? [];
41964215
history.push({
@@ -9349,6 +9368,46 @@ export class AutomationEngine implements IAutomationService {
93499368
}
93509369
}
93519370

9371+
/**
9372+
* ADR-0041 — the registration-time half of `trigger-api`'s acceptance
9373+
* criteria: "a per-flow secret; HMAC signature verification". A flow whose
9374+
* binding resolves to the `api` trigger (the kind {@link
9375+
* deriveTriggerBinding} answers — `type: 'api'` or a start-node
9376+
* `triggerType: 'api'`) and whose start node carries no non-blank
9377+
* `config.secret` is refused, whatever its `status`: such a flow can never
9378+
* be armed, and an author who wrote it should hear so at publish time,
9379+
* not from a boot audit.
9380+
*
9381+
* It reads the BINDING's `config` — the same object `trigger-api`'s
9382+
* `start()` reads the secret from — so the two refusals judge one input
9383+
* and cannot disagree about which flows need a secret. The rule is kept in
9384+
* both places deliberately: `@objectstack/trigger-api` does not depend on
9385+
* this package (nor this package on it), and the trigger's own refusal is
9386+
* what protects a host that binds without this engine.
9387+
*
9388+
* Hard-fail, like {@link validateNodeConfigKeys}: every `registerFlow` call
9389+
* site already try/catches per flow, so a refused flow is skipped loudly
9390+
* at boot, and the `/automation` write doors answer the throw as `400
9391+
* VALIDATION_FAILED`.
9392+
*/
9393+
private validateApiTriggerSecret(flowName: string, flow: FlowParsed): void {
9394+
const resolved = this.deriveTriggerBinding(flowName, flow);
9395+
if (resolved?.triggerType !== 'api') return;
9396+
const config = (resolved.binding.config ?? {}) as Record<string, unknown>;
9397+
if (typeof config.secret === 'string' && config.secret.trim() !== '') return;
9398+
const asks = [
9399+
flow.type === 'api' ? "`type: 'api'`" : undefined,
9400+
config.triggerType === 'api' ? "start-node `config.triggerType: 'api'`" : undefined,
9401+
].filter((s): s is string => s !== undefined);
9402+
throw new Error(
9403+
`Flow '${flowName}' rejected: it binds the inbound \`api\` trigger (${asks.join(' and ')}) but its ` +
9404+
`start node declares no \`config.secret\`. An inbound hook is armed only with a per-flow secret that ` +
9405+
`every post is HMAC-verified against (ADR-0041), so this flow could never be armed. Set a non-blank ` +
9406+
`\`config.secret\` on the start node. A flow that is only ever started explicitly and never receives ` +
9407+
`inbound posts is \`type: 'autolaunched'\`, with no \`triggerType: 'api'\` on its start node.`,
9408+
);
9409+
}
9410+
93529411
/**
93539412
* Walk `value` against `schema` in lockstep, collecting keys the schema does
93549413
* not declare into `violations`.

‎packages/services/service-automation/src/flow-trigger-kind-shared-resolver.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,13 @@ describe('[#14328] the engine takes its trigger kind from spec.resolveFlowTrigge
115115
},
116116
{
117117
case: "type: 'api'",
118-
flow: flowWith('api_type', {}, 'api'),
118+
// An `api` flow registers only with its per-flow secret (ADR-0041).
119+
flow: flowWith('api_type', { secret: 'hook-secret' }, 'api'),
119120
expected: 'api',
120121
},
121122
{
122123
case: "triggerType: 'api'",
123-
flow: flowWith('api_token', { triggerType: 'api' }),
124+
flow: flowWith('api_token', { triggerType: 'api', secret: 'hook-secret' }),
124125
expected: 'api',
125126
},
126127
];

0 commit comments

Comments
 (0)