diff --git a/docs/plans/2026-08-04-authz-permissions-implementation.md b/docs/plans/2026-08-04-authz-permissions-implementation.md index 8294afaf..e50606d1 100644 --- a/docs/plans/2026-08-04-authz-permissions-implementation.md +++ b/docs/plans/2026-08-04-authz-permissions-implementation.md @@ -809,6 +809,26 @@ describe("cachedPermissionStore", () => { expect(store.size()).toBeLessThanOrEqual(2); }); + 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 + // Global roles are visible inside every tenant, so the cached t1 entry + // must not survive this 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 }); + // Naive "scope + separator + subject" concatenation makes these two pairs + // produce the same key, serving one subject the other's permissions. + await inner.assignRole("b�c", "editor", { tenantId: "a" }); + expect((await store.assignmentsFor("b�c", { tenantId: "a" })).roles).toEqual(["editor"]); + expect((await store.assignmentsFor("c", { tenantId: "a�b" })).roles).toEqual([]); + }); + test("scoped and global reads cache separately", async () => { const inner = memoryPermissionStore(); const store = cachedPermissionStore(inner, { ttlMs: 60_000 }); @@ -853,15 +873,25 @@ export function cachedPermissionStore( const max = options.max ?? 1_000; const entries = new Map(); - const cacheKey = (subjectId: string, scope?: AuthzScope) => `${scopeKey(scope)}�${subjectId}`; + // Subject and tenant ids are unconstrained strings, so the key must be + // unambiguous: concatenating around a separator lets ("a", "bc") and + // ("ab", "c") collide, which would serve one subject another's + // permissions. JSON encoding escapes the components. + const cacheKey = (subjectId: string, scope?: AuthzScope) => + JSON.stringify([scopeKey(scope), subjectId]); + // Track subjects separately rather than pattern-matching key strings, so a + // global write can find every tenant entry without substring guesswork. + const bySubject = new Map>(); 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); - } + for (const key of bySubject.get(subjectId) ?? []) entries.delete(key); + bySubject.delete(subjectId); + return; } + const key = cacheKey(subjectId, scope); + entries.delete(key); + bySubject.get(subjectId)?.delete(key); }; return { @@ -870,8 +900,15 @@ 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!); + if (entries.size >= max) { + const oldest = entries.keys().next().value!; + entries.delete(oldest); + for (const keys of bySubject.values()) keys.delete(oldest); + } entries.set(key, { at: Date.now(), value }); + let keys = bySubject.get(subjectId); + if (!keys) bySubject.set(subjectId, (keys = new Set())); + keys.add(key); return value; }, async assignRole(subjectId, role, scope) { @@ -892,7 +929,10 @@ export function cachedPermissionStore( }, listSubjects: (scope) => inner.listSubjects(scope), invalidate: drop, - invalidateAll: () => entries.clear(), + invalidateAll: () => { + entries.clear(); + bySubject.clear(); + }, size: () => entries.size, }; }