Skip to content

252 stripe hardening#253

Merged
InfinityBowman merged 8 commits into
mainfrom
252-stripe-hardening
Jan 8, 2026
Merged

252 stripe hardening#253
InfinityBowman merged 8 commits into
mainfrom
252-stripe-hardening

Conversation

@InfinityBowman
Copy link
Copy Markdown
Owner

@InfinityBowman InfinityBowman commented Jan 8, 2026

Summary by CodeRabbit

  • New Features

    • Enhanced admin database viewer with filtering, foreign key navigation, and related row linking.
    • Comprehensive Stripe webhook handlers for subscriptions, invoices, payment intents, and customer lifecycle events.
    • Automated dunning emails for failed payment recovery.
    • Checkout session completion tracking with grant management.
  • Tests

    • Added test suites covering subscription, invoice, payment intent, customer, and checkout handlers.
  • Documentation

    • Added Stripe hardening plan and admin dashboard architectural guides.

✏️ Tip: You can customize this high-level summary in your review settings.

InfinityBowman and others added 7 commits January 8, 2026 12:32
- Add webhook event router for routing Stripe events to handlers
- Add checkout handlers (extracted from webhooks.js)
- Add subscription handlers for lifecycle events
- Add invoice handlers for payment tracking and dunning
- Add payment intent handlers for async payment tracking
- Add customer handlers for data sync
- Add subscription status mapping utility
- Integrate router into main webhook endpoint
- Add event ID deduplication for authoritative idempotency

All 76 billing tests pass. Phase 1-2 of stripe hardening plan complete.
- Create dunning.js with queueDunningEmail function
- Integrate dunning into handleInvoicePaymentFailed handler
- Escalating email templates based on attempt count (1-3)
- HTML and plain text email templates with urgency indicators
- Update plan to document EmailQueue DO usage
- Export dunning from handlers barrel

Uses existing EmailQueue Durable Object for reliable delivery with:
- Automatic retry with exponential backoff
- Dead letter queue for failed emails
- Postmark integration

All 76 billing tests pass.
@InfinityBowman InfinityBowman linked an issue Jan 8, 2026 that may be closed by this pull request
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 8, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Introduces modular Stripe webhook handlers for checkout, subscriptions, invoices, payment intents, and customer events with deduplication and ledger tracking. Enhances admin database viewer with schema-aware filtering, foreign key navigation, and expanded API responses. Adds comprehensive test coverage and supporting utilities including dunning email infrastructure and subscription status mapping.

Changes

Cohort / File(s) Summary
Planning & Analysis Documents
packages/docs/audits/flowglad-polar-extraction-analysis-2026-01.md, packages/docs/plans/stripe-hardening-plan.md
Comprehensive documentation detailing admin dashboard architecture, Stripe hardening roadmap, webhook handling patterns, dunning workflow, and phased implementation milestones.
Admin UI Layer
packages/web/src/components/admin/AdminLayout.jsx
Removed max-width constraint (max-w-7xl → removed), expanding content area while maintaining centering.
Admin Database Viewer & Query Infrastructure
packages/web/src/components/admin/DatabaseViewer.jsx, packages/web/src/lib/queryKeys.js, packages/web/src/primitives/useAdminQueries.js
Added column-level schema awareness, foreign key navigation, filtering state, and clickable FK cells; extended query key signatures to include filterBy and filterValue parameters throughout the query chain.
Stripe Webhook Routing & Router
packages/workers/src/routes/billing/webhookRouter.js, packages/workers/src/routes/billing/webhooks.js
Refactored webhook processing from monolithic to modular routing; added event deduplication by Stripe event ID, ledger context extraction, and delegated handler-based event dispatch.
Billing Event Handlers
packages/workers/src/routes/billing/handlers/checkoutHandlers.js, packages/workers/src/routes/billing/handlers/subscriptionHandlers.js, packages/workers/src/routes/billing/handlers/invoiceHandlers.js, packages/workers/src/routes/billing/handlers/paymentIntentHandlers.js, packages/workers/src/routes/billing/handlers/customerHandlers.js
Implemented specialized async handlers for Stripe lifecycle events (checkout, subscription state changes, invoicing, payment intents, customer updates); each logs context, performs DB updates, and returns standardized ledgerContext payload.
Billing Utilities & Dunning
packages/workers/src/routes/billing/handlers/subscriptionStatus.js, packages/workers/src/routes/billing/handlers/dunning.js, packages/workers/src/routes/billing/handlers/index.js
Added subscription status mapping (access levels, grace period logic), dunning email queueing with urgency-based templating (HTML/text), and barrel export for consolidated handler imports.
Billing Handler Tests
packages/workers/src/routes/billing/__tests__/subscriptionHandlers.test.js, packages/workers/src/routes/billing/__tests__/invoiceHandlers.test.js, packages/workers/src/routes/billing/__tests__/paymentIntentHandlers.test.js, packages/workers/src/routes/billing/__tests__/customerHandlers.test.js
Comprehensive test suites covering subscription creation/updates/deletion/pause/resume, invoice payment success/failure/finalization, payment intent lifecycle, and customer sync scenarios with database state validation.
Admin Database Routes
packages/workers/src/routes/admin/database.js
Expanded schema endpoint to include unique, hasDefault fields and optional foreignKey object with table/column references; added safe FK resolution with reference validation.
Test Cleanup & Configuration
packages/workers/src/routes/__tests__/org-auth.test.js, packages/workers/drizzle.config.ts, packages/workers/package.json, pnpm-workspace.yaml
Removed legacy HTTP 410 endpoint tests; added SQLite database credentials to Drizzle config, drizzle-kit studio script, better-sqlite3 devDependency, and workspace build configuration.

