fix(billing): close TOCTOU race in subscription transfer, centralize stripe test mocks

[waleedlatif1]

Summary

• Wrap subscription-transfer check + update in db.transaction with .for('update') row-locks in both app/api/users/me/subscription/[id]/transfer/route.ts and lib/billing/organization.ts to close a TOCTOU race that allowed concurrent requests to attach two subscriptions to the same org
• Add .for('update') terminal to dbChainMock and document it in sim-testing.md
• Add centralized stripe.mock.ts (stripeClientMock, stripePaymentMethodMock, createMockStripeEvent) and migrate billing tests off inline mocks

Type of Change

• Bug fix

Testing

All billing + subscription tests pass (32 tests, 8 files).

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Apr 20, 2026 11:19pm

[cursor]

PR Summary

Medium Risk
Introduces transactional FOR UPDATE locking and new duplicate-subscription checks in billing transfer flows; correctness depends on DB semantics and could impact edge cases/concurrency behavior. Test-only refactors are low risk but touch many files.

Overview
Closes a race in subscription-to-organization transfers by moving the read/check/update flow into db.transaction(...) blocks and taking row-level locks via .for('update') in both the subscription transfer API route and ensureOrganizationForTeamSubscription.

The transfer logic now performs an in-transaction duplicate-org-subscription lookup using ENTITLED_SUBSCRIPTION_STATUSES + inArray(...), returning consistent noop/error/success outcomes without relying on the prior hasPaidSubscription(...) call.

Testing infrastructure updates: extends dbChainMock to support .for('update'), documents it, adds centralized Stripe mocks (stripe-client, stripe-payment-method, createMockStripeEvent), and migrates multiple billing tests to use the shared mocks/DB chain helpers. Also updates bun.lock metadata (configVersion).

^{Reviewed by Cursor Bugbot for commit f14eac2. Configure here.}

[greptile-apps]

Greptile Summary

This PR closes a TOCTOU race in subscription transfers by wrapping the check-then-update logic in db.transaction with .for('update') row-locks on both the subscription and organization rows, applied in both route.ts and the existing-membership branch of organization.ts. It also centralizes Stripe test mocks into stripe.mock.ts and documents the new .for('update') support in the dbChainMock helper.

• The new-org creation path (organization.ts lines 222-250) remains unprotected against concurrent webhook deliveries — noted in a prior review and still open.

Confidence Score: 4/5

Safe to merge for the transfer-route fix; the unresolved new-org creation TOCTOU in organization.ts (flagged in prior review) should be addressed before shipping the webhook path.

The route.ts fix is solid and well-tested. The existing-membership branch in organization.ts is now correctly protected. However, the new-org creation path (lines 222-250 in organization.ts) still lacks transaction protection — a P1 concern raised in a previous review that remains open in this iteration.

apps/sim/lib/billing/organization.ts — new-org creation path (lines 222-250) is unprotected; apps/sim/lib/billing/organization.test.ts — transaction branch is untested.

Important Files Changed

Filename	Overview
apps/sim/app/api/users/me/subscription/[id]/transfer/route.ts	TOCTOU race fully closed: subscription and org rows are locked with .for('update') inside a single transaction, with auth, plan, noop, and duplicate-subscription checks all happening under the lock.
apps/sim/lib/billing/organization.ts	Existing-membership branch now protected by a transaction + row locks; the new-org creation path (lines 222-250) is still unprotected against concurrent webhook deliveries creating duplicate orgs (flagged in previous review).
apps/sim/app/api/users/me/subscription/[id]/transfer/route.test.ts	Good test coverage: non-org plan rejection, successful transfer, noop (already transferred), membership-gated noop, and duplicate-subscription conflict are all covered.
packages/testing/src/mocks/database.mock.ts	Added for mock with thenable that chains .limit/.orderBy/.returning/.groupBy and is documented correctly; mockResolvedValueOnce limitation (returns plain Promise without chained methods) is called out in the JSDoc.
packages/testing/src/mocks/stripe.mock.ts	New centralized stripe mocks with stripeClientMock, stripePaymentMethodMock, and createMockStripeEvent (including all required Stripe.Event fields from prior fix); well-structured and exports are properly wired through the package index.
apps/sim/lib/billing/organization.test.ts	Only covers the early-return (legacy org reference) path; the new transaction branch protecting against duplicate subscriptions has no test coverage.
.claude/rules/sim-testing.md	Rules doc updated to document .for('update') mock behavior — terminal vs. chained usage, mockResolvedValueOnce override semantics, and resetDbChainMock() guidance.

Sequence Diagram

sequenceDiagram
    participant C1 as Concurrent Request 1
    participant C2 as Concurrent Request 2
    participant TX as DB Transaction
    participant Sub as subscription row
    participant Org as organization row

    C1->>TX: BEGIN TRANSACTION
    C1->>Sub: SELECT ... FOR UPDATE (locks row)
    C2->>TX: BEGIN TRANSACTION
    C2-->>Sub: blocked — waiting for lock
    C1->>Org: SELECT ... FOR UPDATE (locks row)
    C1->>Sub: check existingOrgSub
    C1->>Sub: UPDATE referenceId → org-id
    C1->>TX: COMMIT (releases locks)
    C2->>Sub: SELECT ... FOR UPDATE (now acquired)
    C2->>Org: SELECT ... FOR UPDATE
    C2->>Sub: check existingOrgSub → FOUND
    C2->>TX: ROLLBACK (duplicate detected)

Loading

_{Reviews (4): Last reviewed commit: "fix(billing): gate subscription transfer..." | Re-trigger Greptile}

[waleedlatif1]

Re Greptile P1 (new-org branch race in organization.ts lines 222–237): the member.userId unique index (member_user_id_unique in packages/db/schema.ts:989, comment: "Users can only belong to one org") already protects this path. Two concurrent webhook deliveries that both pass the existingMembership.length === 0 check will both call createOrganizationWithOwner, which inserts org + member in a single transaction. The second INSERT INTO member fails with 23505 unique_violation, rolling back the entire tx including the org insert. The caller throws before reaching db.update(subscriptionTable).set({ referenceId: orgId }), so the subscription never points to a phantom org and no duplicate is created. Stripe retries the failed webhook and the second attempt finds the membership from the first commit and takes the existing-membership branch (which is now lock-protected). Declining the suggested change — the DB invariant is the right place for this protection, and adding application-level locking on top would be redundant.

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit f14eac2. Configure here.}