fix(auth): bound the three unbounded session-policy caches

security-policy.ts and session-policy.ts are read from Better Auth's session
create and update hooks, so they run on every session validation. All three
caches were plain Maps with a hand-rolled 'Date.now() - fetchedAt < TTL' read
and no ceiling: entries were released only by an explicit invalidate, so they
grew for the life of the process. membershipCache is the sharpest of the three
because it is keyed by user, not organization — one entry per user who ever
authenticated on that instance.

Move all three to LRUCache, already a direct dependency and the pattern
copilot/entitlements.ts and providers/client-cache.ts use. The library owns the
TTL and the ceiling; every existing invalidate* keeps working unchanged.

Two things to preserve, both now pinned by tests:
- membership results keep their asymmetric TTL (a non-member result expires far
  sooner, so a user who joins through a path this codebase never sees cannot
  dodge the new org's policy). Expressed as membershipCacheTtlMs rather than an
  inline ternary, since it is a security property and not a tuning knob.
- reads test '!== undefined', because a version is a number and a membership is
  nullable — a truthiness check would treat both as a miss.

security-policy.ts had no test file; adds one covering caching, invalidation,
failure fallbacks and the TTL asymmetry.

Author: icecrasher321

apps/sim/lib/auth/security-policy.test.ts

@@ -0,0 +1,122 @@
+/**
+ * @vitest-environment node
+ */
+import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
+import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
+import {
+  getMemberOrganizationId,
+  getSecurityPolicyVersion,
+  invalidateMembershipCache,
+  invalidateSecurityPolicyVersionCache,
+  membershipCacheTtlMs,
+  NEGATIVE_MEMBERSHIP_CACHE_TTL_MS,
+  SECURITY_POLICY_VERSION_CACHE_TTL_MS,
+} from '@/lib/auth/security-policy'
+
+afterAll(resetDbChainMock)
+
+describe('membershipCacheTtlMs', () => {
+  /**
+   * The asymmetry is a security property: a cached `null` lets a user dodge a
+   * new org's policy until it expires, and they can join through paths this
+   * codebase never sees (Better Auth SSO JIT provisioning). A positive result
+   * only changes through leave/transfer, which invalidate explicitly.
+   */
+  it('expires a non-member result far sooner than a member one', () => {
+    expect(membershipCacheTtlMs('org-1')).toBe(SECURITY_POLICY_VERSION_CACHE_TTL_MS)
+    expect(membershipCacheTtlMs(null)).toBe(NEGATIVE_MEMBERSHIP_CACHE_TTL_MS)
+    expect(membershipCacheTtlMs(null)).toBeLessThan(membershipCacheTtlMs('org-1'))
+  })
+})
+
+describe('getMemberOrganizationId', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+    invalidateMembershipCache('user-1')
+  })
+
+  it('serves a repeat lookup from cache instead of re-reading membership', async () => {
+    dbChainMockFns.limit.mockResolvedValue([{ organizationId: 'org-1' }])
+
+    await expect(getMemberOrganizationId('user-1')).resolves.toBe('org-1')
+    await expect(getMemberOrganizationId('user-1')).resolves.toBe('org-1')
+
+    expect(dbChainMockFns.limit).toHaveBeenCalledTimes(1)
+  })
+
+  /** `null` is a real answer, not a miss — re-querying every time would defeat the cache. */
+  it('caches a non-member result too', async () => {
+    dbChainMockFns.limit.mockResolvedValue([])
+
+    await expect(getMemberOrganizationId('user-1')).resolves.toBeNull()
+    await expect(getMemberOrganizationId('user-1')).resolves.toBeNull()
+
+    expect(dbChainMockFns.limit).toHaveBeenCalledTimes(1)
+  })
+
+  it('re-reads after an explicit invalidation', async () => {
+    dbChainMockFns.limit.mockResolvedValue([{ organizationId: 'org-1' }])
+    await expect(getMemberOrganizationId('user-1')).resolves.toBe('org-1')
+
+    invalidateMembershipCache('user-1')
+    dbChainMockFns.limit.mockResolvedValue([{ organizationId: 'org-2' }])
+
+    await expect(getMemberOrganizationId('user-1')).resolves.toBe('org-2')
+    expect(dbChainMockFns.limit).toHaveBeenCalledTimes(2)
+  })
+
+  it('treats a failed read as org-less without caching it', async () => {
+    dbChainMockFns.limit.mockRejectedValueOnce(new Error('database unavailable'))
+    await expect(getMemberOrganizationId('user-1')).resolves.toBeNull()
+
+    dbChainMockFns.limit.mockResolvedValue([{ organizationId: 'org-1' }])
+    await expect(getMemberOrganizationId('user-1')).resolves.toBe('org-1')
+  })
+
+  it('returns null for an absent user without touching the database', async () => {
+    await expect(getMemberOrganizationId(null)).resolves.toBeNull()
+    expect(dbChainMockFns.limit).not.toHaveBeenCalled()
+  })
+})
+
+describe('getSecurityPolicyVersion', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+    invalidateSecurityPolicyVersionCache('org-1')
+  })
+
+  /** The value is a number, so a truthiness check would re-query on every read. */
+  it('serves a repeat lookup from cache', async () => {
+    dbChainMockFns.limit.mockResolvedValue([{ version: 7 }])
+
+    await expect(getSecurityPolicyVersion('org-1')).resolves.toBe(7)
+    await expect(getSecurityPolicyVersion('org-1')).resolves.toBe(7)
+
+    expect(dbChainMockFns.limit).toHaveBeenCalledTimes(1)
+  })
+
+  it('re-reads after an explicit invalidation', async () => {
+    dbChainMockFns.limit.mockResolvedValue([{ version: 7 }])
+    await expect(getSecurityPolicyVersion('org-1')).resolves.toBe(7)
+
+    invalidateSecurityPolicyVersionCache('org-1')
+    dbChainMockFns.limit.mockResolvedValue([{ version: 8 }])
+
+    await expect(getSecurityPolicyVersion('org-1')).resolves.toBe(8)
+  })
+
+  it('falls back to the default version without caching a failed read', async () => {
+    dbChainMockFns.limit.mockRejectedValueOnce(new Error('database unavailable'))
+    await expect(getSecurityPolicyVersion('org-1')).resolves.toBe(1)
+
+    dbChainMockFns.limit.mockResolvedValue([{ version: 9 }])
+    await expect(getSecurityPolicyVersion('org-1')).resolves.toBe(9)
+  })
+
+  it('returns the default for an org-less session without touching the database', async () => {
+    await expect(getSecurityPolicyVersion(null)).resolves.toBe(1)
+    expect(dbChainMockFns.limit).not.toHaveBeenCalled()
+  })
+})

