From 47747d67c27e18fa83dc53722e5d50e0e0a5c805 Mon Sep 17 00:00:00 2001 From: aheizi Date: Mon, 17 Mar 2025 16:17:42 +0800 Subject: [PATCH] validate server config --- src/services/mcp/McpHub.ts | 249 ++++++++++++++++++++++++++----------- 1 file changed, 177 insertions(+), 72 deletions(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index d98e304884..984beabe1b 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -43,26 +43,54 @@ const BaseConfigSchema = z.object({ alwaysAllow: z.array(z.string()).default([]), }) -// Server configuration schema with automatic type inference -export const ServerConfigSchema = z.union([ - // Stdio config (has command field) - BaseConfigSchema.extend({ - command: z.string(), - args: z.array(z.string()).optional(), - env: z.record(z.string()).optional(), - }).transform((data) => ({ - ...data, - type: "stdio" as const, - })), - // SSE config (has url field) - BaseConfigSchema.extend({ - url: z.string().url(), - headers: z.record(z.string()).optional(), - }).transform((data) => ({ - ...data, - type: "sse" as const, - })), -]) +// Custom error messages for better user feedback +const typeErrorMessage = "Server type must be either 'stdio' or 'sse'" +const stdioFieldsErrorMessage = + "For 'stdio' type servers, you must provide a 'command' field and can optionally include 'args' and 'env'" +const sseFieldsErrorMessage = + "For 'sse' type servers, you must provide a 'url' field and can optionally include 'headers'" +const mixedFieldsErrorMessage = + "Cannot mix 'stdio' and 'sse' fields. For 'stdio' use 'command', 'args', and 'env'. For 'sse' use 'url' and 'headers'" +const missingFieldsErrorMessage = "Server configuration must include either 'command' (for stdio) or 'url' (for sse)" + +// Helper function to create a refined schema with better error messages +const createServerTypeSchema = () => { + return z.union([ + // Stdio config (has command field) + BaseConfigSchema.extend({ + type: z.enum(["stdio"]).optional(), + command: z.string().min(1, "Command cannot be empty"), + args: z.array(z.string()).optional(), + env: z.record(z.string()).optional(), + // Ensure no SSE fields are present + url: z.undefined().optional(), + headers: z.undefined().optional(), + }) + .transform((data) => ({ + ...data, + type: "stdio" as const, + })) + .refine((data) => data.type === undefined || data.type === "stdio", { message: typeErrorMessage }), + // SSE config (has url field) + BaseConfigSchema.extend({ + type: z.enum(["sse"]).optional(), + url: z.string().url("URL must be a valid URL format"), + headers: z.record(z.string()).optional(), + // Ensure no stdio fields are present + command: z.undefined().optional(), + args: z.undefined().optional(), + env: z.undefined().optional(), + }) + .transform((data) => ({ + ...data, + type: "sse" as const, + })) + .refine((data) => data.type === undefined || data.type === "sse", { message: typeErrorMessage }), + ]) +} + +// Server configuration schema with automatic type inference and validation +export const ServerConfigSchema = createServerTypeSchema() // Settings schema const McpSettingsSchema = z.object({ @@ -84,6 +112,74 @@ export class McpHub { this.initializeMcpServers() } + /** + * Validates and normalizes server configuration + * @param config The server configuration to validate + * @param serverName Optional server name for error messages + * @returns The validated configuration + * @throws Error if the configuration is invalid + */ + private validateServerConfig(config: any, serverName?: string): z.infer { + // Detect configuration issues before validation + const hasStdioFields = config.command !== undefined + const hasSseFields = config.url !== undefined + + // Check for mixed fields + if (hasStdioFields && hasSseFields) { + throw new Error(mixedFieldsErrorMessage) + } + + // Check if it's a stdio or SSE config and add type if missing + if (!config.type) { + if (hasStdioFields) { + config.type = "stdio" + } else if (hasSseFields) { + config.type = "sse" + } else { + throw new Error(missingFieldsErrorMessage) + } + } else if (config.type !== "stdio" && config.type !== "sse") { + throw new Error(typeErrorMessage) + } + + // Check for type/field mismatch + if (config.type === "stdio" && !hasStdioFields) { + throw new Error(stdioFieldsErrorMessage) + } + if (config.type === "sse" && !hasSseFields) { + throw new Error(sseFieldsErrorMessage) + } + + // Validate the config against the schema + try { + return ServerConfigSchema.parse(config) + } catch (validationError) { + if (validationError instanceof z.ZodError) { + // Extract and format validation errors + const errorMessages = validationError.errors + .map((err) => `${err.path.join(".")}: ${err.message}`) + .join("; ") + throw new Error( + serverName + ? `Invalid configuration for server "${serverName}": ${errorMessages}` + : `Invalid server configuration: ${errorMessages}`, + ) + } + throw validationError + } + } + + /** + * Formats and displays error messages to the user + * @param message The error message prefix + * @param error The error object + */ + private showErrorMessage(message: string, error: unknown): void { + const errorMessage = error instanceof Error ? error.message : `${error}` + console.error(`${message}:`, error) + vscode.window.showErrorMessage(`${message}: ${errorMessage}`) + } + getServers(): McpServer[] { // Only return enabled servers return this.connections.filter((conn) => !conn.server.disabled).map((conn) => conn.server) @@ -133,7 +229,7 @@ export class McpHub { if (arePathsEqual(document.uri.fsPath, settingsPath)) { const content = await fs.readFile(settingsPath, "utf-8") const errorMessage = - "Invalid MCP settings format. Please ensure your settings follow the correct JSON format." + "Invalid MCP settings JSON format. Please ensure your settings follow the correct JSON format." let config: any try { config = JSON.parse(content) @@ -143,13 +239,16 @@ export class McpHub { } const result = McpSettingsSchema.safeParse(config) if (!result.success) { - vscode.window.showErrorMessage(errorMessage) + const errorMessages = result.error.errors + .map((err) => `${err.path.join(".")}: ${err.message}`) + .join("\n") + vscode.window.showErrorMessage(`Invalid MCP settings format: ${errorMessages}`) return } try { await this.updateServerConnections(result.data.mcpServers || {}) } catch (error) { - console.error("Failed to process MCP settings change:", error) + this.showErrorMessage("Failed to process MCP settings change", error) } } }), @@ -160,19 +259,39 @@ export class McpHub { try { const settingsPath = await this.getMcpSettingsFilePath() const content = await fs.readFile(settingsPath, "utf-8") - const config = JSON.parse(content) + let config: any + + try { + config = JSON.parse(content) + } catch (parseError) { + const errorMessage = + "Invalid MCP settings JSON format. Please check your settings file for syntax errors." + console.error(errorMessage, parseError) + vscode.window.showErrorMessage(errorMessage) + return + } // Validate the config using McpSettingsSchema const result = McpSettingsSchema.safeParse(config) if (result.success) { await this.updateServerConnections(result.data.mcpServers || {}) } else { - console.error("Invalid MCP settings format:", result.error) - // Still try to connect with the raw config - await this.updateServerConnections(config.mcpServers || {}) + // Format validation errors for better user feedback + const errorMessages = result.error.errors + .map((err) => `${err.path.join(".")}: ${err.message}`) + .join("\n") + console.error("Invalid MCP settings format:", errorMessages) + vscode.window.showErrorMessage(`Invalid MCP settings format: ${errorMessages}`) + + // Still try to connect with the raw config, but show warnings + try { + await this.updateServerConnections(config.mcpServers || {}) + } catch (error) { + this.showErrorMessage("Failed to initialize MCP servers with raw config", error) + } } } catch (error) { - console.error("Failed to initialize MCP servers:", error) + this.showErrorMessage("Failed to initialize MCP servers", error) } } @@ -210,7 +329,7 @@ export class McpHub { const connection = this.connections.find((conn) => conn.server.name === name) if (connection) { connection.server.status = "disconnected" - this.appendErrorMessage(connection, error.message) + this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`) } await this.notifyWebviewOfServerChanges() } @@ -269,7 +388,7 @@ export class McpHub { const connection = this.connections.find((conn) => conn.server.name === name) if (connection) { connection.server.status = "disconnected" - this.appendErrorMessage(connection, error.message) + this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`) } await this.notifyWebviewOfServerChanges() } @@ -301,15 +420,20 @@ export class McpHub { const connection = this.connections.find((conn) => conn.server.name === name) if (connection) { connection.server.status = "disconnected" - this.appendErrorMessage(connection, error instanceof Error ? error.message : String(error)) + this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`) } throw error } } private appendErrorMessage(connection: McpConnection, error: string) { + // Limit error message length to prevent excessive length + const maxErrorLength = 1000 const newError = connection.server.error ? `${connection.server.error}\n${error}` : error - connection.server.error = newError //.slice(0, 800) + connection.server.error = + newError.length > maxErrorLength + ? newError.substring(0, maxErrorLength) + "...(error message truncated)" + : newError } private async fetchToolsList(serverName: string): Promise { @@ -396,17 +520,9 @@ export class McpHub { // Validate and transform the config let validatedConfig: z.infer try { - // Check if it's a stdio or SSE config and add type if missing - if (!config.type) { - if (config.command) { - config.type = "stdio" - } else if (config.url) { - config.type = "sse" - } - } - validatedConfig = ServerConfigSchema.parse(config) + validatedConfig = this.validateServerConfig(config, name) } catch (error) { - console.error(`Invalid configuration for MCP server ${name}:`, error) + this.showErrorMessage(`Invalid configuration for MCP server "${name}"`, error) continue } @@ -416,7 +532,7 @@ export class McpHub { this.setupFileWatcher(name, validatedConfig) await this.connectToServer(name, validatedConfig) } catch (error) { - console.error(`Failed to connect to new MCP server ${name}:`, error) + this.showErrorMessage(`Failed to connect to new MCP server ${name}`, error) } } else if (!deepEqual(JSON.parse(currentConnection.server.config), config)) { // Existing server with changed config @@ -426,7 +542,7 @@ export class McpHub { await this.connectToServer(name, validatedConfig) console.log(`Reconnected MCP server with updated config: ${name}`) } catch (error) { - console.error(`Failed to reconnect MCP server ${name}:`, error) + this.showErrorMessage(`Failed to reconnect MCP server ${name}`, error) } } // If server exists with same config, do nothing @@ -480,12 +596,20 @@ export class McpHub { await delay(500) // artificial delay to show user that server is restarting try { await this.deleteConnection(serverName) - // Try to connect again using existing config - await this.connectToServer(serverName, JSON.parse(config)) - vscode.window.showInformationMessage(`${serverName} MCP server connected`) + // Parse the config to validate it + const parsedConfig = JSON.parse(config) + try { + // Validate the config + const validatedConfig = this.validateServerConfig(parsedConfig, serverName) + + // Try to connect again using validated config + await this.connectToServer(serverName, validatedConfig) + vscode.window.showInformationMessage(`${serverName} MCP server connected`) + } catch (validationError) { + this.showErrorMessage(`Invalid configuration for MCP server "${serverName}"`, validationError) + } } catch (error) { - console.error(`Failed to restart connection for ${serverName}:`, error) - vscode.window.showErrorMessage(`Failed to connect to ${serverName} MCP server`) + this.showErrorMessage(`Failed to restart ${serverName} MCP server connection`, error) } } @@ -575,13 +699,7 @@ export class McpHub { await this.notifyWebviewOfServerChanges() } } catch (error) { - console.error("Failed to update server disabled state:", error) - if (error instanceof Error) { - console.error("Error details:", error.message, error.stack) - } - vscode.window.showErrorMessage( - `Failed to update server state: ${error instanceof Error ? error.message : String(error)}`, - ) + this.showErrorMessage(`Failed to update server ${serverName} state`, error) throw error } } @@ -628,13 +746,7 @@ export class McpHub { await this.notifyWebviewOfServerChanges() } } catch (error) { - console.error("Failed to update server timeout:", error) - if (error instanceof Error) { - console.error("Error details:", error.message, error.stack) - } - vscode.window.showErrorMessage( - `Failed to update server timeout: ${error instanceof Error ? error.message : String(error)}`, - ) + this.showErrorMessage(`Failed to update server ${serverName} timeout settings`, error) throw error } } @@ -681,13 +793,7 @@ export class McpHub { vscode.window.showWarningMessage(`Server "${serverName}" not found in configuration`) } } catch (error) { - console.error("Failed to delete MCP server:", error) - if (error instanceof Error) { - console.error("Error details:", error.message, error.stack) - } - vscode.window.showErrorMessage( - `Failed to delete MCP server: ${error instanceof Error ? error.message : String(error)}`, - ) + this.showErrorMessage(`Failed to delete MCP server ${serverName}`, error) throw error } } @@ -783,8 +889,7 @@ export class McpHub { await this.notifyWebviewOfServerChanges() } } catch (error) { - console.error("Failed to update always allow settings:", error) - vscode.window.showErrorMessage("Failed to update always allow settings") + this.showErrorMessage(`Failed to update always allow settings for tool ${toolName}`, error) throw error // Re-throw to ensure the error is properly handled } }