From 369edf30af81ad0b284d5c2bfcb96b6086fb0d81 Mon Sep 17 00:00:00 2001 From: Keith Patrick Date: Mon, 9 Mar 2026 03:49:50 +0000 Subject: [PATCH 1/2] feat(security): add environment sanitization for exec tool - Add env.go with environment filtering logic - Add EnvSet and EnvAllowlist config options - Block sensitive variables (PATH, HOME, LD_*, PICOCLAW_*) from LLM override - Add PICOCLAW_* env vars to child processes - Document the feature --- docs/tools_configuration.md | 34 +++- pkg/config/config.go | 10 +- pkg/tools/shell.go | 48 ++++++ pkg/tools/shell/env.go | 316 ++++++++++++++++++++++++++++++++++++ 4 files changed, 401 insertions(+), 7 deletions(-) create mode 100644 pkg/tools/shell/env.go diff --git a/docs/tools_configuration.md b/docs/tools_configuration.md index e64a3a107..8e2e9f752 100644 --- a/docs/tools_configuration.md +++ b/docs/tools_configuration.md @@ -52,10 +52,38 @@ The exec tool is used to execute shell commands. | `enable_deny_patterns` | bool | true | Enable default dangerous command blocking | | `custom_deny_patterns` | array | [] | Custom deny patterns (regular expressions) | -### Functionality +### Environment Sanitization -- **`enable_deny_patterns`**: Set to `false` to completely disable the default dangerous command blocking patterns -- **`custom_deny_patterns`**: Add custom deny regex patterns; commands matching these will be blocked +The exec tool sanitizes the environment passed to child processes: + +1. **Default allowlist** — Only these variables are inherited from the parent process: + - `PATH`, `HOME`, `USER`, `LANG`, `SHELL`, `TERM`, `PWD`, `OLDPWD`, `HOSTNAME`, `LOGNAME`, `TZ`, `DISPLAY`, `TMPDIR`, `EDITOR`, `PAGER` + - Plus: `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY` + - Plus locale: `LC_ALL`, `LC_CTYPE`, `LC_MESSAGES`, `LC_MONETARY`, `LC_NUMERIC`, `LC_TIME`, `LC_PAPER`, `LC_NAME`, `LC_ADDRESS`, `LC_TELEPHONE`, `LC_MEASUREMENT`, `LC_IDENTIFICATION`, `LC_COLLATE` + - Plus: `PICOCLAW_HOME`, `PICOCLAW_CONFIG`, `PICOCLAW_AGENT_WORKSPACE`, `PICOCLAW_EXE`, `PICOCLAW_SERVICE_NAME`, `PICOCLAW_EXEC_TIME`, `PICOCLAW_EXEC_TIMEOUT` + +2. **Config env_set** — Variables from config are merged (can override inherited values) + +3. **Config env_allowlist** — Additional explicit variable names to allow (extends default) + +4. **LLM env injection** — The LLM can inject additional variables per-call via the `env` parameter: + + ```json + { + "name": "exec", + "arguments": { + "command": "echo $DEBUG_MODE", + "env": { + "DEBUG_MODE": "true" + } + } + } + ``` + + **Blocked variables** — The LLM cannot override these sensitive variables: + - `PATH`, `HOME`, `USER`, `LOGNAME`, `SHELL` + - `LD_PRELOAD`, `LD_LIBRARY_PATH`, `LD_AUDIT`, `LD_DEBUG` + - All `PICOCLAW_*` variables ### Default Blocked Command Patterns diff --git a/pkg/config/config.go b/pkg/config/config.go index b3ad050b7..a4e1bf51d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -641,10 +641,12 @@ type CronToolsConfig struct { type ExecConfig struct { ToolConfig ` envPrefix:"PICOCLAW_TOOLS_EXEC_"` - EnableDenyPatterns bool ` env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS" json:"enable_deny_patterns"` - CustomDenyPatterns []string ` env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS" json:"custom_deny_patterns"` - CustomAllowPatterns []string ` env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS" json:"custom_allow_patterns"` - TimeoutSeconds int ` env:"PICOCLAW_TOOLS_EXEC_TIMEOUT_SECONDS" json:"timeout_seconds"` // 0 means use default (60s) + EnableDenyPatterns bool ` env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS" json:"enable_deny_patterns"` + CustomDenyPatterns []string ` env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS" json:"custom_deny_patterns"` + CustomAllowPatterns []string ` env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS" json:"custom_allow_patterns"` + TimeoutSeconds int ` env:"PICOCLAW_TOOLS_EXEC_TIMEOUT_SECONDS" json:"timeout_seconds"` // 0 means use default (60s) + EnvSet map[string]string ` json:"env_set"` // env vars to set for all exec commands + EnvAllowlist []string ` json:"env_allowlist"` // additional env vars to allow (extends default) - use explicit names, not wildcards } type SkillsToolsConfig struct { diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index b8a811d03..62cdf1c9a 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -14,6 +14,7 @@ import ( "time" "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/tools/shell" ) type ExecTool struct { @@ -23,6 +24,7 @@ type ExecTool struct { allowPatterns []*regexp.Regexp customAllowPatterns []*regexp.Regexp restrictToWorkspace bool + cachedEnv map[string]string // cached sanitized env from os.Getenv() at init } var ( @@ -136,6 +138,22 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf timeout = time.Duration(config.Tools.Exec.TimeoutSeconds) * time.Second } + // Get envSet and envAllowlist from config (if provided) + var envSet map[string]string + var envAllowlist []string + if config != nil && config.Tools.Exec.EnvSet != nil { + envSet = config.Tools.Exec.EnvSet + } + if config != nil && config.Tools.Exec.EnvAllowlist != nil { + envAllowlist = config.Tools.Exec.EnvAllowlist + } + + // Ensure PICOCLAW_* vars are set for child processes + envSet = shell.WithPicoclawEnvVars(envSet, workingDir) + + // Build cached env: start with envSet (PICOCLAW_*), then add allowed inherited vars + cachedEnv := shell.WithAllowedEnv(envSet, envAllowlist) + return &ExecTool{ workingDir: workingDir, timeout: timeout, @@ -143,6 +161,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf allowPatterns: nil, customAllowPatterns: customAllowPatterns, restrictToWorkspace: restrict, + cachedEnv: cachedEnv, }, nil } @@ -166,6 +185,13 @@ func (t *ExecTool) Parameters() map[string]any { "type": "string", "description": "Optional working directory for the command", }, + "env": map[string]any{ + "type": "object", + "description": "Additional environment variables to set for this command. Available: PICOCLAW_HOME, PICOCLAW_CONFIG, PICOCLAW_AGENT_WORKSPACE, PICOCLAW_EXE, PICOCLAW_SERVICE_NAME, PICOCLAW_EXEC_TIME (RFC3339), PICOCLAW_EXEC_TIMEOUT. Cannot override: PATH, HOME, USER, LOGNAME, SHELL, LD_PRELOAD, LD_LIBRARY_PATH, LD_AUDIT, LD_DEBUG, PICOCLAW_*", + "additionalProperties": map[string]any{ + "type": "string", + }, + }, }, "required": []string{"command"}, } @@ -217,6 +243,28 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult } else { cmd = exec.CommandContext(cmdCtx, "sh", "-c", command) } + + // Parse env param from LLM (if provided) + var extraEnv map[string]string + if envArg, ok := args["env"].(map[string]any); ok && envArg != nil { + extraEnv = make(map[string]string) + for k, v := range envArg { + if strVal, ok := v.(string); ok { + extraEnv[k] = strVal + } + } + } + + // Add PICOCLAW_EXEC_TIME - timestamp when command is executed + execTimeEnv := map[string]string{ + "PICOCLAW_EXEC_TIME": time.Now().Format(time.RFC3339), + "PICOCLAW_EXEC_TIMEOUT": t.timeout.String(), + } + + // Use sanitized environment - merge cached env with exec time vars and LLM extra env + // Note: cachedEnv is NOT re-filtered - PICOCLAW_* vars are preserved + cmd.Env = shell.MergeEnvVars(t.cachedEnv, execTimeEnv, extraEnv) + if cwd != "" { cmd.Dir = cwd } diff --git a/pkg/tools/shell/env.go b/pkg/tools/shell/env.go new file mode 100644 index 000000000..41c4f4dc3 --- /dev/null +++ b/pkg/tools/shell/env.go @@ -0,0 +1,316 @@ +package shell + +import ( + "os" + "path/filepath" + "runtime" + "strings" +) + +// DefaultEnvAllowlist is the set of environment variable names that are safe +// to propagate to child processes. Everything else is stripped to prevent +// accidental credential leakage. +// +// To add a new variable: +// 1. Add to this map if it's safe to pass through +// 2. Or add a prefix to defaultEnvAllowPrefixes for pattern matching +// Note: Do NOT add wildcard patterns like "*_API_KEY" here - use explicit names +// to avoid accidentally leaking secrets. +var DefaultEnvAllowlist = map[string]bool{ + "PATH": true, + "HOME": true, + "USER": true, + "LANG": true, + "SHELL": true, + "TERM": true, + "PWD": true, + "OLDPWD": true, + "HOSTNAME": true, + "LOGNAME": true, + "TZ": true, + "DISPLAY": true, + "TMPDIR": true, + "EDITOR": true, + "PAGER": true, + "HTTP_PROXY": true, + "HTTPS_PROXY": true, + "NO_PROXY": true, + + // Locale + "LC_ALL": true, + "LC_CTYPE": true, + "LC_MESSAGES": true, + "LC_MONETARY": true, + "LC_NUMERIC": true, + "LC_TIME": true, + "LC_PAPER": true, + "LC_NAME": true, + "LC_ADDRESS": true, + "LC_TELEPHONE": true, + "LC_MEASUREMENT": true, + "LC_IDENTIFICATION": true, + "LC_COLLATE": true, +} + +// defaultEnvAllowPrefixes are env var prefixes that are always allowed. +// Currently empty - all allowed vars are explicit in DefaultEnvAllowlist. +var defaultEnvAllowPrefixes = []string{ + // Currently empty - all allowed vars are explicit +} + +// LLMBlocklist is the set of environment variable names that the LLM +// cannot override, even if passed via the env parameter. These vars +// control fundamental process behavior and could be exploited. +var LLMBlocklist = map[string]bool{ + "PATH": true, // Could hijack command resolution + "HOME": true, // Could redirect file access + "USER": true, // Could impersonate user + "LOGNAME": true, // Could impersonate user + "SHELL": true, // Could change shell behavior + "LD_PRELOAD": true, // Could inject code + "LD_LIBRARY_PATH": true, // Could hijack library resolution + "LD_AUDIT": true, // Could inject code + "LD_DEBUG": true, // Could leak info + + // PICOCLAW_* vars - controlled by the agent, not LLM + "PICOCLAW_HOME": true, + "PICOCLAW_CONFIG": true, + "PICOCLAW_AGENT_WORKSPACE": true, + "PICOCLAW_EXE": true, + "PICOCLAW_SERVICE_NAME": true, + "PICOCLAW_EXEC_TIME": true, + "PICOCLAW_EXEC_TIMEOUT": true, +} + +// windowsEnvAllowlist contains additional variables needed on Windows. +var windowsEnvAllowlist = map[string]bool{ + "PATHEXT": true, + "SYSTEMROOT": true, + "SYSTEMDRIVE": true, + "COMSPEC": true, + "APPDATA": true, + "USERPROFILE": true, + "HOMEDRIVE": true, + "HOMEPATH": true, +} + +// WithAllowedEnv builds a map of allowed environment variables by looking them up. +// This is more efficient than filtering os.Environ() with string parsing. +// It starts with the provided env map, then adds allowed inherited vars (if not set). +// extraAllowlist adds to the default allowlist. +func WithAllowedEnv(envSet map[string]string, extraAllowlist []string) map[string]string { + // Start with provided envSet map + result := envSet + if result == nil { + result = make(map[string]string) + } + + // Add default allowlist (only if not already set) + for k := range DefaultEnvAllowlist { + if _, exists := result[k]; !exists { + if val := os.Getenv(k); val != "" { + result[k] = val + } + } + } + // Add Windows-specific vars + if runtime.GOOS == "windows" { + for k := range windowsEnvAllowlist { + if _, exists := result[k]; !exists { + if val := os.Getenv(k); val != "" { + result[k] = val + } + } + } + } + // Add extra allowlist from config + for _, k := range extraAllowlist { + if _, exists := result[k]; !exists { + if val := os.Getenv(k); val != "" { + result[k] = val + } + } + } + + return result +} + +// LLMBlocklistPrefixes are env var prefixes that the LLM cannot override. +var LLMBlocklistPrefixes = []string{ + "PICOCLAW_", +} + +// isBlocked returns true if the key is in the blocklist or matches a blocked prefix. +func isBlocked(key string) bool { + norm := envKey(key) + if LLMBlocklist[norm] { + return true + } + for _, prefix := range LLMBlocklistPrefixes { + if strings.HasPrefix(norm, prefix) { + return true + } + } + return false +} + +// MergeEnvVars merges multiple env sources into a final []string for exec.Cmd.Env. +// baseEnv is the cached map from AllowedEnv. +// envSet provides explicit key=value pairs (config, not filtered). +// extraEnv provides additional key=value pairs from LLM (filtered by blocklist). +func MergeEnvVars(baseEnv map[string]string, envSet, extraEnv map[string]string) []string { + vars := make(map[string]string, len(baseEnv)+len(envSet)+len(extraEnv)) + + // Start with base env (already filtered) + for k, v := range baseEnv { + vars[envKey(k)] = v + } + + // Add envSet (config-provided, not filtered) + if envSet != nil { + for k, v := range envSet { + vars[envKey(k)] = v + } + } + + // Merge extraEnv (LLM-provided) - filtered by blocklist + if extraEnv != nil { + for k, v := range extraEnv { + if isBlocked(k) { + continue // Skip blocked vars + } + vars[envKey(k)] = v + } + } + + // Convert to []string for exec.Cmd.Env + result := make([]string, 0, len(vars)) + for k, v := range vars { + result = append(result, k+"="+v) + } + return result +} + +// BuildSanitizedEnv constructs a sanitized environment []string suitable for +// exec.Cmd.Env. It filters the inherited environment to only allowlisted variables. +// +// baseEnv is the inherited environment (e.g., from os.Environ() or cached). +// If nil, os.Environ() will be used for backwards compatibility. +// extraAllowlist adds additional variable names to the default allowlist. +// envSet provides explicit key=value pairs from config (override inherited). +// extraEnv provides additional key=value pairs from tool call (merged with envSet). +func BuildSanitizedEnv(baseEnv []string, extraAllowlist []string, envSet, extraEnv map[string]string) []string { + + // Use provided env or fall back to os.Environ + inherited := baseEnv + if inherited == nil { + inherited = os.Environ() + } + + allowed := make(map[string]bool, len(DefaultEnvAllowlist)+len(extraAllowlist)+len(windowsEnvAllowlist)) + for k := range DefaultEnvAllowlist { + allowed[envKey(k)] = true + } + if runtime.GOOS == "windows" { + for k := range windowsEnvAllowlist { + allowed[envKey(k)] = true + } + } + for _, k := range extraAllowlist { + allowed[envKey(k)] = true + } + + vars := make(map[string]string, len(allowed)+len(envSet)+len(extraEnv)) + + for _, entry := range inherited { + k, v, ok := strings.Cut(entry, "=") + if !ok { + continue + } + norm := envKey(k) + if allowed[norm] || isAllowedPrefix(norm) { + vars[norm] = v + } + } + + if envSet != nil { + for k, v := range envSet { + vars[envKey(k)] = v + } + } + + // Merge extraEnv (tool call) - highest priority + // Filter against LLM blocklist to prevent override of sensitive vars + if extraEnv != nil { + for k, v := range extraEnv { + if LLMBlocklist[envKey(k)] { + continue // Skip blocked vars + } + vars[envKey(k)] = v + } + } + + // Convert to []string for exec.Cmd.Env + result := make([]string, 0, len(vars)) + for k, v := range vars { + result = append(result, k+"="+v) + } + return result +} + +// envKey normalizes an environment variable name. On Windows, where env +// vars are case-insensitive, it uppercases the key so that "Path" and +// "PATH" map to the same entry. On other platforms it's a no-op. +func envKey(k string) string { + if runtime.GOOS == "windows" { + return strings.ToUpper(k) + } + return k +} + +func isAllowedPrefix(name string) bool { + for _, prefix := range defaultEnvAllowPrefixes { + if strings.HasPrefix(name, prefix) { + return true + } + } + return false +} + +// WithPicoclawEnvVars ensures PICOCLAW_* vars are set in envSet. +// These are needed for child processes to locate config, workspace, etc. +func WithPicoclawEnvVars(envSet map[string]string, workspace string) map[string]string { + if envSet == nil { + envSet = make(map[string]string) + } + + // Always compute PICOCLAW_* vars - priority: env var > default + if v := os.Getenv("PICOCLAW_HOME"); v != "" { + envSet["PICOCLAW_HOME"] = v + } else if home, _ := os.UserHomeDir(); home != "" { + envSet["PICOCLAW_HOME"] = filepath.Join(home, ".picoclaw") + } + + if v := os.Getenv("PICOCLAW_CONFIG"); v != "" { + envSet["PICOCLAW_CONFIG"] = v + } else if home := envSet["PICOCLAW_HOME"]; home != "" { + envSet["PICOCLAW_CONFIG"] = filepath.Join(home, "config.json") + } + + // Workspace - this is the agent's working directory + if workspace != "" { + envSet["PICOCLAW_AGENT_WORKSPACE"] = workspace + } + + if exe, err := os.Executable(); err == nil { + envSet["PICOCLAW_EXE"] = exe + } + + if v := os.Getenv("PICOCLAW_SERVICE_NAME"); v != "" { + envSet["PICOCLAW_SERVICE_NAME"] = v + } else { + envSet["PICOCLAW_SERVICE_NAME"] = "picoclaw" + } + + return envSet +} From 4b11ef32fe1c501baafc615da77c63b952339684 Mon Sep 17 00:00:00 2001 From: Keith Patrick Date: Mon, 9 Mar 2026 04:42:42 +0000 Subject: [PATCH 2/2] Refactor env handling: extract MapToEnvSlice, clean up unused code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused BuildSanitizedEnv function - Extract MapToEnvSlice helper from MergeEnvVars - MergeEnvVars now returns map, caller converts to []string - Fix lint issues (gci, gofumpt, golines, whitespace) 💘 Generated with Crush --- pkg/config/config.go | 6 +- pkg/tools/shell.go | 2 +- pkg/tools/shell/env.go | 169 +++++++++++------------------------------ 3 files changed, 50 insertions(+), 127 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index a4e1bf51d..b683eadbe 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -640,13 +640,13 @@ type CronToolsConfig struct { } type ExecConfig struct { - ToolConfig ` envPrefix:"PICOCLAW_TOOLS_EXEC_"` + ToolConfig ` envPrefix:"PICOCLAW_TOOLS_EXEC_"` EnableDenyPatterns bool ` env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS" json:"enable_deny_patterns"` CustomDenyPatterns []string ` env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS" json:"custom_deny_patterns"` CustomAllowPatterns []string ` env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS" json:"custom_allow_patterns"` TimeoutSeconds int ` env:"PICOCLAW_TOOLS_EXEC_TIMEOUT_SECONDS" json:"timeout_seconds"` // 0 means use default (60s) - EnvSet map[string]string ` json:"env_set"` // env vars to set for all exec commands - EnvAllowlist []string ` json:"env_allowlist"` // additional env vars to allow (extends default) - use explicit names, not wildcards + EnvSet map[string]string ` json:"env_set"` // env vars to set for all exec commands + EnvAllowlist []string ` json:"env_allowlist"` // additional env vars to allow (extends default) - use explicit names, not wildcards } type SkillsToolsConfig struct { diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 62cdf1c9a..134d1df37 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -263,7 +263,7 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult // Use sanitized environment - merge cached env with exec time vars and LLM extra env // Note: cachedEnv is NOT re-filtered - PICOCLAW_* vars are preserved - cmd.Env = shell.MergeEnvVars(t.cachedEnv, execTimeEnv, extraEnv) + cmd.Env = shell.MapToEnvSlice(shell.MergeEnvVars(t.cachedEnv, execTimeEnv, extraEnv)) if cwd != "" { cmd.Dir = cwd diff --git a/pkg/tools/shell/env.go b/pkg/tools/shell/env.go index 41c4f4dc3..49739f418 100644 --- a/pkg/tools/shell/env.go +++ b/pkg/tools/shell/env.go @@ -17,45 +17,39 @@ import ( // Note: Do NOT add wildcard patterns like "*_API_KEY" here - use explicit names // to avoid accidentally leaking secrets. var DefaultEnvAllowlist = map[string]bool{ - "PATH": true, - "HOME": true, - "USER": true, - "LANG": true, - "SHELL": true, - "TERM": true, - "PWD": true, - "OLDPWD": true, - "HOSTNAME": true, - "LOGNAME": true, - "TZ": true, - "DISPLAY": true, - "TMPDIR": true, - "EDITOR": true, - "PAGER": true, - "HTTP_PROXY": true, - "HTTPS_PROXY": true, - "NO_PROXY": true, + "PATH": true, + "HOME": true, + "USER": true, + "LANG": true, + "SHELL": true, + "TERM": true, + "PWD": true, + "OLDPWD": true, + "HOSTNAME": true, + "LOGNAME": true, + "TZ": true, + "DISPLAY": true, + "TMPDIR": true, + "EDITOR": true, + "PAGER": true, + "HTTP_PROXY": true, + "HTTPS_PROXY": true, + "NO_PROXY": true, // Locale - "LC_ALL": true, - "LC_CTYPE": true, - "LC_MESSAGES": true, - "LC_MONETARY": true, - "LC_NUMERIC": true, - "LC_TIME": true, - "LC_PAPER": true, - "LC_NAME": true, - "LC_ADDRESS": true, - "LC_TELEPHONE": true, - "LC_MEASUREMENT": true, + "LC_ALL": true, + "LC_CTYPE": true, + "LC_MESSAGES": true, + "LC_MONETARY": true, + "LC_NUMERIC": true, + "LC_TIME": true, + "LC_PAPER": true, + "LC_NAME": true, + "LC_ADDRESS": true, + "LC_TELEPHONE": true, + "LC_MEASUREMENT": true, "LC_IDENTIFICATION": true, - "LC_COLLATE": true, -} - -// defaultEnvAllowPrefixes are env var prefixes that are always allowed. -// Currently empty - all allowed vars are explicit in DefaultEnvAllowlist. -var defaultEnvAllowPrefixes = []string{ - // Currently empty - all allowed vars are explicit + "LC_COLLATE": true, } // LLMBlocklist is the set of environment variable names that the LLM @@ -65,21 +59,21 @@ var LLMBlocklist = map[string]bool{ "PATH": true, // Could hijack command resolution "HOME": true, // Could redirect file access "USER": true, // Could impersonate user - "LOGNAME": true, // Could impersonate user + "LOGNAME": true, // Could impersonate user "SHELL": true, // Could change shell behavior "LD_PRELOAD": true, // Could inject code "LD_LIBRARY_PATH": true, // Could hijack library resolution - "LD_AUDIT": true, // Could inject code - "LD_DEBUG": true, // Could leak info + "LD_AUDIT": true, // Could inject code + "LD_DEBUG": true, // Could leak info // PICOCLAW_* vars - controlled by the agent, not LLM - "PICOCLAW_HOME": true, - "PICOCLAW_CONFIG": true, - "PICOCLAW_AGENT_WORKSPACE": true, - "PICOCLAW_EXE": true, - "PICOCLAW_SERVICE_NAME": true, - "PICOCLAW_EXEC_TIME": true, - "PICOCLAW_EXEC_TIMEOUT": true, + "PICOCLAW_HOME": true, + "PICOCLAW_CONFIG": true, + "PICOCLAW_AGENT_WORKSPACE": true, + "PICOCLAW_EXE": true, + "PICOCLAW_SERVICE_NAME": true, + "PICOCLAW_EXEC_TIME": true, + "PICOCLAW_EXEC_TIMEOUT": true, } // windowsEnvAllowlist contains additional variables needed on Windows. @@ -154,11 +148,11 @@ func isBlocked(key string) bool { return false } -// MergeEnvVars merges multiple env sources into a final []string for exec.Cmd.Env. +// MergeEnvVars merges multiple env sources into a map. // baseEnv is the cached map from AllowedEnv. // envSet provides explicit key=value pairs (config, not filtered). // extraEnv provides additional key=value pairs from LLM (filtered by blocklist). -func MergeEnvVars(baseEnv map[string]string, envSet, extraEnv map[string]string) []string { +func MergeEnvVars(baseEnv map[string]string, envSet, extraEnv map[string]string) map[string]string { vars := make(map[string]string, len(baseEnv)+len(envSet)+len(extraEnv)) // Start with base env (already filtered) @@ -183,74 +177,12 @@ func MergeEnvVars(baseEnv map[string]string, envSet, extraEnv map[string]string) } } - // Convert to []string for exec.Cmd.Env - result := make([]string, 0, len(vars)) - for k, v := range vars { - result = append(result, k+"="+v) - } - return result + return vars } -// BuildSanitizedEnv constructs a sanitized environment []string suitable for -// exec.Cmd.Env. It filters the inherited environment to only allowlisted variables. -// -// baseEnv is the inherited environment (e.g., from os.Environ() or cached). -// If nil, os.Environ() will be used for backwards compatibility. -// extraAllowlist adds additional variable names to the default allowlist. -// envSet provides explicit key=value pairs from config (override inherited). -// extraEnv provides additional key=value pairs from tool call (merged with envSet). -func BuildSanitizedEnv(baseEnv []string, extraAllowlist []string, envSet, extraEnv map[string]string) []string { - - // Use provided env or fall back to os.Environ - inherited := baseEnv - if inherited == nil { - inherited = os.Environ() - } - - allowed := make(map[string]bool, len(DefaultEnvAllowlist)+len(extraAllowlist)+len(windowsEnvAllowlist)) - for k := range DefaultEnvAllowlist { - allowed[envKey(k)] = true - } - if runtime.GOOS == "windows" { - for k := range windowsEnvAllowlist { - allowed[envKey(k)] = true - } - } - for _, k := range extraAllowlist { - allowed[envKey(k)] = true - } - - vars := make(map[string]string, len(allowed)+len(envSet)+len(extraEnv)) - - for _, entry := range inherited { - k, v, ok := strings.Cut(entry, "=") - if !ok { - continue - } - norm := envKey(k) - if allowed[norm] || isAllowedPrefix(norm) { - vars[norm] = v - } - } - - if envSet != nil { - for k, v := range envSet { - vars[envKey(k)] = v - } - } - - // Merge extraEnv (tool call) - highest priority - // Filter against LLM blocklist to prevent override of sensitive vars - if extraEnv != nil { - for k, v := range extraEnv { - if LLMBlocklist[envKey(k)] { - continue // Skip blocked vars - } - vars[envKey(k)] = v - } - } - - // Convert to []string for exec.Cmd.Env +// MapToEnvSlice converts a map of environment variables to a []string +// in the format "KEY=value" suitable for exec.Cmd.Env. +func MapToEnvSlice(vars map[string]string) []string { result := make([]string, 0, len(vars)) for k, v := range vars { result = append(result, k+"="+v) @@ -268,15 +200,6 @@ func envKey(k string) string { return k } -func isAllowedPrefix(name string) bool { - for _, prefix := range defaultEnvAllowPrefixes { - if strings.HasPrefix(name, prefix) { - return true - } - } - return false -} - // WithPicoclawEnvVars ensures PICOCLAW_* vars are set in envSet. // These are needed for child processes to locate config, workspace, etc. func WithPicoclawEnvVars(envSet map[string]string, workspace string) map[string]string {