From 492c67e31103a0a81bb24599aaf459349475faa3 Mon Sep 17 00:00:00 2001 From: Thomas Piccirello Date: Thu, 5 May 2022 23:53:59 -0700 Subject: [PATCH] Fix `run` erroneously using local environment variable The local env var should only be given precedence when `--preserve-env` is specified. This also simplifies the key reconciliation logic a bit. --- pkg/cmd/run.go | 73 ++++++++++++++++++++++-------------------- tests/e2e.sh | 1 + tests/e2e/run-mount.sh | 10 +++--- tests/e2e/run.sh | 73 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 39 deletions(-) create mode 100755 tests/e2e/run.sh diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index f6098060..f57dcaf0 100644 --- a/pkg/cmd/run.go +++ b/pkg/cmd/run.go @@ -152,45 +152,22 @@ doppler run --mount secrets.json -- cat secrets.json`, } originalEnv := os.Environ() - existingEnvKeys := map[string]bool{} + existingEnvKeys := map[string]string{} for _, envVar := range originalEnv { // key=value format parts := strings.SplitN(envVar, "=", 2) key := parts[0] - existingEnvKeys[key] = true + value := parts[1] + existingEnvKeys[key] = value } + env := []string{} secrets := map[string]string{} - excludedKeys := []string{"PATH", "PS1", "HOME"} - for name, value := range dopplerSecrets { - useSecret := true - if !shouldMountFile { - // ignore secrets that might conflict with the environment - for _, excludedKey := range excludedKeys { - if excludedKey == name { - utils.LogDebug(fmt.Sprintf("Ignoring restricted secret %s", name)) - useSecret = false - break - } - } - - // skip secret if environment already contains variable w/ same name - if preserveEnv && existingEnvKeys[name] == true { - utils.LogDebug(fmt.Sprintf("Ignoring Doppler secret %s", name)) - useSecret = false - } - } - - if !useSecret { - continue - } - - secrets[name] = value - } - - var env []string var onExit func() if shouldMountFile { + secrets = dopplerSecrets + env = originalEnv + if shouldAutoDetectFormat { if strings.HasSuffix(mountPath, ".env") { mountFormat = "env" @@ -215,15 +192,43 @@ doppler run --mount secrets.json -- cat secrets.json`, // export path to mounted file env = append(env, fmt.Sprintf("%s=%s", "DOPPLER_CLI_SECRETS_PATH", mountPath)) } else { - // export doppler secrets + // remove any reserved keys from secrets + reservedKeys := []string{"PATH", "PS1", "HOME"} + for _, reservedKey := range reservedKeys { + if _, found := dopplerSecrets[reservedKey]; found == true { + utils.LogDebug(fmt.Sprintf("Ignoring reserved secret %s", reservedKey)) + delete(dopplerSecrets, reservedKey) + } + } + + if preserveEnv { + // use doppler secrets + for name, value := range dopplerSecrets { + secrets[name] = value + } + // then use existing env vars + for name, value := range existingEnvKeys { + if _, found := secrets[name]; found == true { + utils.LogDebug(fmt.Sprintf("Ignoring Doppler secret %s", name)) + } + secrets[name] = value + } + } else { + // use existing env vars + for name, value := range existingEnvKeys { + secrets[name] = value + } + // then use doppler secrets + for name, value := range dopplerSecrets { + secrets[name] = value + } + } + for _, envVar := range controllers.MapToEnvFormat(secrets, false) { env = append(env, envVar) } } - // include original environment variables - env = append(env, originalEnv...) - exitCode := 0 var err error diff --git a/tests/e2e.sh b/tests/e2e.sh index f11053b1..8d977b25 100755 --- a/tests/e2e.sh +++ b/tests/e2e.sh @@ -11,6 +11,7 @@ export DOPPLER_CONFIG="prd_e2e_tests" # Run tests "$DIR/e2e/secrets-download-fallback.sh" +"$DIR/e2e/run.sh" "$DIR/e2e/run-fallback.sh" "$DIR/e2e/run-mount.sh" "$DIR/e2e/configure.sh" diff --git a/tests/e2e/run-mount.sh b/tests/e2e/run-mount.sh index 41af82db..97dd5b9b 100755 --- a/tests/e2e/run-mount.sh +++ b/tests/e2e/run-mount.sh @@ -36,7 +36,7 @@ beforeAll beforeEach # verify content of mounted secrets file -EXPECTED_SECRETS='{"DOPPLER_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_ENVIRONMENT":"prd","DOPPLER_ENCLAVE_PROJECT":"cli","DOPPLER_ENVIRONMENT":"prd","DOPPLER_PROJECT":"cli"}' +EXPECTED_SECRETS='{"DOPPLER_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_ENVIRONMENT":"prd","DOPPLER_ENCLAVE_PROJECT":"cli","DOPPLER_ENVIRONMENT":"prd","DOPPLER_PROJECT":"cli","HOME":"123"}' actual="$("$DOPPLER_BINARY" run --mount secrets.json --command "cat \$DOPPLER_CLI_SECRETS_PATH")" if [ "$actual" != "$EXPECTED_SECRETS" ]; then echo "ERROR: mounted secrets file has invalid contents" @@ -65,7 +65,7 @@ fi beforeEach # verify format is auto detected -EXPECTED_SECRETS='DOPPLER_CONFIG="prd_e2e_tests"\nDOPPLER_ENCLAVE_CONFIG="prd_e2e_tests"\nDOPPLER_ENCLAVE_ENVIRONMENT="prd"\nDOPPLER_ENCLAVE_PROJECT="cli"\nDOPPLER_ENVIRONMENT="prd"\nDOPPLER_PROJECT="cli"' +EXPECTED_SECRETS='DOPPLER_CONFIG="prd_e2e_tests"\nDOPPLER_ENCLAVE_CONFIG="prd_e2e_tests"\nDOPPLER_ENCLAVE_ENVIRONMENT="prd"\nDOPPLER_ENCLAVE_PROJECT="cli"\nDOPPLER_ENVIRONMENT="prd"\nDOPPLER_PROJECT="cli"\nHOME="123"' actual="$("$DOPPLER_BINARY" run --mount secrets.env --command "cat \$DOPPLER_CLI_SECRETS_PATH")" if [[ "$actual" != "$(echo -e "$EXPECTED_SECRETS")" ]]; then echo "ERROR: mounted secrets file with auto-detected env format has invalid contents" @@ -75,7 +75,7 @@ fi beforeEach # verify specified format is used -EXPECTED_SECRETS='{"DOPPLER_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_ENVIRONMENT":"prd","DOPPLER_ENCLAVE_PROJECT":"cli","DOPPLER_ENVIRONMENT":"prd","DOPPLER_PROJECT":"cli"}' +EXPECTED_SECRETS='{"DOPPLER_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_ENVIRONMENT":"prd","DOPPLER_ENCLAVE_PROJECT":"cli","DOPPLER_ENVIRONMENT":"prd","DOPPLER_PROJECT":"cli","HOME":"123"}' actual="$("$DOPPLER_BINARY" run --mount secrets.env --mount-format json --command "cat \$DOPPLER_CLI_SECRETS_PATH")" if [[ "$actual" != "$(echo -e "$EXPECTED_SECRETS")" ]]; then echo "ERROR: mounted secrets file with json format has invalid contents" @@ -85,7 +85,7 @@ fi beforeEach # verify specified name transformer is used -EXPECTED_SECRETS='{"TF_VAR_doppler_config":"prd_e2e_tests","TF_VAR_doppler_enclave_config":"prd_e2e_tests","TF_VAR_doppler_enclave_environment":"prd","TF_VAR_doppler_enclave_project":"cli","TF_VAR_doppler_environment":"prd","TF_VAR_doppler_project":"cli"}' +EXPECTED_SECRETS='{"TF_VAR_doppler_config":"prd_e2e_tests","TF_VAR_doppler_enclave_config":"prd_e2e_tests","TF_VAR_doppler_enclave_environment":"prd","TF_VAR_doppler_enclave_project":"cli","TF_VAR_doppler_environment":"prd","TF_VAR_doppler_project":"cli","TF_VAR_home":"123"}' actual="$("$DOPPLER_BINARY" run --mount secrets.json --name-transformer tf-var --command "cat \$DOPPLER_CLI_SECRETS_PATH")" if [[ "$actual" != "$EXPECTED_SECRETS" ]]; then echo "ERROR: mounted secrets file with name transformer has invalid contents" @@ -95,7 +95,7 @@ fi beforeEach # verify existing env value is ignored even when --preserve-env is specified -EXPECTED_SECRETS='{"DOPPLER_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_ENVIRONMENT":"prd","DOPPLER_ENCLAVE_PROJECT":"cli","DOPPLER_ENVIRONMENT":"prd","DOPPLER_PROJECT":"cli"}' +EXPECTED_SECRETS='{"DOPPLER_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_CONFIG":"prd_e2e_tests","DOPPLER_ENCLAVE_ENVIRONMENT":"prd","DOPPLER_ENCLAVE_PROJECT":"cli","DOPPLER_ENVIRONMENT":"prd","DOPPLER_PROJECT":"cli","HOME":"123"}' actual="$(DOPPLER_CONFIG="test" "$DOPPLER_BINARY" run --preserve-env --config prd_e2e_tests --mount secrets.json --command "cat \$DOPPLER_CLI_SECRETS_PATH")" if [ "$actual" != "$EXPECTED_SECRETS" ]; then echo "ERROR: mounted secrets file with --preserve-env has invalid contents" diff --git a/tests/e2e/run.sh b/tests/e2e/run.sh new file mode 100755 index 00000000..6aabb820 --- /dev/null +++ b/tests/e2e/run.sh @@ -0,0 +1,73 @@ +#!/bin/bash + +set -euo pipefail + +TEST_NAME="run" + +cleanup() { + exit_code=$? + if [ "$exit_code" -ne 0 ]; then + echo "ERROR: '$TEST_NAME' tests failed during execution" + afterAll || echo "ERROR: Cleanup failed" + fi + + exit "$exit_code" +} +trap cleanup EXIT + +beforeAll() { + echo "INFO: Executing '$TEST_NAME' tests" + "$DOPPLER_BINARY" run clean --max-age=0s --silent +} + +beforeEach() { + "$DOPPLER_BINARY" run clean --max-age=0s --silent +} + +afterAll() { + echo "INFO: Completed '$TEST_NAME' tests" + "$DOPPLER_BINARY" run clean --max-age=0s --silent +} + +error() { + message=$1 + echo "$message" + exit 1 +} + +beforeAll + +beforeEach + +# verify local env is ignored +config="$(DOPPLER_CONFIG=123 "$DOPPLER_BINARY" run --config prd_e2e_tests -- printenv DOPPLER_CONFIG)" +[[ "$config" == "prd_e2e_tests" ]] || error "ERROR: conflicting local env var is not ignored" + +beforeEach + +# verify local env is used when specifying --preserve-env +config="$(DOPPLER_CONFIG=123 "$DOPPLER_BINARY" run --config prd_e2e_tests --preserve-env -- printenv DOPPLER_CONFIG)" +[[ "$config" == "123" ]] || error "ERROR: conflicting local env var is not used with --preserve-env" + +beforeEach + +# verify local env is used when key isn't specified in Doppler +value="$(NONEXISTENT_KEY=123 "$DOPPLER_BINARY" run -- printenv NONEXISTENT_KEY)" +[[ "$value" == "123" ]] || error "ERROR: local env var is not used" + +beforeEach + +# verify local env is used when key isn't specified in Doppler (and --preserve-env is specified) +value="$(NONEXISTENT_KEY=123 "$DOPPLER_BINARY" run --preserve-env -- printenv NONEXISTENT_KEY)" +[[ "$value" == "123" ]] || error "ERROR: local env var is not used with --preserve-env" + +beforeEach + +# verify reserved secrets are ignored +# first verify the doppler config has a secret named 'HOME', or this test is useless +"$DOPPLER_BINARY" secrets get HOME >/dev/null 2>&1 || error "ERROR: doppler config does not contain 'HOME' secret" +home="$HOME" +value="$("$DOPPLER_BINARY" run -- printenv HOME)" +[[ "$value" == "$home" ]] || error "ERROR: reserved secret is not ignored" + +afterAll