diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 3e53c5c2..6ee70b0c 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -7,6 +7,7 @@ on: - "sdk/openai/**" - "sdk/langchain/**" - "mcp/**/*.py" + - "e2e/**/*.py" - "ruff.toml" - "protocol/**" - ".github/workflows/python.yml" @@ -16,6 +17,7 @@ on: - "sdk/openai/**" - "sdk/langchain/**" - "mcp/**/*.py" + - "e2e/**/*.py" - "ruff.toml" - "protocol/**" diff --git a/README.md b/README.md index b3e310e4..79deb737 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ MicroVM sandboxes for AI agents, built on [cocoon](https://github.com/cocoonstack/cocoon): a fast-boot guest stack, an in-guest product daemon, a per-node control plane with warm pools, and SDKs for Go and Python. Warm claims are sub-millisecond; a pool miss clones from a -golden snapshot in tens of milliseconds; cold boot is ~200ms on bare metal. +golden snapshot in tens of milliseconds; cold boot is ~0.2–0.4 s on bare metal. ``` SDK (Go/Python) sandboxd (per node) guest microVM @@ -74,16 +74,18 @@ performance) — source in - `scripts/` — `boot-bench.sh` (boot phase timing), `bench.sh` (the published benchmark procedure), `sandboxd-e2e.sh` (bare-metal e2e, below), plus the `archive`/`egress`/`intercept`/`socks` e2e drivers +- `packaging/` — the systemd unit deploy installs ## Build & test ```bash -make help # this list +make help # every target; the ones below are the daily set make lint test # Rust: boot/init + silkd (fmt --check, clippy -D warnings, tests) make go-lint # Go: protocol/wire + sandboxd + sdk/go + e2e + mcp, GOOS linux AND darwin make go-test # Go: go test -race across the Go modules make sh-lint # shellcheck every tracked shell script make sandboxd # build dist/sandboxd +make bench # claim-tier + data-plane benchmarks on this node make boot # kernel + initramfs artifact image (docker) # KERNEL_MIRROR=… if kernel.org tarball paths 404 locally make silkd-image # silkd release binary in a scratch carrier image @@ -148,9 +150,9 @@ Boot contract (cmdline keys consumed by sandbox-init): | cmdline key | meaning | |---|---| -| `cocoon.layers=a,b,…` | EROFS layer disks resolved from virtio-blk serials, lowerdir order | +| `cocoon.layers=a,b,…` | EROFS layer disks resolved from virtio-blk serials (or `/dev/vdX` paths, as the Firecracker lane passes them), lowerdir order | | `cocoon.cow=x` | writable ext4 COW disk (same resolution rules) | -| `cocoon.timeout=10` | per-disk wait budget, seconds | +| `cocoon.timeout=10` | wait budget for the whole disk set, seconds (NIC MACs get a fixed 200 ms) | | `cocoon.hostname=h` | set via `sethostname(2)` before handoff | | `ip=addr::gw:mask:host:ethN:off[:dns0[:dns1]]` | cocoon CNI static config: persisted as a MAC-matched networkd unit in the new root (not applied in the initramfs); absent → the image's DHCP fallback covers the NIC | | `sandbox.init=/path` | handoff target, default `/sbin/init` | diff --git a/boot/init/src/boot.rs b/boot/init/src/boot.rs index a69f5beb..0ac85470 100644 --- a/boot/init/src/boot.rs +++ b/boot/init/src/boot.rs @@ -202,9 +202,11 @@ fn mkdir_all(path: &str) -> Result<(), String> { fs::create_dir_all(path).map_err(|err| format!("mkdir {path}: {err}")) } -/// Resolves every virtio-blk serial in one sysfs sweep per poll iteration. fn resolve_disks(ids: &[&str], timeout: Duration) -> Result, String> { - let found = poll_slots(ids.len(), timeout, |found| scan_serials(ids, found)); + let found = poll_slots(ids.len(), timeout, |found| { + record_devices(ids, found); + scan_serials(ids, found); + }); if found.iter().all(Option::is_some) { return Ok(found.into_iter().flatten().collect()); } @@ -246,6 +248,15 @@ fn scan_serials(ids: &[&str], found: &mut [Option]) { } } +// cocoon's Firecracker lane names the device node itself; there is no virtio serial to match +fn record_devices(ids: &[&str], found: &mut [Option]) { + for (slot, id) in found.iter_mut().zip(ids) { + if slot.is_none() && id.starts_with("/dev/") && Path::new(id).exists() { + *slot = Some((*id).into()); + } + } +} + fn record_serial(ids: &[&str], found: &mut [Option], serial: &str, device: &str) { for (i, id) in ids.iter().enumerate() { if found[i].is_none() && *id == serial && Path::new(device).exists() { @@ -281,4 +292,15 @@ mod tests { record_serial(&ids, &mut found, "layer", "/dev/null"); assert_eq!(found[0].as_deref(), Some("/dev/null")); } + + #[test] + fn device_path_ids_resolve_once_the_node_exists() { + let ids = ["/dev/null", "/dev/sandbox-init-missing-device", "layer"]; + let mut found = [None, None, None]; + + record_devices(&ids, &mut found); + assert_eq!(found[0].as_deref(), Some("/dev/null")); + assert!(found[1].is_none()); + assert!(found[2].is_none()); + } } diff --git a/boot/init/src/cfg.rs b/boot/init/src/cfg.rs index ad466c9f..6150e02a 100644 --- a/boot/init/src/cfg.rs +++ b/boot/init/src/cfg.rs @@ -14,7 +14,7 @@ pub struct BootCfg { pub layers: Vec, /// Writable ext4 COW disk ID (same resolution rules as layers). pub cow: String, - /// Per-device wait budget. + /// Wait budget for the whole disk set; NICs get a fixed 200 ms. pub timeout: Duration, pub hostname: Option, /// Static per-NIC config, persisted as networkd units and never applied in the initramfs. @@ -27,7 +27,7 @@ pub struct BootCfg { pub trace: bool, } -/// One `ip=::::::off[:dns0[:dns1]]` param, as cocoon emits it per NIC. +/// One kernel `ip=` param, as cocoon emits it per NIC. #[derive(Debug, PartialEq, Eq)] pub struct IpParam { pub addr: String, @@ -123,7 +123,6 @@ fn params(cmdline: &str) -> impl DoubleEndedIterator { .map(|tok| tok.split_once('=').unwrap_or((tok, ""))) } -/// sandbox.debug value semantics, shared by parse() and debug_requested(). fn debug_token(val: &str) -> bool { val.is_empty() || val == "1" } @@ -136,9 +135,7 @@ fn parse_ip_param(val: &str) -> Option { } let prefix = mask_to_prefix(f[3])?; let gateway = (!f[2].is_empty() && f[2] != "0.0.0.0").then(|| f[2].to_string()); - let dns = f - .get(7..) - .unwrap_or(&[]) + let dns = f[7..] .iter() .filter(|d| !d.is_empty() && **d != "0.0.0.0") .map(|d| d.to_string()) diff --git a/boot/init/src/sys.rs b/boot/init/src/sys.rs index b2470138..3a84cd24 100644 --- a/boot/init/src/sys.rs +++ b/boot/init/src/sys.rs @@ -5,7 +5,6 @@ use std::io; use std::ptr; use std::time::Duration; -/// Mount flags for the kernel API filesystems. pub const MNT_SECURE: libc::c_ulong = libc::MS_NOSUID | libc::MS_NODEV | libc::MS_NOEXEC; pub fn mount( @@ -83,7 +82,7 @@ pub fn exec_init(path: &str) -> String { io::Error::last_os_error().to_string() } -/// Routes PID 1 stdio to /dev/console, covering a cpio built without the console node. +/// Routes PID 1 stdio to /dev/console once devtmpfs is mounted. pub fn claim_console() { // SAFETY: the literal is a live 'static CStr; the fd is validated (>=0) // before any dup2/close and only closed when it is not already a std stream. diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 29a07ec1..4a7774c3 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -265,7 +265,7 @@ powersave, whose C-state/freq-ramp latency lands in the tail. The guest is up 0.26s (its own /proc/uptime) when the first cold exec returns — silkd answers long before a console login prompt would appear. -### 2026-07-10 — bare metal +### 2026-07-10 — bare metal (commit not recorded) | environment | | |---|---| @@ -285,7 +285,7 @@ answers long before a console login prompt would appear. | fs_stat RTT (dial per RPC) | n=200 p50=0.22ms p90=1.71ms p99=3.40ms | | fs_pull throughput (128 MiB) | 595.9 MiB/s best of 3 | -### 2026-07-08 — bare metal +### 2026-07-08 — bare metal (commit not recorded) | environment | | |---|---| @@ -305,7 +305,7 @@ answers long before a console login prompt would appear. | fs_stat RTT (dial per RPC) | n=200 p50=0.17ms p90=0.22ms p99=0.26ms | | fs_pull throughput (128 MiB) | 232.6 MiB/s best of 3 | -### 2026-07-07 — nested (google) +### 2026-07-07 — nested (google, commit not recorded) | environment | | |---|---| diff --git a/docs/deploy.md b/docs/deploy.md index 5afba5a1..9a32394b 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -107,13 +107,13 @@ sandboxd reads one JSON file (`-config`, default | `egress_internal_allow` | unset | CIDR prefixes re-admitted through the egress proxy's SSRF guard, node-wide (every pool and tenant). Prefixes, not a permit-private switch — the guest bridges are themselves ULA/RFC1918. See [egress](egress.md) | | `egress_ca` | unset | [HTTPS-interception](egress.md#https-interception) PKI: `root_cert` (the cluster root baked into intercepted guests; may bundle old+new roots during rotation) plus this node's `intermediate_cert`/`intermediate_key` from `sandboxd ca issue-intermediate`. Required when any pool rule sets `intercept` | | `api_token` | unset | the operator (root) credential: when set, guards the node-level endpoints (Bearer) with full access, including release-by-id cleanup. Per-sandbox tokens guard ordinary sandbox-scoped calls | -| `tenants` | unset | multi-tenant tokens next to `api_token`: `[{"name": "acme", "token": "…", "max_claims": 50}]`. A tenant token reaches the resource-creating verbs (claim, fork, promote, checkpoint, preview), catalog discovery, and its own sandbox/checkpoint listings; everything it creates is stamped with the tenant name. Root-only surfaces (per-id sandbox reads, `GET /v1/checkpoints/{id}/blob`, `GET /v1/info`, `PUT /v1/pools`, `POST/DELETE /v1/drain`, `/metrics`) answer it 403. `max_claims` (0 = unlimited) caps that tenant's live claims next to the node-wide cap. Requires `api_token` set. Names and tokens must be unique, tokens distinct from `api_token`. On a cluster all nodes must carry the same tenants set (the SDK replays whichever token authorized a redirect), and per-node caps mean a tenant's effective cluster limit is `max_claims` × nodes. Empty = exactly the single-token behavior | +| `tenants` | unset | multi-tenant tokens next to `api_token`: `[{"name": "acme", "token": "…", "max_claims": 50, "egress": {…}}]` (the optional `egress` policy is intersected with the pool's, see [egress](egress.md)). A tenant token reaches the resource-creating verbs (claim, fork, promote, checkpoint, preview), catalog discovery, and its own sandbox/checkpoint listings; everything it creates is stamped with the tenant name. Root-only surfaces (per-id sandbox reads and stats, `GET /v1/checkpoints/{id}/blob`, `GET /v1/info`, `PUT /v1/pools`, `POST/DELETE /v1/drain`, `/metrics`) answer it 403. `max_claims` (0 = unlimited) caps that tenant's live claims next to the node-wide cap. Requires `api_token` set. Names and tokens must be unique, tokens distinct from `api_token`. On a cluster all nodes must carry the same tenants set (the SDK replays whichever token authorized a redirect), and per-node caps mean a tenant's effective cluster limit is `max_claims` × nodes. Empty = exactly the single-token behavior | | `max_fork_count` | 16 | children a single `fork` may create; each is a full-RAM VM, so this bounds one request's memory blast radius to the node's capacity | | `refill_concurrency` | 0 (auto) | concurrent VM provisioning budget, shared by warm-pool refills, fork clones, and the reap/hibernate/reconcile engine batches. 0 sizes it from the node: `NumCPU*2/3` clamped to [4, 256] — a 384-core node gets 256; small nodes keep a floor of 4 | | `release_delay_seconds` | 0 | seconds a released VM waits in the removal queue before `cocoon vm rm`. The claim is dropped and journaled and its egress listener closed at release; the VM removal, the volume hold release and the egress tap unlock run on the first reap tick (5 s) after the delay, on the `refill_concurrency` budget like every other batch teardown, so a burst of releases does not compete with the claims still running. Until then the VM holds its memory and its volume reservations without counting as a claim. 0 removes inline | | `preview_listen` | (off) | address for a preview HTTP server that serves guest ports under signed URLs; needs `preview_secret` | | `preview_secret` | — | cluster-shared HMAC secret signing preview tokens (all nodes share one) | -| `preview_advertise` | = `preview_listen` | the browser-facing preview base URL; nodes behind one TLS proxy may share it, while signed tokens route internally through each owner's `advertise_addr` | +| `preview_advertise` | = `preview_listen` | the browser-facing preview base URL, minted into every preview URL, so it must name a routable host (a wildcard `preview_listen` needs it set explicitly); nodes behind one TLS proxy may share it, while signed tokens route internally through each owner's `advertise_addr` | | `checkpoint_dir` | `/checkpoints` | where checkpoints and promoted templates live. Point it at a shared FUSE mount (JuiceFS over object storage, NFS) and every node sharing the mount can branch every checkpoint — records are generation-addressed with `meta.json` as the atomic commit pointer, so no cross-node locking is required of the filesystem. A re-publish retains its superseded export generation for at least ~1h and until a following hourly sweep so an in-flight clone that resolved the old metadata can finish; budget the current generation plus every generation retained across that grace-and-sweep window. An explicit delete can make a concurrent clone fail visibly. One contract on any shared root (mount or bucket): a template key has a single writer — promotes go to the sandbox's owner node, and operators must not race promotes of one name from different nodes (checkpoint ids are node-generated and never collide) | | `checkpoint_store` | dir | checkpoint AND promoted-template backend (both live in one store root, id-namespaced ck_/tp_): `{"kind": "s3", "s3": {"bucket": "…", "prefix": "ck/", "endpoint": "…", "region": "…", "force_path_style": true}}` stores checkpoints in object storage (any node claims any checkpoint, no shared mount needed). Credentials come from the standard AWS chain (env/IAM role), never this file. Re-publish retains prior export generations until Delete so an in-flight fetch that selected old metadata can finish; budget storage for those generations. An explicit S3 Delete can still make a concurrent fetch that has not finished materializing fail visibly. A crash between upload and the meta.json commit marker leaves orphan objects invisible to listings — add an S3 lifecycle rule to reclaim them. Absent = the dir backend at `checkpoint_dir` | | `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 | diff --git a/docs/desktop.md b/docs/desktop.md index 55063383..f917fbb6 100644 --- a/docs/desktop.md +++ b/docs/desktop.md @@ -31,7 +31,7 @@ before it joins the warm pool, so the snapshot already carries a listening server: ```json -"warmup": ["bash", "-lc", "for i in $(seq 1 1100); do curl -sf -m 2 -o /dev/null http://127.0.0.1:5000/screenshot && exit 0; sleep 0.1; done; exit 1"] +"warmup": ["bash", "-lc", "end=$((SECONDS+100)); until curl -sf -m 2 -o /dev/null http://127.0.0.1:5000/screenshot; do [ $SECONDS -ge $end ] && exit 1; sleep 0.1; done"] ``` Measured on a 192-core node running the OSWorld task set at 96 concurrent diff --git a/docs/egress.md b/docs/egress.md index 265573a7..78851cba 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -3,8 +3,9 @@ A sandbox gets the **effect** of a credential, never the credential. Outbound access is an allow-listed, audited resource enforced on the host: the secret lives host-side and enters no guest memory, so prompt injection can exfiltrate -at most the proxy's answers, and every credentialed call lands in the audit and -usage journals keyed by sandbox and tenant. Default-deny. +at most the proxy's answers, and every credentialed call lands in the usage +journal keyed by sandbox and tenant (and in the audit journal when `audit_log` +is on). Default-deny. The same host proxy also serves the **none lane** (a NIC-less Cloud Hypervisor guest) over vsock, so a network-less sandbox can call approved APIs with @@ -61,7 +62,7 @@ opts in with rules that all carry a nonempty `methods` list without `CONNECT`, or all `intercept`, is rejected at load. A pool whose policy does not opt in pays nothing for the door on the claim path, whatever its tenants' policies say. -Audit lines carry `"method":"SOCKS5"`. +Audit lines (with `audit_log` on) carry `"method":"SOCKS5"`. ```sh curl --socks5-hostname 127.0.0.1:1080 imaps://imap.example.com/ # allowed: {"socks5": true, "allow": [{"host": "imap.example.com"}]} @@ -203,9 +204,10 @@ woken sandbox binds at arm time. method and the secret injected (see below). Only a pool rule may set it. - No policy on a claim ⇒ no egress at all (the proxy is not started). -Each decision is written to `audit.jsonl` (`op:"egress"`, `dest`, `port`, -`decision`, and the secret **name** in `secret`) and metered as an `egress` -usage event. +Each decision is metered as an `egress` usage event and, when `audit_log` is +on, written to `audit.jsonl` (`op:"egress"`, `dest`, `port`, `method`, +`decision`, and the secret **name** in `secret`); an intercepted `CONNECT` is +recorded as a decision of its own before its inner requests. ## HTTPS interception diff --git a/docs/index.md b/docs/index.md index cee04e63..7b3130c9 100644 --- a/docs/index.md +++ b/docs/index.md @@ -3,7 +3,7 @@ MicroVM sandboxes for AI agents, built on [cocoon](https://github.com/cocoonstack/cocoon). Warm claims are sub-millisecond; a pool miss clones from a golden snapshot in tens of -milliseconds; cold boot is ~200ms on bare metal. +milliseconds; cold boot is ~0.2–0.4 s on bare metal. ``` SDK (Go/Python) sandboxd (per node) guest microVM diff --git a/docs/openai-adapter.md b/docs/openai-adapter.md index 0c99c610..8522b00b 100644 --- a/docs/openai-adapter.md +++ b/docs/openai-adapter.md @@ -33,7 +33,7 @@ pair over the sync [Python SDK](sdk-python.md), bridged with |---|---| | `create` | `Client.new` — one claimed sandbox per session | | session `exec` | `Sandbox.run` (stdout/stderr/exit); a per-call `timeout` is `run`'s wall clock and surfaces as `TimeoutError` | -| `read` / `write` | `Sandbox.read_file` / `write_file`; a missing path on `read` → `FileNotFoundError` | +| `read` / `write` | `Sandbox.read_file` / `write_file`; a missing path on `read` → `FileNotFoundError`; a per-call `user` raises `NotImplementedError` (fs verbs carry no user on the wire) | | `persist_workspace` / `hydrate_workspace` | `Sandbox.pull` / `push` (tar) | | exposed port | `Sandbox.proxy_port` | | `delete` | `Sandbox.close` (release) | diff --git a/docs/performance.md b/docs/performance.md index 9e162054..f1fed78a 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -24,8 +24,8 @@ bare metal, `small` tier: | cold boot (no golden yet) | **~215–400 ms** | full boot from the template image to silkd answering | A guarded-egress claim binds its proxy doors at refill rather than at claim -(#177): on bare metal that took the warm claim with the HTTP door from 307 to -263 µs p50, and with both doors from 351 to 274 µs. +(#177, cocoon-test2 bare metal, 2026-09-14): that took the warm claim with the +HTTP door from 307 to 263 µs p50, and with both doors from 351 to 274 µs. Cloud Hypervisor lifecycle latency (bare metal, vsock agent-ready): @@ -72,8 +72,9 @@ TLS edge, a handshake. From proto 2 silkd serves RPCs back to back, and both SDKs keep one relay connection per handle with a 30s idle window. `e2e/cmd/rpcbench`, warm `rt:24.04` sandbox, `net=none`, n=200 `fs_stat` -RPCs per arm, bare metal, client on the node. The two arms interleave sample -by sample and swap which one leads, so drift cannot land on one of them. +RPCs per arm, cocoon-test1 bare metal, client on the node, 2026-09-16 at +5158a14. The two arms interleave sample by sample and swap which one leads, +so drift cannot land on one of them. | per-RPC wall | plain p50 | plain p90 | TLS edge p50 | TLS edge p90 | | --- | --- | --- | --- | --- | @@ -175,7 +176,8 @@ pre-dialer declined — behind a TLS edge it tracks the dial arm, because it hides a handshake only while RPCs arrive slower than a dial completes. **Template pre-check meta GET**: a single `ReadMeta` against MinIO -measures 4.76ms cross-host (sub-ms node-local, 20-50ms on WAN S3). It +measured 4.76ms cross-host (sub-ms node-local, 20-50ms on WAN S3; a +2026-07 spot check, host and commit not recorded). It would fire once per warm-miss claim of a promoted template whose key gossip advertises, against a provision that already costs a >=48ms clone plus the export fetch. Declined until a measurement shows it matters. diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index 686a4e0c..eb6f6dce 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -201,8 +201,9 @@ permits it. `nodes` counts members advertising the name; `size_bytes` and `available` are a best-effort stat of the answering node's image. Membership is eventually consistent by one gossip tick. `writable` is the entry's catalog configuration, fleet-uniform like the access -list; the field is emitted (as `true`) only for a writable entry and omitted -otherwise, so a read-only entry's response is byte-identical to v1. +list; the field is emitted (as `true`) only for a writable entry this node +declares and omitted otherwise (peer-only rows never carry it), so a read-only +entry's response is byte-identical to v1. ## POST /v1/sandboxes/{id}/release @@ -547,7 +548,7 @@ empty `argv`, negative `timeout_seconds`, an unknown field, or a silkd `bad_request` — a command that is missing or not executable and a `cwd` that does not exist are the caller's, not 502s; 401 missing bearer token; 404 unknown sandbox or wrong token; 413 when stdout+stderr exceed 8 MiB; 502 guest unreachable or any other -silkd error; 504 when the command outlives `timeout_seconds` or the request. A hibernated sandbox wakes transparently like on the relay. +silkd error; 504 when the command outlives `timeout_seconds`. A client that hangs up first gets no reply and the command is killed. A hibernated sandbox wakes transparently like on the relay. ## GET /v1/sandboxes/{id}/owner diff --git a/docs/sdk-python.md b/docs/sdk-python.md index bddb5318..cb2641cd 100644 --- a/docs/sdk-python.md +++ b/docs/sdk-python.md @@ -11,8 +11,11 @@ with client.new("ghcr.io/cocoonstack/sandbox/rt:24.04") as sb: `pip install cocoonstack-sandbox` — stdlib-only, no dependencies, and synchronous by design (agent frameworks that need async wrap calls in `asyncio.to_thread`, exactly like the [OpenAI adapter](openai-adapter.md) -does). It matches the [Go SDK](sdk.md) guest and data-plane surface; operator -pool retuning (`SetPools`/`SetPoolsCluster`) remains Go-only. Wire fidelity is +does). It matches the [Go SDK](sdk.md) guest and data-plane surface, with two +exceptions: operator pool retuning (`SetPools`/`SetPoolsCluster`) remains +Go-only, and file and tar payloads are whole `bytes` values rather than +streams (`read_file`/`pull` return them, `write_file`/`push` take them; the +wire is chunked, the caller's copy is not). Wire fidelity is pinned by the shared protocol fixture corpus that the Rust guest, Go, and Python all round-trip in CI. @@ -175,9 +178,10 @@ egress request) is not swept; the idle clock restarts when that connection ends. Data-plane calls share a handle's relay connection: after a call the SDK keeps the connection for 30 seconds (`Client(..., keep_alive=...)` tunes the window; 0 dials per call) and the next call on that handle sends its request -on it, so a busy handle pays the dial, upgrade and TLS handshake once. A kept -connection counts as live for `idle_hibernate_seconds` until it closes, so -keep the window below that setting; `close` and `hibernate` drop it at once. +on it, so a busy handle pays the dial, upgrade and TLS handshake once. A +handle parks at most 8 idle connections. A kept connection counts as live for +`idle_hibernate_seconds` until it closes, so keep the window below that +setting; `close` and `hibernate` drop it at once. Streams (`watch`, `open_pty`, `dial_port`, an LSP session) take a connection of their own, and a guest whose silkd predates the back-to-back protocol gets one connection per call as before. @@ -295,7 +299,8 @@ listener for unmodified local tools (browsers, curl); close the returned socket to stop, and a guest port that closes ends the local connection too. `preview_url` mints a signed URL served by the node's preview listener, clamped to the claim's remaining lease — the URL dies -with the sandbox, and a node without `preview_listen` answers 501. +with the sandbox, and a node without `preview_listen` answers 501. Minting +is resource-creating and takes the api token, like fork and checkpoint. ## Node info @@ -396,10 +401,10 @@ with closing(sb.find_iter("/work", r"TODO")) as stream: # streamed; closing en for m in stream: if m["line"] > 100: break -# [{"file", "line", "content"}]; glob is anchored *? wildcards on the file name +# [{"type", "file", "line", "content"}]; glob is anchored *? wildcards on the file name results = sb.replace(["/work/main.py"], r"foo", "bar") -# [{"file", "replacements"}]; per-file atomic +# [{"type", "file", "replacements"}]; per-file atomic ``` Patterns are regular expressions evaluated in the guest — no shell quoting. @@ -408,7 +413,7 @@ Patterns are regular expressions evaluated in the guest — no shell quoting. ```python w = sb.watch("/work", recursive=True) -for ev in w: # {"kind", "path"} +for ev in w: # {"type", "kind", "path"} print(ev["kind"], ev["path"]) # created|modified|deleted|renamed w.close() ``` @@ -423,12 +428,12 @@ when the relay drops, which `w.error` tells apart from a clean close (`None`). ```python sb.git_clone(url, "/work/repo", branch="main", depth=1, auth=token) # egress lane only -st = sb.git_status("/work/repo") # {"branch", "ahead", "behind", "files", "truncated"?} +st = sb.git_status("/work/repo") # {"type", "branch", "ahead", "behind", "files", "truncated"?} sb.git_add("/work/repo", ["a.txt"]) sha = sb.git_commit("/work/repo", "message", "Dev ") sb.git_push("/work/repo", auth=token) # egress lane only sb.git_pull("/work/repo", auth=token) # egress lane only -br = sb.git_branches("/work/repo") # {"current", "branches"} +br = sb.git_branches("/work/repo") # {"type", "current", "branches"} sb.git_create_branch("/work/repo", "feature") sb.git_checkout("/work/repo", "feature") sb.git_delete_branch("/work/repo", "feature") diff --git a/docs/sdk.md b/docs/sdk.md index caeb5960..17fb3cf7 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -159,12 +159,14 @@ stream; the guest protocol and port-forwarding frames do not change. Data-plane calls share a handle's relay connection: after a call the SDK keeps the connection for 30 seconds (`WithKeepAlive` tunes the window; 0 dials per call) and the next call on that handle sends its request on it, so -a busy handle pays the dial, upgrade and TLS handshake once. A kept -connection counts as live for `idle_hibernate_seconds` until it closes, so -keep the window below that setting; `Close` and `Hibernate` drop it at once. -Long-lived streams (`Watch`, `OpenPty`, `DialPort`, an LSP session) take a -connection of their own, and a guest whose silkd predates the back-to-back -protocol gets one connection per call as before. +a busy handle pays the dial, upgrade and TLS handshake once. A handle parks +at most 8 idle connections. A kept connection counts as live for +`idle_hibernate_seconds` until it closes, so keep the window below that +setting; `Close` and `Hibernate` drop it at once. Long-lived streams +(`Watch`, `OpenPty`, `DialPort`, an LSP session) take a connection of their +own. Reuse needs a Unix client (the liveness peek is a `MSG_PEEK`); on +Windows, and against a guest whose silkd predates the back-to-back protocol, +every call dials as before. An explicit scheme in an owner, redirect, peer, or `Attach` address wins; a bare address inherits the entry client's scheme. Trust settings are shared @@ -409,8 +411,9 @@ url, err := sb.PreviewURL(ctx, 8080, 30*time.Minute) ``` Mints a signed, shareable URL serving the guest HTTP port from a plain -browser via the node's preview listener. The TTL is clamped to the claim's -remaining lease, and the URL dies with the sandbox — release or reap +browser via the node's preview listener. Minting is resource-creating and +takes the api token, like fork and checkpoint. The TTL is clamped to the +claim's remaining lease, and the URL dies with the sandbox — release or reap revokes it with no extra state. Answers 501 when the node has no `preview_listen` configured; see [deploy](deploy.md#preview-urls). @@ -489,6 +492,7 @@ Idle sessions are reaped guest-side after 30 minutes. ```go err := sb.WriteFile(ctx, "/work/a.txt", data, nil) // atomic; *uint32 mode optional data, err := sb.ReadFile(ctx, "/work/a.txt") +err = sb.ReadFileTo(ctx, "/work/big.bin", w) // streams into an io.Writer ents, err := sb.ListDir(ctx, "/work") // []wire.DirEntry{Name,Kind,Size} info, err := sb.Stat(ctx, "/work/a.txt") // wire.FileInfo{Kind,Size,Mode,MtimeEpochSecs} err = sb.Mkdir(ctx, "/work/sub", true) // parents diff --git a/e2e/cmd/androidsmoke/main.go b/e2e/cmd/androidsmoke/main.go index 0456384e..2aaafc3c 100644 --- a/e2e/cmd/androidsmoke/main.go +++ b/e2e/cmd/androidsmoke/main.go @@ -82,9 +82,8 @@ func run(addr, token, template string) error { return nil } -// The framework (zygote, system_server) starts minutes after init on a -// nested first boot; silkd answers long before. func initTree(ctx context.Context, sb *sandbox.Sandbox) error { + // the framework (zygote, system_server) starts minutes after init on a nested first boot; silkd answers long before deadline := time.Now().Add(5 * time.Minute) for { out, err := sb.Exec(ctx, "/system/bin/sh", "-c", shellPath+"; ps -A") diff --git a/e2e/cmd/crossnode/main.go b/e2e/cmd/crossnode/main.go index cfe4680b..c3a00ff3 100644 --- a/e2e/cmd/crossnode/main.go +++ b/e2e/cmd/crossnode/main.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/cocoonstack/sandbox/e2e/internal/harness" sandbox "github.com/cocoonstack/sandbox/sdk/go" ) @@ -35,14 +36,10 @@ func run(addrA, addrB, token, template string) error { ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute) defer cancel() - ca, err := sandbox.Connect(addrA, sandbox.WithAPIToken(token)) - if err != nil { - return fmt.Errorf("connect A: %w", err) - } t0 := time.Now() - sbA, err := ca.New(ctx, template, sandbox.WithNetwork(sandbox.NetNone)) + _, sbA, err := harness.Claim(ctx, addrA, token, template, sandbox.WithNetwork(sandbox.NetNone)) if err != nil { - return fmt.Errorf("claim on A: %w", err) + return fmt.Errorf("on A: %w", err) } defer func() { _ = sbA.Close() }() fmt.Printf(" A: claim %s in %.1fs\n", template, time.Since(t0).Seconds()) diff --git a/e2e/cmd/egresssmoke/main.go b/e2e/cmd/egresssmoke/main.go index edb5a5ab..270c3d58 100644 --- a/e2e/cmd/egresssmoke/main.go +++ b/e2e/cmd/egresssmoke/main.go @@ -109,7 +109,7 @@ func run(addr, token, template, wantToken, netShape, reach, nicAddr, echo string } 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) { + if out, _ := sb.Exec(ctx, "sh", "-c", "env; cat /proc/1/environ 2>/dev/null | tr '\\0' '\\n'"); strings.Contains(out, wantToken) { return fmt.Errorf("secret value leaked into the guest") } fmt.Println(" secret absent from guest env") diff --git a/e2e/cmd/interceptsmoke/main.go b/e2e/cmd/interceptsmoke/main.go index 4c20fc22..61ca61e0 100644 --- a/e2e/cmd/interceptsmoke/main.go +++ b/e2e/cmd/interceptsmoke/main.go @@ -84,7 +84,7 @@ func run(addr, token, template, echo, secret, issuer string) error { func grepLine(out, needle string) string { for line := range strings.SplitSeq(out, "\n") { - if strings.Contains(strings.ToLower(line), needle) { + if strings.Contains(strings.ToLower(line), strings.ToLower(needle)) { return line } } diff --git a/e2e/cmd/lifecycle/main.go b/e2e/cmd/lifecycle/main.go index 870efeb5..d3bec0bc 100644 --- a/e2e/cmd/lifecycle/main.go +++ b/e2e/cmd/lifecycle/main.go @@ -34,7 +34,8 @@ func main() { } func run(addr, token, template, netShape string, wait time.Duration) error { - ctx := context.Background() + ctx, cancel := context.WithTimeout(context.Background(), wait+5*time.Minute) + defer cancel() client, sb, err := harness.Claim(ctx, addr, token, template, sandbox.WithNetwork(sandbox.NetShape(netShape))) if err != nil { return err diff --git a/e2e/cmd/meshsmoke/main.go b/e2e/cmd/meshsmoke/main.go index ac7756f0..2807ee62 100644 --- a/e2e/cmd/meshsmoke/main.go +++ b/e2e/cmd/meshsmoke/main.go @@ -13,6 +13,7 @@ import ( "strings" "time" + "github.com/cocoonstack/sandbox/e2e/internal/harness" sandbox "github.com/cocoonstack/sandbox/sdk/go" ) @@ -34,14 +35,9 @@ func run(addr, peer, token, template string) error { ctx, cancel := context.WithTimeout(context.Background(), 180*time.Second) defer cancel() - client, err := sandbox.Connect(addr, sandbox.WithAPIToken(token)) + client, sb, err := harness.Claim(ctx, addr, token, template, sandbox.WithNetwork(sandbox.NetNone)) if err != nil { - return err - } - - sb, err := client.New(ctx, template, sandbox.WithNetwork(sandbox.NetNone)) - if err != nil { - return fmt.Errorf("claim via %s: %w", addr, err) + return fmt.Errorf("via %s: %w", addr, err) } defer func() { _ = sb.Close() }() if peer != "" && sb.Owner() != peer { diff --git a/e2e/cmd/rpcbench/main.go b/e2e/cmd/rpcbench/main.go index 8d1dcfd5..4da4157e 100644 --- a/e2e/cmd/rpcbench/main.go +++ b/e2e/cmd/rpcbench/main.go @@ -145,7 +145,6 @@ func run(addr, token, template, caCert string, n int) error { return nil } -// sampleDial times one RPC on a freshly dialed and upgraded connection. func sampleDial(dial func() (net.Conn, error)) (time.Duration, error) { start := time.Now() conn, err := dial() @@ -167,7 +166,6 @@ func sampleKept(ctx context.Context, sb *sandbox.Sandbox) (time.Duration, error) return time.Since(start), nil } -// statRPC drives one hand-dialed connection through one RPC and closes it. func statRPC(conn net.Conn) error { defer func() { _ = conn.Close() }() sc := silkd.NewConn(conn) diff --git a/mcp/main.go b/mcp/main.go index cb41e7f1..73a68afb 100644 --- a/mcp/main.go +++ b/mcp/main.go @@ -1,8 +1,7 @@ // sandbox-mcp is a Model Context Protocol server over stdio: it exposes the // sandbox surface (claim, exec, files, fork, checkpoint, promote, hibernate) -// as MCP tools, so MCP clients — Claude Code, Cursor, agent frameworks — -// drive real microVM sandboxes with no extra glue. One process serves one -// sandboxd endpoint, configured by flags or environment. +// as MCP tools. One process serves one sandboxd endpoint, configured by +// flags or environment. package main import ( @@ -12,17 +11,25 @@ import ( "flag" "fmt" "os" + "os/signal" + "syscall" + "time" "github.com/projecteru2/core/log" coretypes "github.com/projecteru2/core/types" ) +// shutdownGrace bounds the wait for the read loop to notice its closed stdin before main releases the claims itself. +const shutdownGrace = 10 * time.Second + func main() { ctx := context.Background() if err := log.SetupLog(ctx, &coretypes.ServerLogConfig{Level: "error"}, ""); err != nil { fmt.Fprintln(os.Stderr, "setup log:", err) os.Exit(1) } + ctx, stop := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) + defer stop() addr := flag.String("addr", cmp.Or(os.Getenv("SANDBOXD_ADDR"), "127.0.0.1:7777"), "sandboxd address") token := flag.String("token", os.Getenv("SANDBOXD_TOKEN"), "node api token") template := flag.String("template", cmp.Or(os.Getenv("SANDBOXD_TEMPLATE"), "rt:24.04"), "default template ref") @@ -32,7 +39,20 @@ func main() { if err != nil { log.WithFunc("main").Fatalf(ctx, err, "connect sandboxd") } - if err := srv.serve(ctx, bufio.NewReader(os.Stdin), os.Stdout); err != nil { - log.WithFunc("main").Fatalf(ctx, err, "serve stdio") + served := make(chan error, 1) + go func() { served <- srv.serve(ctx, bufio.NewReader(os.Stdin), os.Stdout) }() + select { + case err := <-served: + if err != nil { + log.WithFunc("main").Fatalf(ctx, err, "serve stdio") + } + case <-ctx.Done(): + // the canceled ctx ends the tool call in flight; closing stdin ends the read loop, whose defer releases every claim + _ = os.Stdin.Close() + select { + case <-served: + case <-time.After(shutdownGrace): + srv.closeBoxes() // waits for a release the read loop's defer already started + } } } diff --git a/mcp/server.go b/mcp/server.go index c78a2160..3e682aa8 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "encoding/json" + "errors" "fmt" "io" "maps" @@ -39,16 +40,17 @@ type rpcErrorBody struct { Message string `json:"message"` } -// server owns one sandboxd client and the handles minted over this stdio -// session: MCP tools address sandboxes and checkpoints by id, so the live -// handles (with their tokens) stay here. +// server owns one sandboxd client and the live handles (with their tokens) this stdio session minted. type server struct { client *sandbox.Client template string - mu sync.Mutex - boxes map[string]*sandbox.Sandbox - ckpts map[string]*sandbox.Checkpoint + mu sync.Mutex + closed bool + boxes map[string]*sandbox.Sandbox + ckpts map[string]*sandbox.Checkpoint + release sync.Once + released chan struct{} } func newServer(addr, token, template string) (*server, error) { @@ -65,6 +67,7 @@ func newServer(addr, token, template string) (*server, error) { template: template, boxes: map[string]*sandbox.Sandbox{}, ckpts: map[string]*sandbox.Checkpoint{}, + released: make(chan struct{}), }, nil } @@ -76,7 +79,7 @@ func (s *server) serve(ctx context.Context, r *bufio.Reader, w io.Writer) error for { line, err := r.ReadBytes('\n') if len(line) == 0 && err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { return nil } return err @@ -140,12 +143,11 @@ func (s *server) dispatch(ctx context.Context, req *rpcRequest) rpcResponse { func (s *server) callTool(ctx context.Context, name string, args json.RawMessage) (string, error) { ctx, cancel := context.WithTimeout(ctx, execTimeout) defer cancel() - for _, t := range tools { - if t.name == name { - return t.handler(ctx, s, args) - } + i := slices.IndexFunc(tools, func(t tool) bool { return t.name == name }) + if i < 0 { + return "", fmt.Errorf("unknown tool %q", name) } - return "", fmt.Errorf("unknown tool %q", name) + return tools[i].handler(ctx, s, args) } func (s *server) box(id string) (*sandbox.Sandbox, error) { @@ -158,22 +160,34 @@ func (s *server) box(id string) (*sandbox.Sandbox, error) { return sb, nil } -// closeBoxes releases everything this session claimed; the lease would -// otherwise hold the VMs until it expires. +// closeBoxes releases everything this session claimed and returns once the releases are done; a second caller waits for the first. func (s *server) closeBoxes() { - s.mu.Lock() - boxes := slices.Collect(maps.Values(s.boxes)) - clear(s.boxes) - s.mu.Unlock() - for _, sb := range boxes { - _ = sb.Close() - } + s.release.Do(func() { + s.mu.Lock() + s.closed = true + boxes := slices.Collect(maps.Values(s.boxes)) + clear(s.boxes) + s.mu.Unlock() + var wg sync.WaitGroup + for _, sb := range boxes { + wg.Go(func() { _ = sb.Close() }) + } + wg.Wait() + close(s.released) + }) + <-s.released } func (s *server) trackBox(sb *sandbox.Sandbox) { s.mu.Lock() - defer s.mu.Unlock() - s.boxes[sb.ID] = sb + closed := s.closed + if !closed { + s.boxes[sb.ID] = sb + } + s.mu.Unlock() + if closed { + _ = sb.Close() // claimed after the session's release ran; nothing else would ever release it + } } func (s *server) dropBox(id string) { diff --git a/mcp/server_test.go b/mcp/server_test.go index fa80a61b..0a17a606 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -10,7 +10,10 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" + "sync/atomic" "testing" + "time" "github.com/cocoonstack/sandbox/protocol/wire" "github.com/cocoonstack/sandbox/sdk/go/silkd/silkdtest" @@ -108,6 +111,75 @@ func TestReadFileStopsAtTheCap(t *testing.T) { } } +func TestTrackBoxAfterCloseReleasesTheClaim(t *testing.T) { + released := make(chan string, 1) + node := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/release") { + released <- r.URL.Path + } + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(node.Close) + addr := strings.TrimPrefix(node.URL, "http://") + srv, err := newServer(addr, "", "rt:24.04") + if err != nil { + t.Fatalf("server: %v", err) + } + srv.closeBoxes() + srv.trackBox(srv.client.Attach(addr, "sb_late", "tok")) + select { + case path := <-released: + if !strings.Contains(path, "sb_late") { + t.Errorf("released %s, want sb_late", path) + } + case <-time.After(5 * time.Second): + t.Fatal("a claim tracked after closeBoxes was never released") + } + if len(srv.boxes) != 0 { + t.Errorf("boxes = %d after a late track, want 0", len(srv.boxes)) + } +} + +func TestCloseBoxesWaitsForEveryReleaseOnce(t *testing.T) { + var releases atomic.Int32 + node := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/release") { + time.Sleep(200 * time.Millisecond) + releases.Add(1) + } + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(node.Close) + addr := strings.TrimPrefix(node.URL, "http://") + srv, err := newServer(addr, "", "rt:24.04") + if err != nil { + t.Fatalf("server: %v", err) + } + for _, id := range []string{"sb_a", "sb_b", "sb_c"} { + srv.trackBox(srv.client.Attach(addr, id, "tok")) + } + var wg sync.WaitGroup + for range 2 { + wg.Go(srv.closeBoxes) + } + wg.Wait() + if got := releases.Load(); got != 3 { + t.Errorf("releases = %d after closeBoxes returned, want 3", got) + } +} + +func TestExecRejectsAnEmptyCommand(t *testing.T) { + replies := serveLines(t, newTestServer(t, nil), + `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"create_sandbox","arguments":{}}}`, + `{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"exec","arguments":{"sandbox_id":"sb_1"}}}`, + `{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"spawn","arguments":{"sandbox_id":"sb_1","command":""}}}`) + for _, id := range []int{2, 3} { + if !strings.Contains(toolText(t, replies[id]), "command must not be empty") { + t.Errorf("reply %d accepted an empty command: %q", id, toolText(t, replies[id])) + } + } +} + func TestCreateSandboxRejectsNegativeTTL(t *testing.T) { replies := serveLines(t, newTestServer(t, nil), `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"create_sandbox","arguments":{"ttl_seconds":-5}}}`) diff --git a/mcp/tools.go b/mcp/tools.go index 75e02e12..fcc0eabc 100644 --- a/mcp/tools.go +++ b/mcp/tools.go @@ -152,8 +152,15 @@ type cmdArgs struct { Cwd string `json:"cwd"` } -// cappedOutput keeps the first execOutputCap bytes; an agent that tails a firehose must not grow this process. -// An exec keeps draining past the cap so the command runs on; a read stops there instead. +func parseCommand(s *server, raw json.RawMessage) (cmdArgs, *sandbox.Sandbox, error) { + args, sb, err := parseAndBox[cmdArgs](s, raw) + if err == nil && args.Command == "" { + return args, nil, errors.New("command must not be empty") + } + return args, sb, err +} + +// cappedOutput keeps the first execOutputCap bytes: an agent that tails a firehose must not grow this process. type cappedOutput struct { strings.Builder stopAtCap bool @@ -173,7 +180,7 @@ func (o *cappedOutput) Write(p []byte) (int, error) { } func toolExec(ctx context.Context, s *server, raw json.RawMessage) (string, error) { - args, sb, err := parseAndBox[cmdArgs](s, raw) + args, sb, err := parseCommand(s, raw) if err != nil { return "", err } @@ -195,7 +202,7 @@ func toolExec(ctx context.Context, s *server, raw json.RawMessage) (string, erro } func toolSpawn(ctx context.Context, s *server, raw json.RawMessage) (string, error) { - args, sb, err := parseAndBox[cmdArgs](s, raw) + args, sb, err := parseCommand(s, raw) if err != nil { return "", err } diff --git a/os-image/base/24.04/Dockerfile b/os-image/base/24.04/Dockerfile index 1f9e6268..ad83ec96 100644 --- a/os-image/base/24.04/Dockerfile +++ b/os-image/base/24.04/Dockerfile @@ -4,8 +4,8 @@ # local-build fallback and may lag the pin. ARG BOOT_IMAGE=ghcr.io/cocoonstack/sandbox/boot:6.18.37 # silkd carrier image (built from silkd/Dockerfile). CI overrides with the -# version-pinned tag; a silkd change refreshes this via the workflow_run chain. -ARG SILKD_IMAGE=ghcr.io/cocoonstack/sandbox/silkd:0.1.0 +# version-pinned tag from images.yml; the default is only a local-build fallback. +ARG SILKD_IMAGE=ghcr.io/cocoonstack/sandbox/silkd:0.1.11 FROM ${BOOT_IMAGE} AS boot FROM ${SILKD_IMAGE} AS silkd diff --git a/os-image/desktop/24.04/Dockerfile b/os-image/desktop/24.04/Dockerfile index 3cd5e1b4..41ca98dc 100644 --- a/os-image/desktop/24.04/Dockerfile +++ b/os-image/desktop/24.04/Dockerfile @@ -1,7 +1,7 @@ # desktop sandbox flavor: base + a GNOME session on Xvfb with the OSWorld # guest server (osworld-server) on guest loopback 5000, the OSWorld Chrome # CDP bridge on loopback 9222, and the OSWorld app set (Chrome, LibreOffice, -# GIMP, VLC). The microVM has no display device, so the session runs on a +# GIMP, VLC and the second-layer apps the README lists). The microVM has no display device, so the session runs on a # virtual framebuffer with Mesa software GL; nothing needs a VT or DRM. ARG BASE_IMAGE=ghcr.io/cocoonstack/sandbox/base:24.04 diff --git a/os-image/node/24.04/Dockerfile b/os-image/node/24.04/Dockerfile index 3984db5b..f0134fd8 100644 --- a/os-image/node/24.04/Dockerfile +++ b/os-image/node/24.04/Dockerfile @@ -1,6 +1,6 @@ # node sandbox flavor: base + Node.js 22 LTS from the official tarball into # /usr/local, plus the native-module build toolchain (build-essential, -# python3, unzip) so package installs with node-gyp steps need only the +# python3, python3-setuptools, unzip) so package installs with node-gyp steps need only the # packages themselves from the network. ARG BASE_IMAGE=ghcr.io/cocoonstack/sandbox/base:24.04 diff --git a/protocol/wire/fixtures/v1/req_fs_find.json b/protocol/wire/fixtures/v1/req_fs_find.json index 84793d3f..d04aee61 100644 --- a/protocol/wire/fixtures/v1/req_fs_find.json +++ b/protocol/wire/fixtures/v1/req_fs_find.json @@ -1 +1 @@ -{"v":1,"op":"fs_find","path":"/work/proj","pattern":"TODO","glob":".rs"} +{"v":1,"op":"fs_find","path":"/work/proj","pattern":"TODO","glob":"*.rs"} diff --git a/protocol/wire/fixtures/v1/resp_info.json b/protocol/wire/fixtures/v1/resp_info.json index d60da44f..2c91fc30 100644 --- a/protocol/wire/fixtures/v1/resp_info.json +++ b/protocol/wire/fixtures/v1/resp_info.json @@ -1 +1 @@ -{"type":"info","version":"0.1.0","proto":1,"uptime_secs":12,"procs":1,"sessions":0} +{"type":"info","version":"0.1.0","proto":2,"uptime_secs":12,"procs":1,"sessions":0} diff --git a/protocol/wire/frame.go b/protocol/wire/frame.go index ed37b6f0..d2fe575f 100644 --- a/protocol/wire/frame.go +++ b/protocol/wire/frame.go @@ -676,14 +676,6 @@ func DecodeRequest(line []byte) (Request, error) { return dec(line) } -// NewFrameScanner wraps r for newline-delimited frames capped at MaxFrame. -// No pre-sized buffer: bulk frames outgrow any fixed start anyway. -func NewFrameScanner(r io.Reader) *bufio.Scanner { - sc := bufio.NewScanner(r) - sc.Buffer(nil, MaxFrame) - return sc -} - // DecodeResponse parses one frame into its type's concrete Go type. Byte // fields are freshly allocated per frame, so callers may retain them. func DecodeResponse(line []byte) (Response, error) { @@ -698,6 +690,14 @@ func DecodeResponse(line []byte) (Response, error) { return dec(line) } +// NewFrameScanner wraps r for newline-delimited frames capped at MaxFrame. +// No pre-sized buffer: bulk frames outgrow any fixed start anyway. +func NewFrameScanner(r io.Reader) *bufio.Scanner { + sc := bufio.NewScanner(r) + sc.Buffer(nil, MaxFrame) + return sc +} + // AppendBulkRequest renders a data-carrying request frame — // {"v":1,"op":,"data":""} plus newline — into buf, reused across // calls on the bulk send paths (base64's alphabet needs no JSON escaping). @@ -745,9 +745,7 @@ func fastBulk(tag string, slow respDecoder, mk func([]byte) Response) respDecode } } -// encodeTagged marshals v as a flat object and splices the tag head in front -// of its fields, so wire tags never live on the structs themselves. The -// spare capacity byte lets Conn.Send append the newline without a copy. +// encodeTagged splices the tag head in front of v's fields, so wire tags never live on the structs; the spare capacity byte lets Conn.Send append the newline without a copy. func encodeTagged(head string, v any) ([]byte, error) { body, err := json.Marshal(v) if err != nil { @@ -762,9 +760,7 @@ func encodeTagged(head string, v any) ([]byte, error) { return append(frame, body[1:]...), nil } -// frameTag returns the frame's dispatch tag: a canonical head (both producers -// emit the tag first, unescaped) yields it with a prefix cut; anything else -// takes the full token walk. +// frameTag cuts the dispatch tag off a canonical head (both producers emit it first, unescaped) and falls back to the token walk. func frameTag(line, head []byte, key string) ([]byte, error) { if rest, ok := bytes.CutPrefix(line, head); ok { if end := bytes.IndexAny(rest, `"\`); end >= 0 && rest[end] == '"' { @@ -775,9 +771,7 @@ func frameTag(line, head []byte, key string) ([]byte, error) { return []byte(tag), err } -// scanTag extracts the string value of a top-level key without decoding the -// whole frame; key order never affects correctness, only how early the walk -// stops. A missing tag returns "" (the callers' unknown-tag error). +// scanTag walks the top-level keys without decoding the frame; a missing tag returns "" for the callers' unknown-tag error. func scanTag(line []byte, key string) (string, error) { dec := json.NewDecoder(bytes.NewReader(line)) tok, err := dec.Token() @@ -839,7 +833,6 @@ func decodeAs[T any](line []byte) (*T, error) { return v, nil } -// decodeReq / decodeResp adapt decodeAs to the interface-typed decoder maps. func decodeReq[T any, PT reqPtr[T]](line []byte) (Request, error) { v, err := decodeAs[T](line) if err != nil { diff --git a/protocol/wire/frame_test.go b/protocol/wire/frame_test.go index eb9ee14a..b7df54d2 100644 --- a/protocol/wire/frame_test.go +++ b/protocol/wire/frame_test.go @@ -98,6 +98,9 @@ func TestEveryVerbHasAFixture(t *testing.T) { GitPush{}, GitPull{}, GitBranch{}, + LspStart{}, + LspRequest{}, + LspStop{}, Data{}, DataEnd{}, } @@ -122,6 +125,7 @@ func TestEveryVerbHasAFixture(t *testing.T) { GitStatusResult{}, GitCommitResult{}, GitBranches{}, + LspStarted{}, } entries, err := os.ReadDir(fixtureDir) diff --git a/sandboxd/ca.go b/sandboxd/ca.go index 789ec7d8..488a1953 100644 --- a/sandboxd/ca.go +++ b/sandboxd/ca.go @@ -94,7 +94,7 @@ func writeCAFiles(dir, name string, certPEM, keyPEM []byte, force bool) error { if err := writeKeyMaterial(keyPath, keyPEM, 0o600, force); err != nil { return fmt.Errorf("write key: %w", err) } - // Create sets the mode only on a new file; an overwrite must not keep looser bits. + // O_CREATE sets the mode only on a new file; an overwrite must not keep looser bits. if err := os.Chmod(keyPath, 0o600); err != nil { return fmt.Errorf("chmod key: %w", err) } diff --git a/sandboxd/config/config.go b/sandboxd/config/config.go index df0e2b4f..6d3b65a1 100644 --- a/sandboxd/config/config.go +++ b/sandboxd/config/config.go @@ -178,27 +178,20 @@ type Config struct { DataDir string `json:"data_dir"` CocoonBin string `json:"cocoon_bin"` - AdvertiseAddr string `json:"advertise_addr,omitempty"` - ClientAdvertise string `json:"client_advertise,omitempty"` - - Bridges []string `json:"bridges,omitempty"` - - Networks []string `json:"networks,omitempty"` + AdvertiseAddr string `json:"advertise_addr,omitempty"` + ClientAdvertise string `json:"client_advertise,omitempty"` + Bridges []string `json:"bridges,omitempty"` + Networks []string `json:"networks,omitempty"` RestoreMode types.RestoreMode `json:"restore_mode,omitempty"` + NoDirectIO bool `json:"no_direct_io,omitzero"` + NoBalloon bool `json:"no_balloon,omitzero"` - NoDirectIO bool `json:"no_direct_io,omitzero"` - - NoBalloon bool `json:"no_balloon,omitzero"` - - APIToken string `json:"api_token,omitempty"` //nolint:gosec // config field, not a hardcoded credential - - Tenants []TenantSpec `json:"tenants,omitempty"` - - Secrets []egress.SecretSpec `json:"secrets,omitempty"` - - IdleHibernateSeconds int `json:"idle_hibernate_seconds,omitzero"` + APIToken string `json:"api_token,omitempty"` //nolint:gosec // config field, not a hardcoded credential + Tenants []TenantSpec `json:"tenants,omitempty"` + Secrets []egress.SecretSpec `json:"secrets,omitempty"` + IdleHibernateSeconds int `json:"idle_hibernate_seconds,omitzero"` ArchiveAfterSeconds int `json:"archive_after_seconds,omitzero"` ArchiveDeleteAfterSeconds int `json:"archive_delete_after_seconds,omitzero"` @@ -206,33 +199,23 @@ type Config struct { PreviewSecret string `json:"preview_secret,omitempty"` //nolint:gosec // config field, not a hardcoded credential PreviewAdvertise string `json:"preview_advertise,omitempty"` - CheckpointDir string `json:"checkpoint_dir,omitempty"` - - CheckpointStore *StoreConfig `json:"checkpoint_store,omitempty"` - - CheckpointPeerHeal bool `json:"checkpoint_peer_heal,omitzero"` - - EgressInternalAllow []string `json:"egress_internal_allow,omitempty"` + CheckpointDir string `json:"checkpoint_dir,omitempty"` + CheckpointStore *StoreConfig `json:"checkpoint_store,omitempty"` + CheckpointPeerHeal bool `json:"checkpoint_peer_heal,omitzero"` + CheckpointTTLHours int `json:"checkpoint_ttl_hours,omitzero"` - CheckpointTTLHours int `json:"checkpoint_ttl_hours,omitzero"` + EgressInternalAllow []string `json:"egress_internal_allow,omitempty"` + EgressCA *EgressCAConfig `json:"egress_ca,omitempty"` - MaxClaims int `json:"max_claims,omitzero"` - - AuditLog bool `json:"audit_log,omitzero"` - - MaxForkCount int `json:"max_fork_count,omitzero"` + MaxClaims int `json:"max_claims,omitzero"` + MaxForkCount int `json:"max_fork_count,omitzero"` + RefillConcurrency int `json:"refill_concurrency,omitzero"` + ReleaseDelaySeconds int `json:"release_delay_seconds,omitzero"` + AuditLog bool `json:"audit_log,omitzero"` Volumes []VolumeSpec `json:"volumes,omitempty"` - - RefillConcurrency int `json:"refill_concurrency,omitzero"` - - ReleaseDelaySeconds int `json:"release_delay_seconds,omitzero"` - - Mesh *MeshConfig `json:"mesh,omitempty"` - - EgressCA *EgressCAConfig `json:"egress_ca,omitempty"` - - Pools []PoolSpec `json:"pools"` + Mesh *MeshConfig `json:"mesh,omitempty"` + Pools []PoolSpec `json:"pools"` } // HasEgress reports whether the node can attach egress-lane VMs. @@ -322,8 +305,11 @@ func (c *Config) validate() error { if err := c.RestoreMode.Validate(); err != nil { return fmt.Errorf("restore_mode: %w", err) } - if c.PreviewListen != "" && c.PreviewSecret == "" { - return fmt.Errorf("preview_listen needs preview_secret") + if err := c.validatePreview(); err != nil { + return err + } + if err := c.validatePoolKeys(); err != nil { + return err } if err := validateArchiveWindow(c.IdleHibernateSeconds, c.ArchiveAfterSeconds, c.ArchiveDeleteAfterSeconds); err != nil { return err @@ -369,6 +355,7 @@ func (c *Config) validate() error { func (c *Config) validateVolumes() error { names := make(map[string]struct{}, len(c.Volumes)) + paths := make(map[string]string, len(c.Volumes)) tenants := make(map[string]struct{}, len(c.Tenants)) for _, tenant := range c.Tenants { tenants[tenant.Name] = struct{}{} @@ -384,6 +371,10 @@ func (c *Config) validateVolumes() error { if !filepath.IsAbs(volume.Path) { return fmt.Errorf("volume %q path must be absolute", volume.Name) } + if other, ok := paths[filepath.Clean(volume.Path)]; ok { + return fmt.Errorf("volume %q shares its path with volume %q: admission is keyed by name", volume.Name, other) + } + paths[filepath.Clean(volume.Path)] = volume.Name if !types.ValidDirectIO(volume.DirectIO) { return fmt.Errorf("volume %q directio must be on, off, or auto, got %q", volume.Name, volume.DirectIO) } @@ -396,6 +387,30 @@ func (c *Config) validateVolumes() error { return nil } +func (c *Config) validatePreview() error { + if c.PreviewListen == "" { + return nil + } + if c.PreviewSecret == "" { + return fmt.Errorf("preview_listen needs preview_secret") + } + if !namesHost(c.PreviewAdvertise) { + return fmt.Errorf("preview_advertise %q is minted into every preview URL and must name a routable host", c.PreviewAdvertise) + } + return nil +} + +func (c *Config) validatePoolKeys() error { + pools := make(map[types.PoolKey]struct{}, len(c.Pools)) + for _, p := range c.Pools { + if _, ok := pools[p.PoolKey]; ok { + return fmt.Errorf("duplicate pool %s %s/%s", p.Template, p.Net, p.Size) + } + pools[p.PoolKey] = struct{}{} + } + return nil +} + func (c *Config) validateClientAdvertise() error { if c.ClientAdvertise == "" { return nil @@ -596,3 +611,17 @@ func validateShards(names []string, field, kind string) error { } return nil } + +// namesHost reads addr the way Mint does and reports whether it carries a host a client could dial. +func namesHost(addr string) bool { + if !strings.Contains(addr, "://") { + addr = "http://" + addr + } + u, err := url.Parse(addr) + return err == nil && u.Hostname() != "" && !isUnspecifiedHost(u.Hostname()) +} + +func isUnspecifiedHost(host string) bool { + ip, _ := netip.ParseAddr(host) + return ip.IsUnspecified() +} diff --git a/sandboxd/config/config_test.go b/sandboxd/config/config_test.go index b3307643..304955de 100644 --- a/sandboxd/config/config_test.go +++ b/sandboxd/config/config_test.go @@ -136,6 +136,12 @@ func TestLoadRejectsInvalid(t *testing.T) { {"checkpoint peer heal without cluster key", `{"checkpoint_peer_heal":true,"pools":[],"mesh":{"bind":"node1:7946"}}`, "requires an encrypted mesh"}, {"checkpoint peer heal without ttl", `{"checkpoint_peer_heal":true,"pools":[],"mesh":{"bind":"node1:7946","cluster_key":"MDEyMzQ1Njc4OWFiY2RlZg=="}}`, "requires checkpoint_ttl_hours"}, {"checkpoint peer heal without api_token", `{"checkpoint_peer_heal":true,"pools":[],"mesh":{"bind":"node1:7946","cluster_key":"MDEyMzQ1Njc4OWFiY2RlZg=="},"checkpoint_ttl_hours":1}`, "requires api_token"}, + {"duplicate pool key", `{"pools":[{"template":"rt:24.04","warm":2,"egress":{"allow":[{"host":"x"}]}},{"template":"rt:24.04","warm":8}]}`, "duplicate pool"}, + {"preview advertise defaults to a wildcard listen", `{"preview_listen":":8443","preview_secret":"s","pools":[]}`, "routable host"}, + {"preview advertise names no host", `{"preview_listen":"127.0.0.1:8443","preview_secret":"s","preview_advertise":"https://:8443","pools":[]}`, "routable host"}, + {"preview advertise unspecified bare host", `{"preview_listen":"127.0.0.1:8443","preview_secret":"s","preview_advertise":"0.0.0.0","pools":[]}`, "routable host"}, + {"preview advertise unspecified bare ipv6", `{"preview_listen":"127.0.0.1:8443","preview_secret":"s","preview_advertise":"[::]","pools":[]}`, "routable host"}, + {"duplicate volume path", `{"pools":[],"volumes":[{"name":"models","path":"/srv/models.img"},{"name":"models-rw","path":"/srv/models.img","writable":true}]}`, "shares its path"}, {"bad volume name", `{"pools":[],"volumes":[{"name":"ImageNet","path":"/srv/datasets/a.img"}]}`, "volume name"}, {"reserved volume name", `{"pools":[],"volumes":[{"name":"cocoon-data","path":"/srv/datasets/a.img"}]}`, "not start with cocoon-"}, {"duplicate volume name", `{"pools":[],"volumes":[{"name":"data","path":"/srv/datasets/a.img"},{"name":"data","path":"/srv/datasets/b.img"}]}`, "duplicate volume"}, @@ -152,6 +158,17 @@ func TestLoadRejectsInvalid(t *testing.T) { } } +func TestLoadAcceptsPreviewAdvertiseShapes(t *testing.T) { + for _, advertise := range []string{"preview.example.com", "preview.example.com:8443", "https://preview.example.com", "10.0.0.5:8443"} { + t.Run(advertise, func(t *testing.T) { + path := writeConfig(t, `{"preview_listen":":8443","preview_secret":"s","preview_advertise":"`+advertise+`","pools":[]}`) + if _, err := Load(path); err != nil { + t.Errorf("Load: %v, want the shape Mint accepts to pass validation", err) + } + }) + } +} + func TestLoadAcceptsVolumes(t *testing.T) { path := writeConfig(t, `{"pools":[],"volumes":[ {"name":"imagenet","path":"/srv/datasets/imagenet.img"}, diff --git a/sandboxd/egress/bench_test.go b/sandboxd/egress/bench_test.go index 564f493e..fb925f41 100644 --- a/sandboxd/egress/bench_test.go +++ b/sandboxd/egress/bench_test.go @@ -12,7 +12,7 @@ import ( ) func BenchmarkSignLeaf(b *testing.B) { - ca := benchCA(b) + ca := testCAOnly(b) for b.Loop() { if _, err := ca.SignLeaf("bench.example.com"); err != nil { b.Fatalf("sign: %v", err) @@ -56,23 +56,6 @@ func BenchmarkSplicedHandshake(b *testing.B) { } } -func benchCA(b *testing.B) *CA { - b.Helper() - rootCert, rootKey, err := GenerateRoot("bench root") - if err != nil { - b.Fatalf("root: %v", err) - } - interCert, interKey, err := IssueIntermediate(rootCert, rootKey, "bench") - if err != nil { - b.Fatalf("intermediate: %v", err) - } - ca, err := LoadCA(rootCert, interCert, interKey) - if err != nil { - b.Fatalf("load ca: %v", err) - } - return ca -} - func benchFront(b *testing.B, intercept bool) (proxyAddr string, roots *x509.CertPool) { b.Helper() upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -83,14 +66,14 @@ func benchFront(b *testing.B, intercept bool) (proxyAddr string, roots *x509.Cer var ca *CA roots = x509.NewCertPool() if intercept { - ca = benchCA(b) + ca = testCAOnly(b) if !roots.AppendCertsFromPEM(ca.CertPEM()) { b.Fatal("append cluster root") } } else { roots.AddCert(upstream.Certificate()) } - p := New("sb_b", "", Policy{Allow: []Rule{rule}}, nil, ca, fixedDial(upstream.Listener.Addr().String()), nil, nil) + p := New(Policy{Allow: []Rule{rule}}, nil, ca, fixedDial(upstream.Listener.Addr().String()), nil, nil) if ca != nil { p.mitmTr.TLSClientConfig.RootCAs = func() *x509.CertPool { pool := x509.NewCertPool() diff --git a/sandboxd/egress/ca_issue.go b/sandboxd/egress/ca_issue.go index 696aee02..2666bf2b 100644 --- a/sandboxd/egress/ca_issue.go +++ b/sandboxd/egress/ca_issue.go @@ -71,7 +71,7 @@ func IssueIntermediate(rootCertPEM, rootKeyPEM []byte, nodeID string) (certPEM, return marshalCA(tmpl, root, &key.PublicKey, rootKey, key) } -func marshalCA(tmpl, parent *x509.Certificate, pub, signer, keyToPEM any) (certPEM, keyPEM []byte, err error) { +func marshalCA(tmpl, parent *x509.Certificate, pub *ecdsa.PublicKey, signer, keyToPEM *ecdsa.PrivateKey) (certPEM, keyPEM []byte, err error) { der, err := x509.CreateCertificate(rand.Reader, tmpl, parent, pub, signer) if err != nil { return nil, nil, fmt.Errorf("create cert: %w", err) diff --git a/sandboxd/egress/ca_test.go b/sandboxd/egress/ca_test.go index beb518f3..af78f34c 100644 --- a/sandboxd/egress/ca_test.go +++ b/sandboxd/egress/ca_test.go @@ -187,19 +187,24 @@ func TestFingerprintIsRootAndStable(t *testing.T) { } } -func testCA(t *testing.T) (*CA, []byte) { - t.Helper() +func testCA(tb testing.TB) (*CA, []byte) { + tb.Helper() rootCert, rootKey, err := GenerateRoot("test root") if err != nil { - t.Fatalf("generate root: %v", err) + tb.Fatalf("generate root: %v", err) } interCert, interKey, err := IssueIntermediate(rootCert, rootKey, "node1") if err != nil { - t.Fatalf("issue intermediate: %v", err) + tb.Fatalf("issue intermediate: %v", err) } ca, err := LoadCA(rootCert, interCert, interKey) if err != nil { - t.Fatalf("load ca: %v", err) + tb.Fatalf("load ca: %v", err) } return ca, rootCert } + +func testCAOnly(tb testing.TB) *CA { + ca, _ := testCA(tb) + return ca +} diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index d2ecd8e3..7a9c42fb 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -16,7 +16,7 @@ import ( func TestInterceptLeafCaches(t *testing.T) { ca, _ := testCA(t) - p := New("s", "", Policy{}, nil, ca, fixedDial("127.0.0.1:1"), nil, nil) + p := New(Policy{}, nil, ca, fixedDial("127.0.0.1:1"), nil, nil) a, err := p.leafFor("example.com") if err != nil { t.Fatalf("leaf a: %v", err) @@ -33,7 +33,7 @@ func TestInterceptLeafCaches(t *testing.T) { func TestInterceptLeafClampedToTheIntermediateStaysCached(t *testing.T) { ca, _ := testCA(t) ca.interCert.NotAfter = time.Now().Add(time.Hour).Truncate(time.Second) - p := New("s", "", Policy{}, nil, ca, fixedDial("127.0.0.1:1"), nil, nil) + p := New(Policy{}, nil, ca, fixedDial("127.0.0.1:1"), nil, nil) a, err := p.leafFor("example.com") if err != nil { t.Fatalf("leafFor: %v", err) @@ -49,7 +49,7 @@ func TestInterceptLeafClampedToTheIntermediateStaysCached(t *testing.T) { func TestInterceptLeafCacheBounded(t *testing.T) { ca, _ := testCA(t) - p := New("s", "", Policy{}, nil, ca, fixedDial("127.0.0.1:1"), nil, nil) + p := New(Policy{}, nil, ca, fixedDial("127.0.0.1:1"), nil, nil) for i := range maxLeaves + 50 { if _, err := p.leafFor(fmt.Sprintf("h%d.example.com", i)); err != nil { t.Fatalf("leaf %d: %v", i, err) @@ -82,6 +82,9 @@ func TestInterceptInjectsSecretIntoHTTPS(t *testing.T) { if seen := resp.Header.Get("X-Host-Seen"); seen != "example.com" { t.Errorf("upstream saw Host %q, want the guest's own header preserved", seen) } + if ev := recvEvent(t, events); ev.Method != http.MethodConnect || ev.Host != "example.com" || ev.Decision != DecisionAllow { + t.Errorf("tunnel audit event = %+v, want CONNECT/example.com/allow", ev) + } if ev := recvEvent(t, events); ev.Method != http.MethodGet || ev.Injected != "gh" || ev.Decision != DecisionAllow { t.Errorf("audit event = %+v, want GET/gh/allow", ev) } @@ -233,25 +236,14 @@ func interceptProxyPolicy(t *testing.T, policy Policy, events chan Event) (*Prox })) t.Cleanup(upstream.Close) - rootCert, rootKey, err := GenerateRoot("test root") - if err != nil { - t.Fatalf("generate root: %v", err) - } - interCert, interKey, err := IssueIntermediate(rootCert, rootKey, "node1") - if err != nil { - t.Fatalf("issue intermediate: %v", err) - } - ca, err := LoadCA(rootCert, interCert, interKey) - if err != nil { - t.Fatalf("load ca: %v", err) - } + ca, _ := testCA(t) secrets := fakeSecrets{"gh": {"Authorization", "Bearer SECRET"}} audit := func(ev Event) { if events != nil { events <- ev } } - p := New("sb_1", "acme", policy, secrets, ca, fixedDial(upstream.Listener.Addr().String()), audit, nil) + p := New(policy, secrets, ca, fixedDial(upstream.Listener.Addr().String()), audit, nil) guestRoots := x509.NewCertPool() if !guestRoots.AppendCertsFromPEM(ca.CertPEM()) { diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index edb73fc1..fdca0dc3 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -148,15 +148,15 @@ type Evaluator interface { ServesSocks() bool } +type composite struct { + pool, tenant Policy +} + // Compose intersects a pool and a tenant policy; the pool rule wins on a double allow. func Compose(pool, tenant Policy) Evaluator { return composite{pool: pool, tenant: tenant} } -type composite struct { - pool, tenant Policy -} - func (c composite) Eval(host, method string, port uint16) (Rule, Decision) { rule, pd := c.pool.Eval(host, method, port) if pd != DecisionAllow { diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index 1b2e5177..76031da1 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -50,8 +50,6 @@ type Holder interface { // Event is one audited egress attempt; Injected names the credential, never its value. type Event struct { - Sandbox string - Tenant string Method string Host string Port uint16 @@ -61,8 +59,6 @@ type Event struct { // Proxy is one sandbox's forward proxy, gated by Policy and audited per request. type Proxy struct { - sandbox string - tenant string policy Evaluator secrets Secrets ca *CA @@ -83,10 +79,8 @@ type Proxy struct { } // New builds a Proxy for one sandbox; secrets, ca, audit and holder may be nil, dial must not. -func New(sandbox, tenant string, policy Evaluator, secrets Secrets, ca *CA, dial DialFunc, audit AuditFunc, holder Holder) *Proxy { +func New(policy Evaluator, secrets Secrets, ca *CA, dial DialFunc, audit AuditFunc, holder Holder) *Proxy { p := &Proxy{ - sandbox: sandbox, - tenant: tenant, policy: policy, secrets: secrets, ca: ca, @@ -148,7 +142,6 @@ func (p *Proxy) untrack(conn net.Conn) { p.connMu.Unlock() } -// hold takes the holder for one request and returns its release. func (p *Proxy) hold() func() { if p.holder == nil { return func() {} @@ -165,11 +158,11 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { return } decision, intercept := p.tunnelDecision(host, port) + p.record(Event{Method: r.Method, Host: host, Port: port, Decision: decision}) if intercept { p.serveIntercept(w, r, host, port) return } - p.record(Event{Method: r.Method, Host: host, Port: port, Decision: decision}) if decision == DecisionDeny { denied(w, host) return @@ -274,11 +267,9 @@ func (p *Proxy) inject(rule Rule, h http.Header) string { } func (p *Proxy) record(ev Event) { - if p.audit == nil { - return + if p.audit != nil { + p.audit(ev) } - ev.Sandbox, ev.Tenant = p.sandbox, p.tenant - p.audit(ev) } func stripHop(h http.Header) { diff --git a/sandboxd/egress/proxy_test.go b/sandboxd/egress/proxy_test.go index 62b91d86..f5e3c716 100644 --- a/sandboxd/egress/proxy_test.go +++ b/sandboxd/egress/proxy_test.go @@ -24,7 +24,7 @@ func TestForwardAllowInjectsSecretAndOverwritesGuestHeader(t *testing.T) { policy := Policy{Allow: []Rule{{Host: "api.internal", Secret: "gh"}}} secrets := fakeSecrets{"gh": {"Authorization", "Bearer SECRET"}} events := make(chan Event, 4) - p := New("sb_1", "acme", policy, secrets, nil, fixedDial(upstream.Listener.Addr().String()), func(ev Event) { events <- ev }, nil) + p := New(policy, secrets, nil, fixedDial(upstream.Listener.Addr().String()), func(ev Event) { events <- ev }, nil) front := httptest.NewServer(p) defer front.Close() @@ -48,8 +48,8 @@ func TestForwardAllowInjectsSecretAndOverwritesGuestHeader(t *testing.T) { t.Errorf("upstream saw Authorization %q, want the injected secret (guest value overwritten)", gotAuth) } ev := recvEvent(t, events) - if ev.Decision != DecisionAllow || ev.Injected != "gh" || ev.Sandbox != "sb_1" || ev.Tenant != "acme" { - t.Errorf("audit event = %+v, want allow/gh/sb_1/acme", ev) + if ev.Decision != DecisionAllow || ev.Injected != "gh" { + t.Errorf("audit event = %+v, want allow/gh", ev) } } @@ -60,7 +60,7 @@ func TestRelayKeepsTheUpstreamConnForAClosingGuest(t *testing.T) { _, _ = io.WriteString(w, "hello") })) defer upstream.Close() - p := New("sb_1", "acme", Policy{Allow: []Rule{{Host: "api.internal"}}}, nil, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) + p := New(Policy{Allow: []Rule{{Host: "api.internal"}}}, nil, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) front := httptest.NewServer(p) defer front.Close() @@ -93,7 +93,7 @@ func TestCloseReleasesIdleUpstreamConns(t *testing.T) { defer upstream.Close() policy := Policy{Allow: []Rule{{Host: "api.internal"}}} - p := New("sb_1", "acme", policy, nil, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) + p := New(policy, nil, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) front := httptest.NewServer(p) defer front.Close() @@ -125,7 +125,7 @@ func TestForwardNeverInjectsInterceptSecret(t *testing.T) { policy := Policy{Allow: []Rule{{Host: "api.internal", Secret: "gh", Intercept: true}}} secrets := fakeSecrets{"gh": {"Authorization", "Bearer SECRET"}} - p := New("sb_1", "acme", policy, secrets, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) + p := New(policy, secrets, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) front := httptest.NewServer(p) defer front.Close() @@ -154,7 +154,7 @@ func TestForwardStripsHopHeaders(t *testing.T) { defer upstream.Close() policy := Policy{Allow: []Rule{{Host: "api.internal"}}} - p := New("sb_1", "", policy, nil, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) + p := New(policy, nil, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) front := httptest.NewServer(p) defer front.Close() @@ -181,7 +181,7 @@ func TestForwardStripsHopHeaders(t *testing.T) { func TestForwardDeniedIsTyped(t *testing.T) { policy := Policy{Allow: []Rule{{Host: "api.internal"}}} events := make(chan Event, 4) - p := New("sb_1", "acme", policy, nil, nil, fixedDial("127.0.0.1:1"), func(ev Event) { events <- ev }, nil) + p := New(policy, nil, nil, fixedDial("127.0.0.1:1"), func(ev Event) { events <- ev }, nil) front := httptest.NewServer(p) defer front.Close() @@ -202,7 +202,7 @@ func TestConnectAllowTunnels(t *testing.T) { echo := echoServer(t) policy := Policy{Allow: []Rule{{Host: "echo.internal"}}} events := make(chan Event, 4) - p := New("sb_1", "acme", policy, nil, nil, fixedDial(echo), func(ev Event) { events <- ev }, nil) + p := New(policy, nil, nil, fixedDial(echo), func(ev Event) { events <- ev }, nil) front := httptest.NewServer(p) defer front.Close() @@ -229,7 +229,7 @@ func TestConnectAllowTunnels(t *testing.T) { func TestCloseEndsSplicedTunnel(t *testing.T) { echo := echoServer(t) - p := New("sb_1", "", Policy{Allow: []Rule{{Host: "echo.internal"}}}, nil, nil, fixedDial(echo), nil, nil) + p := New(Policy{Allow: []Rule{{Host: "echo.internal"}}}, nil, nil, fixedDial(echo), nil, nil) front := httptest.NewServer(p) defer front.Close() @@ -257,7 +257,7 @@ func TestCloseEndsSplicedTunnel(t *testing.T) { func TestCloseEndsTunnelWhoseUpstreamStaysSilent(t *testing.T) { var held holdCounter - p := New("sb_1", "", Policy{Allow: []Rule{{Host: "quiet.internal"}}}, nil, nil, fixedDial(silentServer(t)), nil, &held) + p := New(Policy{Allow: []Rule{{Host: "quiet.internal"}}}, nil, nil, fixedDial(silentServer(t)), nil, &held) front := httptest.NewServer(p) defer front.Close() @@ -277,7 +277,7 @@ func TestCloseEndsTunnelWhoseUpstreamStaysSilent(t *testing.T) { func TestConnectDeniedIsTyped(t *testing.T) { policy := Policy{Allow: []Rule{{Host: "echo.internal"}}} - p := New("sb_1", "acme", policy, nil, nil, fixedDial("127.0.0.1:1"), nil, nil) + p := New(policy, nil, nil, fixedDial("127.0.0.1:1"), nil, nil) front := httptest.NewServer(p) defer front.Close() @@ -292,7 +292,7 @@ func TestPortRuleGatesConnectAndForward(t *testing.T) { echo := echoServer(t) policy := Policy{Allow: []Rule{{Host: "echo.internal", Ports: []uint16{443}}}} events := make(chan Event, 4) - p := New("sb_1", "acme", policy, nil, nil, fixedDial(echo), func(ev Event) { events <- ev }, nil) + p := New(policy, nil, nil, fixedDial(echo), func(ev Event) { events <- ev }, nil) front := httptest.NewServer(p) defer front.Close() @@ -325,7 +325,7 @@ func TestPortRuleGatesConnectAndForward(t *testing.T) { func TestUnparsablePortIsDeniedNotDefaulted(t *testing.T) { policy := Policy{Allow: []Rule{{Host: "echo.internal", Ports: []uint16{443}}}} events := make(chan Event, 4) - p := New("sb_1", "acme", policy, nil, nil, fixedDial(echoServer(t)), func(ev Event) { events <- ev }, nil) + p := New(policy, nil, nil, fixedDial(echoServer(t)), func(ev Event) { events <- ev }, nil) front := httptest.NewServer(p) defer front.Close() @@ -388,7 +388,7 @@ func TestUnparsablePortIsDeniedNotDefaulted(t *testing.T) { func TestBracketedIPv6AuthorityIsUnwrapped(t *testing.T) { policy := Policy{Allow: []Rule{{Host: "*"}}} events := make(chan Event, 4) - p := New("sb_1", "acme", policy, nil, nil, fixedDial(echoServer(t)), func(ev Event) { events <- ev }, nil) + p := New(policy, nil, nil, fixedDial(echoServer(t)), func(ev Event) { events <- ev }, nil) front := httptest.NewServer(p) defer front.Close() diff --git a/sandboxd/egress/secrets.go b/sandboxd/egress/secrets.go index a26486ae..097210d8 100644 --- a/sandboxd/egress/secrets.go +++ b/sandboxd/egress/secrets.go @@ -3,19 +3,14 @@ package egress import ( "fmt" "os" + "slices" "strings" "golang.org/x/net/http/httpguts" ) -// nonInjectable rejects secret headers the transport owns or strips. -var nonInjectable = func() map[string]struct{} { - m := map[string]struct{}{"host": {}, "content-length": {}} - for _, h := range hopHeaders { - m[strings.ToLower(h)] = struct{}{} - } - return m -}() +// nonInjectable lists the secret headers the transport owns or strips. +var nonInjectable = slices.Concat([]string{"Host", "Content-Length"}, hopHeaders) // SecretSpec declares a node-side credential the proxy injects, valued from ValueEnv. type SecretSpec struct { @@ -35,8 +30,7 @@ func (s SecretSpec) Validate() error { return fmt.Errorf("secret %q: header %q is not a valid header name", s.Name, s.Header) case s.ValueEnv == "": return fmt.Errorf("secret %q: needs value_env", s.Name) - } - if _, bad := nonInjectable[strings.ToLower(s.Header)]; bad { + case slices.ContainsFunc(nonInjectable, func(h string) bool { return strings.EqualFold(h, s.Header) }): return fmt.Errorf("secret %q: header %s is not injectable", s.Name, s.Header) } return nil diff --git a/sandboxd/egress/socks.go b/sandboxd/egress/socks.go index 3abd9e3c..d54990fa 100644 --- a/sandboxd/egress/socks.go +++ b/sandboxd/egress/socks.go @@ -23,6 +23,7 @@ const ( socksAtypName = 0x03 socksAtypIPv6 = 0x04 socksGranted = 0x00 + socksFailure = 0x01 socksDenied = 0x02 socksUnreached = 0x04 socksBadCmd = 0x07 @@ -119,7 +120,10 @@ func socksHandshake(conn net.Conn) (host string, port uint16, ok bool) { if n, err = readN(conn, 1); err != nil { return "", 0, false } - addrLen = int(n[0]) + if addrLen = int(n[0]); addrLen == 0 { + _ = socksReply(conn, socksFailure) + return "", 0, false + } default: _ = socksReply(conn, socksBadAtyp) return "", 0, false diff --git a/sandboxd/egress/socks_test.go b/sandboxd/egress/socks_test.go index 4ae3c24e..2e0fad94 100644 --- a/sandboxd/egress/socks_test.go +++ b/sandboxd/egress/socks_test.go @@ -91,6 +91,7 @@ func TestSocksRefusesWhatItCannotServe(t *testing.T) { {"bind command", socksGreeting, []byte{socksVersion, 0x02, 0, socksAtypIPv4, 127, 0, 0, 1, 1, 187}, []byte{socksVersion, socksBadCmd}}, {"udp associate", socksGreeting, []byte{socksVersion, 0x03, 0, socksAtypIPv4, 127, 0, 0, 1, 1, 187}, []byte{socksVersion, socksBadCmd}}, {"unknown address type", socksGreeting, []byte{socksVersion, socksConnect, 0, 0x05}, []byte{socksVersion, socksBadAtyp}}, + {"zero-length domain name", socksGreeting, []byte{socksVersion, socksConnect, 0, socksAtypName, 0, 1, 187}, []byte{socksVersion, socksFailure}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -200,7 +201,7 @@ func socksProxy(t *testing.T, policy Policy, ca *CA, dial DialFunc, events chan if events != nil { audit = func(ev Event) { events <- ev } } - p := New("sb_1", "acme", policy, nil, ca, dial, audit, holder) + p := New(policy, nil, ca, dial, audit, holder) ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("listen socks: %v", err) diff --git a/sandboxd/engine/engine.go b/sandboxd/engine/engine.go index 39e90312..6d86afdc 100644 --- a/sandboxd/engine/engine.go +++ b/sandboxd/engine/engine.go @@ -33,22 +33,20 @@ const ( StaleCreateNotCreating StaleCreateOutcome = "not-creating" StaleCreateNotFound StaleCreateOutcome = "not-found" - argName = "--name" - argOutput = "--output" - argNetwork = "--network" - argBridge = "--bridge" - formatJSON = "json" - silkdPort = 2048 // silkd's fixed guest vsock port, the claim-ready anchor - egressPort = 2049 // guest→host egress port; VMM maps it to _2049 - socksPort = 2050 - cmdTimeout = 2 * time.Minute - probeInterval = 20 * time.Millisecond - connectMax = 64 // "OK " handshake reply cap - infoMax = 4096 // info response frame cap - outputTail = 400 - - // portForwardMax caps the port_forward handshake reply line. - portForwardMax = 4096 + argName = "--name" + argOutput = "--output" + argNetwork = "--network" + argBridge = "--bridge" + formatJSON = "json" + silkdPort = 2048 // silkd's fixed guest vsock port, the claim-ready anchor + egressPort = 2049 // guest→host egress port; VMM maps it to _2049 + socksPort = 2050 + cmdTimeout = 2 * time.Minute + probeInterval = 20 * time.Millisecond + connectMax = 64 // "OK " handshake reply cap + infoMax = 4096 // info response frame cap + outputTail = 400 + portForwardMax = 4096 // port_forward handshake reply line cap ) // capacitySignatures mean the node cannot attach another VM, not that this VM failed. @@ -431,7 +429,6 @@ func shardOf(shards []string, name string) string { return shards[int(h.Sum32()>>1)%len(shards)] } -// respFail renders a non-success reply: the error text, or the unexpected frame's type. func respFail(resp wire.Response) string { if errResp, ok := resp.(*wire.ErrorResp); ok { return errResp.Error() @@ -439,7 +436,6 @@ func respFail(resp wire.Response) string { return "unexpected frame " + resp.RespType() } -// parseRecord is best-effort: an unparseable record yields the zero value. func parseRecord(ctx context.Context, out []byte) types.VMRecord { var rec types.VMRecord if err := json.Unmarshal(out, &rec); err != nil { diff --git a/sandboxd/engine/installca.go b/sandboxd/engine/installca.go index 8882cba8..51cbb8df 100644 --- a/sandboxd/engine/installca.go +++ b/sandboxd/engine/installca.go @@ -11,8 +11,9 @@ import ( ) const ( - caCertGuestPath = "/usr/local/share/ca-certificates/sandbox-egress.crt" - caBundlePath = "/etc/ssl/certs/ca-certificates.crt" + caCertGuestPath = "/usr/local/share/ca-certificates/sandbox-egress.crt" + caBundlePath = "/etc/ssl/certs/ca-certificates.crt" + installOutputMax = 4096 // guestExecPATH is set because silkd starts the guest command with an empty environment. guestExecPATH = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ) @@ -98,9 +99,8 @@ func (e *Engine) silkdExec(ctx context.Context, vsockSocket string, argv ...stri } } -// appendCapped keeps the first 4096 bytes of combined output for error context. func appendCapped(out, data []byte) []byte { - if room := 4096 - len(out); room > 0 { + if room := installOutputMax - len(out); room > 0 { out = append(out, data[:min(len(data), room)]...) } return out diff --git a/sandboxd/log.go b/sandboxd/log.go index 01cbba03..11335344 100644 --- a/sandboxd/log.go +++ b/sandboxd/log.go @@ -48,11 +48,14 @@ func setupLog(ctx context.Context, level string) error { return err } logger := log.GetGlobalLogger() - w := journalWriter{console: zerolog.ConsoleWriter{TimeFormat: time.RFC822}, out: os.Stderr} - *logger = zerolog.New(w).With().Timestamp().Logger().Level(logger.GetLevel()) + *logger = zerolog.New(newJournalWriter(os.Stderr)).With().Timestamp().Logger().Level(logger.GetLevel()) return nil } +func newJournalWriter(out io.Writer) journalWriter { + return journalWriter{console: zerolog.ConsoleWriter{TimeFormat: time.RFC822, NoColor: true}, out: out} +} + func syslogPrefix(level zerolog.Level) string { switch level { case zerolog.PanicLevel, zerolog.FatalLevel: diff --git a/sandboxd/log_test.go b/sandboxd/log_test.go index fce37515..879116e2 100644 --- a/sandboxd/log_test.go +++ b/sandboxd/log_test.go @@ -5,7 +5,6 @@ import ( "io" "strings" "testing" - "time" "github.com/rs/zerolog" ) @@ -36,6 +35,9 @@ func TestJournalWriterPrefixesEveryRecordWithItsSyslogLevel(t *testing.T) { if strings.Count(line, "\n") != 1 { t.Errorf("line = %q, want exactly one record", line) } + if strings.Contains(line, "\x1b[") { + t.Errorf("line = %q, want no ANSI color in a journald record", line) + } }) } } @@ -60,7 +62,7 @@ func TestSyslogPrefixMapsFatalToCritical(t *testing.T) { } func testLogger(out io.Writer) zerolog.Logger { - return zerolog.New(journalWriter{console: zerolog.ConsoleWriter{TimeFormat: time.RFC822, NoColor: true}, out: out}) + return zerolog.New(newJournalWriter(out)) } type countingWriter struct { diff --git a/sandboxd/mesh/epoch.go b/sandboxd/mesh/epoch.go index c0ec53cc..656cad1e 100644 --- a/sandboxd/mesh/epoch.go +++ b/sandboxd/mesh/epoch.go @@ -9,7 +9,6 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/utils" ) -// loadEpoch reads the persisted gossip epoch, or 0 when it is absent, unreadable, or corrupt. func loadEpoch(path string) uint64 { raw, err := os.ReadFile(path) //nolint:gosec // node-local data-dir path if err != nil { diff --git a/sandboxd/mesh/mesh.go b/sandboxd/mesh/mesh.go index 47a98f75..85dd4c26 100644 --- a/sandboxd/mesh/mesh.go +++ b/sandboxd/mesh/mesh.go @@ -69,7 +69,7 @@ func New(ctx context.Context, cfg *memberlist.Config, nodeID, selfAddr string, s view: map[string]NodeState{}, live: map[string]struct{}{}, } - if err := m.persistEpoch(m.leased); err != nil { + if err := storeEpoch(m.epochPath, m.leased); err != nil { return nil, fmt.Errorf("persist mesh epoch: %w", err) } m.view[nodeID] = m.self @@ -112,7 +112,7 @@ func (m *Mesh) UpdateSelf(ctx context.Context, pools map[string]int, templates, m.mu.Unlock() if epoch > m.leased { leased := epoch + epochLease - if err := m.persistEpoch(leased); err != nil { + if err := storeEpoch(m.epochPath, leased); err != nil { log.WithFunc("mesh.UpdateSelf").Warnf(ctx, "persist epoch: %v", err) return } @@ -186,12 +186,12 @@ func (m *Mesh) TemplateOwners(keyHash string) []string { return m.owners(func(st NodeState) bool { return slices.Contains(st.Templates, keyHash) }) } -// VolumeOwners returns peers that advertise every requested volume; self is excluded. +// VolumeOwners returns up to two peers that advertise every requested volume; self is excluded. func (m *Mesh) VolumeOwners(names []string) []string { return m.owners(func(st NodeState) bool { return containsAll(st.Volumes, names) }) } -// TemplateVolumeOwners returns peers holding both the template and every requested volume. +// TemplateVolumeOwners returns up to two peers holding both the template and every requested volume. func (m *Mesh) TemplateVolumeOwners(keyHash string, names []string) []string { return m.owners(func(st NodeState) bool { return slices.Contains(st.Templates, keyHash) && containsAll(st.Volumes, names) @@ -272,10 +272,6 @@ func (m *Mesh) warmCandidates(keyHash string, match nodeMatch) []string { return []string{a.addr, b.addr} } -func (m *Mesh) persistEpoch(epoch uint64) error { - return storeEpoch(m.epochPath, epoch) -} - func (m *Mesh) owners(match nodeMatch) []string { m.mu.Lock() var owners []string @@ -326,16 +322,6 @@ func (m *Mesh) merge(states []NodeState) { } } -func short(digest string) string { - return digest[:min(len(digest), 12)] -} - -func containsAll(have, need []string) bool { - return len(need) > 0 && !slices.ContainsFunc(need, func(name string) bool { - return !slices.Contains(have, name) - }) -} - var _ memberlist.Delegate = (*delegate)(nil) type delegate Mesh @@ -365,3 +351,13 @@ func (e *eventDelegate) NotifyUpdate(*memberlist.Node) {} func (e *eventDelegate) NotifyLeave(n *memberlist.Node) { (*Mesh)(e).forget(n.Name) } + +func short(digest string) string { + return digest[:min(len(digest), 12)] +} + +func containsAll(have, need []string) bool { + return len(need) > 0 && !slices.ContainsFunc(need, func(name string) bool { + return !slices.Contains(have, name) + }) +} diff --git a/sandboxd/pool/archive.go b/sandboxd/pool/archive.go index 146fba06..4eb4d89e 100644 --- a/sandboxd/pool/archive.go +++ b/sandboxd/pool/archive.go @@ -37,7 +37,6 @@ func (m *Manager) archiveEnabledFor(key types.PoolKey) bool { return m.archiveAfterFor(key) > 0 } -// archiveOnce checkpoints hibernated claims idle past their archive threshold and drops their VM. func (m *Manager) archiveOnce(ctx context.Context) { if !m.archiveEnabled.Load() { return @@ -77,7 +76,6 @@ func (m *Manager) archiveOnce(ctx context.Context) { }() } -// archive swaps a hibernated claim's backing from local VM to a store checkpoint. func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error { // must finish even if the sweep ctx is canceled, or the record diverges from the store. ctx = context.WithoutCancel(ctx) @@ -119,7 +117,7 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error { js := m.store.set(sb) m.mu.Unlock() if saveErr := m.store.commit(js); saveErr != nil { - // roll back so memory matches the still-durable hibernated record; drop the orphan ck. + // roll back so memory matches the still-durable hibernated record m.mu.Lock() var rb claimSnapshot // a Release that landed meanwhile already deleted the claim; re-setting it would resurrect it @@ -138,7 +136,7 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error { // disarm under Transition so a wake right after the release is not clobbered by a late disarm m.disarmEgress(sb.ID, true) sb.Transition.Unlock() - // committed: the store ck is authoritative now; reclaim the local footprint. + // the store ck is authoritative from here m.destroy(ctx, vmName) m.dropSnap(ctx, snap) m.counters.archives.Add(1) @@ -239,7 +237,6 @@ func (m *Manager) commitWake(ctx context.Context, sb *types.Sandbox, vmName, soc return true, true } -// deleteOrphanArchiveCk drops the published ck when archive() aborts pre-commit. func (m *Manager) deleteOrphanArchiveCk(ctx context.Context, ckID string) { if err := m.deleteArchiveCk(ctx, ckID); err != nil { log.WithFunc("pool.deleteOrphanArchiveCk").Warnf(ctx, "delete orphaned archive ck %s: %v", ckID, err) @@ -255,7 +252,10 @@ func (m *Manager) markArchiveCk(ckID string) error { if err := os.MkdirAll(dir, 0o750); err != nil { return err } - return os.WriteFile(filepath.Join(dir, ckID), nil, 0o600) + if err := os.WriteFile(filepath.Join(dir, ckID), nil, 0o600); err != nil { + return err + } + return syncDir(dir) } func (m *Manager) archiveDeleteMarkers() ([]string, error) { @@ -364,15 +364,6 @@ func (m *Manager) retryArchiveDelete(ctx context.Context, ckID string) { } func (m *Manager) archiveCkPinned(ckID string) bool { - m.mu.Lock() - defer m.mu.Unlock() - if _, ok := m.pendingCks[ckID]; ok { - return true - } - for _, sb := range m.claimed { - if sb.ArchiveCk == ckID { - return true - } - } - return false + _, pinned := m.pinnedArchiveCks()[ckID] + return pinned } diff --git a/sandboxd/pool/checkpoint.go b/sandboxd/pool/checkpoint.go index f2c6e5dd..8f8fa34a 100644 --- a/sandboxd/pool/checkpoint.go +++ b/sandboxd/pool/checkpoint.go @@ -110,7 +110,6 @@ func (m *Manager) Checkpoints(ctx context.Context, tenant string) ([]types.Check // DeleteCheckpoint removes a checkpoint's snapshot and record, broadcasting when fleet-scoped. func (m *Manager) DeleteCheckpoint(ctx context.Context, ckptID, tenant string, scope DeleteScope) error { - // a rejected id must not leave a lock-map entry if !store.CheckpointIDRe.MatchString(ckptID) { return ErrUnknownCheckpoint } @@ -165,7 +164,6 @@ func (m *Manager) FetchCheckpoint(ctx context.Context, ckptID string) (string, [ return dir, meta, func() { l.RUnlock(); m.recDone(ckptID) }, nil } -// publishCheckpoint stages, writes the meta, and publishes, returning the record and source snap. func (m *Manager) publishCheckpoint(ctx context.Context, sb *types.Sandbox, ckID, name, tenant string, archive bool) (types.Checkpoint, string, error) { ckpt := types.Checkpoint{ ID: ckID, @@ -203,6 +201,12 @@ func (m *Manager) claimLoaded(ctx context.Context, ckpt types.Checkpoint, ttl ti l := m.recLock(ckpt.ID) l.RLock() defer func() { l.RUnlock(); m.recDone(ckpt.ID) }() + if ckpt.Archive { + return nil, ErrUnknownCheckpoint // a wake image, not a branchable checkpoint + } + if !ckpt.Key.Capturable() { + return nil, ErrNoEgressFork + } dir, _, _, err := m.ckpts.Fetch(ctx, ckpt.ID) if errors.Is(err, store.ErrNotFound) { return nil, ErrUnknownCheckpoint // deleted between the pre-check and the lock @@ -210,12 +214,6 @@ func (m *Manager) claimLoaded(ctx context.Context, ckpt types.Checkpoint, ttl ti if err != nil { return nil, fmt.Errorf("fetch checkpoint: %w", err) } - if ckpt.Archive { - return nil, ErrUnknownCheckpoint // a wake image, not a branchable checkpoint - } - if !ckpt.Key.Capturable() { - return nil, ErrNoEgressFork - } sb, err := m.provision(ctx, ckpt.Key, dir) if err != nil { return nil, err @@ -298,7 +296,7 @@ func (m *Manager) markHealPending(ckptID string) { m.healPending[ckptID] = struct{}{} } -// clearHealPending un-marks ckptID and reports a veto; call under ckptID's recLock. +// clearHealPending un-marks ckptID and reports a veto. func (m *Manager) clearHealPending(ckptID string) (aborted bool) { m.recLocksMu.Lock() defer m.recLocksMu.Unlock() @@ -331,7 +329,6 @@ func (m *Manager) pinnedArchiveCks() map[string]struct{} { return pinned } -// sweepExpiredCheckpoints ages out checkpoints older than the configured TTL. func (m *Manager) sweepExpiredCheckpoints(ctx context.Context) { if m.ckptTTL <= 0 || !m.ckptSweeping.CompareAndSwap(false, true) { return diff --git a/sandboxd/pool/checkpoint_heal_test.go b/sandboxd/pool/checkpoint_heal_test.go index 0c2c94e7..8f8f8e29 100644 --- a/sandboxd/pool/checkpoint_heal_test.go +++ b/sandboxd/pool/checkpoint_heal_test.go @@ -294,104 +294,37 @@ func TestClaimCheckpointHealConcurrencyCapRejectsExtra(t *testing.T) { } } -func TestValidateHealedCheckpointAcceptsValid(t *testing.T) { - dir := t.TempDir() - plantHealedRecord(t, dir, types.Checkpoint{ID: "ck_00000000000000aa"}) - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err != nil { - t.Errorf("validate: %v, want a valid record accepted", err) - } -} - -func TestValidateHealedCheckpointRejectsMismatchedID(t *testing.T) { - dir := t.TempDir() - plantHealedRecord(t, dir, types.Checkpoint{ID: "ck_00000000000000bb"}) - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err == nil { - t.Error("validate accepted a mismatched id") - } -} - -func TestValidateHealedCheckpointRejectsMissingMeta(t *testing.T) { - dir := t.TempDir() - if err := os.MkdirAll(filepath.Join(dir, store.ExportDir), 0o750); err != nil { - t.Fatalf("setup: %v", err) - } - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err == nil { - t.Error("validate accepted a record with no meta.json") - } -} - -func TestValidateHealedCheckpointRejectsMissingExport(t *testing.T) { - dir := t.TempDir() - - meta, err := json.Marshal(types.Checkpoint{ID: "ck_00000000000000aa", Key: testKey}) - if err != nil { - t.Fatalf("setup: %v", err) - } - if err := os.WriteFile(filepath.Join(dir, store.MetaFile), meta, 0o600); err != nil { - t.Fatalf("setup: %v", err) - } - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err == nil { - t.Error("validate accepted a record with no export dir") - } -} - -func TestValidateHealedCheckpointRejectsArchive(t *testing.T) { - dir := t.TempDir() - plantHealedRecord(t, dir, types.Checkpoint{ID: "ck_00000000000000aa", Archive: true}) - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err == nil { - t.Error("validate accepted an archive-flagged record") - } -} - -func TestValidateHealedCheckpointRejectsEmptyExport(t *testing.T) { - dir := t.TempDir() - if err := os.MkdirAll(filepath.Join(dir, store.ExportDir), 0o750); err != nil { - t.Fatalf("setup: %v", err) - } - meta, err := json.Marshal(types.Checkpoint{ID: "ck_00000000000000aa", Key: testKey}) - if err != nil { - t.Fatalf("setup: %v", err) - } - if err := os.WriteFile(filepath.Join(dir, store.MetaFile), meta, 0o600); err != nil { - t.Fatalf("setup: %v", err) - } - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err == nil { - t.Error("validate accepted a record with an empty export") - } -} - -func TestValidateHealedCheckpointRejectsInvalidKey(t *testing.T) { - dir := t.TempDir() - plantHealedRecord(t, dir, types.Checkpoint{ID: "ck_00000000000000aa", Key: types.PoolKey{Net: "bogus"}}) - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err == nil { - t.Error("validate accepted a record with an invalid key") - } -} - -func TestValidateHealedCheckpointRejectsEgressKey(t *testing.T) { - dir := t.TempDir() +func TestValidateHealedCheckpoint(t *testing.T) { + const id = "ck_00000000000000aa" + valid := types.Checkpoint{ID: id, Key: testKey} egress := testKey egress.Net = types.NetEgress - plantHealedRecord(t, dir, types.Checkpoint{ID: "ck_00000000000000aa", Key: egress}) - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err == nil { - t.Error("validate accepted a non-branchable egress-lane record") - } -} - -func TestValidateHealedCheckpointRejectsContentlessExport(t *testing.T) { - dir := t.TempDir() - if err := os.MkdirAll(filepath.Join(dir, store.ExportDir, "sub"), 0o750); err != nil { - t.Fatalf("setup: %v", err) - } - meta, err := json.Marshal(types.Checkpoint{ID: "ck_00000000000000aa", Key: testKey}) - if err != nil { - t.Fatalf("setup: %v", err) - } - if err := os.WriteFile(filepath.Join(dir, store.MetaFile), meta, 0o600); err != nil { - t.Fatalf("setup: %v", err) - } - if err := validateHealedCheckpoint(dir, "ck_00000000000000aa"); err == nil { - t.Error("validate accepted an export with no regular file") + tests := []struct { + name string + plant func(t *testing.T, dir string) + want bool + }{ + {"accepts a valid record", func(t *testing.T, dir string) { plantHealedRecord(t, dir, types.Checkpoint{ID: id}) }, true}, + {"rejects a mismatched id", func(t *testing.T, dir string) { plantHealedRecord(t, dir, types.Checkpoint{ID: "ck_00000000000000bb"}) }, false}, + {"rejects a missing meta", func(t *testing.T, dir string) { plantExportDir(t, dir) }, false}, + {"rejects a missing export", func(t *testing.T, dir string) { plantMeta(t, dir, valid) }, false}, + {"rejects an archive", func(t *testing.T, dir string) { plantHealedRecord(t, dir, types.Checkpoint{ID: id, Archive: true}) }, false}, + {"rejects an empty export", func(t *testing.T, dir string) { plantExportDir(t, dir); plantMeta(t, dir, valid) }, false}, + {"rejects an invalid key", func(t *testing.T, dir string) { + plantHealedRecord(t, dir, types.Checkpoint{ID: id, Key: types.PoolKey{Net: "bogus"}}) + }, false}, + {"rejects an egress key", func(t *testing.T, dir string) { plantHealedRecord(t, dir, types.Checkpoint{ID: id, Key: egress}) }, false}, + {"rejects a contentless export", func(t *testing.T, dir string) { plantExportDir(t, dir, "sub"); plantMeta(t, dir, valid) }, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + tt.plant(t, dir) + err := validateHealedCheckpoint(dir, id) + if (err == nil) != tt.want { + t.Errorf("validate: %v, want accepted=%v", err, tt.want) + } + }) } } @@ -535,3 +468,21 @@ func plantHealedRecord(t *testing.T, dir string, ckpt types.Checkpoint) { t.Fatalf("setup: %v", err) } } + +func plantExportDir(t *testing.T, dir string, sub ...string) { + t.Helper() + if err := os.MkdirAll(filepath.Join(append([]string{dir, store.ExportDir}, sub...)...), 0o750); err != nil { + t.Fatalf("setup: %v", err) + } +} + +func plantMeta(t *testing.T, dir string, ck types.Checkpoint) { + t.Helper() + meta, err := json.Marshal(ck) + if err != nil { + t.Fatalf("setup: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, store.MetaFile), meta, 0o600); err != nil { + t.Fatalf("setup: %v", err) + } +} diff --git a/sandboxd/pool/checkpoint_test.go b/sandboxd/pool/checkpoint_test.go index 8ee529aa..810fd5f7 100644 --- a/sandboxd/pool/checkpoint_test.go +++ b/sandboxd/pool/checkpoint_test.go @@ -1,6 +1,7 @@ package pool import ( + "context" "encoding/json" "errors" "os" @@ -9,6 +10,8 @@ import ( "testing" "time" + "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/store" "github.com/cocoonstack/sandbox/sandboxd/types" ) @@ -117,6 +120,37 @@ func TestRetentionSweepsExpiredCheckpoints(t *testing.T) { } } +func TestRunRefillsWhileTheRetentionSweepBlocks(t *testing.T) { + eng := newFakeEngine() + m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1}) + m.pools[testKey].goldenDir = "/goldens/x" + m.ckptTTL = time.Hour + stall := &stallingMetasStore{Store: m.ckpts, entered: make(chan struct{}, 1), release: make(chan struct{})} + m.ckpts = stall + + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan struct{}) + go func() { + m.Run(ctx) + close(done) + }() + defer func() { + close(stall.release) + cancel() + <-done + }() + <-stall.entered + + waitFor(t, func() bool { + m.mu.Lock() + defer m.mu.Unlock() + return len(m.pools[testKey].warm) > 0 + }) + if got := eng.cloneCount(); got != 1 { + t.Errorf("clones=%d, want one warm VM spawned while the store listing is stuck", got) + } +} + func TestCheckpointTenantIsolation(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) @@ -215,3 +249,18 @@ func TestRejectedCheckpointOpsLeakNoRecLock(t *testing.T) { t.Errorf("recLocks grew %d->%d over a wrong-tenant delete, want no growth", before, got) } } + +type stallingMetasStore struct { + store.Store + entered chan struct{} + release chan struct{} +} + +func (s *stallingMetasStore) Metas(ctx context.Context) ([][]byte, error) { + select { + case s.entered <- struct{}{}: + default: + } + <-s.release + return s.Store.Metas(ctx) +} diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index 7137dd65..f593243e 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -5,6 +5,7 @@ import ( "crypto/subtle" "fmt" "net" + "slices" "sync" "time" @@ -13,7 +14,6 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/types" ) -// reapAction is what reapOnce does with an expired claim. type reapAction int const ( @@ -98,7 +98,7 @@ func (m *Manager) ClaimDeadline(id, token string) (time.Time, error) { return sb.Deadline, nil } -// PreviewDial authorizes one preview request and opens its guest connection. +// PreviewDial opens a preview request's guest connection; the caller verified the HMAC token. func (m *Manager) PreviewDial(ctx context.Context, id string, port uint16) (net.Conn, error) { m.mu.Lock() sb, ok := m.claimed[id] @@ -189,7 +189,7 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand removed := vmName == "" switch { case removed: - m.finishVolumeTeardown(ctx, td) // archived: no VM to confirm gone + m.finishVolumeTeardown(ctx, td) case m.releaseDelay > 0: m.queueRemoval(vmName, id, "", td, time.Now().Add(m.releaseDelay)) default: @@ -382,22 +382,21 @@ func (m *Manager) reapOnce(ctx context.Context) { } logger := log.WithFunc("pool.reapOnce") - keep := expired[:0] - for _, v := range expired { - if v.action == reapPurge { - if markErr := m.markArchiveCk(v.ck); markErr != nil { - logger.Errorf(ctx, markErr, "mark archive ck %s; keeping %s", v.ck, v.id) - continue - } + expired = slices.DeleteFunc(expired, func(v victim) bool { + if v.action != reapPurge { + return false } - keep = append(keep, v) - } - expired = keep + markErr := m.markArchiveCk(v.ck) + if markErr != nil { + logger.Errorf(ctx, markErr, "mark archive ck %s; keeping %s", v.ck, v.id) + } + return markErr != nil + }) if len(expired) == 0 { return } m.mu.Lock() - keep = expired[:0] + keep := expired[:0] var purgeCks, purged []string for _, v := range expired { if v.action == reapPurge { diff --git a/sandboxd/pool/drain.go b/sandboxd/pool/drain.go index 6b55fce6..e7891209 100644 --- a/sandboxd/pool/drain.go +++ b/sandboxd/pool/drain.go @@ -11,9 +11,7 @@ func (m *Manager) Drain(ctx context.Context) { trim = append(trim, p.trimWarm(0)...) } m.mu.Unlock() - m.runBounded(context.WithoutCancel(ctx), len(trim), func(ctx context.Context, i int) { - m.destroy(ctx, trim[i]) - }).Wait() + m.destroyAll(context.WithoutCancel(ctx), trim).Wait() } // Uncordon lifts a drain and kicks an immediate refill. diff --git a/sandboxd/pool/egress.go b/sandboxd/pool/egress.go index 0c3daf8d..356a44e8 100644 --- a/sandboxd/pool/egress.go +++ b/sandboxd/pool/egress.go @@ -9,11 +9,11 @@ import ( "net/netip" "os" "slices" + "sync" "syscall" "time" "github.com/projecteru2/core/log" - "golang.org/x/net/netutil" "github.com/cocoonstack/sandbox/sandboxd/egress" "github.com/cocoonstack/sandbox/sandboxd/engine" @@ -51,7 +51,6 @@ var ( } ) -// egressListener is one sandbox's egress accept points over the per-sandbox UDS pair. type egressListener struct { srv *http.Server proxy *egress.Proxy @@ -74,6 +73,49 @@ func (e *egressListener) close() { } } +// doorListener caps a door's live connections; the accepted conn stays a *net.UnixConn so splice and half-close keep working. +type doorListener struct { + *net.UnixListener + slots chan struct{} + done chan struct{} + once sync.Once +} + +func limitDoor(ln *net.UnixListener) *doorListener { + return &doorListener{UnixListener: ln, slots: make(chan struct{}, egressDoorConns), done: make(chan struct{})} +} + +func (l *doorListener) Accept() (net.Conn, error) { + select { + case l.slots <- struct{}{}: + case <-l.done: + return nil, net.ErrClosed + } + conn, err := l.AcceptUnix() + if err != nil { + <-l.slots + return nil, err + } + return &doorConn{UnixConn: conn, slots: l.slots}, nil +} + +func (l *doorListener) Close() error { + l.once.Do(func() { close(l.done) }) + return l.UnixListener.Close() +} + +type doorConn struct { + *net.UnixConn + slots chan struct{} + once sync.Once +} + +func (c *doorConn) Close() error { + err := c.UnixConn.Close() + c.once.Do(func() { <-c.slots }) + return err +} + // armEgress locks the egress-lane NIC then binds the proxy: fail-closed, never a free NIC. func (m *Manager) armEgress(ctx context.Context, sb *types.Sandbox) error { if err := m.lockEgressNIC(ctx, sb); err != nil { @@ -144,7 +186,7 @@ func (m *Manager) tapOf(ctx context.Context, vmName string) (string, error) { return "", fmt.Errorf("no tap for %s", vmName) } -// armEgressProxy serves the egress proxy when the effective policy permits something, on the doors refill pre-bound where it could. +// armEgressProxy serves the proxy on the pre-bound doors, binding them when refill could not, once the effective policy permits anything. func (m *Manager) armEgressProxy(ctx context.Context, sb *types.Sandbox) error { if !m.guardedEgress || sb.VsockSocket == "" { return nil @@ -166,7 +208,7 @@ func (m *Manager) armEgressProxy(ctx context.Context, sb *types.Sandbox) error { if !intercepts { ca = nil } - proxy := egress.New(id, tenant, policy, m.egressSecrets, ca, m.dial, + proxy := egress.New(policy, m.egressSecrets, ca, m.dial, func(ev egress.Event) { m.recordEgress(evCtx, id, tenant, ev) }, sb) if el != nil && (reached(el.ln) || (el.socks != nil && reached(el.socks))) { el.close() @@ -180,10 +222,6 @@ func (m *Manager) armEgressProxy(ctx context.Context, sb *types.Sandbox) error { } el.srv = &http.Server{Handler: proxy, ReadHeaderTimeout: 30 * time.Second} el.proxy = proxy - el.ln = netutil.LimitListener(el.ln, egressDoorConns) - if el.socks != nil { - el.socks = netutil.LimitListener(el.socks, egressDoorConns) - } m.mu.Lock() displaced := m.egressListeners[id] m.egressListeners[id] = el @@ -260,7 +298,6 @@ func (m *Manager) disarmEgress(id string, removed bool) { } } -// poolIntercepts reports whether the configured pool for key HTTPS-intercepts. func (m *Manager) poolIntercepts(key types.PoolKey) bool { m.mu.Lock() defer m.mu.Unlock() @@ -274,7 +311,7 @@ func (m *Manager) effectivePolicy(sb *types.Sandbox) (egress.Evaluator, bool) { poolPol := m.poolEgress[sb.Key] tenantPol := m.tenantEgress[sb.Tenant] if poolPol == nil { - if m.pools[sb.Key] != nil || sb.Tenant == "" || tenantPol == nil { + if _, pooled := m.activePool(sb.Key); pooled || sb.Tenant == "" || tenantPol == nil { return nil, false } return *tenantPol, true @@ -288,27 +325,28 @@ func (m *Manager) effectivePolicy(sb *types.Sandbox) (egress.Evaluator, bool) { return egress.Compose(*poolPol, *tenantPol), true } -// bindEgress binds the HTTP door and, when the policy serves it, the SOCKS5 door of one VM. func bindEgress(vsock string, socks bool) (*egressListener, error) { el := &egressListener{path: engine.EgressSocketPath(vsock)} - var err error - if el.ln, err = listenUnix(el.path); err != nil { + ln, err := listenUnix(el.path) + if err != nil { return nil, fmt.Errorf("listen egress: %w", err) } + el.ln = limitDoor(ln) if socks { el.socksPath = engine.SocksSocketPath(vsock) - if el.socks, err = listenUnix(el.socksPath); err != nil { + if ln, err = listenUnix(el.socksPath); err != nil { el.close() return nil, fmt.Errorf("listen socks: %w", err) } + el.socks = limitDoor(ln) } return el, nil } // listenUnix binds path after clearing a stale socket from a prior life that would block the bind. -func listenUnix(path string) (net.Listener, error) { +func listenUnix(path string) (*net.UnixListener, error) { _ = os.Remove(path) - return net.Listen("unix", path) + return net.ListenUnix("unix", &net.UnixAddr{Name: path, Net: "unix"}) } // reached reports whether a guest connected to a door before it was armed; only a clean EAGAIN says no. @@ -331,7 +369,6 @@ func reached(ln net.Listener) bool { return waiting } -// newEgressDialer blocks internal targets, then re-admits exactly the node-named prefixes. func newEgressDialer(allow []netip.Prefix) *net.Dialer { return &net.Dialer{Control: func(_, address string, _ syscall.RawConn) error { host, _, err := net.SplitHostPort(address) diff --git a/sandboxd/pool/egress_test.go b/sandboxd/pool/egress_test.go index aefdb01c..f05a02dc 100644 --- a/sandboxd/pool/egress_test.go +++ b/sandboxd/pool/egress_test.go @@ -1,8 +1,10 @@ package pool import ( + "bufio" "context" "errors" + "fmt" "io" "net" "net/http" @@ -218,24 +220,27 @@ func TestEffectivePolicyComposition(t *testing.T) { name string tenant string pooled bool + removed bool pool, tnPol *egress.Policy allow, deny string wantArmed bool }{ - {"root takes the pool policy whole", "", true, both, nil, "a.test", "z.test", true}, - {"root without a pool policy", "", true, nil, nil, "", "", false}, - {"tenant intersects", "acme", true, both, tenantOnly, "b.test", "a.test", true}, - {"tenant declaring no policy", "acme", true, both, nil, "", "", false}, - {"tenant on a policyless pool", "acme", true, nil, tenantOnly, "", "", false}, - {"tenant on a promoted template", "acme", false, nil, tenantOnly, "b.test", "a.test", true}, - {"root on a promoted template", "", false, nil, nil, "", "", false}, - {"neither", "acme", false, nil, nil, "", "", false}, + {"root takes the pool policy whole", "", true, false, both, nil, "a.test", "z.test", true}, + {"root without a pool policy", "", true, false, nil, nil, "", "", false}, + {"tenant intersects", "acme", true, false, both, tenantOnly, "b.test", "a.test", true}, + {"tenant declaring no policy", "acme", true, false, both, nil, "", "", false}, + {"tenant on a policyless pool", "acme", true, false, nil, tenantOnly, "", "", false}, + {"tenant on a policyless pool being removed", "acme", true, true, nil, tenantOnly, "b.test", "a.test", true}, + {"tenant on a promoted template", "acme", false, false, nil, tenantOnly, "b.test", "a.test", true}, + {"root on a promoted template", "", false, false, nil, nil, "", "", false}, + {"neither", "acme", false, false, nil, nil, "", "", false}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { m.pools = map[types.PoolKey]*pool{} if tc.pooled { m.pools[testKey] = newPool(testKey) + m.pools[testKey].removed = tc.removed } m.poolEgress = map[types.PoolKey]*egress.Policy{} if tc.pool != nil { @@ -658,6 +663,56 @@ func TestTrimmedWarmVMClosesItsDoors(t *testing.T) { } } +func TestEgressDoorHalfCloseReachesTheGuest(t *testing.T) { + origin, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("origin: %v", err) + } + t.Cleanup(func() { _ = origin.Close() }) + go func() { + conn, acceptErr := origin.Accept() + if acceptErr != nil { + return + } + _, _ = io.WriteString(conn, "hello") + _ = conn.Close() + }() + pol := &egress.Policy{Allow: []egress.Rule{{Host: "127.0.0.1"}}} + m := egressManager(t, newFakeEngine(), config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: pol}) + m.dial = (&net.Dialer{}).DialContext + sb := vsockSandbox(t, "sb_halfclose") + if err := m.armEgress(t.Context(), sb); err != nil { + t.Fatalf("arm egress: %v", err) + } + t.Cleanup(func() { m.disarmEgress(sb.ID, true) }) + + door, dialErr := net.Dial("unix", engine.EgressSocketPath(sb.VsockSocket)) + if dialErr != nil { + t.Fatalf("dial door: %v", dialErr) + } + t.Cleanup(func() { _ = door.Close() }) + _ = door.SetDeadline(time.Now().Add(5 * time.Second)) + target := origin.Addr().String() + if _, writeErr := fmt.Fprintf(door, "CONNECT %s HTTP/1.1\r\nHost: %s\r\n\r\n", target, target); writeErr != nil { + t.Fatalf("connect: %v", writeErr) + } + resp, readErr := http.ReadResponse(bufio.NewReader(door), nil) + if readErr != nil { + t.Fatalf("read connect reply: %v", readErr) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("connect status %d, want 200", resp.StatusCode) + } + body, bodyErr := io.ReadAll(resp.Body) + if bodyErr != nil { + t.Fatalf("tunnel did not end with EOF after the origin closed: %v", bodyErr) + } + if string(body) != "hello" { + t.Errorf("tunnel body %q, want hello", body) + } +} + func TestEgressDoorCapsConcurrentConnections(t *testing.T) { origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, "ok") diff --git a/sandboxd/pool/fork.go b/sandboxd/pool/fork.go index 8b525ca2..5f3ca5bd 100644 --- a/sandboxd/pool/fork.go +++ b/sandboxd/pool/fork.go @@ -85,10 +85,3 @@ func (m *Manager) forkSource(ctx context.Context, sb *types.Sandbox) (vmProvisio return func(name string) (types.VMRecord, error) { return m.eng.Clone(ctx, exportDir, name, sb.Key) }, func() { _ = os.RemoveAll(dir) }, nil } - -// lockedVMName reads the VM name under the lock that archive() clears it under. -func lockedVMName(sb *types.Sandbox) string { - sb.Transition.Lock() - defer sb.Transition.Unlock() - return sb.VMName -} diff --git a/sandboxd/pool/hibernate.go b/sandboxd/pool/hibernate.go index 8480b278..f5bfed05 100644 --- a/sandboxd/pool/hibernate.go +++ b/sandboxd/pool/hibernate.go @@ -51,13 +51,8 @@ func (m *Manager) hibernateLocked(ctx context.Context, sb *types.Sandbox) error // cocoon resumes the guest before its fresh tap can be re-locked return ErrNoEgressHibernate } - // an adopted hibernate was never billed, so record it here - adopted, resolveErr := m.resolvePendingSnap(ctx, sb) - if adopted { - m.recordHibernate(ctx, sb) - } - if resolveErr != nil { - return resolveErr + if _, err := m.settlePendingSnap(ctx, sb); err != nil { + return err } if sb.HibernateSnap != "" { m.disarmEgress(sb.ID, true) @@ -80,9 +75,8 @@ func (m *Manager) hibernateLocked(ctx context.Context, sb *types.Sandbox) error } if hibErr := m.eng.Hibernate(ctx, sb.VMName, snap); hibErr != nil { // the engine can report failure after the snapshot landed - adopted, resolveErr := m.resolvePendingSnap(ctx, sb) + adopted, resolveErr := m.settlePendingSnap(ctx, sb) if adopted { - m.recordHibernate(ctx, sb) m.disarmEgress(sb.ID, true) if resolveErr != nil { return fmt.Errorf("hibernate %s: persist claims: %w", sb.ID, resolveErr) @@ -118,11 +112,7 @@ func (m *Manager) wakeResolved(ctx context.Context, sb *types.Sandbox) (string, return m.wakeArchived(ctx, sb) } // settle a dangling intent before choosing running vs hibernated - adopted, err := m.resolvePendingSnap(ctx, sb) - if adopted { - m.recordHibernate(ctx, sb) - } - if err != nil { + if _, err := m.settlePendingSnap(ctx, sb); err != nil { return "", fmt.Errorf("wake %s: %w", sb.ID, err) } if sb.HibernateSnap == "" { @@ -181,7 +171,6 @@ func (m *Manager) wakeResolved(ctx context.Context, sb *types.Sandbox) (string, return sock, nil } -// idleOnce hibernates claims idle past their pool's (or the node's) threshold. func (m *Manager) idleOnce(ctx context.Context) { if !m.idleEnabled.Load() { return @@ -283,6 +272,15 @@ func (m *Manager) setPendingSnap(sb *types.Sandbox, snap string) claimSnapshot { return m.store.set(sb) } +// settlePendingSnap settles a dangling hibernate intent and bills an adopted one, which never was. +func (m *Manager) settlePendingSnap(ctx context.Context, sb *types.Sandbox) (adopted bool, err error) { + adopted, err = m.resolvePendingSnap(ctx, sb) + if adopted { + m.recordHibernate(ctx, sb) + } + return adopted, err +} + // resolvePendingSnap settles an unconfirmed hibernate intent against the engine's snapshot list. func (m *Manager) resolvePendingSnap(ctx context.Context, sb *types.Sandbox) (adopted bool, err error) { if sb.PendingSnap == "" { @@ -325,7 +323,6 @@ func (m *Manager) recordHibernate(ctx context.Context, sb *types.Sandbox) { m.recordUsage(ctx, usageEvent{Event: "hibernate", ID: sb.ID, VMName: sb.VMName}) } -// skipIdle reports the claims an idle sweep must leave alone. func skipIdle(sb *types.Sandbox, idle time.Duration, now time.Time) bool { return idle <= 0 || sb.Key.Net == types.NetEgress || hasAppliedVolumes(sb) || sb.HibernateSnap != "" || sb.ArchiveCk != "" || sb.Busy() || now.Sub(sb.LastSeen()) < idle diff --git a/sandboxd/pool/journal.go b/sandboxd/pool/journal.go index e1fc1b52..11440635 100644 --- a/sandboxd/pool/journal.go +++ b/sandboxd/pool/journal.go @@ -20,7 +20,7 @@ type usageEvent struct { KeyHash string `json:"key,omitempty"` // claim only Tenant string `json:"tenant,omitempty"` // claim only Volumes []string `json:"volumes,omitempty"` // claim only - VolumesRW []string `json:"volumes_rw,omitempty"` // claim only: the write-enabled subset + VolumesRW []string `json:"volumes_rw,omitempty"` // claim only Children []string `json:"children,omitempty"` // fork only Reference string `json:"ref,omitempty"` // promote: template; checkpoint: ckpt id } @@ -63,12 +63,6 @@ func (j *journal) append(v any) error { return errors.Join(rotateErr, err) } -func (j *journal) close() error { - j.mu.Lock() - defer j.mu.Unlock() - return j.f.Close() -} - // rotate moves the live file to .1 and reopens; the old descriptor closes last. func (j *journal) rotate() error { renameErr := os.Rename(j.path, j.path+".1") diff --git a/sandboxd/pool/journal_test.go b/sandboxd/pool/journal_test.go index 686b7a40..21dbe347 100644 --- a/sandboxd/pool/journal_test.go +++ b/sandboxd/pool/journal_test.go @@ -21,7 +21,6 @@ func TestJournalRotateFailureDegradesAndRecovers(t *testing.T) { if err != nil { t.Fatalf("new journal: %v", err) } - defer func() { _ = j.close() }() if err = os.Mkdir(path+".1", 0o750); err != nil { t.Fatalf("setup: %v", err) @@ -66,7 +65,6 @@ func TestJournalRotateReopenFailureKeepsWriting(t *testing.T) { if err != nil { t.Fatalf("new journal: %v", err) } - defer func() { _ = j.close() }() j.path = filepath.Join(dir, "gone", "usage.jsonl") if err = j.append(map[string]string{"ev": "a"}); err == nil { t.Fatal("append across a failed reopen must surface the error") diff --git a/sandboxd/pool/operator.go b/sandboxd/pool/operator.go index 4c3c2654..45bbbb54 100644 --- a/sandboxd/pool/operator.go +++ b/sandboxd/pool/operator.go @@ -52,7 +52,6 @@ func (m *Manager) byID(id string) (*types.Sandbox, bool) { return sb, sb != nil } -// wake reuses the relay's resolve path, then discards the socket. func (m *Manager) wake(ctx context.Context, sb *types.Sandbox) error { sb.Touch() _, err := m.wakeResolved(ctx, sb) diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index 5bf75ef2..eb5acee0 100644 --- a/sandboxd/pool/pool.go +++ b/sandboxd/pool/pool.go @@ -54,8 +54,7 @@ const ( recommitBackoff = 20 * time.Millisecond 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. + refillFailStreak = 8 refillBackoffBase = 250 * time.Millisecond refillBackoffMax = buildRetryDelay // fallbacks when a Manager is built from a Config that skipped config.Load's defaulting @@ -484,7 +483,7 @@ func (m *Manager) Run(ctx context.Context) { // store retention is hourly: each sweep is cluster-visible I/O on a shared root storeSweep := time.NewTicker(time.Hour) defer storeSweep.Stop() - m.sweepExpiredCheckpoints(ctx) + go m.sweepExpiredCheckpoints(ctx) m.refillOnce(ctx) for { select { @@ -503,7 +502,7 @@ func (m *Manager) Run(ctx context.Context) { go m.retryArchiveDeletes(ctx) case <-storeSweep.C: m.sweepStoreGenerations(ctx) - m.sweepExpiredCheckpoints(ctx) + go m.sweepExpiredCheckpoints(ctx) } } } @@ -664,7 +663,7 @@ func loadEgressCA(cfg *config.EgressCAConfig) (*egress.CA, error) { return egress.LoadCA(root, interCert, interKey) } -// The dir default lives here, not config.applyDefaults: tests build Config directly. +// the dir default lives here, not config.applyDefaults: tests build Config directly func newStoreView(ctx context.Context, cfg *config.Config, staging string, idRe *regexp.Regexp) (store.Store, error) { if cs := cfg.CheckpointStore; cs != nil && cs.Kind == "s3" { return s3.New(ctx, *cs.S3, filepath.Join(cfg.DataDir, staging), idRe) @@ -705,6 +704,13 @@ func vmName(key types.PoolKey) string { return vmPrefix + key.Hash() + "-" + randHex(6) } +// lockedVMName reads the VM name under the lock that archive() clears it under. +func lockedVMName(sb *types.Sandbox) string { + sb.Transition.Lock() + defer sb.Transition.Unlock() + return sb.VMName +} + func randHex(n int) string { b := make([]byte, n) _, _ = rand.Read(b) // never fails per crypto/rand contract diff --git a/sandboxd/pool/promote_test.go b/sandboxd/pool/promote_test.go index a8f07438..1145d561 100644 --- a/sandboxd/pool/promote_test.go +++ b/sandboxd/pool/promote_test.go @@ -311,14 +311,7 @@ func TestPromoteFailsClosedOnMetaError(t *testing.T) { func TestPromoteRefusesCrossTenantOverwrite(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - claim := func(tenant string) *types.Sandbox { - t.Helper() - sb, err := m.ClaimProvision(t.Context(), testKey, time.Hour, tenant, "", nil) - if err != nil { - t.Fatalf("claim %q: %v", tenant, err) - } - return sb - } + claim := func(tenant string) *types.Sandbox { return claimTenant(t, m, tenant) } a := claim("acme") if _, _, err := m.Promote(t.Context(), a.ID, Cred{Token: a.Token}, "shared:v1", "acme"); err != nil { t.Fatalf("acme promote: %v", err) @@ -336,14 +329,7 @@ func TestPromoteRefusesCrossTenantOverwrite(t *testing.T) { func TestTemplateClaimIsTenantScoped(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - claim := func(tenant string) *types.Sandbox { - t.Helper() - sb, err := m.ClaimProvision(t.Context(), testKey, time.Hour, tenant, "", nil) - if err != nil { - t.Fatalf("claim %q: %v", tenant, err) - } - return sb - } + claim := func(tenant string) *types.Sandbox { return claimTenant(t, m, tenant) } a := claim("acme") private, _, err := m.Promote(t.Context(), a.ID, Cred{Token: a.Token}, "acme-private", "acme") @@ -443,3 +429,12 @@ func TestTemplateHashesSortedForMeshCompare(t *testing.T) { t.Errorf("TemplateHashes not sorted: %v", hashes) } } + +func claimTenant(t *testing.T, m *Manager, tenant string) *types.Sandbox { + t.Helper() + sb, err := m.ClaimProvision(t.Context(), testKey, time.Hour, tenant, "", nil) + if err != nil { + t.Fatalf("claim %q: %v", tenant, err) + } + return sb +} diff --git a/sandboxd/pool/reclock.go b/sandboxd/pool/reclock.go new file mode 100644 index 00000000..e86d680c --- /dev/null +++ b/sandboxd/pool/reclock.go @@ -0,0 +1,50 @@ +package pool + +import "sync" + +// recLock takes the per-record lock and a live reference; pair with recDone. +func (m *Manager) recLock(id string) *sync.RWMutex { + m.recLocksMu.Lock() + defer m.recLocksMu.Unlock() + m.recRefs[id]++ + l := m.recLocks[id] + if l == nil { + l = &sync.RWMutex{} + m.recLocks[id] = l + } + return l +} + +// recDone releases a reference taken by recLock; call after Unlock/RUnlock, never before. +func (m *Manager) recDone(id string) { + m.recLocksMu.Lock() + defer m.recLocksMu.Unlock() + m.recRefs[id]-- + if m.recRefs[id] <= 0 { + delete(m.recRefs, id) + m.evictIfPending(id) + } +} + +// recDoneEvict is recDone for a just-deleted record: the lock slot goes at zero references. +func (m *Manager) recDoneEvict(id string) { + m.recLocksMu.Lock() + defer m.recLocksMu.Unlock() + m.recRefs[id]-- + if m.recRefs[id] <= 0 { + delete(m.recRefs, id) + delete(m.recEvict, id) + delete(m.recLocks, id) + return + } + // a holder or waiter remains; the call that drops the last reference evicts instead + m.recEvict[id] = struct{}{} +} + +// evictIfPending drops id's lock entry if a delete asked for eviction; callers hold recLocksMu. +func (m *Manager) evictIfPending(id string) { + if _, ok := m.recEvict[id]; ok { + delete(m.recEvict, id) + delete(m.recLocks, id) + } +} diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index be9f8788..8d830012 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -69,8 +69,9 @@ func (m *Manager) Reconcile(ctx context.Context) error { m.adoptGolden(p) } m.mu.Unlock() - // a crash mid-export leaves a *.tmp staging dir nothing in this life reuses + // a crash mid-export leaves a golden *.tmp or a fork-* staging dir nothing in this life reuses tmps, _ := filepath.Glob(filepath.Join(m.goldensDir(), "*.tmp")) + forks, _ := filepath.Glob(filepath.Join(m.dataDir, "fork-*")) logger := log.WithFunc("pool.Reconcile") if err := m.ckpts.SweepStaging(); err != nil { logger.Error(ctx, err, "sweep checkpoint staging") @@ -78,7 +79,7 @@ func (m *Manager) Reconcile(ctx context.Context) error { if err := m.tpls.SweepStaging(); err != nil { logger.Error(ctx, err, "sweep template staging") } - for _, tmp := range tmps { + for _, tmp := range slices.Concat(tmps, forks) { _ = os.RemoveAll(tmp) } diff --git a/sandboxd/pool/reconcile_test.go b/sandboxd/pool/reconcile_test.go index d5b23e94..799cc3e1 100644 --- a/sandboxd/pool/reconcile_test.go +++ b/sandboxd/pool/reconcile_test.go @@ -3,6 +3,8 @@ package pool import ( "context" "errors" + "os" + "path/filepath" "slices" "testing" @@ -152,6 +154,24 @@ func TestReconcileStaleRunningVMSkipsVerb(t *testing.T) { } } +func TestReconcileSweepsInterruptedForkExports(t *testing.T) { + dir := t.TempDir() + stale := filepath.Join(dir, "fork-abc123", "export") + if err := os.MkdirAll(stale, 0o750); err != nil { + t.Fatalf("setup: %v", err) + } + if err := os.WriteFile(filepath.Join(stale, "disk.img"), []byte("x"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + m := newTestManagerAt(t, newFakeEngine(), dir) + if err := m.Reconcile(t.Context()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if _, err := os.Stat(filepath.Dir(stale)); !errors.Is(err, os.ErrNotExist) { + t.Errorf("fork export dir survived Reconcile: %v", err) + } +} + type staleCreateCase struct { name string outcome engine.StaleCreateOutcome diff --git a/sandboxd/pool/refill.go b/sandboxd/pool/refill.go index f66d4321..2f77ab76 100644 --- a/sandboxd/pool/refill.go +++ b/sandboxd/pool/refill.go @@ -87,12 +87,7 @@ func (m *Manager) shrinkOnce(ctx context.Context) { trim = append(trim, p.shrink(now)...) } m.mu.Unlock() - if len(trim) == 0 { - return - } - m.runBounded(ctx, len(trim), func(ctx context.Context, i int) { - m.destroy(ctx, trim[i]) - }) + m.destroyAll(ctx, trim) } func (m *Manager) refillOne(ctx context.Context, p *pool, golden string) { @@ -475,6 +470,10 @@ func (m *Manager) findVM(ctx context.Context, name string) (types.VMRecord, bool } // runBounded fans f over n items on the refill semaphore, sharing the node-wide budget. +func (m *Manager) destroyAll(ctx context.Context, names []string) *sync.WaitGroup { + return m.runBounded(ctx, len(names), func(ctx context.Context, i int) { m.destroy(ctx, names[i]) }) +} + func (m *Manager) runBounded(ctx context.Context, n int, f func(context.Context, int)) *sync.WaitGroup { var wg sync.WaitGroup for i := range n { diff --git a/sandboxd/pool/setpools.go b/sandboxd/pool/setpools.go index 12d11028..c3508b5a 100644 --- a/sandboxd/pool/setpools.go +++ b/sandboxd/pool/setpools.go @@ -63,9 +63,7 @@ func (m *Manager) SetPools(ctx context.Context, specs []config.PoolSpec) error { m.mu.Unlock() runCtx := context.WithoutCancel(ctx) - m.runBounded(runCtx, len(trim), func(ctx context.Context, i int) { - m.destroy(ctx, trim[i]) - }).Wait() + m.destroyAll(runCtx, trim).Wait() m.refillOnce(runCtx) // persist the applied set so a restart rebuilds from it, not the config seed. persisted := slices.Collect(maps.Values(desired)) diff --git a/sandboxd/pool/telemetry.go b/sandboxd/pool/telemetry.go index 157ac5b8..68592cb8 100644 --- a/sandboxd/pool/telemetry.go +++ b/sandboxd/pool/telemetry.go @@ -95,9 +95,6 @@ func (m *Manager) TenantClaims() map[string]int { // Audit records one relayed request frame against a sandbox: the op and addressing fields only. func (m *Manager) Audit(ctx context.Context, id string, line []byte) { - if m.audit == nil { - return - } if len(line) > AuditLineCap { // the addressing fields stay unread at this size, but a padded frame must not escape the journal m.recordAudit(ctx, id, auditFrame{Op: "oversized"}) diff --git a/sandboxd/pool/telemetry_test.go b/sandboxd/pool/telemetry_test.go index e86c9a4d..5095580c 100644 --- a/sandboxd/pool/telemetry_test.go +++ b/sandboxd/pool/telemetry_test.go @@ -61,7 +61,6 @@ func TestJournalRotationKeepsOneBackup(t *testing.T) { if err != nil { t.Fatalf("newJournal: %v", err) } - defer func() { _ = j.close() }() j.size = journalMaxBytes if err := j.append(usageEvent{Event: "claim", ID: "sb_1"}); err != nil { diff --git a/sandboxd/pool/template.go b/sandboxd/pool/template.go index e231becd..273fe3b8 100644 --- a/sandboxd/pool/template.go +++ b/sandboxd/pool/template.go @@ -8,7 +8,6 @@ import ( "os" "path/filepath" "slices" - "sync" "time" "github.com/cocoonstack/sandbox/sandboxd/store" @@ -170,53 +169,6 @@ func (m *Manager) pooled(key types.PoolKey) bool { return ok } -// recLock takes the per-record lock and a live reference (pair with recDone); clones and wakes hold it shared so a delete or re-publish never runs under an in-flight read. -func (m *Manager) recLock(id string) *sync.RWMutex { - m.recLocksMu.Lock() - defer m.recLocksMu.Unlock() - m.recRefs[id]++ - l := m.recLocks[id] - if l == nil { - l = &sync.RWMutex{} - m.recLocks[id] = l - } - return l -} - -// recDone releases a reference taken by recLock; call after Unlock/RUnlock, never before. -func (m *Manager) recDone(id string) { - m.recLocksMu.Lock() - defer m.recLocksMu.Unlock() - m.recRefs[id]-- - if m.recRefs[id] <= 0 { - delete(m.recRefs, id) - m.evictIfPending(id) - } -} - -// recDoneEvict is recDone for a just-deleted record: the lock slot goes at zero references. -func (m *Manager) recDoneEvict(id string) { - m.recLocksMu.Lock() - defer m.recLocksMu.Unlock() - m.recRefs[id]-- - if m.recRefs[id] <= 0 { - delete(m.recRefs, id) - delete(m.recEvict, id) - delete(m.recLocks, id) - return - } - // a holder or waiter remains; the call that drops the last reference evicts instead - m.recEvict[id] = struct{}{} -} - -// evictIfPending drops id's lock entry if a delete asked for eviction; callers hold recLocksMu. -func (m *Manager) evictIfPending(id string) { - if _, ok := m.recEvict[id]; ok { - delete(m.recEvict, id) - delete(m.recLocks, id) - } -} - // checkTemplateOwner rejects publishing or deleting over another tenant's record. func (m *Manager) checkTemplateOwner(ctx context.Context, id, tenant string) error { if tenant == "" { @@ -236,11 +188,54 @@ func (m *Manager) checkTemplateOwner(ctx context.Context, id, tenant string) err return nil } +func (m *Manager) publishTemplate(ctx context.Context, snap string, key types.PoolKey, tenant string) (string, error) { + id := store.TemplateID(key.Hash()) + staging, err := m.tpls.Stage(id) + if err != nil { + return "", fmt.Errorf("stage template: %w", err) + } + defer func() { _ = os.RemoveAll(staging) }() + if err = m.eng.SnapshotExport(ctx, snap, filepath.Join(staging, store.ExportDir)); err != nil { + return "", fmt.Errorf("export template: %w", err) + } + return m.commitTemplate(ctx, staging, id, tenant) +} + +func (m *Manager) commitTemplate(ctx context.Context, staging, id, tenant string) (string, error) { + l := m.recLock(id) + l.Lock() + defer func() { l.Unlock(); m.recDone(id) }() + if ownerErr := m.checkTemplateOwner(ctx, id, tenant); ownerErr != nil { + return "", ownerErr + } + meta, err := json.Marshal(templateRecord{ID: id, Tenant: tenant, CreatedAt: time.Now()}) + if err != nil { + return "", err + } + if err = os.WriteFile(filepath.Join(staging, store.MetaFile), meta, 0o600); err != nil { + return "", err + } + digest, err := m.tpls.PublishDigested(ctx, staging, id) + if err != nil { + return "", fmt.Errorf("publish template: %w", err) + } + m.tplMu.Lock() + m.tplSet[id] = tenant + m.tplMu.Unlock() + return digest, nil +} + type goldenResolution struct { dir string templateDigest string promoted bool - release func() + unlock func() +} + +func (g goldenResolution) release() { + if g.unlock != nil { + g.unlock() + } } // resolveGolden resolves a key's clone source: the pool golden, else a promoted template. @@ -252,10 +247,10 @@ func (m *Manager) resolveGolden(ctx context.Context, key types.PoolKey, tenant s } m.mu.Unlock() if dir != "" { - return goldenResolution{dir: dir, release: func() {}}, nil + return goldenResolution{dir: dir}, nil } if key.Net == types.NetEgress { - return goldenResolution{release: func() {}}, nil // never resume a live-captured template on the egress lane; cold-boot instead + return goldenResolution{}, nil // never resume a live-captured template on the egress lane; cold-boot instead } id := store.TemplateID(key.Hash()) l := m.recLock(id) @@ -264,64 +259,27 @@ func (m *Manager) resolveGolden(ctx context.Context, key types.PoolKey, tenant s if errors.Is(err, store.ErrNotFound) { l.RUnlock() m.recDoneEvict(id) - return goldenResolution{release: func() {}}, nil + return goldenResolution{}, nil } if err != nil { l.RUnlock() m.recDone(id) - return goldenResolution{release: func() {}}, err + return goldenResolution{}, err } cleanup := func() { l.RUnlock(); m.recDone(id) } var rec templateRecord if err := json.Unmarshal(meta, &rec); err != nil { cleanup() - return goldenResolution{release: func() {}}, fmt.Errorf("decode template metadata: %w", err) + return goldenResolution{}, fmt.Errorf("decode template metadata: %w", err) } if rec.Tenant != "" && !tenantOwns(tenant, rec.Tenant) { cleanup() - return goldenResolution{release: func() {}}, nil + return goldenResolution{}, nil } return goldenResolution{ dir: dir, templateDigest: digest, promoted: true, - release: cleanup, + unlock: cleanup, }, nil } - -func (m *Manager) publishTemplate(ctx context.Context, snap string, key types.PoolKey, tenant string) (string, error) { - id := store.TemplateID(key.Hash()) - staging, err := m.tpls.Stage(id) - if err != nil { - return "", fmt.Errorf("stage template: %w", err) - } - defer func() { _ = os.RemoveAll(staging) }() - if err = m.eng.SnapshotExport(ctx, snap, filepath.Join(staging, store.ExportDir)); err != nil { - return "", fmt.Errorf("export template: %w", err) - } - return m.commitTemplate(ctx, staging, id, tenant) -} - -func (m *Manager) commitTemplate(ctx context.Context, staging, id, tenant string) (string, error) { - l := m.recLock(id) - l.Lock() - defer func() { l.Unlock(); m.recDone(id) }() - if ownerErr := m.checkTemplateOwner(ctx, id, tenant); ownerErr != nil { - return "", ownerErr - } - meta, err := json.Marshal(templateRecord{ID: id, Tenant: tenant, CreatedAt: time.Now()}) - if err != nil { - return "", err - } - if err = os.WriteFile(filepath.Join(staging, store.MetaFile), meta, 0o600); err != nil { - return "", err - } - digest, err := m.tpls.PublishDigested(ctx, staging, id) - if err != nil { - return "", fmt.Errorf("publish template: %w", err) - } - m.tplMu.Lock() - m.tplSet[id] = tenant - m.tplMu.Unlock() - return digest, nil -} diff --git a/sandboxd/pool/volume.go b/sandboxd/pool/volume.go index ca38660c..5054afb1 100644 --- a/sandboxd/pool/volume.go +++ b/sandboxd/pool/volume.go @@ -24,16 +24,6 @@ const ( volumeQuiesceMax = 10 * time.Second ) -type catalogVolume struct { - disk engine.VolumeSpec - tenants []string - writable bool -} - -func (v catalogVolume) allowed(tenant string) bool { - return tenant == "" || len(v.tenants) == 0 || slices.Contains(v.tenants, tenant) -} - // volumeHolders is one name's live admission state: a writer excludes every other claim. type volumeHolders struct { writers int @@ -51,6 +41,16 @@ type volumeTeardown struct { clears []string } +type catalogVolume struct { + disk engine.VolumeSpec + tenants []string + writable bool +} + +func (v catalogVolume) allowed(tenant string) bool { + return tenant == "" || len(v.tenants) == 0 || slices.Contains(v.tenants, tenant) +} + // Volumes reports the caller-visible fleet catalog; an empty tenant means root. func (m *Manager) Volumes(tenant string, holders map[string]int) []types.VolumeInfo { infos := make([]types.VolumeInfo, 0, len(m.volumes)) @@ -170,21 +170,15 @@ func (m *Manager) confirmVolumesClean(volumes []resolvedVolume) error { // applyVolumes brings the resolved set up concurrently; cocoon serializes the attach per VM. func (m *Manager) applyVolumes(ctx context.Context, sb *types.Sandbox, volumes []resolvedVolume, applied []types.Volume) error { - switch len(volumes) { - case 0: + if len(volumes) == 0 { return nil - case 1: - if err := m.applyVolume(ctx, sb, volumes[0]); err != nil { - return err - } - default: - group, groupCtx := errgroup.WithContext(ctx) - for _, volume := range volumes { - group.Go(func() error { return m.applyVolume(groupCtx, sb, volume) }) - } - if err := group.Wait(); err != nil { - return err - } + } + group, groupCtx := errgroup.WithContext(ctx) + for _, volume := range volumes { + group.Go(func() error { return m.applyVolume(groupCtx, sb, volume) }) + } + if err := group.Wait(); err != nil { + return err } sb.Volumes = applied return nil @@ -225,7 +219,7 @@ func (m *Manager) quiesceVolumes(ctx context.Context, sb *types.Sandbox) volumeT defer cancel() stuck := false for _, volume := range slices.Backward(sb.Volumes) { - if !volume.RW() || volume.Mount == "" { + if !needsUnmount(volume) { continue // attach-only: no mount to bring down, VM removal closes the device } if err := m.eng.UnmountVolume(ctx, sb.VsockSocket, volume.Mount); err != nil { @@ -321,13 +315,17 @@ func quiesceBudget(mounts int) time.Duration { func writableMounts(volumes []types.Volume) int { mounts := 0 for _, volume := range volumes { - if volume.RW() && volume.Mount != "" { + if needsUnmount(volume) { mounts++ } } return mounts } +func needsUnmount(volume types.Volume) bool { + return volume.RW() && volume.Mount != "" +} + func appliedVolumes(volumes []resolvedVolume) []types.Volume { applied := make([]types.Volume, len(volumes)) for i, volume := range volumes { diff --git a/sandboxd/server/exec.go b/sandboxd/server/exec.go index 298ffd90..4af9280d 100644 --- a/sandboxd/server/exec.go +++ b/sandboxd/server/exec.go @@ -66,11 +66,7 @@ func (s *Server) handleExec(w http.ResponseWriter, r *http.Request) { ctx, cancel = context.WithTimeout(ctx, time.Duration(req.TimeoutSeconds)*time.Second) defer cancel() } - stop := context.AfterFunc(ctx, func() { _ = guest.Close() }) - defer func() { - stop() - _ = guest.Close() - }() + defer closeOnCancel(ctx, guest)() frame, _ := wire.EncodeRequest(wire.Exec{Argv: req.Argv, Cwd: req.Cwd, Env: req.Env}) frame = append(frame, '\n') if s.mgr.AuditEnabled() { @@ -122,11 +118,7 @@ func (s *Server) killExec(ctx context.Context, id, token string, pid uint32) err if err != nil { return err } - stop := context.AfterFunc(ctx, func() { _ = guest.Close() }) - defer func() { - stop() - _ = guest.Close() - }() + defer closeOnCancel(ctx, guest)() frame, _ := wire.EncodeRequest(wire.Kill{PID: pid}) if _, err = guest.Write(append(frame, '\n')); err != nil { return err @@ -177,3 +169,12 @@ func collectExec(guest net.Conn) (resp ExecResponse, pid uint32, err error) { } return ExecResponse{}, pid, io.ErrUnexpectedEOF } + +// closeOnCancel closes conn when ctx ends and once more when the returned func runs. +func closeOnCancel(ctx context.Context, conn net.Conn) func() { + stop := context.AfterFunc(ctx, func() { _ = conn.Close() }) + return func() { + stop() + _ = conn.Close() + } +} diff --git a/sandboxd/server/exec_test.go b/sandboxd/server/exec_test.go index 6a60f082..bece9766 100644 --- a/sandboxd/server/exec_test.go +++ b/sandboxd/server/exec_test.go @@ -189,16 +189,7 @@ func (m *execManager) WakeAgentSocket(ctx context.Context, id, token string) (st func postExec(t *testing.T, ts *httptest.Server, body string) (int, []byte) { t.Helper() - req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, ts.URL+"/v1/sandboxes/sb_1/exec", strings.NewReader(body)) - if err != nil { - t.Fatalf("new request: %v", err) - } - req.Header.Set("Authorization", "Bearer tok") - req.Header.Set("Content-Type", "application/json") - resp, err := ts.Client().Do(req) - if err != nil { - t.Fatalf("post exec: %v", err) - } + resp := postJSON(t, ts.URL+"/v1/sandboxes/sb_1/exec", "tok", body) defer resp.Body.Close() out, err := io.ReadAll(resp.Body) if err != nil { diff --git a/sandboxd/server/metrics.go b/sandboxd/server/metrics.go index 22a20535..6ed442f3 100644 --- a/sandboxd/server/metrics.go +++ b/sandboxd/server/metrics.go @@ -5,15 +5,8 @@ import ( "maps" "net/http" "slices" - - "github.com/cocoonstack/sandbox/sandboxd/pool" ) -// SandboxListResponse is the wire reply of GET /v1/sandboxes. -type SandboxListResponse struct { - Sandboxes []pool.SandboxSummary `json:"sandboxes"` -} - // handleMetrics renders Prometheus text format by hand, counters and gauges only. func (s *Server) handleMetrics(w http.ResponseWriter, _ *http.Request) { pools, g := s.mgr.Info() @@ -84,8 +77,3 @@ func (s *Server) handleMetrics(w http.ResponseWriter, _ *http.Request) { _, _ = fmt.Fprintf(w, "sandboxd_%s %d\n", row.name, row.value) } } - -// handleSandboxes lists the live claims visible to the caller — never tokens. -func (s *Server) handleSandboxes(w http.ResponseWriter, r *http.Request) { - writeJSON(w, http.StatusOK, SandboxListResponse{Sandboxes: s.mgr.Sandboxes(tenantFrom(r.Context()))}) -} diff --git a/sandboxd/server/preview.go b/sandboxd/server/preview.go index 87d22a19..66467ecb 100644 --- a/sandboxd/server/preview.go +++ b/sandboxd/server/preview.go @@ -188,7 +188,7 @@ func (s *Server) handlePreview(w http.ResponseWriter, r *http.Request) { id := r.PathValue("id") deadline, err := s.mgr.ClaimDeadline(id, req.Token) if err != nil { - writePoolErr(w, err) + writeResult(w, r, "preview", id, "preview failed", err, func() {}) return } ttl := req.TTL() diff --git a/sandboxd/server/server.go b/sandboxd/server/server.go index 37133f79..4b23d710 100644 --- a/sandboxd/server/server.go +++ b/sandboxd/server/server.go @@ -128,6 +128,11 @@ type InfoResponse struct { AtCapacityReason string `json:"at_capacity_reason,omitempty"` } +// SandboxListResponse is the wire reply of GET /v1/sandboxes. +type SandboxListResponse struct { + Sandboxes []pool.SandboxSummary `json:"sandboxes"` +} + // PoolUpdateRequest is the wire body of PUT /v1/pools; omitted pools are drained. type PoolUpdateRequest struct { Pools []config.PoolSpec `json:"pools"` @@ -354,17 +359,22 @@ func (s *Server) handleSandbox(w http.ResponseWriter, r *http.Request) { id := r.PathValue("id") sb, ok := s.mgr.Sandbox(id) if !ok { - writeErr(w, http.StatusNotFound, "unknown sandbox") + writePoolErr(w, pool.ErrUnknownSandbox) return } writeJSON(w, http.StatusOK, sb) } +// handleSandboxes lists the live claims visible to the caller — never tokens. +func (s *Server) handleSandboxes(w http.ResponseWriter, r *http.Request) { + writeJSON(w, http.StatusOK, SandboxListResponse{Sandboxes: s.mgr.Sandboxes(tenantFrom(r.Context()))}) +} + func (s *Server) handleSandboxStats(w http.ResponseWriter, r *http.Request) { id := r.PathValue("id") st, ok := s.mgr.Stats(r.Context(), id) if !ok { - writeErr(w, http.StatusNotFound, "unknown sandbox") + writePoolErr(w, pool.ErrUnknownSandbox) return } writeJSON(w, http.StatusOK, st) @@ -495,18 +505,13 @@ func (s *Server) handleDeleteCheckpoint(w http.ResponseWriter, r *http.Request) func (s *Server) handleDeleteTemplate(w http.ResponseWriter, r *http.Request) { q := r.URL.Query() - req := types.ClaimRequest{ - Template: q.Get("template"), - Net: types.NetShape(q.Get("net")), - Size: types.Size(q.Get("size")), - } - key := req.Key() + key := types.PoolKey{Template: q.Get("template"), Net: types.NetShape(q.Get("net")), Size: types.Size(q.Get("size"))}.Defaulted() err := s.mgr.DeleteTemplate(r.Context(), key, tenantFrom(r.Context())) if errors.Is(err, pool.ErrUnknownTemplate) && s.placer != nil && q.Get("no_redirect") == "" && s.writeRedirect(w, s.templateOwners(s.placer.TemplateOwners, key.Hash(), tenantFrom(r.Context()))) { return } - writeResult(w, r, "delete template", req.Template, "delete template failed", err, func() { + writeResult(w, r, "delete template", key.Template, "delete template failed", err, func() { w.WriteHeader(http.StatusNoContent) }) } diff --git a/sandboxd/store/dir/dir.go b/sandboxd/store/dir/dir.go index a56a4d2d..b7883e45 100644 --- a/sandboxd/store/dir/dir.go +++ b/sandboxd/store/dir/dir.go @@ -439,8 +439,8 @@ func collectDigestSources(root string) ([]digestSource, error) { if infoErr != nil { return infoErr } - if !entryInfo.Mode().IsRegular() { - return fmt.Errorf("unsupported export entry %s (%s)", rel, entryInfo.Mode().Type()) + if err := store.RequireRegular(rel, entryInfo.Mode()); err != nil { + return err } sources = append(sources, digestSource{ path: path, diff --git a/sandboxd/store/s3/s3.go b/sandboxd/store/s3/s3.go index 2800eb05..edaeb713 100644 --- a/sandboxd/store/s3/s3.go +++ b/sandboxd/store/s3/s3.go @@ -32,6 +32,11 @@ import ( const ( digestMetadataKey = "content-digest" uploadPartSizeBytes = 16 << 20 + uploadConcurrency = 8 // multipart parts in flight per transfer + metaReadConcurrency = 8 + publishConcurrency = 4 // files in parallel; each already multiparts internally + fetchConcurrency = 4 + deleteBatch = 1000 // DeleteObjects caps one request at 1000 keys ) // Config selects the bucket; credentials come from the standard AWS chain, never this config. @@ -92,7 +97,7 @@ func New(ctx context.Context, cfg Config, stagingRoot string, idRe *regexp.Regex // snapshot exports are hundreds of MB: multipart concurrency keeps transfers bandwidth-bound. tm := transfermanager.New(client, func(o *transfermanager.Options) { o.PartSizeBytes = uploadPartSizeBytes - o.Concurrency = 8 + o.Concurrency = uploadConcurrency }) return &Store{client: client, tm: tm, bucket: cfg.Bucket, prefix: cfg.Prefix, staging: stagingRoot, idRe: idRe}, nil } @@ -154,15 +159,17 @@ func (s *Store) Metas(ctx context.Context) ([][]byte, error) { } metas := make([][]byte, len(ids)) g, gctx := errgroup.WithContext(ctx) - g.SetLimit(8) + g.SetLimit(metaReadConcurrency) for i, id := range ids { g.Go(func() error { raw, err := s.ReadMeta(gctx, id) - if err == nil { - metas[i] = raw - } else if !errors.Is(err, store.ErrNotFound) { - return err // absence mid-list is a race, not a failure + if errors.Is(err, store.ErrNotFound) { + return nil // absence mid-list is a race, not a failure } + if err != nil { + return err + } + metas[i] = raw return nil }) } @@ -187,8 +194,10 @@ func (s *Store) Delete(ctx context.Context, id string) error { return s.deleteKeys(ctx, []string{metaKey}) } -// SweepStaging clears local staging residue at startup; objects orphaned by a crash before meta.json are reclaimed by the bucket's S3 lifecycle rule (see deploy). -func (s *Store) SweepStaging() error { return utils.RemoveDirEntries(s.staging, nil) } +// SweepStaging clears local staging residue at startup. +func (s *Store) SweepStaging() error { + return utils.RemoveDirEntries(s.staging, nil) // objects orphaned before meta.json fall to the bucket's lifecycle rule +} func (s *Store) SweepGenerations() error { return nil } @@ -222,7 +231,7 @@ func (s *Store) publish(ctx context.Context, staging, id string, digested bool) } digestFiles := make([]store.DigestFile, len(files)) g, gctx := errgroup.WithContext(ctx) - g.SetLimit(4) // files in parallel; each already multiparts internally + g.SetLimit(publishConcurrency) for i, file := range files { g.Go(func() error { if !digested { @@ -277,7 +286,7 @@ func (s *Store) publishFiles(staging, id, gen string, digested bool) ([]publishF } files = append(files, publishFile{ path: path, - key: s.key(id, gen+strings.TrimPrefix(rel, store.ExportDir)), + key: s.key(id, gen+"/"+strings.TrimPrefix(rel, store.ExportDir+"/")), }) return nil } @@ -286,8 +295,8 @@ func (s *Store) publishFiles(staging, id, gen string, digested bool) ([]publishF return err } rel = filepath.ToSlash(rel) - if !info.Mode().IsRegular() { - return fmt.Errorf("digest export %s is not a regular file", rel) + if err := store.RequireRegular(rel, info.Mode()); err != nil { + return err } files = append(files, publishFile{ path: path, @@ -325,7 +334,7 @@ func (s *Store) populate(ctx context.Context, id string, meta []byte, gen string return fmt.Errorf("record %s has no export", id) } g, gctx := errgroup.WithContext(ctx) - g.SetLimit(4) + g.SetLimit(fetchConcurrency) for _, key := range keys { g.Go(func() error { return s.download(gctx, key, filepath.Join(local, store.ExportDir, strings.TrimPrefix(key, exportPrefix))) @@ -373,8 +382,8 @@ func (s *Store) uploadDigested(ctx context.Context, key, path, digestPath string if err != nil { return store.DigestFile{}, err } - if !info.Mode().IsRegular() { - return store.DigestFile{}, fmt.Errorf("digest export %s is not a regular file", digestPath) + if err := store.RequireRegular(digestPath, info.Mode()); err != nil { + return store.DigestFile{}, err } reader := newDigestReader(f, digestPath, info.Size()) if err := s.uploadReader(ctx, key, reader, info.Size(), nil); err != nil { @@ -414,8 +423,7 @@ func (s *Store) download(ctx context.Context, key, path string) error { } func (s *Store) deleteKeys(ctx context.Context, keys []string) error { - // DeleteObjects caps one batch at 1000 keys. - for chunk := range slices.Chunk(keys, 1000) { + for chunk := range slices.Chunk(keys, deleteBatch) { objs := make([]s3types.ObjectIdentifier, len(chunk)) for i := range chunk { objs[i] = s3types.ObjectIdentifier{Key: &chunk[i]} diff --git a/sandboxd/store/s3/s3_test.go b/sandboxd/store/s3/s3_test.go index 29d90b3e..e5d17a4a 100644 --- a/sandboxd/store/s3/s3_test.go +++ b/sandboxd/store/s3/s3_test.go @@ -451,8 +451,7 @@ func (m *recordingTransferManager) UploadObject( _ ...func(*transfermanager.Options), ) (*transfermanager.UploadObjectOutput, error) { if input.ContentLength != nil { - length := *input.ContentLength - m.contentLength = &length + m.contentLength = new(*input.ContentLength) } read, err := io.CopyBuffer(io.Discard, input.Body, make([]byte, 113<<10)) m.read = read diff --git a/sandboxd/store/store.go b/sandboxd/store/store.go index b3974011..72a2a095 100644 --- a/sandboxd/store/store.go +++ b/sandboxd/store/store.go @@ -10,6 +10,8 @@ import ( "crypto/sha256" "encoding/hex" "errors" + "fmt" + "io/fs" "regexp" "strings" ) @@ -38,7 +40,7 @@ type Store interface { Publish(ctx context.Context, staging, id string) error // PublishDigested applies Publish semantics and returns the export digest. PublishDigested(ctx context.Context, staging, id string) (string, error) - // Fetch materializes a record's export locally and returns a release to hold until the clone ends. + // Fetch materializes a record's export locally; the caller holds the record lock until the clone ends. Fetch(ctx context.Context, id string) (dir string, meta []byte, digest string, err error) // ReadMeta returns a record's metadata, or an error when the record does not exist. ReadMeta(ctx context.Context, id string) ([]byte, error) @@ -66,3 +68,11 @@ func ExportGenHash(meta []byte) string { sum := sha256.Sum256(meta) return hex.EncodeToString(sum[:8]) } + +// RequireRegular rejects an export entry that is not a regular file. +func RequireRegular(rel string, mode fs.FileMode) error { + if !mode.IsRegular() { + return fmt.Errorf("export entry %s is not a regular file (%s)", rel, mode.Type()) + } + return nil +} diff --git a/sandboxd/store/storetest/storetest.go b/sandboxd/store/storetest/storetest.go index daf0bb81..b1a86a2a 100644 --- a/sandboxd/store/storetest/storetest.go +++ b/sandboxd/store/storetest/storetest.go @@ -51,7 +51,6 @@ func RunContract(t *testing.T, st store.Store) { t.Fatalf("fetched export: %q, %v", got, err) } - // a half-published checkpoint (no meta) is invisible to Metas. orphan, err := st.Stage("ck_00000000000000bb") if err != nil { t.Fatalf("Stage orphan: %v", err) @@ -70,7 +69,6 @@ func RunContract(t *testing.T, st store.Store) { t.Fatalf("SweepStaging: %v", err) } - // re-publish replaces: a leftover first-generation file would corrupt a clone. second, err := st.Stage(id) if err != nil { t.Fatalf("Stage second: %v", err) diff --git a/scripts/bench.sh b/scripts/bench.sh index 2a8c3dac..76a9a676 100755 --- a/scripts/bench.sh +++ b/scripts/bench.sh @@ -64,7 +64,11 @@ stats() { } echo "== build" -if [[ -n ${SANDBOXD_BIN:-} && -n ${DEMO_BIN:-} ]]; then +if [[ -n ${SANDBOXD_BIN:-} || -n ${DEMO_BIN:-} || -n ${RPCBENCH_BIN:-} || -n ${PULLBENCH_BIN:-} ]]; then + [[ -n ${SANDBOXD_BIN:-} && -n ${DEMO_BIN:-} && -n ${RPCBENCH_BIN:-} && -n ${PULLBENCH_BIN:-} ]] || { + echo "prebuilt runs need SANDBOXD_BIN, DEMO_BIN, RPCBENCH_BIN and PULLBENCH_BIN together" + exit 1 + } cp "$SANDBOXD_BIN" "$DATA/sandboxd" && cp "$DEMO_BIN" "$DATA/demo" cp "$RPCBENCH_BIN" "$DATA/rpcbench" && cp "$PULLBENCH_BIN" "$DATA/pullbench" else @@ -179,6 +183,7 @@ cat </dev/null || echo unknown)/$(cat /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference 2>/dev/null || echo -) | | cocoon | $(cocoon version 2>/dev/null | awk '/Version/{print $2; exit}') | +| sandboxd | $("$DATA/sandboxd" -version 2>/dev/null || echo unknown) | | template | $TEMPLATE @ ${digest:-unknown} | | claim tier | p50 | p90 | max | n | diff --git a/sdk/go/checkpoint.go b/sdk/go/checkpoint.go index fa331252..99f42c6d 100644 --- a/sdk/go/checkpoint.go +++ b/sdk/go/checkpoint.go @@ -8,6 +8,31 @@ import ( "time" ) +type checkpointRequest struct { + Token string `json:"token"` + Name string `json:"name,omitempty"` +} + +type checkpointRecord struct { + ID string `json:"id"` + Name string `json:"name,omitempty"` + SandboxID string `json:"sandbox_id"` + CreatedAt time.Time `json:"created_at"` +} + +type checkpointResponse struct { + Checkpoint checkpointRecord `json:"checkpoint"` +} + +type checkpointClaimRequest struct { + TTLSeconds int `json:"ttl_seconds,omitzero"` + NoRedirect bool `json:"no_redirect,omitzero"` +} + +type checkpointListResponse struct { + Checkpoints []checkpointRecord `json:"checkpoints"` +} + // Checkpoint is a captured sandbox state bound to the node that holds it. // New claims fresh sandboxes branched from the captured moment, any number // of times; the source sandbox is unaffected and can keep being @@ -32,8 +57,8 @@ func (ck *Checkpoint) New(ctx context.Context, opts ...Option) (*Sandbox, error) for _, opt := range opts { opt(&claim) } - if len(claim.Volumes) > 0 || claim.ClaimRef != "" { - return nil, errors.New("checkpoint claims do not accept WithVolumes or WithClaimRef") + if len(claim.Volumes) > 0 || claim.VolumesAttachOnly || claim.ClaimRef != "" { + return nil, errors.New("checkpoint claims do not accept WithVolumes, WithVolumesAttachOnly or WithClaimRef") } if err := claim.rejectPinnedAxes(); err != nil { return nil, err @@ -63,9 +88,7 @@ func (ck *Checkpoint) claimAt(ctx context.Context, addr string, body []byte) (cl return doJSON[claimResponse](ctx, ck.c, http.MethodPost, addr, "/v1/checkpoints/"+ck.ID+"/claim", bytes.NewReader(body), ck.c.apiToken, "claim checkpoint") } -// Checkpoint captures the sandbox's full state — memory, disk, running -// processes — without stopping it, and returns a handle that branches new -// sandboxes from that exact moment. name is an optional label. +// Checkpoint captures the sandbox's full state (memory, disk, running processes) without stopping it and returns a handle that branches new sandboxes from that moment. func (s *Sandbox) Checkpoint(ctx context.Context, name string) (*Checkpoint, error) { body, err := encodeBody("checkpoint", checkpointRequest{Token: s.token, Name: name}) if err != nil { @@ -104,28 +127,3 @@ func checkpointHandle(c *Client, addr string, rec checkpointRecord) *Checkpoint c: c, addr: addr, } } - -type checkpointRequest struct { - Token string `json:"token"` - Name string `json:"name,omitempty"` -} - -type checkpointRecord struct { - ID string `json:"id"` - Name string `json:"name,omitempty"` - SandboxID string `json:"sandbox_id"` - CreatedAt time.Time `json:"created_at"` -} - -type checkpointResponse struct { - Checkpoint checkpointRecord `json:"checkpoint"` -} - -type checkpointClaimRequest struct { - TTLSeconds int `json:"ttl_seconds,omitzero"` - NoRedirect bool `json:"no_redirect,omitzero"` -} - -type checkpointListResponse struct { - Checkpoints []checkpointRecord `json:"checkpoints"` -} diff --git a/sdk/go/checkpoint_test.go b/sdk/go/checkpoint_test.go index 493c24a3..0ff3b90a 100644 --- a/sdk/go/checkpoint_test.go +++ b/sdk/go/checkpoint_test.go @@ -45,14 +45,26 @@ func TestCheckpointNewFollowsRedirect(t *testing.T) { } } -func TestCheckpointNewRejectsClaimRefLocally(t *testing.T) { - ck := &Checkpoint{} - sb, err := ck.New(t.Context(), WithClaimRef("ns/claim")) - if err == nil || !strings.Contains(err.Error(), "WithClaimRef") { - t.Errorf("err %v, want local WithClaimRef rejection", err) - } - if sb != nil { - t.Errorf("sandbox %+v, want nil", sb) +func TestCheckpointNewRejectsVolumeOptionsLocally(t *testing.T) { + tests := []struct { + name string + opt Option + }{ + {"WithClaimRef", WithClaimRef("ns/claim")}, + {"WithVolumesAttachOnly", WithVolumesAttachOnly()}, + {"WithVolumes", WithVolumes(Volume{Name: "data"})}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ck := &Checkpoint{} + sb, err := ck.New(t.Context(), tt.opt) + if err == nil || !strings.Contains(err.Error(), tt.name) { + t.Errorf("err %v, want local %s rejection", err, tt.name) + } + if sb != nil { + t.Errorf("sandbox %+v, want nil", sb) + } + }) } } diff --git a/sdk/go/client.go b/sdk/go/client.go index 61515126..64177090 100644 --- a/sdk/go/client.go +++ b/sdk/go/client.go @@ -33,18 +33,6 @@ type claimEncoder func(noRedirect, requirePromoted bool) ([]byte, error) type claimPoster func(addr string, body []byte) (claimResponse, error) -type claimResponse struct { - ID string `json:"id"` - Token string `json:"token"` - Deadline time.Time `json:"deadline"` - OwnerAddr string `json:"owner_addr,omitempty"` - FromCheckpoint string `json:"from_checkpoint,omitempty"` - TemplateDigest string `json:"template_digest,omitempty"` - Volumes []Volume `json:"volumes,omitempty"` - Redirect []string `json:"redirect,omitempty"` - RequirePromoted bool `json:"require_promoted,omitzero"` -} - type volumeListResponse struct { Volumes []VolumeInfo `json:"volumes"` } @@ -122,6 +110,18 @@ func (r claimRequest) validateVolumes() error { return nil } +type claimResponse struct { + ID string `json:"id"` + Token string `json:"token"` + Deadline time.Time `json:"deadline"` + OwnerAddr string `json:"owner_addr,omitempty"` + FromCheckpoint string `json:"from_checkpoint,omitempty"` + TemplateDigest string `json:"template_digest,omitempty"` + Volumes []Volume `json:"volumes,omitempty"` + Redirect []string `json:"redirect,omitempty"` + RequirePromoted bool `json:"require_promoted,omitzero"` +} + // Client talks to one sandboxd node. type Client struct { addr string @@ -132,7 +132,7 @@ type Client struct { keepAlive time.Duration } -// Connect returns a client for a sandboxd node. +// Connect returns a client for a sandboxd node; a comma-separated seed list keeps only its first address. func Connect(addr string, opts ...ClientOption) (*Client, error) { first, _, _ := strings.Cut(addr, ",") first = strings.TrimSpace(first) @@ -188,12 +188,12 @@ func (c *Client) Lookup(ctx context.Context, id, token string) (*Sandbox, error) if owner, err := c.ownerAt(ctx, c.addr, id, token); err == nil { return &Sandbox{ID: id, token: token, c: c, owner: owner}, nil } - addrs, _ := c.peersOrErr(ctx) + addrs, peersErr := c.peersOrErr(ctx) owner, ok := scatter(ctx, addrs, func(ctx context.Context, addr string) (string, error) { return c.ownerAt(ctx, addr, id, token) }) if !ok { - return nil, fmt.Errorf("lookup %s: no owner found", id) + return nil, errors.Join(fmt.Errorf("lookup %s: no owner found", id), peersErr) } return &Sandbox{ID: id, token: token, c: c, owner: owner}, nil } diff --git a/sdk/go/client_test.go b/sdk/go/client_test.go index 57ede843..9ca36a9c 100644 --- a/sdk/go/client_test.go +++ b/sdk/go/client_test.go @@ -491,6 +491,28 @@ func TestLookupScatter(t *testing.T) { } } +func TestLookupReportsAFailedPeerDiscovery(t *testing.T) { + entry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/sandboxes/sb_1/owner": + w.WriteHeader(http.StatusNotFound) + case "/v1/peers": + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = io.WriteString(w, `{"error":"gossip not ready"}`) + } + })) + t.Cleanup(entry.Close) + + _, err := testClient(t, entry).Lookup(t.Context(), "sb_1", "tok") + he, ok := errors.AsType[*APIError](err) + if !ok || he.Verb != "peers" || he.Status != http.StatusServiceUnavailable { + t.Fatalf("Lookup: %v, want the peers 503 joined into the error", err) + } + if !strings.Contains(err.Error(), "no owner found") { + t.Errorf("Lookup: %v, want the no-owner verdict kept alongside", err) + } +} + func TestAPIErrorDrainsOversizedBody(t *testing.T) { body := strings.NewReader(`{"error":"` + strings.Repeat("x", 3*4096) + `"}`) err := apiError("claim", &http.Response{StatusCode: http.StatusTooManyRequests, Body: io.NopCloser(body)}) diff --git a/sdk/go/keepalive_test.go b/sdk/go/keepalive_test.go index d4d855a6..78436354 100644 --- a/sdk/go/keepalive_test.go +++ b/sdk/go/keepalive_test.go @@ -55,6 +55,22 @@ func TestOldDaemonDialsPerCall(t *testing.T) { } } +func TestKeepAliveOffSkipsTheProbe(t *testing.T) { + var upgrades atomic.Int32 + sb := testSandbox(t, newAgentServer(t, func(c net.Conn) { + upgrades.Add(1) + silkdtest.ServeConnOnce(c) + }), WithKeepAlive(0)) + for range 3 { + if out, err := sb.Exec(t.Context(), "echo", "42"); err != nil || out != "42\n" { + t.Fatalf("exec: %q, %v", out, err) + } + } + if got := upgrades.Load(); got != 3 { + t.Errorf("upgrades = %d, want 3: nothing can park, so no probe dial", got) + } +} + func TestOldDaemonConnectionIsNotParked(t *testing.T) { if !canProbe { t.Skip("no parked-connection probe on this platform") diff --git a/sdk/go/port.go b/sdk/go/port.go index 68e74ad9..5af3042d 100644 --- a/sdk/go/port.go +++ b/sdk/go/port.go @@ -14,6 +14,16 @@ import ( "github.com/cocoonstack/sandbox/sdk/go/silkd" ) +type previewRequest struct { + Token string `json:"token"` + Port uint16 `json:"port"` + TTLSeconds int `json:"ttl_seconds,omitzero"` +} + +type previewResponse struct { + URL string `json:"url"` +} + var _ net.Conn = (*PortConn)(nil) // PortConn is a net.Conn to a TCP port inside the sandbox, relayed over the @@ -33,16 +43,7 @@ func (p *PortConn) Read(b []byte) (int, error) { return p.out.Read(b) } // Write chunks b so no frame (with its base64 and envelope overhead) can // exceed the protocol cap — one oversized frame would kill the relay. func (p *PortConn) Write(b []byte) (int, error) { - sent := 0 - for len(b) > 0 { - chunk := b[:min(len(b), wire.PortWriteChunk)] - if err := p.conn.Send(&wire.Data{Data: chunk}); err != nil { - return sent, err - } - sent += len(chunk) - b = b[len(chunk):] - } - return sent, nil + return sendChunks(p.conn, wire.PortWriteChunk, func(chunk []byte) wire.Request { return &wire.Data{Data: chunk} }, b) } // CloseWrite half-closes the guest socket (the server sees EOF) while reads @@ -167,23 +168,13 @@ func (s *Sandbox) proxyConn(ctx context.Context, local net.Conn, port uint16) { <-done } -// Half-close so the peer sees EOF while the tail still drains. +// closeWrite half-closes so the peer sees EOF while the tail still drains. func closeWrite(conn net.Conn) { if cw, ok := conn.(interface{ CloseWrite() error }); ok { _ = cw.CloseWrite() } } -type previewRequest struct { - Token string `json:"token"` - Port uint16 `json:"port"` - TTLSeconds int `json:"ttl_seconds,omitzero"` -} - -type previewResponse struct { - URL string `json:"url"` -} - type portAddr string func (a portAddr) Network() string { return "silkd" } diff --git a/sdk/go/pty.go b/sdk/go/pty.go index 98e9833a..b04db459 100644 --- a/sdk/go/pty.go +++ b/sdk/go/pty.go @@ -40,16 +40,7 @@ func (p *Pty) Read(b []byte) (int, error) { // Write feeds input to the terminal, chunked to stay under the frame cap. func (p *Pty) Write(b []byte) (int, error) { - sent := 0 - for len(b) > 0 { - chunk := b[:min(len(b), stdinChunk)] - if err := p.conn.Send(&wire.Stdin{Data: chunk}); err != nil { - return sent, err - } - sent += len(chunk) - b = b[len(chunk):] - } - return sent, nil + return sendChunks(p.conn, stdinChunk, func(chunk []byte) wire.Request { return &wire.Stdin{Data: chunk} }, b) } // Resize adjusts the terminal window; it is a separate RPC keyed by pid. diff --git a/sdk/go/sandbox.go b/sdk/go/sandbox.go index 11c12ff4..495c8932 100644 --- a/sdk/go/sandbox.go +++ b/sdk/go/sandbox.go @@ -228,7 +228,7 @@ func (s *Sandbox) connect(ctx context.Context, req wire.Request) (*agentConn, er if err != nil { return nil, err } - if s.proto.Load() != 0 { + if s.proto.Load() != 0 || !canProbe || s.c.keepAlive <= 0 { return c.sent(req) } proto, err := c.probeProto(ctx, req) diff --git a/sdk/go/silkd/silkdtest/fake.go b/sdk/go/silkd/silkdtest/fake.go index 52039e9d..458f061b 100644 --- a/sdk/go/silkd/silkdtest/fake.go +++ b/sdk/go/silkd/silkdtest/fake.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bufio" "bytes" + "errors" "io" "io/fs" "maps" @@ -158,9 +159,7 @@ func (f *Fake) fsRm(conn net.Conn, req *wire.FsRm) { } } -// fsPush extracts the uploaded tar under dest; fsPull streams a path back -// as a tar with the basename as the entry root — matching silkd's contract -// closely enough for SDK round-trip tests. +// fsPush extracts the uploaded tar under dest, close enough to silkd for SDK round trips. func (f *Fake) fsPush(conn net.Conn, r *bufio.Reader, dest string) { data, err := drainUpload(r) if err != nil { @@ -171,7 +170,7 @@ func (f *Fake) fsPush(conn net.Conn, r *bufio.Reader, dest string) { tr := tar.NewReader(bytes.NewReader(data)) for { hdr, err := tr.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { @@ -198,6 +197,7 @@ func (f *Fake) fsPush(conn net.Conn, r *bufio.Reader, dest string) { f.done(conn, nil) } +// fsPull streams path back as a tar rooted at its basename. func (f *Fake) fsPull(conn net.Conn, path string) { root := f.abs(path) base := filepath.Base(path) @@ -332,9 +332,7 @@ func (f *Fake) fsReplace(conn net.Conn, req *wire.FsReplace) { send(conn, wire.Done{}) } -// fsWatch fakes a watch: ready, one synthetic created event, then it holds -// the stream open until the client disconnects — the connection-bound -// contract. +// fsWatch answers ready and one synthetic created event, then holds the stream until the client disconnects. func (f *Fake) fsWatch(conn net.Conn, r *bufio.Reader, req *wire.FsWatch) { if _, err := os.Stat(f.abs(req.Path)); err != nil { errFrame(conn, wire.KindNotFound, err.Error()) @@ -384,9 +382,7 @@ func (f *Fake) abs(path string) string { return filepath.Join(f.Root, filepath.Clean("/"+path)) } -// portEcho fakes port_forward: port 1 is refused (not_found, mirroring a -// dead port), anything else answers Ready and echoes Data frames until -// DataEnd (→ Done, like the server closing after EOF) or disconnect. +// portEcho refuses port 1 as not_found and echoes Data frames on any other port until DataEnd (Done) or disconnect. func portEcho(conn net.Conn, r *bufio.Reader, port uint16) { if port == 1 { errFrame(conn, wire.KindNotFound, "connect refused") @@ -411,8 +407,7 @@ func portEcho(conn net.Conn, r *bufio.Reader, port uint16) { } } -// ptyEcho fakes a pty: Started, then echo each stdin chunk as stdout until a -// chunk contains "exit" (→ Exit 0) or the client disconnects. +// ptyEcho echoes each stdin chunk as stdout until a chunk contains "exit" (Exit 0) or the client disconnects. func ptyEcho(conn net.Conn, r *bufio.Reader) { send(conn, &wire.Started{PID: 9999}) for { @@ -430,9 +425,7 @@ func ptyEcho(conn net.Conn, r *bufio.Reader) { } } -// globRegexp mirrors silkd's glob translation exactly — `*`/`?` wildcards, -// everything else literal (never `filepath.Match`, whose char classes silkd -// does not have). The quoted pattern always compiles. +// globRegexp mirrors silkd's translation: * and ? wildcards, everything else literal, never filepath.Match's char classes. func globRegexp(glob string) *regexp.Regexp { pat := regexp.QuoteMeta(glob) pat = strings.ReplaceAll(pat, `\*`, ".*") diff --git a/sdk/go/utils.go b/sdk/go/utils.go index 9227c4df..e7b16a68 100644 --- a/sdk/go/utils.go +++ b/sdk/go/utils.go @@ -3,6 +3,7 @@ package sandbox import ( "cmp" "context" + "errors" "fmt" "io" "iter" @@ -17,7 +18,6 @@ type respPtr[T any] interface { wire.Response } -// doneRPC sends a request that answers with Done or an error frame. func (s *Sandbox) doneRPC(ctx context.Context, req wire.Request) error { conn, l, err := s.call(ctx, req) if err != nil { @@ -49,7 +49,7 @@ func (s *Sandbox) uploadRPC(ctx context.Context, req wire.Request, r io.Reader) return err } } - if readErr == io.EOF { + if errors.Is(readErr, io.EOF) { break } if readErr != nil { @@ -62,7 +62,6 @@ func (s *Sandbox) uploadRPC(ctx context.Context, req wire.Request, r io.Reader) return l.done(<-terminal) } -// downloadRPC sends req and drains its Data stream into sink until Done. func (s *Sandbox) downloadRPC(ctx context.Context, req wire.Request, sink func([]byte) error) error { conn, l, err := s.call(ctx, req) if err != nil { @@ -103,7 +102,6 @@ func pumpStdio(ctx context.Context, conn *silkd.Conn, stdout, stderr io.Writer) } } -// oneShotRPC sends req and returns its single typed reply frame. func oneShotRPC[T any, PT respPtr[T]](ctx context.Context, s *Sandbox, req wire.Request) (*T, error) { conn, l, err := s.call(ctx, req) if err != nil { @@ -114,7 +112,6 @@ func oneShotRPC[T any, PT respPtr[T]](ctx context.Context, s *Sandbox, req wire. return v, l.done(err) } -// collectRPC sends req and gathers every streamed frame of type T until Done. func collectRPC[T any, PT respPtr[T]](ctx context.Context, s *Sandbox, req wire.Request) ([]T, error) { var out []T for v, err := range streamRPC[T, PT](ctx, s, req) { @@ -185,7 +182,6 @@ func drainData(ctx context.Context, conn *silkd.Conn, sink func([]byte) error) e } } -// terminalErr reads one frame and requires it to be Done (else the error). func terminalErr(ctx context.Context, conn *silkd.Conn) error { _, err := expect[wire.Done](ctx, conn) return err @@ -214,7 +210,7 @@ func recv(ctx context.Context, conn *silkd.Conn) (wire.Response, error) { if ctxErr := ctx.Err(); ctxErr != nil { return nil, ctxErr } - if err == io.EOF { + if errors.Is(err, io.EOF) { return nil, fmt.Errorf("connection closed before a terminal frame") } return nil, err @@ -225,3 +221,17 @@ func recv(ctx context.Context, conn *silkd.Conn) (wire.Response, error) { func unexpected(resp wire.Response) error { return fmt.Errorf("unexpected frame %q", resp.RespType()) } + +// sendChunks feeds b to conn in frames of at most size bytes, so no frame outgrows the protocol cap. +func sendChunks(conn *silkd.Conn, size int, frame func([]byte) wire.Request, b []byte) (int, error) { + sent := 0 + for len(b) > 0 { + chunk := b[:min(len(b), size)] + if err := conn.Send(frame(chunk)); err != nil { + return sent, err + } + sent += len(chunk) + b = b[len(chunk):] + } + return sent, nil +} diff --git a/sdk/go/volumes_test.go b/sdk/go/volumes_test.go index 483b8f1d..ac3617df 100644 --- a/sdk/go/volumes_test.go +++ b/sdk/go/volumes_test.go @@ -118,24 +118,35 @@ func TestTemplateNewVolumeClaimFollowsRedirect(t *testing.T) { } func TestClientVolumes(t *testing.T) { - want := []VolumeInfo{{Name: "imagenet", DefaultMount: "/volumes/imagenet", SizeBytes: 42, Available: true, Nodes: 3}} - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet || r.URL.Path != "/v1/volumes" { - t.Errorf("request = %s %s", r.Method, r.URL.Path) - } - if got := r.Header.Get("Authorization"); got != "Bearer sekret" { - t.Errorf("authorization = %q", got) - } - _ = json.NewEncoder(w).Encode(volumeListResponse{Volumes: want}) - })) - t.Cleanup(ts.Close) - - got, err := testClient(t, ts, WithAPIToken("sekret")).Volumes(t.Context()) - if err != nil { - t.Fatalf("Volumes: %v", err) + imagenet := VolumeInfo{Name: "imagenet", DefaultMount: "/volumes/imagenet", SizeBytes: 42, Available: true, Nodes: 3} + tests := []struct { + name string + want []VolumeInfo + }{ + {"read-only catalog", []VolumeInfo{imagenet}}, + {"writable entry decodes", []VolumeInfo{imagenet, {Name: "scratch", DefaultMount: "/volumes/scratch", SizeBytes: 7, Available: true, Nodes: 1, Writable: true}}}, } - if !slices.Equal(got, want) { - t.Errorf("volumes = %+v, want %+v", got, want) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/v1/volumes" { + t.Errorf("request = %s %s", r.Method, r.URL.Path) + } + if got := r.Header.Get("Authorization"); got != "Bearer sekret" { + t.Errorf("authorization = %q", got) + } + _ = json.NewEncoder(w).Encode(volumeListResponse{Volumes: tt.want}) + })) + t.Cleanup(ts.Close) + + got, err := testClient(t, ts, WithAPIToken("sekret")).Volumes(t.Context()) + if err != nil { + t.Fatalf("Volumes: %v", err) + } + if !slices.Equal(got, tt.want) { + t.Errorf("volumes = %+v, want %+v", got, tt.want) + } + }) } } @@ -304,22 +315,3 @@ func TestSandboxVolumesEchoMode(t *testing.T) { t.Errorf("volumes = %+v, want %+v", sb.Volumes, want) } } - -func TestClientVolumesDecodesWritable(t *testing.T) { - want := []VolumeInfo{ - {Name: "imagenet", DefaultMount: "/volumes/imagenet", SizeBytes: 42, Available: true, Nodes: 3}, - {Name: "scratch", DefaultMount: "/volumes/scratch", SizeBytes: 7, Available: true, Nodes: 1, Writable: true}, - } - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _ = json.NewEncoder(w).Encode(volumeListResponse{Volumes: want}) - })) - t.Cleanup(ts.Close) - - got, err := testClient(t, ts).Volumes(t.Context()) - if err != nil { - t.Fatalf("Volumes: %v", err) - } - if !slices.Equal(got, want) { - t.Errorf("volumes = %+v, want %+v", got, want) - } -} diff --git a/sdk/langchain/README.md b/sdk/langchain/README.md index 665f1cda..9d50d9fb 100644 --- a/sdk/langchain/README.md +++ b/sdk/langchain/README.md @@ -9,6 +9,7 @@ directory listing, with disk state persisting across calls. ```python from cocoonsandbox_langchain import CocoonToolkit +from langgraph.prebuilt import create_react_agent # langgraph is a separate install with CocoonToolkit("10.0.0.5:7777", api_token="...") as kit: agent = create_react_agent(model, kit.get_tools()) diff --git a/sdk/langchain/cocoonsandbox_langchain/toolkit.py b/sdk/langchain/cocoonsandbox_langchain/toolkit.py index 32bad383..048eb918 100644 --- a/sdk/langchain/cocoonsandbox_langchain/toolkit.py +++ b/sdk/langchain/cocoonsandbox_langchain/toolkit.py @@ -11,8 +11,8 @@ import time from collections.abc import Callable -from cocoonsandbox import Client, Sandbox -from langchain_core.tools import StructuredTool +from cocoonsandbox import Client, Sandbox, SilkdError +from langchain_core.tools import StructuredTool, ToolException from pydantic import BaseModel, Field # one call's wall clock over the claim, the dial and the command; the tool description states it @@ -34,12 +34,7 @@ class PathInput(BaseModel): class CocoonToolkit: - """LangChain tools backed by one sandbox, claimed on first tool use. - - from_checkpoint branches a fresh sandbox from that checkpoint's captured - moment instead of claiming a clean template — agents resume from - prepared state. - """ + """LangChain tools over one sandbox, claimed on first tool use; from_checkpoint branches it from a checkpoint.""" def __init__( self, @@ -105,9 +100,7 @@ def get_tools(self) -> list[StructuredTool]: ] def close(self) -> None: - """Releases the sandbox; safe to call twice or before any claim. - Tools invoked after close raise instead of silently claiming a - sandbox nothing would ever release.""" + """Releases the sandbox; safe to call twice or before any claim, and tools invoked afterwards raise.""" with self._lock: sb, self._sb = self._sb, None self._closed = True @@ -133,7 +126,7 @@ async def arun(**kwargs): return await asyncio.to_thread(func, **kwargs) return StructuredTool.from_function( - func=func, coroutine=arun, name=name, description=description, args_schema=schema + func=func, coroutine=arun, name=name, description=description, args_schema=schema, handle_tool_error=True ) def _exec(self, command: str, cwd: str = "") -> str: @@ -165,7 +158,12 @@ def _write_file(self, path: str, content: str) -> str: return f"wrote {path}" def _read_file(self, path: str) -> str: - return self.sandbox().read_file(path).decode(errors="replace") + try: + return self.sandbox().read_file(path).decode(errors="replace") + except SilkdError as exc: + if exc.kind != "not_found": + raise + raise ToolException(f"{path}: no such file") from None def _list_dir(self, path: str) -> str: return json.dumps(self.sandbox().list_dir(path)) diff --git a/sdk/langchain/pyproject.toml b/sdk/langchain/pyproject.toml index e5579bd1..02cabc58 100644 --- a/sdk/langchain/pyproject.toml +++ b/sdk/langchain/pyproject.toml @@ -13,10 +13,3 @@ dependencies = ["langchain-core>=1.0", "pydantic>=2", "cocoonstack-sandbox>=0.1. [tool.setuptools.packages.find] include = ["cocoonsandbox_langchain*"] - -[tool.ruff] -line-length = 120 -target-version = "py310" - -[tool.ruff.lint] -select = ["E", "F", "W", "I", "UP", "B", "SIM"] diff --git a/sdk/langchain/tests/test_toolkit.py b/sdk/langchain/tests/test_toolkit.py index 73cde3c0..59cdcb85 100644 --- a/sdk/langchain/tests/test_toolkit.py +++ b/sdk/langchain/tests/test_toolkit.py @@ -5,7 +5,7 @@ import time import pytest - +from cocoonsandbox import SilkdError from cocoonsandbox_langchain import CocoonToolkit from cocoonsandbox_langchain.toolkit import CALL_TIMEOUT @@ -117,3 +117,14 @@ def hooked(monkeypatch): fake = FakeSandbox() monkeypatch.setattr(kit, "_claim", lambda deadline=None: fake) return kit, fake + + +def test_read_file_reports_a_missing_path_as_a_tool_error(monkeypatch): + kit, fake = hooked(monkeypatch) + + def missing(path): + raise SilkdError("not_found", "no such file") + + fake.read_file = missing + tool = next(t for t in kit.get_tools() if t.name == "sandbox_read_file") + assert "no such file" in tool.invoke({"path": "/nope"}) diff --git a/sdk/openai/cocoonsandbox_openai/adapter.py b/sdk/openai/cocoonsandbox_openai/adapter.py index 5fd676a7..21834ede 100644 --- a/sdk/openai/cocoonsandbox_openai/adapter.py +++ b/sdk/openai/cocoonsandbox_openai/adapter.py @@ -6,6 +6,7 @@ from __future__ import annotations import asyncio +import contextlib import io import socket import uuid @@ -142,19 +143,24 @@ async def create( ) -> SandboxSession: client = Client(options.addr, api_token=options.api_token) sb = await asyncio.to_thread(client.new, options.template, net=options.net, ttl_seconds=options.ttl_seconds) - if manifest is None: - manifest = Manifest(root="/workspace") - session_id = uuid.uuid4() - state = CocoonSandboxSessionState( - session_id=session_id, - manifest=manifest, - snapshot=resolve_snapshot(snapshot, str(session_id)), - addr=options.addr, - api_token=options.api_token, - sandbox_id=sb.id, - sandbox_token=sb.token, - owner=sb.owner, - ) + try: + if manifest is None: + manifest = Manifest(root="/workspace") + session_id = uuid.uuid4() + state = CocoonSandboxSessionState( + session_id=session_id, + manifest=manifest, + snapshot=resolve_snapshot(snapshot, str(session_id)), + addr=options.addr, + api_token=options.api_token, + sandbox_id=sb.id, + sandbox_token=sb.token, + owner=sb.owner, + ) + except BaseException: + with contextlib.suppress(Exception): # the construction error is the one worth reading + await asyncio.to_thread(sb.close) + raise return self._wrap_session(CocoonSandboxSession.from_state(state)) async def delete(self, session: SandboxSession) -> SandboxSession: diff --git a/sdk/openai/pyproject.toml b/sdk/openai/pyproject.toml index 87aa4db7..2a24c4c9 100644 --- a/sdk/openai/pyproject.toml +++ b/sdk/openai/pyproject.toml @@ -13,10 +13,3 @@ dependencies = ["openai-agents>=0.17", "cocoonstack-sandbox>=0.1.11"] [tool.setuptools.packages.find] include = ["cocoonsandbox_openai*"] - -[tool.ruff] -line-length = 120 -target-version = "py310" - -[tool.ruff.lint] -select = ["E", "F", "W", "I", "UP", "B", "SIM"] diff --git a/sdk/openai/tests/test_adapter.py b/sdk/openai/tests/test_adapter.py index d0ff30a3..146738ed 100644 --- a/sdk/openai/tests/test_adapter.py +++ b/sdk/openai/tests/test_adapter.py @@ -12,7 +12,6 @@ import pytest from cocoonsandbox import ProtocolError, SilkdError - from cocoonsandbox_openai import CocoonSandboxClient, CocoonSandboxClientOptions, CocoonSandboxSessionState @@ -171,3 +170,39 @@ async def go(): assert calls["push"] == ("/workspace", b"tar-in") asyncio.run(go()) + + +def test_create_keeps_the_construction_error_when_the_release_also_fails(node, monkeypatch): + def bad_snapshot(*args, **kwargs): + raise ValueError("bad snapshot") + + def bad_close(self): + raise RuntimeError("node gone") + + monkeypatch.setattr("cocoonsandbox_openai.adapter.resolve_snapshot", bad_snapshot) + monkeypatch.setattr("cocoonsandbox.Sandbox.close", bad_close) + + async def go(): + client = CocoonSandboxClient() + with pytest.raises(ValueError, match="bad snapshot"): + await client.create(options=CocoonSandboxClientOptions(addr=node)) + + asyncio.run(go()) + + +def test_create_releases_the_claim_when_state_construction_fails(node, monkeypatch): + released = [] + + def bad_snapshot(*args, **kwargs): + raise ValueError("bad snapshot") + + monkeypatch.setattr("cocoonsandbox_openai.adapter.resolve_snapshot", bad_snapshot) + monkeypatch.setattr("cocoonsandbox.Sandbox.close", lambda self: released.append(self.id)) + + async def go(): + client = CocoonSandboxClient() + with pytest.raises(ValueError): + await client.create(options=CocoonSandboxClientOptions(addr=node)) + + asyncio.run(go()) + assert released == ["sb_1"] diff --git a/sdk/python/README.md b/sdk/python/README.md index 74b1849e..e2461c25 100644 --- a/sdk/python/README.md +++ b/sdk/python/README.md @@ -15,8 +15,9 @@ with client.new("ghcr.io/cocoonstack/sandbox/rt:24.04") as sb: ``` stdlib-only and synchronous — `pip install cocoonstack-sandbox` brings no -dependencies. The surface mirrors the Go SDK: exec/run, streaming files, -tar push/pull, find/replace, watch, persistent sessions, git verbs, pty, +dependencies. The surface mirrors the Go SDK (files and tar streams as whole +`bytes` values): exec/run, file read/write, tar push/pull, find/replace, +watch, persistent sessions, git verbs, pty, port dial/proxy/preview URLs, background processes (spawn/ps/kill/ logs/attach), fork, hibernate, promote, checkpoints, and the LSP broker. Wire fidelity is pinned by the shared protocol fixture corpus (Rust + Go + diff --git a/sdk/python/cocoonsandbox/__init__.py b/sdk/python/cocoonsandbox/__init__.py index 46513e08..0a14923e 100644 --- a/sdk/python/cocoonsandbox/__init__.py +++ b/sdk/python/cocoonsandbox/__init__.py @@ -11,7 +11,7 @@ from .checkpoint import Checkpoint from .client import Client -from .errors import APIError, ExitError, ProtocolError, SandboxError, SilkdError +from .errors import APIError, ExitError, ProtocolError, SandboxError, SandboxTimeout, SilkdError from .sandbox import Lsp, PortConn, Pty, Sandbox, Session, Watcher from .template import Template @@ -26,6 +26,7 @@ "Pty", "Sandbox", "SandboxError", + "SandboxTimeout", "Session", "SilkdError", "Template", diff --git a/sdk/python/cocoonsandbox/checkpoint.py b/sdk/python/cocoonsandbox/checkpoint.py index 3a1afe84..f76dba98 100644 --- a/sdk/python/cocoonsandbox/checkpoint.py +++ b/sdk/python/cocoonsandbox/checkpoint.py @@ -4,6 +4,8 @@ from typing import TYPE_CHECKING, Any +from .errors import require_field + if TYPE_CHECKING: from .client import Client from .sandbox import Sandbox @@ -15,7 +17,7 @@ class Checkpoint: def __init__(self, client: Client, addr: str, rec: dict[str, Any]) -> None: self._client = client self._addr = addr - self.id = rec["id"] + self.id = require_field(rec, "id", "checkpoint") self.name = rec.get("name", "") self.sandbox_id = rec.get("sandbox_id", "") self.created_at = rec.get("created_at", "") diff --git a/sdk/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index c59d6db2..e50bb6e8 100644 --- a/sdk/python/cocoonsandbox/client.py +++ b/sdk/python/cocoonsandbox/client.py @@ -17,7 +17,7 @@ from .checkpoint import Checkpoint from .conn import Conn, dial_agent, remaining_timeout from .endpoint import _endpoint_url -from .errors import APIError +from .errors import APIError, SandboxTimeout, require_field from .sandbox import Sandbox T = TypeVar("T") @@ -37,7 +37,10 @@ def __init__( ssl_context: ssl.SSLContext | None = None, keep_alive: float = 30.0, ) -> None: - """keep_alive keeps a handle's idle relay connection, which holds the sandbox's idle clock; 0 dials per call.""" + """keep_alive keeps a handle's idle relay connection, which holds the sandbox's idle clock; 0 dials per call. + + ssl_context is set to offer ALPN http/1.1, which the relay upgrade needs; pass a dedicated context. + """ endpoint = _endpoint_url(addr.split(",")[0].strip()) self.addr = endpoint.geturl().removeprefix("http://") self._scheme = endpoint.scheme @@ -145,7 +148,7 @@ def _claim_from( reply = self._post_json(addr, path, claim, verb, deadline=deadline) redirect = reply.get("redirect") or [] if not redirect: - return self._handle_from(addr, reply) + return self._handle_from(addr, reply, verb) claim["no_redirect"] = True if reply.get("require_promoted"): claim["require_promoted"] = True @@ -154,7 +157,7 @@ def post(peer: str) -> dict[str, Any]: return self._post_json(peer, path, claim, verb, deadline=deadline) owner, reply = _redirect_fallback(addr, redirect, post, verb) - return self._handle_from(owner, reply) + return self._handle_from(owner, reply, verb) def _peers(self) -> list[str]: try: @@ -167,11 +170,11 @@ def _peers(self) -> list[str]: except APIError: return [] - def _handle_from(self, dialed: str, reply: dict[str, Any]) -> Sandbox: + def _handle_from(self, dialed: str, reply: dict[str, Any], verb: str = "claim") -> Sandbox: return Sandbox( client=self, - id=reply["id"], - token=reply["token"], + id=require_field(reply, "id", verb), + token=require_field(reply, "token", verb), owner=reply.get("owner_addr") or dialed, deadline=reply.get("deadline", ""), from_checkpoint=reply.get("from_checkpoint", ""), @@ -215,7 +218,7 @@ def _request( raise APIError(verb, exc.code, detail) from None except (urllib.error.URLError, OSError, http.client.HTTPException) as exc: if deadline is not None and time.monotonic() >= deadline: - raise TimeoutError(f"{verb} timed out") from None + raise SandboxTimeout(f"{verb} timed out") from None detail = str(exc.reason) if isinstance(exc, urllib.error.URLError) else str(exc) raise APIError(verb, 0, detail) from None if not raw: diff --git a/sdk/python/cocoonsandbox/conn.py b/sdk/python/cocoonsandbox/conn.py index cb420def..d3a0fb21 100644 --- a/sdk/python/cocoonsandbox/conn.py +++ b/sdk/python/cocoonsandbox/conn.py @@ -12,7 +12,7 @@ from collections.abc import Iterator from typing import Any, BinaryIO, Protocol, TypeVar -from .errors import APIError, ProtocolError, SilkdError +from .errors import APIError, ProtocolError, SandboxTimeout, SilkdError from .frames import MAX_FRAME, decode_response, encode_request KEEP_ALIVE_CONNS = 8 @@ -42,6 +42,9 @@ def __init__(self, sock: socket.socket, reader: BinaryIO) -> None: def send(self, op: str, **fields: object) -> None: self._sock.sendall(encode_request(op, **fields)) + def settimeout(self, timeout: float | None) -> None: + self._sock.settimeout(timeout) + def abort(self) -> None: """Unblocks a socket operation from another thread; close() still owns the socket.""" with contextlib.suppress(OSError): @@ -223,5 +226,5 @@ def remaining_timeout(timeout: float, deadline: float | None, what: str = "agent return timeout remaining = deadline - time.monotonic() if remaining <= 0: - raise TimeoutError(f"{what} timed out") + raise SandboxTimeout(f"{what} timed out") return min(timeout, remaining) diff --git a/sdk/python/cocoonsandbox/errors.py b/sdk/python/cocoonsandbox/errors.py index 4b8f16eb..41f58f4a 100644 --- a/sdk/python/cocoonsandbox/errors.py +++ b/sdk/python/cocoonsandbox/errors.py @@ -1,10 +1,16 @@ """Error types raised by the SDK.""" +from typing import Any + class SandboxError(Exception): """Base class for all SDK errors.""" +class SandboxTimeout(SandboxError, TimeoutError): + """A caller's timeout or deadline expired before the call finished.""" + + class APIError(SandboxError): """A sandboxd control-plane call failed.""" @@ -41,3 +47,10 @@ def __init__(self, code: int, stderr: str, stdout: str = ""): class ProtocolError(SandboxError): """The peer broke the frame protocol (unexpected frame, oversized line).""" + + +def require_field(reply: dict[str, Any], key: str, verb: str) -> Any: + """The named member of a control-plane reply, or an APIError naming the verb that got a truncated one.""" + if key not in reply: + raise APIError(verb, 0, f"reply without {key}") + return reply[key] diff --git a/sdk/python/cocoonsandbox/frames.py b/sdk/python/cocoonsandbox/frames.py index 2bfcefce..64dd4aa6 100644 --- a/sdk/python/cocoonsandbox/frames.py +++ b/sdk/python/cocoonsandbox/frames.py @@ -12,12 +12,15 @@ KEEP_ALIVE_PROTO = 2 MAX_FRAME = 8 * 1024 * 1024 FS_CHUNK = 256 * 1024 -# bulk streams chunk larger than silkd's FS_CHUNK: fewer frames per byte, still under MAX_FRAME after base64. +# tar and port streams chunk at 1 MiB: fewer frames per byte, still under MAX_FRAME after base64 BULK_CHUNK = 1 << 20 def encode_request(op: str, **fields: object) -> bytes: """Encodes one request, omitting None fields and base64-encoding byte values.""" + if len(fields) == 1 and isinstance(data := fields.get("data"), (bytes, bytearray, memoryview)): + # base64 needs no JSON escaping, so a bare data frame renders without a str round trip + return b'{"v":%d,"op":"%s","data":"%s"}\n' % (PROTO_VERSION, op.encode(), base64.b64encode(data)) frame = {"v": PROTO_VERSION, "op": op} for key, value in fields.items(): if value is None: diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index 0d6e988c..3e9c8e19 100644 --- a/sdk/python/cocoonsandbox/sandbox.py +++ b/sdk/python/cocoonsandbox/sandbox.py @@ -10,8 +10,8 @@ from typing import TYPE_CHECKING, Any, cast from .checkpoint import Checkpoint -from .conn import Conn, ConnPool, _Closeable -from .errors import APIError, ExitError, ProtocolError, SandboxError, SilkdError +from .conn import Conn, ConnPool, _Closeable, remaining_timeout +from .errors import APIError, ExitError, ProtocolError, SandboxError, SandboxTimeout, SilkdError, require_field from .frames import BULK_CHUNK, FS_CHUNK, KEEP_ALIVE_PROTO from .template import Template @@ -102,7 +102,7 @@ def run( conn = self._connect(deadline) except (ProtocolError, TimeoutError): if deadline is not None and time.monotonic() >= deadline: - raise TimeoutError(f"command did not finish within {timeout}s") from None + raise SandboxTimeout(f"command did not finish within {timeout}s") from None raise with self._lease(conn): watchdog = _arm_watchdog(conn, deadline, expired) @@ -125,7 +125,7 @@ def run( code = _pump_stdio(conn, on_stdout, on_stderr) except (ProtocolError, OSError): if expired.is_set(): - raise TimeoutError(f"command did not finish within {timeout}s") from None + raise SandboxTimeout(f"command did not finish within {timeout}s") from None raise except SilkdError: if pump is not None: @@ -136,8 +136,8 @@ def run( watchdog.cancel() if pump is not None: pump.join() # the pump's frames must not land on the next RPC - if code is None: - raise ProtocolError("exec stream ended without an exit frame") + if code is None: + raise ProtocolError("exec stream ended without an exit frame") return code def spawn(self, *argv: str, cwd: str = "", env: dict[str, str] | None = None, user: str = "") -> int: @@ -145,11 +145,11 @@ def spawn(self, *argv: str, cwd: str = "", env: dict[str, str] | None = None, us started = self._call( "exec", "started", argv=list(argv), cwd=cwd or None, env=env, user=user or None, detach=True ) - return cast(int, started["pid"]) + return cast(int, _need(started, "pid")) def ps(self) -> list[dict[str, Any]]: """Lists tracked processes: {pid, argv, detached, state, exit_code?, started_at_epoch_secs}.""" - return cast(list[dict[str, Any]], self._call("ps", "procs")["procs"]) + return cast(list[dict[str, Any]], _need(self._call("ps", "procs"), "procs")) def kill(self, pid: int, signal: int | None = None) -> None: """Signals a tracked process (default SIGKILL); killing one that already exited is a no-op success.""" @@ -196,7 +196,7 @@ def list_dir(self, path: str) -> list[dict[str, Any]]: def stat(self, path: str) -> dict[str, Any]: """Returns {kind, size, mode, mtime_epoch_secs} for path.""" - return cast(dict[str, Any], self._call("fs_stat", "stat", path=path)["info"]) + return cast(dict[str, Any], _need(self._call("fs_stat", "stat", path=path), "info")) def mkdir(self, path: str, parents: bool = False) -> None: self._done_rpc("fs_mkdir", path=path, parents=parents or None) @@ -280,7 +280,7 @@ def watch(self, path: str, recursive: bool = False) -> Watcher: def session(self, cwd: str = "", env: dict[str, str] | None = None) -> Session: """Creates a persistent shell: cd/export/aliases survive across exec calls routed into it.""" created = self._call("session_create", "session_created", cwd=cwd or None, env=env) - return Session(self, created["id"]) + return Session(self, _need(created, "id")) def sessions(self) -> list[str]: return self._call("session_list", "sessions").get("sessions") or [] @@ -291,7 +291,7 @@ def fork(self, count: int, ttl_seconds: int = 0) -> list[Sandbox]: if ttl_seconds: body["ttl_seconds"] = ttl_seconds reply = self._client._post_json(self.owner, f"/v1/sandboxes/{self.id}/fork", body, "fork") - return [self._client._handle_from(self.owner, child) for child in reply.get("children") or []] + return [self._client._handle_from(self.owner, child, "fork") for child in reply.get("children") or []] def hibernate(self) -> None: """Snapshots and stops the VM; the next guest call restores its state.""" @@ -306,14 +306,14 @@ def checkpoint(self, name: str = "") -> Checkpoint: if name: body["name"] = name reply = self._client._post_json(self.owner, f"/v1/sandboxes/{self.id}/checkpoint", body, "checkpoint") - return Checkpoint(self._client, self.owner, reply["checkpoint"]) + return Checkpoint(self._client, self.owner, require_field(reply, "checkpoint", "checkpoint")) def promote(self, template: str) -> Template: """Publishes a claimable template and returns an owner-bound handle.""" reply = self._client._post_json( self.owner, f"/v1/sandboxes/{self.id}/promote", {"token": self.token, "template": template}, "promote" ) - key = reply["key"] + key = require_field(reply, "key", "promote") return Template( self._client, self.owner, @@ -326,7 +326,7 @@ def promote(self, template: str) -> Template: def start_lsp(self, language: str, root: str = "") -> Lsp: """Starts the registered language server; missing servers raise not_found.""" started = self._call("lsp_start", "lsp_started", language=language, root=root or None) - return Lsp(self, started["server_id"]) + return Lsp(self, _need(started, "server_id")) def open_pty( self, cols: int = 80, rows: int = 24, cwd: str = "", env: dict[str, str] | None = None, user: str = "" @@ -335,7 +335,7 @@ def open_pty( conn, started = self._open_stream( "pty_open", expect="started", cols=cols, rows=rows, cwd=cwd or None, env=env, user=user or None ) - return Pty(self, conn, started["pid"]) + return Pty(self, conn, _need(started, "pid")) def proxy_port(self, local_addr: str, port: int) -> socket.socket: """Serves a guest port on a local socket; closing the listener stops new connections.""" @@ -350,7 +350,7 @@ def preview_url(self, port: int, ttl_seconds: int = 0) -> str: if ttl_seconds: body["ttl_seconds"] = ttl_seconds reply = self._client._post_json(self.owner, f"/v1/sandboxes/{self.id}/preview", body, "preview") - return cast(str, reply["url"]) + return cast(str, require_field(reply, "url", "preview")) def dial_port(self, port: int) -> PortConn: """Opens a byte stream to 127.0.0.1:port inside the guest.""" @@ -380,9 +380,11 @@ def _connect(self, deadline: float | None = None) -> Conn: if self._proto or self._client.keep_alive <= 0: return conn try: + conn.settimeout(remaining_timeout(self._client.timeout, deadline, "agent probe")) conn.send("info") proto = int(_expect(conn, "info").get("proto") or 1) - except Exception: + conn.settimeout(None) + except BaseException: conn.close() raise self._proto = proto @@ -417,7 +419,7 @@ def _open_stream(self, op: str, expect: str = "ready", **fields: object) -> tupl try: conn.send(op, **fields) frame = _expect(conn, expect) - except Exception: + except BaseException: conn.close() raise return conn, frame @@ -498,7 +500,8 @@ def exec(self, *argv: str) -> str: return self._sandbox.exec(*argv, session=self.id) def close(self) -> None: - self._sandbox._done_rpc("session_rm", id=self.id) + """Ends the shell; a session already gone is not an error.""" + _rpc_ignoring_not_found(self._sandbox, "session_rm", id=self.id) class Watcher(_Closeable): @@ -506,6 +509,7 @@ class Watcher(_Closeable): def __init__(self, conn: Conn) -> None: self._conn = conn + self._closed = False self.error: Exception | None = None def __iter__(self) -> Iterator[dict[str, Any]]: @@ -513,12 +517,14 @@ def __iter__(self) -> Iterator[dict[str, Any]]: try: frame = self._conn.recv() except ProtocolError as e: - self.error = e + if not self._closed: + self.error = e return if frame["type"] == "event": yield frame def close(self) -> None: + self._closed = True self._conn.close() @@ -530,17 +536,21 @@ def __init__(self, sandbox: Sandbox, conn: Conn, pid: int) -> None: self._conn = conn self.pid = pid self.exit_code: int | None = None + self._eof = False def read(self) -> bytes: """The next output chunk; b'' once the shell exits, after which exit_code holds the shell's status.""" + if self._eof: + return b"" frame = self._conn.recv() if frame["type"] == "exit": self.exit_code = frame.get("code") + self._eof = True return b"" return frame.get("data") or b"" def write(self, data: bytes) -> None: - self._conn.send("stdin", data=data) + _send_chunks(self._conn, data, op="stdin") def resize(self, cols: int, rows: int) -> None: self._sandbox._done_rpc("pty_resize", pid=self.pid, cols=cols, rows=rows) @@ -557,13 +567,13 @@ def __init__(self, sandbox: Sandbox, server_id: str) -> None: self.server_id = server_id def request(self) -> PortConn: - """Opens the JSON-RPC byte stream: writes go to the server's stdin, recv returns its stdout.""" + """Opens the one JSON-RPC stream a server serves; its end reaps the server, so start a new one to work again.""" conn, _ = self._sandbox._open_stream("lsp_request", server_id=self.server_id) return PortConn(conn) def stop(self) -> None: - """Kills the language server.""" - self._sandbox._done_rpc("lsp_stop", server_id=self.server_id) + """Kills the language server early; one already reaped is not an error.""" + _rpc_ignoring_not_found(self._sandbox, "lsp_stop", server_id=self.server_id) close = stop @@ -644,6 +654,20 @@ def _expect(conn: Conn, frame_type: str) -> dict[str, Any]: return frame +def _need(frame: dict[str, Any], key: str) -> Any: + if key not in frame: + raise ProtocolError(f"{frame['type']} frame without {key}") + return frame[key] + + +def _rpc_ignoring_not_found(sandbox: Sandbox, op: str, **fields: object) -> None: + try: + sandbox._done_rpc(op, **fields) + except SilkdError as exc: + if exc.kind != "not_found": + raise + + def _drain_data(conn: Conn) -> bytes: chunks = [] for frame in conn.recv_until("done"): diff --git a/sdk/python/cocoonsandbox/template.py b/sdk/python/cocoonsandbox/template.py index d367e403..7573ce29 100644 --- a/sdk/python/cocoonsandbox/template.py +++ b/sdk/python/cocoonsandbox/template.py @@ -43,7 +43,6 @@ def new( return self._client._handle_from(self._addr, reply) def delete(self) -> None: - """Removes the template from its node.""" from .client import _template_query query = _template_query(self.name, self.net, self.size) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index a28f8b9b..4e6cbd22 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -1,18 +1,52 @@ """Fixtures and helpers shared by the suites: in-process fake nodes, addresses that refuse a connection, and the relay-side upgrade handshake.""" +import json import socket import threading import time -from http.server import HTTPServer +from http.server import BaseHTTPRequestHandler, HTTPServer from typing import BinaryIO, Callable import pytest -from test_client import FakeNode from cocoonsandbox import Client, Sandbox +class FakeNode(BaseHTTPRequestHandler): + routes = {} + + def do_POST(self): + self._dispatch("POST") + + def do_GET(self): + self._dispatch("GET") + + def do_DELETE(self): + self._dispatch("DELETE") + + def log_message(self, *args): + pass + + def _dispatch(self, method): + length = int(self.headers.get("Content-Length") or 0) + body = json.loads(self.rfile.read(length)) if length else {} + handler = self.routes.get((method, self.path.split("?")[0])) + if handler is None: + self._reply(404, {"error": "no route"}) + return + code, reply = handler(body, self.path) + self._reply(code, reply) + + def _reply(self, code, payload): + raw = json.dumps(payload).encode() + self.send_response(code) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(raw))) + self.end_headers() + self.wfile.write(raw) + + def sandbox_at(addr: str, **client_kwargs) -> Sandbox: return Sandbox(client=Client(addr, **client_kwargs), id="sb_1", token="tok", owner=addr) @@ -50,6 +84,28 @@ def spawn(routes): server.shutdown() +@pytest.fixture +def raw_reply(): + servers = [] + + def serve(response: bytes) -> str: + server = socket.create_server(("127.0.0.1", 0)) + servers.append(server) + + def answer(): + conn, _ = server.accept() + conn.recv(4096) + conn.sendall(response) + conn.close() + + threading.Thread(target=answer, daemon=True).start() + return f"127.0.0.1:{server.getsockname()[1]}" + + yield serve + for server in servers: + server.close() + + @pytest.fixture def black_hole(): sock = socket.socket() diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 2fbbc36e..f43db9f5 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -1,49 +1,15 @@ """Control-plane claims, redirects, volume discovery, errors, and checkpoints.""" -import json import threading import time -from http.server import BaseHTTPRequestHandler, HTTPServer +from http.server import HTTPServer import pytest +from conftest import FakeNode from cocoonsandbox import APIError, Client, Template -class FakeNode(BaseHTTPRequestHandler): - routes = {} - - def do_POST(self): - self._dispatch("POST") - - def do_GET(self): - self._dispatch("GET") - - def do_DELETE(self): - self._dispatch("DELETE") - - def log_message(self, *args): - pass - - def _dispatch(self, method): - length = int(self.headers.get("Content-Length") or 0) - body = json.loads(self.rfile.read(length)) if length else {} - handler = self.routes.get((method, self.path.split("?")[0])) - if handler is None: - self._reply(404, {"error": "no route"}) - return - code, reply = handler(body, self.path) - self._reply(code, reply) - - def _reply(self, code, payload): - raw = json.dumps(payload).encode() - self.send_response(code) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(raw))) - self.end_headers() - self.wfile.write(raw) - - @pytest.fixture def node(): server = HTTPServer(("127.0.0.1", 0), FakeNode) diff --git a/sdk/python/tests/test_frames.py b/sdk/python/tests/test_frames.py index 9a995bc6..a35db0b7 100644 --- a/sdk/python/tests/test_frames.py +++ b/sdk/python/tests/test_frames.py @@ -2,6 +2,7 @@ inside a nested unknown field or trailing bytes after the frame must never yield a silently wrong payload — unexpected shapes take the full parse.""" +import base64 import json import pytest @@ -54,3 +55,11 @@ def test_tag_after_other_keys_takes_the_full_parse(): def test_malformed_frame_rejected(raw): with pytest.raises(json.JSONDecodeError): frames.decode_response(raw) + + +def test_bare_data_frame_fast_path_matches_the_generic_encoding(): + payload = bytes(range(256)) + fast = frames.encode_request("data", data=payload) + generic = frames.encode_request("stdin", data=payload, extra=None).replace(b'"op":"stdin"', b'"op":"data"') + assert fast == generic + assert json.loads(fast)["data"] == base64.b64encode(payload).decode() diff --git a/sdk/python/tests/test_hardening.py b/sdk/python/tests/test_hardening.py index 26f5dcf0..7d280b4b 100644 --- a/sdk/python/tests/test_hardening.py +++ b/sdk/python/tests/test_hardening.py @@ -1,13 +1,17 @@ """Regression tests for the SDK error-surface hardening.""" +import base64 import json import socket import threading +import time import pytest +from conftest import sandbox_at -from cocoonsandbox import APIError, Client, ProtocolError, SilkdError, Watcher -from cocoonsandbox.conn import Conn, dial_agent +from cocoonsandbox import APIError, Client, Lsp, ProtocolError, Pty, SandboxError, Session, SilkdError, Watcher +from cocoonsandbox.conn import Conn, dial_agent, remaining_timeout +from cocoonsandbox.frames import FS_CHUNK def test_dial_agent_rejects_control_chars_in_identity(): @@ -40,74 +44,99 @@ def test_watcher_ends_cleanly_on_garbage_frame(): assert list(watcher) == [] -def test_request_wraps_truncated_body_as_api_error(): - server = socket.create_server(("127.0.0.1", 0)) - port = server.getsockname()[1] - - def serve(): - conn, _ = server.accept() - conn.recv(4096) - conn.sendall(b"HTTP/1.1 200 OK\r\nContent-Length: 100\r\n\r\nshort") - conn.close() - - threading.Thread(target=serve, daemon=True).start() - try: - with pytest.raises(APIError): - Client(f"127.0.0.1:{port}").info() - finally: - server.close() - - -def test_dial_agent_rejects_negative_content_length(): - server = socket.create_server(("127.0.0.1", 0)) - port = server.getsockname()[1] - - def serve(): - conn, _ = server.accept() - conn.recv(4096) - conn.sendall(b"HTTP/1.1 500 nope\r\nContent-Length: -1\r\n\r\nboom") - conn.close() - - threading.Thread(target=serve, daemon=True).start() - try: - with pytest.raises(ProtocolError): - dial_agent(f"127.0.0.1:{port}", "sb_1", "tok", 0.5) - finally: - server.close() - - -def test_error_response_with_truncated_body_wraps_as_api_error(): - server = socket.create_server(("127.0.0.1", 0)) - port = server.getsockname()[1] - - def serve(): - conn, _ = server.accept() - conn.recv(4096) - conn.sendall(b"HTTP/1.1 500 Internal Server Error\r\nContent-Length: 100\r\n\r\nshort") - conn.close() - - threading.Thread(target=serve, daemon=True).start() - try: - with pytest.raises(APIError) as exc_info: - Client(f"127.0.0.1:{port}").info() - finally: - server.close() - assert exc_info.value.status == 500 - - -def test_request_wraps_malformed_json_as_api_error(): - server = socket.create_server(("127.0.0.1", 0)) - port = server.getsockname()[1] - - def serve(): - conn, _ = server.accept() - conn.recv(4096) - conn.sendall(b"HTTP/1.1 200 OK\r\nContent-Length: 3\r\nContent-Type: application/json\r\n\r\nnot") - conn.close() - - threading.Thread(target=serve, daemon=True).start() - try: - with pytest.raises(APIError): - Client(f"127.0.0.1:{port}").info() - finally: - server.close() +@pytest.mark.parametrize( + ("response", "call", "raises", "status"), + [ + (b"HTTP/1.1 200 OK\r\nContent-Length: 100\r\n\r\nshort", "info", APIError, None), + (b"HTTP/1.1 500 Internal Server Error\r\nContent-Length: 100\r\n\r\nshort", "info", APIError, 500), + (b"HTTP/1.1 200 OK\r\nContent-Length: 3\r\nContent-Type: application/json\r\n\r\nnot", "info", APIError, None), + (b"HTTP/1.1 500 nope\r\nContent-Length: -1\r\n\r\nboom", "dial", ProtocolError, None), + ], + ids=["truncated body", "truncated error body", "malformed json", "negative content-length on dial"], +) +def test_raw_replies_surface_as_typed_errors(raw_reply, response, call, raises, status): + addr = raw_reply(response) + with pytest.raises(raises) as exc_info: + if call == "dial": + dial_agent(addr, "sb_1", "tok", 0.5) + else: + Client(addr).info() + if status is not None: + assert exc_info.value.status == status + + +def test_empty_claim_reply_is_an_api_error(spawn_node): + addr = spawn_node({("POST", "/v1/claim"): lambda body, path: (200, {})}) + with pytest.raises(APIError, match="reply without id"): + Client(addr).new("rt:24.04") + + +def test_pty_read_returns_empty_after_the_exit_frame(): + client_sock, guest_sock = socket.socketpair() + guest_sock.sendall(json.dumps({"type": "exit", "code": 3}).encode() + b"\n") + guest_sock.close() + pty = Pty(sandbox_at("127.0.0.1:1"), Conn(client_sock, client_sock.makefile("rb")), 1) + assert pty.read() == b"" + assert pty.exit_code == 3 + assert pty.read() == b"" + + +def test_pty_write_chunks_a_large_buffer(): + client_sock, guest_sock = socket.socketpair() + pty = Pty(sandbox_at("127.0.0.1:1"), Conn(client_sock, client_sock.makefile("rb")), 1) + frames = [] + + def drain(): + reader = guest_sock.makefile("rb") + for _ in range(3): + frames.append(json.loads(reader.readline())) + + reader_thread = threading.Thread(target=drain, daemon=True) + reader_thread.start() + pty.write(b"x" * (3 * FS_CHUNK)) + reader_thread.join(5) + assert [f["op"] for f in frames] == ["stdin"] * 3 + assert all(len(base64.b64decode(f["data"])) == FS_CHUNK for f in frames) + + +def test_session_and_lsp_close_twice(monkeypatch): + sb = sandbox_at("127.0.0.1:1") + calls = [] + + def gone(op, **fields): + calls.append(op) + raise SilkdError("not_found", "no such thing") + + monkeypatch.setattr(sb, "_done_rpc", gone) + Session(sb, "s1").close() + with Lsp(sb, "srv") as lsp: + lsp.stop() + assert calls == ["session_rm", "lsp_stop", "lsp_stop"] + + def broken(op, **fields): + raise SilkdError("internal", "boom") + + monkeypatch.setattr(sb, "_done_rpc", broken) + with pytest.raises(SilkdError): + Session(sb, "s1").close() + + +def test_watcher_error_stays_none_after_close(): + client_sock, guest_sock = socket.socketpair() + guest_sock.sendall(json.dumps({"type": "ready"}).encode() + b"\n") + watcher = Watcher(Conn(client_sock, client_sock.makefile("rb"))) + events = [] + consumer = threading.Thread(target=lambda: events.extend(watcher), daemon=True) + consumer.start() + time.sleep(0.1) + watcher.close() + consumer.join(5) + assert not consumer.is_alive() + assert events == [] and watcher.error is None + + +def test_sandbox_timeout_is_both_hierarchies(): + with pytest.raises(SandboxError): + remaining_timeout(1.0, time.monotonic() - 1, "probe") + with pytest.raises(TimeoutError): + remaining_timeout(1.0, time.monotonic() - 1, "probe") diff --git a/sdk/python/tests/test_keepalive.py b/sdk/python/tests/test_keepalive.py index 8710ae4f..8e57fd4c 100644 --- a/sdk/python/tests/test_keepalive.py +++ b/sdk/python/tests/test_keepalive.py @@ -5,7 +5,9 @@ import json import socket import threading +import time +import pytest from conftest import accept_upgrade, sandbox_at, wait_until from cocoonsandbox.frames import KEEP_ALIVE_PROTO @@ -72,6 +74,26 @@ def test_peer_hang_up_is_noticed_before_reuse(): assert agent.upgrades == 2 +def test_run_timeout_bounds_the_proto_probe(): + server = socket.create_server(("127.0.0.1", 0)) + + def serve(): + conn, _ = server.accept() + accept_upgrade(conn) + time.sleep(3) + conn.close() + + threading.Thread(target=serve, daemon=True).start() + sb = sandbox_at(f"127.0.0.1:{server.getsockname()[1]}") + started = time.monotonic() + try: + with pytest.raises(TimeoutError): + sb.run(["true"], timeout=0.5) + finally: + server.close() + assert time.monotonic() - started < 2, "the unanswered info probe outlived the run timeout" + + def test_close_drains_the_parked_connection(monkeypatch): agent = FakeAgent() sb = sandbox_at(agent.addr) diff --git a/sdk/python/tests/test_proxy.py b/sdk/python/tests/test_proxy.py index 629a8b82..06f7f25b 100644 --- a/sdk/python/tests/test_proxy.py +++ b/sdk/python/tests/test_proxy.py @@ -5,7 +5,7 @@ import socket import time -from cocoonsandbox import Client, Sandbox +from conftest import sandbox_at class FakePortConn: @@ -19,8 +19,8 @@ def send(self, data: bytes) -> None: self._inbox.append(b"echo:" + data) def recv(self) -> bytes: - deadline = time.time() + 5 - while not self._inbox and not self._closed and time.time() < deadline: + deadline = time.monotonic() + 5 + while not self._inbox and not self._closed and time.monotonic() < deadline: time.sleep(0.01) return self._inbox.pop(0) if self._inbox else b"" @@ -54,7 +54,7 @@ def close(self) -> None: def test_proxy_port_accepts_and_pipes(monkeypatch): - sb = Sandbox(client=Client("127.0.0.1:1"), id="sb_1", token="tok", owner="127.0.0.1:1") + sb = sandbox_at("127.0.0.1:1") monkeypatch.setattr(sb, "dial_port", lambda port: FakePortConn()) listener = sb.proxy_port("127.0.0.1:0", 8080) @@ -62,8 +62,8 @@ def test_proxy_port_accepts_and_pipes(monkeypatch): c = socket.create_connection(listener.getsockname(), timeout=5) c.sendall(b"hello") c.settimeout(5) - got, deadline = b"", time.time() + 5 - while b"echo:hello" not in got and time.time() < deadline: + got, deadline = b"", time.monotonic() + 5 + while b"echo:hello" not in got and time.monotonic() < deadline: chunk = c.recv(256) if not chunk: break @@ -75,7 +75,7 @@ def test_proxy_port_accepts_and_pipes(monkeypatch): def test_proxy_port_ends_the_local_connection_when_the_guest_port_is_closed(monkeypatch): - sb = Sandbox(client=Client("127.0.0.1:1"), id="sb_1", token="tok", owner="127.0.0.1:1") + sb = sandbox_at("127.0.0.1:1") monkeypatch.setattr(sb, "dial_port", lambda port: ClosedPortConn()) listener = sb.proxy_port("127.0.0.1:0", 8080) diff --git a/silkd/src/exec.rs b/silkd/src/exec.rs index 58da4c1f..77207ca5 100644 --- a/silkd/src/exec.rs +++ b/silkd/src/exec.rs @@ -10,7 +10,7 @@ use tokio::sync::mpsc; use tokio::time::timeout; use crate::proc::{Chunk, Proc, Table, synth_pid, write_chunk}; -use crate::proto::{ErrorKind, ExecReq, Request, Response}; +use crate::proto::{self, ErrorKind, ExecReq, Request, Response}; use crate::sysutil; /// Post-exit drain window; it bounds a daemonizing grandchild, and a stalled client loses the undrained tail. @@ -46,19 +46,19 @@ where if let Some(ref user) = req.user && let Err(e) = sysutil::apply_user(&mut cmd, user) { - return crate::proto::error_frame(out, ErrorKind::BadRequest, e).await; + return proto::error_frame(out, ErrorKind::BadRequest, e).await; } let mut child = match cmd.spawn() { Ok(c) => c, Err(e) => { - return crate::proto::error_frame(out, spawn_kind(&e), format!("spawn: {e}")).await; + return proto::error_frame(out, spawn_kind(&e), format!("spawn: {e}")).await; } }; let pid = child.id().unwrap_or_else(synth_pid); let proc = table.register(pid, req.argv, req.detach, now_secs); - if let Err(e) = crate::proto::write_frame(out, &Response::Started { pid }).await { + if let Err(e) = proto::write_frame(out, &Response::Started { pid }).await { // the client never learned this pid, so nothing else will ever reap the child. table.remove_if(pid, &proc); let _ = child.start_kill(); @@ -71,9 +71,7 @@ where // a backpressured mpsc paces the child to the foreground client; attachers ride the lossy broadcast. let (fg_tx, fg_rx) = mpsc::channel::(FG_CAP); - let pump_fg = (!req.detach).then(|| fg_tx.clone()); - let sup_fg = (!req.detach).then(|| fg_tx.clone()); - drop(fg_tx); // only the pump/supervise clones keep fg_rx open + let (pump_fg, sup_fg) = (!req.detach).then(|| (fg_tx.clone(), fg_tx)).unzip(); let pump = tokio::spawn(pump_out(Arc::clone(&proc), stdout, stderr, pump_fg)); let pump_abort = pump.abort_handle(); @@ -127,7 +125,7 @@ where } let code = supervise.await.unwrap_or(-1); table.remove_if(pid, &proc); - crate::proto::write_frame(out, &Response::Exit { code }).await + proto::write_frame(out, &Response::Exit { code }).await } /// A missing or unrunnable argv or cwd is the caller's mistake; anything else is the guest's. @@ -180,7 +178,7 @@ where R: AsyncRead + Unpin, { let make: fn(Vec) -> Chunk = if stderr { Chunk::Stderr } else { Chunk::Stdout }; - let mut buf = [0u8; crate::proto::READ_CHUNK]; + let mut buf = [0u8; proto::READ_CHUNK]; loop { let n = match r.read(&mut buf).await { Ok(0) | Err(_) => break, diff --git a/silkd/src/find.rs b/silkd/src/find.rs index e522c7e6..c0c56537 100644 --- a/silkd/src/find.rs +++ b/silkd/src/find.rs @@ -11,7 +11,7 @@ use tokio::sync::{Semaphore, SemaphorePermit, mpsc}; use crate::proto::{self, ErrorKind, Response, err_frame}; -/// Size above which find skips a file unread. +/// Size above which find skips a file unread and replace reports zero replacements. pub const FIND_MAX_FILE: u64 = 8 * 1024 * 1024; /// Match frames in flight between the walking thread and the writer. @@ -223,7 +223,7 @@ where // one blocking-pool dispatch for the whole tree, not three per file. let (tx, mut rx) = mpsc::channel::(MATCH_QUEUE); let budget = Arc::new(MatchBudget::new(budget_bytes, Handle::current())); - let walker_budget = budget.clone(); + let walker_budget = Arc::clone(&budget); let walk = tokio::task::spawn_blocking(move || { Walk { re: &re, @@ -237,13 +237,18 @@ where let mut buf = Vec::new(); let mut failed = None; let mut gone = false; + let mut pipelined = false; loop { tokio::select! { biased; - // the client sends nothing during a find, so any readable state ends the walk. - _ = reader.fill_buf() => { - gone = true; - break; + // a hung-up client ends the walk; a request pipelined behind the find waits for done + r = reader.fill_buf(), if !pipelined => { + if matches!(r, Ok(buf) if !buf.is_empty()) { + pipelined = true; + } else { + gone = true; + break; + } } n = rx.recv_many(&mut batch, MATCH_QUEUE) => { if n == 0 { diff --git a/silkd/src/fs.rs b/silkd/src/fs.rs index 31b090fe..90455ea0 100644 --- a/silkd/src/fs.rs +++ b/silkd/src/fs.rs @@ -50,8 +50,8 @@ where /// Streams the regular file at `path` back as `data` frames, then `done`. pub async fn read(w: &mut W, path: String) -> io::Result<()> { - // O_NONBLOCK keeps a writerless FIFO from parking the open; the kind check runs on the opened fd, so a swap - // between check and open cannot turn the read into a device stream + // O_NONBLOCK keeps a writerless FIFO from parking the open + // the kind check runs on the opened fd, so a path swap cannot turn the read into a device stream let opened = tokio::task::spawn_blocking(move || { let file = std::fs::OpenOptions::new() .read(true) diff --git a/silkd/src/lsp.rs b/silkd/src/lsp.rs index 5d5b43ae..0916374d 100644 --- a/silkd/src/lsp.rs +++ b/silkd/src/lsp.rs @@ -128,7 +128,7 @@ impl Broker { let res = pump_stdout(stdout, w).await; // aborting mid-write is safe: the child is killed next. feed.abort(); - let _ = self.reap(server_id).await; + self.reap(server_id).await; res } diff --git a/silkd/src/net.rs b/silkd/src/net.rs index 50e13ff3..b68b4e76 100644 --- a/silkd/src/net.rs +++ b/silkd/src/net.rs @@ -24,7 +24,7 @@ pub fn has_egress() -> bool { 1 => return true, _ => {} } - // Probed once: the NIC set is fixed for the guest's life. + // probed once: the NIC set is fixed for the guest's life static DEVICE_BACKED: LazyLock = LazyLock::new(|| { let Ok(entries) = std::fs::read_dir("/sys/class/net") else { return true; diff --git a/silkd/src/proc.rs b/silkd/src/proc.rs index bbdef43c..0d3fa2b8 100644 --- a/silkd/src/proc.rs +++ b/silkd/src/proc.rs @@ -26,16 +26,6 @@ pub enum Chunk { Exit(i32), } -impl Chunk { - pub fn into_response(self) -> Response { - match self { - Chunk::Stdout(data) => Response::Stdout { data }, - Chunk::Stderr(data) => Response::Stderr { data }, - Chunk::Exit(code) => Response::Exit { code }, - } - } -} - /// Registry of running and recently-exited processes. #[derive(Clone, Default)] pub struct Table { @@ -154,7 +144,7 @@ impl Proc { /// Resizes the pty's window; errors if this proc is a plain exec. pub fn resize(&self, cols: u16, rows: u16) -> std::io::Result<()> { match &*sysutil::lock(&self.pty_master) { - Some(fd) => crate::sysutil::set_winsize(fd.as_raw_fd(), cols, rows), + Some(fd) => sysutil::set_winsize(fd.as_raw_fd(), cols, rows), None => Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, "not a pty", @@ -172,14 +162,14 @@ impl Proc { /// Snapshot of retained output for `logs`. pub fn replay(&self) -> Vec { - sysutil::lock(&self.ring).drain_view() + sysutil::lock(&self.ring).snapshot() } /// Snapshots retained output and subscribes to live output under one lock. pub fn attach_stream(&self) -> (Vec, broadcast::Receiver) { let mut ring = sysutil::lock(&self.ring); let tx = self.tx.get_or_init(|| broadcast::channel(OUTPUT_FANOUT).0); - (ring.drain_view(), tx.subscribe()) + (ring.snapshot(), tx.subscribe()) } fn attached(&self) -> Option<&broadcast::Sender> { @@ -227,7 +217,7 @@ impl Ring { } /// Retained output, with adjacent same-stream segments coalesced into one chunk. - fn drain_view(&mut self) -> Vec { + fn snapshot(&mut self) -> Vec { let bytes = self.buf.make_contiguous(); let mut out: Vec = Vec::new(); let mut off = 0; @@ -263,7 +253,7 @@ pub async fn write_chunk( match chunk { Chunk::Stdout(data) => proto::write_chunk_frame(w, buf, "stdout", &data).await, Chunk::Stderr(data) => proto::write_chunk_frame(w, buf, "stderr", &data).await, - other => proto::write_frame(w, &other.into_response()).await, + Chunk::Exit(code) => proto::write_frame(w, &Response::Exit { code }).await, } } diff --git a/silkd/src/proto.rs b/silkd/src/proto.rs index 86c0ba64..5e613f74 100644 --- a/silkd/src/proto.rs +++ b/silkd/src/proto.rs @@ -423,7 +423,12 @@ pub async fn write_frames( ) -> io::Result<()> { buf.clear(); for frame in frames { - render_frame(buf, frame)?; + if let Err(e) = render_frame(buf, frame) { + // the frames rendered before the oversized one still reach the client + w.write_all(buf).await?; + w.flush().await?; + return Err(e); + } if buf.len() > MAX_FRAME { w.write_all(buf).await?; buf.clear(); @@ -581,7 +586,10 @@ fn cap_check(len: usize) -> io::Result<()> { fn render_frame(buf: &mut Vec, resp: &Response) -> io::Result<()> { let start = buf.len(); serde_json::to_writer(&mut *buf, resp).map_err(io::Error::other)?; - cap_check(buf.len() - start)?; + if let Err(e) = cap_check(buf.len() - start) { + buf.truncate(start); + return Err(e); + } buf.push(b'\n'); Ok(()) } @@ -712,6 +720,35 @@ mod tests { println!("{CHUNKS} x {BULK_CHUNK} B chunks: serde {serde_dt:?}, hand-rendered {hand_dt:?}"); } + #[tokio::test] + async fn write_frames_keeps_the_frames_before_an_oversized_one() { + let mut sink = Vec::new(); + let mut buf = Vec::new(); + let frames = [ + Response::Match { + file: "a.txt".into(), + line: 1, + content: "short".into(), + }, + Response::Match { + file: "b.txt".into(), + line: 1, + content: "\u{1}".repeat(MAX_FRAME / 4), + }, + ]; + let err = write_frames(&mut sink, &mut buf, &frames) + .await + .expect_err("oversized frame must fail"); + assert_eq!(err.kind(), io::ErrorKind::InvalidData); + let sent = String::from_utf8(sink).expect("utf8"); + assert_eq!( + sent.lines().count(), + 1, + "exactly the short frame reached the sink" + ); + assert!(sent.contains("a.txt"), "{sent}"); + } + #[tokio::test] async fn frame_reader_caps_oversized_lines() { let big = vec![b'a'; MAX_FRAME + 2]; @@ -797,7 +834,11 @@ mod tests { if stem.starts_with("req_") { serde_json::from_slice::(&raw).unwrap_or_else(|e| panic!("{name}: {e}")); } else if stem.starts_with("resp_") { - serde_json::from_slice::(&raw).unwrap_or_else(|e| panic!("{name}: {e}")); + let resp: Response = + serde_json::from_slice(&raw).unwrap_or_else(|e| panic!("{name}: {e}")); + let back = serde_json::to_value(&resp).expect("encode response"); + let want: serde_json::Value = serde_json::from_slice(&raw).expect("fixture json"); + assert_eq!(back, want, "{name}: round-trip drift"); } else { continue; } diff --git a/silkd/src/pty.rs b/silkd/src/pty.rs index 3ce76c5c..266b1cb6 100644 --- a/silkd/src/pty.rs +++ b/silkd/src/pty.rs @@ -12,7 +12,7 @@ use tokio::sync::{mpsc, oneshot}; use tokio::time::timeout; use crate::proc::{Chunk, Proc, Table, synth_pid}; -use crate::proto::{ErrorKind, PtyReq, READ_CHUNK, Request, Response}; +use crate::proto::{self, ErrorKind, PtyReq, READ_CHUNK, Request, Response}; use crate::sysutil; /// Bounds the post-exit drain of the master's buffered tail so a stuck fd cannot wedge teardown. @@ -30,11 +30,11 @@ pub async fn open( ) -> std::io::Result<()> { let (master, slave) = match sysutil::openpty(req.cols, req.rows) { Ok(pair) => pair, - Err(e) => return crate::proto::err_frame(out, &e, "openpty").await, + Err(e) => return proto::err_frame(out, &e, "openpty").await, }; let resize_fd = match master.try_clone() { Ok(fd) => fd, - Err(e) => return crate::proto::err_frame(out, &e, "dup pty master").await, + Err(e) => return proto::err_frame(out, &e, "dup pty master").await, }; let shell = std::env::var("SHELL").unwrap_or_else(|_| "/bin/bash".to_string()); @@ -49,7 +49,7 @@ pub async fn open( if let Some(user) = &req.user && let Err(e) = sysutil::apply_user(&mut cmd, user) { - return crate::proto::error_frame(out, ErrorKind::BadRequest, e).await; + return proto::error_frame(out, ErrorKind::BadRequest, e).await; } match (slave.try_clone(), slave.try_clone()) { (Ok(in_fd), Ok(out_fd)) => { @@ -58,7 +58,7 @@ pub async fn open( .stderr(Stdio::from(slave)); } (Err(e), _) | (_, Err(e)) => { - return crate::proto::err_frame(out, &e, "dup pty slave").await; + return proto::err_frame(out, &e, "dup pty slave").await; } } sysutil::adopt_controlling_tty(&mut cmd); @@ -66,7 +66,7 @@ pub async fn open( let mut child = match cmd.spawn() { Ok(c) => c, - Err(e) => return crate::proto::err_frame(out, &e, "spawn shell").await, + Err(e) => return proto::err_frame(out, &e, "spawn shell").await, }; // drop cmd so its slave dups close, else the master never sees the shell's exit and the pump hangs. drop(cmd); @@ -80,23 +80,20 @@ pub async fn open( Err(e) => { let _ = child.start_kill(); table.remove_if(pid, &proc); - return crate::proto::err_frame(out, &e, "watch pty master").await; + return proto::err_frame(out, &e, "watch pty master").await; } }; - if crate::proto::write_frame(out, &Response::Started { pid }) - .await - .is_err() - { + if let Err(e) = proto::write_frame(out, &Response::Started { pid }).await { let _ = child.start_kill(); finish(&proc, -1); table.remove_if(pid, &proc); - return Ok(()); + return Err(e); } let code = pump(&master, &proc, client, out, &mut child).await; finish(&proc, code); table.remove_if(pid, &proc); - crate::proto::write_frame(out, &Response::Exit { code }).await + proto::write_frame(out, &Response::Exit { code }).await } /// Applies a resize to a live pty by pid. @@ -111,8 +108,8 @@ pub async fn resize( return Ok(()); }; match proc.resize(cols, rows) { - Ok(()) => crate::proto::write_frame(w, &Response::Done).await, - Err(e) => crate::proto::error_frame(w, ErrorKind::BadRequest, e.to_string()).await, + Ok(()) => proto::write_frame(w, &Response::Done).await, + Err(e) => proto::error_frame(w, ErrorKind::BadRequest, e.to_string()).await, } } @@ -154,7 +151,7 @@ async fn pump( Ok(Ok(0)) | Ok(Err(_)) => eof = true, Ok(Ok(n)) => { proc.emit_bytes(false, &buf[..n]); - if crate::proto::write_chunk_frame(out, &mut frame, "stdout", &buf[..n]).await.is_err() { + if proto::write_chunk_frame(out, &mut frame, "stdout", &buf[..n]).await.is_err() { let _ = child.start_kill(); return sysutil::wait_code(child).await; } @@ -205,7 +202,7 @@ async fn drain( Ok(Ok(0)) | Ok(Err(_)) => return, Ok(Ok(n)) => { proc.emit_bytes(false, &buf[..n]); - if crate::proto::write_chunk_frame(out, frame, "stdout", &buf[..n]) + if proto::write_chunk_frame(out, frame, "stdout", &buf[..n]) .await .is_err() { diff --git a/silkd/src/watch.rs b/silkd/src/watch.rs index 145891ef..3652c809 100644 --- a/silkd/src/watch.rs +++ b/silkd/src/watch.rs @@ -59,7 +59,7 @@ where return Ok(()); } }, - // the client sends nothing during a watch, so any readable state ends it. + // a watch is connection-bound: a hang-up or a request pipelined behind it ends it _ = reader.fill_buf() => return Ok(()), } } diff --git a/silkd/tests/common/mod.rs b/silkd/tests/common/mod.rs index e9be1e9c..40cb6334 100644 --- a/silkd/tests/common/mod.rs +++ b/silkd/tests/common/mod.rs @@ -29,17 +29,18 @@ pub fn connect(state: &Arc) -> (FrameWriter, FrameLines, JoinHandle Value { let line = timeout(DEADLINE, lines.next_line()) .await .expect("deadline") - .unwrap() + .expect("read line") .expect("stream closed"); - serde_json::from_str(&line).unwrap() + serde_json::from_str(&line).unwrap_or_else(|e| panic!("bad frame {line:?}: {e}")) } pub async fn frames_until(lines: &mut FrameLines, pred: impl Fn(&Value) -> bool) -> Vec { @@ -58,7 +59,7 @@ pub async fn roundtrip(request_line: &str) -> Vec { request_on(&Arc::new(State::new()), &[request_line.to_string()]).await } -pub async fn request_on(state: &Arc, lines: &[String]) -> Vec { +async fn request_on(state: &Arc, lines: &[String]) -> Vec { let (mut cw, mut out, handle) = connect(state); for line in lines { cw.write_all(line.as_bytes()).await.unwrap(); diff --git a/silkd/tests/find_e2e.rs b/silkd/tests/find_e2e.rs index dcee9292..fc497b10 100644 --- a/silkd/tests/find_e2e.rs +++ b/silkd/tests/find_e2e.rs @@ -21,6 +21,27 @@ async fn find_frames(req: Value) -> Vec { frames } +#[tokio::test] +async fn find_serves_a_request_pipelined_behind_it_after_done() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("a.txt"), "needle\n") + .await + .unwrap(); + let (mut cw, mut out, handle) = connect(&Arc::new(State::new())); + send( + &mut cw, + json!({"op":"fs_find","path":dir.path().to_str().unwrap(),"pattern":"needle"}), + ) + .await; + send(&mut cw, json!({"op":"info"})).await; + let frames = frames_until(&mut out, |f| matches!(type_of(f), "done" | "error")).await; + assert_eq!(type_of(frames.last().unwrap()), "done", "{frames:?}"); + assert!(frames.iter().any(|f| type_of(f) == "match"), "{frames:?}"); + assert_eq!(type_of(&common::next_frame(&mut out).await), "info"); + cw.shutdown().await.unwrap(); + handle.await.unwrap().unwrap(); +} + #[tokio::test] async fn find_streams_line_matches() { let dir = tempfile::tempdir().unwrap(); diff --git a/silkd/tests/git_e2e.rs b/silkd/tests/git_e2e.rs index 4851573f..9c82c5e5 100644 --- a/silkd/tests/git_e2e.rs +++ b/silkd/tests/git_e2e.rs @@ -8,7 +8,7 @@ use serde_json::json; use common::{exchange, type_of}; fn git(dir: &std::path::Path, args: &[&str]) { - let ok = std::process::Command::new("git") + let out = std::process::Command::new("git") .arg("-C") .arg(dir) .args(args) @@ -17,10 +17,12 @@ fn git(dir: &std::path::Path, args: &[&str]) { .env("GIT_COMMITTER_NAME", "t") .env("GIT_COMMITTER_EMAIL", "t@e") .output() - .unwrap() - .status - .success(); - assert!(ok, "git {args:?} failed"); + .expect("run git"); + assert!( + out.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); } fn init_repo() -> tempfile::TempDir { @@ -104,6 +106,7 @@ async fn commit_failure_surfaces_git_error() { .to_string()]) .await; assert_eq!(type_of(&frames[0]), "error"); + assert_eq!(frames[0]["kind"], "internal", "{frames:?}"); } #[tokio::test] diff --git a/silkd/tests/pty_e2e.rs b/silkd/tests/pty_e2e.rs index cfa71750..bc984146 100644 --- a/silkd/tests/pty_e2e.rs +++ b/silkd/tests/pty_e2e.rs @@ -3,19 +3,59 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] mod common; +use std::collections::HashMap; +use std::pin::Pin; use std::sync::Arc; +use std::task::{Context, Poll}; use std::time::Duration; use serde_json::{Value, json}; +use silkd::proc::Table; +use silkd::proto::PtyReq; +use silkd::pty; use silkd::server::State; -use tokio::io::AsyncWriteExt; +use tokio::io::{AsyncWrite, AsyncWriteExt}; +use tokio::sync::mpsc; use common::{FrameLines, b64, connect, decode, exchange, frames_until, one, send, type_of}; +struct FailingSink(Vec); + +impl AsyncWrite for FailingSink { + fn poll_write( + mut self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + self.0.extend_from_slice(buf); + Poll::Ready(Err(std::io::Error::other("client gone"))) + } + + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } +} + async fn read_until(lines: &mut FrameLines, pred: impl Fn(&Value) -> bool) -> Value { frames_until(lines, pred).await.pop().unwrap() } +async fn wait_gone(pid: u32) { + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + // SAFETY: kill(2) with signal 0 only probes whether the pid exists; it takes no pointers. + while unsafe { libc::kill(pid as libc::pid_t, 0) } == 0 { + assert!( + tokio::time::Instant::now() < deadline, + "shell {pid} still alive 5 s after the started write failed" + ); + tokio::time::sleep(Duration::from_millis(20)).await; + } +} + #[tokio::test] async fn pty_runs_a_shell_and_echoes() { let state = Arc::new(State::new()); @@ -38,7 +78,7 @@ async fn pty_runs_a_shell_and_echoes() { send(&mut cw, json!({"op":"stdin","data":b64(b"exit\n")})).await; let exit = read_until(&mut lines, |v| v["type"] == "exit").await; - assert!(exit["code"].is_number()); + assert_eq!(exit["code"], 0, "{exit:?}"); cw.shutdown().await.unwrap(); handle.await.unwrap().unwrap(); @@ -106,3 +146,31 @@ async fn pty_disconnect_tears_down_without_spin() { "pty still in table after disconnect: {ps:?}" ); } + +#[tokio::test] +async fn pty_started_write_failure_kills_the_shell_and_unregisters_it() { + let table = Table::new(); + let (_client, client_rx) = mpsc::channel(1); + let req = PtyReq { + cols: 80, + rows: 24, + cwd: None, + env: HashMap::new(), + user: None, + }; + let mut sink = FailingSink(Vec::new()); + + let err = pty::open(&table, 0, req, client_rx, &mut sink) + .await + .expect_err("open must propagate the started write failure"); + assert_eq!(err.to_string(), "client gone"); + + let started: Value = serde_json::from_slice(&sink.0).expect("started frame reached the writer"); + assert_eq!(started["type"], "started"); + let pid = started["pid"].as_u64().unwrap() as u32; + assert!( + table.list().is_empty(), + "pty still registered after the write failure" + ); + wait_gone(pid).await; +} diff --git a/silkd/tests/watch_e2e.rs b/silkd/tests/watch_e2e.rs index a7ddf41c..59379572 100644 --- a/silkd/tests/watch_e2e.rs +++ b/silkd/tests/watch_e2e.rs @@ -8,49 +8,31 @@ use std::time::Duration; use serde_json::json; use silkd::server::State; -use tokio::io::AsyncWriteExt; + +use common::{connect, exchange, next_frame, send, type_of}; #[tokio::test] async fn watch_streams_events_until_disconnect() { let dir = tempfile::tempdir().unwrap(); let state = Arc::new(State::new()); - let (mut cw, mut lines, handle) = common::connect(&state); - let req = json!({ - "op": "fs_watch", - "path": dir.path().to_str().unwrap(), - "recursive": true - }) - .to_string(); - cw.write_all(req.as_bytes()).await.unwrap(); - cw.write_all(b"\n").await.unwrap(); - - let ready = tokio::time::timeout(Duration::from_secs(5), lines.next_line()) - .await - .expect("no ready within 5s") - .unwrap() - .expect("stream closed before ready"); - let frame: serde_json::Value = serde_json::from_str(&ready).unwrap(); - assert_eq!(frame["type"], "ready", "got {frame:?}"); + let (mut cw, mut lines, handle) = connect(&state); + send( + &mut cw, + json!({"op": "fs_watch", "path": dir.path().to_str().unwrap(), "recursive": true}), + ) + .await; + assert_eq!(type_of(&next_frame(&mut lines).await), "ready"); tokio::fs::write(dir.path().join("new.txt"), b"hi") .await .unwrap(); - tokio::time::timeout(Duration::from_secs(5), async { - loop { - let line = lines - .next_line() - .await - .unwrap() - .expect("stream closed before an event"); - let frame: serde_json::Value = serde_json::from_str(&line).unwrap(); - assert_eq!(frame["type"], "event", "got {frame:?}"); - if frame["path"].as_str().unwrap().ends_with("new.txt") { - return; - } + loop { + let frame = next_frame(&mut lines).await; + assert_eq!(type_of(&frame), "event", "got {frame:?}"); + if frame["path"].as_str().unwrap().ends_with("new.txt") { + break; } - }) - .await - .expect("no new.txt event within 5s"); + } drop(lines); drop(cw); @@ -63,11 +45,9 @@ async fn watch_streams_events_until_disconnect() { #[tokio::test] async fn watch_missing_path_is_error() { - let frames = common::exchange(&[json!({ - "op": "fs_watch", - "path": "/no/such/dir/silkd-watch" - }) - .to_string()]) - .await; - assert_eq!(common::type_of(&frames[0]), "error"); + let frames = + exchange(&[json!({"op": "fs_watch", "path": "/no/such/dir/silkd-watch"}).to_string()]) + .await; + assert_eq!(type_of(&frames[0]), "error"); + assert_eq!(frames[0]["kind"], "not_found", "{frames:?}"); }