🔗 refactor: URL sanitization for MCP logging (#9632)

This commit is contained in:
Danny Avila 2025-09-14 18:55:32 -04:00 committed by GitHub
parent 5bfb06b417
commit 7a9a99d2a0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 88 additions and 35 deletions

View file

@ -6,6 +6,7 @@ import type { FlowStateManager } from '~/flow/manager';
import type { FlowMetadata } from '~/flow/types';
import type * as t from './types';
import { MCPTokenStorage, MCPOAuthHandler } from '~/mcp/oauth';
import { sanitizeUrlForLogging } from './utils';
import { MCPConnection } from './connection';
import { processMCPEnv } from '~/utils';
@ -308,7 +309,9 @@ export class MCPConnectionFactory {
metadata?: OAuthMetadata;
} | null> {
const serverUrl = (this.serverConfig as t.SSEOptions | t.StreamableHTTPOptions).url;
logger.debug(`${this.logPrefix} \`handleOAuthRequired\` called with serverUrl: ${serverUrl}`);
logger.debug(
`${this.logPrefix} \`handleOAuthRequired\` called with serverUrl: ${serverUrl ? sanitizeUrlForLogging(serverUrl) : 'undefined'}`,
);
if (!this.flowManager || !serverUrl) {
logger.error(

View file

@ -7,6 +7,7 @@ import type { JsonSchemaType } from '~/types';
import type * as t from '~/mcp/types';
import { ConnectionsRepository } from '~/mcp/ConnectionsRepository';
import { detectOAuthRequirement } from '~/mcp/oauth';
import { sanitizeUrlForLogging } from '~/mcp/utils';
import { processMCPEnv } from '~/utils';
import { CONSTANTS } from '~/mcp/enum';
@ -183,7 +184,7 @@ export class MCPServersRegistry {
const prefix = this.prefix(serverName);
const config = this.parsedConfigs[serverName];
logger.info(`${prefix} -------------------------------------------------┐`);
logger.info(`${prefix} URL: ${config.url}`);
logger.info(`${prefix} URL: ${config.url ? sanitizeUrlForLogging(config.url) : 'N/A'}`);
logger.info(`${prefix} OAuth Required: ${config.requiresOAuth}`);
logger.info(`${prefix} Capabilities: ${config.capabilities}`);
logger.info(`${prefix} Tools: ${config.tools}`);

View file

@ -1,15 +1,15 @@
import { EventEmitter } from 'events';
import { logger } from '@librechat/data-schemas';
import { fetch as undiciFetch, Agent } from 'undici';
import {
StdioClientTransport,
getDefaultEnvironment,
} from '@modelcontextprotocol/sdk/client/stdio.js';
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
import { ResourceListChangedNotificationSchema } from '@modelcontextprotocol/sdk/types.js';
import { WebSocketClientTransport } from '@modelcontextprotocol/sdk/client/websocket.js';
import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { logger } from '@librechat/data-schemas';
import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
import { WebSocketClientTransport } from '@modelcontextprotocol/sdk/client/websocket.js';
import { ResourceListChangedNotificationSchema } from '@modelcontextprotocol/sdk/types.js';
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js';
import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types.js';
import type {
@ -18,8 +18,9 @@ import type {
Response as UndiciResponse,
} from 'undici';
import type { MCPOAuthTokens } from './oauth/types';
import { mcpConfig } from './mcpConfig';
import type * as t from './types';
import { sanitizeUrlForLogging } from './utils';
import { mcpConfig } from './mcpConfig';
type FetchLike = (url: string | URL, init?: RequestInit) => Promise<Response>;
@ -238,7 +239,9 @@ export class MCPConnection extends EventEmitter {
}
this.url = options.url;
const url = new URL(options.url);
logger.info(`${this.getLogPrefix()} Creating SSE transport: ${url.toString()}`);
logger.info(
`${this.getLogPrefix()} Creating SSE transport: ${sanitizeUrlForLogging(url)}`,
);
const abortController = new AbortController();
/** Add OAuth token to headers if available */
@ -293,7 +296,7 @@ export class MCPConnection extends EventEmitter {
this.url = options.url;
const url = new URL(options.url);
logger.info(
`${this.getLogPrefix()} Creating streamable-http transport: ${url.toString()}`,
`${this.getLogPrefix()} Creating streamable-http transport: ${sanitizeUrlForLogging(url)}`,
);
const abortController = new AbortController();
@ -473,7 +476,9 @@ export class MCPConnection extends EventEmitter {
logger.warn(`${this.getLogPrefix()} OAuth authentication required`);
this.oauthRequired = true;
const serverUrl = this.url;
logger.debug(`${this.getLogPrefix()} Server URL for OAuth: ${serverUrl}`);
logger.debug(
`${this.getLogPrefix()} Server URL for OAuth: ${serverUrl ? sanitizeUrlForLogging(serverUrl) : 'undefined'}`,
);
const oauthTimeout = this.options.initTimeout ?? 60000 * 2;
/** Promise that will resolve when OAuth is handled */

View file

@ -17,6 +17,7 @@ import type {
MCPOAuthTokens,
OAuthMetadata,
} from './types';
import { sanitizeUrlForLogging } from '~/mcp/utils';
/** Type for the OAuth metadata from the SDK */
type SDKOAuthMetadata = Parameters<typeof registerClient>[1]['metadata'];
@ -33,7 +34,9 @@ export class MCPOAuthHandler {
resourceMetadata?: OAuthProtectedResourceMetadata;
authServerUrl: URL;
}> {
logger.debug(`[MCPOAuth] discoverMetadata called with serverUrl: ${serverUrl}`);
logger.debug(
`[MCPOAuth] discoverMetadata called with serverUrl: ${sanitizeUrlForLogging(serverUrl)}`,
);
let authServerUrl = new URL(serverUrl);
let resourceMetadata: OAuthProtectedResourceMetadata | undefined;
@ -60,11 +63,15 @@ export class MCPOAuthHandler {
}
// Discover OAuth metadata
logger.debug(`[MCPOAuth] Discovering OAuth metadata from ${authServerUrl}`);
logger.debug(
`[MCPOAuth] Discovering OAuth metadata from ${sanitizeUrlForLogging(authServerUrl)}`,
);
const rawMetadata = await discoverAuthorizationServerMetadata(authServerUrl);
if (!rawMetadata) {
logger.error(`[MCPOAuth] Failed to discover OAuth metadata from ${authServerUrl}`);
logger.error(
`[MCPOAuth] Failed to discover OAuth metadata from ${sanitizeUrlForLogging(authServerUrl)}`,
);
throw new Error('Failed to discover OAuth metadata');
}
@ -88,12 +95,15 @@ export class MCPOAuthHandler {
resourceMetadata?: OAuthProtectedResourceMetadata,
redirectUri?: string,
): Promise<OAuthClientInformation> {
logger.debug(`[MCPOAuth] Starting client registration for ${serverUrl}, server metadata:`, {
grant_types_supported: metadata.grant_types_supported,
response_types_supported: metadata.response_types_supported,
token_endpoint_auth_methods_supported: metadata.token_endpoint_auth_methods_supported,
scopes_supported: metadata.scopes_supported,
});
logger.debug(
`[MCPOAuth] Starting client registration for ${sanitizeUrlForLogging(serverUrl)}, server metadata:`,
{
grant_types_supported: metadata.grant_types_supported,
response_types_supported: metadata.response_types_supported,
token_endpoint_auth_methods_supported: metadata.token_endpoint_auth_methods_supported,
scopes_supported: metadata.scopes_supported,
},
);
/** Client metadata based on what the server supports */
const clientMetadata = {
@ -114,7 +124,9 @@ export class MCPOAuthHandler {
`[MCPOAuth] Server ${serverUrl} supports \`refresh_token\` grant type, adding to request`,
);
} else {
logger.debug(`[MCPOAuth] Server ${serverUrl} does not support \`refresh_token\` grant type`);
logger.debug(
`[MCPOAuth] Server ${sanitizeUrlForLogging(serverUrl)} does not support \`refresh_token\` grant type`,
);
}
clientMetadata.grant_types = requestedGrantTypes;
@ -139,19 +151,25 @@ export class MCPOAuthHandler {
clientMetadata.scope = availableScopes.join(' ');
}
logger.debug(`[MCPOAuth] Registering client for ${serverUrl} with metadata:`, clientMetadata);
logger.debug(
`[MCPOAuth] Registering client for ${sanitizeUrlForLogging(serverUrl)} with metadata:`,
clientMetadata,
);
const clientInfo = await registerClient(serverUrl, {
metadata: metadata as unknown as SDKOAuthMetadata,
clientMetadata,
});
logger.debug(`[MCPOAuth] Client registered successfully for ${serverUrl}:`, {
client_id: clientInfo.client_id,
has_client_secret: !!clientInfo.client_secret,
grant_types: clientInfo.grant_types,
scope: clientInfo.scope,
});
logger.debug(
`[MCPOAuth] Client registered successfully for ${sanitizeUrlForLogging(serverUrl)}:`,
{
client_id: clientInfo.client_id,
has_client_secret: !!clientInfo.client_secret,
grant_types: clientInfo.grant_types,
scope: clientInfo.scope,
},
);
return clientInfo;
}
@ -165,7 +183,9 @@ export class MCPOAuthHandler {
userId: string,
config: MCPOptions['oauth'] | undefined,
): Promise<{ authorizationUrl: string; flowId: string; flowMetadata: MCPOAuthFlowMetadata }> {
logger.debug(`[MCPOAuth] initiateOAuthFlow called for ${serverName} with URL: ${serverUrl}`);
logger.debug(
`[MCPOAuth] initiateOAuthFlow called for ${serverName} with URL: ${sanitizeUrlForLogging(serverUrl)}`,
);
const flowId = this.generateFlowId(userId, serverName);
const state = this.generateState();
@ -226,7 +246,9 @@ export class MCPOAuthHandler {
metadata,
};
logger.debug(`[MCPOAuth] Authorization URL generated: ${authorizationUrl.toString()}`);
logger.debug(
`[MCPOAuth] Authorization URL generated: ${sanitizeUrlForLogging(authorizationUrl.toString())}`,
);
return {
authorizationUrl: authorizationUrl.toString(),
flowId,
@ -234,10 +256,14 @@ export class MCPOAuthHandler {
};
}
logger.debug(`[MCPOAuth] Starting auto-discovery of OAuth metadata from ${serverUrl}`);
logger.debug(
`[MCPOAuth] Starting auto-discovery of OAuth metadata from ${sanitizeUrlForLogging(serverUrl)}`,
);
const { metadata, resourceMetadata, authServerUrl } = await this.discoverMetadata(serverUrl);
logger.debug(`[MCPOAuth] OAuth metadata discovered, auth server URL: ${authServerUrl}`);
logger.debug(
`[MCPOAuth] OAuth metadata discovered, auth server URL: ${sanitizeUrlForLogging(authServerUrl)}`,
);
/** Dynamic client registration based on the discovered metadata */
const redirectUri = config?.redirect_uri || this.getDefaultRedirectUri(serverName);
@ -276,7 +302,9 @@ export class MCPOAuthHandler {
codeVerifier = authResult.codeVerifier;
logger.debug(`[MCPOAuth] startAuthorization completed successfully`);
logger.debug(`[MCPOAuth] Authorization URL: ${authorizationUrl.toString()}`);
logger.debug(
`[MCPOAuth] Authorization URL: ${sanitizeUrlForLogging(authorizationUrl.toString())}`,
);
/** Add state parameter with flowId to the authorization URL */
authorizationUrl.searchParams.set('state', flowId);
@ -515,7 +543,7 @@ export class MCPOAuthHandler {
body.append('client_id', metadata.clientInfo.client_id);
}
logger.debug(`[MCPOAuth] Refresh request to: ${tokenUrl}`, {
logger.debug(`[MCPOAuth] Refresh request to: ${sanitizeUrlForLogging(tokenUrl)}`, {
body: body.toString(),
headers,
});
@ -695,7 +723,9 @@ export class MCPOAuthHandler {
}
// perform the revoke request
logger.info(`[MCPOAuth] Revoking tokens for ${serverName} via ${revokeUrl.toString()}`);
logger.info(
`[MCPOAuth] Revoking tokens for ${serverName} via ${sanitizeUrlForLogging(revokeUrl.toString())}`,
);
const response = await fetch(revokeUrl, {
method: 'POST',
body: body.toString(),

View file

@ -31,3 +31,17 @@ export function normalizeServerName(serverName: string): string {
return normalized;
}
/**
* Sanitizes a URL by removing query parameters to prevent credential leakage in logs.
* @param url - The URL to sanitize (string or URL object)
* @returns The sanitized URL string without query parameters
*/
export function sanitizeUrlForLogging(url: string | URL): string {
try {
const urlObj = typeof url === 'string' ? new URL(url) : url;
return `${urlObj.protocol}//${urlObj.host}${urlObj.pathname}`;
} catch {
return '[invalid URL]';
}
}