mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-04-03 06:17:21 +02:00
🔒 feat: Sanitize Placeholders in User-provided MCP Server Config (#11486)
* 🔒 feat: Sanitize Placeholders in User-provider MCP Server Config Headers
* Implemented sanitization for dangerous credential placeholders in headers to prevent credential exfiltration when MCP servers are shared.
* Added tests to verify that dangerous placeholders are stripped from headers during both add and update operations, while safe placeholders are preserved.
* Refactored ServerConfigsDB to include a new sanitizeCredentialPlaceholders function for header processing.
* ci: tests for preserving credential placeholders in YAML configs
* Introduced new tests to ensure that LIBRECHAT_OPENID and LIBRECHAT_USER placeholders are preserved in admin configuration headers when added to the cache.
* Validated that the expected placeholders remain intact during retrieval, enhancing the integrity of configuration management.
This commit is contained in:
Danny Avila
GitHub
parent
18a0e8a8b0
commit
ee44c6344d
3 changed files with 306 additions and 52 deletions
|
|
@ -1,21 +1,52 @@
|
|||
import { Types } from 'mongoose';
|
||||
import {
|
||||
AccessRoleIds,
|
||||
PermissionBits,
|
||||
PrincipalType,
|
||||
ResourceType,
|
||||
AccessRoleIds,
|
||||
PrincipalType,
|
||||
PermissionBits,
|
||||
} from 'librechat-data-provider';
|
||||
import {
|
||||
AllMethods,
|
||||
MCPServerDocument,
|
||||
createMethods,
|
||||
logger,
|
||||
encryptV2,
|
||||
decryptV2,
|
||||
} from '@librechat/data-schemas';
|
||||
import { logger, encryptV2, decryptV2, createMethods } from '@librechat/data-schemas';
|
||||
import type { AllMethods, MCPServerDocument } from '@librechat/data-schemas';
|
||||
import type { IServerConfigsRepositoryInterface } from '~/mcp/registry/ServerConfigsRepositoryInterface';
|
||||
import { AccessControlService } from '~/acl/accessControlService';
|
||||
import type { ParsedServerConfig, AddServerResult } from '~/mcp/types';
|
||||
import { AccessControlService } from '~/acl/accessControlService';
|
||||
|
||||
/**
|
||||
* Regex patterns for credential placeholders that should not be allowed in user-provided headers.
|
||||
* These placeholders would substitute the CALLING user's credentials, creating a security risk
|
||||
* when MCP servers are shared between users (credential exfiltration).
|
||||
*
|
||||
* Safe placeholders like {{MCP_API_KEY}} are allowed as they resolve from the user's own plugin auth.
|
||||
*/
|
||||
const DANGEROUS_CREDENTIAL_PATTERNS = [
|
||||
/\{\{LIBRECHAT_OPENID_[^}]+\}\}/g,
|
||||
/\{\{LIBRECHAT_USER_[^}]+\}\}/g,
|
||||
];
|
||||
|
||||
/**
|
||||
* Sanitizes headers by removing dangerous credential placeholders.
|
||||
* This prevents credential exfiltration when MCP servers are shared between users.
|
||||
*
|
||||
* @param headers - The headers object to sanitize
|
||||
* @returns Sanitized headers with dangerous placeholders removed
|
||||
*/
|
||||
function sanitizeCredentialPlaceholders(
|
||||
headers?: Record<string, string>,
|
||||
): Record<string, string> | undefined {
|
||||
if (!headers) {
|
||||
return headers;
|
||||
}
|
||||
|
||||
const sanitized: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(headers)) {
|
||||
let sanitizedValue = value;
|
||||
for (const pattern of DANGEROUS_CREDENTIAL_PATTERNS) {
|
||||
sanitizedValue = sanitizedValue.replace(pattern, '');
|
||||
}
|
||||
sanitized[key] = sanitizedValue;
|
||||
}
|
||||
return sanitized;
|
||||
}
|
||||
|
||||
/**
|
||||
* DB backed config storage
|
||||
|
|
@ -46,13 +77,13 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
let accessibleAgentIds: Types.ObjectId[];
|
||||
|
||||
if (!userId) {
|
||||
// Get publicly accessible agents
|
||||
/** Publicly accessible agents */
|
||||
accessibleAgentIds = await this._aclService.findPubliclyAccessibleResources({
|
||||
resourceType: ResourceType.AGENT,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
});
|
||||
} else {
|
||||
// Get user-accessible agents
|
||||
/** User-accessible agents */
|
||||
accessibleAgentIds = await this._aclService.findAccessibleResources({
|
||||
userId,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
|
|
@ -64,7 +95,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
return false;
|
||||
}
|
||||
|
||||
// Check if any accessible agent has this MCP server
|
||||
const Agent = this._mongoose.model('Agent');
|
||||
const exists = await Agent.exists({
|
||||
_id: { $in: accessibleAgentIds },
|
||||
|
|
@ -95,9 +125,17 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
'[ServerConfigsDB.add] User ID is required to create a database-stored MCP server.',
|
||||
);
|
||||
}
|
||||
// Transform user-provided API key config (adds customUserVars and headers)
|
||||
const transformedConfig = this.transformUserApiKeyConfig(config);
|
||||
// Encrypt sensitive fields before storing in database
|
||||
|
||||
const sanitizedConfig = {
|
||||
...config,
|
||||
headers: sanitizeCredentialPlaceholders(
|
||||
(config as ParsedServerConfig & { headers?: Record<string, string> }).headers,
|
||||
),
|
||||
} as ParsedServerConfig;
|
||||
|
||||
/** Transformed user-provided API key config (adds customUserVars and headers) */
|
||||
const transformedConfig = this.transformUserApiKeyConfig(sanitizedConfig);
|
||||
/** Encrypted config before storing in database */
|
||||
const encryptedConfig = await this.encryptConfig(transformedConfig);
|
||||
const createdServer = await this._dbMethods.createMCPServer({
|
||||
config: encryptedConfig,
|
||||
|
|
@ -135,16 +173,20 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
}
|
||||
|
||||
const existingServer = await this._dbMethods.findMCPServerByServerName(serverName);
|
||||
let configToSave: ParsedServerConfig = { ...config };
|
||||
|
||||
// Transform user-provided API key config (adds customUserVars and headers)
|
||||
let configToSave: ParsedServerConfig = {
|
||||
...config,
|
||||
headers: sanitizeCredentialPlaceholders(
|
||||
(config as ParsedServerConfig & { headers?: Record<string, string> }).headers,
|
||||
),
|
||||
} as ParsedServerConfig;
|
||||
|
||||
/** Transformed user-provided API key config (adds customUserVars and headers) */
|
||||
configToSave = this.transformUserApiKeyConfig(configToSave);
|
||||
|
||||
// Encrypt NEW secrets only (secrets provided in this update)
|
||||
// We must do this BEFORE preserving existing encrypted secrets
|
||||
/** Encrypted config before storing in database */
|
||||
configToSave = await this.encryptConfig(configToSave);
|
||||
|
||||
// Preserve existing OAuth client_secret if not provided in update (already encrypted)
|
||||
if (!config.oauth?.client_secret && existingServer?.config?.oauth?.client_secret) {
|
||||
configToSave = {
|
||||
...configToSave,
|
||||
|
|
@ -155,8 +197,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
};
|
||||
}
|
||||
|
||||
// Preserve existing API key if not provided in update (already encrypted)
|
||||
// Only preserve if both old and new configs use admin mode to avoid cross-mode key leakage
|
||||
if (
|
||||
config.apiKey?.source === 'admin' &&
|
||||
!config.apiKey?.key &&
|
||||
|
|
@ -174,7 +214,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
};
|
||||
}
|
||||
|
||||
// specific user permissions for action permission will be handled in the controller calling the update method of the registry
|
||||
await this._dbMethods.updateMCPServer(serverName, { config: configToSave });
|
||||
}
|
||||
|
||||
|
|
@ -207,7 +246,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
const server = await this._dbMethods.findMCPServerByServerName(serverName);
|
||||
if (!server) return undefined;
|
||||
|
||||
// Check public access if no userId
|
||||
if (!userId) {
|
||||
const directlyAccessibleMCPIds = (
|
||||
await this._aclService.findPubliclyAccessibleResources({
|
||||
|
|
@ -219,7 +257,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
return await this.mapDBServerToParsedConfig(server);
|
||||
}
|
||||
|
||||
// Check access via publicly accessible agents
|
||||
const hasAgentAccess = await this.hasAccessViaAgent(serverName);
|
||||
if (hasAgentAccess) {
|
||||
logger.debug(
|
||||
|
|
@ -234,7 +271,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
return undefined;
|
||||
}
|
||||
|
||||
// Check direct user access
|
||||
const userHasDirectAccess = await this._aclService.checkPermission({
|
||||
userId,
|
||||
resourceType: ResourceType.MCPSERVER,
|
||||
|
|
@ -249,7 +285,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
return await this.mapDBServerToParsedConfig(server);
|
||||
}
|
||||
|
||||
// Check agent access (user can VIEW an agent that has this MCP server)
|
||||
/** Check agent access (user can VIEW an agent that has this MCP server) */
|
||||
const hasAgentAccess = await this.hasAccessViaAgent(serverName, userId);
|
||||
if (hasAgentAccess) {
|
||||
logger.debug(
|
||||
|
|
@ -270,7 +306,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
* @returns record of parsed configs
|
||||
*/
|
||||
public async getAll(userId?: string): Promise<Record<string, ParsedServerConfig>> {
|
||||
// 1. Get directly accessible MCP IDs
|
||||
let directlyAccessibleMCPIds: Types.ObjectId[] = [];
|
||||
if (!userId) {
|
||||
logger.debug(`[ServerConfigsDB.getAll] fetching all publicly shared mcp servers`);
|
||||
|
|
@ -289,18 +324,15 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
});
|
||||
}
|
||||
|
||||
// 2. Get agent-accessible MCP server names
|
||||
let agentMCPServerNames: string[] = [];
|
||||
let accessibleAgentIds: Types.ObjectId[] = [];
|
||||
|
||||
if (!userId) {
|
||||
// Get publicly accessible agents
|
||||
accessibleAgentIds = await this._aclService.findPubliclyAccessibleResources({
|
||||
resourceType: ResourceType.AGENT,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
});
|
||||
} else {
|
||||
// Get user-accessible agents
|
||||
accessibleAgentIds = await this._aclService.findAccessibleResources({
|
||||
userId,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
|
|
@ -309,7 +341,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
}
|
||||
|
||||
if (accessibleAgentIds.length > 0) {
|
||||
// Efficient query: get agents with non-empty mcpServerNames
|
||||
const Agent = this._mongoose.model('Agent');
|
||||
const agentsWithMCP = await Agent.find(
|
||||
{
|
||||
|
|
@ -319,7 +350,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
{ mcpServerNames: 1 },
|
||||
).lean();
|
||||
|
||||
// Flatten and dedupe server names
|
||||
agentMCPServerNames = [
|
||||
...new Set(
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
|
|
@ -328,12 +358,10 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
];
|
||||
}
|
||||
|
||||
// 3. Fetch directly accessible MCP servers
|
||||
const directResults = await this._dbMethods.getListMCPServersByIds({
|
||||
ids: directlyAccessibleMCPIds,
|
||||
});
|
||||
|
||||
// 4. Build result with direct access servers (parallel decryption)
|
||||
const parsedConfigs: Record<string, ParsedServerConfig> = {};
|
||||
const directData = directResults.data || [];
|
||||
const directServerNames = new Set(directData.map((s) => s.serverName));
|
||||
|
|
@ -345,7 +373,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
parsedConfigs[s.serverName] = directParsed[i];
|
||||
});
|
||||
|
||||
// 5. Fetch agent-accessible servers (excluding already direct)
|
||||
const agentOnlyServerNames = agentMCPServerNames.filter((name) => !directServerNames.has(name));
|
||||
|
||||
if (agentOnlyServerNames.length > 0) {
|
||||
|
|
@ -383,7 +410,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
dbId: (serverDBDoc._id as Types.ObjectId).toString(),
|
||||
updatedAt: serverDBDoc.updatedAt?.getTime(),
|
||||
};
|
||||
// Decrypt sensitive fields after retrieval from database
|
||||
return await this.decryptConfig(config);
|
||||
}
|
||||
|
||||
|
|
@ -421,7 +447,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
},
|
||||
};
|
||||
|
||||
// Cast to access headers property (not available on Stdio type)
|
||||
/** Cast to access headers property (not available on Stdio type) */
|
||||
const resultWithHeaders = result as ParsedServerConfig & {
|
||||
headers?: Record<string, string>;
|
||||
};
|
||||
|
|
@ -446,7 +472,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
private async encryptConfig(config: ParsedServerConfig): Promise<ParsedServerConfig> {
|
||||
let result = { ...config };
|
||||
|
||||
// Encrypt admin-provided API key
|
||||
if (result.apiKey?.source === 'admin' && result.apiKey.key) {
|
||||
try {
|
||||
result.apiKey = {
|
||||
|
|
@ -459,7 +484,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
}
|
||||
}
|
||||
|
||||
// Encrypt OAuth client_secret
|
||||
if (result.oauth?.client_secret) {
|
||||
try {
|
||||
result = {
|
||||
|
|
@ -486,7 +510,6 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
private async decryptConfig(config: ParsedServerConfig): Promise<ParsedServerConfig> {
|
||||
let result = { ...config };
|
||||
|
||||
// Handle API key decryption (admin-provided only)
|
||||
if (result.apiKey?.source === 'admin' && result.apiKey.key) {
|
||||
try {
|
||||
result.apiKey = {
|
||||
|
|
@ -504,9 +527,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
}
|
||||
}
|
||||
|
||||
// Handle OAuth client_secret decryption
|
||||
if (result.oauth?.client_secret) {
|
||||
// Cast oauth to type with client_secret since we've verified it exists
|
||||
const oauthConfig = result.oauth as { client_secret: string } & typeof result.oauth;
|
||||
try {
|
||||
result = {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue