refactor: Rename sandbox Stop to Prune, remove DisabledTool, and introduce a centralized sandbox pruning loop.

This commit is contained in:
0x5487 2026-02-21 19:11:38 +08:00
parent 824f4f9309
commit 0f576ba3a5
16 changed files with 323 additions and 242 deletions

View file

@ -74,27 +74,15 @@ func NewAgentInstance(
roContainer := isContainerReadOnlySandbox(cfg) roContainer := isContainerReadOnlySandbox(cfg)
toolsRegistry := tools.NewToolRegistry() toolsRegistry := tools.NewToolRegistry()
toolsRegistry.Register(tools.NewReadFileToolWithSandbox(workspace, restrict, readSb)) toolsRegistry.Register(tools.NewReadFileToolWithSandbox(workspace, restrict, readSb))
if roContainer { if !roContainer {
toolsRegistry.Register(tools.NewDisabledTool(
"write_file",
"Write content to a file",
"write_file is disabled when sandbox workspace_access=ro",
))
} else {
toolsRegistry.Register(tools.NewWriteFileToolWithSandbox(workspace, restrict, writeSb)) toolsRegistry.Register(tools.NewWriteFileToolWithSandbox(workspace, restrict, writeSb))
} }
toolsRegistry.Register(tools.NewListDirTool(workspace, restrict)) toolsRegistry.Register(tools.NewListDirTool(workspace, restrict))
toolsRegistry.Register(tools.NewExecToolWithSandbox(workspace, restrict, cfg, execSb)) toolsRegistry.Register(tools.NewExecToolWithSandbox(workspace, restrict, cfg, execSb))
if roContainer { if !roContainer {
toolsRegistry.Register(tools.NewDisabledTool(
"edit_file",
"Edit a file by replacing old_text with new_text. The old_text must exist exactly in the file.",
"edit_file is disabled when sandbox workspace_access=ro",
))
} else {
toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict)) toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict))
}
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict)) toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
}
sessionsDir := filepath.Join(workspace, "sessions") sessionsDir := filepath.Join(workspace, "sessions")
sessionsManager := session.NewSessionManager(sessionsDir) sessionsManager := session.NewSessionManager(sessionsDir)

View file

@ -96,7 +96,7 @@ func TestNewAgentInstance_DefaultsTemperatureWhenUnset(t *testing.T) {
} }
} }
func TestNewAgentInstance_ReadOnlyContainerDisablesWriteAndEdit(t *testing.T) { func TestNewAgentInstance_ReadOnlyContainerOmitsWriteTools(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "agent-instance-test-*") tmpDir, err := os.MkdirTemp("", "agent-instance-test-*")
if err != nil { if err != nil {
t.Fatalf("Failed to create temp dir: %v", err) t.Fatalf("Failed to create temp dir: %v", err)
@ -121,20 +121,17 @@ func TestNewAgentInstance_ReadOnlyContainerDisablesWriteAndEdit(t *testing.T) {
provider := &mockProvider{} provider := &mockProvider{}
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider) agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider)
for _, name := range []string{"write_file", "edit_file", "append_file"} {
if _, ok := agent.Tools.Get(name); ok {
t.Fatalf("%s should not be registered in ro sandbox", name)
}
}
writeRes := agent.Tools.Execute(context.Background(), "write_file", map[string]interface{}{ writeRes := agent.Tools.Execute(context.Background(), "write_file", map[string]interface{}{
"path": "a.txt", "path": "a.txt",
"content": "hello", "content": "hello",
}) })
if !writeRes.IsError || !strings.Contains(writeRes.ForLLM, "workspace_access=ro") { if !writeRes.IsError || !strings.Contains(writeRes.ForLLM, "not found") {
t.Fatalf("write_file should be disabled in ro sandbox, got: %+v", writeRes) t.Fatalf("write_file should be absent in ro sandbox, got: %+v", writeRes)
}
editRes := agent.Tools.Execute(context.Background(), "edit_file", map[string]interface{}{
"path": "a.txt",
"old_text": "h",
"new_text": "H",
})
if !editRes.IsError || !strings.Contains(editRes.ForLLM, "workspace_access=ro") {
t.Fatalf("edit_file should be disabled in ro sandbox, got: %+v", editRes)
} }
} }

View file

