Proposal: Support .env files

[GeoffreyBooth]

Spinning off from #43973 (comment), I propose that Node support loading .env files. Node would read such files as part of startup and set all defined environment variables onto process.env before any user code is evaluated; and if one of the defined variables is NODE_OPTIONS, it would be treated as if the NODE_OPTIONS environment variable had been set before the normal processing of that variable. cc @nodejs/cpp-reviewers

Motivation

This would solve nodejs/loaders#98, providing users a way to tell Node to use particular loaders via configuration file rather than CLI flag. To do so, they could create a .env file with a line like NODE_OPTIONS='--loader my-loader'. cc @nodejs/loaders Though the intent is to support all of the options allowed by NODE_OPTIONS, not just things related to loaders.

Prior art

dotenv

The dotenv module supports reading a .env file, or other file as defined by configuration. It’s enabled via a function call in user code, which would be too late for configuring Node.

Deno

Deno supports an API very similar to dotenv.

Bun

Bun supports this approach:

bun automatically reads configuration from .env.local, .env.development and .env (in that order)

We could do the same. Bun also supports config files; we could always choose add support for that in addition to reading .env files; like with Bun, I think both features are complementary.

[Trott]

I imagine we would be concerned about making this the default behavior and so we would make it opt-in. I imagine there's an attack vector where people slip in hidden .env files to trick people to talk to a malicious entity's server for example. But if we made it opt-in, it probably doesn't solve the "need to send a CLI flag to enable" problem for loaders. (Or am I failing to be sufficiently imaginative about how people might opt in?)

[Trott]

It looks like deno is opt-in via import while bun reads .env by default. But I think it's a different consideration for a new project like bun wherein there isn't a decade of people using it where it doesn't automatically read .env files. So there isn't an enormous user base to be unaware of the change. Instead, it's just how bun has always behaved (which isn't very long) and presumably the user base (which is small for now) is aware.

[Trott]

I guess maybe someone could opt in via a package.json setting maybe, but then why not specify the loaders information there rather than in .env? (Honest question, not rhetorical. There may very well be excellent reasons and I am simply unaware of all the pertinent details.)

[aduh95]

I guess maybe someone could opt in via a package.json setting maybe, but then why not specify the loaders information there rather than in .env? (Honest question, not rhetorical. There may very well be excellent reasons and I am simply unaware of all the pertinent details.)

That was also the idea I tried to express in #43973 (comment), having a { "env": "./.env" } (or { "env": { "dev": "./.env.development", "default": "./.env"} the way "exports" and "imports" work already) in the package.json seems the most logical approach. The upside on the top of my head:

