fix(auth): prefer a mid-flight published version over the default on read failure

Review catch. The success path re-checks the cache after its await so a
concurrent publish wins, but the catch path returned `DEFAULT_VERSION`
unconditionally.

The default is not the safe fallback it looks like — it is precisely the version
a never-bumped org carries. So a lookup that started before a revoke, and then
failed, could report `1` while this process already held the bumped counter:
the cookie-cache version would still MATCH pre-bump cookies and the
just-revoked session would keep being served from cache, which is the one thing
the bump exists to stop.

The catch now returns the cached value when one is present. It can only be
equal to or higher than the default, so it forces the same revalidation or more.

Author: waleedlatif1

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

@@ -76,7 +76,19 @@ describe('security policy', () => {
       expect(await getSecurityPolicyVersion(orgId)).toBe(7)
     })
 
-    it('falls back to the default when the read fails', async () => {
+    it('prefers a version published mid-flight over the default when the read fails', async () => {
+      const orgId = nextOrgId()
+      // A revoke/policy save publishes the bump while this read is in flight,
+      // then the read fails. Returning the default would reproduce the exact
+      // version a pre-bump cookie carries and keep it matching.
+      dbChainMockFns.limit.mockImplementationOnce(() => {
+        setSecurityPolicyVersion(orgId, 6)
+        throw new Error('connection reset')
+      })
+      expect(await getSecurityPolicyVersion(orgId)).toBe(6)
+    })
+
+    it('falls back to the default when the read fails and nothing is cached', async () => {
       dbChainMockFns.limit.mockImplementationOnce(() => {
         throw new Error('connection reset')
       })

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

@@ -66,9 +66,12 @@ const membershipCache = new LRUCache<string, MembershipCacheEntry>({
  * planned consumers): any feature that needs cached session cookies to
  * re-validate bumps this one counter.
  *
- * A failed read falls back to the default rather than the last known value.
- * That errs toward MORE revalidation, not less: a version that reads lower than
- * the stored one mismatches the cookie and forces a database session read.
+ * A failed read prefers whatever this process last knew over the default. The
+ * default is not the safe fallback it looks like: it is the version a
+ * never-bumped org carries, so returning it can MATCH a pre-bump cookie and
+ * keep a just-revoked session serving from the cookie cache. A retained or
+ * freshly published value can only be equal or higher, so it either forces the
+ * same revalidation or more of it.
  */
 export async function getSecurityPolicyVersion(
   organizationId: string | null | undefined
@@ -96,11 +99,11 @@ export async function getSecurityPolicyVersion(
     versionCache.set(organizationId, version)
     return version
   } catch (error) {
-    logger.error('Failed to resolve security policy version; using default', {
-      organizationId,
-      error,
-    })
-    return DEFAULT_VERSION
+    logger.error('Failed to resolve security policy version', { organizationId, error })
+    // A publish, or a concurrent read, may have landed while this one was in
+    // flight. Prefer it — the default could reproduce exactly the version a
+    // pre-bump cookie carries, leaving a revoked session on the cookie cache.
+    return versionCache.get(organizationId) ?? DEFAULT_VERSION
   }
 }