@ -14,7 +14,6 @@ import (
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
"sync"
"time" "time"
"github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/container"
@ -64,11 +63,10 @@ type ContainerSandbox struct {
startErr error startErr error
fs FsBridge fs FsBridge
hash string hash string
loopMu sync.Mutex
loopStop context.CancelFunc
loopDone chan struct{}
} }
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) == "" {
@ -103,7 +101,7 @@ func NewContainerSandbox(cfg ContainerSandboxConfig) *ContainerSandbox {
return sb return sb
} }
// Start initializes docker connectivity, validates config, and starts background prune scheduling. // 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 {
if err := validateSandboxSecurity(c.cfg); err != nil { if err := validateSandboxSecurity(c.cfg); err != nil {
c.startErr = err c.startErr = err
@ -148,74 +146,28 @@ func (c *ContainerSandbox) Start(ctx context.Context) error {
} }
c.startErr = nil c.startErr = nil
_ = c.maybePrune(ctx)
c.ensurePruneLoop()
return nil return nil
} }
// Stop terminates background prune scheduling and stops the managed container if present. // Prune reclaims container sandbox resources.
func (c *ContainerSandbox) Stop(ctx context.Context) error { // This is the container-specific cleanup boundary where implementations should
c.stopPruneLoop(ctx) // stop and remove this sandbox container.
if c.cli == nil { func (c *ContainerSandbox) Prune(ctx context.Context) error {
return nil containerName := strings.TrimSpace(c.cfg.ContainerName)
} if containerName == "" {
if c.cfg.ContainerName != "" {
_ = c.cli.ContainerStop(ctx, c.cfg.ContainerName, container.StopOptions{})
}
return nil return nil
} }
func (c *ContainerSandbox) ensurePruneLoop() { var firstErr error
if c.cfg.PruneIdleHours <= 0 && c.cfg.PruneMaxAgeDays <= 0 { if c.cli != nil {
return if err := c.stopAndRemoveContainer(ctx, containerName); err != nil {
} firstErr = err
c.loopMu.Lock()
defer c.loopMu.Unlock()
if c.loopStop != nil {
return
}
loopCtx, cancel := context.WithCancel(context.Background())
done := make(chan struct{})
c.loopStop = cancel
c.loopDone = done
go func() {
ticker := time.NewTicker(5 * time.Minute)
defer func() {
ticker.Stop()
close(done)
}()
for {
select {
case <-loopCtx.Done():
return
case <-ticker.C:
_ = c.maybePrune(loopCtx)
} }
} }
}() if err := removeRegistryEntry(c.registryPath(), containerName); err != nil && firstErr == nil {
} firstErr = err
func (c *ContainerSandbox) stopPruneLoop(ctx context.Context) {
if ctx == nil {
ctx = context.Background()
}
c.loopMu.Lock()
stop := c.loopStop
done := c.loopDone
c.loopStop = nil
c.loopDone = nil
c.loopMu.Unlock()
if stop == nil {
return
}
stop()
if done == nil {
return
}
select {
case <-done:
case <-ctx.Done():
} }
return firstErr
} }
// Exec ensures the container is ready and runs the requested command inside the sandbox. // Exec ensures the container is ready and runs the requested command inside the sandbox.
@ -306,8 +258,6 @@ func (c *ContainerSandbox) Fs() FsBridge {
} }
func (c *ContainerSandbox) ensureContainer(ctx context.Context) error { func (c *ContainerSandbox) ensureContainer(ctx context.Context) error {
_ = c.maybePrune(ctx)
inspect, err := c.cli.ContainerInspect(ctx, c.cfg.ContainerName) inspect, err := c.cli.ContainerInspect(ctx, c.cfg.ContainerName)
if err != nil { if err != nil {
return c.createAndStart(ctx) return c.createAndStart(ctx)
@ -421,37 +371,49 @@ func (c *ContainerSandbox) binds() []string {
} }
func (c *ContainerSandbox) registryPath() string { func (c *ContainerSandbox) registryPath() string {
root := strings.TrimSpace(c.cfg.WorkspaceRoot) return filepath.Join(c.sandboxStateDir(), defaultSandboxRegistryFile)
if root == "" {
root = strings.TrimSpace(c.cfg.Workspace)
}
if root == "" {
root = osTempDir()
}
return filepath.Join(root, "state", "registry.json")
} }
func (c *ContainerSandbox) maybePrune(ctx context.Context) error { func (c *ContainerSandbox) sandboxStateDir() string {
if c.cfg.PruneIdleHours <= 0 && c.cfg.PruneMaxAgeDays <= 0 { return filepath.Join(resolvePicoClawHomeDir(), "state", "sandbox")
}
func resolvePicoClawHomeDir() string {
if envHome := strings.TrimSpace(os.Getenv("PICOCLAW_HOME")); envHome != "" {
if abs := resolveAbsPath(expandHomePath(envHome)); strings.TrimSpace(abs) != "" {
return abs
}
}
if home, err := os.UserHomeDir(); err == nil && strings.TrimSpace(home) != "" {
return filepath.Join(home, ".picoclaw")
}
return filepath.Join(osTempDir(), ".picoclaw")
}
func (c *ContainerSandbox) stopAndRemoveContainer(ctx context.Context, containerName string) error {
timeout := 10
_ = c.cli.ContainerStop(ctx, containerName, container.StopOptions{Timeout: &timeout})
if err := c.cli.ContainerRemove(ctx, containerName, container.RemoveOptions{Force: true}); err != nil {
return err
}
return nil return nil
} }
if c.cli == nil {
func stopAndRemoveContainerByName(ctx context.Context, containerName string) error {
name := strings.TrimSpace(containerName)
if name == "" {
return nil return nil
} }
regPath := c.registryPath() cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
registryMu.Lock()
data, err := loadRegistry(regPath)
registryMu.Unlock()
if err != nil { if err != nil {
return err return err
} }
now := time.Now().UnixMilli() defer cli.Close()
for _, e := range data.Entries {
if !shouldPruneEntry(c.cfg, now, e) { timeout := 10
continue _ = cli.ContainerStop(ctx, name, container.StopOptions{Timeout: &timeout})
} if err := cli.ContainerRemove(ctx, name, container.RemoveOptions{Force: true}); err != nil {
_ = c.cli.ContainerRemove(ctx, e.ContainerName, container.RemoveOptions{Force: true}) return err
_ = removeRegistryEntry(regPath, e.ContainerName)
} }
return nil return nil
} }

View file

@ -9,7 +9,6 @@ import (
"testing" "testing"
"time" "time"
"github.com/docker/docker/client"
"github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/config"
) )
@ -24,7 +23,7 @@ func TestContainerSandbox_StartCreatesWorkspaceBeforeDockerPing(t *testing.T) {
err := sb.Start(context.Background()) err := sb.Start(context.Background())
if err == nil { if err == nil {
_ = sb.Stop(context.Background()) _ = sb.Prune(context.Background())
t.Skip("docker daemon available in this environment; skip unavailable-path assertion") t.Skip("docker daemon available in this environment; skip unavailable-path assertion")
} }
if !strings.Contains(err.Error(), "docker daemon unavailable") { if !strings.Contains(err.Error(), "docker daemon unavailable") {
@ -38,50 +37,14 @@ func TestContainerSandbox_StartCreatesWorkspaceBeforeDockerPing(t *testing.T) {
} }
} }
func TestContainerSandbox_PruneLoopLifecycleAndNoopPrune(t *testing.T) { func TestContainerSandbox_NoopPruneWithoutClient(t *testing.T) {
sb := NewContainerSandbox(ContainerSandboxConfig{ sb := NewContainerSandbox(ContainerSandboxConfig{
PruneIdleHours: 1, PruneIdleHours: 1,
PruneMaxAgeDays: 0, PruneMaxAgeDays: 0,
}) })
sb.ensurePruneLoop() if err := sb.Prune(context.Background()); err != nil {
if sb.loopStop == nil || sb.loopDone == nil { t.Fatalf("Prune() with nil client should be noop, got: %v", err)
t.Fatal("expected prune loop to start")
}
sb.stopPruneLoop(nil)
if sb.loopStop != nil || sb.loopDone != nil {
t.Fatal("expected prune loop state reset after stop")
}
if err := sb.maybePrune(context.Background()); err != nil {
t.Fatalf("maybePrune() with nil client should be noop, got: %v", err)
}
}
func TestContainerSandbox_MaybePruneDisabledAndLoadError(t *testing.T) {
disabled := NewContainerSandbox(ContainerSandboxConfig{})
if err := disabled.maybePrune(context.Background()); err != nil {
t.Fatalf("maybePrune() should return nil when both prune rules disabled: %v", err)
}
root := t.TempDir()
stateDir := filepath.Join(root, "state")
if err := os.MkdirAll(stateDir, 0o755); err != nil {
t.Fatalf("mkdir state dir: %v", err)
}
regPath := filepath.Join(stateDir, "registry.json")
if err := os.WriteFile(regPath, []byte("{not-json"), 0o644); err != nil {
t.Fatalf("write invalid registry: %v", err)
}
sb := NewContainerSandbox(ContainerSandboxConfig{
WorkspaceRoot: root,
PruneIdleHours: 1,
PruneMaxAgeDays: 0,
})
sb.cli = &client.Client{}
if err := sb.maybePrune(context.Background()); err == nil {
t.Fatal("expected maybePrune() to return registry load error")
} }
} }
@ -215,11 +178,10 @@ func TestContainerSandbox_StopWithoutClient(t *testing.T) {
PruneIdleHours: 1, PruneIdleHours: 1,
PruneMaxAgeDays: 1, PruneMaxAgeDays: 1,
}) })
sb.ensurePruneLoop()
ctx, cancel := context.WithTimeout(context.Background(), time.Second) ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel() defer cancel()
if err := sb.Stop(ctx); err != nil { if err := sb.Prune(ctx); err != nil {
t.Fatalf("Stop() error: %v", err) t.Fatalf("Prune() error: %v", err)
} }
} }

