src: fix vm bug for configurable globalThis

Object.defineProperty allows to change the value for
non-writable properties if they are configurable. We
missed that case when checking if a
property is read-only.

Fixes: #47799
PR-URL: #51602
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

src/node_contextify.cc

@@ -609,11 +609,14 @@ void ContextifyContext::PropertyDefinerCallback(
   bool read_only =
       static_cast<int>(attributes) &
           static_cast<int>(PropertyAttribute::ReadOnly);
+  bool dont_delete = static_cast<int>(attributes) &
+                     static_cast<int>(PropertyAttribute::DontDelete);
 
-  // If the property is set on the global as read_only, don't change it on
-  // the global or sandbox.
-  if (is_declared && read_only)
+  // If the property is set on the global as neither writable nor
+  // configurable, don't change it on the global or sandbox.
+  if (is_declared && read_only && dont_delete) {
     return;
+  }
 
   Local<Object> sandbox = ctx->sandbox();
 

test/parallel/test-vm-global-configurable-properties.js

@@ -0,0 +1,15 @@
+'use strict';
+// https://github.com/nodejs/node/issues/47799
+
+require('../common');
+const assert = require('assert');
+const vm = require('vm');
+
+const ctx = vm.createContext();
+
+const window = vm.runInContext('this', ctx);
+
+Object.defineProperty(window, 'x', { value: '1', configurable: true });
+assert.strictEqual(window.x, '1');
+Object.defineProperty(window, 'x', { value: '2', configurable: true });
+assert.strictEqual(window.x, '2');