opencode/picoclaw
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(sandbox): Unify host process management and refine tool enablement logic within the sandbox.

Browse source
This commit is contained in:
0x5487 2026-02-27 13:11:14 +08:00
parent 7a402c4972
commit a75ff2cf82
22 changed files with 832 additions and 817 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
pkg/agent/instance.go
View file

@ -64,27 +64,35 @@ func NewAgentInstance(
toolsRegistry := tools.NewToolRegistry()
sandboxManager := sandbox.NewFromConfigWithAgent(workspace, restrict, cfg, agentID)
isSandboxAllowed := func(toolName string) bool {
return sandboxManager == nil || sandbox.IsToolSandboxEnabled(cfg, toolName)
// isToolEnabled determines if a base system tool should be registered for the LLM.
// SandboxManager *always* provides a Sandbox context (HostSandbox if mode="off").
isToolEnabled := func(toolName string) bool {
// If the user explicitly disables the sandbox mode (Mode = "off"), they are opting out of
// safety isolation. We grant the LLM access to all core tools via the HostSandbox.
if isSandboxModeOff(cfg) {
return true
}
// Otherwise, respect the fine-grained allow/deny policy defined in the Sandbox config.
return sandbox.IsToolSandboxEnabled(cfg, toolName)
}
if isSandboxAllowed("read_file") {
if isToolEnabled("read_file") {
toolsRegistry.Register(tools.NewReadFileTool(workspace, restrict))
}
if !roContainer && isSandboxAllowed("write_file") {
if !roContainer && isToolEnabled("write_file") {
toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
}
if isSandboxAllowed("list_dir") {
if isToolEnabled("list_dir") {
toolsRegistry.Register(tools.NewListDirTool(workspace, restrict))
}
if isSandboxAllowed("exec") {
if isToolEnabled("exec") {
toolsRegistry.Register(tools.NewExecToolWithConfig(workspace, restrict, cfg))
}
if !roContainer {
if isSandboxAllowed("edit_file") {
if isToolEnabled("edit_file") {
toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict))
}
if isSandboxAllowed("append_file") {
if isToolEnabled("append_file") {
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
}
}
@ -170,8 +178,15 @@ func isContainerReadOnlySandbox(cfg *config.Config) bool {
if cfg == nil {
return false
}
return strings.EqualFold(strings.TrimSpace(cfg.Agents.Defaults.Sandbox.Mode), "all") &&
strings.EqualFold(strings.TrimSpace(cfg.Agents.Defaults.Sandbox.WorkspaceAccess), "ro")
return cfg.Agents.Defaults.Sandbox.Mode == config.SandboxModeAll &&
cfg.Agents.Defaults.Sandbox.WorkspaceAccess == config.WorkspaceAccessRO
}
func isSandboxModeOff(cfg *config.Config) bool {
if cfg == nil {
return false
}
return cfg.Agents.Defaults.Sandbox.Mode == config.SandboxModeOff
}
func expandHome(path string) string {

33
pkg/agent/instance_test.go
View file

@ -135,3 +135,36 @@ func TestNewAgentInstance_ReadOnlyContainerOmitsWriteTools(t *testing.T) {
t.Fatalf("write_file should be absent in ro sandbox, got: %+v", writeRes)
}
}
func TestNewAgentInstance_SandboxModeOffRegistersFullToolSet(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "agent-instance-test-*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)
cfg := &config.Config{
Agents: config.AgentsConfig{
Defaults: config.AgentDefaults{
Workspace: tmpDir,
Model: "test-model",
MaxTokens: 1234,
MaxToolIterations: 5,
Sandbox: config.AgentSandboxConfig{
Mode: "off",
},
},
},
}
// Keep sandbox allowlist narrow to ensure mode=off bypasses sandbox policy gating.
cfg.Tools.Sandbox.Tools.Allow = []string{"exec"}
provider := &mockProvider{}
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider)
for _, name := range []string{"read_file", "write_file", "list_dir", "exec", "edit_file", "append_file"} {
if _, ok := agent.Tools.Get(name); !ok {
t.Fatalf("%s should be registered when sandbox.mode=off", name)
}
}
}

19
pkg/agent/loop.go
View file

@ -409,16 +409,17 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
// it against the main session key for non-main mode.
ctx = sandbox.WithSessionKey(ctx, opts.SessionKey)
// Resolve sandbox environment for this run
if agent.SandboxManager != nil {
sb, err := agent.SandboxManager.Resolve(ctx)
if err != nil {
logger.ErrorCF("agent", "Failed to resolve sandbox", map[string]any{"error": err.Error()})
return "", fmt.Errorf("failed to resolve sandbox: %w", err)
}
// Add sandbox to context for thread-safe access in tools via registry
ctx = sandbox.WithSandbox(ctx, sb)
// Resolve sandbox environment for this run.
// We guarantee SandboxManager is non-nil (fallback to host manager) in NewAgentInstance.
ctx = sandbox.WithManager(ctx, agent.SandboxManager)
sb, err := agent.SandboxManager.Resolve(ctx)
if err != nil {
logger.ErrorCF("agent", "Failed to resolve sandbox", map[string]any{"error": err.Error()})
return "", fmt.Errorf("failed to resolve sandbox: %w", err)
}
// Add pre-resolved sandbox to context for thread-safe access in tools
ctx = sandbox.WithSandbox(ctx, sb)
// 2. Build messages (skip history for heartbeat)
var history []providers.Message

110
pkg/agent/sandbox/container.go
View file

@ -7,6 +7,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/fs"
"math"
"os"
"path"
@ -101,7 +102,7 @@ func NewContainerSandbox(cfg ContainerSandboxConfig) *ContainerSandbox {
if cfg.Env == nil {
cfg.Env = map[string]string{"LANG": "C.UTF-8"}
}
cfg.WorkspaceAccess = normalizeWorkspaceAccess(cfg.WorkspaceAccess)
cfg.WorkspaceAccess = string(normalizeWorkspaceAccess(config.WorkspaceAccess(cfg.WorkspaceAccess)))
cfg.WorkspaceRoot = strings.TrimSpace(cfg.WorkspaceRoot)
sb := &ContainerSandbox{cfg: cfg}
sb.hash = computeContainerConfigHash(cfg)
@ -125,7 +126,7 @@ func (c *ContainerSandbox) Start(ctx context.Context) error {
c.startErr = err
return err
}
if strings.TrimSpace(c.cfg.Workspace) != "" && c.cfg.WorkspaceAccess == "none" {
if strings.TrimSpace(c.cfg.Workspace) != "" && c.cfg.WorkspaceAccess == string(config.WorkspaceAccessNone) {
if err := os.MkdirAll(c.cfg.Workspace, 0o755); err != nil {
c.startErr = fmt.Errorf("sandbox workspace init failed: %w", err)
return c.startErr
@ -255,6 +256,13 @@ func (c *ContainerSandbox) ExecStream(
if err != nil {
return nil, fmt.Errorf("docker exec attach failed: %w", err)
}
// Prevent stdcopy.StdCopy from blocking indefinitely if the container hangs.
// We force close the hijacked connection when the context times out.
go func() {
<-execCtx.Done()
attach.Close()
}()
defer attach.Close()
var stdout, stderr bytes.Buffer
@ -404,16 +412,16 @@ func (c *ContainerSandbox) binds() []string {
}
if hostDir != "" {
if c.cfg.WorkspaceAccess == "none" {
if c.cfg.WorkspaceAccess == string(config.WorkspaceAccessNone) {
// Ensure the isolated directory exists on the host so Docker doesn't create it as root
_ = os.MkdirAll(hostDir, 0o755)
}
// Add :Z flag for SELinux (Podman) to label the content with a private unshared label.
// This fixes errors like: "crun: getcwd: Operation not permitted: OCI permission denied"
switch c.cfg.WorkspaceAccess {
case "ro":
switch config.WorkspaceAccess(c.cfg.WorkspaceAccess) {
case config.WorkspaceAccessRO:
binds = append(binds, fmt.Sprintf("%s:%s:ro,Z", hostDir, c.cfg.Workdir))
case "rw", "none":
case config.WorkspaceAccessRW, config.WorkspaceAccessNone:
binds = append(binds, fmt.Sprintf("%s:%s:rw,Z", hostDir, c.cfg.Workdir))
default:
// Default to no mount for unknown access types
@ -574,7 +582,7 @@ func (f *containerFS) ReadFile(ctx context.Context, p string) ([]byte, error) {
return content, nil
}
}
return nil, fmt.Errorf("file not found in container: %s", containerPath)
return nil, fmt.Errorf("file not found in container %s: %w", containerPath, fs.ErrNotExist)
}
func (f *containerFS) WriteFile(ctx context.Context, p string, data []byte, mkdir bool) error {
@ -630,8 +638,94 @@ func (f *containerFS) WriteFile(ctx context.Context, p string, data []byte, mkdi
return nil
}
func (f *containerFS) ReadDir(ctx context.Context, p string) ([]os.DirEntry, error) {
if err := f.sb.ensureContainer(ctx); err != nil {
return nil, err
}
containerPath, err := resolveContainerPathWithRoot(f.sb.cfg.Workdir, p)
if err != nil {
return nil, err
}
rc, _, err := f.sb.cli.CopyFromContainer(ctx, f.sb.cfg.ContainerName, containerPath)
if err != nil {
return nil, fmt.Errorf("docker copy from container failed: %w", err)
}
defer rc.Close()
var entries []os.DirEntry
tr := tar.NewReader(rc)
entries, err = parseTopLevelDirEntriesFromTar(tr)
if err != nil {
return nil, err
}
return entries, nil
}
func parseTopLevelDirEntriesFromTar(tr *tar.Reader) ([]os.DirEntry, error) {
var entries []os.DirEntry
seen := make(map[string]struct{})
rootName := ""
first := true
for {
hdr, err := tr.Next()
if err == io.EOF {
break
}
if err != nil {
return nil, fmt.Errorf("tar read failed: %w", err)
}
cleanName := path.Clean(hdr.Name)
if first {
first = false
rootName = cleanName
continue
}
relName := cleanName
if rootName != "" && rootName != "." {
if relName == rootName {
continue
}
prefix := rootName + "/"
relName = strings.TrimPrefix(relName, prefix)
}
relName = strings.TrimPrefix(relName, "./")
if relName == "" || relName == "." {
continue
}
if strings.Contains(relName, "/") {
// CopyFromContainer is recursive; keep only immediate children.
continue
}
if _, ok := seen[relName]; ok {
continue
}
seen[relName] = struct{}{}
entries = append(entries, &containerDirEntry{
name: relName,
info: hdr.FileInfo(),
})
}
return entries, nil
}
type containerDirEntry struct {
name string
info os.FileInfo
}
func (d *containerDirEntry) Name() string { return d.name }
func (d *containerDirEntry) IsDir() bool { return d.info.IsDir() }
func (d *containerDirEntry) Type() os.FileMode { return d.info.Mode().Type() }
func (d *containerDirEntry) Info() (os.FileInfo, error) { return d.info, nil }
func (c *ContainerSandbox) hostDirForContainerPath(containerDir string) (string, bool) {
if c.cfg.WorkspaceAccess == "ro" {
if c.cfg.WorkspaceAccess == string(config.WorkspaceAccessRO) {
return "", false
}
workspace := strings.TrimSpace(c.cfg.Workspace)

61
pkg/agent/sandbox/container_integration_test.go
View file

@ -103,6 +103,22 @@ func TestContainerSandbox_Integration_ExecReadWrite(t *testing.T) {
if strings.TrimSpace(pwdRes.Stdout) != "/workspace/it" {
t.Fatalf("pwd mismatch: got %q want %q", strings.TrimSpace(pwdRes.Stdout), "/workspace/it")
}
// Test ReadDir
entries, err := sb.Fs().ReadDir(ctx, "it")
if err != nil {
t.Fatalf("ReadDir failed: %v", err)
}
found := false
for _, e := range entries {
if e.Name() == "write.txt" {
found = true
break
}
}
if !found {
t.Errorf("ReadDir result missing 'write.txt'")
}
}
func TestContainerSandbox_Integration_WriteFileMkdirInContainerTmp(t *testing.T) {
@ -364,3 +380,48 @@ func TestContainerSandbox_Integration_ExecTimeoutRespectsRequest(t *testing.T) {
t.Fatalf("expected timeout to trigger early, took %v", time.Since(start))
}
}
func TestContainerSandbox_Integration_ExecTimeoutBreaksStdCopyBlock(t *testing.T) {
if os.Getenv("PICOCLAW_RUN_DOCKER_TESTS") != "1" {
t.Skip("set PICOCLAW_RUN_DOCKER_TESTS=1 to run docker integration tests")
}
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
containerName := fmt.Sprintf("picoclaw-test-timeout-block-%d", time.Now().UnixNano())
image := strings.TrimSpace(os.Getenv("PICOCLAW_DOCKER_TEST_IMAGE"))
if image == "" {
image = "debian:bookworm-slim"
}
sb := NewContainerSandbox(ContainerSandboxConfig{
Image: image,
ContainerName: containerName,
Workspace: t.TempDir(),
})
if err := sb.Start(ctx); err != nil {
t.Fatalf("sandbox start failed: %v", err)
}
defer func() {
_ = sb.Prune(context.Background())
if sb.cli != nil {
_ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true})
}
}()
start := time.Now()
// Run a command that sleeps for a very long time holding the stream open.
// We set a 500ms timeout. If StdCopy isn't broken asynchronously, the Exec call will hang.
_, err := sb.Exec(ctx, ExecRequest{
Command: "sh -c 'sleep 1000'",
TimeoutMs: 500,
})
if err == nil {
t.Fatal("expected timeout error for hanging command")
}
elapsed := time.Since(start)
if elapsed > 2*time.Second {
t.Fatalf("expected timeout to trigger within 2s, but it blocked for %v (StdCopy might be hanging)", elapsed)
}
}

44
pkg/agent/sandbox/container_test.go
View file

@ -1,6 +1,8 @@
package sandbox
import (
"archive/tar"
"bytes"
"context"
"errors"
"os"
@ -53,6 +55,48 @@ func TestResolveContainerPath_RejectsAbsoluteOutsideWorkspace(t *testing.T) {
}
}
func TestParseTopLevelDirEntriesFromTar_KeepImmediateChildren(t *testing.T) {
var buf bytes.Buffer
tw := tar.NewWriter(&buf)
writeHeader := func(hdr *tar.Header, content []byte) {
t.Helper()
if err := tw.WriteHeader(hdr); err != nil {
t.Fatalf("write header failed: %v", err)
}
if len(content) > 0 {
if _, err := tw.Write(content); err != nil {
t.Fatalf("write content failed: %v", err)
}
}
}
writeHeader(&tar.Header{Name: "it", Typeflag: tar.TypeDir, Mode: 0o755}, nil)
writeHeader(&tar.Header{Name: "it/write.txt", Typeflag: tar.TypeReg, Mode: 0o644, Size: 1}, []byte("x"))
writeHeader(&tar.Header{Name: "it/sub", Typeflag: tar.TypeDir, Mode: 0o755}, nil)
writeHeader(&tar.Header{Name: "it/sub/nested.txt", Typeflag: tar.TypeReg, Mode: 0o644, Size: 1}, []byte("y"))
if err := tw.Close(); err != nil {
t.Fatalf("tar close failed: %v", err)
}
entries, err := parseTopLevelDirEntriesFromTar(tar.NewReader(&buf))
if err != nil {
t.Fatalf("parseTopLevelDirEntriesFromTar failed: %v", err)
}
got := make(map[string]struct{}, len(entries))
for _, e := range entries {
got[e.Name()] = struct{}{}
}
for _, want := range []string{"write.txt", "sub"} {
if _, ok := got[want]; !ok {
t.Fatalf("missing top-level entry %q in %+v", want, got)
}
}
if _, ok := got["nested.txt"]; ok {
t.Fatalf("nested file should not appear in top-level entries: %+v", got)
}
}
func TestBuildExecCommand_DefaultShell(t *testing.T) {
sb := NewContainerSandbox(ContainerSandboxConfig{})
cmd, wd, err := sb.buildExecCommand(ExecRequest{

112
pkg/agent/sandbox/host.go
View file

@ -5,6 +5,7 @@ import (
"context"
"fmt"
"io"
"io/fs"
"os"
"os/exec"
"path/filepath"
@ -90,7 +91,7 @@ func (h *HostSandbox) ExecStream(
}
if req.WorkingDir != "" {
dir, err := validatePath(req.WorkingDir, h.workspace, h.restrict)
dir, err := ValidatePath(req.WorkingDir, h.workspace, h.restrict)
if err != nil {
return nil, err
}
@ -105,6 +106,9 @@ func (h *HostSandbox) ExecStream(
if err != nil {
return nil, fmt.Errorf("stderr pipe setup failed: %w", err)
}
prepareCommandForTermination(cmd)
if err := cmd.Start(); err != nil {
return nil, err
}
@ -158,9 +162,11 @@ func (h *HostSandbox) ExecStream(
waitErr := cmd.Wait()
if streamErr != nil {
_ = terminateProcessTree(cmd)
return nil, streamErr
}
if cmdCtx.Err() != nil {
_ = terminateProcessTree(cmd)
return nil, cmdCtx.Err()
}
@ -205,7 +211,7 @@ func (h *hostFS) getSafeRelPath(path string) (string, error) {
func (h *hostFS) ReadFile(ctx context.Context, path string) ([]byte, error) {
if !h.restrict || h.workspace == "" || h.root == nil {
// Unrestricted mode continues to use traditional resolution
resolved, err := validatePath(path, h.workspace, h.restrict)
resolved, err := ValidatePath(path, h.workspace, h.restrict)
if err != nil {
return nil, err
}
@ -225,7 +231,7 @@ func (h *hostFS) ReadFile(ctx context.Context, path string) ([]byte, error) {
func (h *hostFS) WriteFile(ctx context.Context, path string, data []byte, mkdir bool) error {
if !h.restrict || h.workspace == "" || h.root == nil {
// Unrestricted mode continues to use traditional resolution
resolved, err := validatePath(path, h.workspace, h.restrict)
resolved, err := ValidatePath(path, h.workspace, h.restrict)
if err != nil {
return err
}
@ -234,7 +240,17 @@ func (h *hostFS) WriteFile(ctx context.Context, path string, data []byte, mkdir
return err
}
}
return os.WriteFile(resolved, data, 0o644)
// Atomic write: write to temp file then rename to prevent partial writes.
tmpPath := fmt.Sprintf("%s.%d.tmp", resolved, time.Now().UnixNano())
if err := os.WriteFile(tmpPath, data, 0o644); err != nil {
os.Remove(tmpPath)
return fmt.Errorf("failed to write temp file: %w", err)
}
if err := os.Rename(tmpPath, resolved); err != nil {
os.Remove(tmpPath)
return fmt.Errorf("failed to replace original file: %w", err)
}
return nil
}
relPath, err := h.getSafeRelPath(path)
@ -248,80 +264,32 @@ func (h *hostFS) WriteFile(ctx context.Context, path string, data []byte, mkdir
return err
}
}
// Uses OS-level guarantees to restrict the file writing within the root descriptor.
return h.root.WriteFile(relPath, data, 0o644)
// Atomic write within os.Root: write to temp file then rename.
tmpRelPath := fmt.Sprintf("%s.%d.tmp", relPath, time.Now().UnixNano())
if err := h.root.WriteFile(tmpRelPath, data, 0o644); err != nil {
h.root.Remove(tmpRelPath)
return fmt.Errorf("failed to write temp file: %w", err)
}
if err := h.root.Rename(tmpRelPath, relPath); err != nil {
h.root.Remove(tmpRelPath)
return fmt.Errorf("failed to rename temp file over target: %w", err)
}
return nil
}
// validatePath ensures the given path is within the workspace if restrict is true but does not ensure atomic TOCTOU protection.
// It is kept for setting string-based fields like cmd.Dir where os.Root cannot be directly mapped.
// The secure file operations boundary relies on os.Root implemented in FsBridge.
// validatePath ensures the given path is within the workspace if restrict is true.
func validatePath(path, workspace string, restrict bool) (string, error) {
if workspace == "" {
return path, nil
}
absWorkspace, err := filepath.Abs(workspace)
if err != nil {
return "", fmt.Errorf("failed to resolve workspace path: %w", err)
}
var absPath string
if filepath.IsAbs(path) {
absPath = filepath.Clean(path)
} else {
absPath, err = filepath.Abs(filepath.Join(absWorkspace, path))
func (h *hostFS) ReadDir(ctx context.Context, path string) ([]os.DirEntry, error) {
if !h.restrict || h.workspace == "" || h.root == nil {
resolved, err := ValidatePath(path, h.workspace, h.restrict)
if err != nil {
return "", fmt.Errorf("failed to resolve file path: %w", err)
return nil, err
}
return os.ReadDir(resolved)
}
if restrict {
if !isWithinWorkspace(absPath, absWorkspace) {
return "", ErrOutsideWorkspace
}
var resolved string
workspaceReal := absWorkspace
if resolved, err = filepath.EvalSymlinks(absWorkspace); err == nil {
workspaceReal = resolved
}
if resolved, err = filepath.EvalSymlinks(absPath); err == nil {
if !isWithinWorkspace(resolved, workspaceReal) {
return "", ErrOutsideWorkspace
}
} else if os.IsNotExist(err) {
var parentResolved string
if parentResolved, err = resolveExistingAncestor(filepath.Dir(absPath)); err == nil {
if !isWithinWorkspace(parentResolved, workspaceReal) {
return "", fmt.Errorf("access denied: symlink resolves outside workspace")
}
} else if !os.IsNotExist(err) {
return "", fmt.Errorf("failed to resolve path: %w", err)
}
} else {
return "", fmt.Errorf("failed to resolve path: %w", err)
}
relPath, err := h.getSafeRelPath(path)
if err != nil {
return nil, err
}
return absPath, nil
}
func resolveExistingAncestor(path string) (string, error) {
for current := filepath.Clean(path); ; current = filepath.Dir(current) {
if resolved, err := filepath.EvalSymlinks(current); err == nil {
return resolved, nil
} else if !os.IsNotExist(err) {
return "", err
}
if filepath.Dir(current) == current {
return "", os.ErrNotExist
}
}
}
func isWithinWorkspace(candidate, workspace string) bool {
rel, err := filepath.Rel(filepath.Clean(workspace), filepath.Clean(candidate))
return err == nil && filepath.IsLocal(rel)
return fs.ReadDir(h.root.FS(), relPath)
}

2
pkg/tools/shell_process_unix.go → pkg/agent/sandbox/host_process_unix.go
View file

@ -1,6 +1,6 @@
//go:build !windows
package tools
package sandbox
import (
"os/exec"

2
pkg/tools/shell_process_windows.go → pkg/agent/sandbox/host_process_windows.go
View file

@ -1,6 +1,6 @@
//go:build windows
package tools
package sandbox
import (
"os/exec"

79
pkg/agent/sandbox/host_test.go
View file

@ -77,7 +77,7 @@ func TestHostSandbox_ExecAndFs(t *testing.T) {
func TestHostSandbox_ResolvePathRestrictions(t *testing.T) {
root := t.TempDir()
got, err := validatePath("a/b.txt", root, true)
got, err := ValidatePath("a/b.txt", root, true)
if err != nil {
t.Fatalf("resolvePath relative error: %v", err)
}
@ -86,7 +86,7 @@ func TestHostSandbox_ResolvePathRestrictions(t *testing.T) {
t.Fatalf("resolvePath relative got %q, want %q", got, want)
}
_, err = validatePath(filepath.Join(root, "..", "outside.txt"), root, true)
_, err = ValidatePath(filepath.Join(root, "..", "outside.txt"), root, true)
if err == nil || !errors.Is(err, ErrOutsideWorkspace) {
t.Fatalf("expected outside workspace error, got: %v", err)
}
@ -97,7 +97,7 @@ func TestHostSandbox_ResolvePathRestrictions(t *testing.T) {
}
link := filepath.Join(root, "link.txt")
if err := os.Symlink(target, link); err == nil {
_, err = validatePath("link.txt", root, true)
_, err = ValidatePath("link.txt", root, true)
if err == nil || !errors.Is(err, ErrOutsideWorkspace) {
t.Fatalf("expected symlink outside error, got: %v", err)
}
@ -158,6 +158,56 @@ func TestHostFS_ReadFileWriteFile_Restricted(t *testing.T) {
}
}
func TestHostFS_ReadDir(t *testing.T) {
root := t.TempDir()
os.MkdirAll(filepath.Join(root, "a/b"), 0o755)
os.WriteFile(filepath.Join(root, "a/f1.txt"), []byte("1"), 0o644)
os.WriteFile(filepath.Join(root, "a/b/f2.txt"), []byte("2"), 0o644)
sb := NewHostSandbox(root, true)
if err := sb.Start(context.Background()); err != nil {
t.Fatal(err)
}
defer sb.Prune(context.Background())
// Test restricted ReadDir on "a"
entries, err := sb.Fs().ReadDir(context.Background(), "a")
if err != nil {
t.Fatalf("ReadDir failed: %v", err)
}
foundF1 := false
foundB := false
for _, e := range entries {
if e.Name() == "f1.txt" && !e.IsDir() {
foundF1 = true
}
if e.Name() == "b" && e.IsDir() {
foundB = true
}
}
if !foundF1 || !foundB {
t.Errorf("ReadDir result missing expected entries: foundF1=%v, foundB=%v", foundF1, foundB)
}
// Test unrestricted ReadDir
sb2 := NewHostSandbox(root, false)
entries2, err := sb2.Fs().ReadDir(context.Background(), root)
if err != nil {
t.Fatalf("ReadDir unrestricted failed: %v", err)
}
foundA := false
for _, e := range entries2 {
if e.Name() == "a" {
foundA = true
break
}
}
if !foundA {
t.Errorf("ReadDir unrestricted missing 'a'")
}
}
func TestHostFS_ReadFileWriteFile_Unrestricted(t *testing.T) {
root := t.TempDir()
sb := NewHostSandbox(root, false)
@ -264,16 +314,13 @@ func TestHostFS_ReadFileWriteFile_WithoutWorkspaceOrRoot(t *testing.T) {
content := []byte("hello empty workspace")
target := filepath.Join(root, "empty.txt")
if err := sb.Fs().WriteFile(context.Background(), target, content, true); err != nil {
t.Fatalf("WriteFile failed: %v", err)
if err := sb.Fs().WriteFile(context.Background(), target, content, true); err == nil {
t.Fatalf("expected WriteFile to fail due to empty workspace with restrict=true")
}
readContent, err := sb.Fs().ReadFile(context.Background(), target)
if err != nil {
t.Fatalf("ReadFile failed: %v", err)
}
if string(readContent) != string(content) {
t.Fatalf("content mismatch")
_, err := sb.Fs().ReadFile(context.Background(), target)
if err == nil {
t.Fatalf("expected ReadFile to fail due to empty workspace with restrict=true")
}
// Test case where root is nil explicitly
@ -284,7 +331,7 @@ func TestHostFS_ReadFileWriteFile_WithoutWorkspaceOrRoot(t *testing.T) {
t.Fatalf("WriteFile failed: %v", err)
}
readContent, err = sb2.Fs().ReadFile(context.Background(), "nil_root_test.txt")
readContent, err := sb2.Fs().ReadFile(context.Background(), "nil_root_test.txt")
if err != nil {
t.Fatalf("ReadFile failed: %v", err)
}
@ -294,9 +341,9 @@ func TestHostFS_ReadFileWriteFile_WithoutWorkspaceOrRoot(t *testing.T) {
}
func TestValidatePathErrors(t *testing.T) {
_, err := validatePath("/a/b/c", "", true)
if err != nil {
t.Fatalf("expected no err for empty workspace with abs path")
_, err := ValidatePath("/a/b/c", "", true)
if err == nil {
t.Fatalf("expected err for empty workspace with restrict=true")
}
root := t.TempDir()
@ -306,7 +353,7 @@ func TestValidatePathErrors(t *testing.T) {
if err != nil {
t.Fatal(err)
}
_, err = validatePath("a.txt/b.txt", root, true)
_, err = ValidatePath("a.txt/b.txt", root, true)
if err == nil {
t.Fatalf("expected error when ancestor is file")
}

117
pkg/agent/sandbox/manager.go
View file

@ -27,11 +27,11 @@ func NewFromConfig(workspace string, restrict bool, cfg *config.Config) Sandbox
}
// NewFromConfigWithAgent builds the sandbox Manager for an agent.
// Returns nil when sandboxing is disabled (mode=off), so callers can check manager != nil.
// It always returns a non-nil Manager (falling back to a host manager or error manager if needed).
func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config, agentID string) Manager {
mode := "all"
scope := "agent"
workspaceAccess := "none"
mode := config.SandboxModeAll
scope := config.SandboxScopeAgent
workspaceAccess := config.WorkspaceAccessNone
workspaceRoot := "~/.picoclaw/sandboxes"
image := "picoclaw-sandbox:bookworm-slim"
containerPrefix := "picoclaw-sandbox-"
@ -41,14 +41,14 @@ func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config,
if cfg != nil {
sb := cfg.Agents.Defaults.Sandbox
if strings.TrimSpace(sb.Mode) != "" {
mode = strings.TrimSpace(sb.Mode)
if sb.Mode != "" {
mode = sb.Mode
}
if strings.TrimSpace(sb.Scope) != "" {
scope = strings.TrimSpace(sb.Scope)
if sb.Scope != "" {
scope = sb.Scope
}
if strings.TrimSpace(sb.WorkspaceAccess) != "" {
workspaceAccess = strings.TrimSpace(sb.WorkspaceAccess)
if sb.WorkspaceAccess != "" {
workspaceAccess = sb.WorkspaceAccess
}
if strings.TrimSpace(sb.WorkspaceRoot) != "" {
workspaceRoot = strings.TrimSpace(sb.WorkspaceRoot)
@ -71,13 +71,14 @@ func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config,
agentID = routing.NormalizeAgentID(agentID)
resolvedMode := normalizeSandboxMode(mode)
if resolvedMode == "off" {
return nil // sandbox disabled; host-level access is handled directly by tools
}
host := NewHostSandbox(workspace, restrict)
_ = host.Start(context.Background())
// When sandbox is disabled, skip building container infrastructure entirely.
if resolvedMode == config.SandboxModeOff {
return &hostOnlyManager{host: host}
}
resolvedScope := normalizeSandboxScope(scope)
normalizedAccess := normalizeWorkspaceAccess(workspaceAccess)
workspaceRootAbs := resolveAbsPath(expandHomePath(workspaceRoot))
@ -105,31 +106,33 @@ func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config,
return manager
}
func normalizeWorkspaceAccess(access string) string {
v := strings.ToLower(strings.TrimSpace(access))
func normalizeWorkspaceAccess(access config.WorkspaceAccess) config.WorkspaceAccess {
v := config.WorkspaceAccess(strings.ToLower(strings.TrimSpace(string(access))))
switch v {
case "ro", "rw":
case config.WorkspaceAccessRO, config.WorkspaceAccessRW:
return v
default:
return "none"
return config.WorkspaceAccessNone
}
}
func normalizeSandboxMode(mode string) string {
switch strings.ToLower(strings.TrimSpace(mode)) {
case "all", "non-main":
return strings.ToLower(strings.TrimSpace(mode))
func normalizeSandboxMode(mode config.SandboxMode) config.SandboxMode {
v := config.SandboxMode(strings.ToLower(strings.TrimSpace(string(mode))))
switch v {
case config.SandboxModeAll, config.SandboxModeNonMain:
return v
default:
return "off"
return config.SandboxModeOff
}
}
func normalizeSandboxScope(scope string) string {
switch strings.ToLower(strings.TrimSpace(scope)) {
case "session", "shared":
return strings.ToLower(strings.TrimSpace(scope))
func normalizeSandboxScope(scope config.SandboxScope) config.SandboxScope {
v := config.SandboxScope(strings.ToLower(strings.TrimSpace(string(scope))))
switch v {
case config.SandboxScopeSession, config.SandboxScopeShared:
return v
default:
return "agent"
return config.SandboxScopeAgent
}
}
@ -165,13 +168,13 @@ func resolveAbsPath(p string) string {
}
type scopedSandboxManager struct {
mode string
scope string
mode config.SandboxMode
scope config.SandboxScope
agentID string
host Sandbox
image string
containerPrefix string
workspaceAccess string
workspaceAccess config.WorkspaceAccess
workspaceRoot string
agentWorkspace string
pruneIdleHours int
@ -188,7 +191,7 @@ type scopedSandboxManager struct {
}
func (m *scopedSandboxManager) Start(ctx context.Context) error {
if m.mode == "off" {
if m.mode == config.SandboxModeOff {
return nil
}
if _, err := m.getOrCreateSandbox(ctx, m.defaultScopeKey()); err != nil {
@ -361,9 +364,9 @@ func (m *scopedSandboxManager) Resolve(ctx context.Context) (Sandbox, error) {
func (m *scopedSandboxManager) shouldSandbox(ctx context.Context) bool {
switch m.mode {
case "all":
case config.SandboxModeAll:
return true
case "non-main":
case config.SandboxModeNonMain:
// Sandbox all sessions except the agent's main session.
// Normalize before comparing to handle aliases like "main" or bare agent keys
return m.normalizeSessionKey(SessionKeyFromContext(ctx)) != m.mainSessionKey()
@ -397,9 +400,9 @@ func (m *scopedSandboxManager) normalizeSessionKey(raw string) string {
func (m *scopedSandboxManager) scopeKeyFromContext(ctx context.Context) string {
sessionKey := m.normalizeSessionKey(SessionKeyFromContext(ctx))
switch m.scope {
case "shared":
case config.SandboxScopeShared:
return "shared"
case "session":
case config.SandboxScopeSession:
return sessionKey
default:
if parsed := routing.ParseAgentSessionKey(sessionKey); parsed != nil {
@ -434,7 +437,7 @@ func (m *scopedSandboxManager) getOrCreateSandbox(ctx context.Context, scopeKey
func (m *scopedSandboxManager) buildScopedContainerSandbox(scopeKey string) Sandbox {
workspace := m.agentWorkspace
if m.workspaceAccess == "none" || strings.TrimSpace(workspace) == "" {
if m.workspaceAccess == config.WorkspaceAccessNone || strings.TrimSpace(workspace) == "" {
workspace = filepath.Join(m.workspaceRoot, slugScopeKey(scopeKey), "workspace")
}
return NewContainerSandbox(ContainerSandboxConfig{
@ -443,7 +446,7 @@ func (m *scopedSandboxManager) buildScopedContainerSandbox(scopeKey string) Sand
ContainerPrefix: m.containerPrefix,
Workspace: workspace,
AgentWorkspace: m.agentWorkspace,
WorkspaceAccess: m.workspaceAccess,
WorkspaceAccess: string(m.workspaceAccess),
WorkspaceRoot: m.workspaceRoot,
PruneIdleHours: m.pruneIdleHours,
PruneMaxAgeDays: m.pruneMaxAgeDays,
@ -494,6 +497,17 @@ func (f *managerFS) WriteFile(ctx context.Context, path string, data []byte, mkd
return sb.Fs().WriteFile(ctx, path, data, mkdir)
}
func (f *managerFS) ReadDir(ctx context.Context, path string) ([]os.DirEntry, error) {
if !f.m.shouldSandbox(ctx) {
return f.m.host.Fs().ReadDir(ctx, path)
}
sb, err := f.m.getOrCreateSandbox(ctx, f.m.scopeKeyFromContext(ctx))
if err != nil {
return nil, err
}
return sb.Fs().ReadDir(ctx, path)
}
var nonAlnum = regexp.MustCompile(`[^a-z0-9._-]+`)
func slugScopeKey(scopeKey string) string {
@ -513,6 +527,29 @@ func slugScopeKey(scopeKey string) string {
return safe + "-" + hex.EncodeToString(sum[:4])
}
// hostOnlyManager is a lightweight Manager used when sandbox mode is "off".
// It delegates all operations directly to the HostSandbox, avoiding
// unnecessary container infrastructure setup.
type hostOnlyManager struct {
host Sandbox
}
func (h *hostOnlyManager) Start(ctx context.Context) error { return nil }
func (h *hostOnlyManager) Prune(ctx context.Context) error { return h.host.Prune(ctx) }
func (h *hostOnlyManager) Resolve(ctx context.Context) (Sandbox, error) { return h.host, nil }
func (h *hostOnlyManager) Fs() FsBridge { return h.host.Fs() }
func (h *hostOnlyManager) Exec(ctx context.Context, req ExecRequest) (*ExecResult, error) {
return h.host.Exec(ctx, req)
}
func (h *hostOnlyManager) ExecStream(
ctx context.Context,
req ExecRequest,
onEvent func(ExecEvent) error,
) (*ExecResult, error) {
return h.host.ExecStream(ctx, req, onEvent)
}
type unavailableSandboxManager struct {
err error
fs FsBridge
@ -562,3 +599,7 @@ func (e *errorFS) ReadFile(ctx context.Context, path string) ([]byte, error) {
func (e *errorFS) WriteFile(ctx context.Context, path string, data []byte, mkdir bool) error {
return fmt.Errorf("sandbox unavailable: %w", e.err)
}
func (e *errorFS) ReadDir(ctx context.Context, path string) ([]os.DirEntry, error) {
return nil, fmt.Errorf("sandbox unavailable: %w", e.err)
}

78
pkg/agent/sandbox/path.go Normal file
View file

@ -0,0 +1,78 @@
package sandbox
import (
"fmt"
"os"
"path/filepath"
)
// ValidatePath ensures the given path is within the workspace if restrict is true.
func ValidatePath(path, workspace string, restrict bool) (string, error) {
if workspace == "" {
return "", fmt.Errorf("workspace is not defined")
}
absWorkspace, err := filepath.Abs(workspace)
if err != nil {
return "", fmt.Errorf("failed to resolve workspace path: %w", err)
}
var absPath string
if filepath.IsAbs(path) {
absPath = filepath.Clean(path)
} else {
absPath, err = filepath.Abs(filepath.Join(absWorkspace, path))
if err != nil {
return "", fmt.Errorf("failed to resolve file path: %w", err)
}
}
if restrict {
if !isWithinWorkspace(absPath, absWorkspace) {
return "", ErrOutsideWorkspace
}
var resolved string
workspaceReal := absWorkspace
if resolved, err = filepath.EvalSymlinks(absWorkspace); err == nil {
workspaceReal = resolved
}
if resolved, err = filepath.EvalSymlinks(absPath); err == nil {
if !isWithinWorkspace(resolved, workspaceReal) {
return "", ErrOutsideWorkspace
}
} else if os.IsNotExist(err) {
var parentResolved string
if parentResolved, err = resolveExistingAncestor(filepath.Dir(absPath)); err == nil {
if !isWithinWorkspace(parentResolved, workspaceReal) {
return "", fmt.Errorf("access denied: symlink resolves outside workspace")
}
} else if !os.IsNotExist(err) {
return "", fmt.Errorf("failed to resolve path: %w", err)
}
} else {
return "", fmt.Errorf("failed to resolve path: %w", err)
}
}
return absPath, nil
}
func resolveExistingAncestor(path string) (string, error) {
for current := filepath.Clean(path); ; current = filepath.Dir(current) {
if resolved, err := filepath.EvalSymlinks(current); err == nil {
return resolved, nil
} else if !os.IsNotExist(err) {
return "", err
}
if filepath.Dir(current) == current {
return "", os.ErrNotExist
}
}
}
func isWithinWorkspace(candidate, workspace string) bool {
rel, err := filepath.Rel(filepath.Clean(workspace), filepath.Clean(candidate))
return err == nil && filepath.IsLocal(rel)
}

40
pkg/agent/sandbox/sandbox.go
View file

@ -2,7 +2,10 @@ package sandbox
import (
"context"
"os"
"strings"
"github.com/sipeed/picoclaw/pkg/logger"
)
// Sandbox abstracts command execution and filesystem access.
@ -97,12 +100,41 @@ func WithSandbox(ctx context.Context, sb Sandbox) context.Context {
return context.WithValue(ctx, sandboxContextKey{}, sb)
}
// SandboxFromContext returns the sandbox instance attached by WithSandbox.
func SandboxFromContext(ctx context.Context) Sandbox {
// FromContext returns the sandbox instance attached by WithSandbox.
// If no pre-resolved sandbox exists, it attempts to resolve one via the Manager in context.
func FromContext(ctx context.Context) Sandbox {
if ctx == nil {
return nil
}
v, _ := ctx.Value(sandboxContextKey{}).(Sandbox)
// 1. Try pre-resolved sandbox
if v, ok := ctx.Value(sandboxContextKey{}).(Sandbox); ok && v != nil {
return v
}
// 2. Try on-demand resolution via Manager
if m := managerFromContext(ctx); m != nil {
sb, err := m.Resolve(ctx)
if err != nil {
logger.WarnCF("sandbox", "FromContext: manager.Resolve failed", map[string]any{"error": err.Error()})
} else if sb != nil {
return sb
}
}
return nil
}
type managerContextKey struct{}
// WithManager returns a derived context carrying the sandbox manager.
func WithManager(ctx context.Context, m Manager) context.Context {
return context.WithValue(ctx, managerContextKey{}, m)
}
// managerFromContext returns the manager attached by WithManager.
func managerFromContext(ctx context.Context) Manager {
if ctx == nil {
return nil
}
v, _ := ctx.Value(managerContextKey{}).(Manager)
return v
}
@ -113,6 +145,8 @@ type FsBridge interface {
// WriteFile writes data to a sandbox-visible path.
// When mkdir is true, missing parent directories should be created.
WriteFile(ctx context.Context, path string, data []byte, mkdir bool) error
// ReadDir reads the named directory and returns a list of directory entries.
ReadDir(ctx context.Context, path string) ([]os.DirEntry, error)
}
func aggregateExecStream(execFn func(onEvent func(ExecEvent) error) (*ExecResult, error)) (*ExecResult, error) {

33
pkg/config/config.go
View file

@ -539,10 +539,37 @@ type AgentSandboxDockerConfig struct {
Binds []string `json:"binds" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_DOCKER_BINDS"`
}
// SandboxMode defines the operational mode of the agent sandbox.
type SandboxMode string
const (
SandboxModeOff SandboxMode = "off" // Sandbox disabled (host execution)
SandboxModeNonMain SandboxMode = "non-main" // Sandbox all sessions except main
SandboxModeAll SandboxMode = "all" // Sandbox all sessions
)
// SandboxScope defines the isolation scope of the sandbox container.
type SandboxScope string
const (
SandboxScopeSession SandboxScope = "session" // One container per session
SandboxScopeAgent SandboxScope = "agent" // One container per agent (shared across sessions)
SandboxScopeShared SandboxScope = "shared" // One container shared by all agents
)
// WorkspaceAccess defines how the agent workspace is exposed to the sandbox.
type WorkspaceAccess string
const (
WorkspaceAccessNone WorkspaceAccess = "none" // No workspace access
WorkspaceAccessRO WorkspaceAccess = "ro" // Read-only access
WorkspaceAccessRW WorkspaceAccess = "rw" // Read-write access
)
type AgentSandboxConfig struct {
Mode string `json:"mode" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_MODE"`
Scope string `json:"scope" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_SCOPE"`
WorkspaceAccess string `json:"workspace_access" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_WORKSPACE_ACCESS"`
Mode SandboxMode `json:"mode" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_MODE"`
Scope SandboxScope `json:"scope" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_SCOPE"`
WorkspaceAccess WorkspaceAccess `json:"workspace_access" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_WORKSPACE_ACCESS"`
WorkspaceRoot string `json:"workspace_root" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_WORKSPACE_ROOT"`
Docker AgentSandboxDockerConfig `json:"docker" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_DOCKER"`
Prune AgentSandboxPruneConfig `json:"prune" env:"PICOCLAW_AGENTS_DEFAULTS_SANDBOX_PRUNE"`

6
pkg/config/defaults.go
View file

@ -18,9 +18,9 @@ func DefaultConfig() *Config {
Temperature: nil, // nil means use provider default
MaxToolIterations: 20,
Sandbox: AgentSandboxConfig{
Mode: "off",
Scope: "agent",
WorkspaceAccess: "none",
Mode: SandboxModeOff,
Scope: SandboxScopeAgent,
WorkspaceAccess: WorkspaceAccessNone,
WorkspaceRoot: "~/.picoclaw/sandboxes",
Docker: AgentSandboxDockerConfig{
Image: "picoclaw-sandbox:bookworm-slim",

129
pkg/tools/edit.go
View file

@ -2,9 +2,8 @@ package tools
import (
"context"
"errors"
"fmt"
"io/fs"
"os"
"strings"
"github.com/sipeed/picoclaw/pkg/agent/sandbox"
@ -12,19 +11,13 @@ import (
// EditFileTool edits a file by replacing old_text with new_text.
// The old_text must exist exactly in the file.
type EditFileTool struct {
fs fileSystem
}
type EditFileTool struct{}
// NewEditFileTool creates a new EditFileTool with optional directory restriction.
// NewEditFileTool creates a new EditFileTool.
// The workspace and restrict parameters are kept for compatibility with tool registry signatures
// but are no longer used since filesystem access is entirely delegated to the Sandbox context.
func NewEditFileTool(workspace string, restrict bool) *EditFileTool {
var fs fileSystem
if restrict {
fs = &sandboxFs{workspace: workspace}
} else {
fs = &hostFs{}
}
return &EditFileTool{fs: fs}
return &EditFileTool{}
}
func (t *EditFileTool) Name() string {
@ -72,51 +65,35 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
return ErrorResult("new_text is required")
}
sb := sandbox.SandboxFromContext(ctx)
if sb != nil {
content, err := sb.Fs().ReadFile(ctx, path)
if err != nil {
return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
}
contentStr := string(content)
if !strings.Contains(contentStr, oldText) {
return ErrorResult("old_text not found in file. Make sure it matches exactly")
}
count := strings.Count(contentStr, oldText)
if count > 1 {
return ErrorResult(
fmt.Sprintf("old_text appears %d times. Please provide more context to make it unique", count),
)
}
newContent := strings.Replace(contentStr, oldText, newText, 1)
err = sb.Fs().WriteFile(ctx, path, []byte(newContent), true)
if err != nil {
return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
}
return SilentResult(fmt.Sprintf("File edited: %s", path))
sb := sandbox.FromContext(ctx)
if sb == nil {
return ErrorResult("sandbox environment unavailable")
}
if err := editFile(t.fs, path, oldText, newText); err != nil {
content, err := sb.Fs().ReadFile(ctx, path)
if err != nil {
return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
}
newContent, err := replaceEditContent(content, oldText, newText)
if err != nil {
return ErrorResult(err.Error())
}
err = sb.Fs().WriteFile(ctx, path, newContent, true)
if err != nil {
return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
}
return SilentResult(fmt.Sprintf("File edited: %s", path))
}
type AppendFileTool struct {
fs fileSystem
}
type AppendFileTool struct{}
// NewAppendFileTool creates a new AppendFileTool.
// The workspace and restrict parameters are kept for compatibility with tool registry signatures
// but are no longer used since filesystem access is entirely delegated to the Sandbox context.
func NewAppendFileTool(workspace string, restrict bool) *AppendFileTool {
var fs fileSystem
if restrict {
fs = &sandboxFs{workspace: workspace}
} else {
fs = &hostFs{}
}
return &AppendFileTool{fs: fs}
return &AppendFileTool{}
}
func (t *AppendFileTool) Name() string {
@ -155,54 +132,24 @@ func (t *AppendFileTool) Execute(ctx context.Context, args map[string]any) *Tool
return ErrorResult("content is required")
}
sb := sandbox.SandboxFromContext(ctx)
if sb != nil {
// Implement Append using Read + Write if no Append in FsBridge
oldContent, err := sb.Fs().ReadFile(ctx, path)
if err != nil && !strings.Contains(err.Error(), "no such file") {
return ErrorResult(fmt.Sprintf("failed to read file for append: %v", err))
}
newContent := string(oldContent) + content
err = sb.Fs().WriteFile(ctx, path, []byte(newContent), true)
if err != nil {
return ErrorResult(fmt.Sprintf("failed to append (write) to file: %v", err))
}
return SilentResult(fmt.Sprintf("Appended to %s", path))
sb := sandbox.FromContext(ctx)
if sb == nil {
return ErrorResult("sandbox environment unavailable")
}
if err := appendFile(t.fs, path, content); err != nil {
return ErrorResult(err.Error())
// Implement Append using Read + Write
oldContent, err := sb.Fs().ReadFile(ctx, path)
if err != nil && !os.IsNotExist(err) && !strings.Contains(err.Error(), "not found") {
return ErrorResult(fmt.Sprintf("failed to read file for append: %v", err))
}
newContent := append(oldContent, []byte(content)...)
err = sb.Fs().WriteFile(ctx, path, newContent, true)
if err != nil {
return ErrorResult(fmt.Sprintf("failed to append (write) to file: %v", err))
}
return SilentResult(fmt.Sprintf("Appended to %s", path))
}
// editFile reads the file via sysFs, performs the replacement, and writes back.
// It uses a fileSystem interface, allowing the same logic for both restricted and unrestricted modes.
func editFile(sysFs fileSystem, path, oldText, newText string) error {
content, err := sysFs.ReadFile(path)
if err != nil {
return err
}
newContent, err := replaceEditContent(content, oldText, newText)
if err != nil {
return err
}
return sysFs.WriteFile(path, newContent)
}
// appendFile reads the existing content (if any) via sysFs, appends new content, and writes back.
func appendFile(sysFs fileSystem, path, appendContent string) error {
content, err := sysFs.ReadFile(path)
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return err
}
newContent := append(content, []byte(appendContent)...)
return sysFs.WriteFile(path, newContent)
}
// replaceEditContent handles the core logic of finding and replacing a single occurrence of oldText.
func replaceEditContent(content []byte, oldText, newText string) ([]byte, error) {
contentStr := string(content)

63
pkg/tools/edit_test.go
View file

@ -8,6 +8,8 @@ import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/sipeed/picoclaw/pkg/agent/sandbox"
)
// TestEditTool_EditFile_Success verifies successful file editing
@ -17,7 +19,8 @@ func TestEditTool_EditFile_Success(t *testing.T) {
os.WriteFile(testFile, []byte("Hello World\nThis is a test"), 0o644)
tool := NewEditFileTool(tmpDir, true)
ctx := context.Background()
sb := sandbox.NewHostSandbox(tmpDir, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": testFile,
"old_text": "World",
@ -61,7 +64,8 @@ func TestEditTool_EditFile_NotFound(t *testing.T) {
testFile := filepath.Join(tmpDir, "nonexistent.txt")
tool := NewEditFileTool(tmpDir, true)
ctx := context.Background()
sb := sandbox.NewHostSandbox(tmpDir, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": testFile,
"old_text": "old",
@ -75,10 +79,9 @@ func TestEditTool_EditFile_NotFound(t *testing.T) {
t.Errorf("Expected error for non-existent file")
}
// Should mention file not found
if !strings.Contains(result.ForLLM, "not found") && !strings.Contains(result.ForUser, "not found") {
t.Errorf("Expected 'file not found' message, got ForLLM: %s", result.ForLLM)
}
// Should mention file not found or no such file
assert.True(t, strings.Contains(result.ForLLM, "not found") || strings.Contains(result.ForLLM, "no such file"),
"Expected 'not found' or 'no such file' message, got ForLLM: %s", result.ForLLM)
}
// TestEditTool_EditFile_OldTextNotFound verifies error when old_text doesn't exist
@ -88,7 +91,8 @@ func TestEditTool_EditFile_OldTextNotFound(t *testing.T) {
os.WriteFile(testFile, []byte("Hello World"), 0o644)
tool := NewEditFileTool(tmpDir, true)
ctx := context.Background()
sb := sandbox.NewHostSandbox(tmpDir, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": testFile,
"old_text": "Goodbye",
@ -115,7 +119,8 @@ func TestEditTool_EditFile_MultipleMatches(t *testing.T) {
os.WriteFile(testFile, []byte("test test test"), 0o644)
tool := NewEditFileTool(tmpDir, true)
ctx := context.Background()
sb := sandbox.NewHostSandbox(tmpDir, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": testFile,
"old_text": "test",
@ -143,7 +148,8 @@ func TestEditTool_EditFile_OutsideAllowedDir(t *testing.T) {
os.WriteFile(testFile, []byte("content"), 0o644)
tool := NewEditFileTool(tmpDir, true) // Restrict to tmpDir
ctx := context.Background()
sb := sandbox.NewHostSandbox(tmpDir, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": testFile,
"old_text": "content",
@ -169,8 +175,9 @@ func TestEditTool_EditFile_OutsideAllowedDir(t *testing.T) {
// TestEditTool_EditFile_MissingPath verifies error handling for missing path
func TestEditTool_EditFile_MissingPath(t *testing.T) {
tool := NewEditFileTool("", false)
ctx := context.Background()
tool := NewEditFileTool("/", false)
sb := sandbox.NewHostSandbox("/", false)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"old_text": "old",
"new_text": "new",
@ -186,8 +193,9 @@ func TestEditTool_EditFile_MissingPath(t *testing.T) {
// TestEditTool_EditFile_MissingOldText verifies error handling for missing old_text
func TestEditTool_EditFile_MissingOldText(t *testing.T) {
tool := NewEditFileTool("", false)
ctx := context.Background()
tool := NewEditFileTool("/", false)
sb := sandbox.NewHostSandbox("/", false)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": "/tmp/test.txt",
"new_text": "new",
@ -203,8 +211,9 @@ func TestEditTool_EditFile_MissingOldText(t *testing.T) {
// TestEditTool_EditFile_MissingNewText verifies error handling for missing new_text
func TestEditTool_EditFile_MissingNewText(t *testing.T) {
tool := NewEditFileTool("", false)
ctx := context.Background()
tool := NewEditFileTool("/", false)
sb := sandbox.NewHostSandbox("/", false)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": "/tmp/test.txt",
"old_text": "old",
@ -224,8 +233,9 @@ func TestEditTool_AppendFile_Success(t *testing.T) {
testFile := filepath.Join(tmpDir, "test.txt")
os.WriteFile(testFile, []byte("Initial content"), 0o644)
tool := NewAppendFileTool("", false)
ctx := context.Background()
tool := NewAppendFileTool("/", false)
sb := sandbox.NewHostSandbox("/", false)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": testFile,
"content": "\nAppended content",
@ -264,7 +274,7 @@ func TestEditTool_AppendFile_Success(t *testing.T) {
// TestEditTool_AppendFile_MissingPath verifies error handling for missing path
func TestEditTool_AppendFile_MissingPath(t *testing.T) {
tool := NewAppendFileTool("", false)
tool := NewAppendFileTool("/", false)
ctx := context.Background()
args := map[string]any{
"content": "test",
@ -280,7 +290,7 @@ func TestEditTool_AppendFile_MissingPath(t *testing.T) {
// TestEditTool_AppendFile_MissingContent verifies error handling for missing content
func TestEditTool_AppendFile_MissingContent(t *testing.T) {
tool := NewAppendFileTool("", false)
tool := NewAppendFileTool("/", false)
ctx := context.Background()
args := map[string]any{
"path": "/tmp/test.txt",
@ -349,7 +359,8 @@ func TestReplaceEditContent(t *testing.T) {
func TestAppendFileTool_AppendToNonExistent_Restricted(t *testing.T) {
workspace := t.TempDir()
tool := NewAppendFileTool(workspace, true)
ctx := context.Background()
sb := sandbox.NewHostSandbox(workspace, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": "brand_new_file.txt",
@ -379,7 +390,8 @@ func TestAppendFileTool_Restricted_Success(t *testing.T) {
assert.NoError(t, err)
tool := NewAppendFileTool(workspace, true)
ctx := context.Background()
sb := sandbox.NewHostSandbox(workspace, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": testFile,
"content": " appended",
@ -403,7 +415,8 @@ func TestEditFileTool_Restricted_InPlaceEdit(t *testing.T) {
assert.NoError(t, err)
tool := NewEditFileTool(workspace, true)
ctx := context.Background()
sb := sandbox.NewHostSandbox(workspace, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": testFile,
"old_text": "World",
@ -424,7 +437,8 @@ func TestEditFileTool_Restricted_InPlaceEdit(t *testing.T) {
func TestEditFileTool_Restricted_FileNotFound(t *testing.T) {
workspace := t.TempDir()
tool := NewEditFileTool(workspace, true)
ctx := context.Background()
sb := sandbox.NewHostSandbox(workspace, true)
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
"path": "no_such_file.txt",
"old_text": "old",
@ -433,5 +447,6 @@ func TestEditFileTool_Restricted_FileNotFound(t *testing.T) {
result := tool.Execute(ctx, args)
assert.True(t, result.IsError)
assert.Contains(t, result.ForLLM, "not found")
assert.True(t, strings.Contains(result.ForLLM, "not found") || strings.Contains(result.ForLLM, "no such file"),
"Expected 'not found' or 'no such file' message, got ForLLM: %s", result.ForLLM)
}

308
pkg/tools/filesystem.go
View file

@ -3,98 +3,19 @@ package tools
import (
"context"
"fmt"
"io/fs"
"os"
"path/filepath"
"strings"
"time"
"github.com/sipeed/picoclaw/pkg/agent/sandbox"
)
// validatePath ensures the given path is within the workspace if restrict is true.
func validatePath(path, workspace string, restrict bool) (string, error) {
if workspace == "" {
return path, fmt.Errorf("workspace is not defined")
}
absWorkspace, err := filepath.Abs(workspace)
if err != nil {
return "", fmt.Errorf("failed to resolve workspace path: %w", err)
}
var absPath string
if filepath.IsAbs(path) {
absPath = filepath.Clean(path)
} else {
absPath, err = filepath.Abs(filepath.Join(absWorkspace, path))
if err != nil {
return "", fmt.Errorf("failed to resolve file path: %w", err)
}
}
if restrict {
if !isWithinWorkspace(absPath, absWorkspace) {
return "", fmt.Errorf("access denied: path is outside the workspace")
}
var resolved string
workspaceReal := absWorkspace
if resolved, err = filepath.EvalSymlinks(absWorkspace); err == nil {
workspaceReal = resolved
}
if resolved, err = filepath.EvalSymlinks(absPath); err == nil {
if !isWithinWorkspace(resolved, workspaceReal) {
return "", fmt.Errorf("access denied: symlink resolves outside workspace")
}
} else if os.IsNotExist(err) {
var parentResolved string
if parentResolved, err = resolveExistingAncestor(filepath.Dir(absPath)); err == nil {
if !isWithinWorkspace(parentResolved, workspaceReal) {
return "", fmt.Errorf("access denied: symlink resolves outside workspace")
}
} else if !os.IsNotExist(err) {
return "", fmt.Errorf("failed to resolve path: %w", err)
}
} else {
return "", fmt.Errorf("failed to resolve path: %w", err)
}
}
return absPath, nil
}
func resolveExistingAncestor(path string) (string, error) {
for current := filepath.Clean(path); ; current = filepath.Dir(current) {
if resolved, err := filepath.EvalSymlinks(current); err == nil {
return resolved, nil
} else if !os.IsNotExist(err) {
return "", err
}
if filepath.Dir(current) == current {
return "", os.ErrNotExist
}
}
}
func isWithinWorkspace(candidate, workspace string) bool {
rel, err := filepath.Rel(filepath.Clean(workspace), filepath.Clean(candidate))
return err == nil && filepath.IsLocal(rel)
}
type ReadFileTool struct {
fs fileSystem
}
type ReadFileTool struct{}
// NewReadFileTool creates a new ReadFileTool.
// The workspace and restrict parameters are kept for compatibility with tool registry signatures
// but are no longer used since filesystem access is entirely delegated to the Sandbox context.
func NewReadFileTool(workspace string, restrict bool) *ReadFileTool {
var fs fileSystem
if restrict {
fs = &sandboxFs{workspace: workspace}
} else {
fs = &hostFs{}
}
return &ReadFileTool{fs: fs}
return &ReadFileTool{}
}
func (t *ReadFileTool) Name() string {
@ -124,34 +45,22 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
return ErrorResult("path is required")
}
sb := sandbox.SandboxFromContext(ctx)
if sb != nil {
content, err := sb.Fs().ReadFile(ctx, path)
if err != nil {
return ErrorResult(fmt.Sprintf("failed to read file from sandbox: %v", err))
}
return NewToolResult(string(content))
sb := sandbox.FromContext(ctx)
if sb == nil {
return ErrorResult("sandbox environment unavailable")
}
content, err := t.fs.ReadFile(path)
content, err := sb.Fs().ReadFile(ctx, path)
if err != nil {
return ErrorResult(err.Error())
return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
}
return NewToolResult(string(content))
}
type WriteFileTool struct {
fs fileSystem
}
type WriteFileTool struct{}
func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool {
var fs fileSystem
if restrict {
fs = &sandboxFs{workspace: workspace}
} else {
fs = &hostFs{}
}
return &WriteFileTool{fs: fs}
return &WriteFileTool{}
}
func (t *WriteFileTool) Name() string {
@ -190,35 +99,21 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *ToolR
return ErrorResult("content is required")
}
sb := sandbox.SandboxFromContext(ctx)
if sb != nil {
if err := sb.Fs().WriteFile(ctx, path, []byte(content), true); err != nil {
return ErrorResult(fmt.Sprintf("failed to write file to sandbox: %v", err))
}
return SilentResult(fmt.Sprintf("File written to sandbox: %s", path))
sb := sandbox.FromContext(ctx)
if sb == nil {
return ErrorResult("sandbox environment unavailable")
}
if err := t.fs.WriteFile(path, []byte(content)); err != nil {
return ErrorResult(err.Error())
if err := sb.Fs().WriteFile(ctx, path, []byte(content), true); err != nil {
return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
}
return SilentResult(fmt.Sprintf("File written: %s", path))
}
// ListDirTool lists files and directories at a given path.
// Phase 1: host-only execution; sandbox routing is deferred to Phase 2 (see sandbox.md §6.2).
type ListDirTool struct {
fs fileSystem
}
type ListDirTool struct{}
func NewListDirTool(workspace string, restrict bool) *ListDirTool {
var fs fileSystem
if restrict {
fs = &sandboxFs{workspace: workspace}
} else {
fs = &hostFs{}
}
return &ListDirTool{fs: fs}
return &ListDirTool{}
}
func (t *ListDirTool) Name() string {
@ -248,7 +143,12 @@ func (t *ListDirTool) Execute(ctx context.Context, args map[string]any) *ToolRes
path = "."
}
entries, err := t.fs.ReadDir(path)
sb := sandbox.FromContext(ctx)
if sb == nil {
return ErrorResult("sandbox environment unavailable")
}
entries, err := sb.Fs().ReadDir(ctx, path)
if err != nil {
return ErrorResult(fmt.Sprintf("failed to read directory: %v", err))
}
@ -266,161 +166,3 @@ func formatDirEntries(entries []os.DirEntry) *ToolResult {
}
return NewToolResult(result.String())
}
// fileSystem abstracts reading, writing, and listing files, allowing both
// unrestricted (host filesystem) and sandbox (os.Root) implementations to share the same polymorphic interface.
type fileSystem interface {
ReadFile(path string) ([]byte, error)
WriteFile(path string, data []byte) error
ReadDir(path string) ([]os.DirEntry, error)
}
// hostFs is an unrestricted fileReadWriter that operates directly on the host filesystem.
type hostFs struct{}
func (h *hostFs) ReadFile(path string) ([]byte, error) {
content, err := os.ReadFile(path)
if err != nil {
if os.IsNotExist(err) {
return nil, fmt.Errorf("failed to read file: file not found: %w", err)
}
if os.IsPermission(err) {
return nil, fmt.Errorf("failed to read file: access denied: %w", err)
}
return nil, fmt.Errorf("failed to read file: %w", err)
}
return content, nil
}
func (h *hostFs) ReadDir(path string) ([]os.DirEntry, error) {
return os.ReadDir(path)
}
func (h *hostFs) WriteFile(path string, data []byte) error {
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, 0o755); err != nil {
return fmt.Errorf("failed to create parent directories: %w", err)
}
// We use a "write-then-rename" pattern here to ensure an atomic write.
// This prevents the target file from being left in a truncated or partial state
// if the operation is interrupted, as the rename operation is atomic on Linux.
tmpPath := fmt.Sprintf("%s.%d.tmp", path, time.Now().UnixNano())
if err := os.WriteFile(tmpPath, data, 0o644); err != nil {
os.Remove(tmpPath) // Ensure cleanup of partial/empty temp file
return fmt.Errorf("failed to write temp file: %w", err)
}
if err := os.Rename(tmpPath, path); err != nil {
os.Remove(tmpPath)
return fmt.Errorf("failed to replace original file: %w", err)
}
return nil
}
// sandboxFs is a sandboxed fileSystem that operates within a strictly defined workspace using os.Root.
type sandboxFs struct {
workspace string
}
func (r *sandboxFs) execute(path string, fn func(root *os.Root, relPath string) error) error {
if r.workspace == "" {
return fmt.Errorf("workspace is not defined")
}
root, err := os.OpenRoot(r.workspace)
if err != nil {
return fmt.Errorf("failed to open workspace: %w", err)
}
defer root.Close()
relPath, err := getSafeRelPath(r.workspace, path)
if err != nil {
return err
}
return fn(root, relPath)
}
func (r *sandboxFs) ReadFile(path string) ([]byte, error) {
var content []byte
err := r.execute(path, func(root *os.Root, relPath string) error {
fileContent, err := root.ReadFile(relPath)
if err != nil {
if os.IsNotExist(err) {
return fmt.Errorf("failed to read file: file not found: %w", err)
}
// os.Root returns "escapes from parent" for paths outside the root
if os.IsPermission(err) || strings.Contains(err.Error(), "escapes from parent") ||
strings.Contains(err.Error(), "permission denied") {
return fmt.Errorf("failed to read file: access denied: %w", err)
}
return fmt.Errorf("failed to read file: %w", err)
}
content = fileContent
return nil
})
return content, err
}
func (r *sandboxFs) WriteFile(path string, data []byte) error {
return r.execute(path, func(root *os.Root, relPath string) error {
dir := filepath.Dir(relPath)
if dir != "." && dir != "/" {
if err := root.MkdirAll(dir, 0o755); err != nil {
return fmt.Errorf("failed to create parent directories: %w", err)
}
}
// We use a "write-then-rename" pattern here to ensure an atomic write.
// This prevents the target file from being left in a truncated or partial state
// if the operation is interrupted, as the rename operation is atomic on Linux.
tmpRelPath := fmt.Sprintf("%s.%d.tmp", relPath, time.Now().UnixNano())
if err := root.WriteFile(tmpRelPath, data, 0o644); err != nil {
root.Remove(tmpRelPath) // Ensure cleanup of partial/empty temp file
return fmt.Errorf("failed to write to temp file: %w", err)
}
if err := root.Rename(tmpRelPath, relPath); err != nil {
root.Remove(tmpRelPath)
return fmt.Errorf("failed to rename temp file over target: %w", err)
}
return nil
})
}
func (r *sandboxFs) ReadDir(path string) ([]os.DirEntry, error) {
var entries []os.DirEntry
err := r.execute(path, func(root *os.Root, relPath string) error {
dirEntries, err := fs.ReadDir(root.FS(), relPath)
if err != nil {
return err
}
entries = dirEntries
return nil
})
return entries, err
}
// Helper to get a safe relative path for os.Root usage
func getSafeRelPath(workspace, path string) (string, error) {
if workspace == "" {
return "", fmt.Errorf("workspace is not defined")
}
rel := filepath.Clean(path)
if filepath.IsAbs(rel) {
var err error
rel, err = filepath.Rel(workspace, rel)
if err != nil {
return "", fmt.Errorf("failed to calculate relative path: %w", err)
}
}
if !filepath.IsLocal(rel) {
return "", fmt.Errorf("path escapes workspace: %s", path)
}
return rel, nil
}

186
pkg/tools/filesystem_test.go
View file

@ -2,13 +2,15 @@ package tools
import (
"context"
"io"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/sipeed/picoclaw/pkg/agent/sandbox"
)
// TestFilesystemTool_ReadFile_Success verifies successful file reading
@ -18,7 +20,11 @@ func TestFilesystemTool_ReadFile_Success(t *testing.T) {
os.WriteFile(testFile, []byte("test content"), 0o644)
tool := NewReadFileTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
fs: sandbox.NewHostSandbox(tmpDir, false).Fs(),
})
// We must ensure the mock FsBridge can actually read the TempDir.
// but stubSandbox uses hostFs internally for Fs(). ReadFile so this just works if injected.
args := map[string]any{
"path": testFile,
}
@ -45,7 +51,9 @@ func TestFilesystemTool_ReadFile_Success(t *testing.T) {
// TestFilesystemTool_ReadFile_NotFound verifies error handling for missing file
func TestFilesystemTool_ReadFile_NotFound(t *testing.T) {
tool := NewReadFileTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
err: fmt.Errorf("failed to read file: file not found"),
})
args := map[string]any{
"path": "/nonexistent_file_12345.txt",
}
@ -66,7 +74,7 @@ func TestFilesystemTool_ReadFile_NotFound(t *testing.T) {
// TestFilesystemTool_ReadFile_MissingPath verifies error handling for missing path
func TestFilesystemTool_ReadFile_MissingPath(t *testing.T) {
tool := &ReadFileTool{}
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{})
args := map[string]any{}
result := tool.Execute(ctx, args)
@ -88,7 +96,9 @@ func TestFilesystemTool_WriteFile_Success(t *testing.T) {
testFile := filepath.Join(tmpDir, "newfile.txt")
tool := NewWriteFileTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
fs: sandbox.NewHostSandbox(tmpDir, false).Fs(),
})
args := map[string]any{
"path": testFile,
"content": "hello world",
@ -127,7 +137,9 @@ func TestFilesystemTool_WriteFile_CreateDir(t *testing.T) {
testFile := filepath.Join(tmpDir, "subdir", "newfile.txt")
tool := NewWriteFileTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
fs: sandbox.NewHostSandbox(tmpDir, false).Fs(),
})
args := map[string]any{
"path": testFile,
"content": "test",
@ -153,7 +165,7 @@ func TestFilesystemTool_WriteFile_CreateDir(t *testing.T) {
// TestFilesystemTool_WriteFile_MissingPath verifies error handling for missing path
func TestFilesystemTool_WriteFile_MissingPath(t *testing.T) {
tool := NewWriteFileTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{})
args := map[string]any{
"content": "test",
}
@ -169,7 +181,7 @@ func TestFilesystemTool_WriteFile_MissingPath(t *testing.T) {
// TestFilesystemTool_WriteFile_MissingContent verifies error handling for missing content
func TestFilesystemTool_WriteFile_MissingContent(t *testing.T) {
tool := NewWriteFileTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{})
args := map[string]any{
"path": "/tmp/test.txt",
}
@ -195,8 +207,10 @@ func TestFilesystemTool_ListDir_Success(t *testing.T) {
os.WriteFile(filepath.Join(tmpDir, "file2.txt"), []byte("content"), 0o644)
os.Mkdir(filepath.Join(tmpDir, "subdir"), 0o755)
tool := NewListDirTool("", false)
ctx := context.Background()
tool := NewListDirTool(tmpDir, false)
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
fs: sandbox.NewHostSandbox(tmpDir, false).Fs(),
})
args := map[string]any{
"path": tmpDir,
}
@ -219,8 +233,12 @@ func TestFilesystemTool_ListDir_Success(t *testing.T) {
// TestFilesystemTool_ListDir_NotFound verifies error handling for non-existent directory
func TestFilesystemTool_ListDir_NotFound(t *testing.T) {
tool := NewListDirTool("", false)
ctx := context.Background()
tmpDir := t.TempDir()
tool := NewListDirTool(tmpDir, false)
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
fs: sandbox.NewHostSandbox(tmpDir, false).Fs(),
err: fmt.Errorf("failed to read directory: file not found"),
})
args := map[string]any{
"path": "/nonexistent_directory_12345",
}
@ -240,8 +258,10 @@ func TestFilesystemTool_ListDir_NotFound(t *testing.T) {
// TestFilesystemTool_ListDir_DefaultPath verifies default to current directory
func TestFilesystemTool_ListDir_DefaultPath(t *testing.T) {
tool := NewListDirTool("", false)
ctx := context.Background()
tool := NewListDirTool(".", false)
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
fs: sandbox.NewHostSandbox(".", false).Fs(),
})
args := map[string]any{}
result := tool.Execute(ctx, args)
@ -271,7 +291,9 @@ func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) {
}
tool := NewReadFileTool(workspace, true)
result := tool.Execute(context.Background(), map[string]any{
result := tool.Execute(sandbox.WithSandbox(context.Background(), &stubSandbox{
fs: sandbox.NewHostSandbox(workspace, true).Fs(),
}), map[string]any{
"path": link,
})
@ -295,7 +317,9 @@ func TestFilesystemTool_EmptyWorkspace_AccessDenied(t *testing.T) {
secretFile := filepath.Join(tmpDir, "shadow")
os.WriteFile(secretFile, []byte("secret data"), 0o600)
result := tool.Execute(context.Background(), map[string]any{
result := tool.Execute(sandbox.WithSandbox(context.Background(), &stubSandbox{
err: fmt.Errorf("workspace is not defined"),
}), map[string]any{
"path": secretFile,
})
@ -342,7 +366,9 @@ func TestRootMkdirAll(t *testing.T) {
func TestFilesystemTool_WriteFile_Restricted_CreateDir(t *testing.T) {
workspace := t.TempDir()
tool := NewWriteFileTool(workspace, true)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
fs: sandbox.NewHostSandbox(workspace, true).Fs(),
})
testFile := "deep/nested/path/to/file.txt"
content := "deep content"
@ -360,129 +386,3 @@ func TestFilesystemTool_WriteFile_Restricted_CreateDir(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, content, string(data))
}
// TestHostRW_Read_PermissionDenied verifies that hostRW.Read surfaces access denied errors.
func TestHostRW_Read_PermissionDenied(t *testing.T) {
if os.Getuid() == 0 {
t.Skip("skipping permission test: running as root")
}
tmpDir := t.TempDir()
protected := filepath.Join(tmpDir, "protected.txt")
err := os.WriteFile(protected, []byte("secret"), 0o000)
assert.NoError(t, err)
defer os.Chmod(protected, 0o644) // ensure cleanup
_, err = (&hostFs{}).ReadFile(protected)
assert.Error(t, err)
assert.Contains(t, err.Error(), "access denied")
}
// TestHostRW_Read_Directory verifies that hostRW.Read returns an error when given a directory path.
func TestHostRW_Read_Directory(t *testing.T) {
tmpDir := t.TempDir()
_, err := (&hostFs{}).ReadFile(tmpDir)
assert.Error(t, err, "expected error when reading a directory as a file")
}
// TestRootRW_Read_Directory verifies that rootRW.Read returns an error when given a directory.
func TestRootRW_Read_Directory(t *testing.T) {
workspace := t.TempDir()
root, err := os.OpenRoot(workspace)
assert.NoError(t, err)
defer root.Close()
// Create a subdirectory
err = root.Mkdir("subdir", 0o755)
assert.NoError(t, err)
_, err = (&sandboxFs{workspace: workspace}).ReadFile("subdir")
assert.Error(t, err, "expected error when reading a directory as a file")
}
// TestHostRW_Write_ParentDirMissing verifies that hostRW.Write creates parent dirs automatically.
func TestHostRW_Write_ParentDirMissing(t *testing.T) {
tmpDir := t.TempDir()
target := filepath.Join(tmpDir, "a", "b", "c", "file.txt")
err := (&hostFs{}).WriteFile(target, []byte("hello"))
assert.NoError(t, err)
data, err := os.ReadFile(target)
assert.NoError(t, err)
assert.Equal(t, "hello", string(data))
}
// TestRootRW_Write_ParentDirMissing verifies that rootRW.Write creates
// nested parent directories automatically within the sandbox.
func TestRootRW_Write_ParentDirMissing(t *testing.T) {
workspace := t.TempDir()
relPath := "x/y/z/file.txt"
err := (&sandboxFs{workspace: workspace}).WriteFile(relPath, []byte("nested"))
assert.NoError(t, err)
data, err := os.ReadFile(filepath.Join(workspace, relPath))
assert.NoError(t, err)
assert.Equal(t, "nested", string(data))
}
// TestHostRW_Write verifies the hostRW.Write helper function
func TestHostRW_Write(t *testing.T) {
tmpDir := t.TempDir()
testFile := filepath.Join(tmpDir, "atomic_test.txt")
testData := []byte("atomic test content")
err := (&hostFs{}).WriteFile(testFile, testData)
assert.NoError(t, err)
content, err := os.ReadFile(testFile)
assert.NoError(t, err)
assert.Equal(t, testData, content)
// Verify it overwrites correctly
newData := []byte("new atomic content")
err = (&hostFs{}).WriteFile(testFile, newData)
assert.NoError(t, err)
content, err = os.ReadFile(testFile)
assert.NoError(t, err)
assert.Equal(t, newData, content)
}
// TestRootRW_Write verifies the rootRW.Write helper function
func TestRootRW_Write(t *testing.T) {
tmpDir := t.TempDir()
relPath := "atomic_root_test.txt"
testData := []byte("atomic root test content")
erw := &sandboxFs{workspace: tmpDir}
err := erw.WriteFile(relPath, testData)
assert.NoError(t, err)
root, err := os.OpenRoot(tmpDir)
assert.NoError(t, err)
defer root.Close()
f, err := root.Open(relPath)
assert.NoError(t, err)
defer f.Close()
content, err := io.ReadAll(f)
assert.NoError(t, err)
assert.Equal(t, testData, content)
// Verify it overwrites correctly
newData := []byte("new root atomic content")
err = erw.WriteFile(relPath, newData)
assert.NoError(t, err)
f2, err := root.Open(relPath)
assert.NoError(t, err)
defer f2.Close()
content, err = io.ReadAll(f2)
assert.NoError(t, err)
assert.Equal(t, newData, content)
}

126
pkg/tools/shell.go
View file

@ -1,16 +1,13 @@
package tools
import (
"bytes"
"context"
"errors"
"fmt"
"os"
"os/exec"
"path"
"path/filepath"
"regexp"
"runtime"
"strings"
"time"
@ -148,11 +145,11 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
cwd := t.workingDir
if wd != "" {
if t.restrictToWorkspace && t.workingDir != "" {
resolvedWD, err := validatePath(wd, t.workingDir, true)
resolvedWD, err := sandbox.ValidatePath(wd, t.workingDir, true)
if err != nil {
// In sandbox mode, allow explicit container workspace paths when
// restrict_to_workspace is enabled.
sb := sandbox.SandboxFromContext(ctx)
sb := sandbox.FromContext(ctx)
if sb != nil && filepath.IsAbs(wd) && isSandboxWorkspaceAbsolutePath(wd) {
cwd = wd
} else {
@ -176,96 +173,19 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
return ErrorResult(guardError)
}
sb := sandbox.SandboxFromContext(ctx)
if sb != nil {
sandboxWD := t.resolveSandboxWorkingDir(cwd)
res, err := sb.Exec(ctx, sandbox.ExecRequest{
Command: command,
WorkingDir: sandboxWD,
TimeoutMs: t.timeout.Milliseconds(),
})
if err != nil {
return ErrorResult(fmt.Sprintf("sandbox exec failed: %v", err))
}
output := res.Stdout
if res.Stderr != "" {
output += "\nSTDERR:\n" + res.Stderr
}
if output == "" {
output = "(no output)"
}
if res.ExitCode != 0 {
output += fmt.Sprintf("\nExit code: %d", res.ExitCode)
return &ToolResult{
ForLLM: output,
ForUser: output,
IsError: true,
}
}
return &ToolResult{
ForLLM: output,
ForUser: output,
IsError: false,
}
}
// timeout == 0 means no timeout
var cmdCtx context.Context
var cancel context.CancelFunc
if t.timeout > 0 {
cmdCtx, cancel = context.WithTimeout(ctx, t.timeout)
} else {
cmdCtx, cancel = context.WithCancel(ctx)
}
defer cancel()
var cmd *exec.Cmd
if runtime.GOOS == "windows" {
cmd = exec.CommandContext(cmdCtx, "powershell", "-NoProfile", "-NonInteractive", "-Command", command)
} else {
cmd = exec.CommandContext(cmdCtx, "sh", "-c", command)
}
if cwd != "" {
cmd.Dir = cwd
}
prepareCommandForTermination(cmd)
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr
if err := cmd.Start(); err != nil {
return ErrorResult(fmt.Sprintf("failed to start command: %v", err))
}
done := make(chan error, 1)
go func() {
done <- cmd.Wait()
}()
var err error
select {
case err = <-done:
case <-cmdCtx.Done():
terminateProcessTree(cmd)
select {
case err = <-done:
case <-time.After(2 * time.Second):
if cmd.Process != nil {
_ = cmd.Process.Kill()
}
err = <-done
}
}
output := stdout.String()
if stderr.Len() > 0 {
output += "\nSTDERR:\n" + stderr.String()
sb := sandbox.FromContext(ctx)
if sb == nil {
return ErrorResult("sandbox environment unavailable")
}
sandboxWD := t.resolveSandboxWorkingDir(cwd)
res, err := sb.Exec(ctx, sandbox.ExecRequest{
Command: command,
WorkingDir: sandboxWD,
TimeoutMs: t.timeout.Milliseconds(),
})
if err != nil {
if errors.Is(cmdCtx.Err(), context.DeadlineExceeded) {
if errors.Is(err, context.DeadlineExceeded) || strings.Contains(err.Error(), "context deadline exceeded") {
msg := fmt.Sprintf("command timed out after %v", t.timeout)
return &ToolResult{
ForLLM: msg,
@ -273,14 +193,12 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
IsError: true,
}
}
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
output += fmt.Sprintf("\nExit code: %d", exitErr.ExitCode())
} else {
output += fmt.Sprintf("\nError: %v", err)
}
return ErrorResult(fmt.Sprintf("sandbox exec failed: %v", err))
}
output := res.Stdout
if res.Stderr != "" {
output += "\nSTDERR:\n" + res.Stderr
}
if output == "" {
output = "(no output)"
}
@ -290,10 +208,18 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
output = output[:maxLen] + fmt.Sprintf("\n... (truncated, %d more chars)", len(output)-maxLen)
}
if res.ExitCode != 0 {
output += fmt.Sprintf("\nExit code: %d", res.ExitCode)
return &ToolResult{
ForLLM: output,
ForUser: output,
IsError: true,
}
}
return &ToolResult{
ForLLM: output,
ForUser: output,
IsError: err != nil,
IsError: false,
}
}

50
pkg/tools/shell_test.go
View file

@ -16,6 +16,7 @@ type stubSandbox struct {
lastReq sandbox.ExecRequest
err error
res *sandbox.ExecResult
fs sandbox.FsBridge
}
func (s *stubSandbox) Start(ctx context.Context) error { return nil }
@ -24,7 +25,14 @@ func (s *stubSandbox) Prune(ctx context.Context) error { return nil }
func (s *stubSandbox) Resolve(ctx context.Context) (sandbox.Sandbox, error) {
return s, nil
}
func (s *stubSandbox) Fs() sandbox.FsBridge { return nil }
func (s *stubSandbox) Fs() sandbox.FsBridge {
if s.fs != nil {
return s.fs
}
return sandbox.NewHostSandbox("", false).Fs()
}
func (s *stubSandbox) Exec(ctx context.Context, req sandbox.ExecRequest) (*sandbox.ExecResult, error) {
return sandboxAggregateFromStub(ctx, req, s.ExecStream)
}
@ -103,7 +111,9 @@ func sandboxAggregateFromStub(
func TestShellTool_Success(t *testing.T) {
tool := NewExecTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
res: &sandbox.ExecResult{Stdout: "hello world", ExitCode: 0},
})
args := map[string]any{
"command": "echo 'hello world'",
}
@ -130,7 +140,13 @@ func TestShellTool_Success(t *testing.T) {
func TestShellTool_Failure(t *testing.T) {
tool := NewExecTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
res: &sandbox.ExecResult{
Stdout: "",
Stderr: "ls: cannot access '/nonexistent_directory_12345': No such file or directory",
ExitCode: 2,
},
})
args := map[string]any{
"command": "ls /nonexistent_directory_12345",
}
@ -158,7 +174,9 @@ func TestShellTool_Timeout(t *testing.T) {
tool := NewExecTool("", false)
tool.SetTimeout(100 * time.Millisecond)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
err: context.DeadlineExceeded,
})
args := map[string]any{
"command": "sleep 10",
}
@ -185,7 +203,9 @@ func TestShellTool_WorkingDir(t *testing.T) {
tool := NewExecTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
res: &sandbox.ExecResult{Stdout: "test content\n", ExitCode: 0},
})
args := map[string]any{
"command": "cat test.txt",
"working_dir": tmpDir,
@ -206,7 +226,9 @@ func TestShellTool_WorkingDir(t *testing.T) {
func TestShellTool_DangerousCommand(t *testing.T) {
tool := NewExecTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
res: &sandbox.ExecResult{Stdout: "", Stderr: "", ExitCode: 1},
})
args := map[string]any{
"command": "rm -rf /",
}
@ -227,7 +249,9 @@ func TestShellTool_DangerousCommand(t *testing.T) {
func TestShellTool_MissingCommand(t *testing.T) {
tool := NewExecTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
res: &sandbox.ExecResult{Stdout: "", Stderr: "", ExitCode: 1},
})
args := map[string]any{}
result := tool.Execute(ctx, args)
@ -242,7 +266,9 @@ func TestShellTool_MissingCommand(t *testing.T) {
func TestShellTool_StderrCapture(t *testing.T) {
tool := NewExecTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
res: &sandbox.ExecResult{Stdout: "stdout", Stderr: "stderr", ExitCode: 0},
})
args := map[string]any{
"command": "sh -c 'echo stdout; echo stderr >&2'",
}
@ -262,7 +288,9 @@ func TestShellTool_StderrCapture(t *testing.T) {
func TestShellTool_OutputTruncation(t *testing.T) {
tool := NewExecTool("", false)
ctx := context.Background()
ctx := sandbox.WithSandbox(context.Background(), &stubSandbox{
res: &sandbox.ExecResult{Stdout: strings.Repeat("x", 20000), ExitCode: 0},
})
// Generate long output (>10000 chars)
args := map[string]any{
"command": "python3 -c \"print('x' * 20000)\" || echo " + strings.Repeat("x", 20000),
@ -289,7 +317,9 @@ func TestShellTool_WorkingDir_OutsideWorkspace(t *testing.T) {
}
tool := NewExecTool(workspace, true)
result := tool.Execute(context.Background(), map[string]any{
result := tool.Execute(sandbox.WithSandbox(context.Background(), &stubSandbox{
res: &sandbox.ExecResult{Stdout: "", Stderr: "", ExitCode: 1},
}), map[string]any{
"command": "pwd",
"working_dir": outsideDir,
})

16
pkg/tools/shell_timeout_unix_test.go
View file

@ -11,6 +11,8 @@ import (
"syscall"
"testing"
"time"
"github.com/sipeed/picoclaw/pkg/agent/sandbox"
)
func processExists(pid int) bool {
@ -22,15 +24,25 @@ func processExists(pid int) bool {
}
func TestShellTool_TimeoutKillsChildProcess(t *testing.T) {
tool := NewExecTool(t.TempDir(), false)
workspace := t.TempDir()
tool := NewExecTool(workspace, false)
tool.SetTimeout(500 * time.Millisecond)
sb := sandbox.NewHostSandbox(workspace, false)
err := sb.Start(context.Background())
if err != nil {
t.Fatalf("failed to start sandbox: %v", err)
}
defer sb.Prune(context.Background())
ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]any{
// Spawn a child process that would outlive the shell unless process-group kill is used.
"command": "sleep 60 & echo $! > child.pid; wait",
}
result := tool.Execute(context.Background(), args)
result := tool.Execute(ctx, args)
if !result.IsError {
t.Fatalf("expected timeout error, got success: %s", result.ForLLM)
}