Follow-up to #5334

.changes/unreleased/Security-20220606-230353.yaml

@@ -0,0 +1,7 @@
+kind: Security
+body: Move string interpolation of "secret" env vars outside of Jinja context. Update "contexts" README
+time: 2022-06-06T23:03:53.022568+02:00
+custom:
+  Author: jtcohen6
+  Issue: "4796"
+  PR: "5334"

.changes/unreleased/Under the Hood-20220616-120516.yaml

@@ -0,0 +1,7 @@
+kind: Under the Hood
+body: 'Add annotation to render_value method reimplemented in #5334'
+time: 2022-06-16T12:05:16.474078+02:00
+custom:
+  Author: jtcohen6
+  Issue: "4796"
+  PR: "5382"

.changie.yaml

@@ -14,6 +14,7 @@ kinds:
 - label: Docs
 - label: Under the Hood
 - label: Dependencies
+- label: Security
 custom:
 - key: Author
   label: GitHub Username(s) (separated by a single space if multiple)

core/dbt/config/renderer.py

@@ -178,7 +178,12 @@ def name(self):
         return "Secret"
 
     def render_value(self, value: Any, keypath: Optional[Keypath] = None) -> Any:
+        # First, standard Jinja rendering, with special handling for 'secret' environment variables
+        # "{{ env_var('DBT_SECRET_ENV_VAR') }}" -> "$$$DBT_SECRET_START$$$DBT_SECRET_ENV_{VARIABLE_NAME}$$$DBT_SECRET_END$$$"
+        # This prevents Jinja manipulation of secrets via macros/filters that might leak partial/modified values in logs
         rendered = super().render_value(value, keypath)
+        # Now, detect instances of the placeholder value ($$$DBT_SECRET_START...DBT_SECRET_END$$$)
+        # and replace them with the actual secret value
         if SECRET_ENV_PREFIX in str(rendered):
             search_group = f"({SECRET_ENV_PREFIX}(.*))"
             pattern = SECRET_PLACEHOLDER.format(search_group).replace("$", r"\$")

tests/functional/context_methods/test_secret_env_vars.py

@@ -134,3 +134,25 @@ def test_fail_clone_with_scrubbing(self, project):
             _, log_output = run_dbt_and_capture(["deps"])
 
         assert "abc123" not in str(excinfo.value)
+
+
+class TestCloneFailSecretNotRendered(TestCloneFailSecretScrubbed):
+    # as above, with some Jinja manipulation
+    @pytest.fixture(scope="class")
+    def packages(self):
+        return {
+            "packages": [
+                {
+                    "git": "https://fakeuser:{{ env_var('DBT_ENV_SECRET_GIT_TOKEN') | join(' ') }}@github.com/dbt-labs/fake-repo.git"
+                },
+            ]
+        }
+
+    def test_fail_clone_with_scrubbing(self, project):
+        with pytest.raises(InternalException) as excinfo:
+            _, log_output = run_dbt_and_capture(["deps"])
+
+        # we should not see any manipulated form of the secret value (abc123) here
+        # we should see a manipulated form of the placeholder instead
+        assert "a b c 1 2 3" not in str(excinfo.value)
+        assert "D B T _ E N V _ S E C R E T _ G I T _ T O K E N" in str(excinfo.value)