docs: fix cache-key collision in the Task 4 plan snippet
The plan's cachedPermissionStore used scopeKey + U+FFFD + subjectId as a
cache key with no escaping, so ('a', 'b<sep>c') and ('a<sep>b', 'c') collide
and one subject is served another's permissions. Subject and tenant ids are
unconstrained strings, so nothing prevented it.
Key is now JSON-encoded, and the global-write sweep tracks keys per subject
instead of substring-matching. Adds the two regression tests that were
missing: cross-tenant invalidation on a global write, and key collision.
Ruled by the human as plan-mandated; source of truth amended so a re-run of
the plan does not reintroduce the defect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -809,6 +809,26 @@ describe("cachedPermissionStore", () => {
|
|||||||
expect(store.size()).toBeLessThanOrEqual(2);
|
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 () => {
|
test("scoped and global reads cache separately", async () => {
|
||||||
const inner = memoryPermissionStore();
|
const inner = memoryPermissionStore();
|
||||||
const store = cachedPermissionStore(inner, { ttlMs: 60_000 });
|
const store = cachedPermissionStore(inner, { ttlMs: 60_000 });
|
||||||
@@ -853,15 +873,25 @@ export function cachedPermissionStore(
|
|||||||
const max = options.max ?? 1_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 }>();
|
||||||
|
|
||||||
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", "b<sep>c") and
|
||||||
|
// ("a<sep>b", "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<string, Set<string>>();
|
||||||
const drop = (subjectId: string, scope?: AuthzScope) => {
|
const drop = (subjectId: string, scope?: AuthzScope) => {
|
||||||
entries.delete(cacheKey(subjectId, scope));
|
|
||||||
// A global write changes what every tenant sees for that subject.
|
// A global write changes what every tenant sees for that subject.
|
||||||
if (scopeKey(scope) === "") {
|
if (scopeKey(scope) === "") {
|
||||||
for (const key of [...entries.keys()]) {
|
for (const key of bySubject.get(subjectId) ?? []) entries.delete(key);
|
||||||
if (key.endsWith(`�${subjectId}`)) entries.delete(key);
|
bySubject.delete(subjectId);
|
||||||
}
|
return;
|
||||||
}
|
}
|
||||||
|
const key = cacheKey(subjectId, scope);
|
||||||
|
entries.delete(key);
|
||||||
|
bySubject.get(subjectId)?.delete(key);
|
||||||
};
|
};
|
||||||
|
|
||||||
return {
|
return {
|
||||||
@@ -870,8 +900,15 @@ export function cachedPermissionStore(
|
|||||||
const hit = entries.get(key);
|
const hit = entries.get(key);
|
||||||
if (hit && Date.now() - hit.at < ttlMs) return hit.value;
|
if (hit && Date.now() - hit.at < ttlMs) return hit.value;
|
||||||
const value = await inner.assignmentsFor(subjectId, scope);
|
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 });
|
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;
|
return value;
|
||||||
},
|
},
|
||||||
async assignRole(subjectId, role, scope) {
|
async assignRole(subjectId, role, scope) {
|
||||||
@@ -892,7 +929,10 @@ export function cachedPermissionStore(
|
|||||||
},
|
},
|
||||||
listSubjects: (scope) => inner.listSubjects(scope),
|
listSubjects: (scope) => inner.listSubjects(scope),
|
||||||
invalidate: drop,
|
invalidate: drop,
|
||||||
invalidateAll: () => entries.clear(),
|
invalidateAll: () => {
|
||||||
|
entries.clear();
|
||||||
|
bySubject.clear();
|
||||||
|
},
|
||||||
size: () => entries.size,
|
size: () => entries.size,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user