From ba8e72944344eb54b389c512cd22dfe02240e58c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Apr 2026 02:25:00 -0500 Subject: [PATCH 1/4] [substitutions] [packages] Fix `substitutions: !include file.yaml` regression Resolve a deferred IncludeFile before validating the substitutions shape in do_substitution_pass, and before wrapping it in UserDict in do_packages_pass. Fixes esphome/esphome#15848 --- esphome/components/packages/__init__.py | 9 ++++++++- esphome/components/substitutions/__init__.py | 5 +++++ .../15-substitutions_as_include.approved.yaml | 5 +++++ .../substitutions/15-substitutions_as_include.input.yaml | 5 +++++ .../fixtures/substitutions/15-substitutions_inc.yaml | 1 + ...-substitutions_as_include_with_packages.approved.yaml | 5 +++++ .../16-substitutions_as_include_with_packages.input.yaml | 9 +++++++++ 7 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/fixtures/substitutions/15-substitutions_as_include.approved.yaml create mode 100644 tests/unit_tests/fixtures/substitutions/15-substitutions_as_include.input.yaml create mode 100644 tests/unit_tests/fixtures/substitutions/15-substitutions_inc.yaml create mode 100644 tests/unit_tests/fixtures/substitutions/16-substitutions_as_include_with_packages.approved.yaml create mode 100644 tests/unit_tests/fixtures/substitutions/16-substitutions_as_include_with_packages.input.yaml diff --git a/esphome/components/packages/__init__.py b/esphome/components/packages/__init__.py index 3f3df75351..58acc94a41 100644 --- a/esphome/components/packages/__init__.py +++ b/esphome/components/packages/__init__.py @@ -516,7 +516,14 @@ def do_packages_pass( if CONF_PACKAGES not in config: return config - substitutions = UserDict(config.pop(CONF_SUBSTITUTIONS, {})) + raw_substitutions = config.pop(CONF_SUBSTITUTIONS, {}) + if isinstance(raw_substitutions, yaml_util.IncludeFile): + # Resolve `substitutions: !include file.yaml` before feeding into UserDict. + with cv.prepend_path(CONF_SUBSTITUTIONS): + raw_substitutions, _ = resolve_include( + raw_substitutions, [], ContextVars(), strict_undefined=False + ) + substitutions = UserDict(raw_substitutions) processor = _PackageProcessor( substitutions, command_line_substitutions, skip_update ) diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index 94aebbbfe3..3cdf79beea 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -429,6 +429,11 @@ def do_substitution_pass( # Use merge_dicts_ordered to preserve OrderedDict type for move_to_end() substitutions = config.pop(CONF_SUBSTITUTIONS, {}) with cv.prepend_path(CONF_SUBSTITUTIONS): + if isinstance(substitutions, IncludeFile): + # Resolve `substitutions: !include file.yaml` before validating the shape. + substitutions, _ = resolve_include( + substitutions, [], ContextVars(), strict_undefined=False + ) if not isinstance(substitutions, dict): raise cv.Invalid( f"Substitutions must be a key to value mapping, got {type(substitutions)}" diff --git a/tests/unit_tests/fixtures/substitutions/15-substitutions_as_include.approved.yaml b/tests/unit_tests/fixtures/substitutions/15-substitutions_as_include.approved.yaml new file mode 100644 index 0000000000..14aa707def --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/15-substitutions_as_include.approved.yaml @@ -0,0 +1,5 @@ +substitutions: + wifi_password: sub_password +wifi: + ssid: main_ssid + password: sub_password diff --git a/tests/unit_tests/fixtures/substitutions/15-substitutions_as_include.input.yaml b/tests/unit_tests/fixtures/substitutions/15-substitutions_as_include.input.yaml new file mode 100644 index 0000000000..5909e7bf4f --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/15-substitutions_as_include.input.yaml @@ -0,0 +1,5 @@ +substitutions: !include 15-substitutions_inc.yaml + +wifi: + ssid: main_ssid + password: $wifi_password diff --git a/tests/unit_tests/fixtures/substitutions/15-substitutions_inc.yaml b/tests/unit_tests/fixtures/substitutions/15-substitutions_inc.yaml new file mode 100644 index 0000000000..44d9a4b9ef --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/15-substitutions_inc.yaml @@ -0,0 +1 @@ +wifi_password: sub_password diff --git a/tests/unit_tests/fixtures/substitutions/16-substitutions_as_include_with_packages.approved.yaml b/tests/unit_tests/fixtures/substitutions/16-substitutions_as_include_with_packages.approved.yaml new file mode 100644 index 0000000000..14aa707def --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/16-substitutions_as_include_with_packages.approved.yaml @@ -0,0 +1,5 @@ +substitutions: + wifi_password: sub_password +wifi: + ssid: main_ssid + password: sub_password diff --git a/tests/unit_tests/fixtures/substitutions/16-substitutions_as_include_with_packages.input.yaml b/tests/unit_tests/fixtures/substitutions/16-substitutions_as_include_with_packages.input.yaml new file mode 100644 index 0000000000..a2e72f33a2 --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/16-substitutions_as_include_with_packages.input.yaml @@ -0,0 +1,9 @@ +substitutions: !include 15-substitutions_inc.yaml + +packages: + wifi_pkg: + wifi: + password: $wifi_password + +wifi: + ssid: main_ssid From e67b65b669b8a92d0d10edb38448807dfb13d50d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Apr 2026 02:42:03 -0500 Subject: [PATCH 2/4] [substitutions] [packages] Address review feedback - Seed `resolve_include` context with `command_line_substitutions` so `substitutions: !include ${var}.yaml` can reference CLI-provided vars in the include filename (parallels the `packages: !include` path). - Validate shape of resolved substitutions in `do_packages_pass` and raise `cv.Invalid` under `CONF_SUBSTITUTIONS` instead of letting `UserDict()` fail with a low-level exception on a non-mapping. - Fixture 17 exercises the CLI-templated include filename. --- esphome/components/packages/__init__.py | 17 +++++++++++++---- esphome/components/substitutions/__init__.py | 7 ++++++- ...-substitutions_include_cli_var.approved.yaml | 6 ++++++ .../17-substitutions_include_cli_var.input.yaml | 8 ++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 tests/unit_tests/fixtures/substitutions/17-substitutions_include_cli_var.approved.yaml create mode 100644 tests/unit_tests/fixtures/substitutions/17-substitutions_include_cli_var.input.yaml diff --git a/esphome/components/packages/__init__.py b/esphome/components/packages/__init__.py index 58acc94a41..a46783bb58 100644 --- a/esphome/components/packages/__init__.py +++ b/esphome/components/packages/__init__.py @@ -517,11 +517,20 @@ def do_packages_pass( return config raw_substitutions = config.pop(CONF_SUBSTITUTIONS, {}) - if isinstance(raw_substitutions, yaml_util.IncludeFile): - # Resolve `substitutions: !include file.yaml` before feeding into UserDict. - with cv.prepend_path(CONF_SUBSTITUTIONS): + with cv.prepend_path(CONF_SUBSTITUTIONS): + if isinstance(raw_substitutions, yaml_util.IncludeFile): + # Resolve `substitutions: !include file.yaml` before feeding into UserDict. + # Seed with command-line substitutions so `!include ${var}.yaml` can + # reference CLI-provided vars in the filename. raw_substitutions, _ = resolve_include( - raw_substitutions, [], ContextVars(), strict_undefined=False + raw_substitutions, + [], + ContextVars(command_line_substitutions or {}), + strict_undefined=False, + ) + if not isinstance(raw_substitutions, dict): + raise cv.Invalid( + f"Substitutions must be a key to value mapping, got {type(raw_substitutions)}" ) substitutions = UserDict(raw_substitutions) processor = _PackageProcessor( diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index 3cdf79beea..6f24011210 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -431,8 +431,13 @@ def do_substitution_pass( with cv.prepend_path(CONF_SUBSTITUTIONS): if isinstance(substitutions, IncludeFile): # Resolve `substitutions: !include file.yaml` before validating the shape. + # Seed with command-line substitutions so `!include ${var}.yaml` can + # reference CLI-provided vars in the filename. substitutions, _ = resolve_include( - substitutions, [], ContextVars(), strict_undefined=False + substitutions, + [], + ContextVars(command_line_substitutions or {}), + strict_undefined=False, ) if not isinstance(substitutions, dict): raise cv.Invalid( diff --git a/tests/unit_tests/fixtures/substitutions/17-substitutions_include_cli_var.approved.yaml b/tests/unit_tests/fixtures/substitutions/17-substitutions_include_cli_var.approved.yaml new file mode 100644 index 0000000000..f1fd5fb078 --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/17-substitutions_include_cli_var.approved.yaml @@ -0,0 +1,6 @@ +substitutions: + subs_file: 15-substitutions_inc + wifi_password: sub_password +wifi: + ssid: main_ssid + password: sub_password diff --git a/tests/unit_tests/fixtures/substitutions/17-substitutions_include_cli_var.input.yaml b/tests/unit_tests/fixtures/substitutions/17-substitutions_include_cli_var.input.yaml new file mode 100644 index 0000000000..3248504b46 --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/17-substitutions_include_cli_var.input.yaml @@ -0,0 +1,8 @@ +command_line_substitutions: + subs_file: 15-substitutions_inc + +substitutions: !include ${subs_file}.yaml + +wifi: + ssid: main_ssid + password: $wifi_password From f579cf48a0c4e14ef56e0a18ce63d3800378b80c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Apr 2026 02:46:22 -0500 Subject: [PATCH 3/4] [substitutions] [packages] Extract resolve_substitutions_block helper Deduplicate the resolve+shape-check block between do_substitution_pass and do_packages_pass, and drop the raw_substitutions/substitutions naming split. --- esphome/components/packages/__init__.py | 20 +++------- esphome/components/substitutions/__init__.py | 42 +++++++++++++------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/esphome/components/packages/__init__.py b/esphome/components/packages/__init__.py index a46783bb58..252a24061a 100644 --- a/esphome/components/packages/__init__.py +++ b/esphome/components/packages/__init__.py @@ -10,6 +10,7 @@ from esphome.components.substitutions import ( ContextVars, push_context, resolve_include, + resolve_substitutions_block, substitute, ) from esphome.components.substitutions.jinja import has_jinja @@ -516,23 +517,12 @@ def do_packages_pass( if CONF_PACKAGES not in config: return config - raw_substitutions = config.pop(CONF_SUBSTITUTIONS, {}) with cv.prepend_path(CONF_SUBSTITUTIONS): - if isinstance(raw_substitutions, yaml_util.IncludeFile): - # Resolve `substitutions: !include file.yaml` before feeding into UserDict. - # Seed with command-line substitutions so `!include ${var}.yaml` can - # reference CLI-provided vars in the filename. - raw_substitutions, _ = resolve_include( - raw_substitutions, - [], - ContextVars(command_line_substitutions or {}), - strict_undefined=False, + substitutions = UserDict( + resolve_substitutions_block( + config.pop(CONF_SUBSTITUTIONS, {}), command_line_substitutions ) - if not isinstance(raw_substitutions, dict): - raise cv.Invalid( - f"Substitutions must be a key to value mapping, got {type(raw_substitutions)}" - ) - substitutions = UserDict(raw_substitutions) + ) processor = _PackageProcessor( substitutions, command_line_substitutions, skip_update ) diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index 6f24011210..dfdf186e5a 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -414,6 +414,31 @@ def _warn_unresolved_variables(errors: ErrList) -> None: ) +def resolve_substitutions_block( + substitutions: Any, + command_line_substitutions: dict[str, Any] | None, +) -> dict[str, Any]: + """Resolve a deferred ``substitutions: !include file.yaml`` and validate the shape. + + The caller is responsible for wrapping the call in + ``cv.prepend_path(CONF_SUBSTITUTIONS)`` for error reporting. + ``command_line_substitutions`` seeds the filename context so + ``substitutions: !include ${var}.yaml`` can reference CLI-provided vars. + """ + if isinstance(substitutions, IncludeFile): + substitutions, _ = resolve_include( + substitutions, + [], + ContextVars(command_line_substitutions or {}), + strict_undefined=False, + ) + if not isinstance(substitutions, dict): + raise cv.Invalid( + f"Substitutions must be a key to value mapping, got {type(substitutions)}" + ) + return substitutions + + def do_substitution_pass( config: OrderedDict, command_line_substitutions: dict[str, Any] | None = None ) -> OrderedDict: @@ -429,20 +454,9 @@ def do_substitution_pass( # Use merge_dicts_ordered to preserve OrderedDict type for move_to_end() substitutions = config.pop(CONF_SUBSTITUTIONS, {}) with cv.prepend_path(CONF_SUBSTITUTIONS): - if isinstance(substitutions, IncludeFile): - # Resolve `substitutions: !include file.yaml` before validating the shape. - # Seed with command-line substitutions so `!include ${var}.yaml` can - # reference CLI-provided vars in the filename. - substitutions, _ = resolve_include( - substitutions, - [], - ContextVars(command_line_substitutions or {}), - strict_undefined=False, - ) - if not isinstance(substitutions, dict): - raise cv.Invalid( - f"Substitutions must be a key to value mapping, got {type(substitutions)}" - ) + substitutions = resolve_substitutions_block( + substitutions, command_line_substitutions + ) substitutions = merge_dicts_ordered( substitutions, command_line_substitutions or {} ) From 78853260a5b9a9fd676dc7515f454ba7fa161d73 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Apr 2026 02:50:20 -0500 Subject: [PATCH 4/4] [substitutions] Lock in shape check and document single-shot resolve - Add unit tests asserting cv.Invalid when `substitutions: !include list.yaml` resolves to a non-mapping, covering both do_substitution_pass and do_packages_pass. - Note in resolve_substitutions_block that the resolve is single-shot and chained top-level includes are not supported (matches _walk_packages for `packages: !include`). --- esphome/components/substitutions/__init__.py | 3 ++ tests/unit_tests/test_substitutions.py | 51 ++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index dfdf186e5a..e451ad5db8 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -426,6 +426,9 @@ def resolve_substitutions_block( ``substitutions: !include ${var}.yaml`` can reference CLI-provided vars. """ if isinstance(substitutions, IncludeFile): + # Single-shot resolution — matches ``_walk_packages`` for the + # ``packages: !include`` entry point. Chained includes (an include that + # itself loads another ``!include`` at the top level) are not supported. substitutions, _ = resolve_include( substitutions, [], diff --git a/tests/unit_tests/test_substitutions.py b/tests/unit_tests/test_substitutions.py index 01c669e542..71bbd9db86 100644 --- a/tests/unit_tests/test_substitutions.py +++ b/tests/unit_tests/test_substitutions.py @@ -675,6 +675,57 @@ def test_include_filename_substitution_undefined_var(tmp_path: Path) -> None: substitutions.do_substitution_pass(config) +def test_do_substitution_pass_included_substitutions_must_be_mapping( + tmp_path: Path, +) -> None: + """`substitutions: !include list.yaml` where the file holds a list raises cv.Invalid. + + Locks in the shape check that runs after the deferred IncludeFile has been + resolved. + """ + parent = tmp_path / "main.yaml" + parent.write_text("") + + def loader(path: Path): + return ["not", "a", "mapping"] + + include = yaml_util.IncludeFile(parent, "subs.yaml", None, loader) + config = OrderedDict({CONF_SUBSTITUTIONS: include}) + + with pytest.raises( + cv.Invalid, match="Substitutions must be a key to value mapping" + ): + substitutions.do_substitution_pass(config) + + +def test_do_packages_pass_included_substitutions_must_be_mapping( + tmp_path: Path, +) -> None: + """`substitutions: !include list.yaml` alongside `packages:` raises cv.Invalid. + + Without the shape check, ``UserDict(...)`` would surface a low-level + ``TypeError``; the explicit ``cv.Invalid`` points at the substitutions path. + """ + parent = tmp_path / "main.yaml" + parent.write_text("") + + def loader(path: Path): + return ["not", "a", "mapping"] + + include = yaml_util.IncludeFile(parent, "subs.yaml", None, loader) + config = OrderedDict( + { + CONF_SUBSTITUTIONS: include, + "packages": {"noop": {"wifi": {"ssid": "main"}}}, + } + ) + + with pytest.raises( + cv.Invalid, match="Substitutions must be a key to value mapping" + ): + do_packages_pass(config) + + def test_resolve_package_undefined_var_in_include_filename(tmp_path: Path) -> None: """An undefined substitution in a package include filename raises cv.Invalid.