apps/sim/lib/auth/security-policy.ts

@@ -2,6 +2,7 @@ import { db } from '@sim/db'
 import { member, organization } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { eq } from 'drizzle-orm'
+import { LRUCache } from 'lru-cache'
 
 const logger = createLogger('SecurityPolicy')
 
@@ -16,12 +17,16 @@ export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000
 
 const DEFAULT_VERSION = 1
 
-interface VersionCacheEntry {
-  version: number
-  fetchedAt: number
-}
-
-const versionCache = new Map<string, VersionCacheEntry>()
+/**
+ * Read on every session create and refresh, keyed by organization, so an
+ * unbounded `Map` grew for the life of the process. `LRUCache` supplies both
+ * the TTL and a ceiling; `invalidateSecurityPolicyVersionCache` still evicts
+ * by key. Reads must test `!== undefined`, since the value is a number.
+ */
+const versionCache = new LRUCache<string, number>({
+  max: 1000,
+  ttl: SECURITY_POLICY_VERSION_CACHE_TTL_MS,
+})
 
 /**
  * Resolves the org's security-policy version — the shared monotonic counter
@@ -36,9 +41,7 @@ export async function getSecurityPolicyVersion(
   if (!organizationId) return DEFAULT_VERSION
 
   const cached = versionCache.get(organizationId)
-  if (cached && Date.now() - cached.fetchedAt < SECURITY_POLICY_VERSION_CACHE_TTL_MS) {
-    return cached.version
-  }
+  if (cached !== undefined) return cached
 
   try {
     const [row] = await db
@@ -48,7 +51,7 @@ export async function getSecurityPolicyVersion(
       .limit(1)
 
     const version = row?.version ?? DEFAULT_VERSION
-    versionCache.set(organizationId, { version, fetchedAt: Date.now() })
+    versionCache.set(organizationId, version)
     return version
   } catch (error) {
     logger.error('Failed to resolve security policy version; using default', {
@@ -64,12 +67,22 @@ export function invalidateSecurityPolicyVersionCache(organizationId: string): vo
   versionCache.delete(organizationId)
 }
 
+/**
+ * Wraps the value in an object because `LRUCache` cannot store `null`, and a
+ * non-member is exactly what `null` means here.
+ *
+ * Keyed by user rather than organization, so this was the least bounded cache
+ * of the three: one entry per user who ever authenticated on the process,
+ * released only by an explicit join/leave invalidation.
+ */
 interface MembershipCacheEntry {
   organizationId: string | null
-  fetchedAt: number
 }
 
