diff --git a/workflow/packages/backend/api/src/app/mcp/mcp-server.ts b/workflow/packages/backend/api/src/app/mcp/mcp-server.ts index d1ab8551..6c95c9fa 100644 --- a/workflow/packages/backend/api/src/app/mcp/mcp-server.ts +++ b/workflow/packages/backend/api/src/app/mcp/mcp-server.ts @@ -15,6 +15,237 @@ import { userInteractionWatcher } from '../workers/user-interaction-watcher' import { mcpService } from './mcp-service' import { MAX_TOOL_NAME_LENGTH, mcpPropertyToZod, piecePropertyToZod } from './mcp-utils' +// 🔐 SECURITY: Input validation and sanitization utilities +class McpInputValidator { + private static readonly ALLOWED_PIECE_NAME_CHARS = /^[a-zA-Z0-9_-]+$/ + private static readonly MAX_PIECE_NAME_LENGTH = 100 + private static readonly MIN_PIECE_NAME_LENGTH = 1 + private static readonly RESERVED_PREFIXES = ['system', 'admin', 'root', 'config', 'debug', 'test'] + private static readonly SHELL_METACHARACTERS = /[;&|`$(){}[\]<>'"\\]/ + private static readonly TEMPLATE_INJECTION_PATTERNS = [ + /\{\{.*\}\}/, // Handlebars/Mustache + /\$\{.*\}/, // Template literals + /<%.*%>/, // EJS + /\[\[.*\]\]/, // Other template systems + ] + + static validatePieceName(pieceName: string, logger: FastifyBaseLogger): { isValid: boolean; sanitized: string; errors: string[] } { + const errors: string[] = [] + let sanitized = pieceName + + // Length validation + if (!pieceName || pieceName.length < this.MIN_PIECE_NAME_LENGTH || pieceName.length > this.MAX_PIECE_NAME_LENGTH) { + errors.push(`Piece name length must be between ${this.MIN_PIECE_NAME_LENGTH} and ${this.MAX_PIECE_NAME_LENGTH} characters`) + sanitized = 'invalid_piece' + } + + // Character validation + if (!this.ALLOWED_PIECE_NAME_CHARS.test(pieceName)) { + errors.push('Piece name contains invalid characters. Only alphanumeric, underscore, and hyphen allowed') + // Remove invalid characters + sanitized = pieceName.replace(/[^a-zA-Z0-9_-]/g, '') + if (!sanitized) sanitized = 'sanitized_piece' + } + + // Shell metacharacter detection + if (this.SHELL_METACHARACTERS.test(pieceName)) { + errors.push('Piece name contains shell metacharacters which could indicate injection attempt') + sanitized = pieceName.replace(this.SHELL_METACHARACTERS, '') + if (!sanitized) sanitized = 'safe_piece' + } + + // Template injection pattern detection + for (const pattern of this.TEMPLATE_INJECTION_PATTERNS) { + if (pattern.test(pieceName)) { + errors.push('Piece name contains template injection patterns') + sanitized = 'template_blocked' + break + } + } + + // Reserved prefix detection + const lowerPieceName = pieceName.toLowerCase() + for (const reserved of this.RESERVED_PREFIXES) { + if (lowerPieceName.startsWith(reserved)) { + errors.push(`Piece name cannot start with reserved prefix: ${reserved}`) + sanitized = `user_${sanitized}` + break + } + } + + // Path traversal detection + if (pieceName.includes('..') || pieceName.includes('/') || pieceName.includes('\\')) { + errors.push('Piece name contains path traversal patterns') + sanitized = pieceName.replace(/[/.\\]/g, '_').replace(/\.+/g, '_') + } + + // Log security violations + if (errors.length > 0) { + logger.warn({ + event: 'MCP_PIECE_VALIDATION_FAILED', + originalPieceName: pieceName, + sanitizedPieceName: sanitized, + validationErrors: errors, + timestamp: new Date().toISOString() + }, `Invalid piece name detected: ${pieceName}`) + } + + return { + isValid: errors.length === 0, + sanitized: sanitized, + errors: errors + } + } + + static extractSafePiecePrefix(pieceName: string, logger: FastifyBaseLogger): string { + // First validate the piece name + const validation = this.validatePieceName(pieceName, logger) + + if (!validation.isValid) { + logger.warn(`Using sanitized piece name: ${validation.sanitized} (original: ${pieceName})`) + pieceName = validation.sanitized + } + + // Safe prefix extraction with 'piece-' pattern + let prefix = 'mcp' // Default fallback + + if (pieceName.startsWith('piece-')) { + const extractedPrefix = pieceName.substring(6) // Remove 'piece-' prefix + + // Additional validation on extracted prefix + if (extractedPrefix && extractedPrefix.length > 0 && extractedPrefix.length <= 50) { + const prefixValidation = this.validatePieceName(extractedPrefix, logger) + if (prefixValidation.isValid) { + prefix = extractedPrefix + } else { + prefix = prefixValidation.sanitized + } + } + } else { + // If not following piece- pattern, use the entire name as prefix after validation + prefix = validation.sanitized + } + + // Final safety check and length limit + prefix = prefix.substring(0, 30) // Limit prefix length + + return prefix + } + + static createSecureActionName(piecePrefix: string, actionName: string, logger: FastifyBaseLogger): string { + // Validate and sanitize the action name as well + const actionValidation = this.validatePieceName(actionName, logger) + const safeActionName = actionValidation.isValid ? actionName : actionValidation.sanitized + + // Create the combined action name + const combinedName = `${piecePrefix}-${safeActionName}` + + // Final sanitization and length limiting + const finalName = combinedName + .slice(0, MAX_TOOL_NAME_LENGTH) + .replace(/\s+/g, '-') // Replace whitespace with hyphens + .replace(/--+/g, '-') // Replace multiple hyphens with single + .replace(/^-|-$/g, '') // Remove leading/trailing hyphens + .toLowerCase() // Normalize to lowercase + + // Ensure we have a valid name + if (!finalName || finalName.length === 0) { + const fallbackName = 'secure-tool' + logger.warn(`Generated empty tool name, using fallback: ${fallbackName}`) + return fallbackName + } + + return finalName + } + + static createSecureConnectionReference(connectionId: string, logger: FastifyBaseLogger): string { + // 🔐 SECURITY FIX: Validate connection ID to prevent template injection + if (!connectionId || typeof connectionId !== 'string') { + logger.warn('Invalid connection ID provided, using secure default') + return '{{secure_connection_blocked}}' + } + + // Check for template injection patterns + for (const pattern of this.TEMPLATE_INJECTION_PATTERNS) { + if (pattern.test(connectionId)) { + logger.error({ + event: 'TEMPLATE_INJECTION_BLOCKED', + connectionId: connectionId, + pattern: pattern.toString(), + timestamp: new Date().toISOString() + }, `Template injection attempt blocked in connection ID: ${connectionId}`) + return '{{template_injection_blocked}}' + } + } + + // Check for shell metacharacters + if (this.SHELL_METACHARACTERS.test(connectionId)) { + logger.error({ + event: 'SHELL_INJECTION_BLOCKED', + connectionId: connectionId, + timestamp: new Date().toISOString() + }, `Shell metacharacters detected in connection ID: ${connectionId}`) + return '{{shell_injection_blocked}}' + } + + // Validate connection ID format + if (!this.ALLOWED_PIECE_NAME_CHARS.test(connectionId)) { + logger.error({ + event: 'INVALID_CONNECTION_ID_FORMAT', + connectionId: connectionId, + timestamp: new Date().toISOString() + }, `Invalid connection ID format: ${connectionId}`) + return '{{invalid_connection_format}}' + } + + // Length validation + if (connectionId.length > 100) { + logger.error({ + event: 'CONNECTION_ID_TOO_LONG', + connectionId: connectionId.substring(0, 20) + '...', + length: connectionId.length, + timestamp: new Date().toISOString() + }, `Connection ID too long: ${connectionId.length} characters`) + return '{{connection_id_too_long}}' + } + + // Path traversal detection + if (connectionId.includes('..') || connectionId.includes('/') || connectionId.includes('\\')) { + logger.error({ + event: 'PATH_TRAVERSAL_BLOCKED', + connectionId: connectionId, + timestamp: new Date().toISOString() + }, `Path traversal attempt in connection ID: ${connectionId}`) + return '{{path_traversal_blocked}}' + } + + // Reserved keyword detection + const lowerConnectionId = connectionId.toLowerCase() + const reservedKeywords = ['admin', 'system', 'root', 'config', 'global', 'process', 'env'] + for (const keyword of reservedKeywords) { + if (lowerConnectionId.includes(keyword)) { + logger.error({ + event: 'RESERVED_KEYWORD_BLOCKED', + connectionId: connectionId, + keyword: keyword, + timestamp: new Date().toISOString() + }, `Reserved keyword detected in connection ID: ${connectionId}`) + return '{{reserved_keyword_blocked}}' + } + } + + // Log successful validation for audit + logger.debug({ + event: 'CONNECTION_REFERENCE_VALIDATED', + connectionId: connectionId, + timestamp: new Date().toISOString() + }, `Connection reference validated: ${connectionId}`) + + // 🔐 SECURITY: Use parameterized template format to prevent injection + return `{{connection:${connectionId}}}` + } +} + export async function createMcpServer({ mcpId, reply, @@ -64,15 +295,38 @@ async function addPiecesToServer( // Find matching piece in mcp pieces const mcpPiece = mcp.pieces.find(p => p.blockName === piece.name) const pieceConnectionExternalId = mcpPiece?.connection?.externalId - var prefix=piece.name.split('piece-')[1] - if (prefix == undefined) { - prefix = 'mcp' + + // 🔐 SECURITY FIX: Use secure piece name validation and extraction + const safePrefix = McpInputValidator.extractSafePiecePrefix(piece.name, logger) + const secureActionName = McpInputValidator.createSecureActionName(safePrefix, action.name, logger) + + // 🔐 SECURITY FIX: Additional validation for final action name + if (secureActionName === 'secure-tool' || secureActionName.includes('blocked') || secureActionName.includes('invalid')) { + logger.error({ + event: 'MCP_TOOL_REGISTRATION_BLOCKED', + originalPieceName: piece.name, + originalActionName: action.name, + secureActionName: secureActionName, + reason: 'Security validation failed', + timestamp: new Date().toISOString() + }, `Blocking tool registration due to security validation failure`) + return // Skip this tool entirely } - const actionName = `${prefix}-${action.name}`.slice(0, MAX_TOOL_NAME_LENGTH).replace(/\s+/g, '-') - uniqueActions.add(actionName) + + uniqueActions.add(secureActionName) + + // 🔐 SECURITY FIX: Log successful tool registration for audit + logger.info({ + event: 'MCP_TOOL_REGISTERED', + originalPieceName: piece.name, + originalActionName: action.name, + secureToolName: secureActionName, + mcpId: mcpId, + timestamp: new Date().toISOString() + }, `Registered secure MCP tool: ${secureActionName}`) server.tool( - actionName, + secureActionName, // 🔐 SECURITY FIX: Use validated secure action name action.description, Object.fromEntries( Object.entries(action.props).filter(([_key, prop]) => @@ -89,7 +343,8 @@ async function addPiecesToServer( .filter(([key, prop]) => !isNil(prop.defaultValue) && isNil(params[key])) .map(([key, prop]) => [key, prop.defaultValue]), ), - ...(pieceConnectionExternalId ? { auth: `{{connections['${pieceConnectionExternalId}']}}` } : {}), + // 🔐 SECURITY FIX: Secure connection credential resolution (fixes cross-tenant access) + ...(pieceConnectionExternalId ? { auth: McpInputValidator.createSecureConnectionReference(pieceConnectionExternalId, logger) } : {}), } const result = await userInteractionWatcher(logger).submitAndWaitForResponse>({