From f829aca9fb26921746530986e96525d2b4e33983 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 16 Aug 2026 22:30:46 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=A9=20fix:=20Align=20Tenant=20and=20MC?= =?UTF-8?q?P=20Configuration=20Resolution=20(#14904)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Align Tenant and MCP Configuration Resolution * fix: Preserve Operator-Owned MCP Entries * fix: Preserve Configuration Source Ownership * style: Normalize Middleware Import Order * fix: Preserve Process Server Precedence * test: Align Tenant-Aware E2E Setup --- .env.example | 4 + api/server/experimental.js | 18 ++- api/server/experimental.spec.js | 9 ++ api/server/index.js | 11 +- api/server/services/AuthService.js | 5 +- api/server/services/AuthService.spec.js | 31 ++++ e2e/playwright.config.mock.ts | 1 + packages/api/src/admin/config.handler.spec.ts | 78 ++++++++++ packages/api/src/admin/config.ts | 29 ++++ .../src/mcp/registry/MCPServersRegistry.ts | 22 ++- .../__tests__/MCPServersRegistry.test.ts | 29 ++++ .../api/src/middleware/preAuthTenant.spec.ts | 29 +++- packages/api/src/middleware/preAuthTenant.ts | 6 +- packages/data-provider/src/mcp.ts | 27 ++++ .../data-schemas/src/app/resolution.spec.ts | 147 ++++++++++++++++++ packages/data-schemas/src/app/resolution.ts | 73 ++++++++- 16 files changed, 499 insertions(+), 20 deletions(-) diff --git a/.env.example b/.env.example index 84906756e3..0613685b58 100644 --- a/.env.example +++ b/.env.example @@ -70,6 +70,10 @@ NO_INDEX=true # Defaulted to 1. TRUST_PROXY=1 +# Trust X-Tenant-Id on unauthenticated routes. Disabled by default. +# Enable only when a trusted reverse proxy strips any client-supplied value and sets its own. +# TRUST_TENANT_HEADER=false + # Minimum password length for user authentication # Default: 8 # Note: When using LDAP authentication, you may want to set this to 1 diff --git a/api/server/experimental.js b/api/server/experimental.js index f702297474..a259a8e31c 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -299,6 +299,18 @@ if (cluster.isMaster) { app.disable('x-powered-by'); app.set('trust proxy', trusted_proxy); + if (isEnabled(process.env.TRUST_TENANT_HEADER)) { + logger.warn( + '[Security] TRUST_TENANT_HEADER is active. Ensure your reverse proxy strips and sets ' + + 'X-Tenant-Id — untrusted clients must not be able to supply it directly.', + ); + } else if (isEnabled(process.env.TENANT_ISOLATION_STRICT)) { + logger.warn( + '[Security] TENANT_ISOLATION_STRICT is active while TRUST_TENANT_HEADER is disabled. ' + + 'Pre-authentication tenant headers will be ignored.', + ); + } + /** Seed database (idempotent) */ await runAsSystem(seedDatabase); @@ -421,8 +433,8 @@ if (cluster.isMaster) { app.use(capabilityContextMiddleware); /** Routes */ - app.use('/oauth', routes.oauth); - app.use('/api/auth', routes.auth); + app.use('/oauth', preAuthTenantMiddleware, routes.oauth); + app.use('/api/auth', preAuthTenantMiddleware, routes.auth); app.use('/api/admin', routes.adminAuth); app.use('/api/admin/skills', routes.adminSkills); app.use('/api/actions', routes.actions); @@ -444,7 +456,7 @@ if (cluster.isMaster) { app.use('/api/assistants', routes.assistants); app.use('/api/files', await routes.files.initialize()); app.use('/images/', createValidateImageRequest(appConfig.secureImageLinks), routes.staticRoute); - app.use('/api/share', routes.share); + app.use('/api/share', preAuthTenantMiddleware, routes.share); app.use('/api/roles', routes.roles); app.use('/api/agents', routes.agents); app.use('/api/banner', routes.banner); diff --git a/api/server/experimental.spec.js b/api/server/experimental.spec.js index 371f36b29f..2287011154 100644 --- a/api/server/experimental.spec.js +++ b/api/server/experimental.spec.js @@ -19,4 +19,13 @@ describe('Experimental server configuration', () => { /await runAsSystem\(async \(\) => \{\s+await performStartupChecks\(appConfig\);\s+await updateInterfacePerms/, ); }); + + it('matches the standard server pre-authentication tenant routes', () => { + expect(source).toContain("app.use('/oauth', preAuthTenantMiddleware, routes.oauth);"); + expect(source).toContain("app.use('/api/auth', preAuthTenantMiddleware, routes.auth);"); + expect(source).toContain( + "app.use('/api/config', preAuthTenantMiddleware, optionalJwtAuth, routes.config);", + ); + expect(source).toContain("app.use('/api/share', preAuthTenantMiddleware, routes.share);"); + }); }); diff --git a/api/server/index.js b/api/server/index.js index b6a483d7b7..6732f1eaf3 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -141,10 +141,15 @@ const startServer = async () => { app.disable('x-powered-by'); app.set('trust proxy', trusted_proxy); - if (isEnabled(process.env.TENANT_ISOLATION_STRICT)) { + if (isEnabled(process.env.TRUST_TENANT_HEADER)) { logger.warn( - '[Security] TENANT_ISOLATION_STRICT is active. Ensure your reverse proxy strips or sets ' + - 'the X-Tenant-Id header — untrusted clients must not be able to set it directly.', + '[Security] TRUST_TENANT_HEADER is active. Ensure your reverse proxy strips and sets ' + + 'X-Tenant-Id — untrusted clients must not be able to supply it directly.', + ); + } else if (isEnabled(process.env.TENANT_ISOLATION_STRICT)) { + logger.warn( + '[Security] TENANT_ISOLATION_STRICT is active while TRUST_TENANT_HEADER is disabled. ' + + 'Pre-authentication tenant headers will be ignored.', ); } diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index a4a59bb8b6..40595d3081 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -369,8 +369,9 @@ const registerUser = async (user, additionalData = {}) => { return { status: 200, message: genericVerificationMessage }; } - //determine if this is the first registered user (not counting anonymous_user) - const isFirstRegisteredUser = (await countUsers()) === 0; + // Only the first user in the unscoped, single-tenant deployment bootstraps ADMIN. + // Tenant administrators must be provisioned through a trusted administrative flow. + const isFirstRegisteredUser = !tenantId && (await countUsers()) === 0; const salt = bcrypt.genSaltSync(10); const newUserData = { diff --git a/api/server/services/AuthService.spec.js b/api/server/services/AuthService.spec.js index ce4dd517ff..4750a52cbd 100644 --- a/api/server/services/AuthService.spec.js +++ b/api/server/services/AuthService.spec.js @@ -1382,6 +1382,37 @@ describe('registerUser - allowedDomains admin-panel override', () => { expect(getAppConfig).toHaveBeenCalledWith({ tenantId: 'tenant-x' }); }); + it('does not bootstrap a tenant-scoped registrant as an administrator', async () => { + getTenantId.mockReturnValue('attacker-selected-tenant'); + createUser.mockResolvedValue({ _id: 'new-user', emailVerified: true }); + checkEmailConfig.mockReturnValue(false); + + await registerUser(validUser); + + expect(countUsers).not.toHaveBeenCalled(); + expect(createUser).toHaveBeenCalledWith( + expect.objectContaining({ role: 'USER' }), + undefined, + expect.any(Boolean), + true, + ); + }); + + it('preserves first-user administrator bootstrap without a tenant context', async () => { + createUser.mockResolvedValue({ _id: 'new-user', emailVerified: true }); + checkEmailConfig.mockReturnValue(false); + + await registerUser(validUser); + + expect(countUsers).toHaveBeenCalledTimes(1); + expect(createUser).toHaveBeenCalledWith( + expect.objectContaining({ role: 'ADMIN' }), + undefined, + expect.any(Boolean), + true, + ); + }); + it('should block registration when the resolved allowedDomains rejects the email', async () => { isEmailDomainAllowed.mockReturnValue(false); diff --git a/e2e/playwright.config.mock.ts b/e2e/playwright.config.mock.ts index 9b1762c0de..d215f09256 100644 --- a/e2e/playwright.config.mock.ts +++ b/e2e/playwright.config.mock.ts @@ -37,6 +37,7 @@ const chromiumChannel = process.env.E2E_CHROMIUM_CHANNEL || undefined; const vanillaOverrides = { TENANT_ISOLATION_STRICT: 'false', + TRUST_TENANT_HEADER: 'true', OPENAI_API_KEY: 'user_provided', OPENID_CLIENT_ID: '', OPENID_ISSUER: '', diff --git a/packages/api/src/admin/config.handler.spec.ts b/packages/api/src/admin/config.handler.spec.ts index 581bd6116a..1bc8be13c5 100644 --- a/packages/api/src/admin/config.handler.spec.ts +++ b/packages/api/src/admin/config.handler.spec.ts @@ -413,6 +413,49 @@ describe('createAdminConfigHandlers', () => { expect(res.statusCode).toBe(400); }); + it('rejects process-backed MCP servers in database overrides', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'user', principalId: 'u1' }, + body: { + overrides: { + mcpServers: { + injected: { type: 'stdio', command: '/bin/sh', args: ['-c', 'id'] }, + }, + }, + }, + }); + const res = mockRes(); + + await handlers.upsertConfigOverrides(req, res); + + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ + error: 'Process-backed MCP servers can only be configured in librechat.yaml', + }); + expect(deps.upsertConfig).not.toHaveBeenCalled(); + }); + + it('rejects process-backed MCP servers supplied through the runtime config alias', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'user', principalId: 'u1' }, + body: { + overrides: { + mcpConfig: { + injected: { command: '/bin/sh', args: ['-c', 'id'] }, + }, + }, + }, + }); + const res = mockRes(); + + await handlers.upsertConfigOverrides(req, res); + + expect(res.statusCode).toBe(400); + expect(deps.upsertConfig).not.toHaveBeenCalled(); + }); + it('strips permission fields from interface overrides but keeps UI fields', async () => { const { handlers, deps } = createHandlers({ upsertConfig: jest.fn().mockResolvedValue({ _id: 'c1', configVersion: 1 }), @@ -980,6 +1023,41 @@ describe('createAdminConfigHandlers', () => { expect(patchedFields['interface.modelSelect']).toBe(false); }); + it('rejects process-backed MCP server field patches', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'user', principalId: 'u1' }, + body: { + entries: [{ fieldPath: 'mcpServers.injected.command', value: '/bin/sh' }], + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ + error: 'Process-backed MCP servers can only be configured in librechat.yaml', + }); + expect(deps.patchConfigFields).not.toHaveBeenCalled(); + }); + + it('rejects process-backed MCP field patches through the runtime config alias', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'user', principalId: 'u1' }, + body: { + entries: [{ fieldPath: 'mcpConfig.injected.command', value: '/bin/sh' }], + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(400); + expect(deps.patchConfigFields).not.toHaveBeenCalled(); + }); + it('rejects array-valued Langfuse secret ancestors', async () => { const { handlers, deps } = createHandlers(); const req = mockReq({ diff --git a/packages/api/src/admin/config.ts b/packages/api/src/admin/config.ts index 6bb09f94c9..b6961eacd3 100644 --- a/packages/api/src/admin/config.ts +++ b/packages/api/src/admin/config.ts @@ -6,6 +6,9 @@ import { PrincipalModel, INTERFACE_PERMISSION_FIELDS, PERMISSION_SUB_KEYS, + hasProcessMCPServerConfig, + isProcessMCPServerConfig, + isProcessMCPServerField, } from 'librechat-data-provider'; import type { AppConfig, ConfigSection, IConfig, SystemCapability } from '@librechat/data-schemas'; import type { TCustomConfig } from 'librechat-data-provider'; @@ -31,6 +34,8 @@ const MAX_PATCH_ENTRIES = 100; const DEFAULT_PRIORITY = 10; const BASE_ONLY_OVERRIDE_SECTIONS = new Set(BASE_ONLY_CONFIG_SECTIONS); const BASE_PRINCIPAL_OVERRIDE_SECTIONS = new Set(BASE_PRINCIPAL_CONFIG_SECTIONS); +const PROCESS_MCP_CONFIG_ERROR = + 'Process-backed MCP servers can only be configured in librechat.yaml'; export function isValidFieldPath(path: string): boolean { return ( @@ -52,6 +57,19 @@ function isBaseOnlyFieldPath(fieldPath: string): boolean { return BASE_ONLY_OVERRIDE_SECTIONS.has(getTopLevelSection(fieldPath)); } +function isProcessMCPServerFieldPath(fieldPath: string, value: unknown): boolean { + const [section, _serverName, field] = fieldPath.split('.'); + if (section !== 'mcpServers' && section !== 'mcpConfig') { + return false; + } + if (field == null) { + return fieldPath === section + ? hasProcessMCPServerConfig(value) + : isProcessMCPServerConfig(value); + } + return isProcessMCPServerField(field) || (field === 'type' && value === 'stdio'); +} + /** * Returns true if `fieldPath` targets an interface permission field or permission sub-key. * @@ -501,6 +519,14 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(400).json({ error: 'overrides must be a plain object' }); } + const rawOverrides = overrides as Record; + if ( + hasProcessMCPServerConfig(rawOverrides.mcpServers) || + hasProcessMCPServerConfig(rawOverrides.mcpConfig) + ) { + return res.status(400).json({ error: PROCESS_MCP_CONFIG_ERROR }); + } + if (priority != null && (typeof priority !== 'number' || priority < 0)) { return res.status(400).json({ error: 'priority must be a non-negative number' }); } @@ -702,6 +728,9 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { .status(400) .json({ error: `Invalid or unsafe field path: ${entry.fieldPath}` }); } + if (isProcessMCPServerFieldPath(entry.fieldPath, entry.value)) { + return res.status(400).json({ error: PROCESS_MCP_CONFIG_ERROR }); + } if (isConfigSecretDescendantPath(entry.fieldPath)) { return res .status(400) diff --git a/packages/api/src/mcp/registry/MCPServersRegistry.ts b/packages/api/src/mcp/registry/MCPServersRegistry.ts index e91c0c8dcf..cdfff6b75a 100644 --- a/packages/api/src/mcp/registry/MCPServersRegistry.ts +++ b/packages/api/src/mcp/registry/MCPServersRegistry.ts @@ -1,6 +1,7 @@ import { Keyv } from 'keyv'; import { createHash } from 'crypto'; import { logger } from '@librechat/data-schemas'; +import { isProcessMCPServerConfig } from 'librechat-data-provider'; import type { IServerConfigsRepositoryInterface } from './ServerConfigsRepositoryInterface'; import type * as t from '~/mcp/types'; import { @@ -44,11 +45,11 @@ function resolveServerSource( * same-name base entry. The base's source is normally inherited so downstream * recovery routes to the base's storage tier. * - * SECURITY INVARIANT — a `'plugin'` base is the exception: its no-resolve + * SECURITY INVARIANT — a remote `'plugin'` base is the exception: its no-resolve * provenance must never transfer to an operator-authored override, or * `processMCPEnv` would stop resolving the operator's own `${VAR}` placeholders. - * The override supersedes the plugin (operator config outranks a plugin server), - * so it keeps its own trusted source instead. + * The override supersedes a remote plugin server, so it keeps its own trusted + * source instead. Process-backed base entries never reach this overlay path. */ function overlaySource( base: t.ParsedServerConfig, @@ -304,10 +305,11 @@ export class MCPServersRegistry { * getAllServerConfigs so list views and single-server lookups agree on * the same name: * 1. user-tier base entry wins absolutely over a config-tier candidate - * 2. healthy YAML/DB base wins over a failed (inspectionFailed) candidate - * 3. healthy candidate overlays its fields onto the base, preserving the + * 2. process-backed base entries win absolutely over a config-tier candidate + * 3. healthy YAML/DB base wins over a failed (inspectionFailed) candidate + * 4. healthy candidate overlays its fields onto the base, preserving the * base entry's source tag so downstream recovery routes correctly - * 4. with no base, the candidate is returned as-is (config-only server) + * 5. with no base, the candidate is returned as-is (config-only server) * * readThroughCache memoizes only the global YAML/DB lookup; the per-call * configServers candidate is tenant-scoped and is never cached, so a @@ -337,6 +339,7 @@ export class MCPServersRegistry { if (!candidate) return base; if (base?.source === 'user') return base; + if (isProcessMCPServerConfig(base)) return base; if (candidate.inspectionFailed) return base ?? candidate; return base ? { ...candidate, source: overlaySource(base, candidate) } : candidate; } @@ -355,10 +358,11 @@ export class MCPServersRegistry { * and User-DB entries. * * Precedence (lowest to highest): YAML cache > Config-tier overrides (success only) > User DB. - * Two guards keep the merge safe: + * Three guards keep the merge safe: * 1. Config-tier entries carrying `inspectionFailed: true` never overlay an existing * base entry; the healthy base is preserved for the duration of the retry window. * 2. User-DB entries (`source: 'user'`) are never replaced by Config-tier overlays. + * 3. Process-backed base entries are never replaced by Config-tier overlays. * On a successful overlay the base entry's `source` field is preserved so downstream * recovery logic routes to the correct storage location — except a `'plugin'` base, * whose no-resolve provenance must not transfer to the operator override (see @@ -379,6 +383,10 @@ export class MCPServersRegistry { logger.debug(`[MCP][config][${name}] Admin override shadowed by user-tier entry`); continue; } + if (isProcessMCPServerConfig(result[name])) { + logger.debug(`[MCP][config][${name}] Admin override shadowed by process-backed entry`); + continue; + } if (override.inspectionFailed && result[name]) continue; const baseEntry = result[name]; result[name] = baseEntry diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts index 7bee1e0f05..68816c9774 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts @@ -367,6 +367,35 @@ describe('MCPServersRegistry', () => { delete process.env.TEST_OPERATOR_SECRET; } }); + + it('keeps a process-backed plugin server authoritative over config-tier overrides', async () => { + const pluginBase: t.ParsedServerConfig = { + source: 'plugin', + type: 'stdio', + command: 'node', + args: ['trusted-plugin-server.js'], + }; + await registry['cacheConfigsRepo'].add('shared-process', pluginBase); + + const override: t.ParsedServerConfig = { + source: 'config', + type: 'streamable-http', + url: 'https://override.example.com/mcp', + requiresOAuth: false, + }; + + const all = await registry.getAllServerConfigs('user-1', { + 'shared-process': override, + }); + expect(all['shared-process']).toMatchObject(pluginBase); + expect(all['shared-process']).not.toHaveProperty('url'); + + const single = await registry.getServerConfig('shared-process', 'user-1', { + 'shared-process': override, + }); + expect(single).toMatchObject(pluginBase); + expect(single).not.toHaveProperty('url'); + }); }); describe('resolveAllowlists (per-request, tenant-scoped)', () => { diff --git a/packages/api/src/middleware/preAuthTenant.spec.ts b/packages/api/src/middleware/preAuthTenant.spec.ts index 51616da1e5..c92ecbdab4 100644 --- a/packages/api/src/middleware/preAuthTenant.spec.ts +++ b/packages/api/src/middleware/preAuthTenant.spec.ts @@ -13,6 +13,7 @@ jest.mock('@librechat/data-schemas', () => ({ })); describe('preAuthTenantMiddleware', () => { + const originalTrustTenantHeader = process.env.TRUST_TENANT_HEADER; let req: { headers: Record; ip?: string; @@ -24,10 +25,19 @@ describe('preAuthTenantMiddleware', () => { beforeEach(() => { jest.clearAllMocks(); + delete process.env.TRUST_TENANT_HEADER; req = { headers: {} }; res = {}; }); + afterAll(() => { + if (originalTrustTenantHeader === undefined) { + delete process.env.TRUST_TENANT_HEADER; + return; + } + process.env.TRUST_TENANT_HEADER = originalTrustTenantHeader; + }); + it('calls next() without ALS context when no X-Tenant-Id header is present', () => { let capturedTenantId: string | undefined = 'sentinel'; const capturedNext: NextFunction = () => { @@ -49,7 +59,19 @@ describe('preAuthTenantMiddleware', () => { expect(capturedTenantId).toBeUndefined(); }); - it('wraps downstream in ALS context when X-Tenant-Id header is present', () => { + it('ignores X-Tenant-Id unless the deployment explicitly trusts the header', () => { + req.headers = { 'x-tenant-id': 'attacker-selected' }; + let capturedTenantId: string | undefined = 'sentinel'; + const capturedNext: NextFunction = () => { + capturedTenantId = getTenantId(); + }; + + preAuthTenantMiddleware(req as Request, res as Response, capturedNext); + expect(capturedTenantId).toBeUndefined(); + }); + + it('wraps downstream in ALS context when the deployment trusts X-Tenant-Id', () => { + process.env.TRUST_TENANT_HEADER = 'TRUE'; req.headers = { 'x-tenant-id': 'acme-corp' }; let capturedTenantId: string | undefined; const capturedNext: NextFunction = () => { @@ -85,6 +107,7 @@ describe('preAuthTenantMiddleware', () => { }); it('ignores __SYSTEM__ sentinel and logs warning', () => { + process.env.TRUST_TENANT_HEADER = 'true'; req.headers = { 'x-tenant-id': '__SYSTEM__' }; req.ip = '10.0.0.1'; req.path = '/api/config'; @@ -102,6 +125,7 @@ describe('preAuthTenantMiddleware', () => { }); it('ignores array-valued headers (Express can produce these)', () => { + process.env.TRUST_TENANT_HEADER = 'true'; req.headers = { 'x-tenant-id': ['a', 'b'] as unknown as string }; let capturedTenantId: string | undefined = 'sentinel'; const capturedNext: NextFunction = () => { @@ -113,6 +137,7 @@ describe('preAuthTenantMiddleware', () => { }); it('ignores tenant IDs containing invalid characters and logs warning', () => { + process.env.TRUST_TENANT_HEADER = 'true'; req.headers = { 'x-tenant-id': 'tenant:injected' }; req.ip = '192.168.1.1'; req.path = '/api/auth/login'; @@ -130,6 +155,7 @@ describe('preAuthTenantMiddleware', () => { }); it('trims whitespace from tenant ID header', () => { + process.env.TRUST_TENANT_HEADER = 'true'; req.headers = { 'x-tenant-id': ' acme-corp ' }; let capturedTenantId: string | undefined; const capturedNext: NextFunction = () => { @@ -141,6 +167,7 @@ describe('preAuthTenantMiddleware', () => { }); it('ignores tenant IDs exceeding max length and logs warning', () => { + process.env.TRUST_TENANT_HEADER = 'true'; req.headers = { 'x-tenant-id': 'a'.repeat(200) }; req.ip = '192.168.1.1'; req.path = '/api/share/abc'; diff --git a/packages/api/src/middleware/preAuthTenant.ts b/packages/api/src/middleware/preAuthTenant.ts index d24064af46..90ea29e01a 100644 --- a/packages/api/src/middleware/preAuthTenant.ts +++ b/packages/api/src/middleware/preAuthTenant.ts @@ -1,6 +1,7 @@ import { logger, SYSTEM_TENANT_ID } from '@librechat/data-schemas'; import type { Request, Response, NextFunction } from 'express'; import { buildRequestContext, runWithTenantContext } from './tenant'; +import { isEnabled } from '~/utils'; /** * Pre-authentication tenant context middleware for unauthenticated routes. @@ -19,7 +20,8 @@ import { buildRequestContext, runWithTenantContext } from './tenant'; * **How the header gets set**: The deployment's reverse proxy, auth gateway, * or OpenID strategy sets `X-Tenant-Id` based on subdomain, path, or OIDC claim. * This middleware does NOT resolve tenants from subdomains or tokens — that is - * the responsibility of the deployment layer. + * the responsibility of the deployment layer. Header-based resolution is disabled + * unless the operator explicitly sets `TRUST_TENANT_HEADER=true`. * * **Design**: Intentionally minimal. No subdomain parsing, no OIDC claim * extraction, no YAML-driven strategy. Multi-tenant deployments can: @@ -37,7 +39,7 @@ export function preAuthTenantMiddleware(req: Request, res: Response, next: NextF const raw = req.headers['x-tenant-id']; const requestContext = buildRequestContext(req); - if (!raw || typeof raw !== 'string') { + if (!raw || typeof raw !== 'string' || !isEnabled(process.env.TRUST_TENANT_HEADER)) { runWithTenantContext(requestContext, next); return; } diff --git a/packages/data-provider/src/mcp.ts b/packages/data-provider/src/mcp.ts index 878a381e60..677fcdef34 100644 --- a/packages/data-provider/src/mcp.ts +++ b/packages/data-provider/src/mcp.ts @@ -249,6 +249,33 @@ const ProxyUrlSchema = z }, ); +const PROCESS_MCP_SERVER_FIELDS = new Set(['command', 'args', 'env', 'cwd', 'stderr']); + +export function isProcessMCPServerField(field: string): boolean { + return PROCESS_MCP_SERVER_FIELDS.has(field); +} + +export function isProcessMCPServerConfig(value: unknown): boolean { + if (value == null || typeof value !== 'object' || Array.isArray(value)) { + return false; + } + + const config = value as Record; + if (config.type === 'stdio') { + return true; + } + + return Object.keys(config).some(isProcessMCPServerField); +} + +export function hasProcessMCPServerConfig(value: unknown): boolean { + if (value == null || typeof value !== 'object' || Array.isArray(value)) { + return false; + } + + return Object.values(value).some(isProcessMCPServerConfig); +} + export const StdioOptionsSchema = BaseOptionsSchema.extend({ type: z.literal('stdio').default('stdio'), obo: z.undefined().optional(), diff --git a/packages/data-schemas/src/app/resolution.spec.ts b/packages/data-schemas/src/app/resolution.spec.ts index 956e58056a..de47138f39 100644 --- a/packages/data-schemas/src/app/resolution.spec.ts +++ b/packages/data-schemas/src/app/resolution.spec.ts @@ -525,6 +525,113 @@ describe('mergeConfigOverrides', () => { expect(result.mcpServers).toBeUndefined(); }); + it('drops process-backed MCP servers from database overrides', () => { + const base = { + ...baseConfig, + mcpConfig: { + operator: { type: 'stdio', command: 'node', args: ['trusted-server.js'] }, + }, + } as unknown as AppConfig; + const configs = [ + fakeConfig( + { + mcpServers: { + injected: { type: 'stdio', command: '/bin/sh', args: ['-c', 'id'] }, + remote: { type: 'streamable-http', url: 'https://mcp.example.com' }, + }, + }, + 10, + ), + ]; + + const result = mergeConfigOverrides(base, configs) as unknown as Record; + const mcpConfig = result.mcpConfig as Record; + + expect(mcpConfig.injected).toBeUndefined(); + expect(mcpConfig.remote).toEqual({ + type: 'streamable-http', + url: 'https://mcp.example.com', + }); + expect(mcpConfig.operator).toEqual({ + type: 'stdio', + command: 'node', + args: ['trusted-server.js'], + }); + }); + + it('does not let database overrides mutate an operator-owned stdio server', () => { + const base = { + ...baseConfig, + mcpConfig: { + operator: { type: 'stdio', command: 'node', args: ['trusted-server.js'] }, + }, + } as unknown as AppConfig; + const configs = [ + fakeConfig( + { + mcpServers: { + operator: { command: '/bin/sh', args: ['-c', 'id'] }, + }, + }, + 10, + ), + ]; + + const result = mergeConfigOverrides(base, configs) as unknown as Record; + const mcpConfig = result.mcpConfig as Record; + + expect(mcpConfig.operator).toEqual({ + type: 'stdio', + command: 'node', + args: ['trusted-server.js'], + }); + }); + + it('does not let scalar database overrides disable an operator-owned stdio server', () => { + const base = { + ...baseConfig, + mcpConfig: { + operator: { type: 'stdio', command: 'node', args: ['trusted-server.js'] }, + }, + } as unknown as AppConfig; + const configs = [ + fakeConfig( + { + mcpServers: { + operator: null, + }, + }, + 10, + ), + ]; + + const result = mergeConfigOverrides(base, configs) as unknown as Record; + const mcpConfig = result.mcpConfig as Record; + + expect(mcpConfig.operator).toEqual({ + type: 'stdio', + command: 'node', + args: ['trusted-server.js'], + }); + }); + + it('drops process-backed MCP servers supplied through the runtime config alias', () => { + const configs = [ + fakeConfig( + { + mcpConfig: { + injected: { command: '/bin/sh', args: ['-c', 'id'] }, + }, + }, + 10, + ), + ]; + + const result = mergeConfigOverrides(baseConfig, configs) as unknown as Record; + + expect(result.mcpConfig).toEqual({}); + }); + it('applies tombstones after remapping YAML paths to AppConfig paths', () => { const base = { mcpConfig: { @@ -554,6 +661,46 @@ describe('mergeConfigOverrides', () => { }); }); + it.each(['mcpServers.operator', 'mcpServers.operator.command'])( + 'does not let the %s tombstone alter an operator-owned stdio server', + (tombstone) => { + const base = { + mcpConfig: { + operator: { type: 'stdio', command: 'node', args: ['trusted-server.js'] }, + }, + } as unknown as AppConfig; + + const result = mergeConfigOverrides(base, [ + fakeConfig({}, 10, [tombstone]), + ]) as unknown as Record; + const mcpConfig = result.mcpConfig as Record; + + expect(mcpConfig.operator).toEqual({ + type: 'stdio', + command: 'node', + args: ['trusted-server.js'], + }); + }, + ); + + it('preserves operator-owned stdio servers when the MCP section is tombstoned', () => { + const base = { + mcpConfig: { + operator: { type: 'stdio', command: 'node', args: ['trusted-server.js'] }, + remote: { type: 'streamable-http', url: 'https://mcp.example.com' }, + }, + } as unknown as AppConfig; + + const result = mergeConfigOverrides(base, [ + fakeConfig({}, 10, ['mcpServers']), + ]) as unknown as Record; + const mcpConfig = result.mcpConfig as Record; + + expect(mcpConfig).toEqual({ + operator: { type: 'stdio', command: 'node', args: ['trusted-server.js'] }, + }); + }); + it('lets a higher-priority override recreate a lower-priority tombstoned path', () => { const base = { mcpConfig: { diff --git a/packages/data-schemas/src/app/resolution.ts b/packages/data-schemas/src/app/resolution.ts index 6e45cb1605..1f79163df9 100644 --- a/packages/data-schemas/src/app/resolution.ts +++ b/packages/data-schemas/src/app/resolution.ts @@ -3,6 +3,7 @@ import { BASE_ONLY_CONFIG_SECTIONS, INTERFACE_PERMISSION_FIELDS, PERMISSION_SUB_KEYS, + isProcessMCPServerConfig, } from 'librechat-data-provider'; import type { TCustomConfig } from 'librechat-data-provider'; import type { AppConfig, IConfig } from '~/types'; @@ -88,6 +89,32 @@ function deletePath(target: T, path: string): T { return result as T; } +function deleteConfigPath(target: T, path: string): T { + const [section, serverName] = path.split('.'); + if (section !== 'mcpConfig') { + return deletePath(target, path); + } + + const mcpConfig = target.mcpConfig; + if (mcpConfig == null || typeof mcpConfig !== 'object' || Array.isArray(mcpConfig)) { + return deletePath(target, path); + } + + const servers = mcpConfig as AnyObject; + if (serverName != null) { + return isProcessMCPServerConfig(servers[serverName]) ? target : deletePath(target, path); + } + + const processServers = Object.fromEntries( + Object.entries(servers).filter(([, serverConfig]) => isProcessMCPServerConfig(serverConfig)), + ); + if (Object.keys(processServers).length === 0) { + return deletePath(target, path); + } + + return { ...target, mcpConfig: processServers } as T; +} + function mergeArrayByKey( target: AnyObject[], source: AnyObject[], @@ -181,6 +208,43 @@ function deepMerge(target: T, source: AnyObject, depth = 0, return result as T; } +function filterMCPServerOverrides(value: unknown, current: unknown): AnyObject { + if (value == null || typeof value !== 'object' || Array.isArray(value)) { + return {}; + } + + const currentServers = + current != null && typeof current === 'object' && !Array.isArray(current) + ? (current as AnyObject) + : {}; + const filtered: AnyObject = {}; + + for (const [serverName, serverOverride] of Object.entries(value)) { + const currentServer = currentServers[serverName]; + if ( + serverOverride == null || + typeof serverOverride !== 'object' || + Array.isArray(serverOverride) + ) { + if (!isProcessMCPServerConfig(currentServer)) { + filtered[serverName] = serverOverride; + } + continue; + } + + const baseServer = + currentServer != null && typeof currentServer === 'object' && !Array.isArray(currentServer) + ? (currentServer as AnyObject) + : {}; + const resolved = deepMerge(baseServer, serverOverride as AnyObject); + if (!isProcessMCPServerConfig(resolved)) { + filtered[serverName] = serverOverride; + } + } + + return filtered; +} + /** * Merge DB config overrides into a base AppConfig. * @@ -203,7 +267,7 @@ export function mergeConfigOverrides(baseConfig: AppConfig, configs: IConfig[]): typeof path === 'string' && (isBasePrincipal || !BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(path.split('.')[0])) ) { - merged = deletePath(merged, remapOverridePath(path)); + merged = deleteConfigPath(merged, remapOverridePath(path)); } } } @@ -218,7 +282,12 @@ export function mergeConfigOverrides(baseConfig: AppConfig, configs: IConfig[]): continue; } const mappedKey = OVERRIDE_KEY_MAP[key as keyof typeof OVERRIDE_KEY_MAP] ?? key; - if ( + if (mappedKey === 'mcpConfig') { + remapped[mappedKey] = filterMCPServerOverrides( + value, + (merged as unknown as AnyObject)[mappedKey], + ); + } else if ( key === 'interface' && value != null && typeof value === 'object' &&