From 3be057b6a44cccc310580167ee1d41cef059244a Mon Sep 17 00:00:00 2001
From: Nathaniel May <nathaniel.may@fishtownanalytics.com>
Date: Thu, 3 Feb 2022 15:47:46 -0500
Subject: [PATCH] Avoid saving secrets in SecretContext (#4665) (#4672)

---
 CHANGELOG.md                                  |  1 +
 core/dbt/context/secret.py                    |  7 ++-
 .../068_partial_parsing_tests/test_pp_vars.py | 59 +++++++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b9110b0cc1d..8906f92262f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@
 - Fix bug in retry logic for bad response from hub and when there is a bad git tarball download. ([#4577](https://github.com/dbt-labs/dbt-core/issues/4577), [#4579](https://github.com/dbt-labs/dbt-core/issues/4579), [#4609](https://github.com/dbt-labs/dbt-core/pull/4609)) 
 - Restore previous log level (DEBUG) when a test depends on a disabled resource. Still WARN if the resource is missing ([#4594](https://github.com/dbt-labs/dbt-core/issues/4594), [#4647](https://github.com/dbt-labs/dbt-core/pull/4647))
 - User wasn't asked for permission to overwite a profile entry when running init inside an existing project ([#4375](https://github.com/dbt-labs/dbt-core/issues/4375), [#4447](https://github.com/dbt-labs/dbt-core/pull/4447))
+- A change in secret environment variables won't trigger a full reparse [#4650](https://github.com/dbt-labs/dbt-core/issues/4650) [4665](https://github.com/dbt-labs/dbt-core/pull/4665)
 
 Contributors:
 - [@NiallRees](https://github.com/NiallRees) ([#4447](https://github.com/dbt-labs/dbt-core/pull/4447))
diff --git a/core/dbt/context/secret.py b/core/dbt/context/secret.py
index f6153853653..5a53f2d4786 100644
--- a/core/dbt/context/secret.py
+++ b/core/dbt/context/secret.py
@@ -4,6 +4,7 @@
 from .base import BaseContext, contextmember
 
 from dbt.exceptions import raise_parsing_error
+from dbt.logger import SECRET_ENV_PREFIX
 
 
 class SecretContext(BaseContext):
@@ -27,7 +28,11 @@ def env_var(self, var: str, default: Optional[str] = None) -> str:
             return_value = default
 
         if return_value is not None:
-            self.env_vars[var] = return_value
+            # do not save secret environment variables
+            if not var.startswith(SECRET_ENV_PREFIX):
+                self.env_vars[var] = return_value
+
+            # return the value even if its a secret
             return return_value
         else:
             msg = f"Env var required but not provided: '{var}'"
diff --git a/test/integration/068_partial_parsing_tests/test_pp_vars.py b/test/integration/068_partial_parsing_tests/test_pp_vars.py
index 08b6b162515..0c4c0ec5a4e 100644
--- a/test/integration/068_partial_parsing_tests/test_pp_vars.py
+++ b/test/integration/068_partial_parsing_tests/test_pp_vars.py
@@ -2,6 +2,7 @@
 from dbt.contracts.graph.manifest import Manifest
 from dbt.contracts.files import ParseFileType
 from dbt.contracts.results import TestStatus
+from dbt.logger import SECRET_ENV_PREFIX
 from dbt.parser.partial import special_override_macros
 from test.integration.base import DBTIntegrationTest, use_profile, normalize, get_manifest
 import shutil
@@ -300,6 +301,7 @@ def test_postgres_project_env_vars(self):
         # cleanup
         del os.environ['ENV_VAR_NAME']
 
+
 class ProfileEnvVarTest(BasePPTest):
 
     @property
@@ -352,3 +354,60 @@ def test_postgres_profile_env_vars(self):
         manifest = get_manifest()
         self.assertNotEqual(env_vars_checksum, manifest.state_check.profile_env_vars_hash.checksum)
 
+
+class ProfileSecretEnvVarTest(BasePPTest):
+
+    @property
+    def profile_config(self):
+        # Need to set these here because the base integration test class
+        # calls 'load_config' before the tests are run.
+        # Note: only the specified profile is rendered, so there's no
+        # point it setting env_vars in non-used profiles.
+        os.environ['ENV_VAR_USER'] = 'root'
+        os.environ[SECRET_ENV_PREFIX + 'PASS'] = 'password'
+        return {
+            'config': {
+                'send_anonymous_usage_stats': False
+            },
+            'test': {
+                'outputs': {
+                    'dev': {
+                        'type': 'postgres',
+                        'threads': 1,
+                        'host': self.database_host,
+                        'port': 5432,
+                        'user': "root",
+                        'pass': "password",
+                        'user': "{{ env_var('ENV_VAR_USER') }}",
+                        'pass': "{{ env_var('DBT_ENV_SECRET_PASS') }}",
+                        'dbname': 'dbt',
+                        'schema': self.unique_schema()
+                    },
+                },
+                'target': 'dev'
+            }
+        }
+
+    @use_profile('postgres')
+    def test_postgres_profile_secret_env_vars(self):
+
+        # Initial run
+        os.environ['ENV_VAR_USER'] = 'root'
+        os.environ[SECRET_ENV_PREFIX + 'PASS'] = 'password'
+        self.setup_directories()
+        self.copy_file('test-files/model_one.sql', 'models/model_one.sql')
+        results = self.run_dbt(["run"])
+        manifest = get_manifest()
+        env_vars_checksum = manifest.state_check.profile_env_vars_hash.checksum
+
+        # Change a secret var, it shouldn't register because we shouldn't save secrets.
+        os.environ[SECRET_ENV_PREFIX + 'PASS'] = 'password2'
+        # this dbt run is going to fail because the password isn't actually the right one,
+        # but that doesn't matter because we just want to see if the manifest has included
+        # the secret in the hash of environment variables.
+        (results, log_output) = self.run_dbt_and_capture(["run"], expect_pass=False)
+        # I020 is the event code for "env vars used in profiles.yml have changed"
+        self.assertFalse('I020' in log_output) 
+        manifest = get_manifest()
+        self.assertEqual(env_vars_checksum, manifest.state_check.profile_env_vars_hash.checksum)
+