From a3ea77c2f1206939c0f59aa90870485270998b3e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 11:32:15 -0500 Subject: [PATCH] [core] Keep templated !include filenames as strings so Windows path normalization cannot corrupt them (#18549) --- esphome/components/substitutions/__init__.py | 5 +- esphome/yaml_util.py | 28 ++++++---- tests/unit_tests/test_bundle.py | 56 +++++++++++++++++++- tests/unit_tests/test_substitutions.py | 19 +++++++ tests/unit_tests/test_yaml_util.py | 27 +++++++++- 5 files changed, 120 insertions(+), 15 deletions(-) diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index b4fcf36c9e..5ef7a699eb 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -363,13 +363,12 @@ def resolve_include( an explicit non-goal here. """ original = include.file - original_str = str(original) filename = str( _expand_substitutions( - original_str, path + ["file"], context_vars, strict_undefined, errors + original, path + ["file"], context_vars, strict_undefined, errors ) ) - substituted = filename != original_str + substituted = filename != original if substituted: include = include.with_file(filename) try: diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index d3c6caf60b..c280e550c9 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -231,18 +231,22 @@ class IncludeFile: def __init__( self, parent_file: Path, - file: Path | str, + file: str, vars: dict[str, Any] | None, yaml_loader: Callable[[Path], Any], ) -> None: self.parent_file = parent_file - self.file = Path(file) + # The raw include text may be a substitution/Jinja expression, so it + # must never round-trip through Path(): on Windows, WindowsPath str() + # rewrites "/" to "\", which Jinja then decodes as escapes like + # "\b" -> backspace (issue #18545). + self.file = file self.vars = vars self.yaml_loader = yaml_loader self._content: Any = _UNSET def __repr__(self) -> str: - return f"IncludeFile({self.file.as_posix()})" + return f"IncludeFile({self.file})" def load(self) -> Any: """Load and cache the included file content. @@ -258,15 +262,15 @@ class IncludeFile: raise Invalid( f"Cannot load include with unresolved substitutions: {self.file}" ) - self._content = self.yaml_loader(Path(self.parent_file.parent / self.file)) + self._content = self.yaml_loader(self.parent_file.parent / self.file) self._content = add_context(self._content, self.vars) return self._content def has_unresolved_expressions(self) -> bool: """Check if the filename contains substitution variables or Jinja expressions.""" - return has_substitution_or_expression(str(self.file)) + return has_substitution_or_expression(self.file) - def with_file(self, file: Path | str) -> IncludeFile: + def with_file(self, file: str) -> IncludeFile: """Clone this include with *file* as the filename.""" return IncludeFile(self.parent_file, file, self.vars, self.yaml_loader) @@ -313,7 +317,7 @@ def _candidate_include_paths(include: IncludeFile) -> list[Path]: parent_dir = include.parent_file.parent parent_resolved = include.parent_file.resolve() candidates: list[Path] = [] - for pattern in include_candidate_patterns(str(include.file)): + for pattern in include_candidate_patterns(include.file): if "*" in pattern: matches = sorted(_glob_include_candidates(parent_dir, pattern)) else: @@ -362,7 +366,7 @@ def _load_include_candidates( continue expanded_paths.add(candidate) try: - loaded = include.with_file(candidate).load() + loaded = include.with_file(candidate.as_posix()).load() except (EsphomeError, Invalid) as err: # Unlike an unresolved pattern (expected during the discovery # re-parse), a matched on-disk candidate that fails to load is a @@ -794,6 +798,10 @@ class ESPHomeLoaderMixin: file = fields.get("file") if file is None: raise yaml.MarkedYAMLError("Must include 'file'", node.start_mark) + if not isinstance(file, str): + raise yaml.MarkedYAMLError( + "Include 'file' must be a string", node.start_mark + ) vars = fields.get(CONF_VARS) return file, vars @@ -1333,11 +1341,11 @@ class ESPHomeDumper(yaml.SafeDumper): def represent_include_file(self, value): if value.vars: - mapping = {"file": value.file.as_posix(), "vars": value.vars} + mapping = {"file": value.file, "vars": value.vars} return self.represent_mapping( tag="!include", mapping=mapping, flow_style=False ) - return self.represent_scalar(tag="!include", value=value.file.as_posix()) + return self.represent_scalar(tag="!include", value=value.file) def represent_id(self, value): if is_secret(value.id): diff --git a/tests/unit_tests/test_bundle.py b/tests/unit_tests/test_bundle.py index 29e917fe44..1abc7a3ab8 100644 --- a/tests/unit_tests/test_bundle.py +++ b/tests/unit_tests/test_bundle.py @@ -29,8 +29,9 @@ from esphome.bundle import ( read_bundle_manifest, remap_bundle_path, ) +from esphome.components.substitutions import do_substitution_pass from esphome.core import CORE, EsphomeError -from esphome.yaml_util import force_load_include_files +from esphome.yaml_util import force_load_include_files, load_yaml # --------------------------------------------------------------------------- # Helpers @@ -1277,6 +1278,59 @@ def test_discover_files_bundles_all_include_candidates(tmp_path: Path) -> None: assert "includes/empty.yaml" in paths +@pytest.mark.parametrize("enable_proxy", [True, False]) +def test_bundle_roundtrip_templated_include_with_path_separator( + tmp_path: Path, enable_proxy: bool +) -> None: + r"""The issue-18545 flow: a Jinja !include whose branches contain "/" still + resolves after the bundle is extracted on the build server. + + Windows is the leg that regresses: the raw expression text must survive + verbatim, or its separators get rewritten to "\" and Jinja decodes + sequences like "\b" as string escapes. + """ + config_dir = _setup_config_dir( + tmp_path, + files={ + "includes/boards/board.yaml": ( + "packages:\n" + ' - !include ${ "bluetooth/bluetooth_proxy_single_core.yaml"' + ' if enable_bluetooth_proxy else "../empty.yaml" }\n' + ), + "includes/boards/bluetooth/bluetooth_proxy_single_core.yaml": ( + "bluetooth_proxy:\n active: true\n" + ), + "includes/empty.yaml": "{}\n", + }, + ) + (config_dir / "test.yaml").write_text( + "substitutions:\n" + f" enable_bluetooth_proxy: {str(enable_proxy).lower()}\n" + "esphome:\n name: test\n" + "packages:\n - !include includes/boards/board.yaml\n" + ) + + result = ConfigBundleCreator({}).create_bundle() + bundle_path = tmp_path / "device.esphomebundle.tar.gz" + bundle_path.write_bytes(result.data) + + # Both conditional branches must ship in the bundle. + paths = [f.path for f in result.files] + assert "includes/boards/bluetooth/bluetooth_proxy_single_core.yaml" in paths + assert "includes/empty.yaml" in paths + + # Extract to a fresh directory and resolve the config from there, as a + # remote build server would. + extracted_config = extract_bundle(bundle_path, tmp_path / "remote") + config = do_substitution_pass(load_yaml(extracted_config)) + + board_pkg = config["packages"][0]["packages"][0] + if enable_proxy: + assert board_pkg == {"bluetooth_proxy": {"active": True}} + else: + assert board_pkg == {} + + def test_discover_files_candidate_outside_config_dir_skipped( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: diff --git a/tests/unit_tests/test_substitutions.py b/tests/unit_tests/test_substitutions.py index f4063237b1..73c6e496a9 100644 --- a/tests/unit_tests/test_substitutions.py +++ b/tests/unit_tests/test_substitutions.py @@ -744,6 +744,25 @@ def test_include_filename_substitution_undefined_var(tmp_path: Path) -> None: substitutions.do_substitution_pass(config) +def test_include_filename_jinja_expression_with_path_separator( + tmp_path: Path, +) -> None: + """A jinja !include whose string literals contain "/" resolves correctly (issue #18545).""" + main_file = tmp_path / "main.yaml" + main_file.write_text( + "substitutions:\n" + " enable_bluetooth_proxy: true\n" + "result: !include " + '${ "bluetooth/proxy.yaml" if enable_bluetooth_proxy else "../empty.yaml" }\n' + ) + (tmp_path / "bluetooth").mkdir() + (tmp_path / "bluetooth" / "proxy.yaml").write_text("value: 42\n") + + config = yaml_util.load_yaml(main_file) + config = substitutions.do_substitution_pass(config) + assert config["result"] == {"value": 42} + + def test_raise_first_undefined_logs_extras_at_debug( caplog: pytest.LogCaptureFixture, ) -> None: diff --git a/tests/unit_tests/test_yaml_util.py b/tests/unit_tests/test_yaml_util.py index e0a81652e3..3bdbd04396 100644 --- a/tests/unit_tests/test_yaml_util.py +++ b/tests/unit_tests/test_yaml_util.py @@ -701,6 +701,31 @@ def test_include_file_has_unresolved_expressions( assert include.has_unresolved_expressions() == expected +def test_mapping_include_non_string_file_rejected(tmp_path: Path) -> None: + """The mapping !include form rejects a non-string 'file' with a clear error.""" + entry = tmp_path / "entry.yaml" + entry.write_text("wifi: !include\n file: [not, a, string]\n") + with pytest.raises(EsphomeError, match="Include 'file' must be a string"): + yaml_util.load_yaml(entry) + + +def test_include_file_templated_filename_stays_raw_string(tmp_path: Path) -> None: + """A templated filename keeps its verbatim text (issue #18545).""" + parent = tmp_path / "main.yaml" + expr = '${ "bluetooth/proxy.yaml" if enable_bluetooth_proxy else "../empty.yaml" }' + include = yaml_util.IncludeFile(parent, expr, None, lambda _: {}) + assert include.file == expr + assert include.has_unresolved_expressions() + assert repr(include) == f"IncludeFile({expr})" + + +def test_represent_include_file_templated() -> None: + """Dumping a templated IncludeFile emits the raw expression unchanged.""" + expr = '${ "a/b.yaml" if flag else "../c.yaml" }' + include = yaml_util.IncludeFile(Path("/fake/main.yaml"), expr, None, lambda _: {}) + assert yaml_util.dump({"key": include}) == f"key: !include '{expr}'\n" + + def test_include_in_list_context() -> None: """!include of a file returning a list is handled correctly, including when that list itself contains a nested IncludeFile.""" @@ -1051,7 +1076,7 @@ class _StubInclude: ) -> None: # Default parent lives in a nonexistent directory so unresolved # stubs never glob real files during candidate expansion. - self.file = Path(file) + self.file = file self.parent_file = parent_file or Path("/nonexistent/parent.yaml") self._unresolved = unresolved self._load_result = load_result if load_result is not None else {}