fix(billing): pre-LIVE critical fixes — free plan extras, /plans DoS, dispute/customer handlers (#3602)

1. Free plan + extras pack: incrementMeter now debits extras when meterQuota=0
   (was: extras balance never decreased — infinite usage from a $5 pack).

2. /api/billing/plans: in-flight promise dedup in BillingPlansService.getPlans
   + per-IP rate limit profile (60s/30 req in prod) on the public route.
   Hardens against Stripe-API-quota DoS that would break LIVE checkout +
   webhook signature verification.

3. Webhook handlers:
   - customer.deleted: nulls stripeCustomerId/subscriptionId, plan=free,
     status=canceled + meter rotation. Prevents "No such customer" crashes
     on next checkout when a customer is deleted in the Stripe dashboard.
   - charge.dispute.created: log + emit billing.dispute.opened event
     (no auto-debit — ~50% of disputes are won by merchant; real claw-back
     happens later on charge.dispute.funds_withdrawn or charge.refunded).

Audit findings before LIVE recalibration ship. No drive-by changes.

Author: PierreBrisorgueil

config/defaults/production.config.js

@@ -38,6 +38,15 @@ const config = {
       standardHeaders: true,
       legacyHeaders: false,
     },
+    // Public, unauthenticated route that fans out to Stripe on cache miss.
+    // Tighter window than `api` to harden against Stripe-API-quota DoS.
+    billingPlans: {
+      windowMs: 60 * 1000,
+      max: 30,
+      message: { message: 'Too many requests, please try again later.' },
+      standardHeaders: true,
+      legacyHeaders: false,
+    },
   },
   log: {
     format: 'custom',

config/defaults/test.config.js

@@ -41,6 +41,9 @@ const config = {
     api: {
       max: Number.MAX_SAFE_INTEGER, // disable rate limiting in tests
     },
+    billingPlans: {
+      max: Number.MAX_SAFE_INTEGER, // disable rate limiting in tests
+    },
   },
   uploads: {
     avatar: {

modules/billing/controllers/billing.webhook.controller.js

@@ -62,6 +62,16 @@ const handleWebhook = async (req, res) => {
           BillingWebhookService.handleChargeRefunded(e.data.object),
         );
         break;
+      case 'customer.deleted':
+        await BillingWebhookService.withIdempotency(event, (e) =>
+          BillingWebhookService.handleCustomerDeleted(e.data.object, e),
+        );
+        break;
+      case 'charge.dispute.created':
+        await BillingWebhookService.withIdempotency(event, (e) =>
+          BillingWebhookService.handleChargeDisputeCreated(e.data.object, e),
+        );
+        break;
       default:
         break;
     }

modules/billing/routes/billing.routes.js

@@ -4,6 +4,7 @@
 import config from '../../../config/index.js';
 import passport from 'passport';
 
+import limiters from '../../../lib/middlewares/rateLimiter.js';
 import model from '../../../lib/middlewares/model.js';
 import policy from '../../../lib/middlewares/policy.js';
 import organization from '../../organizations/middlewares/organizations.middleware.js';
@@ -28,7 +29,11 @@ export default (app) => {
   ];
 
   // plans (public)
-  app.route('/api/billing/plans').get(billingPlans.getPlans);
+  // Rate-limited at the edge: this route fans out to Stripe products+prices on cache miss,
+  // so an unauthenticated flood would drain the Stripe API quota and break LIVE checkout +
+  // webhook signature verification. Service layer also dedups concurrent in-flight fetches.
+  // Falls back to a passthrough middleware when `billingPlans` profile is not configured.
+  app.route('/api/billing/plans').get(limiters.billingPlans, billingPlans.getPlans);
 
   // checkout
   app

modules/billing/services/billing.plans.service.js

@@ -10,6 +10,17 @@ let cachedPlans = null;
 let cacheTimestamp = 0;
 const CACHE_TTL = 60 * 60 * 1000; // 1 hour
 
+/**
+ * In-flight fetch promise — used to deduplicate concurrent cache-miss callers.
+ * The /api/billing/plans route is public, so a thundering herd on cache miss
+ * (e.g. cache expiry under traffic, or a small DoS) would otherwise fire N
+ * parallel `stripe.products.list().autoPagingToArray()` + `stripe.prices.list()`
+ * calls and exhaust the Stripe API quota — breaking live checkouts and webhook
+ * signature verification. With this dedup, only one Stripe round-trip happens
+ * per cache-miss window regardless of concurrency.
+ */
+let inFlightFetch = null;
+
 /**
  * Default free plan returned when Stripe is not configured
  */
@@ -83,10 +94,24 @@ const getPlans = async () => {
   const now = Date.now();
   if (cachedPlans && now - cacheTimestamp < CACHE_TTL) return cachedPlans;
 
-  const plans = await fetchPlansFromStripe(stripe);
-  cachedPlans = plans;
-  cacheTimestamp = Date.now();
-  return plans;
+  // In-flight dedup: if another caller is already fetching, await its result
+  // instead of issuing parallel Stripe API calls. Reset on completion so the
+  // next cache miss can issue a fresh fetch (success OR failure — failures must
+  // not poison the slot, otherwise the next call retries cleanly).
+  if (inFlightFetch) return inFlightFetch;
+
+  inFlightFetch = (async () => {
+    try {
+      const plans = await fetchPlansFromStripe(stripe);
+      cachedPlans = plans;
+      cacheTimestamp = Date.now();
+      return plans;
+    } finally {
+      inFlightFetch = null;
+    }
+  })();
+
+  return inFlightFetch;
 };
 
 export default {

modules/billing/services/billing.usage.service.js

@@ -122,10 +122,17 @@ const incrementMeter = async (organizationId, units, breakdown, idempotencyKey)
   const newMeterUsed = updatedDoc.meterUsed ?? 0;
   const effectiveQuota = updatedDoc.meterQuota ?? meterQuota;
 
-  // Overflow detection: units consumed beyond the plan quota go to extras
+  // Overflow detection: units consumed beyond the plan quota go to extras.
+  // Free plan (effectiveQuota === 0): every unit must be debited from extras —
+  // requireQuota middleware lets the request through only when extrasBalance > 0,
+  // so reaching this branch means the org pays for usage from its extras pack.
+  // Without this branch the extras balance would never decrease on free plans
+  // (infinite usage from a $5 pack — money leak).
   let extrasConsumed = 0;
-  if (effectiveQuota > 0 && newMeterUsed > effectiveQuota) {
-    const previousUsed = newMeterUsed - units;
+  if (effectiveQuota === 0) {
+    extrasConsumed = units;
+  } else if (newMeterUsed > effectiveQuota) {
+    const previousUsed = Math.max(0, newMeterUsed - units);
     const overflowStart = Math.max(previousUsed, effectiveQuota);
     extrasConsumed = newMeterUsed - overflowStart;
   }

modules/billing/services/billing.webhook.service.js

@@ -565,6 +565,155 @@ const handleChargeRefunded = async (charge) => {
   }
 };
 
+/**
+ * @description Handle customer.deleted event — null out Stripe customer/subscription refs.
+ *       Triggered when an admin deletes a customer in the Stripe dashboard. If we keep the
+ *       stale stripeCustomerId in our DB, the next _ensureStripeCustomer / checkout call
+ *       will hit "No such customer" and crash the user's checkout flow.
+ *
+ *       Conservative recovery:
+ *         1. Find the subscription by Stripe customer id.
+ *         2. Null out stripeCustomerId + stripeSubscriptionId, force plan='free' / status='canceled'.
+ *         3. Sync organization plan to free.
+ *         4. Force meter rotation so the org no longer carries the paid quota snapshot.
+ *
+ *       No-op when no matching subscription exists in our DB (deletion of a customer we never
+ *       provisioned, or already cleaned up).
+ * @param {Object} customer - Stripe customer object (data.object of customer.deleted event).
+ * @param {Object} event - Full Stripe event (for ordering / event-newer guard).
+ * @returns {Promise<void>}
+ */
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
+const handleCustomerDeleted = async (customer, event) => {
+  const stripeCustomerId = customer?.id;
+  if (!stripeCustomerId) return;
+
+  const existing = await SubscriptionRepository.findByStripeCustomerId(stripeCustomerId);
+  if (!existing) return;
+
+  const updated = await SubscriptionRepository.updateIfEventNewer(
+    String(existing._id),
+    event.created,
+    event.id,
+    {
+      stripeCustomerId: null,
+      stripeSubscriptionId: null,
+      plan: 'free',
+      status: 'canceled',
+    },
+    'subscription',
+  );
+  if (!updated) {
+    logger.info('[billing.webhook] skipped stale event', { eventId: event.id, type: event.type });
+    return;
+  }
+
+  const organizationId = String(existing.organization?._id || existing.organization);
+  await syncOrganizationPlan(organizationId, 'free');
+
+  // Force meter reset so the cancelled org no longer retains a paid-plan snapshot.
+  try {
+    await BillingResetService.forceRotateForPlanChange(organizationId, { preserveUsage: false });
+  } catch (err) {
+    logger.error('[billing.webhook] forceRotateForPlanChange on customer.deleted failed (non-fatal)', {
+      organizationId,
+      error: err?.message ?? String(err),
+      stack: err?.stack,
+    });
+  }
+};
+
+/**
+ * @description Handle charge.dispute.created event — log + emit (no auto-debit).
+ *       Triggered when a cardholder files a chargeback. Stripe will eventually debit the
+ *       merchant bank account, but disputes are often won by the merchant (~50% are user
+ *       error / "I don't recognise this charge"). Aggressive auto-debit on dispute opening
+ *       would punish customers who are filing legitimate disputes that we eventually win.
+ *
+ *       Conservative policy: emit a `billing.dispute.opened` event for downstream listeners
+ *       (admin notifications) and log a critical alert. Real claw-back happens later via
+ *       `charge.dispute.funds_withdrawn` (when the dispute is lost) or `charge.refunded`
+ *       (when we choose to refund), both of which already debit the ledger via
+ *       handleChargeRefunded / refundPartial.
+ *
+ *       Resolves organizationId via charge metadata first, then falls back to the
+ *       PaymentIntent metadata patched by handleCheckoutPaymentCompleted.
+ * @param {Object} dispute - Stripe dispute object (data.object of charge.dispute.created).
+ * @param {Object} event - Full Stripe event.
+ * @returns {Promise<void>}
+ */
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
+const handleChargeDisputeCreated = async (dispute, event) => {
+  const chargeId = dispute?.charge;
+  const disputeId = dispute?.id;
+  const amount = dispute?.amount ?? 0;
+  const reason = dispute?.reason ?? null;
+
+  let organizationId = null;
+  let stripeSessionId = null;
+  let paymentIntentId = null;
+
+  // Best-effort: fetch charge → resolve org via charge or PaymentIntent metadata
+  // (mirrors the resolver pattern in handleChargeRefunded).
+  if (chargeId) {
+    const stripe = getStripe();
+    if (stripe) {
+      try {
+        const charge = await stripe.charges.retrieve(chargeId);
+        const meta = charge?.metadata ?? {};
+        organizationId = meta.organizationId ?? null;
+        stripeSessionId = meta.stripeSessionId ?? null;
+        paymentIntentId = charge?.payment_intent ?? null;
+
+        if ((!organizationId || !mongoose.Types.ObjectId.isValid(organizationId)) && paymentIntentId) {
+          const paymentIntent = await stripe.paymentIntents.retrieve(paymentIntentId);
+          const piMeta = paymentIntent?.metadata ?? {};
+          if (!organizationId && piMeta.organizationId && mongoose.Types.ObjectId.isValid(piMeta.organizationId)) {
+            organizationId = piMeta.organizationId;
+          }
+          if (!stripeSessionId) stripeSessionId = piMeta.stripeSessionId ?? null;
+        }
+      } catch (err) {
+        logger.error('[billing.webhook] dispute charge/PI fetch failed', {
+          chargeId,
+          disputeId,
+          error: err?.message ?? String(err),
+          stack: err?.stack,
+        });
+      }
+    }
+  }
+
+  // Critical alert — ops MUST react manually (decide to fight or accept the dispute).
+  // No auto-debit: real claw-back happens on charge.dispute.funds_withdrawn / charge.refunded.
+  logger.error('[billing] dispute opened — manual review required', {
+    disputeId,
+    chargeId,
+    organizationId,
+    stripeSessionId,
+    amount,
+    reason,
+    eventId: event?.id,
+  });
+
+  try {
+    billingEvents.emit('billing.dispute.opened', {
+      disputeId,
+      chargeId,
+      organizationId,
+      stripeSessionId,
+      amount,
+      reason,
+    });
+  } catch (evtErr) {
+    // Listener errors must not break webhook processing — log for traceability.
+    logger.error('[billing.webhook] billing.dispute.opened listener error (non-fatal)', {
+      error: evtErr?.message ?? String(evtErr),
+      stack: evtErr?.stack,
+    });
+  }
+};
+
 export default {
   withIdempotency,
   handleCheckoutSessionCompleted,
@@ -575,4 +724,6 @@ export default {
   handleInvoicePaymentFailed,
   handleInvoicePaymentSucceeded,
   handleChargeRefunded,
+  handleCustomerDeleted,
+  handleChargeDisputeCreated,
 };

modules/billing/tests/billing.plans.unit.tests.js

@@ -226,6 +226,91 @@ describe('Billing plans service unit tests:', () => {
     expect(mod.default.fetchPlansFromStripe).toBeUndefined();
   });
 
+  // ── In-flight dedup — Stripe-API-quota DoS hardening ────────────────────
+  // The /api/billing/plans route is public, so a thundering herd on cache miss
+  // (or a small DoS) would otherwise fire N parallel `stripe.products.list +
+  // stripe.prices.list` calls and exhaust the Stripe API quota — breaking live
+  // checkouts and webhook signature verification. The service layer dedups
+  // concurrent in-flight fetches into a single Stripe round-trip.
+
+  test('100 concurrent getPlans() calls during cache miss → exactly ONE Stripe products.list call', async () => {
+    const mod = await import('../services/billing.plans.service.js');
+    BillingPlansService = mod.default;
+
+    // Resolve manually so all 100 callers race the same in-flight promise.
+    let resolveProducts;
+    mockStripeInstance.products.list.mockReturnValue({
+      autoPagingToArray: jest.fn().mockReturnValue(
+        new Promise((resolve) => {
+          resolveProducts = () => resolve(productsData);
+        }),
+      ),
+    });
+
+    const inflight = Array.from({ length: 100 }, () => BillingPlansService.getPlans());
+
+    // Resolve the underlying Stripe call once — all 100 callers should share its result.
+    resolveProducts();
+    const results = await Promise.all(inflight);
+
+    expect(mockStripeInstance.products.list).toHaveBeenCalledTimes(1);
+    expect(mockStripeInstance.prices.list).toHaveBeenCalledTimes(1);
+    // Every caller gets the same plan list.
+    for (const plans of results) expect(plans).toHaveLength(2);
+  });
+
+  test('after cache TTL expires, a fresh thundering herd still triggers ONE fetch', async () => {
+    const mod = await import('../services/billing.plans.service.js');
+    BillingPlansService = mod.default;
+
+    // First call populates the cache.
+    await BillingPlansService.getPlans();
+    expect(mockStripeInstance.products.list).toHaveBeenCalledTimes(1);
+
+    // Advance time past TTL.
+    const originalDateNow = Date.now;
+    Date.now = jest.fn().mockReturnValue(originalDateNow() + 61 * 60 * 1000);
+
+    // 50 concurrent callers after TTL expires.
+    let resolveProducts;
+    mockStripeInstance.products.list.mockReturnValue({
+      autoPagingToArray: jest.fn().mockReturnValue(
+        new Promise((resolve) => {
+          resolveProducts = () => resolve(productsData);
+        }),
+      ),
+    });
+
+    const inflight = Array.from({ length: 50 }, () => BillingPlansService.getPlans());
+    resolveProducts();
+    await Promise.all(inflight);
+
+    // Total: 1 (initial) + 1 (after TTL) — not 1 + 50.
+    expect(mockStripeInstance.products.list).toHaveBeenCalledTimes(2);
+
+    Date.now = originalDateNow;
+  });
+
+  test('in-flight slot resets after fetch failure → next call retries cleanly', async () => {
+    const mod = await import('../services/billing.plans.service.js');
+    BillingPlansService = mod.default;
+
+    // First fetch fails — slot must reset, otherwise the next call hangs forever.
+    mockStripeInstance.products.list.mockReturnValueOnce({
+      autoPagingToArray: jest.fn().mockRejectedValue(new Error('Stripe API down')),
+    });
+
+    await expect(BillingPlansService.getPlans()).rejects.toThrow('Stripe API down');
+
+    // Second call should issue a NEW Stripe round-trip (slot was cleared in finally).
+    mockStripeInstance.products.list.mockReturnValueOnce(mockListResult(productsData));
+    mockStripeInstance.prices.list.mockReturnValueOnce(mockListResult(pricesData));
+
+    const plans = await BillingPlansService.getPlans();
+    expect(plans).toHaveLength(2);
+    expect(mockStripeInstance.products.list).toHaveBeenCalledTimes(2);
+  });
+
   test('facade: getPlans returns array of plan objects', async () => {
     const mod = await import('../services/billing.plans.service.js');
     BillingPlansService = mod.default;