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 <vowstar@gmail.com>
This commit is contained in:
parent
6053baaf82
commit
10884f7f81
7 changed files with 206 additions and 49 deletions
|
|
@ -1,9 +1,11 @@
|
|||
package agent
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"log"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"github.com/sipeed/picoclaw/pkg/config"
|
||||
|
|
@ -49,18 +51,23 @@ func NewAgentInstance(
|
|||
|
||||
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, readRestrict))
|
||||
toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
|
||||
toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict))
|
||||
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)
|
||||
|
|
@ -190,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
|
||||
|
|
|
|||
|
|
@ -535,6 +535,7 @@ 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"`
|
||||
CustomAllowPatterns []string `json:"custom_allow_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS"`
|
||||
}
|
||||
|
||||
type MediaCleanupConfig struct {
|
||||
|
|
@ -544,6 +545,8 @@ type MediaCleanupConfig struct {
|
|||
}
|
||||
|
||||
type ToolsConfig struct {
|
||||
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"`
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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 == "" {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ type ExecTool struct {
|
|||
timeout time.Duration
|
||||
denyPatterns []*regexp.Regexp
|
||||
allowPatterns []*regexp.Regexp
|
||||
customAllowPatterns []*regexp.Regexp
|
||||
restrictToWorkspace bool
|
||||
}
|
||||
|
||||
|
|
@ -98,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
|
||||
|
|
@ -118,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...)
|
||||
}
|
||||
|
|
@ -127,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
|
||||
}
|
||||
|
|
@ -281,11 +291,22 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
|||
cmd := strings.TrimSpace(command)
|
||||
lower := strings.ToLower(cmd)
|
||||
|
||||
// Custom allow patterns exempt a command from deny checks.
|
||||
explicitlyAllowed := false
|
||||
for _, pattern := range t.customAllowPatterns {
|
||||
if pattern.MatchString(lower) {
|
||||
explicitlyAllowed = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if !explicitlyAllowed {
|
||||
for _, pattern := range t.denyPatterns {
|
||||
if pattern.MatchString(lower) {
|
||||
return "Command blocked by safety guard (dangerous pattern detected)"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if len(t.allowPatterns) > 0 {
|
||||
allowed := false
|
||||
|
|
|
|||
|
|
@ -7,6 +7,8 @@ import (
|
|||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/sipeed/picoclaw/pkg/config"
|
||||
)
|
||||
|
||||
// TestShellTool_Success verifies successful command execution
|
||||
|
|
@ -387,3 +389,37 @@ func TestShellTool_SafePathsInWorkspaceRestriction(t *testing.T) {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 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")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue