From e386ab0dd17b640a2ca6f33111fa805dc403be36 Mon Sep 17 00:00:00 2001
From: "F. Hinkelmann" <franziska.hinkelmann@gmail.com>
Date: Thu, 1 Feb 2024 04:12:15 -0500
Subject: [PATCH] src: fix vm bug for configurable globalThis
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: https://github.com/nodejs/node/issues/47799
PR-URL: https://github.com/nodejs/node/pull/51602
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
---
 src/node_contextify.cc                            |  9 ++++++---
 .../test-vm-global-configurable-properties.js     | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)
 create mode 100644 test/parallel/test-vm-global-configurable-properties.js

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 23b87657cee1bb..d22ca507614b1a 100644
--- a/src/node_contextify.cc
+++ b/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();
 
diff --git a/test/parallel/test-vm-global-configurable-properties.js b/test/parallel/test-vm-global-configurable-properties.js
new file mode 100644
index 00000000000000..4428e747eae36d
--- /dev/null
+++ b/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');