Plugin system

[temsa]

The sandbox is not 100% useful if we cannot contribute some functions/modules to it.

I've started yesterday a fork (but did not push any code yet to github) to use broadway as a plugin system. I think I'd also like to use either node-0.6 child api or node*fork as a fallback for exchanging between sandbox and shovel.

I've notably moved the "console" available in the sandbox as a plugin. I'd also like to provide some great control over the sandbox (set cpu/memory limits, chroot, etc.) for posix platforms thanks to a plugin based on node-posix.

Are you interested in such a patch, and will you maintain the result actively, or should I fork the project, rename it and publish it on my own ?

[temsa]

I guess I'd rather use intercom for exchanging between sandbox and shovel, as it is a pretty neat communication system on top of node-0.6 / node-fork communication channel

[gf3]

Yes, patches are always welcomed and encouraged. I had considered a couple ways of loading code, but never really came around to implementing any of them. A plugin system sounds pretty cool/useful. I'd love to see how you've tackled the issue.

[rehanift]

I started working on implementing something like this ontop of @gf3's sandbox module a while back: https://github.com/rehanift/sandbox. Basically I added the ability to add/define methods to the vm.runInNewContext scope.

However, I ended up fighting more and more with using stdin/stdout as a robust communication channel and ended up re-coding the idea into a new project: https://github.com/rehanift/engine.js. It uses ZeroMQ sockets as communication channels, which I have found to be more reliable, flexible, and easier to work with. The project is still pretty young, but its fairly well-tested (unit & end-to-end).

One of the next things Im planning on doing is adding support for CommonJS style modules to be easily included into contexts. I'd be very interested in seeing what additions you're making to @gf3's sandbox and possibly porting them to engine.js

[gf3]

@rehanift engine.js looks badass, well done. Good call using a queue like ZeroMQ, that certainly helps clean up a lot of the interfacing and allows for more providers.

[rehanift]

@gf3 Thanks for the kudos. Im looking forward to writing other engine.js clients in other languages, leveraging ZeroMQ as the communication protocol.

[temsa]

@rehanift : engine looks great, I may end use it instead of what I do.

I'll still be working on sandbox (mine is already pretty usable, but with 0.6.x, because of node-fork being loaded even if not necessary and not compatible with 0.6.x because it shims it ).

@gf3 my problem is : I've change the result API to be more "node standards" like, e.g. using function(err,result) as a result callback, rather than returning an output object. Would it still be accepted as a contribution ?

You can already have a look at https://github.com/temsa/sandbox

[bmeck]

Placing this here before I do any work on plugins : we should provide wrappers to objects rather than direct access to avoid attacks. This could be done with Harmony's proxies or through a straight up direct copy (we can avoid GC and long lived processes for most situations and can address them later). Will most likely implement this with broadway.

[temsa]

I've updated a lot my repository yesterday if you want to have a look at it. (see example/example.js)

What is working:

• Running a program in the sandbox and control the error (and stack) returned to the user (thanks to stackedy). The obtained stack is really nice :)
• Having setTimeout/setInterval/clearTimeout/clearInterval in the sandbox thanks to stackedy again)
• Loading plugins both on sandbox and shovel side thanks to broadway
• Communicating with plugins thanks to events thanks to broadway and EventEmitter2
• Communicating between sandbox and shovel thanks to intercom and EventEmitter2, which should work both on node-0.4.x and node-0.6.x
• Provide a console plugin that provide a way to log from sandbox

What is NOT working yet:

• Obtaining a result from the sandbox directly (without a function provided in the sandbox), see this bug in stackedy @substack
• Timeout plugin is not working properly, but shouldn't be far, as before I did some refactoring it worked
• Dnode, which is used by intercom, has a memoryleak issue which is only cleanly fixed thanks to a network-wide garbage collection. I hope @substack will merge the pullrequest (and @isaacs too for tap-runner), but for the sandbox usage, it's not a major issue

[isaacs]

The link to tap-runner actually goes to dnode. Was there something here I should look at?

[temsa]

@isaacs the link is in the dnode pullrequest which needs this tap-runner pullrequest https://github.com/isaacs/tap-runner/pull/6

