From eda6e373323861fea99630013946d7aeea94ade0 Mon Sep 17 00:00:00 2001 From: lxowalle <83055338+lxowalle@users.noreply.github.com> Date: Wed, 18 Feb 2026 19:31:15 +0800 Subject: [PATCH] feat: Support modifying the command filtering list of the exec tool (#410) --- cmd/picoclaw/main.go | 6 +- docs/tools_configuration.md | 122 ++++++++++++++++++++++++++++++++++++ pkg/agent/loop.go | 2 +- pkg/config/config.go | 9 +++ pkg/tools/cron.go | 7 ++- pkg/tools/shell.go | 119 ++++++++++++++++++++++------------- 6 files changed, 215 insertions(+), 50 deletions(-) create mode 100644 docs/tools_configuration.md diff --git a/cmd/picoclaw/main.go b/cmd/picoclaw/main.go index fd7ec484a..128f8c421 100644 --- a/cmd/picoclaw/main.go +++ b/cmd/picoclaw/main.go @@ -563,7 +563,7 @@ func gatewayCmd() { // Setup cron tool and service execTimeout := time.Duration(cfg.Tools.Cron.ExecTimeoutMinutes) * time.Minute - cronService := setupCronTool(agentLoop, msgBus, cfg.WorkspacePath(), cfg.Agents.Defaults.RestrictToWorkspace, execTimeout) + cronService := setupCronTool(agentLoop, msgBus, cfg.WorkspacePath(), cfg.Agents.Defaults.RestrictToWorkspace, execTimeout, cfg) heartbeatService := heartbeat.NewHeartbeatService( cfg.WorkspacePath(), @@ -988,14 +988,14 @@ func getConfigPath() string { return filepath.Join(home, ".picoclaw", "config.json") } -func setupCronTool(agentLoop *agent.AgentLoop, msgBus *bus.MessageBus, workspace string, restrict bool, execTimeout time.Duration) *cron.CronService { +func setupCronTool(agentLoop *agent.AgentLoop, msgBus *bus.MessageBus, workspace string, restrict bool, execTimeout time.Duration, config *config.Config) *cron.CronService { cronStorePath := filepath.Join(workspace, "cron", "jobs.json") // Create cron service cronService := cron.NewCronService(cronStorePath, nil) // Create and register CronTool - cronTool := tools.NewCronTool(cronService, agentLoop, msgBus, workspace, restrict, execTimeout) + cronTool := tools.NewCronTool(cronService, agentLoop, msgBus, workspace, restrict, execTimeout, config) agentLoop.RegisterTool(cronTool) // Set the onJob handler diff --git a/docs/tools_configuration.md b/docs/tools_configuration.md new file mode 100644 index 000000000..8777ddbd6 --- /dev/null +++ b/docs/tools_configuration.md @@ -0,0 +1,122 @@ +# Tools Configuration + +PicoClaw's tools configuration is located in the `tools` field of `config.json`. + +## Directory Structure + +```json +{ + "tools": { + "web": { ... }, + "exec": { ... }, + "approval": { ... }, + "cron": { ... } + } +} +``` + +## Web Tools + +Web tools are used for web search and fetching. + +### Brave + +| Config | Type | Default | Description | +|--------|------|---------|-------------| +| `enabled` | bool | false | Enable Brave search | +| `api_key` | string | - | Brave Search API key | +| `max_results` | int | 5 | Maximum number of results | + +### DuckDuckGo + +| Config | Type | Default | Description | +|--------|------|---------|-------------| +| `enabled` | bool | true | Enable DuckDuckGo search | +| `max_results` | int | 5 | Maximum number of results | + +### Perplexity + +| Config | Type | Default | Description | +|--------|------|---------|-------------| +| `enabled` | bool | false | Enable Perplexity search | +| `api_key` | string | - | Perplexity API key | +| `max_results` | int | 5 | Maximum number of results | + +## Exec Tool + +The exec tool is used to execute shell commands. + +| Config | Type | Default | Description | +|--------|------|---------|-------------| +| `enable_deny_patterns` | bool | true | Enable default dangerous command blocking | +| `custom_deny_patterns` | array | [] | Custom deny patterns (regular expressions) | + +### Functionality + +- **`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 + +### Default Blocked Command Patterns + +By default, PicoClaw blocks the following dangerous commands: + +- Delete commands: `rm -rf`, `del /f/q`, `rmdir /s` +- Disk operations: `format`, `mkfs`, `diskpart`, `dd if=`, writing to `/dev/sd*` +- System operations: `shutdown`, `reboot`, `poweroff` +- Command substitution: `$()`, `${}`, backticks +- Pipe to shell: `| sh`, `| bash` +- Privilege escalation: `sudo`, `chmod`, `chown` +- Process control: `pkill`, `killall`, `kill -9` +- Remote operations: `curl | sh`, `wget | sh`, `ssh` +- Package management: `apt`, `yum`, `dnf`, `npm install -g`, `pip install --user` +- Containers: `docker run`, `docker exec` +- Git: `git push`, `git force` +- Other: `eval`, `source *.sh` + +### Configuration Example + +```json +{ + "tools": { + "exec": { + "enable_deny_patterns": true, + "custom_deny_patterns": [ + "\\brm\\s+-r\\b", + "\\bkillall\\s+python" + ], + } + } +} +``` + +## Approval Tool + +The approval tool controls permissions for dangerous operations. + +| Config | Type | Default | Description | +|--------|------|---------|-------------| +| `enabled` | bool | true | Enable approval functionality | +| `write_file` | bool | true | Require approval for file writes | +| `edit_file` | bool | true | Require approval for file edits | +| `append_file` | bool | true | Require approval for file appends | +| `exec` | bool | true | Require approval for command execution | +| `timeout_minutes` | int | 5 | Approval timeout in minutes | + +## Cron Tool + +The cron tool is used for scheduling periodic tasks. + +| Config | Type | Default | Description | +|--------|------|---------|-------------| +| `exec_timeout_minutes` | int | 5 | Execution timeout in minutes, 0 means no limit | + +## Environment Variables + +All configuration options can be overridden via environment variables with the format `PICOCLAW_TOOLS_
_`: + +For example: +- `PICOCLAW_TOOLS_WEB_BRAVE_ENABLED=true` +- `PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS=false` +- `PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES=10` + +Note: Array-type environment variables are not currently supported and must be set via the config file. diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index d3afa298e..8c6c58c96 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -71,7 +71,7 @@ func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msg registry.Register(tools.NewAppendFileTool(workspace, restrict)) // Shell execution - registry.Register(tools.NewExecTool(workspace, restrict)) + registry.Register(tools.NewExecToolWithConfig(workspace, restrict, cfg)) if searchTool := tools.NewWebSearchTool(tools.WebSearchToolOptions{ BraveAPIKey: cfg.Tools.Web.Brave.APIKey, diff --git a/pkg/config/config.go b/pkg/config/config.go index 92a4a5862..a1cc978b6 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -227,9 +227,15 @@ type CronToolsConfig struct { ExecTimeoutMinutes int `json:"exec_timeout_minutes" env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES"` // 0 means no timeout } +type ExecConfig struct { + EnableDenyPatterns bool `json:"enable_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"` + CustomDenyPatterns []string `json:"custom_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"` +} + type ToolsConfig struct { Web WebToolsConfig `json:"web"` Cron CronToolsConfig `json:"cron"` + Exec ExecConfig `json:"exec"` } func DefaultConfig() *Config { @@ -347,6 +353,9 @@ func DefaultConfig() *Config { Cron: CronToolsConfig{ ExecTimeoutMinutes: 5, // default 5 minutes for LLM operations }, + Exec: ExecConfig{ + EnableDenyPatterns: true, + }, }, Heartbeat: HeartbeatConfig{ Enabled: true, diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go index 21bee42ef..e2764d8ac 100644 --- a/pkg/tools/cron.go +++ b/pkg/tools/cron.go @@ -7,6 +7,7 @@ import ( "time" "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/cron" "github.com/sipeed/picoclaw/pkg/utils" ) @@ -29,9 +30,9 @@ type CronTool struct { // NewCronTool creates a new CronTool // execTimeout: 0 means no timeout, >0 sets the timeout duration -func NewCronTool(cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string, restrict bool, execTimeout time.Duration) *CronTool { - execTool := NewExecTool(workspace, restrict) - execTool.SetTimeout(execTimeout) // 0 means no timeout +func NewCronTool(cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string, restrict bool, execTimeout time.Duration, config *config.Config) *CronTool { + execTool := NewExecToolWithConfig(workspace, restrict, config) + execTool.SetTimeout(execTimeout) return &CronTool{ cronService: cronService, executor: executor, diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 9c82b2748..bd612d9ae 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "github.com/sipeed/picoclaw/pkg/config" "os" "os/exec" "path/filepath" @@ -21,50 +22,82 @@ type ExecTool struct { restrictToWorkspace bool } +var defaultDenyPatterns = []*regexp.Regexp{ + regexp.MustCompile(`\brm\s+-[rf]{1,2}\b`), + regexp.MustCompile(`\bdel\s+/[fq]\b`), + regexp.MustCompile(`\brmdir\s+/s\b`), + regexp.MustCompile(`\b(format|mkfs|diskpart)\b\s`), // Match disk wiping commands (must be followed by space/args) + regexp.MustCompile(`\bdd\s+if=`), + regexp.MustCompile(`>\s*/dev/sd[a-z]\b`), // Block writes to disk devices (but allow /dev/null) + regexp.MustCompile(`\b(shutdown|reboot|poweroff)\b`), + regexp.MustCompile(`:\(\)\s*\{.*\};\s*:`), + regexp.MustCompile(`\$\([^)]+\)`), + regexp.MustCompile(`\$\{[^}]+\}`), + regexp.MustCompile("`[^`]+`"), + regexp.MustCompile(`\|\s*sh\b`), + regexp.MustCompile(`\|\s*bash\b`), + regexp.MustCompile(`;\s*rm\s+-[rf]`), + regexp.MustCompile(`&&\s*rm\s+-[rf]`), + regexp.MustCompile(`\|\|\s*rm\s+-[rf]`), + regexp.MustCompile(`>\s*/dev/null\s*>&?\s*\d?`), + regexp.MustCompile(`<<\s*EOF`), + regexp.MustCompile(`\$\(\s*cat\s+`), + regexp.MustCompile(`\$\(\s*curl\s+`), + regexp.MustCompile(`\$\(\s*wget\s+`), + regexp.MustCompile(`\$\(\s*which\s+`), + regexp.MustCompile(`\bsudo\b`), + regexp.MustCompile(`\bchmod\s+[0-7]{3,4}\b`), + regexp.MustCompile(`\bchown\b`), + regexp.MustCompile(`\bpkill\b`), + regexp.MustCompile(`\bkillall\b`), + regexp.MustCompile(`\bkill\s+-[9]\b`), + regexp.MustCompile(`\bcurl\b.*\|\s*(sh|bash)`), + regexp.MustCompile(`\bwget\b.*\|\s*(sh|bash)`), + regexp.MustCompile(`\bnpm\s+install\s+-g\b`), + regexp.MustCompile(`\bpip\s+install\s+--user\b`), + regexp.MustCompile(`\bapt\s+(install|remove|purge)\b`), + regexp.MustCompile(`\byum\s+(install|remove)\b`), + regexp.MustCompile(`\bdnf\s+(install|remove)\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`), + regexp.MustCompile(`\bssh\b.*@`), + regexp.MustCompile(`\beval\b`), + regexp.MustCompile(`\bsource\s+.*\.sh\b`), +} + func NewExecTool(workingDir string, restrict bool) *ExecTool { - denyPatterns := []*regexp.Regexp{ - regexp.MustCompile(`\brm\s+-[rf]{1,2}\b`), - regexp.MustCompile(`\bdel\s+/[fq]\b`), - regexp.MustCompile(`\brmdir\s+/s\b`), - regexp.MustCompile(`\b(format|mkfs|diskpart)\b\s`), // Match disk wiping commands (must be followed by space/args) - regexp.MustCompile(`\bdd\s+if=`), - regexp.MustCompile(`>\s*/dev/sd[a-z]\b`), // Block writes to disk devices (but allow /dev/null) - regexp.MustCompile(`\b(shutdown|reboot|poweroff)\b`), - regexp.MustCompile(`:\(\)\s*\{.*\};\s*:`), - regexp.MustCompile(`\$\([^)]+\)`), - regexp.MustCompile(`\$\{[^}]+\}`), - regexp.MustCompile("`[^`]+`"), - regexp.MustCompile(`\|\s*sh\b`), - regexp.MustCompile(`\|\s*bash\b`), - regexp.MustCompile(`;\s*rm\s+-[rf]`), - regexp.MustCompile(`&&\s*rm\s+-[rf]`), - regexp.MustCompile(`\|\|\s*rm\s+-[rf]`), - regexp.MustCompile(`>\s*/dev/null\s*>&?\s*\d?`), - regexp.MustCompile(`<<\s*EOF`), - regexp.MustCompile(`\$\(\s*cat\s+`), - regexp.MustCompile(`\$\(\s*curl\s+`), - regexp.MustCompile(`\$\(\s*wget\s+`), - regexp.MustCompile(`\$\(\s*which\s+`), - regexp.MustCompile(`\bsudo\b`), - regexp.MustCompile(`\bchmod\s+[0-7]{3,4}\b`), - regexp.MustCompile(`\bchown\b`), - regexp.MustCompile(`\bpkill\b`), - regexp.MustCompile(`\bkillall\b`), - regexp.MustCompile(`\bkill\s+-[9]\b`), - regexp.MustCompile(`\bcurl\b.*\|\s*(sh|bash)`), - regexp.MustCompile(`\bwget\b.*\|\s*(sh|bash)`), - regexp.MustCompile(`\bnpm\s+install\s+-g\b`), - regexp.MustCompile(`\bpip\s+install\s+--user\b`), - regexp.MustCompile(`\bapt\s+(install|remove|purge)\b`), - regexp.MustCompile(`\byum\s+(install|remove)\b`), - regexp.MustCompile(`\bdnf\s+(install|remove)\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`), - regexp.MustCompile(`\bssh\b.*@`), - regexp.MustCompile(`\beval\b`), - regexp.MustCompile(`\bsource\s+.*\.sh\b`), + return NewExecToolWithConfig(workingDir, restrict, nil) +} + +func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Config) *ExecTool { + denyPatterns := make([]*regexp.Regexp, 0) + + enableDenyPatterns := true + if config != nil { + execConfig := config.Tools.Exec + enableDenyPatterns = execConfig.EnableDenyPatterns + if enableDenyPatterns { + if len(execConfig.CustomDenyPatterns) > 0 { + fmt.Printf("Using custom deny patterns: %v\n", execConfig.CustomDenyPatterns) + for _, pattern := range execConfig.CustomDenyPatterns { + re, err := regexp.Compile(pattern) + if err != nil { + fmt.Printf("Invalid custom deny pattern %q: %v\n", pattern, err) + continue + } + denyPatterns = append(denyPatterns, re) + } + } else { + denyPatterns = append(denyPatterns, defaultDenyPatterns...) + } + } else { + // If deny patterns are disabled, we won't add any patterns, allowing all commands. + fmt.Println("Warning: deny patterns are disabled. All commands will be allowed.") + } + } else { + denyPatterns = append(denyPatterns, defaultDenyPatterns...) } return &ExecTool{