Repository navigation
Commit bb0db13
fix(plugin-webhooks): the redeliver veto refuses a delivery whose subscription is inactive (#22836)
Fixes #22804
Clause-②: no
## What changes
`createWebhookRedeliverGuard`
(`packages/plugins/plugin-webhooks/src/redeliver-guard.ts`), the
redeliver veto, now refuses a delivery whose `sys_webhook` subscription
is inactive. This follows triage's reading on the card: an explicit
replay is "the dispatcher" for `active`. The refusal works like the gone
case. The door answers `409 DELIVERY_NOT_ELIGIBLE` and leaves the row
untouched. The reason names the webhook, says it is inactive, and names
the remedy:
> webhook 'NAME' is inactive, and an inactive webhook is skipped by the
dispatcher, so this delivery is refused rather than sent to an endpoint
that was switched off. Fix: re-activate the webhook, then redeliver; new
events are delivered again from the moment it is active.
On the wire, the outbox puts its own prefix before the reason: `Delivery
row 'ID' cannot be redelivered: REASON`.
- **Order.** The new check runs after "gone" and before the secret
checks. So an inactive webhook whose key is also unrecoverable is
refused as inactive, the operator's first remedy. The gone and
unrecoverable-secret refusals keep their text and their order. Ruling
`5271033283` on #8069 says no operator action may send an unsigned
delivery. This change only adds a refusal and weakens nothing.
- **The `active` field description stays as it is.** "Inactive webhooks
are skipped by the dispatcher" is now true of both paths.
- The guard's docblock lists the refusal cases as 1 gone, 2 inactive, 3
unrecoverable secret, 4 lookup failed (refused by the caller). The
`SYSTEM_CTX` docblock now says what the read is for: existence, the
`active` switch, name and secret posture.
## Measured (the dispatch's mechanism assumptions)
1. **The column.** Its name is `active`: `Field.boolean`, `required:
true`, `defaultValue: true` (`sys-webhook.object.ts:175`–`181`). The
guard reads with `engine.findOne(subscriptionsObject, { where: { id } },
{ context: { isSystem: true } })`, with no projection, so the column is
in the row. Booleans come back as JS booleans on every driver: the SQL
driver converts SQLite/MySQL `0`/`1` to booleans in `formatOutput` and
leaves null as null, and Postgres is native.
**Null or absent `active`: refused. I decided this from the dispatcher's
predicate, not from the declared default.** The dispatch suggested the
declared default, so this is a deviation, stated here. The automatic
dispatcher loads `{ where: { active: true } }` (`auto-enqueuer.ts:393`)
and so skips a null or absent row on every driver. If the guard allowed
that row, the field's sentence would be false again for that row. The
declared default and `required` are why the engine never writes such a
row (it fills `true` on insert), so the two readings differ only on rows
written outside the engine. There, refusing is the fail-closed side and
matches what Setup shows. The test is `subscription.active !== true`.
2. **The door.** Measured on both faces: the `webhooks` slot's
`handleRedeliver` and the self-hosted mount. The guard's string reaches
the door as `409` `DELIVERY_NOT_ELIGIBLE` with the string as the
message, through `assertRedeliverAllowed`
(`service-messaging/src/http-outbox.ts`). The member and the mount
answer byte-equal (status, headers, body), as the parity pin asserts.
3. **The spec docblock.** The `IWebhookService.handleRedeliver` text,
"`DELIVERY_NOT_ELIGIBLE` (the row is not finished, or the producer's
veto refused it)", covers the third case: verified, no change. One
sentence in the same file's header now under-describes the veto: "the
messaging service's veto over replaying a webhook row whose subscription
or signing secret is gone". It still holds for the two cases it names,
but it lists two of the three. `packages/spec` is untouched, as the
claim directs; the report names the sentence for the seat to route.
4. **The order and existing pins.** No existing pin depends on the
order. One existing fixture changed. The
`webhook-system-context.pin.test.ts` row had no `active` key, and the
new rule refuses that. The fixture gains `active: true`, as every
engine-written row carries it. The pin still asserts what it is for, the
read's `isSystem` context.
## Pins
- Guard (`webhook-drop-durable-record.test.ts`, new describe):
- inactive is refused, naming the webhook, `inactive` and the remedy;
- null and absent `active` are refused;
- inactive is checked before the secret: the resolver is never called;
- control: active, signed and unsigned, is allowed;
- control: gone keeps its own refusal.
- Door (`webhook-redeliver-member.test.ts`):
- inactive answers `409` `DELIVERY_NOT_ELIGIBLE` on both faces, with the
reason, and the row stays `dead`;
- CONTROL: active and signed is replayed by both faces (`200`,
`pending`);
- CONTROL: gone is unchanged;
- CONTROL: the same delivery is replayed once re-activated;
- the inactive case is also a row in the existing every-refusal table
and in the member/mount byte-parity table.
- Refusal pins assert the envelope's `code` and the HTTP `status`. On
the message they assert the named subject (`webhook 'paused_hook'`), the
state word `inactive`, and the remedy fragment `re-activate the webhook,
then redeliver`. The card names those as the reason's contract, so the
whole sentence is not pinned.
## Ablation (every negative pin)
The ablation was done on the committed fix (`4c3ef9cef`), through
`scripts/ablation-replace.mjs`, wrapped with a `trap` restore. The
anchor `if (subscription.active !== true) {` became `if (false) {`.
- On disk: anchor 1 to 0, replacement 0 to 1, blob `4a8096099b21` to
`684c106c56e3`.
- Result: 2 of 3 files and **7 of 35 tests red**. These are the five
pins that assert a refusal (the three guard refusal pins, the door's
inactive pin and the re-activated control's first leg) plus the two
tables that now carry the inactive row.
- The controls stayed green: active and signed replayed, gone unchanged,
and the guard's active and gone controls.
- Restore was proven: blob equal to the HEAD blob, `git diff HEAD`
empty, and the tree clean.
- The tests import the subject by relative source path
(`./redeliver-guard.js`, `./webhook-outbox-plugin.js`), not through a
package `exports`. So no `dist/` is in the loop, and the dist preflight
does not apply.
## Verification
All runs below are at `4c3ef9cef`. `os-verify-lock` timings are
shared-box figures.
- `pnpm --filter '@objectstack/plugin-webhooks^...' build` (the
dependency closure): VERDICT command-exit 0.
- `pnpm --filter @objectstack/plugin-webhooks build`:
`check-dts-emitted` 4/4.
- `pnpm --filter @objectstack/plugin-webhooks typecheck`: `tsc --noEmit`
and the scripts program clean, and `check:test-typecheck: OK`, 0 errors.
- `pnpm --filter @objectstack/plugin-webhooks test`: **17 files, 191
tests passed**.
- Gates: I re-derived the gate list on this change: `node
scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` gives 65 commands, the dispatch's 54 plus 11
that the changeset brings. **All 65 exit 0**, and `--ran` reconciles
them: "65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN", a derived zero,
because every line records its exit code.
- Three first runs answered exit 3, PREREQUISITE NOT MET, and none of
them counted as a measurement. `check-plugin-teardown-shape.mjs
--self-test` needed its pinned fixture commit in this shallow clone,
fetched at depth 1. `check:i18n` and `check:dual-build-cjs-loads` needed
built output, so I built the whole workspace (`turbo run build
--filter='!@objectstack/docs'`, 73/73 tasks). After those steps, all
three re-ran at exit 0.
- Every gate that reads `dist/` was re-run on the fully built tree:
`check:dts-closure` (73 packages, 172/172), `check:published-files`,
`check:lean-entry-closure` and `check:sourcemap-no-sources-content`.
- ESLint, narrowed to the 4 changed `.ts` files: 0 errors and 0 warnings
over 4 files, a count read from `--format json`. The config ignores none
of the four (`ESLint.isPathIgnored` is false for each), and it enables
no type-aware linting (no `parserOptions.project`), so this diff cannot
move any untouched file's verdict. The repo-wide `pnpm lint` is CI's.
- The branch was not merged with `origin/main`. The 3 commits since base
(`efcbac73c`, trigger-api, mcp and the objectui pin) touch none of these
files, and `git merge-tree` against `efcbac73c` is clean. CI's merge ref
and the queue test the joint tree.
The public surface is unchanged in bytes: no export, type, code or
status moves. So the import sites owe no tests of their own.
## Acceptance notes
- `IWebhookService` header sentence
(`packages/spec/src/contracts/webhook-service.ts`): see measured item 3.
Left for the seat, as the claim directs.
- `WebhookOutboxPlugin.installRedeliverGuard`'s docblock still describes
the veto by its signing half ("refuses a webhook row whose signing
configuration is no longer available"). It is still true, just narrower
than the veto now is. That file is outside the claim's file surface, so
it is left alone.
- `content/docs/automation/webhooks.mdx` lists `409
DELIVERY_NOT_ELIGIBLE` without naming the veto's cases, and its `active`
row now reads true of both paths. No sentence there is false.
Disclosure: the card is labelled `security`. This body describes
behaviour only, with no reproduction recipe.
---
_Generated by [Claude
Code](https://claude.ai/code/session_013LbZ9MhPp1iriZEtgJqioA)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 1eff322 commit bb0db13
5 files changed
Lines changed: 207 additions & 14 deletions
File tree
- .changeset
- packages/plugins/plugin-webhooks/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
25 | 37 | | |
26 | 38 | | |
27 | 39 | | |
28 | 40 | | |
29 | | - | |
| 41 | + | |
30 | 42 | | |
31 | 43 | | |
32 | 44 | | |
33 | | - | |
34 | | - | |
35 | | - | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
36 | 48 | | |
37 | 49 | | |
38 | | - | |
| 50 | + | |
39 | 51 | | |
40 | 52 | | |
41 | 53 | | |
| |||
64 | 76 | | |
65 | 77 | | |
66 | 78 | | |
67 | | - | |
68 | | - | |
| 79 | + | |
| 80 | + | |
69 | 81 | | |
70 | 82 | | |
71 | 83 | | |
| |||
107 | 119 | | |
108 | 120 | | |
109 | 121 | | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
110 | 136 | | |
111 | 137 | | |
112 | 138 | | |
| |||
120 | 146 | | |
121 | 147 | | |
122 | 148 | | |
123 | | - | |
124 | | - | |
| 149 | + | |
| 150 | + | |
125 | 151 | | |
126 | 152 | | |
127 | 153 | | |
| |||
Lines changed: 58 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
386 | 386 | | |
387 | 387 | | |
388 | 388 | | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
Lines changed: 95 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
26 | 30 | | |
27 | 31 | | |
28 | 32 | | |
| |||
39 | 43 | | |
40 | 44 | | |
41 | 45 | | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
42 | 50 | | |
43 | 51 | | |
44 | 52 | | |
| |||
54 | 62 | | |
55 | 63 | | |
56 | 64 | | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
57 | 69 | | |
58 | 70 | | |
59 | 71 | | |
| |||
88 | 100 | | |
89 | 101 | | |
90 | 102 | | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
91 | 107 | | |
92 | 108 | | |
93 | 109 | | |
| |||
243 | 259 | | |
244 | 260 | | |
245 | 261 | | |
| 262 | + | |
246 | 263 | | |
247 | 264 | | |
248 | 265 | | |
| |||
257 | 274 | | |
258 | 275 | | |
259 | 276 | | |
| 277 | + | |
260 | 278 | | |
261 | 279 | | |
262 | 280 | | |
| |||
269 | 287 | | |
270 | 288 | | |
271 | 289 | | |
| 290 | + | |
272 | 291 | | |
273 | 292 | | |
274 | 293 | | |
| |||
308 | 327 | | |
309 | 328 | | |
310 | 329 | | |
| 330 | + | |
311 | 331 | | |
312 | 332 | | |
313 | 333 | | |
| |||
323 | 343 | | |
324 | 344 | | |
325 | 345 | | |
| 346 | + | |
326 | 347 | | |
327 | 348 | | |
328 | 349 | | |
| |||
401 | 422 | | |
402 | 423 | | |
403 | 424 | | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
0 commit comments