View file

@ -47,7 +47,7 @@ func TestContainerSandbox_Integration_ExecReadWrite(t *testing.T) {
t.Fatalf("sandbox start failed: %v", err) t.Fatalf("sandbox start failed: %v", err)
} }
defer func() { defer func() {
_ = sb.Stop(context.Background()) _ = sb.Prune(context.Background())
if sb.cli != nil { if sb.cli != nil {
_ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true}) _ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true})
} }
@ -134,7 +134,7 @@ func TestContainerSandbox_Integration_WriteFileMkdirInContainerTmp(t *testing.T)
t.Fatalf("sandbox start failed: %v", err) t.Fatalf("sandbox start failed: %v", err)
} }
defer func() { defer func() {
_ = sb.Stop(context.Background()) _ = sb.Prune(context.Background())
if sb.cli != nil { if sb.cli != nil {
_ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true}) _ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true})
} }
@ -183,7 +183,7 @@ func TestContainerSandbox_Integration_SetupCommandSuccess(t *testing.T) {
t.Fatalf("sandbox start failed: %v", err) t.Fatalf("sandbox start failed: %v", err)
} }
defer func() { defer func() {
_ = sb.Stop(context.Background()) _ = sb.Prune(context.Background())
if sb.cli != nil { if sb.cli != nil {
_ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true}) _ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true})
} }
@ -227,7 +227,7 @@ func TestContainerSandbox_Integration_SetupCommandFailureRemovesContainer(t *tes
t.Fatalf("sandbox start failed: %v", err) t.Fatalf("sandbox start failed: %v", err)
} }
defer func() { defer func() {
_ = sb.Stop(context.Background()) _ = sb.Prune(context.Background())
if sb.cli != nil { if sb.cli != nil {
_ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true}) _ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true})
} }
@ -271,7 +271,7 @@ func TestContainerSandbox_Integration_MaybePruneRemovesOldContainer(t *testing.T
t.Fatalf("sandbox start failed: %v", err) t.Fatalf("sandbox start failed: %v", err)
} }
defer func() { defer func() {
_ = sb.Stop(context.Background()) _ = sb.Prune(context.Background())
if sb.cli != nil { if sb.cli != nil {
_ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true}) _ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true})
} }
@ -292,8 +292,15 @@ func TestContainerSandbox_Integration_MaybePruneRemovesOldContainer(t *testing.T
t.Fatalf("upsert old registry entry failed: %v", err) t.Fatalf("upsert old registry entry failed: %v", err)
} }
if err := sb.maybePrune(ctx); err != nil { manager := &scopedSandboxManager{
t.Fatalf("maybePrune failed: %v", err) pruneIdleHours: 1,
pruneMaxAgeDays: 0,
scoped: map[string]Sandbox{
"agent:main": sb,
},
}
if err := manager.pruneOnce(ctx); err != nil {
t.Fatalf("pruneOnce failed: %v", err)
} }
if _, err := sb.cli.ContainerInspect(ctx, containerName); err == nil { if _, err := sb.cli.ContainerInspect(ctx, containerName); err == nil {
@ -333,7 +340,7 @@ func TestContainerSandbox_Integration_ExecTimeoutRespectsRequest(t *testing.T) {
t.Fatalf("sandbox start failed: %v", err) t.Fatalf("sandbox start failed: %v", err)
} }
defer func() { defer func() {
_ = sb.Stop(context.Background()) _ = sb.Prune(context.Background())
if sb.cli != nil { if sb.cli != nil {
_ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true}) _ = sb.cli.ContainerRemove(context.Background(), containerName, container.RemoveOptions{Force: true})
} }

View file

@ -260,13 +260,26 @@ func TestContainerSandbox_Binds_WorkspaceAccessModes(t *testing.T) {
} }
} }
func TestContainerSandbox_RegistryPath_UsesWorkspaceRoot(t *testing.T) { func TestContainerSandbox_RegistryPath_UsesSandboxStateDir(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
sb := NewContainerSandbox(ContainerSandboxConfig{ sb := NewContainerSandbox(ContainerSandboxConfig{
Workspace: "/tmp/ws", Workspace: "/tmp/ws",
WorkspaceRoot: "/tmp/sbx", WorkspaceRoot: "/tmp/sbx",
}) })
if got := sb.registryPath(); got != "/tmp/sbx/state/registry.json" { want := filepath.Join(home, ".picoclaw", "state", "sandbox", "containers.json")
t.Fatalf("registryPath = %q, want %q", got, "/tmp/sbx/state/registry.json") if got := sb.registryPath(); got != want {
t.Fatalf("registryPath = %q, want %q", got, want)
}
}
func TestContainerSandbox_RegistryPath_UsesPicoClawHomeOverride(t *testing.T) {
picoHome := t.TempDir()
t.Setenv("PICOCLAW_HOME", picoHome)
sb := NewContainerSandbox(ContainerSandboxConfig{})
want := filepath.Join(picoHome, "state", "sandbox", "containers.json")
if got := sb.registryPath(); got != want {
t.Fatalf("registryPath = %q, want %q", got, want)
} }
} }

