diff --git a/esphome/bundle.py b/esphome/bundle.py index d3a982658d..0315f9adb1 100644 --- a/esphome/bundle.py +++ b/esphome/bundle.py @@ -107,7 +107,13 @@ def _find_used_secret_keys(yaml_files: list[Path]) -> set[str]: for fpath in yaml_files: try: text = fpath.read_text(encoding="utf-8") - except (OSError, UnicodeDecodeError): + except (OSError, UnicodeDecodeError) as err: + _LOGGER.warning( + "Could not scan %s for !secret references (%s); the bundled " + "secret set may be incomplete", + fpath, + err, + ) continue keys |= yaml_util.find_secret_references(text) return keys @@ -383,6 +389,18 @@ class ConfigBundleCreator: must ship every candidate so the remote build can pick any one. """ discovered = yaml_util.discover_user_yaml_files(self._config_path) + if discovered.load_errors: + _LOGGER.warning( + "Bundle may be incomplete; could not load all configuration files: %s", + "; ".join(discovered.load_errors), + ) + if discovered.unresolved: + _LOGGER.warning( + "Bundle may be incomplete; %d !include path(s) use " + "substitutions and cannot be captured: %s", + len(discovered.unresolved), + ", ".join(discovered.unresolved), + ) self._secrets_paths.update(discovered.secrets) config_resolved = self._config_path.resolve() for fpath in discovered.files: diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 5d8535eacc..c0cfd55063 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -2764,6 +2764,10 @@ message BluetoothSetConnectionParamsResponse { // Embed the user's YAML in firmware and stream it back over the API so a lost // config can be recovered from a running device. The device only stores the // compressed bytes; decompression happens client-side. +// +// A GetYamlRequest received while a transfer is already streaming on the same +// connection is ignored; the in-flight transfer continues undisturbed. To +// restart a transfer, reconnect. message GetYamlRequest { option (id) = 149; option (source) = SOURCE_CLIENT; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 38ca18198a..fa8ad3d4d5 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1202,20 +1202,24 @@ void APIConnection::on_camera_image_request(const CameraImageRequest &msg) { #endif #ifdef USE_STORE_YAML -// Chunk size per GetYamlResponse. Small enough to leave room for the protobuf frame -// inside the 65535-byte API limit and friendly to TCP MSS. -static constexpr size_t STORE_YAML_CHUNK_SIZE = 512; #ifdef USE_ESP8266 // On ESP8266 the blob lives in instruction flash and can't be read directly, so -// each chunk is bounced through this static buffer via progmem_memcpy. Shared -// across connections is safe because the API loop is single-threaded and each -// chunk is filled and consumed atomically inside one `try_send_store_yaml_` -// iteration. Every other platform sends straight from the blob, zero-copy. -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -static uint8_t store_yaml_chunk_buf[STORE_YAML_CHUNK_SIZE]; +// each chunk is bounced through a heap buffer via progmem_memcpy. The buffer +// exists only while a transfer is in flight; retrieval is rare, so no RAM is +// held for the firmware's lifetime. Every other platform sends straight from +// the blob, zero-copy, in MTU-sized chunks. +static constexpr size_t STORE_YAML_CHUNK_SIZE = 512; #endif void APIConnection::on_get_yaml_request() { + // A re-request while a transfer is in flight is ignored; see the + // GetYamlRequest comment in api.proto. A client that wants to restart + // must reconnect. + if (this->store_yaml_pos_ != std::numeric_limits::max()) + return; +#ifdef USE_ESP8266 + this->store_yaml_chunk_buf_ = std::make_unique(STORE_YAML_CHUNK_SIZE); +#endif // All responses — including the single data-less done=true frame for a // missing/empty blob — go through the loop-driven retry below, so a full // TX buffer at request time can't strand the client without a terminal frame. @@ -1230,6 +1234,12 @@ void APIConnection::try_send_store_yaml_() { // treat that like an empty blob and send just the terminal frame. const size_t total = comp == nullptr ? 0 : comp->get_size(); +#ifdef USE_ESP8266 + const size_t chunk_size = STORE_YAML_CHUNK_SIZE; +#else + const size_t chunk_size = MAX_BATCH_PACKET_SIZE; +#endif + // Camera-style streaming: advance the position only after a successful send, // so a WOULD_BLOCK simply retries the same chunk on the next loop iteration. while (true) { @@ -1237,13 +1247,13 @@ void APIConnection::try_send_store_yaml_() { return; const size_t remaining = total - this->store_yaml_pos_; - const size_t to_send = std::min(remaining, STORE_YAML_CHUNK_SIZE); + const size_t to_send = std::min(remaining, chunk_size); GetYamlResponse resp; if (to_send != 0) { #ifdef USE_ESP8266 - progmem_memcpy(store_yaml_chunk_buf, comp->get_data() + this->store_yaml_pos_, to_send); - resp.set_data(store_yaml_chunk_buf, to_send); + progmem_memcpy(this->store_yaml_chunk_buf_.get(), comp->get_data() + this->store_yaml_pos_, to_send); + resp.set_data(this->store_yaml_chunk_buf_.get(), to_send); #else resp.set_data(comp->get_data() + this->store_yaml_pos_, to_send); #endif @@ -1268,6 +1278,9 @@ void APIConnection::try_send_store_yaml_() { // Final response (with done=true) sent successfully. this->store_yaml_pos_ = std::numeric_limits::max(); +#ifdef USE_ESP8266 + this->store_yaml_chunk_buf_.reset(); +#endif } #endif diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index aac061c4e2..8a7dfa4283 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -29,6 +29,7 @@ #include #include +#include #include namespace esphome { @@ -406,6 +407,12 @@ class APIConnection final : public APIServerConnectionBase { void try_send_store_yaml_(); // Streaming offset into the PROGMEM blob; max() means "not streaming". size_t store_yaml_pos_{std::numeric_limits::max()}; +#ifdef USE_ESP8266 + // Bounce buffer for progmem_memcpy, alive only while a transfer is in + // flight; retrieval is rare, so the RAM is not held for the firmware's + // lifetime. Freed on the terminal frame or with the connection. + std::unique_ptr store_yaml_chunk_buf_; +#endif #endif #ifdef USE_API_HOMEASSISTANT_STATES diff --git a/esphome/components/store_yaml/__init__.py b/esphome/components/store_yaml/__init__.py index d950465d5d..40982cf5dc 100644 --- a/esphome/components/store_yaml/__init__.py +++ b/esphome/components/store_yaml/__init__.py @@ -171,6 +171,40 @@ class _SensitiveValue: config_path: str # dotted path, for warnings (never log the value itself) +def _iter_scalars( + node: object, path: tuple[str, ...] = () +) -> Generator[tuple[tuple[str, ...], object]]: + """Yield (config_path, value) for every scalar in a config tree.""" + if isinstance(node, dict): + for key, value in node.items(): + yield from _iter_scalars(value, (*path, str(key))) + elif isinstance(node, (list, tuple)): + for item in node: + yield from _iter_scalars(item, path) + elif isinstance(node, (str, int, float)) and not isinstance(node, bool): + yield path, node + + +def _warn_sensitive_collisions(sensitive: dict[str, _SensitiveValue]) -> None: + """Redaction is value-keyed: any scalar equal to a sensitive value is + rewritten to its `!secret` reference, including unrelated ones (e.g. + `platform: esp32` when a password is literally "esp32"). Filling in a + different value during recovery would then silently rewrite those + unrelated scalars too — warn so the trap is documented, not silent.""" + for path, value in _iter_scalars(CORE.config): + if isinstance(value, yaml_util.SensitiveStr): + continue + info = sensitive.get(str(value)) + if info is not None: + _LOGGER.warning( + "store_yaml: the sensitive value at %s also matches the scalar " + "at %s; the recovered config will reference !secret %s there too", + info.config_path, + ".".join(path), + info.secret_name, + ) + + def _collect_sensitive_values() -> dict[str, _SensitiveValue]: """Map each cv.sensitive value in the validated config to the `!secret` name it should be recovered as. @@ -197,20 +231,68 @@ def _collect_sensitive_values() -> dict[str, _SensitiveValue]: return result -# Envelope path of the note recording includes that could not be captured. +# Envelope path of the note recording content that could not be captured. UNCAPTURED_NOTE_PATH = "store_yaml_uncaptured.yaml" -def _uncaptured_note(unresolved: list[str]) -> tuple[str, bytes]: - """Comment-only YAML entry listing includes that could not be captured, so +def _uncaptured_note( + unresolved: list[str], remote_packages: list[str] +) -> tuple[str, bytes]: + """Comment-only YAML entry listing content that could not be captured, so a recovered config never silently appears complete. Emitted for both the redacted and verbatim paths; user files are never modified to carry it.""" - text = ( - "# store_yaml: the following !include paths use substitutions and\n" - "# could not be captured; restore these files manually:\n" - + "".join(f"# {inc}\n" for inc in unresolved) - ) - return (UNCAPTURED_NOTE_PATH, text.encode("utf-8")) + parts = ["# store_yaml: the following content could not be captured.\n"] + if unresolved: + parts.append( + "# These !include paths use substitutions; restore the files manually:\n" + + "".join(f"# {inc}\n" for inc in unresolved) + ) + if remote_packages: + parts.append( + "# These packages come from remote sources; re-fetch them to\n" + "# complete this config:\n" + + "".join(f"# {pkg}\n" for pkg in remote_packages) + ) + return (UNCAPTURED_NOTE_PATH, "".join(parts).encode("utf-8")) + + +def _find_remote_packages(entries: list[tuple[str, Path]]) -> list[str]: + """Describe every `packages:` entry that pulls content from a remote source. + + Remote packages are downloaded during validation, which the fresh parse + used for discovery never reaches, so their files cannot be embedded. The + entry file still records the source, so the config is re-fetchable; this + only makes the gap visible instead of silent. + """ + remote: list[str] = [] + for _, path in entries: + try: + tree = yaml_util.load_yaml(path, clear_secrets=False) + except EsphomeError: + # Discovery already loaded this file once; a failure here would + # have been reported as a load_error and failed the build. + continue + if not isinstance(tree, dict): + continue + packages = tree.get("packages") + if isinstance(packages, dict): + candidates = packages.items() + elif isinstance(packages, list): + candidates = ((None, item) for item in packages) + else: + continue + for name, value in candidates: + desc = None + if isinstance(value, dict) and "url" in value: + url = value.get("url") + ref = value.get("ref") + desc = f"{url}@{ref}" if ref else str(url) + elif isinstance(value, str) and "//" in value: + # Shorthand form, e.g. `github://org/repo/file.yaml@main` + desc = value + if desc is not None: + remote.append(f"{name}: {desc}" if name is not None else desc) + return remote def _build_secrets_skeleton(keys: set[str]) -> bytes: @@ -233,6 +315,7 @@ def _generate_redacted_files( comments and formatting of the originals are not preserved. """ sensitive = _collect_sensitive_values() + _warn_sensitive_collisions(sensitive) texts: dict[str, str] = {} registered = {value: info.secret_name for value, info in sensitive.items()} @@ -264,6 +347,28 @@ def _generate_redacted_files( "embed secrets deliberately." ) + # The swap replaces whole scalars only. A sensitive value embedded inside + # a larger scalar — a lambda body, a URL like http://user:pw@host — is not + # swapped there, so scan the generated output for every sensitive value as + # a substring and fail the build on any hit. This can false-positive on + # short values that legitimately appear as substrings (an SSID of "esp32" + # inside "esp32dev"); shipping a promised-redacted secret is the worse + # failure, so fail closed. + embedded = [ + f"{info.config_path} (inside {rel})" + for value, info in sensitive.items() + for rel, text in texts.items() + if value in text + ] + if embedded: + raise EsphomeError( + "store_yaml: sensitive value(s) appear embedded inside larger " + f"values: {', '.join(embedded)}. Redaction only replaces whole " + "scalars, so these would ship unredacted. Move the value into a " + "`!secret` referenced on its own, or set `include_secrets: true` " + "to embed secrets deliberately." + ) + skeleton = _build_secrets_skeleton(skeleton_keys) result = [ (rel, skeleton if rel in secret_rels else texts[rel].encode("utf-8")) @@ -284,7 +389,13 @@ def _pack_envelope(files: list[tuple[str, bytes]]) -> bytes: All integers are little-endian. """ parts: list[bytes] = [ENVELOPE_MAGIC, struct.pack(" 0xFFFF: raise EsphomeError( @@ -299,7 +410,13 @@ def _pack_envelope(files: list[tuple[str, bytes]]) -> bytes: def unpack_envelope(blob: bytes) -> dict[str, bytes]: """Inverse of `_pack_envelope`: the reference decoder for the EHY1 envelope, - used by tests and client-side recovery tooling.""" + used by tests and client-side recovery tooling. + + Absolute and drive-qualified paths are rejected: the packer never emits + them, so their presence means a malformed or hostile envelope. Relative + paths with ``..`` components are legitimate (the packer emits them for + files outside the config root), so callers that write files to disk must + still confine the resulting paths to their target directory.""" if blob[:4] != ENVELOPE_MAGIC: raise EsphomeError("envelope must start with EHY1 magic") pos = 4 @@ -313,6 +430,8 @@ def unpack_envelope(blob: bytes) -> dict[str, bytes]: if pos + path_len > len(blob): raise EsphomeError("truncated envelope") path = blob[pos : pos + path_len].decode("utf-8") + if path.startswith(("/", "\\")) or (len(path) >= 2 and path[1] == ":"): + raise EsphomeError(f"envelope contains non-relative path: {path}") pos += path_len (content_len,) = struct.unpack_from(" None: files = _read_files_verbatim(entries) else: files = _generate_redacted_files(entries, secret_rels) - if discovered.unresolved: - files.append(_uncaptured_note(discovered.unresolved)) + remote_packages = _find_remote_packages(entries) + if remote_packages: + _LOGGER.warning( + "store_yaml: %d package(s) come from remote sources and cannot be " + "captured (%s); the embedded recovery data records the source so " + "they can be re-fetched", + len(remote_packages), + ", ".join(remote_packages), + ) + if discovered.unresolved or remote_packages: + files.append(_uncaptured_note(discovered.unresolved, remote_packages)) envelope = _pack_envelope(files) compressed = zstd.compress(envelope, level=ZSTD_LEVEL) diff --git a/tests/components/store_yaml/validate.esp32-idf.yaml b/tests/components/store_yaml/validate.esp32-idf.yaml new file mode 100644 index 0000000000..cc09a556d4 --- /dev/null +++ b/tests/components/store_yaml/validate.esp32-idf.yaml @@ -0,0 +1,9 @@ +wifi: + ssid: MySSID + password: password1 + +api: + encryption: + key: AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8= + +store_yaml: diff --git a/tests/unit_tests/components/test_store_yaml.py b/tests/unit_tests/components/test_store_yaml.py index 95e5b182de..4d281527d7 100644 --- a/tests/unit_tests/components/test_store_yaml.py +++ b/tests/unit_tests/components/test_store_yaml.py @@ -9,8 +9,11 @@ import pytest from esphome import yaml_util from esphome.components.store_yaml import ( + CONF_ALLOW_UNENCRYPTED, SECRETS_SKELETON_HEADER, UNCAPTURED_NOTE_PATH, + _final_validate, + _find_remote_packages, _gather_files, _generate_redacted_files, _pack_envelope, @@ -18,7 +21,9 @@ from esphome.components.store_yaml import ( _uncaptured_note, unpack_envelope, ) +import esphome.config_validation as cv from esphome.core import CORE, EsphomeError +import esphome.final_validate as fv from esphome.yaml_util import DiscoveredYamlFiles, SensitiveStr @@ -213,18 +218,15 @@ def test_redacted_quoted_inline_value(project: Path, quote: str) -> None: def test_redacted_swap_is_whole_scalar_and_value_keyed(project: Path) -> None: """Every whole scalar equal to the sensitive value is swapped (value-keyed, - like `!secret` itself); substrings inside other scalars are never touched. - The recovered config stays semantically identical once the secret is filled.""" - (project / "wifi.yaml").write_text( - "platform: esp32\nnote: esp32 is great\npassword: esp32\n" - ) + like `!secret` itself). The recovered config stays semantically identical + once the secret is filled.""" + (project / "wifi.yaml").write_text("platform: esp32\npassword: esp32\n") CORE.config = {"wifi": [{"password": SensitiveStr("esp32")}]} discovered = _sources(project, "wifi.yaml") files = _gather_redacted(discovered) text = files["wifi.yaml"].decode() assert "password: !secret 'wifi_password'" in text assert "platform: !secret 'wifi_password'" in text - assert "note: esp32 is great" in text def test_redacted_include_reference_round_trips(project: Path) -> None: @@ -277,13 +279,53 @@ def test_redacted_accepts_secret_only_values(project: Path) -> None: def test_uncaptured_note_lists_missing_includes() -> None: """Substitution-pathed includes that can't be captured are recorded in a dedicated envelope entry (both modes), not just a compile-time log line.""" - rel, content = _uncaptured_note(["${board}.yaml"]) + rel, content = _uncaptured_note(["${board}.yaml"], []) assert rel == UNCAPTURED_NOTE_PATH text = content.decode() assert text.startswith("# store_yaml:") assert "# ${board}.yaml" in text +def test_uncaptured_note_lists_remote_packages() -> None: + """Remote packages that can't be captured are recorded with their source + so the user knows to re-fetch them.""" + rel, content = _uncaptured_note( + [], ["base: https://github.com/org/repo@main", "github://org/repo/file.yaml"] + ) + assert rel == UNCAPTURED_NOTE_PATH + text = content.decode() + assert "# base: https://github.com/org/repo@main" in text + assert "# github://org/repo/file.yaml" in text + + +def test_find_remote_packages_detects_url_and_shorthand(project: Path) -> None: + """`packages:` entries with a url (dict or shorthand string) are reported; + local `!include` packages are not.""" + (project / "entry.yaml").write_text( + "packages:\n" + " base:\n" + " url: https://github.com/org/repo\n" + " ref: main\n" + " files: [common.yaml]\n" + " shorthand: github://org/repo/file.yaml@main\n" + " local: !include wifi.yaml\n" + "esphome:\n name: test\n" + ) + discovered = _sources(project, "entry.yaml", "wifi.yaml") + entries, _ = _gather_files(discovered) + remote = _find_remote_packages(entries) + assert remote == [ + "base: https://github.com/org/repo@main", + "shorthand: github://org/repo/file.yaml@main", + ] + + +def test_find_remote_packages_ignores_local_only(project: Path) -> None: + discovered = _sources(project, "entry.yaml", "wifi.yaml") + entries, _ = _gather_files(discovered) + assert _find_remote_packages(entries) == [] + + def test_redacted_skips_empty_sensitive_values(project: Path) -> None: """Empty defaults (e.g. mqtt password) are never swapped.""" (project / "wifi.yaml").write_text("ssid: my_ssid\n") @@ -362,3 +404,87 @@ def test_unpack_envelope_rejects_trailing_bytes() -> None: blob = _pack_envelope([("entry.yaml", b"esphome:\n")]) with pytest.raises(EsphomeError, match="trailing"): unpack_envelope(blob + b"\x00") + + +def test_pack_envelope_rejects_duplicate_paths() -> None: + """A duplicate path would silently clobber the earlier entry on unpack.""" + with pytest.raises(EsphomeError, match="duplicate"): + _pack_envelope([("entry.yaml", b"a: 1\n"), ("entry.yaml", b"b: 2\n")]) + + +@pytest.mark.parametrize("path", ["/etc/passwd", "\\evil.yaml", "C:/evil.yaml"]) +def test_unpack_envelope_rejects_non_relative_paths(path: str) -> None: + """The packer never emits absolute or drive-qualified paths, so their + presence means a malformed or hostile envelope.""" + blob = _pack_envelope([(path, b"boom\n")]) + with pytest.raises(EsphomeError, match="non-relative"): + unpack_envelope(blob) + + +# --------------------------------------------------------------------------- +# embedded-value leak scan and collision warning +# --------------------------------------------------------------------------- + + +def test_redacted_embedded_sensitive_value_fails_build(project: Path) -> None: + """A sensitive value inside a larger scalar (URL, lambda body) is not + swapped by the whole-scalar redaction; the substring scan fails closed.""" + (project / "wifi.yaml").write_text( + "password: my_password\nurl: http://user:my_password@host\n" + ) + CORE.config = {"wifi": [{"password": SensitiveStr("my_password")}]} + discovered = _sources(project, "wifi.yaml") + with pytest.raises(EsphomeError, match="embedded"): + _gather_redacted(discovered) + + +def test_redacted_warns_on_value_collision( + project: Path, caplog: pytest.LogCaptureFixture +) -> None: + """An unrelated scalar equal to a sensitive value gets rewritten by the + value-keyed swap; a warning documents the trap.""" + (project / "wifi.yaml").write_text("password: esp32\nplatform: esp32\n") + CORE.config = { + "wifi": [{"password": SensitiveStr("esp32")}], + "sensor": [{"platform": "esp32"}], + } + discovered = _sources(project, "wifi.yaml") + files = _gather_redacted(discovered) + assert files["wifi.yaml"] == ( + b"password: !secret 'wifi_password'\nplatform: !secret 'wifi_password'\n" + ) + assert "also matches the scalar at sensor.platform" in caplog.text + + +# --------------------------------------------------------------------------- +# _final_validate encryption gate +# --------------------------------------------------------------------------- + + +def _run_final_validate(full_config: dict, config: dict) -> dict: + token = fv.full_config.set(full_config) + try: + return _final_validate(config) + finally: + fv.full_config.reset(token) + + +def test_final_validate_accepts_encrypted_api() -> None: + config = {CONF_ALLOW_UNENCRYPTED: False} + full = { + "api": {"encryption": {"key": "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8="}} + } + assert _run_final_validate(full, config) is config + + +def test_final_validate_rejects_unencrypted_api() -> None: + with pytest.raises(cv.Invalid, match="requires API encryption"): + _run_final_validate({"api": {}}, {CONF_ALLOW_UNENCRYPTED: False}) + + +def test_final_validate_allows_unencrypted_with_escape_hatch( + caplog: pytest.LogCaptureFixture, +) -> None: + config = {CONF_ALLOW_UNENCRYPTED: True} + assert _run_final_validate({"api": {}}, config) is config + assert "without API encryption" in caplog.text