fix(billing): never let the identity record block an erasure, and keep later links unchecked

- Recording a deleted account's identity digests now runs in a savepoint and
  only logs on failure, so an anti-abuse write can never abort the holder's
  deletion; before this change nothing of the kind could block it.
- A stats row the fallback creates gets only the check signup would have made:
  the email always, and the linked sign-ins only for a user created in the
  same auth request. A legacy user without a stats row who links a sign-in
  later is no longer withheld through the fallback.
- Docs say exactly who is checked: every user Better Auth creates, SCIM- and
  SSO-provisioned ones included, is checked by email at creation; only a
  sign-in linked in a later request never withholds anything.

Author: waleedlatif1

apps/sim/lib/auth/auth.ts

@@ -646,7 +646,7 @@ export const auth = betterAuth({
 
           try {
             const { ensureUserStatsExists } = await import('@/lib/billing/core/usage')
-            await ensureUserStatsExists(account.userId)
+            await ensureUserStatsExists(account.userId, context)
           } catch (error) {
             logger.error('[databaseHooks.account.create.after] Failed to ensure user stats', {
               userId: account.userId,

apps/sim/lib/billing/core/usage.test.ts

@@ -450,6 +450,25 @@ describe('withholding the free allowance from a returning deleted identity', ()
     expect(forfeited()).toBe(true)
   })
 
+  it('checks a fallback row’s linked sign-ins only for a user created in the same request', async () => {
+    mockUserHasDeletedAccountIdentity.mockImplementation(
+      async (_userId: string, { includeSignIns }: { includeSignIns: boolean }) => includeSignIns
+    )
+    dbChainMockFns.returning
+      .mockResolvedValueOnce([{ id: 'stats-1' }])
+      .mockResolvedValueOnce([{ id: 'stats-1' }])
+
+    await ensureUserStatsExists('user-1', {})
+    await ensureUserStatsExists('user-1')
+    expect(forfeited()).toBe(false)
+
+    const signupRequest = {}
+    await handleNewUser('user-1', signupRequest)
+    dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'stats-1' }])
+    await ensureUserStatsExists('user-1', signupRequest)
+    expect(forfeited()).toBe(true)
+  })
+
   it('never fails the caller when withholding from a fallback row fails', async () => {
     mockUserHasDeletedAccountIdentity.mockResolvedValue(true)
     dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'stats-1' }])

apps/sim/lib/billing/core/usage.ts

