diff --git a/.changeset/22826-refuse-body-only-signature.md b/.changeset/22826-refuse-body-only-signature.md new file mode 100644 index 00000000000..33f795a896d --- /dev/null +++ b/.changeset/22826-refuse-body-only-signature.md @@ -0,0 +1,25 @@ +--- +'@objectstack/trigger-api': major +'@objectstack/spec': patch +--- + +feat(trigger-api)!: the inbound hook door refuses the body-only `x-objectstack-signature` form, so only the timestamped, replay-protected form verifies + +Clause-②: no (narrowing) + + + +**BREAKING**: an accept-set narrowing on a runtime door, shipped as `major` on the v18 line (`.changeset/pre.json` is open on `main` in `next` pre mode, so the release is `18.0.0-next.*`). + +**What narrows, from → to.** + +- `POST /api/v1/automation/hooks/:flowName/:hookId`, through the self-hosted mount and through `ITriggerApiService.handleInboundHook` alike. A post whose `x-objectstack-signature` was the body-only value (`sha256=` and the hex HMAC-SHA256 of the raw body under the flow's secret) was accepted, `202` and enqueued, with a deprecation `warn` line once per armed hook. It now gets the same `401 INVALID_SIGNATURE` every post that does not verify gets, and nothing is enqueued, even when the value is the secret's HMAC of the body. The body-only form signs no time, so it carries no replay protection. +- The one accepted form is the timestamped one: `t=,v1=`, the HMAC-SHA256 of `.` under the flow's secret, refused when `t` is more than 300 seconds from the server clock. +- The deprecation line is gone. The refusal's `warn` line carries the cause `signature-body-only`, beside `unknown-flow`, `wrong-hook-id`, `signature-absent`, `signature-malformed`, `signature-mismatch` and `signature-outside-window`. It never carries the secret, the signature value or the body. +- `verifySignature(secret, rawBody, header)`, exported by `@objectstack/trigger-api`, answers as the door does: `true` only for the timestamped form inside its window, `false` for a body-only value. Its signature is unchanged. + +**Unchanged: the outbound contract.** `@objectstack/core`'s `verifyHttpSignature` still verifies both forms, and `signHttpBody` still writes the body-only value: the messaging outbox and the flow `http` node sign it for third-party receivers, as the outbound webhook contract publishes. Only this inbound door refuses it. + +**Who sees a difference.** A sender that signs the timestamped form, as the inbound-hook guidance teaches (the `objectstack-automation` skill's Inbound webhook section, `os explain flow-api-trigger-secret-missing`, `signHttpBodyAt` in `@objectstack/core`), sees none. A sender still on the body-only value now gets `401`, and the trigger's `warn` line for its post names `signature-body-only`, which tells an operator its secret is right and its signing form is the retired one. + +**`@objectstack/spec`.** The `ITriggerApiService.handleInboundHook` docblock names the timestamped form as the only one accepted and the body-only form as refused. The type is unchanged. diff --git a/packages/qa/dogfood/test/flow-credential-channel.dogfood.test.ts b/packages/qa/dogfood/test/flow-credential-channel.dogfood.test.ts index 5ec39524e63..bb3e9e3a6d4 100644 --- a/packages/qa/dogfood/test/flow-credential-channel.dogfood.test.ts +++ b/packages/qa/dogfood/test/flow-credential-channel.dogfood.test.ts @@ -89,7 +89,11 @@ function inbound(name: string, secrets: { hook?: string; sign?: string } = {}, l }; } -const sign = (secret: string, body: string) => 'sha256=' + createHmac('sha256', secret).update(body, 'utf8').digest('hex'); +/** The timestamped `x-objectstack-signature`, the one form the inbound door accepts, signed now from the published recipe. */ +const sign = (secret: string, body: string) => { + const t = Math.floor(Date.now() / 1000); + return `t=${t},v1=` + createHmac('sha256', secret).update(`${t}.${body}`, 'utf8').digest('hex'); +}; const rowsOf = (r: any): any[] => (Array.isArray(r) ? r : Array.isArray(r?.records) ? r.records : Array.isArray(r?.value) ? r.value : []); const hasAny = (v: unknown, secrets: readonly string[]) => secrets.some((s) => JSON.stringify(v ?? null).includes(s)); diff --git a/packages/runtime/src/domains/automation-hooks.test.ts b/packages/runtime/src/domains/automation-hooks.test.ts index abf03a25166..5a0cbd72ddf 100644 --- a/packages/runtime/src/domains/automation-hooks.test.ts +++ b/packages/runtime/src/domains/automation-hooks.test.ts @@ -11,8 +11,9 @@ * is a test double, because the real one is a separate card (#22774), but its * verifier is not: it reads the raw body off the `Request` it is handed and * checks the `x-objectstack-signature` with `verifyHttpSignature`, the very - * function `ApiTrigger.handleRequest` verifies with (`@objectstack/core`), and - * answers every post that does not verify with the trigger's one refusal. So a + * function `ApiTrigger.handleRequest` verifies with (`@objectstack/core`), + * accepts the timestamped form only, as that door does, and answers every + * post that does not verify with the trigger's one refusal. So a * domain that handed it anything but the transport's own unread request, or a * re-serialised body, would be caught by the verifier, not by an assertion * about plumbing. @@ -43,7 +44,7 @@ */ import { describe, it, expect, vi } from 'vitest'; -import { isAuthGateAllowlisted, matchInboundHookRoute, signHttpBody, verifyHttpSignature } from '@objectstack/core'; +import { isAuthGateAllowlisted, matchInboundHookRoute, signHttpBodyAt, verifyHttpSignature } from '@objectstack/core'; import { HttpDispatcher } from '../http-dispatcher.js'; import type { HttpProtocolContext, KernelResolver } from '../http-dispatcher.js'; import { createDispatcherPlugin } from '../dispatcher-plugin.js'; @@ -61,11 +62,15 @@ const JSON_HEADERS = { 'Content-Type': 'application/json' }; const json = (status: number, body: unknown) => new Response(JSON.stringify(body), { status, headers: JSON_HEADERS }); const REFUSAL = { success: false, error: { code: 'INVALID_SIGNATURE', message: 'Signature verification failed.' } }; +/** The timestamped signature form, the one the inbound door accepts, signed now. */ +const sign = (body: string, secret: string) => signHttpBodyAt(body, secret, Math.floor(Date.now() / 1000)); + /** * A `handleInboundHook` member with the trigger's verifier inside it: the hook * armed for each flow, the raw body read once off the request, the signature - * checked by core's `verifyHttpSignature`, one refusal for every post that does - * not verify, `202` for one that does. It records every call. + * checked by core's `verifyHttpSignature` and accepted in the timestamped form + * only, one refusal for every post that does not verify, `202` for one that + * does. It records every call. */ function verifierMember(armed: Record = { [FLOW]: { hookId: HOOK, secret: SECRET } }) { const calls: Array<{ request: Request; hook: { flowName: string; hookId: string }; body: string }> = []; @@ -77,7 +82,7 @@ function verifierMember(armed: Record ({ request }); function post(path = ROUTE, opts: { body?: string; signature?: string | null; method?: string } = {}) { const body = opts.body ?? BODY; - const signature = opts.signature === undefined ? signHttpBody(body, SECRET) : opts.signature; + const signature = opts.signature === undefined ? sign(body, SECRET) : opts.signature; return new Request(`${ORIGIN}/api/v1${path}`, { method: opts.method ?? 'POST', headers: { ...JSON_HEADERS, ...(signature !== null ? { 'x-objectstack-signature': signature } : {}) }, @@ -151,8 +156,8 @@ describe('[#22773] the inbound hook reaches the verifier on a kernel with no htt const member = verifierMember(); const { dispatcher } = makeDispatcher({ 'trigger-api': { handleInboundHook: member.handleInboundHook } }); const cases: Array<[string, Request, string]> = [ - ['wrong secret', post(ROUTE, { signature: signHttpBody(BODY, 'not-the-secret') }), ROUTE], - ['body changed after signing', post(ROUTE, { body: `${BODY} `, signature: signHttpBody(BODY, SECRET) }), ROUTE], + ['wrong secret', post(ROUTE, { signature: sign(BODY, 'not-the-secret') }), ROUTE], + ['body changed after signing', post(ROUTE, { body: `${BODY} `, signature: sign(BODY, SECRET) }), ROUTE], ['unsigned', post(ROUTE, { signature: null }), ROUTE], ['malformed signature', post(ROUTE, { signature: 'sha256=zz' }), ROUTE], ['unknown flow', post('/automation/hooks/no_such_flow/default'), '/automation/hooks/no_such_flow/default'], @@ -177,7 +182,7 @@ describe('[#22773] the inbound hook reaches the verifier on a kernel with no htt const signed = await dispatcher.dispatch('POST', ROUTE, {}, {}, ctx(post())); expect((signed.result as Response).status).toBe(202); - const forged = await dispatcher.dispatch('POST', ROUTE, {}, {}, ctx(post(ROUTE, { signature: signHttpBody(BODY, 'session-is-no-credential') }))); + const forged = await dispatcher.dispatch('POST', ROUTE, {}, {}, ctx(post(ROUTE, { signature: sign(BODY, 'session-is-no-credential') }))); expect((forged.result as Response).status).toBe(401); expect(member.handleInboundHook).toHaveBeenCalledTimes(2); }); diff --git a/packages/spec/src/contracts/trigger-api-service.ts b/packages/spec/src/contracts/trigger-api-service.ts index 0a156ddfaa0..f1cd8b8a036 100644 --- a/packages/spec/src/contracts/trigger-api-service.ts +++ b/packages/spec/src/contracts/trigger-api-service.ts @@ -54,14 +54,19 @@ export interface ITriggerApiService { * id gets this `503`, signed or not. 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. - * - It verifies the `x-objectstack-signature` header (`sha256=` and the hex - * HMAC-SHA256 of the raw body under that secret). A missing or wrong - * signature gets the unverified-post refusal below. + * - It verifies the `x-objectstack-signature` header in its timestamped + * form, the only one it accepts: `t=,v1=`, where the + * hex is the HMAC-SHA256 of `.` under that secret, and a `t` + * more than 300 seconds from the server's clock is refused. A missing or + * wrong signature, one outside that window, and one in the body-only form + * (`sha256=` and the hex HMAC-SHA256 of the raw body alone, which signs + * no time; refused even when it is the secret's HMAC of the body) get the + * unverified-post refusal below. * - **One refusal for every post that does not verify** (#22806): an * unknown flow, a wrong hook id, an absent signature and a bad one all * answer `401 INVALID_SIGNATURE`, with one body. So the answer does not * tell an armed flow name from an unarmed one, whichever hook id a flow - * is armed with. The cause (which of those four) goes to the server log, + * is armed with. The cause (which of those) goes to the server log, * ⛔ never to the sender. FROM → TO: an unknown flow and a wrong hook id * used to answer `404 RESOURCE_NOT_FOUND`; they now answer the same * `401 INVALID_SIGNATURE` an absent or bad signature always has. @@ -88,9 +93,9 @@ export interface ITriggerApiService { * again differs from them (key order, whitespace, escapes), so it would * fail verification for every real sender. ⛔ Nor does the caller check, * pre-filter or short-circuit the signature: there is one verifier, and it - * is behind this member. Anything a later signing scheme adds (a - * timestamp, for one) arrives in the same forwarded request, so this - * signature gains no field for it. + * is behind this member. Anything the signing scheme carries beyond the + * body (the accepted form's timestamp, for one) arrives in the same + * forwarded request, so this signature gains no field for it. * * **The signature is the only credential.** This member takes no * `ExecutionContext` and reads no session, cookie or `Authorization` header: diff --git a/packages/triggers/trigger-api/src/api-trigger.test.ts b/packages/triggers/trigger-api/src/api-trigger.test.ts index 5f7a507ea09..9880c725a23 100644 --- a/packages/triggers/trigger-api/src/api-trigger.test.ts +++ b/packages/triggers/trigger-api/src/api-trigger.test.ts @@ -2,6 +2,7 @@ import { createHmac } from 'node:crypto'; import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { signHttpBody, verifyHttpSignature } from '@objectstack/core'; import { ApiTrigger, verifySignature } from './api-trigger.js'; import type { QueueServiceSurface } from './api-trigger.js'; @@ -39,10 +40,33 @@ function makeFakeQueue() { const logger = { info: vi.fn(), warn: vi.fn() }; -function sig(secret: string, body: string): string { +/** + * The two signature forms, derived with `createHmac` directly from the + * published recipes rather than through `@objectstack/core`: a third-party + * sender computes them on its own, so these pins check the receiver against + * the wire form, not against the function it verifies with. + * + * timestamped (the one this door accepts): + * x-objectstack-signature: t=,v1=."> + * body-only (refused at this door; still what our outbound senders sign): + * x-objectstack-signature: sha256= + */ +function sigV1(secret: string, body: string, t: number = Math.floor(Date.now() / 1000)): string { + return `t=${t},v1=` + createHmac('sha256', secret).update(`${t}.${body}`, 'utf8').digest('hex'); +} + +function sigBodyOnly(secret: string, body: string): string { return 'sha256=' + createHmac('sha256', secret).update(body, 'utf8').digest('hex'); } +/** The `cause` of every refusal line `log` has recorded so far (the structured field, never the prose). */ +function refusalCausesOf(log: { warn: ReturnType }): unknown[] { + return log.warn.mock.calls + .map((call) => call[1] as { cause?: unknown } | undefined) + .filter((meta) => meta != null && typeof meta === 'object' && 'cause' in meta) + .map((meta) => meta!.cause); +} + describe('ApiTrigger', () => { let queue: ReturnType; let trigger: ApiTrigger; @@ -59,19 +83,20 @@ describe('ApiTrigger', () => { trigger.start({ flowName: 'lead_intake', config }, async (ctx) => { runs.push(ctx); }); } - it('verifySignature accepts a correct GitHub-style signature and rejects others', () => { + it('verifySignature accepts a correct timestamped signature and rejects the rest, the right secret\'s body-only one included', () => { const body = '{"a":1}'; - expect(verifySignature('s3cret', body, sig('s3cret', body))).toBe(true); - expect(verifySignature('s3cret', body, sig('wrong', body))).toBe(false); + expect(verifySignature('s3cret', body, sigV1('s3cret', body))).toBe(true); + expect(verifySignature('s3cret', body, sigV1('wrong', body))).toBe(false); expect(verifySignature('s3cret', body, undefined)).toBe(false); expect(verifySignature('s3cret', body, 'sha256=zz')).toBe(false); + expect(verifySignature('s3cret', body, sigBodyOnly('s3cret', body))).toBe(false); }); it('202-enqueues a valid post and the consumer runs the flow with the payload as record', async () => { arm({ hookId: 'hk1', secret: 's3cret' }); const body = JSON.stringify({ title: 'New lead', amount: 5000 }); const res = await trigger.handleRequest({ - flowName: 'lead_intake', hookId: 'hk1', rawBody: body, signatureHeader: sig('s3cret', body), + flowName: 'lead_intake', hookId: 'hk1', rawBody: body, signatureHeader: sigV1('s3cret', body), }); expect(res.status).toBe(202); expect(res.body.accepted).toBe(true); @@ -96,7 +121,7 @@ describe('ApiTrigger', () => { const body = '{"x":1}'; expect((await trigger.handleRequest({ flowName: 'lead_intake', hookId: 'hk1', rawBody: body })).status).toBe(401); expect((await trigger.handleRequest({ - flowName: 'lead_intake', hookId: 'hk1', rawBody: body, signatureHeader: sig('other', body), + flowName: 'lead_intake', hookId: 'hk1', rawBody: body, signatureHeader: sigV1('other', body), })).status).toBe(401); }); @@ -131,17 +156,17 @@ describe('ApiTrigger', () => { it('400s non-object or invalid JSON bodies', async () => { arm({ secret: 's3cret' }); expect((await trigger.handleRequest({ - flowName: 'lead_intake', hookId: 'default', rawBody: 'not json', signatureHeader: sig('s3cret', 'not json'), + flowName: 'lead_intake', hookId: 'default', rawBody: 'not json', signatureHeader: sigV1('s3cret', 'not json'), })).status).toBe(400); expect((await trigger.handleRequest({ - flowName: 'lead_intake', hookId: 'default', rawBody: '[1,2]', signatureHeader: sig('s3cret', '[1,2]'), + flowName: 'lead_intake', hookId: 'default', rawBody: '[1,2]', signatureHeader: sigV1('s3cret', '[1,2]'), })).status).toBe(400); }); it('passes x-idempotency-key through to the queue dedup window', async () => { arm({ secret: 's3cret' }); await trigger.handleRequest({ - flowName: 'lead_intake', hookId: 'default', rawBody: '{}', signatureHeader: sig('s3cret', '{}'), idempotencyKey: 'evt_42', + flowName: 'lead_intake', hookId: 'default', rawBody: '{}', signatureHeader: sigV1('s3cret', '{}'), idempotencyKey: 'evt_42', }); expect(queue.published[0].idempotencyKey).toBe('evt_42'); }); @@ -149,7 +174,7 @@ describe('ApiTrigger', () => { it('503s when no queue service is registered', async () => { const t = new ApiTrigger(() => null, logger as any); t.start({ flowName: 'f', config: { secret: 's3cret' } }, async () => {}); - const res = await t.handleRequest({ flowName: 'f', hookId: 'default', rawBody: '{}', signatureHeader: sig('s3cret', '{}') }); + const res = await t.handleRequest({ flowName: 'f', hookId: 'default', rawBody: '{}', signatureHeader: sigV1('s3cret', '{}') }); expect(res.status).toBe(503); }); @@ -157,7 +182,7 @@ describe('ApiTrigger', () => { arm({ hookId: 'hk1', secret: 's3cret' }); trigger.stop('lead_intake'); const res = await trigger.handleRequest({ - flowName: 'lead_intake', hookId: 'hk1', rawBody: '{}', signatureHeader: sig('s3cret', '{}'), + flowName: 'lead_intake', hookId: 'hk1', rawBody: '{}', signatureHeader: sigV1('s3cret', '{}'), }); expect(res.status).toBe(401); expect((res.body as any).error.code).toBe('INVALID_SIGNATURE'); @@ -182,7 +207,7 @@ describe('ApiTrigger — a secret read at verification time', () => { const t = new ApiTrigger(() => queue, logger as any); t.start({ flowName: 'held_intake', config, resolveSecret }, async () => {}); const post = (secret: string) => - t.handleRequest({ flowName: 'held_intake', hookId: 'default', rawBody: '{"a":1}', signatureHeader: sig(secret, '{"a":1}') }); + t.handleRequest({ flowName: 'held_intake', hookId: 'default', rawBody: '{"a":1}', signatureHeader: sigV1(secret, '{"a":1}') }); return { t, queue, post }; } @@ -222,7 +247,7 @@ describe('ApiTrigger — a secret read at verification time', () => { it('a wrong hook id is refused before any secret is read', async () => { let reads = 0; const { t } = armWith(async () => { reads += 1; return HELD; }); - const res = await t.handleRequest({ flowName: 'held_intake', hookId: 'wrong', rawBody: '{}', signatureHeader: sig(HELD, '{}') }); + const res = await t.handleRequest({ flowName: 'held_intake', hookId: 'wrong', rawBody: '{}', signatureHeader: sigV1(HELD, '{}') }); expect(res.status).toBe(401); expect((res.body as any).error.code).toBe('INVALID_SIGNATURE'); expect(reads).toBe(0); @@ -230,28 +255,20 @@ describe('ApiTrigger — a secret read at verification time', () => { }); /** - * The timestamped signature form beside the body-only one. A third-party - * sender computes it on its own, so the helper below derives it with - * `createHmac` directly from the published recipe rather than through - * `@objectstack/core`: these pins check the receiver against the wire form, - * not against the function it verifies with. - * - * x-objectstack-signature: t=,v1=."> + * The timestamped signature form, the one this door accepts, and the + * body-only form, which it refuses. The helpers at the top of this file + * derive both from the published recipes (`sigV1`, `sigBodyOnly`). * * Every refusal asserts the whole bad-signature envelope — `401`, code * `INVALID_SIGNATURE` — and that nothing reached the queue. */ -describe('ApiTrigger — the timestamped signature form', () => { +describe('ApiTrigger — the timestamped signature form, and the body-only form refused', () => { const NOW = 1760000000; const WINDOW = 300; const SECRET = 'pin-trigger-v1-3f9a'; const BODY = JSON.stringify({ event: 'invoice.paid', id: 'evt_77' }); const REFUSED = { status: 401, body: { success: false, error: { code: 'INVALID_SIGNATURE', message: 'Signature verification failed.' } } }; - function sigV1(secret: string, body: string, t: number): string { - return `t=${t},v1=` + createHmac('sha256', secret).update(`${t}.${body}`, 'utf8').digest('hex'); - } - let queue: ReturnType; let log: { info: ReturnType; warn: ReturnType }; let trigger: ApiTrigger; @@ -273,20 +290,16 @@ describe('ApiTrigger — the timestamped signature form', () => { const post = (signatureHeader: string | undefined, rawBody = BODY) => trigger.handleRequest({ flowName: 'invoice_intake', hookId: 'hk_v1', rawBody, signatureHeader }); - /** The deprecation lines this trigger has logged so far. */ - const deprecationLines = () => - log.warn.mock.calls.map((c) => String(c[0])).filter((line) => line.includes('deprecated')); - - it('accepts a fresh timestamped post: 202, enqueued, the flow runs — and no deprecation line', async () => { + it('CONTROL: accepts a fresh timestamped post: 202, enqueued, the flow runs, and nothing is logged as refused', async () => { const res = await post(sigV1(SECRET, BODY, NOW)); expect(res.status).toBe(202); expect(res.body).toEqual({ accepted: true, messageId: 'msg_1' }); expect(await queue.deliver()).toBe(1); expect(runs[0].record).toEqual({ event: 'invoice.paid', id: 'evt_77' }); - expect(deprecationLines()).toEqual([]); + expect(refusalCausesOf(log)).toEqual([]); }); - it('refuses a replay outside the window: the post it accepted, sent again later, is a 401 and is not enqueued', async () => { + it('CONTROL: refuses a replay outside the window: the post it accepted, sent again later, is a 401 and is not enqueued', async () => { const header = sigV1(SECRET, BODY, NOW); expect((await post(header)).status).toBe(202); expect(queue.published).toHaveLength(1); @@ -296,7 +309,7 @@ describe('ApiTrigger — the timestamped signature form', () => { expect(queue.published).toHaveLength(1); }); - it('refuses a stale timestamp the HMAC genuinely covers, and a future one past the window; accepts the edges', async () => { + it('CONTROL: refuses a stale timestamp the HMAC genuinely covers, and a future one past the window; accepts the edges', async () => { expect(await post(sigV1(SECRET, BODY, NOW - WINDOW - 1))).toEqual(REFUSED); expect(await post(sigV1(SECRET, BODY, NOW + WINDOW + 1))).toEqual(REFUSED); expect(queue.published).toEqual([]); @@ -305,13 +318,12 @@ describe('ApiTrigger — the timestamped signature form', () => { }); it('refuses a stale timestamp carried with a valid BODY-ONLY signature — a timestamp claims the versioned form, with no fallback', async () => { - const bodyOnlyHex = sig(SECRET, BODY).slice('sha256='.length); + const bodyOnlyHex = sigBodyOnly(SECRET, BODY).slice('sha256='.length); for (const t of [NOW - WINDOW - 1, NOW - 3600, NOW]) { expect(await post(`t=${t},v1=${bodyOnlyHex}`), `v1 at ${t}`).toEqual(REFUSED); expect(await post(`t=${t},sha256=${bodyOnlyHex}`), `sha256 at ${t}`).toEqual(REFUSED); } expect(queue.published).toEqual([]); - expect(deprecationLines()).toEqual([]); }); it('refuses a malformed timestamped value with the same envelope a bad signature gets', async () => { @@ -328,7 +340,7 @@ describe('ApiTrigger — the timestamped signature form', () => { expect(await post(malformed), malformed).toEqual(REFUSED); } // The same envelope as a plain bad signature. - expect(await post(sig('not-the-secret', BODY))).toEqual(REFUSED); + expect(await post(sigBodyOnly('not-the-secret', BODY))).toEqual(REFUSED); expect(queue.published).toEqual([]); }); @@ -338,31 +350,42 @@ describe('ApiTrigger — the timestamped signature form', () => { expect(queue.published).toEqual([]); }); - it('CONTROL: still accepts a body-only signature, and logs its deprecation once per armed hook, naming the flow and the hook', async () => { - expect((await post(sig(SECRET, BODY))).status).toBe(202); - expect((await post(sig(SECRET, BODY))).status).toBe(202); - expect(queue.published).toHaveLength(2); + it('refuses a body-only signature that IS the secret\'s HMAC of the body: the bad-signature envelope, nothing enqueued, one log line per post (the refusal, cause signature-body-only)', async () => { + const bodyOnly = sigBodyOnly(SECRET, BODY); + // The value is genuinely right for the secret: core's verifier, the + // receiver half of the outbound form, still verifies it. The refusal + // is the door's policy, not a broken HMAC. + expect(verifyHttpSignature(BODY, SECRET, bodyOnly)).toEqual({ valid: true, scheme: 'body-only' }); - const lines = deprecationLines(); - expect(lines).toHaveLength(1); - expect(lines[0]).toContain("flow 'invoice_intake'"); - expect(lines[0]).toContain("hook 'hk_v1'"); - expect(lines[0]).toContain('t=,v1='); + for (const attempt of ['first post', 'the same post again']) { + log.warn.mockClear(); + expect(await post(bodyOnly), attempt).toEqual(REFUSED); + // Exactly one warn line, and it is the refusal: no deprecation line beside it. + expect(log.warn, attempt).toHaveBeenCalledTimes(1); + expect(refusalCausesOf(log), attempt).toEqual(['signature-body-only']); + } + expect(queue.published).toEqual([]); + expect(await queue.deliver()).toBe(0); + expect(runs).toEqual([]); - // A timestamped post adds nothing to it; re-arming the hook says it once more. + // CONTROL: the same secret over the same body, in the timestamped form, is accepted. expect((await post(sigV1(SECRET, BODY, NOW))).status).toBe(202); - expect(deprecationLines()).toHaveLength(1); - trigger.stop('invoice_intake'); - trigger.start({ flowName: 'invoice_intake', config: { hookId: 'hk_v1', secret: SECRET } }, async () => {}); - expect((await post(sig(SECRET, BODY))).status).toBe(202); - expect(deprecationLines()).toHaveLength(2); + expect(queue.published).toHaveLength(1); }); - it('verifySignature, the exported check, accepts both forms and refuses a stale one', () => { + it('verifySignature, the exported check, accepts the timestamped form only: the right secret\'s body-only value and a stale one are refused', () => { expect(verifySignature(SECRET, BODY, sigV1(SECRET, BODY, NOW))).toBe(true); - expect(verifySignature(SECRET, BODY, sig(SECRET, BODY))).toBe(true); + expect(verifySignature(SECRET, BODY, sigBodyOnly(SECRET, BODY))).toBe(false); expect(verifySignature(SECRET, BODY, sigV1(SECRET, BODY, NOW - WINDOW - 1))).toBe(false); }); + + it('CONTROL: the outbound form is unchanged — core\'s signHttpBody still writes the body-only value, byte for byte, and core still verifies it', () => { + // Ruling B keeps the outbound contract as published: the messaging + // outbox and the flow `http` node sign body-only for third-party + // receivers. Only this inbound door refuses that form. + expect(signHttpBody(BODY, SECRET)).toBe(sigBodyOnly(SECRET, BODY)); + expect(verifyHttpSignature(BODY, SECRET, signHttpBody(BODY, SECRET))).toEqual({ valid: true, scheme: 'body-only' }); + }); }); /** @@ -373,6 +396,9 @@ describe('ApiTrigger — the timestamped signature form', () => { * status and the same body, so the answer does not separate armed flow names * from unarmed ones. The cause goes to the server log only. * + * A body-only signature that is the right secret's HMAC of the body is one + * of them: it gets the same answer, and the log records its own cause. + * * These pins assert the status, the code, and that every answer equals every * other; the message prose is not pinned. Two CONTROLS beside them: a valid * post is still accepted under the default and a custom hook id, and the @@ -384,10 +410,6 @@ describe('ApiTrigger — one answer for every post that does not verify', () => const SECRET_CUSTOM = 'pin-uniform-custom-9d04'; const BODY = JSON.stringify({ event: 'uniform.pin', marker: 'pin-body-marker-4b7e' }); - function sigV1(secret: string, body: string, t: number): string { - return `t=${t},v1=` + createHmac('sha256', secret).update(`${t}.${body}`, 'utf8').digest('hex'); - } - let queue: ReturnType; let log: { info: ReturnType; warn: ReturnType }; let trigger: ApiTrigger; @@ -418,16 +440,18 @@ describe('ApiTrigger — one answer for every post that does not verify', () => const BAD_SIGNATURES = [ ['unsigned', undefined, 'signature-absent'], ['a malformed signature', 'not-a-signature-value', 'signature-malformed'], - ['a body-only signature under another secret', sig('not-the-secret', BODY), 'signature-mismatch'], + ['a body-only signature under another secret', sigBodyOnly('not-the-secret', BODY), 'signature-mismatch'], ['a timestamped signature under another secret', sigV1('not-the-secret', BODY, NOW), 'signature-mismatch'], ] as const; /** Posts that carry the RIGHT secret for some flow and still must not verify. */ const SIGNED_BUT_UNVERIFIED = [ - ['the right secret, posted with the wrong hook id', 'armed_on_custom', 'default', sig(SECRET_CUSTOM, BODY), 'wrong-hook-id'], - ['another flow\'s secret, posted to an unknown flow', 'never_armed', 'default', sig(SECRET_DEFAULT, BODY), 'unknown-flow'], - ['another flow\'s secret, posted to an armed flow', 'armed_on_default', 'default', sig(SECRET_CUSTOM, BODY), 'signature-mismatch'], + ['the right secret, posted with the wrong hook id', 'armed_on_custom', 'default', sigV1(SECRET_CUSTOM, BODY, NOW), 'wrong-hook-id'], + ['another flow\'s secret, posted to an unknown flow', 'never_armed', 'default', sigV1(SECRET_DEFAULT, BODY, NOW), 'unknown-flow'], + ['another flow\'s secret, posted to an armed flow', 'armed_on_default', 'default', sigV1(SECRET_CUSTOM, BODY, NOW), 'signature-mismatch'], ['the right secret, with a timestamp outside the window', 'armed_on_default', 'default', sigV1(SECRET_DEFAULT, BODY, NOW - 3600), 'signature-outside-window'], + ['the right secret, in the body-only form', 'armed_on_default', 'default', sigBodyOnly(SECRET_DEFAULT, BODY), 'signature-body-only'], + ['the right secret, in the body-only form, under a custom hook id', 'armed_on_custom', 'hk_custom', sigBodyOnly(SECRET_CUSTOM, BODY), 'signature-body-only'], ] as const; type Case = { label: string; flowName: string; hookId: string; header: string | undefined; cause: string }; @@ -443,11 +467,7 @@ describe('ApiTrigger — one answer for every post that does not verify', () => trigger.handleRequest({ flowName: c.flowName, hookId: c.hookId, rawBody: BODY, signatureHeader: c.header }); /** The `cause` of every refusal line logged so far. */ - const refusalCauses = () => - log.warn.mock.calls - .map((call) => call[1] as { cause?: unknown } | undefined) - .filter((meta) => meta != null && typeof meta === 'object' && 'cause' in meta) - .map((meta) => meta!.cause); + const refusalCauses = () => refusalCausesOf(log); it('every unverified post gets the same status and the same envelope, and nothing is enqueued', async () => { expect(CASES).toHaveLength(ADDRESSES.length * BAD_SIGNATURES.length + SIGNED_BUT_UNVERIFIED.length); @@ -478,18 +498,16 @@ describe('ApiTrigger — one answer for every post that does not verify', () => } }); - it('CONTROL: a valid post is still accepted under the default hook id and under a custom one, in both signature forms', async () => { + it('CONTROL: a valid post is still accepted under the default hook id and under a custom one, in the timestamped form', async () => { for (const [flowName, hookId, secret] of [ ['armed_on_default', 'default', SECRET_DEFAULT], ['armed_on_custom', 'hk_custom', SECRET_CUSTOM], ] as const) { - for (const header of [sigV1(secret, BODY, NOW), sig(secret, BODY)]) { - const res = await trigger.handleRequest({ flowName, hookId, rawBody: BODY, signatureHeader: header }); - expect(res.status, `${flowName}/${hookId}`).toBe(202); - expect(res.body.accepted).toBe(true); - } + const res = await trigger.handleRequest({ flowName, hookId, rawBody: BODY, signatureHeader: sigV1(secret, BODY, NOW) }); + expect(res.status, `${flowName}/${hookId}`).toBe(202); + expect(res.body.accepted).toBe(true); } - expect(queue.published).toHaveLength(4); + expect(queue.published).toHaveLength(2); expect(refusalCauses()).toEqual([]); }); @@ -499,7 +517,7 @@ describe('ApiTrigger — one answer for every post that does not verify', () => { flowName: 'armed_unreadable', config: {}, resolveSecret: async () => { reads += 1; throw new Error('store offline'); } }, async () => {}, ); - for (const header of [undefined, sig('not-the-secret', BODY)]) { + for (const header of [undefined, sigBodyOnly('not-the-secret', BODY)]) { const res = await trigger.handleRequest({ flowName: 'armed_unreadable', hookId: 'default', rawBody: BODY, signatureHeader: header }); expect(res.status).toBe(503); expect((res.body as any).error.code).toBe('SERVICE_UNAVAILABLE'); diff --git a/packages/triggers/trigger-api/src/api-trigger.ts b/packages/triggers/trigger-api/src/api-trigger.ts index 7b5457059f5..aff2541dd93 100644 --- a/packages/triggers/trigger-api/src/api-trigger.ts +++ b/packages/triggers/trigger-api/src/api-trigger.ts @@ -78,35 +78,40 @@ interface ArmedHook { secret: HookSecret; queue: string; callback: (ctx: AutomationContext) => Promise; - /** - * Whether this arming has already logged that it accepted a body-only - * signature. The deprecation line is said once per armed hook, never once - * per post, so a sender that keeps posting cannot flood the log with it. - */ - bodyOnlyDeprecationLogged: boolean; } +/** + * Why an inbound post's signature did not verify at this door. The first four + * carry `verifyHttpSignature`'s own `reason`, the one vocabulary for them. + * `signature-body-only` is the one this door adds: core verified the body-only + * form, and this door accepts only the timestamped one ({@link verifyInboundSignature}). + */ +type UnverifiedSignatureCause = + | `signature-${Extract['reason']}` + | 'signature-body-only'; + /** * Why an inbound post did not verify. Recorded in the server log only, ⛔ never * sent: every one of these gets the same answer ({@link unverifiedPostAnswer}), * so an operator can still tell a misconfigured sender's wrong URL from its - * wrong secret while the sender cannot. The `signature-*` kinds carry - * `verifyHttpSignature`'s own `reason`, the one vocabulary for them. + * wrong secret, or from its retired signing form, while the sender cannot. */ -type UnverifiedPostCause = - | 'unknown-flow' - | 'wrong-hook-id' - | `signature-${Extract['reason']}`; +type UnverifiedPostCause = 'unknown-flow' | 'wrong-hook-id' | UnverifiedSignatureCause; const UNVERIFIED_POST_CAUSE_TEXT: Readonly> = { 'unknown-flow': 'no hook is armed for the flow it names', 'wrong-hook-id': 'the hook id it names is not the one its flow is armed with', 'signature-absent': 'it carries no x-objectstack-signature', - 'signature-malformed': 'its x-objectstack-signature has neither accepted form', + 'signature-malformed': 'its x-objectstack-signature is in neither the timestamped form nor the body-only one', 'signature-mismatch': "its x-objectstack-signature is not the hook secret's HMAC of the body it carries", 'signature-outside-window': `its timestamped x-objectstack-signature verified, with a timestamp more than ` + `${HTTP_SIGNATURE_TOLERANCE_SECONDS}s from this server's clock (a replay, or a sender clock that is wrong)`, + 'signature-body-only': + `its x-objectstack-signature is in the body-only form (sha256=), which ` + + `this door does not accept because it signs no time. The sender signs the timestamped form instead: ` + + `t=,v1=.">, refused once t is more than ` + + `${HTTP_SIGNATURE_TOLERANCE_SECONDS}s from this server's clock (@objectstack/core signHttpBodyAt)`, }; /** @@ -133,25 +138,54 @@ function safeEqual(a: string, b: string): boolean { } /** - * GitHub/Stripe-style HMAC verification of `x-objectstack-signature` against - * the shared per-flow secret, through `@objectstack/core`'s - * `verifyHttpSignature` — the one definition of the scheme every ObjectStack + * The inbound door's judgement of a post's `x-objectstack-signature`: the one + * place it is made, for {@link ApiTrigger.handleRequest} and + * {@link verifySignature} alike. The HMAC is checked by `@objectstack/core`'s + * `verifyHttpSignature`, the one definition of the scheme every ObjectStack * sender signs with, so this receiver holds no copy of the signed material. - * Two forms verify: + * What this door adds is which verified form it accepts: the timestamped one + * only. * * - `t=,v1=`, the HMAC of `.`: the timestamp * is signed, and a `t` more than `HTTP_SIGNATURE_TOLERANCE_SECONDS` (five * minutes) from this clock is refused, so a captured post stops verifying. - * - `sha256=`, the HMAC of the raw body alone. Still accepted, so no - * existing sender breaks, and deprecated: it signs no time, so a captured - * post verifies forever. {@link ApiTrigger.handleRequest} logs it once per - * armed hook. + * The one accepted form. + * - `sha256=`, the HMAC of the raw body alone, is refused even when it is + * the secret's HMAC of the body: it signs no time, so a captured post would + * verify forever (ruling B on #22805; the sender guidance teaches only the + * timestamped form). Core keeps verifying that form because it is the + * receiver half of the form our own outbound senders sign with + * (`signHttpBody`); refusing it inbound is this door's policy, held here. * - * A value that claims the timestamped form is judged by it alone, and every - * refusal is the same `false`. + * A value that claims the timestamped form is judged by it alone. The `cause` + * of a refusal is for the server log, ⛔ never for the sender. + */ +function verifyInboundSignature( + rawBody: string, + secret: string, + header: string | undefined, +): { readonly valid: true } | { readonly valid: false; readonly cause: UnverifiedSignatureCause } { + const verdict = verifyHttpSignature(rawBody, secret, header); + if (!verdict.valid) return { valid: false, cause: `signature-${verdict.reason}` }; + // Every verified scheme is named, so a scheme core adds later fails to + // compile here until this door decides whether it accepts it. + switch (verdict.scheme) { + case 'v1': + return { valid: true }; + case 'body-only': + return { valid: false, cause: 'signature-body-only' }; + } +} + +/** + * HMAC verification of `x-objectstack-signature` against the shared per-flow + * secret, exactly as the inbound door judges it ({@link verifyInboundSignature}): + * `true` only for the timestamped form `t=,v1=` inside its + * five-minute window. The body-only `sha256=` form is `false` even when + * it is the secret's HMAC of the body, and every refusal is the same `false`. */ export function verifySignature(secret: string, rawBody: string, header: string | undefined): boolean { - return verifyHttpSignature(rawBody, secret, header).valid; + return verifyInboundSignature(rawBody, secret, header).valid; } /** @@ -167,8 +201,9 @@ export function verifySignature(secret: string, rawBody: string, header: string * POST /api/v1/automation/hooks/:flowName/:hookId * → 401 INVALID_SIGNATURE — every post that does not verify, one answer: * an unknown flow, a wrong hookId, a missing or bad HMAC signature - * (a timestamped one outside its window included — see - * {@link verifySignature}). The cause goes to the server log only. + * (a timestamped one outside its window, and one in the body-only + * form, included — see {@link verifyInboundSignature}). The cause + * goes to the server log only. * → 503 the armed hook's secret cannot be read (the one residual: it * answers a post naming that flow and its hookId, signed or not) * → 400 non-JSON body @@ -230,9 +265,7 @@ export class ApiTrigger implements FlowTrigger { const queue = `${QUEUE_PREFIX}:${binding.flowName}`; const secret: HookSecret = resolve ? { kind: 'resolved', resolve } : { kind: 'literal', secret: literal! }; - const hook: ArmedHook = { - flowName: binding.flowName, hookId, secret, queue, callback, bodyOnlyDeprecationLogged: false, - }; + const hook: ArmedHook = { flowName: binding.flowName, hookId, secret, queue, callback }; this.hooks.set(binding.flowName, hook); const q = this.getQueue(); @@ -312,19 +345,11 @@ export class ApiTrigger implements FlowTrigger { body: { success: false, error: { code: 'SERVICE_UNAVAILABLE', message: 'The hook secret is unavailable.' } }, }; } - const verdict = verifyHttpSignature(input.rawBody, secret, input.signatureHeader); - if (!verdict.valid) return this.refuseUnverified(`signature-${verdict.reason}`, hook.flowName); - if (verdict.scheme === 'body-only' && !hook.bodyOnlyDeprecationLogged) { - hook.bodyOnlyDeprecationLogged = true; - this.logger.warn( - `[trigger-api] flow '${hook.flowName}' hook '${hook.hookId}' accepted a post signed with the deprecated ` + - `body-only x-objectstack-signature (sha256=). It signs no time, so a ` + - `captured post can be replayed and still verify. Sign with the timestamped form instead: ` + - `t=,v1=.">, refused once t is more than ` + - `${HTTP_SIGNATURE_TOLERANCE_SECONDS}s from this server's clock (@objectstack/core signHttpBodyAt). ` + - `Logged once per armed hook.`, - ); - } + // Verified before the body is parsed or anything is enqueued; a + // body-only signature is refused here like any other that does not + // verify, with its own cause in the log. + const verdict = verifyInboundSignature(input.rawBody, secret, input.signatureHeader); + if (!verdict.valid) return this.refuseUnverified(verdict.cause, hook.flowName); let payload: Record; try { diff --git a/packages/triggers/trigger-api/src/inbound-hook-member.test.ts b/packages/triggers/trigger-api/src/inbound-hook-member.test.ts index 75fee83fc28..3eb37ffaa01 100644 --- a/packages/triggers/trigger-api/src/inbound-hook-member.test.ts +++ b/packages/triggers/trigger-api/src/inbound-hook-member.test.ts @@ -16,8 +16,9 @@ * What this pins: * 1. the member, read off the started slot on a kernel with no HTTP server * (the hosted shape), answers every arm of the contract, each with its - * `code` and `status`, in both signature forms, and enqueues nothing for - * a post that does not verify; + * `code` and `status`: the timestamped signature form accepted, the + * body-only form refused even under the hook's own secret, and nothing + * enqueued for a post that does not verify; * 2. parity: for the same request the member answers what the self-hosted * mount answers on `http.server`, status, headers and body bytes; * 3. the composition: the route is mounted once where a raw app exists, read @@ -231,6 +232,7 @@ const ROWS: Row[] = [ { label: 'a body-only signature under another secret', flowName: FLOW, hookId: HOOK_ID, body: BODY, headers: { 'x-objectstack-signature': sigBodyOnly('not-the-secret', BODY) }, status: 401, code: 'INVALID_SIGNATURE', published: false }, { label: 'a timestamped signature under another secret', flowName: FLOW, hookId: HOOK_ID, body: BODY, headers: { 'x-objectstack-signature': sigV1('not-the-secret', BODY, NOW) }, status: 401, code: 'INVALID_SIGNATURE', published: false }, { label: 'the right secret, a timestamp outside the window', flowName: FLOW, hookId: HOOK_ID, body: BODY, headers: { 'x-objectstack-signature': sigV1(SECRET, BODY, NOW - 3600) }, status: 401, code: 'INVALID_SIGNATURE', published: false }, + { label: 'the right secret, in the body-only form (refused: the timestamped form is the only one accepted)', flowName: FLOW, hookId: HOOK_ID, body: BODY, headers: { 'x-objectstack-signature': sigBodyOnly(SECRET, BODY) }, status: 401, code: 'INVALID_SIGNATURE', published: false }, { label: 'unsigned, from a signed-in caller', flowName: FLOW, hookId: HOOK_ID, body: BODY, headers: { Authorization: 'Bearer tok_admin', Cookie: 'better-auth.session_token=tok.sig' }, status: 401, code: 'INVALID_SIGNATURE', published: false }, // The residual: an armed hook whose secret cannot be read. { label: 'an unreadable secret, unsigned', flowName: UNREADABLE, hookId: 'default', body: BODY, headers: {}, status: 503, code: 'SERVICE_UNAVAILABLE', published: false }, @@ -238,9 +240,8 @@ const ROWS: Row[] = [ // Only then is the body read as JSON. { label: 'a verified body that is not JSON', flowName: FLOW, hookId: HOOK_ID, body: '{not json', headers: sig('{not json'), status: 400, code: 'INVALID_REQUEST', published: false }, { label: 'a verified body that is a JSON array', flowName: FLOW, hookId: HOOK_ID, body: '[1,2]', headers: sig('[1,2]'), status: 400, code: 'INVALID_REQUEST', published: false }, - // Accepted: both signature forms, the empty body, and bytes as the sender wrote them. + // Accepted: the timestamped signature form, the empty body, and bytes as the sender wrote them. { label: 'a timestamped signature', flowName: FLOW, hookId: HOOK_ID, body: BODY, headers: sig(BODY), status: 202, code: null, published: true }, - { label: 'a body-only signature (deprecated, still accepted)', flowName: FLOW, hookId: HOOK_ID, body: BODY, headers: { 'x-objectstack-signature': sigBodyOnly(SECRET, BODY) }, status: 202, code: null, published: true }, { label: 'an empty body, signed', flowName: FLOW, hookId: HOOK_ID, body: '', headers: sig(''), status: 202, code: null, published: true }, { label: 'a hand-written body, signed over its own bytes', flowName: FLOW, hookId: HOOK_ID, body: HAND_WRITTEN_BODY, headers: sig(HAND_WRITTEN_BODY), status: 202, code: null, published: true }, ]; diff --git a/packages/triggers/trigger-api/src/plugin.ts b/packages/triggers/trigger-api/src/plugin.ts index 858161694f2..b5239f40507 100644 --- a/packages/triggers/trigger-api/src/plugin.ts +++ b/packages/triggers/trigger-api/src/plugin.ts @@ -114,9 +114,9 @@ async function serveInboundHook( * - [#22774] registers the `trigger-api` service, whose * `handleInboundHook(request, hook)` serves the same door from a * web-standard `Request`, for a kernel with no raw app; - * - validates hookId + HMAC signature (`x-objectstack-signature`, - * GitHub/Stripe style, constant-time) against the flow's start-node - * config; + * - validates hookId + HMAC signature (`x-objectstack-signature`, the + * timestamped `t=…,v1=…` form only, constant-time) against the flow's + * start-node config; * - **enqueues** the payload (`queue` service) and ACKs 202 — flow * execution happens on the queue consumer, never in-band with the * inbound request (ADR-0041 §5: boundary triggers must not let a slow diff --git a/packages/triggers/trigger-api/src/trigger-api-route-ledger.ts b/packages/triggers/trigger-api/src/trigger-api-route-ledger.ts index d4f893c73a7..8619c875dea 100644 --- a/packages/triggers/trigger-api/src/trigger-api-route-ledger.ts +++ b/packages/triggers/trigger-api/src/trigger-api-route-ledger.ts @@ -116,8 +116,8 @@ export const TRIGGER_API_ROUTE_LEDGER: readonly TriggerApiRouteLedgerEntry[] = [ family: 'inbound-hooks', disposition: 'server-only', note: - 'the door a third-party sender posts through, not an SDK call: GitHub/Stripe-style HMAC over the raw body, or over ' - + 'a signed timestamp and the raw body checked against a tolerance window (`x-objectstack-signature`, ' + 'the door a third-party sender posts through, not an SDK call: an HMAC over a signed timestamp and the raw body, ' + + 'checked against a tolerance window (`x-objectstack-signature`, the timestamped form only, a body-only one refused; ' + 'constant-time, through core\'s `verifyHttpSignature`) against the flow start-node `secret`, then ENQUEUE + 202 — the flow ' + 'runs on the queue consumer, never in-band (ADR-0041 §5). Measured, not assumed: `@objectstack/client`\'s whole ' + '`automation` namespace targets the dispatcher domain `/api/v1/automation`, and no client method builds a '