fix(sandbox): keep the network grant current; check fd handoffs structurally

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")
@@ -420,18 +425,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
 }
 

cmd/odek/red_fd_handoff_test.go

@@ -1,9 +1,11 @@
 package main
 
 import (
-	"os"
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"testing"
 )
@@ -13,22 +15,136 @@ 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\(\)`)
 	for _, f := range files {
-		data, err := os.ReadFile(f)
-		if err != nil {
-			t.Fatal(err)
+		for _, v := range fdHandoffViolations(t, f, nil) {
+			t.Error(v)
 		}
-		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)
+	}
+}
+
+// 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
+	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
 }

cmd/odek/red_stale_network_grant_test.go

@@ -0,0 +1,84 @@
+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")
+	}
+}
+
+// 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")
+	}
+}

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()
 	}