Allow setting variable with a value containing strings about dotenv-cli HOT 7 CLOSED

I'm not sure if this modified regex is the correct solution https://regex101.com/r/O9zira/1

^\w+=[a-zA-Z0-9"=^!?%@_&\-/:;.'"\s]+$

from dotenv-cli.

I'm a bit worried about security issues and I don't have a lot of time to investigate all these regex changes. Are you also using this on Windows? Because otherwise you can use #56 (comment)

from dotenv-cli.

Are you also using this on Windows? Because otherwise you can use #56 (comment)

Unfortunately yes we need our script to be cross platform on Windows as well.

from dotenv-cli.

Any traction on this so far? This is a major sticking point for us. Env vars with spaces are a must have for us. 🙁

from dotenv-cli.

Switched over to @dotenvx/dotenvx for now, but my methods are not cross-platform.

NODE_ENV=production \
NODE_OPTIONS='--no-experimental-fetch --trace-warnings' \
    npx dotenvx run --env-file=.env.production -- \
    next start

Combining @dotenvx/dotenvx with cross-env might be messy, but it would get the job done, at least until this is resolved.

This is what I was attempting to use before:

dotenv -e .env.production -v NODE_ENV=production -v NODE_OPTIONS='--no-experimental-fetch --trace-warnings' next start

For what it's worth, I'm struggling to understand the security issue you mentioned. The syntax you referenced, afterall, is a function of the shell interpreter, and would be working as expected in any other CLI application:

node cli.js -v test="$(echo bax; echo foo=bar)" -p test
# outputs bax
node cli.js -v test="$(echo bax; echo foo=bar)" -p foo
# ouputs bar

Double quotes in BASH signify a string with interpolation enabled. It's expected that anything within "$(...)" would not be idempotent. This is actually a strong feature of bash. Preventing that may be considered an anti-pattern, especially since someone executing either of the above would likely be doing so intentionally and expecting it to work.

It might be prudent to use something like yargs, which is presently rigorously mature. Granted, it adds bulk to the package. But if security is a concern, there's no doubting the maturity of the project. And I would definitely see that as a beneficial trade-off, and might ease the pressure of vetting changes like these over security concerns! 🙂

from dotenv-cli.

Preventing that may be considered an anti-pattern, especially since someone executing either of the above would likely be doing so intentionally and expecting it to work.

You would expect that if you set test with -v that it can also set other variables like foo? Which is what is happening above. Especially with newlines, it could be problematic if only part of the value is in the env variable (e.g. with some encryption keys, it's often multiple lines). That's not a security issue but still unexpected results. I'm happy to merge a PR but I think this is getting more complicated than a simple CLI tool and it warrants some tests.

I'm not inclined to merge anything which will increase the maintenance burden of this tool for me.

from dotenv-cli.

@entropitor Hahah, Oops! It was a late Friday, and I misunderstood what was being demonstrated. I apologize.

After looking things over, and especially given the fact that the goal is to avoid over-complicating, I think the right direction would actually be to simplify, rather than over-complicate.

I went ahead and implemented the changes I recommended in this PR: #105

tl;dr:

• We can actually simplify things a lot, instead of increasing the complexity of the regex
• We should just treat everything after the first = as the value
• This is compliant with the way that dotenv expects, including the ability to have newline characters in values
• dotenv.parse() expects the body of a .env file, which is why using it to parse the CLI args creates the security hole.

 function validateCmdVariable (param) {
-  if (!param.match(/^\w+=[a-zA-Z0-9"=^!?%@_&\-/:;.]+$/)) {
+  if (!param.match(/^\w+=.+$/)) {
     console.error('Unexpected argument ' + param + '. Expected variable in format variable=value')
     process.exit(1)
   }

-  return param
+  // Remember, just a POC
+  const [key, ...val] = param.split(/=/)
+  return [key, val.join('=')]
 }
 const variables = []
 if (argv.v) {
@@ -71,8 +72,10 @@ if (argv.v) {
     variables.push(...argv.v.map(validateCmdVariable))
   }
 }
-// This expects a Buffer, because we should be passing the contents of a file here.
-const parsedVariables = dotenv.parse(Buffer.from(variables.join('\n')))
+
+// variables is now in the form of [ [key, val], [key, val] ],
+// so we can plug it straight into `Object.fromEntries()`
+const parsedVariables = Object.fromEntries(variables)

 if (argv.debug) {
   console.log(paths)

node cli.js -v foo=bar=baz -v "pew=pew" -v NODE_OPTIONS='--no-experimental-fetch --trace-warnings --loader=ts-node/tsm --loader=esmock' --debug

[ '.env' ]
{
  foo: 'bar=baz',
  pew: 'pew',
  NODE_OPTIONS: '--no-experimental-fetch --trace-warnings --loader=ts-node/tsm --loader=esmock'
}

node cli.js -v test="$(echo bax; echo foo=bar)" -p test
# Outputs Unexpected argument error
node cli.js -v test="$(echo bax; echo foo=bar)" -p foo
# Outputs Unexpected argument error

-  if (!param.match(/^\w+=.+$/)) {
+  if (!param.match(/^\w+=.+$/m)) {

node cli.js -v test="$(echo bax; echo foo=bar)" -p test
# outputs `bax\nfoo=bar`
node cli.js -v test="$(echo bax; echo foo=bar)" -p foo
# ouputs ''

from dotenv-cli.