From e1cedaba877fcb183a9f8c49b78d4f2464b49f45 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jul 2026 13:28:22 -1000 Subject: [PATCH] [bundle] Remap absolute file paths when compiling an extracted bundle (#17765) --- esphome/bundle.py | 101 +++++++- esphome/config_validation.py | 30 ++- tests/unit_tests/test_bundle.py | 270 ++++++++++++++++++++- tests/unit_tests/test_config_validation.py | 77 ++++++ 4 files changed, 469 insertions(+), 9 deletions(-) diff --git a/esphome/bundle.py b/esphome/bundle.py index 88df87c3ba..dcaea03646 100644 --- a/esphome/bundle.py +++ b/esphome/bundle.py @@ -12,7 +12,7 @@ from enum import StrEnum import io import json import logging -from pathlib import Path +from pathlib import Path, PurePath, PurePosixPath, PureWindowsPath import re import shutil import tarfile @@ -51,6 +51,7 @@ class ManifestKey(StrEnum): MANIFEST_VERSION = "manifest_version" ESPHOME_VERSION = "esphome_version" CONFIG_FILENAME = "config_filename" + CONFIG_DIR = "config_dir" FILES = "files" HAS_SECRETS = "has_secrets" @@ -127,6 +128,12 @@ class BundleData: """Files components asked to include, keyed under DOMAIN in CORE.data.""" extra_files: list[Path] = field(default_factory=list) + # Original config dir parsed from an extracted bundle's manifest.json, + # kept in the path flavor of the machine the bundle was created on. + # The checked flag makes the manifest lookup happen at most once per run; + # CORE.data is cleared between runs. + original_config_dir: PurePath | None = None + original_config_dir_checked: bool = False def _get_data() -> BundleData: @@ -148,6 +155,94 @@ def add_bundle_file(path: Path) -> None: _get_data().extra_files.append(CORE.relative_config_path(path)) +# Windows paths start with a drive letter or contain backslashes; POSIX +# paths do neither in practice, so this is how the flavor of a recorded +# path string is recognized on any host. +_WINDOWS_DRIVE_RE = re.compile(r"^[A-Za-z]:") + + +def _path_flavor(value: str) -> type[PurePath]: + """Pick the pure path class matching the flavor ``value`` was written in.""" + if "\\" in value or _WINDOWS_DRIVE_RE.match(value): + return PureWindowsPath + return PurePosixPath + + +def _load_original_config_dir() -> PurePath | None: + """Read the original config dir from an extracted bundle's manifest. + + Returns None when the current config dir is not an extracted bundle or + the manifest does not record the original config dir. + """ + manifest_path = CORE.config_dir / MANIFEST_FILENAME + try: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + except FileNotFoundError: + # The common case: this config dir is not an extracted bundle. + return None + except (OSError, UnicodeDecodeError, json.JSONDecodeError) as err: + # A manifest.json is present but unreadable or malformed. Say so + # instead of letting it look identical to "not a bundle". + _LOGGER.warning("Bundle: ignoring unreadable %s: %s", manifest_path, err) + return None + if not isinstance(manifest, dict): + return None + # A manifest.json in the config dir does not have to be ours. Only trust + # one that looks like a bundle manifest for exactly this config file. + version = manifest.get(ManifestKey.MANIFEST_VERSION) + if not isinstance(version, int) or version < 1: + return None + if manifest.get(ManifestKey.CONFIG_FILENAME) != CORE.config_path.name: + return None + config_dir = manifest.get(ManifestKey.CONFIG_DIR) + if not isinstance(config_dir, str) or not config_dir: + return None + return _path_flavor(config_dir)(config_dir) + + +def remap_bundle_path(value: str) -> Path | None: + """Remap an absolute path from the machine a bundle was created on. + + A bundled config may reference files by absolute path. The referenced + files ship inside the bundle at their config-relative locations, but the + YAML text is copied verbatim, so after extraction on another machine the + absolute reference points at a path that only existed on the creating + machine. The bundle manifest records that machine's config dir; when + ``value`` names a path that lived under it, return the corresponding + file next to the extracted config. + + ``value`` is the raw path string from the config. It is parsed with the + original machine's path flavor, so a bundle created on Windows remaps on + a POSIX build server and vice versa. + + Returns None when not compiling an extracted bundle, when ``value`` was + not under the original config dir, or when the bundle does not contain + the file. + """ + data = _get_data() + if not data.original_config_dir_checked: + data.original_config_dir_checked = True + data.original_config_dir = _load_original_config_dir() + original_dir = data.original_config_dir + if original_dir is None: + return None + path = type(original_dir)(value) + if not path.is_absolute(): + return None + try: + rel = path.relative_to(original_dir) + except ValueError: + return None + # relative_to is lexical, so ".." segments survive it. Refuse them: the + # remapped file must land strictly inside the extracted config tree. + if ".." in rel.parts: + return None + remapped = CORE.relative_config_path(Path(*rel.parts)) + if not remapped.exists(): + return None + return remapped + + @dataclass class BundleFile: """A file to include in the bundle.""" @@ -174,6 +269,7 @@ class BundleManifest: config_filename: str files: list[str] has_secrets: bool + config_dir: str | None = None class ConfigBundleCreator: @@ -438,6 +534,7 @@ class ConfigBundleCreator: ManifestKey.MANIFEST_VERSION: CURRENT_MANIFEST_VERSION, ManifestKey.ESPHOME_VERSION: const.__version__, ManifestKey.CONFIG_FILENAME: self._config_path.name, + ManifestKey.CONFIG_DIR: str(self._config_dir), ManifestKey.FILES: [f.path for f in files], ManifestKey.HAS_SECRETS: has_secrets, } @@ -522,12 +619,14 @@ def read_bundle_manifest(bundle_path: Path) -> BundleManifest: except tarfile.TarError as err: raise EsphomeError(f"Failed to read bundle: {err}") from err + config_dir = manifest.get(ManifestKey.CONFIG_DIR) return BundleManifest( manifest_version=manifest[ManifestKey.MANIFEST_VERSION], esphome_version=manifest.get(ManifestKey.ESPHOME_VERSION, "unknown"), config_filename=manifest[ManifestKey.CONFIG_FILENAME], files=manifest.get(ManifestKey.FILES, []), has_secrets=manifest.get(ManifestKey.HAS_SECRETS, False), + config_dir=config_dir if isinstance(config_dir, str) else None, ) diff --git a/esphome/config_validation.py b/esphome/config_validation.py index 3f7c8ff783..713df5452a 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -1938,14 +1938,29 @@ def dimensions(value): return dimensions([match.group(1), match.group(2)]) +def _remap_bundle_path(value: str) -> Path | None: + """Resolve a path from the machine an extracted bundle was created on. + + An absolute path in a config compiled from an extracted bundle may point + at the machine the bundle was created on; the bundle ships the file at + its config-relative location instead. + """ + from esphome.bundle import remap_bundle_path + + return remap_bundle_path(value) + + def directory(value: object) -> Path: value = string(value) path = CORE.relative_config_path(value) if not path.exists(): - raise Invalid( - f"Could not find directory '{path}'. Please make sure it exists (full path: {path.resolve()})." - ) + remapped = _remap_bundle_path(value) + if remapped is None: + raise Invalid( + f"Could not find directory '{path}'. Please make sure it exists (full path: {path.resolve()})." + ) + path = remapped if not path.is_dir(): raise Invalid( f"Path '{path}' is not a directory (full path: {path.resolve()})." @@ -1958,9 +1973,12 @@ def file_(value: object) -> Path: path = CORE.relative_config_path(value) if not path.exists(): - raise Invalid( - f"Could not find file '{path}'. Please make sure it exists (full path: {path.resolve()})." - ) + remapped = _remap_bundle_path(value) + if remapped is None: + raise Invalid( + f"Could not find file '{path}'. Please make sure it exists (full path: {path.resolve()})." + ) + path = remapped if not path.is_file(): raise Invalid(f"Path '{path}' is not a file (full path: {path.resolve()}).") return path diff --git a/tests/unit_tests/test_bundle.py b/tests/unit_tests/test_bundle.py index 6cecb63c2d..f0abcc74c6 100644 --- a/tests/unit_tests/test_bundle.py +++ b/tests/unit_tests/test_bundle.py @@ -27,6 +27,7 @@ from esphome.bundle import ( is_bundle_path, prepare_bundle_for_compile, read_bundle_manifest, + remap_bundle_path, ) from esphome.core import CORE, EsphomeError from esphome.yaml_util import force_load_include_files @@ -478,7 +479,10 @@ def test_read_bundle_manifest_corrupted_tar(tmp_path: Path) -> None: def test_read_bundle_manifest(tmp_path: Path) -> None: bundle_path = _make_bundle( tmp_path, - manifest_overrides={ManifestKey.HAS_SECRETS: True}, + manifest_overrides={ + ManifestKey.HAS_SECRETS: True, + ManifestKey.CONFIG_DIR: "/original/config", + }, extra_files={"secrets.yaml": b"wifi: test\n"}, ) @@ -489,6 +493,7 @@ def test_read_bundle_manifest(tmp_path: Path) -> None: assert manifest.esphome_version == "2026.2.0-test" assert manifest.config_filename == "test.yaml" assert manifest.has_secrets is True + assert manifest.config_dir == "/original/config" def test_read_bundle_manifest_minimal(tmp_path: Path) -> None: @@ -508,6 +513,266 @@ def test_read_bundle_manifest_minimal(tmp_path: Path) -> None: assert result.esphome_version == "unknown" assert not result.files assert result.has_secrets is False + assert result.config_dir is None + + +def test_read_bundle_manifest_non_string_config_dir(tmp_path: Path) -> None: + """A malformed config_dir value is dropped rather than propagated.""" + bundle_path = _make_bundle( + tmp_path, manifest_overrides={ManifestKey.CONFIG_DIR: 42} + ) + + assert read_bundle_manifest(bundle_path).config_dir is None + + +# --------------------------------------------------------------------------- +# remap_bundle_path +# --------------------------------------------------------------------------- + + +ORIGINAL_CONFIG_DIR = "/original/config" + + +def _bundle_manifest_dict(**overrides: Any) -> dict[str, Any]: + """Manifest content an extracted bundle would contain.""" + manifest: dict[str, Any] = { + ManifestKey.MANIFEST_VERSION: CURRENT_MANIFEST_VERSION, + ManifestKey.CONFIG_FILENAME: "test.yaml", + ManifestKey.CONFIG_DIR: ORIGINAL_CONFIG_DIR, + } + manifest.update(overrides) + return manifest + + +def _setup_extracted_dir( + tmp_path: Path, + manifest: dict[str, Any] | str | None, + files: dict[str, str] | None = None, +) -> Path: + """Create a directory shaped like an extracted bundle and point CORE at it.""" + extract_dir = _setup_config_dir(tmp_path, files) + if manifest is not None: + content = manifest if isinstance(manifest, str) else json.dumps(manifest) + (extract_dir / MANIFEST_FILENAME).write_text(content) + return extract_dir + + +def test_remap_bundle_path_success(tmp_path: Path) -> None: + """A stale absolute path resolves to the bundled copy next to the config.""" + extract_dir = _setup_extracted_dir( + tmp_path, _bundle_manifest_dict(), files={"boards/partitions.csv": "csv\n"} + ) + + remapped = remap_bundle_path(f"{ORIGINAL_CONFIG_DIR}/boards/partitions.csv") + + assert remapped == extract_dir / "boards" / "partitions.csv" + assert remapped.is_file() + + +@pytest.mark.parametrize( + "value", + [ + pytest.param(r"C:\Users\nick\esphome\boards\partitions.csv", id="backslashes"), + pytest.param("C:/Users/nick/esphome/boards/partitions.csv", id="forward"), + pytest.param(r"c:\users\NICK\esphome\boards\partitions.csv", id="case"), + ], +) +def test_remap_bundle_path_windows_bundle_on_posix(tmp_path: Path, value: str) -> None: + """A bundle created on Windows remaps on a build server with another layout.""" + extract_dir = _setup_extracted_dir( + tmp_path, + _bundle_manifest_dict(**{ManifestKey.CONFIG_DIR: r"C:\Users\nick\esphome"}), + files={"boards/partitions.csv": "csv\n"}, + ) + + remapped = remap_bundle_path(value) + + assert remapped == extract_dir / "boards" / "partitions.csv" + assert remapped.is_file() + + +def test_remap_bundle_path_windows_bundle_path_not_under_config_dir( + tmp_path: Path, +) -> None: + """A Windows path outside the original config dir is left alone.""" + _setup_extracted_dir( + tmp_path, + _bundle_manifest_dict(**{ManifestKey.CONFIG_DIR: r"C:\Users\nick\esphome"}), + files={"partitions.csv": "csv\n"}, + ) + + assert remap_bundle_path(r"D:\other\partitions.csv") is None + + +def test_remap_bundle_path_windows_profile_with_spaces(tmp_path: Path) -> None: + r"""A Windows profile like C:\Users\First Last remaps like any other dir.""" + extract_dir = _setup_extracted_dir( + tmp_path, + _bundle_manifest_dict( + **{ManifestKey.CONFIG_DIR: r"C:\Users\First Last\esphome"} + ), + files={"boards/my partitions.csv": "csv\n"}, + ) + + remapped = remap_bundle_path( + r"C:\Users\First Last\esphome\boards\my partitions.csv" + ) + + assert remapped == extract_dir / "boards" / "my partitions.csv" + assert remapped.is_file() + + +def test_remap_bundle_path_unc_config_dir(tmp_path: Path) -> None: + """A bundle created from a UNC share remaps like any other Windows path.""" + extract_dir = _setup_extracted_dir( + tmp_path, + _bundle_manifest_dict(**{ManifestKey.CONFIG_DIR: r"\\server\share\esphome"}), + files={"partitions.csv": "csv\n"}, + ) + + remapped = remap_bundle_path(r"\\server\share\esphome\partitions.csv") + + assert remapped == extract_dir / "partitions.csv" + + +def test_remap_bundle_path_flavor_mismatch(tmp_path: Path) -> None: + """A POSIX style value cannot come from a Windows config dir; no remap.""" + _setup_extracted_dir( + tmp_path, + _bundle_manifest_dict(**{ManifestKey.CONFIG_DIR: r"C:\Users\nick\esphome"}), + files={"partitions.csv": "csv\n"}, + ) + + assert remap_bundle_path("/original/config/partitions.csv") is None + + +def test_remap_bundle_path_rejects_traversal(tmp_path: Path) -> None: + """A remap may never escape the extracted config tree.""" + extract_dir = _setup_extracted_dir(tmp_path, _bundle_manifest_dict()) + (tmp_path / "outside.csv").write_text("csv\n") + assert (extract_dir / ".." / "outside.csv").resolve().is_file() + + assert remap_bundle_path(f"{ORIGINAL_CONFIG_DIR}/../outside.csv") is None + + +def test_remap_bundle_path_relative_value(tmp_path: Path) -> None: + """Relative references resolve normally and are never remapped.""" + _setup_extracted_dir(tmp_path, _bundle_manifest_dict()) + + assert remap_bundle_path("missing.csv") is None + + +def test_remap_bundle_path_no_manifest(tmp_path: Path) -> None: + """A config dir without a manifest is not an extracted bundle.""" + _setup_extracted_dir(tmp_path, None, files={"partitions.csv": "csv\n"}) + + assert remap_bundle_path(f"{ORIGINAL_CONFIG_DIR}/partitions.csv") is None + + +@pytest.mark.parametrize( + "manifest", + [ + pytest.param("{not json", id="malformed_json"), + pytest.param("[]", id="not_a_dict"), + pytest.param( + _bundle_manifest_dict(**{ManifestKey.MANIFEST_VERSION: "x"}), + id="version_not_int", + ), + pytest.param( + _bundle_manifest_dict(**{ManifestKey.MANIFEST_VERSION: 0}), + id="version_zero", + ), + pytest.param( + _bundle_manifest_dict(**{ManifestKey.CONFIG_FILENAME: "other.yaml"}), + id="config_filename_mismatch", + ), + pytest.param( + { + ManifestKey.MANIFEST_VERSION: CURRENT_MANIFEST_VERSION, + ManifestKey.CONFIG_FILENAME: "test.yaml", + }, + id="config_dir_missing", + ), + pytest.param( + _bundle_manifest_dict(**{ManifestKey.CONFIG_DIR: ""}), + id="config_dir_empty", + ), + ], +) +def test_remap_bundle_path_untrusted_manifest( + tmp_path: Path, manifest: dict[str, Any] | str +) -> None: + """Manifests that do not look like this bundle's manifest are ignored.""" + _setup_extracted_dir(tmp_path, manifest, files={"partitions.csv": "csv\n"}) + + assert remap_bundle_path(f"{ORIGINAL_CONFIG_DIR}/partitions.csv") is None + + +def test_remap_bundle_path_unreadable_manifest_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A present but broken manifest is reported, not silently ignored.""" + _setup_extracted_dir(tmp_path, "{not json", files={"partitions.csv": "csv\n"}) + + assert remap_bundle_path(f"{ORIGINAL_CONFIG_DIR}/partitions.csv") is None + assert "ignoring unreadable" in caplog.text + + +def test_remap_bundle_path_outside_original_config_dir(tmp_path: Path) -> None: + """Paths that were not under the original config dir are left alone.""" + _setup_extracted_dir(tmp_path, _bundle_manifest_dict()) + + assert remap_bundle_path("/elsewhere/partitions.csv") is None + + +def test_remap_bundle_path_bundled_copy_missing(tmp_path: Path) -> None: + """No remap when the bundle does not contain the file.""" + _setup_extracted_dir(tmp_path, _bundle_manifest_dict()) + + assert remap_bundle_path(f"{ORIGINAL_CONFIG_DIR}/partitions.csv") is None + + +def test_remap_bundle_path_manifest_read_once(tmp_path: Path) -> None: + """The manifest lookup result is cached for the rest of the run.""" + extract_dir = _setup_extracted_dir( + tmp_path, _bundle_manifest_dict(), files={"partitions.csv": "csv\n"} + ) + + first = remap_bundle_path(f"{ORIGINAL_CONFIG_DIR}/partitions.csv") + assert first == extract_dir / "partitions.csv" + + (extract_dir / MANIFEST_FILENAME).unlink() + second = remap_bundle_path(f"{ORIGINAL_CONFIG_DIR}/partitions.csv") + assert second == first + + +def test_remap_bundle_path_round_trip(tmp_path: Path) -> None: + """A file referenced by absolute path survives bundle create and extract. + + Reproduces https://github.com/esphome/esphome/issues/17755: the config + names its partitions csv by absolute path, the bundle is extracted on a + machine where that path does not exist, and the reference must resolve + to the bundled copy. + """ + config_dir = _setup_config_dir(tmp_path, files={"partitions.csv": "nvs,data\n"}) + abs_path = (config_dir / "partitions.csv").resolve() + + creator = ConfigBundleCreator({"esp32": {"partitions": abs_path}}) + result = creator.create_bundle() + + bundle_path = tmp_path / f"device{BUNDLE_EXTENSION}" + bundle_path.write_bytes(result.data) + target = tmp_path / "build_server" + config_path = extract_bundle(bundle_path, target) + + # Simulate the build server: fresh run, original config dir gone + CORE.reset() + CORE.config_path = config_path + shutil.rmtree(config_dir) + + remapped = remap_bundle_path(str(abs_path)) + assert remapped == target.resolve() / "partitions.csv" + assert remapped.is_file() # --------------------------------------------------------------------------- @@ -1261,7 +1526,7 @@ def test_create_bundle_produces_valid_archive(tmp_path: Path) -> None: def test_create_bundle_manifest_content(tmp_path: Path) -> None: - _setup_config_dir(tmp_path) + config_dir = _setup_config_dir(tmp_path) creator = ConfigBundleCreator({}) result = creator.create_bundle() @@ -1269,6 +1534,7 @@ def test_create_bundle_manifest_content(tmp_path: Path) -> None: manifest = result.manifest assert manifest[ManifestKey.MANIFEST_VERSION] == CURRENT_MANIFEST_VERSION assert manifest[ManifestKey.CONFIG_FILENAME] == "test.yaml" + assert manifest[ManifestKey.CONFIG_DIR] == str(config_dir.resolve()) assert "test.yaml" in manifest[ManifestKey.FILES] diff --git a/tests/unit_tests/test_config_validation.py b/tests/unit_tests/test_config_validation.py index fd21ac92ea..1da3d5593a 100644 --- a/tests/unit_tests/test_config_validation.py +++ b/tests/unit_tests/test_config_validation.py @@ -1,3 +1,4 @@ +import json from pathlib import Path import string @@ -2912,3 +2913,79 @@ def test_rename_key_present() -> None: def test_rename_key_absent() -> None: assert cv.rename_key("old", "new")({"other": 5}) == {"other": 5} + + +def test_file__existing_relative_path(setup_core: Path) -> None: + (setup_core / "partitions.csv").write_text("csv\n") + + assert cv.file_("partitions.csv") == setup_core / "partitions.csv" + + +def test_file__missing_raises(setup_core: Path) -> None: + with pytest.raises(Invalid, match="Could not find file"): + cv.file_("partitions.csv") + + +def test_file__remaps_bundle_absolute_path(setup_core: Path) -> None: + """A stale absolute path in an extracted bundle resolves to the bundled copy.""" + manifest = { + "manifest_version": 1, + "config_filename": "test.yaml", + "config_dir": "/original/config", + } + (setup_core / "manifest.json").write_text(json.dumps(manifest)) + (setup_core / "partitions.csv").write_text("csv\n") + + assert cv.file_("/original/config/partitions.csv") == setup_core / "partitions.csv" + + +def test_file__missing_absolute_path_without_bundle(setup_core: Path) -> None: + with pytest.raises(Invalid, match="Could not find file"): + cv.file_("/original/config/partitions.csv") + + +def test_file__remaps_windows_bundle_absolute_path(setup_core: Path) -> None: + """A bundle created on Windows resolves on a host with another layout.""" + manifest = { + "manifest_version": 1, + "config_filename": "test.yaml", + "config_dir": "C:\\Users\\nick\\esphome", + } + (setup_core / "manifest.json").write_text(json.dumps(manifest)) + (setup_core / "partitions.csv").write_text("csv\n") + + result = cv.file_("C:\\Users\\nick\\esphome\\partitions.csv") + + assert result == setup_core / "partitions.csv" + + +def test_directory_remaps_bundle_absolute_path(setup_core: Path) -> None: + """A stale absolute directory in an extracted bundle resolves to the bundled copy.""" + manifest = { + "manifest_version": 1, + "config_filename": "test.yaml", + "config_dir": "/original/config", + } + (setup_core / "manifest.json").write_text(json.dumps(manifest)) + (setup_core / "headers").mkdir() + + assert cv.directory("/original/config/headers") == setup_core / "headers" + + +def test_directory_missing_raises(setup_core: Path) -> None: + with pytest.raises(Invalid, match="Could not find directory"): + cv.directory("/original/config/headers") + + +def test_file__remapped_path_is_directory_raises(setup_core: Path) -> None: + """A remapped path that is a directory still fails file validation.""" + manifest = { + "manifest_version": 1, + "config_filename": "test.yaml", + "config_dir": "/original/config", + } + (setup_core / "manifest.json").write_text(json.dumps(manifest)) + (setup_core / "headers").mkdir() + + with pytest.raises(Invalid, match="is not a file"): + cv.file_("/original/config/headers")