Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-webhooks): move webhook custom headers onto the encrypted channel (#7986) - #8114
Merged
Merged
Conversation
…channel (#7986) #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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LiA1beRqJfW2XEmPVRoMqT
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
huangyiirene
marked this pull request as ready for review
August 12, 2026 16:32
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes part of #7986 — ①-a only. ①-b and ①-f are deliberately not in this PR; see What this does NOT close below.
The finding, restated
#7799 moved the webhook signing secret out of
sys_webhook.definition_jsoninto an encryptedsigning_secretcolumn. It did not move the customheadersmap — andheadersis the ordinary place anAuthorization: Bearer …goes.sys_webhookdeclares noenableblock at all, so it keeps the full default data API. An ordinaryGET /api/v1/data/sys_webhookreturned the whole header map, credentials included, to every persona that can read the object — with none of the retention bound that eventually ages outsys_http_delivery's copies.Not a regression from PR #7901: the exposure predates it and nothing that card did made it worse. What was wrong was the conclusion a reader would reasonably draw from #7799 — that webhook credentials are no longer in a blob.
The shape, and the two I rejected
The card offered three candidates and the dispatch deliberately pre-selected none. I measured, then chose option 1 — move the whole map onto the encrypted channel — for ①-a.
Why the whole map and not just the credential-looking entries. Only some entries are credentials and the platform cannot tell which.
authorization,x-api-key, …) — rejected. Fail-open on exactly the custom spellings most likely to be a credential in practice (X-Acme-Token,X-Vendor-Key). A heuristic that silently passes the one header that mattered is worse than none, because it reads as coverage. Every other credential decision in this repo fails closed.secretHeaders: [...]) — rejected here, escalated. That is a change to the authoring envelope (packages/spec/src/automation/webhook.zod.ts) and belongs to the spec surface. It would also still leave thesource: 'flow'half of the same exposure untouched: a flowhttpnode's headers are interpolated per run and never pass throughWebhookSchemaat all.sys_webhook.definition_json#7799's whole argument is that cleartext at rest is the defect, andsys_webhookis the surface that argument was made about: a configuration table with no retention bound holding the only copy, which reaches backups, replicas and exports. Nothing distinguishes the header map from the key there.The cost option 1 is accused of — "it encrypts non-sensitive headers too and the admin UI can't read them" — is measured and small:
definition_jsonis a raw JSON textarea pending a real builder (sys-webhook.object.tssays so in as many words), so what an admin loses is the ability to read back aContent-Typethey typed, on a surface that was never the intended authoring UI. Writing still works, with the same mask-echo semanticssigning_secretalready has.What changed
The authored
headersmap lands in a newsys_webhook.headers_secretcolumn on the engine's encrypted credential channel, exactly assigning_secretdoes — engine encrypts intosys_secret, row keeps an opaquesecret:<id>ref, every read path returns a mask.definition_jsoncarries the same envelope minus both credential passengers.webhook-headers.ts(new) — the seam, shaped deliberately likewebhook-secret.tsso a reader who has understood [security] The webhook signing secret is stored in cleartext insys_webhook.definition_json#7799 has already understood this.bootstrap-declared-webhooks.ts— routesheadersto the new column, with aheadersPatchcompare-before-write mirroringsecretPatch. Asecret-typed write always mints a freshsys_secretrow and the engine never deletes the superseded one, so a blind restatement would leak one orphan cipher row per webhook per restart.auto-enqueuer.ts—attachSecret+ newattachHeaders, composed byattachCredentials. See the Regression from #7799: for ~60s after every restart, a webhook holding an encrypted signing secret silently drops its subscription — no delivery, nosys_http_deliveryrow, while it still readsactive:true#8022 reading below.migrate-webhook-secrets.ts— the existing boot sweep now moves cleartext headers too, in the same single update as the key. Two updates could land a blob stripped of its headers while the encrypted copy was never written, which breaks the sweep's own "never widens the blast radius" rule.packages/spec/liveness/webhook.json— theheadersprop's evidence described a sink that no longer exists (dispatcher reads defn.headers). Updated; 3-line diff, status stayslive.Nothing about authoring changes.
webhook.zod.tsis untouched, authors still writeheaders: { … }, and every authored header is still delivered on the wire byte-for-byte.Fail-closed, and the explicit #8022 reading
The drop rule. A stored map that cannot be decrypted drops the subscription rather than delivering without it — the same trade #7799 made for the signature. This needs restating because the intuition runs the other way: a missing
Authorizationlooks self-announcing, since the receiver answers 401 and the attempt lands insys_http_delivery. But that is only the authenticated case. Against an endpoint that does not require the header — a routingX-Tenant-Id, anX-Environment: staging— the delivery succeeds while quietly deviating from the configuration the author wrote, and nothing anywhere records that it went out incomplete.#8022's re-arm timing is unchanged, and that is measured, not assumed.
onCryptoProviderChangeis still bound before the first cache build;rearmAfterCryptoRegisteredstill refuses to coalesce onto the in-flight build. What changed is insidedoRefresh: both credentials are now resolved on the same rebuilt cache. That is load-bearing — a header map recovered on any other cadence would let the enqueuer re-arm into a delivery that is correctly signed and silently missing itsAuthorization, which is both cards' failure mode at once. Pinned by re-arms with its headers when the CryptoProvider registers.The say-once ledger.
droppedForSecretis now cleared only when both credentials resolve. Clearing it per-credential would let a row whose secret resolves and whose headers do not wipe its own "already reported" mark every refresh and shout the sameerrorevery 60s — precisely the unreadable-error-channel failure #8022's say-once rule exists to prevent.One behavioural widening, stated plainly. The pre-crypto boot window now drops header-bearing webhooks as well as secret-bearing ones.
#8022's re-arm closes that window on registration, andpackages/cli/src/commands/serve.tsalways wires a provider (LocalCryptoProvider in dev; production refuses to start without a stable key), so the window is the one #8022 already characterised. A webhook with no credentials still needs no CryptoProvider at all — pinned.Reverse verification
Every pin's reading on
origin/main(9e45153) and after. 10 new pins; full suite 56/56 green after.origin/mainsys_webhookrowdefinition_jsonand in the data-API readsys_secretrow per restart if the headers write is unconditionalsigning_secretstill ciphertext, signing still byte-identicalcode+status† Pins 9 and 10 are red on
mainat their precondition (sys_secretholds 2 cipher rows — one per credential), because onmainonly one credential is encrypted. Their behavioural assertions are what they guard after the fix. Flagging this rather than claiming a cleaner red: the population they need does not exist onmain.Pin 5 is the weakest of the set — on
maintheheaders_secretcolumn does not exist, sostored[field] ?? nullis triviallynull. Its non-vacuous half on both sides is thesys_secretsize assertion.Existing assertions I changed (not deleted): five #7799 pins asserted
sys_secretrow counts written when the shared fixture carried one credential; the fixture authors headers too, so the counts are now 2 (or 1). I updated the counts and kept what each pin guards, and re-pointed the "cipher store holds a transform" assertion to select bynamespace/keyinstead of by array position so it still says which row is the signing secret's. One assertion in the secret's bytes appear nowhere… previously pinnedheadersas a survivor of thedefinition_jsonstrip;timeoutMscarries that half now. For the "authored without a secret" pin I droppedheadersfrom its fixture too, so it keeps asserting exactly what it always asserted — a credential-free webhook does no crypto at all.No test was deleted.
Gates
plugin-webhookssuitetsc --noEmit(plugin-webhooks)check-test-source-aliaswebhook-secret-at-rest.test.ts) and no new workspace dependency.KNOWN_UNALIASED_TEST_IMPORTSnot touched.check:type-check-coveragecheck:type-check-debt(--re-measure)check:i18n@objectstack/clibuilt); 4 locale bundles updated for the new field, following #7799's precedentmanaged_bynot re-frozen → re-run reportsfound: 0and mints no second cipher rowINTERNAL_ERROR/500pair, now with thefieldnaming which credential.changeset/webhook-custom-headers-encrypted-at-rest.mdcontent/docs/releases/**,docs/adr/**,.claude/skills/**,skills/**,sys-session.object.ts,packages/plugins/plugin-email/**,webhook.zod.ts; noenableblock added tosys_webhookWhat this does NOT close
①-b —
sys_http_delivery.headers_jsonis still cleartext. Measured on this branch, after the fix: the enqueuer decrypts the map and hands it toenqueue(), which writes it verbatim to the delivery row.{ "bearerReadableViaDataApi": true, "bearerAtRest": true, "headersJson": "{\"Authorization\":\"Bearer prod_tok_…\",\"X-Team\":\"crm\"}", "secretReadable": false, "sysSecretRows": 2 }So #7986 stays open, and the "webhook headers are no longer in a blob" reading would be exactly the mistake this issue was filed to correct about #7799. Stated in the changeset for the same reason.
I stopped rather than landing a services-lane shape for ①-b, because every available option there is either structurally wrong or exits this lane:
Field.secret()onheaders_json— structurally wrong, three ways. (a)sys_http_deliverygets one row per delivery; asecret-typed write mints asys_secretrow every time and the engine has no delete cascade and no retention onsys_secret, so every delivery would leave an orphan cipher row forever while the delivery row itself ages out at 30d. The repo already treats one orphan per webhook per restart as a bug worthsecretPatch— one per delivery is orders of magnitude worse. (b)encryptSecretFieldsthrows with no CryptoProvider, andAutoEnqueuerenqueues fire-and-forget (void … .catch(warn)), so header-bearing webhooks would silently stop delivering in Regression from #7799: for ~60s after every restart, a webhook holding an encrypted signing secret silently drops its subscription — no delivery, nosys_http_deliveryrow, while it still readsactive:true#8022's boot window with no re-arm. (c)claim()would need aresolveSecretField(driver read + decrypt) per row on the dispatcher tick.webhook.zod.tsand the flowHttpConfigSchema. This is the STOP fork; it transfers todomain:spec.internal: true—omitInternalFieldshas no system carve-out by explicit design (api-key-ui-lifecycle (secondary): thekeycolumn (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728), andSqlHttpOutbox.claim()reads headers throughengine.find, so this breaks its own consumer. api-key-ui-lifecycle (secondary): thekeycolumn (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728 itself names the remedy — "a purpose-built privileged accessor, the wayresolveSecretdoes" — which is an objectql change, another lane.Also worth recording: #7722's shape does not extend here. It worked because a signature is a one-way derivative that can replace the secret. Headers must be sent verbatim. A store-a-reference-and-resolve-at-send variant is impossible generically — for
source: 'flow'(http-nodes.ts) the headers are interpolated per run from run-scoped variables,refIdis a node id, and there is no config row to read back.①-f —
sys_email.headers_jsonwas examined and deferred, not overlooked. The dispatch cut it for an in-flight conflict (#7801 / PR #8064 holdsplugin-email/src/email-service.ts, which is ①-f's read-back point), not a value judgement. It is the same shape as ①-b, so it should follow whatever ①-b's decision turns out to be.Generated by Claude Code