View file

@ -10,6 +10,7 @@ import (
"regexp" "regexp"
"strings" "strings"
"sync" "sync"
"time"
"github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/config"
"github.com/sipeed/picoclaw/pkg/routing" "github.com/sipeed/picoclaw/pkg/routing"
@ -22,7 +23,7 @@ func NewFromConfig(workspace string, restrict bool, cfg *config.Config) Sandbox
// NewFromConfigWithAgent builds a sandbox instance with an explicit agent ID context. // NewFromConfigWithAgent builds a sandbox instance with an explicit agent ID context.
func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config, agentID string) Sandbox { func NewFromConfigWithAgent(workspace string, restrict bool, cfg *config.Config, agentID string) Sandbox {
mode := "off" mode := "all"
scope := "agent" scope := "agent"
workspaceAccess := "none" workspaceAccess := "none"
workspaceRoot := "~/.picoclaw/sandboxes" workspaceRoot := "~/.picoclaw/sandboxes"
@ -172,17 +173,26 @@ type scopedSandboxManager struct {
mu sync.Mutex mu sync.Mutex
scoped map[string]Sandbox scoped map[string]Sandbox
fs FsBridge fs FsBridge
loopMu sync.Mutex
loopStop context.CancelFunc
loopDone chan struct{}
} }
func (m *scopedSandboxManager) Start(ctx context.Context) error { func (m *scopedSandboxManager) Start(ctx context.Context) error {
if m.mode == "off" { if m.mode == "off" {
return nil return nil
} }
_, err := m.getOrCreateSandbox(ctx, m.defaultScopeKey()) if _, err := m.getOrCreateSandbox(ctx, m.defaultScopeKey()); err != nil {
return err return err
} }
m.ensurePruneLoop()
return nil
}
func (m *scopedSandboxManager) Prune(ctx context.Context) error {
m.stopPruneLoop(ctx)
func (m *scopedSandboxManager) Stop(ctx context.Context) error {
m.mu.Lock() m.mu.Lock()
scoped := make([]Sandbox, 0, len(m.scoped)) scoped := make([]Sandbox, 0, len(m.scoped))
for _, sb := range m.scoped { for _, sb := range m.scoped {
@ -192,13 +202,118 @@ func (m *scopedSandboxManager) Stop(ctx context.Context) error {
var firstErr error var firstErr error
for _, sb := range scoped { for _, sb := range scoped {
if err := sb.Stop(ctx); err != nil && firstErr == nil { if err := sb.Prune(ctx); err != nil && firstErr == nil {
firstErr = err firstErr = err
} }
} }
return firstErr return firstErr
} }
func (m *scopedSandboxManager) ensurePruneLoop() {
if m.pruneIdleHours <= 0 && m.pruneMaxAgeDays <= 0 {
return
}
m.loopMu.Lock()
defer m.loopMu.Unlock()
if m.loopStop != nil {
return
}
loopCtx, cancel := context.WithCancel(context.Background())
done := make(chan struct{})
m.loopStop = cancel
m.loopDone = done
go func() {
ticker := time.NewTicker(5 * time.Minute)
defer func() {
ticker.Stop()
close(done)
}()
for {
select {
case <-loopCtx.Done():
return
case <-ticker.C:
_ = m.pruneOnce(loopCtx)
}
}
}()
}
func (m *scopedSandboxManager) stopPruneLoop(ctx context.Context) {
if ctx == nil {
ctx = context.Background()
}
m.loopMu.Lock()
stop := m.loopStop
done := m.loopDone
m.loopStop = nil
m.loopDone = nil
m.loopMu.Unlock()
if stop == nil {
return
}
stop()
if done == nil {
return
}
select {
case <-done:
case <-ctx.Done():
}
}
func (m *scopedSandboxManager) pruneOnce(ctx context.Context) error {
if m.pruneIdleHours <= 0 && m.pruneMaxAgeDays <= 0 {
return nil
}
regPath := filepath.Join(resolvePicoClawHomeDir(), "state", "sandbox", defaultSandboxRegistryFile)
registryMu.Lock()
data, err := loadRegistry(regPath)
registryMu.Unlock()
if err != nil {
return err
}
pruneCfg := ContainerSandboxConfig{
PruneIdleHours: m.pruneIdleHours,
PruneMaxAgeDays: m.pruneMaxAgeDays,
}
now := time.Now().UnixMilli()
m.mu.Lock()
byContainer := make(map[string]Sandbox, len(m.scoped))
for _, sb := range m.scoped {
if containerSb, ok := sb.(*ContainerSandbox); ok {
byContainer[containerSb.cfg.ContainerName] = sb
}
}
m.mu.Unlock()
var firstErr error
for _, entry := range data.Entries {
if !shouldPruneEntry(pruneCfg, now, entry) {
continue
}
if sb, ok := byContainer[entry.ContainerName]; ok {
if err := sb.Prune(ctx); err != nil && firstErr == nil {
firstErr = err
}
continue
}
if err := stopAndRemoveContainerByName(ctx, entry.ContainerName); err != nil && firstErr == nil {
firstErr = err
}
if err := removeRegistryEntry(regPath, entry.ContainerName); err != nil && firstErr == nil {
firstErr = err
}
}
return firstErr
}
func (m *scopedSandboxManager) Exec(ctx context.Context, req ExecRequest) (*ExecResult, error) { func (m *scopedSandboxManager) Exec(ctx context.Context, req ExecRequest) (*ExecResult, error) {
if !m.shouldSandbox(ctx) { if !m.shouldSandbox(ctx) {
return m.host.Exec(ctx, req) return m.host.Exec(ctx, req)

View file

@ -2,7 +2,10 @@ package sandbox
import ( import (
"context" "context"
"os"
"path/filepath"
"testing" "testing"
"time"
"github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/config"
) )
@ -30,8 +33,8 @@ func TestNewFromConfig_HostMode(t *testing.T) {
if _, ok := sb.(*HostSandbox); !ok { if _, ok := sb.(*HostSandbox); !ok {
t.Fatalf("expected HostSandbox, got %T", sb) t.Fatalf("expected HostSandbox, got %T", sb)
} }
if err := sb.Stop(context.Background()); err != nil { if err := sb.Prune(context.Background()); err != nil {
t.Fatalf("Stop() error: %v", err) t.Fatalf("Prune() error: %v", err)
} }
} }
@ -50,3 +53,47 @@ func TestNewFromConfig_AllModeReturnsUnavailableWhenBlocked(t *testing.T) {
t.Fatal("expected unavailable sandbox start error") t.Fatal("expected unavailable sandbox start error")
} }
} }
func TestScopedSandboxManager_PruneLoopLifecycle(t *testing.T) {
m := &scopedSandboxManager{
mode: "all",
pruneIdleHours: 1,
pruneMaxAgeDays: 0,
scoped: map[string]Sandbox{},
}
m.ensurePruneLoop()
if m.loopStop == nil || m.loopDone == nil {
t.Fatal("expected manager prune loop to start")
}
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
m.stopPruneLoop(ctx)
if m.loopStop != nil || m.loopDone != nil {
t.Fatal("expected manager prune loop state reset after stop")
}
}
func TestScopedSandboxManager_PruneOnceLoadRegistryError(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
stateDir := filepath.Join(home, ".picoclaw", "state", "sandbox")
if err := os.MkdirAll(stateDir, 0o755); err != nil {
t.Fatalf("mkdir state dir: %v", err)
}
regPath := filepath.Join(stateDir, "containers.json")
if err := os.WriteFile(regPath, []byte("{not-json"), 0o644); err != nil {
t.Fatalf("write invalid registry: %v", err)
}
m := &scopedSandboxManager{
mode: "all",
pruneIdleHours: 1,
pruneMaxAgeDays: 0,
scoped: map[string]Sandbox{},
}
if err := m.pruneOnce(context.Background()); err == nil {
t.Fatal("expected pruneOnce() to return registry load error")
}
}

View file

@ -31,7 +31,7 @@ func (h *HostSandbox) Start(ctx context.Context) error {
return nil return nil
} }
func (h *HostSandbox) Stop(ctx context.Context) error { func (h *HostSandbox) Prune(ctx context.Context) error {
return nil return nil
} }

View file

@ -15,8 +15,8 @@ func TestHostSandbox_StartStopFs(t *testing.T) {
if err := sb.Start(context.Background()); err != nil { if err := sb.Start(context.Background()); err != nil {
t.Fatalf("Start() error: %v", err) t.Fatalf("Start() error: %v", err)
} }
if err := sb.Stop(context.Background()); err != nil { if err := sb.Prune(context.Background()); err != nil {
t.Fatalf("Stop() error: %v", err) t.Fatalf("Prune() error: %v", err)
} }
if sb.Fs() == nil { if sb.Fs() == nil {
t.Fatal("Fs() returned nil") t.Fatal("Fs() returned nil")
@ -110,8 +110,8 @@ func TestUnavailableSandboxAndUtilHelpers(t *testing.T) {
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")
} }
if err := sb.Stop(context.Background()); err != nil { if err := sb.Prune(context.Background()); err != nil {
t.Fatalf("Stop() error: %v", err) t.Fatalf("Prune() error: %v", err)
} }
if _, err := sb.Exec(context.Background(), ExecRequest{Command: "echo hi"}); err == nil { if _, err := sb.Exec(context.Background(), ExecRequest{Command: "echo hi"}); err == nil {
t.Fatal("expected Exec() error") t.Fatal("expected Exec() error")

View file

@ -11,9 +11,11 @@ type Sandbox interface {
// Implementations should prepare resources that are expensive to set up lazily // Implementations should prepare resources that are expensive to set up lazily
// (for example, container client connectivity checks). // (for example, container client connectivity checks).
Start(ctx context.Context) error Start(ctx context.Context) error
// Stop releases runtime resources acquired by Start. // Prune performs sandbox resource reclamation.
// Implementations should release reclaimable runtime resources and remove
// sandbox artifacts (for example containers) according to their policy.
// It should be safe to call multiple times. // It should be safe to call multiple times.
Stop(ctx context.Context) error Prune(ctx context.Context) error
// Exec runs a command in sandbox context. // Exec runs a command in sandbox context.
// Command/Args semantics follow ExecRequest; a non-zero exit code should be // Command/Args semantics follow ExecRequest; a non-zero exit code should be
// returned in ExecResult.ExitCode, while transport/runtime failures return error. // returned in ExecResult.ExitCode, while transport/runtime failures return error.

View file

@ -22,7 +22,7 @@ func NewUnavailableSandbox(err error) Sandbox {
} }
func (u *unavailableSandbox) Start(ctx context.Context) error { return u.err } func (u *unavailableSandbox) Start(ctx context.Context) error { return u.err }
func (u *unavailableSandbox) Stop(ctx context.Context) error { return nil } func (u *unavailableSandbox) Prune(ctx context.Context) error { return nil }
func (u *unavailableSandbox) Fs() FsBridge { return u.fs } func (u *unavailableSandbox) Fs() FsBridge { return u.fs }
func (u *unavailableSandbox) Exec(ctx context.Context, req ExecRequest) (*ExecResult, error) { func (u *unavailableSandbox) Exec(ctx context.Context, req ExecRequest) (*ExecResult, error) {
return aggregateExecStream(func(onEvent func(ExecEvent) error) (*ExecResult, error) { return aggregateExecStream(func(onEvent func(ExecEvent) error) (*ExecResult, error) {

View file

@ -19,7 +19,7 @@ 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) Stop(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) 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

@ -1,46 +0,0 @@
package tools
import (
"context"
)
// DisabledTool keeps a stable tool surface but always returns an error result.
type DisabledTool struct {
name string
description string
reason string
}
func NewDisabledTool(name, description, reason string) *DisabledTool {
return &DisabledTool{
name: name,
description: description,
reason: reason,
}
}
func (t *DisabledTool) Name() string {
return t.name
}
func (t *DisabledTool) Description() string {
if t.description != "" {
return t.description
}
return "This tool is disabled in current sandbox policy."
}
func (t *DisabledTool) Parameters() map[string]interface{} {
return map[string]interface{}{
"type": "object",
"properties": map[string]interface{}{},
}
}
func (t *DisabledTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult {
msg := t.reason
if msg == "" {
msg = "tool is disabled"
}
return ErrorResult(msg)
}

View file

@ -7,6 +7,7 @@ import (
"fmt" "fmt"
"os" "os"
"os/exec" "os/exec"
"path"
"path/filepath" "path/filepath"
"regexp" "regexp"
"runtime" "runtime"
@ -154,9 +155,16 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
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
// restrict_to_workspace is enabled.
if t.sandbox != nil && filepath.IsAbs(wd) && isSandboxWorkspaceAbsolutePath(wd) {
cwd = wd
} else {
return ErrorResult("Command blocked by safety guard (" + err.Error() + ")") return ErrorResult("Command blocked by safety guard (" + err.Error() + ")")
} }
} else {
cwd = resolvedWD cwd = resolvedWD
}
} else { } else {
cwd = wd cwd = wd
} }
@ -390,7 +398,14 @@ func (t *ExecTool) resolveSandboxWorkingDir(cwd string) string {
} }
} }
} }
return "." // Preserve explicit absolute paths in sandbox mode (e.g. /tmp/logs),
// instead of silently downgrading to ".".
return filepath.ToSlash(trimmed)
}
func isSandboxWorkspaceAbsolutePath(wd string) bool {
clean := path.Clean(filepath.ToSlash(strings.TrimSpace(wd)))
return clean == "/workspace" || strings.HasPrefix(clean, "/workspace/")
} }
func (t *ExecTool) SetTimeout(timeout time.Duration) { func (t *ExecTool) SetTimeout(timeout time.Duration) {

View file

@ -19,7 +19,7 @@ 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) Stop(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) 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)
@ -370,7 +370,7 @@ func TestShellTool_SandboxMapsHostWorkingDirToRelative(t *testing.T) {
} }
} }
func TestShellTool_SandboxUsesDotForUnmappedAbsoluteDir(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 := NewExecToolWithSandbox(workspace, true, nil, sb)
@ -378,14 +378,33 @@ func TestShellTool_SandboxUsesDotForUnmappedAbsoluteDir(t *testing.T) {
ctx := context.Background() ctx := context.Background()
args := map[string]interface{}{ args := map[string]interface{}{
"command": "echo test", "command": "echo test",
"working_dir": "/outside/path", "working_dir": "/workspace/subdir",
} }
result := tool.Execute(ctx, args) result := tool.Execute(ctx, args)
if result.IsError { if result.IsError {
t.Fatalf("expected success, got error: %s", result.ForLLM) t.Fatalf("expected success, got error: %s", result.ForLLM)
} }
if sb.lastReq.WorkingDir != "." { if sb.lastReq.WorkingDir != "/workspace/subdir" {
t.Fatalf("sandbox working_dir = %q, want .", sb.lastReq.WorkingDir) t.Fatalf("sandbox working_dir = %q, want /workspace/subdir", sb.lastReq.WorkingDir)
}
}
func TestShellTool_SandboxBlocksAbsoluteNonWorkspaceWorkingDirWhenRestricted(t *testing.T) {
workspace := t.TempDir()
sb := &stubSandbox{}
tool := NewExecToolWithSandbox(workspace, true, nil, sb)
ctx := context.Background()
args := map[string]interface{}{
"command": "echo test",
"working_dir": "/tmp/logs",
}
result := tool.Execute(ctx, args)
if !result.IsError {
t.Fatalf("expected error for /tmp/logs with restrict_to_workspace=true, got: %s", result.ForLLM)
}
if !strings.Contains(result.ForLLM, "blocked") {
t.Fatalf("expected blocked error, got: %s", result.ForLLM)
} }
} }