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",