process: reject accessor/partial descriptors in Object.defineProperty(process.env, ...)

src/js/builtins/ProcessObjectInternals.ts

@@ -528,13 +528,33 @@ export function windowsEnv(
       return typeof p !== "symbol" ? delete internalEnv[k] : false;
     },
     defineProperty(_, p, attributes) {
+      // Node's EnvDefiner rejects anything but a fully-specified writable,
+      // enumerable, configurable data descriptor. Validate before touching
+      // envMapList or the real environment block.
+      if (!("value" in attributes)) {
+        if ("get" in attributes || "set" in attributes) {
+          throw $ERR_INVALID_OBJECT_DEFINE_PROPERTY(
+            "'process.env' does not accept an accessor(getter/setter) descriptor",
+          );
+        }
+        throw $ERR_INVALID_OBJECT_DEFINE_PROPERTY(
+          "'process.env' only accepts a configurable, writable, and enumerable data descriptor",
+        );
+      }
+      if (attributes.writable !== true || attributes.enumerable !== true || attributes.configurable !== true) {
+        throw $ERR_INVALID_OBJECT_DEFINE_PROPERTY(
+          "'process.env' only accepts a configurable, writable, and enumerable data descriptor",
+        );
+      }
       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)) {
+      const value = String(attributes.value);
+      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;
     },
     getOwnPropertyDescriptor(target, p) {
       if (typeof p === "string") {

src/jsc/bindings/JSEnvironmentVariableMap.cpp

@@ -19,6 +19,7 @@
 #include <JavaScriptCore/PropertyNameArray.h>
 #include <JavaScriptCore/PropertyDescriptor.h>
 #include "BunProcess.h"
+#include "ErrorCode.h"
 #include "ScriptExecutionContext.h"
 #include "SharedEnvStore.h"
 #include "wtf/NeverDestroyed.h"
@@ -364,6 +365,94 @@ static ALWAYS_INLINE void syncWindowsEnv(SharedEnvStore* store, const String& ke
 #endif
 }
 
+// Node.js rejects any Object.defineProperty on process.env whose descriptor is
+// not a fully-specified {value, writable: true, enumerable: true, configurable:
+// true} data descriptor (node_env_var.cc EnvDefiner). An accessor could never be
+// reflected into the real environment block.
+static bool throwIfInvalidEnvDescriptor(JSGlobalObject* globalObject, JSC::ThrowScope& scope, const PropertyDescriptor& descriptor)
+{
+    static constexpr auto dataMsg = "'process.env' only accepts a configurable, writable, and enumerable data descriptor"_s;
+    if (descriptor.value()) {
+        if (!descriptor.writablePresent() || !descriptor.enumerablePresent() || !descriptor.configurablePresent()
+            || !descriptor.writable() || !descriptor.enumerable() || !descriptor.configurable()) {
+            throwError(globalObject, scope, ErrorCode::ERR_INVALID_OBJECT_DEFINE_PROPERTY, dataMsg);
+            return false;
+        }
+        return true;
+    }
+    if (descriptor.getterPresent() || descriptor.setterPresent()) {
+        throwError(globalObject, scope, ErrorCode::ERR_INVALID_OBJECT_DEFINE_PROPERTY,
+            "'process.env' does not accept an accessor(getter/setter) descriptor"_s);
+        return false;
+    }
+    throwError(globalObject, scope, ErrorCode::ERR_INVALID_OBJECT_DEFINE_PROPERTY, dataMsg);
+    return false;
+}
+
+// The regular process.env object: a plain object with the defineOwnProperty
+// override above. No instance state, so no custom subspace.
+class JSProcessEnvMap final : public JSC::JSNonFinalObject {
+public:
+    using Base = JSC::JSNonFinalObject;
+
+    static constexpr unsigned StructureFlags = Base::StructureFlags;
+
+    template<typename CellType, JSC::SubspaceAccess>
+    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<JSProcessEnvMap>(vm)) JSProcessEnvMap(vm, structure);
+        ptr->finishCreation(vm);
+        return ptr;
+    }
+
+    static bool defineOwnProperty(JSObject* object, JSGlobalObject* globalObject, JSC::PropertyName propertyName, const JSC::PropertyDescriptor& descriptor, bool shouldThrow)
+    {
+        VM& vm = JSC::getVM(globalObject);
+        auto scope = DECLARE_THROW_SCOPE(vm);
+        if (!throwIfInvalidEnvDescriptor(globalObject, scope, descriptor))
+            return false;
+        // Node's EnvDefiner delegates to EnvSetter, which coerces the value to a
+        // string; do the same so all three process.env variants match.
+        auto* string = descriptor.value().toString(globalObject);
+        RETURN_IF_EXCEPTION(scope, false);
+        PropertyDescriptor stringDescriptor(string, 0);
+        RELEASE_AND_RETURN(scope, Base::defineOwnProperty(object, globalObject, propertyName, stringDescriptor, shouldThrow));
+    }
+
+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) };
+
+JSObject* createEmptyProcessEnvMap(Zig::GlobalObject* globalObject)
+{
+    VM& vm = globalObject->vm();
+    auto* structure = JSProcessEnvMap::createStructure(vm, globalObject, globalObject->objectPrototype());
+    return JSProcessEnvMap::create(vm, structure);
+}
+
 // ============================================================================
 // worker_threads SHARE_ENV
 //
