diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index c185c488..dd4989d2 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -444,7 +444,7 @@ destroyed and the pool entry retires. `net`/`size` default like a claim's. Answers the fresh `GET /v1/info` payload. 400 bad key, negative warm/idle, `warm_max` below `warm`, `idle_hibernate_seconds` on an egress pool, a negative archive duration or an `archive_after_seconds` not above the pool's -`idle_hibernate_seconds`, duplicate pool, or a config-owned `egress`/`warmup`/`capture_trim`/`storage` +`idle_hibernate_seconds`, duplicate pool, or a config-owned `egress`/`warmup`/`capture_trim`/`storage`/`egress_upstream_env` field; 401 bad api token; 409 egress pool on a node without an egress attachment. diff --git a/mcp/server.go b/mcp/server.go index 7702be9a..acd2badc 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -2,6 +2,7 @@ package main import ( "bufio" + "cmp" "context" "encoding/json/jsontext" "encoding/json/v2" @@ -120,11 +121,8 @@ func (s *server) dispatch(ctx context.Context, req *rpcRequest) rpcResponse { } text, err := s.callTool(ctx, call.Name, call.Arguments) if err != nil { - if text == "" { - text = err.Error() - } return result(req.ID, map[string]any{ - "content": []map[string]any{{"type": "text", "text": text}}, + "content": []map[string]any{{"type": "text", "text": cmp.Or(text, err.Error())}}, "isError": true, }) } diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index 81c69489..465fa11c 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -80,15 +80,13 @@ func (p Policy) Validate() error { case "*.": return fmt.Errorf("allow[%d]: host %q needs a domain after the wildcard", i, r.Host) } - seen := make(map[uint16]struct{}, len(r.Ports)) - for _, port := range r.Ports { + for j, port := range r.Ports { if port == 0 { return fmt.Errorf("allow[%d]: port 0 is not a destination", i) } - if _, dup := seen[port]; dup { + if slices.Contains(r.Ports[:j], port) { return fmt.Errorf("allow[%d]: port %d repeated", i, port) } - seen[port] = struct{}{} } } if p.Socks5 && !p.admitsTunnel() { diff --git a/sandboxd/pool/archive.go b/sandboxd/pool/archive.go index 8c7a6272..e977abf7 100644 --- a/sandboxd/pool/archive.go +++ b/sandboxd/pool/archive.go @@ -315,12 +315,11 @@ func (m *Manager) retryArchiveDelete(ctx context.Context, ckID string) { } err := m.ckpts.Delete(ctx, ckID) l.Unlock() + m.recDone(ckID) if err != nil { - m.recDone(ckID) log.WithFunc("pool.retryArchiveDelete").Warnf(ctx, "delete %s: %v", ckID, err) return } - m.recDone(ckID) if err := m.clearArchiveCk(ckID); err != nil { log.WithFunc("pool.retryArchiveDelete").Warnf(ctx, "clear %s: %v", ckID, err) } diff --git a/sandboxd/pool/checkpoint.go b/sandboxd/pool/checkpoint.go index ff730394..c894d935 100644 --- a/sandboxd/pool/checkpoint.go +++ b/sandboxd/pool/checkpoint.go @@ -361,12 +361,8 @@ func (m *Manager) deleteCkLocked(ctx context.Context, ckID string) error { l.Lock() err := m.ckpts.Delete(ctx, ckID) l.Unlock() - if err != nil { - m.recDone(ckID) - return err - } m.recDone(ckID) - return nil + return err } func (m *Manager) loadCheckpoint(ctx context.Context, ckptID string) (types.Checkpoint, error) { diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index d1f7248c..2653145b 100644 --- a/sandboxd/pool/pool.go +++ b/sandboxd/pool/pool.go @@ -384,14 +384,7 @@ type Manager struct { // NewManager builds a manager from the node config; ctx bounds backend construction. func NewManager(ctx context.Context, cfg *config.Config, eng Engine, secrets *egress.SecretStore) (*Manager, error) { - maxFork := cfg.MaxForkCount - if maxFork < 1 { - maxFork = defaultMaxFork - } - refill := cfg.RefillConcurrency - if refill < 1 { - refill = defaultRefill - } + refill := cmp.Or(cfg.RefillConcurrency, defaultRefill) m := &Manager{ eng: eng, dataDir: cfg.DataDir, @@ -400,7 +393,7 @@ func NewManager(ctx context.Context, cfg *config.Config, eng Engine, secrets *eg releaseDelay: time.Duration(cfg.ReleaseDelaySeconds) * time.Second, vmmRestart: cfg.VMMRestart, cocoondSocket: cfg.CocoondSocket, - maxFork: maxFork, + maxFork: cmp.Or(cfg.MaxForkCount, defaultMaxFork), store: newClaimStore(cfg.DataDir, cfg.SyncClaims), volumes: make(map[string]catalogVolume, len(cfg.Volumes)), volumeAdmission: map[string]volumeHolders{}, diff --git a/sandboxd/pool/poolstore.go b/sandboxd/pool/poolstore.go index 084c0005..cf8b6623 100644 --- a/sandboxd/pool/poolstore.go +++ b/sandboxd/pool/poolstore.go @@ -101,7 +101,7 @@ func (m *Manager) adoptPersistedPools(ctx context.Context) error { return nil } -// poolSeedHash digests a pool set's warm-target shape, order-independent, without the config-owned egress, warmup, capture_trim and storage. +// poolSeedHash digests a pool set's warm-target shape, order-independent, without the config-owned egress, warmup, capture_trim, storage and egress_upstream_env. func poolSeedHash(specs []config.PoolSpec) string { shaped := slices.Clone(specs) for i := range shaped { @@ -109,6 +109,7 @@ func poolSeedHash(specs []config.PoolSpec) string { shaped[i].Warmup = nil shaped[i].CaptureTrim = false shaped[i].Storage = "" + shaped[i].EgressUpstreamEnv = "" } slices.SortFunc(shaped, func(a, b config.PoolSpec) int { return strings.Compare(a.Hash(), b.Hash()) }) return utils.DigestHex(shaped) diff --git a/sandboxd/pool/poolstore_test.go b/sandboxd/pool/poolstore_test.go index 2d7ec6d4..27ec6b86 100644 --- a/sandboxd/pool/poolstore_test.go +++ b/sandboxd/pool/poolstore_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/egress" "github.com/cocoonstack/sandbox/sandboxd/types" ) @@ -68,6 +69,24 @@ func TestPoolSeedHashBytesArePinned(t *testing.T) { } } +func TestPoolSeedHashIgnoresConfigOwnedFields(t *testing.T) { + base := config.PoolSpec{PoolKey: seedKey, Warm: 2} + want := poolSeedHash([]config.PoolSpec{base}) + for name, edit := range map[string]func(*config.PoolSpec){ + "egress": func(s *config.PoolSpec) { s.Egress = &egress.Policy{Allow: []egress.Rule{{Host: "api.example.com"}}} }, + "warmup": func(s *config.PoolSpec) { s.Warmup = []string{"true"} }, + "capture_trim": func(s *config.PoolSpec) { s.CaptureTrim = true }, + "storage": func(s *config.PoolSpec) { s.Storage = "20G" }, + "egress_upstream_env": func(s *config.PoolSpec) { s.EgressUpstreamEnv = "POOL_UPSTREAM" }, + } { + spec := base + edit(&spec) + if got := poolSeedHash([]config.PoolSpec{spec}); got != want { + t.Errorf("a %s edit changed the seed: the API cannot carry it, so a restart would warn of an override", name) + } + } +} + func TestConcurrentSetPoolsPersistLatest(t *testing.T) { dir := t.TempDir() m := newTestManagerAt(t, newFakeEngine(), dir, config.PoolSpec{PoolKey: seedKey, Warm: 1}) diff --git a/sandboxd/pool/template.go b/sandboxd/pool/template.go index f98a32cc..c26576dd 100644 --- a/sandboxd/pool/template.go +++ b/sandboxd/pool/template.go @@ -354,18 +354,15 @@ func (m *Manager) resolveGolden(ctx context.Context, key types.PoolKey, tenant s id := store.TemplateID(key.Hash()) l := m.recLock(id) l.RLock() + cleanup := func() { l.RUnlock(); m.recDone(id) } dir, meta, digest, err := m.tpls.Fetch(ctx, id) - if errors.Is(err, store.ErrNotFound) { - l.RUnlock() - m.recDone(id) - return goldenResolution{}, nil - } if err != nil { - l.RUnlock() - m.recDone(id) + cleanup() + if errors.Is(err, store.ErrNotFound) { + return goldenResolution{}, nil + } return goldenResolution{}, err } - cleanup := func() { l.RUnlock(); m.recDone(id) } var rec templateRecord if err := json.Unmarshal(meta, &rec); err != nil { cleanup() diff --git a/sandboxd/server/server.go b/sandboxd/server/server.go index d0821e34..f60b783e 100644 --- a/sandboxd/server/server.go +++ b/sandboxd/server/server.go @@ -2,6 +2,7 @@ package server import ( + "cmp" "context" "crypto/subtle" "encoding/json/jsontext" @@ -468,10 +469,7 @@ func (s *Server) handleSandboxes(w http.ResponseWriter, r *http.Request) { writeErr(w, http.StatusBadRequest, err.Error()) return } - scope := tenantFrom(r.Context()) - if scope == "" { - scope = query.Get("tenant") - } + scope := cmp.Or(tenantFrom(r.Context()), query.Get("tenant")) writeJSON(w, http.StatusOK, SandboxListResponse{Sandboxes: s.mgr.Sandboxes(scope, query.Get("claim_ref"), filter)}) } diff --git a/sandboxd/store/peer/transport.go b/sandboxd/store/peer/transport.go index 46052632..dc16f45a 100644 --- a/sandboxd/store/peer/transport.go +++ b/sandboxd/store/peer/transport.go @@ -145,8 +145,6 @@ func Untar(r io.Reader, dst string) error { if err := writeFile(target, tr, os.FileMode(hdr.Mode).Perm()); err != nil { //nolint:gosec // Perm masks to 0777 return err } - default: - continue } } } diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index a56b7f70..9351d746 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -779,10 +779,7 @@ func ValidateVolumes(volumes []Volume, attachOnly bool) ([]Volume, error) { applied[i] = Volume{Name: volume.Name, Mode: mode, AttachOnly: true} continue } - mount := volume.Mount - if mount == "" { - mount = DefaultVolumeMount(volume.Name) - } + mount := cmp.Or(volume.Mount, DefaultVolumeMount(volume.Name)) if err := validateVolumeMount(mount); err != nil { return nil, fmt.Errorf("volumes[%d] mount %q: %w", i, mount, err) } diff --git a/sdk/go/pools.go b/sdk/go/pools.go index 526b9a9b..428a3d51 100644 --- a/sdk/go/pools.go +++ b/sdk/go/pools.go @@ -6,7 +6,7 @@ import ( "net/http" ) -// PoolSpec is a desired warm pool for SetPools; egress, warmup, capture_trim and storage are config-owned, so it has no field for them. +// PoolSpec is a desired warm pool for SetPools; egress, warmup, capture_trim, storage and egress_upstream_env are config-owned, so it has no field for them. type PoolSpec struct { Template string `json:"template"` Net NetShape `json:"net,omitempty"` diff --git a/sdk/python/cocoonsandbox/frames.py b/sdk/python/cocoonsandbox/frames.py index f9e15af9..4a2ea58c 100644 --- a/sdk/python/cocoonsandbox/frames.py +++ b/sdk/python/cocoonsandbox/frames.py @@ -4,6 +4,7 @@ import base64 import binascii +import contextlib import json import sys from typing import Any @@ -41,10 +42,8 @@ def decode_response(line: bytes) -> dict[str, Any]: if te > 0 and line[9:te] in (b"stdout", b"stderr", b"data") and line.startswith(b'","data":"', te): de = line.find(b'"', te + 10) if de > 0 and line[de:] in (b'"}', b'"}\n'): - try: + with contextlib.suppress(binascii.Error): return {"type": line[9:te].decode(), "data": base64.b64decode(line[te + 10 : de], validate=True)} - except binascii.Error: - pass frame = json.loads(line) if not isinstance(frame, dict) or "type" not in frame: raise ValueError(f"frame without a type tag: {line[:80]!r}") diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index d25107e7..dbee99b8 100644 --- a/sdk/python/cocoonsandbox/sandbox.py +++ b/sdk/python/cocoonsandbox/sandbox.py @@ -668,8 +668,7 @@ def cut() -> None: def _feed_stdin(conn: Conn, stdin: bytes) -> None: with contextlib.suppress(SandboxError, OSError): # the reader reports the real failure - if stdin: - _send_chunks(conn, stdin, op="stdin") + _send_chunks(conn, stdin, op="stdin") conn.send("stdin_close")