review: audit cleanups

.github/workflows/sandboxd.yml

@@ -1,5 +1,5 @@
 # Host-side Go: the sandboxd daemon, the SDK, and the in-process e2e module.
-# protocol/ is in the paths because the SDK's corpus test consumes the
+# protocol/ is in the paths because protocol/wire's corpus test consumes the
 # fixtures — a fixture change must re-prove the Go round-trip.
 name: sandboxd
 

docs/deploy.md

@@ -66,6 +66,12 @@ and send its entries again; a write that succeeds has rewritten its row. A
 `claims.json` that still holds a missing entry stops v0.1.24 from starting, and
 a row that holds one cannot be adopted by it.
 
+From v0.1.26, a static volume `tenants` name that does not match the
+[tenant API](sandboxd-api.md#tenants-v1tenants) syntax fails config load.
+v0.1.25 kept such a name, and it matched no tenant. Correct each one before
+starting the new binary. An empty list permits every tenant, so to keep a
+volume closed, remove its entry instead of emptying its list.
+
 ## Configuration
 
 sandboxd reads one JSON file (`-config`, default

docs/sandboxd-api.md

@@ -367,12 +367,10 @@ Sandboxd does not create, check, or delete backings. Keep them outside
 
 Static and runtime entries share one `tenants` rule. An omitted or empty list
 permits every authenticated scope. A nonempty list permits only claims owned by
-the listed tenants, and root-owned claims can use any entry. Runtime
-registration rejects tenant names that do not match the tenant API syntax.
-Static config preserves its supplied tenant lists. An invalid static name
-matches no tenant, so a nonempty list containing only invalid names permits no
-tenant. A valid name for a tenant that does not exist yet grants nothing until
-the tenant is created.
+the listed tenants, and root-owned claims can use any entry. A listed name must
+match the tenant API syntax: runtime registration rejects one that does not,
+and static config fails to load. A valid name for a tenant that does not exist
+yet grants nothing until the tenant is created.
 
 ## DELETE /v1/volumes/{name}
 

e2e/cmd/egresssmoke/main.go

@@ -1,5 +1,4 @@
-// egresssmoke checks guarded egress: an allowed origin gets the host-injected
-// credential, direct egress is blocked, and a disallowed host is denied.
+// egresssmoke checks guarded egress: an allowed origin gets the host-injected credential, direct egress is blocked, and a disallowed host is denied.
 package main
 
 import (

e2e/cmd/rpcbench/main.go

@@ -1,5 +1,4 @@
-// rpcbench compares dial-per-RPC against the SDK's kept connection on a live
-// node, interleaved sample by sample with the lead swapped.
+// rpcbench compares dial-per-RPC against the SDK's kept connection on a live node, interleaved sample by sample with the lead swapped.
 package main
 
 import (

mcp/server_test.go

@@ -268,7 +268,9 @@ func toolText(t *testing.T, resp map[string]any) string {
 	return fmt.Sprintf("%v", as[map[string]any](t, content[0])["text"])
 }
 
-func newTestServer(t *testing.T, extra func(http.ResponseWriter, *http.Request) bool) *server {
+type routeFunc func(http.ResponseWriter, *http.Request) bool
+
+func newTestServer(t *testing.T, extra routeFunc) *server {
 	t.Helper()
 	node := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch {
@@ -289,7 +291,7 @@ func newTestServer(t *testing.T, extra func(http.ResponseWriter, *http.Request)
 	return srv
 }
 
-func agentRoute(t *testing.T, reply func(req string) (frames string, drop bool)) func(http.ResponseWriter, *http.Request) bool {
+func agentRoute(t *testing.T, reply func(req string) (frames string, drop bool)) routeFunc {
 	t.Helper()
 	return func(w http.ResponseWriter, r *http.Request) bool {
 		if !strings.HasSuffix(r.URL.Path, "/agent") {

protocol/wire/frame.go

@@ -48,10 +48,11 @@ const (
 )
 
 var (
-	requestHead = `{"v":` + strconv.Itoa(ProtoVersion) + `,"op":"`
+	requestHead  = `{"v":` + strconv.Itoa(ProtoVersion) + `,"op":"`
+	responseHead = `{"type":"`
 
 	reqTagHead  = []byte(requestHead)
-	respTagHead = []byte(`{"type":"`)
+	respTagHead = []byte(responseHead)
 
 	requestDecoders = map[string]func([]byte) (Request, error){
 		"exec":           decodeReq[Exec],
@@ -651,7 +652,7 @@ func EncodeRequest(r Request) ([]byte, error) {
 
 // EncodeResponse renders {"type":...,fields} without a trailing newline.
 func EncodeResponse(r Response) ([]byte, error) {
-	return encodeTagged(`{"type":"`+r.RespType()+`"`, r)
+	return encodeTagged(responseHead+r.RespType()+`"`, r)
 }
 
 // DecodeRequest parses one frame into its op's concrete type.

sandboxd/config/config.go

@@ -910,6 +910,11 @@ func ValidateVolumeCatalog(volumes []VolumeSpec) error {
 		if !types.ValidDirectIO(volume.DirectIO) {
 			return fmt.Errorf("volume %q directio must be on, off, or auto, got %q", volume.Name, volume.DirectIO)
 		}
+		for _, tenant := range volume.Tenants {
+			if !types.NameRe.MatchString(tenant) {
+				return fmt.Errorf("volume %q tenant %q must match %s", volume.Name, tenant, types.NameRe)
+			}
+		}
 	}
 	return nil
 }

sandboxd/config/config_test.go

@@ -182,6 +182,7 @@ func TestLoadRejectsInvalid(t *testing.T) {
 		{"duplicate volume name", `{"pools":[],"volumes":[{"name":"data","path":"/srv/datasets/a.img"},{"name":"data","path":"/srv/datasets/b.img"}]}`, "duplicate volume"},
 		{"relative volume path", `{"pools":[],"volumes":[{"name":"data","path":"datasets/a.img"}]}`, "path must be absolute"},
 		{"bad volume directio", `{"pools":[],"volumes":[{"name":"data","path":"/srv/datasets/a.img","directio":"yes"}]}`, "directio must be"},
+		{"bad volume tenant", `{"pools":[],"volumes":[{"name":"data","path":"/srv/datasets/a.img","tenants":["has space"]}]}`, `tenant "has space" must match`},
 		{"relative volume root", `{"pools":[],"volume_roots":["srv/datasets"]}`, "volume_roots"},
 		{"empty volume root", `{"pools":[],"volume_roots":[""]}`, "volume_roots"},
 		{"reader without warm", `{"pools":[{"template":"rt:24.04","warm":0,"reader":true}]}`, "reader needs warm"},

sandboxd/outbound/tun.go

@@ -48,11 +48,11 @@ func (h *Host) RearmTun(ctx context.Context, sb *types.Sandbox) {
 	old := d.takeTun()
 	h.mu.Unlock()
 	h.dropTun(old)
-	v := h.o.View()
-	policy, ok := v.Resolve(sb)
 	if d == nil {
 		return
 	}
+	v := h.o.View()
+	policy, ok := v.Resolve(sb)
 	if !ok {
 		if sb.TunAddr.IsValid() {
 			h.tunDown(sb.ID, sb.VsockSocket)

sandboxd/pool/volume_acl_test.go

@@ -1,12 +1,8 @@
 package pool
 
 import (
-	"fmt"
-	"os"
-	"path/filepath"
 	"testing"
 
-	"github.com/cocoonstack/sandbox/sandboxd/config"
 	"github.com/cocoonstack/sandbox/sandboxd/types"
 )
 
@@ -33,23 +29,3 @@ func TestRuntimeCatalogEmptyACLIsPublic(t *testing.T) {
 		}
 	}
 }
-
-func TestInvalidStaticTenantListNeverBecomesPublic(t *testing.T) {
-	path := writeVolumeImage(t, "data", "private contents")
-	file := filepath.Join(t.TempDir(), "config.json")
-	raw := fmt.Sprintf(`{"pools":[],"volumes":[{"name":"data","path":%q,"tenants":["has space"]}]}`, path)
-	if err := os.WriteFile(file, []byte(raw), 0o600); err != nil {
-		t.Fatalf("WriteFile: %v", err)
-	}
-	cfg, err := config.Load(file)
-	if err != nil {
-		t.Fatalf("legacy config stopped booting: %v", err)
-	}
-	m := newVolumeManager(t, newFakeEngine(), cfg.Volumes)
-	if got := m.Volumes("acme", nil); len(got) != 0 {
-		t.Fatalf("invalid restricted ACL became public: %+v", got)
-	}
-	if err = m.RegisterVolume(t.Context(), "other", types.VolumeRegistration{Path: path + ".other"}); err != nil {
-		t.Fatalf("invalid legacy ACL blocked unrelated registration: %v", err)
-	}
-}

sandboxd/pool/volume_catalog.go

@@ -53,11 +53,6 @@ func (c *volumeCatalog) withinRoots(path string) bool {
 
 func (c *volumeCatalog) normalize(name string, spec types.VolumeRegistration) (*catalogVolume, error) {
 	spec.DirectIO = cmp.Or(spec.DirectIO, types.DirectIOOff)
-	for _, tenant := range spec.Tenants {
-		if !types.NameRe.MatchString(tenant) {
-			return nil, fmt.Errorf("%w: volume %q tenant %q must match %s", ErrBadVolume, name, tenant, types.NameRe)
-		}
-	}
 	if err := config.ValidateVolumeCatalog([]config.VolumeSpec{{
 		Name: name, Path: spec.Path, DirectIO: spec.DirectIO, Writable: spec.Writable, Tenants: spec.Tenants,
 	}}); err != nil {

sandboxd/server/server_test.go

@@ -2440,6 +2440,8 @@ type idActionFunc func(string) error
 
 type guestFunc func(net.Conn)
 
+type managerEdit func(*fakeManager)
+
 type fakeManager struct {
 	counters             pool.Counters
 	registerVolume       func(context.Context, string, types.VolumeRegistration) error
@@ -3033,6 +3035,7 @@ func (f *fakePlacer) TemplateVolumeOwners(probe string, _ []string) []string {
 	}
 	return f.templateVolumeOwners
 }
+
 func (f *fakePlacer) VolumeHolders() map[string]int { return f.volumeHolders }
 func (f *fakePlacer) PeerAddrs() []string           { return f.addrs }
 func (f *fakePlacer) ClientAddr(addr string) string { return addr }
@@ -3184,5 +3187,3 @@ func credToken(cred pool.Cred) string {
 	}
 	return cred.Token
 }
-
-type managerEdit func(*fakeManager)

sandboxd/types/types.go

@@ -233,17 +233,6 @@ func (k PoolKey) Validate() error {
 	return nil
 }
 
-// envView is an immutable env with its inject entries in name order.
-type envView struct {
-	env     Env
-	injects []namedVar
-}
-
-type namedVar struct {
-	name string
-	v    EnvVar
-}
-
 // MoveOut is the source's journal of a live move: the record's first epoch, cocoon's series and the target.
 type MoveOut struct {
 	Epoch  int64  `json:"epoch"`
@@ -266,6 +255,17 @@ func (l EgressLimit) Validate() error {
 	return nil
 }
 
+// envView is an immutable env with its inject entries in name order.
+type envView struct {
+	env     Env
+	injects []namedVar
+}
+
+type namedVar struct {
+	name string
+	v    EnvVar
+}
+
 // Sandbox is the node-local record of one pooled or claimed VM.
 type Sandbox struct {
 	ID     string  `json:"id"`

sdk/go/keepalive_test.go

@@ -164,7 +164,7 @@ func TestPeerQuiet(t *testing.T) {
 	waitUntil(t, func() bool { return !probe.quiet() }, "hang-up not noticed")
 }
 
-func countingSandbox(t *testing.T, serve func(net.Conn), opts ...ClientOption) (*Sandbox, *atomic.Int32) {
+func countingSandbox(t *testing.T, serve connHandler, opts ...ClientOption) (*Sandbox, *atomic.Int32) {
 	t.Helper()
 	var upgrades atomic.Int32
 	sb := testSandbox(t, newAgentServer(t, func(c net.Conn) {