@@ -452,6 +541,11 @@ class JSSharedEnvMap final : public JSC::JSNonFinalObject {
 
 const JSC::ClassInfo JSSharedEnvMap::s_info = { "ProcessEnv"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSSharedEnvMap) };
 
+bool isProcessEnvClassInfo(const JSC::ClassInfo* info)
+{
+    return info == JSProcessEnvMap::info() || info == JSSharedEnvMap::info();
+}
+
 bool JSSharedEnvMap::getOwnPropertySlot(JSObject* object, JSGlobalObject* globalObject, PropertyName propertyName, PropertySlot& slot)
 {
     VM& vm = JSC::getVM(globalObject);
@@ -602,23 +696,11 @@ bool JSSharedEnvMap::defineOwnProperty(JSObject* object, JSGlobalObject* globalO
     VM& vm = JSC::getVM(globalObject);
     auto scope = DECLARE_THROW_SCOPE(vm);
 
+    if (!throwIfInvalidEnvDescriptor(globalObject, scope, descriptor))
+        return false;
+
     auto* uid = propertyName.uid();
-    if (propertyName.isSymbol() || !uid || !descriptor.isDataDescriptor() || !descriptor.value()) {
-        // The descriptor lands on the Base object, but getOwnPropertySlot reads the
-        // store first, so a store entry would shadow it. Move the entry onto Base as
-        // an enumerable data property first: a partial descriptor then keeps that
-        // enumerability, exactly as it does on the regular process.env. (Node rejects
-        // accessors on process.env outright — on both maps — so match bun's own map.)
-        if (!propertyName.isSymbol() && uid) {
-            if (auto* store = sharedEnvStoreFor(object)) {
-                String existing = store->get(String(uid));
-                if (!existing.isNull()) {
-                    syncWindowsEnv(store, String(uid), nullptr);
-                    store->remove(String(uid));
-                    object->putDirect(vm, propertyName, jsString(vm, existing), 0);
-                }
-            }
-        }
+    if (propertyName.isSymbol() || !uid) {
         RELEASE_AND_RETURN(scope, Base::defineOwnProperty(object, globalObject, propertyName, descriptor, shouldThrow));
     }
 
@@ -749,12 +831,8 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject)
 
     void* list;
     size_t count = Bun__getEnvCount(globalObject, &list);
-    JSC::JSObject* object = nullptr;
-    if (count < 63) {
-        object = constructEmptyObject(globalObject, globalObject->objectPrototype(), count);
-    } else {
-        object = constructEmptyObject(globalObject, globalObject->objectPrototype());
-    }
+    auto* structure = JSProcessEnvMap::createStructure(vm, globalObject, globalObject->objectPrototype());
+    JSC::JSObject* object = JSProcessEnvMap::create(vm, structure);
 
 #if OS(WINDOWS)
     JSArray* keyArray = constructEmptyArray(globalObject, nullptr, count);

src/jsc/bindings/JSEnvironmentVariableMap.h

@@ -13,6 +13,15 @@ namespace Bun {
 
 JSC::JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject);
 
+// Empty process.env for a worker handed a snapshot of the spawning thread's env:
+// same class as the ordinary map so defineOwnProperty validation applies on
+// worker threads too. Caller populates it.
+JSC::JSObject* createEmptyProcessEnvMap(Zig::GlobalObject* globalObject);
+
+// JSProcessEnvMap or JSSharedEnvMap. Both behave as plain objects for structured
+// cloning and are whitelisted at SerializedScriptValue's ObjectStartState gate.
+bool isProcessEnvClassInfo(const JSC::ClassInfo*);
+
 // worker_threads SHARE_ENV: a `process.env` whose reads/writes/enumeration go
 // through the SharedEnvStore of the tree its global belongs to.
 JSC::JSValue createSharedEnvironmentVariablesMap(Zig::GlobalObject* globalObject);

src/jsc/bindings/ZigGlobalObject.cpp

@@ -562,12 +562,17 @@ extern "C" JSC::JSGlobalObject* Zig__GlobalObject__create(void* console_client,
                     strings.append(jsString(vm, value));
                 }
 
-                auto env = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), size >= JSFinalObject::maxInlineCapacity ? JSFinalObject::maxInlineCapacity : size);
+                auto env = Bun::createEmptyProcessEnvMap(globalObject);
+                // putDirectMayBeIndex on a JSNonFinalObject dispatches index keys
+                // through the method table's defineOwnProperty, which declares a
+                // ThrowScope. This runs before topEntryFrame is set, so catch it here.
+                auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
                 size_t i = 0;
                 for (auto k : map) {
                     // They can have environment variables with numbers as keys.
                     // So we must use putDirectMayBeIndex to handle that.
                     env->putDirectMayBeIndex(globalObject, JSC::Identifier::fromString(vm, WTF::move(k.key)), strings.at(i++));
+                    scope.assertNoException();
                 }
                 globalObject->m_processEnvObject.set(vm, globalObject, env);
             } else if (options.sharedEnvStore) {

src/jsc/bindings/webcore/SerializedScriptValue.cpp

@@ -120,6 +120,7 @@
 #include "CryptoKeyType.h"
 #include "JSNodePerformanceHooksHistogram.h"
 #include "../napi.h"
+#include "../JSEnvironmentVariableMap.h"
 #include <limits>
 #include <algorithm>
 
@@ -2812,7 +2813,12 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
             // like a plain object from JS's perspective (matches Node.js).
             // ObjectPrototype is allowed because %Object.prototype% is an immutable
             // prototype exotic object that the spec carves out of this rejection.
-            if (inObject->classInfo() != JSFinalObject::info() && inObject->classInfo() != Zig::NapiPrototype::info() && inObject->classInfo() != JSC::ObjectPrototype::info())
+            // process.env (JSProcessEnvMap / JSSharedEnvMap) is a plain object whose
+            // only method-table override is defineOwnProperty; Node clones it.
+            if (inObject->classInfo() != JSFinalObject::info()
+                && inObject->classInfo() != Zig::NapiPrototype::info()
+                && inObject->classInfo() != JSC::ObjectPrototype::info()
+                && !Bun::isProcessEnvClassInfo(inObject->classInfo()))
                 return SerializationReturnCode::DataCloneError;
             inputObjectStack.append(inObject);
             indexStack.append(0);

test/js/node/process/process.test.js

@@ -218,6 +218,59 @@ it("process.env", () => {
   expect(process.env["LOL SMILE latin1 <abc>"]).toBe(undefined);
 });
 
+it("Object.defineProperty on process.env rejects accessor and partial descriptors", () => {
+  const dataMsg = "'process.env' only accepts a configurable, writable, and enumerable data descriptor";
+  const accessorMsg = "'process.env' does not accept an accessor(getter/setter) descriptor";
+  const expectThrow = (fn, message) =>
+    expect(fn).toThrow(
+      expect.objectContaining({ name: "TypeError", code: "ERR_INVALID_OBJECT_DEFINE_PROPERTY", message }),
+    );
+
+  expectThrow(() => Object.defineProperty(process.env, "goo", { get() {}, set() {} }), accessorMsg);
+  expectThrow(() => Object.defineProperty(process.env, "goo", { get() {} }), accessorMsg);
+  expectThrow(() => Object.defineProperty(process.env, "foo", { value: "foo1" }), dataMsg);
+  for (const attr of ["configurable", "writable", "enumerable"]) {
+    expectThrow(() => Object.defineProperty(process.env, "goo", { [attr]: false }), dataMsg);
+    expectThrow(
+      () =>
+        Object.defineProperty(process.env, "goo", {
+          value: "v",
+          configurable: true,
+          writable: true,
+          enumerable: true,
+          [attr]: false,
+        }),
+      dataMsg,
+    );
+  }
+  expect(process.env.foo).toBeUndefined();
+  expect(process.env.goo).toBeUndefined();
+
+  Object.defineProperty(process.env, "goo", { value: "goo", configurable: true, writable: true, enumerable: true });
+  expect(process.env.goo).toBe("goo");
+  delete process.env.goo;
+
+  // Node's EnvDefiner delegates to EnvSetter, which coerces to a string.
+  Object.defineProperty(process.env, "goo", { value: 42, configurable: true, writable: true, enumerable: true });
+  expect(process.env.goo).toBe("42");
+  expect(typeof process.env.goo).toBe("string");
+  delete process.env.goo;
+});
+
+// process.env is no longer a JSFinalObject; it must still structured-clone as
+// a plain object so postMessage / workerData keep working like Node. On Windows
+// process.env is a Proxy and the serializer rejects Proxy objects (pre-existing).
+it.skipIf(isWindows)("structuredClone(process.env) produces a plain-object snapshot", () => {
+  process.env.__SC_PROBE = "hello";
+  try {
+    const clone = structuredClone(process.env);
+    expect(clone.__SC_PROBE).toBe("hello");
+    expect(Object.getPrototypeOf(clone)).toBe(Object.prototype);
+  } finally {
+    delete process.env.__SC_PROBE;
+  }
+});
+
 it("process.env is spreadable and editable", () => {
   process.env["LOL SMILE UTF16 😂"] = "😂";
   const { "LOL SMILE UTF16 😂": lol, ...rest } = process.env;

test/js/node/test/parallel/test-process-env-ignore-getter-setter.js

@@ -0,0 +1,67 @@
+'use strict';
+require('../common');
+const assert = require('assert');
+
+assert.throws(
+  () => {
+    Object.defineProperty(process.env, 'foo', {
+      value: 'foo1'
+    });
+  },
+  {
+    code: 'ERR_INVALID_OBJECT_DEFINE_PROPERTY',
+    name: 'TypeError',
+    message: '\'process.env\' only accepts a ' +
+        'configurable, writable,' +
+        ' and enumerable data descriptor'
+  }
+);
+
+assert.strictEqual(process.env.foo, undefined);
+process.env.foo = 'foo2';
+assert.strictEqual(process.env.foo, 'foo2');
+
+assert.throws(
+  () => {
+    Object.defineProperty(process.env, 'goo', {
+      get() {
+        return 'goo';
+      },
+      set() {}
+    });
+  },
+  {
+    code: 'ERR_INVALID_OBJECT_DEFINE_PROPERTY',
+    name: 'TypeError',
+    message: '\'process.env\' does not accept an ' +
+        'accessor(getter/setter) descriptor'
+  }
+);
+
+const attributes = ['configurable', 'writable', 'enumerable'];
+
+for (const attribute of attributes) {
+  assert.throws(
+    () => {
+      Object.defineProperty(process.env, 'goo', {
+        [attribute]: false
+      });
+    },
+    {
+      code: 'ERR_INVALID_OBJECT_DEFINE_PROPERTY',
+      name: 'TypeError',
+      message: '\'process.env\' only accepts a ' +
+          'configurable, writable,' +
+          ' and enumerable data descriptor'
+    }
+  );
+}
+
+assert.strictEqual(process.env.goo, undefined);
+Object.defineProperty(process.env, 'goo', {
+  value: 'goo',
+  configurable: true,
+  writable: true,
+  enumerable: true
+});
+assert.strictEqual(process.env.goo, 'goo');