Skip to content

Commit d5370c9

Browse files
vowstarvowstar
authored
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 #964 Closes #965 Signed-off-by: Huang Rui <vowstar@gmail.com> * 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> --------- Signed-off-by: Huang Rui <vowstar@gmail.com>
1 parent b263375 commit d5370c9

File tree

7 files changed

+319
-63
lines changed

7 files changed

+319
-63
lines changed

‎pkg/agent/instance.go‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package agent
22

33
import (
4+
"fmt"
45
"log"
56
"os"
67
"path/filepath"
8+
"regexp"
79
"strings"
810

911
"github.com/sipeed/picoclaw/pkg/config"
@@ -48,18 +50,24 @@ func NewAgentInstance(
4850
fallbacks := resolveAgentFallbacks(agentCfg, defaults)
4951

5052
restrict := defaults.RestrictToWorkspace
53+
readRestrict := restrict && !defaults.AllowReadOutsideWorkspace
54+
55+
// Compile path whitelist patterns from config.
56+
allowReadPaths := compilePatterns(cfg.Tools.AllowReadPaths)
57+
allowWritePaths := compilePatterns(cfg.Tools.AllowWritePaths)
58+
5159
toolsRegistry := tools.NewToolRegistry()
52-
toolsRegistry.Register(tools.NewReadFileTool(workspace, restrict))
53-
toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
54-
toolsRegistry.Register(tools.NewListDirTool(workspace, restrict))
60+
toolsRegistry.Register(tools.NewReadFileTool(workspace, readRestrict, allowReadPaths))
61+
toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict, allowWritePaths))
62+
toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict, allowReadPaths))
5563
execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg)
5664
if err != nil {
5765
log.Fatalf("Critical error: unable to initialize exec tool: %v", err)
5866
}
5967
toolsRegistry.Register(execTool)
6068

61-
toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict))
62-
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
69+
toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict, allowWritePaths))
70+
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict, allowWritePaths))
6371

6472
sessionsDir := filepath.Join(workspace, "sessions")
6573
sessionsManager := session.NewSessionManager(sessionsDir)
@@ -189,6 +197,19 @@ func resolveAgentFallbacks(agentCfg *config.AgentConfig, defaults *config.AgentD
189197
return defaults.ModelFallbacks
190198
}
191199

