fix(envd): serve procfs/sysfs files on identity download path - #3364
fix(envd): serve procfs/sysfs files on identity download path#3364mishushakov wants to merge 1 commit into
Conversation
GET /files returned an empty 200 for virtual files (e.g. /proc/sys/kernel/random/boot_id) unless the client negotiated gzip. http.ServeContent sizes the response by seeking to the end of the file, and virtual files report stat size 0, so the identity path served Content-Length: 0 with no body while the gzip path streamed the real content. When stat reports size 0, buffer the file (capped at 16 MiB) and serve it from a bytes.Reader so both genuinely empty files and virtual files get a correct Content-Length, preserving Range/conditional semantics. Files that outgrow the cap are streamed without a length, matching the gzip path. Fixes #3363 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit 70db01a. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
tbh this is an edge case and does not require immediate attention |
❌ 7 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — a well-tested, contained fix for the reported bug.
What was reviewed: the zero-size buffering branch (correct Content-Length, Range/conditional handling preserved via bytes.Reader), the overflow-to-streaming fallback for files exceeding the 16MiB cap, and the gzip path (unaffected, as intended). The finder-flagged concern about the overflow branch potentially hanging on non-terminating virtual files was examined by the bug-hunting system and ruled out.
Extended reasoning...
Overview
This PR fixes a real bug (#3363): the identity (non-gzip) download path in envd used http.ServeContent directly against the file handle, which seeks to determine content length. For procfs/sysfs files that report stat.Size() == 0 but yield real content on read, this produced an empty 200 response. The fix buffers zero-stat-size files (capped at 16 MiB) into memory and serves them via a bytes.Reader, which lets http.ServeContent correctly size the response and still handle Range/conditional requests. Files exceeding the cap fall back to unbuffered streaming via io.Copy, mirroring the existing gzip path's behavior. Change is isolated to packages/envd/internal/api/download.go, with new tests and a version bump per the envd behavioral-change policy.
Security risks
No new auth, permission, or data-exposure surface — the code path already required the existing signing/auth checks earlier in GetFiles, and this change only affects how bytes already permitted to be read are packaged into the HTTP response. The main-loop bug hunting system flagged a candidate concern that the overflow-streaming branch could hang forever on a non-terminating virtual file (e.g., an unbounded /proc entry), but that was examined and ruled out — it doesn't introduce a materially different risk than the pre-existing streaming behavior on the gzip path.
Level of scrutiny
This warrants light scrutiny: it's a small, single-purpose fix to a non-auth code path in envd (in-VM daemon), backed by targeted regression tests (empty file, virtual file content, and Range request against a real procfs file) that were verified to fail on old code and pass on the fix. The blast radius is limited to file-download responses for zero-stat-size files.
Other factors
Tests are meaningful (not just happy-path assertions) and include a Linux-specific Range/conditional test against real procfs, matching the PR's own verification claims. The version bump follows the project's documented policy for envd behavioral changes. No outstanding reviewer comments to address; only automated summary from Cursor Bugbot, no bugs raised.
5aad415 to
d71980e
Compare
1 participant
Fixes #3363
Problem
GET /filesreturned an empty 200 for procfs/sysfs files (e.g./proc/sys/kernel/random/boot_id) unless the client negotiated gzip. Virtual files report stat size 0 even though reading them yields content, and the identity path delegates tohttp.ServeContent, which sizes the response by seeking to the end of the file — so it setContent-Length: 0and copied zero bytes. The gzip path streams withio.Copyand was unaffected, which is why undici/httpx-based SDKs worked by accident while Cloudflare Workersfetchand plaincurl/wgetsilently read virtual files as empty.Fix
On the identity path, when
stat.Size() == 0, buffer the file (capped at 16 MiB) and serve it viahttp.ServeContentwith abytes.Reader:Content-Length: 0/proc/version)Also bumps the envd version to 0.6.11 per the behavioral-change policy.
Testing
/proc/version(Linux-only), and a Range request against/proc/version(Linux-only).packages/envdtest suite passes on Linux (docker,golang:1.26) and macOS;make lintandmake fmtare clean for the touched packages.🤖 Generated with Claude Code