Skip to content

review: stop during a capture, reader leak, tenant cap, egress audit, silkd hang-ups, tests that fail - #444

Merged
CMGS merged 19 commits into
mainfrom
review/post-441
Nov 4, 2026
Merged

CMGS merged 19 commits into
mainfrom
review/post-441

Conversation

@CMGS

@CMGS CMGS commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Whole-repo audit after #441. This round read what earlier rounds had not: every test file for whether it can fail, all eight cross-file flows again, every doc against the code, and the shell scripts, Dockerfiles, CI, Makefile and packaging.

Bugs

Each fix is its own commit with a test that fails on the old code, unless noted.

  • A stop during a hibernate stranded or wiped the guest. sandboxd exited about 5 s after SIGTERM without waiting for cocoon. cocoon then died on its next stderr write, after the snapshot persisted and before the VMM stopped: the guest stayed Paused, its record read running, and every exec got 502 until vm start. With stderr kept alive, the next start's reconcile ran while the capture was still going and later cold-booted the guest, losing its memory. sandboxd now starts no cocoon command once stopping and waits for the running ones; the unit gets TimeoutStopSec=27min (15 minute budget plus the 11 minute grace). Hardware below.
  • A reader connection outlived its client. A client that stopped reading mid-reply and then closed left the router blocked writing to the reader. The reader VM, the goroutine and shutdown's relay wait were held until restart.
  • A tenant's max_claims could be skipped. review: pty stdin wedge, unfence channel, mesh bind, cuts and docs #440 removed the per-name eviction check, so a tenant write anywhere in the cell during a cold resolve left the tenant uncached, and Peek then answered Unknown. Restored with its test.
  • Egress audit and usage. The internal-address guard's refusal was audited as allow and skipped the refusal limiter. The forward path wrote a 0/0 egress_bytes event for every request whose dial failed, at request rate.
  • A replica round held the claim's transition across an S3 delete, so every exec and dial to a replicated claim waited on it once per interval. The previous replica is now deleted after the unlock. No test: the stall needs lock timing a unit test cannot pin.
  • A heal split its 30 minutes evenly across owners, so with eight owners each attempt got 225 s, even when the earlier owners refused at once. Each attempt now gets what is left over the owners still to try.
  • A healed checkpoint's files were never synced before its meta was committed. After a power loss, the durable meta pointed at zeroed data, and that local copy shadowed the good owner. No test: fsync is not observable in a unit test.
  • A cold boot read the claim's env without the manager lock, racing an env PATCH. No test: the race needs a concurrent PATCH at one point in the restart.
  • silkd: a request that failed to parse after a streaming RPC got no answer. The feeder dropped it, so a client on a reused connection waited forever. A fresh connection already answered bad_request.
  • silkd: attach never noticed a hang-up. Attaching to a quiet detached process and dropping the connection held the task, its vsock fd, its buffer and a broadcast receiver until the process wrote or exited.
  • Python SDK: run fed all of stdin after the command exited. A large stdin outlived the timeout, and a guest that stopped reading hung the call. It now aborts the connection once the exit frame arrives, as the Go SDK does.

Tests that could not fail

Fourteen tests would pass with the production check they cover removed, by reading: the egress wake guards, the NIC lock error, the untar path check, symlink skipping, the plain-publish digest removal, the device probe bound, the frame cap, the zero-ttl renew default, the single teardown, the kept archive intent, the warm claim counter, the closeBoxes wait, sparse holes on Linux, and a SOCKS close that a read deadline satisfied. Ten of these were checked by removing the check: the tightened test now fails (NIC lock, untar path check, symlink skip, digest removal, both egress wake guards, IPv4 preference, probe bound, frame cap, zero-ttl default). The rest were tightened by reading. Separately, the cell-isolation check now runs after the cell has settled, and a fake detach error real cocoon never returns is gone.

Build, CI and docs

  • make lint and make test run cargo with --locked, like CI. make extract no longer deletes dist/sandboxd.
  • credentials-e2e.sh: grep -q under pipefail could let a leaked secret pass the check.
  • python.yml gets a read-only token like the other workflows.
  • The published live-volumes page dropped a Go composite literal into Liquid; the example also checked a field that is never set on success.
  • Docs now list failed sandboxes in three 409 lists, say that the template digest guard holds per node, that the SOCKS door is settled at claim time, when a replicated claim has no row after a wake, and that a publish holds up to 192 MiB with concurrency capped by the layer above.

Kept, with reasons

  • Actions stay pinned by tag, the org convention. The guest's python-lsp-server stays unpinned like apt.
  • S3 publish memory has no node cap; the layer above limits concurrent checkpoints.
  • S3 delete still re-stamps manifests, the documented contract with offline reclaim.
  • The port relay's 32 KiB copies are a perf hypothesis until measured.
  • Release and image jobs triggered from a non-main ref, and e2e cleanups removing every sbx-* VM, need a maintainer acting outside the contract.

Hardware

Bare metal, cocoon v0.7.6, a 4 GiB guest with 3.5 GiB dirtied in /dev/shm, sandboxd stopped during vm hibernate and restarted:

sandboxd Stop 1 s into the capture Stop at 15 s, capture held at 1 MB/s
main VMM Paused, exec 502, vm start needed same
main, cocoon stderr to a file guest cold-booted, memory lost same
this branch stop waits for the capture, claim adopted as hibernated, exec wakes it with memory intact stop waited 27 s for the capture, same result

No orphan snapshot was left in any run on this branch.

