Skip to content

Commit

Permalink
Scrub secret vars
Browse files Browse the repository at this point in the history
- Scrub secret vars in RequiredVarNotFoundError
- Scrub secret vars in StateCheckVarsHash event
- Scrub secret vars in run results
  • Loading branch information
nielspardon committed Apr 16, 2024
1 parent ee74a60 commit 457fd68
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 5 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20240307-153622.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Support scrubbing secret vars
time: 2024-03-07T15:36:22.754627+01:00
custom:
Author: nielspardon
Issue: "7247"
24 changes: 23 additions & 1 deletion core/dbt/artifacts/schemas/run/v5/run.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import threading
from typing import Any, Optional, Iterable, Tuple, Sequence, Dict, TYPE_CHECKING
import copy
from dataclasses import dataclass, field
from datetime import datetime


from dbt.constants import SECRET_ENV_PREFIX
from dbt.artifacts.resources import CompiledResource
from dbt.artifacts.schemas.base import (
BaseArtifactMetadata,
Expand All @@ -19,6 +21,7 @@
ExecutionResult,
)
from dbt_common.clients.system import write_json
from dbt.exceptions import scrub_secrets


if TYPE_CHECKING:
Expand Down Expand Up @@ -123,7 +126,26 @@ def from_execution_results(
dbt_schema_version=str(cls.dbt_schema_version),
generated_at=generated_at,
)
return cls(metadata=meta, results=processed_results, elapsed_time=elapsed_time, args=args)

secret_vars = [

Check warning on line 130 in core/dbt/artifacts/schemas/run/v5/run.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/artifacts/schemas/run/v5/run.py#L130

Added line #L130 was not covered by tests
v for k, v in args["vars"].items() if k.startswith(SECRET_ENV_PREFIX) and v.strip()
]

scrubbed_args = copy.deepcopy(args)

Check warning on line 134 in core/dbt/artifacts/schemas/run/v5/run.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/artifacts/schemas/run/v5/run.py#L134

Added line #L134 was not covered by tests

# scrub secrets in invocation command
scrubbed_args["invocation_command"] = scrub_secrets(

Check warning on line 137 in core/dbt/artifacts/schemas/run/v5/run.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/artifacts/schemas/run/v5/run.py#L137

Added line #L137 was not covered by tests
scrubbed_args["invocation_command"], secret_vars
)

# scrub secrets in vars dict
scrubbed_args["vars"] = {

Check warning on line 142 in core/dbt/artifacts/schemas/run/v5/run.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/artifacts/schemas/run/v5/run.py#L142

Added line #L142 was not covered by tests
k: scrub_secrets(v, secret_vars) for k, v in scrubbed_args["vars"].items()
}

return cls(

Check warning on line 146 in core/dbt/artifacts/schemas/run/v5/run.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/artifacts/schemas/run/v5/run.py#L146

Added line #L146 was not covered by tests
metadata=meta, results=processed_results, elapsed_time=elapsed_time, args=scrubbed_args
)

@classmethod
def compatible_previous_versions(cls) -> Iterable[Tuple[str, int]]:
Expand Down
7 changes: 6 additions & 1 deletion core/dbt/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

from dbt_common.dataclass_schema import ValidationError

from dbt.constants import SECRET_ENV_PREFIX


if TYPE_CHECKING:
import agate
Expand Down Expand Up @@ -333,7 +335,10 @@ def get_message(self) -> str:
pretty_vars = json.dumps(dct, sort_keys=True, indent=4)

msg = f"Required var '{self.var_name}' not found in config:\nVars supplied to {node_name} = {pretty_vars}"
return msg
return scrub_secrets(msg, self.var_secrets())

def var_secrets(self) -> List[str]:
return [v for k, v in self.merged.items() if k.startswith(SECRET_ENV_PREFIX) and v.strip()]


