asvd / jailed

execute untrusted code with custom permissions
MIT License
1k stars 73 forks source link

Make it work in webpack #22

Closed cristiano-belloni closed 8 years ago

cristiano-belloni commented 8 years ago

Webpack's static analysis breaks jailed: it's probably because child_process is required dynamically. Anyway, would it be possible to follow this to make it work in the browser?

Just adding:

browser: { fs: false, child_process: false } to the package.json would do the trick

asvd commented 8 years ago

I am not familiar with Webpack. Jailed checks if window is defined to determine if the environment is node or browser. Can you please make sure that the application is properly assembled with such a workaround and initializes a plugin in a sandboxed iframe?

cristiano-belloni commented 8 years ago

Hello @asvd , This is not a runtime issue. At runtime, jailed recognizes the right environment and works just fine. This is a Webpack "compilation" issue, which happens before runtime.

When Webpack does a static analysis to substitute the require statements with real modules, it can't recognize dynamic requires, like here: https://github.com/asvd/jailed/blob/1682cdb58dea8d6f98554ccf34a433e8cc686da0/lib/jailed.js#L192.

This causes the compilation to fail.

Generally, with packages that run indifferently on the client and the server side, it's good to give an hint that some modules your package requires are and will always be unavailable on the browser. There's a field in the package.json for that, as described here: https://github.com/defunctzombie/package-browser-field-spec

That's why I suggested to add the browser: { fs: false, child_process: false } line - it gives info to whoever is trying to compile your package to not bother with those dependencies in browser land.

cristiano-belloni commented 8 years ago

(I'm already running it with a workaround, by the way - but it would be nice for jailed to run out-of-the-box with webpack for everyone).

asvd commented 8 years ago

Yep, this is how I understood the webpack works. I just needed you to confirm that after the compilation jailed properly works in web-environment recognizing it on its own (otherwise the proposed workaround would make no sence).

Will update the package.json hopely this evening, thanks for the contribution. (optionally you could submit a pull-request ;-))