From dc0771308aa7a32430c5d4bec04f389db492feb0 Mon Sep 17 00:00:00 2001
From: Ajay Ghanwat <ghanwat.ajay@workroot.in>
Date: Tue, 4 Aug 2026 17:04:19 +0530
Subject: [PATCH] fix(authz): eliminate cache-key collision in
 cachedPermissionStore

The scope-prefix concatenation cacheKey used a bare U+FFFD separator with
no escaping, so an adversarial subject/tenant id containing that character
could collide with a different subject/tenant pair and leak cached roles
across tenants. Switch to JSON.stringify([scopeKey, subjectId]) for an
unambiguous key.

Also replace the untested key.endsWith() substring sweep used to
invalidate a subject across all tenants on a global write with an
explicit bySubject index, and add test coverage for both the collision
and the cross-tenant invalidation sweep.
---
 packages/authz/src/store.ts              | 37 ++++++++++++++++++------
 packages/authz/test/store-cached.test.ts | 16 ++++++++++
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/packages/authz/src/store.ts b/packages/authz/src/store.ts
index 9ba7c5ee..a8848814 100644
--- a/packages/authz/src/store.ts
+++ b/packages/authz/src/store.ts
@@ -113,17 +113,22 @@ export function cachedPermissionStore(
 ): CachedPermissionStore {
   const ttlMs = options.ttlMs ?? 5_000;
   const max = options.max ?? 1_000;
-  const entries = new Map<string, { at: number; value: SubjectAssignments }>();
+  const entries = new Map<string, { at: number; value: SubjectAssignments; subjectId: string }>();
+  const bySubject = new Map<string, Set<string>>();
 
-  const cacheKey = (subjectId: string, scope?: AuthzScope) => `${scopeKey(scope)}�${subjectId}`;
+  const cacheKey = (subjectId: string, scope?: AuthzScope) =>
+    JSON.stringify([scopeKey(scope), subjectId]);
   const drop = (subjectId: string, scope?: AuthzScope) => {
-    entries.delete(cacheKey(subjectId, scope));
     // A global write changes what every tenant sees for that subject.
     if (scopeKey(scope) === "") {
-      for (const key of [...entries.keys()]) {
-        if (key.endsWith(`�${subjectId}`)) entries.delete(key);
-      }
+      const keys = bySubject.get(subjectId);
+      if (keys) for (const key of keys) entries.delete(key);
+      bySubject.delete(subjectId);
+      return;
     }
+    const key = cacheKey(subjectId, scope);
+    entries.delete(key);
+    bySubject.get(subjectId)?.delete(key);
   };
 
   return {
@@ -132,8 +137,19 @@ export function cachedPermissionStore(
       const hit = entries.get(key);
       if (hit && Date.now() - hit.at < ttlMs) return hit.value;
       const value = await inner.assignmentsFor(subjectId, scope);
-      if (entries.size >= max) entries.delete(entries.keys().next().value!);
-      entries.set(key, { at: Date.now(), value });
+      if (entries.size >= max) {
+        const oldestKey = entries.keys().next().value!;
+        const oldest = entries.get(oldestKey);
+        entries.delete(oldestKey);
+        if (oldest) bySubject.get(oldest.subjectId)?.delete(oldestKey);
+      }
+      entries.set(key, { at: Date.now(), value, subjectId });
+      let keys = bySubject.get(subjectId);
+      if (!keys) {
+        keys = new Set();
+        bySubject.set(subjectId, keys);
+      }
+      keys.add(key);
       return value;
     },
     async assignRole(subjectId, role, scope) {
@@ -154,7 +170,10 @@ export function cachedPermissionStore(
     },
     listSubjects: (scope) => inner.listSubjects(scope),
     invalidate: drop,
-    invalidateAll: () => entries.clear(),
+    invalidateAll: () => {
+      entries.clear();
+      bySubject.clear();
+    },
     size: () => entries.size,
   };
 }
diff --git a/packages/authz/test/store-cached.test.ts b/packages/authz/test/store-cached.test.ts
index 7114a571..23e5a7df 100644
--- a/packages/authz/test/store-cached.test.ts
+++ b/packages/authz/test/store-cached.test.ts
@@ -63,4 +63,20 @@ describe("cachedPermissionStore", () => {
     expect((await store.assignmentsFor("u1")).roles).toEqual([]);
     expect((await store.assignmentsFor("u1", { tenantId: "t1" })).roles).toEqual(["editor"]);
   });
+
+  test("a global write invalidates the subject in every tenant", async () => {
+    const inner = memoryPermissionStore();
+    const store = cachedPermissionStore(inner, { ttlMs: 60_000 });
+    await store.assignmentsFor("u1", { tenantId: "t1" }); // warm the tenant entry
+    await store.assignRole("u1", "editor"); // global write
+    expect((await store.assignmentsFor("u1", { tenantId: "t1" })).roles).toEqual(["editor"]);
+  });
+
+  test("cache keys cannot collide across subject/tenant boundaries", async () => {
+    const inner = memoryPermissionStore();
+    const store = cachedPermissionStore(inner, { ttlMs: 60_000 });
+    await inner.assignRole("b\uFFFDc", "editor", { tenantId: "a" });
+    expect((await store.assignmentsFor("b\uFFFDc", { tenantId: "a" })).roles).toEqual(["editor"]);
+    expect((await store.assignmentsFor("c", { tenantId: "a\uFFFDb" })).roles).toEqual([]);
+  });
 });