Skip to content

feat: Dodo Payments integration + entitlement engine & webhook pipeline#2024

Open
SebastienMelki wants to merge 75 commits intomainfrom
dodo_payments
Open

feat: Dodo Payments integration + entitlement engine & webhook pipeline#2024
SebastienMelki wants to merge 75 commits intomainfrom
dodo_payments

Conversation

@SebastienMelki
Copy link
Copy Markdown
Collaborator

@SebastienMelki SebastienMelki commented Mar 21, 2026

Summary

Integrates Dodo Payments as the billing provider for WorldMonitor, covering the foundation layer (Phases 14-16). This adds a full subscription-to-entitlement pipeline: webhook ingestion, idempotent event processing, config-driven feature flags, API gateway enforcement, and frontend panel gating.

What's done (Phases 14-16)

Phase 14 — Foundation & Schema

  • Installed @dodopayments/convex component and registered it in convex.config.ts
  • Extended Convex schema with 6 payment tables (subscriptions, webhookEvents, entitlements, productPlanMappings, customers, invoices)
  • Auth stub (convex/lib/auth.ts) + env helper (convex/lib/env.ts)
  • Seed mutation with real Dodo product IDs for product-to-plan mappings

Phase 15 — Webhook Pipeline

  • HTTP endpoint at /dodo/webhook with HMAC-SHA256 signature verification
  • Idempotent event processor — deduplicates by eventId, dispatches to typed handlers
  • Subscription lifecycle handlers: subscription.created, subscription.updated, subscription.cancelled, subscription.expired and more
  • Entitlement upsert on every subscription state change (config-driven via PLAN_FEATURES map)
  • 10 contract tests for the webhook processing pipeline

Phase 16 — Entitlement Engine

  • Config-driven PLAN_FEATURES map with tier levels per feature flag
  • Convex query entitlements:getForUser for reactive entitlement lookups
  • Redis cache sync action for low-latency gateway checks
  • API gateway middleware (server/_shared/entitlement-check.ts) — enforces entitlements on protected routes with Redis fast-path + Convex fallback
  • Frontend entitlement service (src/services/entitlements.ts) — reactive ConvexClient subscription, panel gating in panel-layout.ts
  • 6 gateway enforcement unit tests + 6 Convex entitlement contract tests

What's left (Phases 17-18)

  • Phase 17: Checkout flow & pricing UI — Dodo checkout session creation, pricing page component, plan selection UX
  • Phase 18: Billing portal & plan management — self-serve upgrade/downgrade, invoice history, cancellation flow

Architecture

Browser ──► API Gateway (entitlement-check middleware)
              │                          │
              │ Redis cache (fast path)  │ Convex fallback
              ▼                          ▼
         Redis ◄──── cacheActions ◄──── Convex entitlements table
                                              ▲
                                              │ upsertEntitlements
                                              │
         Dodo webhook ──► HTTP endpoint ──► webhookMutations
                          (sig verify)       (idempotent dispatch)
                                              │
                                              ▼
                                        subscriptionHelpers
                                        (lifecycle handlers)

Notes

  • Auth is currently stubbed (resolveUserId in convex/lib/auth.ts) pending Auth integration from PR feat(auth): integrate clerk.dev  #1812
  • PLAN_FEATURES config is the single source of truth for what each plan unlocks — adding a new feature flag is a one-line change
  • All webhook processing is idempotent (dedup by eventId + status guards)

Files changed

26 files, ~2700 lines added across convex/, server/, and src/.

@koala73 — Would appreciate a look at the schema design and the entitlement config map shape. Happy to walk through the webhook flow if helpful.


🤖 Generated with Claude Code

SebastienMelki and others added 21 commits March 21, 2026 17:12
- Install @dodopayments/[email protected] with peer deps satisfied
- Create convex/convex.config.ts with defineApp() and dodopayments component
- Add TODO for betterAuth registration when PR #1812 merges

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Add subscriptions table with status enum, indexes, and raw payload
- Add entitlements table (one record per user) with features blob
- Add customers table keyed by userId with optional dodoCustomerId
- Add webhookEvents table for full audit trail (retained forever)
- Add paymentEvents table for billing history (charge/refund)
- Add productPlans table for product-to-plan mapping in DB
- All existing tables (registrations, contactMessages, counters) unchanged

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Create convex/lib/auth.ts with resolveUserId (returns test-user-001 in dev)
  and requireUserId (throws on unauthenticated) as sole auth entry points
- Create convex/lib/env.ts with requireEnv for runtime env var validation
- Append DODO_API_KEY, DODO_WEBHOOK_SECRET, DODO_PAYMENTS_WEBHOOK_SECRET,
  and DODO_BUSINESS_ID to .env.example with setup instructions
- Document dual webhook secret naming (library vs app convention)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Idempotent upsert mutation for 5 Dodo product-to-plan mappings
- Placeholder product IDs to be replaced after Dodo dashboard setup
- listProductPlans query for verification and downstream use

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Define PlanFeatures type with 5 feature dimensions
- Add PLAN_FEATURES config for 6 tiers (free through enterprise)
- Export getFeaturesForPlan helper with free-tier fallback
- Export FREE_FEATURES constant for default entitlements

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Custom httpAction verifying Dodo webhook signatures via @dodopayments/core
- Returns 400 for missing headers, 401 for invalid signature, 500 for processing errors
- HTTP router at /dodopayments-webhook dispatches POST to webhook handler
- Synchronous processing before 200 response (within Dodo 15s timeout)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Add upsertEntitlements helper (creates/updates per userId, no duplicates)
- Add isNewerEvent guard for out-of-order webhook rejection
- Add handleSubscriptionActive (creates subscription + entitlements)
- Add handleSubscriptionRenewed (extends period + entitlements)
- Add handleSubscriptionOnHold (pauses without revoking entitlements)
- Add handleSubscriptionCancelled (preserves entitlements until period end)
- Add handleSubscriptionPlanChanged (updates plan + recomputes entitlements)
- Add handlePaymentEvent (records charge events for succeeded/failed)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…leton

- processWebhookEvent internalMutation with idempotency via by_webhookId index
- Switch dispatch for 7 event types: 5 subscription + 2 payment events
- Stub handlers log TODO for each event type (to be implemented in Plan 03)
- Error handling marks failed events and re-throws for HTTP 500 + Dodo retry

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Update auto-generated api.d.ts with new payment module types
- SUMMARY, STATE, and ROADMAP updated (.planning/ gitignored)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Replace 6 stub handler functions with imports from subscriptionHelpers
- All 7 event types (5 subscription + 2 payment) dispatch to real handlers
- Error handling preserves failed event status in webhookEvents table
- Complete end-to-end pipeline: HTTP action -> mutation -> handler functions

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…e vitest

