Skip to content

vm: properly support symbols on globals

Rodrigo Muino Tomonari requested to merge github/fork/dubzzz/fix-global-access-issue into main

A regression has been introduced in node 18.2.0, it lakes the following snippet fails while it used to work in the past:

const assert = require('assert');
const vm = require('vm');
const global = vm.runInContext('this', vm.createContext());
const totoSymbol = Symbol.for('toto');
Object.defineProperty(global, totoSymbol, {
  enumerable: true,
  writable: true,
  value: 4,
  configurable: true,
});
assert(Object.getOwnPropertySymbols(global).includes(totoSymbol));

The PR that introduced the regression is: https://github.com/nodejs/node/pull/42963. So I basically attempted to start understanding what it changed to make it still fix the initial issue while not breaking the symbol related one.

Fixes: https://github.com/nodejs/node/issues/45983

Merge request reports