Repository navigation
Conversation
A file system can report st_size 0 for a regular file that has content. read_env_file_contents returned Empty for such a file before any read, so the file loaded nothing and bun printed no message. Node reads the file until a read returns 0 bytes. The loader now calls fstat once, uses st_size only to presize the buffer, and reads with read(2) until a read returns 0 bytes. This applies to an --env-file argument and to the default .env files. A default entry that is not a regular file is still skipped. A default .env file that fails to read is now reported (unless quiet) and skipped for every errno. Before, an errno outside ENOMEM/EPIPE/EACCES/EISDIR stopped the process. An --env-file argument that fails to read keeps its result.
|
Updated 7:34 PM PT - Oct 6th, 2026
✅ @robobun, your commit 709162c5e492c9d3fd07599fd81e1c526af98f76 passed in 🧪 To try this PR locally: bunx bun-pr 44026That installs a local version of the PR into your bun-44026 --bun |
|
Status: the fix is pushed, and it waits for CI and for review. How I reproduced it (Linux x64, Bun 1.4.2 and main). The newlines in the variable put a dotenv line into a file that reports CARRIER=$'\nPROBE=from-procfs\n' bun --env-file=/proc/self/environ -e 'console.log(process.env.PROBE)' # undefined
CARRIER=$'\nPROBE=from-procfs\n' node --env-file=/proc/self/environ -e 'console.log(process.env.PROBE)' # from-procfs
Test: |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. Walkthrough
ChangesEnvironment File Reading
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The env-file reading change has no identified merge-blocking issue after normal checks. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the Ok(0) => Empty mapping and the st_size presize: read_to_end_into goes through read_fill_vec, which returns the total bytes appended (not the last chunk), so a non-empty file cannot be misreported as empty; and both the up-front try_reserve_exact and the per-chunk growth are fallible, so an absurd st_size surfaces as ReadErr(oom) rather than an abort.
Extended reasoning...
The change rewrites the ~30-line read_env_file_contents in src/dotenv/env_loader.rs to read every env file to EOF via read_to_end_into and widens the recoverable-errno set for default .env* discovery, plus new Linux-only procfs/sysfs tests. It touches no auth, crypto, or injection surface; the two posted findings (silent skip on any errno under quiet, and unbounded reads of zero-size endless files) are the reasons a human should weigh the policy change.
Findings marked 🟡 are optional suggestions and need no follow-up push.
| use bun_sys::E; | ||
| match err.get_errno() { | ||
| E::ENOMEM | E::EPIPE | E::EACCES | E::EISDIR => Ok(ReadEnvFile::ReadErr(err)), | ||
| _ if source == EnvFileSource::Default => Ok(ReadEnvFile::ReadErr(err)), |
There was a problem hiding this comment.
🔴 Script runs now exit 0 with the default .env silently unloaded on any read error, where the base exits non-zero. The new arm at src/dotenv/env_loader.rs:910 turns every errno for EnvFileSource::Default into ReadEnvFile::ReadErr, and the bun run/script path sets quiet = true (src/runtime/cli/run_command.rs:601), so nothing is printed. Fix: keep unexpected errnos loud on every caller, e.g. print unlisted errnos regardless of quiet (or keep propagating them) while still skipping the file for the listed ENOMEM/EPIPE/EACCES/EISDIR set; the new test asserting script.stderr is "" certifies the silence and should change with it.
Why this was flagged
A user runs bun index.ts in a directory whose .env is a regular file that fails on read(2): EIO from a failing disk or network mount, EINVAL/ENOTSUP from an unusual file system, or the /sys/class/net/lo/speed symlink the PR's own test uses. load_env_file at src/dotenv/env_loader.rs:794 calls read_env_file_contents with EnvFileSource::Default; the new arm at src/dotenv/env_loader.rs:910 maps the errno to ReadEnvFile::ReadErr. The caller at src/dotenv/env_loader.rs:796-803 prints only if !self.quiet, and the script entry point sets env_loader.quiet = true at src/runtime/cli/run_command.rs:601 (also src/runtime/cli/run_command.rs:3512 and src/install/lockfile.rs:1805). The process then continues with the .env values missing and exits 0. On the base branch the same errno hit _ => Err(err.into()) and the run failed with a non-zero exit. The PR text says such a file is 'reported and skipped', but reporting only happens for non-quiet callers such as bun install; the new test '.env that fails to read, behind a symlink' in test/cli/run/env.test.ts asserts…
Verification: normal — acknowledged in diff: the new test ".env that fails to read, behind a symlink" pins expect(script.stderr).toBe("") and exitCode 0 for the script run, and the PR table row says "loads .env.local, exit 0"; the author's claim that default discovery "reports the file and continues" holds only for non-quiet callers (bun install), not for bun index.ts, where nothing is…
| return Ok(ReadEnvFile::ReadErr(bun_sys::Error::oom())); | ||
| } | ||
| // `read(2)`, not `pread`: a pipe or a device cannot seek. | ||
| match file.read_to_end_into(&mut buf) { |
There was a problem hiding this comment.
🟡 (optional) A checkout whose .env points at an endless zero-size procfs file now grows bun's heap without bound until the kernel OOM-kills it; the base skipped the file with one fstat. src/dotenv/env_loader.rs:903 reads every regular file to EOF, and the base's st_size == 0 short-circuit is gone, so /proc/self/pagemap (S_IFREG, st_size 0, ~256 GiB of zeros) is read in doubling chunks. Fix: keep a bound for the discovery path and --env-file alike, e.g. a total byte cap on read_to_end_into for env files, or restore the size-0 skip for Default sources. Under a cgroup limit malloc does not fail, so the PR's "skipped, exit 0" only holds with RLIMIT_AS.
Why this was flagged
Trigger: a .env (or .env.local etc.) that is a symlink to a regular file with st_size 0 and no EOF, e.g. /proc/self/pagemap, reached through load_env_file at src/dotenv/env_loader.rs:794 on every script start, every bun install, and once per Worker; also --env-file=/proc/self/pagemap via src/dotenv/env_loader.rs:838. The base at 97246d0 src/dotenv/env_loader.rs:893-895 returned ReadEnvFile::Empty for a regular file with stat.st_size == 0 before any read, so this population was never read; the dismissal's "base already reads sparse files to the end" only covers st_size > 0. Now src/dotenv/env_loader.rs:903 calls read_to_end_into, whose loop in src/sys/lib.rs:4420-4432 grows the Vec via amortized try_reserve(8192) until a read returns 0. On Linux with overcommit or a cgroup memory limit, try_reserve keeps succeeding and read touches the pages, so the process is SIGKILLed by the OOM killer (or thrashes swap) rather than getting the ENOMEM-then-skip path the PR measured under prlimit --as. The user sees bun killed with no message where the base ran normally.…
Verification: normal (trigger is adversarial/contrived: a .env* in cwd, or an --env-file, that is a symlink to an st_size-0 regular procfs file such as /proc/self/pagemap) — acknowledged in diff: the PR description lists "A regular file with st_size 0 and no end is read until allocation fails. /proc/self/pagemap under a 4 GiB limit: 512 MiB read, then skipped, exit 0" and "A bound is a separate…
|
@robobun wake up!! |
|
@robobun wake up!! |
…file-zero-st-size
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| /// An `--env-file` argument: read whatever its kind, as in Node. A read | ||
| /// errno outside ENOMEM/EPIPE/EACCES/EISDIR stops the process. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // `st_size` only presizes the buffer: a file system can report 0 for a file | ||
| // that has content. The file ends where a read returns 0 bytes. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
|
State of this PR:
|
Problem
st_size0 and has content loads nothing. Bun prints no message and exits 0. Node v26.3.0 and Bun 1.4.0 load the file.read_env_file_contents(src/dotenv/env_loader.rs:893) returnsEmptyonst_size == 0, before any read. dotenv: skip .env entries that are not regular files #40711 added the check.Fix
fstatonce and usesst_sizeonly to presize the buffer. It reads until a read returns 0 bytes. This applies to--env-fileand to the default.envfiles..envthat fails to read is reported and skipped. Before, most errnos stopped bun with no message.test/cli/run/env.test.ts(4 new tests, 3 fail with main'ssrc/),no-envfile.test.ts,run-process-env.test.ts.--env-filekeeps its result (cli: fail when an explicit --env-file cannot be loaded #36523 owns it).Background
Loader::loadloads the--env-filelist or the default.env*names. Both end inread_env_file_contents.st_sizeis the size that the file system reports. procfs makes the content on read and reports 0.fstat(4 to 3 syscalls).Downsides
malloc(8192).st_size0 and no end is read until allocation fails./proc/self/pagemapunder a 4 GiB limit: 512 MiB read, then skipped, exit 0. Node aborts.Notes
Repro on main (Linux). The newlines in the variable put a dotenv line into the file.
A
.envthat is a symlink to the same file is skipped the same way. Released versions: 1.3.0 skips the file, 1.4.0 loads it, 1.4.1 and 1.4.2 skip it. The Zig loader had the check, the Rust port dropped it, and #40711 restored it together with the skip of directories, FIFOs and sockets.The fixture is not a use case. A plain
/proc/<pid>/environis not a dotenv file. Its entries end with a NUL, not with a newline, so it parses as one variable. Bun also keeps a NUL inside a value, and Node cuts the value there. The tests put a line of their own into the file through a variable with newlines.Until a release has this.
bun --env-file=<(cat FILE)andprocess.loadEnvFile(FILE)load such a file on main.Read failures. sysfs answers a read of
/sys/class/net/lo/speedwith EINVAL.--env-file=a.env,/sys/class/net/lo/speedinvalid format.envsymlink to it, beside.env.local.env.local, exit 0bun installin that directoryerror: An internal error occurred (EINVAL)EINVAL error loading .env file, then installsThe first row is unchanged on purpose. #21105 and #36523 decide what an
--env-filethat cannot be loaded does. Default discovery is best effort, so it reports the file and continues. It already did that for ENOMEM, EPIPE, EACCES and EISDIR.Measurements. Linux x64, release builds of main and of this branch.
Syscalls on the env file descriptor between
openandclose(strace):fstatfstatpreadpreadfstatreadreadfstatfstatreadst_size0fstat, loads nothingfstatreadread--env-file).envsymlink.env, 4 GiB limitThe default
.envfiles give the same numbers as--env-file. The whole process,bun -e 0with a 33 B.env: 249 to 248 syscalls. A.envis opened once per discovery pass: once for the main thread, once perWorker, once forbun install. An empty.envwith oneWorker: 2 to 4 syscalls. A 33 B.envwith oneWorker: 8 to 6.Inside
read_env_file_contents(gdb, three runs agree):mi_malloc(49)mi_malloc(49)mi_malloc(102416)mi_malloc(102416)mi_malloc(8192), 1mi_freest_size0mi_malloc(8192)Binary size (
size -A):.text58299580 to 58299580 (+0 B), stripped file 80995912 to 80995912 (+0 B),bun-profile-616 B. Two builds of main are byte-identical, so the floor is 0 B.Startup wall-clock,
bun -e 0, 300 interleaved runs, medians: 7.74 ms and 7.60 ms with a 33 B.env, 6.98 ms and 6.99 ms with an empty.env. A second set of copies of the same binaries moved the medians by 1.2 ms, so there is no difference above the floor.No size bound. Node has none, and #40837 documented the same for
--env-file=/dev/zero. A sparse.envof anyst_sizeis already read to its end on main. Underprlimit --as=4294967296,/proc/self/pagemapas--env-fileor behind a.envsymlink: main doesfstatand no read. This PR does 17 reads for 512 MiB in 1.3 s, the next allocation fails, the file is skipped, and bun exits 0. Node exits 134 withstd::bad_allocafter 4.8 s. A bound is a separate decision and a small follow-up if it is wanted.pread to read. The descriptor is new, so its offset is 0 and both calls read the same bytes. The one exception is macOS, where
/dev/fd/Nis a dup that shares the offset of the caller's descriptor. There the file is now read from that offset, ascatand Node read it.The design. A review before the code was written picked this reader over two others. The removal of the check for
--env-fileonly leaves the default files with the bug. A reader withoutfstat, as in Node, costs more for an env file of 8192 bytes or more: 14 syscalls for 100 KiB. The review planned two PRs. Both rewrite the same 30-line function, so this is one.Same class, not in this PR. Each reader has its own PR: #41590, #43910, #41593, #43871, #43964.
Suites. Debug build:
env.test.ts111 pass,no-envfile.test.ts9 pass,run-process-env.test.ts2 pass.cargo check -p bun_dotenvfor x86_64-pc-windows-msvc and aarch64-apple-darwin,cargo clippy -p bun_dotenvclean.test/cli/test/parallel.test.ts: 43 pass, and 2 cases fail the same way with main'ssrc/in this debug build (each worker has a unique JEST_WORKER_ID,lazily scales workers). The new tests run on Linux only. Windows and macOS have no file of this kind to test with.Tools.
strace7.2 built from its release tarball,gdb16.3, binutilssize.bloaty,hyperfine,perfandvalgrindare not installed here.prlimit --asreplaces a memory cgroup.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/run/env.test.ts