Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using a better env file library #91

Open
sholladay opened this issue Jul 19, 2020 · 8 comments
Open

Consider using a better env file library #91

sholladay opened this issue Jul 19, 2020 · 8 comments

Comments

@sholladay
Copy link

The boilerplate currently uses the dotenv package, which is less than ideal because it's easy to make mistakes with it and accidentally commit secrets to the git repository, among other issues.

It would be great if the boilerplate could use a safer library, such as envy or dotenv-safe, which provide various levels of validation.

@devinivy
Copy link
Member

I am actually a fan of envy. If we did decide to go that direction, could we contribute to update envy's tests for some recent versions of node? (I realize even dotenv's test suite is a little bit behind too!) What do others think?

Here are some additional thoughts:

  • I don't see how it is easier to commit secrets with our setup versus envy's or dotenv-safe's. We identically gitignore server/.env and keep a server/.env-keep as an example file. Do dotenv-safe and envy confirm that the file is gitignored at runtime, or something along those lines?
  • The main difference with dotenv-safe seems to be that it ensures the .env file and the example file are in sync, and is stricter about the example file's contents being present in the environment.
  • The main difference with envy (aside from not mutating the environment) seems to be that you only have access to the variables that you explicitly opt-in to using the .env file and example file.
  • I prefer envy's approach to dotenv-safe because it isn't as pedantic about .env and the example file matching each other, and makes more room for defaults to be applied within the app. I also like that envy doesn't modify the environment itself, which reinforces that all configuration for the app should be determined in/near the app's entrypoint and passed down as e.g. plugin options.
  • I appreciate that dotenv does not have any dependencies, and dotenv-safe only depends on dotenv. On the other hand, envy has multiple dependencies. I do aim to keep the boilerplate dependencies as light as possible in order to introduce minimal transitive dependencies outside of the hapi ecosystem, so it is a consideration.
  • I like the idea of using a dependency that is more related to the hapi community :)

@sholladay
Copy link
Author

sholladay commented Jul 21, 2020

I created envy after a relatively junior developer at my company accidentally committed sensitive secrets, including database credentials, and pushed them to the git repo, where they were subsequently seen by people who shouldn't have had access to them. I then spent all day rolling passwords and API keys, combing through logs to check for misuse, and educating people on what happened, etc. It was a bad time and I set out to ensure that would not happen again (and it hasn't).

The workflow went something like this:

  1. I sent them the credentials on an end-to-end encrypted messaging system with a brief description of what to do with them
  2. They put the credentials in .env.example instead of .env, apparently because they misunderstood its purpose
  3. They ran cp .env.example .env after realizing their mistake
  4. They started the app, it loaded successfully, and they proceeded with their development work, thinking nothing of it
  5. At some point later, they did git commit -a and git push, followed by a PR

I prefer to look at this not as a problem with the developer but as a problem with the system. It was too easy to make this mistake. In some sense, this is actually a failing of the .env.example pattern. At a glance, it looks like you're supposed to put your values in there. Yet, I didn't want to get rid of having a .env.example file because it has been genuinely useful as a form of documentation and lightweight validation.

Thus, envy was created to help catch mistakes while still supporting .env.example. It performs a number of sanity checks:

  • Validates that .env.example does not have any values because they could be actual secrets
  • Validates that .env.example variables are present either globally or in .env
  • Validates that file permissions for .env and .env.example are secure (e.g. not allowed to be writable by other users)
  • Validates that the .env file is a hidden file
  • Validates that the .env file is in .gitignore

Amazingly, no other library does these checks. Even dotenv-safe only checks for missing variables, but makes no effort to prevent committing secrets.

I don't see how it is easier to commit secrets with our setup versus envy's or dotenv-safe's. We identically gitignore server/.env and keep a server/.env-keep as an example file. Do dotenv-safe and envy confirm that the file is gitignored at runtime, or something along those lines?

There are two main ways secrets could end up in the repo, both of which envy detects at runtime:

  1. Developers may screw up the .gitignore in various ways, e.g. omitting the .env file or ignoring the wrong file path by making a typo or using the wrong directory
  2. Developers copy their secrets into .env.example instead of .env

The main difference with dotenv-safe seems to be that it ensures the .env file and the example file are in sync, and is stricter about the example file's contents being present in the environment.

Correct. dotenv-safe is a small improvement over dotenv but I still find it to be a useful addition in the sense that secrets being out of sync with the example file can be a pain. To be clear, that library considers "safety" to mean that the .env.example file is treated as a list of required variables. Preventing leaking of secrets is not a goal of that library.

The main difference with envy (aside from not mutating the environment) seems to be that you only have access to the variables that you explicitly opt-in to using the .env file and example file.

Yes, envy returns an object that contains:

  • Entries defined in .env.example and found in either process.env or the .env file
  • Extraneous entries that are not defined in .env.example but are found in the .env file

At the moment, envy does not return extraneous entries in process.env that are missing in both .env and .env.example. This is a design decision that I was on the fence about for a while. I'm open to discussing changes to that behavior or adding an API that includes all process.env entries. Would definitely like some feedback on this. Ultimately, I chose not to include those entries because it's a little more flexible - the user can still access process.env manually if they need to.

