Skip to content

Commit 9a0c0b5

Browse files
fix(plugin-webhooks): refuse the retired definition_json credential location at delivery and at the write door (#19944)
Closes #10164 Clause-②: no (narrowing) Step 3 of the retirement ruled on #9930, now in scope under the 2026-09-23 maintainer ruling recorded on #10164: 「10164 不考虑旧的」 and 「10164 现在就做,不等v18」. The legacy cleartext credential location, the `secret` / `headers` keys inside `sys_webhook.definition_json`, goes from warn-and-accept to a loud, dated refusal. The refusal applies at both doors in one change. ## What changed **Delivery door** (`AutoEnqueuer.attachSecret` / `attachHeaders`, `auto-enqueuer.ts`). A row whose signing secret or header map exists ONLY as the legacy key, with nothing in `signing_secret` / `headers_secret`, is no longer delivered from the cleartext. The new `refuseLegacyCleartext` parks the subscription in the same fail-closed shape an unrecoverable encrypted credential already uses: - nothing is sent; - every matching event becomes a `dead` `sys_http_delivery` row with 0 attempts, no signature, no headers, and an `error` reading `[VALIDATION_ERROR/400] ...`; - the drop is reported ONCE at `error`, on the existing say-once ledger. Its meta carries `code`, `status`, `field: 'definition_json'` and the refused `keys`. `readLegacySecret` / `readLegacyHeaders` stay, because the boot sweep is their other caller and the sweep is the remedy. **Write door** (new `webhook-legacy-cleartext.ts`, `bindWebhookLegacyCleartextGate`). A `beforeInsert` / `beforeUpdate` hook on `sys_webhook` refuses any `definition_json` that carries a `secret` or `headers` key, whatever its value. It follows the #8566 shape: the same hook seam as the `headers_secret` shape gate, bound in `bootDeclaredWebhooks` before the seeder and the sweep, unbound in `destroy()`. It is not exempt for `isSystem`. The error, `WebhookLegacyCleartextError`, carries `code: 'VALIDATION_ERROR'`, `status: 400`, `object`, `field` and `keys`. It judges key presence: `"headers": {}` still teaches the wrong location. It never echoes a value. A clean blob, an omitted `definition_json` and a non-JSON blob all pass. The last is not this gate's verdict. **Refusal text**: it is dated `2026-09-23` (the ruling's date; the message says why), names the sweep `migrateLegacyWebhookSecrets`, and gives the remedy. Both remedies need a registered CryptoProvider. With one registered, either restart so the sweep converts the row, or write the values into `signing_secret` / `headers_secret` and remove the keys. The text carries no tracker number. Per 「不考虑旧的」, there is no transition path for a deployment without a `CryptoProvider`. **Changeset**: `@objectstack/plugin-webhooks: minor` with a **BREAKING** banner (launch-window convention; `check-changeset-no-major` refuses `major`). It states plainly that webhooks still on the legacy shape STOP DELIVERING, and gives the remedy. ADR-0087 disposition: `not-required (no-migration-prescription)`. Nothing authored moves: `packages/spec` is untouched, and `WebhookSchema` still declares `secret` / `headers`, which the materializer still routes to the encrypted columns. The refused shape is a stored data row, and its converter, the boot sweep, already ships. ⛔ Zero `packages/spec`. ⛔ No `content/docs/releases/**`. ## Tests (all in `packages/plugins/plugin-webhooks`) - `webhook-secret-at-rest.test.ts`. The two "un-swept row keeps signing / keeps delivering" tests are rewritten as refusal witnesses on a real `ObjectQL` engine. Each asserts no call reaches the receiver, exactly one `error` whose meta matches `{ code: 'VALIDATION_ERROR', status: 400, field: 'definition_json', keys: [...] }`, and a parked dead row whose `error` starts `[VALIDATION_ERROR/400]`. Each also checks that the prose carries the date, the sweep name, the remedy, no credential and no tracker number. A control leg shows the same row, once swept, delivers its headers. - Write-door suite on a real engine: - update with `headers`, with `secret`, and with both is refused with the envelope (`code`, `status`, `object`, `field`, `keys`), and nothing lands; - an insert is refused whole; - an emptied key is refused; - a clean blob, an omitted blob and a non-JSON blob pass; - an unbound counterfactual lets the write land; - the boot sweep still converts a pre-existing legacy row with the gate bound; - the declared-webhook seeder still materializes secret + headers through the gate. - `webhook-signing-secret.test.ts`. Fixture re-spelled onto the encrypted columns, with a `resolveSecretField` stub. It pinned the signature path by riding the legacy blob, which is now refused. ## Reverse verification (from committed state, `scripts/ablation-replace.mjs` WRAP mode, restore proven by blob hash == HEAD and an empty `git diff HEAD`) | leg | mutation | witness file result | |---|---|---| | A | `attachSecret` honours the legacy secret again | 1 failed / 45 passed: "an un-swept row is REFUSED, not signed from the blob" | | B | `attachHeaders` honours the legacy headers again | 1 failed / 45 passed: "an un-swept row is REFUSED, not delivered with the blob's headers" | | C | the write-door verdict passes everything | 5 failed / 41 passed: all five write-door refusal cases | All three were restored: `auto-enqueuer.ts` blob `1cfd125c6c79` == HEAD, `webhook-legacy-cleartext.ts` blob `1c50d2b4321b` == HEAD. ## Local verification (HEAD `36d17abc8e`) - `pnpm --filter @objectstack/plugin-webhooks exec vitest run`: 13 files, 160 tests passed. - `pnpm --filter @objectstack/plugin-webhooks run typecheck`: exit 0, measured at `7d448ba52a`. The only later commit touches `scripts/doc-authoring-prose-id.baseline.json`. - eslint over the 7 changed `.ts` files: 7 files, 0 errors, 0 warnings. Population read from `eslint.config.mjs`; `--format json` counts. Invariance: type-aware linting is not enabled (no `parserOptions.project`), so this diff cannot move any untouched file's verdict. - `dispatch-gates --ran`: 101 derived families accounted for, 98 run green and 3 NOT MEASURED: - `check:skill-examples` needs built client SDK output; - `check:dual-build-cjs-loads` and `check:type-check-debt` need the full workspace build. All three exited 3 on their own prerequisite, and CI builds the full closure. `check:doc-authoring` first went red on a burn-down, meaning the removed warn strings cited two tracker ids. The baseline was regenerated with the gate's own shrink-only command (`#7799`/`#7986` 3 to 2 in `auto-enqueuer.ts`). `check:i18n` went green after its prerequisite closure was built. `check-adr-0087-registration`, `check-changeset-no-major` and `check-empty-changeset` are green. ## Acceptance notes - **Docs**: `content/docs/automation/webhooks.mdx` §3.1 said an un-swept row "is still delivered from the blob, with a `warn`". That sentence is corrected to the refusal, in the one page describing this exact behaviour. No other hand-written page describes legacy acceptance. - **Not exported from the package entry.** `bindWebhookLegacyCleartextGate` is wired by `WebhookOutboxPlugin` only. The `headers_secret` gate is exported for hosts that boot the pieces themselves; this one is deliberately not, to keep the public surface unchanged under `no (narrowing)`. A consumer branches on `code` / `status`. - **Residue left as-is (observation, not filed)**: a row holding BOTH an encrypted credential and a legacy key is not refused at delivery. The encrypted value wins and the legacy key is ignored, as before. Such a row can no longer be written through the engine. Any existing one is still readable over the data API until the boot sweep, which treats it as found and writes the blob's value into the encrypted column. Whether that overwrite can clobber a rotated key on a real deployment is unmeasured. Carrier: none. - Commit trailers are the model-free pair AGENTS.md prescribes. The harness's own attribution reminder asked for a model-named trailer, which the pre-push hook refuses; AGENTS.md wins. --- _Generated by [Claude Code](https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent aeaaa44 commit 9a0c0b5

10 files changed

Lines changed: 645 additions & 62 deletions
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
'@objectstack/plugin-webhooks': minor
3+
---
4+
5+
fix(plugin-webhooks): a webhook credential stored as cleartext inside `sys_webhook.definition_json` is refused, at the delivery path and at the write door (#10164)
6+
7+
Clause-②: no (narrowing)
8+
9+
**BREAKING** — shipped as `minor` under the launch-window convention
10+
(`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by
11+
this banner and the ADR-0087 disposition below, never by the level).
12+
13+
**Webhooks that still carry the legacy cleartext shape STOP DELIVERING.** A
14+
`sys_webhook` row whose signing secret or custom header map exists only as a
15+
`secret` / `headers` key inside `definition_json` — with nothing stored in the
16+
encrypted `signing_secret` / `headers_secret` column — used to be delivered from
17+
that cleartext with a `warn`. It is now refused, dated `2026-09-23`:
18+
19+
- **Delivery path.** The subscription is PARKED, the same fail-closed shape as an
20+
encrypted credential that cannot be recovered: nothing is sent, and every
21+
matching record change is recorded in `sys_http_delivery` as a `dead` row with
22+
0 attempts, no signature and no headers. Its `error` names the refusal as
23+
`[VALIDATION_ERROR/400]`, and the drop is reported once at `error`, with
24+
`code: 'VALIDATION_ERROR'`, `status: 400`, `field: 'definition_json'` and the
25+
refused `keys` in the log meta.
26+
- **Write door** (when `WebhookOutboxPlugin` is mounted, the standard mount). A
27+
`sys_webhook` insert or update whose `definition_json` carries a `secret` or
28+
`headers` key, whatever its value, is refused before anything is stored
29+
(`VALIDATION_ERROR` / `400`, with `object`, `field` and `keys` on the error).
30+
That covers a raw `PATCH /api/v1/data/sys_webhook`. It also covers a Setup-form
31+
save that echoes back a legacy blob unchanged. Omitting `definition_json`, or
32+
writing one without those keys, is unaffected. The plugin binds this refusal
33+
itself, and it is not exported from the package entry: a host that composes
34+
`AutoEnqueuer` on its own still gets the delivery-path refusal above, but not
35+
this write-door refusal.
36+
37+
**Fix.** Both remedies need a registered `CryptoProvider`
38+
(`engine.setCryptoProvider` — `LocalCryptoProvider` in dev, KMS/Vault in
39+
production), because writing a `secret`-typed column is itself refused without
40+
one. With a provider registered, either:
41+
42+
- restart, and the boot sweep `migrateLegacyWebhookSecrets` moves both values into
43+
their encrypted columns and strips them from `definition_json` in one update. The
44+
subscription re-arms at the next refresh.
45+
- or re-author the webhook yourself. Write the key into `signing_secret` and the
46+
header map into `headers_secret` (a JSON object of string values), then remove
47+
both keys from `definition_json`.
48+
49+
There is no transition path for a deployment that runs with no `CryptoProvider`.
50+
51+
Unchanged: authoring. `defineWebhook({ secret, headers })` is written exactly as
52+
before, and the boot materializer still routes each value to its encrypted column.
53+
A row whose encrypted columns are set delivers exactly as before.
54+
55+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authored moves: `packages/spec` is untouched, `WebhookSchema` still declares `secret` and `headers`, and the materializer still routes them to their encrypted columns, so `objectstack migrate meta` has nothing to rewrite and the ledger has no row to gain. What is refused is a stored DATA-row shape on `sys_webhook`, and its converter already ships and runs at every boot: `migrateLegacyWebhookSecrets`, named in the refusal and in the Fix paragraph above. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers a data-row sweep (not `registered` / `already-registered`); and the change is runtime behaviour, not a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->

‎content/docs/automation/webhooks.mdx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,12 @@ own column.
153153

154154
A row the sweep has not converted — for example a pre-v17 row on a runtime with
155155
no `CryptoProvider` wired, where the encrypted write is refused and the row is
156-
deliberately left intact — is still delivered from the blob, with a `warn`
157-
naming those headers as cleartext. That fallback exists for rows written before
158-
the move; it is **not** a supported place to author. Never type a token into
156+
deliberately left intact — is **not** delivered: the subscription is parked and
157+
each event is recorded in `sys_http_delivery` as a dead row carrying a dated
158+
refusal, and a write that puts `secret` or `headers` into `definition_json` is
159+
refused (`VALIDATION_ERROR` / `400`). Both remedies need a registered
160+
`CryptoProvider`. With one registered, either restart so the sweep converts the
161+
row, or move the values into their encrypted columns yourself. Never type a token into
159162
`definition_json`: it is returned in full by the generic data API, and nothing
160163
ages it out.
161164
</Callout>

‎packages/plugins/plugin-webhooks/src/auto-enqueuer.ts‎

Lines changed: 69 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ import {
1616
readLegacyHeaders,
1717
resolveWebhookHeaders,
1818
} from './webhook-headers.js';
19+
import {
20+
WebhookLegacyCleartextError,
21+
type LegacyDefinitionCredentialKey,
22+
} from './webhook-legacy-cleartext.js';
1923

2024
/**
2125
* The authored trigger vocabulary, taken from the spec rather than restated
@@ -555,9 +559,10 @@ export class AutoEnqueuer {
555559
* returns a mask, so presence is decidable here but the value is not;
556560
* `resolveWebhookSecret` dereferences it server-side.
557561
* 2. `definition_json.secret` — a row not yet swept by
558-
* `migrateLegacyWebhookSecrets` (or hand-edited back in). Still honoured
559-
* so an un-migrated deployment keeps signing, and warned about once per
560-
* refresh so the exposure is visible rather than silently permanent.
562+
* `migrateLegacyWebhookSecrets` (or hand-edited back in). No longer
563+
* honoured: the subscription is PARKED with a dated refusal
564+
* ({@link refuseLegacyCleartext}). It used to sign from the cleartext
565+
* with a warning, which kept the exposure alive indefinitely.
561566
* 3. Neither — an unsigned webhook, which is a legitimate authored choice
562567
* (`secret` is optional on the envelope).
563568
*
@@ -589,16 +594,11 @@ export class AutoEnqueuer {
589594
return false;
590595
}
591596

592-
const legacy = readLegacySecret(row?.definition_json);
593-
if (legacy) {
594-
this.logger?.warn?.(
595-
`[webhook-auto-enqueuer] webhook '${sub.name}' still carries its signing secret as ` +
596-
`CLEARTEXT in definition_json, readable over the data API (#7799). Signing continues ` +
597-
`from it; run the boot sweep (migrateLegacyWebhookSecrets) with a CryptoProvider wired ` +
598-
`to move it into sys_secret.`,
599-
{ id: sub.id },
600-
);
601-
sub.secret = legacy;
597+
// A key that exists ONLY as legacy cleartext in the blob is refused,
598+
// not honoured — see `webhook-legacy-cleartext.ts`.
599+
if (readLegacySecret(row?.definition_json)) {
600+
this.refuseLegacyCleartext(sub, 'secret');
601+
return false;
602602
}
603603
return true;
604604
}
@@ -644,21 +644,66 @@ export class AutoEnqueuer {
644644
return false;
645645
}
646646

647-
const legacy = readLegacyHeaders(row?.definition_json);
648-
if (legacy) {
649-
this.logger?.warn?.(
650-
`[webhook-auto-enqueuer] webhook '${sub.name}' still carries its custom headers as ` +
651-
`CLEARTEXT in definition_json, readable over the data API (#7986) — that map is the ` +
652-
`ordinary place an Authorization header goes. Delivery continues from it; run the boot ` +
653-
`sweep (migrateLegacyWebhookSecrets) with a CryptoProvider wired to move them into ` +
654-
`sys_secret.`,
655-
{ id: sub.id },
656-
);
657-
sub.headers = legacy;
647+
// Same refusal as the signing secret's, in the same stroke: refusing
648+
// one passenger while honouring the other would leave half the retired
649+
// location alive.
650+
if (readLegacyHeaders(row?.definition_json)) {
651+
this.refuseLegacyCleartext(sub, 'headers');
652+
return false;
658653
}
659654
return true;
660655
}
661656

657+
/**
658+
* Refuse a credential whose ONLY copy is the retired cleartext key in
659+
* `definition_json` — PARK the subscription, the same fail-closed shape as
660+
* an unrecoverable encrypted credential: nothing is sent, and every matching
661+
* event is recorded as a dead `sys_http_delivery` row carrying the refusal.
662+
*
663+
* This replaced warn-and-accept. The warning said the right thing and
664+
* changed nothing: the delivery still went out with a credential that
665+
* `GET /api/v1/data/sys_webhook` hands to every persona that can read the
666+
* object, and a warning nobody acts on is declared-but-not-enforced.
667+
*
668+
* Why `error`, said once: after the refusal `sys_webhook` still reads
669+
* `active: true` while every matching record change goes undelivered — the
670+
* AGENTS.md "Degradation log levels" question answers yes. The say-once
671+
* ledger is the one {@link reportDrop} uses, on purpose: a subscription is
672+
* either armed or parked, whichever credential parked it.
673+
*/
674+
private refuseLegacyCleartext(sub: CachedSubscription, key: LegacyDefinitionCredentialKey): void {
675+
const refusal = new WebhookLegacyCleartextError('delivery', [key], {
676+
object: this.subscriptionsObject,
677+
webhook: sub.name,
678+
});
679+
const meta = {
680+
id: sub.id,
681+
webhook: sub.name,
682+
field: refusal.field,
683+
keys: refusal.keys,
684+
code: refusal.code,
685+
status: refusal.status,
686+
};
687+
if (this.droppedForSecret.has(sub.id)) {
688+
this.logger?.debug?.(
689+
`[webhook-auto-enqueuer] webhook '${sub.name}' is still parked: its ${key} exists only as `
690+
+ 'retired cleartext in definition_json',
691+
meta,
692+
);
693+
} else {
694+
this.droppedForSecret.add(sub.id);
695+
const message = `[webhook-auto-enqueuer] ${refusal.message}`;
696+
if (typeof this.logger?.error === 'function') {
697+
this.logger?.error(message, refusal, meta);
698+
} else {
699+
this.logger?.warn?.(message, meta);
700+
}
701+
}
702+
sub.secret = undefined;
703+
sub.headers = undefined;
704+
sub.parkedReason = `[${refusal.code}/${refusal.status}] ${refusal.message}`;
705+
}
706+
662707
/**
663708
* [#8022] Report a subscription dropped for an unresolvable signing key.
664709
*

‎packages/plugins/plugin-webhooks/src/sys-webhook-api-exposure.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,8 @@ describe('#9756 — sys_webhook declares its data-API exposure explicitly', () =
103103

104104
it('leaves the reachable-cleartext fields reachable — the card is not closed by this', () => {
105105
// `url` (#8025, won't-fix on masking) and a legacy row's un-migrated
106-
// `definition_json.headers` (#7986, still read by `readLegacyHeaders`) are
106+
// `definition_json.headers` (#7986; no longer delivered from, but still held
107+
// by a row the boot sweep could not convert) are
107108
// served by `get`/`list`, which the console requires. Stated as an
108109
// assertion so nobody reads the new `enable` block as having removed them.
109110
expect(apiExposureDenialReason(ENABLE, 'get')).toBeNull();

‎packages/plugins/plugin-webhooks/src/sys-webhook.object.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,10 @@ export const SysWebhook = ObjectSchema.create({
354354
* on this object was reduced. It was not, and `apiMethods` is the wrong
355355
* instrument for it: `url` (#8025 — won't-fix on masking, because the URL is
356356
* the routing key an operator must be able to see, search, sort and edit) and
357-
* a legacy row's un-migrated `definition_json.headers` (#7986 —
358-
* `readLegacyHeaders` in `auto-enqueuer.ts` still reads them and warns) are
357+
* a legacy row's un-migrated `definition_json.headers` (#7986 — no longer
358+
* delivered from, and refused at the write door, since
359+
* `webhook-legacy-cleartext.ts`; a row the boot sweep could not convert
360+
* still holds them until it does) are
359361
* both served by `get`/`list`, which is exactly what the console requires.
360362
* Any set that removes them removes the admin surface with them. A survey
361363
* that greps this file for `enable:` and stops is measuring the wrong thing;

0 commit comments

Comments
 (0)