Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4e3043b
process.env: replace plain object with Node-semantics exotic on POSIX
robobun Jul 26, 2026
cecf118
process.env (Windows proxy): throw on Symbol key/value, match set-tra…
robobun Jul 26, 2026
8023a21
env_var: drop unnecessary #[allow(dead_code)] on reset()
robobun Jul 26, 2026
c445be8
BunObject.rs: drop stale 'Returns the env_loader map entry count' doc…
robobun Jul 26, 2026
47ade95
address review: libcPathForDlopen for musl, skipIf(!cc), afterEach cl…
robobun Jul 26, 2026
fffc7ba
process-env-exotic.test: run freeze/seal probe in subprocess so a fai…
robobun Jul 26, 2026
1a17569
test/preload: skip CI when copying bunEnv into process.env
robobun Jul 26, 2026
808e952
address review: seqlock for string env_var cache, lock map.put/remove…
robobun Jul 26, 2026
210f25b
address review: environ RwLock around setenv/getenv_z, $-prefix read …
robobun Jul 26, 2026
1b2e74d
Bun__ProcessEnv__put: self-truncate NUL + filter empty/= keys; seqloc…
robobun Jul 26, 2026
5143c3e
env_var: store owned copy after runtime setenv (musl frees previous v…
robobun Jul 26, 2026
246257b
env_var::set_owned: lsan-ignore the leaked Box
robobun Jul 26, 2026
9e326a3
Bun__ProcessEnv__delete: clear matching proxy_env_storage slot; fix s…
robobun Jul 26, 2026
7425d37
env_var string cache: always store a leaked owned copy; drop dead res…
robobun Jul 26, 2026
b776d54
getenv_z: copy under ENVIRON_LOCK; worker loader skips load_process r…
robobun Jul 26, 2026
6130d1f
env_var: re-check get_cached() after getenv_z in get_force_reload/pla…
robobun Jul 26, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 115 additions & 27 deletions src/bun_core/env_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
// `New`/`PlatformSpecificNew` are `macro_rules!` that emit a module per env var; the macros
// must be defined (or `#[macro_use]`d) before the declarations.

use core::sync::atomic::{AtomicPtr, AtomicU8, AtomicU64, AtomicUsize, Ordering};
use core::sync::atomic::{AtomicPtr, AtomicU8, AtomicU32, AtomicU64, AtomicUsize, Ordering};

// MOVE_DOWN: bun_core::ZStr → bun_core (move-in pass).
use crate::ZStr;
Expand Down Expand Up @@ -309,7 +309,13 @@ pub(crate) mod kind {

// A single Cache struct; per-var uniqueness comes from each var owning its own
// `static CACHE: Cache`.
//
// `(ptr, len)` is a split-word cache of a `getenv` result; `seq` is a
// seqlock so a concurrent reader never pairs a stale `len` with a new
// `ptr` after `reset()`/reload stores a different slice (the
// `process.env` write path does `setenv()` then `reset()`).
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) struct Cache {
seq: AtomicU32,
ptr_value: AtomicPtr<u8>,
len_value: AtomicUsize,
}
Expand All @@ -325,50 +331,84 @@ pub(crate) mod kind {
impl Cache {
pub(crate) const fn new() -> Self {
Self {
seq: AtomicU32::new(0),
ptr_value: AtomicPtr::new(NOT_LOADED_PTR),
len_value: AtomicUsize::new(NOT_LOADED_LEN),
}
}

pub(crate) fn get_cached(&self) -> Output {
let len = self.len_value.load(Ordering::Acquire);

if len == NOT_LOADED_LEN {
return CacheOutput::Unknown;
loop {
let s1 = self.seq.load(Ordering::Acquire);
if s1 & 1 != 0 {
core::hint::spin_loop();
continue;
}
let len = self.len_value.load(Ordering::Acquire);
let ptr = self.ptr_value.load(Ordering::Acquire);
if self.seq.load(Ordering::Acquire) != s1 {
core::hint::spin_loop();
continue;
}
if len == NOT_LOADED_LEN {
return CacheOutput::Unknown;
}
if len == NOT_SET_LEN {
return CacheOutput::NotSet;
}
// SAFETY: (ptr, len) were stored together under the seqlock from a
// valid &'static [u8] returned by getenv_z (which borrows from libc's
// environ; the slot for a key that was setenv()'d stays valid because
// glibc/musl never free the previous value).
return CacheOutput::Value(unsafe { core::slice::from_raw_parts(ptr, len) });
}
Comment thread
robobun marked this conversation as resolved.
Outdated
}

if len == NOT_SET_LEN {
return CacheOutput::NotSet;
#[inline]
fn write_under_seqlock(&self, f: impl FnOnce()) {
// Odd seq = write in progress; even = stable. CAS even→odd so
// concurrent writers exclude one another (fetch_add would let a
// second writer bump odd→even and run f() alongside the first).
Comment thread
robobun marked this conversation as resolved.
loop {
let s = self.seq.load(Ordering::Relaxed);
if s & 1 == 0
&& self
.seq
.compare_exchange_weak(s, s + 1, Ordering::AcqRel, Ordering::Relaxed)
.is_ok()
{
break;
}
core::hint::spin_loop();
}

let ptr = self.ptr_value.load(Ordering::Relaxed);

// SAFETY: ptr/len were stored together in deser_and_invalidate from a valid
// &'static [u8] returned by getenv_z (envp memory lives for process lifetime).
CacheOutput::Value(unsafe { core::slice::from_raw_parts(ptr, len) })
f();
self.seq.fetch_add(1, Ordering::Release);
}

#[inline]
pub(crate) fn deser_and_invalidate(
&self,
raw_env: Option<&'static [u8]>,
) -> Option<ValueType> {
// The implementation is racy and allows two threads to both set the value at
// the same time, as long as the value they are setting is the same. This is
// difficult to write an assertion for since it requires the DEV path take a
// .swap() path rather than a plain .store().

if let Some(ev) = raw_env {
self.ptr_value
.store(ev.as_ptr().cast_mut(), Ordering::Relaxed);
self.len_value.store(ev.len(), Ordering::Release);
} else {
self.ptr_value.store(NOT_SET_PTR, Ordering::Relaxed);
self.len_value.store(NOT_SET_LEN, Ordering::Release);
}

self.write_under_seqlock(|| {
if let Some(ev) = raw_env {
self.ptr_value
.store(ev.as_ptr().cast_mut(), Ordering::Release);
self.len_value.store(ev.len(), Ordering::Release);
} else {
self.ptr_value.store(NOT_SET_PTR, Ordering::Release);
self.len_value.store(NOT_SET_LEN, Ordering::Release);
}
});
raw_env
}

#[inline]
pub(crate) fn reset(&self) {
self.write_under_seqlock(|| {
self.len_value.store(NOT_LOADED_LEN, Ordering::Release);
});
}
}
}

Expand Down Expand Up @@ -448,6 +488,12 @@ pub(crate) mod kind {
);
Some(string_is_truthy)
}

#[inline]
pub(crate) fn reset(&self) {
self.value
.store(StoredType::Unknown as u8, Ordering::Relaxed);
}
}
}

Expand Down Expand Up @@ -619,6 +665,11 @@ pub(crate) mod kind {
}
}
}

#[inline]
pub(crate) fn reset(&self) {
self.value.store(UNKNOWN_SENTINEL, Ordering::Relaxed);
}
}
}
}
Expand Down Expand Up @@ -763,6 +814,12 @@ macro_rules! platform_specific_new {
}
}

/// Drop the cached value so the next `get()` re-reads libc; used by
/// the `process.env` write path after `setenv()`.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn reset() {
CACHE.reset();
}
Comment thread
robobun marked this conversation as resolved.

/// Retrieve the value of the environment variable, reloading it from the environment.
/// Fails if the current platform is unsupported.
fn get_force_reload() -> Option<K::ValueType> {
Expand Down Expand Up @@ -951,3 +1008,34 @@ macro_rules! new_feature_flag {
};
}
pub(crate) use new_feature_flag;

/// Drop the cached value for the typed accessor whose key matches `name`, so
/// the next `get()` re-reads libc. `process.env` writes call `setenv()` and
/// then this — otherwise `os.homedir()` / `os.tmpdir()` would keep returning
/// the value cached on first read.
///
/// Only the string-kind accessors that are read at runtime (after VM startup)
/// are listed; the `BUN_*` flags are read once on boot and intentionally left
/// cached.
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
pub fn invalidate_for_setenv(name: &[u8]) {
macro_rules! try_var {
($v:ident) => {
if let Some(k) = $v::platform_key() {
if crate::strings::eql(k.as_bytes(), name) {
$v::reset();
return;
}
}
};
}
try_var!(HOME);
try_var!(PATH);
try_var!(USER);
try_var!(TMPDIR);
try_var!(TEMP);
try_var!(TMP);
try_var!(SHELL);
try_var!(XDG_CACHE_HOME);
try_var!(XDG_CONFIG_HOME);
try_var!(XDG_DATA_HOME);
}
22 changes: 20 additions & 2 deletions src/bun_core/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,20 @@
}
}