- Add convex-test, vitest, @edge-runtime/vm as dev dependencies
- Create vitest.config.mts scoped to convex/__tests__/ with edge-runtime environment
- Add test:convex and test:convex:watch npm scripts
- Test all 5 subscription lifecycle events (active, renewed, on_hold, cancelled, plan_changed)
- Test both payment events (succeeded, failed)
- Test deduplication by webhook-id (same id processed only once)
- Test out-of-order event rejection (older timestamp skipped)
- Test subscription reactivation (cancelled -> active on same subscription_id)
- Verify entitlements created/updated with correct plan features
convex-test uses Vite-specific import.meta.glob and has generic type
mismatches with tsc. Tests run correctly via vitest; excluding from
convex typecheck avoids false positives.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…query

- Add tier: number to PlanFeatures type (0=free, 1=pro, 2=api, 3=enterprise)
- Add tier values to all plan entries in PLAN_FEATURES config
- Create convex/entitlements.ts with getEntitlementsForUser public query
- Free-tier fallback for missing or expired entitlements

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Create convex/payments/cacheActions.ts with syncEntitlementCache internal action
- Wire upsertEntitlements to schedule cache sync via ctx.scheduler.runAfter(0, ...)
- Add deleteRedisKey() to server/_shared/redis.ts for explicit cache invalidation
- Redis keys use raw format (entitlements:{userId}) with 1-hour TTL
- Cache write failures logged but do not break webhook pipeline

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Create entitlement-check middleware with Redis cache + Convex fallback
- Replace PREMIUM_RPC_PATHS boolean Set with ENDPOINT_ENTITLEMENTS tier map
- Wire checkEntitlement into gateway between API key and rate limiting
- Add raw parameter to setCachedJson for user-scoped entitlement keys
- Fail-open on missing auth/cache failures for graceful degradation

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…Client subscription

- Add VITE_CONVEX_URL to .env.example for frontend Convex access
- Create src/services/entitlements.ts with lazy-loaded ConvexClient
- Export initEntitlementSubscription, onEntitlementChange, getEntitlementState, hasFeature, hasTier, isEntitled
- ConvexClient only loaded when userId available and VITE_CONVEX_URL configured
- Graceful degradation: log warning and skip when Convex unavailable

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Add shouldUnlockPremium() helper that checks entitlements first, falls back to legacy isProUser()
- Boot ConvexClient entitlement subscription in PanelLayoutManager constructor
- Replace all direct isProUser()/getSecretState() gating with shouldUnlockPremium()
- Add reactive onEntitlementChange listener for real-time entitlement detection
- Panels degrade to current behavior when Convex is unavailable

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Free-tier defaults for unknown userId
- Active entitlements for subscribed user
- Free-tier fallback for expired entitlements
- Correct tier mapping for api_starter and enterprise plans
- getFeaturesForPlan fallback for unknown plan keys

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- getRequiredTier: gated vs ungated endpoint tier lookup
- checkEntitlement: ungated pass-through, missing userId graceful degradation
- checkEntitlement: 403 for insufficient tier, null for sufficient tier
- Dependency injection pattern (_testCheckEntitlement) for clean testability
- vitest.config.mts include expanded to server/__tests__/

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@SebastienMelki SebastienMelki requested a review from koala73 March 21, 2026 19:11
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
worldmonitor Ready Ready Preview, Comment Mar 27, 2026 6:32pm

Request Review

SebastienMelki and others added 7 commits March 21, 2026 22:06
- DodoPayments component wraps checkout with server-side API key
- Accepts productId, returnUrl, discountCode, referralCode args
- Always enables discount code input (PROMO-01)
- Forwards affiliate referral as checkout metadata (PROMO-02)
- Dark theme customization for checkout overlay

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…ing toggle

- 4 tiers: Free, Pro, API, Enterprise with feature comparison
- Monthly/annual toggle with "Save 17%" badge for Pro
- Checkout buttons using Dodo static payment links
- Pro tier visually highlighted with green border and "Most Popular" badge
- Staggered entrance animations via motion
- Referral code forwarding via refCode prop

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…tlements

- Create src/services/convex-client.ts with getConvexClient() and getConvexApi()
- Lazy-load ConvexClient via dynamic import to preserve bundle size
- Refactor entitlements.ts to use shared client instead of inline creation
- Both checkout and entitlement services will share one WebSocket connection

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
… forwarding

- Import and render PricingSection between EnterpriseShowcase and PricingTable
- Pass refCode from getRefCode() URL param to PricingSection for checkout link forwarding
- Update navbar CTA and TwoPathSplit Pro CTA to anchor to #pricing section
- Keep existing waitlist form in Footer for users not ready to buy
- Build succeeds with no new errors

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@SebastienMelki
Copy link
Copy Markdown
Collaborator Author

Merged main into dodo_payments (ea39e3a)

The Clerk auth integration from PR #1812 is now merged in. Resolved 9 conflicts:

  • CSP: Combined Clerk + Dodo entries across index.html, vercel.json, and tauri.conf.json
  • Gateway: Kept session resolution + entitlement pipeline, added shared PREMIUM_RPC_PATHS import from src/shared/premium-paths.ts
  • Premium access: All checks now use isEntitled() || API key || Clerk Pro consistently (App.ts, data-loader.ts, panel-layout.ts)
  • Panel gating: Adopted main's reactive pattern (updatePanelGating via auth subscription) with isEntitled() incorporated
  • Convex config: Kept dodopayments plugin, removed stale betterAuth TODO
  • Env vars: Both Dodo and Clerk sections preserved

TypeScript compiles clean. Branch is ready for review.

@koala73 Ready for a look when you get a chance!

Copy link
Copy Markdown
Owner

@koala73 koala73 left a comment

Choose a reason for hiding this comment

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

Review: Round 8

Verified: commit ea39e3a4c | tsc --noEmit clean | 26/26 tests pass


P0 → RESOLVED ✓

isTrustedOrigin bypass is gone. forceKey: isTierGated && !sessionUserId is correct. The gateway now properly enforces API key for unauthenticated callers and Clerk JWT for authenticated ones.


P1 — Clerk auth never initializes for new users (bootstrapping deadlock)

File: src/App.ts:776-779, src/App.ts:849

if (isProUser()) {          // ← line 776
  await initAuthState();    // Clerk never loads if this is false
  initAuthAnalytics();
}
// ...
if (isProUser()) this.eventHandlers.setupAuthWidget();  // ← line 849

isProUser() returns false on any fresh browser session where localStorage has no wm-pro-key, wm-widget-key, or active Clerk role. For new users — the exact users Dodo payments is meant to convert — isProUser() is always false. Clerk never loads, the sign-in button never mounts, and users can never authenticate. The guard uses the very thing it is trying to initialize as its precondition.

