diff --git a/pkg/agent/sandbox/container.go b/pkg/agent/sandbox/container.go index fac74d412..4a9bd1587 100644 --- a/pkg/agent/sandbox/container.go +++ b/pkg/agent/sandbox/container.go @@ -465,7 +465,7 @@ func (c *ContainerSandbox) sandboxStateDir() string { } func (c *ContainerSandbox) stopAndRemoveContainer(ctx context.Context, containerName string) error { - timeout := 10 + timeout := 5 _ = c.cli.ContainerStop(ctx, containerName, container.StopOptions{Timeout: &timeout}) if err := c.cli.ContainerRemove(ctx, containerName, container.RemoveOptions{Force: true}); err != nil { return err @@ -484,7 +484,7 @@ func stopAndRemoveContainerByName(ctx context.Context, containerName string) err } defer cli.Close() - timeout := 10 + timeout := 5 _ = cli.ContainerStop(ctx, name, container.StopOptions{Timeout: &timeout}) if err := cli.ContainerRemove(ctx, name, container.RemoveOptions{Force: true}); err != nil { return err diff --git a/pkg/agent/sandbox/container_integration_test.go b/pkg/agent/sandbox/container_integration_test.go index 4f97053d6..2910dcc4b 100644 --- a/pkg/agent/sandbox/container_integration_test.go +++ b/pkg/agent/sandbox/container_integration_test.go @@ -249,8 +249,8 @@ func TestContainerSandbox_Integration_MaybePruneRemovesOldContainer(t *testing.T } // Explicitly call Prune (scoped manager would normally do this in loop) - if err := sb.Prune(ctx); err != nil { - t.Fatalf("Prune failed: %v", err) + if pruneErr := sb.Prune(ctx); pruneErr != nil { + t.Fatalf("Prune failed: %v", pruneErr) } // Verify container is gone @@ -313,10 +313,15 @@ func TestContainerSandbox_Integration_ExecTimeoutBreaksStdCopyBlock(t *testing.T } defer sb.Prune(ctx) + // Ensure container is started so the timeout only applies to command execution + if _, err := sb.Exec(ctx, ExecRequest{Command: "true"}); err != nil { + t.Fatalf("warmup failed: %v", err) + } + // Simulate a command that hangs and might block output readers start := time.Now() _, err := sb.Exec(ctx, ExecRequest{ - Command: "cat", // Blocks waiting for stdin which is never provided + Command: "sleep 10", // Guaranteed to hang TimeoutMs: 500, }) elapsed := time.Since(start) diff --git a/pkg/agent/sandbox/security.go b/pkg/agent/sandbox/security.go index d7263e2ce..26c9359c8 100644 --- a/pkg/agent/sandbox/security.go +++ b/pkg/agent/sandbox/security.go @@ -10,25 +10,36 @@ import ( ) var blockedHostPaths = []string{ + "/boot", + "/dev", "/etc", "/private/etc", - "/proc", - "/sys", - "/dev", - "/root", - "/boot", - "/run", - "/var/run", "/private/var/run", - "/run/docker.sock", - "/var/run/docker.sock", "/private/var/run/docker.sock", - "/run/user", + "/proc", + "/root", + "/run", + "/run/containerd", + "/run/crio", + "/run/docker.sock", "/run/podman", + "/run/user", + "/sys", "/tmp/podman.sock", + "/var/run", + "/var/run/containerd", + "/var/run/crio", + "/var/run/docker.sock", "/xdg_runtime_dir", } +var blockedHostPathSuffixes = []string{ + "/.docker/run/docker.sock", + "/.docker/desktop/docker.sock", + "/.colima/default/docker.sock", + "/.rd/docker.sock", +} + var blockedEnvVarPatterns = []*regexp.Regexp{ regexp.MustCompile(`(?i)^ANTHROPIC_API_KEY$`), regexp.MustCompile(`(?i)^OPENAI_API_KEY$`), @@ -97,6 +108,18 @@ func validateBindSourcePath(bind, source string) error { return fmt.Errorf("sandbox security: bind mount %q targets blocked path %q", bind, blocked) } } + for _, suffix := range blockedHostPathSuffixes { + if source == suffix || strings.HasSuffix(source, suffix) { + return fmt.Errorf("sandbox security: bind mount %q targets blocked path suffix %q", bind, suffix) + } + } + isSocket, err := isUnixSocketPath(source) + if err != nil { + return fmt.Errorf("sandbox security: bind mount %q source %q cannot be validated: %w", bind, source, err) + } + if isSocket { + return fmt.Errorf("sandbox security: bind mount %q targets unix socket %q", bind, source) + } return nil } @@ -194,3 +217,18 @@ func isBlockedEnvVarKey(key string) bool { var filepathEvalSymlinks = func(path string) (string, error) { return filepath.EvalSymlinks(path) } + +var osLstat = func(path string) (os.FileInfo, error) { + return os.Lstat(path) +} + +func isUnixSocketPath(p string) (bool, error) { + fi, err := osLstat(p) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + return fi.Mode()&os.ModeSocket != 0, nil +} diff --git a/pkg/agent/sandbox/security_test.go b/pkg/agent/sandbox/security_test.go index 33c46a442..6a22b5639 100644 --- a/pkg/agent/sandbox/security_test.go +++ b/pkg/agent/sandbox/security_test.go @@ -1,6 +1,7 @@ package sandbox import ( + "net" "os" "path/filepath" "strings" @@ -17,6 +18,26 @@ func TestValidateBindMounts_BlocksDangerousPath(t *testing.T) { } } +func TestValidateBindMounts_BlocksAdditionalRuntimePaths(t *testing.T) { + err := validateBindMounts([]string{"/run/containerd/containerd.sock:/mnt/runtime.sock:ro"}) + if err == nil { + t.Fatal("expected blocked runtime bind path error") + } + if !strings.Contains(err.Error(), "blocked path") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestValidateBindMounts_BlocksDangerousSocketSuffixes(t *testing.T) { + err := validateBindMounts([]string{"/home/user/.docker/run/docker.sock:/mnt/docker.sock:ro"}) + if err == nil { + t.Fatal("expected blocked dangerous socket suffix error") + } + if !strings.Contains(err.Error(), "blocked path suffix") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestValidateBindMounts_BlocksNonAbsoluteSource(t *testing.T) { err := validateBindMounts([]string{"myvol:/mnt"}) if err == nil { @@ -33,6 +54,40 @@ func TestValidateBindMounts_AllowsProjectPath(t *testing.T) { } } +func TestValidateBindMounts_BlocksUnixSocketSource(t *testing.T) { + tmpDir := t.TempDir() + socketPath := filepath.Join(tmpDir, "agent.sock") + ln, err := net.Listen("unix", socketPath) + if err != nil { + t.Fatalf("listen unix: %v", err) + } + defer ln.Close() + + err = validateBindMounts([]string{socketPath + ":/workspace/agent.sock:ro"}) + if err == nil { + t.Fatal("expected unix socket source to be blocked") + } + if !strings.Contains(err.Error(), "unix socket") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestValidateBindMounts_BlocksSymlinkToBlockedPath(t *testing.T) { + tmpDir := t.TempDir() + link := filepath.Join(tmpDir, "etc-link") + if err := os.Symlink("/etc", link); err != nil { + t.Skipf("symlink not supported in this environment: %v", err) + } + + err := validateBindMounts([]string{link + ":/workspace/etc:ro"}) + if err == nil { + t.Fatal("expected symlink-resolved blocked path error") + } + if !strings.Contains(err.Error(), "blocked path") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestValidateNetworkMode_BlocksHost(t *testing.T) { if err := validateNetworkMode("HOST"); err == nil { t.Fatal("expected host network mode to be blocked") @@ -138,7 +193,9 @@ func TestTryRealpathAbsolute_Branches(t *testing.T) { } old := filepathEvalSymlinks + oldLstat := osLstat t.Cleanup(func() { filepathEvalSymlinks = old }) + t.Cleanup(func() { osLstat = oldLstat }) filepathEvalSymlinks = old if got := tryRealpathAbsolute(link); got == link { @@ -155,3 +212,36 @@ func TestTryRealpathAbsolute_Branches(t *testing.T) { t.Fatalf("tryRealpathAbsolute(nonexistent) got %q", got) } } + +func TestIsUnixSocketPath_Branches(t *testing.T) { + tmpDir := t.TempDir() + socketPath := filepath.Join(tmpDir, "s.sock") + ln, err := net.Listen("unix", socketPath) + if err != nil { + t.Fatalf("listen unix: %v", err) + } + defer ln.Close() + + ok, err := isUnixSocketPath(socketPath) + if err != nil { + t.Fatalf("isUnixSocketPath(socket) error: %v", err) + } + if !ok { + t.Fatal("expected socket path to be detected") + } + + ok, err = isUnixSocketPath(filepath.Join(tmpDir, "missing.sock")) + if err != nil { + t.Fatalf("isUnixSocketPath(missing) error: %v", err) + } + if ok { + t.Fatal("missing path should not be detected as socket") + } + + old := osLstat + t.Cleanup(func() { osLstat = old }) + osLstat = func(path string) (os.FileInfo, error) { return nil, os.ErrPermission } + if _, err := isUnixSocketPath(socketPath); err == nil { + t.Fatal("expected lstat permission error to be returned") + } +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 08cbb85d6..3f7242a9f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -898,7 +898,7 @@ func (c *Config) ValidateModelList() error { } // IntPtr is a convenience helper that returns a pointer to the provided int value. -// It is used to initialise *int config fields with literal defaults. +// It is used to initialize *int config fields with literal defaults. func IntPtr(v int) *int { return &v } diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go index 84f66546b..800f05695 100644 --- a/pkg/tools/cron.go +++ b/pkg/tools/cron.go @@ -309,9 +309,8 @@ func (t *CronTool) ExecuteJob(ctx context.Context, job *cron.CronJob) string { // Execute command if present if job.Payload.Command != "" { var output string - cwd := "" if t.execGuard != nil { - cwd = t.execGuard.workingDir + cwd := t.execGuard.workingDir if guardError := t.execGuard.guardCommand(job.Payload.Command, cwd); guardError != "" { output = fmt.Sprintf("Error executing scheduled command: %s", guardError) pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second)