From d35536d5559d2731dbbbbb9a5ca1627085731ad4 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:15:52 +0100 Subject: [PATCH 1/4] fix(sandbox): drop the network grant when a later approval requests no networks recordProjectNetworkGrant returned early for an approved override without network fields, leaving an earlier grant for the same directory in place. The record now always matches the latest approval. Co-Authored-By: Claude Opus 5.5 --- cmd/odek/project_sandbox_approval.go | 10 +++--- cmd/odek/red_stale_network_grant_test.go | 42 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 cmd/odek/red_stale_network_grant_test.go diff --git a/cmd/odek/project_sandbox_approval.go b/cmd/odek/project_sandbox_approval.go index b93eca69..626fc5e5 100644 --- a/cmd/odek/project_sandbox_approval.go +++ b/cmd/odek/project_sandbox_approval.go @@ -420,18 +420,20 @@ var ( projectNetworkGrants = map[string]map[string]bool{} ) -// recordProjectNetworkGrant remembers the networks behind an approval of a -// project override that requests networks. +// recordProjectNetworkGrant remembers the networks behind the latest +// approval for projectDir. An approved override that requests no networks +// drops any earlier grant, so the record always matches the latest approval. func recordProjectNetworkGrant(projectDir string, o config.ProjectSandboxOverride, nets []sandbox.NetworkInfo) { + projectNetworkGrantsMu.Lock() + defer projectNetworkGrantsMu.Unlock() if !o.HasNetworks && !o.HasComposeProject { + delete(projectNetworkGrants, projectDir) return } ids := map[string]bool{} for _, n := range nets { ids[n.ID] = true } - projectNetworkGrantsMu.Lock() - defer projectNetworkGrantsMu.Unlock() projectNetworkGrants[projectDir] = ids } diff --git a/cmd/odek/red_stale_network_grant_test.go b/cmd/odek/red_stale_network_grant_test.go new file mode 100644 index 00000000..3f27f930 --- /dev/null +++ b/cmd/odek/red_stale_network_grant_test.go @@ -0,0 +1,42 @@ +package main + +import ( + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/BackendStack21/odek/internal/config" +) + +// The recorded network grant always reflects the latest approval for the +// directory: an approval whose override requests no networks drops an +// earlier network grant instead of leaving it in place. +func TestRED_ProjectNetworkGrant_LaterApprovalWithoutNetworksClearsIt(t *testing.T) { + approvalEnv(t) + resetProjectNetworkGrants() + d := &mutableComposeDocker{nets: map[string]fakeNet{"shared": {id: id("a"), internal: true}}} + d.install(t) + if err := approveProjectSandboxWithTTY(netOverride("shared"), strings.NewReader("y\n"), io.Discard, true); err != nil { + t.Fatal(err) + } + wd, _ := os.Getwd() + dir, _ := filepath.Abs(wd) + projectNetworkGrantsMu.Lock() + _, ok := projectNetworkGrants[dir] + projectNetworkGrantsMu.Unlock() + if !ok { + t.Fatal("the first approval must record a network grant") + } + memOnly := config.ResolvedConfig{ProjectSandboxOverride: config.ProjectSandboxOverride{HasMemory: true, Memory: "512m"}} + if err := approveProjectSandboxWithTTY(memOnly, strings.NewReader("y\n"), io.Discard, true); err != nil { + t.Fatal(err) + } + projectNetworkGrantsMu.Lock() + _, stale := projectNetworkGrants[dir] + projectNetworkGrantsMu.Unlock() + if stale { + t.Error("an approval without network fields must drop the earlier network grant") + } +} From 297b818ae2229f3498e7092569a7f47a6c24602d Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:16:43 +0100 Subject: [PATCH 2/4] test(cmd): check fd handoffs structurally, not by matching one line The fd-handoff guard only matched .Fd() on the same line as t.Setenv, so a descriptor read into a variable first slipped past. Parse the test files instead: in any test that sets the key or frame fd env var, every .Fd() call must sit inside syscall.Dup or dupFD. The malformed-value loop of the key-fd parse test now hands over a dup as well and closes it when the reader rejects the value. Co-Authored-By: Claude Opus 5.5 --- cmd/odek/red_fd_handoff_test.go | 82 +++++++++++++++++-- .../redres_2026_10_14_runtime_ops_test.go | 11 ++- 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/cmd/odek/red_fd_handoff_test.go b/cmd/odek/red_fd_handoff_test.go index 86b94370..8c63a4de 100644 --- a/cmd/odek/red_fd_handoff_test.go +++ b/cmd/odek/red_fd_handoff_test.go @@ -1,10 +1,10 @@ package main import ( - "os" + "go/ast" + "go/parser" + "go/token" "path/filepath" - "regexp" - "strings" "testing" ) @@ -13,22 +13,86 @@ import ( // 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. +// fails with "unlinkat: bad file descriptor". So in every test function that +// sets one of those env vars, each .Fd() call must sit inside syscall.Dup or +// dupFD: the reader is only ever handed a descriptor of its own, however the +// number reaches the env var. 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\(\)`) + fset := token.NewFileSet() for _, f := range files { - data, err := os.ReadFile(f) + file, err := parser.ParseFile(fset, f, nil, 0) 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) + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Body == nil || fn.Name.Name == "dupFD" || !setsFDEnvVar(fn.Body) { + continue + } + for _, pos := range rawFdCalls(fn.Body) { + t.Errorf("%s: %s reads an owned descriptor with .Fd() outside syscall.Dup/dupFD while handing fds to the key/frame reader", + fset.Position(pos), fn.Name.Name) + } + } + } +} + +// setsFDEnvVar reports whether body calls t.Setenv(keyFDEnvVar|frameFDEnvVar, …). +func setsFDEnvVar(body *ast.BlockStmt) bool { + found := false + ast.Inspect(body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || len(call.Args) == 0 { + return true + } + if sel, ok := call.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "Setenv" { + if id, ok := call.Args[0].(*ast.Ident); ok && (id.Name == "keyFDEnvVar" || id.Name == "frameFDEnvVar") { + found = true } } + return !found + }) + return found +} + +// rawFdCalls returns the positions of .Fd() calls in body that are not +// nested inside a syscall.Dup(…) or dupFD(…) call. +func rawFdCalls(body *ast.BlockStmt) []token.Pos { + var out []token.Pos + var walk func(n ast.Node, dup bool) + walk = func(n ast.Node, dup bool) { + ast.Inspect(n, func(c ast.Node) bool { + call, ok := c.(*ast.CallExpr) + if !ok { + return true + } + if isDupCall(call) && !dup { + for _, a := range call.Args { + walk(a, true) + } + return false + } + if sel, ok := call.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "Fd" && len(call.Args) == 0 && !dup { + out = append(out, call.Pos()) + } + return true + }) + } + walk(body, false) + return out +} + +func isDupCall(call *ast.CallExpr) bool { + switch fun := call.Fun.(type) { + case *ast.Ident: + return fun.Name == "dupFD" + case *ast.SelectorExpr: + pkg, ok := fun.X.(*ast.Ident) + return ok && pkg.Name == "syscall" && fun.Sel.Name == "Dup" } + return false } 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 a0072b80..70b172eb 100644 --- a/cmd/odek/redres_2026_10_14_runtime_ops_test.go +++ b/cmd/odek/redres_2026_10_14_runtime_ops_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strconv" "strings" "testing" "time" @@ -36,8 +37,11 @@ func TestRED_RtOps_KeyFDStrictParse(t *testing.T) { t.Fatal(err) } w.Close() - fd := int(r.Fd()) - val := fmt.Sprintf("%d", fd) + // A dup of r: if a malformed value were ever accepted, the reader + // would close its own descriptor, never one r still owns. + dup := dupFD(t, r) + fd, _ := strconv.Atoi(dup) + val := dup if bad == "3x" || bad == "03x" { val += "x" } else if bad == "+3" { @@ -54,6 +58,9 @@ func TestRED_RtOps_KeyFDStrictParse(t *testing.T) { t.Setenv(keyFDEnvVar, val) if got := readKeyFromInheritedFD(); got != "" { t.Errorf("env %q accepted, key %q", val, got) + } else { + // Rejected before it was opened: the dup is still ours to close. + os.NewFile(uintptr(fd), "rejected-key-fd").Close() } r.Close() } From 0a665402d11d9a2c9afff9e96341f00c25b3d43c Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:25:28 +0100 Subject: [PATCH 3/4] fix(sandbox): keep the network grant in step with every successful approval A Dockerfile-only approval, or a run with nothing to approve, left an earlier network grant for the directory in place. Every approval path now records the grant for its own override, which drops it when the project requests no networks. Co-Authored-By: Claude Opus 5.5 --- cmd/odek/project_sandbox_approval.go | 13 +++++--- cmd/odek/red_stale_network_grant_test.go | 42 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/cmd/odek/project_sandbox_approval.go b/cmd/odek/project_sandbox_approval.go index 626fc5e5..3d825d2c 100644 --- a/cmd/odek/project_sandbox_approval.go +++ b/cmd/odek/project_sandbox_approval.go @@ -62,6 +62,13 @@ func approveProjectSandboxWithTTY(resolved config.ResolvedConfig, stdin io.Reade o.HasUser || o.HasMemory || o.HasCPUs || o.HasPidsLimit || o.HasNetworks || o.HasComposeProject dfHash, dfRequired := dockerfileBuildRequirement(resolved) if !hasOverride && !dfRequired { + // Nothing to approve: the project requests no networks, so no + // earlier network grant for this directory may linger. + if wd, err := os.Getwd(); err == nil { + if dir, err := filepath.Abs(wd); err == nil { + recordProjectNetworkGrant(dir, o, nil) + } + } return nil } @@ -185,8 +192,8 @@ func approveProjectSandboxWithTTY(resolved config.ResolvedConfig, stdin io.Reade } if hasOverride { recordOverrideGrant(projectDir, overrideKey) - recordProjectNetworkGrant(projectDir, o, nets) } + recordProjectNetworkGrant(projectDir, o, nets) return nil case "t", "trust": if hasOverride { @@ -199,9 +206,7 @@ func approveProjectSandboxWithTTY(resolved config.ResolvedConfig, stdin io.Reade if err := saveProjectSandboxApprovals(approved); err != nil { return fmt.Errorf("project sandbox approval: save approvals: %w", err) } - if hasOverride { - recordProjectNetworkGrant(projectDir, o, nets) - } + recordProjectNetworkGrant(projectDir, o, nets) return nil default: return fmt.Errorf("project sandbox config was not approved") diff --git a/cmd/odek/red_stale_network_grant_test.go b/cmd/odek/red_stale_network_grant_test.go index 3f27f930..6e28caab 100644 --- a/cmd/odek/red_stale_network_grant_test.go +++ b/cmd/odek/red_stale_network_grant_test.go @@ -40,3 +40,45 @@ func TestRED_ProjectNetworkGrant_LaterApprovalWithoutNetworksClearsIt(t *testing t.Error("an approval without network fields must drop the earlier network grant") } } + +// The same holds for approvals that carry no override at all: a Dockerfile.odek +// build approval, or a run with nothing to approve. +func TestRED_ProjectNetworkGrant_ApprovalWithoutOverrideClearsIt(t *testing.T) { + approvalEnv(t) + d := &mutableComposeDocker{nets: map[string]fakeNet{"shared": {id: id("a"), internal: true}}} + d.install(t) + wd, _ := os.Getwd() + dir, _ := filepath.Abs(wd) + granted := func() bool { + projectNetworkGrantsMu.Lock() + defer projectNetworkGrantsMu.Unlock() + _, ok := projectNetworkGrants[dir] + return ok + } + grant := func() { + t.Helper() + resetProjectNetworkGrants() + if err := approveProjectSandboxWithTTY(netOverride("shared"), strings.NewReader("y\n"), io.Discard, true); err != nil || !granted() { + t.Fatalf("setup: network approval must record a grant (err=%v)", err) + } + } + + grant() + if err := approveProjectSandboxWithTTY(config.ResolvedConfig{}, strings.NewReader(""), io.Discard, false); err != nil { + t.Fatal(err) + } + if granted() { + t.Error("a run with nothing to approve must drop the earlier network grant") + } + + grant() + if err := os.WriteFile(filepath.Join(dir, "Dockerfile.odek"), []byte("FROM alpine:3\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := approveProjectSandboxWithTTY(config.ResolvedConfig{Sandbox: true}, strings.NewReader("y\n"), io.Discard, true); err != nil { + t.Fatal(err) + } + if granted() { + t.Error("a Dockerfile-only approval must drop the earlier network grant") + } +} From b197d26ea153f9f3b6e86b392ef75c5048d3664a Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:25:41 +0100 Subject: [PATCH 4/4] test(cmd): self-test the fd-handoff checker against fixtures Feed the checker raw handoffs (direct, through a variable, inside a closure) and dup handoffs, so it cannot quietly stop flagging anything. Co-Authored-By: Claude Opus 5.5 --- cmd/odek/red_fd_handoff_test.go | 78 +++++++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 13 deletions(-) diff --git a/cmd/odek/red_fd_handoff_test.go b/cmd/odek/red_fd_handoff_test.go index 8c63a4de..23840e34 100644 --- a/cmd/odek/red_fd_handoff_test.go +++ b/cmd/odek/red_fd_handoff_test.go @@ -1,10 +1,12 @@ package main import ( + "fmt" "go/ast" "go/parser" "go/token" "path/filepath" + "strings" "testing" ) @@ -22,25 +24,75 @@ func TestRED_FDHandoffTestsPassADuplicatedDescriptor(t *testing.T) { if err != nil { t.Fatal(err) } - fset := token.NewFileSet() for _, f := range files { - file, err := parser.ParseFile(fset, f, nil, 0) - if err != nil { - t.Fatal(err) + for _, v := range fdHandoffViolations(t, f, nil) { + t.Error(v) } - for _, decl := range file.Decls { - fn, ok := decl.(*ast.FuncDecl) - if !ok || fn.Body == nil || fn.Name.Name == "dupFD" || !setsFDEnvVar(fn.Body) { - continue - } - for _, pos := range rawFdCalls(fn.Body) { - t.Errorf("%s: %s reads an owned descriptor with .Fd() outside syscall.Dup/dupFD while handing fds to the key/frame reader", - fset.Position(pos), fn.Name.Name) - } + } +} + +// The checker itself must flag a raw handoff, however the number reaches +// the env var, and accept a dup. +func TestRED_FDHandoffChecker_FlagsRawAndAcceptsDup(t *testing.T) { + const src = `package main + +func rawDirect(t *testing.T, r *os.File) { t.Setenv(keyFDEnvVar, fmt.Sprint(r.Fd())) } + +func rawViaVariable(t *testing.T, r *os.File) { + fd := int(r.Fd()) + t.Setenv(frameFDEnvVar, strconv.Itoa(fd)) +} + +func rawInClosure(t *testing.T, r *os.File) { + f := func() string { return fmt.Sprint(r.Fd()) } + os.Setenv(keyFDEnvVar, f()) +} + +func viaDupFD(t *testing.T, r *os.File) { t.Setenv(keyFDEnvVar, dupFD(t, r)) } + +func viaSyscallDup(t *testing.T, r *os.File) { + fd, _ := syscall.Dup(int(r.Fd())) + t.Setenv(keyFDEnvVar, strconv.Itoa(fd)) +} + +func unrelated(r *os.File) uintptr { return r.Fd() } +` + got := fdHandoffViolations(t, "fixture_test.go", src) + want := []string{"rawDirect", "rawViaVariable", "rawInClosure"} + if len(got) != len(want) { + t.Fatalf("violations = %q, want one each for %v", got, want) + } + for i, name := range want { + if !strings.Contains(got[i], " "+name+" ") { + t.Errorf("violation %d = %q, want it to name %s", i, got[i], name) } } } +// fdHandoffViolations parses one test file (or src when non-nil) and +// describes every .Fd() call outside syscall.Dup/dupFD in a function that +// sets the key or frame fd env var. +func fdHandoffViolations(t *testing.T, name string, src any) []string { + t.Helper() + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, name, src, 0) + if err != nil { + t.Fatal(err) + } + var out []string + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Body == nil || fn.Name.Name == "dupFD" || !setsFDEnvVar(fn.Body) { + continue + } + for _, pos := range rawFdCalls(fn.Body) { + out = append(out, fmt.Sprintf("%s: %s reads an owned descriptor with .Fd() outside syscall.Dup/dupFD while handing fds to the key/frame reader", + fset.Position(pos), fn.Name.Name)) + } + } + return out +} + // setsFDEnvVar reports whether body calls t.Setenv(keyFDEnvVar|frameFDEnvVar, …). func setsFDEnvVar(body *ast.BlockStmt) bool { found := false