From b483feae8bf9346b5e39e8b0860b1e9edaaef20b Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 1 Jun 2026 18:00:30 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=91=20refactor:=20Scope=20Role=20Cache?= =?UTF-8?q?=20Keys=20by=20Isolation=20Context=20(#13454)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ROLES cache is a single process-wide store, but role documents are per-tenant (unique index { name, tenantId }). getRoleByName checked the cache by role name BEFORE the tenant-scoped DB read, so a warm entry written under one tenant's context was served to another tenant — leaking that tenant's permission bits into the other's authorization decisions. Scope every ROLES cache key with scopedCacheKey(), which appends the active tenantId from the AsyncLocalStorage tenant context. It is a no-op when no tenant context is set (or under runAsSystem), so single-tenant deployments behave exactly as before. Adds role.cache.spec.ts with a real Map-backed cache: two tenants sharing a role name receive their own permissions, the cache key is tenant-scoped, the same-tenant fast path still avoids a second DB read, and single-tenant mode still uses the unscoped key. --- .../src/methods/role.cache.spec.ts | 141 ++++++++++++++++++ packages/data-schemas/src/methods/role.ts | 22 +-- 2 files changed, 154 insertions(+), 9 deletions(-) create mode 100644 packages/data-schemas/src/methods/role.cache.spec.ts diff --git a/packages/data-schemas/src/methods/role.cache.spec.ts b/packages/data-schemas/src/methods/role.cache.spec.ts new file mode 100644 index 0000000000..512c47ea88 --- /dev/null +++ b/packages/data-schemas/src/methods/role.cache.spec.ts @@ -0,0 +1,141 @@ +import mongoose from 'mongoose'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import { PermissionTypes, Permissions } from 'librechat-data-provider'; +import type { IRole } from '..'; +import { createRoleMethods } from './role'; +import { createModels } from '../models'; +import { tenantStorage } from '../config/tenantContext'; + +jest.mock('~/config/winston', () => ({ + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +})); + +/** + * Real Map-backed cache. A jest.fn mock can only assert which key was passed; + * a real store reproduces the actual collision: when two tenants write under the + * same key, the second read serves the first tenant's value. + */ +function createMapCache() { + const store = new Map(); + return { + store, + get: async (k: string): Promise => store.get(k), + set: async (k: string, v: unknown): Promise => { + store.set(k, v); + }, + del: async (k: string): Promise => { + store.delete(k); + }, + }; +} + +const TENANT_A = 'tenant-aaaaaaaaaaaaaaaaaaaa'; +const TENANT_B = 'tenant-bbbbbbbbbbbbbbbbbbbb'; +const ROLE_NAME = 'EDITOR'; + +let mongoServer: MongoMemoryServer; +let Role: mongoose.Model; +let cache: ReturnType; +let getRoleByName: ReturnType['getRoleByName']; + +function runAs(tenantId: string, fn: () => Promise): Promise { + return tenantStorage.run({ tenantId }, fn); +} + +function usePromptsPermission(role: IRole | null | undefined): boolean | undefined { + const permissions = (role as unknown as { permissions?: Record> }) + ?.permissions; + return permissions?.[PermissionTypes.PROMPTS]?.[Permissions.USE]; +} + +async function seedRole(tenantId: string, useValue: boolean): Promise { + await runAs(tenantId, async () => { + await new Role({ + name: ROLE_NAME, + permissions: { [PermissionTypes.PROMPTS]: { [Permissions.USE]: useValue } }, + }).save(); + }); +} + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + createModels(mongoose); + Role = mongoose.models.Role as mongoose.Model; +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); +}); + +beforeEach(async () => { + await Role.deleteMany({}); + cache = createMapCache(); + const methods = createRoleMethods(mongoose, { getCache: () => cache }); + getRoleByName = methods.getRoleByName; +}); + +describe('getRoleByName cache is scoped to the active tenant', () => { + it('does not serve one tenant a cached role belonging to another tenant', async () => { + await seedRole(TENANT_A, true); + await seedRole(TENANT_B, false); + + const roleA = await runAs(TENANT_A, () => getRoleByName(ROLE_NAME)); + expect(usePromptsPermission(roleA)).toBe(true); + expect(cache.store.size).toBeGreaterThan(0); + + const roleB = await runAs(TENANT_B, () => getRoleByName(ROLE_NAME)); + expect(usePromptsPermission(roleB)).toBe(false); + + const roleAAgain = await runAs(TENANT_A, () => getRoleByName(ROLE_NAME)); + expect(usePromptsPermission(roleAAgain)).toBe(true); + }); + + it('appends the tenant id to the cache key', async () => { + await seedRole(TENANT_A, true); + await seedRole(TENANT_B, false); + + await runAs(TENANT_A, () => getRoleByName(ROLE_NAME)); + await runAs(TENANT_B, () => getRoleByName(ROLE_NAME)); + + expect(cache.store.has(`${ROLE_NAME}:${TENANT_A}`)).toBe(true); + expect(cache.store.has(`${ROLE_NAME}:${TENANT_B}`)).toBe(true); + expect(cache.store.has(ROLE_NAME)).toBe(false); + }); + + it('serves the cached value within the same tenant without a second DB read', async () => { + await seedRole(TENANT_A, true); + + const first = await runAs(TENANT_A, () => getRoleByName(ROLE_NAME)); + expect(usePromptsPermission(first)).toBe(true); + + const findOneSpy = jest.spyOn(Role, 'findOne'); + const second = await runAs(TENANT_A, () => getRoleByName(ROLE_NAME)); + expect(usePromptsPermission(second)).toBe(true); + expect(findOneSpy).not.toHaveBeenCalled(); + findOneSpy.mockRestore(); + }); + + it('uses the unscoped key when no tenant context is active (single-tenant)', async () => { + const previousDefault = process.env.DEFAULT_TENANT_ID; + delete process.env.DEFAULT_TENANT_ID; + try { + await seedRole(TENANT_A, true); + + const role = await getRoleByName(ROLE_NAME); + expect(usePromptsPermission(role)).toBe(true); + expect(cache.store.has(ROLE_NAME)).toBe(true); + expect(cache.store.has(`${ROLE_NAME}:${TENANT_A}`)).toBe(false); + } finally { + if (previousDefault === undefined) { + delete process.env.DEFAULT_TENANT_ID; + } else { + process.env.DEFAULT_TENANT_ID = previousDefault; + } + } + }); +}); diff --git a/packages/data-schemas/src/methods/role.ts b/packages/data-schemas/src/methods/role.ts index f0ffdb1700..ab3cce196f 100644 --- a/packages/data-schemas/src/methods/role.ts +++ b/packages/data-schemas/src/methods/role.ts @@ -7,6 +7,7 @@ import { } from 'librechat-data-provider'; import type { Model } from 'mongoose'; import type { IRole, IUser } from '~/types'; +import { scopedCacheKey } from '~/config/tenantContext'; import logger from '~/config/winston'; const systemRoleValues = new Set(Object.values(SystemRoles)); @@ -94,7 +95,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol const cache = deps.getCache?.(CacheKeys.ROLES); try { if (cache) { - const cachedRole = await cache.get(roleName); + const cachedRole = await cache.get(scopedCacheKey(roleName)); if (cachedRole) { return cachedRole as IRole; } @@ -109,12 +110,12 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol if (!role && systemRoleValues.has(roleName)) { const newRole = await new Role(roleDefaults[roleName as keyof typeof roleDefaults]).save(); if (cache) { - await cache.set(roleName, newRole); + await cache.set(scopedCacheKey(roleName), newRole); } return newRole.toObject() as IRole; } if (cache) { - await cache.set(roleName, role); + await cache.set(scopedCacheKey(roleName), role); } return role as unknown as IRole; } catch (error) { @@ -135,9 +136,12 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol .exec(); if (cache) { if (updates.name && updates.name !== roleName) { - await Promise.all([cache.set(roleName, null), cache.set(updates.name, role)]); + await Promise.all([ + cache.set(scopedCacheKey(roleName), null), + cache.set(scopedCacheKey(updates.name), role), + ]); } else { - await cache.set(roleName, role); + await cache.set(scopedCacheKey(roleName), role); } } return role as unknown as IRole; @@ -296,7 +300,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol const cache = deps.getCache?.(CacheKeys.ROLES); const updatedRole = await Role.findOne({ name: roleName }).select('-__v').lean().exec(); if (cache) { - await cache.set(roleName, updatedRole); + await cache.set(scopedCacheKey(roleName), updatedRole); } logger.info(`Updated role '${roleName}' and removed old schema fields`); @@ -366,7 +370,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol const cache = deps.getCache?.(CacheKeys.ROLES); if (cache) { const updatedRole = await Role.findById(role._id).lean().exec(); - await cache.set(role.name, updatedRole); + await cache.set(scopedCacheKey(role.name), updatedRole); } migratedCount++; @@ -418,7 +422,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol try { const cache = deps.getCache?.(CacheKeys.ROLES); if (cache) { - await cache.set(role.name, role.toObject()); + await cache.set(scopedCacheKey(role.name), role.toObject()); } } catch (cacheError) { logger.error(`[createRoleByName] cache set failed for "${role.name}":`, cacheError); @@ -454,7 +458,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol // Setting null evicts the stale document. getRoleByName treats falsy cached // values as a miss and falls through to the DB, so this does not provide // negative caching — it only prevents serving the pre-deletion document. - await cache.set(roleName, null); + await cache.set(scopedCacheKey(roleName), null); } } catch (cacheError) { logger.error(`[deleteRoleByName] cache invalidation failed for "${roleName}":`, cacheError);