class PackageNotFoundForMacroError(CompilationError):
Expand Down
7 changes: 6 additions & 1 deletion core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
MANIFEST_FILE_NAME,
PARTIAL_PARSE_FILE_NAME,
SEMANTIC_MANIFEST_FILE_NAME,
SECRET_ENV_PREFIX,
)
from dbt_common.helper_types import PathSet
from dbt_common.events.functions import fire_event, get_invocation_id, warn_or_error
Expand Down Expand Up @@ -116,6 +117,7 @@
TargetNotFoundError,
AmbiguousAliasError,
InvalidAccessTypeError,
scrub_secrets,
)
from dbt.parser.base import Parser
from dbt.parser.analysis import AnalysisParser
Expand Down Expand Up @@ -989,6 +991,9 @@ def build_manifest_state_check(self):
# of env_vars, that would need to change.
# We are using the parsed cli_vars instead of config.args.vars, in order
# to sort them and avoid reparsing because of ordering issues.
secret_vars = [
v for k, v in config.cli_vars.items() if k.startswith(SECRET_ENV_PREFIX) and v.strip()
]
stringified_cli_vars = pprint.pformat(config.cli_vars)
vars_hash = FileHash.from_contents(
"\x00".join(
Expand All @@ -1003,7 +1008,7 @@ def build_manifest_state_check(self):
fire_event(
StateCheckVarsHash(
checksum=vars_hash.checksum,
vars=stringified_cli_vars,
vars=scrub_secrets(stringified_cli_vars, secret_vars),
profile=config.args.profile,
target=config.args.target,
version=__version__,
Expand Down
81 changes: 80 additions & 1 deletion tests/functional/context_methods/test_cli_vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@

from tests.fixtures.dbt_integration_project import dbt_integration_project # noqa: F401

from dbt.tests.util import run_dbt, get_artifact, write_config_file
from dbt.tests.util import (
run_dbt,
run_dbt_and_capture,
get_logging_events,
get_artifact,
write_config_file,
)
from dbt.tests.fixtures.project import write_project_files
from dbt.exceptions import DbtRuntimeError, CompilationError

Expand Down Expand Up @@ -206,3 +212,76 @@ def test_vars_in_selectors(self, project):
# Var in cli_vars works
results = run_dbt(["run", "--vars", "snapshot_target: dev"])
assert len(results) == 1


models_scrubbing__schema_yml = """
version: 2
models:
- name: simple_model
columns:
- name: simple
data_tests:
- accepted_values:
values:
- abc
"""

models_scrubbing__simple_model_sql = """
select
'{{ var("DBT_ENV_SECRET_simple") }}'::varchar as simple
"""


class TestCLIVarsScrubbing:
@pytest.fixture(scope="class")
def models(self):
return {
"schema.yml": models_scrubbing__schema_yml,
"simple_model.sql": models_scrubbing__simple_model_sql,
}

def test__run_results_scrubbing(self, project):
results, output = run_dbt_and_capture(
[
"--debug",
"--log-format",
"json",
"run",
"--vars",
"{DBT_ENV_SECRET_simple: abc, unused: def}",
]
)
assert len(results) == 1

run_results = get_artifact(project.project_root, "target", "run_results.json")
assert run_results["args"]["vars"] == {
"DBT_ENV_SECRET_simple": "*****",
"unused": "def",
}

log_events = get_logging_events(log_output=output, event_name="StateCheckVarsHash")
assert len(log_events) == 1
assert (
log_events[0]["data"]["vars"] == "{'DBT_ENV_SECRET_simple': '*****', 'unused': 'def'}"
)

def test__exception_scrubbing(self, project):
results, output = run_dbt_and_capture(
[
"--debug",
"--log-format",
"json",
"run",
"--vars",
"{DBT_ENV_SECRET_unused: abc, unused: def}",
],
False,
)
assert len(results) == 1

log_events = get_logging_events(log_output=output, event_name="CatchableExceptionOnRun")
assert len(log_events) == 1
assert (
'{\n "DBT_ENV_SECRET_unused": "*****",\n "unused": "def"\n }'
in log_events[0]["info"]["msg"]
)
2 changes: 1 addition & 1 deletion tests/unit/test_parse_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def _new_file(self, searched, name, match):
class TestPartialParse(unittest.TestCase):
def setUp(self) -> None:
mock_project = MagicMock(RuntimeConfig)
mock_project.cli_vars = ""
mock_project.cli_vars = {}
mock_project.args = MagicMock()
mock_project.args.profile = "test"
mock_project.args.target = "test"
Expand Down

0 comments on commit 457fd68

Please sign in to comment.