I prefer envy's approach to dotenv-safe because it isn't as pedantic about .env and the example file matching each other, and makes more room for defaults to be applied within the app. I also like that envy doesn't modify the environment itself, which reinforces that all configuration for the app should be determined in/near the app's entrypoint and passed down as e.g. plugin options.

Agreed. There's no need for .env and .env.example to be strictly in sync, so long as the variables defined in .env.example are available globally in process.env. In fact, if envy finds all of the required variables in process.env, it short-circuits and avoids reading the .env file. This is another design decision I am open to refining further. My philosophy here is that process.env takes precedence because values in there may come from someone running MY_VAR=foo node app.js. It's convenient to be able to override variables on the command line that way, so envy respects that.

I appreciate that dotenv does not have any dependencies, and dotenv-safe only depends on dotenv. On the other hand, envy has multiple dependencies. I do aim to keep the boilerplate dependencies as light as possible in order to introduce minimal transitive dependencies outside of the hapi ecosystem, so it is a consideration.

dotenv lacks dependencies because its behavior is overly simplistic. Every dependency that envy has is for a meaningful purpose, either related to ergonomics of the API or the sanity checks.

I care a lot about dependencies. I thoroughly investigate all of my dependencies, including deeply nested ones, and aim to read and understand all of their code. No small task. One of the ways I make this manageable is to only use modules by Sindre Sorhus, when possible. He keeps his code lean and simple.

Here is the entire dependency tree for envy:

 npm ls --production
envy@2.0.0 /Users/sholladay/Code/personal/envy
├── camelcase@5.3.1
├─┬ camelcase-keys@5.2.0
 ├── camelcase@5.3.1 deduped
 ├── map-obj@3.1.0
 └── quick-lru@1.1.0
├── filter-obj@1.1.0
├── is-wsl@1.1.0
└─┬ path-type@3.0.0
  └── pify@3.0.0

Every single one of these modules is made by Sindre. They are actively maintained, stable, and have high quality tests, documentation, etc.

Also notice that most of them are simply utilities that arguably ought to be in JavaScript's standard library. The pify dependency will be removed when I update to path-type@4, further simplifying things.

I like the idea of using a dependency that is more related to the hapi community :)

Awesome, well it's definitely meant to play nicely with hapi. One of the things I strongly dislike about the Express ecosystem is the prevalent use of process.env and I'm so thankful that hapi doesn't do that. envy very much encourages the hapi way of doing things. I use envy for all of my production applications. I dogfood it with hapi, too, It even secures the Stripe secrets for my own personal website.

@YoannMa
Copy link

YoannMa commented Jul 21, 2020

I think it's overall a good idea, having a "safer" default is always good (plus it follows the hapi "way")

My only struggle when using envy is the behavior :

Validates that .env.example does not have any values because they could be actual secrets

Because I mostly want to use .env.exemple as the exemple with default non-production values, that way you can quickly get the project working with a single cp .env.exemple .env. But I agree with why envy does not allow that behavior at all.

@sholladay
Copy link
Author

My suggested workaround is to either:

  • Put the default values in your JavaScript, e.g. using a joi schema to provide the defaults. Hopefully you're validating the config, anyway. This is how I do it, personally.
  • Put the default values as commented out lines in the .env.example file. When your team copies it, they have to explicitly uncomment the lines to use them.

These solutions aren't as bad as they might sound because I would urge you to only do the above for simple, non-sensitive config values like a port number or apiUrl. It's not appropriate for things like API keys.

@YoannMa
Copy link

YoannMa commented Jul 21, 2020

That's mostly what I do indeed, using confidence (this project use it as well now that I looked at it) for default values.

@devinivy
Copy link
Member

@sholladay I am playing with envy in a baseline pal boilerplate, and am running into an issue I can't resolve myself: the boilerplate only has one env variable (PORT) which also happens to be optional. I am happy to mark it as optional by commenting it out and applying the default with joi, but envy doesn't allow an empty example file. I am sort of at an impasse right now, as I don't want users to have to fill-in any environment variables in order to run their new project, but I also would like envy to be all setup for them. Do you have any recommendations either for either 1. envy feature requests that you would be open to receiving a PR for or 2. adjusting my usage of envy? For example: would you be open to allowing example files without an entry, possibly behind a flag?

@sholladay
Copy link
Author

At the time I was thinking that an empty example file would likely be a mistake. But your use case seems valid. Let's remove that assertion. PR welcome.

I'm happy to help work on this if you run into any issues, but I'm currently in the process of selling my house so fairly busy for a few weeks.

@devinivy
Copy link
Member

Just an update here for those interested: I am favoring an eventual move to envy, but there are some loose ends regarding Windows support that are still in progress, and I don't expect it to land for the upcoming version of the boilerplate. Before moving to envy I would also like to alter how environment variables are handled in confidence, and that work is also in progress but not finalized. Here are the relevant issues: sholladay/envy#4 sholladay/envy#8 hapipal/confidence#106 hapipal/confidence#110.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants