search

pierrec / node-eval

Evaluate node require() module content directly
MIT License
93 stars 16 forks source link

fix: only merge non-enumerable global properties that are not in VM scope #28

Closed Josh-Cena closed 2 years ago

Josh-Cena commented 2 years ago

Follow-up of #27. Fix #29

Case as where #27 failed:

new vm.Script("globalThis.Prism = 1; console.log(Prism)").runInNewContext({ globalThis });

Because the globalThis in this case is an exotic object, the latter Prism isn't actually made a "global".

new vm.Script("console.log({} instanceof Object)").runInNewContext({ console, Object });

Logs false because Object is exotic. Removing Object from the scope causes it to log true.

The resolution is to filter out anything that's already in the VM scope.

cc @pierrec This is a hotfix. cc @slorber FYI

Josh-Cena commented 2 years ago

would still be useful to allow excluding some globals?

I don't think that's necessarily the goal of this module and I frankly can't see how that's useful after this bug is fixed😅 Since we have a forked SSG plugin, if you really want to exclude some globals, just use includeGlobals=false and use the more verbose scope instead. WDYT?