fix(webhooks): harden verify, create, replay, and relay edge cases

Adversarial audit follow-ups on the unreleased webhooks group:

- verify: reject an explicit empty --payload as a usage error instead of
  hashing an `undefined` pre-image and silently failing (exit 2, not 1).
- create: propagate an AuthError from the post-create secret fetch instead
  of masking it as "secret unavailable"; tag the genuine partial-failure
  with the new webhook_secret_fetch_failed code for agent branching.
- replay: `--until` alone now points at the missing --since rather than
  emitting the vaguer "pass <msg_id> or --since" hint.
- relay-client: route the 1008 token-collision redial through the standard
  reconnect backoff (no zero-delay storm) and guard onopen against a stop()
  that races socket construction.
- README: document at-least-once redelivery on reconnect (handlers must
  key on svix-id).

Adds tests for each fix. Full suite 1934 pass / 0 fail.

Claude-Session: https://claude.ai/code/session_015J6Sduw5KeHz6SxLEBfViF

Author: rafa-thayto

packages/cli-core/src/commands/webhooks/README.md

@@ -250,6 +250,7 @@ Behavior notes:
 - **Keepalive**: the relay server pings ~every 21s, but Bun's client WebSocket auto-pongs below the JS API (no ping events). A probe timer fires every `RELAY_SILENCE_TIMEOUT_MS / 2` (~15s) and, once at least `RELAY_SILENCE_TIMEOUT_MS` (30s) has elapsed with no inbound message, sends an active `ws.ping()` (so a probe lands 30–45s into a silence, depending on timer phase) — writes to a dead link surface as close/error, which redials with the same token. Reconnects never change the relay URL.
 - **Per-delivery output**: human mode prints `time --> event_type msg_…` then `<-- status method path ms` via `log.ui` (bypasses the stderr throttle). Diagnostics: 401 → `clerkMiddleware` public-route hint; 400 → raw-body/`verifyWebhook()` order hint; 5xx → response body inline plus the exact `clerk webhooks replay <msg_id>` line; unreachable handler → synthetic **502** framed back to the relay.
 - **Verification**: deliveries failing HMAC are warned about and still forwarded (the mismatch means the relay secret diverged, not that the local handler should silently miss events).
+- **At-least-once**: forwarding is at-least-once, like any webhook stream. If the relay socket drops while a delivery is mid-forward, its response frame is sent on the closed socket and dropped, so Svix may redeliver it (and the new inbox URL after a 1008 rotation only appears in the next `ready` line on restart). Local handlers must key on `svix-id` and be idempotent.
 - **Agent/`--json` mode**: NDJSON on stdout. Every line carries a `type` discriminator: one `{ "type": "ready", ... }` line (`relay_url`, `signing_secret`, `endpoint_id`, `events_filter`), then one `{ "type": "event", ... }` line per delivery (`svix_id`, `event_type`, `headers`, `body_b64`, `forward_status`, `latency_ms`). An event line saved to a file is directly consumable by `verify --delivery @file`.
 - **SIGINT**: `listen` replaces the global cleanup-free handler before opening the socket: close socket, drain in-flight forwards, exit 130. The relay endpoint is **never** deleted on exit — its URL and `whsec_` stay stable across restarts. `listen` never exits 0.
 

packages/cli-core/src/commands/webhooks/create.test.ts

@@ -1,5 +1,5 @@
 import { test, expect, describe, beforeEach, afterEach, mock } from "bun:test";
-import { CliError, ERROR_CODE, PlapiError } from "../../lib/errors.ts";
+import { AuthError, CliError, ERROR_CODE, PlapiError } from "../../lib/errors.ts";
 import { useCaptureLog } from "../../test/lib/stubs.ts";
 
 const mockCreateWebhookEndpoint = mock();
@@ -134,9 +134,26 @@ describe("webhooks create", () => {
     const promise = webhooksCreate({ url: "https://example.com/webhooks" });
 
     await expect(promise).rejects.toBeInstanceOf(CliError);
+    await expect(promise).rejects.toMatchObject({
+      code: ERROR_CODE.WEBHOOK_SECRET_FETCH_FAILED,
+    });
     await expect(promise).rejects.toThrow(
       "Endpoint created (id: ep_new) but the signing secret could not be fetched. " +
         "Run 'clerk webhooks secret ep_new' to retrieve it.",
     );
   });
+
+  test("partial failure: an auth error on the secret fetch is not masked as a secret-fetch failure", async () => {
+    mockGetWebhookEndpointSecret.mockRejectedValue(
+      new AuthError({
+        reason: "session_expired",
+        message: "Session expired. Run `clerk auth login`.",
+      }),
+    );
+
+    const promise = webhooksCreate({ url: "https://example.com/webhooks" });
+
+    await expect(promise).rejects.toBeInstanceOf(AuthError);
+    await expect(promise).rejects.toThrow("Session expired");
+  });
 });

packages/cli-core/src/commands/webhooks/create.ts

@@ -1,5 +1,5 @@
 import { resolveAppContext } from "../../lib/config.ts";
-import { CliError, throwUsageError } from "../../lib/errors.ts";
+import { AuthError, CliError, ERROR_CODE, throwUsageError } from "../../lib/errors.ts";
 import { log } from "../../lib/log.ts";
 import {
   createWebhookEndpoint,
@@ -46,12 +46,17 @@ export async function webhooksCreate(options: WebhooksCreateOptions = {}): Promi
   let secret: string;
   try {
     ({ secret } = await getWebhookEndpointSecret(ctx.appId, ctx.instanceId, endpoint.id));
-  } catch {
+  } catch (error) {
+    // An auth failure on the second call is the real problem — let it surface
+    // with its own reason and docs URL instead of being masked as a transient
+    // secret-fetch hiccup.
+    if (error instanceof AuthError) throw error;
     // Create is atomic; the secret fetch is a second call. Never leave a
     // silent orphan — surface the new ID and the exact recovery command.
     throw new CliError(
       `Endpoint created (id: ${endpoint.id}) but the signing secret could not be fetched. ` +
         `Run 'clerk webhooks secret ${endpoint.id}' to retrieve it.`,
+      { code: ERROR_CODE.WEBHOOK_SECRET_FETCH_FAILED },
     );
   }
 

packages/cli-core/src/commands/webhooks/relay-client.test.ts

@@ -219,7 +219,7 @@ describe("RelayClient", () => {
     expect(harness.rotated).toHaveLength(0);
   });
 
-  test("a 1008 collision rotates to a fresh c_ token, persists it, and redials", async () => {
+  test("a 1008 collision rotates to a fresh c_ token, persists it, and redials after the reconnect delay", async () => {
     const harness = await openClient();
     wsAt(0).fireClose(RELAY_CLOSE_TOKEN_COLLISION);
     await flush();
@@ -228,12 +228,31 @@ describe("RelayClient", () => {
     expect(harness.client.token).toMatch(/^c_[0-9A-Za-z]{10}$/);
     expect(harness.rotated).toEqual([harness.client.token]);
 
-    expect(FakeWebSocket.instances).toHaveLength(2); // immediate redial, no reconnect-delay timer
+    // Redial is deferred through the reconnect backoff so a relay that rejects
+    // every fresh token can't drive a zero-delay reconnect storm.
+    expect(FakeWebSocket.instances).toHaveLength(1);
+    expect(timeoutDelay).toBe(RELAY_RECONNECT_DELAY_MS);
+
+    timeoutCallback?.();
+    expect(FakeWebSocket.instances).toHaveLength(2);
     wsAt(1).open();
     expect(wsAt(1).sent[0]).toBe(encodeStartFrame(harness.client.token));
     expect(harness.reconnects).toBe(0); // collision is not a generic reconnect
   });
 
+  test("stop() before the socket opens suppresses the start frame and the keepalive probe", async () => {
+    const harness = makeClient();
+    void harness.client.start(); // never resolves; the socket never finishes opening
+    const ws = wsAt(0);
+
+    harness.client.stop();
+    ws.open();
+
+    expect(ws.sent).toHaveLength(0); // no start frame on a stopped client
+    expect(ws.closedWith).toBe(1000);
+    expect(intervalCallback).toBeUndefined(); // probe timer never armed
+  });
+
   test("stop() closes with 1000 and suppresses any further reconnect", async () => {
     const harness = await openClient();
     const ws = wsAt(0);

packages/cli-core/src/commands/webhooks/relay-client.ts

@@ -63,6 +63,12 @@ export class RelayClient {
     this.ws = ws;
 
     ws.onopen = () => {
+      // stop() may have raced in between `new WebSocket` and this open; if so,
+      // don't send the start frame, arm the probe timer, or resolve start().
+      if (this.stopped) {
+        ws.close(1000);
+        return;
+      }
       log.debug(`relay: connected, sending start frame (token=${this.token})`);
       ws.send(encodeStartFrame(this.token));
       this.lastActivityAt = Date.now();
@@ -93,10 +99,15 @@ export class RelayClient {
       if (this.stopped) return;
 
       if (event.code === RELAY_CLOSE_TOKEN_COLLISION) {
-        // Another listener holds this token: rotate, persist, redial.
+        // Another listener holds this token: rotate, persist, redial. Reconnect
+        // through the same backoff as a normal drop so a relay that rejects
+        // every fresh token can't spin a zero-delay reconnect storm.
         this.token = generateRelayToken();
         log.debug("relay: token collision (1008), rotating token");
-        void this.options.onTokenRotated(this.token).then(() => this.connect());
+        void this.options.onTokenRotated(this.token).then(() => {
+          if (this.stopped) return;
+          setTimeout(() => this.connect(), RELAY_RECONNECT_DELAY_MS);
+        });
         return;
       }
 

packages/cli-core/src/commands/webhooks/replay.test.ts

@@ -91,6 +91,12 @@ describe("webhooks replay", () => {
     expect(mockRecoverWebhookMessages).not.toHaveBeenCalled();
   });
 
+  test("--until alone points at the missing --since instead of a vaguer hint", async () => {
+    await expect(webhooksReplay({ until: "2026-05-01T00:00:00Z" })).rejects.toThrow(
+      "--until requires --since.",
+    );
+  });
+
   test("resends one message to an explicit --endpoint without prompting", async () => {
     await webhooksReplay({ msgId: "msg_1", endpoint: "ep_1" });
 

packages/cli-core/src/commands/webhooks/replay.ts

@@ -28,12 +28,14 @@ function validateReplayMode(options: WebhooksReplayOptions): "resend" | "recover
   if (options.msgId && options.since) {
     throwUsageError("Pass either a <msg_id> or --since, not both.");
   }
-  if (!options.msgId && !options.since) {
-    throwUsageError("Pass a <msg_id> to resend one delivery, or --since <ISO> to bulk-recover.");
-  }
+  // Check the orphaned-`--until` case before the generic "neither" message, so
+  // `replay --until <ISO>` points at the real problem instead of a vaguer hint.
   if (options.until && !options.since) {
     throwUsageError("--until requires --since.");
   }
+  if (!options.msgId && !options.since) {
+    throwUsageError("Pass a <msg_id> to resend one delivery, or --since <ISO> to bulk-recover.");
+  }
   if (options.since) {
     assertRfc3339(options.since, "--since");
     if (options.until) assertRfc3339(options.until, "--until");

packages/cli-core/src/commands/webhooks/verify.test.ts

@@ -179,6 +179,26 @@ describe("webhooks verify command", () => {
     expect(captured.err).toContain("Signature verified.");
   });
 
+  test("a multi-line --delivery file verifies against the first non-empty line", async () => {
+    const firstLine = JSON.stringify({
+      headers: { "svix-id": ID, "svix-timestamp": TIMESTAMP, "svix-signature": VALID_SIGNATURE },
+      body_b64: Buffer.from(PAYLOAD, "utf8").toString("base64"),
+    });
+    const secondLine = JSON.stringify({
+      headers: {
+        "svix-id": "msg_later",
+        "svix-timestamp": TIMESTAMP,
+        "svix-signature": "v1,deadbeef",
+      },
+      body_b64: Buffer.from('{"type":"user.deleted"}', "utf8").toString("base64"),
+    });
+    const deliveryPath = await writeTempFile("events.ndjson", `${firstLine}\n${secondLine}\n`);
+
+    await webhooksVerify({ secret: SECRET, delivery: `@${deliveryPath}` });
+
+    expect(captured.err).toContain("Signature verified.");
+  });
+
   test("explicit flags override --delivery fields", async () => {
     const line = JSON.stringify({
       headers: {
@@ -274,6 +294,18 @@ describe("webhooks verify command", () => {
         payload: "{}",
       },
     },
+    {
+      // An explicit empty --payload must surface as a usage error, not fall
+      // through to the --delivery body or hash an `undefined` pre-image.
+      label: "empty --payload",
+      options: {
+        secret: SECRET,
+        id: ID,
+        timestamp: TIMESTAMP,
+        signature: VALID_SIGNATURE,
+        payload: "",
+      },
+    },
   ])("$label is a usage error", async ({ options }) => {
     await expect(webhooksVerify(options)).rejects.toMatchObject({
       code: ERROR_CODE.USAGE_ERROR,

packages/cli-core/src/commands/webhooks/verify.ts

@@ -162,9 +162,13 @@ export async function webhooksVerify(options: WebhooksVerifyOptions = {}): Promi
     );
   }
 
-  const payload = options.payload
-    ? await readFileOrStdin(options.payload, "--payload")
-    : fields.payload;
+  // Nullish-coalesce, not truthiness: an explicit empty `--payload` must reach
+  // readFileOrStdin (which rejects it as neither @file nor -) instead of
+  // silently falling through to the --delivery body or an `undefined` HMAC.
+  const payload =
+    options.payload !== undefined
+      ? await readFileOrStdin(options.payload, "--payload")
+      : fields.payload;
 
   const valid = verifyWebhookSignature({
     secret: options.secret,

packages/cli-core/src/lib/errors.ts

@@ -63,6 +63,8 @@ export const ERROR_CODE = {
   UNKNOWN_EVENT_TYPE: "unknown_event_type",
   /** Offline webhook signature verification found no matching entry. */
   INVALID_WEBHOOK_SIGNATURE: "invalid_webhook_signature",
+  /** Endpoint was created but its signing secret could not be fetched. */
+  WEBHOOK_SECRET_FETCH_FAILED: "webhook_secret_fetch_failed",
 } as const;
 
 export type ErrorCode = (typeof ERROR_CODE)[keyof typeof ERROR_CODE];