Sequence Diagrams

sequenceDiagram
    participant Client as Stripe
    participant Router as WebhookRouter
    participant Handler as Event Handler
    participant DB as Database
    participant Logger as Logger
    
    Client->>Router: POST webhook event
    Router->>Router: Verify signature
    Router->>DB: Check dedupe by eventId
    alt Event already processed
        DB-->>Router: Ledger exists
        Router-->>Client: 200 SKIPPED_DUPLICATE
    else New event
        Router->>Handler: routeStripeEvent(event, ctx)
        Handler->>DB: Query/Update subscription/invoice
        Handler->>Logger: Log event action
        DB-->>Handler: Operation result
        Logger-->>Handler: Acknowledged
        Handler-->>Router: {handled, result, ledgerContext}
        Router->>DB: Insert/update stripe_event_ledger
        Router-->>Client: 200 webhook_processed
    end
Loading
sequenceDiagram
    participant Invoice as Invoice Handler
    participant DB as Database
    participant User as User Lookup
    participant Queue as EmailQueue DO
    participant Email as Email Service
    
    Invoice->>DB: Find subscription by ID
    alt Payment failed
        DB-->>Invoice: subscription found, past_due
        Invoice->>User: Resolve user by customerId
        User-->>Invoice: user found
        Invoice->>Queue: queueDunningEmail(params, ctx)
        Queue->>Queue: Select template by attemptCount
        Queue->>Queue: Build HTML + text bodies
        Queue->>Email: POST enqueue payload
        Email-->>Queue: success response
        Queue-->>Invoice: result.success
        Invoice->>Logger: Log dunning_email_queued
    end
Loading
sequenceDiagram
    participant UI as DatabaseViewer UI
    participant Query as useAdminTableRows Hook
    participant API as Admin Database Route
    participant Schema as Schema Cache
    participant DB as Database
    
    UI->>UI: User clicks FK cell
    UI->>UI: navigateToForeignKey(table, column)
    UI->>Query: Call hook with filterBy/filterValue
    Query->>API: GET /admin/tables/{table}/rows?filterBy=column&filterValue=x
    API->>Schema: useAdminTableSchema(table)
    Schema->>DB: Fetch table schema with FK metadata
    DB-->>Schema: Schema + foreignKey references
    Schema-->>API: columnSchemaMap
    API->>DB: Query rows filtered by column=value
    DB-->>API: Filtered result set
    API-->>Query: Rows + schema metadata
    Query-->>UI: Update state + re-render
    UI->>UI: Display filtered rows with FK indicators
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • 238 ai agent readiness #239: Modifies the billing/webhook/checkout surface in workers package with modular Stripe webhook routing and billing handler architecture.
  • add a database viewer #230: Extends admin database viewer with schema awareness and filter parameters, building directly on the same DatabaseViewer, queryKeys, and admin route functionality.
  • 212 stripe better auth #214: Overlapping Stripe/billing changes including stripe_event_ledger schema, webhook/ledger handling, and billing handler/route implementations.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title '252 stripe hardening' refers to the issue number and general topic but lacks clarity. A reader scanning git history would not understand what was specifically changed—it could refer to multiple initiatives (webhooks, handlers, infrastructure, etc.) without conveying the primary change. Consider a more descriptive title such as 'Add Stripe webhook handlers and payment event processing' or 'Implement Stripe subscription and payment intent event handlers' to clarify the main implementation focus.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 89.66% which is sufficient. The required threshold is 80.00%.

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


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

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

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Jan 8, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
corates 480b66c Commit Preview URL Jan 08 2026, 08:58 PM

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In @packages/docs/audits/flowglad-polar-extraction-analysis-2026-01.md:
- Around line 446-451: Remove the duplicate markdown heading by deleting the
second occurrence of "## 2. Stripe Implementation Hardening" (the duplicate
heading text) so only a single "## 2. Stripe Implementation Hardening" remains
in the document; ensure surrounding content and section numbering remain intact
after removal.

