Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions src/jsc/JSGlobalObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,12 @@ impl JSGlobalObject {
ZigGlobalObject__makeNapiEnvForFFI(self)
}

/// The live `process.env` JS object (forces the lazy init). Mutations made
/// via `process.env.X = ...` / `delete process.env.X` are visible here.
pub fn process_env_object(&self) -> JsResult<JSValue> {
crate::from_js_host_call(self, || ZigGlobalObject__processEnvObject(self))
}
Comment thread
claude[bot] marked this conversation as resolved.

#[inline]
pub fn assert_on_js_thread(&self) {
if cfg!(debug_assertions) {
Expand Down Expand Up @@ -1679,6 +1685,7 @@ unsafe extern "C" {
-> JSValue;

safe fn ZigGlobalObject__makeNapiEnvForFFI(this: &JSGlobalObject) -> *mut c_void;
safe fn ZigGlobalObject__processEnvObject(this: &JSGlobalObject) -> JSValue;

safe fn JSC__JSGlobalObject__bunVM(this: &JSGlobalObject) -> *mut c_void;
safe fn JSC__JSGlobalObject__vm(this: &JSGlobalObject) -> *mut VM;
Expand Down
9 changes: 9 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1882,6 +1882,15 @@ extern "C" napi_env ZigGlobalObject__makeNapiEnvForFFI(Zig::GlobalObject* global
return globalObject->makeNapiEnvForFFI();
}

extern "C" JSC::EncodedJSValue ZigGlobalObject__processEnvObject(Zig::GlobalObject* globalObject)
{
auto& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);
JSC::JSObject* object = globalObject->processEnvObject();
RETURN_IF_EXCEPTION(scope, {});
return JSValue::encode(object);
}

JSC_DEFINE_HOST_FUNCTION(jsFunctionPerformMicrotaskVariadic, (JSGlobalObject * globalObject, CallFrame* callframe))
{
auto& vm = JSC::getVM(globalObject);
Expand Down
7 changes: 4 additions & 3 deletions src/runtime/api/BunObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,9 +614,10 @@ pub(crate) fn which(global_this: &JSGlobalObject, callframe: &CallFrame) -> JsRe
return Ok(JSValue::NULL);
}

// SAFETY: `transpiler.env` / `.fs` are process-lifetime singletons set during VM init.
let mut path_str =
ZigStringSlice::from_utf8_never_free(vm.env_loader().get(b"PATH").unwrap_or(b""));
let mut path_str = match global_this.process_env_object()?.get(global_this, "PATH")? {
Some(v) if !v.is_undefined_or_null() => v.to_slice(global_this)?,
_ => ZigStringSlice::from_utf8_never_free(b""),
};
let mut cwd_str = ZigStringSlice::from_utf8_never_free(vm.top_level_dir());

if let Some(arg) = arguments.next_eat() {
Expand Down
58 changes: 28 additions & 30 deletions src/runtime/api/bun/js_bun_spawn_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,6 @@ pub(crate) fn spawn_maybe_sync<const IS_SYNC: bool>(
// drops after `spawn_process` returns, freeing all argv/env allocations.
let mut cstr_storage: Vec<ZBox> = Vec::new();

let mut override_env = false;
let mut env_array: Vec<CStrPtr> = Vec::new();
// SAFETY: `bun_vm()` returns the live VirtualMachine for this thread; it
// outlives this call frame.
Expand All @@ -383,9 +382,9 @@ pub(crate) fn spawn_maybe_sync<const IS_SYNC: bool>(
let mut lazy = false;
let mut on_exit_callback = JSValue::ZERO;
let mut on_disconnect_callback = JSValue::ZERO;
// `env_loader()` is the audited safe accessor for the per-VM DotEnv loader
// (process-lifetime; centralised non-null deref in `VirtualMachine`).
let mut path: &[u8] = jsc_vm.env_loader().get(b"PATH").unwrap_or(b"");
// Populated from the explicit `env:` option or, when absent, the live
// `process.env` object; both via `append_envp_from_js` below.
let mut path: &[u8] = b"";
let mut argv: Vec<CStrPtr> = Vec::new();
let cmd_value: JSValue;
let mut detached = false;
Expand Down Expand Up @@ -601,7 +600,6 @@ pub(crate) fn spawn_maybe_sync<const IS_SYNC: bool>(
);
};

override_env = true;
// If the env object does not include a $PATH, it must disable path lookup for argv[0]
let mut new_path: &[u8] = b"";
// `JSObject` is an `opaque_ffi!` ZST handle; `opaque_ref` is the
Expand All @@ -614,6 +612,8 @@ pub(crate) fn spawn_maybe_sync<const IS_SYNC: bool>(
&mut cstr_storage,
)?;
path = new_path;
} else {
inherit_process_env(global_this, &mut env_array, &mut path, &mut cstr_storage)?;
}

get_argv(
Expand Down Expand Up @@ -894,6 +894,7 @@ pub(crate) fn spawn_maybe_sync<const IS_SYNC: bool>(
}
}
} else {
inherit_process_env(global_this, &mut env_array, &mut path, &mut cstr_storage)?;
get_argv(
global_this,
cmd_value,
Expand All @@ -908,31 +909,6 @@ pub(crate) fn spawn_maybe_sync<const IS_SYNC: bool>(

bun_output::scoped_log!(Subprocess, "spawn maxBuffer: {:?}", max_buffer);

// Owns the `K=V\0` storage when inheriting the parent env; the struct
// lives until spawn returns.
let mut inherited_env_storage: Option<bun_dotenv::NullDelimitedEnvMap> = None;
if !override_env && env_array.is_empty() {
// `Transpiler::env_mut()` is the audited safe `&mut Loader` accessor
// (per-VM DotEnv loader, valid for VM lifetime; centralised
// single-unsafe deref). `.map` is its `&'a mut Map` slot.
let envmap = match jsc_vm
.transpiler
.env_mut()
.map
.create_null_delimited_env_map()
{
Ok(m) => m,
Err(_) => return Err(global_this.throw_out_of_memory()),
};
// Note: `as_slice()` *includes* the trailing null, so strip it; the
// common tail below re-appends one after the optional NODE_CHANNEL_*
// entries.
let entries = envmap.as_slice();
env_array.extend_from_slice(&entries[..entries.len().saturating_sub(1)]);
inherited_env_storage = Some(envmap);
}
let _ = &inherited_env_storage;

for fd_index in 0..stdio.len() {
if stdio[fd_index].can_use_memfd() {
if stdio[fd_index].use_memfd(fd_index as u32) {
Expand Down Expand Up @@ -2094,6 +2070,28 @@ fn throw_command_not_found(global_this: &JSGlobalObject, command: &[u8]) -> JsEr
global_this.throw_value(err.to_error_instance(global_this))
}

/// Populate `envp` / `path` from the live `process.env` JS object so runtime
/// mutations (set/delete/PATH edits) reach the child.
fn inherit_process_env(
global_this: &JSGlobalObject,
envp: &mut Vec<CStrPtr>,
path: &mut &[u8],
storage: &mut Vec<ZBox>,
) -> JsResult<()> {
let process_env = global_this.process_env_object()?;
process_env.ensure_still_alive();
if let Some(object) = process_env.get_object() {
append_envp_from_js(
global_this,
JSObject::opaque_ref(object),
envp,
path,
storage,
)?;
}
Ok(())
}

/// `storage` receives ownership of every `K=V\0` line whose pointer is pushed
/// into `envp` (and, for `PATH=`, sliced into `*path`); the caller's
/// `Vec<ZBox>` is dropped after `spawn_process` returns.
Expand Down
67 changes: 65 additions & 2 deletions test/js/bun/spawn/spawn-env.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,69 @@
import { spawn } from "bun";
import { test } from "bun:test";
import { bunExe } from "harness";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import { chmodSync } from "node:fs";
import { join } from "node:path";

// Bun.spawn / Bun.spawnSync with no `env:` option must inherit the *live*
// process.env, including runtime mutations, not the startup snapshot.
describe("default env inherits live process.env", () => {
const printEnv = isWindows
? `[process.execPath, "-e", "process.stdout.write((process.env.BUN_TEST_SPAWN_ENV_SET ?? '[unset]') + ',' + (process.env.BUN_TEST_SPAWN_ENV_DEL ?? '[unset]'))"]`
: `["/bin/sh", "-c", "printf '%s,%s' \\"\${BUN_TEST_SPAWN_ENV_SET-[unset]}\\" \\"\${BUN_TEST_SPAWN_ENV_DEL-[unset]}\\""]`;

for (const [label, call] of [
["Bun.spawnSync({cmd})", `Bun.spawnSync({ cmd: ${printEnv} }).stdout.toString()`],
["Bun.spawnSync([cmd])", `Bun.spawnSync(${printEnv}).stdout.toString()`],
["Bun.spawn({cmd})", `await (await Bun.spawn({ cmd: ${printEnv} }).stdout).text()`],
] as const) {
test.concurrent(label, async () => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.env.BUN_TEST_SPAWN_ENV_SET = "runtime-value";
delete process.env.BUN_TEST_SPAWN_ENV_DEL;
process.stdout.write(${call});`,
],
env: { ...bunEnv, BUN_TEST_SPAWN_ENV_DEL: "startup-value" },
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr, exitCode }).toEqual({
stdout: "runtime-value,[unset]",
stderr: "",
exitCode: 0,
});
});
}

test.concurrent.skipIf(isWindows)("PATH mutation is used for argv0 lookup and Bun.which", async () => {

Check warning on line 40 in test/js/bun/spawn/spawn-env.test.ts

View check run for this annotation

Claude / Claude Code Review

PATH-mutation test skips Windows with no equivalent coverage or skip comment

This test is `skipIf(isWindows)` with no Windows variant, so the PR's two PATH-related changes — `Bun.which` reading its default PATH from `process.env` (BunObject.rs:617) and `inherit_process_env` populating `path` for argv0 lookup — are unasserted on Windows, where the PATH lookup goes through the case-insensitive `Path` key (`JSSharedEnvMap`). REVIEW.md → "Cover the variant matrix … POSIX/Windows branches". A Windows variant (`.cmd` tool + `;` separator + `process.env.Path`) is straightforwar
Comment thread
robobun marked this conversation as resolved.
using dir = tempDir("spawn-env-path", {
"bun_test_spawn_env_tool": "#!/bin/sh\nprintf found-the-tool\n",
});
const toolDir = String(dir);
chmodSync(join(toolDir, "bun_test_spawn_env_tool"), 0o755);

await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.env.PATH = ${JSON.stringify(toolDir)} + ":" + process.env.PATH;
console.log(Bun.which("bun_test_spawn_env_tool") !== null);
console.log(Bun.spawnSync({ cmd: ["bun_test_spawn_env_tool"] }).stdout.toString());
console.log(Bun.spawnSync(["bun_test_spawn_env_tool"]).stdout.toString());`,
],
env: bunEnv,
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr, exitCode }).toEqual({
stdout: "true\nfound-the-tool\nfound-the-tool\n",
stderr: "",
exitCode: 0,
});
});
});

test("spawn env", async () => {
const env = {};
Expand Down
Loading