From 3b79d6fcaaa8bb39cba7979982ba61ecd5a8d14f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 7 Oct 2026 13:32:02 -1000 Subject: [PATCH] [store_yaml] Emit the blob as a flash symbol instead of a component Codegen emits STORE_YAML_DATA and STORE_YAML_DATA_SIZE the way the wizard does, so the component, its global pointer and the wait for its setup are gone and the API reads the blob directly. Record whether a sensitive value came from !secret once, merge the unencrypted warnings, compute the remote package list once and trim comments. --- esphome/components/api/api_connection.cpp | 57 +++------ esphome/components/api/api_connection.h | 6 +- esphome/components/store_yaml/__init__.py | 110 ++++++------------ esphome/components/store_yaml/store_yaml.cpp | 27 ----- esphome/components/store_yaml/store_yaml.h | 34 +----- esphome/core/defines.h | 1 + .../unit_tests/components/test_store_yaml.py | 12 +- 7 files changed, 65 insertions(+), 182 deletions(-) delete mode 100644 esphome/components/store_yaml/store_yaml.cpp diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 00da533c889..e2a3b900998 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -366,7 +366,7 @@ void APIConnection::loop() { #ifdef USE_STORE_YAML // Guard inline so the idle hot path pays a compare, not a function call. - if (this->store_yaml_pos_ != std::numeric_limits::max()) { + if (this->store_yaml_pos_ != STORE_YAML_IDLE) { this->try_send_store_yaml_(); } #endif @@ -1223,59 +1223,33 @@ void APIConnection::on_camera_image_request(const CameraImageRequest &msg) { #ifdef USE_STORE_YAML 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()) + // A re-request while a transfer is in flight is ignored (see api.proto) + if (this->store_yaml_pos_ != STORE_YAML_IDLE) return; - // All responses go through the loop-driven retry below, so a full TX - // buffer at request time can't strand the client without a terminal frame. this->store_yaml_pos_ = 0; this->try_send_store_yaml_(); } -// Caller guarantees: store_yaml_pos_ != SIZE_MAX (a request is in flight). void APIConnection::try_send_store_yaml_() { - // A client connecting while later components are still setting up (the app - // loop runs for already-initialized components during setup) can request - // YAML before store_yaml's setup() registered the component. Leave the - // request pending; this is retried from loop() until the component appears. - auto *comp = store_yaml::global_store_yaml; - if (comp == nullptr) - return; - // Codegen always embeds a non-empty blob, so total > 0 here. - const size_t total = comp->get_size(); - - const size_t chunk_size = MAX_BATCH_PACKET_SIZE; - - // 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) { - if (!this->helper_->can_write_without_blocking()) - return; - - const size_t remaining = total - this->store_yaml_pos_; - const size_t to_send = std::min(remaining, chunk_size); - + // Advance only after a successful send, so a full TX buffer retries the same chunk from loop() + while (this->helper_->can_write_without_blocking()) { + const size_t to_send = std::min(STORE_YAML_DATA_SIZE - this->store_yaml_pos_, MAX_BATCH_PACKET_SIZE); GetYamlResponse resp; - resp.data = comp->get_data() + this->store_yaml_pos_; + resp.data = store_yaml::STORE_YAML_DATA + this->store_yaml_pos_; resp.data_len = to_send; if (this->store_yaml_pos_ == 0) { - resp.total_size = static_cast(total); + resp.total_size = STORE_YAML_DATA_SIZE; resp.encoding = StringRef(store_yaml::ENCODING); } - resp.done = (this->store_yaml_pos_ + to_send) >= total; - + resp.done = this->store_yaml_pos_ + to_send == STORE_YAML_DATA_SIZE; if (!this->send_message(resp)) - return; // retry on next loop, pos unchanged - + return; + if (resp.done) { + this->store_yaml_pos_ = STORE_YAML_IDLE; + return; + } this->store_yaml_pos_ += to_send; - if (resp.done) - break; } - - // Final response (with done=true) sent successfully. - this->store_yaml_pos_ = std::numeric_limits::max(); } #endif @@ -2158,9 +2132,6 @@ bool APIConnection::send_device_capabilities_response_() { resp.wizard.configured = true; #endif #ifdef USE_STORE_YAML - // Compile-time knowledge: codegen always embeds a non-empty blob when the - // component is compiled in. Deriving this from the runtime pointer could - // report false to a client connecting before store_yaml's setup() ran. resp.store_yaml.supported = true; #endif return this->send_message(resp); diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index 6689933955d..3275d2601ba 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -30,7 +30,6 @@ #include #include -#include #include namespace esphome { @@ -443,8 +442,9 @@ class APIConnection final : public APIServerConnectionBase { #ifdef USE_STORE_YAML void try_send_store_yaml_(); - // Streaming offset into the PROGMEM blob; max() means "not streaming". - size_t store_yaml_pos_{std::numeric_limits::max()}; + static constexpr size_t STORE_YAML_IDLE = std::numeric_limits::max(); + // Streaming offset into STORE_YAML_DATA, or STORE_YAML_IDLE + size_t store_yaml_pos_{STORE_YAML_IDLE}; #endif #ifdef USE_API_HOMEASSISTANT_STATES diff --git a/esphome/components/store_yaml/__init__.py b/esphome/components/store_yaml/__init__.py index 56745e40a94..19cc0f4ab6e 100644 --- a/esphome/components/store_yaml/__init__.py +++ b/esphome/components/store_yaml/__init__.py @@ -12,8 +12,8 @@ from esphome.components import packages from esphome.components.api import CONF_ENCRYPTION from esphome.config_helpers import Extend, Remove import esphome.config_validation as cv -from esphome.const import CONF_API, CONF_ID, CONF_KEY, CONF_RAW_DATA_ID -from esphome.core import CORE, EsphomeError, HexInt, Lambda +from esphome.const import CONF_API, CONF_KEY +from esphome.core import CORE, EsphomeError, Lambda import esphome.final_validate as fv from esphome.helpers import ensure_unique_string, zstd_module from esphome.types import ConfigType @@ -26,8 +26,6 @@ DEPENDENCIES = ["api"] CONF_INCLUDE_SECRETS = "include_secrets" CONF_ALLOW_UNENCRYPTED = "allow_unencrypted" -store_yaml_ns = cg.esphome_ns.namespace("store_yaml") -StoreYamlComponent = store_yaml_ns.class_("StoreYamlComponent", cg.Component) # Compression level for zstd; 22 is the max and gives ~70-90% reduction on YAML. ZSTD_LEVEL = 22 @@ -44,40 +42,28 @@ UNCAPTURED_NOTE_PATH = "store_yaml_uncaptured.yaml" CONFIG_SCHEMA = cv.Schema( { - cv.GenerateID(): cv.declare_id(StoreYamlComponent), - cv.GenerateID(CONF_RAW_DATA_ID): cv.declare_id(cg.uint8), cv.Optional(CONF_INCLUDE_SECRETS, default=False): cv.boolean, cv.Optional(CONF_ALLOW_UNENCRYPTED, default=False): cv.boolean, } -).extend(cv.COMPONENT_SCHEMA) +) def _final_validate(config: ConfigType) -> ConfigType: - """Require API encryption: an unauthenticated client could otherwise pull - the embedded YAML (which may include Wi-Fi credentials or opted-in - secrets). The escape hatch ``allow_unencrypted: true`` exists for - isolated lab setups where the user has accepted the trade-off.""" + """Require an API encryption key unless ``allow_unencrypted`` is set.""" full = fv.full_config.get() api_conf = full.get(CONF_API, {}) - # Explicitly require a configured key: a keyless provisionable - # `api: encryption:` accepts the well-known all-zeros PSK until - # provisioned, so anyone could provision it and pull the YAML. + # A keyless `encryption:` accepts the all-zeros PSK until provisioned encryption = api_conf.get(CONF_ENCRYPTION) or {} if CONF_KEY in encryption: return config if config.get(CONF_ALLOW_UNENCRYPTED): - if config.get(CONF_INCLUDE_SECRETS): - _LOGGER.warning( - "store_yaml is enabled without API encryption and with " - "include_secrets; any client that can reach the device on the " - "network can pull the embedded YAML including the verbatim " - "contents of secrets.yaml." - ) - else: - _LOGGER.warning( - "store_yaml is enabled without API encryption; any client that " - "can reach the device on the network can pull the embedded YAML." - ) + _LOGGER.warning( + "store_yaml is enabled without API encryption; any client that can " + "reach the device on the network can pull the embedded YAML%s.", + " including the verbatim contents of secrets.yaml" + if config.get(CONF_INCLUDE_SECRETS) + else "", + ) return config raise cv.Invalid( "store_yaml requires API encryption (configure `api.encryption.key`). " @@ -198,6 +184,7 @@ def _iter_nodes( class _SensitiveValue: secret_name: str config_path: str # dotted path, for warnings (never log the value itself) + from_secret: bool # loaded through a real `!secret` # Last path segments the value is sensitive at (e.g. {"password"}), used to # tell the value's own occurrences apart from unrelated collisions. sensitive_keys: set[str] @@ -206,24 +193,11 @@ class _SensitiveValue: def _check_sensitive_usage( sensitive: dict[str, _SensitiveValue], trees: dict[str, object] ) -> None: - """One pass over the parse trees that will be dumped, checking every place - a sensitive value shows up beyond its own whole-scalar occurrences. + """Check where sensitive values occur beyond their own whole scalars. - - As a mapping key: fail the build. The dumper's value-keyed swap would - rewrite the key and corrupt the recovered structure. - - Strictly inside a larger scalar (a lambda body, a URL like - http://user:pw@host): the whole-scalar swap cannot redact it, so it - would ship verbatim. Inline sensitive values fail the build (the - move-into-!secret remedy applies); values already sourced from a real - `!secret` only warn, since overlaps like an SSID inside an entity name - are common and the value stays in the user's secrets.yaml either way. - Scanning tree scalars (not serialized text) means key names, tags, and - generated `!secret` references can never false-positive. - - As a whole scalar at an unrelated location: warn only. The swap rewrites - it to the `!secret` reference, which stays semantically identical until - the user fills in a different value during recovery. Occurrences under - the value's own sensitive key and swapped `substitutions:` definitions - (which keep `${...}` references working) are expected and stay silent. + - As a mapping key: fail, the swap would rewrite the key. + - Inside a larger scalar: fail for inline values, warn for real `!secret` values. + - As a whole scalar elsewhere: warn, it is recovered as the `!secret` reference. """ if not sensitive: return @@ -258,7 +232,7 @@ def _check_sensitive_usage( for value, other in sensitive.items(): if value not in text: continue - if yaml_util.is_secret(value) is not None: + if other.from_secret: # The value lives in a real secrets.yaml, so the # move-into-!secret remedy does not apply; overlaps like # an SSID inside an entity name are common and benign. @@ -310,10 +284,13 @@ def _collect_sensitive_values() -> dict[str, _SensitiveValue]: entry = result.get(value) if entry is None: name = yaml_util.is_secret(value) + from_secret = name is not None if name is None: name = ensure_unique_string("_".join(path) or "secret", used) used.add(name) - entry = result[value] = _SensitiveValue(name, ".".join(path), set()) + entry = result[value] = _SensitiveValue( + name, ".".join(path), from_secret, set() + ) if path: entry.sensitive_keys.add(path[-1]) return result @@ -344,14 +321,7 @@ def _uncaptured_note( def _remote_package_descriptions() -> list[str]: - """Describe every remote source packages were fetched from. - - Remote packages are downloaded while the config is processed; the packages - component records each source, and this formats that record. Their files - cannot be embedded, but the entry file still records the package config, so - the config is re-fetchable; this only makes the gap visible instead of - silent. - """ + """Describe every remote source packages were fetched from; their files cannot be embedded.""" return [ f"{source.url}@{source.ref}" if source.ref else source.url for source in packages.get_remote_package_sources() @@ -365,7 +335,7 @@ def _build_secrets_skeleton(keys: set[str]) -> bytes: def _generate_redacted_files( - entries: list[tuple[str, Path]], secret_rels: set[str] + entries: list[tuple[str, Path]], secret_rels: set[str], remote_packages: list[str] ) -> list[tuple[str, bytes]]: """Re-generate each captured file from its parse tree with cv.sensitive values emitted as `!secret ` references, and replace secrets files @@ -397,17 +367,16 @@ def _generate_redacted_files( # redaction promise cannot be verified — fail the build. leaked = [ info.config_path - for value, info in sensitive.items() - if yaml_util.is_secret(value) is None and info.secret_name not in skeleton_keys + for info in sensitive.values() + if not info.from_secret and info.secret_name not in skeleton_keys ] if leaked: - remote = _remote_package_descriptions() raise EsphomeError( "store_yaml: could not redact the sensitive value(s) of " f"{', '.join(leaked)}. The value was not found in any captured " "file; it may be composed via substitutions, set on the command " "line with -s, or defined inside a remote package" - + (f" ({', '.join(remote)})" if remote else "") + + (f" ({', '.join(remote_packages)})" if remote_packages else "") + ". Reference it with `!secret` in the YAML, or set " "`include_secrets: true` to embed secrets deliberately." ) @@ -496,22 +465,15 @@ def unpack_envelope(blob: bytes) -> dict[str, bytes]: async def to_code(config: ConfigType) -> None: cg.add_define("USE_STORE_YAML") - # Discover the user's on-disk YAML files via a fresh re-parse — same - # pattern bundle.py uses. Running at codegen time (rather than keeping a - # listener installed across validation) avoids capturing framework YAML - # that components load internally (e.g. LVGL's `hello_world.yaml`), and - # costs nothing on validate-only runs or configs without this component. - # This re-parse (and the per-file loads in _generate_redacted_files) also - # repopulates yaml_util's secret registry, which save_compiled_config - # wiped earlier in write_cpp via dump(show_secrets=True); the redaction - # swap and is_secret() checks below rely on that registration. + # Re-parse at codegen time to capture only user files; this also refills the + # secret registry that save_compiled_config cleared, which redaction relies on. discovered = yaml_util.discover_user_yaml_files(CORE.config_path) entries, secret_rels = _gather_files(discovered) + remote_packages = _remote_package_descriptions() if config[CONF_INCLUDE_SECRETS]: files = _read_files_verbatim(entries) else: - files = _generate_redacted_files(entries, secret_rels) - remote_packages = _remote_package_descriptions() + files = _generate_redacted_files(entries, secret_rels, remote_packages) if remote_packages: _LOGGER.warning( "store_yaml: %d package(s) come from remote sources and cannot be " @@ -533,9 +495,7 @@ async def to_code(config: ConfigType) -> None: 100.0 * len(compressed) / len(envelope), ) - rhs = [HexInt(b) for b in compressed] - prog_arr = cg.progmem_array(config[CONF_RAW_DATA_ID], rhs) - - var = cg.new_Pvariable(config[CONF_ID]) - await cg.register_component(var, config) - cg.add(var.set_data(prog_arr, len(compressed), len(envelope))) + cg.extern_progmem_array( + "esphome::store_yaml::STORE_YAML_DATA", cg.uint8, list(compressed) + ) + cg.add_define("STORE_YAML_DATA_SIZE", len(compressed)) diff --git a/esphome/components/store_yaml/store_yaml.cpp b/esphome/components/store_yaml/store_yaml.cpp deleted file mode 100644 index 0da1ad479ba..00000000000 --- a/esphome/components/store_yaml/store_yaml.cpp +++ /dev/null @@ -1,27 +0,0 @@ -#include "store_yaml.h" - -#ifdef USE_STORE_YAML - -#include "esphome/core/log.h" - -namespace esphome::store_yaml { - -static const char *const TAG = "store_yaml"; - -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -StoreYamlComponent *global_store_yaml = nullptr; - -void StoreYamlComponent::setup() { global_store_yaml = this; } - -void StoreYamlComponent::dump_config() { - ESP_LOGCONFIG(TAG, - "YAML:\n" - " Compressed size: %zu bytes\n" - " Uncompressed size: %zu bytes\n" - " Encoding: %s", - this->size_, this->uncompressed_size_, ENCODING); -} - -} // namespace esphome::store_yaml - -#endif // USE_STORE_YAML diff --git a/esphome/components/store_yaml/store_yaml.h b/esphome/components/store_yaml/store_yaml.h index c76a68c42a7..2aa5436d756 100644 --- a/esphome/components/store_yaml/store_yaml.h +++ b/esphome/components/store_yaml/store_yaml.h @@ -3,41 +3,17 @@ #include "esphome/core/defines.h" #ifdef USE_STORE_YAML -#include "esphome/core/component.h" +#include + #include "esphome/core/hal.h" namespace esphome::store_yaml { -// "zstd" — published in GetYamlResponse.encoding so clients know how to decompress. +/// Published in GetYamlResponse.encoding so clients know how to decompress. constexpr const char *ENCODING = "zstd"; -class StoreYamlComponent : public Component { - public: - void setup() override; - void dump_config() override; - - // Called once from codegen with the PROGMEM blob. - void set_data(const uint8_t *data, size_t size, size_t uncompressed_size) { - this->data_ = data; - this->size_ = size; - this->uncompressed_size_ = uncompressed_size; - } - size_t get_size() const { return this->size_; } - - // Raw pointer to the PROGMEM blob. On ESP8266 the address is in instruction - // flash and must be read via `progmem_memcpy`; everywhere else PROGMEM is a - // no-op and the data is directly addressable. - const uint8_t *get_data() const { return this->data_; } - - protected: - // Points to a `const uint8_t[] PROGMEM` array emitted by codegen. - const uint8_t *data_{nullptr}; - size_t size_{0}; - size_t uncompressed_size_{0}; -}; - -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -extern StoreYamlComponent *global_store_yaml; +/// The YAML envelope as zstd, built by codegen and kept in flash. STORE_YAML_DATA_SIZE bytes long. +extern const uint8_t STORE_YAML_DATA[] PROGMEM; } // namespace esphome::store_yaml diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 127c83f934b..6b11d2115e0 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -286,6 +286,7 @@ #define API_MAX_SEND_QUEUE 8 #define API_USER_ACTION_STRINGS_SCRATCH_SIZE 64 #define API_WIZARD_DATA_SIZE 1 +#define STORE_YAML_DATA_SIZE 1 #define API_WIZARD_INPUT_COUNT 1 #define MAX_API_CONNECTIONS 6 // The Improv library is not in the Zephyr tidy environment diff --git a/tests/unit_tests/components/test_store_yaml.py b/tests/unit_tests/components/test_store_yaml.py index 3293497c841..004de8a909d 100644 --- a/tests/unit_tests/components/test_store_yaml.py +++ b/tests/unit_tests/components/test_store_yaml.py @@ -57,7 +57,9 @@ def _sources( def _gather_redacted(discovered: DiscoveredYamlFiles) -> dict[str, bytes]: entries, secret_rels = _gather_files(discovered) - return dict(_generate_redacted_files(entries, secret_rels)) + return dict( + _generate_redacted_files(entries, secret_rels, _remote_package_descriptions()) + ) # --------------------------------------------------------------------------- @@ -306,12 +308,12 @@ def test_remote_package_descriptions_read_packages_record( processing are formatted as url@ref (url alone when ref is absent).""" monkeypatch.delitem(CORE.data, packages.DOMAIN, raising=False) data = packages._get_data() - data.remote_sources.append( + data.remote_sources[ packages.RemotePackageSource("https://github.com/org/repo", "main") - ) - data.remote_sources.append( + ] = None + data.remote_sources[ packages.RemotePackageSource("https://github.com/org/other", None) - ) + ] = None assert _remote_package_descriptions() == [ "https://github.com/org/repo@main", "https://github.com/org/other",