Skip to content

Commit

Permalink
Exclude some profile fields from Jinja rendering when they are not va…
Browse files Browse the repository at this point in the history
…lid 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
  • Loading branch information
peterallenwebb authored May 17, 2023
1 parent 5f7ae2f commit dea3181
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 1 deletion.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20230515-142851.yaml
Original file line number Diff line number Diff line change
@@ -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"
12 changes: 11 additions & 1 deletion core/dbt/config/renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
64 changes: 64 additions & 0 deletions tests/functional/profiles/test_profiles_yml.py
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit dea3181

Please sign in to comment.