diff --git a/AGENTS.md b/AGENTS.md index 94e3d01b..67b28b6c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -284,11 +284,10 @@ ODEK_E2E=true go test -v -count=1 ./cmd/odek/ -run "TestE2E_" # MCP E2E tests (builds fakeserver from source at test time) ODEK_E2E=true go test -v -count=1 ./cmd/odek/ -run "TestMCPClientE2E_" -# Sandbox integration tests (requires Docker). Tests swap HOME, which hides -# Docker Desktop's CLI context, so point DOCKER_HOST at the daemon or the -# Docker-gated tests skip as "Docker daemon unavailable". -DOCKER_HOST=$(docker context inspect --format '{{.Endpoints.docker.Host}}') \ - ODEK_E2E=true go test -v -count=1 ./cmd/odek/ -run "TestSandbox|TestE2E_Sandbox" +# Sandbox integration tests (requires Docker). TestMain pins the active docker +# context's endpoint as DOCKER_HOST before it swaps in a fixture HOME, so Docker +# Desktop is reached too. +ODEK_E2E=true go test -v -count=1 ./cmd/odek/ -run "TestSandbox|TestE2E_Sandbox" # Prompt-injection red-team E2E (on-demand only — NOT on CI; its own gate, # ODEK_PI_E2E, is separate from ODEK_E2E). Builds the binary, plants IPI diff --git a/cmd/odek/dockerhost_test.go b/cmd/odek/dockerhost_test.go new file mode 100644 index 00000000..5b9e86be --- /dev/null +++ b/cmd/odek/dockerhost_test.go @@ -0,0 +1,28 @@ +package main + +import ( + "context" + "os" + "os/exec" + "strings" + "time" +) + +// pinDockerHostForTests exports the active docker context's endpoint as +// DOCKER_HOST while HOME is still the operator's, so the docker CLI keeps +// reaching the daemon after TestMain swaps in a fixture HOME. An explicit +// DOCKER_HOST or DOCKER_CONTEXT, or no docker CLI, leaves the env alone. +func pinDockerHostForTests() { + if os.Getenv("DOCKER_HOST") != "" || os.Getenv("DOCKER_CONTEXT") != "" { + return + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + out, err := exec.CommandContext(ctx, "docker", "context", "inspect", "--format", "{{.Endpoints.docker.Host}}").Output() + if err != nil { + return + } + if host := strings.TrimSpace(string(out)); host != "" { + os.Setenv("DOCKER_HOST", host) + } +} diff --git a/cmd/odek/main.go b/cmd/odek/main.go index 25a1d23e..3e02f57f 100644 --- a/cmd/odek/main.go +++ b/cmd/odek/main.go @@ -2660,7 +2660,10 @@ func setupSandbox(tools []odek.Tool, cfg sandboxConfig) (containerName string, c if nerr != nil { return "", nil, nerr } - if gerr := checkProjectNetworkGrant(cfg.ResolvedNetworks); gerr != nil { + if gerr := checkAllowedNetworkIDs(cfg); gerr != nil { + return "", nil, gerr + } + if gerr := checkProjectNetworkGrant(cfg); gerr != nil { return "", nil, gerr } resolvedNets := cfg.ResolvedNetworks diff --git a/cmd/odek/project_sandbox_approval.go b/cmd/odek/project_sandbox_approval.go index 518b8ae1..b93eca69 100644 --- a/cmd/odek/project_sandbox_approval.go +++ b/cmd/odek/project_sandbox_approval.go @@ -413,18 +413,11 @@ func resolveOverrideNetworks(o config.ProjectSandboxOverride) ([]sandbox.Network return cfg.ResolvedNetworks, nil } -// projectNetworkGrant records, for one project directory, the networks the -// operator approved: the names and Compose project the project requested and -// the IDs they resolved to when approval was given. -type projectNetworkGrant struct { - names map[string]bool - compose string - ids map[string]bool -} - +// projectNetworkGrants records, per project directory, the IDs of the +// networks the operator approved for the project's own network request. var ( projectNetworkGrantsMu sync.Mutex - projectNetworkGrants = map[string]projectNetworkGrant{} + projectNetworkGrants = map[string]map[string]bool{} ) // recordProjectNetworkGrant remembers the networks behind an approval of a @@ -433,39 +426,46 @@ func recordProjectNetworkGrant(projectDir string, o config.ProjectSandboxOverrid if !o.HasNetworks && !o.HasComposeProject { return } - g := projectNetworkGrant{names: map[string]bool{}, ids: map[string]bool{}} - if o.HasNetworks { - for _, n := range o.Networks { - g.names[n] = true - } - } - if o.HasComposeProject { - g.compose = o.ComposeProject - } + ids := map[string]bool{} for _, n := range nets { - g.ids[n.ID] = true + ids[n.ID] = true } projectNetworkGrantsMu.Lock() defer projectNetworkGrantsMu.Unlock() - projectNetworkGrants[projectDir] = g + projectNetworkGrants[projectDir] = ids } // resetProjectNetworkGrants forgets every recorded grant. func resetProjectNetworkGrants() { projectNetworkGrantsMu.Lock() defer projectNetworkGrantsMu.Unlock() - projectNetworkGrants = map[string]projectNetworkGrant{} + projectNetworkGrants = map[string]map[string]bool{} } // checkProjectNetworkGrant enforces the project network approval at the // point of container creation. Networks are resolved again at setup, so -// every network the project contributes (by name or through its Compose -// project) must have an ID the operator approved: a network that joined the -// Compose project, or was recreated under an approved name, after the prompt -// is refused. Networks configured by the operator need no grant, and -// ODEK_APPROVE_PROJECT_SANDBOX=1 approves whatever the project requests. -func checkProjectNetworkGrant(nets []sandbox.NetworkInfo) error { - if os.Getenv("ODEK_APPROVE_PROJECT_SANDBOX") == "1" || len(nets) == 0 { +// every network the project config contributes (by name or through its +// Compose project) must have an ID the operator approved in this process: a +// network that joined the Compose project, or was recreated under an +// approved name, after the prompt is refused, and so is a project request +// that was never approved here. Networks configured by the operator need no +// grant, and ODEK_APPROVE_PROJECT_SANDBOX=1 approves whatever the project +// requests. +func checkProjectNetworkGrant(cfg sandboxConfig) error { + if os.Getenv("ODEK_APPROVE_PROJECT_SANDBOX") == "1" { + return nil + } + requested := map[string]bool{} + for _, n := range cfg.ProjectNetworks { + requested[n] = true + } + var owned []sandbox.NetworkInfo + for _, n := range cfg.ResolvedNetworks { + if requested[n.Name] || (cfg.ProjectComposeProject != "" && n.ComposeProject == cfg.ProjectComposeProject) { + owned = append(owned, n) + } + } + if len(owned) == 0 { return nil } projectDir, err := os.Getwd() @@ -476,21 +476,46 @@ func checkProjectNetworkGrant(nets []sandbox.NetworkInfo) error { return fmt.Errorf("%w: project sandbox networks: working directory: %w", errSandboxConfig, err) } projectNetworkGrantsMu.Lock() - g, ok := projectNetworkGrants[projectDir] + ids, granted := projectNetworkGrants[projectDir] projectNetworkGrantsMu.Unlock() - if !ok { - return nil - } var unapproved []string - for _, n := range nets { - owned := g.names[n.Name] || (g.compose != "" && n.ComposeProject == g.compose) - if owned && !g.ids[n.ID] { + for _, n := range owned { + if !ids[n.ID] { unapproved = append(unapproved, n.Name) } } - if len(unapproved) > 0 { - return fmt.Errorf("%w: project sandbox networks changed since they were approved: %s; run again to review and approve them", - errSandboxConfig, danger.SanitizeInline(strings.Join(unapproved, ", "))) + if len(unapproved) == 0 { + return nil + } + names := danger.SanitizeInline(strings.Join(unapproved, ", ")) + if !granted { + return fmt.Errorf("%w: project sandbox networks were not approved: %s; run again to review and approve them", + errSandboxConfig, names) + } + return fmt.Errorf("%w: project sandbox networks changed since they were approved: %s; run again to review and approve them", + errSandboxConfig, names) +} + +// checkAllowedNetworkIDs limits a sub-agent to the networks its parent +// attached: every resolved network must still carry one of the stamped IDs. +// A network recreated under the parent's name has a new ID and is refused. +func checkAllowedNetworkIDs(cfg sandboxConfig) error { + if len(cfg.AllowedNetworkIDs) == 0 { + return nil + } + allowed := map[string]bool{} + for _, id := range cfg.AllowedNetworkIDs { + allowed[id] = true + } + var changed []string + for _, n := range cfg.ResolvedNetworks { + if !allowed[n.ID] { + changed = append(changed, n.Name) + } + } + if len(changed) > 0 { + return fmt.Errorf("%w: sandbox networks changed since the parent sandbox resolved them: %s", + errSandboxConfig, danger.SanitizeInline(strings.Join(changed, ", "))) } return nil } diff --git a/cmd/odek/red_fd_handoff_test.go b/cmd/odek/red_fd_handoff_test.go new file mode 100644 index 00000000..86b94370 --- /dev/null +++ b/cmd/odek/red_fd_handoff_test.go @@ -0,0 +1,34 @@ +package main + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// readKeyFromInheritedFD and readFrameNonceFromInheritedFD take ownership of +// the descriptor named in their env var and close it. A test that names a +// descriptor still owned by an *os.File gets it closed twice: the file's +// finalizer later closes whatever the number has been reused for — in a +// parallel test often the directory fd os.RemoveAll is walking, which then +// fails with "unlinkat: bad file descriptor". Tests must hand over a dup. +func TestRED_FDHandoffTestsPassADuplicatedDescriptor(t *testing.T) { + files, err := filepath.Glob("*_test.go") + if err != nil { + t.Fatal(err) + } + raw := regexp.MustCompile(`t\.Setenv\((keyFDEnvVar|frameFDEnvVar)[^\n]*\.Fd\(\)`) + for _, f := range files { + data, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + for _, m := range raw.FindAllString(string(data), -1) { + if !strings.Contains(m, "Dup") && !strings.Contains(m, "dupFD") { + t.Errorf("%s hands an owned descriptor to the fd reader: %s (use dupFD)", f, m) + } + } + } +} diff --git a/cmd/odek/red_network_residuals_test.go b/cmd/odek/red_network_residuals_test.go new file mode 100644 index 00000000..95f56612 --- /dev/null +++ b/cmd/odek/red_network_residuals_test.go @@ -0,0 +1,97 @@ +package main + +import ( + "errors" + "io" + "os" + "reflect" + "strings" + "testing" + + odek "github.com/BackendStack21/odek" + "github.com/BackendStack21/odek/internal/config" + "github.com/BackendStack21/odek/internal/sandbox" +) + +// A project that requests networks is held to an approval recorded in this +// process: setup that finds no grant refuses instead of attaching them. +func TestRED_ProjectNetworkGrant_MissingGrantIsRefused(t *testing.T) { + approvalEnv(t) + resetProjectNetworkGrants() + d := &mutableComposeDocker{project: "app", nets: map[string]fakeNet{ + "app_default": {id: id("a"), internal: true, compose: "app"}, + }} + d.install(t) + resolved := config.ResolvedConfig{ + SandboxComposeProject: "app", + ProjectSandboxOverride: config.ProjectSandboxOverride{HasComposeProject: true, ComposeProject: "app"}, + } + cfg, err := resolveSandboxNetworks(sandboxConfigFromResolved(resolved), io.Discard) + if err != nil { + t.Fatal(err) + } + err = checkProjectNetworkGrant(cfg) + if err == nil || !errors.Is(err, errSandboxConfig) { + t.Fatalf("project networks without a recorded approval must be refused, got %v", err) + } + t.Setenv("ODEK_APPROVE_PROJECT_SANDBOX", "1") + if err := checkProjectNetworkGrant(cfg); err != nil { + t.Fatalf("env approval covers setup: %v", err) + } +} + +// The parent stamps the IDs of the networks it attached; a child attaches a +// network only while it still has one of those IDs. +func TestRED_SubagentNetworks_StampedIDsAreEnforced(t *testing.T) { + d := &delegateTasksTool{} + recordChildSandbox([]odek.Tool{d}, sandboxConfig{ + Networks: []string{"db"}, + ResolvedNetworks: []sandbox.NetworkInfo{{Name: "db", ID: id("a")}}, + }) + _, posture := d.childPosture() + if posture == nil || !reflect.DeepEqual(posture.NetworkIDs, []string{id("a")}) { + t.Fatalf("parent must stamp the resolved network IDs, got %+v", posture) + } + + resolved := config.ResolvedConfig{SandboxNetworks: []string{"db"}} + applyTaskSandboxPosture(&resolved, &taskSandboxPosture{ + Networks: []string{"db"}, NetworkIDs: []string{id("a"), "not-an-id", strings.ToUpper(id("b"))}, + }) + cfg := sandboxConfigFromResolved(resolved) + if !reflect.DeepEqual(cfg.AllowedNetworkIDs, []string{id("a")}) { + t.Fatalf("child must keep only valid stamped IDs, got %v", cfg.AllowedNetworkIDs) + } + + cfg.ResolvedNetworks = []sandbox.NetworkInfo{{Name: "db", ID: id("a")}} + if err := checkAllowedNetworkIDs(cfg); err != nil { + t.Fatalf("the parent's network must be accepted: %v", err) + } + cfg.ResolvedNetworks = []sandbox.NetworkInfo{{Name: "db", ID: id("c")}} + err := checkAllowedNetworkIDs(cfg) + if err == nil || !errors.Is(err, errSandboxConfig) || !strings.Contains(err.Error(), "db") { + t.Fatalf("a network recreated under the parent's name must be refused, got %v", err) + } + // No stamped IDs: nothing to enforce (the child of an unsandboxed or + // network-less parent attaches no networks anyway). + if err := checkAllowedNetworkIDs(sandboxConfig{ResolvedNetworks: cfg.ResolvedNetworks}); err != nil { + t.Fatalf("no stamped IDs must not refuse: %v", err) + } +} + +func TestRED_SetupSandbox_ChecksStampedNetworkIDs(t *testing.T) { + src := mustReadFile(t, "main.go") + i := strings.Index(src, "func setupSandbox(") + j := strings.Index(src[i:], "sandbox.BuildRunArgsChecked(") + if i < 0 || j < 0 || !strings.Contains(src[i:i+j], "checkAllowedNetworkIDs(") { + t.Fatal("setupSandbox must check stamped network IDs before creating the container") + } +} + +func mustReadFile(t *testing.T, name string) string { + t.Helper() + data, err := os.ReadFile(name) + if err != nil { + t.Fatal(err) + } + return string(data) +} diff --git a/cmd/odek/red_project_network_grant_test.go b/cmd/odek/red_project_network_grant_test.go index 50d94862..9a537b41 100644 --- a/cmd/odek/red_project_network_grant_test.go +++ b/cmd/odek/red_project_network_grant_test.go @@ -47,14 +47,15 @@ func (m *mutableComposeDocker) install(t *testing.T) { t.Cleanup(func() { sandbox.DockerRunner = old }) } -// setupNets resolves networks the way setupSandbox does for cfg. -func setupNets(t *testing.T, cfg sandboxConfig) []sandbox.NetworkInfo { +// setupNets resolves networks the way setupSandbox does for cfg and returns +// the config setupSandbox checks. +func setupNets(t *testing.T, cfg sandboxConfig) sandboxConfig { t.Helper() got, err := resolveSandboxNetworks(cfg, io.Discard) if err != nil { t.Fatal(err) } - return got.ResolvedNetworks + return got } func composeOverride(project string) config.ResolvedConfig { @@ -70,7 +71,7 @@ func TestRED_ProjectNetworkGrant_NetworkJoiningAfterApprovalIsRefused(t *testing if err := approveProjectSandboxWithTTY(composeOverride("app"), strings.NewReader("y\n"), io.Discard, true); err != nil { t.Fatal(err) } - cfg := sandboxConfig{ComposeProject: "app"} + cfg := sandboxConfig{ComposeProject: "app", ProjectComposeProject: "app"} if err := checkProjectNetworkGrant(setupNets(t, cfg)); err != nil { t.Fatalf("the approved networks must be accepted at setup: %v", err) @@ -96,7 +97,7 @@ func TestRED_ProjectNetworkGrant_RecreatedNetworkIsRefused(t *testing.T) { t.Fatal(err) } d.nets["shared"] = fakeNet{id: id("d"), internal: false} - if err := checkProjectNetworkGrant(setupNets(t, sandboxConfig{Networks: []string{"shared"}})); err == nil { + if err := checkProjectNetworkGrant(setupNets(t, sandboxConfig{Networks: []string{"shared"}, ProjectNetworks: []string{"shared"}})); err == nil { t.Fatal("a network recreated under the approved name must be refused") } } @@ -117,7 +118,7 @@ func TestRED_ProjectNetworkGrant_PersistedTrustIsCheckedToo(t *testing.T) { } // …and its setup is still held to the networks that trust covered. d.nets["app_admin"] = fakeNet{id: id("c"), compose: "app"} - if err := checkProjectNetworkGrant(setupNets(t, sandboxConfig{ComposeProject: "app"})); err == nil { + if err := checkProjectNetworkGrant(setupNets(t, sandboxConfig{ComposeProject: "app", ProjectComposeProject: "app"})); err == nil { t.Fatal("setup after a persisted approval must still refuse an unapproved network") } } @@ -136,7 +137,7 @@ func TestRED_ProjectNetworkGrant_OperatorNetworksAndEnvApprovalUnaffected(t *tes if err := approveProjectSandboxWithTTY(netOverride("opnet"), strings.NewReader(""), io.Discard, false); err != nil { t.Fatal(err) } - if err := checkProjectNetworkGrant(setupNets(t, sandboxConfig{Networks: []string{"opnet"}})); err != nil { + if err := checkProjectNetworkGrant(setupNets(t, sandboxConfig{Networks: []string{"opnet"}, ProjectNetworks: []string{"opnet"}})); err != nil { t.Fatalf("env approval must cover setup: %v", err) } } diff --git a/cmd/odek/redres_2026_10_14_runtime_ops_test.go b/cmd/odek/redres_2026_10_14_runtime_ops_test.go index 392c221e..a0072b80 100644 --- a/cmd/odek/redres_2026_10_14_runtime_ops_test.go +++ b/cmd/odek/redres_2026_10_14_runtime_ops_test.go @@ -64,7 +64,10 @@ func TestRED_RtOps_KeyFDStrictParse(t *testing.T) { } _, _ = w.WriteString("secret-key\n") w.Close() - t.Setenv(keyFDEnvVar, fmt.Sprintf("%d", int(r.Fd()))) + defer r.Close() + // The reader closes the descriptor it is handed, so it gets a dup: r + // keeps its own, and its finalizer never closes a reused number. + t.Setenv(keyFDEnvVar, dupFD(t, r)) if got := readKeyFromInheritedFD(); got != "secret-key" { t.Errorf("valid fd rejected: %q", got) } diff --git a/cmd/odek/sandbox_config_error.go b/cmd/odek/sandbox_config_error.go index 57e0c912..a1b8f630 100644 --- a/cmd/odek/sandbox_config_error.go +++ b/cmd/odek/sandbox_config_error.go @@ -76,9 +76,13 @@ func sandboxConfigFromResolved(resolved config.ResolvedConfig) sandboxConfig { PidsLimit: resolved.SandboxPidsLimit, Networks: resolved.SandboxNetworks, ComposeProject: resolved.SandboxComposeProject, - User: resolved.SandboxUser, - Env: resolved.SandboxEnv, - Volumes: resolved.SandboxVolumes, + // The project's own request, which the approval gate covers. + ProjectNetworks: projectNetworkRequest(resolved.ProjectSandboxOverride), + ProjectComposeProject: projectComposeRequest(resolved.ProjectSandboxOverride), + AllowedNetworkIDs: resolved.SandboxAllowedNetworkIDs, + User: resolved.SandboxUser, + Env: resolved.SandboxEnv, + Volumes: resolved.SandboxVolumes, } } @@ -134,3 +138,21 @@ func ensureSandboxImage(image string) error { } return nil } + +// projectNetworkRequest returns the network names the project config itself +// requested. +func projectNetworkRequest(o config.ProjectSandboxOverride) []string { + if !o.HasNetworks { + return nil + } + return o.Networks +} + +// projectComposeRequest returns the Compose project the project config +// itself requested. +func projectComposeRequest(o config.ProjectSandboxOverride) string { + if !o.HasComposeProject { + return "" + } + return o.ComposeProject +} diff --git a/cmd/odek/subagent_e2e_test.go b/cmd/odek/subagent_e2e_test.go index dc2e266f..d49aa855 100644 --- a/cmd/odek/subagent_e2e_test.go +++ b/cmd/odek/subagent_e2e_test.go @@ -72,6 +72,10 @@ func TestMain(m *testing.M) { // (ODEK_PI_BENCH) borrows the operator's provider credentials from the // real ~/.odek. Nothing else reads it, and the hermetic default stands. operatorRealHome, _ = os.UserHomeDir() + // The docker CLI finds a Docker Desktop daemon through the context under + // the real ~/.docker, which the fixture HOME below hides; every + // Docker-gated test would then skip as "Docker daemon unavailable". + pinDockerHostForTests() if os.Getenv("ODEK_TEST_KEEP_REAL_HOME") != "1" && os.Getenv("ODEK_TEST_HOME") == "" { fixture, err := os.MkdirTemp("", "odek-test-home-*") if err != nil { diff --git a/cmd/odek/subagent_posture.go b/cmd/odek/subagent_posture.go index 1f50db06..b24e7896 100644 --- a/cmd/odek/subagent_posture.go +++ b/cmd/odek/subagent_posture.go @@ -70,6 +70,7 @@ type taskSandboxPosture struct { PidsLimit string `json:"pids_limit,omitempty"` Networks []string `json:"networks,omitempty"` ComposeProject string `json:"compose_project,omitempty"` + NetworkIDs []string `json:"network_ids,omitempty"` User string `json:"user,omitempty"` // ApprovalKeys are the project-sandbox approval keys the parent was // granted this session (project directory + content hash bound). @@ -78,6 +79,9 @@ type taskSandboxPosture struct { var approvalKeyPattern = regexp.MustCompile(`^[0-9a-f]{64}$`) +// networkIDPattern matches a full Docker network ID. +var networkIDPattern = regexp.MustCompile(`^[0-9a-f]{64}$`) + const maxTaskApprovalKeys = 16 // recordChildToolFilter stamps the parent's configured tool filter onto every @@ -111,9 +115,14 @@ func recordChildSandbox(tools []odek.Tool, cfg sandboxConfig) { for _, t := range tools { if d, ok := t.(*delegateTasksTool); ok { d.sessMu.Lock() + var ids []string + for _, n := range cfg.ResolvedNetworks { + ids = append(ids, n.ID) + } d.childSandbox = &taskSandboxPosture{ Readonly: cfg.Readonly, Image: cfg.Image, Network: cfg.Network, Memory: cfg.Memory, CPUs: cfg.CPUs, PidsLimit: cfg.PidsLimit, Networks: cfg.Networks, ComposeProject: cfg.ComposeProject, User: cfg.User, + NetworkIDs: ids, } d.sessMu.Unlock() } @@ -176,6 +185,15 @@ func applyTaskSandboxPosture(resolved *config.ResolvedConfig, p *taskSandboxPost } resolved.SandboxNetworks = nets resolved.SandboxComposeProject = "" + // The stamped IDs pin those names to the networks the parent attached, + // so a network recreated under the same name is refused at setup. + var ids []string + for _, id := range p.NetworkIDs { + if len(ids) < sandbox.MaxNetworks && networkIDPattern.MatchString(id) { + ids = append(ids, id) + } + } + resolved.SandboxAllowedNetworkIDs = ids if p.User != "" { resolved.SandboxUser = p.User } diff --git a/docs/SANDBOXING.md b/docs/SANDBOXING.md index 82201bc7..56d07a22 100644 --- a/docs/SANDBOXING.md +++ b/docs/SANDBOXING.md @@ -243,7 +243,7 @@ Service DNS names on those networks: postgres, redis - localhost is the container itself, not the host or those services; reach them by name. ``` -Network and service names come from Docker and, through a Compose file, can be chosen by the project, so they are listed inside the same nonce'd untrusted-content wrapper as tool output: a name that passes the charset check but reads like an instruction is still only data. Sub-agents only narrow: a child attaches a subset of the network names its parent resolved and never attaches a network its Compose project gained after the parent resolved it. +Network and service names come from Docker and, through a Compose file, can be chosen by the project, so they are listed inside the same nonce'd untrusted-content wrapper as tool output: a name that passes the charset check but reads like an instruction is still only data. Sub-agents only narrow: a child attaches a subset of the networks its parent resolved, pinned to the IDs the parent attached, so it never attaches a network its Compose project gained later or one recreated under the same name. ## File injection diff --git a/docs/SECURITY.md b/docs/SECURITY.md index cfaadc23..7211bcd1 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -500,7 +500,7 @@ These fields can only be set from operator-controlled sources: `~/.odek/config.j - `schedules.dangerous` / `schedules.max_concurrent` / `schedules.catchup` / `schedules.allow_telegram_management` / `schedules.telegram_admin_*` / `maintenance` / `trusted_proxies` / `tools.enabled` — schedule policy (including Telegram admin lists and concurrency), storage-maintenance policy, rate-limit proxy trust, and tool enablement are operator decisions. A project `schedules` overlay is merged field-by-field so a timezone-only project file cannot wipe global admin lists. - `sandbox: false` / `sandbox_readonly: false` — a project cannot disable the sandbox or its read-only enforcement. -**Sandbox knobs are approval-gated.** Project sandbox settings (`sandbox_env`, `sandbox_image`, `sandbox_network`, `sandbox_networks`, `sandbox_compose_project`, `sandbox_volumes`) are gated behind explicit operator approval rather than silently rejected: interactive TTY prompt (`y` = once, `t` = trust this project, `N` = deny), persistent per-project approvals in `~/.odek/project_sandbox_approvals.json`, or `ODEK_APPROVE_PROJECT_SANDBOX=1` for CI/non-interactive use. Non-TTY runs without the bypass fail closed. A warning is shown when `sandbox_env` values contain `${VAR}` or bare `$VAR` host-environment interpolation, listing each interpolating key with its template (never the expanded value), so a malicious repo cannot silently exfiltrate host secrets, pull an attacker-controlled image, or widen the container's network access. Project-requested networks are shown with their full ID, `Internal` flag and Compose project, the approval key binds those IDs, and `setupSandbox` checks the networks it resolves again at container creation against the approved IDs: a network that joined the Compose project, or was recreated under an approved name, after the prompt is refused with a configuration error. +**Sandbox knobs are approval-gated.** Project sandbox settings (`sandbox_env`, `sandbox_image`, `sandbox_network`, `sandbox_networks`, `sandbox_compose_project`, `sandbox_volumes`) are gated behind explicit operator approval rather than silently rejected: interactive TTY prompt (`y` = once, `t` = trust this project, `N` = deny), persistent per-project approvals in `~/.odek/project_sandbox_approvals.json`, or `ODEK_APPROVE_PROJECT_SANDBOX=1` for CI/non-interactive use. Non-TTY runs without the bypass fail closed. A warning is shown when `sandbox_env` values contain `${VAR}` or bare `$VAR` host-environment interpolation, listing each interpolating key with its template (never the expanded value), so a malicious repo cannot silently exfiltrate host secrets, pull an attacker-controlled image, or widen the container's network access. Project-requested networks are shown with their full ID, `Internal` flag and Compose project, the approval key binds those IDs, and `setupSandbox` checks the networks it resolves again at container creation against the approved IDs: a network that joined the Compose project, or was recreated under an approved name, after the prompt is refused with a configuration error, and so are project-requested networks for which no approval was recorded in the process. Sub-agents receive the IDs of the networks their parent attached and refuse a network that no longer carries one of them. **Execution budgets use a clamp merge.** The `limits` config section (`internal/budget` + `clampProjectLimits` in `internal/config/loader.go`) uses a clamp instead of the usual overlay: the global `~/.odek/config.json` may set any execution budget, but the untrusted project `./odek.json` may only *lower* one — raise attempts are clamped to the global value with a stderr warning, and zeroing/omitting a field re-inherits the global limit — so a checked-in config can never disable the operator's runtime/token/cost caps. Project-set per-million prices are rejected outright because a lower price would silently weaken cost enforcement. CLI flags are layer-4 operator intent and may set limits explicitly in either direction. Enforcement is fail-stop: on exhaustion the loop emits `budget_exceeded`, persists the latest safe session state, and returns a typed `budget.Error` (CLI exit code 4). diff --git a/internal/config/loader.go b/internal/config/loader.go index f46c16e3..7f24808a 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -1014,6 +1014,11 @@ type ResolvedConfig struct { // --sandbox-compose. SandboxComposeProject string + // SandboxAllowedNetworkIDs is set only for sub-agents, from the parent's + // task file: the IDs of the networks the parent attached. It is never + // read from a config file or the environment. + SandboxAllowedNetworkIDs []string + // SandboxUser sets the container user (e.g. "1000:1000" or "node"). // Empty string means root (default Docker behavior). // Config: sandbox_user, ODEK_SANDBOX_USER, --sandbox-user. diff --git a/internal/danger/classifier.go b/internal/danger/classifier.go index df476487..66e29490 100644 --- a/internal/danger/classifier.go +++ b/internal/danger/classifier.go @@ -407,7 +407,14 @@ func pathWithin(abs, dir string) bool { func accountHomes(abs string) []string { var homes []string if home, _ := os.UserHomeDir(); home != "" { + home = filepath.Clean(home) homes = append(homes, home) + // abs has its /private prefix stripped, so a $HOME spelled through + // /private (/private/var/..., /private/tmp/...) is compared in the + // same form. + if strings.HasPrefix(home, "/private/") { + homes = append(homes, strings.TrimPrefix(home, "/private")) + } } if physical := currentHomeDirs(); len(physical) > 1 { homes = append(homes, physical[1:]...) diff --git a/internal/danger/red_private_home_test.go b/internal/danger/red_private_home_test.go new file mode 100644 index 00000000..20a447fa --- /dev/null +++ b/internal/danger/red_private_home_test.go @@ -0,0 +1,24 @@ +package danger + +import "testing" + +// macOS spells /var and /tmp as symlinks into /private, and the classifier +// compares paths with the /private prefix stripped. A $HOME spelled through +// /private must still protect its own trust anchors and credential files. +func TestRED_ClassifyPath_HomeSpelledThroughPrivate(t *testing.T) { + t.Setenv("HOME", "/private/var/folders/xy/abc/T/odek-home") + for _, p := range []string{ + "/private/var/folders/xy/abc/T/odek-home/.odek/config.json", + "/var/folders/xy/abc/T/odek-home/.odek/secrets.env", + "/private/var/folders/xy/abc/T/odek-home/.ssh/id_ed25519", + "/private/var/folders/xy/abc/T/odek-home/.zshrc", + } { + if got := ClassifyPath(p); got != SystemWrite { + t.Errorf("ClassifyPath(%q) = %s, want system_write", p, got) + } + } + // An ordinary file in that home stays a local write. + if got := ClassifyPath("/private/var/folders/xy/abc/T/odek-home/notes.txt"); got != LocalWrite { + t.Errorf("ordinary home file = %s, want local_write", got) + } +} diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index 9187de4d..1ff53a25 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -78,6 +78,14 @@ type Config struct { // ComposeProject names a Docker Compose project whose networks are // attached (resolved by label, never by guessing "_default"). ComposeProject string + // ProjectNetworks and ProjectComposeProject are the networks the project + // config itself requested; they need operator approval. + ProjectNetworks []string + ProjectComposeProject string + // AllowedNetworkIDs, when set, limits attachment to these network IDs: a + // sub-agent may attach only networks that still carry the IDs its + // parent resolved. + AllowedNetworkIDs []string } // ResolveImage determines the Docker image for a sandbox container.