From 6a1b89ee9155e3d06afad7bda48c66cc7df4864a Mon Sep 17 00:00:00 2001 From: Peter Webb Date: Wed, 17 May 2023 11:38:48 -0400 Subject: [PATCH 1/2] Exclude some profile fields from Jinja rendering when they are not valid Jinja. (#7630) * CT-2583: Exclude some profile fields from Jinja rendering. * CT-2583: Add functional test. * CT-2583: Change approach to password jinja detection * CT-2583: Extract string constant and add additional checks * CT-2583: Improve unit test coverage --- .../unreleased/Fixes-20230515-142851.yaml | 6 ++ core/dbt/config/renderer.py | 12 +++- .../functional/profiles/test_profiles_yml.py | 64 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 .changes/unreleased/Fixes-20230515-142851.yaml create mode 100644 tests/functional/profiles/test_profiles_yml.py diff --git a/.changes/unreleased/Fixes-20230515-142851.yaml b/.changes/unreleased/Fixes-20230515-142851.yaml new file mode 100644 index 00000000000..a64f3a4f88b --- /dev/null +++ b/.changes/unreleased/Fixes-20230515-142851.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Exclude password fields from Jinja rendering. +time: 2023-05-15T14:28:51.400321-04:00 +custom: + Author: peterallenwebb + Issue: "7629" diff --git a/core/dbt/config/renderer.py b/core/dbt/config/renderer.py index 68958dbbce5..7e20bf54b3e 100644 --- a/core/dbt/config/renderer.py +++ b/core/dbt/config/renderer.py @@ -182,7 +182,17 @@ 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) + + try: + rendered = super().render_value(value, keypath) + except Exception as ex: + if keypath and "password" in keypath: + # Passwords sometimes contain jinja-esque characters, but we + # don't want to render them if they aren't valid jinja. + rendered = value + else: + raise ex + # 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): diff --git a/tests/functional/profiles/test_profiles_yml.py b/tests/functional/profiles/test_profiles_yml.py new file mode 100644 index 00000000000..50771c24132 --- /dev/null +++ b/tests/functional/profiles/test_profiles_yml.py @@ -0,0 +1,64 @@ +import pathlib +from test_profile_dir import environ + +from dbt.cli.main import dbtRunner + +jinjaesque_password = "no{{jinja{%re{#ndering" + +profile_with_jinjaesque_password = f"""test: + outputs: + default: + dbname: my_db + host: localhost + password: {jinjaesque_password} + port: 12345 + schema: dummy + threads: 4 + type: postgres + user: peter.webb + target: default +""" + +profile_with_env_password = """test: + outputs: + default: + dbname: my_db + host: localhost + password: "{{ env_var('DBT_PASSWORD') }}" + port: 12345 + schema: dummy + threads: 4 + type: postgres + user: peter.webb + target: default +""" + + +class TestProfileParsing: + def write_profiles_yml(self, profiles_root, content) -> None: + with open(pathlib.Path(profiles_root, "profiles.yml"), "w") as profiles_yml: + profiles_yml.write(content) + + def test_password_not_jinja_rendered_when_invalid(self, project, profiles_root) -> None: + """Verifies that passwords that contain Jinja control characters, but which are + not valid Jinja, do not cause errors.""" + self.write_profiles_yml(profiles_root, profile_with_jinjaesque_password) + + events = [] + result = dbtRunner(callbacks=[events.append]).invoke(["parse"]) + assert result.success + + for e in events: + assert "no{{jinja{%re{#ndering" not in e.info.msg + + def test_password_jinja_rendered_when_valid(self, project, profiles_root) -> None: + """Verifies that a password value that is valid Jinja is rendered as such, + and that it doesn't cause problems if the resulting value looks like Jinja""" + self.write_profiles_yml(profiles_root, profile_with_env_password) + + events = [] + with environ({"DBT_PASSWORD": jinjaesque_password}): + result = dbtRunner(callbacks=[events.append]).invoke(["parse"]) + + assert result.success + assert project.adapter.config.credentials.password == jinjaesque_password From 623231905a21707555fb760e118a24df189cdbd9 Mon Sep 17 00:00:00 2001 From: Peter Allen Webb Date: Tue, 19 Sep 2023 14:22:38 -0400 Subject: [PATCH 2/2] Remove unneeded unit test. --- .../functional/profiles/test_profiles_yml.py | 64 ------------------- 1 file changed, 64 deletions(-) delete mode 100644 tests/functional/profiles/test_profiles_yml.py diff --git a/tests/functional/profiles/test_profiles_yml.py b/tests/functional/profiles/test_profiles_yml.py deleted file mode 100644 index 50771c24132..00000000000 --- a/tests/functional/profiles/test_profiles_yml.py +++ /dev/null @@ -1,64 +0,0 @@ -import pathlib -from test_profile_dir import environ - -from dbt.cli.main import dbtRunner - -jinjaesque_password = "no{{jinja{%re{#ndering" - -profile_with_jinjaesque_password = f"""test: - outputs: - default: - dbname: my_db - host: localhost - password: {jinjaesque_password} - port: 12345 - schema: dummy - threads: 4 - type: postgres - user: peter.webb - target: default -""" - -profile_with_env_password = """test: - outputs: - default: - dbname: my_db - host: localhost - password: "{{ env_var('DBT_PASSWORD') }}" - port: 12345 - schema: dummy - threads: 4 - type: postgres - user: peter.webb - target: default -""" - - -class TestProfileParsing: - def write_profiles_yml(self, profiles_root, content) -> None: - with open(pathlib.Path(profiles_root, "profiles.yml"), "w") as profiles_yml: - profiles_yml.write(content) - - def test_password_not_jinja_rendered_when_invalid(self, project, profiles_root) -> None: - """Verifies that passwords that contain Jinja control characters, but which are - not valid Jinja, do not cause errors.""" - self.write_profiles_yml(profiles_root, profile_with_jinjaesque_password) - - events = [] - result = dbtRunner(callbacks=[events.append]).invoke(["parse"]) - assert result.success - - for e in events: - assert "no{{jinja{%re{#ndering" not in e.info.msg - - def test_password_jinja_rendered_when_valid(self, project, profiles_root) -> None: - """Verifies that a password value that is valid Jinja is rendered as such, - and that it doesn't cause problems if the resulting value looks like Jinja""" - self.write_profiles_yml(profiles_root, profile_with_env_password) - - events = [] - with environ({"DBT_PASSWORD": jinjaesque_password}): - result = dbtRunner(callbacks=[events.append]).invoke(["parse"]) - - assert result.success - assert project.adapter.config.credentials.password == jinjaesque_password