From 500d4aa9e92a286b9c3c4b109df8886e8e01bf4f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 20 Jul 2026 10:13:56 -1000 Subject: [PATCH] [wireguard] Mark private keys sensitive, stop redacting public keys (#17736) --- esphome/__main__.py | 12 +++++- esphome/components/wireguard/__init__.py | 4 +- tests/component_tests/wireguard/__init__.py | 1 + tests/component_tests/wireguard/test_init.py | 44 ++++++++++++++++++++ tests/unit_tests/test_main.py | 40 ++++++++++++++++++ 5 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 tests/component_tests/wireguard/__init__.py create mode 100644 tests/component_tests/wireguard/test_init.py diff --git a/esphome/__main__.py b/esphome/__main__.py index 4abd18d239..553a8b390f 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -1510,10 +1510,18 @@ def _redact_with_legacy_fallback(output: str) -> str: m = _LEGACY_REDACTION_RE.search(line) if m is None: continue + key = m.group("key") if not in_substitutions: - unmarked.add(m.group("key")) + # Public keys (e.g. wireguard's peer_public_key) are not secret; + # redacting them and telling maintainers to mark them cv.sensitive + # would be wrong on both counts. Substitution keys are user-named + # with no schema behind them, so anything secret-shaped there + # (public or not) stays conservatively redacted. + if "public" in key.split("_"): + continue + unmarked.add(key) lines[i] = ( - f"{line[: m.start()]}{m.group('key')}: " + f"{line[: m.start()]}{key}: " f"\\033[8m{m.group('val')}\\033[28m{line[m.end() :]}" ) output = "\n".join(lines) diff --git a/esphome/components/wireguard/__init__.py b/esphome/components/wireguard/__init__.py index ff98cfc966..ea9e5a3b0c 100644 --- a/esphome/components/wireguard/__init__.py +++ b/esphome/components/wireguard/__init__.py @@ -74,11 +74,11 @@ CONFIG_SCHEMA = cv.All( cv.GenerateID(CONF_TIME_ID): cv.use_id(time.RealTimeClock), cv.Required(CONF_ADDRESS): cv.ipv4address, cv.Optional(CONF_NETMASK, default="255.255.255.255"): cv.ipv4address, - cv.Required(CONF_PRIVATE_KEY): _wireguard_key, + cv.Required(CONF_PRIVATE_KEY): cv.sensitive(_wireguard_key), cv.Required(CONF_PEER_ENDPOINT): cv.string, cv.Required(CONF_PEER_PUBLIC_KEY): _wireguard_key, cv.Optional(CONF_PEER_PORT, default=51820): cv.port, - cv.Optional(CONF_PEER_PRESHARED_KEY): _wireguard_key, + cv.Optional(CONF_PEER_PRESHARED_KEY): cv.sensitive(_wireguard_key), cv.Optional(CONF_PEER_ALLOWED_IPS, default=["0.0.0.0/0"]): cv.ensure_list( _cidr_network ), diff --git a/tests/component_tests/wireguard/__init__.py b/tests/component_tests/wireguard/__init__.py new file mode 100644 index 0000000000..82b57e8fef --- /dev/null +++ b/tests/component_tests/wireguard/__init__.py @@ -0,0 +1 @@ +"""Tests for the wireguard component.""" diff --git a/tests/component_tests/wireguard/test_init.py b/tests/component_tests/wireguard/test_init.py new file mode 100644 index 0000000000..556d14cd00 --- /dev/null +++ b/tests/component_tests/wireguard/test_init.py @@ -0,0 +1,44 @@ +"""Tests for the wireguard component schema.""" + +import pytest + +from esphome.components.wireguard import CONFIG_SCHEMA +from esphome.const import PlatformFramework +from esphome.yaml_util import SensitiveStr +from tests.component_tests.types import SetCoreConfigCallable + +# Any 42 base64 chars plus a valid terminator satisfies _WG_KEY_REGEX. +PRIVATE_KEY = "a" * 42 + "A=" +PEER_PUBLIC_KEY = "b" * 42 + "A=" +PEER_PRESHARED_KEY = "c" * 42 + "A=" + + +@pytest.mark.parametrize( + ("field", "value", "sensitive"), + [ + ("private_key", PRIVATE_KEY, True), + ("peer_preshared_key", PEER_PRESHARED_KEY, True), + ("peer_public_key", PEER_PUBLIC_KEY, False), + ], +) +def test_key_sensitivity( + field: str, + value: str, + sensitive: bool, + set_core_config: SetCoreConfigCallable, +) -> None: + """The private and preshared keys are secrets and must be tagged so dump + tooling redacts them deterministically; the peer's public key is not a + secret and must stay readable in redacted dumps (see issue #17718).""" + set_core_config(PlatformFramework.ESP32_IDF) + config = CONFIG_SCHEMA( + { + "address": "10.0.0.2", + "private_key": PRIVATE_KEY, + "peer_endpoint": "wg.example.com", + "peer_public_key": PEER_PUBLIC_KEY, + "peer_preshared_key": PEER_PRESHARED_KEY, + } + ) + assert isinstance(config[field], SensitiveStr) == sensitive + assert config[field] == value diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 9a9aafec43..a1ed89bf5d 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -442,6 +442,46 @@ def test_redact_with_legacy_fallback__does_not_match_fragment_as_suffix( assert not any("legacy substring" in rec.message for rec in caplog.records) +@pytest.mark.parametrize("field", ["public_key", "peer_public_key"]) +def test_redact_with_legacy_fallback__skips_public_key_fields( + field: str, + caplog: pytest.LogCaptureFixture, +) -> None: + """Public keys are not secret; fields with a ``public`` name segment + must pass through unredacted and without the migration warning + (see issue #17718).""" + text = f"{field}: c29tZXB1YmxpY2tleQ==\n" + with caplog.at_level(logging.WARNING, logger="esphome.__main__"): + out = _redact_with_legacy_fallback(text) + assert out == text + assert not any("legacy substring" in rec.message for rec in caplog.records) + + +def test_redact_with_legacy_fallback__public_substitution_still_redacted( + caplog: pytest.LogCaptureFixture, +) -> None: + """Substitution keys are user-named with no schema behind them, so the + public-key exemption does not apply there; a ``public``-named substitution + keeps the conservative silent redaction.""" + text = "substitutions:\n public_key: something\nesphome:\n name: x\n" + with caplog.at_level(logging.WARNING, logger="esphome.__main__"): + out = _redact_with_legacy_fallback(text) + assert "public_key: \\033[8msomething\\033[28m" in out + assert not any("legacy substring" in rec.message for rec in caplog.records) + + +def test_redact_with_legacy_fallback__public_must_be_a_whole_segment( + caplog: pytest.LogCaptureFixture, +) -> None: + """The exemption matches ``public`` as an underscore-separated segment, + not a substring; an unrelated name like ``republic_key`` keeps the + conservative redaction.""" + with caplog.at_level(logging.WARNING, logger="esphome.__main__"): + out = _redact_with_legacy_fallback("republic_key: abc\n") + assert "republic_key: \\033[8mabc\\033[28m" in out + assert any("'republic_key'" in rec.message for rec in caplog.records) + + def test_redact_with_legacy_fallback__substitutions_redacted_without_warning( caplog: pytest.LogCaptureFixture, ) -> None: