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.
This commit is contained in:
parent
574a59e1a1
commit
ca390846a5
4 changed files with 89 additions and 1 deletions
|
|
@ -450,6 +450,9 @@ type CronToolsConfig struct {
|
||||||
type ExecConfig struct {
|
type ExecConfig struct {
|
||||||
EnableDenyPatterns bool `json:"enable_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"`
|
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"`
|
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 {
|
type ToolPolicyConfig struct {
|
||||||
|
|
|
||||||
|
|
@ -289,6 +289,7 @@ func DefaultConfig() *Config {
|
||||||
},
|
},
|
||||||
Exec: ExecConfig{
|
Exec: ExecConfig{
|
||||||
EnableDenyPatterns: true,
|
EnableDenyPatterns: true,
|
||||||
|
MaxCommandLength: 10000,
|
||||||
},
|
},
|
||||||
Security: SecurityConfig{
|
Security: SecurityConfig{
|
||||||
DefaultMaxArgSize: 100000,
|
DefaultMaxArgSize: 100000,
|
||||||
|
|
|
||||||
|
|
@ -22,6 +22,7 @@ type ExecTool struct {
|
||||||
denyPatterns []*regexp.Regexp
|
denyPatterns []*regexp.Regexp
|
||||||
allowPatterns []*regexp.Regexp
|
allowPatterns []*regexp.Regexp
|
||||||
restrictToWorkspace bool
|
restrictToWorkspace bool
|
||||||
|
maxCommandLength int
|
||||||
}
|
}
|
||||||
|
|
||||||
var defaultDenyPatterns = []*regexp.Regexp{
|
var defaultDenyPatterns = []*regexp.Regexp{
|
||||||
|
|
@ -102,12 +103,32 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf
|
||||||
denyPatterns = append(denyPatterns, defaultDenyPatterns...)
|
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{
|
return &ExecTool{
|
||||||
workingDir: workingDir,
|
workingDir: workingDir,
|
||||||
timeout: 60 * time.Second,
|
timeout: 60 * time.Second,
|
||||||
denyPatterns: denyPatterns,
|
denyPatterns: denyPatterns,
|
||||||
allowPatterns: nil,
|
allowPatterns: allowPatterns,
|
||||||
restrictToWorkspace: restrict,
|
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 {
|
func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
cmd := strings.TrimSpace(command)
|
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)
|
lower := strings.ToLower(cmd)
|
||||||
|
|
||||||
for _, pattern := range t.denyPatterns {
|
for _, pattern := range t.denyPatterns {
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,9 @@ import (
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/sipeed/picoclaw/pkg/config"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
)
|
)
|
||||||
|
|
||||||
// TestShellTool_Success verifies successful command execution
|
// 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)
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue