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 <vowstar@gmail.com>
This commit is contained in:
parent
b26337501c
commit
6053baaf82
4 changed files with 114 additions and 15 deletions
|
|
@ -48,10 +48,11 @@ func NewAgentInstance(
|
||||||
fallbacks := resolveAgentFallbacks(agentCfg, defaults)
|
fallbacks := resolveAgentFallbacks(agentCfg, defaults)
|
||||||
|
|
||||||
restrict := defaults.RestrictToWorkspace
|
restrict := defaults.RestrictToWorkspace
|
||||||
|
readRestrict := restrict && !defaults.AllowReadOutsideWorkspace
|
||||||
toolsRegistry := tools.NewToolRegistry()
|
toolsRegistry := tools.NewToolRegistry()
|
||||||
toolsRegistry.Register(tools.NewReadFileTool(workspace, restrict))
|
toolsRegistry.Register(tools.NewReadFileTool(workspace, readRestrict))
|
||||||
toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
|
toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
|
||||||
toolsRegistry.Register(tools.NewListDirTool(workspace, restrict))
|
toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict))
|
||||||
execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg)
|
execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Fatalf("Critical error: unable to initialize exec tool: %v", err)
|
log.Fatalf("Critical error: unable to initialize exec tool: %v", err)
|
||||||
|
|
|
||||||
|
|
@ -170,6 +170,7 @@ type SessionConfig struct {
|
||||||
type AgentDefaults struct {
|
type AgentDefaults struct {
|
||||||
Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"`
|
Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"`
|
||||||
RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_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"`
|
Provider string `json:"provider" env:"PICOCLAW_AGENTS_DEFAULTS_PROVIDER"`
|
||||||
ModelName string `json:"model_name,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL_NAME"`
|
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
|
Model string `json:"model" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL"` // Deprecated: use model_name instead
|
||||||
|
|
|
||||||
|
|
@ -34,7 +34,10 @@ var (
|
||||||
`\b(format|mkfs|diskpart)\b\s`,
|
`\b(format|mkfs|diskpart)\b\s`,
|
||||||
),
|
),
|
||||||
regexp.MustCompile(`\bdd\s+if=`),
|
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(`\b(shutdown|reboot|poweroff)\b`),
|
||||||
regexp.MustCompile(`:\(\)\s*\{.*\};\s*:`),
|
regexp.MustCompile(`:\(\)\s*\{.*\};\s*:`),
|
||||||
regexp.MustCompile(`\$\([^)]+\)`),
|
regexp.MustCompile(`\$\([^)]+\)`),
|
||||||
|
|
@ -45,7 +48,6 @@ var (
|
||||||
regexp.MustCompile(`;\s*rm\s+-[rf]`),
|
regexp.MustCompile(`;\s*rm\s+-[rf]`),
|
||||||
regexp.MustCompile(`&&\s*rm\s+-[rf]`),
|
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*EOF`),
|
||||||
regexp.MustCompile(`\$\(\s*cat\s+`),
|
regexp.MustCompile(`\$\(\s*cat\s+`),
|
||||||
regexp.MustCompile(`\$\(\s*curl\s+`),
|
regexp.MustCompile(`\$\(\s*curl\s+`),
|
||||||
|
|
@ -75,6 +77,19 @@ var (
|
||||||
|
|
||||||
// absolutePathPattern matches absolute file paths in commands (Unix and Windows).
|
// absolutePathPattern matches absolute file paths in commands (Unix and Windows).
|
||||||
absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
|
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) {
|
func NewExecTool(workingDir string, restrict bool) (*ExecTool, error) {
|
||||||
|
|
@ -303,6 +318,10 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if safePaths[p] {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
rel, err := filepath.Rel(cwdPath, p)
|
rel, err := filepath.Rel(cwdPath, p)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
continue
|
continue
|
||||||
|
|
|
||||||
|
|
@ -309,3 +309,81 @@ 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue