review: whole-repo round on the SOCKS5 and locked-NIC state

README.md

@@ -56,8 +56,8 @@ performance) — source in
 - `e2e/` — in-process full-stack tests (real pool/engine/relay/SDK, fake
   cocoon+guest) plus bare-metal acceptance drivers under `cmd/`: `demo`,
   `smoke`, `meshsmoke`, `crossnode`, `coldproof`, `egresssmoke`,
-  `interceptsmoke`, `volumesmoke`, `lifecycle` (idle→hibernate→archive),
-  `androidsmoke`, `browsersmoke`, `desktopsmoke`, and the `pullbench`/`pushbench`/`rpcbench`
+  `interceptsmoke`, `sockssmoke`, `volumesmoke`, `lifecycle` (idle→hibernate→archive),
+  `androidsmoke`, `browsersmoke`, `desktopsmoke`, and the `pullbench`/`pushbench`/`rpcbench`/`qaab`
   perf drivers
 - `boot/kernel/` — kernel version pin (`VERSION` + matching tarball `SHA256`,
   bump both together) + config fragment (amd64: over `x86_64_defconfig` +

docs/desktop.md

@@ -20,8 +20,12 @@ up a few seconds later — poll `GET /screenshot` until it returns 200.
 
 ## Claim shape
 
-- **Lane**: `net=none` for local tasks (os, office, file work); `net=egress`
-  when tasks visit the OSWorld mocked websites or the real web.
+- **Lane**: `net=none` — for local tasks (os, office, file work) as it is, and
+  with an [egress policy](egress.md) when tasks visit the OSWorld mocked
+  websites or the real web; the session's browsers reach the web through the
+  relay. `net=egress` on a guarded bridge gives the desktop no web: the NIC is
+  locked and the session's proxy setup does not follow the lock marker (a
+  CNI-backed lane still routes directly).
 - **Size**: `2xlarge` (8 CPU / 16G) — the t3.xlarge class the OSWorld AWS
   image runs on; the idle session is ~0.5 GB anonymous memory with
   gnome-shell around 290 MB RSS, and the headroom is for the apps.
@@ -73,7 +77,9 @@ Configure it with `SANDBOXD_ADDR`, `SANDBOXD_TOKEN`, `COCOON_TEMPLATE`
   profile and debugging port.
 - On a guest with no NIC, `guest-proxy.service` points the session's browsers
   and `osworld-server`'s commands at silkd's loopback relay; a guest with a
-  NIC routes directly and the unit does nothing.
+  NIC routes directly and the unit does nothing. The unit tests NIC presence
+  only, so it does not follow the locked-NIC marker silkd honors on the bridge
+  egress lane; run desktop pools on the none lane.
 - No Thunderbird or VS Code yet (snap-only on 24.04 / vendor repo); tasks
   targeting them are out of scope for this flavor version.
 - x86_64 only.

e2e/cmd/browsersmoke/main.go

@@ -18,8 +18,7 @@ import (
 
 const (
 	cdpPort = 9222
-	// Chromium starts after silkd, so the claim returns before CDP is up;
-	// a cold first launch on a loaded node can take tens of seconds.
+	// Chromium starts after silkd, so the claim returns before CDP is up; a cold first launch takes tens of seconds.
 	cdpWait = 3 * time.Minute
 )
 

e2e/cmd/desktopsmoke/main.go

@@ -19,8 +19,7 @@ import (
 
 const (
 	serverPort = 5000
-	// The GNOME session and osworld-server start after silkd; a cold first
-	// launch on a loaded node can take tens of seconds.
+	// the GNOME session and osworld-server start after silkd; a cold first launch on a loaded node takes tens of seconds.
 	serverWait = 3 * time.Minute
 	clickX     = 300
 	clickY     = 300

e2e/cmd/egresssmoke/main.go

@@ -70,8 +70,7 @@ func run(addr, token, template, wantToken, netShape, reach, nicAddr, echo string
 	defer func() { _ = sb.Close() }()
 	fmt.Printf("  claimed %s-lane sandbox %s\n", netShape, sb.ID)
 
-	// The image does not configure the NIC, so bring it up statically to give the
-	// guest a real route toward -reach; a blocked direct egress is then the lock.
+	// the image leaves the NIC unconfigured; a static address gives the guest a route toward -reach, so a blocked direct egress is the lock.
 	if nicAddr != "" {
 		out, cfgErr := sb.Exec(ctx, "sh", "-c", fmt.Sprintf(
 			"nic=$(ip -o link | awk -F': ' '$2 ~ /^(eth|en)/{sub(/@.*/,\"\",$2); print $2; exit}'); "+
@@ -94,9 +93,7 @@ func run(addr, token, template, wantToken, netShape, reach, nicAddr, echo string
 	}
 	fmt.Println("  direct egress blocked")
 
-	// The injection origin is a public echo reached through the proxy: post-#26
-	// the SSRF guard refuses private/loopback destinations even when allow-listed
-	// (issue #27, Option A), so a private origin cannot exercise injection.
+	// the SSRF guard refuses private and loopback destinations even when allow-listed, so injection needs a public echo.
 	seen, err := sb.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -s -x %s http://%s/get", proxy, echo))
 	if err != nil {
 		return fmt.Errorf("proxied exec: %w", err)

e2e/cmd/interceptsmoke/main.go

@@ -73,7 +73,7 @@ func run(addr, token, template, echo, secret, issuer string) error {
 		fmt.Printf("  guest's TLS leaf issued by this node's intermediate (%s)\n", strings.TrimSpace(gotIssuer))
 	}
 
-	blocked, _ := sb.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -s -m 3 https://%s/ >/dev/null 2>&1 && echo REACHED || echo BLOCKED", echo))
+	blocked, _ := sb.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -s -m 3 --noproxy '*' https://%s/ >/dev/null 2>&1 && echo REACHED || echo BLOCKED", echo))
 	if strings.TrimSpace(blocked) != "BLOCKED" {
 		return fmt.Errorf("guest reached the origin without the proxy: %q", strings.TrimSpace(blocked))
 	}

e2e/cmd/sockssmoke/main.go

@@ -1,8 +1,8 @@
 // Command sockssmoke proves the SOCKS5 egress channel from inside a claimed
 // sandbox: an allowed host tunnels over 127.0.0.1:1080, a GET-only host is
 // reachable on the HTTP proxy but refused on SOCKS5, an unlisted host is
-// refused, IMAPS rides the tunnel, and a pool without a policy leaves the
-// host door unwired so the guest's dial is refused.
+// refused, IMAPS rides the tunnel, and no tunnel opens from a pool without a
+// policy.
 package main
 
 import (
@@ -84,9 +84,9 @@ func run(addr, token, template, echo, getOnly, imap string) error {
 	}
 	defer func() { _ = bare.Close() }()
 	if code := curl(ctx, bare, "--socks5-hostname", socksProxy, "http://"+echo+"/get"); code != "000" {
-		return fmt.Errorf("policy-less pool served SOCKS5 with %q, want the host door unwired and the dial refused", code)
+		return fmt.Errorf("policy-less pool opened a SOCKS5 tunnel with %q, want none", code)
 	}
-	fmt.Println("  pool without a policy: host door unwired, guest dial refused")
+	fmt.Println("  pool without a policy: no SOCKS5 tunnel opens")
 	return nil
 }
 

sandboxd/config/config_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"path/filepath"
 	"slices"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -80,9 +81,11 @@ func TestAutoRefillConcurrency(t *testing.T) {
 		{cpus: 384, want: 256},
 		{cpus: 768, want: 256},
 	} {
-		if got := autoRefillConcurrency(tt.cpus); got != tt.want {
-			t.Errorf("autoRefillConcurrency(%d) = %d, want %d", tt.cpus, got, tt.want)
-		}
+		t.Run(strconv.Itoa(tt.cpus)+" cpus", func(t *testing.T) {
+			if got := autoRefillConcurrency(tt.cpus); got != tt.want {
+				t.Errorf("autoRefillConcurrency(%d) = %d, want %d", tt.cpus, got, tt.want)
+			}
+		})
 	}
 }
 

sandboxd/egress/policy.go

@@ -112,7 +112,7 @@ func (p Policy) EvalInner(host, method string) (Rule, Decision) {
 	return Rule{}, DecisionDeny
 }
 
-// Tunnels reports whether some rule can admit an opaque tunnel; false leaves the SOCKS5 listener unbound, and a composite answers true even when its two host sets never meet.
+// Tunnels reports whether some rule can admit an opaque tunnel; a composite answers true even when its two host sets never meet.
 func (p Policy) Tunnels() bool {
 	return slices.ContainsFunc(p.Allow, func(r Rule) bool { return !r.Intercept && r.matchMethod(http.MethodConnect) })
 }

sandboxd/egress/proxy.go

@@ -28,6 +28,9 @@ var hopHeaders = []string{
 // DialFunc opens the upstream connection for a permitted request.
 type DialFunc func(ctx context.Context, network, addr string) (net.Conn, error)
 
+// AuditFunc receives every egress decision the proxy takes.
+type AuditFunc func(Event)
+
 // Secrets resolves a rule's Secret name to the header it injects.
 type Secrets interface {
 	Header(name string) (header, value string, ok bool)
@@ -50,7 +53,7 @@ type Proxy struct {
 	policy  Evaluator
 	secrets Secrets
 	ca      *CA
-	audit   func(Event)
+	audit   AuditFunc
 	dial    DialFunc
 	tr      *http.Transport
 	mitmTr  *http.Transport
@@ -66,7 +69,7 @@ type Proxy struct {
 }
 
 // New builds a Proxy for one sandbox; secrets, ca, and audit may be nil, dial must not.
-func New(sandbox, tenant string, policy Evaluator, secrets Secrets, ca *CA, dial DialFunc, audit func(Event)) *Proxy {
+func New(sandbox, tenant string, policy Evaluator, secrets Secrets, ca *CA, dial DialFunc, audit AuditFunc) *Proxy {
 	p := &Proxy{
 		sandbox: sandbox,
 		tenant:  tenant,

sandboxd/egress/socks.go

@@ -28,18 +28,20 @@ const (
 	socksBadCmd    = 0x07
 	socksBadAtyp   = 0x08
 
-	socksTimeout = 30 * time.Second
+	socksTimeout       = 30 * time.Second
+	socksAcceptBackoff = 50 * time.Millisecond
 )
 
 // ServeSOCKS serves SOCKS5 on ln until it closes; a tunnel takes the decision of an un-intercepted CONNECT.
-func (p *Proxy) ServeSOCKS(ctx context.Context, ln net.Listener) error {
+func (p *Proxy) ServeSOCKS(ctx context.Context, ln net.Listener) {
 	for {
 		conn, err := ln.Accept()
+		if errors.Is(err, net.ErrClosed) {
+			return
+		}
 		if err != nil {
-			if errors.Is(err, net.ErrClosed) {
-				return nil
-			}
-			return err
+			time.Sleep(socksAcceptBackoff)
+			continue
 		}
 		go p.serveSocks(ctx, conn)
 	}

sandboxd/egress/socks_test.go

@@ -157,7 +157,7 @@ func socksProxy(t *testing.T, policy Policy, ca *CA, dial DialFunc, events chan
 		_ = ln.Close()
 		p.Close()
 	})
-	go func() { _ = p.ServeSOCKS(t.Context(), ln) }()
+	go p.ServeSOCKS(t.Context(), ln)
 	return p, ln.Addr().String()
 }
 

sandboxd/engine/engine.go

@@ -479,7 +479,6 @@ func parseSemver(s string) ([3]int, bool) {
 	return out, true
 }
 
-// readBufLine takes one line from r, capped at max bytes.
 func readBufLine(r *bufio.Reader, max int) (string, error) {
 	line, err := r.ReadSlice('\n')
 	if err != nil {

sandboxd/pool/egress.go

@@ -157,7 +157,7 @@ func (m *Manager) armEgressProxy(ctx context.Context, sb *types.Sandbox) error {
 	m.mu.Unlock()
 	go func() { _ = el.srv.Serve(el.ln) }()
 	if el.socks != nil {
-		go func() { _ = proxy.ServeSOCKS(evCtx, el.socks) }()
+		go proxy.ServeSOCKS(evCtx, el.socks)
 	}
 	return nil
 }

sandboxd/pool/pool.go

@@ -44,9 +44,9 @@ const (
 	coldProbeTimeout  = 90 * time.Second
 	// a clone answers in ~1s even saturated, so a silent one is replaced, not waited on
 	warmProbeTimeout = 5 * time.Second
-	// One list; a wrong answer only costs an extra sweep.
+	// one list; a wrong answer only costs an extra sweep.
 	removeVerifyTimeout = 15 * time.Second
-	// A full node clears only when VMs go away; retrying sooner buys nothing.
+	// a full node clears only when VMs go away; retrying sooner buys nothing.
 	capacityBackoff    = buildRetryDelay
 	vsockPollInterval  = 100 * time.Millisecond
 	defaultTTL         = 5 * time.Minute
@@ -55,14 +55,14 @@ const (
 	recommitMaxBackoff = 5 * time.Second
 	// one failed boot is ordinary; only an unbroken run predicts the next failure
 	refillFailStreak = 8
-	// Doubles per failure past the streak.
+	// doubles per failure past the streak.
 	refillBackoffBase = 250 * time.Millisecond
 	refillBackoffMax  = buildRetryDelay
 	// fallbacks when a Manager is built from a Config that skipped config.Load's defaulting
 	defaultMaxFork = 16
 	defaultRefill  = 4
 
-	// A heal pulls a whole guest memory image, so a few saturate disk and NIC.
+	// a heal pulls a whole guest memory image, so a few saturate disk and NIC.
 	maxConcurrentHeals = 4
 
 	vmPrefix        = "sbx-"
@@ -92,7 +92,7 @@ var (
 	ErrNoEgressFork      = errors.New("egress-lane sandboxes cannot fork, checkpoint, or promote: a resumed guest egresses before its fresh tap can be locked")
 	ErrVolumeCapture     = errors.New("sandboxes with volumes cannot hibernate, fork, checkpoint, or promote")
 	ErrVolumeBusy        = errors.New("volume is held by another claim")
-	// Replaying a journal takes a writable mount, so readers stay out.
+	// replaying a journal takes a writable mount, so readers stay out.
 	ErrVolumeNeedsRecovery = errors.New("volume needs recovery by a writable claim")
 	ErrQuota               = errors.New("node claim quota reached")
 
@@ -691,7 +691,6 @@ func logSweepResult(ctx context.Context, logger *log.Fields, err error, okMsg, f
 	}
 }
 
-// benignSweepErr reports whether err is the expected outcome of a housekeeping sweep.
 func benignSweepErr(err error) bool {
 	return errors.Is(err, ErrUnknownSandbox) || errors.Is(err, errWokeMeanwhile) ||
 		errors.Is(err, ErrNoEgressHibernate)

sandboxd/pool/reconcile.go

@@ -50,7 +50,7 @@ func (m *Manager) Reconcile(ctx context.Context) error {
 			// running with either flag set means a crashed wake or an intent the engine never saw
 			sb.HibernateSnap, sb.PendingSnap = "", ""
 		case ok && sb.HibernateSnap != "":
-			// Hibernated: the VM is stopped by design and wakes on demand.
+			// hibernated: the VM is stopped by design and wakes on demand.
 			sb.PendingSnap = ""
 		case ok && sb.PendingSnap != "" && (snapsErr != nil || slices.Contains(snaps, sb.PendingSnap)):
 			// the journaled intent names the wake image, so adopt it when the snapshot is there
@@ -139,7 +139,7 @@ func (m *Manager) removeStaleVM(ctx context.Context, name string, rec types.VMRe
 		// a canceled ctx must not skip the busy check guarding the forced remove
 		switch outcome, err := m.eng.ReconcileStaleCreate(context.WithoutCancel(ctx), name); {
 		case err != nil:
-			// Verb missing (cocoon < v0.5.8) or failed: keep the old sweep.
+			// verb missing (cocoon < v0.5.8) or failed: keep the old sweep.
 			logger.Warnf(ctx, "reconcile stale create %s: %v; removing", name, err)
 		case outcome == engine.StaleCreateCollected, outcome == engine.StaleCreateNotFound:
 			return true
@@ -176,6 +176,9 @@ func (m *Manager) resyncEgress(ctx context.Context, live map[string]types.VMReco
 			if err == nil && !locked[tap] {
 				err = netfilter.Lock(tap)
 			}
+			if err == nil && !locked[tap] {
+				err = m.markLockedNIC(ctx, sb.Key, sb.VsockSocket)
+			}
 			if err != nil {
 				logger.Errorf(ctx, err, "ensure egress lock %s; quarantining", sb.ID)
 				quarantine = append(quarantine, sb)
@@ -186,7 +189,7 @@ func (m *Manager) resyncEgress(ctx context.Context, live map[string]types.VMReco
 			logger.Errorf(ctx, proxyErr, "arm egress proxy %s", sb.ID)
 		}
 	}
-	// Quarantine before the sweep so a failed remove's still-running tap is kept.
+	// quarantine before the sweep so a failed remove's still-running tap is kept.
 	keep := make(map[string]bool, len(live))
 	for _, sb := range quarantine {
 		if m.quarantineClaim(ctx, sb) {
@@ -195,7 +198,7 @@ func (m *Manager) resyncEgress(ctx context.Context, live map[string]types.VMReco
 			keep[sb.TAP] = true
 		}
 	}
-	// Keep every still-running VM's tap; sweep only a confirmed-gone VM's table.
+	// keep every still-running VM's tap; sweep only a confirmed-gone VM's table.
 	for name, rec := range live {
 		if tap := rec.TapDevice(); tap != "" && !removed[name] {
 			keep[tap] = true
@@ -206,7 +209,7 @@ func (m *Manager) resyncEgress(ctx context.Context, live map[string]types.VMReco
 	}
 }
 
-// A failed remove stays out of service and queued until teardown succeeds.
+// quarantineClaim parks a claim whose teardown failed: out of service and queued until a remove succeeds.
 func (m *Manager) quarantineClaim(ctx context.Context, sb *types.Sandbox) bool {
 	td := m.quiesceVolumes(ctx, sb)
 	gone := m.removeOrRetry(ctx, sb.VMName, sb.ID, "", td)

sdk/langchain/cocoonsandbox_langchain/toolkit.py

@@ -48,7 +48,7 @@ def __init__(
         net: str = "",
         ttl_seconds: int = 0,
         from_checkpoint: str = "",
-    ):
+    ) -> None:
         self._client = Client(addr, api_token=api_token, timeout=CALL_TIMEOUT)
         self._template = template
         self._net = net
@@ -61,7 +61,7 @@ def __init__(
     def __enter__(self) -> CocoonToolkit:
         return self
 
-    def __exit__(self, *exc) -> None:
+    def __exit__(self, *exc: object) -> None:
         self.close()
 
     def get_tools(self) -> list[StructuredTool]:

sdk/python/cocoonsandbox/client.py

@@ -26,7 +26,7 @@
 class Client:
     """Talks to one sandboxd node (and, transparently, its cluster)."""
 
-    def __init__(self, addr: str, api_token: str = "", timeout: float = 120.0):
+    def __init__(self, addr: str, api_token: str = "", timeout: float = 120.0) -> None:
         self.addr = addr.split(",")[0].strip()
         self.api_token = api_token
         self.timeout = timeout
@@ -143,7 +143,7 @@ def post(peer):
         owner, reply = _redirect_fallback(addr, redirect, post, verb)
         return self._handle_from(owner, reply)
 
-    def _peers(self) -> list:
+    def _peers(self) -> list[str]:
         # /v1/peers is tenant-accessible; /v1/info is operator-only, so a tenant cannot read peers from it.
         try:
             return (
@@ -171,7 +171,7 @@ def _post_json(self, addr: str, path: str, body: dict, verb: str) -> dict:
         return self._request(addr, "POST", path, body, verb)
 
     def _request(
-        self, addr: str, method: str, path: str, body, verb: str, bearer: str = "", timeout: float = 0.0
+        self, addr: str, method: str, path: str, body: dict | None, verb: str, bearer: str = "", timeout: float = 0.0
     ) -> dict:
         """Issues one control-plane request. bearer overrides the api token —
         sandbox-scoped verbs (release, hibernate) authenticate with the