This is a functional blocker for the feature: paying users cannot create a Clerk session to link their subscription.

Fix: Remove the isProUser() guards around initAuthState() and setupAuthWidget(). Gate only on !isDesktopRuntime() if Clerk should not load in the desktop app.

(Tracked in TODO 034)


P1 — getUserId() always returns anon UUID — Clerk user ID never used

File: src/services/user-identity.ts:59-61

// 1. Clerk auth — when the Clerk branch merges, this becomes:
//    const clerk = window.Clerk;
//    if (clerk?.user?.id) return clerk.user.id;

PR #1812 has merged. src/services/clerk.ts is now in main and provides getClerk() and getCurrentClerkUser(). The stub was never filled in.

Consequence: all calls to getUserId() — including startCheckout(), initSubscriptionWatch(), and initEntitlementSubscription() — receive an anon UUID even when the user is signed into Clerk. Every Dodo purchase is keyed to the anon UUID, not the Clerk ID. The entitlement cache is also keyed to the anon UUID. The gateway's checkEntitlement looks up by Clerk JWT sub — those two IDs never match. claimSubscription can never be triggered because nothing detects the anon → Clerk transition.

Fix:

import { getClerk } from './clerk';

export function getUserId(): string | null {
  // 1. Clerk auth
  const clerk = getClerk();
  if (clerk?.user?.id) return clerk.user.id;
  // 2. Legacy wm-pro-key ...
  // 3. Stable anon ID ...
}

