From 3fb03c6fe381af078f90e9e4212ff69c210fd2fb Mon Sep 17 00:00:00 2001
From: Chengzhong Wu <legendecas@gmail.com>
Date: Sun, 24 May 2026 17:19:30 -0400
Subject: [PATCH] src: fix ContextifyContext property definer interception
 result

Signed-off-by: Chengzhong Wu <legendecas@gmail.com>
---
 src/node_contextify.cc                        | 17 +++++++------
 .../test-vm-property-definer-interception.js  | 24 +++++++++++++++++++
 2 files changed, 32 insertions(+), 9 deletions(-)
 create mode 100644 test/parallel/test-vm-property-definer-interception.js

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 14534ab89650b4..f319420ae02f35 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -700,8 +700,7 @@ Intercepted ContextifyContext::PropertyDefinerCallback(
         if (desc.has_configurable()) {
           desc_for_sandbox->set_configurable(desc.configurable());
         }
-        // Set the property on the sandbox.
-        USE(sandbox->DefineProperty(context, property, *desc_for_sandbox));
+        return sandbox->DefineProperty(context, property, *desc_for_sandbox);
       };
 
   if (desc.has_get() || desc.has_set()) {
@@ -709,23 +708,23 @@ Intercepted ContextifyContext::PropertyDefinerCallback(
         desc.has_get() ? desc.get() : Undefined(isolate).As<Value>(),
         desc.has_set() ? desc.set() : Undefined(isolate).As<Value>());
 
-    define_prop_on_sandbox(&desc_for_sandbox);
-    // TODO(https://github.com/nodejs/node/issues/52634): this should return
-    // kYes to behave according to the expected semantics.
+    if (define_prop_on_sandbox(&desc_for_sandbox).FromMaybe(false))
+      return Intercepted::kYes;
     return Intercepted::kNo;
   } else {
     Local<Value> value =
         desc.has_value() ? desc.value() : Undefined(isolate).As<Value>();
 
+    Maybe<bool> result;
     if (desc.has_writable()) {
       PropertyDescriptor desc_for_sandbox(value, desc.writable());
-      define_prop_on_sandbox(&desc_for_sandbox);
+      result = define_prop_on_sandbox(&desc_for_sandbox);
     } else {
       PropertyDescriptor desc_for_sandbox(value);
-      define_prop_on_sandbox(&desc_for_sandbox);
+      result = define_prop_on_sandbox(&desc_for_sandbox);
     }
-    // TODO(https://github.com/nodejs/node/issues/52634): this should return
-    // kYes to behave according to the expected semantics.
+
+    if (result.FromMaybe(false)) return Intercepted::kYes;
     return Intercepted::kNo;
   }
 }
diff --git a/test/parallel/test-vm-property-definer-interception.js b/test/parallel/test-vm-property-definer-interception.js
new file mode 100644
index 00000000000000..9e59d9ffd603e2
--- /dev/null
+++ b/test/parallel/test-vm-property-definer-interception.js
@@ -0,0 +1,24 @@
+'use strict';
+
+require('../common');
+const vm = require('vm');
+const assert = require('assert');
+
+// Each [[DefineOwnProperty]] intercepted by the definer should invoke the
+// sandbox's [[DefineOwnProperty]] exactly once.
+{
+  let count = 0;
+  const sandbox = new Proxy({}, {
+    defineProperty(target, key, desc) {
+      count++;
+      return Reflect.defineProperty(target, key, desc);
+    },
+  });
+  const ctx = vm.createContext(sandbox);
+  vm.runInContext(`
+    Object.defineProperty(this, 'a', { value: 1 });
+    Object.defineProperty(this, 'b', { value: 2, writable: true });
+    Object.defineProperty(this, 'c', { get() { return 3; } });
+  `, ctx);
+  assert.strictEqual(count, 3);
+}