fix(realtime): order role-cache writes by read start, not write time

Two authorizations can start in one order and finish in the other, so the
decision written last can come from the older read. A join that authorized
before a revocation but returned after the sweep's denial would bury it,
handing the socket another full cache TTL of access. Every writer now takes a
monotonic ticket before it queries and yields only to a later-started read.

Author: waleedlatif1

apps/realtime/src/handlers/room-join-auth.ts

@@ -1,7 +1,7 @@
 import type { createLogger } from '@sim/logger'
 import { authorizeRoom } from '@sim/platform-authz/rooms'
 import type { RoomRef } from '@sim/realtime-protocol/rooms'
-import { recordRoomPermissionIfUnchanged, snapshotRoomPermission } from '@/middleware/permissions'
+import { beginRoomPermissionRead, commitRoomPermission } from '@/middleware/permissions'
 
 type Authorized = Awaited<ReturnType<typeof authorizeRoom>>
 
@@ -35,10 +35,10 @@ export async function resolveRoomJoinAuth(
   const { userId, room, action, logger, logLabel, messages, emitError } = params
 
   let authorized: Authorized
-  // Captured before the query so a decision recorded WHILE it was in flight (the
-  // access-revalidation sweep evicting this user) is never overwritten by this
-  // older read — see {@link recordRoomPermissionIfUnchanged}.
-  const snapshot = snapshotRoomPermission(userId, room)
+  // Taken before the query so this read is ordered against every other one: a
+  // decision from a later-started read (the access-revalidation sweep's denial)
+  // is never overwritten by this older result — see {@link commitRoomPermission}.
+  const readSeq = beginRoomPermissionRead()
   try {
     authorized = await authorizeRoom({ userId, room, action })
   } catch (error) {
@@ -54,7 +54,7 @@ export async function resolveRoomJoinAuth(
   // not authorizable here) resolved no permission at all and is deliberately not
   // recorded. A 404 records `null`: the resource is genuinely gone.
   if (authorized.status !== 400) {
-    recordRoomPermissionIfUnchanged(userId, room, authorized.workspacePermission, snapshot)
+    commitRoomPermission(userId, room, authorized.workspacePermission, readSeq)
   }
 
   if (!authorized.allowed) {

apps/realtime/src/middleware/permissions.test.ts

@@ -519,6 +519,43 @@ describe('verifyWorkflowAccess role-cache refresh', () => {
     expect(await resolveCurrentWorkflowRole(userId, workflowId, 'read')).toBe('write')
   })
 
+  it('does not let a stale join-time allow bury a sweep denial that started later', async () => {
+    const userId = 'vw-user-3'
+    const workflowId = 'vw-wf-3'
+
+    // Hand out a controllable promise per authorization call, so the two reads can be
+    // started in one order and settled in the other.
+    const settle: Array<(value: { allowed: boolean; workspacePermission: string | null }) => void> =
+      []
+    mockAuthorize.mockImplementation(
+      () =>
+        new Promise((resolve) => {
+          settle.push(resolve)
+        })
+    )
+
+    // A join-style verify starts FIRST (against pre-revocation state) and stalls.
+    const staleJoin = verifyWorkflowAccess(userId, workflowId)
+    for (let i = 0; i < 10 && settle.length < 1; i++) await Promise.resolve()
+    expect(settle).toHaveLength(1)
+
+    // The sweep's authorization starts AFTER it, and stalls too.
+    const sweep = resolveCurrentWorkflowRole(userId, workflowId, 'read')
+    for (let i = 0; i < 10 && settle.length < 2; i++) await Promise.resolve()
+    expect(settle).toHaveLength(2)
+
+    // The sweep's denial lands first, then the older join's allow. Ordering by WRITE
+    // time would let the join bury the denial and hand the socket another full TTL of
+    // access; ordering by read start keeps the denial in force.
+    settle[1]({ allowed: false, workspacePermission: null })
+    expect(await sweep).toBeNull()
+    settle[0]({ allowed: true, workspacePermission: 'write' })
+    await staleJoin
+
+    mockAuthorize.mockRejectedValue(new Error('must not re-query'))
+    expect(await resolveCurrentWorkflowRole(userId, workflowId, 'read')).toBeNull()
+  })
+
   it('does not let a stale in-flight resolution overwrite a fresher verify decision', async () => {
     const userId = 'vw-user-2'
     const workflowId = 'vw-wf-2'

apps/realtime/src/middleware/permissions.ts

@@ -102,9 +102,34 @@ const MAX_ROLE_CACHE_ENTRIES = 5_000
 interface CachedRole {
   /** Authoritative workspace role, or `null` when the user has no access. */
   role: string | null
+  /**
+   * The {@link beginRoomPermissionRead} ticket of the query this decision came
+   * from — i.e. when its DB read STARTED, not when it was written.
+   */
+  readSeq: number
   expiresAt: number
 }
 
+/**
+ * Monotonic ticket counter establishing a total order over authorization READS.
+ *
+ * Ordering by write time is not sufficient: two readers can start their queries in
+ * one order and finish in the other, so the decision written last can be derived
+ * from the older read. A join that authorized before a revocation but returned
+ * after the sweep's denial would then win — leaving a revoked user with another
+ * full TTL of access. Every writer therefore takes a ticket before it queries and
+ * a decision only yields to one from a strictly later-started read.
+ */
+let roleReadSeqCounter = 0
+
+/**
+ * Takes a read ticket. Call immediately BEFORE issuing an authorization query, and
+ * pass the result to {@link commitRoomPermission} once it returns.
+ */
+export function beginRoomPermissionRead(): number {
+  return ++roleReadSeqCounter
+}
+
 /**
  * Per-pod cache of authoritative workspace roles, keyed by
  * `${userId}:${roomName(room)}`.
@@ -142,19 +167,31 @@ function purgeExpiredRoles(now: number): void {
   }
 }
 
-/**
- * Records a freshly-read authoritative decision into the role cache. Every
- * successful DB read of a user's workspace role goes through this — including
- * the join-time {@link verifyWorkflowAccess} — so a stale cached revocation
- * never outlives a newer authoritative read (e.g. a re-granted user re-joining
- * within the TTL of the sweep's recorded `null`).
- */
-function recordRoleDecision(key: string, role: string | null): void {
+function recordRoleDecision(key: string, role: string | null, readSeq: number): void {
   const now = Date.now()
   if (roleCache.size >= MAX_ROLE_CACHE_ENTRIES) {
     purgeExpiredRoles(now)
   }
-  roleCache.set(key, { role, expiresAt: now + ROLE_REVALIDATION_TTL_MS })
+  roleCache.set(key, { role, readSeq, expiresAt: now + ROLE_REVALIDATION_TTL_MS })
+}
+
+/**
+ * Commits a freshly-read authoritative decision, unless a decision from a
+ * strictly later-started read is already cached — in which case that one stands
+ * and is returned instead.
+ *
+ * Every successful DB read of a user's workspace role goes through this: the
+ * cached resolver, the join-time {@link verifyWorkflowAccess}, and the room join
+ * authorizer. That is what keeps a stale cached revocation from outliving a newer
+ * authoritative read (a re-granted user re-joining inside the sweep's recorded
+ * `null` TTL) while equally keeping a stale ALLOW from burying the sweep's fresher
+ * denial. Returns the decision that ends up in force.
+ */
+function commitRoleDecision(key: string, role: string | null, readSeq: number): string | null {
+  const existing = roleCache.get(key)
+  if (existing !== undefined && existing.readSeq > readSeq) return existing.role
+  recordRoleDecision(key, role, readSeq)
+  return role
 }
 
 /**
@@ -195,19 +232,13 @@ async function resolveRoleUncached(
   room: RoomRef,
   fallbackRole: string
 ): Promise<string | null> {
-  const entryBeforeQuery = roleCache.get(key)
+  const readSeq = beginRoomPermissionRead()
   try {
     const role = await readAuthoritativeRoomPermission(userId, room)
-    // A fresh authoritative read (e.g. a join-time verifyWorkflowAccess) may
-    // have recorded a decision while this query was in flight. That write is
-    // newer than this query's read snapshot, so prefer it instead of
-    // overwriting it with a potentially stale result.
-    const entryAfterQuery = roleCache.get(key)
-    if (entryAfterQuery !== undefined && entryAfterQuery !== entryBeforeQuery) {
-      return entryAfterQuery.role
-    }
-    recordRoleDecision(key, role)
-    return role
+    // Yields only to a decision from a later-STARTED read (see commitRoleDecision).
+    // Comparing write order instead would let a join whose authorize began before
+    // this one — but returned after it — bury this result.
+    return commitRoleDecision(key, role, readSeq)
   } catch (error) {
     logger.warn(
       `Failed to re-validate role for user ${userId} on ${roomName(room)}; using last known role`,
@@ -291,50 +322,35 @@ export function peekRoomPermission(userId: string, room: RoomRef): string | null
 }
 
 /**
- * Records an authoritative decision read outside this module (the room join
+ * Commits an authoritative decision read outside this module (the room join
  * authorizer) into the shared cache, so the sweep and the per-frame gates start
- * warm and a re-granted user is never held out by a stale cached revocation.
+ * warm on the room a socket just joined.
  *
- * Unconditional: the caller's read is taken as the newest word. Use
- * {@link recordRoomPermissionIfUnchanged} when the read was slow enough that a
- * newer decision could have landed meanwhile.
+ * `readSeq` must come from a {@link beginRoomPermissionRead} taken BEFORE the
+ * caller's query: the decision is discarded if a later-started read already
+ * committed one, so neither a stale ALLOW can bury the sweep's fresher denial nor
+ * a stale revocation outlive a re-grant.
  */
-export function recordRoomPermission(
+export function commitRoomPermission(
   userId: string,
   room: RoomRef,
-  permission: PermissionType | null
+  permission: PermissionType | null,
+  readSeq: number
 ): void {
-  recordRoleDecision(roleCacheKey(userId, room), permission)
-}
-
-/** Opaque token identifying the cache entry a caller observed before its query. */
-export type RoomPermissionSnapshot = object | undefined
-
-/** Captures the current cache entry for a (user, room), for {@link recordRoomPermissionIfUnchanged}. */
-export function snapshotRoomPermission(userId: string, room: RoomRef): RoomPermissionSnapshot {
-  return roleCache.get(roleCacheKey(userId, room))
+  commitRoleDecision(roleCacheKey(userId, room), permission, readSeq)
 }
 
 /**
- * Records an authoritative decision only if no other decision landed since
- * `snapshot` was taken.
- *
- * Without this, a join whose authorization query started before a revocation but
- * returned after the sweep recorded that revocation would overwrite it with its own
- * stale "allowed", handing the socket another full TTL of access. Entry identity
- * (not a timestamp) is the comparison, mirroring the in-flight ordering guard the
- * cached resolver already applies to its own queries — so it is exact regardless of
- * how many writes land inside one millisecond.
+ * Records a decision as the newest word, taking its read ticket at write time.
+ * For callers that have just observed the authoritative state with no query of
+ * their own to order against.
  */
-export function recordRoomPermissionIfUnchanged(
+export function recordRoomPermission(
   userId: string,
   room: RoomRef,
-  permission: PermissionType | null,
-  snapshot: RoomPermissionSnapshot
+  permission: PermissionType | null
 ): void {
-  const key = roleCacheKey(userId, room)
-  if (roleCache.get(key) !== snapshot) return
-  recordRoleDecision(key, permission)
+  recordRoleDecision(roleCacheKey(userId, room), permission, beginRoomPermissionRead())
 }
 
 /**
@@ -376,6 +392,9 @@ export async function verifyWorkflowAccess(
   userId: string,
   workflowId: string
 ): Promise<{ hasAccess: boolean; role?: string; workspaceId?: string }> {
+  // Taken before the reads below, so this decision yields to any authorization
+  // that started later (e.g. the sweep's denial) instead of by write order.
+  const readSeq = beginRoomPermissionRead()
   try {
     const workflowData = await db
       .select({
@@ -398,9 +417,10 @@ export async function verifyWorkflowAccess(
       action: 'read',
     })
 
-    recordRoleDecision(
+    commitRoleDecision(
       roleCacheKey(userId, { type: ROOM_TYPES.WORKFLOW, id: workflowId }),
-      authorization.allowed ? (authorization.workspacePermission ?? null) : null
+      authorization.allowed ? (authorization.workspacePermission ?? null) : null,
+      readSeq
     )
 
     if (!authorization.allowed || !authorization.workspacePermission) {