motdotla / dotenv-expand

Variable expansion for dotenv. Expand variables already on your machine for use in your .env file.
https://dotenvx.com
BSD 2-Clause "Simplified" License
935 stars 93 forks source link

supporting expansion while simultaneously supporting passwords with `$` in them (common in generated passwords like postgres) #101

Closed ellenaua closed 7 months ago

ellenaua commented 1 year ago

Hi! I reported an issue to serverless but then found that dotenv-expand is actually causing the problem.

The problem is:

How to reproduce

Create an .env file with this content:

POSTGRESQL_DATABASE=some_db
POSTGRESQL_HOST=some_host
POSTGRESQL_PASSWORD="ab$defgh@jk"
POSTGRESQL_USERNAME=some_user

And create test.js file with this content:

const dotenv = require('dotenv');
const dotenvExpand = require('dotenv-expand');

const config = dotenv.config({ path: './.env' });
console.log(dotenvExpand.expand(config));

Run node test.js and you'll see that POSTGRESQL_PASSWORD="ab@jk" ($defgh disappeared)

Possible solutions

Is it possible to provide some option to 'dotenv-expand' so that it does nothing if variable $defgh does not exist? Currently it replaces it by empty string.

perry-mitchell commented 1 year ago

I can confirm this issue.

Take this example:

const dotenv  = require("dotenv");
const dotenvExpand = require("dotenv-expand");

const raw = dotenv.parse(`
PASS="abc123$51s.test.123"
`);
const expanded = dotenvExpand.expand(raw);

console.log(raw);
console.log(expanded);

This will print:

{ PASS: 'abc123$test.123' }
{ PASS: 'abc123$test.123' }

Which shows that nothing is expanded when just using parse.

But if I change it to read the same data from a .env file, like so:

const dotenv  = require("dotenv");
const dotenvExpand = require("dotenv-expand");

const raw = dotenv.config({ path: "./.env" });
const expanded = dotenvExpand.expand(raw);

console.log(raw);
console.log(expanded);

I get:

{ parsed: { PASS: 'abc123$51s.test.123' } }
{ parsed: { PASS: 'abc123.test.123' } }

Notice that the $51s part is gone after the expand call.

perry-mitchell commented 1 year ago

@motdotla Would you kindly take a look at https://github.com/motdotla/dotenv-expand/pull/102 ?

perry-mitchell commented 1 year ago

As an extra note, if it helps anyone - if you're using something like dotenv-cli and docker, be careful to make sure that docker isn't using the .env file to create the container, as these variables will not only override dotenv inside the container, but they'll be escaped differently to how this library handles it.

qraynaud commented 11 months ago

Is it an issue? You should escape the $ sign with a \. There is a related issue: if the env variable (before parsing) is set with a $ inside it should not try to treat it as a variable. It’s not.

motdotla commented 7 months ago

@perry-mitchell and @ellenaua

  1. do you not have control over that password with the dollar sign in it? ab$defghijk
  2. do you not have control over whether dotenv-expand is already installed?

if so, i can understand your frustration, and we definitely need an elegant fix to solve the problem.

you mentioned you are using serverless - are you able to remove usage of dotenv-expand from it and just use dotenv or is that not an option?

motdotla commented 7 months ago

I understand that you can use $ to escape a $, but that is annoying to deal with when you end up needing to put it in passwords. Is it possible to change the $ to some other thing? @eatonjl

https://github.com/motdotla/dotenv-expand/issues/95

motdotla commented 7 months ago

In CapRover the environment variables don't need a \ to mark dollar signs that aren't expansions, so whenever I have to move environment variables from CapRover to local I need to find all $ and add a \ before them, which is annoying and error-prone.

Also, this would allow variable expansion to be aligned with the usage of backticks in Javascript:

https://github.com/motdotla/dotenv-expand/issues/83

qraynaud commented 7 months ago

I think the main issue is that $ values are expanded when they were in the env before. Issues like CapRover can easily be streamlined with scripting (it’s easy to escape $ automatically).

I was having this problem that env variables already present containing a $ were all scrambled by dotenv-expand and I had to recode all of it for our internal use. I think I came up with a simplier and more robust implementation:

const VAR_REGEX =
  // Match $var ${var} ${var:-default} and \$ in a string
  /\\?\$(?:\{(?<brackets>\w+)(?:\s*:-(?<defaultValue>[^}]*?))?\}|(?<noBrakets>\w+))/g;

const expand = (env: DotEnvReturnValue) => {
  for (const [key, value] of Object.entries(env.parsed ?? {})) {
      // Do not replace variables that where alreay in the env
      if (process.env[key] !== value) {
        continue;
      }

      // Replace process.env value properly
      process.env[key] = value.replaceAll(
        VAR_REGEX,
        (match, brackets, defaultValue, noBrakets) =>
          match[0] === "\\"
            ? match.substring(1)
            : process.env[brackets || noBrakets] ?? defaultValue ?? "",
      );
    }
}

Hope this can help you get things straight @motdotla!

motdotla commented 7 months ago

i think another problem here is that if a value contains a $ and it is not intended to be a variable, we should be able to wrap it in single quotes.

PASSWORD='pas$word'
qraynaud commented 7 months ago

@motdotla that would be ideal but it would require a partial rewrite of dotenv to support the single quote syntax and to report in the return value which values it found between single quotes.

I think we should split this in 2 separate issues :

motdotla commented 7 months ago

@qraynaud you're right.

work started here with failing test: https://github.com/motdotla/dotenv-expand/pull/104

motdotla commented 7 months ago

a bug that is that if you have your env containing MY_PASSWORD=pas$word and your .env file that contains MY_PASSWORD=test then dotenv-expand tries to expand $word in the env while this is obviously invalid (it should only expand values coming from an .env file)

this is fixed and released as v11.0.0

https://github.com/motdotla/dotenv-expand/blob/master/CHANGELOG.md#1100-2024-02-10

a feature request (that can be implemented whenever, that’s not a bug) about supporting single quote syntax in dotenv and dotenv-expand (that’s not hard to do, just cumbersome)

this is going to its own issue to track

https://github.com/motdotla/dotenv-expand/issues/112