Under a real systemd unit, with the capture held and systemctl stop at 15 s:

TimeoutStopSec Stop returned Unit result cocoon After restart
27min after 26 s, when the capture ended success exited 0 hibernated, wakes with its marker
10s after 10 s timeout, SIGKILL killed by SIGPIPE VMM Paused, exec 502 until vm start

Everything below is main against this branch on the same hosts. Two nodes, cocoon v0.7.6.

Check main this branch
Request that fails to parse after an exec, in a real guest no frame in 8 s bad_request at once
silkd fds after dropping 10 attaches (baseline 19) still 29 after 20 s 19 within 1 s
Audit of 3 internal-address requests under a * rule 3 allow 3 deny
egress_bytes events for those requests, a failed lookup and one real request 4 1, the real request
Reader VM after its client leaves mid-reply still up after 80 s released within 40 s
Daemon stop after that still running after 13 s, needed SIGKILL 0.03 s
Files fsynced by a heal before its meta commits meta only every export file and the meta
New exec during replica rounds, with S3 delete and list slowed by 2 s max 6.01 s, one spike per delete max 0.06 s and 0.09 s

On this branch: all five egress scripts pass, on both lanes. Peer heal passes, including an owner that answers the probe and then refuses the transfer. The S3 contract and archive-to-S3 pass against a real endpoint. A lost node's claim is adopted by a peer and wakes from its latest replica with memory and processes intact.

Not exercised on hardware: the heal budget difference itself, which needs a transfer longer than the old 15 minute slice (unit test only), and an actual host loss (the node loss was a clean stop with its VM left running).

Gates

  • make go-lint: 10 0 issues. lines (5 modules × linux and darwin).
  • make go-test: 27 packages ok.
  • asl on all five modules, linux and darwin: no findings. The four advisory forwarder findings are kept in the ledger.
  • Rust in a rust:1.99.1 arm64 container and on macOS: fmt and clippy clean. silkd 162 tests passed on Linux and 161 on macOS, 3 ignored. boot/init 17 passed on Linux and 15 on macOS.
  • Python 3.12 in a container: ruff clean; sdk/python 278 passed; mypy no issues; openai 10 passed; langchain 15 passed. Python 3.9: 278 passed.
  • shellcheck on every tracked script: clean.

Size

Production +144 −58 (net +86); tests +318 −63; docs, CI and scripts +39 −28. Comment lines in Go and Rust: +2 −2, so no file gains a comment. The production growth is the eleven fixes above: the engine stop +28 (a mutex, a flag, a WaitGroup and the var block that now holds the stop sentinel), the heal fsync +18, silkd +14, the restored tenant check +10, egress +8, and under +3 for each of the rest.

CMGS added 19 commits November 4, 2026 18:42
Fourteen tests passed with the production check they cover removed: the
egress wake guards, the NIC lock error, the untar path check, symlink
skipping at the source, the plain-publish digest removal, the device probe
bound, the frame cap, the zero-ttl renew default, the single teardown, the
kept archive intent, the warm claim counter, the closeBoxes wait, sparse
holes on Linux, and a mid-handshake SOCKS close that a deadline satisfied.
The cell-isolation check now runs after the cell has settled, and a fake
detach error real cocoon never returns is gone.
A stop that lands during a hibernate closed cocoon's stderr pipe. cocoon died
on its next log line, after the snapshot persisted and before the VMM stopped,
leaving the guest paused while its record read running; with the pipe kept,
the next start's reconcile raced the capture and cold-booted the guest. On
test2 both shapes reproduced, and with this change the stop waited for the
capture and the restarted node woke the claim with its memory.
A client that stopped reading and then closed left the router blocked writing
to the reader, which never got closed, so the reader VM, the goroutine and
shutdown's relay wait were held forever.
#440 dropped the per-name check, so a tenant write anywhere in the cell during
a cold resolve left the tenant uncached, and Peek then skipped its max_claims.
…reports no transfer

The guard's refusal was audited as allow and skipped the refusal limiter, and
the forward path wrote a 0/0 egress_bytes event for every request whose dial
failed, unthrottled.
…e claim

The S3 delete ran under the transition lock, so every exec and dial to a
replicated claim waited on it once per interval.
An even split gave eight owners 225 s each, so a large heal failed on every
owner though the first ones refused at once.
A power loss after the heal left a durable meta over zeroed data, and that
local copy then shadowed the good owner.
…equest

The feeder dropped it silently, so a client on a reused connection waited for
a reply that never came; a fresh connection already answered bad_request.
Attaching to a quiet detached process and dropping the connection held the
task, its vsock fd, its buffer and a broadcast receiver until the process wrote
or exited.
run joined the stdin pump after the exit frame, so a large stdin outlived the
timeout and a guest that stopped reading hung the call; it now aborts the
connection and closes it instead of parking it, as the Go SDK does.
run keeps _lease and only aborts a pump still sending; the refused-dial egress
tests share one table; the heal test reuses sleepyPuller; the engine tests share
waitFile; silkd names the feeder's parse result.
@CMGS
CMGS merged commit a08f552 into main Nov 4, 2026
4 checks passed
@CMGS
CMGS deleted the review/post-441 branch November 4, 2026 14:13
CMGS added a commit that referenced this pull request Nov 4, 2026
…tart

Since #444 a cocoon command asked for after Close answers an error. A target stopping between its commit and its vm start read that as a guest that cannot start, and failed a claim whose guest was intact, so its next start cold-booted it without its memory. The journal now keeps the claim and the restart starts the guest.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant