From ca390846a5751f494bdffe91464804ccf6d74b63 Mon Sep 17 00:00:00 2001 From: Paul De Velder Date: Mon, 23 Feb 2026 14:59:33 +0100 Subject: [PATCH] feat(security): add exec allowlist mode and command length limit Add opt-in allowlist for exec tool alongside existing deny patterns. When enabled, only commands matching allow patterns are permitted. Also enforce a configurable max command length (default 10000 chars) to prevent input abuse. --- pkg/config/config.go | 3 +++ pkg/config/defaults.go | 1 + pkg/tools/shell.go | 28 +++++++++++++++++++- pkg/tools/shell_test.go | 58 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 73e6780af..035d6d37c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -450,6 +450,9 @@ type CronToolsConfig struct { 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"` + EnableAllowlist bool `json:"enable_allowlist" env:"PICOCLAW_TOOLS_EXEC_ENABLE_ALLOWLIST"` + AllowPatterns []string `json:"allow_patterns" env:"PICOCLAW_TOOLS_EXEC_ALLOW_PATTERNS"` + MaxCommandLength int `json:"max_command_length" env:"PICOCLAW_TOOLS_EXEC_MAX_COMMAND_LENGTH"` } type ToolPolicyConfig struct { diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index bf041f2ed..1a2e66b3e 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -289,6 +289,7 @@ func DefaultConfig() *Config { }, Exec: ExecConfig{ EnableDenyPatterns: true, + MaxCommandLength: 10000, }, Security: SecurityConfig{ DefaultMaxArgSize: 100000, diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a1ee0b6e1..5acff48d0 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -22,6 +22,7 @@ type ExecTool struct { denyPatterns []*regexp.Regexp allowPatterns []*regexp.Regexp restrictToWorkspace bool + maxCommandLength int } var defaultDenyPatterns = []*regexp.Regexp{ @@ -102,12 +103,32 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf denyPatterns = append(denyPatterns, defaultDenyPatterns...) } + var allowPatterns []*regexp.Regexp + maxCmdLen := 10000 + if config != nil { + execConfig := config.Tools.Exec + if execConfig.EnableAllowlist && len(execConfig.AllowPatterns) > 0 { + for _, pattern := range execConfig.AllowPatterns { + re, err := regexp.Compile(pattern) + if err != nil { + fmt.Printf("Invalid allow pattern %q: %v\n", pattern, err) + continue + } + allowPatterns = append(allowPatterns, re) + } + } + if execConfig.MaxCommandLength > 0 { + maxCmdLen = execConfig.MaxCommandLength + } + } + return &ExecTool{ workingDir: workingDir, timeout: 60 * time.Second, denyPatterns: denyPatterns, - allowPatterns: nil, + allowPatterns: allowPatterns, restrictToWorkspace: restrict, + maxCommandLength: maxCmdLen, } } @@ -259,6 +280,11 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult func (t *ExecTool) guardCommand(command, cwd string) string { cmd := strings.TrimSpace(command) + + if t.maxCommandLength > 0 && len(cmd) > t.maxCommandLength { + return "Command blocked by safety guard (exceeds max command length)" + } + lower := strings.ToLower(cmd) for _, pattern := range t.denyPatterns { diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 6d35815e8..12edd5c78 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -7,6 +7,9 @@ import ( "strings" "testing" "time" + + "github.com/sipeed/picoclaw/pkg/config" + "github.com/stretchr/testify/assert" ) // TestShellTool_Success verifies successful command execution @@ -272,3 +275,58 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) { ) } } + +// --- Allowlist and Command Length Tests --- + +func TestShellTool_MaxCommandLength_Blocks(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Exec.MaxCommandLength = 50 + tool := NewExecToolWithConfig(t.TempDir(), false, cfg) + + longCmd := strings.Repeat("a", 100) + result := tool.Execute(context.Background(), map[string]any{"command": longCmd}) + + assert.True(t, result.IsError) + assert.Contains(t, result.ForLLM, "max command length") +} + +func TestShellTool_MaxCommandLength_Allows(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Exec.MaxCommandLength = 100 + tool := NewExecToolWithConfig(t.TempDir(), false, cfg) + + result := tool.Execute(context.Background(), map[string]any{"command": "echo hello"}) + assert.False(t, result.IsError, "short command should be allowed: %s", result.ForLLM) +} + +func TestShellTool_AllowlistMode_Blocks(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Exec.EnableAllowlist = true + cfg.Tools.Exec.AllowPatterns = []string{`\becho\b`, `\bls\b`} + tool := NewExecToolWithConfig(t.TempDir(), false, cfg) + + // "cat" is not in the allowlist + result := tool.Execute(context.Background(), map[string]any{"command": "cat /etc/passwd"}) + assert.True(t, result.IsError) + assert.Contains(t, result.ForLLM, "allowlist") +} + +func TestShellTool_AllowlistMode_Allows(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Exec.EnableAllowlist = true + cfg.Tools.Exec.AllowPatterns = []string{`\becho\b`, `\bls\b`} + tool := NewExecToolWithConfig(t.TempDir(), false, cfg) + + result := tool.Execute(context.Background(), map[string]any{"command": "echo hello"}) + assert.False(t, result.IsError, "echo should be allowed: %s", result.ForLLM) +} + +func TestShellTool_AllowlistDisabled_AllowsAll(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Exec.EnableAllowlist = false + tool := NewExecToolWithConfig(t.TempDir(), false, cfg) + + // Without allowlist, normal commands pass (unless blocked by deny patterns) + result := tool.Execute(context.Background(), map[string]any{"command": "echo test"}) + assert.False(t, result.IsError) +}