refactor: reimplement sandbox management with a Manager interface for context-aware resolution and enable non-main sandboxing mode.

This commit is contained in:
0x5487 2026-02-22 21:12:34 +08:00
parent 549195406e
commit c69024892c
16 changed files with 246 additions and 168 deletions

View file

@ -29,6 +29,7 @@ type AgentInstance struct {
Sessions *session.SessionManager Sessions *session.SessionManager
ContextBuilder *ContextBuilder ContextBuilder *ContextBuilder
Tools *tools.ToolRegistry Tools *tools.ToolRegistry
SandboxManager sandbox.Manager
Subagents *config.SubagentsConfig Subagents *config.SubagentsConfig
SkillsFilter []string SkillsFilter []string
Candidates []providers.FallbackCandidate Candidates []providers.FallbackCandidate
@ -58,40 +59,31 @@ func NewAgentInstance(
} }
restrict := defaults.RestrictToWorkspace restrict := defaults.RestrictToWorkspace
sb := sandbox.NewFromConfigWithAgent(workspace, restrict, cfg, agentID)
isSandboxOff := true
if cfg != nil {
mode := strings.ToLower(strings.TrimSpace(cfg.Agents.Defaults.Sandbox.Mode))
if mode == "all" || mode == "non-main" {
isSandboxOff = false
}
}
roContainer := isContainerReadOnlySandbox(cfg) roContainer := isContainerReadOnlySandbox(cfg)
toolsRegistry := tools.NewToolRegistry() toolsRegistry := tools.NewToolRegistry()
// Helper to check if tool is allowed (either sandbox is off or policy allows it) sandboxManager := sandbox.NewFromConfigWithAgent(workspace, restrict, cfg, agentID)
isAllowed := func(toolName string) bool { isSandboxAllowed := func(toolName string) bool {
return isSandboxOff || sandbox.IsToolSandboxEnabled(cfg, toolName) return sandboxManager == nil || sandbox.IsToolSandboxEnabled(cfg, toolName)
} }
if isAllowed("read_file") { if isSandboxAllowed("read_file") {
toolsRegistry.Register(tools.NewReadFileToolWithSandbox(workspace, restrict, sb)) toolsRegistry.Register(tools.NewReadFileTool(workspace, restrict))
} }
if !roContainer && isAllowed("write_file") { if !roContainer && isSandboxAllowed("write_file") {
toolsRegistry.Register(tools.NewWriteFileToolWithSandbox(workspace, restrict, sb)) toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
} }
if isAllowed("list_dir") { if isSandboxAllowed("list_dir") {
toolsRegistry.Register(tools.NewListDirTool(workspace, restrict)) toolsRegistry.Register(tools.NewListDirTool(workspace, restrict))
} }
if isAllowed("exec") { if isSandboxAllowed("exec") {
toolsRegistry.Register(tools.NewExecToolWithSandbox(workspace, restrict, cfg, sb)) toolsRegistry.Register(tools.NewExecToolWithConfig(workspace, restrict, cfg))
} }
if !roContainer { if !roContainer {
if isAllowed("edit_file") { if isSandboxAllowed("edit_file") {
toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict)) toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict))
} }
if isAllowed("append_file") { if isSandboxAllowed("append_file") {
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict)) toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
} }
} }
@ -138,6 +130,7 @@ func NewAgentInstance(
Sessions: sessionsManager, Sessions: sessionsManager,
ContextBuilder: contextBuilder, ContextBuilder: contextBuilder,
Tools: toolsRegistry, Tools: toolsRegistry,
SandboxManager: sandboxManager,
Subagents: subagents, Subagents: subagents,
SkillsFilter: skillsFilter, SkillsFilter: skillsFilter,
Candidates: candidates, Candidates: candidates,

View file

@ -400,9 +400,24 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
} }
} }
// 1. Update tool contexts // 1. Prepare tool contexts and sandbox
al.updateToolContexts(agent, opts.Channel, opts.ChatID) al.updateToolContexts(agent, opts.Channel, opts.ChatID)
// Inject the routing session key so sandbox.shouldSandbox() can compare
// 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)
}
// 2. Build messages (skip history for heartbeat) // 2. Build messages (skip history for heartbeat)
var history []providers.Message var history []providers.Message
var summary string var summary string

View file

