Skip to content

Commit f26b2f4

Browse files
committed
fix(trigger-api,spec): one answer for every inbound hook post that does not verify
An unknown flow, a wrong hook id, an absent signature and a bad one now all get the same 401 INVALID_SIGNATURE status and body from handleRequest, where the first two answered 404 RESOURCE_NOT_FOUND. The cause goes to the server log as a structured field only. The secret-unavailable 503 is unchanged and named as the residual; the hook is still matched before its secret is read, so that 503 answers only a post that names an armed flow and its hook id. The ITriggerApiService.handleInboundHook docblock states the one refusal and the residual (FROM -> TO), and defers the accepted signature forms to core's verifyHttpSignature. Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp Co-authored-by: Claude <noreply@anthropic.com>
1 parent c11b758 commit f26b2f4

4 files changed

Lines changed: 267 additions & 33 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/trigger-api': minor
3+
'@objectstack/spec': minor
4+
---
5+
6+
fix(trigger-api,spec): every inbound hook post that does not verify gets one answer, `401 INVALID_SIGNATURE`, whatever its flow and hook id
7+
8+
Clause-②: yes
9+
10+
- **FROM → TO.** On `POST /api/v1/automation/hooks/:flowName/:hookId`, a post that named no armed flow, or the wrong hook id, answered `404 RESOURCE_NOT_FOUND`, while a post naming an armed flow and its hook id with an absent or bad `x-objectstack-signature` answered `401 INVALID_SIGNATURE`. Now all four answer `401 INVALID_SIGNATURE` with the same body, so the answer no longer tells an armed flow name from an unarmed one, whichever hook id a flow is armed with.
11+
- **Who sees a difference.** A correctly configured sender (the flow's URL, its hook id, its secret) sees none: it still gets `202`. A sender posting to a flow that is not armed, or with a hook id that was rotated away, now gets `401` where it got `404`. A sender with a wrong secret or signing code still gets `401`, as before.
12+
- **The cause is in the server log.** The trigger logs one `warn` line per refused post, with the cause and the flow name the post carried as structured fields: `cause` is one of `unknown-flow`, `wrong-hook-id`, `signature-absent`, `signature-malformed`, `signature-mismatch` or `signature-outside-window`. The line never carries the secret, the signature value or the body.
13+
- **Unchanged: the `503` residual.** A post that names an armed flow and its hook id while that hook's secret cannot be read still answers `503 SERVICE_UNAVAILABLE`, signed or not, without verifying. It stays a `503` on purpose: it is the deployment's fault, a sender retries it, and it lasts only while the secret cannot be read. A wrong hook id never reaches the secret read, so it gets the uniform `401` even then.
14+
- **`@objectstack/spec`.** The `ITriggerApiService.handleInboundHook` docblock states the one refusal and names the `503` as its residual. The type is unchanged.
15+
16+
**To adopt it.** A sender or monitor that read `404` from this route as "wrong URL" can no longer tell that from a bad signature by the answer: read the trigger's `warn` line for the post instead, whose `cause` names which one it was.

‎packages/spec/src/contracts/trigger-api-service.ts‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,21 +43,28 @@ export interface ITriggerApiService {
4343
* What it serves, exactly as the plugin's self-hosted mount serves it
4444
* today, in this order:
4545
* - It looks up the hook armed for `hook.flowName` and compares
46-
* `hook.hookId` with it in constant time. An unknown flow and a wrong
47-
* hook id answer identically, `404 RESOURCE_NOT_FOUND`. The hook is
48-
* matched BEFORE the signature is verified, so an unsigned post that
49-
* names an armed flow and its hook id gets the `503` or `401` below
50-
* instead of that `404`: the hook id, not the flow name, is what keeps
51-
* an armed flow unprobeable. A flow left on the default hook id
52-
* (`default`, when its start node's `config.hookId` names none) can be
53-
* probed that way. Whether every unverified post should get one uniform
54-
* refusal is the hardening card's question (#22769), not this contract's.
46+
* `hook.hookId` with it in constant time. A post that names no armed
47+
* flow, or the wrong hook id, gets the unverified-post refusal below,
48+
* before any secret is read.
5549
* - It reads that hook's own secret, at this moment, so a rotation applies
5650
* to the next post. A secret that cannot be read answers
5751
* `503 SERVICE_UNAVAILABLE`. ⛔ It never verifies against no secret.
58-
* - It verifies the `x-objectstack-signature` header (`sha256=` and the hex
59-
* HMAC-SHA256 of the raw body under that secret). A missing or wrong
60-
* signature answers `401 INVALID_SIGNATURE`.
52+
* This `503` is the one residual of the refusal below: while an armed
53+
* hook's secret cannot be read, a post that names that flow and its hook
54+
* id gets this `503`, signed or not. It stays a `503` on purpose: it is
55+
* the deployment's fault, a sender retries it, and it lasts only while
56+
* the secret cannot be read.
57+
* - It verifies the `x-objectstack-signature` header over the raw body
58+
* under that secret, through `@objectstack/core`'s `verifyHttpSignature`,
59+
* the one definition of the forms a sender may sign with.
60+
* - **One refusal for every post that does not verify** (#22806): an
61+
* unknown flow, a wrong hook id, an absent signature and a bad one all
62+
* answer `401 INVALID_SIGNATURE`, with one body. So the answer does not
63+
* tell an armed flow name from an unarmed one, whichever hook id a flow
64+
* is armed with. The cause (which of those four) goes to the server log,
65+
* ⛔ never to the sender. FROM → TO: an unknown flow and a wrong hook id
66+
* used to answer `404 RESOURCE_NOT_FOUND`; they now answer the same
67+
* `401 INVALID_SIGNATURE` an absent or bad signature always has.
6168
* - Only then does it read the body as JSON. A body that is not valid JSON,
6269
* or not a JSON object, answers `400 INVALID_REQUEST`; an empty body is
6370
* the empty object.

‎packages/triggers/trigger-api/src/api-trigger.test.ts‎

Lines changed: 165 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,13 @@ describe('ApiTrigger', () => {
8282
expect(runs[0].params).toEqual({ title: 'New lead', amount: 5000 });
8383
});
8484

85-
it('answers 404 identically for unknown flows and wrong hookIds (no probing oracle)', async () => {
85+
it('answers unknown flows and wrong hookIds identically, with the unverified-post refusal', async () => {
8686
arm({ hookId: 'hk1', secret: 's3cret' });
8787
const a = await trigger.handleRequest({ flowName: 'nope', hookId: 'hk1', rawBody: '{}' });
8888
const b = await trigger.handleRequest({ flowName: 'lead_intake', hookId: 'wrong', rawBody: '{}' });
8989
expect(a).toEqual(b);
90-
expect(a.status).toBe(404);
90+
expect(a.status).toBe(401);
91+
expect((a.body as any).error.code).toBe('INVALID_SIGNATURE');
9192
});
9293

9394
it('401s a missing or bad signature', async () => {
@@ -115,11 +116,12 @@ describe('ApiTrigger', () => {
115116
expect(queue.subscribed).toEqual([]);
116117
expect(logger.info).not.toHaveBeenCalledWith(expect.stringContaining('armed:'));
117118

118-
// A post to the flow finds no hook: the same 404 an unknown flow
119-
// gets, and nothing reaches the queue or the flow.
119+
// A post to the flow finds no hook: the same refusal an unknown
120+
// flow gets, and nothing reaches the queue or the flow.
120121
const res = await trigger.handleRequest({ flowName: 'lead_intake', hookId: 'default', rawBody: '{"x":1}' });
121-
expect(res.status).toBe(404);
122-
expect(res.body).toEqual({ success: false, error: { code: 'RESOURCE_NOT_FOUND', message: 'No such hook.' } });
122+
expect(res).toEqual(await trigger.handleRequest({ flowName: 'never_armed', hookId: 'default', rawBody: '{"x":1}' }));
123+
expect(res.status).toBe(401);
124+
expect((res.body as any).error.code).toBe('INVALID_SIGNATURE');
123125
expect(queue.published).toEqual([]);
124126
expect(await queue.deliver()).toBe(0);
125127
expect(runs).toEqual([]);
@@ -154,8 +156,12 @@ describe('ApiTrigger', () => {
154156
it('stop() disarms the hook and unsubscribes the queue', async () => {
155157
arm({ hookId: 'hk1', secret: 's3cret' });
156158
trigger.stop('lead_intake');
157-
const res = await trigger.handleRequest({ flowName: 'lead_intake', hookId: 'hk1', rawBody: '{}' });
158-
expect(res.status).toBe(404);
159+
const res = await trigger.handleRequest({
160+
flowName: 'lead_intake', hookId: 'hk1', rawBody: '{}', signatureHeader: sig('s3cret', '{}'),
161+
});
162+
expect(res.status).toBe(401);
163+
expect((res.body as any).error.code).toBe('INVALID_SIGNATURE');
164+
expect(queue.published).toEqual([]);
159165
expect(trigger.listHooks()).toHaveLength(0);
160166
});
161167
});
@@ -213,11 +219,12 @@ describe('ApiTrigger — a secret read at verification time', () => {
213219
}
214220
});
215221

216-
it('an unknown hook still answers 404 before any secret is read (no oracle)', async () => {
222+
it('a wrong hook id is refused before any secret is read', async () => {
217223
let reads = 0;
218224
const { t } = armWith(async () => { reads += 1; return HELD; });
219225
const res = await t.handleRequest({ flowName: 'held_intake', hookId: 'wrong', rawBody: '{}', signatureHeader: sig(HELD, '{}') });
220-
expect(res.status).toBe(404);
226+
expect(res.status).toBe(401);
227+
expect((res.body as any).error.code).toBe('INVALID_SIGNATURE');
221228
expect(reads).toBe(0);
222229
});
223230
});
@@ -357,3 +364,151 @@ describe('ApiTrigger — the timestamped signature form', () => {
357364
expect(verifySignature(SECRET, BODY, sigV1(SECRET, BODY, NOW - WINDOW - 1))).toBe(false);
358365
});
359366
});
367+
368+
/**
369+
* One answer for every post that does not verify, whatever the flow and the
370+
* hook id: an unknown flow, an armed flow under the default hook id, an armed
371+
* flow under a custom hook id posted with the wrong hook id, and each of those
372+
* unsigned or with a signature that does not verify. Every answer is the same
373+
* status and the same body, so the answer does not separate armed flow names
374+
* from unarmed ones. The cause goes to the server log only.
375+
*
376+
* These pins assert the status, the code, and that every answer equals every
377+
* other; the message prose is not pinned. Two CONTROLS beside them: a valid
378+
* post is still accepted under the default and a custom hook id, and the
379+
* `503` for a secret that cannot be read (the declared residual) is unchanged.
380+
*/
381+
describe('ApiTrigger — one answer for every post that does not verify', () => {
382+
const NOW = 1760000000;
383+
const SECRET_DEFAULT = 'pin-uniform-default-61c2';
384+
const SECRET_CUSTOM = 'pin-uniform-custom-9d04';
385+
const BODY = JSON.stringify({ event: 'uniform.pin', marker: 'pin-body-marker-4b7e' });
386+
387+
function sigV1(secret: string, body: string, t: number): string {
388+
return `t=${t},v1=` + createHmac('sha256', secret).update(`${t}.${body}`, 'utf8').digest('hex');
389+
}
390+
391+
let queue: ReturnType<typeof makeFakeQueue>;
392+
let log: { info: ReturnType<typeof vi.fn>; warn: ReturnType<typeof vi.fn> };
393+
let trigger: ApiTrigger;
394+
395+
beforeEach(() => {
396+
vi.useFakeTimers({ toFake: ['Date'] });
397+
vi.setSystemTime(NOW * 1000);
398+
queue = makeFakeQueue();
399+
log = { info: vi.fn(), warn: vi.fn() };
400+
trigger = new ApiTrigger(() => queue, log as any);
401+
trigger.start({ flowName: 'armed_on_default', config: { secret: SECRET_DEFAULT } }, async () => {});
402+
trigger.start({ flowName: 'armed_on_custom', config: { hookId: 'hk_custom', secret: SECRET_CUSTOM } }, async () => {});
403+
});
404+
afterEach(() => {
405+
vi.useRealTimers();
406+
});
407+
408+
/** Where a post is sent, and the cause the log must record whatever it is signed with (`null`: the signature's). */
409+
const ADDRESSES = [
410+
['an unknown flow', 'never_armed', 'default', 'unknown-flow'],
411+
['an unknown flow, under a custom-looking hook id', 'never_armed', 'hk_custom', 'unknown-flow'],
412+
['an armed flow under the default hook id', 'armed_on_default', 'default', null],
413+
['an armed flow under a custom hook id, posted with the wrong hook id', 'armed_on_custom', 'default', 'wrong-hook-id'],
414+
['an armed flow under a custom hook id, posted with its hook id', 'armed_on_custom', 'hk_custom', null],
415+
] as const;
416+
417+
/** A signature that does not verify for any of the addresses, and the cause it records. */
418+
const BAD_SIGNATURES = [
419+
['unsigned', undefined, 'signature-absent'],
420+
['a malformed signature', 'not-a-signature-value', 'signature-malformed'],
421+
['a body-only signature under another secret', sig('not-the-secret', BODY), 'signature-mismatch'],
422+
['a timestamped signature under another secret', sigV1('not-the-secret', BODY, NOW), 'signature-mismatch'],
423+
] as const;
424+
425+
/** Posts that carry the RIGHT secret for some flow and still must not verify. */
426+
const SIGNED_BUT_UNVERIFIED = [
427+
['the right secret, posted with the wrong hook id', 'armed_on_custom', 'default', sig(SECRET_CUSTOM, BODY), 'wrong-hook-id'],
428+
['another flow\'s secret, posted to an unknown flow', 'never_armed', 'default', sig(SECRET_DEFAULT, BODY), 'unknown-flow'],
429+
['another flow\'s secret, posted to an armed flow', 'armed_on_default', 'default', sig(SECRET_CUSTOM, BODY), 'signature-mismatch'],
430+
['the right secret, with a timestamp outside the window', 'armed_on_default', 'default', sigV1(SECRET_DEFAULT, BODY, NOW - 3600), 'signature-outside-window'],
431+
] as const;
432+
433+
type Case = { label: string; flowName: string; hookId: string; header: string | undefined; cause: string };
434+
const CASES: Case[] = [
435+
...ADDRESSES.flatMap(([where, flowName, hookId, addressCause]) =>
436+
BAD_SIGNATURES.map(([how, header, signatureCause]) => ({
437+
label: `${where}, ${how}`, flowName, hookId, header, cause: addressCause ?? signatureCause,
438+
}))),
439+
...SIGNED_BUT_UNVERIFIED.map(([label, flowName, hookId, header, cause]) => ({ label, flowName, hookId, header, cause })),
440+
];
441+
442+
const post = (c: Case) =>
443+
trigger.handleRequest({ flowName: c.flowName, hookId: c.hookId, rawBody: BODY, signatureHeader: c.header });
444+
445+
/** The `cause` of every refusal line logged so far. */
446+
const refusalCauses = () =>
447+
log.warn.mock.calls
448+
.map((call) => call[1] as { cause?: unknown } | undefined)
449+
.filter((meta) => meta != null && typeof meta === 'object' && 'cause' in meta)
450+
.map((meta) => meta!.cause);
451+
452+
it('every unverified post gets the same status and the same envelope, and nothing is enqueued', async () => {
453+
expect(CASES).toHaveLength(ADDRESSES.length * BAD_SIGNATURES.length + SIGNED_BUT_UNVERIFIED.length);
454+
const answers = [];
455+
for (const c of CASES) answers.push({ label: c.label, answer: await post(c) });
456+
457+
const [first] = answers;
458+
expect(first!.answer.status).toBe(401);
459+
expect(first!.answer.body.success).toBe(false);
460+
expect((first!.answer.body as any).error.code).toBe('INVALID_SIGNATURE');
461+
for (const { label, answer } of answers) {
462+
expect(answer, label).toEqual(first!.answer);
463+
}
464+
expect(queue.published).toEqual([]);
465+
});
466+
467+
it('the server log records the real cause of each refusal, once, and never the secret, the signature or the body', async () => {
468+
for (const c of CASES) {
469+
log.warn.mockClear();
470+
await post(c);
471+
expect(refusalCauses(), c.label).toEqual([c.cause]);
472+
const logged = JSON.stringify([...log.warn.mock.calls, ...log.info.mock.calls]);
473+
for (const secret of [SECRET_DEFAULT, SECRET_CUSTOM]) {
474+
expect(logged.includes(secret), `${c.label}: the secret`).toBe(false);
475+
}
476+
if (c.header) expect(logged.includes(c.header), `${c.label}: the signature value`).toBe(false);
477+
expect(logged.includes('pin-body-marker-4b7e'), `${c.label}: the body`).toBe(false);
478+
}
479+
});
480+
481+
it('CONTROL: a valid post is still accepted under the default hook id and under a custom one, in both signature forms', async () => {
482+
for (const [flowName, hookId, secret] of [
483+
['armed_on_default', 'default', SECRET_DEFAULT],
484+
['armed_on_custom', 'hk_custom', SECRET_CUSTOM],
485+
] as const) {
486+
for (const header of [sigV1(secret, BODY, NOW), sig(secret, BODY)]) {
487+
const res = await trigger.handleRequest({ flowName, hookId, rawBody: BODY, signatureHeader: header });
488+
expect(res.status, `${flowName}/${hookId}`).toBe(202);
489+
expect(res.body.accepted).toBe(true);
490+
}
491+
}
492+
expect(queue.published).toHaveLength(4);
493+
expect(refusalCauses()).toEqual([]);
494+
});
495+
496+
it('CONTROL: a secret that cannot be read still answers 503 for its flow and hook id, signed or not; a wrong hook id never reads it', async () => {
497+
let reads = 0;
498+
trigger.start(
499+
{ flowName: 'armed_unreadable', config: {}, resolveSecret: async () => { reads += 1; throw new Error('store offline'); } },
500+
async () => {},
501+
);
502+
for (const header of [undefined, sig('not-the-secret', BODY)]) {
503+
const res = await trigger.handleRequest({ flowName: 'armed_unreadable', hookId: 'default', rawBody: BODY, signatureHeader: header });
504+
expect(res.status).toBe(503);
505+
expect((res.body as any).error.code).toBe('SERVICE_UNAVAILABLE');
506+
}
507+
expect(reads).toBe(2);
508+
509+
const wrongHook = await trigger.handleRequest({ flowName: 'armed_unreadable', hookId: 'other', rawBody: BODY });
510+
expect(wrongHook).toEqual(await post(CASES[0]!));
511+
expect(reads).toBe(2);
512+
expect(queue.published).toEqual([]);
513+
});
514+
});

0 commit comments

Comments
 (0)