diff --git a/src/bun.js/bindings/JSEnvironmentVariableMap.cpp b/src/bun.js/bindings/JSEnvironmentVariableMap.cpp index f8e469c511..3b853961cb 100644 --- a/src/bun.js/bindings/JSEnvironmentVariableMap.cpp +++ b/src/bun.js/bindings/JSEnvironmentVariableMap.cpp @@ -391,7 +391,16 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) args.append(object); args.append(keyArray); args.append(editWindowsEnvVar); - auto clientData = WebCore::clientData(vm); +#else + // Wrap the env object in a Proxy that coerces all assigned values to strings. + // This matches Node.js behavior where `process.env.FOO = undefined` results in + // `process.env.FOO === "undefined"` (string), not `undefined` (the value). + JSC::JSFunction* getSourceEvent = JSC::JSFunction::create(vm, globalObject, processObjectInternalsPosixEnvCodeGenerator(vm), globalObject); + RETURN_IF_EXCEPTION(scope, {}); + JSC::MarkedArgumentBuffer args; + args.append(object); +#endif + JSC::CallData callData = JSC::getCallData(getSourceEvent); NakedPtr returnedException = nullptr; auto result = JSC::profiledCall(globalObject, JSC::ProfilingReason::API, getSourceEvent, callData, globalObject->globalThis(), args, returnedException); @@ -403,8 +412,5 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) } RELEASE_AND_RETURN(scope, result); -#else - return object; -#endif } } diff --git a/src/js/builtins/ProcessObjectInternals.ts b/src/js/builtins/ProcessObjectInternals.ts index 67156abc56..0b041f1108 100644 --- a/src/js/builtins/ProcessObjectInternals.ts +++ b/src/js/builtins/ProcessObjectInternals.ts @@ -393,7 +393,9 @@ export function windowsEnv( 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 - value = String(value); // If toString() throws, we want to avoid it existing in the envMapList + // Use string concatenation to coerce value to string. This throws for Symbols, + // matching Node.js behavior, and ensures the value is always a string. + value = "" + value; if (!(k in internalEnv) && !envMapList.includes(p)) { envMapList.push(p); } @@ -434,6 +436,42 @@ export function windowsEnv( }); } +export function posixEnv(internalEnv: InternalEnvMap) { + return new Proxy(internalEnv, { + get(target, p) { + return typeof p === "string" ? target[p] : undefined; + }, + set(target, p, value) { + const k = String(p); + // Coerce all values to strings to match Node.js behavior. + // Use string concatenation ('' + value) instead of String(value) because + // concatenation throws for Symbols, matching Node.js behavior. + value = "" + value; + target[k] = value; + return true; + }, + has(target, p) { + return typeof p !== "symbol" ? String(p) in target : false; + }, + deleteProperty(target, p) { + return typeof p !== "symbol" ? delete target[String(p)] : false; + }, + defineProperty(target, p, attributes) { + const k = String(p); + if ("value" in attributes) { + attributes = { ...attributes, value: "" + attributes.value }; + } + return $Object.$defineProperty(target, k, attributes); + }, + getOwnPropertyDescriptor(target, p) { + return typeof p === "string" ? Reflect.getOwnPropertyDescriptor(target, p) : undefined; + }, + ownKeys(target) { + return Reflect.ownKeys(target); + }, + }); +} + export function getChannel() { const EventEmitter = require("node:events"); const setRef = $newZigFunction("node_cluster_binding.zig", "setRef", 1); diff --git a/test/regression/issue/26388.test.ts b/test/regression/issue/26388.test.ts new file mode 100644 index 0000000000..74a46c5fff --- /dev/null +++ b/test/regression/issue/26388.test.ts @@ -0,0 +1,117 @@ +import { afterEach, describe, expect, test } from "bun:test"; + +// Issue #26388: process.env should coerce values to strings like Node.js does +// When assigning undefined, null, numbers, or objects to process.env properties, +// Node.js converts them to strings, but Bun was storing the actual JavaScript values. + +const TEST_ENV_KEYS = [ + "TEST_UNDEFINED", + "TEST_JSON_UNDEFINED", + "TEST_NULL", + "TEST_NUMBER", + "TEST_TRUE", + "TEST_FALSE", + "TEST_OBJECT", + "TEST_ARRAY", + "TEST_STRING", + "TEST_EMPTY", + "TEST_CUSTOM_TOSTRING", + "TEST_SYMBOL", + "TEST_OVERWRITE", +]; + +describe("process.env string coercion", () => { + afterEach(() => { + for (const key of TEST_ENV_KEYS) { + delete process.env[key]; + } + }); + + test("undefined is coerced to 'undefined' string", () => { + process.env.TEST_UNDEFINED = undefined as unknown as string; + expect(process.env.TEST_UNDEFINED).toBe("undefined"); + expect(typeof process.env.TEST_UNDEFINED).toBe("string"); + }); + + test("JSON.stringify(undefined) is coerced to 'undefined' string", () => { + // JSON.stringify(undefined) returns undefined (not the string "undefined") + // This is the exact case that breaks Vite 8 + rolldown + process.env.TEST_JSON_UNDEFINED = JSON.stringify(undefined) as unknown as string; + expect(process.env.TEST_JSON_UNDEFINED).toBe("undefined"); + expect(typeof process.env.TEST_JSON_UNDEFINED).toBe("string"); + }); + + test("null is coerced to 'null' string", () => { + process.env.TEST_NULL = null as unknown as string; + expect(process.env.TEST_NULL).toBe("null"); + expect(typeof process.env.TEST_NULL).toBe("string"); + }); + + test("number is coerced to string", () => { + process.env.TEST_NUMBER = 123 as unknown as string; + expect(process.env.TEST_NUMBER).toBe("123"); + expect(typeof process.env.TEST_NUMBER).toBe("string"); + }); + + test("boolean true is coerced to 'true' string", () => { + process.env.TEST_TRUE = true as unknown as string; + expect(process.env.TEST_TRUE).toBe("true"); + expect(typeof process.env.TEST_TRUE).toBe("string"); + }); + + test("boolean false is coerced to 'false' string", () => { + process.env.TEST_FALSE = false as unknown as string; + expect(process.env.TEST_FALSE).toBe("false"); + expect(typeof process.env.TEST_FALSE).toBe("string"); + }); + + test("object is coerced to '[object Object]' string", () => { + process.env.TEST_OBJECT = { foo: "bar" } as unknown as string; + expect(process.env.TEST_OBJECT).toBe("[object Object]"); + expect(typeof process.env.TEST_OBJECT).toBe("string"); + }); + + test("array is coerced to comma-separated string", () => { + process.env.TEST_ARRAY = [1, 2, 3] as unknown as string; + expect(process.env.TEST_ARRAY).toBe("1,2,3"); + expect(typeof process.env.TEST_ARRAY).toBe("string"); + }); + + test("string stays as string", () => { + process.env.TEST_STRING = "hello"; + expect(process.env.TEST_STRING).toBe("hello"); + expect(typeof process.env.TEST_STRING).toBe("string"); + }); + + test("empty string stays as empty string", () => { + process.env.TEST_EMPTY = ""; + expect(process.env.TEST_EMPTY).toBe(""); + expect(typeof process.env.TEST_EMPTY).toBe("string"); + }); + + test("object with custom toString() uses it", () => { + const obj = { + toString() { + return "custom-string"; + }, + }; + process.env.TEST_CUSTOM_TOSTRING = obj as unknown as string; + expect(process.env.TEST_CUSTOM_TOSTRING).toBe("custom-string"); + expect(typeof process.env.TEST_CUSTOM_TOSTRING).toBe("string"); + }); + + test("Symbol throws TypeError", () => { + expect(() => { + process.env.TEST_SYMBOL = Symbol("test") as unknown as string; + }).toThrow(); + }); + + test("overwriting existing env var coerces to string", () => { + process.env.TEST_OVERWRITE = "initial"; + expect(process.env.TEST_OVERWRITE).toBe("initial"); + + process.env.TEST_OVERWRITE = 456 as unknown as string; + expect(process.env.TEST_OVERWRITE).toBe("456"); + expect(typeof process.env.TEST_OVERWRITE).toBe("string"); + }); +});