From d5370c9605c4f007373272ec628f41a34b681eae Mon Sep 17 00:00:00 2001 From: Huang Rui Date: Mon, 2 Mar 2026 12:22:02 +0800 Subject: [PATCH] fix(tools): allow /dev/null redirection and add read/write sandbox split (#967) * fix(tools): allow /dev/null redirection and add read/write sandbox split - Remove deny pattern that incorrectly blocked redirects to /dev/null - Expand block device write pattern to cover nvme, mmcblk, vd, xvd, hd, loop, dm-, md, sr and nbd in addition to sd - Add safe path whitelist for kernel pseudo-devices so workspace path check does not reject /dev/null, /dev/zero, /dev/random, /dev/urandom, /dev/stdin, /dev/stdout and /dev/stderr - Add allow_read_outside_workspace config option (default true) so file read and list tools are unrestricted while write tools stay sandboxed Closes https://github.com/sipeed/picoclaw/issues/964 Closes https://github.com/sipeed/picoclaw/issues/965 Signed-off-by: Huang Rui * feat(tools): add configurable allow patterns and path whitelists - Add custom_allow_patterns to exec config so users can exempt specific commands from deny pattern checks - Add allow_read_paths and allow_write_paths regex lists to tools config for whitelisting specific paths outside the workspace - Introduce whitelistFs that wraps sandboxFs and falls through to hostFs for paths matching whitelist patterns - Use variadic constructor signatures to keep backward compatibility Suggested-by: lxowalle Signed-off-by: Huang Rui --------- Signed-off-by: Huang Rui --- pkg/agent/instance.go | 31 ++++++++-- pkg/config/config.go | 40 ++++++------ pkg/tools/edit.go | 25 ++++---- pkg/tools/filesystem.go | 88 ++++++++++++++++++++------- pkg/tools/filesystem_test.go | 34 +++++++++++ pkg/tools/shell.go | 48 +++++++++++++-- pkg/tools/shell_test.go | 114 +++++++++++++++++++++++++++++++++++ 7 files changed, 318 insertions(+), 62 deletions(-) diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index e597542e4..ed438059f 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -1,9 +1,11 @@ package agent import ( + "fmt" "log" "os" "path/filepath" + "regexp" "strings" "github.com/sipeed/picoclaw/pkg/config" @@ -48,18 +50,24 @@ func NewAgentInstance( fallbacks := resolveAgentFallbacks(agentCfg, defaults) restrict := defaults.RestrictToWorkspace + readRestrict := restrict && !defaults.AllowReadOutsideWorkspace + + // Compile path whitelist patterns from config. + allowReadPaths := compilePatterns(cfg.Tools.AllowReadPaths) + allowWritePaths := compilePatterns(cfg.Tools.AllowWritePaths) + toolsRegistry := tools.NewToolRegistry() - toolsRegistry.Register(tools.NewReadFileTool(workspace, restrict)) - toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict)) - toolsRegistry.Register(tools.NewListDirTool(workspace, restrict)) + toolsRegistry.Register(tools.NewReadFileTool(workspace, readRestrict, allowReadPaths)) + toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict, allowWritePaths)) + toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict, allowReadPaths)) execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg) if err != nil { log.Fatalf("Critical error: unable to initialize exec tool: %v", err) } toolsRegistry.Register(execTool) - toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict)) - toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict)) + toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict, allowWritePaths)) + toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict, allowWritePaths)) sessionsDir := filepath.Join(workspace, "sessions") sessionsManager := session.NewSessionManager(sessionsDir) @@ -189,6 +197,19 @@ func resolveAgentFallbacks(agentCfg *config.AgentConfig, defaults *config.AgentD return defaults.ModelFallbacks } +func compilePatterns(patterns []string) []*regexp.Regexp { + compiled := make([]*regexp.Regexp, 0, len(patterns)) + for _, p := range patterns { + re, err := regexp.Compile(p) + if err != nil { + fmt.Printf("Warning: invalid path pattern %q: %v\n", p, err) + continue + } + compiled = append(compiled, re) + } + return compiled +} + func expandHome(path string) string { if path == "" { return path diff --git a/pkg/config/config.go b/pkg/config/config.go index 55d0cfb2c..9f4769de4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -168,17 +168,18 @@ type SessionConfig struct { } type AgentDefaults struct { - Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"` - RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE"` - Provider string `json:"provider" env:"PICOCLAW_AGENTS_DEFAULTS_PROVIDER"` - ModelName string `json:"model_name,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL_NAME"` - Model string `json:"model" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL"` // Deprecated: use model_name instead - ModelFallbacks []string `json:"model_fallbacks,omitempty"` - ImageModel string `json:"image_model,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_IMAGE_MODEL"` - ImageModelFallbacks []string `json:"image_model_fallbacks,omitempty"` - MaxTokens int `json:"max_tokens" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOKENS"` - Temperature *float64 `json:"temperature,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_TEMPERATURE"` - MaxToolIterations int `json:"max_tool_iterations" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOOL_ITERATIONS"` + Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"` + RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE"` + AllowReadOutsideWorkspace bool `json:"allow_read_outside_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_ALLOW_READ_OUTSIDE_WORKSPACE"` + Provider string `json:"provider" env:"PICOCLAW_AGENTS_DEFAULTS_PROVIDER"` + ModelName string `json:"model_name,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL_NAME"` + Model string `json:"model" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL"` // Deprecated: use model_name instead + ModelFallbacks []string `json:"model_fallbacks,omitempty"` + ImageModel string `json:"image_model,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_IMAGE_MODEL"` + ImageModelFallbacks []string `json:"image_model_fallbacks,omitempty"` + MaxTokens int `json:"max_tokens" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOKENS"` + Temperature *float64 `json:"temperature,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_TEMPERATURE"` + MaxToolIterations int `json:"max_tool_iterations" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOOL_ITERATIONS"` } // GetModelName returns the effective model name for the agent defaults. @@ -532,8 +533,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"` + 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"` + CustomAllowPatterns []string `json:"custom_allow_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS"` } type MediaCleanupConfig struct { @@ -543,11 +545,13 @@ type MediaCleanupConfig struct { } type ToolsConfig struct { - Web WebToolsConfig `json:"web"` - Cron CronToolsConfig `json:"cron"` - Exec ExecConfig `json:"exec"` - Skills SkillsToolsConfig `json:"skills"` - MediaCleanup MediaCleanupConfig `json:"media_cleanup"` + AllowReadPaths []string `json:"allow_read_paths" env:"PICOCLAW_TOOLS_ALLOW_READ_PATHS"` + AllowWritePaths []string `json:"allow_write_paths" env:"PICOCLAW_TOOLS_ALLOW_WRITE_PATHS"` + Web WebToolsConfig `json:"web"` + Cron CronToolsConfig `json:"cron"` + Exec ExecConfig `json:"exec"` + Skills SkillsToolsConfig `json:"skills"` + MediaCleanup MediaCleanupConfig `json:"media_cleanup"` } type SkillsToolsConfig struct { diff --git a/pkg/tools/edit.go b/pkg/tools/edit.go index d3ab267bf..d5bebf4a2 100644 --- a/pkg/tools/edit.go +++ b/pkg/tools/edit.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io/fs" + "regexp" "strings" ) @@ -15,14 +16,12 @@ type EditFileTool struct { } // NewEditFileTool creates a new EditFileTool with optional directory restriction. -func NewEditFileTool(workspace string, restrict bool) *EditFileTool { - var fs fileSystem - if restrict { - fs = &sandboxFs{workspace: workspace} - } else { - fs = &hostFs{} +func NewEditFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *EditFileTool { + var patterns []*regexp.Regexp + if len(allowPaths) > 0 { + patterns = allowPaths[0] } - return &EditFileTool{fs: fs} + return &EditFileTool{fs: buildFs(workspace, restrict, patterns)} } func (t *EditFileTool) Name() string { @@ -80,14 +79,12 @@ type AppendFileTool struct { fs fileSystem } -func NewAppendFileTool(workspace string, restrict bool) *AppendFileTool { - var fs fileSystem - if restrict { - fs = &sandboxFs{workspace: workspace} - } else { - fs = &hostFs{} +func NewAppendFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *AppendFileTool { + var patterns []*regexp.Regexp + if len(allowPaths) > 0 { + patterns = allowPaths[0] } - return &AppendFileTool{fs: fs} + return &AppendFileTool{fs: buildFs(workspace, restrict, patterns)} } func (t *AppendFileTool) Name() string { diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index 03d461dcc..cd8da3195 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -6,6 +6,7 @@ import ( "io/fs" "os" "path/filepath" + "regexp" "strings" "time" @@ -87,14 +88,12 @@ type ReadFileTool struct { fs fileSystem } -func NewReadFileTool(workspace string, restrict bool) *ReadFileTool { - var fs fileSystem - if restrict { - fs = &sandboxFs{workspace: workspace} - } else { - fs = &hostFs{} +func NewReadFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *ReadFileTool { + var patterns []*regexp.Regexp + if len(allowPaths) > 0 { + patterns = allowPaths[0] } - return &ReadFileTool{fs: fs} + return &ReadFileTool{fs: buildFs(workspace, restrict, patterns)} } func (t *ReadFileTool) Name() string { @@ -135,14 +134,12 @@ type WriteFileTool struct { fs fileSystem } -func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool { - var fs fileSystem - if restrict { - fs = &sandboxFs{workspace: workspace} - } else { - fs = &hostFs{} +func NewWriteFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *WriteFileTool { + var patterns []*regexp.Regexp + if len(allowPaths) > 0 { + patterns = allowPaths[0] } - return &WriteFileTool{fs: fs} + return &WriteFileTool{fs: buildFs(workspace, restrict, patterns)} } func (t *WriteFileTool) Name() string { @@ -192,14 +189,12 @@ type ListDirTool struct { fs fileSystem } -func NewListDirTool(workspace string, restrict bool) *ListDirTool { - var fs fileSystem - if restrict { - fs = &sandboxFs{workspace: workspace} - } else { - fs = &hostFs{} +func NewListDirTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *ListDirTool { + var patterns []*regexp.Regexp + if len(allowPaths) > 0 { + patterns = allowPaths[0] } - return &ListDirTool{fs: fs} + return &ListDirTool{fs: buildFs(workspace, restrict, patterns)} } func (t *ListDirTool) Name() string { @@ -394,6 +389,57 @@ func (r *sandboxFs) ReadDir(path string) ([]os.DirEntry, error) { return entries, err } +// whitelistFs wraps a sandboxFs and allows access to specific paths outside +// the workspace when they match any of the provided patterns. +type whitelistFs struct { + sandbox *sandboxFs + host hostFs + patterns []*regexp.Regexp +} + +func (w *whitelistFs) matches(path string) bool { + for _, p := range w.patterns { + if p.MatchString(path) { + return true + } + } + return false +} + +func (w *whitelistFs) ReadFile(path string) ([]byte, error) { + if w.matches(path) { + return w.host.ReadFile(path) + } + return w.sandbox.ReadFile(path) +} + +func (w *whitelistFs) WriteFile(path string, data []byte) error { + if w.matches(path) { + return w.host.WriteFile(path, data) + } + return w.sandbox.WriteFile(path, data) +} + +func (w *whitelistFs) ReadDir(path string) ([]os.DirEntry, error) { + if w.matches(path) { + return w.host.ReadDir(path) + } + return w.sandbox.ReadDir(path) +} + +// buildFs returns the appropriate fileSystem implementation based on restriction +// settings and optional path whitelist patterns. +func buildFs(workspace string, restrict bool, patterns []*regexp.Regexp) fileSystem { + if !restrict { + return &hostFs{} + } + sandbox := &sandboxFs{workspace: workspace} + if len(patterns) > 0 { + return &whitelistFs{sandbox: sandbox, patterns: patterns} + } + return sandbox +} + // Helper to get a safe relative path for os.Root usage func getSafeRelPath(workspace, path string) (string, error) { if workspace == "" { diff --git a/pkg/tools/filesystem_test.go b/pkg/tools/filesystem_test.go index 6f896e22d..666004cd4 100644 --- a/pkg/tools/filesystem_test.go +++ b/pkg/tools/filesystem_test.go @@ -5,6 +5,7 @@ import ( "io" "os" "path/filepath" + "regexp" "strings" "testing" @@ -486,3 +487,36 @@ func TestRootRW_Write(t *testing.T) { assert.NoError(t, err) assert.Equal(t, newData, content) } + +// TestWhitelistFs_AllowsMatchingPaths verifies that whitelistFs allows access to +// paths matching the whitelist patterns while blocking non-matching paths. +func TestWhitelistFs_AllowsMatchingPaths(t *testing.T) { + workspace := t.TempDir() + outsideDir := t.TempDir() + outsideFile := filepath.Join(outsideDir, "allowed.txt") + os.WriteFile(outsideFile, []byte("outside content"), 0o644) + + // Pattern allows access to the outsideDir. + patterns := []*regexp.Regexp{regexp.MustCompile(`^` + regexp.QuoteMeta(outsideDir))} + + tool := NewReadFileTool(workspace, true, patterns) + + // Read from whitelisted path should succeed. + result := tool.Execute(context.Background(), map[string]any{"path": outsideFile}) + if result.IsError { + t.Errorf("expected whitelisted path to be readable, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "outside content") { + t.Errorf("expected file content, got: %s", result.ForLLM) + } + + // Read from non-whitelisted path outside workspace should fail. + otherDir := t.TempDir() + otherFile := filepath.Join(otherDir, "blocked.txt") + os.WriteFile(otherFile, []byte("blocked"), 0o644) + + result = tool.Execute(context.Background(), map[string]any{"path": otherFile}) + if !result.IsError { + t.Errorf("expected non-whitelisted path to be blocked, got: %s", result.ForLLM) + } +} diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 2fd22353f..a0c83eb1e 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -21,6 +21,7 @@ type ExecTool struct { timeout time.Duration denyPatterns []*regexp.Regexp allowPatterns []*regexp.Regexp + customAllowPatterns []*regexp.Regexp restrictToWorkspace bool } @@ -34,7 +35,10 @@ var ( `\b(format|mkfs|diskpart)\b\s`, ), regexp.MustCompile(`\bdd\s+if=`), - regexp.MustCompile(`>\s*/dev/sd[a-z]\b`), // Block writes to disk devices (but allow /dev/null) + // Block writes to block devices (all common naming schemes). + 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(`:\(\)\s*\{.*\};\s*:`), regexp.MustCompile(`\$\([^)]+\)`), @@ -45,7 +49,6 @@ var ( 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+`), @@ -75,6 +78,19 @@ var ( // absolutePathPattern matches absolute file paths in commands (Unix and Windows). absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`) + + // safePaths are kernel pseudo-devices that are always safe to reference in + // commands, regardless of workspace restriction. They contain no user data + // and cannot cause destructive writes. + safePaths = map[string]bool{ + "/dev/null": true, + "/dev/zero": true, + "/dev/random": true, + "/dev/urandom": true, + "/dev/stdin": true, + "/dev/stdout": true, + "/dev/stderr": true, + } ) func NewExecTool(workingDir string, restrict bool) (*ExecTool, error) { @@ -83,6 +99,7 @@ func NewExecTool(workingDir string, restrict bool) (*ExecTool, error) { func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Config) (*ExecTool, error) { denyPatterns := make([]*regexp.Regexp, 0) + customAllowPatterns := make([]*regexp.Regexp, 0) if config != nil { execConfig := config.Tools.Exec @@ -103,6 +120,13 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf // 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.") } + for _, pattern := range execConfig.CustomAllowPatterns { + re, err := regexp.Compile(pattern) + if err != nil { + return nil, fmt.Errorf("invalid custom allow pattern %q: %w", pattern, err) + } + customAllowPatterns = append(customAllowPatterns, re) + } } else { denyPatterns = append(denyPatterns, defaultDenyPatterns...) } @@ -112,6 +136,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf timeout: 60 * time.Second, denyPatterns: denyPatterns, allowPatterns: nil, + customAllowPatterns: customAllowPatterns, restrictToWorkspace: restrict, }, nil } @@ -266,9 +291,20 @@ func (t *ExecTool) guardCommand(command, cwd string) string { cmd := strings.TrimSpace(command) lower := strings.ToLower(cmd) - for _, pattern := range t.denyPatterns { + // Custom allow patterns exempt a command from deny checks. + explicitlyAllowed := false + for _, pattern := range t.customAllowPatterns { if pattern.MatchString(lower) { - return "Command blocked by safety guard (dangerous pattern detected)" + explicitlyAllowed = true + break + } + } + + if !explicitlyAllowed { + for _, pattern := range t.denyPatterns { + if pattern.MatchString(lower) { + return "Command blocked by safety guard (dangerous pattern detected)" + } } } @@ -303,6 +339,10 @@ func (t *ExecTool) guardCommand(command, cwd string) string { continue } + if safePaths[p] { + continue + } + rel, err := filepath.Rel(cwdPath, p) if err != nil { continue diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 1a179547a..a6abca8ea 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -7,6 +7,8 @@ import ( "strings" "testing" "time" + + "github.com/sipeed/picoclaw/pkg/config" ) // TestShellTool_Success verifies successful command execution @@ -309,3 +311,115 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) { ) } } + +// TestShellTool_DevNullAllowed verifies that /dev/null redirections are not blocked (issue #964). +func TestShellTool_DevNullAllowed(t *testing.T) { + tmpDir := t.TempDir() + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + commands := []string{ + "echo hello 2>/dev/null", + "echo hello >/dev/null", + "echo hello > /dev/null", + "echo hello 2> /dev/null", + "echo hello >/dev/null 2>&1", + "find " + tmpDir + " -name '*.go' 2>/dev/null", + } + + for _, cmd := range commands { + result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + if result.IsError && strings.Contains(result.ForLLM, "blocked") { + t.Errorf("command should not be blocked: %s\n error: %s", cmd, result.ForLLM) + } + } +} + +// TestShellTool_BlockDevices verifies that writes to block devices are blocked (issue #965). +func TestShellTool_BlockDevices(t *testing.T) { + tool, err := NewExecTool("", false) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + blocked := []string{ + "echo x > /dev/sda", + "echo x > /dev/hda", + "echo x > /dev/vda", + "echo x > /dev/xvda", + "echo x > /dev/nvme0n1", + "echo x > /dev/mmcblk0", + "echo x > /dev/loop0", + "echo x > /dev/dm-0", + "echo x > /dev/md0", + "echo x > /dev/sr0", + "echo x > /dev/nbd0", + } + + for _, cmd := range blocked { + result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + if !result.IsError { + t.Errorf("expected block device write to be blocked: %s", cmd) + } + } +} + +// TestShellTool_SafePathsInWorkspaceRestriction verifies that safe kernel pseudo-devices +// are allowed even when workspace restriction is active. +func TestShellTool_SafePathsInWorkspaceRestriction(t *testing.T) { + tmpDir := t.TempDir() + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + // These reference paths outside workspace but should be allowed via safePaths. + commands := []string{ + "cat /dev/urandom | head -c 16 | od", + "echo test > /dev/null", + "dd if=/dev/zero bs=1 count=1", + } + + for _, cmd := range commands { + result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") { + t.Errorf("safe path should not be blocked by workspace check: %s\n error: %s", cmd, result.ForLLM) + } + } +} + +// TestShellTool_CustomAllowPatterns verifies that custom allow patterns exempt +// commands from deny pattern checks. +func TestShellTool_CustomAllowPatterns(t *testing.T) { + cfg := &config.Config{ + Tools: config.ToolsConfig{ + Exec: config.ExecConfig{ + EnableDenyPatterns: true, + CustomAllowPatterns: []string{`\bgit\s+push\s+origin\b`}, + }, + }, + } + + tool, err := NewExecToolWithConfig("", false, cfg) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + // "git push origin main" should be allowed by custom allow pattern. + result := tool.Execute(context.Background(), map[string]any{ + "command": "git push origin main", + }) + if result.IsError && strings.Contains(result.ForLLM, "blocked") { + t.Errorf("custom allow pattern should exempt 'git push origin main', got: %s", result.ForLLM) + } + + // "git push upstream main" should still be blocked (does not match allow pattern). + result = tool.Execute(context.Background(), map[string]any{ + "command": "git push upstream main", + }) + if !result.IsError { + t.Errorf("'git push upstream main' should still be blocked by deny pattern") + } +}