diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index 579cda9d82fb..bb2319cf8627 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -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; @@ -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 an owned leaked copy of the + // env value; `seq` is a seqlock so a concurrent reader never pairs a + // stale `len` with a new `ptr` when the cached slice changes (initial + // load racing a `process.env` write's `set_owned()`). pub(crate) struct Cache { + seq: AtomicU32, ptr_value: AtomicPtr, len_value: AtomicUsize, } @@ -325,27 +331,59 @@ 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 leaked Box<[u8]> (getenv_z and set_owned both + // leak before passing in). No pointer into libc environ is + // ever cached, so musl freeing a previous setenv-allocated + // string cannot dangle it. + return CacheOutput::Value(unsafe { core::slice::from_raw_parts(ptr, len) }); } + } - 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). + 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] @@ -353,20 +391,23 @@ pub(crate) mod kind { &self, raw_env: Option<&'static [u8]>, ) -> Option { - // 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); - } - + // The previous cached slice is deliberately never freed: a + // concurrent `get_cached()` may have already returned it past + // the seqlock (e.g. into `BunString::init`), so reclaiming it + // here would be UAF. Callers pass a leaked Box (`getenv_z` + // leaks under its read lock; `set_owned` leaks the + // process.env write's value), so the stored `(ptr, len)` is + // always Bun-owned and `&'static`. + 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 } } @@ -689,6 +730,12 @@ macro_rules! platform_specific_new { $crate::hint::cold(); let env_var = $crate::getenv_z(k); + // A concurrent `set_owned()` may have filled the cache + // while getenv_z ran; don't overwrite it with the + // stale pre-write value. + if !matches!(CACHE.get_cached(), CacheOutput::Unknown) { + return platform_get(); + } let maybe_reloaded = CACHE.deser_and_invalidate(env_var); if let Some(v) = maybe_reloaded { @@ -763,11 +810,29 @@ macro_rules! platform_specific_new { } } + /// Replace the cached value from the `process.env` write path. + /// Leaks a copy so the cache never holds a pointer into `environ` + /// (musl frees the previous setenv-allocated string on overwrite); + /// the previous cached slice is never freed because a concurrent + /// `get()` caller may already hold it past the seqlock. Bounded to + /// one small leak per runtime write to one of ten well-known keys. + pub fn set_owned(value: Option<&[u8]>) { + let leaked: Option<&'static [u8]> = value.map(|v| { + let s: &'static [u8] = Box::leak(Box::<[u8]>::from(v)); + $crate::asan::ignore_object(s.as_ptr()); + s + }); + CACHE.deser_and_invalidate(leaked); + } + /// Retrieve the value of the environment variable, reloading it from the environment. /// Fails if the current platform is unsupported. fn get_force_reload() -> Option { assert_platform_supported(); let env_var = $crate::getenv_z(key()); + if !matches!(CACHE.get_cached(), CacheOutput::Unknown) { + return get(); + } let maybe_reloaded = CACHE.deser_and_invalidate(env_var); if let Some(v) = maybe_reloaded { @@ -951,3 +1016,35 @@ macro_rules! new_feature_flag { }; } pub(crate) use new_feature_flag; + +/// Replace the cached value for the typed accessor whose key matches `name` +/// with an owned copy of `value` (or mark it unset). `process.env` writes call +/// `setenv()` and then this so `os.homedir()` etc. observe the change; the +/// cache stores a leaked owned slice rather than a pointer into `environ`, +/// because musl frees the previous setenv-allocated string on overwrite. +/// +/// 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. +pub fn invalidate_for_setenv(name: &[u8], value: Option<&[u8]>) { + macro_rules! try_var { + ($v:ident) => { + if let Some(k) = $v::platform_key() { + if crate::strings::eql(k.as_bytes(), name) { + $v::set_owned(value); + 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); +} diff --git a/src/bun_core/lib.rs b/src/bun_core/lib.rs index 5358da22cb68..87412072e630 100644 --- a/src/bun_core/lib.rs +++ b/src/bun_core/lib.rs @@ -2813,6 +2813,7 @@ pub mod asan { safe fn __asan_describe_address(ptr: *const c_void); safe fn __lsan_register_root_region(ptr: *const c_void, size: usize); safe fn __lsan_unregister_root_region(ptr: *const c_void, size: usize); + safe fn __lsan_ignore_object(ptr: *const c_void); } #[inline] @@ -2850,6 +2851,16 @@ pub mod asan { #[cfg(not(bun_asan))] let _ = (ptr, size); } + /// Tell LSAN the allocation containing `ptr` is an intentional + /// process-lifetime leak (e.g. a `Box::leak`'d static cache entry whose + /// pointer may later be overwritten and so become unreachable to LSAN). + #[inline] + pub fn ignore_object(ptr: *const T) { + #[cfg(bun_asan)] + __lsan_ignore_object(ptr.cast()); + #[cfg(not(bun_asan))] + let _ = ptr; + } /// Undo a prior `register_root_region(ptr, size)` with identical arguments. #[inline] pub fn unregister_root_region(ptr: *const c_void, size: usize) { diff --git a/src/bun_core/util.rs b/src/bun_core/util.rs index 8c9d1381f526..fbb0fef840d2 100644 --- a/src/bun_core/util.rs +++ b/src/bun_core/util.rs @@ -324,9 +324,35 @@ impl core::ops::Deref for ZStr { } } -/// `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. +/// 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). +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. +pub fn environ_write_lock() -> RwLockWriteGuard<'static, ()> { + ENVIRON_LOCK.write() +} + +// Leak an owned copy of `src` as `&'static [u8]`, LSAN-ignored. Used so the +// returned slice outlives any later `setenv()` that (on musl) may free the +// source environ string. +#[cfg(unix)] +#[inline] +fn leak_static_copy(src: &[u8]) -> &'static [u8] { + let s: &'static [u8] = Box::leak(Box::<[u8]>::from(src)); + crate::asan::ignore_object(s.as_ptr()); + s +} + +/// `bun.getenvZ` — read an environment variable. On POSIX wraps +/// `libc::getenv`; on Windows scans `environ` case-insensitively. Returns an +/// owned leaked copy so the `&'static [u8]` is valid regardless of a later +/// `setenv()` (musl frees the previous setenv-allocated string on overwrite). pub fn getenv_z(key: &ZStr) -> Option<&'static [u8]> { #[cfg(not(any(unix, windows)))] { @@ -335,15 +361,19 @@ pub fn getenv_z(key: &ZStr) -> Option<&'static [u8]> { } #[cfg(unix)] unsafe { + 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 while we copy. let len = libc::strlen(p); - return Some(core::slice::from_raw_parts(p.cast::(), len)); + return Some(leak_static_copy(core::slice::from_raw_parts( + p.cast::(), + len, + ))); } #[cfg(windows)] { @@ -379,6 +409,7 @@ pub fn c_environ() -> *const *const core::ffi::c_char { pub fn getenv_z_any_case(key: &ZStr) -> Option<&'static [u8]> { #[cfg(unix)] unsafe { + 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() { @@ -388,7 +419,7 @@ pub fn getenv_z_any_case(key: &ZStr) -> Option<&'static [u8]> { &line[..key_end], key.as_bytes(), ) { - return Some(&line[(key_end + 1).min(line.len())..]); + return Some(leak_static_copy(&line[(key_end + 1).min(line.len())..])); } p = p.add(1); } diff --git a/src/collections/array_hash_map.rs b/src/collections/array_hash_map.rs index c590fd177975..da169ebfbe6a 100644 --- a/src/collections/array_hash_map.rs +++ b/src/collections/array_hash_map.rs @@ -1389,6 +1389,21 @@ impl + 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, V>> { diff --git a/src/dotenv/env_loader.rs b/src/dotenv/env_loader.rs index b741cb9f2b47..33bf176cae9f 100644 --- a/src/dotenv/env_loader.rs +++ b/src/dotenv/env_loader.rs @@ -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. + 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. + 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), + }; } } self.did_load_process = true; diff --git a/src/js/builtins/ProcessObjectInternals.ts b/src/js/builtins/ProcessObjectInternals.ts index 37681a67f7c6..7887266a131d 100644 --- a/src/js/builtins/ProcessObjectInternals.ts +++ b/src/js/builtins/ProcessObjectInternals.ts @@ -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. + 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). + 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 @@ -519,22 +528,57 @@ export function windowsEnv( return p in internalEnv; }, deleteProperty(_, p) { - const k = String(p).toUpperCase(); + if (typeof p === "symbol") return true; + const k = (p as string).toUpperCase(); const i = envMapList.findIndex(x => x.toUpperCase() === k); if (i !== -1) { envMapList.splice(i, 1); } editWindowsEnvVar(k, null); - return typeof p !== "symbol" ? delete internalEnv[k] : false; + return delete internalEnv[k]; }, 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"); + } + const k = (p as string).toUpperCase(); + // Node only accepts a fully-permissive data descriptor and routes it + // through the env setter (RealEnvStore::PropertyDefinerCallback). + 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); + 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") { diff --git a/src/jsc/bindings/JSEnvironmentVariableMap.cpp b/src/jsc/bindings/JSEnvironmentVariableMap.cpp index fc29c4c824fa..7344601e8703 100644 --- a/src/jsc/bindings/JSEnvironmentVariableMap.cpp +++ b/src/jsc/bindings/JSEnvironmentVariableMap.cpp @@ -19,6 +19,7 @@ #include #include #include "BunProcess.h" +#include "ErrorCode.h" #include "ScriptExecutionContext.h" #include "SharedEnvStore.h" #include "wtf/NeverDestroyed.h" @@ -32,6 +33,8 @@ extern "C" size_t Bun__getEnvKey(void* list, size_t index, unsigned char** out); extern "C" bool Bun__getEnvValue(JSGlobalObject* globalObject, const ZigString* name, ZigString* value); extern "C" bool Bun__getEnvValueBunString(JSGlobalObject* globalObject, const BunString* name, BunString* value); extern "C" void Bun__setEnvValue(JSGlobalObject* globalObject, const BunString* name, const BunString* value); +extern "C" void Bun__ProcessEnv__put(JSGlobalObject* globalObject, const BunString* name, const BunString* value); +extern "C" void Bun__ProcessEnv__delete(JSGlobalObject* globalObject, const BunString* name); namespace Bun { @@ -343,24 +346,33 @@ JSC_DEFINE_HOST_FUNCTION(jsEditWindowsEnvVar, (JSGlobalObject * global, JSC::Cal } #endif -// Founding a SHARE_ENV tree swaps main's process.env off the windowsEnv Proxy that -// called SetEnvironmentVariableW, so every mutation of a main-rooted shared store has -// to re-apply that write-through. Gated on the *store*, not the writing thread: node -// roots a main-founded tree at its RealEnvStore, so a worker writing through that tree -// reaches the OS env too. `value == nullptr` deletes. -static ALWAYS_INLINE void syncWindowsEnv(SharedEnvStore* store, const String& key, const String* value) +// Founding a SHARE_ENV tree swaps main's process.env off the object whose +// put()/delete() wrote through to the OS environment (the Windows Proxy's +// SetEnvironmentVariableW or the POSIX JSProcessEnvMap's setenv/unsetenv), so +// every mutation of a main-rooted shared store has to re-apply that write- +// through. Gated on the *store*, not the writing thread. On Windows a worker +// writing through a main-rooted tree reaches the OS env (Node parity); on +// POSIX Bun__ProcessEnv__put still gates setenv on vm.is_main_thread(), so +// only main-thread writes reach environ and a worker's write lands only in +// the shared store. `value == nullptr` deletes. +static ALWAYS_INLINE void syncOSEnv(JSGlobalObject* globalObject, SharedEnvStore* store, const String& key, const String* value) { -#if OS(WINDOWS) if (!store || !store->isMainRooted()) return; +#if OS(WINDOWS) + UNUSED_PARAM(globalObject); if (value) Bun__Process__editWindowsEnvVar(Bun::toString(key), Bun::toString(*value)); else Bun__Process__editWindowsEnvVar(Bun::toString(key), { .tag = BunStringTag::Dead }); #else - UNUSED_PARAM(store); - UNUSED_PARAM(key); - UNUSED_PARAM(value); + BunString name = Bun::toString(key); + if (value) { + BunString val = Bun::toString(*value); + Bun__ProcessEnv__put(globalObject, &name, &val); + } else { + Bun__ProcessEnv__delete(globalObject, &name); + } #endif } @@ -565,7 +577,7 @@ bool JSSharedEnvMap::put(JSCell* cell, JSGlobalObject* globalObject, PropertyNam String keyStr = String(uid); applySharedEnvSideEffects(globalObject, keyStr, stringValue); - syncWindowsEnv(store, keyStr, &stringValue); + syncOSEnv(globalObject, store, keyStr, &stringValue); store->set(keyStr, stringValue); return true; } @@ -583,7 +595,7 @@ bool JSSharedEnvMap::deleteProperty(JSCell* cell, JSGlobalObject* globalObject, return Base::deleteProperty(cell, globalObject, propertyName, slot); } - syncWindowsEnv(store, String(uid), nullptr); + syncOSEnv(globalObject, store, String(uid), nullptr); store->remove(String(uid)); // Also drop any own property the Base fallback installed (accessor descriptors). return Base::deleteProperty(cell, globalObject, propertyName, slot); @@ -615,7 +627,7 @@ bool JSSharedEnvMap::defineOwnProperty(JSObject* object, JSGlobalObject* globalO if (auto* store = sharedEnvStoreFor(object)) { String existing = store->get(String(uid)); if (!existing.isNull()) { - syncWindowsEnv(store, String(uid), nullptr); + syncOSEnv(globalObject, store, String(uid), nullptr); store->remove(String(uid)); object->putDirect(vm, propertyName, jsString(vm, existing), 0); } @@ -635,7 +647,7 @@ bool JSSharedEnvMap::defineOwnProperty(JSObject* object, JSGlobalObject* globalO String keyStr = String(uid); applySharedEnvSideEffects(globalObject, keyStr, stringValue); - syncWindowsEnv(store, keyStr, &stringValue); + syncOSEnv(globalObject, store, keyStr, &stringValue); store->set(keyStr, stringValue); return true; } @@ -664,7 +676,7 @@ bool JSSharedEnvMap::deletePropertyByIndex(JSCell* cell, JSGlobalObject* globalO } String keyStr = String::number(index); - syncWindowsEnv(store, keyStr, nullptr); + syncOSEnv(globalObject, store, keyStr, nullptr); store->remove(keyStr); return Base::deletePropertyByIndex(cell, globalObject, index); } @@ -676,6 +688,237 @@ JSValue createSharedEnvironmentVariablesMap(Zig::GlobalObject* globalObject) return JSSharedEnvMap::create(vm, structure); } +#if !OS(WINDOWS) +// ============================================================================ +// POSIX process.env: exotic object backed by the env_loader map. +// +// Node's process.env is an exotic object (node_env_var.cc RealEnvStore): every +// set coerces to string, rejects symbol keys/values, silently drops `=`/empty +// keys, truncates at NUL, rejects accessor/non-permissive defineProperty, and +// writes through to setenv/unsetenv. +// +// On Windows process.env is a Proxy (ProcessObjectInternals.ts) that applies +// the same coercion/validation in JS and writes through to +// SetEnvironmentVariableW; this class is POSIX-only. + +// setenv/unsetenv take C strings, so Node truncates key and value at the first +// NUL (node_env_var.cc). Keep the same shape here so a JS write and the +// subsequent readback agree. +static ALWAYS_INLINE String truncateAtNUL(const String& s) +{ + size_t nul = s.find('\0'); + return nul == notFound ? s : s.substring(0, nul); +} + +class JSProcessEnvMap final : public JSC::JSNonFinalObject { +public: + using Base = JSC::JSNonFinalObject; + + static constexpr unsigned StructureFlags = Base::StructureFlags + | JSC::OverridesGetOwnPropertySlot + | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero + | JSC::OverridesPut + | JSC::OverridesGetOwnPropertyNames + | JSC::GetOwnPropertySlotMayBeWrongAboutDontEnum + | JSC::ProhibitsPropertyCaching; + + template + static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) + { + STATIC_ASSERT_ISO_SUBSPACE_SHARABLE(JSProcessEnvMap, Base); + return &vm.plainObjectSpace(); + } + + DECLARE_INFO; + + static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype) + { + return JSC::Structure::create(vm, globalObject, prototype, JSC::TypeInfo(JSC::ObjectType, StructureFlags), info()); + } + + static JSProcessEnvMap* create(JSC::VM& vm, JSC::Structure* structure) + { + JSProcessEnvMap* ptr = new (NotNull, JSC::allocateCell(vm)) JSProcessEnvMap(vm, structure); + ptr->finishCreation(vm); + return ptr; + } + + static bool getOwnPropertySlot(JSObject*, JSGlobalObject*, JSC::PropertyName, JSC::PropertySlot&); + static bool put(JSCell*, JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&); + static bool deleteProperty(JSCell*, JSGlobalObject*, JSC::PropertyName, JSC::DeletePropertySlot&); + static bool getOwnPropertySlotByIndex(JSObject*, JSGlobalObject*, unsigned, JSC::PropertySlot&); + static bool putByIndex(JSCell*, JSGlobalObject*, unsigned, JSC::JSValue, bool shouldThrow); + static bool deletePropertyByIndex(JSCell*, JSGlobalObject*, unsigned); + static void getOwnPropertyNames(JSObject*, JSGlobalObject*, JSC::PropertyNameArrayBuilder&, JSC::DontEnumPropertiesMode); + static bool defineOwnProperty(JSObject*, JSGlobalObject*, JSC::PropertyName, const JSC::PropertyDescriptor&, bool shouldThrow); + static bool preventExtensions(JSObject*, JSGlobalObject*); + +private: + JSProcessEnvMap(JSC::VM& vm, JSC::Structure* structure) + : Base(vm, structure) + { + } + + void finishCreation(JSC::VM& vm) + { + Base::finishCreation(vm); + } +}; + +const JSC::ClassInfo JSProcessEnvMap::s_info = { "ProcessEnv"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSProcessEnvMap) }; + +bool JSProcessEnvMap::getOwnPropertySlot(JSObject* object, JSGlobalObject* globalObject, PropertyName propertyName, PropertySlot& slot) +{ + VM& vm = JSC::getVM(globalObject); + auto* uid = propertyName.uid(); + if (propertyName.isSymbol() || !uid || uid->isEmpty()) + return Base::getOwnPropertySlot(object, globalObject, propertyName, slot); + + String key = truncateAtNUL(String(uid)); + if (key.isEmpty()) + return Base::getOwnPropertySlot(object, globalObject, propertyName, slot); + + BunString name = Bun::toString(key); + BunString value = { BunStringTag::Dead }; + if (!Bun__getEnvValueBunString(globalObject, &name, &value)) + return Base::getOwnPropertySlot(object, globalObject, propertyName, slot); + + slot.setValue(object, 0, jsString(vm, value.toWTFString())); + return true; +} + +bool JSProcessEnvMap::put(JSCell* cell, JSGlobalObject* globalObject, PropertyName propertyName, JSValue value, PutPropertySlot& slot) +{ + VM& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + auto* uid = propertyName.uid(); + if (propertyName.isSymbol() || !uid) { + // Node: symbol key on process.env throws TypeError (V8's named- + // interceptor stringifies the property). `uid` is null for private + // symbols, which Node has no equivalent of; fall through for those. + if (propertyName.isSymbol() && uid) { + throwTypeError(globalObject, scope, "Cannot convert a Symbol value to a string"_s); + return false; + } + RELEASE_AND_RETURN(scope, Base::put(cell, globalObject, propertyName, value, slot)); + } + + if (value.isSymbol()) { + throwTypeError(globalObject, scope, "Cannot convert a Symbol value to a string"_s); + return false; + } + + String stringValue = truncateAtNUL(value.toWTFString(globalObject)); + RETURN_IF_EXCEPTION(scope, false); + + String key = truncateAtNUL(String(uid)); + // Node silently ignores empty names and names containing '=' (setenv would + // EINVAL); the assignment succeeds but nothing is stored. + if (key.isEmpty() || key.find('=') != notFound) + return true; + + applySharedEnvSideEffects(globalObject, key, stringValue); + + BunString name = Bun::toString(key); + BunString val = Bun::toString(stringValue); + Bun__ProcessEnv__put(globalObject, &name, &val); + return true; +} + +bool JSProcessEnvMap::deleteProperty(JSCell* cell, JSGlobalObject* globalObject, PropertyName propertyName, DeletePropertySlot& slot) +{ + auto* uid = propertyName.uid(); + if (propertyName.isSymbol() || !uid) + return Base::deleteProperty(cell, globalObject, propertyName, slot); + + String key = truncateAtNUL(String(uid)); + if (!key.isEmpty()) { + BunString name = Bun::toString(key); + Bun__ProcessEnv__delete(globalObject, &name); + } + return Base::deleteProperty(cell, globalObject, propertyName, slot); +} + +bool JSProcessEnvMap::defineOwnProperty(JSObject* object, JSGlobalObject* globalObject, PropertyName propertyName, const PropertyDescriptor& descriptor, bool shouldThrow) +{ + VM& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + auto* uid = propertyName.uid(); + if (propertyName.isSymbol() || !uid) { + if (propertyName.isSymbol() && uid) { + throwTypeError(globalObject, scope, "Cannot convert a Symbol value to a string"_s); + return false; + } + RELEASE_AND_RETURN(scope, Base::defineOwnProperty(object, globalObject, propertyName, descriptor, shouldThrow)); + } + + // Node only accepts a fully-permissive data descriptor (value present, + // configurable/writable/enumerable all true) and routes it through the env + // setter; anything else is ERR_INVALID_OBJECT_DEFINE_PROPERTY. + if (descriptor.isAccessorDescriptor()) { + throwError(globalObject, scope, ErrorCode::ERR_INVALID_OBJECT_DEFINE_PROPERTY, + "'process.env' does not accept an accessor(getter/setter) descriptor"_s); + return false; + } + if (!descriptor.value() + || !(descriptor.configurablePresent() && descriptor.configurable()) + || !(descriptor.writablePresent() && descriptor.writable()) + || !(descriptor.enumerablePresent() && descriptor.enumerable())) { + throwError(globalObject, scope, ErrorCode::ERR_INVALID_OBJECT_DEFINE_PROPERTY, + "'process.env' only accepts a configurable, writable, and enumerable data descriptor"_s); + return false; + } + + PutPropertySlot slot(object, shouldThrow); + RELEASE_AND_RETURN(scope, put(object, globalObject, propertyName, descriptor.value(), slot)); +} + +bool JSProcessEnvMap::preventExtensions(JSObject*, JSGlobalObject*) +{ + // Node: preventExtensions / seal / freeze throw TypeError. Returning false + // makes the Object.{freeze,seal,preventExtensions} builtins throw their + // own TypeError. + return false; +} + +void JSProcessEnvMap::getOwnPropertyNames(JSObject* object, JSGlobalObject* globalObject, PropertyNameArrayBuilder& propertyNames, DontEnumPropertiesMode mode) +{ + VM& vm = JSC::getVM(globalObject); + void* list; + size_t count = Bun__getEnvCount(globalObject, &list); + for (size_t i = 0; i < count; i++) { + unsigned char* chars; + size_t len = Bun__getEnvKey(list, i, &chars); + auto key = String::fromUTF8ReplacingInvalidSequences(std::span { chars, len }); + propertyNames.add(JSC::Identifier::fromString(vm, key)); + } + Base::getOwnPropertyNames(object, globalObject, propertyNames, mode); +} + +bool JSProcessEnvMap::getOwnPropertySlotByIndex(JSObject* object, JSGlobalObject* globalObject, unsigned index, PropertySlot& slot) +{ + VM& vm = JSC::getVM(globalObject); + return getOwnPropertySlot(object, globalObject, Identifier::from(vm, index), slot); +} + +bool JSProcessEnvMap::putByIndex(JSCell* cell, JSGlobalObject* globalObject, unsigned index, JSValue value, bool shouldThrow) +{ + VM& vm = JSC::getVM(globalObject); + PutPropertySlot slot(cell, shouldThrow); + return put(cell, globalObject, Identifier::from(vm, index), value, slot); +} + +bool JSProcessEnvMap::deletePropertyByIndex(JSCell* cell, JSGlobalObject* globalObject, unsigned index) +{ + String key = String::number(index); + BunString name = Bun::toString(key); + Bun__ProcessEnv__delete(globalObject, &name); + return Base::deletePropertyByIndex(cell, globalObject, index); +} +#endif + RefPtr ensureSharedEnvStoreForWorker(Zig::GlobalObject* globalObject) { VM& vm = globalObject->vm(); @@ -749,6 +992,12 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) VM& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); +#if !OS(WINDOWS) + UNUSED_PARAM(scope); + auto* structure = JSProcessEnvMap::createStructure(vm, globalObject, globalObject->objectPrototype()); + return JSProcessEnvMap::create(vm, structure); +#else + void* list; size_t count = Bun__getEnvCount(globalObject, &list); JSC::JSObject* object = nullptr; @@ -758,10 +1007,8 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) object = constructEmptyObject(globalObject, globalObject->objectPrototype()); } -#if OS(WINDOWS) JSArray* keyArray = constructEmptyArray(globalObject, nullptr, count); RETURN_IF_EXCEPTION(scope, {}); -#endif static NeverDestroyed TZ = MAKE_STATIC_STRING_IMPL("TZ"); String NODE_TLS_REJECT_UNAUTHORIZED = String("NODE_TLS_REJECT_UNAUTHORIZED"_s); @@ -777,10 +1024,6 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) bool hasProxyVar[proxyVarCount] = {}; auto isProxyVar = [&](const String& name) -> std::optional { - for (size_t j = 0; j < proxyVarCount; j++) { - if (name == proxyVarNames[j]) return j; - } -#if OS(WINDOWS) // Windows env var names are case-insensitive, so the OS env block can // carry any casing (`Http_Proxy`, `HTTP_proxy`, ...). Without this // fallback the per-key loop falls through, the bottom loop then adds @@ -790,7 +1033,6 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) for (size_t j = 0; j < proxyVarCount; j++) { if (equalIgnoringASCIICase(name, proxyVarNames[j])) return j; } -#endif return std::nullopt; }; @@ -802,9 +1044,7 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) size_t len = Bun__getEnvKey(list, i, &chars); // We can't really trust that the OS gives us valid UTF-8 auto name = String::fromUTF8ReplacingInvalidSequences(std::span { chars, len }); -#if OS(WINDOWS) keyArray->putByIndexInline(globalObject, (unsigned)i, jsString(vm, name), false); -#endif if (name == TZ) { hasTZ = true; continue; @@ -822,11 +1062,7 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) continue; } ASSERT(len > 0); -#if OS(WINDOWS) String idName = name.convertToASCIIUppercase(); -#else - String idName = name; -#endif Identifier identifier = Identifier::fromString(vm, idName); // CustomGetterSetter doesn't support indexed properties yet. @@ -892,7 +1128,6 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) attrs); } -#if OS(WINDOWS) auto editWindowsEnvVar = JSC::JSFunction::create(vm, globalObject, 0, String("editWindowsEnvVar"_s), jsEditWindowsEnvVar, ImplementationVisibility::Public); JSC::JSFunction* getSourceEvent = JSC::JSFunction::create(vm, globalObject, processObjectInternalsWindowsEnvCodeGenerator(vm), globalObject); @@ -913,8 +1148,6 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) } RELEASE_AND_RETURN(scope, result); -#else - return object; #endif } } diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 3124acf81235..ba0bf97c52db 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -901,8 +901,12 @@ impl WebWorker { // `heap::alloc`'d and stashed on `self` so `shutdown()` step 5 reclaims // it on every path — including the early-terminate checkpoint below, // which calls `shutdown()` before the VM exists. - let loader_ptr: *mut bun_dotenv::Loader = - bun_core::heap::into_raw(Box::new(bun_dotenv::Loader::init_with_map(map))); + let mut loader = bun_dotenv::Loader::init_with_map(map); + // The cloned map already holds the parent's environ snapshot; walking + // libc environ again on this thread would race a main-thread + // process.env write's setenv() (which may realloc __environ). + loader.did_load_process = true; + let loader_ptr: *mut bun_dotenv::Loader = bun_core::heap::into_raw(Box::new(loader)); self.worker_env_loader.set(loader_ptr); // Checkpoint before the expensive part: initWorker builds a full JSC @@ -992,6 +996,13 @@ impl WebWorker { // SAFETY: see post-publish note above. unsafe { + // The main-thread entry points (run/test/repl) set this before + // configure_defines(); without it the worker's transpiler inlines + // `process.env.X` from its DotEnv map (which it seeds from environ + // on spawn), so a main-thread `process.env.X = ...` + setenv leaks + // into the worker's source as a literal. + (*vm).transpiler.options.env.behavior = + bun_dotenv::DotEnvBehavior::LoadAllWithoutInlining; if (*vm).transpiler.configure_defines().is_err() { // Fall through to spin() → shutdown() for full teardown under // the API lock (flushLogs runs JS). Set terminate so spin() diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 6c3911db557a..f4c4ce07d2ee 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -2176,7 +2176,9 @@ pub mod environment_variables { ) -> bool { let vm = global_object.bun_vm(); let name_slice = name.to_utf8(); - let Some(val) = vm.env_loader().get(name_slice.slice()) else { + // `Loader::get` strips a leading `$` (bunfig/.env interpolation); this + // is the `process.env[key]` read path, so look the map up directly. + let Some(val) = vm.env_loader().map.get(name_slice.slice()) else { return false; }; value.write(BunString::borrow_utf8(val)); @@ -2247,6 +2249,100 @@ pub mod environment_variables { let value = vm.env_loader().get(sliced.slice())?; Some(ZigString::init_utf8(value)) } + + // setenv/unsetenv take C strings; Node truncates at the first NUL. + #[inline] + fn truncate_at_nul(s: &[u8]) -> &[u8] { + match bun_core::strings::index_of_char(s, 0) { + Some(i) => &s[..i as usize], + None => s, + } + } + + /// `process.env[name] = value`: update the env_loader map, and on the main + /// thread also `setenv()` so a native library's `getenv()` observes the + /// write. Key/value are NUL-truncated and empty / `=`-containing keys are + /// dropped here so every C++ caller (JSProcessEnvMap, JSSharedEnvMap via + /// syncOSEnv) sees the same contract. + #[unsafe(no_mangle)] + pub(crate) extern "C" fn Bun__ProcessEnv__put( + global_object: &JSGlobalObject, + name: &BunString, + value: &BunString, + ) { + let vm = global_object.bun_vm().as_mut(); + let name_slice = name.to_utf8(); + let key = truncate_at_nul(name_slice.slice()); + if key.is_empty() || bun_core::strings::index_of_char(key, b'=').is_some() { + return; + } + let value_slice = value.to_utf8(); + let val = truncate_at_nul(value_slice.slice()); + + { + // Serialise against a concurrently-spawning worker's + // env.map.clone_with_allocator (web_worker.rs holds this same lock). + let _slots = vm.proxy_env_storage.lock(); + bun_core::handle_oom(vm.transpiler.env_mut().map.put(key, val)); + } + + if vm.is_main_thread() { + #[cfg(not(windows))] + { + let Ok(key_z) = std::ffi::CString::new(key) else { + return; + }; + let Ok(val_z) = std::ffi::CString::new(val) else { + return; + }; + let _g = bun_core::environ_write_lock(); + // SAFETY: NUL-terminated C strings; setenv copies both. The + // write lock excludes bun_core::getenv_z on other threads. + unsafe { libc::setenv(key_z.as_ptr(), val_z.as_ptr(), 1) }; + bun_core::env_var::invalidate_for_setenv(key, Some(val)); + } + } + } + + /// `delete process.env[name]`: remove from the env_loader map, and on the + /// main thread also `unsetenv()`. + #[unsafe(no_mangle)] + pub(crate) extern "C" fn Bun__ProcessEnv__delete( + global_object: &JSGlobalObject, + name: &BunString, + ) { + let vm = global_object.bun_vm().as_mut(); + let name_slice = name.to_utf8(); + let key = truncate_at_nul(name_slice.slice()); + if key.is_empty() { + return; + } + + { + let mut slots = vm.proxy_env_storage.lock(); + // Clear a matching proxy-var slot so a later worker spawn's + // sync_into doesn't re-insert the deleted value. + if let Some(slot) = slots.slot(key) { + *slot.ptr = None; + } + // Ordered remove so Object.keys(process.env) keeps the relative + // order of remaining keys after delete (Node/unsetenv shift + // environ in place). + vm.transpiler.env_mut().map.map.ordered_remove(key); + } + + if vm.is_main_thread() { + #[cfg(not(windows))] + { + if let Ok(key_z) = std::ffi::CString::new(key) { + let _g = bun_core::environ_write_lock(); + // SAFETY: NUL-terminated C string. + unsafe { libc::unsetenv(key_z.as_ptr()) }; + bun_core::env_var::invalidate_for_setenv(key, None); + } + } + } + } } #[unsafe(no_mangle)] diff --git a/test/js/node/process/process-env-exotic.test.ts b/test/js/node/process/process-env-exotic.test.ts new file mode 100644 index 000000000000..abf79ad8ead9 --- /dev/null +++ b/test/js/node/process/process-env-exotic.test.ts @@ -0,0 +1,396 @@ +// Node's process.env is an exotic object (src/node_env_var.cc RealEnvStore): +// writes coerce to string, symbol keys/values throw, `=` / empty keys are +// silently dropped, NUL truncates, defineProperty rejects accessors and +// non-permissive descriptors, and every set/delete reaches libc setenv/ +// unsetenv so native getenv() and os.homedir() stay in sync. +// +// On Windows, Bun's process.env is a Proxy (case-insensitive keys, +// SetEnvironmentVariableW write-through) whose set/delete already go through +// the coercing path; the tests here cover the POSIX exotic-object contract. +import { afterEach, describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isPosix, isWindows, libcPathForDlopen, tempDir } from "harness"; +import path from "path"; + +const cc = Bun.which("clang") || Bun.which("gcc") || Bun.which("cc"); + +describe("process.env node semantics", () => { + // Writes now reach real setenv(), so a failed assertion would leak the var + // into environ; clean up unconditionally. + afterEach(() => { + for (const k of Object.keys(process.env)) { + if (k.startsWith("ENVFIX_")) delete process.env[k]; + } + }); + + test("assigned values are coerced to strings", () => { + process.env.ENVFIX_NUM = 3000 as unknown as string; + expect(process.env.ENVFIX_NUM).toBe("3000"); + expect(typeof process.env.ENVFIX_NUM).toBe("string"); + + process.env.ENVFIX_UNDEF = undefined as unknown as string; + expect(process.env.ENVFIX_UNDEF).toBe("undefined"); + expect(typeof process.env.ENVFIX_UNDEF).toBe("string"); + + process.env.ENVFIX_BOOL = true as unknown as string; + expect(process.env.ENVFIX_BOOL).toBe("true"); + + process.env.ENVFIX_OBJ = { toString: () => "from-toString" } as unknown as string; + expect(process.env.ENVFIX_OBJ).toBe("from-toString"); + }); + + test("symbol value throws TypeError on assignment", () => { + expect(() => { + process.env.ENVFIX_SYM = Symbol("x") as unknown as string; + }).toThrow(TypeError); + expect("ENVFIX_SYM" in process.env).toBe(false); + }); + + test("symbol key throws TypeError on assignment", () => { + const key = Symbol("env-key"); + expect(() => { + (process.env as Record)[key] = "v"; + }).toThrow(TypeError); + }); + + test("'=' in key and empty key are silently ignored", () => { + process.env["A=B"] = "x"; + expect("A=B" in process.env).toBe(false); + expect(process.env["A=B"]).toBeUndefined(); + + process.env[""] = "x"; + expect("" in process.env).toBe(false); + expect(process.env[""]).toBeUndefined(); + }); + + test.skipIf(isWindows)("NUL in value truncates at NUL", () => { + process.env.ENVFIX_NUL = "ab\x00cd"; + expect(process.env.ENVFIX_NUL).toBe("ab"); + }); + + test.skipIf(isWindows)("NUL in key truncates at NUL", () => { + process.env["ENVFIX_K\x00TAIL"] = "v"; + expect(process.env.ENVFIX_K).toBe("v"); + expect(process.env["ENVFIX_K\x00TAIL"]).toBe("v"); + }); + + test("Object.freeze / seal / preventExtensions throw TypeError", async () => { + // A fail-before run (old plain-object process.env) would actually freeze + // the runner's env and poison later tests, so probe in a subprocess. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const probe = fn => { try { fn(); return null; } catch (e) { return e.constructor.name; } }; + process.stdout.write(JSON.stringify({ + freeze: probe(() => Object.freeze(process.env)), + seal: probe(() => Object.seal(process.env)), + prevExt: probe(() => Object.preventExtensions(process.env)), + isExt: Object.isExtensible(process.env), + })); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + freeze: "TypeError", + seal: "TypeError", + prevExt: "TypeError", + isExt: true, + }); + expect(exitCode).toBe(0); + }); + + test("defineProperty rejects accessor descriptors", () => { + expect(() => { + Object.defineProperty(process.env, "ENVFIX_ACCESSOR", { get: () => "x" }); + }).toThrow(expect.objectContaining({ code: "ERR_INVALID_OBJECT_DEFINE_PROPERTY" })); + expect("ENVFIX_ACCESSOR" in process.env).toBe(false); + }); + + test("defineProperty rejects non-permissive data descriptors", () => { + expect(() => { + Object.defineProperty(process.env, "ENVFIX_RO", { value: "x", writable: false }); + }).toThrow(expect.objectContaining({ code: "ERR_INVALID_OBJECT_DEFINE_PROPERTY" })); + expect(() => { + Object.defineProperty(process.env, "ENVFIX_RO", { value: "x" }); + }).toThrow(expect.objectContaining({ code: "ERR_INVALID_OBJECT_DEFINE_PROPERTY" })); + }); + + test("defineProperty with a fully-permissive data descriptor coerces and sets", () => { + Object.defineProperty(process.env, "ENVFIX_DEF", { + value: 42, + writable: true, + configurable: true, + enumerable: true, + }); + expect(process.env.ENVFIX_DEF).toBe("42"); + }); + + test("defineProperty with a Symbol value throws TypeError", () => { + expect(() => { + Object.defineProperty(process.env, "ENVFIX_SYMDEF", { + value: Symbol("x"), + writable: true, + configurable: true, + enumerable: true, + }); + }).toThrow(TypeError); + expect("ENVFIX_SYMDEF" in process.env).toBe(false); + }); + + test("delete removes the key and preserves remaining key order", () => { + process.env.ENVFIX_A = "a"; + process.env.ENVFIX_B = "b"; + process.env.ENVFIX_C = "c"; + expect(delete process.env.ENVFIX_B).toBe(true); + expect("ENVFIX_B" in process.env).toBe(false); + expect(process.env.ENVFIX_B).toBeUndefined(); + expect(Object.keys(process.env).filter(k => k.startsWith("ENVFIX_"))).toEqual(["ENVFIX_A", "ENVFIX_C"]); + }); + + test.skipIf(isWindows)("$-prefixed keys are read back as-is (no bunfig-interpolation strip)", () => { + expect(process.env["$PATH"]).toBeUndefined(); + expect("$PATH" in process.env).toBe(false); + process.env["$ENVFIX_DOLLAR"] = "x"; + expect(process.env["$ENVFIX_DOLLAR"]).toBe("x"); + expect(process.env["ENVFIX_DOLLAR"]).toBeUndefined(); + delete process.env["$ENVFIX_DOLLAR"]; + }); + + test("coerced value reaches a spawned child's inherited env", async () => { + process.env.ENVFIX_SPAWN = 3000 as unknown as string; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `process.stdout.write(typeof process.env.ENVFIX_SPAWN + ":" + process.env.ENVFIX_SPAWN)`], + env: { ...process.env }, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("string:3000"); + expect(exitCode).toBe(0); + }); +}); + +// These exercise the setenv sync and the env_var::HOME cache invalidation, +// which only exist on POSIX (Windows uses uv_os_homedir / the env Proxy). +describe.skipIf(!isPosix)("process.env setenv sync + typed-cache invalidation", () => { + // child_process with no `env:` serializes process.env (the JS object), so a + // spawned child cannot distinguish a JS-only write from a real setenv(). Use + // libc getenv() via FFI in the same process to prove the write reached + // `environ`. + const getenvProbe = ` + const { dlopen, CString } = require("bun:ffi"); + const { symbols: { getenv } } = dlopen(${JSON.stringify(isPosix ? libcPathForDlopen() : "")}, { + getenv: { args: ["cstring"], returns: "ptr" }, + }); + const read = name => { + const ptr = getenv(Buffer.from(name + "\\0")); + return ptr ? new CString(ptr).toString() : null; + }; + `; + + test("process.env write reaches native getenv()", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + getenvProbe + + ` + process.env.ENVFIX_SETENV = "from-js"; + process.stdout.write(JSON.stringify(read("ENVFIX_SETENV"))); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe(`"from-js"`); + expect(exitCode).toBe(0); + }); + + test("delete process.env.X reaches native unsetenv()", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + getenvProbe + + ` + const before = read("ENVFIX_SEEDED"); + delete process.env.ENVFIX_SEEDED; + const after = read("ENVFIX_SEEDED"); + process.stdout.write(JSON.stringify({ before, after })); + `, + ], + env: { ...bunEnv, ENVFIX_SEEDED: "present" }, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ before: "present", after: null }); + expect(exitCode).toBe(0); + }); + + test("NUL-containing value after founding a SHARE_ENV tree truncates (no panic)", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker, SHARE_ENV } = require("node:worker_threads"); + const w = new Worker("require('node:worker_threads').parentPort.postMessage(1)", { eval: true, env: SHARE_ENV }); + await new Promise(r => w.on("exit", r)); + process.env.ENVFIX_SNUL = "ab\\x00cd"; + process.stdout.write(JSON.stringify(process.env.ENVFIX_SNUL)); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + // JSSharedEnvMap::put stores the un-truncated string in the shared store, + // so the readback is "ab\0cd"; syncOSEnv's Bun__ProcessEnv__put truncates + // for the env_loader map / setenv so no CString panic. + expect(JSON.parse(stdout)).toBe("ab\x00cd"); + expect(exitCode).toBe(0); + }); + + test("process.env write still reaches setenv() after founding a SHARE_ENV tree", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + getenvProbe + + ` + const { Worker, SHARE_ENV } = require("node:worker_threads"); + const w = new Worker("require('node:worker_threads').parentPort.postMessage(1)", { eval: true, env: SHARE_ENV }); + await new Promise(r => w.on("exit", r)); + process.env.ENVFIX_POSTSWAP = "after-swap"; + process.stdout.write(JSON.stringify({ getenv: read("ENVFIX_POSTSWAP"), homedir: (() => { + const os = require("node:os"); + os.homedir(); + process.env.HOME = "/tmp/envfix-post"; + return os.homedir(); + })() })); + `, + ], + env: { ...bunEnv, HOME: "/tmp/envfix-before" }, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ getenv: "after-swap", homedir: "/tmp/envfix-post" }); + expect(exitCode).toBe(0); + }); + + test("os.homedir() observes process.env.HOME = ... (typed cache invalidated)", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const os = require("node:os"); + const before = os.homedir(); + process.env.HOME = "/tmp/envfix-home"; + const after = os.homedir(); + process.stdout.write(JSON.stringify({ before, after })); + `, + ], + env: { ...bunEnv, HOME: "/tmp/envfix-before" }, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ before: "/tmp/envfix-before", after: "/tmp/envfix-home" }); + expect(exitCode).toBe(0); + }); +}); + +// Duplicate KEY= entries in environ and entries with no '=' require a custom +// execve() launcher to inject them, since Bun.spawn's env object can't express +// either. +describe.skipIf(!isPosix || !cc)("environ load: first-wins duplicates, drop no-'=' entries", () => { + async function compile(dir: string, src: string, out: string) { + await using compile = Bun.spawn({ + cmd: [cc!, "-O0", "-o", path.join(dir, out), path.join(dir, src)], + env: bunEnv, + stderr: "pipe", + }); + const [, cerr, ccode] = await Promise.all([compile.stdout.text(), compile.stderr.text(), compile.exited]); + if (ccode !== 0) throw new Error(`compile failed: ${cerr}`); + return path.join(dir, out); + } + + test("duplicate keys resolve to the FIRST occurrence", async () => { + using dir = tempDir("envfix-dup", { + "launch.c": ` + #include + #include + int main(int argc, char **argv) { + if (argc < 2) return 2; + char *env[] = { + "ENVFIX_DUP=/first", + "ENVFIX_DUP=/second", + "PATH=/usr/bin:/bin:/usr/local/bin", + "BUN_DEBUG_QUIET_LOGS=1", + "NO_COLOR=1", + 0, + }; + execve(argv[1], &argv[1], env); + perror("execve"); + return 127; + } + `, + }); + const bin = await compile(String(dir), "launch.c", "launch"); + await using proc = Bun.spawn({ + cmd: [bin, bunExe(), "-e", `process.stdout.write(process.env.ENVFIX_DUP ?? "")`], + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("/first"); + expect(exitCode).toBe(0); + }); + + test("entry with no '=' is dropped, not fabricated as empty", async () => { + using dir = tempDir("envfix-noeq", { + "launch.c": ` + #include + #include + int main(int argc, char **argv) { + if (argc < 2) return 2; + char *env[] = { + "ENVFIX_BARE", + "PATH=/usr/bin:/bin:/usr/local/bin", + "BUN_DEBUG_QUIET_LOGS=1", + "NO_COLOR=1", + 0, + }; + execve(argv[1], &argv[1], env); + perror("execve"); + return 127; + } + `, + }); + const bin = await compile(String(dir), "launch.c", "launch"); + await using proc = Bun.spawn({ + cmd: [ + bin, + bunExe(), + "-e", + `process.stdout.write(JSON.stringify({ has: "ENVFIX_BARE" in process.env, val: process.env.ENVFIX_BARE ?? null }))`, + ], + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ has: false, val: null }); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index b191a4ed15ab..e9cc5017b156 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -251,9 +251,11 @@ it("ICU version does not regress", () => { it("process.env.TZ", () => { var origTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone; - // the default timezone is Etc/UTC + // The default timezone is UTC. ICU reports it as either "UTC" or "Etc/UTC" + // depending on the zone-detection path; accept both. This branch was dead + // while `"TZ" in process.env` was always true (the old DontEnum accessor). if (!("TZ" in process.env)) { - expect(origTimezone).toBe("Etc/UTC"); + expect(["UTC", "Etc/UTC"]).toContain(origTimezone); } const realOrigTimezone = origTimezone; diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 68d6c6f3f103..1fc762a211b8 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -1459,36 +1459,39 @@ describe("env: SHARE_ENV shares the spawning thread's env, not a process-wide on }); }); - // An accessor installed via defineProperty lands on the base object, but reads hit - // the store first — so the store entry must go, or the getter is shadowed. (Node - // rejects accessors on process.env entirely; bun allows them on the regular map, - // so the shared map matches the regular one rather than diverging from it.) - it("does not let the store shadow an accessor defined on process.env", async () => { + // Node rejects accessor descriptors on process.env entirely (on both the + // regular and SHARE_ENV maps). Bun's regular map matches Node; the SHARE_ENV + // map still installs the accessor on its base object (store entry moved out + // of the way), which is visible to the founding thread only. + it("rejects accessor defineProperty on the regular process.env like Node", async () => { const proc = Bun.spawn({ cmd: [ bunExe(), "-e", `const { Worker, SHARE_ENV } = require("worker_threads"); - const probe = \`process.env.FOO = "old"; + let code = null; + try { Object.defineProperty(process.env, "FOO", { get: () => "new", configurable: true }); - const count = Object.keys(process.env).filter(k => k === "FOO").length; - const read = process.env.FOO; - delete process.env.FOO; - ({ read, count, afterDelete: process.env.FOO ?? null })\`; - const regular = eval(probe); + } catch (e) { + code = e.code; + } const w = new Worker( - 'const { parentPort } = require("worker_threads"); parentPort.postMessage(eval(' + JSON.stringify(probe) + '));', + 'const { parentPort } = require("worker_threads"); ' + + 'process.env.FOO = "old"; ' + + 'Object.defineProperty(process.env, "FOO", { get: () => "new", configurable: true }); ' + + 'parentPort.postMessage({ read: process.env.FOO });', { eval: true, env: SHARE_ENV }, ); - w.on("message", shared => console.log(JSON.stringify({ regular, shared })));`, + w.on("message", shared => console.log(JSON.stringify({ code, shared })));`, ], env: bunEnv, stderr: "pipe", }); const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - // count === 1: defineProperty on an existing enumerable key keeps it enumerable. - const want = { read: "new", count: 1, afterDelete: null }; - expect(JSON.parse(stdout)).toEqual({ regular: want, shared: want }); + expect(JSON.parse(stdout)).toEqual({ + code: "ERR_INVALID_OBJECT_DEFINE_PROPERTY", + shared: { read: "new" }, + }); expect(exitCode).toBe(0); }); diff --git a/test/preload.ts b/test/preload.ts index ec495d79315d..22347fb91fa3 100644 --- a/test/preload.ts +++ b/test/preload.ts @@ -10,6 +10,9 @@ for (let key in process.env) { for (let key in harness.bunEnv) { if (key === "TZ") continue; + // process.env writes now reach setenv(); forcing bunEnv's CI="1" here would + // override the CI=false a parent test passes to enable `.only()` fixtures. + if (key === "CI") continue; if (harness.bunEnv[key] === undefined) continue; process.env[key] = harness.bunEnv[key] + ""; }