/// Serialises Bun's own `libc::getenv` / `environ` readers against the
/// `process.env` write path's `libc::setenv`/`unsetenv`. `getenv()` does not
/// take glibc's internal envlock (it is async-signal-safe), so a concurrent
/// `setenv()` that `realloc`s `__environ` while the transpiler pool or a
/// worker is inside `getenv_z()` would be a UAF inside libc. Native addons'
/// direct `getenv()` calls are not covered (same limitation Node carries).
Comment thread
robobun marked this conversation as resolved.
static ENVIRON_LOCK: RwLock<()> = RwLock::new(());

/// Hold this write guard around `libc::setenv`/`unsetenv` so [`getenv_z`] and
/// [`getenv_z_any_case`] on other threads don't walk `__environ` mid-realloc.
Comment thread
robobun marked this conversation as resolved.
pub fn environ_write_lock() -> RwLockWriteGuard<'static, ()> {
ENVIRON_LOCK.write()
}

/// `bun.getenvZ` — read an environment variable. Returns the value as borrowed
/// process-static bytes (env block lives for the process). On POSIX wraps
/// `libc::getenv`; on Windows scans `environ` case-insensitively.
Expand All @@ -334,14 +348,17 @@
return None;
}
#[cfg(unix)]
unsafe {

Check failure on line 351 in src/bun_core/util.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

unsafe block missing a safety comment
let _g = ENVIRON_LOCK.read();
// SAFETY: key is NUL-terminated by ZStr invariant; getenv reads until NUL.
let p = libc::getenv(key.as_ptr());
if p.is_null() {
return None;
}
// SAFETY: getenv returns a pointer into the process env block, valid for
// process lifetime (modulo setenv races).
// SAFETY: getenv returns a pointer into the process env block; the read
// lock serialises against Bun's own setenv path. glibc/musl never free
// a previous value string on overwrite, so the returned slice stays
// valid after the guard drops.
let len = libc::strlen(p);
return Some(core::slice::from_raw_parts(p.cast::<u8>(), len));
}
Expand Down Expand Up @@ -378,7 +395,8 @@
/// CI-detection vars where casing varies across providers).
pub fn getenv_z_any_case(key: &ZStr) -> Option<&'static [u8]> {
#[cfg(unix)]
unsafe {

Check failure on line 398 in src/bun_core/util.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

unsafe block missing a safety comment
let _g = ENVIRON_LOCK.read();
// SAFETY: `environ` is the C env block; entries are NUL-terminated `KEY=VALUE`.
let mut p = c_environ();
while !(*p).is_null() {
Expand Down
15 changes: 15 additions & 0 deletions src/collections/array_hash_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1389,6 +1389,21 @@ impl<V, C: ArrayHashContext<[u8]> + Default, A: MapAllocator> StringArrayHashMap
true
}

/// O(n); preserves insertion order of remaining entries.
pub fn ordered_remove(&mut self, key: &[u8]) -> bool {
let Some(i) = self.find(key) else {
return false;
};
self.inner.keys.remove(i);
self.inner.hashes.remove(i);
self.inner.values.remove(i);
self.inner.drop_index();
if self.inner.keys.len() > INDEX_THRESHOLD {
self.inner.rebuild_index();
}
true
}

/// Removes the entry (swapping the last element into its slot) and
/// returns the owned key/value pair.
pub fn fetch_swap_remove(&mut self, key: &[u8]) -> Option<KV<Box<[u8], A>, V>> {
Expand Down
31 changes: 21 additions & 10 deletions src/dotenv/env_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,19 +583,30 @@ impl Loader {

let environ: &[*const c_char] = bun_sys::environ();
self.map.map.ensure_total_capacity(environ.len())?;
// The kernel accepts duplicate KEY= entries in environ. libc getenv()
// and Node both return the FIRST occurrence, so keep the first and
// skip later duplicates. Indices below `prior_count` were seeded
// before this scan (e.g. `bun test` pre-seeding NODE_ENV) and are
// still overwritten so the process environment retains priority.
Comment thread
robobun marked this conversation as resolved.
let prior_count = self.map.map.count();
for &_env in environ {
// SAFETY: environ entries are NUL-terminated C strings from the OS
let env = unsafe { bun_core::ffi::cstr(_env) }.to_bytes();
if let Some(i) = strings::index_of_char(env, b'=') {
let key = &env[..i as usize];
let value = &env[i as usize + 1..];
if !key.is_empty() {
self.map.put(key, value)?;
}
} else {
if !env.is_empty() {
self.map.put(env, b"")?;
}
// An entry without '=' is malformed per POSIX ("name=value"); libc
// getenv() and Node ignore it.
Comment thread
robobun marked this conversation as resolved.
let Some(i) = strings::index_of_char(env, b'=') else {
continue;
};
let key = &env[..i as usize];
if key.is_empty() {
continue;
}
let value = &env[i as usize + 1..];
let gop = self.map.get_or_put_without_value(key)?;
if !gop.found_existing || gop.index < prior_count {
*gop.value_ptr = HashTableValue {
value: Box::from(value),
};
}
Comment thread
robobun marked this conversation as resolved.
}
Comment thread
robobun marked this conversation as resolved.
self.did_load_process = true;
Expand Down
57 changes: 50 additions & 7 deletions src/js/builtins/ProcessObjectInternals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,9 +490,18 @@ export function windowsEnv(
return internalEnv[p];
},
set(_, p, value) {
const k = String(p).toUpperCase();
$assert(typeof p === "string"); // proxy is only string and symbol. the symbol would have thrown by now
// Node's env setter does `ToString` on key and value, which throws on a
// Symbol; JS `String()` special-cases Symbols, so reject explicitly.
Comment thread
robobun marked this conversation as resolved.
if (typeof p === "symbol" || typeof value === "symbol") {
throw new TypeError("Cannot convert a Symbol value to a string");
}
const k = (p as string).toUpperCase();
value = String(value); // If toString() throws, we want to avoid it existing in the envMapList
// Node silently ignores empty names and names containing '=' (the
// assignment succeeds but nothing is stored, matching RealEnvStore::Set).
Comment thread
robobun marked this conversation as resolved.
if (k === "" || k.indexOf("=") !== -1) {
return true;
}
// Track the key for enumeration if it isn't already there. Don't gate on
// `k in internalEnv`: the proxy-related env-var accessors (HTTP_PROXY,
// HTTPS_PROXY, NO_PROXY and lowercase variants) always exist on
Expand Down Expand Up @@ -528,13 +537,47 @@ export function windowsEnv(
return typeof p !== "symbol" ? delete internalEnv[k] : false;
},
defineProperty(_, p, attributes) {
const k = String(p).toUpperCase();
$assert(typeof p === "string"); // proxy is only string and symbol. the symbol would have thrown by now
if (!(k in internalEnv) && !envMapList.includes(p)) {
if (typeof p === "symbol") {
throw new TypeError("Cannot convert a Symbol value to a string");
}
Comment thread
robobun marked this conversation as resolved.
const k = (p as string).toUpperCase();
// Node only accepts a fully-permissive data descriptor and routes it
// through the env setter (RealEnvStore::PropertyDefinerCallback).
Comment thread
robobun marked this conversation as resolved.
if ("get" in attributes || "set" in attributes) {
throw $ERR_INVALID_OBJECT_DEFINE_PROPERTY(
"'process.env' does not accept an accessor(getter/setter) descriptor",
);
}
if (
!("value" in attributes) ||
attributes.configurable !== true ||
attributes.writable !== true ||
attributes.enumerable !== true
) {
throw $ERR_INVALID_OBJECT_DEFINE_PROPERTY(
"'process.env' only accepts a configurable, writable, and enumerable data descriptor",
);
}
if (typeof attributes.value === "symbol") {
throw new TypeError("Cannot convert a Symbol value to a string");
}
const value = String(attributes.value);
Comment thread
robobun marked this conversation as resolved.
if (k === "" || k.indexOf("=") !== -1) {
return true;
}
if (!envMapList.includes(p) && !envMapList.some(x => x.toUpperCase() === k)) {
envMapList.push(p);
}
editWindowsEnvVar(k, internalEnv[k]);
return $Object.$defineProperty(internalEnv, k, attributes);
editWindowsEnvVar(k, value);
internalEnv[k] = value;
return true;
},
preventExtensions() {
// Node: Object.freeze/seal/preventExtensions on process.env throw.
return false;
},
isExtensible() {
return true;
},
getOwnPropertyDescriptor(target, p) {
if (typeof p === "string") {
Expand Down
Loading
Loading