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

* 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.

* fix(auth): raise the cache ceilings to a memory backstop

getSessionCookieCacheVersion feeds Better Auth's session.cookieCache.version,
so these are read on every session read, not just create/refresh. At max: 1000
a busy instance could exceed the live key set inside the 60s TTL and start
evicting early — never a wrong answer (a miss is one indexed lookup, exactly
the pre-cache behaviour) but a hit-rate cliff on a hot path.

Entries are a few dozen bytes, so headroom is nearly free: orgs 1k -> 20k,
users 10k -> 100k. That is single-digit MB at worst and puts the ceiling far
above any plausible per-instance working set, leaving it as the memory backstop
it was meant to be.

* docs(rules): write down the caching decision tree

The lifecycle-map-vs-TTL-cache distinction, the ceiling-as-backstop sizing, and
the fetchMethod-unless-you-need-a-hang-deadline call each took real digging to
settle. Recording them so the next cache does not re-derive the same answers —
or re-introduce the unbounded tenant-keyed Map this branch just removed.

Notes the two non-obvious traps behind that: ttl alone does not bound memory
without a ceiling, and React cache() is a no-op in Trigger workers, so a gate
that looks free on a settings page is uncached and per-block on the executor.

.claude/rules/sim-caching.md

@@ -0,0 +1,99 @@
+---
+paths:
+  - "apps/sim/lib/**/*.ts"
+  - "apps/sim/providers/**/*.ts"
+  - "apps/sim/executor/**/*.ts"
+  - "apps/sim/tools/**/*.ts"
+---
+
+# In-Process Caching
+
+**Never hand-roll TTL arithmetic.** `lru-cache` is a direct dependency of `apps/sim` and owns
+expiry, the ceiling, and — through `fetchMethod` — request coalescing. A
+`Map` plus `Date.now() - entry.fetchedAt < TTL` re-implements all three, badly.
+
+## First decide whether it is a cache at all
+
+Most module-level `Map`s in this codebase are **not** caches, and forcing them into one is worse
+than leaving them alone.
+
+| Shape | Key dies when | Right tool |
+| --- | --- | --- |
+| **Lifecycle map** — `activeStreams`, `pendingChildRuns`, `memoryStreams`, `handlerRegistry` | the tracked thing ends, and the code deletes it there | plain `Map`. No TTL, no ceiling. |
+| **TTL cache** — a remote read keyed by tenant (org id, user id, workspace id) | time passes | `LRUCache` |
+
+A lifecycle map's key space is unbounded and that is fine, because every key has a defined death.
+Adding a TTL to one introduces an expiry that races the lifecycle. Adding a ceiling silently drops
+live state.
+
+## TTL caches: always set `max`
+
+```ts
+const policyCache = new LRUCache<string, ResolvedSessionPolicy>({
+  max: 20_000,
+  ttl: SESSION_POLICY_CACHE_TTL_MS,
+})
+```
+
+`ttl` alone does **not** bound memory. Without `ttlAutopurge` (itself expensive — one timer per
+entry) an expired entry lingers until something touches its key or the ceiling evicts it. `max` is
+what actually caps the process, which is why a tenant-keyed `Map` grew for the life of the process
+before this rule existed.
+
+**The ceiling is a memory backstop, not an operating limit.** Exceeding it makes the LRU evict
+*inside* the TTL, so each miss becomes one more read — never a wrong answer, it degrades to exactly
+the pre-cache behavior, but it is a hit-rate cliff on whatever path the cache sits on. Entries are
+tens of bytes, so set the cap far above any plausible per-instance working set within the TTL
+window and let it stay a backstop.
+
+**Reads test `!== undefined`, not truthiness**, whenever the value can be `false`, `0`, or `null`.
+`if (cached)` on a cached `false` re-queries on every single call, for exactly the tenants the
+cache exists to protect.
+
+## Async read-through: prefer `fetchMethod`
+
+`fetchMethod` + `cache.fetch(key)` gives TTL, coalescing (concurrent callers share one promise),
+and eviction-on-rejection (`noDeleteOnFetchRejection` defaults to `false`) in one primitive. Reach
+for it before composing anything yourself.
+
+**The one reason to compose instead: a hung producer.** `fetchMethod` has no settle deadline, and
+the app pool sets no `statement_timeout` (`packages/db/db.ts` sets only `connect_timeout` /
+`idle_timeout`, neither of which bounds a query already in flight). Where a wedged read would hold
+every caller for the whole TTL, wrap `coalesceLocally` from `@/lib/concurrency/singleflight` around
+a read-through `LRUCache` instead — it evicts and rejects at its deadline. See
+`lib/api-key/byok-entitlement.ts`, and `lib/oauth/credential-service.ts` for the same shape.
+
+Do **not** build a house wrapper over `lru-cache`. Call sites differ in ways a thin helper cannot
+hold (synchronous memoization with `updateAgeOnGet` in `providers/client-cache.ts`, per-entry TTLs
+in `lib/auth/security-policy.ts`), so a wrapper covering the common case just adds a fourth pattern.
+
+## Cache the gate, never the credential
+
+Entitlements, plans, and policies tolerate bounded staleness **in the safe direction** — a lapsed
+organization keeping its own provider key for another minute costs a little metering and charges
+nobody wrongly. Key material does not: revocation has to be immediate, so
+`getBYOKKey` reads key rows fresh on every call and caches only the entitlement around them.
+
+An outage must not be cached as a negative answer. A resolver that maps a failed read to `false`
+makes an outage indistinguishable from a real lapse, so give it an `onError: 'throw'` option and
+write the cache only on the success path — see `resolveOrganizationPlan`.
+
+**Where a human is waiting, read fresh.** Keep two entry points rather than one cached function:
+the settings surfaces and management use cases must not tell an organization that just upgraded
+that it still lacks a plan, while the execution path underneath can serve from cache
+(`isOrganizationBYOKEntitled` vs `isOrganizationBYOKEntitledCached`).
+
+## React `cache()` does nothing in a worker
+
+`cache()` is request-scoped. Workflows run in Trigger.dev workers, which have no React request
+scope, so a `cache()`-wrapped gate that looks free on a settings page is uncached and per-block on
+the execution path. Anything reached from the executor needs a real cache — see
+`.claude/rules/sim-architecture.md`'s app/worker runtime boundary.
+
+## Invalidation
+
+Add a per-key invalidator only when the code that mutates the value runs in the **same process**
+that reads it. `invalidateSessionPolicyCache` works because the route writing the policy is the one
+serving the reads. An entitlement change arriving on a Stripe webhook lands in one process while
+the readers are per-worker, so an invalidator there would imply an immediacy it cannot deliver —
+the TTL is the real mechanism, and the absence of an invalidator should say so.

