Read --env-file FIFOs until EOF instead of trusting stat.size

[robobun]

Fixes #30520

Problem

bun --env-file=<FIFO> silently loaded zero variables when a writer
was attached. Reproduction:

$ mkfifo /tmp/p
$ ( printf 'FOO=worked\n' > /tmp/p ) &
$ bun --env-file=/tmp/p -e 'console.log(process.env.FOO ?? "<unset>")'
<unset>

This broke 1Password's local-env-file
integration, which exposes secrets through a FIFO so they never touch
disk. cat, dotenv, and node:fs.readFileSync all read the same
FIFO correctly.

Cause

src/dotenv/env_loader.zig (both loadEnvFile and loadEnvFileDynamic)
sized its read buffer from stat.size and short-circuited when
stat.size == 0 or stat.kind != .file:

if (stat.size == 0 or stat.kind != .file) {
    try this.custom_files_loaded.put(file_path, logger.Source.initPathString(file_path, ""));
    return;
}

FIFOs always report st_size == 0 and stat.kind == .named_pipe, so
the loader recorded the path as 'loaded' with empty content and
returned without reading anything.

Fix

Extract the read logic into readEnvFileContents. For non-regular
files (FIFOs, sockets, character devices), read until EOF into a
growing ArrayList (initial capacity 4 KiB, reserves 4 KiB of unused
space before each read) instead of preallocating from stat.size.
Regular files keep the existing fast path that preallocates from
stat.size.

Both loadEnvFile (default .env discovery) and loadEnvFileDynamic
(--env-file=) share the helper.

Verification

test/cli/run/env.test.ts adds two tests inside the --env-file
describe block, gated on isPosix:

1. reads variables from a FIFO with a writer attached — spawns a
   writer that writes a single BUNTEST_FIFO=hello-fifo line, reads
   it back via --env-file, and checks the value surfaces in
   process.env.
2. reads FIFO content larger than the initial read buffer — writes
   500 entries (~16 KiB) through the FIFO to exercise the grow loop,
   then spot-checks the first and last entry.

Both tests fail with git stash push -- src/ and pass with the fix
applied (stash pop).

bun run zig:check-all is clean across Linux x86_64/aarch64, Windows
x86_64, and macOS. The Windows code path is unchanged (keeps using
file.getEndPos()).

[robobun]

^{Updated 7:50 PM PT - May 11th, 2026}

❌ @robobun, your commit bdae676 has 3 failures in Build #53531 (All Failures):

• test/js/bun/s3/s3-storage-class.test.ts - code 1 on 🍎 14 aarch64
• test/cli/install/bun-install-registry.test.ts - code 1 on 🪟 11 aarch64
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64-baseline
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 30521

That installs a local version of the PR into your bun-30521 executable, so you can run:

bun-30521 --bun

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Centralizes env-file reading into readEnvFileContents (reads sized or until-EOF depending on file type), refactors loadEnvFile and loadEnvFileDynamic to use it (handling empty-file nulls), and adds POSIX FIFO tests to validate FIFO-backed env-file reading.

Changes

FIFO-aware env-file reading

