fix(plugin-webhooks): the redeliver veto refuses a delivery whose subscription is inactive

.changeset/22804-redeliver-inactive-veto.md

@@ -0,0 +1,13 @@
+---
+'@objectstack/plugin-webhooks': patch
+---
+
+The webhook redeliver veto now refuses a delivery whose `sys_webhook` subscription is inactive. The `active` field declares that an inactive webhook is skipped by the dispatcher, and an operator's explicit replay is the dispatcher too, so the declaration now holds on both paths.
+
+Clause-②: no
+
+- FROM: the redeliver door (`POST /api/v1/webhooks/redeliver`, and the `webhooks` slot's `handleRedeliver`) replayed a finished delivery whose subscription had `active` switched off. The veto refused only a subscription that was gone, or whose signing secret could not be recovered.
+- TO: the door answers `409 DELIVERY_NOT_ELIGIBLE` for that delivery and leaves the row as it is. The message names the webhook, says it is inactive, and names the remedy. The test is the dispatcher's own predicate, `active === true`, so a row whose `active` is null or absent is refused exactly as the dispatcher skips it. Every row the engine writes carries a boolean there: the field is required, with default `true`.
+- Fix, for an operator who means to replay: re-activate the webhook, then redeliver.
+
+The gone-subscription and unrecoverable-secret refusals are unchanged, in text and in order. The inactive check runs after the gone check and before the secret check, so an inactive webhook whose key is also unrecoverable is refused as inactive, the first remedy to try. No code, status, field or export changes.

packages/plugins/plugin-webhooks/src/redeliver-guard.ts

@@ -21,21 +21,33 @@
  *     should still receive this payload, or under which key — and an operator
  *     deleting a webhook has expressed that it should stop. The maintainer
  *     named this case specifically.
- *  2. **A secret is stored but does not come back.** The subscription is signed
+ *  2. **The subscription is inactive.** [#22804] `active` is the operator's
+ *     off switch for the endpoint, and the field declares that an inactive
+ *     webhook is skipped by the dispatcher. An explicit replay is the
+ *     dispatcher too: sending a past delivery to a switched-off endpoint would
+ *     undo the switch without saying so. The test is the dispatcher's own
+ *     predicate — the subscription cache loads `{ active: true }`, so the
+ *     guard allows only `active === true`, and a row whose `active` is null or
+ *     absent (unreachable through the engine, which fills the declared default
+ *     `true` and requires the field) is refused, exactly as the dispatcher
+ *     skips it. Re-activating the webhook, then replaying, is the open path.
+ *     Checked after "gone" and before the secret, so an inactive webhook whose
+ *     key is also unrecoverable names the operator's first remedy.
+ *  3. **A secret is stored but does not come back.** The subscription is signed
  *     and the key cannot be recovered — a rotated KMS key, an unregistered
  *     CryptoProvider, a deleted `sys_secret` row. Deliveries for it are being
  *     dropped right now; replaying an old one is the same fail-open by another
  *     route.
- *  3. **The lookup itself failed.** Handled by the caller
+ *  4. **The lookup itself failed.** Handled by the caller
  *     (`assertRedeliverAllowed` turns a throwing guard into a refusal), because
  *     "we could not check" must never read as "allowed".
  *
- * It ALLOWS a subscription that is legitimately unsigned (`secret` is optional
- * on the authoring envelope) and any row from another producer (`source !==
- * 'webhook'`) — this guard speaks only for webhook rows.
+ * It ALLOWS an active subscription that is legitimately unsigned (`secret` is
+ * optional on the authoring envelope) and any row from another producer
+ * (`source !== 'webhook'`) — this guard speaks only for webhook rows.
  *
  * ## The narrow fail-open this closes deliberately
- * Case 2 is checked as *"a value is stored but nothing came back"*, not as
+ * Case 3 is checked as *"a value is stored but nothing came back"*, not as
  * *"the resolver threw"*. Presence is decidable from the masked read even
  * though the value is not, so the guard asks the question it can actually
  * answer.
@@ -64,8 +76,8 @@ import {
  * explicit system opt-in. The guard runs inside the messaging service's
  * redeliver path, after the delivery row has been read under the requesting
  * caller's organization, and reads the subscription that row belongs to only
- * for its existence, name and secret posture — the inputs of the refusal
- * reason it returns. What it reads and returns is unchanged by the opt-in; it
+ * for its existence, `active` switch, name and secret posture — the inputs of
+ * the refusal reason it returns. What it reads and returns is unchanged by the opt-in; it
  * may simply no longer rely on a missing principal to pass the security
  * middleware's principal-less hand-off, which ADR-0096 D5 closes.
  */
@@ -107,6 +119,20 @@ export function createWebhookRedeliverGuard(
             );
         }
 
+        // [#22804] Case 2: the off switch. `=== true` is the dispatcher's own
+        // predicate (`AutoEnqueuer` loads `{ active: true }`), so a replay is
+        // refused for exactly the rows automatic dispatch skips — `false`, and
+        // a null or absent value too. Before the secret checks on purpose:
+        // re-activating is the remedy an operator reaches for first.
+        if (subscription.active !== true) {
+            return (
+                `webhook '${String(subscription.name ?? row.refId)}' 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.'
+            );
+        }
+
         // Presence is decidable on the masked read — a set secret comes back as
         // the engine's mask, an unset one as null — even though the value is not.
         const storesSecret =
@@ -120,8 +146,8 @@ export function createWebhookRedeliverGuard(
         // the rule moved down one level instead of being written twice. This
         // guard's contract is unchanged in both directions, which is the point:
         // a stored-but-unresolvable key still returns the refusal REASON below
-        // (case 2), and any OTHER failure still propagates, because "we could
-        // not check" must never read as "allowed" (case 3, handled by
+        // (case 3), and any OTHER failure still propagates, because "we could
+        // not check" must never read as "allowed" (case 4, handled by
         // `assertRedeliverAllowed`).
         let plaintext: string | undefined;
         try {

packages/plugins/plugin-webhooks/src/webhook-drop-durable-record.test.ts

@@ -386,3 +386,61 @@ describe('webhook redeliver guard — signing configuration availability (#8069)
         await expect(guard({ source: 'flow', refId: 'node_1' })).resolves.toBeUndefined();
     });
 });
+
+/**
+ * [#22804] The off switch. `active: false` is how an operator stops an endpoint
+ * receiving events, and the field declares that an inactive webhook is skipped
+ * by the dispatcher; an explicit replay is the dispatcher too. The refusal
+ * reason is the contract an operator reads: it names the webhook, says it is
+ * inactive, and names the remedy (re-activate, then replay).
+ */
+describe('webhook redeliver guard — an inactive subscription is refused (#22804)', () => {
+    const row = { source: 'webhook', refId: 'wh-1' };
+
+    it('refuses an inactive subscription, naming the webhook, its state and the remedy', async () => {
+        const guard = createWebhookRedeliverGuard(makeEngine([subscriptionRow({ active: false })]));
+        const reason = await guard(row);
+        expect(reason).toContain("webhook 'orders_hook'");
+        expect(reason).toMatch(/\binactive\b/);
+        expect(reason).toMatch(/re-activate the webhook, then redeliver/);
+    });
+
+    it('refuses a null or absent `active` as the dispatcher skips it: only `active === true` replays', async () => {
+        // The subscription cache loads `{ active: true }`, so a row whose
+        // `active` is null or missing is never dispatched; the replay agrees.
+        const noActive: Record<string, unknown> = subscriptionRow();
+        delete noActive.active;
+        for (const subscription of [subscriptionRow({ active: null }), noActive]) {
+            const guard = createWebhookRedeliverGuard(makeEngine([subscription]));
+            await expect(guard(row), JSON.stringify(subscription)).resolves.toMatch(/\binactive\b/);
+        }
+    });
+
+    it('checks inactive before the secret: an unrecoverable key on an inactive webhook is never resolved', async () => {
+        let resolverCalls = 0;
+        const guard = createWebhookRedeliverGuard(
+            makeEngine([subscriptionRow({ active: false, signing_secret: SECRET_REF })], async () => {
+                resolverCalls += 1;
+                throw new Error('no CryptoProvider');
+            }),
+        );
+        await expect(guard(row)).resolves.toMatch(/\binactive\b/);
+        expect(resolverCalls).toBe(0);
+    });
+
+    it('control: the same subscriptions, active, are allowed — signed and unsigned', async () => {
+        const signed = createWebhookRedeliverGuard(
+            makeEngine([subscriptionRow({ active: true, signing_secret: SECRET_REF })], async () => 'whsec_live'),
+        );
+        await expect(signed(row)).resolves.toBeUndefined();
+        const unsigned = createWebhookRedeliverGuard(makeEngine([subscriptionRow({ active: true })]));
+        await expect(unsigned(row)).resolves.toBeUndefined();
+    });
+
+    it('control: a gone subscription keeps its own refusal, not the inactive one', async () => {
+        const guard = createWebhookRedeliverGuard(makeEngine([]));
+        const reason = await guard(row);
+        expect(reason).toContain('no longer exists');
+        expect(reason).not.toMatch(/\binactive\b/);
+    });
+});

packages/plugins/plugin-webhooks/src/webhook-redeliver-member.test.ts

@@ -22,7 +22,11 @@
  *     from `http.server` before the `http-server` alias, and never where none
  *     does; the slot is registered everywhere the veto is, and nowhere else;
  *  4. the veto no longer needs realtime or auto-enqueue: a redelivery of a
- *     delivery whose subscription is gone is refused by both faces.
+ *     delivery whose subscription is gone is refused by both faces;
+ *  5. [#22804] a delivery whose subscription is inactive is refused by both
+ *     faces, `409 DELIVERY_NOT_ELIGIBLE` with a reason naming the state and the
+ *     remedy, beside its controls: active and signed replays, gone keeps its
+ *     own refusal, and the same delivery replays once re-activated.
  */
 
 import { randomUUID } from 'node:crypto';
@@ -39,6 +43,10 @@ const PATH = '/api/v1/webhooks/redeliver';
 const ORIGIN = 'http://webhooks.test';
 const SUBSCRIPTION = 'wh_live';
 const GONE_SUBSCRIPTION = 'wh_gone';
+/** [#22804] A subscription an operator switched off: `active: false`. */
+const INACTIVE_SUBSCRIPTION = 'wh_off';
+/** [#22804] An active subscription that stores a signing secret which resolves. */
+const SIGNED_SUBSCRIPTION = 'wh_signed';
 const ORG_A = 'org_a';
 const ORG_B = 'org_b';
 const silentLogger = { info() {}, warn() {}, error() {}, debug() {} } as any;
@@ -54,6 +62,10 @@ function fakeEngine(subscriptions: Map<string, Record<string, unknown>>) {
         },
         registerHook() { /* provenance stamp + headers gate + cleartext gate */ },
         unregisterHooksByPackage() { return 0; },
+        /** [#22804] The privileged dereference: only the signed subscription's key resolves. */
+        async resolveSecretField(_object: string, id: string) {
+            return id === SIGNED_SUBSCRIPTION ? 'whsec_live' : null;
+        },
     };
 }
 
@@ -88,6 +100,10 @@ class FixturePlugin implements Plugin {
     version = '1.0.0';
     readonly subscriptions = new Map<string, Record<string, unknown>>([
         [SUBSCRIPTION, { id: SUBSCRIPTION, name: 'orders_hook', active: true }],
+        [INACTIVE_SUBSCRIPTION, { id: INACTIVE_SUBSCRIPTION, name: 'paused_hook', active: false }],
+        [SIGNED_SUBSCRIPTION, {
+            id: SIGNED_SUBSCRIPTION, name: 'signed_hook', active: true, signing_secret: 'secret:sec_22804',
+        }],
     ]);
     readonly outbox = new MemoryHttpOutbox();
     readonly messaging = new MessagingService({ logger: silentLogger });
@@ -243,6 +259,7 @@ describe('[#22756] handleRedeliver — the redeliver door from a Request, on a k
         const { member, attempted, pending, parked, statusOf } = await boot();
         const crossOrg = await attempted(ORG_B);
         const vetoed = await attempted(ORG_A, GONE_SUBSCRIPTION);
+        const inactive = await attempted(ORG_A, INACTIVE_SUBSCRIPTION);
         const neverSent = await parked(ORG_A);
         const notFinished = await pending(ORG_A);
 
@@ -257,6 +274,7 @@ describe('[#22756] handleRedeliver — the redeliver door from a Request, on a k
             ['a delivery not yet finished', redeliver({ deliveryId: notFinished }), 409, 'DELIVERY_NOT_ELIGIBLE'],
             ['a parked delivery, never sent', redeliver({ deliveryId: neverSent }), 409, 'DELIVERY_NEVER_SENT'],
             ['a delivery whose subscription is gone (the veto)', redeliver({ deliveryId: vetoed }), 409, 'DELIVERY_NOT_ELIGIBLE'],
+            ['a delivery whose subscription is inactive (the veto)', redeliver({ deliveryId: inactive }), 409, 'DELIVERY_NOT_ELIGIBLE'],
         ];
         for (const [label, request, status, code] of cases) {
             const answer = await snapshot(await member(request));
@@ -269,6 +287,7 @@ describe('[#22756] handleRedeliver — the redeliver door from a Request, on a k
         // Every refusal wrote nothing.
         expect(await statusOf(crossOrg)).toBe('dead');
         expect(await statusOf(vetoed)).toBe('dead');
+        expect(await statusOf(inactive)).toBe('dead');
         expect(await statusOf(neverSent)).toBe('dead');
         expect(await statusOf(notFinished)).toBe('pending');
     });
@@ -308,6 +327,7 @@ describe('[#22756] parity — the member answers what the self-hosted mount answ
         const app = fixture.canonicalApp;
         const crossOrg = await attempted(ORG_B);
         const vetoed = await attempted(ORG_A, GONE_SUBSCRIPTION);
+        const inactive = await attempted(ORG_A, INACTIVE_SUBSCRIPTION);
         const neverSent = await parked(ORG_A);
         const notFinished = await pending(ORG_A);
 
@@ -323,6 +343,7 @@ describe('[#22756] parity — the member answers what the self-hosted mount answ
             ['409 not finished', () => redeliver({ deliveryId: notFinished })],
             ['409 never sent', () => redeliver({ deliveryId: neverSent })],
             ['409 veto', () => redeliver({ deliveryId: vetoed })],
+            ['409 veto, inactive', () => redeliver({ deliveryId: inactive })],
         ];
         for (const [label, make] of requests) {
             const viaMember = await snapshot(await member(make()));
@@ -401,3 +422,76 @@ describe('[#22756] the veto is installed without realtime or auto-enqueue, and t
         expect(plugin.lines('error', 'exposes no registerRedeliverGuard')).toHaveLength(1);
     });
 });
+
+/**
+ * [#22804] The veto refuses a delivery whose subscription is inactive.
+ *
+ * `active: false` is the operator's off switch for an endpoint, and the field
+ * declares that an inactive webhook is skipped by the dispatcher; an explicit
+ * replay is the dispatcher too. The refusal is the gone case's: `409
+ * DELIVERY_NOT_ELIGIBLE`, the row untouched, and a reason an operator can act
+ * on — it names the webhook, says it is inactive, and names the remedy.
+ * Each control below is what a refuse-everything veto would fail.
+ */
+describe('[#22804] the veto refuses a delivery whose subscription is inactive, at the door', () => {
+    const faces = (fixture: FixturePlugin, member: (r: Request) => Promise<Response>) => [
+        ['member', member],
+        ['mount', (r: Request) => fixture.canonicalApp.fetch(r)],
+    ] as const;
+
+    it('inactive: 409 DELIVERY_NOT_ELIGIBLE naming the webhook, its state and the remedy, on both faces', async () => {
+        const { fixture, member, attempted, statusOf } = await boot({ server: 'both' });
+        const inactive = await attempted(ORG_A, INACTIVE_SUBSCRIPTION);
+
+        for (const [face, send] of faces(fixture, member)) {
+            const answer = await snapshot(await send(redeliver({ deliveryId: inactive })));
+            expect(answer.status, face).toBe(409);
+            const body = json(answer);
+            expect(body, face).toMatchObject({ success: false, error: { code: 'DELIVERY_NOT_ELIGIBLE' } });
+            expect(body.error.message, face).toContain("webhook 'paused_hook'");
+            expect(body.error.message, face).toMatch(/\binactive\b/);
+            expect(body.error.message, face).toMatch(/re-activate the webhook, then redeliver/);
+        }
+        expect(await statusOf(inactive)).toBe('dead');
+    });
+
+    it('control: an active, signed subscription is replayed by both faces', async () => {
+        const { fixture, member, attempted, statusOf } = await boot({ server: 'both' });
+        for (const [face, send] of faces(fixture, member)) {
+            const id = await attempted(ORG_A, SIGNED_SUBSCRIPTION);
+            const answer = await snapshot(await send(redeliver({ deliveryId: id })));
+            expect(answer.status, face).toBe(200);
+            expect(json(answer), face).toEqual({ success: true, data: { id, status: 'pending' } });
+            expect(await statusOf(id), face).toBe('pending');
+        }
+    });
+
+    it('control: a gone subscription keeps its own refusal, unchanged', async () => {
+        const { fixture, member, attempted, statusOf } = await boot({ server: 'both' });
+        const gone = await attempted(ORG_A, GONE_SUBSCRIPTION);
+        for (const [face, send] of faces(fixture, member)) {
+            const answer = await snapshot(await send(redeliver({ deliveryId: gone })));
+            expect(answer.status, face).toBe(409);
+            const body = json(answer);
+            expect(body.error.code, face).toBe('DELIVERY_NOT_ELIGIBLE');
+            expect(body.error.message, face).toContain(GONE_SUBSCRIPTION);
+            expect(body.error.message, face).toContain('no longer exists');
+            expect(body.error.message, face).not.toMatch(/\binactive\b/);
+        }
+        expect(await statusOf(gone)).toBe('dead');
+    });
+
+    it('control: the same delivery is replayed once its webhook is re-activated', async () => {
+        const { fixture, member, attempted, statusOf } = await boot({ server: 'both' });
+        const id = await attempted(ORG_A, INACTIVE_SUBSCRIPTION);
+        expect((await member(redeliver({ deliveryId: id }))).status).toBe(409);
+        expect(await statusOf(id)).toBe('dead');
+
+        fixture.subscriptions.get(INACTIVE_SUBSCRIPTION)!.active = true;
+
+        const answer = await snapshot(await member(redeliver({ deliveryId: id })));
+        expect(answer.status).toBe(200);
+        expect(json(answer)).toEqual({ success: true, data: { id, status: 'pending' } });
+        expect(await statusOf(id)).toBe('pending');
+    });
+});