[sandbox audit] Bound server resources, and fix two file-protocol bugs - #26
Draft
Wauplin wants to merge 1 commit into
Draft
[sandbox audit] Bound server resources, and fix two file-protocol bugs#26Wauplin wants to merge 1 commit into
Wauplin wants to merge 1 commit into
Conversation
A pool host has no cgroups to partition its sandboxes, so anything one sandbox can consume without limit, it can take from its neighbours. Several of those limits were missing, and two file-protocol bugs corrupted data rather than merely wasting memory. **A shrinking file desynchronised the connection.** A read advertised `Content-Length` from the file's size, then stopped early if the file got smaller mid-transfer -- and kept the keep-alive connection open. The next response then began mid-body and was read as the tail of this one. It now fails the connection instead: an honest truncated transfer beats a silently desynchronised one. **A ranged write left a stale tail.** Ranged writes deliberately do not truncate, so overwriting a large file with a smaller one through parallel chunks left the old bytes past the new end. New `truncate_to` parameter lets the client state the final size, so the file ends up exactly what was uploaded. **Caller-supplied limits are now clamped by the server.** `max_procs` and `max_mem_mb` came straight from the request body, and `max_mem_mb * 1024 * 1024` was computed without `checked_mul` -- so a value near 2^54 wrapped in release builds and produced either an effectively unlimited address space or a sandbox where nothing could start. Both are bounded and rejected outside their range, and the multiplication is saturating. **`RLIMIT_NOFILE`, `RLIMIT_FSIZE` and `RLIMIT_CPU`** join the two limits that were already set, so one sandbox cannot exhaust the host's descriptors, fill its disk, or spin a core indefinitely. **Output no longer buffers without bound.** The stdout/stderr channel was unbounded, so a command producing faster than the client reads was buffered in the server's heap -- a client that stopped reading a 50 MiB producer took RSS to ~52 MiB, per connection, on a host shared by dozens of sandboxes. A bounded channel applies backpressure through the pipe to the command instead. That is a behaviour change worth naming: a stalled client now slows the command rather than growing the server. It is how `docker logs` behaves, and the right trade on a shared host. **Invalid numeric configuration refuses to start.** These fell back to a default on a parse failure, which for `SBX_CAPACITY` meant an *unlimited* host -- a typo silently removed the packing bound. `SBX_CAPACITY` also defaults to 64 rather than unlimited; unlimited was the absence of a setting, never a considered choice, and the client always sets it. Also: `count` is bounded by the host's remaining capacity rather than an arbitrary 4096 (the env map is cloned per sandbox); the create `env` is capped at 64 KiB; directory listings are paginated with `limit`/`after` and report `truncated`/`next`; and a non-regular file is refused in dedicated mode too, so a FIFO cannot block a connection thread waiting for a writer. Validation: - 48 unit tests still pass. - `scripts/resource-bounds-regression.sh`, all passing: four invalid configs refuse to start; five out-of-range limit requests are rejected while a sane one succeeds; `count: 4000` on a capacity-4 host creates 3 and reports the rest rejected; a 200 KB env is refused; `ulimit -n` inside a sandbox reports 4096; a ranged overwrite leaves no stale tail; a listing honours `limit`, reports truncation, and its cursor advances rather than repeating; a FIFO is refused promptly instead of hanging; a stalled reader leaves RSS flat (648 kB before and after 64 MiB of unread output); and a 5 MB round-trip plus streamed exec still work. - Verified against the parent commit: every config accepted, every out-of-range limit accepted, the 200 KB env accepted, no pagination. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wauplin
force-pushed
the
security/scope-health-metadata
branch
from
September 8, 2026 14:56
93e8010 to
141cec1
Compare
Wauplin
force-pushed
the
security/bound-server-resources
branch
from
September 8, 2026 14:56
4605065 to
8e62c8f
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.
Why
A pool host has no cgroups to partition its sandboxes, so anything one sandbox can consume
without limit, it takes from its neighbours. Several of those limits were missing — and two
file-protocol bugs corrupted data rather than merely wasting memory.
The two data bugs
A shrinking file desynchronised the connection. A read advertised
Content-Lengthfromthe file's size, then stopped early if the file shrank mid-transfer — and kept the keep-alive
connection open. The next response then began mid-body and was read as the tail of this one.
A ranged write left a stale tail. Ranged writes deliberately don't truncate, so
overwriting a large file with a smaller one through parallel chunks left the old bytes past
the new end. (This is also what clippy's
file opened with create, but truncate behavior not definedwas pointing at.)The bounds
max_procs/max_mem_mbcame straight from the request body, andmax_mem_mb * 1024 * 1024had nochecked_mul— a value near 2^54 wrapped in release builds, producing eitheran effectively unlimited address space or a sandbox where nothing can start.
RLIMIT_NPROCandRLIMIT_ASwere set — nothing stopped one sandbox exhaustingthe host's descriptors, filling its disk, or spinning a core.
server RSS to ~52 MiB, per connection.
SBX_CAPACITYthat meant anunlimited host, so a typo silently removed the packing bound.
Behaviour change worth naming
Bounding the output queue means a stalled client now slows the command rather than growing
the server's heap. That's how
docker logsbehaves and the right trade on a shared host, butit is a real change: a command producing faster than its consumer reads will block. The buffer
is ~2 MiB, comfortably more than any interactive command produces between reads.
SBX_CAPACITYalso now defaults to 64 rather than unlimited. Unlimited was the absence ofa setting, never a considered choice, and the client always sets it explicitly.
Also
countis bounded by the host's remaining capacity rather than an arbitrary 4096 (the env mapis cloned per sandbox); the create
envis capped at 64 KiB; directory listings are paginated(
limit/after, reportingtruncated/next); and a non-regular file is refused in dedicatedmode too, so a FIFO can't block a connection thread waiting for a writer.
Validation
scripts/resource-bounds-regression.sh, all passing:Verified against the parent commit: every invalid config accepted, every out-of-range
limit accepted, the 200 KB env accepted, no pagination.
Two of my own test bugs along the way: a 200 KB env on the command line exceeds
ARG_MAX, andI set
SBX_PORTtwice in oneenvinvocation so the valid value won — the check passedagainst a server that had ignored the bad value entirely.
Wire changes
PUT /v1/files/writegainstruncate_to.GET /v1/files/listgainslimit/afterand returnstruncated/next(additive).short and reusing it.
The client-side half of this finding (full output capture even with callbacks,
read_bytes()uploads, fully-materialised parallel downloads) is a separate PR.