@@ -186,23 +186,28 @@ function calculateOrgUsageLimit(
 /**
  * Users created in an auth request, keyed by Better Auth's endpoint context.
  * `user.create.after` and `account.create.after` of one signup receive the same
- * context object, which is how {@link handleNewSignInAccount} tells a signup's
- * first sign-in apart from a sign-in linked to an existing user later.
+ * context object, which is how {@link handleNewSignInAccount} and
+ * {@link ensureUserStatsExists} tell a signup's first sign-in apart from a
+ * sign-in linked to an existing user later.
  */
 const usersCreatedInAuthRequest = new WeakMap<object, string>()
 
 /**
  * Handle new user setup when they join the platform.
  * Creates the userStats record with the free allowance, which is zero when the
  * user's email or an already-linked social sign-in belongs to an account
- * deleted within the retention window.
+ * deleted within the retention window. Every user Better Auth creates comes
+ * through here, including SCIM- and SSO-provisioned users, so their email is
+ * always checked at creation.
  *
  * @param authRequest Better Auth's endpoint context for the signup request, when there is one
  */
 export async function handleNewUser(userId: string, authRequest?: object | null): Promise<void> {
   if (authRequest) usersCreatedInAuthRequest.set(authRequest, userId)
   try {
-    const freeTierEligible = !(await userHasDeletedAccountIdentity(userId))
+    const freeTierEligible = !(await userHasDeletedAccountIdentity(userId, {
+      includeSignIns: true,
+    }))
     if (!freeTierEligible) {
       logger.info('Withholding the free allowance from a returning deleted-account identity', {
         userId,
@@ -229,9 +234,10 @@ export async function handleNewUser(userId: string, authRequest?: object | null)
 /**
  * The account-creation half of the signup check, for a social sign-in that was
  * not yet linked when {@link handleNewUser} ran. Only an account created in the
- * same auth request as its user is a signup; a sign-in linked to an existing
- * user later, including a SCIM- or SSO-provisioned one, never withholds
- * anything. Integration connections are skipped.
+ * same auth request as its user is a signup. A sign-in linked to an existing
+ * user in a later request never withholds anything; that includes a SCIM- or
+ * SSO-provisioned user, whose email was already checked when it was created.
+ * Integration connections are skipped.
  *
  * @param authRequest Better Auth's endpoint context for the request creating the account
  */
@@ -258,12 +264,19 @@ export async function handleNewSignInAccount(
  * Creates one with default values if missing.
  * This is a fallback for cases where the user.create.after hook didn't fire
  * or failed (e.g., OAuth account linking to existing users). A row created
- * here gets the same deleted-identity check as signup; a failed check leaves
- * the allowance in place and never fails the caller.
+ * here gets only the check signup would have made: the email always, and the
+ * linked sign-ins only when the user was created in `authRequest`, so a sign-in
+ * linked later never withholds. A failed check leaves the allowance in place
+ * and never fails the caller.
  *
  * Always writes to the primary — never takes a read-routing executor.
+ *
+ * @param authRequest Better Auth's endpoint context, when called from an auth hook
  */
-export async function ensureUserStatsExists(userId: string): Promise<void> {
+export async function ensureUserStatsExists(
+  userId: string,
+  authRequest?: object | null
+): Promise<void> {
   const created = await db
     .insert(userStats)
     .values({
@@ -277,7 +290,12 @@ export async function ensureUserStatsExists(userId: string): Promise<void> {
   if (created.length === 0) return
 
   try {
-    if (await userHasDeletedAccountIdentity(userId)) await forfeitFreeTierAllowance(userId)
+    const includeSignIns = authRequest
+      ? usersCreatedInAuthRequest.get(authRequest) === userId
+      : false
+    if (await userHasDeletedAccountIdentity(userId, { includeSignIns })) {
+      await forfeitFreeTierAllowance(userId)
+    }
   } catch (error) {
     logger.error('Failed to withhold the free allowance from a fallback stats row', {
       userId,

apps/sim/lib/users/deleted-identity.integration.ts

@@ -248,6 +248,21 @@ describe('deleted account identities and the free allowance', () => {
     expect(await statsOf(stranger)).toEqual({ freeTierEligible: true, limit: getFreeTierLimit() })
   })
 
+  it('never withholds from a stats-less user’s fallback row for a sign-in linked in a later request', async () => {
+    const google = { providerId: 'google', accountId: `google-${generateId()}` }
+    await deleteOwnAccount(
+      await signUp(`google-gone.${generateId()}@deleted-identity.test`, google)
+    )
+    const legacy = await insertUser(`legacy.${generateId()}@deleted-identity.test`)
+    const laterRequest = {}
+    await linkAccount(legacy, google)
+
+    await ensureUserStatsExists(legacy, laterRequest)
+    await handleNewSignInAccount({ userId: legacy, ...google }, laterRequest)
+
+    expect(await statsOf(legacy)).toEqual({ freeTierEligible: true, limit: getFreeTierLimit() })
+  })
+
   it('records only sign-in identities, as keyed digests holding no address or subject', async () => {
     const email = `digest.${generateId()}@deleted-identity.test`
     const githubSubject = `gh-${generateId()}`

apps/sim/lib/users/deleted-identity.test.ts

@@ -0,0 +1,29 @@
+/**
+ * Recording a deleted account's identities is an anti-abuse write inside the
+ * holder's erasure transaction. It must never become a way to block the erasure.
+ */
+import { db } from '@sim/db'
+import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing/mocks/database.mock'
+import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing/mocks/env-flags.mock'
+import { schemaMock } from '@sim/testing/mocks/schema.mock'
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { recordDeletedAccountIdentities } from '@/lib/users/deleted-identity'
+
+describe('recordDeletedAccountIdentities', () => {
+  beforeEach(() => {
+    resetDbChainMock()
+    setEnvFlags({ isBillingEnabled: true })
+    queueTableRows(schemaMock.user, [{ email: 'ada@example.com' }])
+  })
+
+  afterEach(() => {
+    resetEnvFlagsMock()
+  })
+
+  it('never fails the deletion when the identity write fails', async () => {
+    dbChainMockFns.onConflictDoUpdate.mockRejectedValueOnce(new Error('lock timeout'))
+
+    await expect(recordDeletedAccountIdentities(db, 'user-1')).resolves.toBeUndefined()
+    expect(dbChainMockFns.onConflictDoUpdate).toHaveBeenCalled()
+  })
+})

apps/sim/lib/users/deleted-identity.ts

@@ -2,6 +2,7 @@ import { db } from '@sim/db'
 import { account, deletedAccountIdentity, user } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { hmacSha256Hex } from '@sim/security/hmac'
+import { getErrorMessage } from '@sim/utils/errors'
 import { and, eq, gt, inArray, lte, sql } from 'drizzle-orm'
 import { isSignInProviderAllowed, SIGN_IN_PROVIDER_IDS } from '@/lib/auth/constants'
 import { env } from '@/lib/core/config/env'
@@ -138,34 +139,47 @@ async function loadSignInIdentity(executor: DbOrTx, userId: string): Promise<Sig
  * Records the sign-in identities of an account being deleted at its holder's
  * request. Runs inside the deletion transaction, before the `user` row and its
  * `account` rows cascade away, so the record and the erasure commit together.
+ *
+ * The write runs in a savepoint and never fails the deletion: an anti-abuse
+ * record must not become a way to block an erasure request. On failure the
+ * savepoint rolls back, nothing is recorded, and the deletion proceeds.
  */
 export async function recordDeletedAccountIdentities(tx: DbOrTx, userId: string): Promise<void> {
   if (!isBillingEnabled) return
 
-  const hashes = identityHashes(await loadSignInIdentity(tx, userId))
-  if (hashes.length === 0) return
-
-  await tx
-    .delete(deletedAccountIdentity)
-    .where(
-      inArray(
-        deletedAccountIdentity.identityHash,
-        tx
-          .select({ identityHash: deletedAccountIdentity.identityHash })
-          .from(deletedAccountIdentity)
-          .where(lte(deletedAccountIdentity.expiresAt, sql`now()`))
-          .limit(EXPIRED_PURGE_BATCH)
-      )
-    )
-
-  const expiresAt = new Date(Date.now() + DELETED_IDENTITY_RETENTION_MS)
-  await tx
-    .insert(deletedAccountIdentity)
-    .values(hashes.map((identityHash) => ({ identityHash, expiresAt })))
-    .onConflictDoUpdate({
-      target: deletedAccountIdentity.identityHash,
-      set: { deletedAt: sql`now()`, expiresAt },
+  try {
+    await tx.transaction(async (savepoint) => {
+      const hashes = identityHashes(await loadSignInIdentity(savepoint, userId))
+      if (hashes.length === 0) return
+
+      await savepoint
+        .delete(deletedAccountIdentity)
+        .where(
+          inArray(
+            deletedAccountIdentity.identityHash,
+            savepoint
+              .select({ identityHash: deletedAccountIdentity.identityHash })
+              .from(deletedAccountIdentity)
+              .where(lte(deletedAccountIdentity.expiresAt, sql`now()`))
+              .limit(EXPIRED_PURGE_BATCH)
+          )
+        )
+
+      const expiresAt = new Date(Date.now() + DELETED_IDENTITY_RETENTION_MS)
+      await savepoint
+        .insert(deletedAccountIdentity)
+        .values(hashes.map((identityHash) => ({ identityHash, expiresAt })))
+        .onConflictDoUpdate({
+          target: deletedAccountIdentity.identityHash,
+          set: { deletedAt: sql`now()`, expiresAt },
+        })
+    })
+  } catch (error) {
+    logger.warn('Could not record deleted-account identities; continuing the deletion', {
+      userId,
+      error: getErrorMessage(error),
     })
+  }
 }
 
 /**
@@ -196,15 +210,20 @@ export async function isDeletedAccountIdentity(identity: SignInIdentity): Promis
 }
 
 /**
- * Whether an existing user's email, or any social sign-in already linked to it,
- * belonged to an account deleted within the retention window. Used when the
- * user's stats row is created, which in an OAuth signup happens after its first
- * sign-in account is committed. Fails open like {@link isDeletedAccountIdentity}.
+ * Whether an existing user's identities belonged to an account deleted within
+ * the retention window: always its email, and its linked social sign-ins only
+ * when `includeSignIns` is set, which callers do only for a user created in the
+ * current signup request (a sign-in linked later never withholds anything).
+ * Fails open like {@link isDeletedAccountIdentity}.
  */
-export async function userHasDeletedAccountIdentity(userId: string): Promise<boolean> {
+export async function userHasDeletedAccountIdentity(
+  userId: string,
+  { includeSignIns }: { includeSignIns: boolean }
+): Promise<boolean> {
   if (!isBillingEnabled) return false
   try {
-    return await isDeletedAccountIdentity(await loadSignInIdentity(db, userId))
+    const identity = await loadSignInIdentity(db, userId)
+    return await isDeletedAccountIdentity(includeSignIns ? identity : { email: identity.email })
   } catch (error) {
     logger.error('Deleted-identity lookup failed; granting the free allowance', { userId, error })
     return false