From 3d77e3f5ddc467e0ba70d8b972d55283639cc7b2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 15 May 2026 02:02:41 -0700 Subject: [PATCH] [store_yaml] Address review: capability bit, dump suppression, paths, unit tests - Advertise `has_store_yaml` in DeviceInfoResponse so recovery tooling can detect support without timing out against firmware built without USE_STORE_YAML. - Suppress GetYamlResponse from the proto dump path. Every chunk would otherwise log embedded configuration (including opted-in secrets) on builds with HAS_PROTO_MESSAGE_DUMP, and bloat logs during recovery. - Preserve the include graph for files outside the project root: use `os.path.relpath` instead of just the basename so e.g. two `../common.yaml` siblings don't collide on recovery. - Keep `track_yaml_loads` open across `validate_config` so files loaded by remote packages and substitution-resolved includes are captured. - Add focused unit tests for `_gather_files` (redaction, secrets.yml, opt-in, dedupe, external-path handling, missing sources) and `_pack_envelope` (round-trip, UTF-8 paths, overlong-path guard). - Make the test_bundle assertion case-insensitive. --- esphome/components/api/api.proto | 5 + esphome/components/api/api_connection.cpp | 10 +- esphome/components/api/api_pb2.cpp | 6 + esphome/components/api/api_pb2.h | 5 +- esphome/components/api/api_pb2_dump.cpp | 3 + esphome/components/store_yaml/__init__.py | 8 +- esphome/config.py | 41 +++--- .../unit_tests/components/test_store_yaml.py | 132 ++++++++++++++++++ 8 files changed, 190 insertions(+), 20 deletions(-) create mode 100644 tests/unit_tests/components/test_store_yaml.py diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index d83bc6bcef1..a54cffd704e 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -298,6 +298,11 @@ message DeviceInfoResponse { // Serial proxy instance metadata repeated SerialProxyInfo serial_proxies = 25 [(field_ifdef) = "USE_SERIAL_PROXY", (fixed_array_size_define) = "SERIAL_PROXY_COUNT"]; + + // Whether this firmware embeds its YAML configuration for recovery via + // `get_yaml`. Clients use this to skip the request entirely when the + // device cannot answer it instead of waiting for a timeout. + bool has_store_yaml = 26 [(field_ifdef) = "USE_STORE_YAML"]; } message ListEntitiesRequest { diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 5cae20b18e2..c05174a50d2 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1874,6 +1874,9 @@ bool APIConnection::send_device_info_response_() { #ifdef USE_DEEP_SLEEP resp.has_deep_sleep = deep_sleep::global_has_deep_sleep; #endif +#ifdef USE_STORE_YAML + resp.has_store_yaml = store_yaml::global_store_yaml != nullptr && store_yaml::global_store_yaml->get_size() > 0; +#endif #ifdef ESPHOME_PROJECT_NAME #ifdef USE_ESP8266 static const char PROJECT_NAME_PROGMEM[] PROGMEM = ESPHOME_PROJECT_NAME; @@ -2121,10 +2124,15 @@ bool APIConnection::try_to_clear_buffer_slow_(bool log_out_of_space) { bool APIConnection::send_message_(uint32_t payload_size, uint8_t message_type, MessageEncodeFn encode_fn, const void *msg) { #ifdef HAS_PROTO_MESSAGE_DUMP - // Skip dump for log messages (recursive logging risk) and camera frames (high-frequency noise) + // Skip dump for log messages (recursive logging risk), camera frames (high-frequency noise), + // and YAML recovery payloads (every chunk would log the embedded config, including any + // secrets the user opted into). if (message_type != SubscribeLogsResponse::MESSAGE_TYPE #ifdef USE_CAMERA && message_type != CameraImageResponse::MESSAGE_TYPE +#endif +#ifdef USE_STORE_YAML + && message_type != GetYamlResponse::MESSAGE_TYPE #endif ) { auto *proto_msg = static_cast(msg); diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 53b76bc2501..49d44933f80 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -150,6 +150,9 @@ uint8_t *DeviceInfoResponse::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_ for (const auto &it : this->serial_proxies) { ProtoEncode::encode_sub_message(pos PROTO_ENCODE_DEBUG_ARG, buffer, 25, it); } +#endif +#ifdef USE_STORE_YAML + ProtoEncode::encode_bool(pos PROTO_ENCODE_DEBUG_ARG, 26, this->has_store_yaml); #endif return pos; } @@ -212,6 +215,9 @@ uint32_t DeviceInfoResponse::calculate_size() const { for (const auto &it : this->serial_proxies) { size += ProtoSize::calc_message_force(2, it.calculate_size()); } +#endif +#ifdef USE_STORE_YAML + size += ProtoSize::calc_bool(2, this->has_store_yaml); #endif return size; } diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 96ac871b459..9a403179988 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -525,7 +525,7 @@ class SerialProxyInfo final : public ProtoMessage { class DeviceInfoResponse final : public ProtoMessage { public: static constexpr uint8_t MESSAGE_TYPE = 10; - static constexpr uint16_t ESTIMATED_SIZE = 309; + static constexpr uint16_t ESTIMATED_SIZE = 312; #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("device_info_response"); } #endif @@ -580,6 +580,9 @@ class DeviceInfoResponse final : public ProtoMessage { #endif #ifdef USE_SERIAL_PROXY std::array serial_proxies{}; +#endif +#ifdef USE_STORE_YAML + bool has_store_yaml{false}; #endif uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const; uint32_t calculate_size() const; diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 1c652582da8..8f51c68e30b 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -971,6 +971,9 @@ const char *DeviceInfoResponse::dump_to(DumpBuffer &out) const { it.dump_to(out); out.append("\n"); } +#endif +#ifdef USE_STORE_YAML + dump_field(out, ESPHOME_PSTR("has_store_yaml"), this->has_store_yaml); #endif return out.c_str(); } diff --git a/esphome/components/store_yaml/__init__.py b/esphome/components/store_yaml/__init__.py index 28bbd2562a9..3a4d25d00ea 100644 --- a/esphome/components/store_yaml/__init__.py +++ b/esphome/components/store_yaml/__init__.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +import os from pathlib import Path import struct from types import ModuleType @@ -85,8 +86,11 @@ def _gather_files(include_secrets: bool) -> list[tuple[str, bytes]]: try: rel_str = path.relative_to(root).as_posix() except ValueError: - # Outside the project root (e.g. secrets.yaml in $HOME); store basename only. - rel_str = path.name + # Outside the project root (e.g. ../common.yaml or a secrets file in + # $HOME). Use a relative path with ".." components instead of just + # the basename so the include graph is preserved and files from + # different directories with the same basename don't collide. + rel_str = os.path.relpath(path, root).replace(os.sep, "/") files.append((rel_str, content)) diff --git a/esphome/config.py b/esphome/config.py index a82e41242af..6c919219679 100644 --- a/esphome/config.py +++ b/esphome/config.py @@ -1191,25 +1191,34 @@ def _load_config( command_line_substitutions: dict[str, Any], skip_external_update: bool = False ) -> Config: """Load the configuration file.""" - try: - with yaml_util.track_yaml_loads() as loaded_files: + # Keep the file-load listener active across both the YAML parse and the + # validation pass. Substitution and packages resolve deferred `!include` + # references during validation (and remote packages download YAML on + # demand), so the listener must still be installed when those secondary + # loads happen. Components that want the on-disk YAML at codegen time + # (e.g. store_yaml for firmware recovery) read the list out of + # CORE.data["yaml_sources"] after a successful validation. + with yaml_util.track_yaml_loads() as loaded_files: + try: config = yaml_util.load_yaml(CORE.config_path) - # Resolve deferred !include / package references so the listener - # captures every reachable file. Components that want the on-disk - # YAML at codegen time (e.g. store_yaml for firmware recovery) - # read the list out of CORE.data["yaml_sources"] below. + # Resolve any deferred `!include`/package references whose paths + # don't depend on substitutions, so they're captured here too. yaml_util.force_load_include_files(config) - CORE.data["yaml_sources"] = loaded_files - except EsphomeError as e: - raise InvalidYAMLError(e) from e + except EsphomeError as e: + raise InvalidYAMLError(e) from e - try: - return validate_config(config, command_line_substitutions, skip_external_update) - except EsphomeError: - raise - except Exception: - _LOGGER.error("Unexpected exception while reading configuration:") - raise + try: + result = validate_config( + config, command_line_substitutions, skip_external_update + ) + except EsphomeError: + raise + except Exception: + _LOGGER.error("Unexpected exception while reading configuration:") + raise + + CORE.data["yaml_sources"] = loaded_files + return result def load_config( diff --git a/tests/unit_tests/components/test_store_yaml.py b/tests/unit_tests/components/test_store_yaml.py new file mode 100644 index 00000000000..1b9cce936f1 --- /dev/null +++ b/tests/unit_tests/components/test_store_yaml.py @@ -0,0 +1,132 @@ +"""Tests for the store_yaml component's file gathering and envelope packing.""" + +from __future__ import annotations + +from pathlib import Path +import struct + +import pytest + +from esphome.components.store_yaml import ( + ENVELOPE_MAGIC, + REDACTED_PLACEHOLDER, + _gather_files, + _pack_envelope, +) +from esphome.core import CORE, EsphomeError + + +def _unpack_envelope(blob: bytes) -> dict[str, bytes]: + """Inverse of `_pack_envelope` for assertions in tests.""" + assert blob[:4] == ENVELOPE_MAGIC, "envelope must start with EHY1 magic" + pos = 4 + (count,) = struct.unpack_from(" Path: + """Lay out a tiny ESPHome-like project: entry yaml, an include, and a secrets file.""" + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / "entry.yaml").write_text("esphome:\n name: test\n") + (project_dir / "wifi.yaml").write_text("ssid: my_ssid\npassword: my_password\n") + (project_dir / "secrets.yaml").write_text("api_key: SUPER_SECRET\n") + return project_dir + + +@pytest.fixture(autouse=True) +def _reset_core() -> None: + CORE.data.pop("yaml_sources", None) + CORE.config_path = None + yield + CORE.data.pop("yaml_sources", None) + CORE.config_path = None + + +def _set_sources(project_dir: Path, *names: str) -> None: + CORE.config_path = project_dir / "entry.yaml" + CORE.data["yaml_sources"] = [project_dir / name for name in names] + + +def test_gather_redacts_secrets_by_default(project: Path) -> None: + _set_sources(project, "entry.yaml", "wifi.yaml", "secrets.yaml") + files = dict(_gather_files(include_secrets=False)) + assert files["secrets.yaml"] == REDACTED_PLACEHOLDER + assert b"SUPER_SECRET" not in files["secrets.yaml"] + assert files["wifi.yaml"] == (project / "wifi.yaml").read_bytes() + + +def test_gather_redacts_yml_extension(project: Path) -> None: + yml = project / "secrets.yml" + yml.write_text("api_key: OTHER_SECRET\n") + _set_sources(project, "entry.yaml", "secrets.yml") + files = dict(_gather_files(include_secrets=False)) + assert files["secrets.yml"] == REDACTED_PLACEHOLDER + + +def test_gather_embeds_secrets_when_opted_in(project: Path) -> None: + _set_sources(project, "entry.yaml", "secrets.yaml") + files = dict(_gather_files(include_secrets=True)) + assert b"SUPER_SECRET" in files["secrets.yaml"] + + +def test_gather_uses_relative_path_for_external_files( + project: Path, tmp_path: Path +) -> None: + """Files outside the project root use a ``..``-style relative path so they don't collide.""" + sibling = tmp_path / "outside.yaml" + sibling.write_text("foo: bar\n") + _set_sources(project, "entry.yaml") + CORE.data["yaml_sources"].append(sibling) + files = dict(_gather_files(include_secrets=False)) + # project root is `tmp_path/project`, sibling is in `tmp_path` so it + # resolves to `../outside.yaml`. + assert "../outside.yaml" in files + + +def test_gather_deduplicates(project: Path) -> None: + _set_sources(project, "entry.yaml", "wifi.yaml", "wifi.yaml") + files = _gather_files(include_secrets=False) + paths = [p for p, _ in files] + assert paths.count("wifi.yaml") == 1 + + +def test_gather_raises_when_no_sources(project: Path) -> None: + CORE.config_path = project / "entry.yaml" + with pytest.raises(EsphomeError): + _gather_files(include_secrets=False) + + +def test_pack_envelope_roundtrip() -> None: + files = [ + ("entry.yaml", b"esphome:\n name: test\n"), + ("wifi.yaml", b"ssid: a\n"), + ] + blob = _pack_envelope(files) + assert _unpack_envelope(blob) == dict(files) + + +def test_pack_envelope_handles_utf8_paths() -> None: + files = [("dossiers/maison.yaml", b"foo: bar\n")] + blob = _pack_envelope(files) + assert _unpack_envelope(blob) == dict(files) + + +def test_pack_envelope_rejects_overlong_path() -> None: + long_path = "a" * (0xFFFF + 1) + with pytest.raises(EsphomeError): + _pack_envelope([(long_path, b"")])