From 48e6630e9264766fbf9f3f81ac2dfc9e347ef643 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 18 Sep 2025 12:40:47 +0100 Subject: [PATCH] feat: enhance security validation for MCP configurations - Added environment variable checks for CUSTOM_MCP_SECURITY_CHECK, CUSTOM_MCP_PROTOCOL, and HTTP_DENY_LIST across various Docker and application files. - Implemented validation functions in MCP core to prevent command injection and ensure safe environment variable usage --- docker/.env.example | 11 +++- docker/docker-compose-queue-prebuilt.yml | 10 +++ docker/docker-compose.yml | 5 ++ docker/worker/.env.example | 11 +++- docker/worker/docker-compose.yml | 6 ++ .../nodes/tools/MCP/CustomMCP/CustomMCP.ts | 14 +++- .../tools/MCP/Supergateway/SupergatewayMCP.ts | 6 +- packages/components/nodes/tools/MCP/core.ts | 64 +++++++++++++++++++ packages/server/.env.example | 2 + packages/server/src/commands/base.ts | 10 ++- 10 files changed, 130 insertions(+), 9 deletions(-) diff --git a/docker/.env.example b/docker/.env.example index 07b65a45d..8bb6afc5a 100644 --- a/docker/.env.example +++ b/docker/.env.example @@ -163,4 +163,13 @@ JWT_REFRESH_TOKEN_EXPIRY_IN_MINUTES=43200 # REDIS_KEY= # REDIS_CA= # REDIS_KEEP_ALIVE= -# ENABLE_BULLMQ_DASHBOARD= \ No newline at end of file +# ENABLE_BULLMQ_DASHBOARD= + + +############################################################################################################ +############################################## SECURITY #################################################### +############################################################################################################ + +# HTTP_DENY_LIST= +# CUSTOM_MCP_SECURITY_CHECK=true +# CUSTOM_MCP_PROTOCOL=sse #(stdio | sse) \ No newline at end of file diff --git a/docker/docker-compose-queue-prebuilt.yml b/docker/docker-compose-queue-prebuilt.yml index 0115af1b2..af881f0f4 100644 --- a/docker/docker-compose-queue-prebuilt.yml +++ b/docker/docker-compose-queue-prebuilt.yml @@ -139,6 +139,11 @@ services: - REDIS_CA=${REDIS_CA} - REDIS_KEEP_ALIVE=${REDIS_KEEP_ALIVE} - ENABLE_BULLMQ_DASHBOARD=${ENABLE_BULLMQ_DASHBOARD} + + # SECURITY + - CUSTOM_MCP_SECURITY_CHECK=${CUSTOM_MCP_SECURITY_CHECK} + - CUSTOM_MCP_PROTOCOL=${CUSTOM_MCP_PROTOCOL} + - HTTP_DENY_LIST=${HTTP_DENY_LIST} healthcheck: test: ['CMD', 'curl', '-f', 'http://localhost:${PORT:-3000}/api/v1/ping'] interval: 10s @@ -276,6 +281,11 @@ services: - REDIS_CA=${REDIS_CA} - REDIS_KEEP_ALIVE=${REDIS_KEEP_ALIVE} - ENABLE_BULLMQ_DASHBOARD=${ENABLE_BULLMQ_DASHBOARD} + + # SECURITY + - CUSTOM_MCP_SECURITY_CHECK=${CUSTOM_MCP_SECURITY_CHECK} + - CUSTOM_MCP_PROTOCOL=${CUSTOM_MCP_PROTOCOL} + - HTTP_DENY_LIST=${HTTP_DENY_LIST} healthcheck: test: ['CMD', 'curl', '-f', 'http://localhost:${WORKER_PORT:-5566}/healthz'] interval: 10s diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 5b476977b..2ffcfb0b9 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -124,6 +124,11 @@ services: - REDIS_CA=${REDIS_CA} - REDIS_KEEP_ALIVE=${REDIS_KEEP_ALIVE} - ENABLE_BULLMQ_DASHBOARD=${ENABLE_BULLMQ_DASHBOARD} + + # SECURITY + - CUSTOM_MCP_SECURITY_CHECK=${CUSTOM_MCP_SECURITY_CHECK} + - CUSTOM_MCP_PROTOCOL=${CUSTOM_MCP_PROTOCOL} + - HTTP_DENY_LIST=${HTTP_DENY_LIST} ports: - '${PORT}:${PORT}' healthcheck: diff --git a/docker/worker/.env.example b/docker/worker/.env.example index 926224ed2..8bc6afd52 100644 --- a/docker/worker/.env.example +++ b/docker/worker/.env.example @@ -163,4 +163,13 @@ JWT_REFRESH_TOKEN_EXPIRY_IN_MINUTES=43200 # REDIS_KEY= # REDIS_CA= # REDIS_KEEP_ALIVE= -# ENABLE_BULLMQ_DASHBOARD= \ No newline at end of file +# ENABLE_BULLMQ_DASHBOARD= + + +############################################################################################################ +############################################## SECURITY #################################################### +############################################################################################################ + +# HTTP_DENY_LIST= +# CUSTOM_MCP_SECURITY_CHECK=true +# CUSTOM_MCP_PROTOCOL=sse #(stdio | sse) \ No newline at end of file diff --git a/docker/worker/docker-compose.yml b/docker/worker/docker-compose.yml index 703dfed7b..b49e3dcc6 100644 --- a/docker/worker/docker-compose.yml +++ b/docker/worker/docker-compose.yml @@ -124,6 +124,12 @@ services: - REDIS_CA=${REDIS_CA} - REDIS_KEEP_ALIVE=${REDIS_KEEP_ALIVE} - ENABLE_BULLMQ_DASHBOARD=${ENABLE_BULLMQ_DASHBOARD} + + # SECURITY + - CUSTOM_MCP_SECURITY_CHECK=${CUSTOM_MCP_SECURITY_CHECK} + - CUSTOM_MCP_PROTOCOL=${CUSTOM_MCP_PROTOCOL} + - HTTP_DENY_LIST=${HTTP_DENY_LIST} + ports: - '${WORKER_PORT}:${WORKER_PORT}' healthcheck: diff --git a/packages/components/nodes/tools/MCP/CustomMCP/CustomMCP.ts b/packages/components/nodes/tools/MCP/CustomMCP/CustomMCP.ts index 8592692a6..1e03f31f2 100644 --- a/packages/components/nodes/tools/MCP/CustomMCP/CustomMCP.ts +++ b/packages/components/nodes/tools/MCP/CustomMCP/CustomMCP.ts @@ -1,6 +1,6 @@ import { Tool } from '@langchain/core/tools' import { ICommonObject, IDatabaseEntity, INode, INodeData, INodeOptionsValue, INodeParams } from '../../../../src/Interface' -import { MCPToolkit } from '../core' +import { MCPToolkit, validateMCPServerConfig } from '../core' import { getVars, prepareSandboxVars } from '../../../../src/utils' import { DataSource } from 'typeorm' import hash from 'object-hash' @@ -75,8 +75,8 @@ class Custom_MCP implements INode { }, placeholder: mcpServerConfig, warning: - process.env.CUSTOM_MCP_SECURITY_CHECK === 'true' - ? 'In next release, only Remote MCP with url is supported. Read more here' + process.env.CUSTOM_MCP_PROTOCOL === 'sse' + ? 'Only Remote MCP with url is supported. Read more here' : undefined }, { @@ -174,6 +174,14 @@ class Custom_MCP implements INode { serverParams = JSON.parse(serverParamsString) } + if (process.env.CUSTOM_MCP_SECURITY_CHECK !== 'false') { + try { + validateMCPServerConfig(serverParams) + } catch (error) { + throw new Error(`Security validation failed: ${error.message}`) + } + } + // Compatible with stdio and SSE let toolkit: MCPToolkit if (process.env.CUSTOM_MCP_PROTOCOL === 'sse') { diff --git a/packages/components/nodes/tools/MCP/Supergateway/SupergatewayMCP.ts b/packages/components/nodes/tools/MCP/Supergateway/SupergatewayMCP.ts index 610ef5b79..1960928e6 100644 --- a/packages/components/nodes/tools/MCP/Supergateway/SupergatewayMCP.ts +++ b/packages/components/nodes/tools/MCP/Supergateway/SupergatewayMCP.ts @@ -1,7 +1,7 @@ import { Tool } from '@langchain/core/tools' import { ICommonObject, INode, INodeData, INodeOptionsValue, INodeParams } from '../../../../src/Interface' import { getNodeModulesPackagePath } from '../../../../src/utils' -import { MCPToolkit, validateArgsForLocalFileAccess } from '../core' +import { MCPToolkit, validateMCPServerConfig } from '../core' class Supergateway_MCP implements INode { label: string @@ -106,9 +106,9 @@ class Supergateway_MCP implements INode { args: [packagePath, ...processedArgs] } - if (process.env.CUSTOM_MCP_SECURITY_CHECK === 'true') { + if (process.env.CUSTOM_MCP_SECURITY_CHECK !== 'false') { try { - validateArgsForLocalFileAccess(processedArgs) + validateMCPServerConfig(serverParams) } catch (error) { throw new Error(`Security validation failed: ${error.message}`) } diff --git a/packages/components/nodes/tools/MCP/core.ts b/packages/components/nodes/tools/MCP/core.ts index b2c9e63f6..12bc4f979 100644 --- a/packages/components/nodes/tools/MCP/core.ts +++ b/packages/components/nodes/tools/MCP/core.ts @@ -219,3 +219,67 @@ export const validateArgsForLocalFileAccess = (args: string[]): void => { } } } + +export const validateCommandInjection = (args: string[]): void => { + const dangerousPatterns = [ + // Shell metacharacters + /[;&|`$(){}[\]<>]/, + // Command chaining + /&&|\|\||;;/, + // Redirections + />>|<<|>/, + // Backticks and command substitution + /`|\$\(/, + // Process substitution + /<\(|>\(/ + ] + + for (const arg of args) { + if (typeof arg !== 'string') continue + + for (const pattern of dangerousPatterns) { + if (pattern.test(arg)) { + throw new Error(`Argument contains potentially dangerous characters: "${arg}"`) + } + } + } +} + +export const validateEnvironmentVariables = (env: Record): void => { + const dangerousEnvVars = ['PATH', 'LD_LIBRARY_PATH', 'DYLD_LIBRARY_PATH'] + + for (const [key, value] of Object.entries(env)) { + if (dangerousEnvVars.includes(key)) { + throw new Error(`Environment variable '${key}' modification is not allowed`) + } + + if (typeof value === 'string' && value.includes('\0')) { + throw new Error(`Environment variable '${key}' contains null byte`) + } + } +} + +export const validateMCPServerConfig = (serverParams: any): void => { + // Validate the entire server configuration + if (!serverParams || typeof serverParams !== 'object') { + throw new Error('Invalid server configuration') + } + + // Command allowlist - only allow specific safe commands + const allowedCommands = ['node', 'npx', 'python', 'python3', 'docker'] + + if (serverParams.command && !allowedCommands.includes(serverParams.command)) { + throw new Error(`Command '${serverParams.command}' is not allowed. Allowed commands: ${allowedCommands.join(', ')}`) + } + + // Validate arguments if present + if (serverParams.args && Array.isArray(serverParams.args)) { + validateArgsForLocalFileAccess(serverParams.args) + validateCommandInjection(serverParams.args) + } + + // Validate environment variables + if (serverParams.env) { + validateEnvironmentVariables(serverParams.env) + } +} diff --git a/packages/server/.env.example b/packages/server/.env.example index 2c9fcce91..84fc6ce78 100644 --- a/packages/server/.env.example +++ b/packages/server/.env.example @@ -171,6 +171,8 @@ JWT_REFRESH_TOKEN_EXPIRY_IN_MINUTES=43200 ############################################################################################################ # HTTP_DENY_LIST= +# CUSTOM_MCP_SECURITY_CHECK=true +# CUSTOM_MCP_PROTOCOL=sse #(stdio | sse) ############################################################################################################ diff --git a/packages/server/src/commands/base.ts b/packages/server/src/commands/base.ts index cc40e367f..d3dfbf430 100644 --- a/packages/server/src/commands/base.ts +++ b/packages/server/src/commands/base.ts @@ -74,7 +74,10 @@ export abstract class BaseCommand extends Command { REDIS_KEY: Flags.string(), REDIS_CA: Flags.string(), REDIS_KEEP_ALIVE: Flags.string(), - ENABLE_BULLMQ_DASHBOARD: Flags.string() + ENABLE_BULLMQ_DASHBOARD: Flags.string(), + CUSTOM_MCP_SECURITY_CHECK: Flags.string(), + CUSTOM_MCP_PROTOCOL: Flags.string(), + HTTP_DENY_LIST: Flags.string() } protected async stopProcess() { @@ -202,5 +205,10 @@ export abstract class BaseCommand extends Command { if (flags.REMOVE_ON_COUNT) process.env.REMOVE_ON_COUNT = flags.REMOVE_ON_COUNT if (flags.REDIS_KEEP_ALIVE) process.env.REDIS_KEEP_ALIVE = flags.REDIS_KEEP_ALIVE if (flags.ENABLE_BULLMQ_DASHBOARD) process.env.ENABLE_BULLMQ_DASHBOARD = flags.ENABLE_BULLMQ_DASHBOARD + + // Security + if (flags.CUSTOM_MCP_SECURITY_CHECK) process.env.CUSTOM_MCP_SECURITY_CHECK = flags.CUSTOM_MCP_SECURITY_CHECK + if (flags.CUSTOM_MCP_PROTOCOL) process.env.CUSTOM_MCP_PROTOCOL = flags.CUSTOM_MCP_PROTOCOL + if (flags.HTTP_DENY_LIST) process.env.HTTP_DENY_LIST = flags.HTTP_DENY_LIST } }