nakama
nakama copied to clipboard
heroiclabs
Ensure that JavaScript code accessing `process.env` doesn't immediatly crash the VM
As discussed on the forums: This is a very barebones "solution" to deal with JavaScript libraries that expect to read process.env. The related pull request for goja is https://github.com/dop251/goja_nodejs/pull/33
On a technical level this pulls in goja_nodejs as another dependency and enables the process support. So this common pattern to check whether code is run in a production enviroment will sort of work in goja as well. I didn't manage to run the unit tests locally, but locally everything worked fine with the following snippet:
'use strict'
function InitModule(ctx, logger, nk, initializer) {
if (process.env.NODE_ENV === 'production') {
logger.info('Careful, we are in PRODUCTION');
} else {
logger.info('Not in Production.');
}
}
// Reference InitModule to avoid it getting removed on build
!InitModule && InitModule.bind(null);
If I called this using NODE_ENV=production ./nakama --runtime.js_entrypoint "./runtime-test.js"I got the production log, otherwise the other log. Preliminary tests show that this addition allows me to get rid of a rollup plugin that rewrites these kind of checks.
Oh, forgot to ping @lugehorsam as per his request :slightly_smiling_face:
Ping @lugehorsam , I would really like to pull this through and possibly investigate whether even the require layer of goja_nodejs could be "properly" used in nakama.
Hey @MarcusRiemer we are currently evaluating whether this change will conflict with our goal of the JS runtime being entirely sandboxed.
Oh, actually I meant that it would be reasonable to support some of NodeJS APIs at some point ha-ha Thank you for the answer
Fair enough, I can see the sandboxing argument. I would argue that the exact same data is already exposed via runtime.env by Nakama itself, so technically it might also be feasible to do this entirely on the JavaScript layer by setting it as an alias. This would also mitigate the problem, that those two objects could go out of sync.
@MarcusRiemer - in addition to what Luke mentioned, this PR will open up more confusion on what Node compat exists in Nakama, and what Node libraries can be used.
Additionally, our official recommendation for reading env variables (even in Go and Lua) would be to pass through the data via -runtime.env config/command-line flags.
I'm totally on board with this recommendation for code specifically written for Nakama, but most code isn't. And is that confusion necessarily "bad", as long as the compatibility increases over time? I mean technically it's also bad right now because the Nakama documentation doesn't really give much indication about the available JavaScript or node.js features in a structured way.
From my PoV it would be great to have as much of the node.js ecosystem readily available as possible. And I think that a tiny amount of node.js compatability (like supporting process.env) can have a huge impact on the amount of packages available. At least for my personal project this is kind of imperative: It started out as a home grown thing based on socket.io and I would now like to port it to Nakama.
@MarcusRiemer How much of the process surface area would you determine to be a "tiny amount" with a huge impact? I think part of the challenge we have is that it's a slippery slope. The surface area of the process object as an example is huge: https://nodejs.org/api/process.html
Where would we draw the line?
I would draw the line with two goalposts:
- Whatever people are willing to contribute upstream, so that Nakama doesn't have to maintain (or at least not directly). This is why I went to goja_nodejs first.
- It must not compromise the integrity of the sandbox. So this throws things like IO (no matter whether via stdin/stdout, files or network) immediately out of the window.
Measuring the impact could be done by sampling popular packages, throwing their source code at nakama and see what crashes. The next thing on my list would be console.log (which admittedly should be implemented in a Nakama specific way) and then at least my personal stack should be up and running inside Nakama / goja.
All that being said: I do get the hesitation about putting this into the go-core. Thankfully both of these issues (process.env and console.log) could be mitigated solemnly in JavaScript as well. Would you be open to provide an option to load shim files once per VM before the "normal" user space code is loaded? Then I (or someone else) could provide some sort of "best effort" compatability layer that is allowed to set globals and can be inspected by every "normal" JavaScript developer which should also reduce confusion. Something along the lines of this:
// Result must be an object and is merged with the global JS object in goja,
// all "normal" code after this can access `process.env` or `console.log` directly.
function InitShim(ctx) {
const window = {
process: {
env: {}
},
console: {
log: function() {
ctx.logger.info(...arguments)
}
}
}
Object.values(ctx.env).forEach(([k,v]) => window.process.env[k] = v);
return (window);
}
I personally would still prefer to do this in the core and to later on also enable things like require / import without resorting to a JavaScript build tool. But maybe this proposal is a good middle ground to solve the "easy" cases I am trying to advocate for.