Repository navigation
Whole-repo static audit: fence egress and admission, snapshot leak, archive retry spacing, 30 more fixes - #501
Merged
Merged
Conversation
…by ctx, check -b before claiming egresssmoke dropped the exec error of the env leak probe, so a failed exec passed as "secret absent". PortRelay.Dial bounded only the TCP dial, so a relay that never answered the 101 hung portsmoke past its deadline. crossnode claimed and published on A before noticing a missing -b. The egresssmoke -guarded=false negative control had no caller and goes; coldproof's help no longer claims its pooled default is a cold boot.
…ing comments qaab measured the 2026-09-12 queue-affinity A/B and nothing runs it. grep -c exits 1 on a zero count, so portsmoke's err == nil && out == "0" case never held. guestserver writes its report straight to the response, smoke and meshsmoke use errors.New for constant messages, rpcbench builds its upgrade request with ctx, and wire_parity names tenants.TenantSpec instead of reaching it through reflect.
… pipe A closed terminal sends SIGHUP and a dead client turns the next reply write into SIGPIPE; both killed the process before closeBoxes ran, so every claimed sandbox stayed held until its lease ended, against docs/mcp.md.
…ep one reply encoding The MCP spec classes an unknown tool as a protocol error. The reply marshal options live in one var, execTimeout is named toolTimeout since it caps every tool call, and the TTL comment sits on the constant it explains.
silkd maps ENOENT to not_found and every other fs error to internal, and a recursive rm of a missing path is not_found; the fake had the kinds inverted. A detached exec gets only Started from silkd, while the fake also queued output and exit frames on a connection the SDK parks for reuse. Sessions list sorted, Root is unexported, and hijackedConn moves below the exported funcs.
The mesh shutdown was deferred to main's return, after the HTTP drain and eng.Close, which waits for a running capture up to the save budget. Peers kept redirecting claims to a node whose listener had already closed for that whole window. Leaving only drops the node from peers' views; no failover keys off it.
reader is config-owned, so an API spec never carries it and ValidateLimits never fired its warm floor there; a PUT could set warm 0 on a config reader pool and every read of a sleeping claim then woke it, against docs/deploy.md.
rollbackClaim dropped and tore down every claim of the batch without checking it was still the live claim, unlike the renew, env and tun rollbacks. A release landing between the commit and a failed egress arm dropped it twice: the tenant count went low and a second release event was billed.
…onfig validation text sandboxd_nodes had its own creation step only because it arrived after the archives table; one schema creates both, and metastore.EnsureTable is unexported. max_fork_count joins the negative-field table (a zero is already defaulted), the peer-heal error no longer names an internal function, validateEgress is validatePools since it validates every pool, and the usage rescan warning no longer claims a loss when the landed line was merely wiped.
…s at the new owner rebind compared the looked-up owner with the handle's current owner, so when a concurrent call rebound the handle during this call's Lookup, the second call saw no change and returned the old owner's error although the sandbox was reachable. It now compares with the address that failed.
… node leaves time to follow the move A node that drops packets made the first release spend the transport's 30 s dial timeout, the whole release budget, so Lookup ran on an expired ctx and an adopted sandbox was never released, against docs/sdk.md.
…rough, refuse an empty host and malformed rule hosts The server inside an intercepted tunnel had no IdleTimeout, so a guest's pooled keep-alive held the claim busy forever and idle hibernate never fired. The transport added its own gzip for clients that sent no Accept-Encoding, then decompressed on the host, counting decompressed bytes as egress. hostPort accepted CONNECT :443 against its own contract. A rule host such as *example.com or host:443 never matches traffic but Covers read it as a wildcard, so a claim credential write for it was accepted and never injected.
…and drops two restating comments
…s proxy doors Only the tun packet path checked the fence, so after a node lost its lease and a peer adopted a replicated claim from its replica, the old copy kept reaching the internet through the HTTP, SOCKS5 and intercept doors with the claim's credentials, against docs/cluster.md. The claim's evaluator now denies while fenced, the trip ends the open tunnels of replicated claims, and a claim another node took is disarmed before the fence lifts.
… and stop DNS waiters on disarm ArmProxy and RearmTun returned early for a claim whose egress policy a reload removed without sending tun_down, so a woken or moved guest kept tun0 and its default route up and redialed a door that never came back, against docs/advanced-network.md. The follow-up TunDown after a failed TunUp gets its own budget. A DNS query waiting for an in-flight slot now returns when the resolver is cancelled instead of running and auditing for a released claim.
…ient lands on a peer Admission never checked the fence: during a meta-store outage or the first seconds after start a node admitted and billed claims of replicating pools whose every later call answered 503 lease lost. The refusal is a quota refusal, which the claim handler already redirects to a peer. Two tests that claimed on a fenced node now claim before the trip or on an unreplicated pool.
…, fork or reader holds it Release without archive_failover and the reap's destroy dropped the snapshot without the claim's transition lock, while a checkpoint, promote, archive or fork export or a reader copy could hold it under that lock; cocoon refuses the drop then, dropSnap only warned, and the guest-sized image stayed until the next restart. The drop now runs under the transition lock, in the background when the lock is busy.
… retry A failure only cleared the archiving mark, so every 5 s reap tick re-ran the whole export: during a store outage each idle hibernated claim was dumped into data_dir every cycle while holding its transition lock, and an on_expire archive claim kept being paused and saved. The expiry tests now pin the spacing.
…ts targets by default A broken .goreleaser.yml surfaced only when a v* tag ran release.yml, after the tag was public. The separate go build step repeated what lint and test already build. A bare make ran the LOCALBIN mkdir rule instead of help, and archive-e2e.sh lost its executable bit.
settleInterrupted holds the claim's transition lock until the capture that outlived sandboxd exits, up to the 30 min resume budget. The idle sweep picked such a claim, blocked on its lock and kept the sweep flag set, so no claim on the node idle-hibernated meanwhile and idle tenants kept being billed as running.
…lish fails A publish that errored after the store had committed, or whose reply was lost, left a checkpoint no record names; only the startup sweep could reclaim it, and only while the claim was still journaled. The delete marker makes the retry sweep remove it once the store answers.
The golden snapshot was named after the pool hash alone, so a host crash during its save left a pending cocoon record holding that name that the restart sweep cannot see, and every rebuild of the pool failed until an operator ran cocoon gc. The snapshot is transient and found only by its prefix, as hibernate and fork snapshots already are.
A move's unrow consumes the row; when its reply was lost and the move aborted, refusedReplace only refreshed the epoch, but the registry refuses every replace of a consumed row. Env, renew and tun rewrites then reported success with the row unchanged, and every replica round captured and uploaded for nothing until the hourly sweep, leaving no adoptable row. Under the transition lock a consumed own row can only be such a lost reply.
…eader VM Any dial error force-dropped the reader, including a cancel caused only by the caller hanging up, which cut every other live stream on that reader and made the next read pay for a new warm VM.
…, export a fork into its temp dir, and fix wrong comments The pools.json restore loop repeated desiredPools' validation. resolveVolumes and the live path wrapped ValidateVolumes the same way. cocoon's snapshot export accepts an empty target, so the fork's extra export subdirectory goes. The sweepStaleMoves and settlePendingSnap docs described the wrong behavior, and three comments restated the code or a reason that does not hold.
…, so the SDK walks on A require_promoted volume claim at a node that holds the volumes but not the template answered 400 bad volume. The SDK stops a redirect walk on 400 and walks on on 404, so a claim another candidate could serve failed, against docs/cluster.md and docs/sandboxd-api.md. The test that pinned 400 now pins the documented 404.
…p for previews this node minted, and drop an EncodeResponse guard that cannot fire
…ssage, and the silkdOnce doc matches its callers The frame struct decoded only type and data, so a refused or failed guest port read as "port stream ended: error" with silkd's reason dropped. TunUp and TunDown read a Done through silkdOnce, which its doc said never happens.
The claim's removal from claims.json is fsynced but the append to tun-freed.log was not, so after a host crash in the writeback window the address could be handed out again at once and a gateway exit's conntrack could deliver the old claim's retransmits to the new tenant. One fsync per tun release, off the claim path.
The full TCP checksum in segmentOf ran before the merge check, so a link that never merges paid it on every inbound frame for nothing.
… loop ended, and wrap an early close as io.ErrUnexpectedEOF dialAgent had no dial timeout, so a call with a background ctx to an unreachable owner blocked for the kernel's SYN retries, about 127 s on Linux, against 30 s on the control plane. ProxyPort's accept loop returned on any error, EMFILE included, but left the listener open, so later clients hung. A connection closed before a terminal frame now matches errors.Is(err, io.ErrUnexpectedEOF). Three comments restated their functions.
…ailed peer discovery in the lookup error, and close the LangChain toolkit's client The setsockopt calls sat outside the try that closes the socket, so a reset peer (EINVAL on macOS) leaked the fd and leaked a raw OSError. lookup turned any /v1/peers failure into no peers, so a 401 or 5xx surfaced as a bare no owner found, unlike the Go SDK. The toolkit's close released the sandbox but left the client's parked connections open.
…ossip fields, move admission, S3 chunk size and fixture checks to the code The tap lock drops every packet the guest sends, replies and ARP included. WARM_N counts back-to-back warm-tier claims. The shared tun table takes one round trip per candidate address, the outer port is one of senders sockets, and every config reload re-announces. Gossip also carries the client address, move_concurrency is node-wide, a file's last S3 chunk is short, an unparsable frame inside an upload keeps the connection, Rust parses request fixtures rather than round-tripping them, a writer error comes back wrapped, and the unbuilt multi-stream flag note goes.
ArmProxy never arms a no_egress claim, so the address assignTun gave it was never used, yet on a mesh it held a row of the cell's shared address table for the claim's whole life.
…ndex test on a marshal error A Fork failure's t.Fatalf skipped close(stop) and wg.Wait, so the renew goroutine kept calling t.Errorf after the test returned and panicked. A dropped json.Marshal error let the token-leak check pass on empty bytes.
…early when the server exits
The malformed-host check refused any host holding a colon, so an IPv6 allow rule, which matched an unbracketed CONNECT authority before, now failed validation and a config carrying one stopped sandboxd at load.
Routing the restore through desiredPools brought the new reader warm floor into startup, so a pools.json or cell set saved by an earlier PUT with warm 0 on a reader pool failed NewManager and the node never started. The floor stays on the API path.
…d give a no_egress claim no tun address through SetTun either The admission check keyed on the claim key, but replication follows the policy key, so a claim from a promoted template of a replicating pool or from a checkpoint was still admitted and billed while fenced. One helper now gates both admission and finalize. planTun still assigned an overlay address to a no_egress claim. docs/cluster.md names the fence's egress and admission effects.
…aint check requires
Both tables first shipped in v0.1.18, so no released database lacks sandboxd_nodes; the test pinned the separate creation step the schema fold removed.
CMGS
force-pushed
the
review/whole-repo-2026-11-07
branch
from
November 7, 2026 09:45
c4e7dea to
13257b3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Whole-repo static audit of main 3eca9ec: every source file read in full by per-package readers (Go prod, Go tests, Rust, Python, docs, infra) plus five cross-file flow readers, every finding adjudicated against the source, then an independent review of the final diff. No hardware or E2E run in this round; CI is the gate.
The silkd changes from the same round are in a separate PR because they rebuild the guest image chain.
Bugs fixed (each in its own commit, regression test where cheap)
goreleaser checkon PRsCuts and tidy
qaab driver (-198), egresssmoke negative-control flag, dead portsmoke branch, archives nodes.sql folded into schema.sql, duplicate volume normalizer, fork export subdir, restating or wrong comments, egress layout, docs drift (tap lock, bench knobs, tun allocation, gossip fields, move admission, S3 chunk size, fixture checks, fence effects, sync_claims scope).
Numbers
Kept without code (reason)