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/deploy.md

@@ -106,7 +106,7 @@ sandboxd reads one JSON file (`-config`, default
 | `checkpoint_ttl_hours` | 0 (keep forever) | ages out checkpoints older than this; the sweep runs hourly and at startup. Explicit deletes never wait for it. Must be nonzero and match fleet-wide when `checkpoint_peer_heal` is on — it is the expiry eligibility point for a healed replica a delete broadcast missed, after which its next successful hourly sweep removes it; persistent sweep failure extends retention until one succeeds, so it is not a hard ceiling |
 | `checkpoint_peer_heal` | false | on a cluster, lets a node pull a checkpoint it lacks from a peer — found via a live probe, not gossip — rather than failing the branch; see [placement lifecycle](cluster.md#checkpoints-on-a-cluster). Three requirements, all enforced at config load: a nonempty `api_token` (the blob transfer between peers authenticates with it; without one the raw record stream would be open), `mesh.cluster_key` set (the pull presents the fleet `api_token` to an address learned from the peer probe, so the gossip layer carrying that address must itself be authenticated), and `checkpoint_ttl_hours` nonzero (a replica a delete broadcast missed becomes eligible for expiry after it, and its next successful hourly sweep removes it — so it is the finite eligibility point, not an exact ceiling). A shared checkpoint store (`checkpoint_store` kind `s3`) ignores this setting — every node already resolves every checkpoint directly, so there is nothing to heal |
 | `warm_max` (pool entry) | 0 (static) | turns on the demand-adaptive watermark for that pool: the warm target rises from `warm` toward `warm_max` while claims arrive faster than the measured provision lead covers, and decays back over ~a minute of silence |
-| `warmup` (pool entry) | unset | argv run in the golden VM after readiness and before its snapshot, so the files it touches are page-cache-resident in every clone, and again in every clone before it joins the warm pool, so those pages are already faulted into the restored VM when the first command runs — e.g. `["node", "-e", "0"]` on a Node flavor. It runs under the engine's 2-minute command timeout in silkd's base environment (`PATH`, `TERM`, and the guest image's proxy variables on the none lane); a non-zero exit or a timeout fails the golden build, so the pool stays unfilled until the config is fixed. Config-owned like `egress`: `PUT /v1/pools` rejects it, and a golden built with a different warmup is rebuilt |
+| `warmup` (pool entry) | unset | argv run in the golden VM after readiness and before its snapshot, so the files it touches are page-cache-resident in every clone, and again in every clone before it joins the warm pool, so those pages are already faulted into the restored VM when the first command runs — e.g. `["node", "-e", "0"]` on a Node flavor. It runs under the engine's 2-minute command timeout in silkd's base environment (`PATH`, `TERM`, and the guest image's proxy variables wherever nothing routes directly — the none lane and the locked bridge egress lane — with the relay not yet armed, since arming happens at claim); a non-zero exit or a timeout fails the golden build, so the pool stays unfilled until the config is fixed. Config-owned like `egress`: `PUT /v1/pools` rejects it, and a golden built with a different warmup is rebuilt |
 | `max_claims` | 0 (unlimited) | node-wide cap on live claims; claim/fork/branch requests beyond it answer 429 with the pool state unharmed (on a cluster, normal warm-candidate placement applies, with volume claims limited to candidates holding every requested volume) |
 | `audit_log` | false | append every relayed request frame's op + addressing fields (never payloads) to `<data_dir>/audit.jsonl`, size-rotated with one `.1` backup. Records are `{t, id, op}` plus whichever addressing fields the op carries (`argv`, `path`, `dest`, `from`, `to`, `url`, `session`, `port`), plus `method` (`GET`, `CONNECT`, `SOCKS5`, …), `decision` and `secret` (the ref name, never its value) on `egress` records; preview accesses record as op `preview`, one per request. A request frame whose first line exceeds 4 KiB is skipped, never truncated |
 | `idle_hibernate_seconds` | 0 (off) | node-wide idle policy for unpooled claims (template/checkpoint claims): a none-lane claim with no data-plane connection for this long is hibernated; the next call that reaches the guest wakes it transparently. Per-pool `idle_hibernate_seconds` does the same for that pool's claims; pooled keys ignore the node-wide value, and egress pools reject it because they cannot resume safely. Opt in deliberately: a wake costs latency and the snapshot, so callers with their own idle logic must not pay twice |

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.

docs/egress.md

@@ -20,10 +20,11 @@ proxy evaluates the policy, injects the matched rule's secret host-side, dials
 the origin, and relays the response.
 
 The base image sets `http_proxy`/`https_proxy`/`no_proxy` on silkd's unit, and
-silkd forwards exactly those variables into every exec — only when the guest
-has no NIC beyond `lo`/`sit0`, so a lane with a working NIC is never steered
-into a relay whose host side is closed. An unconfigured client just works, and
-`-x` still does:
+silkd forwards exactly those variables into every exec — whenever nothing
+routes directly: the guest has no NIC beyond `lo`/`sit0`, or the host has
+locked the NIC it has (the egress lane below). A lane with a working NIC is
+never steered into a relay whose host side is closed. An unconfigured client
+just works, and `-x` still does:
 
 ```sh
 curl https://api.github.com/user                            # allowed + credentialed
@@ -73,6 +74,12 @@ remove keeps an existing lock in place; the next restart retries the remove). A
 lock that never applied plus a failed remove leaves the VM unguarded until a
 later remove succeeds.
 
+Because the lock is invisible from inside the guest, sandboxd also tells silkd
+about it: a marker written into the guest before the golden snapshot (or on a
+cold boot) makes silkd hand the proxy variables to every exec on this lane
+exactly as on the none lane, while git keeps running as on any lane with a NIC.
+A golden built before the marker existed is rebuilt, not adopted.
+
 Egress-lane sandboxes do not hibernate, archive, fork, checkpoint, or promote:
 cocoon resumes a guest before its fresh tap can be re-locked, so any resume from
 a snapshot would open an unlocked-NIC window. Keeping the lane live holds the

docs/silkd.md

@@ -68,9 +68,11 @@ the host's [guarded-egress](egress.md) proxy over vsock (`CID2:2049`), and
 `127.0.0.1:1080` to its SOCKS5 door (`CID2:2050`) for clients whose protocol
 has no HTTP-proxy form; when the host wired no policy the per-connection dial
 is refused, so the ports are inert.
-On the no-network lane silkd forwards the image-baked proxy variables
-(`http_proxy` and friends) into every exec, so unconfigured clients use the
-relay without being told; a lane with its own network never gets them.
+Where nothing routes directly — the no-network lane, or a NIC the host has
+nft-locked and marked as such via `/etc/silkd-nic-locked` — silkd forwards the
+image-baked proxy variables (`http_proxy` and friends) into every exec, so
+unconfigured clients use the relay without being told; a lane with its own
+routed network never gets them.
 
 ## Limits
 

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}'); "+
@@ -81,7 +80,7 @@ func run(addr, token, template, wantToken, netShape, reach, nicAddr, echo string
 		}
 	}
 
-	out, _ := sb.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -s -m 3 %s || echo BLOCKED", target))
+	out, _ := sb.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -s -m 3 --noproxy '*' %s || echo BLOCKED", target))
 	if guarded && !strings.Contains(out, "BLOCKED") {
 		return fmt.Errorf("guest reached the origin without the proxy: %q", out)
 	}
@@ -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)
@@ -106,6 +103,12 @@ func run(addr, token, template, wantToken, netShape, reach, nicAddr, echo string
 	}
 	fmt.Println("  allowed origin reached; injected credential observed host-side")
 
+	implicit, _ := sb.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -s -m 10 http://%s/get", echo))
+	if !strings.Contains(implicit, wantToken) {
+		return fmt.Errorf("origin %s not reached without -x: silkd did not hand the proxy variables to the exec: %q", echo, strings.TrimSpace(implicit))
+	}
+	fmt.Println("  allowed origin reached with no -x: proxy variables present in the exec")
+
 	if out, _ := sb.Exec(ctx, "sh", "-c", "env; cat /proc/1/environ 2>/dev/null | tr '\\0' '\\n'"); wantToken != "" && strings.Contains(out, wantToken) {
 		return fmt.Errorf("secret value leaked into the guest")
 	}

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
 }
 

e2e/fakeengine_test.go

@@ -110,6 +110,8 @@ func (f *fakeEngine) DialGuestPort(context.Context, string, uint16) (net.Conn, e
 
 func (f *fakeEngine) InstallCACert(context.Context, string, []byte) error { return nil }
 
+func (f *fakeEngine) MarkNICLocked(context.Context, string) error { return nil }
+
 func (f *fakeEngine) Warmup(context.Context, string, []string) error { return nil }
 
 func (f *fakeEngine) DiskAttach(_ context.Context, vmName string, spec engine.VolumeSpec) error {

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/engine/nicmark.go

@@ -0,0 +1,12 @@
+package engine
+
+import "context"
+
+const nicLockedMark = "/etc/silkd-nic-locked"
+
+// MarkNICLocked tells silkd the guest's NIC is nft-locked, so it steers execs at the relay.
+func (e *Engine) MarkNICLocked(ctx context.Context, vsockSocket string) error {
+	ctx, cancel := context.WithTimeout(ctx, cmdTimeout)
+	defer cancel()
+	return e.silkdWriteFile(ctx, vsockSocket, nicLockedMark, 0o644, nil)
+}