fix(trigger-api,service-automation): refuse an api flow with no per-flow secret, at arm time and at registration (#20529)

.changeset/20529-api-trigger-requires-secret.md

@@ -0,0 +1,45 @@
+---
+'@objectstack/trigger-api': minor
+'@objectstack/service-automation': minor
+---
+
+fix(trigger-api,service-automation): an `api` flow with no per-flow secret is refused, at arm time and at registration (#20529)
+
+Clause-②: no (narrowing)
+
+**BREAKING** — shipped as `minor` under the launch-window convention
+(`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by
+this banner and the ADR-0087 disposition below, never by the level).
+
+ADR-0041's `trigger-api` acceptance criteria name a per-flow secret and HMAC
+signature verification. The trigger used to arm a flow's inbound hook without a
+secret, with only a warning, and that hook skipped signature verification. An
+`api` flow whose start node carries no non-blank `config.secret` is now refused
+in two places:
+
+- **At registration** (`@objectstack/service-automation`). `registerFlow` refuses
+  a flow whose binding resolves to the `api` trigger (`type: 'api'`, or a start
+  node with `triggerType: 'api'`) when the start node declares no non-blank
+  `config.secret`, whatever the flow's `status`. The error names the flow and
+  `config.secret`. The `/automation` create, update and clone doors answer it as
+  `400 VALIDATION_FAILED`, like every other registration refusal. At boot the
+  flow is skipped and the existing `[Automation] failed to register flow` warning
+  names it.
+- **At arm time** (`@objectstack/trigger-api`). `ApiTrigger.start()` throws,
+  naming the flow and `config.secret`, before it stores a hook or subscribes a
+  queue consumer. The engine logs `Failed to bind flow` and the flow stays
+  unbound. This covers a host that binds the trigger without the engine. The
+  arm-time `armed WITHOUT a secret` warning is gone, since that state no longer
+  exists. Every armed hook verifies the signature on every post.
+
+**Fix.** Give the flow's start node a non-blank `config.secret` and sign each
+post with it, as the `x-objectstack-signature` header already documents. A flow
+that is only ever started explicitly (`engine.execute()`, or the `/automation`
+trigger route) and is not meant to receive inbound posts is an `autolaunched`
+flow. Declare it `type: 'autolaunched'`, with no `triggerType: 'api'` on its
+start node, and it needs no secret.
+
+Unchanged: a flow that already carries a secret registers, arms and verifies
+exactly as before.
+
+<!-- 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`). -->

packages/services/service-automation/src/api-trigger-secret-registration.test.ts

@@ -0,0 +1,135 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * ADR-0041 — `trigger-api`'s acceptance criteria name "a per-flow secret; HMAC
+ * signature verification". A flow whose binding resolves to the `api` trigger
+ * and whose start node carries no non-blank `config.secret` is refused at
+ * REGISTRATION — the publish seam — so its author learns before deploying.
+ * (`@objectstack/trigger-api`'s own `start()` refuses the same binding for a
+ * host that binds without this engine; its tests pin that half.)
+ *
+ * Every refusal case asserts the substance, not only the throw: the flow is
+ * absent from the engine afterwards and the api trigger was never started. The
+ * contrast cases pin what stays legal — a signed `api` flow registers and its
+ * trigger receives that secret, and a flow that binds no `api` trigger needs
+ * none.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { AutomationEngine } from './engine.js';
+import type { FlowTrigger, FlowTriggerBinding } from './engine.js';
+
+function createTestLogger() {
+    return { debug() {}, info() {}, warn() {}, error() {} } as any;
+}
+
+/** A minimal registrable flow whose start node carries `config`. */
+function flowWith(
+    name: string,
+    config: Record<string, unknown>,
+    type: string = 'api',
+    extra: Record<string, unknown> = {},
+) {
+    return {
+        name,
+        label: name,
+        type,
+        status: 'active',
+        ...extra,
+        nodes: [
+            { id: 'start', type: 'start', label: 'Start', config },
+            { id: 'end', type: 'end', label: 'End' },
+        ],
+        edges: [{ id: 'e1', source: 'start', target: 'end' }],
+    };
+}
+
+describe('ADR-0041 — an api flow registers only with its per-flow secret', () => {
+    let engine: AutomationEngine;
+    let started: FlowTriggerBinding[];
+    let stopped: string[];
+
+    beforeEach(() => {
+        engine = new AutomationEngine(createTestLogger());
+        started = [];
+        stopped = [];
+        // A recording `api` trigger, registered BEFORE the flows, so a flow that
+        // got past registration would be started on it at once.
+        const trigger: FlowTrigger = {
+            type: 'api',
+            start: (binding) => {
+                started.push(binding);
+            },
+            stop: (flowName) => {
+                stopped.push(flowName);
+            },
+        };
+        engine.registerTrigger(trigger);
+    });
+
+    const refused: Array<{ label: string; flow: ReturnType<typeof flowWith> }> = [
+        { label: "a `type: 'api'` flow with no secret", flow: flowWith('no_secret', {}) },
+        { label: "a `type: 'api'` flow with a blank secret", flow: flowWith('blank_secret', { secret: '   ' }) },
+        { label: "a `type: 'api'` flow with a non-string secret", flow: flowWith('numeric_secret', { secret: 42 }) },
+        {
+            label: "a start-node `triggerType: 'api'` flow with no secret",
+            flow: flowWith('token_no_secret', { triggerType: 'api', hookId: 'intake' }, 'autolaunched'),
+        },
+        {
+            // Status-agnostic, like every other registration refusal: an
+            // `obsolete` flow is re-enabled by a toggle, not by re-registering.
+            label: "an obsolete `type: 'api'` flow with no secret",
+            flow: flowWith('obsolete_no_secret', {}, 'api', { status: 'obsolete' }),
+        },
+    ];
+
+    for (const row of refused) {
+        it(`refuses ${row.label} at registration, naming the flow and config.secret, and arms nothing`, async () => {
+            const name = row.flow.name;
+            expect(() => engine.registerFlow(name, row.flow as never)).toThrow(
+                new RegExp(`Flow '${name}' rejected: .*\`api\` trigger.*config\\.secret`, 's'),
+            );
+
+            // Not registered: nothing to read back, nothing to run, nothing armed.
+            expect(await engine.getFlow(name)).toBeNull();
+            expect(engine.getFlowRuntimeStates().map((s) => s.name)).not.toContain(name);
+            expect(started).toEqual([]);
+        });
+    }
+
+    it('registers and binds an api flow that carries its secret, handing the trigger that secret', async () => {
+        engine.registerFlow('signed_hook', flowWith('signed_hook', { hookId: 'intake', secret: 's3cret' }) as never);
+
+        expect(await engine.getFlow('signed_hook')).not.toBeNull();
+        expect(started).toHaveLength(1);
+        expect(started[0].flowName).toBe('signed_hook');
+        expect(started[0].config).toMatchObject({ hookId: 'intake', secret: 's3cret' });
+        const state = engine.getFlowRuntimeStates().find((s) => s.name === 'signed_hook');
+        expect(state?.triggerType).toBe('api');
+    });
+
+    it('requires nothing of a flow that binds no api trigger — an autolaunched flow registers without a secret', async () => {
+        engine.registerFlow('manual', flowWith('manual', {}, 'autolaunched') as never);
+
+        expect(await engine.getFlow('manual')).not.toBeNull();
+        expect(started).toEqual([]);
+        expect((await engine.execute('manual')).success).toBe(true);
+    });
+
+    it('refuses a re-registration that drops the secret, and the registered signed version stays armed', async () => {
+        engine.registerFlow('signed_hook', flowWith('signed_hook', { secret: 's3cret' }) as never);
+        expect(started).toHaveLength(1);
+
+        expect(() => engine.registerFlow('signed_hook', flowWith('signed_hook', {}) as never)).toThrow(
+            /Flow 'signed_hook' rejected: .*config\.secret/s,
+        );
+
+        // The refused definition never replaced the stored one, and the trigger
+        // was neither stopped nor re-started with the unsigned binding.
+        const stored = await engine.getFlow('signed_hook');
+        const start = stored?.nodes.find((n) => n.type === 'start');
+        expect((start?.config as Record<string, unknown> | undefined)?.secret).toBe('s3cret');
+        expect(started).toHaveLength(1);
+        expect(stopped).toEqual([]);
+    });
+});

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

@@ -1306,7 +1306,9 @@ describe('AutomationEngine - Execution History', () => {
     const simpleFlow = {
         name: 'test_flow',
         label: 'Test Flow',
-        type: 'api' as const,
+        // Started explicitly (`engine.execute`), never by an inbound post —
+        // an `api` flow is an inbound hook and needs a `config.secret` (ADR-0041).
+        type: 'autolaunched' as const,
         nodes: [
             { id: 'start', type: 'start' as const, label: 'Start' },
             { id: 'end', type: 'end' as const, label: 'End' },

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

@@ -3433,6 +3433,19 @@ export class AutomationEngine implements IAutomationService {
     ): { triggerType: string; binding: FlowTriggerBinding } | undefined {
         const flow = this.flows.get(flowName);
         if (!flow) return undefined;
+        return this.deriveTriggerBinding(flowName, flow);
+    }
+
+    /**
+     * {@link resolveTriggerBinding}'s body, over a flow that need not be
+     * registered yet — so {@link validateApiTriggerSecret} judges, at
+     * registration, the very binding {@link activateFlowTrigger} would hand the
+     * trigger, rather than a second reading of the start node.
+     */
+    private deriveTriggerBinding(
+        flowName: string,
+        flow: FlowParsed,
+    ): { triggerType: string; binding: FlowTriggerBinding } | undefined {
         const startNode = flow.nodes.find(n => n.type === 'start');
         const config = (startNode?.config ?? {}) as Record<string, unknown>;
         const condition = (config.condition as FlowTriggerBinding['condition']) ?? undefined;
@@ -4191,6 +4204,12 @@ export class AutomationEngine implements IAutomationService {
         // safe to run.
         this.validateFlowExpressions(name, parsed);
 
+        // ADR-0041 — an `api` flow's inbound hook requires a per-flow secret.
+        // Refused here, at the publish seam, so the author learns before
+        // deploying; `trigger-api`'s own `start()` refuses the same binding for
+        // a host that binds without this engine.
+        this.validateApiTriggerSecret(name, parsed);
+
         // Version history management
         const history = this.flowVersionHistory.get(name) ?? [];
         history.push({
@@ -9349,6 +9368,46 @@ export class AutomationEngine implements IAutomationService {
         }
     }
 
+    /**
+     * ADR-0041 — the registration-time half of `trigger-api`'s acceptance
+     * criteria: "a per-flow secret; HMAC signature verification". A flow whose
+     * binding resolves to the `api` trigger (the kind {@link
+     * deriveTriggerBinding} answers — `type: 'api'` or a start-node
+     * `triggerType: 'api'`) and whose start node carries no non-blank
+     * `config.secret` is refused, whatever its `status`: such a flow can never
+     * be armed, and an author who wrote it should hear so at publish time,
+     * not from a boot audit.
+     *
+     * It reads the BINDING's `config` — the same object `trigger-api`'s
+     * `start()` reads the secret from — so the two refusals judge one input
+     * and cannot disagree about which flows need a secret. The rule is kept in
+     * both places deliberately: `@objectstack/trigger-api` does not depend on
+     * this package (nor this package on it), and the trigger's own refusal is
+     * what protects a host that binds without this engine.
+     *
+     * Hard-fail, like {@link validateNodeConfigKeys}: every `registerFlow` call
+     * site already try/catches per flow, so a refused flow is skipped loudly
+     * at boot, and the `/automation` write doors answer the throw as `400
+     * VALIDATION_FAILED`.
+     */
+    private validateApiTriggerSecret(flowName: string, flow: FlowParsed): void {
+        const resolved = this.deriveTriggerBinding(flowName, flow);
+        if (resolved?.triggerType !== 'api') return;
+        const config = (resolved.binding.config ?? {}) as Record<string, unknown>;
+        if (typeof config.secret === 'string' && config.secret.trim() !== '') return;
+        const asks = [
+            flow.type === 'api' ? "`type: 'api'`" : undefined,
+            config.triggerType === 'api' ? "start-node `config.triggerType: 'api'`" : undefined,
+        ].filter((s): s is string => s !== undefined);
+        throw new Error(
+            `Flow '${flowName}' rejected: it binds the inbound \`api\` trigger (${asks.join(' and ')}) but its ` +
+            `start node declares no \`config.secret\`. An inbound hook is armed only with a per-flow secret that ` +
+            `every post is HMAC-verified against (ADR-0041), so this flow could never be armed. Set a non-blank ` +
+            `\`config.secret\` on the start node. A flow that is only ever started explicitly and never receives ` +
+            `inbound posts is \`type: 'autolaunched'\`, with no \`triggerType: 'api'\` on its start node.`,
+        );
+    }
+
     /**
      * Walk `value` against `schema` in lockstep, collecting keys the schema does
      * not declare into `violations`.

packages/services/service-automation/src/flow-activation-ledger.test.ts

@@ -148,7 +148,8 @@ describe('ADR-0126 §7.2 — a ledger-disabled flow refuses at the execute() sea
         ['record-change', { objectName: 'lead', triggerType: 'record-after-create' }, 'record_change'],
         ['schedule', { schedule: '0 9 * * *' }, 'schedule'],
         ['time-relative', { timeRelative: { object: 'task', field: 'due_at' }, schedule: '0 * * * *' }, 'time_relative'],
-        ['api', { triggerType: 'api' }, 'api'],
+        // An `api` flow registers only with its per-flow secret (ADR-0041).
+        ['api', { triggerType: 'api', secret: 'hook-secret' }, 'api'],
     ];
 
     for (const [label, startConfig, triggerKey] of entryPaths) {

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

@@ -115,12 +115,13 @@ describe('[#14328] the engine takes its trigger kind from spec.resolveFlowTrigge
         },
         {
             case: "type: 'api'",
-            flow: flowWith('api_type', {}, 'api'),
+            // An `api` flow registers only with its per-flow secret (ADR-0041).
+            flow: flowWith('api_type', { secret: 'hook-secret' }, 'api'),
             expected: 'api',
         },
         {
             case: "triggerType: 'api'",
-            flow: flowWith('api_token', { triggerType: 'api' }),
+            flow: flowWith('api_token', { triggerType: 'api', secret: 'hook-secret' }),
             expected: 'api',
         },
     ];