In @packages/workers/src/routes/billing/__tests__/invoiceHandlers.test.js:
- Around line 30-45: Replace the local seedSubscription helper in
invoiceHandlers.test.js with the shared seedSubscription helper from the test
helpers module; remove the ad-hoc function and import seedSubscription from the
shared helpers used across tests, and update all test calls to pass the helper's
expected schema (use Unix-second timestamps for createdAt/updatedAt, include
stripeSubscriptionId, stripeCustomerId, referenceId, status, etc.) so the Zod
validation in the shared helper accepts the fixtures (e.g., compute nowSec =
Math.floor(Date.now()/1000) and pass createdAt: nowSec, updatedAt: nowSec).

In @packages/workers/src/routes/billing/handlers/dunning.js:
- Around line 126-179: The HTML email in buildDunningEmailHtml interpolates
userName, amount, and invoiceUrl directly which can lead to XSS; add an
escapeHtml helper that replaces &, <, >, and " (and returns empty string for
null/undefined) and use it when inserting userName and amount into the template
(e.g., ${escapeHtml(userName || 'there')} and ${escapeHtml(String(amount))});
also sanitize invoiceUrl for the href by escaping it and optionally
validating/normalizing it to an allowed protocol (e.g., only http/https) before
embedding to avoid javascript: URLs.

In @packages/workers/src/routes/billing/handlers/invoiceHandlers.js:
- Around line 95-96: The function handleInvoicePaymentFailed destructures _env
from ctx but later uses ctx.env, creating an inconsistent unused variable;
change the destructuring to use env (const { db, logger, env } = ctx) or stop
destructuring and consistently use ctx.env, and update any references inside
handleInvoicePaymentFailed to use the chosen form so _env is not left unused and
env access is consistent.
🧹 Nitpick comments (14)
packages/workers/src/routes/admin/database.js (1)

143-173: LGTM! Safe FK detection with appropriate fallback.

The foreign key detection logic correctly:

  • Extracts FK references from Drizzle column config
  • Validates referenced tables against ALLOWED_TABLES to prevent exposure
  • Uses try-catch to gracefully skip FK info on resolution errors

The implementation safely handles edge cases without breaking the endpoint response.

💡 Optional: Add debug logging for FK resolution failures

Consider logging FK resolution errors in development to aid schema debugging:

       try {
         const refColumn = refFn();
         if (refColumn?.table) {
           const refTableName = Object.entries(dbSchema).find(([, t]) => t === refColumn.table)?.[0];
           if (refTableName && ALLOWED_TABLES.includes(refTableName)) {
             columnInfo.foreignKey = {
               table: refTableName,
               column: refColumn.name,
             };
           }
         }
-      } catch {
+      } catch (err) {
+        // FK resolution failed, skip FK info
+        console.debug(`Failed to resolve FK for ${name}:`, err.message);
-        // Reference function failed, skip FK info
       }
packages/workers/src/routes/billing/__tests__/subscriptionHandlers.test.js (1)

31-46: Consider extracting seedSubscription to shared test helpers.

The local seedSubscription helper works correctly, but based on the coding guidelines, test data seeding should use provided seed* helpers from packages/workers/src/__tests__/helpers.js for consistency across test files. If seedSubscription doesn't exist in helpers.js, consider adding it there to enable reuse in other billing-related tests.

packages/workers/src/routes/billing/handlers/checkoutHandlers.js (2)

116-118: setMonth() edge case may cause unexpected date calculations.

