Proposal: --config=file command line argument

[jasnell]

In a number of conversations (e.g. #11888), the idea of introducing either a NODEOPT environment variable that would allow arbitrary command line flags to be set via the environment, or a .noderc configuration file that could provide configuration options has been discussed. These choices have their merits and their disadvantages. One of the key disadvantages for me is the potential for the configuration collisions across applications and spooky action coming from environment settings that are unknown to the user. I much prefer explicit opt-in solutions.

One approach that could work that can potentially fill the gap is allowing configuration settings to be set using a .noderc type configuration file that is explicitly passed in via command line argument:

$ node --config=/some/path/.noderc app.js

If a user wanted to make use of environment variables in various environments, they could easily do so via variable expansion:

$ export NODEOPT=/some/path/.noderc_production
$ node --config=${NODEOPT}

[jchip]

The key requirement I wanted to address with #11888 is being able to do the preload even in node invocations I didn't start explicitly.

For example, if I do node -r ./preload.js app.js, and app.js in turn invokes node, then that would not have the effects of preload.js.

How can I get that with this approach?

[addaleax]

I’d really like it if we could set env vars and flags in (almost) 1:1 correspondence. That would make our API more consistent and give users control over which node processes they want to target.

Also, I’m not really a fan of adding a config file for Node, except maybe for the REPL (because that’s an actual application that happens to ship with the platform).

[mscdex]

Using an environment variable for each flag might cause issues/headaches if a node process forks, since process.env is used as the default environment and some people may not be expecting flags to leak into such child processes. *shrug*

I realize there are already other environment variables that could have similar issues, but I think adding a new one per flag could make it worse?

[addaleax]

@mscdex That’s more or less what I meant – people can use environment variables if they want that recursive behaviour, and flags otherwise. Imho that’s a pretty traditional approach, and one of the primary reasons environment variables exist.

[jasnell]

The key problem I have with that approach is that the environment variables would have an effect even if the users don't want to use them, and many users may not be aware of them at all. For some environment variables (like NODE_DISABLE_COLORS) that's not a big deal at all. For others (like the proposed NODE_PRELOAD) it could have huge unforeseen impact. It's too indiscriminate to allow all flags to be set that way -- note that there are several flags that we explicitly decided would not be usable as environment variables specifically because of security concerns. It's far too easy for untrusted code to set environment variables in ways the average user typically would never notice.

[addaleax]

note that there are several flags that we explicitly decided would not be usable as environment variables specifically because of security concerns

Right, that’s why I said “almost” – I can see that there are valid reasons for exceptions. I don’t think NODE_PRELOAD should be such an exception, though.

It's far too easy for untrusted code to set environment variables in ways the average user typically would never notice.

I’d be really curious to a scenario in which this is true… if you allow untrusted code to run and set arbitrary environment variables, it seems to me you’d have a problem anyway (messing with HOME, SHELL, LD_PRELOAD, etc.).

[jasnell]

I'd be really curious to a scenario in which this is true

One scenario is a variation of this: http://blog.npmjs.org/post/141702881055/package-install-scripts-vulnerability ... Install scripts run under the users identity and permissions and have the ability to modify both file system and environment variables in persistent ways that may not be detected. Yes, users can turn off install scripts per the vulnerability report, but how many users know about that and how many actually do turn them off? Sure, this is not a problem that is specific to the use of environment variables and there are all sorts of other risks and vulnerabilities this would allow, but it's one way that a NODE_PRELOAD environment variable could be abused to inject malicious code in every Node.js process running under a user's account. I don't think we should be making it any easier.

[jasnell]

(and yes, the install script problem generally applies to all environment variables including HOMe, SHELL, LD_PRELOAD, etc... which is why install scripts really ought to be turned off by default)

[jchip]

@jasnell for the security concern, I'd like to mention what I did in #11888, where NODE_PRELOAD can only specify modules from the installed Node's global node_modules. I know you said you are stewing over it. I wonder if that's an option opened for discussion?

[jasnell]

Yeah, I haven't completely ruled it out, by any means. I'm still exploring and kicking around ideas. My key concern here is that I don't want to make it easier for attackers. I definitely not saying that this would introduce a new security risk but I'm afraid that it could make an existing one easier to exploit. So I'm wanting to be very careful in how we proceed.

[sam-github]

I don't object to this proposal, but it doesn't address the use-case that NODEOPT="-r node-report" or NODE_PRELOAD=node-report does.

This use-case requires the developer of the code that runs node to pre-anticipate that they want runtime configuration, and to implement it by writing their start script appropriately. They could do this already, actually, with some variant of:

scripts: {
  "start": "node $(cat ~/.nodeopt) server/server.js"
}

However, that doesn't solve my use-cases.

The problem I've run into (not just once, but over-and-over again, with strong-pm/strong-supervisor, with several indepent projects at multiple companies building docker containers for node servers, when webexed into customer sites at 3am, when looking at building tooling for running node apps in the cloud, etc.) is how to start and configure thirdparty apps, apps that I didn't write, that weren't written to be runtime configurable, and where I can't programmatically rewrite their start scripts. This --config suggestion, and all the "just hand modify the app's start script" suggestions are not possible under these conditions.

There is only one convention for starting node servers, and that is npm start: https://docs.npmjs.com/cli/start

What this means in practice is that node server's aren't directly runnable by node... they are actually shell scripts. The script may be quite simple ("node --abort-on-uncaught-exception server/server.js"), or might be more complex ("./start.sh").

This means that tooling that packages and deploys generic node servers, ones that are written in the conventional way, but have not installed any production tooling, and have not specially written their start script, have no way of being customized in production. I believe this is one cause of the poor production debug and deploy tooling situation for node ATM.

#2738 and strongloop/strong-supervisor#56 (comment) for example, --max-old-space is something a cloud environment might want to set, and could do with NODEOPT=--max-old-space=XXX, but at the moment our only choice is to exhaustively tell every single app author about the CLI opt, and how to tune it.

"NODEOPT=--abort-on-uncaught-exception" would also be useful.

Deployment tools aren't the only use-case, @jasnell (I think) mentioned how something like NODEOPT could surprisingly apply to node process spawned by npm life-cycle scripts. Its true, it could surprise people. However, life-cycle scripts can literally NOT be modified by a user, if I do npm install, and in the middle of it one of the install scripts spawns a node child, and its dying, and I want to inject node-report, because something is happening that I can't reproduce when running the package script standalone, I'm currently dead in the water. npm downloaded the package and ran the script, I don't have any way to modify the script before npm runs it. Being able to do NODEOPT="-rnode-report --abort-on-uncaught-exception" would give me control. Spooky action at a distance? Sure, it is, but when you can't get close to the misbehaving code, you need action-at-a-distance.

I don't think every node option should be valid in NODEOPT (-e makes no sense), but right now we are doing a one-by-one addition of env vars.

More related issues (non-exhaustive, just from my notes):

• src: command line option to preload modules #881 cc: @ofrobots
• Add NODE_PRESERVE_SYMLINKS environment variable #8749
• Stack Trace on Depreciation Warnings #9516 (comment)
• --tls-cipher-list can only be set by CLI, would be useful to have an env var equiv
• --trace-opt/deopt would be great to inject, perf, etc...

[jasnell]

After speaking with a few folks about it in detail, I'm going to drop my general objection to NODEOPT but still definitely prefer it over the NODE_PRELOAD proposal.

My preference would be for:

• Any security-related command line flag (such as --tls-cipher-list and the --revert= flags) to be unsupported in NODEOPT by default, perhaps with an explicit opt in that would allow those to be used. (The --tls-cipher-list flag was one of those that was specifically limited to command line when it was introduced).
• child_process to require an explicit opt-in in order to automatically inherit NODEOPT.
• For Node.js embedders (e.g. electron) to explicitly be required to opt-in to NODEOPT support.