diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index db3327aa275..5fb95fa0d5c 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -1514,17 +1514,27 @@ class ESPHomeDumper(yaml.SafeDumper): return self.represent_secret(value.value) return self.represent_scalar(tag="!lambda", value=value.value, style="|") + def _represent_tagged_secret(self, tag, value): + # Writing the secret beats leaking the value, but the tag is lost, so + # the dumped config no longer means the same thing when loaded back. + _LOGGER.warning( + "%s value matches secret '%s'; written as !secret, dropping the %s tag", + tag, + _SECRET_VALUES[str(value)], + tag, + ) + return self.represent_secret(value) + def represent_extend(self, value): # Consult is_secret like the other scalar representers so a payload - # equal to a registered secret is never written out in cleartext; the - # tag is lost then, which beats leaking the value. + # equal to a registered secret is never written out in cleartext. if is_secret(value.value): - return self.represent_secret(value.value) + return self._represent_tagged_secret("!extend", value.value) return self.represent_scalar(tag="!extend", value=value.value) def represent_remove(self, value): if is_secret(value.value): - return self.represent_secret(value.value) + return self._represent_tagged_secret("!remove", value.value) return self.represent_scalar(tag="!remove", value=value.value) def represent_include_file(self, value): @@ -1540,7 +1550,7 @@ class ESPHomeDumper(yaml.SafeDumper): tag="!include", mapping=mapping, flow_style=False ) if is_secret(value.file): - return self.represent_secret(value.file) + return self._represent_tagged_secret("!include", value.file) return self.represent_scalar(tag="!include", value=value.file) def represent_id(self, value): diff --git a/tests/unit_tests/test_bundle.py b/tests/unit_tests/test_bundle.py index 8a2510cf0a0..6a086767840 100644 --- a/tests/unit_tests/test_bundle.py +++ b/tests/unit_tests/test_bundle.py @@ -139,10 +139,17 @@ def test_find_used_secret_keys_no_secrets(tmp_path: Path) -> None: assert keys == set() -def test_find_used_secret_keys_missing_file(tmp_path: Path) -> None: +def test_find_used_secret_keys_missing_file( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: missing = tmp_path / "does_not_exist.yaml" - keys = _find_used_secret_keys([missing]) + with caplog.at_level("WARNING", logger="esphome.bundle"): + keys = _find_used_secret_keys([missing]) assert keys == set() + assert any( + "Could not scan" in r.message and "does_not_exist.yaml" in r.message + for r in caplog.records + ) def test_find_used_secret_keys_deduplicates(tmp_path: Path) -> None: diff --git a/tests/unit_tests/test_yaml_util.py b/tests/unit_tests/test_yaml_util.py index bd839ecdde0..914b1948070 100644 --- a/tests/unit_tests/test_yaml_util.py +++ b/tests/unit_tests/test_yaml_util.py @@ -2065,12 +2065,18 @@ def test_merge_include_no_overlap_records_nothing(tmp_path: Path) -> None: assert yaml_util.take_dropped_merge_keys() == [] -def test_wrapper_representers_consult_is_secret() -> None: +def test_wrapper_representers_consult_is_secret( + caplog: pytest.LogCaptureFixture, +) -> None: """!extend / !remove payloads and scalar !include paths equal to a - registered secret are swapped, never written in cleartext.""" + registered secret are swapped, never written in cleartext, and the lost + tag is logged.""" from esphome.config_helpers import Extend, Remove - with yaml_util.secret_values_registered({"hunter2": "the_secret"}): + with ( + caplog.at_level("WARNING", logger="esphome.yaml_util"), + yaml_util.secret_values_registered({"hunter2": "the_secret"}), + ): out = yaml_util.dump( { "a": Extend("hunter2"), @@ -2081,6 +2087,9 @@ def test_wrapper_representers_consult_is_secret() -> None: assert out.count("!secret 'the_secret'") == 2 assert "hunter2" not in out assert "!extend 'plain_id'" in out + lost = [r.message for r in caplog.records if "dropping the" in r.message] + assert len(lost) == 2 + assert all("hunter2" not in m for m in lost) def test_scalar_include_path_equal_to_secret_is_swapped() -> None: