From 4b57679bfc1f9fcbff134e0538b40635343ec46a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 16:03:22 +0000 Subject: [PATCH] fix(plugin-webhooks): move webhook custom headers onto the encrypted channel (#7986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #7799 moved the signing secret out of `sys_webhook.definition_json`. It left the custom `headers` map behind — and `headers` is the ordinary place an `Authorization: Bearer …` goes. `sys_webhook` declares no `enable` block at all, so `GET /api/v1/data/sys_webhook` handed the whole map back to every persona that can read the object, with none of the retention bound that eventually ages out the delivery table's copies. The authored map now lands in a new `headers_secret` column on the same encrypted channel as the key: the engine encrypts it into `sys_secret`, the row keeps an opaque ref, reads return a mask, and the enqueuer recovers it through `engine.resolveSecretField()` on the SAME cache refresh that recovers the signing secret — so #8022's re-arm cannot produce a correctly-signed delivery with its headers missing. The existing boot sweep moves already-persisted cleartext headers out of the blob in one idempotent update alongside the key. The whole map moves rather than the credential-looking entries: only some entries are credentials and the platform cannot tell which. Guessing from the header name is fail-OPEN on exactly the custom spellings (`X-Acme-Token`) most likely to be one; letting the author declare which are sensitive is a change to the authoring envelope and belongs to the spec surface. `webhook.zod.ts` is untouched and every authored header is still delivered byte-for-byte. Fail-closed and symmetric with #7799: a stored map that cannot be decrypted DROPS the subscription rather than delivering without it. Against an endpoint that does not require the header, a delivery missing its `Authorization` succeeds while quietly deviating from the authored configuration, and nothing records that it went out incomplete. Does NOT close the issue end to end: the same headers are still written in cleartext to `sys_http_delivery.headers_json` (measured after this change), and closing that half needs a decision outside this package. `sys_email.headers_json` is untouched for an in-flight-conflict reason, not a value judgement. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LiA1beRqJfW2XEmPVRoMqT --- ...ebhook-custom-headers-encrypted-at-rest.md | 65 ++++ .../plugin-webhooks/src/auto-enqueuer.ts | 199 +++++++++-- .../src/bootstrap-declared-webhooks.ts | 31 +- packages/plugins/plugin-webhooks/src/index.ts | 6 + .../src/migrate-webhook-secrets.ts | 46 ++- .../plugin-webhooks/src/sys-webhook.object.ts | 42 ++- .../src/translations/en.objects.generated.ts | 4 + .../translations/es-ES.objects.generated.ts | 4 + .../translations/ja-JP.objects.generated.ts | 4 + .../translations/zh-CN.objects.generated.ts | 4 + .../plugin-webhooks/src/webhook-headers.ts | 235 +++++++++++++ .../src/webhook-secret-at-rest.test.ts | 326 +++++++++++++++++- .../plugin-webhooks/src/webhook-secret.ts | 12 +- packages/spec/liveness/webhook.json | 6 +- 14 files changed, 911 insertions(+), 73 deletions(-) create mode 100644 .changeset/webhook-custom-headers-encrypted-at-rest.md create mode 100644 packages/plugins/plugin-webhooks/src/webhook-headers.ts diff --git a/.changeset/webhook-custom-headers-encrypted-at-rest.md b/.changeset/webhook-custom-headers-encrypted-at-rest.md new file mode 100644 index 0000000000..0f89a4cd3b --- /dev/null +++ b/.changeset/webhook-custom-headers-encrypted-at-rest.md @@ -0,0 +1,65 @@ +--- +"@objectstack/plugin-webhooks": patch +--- + +fix(plugin-webhooks): webhook custom `headers` are encrypted at rest instead of riding `definition_json` in cleartext (#7986) + +`#7799` moved the webhook **signing secret** out of `sys_webhook.definition_json` +into an encrypted `signing_secret` column. It did not move the custom **headers** +map — and `headers` is the ordinary place an `Authorization: Bearer …` goes. + +`sys_webhook` declares **no `enable` block at all**, so it keeps the full default +data API: an ordinary `GET /api/v1/data/sys_webhook` handed the whole header map, +credentials included, to every persona that can read the object. Unlike the +delivery table's copies, nothing ages this out — the configuration row is +retained for the life of the webhook. + +This is a **scope-of-the-original-fix** finding, not a regression: the exposure +predates `#7799` and nothing that card did made it worse. What was wrong was the +conclusion a reader would reasonably draw from it — that webhook credentials are +no longer in a blob. + +**What changed.** The authored `headers` map now lands in a new +`sys_webhook.headers_secret` column on the engine's encrypted credential channel, +exactly as `signing_secret` does: the engine encrypts it into `sys_secret`, the +row keeps only an opaque `secret:` ref, and every read path returns a mask. +`definition_json` carries the same envelope minus both credential passengers. The +auto-enqueuer recovers the map server-side through `engine.resolveSecretField()` +on the **same** cache refresh that recovers the signing key, and the existing +boot sweep (`migrateLegacyWebhookSecrets`) now moves already-persisted cleartext +headers out of the blob in the same single, idempotent update it uses for the +key. + +**Nothing about authoring changes.** Authors still write +`headers: { … }` on `defineWebhook()`, `webhook.zod.ts` is untouched, and every +authored header is still delivered on the wire byte-for-byte. + +**The whole map moves, not just the credential-looking entries.** Only some +entries are credentials and the platform cannot tell which. Guessing from the +header name (`authorization`, `x-api-key`, …) is fail-**open** on exactly the +custom spellings — `X-Acme-Token` — most likely to be one, and a heuristic that +silently passes the header that mattered is worse than none because it reads as +coverage. Letting the author declare which are sensitive is a change to the +authoring envelope and belongs to the spec surface. The cost this shape is +accused of is measured and small: `definition_json` is a raw JSON textarea +pending a real builder, so what an admin loses is the ability to read back a +`Content-Type` they typed. + +**Fail-closed, and symmetric with `#7799`.** With no CryptoProvider the engine +refuses the write rather than storing cleartext; a stored map that cannot be +decrypted **drops** the subscription rather than delivering it with its headers +silently missing. That drop is deliberately the same trade the signing secret +makes: against an endpoint that does not require the header, a delivery missing +its `Authorization` **succeeds** while quietly deviating from the configuration +the author wrote, and nothing records that it went out incomplete. Subscriptions +dropped this way re-arm on CryptoProvider registration exactly as `#8022` made +them — the header map is resolved on the same rebuilt cache as the key, so a +re-arm can never produce a correctly-signed delivery with no headers on it. + +**This does not close the exposure end to end.** The same headers are still +written in cleartext to `sys_http_delivery.headers_json` at enqueue time, and +that table is readable over the data API (`apiMethods: ['get','list']`, 30-day +retention). Measured after this change: the credential is still recoverable +there. Closing that half needs a decision outside this package and is tracked on +#7986; `sys_email.headers_json` (the same shape, on the email delivery row) is +untouched here for the same reason. diff --git a/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts b/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts index 355c951610..fc519ad8eb 100644 --- a/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts +++ b/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts @@ -11,6 +11,11 @@ import { readLegacySecret, resolveWebhookSecret, } from './webhook-secret.js'; +import { + WEBHOOK_HEADERS_FIELD, + readLegacyHeaders, + resolveWebhookHeaders, +} from './webhook-headers.js'; /** * The authored trigger vocabulary, taken from the spec rather than restated @@ -27,6 +32,47 @@ type WebhookTrigger = WebhookTriggerType; */ export type HttpEnqueueFn = (input: EnqueueHttpInput) => Promise; +/** + * Which encrypted credential a drop is about, and the words its report needs. + * + * Parameterised rather than duplicated because the two reports differ only in + * the noun and the consequence clause — everything an `error` owes (the + * consequence, concretely, and the fix) is identical, and a second hand-written + * copy is how one of them drifts into being less actionable than the other. + */ +interface DropReason { + /** Column the value lives in — travels in the ADR-0112 meta. */ + field: string; + /** How the credential is named in prose. */ + noun: string; + /** Indefinite form for "webhook X holds …". */ + article: string; + /** What delivering anyway would mean — the harm being refused. */ + ratherThan: string; + /** Issue this drop rule comes from. */ + issue: string; + /** Issue pair for the repeat line. */ + issues: string; +} + +const SIGNING_SECRET_CREDENTIAL: DropReason = { + field: WEBHOOK_SECRET_FIELD, + noun: 'signing secret', + article: 'an encrypted signing secret', + ratherThan: 'delivered unsigned', + issue: '#7799', + issues: '#7799/#8022', +}; + +const CUSTOM_HEADERS_CREDENTIAL: DropReason = { + field: WEBHOOK_HEADERS_FIELD, + noun: 'custom header map', + article: 'encrypted custom headers', + ratherThan: 'delivered without the headers it was authored with', + issue: '#7986', + issues: '#7986/#8022', +}; + /** * Optional logger interface (subset of console / kernel logger). */ @@ -126,7 +172,11 @@ export class AutoEnqueuer { /** [#8022] Detach for the engine's crypto-registration listener. */ private unbindCryptoListener: (() => void) | undefined; /** - * [#8022] Webhook ids currently dropped for an unresolvable signing key. + * [#8022] Webhook ids currently dropped for an unresolvable credential — + * the signing key (#7799) or, since #7986, the custom header map. ONE set + * for both on purpose: a subscription is either armed or dropped, so a + * per-credential ledger would let a row already silenced for its key report + * loudly again for its headers on the very next refresh. * Held so the loud first report is said ONCE per outage (AGENTS.md * "Degradation log levels": *say it once, at the first degradation*) and * again if the same webhook breaks after recovering — not once per row per @@ -259,14 +309,15 @@ export class AutoEnqueuer { for (const row of rows) { const sub = this.parseRow(row); if (!sub) continue; - // [#7799] The signing key is no longer in the row we just read — it - // lives encrypted in `sys_secret`, and this read path returns only a - // mask. Dereference it here, on the 60s refresh, rather than per - // event: the cache already holds the plaintext in memory (it always - // did), so this changes where the value comes FROM, not how long it - // is held. A row whose key cannot be recovered is DROPPED — see - // `attachSecret`. - if (!(await this.attachSecret(sub, row))) continue; + // [#7799, #7986] Neither credential is in the row we just read — + // the signing key and the custom header map both live encrypted in + // `sys_secret`, and this read path returns only a mask. Dereference + // them here, on the 60s refresh, rather than per event: the cache + // already holds the plaintext in memory (it always did), so this + // changes where the values come FROM, not how long they are held. A + // row whose credentials cannot be recovered is DROPPED — see + // `attachCredentials`. + if (!(await this.attachCredentials(sub, row))) continue; // Empty objectName == "any object" → indexed under '*'. const key = sub.objectName ?? '*'; const arr = next.get(key) ?? []; @@ -295,8 +346,42 @@ export class AutoEnqueuer { } /** - * [#7799] Resolve `sub.secret` for one cached subscription. Returns `false` - * when the subscription must be dropped from the cache. + * [#7799, #7986] Resolve BOTH encrypted credentials for one cached + * subscription. Returns `false` when the subscription must be dropped from + * the cache. + * + * The two halves are deliberately resolved on the SAME build rather than on + * separate schedules. #8022's re-arm rebuilds the whole cache when a + * CryptoProvider registers; a header map recovered on any other cadence + * would let the enqueuer re-arm into a delivery that is correctly signed and + * silently missing its `Authorization`, which is the failure mode of both + * cards at once. + * + * The drop ledger is cleared only when BOTH succeed — otherwise a row whose + * secret resolves and whose headers do not would clear its own "already + * reported" mark on every refresh and shout the same `error` every 60s, + * which is precisely the unreadable-error-channel failure #8022's say-once + * rule exists to prevent. + * + * Cost: up to two point reads + two decrypts per credential-bearing row per + * refresh (default 60s), off the write path entirely. Deliberately NOT + * memoised across refreshes — the only cheap cache key would be + * `updated_at`, which nothing guarantees is stamped when a credential is + * rotated, and a stale key signs every delivery with a signature the + * receiver rejects. + */ + private async attachCredentials(sub: CachedSubscription, row: any): Promise { + if (!(await this.attachSecret(sub, row))) return false; + if (!(await this.attachHeaders(sub, row))) return false; + // Recovered — a later break is a new outage and gets said loudly again + // rather than being swallowed as a repeat. + this.droppedForSecret.delete(sub.id); + return true; + } + + /** + * [#7799] Resolve `sub.secret`. Returns `false` when the subscription must + * be dropped. * * Three sources, in order: * 1. `sys_webhook.signing_secret` — the encrypted column. The read path @@ -314,25 +399,16 @@ export class AutoEnqueuer { * origin (#7722, #7799): a webhook that stops arriving is visible and gets * investigated, while one that keeps arriving unsigned is invisible and * teaches the receiver to accept unauthenticated traffic. - * - * Cost: one point read + one decrypt per secret-bearing row per refresh - * (default 60s), off the write path entirely. Deliberately NOT memoised - * across refreshes — the only cheap cache key would be `updated_at`, which - * nothing guarantees is stamped when a secret is rotated, and a stale key - * signs every delivery with a signature the receiver rejects. */ private async attachSecret(sub: CachedSubscription, row: any): Promise { try { const stored = await resolveWebhookSecret(this.engine, row, this.subscriptionsObject); if (stored) { sub.secret = stored; - // Recovered — a later break is a new outage and gets said loudly - // again rather than being swallowed as a repeat. - this.droppedForSecret.delete(sub.id); return true; } } catch (err) { - this.reportDrop(sub, err); + this.reportDrop(sub, err, SIGNING_SECRET_CREDENTIAL); return false; } @@ -347,7 +423,49 @@ export class AutoEnqueuer { ); sub.secret = legacy; } - this.droppedForSecret.delete(sub.id); + return true; + } + + /** + * [#7986] Resolve `sub.headers` from the encrypted column, with the same + * three-source shape as {@link attachSecret} and for the same reasons. + * + * A stored-but-unresolvable header map DROPS the subscription rather than + * delivering without it. That is the identical trade #7799 made for the + * signature, and it needs restating because the intuition runs the other + * way: a missing `Authorization` looks self-announcing, since the receiver + * answers 401 and the attempt lands in `sys_http_delivery` for anyone to + * find. But that is only the AUTHENTICATED case. Against an endpoint that + * does not require the header — a routing `X-Tenant-Id`, an + * `X-Environment: staging` — the delivery SUCCEEDS while quietly deviating + * from the configuration the author wrote, and nothing anywhere records + * that it went out incomplete. A subscription that stops is visible; a + * delivery that arrives subtly wrong is not. + */ + private async attachHeaders(sub: CachedSubscription, row: any): Promise { + try { + const stored = await resolveWebhookHeaders(this.engine, row, this.subscriptionsObject); + if (stored) { + sub.headers = stored; + return true; + } + } catch (err) { + this.reportDrop(sub, err, CUSTOM_HEADERS_CREDENTIAL); + return false; + } + + const legacy = readLegacyHeaders(row?.definition_json); + if (legacy) { + this.logger.warn?.( + `[webhook-auto-enqueuer] webhook '${sub.name}' still carries its custom headers as ` + + `CLEARTEXT in definition_json, readable over the data API (#7986) — that map is the ` + + `ordinary place an Authorization header goes. Delivery continues from it; run the boot ` + + `sweep (migrateLegacyWebhookSecrets) with a CryptoProvider wired to move them into ` + + `sys_secret.`, + { id: sub.id }, + ); + sub.headers = legacy; + } return true; } @@ -376,11 +494,15 @@ export class AutoEnqueuer { * the pair, not on message text. Same pair the seeder's refusal carries for * the same underlying cause. */ - private reportDrop(sub: CachedSubscription, err: unknown): void { + private reportDrop( + sub: CachedSubscription, + err: unknown, + credential: DropReason = SIGNING_SECRET_CREDENTIAL, + ): void { const meta = { id: sub.id, webhook: sub.name, - field: WEBHOOK_SECRET_FIELD, + field: credential.field, code: WEBHOOK_SECRET_REFUSAL_CODE, status: WEBHOOK_SECRET_REFUSAL_STATUS, err: (err as Error)?.message ?? err, @@ -388,21 +510,21 @@ export class AutoEnqueuer { if (this.droppedForSecret.has(sub.id)) { this.logger.debug?.( `[webhook-auto-enqueuer] webhook '${sub.name}' is still dropped for an unresolvable ` + - 'signing secret (#7799/#8022)', + `${credential.noun} (${credential.issues})`, meta, ); return; } this.droppedForSecret.add(sub.id); const message = - `[webhook-auto-enqueuer] webhook '${sub.name}' holds an encrypted signing secret that ` + - 'could not be decrypted — the subscription is DROPPED rather than delivered unsigned ' + - '(#7799), so every matching record change is discarded with NO delivery and NO ' + + `[webhook-auto-enqueuer] webhook '${sub.name}' holds ${credential.article} that ` + + `could not be decrypted — the subscription is DROPPED rather than ${credential.ratherThan} ` + + `(${credential.issue}), so every matching record change is discarded with NO delivery and NO ` + 'sys_http_delivery row, while the row keeps reading active:true in Setup. Fix: register a ' + 'CryptoProvider (engine.setCryptoProvider — LocalCryptoProvider in dev, KMS/Vault in ' + - 'production) with the same key the secret was written under, and make sure the sys_secret ' + - 'row is reachable; the subscription re-arms on registration (#8022) and at the next ' + - 'periodic refresh.'; + `production) with the same key the ${credential.noun} was written under, and make sure the ` + + 'sys_secret row is reachable; the subscription re-arms on registration (#8022) and at the ' + + 'next periodic refresh.'; // The logger surface is a subset of console/kernel logger — `error` is // optional on it, so fall back rather than silently losing the report // on a logger that only implements `warn`. @@ -478,10 +600,11 @@ export class AutoEnqueuer { return null; } - // The "definition_json" field carries advanced config (headers, - // timeout); attempt a best-effort parse. Fall back to top-level fields - // where present. It no longer carries the signing secret (#7799) — - // `attachSecret` sources that from the encrypted column. + // The "definition_json" field carries advanced config (timeout); + // attempt a best-effort parse. Fall back to top-level fields where + // present. It no longer carries either credential — the signing secret + // (#7799) and the custom headers (#7986) are both sourced from their + // encrypted columns by `attachCredentials`. let defn: Record = {}; if (typeof row.definition_json === 'string' && row.definition_json.length > 0) { try { @@ -502,9 +625,9 @@ export class AutoEnqueuer { // method regardless of whether the row was authored before or after // the select change (legacy rows stored 'POST'). method: String(row.method ?? defn.method ?? 'POST').toUpperCase(), - headers: defn.headers, - // `secret` is filled by attachSecret() from the encrypted column, - // NOT read off the row — see #7799. + // `headers` and `secret` are both filled by attachCredentials() + // from their encrypted columns, NOT read off the row — see #7799 + // (secret) and #7986 (headers). timeoutMs: defn.timeoutMs, }; } diff --git a/packages/plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts b/packages/plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts index e4e857ea33..637172d734 100644 --- a/packages/plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts +++ b/packages/plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts @@ -23,9 +23,10 @@ * - `isActive` → `active` * - `triggers` / `url` / `method` / `label` / `description` → same-named columns * - `secret` → `signing_secret` (ENCRYPTED — see below) + * - `headers` → `headers_secret` (ENCRYPTED — #7986, same channel) * - the rest of the parsed {@link Webhook} → `definition_json` (JSON string) * - * ## The secret does NOT go in `definition_json` (#7799) + * ## Neither credential goes in `definition_json` (#7799, #7986) * It used to: `definition_json: JSON.stringify(wh)` serialized the whole * envelope, key included, into an ordinary textarea on an admin-authorable * object — so `GET /api/v1/data/sys_webhook` returned the receiver's only proof @@ -66,6 +67,12 @@ import { isSecretProtectionFailure, splitWebhookSecret, } from './webhook-secret.js'; +import { + WEBHOOK_HEADERS_FIELD, + headersPatch, + serializeHeaders, + splitWebhookHeaders, +} from './webhook-headers.js'; /** System write context — the boot seeder is not an admin authoring action. */ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; @@ -170,6 +177,12 @@ export async function bootstrapDeclaredWebhooks( id: row.id, ...mapWebhookToRow(wh), ...(await secretPatch(engine, wh, row, subscriptionsObject)), + ...(await headersPatch( + engine, + splitWebhookHeaders(wh as Record).headers, + row, + subscriptionsObject, + )), // Adopt pristine/legacy (pre-provenance) rows so future boots // recognize them as package-managed. managed_by: 'package', @@ -181,6 +194,7 @@ export async function bootstrapDeclaredWebhooks( } const { secret } = splitWebhookSecret(wh as Record); + const { headers } = splitWebhookHeaders(wh as Record); const newRow = { id: uid('whk'), ...mapWebhookToRow(wh), @@ -189,6 +203,10 @@ export async function bootstrapDeclaredWebhooks( // ref behind. Omitted entirely when unauthored, so a webhook with no // secret costs no crypto and needs no CryptoProvider. ...(secret ? { [WEBHOOK_SECRET_FIELD]: secret } : {}), + // [#7986] Same channel, same rule, for the header map — omitted when + // unauthored so a header-less webhook still seeds on a host with no + // CryptoProvider wired. + ...(headers ? { [WEBHOOK_HEADERS_FIELD]: serializeHeaders(headers) } : {}), managed_by: 'package', customized: false, created_at: now, @@ -269,13 +287,14 @@ async function secretPatch( /** * Translate a validated {@link Webhook} into `sys_webhook` column values. - * `object → object_name`, `isActive → active`; the envelope MINUS its `secret` - * is stashed in `definition_json` for the enqueuer's advanced-config read - * (headers/timeout). The key itself is written separately, into the encrypted - * `signing_secret` column — see the module header and #7799. + * `object → object_name`, `isActive → active`; the envelope MINUS its two + * credential passengers — `secret` (#7799) and `headers` (#7986) — is stashed + * in `definition_json` for the enqueuer's advanced-config read (`timeoutMs`). + * Both credentials are written separately, into their own encrypted columns. */ function mapWebhookToRow(wh: Webhook): Record { - const { envelope } = splitWebhookSecret(wh as Record); + const { envelope: withoutSecret } = splitWebhookSecret(wh as Record); + const { envelope } = splitWebhookHeaders(withoutSecret as Record); return { name: wh.name, label: wh.label ?? wh.name, diff --git a/packages/plugins/plugin-webhooks/src/index.ts b/packages/plugins/plugin-webhooks/src/index.ts index e0a6dc1d00..b244f0e4cc 100644 --- a/packages/plugins/plugin-webhooks/src/index.ts +++ b/packages/plugins/plugin-webhooks/src/index.ts @@ -34,6 +34,12 @@ export { SysWebhook } from './sys-webhook.object.js'; * cleartext sweep, and so the column name has one spelling. */ export { WEBHOOK_SECRET_FIELD } from './webhook-secret.js'; + +/** + * [#7986] The custom-headers seam — the sibling passenger on the same blob, + * moved onto the same encrypted channel by the same boot sweep. + */ +export { WEBHOOK_HEADERS_FIELD } from './webhook-headers.js'; export { migrateLegacyWebhookSecrets, type MigrateWebhookSecretsResult, diff --git a/packages/plugins/plugin-webhooks/src/migrate-webhook-secrets.ts b/packages/plugins/plugin-webhooks/src/migrate-webhook-secrets.ts index 8eb27438e4..13fda156b7 100644 --- a/packages/plugins/plugin-webhooks/src/migrate-webhook-secrets.ts +++ b/packages/plugins/plugin-webhooks/src/migrate-webhook-secrets.ts @@ -44,8 +44,14 @@ import { WEBHOOK_SECRET_REFUSAL_STATUS, isSecretProtectionFailure, readLegacySecret, - stripSecretFromDefinition, + splitWebhookSecret, } from './webhook-secret.js'; +import { + WEBHOOK_HEADERS_FIELD, + readLegacyHeaders, + serializeHeaders, + splitWebhookHeaders, +} from './webhook-headers.js'; /** System write context — a boot reconciler is not an admin authoring action. */ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; @@ -93,8 +99,16 @@ export async function migrateLegacyWebhookSecrets( } for (const row of rows) { - const legacy = readLegacySecret(row?.definition_json); - if (!legacy || !row?.id) continue; + if (!row?.id) continue; + const legacySecret = readLegacySecret(row.definition_json); + const legacyHeaders = readLegacyHeaders(row.definition_json); + // [#7986] A row counts as found when it carries EITHER passenger. The two + // move in ONE update on purpose: two updates would mint two revisions of + // the same row, and a failure between them could land a blob stripped of + // its headers while the encrypted copy was never written — the exact + // "never widens the blast radius" rule the secret half already states, + // which only holds if the strip and the store stay in the same write. + if (!legacySecret && !legacyHeaders) continue; out.found += 1; try { @@ -102,8 +116,9 @@ export async function migrateLegacyWebhookSecrets( subscriptionsObject, { id: row.id, - [WEBHOOK_SECRET_FIELD]: legacy, - definition_json: stripSecretFromDefinition(row.definition_json as string), + ...(legacySecret ? { [WEBHOOK_SECRET_FIELD]: legacySecret } : {}), + ...(legacyHeaders ? { [WEBHOOK_HEADERS_FIELD]: serializeHeaders(legacyHeaders) } : {}), + definition_json: stripCredentialsFromDefinition(row.definition_json as string), }, { context: SYSTEM_CTX } as any, ); @@ -111,10 +126,13 @@ export async function migrateLegacyWebhookSecrets( } catch (err: any) { out.failed += 1; const protection = isSecretProtectionFailure(err); + const what = legacySecret && legacyHeaders + ? 'signing secret and custom headers' + : legacySecret ? 'signing secret' : 'custom headers'; logger?.warn?.( protection - ? '[webhook] signing secret STILL CLEARTEXT in definition_json — no CryptoProvider to encrypt it (#7799)' - : '[webhook] signing secret migration failed — row left unchanged (#7799)', + ? `[webhook] ${what} STILL CLEARTEXT in definition_json — no CryptoProvider to encrypt them (#7799/#7986)` + : `[webhook] ${what} migration failed — row left unchanged (#7799/#7986)`, { name: row.name ?? row.id, id: row.id, @@ -127,7 +145,19 @@ export async function migrateLegacyWebhookSecrets( } if (out.found > 0) { - logger?.info?.('[webhook] legacy cleartext signing secrets swept into sys_secret', { ...out }); + logger?.info?.('[webhook] legacy cleartext credentials swept into sys_secret', { ...out }); } return out; } + +/** + * Strip both credential passengers from a serialized envelope, preserving every + * other key. Parsed and re-serialized ONCE so the two removals cannot disagree + * about what the blob contained. + */ +function stripCredentialsFromDefinition(definitionJson: string): string { + const parsed = JSON.parse(definitionJson) as Record; + const { envelope: withoutSecret } = splitWebhookSecret(parsed); + const { envelope } = splitWebhookHeaders(withoutSecret as Record); + return JSON.stringify(envelope); +} diff --git a/packages/plugins/plugin-webhooks/src/sys-webhook.object.ts b/packages/plugins/plugin-webhooks/src/sys-webhook.object.ts index 15531a3bd4..cd490af04b 100644 --- a/packages/plugins/plugin-webhooks/src/sys-webhook.object.ts +++ b/packages/plugins/plugin-webhooks/src/sys-webhook.object.ts @@ -181,7 +181,47 @@ export const SysWebhook = ObjectSchema.create({ definition_json: Field.textarea({ label: 'Definition', required: true, - description: 'Serialised Webhook JSON (see @objectstack/spec/automation/webhook) — headers/timeout config. The signing secret is NOT stored here; it lives in the encrypted `signing_secret` field.', + description: 'Serialised Webhook JSON (see @objectstack/spec/automation/webhook) — timeout and the rest of the authored envelope. Credentials are NOT stored here: the signing secret lives in the encrypted `signing_secret` field and the custom headers in the encrypted `headers_secret` field.', + group: 'Definition', + }), + + /** + * [#7986] Custom HTTP headers, in the engine's ENCRYPTED credential channel. + * + * The sibling passenger #7799 left on the blob it emptied. That card was + * framed as "the signing secret is in cleartext" and was fixed exactly as + * framed — but the COLUMN was the problem, and `headers` is the ordinary + * place an `Authorization: Bearer …` goes. `definition_json` is an ordinary + * textarea on an admin-authorable object with no restrictive + * `enable.apiMethods` at all, so `GET /api/v1/data/sys_webhook` returned the + * header map to every persona that can read the object, with none of the + * retention bound that eventually ages out `sys_http_delivery`'s copies. + * + * The WHOLE map moves rather than the credential-looking entries, because + * only some entries are credentials and the platform cannot tell which: + * guessing from the header name is fail-OPEN on exactly the custom spellings + * (`X-Acme-Token`) most likely to be one, and letting the author declare + * which are sensitive is a change to the authoring envelope + * (`webhook.zod.ts`) that belongs to the spec seat. See + * `webhook-headers.ts` for the full comparison. + * + * Stored as the SERIALIZED map (the encrypted channel carries a string); + * the enqueuer recovers and re-parses it server-side through + * `engine.resolveSecretField()` when it refreshes its subscription cache, + * on the same refresh that recovers the signing secret. + * + * Fail-closed by construction, the same way `signing_secret` is: with no + * CryptoProvider the engine REFUSES the write rather than falling back to + * cleartext, and a stored map that cannot be decrypted DROPS the + * subscription rather than delivering it with its headers silently missing. + */ + headers_secret: Field.secret({ + label: 'Custom Headers', + required: false, + description: + 'Custom HTTP headers sent with each delivery, as a JSON object ({"Authorization": "Bearer …"}). ' + + 'Encrypted at rest into sys_secret; reads return a mask, never the headers. Leave the mask ' + + 'untouched to keep the current value.', group: 'Definition', }), diff --git a/packages/plugins/plugin-webhooks/src/translations/en.objects.generated.ts b/packages/plugins/plugin-webhooks/src/translations/en.objects.generated.ts index 75b291cab8..cae6601e96 100644 --- a/packages/plugins/plugin-webhooks/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-webhooks/src/translations/en.objects.generated.ts @@ -65,6 +65,10 @@ export const enObjects: NonNullable = { label: "Definition", help: "Serialised Webhook JSON (see @objectstack/spec/automation/webhook) — full headers/auth/retry/payload config" }, + headers_secret: { + label: "Custom Headers", + help: "Custom HTTP headers sent with each delivery, as a JSON object ({\"Authorization\": \"Bearer …\"}). Encrypted at rest into sys_secret; reads return a mask, never the headers. Leave the mask untouched to keep the current value." + }, signing_secret: { label: "Signing Secret", help: "HMAC-SHA256 key used to sign deliveries (X-Objectstack-Signature). Encrypted at rest into sys_secret; reads return a mask, never the key. Leave the mask untouched to keep the current value." diff --git a/packages/plugins/plugin-webhooks/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-webhooks/src/translations/es-ES.objects.generated.ts index a89c62e52a..3c8f4ff750 100644 --- a/packages/plugins/plugin-webhooks/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-webhooks/src/translations/es-ES.objects.generated.ts @@ -65,6 +65,10 @@ export const esESObjects: NonNullable = { label: "Definición", help: "JSON serializado de Webhook (consulte @objectstack/spec/automation/webhook): configuración completa de cabeceras/auth/reintentos/payload." }, + headers_secret: { + label: "Cabeceras personalizadas", + help: "Cabeceras HTTP personalizadas enviadas con cada entrega, como un objeto JSON ({\"Authorization\": \"Bearer …\"}). Se cifran en reposo en sys_secret; las lecturas devuelven una máscara, nunca las cabeceras. Deje la máscara sin tocar para conservar el valor actual." + }, signing_secret: { label: "Secreto de firma", help: "Clave HMAC-SHA256 usada para firmar las entregas (X-Objectstack-Signature). Se cifra en reposo en sys_secret; las lecturas devuelven una máscara, nunca la clave. Deje la máscara sin tocar para conservar el valor actual." diff --git a/packages/plugins/plugin-webhooks/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-webhooks/src/translations/ja-JP.objects.generated.ts index 0c8030e234..47c992c59b 100644 --- a/packages/plugins/plugin-webhooks/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-webhooks/src/translations/ja-JP.objects.generated.ts @@ -65,6 +65,10 @@ export const jaJPObjects: NonNullable = { label: "定義", help: "シリアライズされた Webhook JSON（@objectstack/spec/automation/webhook 参照）— ヘッダー/認証/リトライ/ペイロード設定を含む" }, + headers_secret: { + label: "カスタムヘッダー", + help: "各配信で送信するカスタム HTTP ヘッダー。JSON オブジェクト形式（{\"Authorization\": \"Bearer …\"}）。保存時は sys_secret に暗号化され、読み取りではマスクのみが返りヘッダーは返りません。マスクをそのままにすると現在の値が維持されます。" + }, signing_secret: { label: "署名シークレット", help: "配信の署名に使う HMAC-SHA256 キー（X-Objectstack-Signature）。保存時は sys_secret に暗号化され、読み取りではマスクのみが返りキーは返りません。マスクをそのままにすると現在の値が維持されます。" diff --git a/packages/plugins/plugin-webhooks/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-webhooks/src/translations/zh-CN.objects.generated.ts index 40d656bac7..af20881855 100644 --- a/packages/plugins/plugin-webhooks/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-webhooks/src/translations/zh-CN.objects.generated.ts @@ -65,6 +65,10 @@ export const zhCNObjects: NonNullable = { label: "定义", help: "序列化的 Webhook JSON（参见 @objectstack/spec/automation/webhook）——包含完整的 headers/auth/retry/payload 配置" }, + headers_secret: { + label: "自定义请求头", + help: "随每次投递发送的自定义 HTTP 请求头，采用 JSON 对象格式（{\"Authorization\": \"Bearer …\"}）。静态存储时加密写入 sys_secret；读取只返回掩码，绝不返回请求头内容。保持掩码不变即保留当前值。" + }, signing_secret: { label: "签名密钥", help: "用于对投递请求签名的 HMAC-SHA256 密钥（X-Objectstack-Signature）。静态存储时加密写入 sys_secret；读取只返回掩码，绝不返回密钥。保持掩码不变即保留当前值。" diff --git a/packages/plugins/plugin-webhooks/src/webhook-headers.ts b/packages/plugins/plugin-webhooks/src/webhook-headers.ts new file mode 100644 index 0000000000..fcc94f3ed1 --- /dev/null +++ b/packages/plugins/plugin-webhooks/src/webhook-headers.ts @@ -0,0 +1,235 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7986] The persistence seam for a webhook's custom `headers` map — the + * sibling passenger #7799 left behind on the blob it emptied. + * + * ## The defect + * #7799 moved the signing secret out of `sys_webhook.definition_json` into an + * encrypted column. It did not move `headers`, and `headers` is the ordinary + * place an `Authorization: Bearer …` goes. Same column, same object with no + * `enable` block at all (so the FULL default data API), same unbounded + * retention — the only thing that differed was which key of the blob the card + * happened to name. `GET /api/v1/data/sys_webhook` handed the header map back + * to every persona that can read the object. + * + * That framing is the finding: the COLUMN was the problem and the secret was + * only one of its passengers. + * + * ## Why the WHOLE map moves, and not just the credential-looking entries + * A signing secret is one opaque value with one consumer. `headers` is an + * open-ended `Record` in which only some entries are + * credentials — and **the platform cannot tell which**. Three ways to decide + * were on the table; this is why the map moves whole: + * + * - **Guess from the header NAME** (`authorization`, `x-api-key`, …). Rejected: + * it is fail-OPEN on precisely the names most likely to be a credential in + * practice — `X-Acme-Token`, `X-Vendor-Key` — and a heuristic that silently + * passes the one header that mattered is worse than no heuristic, because it + * reads as coverage. Every other credential decision in this repo fails + * closed; this one would not. + * - **Have the author DECLARE which are sensitive** (`secretHeaders: [...]`). + * That is a change to the authoring envelope (`webhook.zod.ts`), which is + * the spec seat's surface, not this one — and it would still leave the + * `source: 'flow'` half of the same exposure untouched, because a flow + * `http` node's headers are interpolated per run and never parsed through + * `WebhookSchema` at all. Escalated rather than attempted here (#7986). + * - **Move the whole map.** Fail-closed by construction, needs no authoring + * change, and the cost it is accused of — "it encrypts non-sensitive headers + * too" — is measured and small: `definition_json` is a raw JSON textarea + * pending a real builder (see `sys-webhook.object.ts`), so what an admin + * loses is the ability to READ back a `Content-Type` they typed, on a + * surface that was never the intended authoring UI. + * + * ## The seam + * Identical in shape to `webhook-secret.ts`, deliberately — one mechanism, two + * passengers, so a reader who has understood #7799 has already understood this: + * + * authored `headers` → `sys_webhook.headers_secret` (`type: 'secret'`) + * → engine encrypts the SERIALIZED map → `sys_secret` + * → row keeps only an opaque `secret:` ref + * → every read path returns the mask + * + * `definition_json` → the same envelope MINUS `headers` (and MINUS + * `secret`, as #7799 already established) + * + * The map is serialized because the encrypted channel carries a string. That is + * an encoding detail and not a second format: {@link parseStoredHeaders} is the + * only reader, and it treats anything that is not a flat string map as absent + * rather than guessing. + * + * ## What this file deliberately does NOT do + * - It does not invent a second cipher store, for the same layering reason + * `webhook-secret.ts` gives: the engine owns the `ICryptoProvider`, so the + * plugin writes cleartext INTO the `secret`-typed column exactly once and + * lets the engine's write path wrap it. The fail-closed posture comes free. + * - It does not deliver partially. A row whose stored headers cannot be + * resolved DROPS the subscription rather than delivering it with the headers + * missing — see {@link resolveWebhookHeaders}. + */ + +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { isOpaqueSecretForm } from './webhook-secret.js'; + +/** Column on `sys_webhook` holding the encrypted custom-header map. */ +export const WEBHOOK_HEADERS_FIELD = 'headers_secret'; + +/** Engines that expose the privileged dereference (ObjectQL ≥ #7799). */ +type SecretResolvingEngine = IDataEngine & { + resolveSecretField?(object: string, recordId: string, field: string): Promise; +}; + +/** A header map, as the authoring envelope declares it. */ +export type WebhookHeaders = Record; + +/** + * True when `value` is a flat `Record` with at least one entry. + * + * Anything else — an array, a nested object, a map of numbers — is treated as + * ABSENT rather than coerced. A header map is about to be written onto the + * wire; a coerced `[object Object]` header value is a silently corrupted + * request, and the authoring schema (`z.record(z.string(), z.string())`) + * already rejects the shape at every declared door. + */ +function isHeaderMap(value: unknown): value is WebhookHeaders { + if (!value || typeof value !== 'object' || Array.isArray(value)) return false; + const entries = Object.entries(value as Record); + if (entries.length === 0) return false; + return entries.every(([, v]) => typeof v === 'string'); +} + +/** + * Split an authored envelope into the part that is safe to serialize into + * `definition_json` and the header map that must go to the encrypted column. + * + * The map is REMOVED, not blanked, for the reason `splitWebhookSecret` gives + * about the secret: leaving `"headers": {}` behind still teaches the next + * reader that this blob is where headers live, and a later merge could refill + * it. + */ +export function splitWebhookHeaders>( + wh: T, +): { envelope: Omit; headers: WebhookHeaders | undefined } { + const { headers, ...envelope } = wh as T & { headers?: unknown }; + return { + envelope: envelope as Omit, + headers: isHeaderMap(headers) ? headers : undefined, + }; +} + +/** Serialize a header map for the encrypted column (which carries a string). */ +export function serializeHeaders(headers: WebhookHeaders): string { + return JSON.stringify(headers); +} + +/** Inverse of {@link serializeHeaders}. Non-conforming input reads as absent. */ +export function parseStoredHeaders(stored: unknown): WebhookHeaders | undefined { + if (typeof stored !== 'string' || stored.length === 0) return undefined; + try { + const parsed = JSON.parse(stored); + return isHeaderMap(parsed) ? parsed : undefined; + } catch { + return undefined; + } +} + +/** + * Read a legacy cleartext header map out of a `definition_json` blob. + * + * Rows written before this change — and rows an admin hand-edited into the + * textarea — still carry one. Returns `undefined` for anything else, including + * unparseable JSON (a malformed blob is not a credential). + */ +export function readLegacyHeaders(definitionJson: unknown): WebhookHeaders | undefined { + if (typeof definitionJson !== 'string' || definitionJson.length === 0) return undefined; + try { + const parsed = JSON.parse(definitionJson) as { headers?: unknown } | null; + return isHeaderMap(parsed?.headers) ? parsed.headers : undefined; + } catch { + return undefined; + } +} + +/** + * Recover a row's custom headers. Returns `undefined` when the row stores none + * — which is not an error: `headers` is optional on the authoring envelope. + * + * Throws when headers ARE stored but cannot be dereferenced. Callers must treat + * that as "drop this subscription", never as "deliver without them" — see + * `AutoEnqueuer.attachCredentials` for why partial delivery is the invisible + * failure and a stopped subscription is the visible one. + */ +export async function resolveWebhookHeaders( + engine: IDataEngine, + row: { id: string; [k: string]: unknown }, + object: string, +): Promise { + const stored = row[WEBHOOK_HEADERS_FIELD]; + // Unset / cleared. On the generic read path a set secret comes back as the + // engine's mask (a non-empty string) and an unset one as `null`, so presence + // is decidable here WITHOUT the value ever being readable. + if (stored == null || stored === '') return undefined; + + const resolver = engine as SecretResolvingEngine; + if (typeof resolver.resolveSecretField !== 'function') { + // An engine with no encrypted-field channel stored verbatim what the seeder + // handed it, so the column IS the serialized map — reading it is correct, + // not a fallback. The refusal below is for the narrow case where the value + // is one of objectql's opaque forms and there is no way to invert it. + if (!isOpaqueSecretForm(stored)) return parseStoredHeaders(stored); + throw new Error( + `Webhook "${String(row.name ?? row.id)}" stores encrypted custom headers, but this data engine ` + + 'does not implement resolveSecretField() — they cannot be recovered, so the subscription is ' + + 'dropped rather than delivered without the headers it was authored with (#7986).', + ); + } + const plain = await resolver.resolveSecretField(object, String(row.id), WEBHOOK_HEADERS_FIELD); + return parseStoredHeaders(plain); +} + +/** + * Decide what a RE-SEED should do with an existing row's `headers_secret`. + * + * Same discipline, and the same reason, as `secretPatch` in + * `bootstrap-declared-webhooks.ts`: a `secret`-typed write always mints a fresh + * `sys_secret` ciphertext row and the engine never deletes the superseded one, + * so blindly restating the declared headers on every boot would leak one orphan + * cipher row per webhook per restart. + * + * - declared map differs from stored ⇒ write it (an edit in code propagates); + * - identical ⇒ omit the key entirely, leaving the existing ref untouched; + * - declared headers removed, row still holds some ⇒ write `null` to CLEAR + * (code remains the authority for package rows); + * - engine cannot dereference (older engine, or the compare threw) ⇒ fall back + * to writing the declared value. A correct request beats tidy storage. + */ +export async function headersPatch( + engine: IDataEngine, + declared: WebhookHeaders | undefined, + row: { id: string; [k: string]: unknown }, + object: string, +): Promise> { + const hasStored = row?.[WEBHOOK_HEADERS_FIELD] != null && row[WEBHOOK_HEADERS_FIELD] !== ''; + + if (!declared) return hasStored ? { [WEBHOOK_HEADERS_FIELD]: null } : {}; + + const serialized = serializeHeaders(declared); + const resolver = engine as SecretResolvingEngine; + if (!hasStored || typeof resolver.resolveSecretField !== 'function') { + return { [WEBHOOK_HEADERS_FIELD]: serialized }; + } + + try { + const current = await resolver.resolveSecretField(object, String(row.id), WEBHOOK_HEADERS_FIELD); + // Compared as the CANONICAL serialization on both sides, not as raw + // strings: the stored form was produced by this same function, so key order + // is stable, and a re-parse guards against a hand-edited value that differs + // only in whitespace re-encrypting on every boot. + const stored = parseStoredHeaders(current); + return stored && serializeHeaders(stored) === serialized + ? {} + : { [WEBHOOK_HEADERS_FIELD]: serialized }; + } catch { + return { [WEBHOOK_HEADERS_FIELD]: serialized }; + } +} diff --git a/packages/plugins/plugin-webhooks/src/webhook-secret-at-rest.test.ts b/packages/plugins/plugin-webhooks/src/webhook-secret-at-rest.test.ts index 2a0166a19b..c99a505789 100644 --- a/packages/plugins/plugin-webhooks/src/webhook-secret-at-rest.test.ts +++ b/packages/plugins/plugin-webhooks/src/webhook-secret-at-rest.test.ts @@ -36,6 +36,7 @@ import { bootstrapDeclaredWebhooks } from './bootstrap-declared-webhooks.js'; import { migrateLegacyWebhookSecrets } from './migrate-webhook-secrets.js'; import { SysWebhook } from './sys-webhook.object.js'; import { WEBHOOK_SECRET_FIELD, __objectqlSecretWireForms } from './webhook-secret.js'; +import { WEBHOOK_HEADERS_FIELD } from './webhook-headers.js'; const SECRET = 'whsec_7799_subscriber_key'; const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; @@ -283,15 +284,28 @@ describe('webhook signing secret at rest (#7799)', () => { expect(String(stored.definition_json)).not.toContain(SECRET); // The row keeps an opaque handle, and the cipher store holds a transform. expect(String(stored[WEBHOOK_SECRET_FIELD])).toMatch(/^secret:/); + // Two cipher rows since #7986, not one: this fixture authors a header + // map as well, and it rides the same encrypted channel. Asserted by + // NAMESPACE/KEY rather than by position so the pin still says which row + // is the signing secret's, and so a future third passenger reddens here + // instead of silently shifting an index. const ciphered = Array.from(stores.get('sys_secret')!.values()) as any[]; - expect(ciphered).toHaveLength(1); - expect(ciphered[0].ciphertext).not.toContain(SECRET); + expect(ciphered).toHaveLength(2); + const secretCipher = ciphered.filter((c) => c.key === WEBHOOK_SECRET_FIELD); + expect(secretCipher).toHaveLength(1); + expect(secretCipher[0].namespace).toBe('sys_webhook'); + expect(secretCipher[0].ciphertext).not.toContain(SECRET); // The rest of the envelope survives — this is a strip, not a truncation. + // `headers` used to be asserted here as one of the survivors; #7986 + // moved it onto the same encrypted channel as the key (it is the + // ordinary place an `Authorization: Bearer …` goes), so `timeoutMs` + // carries that half of the assertion now. The headers' own at-rest and + // on-the-wire pins are in the `#7986` blocks below. expect(JSON.parse(String(stored.definition_json))).toMatchObject({ name: 'crm_hook', url: 'https://receiver.example/hook', - headers: { 'X-Team': 'crm' }, + timeoutMs: 30000, }); }); @@ -323,8 +337,10 @@ describe('webhook signing secret at rest (#7799)', () => { await bootstrapDeclaredWebhooks(engine, declared); // Every boot re-seeds package rows; a blind restatement of the key would - // leave one orphan ciphertext row per restart. - expect(stores.get('sys_secret')!.size).toBe(1); + // leave one orphan ciphertext row per restart. Two rows, not one, since + // #7986 — one per credential — and the property under test is that the + // count does not GROW across the three boots. + expect(stores.get('sys_secret')!.size).toBe(2); }); it('rotating the declared secret in code re-encrypts and signs with the new key', async () => { @@ -341,9 +357,17 @@ describe('webhook signing secret at rest (#7799)', () => { it('a webhook authored without a secret still materializes and delivers unsigned', async () => { const { engine, stores } = await buildEngine(); - await bootstrapDeclaredWebhooks(engine, metadataWith([declaredWebhook({ secret: undefined })])); - - // No crypto work at all for a secret-free webhook. + // `headers` dropped alongside `secret` so this pin keeps asserting what + // it always asserted — that a webhook with NO credentials does no crypto + // at all and therefore needs no CryptoProvider. (Since #7986 a header + // map is a credential too; the headers-only case is pinned separately in + // the #7986 block below.) + await bootstrapDeclaredWebhooks( + engine, + metadataWith([declaredWebhook({ secret: undefined, headers: undefined })]), + ); + + // No crypto work at all for a credential-free webhook. expect(stores.get('sys_secret')?.size ?? 0).toBe(0); const { calls } = await deliverOnce(engine); expect(calls).toHaveLength(1); @@ -489,9 +513,10 @@ describe('boot ordering: the cache is built before the CryptoProvider (#8022)', async function bootWithoutCrypto() { const first = await buildEngine(); await bootstrapDeclaredWebhooks(first.engine, metadataWith([declaredWebhook()])); - // Precondition: the key is at rest as ciphertext only — the exact - // population #7799 created, and the only one this defect can reach. - expect(first.stores.get('sys_secret')!.size).toBe(1); + // Precondition: the credentials are at rest as ciphertext only — the + // exact population #7799 created, and the only one this defect can + // reach. Two rows since #7986 (signing secret + header map). + expect(first.stores.get('sys_secret')!.size).toBe(2); return buildEngine({ withCrypto: false, @@ -601,3 +626,282 @@ describe('boot ordering: the cache is built before the CryptoProvider (#8022)', expect(debugs.join('\n')).toMatch(/still dropped for an unresolvable signing secret/); }); }); + +// --------------------------------------------------------------------------- +// #7986 — the SIBLING passenger on the same blob. +// +// #7799 moved the signing secret out of `definition_json`. It did not move the +// custom `headers` map, and `headers` is the ordinary place an +// `Authorization: Bearer …` goes. Same column, same absent `enable` block, same +// unbounded retention — the only thing that differed was which key of the blob +// the card happened to name. +// +// The pins below are deliberately written against a CREDENTIAL-shaped header +// alongside an ordinary one, because the shape decision this issue forced is +// exactly "the platform cannot tell which is which" (see the PR body): both +// move, and the pin has to show the ordinary one still arrives on the wire. +// --------------------------------------------------------------------------- + +/** A credential a real deployment would put in `headers`. Never persisted. */ +const BEARER = 'Bearer prod_tok_7986_do_not_leak'; + + +/** Declared webhook whose header map carries one credential and one ordinary key. */ +function headerBearingWebhook(overrides: Record = {}) { + return declaredWebhook({ + headers: { Authorization: BEARER, 'X-Team': 'crm' }, + ...overrides, + }); +} + +describe('webhook custom headers at rest (#7986)', () => { + it('the credential header appears nowhere in the persisted sys_webhook row', async () => { + const { engine, stores } = await buildEngine(); + + await bootstrapDeclaredWebhooks(engine, metadataWith([headerBearingWebhook()])); + + // ── The read path an ordinary GET /api/v1/data/sys_webhook takes ── + // `sys_webhook` declares no `enable` block at all, so this is the FULL + // default data API — the same condition #7799 called out for the key. + // Substring, not a field check: the defect is the credential NESTED in + // a serialized blob, which any per-field assertion walks straight past. + const viaApi = await engine.find('sys_webhook', { where: { name: 'crm_hook' } }); + expect(viaApi).toHaveLength(1); + expect(JSON.stringify(viaApi)).not.toContain(BEARER); + expect(String(viaApi[0].definition_json)).not.toContain(BEARER); + + // ── And at rest, in the bytes the table itself holds ── + const stored = Array.from(stores.get('sys_webhook')!.values())[0] as any; + expect(JSON.stringify(stored)).not.toContain(BEARER); + expect(String(stored.definition_json)).not.toContain(BEARER); + // The whole map moves, so the ordinary header leaves the blob too — the + // platform cannot tell which entry is the credential, and guessing is + // fail-OPEN on exactly the custom names (`X-Acme-Token`) most likely to + // be one. It is still DELIVERED; the pin below is what proves that. + expect(String(stored.definition_json)).not.toContain('X-Team'); + expect(String(stored[WEBHOOK_HEADERS_FIELD])).toMatch(/^secret:/); + + // The rest of the envelope survives — this is a strip, not a truncation. + expect(JSON.parse(String(stored.definition_json))).toMatchObject({ + name: 'crm_hook', + url: 'https://receiver.example/hook', + timeoutMs: 30000, + }); + }); + + it('still delivers every authored header on the wire, credential included', async () => { + const { engine } = await buildEngine(); + await bootstrapDeclaredWebhooks(engine, metadataWith([headerBearingWebhook()])); + + const { calls, outbox } = await deliverOnce(engine); + + // The point of the whole feature: the receiver still gets what the + // author declared. A fix that merely DELETED the headers from the blob + // would pass the at-rest pin above and break every authenticated + // webhook in production — this is the pin that separates the two. + expect(calls).toHaveLength(1); + expect(calls[0].headers.Authorization).toBe(BEARER); + expect(calls[0].headers['X-Team']).toBe('crm'); + // …and the signing half is untouched (#7799 / #7722). + const expected = createHmac('sha256', SECRET).update(calls[0].body).digest('hex'); + expect(calls[0].headers['X-Objectstack-Signature']).toBe(`sha256=${expected}`); + expect(JSON.stringify(await outbox.list())).not.toContain(SECRET); + }); + + it('re-seeding an unchanged webhook mints no extra cipher rows for the headers', async () => { + const { engine, stores } = await buildEngine(); + const declared = metadataWith([headerBearingWebhook()]); + + await bootstrapDeclaredWebhooks(engine, declared); + await bootstrapDeclaredWebhooks(engine, declared); + await bootstrapDeclaredWebhooks(engine, declared); + + // A `secret`-typed write ALWAYS mints a fresh `sys_secret` row and the + // engine never deletes the old one, so a blind restatement leaks one + // orphan cipher row per webhook per restart — the reason + // `secretPatch` compares before writing. The headers channel owes the + // same discipline. Exactly two: one signing secret, one header map. + expect(stores.get('sys_secret')!.size).toBe(2); + }); + + it('rotating the declared headers in code re-encrypts and delivers the new value', async () => { + const { engine, stores } = await buildEngine(); + await bootstrapDeclaredWebhooks(engine, metadataWith([headerBearingWebhook()])); + await bootstrapDeclaredWebhooks( + engine, + metadataWith([headerBearingWebhook({ headers: { Authorization: 'Bearer rotated_7986' } })]), + ); + + const { calls } = await deliverOnce(engine); + expect(calls[0].headers.Authorization).toBe('Bearer rotated_7986'); + expect(calls[0].headers['X-Team']).toBeUndefined(); + // Code remains the authority for package rows: the superseded map is + // not readable from the row any more either. + const stored = Array.from(stores.get('sys_webhook')!.values())[0] as any; + expect(JSON.stringify(stored)).not.toContain(BEARER); + }); + + it('a webhook authored without headers still materializes and delivers', async () => { + const { engine, stores } = await buildEngine(); + await bootstrapDeclaredWebhooks(engine, metadataWith([declaredWebhook({ headers: undefined })])); + + const { calls } = await deliverOnce(engine); + expect(calls).toHaveLength(1); + // No headers authored ⇒ no header cipher row: only the signing secret's. + expect(stores.get('sys_secret')!.size).toBe(1); + const stored = Array.from(stores.get('sys_webhook')!.values())[0] as any; + expect(stored[WEBHOOK_HEADERS_FIELD] ?? null).toBeNull(); + }); +}); + +describe('legacy cleartext headers migration (#7986)', () => { + /** A pre-#7986 row: headers still in the blob, key already swept by #7799. */ + async function seedLegacyHeaderRow(engine: any) { + await engine.insert('sys_webhook', { + id: 'whk_legacy_hdr', + name: 'legacy_hdr_hook', + label: 'Legacy headers', + object_name: 'contact', + triggers: ['create'], + url: 'https://receiver.example/hook', + method: 'post', + active: true, + definition_json: JSON.stringify({ + name: 'legacy_hdr_hook', url: 'https://receiver.example/hook', + headers: { Authorization: BEARER, 'X-Team': 'crm' }, timeoutMs: 30000, + }), + managed_by: 'admin', + customized: true, + created_at: '2026-01-01T00:00:00.000Z', + }, { context: SYSTEM_CTX } as any); + } + + it('sweeps an admin-authored row the seeder never touches, and keeps it delivering', async () => { + const { engine, stores } = await buildEngine(); + await seedLegacyHeaderRow(engine); + + // Precondition: this is genuinely the exposed shape. + expect(JSON.stringify(await engine.find('sys_webhook', {}))).toContain(BEARER); + + const result = await migrateLegacyWebhookSecrets(engine); + expect(result).toMatchObject({ found: 1, migrated: 1, failed: 0 }); + + // Gone from the API read AND from the bytes at rest. + expect(JSON.stringify(await engine.find('sys_webhook', {}))).not.toContain(BEARER); + expect(JSON.stringify(Array.from(stores.get('sys_webhook')!.values()))).not.toContain(BEARER); + + // …and the same headers still reach the receiver. + const { calls } = await deliverOnce(engine); + expect(calls[0].headers.Authorization).toBe(BEARER); + expect(calls[0].headers['X-Team']).toBe('crm'); + + // The sweep does not re-freeze provenance, and is free on the next boot. + const row = Array.from(stores.get('sys_webhook')!.values())[0] as any; + expect(row.managed_by).toBe('admin'); + expect(await migrateLegacyWebhookSecrets(engine)).toMatchObject({ found: 0, migrated: 0, failed: 0 }); + // Idempotent in the strong sense: the re-run minted no second cipher row. + expect(stores.get('sys_secret')!.size).toBe(1); + }); + + it('an un-swept row keeps delivering — the enqueuer reads the legacy blob and says so', async () => { + const { engine } = await buildEngine(); + await seedLegacyHeaderRow(engine); + + const warnings: string[] = []; + const realtime = new FakeRealtime(); + const outbox = new MemoryHttpOutbox(); + const enqueuer = new AutoEnqueuer(engine, realtime, (i) => outbox.enqueue(i), { + logger: { warn: (m: string) => { warnings.push(m); } }, + }); + await enqueuer.start(); + await realtime.publish(recordEvent('contact', { id: 'c1', name: 'Ada' })); + 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(); + + expect(calls[0].headers.Authorization).toBe(BEARER); + expect(warnings.join('\n')).toMatch(/CLEARTEXT in definition_json/); + }); +}); + +describe('fail-closed and re-arm, extended to headers (#7986 × #7799/#8022)', () => { + async function bootWithoutCrypto() { + const first = await buildEngine(); + await bootstrapDeclaredWebhooks(first.engine, metadataWith([headerBearingWebhook()])); + // Precondition: BOTH credentials are at rest as ciphertext only. + expect(first.stores.get('sys_secret')!.size).toBe(2); + return buildEngine({ + withCrypto: false, + reuse: { driver: first.driver, stores: first.stores }, + }); + } + + const settle = async () => { + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + }; + + it('drops the subscription rather than delivering it with its headers missing', async () => { + const { engine } = await bootWithoutCrypto(); + const realtime = new FakeRealtime(); + const outbox = new MemoryHttpOutbox(); + const errors: Array<{ msg: string; meta: any }> = []; + const enqueuer = new AutoEnqueuer(engine, realtime, (i) => outbox.enqueue(i), { + refreshIntervalMs: 0, + logger: { + error: (msg: string, _e?: unknown, meta?: unknown) => { errors.push({ msg, meta: meta as any }); }, + debug: () => {}, warn: () => {}, + }, + }); + await enqueuer.start(); + await realtime.publish(recordEvent('contact', { id: 'c1', name: 'Ada' })); + await settle(); + const { impl, calls } = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick(); + await enqueuer.stop(); + + // Same boundary #7799 drew for the signature, for the same reason: a + // delivery that ARRIVES having silently dropped the `Authorization` the + // author declared is an invisible deviation from the authored config — + // and against an endpoint that does not require auth it even succeeds. + // A subscription that stops is visible and gets investigated. + expect(calls).toHaveLength(0); + expect(await outbox.list()).toHaveLength(0); + // ADR-0112 — one report, carrying the pair a consumer branches on. + expect(errors).toHaveLength(1); + expect(errors[0].meta).toMatchObject({ code: 'INTERNAL_ERROR', status: 500 }); + }); + + it('re-arms with its headers when the CryptoProvider registers (#8022 timing, unchanged)', async () => { + const { engine } = await bootWithoutCrypto(); + const realtime = new FakeRealtime(); + const outbox = new MemoryHttpOutbox(); + const enqueuer = new AutoEnqueuer(engine, realtime, (i) => outbox.enqueue(i), { + // The 60s escape hatch held shut: only the registration can re-arm. + refreshIntervalMs: 0, + logger: { error: () => {}, warn: () => {}, debug: () => {} }, + }); + await enqueuer.start(); + await realtime.publish(recordEvent('contact', { id: 'c_window', name: 'Ada' })); + await settle(); + expect(await outbox.list()).toHaveLength(0); + + engine.setCryptoProvider(makeFakeCrypto()); + await settle(); + + await realtime.publish(recordEvent('contact', { id: 'c_rearmed', name: 'Grace' })); + await settle(); + const { impl, calls } = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick(); + await enqueuer.stop(); + + // Headers must be recovered on the SAME re-armed build as the secret — + // an implementation that resolved them on a different schedule would + // re-arm into a delivery with no `Authorization` on it. + expect(calls).toHaveLength(1); + expect(calls[0].headers.Authorization).toBe(BEARER); + const expected = createHmac('sha256', SECRET).update(calls[0].body).digest('hex'); + expect(calls[0].headers['X-Objectstack-Signature']).toBe(`sha256=${expected}`); + }); +}); diff --git a/packages/plugins/plugin-webhooks/src/webhook-secret.ts b/packages/plugins/plugin-webhooks/src/webhook-secret.ts index ec3632b644..14dcd67680 100644 --- a/packages/plugins/plugin-webhooks/src/webhook-secret.ts +++ b/packages/plugins/plugin-webhooks/src/webhook-secret.ts @@ -111,12 +111,12 @@ export function readLegacySecret(definitionJson: unknown): string | undefined { } } -/** Strip `secret` from a serialized envelope, preserving every other key. */ -export function stripSecretFromDefinition(definitionJson: string): string { - const parsed = JSON.parse(definitionJson) as Record; - const { envelope } = splitWebhookSecret(parsed); - return JSON.stringify(envelope); -} +// `stripSecretFromDefinition` lived here until #7986. Its single caller — the +// boot sweep — now has to remove BOTH credential passengers from the blob, and +// doing that as two independent parse/serialize round-trips would let the two +// removals disagree about what the blob contained. The sweep owns one +// `stripCredentialsFromDefinition` instead, built from `splitWebhookSecret` + +// `splitWebhookHeaders` over a single parse. /** * objectql's two wire forms for the encrypted channel, restated here ONLY as a diff --git a/packages/spec/liveness/webhook.json b/packages/spec/liveness/webhook.json index 38fb410a35..a4039eabaf 100644 --- a/packages/spec/liveness/webhook.json +++ b/packages/spec/liveness/webhook.json @@ -1,6 +1,6 @@ { "type": "webhook", - "_note": "WebhookSchema (outbound webhook — packages/spec/src/automation/webhook.zod.ts). Governed via a spec-only schema override in the gate (SPEC_ONLY_SCHEMAS): webhook is still NOT a registered metadata type (absent from kernel/metadata-type-schemas.ts) — registering it would turn on Studio webhook CRUD + saveMetaItem overlay + create-seeds; that reassessment is tracked in #3490 and deliberately deferred. TWO THINGS CLOSED THE OLD 'entire surface is dead' classification: (1) #3494 PRUNED the aspirational dead props outright — body / payloadFields / includeSession / retryPolicy / tags / authentication are gone from the schema; (2) the #3461 materializer bridge (PR #3489) makes every REMAINING prop live. `bootstrapDeclaredWebhooks` (plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts) materializes each stack/connector-authored webhook into a `sys_webhook` DATA row on boot — WebhookSchema.parse (:114) → mapWebhookToRow (:191): `object`→`object_name` (:195), `isActive`→`active` (:202), same-named `name`/`label`/`triggers`/`url`/`method`/`description`, `secret`→the encrypted `signing_secret` column (#7799), and the REST of the envelope → `definition_json`. The dispatcher (AutoEnqueuer) reads those rows (auto-enqueuer.ts:175 `where:{active:true}`) and fans out on data.record.* events (plus the aggregate data.records.* a predicate write publishes, dispatched under the opt-in bulk_update/bulk_delete triggers — #4639), reading `object_name`/`name`/`url`/`method`/`triggers` off the row, `headers`/`timeoutMs` back out of `definition_json`, and the signing key through `engine.resolveSecretField()` against the encrypted `signing_secret` column (#7799). So all 11 remaining props are LIVE; there is no dead/experimental prop left, so nothing carries an authorWarn (the old per-webhook `url` heads-up is gone — authoring is no longer a no-op). Seed-not-clobber: an admin-edited row (`customized`) is never re-seeded (bootstrap-declared-webhooks.ts:143). The `object` prop carries the ADR-0054 runtime proof for the whole materialization pipeline (bound high-risk class `webhook-materialization`). Field-level line refs: materializer bootstrap-declared-webhooks.ts, runtime object plugins/plugin-webhooks/src/sys-webhook.object.ts, dispatcher auto-enqueuer.ts.", + "_note": "WebhookSchema (outbound webhook — packages/spec/src/automation/webhook.zod.ts). Governed via a spec-only schema override in the gate (SPEC_ONLY_SCHEMAS): webhook is still NOT a registered metadata type (absent from kernel/metadata-type-schemas.ts) — registering it would turn on Studio webhook CRUD + saveMetaItem overlay + create-seeds; that reassessment is tracked in #3490 and deliberately deferred. TWO THINGS CLOSED THE OLD 'entire surface is dead' classification: (1) #3494 PRUNED the aspirational dead props outright — body / payloadFields / includeSession / retryPolicy / tags / authentication are gone from the schema; (2) the #3461 materializer bridge (PR #3489) makes every REMAINING prop live. `bootstrapDeclaredWebhooks` (plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts) materializes each stack/connector-authored webhook into a `sys_webhook` DATA row on boot — WebhookSchema.parse (:114) → mapWebhookToRow (:191): `object`→`object_name` (:195), `isActive`→`active` (:202), same-named `name`/`label`/`triggers`/`url`/`method`/`description`, `secret`→the encrypted `signing_secret` column (#7799), `headers`→the encrypted `headers_secret` column (#7986), and the REST of the envelope → `definition_json`. The dispatcher (AutoEnqueuer) reads those rows (auto-enqueuer.ts:175 `where:{active:true}`) and fans out on data.record.* events (plus the aggregate data.records.* a predicate write publishes, dispatched under the opt-in bulk_update/bulk_delete triggers — #4639), reading `object_name`/`name`/`url`/`method`/`triggers` off the row, `timeoutMs` back out of `definition_json`, and BOTH credentials through `engine.resolveSecretField()` against their encrypted columns — the signing key from `signing_secret` (#7799) and the custom headers from `headers_secret` (#7986). So all 11 remaining props are LIVE; there is no dead/experimental prop left, so nothing carries an authorWarn (the old per-webhook `url` heads-up is gone — authoring is no longer a no-op). Seed-not-clobber: an admin-edited row (`customized`) is never re-seeded (bootstrap-declared-webhooks.ts:143). The `object` prop carries the ADR-0054 runtime proof for the whole materialization pipeline (bound high-risk class `webhook-materialization`). Field-level line refs: materializer bootstrap-declared-webhooks.ts, runtime object plugins/plugin-webhooks/src/sys-webhook.object.ts, dispatcher auto-enqueuer.ts.", "props": { "name": { "status": "live", @@ -35,8 +35,8 @@ }, "headers": { "status": "live", - "evidence": "Bridge folds the full envelope into sys_webhook.definition_json (bootstrap-declared-webhooks.ts:203, #3489); the dispatcher reads defn.headers and attaches them to the outbound request (auto-enqueuer.ts:275).", - "note": "Was dead pre-bridge (definition_json only reachable via hand-authored rows). Now materialized from authoring." + "evidence": "Bridge writes the authored map to sys_webhook.headers_secret — a `type: 'secret'` column the engine encrypts into sys_secret and masks on read (bootstrap-declared-webhooks.ts mapWebhookToRow + headersPatch, #7986); the dispatcher recovers and re-parses it via engine.resolveSecretField() on each cache refresh (auto-enqueuer.ts attachHeaders) and attaches the headers to the outbound request.", + "note": "Was dead pre-bridge (definition_json only reachable via hand-authored rows), then materialized from authoring into definition_json as CLEARTEXT — recoverable over the ordinary data API, which is #7986: the same column and the same absent `enable` block as #7799's signing secret, one key of the blob later. It no longer rides in that blob; the authored map itself is unchanged. The WHOLE map moves because only some entries are credentials and the platform cannot tell which." }, "timeoutMs": { "status": "live",