diff --git a/README.md b/README.md index df13040a..31cf39d0 100644 --- a/README.md +++ b/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` + diff --git a/docs/deploy.md b/docs/deploy.md index 54e5faba..f3817d05 100644 --- a/docs/deploy.md +++ b/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 `/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 | diff --git a/docs/desktop.md b/docs/desktop.md index 639e7f12..b2842ba4 100644 --- a/docs/desktop.md +++ b/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. diff --git a/docs/egress.md b/docs/egress.md index 73a18c6b..03dd970e 100644 --- a/docs/egress.md +++ b/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 diff --git a/docs/silkd.md b/docs/silkd.md index ea31a25a..9750c7d7 100644 --- a/docs/silkd.md +++ b/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 diff --git a/e2e/cmd/browsersmoke/main.go b/e2e/cmd/browsersmoke/main.go index f1ea03d2..be53fab5 100644 --- a/e2e/cmd/browsersmoke/main.go +++ b/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 ) diff --git a/e2e/cmd/desktopsmoke/main.go b/e2e/cmd/desktopsmoke/main.go index 95d1aea2..b3c07173 100644 --- a/e2e/cmd/desktopsmoke/main.go +++ b/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 diff --git a/e2e/cmd/egresssmoke/main.go b/e2e/cmd/egresssmoke/main.go index c59f41e7..edb5a5ab 100644 --- a/e2e/cmd/egresssmoke/main.go +++ b/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") } diff --git a/e2e/cmd/interceptsmoke/main.go b/e2e/cmd/interceptsmoke/main.go index 46f4a405..4c20fc22 100644 --- a/e2e/cmd/interceptsmoke/main.go +++ b/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)) } diff --git a/e2e/cmd/sockssmoke/main.go b/e2e/cmd/sockssmoke/main.go index 7b722937..5d12d06b 100644 --- a/e2e/cmd/sockssmoke/main.go +++ b/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 } diff --git a/e2e/fakeengine_test.go b/e2e/fakeengine_test.go index ca7909a3..8475478f 100644 --- a/e2e/fakeengine_test.go +++ b/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 { diff --git a/sandboxd/config/config_test.go b/sandboxd/config/config_test.go index f3473075..a4cf391a 100644 --- a/sandboxd/config/config_test.go +++ b/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) + } + }) } } diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index c0bd492e..923e6e06 100644 --- a/sandboxd/egress/policy.go +++ b/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) }) } diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index f898ea33..d451fbef 100644 --- a/sandboxd/egress/proxy.go +++ b/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, diff --git a/sandboxd/egress/socks.go b/sandboxd/egress/socks.go index 2855abb6..adc07b56 100644 --- a/sandboxd/egress/socks.go +++ b/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) } diff --git a/sandboxd/egress/socks_test.go b/sandboxd/egress/socks_test.go index 0251b500..c2f08c9b 100644 --- a/sandboxd/egress/socks_test.go +++ b/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() } diff --git a/sandboxd/engine/engine.go b/sandboxd/engine/engine.go index 53c9c6bc..5f1b8dda 100644 --- a/sandboxd/engine/engine.go +++ b/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 { diff --git a/sandboxd/engine/nicmark.go b/sandboxd/engine/nicmark.go new file mode 100644 index 00000000..64d7002a --- /dev/null +++ b/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) +} diff --git a/sandboxd/pool/egress.go b/sandboxd/pool/egress.go index 61b73659..52a8940c 100644 --- a/sandboxd/pool/egress.go +++ b/sandboxd/pool/egress.go @@ -75,7 +75,7 @@ func (m *Manager) armEgress(ctx context.Context, sb *types.Sandbox) error { // lockEgressNIC nft-locks the egress-lane NIC and records the tap for unlock. func (m *Manager) lockEgressNIC(ctx context.Context, sb *types.Sandbox) error { - if !m.lockEgress || sb.Key.Net != types.NetEgress { + if !m.locksNIC(sb.Key) { return nil } tap := sb.TAP @@ -94,6 +94,21 @@ func (m *Manager) lockEgressNIC(ctx context.Context, sb *types.Sandbox) error { return nil } +// markLockedNIC tells the guest its NIC is nft-locked; silkd cannot see the hook from inside. +func (m *Manager) markLockedNIC(ctx context.Context, key types.PoolKey, sock string) error { + if !m.locksNIC(key) { + return nil + } + if err := m.eng.MarkNICLocked(ctx, sock); err != nil { + return fmt.Errorf("mark nic locked: %w", err) + } + return nil +} + +func (m *Manager) locksNIC(key types.PoolKey) bool { + return m.lockEgress && key.Net == types.NetEgress +} + func (m *Manager) tapOf(ctx context.Context, vmName string) (string, error) { vm, ok, err := m.findVM(ctx, vmName) if err != nil { @@ -142,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 } diff --git a/sandboxd/pool/egress_test.go b/sandboxd/pool/egress_test.go index e4bd512d..659d6c1d 100644 --- a/sandboxd/pool/egress_test.go +++ b/sandboxd/pool/egress_test.go @@ -102,6 +102,58 @@ func TestArmEgressBindsSocksOnlyForTunnelRules(t *testing.T) { } } +func TestLockedLaneMarksGuestOnColdBoots(t *testing.T) { + tests := []struct { + name string + key types.PoolKey + cold bool + want int + }{ + {"golden build on the egress lane", egKey, false, 1}, + {"golden build on the none lane", testKey, false, 0}, + {"cold provision on the egress lane", egKey, true, 1}, + {"cold provision on the none lane", testKey, true, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + eng := newFakeEngine() + m := egressManager(t, eng, config.PoolSpec{PoolKey: tt.key, Warm: 1, Egress: egPolicy}) + if tt.cold { + sb, err := m.provision(t.Context(), tt.key, "") + if err != nil { + t.Fatalf("cold provision: %v", err) + } + m.destroy(t.Context(), sb.VMName) + } else if err := m.buildGoldenSteps(t.Context(), tt.key, "sbx-gb", "snap", filepath.Join(m.goldensDir(), tt.key.Hash())); err != nil { + t.Fatalf("buildGoldenSteps: %v", err) + } + if got := len(eng.nicMarks); got != tt.want { + t.Errorf("MarkNICLocked calls = %d, want %d", got, tt.want) + } + }) + } +} + +func TestGoldenNICSidecarGatesAdoption(t *testing.T) { + m := egressManager(t, newFakeEngine(), config.PoolSpec{PoolKey: egKey, Warm: 1, Egress: egPolicy}) + p := m.pools[egKey] + g := filepath.Join(m.goldensDir(), egKey.Hash()) + if err := os.MkdirAll(g, 0o750); err != nil { + t.Fatalf("stage golden: %v", err) + } + m.adoptGolden(p) + if p.goldenDir != "" { + t.Error("adopted an egress-lane golden that never marked its guest; want rebuild") + } + if err := writeGoldenSidecar(g+nicSidecarSuffix, nicStamp(true)); err != nil { + t.Fatalf("write sidecar: %v", err) + } + m.adoptGolden(p) + if p.goldenDir != g { + t.Error("rejected an egress-lane golden whose sidecar says the guest was marked") + } +} + func TestArmEgressFailsClosedWhenNICUnlockable(t *testing.T) { m := egressManager(t, newFakeEngine(), config.PoolSpec{PoolKey: egKey, Egress: egPolicy}) diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index 1f1b654a..57c9af00 100644 --- a/sandboxd/pool/pool.go +++ b/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-" @@ -73,6 +73,7 @@ const ( vmStateCreating = "creating" caSidecarSuffix = ".cafp" + nicSidecarSuffix = ".nic" warmupSidecarSuffix = ".warmup" ) @@ -91,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") @@ -116,6 +117,7 @@ type Engine interface { Probe(ctx context.Context, vsockSocket string, timeout time.Duration) error DialGuestPort(ctx context.Context, vsockSocket string, port uint16) (net.Conn, error) InstallCACert(ctx context.Context, vsockSocket string, certPEM []byte) error + MarkNICLocked(ctx context.Context, vsockSocket string) error Warmup(ctx context.Context, vsockSocket string, argv []string) error DiskAttach(ctx context.Context, vmName string, spec engine.VolumeSpec) error MountVolume(ctx context.Context, vsockSocket, name, mount string, rw bool) error @@ -689,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) diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 84f01259..64d3e014 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -813,6 +813,7 @@ type fakeEngine struct { removeSeenOps map[string][]string removeSeenDirty map[string][]string dirtyPaths []string + nicMarks []string syncs []string hibernates, restores, snapRemoves []string @@ -1059,6 +1060,13 @@ func (f *fakeEngine) InstallCACert(_ context.Context, vsockSocket string, _ []by return f.installCAErr } +func (f *fakeEngine) MarkNICLocked(_ context.Context, vsockSocket string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.nicMarks = append(f.nicMarks, vsockSocket) + return nil +} + func (f *fakeEngine) Warmup(_ context.Context, sock string, argv []string) error { f.mu.Lock() defer f.mu.Unlock() diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index a4bb8bcd..dfe3e669 100644 --- a/sandboxd/pool/reconcile.go +++ b/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 @@ -165,7 +165,7 @@ func (m *Manager) resyncEgress(ctx context.Context, live map[string]types.VMReco var quarantine []*types.Sandbox for _, sb := range m.claimed { sb.TouchAt(now) - if m.lockEgress && sb.Key.Net == types.NetEgress { + if m.locksNIC(sb.Key) { tap := m.readoptEgressTap(sb, live) if tap == "" { logger.Errorf(ctx, errNoEgressTap, "egress claim %s has no lockable tap; quarantining", sb.ID) @@ -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) diff --git a/sandboxd/pool/refill.go b/sandboxd/pool/refill.go index c794e614..dd9f18f8 100644 --- a/sandboxd/pool/refill.go +++ b/sandboxd/pool/refill.go @@ -191,6 +191,9 @@ func (m *Manager) buildGoldenSteps(ctx context.Context, key types.PoolKey, name, if err != nil { return err } + if err = m.markLockedNIC(ctx, key, sock); err != nil { + return err + } caBaked := m.poolIntercepts(key) if caBaked { if err = m.eng.InstallCACert(ctx, sock, m.egressCA.CertPEM()); err != nil { @@ -210,6 +213,9 @@ func (m *Manager) buildGoldenSteps(ctx context.Context, key types.PoolKey, name, if err := m.writeGoldenCASidecar(final, caBaked); err != nil { return err } + if err := writeGoldenSidecar(final+nicSidecarSuffix, nicStamp(m.locksNIC(key))); err != nil { + return err + } return writeGoldenSidecar(final+warmupSidecarSuffix, warmupStamp(warmup)) } @@ -245,6 +251,7 @@ func (m *Manager) writeGoldenCASidecar(final string, caBaked bool) error { func (m *Manager) adoptGolden(p *pool) { g := filepath.Join(m.goldensDir(), p.hash) if dirExists(g) && m.goldenCAMatches(g, m.poolEgress[p.key].Intercepts()) && + goldenSidecarMatches(g+nicSidecarSuffix, nicStamp(m.locksNIC(p.key))) && goldenSidecarMatches(g+warmupSidecarSuffix, warmupStamp(m.poolWarmups[p.key])) { p.goldenDir = g } @@ -302,26 +309,34 @@ func (m *Manager) exportSource(ctx context.Context, sb *types.Sandbox, exportDir // provision creates one claim-ready VM, cloning from a golden when available. func (m *Manager) provision(ctx context.Context, key types.PoolKey, golden string) (*types.Sandbox, error) { if golden == "" { - sb, err := m.provisionVM(ctx, key, coldProbeTimeout, func(name string) (types.VMRecord, error) { - return m.eng.RunCold(ctx, name, key) - }) - if err != nil { - return nil, err - } - // a pre-golden cold claim must trust the root, or intercepted hosts fail TLS for its life - if m.poolIntercepts(key) { - if err := m.eng.InstallCACert(ctx, sb.VsockSocket, m.egressCA.CertPEM()); err != nil { - m.destroy(ctx, sb.VMName) - return nil, fmt.Errorf("install egress ca: %w", err) - } - } - return sb, nil + return m.provisionCold(ctx, key) } return m.provisionVM(ctx, key, claimProbeTimeout, func(name string) (types.VMRecord, error) { return m.eng.Clone(ctx, golden, name, key) }) } +func (m *Manager) provisionCold(ctx context.Context, key types.PoolKey) (*types.Sandbox, error) { + sb, err := m.provisionVM(ctx, key, coldProbeTimeout, func(name string) (types.VMRecord, error) { + return m.eng.RunCold(ctx, name, key) + }) + if err != nil { + return nil, err + } + if err := m.markLockedNIC(ctx, key, sb.VsockSocket); err != nil { + m.destroy(ctx, sb.VMName) + return nil, err + } + // a pre-golden cold claim must trust the root, or intercepted hosts fail TLS for its life + if m.poolIntercepts(key) { + if err := m.eng.InstallCACert(ctx, sb.VsockSocket, m.egressCA.CertPEM()); err != nil { + m.destroy(ctx, sb.VMName) + return nil, fmt.Errorf("install egress ca: %w", err) + } + } + return sb, nil +} + func (m *Manager) provisionVM(ctx context.Context, key types.PoolKey, probeTimeout time.Duration, create vmProvisioner) (*types.Sandbox, error) { sb, err := m.startVM(ctx, key, create) if err != nil { @@ -488,3 +503,10 @@ func goldenSidecarMatches(path, stamp string) bool { func warmupStamp(argv []string) string { return strings.Join(argv, "\x00") } + +func nicStamp(locked bool) string { + if locked { + return "locked" + } + return "" +} diff --git a/sdk/langchain/cocoonsandbox_langchain/toolkit.py b/sdk/langchain/cocoonsandbox_langchain/toolkit.py index bf9d1e81..8c279f14 100644 --- a/sdk/langchain/cocoonsandbox_langchain/toolkit.py +++ b/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]: diff --git a/sdk/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index 943f5d51..bedc4f23 100644 --- a/sdk/python/cocoonsandbox/client.py +++ b/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 diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index 0f357330..f6dcf450 100644 --- a/sdk/python/cocoonsandbox/sandbox.py +++ b/sdk/python/cocoonsandbox/sandbox.py @@ -34,7 +34,7 @@ def __init__( from_checkpoint: str = "", template_digest: str = "", volumes: list[dict] | None = None, - ): + ) -> None: self._client = client self.id = id self.token = token @@ -426,7 +426,7 @@ def _drain_proc( class Session(_Closeable): """A persistent shell inside the sandbox, addressed by id.""" - def __init__(self, sandbox: Sandbox, id: str): + def __init__(self, sandbox: Sandbox, id: str) -> None: self._sandbox = sandbox self.id = id @@ -441,7 +441,7 @@ def close(self) -> None: class Watcher(_Closeable): """A live filesystem event stream; iterate for {kind, path} events.""" - def __init__(self, conn: Conn): + def __init__(self, conn: Conn) -> None: self._conn = conn self.error: Exception | None = None @@ -463,7 +463,7 @@ def close(self) -> None: class Pty(_Closeable): """An interactive shell under a guest pty; read/write are raw bytes.""" - def __init__(self, sandbox: Sandbox, conn: Conn, pid: int): + def __init__(self, sandbox: Sandbox, conn: Conn, pid: int) -> None: self._sandbox = sandbox self._conn = conn self.pid = pid @@ -492,7 +492,7 @@ class Lsp(_Closeable): """A language server in the sandbox, spoken to over the relay. silkd is a broker: it pipes JSON-RPC bytes; the caller frames and correlates.""" - def __init__(self, sandbox: Sandbox, server_id: str): + def __init__(self, sandbox: Sandbox, server_id: str) -> None: self._sandbox = sandbox self.server_id = server_id @@ -513,7 +513,7 @@ def stop(self) -> None: class PortConn(_Closeable): """A byte stream to a guest port, relayed over the silkd connection.""" - def __init__(self, conn: Conn): + def __init__(self, conn: Conn) -> None: self._conn = conn self._eof = False diff --git a/silkd/src/find.rs b/silkd/src/find.rs index 104a2039..72cd703b 100644 --- a/silkd/src/find.rs +++ b/silkd/src/find.rs @@ -18,7 +18,7 @@ pub const FIND_MAX_FILE: u64 = 8 * 1024 * 1024; /// Match frames in flight between the walking thread and the writer. const MATCH_QUEUE: usize = 256; -/// Bytes of match content in flight; 256 size-bound single-line frames would pin gigabytes. +/// Bytes of match content in flight: above the largest single match, so a reserve never blocks forever, and far below the gigabytes 256 size-bound frames would pin. const MATCH_QUEUE_BYTES: usize = 2 * FIND_MAX_FILE as usize; struct MatchBudget { diff --git a/silkd/src/net.rs b/silkd/src/net.rs index 6d94b4f8..f2110f43 100644 --- a/silkd/src/net.rs +++ b/silkd/src/net.rs @@ -1,8 +1,12 @@ //! Guest network-lane detection: the egress lane has a device-backed NIC, the none lane only virtual interfaces. +use std::path::Path; use std::sync::LazyLock; use std::sync::atomic::{AtomicI8, Ordering}; +/// Marker the host writes when it has nft-locked the guest's NIC; on the root filesystem so a guest reboot keeps it. +const NIC_LOCKED_MARK: &str = "/etc/silkd-nic-locked"; + static LANE_OVERRIDE: AtomicI8 = AtomicI8::new(-1); /// Reports whether the guest can reach a network; only a `device`-backed interface counts, since the kernel auto-creates virtual tunnels. @@ -27,6 +31,11 @@ pub fn has_egress() -> bool { *DEVICE_BACKED } +/// Reports whether execs route directly: a NIC the host has not locked. +pub fn routes_directly() -> bool { + has_egress() && !Path::new(NIC_LOCKED_MARK).exists() +} + /// Lane override for tests: set_var would race every concurrent getenv. pub fn override_egress_for_tests(lane: Option) { LANE_OVERRIDE.store(lane.map_or(-1, i8::from), Ordering::Relaxed); diff --git a/silkd/src/server.rs b/silkd/src/server.rs index 46ca0e1b..4971018e 100644 --- a/silkd/src/server.rs +++ b/silkd/src/server.rs @@ -96,12 +96,12 @@ impl State { proto::write_frame(&mut writer, &Response::SessionCreated { id }).await } Err(e) => { - proto::error_frame( - &mut writer, - ErrorKind::Internal, - format!("session create: {e}"), - ) - .await + let kind = if e.kind() == std::io::ErrorKind::InvalidInput { + ErrorKind::BadRequest + } else { + ErrorKind::Internal + }; + proto::error_frame(&mut writer, kind, format!("session create: {e}")).await } } } diff --git a/silkd/src/session.rs b/silkd/src/session.rs index b0f2ecfc..b2ac70e5 100644 --- a/silkd/src/session.rs +++ b/silkd/src/session.rs @@ -42,6 +42,12 @@ impl Table { Some(s) if !s.is_empty() => s, _ => format!("sess-{}", sysutil::tmp_suffix()), }; + if let Some(name) = env.keys().find(|k| !is_env_name(k)) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("environment name {name:?} is not a shell identifier"), + )); + } let mut child = Command::new("bash") .env_clear() .envs(sysutil::base_env()) @@ -82,7 +88,7 @@ impl Table { shell_quote_into(&mut init, k); init.push('='); shell_quote_into(&mut init, v); - init.push('\n'); + init.push_str(" || exit 1\n"); } { let mut io = session.io.lock().await; @@ -287,10 +293,28 @@ fn shell_quote_into(out: &mut String, s: &str) { out.push('\''); } +fn is_env_name(name: &str) -> bool { + let mut chars = name.chars(); + chars + .next() + .is_some_and(|c| c == '_' || c.is_ascii_alphabetic()) + && chars.all(|c| c == '_' || c.is_ascii_alphanumeric()) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn is_env_name_accepts_only_shell_identifiers() { + for ok in ["A", "_x", "HTTP_PROXY", "a1"] { + assert!(is_env_name(ok), "{ok:?}"); + } + for bad in ["", "1a", "A B", "a-b", "a=b", "é"] { + assert!(!is_env_name(bad), "{bad:?}"); + } + } + #[test] fn shell_quote_into_pins_exact_output() { let cases = [ diff --git a/silkd/src/sysutil.rs b/silkd/src/sysutil.rs index ca42a065..64b3b053 100644 --- a/silkd/src/sysutil.rs +++ b/silkd/src/sysutil.rs @@ -15,7 +15,7 @@ static NSS_LOCK: Mutex<()> = Mutex::new(()); static TMP_SEQ: AtomicU64 = AtomicU64::new(0); -/// Forwarded into every exec: the loopback proxy relay is the no-NIC lane's only way out. +/// Forwarded into every exec that has no direct route: the loopback proxy relay is its only way out. const FORWARDED: [&str; 6] = [ "http_proxy", "https_proxy", @@ -62,14 +62,14 @@ pub fn signal_pid(pid: u32, sig: i32) { } } -/// The environment every exec starts from; the proxy snapshot rides only on the no-network lane. +/// The environment every exec starts from; the proxy snapshot rides only where nothing routes directly. pub fn base_env() -> Vec<(&'static str, &'static str)> { - compose_env(crate::net::has_egress(), proxy_vars()) + compose_env(crate::net::routes_directly(), proxy_vars()) } -/// Applies base_env's lane rule to an env-inheriting child: an own NIC drops the proxy variables. +/// Applies base_env's lane rule to an env-inheriting child: a routed NIC drops the proxy variables. pub fn align_proxy_env(cmd: &mut Command) { - align_proxy_env_for(cmd, crate::net::has_egress()); + align_proxy_env_for(cmd, crate::net::routes_directly()); } /// Resolves a username via getpwnam and de-escalates the command onto it. @@ -212,8 +212,8 @@ fn send_signal(target: libc::pid_t, sig: i32) { unsafe { libc::kill(target, sig) }; } -fn align_proxy_env_for(cmd: &mut Command, nic: bool) { - if !nic { +fn align_proxy_env_for(cmd: &mut Command, direct: bool) { + if !direct { return; } for key in FORWARDED { @@ -221,7 +221,10 @@ fn align_proxy_env_for(cmd: &mut Command, nic: bool) { } } -fn compose_env<'a>(nic: bool, proxy: &'a [(&'static str, String)]) -> Vec<(&'static str, &'a str)> { +fn compose_env<'a>( + direct: bool, + proxy: &'a [(&'static str, String)], +) -> Vec<(&'static str, &'a str)> { let mut env = vec![ ( "PATH", @@ -229,7 +232,7 @@ fn compose_env<'a>(nic: bool, proxy: &'a [(&'static str, String)]) -> Vec<(&'sta ), ("TERM", "xterm-256color"), ]; - if !nic { + if !direct { env.extend(proxy.iter().map(|(k, v)| (*k, v.as_str()))); } env @@ -319,25 +322,25 @@ mod tests { } #[test] - fn base_env_forwards_nothing_when_a_nic_is_present() { + fn base_env_forwards_nothing_when_the_nic_routes() { let proxy = snapshot_proxy(|_| Some("http://127.0.0.1:3128".to_string())); let env = compose_env(true, &proxy); assert!( !env.iter().any(|(k, _)| *k == "http_proxy"), - "proxy variables must stay off a lane with its own NIC", + "proxy variables must stay off a lane that routes directly", ); } #[test] - fn align_proxy_env_scrubs_only_on_a_nic_lane() { + fn align_proxy_env_scrubs_only_where_the_nic_routes() { let removed = |cmd: &Command| cmd.as_std().get_envs().filter(|(_, v)| v.is_none()).count(); - let mut with_nic = Command::new("true"); - align_proxy_env_for(&mut with_nic, true); - assert_eq!(removed(&with_nic), FORWARDED.len()); + let mut direct = Command::new("true"); + align_proxy_env_for(&mut direct, true); + assert_eq!(removed(&direct), FORWARDED.len()); - let mut without_nic = Command::new("true"); - align_proxy_env_for(&mut without_nic, false); - assert_eq!(removed(&without_nic), 0); + let mut relayed = Command::new("true"); + align_proxy_env_for(&mut relayed, false); + assert_eq!(removed(&relayed), 0); } #[test] diff --git a/silkd/tests/session_e2e.rs b/silkd/tests/session_e2e.rs index 5ec58fe1..b2969826 100644 --- a/silkd/tests/session_e2e.rs +++ b/silkd/tests/session_e2e.rs @@ -72,9 +72,9 @@ async fn session_applies_cwd_and_env_at_create() { } #[tokio::test] -async fn session_create_with_hostile_env_key_still_answers() { +async fn session_create_rejects_an_env_name_bash_cannot_export() { let state = Arc::new(State::new()); - let created = tokio::time::timeout( + let f = tokio::time::timeout( Duration::from_secs(8), one( &state, @@ -83,10 +83,33 @@ async fn session_create_with_hostile_env_key_still_answers() { ) .await .expect("session_create hung on a hostile env key"); - assert_eq!(type_of(&created[0]), "session_created", "{created:?}"); - let id = created[0]["id"].as_str().unwrap(); - let got = sh(&state, id, &["sh", "-c", "echo $GOOD"]).await; - assert_eq!(stdout_body(&got).trim(), "ok", "{got:?}"); + assert_eq!(type_of(&f[0]), "error", "{f:?}"); + assert_eq!(f[0]["kind"], "bad_request", "{f:?}"); + assert!( + state.sessions.list().is_empty(), + "{:?}", + state.sessions.list() + ); +} + +#[tokio::test] +async fn session_create_fails_when_bash_refuses_an_export() { + let state = Arc::new(State::new()); + let f = tokio::time::timeout( + Duration::from_secs(8), + one( + &state, + &json!({"op":"session_create","env":{"UID":"1"}}).to_string(), + ), + ) + .await + .expect("session_create hung on a readonly variable"); + assert_eq!(type_of(&f[0]), "error", "{f:?}"); + assert!( + state.sessions.list().is_empty(), + "{:?}", + state.sessions.list() + ); } #[tokio::test]