mirror of
https://github.com/Tencent/WeKnora.git
synced 2026-09-19 10:28:49 +08:00
feat: 新增MCP stdio传输安全验证机制,防止命令注入攻击
This commit is contained in:
@@ -31,6 +31,22 @@ func NewMCPServiceService(
|
||||
|
||||
// CreateMCPService creates a new MCP service
|
||||
func (s *mcpServiceService) CreateMCPService(ctx context.Context, service *types.MCPService) error {
|
||||
// Security validation for stdio transport type
|
||||
if service.TransportType == types.MCPTransportStdio {
|
||||
if service.StdioConfig == nil {
|
||||
return fmt.Errorf("stdio_config is required for stdio transport")
|
||||
}
|
||||
// Validate stdio configuration to prevent command injection (CWE-78)
|
||||
if err := secutils.ValidateStdioConfig(
|
||||
service.StdioConfig.Command,
|
||||
service.StdioConfig.Args,
|
||||
service.EnvVars,
|
||||
); err != nil {
|
||||
logger.GetLogger(ctx).Warnf("MCP service creation blocked due to security validation: %v", err)
|
||||
return fmt.Errorf("security validation failed: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Set default advanced config if not provided
|
||||
if service.AdvancedConfig == nil {
|
||||
service.AdvancedConfig = types.GetDefaultAdvancedConfig()
|
||||
@@ -113,6 +129,33 @@ func (s *mcpServiceService) UpdateMCPService(ctx context.Context, service *types
|
||||
return fmt.Errorf("MCP service not found")
|
||||
}
|
||||
|
||||
// Security validation for stdio transport type when updating stdio config
|
||||
// Determine the final transport type and stdio config after merge
|
||||
finalTransportType := existing.TransportType
|
||||
if service.TransportType != "" {
|
||||
finalTransportType = service.TransportType
|
||||
}
|
||||
finalStdioConfig := existing.StdioConfig
|
||||
if service.StdioConfig != nil {
|
||||
finalStdioConfig = service.StdioConfig
|
||||
}
|
||||
finalEnvVars := existing.EnvVars
|
||||
if service.EnvVars != nil {
|
||||
finalEnvVars = service.EnvVars
|
||||
}
|
||||
|
||||
// Validate if the final configuration uses stdio transport
|
||||
if finalTransportType == types.MCPTransportStdio && finalStdioConfig != nil {
|
||||
if err := secutils.ValidateStdioConfig(
|
||||
finalStdioConfig.Command,
|
||||
finalStdioConfig.Args,
|
||||
finalEnvVars,
|
||||
); err != nil {
|
||||
logger.GetLogger(ctx).Warnf("MCP service update blocked due to security validation: %v", err)
|
||||
return fmt.Errorf("security validation failed: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Store old enabled state BEFORE any updates
|
||||
oldEnabled := existing.Enabled
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
|
||||
"github.com/Tencent/WeKnora/internal/logger"
|
||||
"github.com/Tencent/WeKnora/internal/types"
|
||||
secutils "github.com/Tencent/WeKnora/internal/utils"
|
||||
"github.com/mark3labs/mcp-go/client"
|
||||
"github.com/mark3labs/mcp-go/client/transport"
|
||||
"github.com/mark3labs/mcp-go/mcp"
|
||||
@@ -122,6 +123,16 @@ func NewMCPClient(config *ClientConfig) (MCPClient, error) {
|
||||
return nil, fmt.Errorf("stdio_config is required for stdio transport")
|
||||
}
|
||||
|
||||
// Security validation: validate command, args, and env vars before execution
|
||||
// This prevents command injection attacks (CWE-78)
|
||||
if err := secutils.ValidateStdioConfig(
|
||||
config.Service.StdioConfig.Command,
|
||||
config.Service.StdioConfig.Args,
|
||||
config.Service.EnvVars,
|
||||
); err != nil {
|
||||
return nil, fmt.Errorf("stdio configuration validation failed: %w", err)
|
||||
}
|
||||
|
||||
// Convert env vars map to []string format (KEY=value)
|
||||
envVars := make([]string, 0, len(config.Service.EnvVars))
|
||||
for key, value := range config.Service.EnvVars {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package utils
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"html"
|
||||
"regexp"
|
||||
"strings"
|
||||
@@ -217,3 +218,163 @@ func SanitizeForLogArray(input []string) []string {
|
||||
|
||||
return sanitized
|
||||
}
|
||||
|
||||
// AllowedStdioCommands defines the whitelist of allowed commands for MCP stdio transport
|
||||
// These are the standard MCP server launchers that are considered safe
|
||||
var AllowedStdioCommands = map[string]bool{
|
||||
"uvx": true, // Python package runner (uv)
|
||||
"npx": true, // Node.js package runner
|
||||
}
|
||||
|
||||
// DangerousArgPatterns contains patterns that indicate potentially dangerous arguments
|
||||
var DangerousArgPatterns = []*regexp.Regexp{
|
||||
regexp.MustCompile(`(?i)^-c$`), // Shell command execution flag
|
||||
regexp.MustCompile(`(?i)^--command$`), // Shell command execution flag
|
||||
regexp.MustCompile(`(?i)^-e$`), // Eval flag
|
||||
regexp.MustCompile(`(?i)^--eval$`), // Eval flag
|
||||
regexp.MustCompile(`(?i)[;&|]`), // Shell command chaining
|
||||
regexp.MustCompile(`(?i)\$\(`), // Command substitution
|
||||
regexp.MustCompile("(?i)`"), // Backtick command substitution
|
||||
regexp.MustCompile(`(?i)>\s*[/~]`), // Output redirection to absolute/home path
|
||||
regexp.MustCompile(`(?i)<\s*[/~]`), // Input redirection from absolute/home path
|
||||
regexp.MustCompile(`(?i)^/bin/`), // Direct binary path
|
||||
regexp.MustCompile(`(?i)^/usr/bin/`), // Direct binary path
|
||||
regexp.MustCompile(`(?i)^/sbin/`), // Direct binary path
|
||||
regexp.MustCompile(`(?i)^/usr/sbin/`), // Direct binary path
|
||||
regexp.MustCompile(`(?i)^\.\./`), // Path traversal
|
||||
regexp.MustCompile(`(?i)/\.\./`), // Path traversal in middle
|
||||
regexp.MustCompile(`(?i)^(bash|sh|zsh|ksh|csh|tcsh|fish|dash)$`), // Shell interpreters as args
|
||||
regexp.MustCompile(`(?i)^(curl|wget|nc|netcat|ncat)$`), // Network tools as args
|
||||
regexp.MustCompile(`(?i)^(rm|dd|mkfs|fdisk)$`), // Destructive commands as args
|
||||
}
|
||||
|
||||
// DangerousEnvVarPatterns contains patterns for dangerous environment variable names or values
|
||||
var DangerousEnvVarPatterns = []*regexp.Regexp{
|
||||
regexp.MustCompile(`(?i)^LD_PRELOAD$`), // Library injection
|
||||
regexp.MustCompile(`(?i)^LD_LIBRARY_PATH$`), // Library path manipulation
|
||||
regexp.MustCompile(`(?i)^DYLD_`), // macOS dynamic linker
|
||||
regexp.MustCompile(`(?i)^PATH$`), // PATH manipulation
|
||||
regexp.MustCompile(`(?i)^PYTHONPATH$`), // Python path manipulation
|
||||
regexp.MustCompile(`(?i)^NODE_OPTIONS$`), // Node.js options injection
|
||||
regexp.MustCompile(`(?i)^BASH_ENV$`), // Bash environment file
|
||||
regexp.MustCompile(`(?i)^ENV$`), // Shell environment file
|
||||
regexp.MustCompile(`(?i)^SHELL$`), // Shell override
|
||||
}
|
||||
|
||||
// ValidateStdioCommand validates the command for MCP stdio transport
|
||||
// Returns an error if the command is not in the whitelist or contains dangerous patterns
|
||||
func ValidateStdioCommand(command string) error {
|
||||
if command == "" {
|
||||
return fmt.Errorf("command cannot be empty")
|
||||
}
|
||||
|
||||
// Normalize command (extract base name if it's a path)
|
||||
baseCommand := command
|
||||
if strings.Contains(command, "/") {
|
||||
parts := strings.Split(command, "/")
|
||||
baseCommand = parts[len(parts)-1]
|
||||
}
|
||||
|
||||
// Check against whitelist
|
||||
if !AllowedStdioCommands[baseCommand] {
|
||||
return fmt.Errorf("command '%s' is not in the allowed list. Allowed commands: uvx, npx, node, python, python3, deno, bun", baseCommand)
|
||||
}
|
||||
|
||||
// Additional check: command should not contain path traversal
|
||||
if strings.Contains(command, "..") {
|
||||
return fmt.Errorf("command path contains invalid characters")
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// ValidateStdioArgs validates the arguments for MCP stdio transport
|
||||
// Returns an error if any argument contains dangerous patterns
|
||||
func ValidateStdioArgs(args []string) error {
|
||||
if len(args) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
for i, arg := range args {
|
||||
// Check length
|
||||
if len(arg) > 1024 {
|
||||
return fmt.Errorf("argument %d exceeds maximum length (1024 characters)", i)
|
||||
}
|
||||
|
||||
// Check against dangerous patterns
|
||||
for _, pattern := range DangerousArgPatterns {
|
||||
if pattern.MatchString(arg) {
|
||||
return fmt.Errorf("argument %d contains potentially dangerous pattern: %s", i, SanitizeForLog(arg))
|
||||
}
|
||||
}
|
||||
|
||||
// Check for null bytes
|
||||
if strings.Contains(arg, "\x00") {
|
||||
return fmt.Errorf("argument %d contains null bytes", i)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// ValidateStdioEnvVars validates environment variables for MCP stdio transport
|
||||
// Returns an error if any env var name or value is dangerous
|
||||
func ValidateStdioEnvVars(envVars map[string]string) error {
|
||||
if len(envVars) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
for key, value := range envVars {
|
||||
// Check key against dangerous patterns
|
||||
for _, pattern := range DangerousEnvVarPatterns {
|
||||
if pattern.MatchString(key) {
|
||||
return fmt.Errorf("environment variable '%s' is not allowed for security reasons", key)
|
||||
}
|
||||
}
|
||||
|
||||
// Check key length
|
||||
if len(key) > 256 {
|
||||
return fmt.Errorf("environment variable name '%s' exceeds maximum length", SanitizeForLog(key[:50]))
|
||||
}
|
||||
|
||||
// Check value length
|
||||
if len(value) > 4096 {
|
||||
return fmt.Errorf("environment variable '%s' value exceeds maximum length", key)
|
||||
}
|
||||
|
||||
// Check for null bytes in value
|
||||
if strings.Contains(value, "\x00") {
|
||||
return fmt.Errorf("environment variable '%s' value contains null bytes", key)
|
||||
}
|
||||
|
||||
// Check value for shell injection patterns
|
||||
for _, pattern := range DangerousArgPatterns {
|
||||
if pattern.MatchString(value) {
|
||||
return fmt.Errorf("environment variable '%s' value contains potentially dangerous pattern", key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// ValidateStdioConfig performs comprehensive validation of stdio configuration
|
||||
// This should be called before creating or executing any stdio-based MCP client
|
||||
func ValidateStdioConfig(command string, args []string, envVars map[string]string) error {
|
||||
// Validate command
|
||||
if err := ValidateStdioCommand(command); err != nil {
|
||||
return fmt.Errorf("invalid command: %w", err)
|
||||
}
|
||||
|
||||
// Validate arguments
|
||||
if err := ValidateStdioArgs(args); err != nil {
|
||||
return fmt.Errorf("invalid arguments: %w", err)
|
||||
}
|
||||
|
||||
// Validate environment variables
|
||||
if err := ValidateStdioEnvVars(envVars); err != nil {
|
||||
return fmt.Errorf("invalid environment variables: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user