JavaScript's setMonth() can produce unexpected results at month boundaries. For example, adding 6 months to August 31 results in February 31, which rolls over to March 2 or 3.

Consider using a library like date-fns or calculating with milliseconds for predictable results:

♻️ Safer date calculation
-    const newExpiresAt = new Date(baseExpiresAt * 1000);
-    newExpiresAt.setMonth(newExpiresAt.getMonth() + 6);
+    // Add 6 months worth of days (approximately 182 days) for predictable behavior
+    const SIX_MONTHS_MS = 182 * 24 * 60 * 60 * 1000;
+    const newExpiresAt = new Date(baseExpiresAt * 1000 + SIX_MONTHS_MS);

154-158: Consider handling payment_intent as potentially an object.

Similar to how session.customer is handled (lines 63-64), session.payment_intent may be either a string ID or an expanded object depending on Stripe API settings.

♻️ Consistent ID extraction
       metadata: {
         purchaserUserId,
         stripeCheckoutSessionId: session.id,
-        stripePaymentIntentId: session.payment_intent,
+        stripePaymentIntentId:
+          typeof session.payment_intent === 'string'
+            ? session.payment_intent
+            : session.payment_intent?.id,
       },
packages/workers/src/routes/billing/handlers/customerHandlers.js (1)

68-82: Consider wrapping database operations in try-catch.

Per coding guidelines, database operations should be wrapped in try-catch blocks and return domain errors using createDomainError with SYSTEM_ERRORS.DB_ERROR. If error handling is centralized in the webhook processor, this may be acceptable, but explicit error handling here would make the handlers more robust.

