From 7d0a0ac30c04d9dd79468d48520241e73a84139f Mon Sep 17 00:00:00 2001
From: robobun <117481402+robobun@users.noreply.github.com>
Date: Mon, 13 Jul 2026 18:54:08 +0000
Subject: [PATCH 1/2] node:vm: block sloppy [[Set]] on read-only contextified
 globals

NodeVMGlobalObject::put forwarded every store to the sandbox before
consulting the global object's own attributes, so a sloppy-mode
`globalThis.NaN = 123` inside a vm context created a fresh writable
`NaN` on the sandbox that shadowed the non-writable global on the next
read. The strict-mode path threw the expected TypeError but only after
the sandbox had already been mutated.

Mirror the guard that defineOwnProperty already has and that Node's
contextify PropertySetterCallback applies: if the property exists on the
global object as ReadOnly, route straight to Base::put so the ordinary
sloppy no-op / strict TypeError semantics apply and the sandbox is left
untouched.
---
 src/jsc/bindings/NodeVM.cpp | 13 +++++++++++++
 test/js/node/vm/vm.test.ts  | 25 +++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/src/jsc/bindings/NodeVM.cpp b/src/jsc/bindings/NodeVM.cpp
index fd006bcc3ad1..ee1a27618fa4 100644
--- a/src/jsc/bindings/NodeVM.cpp
+++ b/src/jsc/bindings/NodeVM.cpp
@@ -1086,6 +1086,19 @@ bool NodeVMGlobalObject::put(JSCell* cell, JSGlobalObject* globalObject, Propert
     }
     bool isDeclaredOnGlobalObject = slot.type() == JSC::PutPropertySlot::NewProperty;
     auto scope = DECLARE_THROW_SCOPE(vm);
+
+    // If the property already exists on the global object as read-only, do not
+    // forward the store to the sandbox. Matches Node's contextify
+    // PropertySetterCallback, which intercepts and returns early on ReadOnly.
+    {
+        PropertySlot existingSlot(thisObject, PropertySlot::InternalMethodType::GetOwnProperty, nullptr);
+        bool existsOnGlobal = thisObject->JSC::JSGlobalObject::getOwnPropertySlot(thisObject, globalObject, propertyName, existingSlot);
+        RETURN_IF_EXCEPTION(scope, false);
+        if (existsOnGlobal && (existingSlot.attributes() & PropertyAttribute::ReadOnly) != 0) {
+            RELEASE_AND_RETURN(scope, Base::put(cell, globalObject, propertyName, value, slot));
+        }
+    }
+
     PropertySlot getter(sandbox, PropertySlot::InternalMethodType::Get, nullptr);
     bool isDeclaredOnSandbox = sandbox->getPropertySlot(globalObject, propertyName, getter);
     RETURN_IF_EXCEPTION(scope, false);
diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts
index 296971c440be..127143510617 100644
--- a/test/js/node/vm/vm.test.ts
+++ b/test/js/node/vm/vm.test.ts
@@ -420,6 +420,31 @@ function testRunInContext({ fn, isIsolated, isNew }: TestRunInContextArg) {
       expect(context.baz).toEqual([undefined, "b", "c"]);
       expect(result).toBe(true);
     });
+    test("sloppy assignment to read-only globals is a no-op", () => {
+      const context = createContext({});
+      for (const name of ["NaN", "undefined", "Infinity"]) {
+        const result = fn(
+          `(function () {` +
+            `globalThis[${JSON.stringify(name)}] = 123;` +
+            `var d = Object.getOwnPropertyDescriptor(globalThis, ${JSON.stringify(name)});` +
+            `return { value: String(globalThis[${JSON.stringify(name)}]), writable: d.writable, configurable: d.configurable };` +
+            `})()`,
+          context,
+        );
+        expect({ name, ...result }).toEqual({ name, value: name, writable: false, configurable: false });
+      }
+      expect(Object.keys(context)).toEqual([]);
+    });
+    test("strict assignment to read-only globals throws and does not mutate the sandbox", () => {
+      const context = createContext({});
+      const result = fn(
+        "(function(){'use strict'; try { globalThis.NaN = 5; return 'set'; } catch (e) { return e.constructor.name; } })()",
+        context,
+      );
+      expect(result).toBe("TypeError");
+      expect(fn("String(globalThis.NaN)", context)).toBe("NaN");
+      expect(Object.keys(context)).toEqual([]);
+    });
     test("cannot access `process`", () => {
       const context = createContext({});
       const result = fn("typeof process;", context);

From 20b8ba12bd0d8b76c450b6258c7ea8c9fa158feb Mon Sep 17 00:00:00 2001
From: robobun <117481402+robobun@users.noreply.github.com>
Date: Mon, 13 Jul 2026 18:59:39 +0000
Subject: [PATCH 2/2] test: assert sandbox own-property names, not just
 enumerable keys

---
 test/js/node/vm/vm.test.ts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts
index 127143510617..3d0e2a40ac5d 100644
--- a/test/js/node/vm/vm.test.ts
+++ b/test/js/node/vm/vm.test.ts
@@ -432,8 +432,9 @@ function testRunInContext({ fn, isIsolated, isNew }: TestRunInContextArg) {
           context,
         );
         expect({ name, ...result }).toEqual({ name, value: name, writable: false, configurable: false });
+        expect(Object.prototype.hasOwnProperty.call(context, name)).toBe(false);
       }
-      expect(Object.keys(context)).toEqual([]);
+      expect(Object.getOwnPropertyNames(context)).toEqual([]);
     });
     test("strict assignment to read-only globals throws and does not mutate the sandbox", () => {
       const context = createContext({});
@@ -443,7 +444,8 @@ function testRunInContext({ fn, isIsolated, isNew }: TestRunInContextArg) {
       );
       expect(result).toBe("TypeError");
       expect(fn("String(globalThis.NaN)", context)).toBe("NaN");
-      expect(Object.keys(context)).toEqual([]);
+      expect(Object.prototype.hasOwnProperty.call(context, "NaN")).toBe(false);
+      expect(Object.getOwnPropertyNames(context)).toEqual([]);
     });
     test("cannot access `process`", () => {
       const context = createContext({});