feature/env vars filter (#11)

* Implement secure environment variable management

- Added `secureenv` package for filtering environment variables based on configuration.
- Introduced `EnvConfig` struct to manage allowed system variables and custom variables.
- Updated `Config` struct to include environment configuration.
- Enhanced `DefaultConfig` to initialize secure environment settings.
- Integrated secure environment manager into upstream and client components.
- Added tests for secure environment functionality and integration scenarios.

* Enhance timeout management and logging in client and server components

- Updated timeout duration for tool listing operations to 30 seconds for better handling of Docker container startup and API calls.
- Improved logging for various operations, including connection errors and tool calls, to provide more detailed insights during debugging.
- Added conditional logging for JSON request/response data based on debug level, enhancing traceability without cluttering logs.

* Refactor code for clarity and consistency

- Removed unnecessary blank lines in config validation and test functions.
- Simplified environment variable splitting logic in tests.
- Introduced constants for OS checks to improve readability and maintainability.
- Updated autostart functionality checks to use constants for OS types.
- Enhanced error handling in file operations for better robustness.

Author: Dumbris

internal/config/config.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"encoding/json"
+	"mcpproxy-go/internal/secureenv"
 	"time"
 )
 
@@ -20,6 +21,9 @@ type Config struct {
 	ToolsLimit        int             `json:"tools_limit" mapstructure:"tools-limit"`
 	ToolResponseLimit int             `json:"tool_response_limit" mapstructure:"tool-response-limit"`
 
+	// Environment configuration for secure variable filtering
+	Environment *secureenv.EnvConfig `json:"environment,omitempty" mapstructure:"environment"`
+
 	// Logging configuration
 	Logging *LogConfig `json:"logging,omitempty" mapstructure:"logging"`
 
@@ -154,6 +158,9 @@ func DefaultConfig() *Config {
 		ToolsLimit:        15,
 		ToolResponseLimit: 20000, // Default 20000 characters
 
+		// Default secure environment configuration
+		Environment: secureenv.DefaultEnvConfig(),
+
 		// Default logging configuration
 		Logging: &LogConfig{
 			Level:         "info",
@@ -192,6 +199,12 @@ func (c *Config) Validate() error {
 	if c.ToolResponseLimit < 0 {
 		c.ToolResponseLimit = 0 // 0 means disabled
 	}
+
+	// Ensure Environment config is not nil
+	if c.Environment == nil {
+		c.Environment = secureenv.DefaultEnvConfig()
+	}
+
 	return nil
 }
 

internal/config/loader.go

@@ -297,26 +297,21 @@ func LoadOrCreateConfig(dataDir string) (*Config, error) {
 
 // CreateSampleConfig creates a sample configuration file
 func CreateSampleConfig(path string) error {
-	cfg := &Config{
-		Listen:     ":8080",
-		EnableTray: true,
-		TopK:       5,
-		ToolsLimit: 15,
-		Servers: []*ServerConfig{
-			{
-				Name:    "example",
-				URL:     "http://localhost:8000/mcp/",
-				Enabled: true,
-				Created: now(),
-			},
-			{
-				Name:    "local-command",
-				Command: "mcp-server-example",
-				Args:    []string{"--config", "example.json"},
-				Env:     map[string]string{"DEBUG": "true"},
-				Enabled: true,
-				Created: now(),
-			},
+	cfg := DefaultConfig()
+	cfg.Servers = []*ServerConfig{
+		{
+			Name:    "example",
+			URL:     "http://localhost:8000/mcp/",
+			Enabled: true,
+			Created: now(),
+		},
+		{
+			Name:    "local-command",
+			Command: "mcp-server-example",
+			Args:    []string{"--config", "example.json"},
+			Env:     map[string]string{"DEBUG": "true"},
+			Enabled: true,
+			Created: now(),
 		},
 	}
 

internal/secureenv/manager.go

@@ -0,0 +1,184 @@
+package secureenv
+
+import (
+	"os"
+	"runtime"
+	"strings"
+)
+
+const (
+	osWindows = "windows"
+)
+
+// EnvConfig represents environment configuration for secure filtering
+type EnvConfig struct {
+	InheritSystemSafe bool              `json:"inherit_system_safe"`
+	AllowedSystemVars []string          `json:"allowed_system_vars"`
+	CustomVars        map[string]string `json:"custom_vars"`
+}
+
+// DefaultEnvConfig returns default environment configuration with safe system variables
+func DefaultEnvConfig() *EnvConfig {
+	allowedVars := []string{
+		"PATH",     // Essential for finding executables
+		"HOME",     // User directory path (Unix)
+		"TMPDIR",   // Temporary directory (Unix)
+		"TEMP",     // Temporary directory (Windows)
+		"TMP",      // Temporary directory (Windows)
+		"SHELL",    // Default shell
+		"TERM",     // Terminal type
+		"LANG",     // Language settings
+		"USER",     // Current user (Unix)
+		"USERNAME", // Current user (Windows)
+	}
+
+	// Add Windows-specific variables
+	if runtime.GOOS == osWindows {
+		allowedVars = append(allowedVars,
+			"USERPROFILE",  // User profile directory
+			"APPDATA",      // Application data directory
+			"LOCALAPPDATA", // Local application data directory
+			"PROGRAMFILES", // Program files directory
+			"SYSTEMROOT",   // System root directory
+			"COMSPEC",      // Command interpreter
+		)
+	}
+
+	// Add Unix-specific variables
+	if runtime.GOOS != osWindows {
+		allowedVars = append(allowedVars,
+			"XDG_CONFIG_HOME", // XDG config directory
+			"XDG_DATA_HOME",   // XDG data directory
+			"XDG_CACHE_HOME",  // XDG cache directory
+			"XDG_RUNTIME_DIR", // XDG runtime directory
+		)
+	}
+
+	// Add locale-related variables
+	localeVars := []string{
+		"LC_ALL", "LC_CTYPE", "LC_NUMERIC", "LC_TIME", "LC_COLLATE",
+		"LC_MONETARY", "LC_MESSAGES", "LC_PAPER", "LC_NAME", "LC_ADDRESS",
+		"LC_TELEPHONE", "LC_MEASUREMENT", "LC_IDENTIFICATION",
+	}
+	allowedVars = append(allowedVars, localeVars...)
+
+	return &EnvConfig{
+		InheritSystemSafe: true,
+		AllowedSystemVars: allowedVars,
+		CustomVars:        make(map[string]string),
+	}
+}
+
+// Manager handles secure environment variable filtering
+type Manager struct {
+	config *EnvConfig
+}
+
+// NewManager creates a new secure environment manager
+func NewManager(config *EnvConfig) *Manager {
+	if config == nil {
+		config = DefaultEnvConfig()
+	}
+	return &Manager{config: config}
+}
+
+// BuildSecureEnvironment builds a secure environment variable list
+func (m *Manager) BuildSecureEnvironment() []string {
+	var envVars []string
+
+	// Add safe system environment variables if enabled
+	if m.config.InheritSystemSafe {
+		envVars = append(envVars, m.getFilteredSystemEnv()...)
+	}
+
+	// Add custom environment variables from config
+	for k, v := range m.config.CustomVars {
+		envVars = append(envVars, k+"="+v)
+	}
+
+	return envVars
+}
+
+// getFilteredSystemEnv returns filtered system environment variables
+func (m *Manager) getFilteredSystemEnv() []string {
+	systemEnv := os.Environ()
+	var filtered []string
+
+	for _, envVar := range systemEnv {
+		if m.isEnvVarAllowed(envVar) {
+			filtered = append(filtered, envVar)
+		}
+	}
+
+	return filtered
+}
+
+// isEnvVarAllowed checks if an environment variable is allowed based on the allow-list
+func (m *Manager) isEnvVarAllowed(envVar string) bool {
+	parts := strings.SplitN(envVar, "=", 2)
+	if len(parts) != 2 {
+		return false
+	}
+
+	key := parts[0]
+
+	// Check against allow-list
+	for _, allowedVar := range m.config.AllowedSystemVars {
+		if key == allowedVar {
+			return true
+		}
+
+		// Support wildcard matching for locale variables (LC_*)
+		if strings.HasSuffix(allowedVar, "*") {
+			prefix := strings.TrimSuffix(allowedVar, "*")
+			if strings.HasPrefix(key, prefix) {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
+// GetSystemEnvVar gets a specific system environment variable if allowed
+func (m *Manager) GetSystemEnvVar(key string) (string, bool) {
+	if !m.isKeyAllowed(key) {
+		return "", false
+	}
+
+	value := os.Getenv(key)
+	return value, value != ""
+}
+
+// isKeyAllowed checks if a key is in the allow-list
+func (m *Manager) isKeyAllowed(key string) bool {
+	for _, allowedVar := range m.config.AllowedSystemVars {
+		if key == allowedVar {
+			return true
+		}
+
+		// Support wildcard matching
+		if strings.HasSuffix(allowedVar, "*") {
+			prefix := strings.TrimSuffix(allowedVar, "*")
+			if strings.HasPrefix(key, prefix) {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
+// ValidateConfig validates the environment configuration
+func (m *Manager) ValidateConfig() error {
+	// Environment configuration is always valid with our allow-list approach
+	// The worst case is an empty environment, which is still secure
+	return nil
+}
+
+// GetFilteredEnvCount returns the number of filtered environment variables
+func (m *Manager) GetFilteredEnvCount() (filteredCount, totalCount int) {
+	systemEnv := os.Environ()
+	filteredEnv := m.getFilteredSystemEnv()
+	return len(filteredEnv), len(systemEnv)
+}