refactor: Introduce sandbox manager, consolidate sandbox components, and restructure tests.

Dockerfile.sandbox

@@ -0,0 +1,20 @@
+FROM debian:bookworm-slim@sha256:98f4b71de414932439ac6ac690d7060df1f27161073c5036a7553723881bffbe
+
+ENV DEBIAN_FRONTEND=noninteractive
+
+RUN apt-get update \
+  && apt-get install -y --no-install-recommends \
+    bash \
+    ca-certificates \
+    curl \
+    git \
+    jq \
+    python3 \
+    ripgrep \
+  && rm -rf /var/lib/apt/lists/*
+
+RUN useradd --create-home --shell /bin/bash sandbox
+USER sandbox
+WORKDIR /home/sandbox
+
+CMD ["sleep", "infinity"]

pkg/agent/sandbox/container.go

@@ -70,7 +70,7 @@ const defaultSandboxRegistryFile = "containers.json"
 // NewContainerSandbox creates a container sandbox with normalized defaults and precomputed config hash.
 func NewContainerSandbox(cfg ContainerSandboxConfig) *ContainerSandbox {
 	if strings.TrimSpace(cfg.Image) == "" {
-		cfg.Image = "debian:bookworm-slim"
+		cfg.Image = "openclaw-sandbox:bookworm-slim"
 	}
 	if strings.TrimSpace(cfg.ContainerPrefix) == "" {
 		cfg.ContainerPrefix = "picoclaw-sandbox-"

pkg/agent/sandbox/container_additional_test.go

@@ -1,187 +0,0 @@
-package sandbox
-
-import (
-	"context"
-	"os"
-	"path/filepath"
-	"sort"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/sipeed/picoclaw/pkg/config"
-)
-
-func TestContainerSandbox_StartCreatesWorkspaceBeforeDockerPing(t *testing.T) {
-	workspace := filepath.Join(t.TempDir(), "workspace")
-	workspaceRoot := filepath.Join(t.TempDir(), "sandbox-root")
-	sb := NewContainerSandbox(ContainerSandboxConfig{
-		Workspace:       workspace,
-		WorkspaceRoot:   workspaceRoot,
-		WorkspaceAccess: "none",
-	})
-
-	err := sb.Start(context.Background())
-	if err == nil {
-		_ = sb.Prune(context.Background())
-		t.Skip("docker daemon available in this environment; skip unavailable-path assertion")
-	}
-	if !strings.Contains(err.Error(), "docker daemon unavailable") {
-		t.Fatalf("Start() unexpected error: %v", err)
-	}
-	if _, stErr := os.Stat(workspace); stErr != nil {
-		t.Fatalf("workspace should be created before docker ping: %v", stErr)
-	}
-	if _, stErr := os.Stat(workspaceRoot); stErr != nil {
-		t.Fatalf("workspaceRoot should be created before docker ping: %v", stErr)
-	}
-}
-
-func TestContainerSandbox_NoopPruneWithoutClient(t *testing.T) {
-	sb := NewContainerSandbox(ContainerSandboxConfig{
-		PruneIdleHours:  1,
-		PruneMaxAgeDays: 0,
-	})
-
-	if err := sb.Prune(context.Background()); err != nil {
-		t.Fatalf("Prune() with nil client should be noop, got: %v", err)
-	}
-}
-
-func TestParseByteLimitAndHostConfig(t *testing.T) {
-	if got, err := parseByteLimit("1024"); err != nil || got != 1024 {
-		t.Fatalf("parseByteLimit numeric got (%d,%v), want (1024,nil)", got, err)
-	}
-	if got, err := parseByteLimit("1g"); err != nil || got <= 0 {
-		t.Fatalf("parseByteLimit unit got (%d,%v), want positive", got, err)
-	}
-	if _, err := parseByteLimit("not-a-size"); err == nil {
-		t.Fatal("expected parseByteLimit invalid input error")
-	}
-
-	soft := int64(256)
-	hard := int64(512)
-	sb := NewContainerSandbox(ContainerSandboxConfig{
-		Workspace:       t.TempDir(),
-		Workdir:         "/workspace",
-		ReadOnlyRoot:    true,
-		Network:         "none",
-		CapDrop:         []string{"ALL"},
-		Tmpfs:           []string{"/tmp:rw,noexec,nosuid", "  ", "/run"},
-		PidsLimit:       123,
-		Memory:          "1g",
-		MemorySwap:      "2g",
-		Cpus:            1.5,
-		SeccompProfile:  "sec-profile.json",
-		ApparmorProfile: "apparmor-profile",
-		Ulimits: map[string]config.AgentSandboxDockerUlimitValue{
-			"b": {Soft: &soft},
-			"a": {Hard: &hard},
-		},
-	})
-	hc, err := sb.hostConfig()
-	if err != nil {
-		t.Fatalf("hostConfig() error: %v", err)
-	}
-	if !hc.ReadonlyRootfs {
-		t.Fatal("expected readonly rootfs")
-	}
-	if hc.Resources.PidsLimit == nil || *hc.Resources.PidsLimit != 123 {
-		t.Fatalf("unexpected pids limit: %#v", hc.Resources.PidsLimit)
-	}
-	if hc.Memory <= 0 || hc.MemorySwap <= 0 || hc.NanoCPUs <= 0 {
-		t.Fatalf("expected memory/swap/cpu limits set, got mem=%d swap=%d cpu=%d", hc.Memory, hc.MemorySwap, hc.NanoCPUs)
-	}
-	if len(hc.Tmpfs) != 2 || hc.Tmpfs["/run"] != "" {
-		t.Fatalf("unexpected tmpfs map: %#v", hc.Tmpfs)
-	}
-	if got := strings.Join(hc.SecurityOpt, ","); !strings.Contains(got, "seccomp=sec-profile.json") || !strings.Contains(got, "apparmor=apparmor-profile") {
-		t.Fatalf("security options missing expected profiles: %v", hc.SecurityOpt)
-	}
-	if len(hc.Resources.Ulimits) != 2 {
-		t.Fatalf("expected 2 ulimits, got %d", len(hc.Resources.Ulimits))
-	}
-	gotNames := []string{hc.Resources.Ulimits[0].Name, hc.Resources.Ulimits[1].Name}
-	sorted := append([]string{}, gotNames...)
-	sort.Strings(sorted)
-	if gotNames[0] != sorted[0] || gotNames[1] != sorted[1] {
-		t.Fatalf("expected deterministic sorted ulimits, got %v", gotNames)
-	}
-}
-
-func TestHostConfigRejectsInvalidMemorySettings(t *testing.T) {
-	sb := NewContainerSandbox(ContainerSandboxConfig{
-		Memory: "bad",
-	})
-	if _, err := sb.hostConfig(); err == nil || !strings.Contains(err.Error(), "invalid docker.memory") {
-		t.Fatalf("expected invalid docker.memory error, got %v", err)
-	}
-
-	sb = NewContainerSandbox(ContainerSandboxConfig{
-		MemorySwap: "bad",
-	})
-	if _, err := sb.hostConfig(); err == nil || !strings.Contains(err.Error(), "invalid docker.memory_swap") {
-		t.Fatalf("expected invalid docker.memory_swap error, got %v", err)
-	}
-}
-
-func TestBuildDockerUlimitVariants(t *testing.T) {
-	if _, ok := buildDockerUlimit(" ", config.AgentSandboxDockerUlimitValue{}); ok {
-		t.Fatal("expected empty-name ulimit to be rejected")
-	}
-	if _, ok := buildDockerUlimit("nofile", config.AgentSandboxDockerUlimitValue{}); ok {
-		t.Fatal("expected empty ulimit value to be rejected")
-	}
-
-	soft := int64(10)
-	ul, ok := buildDockerUlimit("nofile", config.AgentSandboxDockerUlimitValue{Soft: &soft})
-	if !ok || ul == nil || ul.Soft != 10 || ul.Hard != 10 {
-		t.Fatalf("expected soft-only to mirror hard, got %#v ok=%v", ul, ok)
-	}
-
-	hard := int64(20)
-	ul, ok = buildDockerUlimit("nofile", config.AgentSandboxDockerUlimitValue{Hard: &hard})
-	if !ok || ul == nil || ul.Soft != 20 || ul.Hard != 20 {
-		t.Fatalf("expected hard-only to mirror soft, got %#v ok=%v", ul, ok)
-	}
-}
-
-func TestContainerHelpers(t *testing.T) {
-	if got := shellEscape("a'b"); got != "'a'\"'\"'b'" {
-		t.Fatalf("shellEscape() got %q", got)
-	}
-	if osTempDir() == "" {
-		t.Fatal("osTempDir() should not be empty")
-	}
-
-	sb := NewContainerSandbox(ContainerSandboxConfig{SetupCommand: "   "})
-	if err := sb.runSetupCommand(context.Background()); err != nil {
-		t.Fatalf("runSetupCommand() empty command should be nil, got %v", err)
-	}
-}
-
-func TestWaitExecDoneContextCancel(t *testing.T) {
-	sb := NewContainerSandbox(ContainerSandboxConfig{})
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
-	code, err := sb.waitExecDone(ctx, "unused")
-	if err == nil {
-		t.Fatal("expected context cancellation error")
-	}
-	if code != 1 {
-		t.Fatalf("unexpected exit code: %d", code)
-	}
-}
-
-func TestContainerSandbox_StopWithoutClient(t *testing.T) {
-	sb := NewContainerSandbox(ContainerSandboxConfig{
-		PruneIdleHours:  1,
-		PruneMaxAgeDays: 1,
-	})
-
-	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
-	defer cancel()
-	if err := sb.Prune(ctx); err != nil {
-		t.Fatalf("Prune() error: %v", err)
-	}
-}

pkg/agent/sandbox/container_test.go

@@ -3,9 +3,12 @@ package sandbox
 import (
 	"context"
 	"errors"
+	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/sipeed/picoclaw/pkg/config"
 )
@@ -295,3 +298,177 @@ func TestContainerSandbox_Start_BlockedSecurityConfig(t *testing.T) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
+
+func TestContainerSandbox_StartCreatesWorkspaceBeforeDockerPing(t *testing.T) {
+	workspace := filepath.Join(t.TempDir(), "workspace")
+	workspaceRoot := filepath.Join(t.TempDir(), "sandbox-root")
+	sb := NewContainerSandbox(ContainerSandboxConfig{
+		Workspace:       workspace,
+		WorkspaceRoot:   workspaceRoot,
+		WorkspaceAccess: "none",
+	})
+
+	err := sb.Start(context.Background())
+	if err == nil {
+		_ = sb.Prune(context.Background())
+		t.Skip("docker daemon available in this environment; skip unavailable-path assertion")
+	}
+	if !strings.Contains(err.Error(), "docker daemon unavailable") {
+		t.Fatalf("Start() unexpected error: %v", err)
+	}
+	if _, stErr := os.Stat(workspace); stErr != nil {
+		t.Fatalf("workspace should be created before docker ping: %v", stErr)
+	}
+	if _, stErr := os.Stat(workspaceRoot); stErr != nil {
+		t.Fatalf("workspaceRoot should be created before docker ping: %v", stErr)
+	}
+}
+
+func TestContainerSandbox_NoopPruneWithoutClient(t *testing.T) {
+	sb := NewContainerSandbox(ContainerSandboxConfig{
+		PruneIdleHours:  1,
+		PruneMaxAgeDays: 0,
+	})
+
+	if err := sb.Prune(context.Background()); err != nil {
+		t.Fatalf("Prune() with nil client should be noop, got: %v", err)
+	}
+}
+
+func TestParseByteLimitAndHostConfig(t *testing.T) {
+	if got, err := parseByteLimit("1024"); err != nil || got != 1024 {
+		t.Fatalf("parseByteLimit numeric got (%d,%v), want (1024,nil)", got, err)
+	}
+	if got, err := parseByteLimit("1g"); err != nil || got <= 0 {
+		t.Fatalf("parseByteLimit unit got (%d,%v), want positive", got, err)
+	}
+	if _, err := parseByteLimit("not-a-size"); err == nil {
+		t.Fatal("expected parseByteLimit invalid input error")
+	}
+
+	soft := int64(256)
+	hard := int64(512)
+	sb := NewContainerSandbox(ContainerSandboxConfig{
+		Workspace:       t.TempDir(),
+		Workdir:         "/workspace",
+		ReadOnlyRoot:    true,
+		Network:         "none",
+		CapDrop:         []string{"ALL"},
+		Tmpfs:           []string{"/tmp:rw,noexec,nosuid", "  ", "/run"},
+		PidsLimit:       123,
+		Memory:          "1g",
+		MemorySwap:      "2g",
+		Cpus:            1.5,
+		SeccompProfile:  "sec-profile.json",
+		ApparmorProfile: "apparmor-profile",
+		Ulimits: map[string]config.AgentSandboxDockerUlimitValue{
+			"b": {Soft: &soft},
+			"a": {Hard: &hard},
+		},
+	})
+	hc, err := sb.hostConfig()
+	if err != nil {
+		t.Fatalf("hostConfig() error: %v", err)
+	}
+	if !hc.ReadonlyRootfs {
+		t.Fatal("expected readonly rootfs")
+	}
+	if hc.Resources.PidsLimit == nil || *hc.Resources.PidsLimit != 123 {
+		t.Fatalf("unexpected pids limit: %#v", hc.Resources.PidsLimit)
+	}
+	if hc.Memory <= 0 || hc.MemorySwap <= 0 || hc.NanoCPUs <= 0 {
+		t.Fatalf("expected memory/swap/cpu limits set, got mem=%d swap=%d cpu=%d", hc.Memory, hc.MemorySwap, hc.NanoCPUs)
+	}
+	if len(hc.Tmpfs) != 2 || hc.Tmpfs["/run"] != "" {
+		t.Fatalf("unexpected tmpfs map: %#v", hc.Tmpfs)
+	}
+	if got := strings.Join(hc.SecurityOpt, ","); !strings.Contains(got, "seccomp=sec-profile.json") || !strings.Contains(got, "apparmor=apparmor-profile") {
+		t.Fatalf("security options missing expected profiles: %v", hc.SecurityOpt)
+	}
+	if len(hc.Resources.Ulimits) != 2 {
+		t.Fatalf("expected 2 ulimits, got %d", len(hc.Resources.Ulimits))
+	}
+	gotNames := []string{hc.Resources.Ulimits[0].Name, hc.Resources.Ulimits[1].Name}
+	sorted := append([]string{}, gotNames...)
+	sort.Strings(sorted)
+	if gotNames[0] != sorted[0] || gotNames[1] != sorted[1] {
+		t.Fatalf("expected deterministic sorted ulimits, got %v", gotNames)
+	}
+}
+
+func TestHostConfigRejectsInvalidMemorySettings(t *testing.T) {
+	sb := NewContainerSandbox(ContainerSandboxConfig{
+		Memory: "bad",
+	})
+	if _, err := sb.hostConfig(); err == nil || !strings.Contains(err.Error(), "invalid docker.memory") {
+		t.Fatalf("expected invalid docker.memory error, got %v", err)
+	}
+
+	sb = NewContainerSandbox(ContainerSandboxConfig{
+		MemorySwap: "bad",
+	})
+	if _, err := sb.hostConfig(); err == nil || !strings.Contains(err.Error(), "invalid docker.memory_swap") {
+		t.Fatalf("expected invalid docker.memory_swap error, got %v", err)
+	}
+}
+
+func TestBuildDockerUlimitVariants(t *testing.T) {
+	if _, ok := buildDockerUlimit(" ", config.AgentSandboxDockerUlimitValue{}); ok {
+		t.Fatal("expected empty-name ulimit to be rejected")
+	}
+	if _, ok := buildDockerUlimit("nofile", config.AgentSandboxDockerUlimitValue{}); ok {
+		t.Fatal("expected empty ulimit value to be rejected")
+	}
+
+	soft := int64(10)
+	ul, ok := buildDockerUlimit("nofile", config.AgentSandboxDockerUlimitValue{Soft: &soft})
+	if !ok || ul == nil || ul.Soft != 10 || ul.Hard != 10 {
+		t.Fatalf("expected soft-only to mirror hard, got %#v ok=%v", ul, ok)
+	}
+
+	hard := int64(20)
+	ul, ok = buildDockerUlimit("nofile", config.AgentSandboxDockerUlimitValue{Hard: &hard})
+	if !ok || ul == nil || ul.Soft != 20 || ul.Hard != 20 {
+		t.Fatalf("expected hard-only to mirror soft, got %#v ok=%v", ul, ok)
+	}
+}
+
+func TestContainerHelpers(t *testing.T) {
+	if got := shellEscape("a'b"); got != "'a'\"'\"'b'" {
+		t.Fatalf("shellEscape() got %q", got)
+	}
+	if osTempDir() == "" {
+		t.Fatal("osTempDir() should not be empty")
+	}
+
+	sb := NewContainerSandbox(ContainerSandboxConfig{SetupCommand: "   "})
+	if err := sb.runSetupCommand(context.Background()); err != nil {
+		t.Fatalf("runSetupCommand() empty command should be nil, got %v", err)
+	}
+}
+
+func TestWaitExecDoneContextCancel(t *testing.T) {
+	sb := NewContainerSandbox(ContainerSandboxConfig{})
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	code, err := sb.waitExecDone(ctx, "unused")
+	if err == nil {
+		t.Fatal("expected context cancellation error")
+	}
+	if code != 1 {
+		t.Fatalf("unexpected exit code: %d", code)
+	}
+}
+
+func TestContainerSandbox_StopWithoutClient(t *testing.T) {
+	sb := NewContainerSandbox(ContainerSandboxConfig{
+		PruneIdleHours:  1,
+		PruneMaxAgeDays: 1,
+	})
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+	if err := sb.Prune(ctx); err != nil {
+		t.Fatalf("Prune() error: %v", err)
+	}
+}

pkg/agent/sandbox/error.go

@@ -0,0 +1,5 @@
+package sandbox
+
+import "errors"
+
+var ErrOutsideWorkspace = errors.New("access denied: symlink resolves outside workspace")

pkg/agent/sandbox/factory_test.go

@@ -1,15 +0,0 @@
-package sandbox
-
-import "testing"
-
-func TestNormalizeWorkspaceAccess(t *testing.T) {
-	if got := normalizeWorkspaceAccess("ro"); got != "ro" {
-		t.Fatalf("normalizeWorkspaceAccess(ro) = %q", got)
-	}
-	if got := normalizeWorkspaceAccess("RW"); got != "rw" {
-		t.Fatalf("normalizeWorkspaceAccess(RW) = %q", got)
-	}
-	if got := normalizeWorkspaceAccess("invalid"); got != "none" {
-		t.Fatalf("normalizeWorkspaceAccess(invalid) = %q", got)
-	}
-}

pkg/agent/sandbox/host.go

@@ -28,10 +28,28 @@ func NewHostSandbox(workspace string, restrict bool) *HostSandbox {
 }
 
 func (h *HostSandbox) Start(ctx context.Context) error {
+	// Initialize os.Root for restricted workspace mode to centrally mitigate TOCTOU (Time-of-Check-Time-of-Use) attacks.
+	if h.restrict && h.workspace != "" {
+		r, err := os.OpenRoot(h.workspace)
+		if err != nil {
+			return fmt.Errorf("failed to open workspace root: %w", err)
+		}
+		if hsFS, ok := h.fs.(*hostFS); ok {
+			hsFS.root = r
+		}
+	}
 	return nil
 }
 
 func (h *HostSandbox) Prune(ctx context.Context) error {
+	// Clean up os.Root file descriptors securely.
+	if h.restrict && h.workspace != "" {
+		if hsFS, ok := h.fs.(*hostFS); ok && hsFS.root != nil {
+			err := hsFS.root.Close()
+			hsFS.root = nil
+			return err
+		}
+	}
 	return nil
 }
 
@@ -67,7 +85,7 @@ func (h *HostSandbox) ExecStream(ctx context.Context, req ExecRequest, onEvent f
 	}
 
 	if req.WorkingDir != "" {
-		dir, err := h.resolvePath(req.WorkingDir)
+		dir, err := validatePath(req.WorkingDir, h.workspace, h.restrict)
 		if err != nil {
 			return nil, err
 		}
@@ -164,18 +182,45 @@ func (h *HostSandbox) ExecStream(ctx context.Context, req ExecRequest, onEvent f
 type hostFS struct {
 	workspace string
 	restrict  bool
+	root      *os.Root // OS-level directory file descriptor to safely confine operations and prevent TOCTOU escapes.
+}
+
+func (h *hostFS) getSafeRelPath(path string) (string, error) {
+	if !filepath.IsAbs(path) {
+		return filepath.Clean(path), nil
+	}
+	if !isWithinWorkspace(path, h.workspace) {
+		return "", ErrOutsideWorkspace
+	}
+	// Rel is safe because isWithinWorkspace returned true
+	rel, _ := filepath.Rel(h.workspace, path)
+	return rel, nil
 }
 
 func (h *hostFS) ReadFile(ctx context.Context, path string) ([]byte, error) {
-	resolved, err := resolvePath(path, h.workspace, h.restrict)
+	if !h.restrict || h.workspace == "" || h.root == nil {
+		// Unrestricted mode continues to use traditional resolution
+		resolved, err := validatePath(path, h.workspace, h.restrict)
 		if err != nil {
 			return nil, err
 		}
 		return os.ReadFile(resolved)
+	}
+
+	relPath, err := h.getSafeRelPath(path)
+	if err != nil {
+		return nil, err
+	}
+
+	// os.Root guarantees that the read operation strictly happens within the workspace directory,
+	// effectively and atomically mitigating TOCTOU (Time-of-Check-Time-of-Use) vulnerabilities via symlinks.
+	return h.root.ReadFile(relPath)
 }
 
 func (h *hostFS) WriteFile(ctx context.Context, path string, data []byte, mkdir bool) error {
-	resolved, err := resolvePath(path, h.workspace, h.restrict)
+	if !h.restrict || h.workspace == "" || h.root == nil {
+		// Unrestricted mode continues to use traditional resolution
+		resolved, err := validatePath(path, h.workspace, h.restrict)
 		if err != nil {
 			return err
 		}
@@ -185,13 +230,28 @@ func (h *hostFS) WriteFile(ctx context.Context, path string, data []byte, mkdir
 			}
 		}
 		return os.WriteFile(resolved, data, 0644)
+	}
+
+	relPath, err := h.getSafeRelPath(path)
+	if err != nil {
+		return err
+	}
+
+	if mkdir {
+		// MkdirAll natively resolves inside os.Root to avoid escapes.
+		if err := h.root.MkdirAll(filepath.Dir(relPath), 0755); err != nil {
+			return err
+		}
+	}
+	// Uses OS-level guarantees to restrict the file writing within the root descriptor.
+	return h.root.WriteFile(relPath, data, 0644)
 }
 
-func (h *HostSandbox) resolvePath(path string) (string, error) {
+// validatePath ensures the given path is within the workspace if restrict is true but does not ensure atomic TOCTOU protection.
-	return resolvePath(path, h.workspace, h.restrict)
+// 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 resolvePath(path, workspace string, restrict bool) (string, error) {
+func validatePath(path, workspace string, restrict bool) (string, error) {
 	if workspace == "" {
 		return path, nil
 	}
@@ -211,29 +271,52 @@ func resolvePath(path, workspace string, restrict bool) (string, error) {
 		}
 	}
 
-	if !restrict {
+	if restrict {
-		return absPath, nil
+		if !isWithinWorkspace(absPath, absWorkspace) {
-	}
+			return "", ErrOutsideWorkspace
-
-	rel, err := filepath.Rel(absWorkspace, absPath)
-	if err != nil {
-		return "", fmt.Errorf("failed to resolve relative path: %w", err)
-	}
-	if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
-		return "", fmt.Errorf("access denied: path is outside the workspace")
 		}
 
+		var resolved string
 		workspaceReal := absWorkspace
-	if resolved, err := filepath.EvalSymlinks(absWorkspace); err == nil {
+		if resolved, err = filepath.EvalSymlinks(absWorkspace); err == nil {
 			workspaceReal = resolved
 		}
 
-	if resolved, err := filepath.EvalSymlinks(absPath); err == nil {
+		if resolved, err = filepath.EvalSymlinks(absPath); err == nil {
-		relResolved, err := filepath.Rel(workspaceReal, resolved)
+			if !isWithinWorkspace(resolved, workspaceReal) {
-		if err != nil || relResolved == ".." || strings.HasPrefix(relResolved, ".."+string(os.PathSeparator)) {
+				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)
+}

pkg/agent/sandbox/host_test.go

@@ -0,0 +1,317 @@
+package sandbox
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+)
+
+func TestHostSandbox_StartStopFs(t *testing.T) {
+	sb := NewHostSandbox(t.TempDir(), true)
+	if err := sb.Start(context.Background()); err != nil {
+		t.Fatalf("Start() error: %v", err)
+	}
+	if err := sb.Prune(context.Background()); err != nil {
+		t.Fatalf("Prune() error: %v", err)
+	}
+	if sb.Fs() == nil {
+		t.Fatal("Fs() returned nil")
+	}
+}
+
+func TestHostSandbox_ExecAndFs(t *testing.T) {
+	root := t.TempDir()
+	sb := NewHostSandbox(root, true)
+
+	if _, err := sb.Exec(context.Background(), ExecRequest{Command: "   "}); err == nil {
+		t.Fatal("expected empty command error")
+	}
+
+	res, err := sb.Exec(context.Background(), ExecRequest{
+		Command: "sh",
+		Args:    []string{"-c", "printf hello"},
+	})
+	if err != nil {
+		t.Fatalf("Exec() error: %v", err)
+	}
+	if res.ExitCode != 0 || res.Stdout != "hello" {
+		t.Fatalf("unexpected exec result: %#v", res)
+	}
+
+	if runtime.GOOS != "windows" {
+		_, err = sb.Exec(context.Background(), ExecRequest{
+			Command:   "sh",
+			Args:      []string{"-c", "sleep 1"},
+			TimeoutMs: 10,
+		})
+		if err == nil {
+			t.Fatal("expected timeout-related error")
+		}
+	}
+
+	_, err = sb.Exec(context.Background(), ExecRequest{
+		Command:    "sh",
+		Args:       []string{"-c", "echo bad"},
+		WorkingDir: "../outside",
+	})
+	if err == nil || !errors.Is(err, ErrOutsideWorkspace) {
+		t.Fatalf("expected working dir restriction error, got: %v", err)
+	}
+
+	if err := sb.Fs().WriteFile(context.Background(), "dir/a.txt", []byte("x"), true); err != nil {
+		t.Fatalf("WriteFile() error: %v", err)
+	}
+	b, err := sb.Fs().ReadFile(context.Background(), "dir/a.txt")
+	if err != nil {
+		t.Fatalf("ReadFile() error: %v", err)
+	}
+	if string(b) != "x" {
+		t.Fatalf("ReadFile() got %q, want x", string(b))
+	}
+}
+
+func TestHostSandbox_ResolvePathRestrictions(t *testing.T) {
+	root := t.TempDir()
+
+	got, err := validatePath("a/b.txt", root, true)
+	if err != nil {
+		t.Fatalf("resolvePath relative error: %v", err)
+	}
+	want := filepath.Join(root, "a", "b.txt")
+	if got != want {
+		t.Fatalf("resolvePath relative got %q, want %q", got, want)
+	}
+
+	_, 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)
+	}
+
+	target := filepath.Join(t.TempDir(), "outside.txt")
+	if err := os.WriteFile(target, []byte("x"), 0o644); err != nil {
+		t.Fatalf("write target file: %v", err)
+	}
+	link := filepath.Join(root, "link.txt")
+	if err := os.Symlink(target, link); err == nil {
+		_, err = validatePath("link.txt", root, true)
+		if err == nil || !errors.Is(err, ErrOutsideWorkspace) {
+			t.Fatalf("expected symlink outside error, got: %v", err)
+		}
+	}
+}
+
+func TestUnavailableSandboxAndUtilHelpers(t *testing.T) {
+	sb := NewUnavailableSandbox(nil)
+	if err := sb.Start(context.Background()); err == nil {
+		t.Fatal("expected Start() error")
+	}
+	if err := sb.Prune(context.Background()); err != nil {
+		t.Fatalf("Prune() error: %v", err)
+	}
+	if _, err := sb.Exec(context.Background(), ExecRequest{Command: "echo hi"}); err == nil {
+		t.Fatal("expected Exec() error")
+	}
+	if _, err := sb.Fs().ReadFile(context.Background(), "a.txt"); err == nil {
+		t.Fatal("expected Fs().ReadFile error")
+	}
+	if err := sb.Fs().WriteFile(context.Background(), "a.txt", []byte("x"), true); err == nil {
+		t.Fatal("expected Fs().WriteFile error")
+	}
+
+	if got := durationMs(123).Milliseconds(); got != 123 {
+		t.Fatalf("durationMs() got %d, want 123", got)
+	}
+	if asExitError(errors.New("x"), nil) {
+		t.Fatal("asExitError should be false for non-exit errors")
+	}
+}
+
+func TestHostFS_ReadFileWriteFile_Restricted(t *testing.T) {
+	root := t.TempDir()
+	sb := NewHostSandbox(root, true)
+	if err := sb.Start(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+
+	content := []byte("hello restrict")
+	if err := sb.Fs().WriteFile(context.Background(), "a/b/c.txt", content, true); err != nil {
+		t.Fatalf("WriteFile failed: %v", err)
+	}
+
+	readContent, err := sb.Fs().ReadFile(context.Background(), "a/b/c.txt")
+	if err != nil {
+		t.Fatalf("ReadFile failed: %v", err)
+	}
+	if string(readContent) != string(content) {
+		t.Fatalf("content mismatch")
+	}
+
+	// Should not be able to write root path
+	if err := sb.Fs().WriteFile(context.Background(), "/etc/passwd_not_exist", []byte("a"), false); err == nil {
+		t.Fatalf("expected access denied error writing outside workspace")
+	}
+
+	// Should not be able to read root path
+	if _, err := sb.Fs().ReadFile(context.Background(), "/etc/passwd"); err == nil {
+		t.Fatalf("expected access denied error reading outside workspace")
+	}
+
+	if err := sb.Prune(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestHostFS_ReadFileWriteFile_Unrestricted(t *testing.T) {
+	root := t.TempDir()
+	sb := NewHostSandbox(root, false)
+
+	// Write file directly into workspace since there's no restrictions
+	content := []byte("hello unrestricted")
+	if err := sb.Fs().WriteFile(context.Background(), "a/b/c_unrestricted.txt", content, true); err != nil {
+		t.Fatalf("WriteFile failed: %v", err)
+	}
+
+	readContent, err := sb.Fs().ReadFile(context.Background(), "a/b/c_unrestricted.txt")
+	if err != nil {
+		t.Fatalf("ReadFile failed: %v", err)
+	}
+	if string(readContent) != string(content) {
+		t.Fatalf("content mismatch")
+	}
+}
+
+func TestHostFS_WriteFileMKdir(t *testing.T) {
+	root := t.TempDir()
+	sb := NewHostSandbox(root, true)
+	sb.Start(context.Background())
+	defer sb.Prune(context.Background())
+
+	err := sb.Fs().WriteFile(context.Background(), "deep/nested/dir/file.txt", []byte("a"), true)
+	if err != nil {
+		t.Fatalf("WriteFile with mkdir failed: %v", err)
+	}
+}
+
+func TestHostFS_WriteFileMKdirFailure(t *testing.T) {
+	root := t.TempDir()
+	sb := NewHostSandbox(root, true)
+	sb.Start(context.Background())
+	defer sb.Prune(context.Background())
+
+	// write to a path where parent is a file instead of dir
+	err := sb.Fs().WriteFile(context.Background(), "a.txt", []byte("a"), false)
+	if err != nil {
+		t.Fatalf("WriteFile failed: %v", err)
+	}
+
+	err = sb.Fs().WriteFile(context.Background(), "a.txt/b.txt", []byte("b"), true)
+	if err == nil {
+		t.Fatalf("Expected MkdirAll to fail because a.txt is a file")
+	}
+}
+
+func TestHostFS_ReadFileFailure(t *testing.T) {
+	root := t.TempDir()
+	sb := NewHostSandbox(root, true)
+	sb.Start(context.Background())
+	defer sb.Prune(context.Background())
+
+	_, err := sb.Fs().ReadFile(context.Background(), "does_not_exist.txt")
+	if err == nil {
+		t.Fatalf("Expected ReadFile to fail for non-existent file")
+	}
+}
+
+func TestHostFS_WriteFileFailure(t *testing.T) {
+	root := t.TempDir()
+	sb := NewHostSandbox(root, true)
+	sb.Start(context.Background())
+	defer sb.Prune(context.Background())
+
+	// Create a read-only directory
+	roDir := filepath.Join(root, "ro")
+	os.Mkdir(roDir, 0o500)
+
+	err := sb.Fs().WriteFile(context.Background(), "ro/failed.txt", []byte("a"), false)
+	if err == nil {
+		t.Fatalf("Expected WriteFile to fail in read-only dir")
+	}
+}
+
+func TestHostSandbox_PruneWhenNilFs(t *testing.T) {
+	// simulate prune condition for code coverage
+	sb := NewHostSandbox(t.TempDir(), true)
+	sb.fs.(*hostFS).root = nil
+	err := sb.Prune(context.Background())
+	if err != nil {
+		t.Fatalf("Prune failed when fs root is nil (%v)", err)
+	}
+}
+
+func TestHostSandbox_StartBadWorkspace(t *testing.T) {
+	sb := NewHostSandbox("/this_should_not_exist_normally_12345/abc", true)
+	err := sb.Start(context.Background())
+	if err == nil {
+		t.Fatalf("Start should fail for non-existing workspace root")
+	}
+}
+
+func TestHostFS_ReadFileWriteFile_WithoutWorkspaceOrRoot(t *testing.T) {
+	root := t.TempDir()
+
+	// Test blank workspace
+	sb := NewHostSandbox("", true)
+	if err := sb.Start(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+
+	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)
+	}
+
+	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")
+	}
+
+	// Test case where root is nil explicitly
+	sb2 := NewHostSandbox(root, true)
+	sb2.fs.(*hostFS).root = nil
+	if err := sb2.Fs().WriteFile(context.Background(), "nil_root_test.txt", content, true); err != nil {
+		t.Fatalf("WriteFile failed: %v", err)
+	}
+
+	readContent, err = sb2.Fs().ReadFile(context.Background(), "nil_root_test.txt")
+	if err != nil {
+		t.Fatalf("ReadFile failed: %v", err)
+	}
+	if string(readContent) != string(content) {
+		t.Fatalf("content mismatch")
+	}
+}
+
+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")
+	}
+
+	root := t.TempDir()
+
+	// target parent is file, evalSymlinks should fail
+	if err := os.WriteFile(filepath.Join(root, "a.txt"), []byte("a"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	_, err = validatePath("a.txt/b.txt", root, true)
+	if err == nil {
+		t.Fatalf("expected error when ancestor is file")
+	}
+}

pkg/agent/sandbox/host_unavailable_util_test.go

@@ -1,132 +0,0 @@
-package sandbox
-
-import (
-	"context"
-	"errors"
-	"os"
-	"path/filepath"
-	"runtime"
-	"strings"
-	"testing"
-)
-
-func TestHostSandbox_StartStopFs(t *testing.T) {
-	sb := NewHostSandbox(t.TempDir(), true)
-	if err := sb.Start(context.Background()); err != nil {
-		t.Fatalf("Start() error: %v", err)
-	}
-	if err := sb.Prune(context.Background()); err != nil {
-		t.Fatalf("Prune() error: %v", err)
-	}
-	if sb.Fs() == nil {
-		t.Fatal("Fs() returned nil")
-	}
-}
-
-func TestHostSandbox_ExecAndFs(t *testing.T) {
-	root := t.TempDir()
-	sb := NewHostSandbox(root, true)
-
-	if _, err := sb.Exec(context.Background(), ExecRequest{Command: "   "}); err == nil {
-		t.Fatal("expected empty command error")
-	}
-
-	res, err := sb.Exec(context.Background(), ExecRequest{
-		Command: "sh",
-		Args:    []string{"-c", "printf hello"},
-	})
-	if err != nil {
-		t.Fatalf("Exec() error: %v", err)
-	}
-	if res.ExitCode != 0 || res.Stdout != "hello" {
-		t.Fatalf("unexpected exec result: %#v", res)
-	}
-
-	if runtime.GOOS != "windows" {
-		_, err = sb.Exec(context.Background(), ExecRequest{
-			Command:   "sh",
-			Args:      []string{"-c", "sleep 1"},
-			TimeoutMs: 10,
-		})
-		if err == nil {
-			t.Fatal("expected timeout-related error")
-		}
-	}
-
-	_, err = sb.Exec(context.Background(), ExecRequest{
-		Command:    "sh",
-		Args:       []string{"-c", "echo bad"},
-		WorkingDir: "../outside",
-	})
-	if err == nil || !strings.Contains(err.Error(), "outside the workspace") {
-		t.Fatalf("expected working dir restriction error, got: %v", err)
-	}
-
-	if err := sb.Fs().WriteFile(context.Background(), "dir/a.txt", []byte("x"), true); err != nil {
-		t.Fatalf("WriteFile() error: %v", err)
-	}
-	b, err := sb.Fs().ReadFile(context.Background(), "dir/a.txt")
-	if err != nil {
-		t.Fatalf("ReadFile() error: %v", err)
-	}
-	if string(b) != "x" {
-		t.Fatalf("ReadFile() got %q, want x", string(b))
-	}
-}
-
-func TestHostSandbox_ResolvePathRestrictions(t *testing.T) {
-	root := t.TempDir()
-	sb := NewHostSandbox(root, true)
-
-	got, err := sb.resolvePath("a/b.txt")
-	if err != nil {
-		t.Fatalf("resolvePath relative error: %v", err)
-	}
-	want := filepath.Join(root, "a", "b.txt")
-	if got != want {
-		t.Fatalf("resolvePath relative got %q, want %q", got, want)
-	}
-
-	_, err = sb.resolvePath(filepath.Join(root, "..", "outside.txt"))
-	if err == nil || !strings.Contains(err.Error(), "outside the workspace") {
-		t.Fatalf("expected outside workspace error, got: %v", err)
-	}
-
-	target := filepath.Join(t.TempDir(), "outside.txt")
-	if err := os.WriteFile(target, []byte("x"), 0o644); err != nil {
-		t.Fatalf("write target file: %v", err)
-	}
-	link := filepath.Join(root, "link.txt")
-	if err := os.Symlink(target, link); err == nil {
-		_, err = sb.resolvePath("link.txt")
-		if err == nil || !strings.Contains(err.Error(), "symlink resolves outside workspace") {
-			t.Fatalf("expected symlink outside error, got: %v", err)
-		}
-	}
-}
-
-func TestUnavailableSandboxAndUtilHelpers(t *testing.T) {
-	sb := NewUnavailableSandbox(nil)
-	if err := sb.Start(context.Background()); err == nil {
-		t.Fatal("expected Start() error")
-	}
-	if err := sb.Prune(context.Background()); err != nil {
-		t.Fatalf("Prune() error: %v", err)
-	}
-	if _, err := sb.Exec(context.Background(), ExecRequest{Command: "echo hi"}); err == nil {
-		t.Fatal("expected Exec() error")
-	}
-	if _, err := sb.Fs().ReadFile(context.Background(), "a.txt"); err == nil {
-		t.Fatal("expected Fs().ReadFile error")
-	}
-	if err := sb.Fs().WriteFile(context.Background(), "a.txt", []byte("x"), true); err == nil {
-		t.Fatal("expected Fs().WriteFile error")
-	}
-
-	if got := durationMs(123).Milliseconds(); got != 123 {
-		t.Fatalf("durationMs() got %d, want 123", got)
-	}
-	if asExitError(errors.New("x"), nil) {
-		t.Fatal("asExitError should be false for non-exit errors")
-	}
-}

pkg/agent/sandbox/manager_test.go

@@ -10,6 +10,18 @@ import (
 	"github.com/sipeed/picoclaw/pkg/config"
 )
 
+func TestNormalizeWorkspaceAccess(t *testing.T) {
+	if got := normalizeWorkspaceAccess("ro"); got != "ro" {
+		t.Fatalf("normalizeWorkspaceAccess(ro) = %q", got)
+	}
+	if got := normalizeWorkspaceAccess("RW"); got != "rw" {
+		t.Fatalf("normalizeWorkspaceAccess(RW) = %q", got)
+	}
+	if got := normalizeWorkspaceAccess("invalid"); got != "none" {
+		t.Fatalf("normalizeWorkspaceAccess(invalid) = %q", got)
+	}
+}
+
 func TestExpandHomePath(t *testing.T) {
 	if got := expandHomePath(""); got != "" {
 		t.Fatalf("expandHomePath(\"\") = %q, want empty", got)

pkg/agent/sandbox/registry_lock_test.go

@@ -1,45 +0,0 @@
-package sandbox
-
-import (
-	"path/filepath"
-	"testing"
-	"time"
-)
-
-func TestRegistryFileLock_AcquireRelease(t *testing.T) {
-	regPath := filepath.Join(t.TempDir(), "sandbox", "registry.json")
-	lock, err := acquireRegistryFileLock(regPath)
-	if err != nil {
-		t.Fatalf("acquireRegistryFileLock failed: %v", err)
-	}
-	lock.release()
-}
-
-func TestRegistryFileLock_WaitsUntilReleased(t *testing.T) {
-	regPath := filepath.Join(t.TempDir(), "sandbox", "registry.json")
-	first, err := acquireRegistryFileLock(regPath)
-	if err != nil {
-		t.Fatalf("first lock failed: %v", err)
-	}
-
-	done := make(chan error, 1)
-	go func() {
-		lock, err := acquireRegistryFileLock(regPath)
-		if err == nil && lock != nil {
-			lock.release()
-		}
-		done <- err
-	}()
-
-	time.Sleep(80 * time.Millisecond)
-	first.release()
-
-	select {
-	case err := <-done:
-		if err != nil {
-			t.Fatalf("second lock should succeed after release, got: %v", err)
-		}
-	case <-time.After(2 * time.Second):
-		t.Fatal("timed out waiting for second lock acquisition")
-	}
-}

pkg/agent/sandbox/registry_test.go

@@ -108,3 +108,41 @@ func TestShouldPruneEntry(t *testing.T) {
 		t.Fatal("did not expect fresh entry to be pruned")
 	}
 }
+
+func TestRegistryFileLock_AcquireRelease(t *testing.T) {
+	regPath := filepath.Join(t.TempDir(), "sandbox", "registry.json")
+	lock, err := acquireRegistryFileLock(regPath)
+	if err != nil {
+		t.Fatalf("acquireRegistryFileLock failed: %v", err)
+	}
+	lock.release()
+}
+
+func TestRegistryFileLock_WaitsUntilReleased(t *testing.T) {
+	regPath := filepath.Join(t.TempDir(), "sandbox", "registry.json")
+	first, err := acquireRegistryFileLock(regPath)
+	if err != nil {
+		t.Fatalf("first lock failed: %v", err)
+	}
+
+	done := make(chan error, 1)
+	go func() {
+		lock, err := acquireRegistryFileLock(regPath)
+		if err == nil && lock != nil {
+			lock.release()
+		}
+		done <- err
+	}()
+
+	time.Sleep(80 * time.Millisecond)
+	first.release()
+
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatalf("second lock should succeed after release, got: %v", err)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("timed out waiting for second lock acquisition")
+	}
+}

pkg/agent/sandbox/security_additional_test.go

@@ -1,89 +0,0 @@
-package sandbox
-
-import (
-	"os"
-	"path/filepath"
-	"testing"
-)
-
-func TestValidateSandboxSecurity_AllowsSafeConfig(t *testing.T) {
-	cfg := ContainerSandboxConfig{
-		Binds:           []string{"/tmp:/workspace:rw"},
-		Network:         "none",
-		SeccompProfile:  "default",
-		ApparmorProfile: "docker-default",
-	}
-	if err := validateSandboxSecurity(cfg); err != nil {
-		t.Fatalf("validateSandboxSecurity() error: %v", err)
-	}
-}
-
-func TestValidateSandboxSecurity_ReturnsFirstPolicyError(t *testing.T) {
-	if err := validateSandboxSecurity(ContainerSandboxConfig{
-		Network: "host",
-	}); err == nil {
-		t.Fatal("expected network policy error")
-	}
-
-	if err := validateSandboxSecurity(ContainerSandboxConfig{
-		SeccompProfile: "unconfined",
-	}); err == nil {
-		t.Fatal("expected seccomp policy error")
-	}
-
-	if err := validateSandboxSecurity(ContainerSandboxConfig{
-		ApparmorProfile: "unconfined",
-	}); err == nil {
-		t.Fatal("expected apparmor policy error")
-	}
-}
-
-func TestParseAndNormalizeHelpers(t *testing.T) {
-	if got := parseBindSourcePath("/a:/b:ro"); got != "/a" {
-		t.Fatalf("parseBindSourcePath() got %q, want /a", got)
-	}
-	if got := parseBindSourcePath("just-source"); got != "just-source" {
-		t.Fatalf("parseBindSourcePath() got %q", got)
-	}
-
-	if got := normalizeHostPath(" "); got != "/" {
-		t.Fatalf("normalizeHostPath(empty) got %q, want /", got)
-	}
-	if got := normalizeHostPath("/tmp///a/"); got != "/tmp/a" {
-		t.Fatalf("normalizeHostPath() got %q, want /tmp/a", got)
-	}
-}
-
-func TestTryRealpathAbsolute_Branches(t *testing.T) {
-	if got := tryRealpathAbsolute("relative/path"); got != "relative/path" {
-		t.Fatalf("tryRealpathAbsolute(relative) got %q", got)
-	}
-
-	root := t.TempDir()
-	target := filepath.Join(root, "target")
-	if err := os.MkdirAll(target, 0o755); err != nil {
-		t.Fatalf("mkdir target: %v", err)
-	}
-	link := filepath.Join(root, "link")
-	if err := os.Symlink(target, link); err != nil {
-		t.Fatalf("symlink create: %v", err)
-	}
-
-	old := filepathEvalSymlinks
-	t.Cleanup(func() { filepathEvalSymlinks = old })
-
-	filepathEvalSymlinks = old
-	if got := tryRealpathAbsolute(link); got == link {
-		t.Fatalf("tryRealpathAbsolute(existing symlink) should resolve, got %q", got)
-	}
-
-	filepathEvalSymlinks = func(path string) (string, error) { return "", os.ErrPermission }
-	if got := tryRealpathAbsolute(link); got != normalizeHostPath(link) {
-		t.Fatalf("tryRealpathAbsolute(eval error) got %q", got)
-	}
-
-	nonexistent := filepath.Join(root, "does-not-exist")
-	if got := tryRealpathAbsolute(nonexistent); got != nonexistent {
-		t.Fatalf("tryRealpathAbsolute(nonexistent) got %q", got)
-	}
-}

pkg/agent/sandbox/security_test.go

@@ -1,6 +1,8 @@
 package sandbox
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -71,3 +73,85 @@ func TestSanitizeEnvVars_BlocksSensitiveKeys(t *testing.T) {
 		t.Fatal("NULLY should be blocked due to null byte")
 	}
 }
+
+func TestValidateSandboxSecurity_AllowsSafeConfig(t *testing.T) {
+	cfg := ContainerSandboxConfig{
+		Binds:           []string{"/tmp:/workspace:rw"},
+		Network:         "none",
+		SeccompProfile:  "default",
+		ApparmorProfile: "docker-default",
+	}
+	if err := validateSandboxSecurity(cfg); err != nil {
+		t.Fatalf("validateSandboxSecurity() error: %v", err)
+	}
+}
+
+func TestValidateSandboxSecurity_ReturnsFirstPolicyError(t *testing.T) {
+	if err := validateSandboxSecurity(ContainerSandboxConfig{
+		Network: "host",
+	}); err == nil {
+		t.Fatal("expected network policy error")
+	}
+
+	if err := validateSandboxSecurity(ContainerSandboxConfig{
+		SeccompProfile: "unconfined",
+	}); err == nil {
+		t.Fatal("expected seccomp policy error")
+	}
+
+	if err := validateSandboxSecurity(ContainerSandboxConfig{
+		ApparmorProfile: "unconfined",
+	}); err == nil {
+		t.Fatal("expected apparmor policy error")
+	}
+}
+
+func TestParseAndNormalizeHelpers(t *testing.T) {
+	if got := parseBindSourcePath("/a:/b:ro"); got != "/a" {
+		t.Fatalf("parseBindSourcePath() got %q, want /a", got)
+	}
+	if got := parseBindSourcePath("just-source"); got != "just-source" {
+		t.Fatalf("parseBindSourcePath() got %q", got)
+	}
+
+	if got := normalizeHostPath(" "); got != "/" {
+		t.Fatalf("normalizeHostPath(empty) got %q, want /", got)
+	}
+	if got := normalizeHostPath("/tmp///a/"); got != "/tmp/a" {
+		t.Fatalf("normalizeHostPath() got %q, want /tmp/a", got)
+	}
+}
+
+func TestTryRealpathAbsolute_Branches(t *testing.T) {
+	if got := tryRealpathAbsolute("relative/path"); got != "relative/path" {
+		t.Fatalf("tryRealpathAbsolute(relative) got %q", got)
+	}
+
+	root := t.TempDir()
+	target := filepath.Join(root, "target")
+	if err := os.MkdirAll(target, 0o755); err != nil {
+		t.Fatalf("mkdir target: %v", err)
+	}
+	link := filepath.Join(root, "link")
+	if err := os.Symlink(target, link); err != nil {
+		t.Fatalf("symlink create: %v", err)
+	}
+
+	old := filepathEvalSymlinks
+	t.Cleanup(func() { filepathEvalSymlinks = old })
+
+	filepathEvalSymlinks = old
+	if got := tryRealpathAbsolute(link); got == link {
+		t.Fatalf("tryRealpathAbsolute(existing symlink) should resolve, got %q", got)
+	}
+
+	filepathEvalSymlinks = func(path string) (string, error) { return "", os.ErrPermission }
+	if got := tryRealpathAbsolute(link); got != normalizeHostPath(link) {
+		t.Fatalf("tryRealpathAbsolute(eval error) got %q", got)
+	}
+
+	nonexistent := filepath.Join(root, "does-not-exist")
+	if got := tryRealpathAbsolute(nonexistent); got != nonexistent {
+		t.Fatalf("tryRealpathAbsolute(nonexistent) got %q", got)
+	}
+}