Layer / File(s)	Summary
File reading helper src/dotenv/env_loader.zig	New readEnvFileContents helper allocates a null-sentinel buffer and returns it with amount_read. Handles Windows via end-position read, returns null for empty regular files, and for non-regular files (FIFOs) reads until EOF with a growable buffer.
Loader refactoring src/dotenv/env_loader.zig	loadEnvFile and loadEnvFileDynamic refactored to call readEnvFileContents and remove duplicated buffer-allocation logic. Both now handle null return (empty file) by short-circuiting parse and updating I/O error handling: error.OutOfMemory propagates, other read errors are best-effort logged and the source is marked not loaded or an empty logger.Source is inserted.
POSIX FIFO tests test/cli/run/env.test.ts	Added mkfifo import and new POSIX-only test describe block with two tests: verifies --env-file reads variables from a FIFO after a writer exits, and verifies FIFO payload larger than initial buffer is fully loaded by checking first and last generated env entries.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Read --env-file FIFOs until EOF instead of trusting stat.size' clearly summarizes the main change: implementing read-until-EOF logic for FIFO files instead of relying on stat.size.
Description check	✅ Passed	The description comprehensively covers both template sections: 'What does this PR do?' is addressed with problem, cause, and fix; 'How did you verify your code works?' details testing approach with specific test cases and validation results.
Linked Issues check	✅ Passed	All coding requirements from #30520 are met: the implementation reads FIFOs until EOF instead of trusting stat.size, handles non-regular files with a growing ArrayList, maintains the fast path for regular files, and adds comprehensive POSIX-gated tests validating FIFO reading.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the FIFO reading bug: src/dotenv/env_loader.zig refactors read logic into readEnvFileContents helper, and test/cli/run/env.test.ts adds FIFO-specific regression tests. No unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 src/dotenv/env_loader.zig:893-901 — Minor: the refactor dropped the caller-scope errdefer this.allocator.free(buf) that previously covered try Parser.parse(...) (and try this.custom_files_loaded.put(...) in loadEnvFileDynamic). The errdefer inside readEnvFileContents goes out of scope once it returns, so if those later calls return error.OutOfMemory, read.buf is leaked. It's effectively unreachable in practice (OOM during startup env loading), but adding errdefer this.allocator.free(read.buf); right after the orelse unwrap in both call sites would restore parity with the old code.

  Extended reasoning...

  What changed

  Before this PR, both loadEnvFile and loadEnvFileDynamic allocated buf directly at caller scope and immediately followed it with errdefer this.allocator.free(buf);. That errdefer remained active for the rest of the function body — in particular across try Parser.parse(...) (which returns OOM!void) and, in loadEnvFileDynamic, across try this.custom_files_loaded.put(...) (also OOM-fallible). If either of those returned an error, the buffer was freed on the way out.

  After the refactor, allocation moved into readEnvFileContents(). That helper has its own errdefer this.allocator.free(buf); (and errdefer list.deinit(); for the FIFO path), but Zig's errdefer is scoped to the enclosing block — once readEnvFileContents returns successfully, those errdefers are gone. Back at the call sites, after const read = read_result orelse { ... }; there is no errdefer protecting read.buf. The subsequent try Parser.parse(...) and try this.custom_files_loaded.put(...) can still return an error, and if they do, read.buf is now leaked where it previously was freed.

  Step-by-step

  1. loadEnvFileDynamic is called with --env-file=/tmp/fifo.
  2. readEnvFileContents reads ~16 KiB from the FIFO into an ArrayList, calls toOwnedSlice(), and returns { .buf = owned, .amount_read = 16384 }. Its internal errdefer list.deinit() is discharged on successful return.
  3. Caller unwraps: const read = read_result orelse { ... }; — read.buf now owns 16 KiB with no cleanup registered.
  4. Parser.parse(source, this.allocator, ...) runs and hits error.OutOfMemory inside allocator.dupe while inserting a value into the map.
  5. The try propagates the error out of loadEnvFileDynamic. No errdefer covers read.buf, so the 16 KiB allocation is leaked. The pre-PR code freed it via the caller-scope errdefer.

  The same applies to loadEnvFile at the try Parser.parse(...) site, and to loadEnvFileDynamic at the trailing try this.custom_files_loaded.put(file_path, source.*).

  Why it doesn't matter much

  The only error these calls can return is error.OutOfMemory. Bun's allocator behavior on OOM is effectively fatal — the process is not going to recover from OOM during startup env-file loading and continue running with a meaningfully observable leak. On the success path the buffer is intentionally never freed anyway (it backs the logger.Source stored on the Loader for the program's lifetime), so this is purely about the OOM error path. This is a strict behavioral regression from the pre-PR code, but the practical impact rounds to zero.

  Fix

  Add errdefer this.allocator.free(read.buf); immediately after the orelse unwrap in both loadEnvFile (after const read = read_result orelse { ... }; around line 897) and loadEnvFileDynamic (around line 947). That restores the original cleanup contract with a one-line change at each site.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/dotenv/env_loader.zig`:
- Around line 811-818: The null terminator is being written at the probed size
variable pos instead of the actual bytes read, so change the sentinel assignment
from buf[pos] = 0 to buf[amount_read] = 0 (and ensure the allocation still has
room for amount_read + 1); update the same change in the second occurrence
(lines 866-873). Locate the read path using bun.sys.readAll, the local variables
buf, pos and amount_read, and write the trailing zero at buf[amount_read] to
ensure the buffer is null-terminated at the real read length.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0ddeb553-b747-433c-a879-138a076a5a7a

📥 Commits

Reviewing files that changed from the base of the PR and between 9a49138 and b462662.

📒 Files selected for processing (1)

• src/dotenv/env_loader.zig