From e8bc017fcc65f14feb49c1b982791b79e53f60c7 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 19 Apr 2024 08:16:56 -0500 Subject: [PATCH 1/5] replace secret placeholder with mask for consistency --- core/dbt/context/base.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/core/dbt/context/base.py b/core/dbt/context/base.py index e969506f625..5ee266f8b6a 100644 --- a/core/dbt/context/base.py +++ b/core/dbt/context/base.py @@ -561,6 +561,20 @@ def log(msg: str, info: bool = False) -> str: {{ log("Running some_macro: " ~ arg1 ~ ", " ~ arg2) }} {% endmacro %}" """ + # Now, detect instances of the placeholder value ($$$DBT_SECRET_START...DBT_SECRET_END$$$) + # and replace them with the standard mask '*****' + if "DBT_SECRET_START" in str(msg): + search_group = f"({SECRET_ENV_PREFIX}(.*))" + from dbt.context.secret import SECRET_PLACEHOLDER + + pattern = SECRET_PLACEHOLDER.format(search_group).replace("$", r"\$") + m = re.search( + pattern, + msg, + ) + if m: + msg = re.sub(pattern, "*****", msg) + if info: fire_event(JinjaLogInfo(msg=msg, node_info=get_node_info())) else: From 76cff61b9565d983f5a6d3260dec77aa7dcf8db2 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Mon, 22 Apr 2024 14:15:57 -0500 Subject: [PATCH 2/5] add test --- .../dependencies/test_dependency_secrets.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/functional/dependencies/test_dependency_secrets.py diff --git a/tests/functional/dependencies/test_dependency_secrets.py b/tests/functional/dependencies/test_dependency_secrets.py new file mode 100644 index 00000000000..f1ac92e0b98 --- /dev/null +++ b/tests/functional/dependencies/test_dependency_secrets.py @@ -0,0 +1,31 @@ +import os +import pytest + +from dbt.tests.util import run_dbt_and_capture +from dbt_common.constants import SECRET_ENV_PREFIX + + +class TestAllowSecretProfilePackage: + @pytest.fixture(scope="class", autouse=True) + def setUp(self): + os.environ[SECRET_ENV_PREFIX + "FOR_LOGGING"] = "super secret" + yield + del os.environ[SECRET_ENV_PREFIX + "FOR_LOGGING"] + + @pytest.fixture(scope="class") + def packages(self): + return { + "packages": [ + { + "package": "dbt-labs/dbt_utils{{ log(env_var('DBT_ENV_SECRET_FOR_LOGGING'), info = true) }}", + "version": "1.0.0", + } + ] + } + + def test_allow_secrets(self, project): + _, log_output = run_dbt_and_capture(["deps"]) + # this will not be written to logs or lock file + assert not ("super secret" in log_output) + assert "*****" in log_output + assert not ("DBT_ENV_SECRET_FOR_LOGGING" in log_output) From 03efa1534852c94140bad58e0ac05b796a324019 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Mon, 22 Apr 2024 14:58:21 -0500 Subject: [PATCH 3/5] changelog --- .changes/unreleased/Fixes-20240422-145811.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20240422-145811.yaml diff --git a/.changes/unreleased/Fixes-20240422-145811.yaml b/.changes/unreleased/Fixes-20240422-145811.yaml new file mode 100644 index 00000000000..76452c41a90 --- /dev/null +++ b/.changes/unreleased/Fixes-20240422-145811.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Use consistent secret scrubbing with the log function. +time: 2024-04-22T14:58:11.990326-05:00 +custom: + Author: emmyoop + Issue: "9987" From 77922530c18618414dba8d250e68148e5feeb460 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Tue, 23 Apr 2024 14:39:42 -0500 Subject: [PATCH 4/5] move SECRET_PLACEHOLDER to avoid circular deps --- core/dbt/config/renderer.py | 4 ++-- core/dbt/constants.py | 2 ++ core/dbt/context/base.py | 8 +++----- core/dbt/context/secret.py | 5 +---- tests/functional/dependencies/test_dependency_secrets.py | 2 +- 5 files changed, 9 insertions(+), 12 deletions(-) diff --git a/core/dbt/config/renderer.py b/core/dbt/config/renderer.py index eee740893b8..ad2c547be21 100644 --- a/core/dbt/config/renderer.py +++ b/core/dbt/config/renderer.py @@ -4,9 +4,9 @@ from dbt.clients.jinja import get_rendered from dbt_common.clients.jinja import catch_jinja -from dbt.constants import SECRET_ENV_PREFIX, DEPENDENCIES_FILE_NAME +from dbt.constants import SECRET_ENV_PREFIX, DEPENDENCIES_FILE_NAME, SECRET_PLACEHOLDER from dbt.context.target import TargetContext -from dbt.context.secret import SecretContext, SECRET_PLACEHOLDER +from dbt.context.secret import SecretContext from dbt.context.base import BaseContext from dbt.adapters.contracts.connection import HasCredentials from dbt.exceptions import DbtProjectError diff --git a/core/dbt/constants.py b/core/dbt/constants.py index 31e4d833185..9ffa7128a69 100644 --- a/core/dbt/constants.py +++ b/core/dbt/constants.py @@ -2,6 +2,8 @@ SECRET_ENV_PREFIX = "DBT_ENV_SECRET_" DEFAULT_ENV_PLACEHOLDER = "DBT_DEFAULT_PLACEHOLDER" +SECRET_PLACEHOLDER = "$$$DBT_SECRET_START$$${}$$$DBT_SECRET_END$$$" + MAXIMUM_SEED_SIZE = 1 * 1024 * 1024 MAXIMUM_SEED_SIZE_NAME = "1MB" diff --git a/core/dbt/context/base.py b/core/dbt/context/base.py index 5ee266f8b6a..1c7f4cf9db4 100644 --- a/core/dbt/context/base.py +++ b/core/dbt/context/base.py @@ -11,7 +11,7 @@ from dbt import utils from dbt.clients.jinja import get_rendered from dbt.clients.yaml_helper import yaml, safe_load, SafeLoader, Loader, Dumper # noqa: F401 -from dbt.constants import SECRET_ENV_PREFIX, DEFAULT_ENV_PLACEHOLDER +from dbt.constants import SECRET_ENV_PREFIX, DEFAULT_ENV_PLACEHOLDER, SECRET_PLACEHOLDER from dbt.contracts.graph.nodes import Resource from dbt.exceptions import ( SecretEnvVarLocationError, @@ -561,12 +561,10 @@ def log(msg: str, info: bool = False) -> str: {{ log("Running some_macro: " ~ arg1 ~ ", " ~ arg2) }} {% endmacro %}" """ - # Now, detect instances of the placeholder value ($$$DBT_SECRET_START...DBT_SECRET_END$$$) - # and replace them with the standard mask '*****' + # Detect instances of the placeholder value ($$$DBT_SECRET_START...DBT_SECRET_END$$$) + # and replace it with the standard mask '*****' if "DBT_SECRET_START" in str(msg): search_group = f"({SECRET_ENV_PREFIX}(.*))" - from dbt.context.secret import SECRET_PLACEHOLDER - pattern = SECRET_PLACEHOLDER.format(search_group).replace("$", r"\$") m = re.search( pattern, diff --git a/core/dbt/context/secret.py b/core/dbt/context/secret.py index 6de99fd5e5b..849e4903a8b 100644 --- a/core/dbt/context/secret.py +++ b/core/dbt/context/secret.py @@ -4,13 +4,10 @@ from .base import BaseContext, contextmember -from dbt.constants import SECRET_ENV_PREFIX, DEFAULT_ENV_PLACEHOLDER +from dbt.constants import SECRET_ENV_PREFIX, DEFAULT_ENV_PLACEHOLDER, SECRET_PLACEHOLDER from dbt.exceptions import EnvVarMissingError -SECRET_PLACEHOLDER = "$$$DBT_SECRET_START$$${}$$$DBT_SECRET_END$$$" - - class SecretContext(BaseContext): """This context is used in profiles.yml + packages.yml. It can render secret env vars that aren't usable elsewhere""" diff --git a/tests/functional/dependencies/test_dependency_secrets.py b/tests/functional/dependencies/test_dependency_secrets.py index f1ac92e0b98..7e0b592409b 100644 --- a/tests/functional/dependencies/test_dependency_secrets.py +++ b/tests/functional/dependencies/test_dependency_secrets.py @@ -25,7 +25,7 @@ def packages(self): def test_allow_secrets(self, project): _, log_output = run_dbt_and_capture(["deps"]) - # this will not be written to logs or lock file + # this will not be written to logs assert not ("super secret" in log_output) assert "*****" in log_output assert not ("DBT_ENV_SECRET_FOR_LOGGING" in log_output) From 6bac246e88438be3d409deca15216c5fb2393fd3 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Tue, 23 Apr 2024 14:56:26 -0500 Subject: [PATCH 5/5] cleanup test --- tests/functional/dependencies/test_dependency_secrets.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/functional/dependencies/test_dependency_secrets.py b/tests/functional/dependencies/test_dependency_secrets.py index 7e0b592409b..12d3ecb489d 100644 --- a/tests/functional/dependencies/test_dependency_secrets.py +++ b/tests/functional/dependencies/test_dependency_secrets.py @@ -5,12 +5,12 @@ from dbt_common.constants import SECRET_ENV_PREFIX -class TestAllowSecretProfilePackage: +class TestSecretInPackage: @pytest.fixture(scope="class", autouse=True) def setUp(self): - os.environ[SECRET_ENV_PREFIX + "FOR_LOGGING"] = "super secret" + os.environ[SECRET_ENV_PREFIX + "_FOR_LOGGING"] = "super secret" yield - del os.environ[SECRET_ENV_PREFIX + "FOR_LOGGING"] + del os.environ[SECRET_ENV_PREFIX + "_FOR_LOGGING"] @pytest.fixture(scope="class") def packages(self): @@ -23,7 +23,7 @@ def packages(self): ] } - def test_allow_secrets(self, project): + def test_mask_secrets(self, project): _, log_output = run_dbt_and_capture(["deps"]) # this will not be written to logs assert not ("super secret" in log_output)