Repository navigation
Conversation
AdaAibaby
requested review from
ValentaTomas,
dobrac and
jakubno
as code owners
November 11, 2026 06:10
AdaAibaby
force-pushed
the
oom-defense-layers
branch
from
November 12, 2026 03:25
6416874 to
fe13901
Compare
PR e2b-dev#3810 capped the /init request body at 1 MiB to stop a guest process from OOM-killing envd via an unbounded io.ReadAll before auth. That is correct but narrow: it patches one endpoint, does nothing about the slowloris surface left by ReadTimeout: 0, and gives no way to tell whether envd is actually being OOM-killed in the fleet. This replaces that single-endpoint fix with layered defense. Layer 1 (server edge, main.go): add ReadHeaderTimeout and MaxHeaderBytes. ReadHeaderTimeout bounds only the header read, so it closes the slowloris surface without cutting the long-lived body / connect streams that require ReadTimeout to stay 0. gosec G112 also flagged the unset value. Layer 2 (per-path body cap): new bodylimit middleware caps every path at 1 MiB by default and exempts the streaming endpoints (/files, /upgrade) and the connect services (/filesystem.*, /process.*), which stream to disk or carry their own framing. Default-deny means a handler added later is capped without the author remembering to. The connect services additionally get connect.WithReadMaxBytes. PostInit keeps the 413 MaxBytesError branch but no longer caps inline; the cap is applied at the edge. Layer 3 (observability, orchestrator): the OOM kills envd already reports off /dev/kmsg were only logged, never counted or classified. Add the orchestrator.sandbox.oom.kills counter split by victim_class (control_plane = envd itself, the event e2b-dev#3563 is about and which should stay 0; workload = a user process hitting memory.max, an expected capacity event), plus a kills_dropped floor for kills past envd's 32-entry window. envd kills now log at Error so they alert on their own. This does NOT change the build-envd-memory-protection flag (memory.min on envd.service), which is the primary root-cause protection and is left to a separate rollout because it rebuilds templates. Refs e2b-dev#3563. Supersedes e2b-dev#3810, e2b-dev#3564.
…mp envd PR e2b-dev#3564's review on the 1 MiB /init body cap raised two gaps this closes. 1. Admission bound at the public API boundary. spec/openapi.yml does not cap envVars, yet the orchestrator serialises the whole env map (with the CA bundle and tokens) into POST /init, which envd now caps, and treats a 413 as a fatal create/resume. A sandbox admitted with 1-16 MiB of env therefore could fail only at envd init. Add one authoritative ceiling in shared consts (EnvdMaxInitBodyBytes) that both sides derive from: envd bodylimit.DefaultMaxBytes is retied to it, and the public API rejects a create whose JSON-serialised env map exceeds MaxSandboxEnvVarsBytes (the ceiling minus a 512 KiB reserve for the CA bundle, tokens, mounts and framing the API cannot see) with 400, before the sandbox is placed. openapi.yml documents the cap on the EnvVars schema. 2. Version bump. The envd body-limit behaviour change shipped without moving packages/envd/pkg/version.go, so X-Envd-Version and rollout logic could not tell which nodes enforce the limit. Bump 0.10.2 -> 0.10.3 and record the threshold as MinEnvdVersionForInitBodyLimit so the capability is introspectable; the cap is safe for all legitimate /init traffic, so nothing gates behaviour on it.
…ow-body slowloris Layer 1 added ReadHeaderTimeout to close the slowloris surface, but the server keeps ReadTimeout at 0 so the long-lived streaming/connect bodies are not cut. That leaves the body phase of a slowloris open on the buffered paths: an attacker finishes the headers fast (ReadHeaderTimeout does not fire), then dribbles a sub-cap /init body one byte at a time, holding the connection and its goroutine open indefinitely -- the size cap (MaxBytesReader) bounds bytes, not time. Close it where the size cap already lives: the bodylimit middleware already runs only on non-exempt (buffered) paths, so set a per-request body read deadline (DefaultBodyTimeout, 30s) there via http.NewResponseController(w).SetReadDeadline. The exempt streaming/connect paths are never wrapped and keep their unbounded read, so no legitimate long body/stream is cut. SetReadDeadline is best-effort: an ErrNotSupported writer degrades to the prior size-only behaviour instead of failing the request. Tests assert the deadline is set on a capped path and NOT on the exempt paths.
AdaAibaby
force-pushed
the
oom-defense-layers
branch
from
November 13, 2026 07:45
fe13901 to
0353f3c
Compare
This branch has not been deployed
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.
Problem
fix #3563.
PR #3810 capped the
/initrequest body at 1 MiB to stop a guest process from OOM-killing envd via an unboundedio.ReadAllbefore auth. That fix is correct but narrow:ReadTimeout: 0;This replaces the single-endpoint fix with layered defense.
Layers
Layer 1 — server edge (
envd/main.go)Add
ReadHeaderTimeout(10s) andMaxHeaderBytes(1 MiB).ReadHeaderTimeoutbounds only the header read, so it closes the slowloris surface without cutting the long-lived body / connect streams that requireReadTimeoutto stay0. gosec G112 also flagged the previously-unset value. Zero behaviour change for legitimate traffic.Layer 2 — per-path body cap (
envd/internal/api/bodylimit/)New middleware caps every path at 1 MiB by default and exempts the streaming endpoints (
/files,/upgrade) and the connect services (/filesystem.*,/process.*), which stream to disk or carry their own framing. Default-deny: a handler added later is capped without the author remembering to. The connect services additionally getconnect.WithReadMaxBytes.PostInitkeeps the 413MaxBytesErrorbranch but no longer caps inline — the cap is applied at the edge, below auth.Layer 3 — observability (
orchestrator/pkg/metrics/sandboxes.go)The OOM kills envd already reports off
/dev/kmsgwere only logged, never counted or classified. Addorchestrator.sandbox.oom.killssplit byvictim_class:control_plane= envd itself was killed — the event bug(envd): PostInit reads request body with io.ReadAll and no size limit, enabling OOM via oversized payload #3563 is about; should stay 0.workload= a user process hit its cgroupmemory.max— an expected capacity event.Plus
orchestrator.sandbox.oom.kills_droppedas a floor for kills past envd's 32-entry window. envd kills now log at Error so they alert on their own. This letssum by (victim_class)answer how much of the fleet's OOM is actually the control plane vs. workload capacity.Deliberately out of scope
This does not flip the
build-envd-memory-protectionfeature flag (memory.minonenvd.service), which is the primary root-cause protection (it stops the kernel reclaiming/killing envd under pressure, covering every allocation path a body cap cannot). It is left to a separate rollout because it rebuilds templates and enters the base-layer cache key. Recommend landing Layer 3 first, readingvictim_class, and deciding memory.min rollout from the data.Testing
go build/go vet/gofumpt -lclean acrossenvd,orchestrator/pkg/metrics,shared/pkg/telemetry.bodylimit(over-cap → 413, at-cap → 200, exempt paths uncapped,exempt()table) andoomVictimClass.internal/apisuite (incl. init) passes.Refs #3563. Supersedes #3810, #3564.
/cc @ryan-sana