[bmeck]

@temsa we need to be very careful in allowing objects to be added to the sandbox without wrapping them in objects originating in the sandbox. Allowing people to exploit cross context vulnerabilities is a huge problem if we allow people to directly send objects between the parent context and the sandbox's child context.

[bmeck]

https://gist.github.com/1609442 is a generalized wrapper for code between a runner and something else, will integrate the wrapping to avoid leaks and rebase the plugins this weekend. Passing in eval and Function as direct properties or arguments will still be a security vulnerability, but if it comes down to it explicitly using those on non-system strings is just asking for pain. If anyone can think of a way to prevent their use I would be appreciative.

[bmeck]

http://github.com/bmeck/sandbox now has broadway installed on it without using stackedy. The wrapper has some issues with prototypes that needs resolving (for safety concerns right now they just are not sent over a context bridge) and some cache invalidation issues due to only sending over copies rather than live objects via proxies, but it should be ok for almost all uses. @temsa, do you have any objects to the stackedy includes / stack traces being a plugin?

[bmeck]

A clarification : Do not use Proxies when talking over a bridge because proxies will allow mutation at odd point in time. IE.

SANDBOX RUNS -> "var x={x:1};setTimeout(function(){x.x=2},1000);x" PARENT CACHES x=1 SANDBOX's x.x becomes 2 PARENT REFERENCES x.x instead of the cache

This can lead to odd things where functions replace value properties etc on existing references. Very hard to debug and prevent potential exploits via type coercion or currying.

[temsa]

What about using module proxying, rather than ES5 proxy?

e.g. a proxy to mikeal/request library should look like:

var request = require("request");
module.exports = function requestProxy(params, callback) {
//here check any possible arguments, like URI, etc.
//...
if(errorDetails)
 return callback(Error("Bad arguments:" + errorDetails));

//... if everything is ok, then call request, and proxy the callback too

request(params, function(err, res, body) {
  if(err) {
    // proxy error feedback for good reporting without providing any extra info
    return callback(newErr);
  }
  // ok there was no error, but we don't want to provide any extra information in the request, like current IP, etc.
  // so you should probably provide a dummy response with only whitelisted informations
  var r = {};
  r.headers = res.headers;
  //add extra stuff here...

  // body should be ok as a string
  var body = (body || '').toString();
  callback(null, r, body);
  });
}

// add extra library api here in the same way we have done for request

[bmeck]

Module proxying and wrappers are fine, it is just that Objects that cross from the sandbox context to the parent context should not be live (they are not updated when the sandbox updates them). This is done in my branch via a copy operation. I just wanted to point out why I use the copy operation rather than proxies when the copy operation is soo much slower. Generally crossing the bridge should be considered an expensive operation, but this can be avoided with a module loader like stackedy that will send it over the bridge only at the very end of a script by buffering actions until all have been aggregated (when possible).

[bmeck]

Going to finish this up this weekend and merge into master, will be a bigish API change so im going to bump a major version @gf3 if that is alright?

[gf3]

@bmeck Yes sounds good to me.

One of my primary concerns is simplicity, no matter the solution we ultimately end up with, we should try to ensure it's the most simple version possible. I think it's important that sandbox be very easy to integrate into a wide variety of projects.

[bmeck]

Gf3 ill update docs and we can discuss

[bmeck]

Fork is updated, working on docs a bit.

[temsa]

My fork has evolved a lot, and now it's ready to run, with all the features I want.

I've had to fork node-stackedy as well for all the features I wanted. Supporting asynchronous code has to be done quite manually for now, I hope I will understand enough @substack code so I can do this automagically (registering any asynchronous callback to now when the execution has finished)

The fork still needs to update the documentation and to update unit tests (I've used examples.js in order to test the features). You're welcome if you want to merge this work, if not, I guess I will publish it as an independant project, while giving you the credits for the original work

[bmeck]

@Temsa, If you get some time, lets chat on irc (im bradleymeck on freenode). I'll take a look and see if we can get the stackedy traces, but thats all that seems to be missing from my branch off of your work.

[k2xl]

Was this ever resolved? I would love to be able to run javascript commands on the child sandbox from the parent node.