[packages] Fix premature substitution of vars in remote package files (#15997)

Co-authored-by: J. Nick Koston <nick+github@koston.org>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

esphome/components/packages/__init__.py

@@ -378,9 +378,8 @@ def _substitute_package_definition(
     Local package contents are left untouched — they will be substituted
     later during the main substitution pass.
     """
-    if isinstance(package_config, str) or (
-        isinstance(package_config, dict) and is_remote_package(package_config)
-    ):
+
+    def do_substitute(package_config: dict | str) -> dict | str:
         # Collect undefined-variable errors (rather than raising strict) so the
         # path walked through a remote-package dict is preserved and the user
         # sees which field (url / path / ref / ...) referenced the undefined
@@ -394,6 +393,22 @@ def _substitute_package_definition(
             errors=errors,
         )
         raise_first_undefined(errors, "package definition")
+        return package_config
+
+    if isinstance(package_config, str):
+        return do_substitute(package_config)
+
+    if isinstance(package_config, dict) and is_remote_package(package_config):
+        # Mark vars as literal to avoid substituting variables in the vars block itself, since they are meant to be
+        # passed as-is to the package YAML and may contain their own substitution expressions that should not
+        # be prematurely evaluated here.
+        if CONF_FILES in package_config:
+            for file_def in package_config[CONF_FILES]:
+                if isinstance(file_def, dict) and CONF_VARS in file_def:
+                    file_def[CONF_VARS] = yaml_util.make_literal(file_def[CONF_VARS])
+
+        package_config = do_substitute(package_config)
+
     return package_config
 
 

esphome/yaml_util.py

@@ -113,6 +113,15 @@ def make_data_base(
         return value
 
 
+def make_literal(value: Any) -> ESPLiteralValue | Any:
+    """Wrap a value in an ESPLiteralValue object."""
+    try:
+        return add_class_to_obj(value, ESPLiteralValue)
+    except TypeError:
+        # Adding class failed, ignore error
+        return value
+
+
 def add_context(value: Any, context_vars: dict[str, Any] | None) -> Any:
     """Tags a list/string/dict value with context vars that must be applied to it and its children
     during the substitution pass. If no vars are given, no tagging is done.
@@ -525,7 +534,7 @@ class ESPHomeLoaderMixin:
             obj = self.construct_sequence(node)
         elif isinstance(node, yaml.MappingNode):
             obj = self.construct_mapping(node)
-        return add_class_to_obj(obj, ESPLiteralValue)
+        return make_literal(obj)
 
     @_add_data_ref
     def construct_extend(self, node: yaml.Node) -> Extend:

tests/component_tests/packages/test_packages.py

@@ -1491,3 +1491,133 @@ def test_substitute_package_definition_includes_source_location(tmp_path: Path)
     line, col = int(match.group(1)), int(match.group(2))
     assert line == 2, f"expected 1-based line 2, got {line} (err={err!r})"
     assert col >= 1, f"expected 1-based column ≥ 1, got {col} (err={err!r})"
+
+
+def test_substitute_package_definition_vars_preserved_literally() -> None:
+    """``vars:`` blocks in remote-package files are not substituted prematurely.
+
+    Variable references inside ``vars:`` may resolve to substitutions
+    contributed by sibling packages that have not yet been loaded, so they
+    must be passed through untouched and resolved later by the package YAML.
+    """
+    pkg = {
+        CONF_URL: "https://github.com/esphome/non-existant-repo",
+        CONF_REF: "main",
+        CONF_FILES: [
+            {
+                CONF_PATH: "common/somefile.yaml",
+                CONF_VARS: {"pin": "${PIN}"},
+            },
+        ],
+    }
+    # Note: PIN is intentionally NOT in the context — it is meant to
+    # be resolved later, when the package YAML is processed.
+    result = _substitute_package_definition(pkg, ContextVars())
+
+    assert result[CONF_FILES][0][CONF_VARS] == {"pin": "${PIN}"}
+
+
+def test_substitute_package_definition_other_fields_still_substituted() -> None:
+    """Marking ``vars:`` literal does not stop substitution of url/ref/path."""
+    ctx = ContextVars({"branch": "release", "org": "esphome"})
+    pkg = {
+        CONF_URL: "https://github.com/${org}/firmware",
+        CONF_REF: "${branch}",
+        CONF_FILES: [
+            {
+                CONF_PATH: "common/sensor.yaml",
+                CONF_VARS: {"pin": "${PIN}"},
+            },
+        ],
+    }
+    result = _substitute_package_definition(pkg, ctx)
+
+    assert result[CONF_URL] == "https://github.com/esphome/firmware"
+    assert result[CONF_REF] == "release"
+    # vars passed through unchanged
+    assert result[CONF_FILES][0][CONF_VARS] == {"pin": "${PIN}"}
+
+
+def test_substitute_package_definition_without_vars_unaffected() -> None:
+    """Files entries without a ``vars:`` block continue to work."""
+    ctx = ContextVars({"branch": "main"})
+    pkg = {
+        CONF_URL: "https://github.com/esphome/firmware",
+        CONF_REF: "${branch}",
+        CONF_FILES: [
+            {CONF_PATH: "file1.yaml"},
+            "file2.yaml",
+        ],
+    }
+    result = _substitute_package_definition(pkg, ctx)
+
+    assert result[CONF_REF] == "main"
+    assert result[CONF_FILES][0] == {CONF_PATH: "file1.yaml"}
+    assert result[CONF_FILES][1] == "file2.yaml"
+
+
+@patch("esphome.yaml_util.load_yaml")
+@patch("pathlib.Path.is_file")
+@patch("esphome.git.clone_or_update")
+def test_remote_package_vars_resolved_against_sibling_package_substitutions(
+    mock_clone_or_update, mock_is_file, mock_load_yaml
+) -> None:
+    """A ``vars:`` reference in one remote package can resolve to a
+    substitution defined in a sibling remote package.
+
+    A higher-priority package declares ``substitutions:`` (e.g. ``SENSOR_PIN: 5``) and a
+    lower-priority package's ``files: -> vars:`` references that substitution.
+    Because packages are processed highest-priority first and ``vars:`` is now
+    preserved literally during package-definition processing, the substitution
+    is resolved correctly when the package YAML itself is loaded.
+    """
+    mock_clone_or_update.return_value = (Path("/tmp/noexists"), MagicMock())
+    mock_is_file.return_value = True
+
+    # Two YAML files mocked from the "remote" repo:
+    #   - platform.yaml exports a substitution ``SENSOR_PIN``
+    #   - sensor.yaml uses ``${pin}`` (which is bound from ``vars:`` to
+    #     ``${SENSOR_PIN}`` and resolved against the merged substitutions).
+    mock_load_yaml.side_effect = [
+        # Order matches reverse-priority traversal (highest priority first).
+        OrderedDict(
+            {
+                CONF_SUBSTITUTIONS: {"SENSOR_PIN": "GPIO5"},
+            }
+        ),
+        OrderedDict(
+            {
+                CONF_SENSOR: [
+                    {
+                        CONF_PLATFORM: TEST_SENSOR_PLATFORM_1,
+                        CONF_NAME: TEST_SENSOR_NAME_1,
+                        "pin": "${pin}",
+                    }
+                ],
+            }
+        ),
+    ]
+
+    config = {
+        CONF_PACKAGES: {
+            "special_sensor": {
+                CONF_URL: "https://github.com/esphome/non-existant-repo",
+                CONF_FILES: [
+                    {
+                        CONF_PATH: "sensor.yaml",
+                        CONF_VARS: {"pin": "${SENSOR_PIN}"},
+                    },
+                ],
+                CONF_REFRESH: "1d",
+            },
+            "platform": {
+                CONF_URL: "https://github.com/esphome/non-existant-repo",
+                CONF_FILES: ["platform.yaml"],
+                CONF_REFRESH: "1d",
+            },
+        }
+    }
+
+    actual = packages_pass(config)
+
+    assert actual[CONF_SENSOR][0]["pin"] == "GPIO5"

tests/unit_tests/test_yaml_util.py

@@ -11,7 +11,13 @@ from esphome.config_helpers import Extend, Remove
 import esphome.config_validation as cv
 from esphome.core import DocumentLocation, DocumentRange, EsphomeError
 from esphome.util import OrderedDict
-from esphome.yaml_util import ESPHomeDataBase, format_path, make_data_base
+from esphome.yaml_util import (
+    ESPHomeDataBase,
+    ESPLiteralValue,
+    format_path,
+    make_data_base,
+    make_literal,
+)
 
 
 @pytest.fixture(autouse=True)
@@ -891,3 +897,57 @@ def test_format_path_empty_path_with_located_current_obj():
     obj = _located("${var}", "main.yaml", 0, 0)
     result = format_path([], obj)
     assert result == "In:  in main.yaml 1:1"
+
+
+def test_make_literal_wraps_dict() -> None:
+    """A dict is wrapped so it becomes an ESPLiteralValue instance."""
+    value = {"key": "${var}"}
+    result = make_literal(value)
+    assert isinstance(result, ESPLiteralValue)
+    assert isinstance(result, dict)
+    assert result == {"key": "${var}"}
+
+
+def test_make_literal_wraps_list() -> None:
+    """A list is wrapped so it becomes an ESPLiteralValue instance."""
+    value = ["${var}", "plain"]
+    result = make_literal(value)
+    assert isinstance(result, ESPLiteralValue)
+    assert isinstance(result, list)
+    assert result == ["${var}", "plain"]
+
+
+def test_make_literal_wraps_string() -> None:
+    """A string is wrapped so it becomes an ESPLiteralValue instance."""
+    result = make_literal("${var}")
+    assert isinstance(result, ESPLiteralValue)
+    assert result == "${var}"
+
+
+def test_make_literal_returns_already_wrapped_value_unchanged() -> None:
+    """Wrapping a value that is already an ESPLiteralValue returns it as-is."""
+    value = make_literal({"key": "value"})
+    assert isinstance(value, ESPLiteralValue)
+    result = make_literal(value)
+    assert result is value
+
+
+def test_make_literal_returns_none_unchanged() -> None:
+    """Values whose class cannot be augmented (e.g. ``None``) are returned as-is."""
+    result = make_literal(None)
+    assert result is None
+
+
+def test_make_literal_blocks_substitution() -> None:
+    """A value wrapped with make_literal is skipped by the substitution pass."""
+    value = make_literal({"pin": "${PIN}"})
+    result = substitutions.substitute(
+        value,
+        path=[],
+        parent_context=substitutions.ContextVars(),
+        strict_undefined=False,
+    )
+    # The literal block must remain untouched, even though the variable is
+    # undefined in the context.
+    assert result == {"pin": "${PIN}"}
+    assert isinstance(result, ESPLiteralValue)