From 0d4ab2093e318c5c0d2b08c07c07738c92c4fd86 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Thu, 16 Dec 2021 16:28:46 -0600 Subject: [PATCH 1/7] scrub message of secrets --- core/dbt/exceptions.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 3994fea7891..256e35aabc7 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -692,9 +692,11 @@ def missing_materialization(model, adapter_type): def bad_package_spec(repo, spec, error_message): - raise InternalException( - "Error checking out spec='{}' for repo {}\n{}".format( - spec, repo, error_message)) + msg = "Error checking out spec='{}' for repo {}\n{}".format(spec, repo, error_message) + for secret in get_secret_env(): + msg = str(msg).replace(secret, "*****") + + raise InternalException(msg) def raise_cache_inconsistent(message): From c0ab7b53b992703e25a1e95f7b58d4e391902e7a Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Thu, 16 Dec 2021 16:37:00 -0600 Subject: [PATCH 2/7] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e9ff13a6ee..f3424f8beba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Fix `DepsStartPackageInstall` event to use package name instead of version number. ([#4482](https://github.com/dbt-labs/dbt-core/pull/4482)) - Reimplement log message to use adapter name instead of the object method. ([#4501](https://github.com/dbt-labs/dbt-core/pull/4501)) - Issue better error message for incompatible schemas ([#4470](https://github.com/dbt-labs/dbt-core/pull/4442), [#4497](https://github.com/dbt-labs/dbt-core/pull/4497)) +- Remove secrets from error related to packages. ([#4507](https://github.com/dbt-labs/dbt-core/pull/4507)) ### Docs - Fix missing data on exposures in docs ([#4467](https://github.com/dbt-labs/dbt-core/issues/4467)) From 3d8ac4b0121221a6844977f56da1de10ae55df55 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 17 Dec 2021 08:36:07 -0600 Subject: [PATCH 3/7] use new scrubbing and scrub more places using git --- core/dbt/clients/git.py | 23 +++++++++++------------ core/dbt/exceptions.py | 29 +++++++++++++++++++---------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/core/dbt/clients/git.py b/core/dbt/clients/git.py index e16da8560e4..b7709e603d4 100644 --- a/core/dbt/clients/git.py +++ b/core/dbt/clients/git.py @@ -8,7 +8,9 @@ GitProgressUpdatingExistingDependency, GitProgressPullingNewDependency, GitNothingToDo, GitProgressUpdatedCheckoutRange, GitProgressCheckedOutAt ) -import dbt.exceptions +from dbt.exceptions import ( + CommandResultError, RuntimeException, bad_package_spec, raise_cloning_problem +) from packaging import version @@ -24,7 +26,7 @@ def _raise_git_cloning_error(repo, revision, error): if re.match("fatal: destination path '(.+)' already exists", stderr): raise error - dbt.exceptions.bad_package_spec(repo, revision, stderr) + bad_package_spec(repo, revision, stderr) def clone(repo, cwd, dirname=None, remove_git_dir=False, revision=None, subdirectory=None): @@ -53,7 +55,7 @@ def clone(repo, cwd, dirname=None, remove_git_dir=False, revision=None, subdirec clone_cmd.append(dirname) try: result = run_cmd(cwd, clone_cmd, env={'LC_ALL': 'C'}) - except dbt.exceptions.CommandResultError as exc: + except CommandResultError as exc: _raise_git_cloning_error(repo, revision, exc) if subdirectory: @@ -61,7 +63,7 @@ def clone(repo, cwd, dirname=None, remove_git_dir=False, revision=None, subdirec clone_cmd_subdir = ['git', 'sparse-checkout', 'set', subdirectory] try: run_cmd(cwd_subdir, clone_cmd_subdir) - except dbt.exceptions.CommandResultError as exc: + except CommandResultError as exc: _raise_git_cloning_error(repo, revision, exc) if remove_git_dir: @@ -105,9 +107,9 @@ def checkout(cwd, repo, revision=None): revision = 'HEAD' try: return _checkout(cwd, repo, revision) - except dbt.exceptions.CommandResultError as exc: + except CommandResultError as exc: stderr = exc.stderr.decode('utf-8').strip() - dbt.exceptions.bad_package_spec(repo, revision, stderr) + bad_package_spec(repo, revision, stderr) def get_current_sha(cwd): @@ -131,14 +133,11 @@ def clone_and_checkout(repo, cwd, dirname=None, remove_git_dir=False, remove_git_dir=remove_git_dir, subdirectory=subdirectory, ) - except dbt.exceptions.CommandResultError as exc: + except CommandResultError as exc: err = exc.stderr.decode('utf-8') exists = re.match("fatal: destination path '(.+)' already exists", err) if not exists: - print( - '\nSomething went wrong while cloning {}'.format(repo) + - '\nCheck the debug logs for more information') - raise + raise_cloning_problem() directory = None start_sha = None @@ -148,7 +147,7 @@ def clone_and_checkout(repo, cwd, dirname=None, remove_git_dir=False, else: matches = re.match("Cloning into '(.+)'", err.decode('utf-8')) if matches is None: - raise dbt.exceptions.RuntimeException( + raise RuntimeException( f'Error cloning {repo} - never saw "Cloning into ..." from git' ) directory = matches.group(1) diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 256e35aabc7..34300e28046 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -2,8 +2,7 @@ import functools from typing import NoReturn, Optional, Mapping, Any -from dbt.logger import get_secret_env -from dbt.events.functions import fire_event +from dbt.events.functions import fire_event, scrub_secrets, env_secrets from dbt.events.types import GeneralWarningMsg, GeneralWarningException from dbt.node_types import NodeType from dbt import flags @@ -54,7 +53,7 @@ class RuntimeException(RuntimeError, Exception): def __init__(self, msg, node=None): self.stack = [] self.node = node - self.msg = msg + self.msg = scrub_secrets(msg, env_secrets()) def add_node(self, node=None): if node is not None and node is not self.node: @@ -400,9 +399,8 @@ class CommandError(RuntimeException): def __init__(self, cwd, cmd, message='Error running command'): super().__init__(message) self.cwd = cwd + cmd = scrub_secrets(cmd, env_secrets()) self.cmd = cmd - for secret in get_secret_env(): - self.cmd = str(self.cmd).replace(secret, "*****") self.args = (cwd, cmd, message) def __str__(self): @@ -466,7 +464,20 @@ def raise_database_error(msg, node=None) -> NoReturn: def raise_dependency_error(msg) -> NoReturn: - raise DependencyException(msg) + raise DependencyException(scrub_secrets(msg, env_secrets())) + + +def raise_cloning_problem(repo) -> NoReturn: + repo = scrub_secrets(repo, env_secrets()) + msg = '''\ + Something went wrong while cloning {} + Check the debug logs for more information + ''' + raise RuntimeException(msg.format(repo)) + + +def raise_git_cloning_problem(msg) -> NoReturn: + raise RuntimeException(scrub_secrets(msg, env_secrets())) def disallow_secret_env_var(env_var_name) -> NoReturn: @@ -693,10 +704,8 @@ def missing_materialization(model, adapter_type): def bad_package_spec(repo, spec, error_message): msg = "Error checking out spec='{}' for repo {}\n{}".format(spec, repo, error_message) - for secret in get_secret_env(): - msg = str(msg).replace(secret, "*****") - raise InternalException(msg) + raise InternalException(scrub_secrets(msg, env_secrets())) def raise_cache_inconsistent(message): @@ -1001,7 +1010,7 @@ def raise_duplicate_alias( def warn_or_error(msg, node=None, log_fmt=None): if flags.WARN_ERROR: - raise_compiler_error(msg, node) + raise_compiler_error(scrub_secrets(msg, env_secrets()), node) else: fire_event(GeneralWarningMsg(msg=msg, log_fmt=log_fmt)) From fee3065ef2b70ec91c66c3178604811f71563104 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 17 Dec 2021 10:54:35 -0600 Subject: [PATCH 4/7] fixed small miss of string conv and missing raise --- core/dbt/clients/git.py | 7 ++++--- core/dbt/exceptions.py | 9 ++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/dbt/clients/git.py b/core/dbt/clients/git.py index b7709e603d4..814277002da 100644 --- a/core/dbt/clients/git.py +++ b/core/dbt/clients/git.py @@ -9,7 +9,8 @@ GitNothingToDo, GitProgressUpdatedCheckoutRange, GitProgressCheckedOutAt ) from dbt.exceptions import ( - CommandResultError, RuntimeException, bad_package_spec, raise_cloning_problem + CommandResultError, RuntimeException, bad_package_spec, raise_git_cloning_error, + raise_git_cloning_problem ) from packaging import version @@ -24,7 +25,7 @@ def _raise_git_cloning_error(repo, revision, error): if 'usage: git' in stderr: stderr = stderr.split('\nusage: git')[0] if re.match("fatal: destination path '(.+)' already exists", stderr): - raise error + raise raise_git_cloning_error(error) bad_package_spec(repo, revision, stderr) @@ -137,7 +138,7 @@ def clone_and_checkout(repo, cwd, dirname=None, remove_git_dir=False, err = exc.stderr.decode('utf-8') exists = re.match("fatal: destination path '(.+)' already exists", err) if not exists: - raise_cloning_problem() + raise_git_cloning_problem() directory = None start_sha = None diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 34300e28046..be32e5d492d 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -399,7 +399,7 @@ class CommandError(RuntimeException): def __init__(self, cwd, cmd, message='Error running command'): super().__init__(message) self.cwd = cwd - cmd = scrub_secrets(cmd, env_secrets()) + cmd = scrub_secrets(str(cmd), env_secrets()) self.cmd = cmd self.args = (cwd, cmd, message) @@ -467,8 +467,7 @@ def raise_dependency_error(msg) -> NoReturn: raise DependencyException(scrub_secrets(msg, env_secrets())) -def raise_cloning_problem(repo) -> NoReturn: - repo = scrub_secrets(repo, env_secrets()) +def raise_git_cloning_problem(repo) -> NoReturn: msg = '''\ Something went wrong while cloning {} Check the debug logs for more information @@ -476,8 +475,8 @@ def raise_cloning_problem(repo) -> NoReturn: raise RuntimeException(msg.format(repo)) -def raise_git_cloning_problem(msg) -> NoReturn: - raise RuntimeException(scrub_secrets(msg, env_secrets())) +def raise_git_cloning_error(msg) -> NoReturn: + raise RuntimeException(msg) def disallow_secret_env_var(env_var_name) -> NoReturn: From 85a0173b1e2764c0933c47bc49edb4c3a304a5f0 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 17 Dec 2021 13:56:07 -0600 Subject: [PATCH 5/7] fix bug with cloning error --- core/dbt/clients/git.py | 7 +++---- core/dbt/exceptions.py | 4 ---- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/core/dbt/clients/git.py b/core/dbt/clients/git.py index 814277002da..509aaefac0f 100644 --- a/core/dbt/clients/git.py +++ b/core/dbt/clients/git.py @@ -9,8 +9,7 @@ GitNothingToDo, GitProgressUpdatedCheckoutRange, GitProgressCheckedOutAt ) from dbt.exceptions import ( - CommandResultError, RuntimeException, bad_package_spec, raise_git_cloning_error, - raise_git_cloning_problem + CommandResultError, RuntimeException, bad_package_spec, raise_git_cloning_problem ) from packaging import version @@ -25,7 +24,7 @@ def _raise_git_cloning_error(repo, revision, error): if 'usage: git' in stderr: stderr = stderr.split('\nusage: git')[0] if re.match("fatal: destination path '(.+)' already exists", stderr): - raise raise_git_cloning_error(error) + raise error bad_package_spec(repo, revision, stderr) @@ -138,7 +137,7 @@ def clone_and_checkout(repo, cwd, dirname=None, remove_git_dir=False, err = exc.stderr.decode('utf-8') exists = re.match("fatal: destination path '(.+)' already exists", err) if not exists: - raise_git_cloning_problem() + raise_git_cloning_problem(repo) directory = None start_sha = None diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index be32e5d492d..348022be760 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -475,10 +475,6 @@ def raise_git_cloning_problem(repo) -> NoReturn: raise RuntimeException(msg.format(repo)) -def raise_git_cloning_error(msg) -> NoReturn: - raise RuntimeException(msg) - - def disallow_secret_env_var(env_var_name) -> NoReturn: """Raise an error when a secret env var is referenced outside allowed rendering contexts""" From 1464f366afd687dca2582b4a7a2f3d16735475d2 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 17 Dec 2021 14:43:32 -0600 Subject: [PATCH 6/7] resolving message issues --- core/dbt/clients/git.py | 5 +++-- core/dbt/exceptions.py | 14 +++++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/core/dbt/clients/git.py b/core/dbt/clients/git.py index 509aaefac0f..cde9ccc77a4 100644 --- a/core/dbt/clients/git.py +++ b/core/dbt/clients/git.py @@ -9,7 +9,8 @@ GitNothingToDo, GitProgressUpdatedCheckoutRange, GitProgressCheckedOutAt ) from dbt.exceptions import ( - CommandResultError, RuntimeException, bad_package_spec, raise_git_cloning_problem + CommandResultError, RuntimeException, bad_package_spec, raise_git_cloning_error, + raise_git_cloning_problem ) from packaging import version @@ -24,7 +25,7 @@ def _raise_git_cloning_error(repo, revision, error): if 'usage: git' in stderr: stderr = stderr.split('\nusage: git')[0] if re.match("fatal: destination path '(.+)' already exists", stderr): - raise error + raise_git_cloning_error(error) bad_package_spec(repo, revision, stderr) diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 348022be760..1c4e6727258 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -399,7 +399,6 @@ class CommandError(RuntimeException): def __init__(self, cwd, cmd, message='Error running command'): super().__init__(message) self.cwd = cwd - cmd = scrub_secrets(str(cmd), env_secrets()) self.cmd = cmd self.args = (cwd, cmd, message) @@ -467,7 +466,20 @@ def raise_dependency_error(msg) -> NoReturn: raise DependencyException(scrub_secrets(msg, env_secrets())) +def raise_git_cloning_error(error: CommandResultError) -> NoReturn: + def __init__(self, cwd, cmd, returncode, stdout, stderr, + message='Got a non-zero returncode'): + self.returncode = returncode + self.stdout = scrub_secrets(stdout, env_secrets()) + self.stderr = scrub_secrets(stderr, env_secrets()) + message = scrub_secrets(message, env_secrets()) + self.args = (cwd, cmd, returncode, stdout, stderr, message) + + raise error + + def raise_git_cloning_problem(repo) -> NoReturn: + repo = scrub_secrets(repo, env_secrets()) msg = '''\ Something went wrong while cloning {} Check the debug logs for more information From c7c8342811fbbc376dcd3d2860c0b69bc9b00741 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 17 Dec 2021 15:49:45 -0600 Subject: [PATCH 7/7] better, more specific scrubbing --- core/dbt/exceptions.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index 1c4e6727258..e7e9e57ef6b 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -467,14 +467,7 @@ def raise_dependency_error(msg) -> NoReturn: def raise_git_cloning_error(error: CommandResultError) -> NoReturn: - def __init__(self, cwd, cmd, returncode, stdout, stderr, - message='Got a non-zero returncode'): - self.returncode = returncode - self.stdout = scrub_secrets(stdout, env_secrets()) - self.stderr = scrub_secrets(stderr, env_secrets()) - message = scrub_secrets(message, env_secrets()) - self.args = (cwd, cmd, returncode, stdout, stderr, message) - + error.cmd = scrub_secrets(str(error.cmd), env_secrets()) raise error