From 2b8713d3af8e0d136bce8eea69a5cd83ad897e5b Mon Sep 17 00:00:00 2001 From: momo-mnsjtxy <122842784+momo-mnsjtxy@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:02:46 -0700 Subject: [PATCH 1/3] feat(agent): add opt-in restricted execution policy alongside workspace containers --- .github/workflows/sandbox-validation.yml | 51 +++ apps/agent/internal/rpc/handler.go | 63 ++- apps/agent/internal/rpc/sandbox.go | 178 ++++++++ .../internal/rpc/sandbox_validation_test.go | 113 +++++ .../internal/sandbox/integration_test.go | 119 +++++ apps/agent/internal/sandbox/sandbox.go | 424 ++++++++++++++++++ .../agent/internal/sandbox/validation_test.go | 232 ++++++++++ apps/server/internal/runtime/acp.go | 53 ++- apps/server/internal/runtime/acp_runtime.go | 7 + .../internal/runtime/agent_tool_runners.go | 20 +- apps/server/internal/runtime/agent_tools.go | 21 +- apps/server/internal/runtime/sandbox_exec.go | 102 +++++ .../server/internal/runtime/sandbox_policy.go | 61 +++ .../internal/runtime/sandbox_policy_test.go | 261 +++++++++++ .../internal/runtime/tool_session_state.go | 5 + docs/sandbox.md | 78 ++++ 16 files changed, 1765 insertions(+), 23 deletions(-) create mode 100644 .github/workflows/sandbox-validation.yml create mode 100644 apps/agent/internal/rpc/sandbox.go create mode 100644 apps/agent/internal/rpc/sandbox_validation_test.go create mode 100644 apps/agent/internal/sandbox/integration_test.go create mode 100644 apps/agent/internal/sandbox/sandbox.go create mode 100644 apps/agent/internal/sandbox/validation_test.go create mode 100644 apps/server/internal/runtime/sandbox_exec.go create mode 100644 apps/server/internal/runtime/sandbox_policy.go create mode 100644 apps/server/internal/runtime/sandbox_policy_test.go create mode 100644 docs/sandbox.md diff --git a/.github/workflows/sandbox-validation.yml b/.github/workflows/sandbox-validation.yml new file mode 100644 index 00000000..38330162 --- /dev/null +++ b/.github/workflows/sandbox-validation.yml @@ -0,0 +1,51 @@ +name: sandbox-validation + +on: + workflow_dispatch: + pull_request: + paths: + - 'apps/agent/**' + - 'apps/server/**' + - '.github/workflows/sandbox-validation.yml' + push: + paths: + - 'apps/agent/**' + - 'apps/server/**' + - '.github/workflows/sandbox-validation.yml' + +permissions: + contents: read + +jobs: + sandbox: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: '1.26.4' + cache-dependency-path: | + apps/agent/go.sum + apps/server/go.sum + - name: Agent tests and race detector + working-directory: apps/agent + run: | + go test -race ./... + go vet ./... + - name: Prepare sandbox test image + run: | + docker pull alpine:3.23 + echo "ZAKURA_SANDBOX_IMAGE=$(docker image inspect alpine:3.23 --format '{{index .RepoDigests 0}}')" >> "$GITHUB_ENV" + - name: Real container isolation and lifecycle checks + working-directory: apps/agent + env: + ZAKURA_SANDBOX_INTEGRATION: '1' + run: go test -count=1 -v ./internal/sandbox + - name: Server regression tests + working-directory: apps/server + env: + CGO_ENABLED: '1' + run: | + go test ./... + go vet ./... diff --git a/apps/agent/internal/rpc/handler.go b/apps/agent/internal/rpc/handler.go index b6438700..18a3da92 100644 --- a/apps/agent/internal/rpc/handler.go +++ b/apps/agent/internal/rpc/handler.go @@ -6,28 +6,34 @@ import ( "encoding/json" "fmt" "io" + "os" "strings" "sync" "github.com/Moonrend/Zakura/apps/agent/internal/docker" "github.com/Moonrend/Zakura/apps/agent/internal/host" + "github.com/Moonrend/Zakura/apps/agent/internal/sandbox" "github.com/Moonrend/Zakura/apps/agent/internal/sys" ) type Handler struct { - Kind string - StorageRoot string - jobs *host.Registry - ptys map[string]*host.LiveStream - mu sync.Mutex + Kind string + StorageRoot string + jobs *host.Registry + sandboxJobs *sandbox.Registry + sandboxEnforced bool + ptys map[string]*host.LiveStream + mu sync.Mutex } func New(kind, storageRoot string) *Handler { return &Handler{ - Kind: kind, - StorageRoot: storageRoot, - jobs: host.NewRegistry(), - ptys: map[string]*host.LiveStream{}, + Kind: kind, + StorageRoot: storageRoot, + jobs: host.NewRegistry(), + sandboxJobs: sandbox.NewRegistry(), + sandboxEnforced: sandboxEnforcement(os.Getenv("ZAKURA_SANDBOX_ENABLED")), + ptys: map[string]*host.LiveStream{}, } } @@ -38,7 +44,24 @@ func (h *Handler) workspace(spaceID string) string { func (h *Handler) Dispatch(ctx context.Context, msg Msg, send func(Msg)) { var err error var result any + + // Do not let an explicit isolation request silently use an unsupported + // execution transport. Docker management is not the sandbox backend API. + switch msg.Method { + case "docker.run", "docker.exec", "docker.exec.start", "docker.attach", "docker.recreate": + requested, policyErr := h.sandboxRequested(msg.Params) + if policyErr != nil { + send(Err(msg.ID, policyErr.Error())) + return + } + if requested { + send(Err(msg.ID, "this execution method is unavailable in sandbox mode; use host.exec or host.exec.start")) + return + } + } switch msg.Method { + case "sandbox.policy": + result = map[string]any{"enforced": h.sandboxEnforced, "defaultExecutionMode": map[bool]string{true: "sandbox", false: "host"}[h.sandboxEnforced], "hostModeIsolated": false, "sandbox": sandbox.Policy()} case "sys.info": var p struct { Light bool `json:"light"` @@ -77,15 +100,24 @@ func (h *Handler) Dispatch(ctx context.Context, msg Msg, send func(Msg)) { case "host.fs.rename": result, err = h.fsRename(msg.Params) case "host.exec": - result, err = h.hostExec(msg.Params) + result, err = h.execWithPolicy(ctx, msg.Params, false) case "host.exec.start": - result, err = h.hostExecStart(msg.Params) + result, err = h.execWithPolicy(ctx, msg.Params, true) case "host.exec.get": - result, err = h.hostExecGet(msg.Params) + result, err = h.jobWithPolicy(msg.Params, false) case "host.exec.kill": - result, err = h.hostExecKill(msg.Params) + result, err = h.jobWithPolicy(msg.Params, true) case "host.pty.start": - result, err = h.ptyStart(msg.Params, send) + if enabled, policyErr := h.sandboxRequested(msg.Params); policyErr != nil { + err = policyErr + } else if enabled { + err = fmt.Errorf("interactive execution is unavailable in sandbox mode; use non-interactive sandbox execution") + } else { + result, err = h.ptyStart(msg.Params, send) + if err == nil { + result, err = executionResult(result, "host") + } + } case "host.pty.write": err = h.ptyWrite(msg.Params) result = map[string]bool{"ok": err == nil} @@ -191,6 +223,9 @@ func (h *Handler) Dispatch(ctx context.Context, msg Msg, send func(Msg)) { _ = json.Unmarshal(msg.Params, &p) message = host.ScrubHostPathsInMessage(h.rootOf(p), message) } + if strings.HasPrefix(msg.Method, "host.exec") { + message = host.ScrubHostPathsInMessage(h.StorageRoot, message) + } send(Err(msg.ID, message)) return } diff --git a/apps/agent/internal/rpc/sandbox.go b/apps/agent/internal/rpc/sandbox.go new file mode 100644 index 00000000..3dc40bf3 --- /dev/null +++ b/apps/agent/internal/rpc/sandbox.go @@ -0,0 +1,178 @@ +package rpc + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/Moonrend/Zakura/apps/agent/internal/sandbox" +) + +var sandboxSpaceID = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_-]{0,127}$`) + +// The authenticated control plane supplies the space identity. Never accept an +// arbitrary root, a storage-root fallback, or a symlink into another workspace. +func (h *Handler) sandboxRoot(spaceID string) (string, error) { + if !sandboxSpaceID.MatchString(spaceID) { + return "", fmt.Errorf("a valid spaceId is required for sandbox execution") + } + root, err := filepath.Abs(h.StorageRoot) + if err != nil || h.StorageRoot == "" { + return "", fmt.Errorf("sandbox storage root is unavailable") + } + resolved, err := filepath.EvalSymlinks(root) + if err != nil || resolved != root { + return "", fmt.Errorf("sandbox storage root must be an existing non-symlink directory") + } + for _, part := range []string{"spaces", spaceID, "workspace"} { + root = filepath.Join(root, part) + if err := os.Mkdir(root, 0755); err != nil && !os.IsExist(err) { + return "", fmt.Errorf("sandbox workspace is unavailable") + } + info, err := os.Lstat(root) + if err != nil || !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { + return "", fmt.Errorf("sandbox workspace must use real directories") + } + } + return root, nil +} + +type sandboxRequest struct { + sandbox.ExecParams + SpaceID string `json:"spaceId"` +} + +func (h *Handler) sandboxExecContext(ctx context.Context, raw json.RawMessage) (any, error) { + var p sandboxRequest + if err := json.Unmarshal(raw, &p); err != nil { + return nil, fmt.Errorf("invalid sandbox execution parameters") + } + root, err := h.sandboxRoot(p.SpaceID) + if err != nil { + return nil, err + } + return sandbox.Run(ctx, root, p.ExecParams) +} + +func (h *Handler) sandboxExecStart(raw json.RawMessage) (any, error) { + var p sandboxRequest + if err := json.Unmarshal(raw, &p); err != nil { + return nil, fmt.Errorf("invalid sandbox execution parameters") + } + root, err := h.sandboxRoot(p.SpaceID) + if err != nil { + return nil, err + } + return h.sandboxJobs.Start(root, p.ExecParams) +} + +func (h *Handler) sandboxJob(raw json.RawMessage, cancel bool) (any, error) { + var p struct { + ID string `json:"id"` + SpaceID string `json:"spaceId"` + } + if err := json.Unmarshal(raw, &p); err != nil { + return nil, fmt.Errorf("invalid sandbox job parameters") + } + root, err := h.sandboxRoot(p.SpaceID) + if err != nil { + return nil, err + } + var snap *sandbox.JobSnap + if cancel { + snap = h.sandboxJobs.KillScoped(p.ID, root) + } else { + snap = h.sandboxJobs.GetScoped(p.ID, root) + } + if snap == nil { + return nil, fmt.Errorf("sandbox job not found") + } + return snap, nil +} + +// Enforcement is captured when the authenticated runner handler is created. +// A request can select stronger isolation but cannot turn operator policy off. +func (h *Handler) sandboxRequested(raw json.RawMessage) (bool, error) { + var p struct { + ExecutionMode string `json:"executionMode"` + } + if err := json.Unmarshal(raw, &p); err != nil { + return false, fmt.Errorf("invalid execution policy parameters") + } + if p.ExecutionMode != "" && p.ExecutionMode != "host" && p.ExecutionMode != "sandbox" { + return false, fmt.Errorf("executionMode must be host or sandbox") + } + return h.sandboxEnforced || p.ExecutionMode == "sandbox", nil +} + +func executionResult(value any, mode string) (any, error) { + raw, err := json.Marshal(value) + if err != nil { + return nil, err + } + result := map[string]any{} + if err := json.Unmarshal(raw, &result); err != nil { + return nil, err + } + result["executionMode"] = mode + result["isolated"] = mode == "sandbox" + return result, nil +} + +func (h *Handler) execWithPolicy(ctx context.Context, raw json.RawMessage, start bool) (any, error) { + enabled, err := h.sandboxRequested(raw) + if err != nil { + return nil, err + } + var result any + mode := "host" + if enabled { + mode = "sandbox" + if start { + result, err = h.sandboxExecStart(raw) + } else { + result, err = h.sandboxExecContext(ctx, raw) + } + } else { + if start { + result, err = h.hostExecStart(raw) + } else { + result, err = h.hostExec(raw) + } + } + if err != nil { + return nil, err + } + return executionResult(result, mode) +} + +func (h *Handler) jobWithPolicy(raw json.RawMessage, cancel bool) (any, error) { + enabled, err := h.sandboxRequested(raw) + if err != nil { + return nil, err + } + var result any + mode := "host" + if enabled { + mode = "sandbox" + result, err = h.sandboxJob(raw, cancel) + } else if cancel { + result, err = h.hostExecKill(raw) + } else { + result, err = h.hostExecGet(raw) + } + if err != nil { + return nil, err + } + return executionResult(result, mode) +} + +// Unknown nonempty values fail closed rather than silently turning policy off. +func sandboxEnforcement(value string) bool { + value = strings.TrimSpace(value) + return value != "" && !strings.EqualFold(value, "false") && value != "0" +} diff --git a/apps/agent/internal/rpc/sandbox_validation_test.go b/apps/agent/internal/rpc/sandbox_validation_test.go new file mode 100644 index 00000000..d88bfe65 --- /dev/null +++ b/apps/agent/internal/rpc/sandbox_validation_test.go @@ -0,0 +1,113 @@ +package rpc + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +func sandboxCall(t *testing.T, h *Handler, method string, p any) Msg { + t.Helper() + raw, err := json.Marshal(p) + if err != nil { + t.Fatal(err) + } + var response Msg + h.Dispatch(context.Background(), Msg{ID: "sandbox-test", Method: method, Params: raw}, func(msg Msg) { response = msg }) + return response +} + +func TestSandboxRPCPolicyAndEnforcement(t *testing.T) { + t.Setenv("ZAKURA_SANDBOX_ENABLED", "") + h := New("computer", t.TempDir()) + for _, mode := range []string{"", "host"} { + enabled, err := h.sandboxRequested(json.RawMessage(`{"executionMode":"` + mode + `"}`)) + if err != nil || enabled { + t.Fatalf("legacy mode unexpectedly isolated: %v %v", enabled, err) + } + } + t.Setenv("ZAKURA_SANDBOX_ENABLED", "true") + enforced := New("computer", t.TempDir()) + t.Setenv("ZAKURA_SANDBOX_ENABLED", "") + enabled, err := enforced.sandboxRequested(json.RawMessage(`{"executionMode":"host"}`)) + if err != nil || !enabled { + t.Fatalf("request downgraded operator policy: %v %v", enabled, err) + } + result := sandboxCall(t, enforced, "sandbox.policy", map[string]any{}) + var policy struct { + Enforced bool `json:"enforced"` + Default string `json:"defaultExecutionMode"` + HostIsolated bool `json:"hostModeIsolated"` + } + if err := json.Unmarshal(result.Result, &policy); err != nil { + t.Fatal(err) + } + if !policy.Enforced || policy.Default != "sandbox" || policy.HostIsolated { + t.Fatalf("incorrect policy disclosure: %+v", policy) + } +} + +func TestSandboxRPCRejectsUnsupportedTransports(t *testing.T) { + t.Setenv("ZAKURA_SANDBOX_ENABLED", "") + h := New("computer", t.TempDir()) + for _, method := range []string{"host.pty.start", "docker.run", "docker.exec", "docker.exec.start", "docker.attach", "docker.recreate"} { + t.Run(method, func(t *testing.T) { + result := sandboxCall(t, h, method, map[string]any{"executionMode": "sandbox", "spaceId": "space-a"}) + if result.Error == "" || !strings.Contains(result.Error, "sandbox") { + t.Fatalf("unsupported transport was not refused: %+v", result) + } + }) + } + for _, method := range []string{"host.exec", "host.exec.start", "host.exec.get", "host.exec.kill"} { + result := sandboxCall(t, h, method, map[string]any{"executionMode": "unknown"}) + if result.Error == "" { + t.Fatalf("%s accepted unknown execution mode", method) + } + } + var response Msg + h.Dispatch(context.Background(), Msg{ID: "invalid", Method: "host.exec", Params: json.RawMessage(`{"`)}, func(msg Msg) { response = msg }) + if response.Error == "" { + t.Fatal("malformed policy accepted") + } +} + +func TestSandboxRPCRequiresWorkspaceAndAvailableBackend(t *testing.T) { + t.Setenv("ZAKURA_SANDBOX_ENABLED", "") + t.Setenv("ZAKURA_SANDBOX_IMAGE", "") + h := New("computer", t.TempDir()) + for _, method := range []string{"host.exec", "host.exec.start", "host.exec.get", "host.exec.kill"} { + result := sandboxCall(t, h, method, map[string]any{"executionMode": "sandbox", "command": []string{"printf", "hello"}}) + if result.Error == "" || !strings.Contains(result.Error, "spaceId") { + t.Fatalf("%s accepted absent workspace: %+v", method, result) + } + } + result := sandboxCall(t, h, "host.exec", map[string]any{"executionMode": "sandbox", "spaceId": "space-a", "command": []string{"printf", "hello"}}) + if result.Error == "" || !strings.Contains(result.Error, "ZAKURA_SANDBOX_IMAGE") { + t.Fatalf("missing image did not fail closed: %+v", result) + } + for _, method := range []string{"host.exec.get", "host.exec.kill"} { + result = sandboxCall(t, h, method, map[string]any{"executionMode": "sandbox", "spaceId": "space-b", "id": "unknown-job"}) + if result.Error == "" || !strings.Contains(result.Error, "not found") { + t.Fatalf("unknown job exposed: %+v", result) + } + } +} + +func TestSandboxRPCRejectsSymlinkWorkspaceRoot(t *testing.T) { + t.Setenv("ZAKURA_SANDBOX_ENABLED", "") + storage := t.TempDir() + target := t.TempDir() + if err := os.MkdirAll(filepath.Join(storage, "spaces", "space-a"), 0755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(storage, "spaces", "space-a", "workspace")); err != nil { + t.Skipf("symlinks unsupported: %v", err) + } + h := New("computer", storage) + if _, err := h.sandboxRoot("space-a"); err == nil { + t.Fatal("symlink workspace accepted") + } +} diff --git a/apps/agent/internal/sandbox/integration_test.go b/apps/agent/internal/sandbox/integration_test.go new file mode 100644 index 00000000..331e5209 --- /dev/null +++ b/apps/agent/internal/sandbox/integration_test.go @@ -0,0 +1,119 @@ +package sandbox + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// This test requires a real local Docker daemon and a pre-pulled trusted image. +// Explicit opt-in makes backend failure fatal, never a successful skipped check. +func TestSandboxRealDockerIntegration(t *testing.T) { + if os.Getenv("ZAKURA_SANDBOX_INTEGRATION") != "1" { + t.Skip("real Docker isolation not requested; unit fixtures do not prove isolation") + } + if os.Getenv("ZAKURA_SANDBOX_IMAGE") == "" { + t.Fatal("integration requires pre-pulled ZAKURA_SANDBOX_IMAGE") + } + if _, err := exec.LookPath("docker"); err != nil { + t.Fatalf("Docker required for opted-in integration: %v", err) + } + listContainers := func() map[string]bool { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := dockerCommand(ctx, "docker", "container", "ls", "--all", "--filter", "name=zakura-sandbox-", "--format", "{{.ID}}") + output, err := cmd.Output() + if err != nil { + t.Fatalf("cannot verify real Docker containers: %v", err) + } + ids := map[string]bool{} + for _, id := range strings.Fields(string(output)) { + ids[id] = true + } + return ids + } + baseline := listContainers() + t.Cleanup(func() { + for id := range listContainers() { + if !baseline[id] { + t.Errorf("sandbox container remains after execution: %s", id) + } + } + }) + root := t.TempDir() + if err := os.Chmod(root, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "input.txt"), []byte("workspace fixture"), 0644); err != nil { + t.Fatal(err) + } + t.Setenv("ZAKURA_TEST_PARENT_SECRET", "must-not-be-inherited") + run := func(t *testing.T, command ...string) ExecResult { + t.Helper() + result, err := Run(context.Background(), root, ExecParams{Command: command, TimeoutMs: 10000}) + if err != nil { + t.Fatal(err) + } + return result + } + t.Run("workspace and user", func(t *testing.T) { + result := run(t, "sh", "-c", "cat /workspace/input.txt; printf '\n'; id -u; pwd") + if result.ExitCode != 0 || result.Stdout != "workspace fixture\n65534\n/workspace\n" { + t.Fatalf("workspace/user mismatch: %+v", result) + } + }) + t.Run("read only workspace", func(t *testing.T) { + result := run(t, "touch", "/workspace/ordinary-output.txt") + if result.ExitCode == 0 { + t.Fatal("workspace unexpectedly writable") + } + if _, err := os.Stat(filepath.Join(root, "ordinary-output.txt")); !os.IsNotExist(err) { + t.Fatalf("unexpected host output: %v", err) + } + }) + t.Run("network and clean environment", func(t *testing.T) { + result := run(t, "sh", "-c", "test -z \"${ZAKURA_TEST_PARENT_SECRET+x}\" && test \"$(ls /sys/class/net)\" = lo") + if result.ExitCode != 0 { + t.Fatalf("network/environment policy failed: %+v", result) + } + }) + t.Run("temporary scratch", func(t *testing.T) { + result := run(t, "sh", "-c", "printf scratch > /tmp/ordinary-output.txt; cat /tmp/ordinary-output.txt") + if result.ExitCode != 0 || result.Stdout != "scratch" { + t.Fatalf("scratch failed: %+v", result) + } + }) + t.Run("output cap", func(t *testing.T) { + result := run(t, "sh", "-c", "head -c 300000 /dev/zero") + if result.ExitCode != 0 || !result.Truncated || len(result.Stdout) != MaxOutputBytes { + t.Fatalf("output bound failed: code=%d bytes=%d truncated=%v", result.ExitCode, len(result.Stdout), result.Truncated) + } + }) + t.Run("timeout", func(t *testing.T) { + result, err := Run(context.Background(), root, ExecParams{Command: []string{"sleep", "30"}, TimeoutMs: 3000}) + if err != nil || !result.TimedOut { + t.Fatalf("timeout/cleanup failure: %+v %v", result, err) + } + }) + t.Run("cancel", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + timer := time.AfterFunc(3*time.Second, cancel) + defer timer.Stop() + result, err := Run(ctx, root, ExecParams{Command: []string{"sleep", "30"}}) + if err != nil || !result.Cancelled { + t.Fatalf("cancellation/cleanup failure: %+v %v", result, err) + } + }) + t.Run("nonzero exit", func(t *testing.T) { + result := run(t, "sh", "-c", "printf diagnostic >&2; exit 7") + if result.ExitCode != 7 || !strings.Contains(result.Stderr, "diagnostic") { + t.Fatalf("exit semantics: %+v", result) + } + }) +} diff --git a/apps/agent/internal/sandbox/sandbox.go b/apps/agent/internal/sandbox/sandbox.go new file mode 100644 index 00000000..850915be --- /dev/null +++ b/apps/agent/internal/sandbox/sandbox.go @@ -0,0 +1,424 @@ +// Package sandbox runs untrusted commands in disposable, restricted containers. +// Filesystem path checks are validation, not an OS isolation boundary. +package sandbox + +import ( + "bytes" + "context" + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "io/fs" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "sync" + "time" +) + +const MaxOutputBytes = 256 * 1024 +const MaxTimeoutMs = 300000 +const MaxStdinBytes = 64 * 1024 +const MaxJobs = 128 +const MaxConcurrent = 4 + +var executionSlots = make(chan struct{}, MaxConcurrent) + +// Enabled cannot be overridden by request or environment. +func Enabled() bool { return true } + +type PolicyInfo struct { + Enabled bool `json:"enabled"` + Backend string `json:"backend"` + ImageConfigured bool `json:"imageConfigured"` + Network string `json:"network"` + WorkspaceReadOnly bool `json:"workspaceReadOnly"` + MaxTimeoutMs int `json:"maxTimeoutMs"` + MaxOutputBytes int `json:"maxOutputBytes"` + MemoryBytes int64 `json:"memoryBytes"` + PidsLimit int `json:"pidsLimit"` +} + +func Policy() PolicyInfo { + return PolicyInfo{Enabled(), "docker", os.Getenv("ZAKURA_SANDBOX_IMAGE") != "", "none", true, MaxTimeoutMs, MaxOutputBytes, 512 * 1024 * 1024, 64} +} + +type ExecParams struct { + Command []string `json:"command"` + WorkingDir string `json:"workingDir"` + Env map[string]string `json:"env"` + TimeoutMs int `json:"timeoutMs"` + Stdin string `json:"stdin"` +} +type ExecResult struct { + ExitCode int `json:"exitCode"` + Stdout string `json:"stdout"` + Stderr string `json:"stderr"` + TimedOut bool `json:"timedOut"` + Cancelled bool `json:"cancelled"` + Truncated bool `json:"truncated"` +} + +type cappedWriter struct { + mu sync.Mutex + b bytes.Buffer + truncated bool +} + +func (w *cappedWriter) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + n := len(p) + remaining := MaxOutputBytes - w.b.Len() + if len(p) > remaining { + p = p[:remaining] + w.truncated = true + } + _, _ = w.b.Write(p) + return n, nil +} +func (w *cappedWriter) snapshot() (string, bool) { + w.mu.Lock() + defer w.mu.Unlock() + return w.b.String(), w.truncated +} + +// buildArgs deliberately accepts no client-provided mounts, user, network or image. +func buildArgs(root string, p ExecParams, name string) ([]string, error) { + image := strings.TrimSpace(os.Getenv("ZAKURA_SANDBOX_IMAGE")) + if image == "" || strings.HasPrefix(image, "-") || strings.ContainsAny(image, "\x00\r\n\t ") { + return nil, errors.New("sandbox requires an operator-configured ZAKURA_SANDBOX_IMAGE") + } + if len(p.Command) == 0 || len(p.Command) > 256 || p.Command[0] == "" { + return nil, errors.New("sandbox command is required") + } + size := 0 + for _, v := range p.Command { + size += len(v) + if strings.ContainsRune(v, 0) { + return nil, errors.New("invalid command") + } + } + if size > 64*1024 || len(p.Stdin) > MaxStdinBytes { + return nil, errors.New("sandbox input limit exceeded") + } + if len(p.Env) > 0 { + return nil, errors.New("sandbox does not accept client environment variables") + } + if p.TimeoutMs < 0 || p.TimeoutMs > MaxTimeoutMs { + return nil, errors.New("sandbox timeout exceeds policy") + } + absolute, err := filepath.Abs(root) + if err != nil { + return nil, err + } + canonical, err := filepath.EvalSymlinks(absolute) + if err != nil { + return nil, fmt.Errorf("sandbox workspace: %w", err) + } + if canonical != absolute { + return nil, errors.New("sandbox workspace must not contain symlink ancestors") + } + info, err := os.Stat(canonical) + if err != nil || !info.IsDir() { + return nil, errors.New("sandbox workspace must be a directory") + } + if canonical == filepath.VolumeName(canonical)+string(filepath.Separator) || strings.ContainsAny(canonical, ",\r\n") { + return nil, errors.New("unsafe sandbox workspace root") + } + // Do not expose host IPC or device endpoints through the workspace mount. + // This check is defense in depth: operators must prevent concurrent host-side + // mutation of the validated workspace while a sandbox is running. + entries := 0 + if err := filepath.WalkDir(canonical, func(_ string, d fs.DirEntry, e error) error { + if e != nil { + return e + } + entries++ + if entries > 100000 { + return errors.New("sandbox workspace entry limit exceeded") + } + if d.Type()&(os.ModeSocket|os.ModeDevice|os.ModeNamedPipe) != 0 { + return errors.New("sandbox workspace contains a special file") + } + return nil + }); err != nil { + return nil, err + } + wd := "/workspace" + if p.WorkingDir != "" { + requested := p.WorkingDir + if requested == "/workspace" { + requested = canonical + } else if strings.HasPrefix(requested, "/workspace/") { + requested = filepath.Join(canonical, filepath.FromSlash(strings.TrimPrefix(requested, "/workspace/"))) + } + if !filepath.IsAbs(requested) { + requested = filepath.Join(canonical, requested) + } + resolved, err := filepath.EvalSymlinks(requested) + if err != nil { + return nil, fmt.Errorf("sandbox working directory: %w", err) + } + rel, err := filepath.Rel(canonical, resolved) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return nil, errors.New("working directory is outside workspace") + } + stat, err := os.Stat(resolved) + if err != nil || !stat.IsDir() { + return nil, errors.New("working directory must be a directory") + } + if rel != "." { + wd += "/" + filepath.ToSlash(rel) + } + } + return append([]string{"create", "--name", name, "--pull=never", "--rm", "--init", "--network=none", "--no-healthcheck", "--read-only", "--user=65534:65534", "--cap-drop=ALL", "--security-opt=no-new-privileges:true", "--pids-limit=64", "--memory=512m", "--memory-swap=512m", "--cpus=1", "--ulimit=nofile=256:256", "--ulimit=core=0:0", "--ipc=none", "--log-driver=none", "--tmpfs=/tmp:rw,noexec,nosuid,nodev,size=64m,mode=1777", "--mount", "type=bind,src=" + canonical + ",dst=/workspace,readonly,bind-propagation=rprivate,bind-recursive=disabled", "--workdir", wd, "--env=HOME=/tmp", "--env=TMPDIR=/tmp", "--env=PATH=/usr/local/bin:/usr/bin:/bin", "--entrypoint", p.Command[0], "-i", image}, p.Command[1:]...), nil +} + +// The Docker client receives no inherited credentials, proxy or application secrets. +// Only a local default daemon is supported; remote/context overrides are excluded. +func dockerCommand(ctx context.Context, bin string, args ...string) *exec.Cmd { + cmd := exec.CommandContext(ctx, bin, args...) + cmd.Env = []string{"PATH=/usr/local/bin:/usr/bin:/bin", "HOME=/nonexistent", "DOCKER_CONFIG=/nonexistent"} + cmd.WaitDelay = 2 * time.Second + return cmd +} + +func Run(ctx context.Context, root string, p ExecParams) (ExecResult, error) { + return run(ctx, root, p, &cappedWriter{}, &cappedWriter{}) +} +func run(ctx context.Context, root string, p ExecParams, out, errout *cappedWriter) (result ExecResult, runErr error) { + select { + case executionSlots <- struct{}{}: + defer func() { <-executionSlots }() + default: + return result, errors.New("sandbox concurrent execution limit reached") + } + var id [16]byte + if _, err := rand.Read(id[:]); err != nil { + return result, err + } + name := "zakura-sandbox-" + hex.EncodeToString(id[:]) + args, err := buildArgs(root, p, name) + if err != nil { + return result, err + } + bin, err := exec.LookPath("docker") + if err != nil { + return result, errors.New("sandbox Docker backend unavailable; host execution refused") + } + timeout := p.TimeoutMs + if timeout == 0 { + timeout = MaxTimeoutMs + } + ctx, cancel := context.WithTimeout(ctx, time.Duration(timeout)*time.Millisecond) + defer cancel() + defer func() { + result.TimedOut = errors.Is(ctx.Err(), context.DeadlineExceeded) + result.Cancelled = errors.Is(ctx.Err(), context.Canceled) + }() + + // Killing the CLI alone does not terminate its container. Always remove the + // named container with a separate bounded context, including create/start races. + defer func() { + cleanCtx, stop := context.WithTimeout(context.Background(), 10*time.Second) + defer stop() + clean := dockerCommand(cleanCtx, bin, "rm", "--force", name) + var diagnostic cappedWriter + clean.Stdout = &diagnostic + clean.Stderr = &diagnostic + if e := clean.Run(); e != nil { + // --rm already removes successfully finished containers. Confirm absence. + probe := dockerCommand(cleanCtx, bin, "container", "ls", "--all", "--filter", "name=^/"+name+"$", "--format", "{{.ID}}") + var check cappedWriter + probe.Stdout = &check + probe.Stderr = &cappedWriter{} + pe := probe.Run() + remaining, _ := check.snapshot() + if pe != nil || strings.TrimSpace(remaining) != "" { + runErr = errors.Join(runErr, errors.New("sandbox cleanup could not be verified; operator attention required")) + } + } + }() + // Image-declared volumes would allocate writable, unbounded daemon storage + // despite --read-only. Approved images must not declare any volumes. + inspect := dockerCommand(ctx, bin, "image", "inspect", "--format", "{{json .Config.Volumes}}", strings.TrimSpace(os.Getenv("ZAKURA_SANDBOX_IMAGE"))) + var imageVolumes cappedWriter + inspect.Stdout = &imageVolumes + inspect.Stderr = &cappedWriter{} + if err := inspect.Run(); err != nil { + return result, errors.New("sandbox image unavailable or policy inspection failed; host execution refused") + } + volumes, tooLarge := imageVolumes.snapshot() + if tooLarge || (strings.TrimSpace(volumes) != "null" && strings.TrimSpace(volumes) != "{}") { + return result, errors.New("sandbox image must not declare writable volumes") + } + // Creation is separate from start: a cancelled creation can at worst leave + // a stopped container, never a running untracked command. + create := dockerCommand(ctx, bin, args...) + create.Stdout = &cappedWriter{} + create.Stderr = errout + if err := create.Run(); err != nil { + result.ExitCode = -1 + result.Stderr, result.Truncated = errout.snapshot() + result.TimedOut = errors.Is(ctx.Err(), context.DeadlineExceeded) + result.Cancelled = errors.Is(ctx.Err(), context.Canceled) + if ctx.Err() != nil { + return result, errors.New("sandbox creation interrupted; a stopped container may require operator cleanup: " + name) + } + return result, fmt.Errorf("sandbox container creation failed; host execution refused: %w", err) + } + cmd := dockerCommand(ctx, bin, "start", "--attach", "--interactive", name) + cmd.Stdout = out + cmd.Stderr = errout + cmd.Stdin = strings.NewReader(p.Stdin) + e := cmd.Run() + result.Stdout, result.Truncated = out.snapshot() + var truncated bool + result.Stderr, truncated = errout.snapshot() + result.Truncated = result.Truncated || truncated + result.TimedOut = errors.Is(ctx.Err(), context.DeadlineExceeded) + result.Cancelled = errors.Is(ctx.Err(), context.Canceled) + if e != nil { + var exit *exec.ExitError + if errors.As(e, &exit) { + result.ExitCode = exit.ExitCode() + if result.ExitCode == 125 { + runErr = errors.New("sandbox container failed to start; host execution refused") + } + } else { + result.ExitCode = -1 + runErr = fmt.Errorf("sandbox execution failed: %w", e) + } + } + return result, runErr +} + +type JobSnap struct { + ID string `json:"id"` + Stdout string `json:"stdout"` + Stderr string `json:"stderr"` + ExitCode *int `json:"exitCode"` + Running bool `json:"running"` + TimedOut bool `json:"timedOut"` + Cancelled bool `json:"cancelled"` + Truncated bool `json:"truncated"` + Error string `json:"error,omitempty"` +} +type job struct { + mu sync.Mutex + id string + root string + out, errout cappedWriter + cancel context.CancelFunc + result *ExecResult + err error +} +type Registry struct { + mu sync.Mutex + jobs map[string]*job + next uint64 +} + +func NewRegistry() *Registry { return &Registry{jobs: make(map[string]*job)} } +func (r *Registry) Start(root string, p ExecParams) (*JobSnap, error) { + if _, err := buildArgs(root, p, "validation"); err != nil { + return nil, err + } + if _, err := exec.LookPath("docker"); err != nil { + return nil, errors.New("sandbox Docker backend unavailable; host execution refused") + } + r.mu.Lock() + defer r.mu.Unlock() + // Completed results are evicted before refusing further concurrent jobs. + if len(r.jobs) >= MaxJobs { + for id, j := range r.jobs { + j.mu.Lock() + done := j.result != nil + j.mu.Unlock() + if done { + delete(r.jobs, id) + } + } + } + if len(r.jobs) >= MaxJobs { + return nil, errors.New("sandbox concurrent job limit reached") + } + r.next++ + var random [16]byte + if _, err := rand.Read(random[:]); err != nil { + return nil, err + } + ctx, cancel := context.WithCancel(context.Background()) + j := &job{id: hex.EncodeToString(random[:]) + "-" + strconv.FormatUint(r.next, 10), root: root, cancel: cancel} + r.jobs[j.id] = j + go func() { + result, err := run(ctx, root, p, &j.out, &j.errout) + j.mu.Lock() + j.result = &result + j.err = err + j.mu.Unlock() + cancel() + }() + return j.snapshot(), nil +} +func (j *job) snapshot() *JobSnap { + j.mu.Lock() + defer j.mu.Unlock() + out, a := j.out.snapshot() + stderr, b := j.errout.snapshot() + s := &JobSnap{ID: j.id, Stdout: out, Stderr: stderr, Running: j.result == nil, Truncated: a || b} + if j.result != nil { + code := j.result.ExitCode + s.ExitCode = &code + s.TimedOut = j.result.TimedOut + s.Cancelled = j.result.Cancelled + } + if j.err != nil { + s.Error = j.err.Error() + } + return s +} +func (r *Registry) Get(id string) *JobSnap { + r.mu.Lock() + j := r.jobs[id] + r.mu.Unlock() + if j == nil { + return nil + } + return j.snapshot() +} +func (r *Registry) Kill(id string) *JobSnap { + r.mu.Lock() + j := r.jobs[id] + r.mu.Unlock() + if j == nil { + return nil + } + j.cancel() + return j.snapshot() +} + +// Scoped access binds a job to the validated caller workspace. +func (r *Registry) GetScoped(id, root string) *JobSnap { + r.mu.Lock() + j := r.jobs[id] + r.mu.Unlock() + if j == nil || j.root != root { + return nil + } + return j.snapshot() +} +func (r *Registry) KillScoped(id, root string) *JobSnap { + r.mu.Lock() + j := r.jobs[id] + r.mu.Unlock() + if j == nil || j.root != root { + return nil + } + j.cancel() + return j.snapshot() +} diff --git a/apps/agent/internal/sandbox/validation_test.go b/apps/agent/internal/sandbox/validation_test.go new file mode 100644 index 00000000..ab846426 --- /dev/null +++ b/apps/agent/internal/sandbox/validation_test.go @@ -0,0 +1,232 @@ +package sandbox + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + "time" +) + +func testPolicy(t *testing.T) string { + t.Helper() + t.Setenv("ZAKURA_SANDBOX_IMAGE", "zakura-test:local") + return t.TempDir() +} + +func TestSandboxRestrictiveArgumentPolicy(t *testing.T) { + root := testPolicy(t) + args, err := buildArgs(root, ExecParams{Command: []string{"printf", "%s", "hello"}}, "test-policy") + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"--pull=never", "--network=none", "--read-only", "--user=65534:65534", "--cap-drop=ALL", "--security-opt=no-new-privileges:true", "--pids-limit=64", "--memory=512m", "--memory-swap=512m", "--cpus=1", "--ipc=none", "--log-driver=none", "--tmpfs=/tmp:rw,noexec,nosuid,nodev,size=64m,mode=1777"} { + found := false + for _, arg := range args { + if arg == want { + found = true + } + } + if !found { + t.Errorf("missing restriction %s", want) + } + } + joined := strings.Join(args, "\n") + if !strings.Contains(joined, "dst=/workspace,readonly,bind-propagation=rprivate") { + t.Error("workspace mount is not read-only/private") + } + if got := strings.Join(args[len(args)-3:], "|"); got != "zakura-test:local|%s|hello" { + t.Fatalf("command arguments changed: %s", got) + } +} + +func TestSandboxInputPolicyRejectsBeforeExecution(t *testing.T) { + root := testPolicy(t) + cases := map[string]ExecParams{ + "empty": {}, "empty executable": {Command: []string{""}}, + "too many args": {Command: make([]string, 257)}, + "oversize command": {Command: []string{strings.Repeat("x", 64*1024+1)}}, + "oversize stdin": {Command: []string{"printf"}, Stdin: strings.Repeat("x", MaxStdinBytes+1)}, + "negative timeout": {Command: []string{"printf"}, TimeoutMs: -1}, + "excessive timeout": {Command: []string{"printf"}, TimeoutMs: MaxTimeoutMs + 1}, + "custom environment": {Command: []string{"printf"}, Env: map[string]string{"EXAMPLE": "value"}}, + } + for name, p := range cases { + t.Run(name, func(t *testing.T) { + if _, err := buildArgs(root, p, "validation"); err == nil { + t.Fatal("invalid request accepted") + } + }) + } + if _, err := buildArgs(root, ExecParams{Command: []string{"printf"}, TimeoutMs: MaxTimeoutMs, Stdin: strings.Repeat("x", MaxStdinBytes)}, "boundary"); err != nil { + t.Fatal(err) + } +} + +func TestSandboxUnavailableFailsClosed(t *testing.T) { + root := testPolicy(t) + p := ExecParams{Command: []string{"printf", "hello"}} + t.Setenv("ZAKURA_SANDBOX_IMAGE", "") + if _, err := Run(context.Background(), root, p); err == nil { + t.Fatal("missing image accepted") + } + t.Setenv("ZAKURA_SANDBOX_IMAGE", "zakura-test:local") + t.Setenv("PATH", t.TempDir()) + if _, err := Run(context.Background(), root, p); err == nil || !strings.Contains(err.Error(), "host execution refused") { + t.Fatalf("missing Docker must fail closed: %v", err) + } + if _, err := NewRegistry().Start(root, p); err == nil { + t.Fatal("background execution accepted missing backend") + } +} + +func TestSandboxWorkingDirectoryMapping(t *testing.T) { + root := testPolicy(t) + if err := os.Mkdir(filepath.Join(root, "project"), 0755); err != nil { + t.Fatal(err) + } + args, err := buildArgs(root, ExecParams{Command: []string{"pwd"}, WorkingDir: "project"}, "workdir") + if err != nil { + t.Fatal(err) + } + for i, arg := range args { + if arg == "--workdir" && args[i+1] == "/workspace/project" { + return + } + } + t.Fatal("working directory was not mapped inside workspace") +} + +func TestSandboxOutputCapAndConcurrentSnapshots(t *testing.T) { + var w cappedWriter + input := []byte(strings.Repeat("a", MaxOutputBytes)) + if n, err := w.Write(input); n != len(input) || err != nil { + t.Fatalf("write: %d %v", n, err) + } + if _, truncated := w.snapshot(); truncated { + t.Fatal("exact boundary marked truncated") + } + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 20; j++ { + _, _ = w.Write([]byte("extra")) + _, _ = w.snapshot() + } + }() + } + wg.Wait() + output, truncated := w.snapshot() + if len(output) != MaxOutputBytes || !truncated { + t.Fatalf("cap failed: bytes=%d truncated=%v", len(output), truncated) + } +} + +// fakeDocker tests client orchestration only. It provides no container or OS isolation. +func fakeDocker(t *testing.T, runBody string) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("synthetic POSIX client fixture") + } + dir := t.TempDir() + script := "#!/bin/sh\ncase \"$1\" in\nimage) printf 'null\\n' ;;\ncreate) exit 0 ;;\nstart) " + runBody + " ;;\nrm) exit 0 ;;\ncontainer) exit 0 ;;\n*) exit 1 ;;\nesac\n" + if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0700); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir) +} + +func TestSandboxSyntheticClientTimeoutAndCancellation(t *testing.T) { + root := testPolicy(t) + fakeDocker(t, "exec /bin/sleep 30") + t.Run("timeout", func(t *testing.T) { + result, err := Run(context.Background(), root, ExecParams{Command: []string{"sleep", "30"}, TimeoutMs: 30}) + if err != nil { + t.Fatal(err) + } + if !result.TimedOut || result.Cancelled { + t.Fatalf("wrong timeout state: %+v", result) + } + }) + t.Run("cancel", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + result, _ := Run(ctx, root, ExecParams{Command: []string{"sleep", "30"}}) + if !result.Cancelled || result.TimedOut { + t.Fatalf("wrong cancel state: %+v", result) + } + }) +} + +func TestSandboxSyntheticClientQuotaAndScopedPermission(t *testing.T) { + root := testPolicy(t) + fakeDocker(t, "exit 0") + registry := NewRegistry() + for i := 0; i < MaxJobs; i++ { + id := string(rune(i + 1)) + registry.jobs[id] = &job{id: id, root: root, cancel: func() {}} + } + if _, err := registry.Start(root, ExecParams{Command: []string{"printf", "hello"}}); err == nil || !strings.Contains(err.Error(), "limit") { + t.Fatalf("concurrent quota not enforced: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + registry.jobs["scoped"] = &job{id: "scoped", root: root, cancel: cancel} + if registry.GetScoped("scoped", "other-workspace") != nil || registry.KillScoped("scoped", "other-workspace") != nil { + t.Fatal("cross-workspace job exposed") + } + if ctx.Err() != nil { + t.Fatal("cross-workspace kill cancelled job") + } + if registry.GetScoped("scoped", root) == nil || registry.KillScoped("scoped", root) == nil { + t.Fatal("owner cannot access job") + } + if ctx.Err() != context.Canceled { + t.Fatal("owner cancellation not delivered") + } +} + +func TestSandboxDockerClientDoesNotInheritSecrets(t *testing.T) { + t.Setenv("ZAKURA_TEST_SECRET", "do-not-forward") + t.Setenv("DOCKER_HOST", "example.invalid") + cmd := dockerCommand(context.Background(), "docker", "version") + for _, entry := range cmd.Env { + if strings.HasPrefix(entry, "ZAKURA_TEST_SECRET=") || strings.HasPrefix(entry, "DOCKER_HOST=") { + t.Fatalf("inherited disallowed environment: %s", entry) + } + } + if cmd.WaitDelay <= 0 || cmd.WaitDelay > 5*time.Second { + t.Fatal("client wait is not bounded") + } +} + +func TestSandboxGlobalExecutionQuota(t *testing.T) { + for i := 0; i < MaxConcurrent; i++ { + executionSlots <- struct{}{} + } + defer func() { + for i := 0; i < MaxConcurrent; i++ { + <-executionSlots + } + }() + if _, err := Run(context.Background(), t.TempDir(), ExecParams{Command: []string{"printf", "hello"}}); err == nil || !strings.Contains(err.Error(), "concurrent execution limit") { + t.Fatalf("global quota not enforced: %v", err) + } +} + +func TestSandboxImageVolumesFailClosed(t *testing.T) { + root := testPolicy(t) + fakeDocker(t, "exit 0") + path := filepath.Join(os.Getenv("PATH"), "docker") + if err := os.WriteFile(path, []byte("#!/bin/sh\ncase \"$1\" in\nimage) printf '{\"/data\":{}}\\n' ;;\n*) exit 1 ;;\nesac\n"), 0700); err != nil { + t.Fatal(err) + } + if _, err := Run(context.Background(), root, ExecParams{Command: []string{"printf", "hello"}}); err == nil || !strings.Contains(strings.ToLower(err.Error()), "volume") { + t.Fatalf("image with writable volume accepted: %v", err) + } +} diff --git a/apps/server/internal/runtime/acp.go b/apps/server/internal/runtime/acp.go index 2b8ff833..1a6da6be 100644 --- a/apps/server/internal/runtime/acp.go +++ b/apps/server/internal/runtime/acp.go @@ -112,6 +112,13 @@ func findAdapter(id string) (acpAdapter, bool) { return acpAdapter{}, false } func (h *handler) runtimeExec(ctx context.Context, tenant, agent, command string, args ...string) (map[string]any, error) { + return h.runtimeExecWithMode(ctx, tenant, agent, "", command, args...) +} + +func (h *handler) runtimeExecWithMode(ctx context.Context, tenant, agent, mode, command string, args ...string) (map[string]any, error) { + if mode != "" && mode != "host" && mode != "sandbox" { + return nil, errors.New("execution_mode must be host or sandbox") + } var nodeID, spaceID, workspaceKind string var rec struct { NodeID string `gorm:"column:node_id"` @@ -123,20 +130,40 @@ func (h *handler) runtimeExec(ctx context.Context, tenant, agent, command string return nil, errors.New("agent runtime node is not online") } nodeID, spaceID, workspaceKind = rec.NodeID, rec.SpaceID, rec.WorkspaceKind + if mode == "sandbox" && workspaceKind != "host" { + return nil, errors.New("sandbox execution requires a host workspace; this workspace mode is unsupported") + } session, err := h.hub.get(nodeID) if err != nil { return nil, err } + if mode == "sandbox" { + var policy struct { + Sandbox struct { + Enabled bool `json:"enabled"` + } `json:"sandbox"` + } + if err := session.call(ctx, "sandbox.policy", map[string]any{}, &policy); err != nil || !policy.Sandbox.Enabled { + return nil, errors.New("runner does not support sandbox execution; upgrade or configure the runner") + } + } argv := append([]string{command}, args...) timeoutMS := int64(30_000) if deadline, ok := ctx.Deadline(); ok { - if remaining := time.Until(deadline).Milliseconds(); remaining > timeoutMS { + if remaining := time.Until(deadline).Milliseconds(); remaining > 0 { timeoutMS = remaining + } else { + return nil, context.DeadlineExceeded } } result := map[string]any{} if workspaceKind == "host" { - err = session.call(ctx, "host.exec", map[string]any{"spaceId": spaceID, "command": argv, "workingDir": "/workspace", "timeoutMs": timeoutMS}, &result) + params := map[string]any{"spaceId": spaceID, "command": argv, "workingDir": "/workspace", "timeoutMs": timeoutMS, "executionMode": mode} + if mode == "sandbox" { + result, err = runSandboxCommand(ctx, session, params) + } else { + err = session.call(ctx, "host.exec", params, &result) + } } else { var containers []struct { DockerID string `json:"dockerId"` @@ -158,6 +185,28 @@ func (h *handler) runtimeExec(ctx context.Context, tenant, agent, command string if err != nil { return nil, err } + // Describe the transport that actually ran the command. A normal workspace + // container provides container isolation but does not apply the restricted + // sandbox policy (network, read-only mount, resource limits, and cleanup). + if mode == "sandbox" && (result["executionMode"] != "sandbox" || result["isolated"] != true) { + return nil, errors.New("runner did not confirm sandbox execution") + } + switch { + case workspaceKind != "host": + result["executionMode"] = "workspace-container" + result["isolated"] = true + result["restrictedPolicyApplied"] = false + case result["executionMode"] == "sandbox": + // Operator enforcement may select sandbox even for a legacy host call. + if result["isolated"] != true { + return nil, errors.New("runner did not confirm sandbox execution") + } + result["restrictedPolicyApplied"] = true + default: + result["executionMode"] = "host" + result["isolated"] = false + result["restrictedPolicyApplied"] = false + } if code, ok := result["exitCode"].(float64); ok && code != 0 { return result, fmt.Errorf("command exited %d: %v", int(code), result["stderr"]) } diff --git a/apps/server/internal/runtime/acp_runtime.go b/apps/server/internal/runtime/acp_runtime.go index 58d9c365..d69d63ce 100644 --- a/apps/server/internal/runtime/acp_runtime.go +++ b/apps/server/internal/runtime/acp_runtime.go @@ -89,6 +89,13 @@ func (m *acpRuntimeManager) closeAll() { } func (m *acpRuntimeManager) ensure(ctx context.Context, tenantID, agentID, sid string) (*acpLiveRuntime, error) { + required, policyErr := m.h.agentSandboxRequired(ctx, tenantID, agentID) + if policyErr != nil { + return nil, policyErr + } + if required { + return nil, errors.New("ACP adapters are unavailable for sandbox agents") + } key := tenantID + "\x00" + sid m.mu.Lock() if current := m.runtimes[key]; current != nil { diff --git a/apps/server/internal/runtime/agent_tool_runners.go b/apps/server/internal/runtime/agent_tool_runners.go index 988a635c..194b6768 100644 --- a/apps/server/internal/runtime/agent_tool_runners.go +++ b/apps/server/internal/runtime/agent_tool_runners.go @@ -253,6 +253,12 @@ func (h *handler) runFSWrite(ctx context.Context, tenant, agent string, args map } func (h *handler) runShellExec(ctx context.Context, tenant, agent string, args map[string]any) (json.RawMessage, error) { + if value, exists := args["execution_mode"]; exists { + mode, ok := value.(string) + if !ok || (mode != "host" && mode != "sandbox") { + return nil, errors.New("execution_mode must be host or sandbox") + } + } command := builtinStringArg(args, "command") if strings.TrimSpace(command) == "" { return nil, errors.New("command is required") @@ -266,7 +272,7 @@ func (h *handler) runShellExec(ctx context.Context, tenant, agent string, args m } callCtx, cancel := context.WithTimeout(ctx, time.Duration(timeoutMS)*time.Millisecond) defer cancel() - result, err := h.runtimeExec(callCtx, tenant, agent, "bash", "-lc", command) + result, err := h.runtimeExecWithMode(callCtx, tenant, agent, builtinStringArg(args, "execution_mode"), "bash", "-lc", command) if result == nil { if err != nil { return nil, err @@ -280,8 +286,14 @@ func (h *handler) runShellExec(ctx context.Context, tenant, agent string, args m stdout, _ = h.truncateToolText(ctx, tenant, agent, stdout, "shell") stderr, _ = h.truncateToolText(ctx, tenant, agent, stderr, "shell") return builtinJSON(map[string]any{ - "stdout": stdout, - "stderr": stderr, - "exitCode": result["exitCode"], + "stdout": stdout, + "stderr": stderr, + "exitCode": result["exitCode"], + "executionMode": result["executionMode"], + "isolated": result["isolated"], + "restrictedPolicyApplied": result["restrictedPolicyApplied"], + "timedOut": result["timedOut"], + "cancelled": result["cancelled"], + "truncated": result["truncated"], }) } diff --git a/apps/server/internal/runtime/agent_tools.go b/apps/server/internal/runtime/agent_tools.go index 59f388ac..2f14ac4c 100644 --- a/apps/server/internal/runtime/agent_tools.go +++ b/apps/server/internal/runtime/agent_tools.go @@ -266,6 +266,13 @@ func (h *handler) agentMCPCatalogTools(ctx context.Context, tenant, agentID, spa } func (h *handler) agentToolCatalog(ctx context.Context, tenant, agentID string) (catalog agentCatalog, err error) { + sandbox, policyErr := h.agentSandboxRequired(ctx, tenant, agentID) + if policyErr != nil { + return agentCatalog{}, policyErr + } + if sandbox { + return sandboxAgentCatalog(), nil + } agent, err := h.store.GetAgent(ctx, tenant, agentID) if err != nil { return agentCatalog{}, err @@ -308,9 +315,10 @@ func (h *handler) agentToolCatalog(ctx context.Context, tenant, agentID string) "path": map[string]any{"type": "string"}, "content": map[string]any{"type": "string"}, }, "path", "content"), - directBuiltin("shell_exec", "Run a shell command in the workspace environment.", map[string]any{ - "command": map[string]any{"type": "string", "description": "Shell command to run in the workspace."}, - "timeout_ms": map[string]any{"type": "integer", "description": "Deadline in ms. Default 60000, max 300000."}, + directBuiltin("shell_exec", "Run a shell command. Legacy host mode uses the configured host or workspace container without the restricted sandbox policy. Sandbox mode uses a restricted per-command container with a read-only workspace and no network, and requires a configured runner.", map[string]any{ + "command": map[string]any{"type": "string", "description": "Shell command to run in the workspace."}, + "execution_mode": map[string]any{"type": "string", "enum": []string{"host", "sandbox"}, "description": "sandbox selects restricted per-command execution. host preserves existing workspace execution: bare host or workspace container according to workspace configuration. The response identifies the actual transport and whether the restricted policy applied. Runner enforcement cannot be disabled by this setting."}, + "timeout_ms": map[string]any{"type": "integer", "description": "Deadline in ms. Default 60000, max 300000."}, }, "command"), directBuiltin("codemode", "Run JavaScript that calls the agent's other tools. The script runs as the body of an async function (top-level await and return allowed) inside the workspace. Call tools as async functions on the tools object, e.g. const r = await tools.mcp__github__search_issues({query: \"x\"}). Use text(value) to append output; return a value to finish. Compose calls and filter results so only what you need comes back.", map[string]any{ "code": map[string]any{"type": "string", "description": "JavaScript source."}, @@ -474,6 +482,13 @@ type cachedCatalog struct { } func (h *handler) cachedAgentToolCatalog(ctx context.Context, tenant, agentID string) (agentCatalog, error) { + sandbox, policyErr := h.agentSandboxRequired(ctx, tenant, agentID) + if policyErr != nil { + return agentCatalog{}, policyErr + } + if sandbox { + return sandboxAgentCatalog(), nil + } key := tenant + "/" + agentID h.agentToolMu.Lock() if h.agentToolCache != nil { diff --git a/apps/server/internal/runtime/sandbox_exec.go b/apps/server/internal/runtime/sandbox_exec.go new file mode 100644 index 00000000..65b9797d --- /dev/null +++ b/apps/server/internal/runtime/sandbox_exec.go @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +package runtime + +import ( + "context" + "errors" + "fmt" + "time" +) + +// Use the runner's scoped job lifecycle so cancelling an agent run also cancels +// its container. Cleanup has a separate deadline and never falls back to host. +func runSandboxCommand(ctx context.Context, runner *runnerSession, params map[string]any) (result map[string]any, err error) { + if ctx.Err() != nil { + return nil, ctx.Err() + } + result = map[string]any{} + startCtx, stopStart := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second) + err = runner.call(startCtx, "host.exec.start", params, &result) + stopStart() + if err != nil { + return nil, fmt.Errorf("sandbox start could not be confirmed; any started job remains bounded by its timeout: %w", err) + } + id, _ := result["id"].(string) + if id == "" { + return nil, errors.New("runner did not return a sandbox job identity") + } + scoped := map[string]any{"id": id, "spaceId": params["spaceId"], "executionMode": "sandbox"} + running := true + defer func() { + if !running { + return + } + cleanup, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if cleanupErr := cancelSandboxJob(cleanup, runner, scoped); cleanupErr != nil { + result = nil + err = errors.Join(err, errors.New("sandbox cancellation could not be confirmed; the job remains bounded by its timeout")) + } + }() + for { + if result["executionMode"] != "sandbox" || result["isolated"] != true { + return nil, errors.New("runner did not confirm sandbox execution") + } + active, ok := result["running"].(bool) + if !ok { + return nil, errors.New("runner returned invalid sandbox lifecycle status") + } + if !active { + if failure, _ := result["error"].(string); failure != "" { + return nil, errors.New("sandbox execution failed; runner configuration or cleanup requires attention") + } + if result["exitCode"] == nil { + return nil, errors.New("sandbox ended without an exit status") + } + running = false + return result, nil + } + timer := time.NewTimer(100 * time.Millisecond) + select { + case <-ctx.Done(): + timer.Stop() + return nil, fmt.Errorf("sandbox run interrupted: %w", ctx.Err()) + case <-timer.C: + } + result = map[string]any{} + if err = runner.call(ctx, "host.exec.get", scoped, &result); err != nil { + return nil, err + } + } +} + +func cancelSandboxJob(ctx context.Context, runner *runnerSession, scoped map[string]any) error { + method := "host.exec.kill" + for { + var state map[string]any + if err := runner.call(ctx, method, scoped, &state); err != nil { + return err + } + if state["executionMode"] != "sandbox" || state["isolated"] != true { + return errors.New("unconfirmed sandbox cancellation") + } + running, ok := state["running"].(bool) + if !ok { + return errors.New("invalid sandbox cancellation status") + } + if !running { + if failure, _ := state["error"].(string); failure != "" { + return errors.New("sandbox cleanup was not confirmed") + } + return nil + } + timer := time.NewTimer(100 * time.Millisecond) + select { + case <-ctx.Done(): + timer.Stop() + return ctx.Err() + case <-timer.C: + } + method = "host.exec.get" + } +} diff --git a/apps/server/internal/runtime/sandbox_policy.go b/apps/server/internal/runtime/sandbox_policy.go new file mode 100644 index 00000000..bb8c4165 --- /dev/null +++ b/apps/server/internal/runtime/sandbox_policy.go @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +package runtime + +import ( + "context" + "encoding/json" + "errors" + "fmt" +) + +// This policy lives in the tenant-owned agent configuration, not tool arguments. +// All tools are denied by default; only the bounded execution transport is +// supported. It deliberately makes no claim to isolate arbitrary ACP adapters. +func (h *handler) agentSandboxRequired(ctx context.Context, tenant, agent string) (bool, error) { + record, err := h.store.GetAgent(ctx, tenant, agent) + if err != nil { + return false, err + } + var config map[string]json.RawMessage + if len(record.Config) > 0 && json.Unmarshal(record.Config, &config) != nil { + return false, errors.New("invalid agent execution policy") + } + raw, present := config["executionMode"] + if !present { + return false, nil + } + var mode string + if json.Unmarshal(raw, &mode) != nil || (mode != "host" && mode != "sandbox") { + return false, errors.New("agent executionMode must be host or sandbox") + } + return mode == "sandbox", nil +} + +func (h *handler) sandboxToolPolicy(ctx context.Context, tenant, agent, name string, args json.RawMessage) (json.RawMessage, error) { + required, err := h.agentSandboxRequired(ctx, tenant, agent) + if err != nil { + return nil, err + } + if !required { + return args, nil + } + if name == "ask_user" { + return args, nil + } + if name != "shell_exec" { + return nil, fmt.Errorf("tool %s is unavailable for sandbox agents", name) + } + var parsed map[string]any + if err := json.Unmarshal(args, &parsed); err != nil || parsed == nil { + return nil, errors.New("invalid sandbox tool arguments") + } + parsed["execution_mode"] = "sandbox" + return json.Marshal(parsed) +} + +func sandboxAgentCatalog() agentCatalog { + return agentCatalog{Tools: []agentTool{directBuiltin("shell_exec", "Execute in the configured restricted sandbox. Workspace is read-only; only bounded /tmp is writable. No network. Only shell_exec and ask_user are allowed; ACP adapters are unavailable.", map[string]any{ + "command": map[string]any{"type": "string"}, + "timeout_ms": map[string]any{"type": "integer", "description": "Deadline in ms, maximum 300000."}, + }, "command"), directBuiltin("ask_user", "Ask the user a question and wait for their reply.", map[string]any{"question": map[string]any{"type": "string"}, "title": map[string]any{"type": "string"}}, "question")}} +} diff --git a/apps/server/internal/runtime/sandbox_policy_test.go b/apps/server/internal/runtime/sandbox_policy_test.go new file mode 100644 index 00000000..77ff89c7 --- /dev/null +++ b/apps/server/internal/runtime/sandbox_policy_test.go @@ -0,0 +1,261 @@ +package runtime + +import ( + "context" + "encoding/json" + "testing" +) + +func TestSandboxAgentPolicyForcesShellAndDeniesOtherTools(t *testing.T) { + d := testDeps(t) + seedTenant(t, d, "tenant") + store := NewStore(d) + space, err := store.CreateSpace(context.Background(), "tenant", Space{Name: "sandbox"}) + if err != nil { + t.Fatal(err) + } + agent, err := store.CreateAgent(context.Background(), "tenant", Agent{Name: "sandbox", SpaceID: space.ID, Config: json.RawMessage(`{"executionMode":"sandbox"}`)}) + if err != nil { + t.Fatal(err) + } + h := &handler{deps: d, store: store} + raw, err := h.sandboxToolPolicy(context.Background(), "tenant", agent.ID, "shell_exec", json.RawMessage(`{"command":"echo hello","execution_mode":"host"}`)) + if err != nil { + t.Fatal(err) + } + var args map[string]any + _ = json.Unmarshal(raw, &args) + if args["execution_mode"] != "sandbox" { + t.Fatalf("model bypassed sandbox: %s", raw) + } + for _, name := range []string{"fs_read", "fs_write", "apply_patch", "codemode", "delegate_agent", "browser_open", "computer_click", "mcp__evil__shell_exec", "instance:shell_exec", " shell_exec", "SHELL_EXEC", "unknown_future_tool"} { + if _, err := h.dispatchAgentTool(context.Background(), "tenant", agent.ID, "", "", name, json.RawMessage(`{}`)); err == nil { + t.Fatalf("allowed %s", name) + } + } + if _, err := h.sandboxToolPolicy(context.Background(), "other-tenant", agent.ID, "shell_exec", json.RawMessage(`{}`)); err == nil { + t.Fatal("cross-tenant agent allowed") + } + catalog, err := h.cachedAgentToolCatalog(context.Background(), "tenant", agent.ID) + if err != nil { + t.Fatal(err) + } + if len(catalog.Tools) != 2 { + t.Fatalf("unexpected sandbox catalog: %+v", catalog) + } + for _, tool := range catalog.Tools { + if tool.Name != "shell_exec" && tool.Name != "ask_user" { + t.Fatalf("unsafe tool: %s", tool.Name) + } + } + manager := newACPRuntimeManager(h) + if _, err := manager.ensure(context.Background(), "tenant", agent.ID, "sid"); err == nil { + t.Fatal("sandbox ACP allowed") + } + for _, bad := range []any{"invalid", false, nil} { + _, err = store.UpdateAgent(context.Background(), "tenant", agent.ID, map[string]any{"config": map[string]any{"executionMode": bad}}) + if err != nil { + t.Fatal(err) + } + if _, err = h.agentSandboxRequired(context.Background(), "tenant", agent.ID); err == nil { + t.Fatalf("invalid mode accepted: %#v", bad) + } + } +} + +func TestSandboxShellUsesPreflightAndNeverFallsBack(t *testing.T) { + for _, scenario := range []string{"supported", "old_runner", "mismatch", "cleanup_error"} { + t.Run(scenario, func(t *testing.T) { + d := testDeps(t) + seedTenant(t, d, "tenant") + store := NewStore(d) + node := "node" + space, err := store.CreateSpace(context.Background(), "tenant", Space{Name: "S", RuntimeNodeID: &node, WorkspaceKind: "host"}) + if err != nil { + t.Fatal(err) + } + agent, err := store.CreateAgent(context.Background(), "tenant", Agent{Name: "A", SpaceID: space.ID}) + if err != nil { + t.Fatal(err) + } + calls := 0 + h := builtinToolHarness(t, d, "rnr_sandbox", func(method string, p map[string]any) (any, error) { + switch method { + case "sys.info": + return map[string]any{"version": "test", "kind": "computer", "storageRoot": "/srv/zakura", "capabilities": map[string]any{}, "hostInfo": map[string]any{}}, nil + case "sandbox.policy": + return map[string]any{"sandbox": map[string]any{"enabled": scenario != "old_runner"}}, nil + case "host.exec.start": + calls++ + if p["executionMode"] != "sandbox" || p["spaceId"] != space.ID { + t.Errorf("unsafe dispatch: %#v", p) + } + if scenario == "cleanup_error" { + return map[string]any{"id": "job", "running": false, "executionMode": "sandbox", "isolated": true, "exitCode": 0, "error": "cleanup failure"}, nil + } + if scenario == "mismatch" { + return map[string]any{"id": "job", "running": false, "executionMode": "host", "isolated": false, "exitCode": 0}, nil + } + return map[string]any{"id": "job", "running": false, "executionMode": "sandbox", "isolated": true, "exitCode": 0, "stdout": "hello", "stderr": "", "truncated": true}, nil + case "host.exec.kill": + return map[string]any{"running": false, "executionMode": "sandbox", "isolated": true}, nil + default: + t.Errorf("unexpected RPC: %s", method) + return nil, nil + } + }) + raw, err := h.runShellExec(context.Background(), "tenant", agent.ID, map[string]any{"command": "echo hello", "execution_mode": "sandbox"}) + if scenario == "supported" { + if err != nil { + t.Fatal(err) + } + var result map[string]any + _ = json.Unmarshal(raw, &result) + if result["executionMode"] != "sandbox" || result["truncated"] != true { + t.Fatalf("lost policy/output state: %s", raw) + } + if calls != 1 { + t.Fatalf("calls %d", calls) + } + } else { + if err == nil { + t.Fatalf("unconfirmed sandbox result accepted: %s", raw) + } + if scenario == "old_runner" && calls != 0 { + t.Fatal("old runner executed command") + } + } + }) + } +} + +func TestSandboxCancellationUsesIndependentCleanupContext(t *testing.T) { + d := testDeps(t) + seedTenant(t, d, "tenant") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + killed := make(chan map[string]any, 1) + h := builtinToolHarness(t, d, "rnr_cancel", func(method string, p map[string]any) (any, error) { + switch method { + case "sys.info": + return map[string]any{"version": "test", "kind": "computer", "storageRoot": "/srv/zakura", "capabilities": map[string]any{}, "hostInfo": map[string]any{}}, nil + case "host.exec.start": + cancel() + return map[string]any{"id": "job", "executionMode": "sandbox", "isolated": true, "running": true, "exitCode": nil}, nil + case "host.exec.kill": + killed <- p + return map[string]any{"id": "job", "executionMode": "sandbox", "isolated": true, "running": false, "exitCode": 137}, nil + default: + t.Errorf("unexpected RPC: %s", method) + return nil, nil + } + }) + runner, err := h.hub.get("node") + if err != nil { + t.Fatal(err) + } + _, err = runSandboxCommand(ctx, runner, map[string]any{"spaceId": "space", "executionMode": "sandbox", "command": []string{"true"}}) + if err == nil { + t.Fatal("cancelled context succeeded") + } + select { + case p := <-killed: + if p["spaceId"] != "space" || p["executionMode"] != "sandbox" || p["id"] != "job" { + t.Fatalf("unscoped cleanup: %#v", p) + } + default: + t.Fatal("cancel did not stop job") + } +} + +func TestRuntimeExecutionClassification(t *testing.T) { + for _, tc := range []struct { + name, workspace, requested, reported string + reportedIsolation bool + expected string + isolated, restricted, wantError bool + }{ + {name: "legacy host", workspace: "host", expected: "host"}, + {name: "legacy workspace container", workspace: "container", expected: "workspace-container", isolated: true}, + {name: "workspace container cannot claim restricted policy", workspace: "container", reported: "sandbox", reportedIsolation: true, expected: "workspace-container", isolated: true}, + {name: "restricted sandbox", workspace: "host", requested: "sandbox", reported: "sandbox", reportedIsolation: true, expected: "sandbox", isolated: true, restricted: true}, + {name: "operator enforced sandbox", workspace: "host", reported: "sandbox", reportedIsolation: true, expected: "sandbox", isolated: true, restricted: true}, + {name: "missing sandbox isolation", workspace: "host", requested: "sandbox", reported: "sandbox", wantError: true}, + {name: "container is not restricted sandbox", workspace: "host", requested: "sandbox", reported: "workspace-container", reportedIsolation: true, wantError: true}, + {name: "operator sandbox missing isolation", workspace: "host", reported: "sandbox", wantError: true}, + } { + t.Run(tc.name, func(t *testing.T) { + d := testDeps(t) + seedTenant(t, d, "tenant") + store := NewStore(d) + node := "node" + space, err := store.CreateSpace(context.Background(), "tenant", Space{Name: "S", RuntimeNodeID: &node, WorkspaceKind: tc.workspace}) + if err != nil { + t.Fatal(err) + } + agent, err := store.CreateAgent(context.Background(), "tenant", Agent{Name: "A", SpaceID: space.ID}) + if err != nil { + t.Fatal(err) + } + h := builtinToolHarness(t, d, "rnr_classification", func(method string, p map[string]any) (any, error) { + switch method { + case "sys.info": + return map[string]any{"version": "test", "kind": "computer", "storageRoot": "/srv/zakura", "capabilities": map[string]any{}, "hostInfo": map[string]any{}}, nil + case "sandbox.policy": + return map[string]any{"sandbox": map[string]any{"enabled": true}}, nil + case "docker.list": + return []map[string]any{{"dockerId": "workspace", "labels": map[string]string{"zakura.purpose": "workspace"}}}, nil + case "host.exec", "host.exec.start", "docker.exec": + expectedMethod := "host.exec" + if tc.workspace != "host" { + expectedMethod = "docker.exec" + } else if tc.requested == "sandbox" { + expectedMethod = "host.exec.start" + } + if method != expectedMethod { + t.Errorf("expected %s, got %s", expectedMethod, method) + } + result := map[string]any{"id": "job", "running": false, "exitCode": 0, "stdout": "ok", "stderr": ""} + if tc.reported != "" { + result["executionMode"] = tc.reported + result["isolated"] = tc.reportedIsolation + } + return result, nil + case "host.exec.kill": + return map[string]any{"running": false, "executionMode": "sandbox", "isolated": true}, nil + default: + t.Errorf("unexpected RPC: %s", method) + return nil, nil + } + }) + result, err := h.runtimeExecWithMode(context.Background(), "tenant", agent.ID, tc.requested, "true") + if tc.wantError { + if err == nil { + t.Fatalf("unconfirmed sandbox accepted: %#v", result) + } + return + } + if err != nil { + t.Fatal(err) + } + if result["executionMode"] != tc.expected || result["isolated"] != tc.isolated || result["restrictedPolicyApplied"] != tc.restricted { + t.Fatalf("incorrect classification: %#v", result) + } + args := map[string]any{"command": "true"} + if tc.requested != "" { + args["execution_mode"] = tc.requested + } + raw, err := h.runShellExec(context.Background(), "tenant", agent.ID, args) + if err != nil { + t.Fatal(err) + } + var toolResult map[string]any + if err := json.Unmarshal(raw, &toolResult); err != nil { + t.Fatal(err) + } + if toolResult["executionMode"] != tc.expected || toolResult["isolated"] != tc.isolated || toolResult["restrictedPolicyApplied"] != tc.restricted { + t.Fatalf("tool output lost execution classification: %s", raw) + } + }) + } +} diff --git a/apps/server/internal/runtime/tool_session_state.go b/apps/server/internal/runtime/tool_session_state.go index ea5f0551..dde6d961 100644 --- a/apps/server/internal/runtime/tool_session_state.go +++ b/apps/server/internal/runtime/tool_session_state.go @@ -25,6 +25,11 @@ func isBuiltinToolName(name string) bool { } func (h *handler) dispatchAgentTool(ctx context.Context, tenant, agent, session, toolCallID, name string, args json.RawMessage) (json.RawMessage, error) { + var policyErr error + args, policyErr = h.sandboxToolPolicy(ctx, tenant, agent, name, args) + if policyErr != nil { + return nil, policyErr + } if toolDisabled(ctx, name) { return nil, fmt.Errorf("tool %s is disabled for this run", name) } diff --git a/docs/sandbox.md b/docs/sandbox.md new file mode 100644 index 00000000..3a23ba20 --- /dev/null +++ b/docs/sandbox.md @@ -0,0 +1,78 @@ +# Restricted agent execution mode + +Zakura already supports long-lived Docker workspaces with container isolation, writable project files, terminals, browser tools, and ACP. This feature adds an optional, more restrictive per-command Docker execution policy and an agent tool allowlist. It does not replace the existing workspace container or claim that previous container execution lacked isolation. Ordinary host execution remains available without container isolation; host working-directory path validation alone is not a sandbox. + +## Selecting the mode + +For an agent-wide tool boundary, set the agent's persisted `config.executionMode` to `"sandbox"` through the existing authenticated agent configuration PATCH API, and configure the runner image below. The server rereads the tenant-scoped agent configuration on every tool invocation. It forces `shell_exec` into sandbox mode even if model arguments request host mode; only `ask_user` is otherwise permitted. Arbitrary MCP tools, filesystem tools, browser tools, code mode, delegation, and service tools are denied. The tool catalog is limited to `shell_exec` and `ask_user`; ACP startup/reuse is refused. Invalid configuration modes fail closed. Omitting the agent configuration keeps legacy host behavior. + +Enable sandbox mode before starting agent runs. Changing the policy does not retroactively stop existing host jobs or ACP processes; stop those separately before relying on the new boundary. + +This is separate from choosing sandbox mode on an individual shell command. Manual user filesystem/admin APIs are outside the agent tool boundary, and configured model providers still receive prompts and conversation content. Sandbox mode is not a promise that no data leaves the application. + + +- `shell_exec` accepts `execution_mode: "sandbox"` or `"host"`. The server checks the runner's `sandbox.policy` capability before requesting sandbox execution, so an older runner cannot silently ignore the mode. +- Agent RPC `host.exec`, `host.exec.start`, `host.exec.get`, and `host.exec.kill` accept `executionMode: "sandbox"`. Supply the same valid `spaceId` and execution mode when polling or cancelling a job. +- Setting `ZAKURA_SANDBOX_ENABLED=true` on the agent enforces sandbox mode when its handler is created. A request for host mode cannot disable operator enforcement. Without enforcement, omitted execution mode preserves host compatibility. +- Shell results distinguish `executionMode: host` (no container isolation), `workspace-container` (existing Docker workspace isolation), and `sandbox` (confirmed restricted policy). `restrictedPolicyApplied` is true only for the last mode. `isolated` records container isolation, not a claim of virtual-machine isolation or containment of the entire application. Sandbox execution adds `timedOut`, `cancelled`, and `truncated`; background jobs also report operational errors. +- `sandbox.policy` describes the configured policy, default mode, and enforcement. Image configuration is not evidence that the Docker daemon or image is usable. +- The restricted execution mode currently supports host workspaces only; it mounts that dedicated workspace into a fresh restricted container. Existing container workspaces retain their normal behavior and cannot opt into this mode yet. Unsupported workspace backends, PTY sessions, and Docker execution/management transports are refused rather than silently downgraded. + +## Operator prerequisites + +Use a trusted, local Linux Docker daemon with a current engine supporting `bind-recursive=disabled`. The daemon must share the agent's filesystem. The agent does not forward `DOCKER_HOST`, context overrides, credentials, proxies, or application environment to the Docker client. + +Provision an approved local image in advance and set `ZAKURA_SANDBOX_IMAGE` on the agent. Pin a reviewed image digest for reproducibility. Execution uses `--pull=never`; it does not pull images. The image must include the executable and supporting files the command needs, contain no application secrets, and declare no Docker `VOLUME` paths. The runtime inspects and refuses images declaring volumes. Image-baked environment and image content are trusted operator inputs, so a clean parent environment does not sanitize a secret-bearing image. + +Workspace files must be readable by UID/GID 65534. Workspace roots must use real directories; sockets, devices and FIFOs are rejected. The operator must prevent concurrent host-side mutations while execution runs. Path and file-type checks have time-of-check/time-of-use limitations; they are not an independent isolation boundary. Other agent file or management APIs remain separately authorized control-plane operations: this feature is a command-execution boundary, not a replacement for control-plane authentication and authorization. + +## Fixed restrictions + +| Resource | Policy | +| --- | --- | +| Network | `none`; no external network interface | +| User | UID/GID 65534, all capabilities dropped, no new privileges | +| Filesystem | Read-only image and read-only `/workspace` bind; nested mounts excluded | +| Writable scratch | Ephemeral `/tmp`, 64 MiB, noexec/nosuid/nodev | +| Memory / swap | 512 MiB memory, no additional swap | +| CPU / processes | 1 CPU, 64 PIDs | +| Open files / core | 256 open files; core dumps disabled | +| Execution timeout | Default and maximum 300,000 ms | +| Captured output | 256 KiB per stdout/stderr stream, then discarded with truncation flag | +| Input | 64 KiB stdin, 64 KiB combined command strings, 256 command arguments | +| Concurrency | 4 active executions; 128 retained/background job records with completed-result eviction | +| Environment | No caller-supplied environment; fixed HOME/TMPDIR/PATH | + +Read-only workspace means build/install workflows must use `/tmp` and cannot persist output back into the project through this API. Runtime resource flags depend on actual Docker/kernel enforcement; argument-policy unit tests alone do not establish enforcement. + +## Failure and lifecycle behavior + +The server uses workspace-scoped background start/get/kill RPCs for sandbox commands. Cancelling an agent run sends a scoped kill and waits up to five seconds for a terminal acknowledgment. Cleanup or terminal execution errors are surfaced, and an unconfirmed cancellation is reported as uncertain rather than successful. + +Starting a job uses a separate ten-second deadline so that an agent-run cancellation does not immediately discard the job identity needed for cleanup. If the start response is lost or cannot be confirmed, the server does not retry: the outcome is unknown, and any started job remains bounded by its hard execution timeout. Operator follow-up may still be needed when connectivity or daemon failures prevent cleanup confirmation. + +Missing configuration, unavailable Docker, image inspection failure, invalid inputs, unsupported isolation options, or container creation failures return errors without executing the command on the host. Container creation and execution are separate. Cancellation during creation may leave a stopped container and reports that uncertainty; it must not start an untracked command. + +Timeouts and cancellation terminate the client, then force-remove the named container using a separate bounded cleanup context. If removal fails, the runtime checks container absence. An inability to verify cleanup is returned as an operational error requiring operator attention. This is best-effort cleanup under daemon failures, not a guarantee against daemon outages. Do not treat a cancelled job as fully cleaned up when its error reports otherwise. + +## Validation + +From `apps/agent`: + +```sh +go test -race ./... +go vet ./... +``` + +`validation_test.go` covers restrictive Docker arguments, request bounds, missing-backend refusal, output bounds under concurrent access, workspace mapping, job quotas, workspace-scoped access, clean Docker client environment, and synthetic timeout/cancellation. The synthetic Docker fixture only tests client orchestration; it does not launch containers or prove OS isolation. RPC tests cover policy enforcement, unsupported transports, missing workspace identity, and fail-closed configuration. + +A separate integration test uses a **real Docker daemon** and benign commands. On a dedicated Docker-capable test host, pre-pull an approved Alpine-compatible image, set its pinned reference, and run: + +```sh +ZAKURA_SANDBOX_INTEGRATION=1 ZAKURA_SANDBOX_IMAGE='' \ + go test ./internal/sandbox -run '^TestSandboxRealDockerIntegration$' -count=1 -v +``` + +The image needs `sh`, `cat`, `id`, `pwd`, `touch`, `ls`, `head`, and `sleep`. The test checks workspace readability, non-root user, denied workspace writes, ephemeral scratch, clean parent environment, loopback-only networking, capped output, timeout/cancellation and absence of newly remaining sandbox containers. Run without concurrent sandbox workloads so cleanup assertions are meaningful. Opt-in with missing Docker/image fails rather than skips. Without opt-in it explicitly skips. + +The development executor had no Docker CLI or daemon access, so real Docker isolation was **not locally verified**. The dedicated CI integration stage must pass on the exact change before claiming real-backend validation. These tests are defensive behavior checks; they do not constitute a sandbox-escape assessment or production certification. From 20f03ed5ab15e89328e6175110c0644ce6293e53 Mon Sep 17 00:00:00 2001 From: momo-mnsjtxy <122842784+momo-mnsjtxy@users.noreply.github.com> Date: Wed, 7 Oct 2026 01:38:24 -0700 Subject: [PATCH 2/3] Fix restricted output persistence, lifecycle budgets and atomic config saves --- .github/workflows/sandbox-validation.yml | 35 +++- .../platform/admin/agent_defaults_test.go | 74 ++++++++ apps/server/internal/platform/admin/routes.go | 48 ++--- .../platform/agentconfig/postgres_test.go | 53 ++++++ .../internal/platform/agentconfig/update.go | 64 +++++++ .../platform/agentconfig/update_test.go | 146 +++++++++++++++ apps/server/internal/runtime/acp.go | 51 +++++- apps/server/internal/runtime/agent_config.go | 52 ++++++ .../internal/runtime/agent_config_test.go | 109 +++++++++++ .../internal/runtime/agent_tool_runners.go | 19 +- apps/server/internal/runtime/http.go | 34 ++-- apps/server/internal/runtime/instances.go | 37 ++-- apps/server/internal/runtime/runner_hub.go | 81 ++++++--- .../runtime/runner_write_deadline_test.go | 50 ++++++ apps/server/internal/runtime/sandbox_exec.go | 39 ++-- .../runtime/sandbox_integration_test.go | 169 ++++++++++++++++++ .../internal/runtime/sandbox_output_test.go | 162 +++++++++++++++++ apps/server/internal/runtime/tool_output.go | 9 + docs/sandbox.md | 16 +- 19 files changed, 1139 insertions(+), 109 deletions(-) create mode 100644 apps/server/internal/platform/admin/agent_defaults_test.go create mode 100644 apps/server/internal/platform/agentconfig/postgres_test.go create mode 100644 apps/server/internal/platform/agentconfig/update.go create mode 100644 apps/server/internal/platform/agentconfig/update_test.go create mode 100644 apps/server/internal/runtime/agent_config.go create mode 100644 apps/server/internal/runtime/agent_config_test.go create mode 100644 apps/server/internal/runtime/runner_write_deadline_test.go create mode 100644 apps/server/internal/runtime/sandbox_integration_test.go create mode 100644 apps/server/internal/runtime/sandbox_output_test.go diff --git a/.github/workflows/sandbox-validation.yml b/.github/workflows/sandbox-validation.yml index 38330162..0ca5f2c2 100644 --- a/.github/workflows/sandbox-validation.yml +++ b/.github/workflows/sandbox-validation.yml @@ -20,6 +20,20 @@ jobs: sandbox: runs-on: ubuntu-latest timeout-minutes: 20 + services: + postgres: + image: postgres:17 + env: + POSTGRES_USER: zakura_test + POSTGRES_PASSWORD: zakura_test + POSTGRES_DB: zakura_test + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U zakura_test" + --health-interval 5s + --health-timeout 5s + --health-retries 10 steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 @@ -35,13 +49,30 @@ jobs: go vet ./... - name: Prepare sandbox test image run: | - docker pull alpine:3.23 - echo "ZAKURA_SANDBOX_IMAGE=$(docker image inspect alpine:3.23 --format '{{index .RepoDigests 0}}')" >> "$GITHUB_ENV" + docker build -t zakura-sandbox-ci:local - <<'DOCKERFILE' + FROM alpine:3.23 + RUN apk add --no-cache bash + DOCKERFILE + echo "ZAKURA_SANDBOX_IMAGE=$(docker image inspect zakura-sandbox-ci:local --format '{{.Id}}')" >> "$GITHUB_ENV" - name: Real container isolation and lifecycle checks working-directory: apps/agent env: ZAKURA_SANDBOX_INTEGRATION: '1' run: go test -count=1 -v ./internal/sandbox + - name: Build runner for server integration + working-directory: apps/agent + run: go build -o "$RUNNER_TEMP/zakura-sandbox-agent" ./cmd/zakura-agent + - name: Real server to runner sandbox integration + working-directory: apps/server + env: + ZAKURA_SERVER_SANDBOX_INTEGRATION: '1' + CGO_ENABLED: '1' + run: ZAKURA_SANDBOX_AGENT_BINARY="$RUNNER_TEMP/zakura-sandbox-agent" go test -count=1 -v ./internal/runtime -run '^TestSandboxServerRunnerDockerIntegration$' -timeout 3m + - name: PostgreSQL config compare-and-swap integration + working-directory: apps/server + env: + ZAKURA_TEST_POSTGRES_DSN: postgres://zakura_test:zakura_test@127.0.0.1:5432/zakura_test?sslmode=disable + run: go test -count=1 -v ./internal/platform/agentconfig -run Postgres - name: Server regression tests working-directory: apps/server env: diff --git a/apps/server/internal/platform/admin/agent_defaults_test.go b/apps/server/internal/platform/admin/agent_defaults_test.go new file mode 100644 index 00000000..f6defb0f --- /dev/null +++ b/apps/server/internal/platform/admin/agent_defaults_test.go @@ -0,0 +1,74 @@ +package admin + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/go-chi/chi/v5" + "gorm.io/gorm" +) + +func TestApplyAgentDefaultsPreservesConcurrentExecutionPolicy(t *testing.T) { + a := infraTestRoutes(t) + for _, statement := range []string{ + `INSERT INTO users(id,email,created_at,updated_at) VALUES('user','user@example.test','now','now')`, + `INSERT INTO tenants(id,slug,name,created_at,updated_at) VALUES('tenant','tenant','Tenant','now','now')`, + `INSERT INTO tenant_memberships(id,tenant_id,user_id,role,created_at,updated_at) VALUES('membership','tenant','user','owner','now','now')`, + `INSERT INTO spaces(id,tenant_id,name,slug,created_at,updated_at) VALUES('space','tenant','Space','space','now','now')`, + `INSERT INTO agents(id,tenant_id,space_id,name,slug,config_json,created_at,updated_at) VALUES('agent','tenant','space','Agent','agent','{"executionMode":"host"}','now','now')`, + } { + if _, err := a.d.DB.Exec(statement); err != nil { + t.Fatal(err) + } + } + var raced atomic.Bool + callback := "test:default-policy-race" + if err := a.d.Gorm.Callback().Query().After("gorm:query").Register(callback, func(tx *gorm.DB) { + if tx.Statement.Table != "agents" || len(tx.Statement.Selects) != 1 || tx.Statement.Selects[0] != "config_json" || raced.Swap(true) { + return + } + _, err := a.d.DB.Exec(`UPDATE agents SET config_json=? WHERE id='agent'`, `{"executionMode":"sandbox","cloud":{"model":"newer"},"providers":{"webSearch":{"defaultEngine":"auto"}}}`) + if err != nil { + t.Errorf("concurrent save: %v", err) + } + }); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = a.d.Gorm.Callback().Query().Remove(callback) }) + request := httptest.NewRequest(http.MethodPost, "/api/admin/users/user/agent-defaults/apply", nil) + route := chi.NewRouteContext() + route.URLParams.Add("id", "user") + request = request.WithContext(context.WithValue(request.Context(), chi.RouteCtxKey, route)) + response := httptest.NewRecorder() + a.applyAgentDefaults(response, request) + if response.Code != http.StatusOK || !raced.Load() { + t.Fatalf("defaults failed: %d %s raced=%v", response.Code, response.Body.String(), raced.Load()) + } + var stored string + if err := a.d.DB.QueryRow(`SELECT config_json FROM agents WHERE id='agent'`).Scan(&stored); err != nil { + t.Fatal(err) + } + var config map[string]any + if err := json.Unmarshal([]byte(stored), &config); err != nil { + t.Fatal(err) + } + if config["executionMode"] != "sandbox" || config["cloud"].(map[string]any)["model"] != "newer" { + t.Fatalf("unrelated settings overwritten: %s", stored) + } + providers := config["providers"].(map[string]any) + search := providers["webSearch"].(map[string]any) + if search["enabled"] != true || search["defaultEngine"] != "auto" || providers["webFetch"].(map[string]any)["enabled"] != true { + t.Fatalf("defaults changed provider settings: %s", stored) + } + response = httptest.NewRecorder() + a.applyAgentDefaults(response, request) + var result map[string]any + _ = json.Unmarshal(response.Body.Bytes(), &result) + if response.Code != http.StatusOK || result["updated"] != float64(0) { + t.Fatalf("repeated defaults should be a no-op: %d %s", response.Code, response.Body.String()) + } +} diff --git a/apps/server/internal/platform/admin/routes.go b/apps/server/internal/platform/admin/routes.go index defcf79f..0e1f028e 100644 --- a/apps/server/internal/platform/admin/routes.go +++ b/apps/server/internal/platform/admin/routes.go @@ -15,6 +15,7 @@ import ( "golang.org/x/crypto/bcrypt" "gorm.io/gorm/clause" + "github.com/Moonrend/Zakura/apps/server/internal/platform/agentconfig" "github.com/Moonrend/Zakura/apps/server/internal/platform/appdeps" "github.com/Moonrend/Zakura/apps/server/internal/platform/db/models" "github.com/Moonrend/Zakura/apps/server/internal/platform/httpx" @@ -988,35 +989,38 @@ func (a *routes) applyAgentDefaults(w http.ResponseWriter, r *http.Request) { updated := 0 for _, tenantID := range tenants { var agents []models.Agent - if e := a.d.Gorm.WithContext(ctx).Select("id,config_json").Where("tenant_id=?", tenantID).Find(&agents).Error; e != nil { + if e := a.d.Gorm.WithContext(ctx).Select("id").Where("tenant_id=?", tenantID).Find(&agents).Error; e != nil { httpx.Error(w, 400, e.Error()) return } for _, agent := range agents { - bag := map[string]any{} - _ = json.Unmarshal([]byte(agent.ConfigJSON), &bag) - providers, _ := bag["providers"].(map[string]any) - if providers == nil { - providers = map[string]any{} - } - for _, key := range []string{"webSearch", "webFetch"} { - value, _ := providers[key].(map[string]any) - if value == nil { - value = map[string]any{} + changed, err := agentconfig.Update(ctx, a.d.Gorm, tenantID, deref(agent.ID), a.now(), nil, func(bag map[string]any) error { + providers, _ := bag["providers"].(map[string]any) + if providers == nil { + providers = map[string]any{} } - value["enabled"] = true - providers[key] = value - } - bag["providers"] = providers - next, _ := json.Marshal(bag) - if string(next) == agent.ConfigJSON { - continue - } - if e := a.d.Gorm.WithContext(ctx).Model(&models.Agent{}).Where("id=? AND tenant_id=?", deref(agent.ID), tenantID).Updates(map[string]any{"config_json": string(next), "updated_at": a.now()}).Error; e != nil { - httpx.Error(w, 400, e.Error()) + for _, key := range []string{"webSearch", "webFetch"} { + value, _ := providers[key].(map[string]any) + if value == nil { + value = map[string]any{} + } + value["enabled"] = true + providers[key] = value + } + bag["providers"] = providers + return nil + }) + if err != nil { + status := http.StatusBadRequest + if errors.Is(err, agentconfig.ErrConflict) { + status = http.StatusConflict + } + httpx.Error(w, status, err.Error()) return } - updated++ + if changed { + updated++ + } } } httpx.JSON(w, 200, map[string]any{"updated": updated, "tenants": len(tenants)}) diff --git a/apps/server/internal/platform/agentconfig/postgres_test.go b/apps/server/internal/platform/agentconfig/postgres_test.go new file mode 100644 index 00000000..1cb4aeb6 --- /dev/null +++ b/apps/server/internal/platform/agentconfig/postgres_test.go @@ -0,0 +1,53 @@ +package agentconfig + +import ( + "context" + "database/sql" + "os" + "testing" + "time" + + _ "github.com/lib/pq" + "gorm.io/driver/postgres" + "gorm.io/gorm" +) + +func TestPostgresUpdateRetriesAgainstFreshConfig(t *testing.T) { + dsn := os.Getenv("ZAKURA_TEST_POSTGRES_DSN") + if dsn == "" { + t.Skip("set ZAKURA_TEST_POSTGRES_DSN to a disposable PostgreSQL test service") + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + pool, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatal(err) + } + defer pool.Close() + pool.SetMaxOpenConns(1) + pool.SetMaxIdleConns(1) + // Pin the session: all reads/writes must target its temporary table, even if + // a connection fails. Never reconnect and fall through to a persistent table. + conn, err := pool.Conn(ctx) + if err != nil { + t.Fatalf("PostgreSQL test service unavailable: %v", err) + } + defer conn.Close() + if err := conn.PingContext(ctx); err != nil { + t.Fatalf("PostgreSQL test service unavailable: %v", err) + } + if _, err := conn.ExecContext(ctx, `SET search_path TO pg_temp`); err != nil { + t.Fatal(err) + } + if _, err := conn.ExecContext(ctx, `CREATE TEMPORARY TABLE agents (id TEXT PRIMARY KEY, tenant_id TEXT NOT NULL, config_json TEXT NOT NULL, updated_at TEXT, enable_memory BOOLEAN)`); err != nil { + t.Fatal(err) + } + if _, err := conn.ExecContext(ctx, `INSERT INTO agents (id,tenant_id,config_json,enable_memory) VALUES ('agent','tenant',$1,true)`, `{"executionMode":"host","cloud":{"model":"before"},"large":9007199254740993}`); err != nil { + t.Fatal(err) + } + db, err := gorm.Open(postgres.New(postgres.Config{Conn: conn, PreferSimpleProtocol: true}), &gorm.Config{}) + if err != nil { + t.Fatal(err) + } + checkUpdateRetriesAgainstFreshConfig(t, db) +} diff --git a/apps/server/internal/platform/agentconfig/update.go b/apps/server/internal/platform/agentconfig/update.go new file mode 100644 index 00000000..737178fd --- /dev/null +++ b/apps/server/internal/platform/agentconfig/update.go @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +package agentconfig + +import ( + "context" + "encoding/json" + "errors" + "strings" + + "gorm.io/gorm" +) + +var ErrConflict = errors.New("agent configuration changed concurrently") + +const maxUpdateAttempts = 16 + +// Update applies a field-scoped mutation to the latest configuration. The +// comparison in the write protects against other processes and full-config +// replacements, on both SQLite and PostgreSQL. mutate may run more than once +// and must not perform external side effects or reuse stale configuration. +// columns contains any accompanying, already validated non-config updates. +func Update(ctx context.Context, db *gorm.DB, tenant, id, updatedAt string, columns map[string]any, mutate func(map[string]any) error) (bool, error) { + for attempt := 0; attempt < maxUpdateAttempts; attempt++ { + if err := ctx.Err(); err != nil { + return false, err + } + var row struct { + Config string `gorm:"column:config_json"` + } + if err := db.WithContext(ctx).Table("agents").Select("config_json").Where("tenant_id=? AND id=?", tenant, id).Take(&row).Error; err != nil { + return false, err + } + var config map[string]any + decoder := json.NewDecoder(strings.NewReader(row.Config)) + decoder.UseNumber() + if !json.Valid([]byte(row.Config)) || decoder.Decode(&config) != nil || config == nil { + return false, errors.New("invalid agent configuration") + } + if err := mutate(config); err != nil { + return false, err + } + next, err := json.Marshal(config) + if err != nil { + return false, err + } + if string(next) == row.Config && len(columns) == 0 { + return false, nil + } + updates := make(map[string]any, len(columns)+2) + for key, value := range columns { + updates[key] = value + } + updates["config_json"] = string(next) + updates["updated_at"] = updatedAt + result := db.WithContext(ctx).Table("agents").Where("tenant_id=? AND id=? AND config_json=?", tenant, id, row.Config).Updates(updates) + if result.Error != nil { + return false, result.Error + } + if result.RowsAffected != 0 { + return true, nil + } + } + return false, ErrConflict +} diff --git a/apps/server/internal/platform/agentconfig/update_test.go b/apps/server/internal/platform/agentconfig/update_test.go new file mode 100644 index 00000000..3f974f31 --- /dev/null +++ b/apps/server/internal/platform/agentconfig/update_test.go @@ -0,0 +1,146 @@ +package agentconfig + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "path/filepath" + "testing" + "time" + + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +func testConfigDB(t *testing.T, initial string) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open(filepath.Join(t.TempDir(), "config.db")), &gorm.Config{}) + if err != nil { + t.Fatal(err) + } + sqlDB, err := db.DB() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = sqlDB.Close() }) + if err := db.Exec(`CREATE TABLE agents (id TEXT PRIMARY KEY, tenant_id TEXT NOT NULL, config_json TEXT NOT NULL, updated_at TEXT, enable_memory BOOLEAN)`).Error; err != nil { + t.Fatal(err) + } + if err := db.Exec(`INSERT INTO agents (id,tenant_id,config_json,enable_memory) VALUES ('agent','tenant',?,true)`, initial).Error; err != nil { + t.Fatal(err) + } + return db +} + +func TestUpdateRetriesAgainstFreshConfig(t *testing.T) { + db := testConfigDB(t, `{"executionMode":"host","cloud":{"model":"before"},"large":9007199254740993}`) + checkUpdateRetriesAgainstFreshConfig(t, db) +} + +func checkUpdateRetriesAgainstFreshConfig(t *testing.T, db *gorm.DB) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + db = db.WithContext(ctx) + calls := 0 + changed, err := Update(ctx, db, "tenant", "agent", "now", map[string]any{"enable_memory": false}, func(config map[string]any) error { + calls++ + if calls == 1 { + if err := db.Exec(`UPDATE agents SET config_json=? WHERE id='agent'`, `{"executionMode":"sandbox","cloud":{"model":"newer","keep":true},"large":9007199254740993}`).Error; err != nil { + return err + } + } + config["cloud"].(map[string]any)["systemPrompt"] = "hello" + return nil + }) + if err != nil || !changed || calls != 2 { + t.Fatalf("changed=%v calls=%d err=%v", changed, calls, err) + } + var row struct { + Config string `gorm:"column:config_json"` + Memory bool `gorm:"column:enable_memory"` + } + if err := db.Table("agents").Take(&row).Error; err != nil { + t.Fatal(err) + } + var config map[string]json.RawMessage + if err := json.Unmarshal([]byte(row.Config), &config); err != nil { + t.Fatal(err) + } + if string(config["executionMode"]) != `"sandbox"` || string(config["large"]) != "9007199254740993" || row.Memory { + t.Fatalf("unrelated fields lost: %+v", row) + } + var cloud map[string]any + _ = json.Unmarshal(config["cloud"], &cloud) + if cloud["model"] != "newer" || cloud["keep"] != true || cloud["systemPrompt"] != "hello" { + t.Fatalf("stale cloud fields: %#v", cloud) + } + // Intentional full replacement remains supported in either policy direction. + for _, mode := range []string{"host", "sandbox"} { + explicit, _ := json.Marshal(map[string]any{"executionMode": mode}) + if err := db.Exec(`UPDATE agents SET config_json=? WHERE tenant_id='tenant' AND id='agent'`, string(explicit)).Error; err != nil { + t.Fatal(err) + } + if _, err := Update(ctx, db, "tenant", "agent", "later", nil, func(config map[string]any) error { + config["cloud"] = map[string]any{"model": "requested"} + return nil + }); err != nil { + t.Fatal(err) + } + if err := db.Table("agents").Take(&row).Error; err != nil { + t.Fatal(err) + } + var saved map[string]any + if err := json.Unmarshal([]byte(row.Config), &saved); err != nil { + t.Fatal(err) + } + if saved["executionMode"] != mode || saved["cloud"].(map[string]any)["model"] != "requested" { + t.Fatalf("explicit policy change lost: %s", row.Config) + } + } +} + +func TestUpdateBoundsContentionAndPreservesLatestConfig(t *testing.T) { + db := testConfigDB(t, `{"executionMode":"sandbox"}`) + calls := 0 + changed, err := Update(context.Background(), db, "tenant", "agent", "now", nil, func(config map[string]any) error { + calls++ + config["cloud"] = map[string]any{"model": "requested"} + return db.Exec(`UPDATE agents SET config_json=? WHERE id='agent'`, fmt.Sprintf(`{"executionMode":"sandbox","revision":%d}`, calls)).Error + }) + if changed || !errors.Is(err, ErrConflict) || calls != maxUpdateAttempts { + t.Fatalf("changed=%v calls=%d err=%v", changed, calls, err) + } + var stored string + if err := db.Raw(`SELECT config_json FROM agents WHERE id='agent'`).Scan(&stored).Error; err != nil { + t.Fatal(err) + } + if stored != fmt.Sprintf(`{"executionMode":"sandbox","revision":%d}`, calls) { + t.Fatalf("conflict overwrote latest config: %s", stored) + } +} + +func TestUpdateRejectsInvalidConfigAndWrongTenant(t *testing.T) { + for _, initial := range []string{`null`, `[]`, `"text"`, `{"executionMode":`} { + t.Run(initial, func(t *testing.T) { + db := testConfigDB(t, initial) + called := false + _, err := Update(context.Background(), db, "tenant", "agent", "now", nil, func(map[string]any) error { + called = true + return nil + }) + if err == nil || called { + t.Fatalf("invalid config reached mutation: called=%v err=%v", called, err) + } + }) + } + db := testConfigDB(t, `{"executionMode":"sandbox"}`) + _, err := Update(context.Background(), db, "other-tenant", "agent", "now", nil, func(map[string]any) error { + t.Fatal("wrong tenant reached mutation") + return nil + }) + if !errors.Is(err, gorm.ErrRecordNotFound) { + t.Fatalf("wrong-tenant result: %v", err) + } +} diff --git a/apps/server/internal/runtime/acp.go b/apps/server/internal/runtime/acp.go index 1a6da6be..e977d75a 100644 --- a/apps/server/internal/runtime/acp.go +++ b/apps/server/internal/runtime/acp.go @@ -69,8 +69,7 @@ func (h *handler) agentConfig(ctx context.Context, tenant, agent string) (Agent, return a, cfg, nil } func (h *handler) saveAgentConfig(ctx context.Context, tenant string, a Agent, cfg map[string]any) error { - _, e := h.store.UpdateAgent(ctx, tenant, a.ID, map[string]any{"config": cfg}) - return e + return h.store.saveAgentConfigChanges(ctx, tenant, a, cfg) } func acpMap(cfg map[string]any) map[string]any { v, ok := cfg["acp"].(map[string]any) @@ -116,6 +115,11 @@ func (h *handler) runtimeExec(ctx context.Context, tenant, agent, command string } func (h *handler) runtimeExecWithMode(ctx context.Context, tenant, agent, mode, command string, args ...string) (map[string]any, error) { + return h.runtimeExecWithModeTimeout(ctx, tenant, agent, mode, 0, command, args...) +} + +// Execution budget is independent of RPC observation and container cleanup. +func (h *handler) runtimeExecWithModeTimeout(ctx context.Context, tenant, agent, mode string, executionTimeoutMS int64, command string, args ...string) (map[string]any, error) { if mode != "" && mode != "host" && mode != "sandbox" { return nil, errors.New("execution_mode must be host or sandbox") } @@ -137,25 +141,50 @@ func (h *handler) runtimeExecWithMode(ctx context.Context, tenant, agent, mode, if err != nil { return nil, err } - if mode == "sandbox" { + if mode == "sandbox" || (workspaceKind == "host" && executionTimeoutMS > 0) { + requested := mode == "sandbox" var policy struct { - Sandbox struct { + Enforced bool `json:"enforced"` + Sandbox struct { Enabled bool `json:"enabled"` } `json:"sandbox"` } - if err := session.call(ctx, "sandbox.policy", map[string]any{}, &policy); err != nil || !policy.Sandbox.Enabled { - return nil, errors.New("runner does not support sandbox execution; upgrade or configure the runner") + policyErr := session.call(ctx, "sandbox.policy", map[string]any{}, &policy) + if policyErr != nil { + // Pre-policy runners explicitly reject the unknown method. A timeout, + // disconnect or other error is not evidence that enforcement is off. + legacyUnsupported := strings.Contains(policyErr.Error(), "未知方法: sandbox.policy") || strings.Contains(policyErr.Error(), "unknown method: sandbox.policy") + if requested || !legacyUnsupported { + return nil, errors.New("runner execution policy could not be confirmed") + } + } else if requested || policy.Enforced { + if !policy.Sandbox.Enabled { + return nil, errors.New("runner does not support sandbox execution; upgrade or configure the runner") + } + mode = "sandbox" } } argv := append([]string{command}, args...) - timeoutMS := int64(30_000) + timeoutMS := executionTimeoutMS + if timeoutMS <= 0 { + timeoutMS = 30_000 + } if deadline, ok := ctx.Deadline(); ok { if remaining := time.Until(deadline).Milliseconds(); remaining > 0 { - timeoutMS = remaining + if executionTimeoutMS <= 0 || remaining < timeoutMS { + timeoutMS = remaining + } } else { return nil, context.DeadlineExceeded } } + if mode != "sandbox" && executionTimeoutMS > 0 { + // Preserve legacy RPC deadline behavior; only verified restricted jobs + // receive additional observation time for bounded terminal cleanup. + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, time.Duration(timeoutMS)*time.Millisecond) + defer cancel() + } result := map[string]any{} if workspaceKind == "host" { params := map[string]any{"spaceId": spaceID, "command": argv, "workingDir": "/workspace", "timeoutMs": timeoutMS, "executionMode": mode} @@ -182,9 +211,10 @@ func (h *handler) runtimeExecWithMode(ctx context.Context, tenant, agent, mode, err = session.call(ctx, "docker.exec", map[string]any{"id": dockerID, "command": argv, "workingDir": "/workspace"}, &result) } } - if err != nil { + if err != nil && result == nil { return nil, err } + executionErr := err // Describe the transport that actually ran the command. A normal workspace // container provides container isolation but does not apply the restricted // sandbox policy (network, read-only mount, resource limits, and cleanup). @@ -207,6 +237,9 @@ func (h *handler) runtimeExecWithMode(ctx context.Context, tenant, agent, mode, result["isolated"] = false result["restrictedPolicyApplied"] = false } + if executionErr != nil { + return result, executionErr + } if code, ok := result["exitCode"].(float64); ok && code != 0 { return result, fmt.Errorf("command exited %d: %v", int(code), result["stderr"]) } diff --git a/apps/server/internal/runtime/agent_config.go b/apps/server/internal/runtime/agent_config.go new file mode 100644 index 00000000..91680c8a --- /dev/null +++ b/apps/server/internal/runtime/agent_config.go @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +package runtime + +import ( + "context" + "encoding/json" + "errors" + "reflect" + + "github.com/Moonrend/Zakura/apps/server/internal/platform/agentconfig" + "gorm.io/gorm" +) + +// updateAgentConfig is for internal field-scoped settings saves. UpdateAgent +// retains the public API's deliberate whole-config replacement semantics. +func (s *Store) updateAgentConfig(ctx context.Context, tenant, id string, columns map[string]any, mutate func(map[string]any) error) (Agent, error) { + _, err := agentconfig.Update(ctx, s.deps.Gorm, tenant, id, runtimeTimeString(s.now()), columns, mutate) + if errors.Is(err, gorm.ErrRecordNotFound) { + err = ErrNotFound + } else if errors.Is(err, agentconfig.ErrConflict) { + err = ErrConflict + } + if err != nil { + return Agent{}, err + } + return s.GetAgent(ctx, tenant, id) +} + +// saveAgentConfigChanges carries only the top-level fields changed relative to +// the caller's original snapshot. In particular, an unchanged executionMode +// must never overwrite a newer policy while saving ACP state. +func (s *Store) saveAgentConfigChanges(ctx context.Context, tenant string, original Agent, next map[string]any) error { + var previous map[string]any + if json.Unmarshal(original.Config, &previous) != nil || previous == nil || next == nil { + return errors.New("invalid agent configuration") + } + _, err := s.updateAgentConfig(ctx, tenant, original.ID, nil, func(current map[string]any) error { + for key := range previous { + if _, ok := next[key]; !ok { + delete(current, key) + } + } + for key, value := range next { + old, exists := previous[key] + if !exists || !reflect.DeepEqual(old, value) { + current[key] = value + } + } + return nil + }) + return err +} diff --git a/apps/server/internal/runtime/agent_config_test.go b/apps/server/internal/runtime/agent_config_test.go new file mode 100644 index 00000000..3496f876 --- /dev/null +++ b/apps/server/internal/runtime/agent_config_test.go @@ -0,0 +1,109 @@ +package runtime + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/Moonrend/Zakura/apps/server/internal/platform/httpx" + "github.com/go-chi/chi/v5" + "gorm.io/gorm" +) + +func TestStaleACPSavePreservesCurrentExecutionPolicy(t *testing.T) { + for _, initial := range []string{`{}`, `{"executionMode":"host"}`} { + t.Run(initial, func(t *testing.T) { + h, agentID := newWebHandlerWithConfig(t, initial) + original, config, err := h.agentConfig(context.Background(), "tenant", agentID) + if err != nil { + t.Fatal(err) + } + config["acp"] = map[string]any{"profiles": map[string]any{"example": map[string]any{"name": "Example"}}} + if _, err := h.store.UpdateAgent(context.Background(), "tenant", agentID, map[string]any{"config": map[string]any{"executionMode": "sandbox", "cloud": map[string]any{"model": "newer"}}}); err != nil { + t.Fatal(err) + } + if err := h.saveAgentConfig(context.Background(), "tenant", original, config); err != nil { + t.Fatal(err) + } + _, saved, err := h.agentConfig(context.Background(), "tenant", agentID) + if err != nil || saved["executionMode"] != "sandbox" || saved["cloud"].(map[string]any)["model"] != "newer" || saved["acp"] == nil { + t.Fatalf("stale save changed unrelated settings: %#v err=%v", saved, err) + } + }) + } +} + +func TestPublicAgentConfigReplacementCanChangeExecutionPolicy(t *testing.T) { + h, agentID := newWebHandlerWithConfig(t, `{"executionMode":"sandbox","old":true}`) + for _, config := range []map[string]any{{"executionMode": "host"}, {"executionMode": "sandbox"}, {}} { + if _, err := h.store.UpdateAgent(context.Background(), "tenant", agentID, map[string]any{"config": config}); err != nil { + t.Fatal(err) + } + a, err := h.store.GetAgent(context.Background(), "tenant", agentID) + if err != nil { + t.Fatal(err) + } + want, _ := json.Marshal(config) + if string(a.Config) != string(want) { + t.Fatalf("public replacement changed: got=%s want=%s", a.Config, want) + } + } +} + +func TestConfigRoutesRetryWithoutLosingExecutionPolicy(t *testing.T) { + for _, kind := range []string{"cloud", "providers"} { + t.Run(kind, func(t *testing.T) { + h, agentID := newWebHandlerWithConfig(t, `{"executionMode":"host","cloud":{"model":"old","remove":"old"},"providers":{"webFetch":{"enabled":false}}}`) + var raced atomic.Bool + callback := "test:agent-policy-race" + if err := h.deps.Gorm.Callback().Query().After("gorm:query").Register(callback, func(tx *gorm.DB) { + if tx.Statement.Table != "agents" || len(tx.Statement.Selects) != 1 || tx.Statement.Selects[0] != "config_json" || raced.Swap(true) { + return + } + _, err := h.deps.DB.Exec(`UPDATE agents SET config_json=? WHERE id=?`, `{"executionMode":"sandbox","cloud":{"model":"newer","remove":"old"},"providers":{"webFetch":{"enabled":true}}}`, agentID) + if err != nil { + t.Errorf("concurrent save: %v", err) + } + }); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = h.deps.Gorm.Callback().Query().Remove(callback) }) + body := `{"systemPrompt":"hello","remove":null}` + method := h.updateCloudConfig + if kind == "providers" { + body = `{"webSearch":{"enabled":true},"enableMemory":false}` + method = h.putAgentProviders + } + request := httptest.NewRequest(http.MethodPut, "/api/agents/"+agentID+"/"+kind, strings.NewReader(body)) + route := chi.NewRouteContext() + route.URLParams.Add("id", agentID) + ctx := context.WithValue(request.Context(), chi.RouteCtxKey, route) + request = request.WithContext(httpx.WithPrincipal(ctx, httpx.Principal{TenantID: "tenant"})) + response := httptest.NewRecorder() + method(response, request) + if response.Code != http.StatusOK || !raced.Load() { + t.Fatalf("route failed: %d %s raced=%v", response.Code, response.Body.String(), raced.Load()) + } + a, config, err := h.agentConfig(context.Background(), "tenant", agentID) + if err != nil || config["executionMode"] != "sandbox" { + t.Fatalf("policy changed: %#v err=%v", config, err) + } + cloud := config["cloud"].(map[string]any) + providers := config["providers"].(map[string]any) + if cloud["model"] != "newer" || providers["webFetch"].(map[string]any)["enabled"] != true { + t.Fatalf("unrelated settings changed: %#v", config) + } + if kind == "cloud" { + if cloud["systemPrompt"] != "hello" || cloud["remove"] != nil { + t.Fatalf("cloud patch not applied: %#v", cloud) + } + } else if providers["webSearch"].(map[string]any)["enabled"] != true || a.EnableMemory { + t.Fatalf("provider patch not applied: %#v memory=%v", providers, a.EnableMemory) + } + }) + } +} diff --git a/apps/server/internal/runtime/agent_tool_runners.go b/apps/server/internal/runtime/agent_tool_runners.go index 194b6768..62259065 100644 --- a/apps/server/internal/runtime/agent_tool_runners.go +++ b/apps/server/internal/runtime/agent_tool_runners.go @@ -270,9 +270,9 @@ func (h *handler) runShellExec(ctx context.Context, tenant, agent string, args m if timeoutMS > agentShellMaxMS { timeoutMS = agentShellMaxMS } - callCtx, cancel := context.WithTimeout(ctx, time.Duration(timeoutMS)*time.Millisecond) + callCtx, cancel := context.WithTimeout(ctx, time.Duration(timeoutMS)*time.Millisecond+sandboxTransportGrace) defer cancel() - result, err := h.runtimeExecWithMode(callCtx, tenant, agent, builtinStringArg(args, "execution_mode"), "bash", "-lc", command) + result, err := h.runtimeExecWithModeTimeout(callCtx, tenant, agent, builtinStringArg(args, "execution_mode"), int64(timeoutMS), "bash", "-lc", command) if result == nil { if err != nil { return nil, err @@ -283,8 +283,17 @@ func (h *handler) runShellExec(ctx context.Context, tenant, agent string, args m } stdout, _ := result["stdout"].(string) stderr, _ := result["stderr"].(string) - stdout, _ = h.truncateToolText(ctx, tenant, agent, stdout, "shell") - stderr, _ = h.truncateToolText(ctx, tenant, agent, stderr, "shell") + serverTruncated := len(stdout) > toolOutputMaxBytes || len(stderr) > toolOutputMaxBytes + if result["executionMode"] == "sandbox" { + // Decide from the actual result, including operator-enforced sandboxing. + // This path must never spill output through host filesystem RPCs. + stdout = boundedToolTextPreview(stdout) + stderr = boundedToolTextPreview(stderr) + } else { + stdout, _ = h.truncateToolText(ctx, tenant, agent, stdout, "shell") + stderr, _ = h.truncateToolText(ctx, tenant, agent, stderr, "shell") + } + runnerTruncated, _ := result["truncated"].(bool) return builtinJSON(map[string]any{ "stdout": stdout, "stderr": stderr, @@ -294,6 +303,6 @@ func (h *handler) runShellExec(ctx context.Context, tenant, agent string, args m "restrictedPolicyApplied": result["restrictedPolicyApplied"], "timedOut": result["timedOut"], "cancelled": result["cancelled"], - "truncated": result["truncated"], + "truncated": runnerTruncated || serverTruncated, }) } diff --git a/apps/server/internal/runtime/http.go b/apps/server/internal/runtime/http.go index 24663c92..bc9a6fc8 100644 --- a/apps/server/internal/runtime/http.go +++ b/apps/server/internal/runtime/http.go @@ -834,26 +834,22 @@ func (h *handler) updateCloudConfig(w http.ResponseWriter, r *http.Request) { return } p := principal(r) - a, e := h.store.GetAgent(r.Context(), p.TenantID, chi.URLParam(r, "id")) - if e != nil { - statusErr(w, e) - return - } - cfg := map[string]any{} - _ = json.Unmarshal(a.Config, &cfg) - cloud, _ := cfg["cloud"].(map[string]any) - if cloud == nil { - cloud = map[string]any{} - } - for k, v := range patch { - if v == nil || v == "" { - delete(cloud, k) - } else { - cloud[k] = v + var cloud map[string]any + _, e := h.store.updateAgentConfig(r.Context(), p.TenantID, chi.URLParam(r, "id"), nil, func(cfg map[string]any) error { + cloud, _ = cfg["cloud"].(map[string]any) + if cloud == nil { + cloud = map[string]any{} } - } - cfg["cloud"] = cloud - a, e = h.store.UpdateAgent(r.Context(), p.TenantID, a.ID, map[string]any{"config": cfg}) + for k, v := range patch { + if v == nil || v == "" { + delete(cloud, k) + } else { + cloud[k] = v + } + } + cfg["cloud"] = cloud + return nil + }) if e != nil { statusErr(w, e) return diff --git a/apps/server/internal/runtime/instances.go b/apps/server/internal/runtime/instances.go index fc3cb995..b423d624 100644 --- a/apps/server/internal/runtime/instances.go +++ b/apps/server/internal/runtime/instances.go @@ -699,31 +699,26 @@ func (h *handler) putAgentProviders(w http.ResponseWriter, r *http.Request) { return } p := principal(r) - agent, err := h.store.GetAgent(r.Context(), p.TenantID, chi.URLParam(r, "id")) - if err != nil { - statusErr(w, err) - return - } - cfg := map[string]any{} - _ = json.Unmarshal(agent.Config, &cfg) - providers, _ := cfg["providers"].(map[string]any) - if providers == nil { - providers = map[string]any{} - } - for _, key := range []string{"webSearch", "webFetch"} { - if value, ok := patch[key]; ok { - providers[key] = value - } - } - cfg["providers"] = providers - agentPatch := map[string]any{"config": cfg} + columns := map[string]any{} if value, ok := patch["enableMemory"]; ok { - agentPatch["enableMemory"] = value + columns["enable_memory"] = value } if value, ok := patch["memoryProviderId"]; ok { - agentPatch["memoryProviderId"] = value + columns["memory_provider_id"] = value } - agent, err = h.store.UpdateAgent(r.Context(), p.TenantID, agent.ID, agentPatch) + agent, err := h.store.updateAgentConfig(r.Context(), p.TenantID, chi.URLParam(r, "id"), columns, func(cfg map[string]any) error { + providers, _ := cfg["providers"].(map[string]any) + if providers == nil { + providers = map[string]any{} + } + for _, key := range []string{"webSearch", "webFetch"} { + if value, ok := patch[key]; ok { + providers[key] = value + } + } + cfg["providers"] = providers + return nil + }) if err != nil { statusErr(w, err) return diff --git a/apps/server/internal/runtime/runner_hub.go b/apps/server/internal/runtime/runner_hub.go index 2b7c57eb..e4434220 100644 --- a/apps/server/internal/runtime/runner_hub.go +++ b/apps/server/internal/runtime/runner_hub.go @@ -53,18 +53,18 @@ type runnerReply struct { } type runnerSession struct { - hub *runnerHub - nodeID string - conn net.Conn - rw *bufio.ReadWriter - writeMu sync.Mutex - mu sync.Mutex - pending map[string]chan runnerReply - streams map[string]func(string, []byte) - done chan struct{} - closeOne sync.Once - ready atomic.Bool - lastSeen atomic.Int64 + hub *runnerHub + nodeID string + conn net.Conn + rw *bufio.ReadWriter + writeGate chan struct{} + mu sync.Mutex + pending map[string]chan runnerReply + streams map[string]func(string, []byte) + done chan struct{} + closeOne sync.Once + ready atomic.Bool + lastSeen atomic.Int64 } func newRunnerHub(deps *appdeps.Dependencies) *runnerHub { @@ -117,7 +117,7 @@ func (h *handler) runnerHubHTTP(w http.ResponseWriter, r *http.Request) { _ = conn.Close() return } - s := &runnerSession{hub: h.hub, nodeID: nodeID, conn: conn, rw: rw, pending: map[string]chan runnerReply{}, streams: map[string]func(string, []byte){}, done: make(chan struct{})} + s := &runnerSession{writeGate: make(chan struct{}, 1), hub: h.hub, nodeID: nodeID, conn: conn, rw: rw, pending: map[string]chan runnerReply{}, streams: map[string]func(string, []byte){}, done: make(chan struct{})} s.lastSeen.Store(time.Now().UnixMilli()) h.hub.install(s) defer func() { @@ -212,13 +212,54 @@ func (s *runnerSession) close(reason error) { } func (s *runnerSession) writeJSON(frame runnerFrame) error { + return s.writeJSONContext(context.Background(), frame) +} + +func (s *runnerSession) writeJSONContext(ctx context.Context, frame runnerFrame) error { raw, err := json.Marshal(frame) if err != nil { return err } - s.writeMu.Lock() - defer s.writeMu.Unlock() - return writeWSFrame(s.rw.Writer, 0x1, raw) + return s.writeFrameContext(ctx, 0x1, raw) +} + +// Bound both waiting for the writer and the socket write itself. An RPC budget +// cannot be enforced by selecting ctx.Done only after an unbounded write. +func (s *runnerSession) writeFrameContext(ctx context.Context, opcode byte, raw []byte) error { + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + select { + case s.writeGate <- struct{}{}: + defer func() { <-s.writeGate }() + case <-ctx.Done(): + return ctx.Err() + case <-s.done: + return errors.New("runtime node agent is offline") + } + if err := ctx.Err(); err != nil { + return err + } + deadline, _ := ctx.Deadline() + if err := s.conn.SetWriteDeadline(deadline); err != nil { + return err + } + interrupted := make(chan struct{}) + stop := context.AfterFunc(ctx, func() { _ = s.conn.SetWriteDeadline(time.Now()); close(interrupted) }) + defer func() { + if !stop() { + <-interrupted + } + _ = s.conn.SetWriteDeadline(time.Time{}) + }() + err := writeWSFrame(s.rw.Writer, opcode, raw) + if err != nil { + // A partially written frame cannot safely be reused by another RPC. + s.close(err) + if ctx.Err() != nil { + return ctx.Err() + } + } + return err } func (s *runnerSession) readLoop() { @@ -232,9 +273,9 @@ func (s *runnerSession) readLoop() { return } if opcode == 0x9 { - s.writeMu.Lock() - _ = writeWSFrame(s.rw.Writer, 0xA, payload) - s.writeMu.Unlock() + if err := s.writeFrameContext(context.Background(), 0xA, payload); err != nil { + return + } continue } if opcode != 0x1 && opcode != 0x2 { @@ -291,7 +332,7 @@ func (s *runnerSession) call(ctx context.Context, method string, params any, out } s.pending[id] = pending s.mu.Unlock() - if err := s.writeJSON(runnerFrame{Type: "req", ID: id, Method: method, Params: raw}); err != nil { + if err := s.writeJSONContext(ctx, runnerFrame{Type: "req", ID: id, Method: method, Params: raw}); err != nil { s.mu.Lock() delete(s.pending, id) s.mu.Unlock() diff --git a/apps/server/internal/runtime/runner_write_deadline_test.go b/apps/server/internal/runtime/runner_write_deadline_test.go new file mode 100644 index 00000000..3b806994 --- /dev/null +++ b/apps/server/internal/runtime/runner_write_deadline_test.go @@ -0,0 +1,50 @@ +package runtime + +import ( + "bufio" + "context" + "errors" + "net" + "testing" + "time" +) + +func TestRunnerWriteDeadlineBoundsStalledPeer(t *testing.T) { + conn, peer := net.Pipe() + defer peer.Close() + defer conn.Close() + s := &runnerSession{conn: conn, rw: bufio.NewReadWriter(bufio.NewReader(conn), bufio.NewWriter(conn)), writeGate: make(chan struct{}, 1), done: make(chan struct{})} + ctx, cancel := context.WithTimeout(context.Background(), 40*time.Millisecond) + defer cancel() + finished := make(chan error, 1) + go func() { finished <- s.writeJSONContext(ctx, runnerFrame{Type: "ping"}) }() + select { + case err := <-finished: + if err == nil { + t.Fatal("stalled peer succeeded") + } + case <-time.After(time.Second): + t.Fatal("socket write exceeded context budget") + } +} + +func TestRunnerWriteQueueHonorsCancellation(t *testing.T) { + conn, peer := net.Pipe() + defer peer.Close() + defer conn.Close() + s := &runnerSession{conn: conn, rw: bufio.NewReadWriter(bufio.NewReader(conn), bufio.NewWriter(conn)), writeGate: make(chan struct{}, 1), done: make(chan struct{})} + s.writeGate <- struct{}{} // another writer owns the connection + ctx, cancel := context.WithCancel(context.Background()) + cancel() + finished := make(chan error, 1) + go func() { finished <- s.writeJSONContext(ctx, runnerFrame{Type: "ping"}) }() + select { + case err := <-finished: + if !errors.Is(err, context.Canceled) { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("writer queue ignored cancellation") + } + <-s.writeGate +} diff --git a/apps/server/internal/runtime/sandbox_exec.go b/apps/server/internal/runtime/sandbox_exec.go index 65b9797d..c0ddd004 100644 --- a/apps/server/internal/runtime/sandbox_exec.go +++ b/apps/server/internal/runtime/sandbox_exec.go @@ -8,6 +8,13 @@ import ( "time" ) +const ( + sandboxStartBudget = 10 * time.Second + // Runner cleanup may use 10 seconds plus a 2-second subprocess wait. + sandboxCleanupBudget = 15 * time.Second + sandboxTransportGrace = sandboxStartBudget + sandboxCleanupBudget + 5*time.Second +) + // Use the runner's scoped job lifecycle so cancelling an agent run also cancels // its container. Cleanup has a separate deadline and never falls back to host. func runSandboxCommand(ctx context.Context, runner *runnerSession, params map[string]any) (result map[string]any, err error) { @@ -15,7 +22,7 @@ func runSandboxCommand(ctx context.Context, runner *runnerSession, params map[st return nil, ctx.Err() } result = map[string]any{} - startCtx, stopStart := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second) + startCtx, stopStart := context.WithTimeout(context.WithoutCancel(ctx), sandboxStartBudget) err = runner.call(startCtx, "host.exec.start", params, &result) stopStart() if err != nil { @@ -31,11 +38,23 @@ func runSandboxCommand(ctx context.Context, runner *runnerSession, params map[st if !running { return } - cleanup, cancel := context.WithTimeout(context.Background(), 5*time.Second) + cleanup, cancel := context.WithTimeout(context.Background(), sandboxCleanupBudget) defer cancel() - if cleanupErr := cancelSandboxJob(cleanup, runner, scoped); cleanupErr != nil { + terminal, cleanupErr := cancelSandboxJob(cleanup, runner, scoped) + if cleanupErr != nil { result = nil err = errors.Join(err, errors.New("sandbox cancellation could not be confirmed; the job remains bounded by its timeout")) + return + } + // Retain bounded output only after verified terminal cleanup. + if ctx.Err() != nil && terminal["exitCode"] != nil { + result = terminal + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + result["timedOut"] = true + } + if errors.Is(ctx.Err(), context.Canceled) && result["timedOut"] != true { + result["cancelled"] = true + } } }() for { @@ -70,31 +89,31 @@ func runSandboxCommand(ctx context.Context, runner *runnerSession, params map[st } } -func cancelSandboxJob(ctx context.Context, runner *runnerSession, scoped map[string]any) error { +func cancelSandboxJob(ctx context.Context, runner *runnerSession, scoped map[string]any) (map[string]any, error) { method := "host.exec.kill" for { var state map[string]any if err := runner.call(ctx, method, scoped, &state); err != nil { - return err + return nil, err } if state["executionMode"] != "sandbox" || state["isolated"] != true { - return errors.New("unconfirmed sandbox cancellation") + return nil, errors.New("unconfirmed sandbox cancellation") } running, ok := state["running"].(bool) if !ok { - return errors.New("invalid sandbox cancellation status") + return nil, errors.New("invalid sandbox cancellation status") } if !running { if failure, _ := state["error"].(string); failure != "" { - return errors.New("sandbox cleanup was not confirmed") + return nil, errors.New("sandbox cleanup was not confirmed") } - return nil + return state, nil } timer := time.NewTimer(100 * time.Millisecond) select { case <-ctx.Done(): timer.Stop() - return ctx.Err() + return nil, ctx.Err() case <-timer.C: } method = "host.exec.get" diff --git a/apps/server/internal/runtime/sandbox_integration_test.go b/apps/server/internal/runtime/sandbox_integration_test.go new file mode 100644 index 00000000..7b8f2b3f --- /dev/null +++ b/apps/server/internal/runtime/sandbox_integration_test.go @@ -0,0 +1,169 @@ +package runtime + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/go-chi/chi/v5" +) + +// Exercises real server tool dispatch -> WebSocket hub -> runner binary -> Docker. +// Only benign fixture commands are used; this is not an escape assessment. +func TestSandboxServerRunnerDockerIntegration(t *testing.T) { + if os.Getenv("ZAKURA_SERVER_SANDBOX_INTEGRATION") != "1" { + t.Skip("real server/runner Docker integration not requested") + } + binary := os.Getenv("ZAKURA_SANDBOX_AGENT_BINARY") + if !filepath.IsAbs(binary) { + t.Fatal("absolute ZAKURA_SANDBOX_AGENT_BINARY required") + } + if _, e := os.Stat(binary); e != nil { + t.Fatal(e) + } + if os.Getenv("ZAKURA_SANDBOX_IMAGE") == "" { + t.Fatal("pre-provisioned image with bash required") + } + if _, e := exec.LookPath("docker"); e != nil { + t.Fatal(e) + } + d := testDeps(t) + seedTenant(t, d, "tenant") + store := NewStore(d) + node := "node" + space, e := store.CreateSpace(context.Background(), "tenant", Space{Name: "S", RuntimeNodeID: &node, WorkspaceKind: "host"}) + if e != nil { + t.Fatal(e) + } + agent, e := store.CreateAgent(context.Background(), "tenant", Agent{Name: "A", SpaceID: space.ID, Config: json.RawMessage(`{"executionMode":"sandbox"}`)}) + if e != nil { + t.Fatal(e) + } + storage := t.TempDir() + workspace := filepath.Join(storage, "spaces", space.ID, "workspace") + if e := os.MkdirAll(workspace, 0755); e != nil { + t.Fatal(e) + } + if e := os.WriteFile(filepath.Join(workspace, "fixture.txt"), []byte("read-only fixture"), 0644); e != nil { + t.Fatal(e) + } + token := "rnr_real_sandbox_fixture" + hash := sha256.Sum256([]byte(token)) + now := d.Clock() + if _, e := d.DB.Exec(`INSERT INTO runtime_nodes(id,tenant_id,name,slug,kind,status,token_hash,capabilities_json,host_info_json,storage_root,labels_json,is_shared,created_at,updated_at) VALUES('node','tenant','Runner','runner','computer','offline',?,'{}','{}',?,'{}',false,?,?)`, hex.EncodeToString(hash[:]), storage, now, now); e != nil { + t.Fatal(e) + } + h := &handler{deps: d, store: store, mcpSessions: map[string]spaceMCPSession{}} + h.hub = newRunnerHub(d) + h.service = NewService(store) + router := chi.NewRouter() + router.Handle("/api/runtime-nodes/hub", http.HandlerFunc(h.runnerHubHTTP)) + server := httptest.NewServer(router) + t.Cleanup(server.Close) + runctx, stop := context.WithCancel(context.Background()) + cmd := exec.CommandContext(runctx, binary, "-server", server.URL, "-token", token, "-data", storage) + cmd.Env = append(os.Environ(), "ZAKURA_SANDBOX_ENABLED=true") + logFile, e := os.CreateTemp(t.TempDir(), "runner-log-") + if e != nil { + t.Fatal(e) + } + cmd.Stdout = logFile + cmd.Stderr = logFile + if e := cmd.Start(); e != nil { + stop() + t.Fatal(e) + } + t.Cleanup(func() { stop(); _ = cmd.Wait(); _ = logFile.Close() }) + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + if _, e := h.hub.get("node"); e == nil { + break + } + time.Sleep(20 * time.Millisecond) + } + if _, e := h.hub.get("node"); e != nil { + t.Fatal("real runner failed to connect", e) + } + call := func(ctx context.Context, script string, timeout int) map[string]any { + t.Helper() + args, _ := json.Marshal(map[string]any{"command": script, "timeout_ms": timeout}) + raw, e := h.dispatchAgentTool(ctx, "tenant", agent.ID, "", "fixture-call", "shell_exec", args) + return decodeBuiltinResult(t, raw, e) + } + assertNoOutputWrites := func() { + t.Helper() + files, e := os.ReadDir(workspace) + if e != nil { + t.Fatal(e) + } + if len(files) != 1 || files[0].Name() != "fixture.txt" { + t.Fatalf("server wrote outside sandbox: %v", files) + } + } + t.Run("long stdout and stderr never persist", func(t *testing.T) { + out := call(context.Background(), `head -c 30000 /dev/zero | tr '\000' x; head -c 30000 /dev/zero | tr '\000' e >&2`, 10000) + if out["truncated"] != true || out["executionMode"] != "sandbox" { + t.Fatalf("incorrect result flags: %v", out["truncated"]) + } + for _, key := range []string{"stdout", "stderr"} { + if s := out[key].(string); len(s) > toolOutputMaxBytes || !strings.Contains(s, "not persisted") { + t.Fatalf("invalid %s preview", key) + } + } + assertNoOutputWrites() + }) + t.Run("deadline preserves final output", func(t *testing.T) { + out := call(context.Background(), `printf 'before timeout'; printf diagnostic >&2; sleep 30`, 3000) + if out["timedOut"] != true || out["stdout"] != "before timeout" || out["stderr"] != "diagnostic" { + t.Fatalf("lost timeout result: %+v", out) + } + assertNoOutputWrites() + }) + t.Run("caller cancellation acknowledges cleanup", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + timer := time.AfterFunc(3*time.Second, cancel) + defer timer.Stop() + out := call(ctx, `printf 'before cancellation'; sleep 30`, 10000) + if out["cancelled"] != true || out["stdout"] != "before cancellation" { + t.Fatalf("lost cancellation result: %+v", out) + } + assertNoOutputWrites() + }) + t.Run("operator enforced result never persists", func(t *testing.T) { + if _, e := store.UpdateAgent(context.Background(), "tenant", agent.ID, map[string]any{"config": map[string]any{}}); e != nil { + t.Fatal(e) + } + out := call(context.Background(), `head -c 30000 /dev/zero | tr '\000' x`, 10000) + if out["executionMode"] != "sandbox" || out["truncated"] != true { + t.Fatal("operator enforcement lost") + } + assertNoOutputWrites() + }) + t.Run("operator enforced cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + timer := time.AfterFunc(3*time.Second, cancel) + defer timer.Stop() + out := call(ctx, `printf 'operator cancellation'; sleep 30`, 10000) + if out["cancelled"] != true || out["stdout"] != "operator cancellation" { + t.Fatalf("lost enforced cancellation: %+v", out) + } + assertNoOutputWrites() + }) + checkctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + remaining, e := exec.CommandContext(checkctx, "docker", "container", "ls", "--all", "--filter", "name=zakura-sandbox-", "--format", "{{.ID}}").Output() + if e != nil || strings.TrimSpace(string(remaining)) != "" { + t.Fatalf("container cleanup unconfirmed: %s %v", remaining, e) + } +} diff --git a/apps/server/internal/runtime/sandbox_output_test.go b/apps/server/internal/runtime/sandbox_output_test.go new file mode 100644 index 00000000..ae553eb7 --- /dev/null +++ b/apps/server/internal/runtime/sandbox_output_test.go @@ -0,0 +1,162 @@ +package runtime + +import ( + "context" + "encoding/json" + "errors" + "strings" + "sync/atomic" + "testing" + "time" +) + +func TestSandboxOutputNeverSpillsAndTracksServerTruncation(t *testing.T) { + for _, tc := range []struct { + name, mode string + size int + runnerTruncated bool + }{ + {"explicit boundary", "sandbox", toolOutputMaxBytes, false}, + {"explicit preview", "sandbox", toolOutputMaxBytes + 1, false}, + {"operator enforced preview", "", toolOutputMaxBytes + 1000, false}, + {"runner truncated", "sandbox", 10, true}, + } { + t.Run(tc.name, func(t *testing.T) { + d := testDeps(t) + seedTenant(t, d, "tenant") + store := NewStore(d) + node := "node" + space, e := store.CreateSpace(context.Background(), "tenant", Space{Name: "S", RuntimeNodeID: &node, WorkspaceKind: "host"}) + if e != nil { + t.Fatal(e) + } + agent, e := store.CreateAgent(context.Background(), "tenant", Agent{Name: "A", SpaceID: space.ID}) + if e != nil { + t.Fatal(e) + } + var writes atomic.Int32 + h := builtinToolHarness(t, d, "rnr_output", func(method string, p map[string]any) (any, error) { + switch method { + case "sys.info": + return map[string]any{"version": "test", "kind": "computer", "storageRoot": "/srv/zakura", "capabilities": map[string]any{}, "hostInfo": map[string]any{}}, nil + case "sandbox.policy": + return map[string]any{"enforced": tc.mode == "", "sandbox": map[string]any{"enabled": true}}, nil + case "host.exec", "host.exec.start": + return map[string]any{"id": "job", "running": false, "executionMode": "sandbox", "isolated": true, "exitCode": 0, "stdout": strings.Repeat("x", tc.size), "stderr": strings.Repeat("e", tc.size), "truncated": tc.runnerTruncated}, nil + case "host.fs.write", "host.fs.mkdir": + writes.Add(1) + return map[string]any{"ok": true}, nil + default: + return nil, errors.New("unexpected RPC " + method) + } + }) + args := map[string]any{"command": "printf fixture"} + if tc.mode != "" { + args["execution_mode"] = tc.mode + } + raw, e := h.runShellExec(context.Background(), "tenant", agent.ID, args) + out := decodeBuiltinResult(t, raw, e) + want := tc.runnerTruncated || tc.size > toolOutputMaxBytes + if out["truncated"] != want { + t.Fatalf("truncation=%v want%v", out["truncated"], want) + } + if writes.Load() != 0 { + t.Fatal("sandbox output wrote to host workspace") + } + for _, key := range []string{"stdout", "stderr"} { + v := out[key].(string) + if len(v) > toolOutputMaxBytes || strings.Contains(v, "full output:") { + t.Fatalf("invalid %s preview", key) + } + } + }) + } +} + +func TestSandboxExecutionTimeoutKeepsFinalOutput(t *testing.T) { + d := testDeps(t) + seedTenant(t, d, "tenant") + store := NewStore(d) + node := "node" + space, _ := store.CreateSpace(context.Background(), "tenant", Space{Name: "S", RuntimeNodeID: &node, WorkspaceKind: "host"}) + agent, _ := store.CreateAgent(context.Background(), "tenant", Agent{Name: "A", SpaceID: space.ID}) + var started time.Time + h := builtinToolHarness(t, d, "rnr_timeout_output", func(method string, p map[string]any) (any, error) { + switch method { + case "sys.info": + return map[string]any{"version": "test", "kind": "computer", "storageRoot": "/srv/zakura", "capabilities": map[string]any{}, "hostInfo": map[string]any{}}, nil + case "sandbox.policy": + return map[string]any{"sandbox": map[string]any{"enabled": true}}, nil + case "host.exec.start": + if p["timeoutMs"].(float64) > 1000 { + t.Error("transport grace increased execution budget") + } + started = time.Now() + return map[string]any{"id": "job", "running": true, "executionMode": "sandbox", "isolated": true}, nil + case "host.exec.get": + if time.Since(started) < 1100*time.Millisecond { + return map[string]any{"id": "job", "running": true, "executionMode": "sandbox", "isolated": true}, nil + } + return map[string]any{"id": "job", "running": false, "executionMode": "sandbox", "isolated": true, "exitCode": 137, "stdout": "before timeout", "stderr": "diagnostic", "timedOut": true}, nil + default: + return nil, errors.New("unexpected RPC " + method) + } + }) + raw, e := h.runShellExec(context.Background(), "tenant", agent.ID, map[string]any{"command": "fixture", "timeout_ms": float64(1000), "execution_mode": "sandbox"}) + out := decodeBuiltinResult(t, raw, e) + if out["timedOut"] != true || out["stdout"] != "before timeout" || out["stderr"] != "diagnostic" { + t.Fatalf("lost terminal result: %s", raw) + } +} + +func TestSandboxCallerCancellationKeepsConfirmedBoundedOutput(t *testing.T) { + d := testDeps(t) + seedTenant(t, d, "tenant") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + h := builtinToolHarness(t, d, "rnr_cancel_output", func(method string, p map[string]any) (any, error) { + switch method { + case "sys.info": + return map[string]any{"version": "test", "kind": "computer", "storageRoot": "/srv/zakura", "capabilities": map[string]any{}, "hostInfo": map[string]any{}}, nil + case "host.exec.start": + cancel() + return map[string]any{"id": "job", "running": true, "executionMode": "sandbox", "isolated": true}, nil + case "host.exec.kill": + return map[string]any{"id": "job", "running": false, "executionMode": "sandbox", "isolated": true, "exitCode": 137, "stdout": "bounded final", "cancelled": true}, nil + default: + return nil, errors.New("unexpected RPC " + method) + } + }) + runner, _ := h.hub.get("node") + out, e := runSandboxCommand(ctx, runner, map[string]any{"spaceId": "space", "executionMode": "sandbox"}) + if !errors.Is(e, context.Canceled) || out["stdout"] != "bounded final" || out["cancelled"] != true { + raw, _ := json.Marshal(out) + t.Fatalf("result=%s err=%v", raw, e) + } +} + +func TestLegacyWorkspaceDeadlineRemainsUnchanged(t *testing.T) { + d := testDeps(t) + seedTenant(t, d, "tenant") + store := NewStore(d) + node := "node" + space, _ := store.CreateSpace(context.Background(), "tenant", Space{Name: "S", RuntimeNodeID: &node, WorkspaceKind: "container"}) + agent, _ := store.CreateAgent(context.Background(), "tenant", Agent{Name: "A", SpaceID: space.ID}) + h := builtinToolHarness(t, d, "rnr_legacy_deadline", func(method string, p map[string]any) (any, error) { + switch method { + case "sys.info": + return map[string]any{"version": "test", "kind": "computer", "storageRoot": "/srv/zakura", "capabilities": map[string]any{}, "hostInfo": map[string]any{}}, nil + case "docker.list": + return []map[string]any{{"dockerId": "ws", "labels": map[string]string{"zakura.purpose": "workspace"}}}, nil + case "docker.exec": + time.Sleep(150 * time.Millisecond) + return map[string]any{"exitCode": 0}, nil + default: + return nil, errors.New("unexpected RPC " + method) + } + }) + _, e := h.runtimeExecWithModeTimeout(context.Background(), "tenant", agent.ID, "host", 25, "true") + if !errors.Is(e, context.DeadlineExceeded) { + t.Fatalf("legacy deadline changed: %v", e) + } +} diff --git a/apps/server/internal/runtime/tool_output.go b/apps/server/internal/runtime/tool_output.go index ab5379a0..e1f1bd67 100644 --- a/apps/server/internal/runtime/tool_output.go +++ b/apps/server/internal/runtime/tool_output.go @@ -83,3 +83,12 @@ func (h *handler) capToolResultJSON(ctx context.Context, tenant, agent string, r } return out } + +// boundedToolTextPreview never writes an artifact. Sandbox results use this +// instead of the ordinary workspace spillover mechanism. +func boundedToolTextPreview(text string) string { + if len(text) <= toolOutputMaxBytes { + return text + } + return text[:toolOutputFallbackHead] + "\n…[output truncated; not persisted by sandbox policy]…\n" + text[len(text)-toolOutputFallbackTail:] +} diff --git a/docs/sandbox.md b/docs/sandbox.md index 3a23ba20..e9409bde 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -47,7 +47,7 @@ Read-only workspace means build/install workflows must use `/tmp` and cannot per ## Failure and lifecycle behavior -The server uses workspace-scoped background start/get/kill RPCs for sandbox commands. Cancelling an agent run sends a scoped kill and waits up to five seconds for a terminal acknowledgment. Cleanup or terminal execution errors are surfaced, and an unconfirmed cancellation is reported as uncertain rather than successful. +The server uses workspace-scoped background start/get/kill RPCs for sandbox commands. Cancelling an agent run sends a scoped kill and waits up to fifteen seconds for a terminal acknowledgment. Cleanup or terminal execution errors are surfaced, and an unconfirmed cancellation is reported as uncertain rather than successful. Starting a job uses a separate ten-second deadline so that an agent-run cancellation does not immediately discard the job identity needed for cleanup. If the start response is lost or cannot be confirmed, the server does not retry: the outcome is unknown, and any started job remains bounded by its hard execution timeout. Operator follow-up may still be needed when connectivity or daemon failures prevent cleanup confirmation. @@ -76,3 +76,17 @@ ZAKURA_SANDBOX_INTEGRATION=1 ZAKURA_SANDBOX_IMAGE=' WebSocket runner -> Docker path. This integration uses benign fixture output and sleep commands, and is separate from direct backend tests. + +Restricted output uses an in-memory bounded preview only, including when runner policy enforces sandboxing for a legacy request. The server does not spill that output through host filesystem RPCs. The structured `truncated` flag combines the runner byte cap and the server 24,000-byte preview threshold. Ordinary host/container workspace spillover behavior is unchanged. + +Command execution timeout is passed separately from the RPC observation budget (up to 30 seconds of transport/cleanup grace). Caller cancellation is still acted upon immediately. Cleanup has an independent 15-second confirmation budget, covering the runner's 10-second cleanup and process wait. Bounded final stdout/stderr and timeout/cancellation flags are preserved only after verified terminal cleanup; unconfirmed cleanup remains an error. + +The opt-in `ZAKURA_SERVER_SANDBOX_INTEGRATION=1` test requires `ZAKURA_SANDBOX_AGENT_BINARY` pointing to a built runner and `ZAKURA_SANDBOX_IMAGE` pointing to the bash-capable image. It checks long stdout/stderr without host writes, final output after timeout, caller cancellation, and operator-enforced sandbox output. Missing prerequisites fail rather than skip when opted in. + +Internal ACP, cloud, provider and admin-default configuration saves now apply field-scoped compare-and-swap mutations to current JSON, retrying a bounded number of times on concurrent changes. They do not restore an unchanged `executionMode` from an old snapshot. The public full-config replacement API remains deliberate replacement, including intentional policy changes; callers using that API must not submit an unrelated stale whole-object snapshot. Regression tests cover SQLite and an opt-in disposable PostgreSQL service. + +RPC write queue acquisition and socket writes are also deadline-bound; an interrupted partial WebSocket frame closes that connection rather than being reused. A failed transport can still make cleanup unconfirmable, which remains an explicit error rather than a containment guarantee. From 10fa4ce79011d122a8d622ed0853bdb10dd39b54 Mon Sep 17 00:00:00 2001 From: momo-mnsjtxy <122842784+momo-mnsjtxy@users.noreply.github.com> Date: Wed, 7 Oct 2026 01:46:11 -0700 Subject: [PATCH 3/3] Preserve fragmented runner output across real WebSocket transport --- .../internal/runtime/runner_fragment_test.go | 69 +++++++++++++++++++ apps/server/internal/runtime/runner_hub.go | 57 ++++++++++++--- .../runtime/sandbox_integration_test.go | 16 +++-- docs/sandbox.md | 2 + 4 files changed, 128 insertions(+), 16 deletions(-) create mode 100644 apps/server/internal/runtime/runner_fragment_test.go diff --git a/apps/server/internal/runtime/runner_fragment_test.go b/apps/server/internal/runtime/runner_fragment_test.go new file mode 100644 index 00000000..64fd0591 --- /dev/null +++ b/apps/server/internal/runtime/runner_fragment_test.go @@ -0,0 +1,69 @@ +package runtime + +import ( + "bufio" + "bytes" + "encoding/binary" + "encoding/json" + "io" + "net" + "strings" + "testing" + "time" +) + +func TestRunnerReassemblesOrdinaryFragmentedOutput(t *testing.T) { + conn, peer := net.Pipe() + defer peer.Close() + defer conn.Close() + pending := make(chan runnerReply, 1) + s := &runnerSession{conn: conn, rw: bufio.NewReadWriter(bufio.NewReader(conn), bufio.NewWriter(conn)), writeGate: make(chan struct{}, 1), done: make(chan struct{}), pending: map[string]chan runnerReply{"42": pending}} + go s.readLoop() + go func() { _, _ = io.Copy(io.Discard, peer) }() // consume pong while client writes + body, _ := json.Marshal(map[string]any{"type": "res", "id": "42", "ok": true, "result": map[string]any{"stdout": strings.Repeat("ordinary fixture", 4000)}}) + write := func(final bool, opcode byte, payload []byte) { + var b bytes.Buffer + first := opcode + if final { + first |= 0x80 + } + b.WriteByte(first) + if len(payload) < 126 { + b.WriteByte(0x80 | byte(len(payload))) + } else { + b.WriteByte(0x80 | 126) + _ = binary.Write(&b, binary.BigEndian, uint16(len(payload))) + } + mask := []byte{1, 2, 3, 4} + b.Write(mask) + for i, v := range payload { + b.WriteByte(v ^ mask[i%4]) + } + if _, e := peer.Write(b.Bytes()); e != nil { + t.Fatal(e) + } + } + for start := 0; start < len(body); start += 4096 { + end := start + 4096 + if end > len(body) { + end = len(body) + } + opcode := byte(0) + if start == 0 { + opcode = 1 + } + write(end == len(body), opcode, body[start:end]) + if start == 0 { + write(true, 9, []byte("fixture ping")) + } + } + select { + case reply := <-pending: + var out map[string]string + if e := json.Unmarshal(reply.result, &out); e != nil || out["stdout"] != strings.Repeat("ordinary fixture", 4000) { + t.Fatalf("lost fragmented JSON: %v", e) + } + case <-time.After(time.Second): + t.Fatal("fragmented runner reply was not delivered") + } +} diff --git a/apps/server/internal/runtime/runner_hub.go b/apps/server/internal/runtime/runner_hub.go index e4434220..e360ca99 100644 --- a/apps/server/internal/runtime/runner_hub.go +++ b/apps/server/internal/runtime/runner_hub.go @@ -264,8 +264,10 @@ func (s *runnerSession) writeFrameContext(ctx context.Context, opcode byte, raw func (s *runnerSession) readLoop() { defer s.close(errors.New("runner connection closed")) + var fragmented []byte + var fragmentOpcode byte for { - opcode, payload, err := readWSFrameSized(s.rw.Reader, 16<<20) + final, opcode, payload, err := readWSFrameParts(s.rw.Reader, 16<<20) if err != nil { return } @@ -278,9 +280,33 @@ func (s *runnerSession) readLoop() { } continue } - if opcode != 0x1 && opcode != 0x2 { + if opcode == 0xA { continue } + switch opcode { + case 0x1, 0x2: + if fragmentOpcode != 0 { + return + } + if !final { + fragmentOpcode = opcode + fragmented = payload + continue + } + case 0x0: + if fragmentOpcode == 0 || len(fragmented)+len(payload) > 16<<20 { + return + } + fragmented = append(fragmented, payload...) + if !final { + continue + } + payload = fragmented + fragmented = nil + fragmentOpcode = 0 + default: + return + } var frame runnerFrame if json.Unmarshal(payload, &frame) != nil { continue @@ -369,42 +395,53 @@ func (s *runnerSession) onStream(id string, callback func(string, []byte)) func( } func readWSFrameSized(r *bufio.Reader, limit uint64) (byte, []byte, error) { + _, opcode, payload, err := readWSFrameParts(r, limit) + return opcode, payload, err +} + +func readWSFrameParts(r *bufio.Reader, limit uint64) (bool, byte, []byte, error) { var head [2]byte if _, err := io.ReadFull(r, head[:]); err != nil { - return 0, nil, err + return false, 0, nil, err } opcode := head[0] & 0x0f + if head[0]&0x70 != 0 { + return false, 0, nil, errors.New("unsupported websocket extensions") + } + if opcode >= 0x8 && (head[0]&0x80 == 0 || head[1]&0x7f > 125) { + return false, 0, nil, errors.New("invalid websocket control frame") + } masked := head[1]&0x80 != 0 if !masked { - return 0, nil, errors.New("client websocket frames must be masked") + return false, 0, nil, errors.New("client websocket frames must be masked") } n := uint64(head[1] & 0x7f) if n == 126 { var value [2]byte if _, err := io.ReadFull(r, value[:]); err != nil { - return 0, nil, err + return false, 0, nil, err } n = uint64(binary.BigEndian.Uint16(value[:])) } else if n == 127 { var value [8]byte if _, err := io.ReadFull(r, value[:]); err != nil { - return 0, nil, err + return false, 0, nil, err } n = binary.BigEndian.Uint64(value[:]) } if n > limit { - return 0, nil, errors.New("websocket frame too large") + return false, 0, nil, errors.New("websocket frame too large") } var mask [4]byte if _, err := io.ReadFull(r, mask[:]); err != nil { - return 0, nil, err + return false, 0, nil, err } payload := make([]byte, n) if _, err := io.ReadFull(r, payload); err != nil { - return 0, nil, err + return false, 0, nil, err } for i := range payload { payload[i] ^= mask[i%4] } - return opcode, payload, nil + return head[0]&0x80 != 0, opcode, payload, nil } diff --git a/apps/server/internal/runtime/sandbox_integration_test.go b/apps/server/internal/runtime/sandbox_integration_test.go index 7b8f2b3f..d6185525 100644 --- a/apps/server/internal/runtime/sandbox_integration_test.go +++ b/apps/server/internal/runtime/sandbox_integration_test.go @@ -93,10 +93,14 @@ func TestSandboxServerRunnerDockerIntegration(t *testing.T) { if _, e := h.hub.get("node"); e != nil { t.Fatal("real runner failed to connect", e) } - call := func(ctx context.Context, script string, timeout int) map[string]any { + call := func(t *testing.T, ctx context.Context, script string, timeout int) map[string]any { t.Helper() args, _ := json.Marshal(map[string]any{"command": script, "timeout_ms": timeout}) raw, e := h.dispatchAgentTool(ctx, "tenant", agent.ID, "", "fixture-call", "shell_exec", args) + if e != nil { + logs, _ := os.ReadFile(logFile.Name()) + t.Logf("fixture runner logs: %s", logs) + } return decodeBuiltinResult(t, raw, e) } assertNoOutputWrites := func() { @@ -110,7 +114,7 @@ func TestSandboxServerRunnerDockerIntegration(t *testing.T) { } } t.Run("long stdout and stderr never persist", func(t *testing.T) { - out := call(context.Background(), `head -c 30000 /dev/zero | tr '\000' x; head -c 30000 /dev/zero | tr '\000' e >&2`, 10000) + out := call(t, context.Background(), `head -c 30000 /dev/zero | tr '\000' x; head -c 30000 /dev/zero | tr '\000' e >&2`, 10000) if out["truncated"] != true || out["executionMode"] != "sandbox" { t.Fatalf("incorrect result flags: %v", out["truncated"]) } @@ -122,7 +126,7 @@ func TestSandboxServerRunnerDockerIntegration(t *testing.T) { assertNoOutputWrites() }) t.Run("deadline preserves final output", func(t *testing.T) { - out := call(context.Background(), `printf 'before timeout'; printf diagnostic >&2; sleep 30`, 3000) + out := call(t, context.Background(), `printf 'before timeout'; printf diagnostic >&2; sleep 30`, 3000) if out["timedOut"] != true || out["stdout"] != "before timeout" || out["stderr"] != "diagnostic" { t.Fatalf("lost timeout result: %+v", out) } @@ -133,7 +137,7 @@ func TestSandboxServerRunnerDockerIntegration(t *testing.T) { defer cancel() timer := time.AfterFunc(3*time.Second, cancel) defer timer.Stop() - out := call(ctx, `printf 'before cancellation'; sleep 30`, 10000) + out := call(t, ctx, `printf 'before cancellation'; sleep 30`, 10000) if out["cancelled"] != true || out["stdout"] != "before cancellation" { t.Fatalf("lost cancellation result: %+v", out) } @@ -143,7 +147,7 @@ func TestSandboxServerRunnerDockerIntegration(t *testing.T) { if _, e := store.UpdateAgent(context.Background(), "tenant", agent.ID, map[string]any{"config": map[string]any{}}); e != nil { t.Fatal(e) } - out := call(context.Background(), `head -c 30000 /dev/zero | tr '\000' x`, 10000) + out := call(t, context.Background(), `head -c 30000 /dev/zero | tr '\000' x`, 10000) if out["executionMode"] != "sandbox" || out["truncated"] != true { t.Fatal("operator enforcement lost") } @@ -154,7 +158,7 @@ func TestSandboxServerRunnerDockerIntegration(t *testing.T) { defer cancel() timer := time.AfterFunc(3*time.Second, cancel) defer timer.Stop() - out := call(ctx, `printf 'operator cancellation'; sleep 30`, 10000) + out := call(t, ctx, `printf 'operator cancellation'; sleep 30`, 10000) if out["cancelled"] != true || out["stdout"] != "operator cancellation" { t.Fatalf("lost enforced cancellation: %+v", out) } diff --git a/docs/sandbox.md b/docs/sandbox.md index e9409bde..9309a548 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -90,3 +90,5 @@ The opt-in `ZAKURA_SERVER_SANDBOX_INTEGRATION=1` test requires `ZAKURA_SANDBOX_A Internal ACP, cloud, provider and admin-default configuration saves now apply field-scoped compare-and-swap mutations to current JSON, retrying a bounded number of times on concurrent changes. They do not restore an unchanged `executionMode` from an old snapshot. The public full-config replacement API remains deliberate replacement, including intentional policy changes; callers using that API must not submit an unrelated stale whole-object snapshot. Regression tests cover SQLite and an opt-in disposable PostgreSQL service. RPC write queue acquisition and socket writes are also deadline-bound; an interrupted partial WebSocket frame closes that connection rather than being reused. A failed transport can still make cleanup unconfirmable, which remains an explicit error rather than a containment guarantee. + +Runner RPC responses are reassembled from bounded WebSocket fragments before JSON decoding. Real client libraries can fragment ordinary long stdout/stderr responses; the server preserves those responses and handles interleaved control frames rather than silently losing them. The combined message remains limited to 16 MiB.