200+
func compilePatterns(patterns []string) []*regexp.Regexp {
201+
compiled := make([]*regexp.Regexp, 0, len(patterns))
202+
for _, p := range patterns {
203+
re, err := regexp.Compile(p)
204+
if err != nil {
205+
fmt.Printf("Warning: invalid path pattern %q: %v\n", p, err)
206+
continue
207+
}
208+
compiled = append(compiled, re)
209+
}
210+
return compiled
211+
}
212+
192213
func expandHome(path string) string {
193214
if path == "" {
194215
return path

‎pkg/config/config.go‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -168,17 +168,18 @@ type SessionConfig struct {
168168
}
169169

170170
type AgentDefaults struct {
171-
Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"`
172-
RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE"`
173-
Provider string `json:"provider" env:"PICOCLAW_AGENTS_DEFAULTS_PROVIDER"`
174-
ModelName string `json:"model_name,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL_NAME"`
175-
Model string `json:"model" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL"` // Deprecated: use model_name instead
176-
ModelFallbacks []string `json:"model_fallbacks,omitempty"`
177-
ImageModel string `json:"image_model,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_IMAGE_MODEL"`
178-
ImageModelFallbacks []string `json:"image_model_fallbacks,omitempty"`
179-
MaxTokens int `json:"max_tokens" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOKENS"`
180-
Temperature *float64 `json:"temperature,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_TEMPERATURE"`
181-
MaxToolIterations int `json:"max_tool_iterations" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOOL_ITERATIONS"`
171+
Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"`
172+
RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE"`
173+
AllowReadOutsideWorkspace bool `json:"allow_read_outside_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_ALLOW_READ_OUTSIDE_WORKSPACE"`
174+
Provider string `json:"provider" env:"PICOCLAW_AGENTS_DEFAULTS_PROVIDER"`
175+
ModelName string `json:"model_name,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL_NAME"`
176+
Model string `json:"model" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL"` // Deprecated: use model_name instead
177+
ModelFallbacks []string `json:"model_fallbacks,omitempty"`
178+
ImageModel string `json:"image_model,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_IMAGE_MODEL"`
179+
ImageModelFallbacks []string `json:"image_model_fallbacks,omitempty"`
180+
MaxTokens int `json:"max_tokens" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOKENS"`
181+
Temperature *float64 `json:"temperature,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_TEMPERATURE"`
182+
MaxToolIterations int `json:"max_tool_iterations" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOOL_ITERATIONS"`
182183
}
183184

184185
// GetModelName returns the effective model name for the agent defaults.
@@ -532,8 +533,9 @@ type CronToolsConfig struct {
532533
}
533534

534535
type ExecConfig struct {
535-
EnableDenyPatterns bool `json:"enable_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"`
536-
CustomDenyPatterns []string `json:"custom_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"`
536+
EnableDenyPatterns bool `json:"enable_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"`
537+
CustomDenyPatterns []string `json:"custom_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"`
538+
CustomAllowPatterns []string `json:"custom_allow_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS"`
537539
}
538540

539541
type MediaCleanupConfig struct {
@@ -543,11 +545,13 @@ type MediaCleanupConfig struct {
543545
}
544546

545547
type ToolsConfig struct {
546-
Web WebToolsConfig `json:"web"`
547-
Cron CronToolsConfig `json:"cron"`
548-
Exec ExecConfig `json:"exec"`
549-
Skills SkillsToolsConfig `json:"skills"`
550-
MediaCleanup MediaCleanupConfig `json:"media_cleanup"`
548+
AllowReadPaths []string `json:"allow_read_paths" env:"PICOCLAW_TOOLS_ALLOW_READ_PATHS"`
549+
AllowWritePaths []string `json:"allow_write_paths" env:"PICOCLAW_TOOLS_ALLOW_WRITE_PATHS"`
550+
Web WebToolsConfig `json:"web"`
551+
Cron CronToolsConfig `json:"cron"`
552+
Exec ExecConfig `json:"exec"`
553+
Skills SkillsToolsConfig `json:"skills"`
554+
MediaCleanup MediaCleanupConfig `json:"media_cleanup"`
551555
}
552556

553557
type SkillsToolsConfig struct {

‎pkg/tools/edit.go‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"io/fs"
8+
"regexp"
89
"strings"
910
)
1011

@@ -15,14 +16,12 @@ type EditFileTool struct {
1516
}
1617

1718
// NewEditFileTool creates a new EditFileTool with optional directory restriction.
18-
func NewEditFileTool(workspace string, restrict bool) *EditFileTool {
19-
var fs fileSystem
20-
if restrict {
21-
fs = &sandboxFs{workspace: workspace}
22-
} else {
23-
fs = &hostFs{}
24-
}
25-
return &EditFileTool{fs: fs}
19+
func NewEditFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *EditFileTool {
20+
var patterns []*regexp.Regexp
21+
if len(allowPaths) > 0 {
22+
patterns = allowPaths[0]
23+
}
24+
return &EditFileTool{fs: buildFs(workspace, restrict, patterns)}
2625
}
2726

2827
func (t *EditFileTool) Name() string {
@@ -80,14 +79,12 @@ type AppendFileTool struct {
8079
fs fileSystem
8180
}
8281

83-
func NewAppendFileTool(workspace string, restrict bool) *AppendFileTool {
84-
var fs fileSystem
85-
if restrict {
86-
fs = &sandboxFs{workspace: workspace}
87-
} else {
88-
fs = &hostFs{}
82+
func NewAppendFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *AppendFileTool {
83+
var patterns []*regexp.Regexp
84+
if len(allowPaths) > 0 {
85+
patterns = allowPaths[0]
8986
}
90-
return &AppendFileTool{fs: fs}
87+
return &AppendFileTool{fs: buildFs(workspace, restrict, patterns)}
9188
}
9289

9390
func (t *AppendFileTool) Name() string {

‎pkg/tools/filesystem.go‎

Lines changed: 67 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"io/fs"
77
"os"
88
"path/filepath"
9+
"regexp"
910
"strings"
1011
"time"
1112

@@ -87,14 +88,12 @@ type ReadFileTool struct {
8788
fs fileSystem
8889
}
8990

90-
func NewReadFileTool(workspace string, restrict bool) *ReadFileTool {
91-
var fs fileSystem
92-
if restrict {
93-
fs = &sandboxFs{workspace: workspace}
94-
} else {
95-
fs = &hostFs{}
91+
func NewReadFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *ReadFileTool {
92+
var patterns []*regexp.Regexp
93+
if len(allowPaths) > 0 {
94+
patterns = allowPaths[0]
9695
}
97-
return &ReadFileTool{fs: fs}
96+
return &ReadFileTool{fs: buildFs(workspace, restrict, patterns)}
9897
}
9998

10099
func (t *ReadFileTool) Name() string {
@@ -135,14 +134,12 @@ type WriteFileTool struct {
135134
fs fileSystem
136135
}
137136

138-
func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool {
139-
var fs fileSystem
140-
if restrict {
141-
fs = &sandboxFs{workspace: workspace}
142-
} else {
143-
fs = &hostFs{}
137+
func NewWriteFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *WriteFileTool {
138+
var patterns []*regexp.Regexp
139+
if len(allowPaths) > 0 {
140+
patterns = allowPaths[0]
144141
}
145-
return &WriteFileTool{fs: fs}
142+
return &WriteFileTool{fs: buildFs(workspace, restrict, patterns)}
146143
}
147144

148145
func (t *WriteFileTool) Name() string {
@@ -192,14 +189,12 @@ type ListDirTool struct {
192189
fs fileSystem
193190
}
194191

195-
func NewListDirTool(workspace string, restrict bool) *ListDirTool {
196-
var fs fileSystem
197-
if restrict {
198-
fs = &sandboxFs{workspace: workspace}
199-
} else {
200-
fs = &hostFs{}
192+
func NewListDirTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *ListDirTool {
193+
var patterns []*regexp.Regexp
194+
if len(allowPaths) > 0 {
195+
patterns = allowPaths[0]
201196
}
202-
return &ListDirTool{fs: fs}
197+
return &ListDirTool{fs: buildFs(workspace, restrict, patterns)}
203198
}
204199

205200
func (t *ListDirTool) Name() string {
@@ -394,6 +389,57 @@ func (r *sandboxFs) ReadDir(path string) ([]os.DirEntry, error) {
394389
return entries, err
395390
}
396391

392+
// whitelistFs wraps a sandboxFs and allows access to specific paths outside
393+
// the workspace when they match any of the provided patterns.
394+
type whitelistFs struct {
395+
sandbox *sandboxFs
396+
host hostFs
397+
patterns []*regexp.Regexp
398+
}
399+
400+
func (w *whitelistFs) matches(path string) bool {
401+
for _, p := range w.patterns {
402+
if p.MatchString(path) {
403+
return true
404+
}
405+
}
406+
return false
407+
}
408+
409+
func (w *whitelistFs) ReadFile(path string) ([]byte, error) {
410+
if w.matches(path) {
411+
return w.host.ReadFile(path)
412+
}
413+
return w.sandbox.ReadFile(path)
414+
}
415+
416+
func (w *whitelistFs) WriteFile(path string, data []byte) error {
417+
if w.matches(path) {
418+
return w.host.WriteFile(path, data)
419+
}
420+
return w.sandbox.WriteFile(path, data)
421+
}
422+
423+
func (w *whitelistFs) ReadDir(path string) ([]os.DirEntry, error) {
424+
if w.matches(path) {
425+
return w.host.ReadDir(path)
426+
}
427+
return w.sandbox.ReadDir(path)
428+
}
429+
430+
// buildFs returns the appropriate fileSystem implementation based on restriction
431+
// settings and optional path whitelist patterns.
432+
func buildFs(workspace string, restrict bool, patterns []*regexp.Regexp) fileSystem {
433+
if !restrict {
434+
return &hostFs{}
435+
}
436+
sandbox := &sandboxFs{workspace: workspace}
437+
if len(patterns) > 0 {
438+
return &whitelistFs{sandbox: sandbox, patterns: patterns}
439+
}
440+
return sandbox
441+
}
442+
397443
// Helper to get a safe relative path for os.Root usage
398444
func getSafeRelPath(workspace, path string) (string, error) {
399445
if workspace == "" {

‎pkg/tools/filesystem_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"io"
66
"os"
77
"path/filepath"
8+
"regexp"
89
"strings"
910
"testing"
1011

@@ -486,3 +487,36 @@ func TestRootRW_Write(t *testing.T) {
486487
assert.NoError(t, err)
487488
assert.Equal(t, newData, content)
488489
}
490+
491+
// TestWhitelistFs_AllowsMatchingPaths verifies that whitelistFs allows access to
492+
// paths matching the whitelist patterns while blocking non-matching paths.
493+
func TestWhitelistFs_AllowsMatchingPaths(t *testing.T) {
494+
workspace := t.TempDir()
495+
outsideDir := t.TempDir()
496+
outsideFile := filepath.Join(outsideDir, "allowed.txt")
497+
os.WriteFile(outsideFile, []byte("outside content"), 0o644)
498+
499+
// Pattern allows access to the outsideDir.
500+
patterns := []*regexp.Regexp{regexp.MustCompile(`^` + regexp.QuoteMeta(outsideDir))}
501+
502+
tool := NewReadFileTool(workspace, true, patterns)
503+
504+
// Read from whitelisted path should succeed.
505+
result := tool.Execute(context.Background(), map[string]any{"path": outsideFile})
506+
if result.IsError {
507+
t.Errorf("expected whitelisted path to be readable, got: %s", result.ForLLM)
508+
}
509+
if !strings.Contains(result.ForLLM, "outside content") {
510+
t.Errorf("expected file content, got: %s", result.ForLLM)
511+
}
512+
513+
// Read from non-whitelisted path outside workspace should fail.
514+
otherDir := t.TempDir()
515+
otherFile := filepath.Join(otherDir, "blocked.txt")
516+
os.WriteFile(otherFile, []byte("blocked"), 0o644)
517+
518+
result = tool.Execute(context.Background(), map[string]any{"path": otherFile})
519+
if !result.IsError {
520+
t.Errorf("expected non-whitelisted path to be blocked, got: %s", result.ForLLM)
521+
}
522+
}

0 commit comments

Comments
 (0)