diff --git a/packages/api/src/app/service.spec.ts b/packages/api/src/app/service.spec.ts index b692aa6eb2..248d42dacd 100644 --- a/packages/api/src/app/service.spec.ts +++ b/packages/api/src/app/service.spec.ts @@ -327,6 +327,51 @@ describe('createAppConfigService', () => { expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); }); + + it('scopes the override cache key to the ALS tenant when no tenantId param is given', async () => { + const { tenantStorage } = jest.requireActual('@librechat/data-schemas'); + const deps = createDeps({ + getApplicableConfigs: jest + .fn() + .mockResolvedValue([{ priority: 10, overrides: { x: 1 }, isActive: true }]), + }); + const { getAppConfig } = createAppConfigService(deps); + + await tenantStorage.run({ tenantId: 'tenant-a' }, async () => + getAppConfig({ role: 'USER' }), + ); + + const overrideKey = [...deps._cache._store.keys()].find((k: string) => + k.includes('_OVERRIDE_:'), + ); + expect(overrideKey).toBe('app_config:_OVERRIDE_:tenant-a:USER'); + expect(overrideKey).not.toContain('__default__'); + }); + + it('does not serve one tenant a cached config built for another tenant', async () => { + const { tenantStorage, getTenantId } = jest.requireActual('@librechat/data-schemas'); + // Each tenant's DB overrides carry a marker derived from the active ALS tenant. + const deps = createDeps({ + getApplicableConfigs: jest + .fn() + .mockImplementation(async () => [ + { priority: 10, overrides: { whoami: getTenantId() }, isActive: true }, + ]), + }); + const { getAppConfig } = createAppConfigService(deps); + + const configA = (await tenantStorage.run({ tenantId: 'tenant-a' }, async () => + getAppConfig({ role: 'USER' }), + )) as TestConfig & { whoami?: string }; + const configB = (await tenantStorage.run({ tenantId: 'tenant-b' }, async () => + getAppConfig({ role: 'USER' }), + )) as TestConfig & { whoami?: string }; + + expect(configA.whoami).toBe('tenant-a'); + expect(configB.whoami).toBe('tenant-b'); + // A cache collision would short-circuit the second tenant's DB read. + expect(deps.getApplicableConfigs).toHaveBeenCalledTimes(2); + }); }); it('does not cache on buildPrincipals error — retries on next request', async () => { diff --git a/packages/api/src/app/service.ts b/packages/api/src/app/service.ts index 613c428fca..658056503f 100644 --- a/packages/api/src/app/service.ts +++ b/packages/api/src/app/service.ts @@ -73,7 +73,10 @@ export function _resetOverrideStrictCache(): void { } function overrideCacheKey(role?: string, userId?: string, tenantId?: string): string { - const tenant = tenantId || '__default__'; + // Fall back to the ALS tenant context before `__default__`: callers that rely on the + // tenant middleware (the common path) pass no explicit tenantId, so without this the + // entry is keyed under the shared `__default__` bucket and leaks across tenants. + const tenant = tenantId || getTenantId() || '__default__'; if (userId && role) { return `_OVERRIDE_:${tenant}:${role}:${userId}`; } @@ -174,16 +177,16 @@ export function createAppConfigService(deps: AppConfigServiceDeps) { return baseConfig; } - // Strict-isolation + no tenant (param or ALS) = pathological path (middleware bypass or - // unauthenticated startup). Pre-tenant calls use baseOnly:true; admin calls carry tenantId. - // If ALS has a tenant, Mongoose scopes queries to that tenant's overrides — must fall through. - // Not cached: the cache key doesn't include ALS context, so a cached __default__ entry would - // be served to later ALS-scoped calls that share the same param-derived key. + // Strict isolation + no tenant anywhere (neither param nor ALS) is pathological: a + // middleware bypass or an unauthenticated startup call. Pre-tenant calls should use + // baseOnly:true and admin calls carry an explicit tenantId. Return the base config + // without caching it under the shared `__default__` bucket. When ALS has a tenant, + // overrideCacheKey scopes the key to it, so we fall through and cache per-tenant. if (principals.length === 0 && !tenantId && !getTenantId() && isStrictOverrideMode()) { return baseConfig; } - if (!tenantId && isStrictOverrideMode() && !_warnedNoTenantInStrictMode) { + if (!tenantId && !getTenantId() && isStrictOverrideMode() && !_warnedNoTenantInStrictMode) { _warnedNoTenantInStrictMode = true; logger.warn( '[getAppConfig] No tenantId in strict mode — falling back to __default__. ' +