@ -14,6 +14,7 @@ import (
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
"sync"
"time" "time"
"github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/container"
@ -58,6 +59,7 @@ type ContainerSandboxConfig struct {
// ContainerSandbox executes commands and filesystem operations inside a managed docker container. // ContainerSandbox executes commands and filesystem operations inside a managed docker container.
type ContainerSandbox struct { type ContainerSandbox struct {
mu sync.Mutex
cfg ContainerSandboxConfig cfg ContainerSandboxConfig
cli *client.Client cli *client.Client
startErr error startErr error
@ -70,10 +72,10 @@ const defaultSandboxRegistryFile = "containers.json"
// NewContainerSandbox creates a container sandbox with normalized defaults and precomputed config hash. // NewContainerSandbox creates a container sandbox with normalized defaults and precomputed config hash.
func NewContainerSandbox(cfg ContainerSandboxConfig) *ContainerSandbox { func NewContainerSandbox(cfg ContainerSandboxConfig) *ContainerSandbox {
if strings.TrimSpace(cfg.Image) == "" { if strings.TrimSpace(cfg.Image) == "" {
cfg.Image = "openclaw-sandbox:bookworm-slim" cfg.Image = "debian:bookworm-slim"
} }
if strings.TrimSpace(cfg.ContainerPrefix) == "" { if strings.TrimSpace(cfg.ContainerPrefix) == "" {
cfg.ContainerPrefix = "picoclaw-sbx-" cfg.ContainerPrefix = "picoclaw-sandbox-"
} }
if strings.TrimSpace(cfg.ContainerName) == "" { if strings.TrimSpace(cfg.ContainerName) == "" {
cfg.ContainerName = cfg.ContainerPrefix + "default" cfg.ContainerName = cfg.ContainerPrefix + "default"
@ -103,11 +105,20 @@ func NewContainerSandbox(cfg ContainerSandboxConfig) *ContainerSandbox {
// Start initializes docker connectivity and validates sandbox runtime requirements. // Start initializes docker connectivity and validates sandbox runtime requirements.
func (c *ContainerSandbox) Start(ctx context.Context) error { func (c *ContainerSandbox) Start(ctx context.Context) error {
c.mu.Lock()
defer c.mu.Unlock()
if c.startErr != nil {
return c.startErr
}
if c.cli != nil {
return nil
}
if err := validateSandboxSecurity(c.cfg); err != nil { if err := validateSandboxSecurity(c.cfg); err != nil {
c.startErr = err c.startErr = err
return err return err
} }
c.cfg.Env = sanitizeEnvVars(c.cfg.Env)
if strings.TrimSpace(c.cfg.Workspace) != "" && c.cfg.WorkspaceAccess == "none" { if strings.TrimSpace(c.cfg.Workspace) != "" && c.cfg.WorkspaceAccess == "none" {
if err := os.MkdirAll(c.cfg.Workspace, 0o755); err != nil { if err := os.MkdirAll(c.cfg.Workspace, 0o755); err != nil {
c.startErr = fmt.Errorf("sandbox workspace init failed: %w", err) c.startErr = fmt.Errorf("sandbox workspace init failed: %w", err)
@ -145,10 +156,14 @@ func (c *ContainerSandbox) Start(ctx context.Context) error {
_, _ = io.Copy(io.Discard, rc) _, _ = io.Copy(io.Discard, rc)
} }
c.startErr = nil
return nil return nil
} }
// Resolve returns the container sandbox itself.
func (c *ContainerSandbox) Resolve(ctx context.Context) (Sandbox, error) {
return c, nil
}
// Prune reclaims container sandbox resources. // Prune reclaims container sandbox resources.
// This is the container-specific cleanup boundary where implementations should // This is the container-specific cleanup boundary where implementations should
// stop and remove this sandbox container. // stop and remove this sandbox container.

View file

@ -104,7 +104,7 @@ func TestHostSandbox_ResolvePathRestrictions(t *testing.T) {
} }
func TestUnavailableSandboxAndUtilHelpers(t *testing.T) { func TestUnavailableSandboxAndUtilHelpers(t *testing.T) {
sb := NewUnavailableSandbox(nil) sb := NewUnavailableSandboxManager(nil)
if err := sb.Start(context.Background()); err == nil { if err := sb.Start(context.Background()); err == nil {
t.Fatal("expected Start() error") t.Fatal("expected Start() error")
} }

View file

@ -4,6 +4,7 @@ import (
"context" "context"
"crypto/sha256" "crypto/sha256"
"encoding/hex" "encoding/hex"
"errors"
"fmt" "fmt"
"os" "os"
"path/filepath" "path/filepath"
@ -16,13 +17,17 @@ import (
"github.com/sipeed/picoclaw/pkg/routing" "github.com/sipeed/picoclaw/pkg/routing"
) )
// NewFromConfig builds a sandbox instance from config and starts it before returning. // NewFromConfig builds a host sandbox from config for host-level execution (e.g. cron jobs).
// It does not return a Manager; use NewFromConfigWithAgent when sandbox routing is needed.
func NewFromConfig(workspace string, restrict bool, cfg *config.Config) Sandbox { func NewFromConfig(workspace string, restrict bool, cfg *config.Config) Sandbox {
return NewFromConfigWithAgent(workspace, restrict, cfg, routing.DefaultAgentID) host := NewHostSandbox(workspace, restrict)
_ = host.Start(context.Background())
return host
} }
// NewFromConfigWithAgent builds a sandbox instance with an explicit agent ID context. // NewFromConfigWithAgent builds the sandbox Manager for an agent.
func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config, agentID string) Sandbox { // Returns nil when sandboxing is disabled (mode=off), so callers can check manager != nil.
func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config, agentID string) Manager {
mode := "all" mode := "all"
scope := "agent" scope := "agent"
workspaceAccess := "none" workspaceAccess := "none"
@ -63,13 +68,15 @@ func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config,
} }
agentID = routing.NormalizeAgentID(agentID) agentID = routing.NormalizeAgentID(agentID)
host := NewHostSandbox(workspace, restrict)
_ = host.Start(context.Background())
resolvedMode := normalizeSandboxMode(mode) resolvedMode := normalizeSandboxMode(mode)
if resolvedMode == "off" { if resolvedMode == "off" {
return host return nil // sandbox disabled; host-level access is handled directly by tools
} }
host := NewHostSandbox(workspace, restrict)
_ = host.Start(context.Background())
resolvedScope := normalizeSandboxScope(scope) resolvedScope := normalizeSandboxScope(scope)
normalizedAccess := normalizeWorkspaceAccess(workspaceAccess) normalizedAccess := normalizeWorkspaceAccess(workspaceAccess)
workspaceRootAbs := resolveAbsPath(expandHomePath(workspaceRoot)) workspaceRootAbs := resolveAbsPath(expandHomePath(workspaceRoot))
@ -92,7 +99,7 @@ func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config,
} }
manager.fs = &managerFS{m: manager} manager.fs = &managerFS{m: manager}
if err := manager.Start(context.Background()); err != nil { if err := manager.Start(context.Background()); err != nil {
return NewUnavailableSandbox(fmt.Errorf("container sandbox unavailable: %w", err)) return NewUnavailableSandboxManager(fmt.Errorf("container sandbox unavailable: %w", err))
} }
return manager return manager
} }
@ -340,15 +347,22 @@ func (m *scopedSandboxManager) Fs() FsBridge {
return m.fs return m.fs
} }
// Resolve returns the specific sandbox instance to be used for the given context.
func (m *scopedSandboxManager) Resolve(ctx context.Context) (Sandbox, error) {
if !m.shouldSandbox(ctx) {
return m.host, nil
}
return m.getOrCreateSandbox(ctx, m.scopeKeyFromContext(ctx))
}
func (m *scopedSandboxManager) shouldSandbox(ctx context.Context) bool { func (m *scopedSandboxManager) shouldSandbox(ctx context.Context) bool {
switch m.mode { switch m.mode {
case "all": case "all":
return true return true
case "non-main": case "non-main":
// Phase 2 deferred: `non-main` requires stable session-key propagation // Sandbox all sessions except the agent's main session.
// across all tool execution paths. For now, keep behavior disabled until // Normalize before comparing to handle aliases like "main" or bare agent keys
// the execution context plumbing is finalized. return m.normalizeSessionKey(SessionKeyFromContext(ctx)) != m.mainSessionKey()
return false
default: default:
return false return false
} }
@ -493,3 +507,49 @@ func slugScopeKey(scopeKey string) string {
sum := sha256.Sum256([]byte(raw)) sum := sha256.Sum256([]byte(raw))
return safe + "-" + hex.EncodeToString(sum[:4]) return safe + "-" + hex.EncodeToString(sum[:4])
} }
type unavailableSandboxManager struct {
err error
fs FsBridge
}
func NewUnavailableSandboxManager(err error) Manager {
if err == nil {
err = errors.New("sandbox unavailable")
}
return &unavailableSandboxManager{
err: err,
fs: &errorFS{err: err},
}
}
func (u *unavailableSandboxManager) Start(ctx context.Context) error { return u.err }
func (u *unavailableSandboxManager) Prune(ctx context.Context) error { return nil }
// Resolve returns an error because the sandbox is unavailable.
func (u *unavailableSandboxManager) Resolve(ctx context.Context) (Sandbox, error) {
return nil, u.err
}
func (u *unavailableSandboxManager) Fs() FsBridge { return u.fs }
func (u *unavailableSandboxManager) Exec(ctx context.Context, req ExecRequest) (*ExecResult, error) {
return aggregateExecStream(func(onEvent func(ExecEvent) error) (*ExecResult, error) {
return u.ExecStream(ctx, req, onEvent)
})
}
func (u *unavailableSandboxManager) ExecStream(ctx context.Context, req ExecRequest, onEvent func(ExecEvent) error) (*ExecResult, error) {
return nil, u.err
}
type errorFS struct {
err error
}
func (e *errorFS) ReadFile(ctx context.Context, path string) ([]byte, error) {
return nil, fmt.Errorf("sandbox unavailable: %w", e.err)
}
func (e *errorFS) WriteFile(ctx context.Context, path string, data []byte, mkdir bool) error {
return fmt.Errorf("sandbox unavailable: %w", e.err)
}

View file

@ -38,10 +38,8 @@ func TestExpandHomePath(t *testing.T) {
} }
func TestNewFromConfig_HostMode(t *testing.T) { func TestNewFromConfig_HostMode(t *testing.T) {
cfg := config.DefaultConfig() // NewFromConfig always returns a HostSandbox regardless of mode config.
cfg.Agents.Defaults.Sandbox.Mode = "off" sb := NewFromConfig(t.TempDir(), true, nil)
sb := NewFromConfig(t.TempDir(), true, cfg)
if _, ok := sb.(*HostSandbox); !ok { if _, ok := sb.(*HostSandbox); !ok {
t.Fatalf("expected HostSandbox, got %T", sb) t.Fatalf("expected HostSandbox, got %T", sb)
} }
@ -57,12 +55,18 @@ func TestNewFromConfig_AllModeReturnsUnavailableWhenBlocked(t *testing.T) {
cfg.Agents.Defaults.Sandbox.Prune.IdleHours = 0 cfg.Agents.Defaults.Sandbox.Prune.IdleHours = 0
cfg.Agents.Defaults.Sandbox.Prune.MaxAgeDays = 0 cfg.Agents.Defaults.Sandbox.Prune.MaxAgeDays = 0
sb := NewFromConfig(t.TempDir(), true, cfg) // NewFromConfigWithAgent is the manager factory; when Docker is unavailable
if _, ok := sb.(*unavailableSandbox); !ok { // it should return an unavailableSandbox that implements Manager.
t.Fatalf("expected unavailableSandbox, got %T", sb) mgr := NewFromConfigWithAgent(t.TempDir(), true, cfg, "test")
if mgr == nil {
t.Fatal("expected non-nil Manager when mode=all")
} }
if err := sb.Start(context.Background()); err == nil { if _, ok := mgr.(*unavailableSandboxManager); !ok {
t.Fatal("expected unavailable sandbox start error") t.Fatalf("expected unavailableSandbox, got %T", mgr)
}
// Resolve() must propagate the unavailability error.
if _, err := mgr.Resolve(context.Background()); err == nil {
t.Fatal("expected unavailable sandbox Resolve() to return error")
} }
} }

View file

@ -17,3 +17,19 @@ func SessionKeyFromContext(ctx context.Context) string {
v, _ := ctx.Value(sessionContextKey{}).(string) v, _ := ctx.Value(sessionContextKey{}).(string)
return v return v
} }
type sandboxContextKey struct{}
// WithSandbox returns a derived context carrying the current sandbox instance.
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 {
if ctx == nil {
return nil
}
v, _ := ctx.Value(sandboxContextKey{}).(Sandbox)
return v
}

View file

@ -28,6 +28,16 @@ type Sandbox interface {
Fs() FsBridge Fs() FsBridge
} }
// Manager is implemented by the scoped sandbox manager.
// It resolves the specific Sandbox instance to use for the current execution context
// (e.g. based on session key / scope). Only the manager needs this; leaf sandboxes
// (HostSandbox, ContainerSandbox) implement Sandbox directly and are the resolved result.
type Manager interface {
Sandbox
// Resolve returns the concrete Sandbox instance to use for the given context.
Resolve(ctx context.Context) (Sandbox, error)
}
// ExecRequest describes a command execution request for Sandbox.Exec. // ExecRequest describes a command execution request for Sandbox.Exec.
type ExecRequest struct { type ExecRequest struct {
// Command is the program or shell command to execute. // Command is the program or shell command to execute.

View file

@ -1,47 +0,0 @@
package sandbox
import (
"context"
"errors"
"fmt"
)
type unavailableSandbox struct {
err error
fs FsBridge
}
func NewUnavailableSandbox(err error) Sandbox {
if err == nil {
err = errors.New("sandbox unavailable")
}
return &unavailableSandbox{
err: err,
fs: &errorFS{err: err},
}
}
func (u *unavailableSandbox) Start(ctx context.Context) error { return u.err }
func (u *unavailableSandbox) Prune(ctx context.Context) error { return nil }
func (u *unavailableSandbox) Fs() FsBridge { return u.fs }
func (u *unavailableSandbox) Exec(ctx context.Context, req ExecRequest) (*ExecResult, error) {
return aggregateExecStream(func(onEvent func(ExecEvent) error) (*ExecResult, error) {
return u.ExecStream(ctx, req, onEvent)
})
}
func (u *unavailableSandbox) ExecStream(ctx context.Context, req ExecRequest, onEvent func(ExecEvent) error) (*ExecResult, error) {
return nil, u.err
}
type errorFS struct {
err error
}
func (e *errorFS) ReadFile(ctx context.Context, path string) ([]byte, error) {
return nil, fmt.Errorf("sandbox unavailable: %w", e.err)
}
func (e *errorFS) WriteFile(ctx context.Context, path string, data []byte, mkdir bool) error {
return fmt.Errorf("sandbox unavailable: %w", e.err)
}

View file

@ -1,6 +1,8 @@
package tools package tools
import "context" import (
"context"
)
// Tool is the interface that all tools must implement. // Tool is the interface that all tools must implement.
type Tool interface { type Tool interface {

View file

@ -35,7 +35,7 @@ type CronTool struct {
// execTimeout: 0 means no timeout, >0 sets the timeout duration. // execTimeout: 0 means no timeout, >0 sets the timeout duration.
func NewCronTool(cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string, restrict bool, execTimeout time.Duration, config *config.Config) *CronTool { func NewCronTool(cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string, restrict bool, execTimeout time.Duration, config *config.Config) *CronTool {
sb := sandbox.NewFromConfig(workspace, restrict, config) sb := sandbox.NewFromConfig(workspace, restrict, config)
guard := NewExecToolWithSandbox(workspace, restrict, config, nil) guard := NewExecToolWithConfig(workspace, restrict, config)
return &CronTool{ return &CronTool{
cronService: cronService, cronService: cronService,
executor: executor, executor: executor,

View file

@ -20,7 +20,11 @@ type cronStubSandbox struct {
func (s *cronStubSandbox) Start(ctx context.Context) error { return nil } func (s *cronStubSandbox) Start(ctx context.Context) error { return nil }
func (s *cronStubSandbox) Prune(ctx context.Context) error { return nil } func (s *cronStubSandbox) Prune(ctx context.Context) error { return nil }
func (s *cronStubSandbox) Fs() sandbox.FsBridge { return nil }
func (s *cronStubSandbox) Resolve(ctx context.Context) (sandbox.Sandbox, error) {
return s, nil
}
func (s *cronStubSandbox) Fs() sandbox.FsBridge { return nil }
func (s *cronStubSandbox) Exec(ctx context.Context, req sandbox.ExecRequest) (*sandbox.ExecResult, error) { func (s *cronStubSandbox) Exec(ctx context.Context, req sandbox.ExecRequest) (*sandbox.ExecResult, error) {
return s.ExecStream(ctx, req, nil) return s.ExecStream(ctx, req, nil)
} }

View file

@ -5,6 +5,8 @@ import (
"fmt" "fmt"
"os" "os"
"strings" "strings"
"github.com/sipeed/picoclaw/pkg/agent/sandbox"
) )
// EditFileTool edits a file by replacing old_text with new_text. // EditFileTool edits a file by replacing old_text with new_text.
@ -72,12 +74,17 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
return ErrorResult(err.Error()) return ErrorResult(err.Error())
} }
if _, err = os.Stat(resolvedPath); os.IsNotExist(err) { var content []byte
return ErrorResult(fmt.Sprintf("file not found: %s", path)) sb := sandbox.SandboxFromContext(ctx)
if sb != nil {
content, err = sb.Fs().ReadFile(ctx, path)
} else {
content, err = os.ReadFile(resolvedPath)
} }
content, err := os.ReadFile(resolvedPath)
if err != nil { if err != nil {
if os.IsNotExist(err) {
return ErrorResult(fmt.Sprintf("file not found: %s", path))
}
return ErrorResult(fmt.Sprintf("failed to read file: %v", err)) return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
} }
@ -96,8 +103,14 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
newContent := strings.Replace(contentStr, oldText, newText, 1) newContent := strings.Replace(contentStr, oldText, newText, 1)
if err := os.WriteFile(resolvedPath, []byte(newContent), 0o644); err != nil { if sb != nil {
return ErrorResult(fmt.Sprintf("failed to write file: %v", err)) if err := sb.Fs().WriteFile(ctx, path, []byte(newContent), true); err != nil {
return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
}
} else {
if err := os.WriteFile(resolvedPath, []byte(newContent), 0o644); err != nil {
return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
}
} }
return SilentResult(fmt.Sprintf("File edited: %s", path)) return SilentResult(fmt.Sprintf("File edited: %s", path))
@ -153,6 +166,20 @@ func (t *AppendFileTool) Execute(ctx context.Context, args map[string]any) *Tool
return ErrorResult(err.Error()) return ErrorResult(err.Error())
} }
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
if err := sb.Fs().WriteFile(ctx, path, []byte(newContent), true); err != nil {
return ErrorResult(fmt.Sprintf("failed to append (write) to file: %v", err))
}
return SilentResult(fmt.Sprintf("Appended to %s", path))
}
f, err := os.OpenFile(resolvedPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) f, err := os.OpenFile(resolvedPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
if err != nil { if err != nil {
return ErrorResult(fmt.Sprintf("failed to open file: %v", err)) return ErrorResult(fmt.Sprintf("failed to open file: %v", err))

View file

@ -84,25 +84,12 @@ func isWithinWorkspace(candidate, workspace string) bool {
type ReadFileTool struct { type ReadFileTool struct {
workspace string workspace string
restrict bool restrict bool
fsBridge sandbox.FsBridge
} }
func NewReadFileTool(workspace string, restrict bool) *ReadFileTool { func NewReadFileTool(workspace string, restrict bool) *ReadFileTool {
return &ReadFileTool{workspace: workspace, restrict: restrict} return &ReadFileTool{workspace: workspace, restrict: restrict}
} }
func NewReadFileToolWithSandbox(workspace string, restrict bool, sb sandbox.Sandbox) *ReadFileTool {
var fsBridge sandbox.FsBridge
if sb != nil {
fsBridge = sb.Fs()
}
return &ReadFileTool{workspace: workspace, restrict: restrict, fsBridge: fsBridge}
}
func NewReadFileToolWithFsBridge(workspace string, restrict bool, fsBridge sandbox.FsBridge) *ReadFileTool {
return &ReadFileTool{workspace: workspace, restrict: restrict, fsBridge: fsBridge}
}
func (t *ReadFileTool) Name() string { func (t *ReadFileTool) Name() string {
return "read_file" return "read_file"
} }
@ -134,8 +121,9 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
content []byte content []byte
err error err error
) )
if t.fsBridge != nil { sb := sandbox.SandboxFromContext(ctx)
content, err = t.fsBridge.ReadFile(ctx, path) if sb != nil {
content, err = sb.Fs().ReadFile(ctx, path)
} else { } else {
var resolvedPath string var resolvedPath string
resolvedPath, err = validatePath(path, t.workspace, t.restrict) resolvedPath, err = validatePath(path, t.workspace, t.restrict)
@ -153,25 +141,12 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
type WriteFileTool struct { type WriteFileTool struct {
workspace string workspace string
restrict bool restrict bool
fsBridge sandbox.FsBridge
} }
func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool { func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool {
return &WriteFileTool{workspace: workspace, restrict: restrict} return &WriteFileTool{workspace: workspace, restrict: restrict}
} }
func NewWriteFileToolWithSandbox(workspace string, restrict bool, sb sandbox.Sandbox) *WriteFileTool {
var fsBridge sandbox.FsBridge
if sb != nil {
fsBridge = sb.Fs()
}
return &WriteFileTool{workspace: workspace, restrict: restrict, fsBridge: fsBridge}
}
func NewWriteFileToolWithFsBridge(workspace string, restrict bool, fsBridge sandbox.FsBridge) *WriteFileTool {
return &WriteFileTool{workspace: workspace, restrict: restrict, fsBridge: fsBridge}
}
func (t *WriteFileTool) Name() string { func (t *WriteFileTool) Name() string {
return "write_file" return "write_file"
} }
@ -208,8 +183,9 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *ToolR
return ErrorResult("content is required") return ErrorResult("content is required")
} }
if t.fsBridge != nil { sb := sandbox.SandboxFromContext(ctx)
if err := t.fsBridge.WriteFile(ctx, path, []byte(content), true); err != nil { if sb != nil {
if err := sb.Fs().WriteFile(ctx, path, []byte(content), true); err != nil {
return ErrorResult(fmt.Sprintf("failed to write file: %v", err)) return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
} }
return SilentResult(fmt.Sprintf("File written: %s", path)) return SilentResult(fmt.Sprintf("File written: %s", path))
@ -232,6 +208,8 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *ToolR
return SilentResult(fmt.Sprintf("File written: %s", path)) 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 { type ListDirTool struct {
workspace string workspace string
restrict bool restrict bool
@ -274,6 +252,7 @@ func (t *ListDirTool) Execute(ctx context.Context, args map[string]any) *ToolRes
} }
entries, err := os.ReadDir(resolvedPath) entries, err := os.ReadDir(resolvedPath)
if err != nil { if err != nil {
return ErrorResult(fmt.Sprintf("failed to read directory: %v", err)) return ErrorResult(fmt.Sprintf("failed to read directory: %v", err))
} }

View file

@ -24,7 +24,6 @@ type ExecTool struct {
denyPatterns []*regexp.Regexp denyPatterns []*regexp.Regexp
allowPatterns []*regexp.Regexp allowPatterns []*regexp.Regexp
restrictToWorkspace bool restrictToWorkspace bool
sandbox sandbox.Sandbox
} }
var defaultDenyPatterns = []*regexp.Regexp{ var defaultDenyPatterns = []*regexp.Regexp{
@ -77,10 +76,6 @@ func NewExecTool(workingDir string, restrict bool) *ExecTool {
} }
func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Config) *ExecTool { func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Config) *ExecTool {
return NewExecToolWithSandbox(workingDir, restrict, config, nil)
}
func NewExecToolWithSandbox(workingDir string, restrict bool, config *config.Config, sb sandbox.Sandbox) *ExecTool {
denyPatterns := make([]*regexp.Regexp, 0) denyPatterns := make([]*regexp.Regexp, 0)
enableDenyPatterns := true enableDenyPatterns := true
@ -115,7 +110,6 @@ func NewExecToolWithSandbox(workingDir string, restrict bool, config *config.Con
denyPatterns: denyPatterns, denyPatterns: denyPatterns,
allowPatterns: nil, allowPatterns: nil,
restrictToWorkspace: restrict, restrictToWorkspace: restrict,
sandbox: sb,
} }
} }
@ -150,14 +144,18 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
return ErrorResult("command is required") return ErrorResult("command is required")
} }
wd, _ := args["working_dir"].(string)
// Resolve the working directory
cwd := t.workingDir cwd := t.workingDir
if wd, ok := args["working_dir"].(string); ok && wd != "" { if wd != "" {
if t.restrictToWorkspace && t.workingDir != "" { if t.restrictToWorkspace && t.workingDir != "" {
resolvedWD, err := validatePath(wd, t.workingDir, true) resolvedWD, err := validatePath(wd, t.workingDir, true)
if err != nil { if err != nil {
// In sandbox mode, allow explicit container workspace paths when // In sandbox mode, allow explicit container workspace paths when
// restrict_to_workspace is enabled. // restrict_to_workspace is enabled.
if t.sandbox != nil && filepath.IsAbs(wd) && isSandboxWorkspaceAbsolutePath(wd) { sb := sandbox.SandboxFromContext(ctx)
if sb != nil && filepath.IsAbs(wd) && isSandboxWorkspaceAbsolutePath(wd) {
cwd = wd cwd = wd
} else { } else {
return ErrorResult("Command blocked by safety guard (" + err.Error() + ")") return ErrorResult("Command blocked by safety guard (" + err.Error() + ")")
@ -171,9 +169,8 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
} }
if cwd == "" { if cwd == "" {
wd, err := os.Getwd() if dir, err := os.Getwd(); err == nil {
if err == nil { cwd = dir
cwd = wd
} }
} }
@ -181,9 +178,10 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
return ErrorResult(guardError) return ErrorResult(guardError)
} }
if t.sandbox != nil { sb := sandbox.SandboxFromContext(ctx)
if sb != nil {
sandboxWD := t.resolveSandboxWorkingDir(cwd) sandboxWD := t.resolveSandboxWorkingDir(cwd)
res, err := t.sandbox.Exec(ctx, sandbox.ExecRequest{ res, err := sb.Exec(ctx, sandbox.ExecRequest{
Command: command, Command: command,
WorkingDir: sandboxWD, WorkingDir: sandboxWD,
TimeoutMs: t.timeout.Milliseconds(), TimeoutMs: t.timeout.Milliseconds(),
@ -252,7 +250,7 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
select { select {
case err = <-done: case err = <-done:
case <-cmdCtx.Done(): case <-cmdCtx.Done():
_ = terminateProcessTree(cmd) terminateProcessTree(cmd)
select { select {
case err = <-done: case err = <-done:
case <-time.After(2 * time.Second): case <-time.After(2 * time.Second):
@ -270,14 +268,19 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
if err != nil { if err != nil {
if errors.Is(cmdCtx.Err(), context.DeadlineExceeded) { if errors.Is(cmdCtx.Err(), context.DeadlineExceeded) {
msg := fmt.Sprintf("Command timed out after %v", t.timeout) msg := fmt.Sprintf("command timed out after %v", t.timeout)
return &ToolResult{ return &ToolResult{
ForLLM: msg, ForLLM: msg,
ForUser: msg, ForUser: msg,
IsError: true, IsError: true,
} }
} }
output += fmt.Sprintf("\nExit code: %v", err) var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
output += fmt.Sprintf("\nExit code: %d", exitErr.ExitCode())
} else {
output += fmt.Sprintf("\nError: %v", err)
}
} }
if output == "" { if output == "" {
@ -289,18 +292,10 @@ 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) output = output[:maxLen] + fmt.Sprintf("\n... (truncated, %d more chars)", len(output)-maxLen)
} }
if err != nil {
return &ToolResult{
ForLLM: output,
ForUser: output,
IsError: true,
}
}
return &ToolResult{ return &ToolResult{
ForLLM: output, ForLLM: output,
ForUser: output, ForUser: output,
IsError: false, IsError: err != nil,
} }
} }

View file

@ -20,7 +20,11 @@ type stubSandbox struct {
func (s *stubSandbox) Start(ctx context.Context) error { return nil } func (s *stubSandbox) Start(ctx context.Context) error { return nil }
func (s *stubSandbox) Prune(ctx context.Context) error { return nil } func (s *stubSandbox) Prune(ctx context.Context) error { return nil }
func (s *stubSandbox) Fs() sandbox.FsBridge { 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) Exec(ctx context.Context, req sandbox.ExecRequest) (*sandbox.ExecResult, error) { func (s *stubSandbox) Exec(ctx context.Context, req sandbox.ExecRequest) (*sandbox.ExecResult, error) {
return sandboxAggregateFromStub(ctx, req, s.ExecStream) return sandboxAggregateFromStub(ctx, req, s.ExecStream)
} }
@ -354,9 +358,9 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) {
func TestShellTool_SandboxMapsHostWorkingDirToRelative(t *testing.T) { func TestShellTool_SandboxMapsHostWorkingDirToRelative(t *testing.T) {
workspace := t.TempDir() workspace := t.TempDir()
sb := &stubSandbox{} sb := &stubSandbox{}
tool := NewExecToolWithSandbox(workspace, true, nil, sb) tool := NewExecTool(workspace, true)
ctx := context.Background() ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]interface{}{ args := map[string]interface{}{
"command": "echo test", "command": "echo test",
"working_dir": filepath.Join(workspace, "subdir"), "working_dir": filepath.Join(workspace, "subdir"),
@ -373,9 +377,9 @@ func TestShellTool_SandboxMapsHostWorkingDirToRelative(t *testing.T) {
func TestShellTool_SandboxAllowsAbsoluteWorkspaceWorkingDir(t *testing.T) { func TestShellTool_SandboxAllowsAbsoluteWorkspaceWorkingDir(t *testing.T) {
workspace := t.TempDir() workspace := t.TempDir()
sb := &stubSandbox{} sb := &stubSandbox{}
tool := NewExecToolWithSandbox(workspace, true, nil, sb) tool := NewExecTool(workspace, true)
ctx := context.Background() ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]interface{}{ args := map[string]interface{}{
"command": "echo test", "command": "echo test",
"working_dir": "/workspace/subdir", "working_dir": "/workspace/subdir",
@ -392,9 +396,9 @@ func TestShellTool_SandboxAllowsAbsoluteWorkspaceWorkingDir(t *testing.T) {
func TestShellTool_SandboxBlocksAbsoluteNonWorkspaceWorkingDirWhenRestricted(t *testing.T) { func TestShellTool_SandboxBlocksAbsoluteNonWorkspaceWorkingDirWhenRestricted(t *testing.T) {
workspace := t.TempDir() workspace := t.TempDir()
sb := &stubSandbox{} sb := &stubSandbox{}
tool := NewExecToolWithSandbox(workspace, true, nil, sb) tool := NewExecTool(workspace, true)
ctx := context.Background() ctx := sandbox.WithSandbox(context.Background(), sb)
args := map[string]interface{}{ args := map[string]interface{}{
"command": "echo test", "command": "echo test",
"working_dir": "/tmp/logs", "working_dir": "/tmp/logs",
@ -411,9 +415,10 @@ func TestShellTool_SandboxBlocksAbsoluteNonWorkspaceWorkingDirWhenRestricted(t *
func TestShellTool_SandboxExecError(t *testing.T) { func TestShellTool_SandboxExecError(t *testing.T) {
workspace := t.TempDir() workspace := t.TempDir()
sb := &stubSandbox{err: fmt.Errorf("sandbox down")} sb := &stubSandbox{err: fmt.Errorf("sandbox down")}
tool := NewExecToolWithSandbox(workspace, true, nil, sb) tool := NewExecTool(workspace, true)
result := tool.Execute(context.Background(), map[string]interface{}{"command": "echo test"}) ctx := sandbox.WithSandbox(context.Background(), sb)
result := tool.Execute(ctx, map[string]interface{}{"command": "echo test"})
if !result.IsError { if !result.IsError {
t.Fatal("expected sandbox error result") t.Fatal("expected sandbox error result")
} }