The vm module has a lot of problems: https://github.com/joyent/node/issues/search?utf8=%E2%9C%93&q=vm

Furthermore, while it's useful in some scenarios, it's not suitable for what I consider a primary use case for a JavaScript virtual machine, what the JavaScript language was literally designed for: building sandboxes to run arbitrary code in a suitably low-risk manner₁, and communicate over proscribed channels with this arbitrary code (eg, via the DOM api.)

Requirements:

I consider these the requirements of a correct JavaScript vm implementation. Our current one is broken.

1. A context should be indistinguishable from the object it's based on.

   Currently:

   > var o = {}; o.o = o; vm.runInContext("this === this.o", vm.createContext(o));
   false
   > var o = vm.createContext({}); o.o = o; vm.runInContext("this === this.o", o);
   false
   

   Both should return true. This makes it unnecessarily difficult to have links to the global context within the context. As far as I can tell, this is the only way:

   > var o = vm.createContext({}); o.o = vm.runInContext("this", o);vm.runInContext("this === this.o", o);
   true
   > o.o === o
   false
   

   However, note that this breaks the link from outside. So, this blocks communication with the child context about any links that should point to the context object.

   This prevents easy simulation of either node.js or DOM environments.

2. Items added to the context from anywhere, should be immediately visible to all programs that can access that context. Example:

   var o = {foo:{bar:"baz"}}
   var s = "setInterval(function() { console.log(foo.bar) }, 100)"
   setTimeout(function () { o.foo.bar = "bloo" }, 1000)
   vm.runInContext(s, o)

   This should print "baz" a few times, and then start printing "bloo" once it's updated in the parent context.

3. As mentioned in Fix issues with v8 contexts in vm module #3039, the vm internal architecture has some problems which cause issues which the Locker object exposes when in debug mode. We need a cleaner way to know when to dispose of the context objects.

@brianmcd's contextify module exhibits the correct behavior in the cases that I've tested. However, the contextify API is not ideal: it adds properties onto the object, and requires user interaction to manually dispose of the context, making it not quite as easy to use as it could be. Also, it leaks the contextify methods into the child context:

> var x = contextify({})
undefined
> x.run("typeof getGlobal")
'function'
> x.run("typeof run")
'function'
> x.run("typeof dispose")
'function'
> x.run("dispose()")
undefined
> x.run("42")
Error: Called run() after dispose().

We should try to stay as close as we can to the current JavaScript API, but with correct semantics and resource management. Also, more of the logic should be moved out of C++ and into lib/vm.js.

1. Creating a new context: var c = vm.createContext({console:console})

2. Creating a link to the global in the context: c.global = vm.getGlobal(c)

3. Iterating over keys in a context: for (var i in c) console.log(i); // prints "console" and "global", only

4. Any changes to the global object in the child context, or to c itself in the parent context, is immediately seen in the other.

5. Disposing of a context happens automatically when the Context object is gc'ed, but can be called explicitly by vm.dispose(c)

6. Cyclical links back to the root object should be detected at the outset, and replaced with vm.getGlobal(obj)

7. Not sure how we'd handle cases where the supplied object is already the instanceof some other class. It would be acceptable for vm.createContext(obj) to throw if Object.getPrototypeOf(obj) !== Object.prototype, and simpler to deal with than trying to paper over the discrepancy. The understanding should be that it will modify the existing object in-place.

8. Not sure how we'd handle functions that return the context object directly, when called from inside the context. Consider:

   function getExternalGlobal () { return o }
   function getLocalGlobal () { return vm.getGlobal(o) }
   var o = vm.createContext({})
   o.getExternalGlobal = getExternalGlobal
   o.getLocalGlobal = getLocalGlobal
   o.externalGlobal = o
   o.localGlobal = vm.getGlobal(o)
   
   // Would be ideal if these three all returned true
   vm.runInContext("this === getExternalGlobal()", o)
   vm.runInContext("this === getLocalGlobal()", o)
   vm.runInContext("this === externalGlobal", o)
   vm.runInContext("this === localGlobal", o)

There are probably much more I'm not thinking of right now. Any fix for this should come along with a very thorough set of this-assignment edge case tests.

/cc @laverdet @piscisaureus @bnoordhuis @brianmcd @tmpvar

The history of browsers has shown us that, in fact, this is much harder than only removing access to various system APIs, but also usually requires isolating potentially nefarious code even further so that its process can be killed if it behaves badly.