Uh oh!
There was an error while loading. Please reload this page.
fix(webhooks): stop storing the subscriber signing secret in cleartext (#7799) - #7901
Merged
Merged
Conversation
#7799) `bootstrapDeclaredWebhooks` serialized the whole validated `Webhook` envelope — `secret` included — into `sys_webhook.definition_json`, and `AutoEnqueuer.parseRow` read `defn.secret` back out to sign deliveries. `definition_json` is an ordinary textarea on an admin-authorable object with no restrictive `enable.apiMethods`, so `GET /api/v1/data/sys_webhook` returned the receiver's only proof of origin to every persona that can read the object. This is the half #7722 scoped out — and unlike the delivery table, nothing ages it out. The authored key now lands in a new `sys_webhook.signing_secret` column of type `secret`: the engine encrypts it into `sys_secret`, keeps an opaque `secret:<id>` ref on the row, and masks it on every read. `definition_json` carries the same envelope minus `secret`. The enqueuer recovers the plaintext server-side on cache refresh, so the delivered `X-Objectstack-Signature` is byte-identical and no receiver changes. The authoring envelope (`webhook.zod.ts`) is untouched. A boot sweep migrates rows that already hold cleartext, including the `managed_by: 'admin'` and `customized: true` rows the seeder deliberately never rewrites. It stores the encrypted copy in the same update that strips the blob, so a failure cannot leave a webhook stripped and unsigned. With no CryptoProvider the engine refuses the write rather than storing cleartext: the webhook is skipped and the legacy row left intact, reported with an ADR-0112 code/status pair. Adds `ObjectQL.resolveSecretField(object, recordId, field)` — the privileged dereference of one row's `secret`-typed field. `resolveSecret()` was documented for "privileged consumers … against the stored ref", but the read mask meant no consumer could obtain that ref; this is what makes the encrypted channel reachable by a server-side consumer at all. It refuses any field not declared `type: 'secret'` so it cannot become a mask bypass over a `password` field (ADR-0100). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CYfQTgc1hRzTXnk8n3Q2
…cret (#7799) `os i18n extract` (merge mode) adds the new `sys_webhook.signing_secret` label and help to the four locale bundles; the zh-CN / ja-JP / es-ES leaf strings are then translated in place rather than left as the English source text. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CYfQTgc1hRzTXnk8n3Q2
…et column (#7799) The `secret` prop is still LIVE, but its evidence pointed at the cleartext path that #7799 removes (`definition_json` → `defn.secret`). Restate it against the `signing_secret` column and `engine.resolveSecretField()`, and correct the two `_note` sentences that described the whole envelope going into `definition_json`. Evidence prose only — no schema, no status change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1CYfQTgc1hRzTXnk8n3Q2
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 109 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:
|
This was referenced Aug 12, 2026
Closed
huangyiirene
marked this pull request as ready for review
August 12, 2026 03:20
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#7799
What was wrong
bootstrap-declared-webhooks.ts:203serialized the whole validatedWebhookenvelope —secretincluded — intodefinition_json, andauto-enqueuer.ts:parseRowreaddefn.secretstraight back out to sign deliveries.definition_jsonis an ordinaryField.textareaon an admin-authorable object with no restrictiveenable.apiMethods, so a plainGET /api/v1/data/sys_webhookreturned the subscriber's HMAC key to every persona that can read the object. That key is the receiver's only proof a delivery came from us.This is the half #7722 scoped out: that issue removed the same secret's per-attempt copies from
sys_http_delivery; this is the remaining cleartext location, and unlike the delivery table nothing ages it out.The shape
The authored key now lands in a new
sys_webhook.signing_secretcolumn oftype: 'secret'— the engine's existing encrypted credential channel:AutoEnqueuerrecovers the plaintext server-side on each cache refresh and hands it to the outbox exactly as before, so the deliveredX-Objectstack-Signatureis byte-identical and no receiver changes.Why this could not be done plugin-locally
Two constraints, both measured rather than assumed:
ICryptoProvideris injected into the engine by the host (serve.ts→engine.setCryptoProvider) and is not a kernel service, soctx.getService()cannot reach it. The datasource-binder shape — a plain text column holdingsys_secret:<id>, written by a binder the consumer owns — is therefore unavailable here. Writing cleartext into asecret-typed column and letting the engine's own write path wrap it is the only door, and it inherits the engine's fail-closed posture for free.maskSecretFieldsreplaces it with the mask on everyfind/findOne, unconditionally and after hooks. SoObjectQL.resolveSecret(), documented for "privileged consumers … against the stored ref", had no supported way for a consumer to obtain that ref. This PR addsObjectQL.resolveSecretField(object, recordId, field)— a driver-level row read plus the existing decrypt — which is what makes the encrypted channel usable by a server-side consumer at all. It refuses any field not declaredtype: 'secret', so it cannot become a generic mask bypass over apasswordfield (plaintext at rest by design, ADR-0100).The authoring envelope is untouched
packages/spec/src/automation/webhook.zod.tsis not modified —secretstays exactly where authors write it, anddefineWebhook({ secret })is unchanged. Nopackages/spec/src/**file is touched, so no schema/docs regeneration is in play. The only file underpackages/spec/is theliveness/webhook.jsonledger, whosesecretevidence prose pointed at the cleartext path this PR removes (status stayslive).Migration / compat — what happens to existing rows
managed_by: 'package', not customized)managed_by: 'admin'migrateLegacyWebhookSecretsat boot.customized: true(frozen package row)ICryptoProviderwiredcode/statuspair so it is visible, not silent.The sweep runs after the seeder (so a just-reseeded row is a no-op), writes with system context (so it does not stamp
customized: trueand freeze a package row), and stores the encrypted copy in the same update that strips the blob — a failure can never leave a webhook stripped and unsigned. It is idempotent and free on every boot after the first.Until a given row is swept,
AutoEnqueuerstill reads the legacy blob and keeps signing, warning each refresh that the value is exposed. A row that has an encrypted key which cannot be decrypted is dropped rather than delivered unsigned — an undelivered webhook is visible and gets investigated; an unsigned one is invisible and teaches the receiver to accept unauthenticated traffic.Re-seeding compares the declared key against the stored plaintext and writes only on an actual change, so a boot no longer mints an orphan
sys_secretrow per webhook per restart; rotating the secret in code still propagates.Tests
packages/plugins/plugin-webhooks/src/webhook-secret-at-rest.test.ts, driven against a realObjectQLengine with the realSysWebhookschema (only the driver is a double) — an engine fake would echo back whatever it was handed and pass every assertion here while the product stayed broken:definition_jsonspecifically. A per-field check walks straight past a key nested in a blob.createHmac('sha256', SECRET)over the raw body) still matches the deliveredX-Objectstack-Signature, and [security] Webhook HMAC signing secrets are persisted in cleartext on everysys_http_deliveryrow #7722's no-secret-on-the-delivery-row invariant still holds.sys_secretrow; rotation re-encrypts and signs with the new key; a secret-free webhook still delivers unsigned with no crypto cost.{ code: 'INTERNAL_ERROR', status: 500 }.webhook-secret.ts(mask, ref prefix) still equal objectql's own exports — the plugin takes no runtime dependency on objectql, and signing with the mask would silently break every delivery.packages/objectql/src/secret-fields.test.tsgains coverage forresolveSecretField: it recovers plaintext when the caller can only see the mask, returnsnullfor an unset secret and a missing row, and refuses both a plain column and apasswordfield.Both stub drivers now return copies of stored rows. Handing out the live object made the doubles lie in the one direction that matters here:
maskSecretFieldsmutates the rows it is given, so a singlefindstamped the mask over the stored ref and the next resolve read it back as "no secret" — an artefact that reads exactly like an engine bug.The doubles added here are drivers (
update(object, id, data)— primary key second), whichcheck:engine-double-contractscopes out; no hand-rolled engine double is added.Gates run locally
plugin-webhooks45/45 ·objectql3315/3315 ·tsc --noEmiton both · eslint on every changed file ·check:engine-double-contract·check:error-code-casing·check:empty-changeset·check:platform-checklist·check:i18n(bundles regenerated; the three non-Englishsigning_secretstrings are translated, not left as English source text).Deliberately not done
INTERNAL_ERROR/500 pair rather than growing the ADR-0112 extension ledger.check:i18n-coveragecould not be measured locally: it fails to lintexamples/app-showcaseuntil the whole workspace is built (@objectstack/connector-mcphas no dist in this worktree), and it refuses to judge a partial round. The new keys are translated in all four locales, so it should not move.🤖 Generated with Claude Code
https://claude.ai/code/session_01S1CYfQTgc1hRzTXnk8n3Q2
Generated by Claude Code