Skip to content

fix(console): add idempotency checks to payment webhook handlers#28403

Open
PanAchy wants to merge 5 commits into
anomalyco:devfrom
PanAchy:fix/billing-payment-idempotency
Open

fix(console): add idempotency checks to payment webhook handlers#28403
PanAchy wants to merge 5 commits into
anomalyco:devfrom
PanAchy:fix/billing-payment-idempotency

Conversation

@PanAchy
Copy link
Copy Markdown
Contributor

@PanAchy PanAchy commented May 19, 2026

Issue for this PR

Closes #28402

Type of change

  • Bug fix

What does this PR do?

Two webhook handlers unconditionally added credits to the workspace balance on every Stripe delivery, with no check for whether the same event had already been processed:

  • checkout.session.completed (manual top-up)
  • invoice.payment_succeeded with billing_reason=manual (auto-reload)

Stripe guarantees at-least-once delivery and retries on any 5xx or timeout. Each retry would insert a duplicate PaymentTable row and increment BillingTable.balance again, effectively granting free credits for every transient server error at webhook time.

Fix — application-level idempotency (both handlers):
Before any DB writes, look up whether a PaymentTable row with the same invoiceID exists. Return early if found.

invoiceID is used instead of paymentID because paymentID is null when a coupon covers the full invoice amount — a unique index on a nullable column in MySQL allows multiple NULL rows, so it would not have prevented duplicates for coupon-only invoices.

// Added to both handlers, before the transaction:
const existingPayment = await Database.use((tx) =>
  tx
    .select({ id: PaymentTable.id })
    .from(PaymentTable)
    .where(eq(PaymentTable.invoiceID, invoiceID))
    .then((rows) => rows[0]),
)
if (existingPayment) return

Fix — schema-level safety net (billing.sql.ts):
Added a unique index on PaymentTable.invoice_id. This enforces idempotency at the DB level even if the application check were bypassed (e.g. race condition between two simultaneous deliveries).

// packages/console/core/src/schema/billing.sql.ts
(table) => [...workspaceIndexes(table), uniqueIndex("payment_invoice_id").on(table.invoiceID)],

Note for maintainers: The schema change requires a migration to be generated. Please run bun db generate (or bun db-dev generate) after merging to produce the migration file. The migrations folder has not been modified in this PR — migration generation requires SST infra access that external contributors do not have.

A minor fix is also included in the auto-reload handler: invoice.payments?.data[0]?.payment now uses optional chaining to avoid a crash when the payments array is empty (e.g. coupon-covered invoice with no payment intent).

How did you verify your code works?

  • Ran bun typecheck in both packages/console/app and packages/console/core — no errors.
  • invoiceID is non-null on all invoice and checkout session events. The unique index on invoice_id is safe to add and correctly prevents duplicates including coupon-only invoices.
  • No automated test added: the existing console test suite only covers pure functions with no mocking infrastructure. Webhook handlers require live DB and Stripe context outside the current test conventions.

Screenshots / recordings

If this is a UI change, please include a screenshot or recording.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

If you do not follow this template your PR will be automatically rejected.

Two webhook handlers unconditionally added credits on every delivery:
- checkout.session.completed (manual top-up)
- invoice.payment_succeeded with billing_reason=manual (auto-reload)

Stripe guarantees at-least-once delivery and retries on any 5xx or
timeout. Each retry would insert a duplicate PaymentTable row and
increment the workspace balance again, effectively granting free credits.

Fix:
- Add a unique index on PaymentTable.paymentID in the schema (requires
  migration generation by a maintainer with infra access)
- In both handlers, check if a payment with this paymentID already
  exists and return early if so, before any balance or DB writes
@github-actions
Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Based on my search, I found one potentially related PR:

PR #28400: fix(console): guard against duplicate refund and use actual refund amount

Why it might be related: This PR also addresses webhook handler safety in the billing system, specifically guarding against duplicate refunds. It shares the same theme of preventing duplicate credit operations and idempotency in webhook handlers, though it targets the charge.refunded handler while PR #28403 focuses on checkout.session.completed and invoice.payment_succeeded.

All other results returned PR #28403 (the current PR), which is expected and shouldn't be flagged as a duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate webhook delivery grants free credits

1 participant