fix(billing): validate webhook plan metadata

[PierreBrisorgueil]

Summary

• Add validatePlan() helper that checks plan values against config.billing.plans before using them
• Apply validation to handleCheckoutCompleted (metadata.plan) and resolvePlan (subscription items metadata)
• Invalid plan values (e.g. Stripe product IDs like prod_ABC123) now fall back to 'free' instead of causing 500 errors
• Add 14 integration tests covering all four webhook handlers (handleCheckoutCompleted, handleSubscriptionUpdated, handleSubscriptionDeleted, handleInvoicePaymentFailed)

Context

QA testing found that checkout.session.completed returned 500 when metadata.plan contained Stripe product IDs instead of plan enum values. This was fixed upstream by adding metadata.planId to Stripe Products, but the webhook code needs to be resilient to invalid values.

Test plan

• Existing 123 billing unit tests pass
• 14 new webhook integration tests pass
• Lint clean
• CodeRabbit review

Summary by CodeRabbit

• Bug Fixes

  • Enhanced plan validation for billing checkout and subscription operations; invalid or unrecognized plan identifiers now safely fall back to the free tier, preventing service disruptions.

• Chores

  • Added integration test coverage for billing webhook handlers.

[coderabbitai]

Walkthrough

Added a config-driven plan validation whitelist to billing webhook handlers. Updated plan resolution for checkout completion and subscription sync to validate plan identifiers against allowed plans and fall back to 'free' for invalid plans. Added comprehensive integration tests for webhook event handlers.

Changes

Cohort / File(s)	Summary
Plan Validation Logic modules/billing/services/billing.webhook.service.js	Added config-driven validPlans whitelist and validatePlan helper function. Updated plan resolution in checkout completion and subscription sync flows to reject unrecognized plan identifiers and fall back to 'free'.
Webhook Handler Tests modules/billing/tests/billing.webhook.integration.tests.js	New integration test suite covering handleCheckoutCompleted, handleSubscriptionUpdated, handleSubscriptionDeleted, and handleInvoicePaymentFailed handlers. Tests verify plan resolution, repository side effects, fallback behavior for invalid/missing plans, and subscription lifecycle field updates (status, currentPeriodEnd, cancelAtPeriodEnd).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• fix(billing): add checkout metadata, harden webhook and customer creation #3265: Modifies webhook plan/metadata handling to add checkout metadata and customer-based organization fallback logic.
• feat(billing): emit plan.changed event on subscription plan change #3274: Changes handleSubscriptionUpdated to detect plan changes and compare plan ranks in subscription handling.

Suggested labels

Fix

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding validation for webhook plan metadata to handle invalid Stripe identifiers.
Description check	✅ Passed	The description covers key aspects (what changed, why, related context) and confirms testing, but is missing several template sections like Scope, full Guardrails checklist, and Risk level.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/billing-webhook-plan-validation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

modules/billing/services/billing.webhook.service.js (1)

117-125: 🧹 Nitpick | 🔵 Trivial

Consider validating previousPlan for consistency.

Unlike newPlan which is validated via resolvePlan, previousPlan is used raw from previous_attributes. If Stripe sends an invalid value like 'prod_ABC123' as the previous plan, it will be emitted in the plan.changed event with isDowngrade: null.

This won't cause 500 errors, but downstream event consumers may receive unexpected plan values.

♻️ Optional: Validate previousPlan

   const previousPlan = previousItems[0]?.price?.metadata?.planId
     || previousItems[0]?.plan?.metadata?.planId
     || null;
-  if (previousPlan && previousPlan !== newPlan) {
+  const validatedPreviousPlan = validatePlan(previousPlan);
+  if (validatedPreviousPlan && validatedPreviousPlan !== newPlan) {
     const prevRank = planRanks[previousPlan];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@modules/billing/services/billing.webhook.service.js` around lines 117 - 125,
Validate the raw previousPlan before using it: extract previousPlan from
event.data.previous_attributes.items.data as you already do, then call the same
resolver (e.g., resolvePlan) or check planRanks to map/normalize it (create
previousResolvedPlan or similar), and use that normalized value in the downgrade
logic (prevRank = planRanks[previousResolvedPlan]) so invalid values like
'prod_ABC123' become null/ignored rather than emitted downstream; update any
uses of previousPlan, prevRank and isDowngrade to rely on the resolved/validated
value.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@modules/billing/tests/billing.webhook.integration.tests.js`:
- Around line 61-66: Add a test that calls
WebhookService.handleSubscriptionUpdated with a subscription payload whose
current items.price.metadata.planId differs from
previous_attributes.items.price.metadata.planId, mocking
mockSubscriptionRepository.findByStripeSubscriptionId and update as needed, then
import the events module (../lib/events.js) and assert its default.emit was
called with 'plan.changed' and a payload containing organizationId (from the
found subscription), previousPlan, newPlan, and isDowngrade boolean; reference
the handleSubscriptionUpdated entrypoint,
mockSubscriptionRepository.findByStripeSubscriptionId/mockSubscriptionRepository.update,
and the events module's default.emit when adding the test.

---

Outside diff comments:
In `@modules/billing/services/billing.webhook.service.js`:
- Around line 117-125: Validate the raw previousPlan before using it: extract
previousPlan from event.data.previous_attributes.items.data as you already do,
then call the same resolver (e.g., resolvePlan) or check planRanks to
map/normalize it (create previousResolvedPlan or similar), and use that
normalized value in the downgrade logic (prevRank =
planRanks[previousResolvedPlan]) so invalid values like 'prod_ABC123' become
null/ignored rather than emitted downstream; update any uses of previousPlan,
prevRank and isDowngrade to rely on the resolved/validated value.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 58a2b2aa-8357-4e02-bce3-da6a7c5eaf6e

📥 Commits

Reviewing files that changed from the base of the PR and between 8b69820 and e7a17ab.

📒 Files selected for processing (2)

• modules/billing/services/billing.webhook.service.js
• modules/billing/tests/billing.webhook.integration.tests.js