-const membershipCache = new Map<string, MembershipCacheEntry>()
+const membershipCache = new LRUCache<string, MembershipCacheEntry>({
+  max: 10_000,
+  ttl: SECURITY_POLICY_VERSION_CACHE_TTL_MS,
+})
 
 /**
  * Negative (non-member) membership results use a much shorter TTL than
@@ -78,7 +91,17 @@ const membershipCache = new Map<string, MembershipCacheEntry>()
  * ones outside this codebase (Better Auth SSO JIT provisioning). Positive
  * results change only through leave/transfer, which invalidate explicitly.
  */
-const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000
+export const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000
+
+/**
+ * The TTL a membership result is cached under. Named rather than inlined at the
+ * `set` call because the asymmetry is a security property, not a tuning knob:
+ * collapsing it to one value would silently restore the dodge the short
+ * negative TTL exists to close.
+ */
+export function membershipCacheTtlMs(organizationId: string | null): number {
+  return organizationId ? SECURITY_POLICY_VERSION_CACHE_TTL_MS : NEGATIVE_MEMBERSHIP_CACHE_TTL_MS
+}
 
 /** Drops the cached membership for a user (call when they join/leave an org). */
 export function invalidateMembershipCache(userId: string): void {
@@ -99,12 +122,7 @@ export async function getMemberOrganizationId(
   if (!userId) return null
 
   const cached = membershipCache.get(userId)
-  if (cached) {
-    const ttl = cached.organizationId
-      ? SECURITY_POLICY_VERSION_CACHE_TTL_MS
-      : NEGATIVE_MEMBERSHIP_CACHE_TTL_MS
-    if (Date.now() - cached.fetchedAt < ttl) return cached.organizationId
-  }
+  if (cached) return cached.organizationId
 
   try {
     const [row] = await db
@@ -114,7 +132,7 @@ export async function getMemberOrganizationId(
       .limit(1)
 
     const organizationId = row?.organizationId ?? null
-    membershipCache.set(userId, { organizationId, fetchedAt: Date.now() })
+    membershipCache.set(userId, { organizationId }, { ttl: membershipCacheTtlMs(organizationId) })
     return organizationId
   } catch (error) {
     logger.error('Failed to resolve org membership; treating session as org-less', {

apps/sim/lib/auth/session-policy.ts

@@ -3,6 +3,7 @@ import type { SessionPolicySettings } from '@sim/db/schema'
 import { organization } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { eq, sql } from 'drizzle-orm'
+import { LRUCache } from 'lru-cache'
 import { MIN_IDLE_TIMEOUT_HOURS } from '@/lib/api/contracts/organization'
 import { getMemberOrganizationId, invalidateMembershipCache } from '@/lib/auth/security-policy'
 import { isOrganizationFeatureEntitled } from '@/lib/billing/core/subscription'
@@ -20,12 +21,15 @@ export interface ResolvedSessionPolicy {
   idleTimeoutHours: number | null
 }
 
-interface PolicyCacheEntry {
-  policy: ResolvedSessionPolicy
-  fetchedAt: number
-}
-
-const policyCache = new Map<string, PolicyCacheEntry>()
+/**
+ * Read on every session create and refresh, keyed by organization, so an
+ * unbounded `Map` grew for the life of the process. `LRUCache` supplies both
+ * the TTL and a ceiling; `invalidateSessionPolicyCache` still evicts by key.
+ */
+const policyCache = new LRUCache<string, ResolvedSessionPolicy>({
+  max: 1000,
+  ttl: SESSION_POLICY_CACHE_TTL_MS,
+})
 
 const NO_POLICY: ResolvedSessionPolicy = {
   maxSessionHours: null,
@@ -46,9 +50,7 @@ export async function getSessionPolicy(
   if (!organizationId) return NO_POLICY
 
   const cached = policyCache.get(organizationId)
-  if (cached && Date.now() - cached.fetchedAt < SESSION_POLICY_CACHE_TTL_MS) {
-    return cached.policy
-  }
+  if (cached) return cached
 
   try {
     const [row] = await db
@@ -67,7 +69,7 @@ export async function getSessionPolicy(
           idleTimeoutHours: settings.idleTimeoutHours ?? null,
         }
       : NO_POLICY
-    policyCache.set(organizationId, { policy, fetchedAt: Date.now() })
+    policyCache.set(organizationId, policy)
     return policy
   } catch (error) {
     logger.error('Failed to resolve session policy; applying no policy', {