From 500944c10b96c1fbccdb8a84c456d04d4695edc1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 15:08:10 +0000 Subject: [PATCH 1/2] fix(service-messaging): store the delivery signature, not the signing secret (#7722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sys_http_delivery` carried the caller's `signingSecret` verbatim on every attempt row, in a table the ordinary data API reads. Reading deliveries recovered the shared key that authenticates ObjectStack to the receiver — for every subscriber at once — and that key is the receiver's only proof of origin, so the blast radius reaches systems this deployment does not control. A delivery's body is decided at enqueue and replayed byte-for-byte by every retry and by `redeliver()`, so its HMAC has exactly one correct value for the row's whole life. The outbox now computes it once at enqueue and stores only the result (`signature`, `sha256=`) — the same value handed to the receiver on the wire, one-way in the key — and drops the secret. Nothing has to resolve a credential at send time, and no secret store gains a row per attempt. The fix sits at the outbox, so both producers (webhook fan-out and the Flow `http` node) stop writing cleartext without changing their enqueue calls. `http-sender.ts` now owns both halves of the signing contract (`deliveryBody` and `signBody`) so the enqueue-time signer and the send-time transport cannot drift into signing one string and posting another. Evidence, both required to pass together: - `http-signature-at-rest.integration.test.ts` — real ObjectQL + driver-sql (better-sqlite3 `:memory:`), real `SqlHttpOutbox` and `HttpDispatcher`: after a real delivery it byte-scans `SELECT *` over every column of the delivery table for the secret, and verifies `X-Objectstack-Signature` by recomputing HMAC-SHA256 over the raw body that arrived. Covers webhook, flow, redelivery and the unsigned case. - `plugin-webhooks/src/webhook-signing-secret.test.ts` — the same two checks driven end-to-end from a `data.record.created` event through the real `AutoEnqueuer`. Reverse-verified: reinstating the cleartext column fails three of the four at-rest cases on the byte-scan assertion. Upgrading: the `signing_secret` column is no longer declared, so an existing database keeps it as an unmapped column holding the old cleartext until `os migrate plan`'s `drop_column` op is applied (destructive, never unattended); those rows also age out on the table's 30-day telemetry retention. Rotate any exposed signing secret. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XuxBHt3YbtJ34CSk6kuNWg --- .../webhook-delivery-signature-not-secret.md | 44 ++++ content/docs/automation/webhooks.mdx | 19 +- .../src/webhook-signing-secret.test.ts | 130 ++++++++++ .../services/service-messaging/package.json | 2 + .../service-messaging/src/http-outbox.test.ts | 25 +- .../service-messaging/src/http-outbox.ts | 33 ++- .../service-messaging/src/http-sender.ts | 49 +++- ...http-signature-at-rest.integration.test.ts | 232 ++++++++++++++++++ .../services/service-messaging/src/index.ts | 5 + .../src/memory-http-outbox.ts | 10 +- .../src/objects/http-delivery.object.ts | 19 +- .../service-messaging/src/sql-http-outbox.ts | 12 +- .../src/translations/en.objects.generated.ts | 5 +- .../translations/es-ES.objects.generated.ts | 5 +- .../translations/ja-JP.objects.generated.ts | 5 +- .../translations/zh-CN.objects.generated.ts | 5 +- pnpm-lock.yaml | 6 + 17 files changed, 578 insertions(+), 28 deletions(-) create mode 100644 .changeset/webhook-delivery-signature-not-secret.md create mode 100644 packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts create mode 100644 packages/services/service-messaging/src/http-signature-at-rest.integration.test.ts diff --git a/.changeset/webhook-delivery-signature-not-secret.md b/.changeset/webhook-delivery-signature-not-secret.md new file mode 100644 index 0000000000..9bd4f6d94e --- /dev/null +++ b/.changeset/webhook-delivery-signature-not-secret.md @@ -0,0 +1,44 @@ +--- +"@objectstack/service-messaging": patch +"@objectstack/plugin-webhooks": patch +--- + +fix(service-messaging): stop persisting webhook HMAC signing secrets on every delivery row (#7722) + +`sys_http_delivery` carried the caller's `signingSecret` verbatim, once per +delivery attempt, in a plain `signing_secret` column — and that table is +readable over the ordinary data API (`GET /api/v1/data/sys_http_delivery`). +Anyone who could read deliveries recovered the shared key that authenticates +ObjectStack to the receiver, for **every** subscriber at once. The signature is +the receiver's only proof of origin, so the blast radius reaches outside the +deployment: a leaked key mints payloads the receiver accepts as genuine, and +rotating it means re-coordinating with every receiver operator. + +**The row now carries the signature, not the key.** A delivery's body is +decided at enqueue and replayed byte-for-byte by every retry and by +`redeliver()` — so the HMAC has exactly one correct value for the row's whole +life. `enqueue()` computes it once from the producer's secret and stores only +the result (`signature`, `sha256=`); the secret is consumed and dropped. +The stored value is what the receiver is handed on the wire anyway and is +one-way in the key, so reading a delivery row tells you what was sent, not how +to forge something else. + +Signing behaviour on the wire is unchanged: `X-Objectstack-Signature` still +carries `sha256=HMAC-SHA256(raw body, secret)` and verifies against the +subscriber's secret exactly as before — now pinned by tests that recompute the +HMAC over the delivered body rather than asserting a header is merely present, +and by an at-rest guard that byte-scans every column of a real delivery table +after a real delivery. + +Producers are unaffected: `enqueueHttp({ …, signingSecret })` keeps its shape +for both callers (webhook fan-out and the Flow `http` node), and the fix sits at +the outbox, so both stop writing cleartext. + +**Upgrading.** The `signing_secret` column is no longer declared, so an existing +database keeps it as an unmapped column holding the old cleartext until it is +dropped: run `os migrate plan` and apply the reported `drop_column` op (it is +classified destructive, so it is never applied unattended). Until then those +rows also age out on the table's existing 30-day telemetry retention. Rotate any +signing secret that was exposed. Code reading `HttpDelivery.signingSecret` off a +row should read `signature` instead — the secret is not available there by +design. diff --git a/content/docs/automation/webhooks.mdx b/content/docs/automation/webhooks.mdx index dc7e73379d..4232499815 100644 --- a/content/docs/automation/webhooks.mdx +++ b/content/docs/automation/webhooks.mdx @@ -122,9 +122,16 @@ node executor and webhook fan-out, so both inherit retry / idempotency / dead-letter from one substrate. Webhook deliveries are the rows with `source = 'webhook'`. It generalises the old design's per-webhook table: `webhook_id` → `ref_id`, `event_id` → `dedup_key`, `event_type` → `label`, -`secret` → `signing_secret`. Rows are managed by the platform and not directly +`secret` → `signature`. Rows are managed by the platform and not directly writable. +The row carries the **signature**, never the signing secret. The body is fixed +at enqueue and replayed byte-for-byte by retries and redelivery, so the HMAC has +exactly one correct value: the outbox computes it once at enqueue from the +subscriber's `secret` and stores only the result — the same `sha256=` the +receiver is handed on the wire. Reading a delivery row tells you what was sent, +not how to forge it. + | Field | Type | Notes | |-------------------|----------|-----------------------------------------------------------------------------| | `id` | text | Primary key. Also doubles as the receiver-side idempotency key. | @@ -135,7 +142,7 @@ writable. | `url` | text | Target URL, snapshotted at enqueue so config edits do not rewrite live rows. | | `method` | text | HTTP method. | | `headers_json` | textarea | Custom headers, serialised. | -| `signing_secret` | text | HMAC secret used for the signature header (see §6). | +| `signature` | text | The `X-Objectstack-Signature` value sent with this delivery, `sha256=` — computed at enqueue; the secret is not stored (see §6). | | `timeout_ms` | number | Per-attempt timeout. | | `payload_json` | textarea | The full payload that will be POSTed. | | `partition_key` | number | `hash(ref_id) mod partitionCount`, precomputed for cheap `WHERE`. | @@ -432,6 +439,14 @@ Where the hex value is `HMAC-SHA256( secret, raw_request_body )`. There is no timestamp, no `{t}.{body}` concatenation, no version (`v1`) prefix, and no replay window — the signature covers the body bytes only. +The signature is computed **once, at enqueue**, and stored on the delivery row +(`signature`); the secret is not. The body is decided at enqueue and every +retry replays it byte-for-byte, so one HMAC is correct for every attempt — and +a delivery row read over the data API exposes the signature that was sent, not +the key that can mint new ones. Rotating a webhook's `secret` changes what +*subsequent* deliveries are signed with; rows already enqueued keep the +signature that matches the body they carry. + ### 6.2 Receiver-side verification Receivers MUST: diff --git a/packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts b/packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts new file mode 100644 index 0000000000..a849be0199 --- /dev/null +++ b/packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts @@ -0,0 +1,130 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7722 — the webhook chain, end to end: a subscriber's signing secret reaches + * the receiver as a verifiable signature and reaches the delivery row as + * nothing at all. + * + * The enqueuer still hands `signingSecret` to the outbox (it belongs to the + * subscriber and the outbox needs it for exactly one HMAC); what changed is that + * the outbox consumes it instead of copying it onto the attempt row. This test + * drives the REAL classes on both sides of that hand-off — `AutoEnqueuer`, + * `MemoryHttpOutbox` (same enqueue path as the SQL outbox), `HttpDispatcher` — + * from a `data.record.created` event, because a unit test of either half alone + * cannot see a secret that leaks between them. + * + * The at-rest scan against a real SQL table lives next to the outbox itself: + * `service-messaging/src/http-signature-at-rest.integration.test.ts`. + */ + +import { randomUUID, createHmac } from 'node:crypto'; +import { describe, expect, it } from 'vitest'; +import { DataEventSchema } from '@objectstack/spec/api'; +import type { + IDataEngine, + IRealtimeService, + RealtimeEventHandler, + RealtimeEventPayload, +} from '@objectstack/spec/contracts'; +import { MemoryHttpOutbox, HttpDispatcher, type FetchImpl } from '@objectstack/service-messaging'; +import { AutoEnqueuer } from './auto-enqueuer.js'; + +const SECRET = 'whsec_7722_subscriber_key'; + +class FakeRealtime implements IRealtimeService { + private subs = new Map(); + private n = 0; + async publish(event: RealtimeEventPayload): Promise { + for (const sub of this.subs.values()) { + const o = sub.opts ?? {}; + if (o.object && event.object !== o.object) continue; + await sub.handler(event); + } + } + async subscribe(_channel: string, handler: any, opts?: any): Promise { + const id = `s-${++this.n}`; + this.subs.set(id, { handler, opts }); + return id; + } + async unsubscribe(id: string): Promise { + this.subs.delete(id); + } +} + +/** Just enough engine to serve the `sys_webhook` subscription cache. */ +function makeEngine(rows: any[]): IDataEngine { + return { + async find() { return rows; }, + async findOne() { return rows[0] ?? null; }, + async insert(_n: string, d: any) { return d; }, + async update() { return { affected: 0 }; }, + async delete() { return { affected: 0 }; }, + async count() { return rows.length; }, + async aggregate() { return []; }, + } as unknown as IDataEngine; +} + +function recordEvent(object: string, record: any): RealtimeEventPayload { + const payload = DataEventSchema.parse({ + id: randomUUID(), + type: 'data.record.created', + object, + recordId: String(record.id), + after: record, + timestamp: '2026-08-11T00:00:00.000Z', + }); + return { type: payload.type, object, payload: { ...payload }, timestamp: payload.timestamp }; +} + +function makeFetch() { + const calls: Array<{ headers: Record; body: string }> = []; + const impl: FetchImpl = async (_url, init) => { + calls.push({ headers: init.headers, body: init.body }); + return { ok: true, status: 200, async text() { return 'ok'; } }; + }; + return { impl, calls }; +} + +describe('webhook signing secret (#7722)', () => { + it('delivers a verifiable signature without persisting the subscriber secret', async () => { + const engine = makeEngine([ + { + id: 'wh-1', + name: 'crm_hook', + active: true, + object_name: 'contact', + triggers: ['create'], + url: 'https://receiver.example/hook', + method: 'POST', + definition_json: JSON.stringify({ secret: SECRET, headers: { 'X-Team': 'crm' } }), + }, + ]); + const realtime = new FakeRealtime(); + const outbox = new MemoryHttpOutbox(); + const enqueuer = new AutoEnqueuer(engine, realtime, (input) => outbox.enqueue(input)); + await enqueuer.start(); + + await realtime.publish(recordEvent('contact', { id: 'c1', name: 'Ada', email: 'ada@example.com' })); + // The enqueue is deliberately fire-and-forget on the hot path. + await new Promise((r) => setTimeout(r, 0)); + + const { impl, calls } = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick(); + await enqueuer.stop(); + + // ── The receiver's check: recompute HMAC-SHA256 over the raw body ── + expect(calls).toHaveLength(1); + const expected = createHmac('sha256', SECRET).update(calls[0].body).digest('hex'); + expect(calls[0].headers['X-Objectstack-Signature']).toBe(`sha256=${expected}`); + // The signed body is the real event, and the subscriber's own headers + // still ride along — a blank delivery would satisfy the HMAC too. + expect(JSON.parse(calls[0].body)).toMatchObject({ object: 'contact', recordId: 'c1', action: 'created' }); + expect(calls[0].headers['X-Team']).toBe('crm'); + + // ── …and no attempt row carries the key that produced it ── + const rows = await outbox.list(); + expect(rows).toHaveLength(1); + expect(JSON.stringify(rows)).not.toContain(SECRET); + expect(rows[0].signature).toBe(`sha256=${expected}`); + }); +}); diff --git a/packages/services/service-messaging/package.json b/packages/services/service-messaging/package.json index f0daadf0b1..2fd37c1f0d 100644 --- a/packages/services/service-messaging/package.json +++ b/packages/services/service-messaging/package.json @@ -25,7 +25,9 @@ "@objectstack/types": "workspace:*" }, "devDependencies": { + "@objectstack/driver-sql": "workspace:*", "@objectstack/metadata-core": "workspace:*", + "@objectstack/objectql": "workspace:*", "@types/node": "^26.1.2", "typescript": "^6.0.3", "vitest": "^4.1.10" diff --git a/packages/services/service-messaging/src/http-outbox.test.ts b/packages/services/service-messaging/src/http-outbox.test.ts index ad3affcff3..78327ad53d 100644 --- a/packages/services/service-messaging/src/http-outbox.test.ts +++ b/packages/services/service-messaging/src/http-outbox.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; +import { createHmac } from 'node:crypto'; import { MemoryHttpOutbox } from './memory-http-outbox.js'; import { HttpDispatcher } from './http-dispatcher.js'; import type { FetchImpl } from './http-sender.js'; @@ -120,6 +121,28 @@ describe('HttpDispatcher', () => { await dispatcher.tick(); - expect(calls[0].headers['X-Objectstack-Signature']).toMatch(/^sha256=/); + // Verify the way a receiver does: recompute HMAC-SHA256 over the RAW + // body that arrived. `toMatch(/^sha256=/)` only proved a header was + // present — it would pass on a signature computed over other bytes. + const expected = createHmac('sha256', 's3cr3t').update(calls[0].body).digest('hex'); + expect(calls[0].headers['X-Objectstack-Signature']).toBe(`sha256=${expected}`); + }); + + // [#7722] The secret is consumed by enqueue, never stored: the row the + // dispatcher works from carries the signature only. + it('keeps the signing secret off the delivery row', async () => { + const outbox = new MemoryHttpOutbox(); + await outbox.enqueue({ + source: 'webhook', + refId: 'w1', + dedupKey: 'e1', + url: 'https://x', + signingSecret: 's3cr3t', + payload: { a: 1 }, + }); + + const [row] = await outbox.list(); + expect(row.signature).toMatch(/^sha256=[0-9a-f]{64}$/); + expect(JSON.stringify(row)).not.toContain('s3cr3t'); }); }); diff --git a/packages/services/service-messaging/src/http-outbox.ts b/packages/services/service-messaging/src/http-outbox.ts index e22f027021..f51a2ebd3e 100644 --- a/packages/services/service-messaging/src/http-outbox.ts +++ b/packages/services/service-messaging/src/http-outbox.ts @@ -9,6 +9,9 @@ * idempotency — with retry / backoff / dead-letter handled by the shared * {@link HttpDispatcher}. * + * Rows are signed, never keyed: the HMAC signature is computed once at enqueue + * and the signing secret is discarded (#7722) — see {@link HttpDelivery.signature}. + * * It generalises the original `plugin-webhooks` outbox so two callers share one * reliable substrate: * - the Flow `http` node executor (`@objectstack/service-automation`), and @@ -54,8 +57,22 @@ export interface HttpDelivery { method?: string; /** Custom headers. */ headers?: Record; - /** HMAC-SHA256 secret. If present, an `X-Objectstack-Signature` header is added. */ - signingSecret?: string; + /** + * Pre-computed `X-Objectstack-Signature` value (`sha256=`), or absent + * for an unsigned delivery. + * + * **A delivery row never holds the signing secret** (#7722). The body is + * fixed at enqueue and never rewritten — retries and `redeliver()` replay + * the same bytes — so the HMAC has exactly one correct value, and the outbox + * computes it once, at enqueue, from {@link EnqueueHttpInput.signingSecret}. + * What lands on the row is the signature: a one-way function of + * (body, secret) that the receiver is handed on the wire anyway, so it + * authenticates this payload without being usable to forge another. The + * secret stays with the subscriber that owns it instead of being copied onto + * every attempt, where it used to sit in cleartext for anyone who could read + * `sys_http_delivery`. + */ + signature?: string; /** Per-request timeout in ms. */ timeoutMs?: number; /** JSON-serialisable body. */ @@ -92,6 +109,13 @@ export interface EnqueueHttpInput { url: string; method?: string; headers?: Record; + /** + * HMAC-SHA256 secret. **Consumed, not stored** (#7722): `enqueue()` signs the + * body with it and persists only the resulting + * {@link HttpDelivery.signature}. It exists on this input — and nowhere on + * the delivery row — so a producer can keep owning its secret (a webhook + * subscriber's, a flow node's) without every attempt taking a cleartext copy. + */ signingSecret?: string; timeoutMs?: number; payload: unknown; @@ -177,8 +201,9 @@ export interface IHttpOutbox { /** * Reset a terminal row (`success` / `failed` / `dead`) back to `pending` so - * the dispatcher re-sends it. Resets `attempts=0`; URL / payload / secret are - * NOT touched (byte-for-byte replay). Throws {@link HttpRedeliverError}. + * the dispatcher re-sends it. Resets `attempts=0`; URL / payload / signature + * are NOT touched (byte-for-byte replay — the same body carries the same + * signature). Throws {@link HttpRedeliverError}. */ redeliver(id: string): Promise; } diff --git a/packages/services/service-messaging/src/http-sender.ts b/packages/services/service-messaging/src/http-sender.ts index b77ed4b6c8..16ad836550 100644 --- a/packages/services/service-messaging/src/http-sender.ts +++ b/packages/services/service-messaging/src/http-sender.ts @@ -9,11 +9,47 @@ import type { HttpAckResult, HttpDelivery } from './http-outbox.js'; * Lifted and generalised from `plugin-webhooks/src/http-sender.ts`: a single * stateless attempt (`sendOnce`) plus the retry-schedule classifier * (`classifyAttempt`). The dispatcher owns claim/ack; this module owns the wire. + * + * It also owns both halves of the HMAC contract — {@link deliveryBody} (the exact + * bytes that are signed AND sent) and {@link signBody} (how they are signed) — so + * the enqueue-time signer and the send-time transport cannot drift into signing + * one string and posting another. See {@link HttpDelivery.signature} for why the + * signature is computed once, at enqueue, instead of re-derived at send time from + * a secret carried on the row. */ /** Default per-request timeout. */ export const DEFAULT_HTTP_TIMEOUT_MS = 15_000; +/** Header carrying the HMAC-SHA256 signature of the request body. */ +export const SIGNATURE_HEADER = 'X-Objectstack-Signature'; + +/** + * The exact request body for a delivery — the bytes that get POSTed, and + * therefore the bytes the signature covers. + * + * A string payload is sent verbatim (a pre-rendered body); anything else is + * JSON-serialised. Stable across a persistence round-trip: `SqlHttpOutbox` + * stores `JSON.stringify(payload)` and re-parses it on claim, and + * `JSON.stringify(JSON.parse(s)) === s` for any `s` that `JSON.stringify` + * produced — so signing at enqueue and sending after a reload agree byte for + * byte. `http-signature-at-rest.integration.test.ts` pins that. + */ +export function deliveryBody(payload: unknown): string { + return typeof payload === 'string' ? payload : JSON.stringify(payload ?? null); +} + +/** + * Compute the `X-Objectstack-Signature` value for a body: `sha256=` of + * `HMAC-SHA256(body, secret)`. + * + * The output is safe to persist (it is handed to the receiver on the wire + * anyway); the `secret` argument is NOT — see {@link HttpDelivery.signature}. + */ +export function signBody(body: string, secret: string): string { + return `sha256=${createHmac('sha256', secret).update(body).digest('hex')}`; +} + /** Truncate response bodies to keep storage cost predictable. */ const RESPONSE_BODY_CAP = 16 * 1024; @@ -55,10 +91,7 @@ export async function sendOnce( delivery: HttpDelivery, fetchImpl: FetchImpl, ): Promise { - const body = - typeof delivery.payload === 'string' - ? delivery.payload - : JSON.stringify(delivery.payload ?? null); + const body = deliveryBody(delivery.payload); const headers: Record = { 'Content-Type': 'application/json', @@ -68,9 +101,11 @@ export async function sendOnce( ...(delivery.label ? { 'X-Objectstack-Event': delivery.label } : {}), ...(delivery.headers ?? {}), }; - if (delivery.signingSecret) { - const sig = createHmac('sha256', delivery.signingSecret).update(body).digest('hex'); - headers['X-Objectstack-Signature'] = `sha256=${sig}`; + // Signed at enqueue (the body is fixed then and never rewritten), so the + // signing secret is not on the row for this attempt — or any other — to + // carry. #7722. + if (delivery.signature) { + headers[SIGNATURE_HEADER] = delivery.signature; } const timeoutMs = delivery.timeoutMs ?? DEFAULT_HTTP_TIMEOUT_MS; diff --git a/packages/services/service-messaging/src/http-signature-at-rest.integration.test.ts b/packages/services/service-messaging/src/http-signature-at-rest.integration.test.ts new file mode 100644 index 0000000000..6b70f10266 --- /dev/null +++ b/packages/services/service-messaging/src/http-signature-at-rest.integration.test.ts @@ -0,0 +1,232 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7722 — the signing secret must not be recoverable from `sys_http_delivery`, + * and signing must still be correct. + * + * The defect: `SqlHttpOutbox.enqueue` copied the caller's `signingSecret` + * verbatim onto every attempt row (`signing_secret`), a table `GET + * /api/v1/data/sys_http_delivery` reads. Anyone who could read deliveries + * recovered the shared key that authenticates ObjectStack to the receiver — the + * receiver's ONLY proof of origin — for every subscriber at once. The blast + * radius is outside the deployment: rotating a leaked key means re-coordinating + * with every receiver operator. + * + * These tests run the REAL storage path — `ObjectQL` + `@objectstack/driver-sql` + * on better-sqlite3 `:memory:`, the production `SqlHttpOutbox`, the production + * `HttpDispatcher` — because both halves of the claim live in that hand-off: the + * bytes a driver actually wrote into a real table, and the header a real + * dispatcher put on the wire after reloading that row. + * + * Two assertions, deliberately not one: + * + * 1. **The bytes are absent.** A `SELECT *` byte-scan over every column of + * every delivery row — not a projection of the fields the schema declares, + * which would pass on a stale physical column the object no longer names. + * Same shape as the scan the sibling datasource credential path passes. + * 2. **The signature still verifies.** The captured `X-Objectstack-Signature` + * is checked by recomputing `HMAC-SHA256(raw body, secret)` the way a + * receiver does. "It didn't throw" would pass on a delivery signed with the + * wrong bytes, or not signed at all. + * + * Assertion 2 also pins the enqueue-time signing this fix introduced: the + * signature is computed BEFORE the row is written and the body is rebuilt AFTER + * a full DB round-trip (`JSON.stringify` → text column → `JSON.parse` → + * `JSON.stringify`), so a round-trip that changed one byte would break the + * verification rather than silently ship an unverifiable delivery. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { createHmac } from 'node:crypto'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SqlHttpOutbox } from './sql-http-outbox.js'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { HttpDelivery, SYS_HTTP_DELIVERY } from './objects/http-delivery.object.js'; +import type { FetchImpl } from './http-sender.js'; + +/** The secret under test — distinctive enough that a byte-scan cannot miss it. */ +const SECRET = 'whsec_7722_do_not_persist_me'; + +/** + * A payload with nesting, unicode and an empty-ish leaf: the `JSON.stringify` → + * store → `JSON.parse` → `JSON.stringify` round-trip has to reproduce it byte + * for byte or the signature computed at enqueue stops matching the body sent. + */ +const PAYLOAD = { + object: 'crm_account', + recordId: 'acc_1', + action: 'created', + timestamp: 1_760_000_000_000, + after: { name: '客户 A', amount: 1234.5, tags: ['a', 'b'], note: '', nested: { deep: true } }, +}; + +function makeSqliteDriver() { + return new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); +} + +/** Records what actually went on the wire. */ +function makeFetch(): { + impl: FetchImpl; + calls: Array<{ url: string; headers: Record; body: string }>; +} { + const calls: Array<{ url: string; headers: Record; body: string }> = []; + const impl: FetchImpl = async (url, init) => { + calls.push({ url, headers: init.headers, body: init.body }); + return { ok: true, status: 204, async text() { return ''; } }; + }; + return { impl, calls }; +} + +describe('sys_http_delivery — signing secret at rest (#7722)', () => { + let engine: ObjectQL | undefined; + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + engine = undefined; + }); + + async function boot() { + const driver = makeSqliteDriver(); + engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(HttpDelivery as any, '@objectstack/service-messaging'); + // Real DDL through the real path: the table below is created by the + // driver from the object definition, so its physical columns are + // exactly the ones production would have. + await engine.syncSchemas(); + return { driver, engine: engine! }; + } + + /** + * Every value in every column of every delivery row, as one string. + * `SELECT *` on purpose — a scan of the declared fields would miss a + * physical column the schema stopped naming. + */ + async function scanDeliveryTable(driver: SqlDriver): Promise { + const rows: Array> = await driver.getKnex()(SYS_HTTP_DELIVERY).select('*'); + expect(rows.length).toBeGreaterThan(0); // a scan of nothing proves nothing + return rows.map((r) => Object.values(r).map((v) => String(v ?? '')).join(' ')).join(' '); + } + + it('a real webhook delivery signs correctly and leaves no secret in the table', async () => { + const { driver, engine: eng } = await boot(); + const outbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + + await outbox.enqueue({ + source: 'webhook', + refId: 'wh_1', + dedupKey: 'crm_account:acc_1:created:1760000000000', + label: 'data.record.created', + url: 'https://receiver.example/hook', + headers: { 'X-Custom': 'keep-me' }, + signingSecret: SECRET, + payload: PAYLOAD, + }); + + const { impl, calls } = makeFetch(); + const dispatcher = new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }); + await dispatcher.tick(); + + // ── The delivery happened, and it verifies as a receiver verifies ── + expect(calls).toHaveLength(1); + const sent = calls[0]; + const expected = createHmac('sha256', SECRET).update(sent.body).digest('hex'); + expect(sent.headers['X-Objectstack-Signature']).toBe(`sha256=${expected}`); + // …over the real payload, not an empty body the HMAC would still "match". + expect(JSON.parse(sent.body)).toEqual(PAYLOAD); + expect(sent.headers['X-Custom']).toBe('keep-me'); + + // ── …and the key that produced it is nowhere in the table ── + const dump = await scanDeliveryTable(driver); + expect(dump).not.toContain(SECRET); + // Guard the guard: the scan does see the row's other content, so a + // silently-empty dump can never be what made the assertion above pass. + expect(dump).toContain('https://receiver.example/hook'); + expect(dump).toContain(sent.headers['X-Objectstack-Signature']); + + // The data API's own read is likewise secret-free (and, being a + // projection of the declared fields, no longer even names a secret). + const apiRows = await eng.find(SYS_HTTP_DELIVERY, {}); + expect(JSON.stringify(apiRows)).not.toContain(SECRET); + }); + + it('holds for the flow producer too — the fix is at the outbox, not in one caller', async () => { + const { driver, engine: eng } = await boot(); + const outbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + + await outbox.enqueue({ + source: 'flow', + refId: 'node_http_1', + dedupKey: 'run_1:node_http_1', + label: 'flow:node_http_1', + url: 'https://receiver.example/flow', + signingSecret: SECRET, + payload: { hello: 'world' }, + }); + + const { impl, calls } = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick(); + + const expected = createHmac('sha256', SECRET).update(calls[0].body).digest('hex'); + expect(calls[0].headers['X-Objectstack-Signature']).toBe(`sha256=${expected}`); + expect(await scanDeliveryTable(driver)).not.toContain(SECRET); + }); + + it('a redelivered row still signs — the signature survives without the secret', async () => { + const { driver, engine: eng } = await boot(); + const outbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + + const id = await outbox.enqueue({ + source: 'webhook', + refId: 'wh_1', + dedupKey: 'once', + url: 'https://receiver.example/hook', + signingSecret: SECRET, + payload: PAYLOAD, + }); + + const first = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: first.impl, partitionCount: 1 }).tick(); + expect((await outbox.list())[0].status).toBe('success'); + + // Redeliver: a terminal row reset to pending and sent again, which is + // where a resolve-at-send-time design would need the secret back. + await outbox.redeliver(id); + const second = makeFetch(); + await new HttpDispatcher({ nodeId: 'n2', outbox, fetchImpl: second.impl, partitionCount: 1 }).tick(); + + expect(second.calls).toHaveLength(1); + // Byte-for-byte replay: same body, same signature, still verifiable. + expect(second.calls[0].body).toBe(first.calls[0].body); + expect(second.calls[0].headers['X-Objectstack-Signature']).toBe( + first.calls[0].headers['X-Objectstack-Signature'], + ); + const expected = createHmac('sha256', SECRET).update(second.calls[0].body).digest('hex'); + expect(second.calls[0].headers['X-Objectstack-Signature']).toBe(`sha256=${expected}`); + + expect(await scanDeliveryTable(driver)).not.toContain(SECRET); + }); + + it('an unsigned delivery sends no signature header at all', async () => { + const { engine: eng } = await boot(); + const outbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + await outbox.enqueue({ + source: 'flow', + refId: 'n1', + dedupKey: 'unsigned', + url: 'https://receiver.example/plain', + payload: { a: 1 }, + }); + + const { impl, calls } = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick(); + + expect(calls[0].headers['X-Objectstack-Signature']).toBeUndefined(); + }); +}); diff --git a/packages/services/service-messaging/src/index.ts b/packages/services/service-messaging/src/index.ts index 4a1368d589..b2b09ae05b 100644 --- a/packages/services/service-messaging/src/index.ts +++ b/packages/services/service-messaging/src/index.ts @@ -131,6 +131,11 @@ export { nextHttpRetryDelayMs, newDeliveryId as newHttpDeliveryId, DEFAULT_HTTP_TIMEOUT_MS, + // The signing pair (#7722) — exported so a receiver-side verifier (or a + // test) recomputes the HMAC over exactly the bytes the sender signed. + deliveryBody as httpDeliveryBody, + signBody as signHttpBody, + SIGNATURE_HEADER as HTTP_SIGNATURE_HEADER, type FetchImpl, type HttpAttemptOutcome, } from './http-sender.js'; diff --git a/packages/services/service-messaging/src/memory-http-outbox.ts b/packages/services/service-messaging/src/memory-http-outbox.ts index 8156bd9968..5342eb8f3b 100644 --- a/packages/services/service-messaging/src/memory-http-outbox.ts +++ b/packages/services/service-messaging/src/memory-http-outbox.ts @@ -2,6 +2,7 @@ import { randomUUID } from 'node:crypto'; import { hashPartition } from './backoff.js'; +import { deliveryBody, signBody } from './http-sender.js'; import { HttpRedeliverError, type EnqueueHttpInput, @@ -17,6 +18,11 @@ import { * Mirrors `MemoryWebhookOutbox`: atomic-claim semantics come for free from the * single-threaded event loop operating on one `Map`. Two instances do NOT share * state — pass the same instance to both dispatchers to simulate one DB. + * + * Signs at enqueue and drops the secret exactly like {@link SqlHttpOutbox} + * (#7722) — the rows a test inspects here have the same shape as the ones the + * SQL outbox persists, so a test cannot pass on a row that production wouldn't + * write. */ export class MemoryHttpOutbox implements IHttpOutbox { private readonly rows = new Map(); @@ -39,7 +45,9 @@ export class MemoryHttpOutbox implements IHttpOutbox { url: input.url, method: input.method ?? 'POST', headers: input.headers, - signingSecret: input.signingSecret, + signature: input.signingSecret + ? signBody(deliveryBody(input.payload), input.signingSecret) + : undefined, timeoutMs: input.timeoutMs, payload: input.payload, status: 'pending', diff --git a/packages/services/service-messaging/src/objects/http-delivery.object.ts b/packages/services/service-messaging/src/objects/http-delivery.object.ts index 11db543353..0d2577b10f 100644 --- a/packages/services/service-messaging/src/objects/http-delivery.object.ts +++ b/packages/services/service-messaging/src/objects/http-delivery.object.ts @@ -10,7 +10,8 @@ import { Field, ObjectSchema } from '@objectstack/spec/data'; * deliveries). Shared by the Flow `http` node executor and webhook fan-out so * both inherit retry / idempotency / dead-letter from one substrate. Generalises * `sys_webhook_delivery`: `webhook_id`→`ref_id`, `event_id`→`dedup_key`, - * `event_type`→`label`, `secret`→`signing_secret`. + * `event_type`→`label`, `secret`→`signature` (#7722 — the row carries the HMAC + * this delivery sends, never the key that computed it). * * Designed for the SqlHttpOutbox claim algorithm: * 1. Producers INSERT pending rows (dedup'd by `(source, dedup_key)`). @@ -123,7 +124,21 @@ export const HttpDelivery = ObjectSchema.create({ method: Field.text({ label: 'Method', required: false, maxLength: 10 }), headers_json: Field.textarea({ label: 'Headers JSON', required: false }), - signing_secret: Field.text({ label: 'HMAC Secret', required: false, maxLength: 256 }), + // [#7722] The SIGNATURE, not the key that produced it. This column used + // to be `signing_secret` — a verbatim copy of the subscriber's shared + // secret on every attempt row — and this table is readable over the + // ordinary data API, so reading it recovered the key that authenticates + // ObjectStack to every receiver. The body is fixed at enqueue and + // replayed byte-for-byte by retries and redelivery, so the outbox signs + // once and keeps only the result: `sha256=`, the same value the + // receiver is handed on the wire, one-way in the secret. + signature: Field.text({ + label: 'HMAC Signature', + required: false, + maxLength: 128, + description: + 'X-Objectstack-Signature sent with this delivery (sha256=). Computed at enqueue; the signing secret is never persisted.', + }), timeout_ms: Field.number({ label: 'Timeout (ms)', required: false }), payload_json: Field.textarea({ label: 'Payload JSON', required: true }), diff --git a/packages/services/service-messaging/src/sql-http-outbox.ts b/packages/services/service-messaging/src/sql-http-outbox.ts index a66bc835f9..4e13992c39 100644 --- a/packages/services/service-messaging/src/sql-http-outbox.ts +++ b/packages/services/service-messaging/src/sql-http-outbox.ts @@ -4,6 +4,7 @@ import { randomUUID } from 'node:crypto'; import type { IDataEngine } from '@objectstack/spec/contracts'; import { hashPartition } from './backoff.js'; import { toEpochMs } from './audit-timestamp.js'; +import { deliveryBody, signBody } from './http-sender.js'; import { HttpRedeliverError, type EnqueueHttpInput, @@ -34,7 +35,7 @@ interface DeliveryRow { url: string; method?: string | null; headers_json?: string | null; - signing_secret?: string | null; + signature?: string | null; timeout_ms?: number | null; payload_json: string; partition_key: number; @@ -106,7 +107,12 @@ export class SqlHttpOutbox implements IHttpOutbox { url: input.url, method: input.method ?? 'POST', headers_json: input.headers ? JSON.stringify(input.headers) : undefined, - signing_secret: input.signingSecret, + // Sign here, store only the signature (#7722). The body is decided + // at enqueue and replayed byte-for-byte by every retry, so one HMAC + // covers every attempt and the secret has no reason to be persisted. + signature: input.signingSecret + ? signBody(deliveryBody(input.payload), input.signingSecret) + : undefined, timeout_ms: input.timeoutMs, payload_json: JSON.stringify(input.payload ?? null), partition_key: hashPartition(input.refId, this.partitionCount), @@ -268,7 +274,7 @@ export class SqlHttpOutbox implements IHttpOutbox { url: r.url, method: r.method ?? undefined, headers: r.headers_json ? JSON.parse(r.headers_json) : undefined, - signingSecret: r.signing_secret ?? undefined, + signature: r.signature ?? undefined, timeoutMs: r.timeout_ms ?? undefined, payload: JSON.parse(r.payload_json), status: r.status, diff --git a/packages/services/service-messaging/src/translations/en.objects.generated.ts b/packages/services/service-messaging/src/translations/en.objects.generated.ts index 61135398cd..1c95aa807b 100644 --- a/packages/services/service-messaging/src/translations/en.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/en.objects.generated.ts @@ -329,8 +329,9 @@ export const enObjects: NonNullable = { headers_json: { label: "Headers JSON" }, - signing_secret: { - label: "HMAC Secret" + signature: { + label: "HMAC Signature", + help: "X-Objectstack-Signature sent with this delivery (sha256=). Computed at enqueue; the signing secret is never persisted." }, timeout_ms: { label: "Timeout (ms)" diff --git a/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts b/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts index d574a3f64b..5b13078504 100644 --- a/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts @@ -329,8 +329,9 @@ export const esESObjects: NonNullable = { headers_json: { label: "Headers JSON" }, - signing_secret: { - label: "HMAC Secret" + signature: { + label: "HMAC Signature", + help: "X-Objectstack-Signature sent with this delivery (sha256=). Computed at enqueue; the signing secret is never persisted." }, timeout_ms: { label: "Timeout (ms)" diff --git a/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts b/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts index a422a8c6b0..f336aae431 100644 --- a/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts @@ -329,8 +329,9 @@ export const jaJPObjects: NonNullable = { headers_json: { label: "Headers JSON" }, - signing_secret: { - label: "HMAC Secret" + signature: { + label: "HMAC Signature", + help: "X-Objectstack-Signature sent with this delivery (sha256=). Computed at enqueue; the signing secret is never persisted." }, timeout_ms: { label: "Timeout (ms)" diff --git a/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts b/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts index 01d7219a3c..b678bb3f54 100644 --- a/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts @@ -329,8 +329,9 @@ export const zhCNObjects: NonNullable = { headers_json: { label: "请求头 JSON" }, - signing_secret: { - label: "HMAC 密钥" + signature: { + label: "HMAC 签名", + help: "本次投递发送的 X-Objectstack-Signature(sha256=)。入队时计算,签名密钥不会落库。" }, timeout_ms: { label: "超时(毫秒)" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 10b9bb20d8..87c802cfb8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2334,9 +2334,15 @@ importers: specifier: workspace:* version: link:../../types devDependencies: + '@objectstack/driver-sql': + specifier: workspace:* + version: link:../../drivers/driver-sql '@objectstack/metadata-core': specifier: workspace:* version: link:../../metadata-core + '@objectstack/objectql': + specifier: workspace:* + version: link:../../objectql '@types/node': specifier: ^26.1.2 version: 26.1.2 From 25e08c710fd1520cbeee5cd7b42628549294b233 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 19:40:26 +0000 Subject: [PATCH 2/2] test(plugin-webhooks): pin the #7722 engine double to the real write-verb dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:engine-double-contract` was red on the new test's fake engine: its `delete()` and `update()` accepted every call shape, including the ones `ObjectQL` refuses. That is the #4434 failure mode — a double looser than the implementation it replaces keeps a suite green over a path that is dead in production — so both verbs now open with the producer's own predicates, `assertEngineDeleteDispatch(options)` and `assertEngineUpdateDispatch(data, options)` from `@objectstack/metadata-core` (added as a devDependency; metadata-core rather than objectql, which would invert a dependency edge). Gate rerun: OK — 152 pinned, 133 in the DEBT ledger, 2 exempt (was: 2 problems, both this file). plugin-webhooks suite 4 files / 35 tests pass; eslint clean on the changed file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XuxBHt3YbtJ34CSk6kuNWg --- packages/plugins/plugin-webhooks/package.json | 1 + .../src/webhook-signing-secret.test.ts | 22 ++++++++++++++++--- pnpm-lock.yaml | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/plugins/plugin-webhooks/package.json b/packages/plugins/plugin-webhooks/package.json index 11708f27ba..9de2d40d16 100644 --- a/packages/plugins/plugin-webhooks/package.json +++ b/packages/plugins/plugin-webhooks/package.json @@ -29,6 +29,7 @@ "@objectstack/spec": "workspace:*" }, "devDependencies": { + "@objectstack/metadata-core": "workspace:*", "@types/node": "^26.1.2", "typescript": "^6.0.3", "vitest": "^4.1.10" diff --git a/packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts b/packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts index a849be0199..5041041c52 100644 --- a/packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts +++ b/packages/plugins/plugin-webhooks/src/webhook-signing-secret.test.ts @@ -26,6 +26,7 @@ import type { RealtimeEventHandler, RealtimeEventPayload, } from '@objectstack/spec/contracts'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { MemoryHttpOutbox, HttpDispatcher, type FetchImpl } from '@objectstack/service-messaging'; import { AutoEnqueuer } from './auto-enqueuer.js'; @@ -51,14 +52,29 @@ class FakeRealtime implements IRealtimeService { } } -/** Just enough engine to serve the `sys_webhook` subscription cache. */ +/** + * Just enough engine to serve the `sys_webhook` subscription cache. + * + * The write verbs open with the PRODUCER's own dispatch predicates + * (`assertEngineUpdateDispatch` / `assertEngineDeleteDispatch` from + * `@objectstack/metadata-core`) rather than accepting anything, because a + * double looser than the real engine turns a green suite into no suite at all + * on exactly the call shapes `ObjectQL` refuses (#4434) — the failure this + * file's own subject is a cousin of. `check:engine-double-contract` pins it. + */ function makeEngine(rows: any[]): IDataEngine { return { async find() { return rows; }, async findOne() { return rows[0] ?? null; }, async insert(_n: string, d: any) { return d; }, - async update() { return { affected: 0 }; }, - async delete() { return { affected: 0 }; }, + async update(_n: string, data: any, options?: any) { + assertEngineUpdateDispatch(data, options); + return { affected: 0 }; + }, + async delete(_n: string, options?: any) { + assertEngineDeleteDispatch(options); + return { affected: 0 }; + }, async count() { return rows.length; }, async aggregate() { return []; }, } as unknown as IDataEngine; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 87c802cfb8..418df8f44f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1772,6 +1772,9 @@ importers: specifier: workspace:* version: link:../../spec devDependencies: + '@objectstack/metadata-core': + specifier: workspace:* + version: link:../../metadata-core '@types/node': specifier: ^26.1.2 version: 26.1.2