ENG-5001: CLI Environment Variable RCE

[Kunamatata]

Description

This pull request contains updates to address the possibility of RCE (Remote Code Execution)

It introduces the following new features for the doppler run command:

• --only-secrets to only include the specified secrets. This new flag will error if a specified secret does not exist in the current config
  • Usage:

    # The following command will only include `SECRET_1,SECRET_2` and `SECRET_3` as env vars
    $ doppler run --only-secrets "SECRET_1,SECRET_2,SECRET_3" -- <command>

• --no-exit-on-missing-only-secrets will prevent exiting on an error of the --only-secrets flag

• Check for dangerous secret names via the CheckForDangerousSecretNames(...) function (will not run when --mount is provided):

  • If any are found a ⚠️ warning will be issued to the user that some of the following dangerous secrets were found in the current config.

    
    // List of Dangerous Secret Names
    "PROMPT_COMMAND"
    "LD_PRELOAD"
    "LD_LIBRARY_PATH"
    "WINDIR",
    "USERPROFILE"
    "DYLD_INSERT_LIBRARIES"

  "PERL5OPT", "PYTHONWARNINGS", "BROWSER" "HOSTNAME" "PHPRC" "NODE_VERSION" "NODE_OPTIONS"

NOTE: All DOPPLER_ variables will not be included by default when setting the --include (-i) flag without specifying them.

Scenarios

PSA: The logs about the environment variable being included is part of the ./app.js. These are not logged by the doppler cli

Note: the --include flag in the images below has been replaced with --only-secrets

• Trying to include secrets that do not exist in the config:

  • ./doppler run --include "SECRET_1,SECRET_2,SECRET_3" -- node ./app.js

• Including a secret that exists

  • ./doppler run --include "DB_URL" -- node ./app.js

• Including multiple secrets that exist

  • ./doppler run --include "DB_URL,STRIPE_KEY,LOGGING" -- node ./app.js
    

• Including multiple secrets that exist but one that does not exist

  • ./doppler run --include "DB_URL,STRIPE_KEY,LOGGING,DOES_NOT_EXIST" -- node ./app.js

• Same as above but with --no-exit-on-missing-included-secrets:

  • ./doppler run --include "DB_URL,STRIPE_KEY,LOGGING,DOES_NOT_EXIST" --no-exit-on-missing-included-secrets -- node ./app.js

• Dangerous Secret Name Detected:

  • ./doppler run -- node ./app.js

• Dangerous Secret Name Detected with --include

  • ./doppler run --include "DB_URL,STRIPE_KEY,LOGGING" -- node ./app.js

Closes #322

[Kunamatata]

Latest Rebase

• Addresses @Piccirello comments and suggestions

Scenarios

• --include renamed to --only-secrets

  ❯ ./doppler run --only-secrets STRIPE_KEY,DB_URL -- env | rg "STRIPE_KEY|DB_URL"                        
  DB_URL=psql://elon@localhost/modelX
  STRIPE_KEY=sk_test_mKciizZfpXhQgXoNZmzECVN

• --only-secrets and -i (shorthand for the flag) with 1 nonexisting secret name

  
  ❯ ./doppler run -i STRIPE_KEY,DOES_NOT_EXIST -- printenv STRIPE_KEY                                
  Doppler Error: the following secrets you are trying to include do not exist in your config:

• DOES_NOT_EXIST

• -i with two non existing secrets

  
  ❯ ./doppler run -i STRIPE_KEY,DOES_NOT_EXIST,MAYBE_THIS -- printenv STRIPE_KEY                     
  Doppler Error: the following secrets you are trying to include do not exist in your config:

• DOES_NOT_EXIST

• MAYBE_THIS

• --only-secrets works with --mount

  ❯ ./doppler run --only-secrets STRIPE_KEY,DB_URL --mount mount.json -- cat mount.json                
  {"DB_URL":"psql://user@localhost/my_db","STRIPE_KEY":"sk_test_mKciizZfpXhQgXoNZmzECVN"}%   

• --only-secrets with --no-exit-on-missing-included-secrets

  
  ❯ ./doppler run --only-secrets STRIPE_KEY,DOES_NOT_EXIST --no-exit-on-missing-included-secrets -- printenv STRIPE_KEY
  Warning: the following secrets you are trying to include do not exist in your config:

• DOES_NOT_EXIST sk_test_mKciizZfpXhQgXoNZmzECVN

[Kunamatata]

Latest Rebase

• Add comments back for flag separation
• Rephrase the error message
• Add Language Interpreters' dangerous environment variable names to the dangerousSecretNames list
• Add a link to documentation for potentially dangerous secret names above CheckForDangerousSecretNames function and dangerousSecretNames list @Piccirello I have questions in the comments above.

[Kunamatata]

Would we be cool with --no-exit instead of the very verbose --no-exit-on-missing-included-secrets

[Kunamatata]

Latest Rebase

• replace utils.PrintWarning with utils.LogWarning so that we log to stderr not to stdout

[Kunamatata]

Latest Rebase

• Addressed @Piccirello's comments and suggestions.
• Rewrote tests for FilterMap
• Wrote tests for SelectSecrets

Ready for re-review.

[Kunamatata]

Latest Rebase @Piccirello

• Add chore: prefix to Filter Map commit
• Replace StringSliceVarP with StringSliceVar
• Use --no-exit-on-missing-only-secrets question here: https://github.com/DopplerHQ/cli/pull/341#discussion_r1014976897

[nmanoogian]

Way to go on your first CLI feature PR, @Kunamatata!