fix(service): don't cache permission snapshots that raced an invalidation

An InvalidateUser/InvalidateChannel/InvalidateAll landing between
getOrPopulate's DB read and its cache store was silently overwritten by
the stale snapshot, serving revoked permissions for up to permCacheTTL
(30s). Guard the cache write with a generation counter bumped by every
invalidation; a populate that lost the race returns its snapshot for the
current request but caches nothing (security scan 2026-07-22, F6).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Server/service/permission.go

@@ -31,6 +31,10 @@ type PermissionService struct {
 
 	mu    sync.RWMutex
 	cache map[int64]*cachedPerms // keyed by userID
+	// gen is bumped by every Invalidate* call. getOrPopulate snapshots it before
+	// its DB read and refuses to cache if it changed, so an invalidation that
+	// races a populate can't be lost (F6).
+	gen uint64
 }
 
 // NewPermissionService creates a PermissionService backed by the given DB.
@@ -101,6 +105,7 @@ func (s *PermissionService) GetRoleForUser(userID int64) (*db.Role, error) {
 func (s *PermissionService) InvalidateUser(userID int64) {
 	s.mu.Lock()
 	delete(s.cache, userID)
+	s.gen++
 	s.mu.Unlock()
 }
 
@@ -110,6 +115,7 @@ func (s *PermissionService) InvalidateUser(userID int64) {
 func (s *PermissionService) InvalidateChannel(_ int64) {
 	s.mu.Lock()
 	s.cache = make(map[int64]*cachedPerms)
+	s.gen++
 	s.mu.Unlock()
 }
 
@@ -117,6 +123,7 @@ func (s *PermissionService) InvalidateChannel(_ int64) {
 func (s *PermissionService) InvalidateAll() {
 	s.mu.Lock()
 	s.cache = make(map[int64]*cachedPerms)
+	s.gen++
 	s.mu.Unlock()
 }
 
@@ -135,6 +142,7 @@ func (s *PermissionService) getOrPopulate(userID int64) *cachedPerms {
 		s.mu.RUnlock()
 		return cp
 	}
+	startGen := s.gen
 	s.mu.RUnlock()
 
 	// Populate.
@@ -156,7 +164,13 @@ func (s *PermissionService) getOrPopulate(userID int64) *cachedPerms {
 	}
 
 	s.mu.Lock()
-	s.cache[userID] = cp
+	// F6: only cache if no invalidation raced our DB read. If gen moved, an
+	// InvalidateUser/InvalidateChannel/InvalidateAll landed after we snapshotted
+	// it, so this snapshot may already be stale — return it for this one request
+	// but don't poison the cache with it for permCacheTTL.
+	if s.gen == startGen {
+		s.cache[userID] = cp
+	}
 	s.mu.Unlock()
 	return cp
 }

Server/service/permission_test.go

@@ -190,3 +190,75 @@ func TestHasChannelPerm_UnknownUserReturnsFalse(t *testing.T) {
 		t.Fatal("expected false for unknown user")
 	}
 }
+
+// raceHookStore wraps a real *db.DB and fires a hook right after the role read
+// inside getOrPopulate, letting a test deterministically inject a concurrent
+// invalidation into the populate's read→store window.
+type raceHookStore struct {
+	*db.DB
+	onGetRole func()
+}
+
+func (s *raceHookStore) GetRoleForUser(userID int64) (*db.Role, error) {
+	r, err := s.DB.GetRoleForUser(userID)
+	if s.onGetRole != nil {
+		s.onGetRole()
+	}
+	return r, err
+}
+
+// TestGetOrPopulate_InvalidationDuringPopulateNotLost locks F6: an invalidation
+// that races a populate (landing after the DB read but before the cache store)
+// must not be silently overwritten by the stale snapshot. Otherwise a just-revoked
+// permission keeps being served for up to permCacheTTL (30s). All three
+// invalidation entry points bump the generation, so each is locked separately.
+func TestGetOrPopulate_InvalidationDuringPopulateNotLost(t *testing.T) {
+	cases := []struct {
+		name       string
+		invalidate func(*PermissionService)
+	}{
+		{"InvalidateUser", func(s *PermissionService) { s.InvalidateUser(1) }},
+		{"InvalidateChannel", func(s *PermissionService) { s.InvalidateChannel(10) }},
+		{"InvalidateAll", func(s *PermissionService) { s.InvalidateAll() }},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			database := newTestDB(t)
+			seedRole(t, database, &db.Role{
+				ID:          permissions.MemberRoleID,
+				Name:        "member",
+				Permissions: permissions.SendMessages | permissions.ReadMessages,
+				Position:    1,
+			})
+			seedUserRole(t, database, 1, permissions.MemberRoleID)
+			seedChannel(t, database, &db.Channel{ID: 10, Name: "general", Type: "text"})
+
+			store := &raceHookStore{DB: database}
+			svc := NewPermissionService(store, permissions.NewChecker(database))
+
+			fired := false
+			store.onGetRole = func() {
+				if fired {
+					return
+				}
+				fired = true
+				// Admin demotes the role (removes SendMessages) and invalidates,
+				// racing this populate between its role read and its cache store.
+				if _, err := database.Exec(`UPDATE roles SET permissions = ? WHERE id = ?`,
+					permissions.ReadMessages, permissions.MemberRoleID); err != nil {
+					t.Errorf("demote role: %v", err)
+				}
+				tc.invalidate(svc)
+			}
+
+			// This populate reads the pre-demotion perms; the racing invalidation
+			// must stop that stale snapshot from being cached.
+			svc.HasChannelPerm(1, 10, permissions.SendMessages)
+
+			// A fresh check must re-read the DB and see the revoked permission.
+			if svc.HasChannelPerm(1, 10, permissions.SendMessages) {
+				t.Fatal("revoked SendMessages served from a stale snapshot; a populate that races an invalidation must not be cached")
+			}
+		})
+	}
+}