CLAUDE.md

@@ -472,6 +472,22 @@ describe('my route', () => {
 
 Use `@sim/testing` mocks/factories over local test data.
 
+## Caching
+
+Never hand-roll TTL arithmetic — a `Map` plus `Date.now() - fetchedAt < TTL` re-implements expiry,
+the ceiling, and coalescing badly. Use `lru-cache` (a direct dependency), and always set `max`:
+`ttl` alone does not bound memory, so a tenant-keyed cache without a ceiling grows for the life of
+the process. Prefer `fetchMethod` + `cache.fetch(key)` for async read-through — it gives TTL,
+coalescing, and eviction-on-rejection in one primitive — and compose `coalesceLocally` around an
+`LRUCache` only when a hung producer would otherwise wedge callers for the whole TTL.
+
+First check the thing is a cache at all: a lifecycle map whose entry is deleted when the tracked
+thing ends (`activeStreams`, `pendingChildRuns`) is a plain `Map`, and giving it a TTL or a ceiling
+introduces an expiry that races the lifecycle. Cache the gate, never the credential — entitlements
+tolerate bounded staleness in the safe direction, key material must stay fresh so revocation is
+immediate. Full decision tree, sizing, `!== undefined` reads, and the invalidation rule are in
+`.claude/rules/sim-caching.md`.
+
 ## Utils Rules
 
 - Never create `utils.ts` for single consumer - inline it

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,24 @@ 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 read, 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.
+ *
+ * The ceiling is a memory backstop, not an operating limit. `getSessionCookieCacheVersion`
+ * feeds Better Auth's `session.cookieCache.version`, so these are read on every session
+ * read — if the live key set ever exceeded the cap, LRU would start evicting inside the
+ * TTL and each miss becomes one indexed lookup. That degrades to exactly the behaviour
+ * before any caching existed (never a wrong answer), but it is a hit-rate cliff on a hot
+ * path, so the cap is set far above any plausible per-instance working set in a 60s window.
+ * Entries are a few dozen bytes, so the headroom costs single-digit MB at worst.
+ */
+const versionCache = new LRUCache<string, number>({
+  max: 20_000,
+  ttl: SECURITY_POLICY_VERSION_CACHE_TTL_MS,
+})
 
 /**
  * Resolves the org's security-policy version — the shared monotonic counter
@@ -36,9 +49,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 +59,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 +75,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: 100_000,
+  ttl: SECURITY_POLICY_VERSION_CACHE_TTL_MS,
+})
 
 /**
  * Negative (non-member) membership results use a much shorter TTL than
@@ -78,7 +99,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 +130,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 +140,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,20 @@ 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.
+ *
+ * The ceiling is a memory backstop rather than an operating limit: exceeding it
+ * only costs an extra indexed lookup per miss, which is what happened before
+ * any caching existed, but it would be a hit-rate cliff on a warm path. Entries
+ * are a few dozen bytes, so the headroom is nearly free.
+ */
+const policyCache = new LRUCache<string, ResolvedSessionPolicy>({
+  max: 20_000,
+  ttl: SESSION_POLICY_CACHE_TTL_MS,
+})
 
 const NO_POLICY: ResolvedSessionPolicy = {
   maxSessionHours: null,
@@ -46,9 +55,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 +74,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', {