fix(billing): compute org coverage independently of the personal-sub lookup and fail closed on unverifiable plan writes

Author: icecrasher321

apps/sim/lib/auth/stripe-adapter-guard.test.ts

@@ -68,6 +68,37 @@ describe('guardSubscriptionPlanWrites', () => {
     expect(base.update).not.toHaveBeenCalled()
   })
 
+  it('strips the plan when the pre-update lookup finds no row (fail closed)', async () => {
+    const base = createBaseAdapter()
+    base.findOne.mockResolvedValueOnce(null)
+
+    const guarded = asAdapter(base)
+    await guarded.update({
+      model: 'subscription',
+      where: WHERE as never,
+      update: { plan: 'pro_6000', status: 'active' },
+    })
+
+    expect(base.update).toHaveBeenCalledWith(
+      expect.objectContaining({ update: { status: 'active' } })
+    )
+  })
+
+  it('writes nothing when a plan-only update targets no readable row', async () => {
+    const base = createBaseAdapter()
+    base.findOne.mockResolvedValueOnce(null)
+
+    const guarded = asAdapter(base)
+    const result = await guarded.update({
+      model: 'subscription',
+      where: WHERE as never,
+      update: { plan: 'pro_6000' },
+    })
+
+    expect(result).toBeNull()
+    expect(base.update).not.toHaveBeenCalled()
+  })
+
   it('passes through non-org plan updates for user-referenced rows', async () => {
     const base = createBaseAdapter()
     base.findOne.mockResolvedValueOnce({ id: 'sub-2', referenceId: 'user-1', plan: 'pro_6000' })

apps/sim/lib/auth/stripe-adapter-guard.ts

@@ -139,11 +139,26 @@ interface SanitizedSubscriptionUpdate {
  * periods, seats, cancellation state) are legitimate Stripe state and still
  * sync. Evaluating every targeted row keeps multi-row updates safe: a mixed
  * personal/org target set must not leak the invalid plan onto the org rows.
+ *
+ * Fails closed on an empty target set: a plan we could not verify against a
+ * readable row never forwards — the row may be mid-creation by a concurrent
+ * webhook delivery, and in the sequential no-row case the write was a no-op
+ * anyway. A legitimate user-referenced plan dropped this way re-syncs via
+ * `syncSubscriptionPlan` in the same webhook callback.
  */
 async function stripPlanWhenOrgReferenced(
   rows: SubscriptionRowSlice[],
   update: Record<string, unknown>
 ): Promise<SanitizedSubscriptionUpdate> {
+  if (rows.length === 0) {
+    logger.warn(
+      'Subscription plan write targeted no readable row; stripping the unverifiable plan',
+      { rejectedPlan: update.plan }
+    )
+    const { plan: _plan, limits: _limits, ...rest } = update
+    return { update: rest, blockedAll: Object.keys(rest).length === 0 }
+  }
+
   let organizationRow: SubscriptionRowSlice | null = null
   for (const row of rows) {
     if (await isOrganizationReference(row.referenceId)) {

apps/sim/lib/billing/organizations/membership.ts

@@ -357,21 +357,13 @@ export async function pauseProSubscriptionForOrgCoverage(
   await db.transaction(async (tx) => {
     await acquireUserBillingIdentityLock(tx, userId)
 
-    const [personalPro] = await tx
-      .select()
-      .from(subscriptionTable)
-      .where(
-        and(
-          eq(subscriptionTable.referenceId, userId),
-          inArray(subscriptionTable.status, ENTITLED_SUBSCRIPTION_STATUSES),
-          sqlIsPro(subscriptionTable.plan)
-        )
-      )
-      .for('update')
-      .limit(1)
-
-    if (!personalPro) return
-
+    /**
+     * Coverage is determined before (and independent of) the personal-sub
+     * lookup: `covered` reports the organization's coverage truth even when
+     * no entitled personal Pro row exists, exactly as the result contract
+     * promises. Callers gate free→paid transition handling on it, so a
+     * personal-sub lookup miss must not read as "not covered".
+     */
     const memberships = await tx
       .select({ organizationId: member.organizationId })
       .from(member)
@@ -396,9 +388,25 @@ export async function pauseProSubscriptionForOrgCoverage(
     if (!coveringSubscription) return
 
     result.covered = true
-    result.subscriptionId = personalPro.id
     result.organizationId = coveringSubscription.referenceId
 
+    const [personalPro] = await tx
+      .select()
+      .from(subscriptionTable)
+      .where(
+        and(
+          eq(subscriptionTable.referenceId, userId),
+          inArray(subscriptionTable.status, ENTITLED_SUBSCRIPTION_STATUSES),
+          sqlIsPro(subscriptionTable.plan)
+        )
+      )
+      .for('update')
+      .limit(1)
+
+    if (!personalPro) return
+
+    result.subscriptionId = personalPro.id
+
     if (personalPro.cancelAtPeriodEnd) return
 
     await tx

apps/sim/lib/billing/organizations/pause-pro-for-coverage.test.ts

@@ -60,9 +60,9 @@ describe('pauseProSubscriptionForOrgCoverage', () => {
 
   it('pauses the personal Pro and queues the Stripe sync when an entitled paid org covers the user', async () => {
     queueWhereResponses([
-      [ACTIVE_PERSONAL_PRO],
       [{ organizationId: 'org-1' }],
       [{ plan: 'team_6000', referenceId: 'org-1' }],
+      [ACTIVE_PERSONAL_PRO],
       // update ... set ... where consumes one more where() call
       [],
     ])
@@ -87,21 +87,29 @@ describe('pauseProSubscriptionForOrgCoverage', () => {
     )
   })
 
-  it('reports not covered when the user has no entitled personal Pro', async () => {
-    queueWhereResponses([[]])
+  it('reports covered even when no entitled personal Pro row exists', async () => {
+    queueWhereResponses([
+      [{ organizationId: 'org-1' }],
+      [{ plan: 'team_6000', referenceId: 'org-1' }],
+      [],
+    ])
 
     const result = await pauseProSubscriptionForOrgCoverage('user-1')
 
-    expect(result).toEqual({ covered: false, paused: false })
+    expect(result).toEqual({
+      covered: true,
+      paused: false,
+      organizationId: 'org-1',
+    })
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
     expect(mockEnqueueOutboxEvent).not.toHaveBeenCalled()
   })
 
   it('reports covered without pausing again when the personal Pro is already pausing', async () => {
     queueWhereResponses([
-      [{ ...ACTIVE_PERSONAL_PRO, cancelAtPeriodEnd: true }],
       [{ organizationId: 'org-1' }],
       [{ plan: 'team_6000', referenceId: 'org-1' }],
+      [{ ...ACTIVE_PERSONAL_PRO, cancelAtPeriodEnd: true }],
     ])
 
     const result = await pauseProSubscriptionForOrgCoverage('user-1')
@@ -117,7 +125,7 @@ describe('pauseProSubscriptionForOrgCoverage', () => {
   })
 
   it('reports not covered when the user is not a member of any organization', async () => {
-    queueWhereResponses([[ACTIVE_PERSONAL_PRO], []])
+    queueWhereResponses([[]])
 
     const result = await pauseProSubscriptionForOrgCoverage('user-1')
 
@@ -126,7 +134,7 @@ describe('pauseProSubscriptionForOrgCoverage', () => {
   })
 
   it('reports not covered when no org subscription is an entitled paid plan', async () => {
-    queueWhereResponses([[ACTIVE_PERSONAL_PRO], [{ organizationId: 'org-1' }], []])
+    queueWhereResponses([[{ organizationId: 'org-1' }], []])
 
     const result = await pauseProSubscriptionForOrgCoverage('user-1')