With this fix in place, callers at auth time get the Clerk ID, and claimSubscription(anonId) can be wired to fire once on first Clerk sign-in (read localStorage.getItem('wm-anon-id') before it's overwritten).


P1 — jwtVerify missing algorithm allowlist — potential alg:none bypass

File: server/_shared/auth-session.ts:43-46

const { payload } = await jwtVerify(token, jwks, {
  issuer: issuerDomain,
  // ← no algorithms field
});

jose defaults to accepting whatever algorithm the token header declares. A crafted token with alg: none or alg: HS256 could bypass signature verification on some jose versions or after a future library update. Clerk issues RS256. Enforce it explicitly.

One-line fix:

const { payload } = await jwtVerify(token, jwks, {
  issuer: issuerDomain,
  algorithms: ['RS256'],
});

(Tracked in TODO 035)


Summary

Issue Status
P0 isTrustedOrigin spoofable bypass Resolved ✓
P1 isProUser() deadlock — Clerk never loads for new users Open
P1 getUserId() never returns Clerk user ID — identity bridge incomplete Open
P1 jwtVerify no algorithms allowlist Open
All prior P0/P1/P2 issues (rounds 1–7) Resolved ✓

Three fixes, all small. Issues 1 and 2 are the functional blockers — without them, paying users cannot link their Clerk identity to their subscription.

@koala73
Copy link
Copy Markdown
Owner

koala73 commented Mar 26, 2026

Round 8 update: Issue #1 (isProUser() guard around initAuthState) is by design — noted, withdrawing that finding.

Two remaining blockers stand:

P1 — getUserId() never returns Clerk user ID (src/services/user-identity.ts:59-61)

The Clerk stub is still commented out. getUserId() returns the anon UUID even for signed-in Clerk users. Every Dodo purchase is keyed to the anon UUID. The gateway's checkEntitlement checks by Clerk JWT sub — the two IDs never match, so claimSubscription can never be triggered. Fix:

import { getClerk } from './clerk';
// ...
const clerk = getClerk();
if (clerk?.user?.id) return clerk.user.id;

Read localStorage.getItem('wm-anon-id') before overwriting it, then call claimSubscription(anonId) on first sign-in.

P1 — jwtVerify no algorithms: ['RS256'] (server/_shared/auth-session.ts:43)

jose accepts whatever algorithm the token header declares without an allowlist. One-line fix:

const { payload } = await jwtVerify(token, jwks, {
  issuer: issuerDomain,
  algorithms: ['RS256'],
});

SebastienMelki and others added 2 commits March 26, 2026 14:53
…premium paths

Free bearer token holders with a valid session bypassed PREMIUM_RPC_PATHS
because sessionUserId being set caused forceKey=false, skipping the role
check entirely. Now explicitly checks bearer role after API key gate.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Merge conflicts in data-loader.ts and panel-layout.ts arose from both
branches implementing premium access differently — dodo_payments added
isEntitled() checks inline while main consolidated to hasPremiumAccess()
from panel-gating. Resolution: add isEntitled() to the centralized
hasPremiumAccess() in panel-gating.ts and use it everywhere.

Gateway fix: the bearer role check used !keyCheck.valid to detect users
without an API key, but keyCheck.valid is true for trusted browser origins
even without a key. Now checks for the actual X-WorldMonitor-Key header.

Also deduplicates jose dependency (^6.0.11 + ^6.2.2 → ^6.2.2).

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@SebastienMelki
Copy link
Copy Markdown
Collaborator Author

Merge conflicts resolved + gateway bug fix

All CI checks now pass (biome, typecheck, unit, gate, Vercel).

What was done

Merge conflicts — Both branches implemented premium access checking differently (dodo_payments added isEntitled() inline, main consolidated to hasPremiumAccess() from panel-gating.ts). Resolved by adding isEntitled() to the centralized hasPremiumAccess() and using it everywhere, removing duplicate local functions (shouldUnlockPremium, local hasPremiumAccess in data-loader).

Gateway bug fix — The bearer role check at L290 used !keyCheck.valid to detect missing API keys, but keyCheck.valid is true for trusted browser origins even without a key — so free-tier bearer tokens on premium endpoints were never rejected. Fixed by checking for the actual X-WorldMonitor-Key header instead.

Cleanup — Deduplicated jose dependency (^6.0.11 + ^6.2.2^6.2.2), removed unused getSecretState import.

Files changed

  • src/services/panel-gating.ts — added isEntitled() to hasPremiumAccess()
  • src/app/data-loader.ts — resolved conflict, removed local hasPremiumAccess function
  • src/app/panel-layout.ts — resolved 3 conflicts, removed shouldUnlockPremium
  • server/gateway.ts — fixed bearer role check condition
  • package.json / package-lock.json — deduplicated jose

Entitlement subscription, subscription watch, checkout overlay, and
payment banners now only initialize for isProUser() — matching the
Clerk auth gate so only wm-pro-key / wm-widget-key holders see it.

Also consolidates inline isEntitled()||getSecretState()||role checks
in App.ts to use the centralized hasPremiumAccess() from panel-gating.

Both gates (Clerk + Dodo) to be removed when ready for all users.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@SebastienMelki
Copy link
Copy Markdown
Collaborator Author

@koala73 Dodo Payments init is now gated behind isProUser() — same gate as Clerk auth.

What's gated:

  • Entitlement subscription (initEntitlementSubscription)
  • Subscription watch (initSubscriptionWatch)
  • Payment failure banner, checkout overlay, checkout return handling
  • Entitlement change listener (the auto-reload on subscription activation)

How it works: Only users with wm-pro-key or wm-widget-key in localStorage (or Clerk role === 'pro') will hit any Dodo Payments code paths. Everyone else gets zero Dodo-related network calls or UI.

Also cleaned up: Replaced scattered isEntitled() || getSecretState(...) || getAuthState().user?.role === 'pro' patterns in App.ts with the centralized hasPremiumAccess() from panel-gating.ts.

When we're ready to ship to all users, we remove both gates (Clerk + Dodo) together.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 27, 2026

Greptile Summary

This PR wires in Dodo Payments as WorldMonitor's billing provider across three layers: a Convex schema extension (6 new tables), a webhook ingestion pipeline with HMAC-SHA256 signature verification and idempotent dispatch, a config-driven PLAN_FEATURES entitlement engine with Redis cache + Convex fallback, and a gateway middleware that enforces tier requirements on the stock-analysis/backtest endpoints. The architecture is well thought-out — out-of-order event rejection, transactional entitlement upserts, fail-open degradation, and a clean claimSubscription anon→auth migration path are all present. Two issues need attention before this is production-ready:\n\n- Entitlement init gated behind legacy isProUser() (src/app/panel-layout.ts): initEntitlementSubscription, initSubscriptionWatch, and onEntitlementChange are all wrapped in if (isProUser()), which checks for a wm-pro-key/wm-widget-key in localStorage or a Clerk role === 'pro' claim. A new subscriber who purchases through Dodo checkout has none of these legacy signals, so the entire entitlement frontend never initialises — isEntitled() stays false and the user cannot access premium panels despite having a valid subscription.\n- Dead retry-detection branch in processWebhookEvent (convex/payments/webhookMutations.ts + convex/schema.ts): The mutation checks for a "previously failed" event to delete-and-retry, but the schema enforces status: v.literal(\"processed\") and records are only written after a successful handler run. Failed transactions roll back without writing any record, so the delete branch is unreachable.\n\nAdditional notes:\n- convex/entitlements.ts: getEntitlementsForUser accepts an arbitrary userId with no auth check — any client can probe any user's plan. Needs a ctx.auth guard once Clerk JWT is wired.\n- server/_shared/auth-session.ts duplicates the JWKS singleton already in server/auth-session.ts — two separate Clerk key-fetch caches per cold start.\n- convex/http.ts registers the webhook at /dodopayments-webhook; the PR description says /dodo/webhook — reconcile with the Dodo dashboard webhook URL before going live.

Confidence Score: 4/5

Safe to merge once the isProUser() entitlement-init gate is removed and the webhook retry schema is clarified; the webhook pipeline and gateway middleware are structurally sound.

Two P1 findings remain: new Dodo subscribers cannot get entitlements loaded due to the legacy isProUser() gate (breaks the primary user path for this feature), and the documented webhook retry logic is dead code. Neither causes data corruption or a security breach, but the first means the feature does not work for its intended audience.

src/app/panel-layout.ts (entitlement init gating), convex/payments/webhookMutations.ts + convex/schema.ts (retry logic vs. schema mismatch), convex/entitlements.ts (unauthenticated userId query)

Important Files Changed

Filename Overview
src/app/panel-layout.ts Adds Dodo billing/entitlement init to the PanelLayoutManager constructor, but gates the entire block behind isProUser() — a legacy check that new Dodo-only subscribers fail, making entitlement init unreachable for the users this feature targets.
convex/payments/webhookMutations.ts Idempotent webhook dispatcher; idempotency for successful events is correct, but the retry previously-failed event branch is dead code because the schema only allows status: processed and failed transactions roll back without writing any record.
convex/payments/subscriptionHelpers.ts Well-structured subscription lifecycle handlers; out-of-order rejection, upsert semantics, and dev-only fallbacks are all properly guarded. Customer upsert and entitlement upsert are atomic within each Convex mutation.
convex/payments/webhookHandlers.ts HMAC-SHA256 signature verification via @dodopayments/core, missing-header 400, invalid-signature 401, internal-error 500 — clean HTTP action with correct error handling.
convex/schema.ts Six new payment tables with appropriate indexes; webhookEvents.status is typed as v.literal(processed) only, which is inconsistent with the retry-logic documentation in webhookMutations.ts.
server/_shared/entitlement-check.ts Redis fast-path + Convex fallback gateway middleware with dependency-injected test seam; fail-open design is intentional for the auth-stub era and well-documented.
server/_shared/auth-session.ts New JWKS-based JWT resolver for the gateway, but duplicates the module-level JWKS singleton already present in the pre-existing server/auth-session.ts, creating two independent Clerk JWKS caches per cold start.
convex/payments/billing.ts Subscription queries, portal session creation, and claimSubscription anon→real-user migration; all DB operations are within a single Convex mutation (transactional), priority-sorted subscription resolution avoids the cancelled-hides-active bug.
convex/entitlements.ts Public query returns free-tier defaults for missing/expired entitlements; accepts arbitrary userId without auth verification — any client can probe any user's plan tier.
convex/http.ts Registers the webhook handler at /dodopayments-webhook, which does not match the /dodo/webhook path referenced throughout the PR description and architecture diagram.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Gateway as Vercel Gateway
    participant Redis
    participant Convex
    participant Dodo as Dodo Payments

    Note over Browser,Dodo: Checkout flow
    Browser->>Convex: createCheckout(productId, userId)
    Convex->>Dodo: POST /checkout (metadata: wm_user_id)
    Dodo-->>Convex: checkout_url
    Convex-->>Browser: checkout_url
    Browser->>Dodo: Complete payment

    Note over Dodo,Convex: Webhook pipeline
    Dodo->>Convex: POST /dodopayments-webhook (HMAC-signed)
    Convex->>Convex: verifyWebhookPayload (HMAC-SHA256)
    Convex->>Convex: processWebhookEvent (idempotency check)
    Convex->>Convex: handleSubscriptionActive
    Convex->>Convex: upsertEntitlements (planKey, validUntil)
    Convex->>Redis: syncEntitlementCache (scheduled action)
    Convex-->>Dodo: 200 OK

    Note over Browser,Redis: Entitlement check (API call)
    Browser->>Gateway: GET /api/market/v1/analyze-stock (Bearer token)
    Gateway->>Gateway: resolveSessionUserId (Clerk JWT)
    Gateway->>Gateway: checkEntitlement(userId, pathname)
    Gateway->>Redis: GET entitlements:{userId}
    alt Cache hit and valid
        Redis-->>Gateway: CachedEntitlements
    else Cache miss
        Gateway->>Convex: getEntitlementsForUser(userId)
        Convex-->>Gateway: EntitlementState
        Gateway->>Redis: SET entitlements:{userId} EX 3600
    end
    alt tier >= requiredTier
        Gateway-->>Browser: 200 proxied response
    else tier < requiredTier
        Gateway-->>Browser: 403 Upgrade required
    end
Loading

Reviews (1): Last reviewed commit: "chore: retrigger CI" | Re-trigger Greptile

Comment on lines +136 to +167

// Dodo Payments: entitlement + billing init gated behind isProUser().
// Same gate as Clerk auth — remove both gates once ready for all users.
if (isProUser()) {
if (handleCheckoutReturn()) {
showCheckoutSuccess();
}

const userId = getUserId();
if (userId) {
initEntitlementSubscription(userId).catch(() => {});
initSubscriptionWatch(userId).catch(() => {});
initPaymentFailureBanner();
}

initCheckoutOverlay(() => showCheckoutSuccess());
}

// Listen for entitlement changes — reload panels to pick up new gating state.
// Skip the initial snapshot to avoid a reload loop for users who already have
// premium via legacy signals (API key / wm-pro-key).
if (isProUser()) {
let skipInitialSnapshot = true;
onEntitlementChange(() => {
if (skipInitialSnapshot) {
skipInitialSnapshot = false;
return;
}
if (isEntitled()) {
console.log('[entitlements] Subscription activated — reloading to unlock panels');
window.location.reload();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Entitlement init unreachable for new Dodo subscribers

The entire Dodo entitlement pipeline — initEntitlementSubscription, initSubscriptionWatch, onEntitlementChange — is gated behind if (isProUser()). isProUser() checks only legacy signals: wm-pro-key/wm-widget-key in localStorage or getAuthState().user?.role === 'pro' (Clerk). A brand-new user who purchases via the Dodo checkout will have none of these signals, so isProUser() returns false and the entire block is skipped.

As a result:

  • initEntitlementSubscription is never called → currentState stays null
  • isEntitled() always returns false for new Dodo subscribers
  • hasPremiumAccess() falls through to the legacy checks and denies access
  • The panel-gating reload (onEntitlementChange) never fires

This means a user who just completed Dodo checkout and has a live subscription.active webhook processed can still not access any premium panels, because the frontend never initializes the entitlement subscription.

The if (isProUser()) guard should be removed from the entitlement init block, or at minimum replaced with a broader condition that also covers unauthenticated/new users (userId != null).

Comment on lines +38 to +52
.withIndex("by_webhookId", (q) => q.eq("webhookId", args.webhookId))
.first();

if (existing) {
if (existing.status === "processed") {
console.warn(`[webhook] Duplicate webhook ${args.webhookId}, already processed — skipping`);
return;
}
// Previously failed — delete the stale record so we can retry cleanly
console.warn(`[webhook] Retrying previously failed webhook ${args.webhookId}`);
await ctx.db.delete(existing._id);
}

// 2. Dispatch to event-type-specific handlers.
// Errors propagate (throw) so Convex rolls back the entire transaction,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 "Previously failed" retry branch is unreachable due to schema constraint

The idempotency check reads:

if (existing.status === "processed") { return; }
// Previously failed — delete the stale record so we can retry cleanly
await ctx.db.delete(existing._id);

However, the webhookEvents schema declares status: v.literal("processed") — only "processed" is a valid value. Records are only inserted after the handler succeeds (at the end of the function). If the handler throws, Convex rolls back the entire transaction and no record is written.

This means:

  1. existing is found → existing.status is always "processed" (the schema enforces it)
  2. The else branch (delete + retry) can never execute

The documented retry contract ("failed events are deleted so the retry can re-process cleanly") is effectively a no-op. If you need to support a genuine "failed" state for observability, consider adding v.literal("failed") to the status union in the schema and writing a failed record in a try/catch.

Comment on lines +27 to +30
export const getEntitlementsForUser = query({
args: { userId: v.string() },
handler: async (ctx, args) => {
const entitlement = await ctx.db
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Unauthenticated entitlement query leaks subscription tier for any user

getEntitlementsForUser is a public query that accepts a userId argument with no auth gate. Any browser client can call it with an arbitrary userId and learn that user's plan tier, validUntil, and feature flags.

export const getEntitlementsForUser = query({
  args: { userId: v.string() },   // ← anyone can supply any userId
  handler: async (ctx, args) => {

The PR notes auth is "stubbed", but entitlement data (current plan, expiry timestamp, feature flags) is sensitive enough to warrant at least a note that this should be auth-gated once Clerk JWT is wired in. Consider returning only the calling user's entitlements (via ctx.auth.getUserIdentity()) and rejecting if the identity doesn't match args.userId, even as an early guard.

Comment on lines +1 to +58
/**
* Gateway-level JWT verification for Clerk bearer tokens.
*
* Extracts and verifies the `Authorization: Bearer <token>` header using
* the JWKS endpoint from CLERK_JWT_ISSUER_DOMAIN. Returns the userId
* (JWT `sub` claim) on success, or null on any failure (fail-open).
*
* Activated by setting CLERK_JWT_ISSUER_DOMAIN env var. When not set,
* all calls return null and the gateway falls back to API-key-only auth.
*/

import { createRemoteJWKSet, jwtVerify } from 'jose';

// Cached JWKS — singleton per cold start, refreshed by jose internally.
let _jwks: ReturnType<typeof createRemoteJWKSet> | null = null;

function getJwks(): ReturnType<typeof createRemoteJWKSet> | null {
if (_jwks) return _jwks;

const issuerDomain = process.env.CLERK_JWT_ISSUER_DOMAIN;
if (!issuerDomain) return null;

const jwksUrl = new URL('/.well-known/jwks.json', issuerDomain);
_jwks = createRemoteJWKSet(jwksUrl);
return _jwks;
}

/**
* Extracts and verifies a bearer token from the request.
* Returns the userId (sub claim) on success, null on any failure.
*
* Fail-open: errors are logged but never thrown.
*/
export async function resolveSessionUserId(request: Request): Promise<string | null> {
try {
const authHeader = request.headers.get('Authorization');
if (!authHeader?.startsWith('Bearer ')) return null;

const token = authHeader.slice(7);
if (!token) return null;

const jwks = getJwks();
if (!jwks) return null; // CLERK_JWT_ISSUER_DOMAIN not configured

const issuerDomain = process.env.CLERK_JWT_ISSUER_DOMAIN!;
const { payload } = await jwtVerify(token, jwks, {
issuer: issuerDomain,
});

return (payload.sub as string) ?? null;
} catch (err) {
console.warn(
'[auth-session] JWT verification failed:',
err instanceof Error ? err.message : String(err),
);
return null;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Duplicate JWKS singleton alongside existing server/auth-session.ts

The pre-existing server/auth-session.ts already initialises a module-level JWKS resolver from CLERK_JWT_ISSUER_DOMAIN and exposes validateBearerToken. This new file creates a second independent JWKS singleton for the same issuer domain. After this PR there are two parallel JWKS caches per cold start.

jose handles key rotation internally per instance, so having two will not cause incorrect auth decisions, but it does mean two in-memory caches and two separate refresh cycles — wasted memory and HTTP requests. Consider having resolveSessionUserId call into the existing validateBearerToken from server/auth-session.ts (or extract the common JWT-verification logic into a shared helper) so there is a single JWKS instance.

Comment on lines +7 to +10
path: "/dodopayments-webhook",
method: "POST",
handler: webhookHandler,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Webhook path mismatch between http.ts and the PR description

The route is registered as /dodopayments-webhook:

http.route({
  path: "/dodopayments-webhook",
  ...

The PR description and architecture diagram consistently refer to the endpoint as /dodo/webhook. This is a documentation mismatch, but more importantly the Dodo dashboard webhook URL must point to the exact Convex HTTP action URL — if the dashboard is configured with /dodo/webhook, all incoming webhooks will receive 404 responses and Dodo's retry queue will exhaust silently. Confirm which path is actually registered in the Dodo webhook settings.

Comment on lines +298 to +308

try {
const finalKey = raw ? key : prefixKey(key);
await fetch(`${url}/del/${encodeURIComponent(finalKey)}`, {
headers: { Authorization: `Bearer ${token}` },
signal: AbortSignal.timeout(REDIS_OP_TIMEOUT_MS),
});
} catch (err) {
console.warn('[redis] deleteRedisKey failed:', errMsg(err));
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 deleteRedisKey uses implicit GET while cacheActions.ts uses POST for the same DEL command

deleteRedisKey omits the method property, so fetch defaults to GET. Meanwhile convex/payments/cacheActions.ts performs the equivalent DEL with method: "POST". Upstash accepts both for path-based commands, so this is not a runtime failure — but the inconsistency is a maintenance hazard. All other write helpers in this file use POST; this should match.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 4f187c3 — added method: 'POST' to deleteRedisKey to match all other write helpers.

Merge main into dodo_payments: combine Cloudflare challenges CSP
additions from main with Dodo checkout frame-src from this branch,
and keep both risk-score/brief CSS (main) and payment UI CSS (dodo).

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Copy link
Copy Markdown
Owner

@koala73 koala73 left a comment

Choose a reason for hiding this comment

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

Code Review: PR #2024 — Dodo Payments Integration

Scope: 57 files, ~2700 lines | Agents used: Security, Architecture, TypeScript, Performance, Simplicity

The core architecture is well-designed — idempotent webhook pipeline, tier-based entitlements, Redis fast-path with Convex fallback, config-driven PLAN_FEATURES. The P1s are all traceable to the auth stub being applied with the wrong failure model: "graceful degradation" is correct for data loading, wrong for a payment gate.


🔴 P1 — Blocks Merge (6 issues)

1. Fail-open entitlement gateserver/_shared/entitlement-check.ts:141-147

When CLERK_JWT_ISSUER_DOMAIN is unset (current auth-stub state), resolveSessionUserId returns null, x-user-id is never set, and checkEntitlement returns null (allow) for every request. All 4 tier-gated endpoints are effectively open to anonymous users. Must fail-closed: block all tier-gated endpoints entirely when Clerk is not configured.

// Current — wrong for a payment gate
if (!userId) {
  return null; // allow — "graceful degradation"
}

2. Client-controlled userId in checkout metadataconvex/payments/checkout.ts:29-33

const userId = authedUserId ?? args.userId; // args.userId is browser-supplied

When resolveUserId returns null, the browser's args.userId is embedded verbatim as metadata.wm_user_id. Webhooks trust this field as identity. An attacker completes a checkout with userId: "victim-clerk-id" → entitlements written to the victim's account. Privilege escalation in both directions. When resolveUserId is null, the action must throw — never fall back to client-supplied identity.

3. Dual auth systems with contradictory logicserver/gateway.ts:248-314

Both PREMIUM_RPC_PATHS + bearer role check AND the new ENDPOINT_ENTITLEMENTS + tier check run on the same 4 endpoints. These can produce different decisions: a pro JWT role (tier 1) passes the legacy check but would be blocked by the tier-2 enforcement on api_* endpoints. JWT role claims also persist after subscription expiry. The PREMIUM_RPC_PATHS/validateBearerToken/role path must be removed entirely — the tier system is the correct mechanism.

4. v.any() on payment-critical cacheconvex/payments/cacheActions.ts:29

features: v.any(),

Bypasses Convex's runtime validator. A malformed features object silently populates Redis and the gateway makes wrong access decisions. Must use v.object(...) matching PlanFeatures.

5. Record<string, any> on all Convex API callssrc/services/convex-client.ts:40

Every payment call site loses TypeScript's type safety. A renamed Convex function fails silently at runtime. Fixable with a lazily resolved typed singleton.

6. Cache stampede on Convex fallbackserver/_shared/entitlement-check.ts:90-122

Redis cache miss sends all concurrent requests to Convex simultaneously with no coalescing. This project's own MEMORY.md documents cachedFetchJson as the mandatory pattern for exactly this scenario. It is not used here.


🟡 P2 — Fix Before First Production Traffic (12 issues)

# Issue Location
1 dispute.lost logs warning but doesn't revoke entitlements — chargebacked users keep premium access indefinitely subscriptionHelpers.ts:590
2 rawPayload: v.any() on 3 tables stores full Dodo payload including customer email, billing address (PII unclassified in Convex) schema.ts:54,89,104
3 Entitlement Redis keys have no live/test prefix — test-mode payment grants production access on shared Upstash entitlement-check.ts:93
4 syncEntitlementCache has no timeout — hung Upstash write silently diverges Redis from Convex for up to 1 hour cacheActions.ts:51-63
5 ENTITLEMENT_CACHE_TTL_SECONDS = 3600 — expired subscription keeps premium access for up to 1 hour after subscription.expired fires cacheActions.ts:14
6 CONVEX_IS_DEV=true in a production Convex deployment silently routes all entitlements to test-user-001 subscriptionHelpers.ts:108
7 claimSubscription uses .unique() on entitlements — throws if a race condition creates 2 rows, breaking the claim mutation entirely billing.ts:236
8 toEpochMs fallback uses Date.now() not eventTimestamp per its own JSDoc — entitlements expire immediately when next_billing_date is missing subscriptionHelpers.ts:199
9 hasUserIdentity() always returns true — the auth stub means getUserId() never returns null user-identity.ts:76
10 JWT verified twice per premium request — resolveSessionUserId + validateBearerToken both call jwtVerify on the same token gateway.ts:243,299
11 getSubscriptionForUser uses .collect() with no limit — unbounded query grows with subscription history billing.ts:57
12 Frontend ConvexClient subscription never unsubscribes — logout + new login reuses the stale subscription for the previous user entitlements.ts:35-65

🔵 P3 — Track as Follow-ups (8 issues)

# Issue
1 changePlan Convex action (~50 LOC) has no callers anywhere — YAGNI, remove before merge
2 claimSubscription (~140 LOC) solves an anon→auth migration that only becomes necessary after Clerk lands — defer
3 by_status index on subscriptions table is unused by any query in this PR
4 webhookEvents.status: v.literal("processed") — a single-value field that encodes no information (failed events are deleted, not marked)
5 pro_monthly and pro_annual have identical entries in PLAN_FEATURES — any feature change requires two edits
6 FALLBACK_USER_ID = "test-user-001" defined independently in both auth.ts and subscriptionHelpers.ts
7 WEB_PREMIUM_PANELS includes daily-market-brief/market-implications — UI-hidden but not in ENDPOINT_ENTITLEMENTS (server unguarded)
8 paymentEvents table has no reader in this PR — write-only audit trail, defer until a billing history UI exists

Summary

The design of the webhook pipeline (idempotency via webhookId, out-of-order rejection, scheduler-based cache sync) is solid. The PLAN_FEATURES one-line extensibility claim holds up.

The 6 P1s all need resolution before merge — particularly #1 (fail-open gate) and #2 (client-controlled identity) which are exploitable in the current auth-stub state. Happy to discuss any of these.

@koala73
Copy link
Copy Markdown
Owner

koala73 commented Mar 27, 2026

Code Review: PR #2024 — Dodo Payments Integration

Scope: 57 files, ~2700 lines | Agents used: Security, Architecture, TypeScript, Performance, Simplicity

The core architecture is well-designed — idempotent webhook pipeline, tier-based entitlements, Redis fast-path with Convex fallback, config-driven PLAN_FEATURES. The P1s are all traceable to the auth stub being applied with the wrong failure model: "graceful degradation" is correct for data loading, wrong for a payment gate.


🔴 P1 — Blocks Merge (6 issues)

1. Fail-open entitlement gateserver/_shared/entitlement-check.ts:141-147

When CLERK_JWT_ISSUER_DOMAIN is unset (current auth-stub state), resolveSessionUserId returns null, x-user-id is never set, and checkEntitlement returns null (allow) for every request. All 4 tier-gated endpoints are effectively open to anonymous users. Must fail-closed: block all tier-gated endpoints entirely when Clerk is not configured.

// Current — wrong for a payment gate
if (!userId) {
  return null; // allow — "graceful degradation"
}

2. Client-controlled userId in checkout metadataconvex/payments/checkout.ts:29-33

const userId = authedUserId ?? args.userId; // args.userId is browser-supplied

When resolveUserId returns null, the browser's args.userId is embedded verbatim as metadata.wm_user_id. Webhooks trust this field as identity. An attacker completes a checkout with userId: "victim-clerk-id" → entitlements written to the victim's account. Privilege escalation in both directions. When resolveUserId is null, the action must throw — never fall back to client-supplied identity.

3. Dual auth systems with contradictory logicserver/gateway.ts:248-314

Both PREMIUM_RPC_PATHS + bearer role check AND the new ENDPOINT_ENTITLEMENTS + tier check run on the same 4 endpoints. A user with a pro JWT role claim (tier 1) passes the legacy check but would be blocked by the tier-2 enforcement on api_* endpoints. JWT role claims also persist after subscription expiry. The PREMIUM_RPC_PATHS/validateBearerToken/role path must be removed — the tier system is the correct mechanism.

4. v.any() on payment-critical cacheconvex/payments/cacheActions.ts:29

Bypasses Convex's runtime validator. A malformed features object silently populates Redis and the gateway makes wrong access decisions. Must use a typed v.object(...) matching PlanFeatures.

5. Record<string, any> on all Convex API callssrc/services/convex-client.ts:40

Every payment call site loses TypeScript type safety. A renamed Convex function fails silently at runtime. Fixable with a lazily resolved typed singleton.

6. Cache stampede on Convex fallbackserver/_shared/entitlement-check.ts:90-122

Redis cache miss sends all concurrent requests to Convex simultaneously with no coalescing. This project's own documented pattern (cachedFetchJson) handles exactly this and is not used here.


🟡 P2 — Fix Before First Production Traffic (12 issues)

# Issue Location
1 dispute.lost logs warning but doesn't revoke entitlements — chargebacked users keep premium access subscriptionHelpers.ts:590
2 rawPayload: v.any() on 3 tables stores full Dodo payload including customer email/billing PII unclassified schema.ts:54,89,104
3 Entitlement Redis keys have no live/test prefix — test-mode payment grants production access on shared Upstash entitlement-check.ts:93
4 syncEntitlementCache has no timeout — hung Upstash write silently diverges Redis from Convex for up to 1 hour cacheActions.ts:51-63
5 ENTITLEMENT_CACHE_TTL_SECONDS = 3600 — expired subscription keeps premium access for up to 1 hour after subscription.expired fires cacheActions.ts:14
6 CONVEX_IS_DEV=true in a production Convex deployment silently routes all entitlements to test-user-001 subscriptionHelpers.ts:108
7 claimSubscription uses .unique() on entitlements — throws if a race condition creates 2 rows, breaking the claim mutation billing.ts:236
8 toEpochMs fallback uses Date.now() not eventTimestamp per its own JSDoc — entitlements expire immediately when next_billing_date is missing subscriptionHelpers.ts:199
9 hasUserIdentity() always returns true — auth stub means getUserId() never returns null user-identity.ts:76
10 JWT verified twice per premium request — resolveSessionUserId + validateBearerToken both call jwtVerify on the same token gateway.ts:243,299
11 getSubscriptionForUser uses .collect() with no limit — unbounded query grows with subscription history billing.ts:57
12 Frontend ConvexClient subscription never unsubscribes — logout + new login reuses the stale subscription for the previous user entitlements.ts:35-65

🔵 P3 — Track as Follow-ups

# Issue
1 changePlan Convex action (~50 LOC) has no callers — YAGNI, remove before merge
2 claimSubscription (~140 LOC) solves an anon→auth migration that only exists after Clerk lands — defer
3 by_status index on subscriptions table is unused by any query in this PR
4 webhookEvents.status: v.literal("processed") — single-value field encoding no information (failed events are deleted, not marked)
5 pro_monthly and pro_annual have identical PLAN_FEATURES entries — any feature change requires two edits
6 FALLBACK_USER_ID = "test-user-001" defined independently in both auth.ts and subscriptionHelpers.ts
7 WEB_PREMIUM_PANELS includes daily-market-brief/market-implications — UI-hidden but not in ENDPOINT_ENTITLEMENTS (server unguarded)
8 paymentEvents table has no reader in this PR — write-only audit trail, defer until billing history UI exists

Happy to walk through any of these. The webhook pipeline design and PLAN_FEATURES extensibility are solid foundations — the P1s are all resolvable without structural changes.

All other write helpers use POST; DEL was implicitly using GET via fetch default.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@koala73
Copy link
Copy Markdown
Owner

koala73 commented Mar 27, 2026

Code Review — Dodo Payments Integration + Entitlement Engine

7 review agents | 57 files, ~2,700 lines | dodo_payments branch

The webhook pipeline architecture is solid: HMAC-SHA256 signature verification on the raw body, idempotent processing via webhookId dedup, Convex transaction rollback on handler failure, and good test coverage for the happy path. The PLAN_FEATURES config map and the Redis-first / Convex-fallback caching pattern are clean.

However, there are 6 blocking issues that must be resolved before merge.


🔴 P1 — BLOCKS MERGE

1. API key callers bypass entitlement check entirely (server/gateway.ts:235-246, server/_shared/entitlement-check.ts:143)

x-user-id is only set when resolveSessionUserId() succeeds (requires Clerk Bearer JWT). checkEntitlement returns null (allow) when the header is absent. Any holder of a valid API key hits the four tier-2-gated endpoints with no entitlement check. This is the inverse of the intended gate.

Fix: Require Bearer JWT for tier-gated endpoints when no x-user-id can be resolved, or explicitly reject bare API-key callers on gated endpoints.


2. dispute.lost does not revoke entitlements (convex/payments/subscriptionHelpers.ts:590-594)

When Dodo sends dispute.lost (chargeback won by user, charge reversed), the handler only logs a console.warn. The user retains paid-tier access until validUntil expires naturally. This is an active fraud vector.

Fix: Call upsertEntitlements(ctx, userId, "free", eventTimestamp, eventTimestamp) on dispute.lost, matching how subscription.expired is handled at line 494. Add a test.


3. PREMIUM_RPC_PATHS JWT check short-circuits before checkEntitlement (server/gateway.ts:251-289)

For Bearer JWT users, the PREMIUM_RPC_PATHS branch (line 255) validates the JWT plan claim (session.role !== 'pro') and either returns 403 or allows through — before checkEntitlement() at line 288 ever runs. This means the Convex entitlement table is not the actual enforcer for authenticated users. The stale JWT plan claim is. A user who just upgraded has a new Convex entitlement row but gets blocked by their old JWT until it expires.

Fix: Remove the session.role !== 'pro' check from the PREMIUM_RPC_PATHS branch. Let checkEntitlement be the sole tier authority.


4. Edge function imports ../../convex/_generated/api (server/_shared/entitlement-check.ts:63)

Project rule (CRITICAL): "Vercel Edge Functions: Self-Contained .js Only — cannot import from outside their directory." This dynamic import traverses into convex/. The project's own architectural plan (sequential-sparking-bear.md) explicitly states: "Vercel Edge can't import Convex. Gateway reads from Redis only, never Convex directly." The existing tests/edge-functions.test.mjs does not cover server/_shared/, so this violation is untested.

Fix: Remove the getConvexSingleton() fallback from entitlement-check.ts. Gateway is Redis-only; Convex entitlement sync happens via cacheActions.syncEntitlementCache (already implemented). Also add server/_shared/ to the edge-function boundary test.


5. No-userId on tier-gated endpoints allows through instead of 401 (server/_shared/entitlement-check.ts:143-147)

If CLERK_JWT_ISSUER_DOMAIN is missing from the Vercel environment, resolveSessionUserId returns null for every request. The entitlement check then skip-allows every gated request. The code comment acknowledges this as "graceful degradation during auth-stub era" — that decision needs to be revisited now that the entitlement system is real.

Fix: Return 401 (not null/allow) when requiredTier !== null && !userId.


6. JWT audience and algorithms not validated in _shared/auth-session.ts (server/_shared/auth-session.ts:46)

server/auth-session.ts correctly validates audience: 'convex' and algorithms: ['RS256']. The new _shared/auth-session.ts validates issuer only. A JWT issued by the same Clerk instance for a different service would pass this check.

Fix: Add audience: 'convex', algorithms: ['RS256'] to the jwtVerify call.


🟡 P2 — Should Fix

  • Cache stampede: getEntitlements uses raw getCachedJson/setCachedJson instead of cachedFetchJson. On Redis cold start, concurrent requests fan out N Convex queries instead of coalescing. Also: a Redis write failure in the Convex fallback path suppresses the result (returns null) rather than returning the fetched value. (server/_shared/entitlement-check.ts:90-122)

  • toEpochMs docstring/impl mismatch: Docstring says fallback uses eventTimestamp, code returns Date.now(). (subscriptionHelpers.ts:192-202)

  • resolveSessionUserId runs on every request: JWT cryptographic verification fires for all ~80+ endpoints, but only 4 are tier-gated. Defer this call to inside the isTierGated branch. (gateway.ts:235)

  • YAGNI — 5 PlanFeatures fields: maxDashboards, apiAccess, apiRateLimit, prioritySupport, exportFormats are defined in 3 duplicated type declarations, stored in Convex + Redis, and tested — but zero production code reads them for gating decisions. Only tier is enforced. Add a comment noting these are aspirational, or remove until enforcement code ships.

  • Dead retry branch: webhookMutations.ts:41-49 has a branch that deletes "stale failed" records, but the schema (v.literal("processed")) makes it impossible for any row to have a non-processed status. The branch is unreachable. (webhookMutations.ts:46-49)


🔵 P3 — Nice-to-Have

  • webhookEvents table has no createdAt field or cleanup strategy — add createdAt: v.number() now for future use
  • handleSubscriptionOnHold, handleSubscriptionCancelled, handleSubscriptionExpired share ~90% identical logic — extract a updateSubscriptionStatus helper
  • pro_monthly and pro_annual have identical PlanFeatures objects — extract a shared PRO_FEATURES constant

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

Labels

High Value Meaningful contribution to the project Not Ready to Merge PR has conflicts, failing checks, or needs work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants