Harden Stripe webhook setup: same-URL cleanup, atomic credentials, shared URL helper

TODO.md

@@ -1030,6 +1030,39 @@ is next touched. Keep the copy plain per the Simple-Language rules.
 
 ---
 
+---
+
+## Stripe webhook setup hardening — deferred edges (from PR #1827)
+
+*Origin: CodeRabbit and Codex review of PR #1827 (the same-URL cleanup +
+atomic credentials + shared URL helper PR). The create-first refactor and
+endpoint-limit fallback were applied in that PR; the one edge below was
+judged out of scope and recorded here.*
+
+`setupWebhookEndpointImpl` in `src/shared/stripe.ts` creates the new endpoint
+only — old same-URL endpoints are deleted by a separate
+`cleanupOldWebhookEndpoints` call that the settings route invokes AFTER
+`settings.update.stripe.webhookConfig` saves the new endpoint ID + secret to
+the DB. This ordering ensures a DB-save failure leaves the old endpoint
+(whose secret matches the DB) in place. If Stripe rejects the create because
+the account is at its webhook-endpoint cap, setup deletes same-URL strays
+(keeping the recorded endpoint intact) and retries the create. One edge
+remains:
+
+- **Same-URL stray listing doesn't paginate.** `fetchWebhookEndpoints` calls
+  `client.webhookEndpoints.list({ limit: 100 })` once and returns `.data`
+  without following Stripe's `has_more` cursor. A site that has accumulated
+  more than 100 webhook endpoints (rare — would require many failed setups
+  or a long-running test environment) would leave strays beyond the first
+  page un-deleted. Impact is limited: the new endpoint is already live and
+  the DB points at it, so leftover strays are duplicate-delivery-only, not
+  a signing-secret mismatch. Fix direction: follow the `has_more`/cursor
+  loop in `fetchWebhookEndpoints` so the same-URL filter sees every
+  endpoint. Starting point: `src/shared/stripe.ts` (`fetchWebhookEndpoints`
+  and `listSameUrlEndpointIds`).
+
+---
+
 ## Bunny subrequest budget follow-ups
 
 *Origin: request-fan-out audit for PR #1820.*

src/features/admin/settings-helpers.ts

@@ -18,7 +18,6 @@ import {
   withAuth,
 } from "#routes/auth.ts";
 import { errorRedirect, jsonResponse, redirect } from "#routes/response.ts";
-import { getEffectiveDomain } from "#shared/config.ts";
 import { logActivity } from "#shared/db/activityLog.ts";
 import { isMaskSentinel } from "#shared/db/settings/mask.ts";
 import { settings } from "#shared/db/settings.ts";
@@ -83,11 +82,6 @@ const advancedSettingsRoute = wrapRoute(ADVANCED_PATH);
 const testRoute = (testFn: () => Promise<unknown>) =>
   gatedPost(OWNER_FORM)(async () => jsonResponse(await testFn()));
 
-/** Build the payment webhook URL from the configured domain.
- * Shared by the settings page (display) and the Stripe handler (setup). */
-const getWebhookUrl = (): string =>
-  `https://${getEffectiveDomain()}/payment/webhook`;
-
 /** Run an optional async validator; return error response or null */
 const runValidate = <T>(
   validate: ValidateFn<T> | undefined,
@@ -411,7 +405,6 @@ export {
   clearableFieldHandler,
   createSettingsHandler,
   defineProviderCredentialsRoute,
-  getWebhookUrl,
   processSecretField,
   saveSecret,
   secretFieldHandler,

src/features/admin/settings-page.ts

@@ -5,7 +5,6 @@
  */
 
 /* jscpd:ignore-start */
-import { getWebhookUrl } from "#routes/admin/settings-helpers.ts";
 import { type AuthSession, ownerPage } from "#routes/auth.ts";
 import type { TypedRouteHandler } from "#routes/router.ts";
 import { getCdnHostname } from "#shared/bunny-cdn.ts";
@@ -17,6 +16,7 @@ import {
 import { settings } from "#shared/db/settings.ts";
 import { EMAIL_PROVIDER_LABELS, getHostEmailConfig } from "#shared/email.ts";
 import { getFlash } from "#shared/flash-context.ts";
+import { getPaymentWebhookUrl } from "#shared/payment-webhook-url.ts";
 import { isStorageEnabled } from "#shared/storage.ts";
 import { getSuperuserState } from "#shared/superuser.ts";
 import { adminSettingsPage } from "#templates/admin/settings.tsx";
@@ -51,7 +51,7 @@ const getSettingsPageState = async () => {
     termsAndConditions: settings.terms,
     theme: settings.theme,
     underlineLinks: settings.underlineLinks,
-    webhookUrl: getWebhookUrl(),
+    webhookUrl: getPaymentWebhookUrl(),
   };
 };
 

src/features/admin/settings-stripe.ts

@@ -4,13 +4,12 @@
  */
 
 import { t } from "#i18n";
-import {
-  defineProviderCredentialsRoute,
-  getWebhookUrl,
-} from "#routes/admin/settings-helpers.ts";
+import { defineProviderCredentialsRoute } from "#routes/admin/settings-helpers.ts";
 import { settings } from "#shared/db/settings.ts";
 import { isDemoMode } from "#shared/demo/mode.ts";
+import { getPaymentWebhookUrl } from "#shared/payment-webhook-url.ts";
 import {
+  cleanupOldWebhookEndpoints,
   detectStripeKeyMode,
   setupWebhookEndpoint,
   testStripeConnection,
@@ -20,15 +19,31 @@ export const stripeRoutes = defineProviderCredentialsRoute<undefined>({
   // Provision the Stripe webhook before the key is persisted, so a setup
   // failure aborts the save leaving nothing configured.
   afterSave: async (value) => {
+    const webhookUrl = getPaymentWebhookUrl();
+    const previousEndpointId = settings.stripe.webhookEndpointId;
     const result = await setupWebhookEndpoint(
       value,
-      getWebhookUrl(),
-      settings.stripe.webhookEndpointId,
+      webhookUrl,
+      previousEndpointId,
     );
     if (!result.success) {
       return `Failed to set up Stripe webhook: ${result.error}`;
     }
+    // Save the new endpoint ID + secret to the DB FIRST. If this fails, the
+    // old endpoint (whose secret matches the DB) stays in place — webhooks
+    // keep delivering with the old config instead of losing the only signed
+    // endpoint mid-replacement.
     await settings.update.stripe.webhookConfig(result);
+    // Now that the new credentials are saved, delete old endpoints. Pass the
+    // previous endpoint ID explicitly — after a domain change it points at
+    // the old URL, so the same-URL listing won't find it. Cleanup is
+    // best-effort: a failure here doesn't unwind the save.
+    await cleanupOldWebhookEndpoints(
+      value,
+      webhookUrl,
+      result.endpointId,
+      previousEndpointId ? [previousEndpointId] : [],
+    );
     return null;
   },
   formId: "settings-stripe",

src/features/api/webhooks.ts

@@ -42,13 +42,13 @@ import {
   verifyTokensWithRealLine,
 } from "#routes/tickets/token-utils.ts";
 import { getSearchParam } from "#routes/url.ts";
-import { getEffectiveDomain } from "#shared/config.ts";
 /* jscpd:ignore-end */
 import { getHiddenPackageMemberIds } from "#shared/db/groups.ts";
 import { getListingWithCount } from "#shared/db/listings/records.ts";
 import { clearSessionTokens } from "#shared/db/processed-payments.ts";
 import { ErrorCode, logDebug, logError } from "#shared/logger.ts";
 import { WEBHOOK_SIGNATURE_HEADERS } from "#shared/payment-providers.ts";
+import { getPaymentWebhookUrl } from "#shared/payment-webhook-url.ts";
 import {
   getActivePaymentProvider,
   type ValidatedPaymentSession,
@@ -354,7 +354,7 @@ const authenticateWebhook = async (
   // webhook using the exact notification URL from the subscription, which is the
   // public https:// URL. Deriving from request.url fails behind CDNs that
   // terminate TLS (the edge runtime sees http:// instead of https://).
-  const webhookUrl = `https://${getEffectiveDomain()}/payment/webhook`;
+  const webhookUrl = getPaymentWebhookUrl();
   const verification = await provider.verifyWebhookSignature(
     payload,
     signature,

src/shared/db/settings.ts

@@ -60,6 +60,7 @@ import {
   writeEncrypted,
   writeOrDelete,
   writeRaw,
+  writeRawBatch,
 } from "#shared/db/settings/raw-writes.ts";
 import {
   clearSetupCompleteCache,
@@ -460,14 +461,12 @@ const settingsBase = {
         secret: string;
         endpointId: string;
       }): Promise<void> => {
-        await writeRaw(
-          CONFIG_KEYS.STRIPE_WEBHOOK_SECRET,
-          await encrypt(config.secret),
-        );
-        await writeRaw(
-          CONFIG_KEYS.STRIPE_WEBHOOK_ENDPOINT_ID,
-          config.endpointId,
-        );
+        // Save both values in one atomic batch so a partial write can never
+        // leave a new secret paired with an old endpoint ID (or vice versa).
+        await writeRawBatch([
+          [CONFIG_KEYS.STRIPE_WEBHOOK_SECRET, await encrypt(config.secret)],
+          [CONFIG_KEYS.STRIPE_WEBHOOK_ENDPOINT_ID, config.endpointId],
+        ]);
         data.stripe_webhook_secret = config.secret;
         data.stripe_webhook_endpoint_id = config.endpointId;
       },

src/shared/payment-webhook-url.ts

@@ -0,0 +1,23 @@
+/**
+ * One home for the payment webhook URL: the route Stripe/Square/SumUp deliver
+ * signed payment callbacks to, and the page a customer's browser lands on
+ * after a hosted checkout. Replacing a provider's endpoint (or building the URL
+ * a webhook signature is verified against) goes through here so every caller
+ * reads the same host.
+ *
+ * The constructor is pure (taking the domain in), leaving the domain lookup to a
+ * thin wrapper so the URL shape can be unit-tested without seeding a domain.
+ */
+
+import { getEffectiveDomain } from "#shared/config.ts";
+
+/** The fixed path under the effective domain that receives payment callbacks. */
+const PAYMENT_WEBHOOK_PATH = "/payment/webhook";
+
+/** Build the public payment webhook URL for the given domain. */
+const paymentWebhookUrl = (domain: string): string =>
+  `https://${domain}${PAYMENT_WEBHOOK_PATH}`;
+
+/** Build the payment webhook URL from the current effective domain. */
+export const getPaymentWebhookUrl = (): string =>
+  paymentWebhookUrl(getEffectiveDomain());