• No need to create a custom config syntax for loaders (if you read the issue, you can see that almost everybody has a different idea of what that config should look like, it seems to me that it's an impossible task to find one that pleases everybody).
• The .env syntax is a well-known standard at this point.
• Defining .env variables has applications broader than just setting up loaders, hence should be more useful to our users.

All in all, I think the .env is both more desirable and more probable to succeed than trying to specify "loaders info" somehow for users.

[bnb]

I imagine there's an attack vector where people slip in hidden .env files to trick people

If someone had access to do that, wouldn't they presumably already be able to do the same thing by setting an environment variable in the system?

[Trott]

If someone had access to do that, wouldn't they presumably already be able to do the same thing by setting an environment variable in the system?

I would not assume that.

For example, perhaps an attacker exploits a bug that allows them to create a file. Perhaps the file is created by an unpriviliged user (such as user nobody on Linux) in a limited part of the filesystem. Perhaps they can't overwrite existing files. Opt-out .env consumption could be extremely useful to such an attacker.

[Trott]

Another potential concern (not necessarily a deal-breaker, but something to think about) is that this might break things for existing dotenv users in at least two ways:

1. If users are conditionally loading a .env file, then something we do (especially if it's not opt-in) may mess things up for them as they may specifically not load the .env file in some situations, and now we could be forcing it to be read.
2. It's a variation on the same thing, but if there are any differences between our treatment of .env files and those of dotenv, we may break things for people in some situations, particularly if we are not opt-in. For example, someone might check process.env.foo and assume that if present, they don't need to load .env. But if our implementation differs from dotenv, then some environment variables may be loaded differently or not at all.

[Trott]

My previous comments aside, this would be a nice convenience for users if we can do it securely and in a backward-compatible way.

[bmuenzenmeyer]

Adding my thoughts as a long-time user and maintainer of JS tooling at my company - I agree this would represent a very nice feature that I see people struggle with. dotenv is of course a great library to turn to in most of our projects, but just like watch introduced a first-party convention for a long-standing userland add-on in chokidar/nodemon/etc - I could see this in the same light.

[aduh95]

IMHO, the best UX to have it to be opt-in and user-defined via setting the relative file URL to the .env file in the package.json, following the same rules than "type": "module" that most users are probably familiar with.

[bnb]

The way dotenv works (most commonly at least) is opt-in per file. I don't think people are generally opting in per-project. This ends up having the added benefit of removing the variables defined in a .env file from any scripts that don't explicitly need them - IMO that's probably good.

[GeoffreyBooth]

The way dotenv works (most commonly at least) is opt-in per file.

All of the examples I see on https://github.com/motdotla/dotenv#readme involve setting the environment variables on process.env.

import * as dotenv from 'dotenv' // see https://github.com/motdotla/dotenv#how-do-i-use-dotenv-with-import
dotenv.config()
import express from 'express'

That’s it. process.env now has the keys and values you defined in your .env file

[aduh95]

Unless we're OK having the env variables related to enabling V8 features not working, the challenges for the implementation would be:

• how to parse efficiently package.json before V8 has started.
• how to parse the .env file before V8 has started.

My understanding is that it would require to have the implementation done in C++, but maybe there are other solutions I'm not thinking of.

[GeoffreyBooth]

I was assuming this would be opt-in via flag for now and become the default as a semver-major change, though obviously the flag opt-in is problematic; it would really only exist as the experimental phase. If we can find an opt-in like package.json that would be great, and we can still decide to make it on by default as a major change.

Another idea is to look for a special filename, like .env.opt-in-node or something obscure (with no content; this would just be a trigger to tell Node to load the .env file). Technically it would be a breaking change but practically not if the filename is obscure enough. I'm not in love with this idea but maybe it inspires an idea that's file-based but easier than parsing package.json.

[aduh95]

Another idea is to look for a special filename, like .env.opt-in-node or something obscure (with no content; this would just be a trigger to tell Node to load the .env file).

I don't think that would address the security concern, as soon as Node.js is looking for a certain file name that it wasn't before, it's an attack vector no matter if the file name is obscure or not. While skipping the JSON parsing would indeed be somewhat easier (although I'm sure we could use an existing open-source JSON parsing lib written in C++), we should be cautious with the security implications (I'm not remotely a security expert, so take what I say with a grain of salt, and by all means please correct me if I say something wrong).

[GeoffreyBooth]

Yeah the attack-vector thing is a very good point, I hadn’t considered that. So package.json is probably the best option. Then there’s the issue of choosing a key name that doesn’t conflict and itself wouldn’t somehow be an attack vector.

It feels like it shouldn’t even require a full JSON parser to do something like “find a .env key at the top level and return its value, where the value could be true or a string.” Like you could loop through every character of the JSON contents, ignoring all escaped characters and tracking open/close brackets and open/close quotes, and as soon as you find ".env" where the number of open brackets is 1, read and return the next value. I don’t know C++, but I feel like I could write this in JavaScript in under 20 lines of code, so hopefully it should be similarly simple in C++ (I wouldn’t say trivial, but not so complex that we need a library and the increased processing cost/time of parsing the entire JSON file and returning all of it).

[aduh95]

Then there’s the issue of choosing a key name that doesn’t conflict and itself wouldn’t somehow be an attack vector.

My idea to let users defined the name of the file by providing themselves the relative file-URL (or path?) to the ENV file (e.g. { "envFile": "./.env" }). That way users have control whether they choose a problematic name or not.

It feels like it shouldn’t even require a full JSON parser to do something like “find a .env key at the top level and return its value, where the value could be true or a string.”

It depends how advanced we want the feature to be. If we want something that can be affected by --conditions, it would probably need an actual JSON parsing (e.g. { "envFile": { "dev": "./.env.development", "default": "./.env" } }). I don't think there's any advantage in supporting boolean values when we could require a well defined path (the same way we don't support boolean values for "main", or "exports"), but I don't feel strongly about that.

[Qard]

Side note: you could just start a V8 isolate exclusively for the JSON parse and then tear it down again before starting the proper application environment isolate.

[bnb]

I'm not sure if this would be possible, but having a method on process - like process.loadEnvFile() - that would append the variables from .env to process.env might be an approach that's opt-in and close enough to how the current DX of dotenv works that it wouldn't be a hard change for people.

[Qard]

There are substantially more V8 options not supported.

I worry this would just draw more attention to an already confusing env var.

Also, as was just brought up, it would introduce prioritization complications--if an env var already exists, will the file take precedence? Should it merge them somehow? What if a value is made explicitly blank in the file? Would that delete a non-blank env var?

There's a lot of ways this could cause issues if it's able to handle Node.js startup configuration. Consider the policy work--a rogue .env file could make nefarious changes to security policy settings.

I would be a lot more comfortable with a more intentionally designed configuration system that's not going to risk accidental security incidents.

As stated previously, I think .env is a nice feature for user code, but I think giving it runtime configuration control is unwise.

To me, it would only be reasonable for it to have configuration control if you had to explicitly point to the .env file with a cli option: node --env-file=.env

[GeoffreyBooth]

There are substantially more V8 options not supported

I can’t think of any V8 options that I’ve ever used; I feel like most developers probably can’t, and so this wouldn’t be a concern. Regardless, that’s not really a reason not to support .env files containing NODE_OPTIONS; if people want additional flags supported in NODE_OPTIONS, that can always happen.

I worry this would just draw more attention to an already confusing env var.

I don’t find it confusing, and I don’t know why we would want to avoid ‘drawing attention’ to it. NODE_OPTIONS isn't deprecated or discouraged.

Also, as was just brought up, it would introduce prioritization complications–if an env var already exists, will the file take precedence? Should it merge them somehow? What if a value is made explicitly blank in the file? Would that delete a non-blank env var?

This is an implementation detail that we need to work out, and we should follow the pattern of how similar runtimes handle this. I would assume real environment variables take precedence and override anything in the .env file; this is how dotenv works by default.

I would be a lot more comfortable with a more intentionally designed configuration system that's not going to risk accidental security incidents.

If this is opt-in, it shouldn't present a security risk. We could ship it opt-in at first (and would have to, unless we want this to wait until Node 20) and then later on debate making it opt-out and the security implications that that would entail. By that point we'd have the shipped, working feature, and it would be easier to theorize about security risks.

As stated previously, I think .env is a nice feature for user code, but I think giving it runtime configuration control is unwise. To me, it would only be reasonable for it to have configuration control if you had to explicitly point to the .env file with a cli option: node –env-file=.env

I simply disagree with this. Users strongly want file-based runtime configuration. This is the most straightforward way to deliver that requested feature. We could additionally support a configuration file, and I would support that too, but that’s a much bigger design and development effort.

[Qard]

Yes, users do want file-based runtime configuration, but I doubt many would find this a great way to do it. Most of Node.js configurations would only be usable through NODE_OPTIONS which would make for one really big and hard to read line if they want to configure a bunch of things. The user experience for considering this as a runtime configuration tool is not great. Perhaps if you followed this up with making specific env var equivalents of many of the popular cli flags it would be less awkward?

Like I said, not going to block this, just want to communicate that it sounds to me like a very awkward way to configure the runtime and there's a lot of security and compatibility concerns to carefully consider. As long as those concerns are heard and considered, I'm satisfied.

[GeoffreyBooth]

specific env var equivalents of many of the popular cli flags

This is a good idea. We already have 24 supported environment variables, including a few that are parallels to flags like NODE_NO_WARNINGS. We could simply expand this list to include more of the popular ones, like NODE_INSPECT and NODE_LOADER. We would still need .env file support to allow specifying these via a file, but it would allow more granular configuration/overrides and a shorter NODE_OPTIONS. (I still think we would need to support NODE_OPTIONS as listed above in addition to any new variables, as it would be unexpected that these variables would work but NODE_OPTIONS wouldn’t.)

[Qard]

Yeah, I would definitely say it should be all or nothing with config support--either support everything properly, or don't include any config support there at all.

[bmeck]

Just piping in that the security policy integrity check passed via CLI wouldn't let policies be tampered with. You have to pass that in anyway for safety reasons and it wouldn't allow tampering even if .env is supported due to CLI precedence and failure if specified multiple times.

…

On Fri, Oct 14, 2022, 1:45 AM Stephen Belanger ***@***.***> wrote: There are substantially more V8 options not supported. I worry this would just draw more attention to an already confusing env var. Also, as was just brought up, it would introduce prioritization complications--if an env var already exists, will the file take precedence? Should it merge them somehow? What if a value is made explicitly blank in the file? Would that delete a non-blank env var? There's a lot of ways this could cause issues if it's able to handle Node.js startup configuration. Consider the policy work--a rogue .env file could make nefarious changes to security policy settings. I would be a lot more comfortable with a more intentionally designed configuration system that's not going to risk accidental security incidents. As stated previously, I think .env is a nice feature for user code, but I think giving it runtime configuration control is unwise. To me, it would only be reasonable for it to have configuration control if you had to explicitly point to the .env file with a cli option: node --env-file=.env — Reply to this email directly, view it on GitHub <#44975 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABZJI7Z5ZRVZ3MZRE6JGKLWDD6OZANCNFSM6AAAAAARC35O5I> . You are receiving this because you are on a team that was mentioned.Message ID: ***@***.***>

[bmeck]

Ah I would also note that for policies we would want to be able to integrity check these .env files but that is a simple port back to c++ for policies. Not an issue, just some work, and probably shouldn't be blocking.

…

On Fri, Oct 14, 2022, 9:34 AM Bradley Farias ***@***.***> wrote: Just piping in that the security policy integrity check passed via CLI wouldn't let policies be tampered with. You have to pass that in anyway for safety reasons and it wouldn't allow tampering even if .env is supported due to CLI precedence and failure if specified multiple times. On Fri, Oct 14, 2022, 1:45 AM Stephen Belanger ***@***.***> wrote: > There are substantially more V8 options not supported. > > I worry this would just draw more attention to an already confusing env > var. > > Also, as was just brought up, it would introduce prioritization > complications--if an env var already exists, will the file take precedence? > Should it merge them somehow? What if a value is made explicitly blank in > the file? Would that delete a non-blank env var? > > There's a lot of ways this could cause issues if it's able to handle > Node.js startup configuration. Consider the policy work--a rogue .env file > could make nefarious changes to security policy settings. > > I would be a lot more comfortable with a more intentionally designed > configuration system that's not going to risk accidental security incidents. > > As stated previously, I think .env is a nice feature for user code, but I > think giving it runtime configuration control is unwise. > > To me, it would only be reasonable for it to have configuration control > if you had to explicitly point to the .env file with a cli option: node > --env-file=.env > > — > Reply to this email directly, view it on GitHub > <#44975 (reply in thread)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AABZJI7Z5ZRVZ3MZRE6JGKLWDD6OZANCNFSM6AAAAAARC35O5I> > . > You are receiving this because you are on a team that was mentioned.Message > ID: ***@***.***> >

[arhart]

What if this were

1. Opt in per node script (only the main script, not dependencies)
2. Somehow part of the file, not (or not only) an environment variable, command line option, file in a well known location, etc

I think the first is needed to avoid introducing security problems into existing scripts.

I think the second is needed to be robust for options which need loaded early (like policy and custom loaders) while also being robust when node is launched as a child process where the parent may not already pass the desired environment or command line option.

How minimally ugly could we make something like that? Options:

1. (Partially) Parse the JavaScript and look for some string directive (like how "use strict" enables strict mode)
2. Look for some file extended attribute, fork, or alternate data stream
3. ?

Something like that could be used for the "ambient configuration" mentioned in #43828

[Qard]

Could bake in support for the // Flags: comment we use in tests. 🤔

[ralyodio]

should have one that's committed and one that isn't.

[GeoffreyBooth]

This is supported now via --env-file.

[aladdin-add]

Is it possible to use dot-env with node.js clis? let's say eslint, I tried NODE_OPTIONS:

$ NODE_OPTIONS="--env-file=.env.local" eslint some-file

and getting the error: node: --env-file= is not allowed in NODE_OPTIONS