diff --git a/pkg/config/config.go b/pkg/config/config.go index 22ccd5aa2..b0e65dbc4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -759,6 +759,16 @@ type ExecConfig struct { TimeoutSeconds int ` env:"PICOCLAW_TOOLS_EXEC_TIMEOUT_SECONDS" json:"timeout_seconds"` // 0 means use default (60s) } +type ExeclineConfig struct { + ToolConfig ` json:","` + DenyDefaultsEnable bool `json:"deny_defaults_enable"` + Deny []string `json:"deny"` + Allow []string `json:"allow"` + TimeoutSeconds int `json:"timeout_seconds"` + EnvSet map[string]string `json:"env_set"` + EnvAllowlist []string `json:"env_allowlist"` +} + type SkillsToolsConfig struct { ToolConfig ` envPrefix:"PICOCLAW_TOOLS_SKILLS_"` Registries SkillsRegistriesConfig ` json:"registries"` @@ -784,7 +794,7 @@ type ToolsConfig struct { Web WebToolsConfig `json:"web"` Cron CronToolsConfig `json:"cron"` Exec ExecConfig `json:"exec"` - Execline ToolConfig `json:"execline" envPrefix:"PICOCLAW_TOOLS_EXELINE_"` + Execline ExeclineConfig `json:"execline"` Skills SkillsToolsConfig `json:"skills"` MediaCleanup MediaCleanupConfig `json:"media_cleanup"` MCP MCPConfig `json:"mcp"` diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index d44c73577..0045bc19e 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -472,6 +472,13 @@ func DefaultConfig() *Config { AllowRemote: true, TimeoutSeconds: 60, }, + Execline: ExeclineConfig{ + ToolConfig: ToolConfig{ + Enabled: true, + }, + DenyDefaultsEnable: true, + TimeoutSeconds: 60, + }, Skills: SkillsToolsConfig{ ToolConfig: ToolConfig{ Enabled: true, diff --git a/pkg/tools/execline.go b/pkg/tools/execline.go index 1b5057565..5d0f08d8b 100644 --- a/pkg/tools/execline.go +++ b/pkg/tools/execline.go @@ -4,23 +4,74 @@ import ( "context" "fmt" "os/exec" - "strings" + "regexp" + "time" "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/tools/shell" ) +// Default deny patterns for execline (Linux only - no variable expansion blocks needed) +var defaultExeclineDenyPatterns = []*regexp.Regexp{ + regexp.MustCompile(`\brm\s+-[rf]{1,2}\b`), + regexp.MustCompile(`\b(format|mkfs|diskpart)\b\s`), + regexp.MustCompile(`\bdd\s+if=`), + // Block device writes + regexp.MustCompile( + `>\s*/dev/(sd[a-z]|hd[a-z]|vd[a-z]|xvd[a-z]|nvme\d|mmcblk\d|loop\d|dm-\d|md\d|sr\d|nbd\d)`, + ), + regexp.MustCompile(`\b(shutdown|reboot|poweroff)\b`), + regexp.MustCompile(`\bsudo\b`), + regexp.MustCompile(`\bdocker\s+run\b`), + regexp.MustCompile(`\bdocker\s+exec\b`), + regexp.MustCompile(`\bgit\s+push\b`), + regexp.MustCompile(`\bgit\s+force\b`), +} + // ExeclineTool executes commands using execlineb instead of shell // Security: execlineb does not support variable expansion ($VAR) or command // substitution $(cmd) by default, reducing attack surface significantly. type ExeclineTool struct { - config *config.Config + config *config.Config + denyPatterns []*regexp.Regexp + allowPatterns []*regexp.Regexp + timeout time.Duration } // NewExeclineTool creates a new ExeclineTool instance func NewExeclineTool(cfg *config.Config) *ExeclineTool { + // Start with default deny patterns only if enabled + var denyPatterns []*regexp.Regexp + if cfg.Tools.Execline.DenyDefaultsEnable { + denyPatterns = append([]*regexp.Regexp{}, defaultExeclineDenyPatterns...) + } + + // Add custom deny patterns from config + for _, p := range cfg.Tools.Execline.Deny { + if r, err := regexp.Compile(p); err == nil { + denyPatterns = append(denyPatterns, r) + } + } + + // Compile allow patterns + var allowPatterns []*regexp.Regexp + for _, p := range cfg.Tools.Execline.Allow { + if r, err := regexp.Compile(p); err == nil { + allowPatterns = append(allowPatterns, r) + } + } + + // Default timeout 60s + timeout := 60 * time.Second + if cfg.Tools.Execline.TimeoutSeconds > 0 { + timeout = time.Duration(cfg.Tools.Execline.TimeoutSeconds) * time.Second + } + return &ExeclineTool{ - config: cfg, + config: cfg, + denyPatterns: denyPatterns, + allowPatterns: allowPatterns, + timeout: timeout, } } @@ -43,6 +94,17 @@ func (t *ExeclineTool) Parameters() map[string]any { "type": "string", "description": "The command to execute (passed as-is, no shell expansion)", }, + "working_dir": 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"}, } @@ -92,16 +154,42 @@ func (t *ExeclineTool) Execute(ctx context.Context, args map[string]any) *ToolRe baseEnv := shell.WithAllowedEnv(nil, nil) var extraEnv map[string]string if t.config != nil { - extraEnv = t.config.Tools.Exec.EnvSet + extraEnv = t.config.Tools.Execline.EnvSet } - execEnv := shell.MergeEnvVars(baseEnv, nil, extraEnv) + + // Parse env param from LLM (if provided) + if envArg, ok := args["env"].(map[string]any); ok && envArg != nil { + if extraEnv == 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(), + } + execEnv = shell.MergeEnvVars(execEnv, execTimeEnv, nil) // Use execlineb to execute // execlineb -c takes a command string and executes it // Unlike sh -c, it doesn't expand $VAR or $(cmd) + // Add timeout to context + ctx, cancel := context.WithTimeout(ctx, t.timeout) + defer cancel() cmd := exec.CommandContext(ctx, "/usr/bin/execlineb", "-c", command) cmd.Env = shell.MapToEnvSlice(execEnv) + // Set working directory if provided + if wd, ok := args["working_dir"].(string); ok && wd != "" { + cmd.Dir = wd + } + output, err := cmd.CombinedOutput() if err != nil { return &ToolResult{ @@ -122,14 +210,26 @@ func (t *ExeclineTool) Execute(ctx context.Context, args map[string]any) *ToolRe // validateCommand checks for dangerous command patterns // Since execline is secure by default, we only block obvious exploits func (t *ExeclineTool) validateCommand(cmd string) error { - // Block obvious shell escape attempts - if strings.Contains(cmd, "&&") || strings.Contains(cmd, "||") { - return fmt.Errorf("control operators (&&, ||) not supported in execline") - } - if strings.Contains(cmd, "|") && strings.Contains(cmd, "sh") { - return fmt.Errorf("pipe to shell detected") - } // Note: $VAR and $(cmd) are simply not expanded by execlineb // They are passed literally to the command, so this is safe + + // Check custom allow patterns first (can override deny) + explicitlyAllowed := false + for _, pattern := range t.allowPatterns { + if pattern.MatchString(cmd) { + explicitlyAllowed = true + break + } + } + + if !explicitlyAllowed { + // Check custom deny patterns + for _, pattern := range t.denyPatterns { + if pattern.MatchString(cmd) { + return fmt.Errorf("command matches blocked pattern") + } + } + } + return nil } diff --git a/pkg/tools/shell/env.go b/pkg/tools/shell/env.go new file mode 100644 index 000000000..ab4a53976 --- /dev/null +++ b/pkg/tools/shell/env.go @@ -0,0 +1,249 @@ +package shell + +import ( + "maps" + "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, + "http_proxy": true, + "HTTPS_PROXY": true, + "https_proxy": true, + "NO_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, + + // systemd/user session (for systemctl --user and journalctl --user) + "XDG_RUNTIME_DIR": true, + "DBUS_SESSION_BUS_ADDRESS": true, +} + +// 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 { + // Copy the map to avoid mutating the caller's map + result := maps.Clone(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 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) map[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 + } + } + + return vars +} + +// 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) + } + 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 +} + +// 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 { + // Copy the map to avoid mutating the caller's map + result := maps.Clone(envSet) + if result == nil { + result = make(map[string]string) + } + + // Always compute PICOCLAW_* vars - priority: env var > default + if v := os.Getenv("PICOCLAW_HOME"); v != "" { + result["PICOCLAW_HOME"] = v + } else if home, _ := os.UserHomeDir(); home != "" { + result["PICOCLAW_HOME"] = filepath.Join(home, ".picoclaw") + } + + if v := os.Getenv("PICOCLAW_CONFIG"); v != "" { + result["PICOCLAW_CONFIG"] = v + } else if home := result["PICOCLAW_HOME"]; home != "" { + result["PICOCLAW_CONFIG"] = filepath.Join(home, "config.json") + } + + // Workspace - this is the agent's working directory + if workspace != "" { + result["PICOCLAW_AGENT_WORKSPACE"] = workspace + } + + if exe, err := os.Executable(); err == nil { + result["PICOCLAW_EXE"] = exe + } + + if v := os.Getenv("PICOCLAW_SERVICE_NAME"); v != "" { + result["PICOCLAW_SERVICE_NAME"] = v + } else { + result["PICOCLAW_SERVICE_NAME"] = "picoclaw" + } + + return result +}