♻️ Add error handling example
+import { createDomainError, SYSTEM_ERRORS } from '@corates/shared';

   if (hasChanges) {
+    try {
       await db
         .update(user)
         .set({
           ...updates,
           updatedAt: new Date(),
         })
         .where(eq(user.id, existingUser.id));
+    } catch (error) {
+      logger.error('customer_update_db_error', { userId: existingUser.id, error: error.message });
+      return {
+        handled: false,
+        result: 'db_error',
+        error: createDomainError(SYSTEM_ERRORS.DB_ERROR, { cause: error }),
+      };
+    }
packages/workers/src/routes/billing/webhooks.js (2)

257-264: Verify ledger status mapping for unhandled events.

When handlerResult.handled is false, the status is set to PROCESSED (Line 258). This seems intentional for "event_type_not_handled" cases, but consider whether IGNORED_UNHANDLED or similar would better distinguish truly processed events from those the system doesn't handle.


288-297: Handler errors mapped to VALIDATION_ERRORS.INVALID_INPUT may be misleading.

When a handler returns an error (e.g., database failure), mapping it to VALIDATION_ERRORS.INVALID_INPUT doesn't accurately represent the error type. Consider using SYSTEM_ERRORS.INTERNAL_ERROR or introducing handler-specific error mapping.

Suggested approach
     // Return error response if handler indicated failure
     if (handlerResult.error) {
-      // Map handler errors to domain errors for consistent API responses
-      const errorDetails = {
-        field: handlerResult.result,
-        value: handlerResult.error,
-      };
-      const domainError = createDomainError(VALIDATION_ERRORS.INVALID_INPUT, errorDetails);
+      // Handler failures are typically internal/system errors, not validation errors
+      const domainError = createDomainError(SYSTEM_ERRORS.INTERNAL_ERROR, {
+        operation: handlerResult.result,
+        originalError: handlerResult.error,
+      });
       return c.json(domainError, domainError.statusCode);
     }
packages/workers/src/routes/billing/handlers/dunning.js (2)

23-43: Validate additional required parameters.

The function only validates userEmail, but invoiceUrl is critical for the email to be useful. If invoiceUrl is missing, the email will have an empty/broken link.

Suggested validation
   if (!userEmail) {
     logger.stripe('dunning_email_skipped_no_email', {
       subscriptionId,
       orgId,
       attemptCount,
     });
     return false;
   }
+
+  if (!invoiceUrl) {
+    logger.stripe('dunning_email_skipped_no_invoice_url', {
+      subscriptionId,
+      orgId,
+      attemptCount,
+    });
+    return false;
+  }

88-120: Consider adding a timeout to the DO fetch call.

The EmailQueue DO fetch has no timeout, which could cause the webhook handler to hang if the DO is unresponsive. Cloudflare Workers have execution time limits, so a long-running fetch could cause the entire webhook to fail.

Suggested approach with AbortController
   try {
     // Queue via EmailQueue DO
     const queueId = env.EMAIL_QUEUE.idFromName('default');
     const queue = env.EMAIL_QUEUE.get(queueId);

+    const controller = new AbortController();
+    const timeoutId = setTimeout(() => controller.abort(), 5000);
+
     const response = await queue.fetch(
       new Request('https://internal/enqueue', {
         method: 'POST',
         headers: { 'Content-Type': 'application/json' },
         body: JSON.stringify(emailPayload),
+        signal: controller.signal,
       }),
     );
+
+    clearTimeout(timeoutId);
packages/workers/src/routes/billing/handlers/subscriptionStatus.js (1)

107-109: Consider whether past_due should be in isActiveSubscription.

Including past_due in isActiveSubscription is a business decision - technically the subscription is still billable but payment has failed. The name might imply only "healthy" states. If intentional, a brief comment would clarify.

Optional clarifying comment
 /**
  * Check if a subscription status indicates active billing
+ * Note: Includes past_due since subscription is still active (just with failed payment)
  * @param {string} status - Stripe subscription status
  * @returns {boolean}
  */
 export function isActiveSubscription(status) {
   return ['active', 'trialing', 'past_due'].includes(status);
 }
packages/workers/src/routes/billing/handlers/subscriptionHandlers.js (4)

234-240: Missing logging when subscription not found in pause handler.

Unlike handleSubscriptionDeleted (Line 181-183) which logs when subscription isn't found, handleSubscriptionPaused silently returns. For consistency and debugging, add logging.

Suggested fix
   if (!existing) {
+    logger.stripe('subscription_not_found_for_pause', {
+      stripeSubscriptionId: sub.id,
+    });
     return {
       handled: true,
       result: 'subscription_not_found',
       ledgerContext: { stripeSubscriptionId: sub.id },
     };
   }

281-287: Missing logging when subscription not found in resume handler.

Same issue as the pause handler - add logging for consistency.

Suggested fix
   if (!existing) {
+    logger.stripe('subscription_not_found_for_resume', {
+      stripeSubscriptionId: sub.id,
+    });
     return {
       handled: true,
       result: 'subscription_not_found',
       ledgerContext: { stripeSubscriptionId: sub.id },
     };
   }

256-263: Inconsistent ledgerContext: missing stripeCustomerId in pause handler.

Other handlers include stripeCustomerId in the ledgerContext (see Lines 86, 159, 213), but handleSubscriptionPaused omits it. For consistency and traceability, include it.

Suggested fix
   return {
     handled: true,
     result: 'paused',
     ledgerContext: {
       stripeSubscriptionId: sub.id,
+      stripeCustomerId: typeof sub.customer === 'string' ? sub.customer : sub.customer?.id,
       orgId: existing.referenceId,
     },
   };

306-313: Inconsistent ledgerContext: missing stripeCustomerId in resume handler.

Same issue as the pause handler - add stripeCustomerId for consistency.

Suggested fix
   return {
     handled: true,
     result: 'resumed',
     ledgerContext: {
       stripeSubscriptionId: sub.id,
+      stripeCustomerId: typeof sub.customer === 'string' ? sub.customer : sub.customer?.id,
       orgId: existing.referenceId,
     },
   };

Comment on lines +446 to +451
## 2. Stripe Implementation Hardening

---

## 2. Stripe Implementation Hardening

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Duplicate heading detected.

Lines 446-451 contain a duplicate "## 2. Stripe Implementation Hardening" heading. Remove the duplicate at line 450 to fix the markdown structure.

📝 Suggested fix
 ---

 ## 2. Stripe Implementation Hardening

----
-
-## 2. Stripe Implementation Hardening

 ### 2.1 Current CoRATES Implementation Analysis
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## 2. Stripe Implementation Hardening
---
## 2. Stripe Implementation Hardening
## 2. Stripe Implementation Hardening
---
### 2.1 Current CoRATES Implementation Analysis
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

450-450: Multiple headings with the same content

(MD024, no-duplicate-heading)

🤖 Prompt for AI Agents
In @packages/docs/audits/flowglad-polar-extraction-analysis-2026-01.md around
lines 446 - 451, Remove the duplicate markdown heading by deleting the second
occurrence of "## 2. Stripe Implementation Hardening" (the duplicate heading
text) so only a single "## 2. Stripe Implementation Hardening" remains in the
document; ensure surrounding content and section numbering remain intact after
removal.

Comment on lines +30 to +45
// Helper to seed a subscription
async function seedSubscription(db, data) {
await db.insert(subscription).values({
id: data.id,
plan: data.plan || 'team',
referenceId: data.referenceId || 'org-1',
stripeCustomerId: data.stripeCustomerId || 'cus_123',
stripeSubscriptionId: data.stripeSubscriptionId,
status: data.status || 'active',
periodStart: data.periodStart || new Date(),
periodEnd: data.periodEnd || new Date(Date.now() + 30 * 24 * 60 * 60 * 1000),
cancelAtPeriodEnd: data.cancelAtPeriodEnd || false,
createdAt: new Date(),
updatedAt: new Date(),
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use shared seedSubscription helper instead of local implementation.

Per coding guidelines, tests should "Reuse provided helpers from packages/workers/src/__tests__/helpers.js... instead of implementing ad-hoc solutions." The shared seedSubscription helper (shown in relevant_code_snippets) includes Zod schema validation and consistent date handling.

♻️ Use shared helper
-import { resetTestDatabase, seedUser } from '@/__tests__/helpers.js';
+import { resetTestDatabase, seedUser, seedSubscription } from '@/__tests__/helpers.js';

-// Helper to seed a subscription
-async function seedSubscription(db, data) {
-  await db.insert(subscription).values({
-    id: data.id,
-    plan: data.plan || 'team',
-    referenceId: data.referenceId || 'org-1',
-    stripeCustomerId: data.stripeCustomerId || 'cus_123',
-    stripeSubscriptionId: data.stripeSubscriptionId,
-    status: data.status || 'active',
-    periodStart: data.periodStart || new Date(),
-    periodEnd: data.periodEnd || new Date(Date.now() + 30 * 24 * 60 * 60 * 1000),
-    cancelAtPeriodEnd: data.cancelAtPeriodEnd || false,
-    createdAt: new Date(),
-    updatedAt: new Date(),
-  });
-}

Then update calls to use the helper's expected schema (with Unix timestamps):

const nowSec = Math.floor(Date.now() / 1000);
await seedSubscription({
  id: 'sub-local-1',
  stripeSubscriptionId: 'sub_123',
  status: 'past_due',
  referenceId: 'org-1',
  stripeCustomerId: 'cus_123',
  createdAt: nowSec,
  updatedAt: nowSec,
});

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In @packages/workers/src/routes/billing/__tests__/invoiceHandlers.test.js around
lines 30 - 45, Replace the local seedSubscription helper in
invoiceHandlers.test.js with the shared seedSubscription helper from the test
helpers module; remove the ad-hoc function and import seedSubscription from the
shared helpers used across tests, and update all test calls to pass the helper's
expected schema (use Unix-second timestamps for createdAt/updatedAt, include
stripeSubscriptionId, stripeCustomerId, referenceId, status, etc.) so the Zod
validation in the shared helper accepts the fixtures (e.g., compute nowSec =
Math.floor(Date.now()/1000) and pass createdAt: nowSec, updatedAt: nowSec).

Comment on lines +126 to +179
function buildDunningEmailHtml({ userName, amount, invoiceUrl, attemptCount, urgency }) {
const urgencyColors = {
low: '#f59e0b', // amber
medium: '#f97316', // orange
high: '#ef4444', // red
};

const color = urgencyColors[urgency] || urgencyColors.medium;

const finalNotice =
attemptCount >= 3 ?
`
<div style="background: #fef2f2; border: 1px solid #fecaca; border-radius: 8px; padding: 16px; margin: 20px 0;">
<p style="margin: 0; color: #991b1b;"><strong>Final Notice:</strong> Your subscription will be canceled if payment is not received.</p>
</div>
`
: '';

return `
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
</head>
<body style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; line-height: 1.6; color: #1f2937; max-width: 600px; margin: 0 auto; padding: 20px;">
<div style="border-left: 4px solid ${color}; padding-left: 16px; margin-bottom: 24px;">
<h1 style="color: ${color}; margin: 0 0 8px 0; font-size: 20px;">Payment Failed</h1>
<p style="margin: 0; color: #6b7280;">Attempt ${attemptCount} of 3</p>
</div>

<p>Hi ${userName || 'there'},</p>

<p>We were unable to process your payment of <strong>${amount}</strong> for your CoRATES subscription.</p>

${finalNotice}

<p>Please update your payment method to continue your subscription:</p>

<a href="${invoiceUrl}" style="display: inline-block; background: #2563eb; color: white; padding: 12px 24px; border-radius: 6px; text-decoration: none; font-weight: 500; margin: 16px 0;">Update Payment Method</a>

<p style="color: #6b7280; font-size: 14px; margin-top: 32px;">
If you have any questions, please contact our support team.
</p>

<hr style="border: none; border-top: 1px solid #e5e7eb; margin: 32px 0;">

<p style="color: #9ca3af; font-size: 12px;">
CoRATES - Collaborative Research Appraisal Tool for Evidence Synthesis
</p>
</body>
</html>
`.trim();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential XSS in HTML email template.

The userName, amount, and invoiceUrl values are interpolated directly into HTML without escaping. While email clients typically sandbox content, some may render JavaScript in certain contexts. Consider escaping HTML entities.

Suggested helper
function escapeHtml(str) {
  if (!str) return '';
  return str
    .replace(/&/g, '&amp;')
    .replace(/</g, '&lt;')
    .replace(/>/g, '&gt;')
    .replace(/"/g, '&quot;');
}

// Then use: ${escapeHtml(userName || 'there')}
🤖 Prompt for AI Agents
In @packages/workers/src/routes/billing/handlers/dunning.js around lines 126 -
179, The HTML email in buildDunningEmailHtml interpolates userName, amount, and
invoiceUrl directly which can lead to XSS; add an escapeHtml helper that
replaces &, <, >, and " (and returns empty string for null/undefined) and use it
when inserting userName and amount into the template (e.g.,
${escapeHtml(userName || 'there')} and ${escapeHtml(String(amount))}); also
sanitize invoiceUrl for the href by escaping it and optionally
validating/normalizing it to an allowed protocol (e.g., only http/https) before
embedding to avoid javascript: URLs.

Comment on lines +95 to +96
export async function handleInvoicePaymentFailed(invoice, ctx) {
const { db, logger, _env } = ctx;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Typo: _env destructured but ctx.env used.

Line 96 destructures _env (unused variable pattern), but Line 167 accesses ctx.env. This works but is inconsistent. Either use env consistently or access via ctx.env throughout.

Suggested fix
 export async function handleInvoicePaymentFailed(invoice, ctx) {
-  const { db, logger, _env } = ctx;
+  const { db, logger, env } = ctx;

Then update Line 167:

-  if (stripeCustomerId && ctx.env?.EMAIL_QUEUE) {
+  if (stripeCustomerId && env?.EMAIL_QUEUE) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function handleInvoicePaymentFailed(invoice, ctx) {
const { db, logger, _env } = ctx;
export async function handleInvoicePaymentFailed(invoice, ctx) {
const { db, logger, env } = ctx;
// ... rest of function body ...
// At line 167, the following change would also be applied:
if (stripeCustomerId && env?.EMAIL_QUEUE) {
// ... email queue logic ...
}
🤖 Prompt for AI Agents
In @packages/workers/src/routes/billing/handlers/invoiceHandlers.js around lines
95 - 96, The function handleInvoicePaymentFailed destructures _env from ctx but
later uses ctx.env, creating an inconsistent unused variable; change the
destructuring to use env (const { db, logger, env } = ctx) or stop destructuring
and consistently use ctx.env, and update any references inside
handleInvoicePaymentFailed to use the chosen form so _env is not left unused and
env access is consistent.

Comment on lines +144 to +148
// Extract failure details
const failureReason =
invoice.last_finalization_error?.message ||
invoice.status_transitions?.finalized_at ||
'unknown';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unclear failure reason extraction logic.

The fallback chain uses status_transitions?.finalized_at as a failure reason, which is a timestamp, not a reason string. This could produce confusing log entries.

Suggested fix
   // Extract failure details
   const failureReason =
     invoice.last_finalization_error?.message ||
-    invoice.status_transitions?.finalized_at ||
+    invoice.last_payment_error?.message ||
     'unknown';

@InfinityBowman InfinityBowman merged commit cfe194e into main Jan 8, 2026
4 checks passed
@InfinityBowman InfinityBowman deleted the 252-stripe-hardening branch January 8, 2026 21:21
@coderabbitai coderabbitai Bot mentioned this pull request Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stripe hardening

2 participants