bundler: copy inlined env values into the define arena instead of aliasing the env map

[robobun]

Problem

• With env inlining on ([serve.static] env = "inline", bun build --env=inline, Bun.build({ env: "inline" }), or a Worker), assigning HTTP_PROXY / HTTPS_PROXY / NO_PROXY on process.env after the define table is built makes later output inline garbage such as "@z..\x03\x00\x00roxy-at-startup.example:8080/..." in place of the value.
• The ASAN build reports heap-use-after-free in bun_js_printer::best_quote_char_for_string, freed by Map::put via Bun__setEnvValue.
• Cause: each process.env.X define pointed at the bytes held by the env map instead of owning them, and a proxy-variable write replaces and frees that map entry while the table is still in use.
• The table outlives such writes in the dev server (built once per server), bun build (a macro can write mid-file), Bun.build (a plugin onLoad can write mid-build) and Workers (built at spawn).

Fix

• Copy each value into the define table's own arena before building the string node, which is what --define values already do. (Bun.build: read env from a per-build copy instead of the VM's live loader #37780 separately gives Bun.build its own copy of the whole map; the two overlap only in tests.)
• Correct because a define is a snapshot of the environment at table-build time, and bytes in the arena live exactly as long as the nodes pointing at them. Cost is one small copy per variable per table build; bun run, bun test and Bun.Transpiler never take this path.
• Known trade: a Worker's arena is never bulk freed, so repeated worker spawns now retain the environment's bytes per spawn on top of the nodes they already retained, until Stop inlining process.env dot-reads in Worker-thread transpiles #34211 stops Workers inlining env.
• Verification: four new tests (dev server, bun build with a macro, Bun.build with a plugin, Worker) fail on the released binary with the clobbered strings above, abort under ASAN on the unfixed debug build, and pass with the fix.

Background

• Env inlining: with env = "inline" the bundler replaces every process.env.X read with a string literal. It builds a define table (one constant per variable) from the env map up front and reuses it for every module it prints.
• The env map is bun's native copy of the environment (the dotenv Loader). Most process.env writes stay in JS; the proxy variables are the exception and are written through to the native map, where Map::put frees the previous entry.
• The define table's nodes come from a bump arena owned by whoever owns the table (the dev server, one Bun.build, the CLI) and freed with it, so anything copied into that arena has the same lifetime as the table.
• RefCountedEnvValue holds the last JS-assigned proxy value so later Workers inherit it; its comment said the env map borrowed those bytes, which stopped being true when Map::put began copying, so the comment is corrected here.

Original description

What

When env inlining is on, copy_env_for_define builds one process.env.X define per variable. env_string_store_put (src/bundler/defines.rs) allocated the E::String node in the define arena but left its bytes pointing into the dotenv Loader's map ("Value bytes alias the long-lived env-map storage"). That storage is not stable: assigning HTTP_PROXY / HTTPS_PROXY / NO_PROXY (or the lowercase variants) on process.env goes through Bun__setEnvValue -> Map::put, which replaces the entry and frees the Box<[u8]> the define points at. Every define table built before that write then prints freed memory the next time a module that reads the variable is parsed or printed.

The define table outlives such writes in four places:

