diff --git a/api/server/middleware/config/app.js b/api/server/middleware/config/app.js index fb5f89b229..cf02682637 100644 --- a/api/server/middleware/config/app.js +++ b/api/server/middleware/config/app.js @@ -17,7 +17,7 @@ const configMiddleware = async (req, res, next) => { }); try { - req.config = await getAppConfig(); + req.config = await getAppConfig({ tenantId: req.user?.tenantId }); next(); } catch (fallbackError) { logger.error('Fallback config middleware error:', fallbackError); diff --git a/packages/api/src/app/service.spec.ts b/packages/api/src/app/service.spec.ts index e5e076c8eb..b692aa6eb2 100644 --- a/packages/api/src/app/service.spec.ts +++ b/packages/api/src/app/service.spec.ts @@ -1,5 +1,5 @@ import type { AppConfig } from '@librechat/data-schemas'; -import { createAppConfigService } from './service'; +import { createAppConfigService, _resetOverrideStrictCache } from './service'; /** Extends AppConfig with mock fields used by merge behavior tests. */ interface TestConfig extends AppConfig { @@ -229,6 +229,124 @@ describe('createAppConfigService', () => { expect((config as TestConfig).x).toBe('admin-only'); }); + it('passes empty principals to getApplicableConfigs when buildPrincipals returns empty', async () => { + const deps = createDeps({ + getUserPrincipals: jest.fn().mockResolvedValue([]), + }); + const { getAppConfig } = createAppConfigService(deps); + + const config = await getAppConfig({ userId: 'uid1', role: 'USER' }); + + expect(deps.getUserPrincipals).toHaveBeenCalledWith({ userId: 'uid1', role: 'USER' }); + expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); + expect(config).toEqual(deps._baseConfig); + }); + + describe('strict mode (TENANT_ISOLATION_STRICT=true)', () => { + beforeEach(() => { + process.env.TENANT_ISOLATION_STRICT = 'true'; + _resetOverrideStrictCache(); + }); + afterEach(() => { + delete process.env.TENANT_ISOLATION_STRICT; + _resetOverrideStrictCache(); + }); + + it('skips DB query for empty principals without tenantId and does not cache', async () => { + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + const config = await getAppConfig(); + + expect(deps.getApplicableConfigs).not.toHaveBeenCalled(); + expect(config).toEqual(deps._baseConfig); + + const setCalls = deps._cache.set.mock.calls.filter( + ([key]: [string, unknown]) => key !== '_BASE_', + ); + expect(setCalls).toHaveLength(0); + }); + + it('queries DB when tenantId is present', async () => { + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + await getAppConfig({ tenantId: 'tenant-a' }); + + expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); + }); + + it('warns once when non-empty principals proceed without tenantId', async () => { + const { logger } = jest.requireActual('@librechat/data-schemas'); + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + await getAppConfig({ role: 'USER' }); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('No tenantId in strict mode')); + const warnCount = warnSpy.mock.calls.length; + + await getAppConfig({ role: 'ADMIN' }); + expect(warnSpy).toHaveBeenCalledTimes(warnCount); + + warnSpy.mockRestore(); + }); + + it('falls through to getApplicableConfigs when ALS has tenant context despite no tenantId param', async () => { + const { tenantStorage } = jest.requireActual('@librechat/data-schemas'); + const deps = createDeps({ + getApplicableConfigs: jest + .fn() + .mockResolvedValue([{ priority: 5, overrides: { restricted: true }, isActive: true }]), + }); + const { getAppConfig } = createAppConfigService(deps); + + const config = await tenantStorage.run({ tenantId: 'tenant-a' }, async () => + getAppConfig(), + ); + + expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); + expect((config as TestConfig).restricted).toBe(true); + }); + }); + + describe('non-strict mode (TENANT_ISOLATION_STRICT unset)', () => { + beforeEach(() => { + delete process.env.TENANT_ISOLATION_STRICT; + _resetOverrideStrictCache(); + }); + afterEach(() => { + _resetOverrideStrictCache(); + }); + + it('passes empty principals through to getApplicableConfigs', async () => { + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + await getAppConfig(); + + expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); + }); + }); + + it('does not cache on buildPrincipals error — retries on next request', async () => { + const deps = createDeps({ + getUserPrincipals: jest + .fn() + .mockRejectedValueOnce(new Error('transient')) + .mockResolvedValue([{ principalType: 'role', principalId: 'USER' }]), + }); + const { getAppConfig } = createAppConfigService(deps); + + const first = await getAppConfig({ userId: 'uid1', role: 'USER' }); + expect(first).toEqual(deps._baseConfig); + expect(deps.getApplicableConfigs).not.toHaveBeenCalled(); + + await getAppConfig({ userId: 'uid1', role: 'USER' }); + expect(deps.getUserPrincipals).toHaveBeenCalledTimes(2); + expect(deps.getApplicableConfigs).toHaveBeenCalledTimes(1); + }); + it('falls back to base config on getApplicableConfigs error', async () => { const deps = createDeps({ getApplicableConfigs: jest.fn().mockRejectedValue(new Error('DB down')), diff --git a/packages/api/src/app/service.ts b/packages/api/src/app/service.ts index 6c5d307709..952544f02c 100644 --- a/packages/api/src/app/service.ts +++ b/packages/api/src/app/service.ts @@ -1,11 +1,16 @@ import { PrincipalType } from 'librechat-data-provider'; -import { logger, mergeConfigOverrides, BASE_CONFIG_PRINCIPAL_ID } from '@librechat/data-schemas'; +import { + logger, + getTenantId, + mergeConfigOverrides, + BASE_CONFIG_PRINCIPAL_ID, +} from '@librechat/data-schemas'; import type { Types } from 'mongoose'; import type { AppConfig, IConfig } from '@librechat/data-schemas'; const BASE_CONFIG_KEY = '_BASE_'; -const DEFAULT_OVERRIDE_CACHE_TTL = 60_000; +export const DEFAULT_OVERRIDE_CACHE_TTL = 60_000; // ── Types ──────────────────────────────────────────────────────────── @@ -50,9 +55,9 @@ function isStrictOverrideMode(): boolean { return (_strictOverride ??= process.env.TENANT_ISOLATION_STRICT === 'true'); } -/** @internal Resets the cached strict-override flag. Exposed for test teardown only. */ let _warnedNoTenantInStrictMode = false; +/** @internal Resets the memoized strict-override flag and one-time no-tenantId warning gate. Exposed for test teardown only. */ export function _resetOverrideStrictCache(): void { _strictOverride = undefined; _warnedNoTenantInStrictMode = false; @@ -60,13 +65,6 @@ export function _resetOverrideStrictCache(): void { function overrideCacheKey(role?: string, userId?: string, tenantId?: string): string { const tenant = tenantId || '__default__'; - if (!tenantId && isStrictOverrideMode() && !_warnedNoTenantInStrictMode) { - _warnedNoTenantInStrictMode = true; - logger.warn( - '[overrideCacheKey] No tenantId in strict mode — falling back to __default__. ' + - 'This likely indicates a code path that bypasses the tenant context middleware.', - ); - } if (userId && role) { return `_OVERRIDE_:${tenant}:${role}:${userId}`; } @@ -168,8 +166,32 @@ export function createAppConfigService(deps: AppConfigServiceDeps) { } } + const principals = await buildPrincipals(role, userId).catch((error: unknown) => { + logger.error('[getAppConfig] Error building principals, falling back to base:', error); + return null; + }); + if (principals === null) { + 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. + if (principals.length === 0 && !tenantId && !getTenantId() && isStrictOverrideMode()) { + return baseConfig; + } + + if (!tenantId && isStrictOverrideMode() && !_warnedNoTenantInStrictMode) { + _warnedNoTenantInStrictMode = true; + logger.warn( + '[getAppConfig] No tenantId in strict mode — falling back to __default__. ' + + 'This likely indicates a code path that bypasses the tenant context middleware.', + ); + } + try { - const principals = await buildPrincipals(role, userId); const configs = await getApplicableConfigs(principals); if (configs.length === 0) {