diff --git a/.github/workflows/sandboxd.yml b/.github/workflows/sandboxd.yml index e84e19521..e9e68632b 100644 --- a/.github/workflows/sandboxd.yml +++ b/.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 diff --git a/docs/deploy.md b/docs/deploy.md index ad9d7cb71..441ac1165 100644 --- a/docs/deploy.md +++ b/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 diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index abf8f87ab..9e877dc96 100644 --- a/docs/sandboxd-api.md +++ b/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} diff --git a/e2e/cmd/egresssmoke/main.go b/e2e/cmd/egresssmoke/main.go index 2427a574b..1a4def379 100644 --- a/e2e/cmd/egresssmoke/main.go +++ b/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 ( diff --git a/e2e/cmd/rpcbench/main.go b/e2e/cmd/rpcbench/main.go index c1dbb92bf..e9e05a7ba 100644 --- a/e2e/cmd/rpcbench/main.go +++ b/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 ( diff --git a/mcp/server_test.go b/mcp/server_test.go index 9fbfee915..8febe938d 100644 --- a/mcp/server_test.go +++ b/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") { diff --git a/protocol/wire/frame.go b/protocol/wire/frame.go index 81ec74578..02710358b 100644 --- a/protocol/wire/frame.go +++ b/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. diff --git a/sandboxd/config/config.go b/sandboxd/config/config.go index a824d422b..881df04f5 100644 --- a/sandboxd/config/config.go +++ b/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 } diff --git a/sandboxd/config/config_test.go b/sandboxd/config/config_test.go index 7dd0c8254..a2b2a9936 100644 --- a/sandboxd/config/config_test.go +++ b/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"}, diff --git a/sandboxd/outbound/tun.go b/sandboxd/outbound/tun.go index ad43bef6b..6d51ce8a5 100644 --- a/sandboxd/outbound/tun.go +++ b/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) diff --git a/sandboxd/pool/volume_acl_test.go b/sandboxd/pool/volume_acl_test.go index c05568735..54c22b92f 100644 --- a/sandboxd/pool/volume_acl_test.go +++ b/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) - } -} diff --git a/sandboxd/pool/volume_catalog.go b/sandboxd/pool/volume_catalog.go index a5248eac1..30f3a6472 100644 --- a/sandboxd/pool/volume_catalog.go +++ b/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 { diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index bb8de3124..ff7616536 100644 --- a/sandboxd/server/server_test.go +++ b/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) diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 605a89f0f..129587771 100644 --- a/sandboxd/types/types.go +++ b/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"` diff --git a/sdk/go/keepalive_test.go b/sdk/go/keepalive_test.go index 85f001986..2770f160b 100644 --- a/sdk/go/keepalive_test.go +++ b/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) {