• bake DevServer ([serve.static] env = "inline"): Framework::init_transpiler runs configure_defines once when Bun.serve() starts and keeps the table for the life of the server. A runtime process.env.HTTPS_PROXY = ... followed by any rebuild of a module that reads process.env.HTTPS_PROXY serves the freed bytes.
• bun build --env=inline: the macro VM shares the CLI transpiler's loader, so a macro assigning a proxy variable frees the bytes before the rest of the file is printed.
• Bun.build() / non-HMR HTML routes: a plugin onLoad (JS thread) assigning the variable mid-build frees the bytes the bundle thread prints later.
• Worker VMs: WebWorker::start_vm calls configure_defines with the Target::Bun default of load_all (the main thread, bun test, the REPL and compiled executables switch to load_all_without_inlining first; workers do not, which is what Stop inlining process.env dot-reads in Worker-thread transpiles #34211 changes), so a worker's table is built from its own env map at spawn. A worker that assigns the variable and then imports a module reading it gets the freed bytes.

Repro

Dev server (bunfig.toml has [serve.static] / env = "inline", index.html loads index.ts which does console.log("v1", process.env.HTTPS_PROXY)), run with HTTPS_PROXY=http://proxy-at-startup.example:8080/aaaa... set:

import html from "./index.html";
const server = Bun.serve({ port: 0, development: true, routes: { "/": html }, fetch: () => new Response("", { status: 404 }) });
await fetch(server.url); // first bundle is fine
process.env.HTTPS_PROXY = "http://changed-at-runtime.example:1/";
writeFileSync("index.ts", 'console.log("v2", process.env.HTTPS_PROXY);'); // triggers a rebuild
// the rebuilt chunk now contains:
//   console.log("v2", "@z..\x03\x00\x00roxy-at-startup.example:8080/aaaa...")

Release 1.4.0 inlines the freed buffer with the allocator's free-list word in its first bytes; the other three variants produce the same shape (bun build --env=inline with a macro: "\x00\x00\x00\x00\x00\x00\x00\x00roxy-at-start.example:8080/..."; a worker posts back " \nA\x00\x02\x00\x00roxy-at-launch.example:8080/..."). The ASAN debug build reports, for all of them:

ERROR: AddressSanitizer: heap-use-after-free
    #0 bun_js_printer::best_quote_char_for_string src/js_printer/lib.rs:680
    ...
freed by thread T0 here:
    <bun_collections::array_hash_map::StringArrayHashMap<bun_dotenv::env_loader::HashTableValue>>::put
    <bun_dotenv::env_loader::Map>::put
    Bun__setEnvValue src/runtime/api/BunObject.rs
    Bun::jsSetterProxyEnvironmentVariable src/jsc/bindings/JSEnvironmentVariableMap.cpp

Fix

env_string_store_put copies the value into bump before building the E::String. bump is the arena the define table's nodes are already allocated from, so the bytes live exactly as long as the nodes that point at them: UserOptions.arena for the dev server (dropped with the DevServer), the per-build arena for Bun.build (dropped with the build), the CLI arena for bun build. This is also what DefineData::parse does for --define values a few lines down, and what E::EString::init documents as its contract (the data must be arena owned).

A define is a snapshot of the environment at the time the table is built (that is what gets baked into the output), so owning the bytes is the semantics the code already intended; the old comment just assumed the map entries were immutable. The cost is one small copy per variable each time a table is built with inlining on. The main VM, bun test and Bun.Transpiler never reach this function, so bun run startup is unaffected.

Workers do reach it, and there the arena is the per-VM transpiler_arena, which wraps the main mimalloc heap and is never bulk freed. The define nodes built at worker spawn already stayed allocated after the worker exited; with this change the value bytes do too, so a process that spawns workers repeatedly retains roughly the byte size of its environment per spawn on top of that (a few KB for a typical environment) until #34211 lands, which stops workers from inlining env at all and takes them off this path entirely. Leaving workers reading freed memory is the worse trade, and fixing the arena ownership or the inlining default is that PR's job rather than this one's.

Two doc lines record the invariant where the next reader will look for it: Map::put frees the previous value (and JS can trigger it through Bun__setEnvValue), and get_http_proxy returns a borrow of such an entry. The RefCountedEnvValue doc in rare_data.rs still said the env map borrows those bytes, which has not been true since Map::put started boxing its own copy, so it is corrected as well. The two S3 call sites in blob/Store.rs that hold a get_http_proxy borrow across JS-visible option reads are the same invariant in a different consumer and have been filed separately.

#37780 is the complementary change for Bun.build: it gives each build its own copy of the loader so the bundle thread stops reading the VM's live map at all (the map can also be mutated while the build is iterating it, which a byte copy does not address). This PR is what covers the dev server, bun build and workers, where the table is built and later read on the same thread and only the aliased bytes were the problem. Each is correct on its own; both append tests to bundler_env.test.ts, so whichever lands second needs a trivial rebase, and the Bun.build plus plugin test here can be dropped at that point since #37780 carries the same case.

Tests

• test/bake/dev/bundle.test.ts: dev server with [serve.static] env = "inline"; checks the first bundle inlines the startup value, assigns process.env.HTTPS_PROXY through a route, rewrites index.ts to force a rebuild, and checks the rebuilt bundle still inlines the startup value.
• test/bundler/bundler_env.test.ts: bun build --env=inline where a macro assigns the variable (checks the output file and runs it), and Bun.build({ env: "inline" }) where a plugin onLoad assigns it mid-build.
• test/js/web/workers/worker.test.ts: a worker assigns HTTPS_PROXY, then imports a module that reads it; the value it posts back must be either the launch value or the assigned one (so the test stays valid once Stop inlining process.env dot-reads in Worker-thread transpiles #34211 makes workers read live), never the contents of the freed entry.

All four fail on the released binary (USE_SYSTEM_BUN=1, the clobbered strings above) and on the unfixed debug build (ASAN abort; the dev test reports "DevServer crashed while waiting for hot reload"), and pass with the fix. Also ran test/bake/dev/bundle.test.ts in full, the worker-env tests in worker.test.ts, bundler_splitting.test.ts, cli/run/env.test.ts and transpiler/macro-test.test.ts against the debug build (four unrelated terminate-race tests in worker.test.ts time out on this machine's ASAN build with or without the new test; they pass on the release build and that file passed on the ASAN lane in CI for the earlier revision of this branch). Self-review surfaced the worker path and the stale docs; both are addressed above.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fcf0f3f5-c08e-4c13-b170-9e72ea4f8965

📥 Commits

Reviewing files that changed from the base of the PR and between 626034f and ad32558.

📒 Files selected for processing (6)

• src/bundler/defines.rs
• src/dotenv/env_loader.rs
• src/jsc/rare_data.rs
• test/bake/dev/bundle.test.ts
• test/bundler/bundler_env.test.ts
• test/js/web/workers/worker.test.ts

---

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

[robobun]

Status: reproduced on the released 1.4.0 binary (the inlined string comes back with the allocator's free-list word over its first bytes) and on the ASAN debug build (heap-use-after-free in bun_js_printer::best_quote_char_for_string, freed by Bun__setEnvValue -> Map::put), for the dev server, bun build --env=inline with a macro, Bun.build with a plugin, and a Worker that assigns the variable and then imports a module reading it. The four tests fail the same way without the src/ change and pass with it.

Ready for review. Reviews so far have no requested changes and no threads are open. On the latest build (https://buildkite.com/bun/bun/builds/93390, head ad32558) every lane that ran the changed files and the new tests passed; the red jobs are test/cli/install/migration/complex-workspace.test.ts (install failure on debian aarch64 / alpine x64), test/cli/install/bun-install.test.ts (the GitHub tarball URL tests, red on darwin 14 and retried on three Linux lanes), test/js/bun/test/parallel/test-docker-build-alpine.ts (x64 ASAN) and the x64-musl verify-baseline step failing outside the runner. None of them touch this change; the three tests have been reported separately. The two darwin 26 jobs on this build expired without ever getting an agent, and earlier builds had the same shape (require-cache.test.ts timing out on the ASAN lane, the same darwin 26 lanes stuck in infra retries). I am not going to keep re-running CI for those; this is waiting on a maintainer.

Related: #37780 (per-build loader copy for Bun.build; the second of the two to land rebases the shared bundler_env.test.ts hunk, and the Bun.build plugin test here can go at that point) and #34211 (stops workers inlining env, which also ends the per-spawn retention described in the body).

[claude]

LGTM — one-line UAF fix that copies env values into the define arena instead of aliasing the mutable env map.

What was reviewed:

• bump.alloc_slice_copy(value) matches the documented contract of E::EString::init (arena-owned data) and mirrors what DefineData::parse already does for --define values a few lines below.
• Tests cover all three affected paths (dev server rebuild, bun build macro, Bun.build plugin), assert the startup value is inlined and the mutated value is absent, and follow harness conventions (subprocess pipe draining, bunEnv spread, tempDir, Buffer.alloc over .repeat).
• Checked that the key side was already safe (built into a local key_buf and duped by get_or_put_value), so only the value needed copying.

Extended reasoning...

Overview

The PR fixes a heap-use-after-free in env inlining. env_string_store_put in src/bundler/defines.rs previously built an E::String whose bytes pointed directly into the dotenv Loader map. When JS assigns one of the proxy env vars (HTTPS_PROXY etc.) via Bun__setEnvValue, Map::put frees the old Box<[u8]>, leaving the define table pointing at freed memory. The fix is a single added line: let value: &[u8] = bump.alloc_slice_copy(value); — the value is now copied into the same arena that already owns the E::String node and the rest of the define table, so its lifetime matches the table's. The old comment claiming the env-map storage was long-lived is replaced with one explaining why it isn't.

Three tests are added covering each place the define table can outlive an env-map mutation: bake DevServer with [serve.static] env = "inline" and a runtime process.env.HTTPS_PROXY write followed by a rebuild; bun build --env=inline with a macro that assigns the variable; and Bun.build({ env: "inline" }) with a plugin onLoad that assigns it mid-build.

Security risks

None introduced. This removes a memory-safety bug (UAF reading freed heap into bundled output). The copy is into an arena the define table already lives in, so no new lifetime is created.

Level of scrutiny

Low-to-medium. The runtime change is one line in a helper with one caller (copy_env_for_define), and it brings the value handling in line with the documented contract of E::EString::init ("data is arena-owned … bulk-freed", src/ast/e.rs:1733) and with what DefineData::parse already does for user --define values in the same file. alloc_slice_copy is the existing arena primitive (src/bun_alloc/MimallocArena.rs:369). The only cost is one small copy per env var when inlining is on; the runtime VM and Bun.Transpiler don't reach this path.

Other factors

• The PR description includes an ASAN trace pinning the free to Map::put under Bun__setEnvValue and the read to the printer, and confirms all three tests fail on USE_SYSTEM_BUN=1 / unfixed ASAN and pass with the fix.
• Tests follow the repo's conventions: itBundled for the CLI case with onAfterBundle + run, a spawned subprocess for the API case with concurrent pipe draining and { ...bunEnv, HTTPS_PROXY: ... }, tempDir from harness, Buffer.alloc(n, fill).toString() instead of .repeat, and stdout asserted before exit code.
• I checked whether the key needed the same treatment: it does not — copy_env_for_define builds the key into a local Vec<u8> and get_or_put_value dupes it, so only the value was aliased.
• The relationship to #37780 (per-build loader clone for Bun.build) is noted in the description; this change is independently correct and covers the dev server / CLI paths that #37780 does not.

[github-actions]

This PR may be a duplicate of:

1. Bun.build: read env from a per-build copy instead of the VM's live loader #37780 - Fixes the same env-inline define aliasing bug in the Bun.build({ env: "inline" }) path (per-build Loader copy instead of an arena byte copy) and adds a near-identical plugin-reassigns-HTTPS_PROXY test to the same test/bundler/bundler_env.test.ts.

🤖 Generated with Claude Code

[robobun]

Not a duplicate of #37780, although the two overlap on one path. #37780 changes js_bundle_completion_task.rs and env_loader.rs only: it gives each Bun.build() its own copy of the loader, and its description notes that the byte copy in env_string_store_put is being done separately, which is this PR. The dev server (Framework::init_transpiler builds the define table from the VM's loader and keeps it for the life of the server) and bun build --env=inline (macros assign through the CLI transpiler's loader) never go through the completion task, so only this change fixes those two; they are what test/bake/dev/bundle.test.ts and the macro test in bundler_env.test.ts cover.

The only duplicated piece is the Bun.build plus plugin test, which both PRs add. Whichever of the two lands second gets rebased, and if #37780 goes in first I will drop that test from here since its test covers the same case.

[robobun]

^{Updated 3:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit ad32558 has 4 failures in Build #93390 (All Failures):

• test/cli/install/bun-install.test.ts - code 1 on 🍎 14 aarch64
• test/js/bun/test/parallel/test-docker-build-alpine.ts - code 1 on 🐧 13 x64-asan
• test/cli/install/migration/complex-workspace.test.ts - code 1: error: Failed to install on 🐧 13 aarch64
• test/cli/install/migration/complex-workspace.test.ts - code 1: error: Failed to install on 🐧 3.23 x64
• step failed outside runner - exit 22 on 🐧 x64-musl - verify-baseline

---

🧪   To try this PR locally:

bunx bun-pr 37784

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

bun-37784 --bun

[claude]

LGTM — one-line arena copy that fixes an ASAN-confirmed UAF, with regression tests for all three affected paths.

What was reviewed:

• env_string_store_put: bump.alloc_slice_copy(value) matches the documented contract of EString::init ("data is arena-owned") and the existing --define path at defines.rs:419.
• Checked copy_env_for_define — the key already goes through get_or_put_value from a reused stack buffer, so only the value aliased the env map; nothing else in the function borrows from env.map.
• Tests follow harness conventions (Buffer.alloc fill, tempDir, {...bunEnv}, concurrent pipe drain, stdout asserted before exitCode, .example hosts) and cover dev-server, CLI-macro, and Bun.build-plugin variants.
• Comment-cop threads on the long comment are resolved; d1e4a28 trimmed it to one line.

Extended reasoning...

Overview

The production change is a single added line in src/bundler/defines.rs: env_string_store_put now copies the env value into the define arena via bump.alloc_slice_copy(value) before wrapping it in E::EString. Previously the E::String node was arena-allocated but its data bytes aliased the dotenv Loader map entry, which Bun__setEnvValue -> Map::put can free (proxy variables are the writes that reach the native map). Three regression tests are added: a bake dev-server test with [serve.static] env = "inline", an itBundled CLI test where a macro reassigns HTTPS_PROXY mid-build, and a spawned Bun.build test where a plugin onLoad does the same.

Security risks

None introduced. The change strictly removes a use-after-free by owning bytes that were previously borrowed. No new inputs are accepted, no validation is loosened, and the copy goes into an arena that already owns the surrounding E::String node and the rest of the define table.

Level of scrutiny

Low-to-medium. The mechanism is a textbook borrowed-slice-outlives-its-backing-storage UAF, confirmed by the ASAN trace in the description (freed by ... Map::put / read by best_quote_char_for_string). The fix is at the layer that creates the aliasing reference, uses the same helper (alloc_slice_copy) the file already uses for the --define JSON path (line 419), and satisfies the documented contract of EString::init in src/ast/e.rs ("data is arena-owned … bulk-freed"). There is no alternative design to weigh — copying into the arena that owns the node is the obvious and only correct shape here. Cost is one small copy per env var when --env=inline/PREFIX_* is on; the disable/load_all_without_inlining early-return means bun run startup is untouched.

Other factors

• I checked whether any other slice in copy_env_for_define aliases the env map: the key is built into a local Vec and passed to get_or_put_value (which must already own it, since the buffer is reused each iteration), so only value needed the copy.
• Tests follow the repo conventions closely: Buffer.alloc(n, fill).toString() instead of .repeat, tempDir + using, {...bunEnv, HTTPS_PROXY: ...}, Promise.all([stdout, stderr, exited]), stdout/stderr asserted before exitCode, test.concurrent for the subprocess case, and .example hostnames (no network). The dev-server test awaits the actual observable condition (fetches the rebuilt bundle) rather than sleeping.
• The three comment-cop inline comments are resolved — the author trimmed the multi-line rationale to a single line in d1e4a28, which is what the diff shows.
• The known overlap with #37780 (per-build Loader copy for Bun.build) is acknowledged in the description and the timeline; the two are complementary and whichever lands second rebases the shared test. That coordination does not block this change.

[claude]

LGTM — one-line UAF fix that copies env values into the define arena instead of aliasing the env map's Box<[u8]> storage.

What was reviewed:

• env_string_store_put now bump.alloc_slice_copy(value) before building the E::EString; same pattern DefineData::parse already uses for --define values in the same file, and bump is the arena the define table itself lives in.
• The rare_data.rs / env_loader.rs edits are doc-comment corrections only (the old RefCountedEnvValue comment claiming the env map borrows its bytes was wrong — Map::put boxes its own copy).
• New tests cover all three paths (dev server, bun build macro, Bun.build plugin), follow harness conventions, and per the status comment fail with ASAN heap-use-after-free on the unfixed build.

Extended reasoning...

Overview

Fixes a heap-use-after-free in env inlining (--env=inline / Bun.build({ env }) / [serve.static] env = "inline"). env_string_store_put in src/bundler/defines.rs previously built E::EString defines whose bytes pointed directly into the dotenv Loader map's Box<[u8]> values. Assigning HTTP_PROXY/HTTPS_PROXY/NO_PROXY on process.env routes through Bun__setEnvValue → Map::put, which drops the old Box<[u8]>, leaving the define pointing at freed memory. The fix is a single bump.alloc_slice_copy(value) so the bytes live in the same arena as the define table.

The other two source edits are doc comments: a warning on Map::put / get_http_proxy that put frees the old value, and a correction to the RefCountedEnvValue doc (the env map keeps its own copy, it does not borrow the .bytes slice — verified against Map::put and ProxyEnvSlots::sync_into, both of which Box::from(value)).

Four new tests: dev-server rebuild after a runtime proxy write, bun build with a macro that reassigns the proxy, Bun.build with a plugin onLoad that reassigns it, and a defensive worker test.

Security risks

None introduced. This closes a memory-safety bug (UAF surfacing freed allocator bytes into bundler output). The fix is a copy into an arena the define already depends on; no new untrusted input handling.

Level of scrutiny

Memory-safety fix, so I looked at the ownership carefully. The functional change is one line and matches the established pattern in the same file (DefineData::parse does bump.alloc_slice_copy(value_str) for user --define values). bump is the arena the E::EString node and the whole Define table are already allocated from, so the copied bytes have exactly the table's lifetime — no new leak, no lifetime mismatch. The key side is unaffected: copy_env_for_define builds key_buf locally and get_or_put_value copies it into the store.

Other factors

• No CODEOWNERS on the touched files.
• The comment-cop inline threads (about comment length in defines.rs) are resolved; the current diff has a single-line comment.
• Tests follow harness conventions (tempDir, bunEnv spread, concurrent pipe drain via Promise.all, exitCode asserted last, Buffer.alloc(n, fill).toString() for large strings). Robobun's status comment confirms all three bundler tests fail with ASAN heap-use-after-free on the unfixed build and pass with the fix.
• The worker test uses toBeOneOf because whether the worker transpiler inlines process.env reads is orthogonal; a finder flagged it as possibly vacuous but the verifier ruled that out — it's a defensive check that the value is one of the two real strings and not freed-memory garbage.
• The overlap with #37780 is acknowledged in the description; the two are complementary and whichever lands second rebases bundler_env.test.ts.