From 5f2e2acf17f7e9b426d7268808d38ae817b07dab Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Sun, 25 Oct 2026 14:16:23 +0100 Subject: [PATCH 1/4] fix(danger): protect a home spelled through /private like any other home classifyPathLexical strips the /private prefix from the target path, but accountHomes compared it with $HOME as spelled. With HOME under /private/var or /private/tmp (macOS physical paths), writes to ~/.ssh, shell rc files and odek's trust anchors classified as local_write instead of system_write. Compare against the stripped spelling too. Co-Authored-By: Claude Opus 5.5 --- internal/danger/classifier.go | 7 +++++++ internal/danger/red_private_home_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 internal/danger/red_private_home_test.go 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) + } +} From c274a5dad54fb349382bb91e7919cfb2423fbbdc Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Sun, 25 Oct 2026 14:22:12 +0100 Subject: [PATCH 2/4] fix(sandbox): refuse unapproved project networks and pin sub-agent networks by ID The setup-time grant check returned early when no approval had been recorded in the process, so a surface that reached setupSandbox without the approval gate would attach project-requested networks unchecked. The sandbox config now carries the project's own network request, and setup refuses those networks unless an approval with their IDs was recorded (ODEK_APPROVE_PROJECT_SANDBOX=1 still approves). Sub-agents narrowed networks by name only, so a network recreated under the same name between the parent's and the child's start was attached under its new ID. The parent now stamps the IDs of the networks it attached into the task file, and the child refuses any network that no longer carries one of them. Co-Authored-By: Claude Opus 5.5 --- cmd/odek/main.go | 5 +- cmd/odek/project_sandbox_approval.go | 103 +++++++++++++-------- cmd/odek/red_network_residuals_test.go | 97 +++++++++++++++++++ cmd/odek/red_project_network_grant_test.go | 15 +-- cmd/odek/sandbox_config_error.go | 28 +++++- cmd/odek/subagent_posture.go | 18 ++++ docs/SANDBOXING.md | 2 +- docs/SECURITY.md | 2 +- internal/config/loader.go | 5 + internal/sandbox/sandbox.go | 8 ++ 10 files changed, 231 insertions(+), 52 deletions(-) create mode 100644 cmd/odek/red_network_residuals_test.go 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_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/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_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/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. From d9fca44593d5a6d5f4de339b85bf72c7cf784787 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Sun, 25 Oct 2026 14:22:53 +0100 Subject: [PATCH 3/4] test(cmd): reach Docker Desktop from the hermetic test HOME TestMain swaps HOME for a fixture directory, which hides the docker CLI context Docker Desktop relies on, so every Docker-gated test skipped as "Docker daemon unavailable" on macOS. Pin the active context's endpoint as DOCKER_HOST before the swap (an explicit DOCKER_HOST or DOCKER_CONTEXT is left alone). Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 9 ++++----- cmd/odek/dockerhost_test.go | 28 ++++++++++++++++++++++++++++ cmd/odek/subagent_e2e_test.go | 4 ++++ 3 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 cmd/odek/dockerhost_test.go 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/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 { From 0f08e48552122a389236c1d44b7fa7152c1e6069 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Sun, 25 Oct 2026 14:24:47 +0100 Subject: [PATCH 4/4] test(cmd): hand the key-fd reader a dup instead of an owned descriptor TestRED_RtOps_KeyFDStrictParse passed r.Fd() of a live pipe to readKeyFromInheritedFD, which closes the descriptor it is handed. r's finalizer later closed the same number again after the kernel had reused it, typically for the directory fd os.RemoveAll was walking in a parallel test, which then failed with "unlinkat: bad file descriptor". Pass a dup, close r explicitly, and pin the rule for every fd-handoff test. Co-Authored-By: Claude Opus 5.5 --- cmd/odek/red_fd_handoff_test.go | 34 +++++++++++++++++++ .../redres_2026_10_14_runtime_ops_test.go | 5 ++- 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 cmd/odek/red_fd_handoff_test.go 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/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) }