mirror of
				https://github.com/esphome/esphome.git
				synced 2025-10-31 15:12:06 +00:00 
			
		
		
		
	[dashboard] Fix binary download with packages using secrets after Path migration (#11313)
This commit is contained in:
		
				
					committed by
					
						 Jesse Hills
						Jesse Hills
					
				
			
			
				
	
			
			
			
						parent
						
							ea609dc0f6
						
					
				
				
					commit
					8c1bd2fd85
				
			| @@ -10,6 +10,10 @@ from esphome.helpers import get_bool_env | |||||||
|  |  | ||||||
| from .util.password import password_hash | from .util.password import password_hash | ||||||
|  |  | ||||||
|  | # Sentinel file name used for CORE.config_path when dashboard initializes. | ||||||
|  | # This ensures .parent returns the config directory instead of root. | ||||||
|  | _DASHBOARD_SENTINEL_FILE = "___DASHBOARD_SENTINEL___.yaml" | ||||||
|  |  | ||||||
|  |  | ||||||
| class DashboardSettings: | class DashboardSettings: | ||||||
|     """Settings for the dashboard.""" |     """Settings for the dashboard.""" | ||||||
| @@ -48,7 +52,12 @@ class DashboardSettings: | |||||||
|         self.config_dir = Path(args.configuration) |         self.config_dir = Path(args.configuration) | ||||||
|         self.absolute_config_dir = self.config_dir.resolve() |         self.absolute_config_dir = self.config_dir.resolve() | ||||||
|         self.verbose = args.verbose |         self.verbose = args.verbose | ||||||
|         CORE.config_path = self.config_dir / "." |         # Set to a sentinel file so .parent gives us the config directory. | ||||||
|  |         # Previously this was `os.path.join(self.config_dir, ".")` which worked because | ||||||
|  |         # os.path.dirname("/config/.") returns "/config", but Path("/config/.").parent | ||||||
|  |         # normalizes to Path("/config") first, then .parent returns Path("/"), breaking | ||||||
|  |         # secret resolution. Using a sentinel file ensures .parent gives the correct directory. | ||||||
|  |         CORE.config_path = self.config_dir / _DASHBOARD_SENTINEL_FILE | ||||||
|  |  | ||||||
|     @property |     @property | ||||||
|     def relative_url(self) -> str: |     def relative_url(self) -> str: | ||||||
|   | |||||||
| @@ -2,11 +2,13 @@ | |||||||
|  |  | ||||||
| from __future__ import annotations | from __future__ import annotations | ||||||
|  |  | ||||||
|  | from argparse import Namespace | ||||||
| from pathlib import Path | from pathlib import Path | ||||||
| import tempfile | import tempfile | ||||||
|  |  | ||||||
| import pytest | import pytest | ||||||
|  |  | ||||||
|  | from esphome.core import CORE | ||||||
| from esphome.dashboard.settings import DashboardSettings | from esphome.dashboard.settings import DashboardSettings | ||||||
|  |  | ||||||
|  |  | ||||||
| @@ -159,3 +161,63 @@ def test_rel_path_with_numeric_args(dashboard_settings: DashboardSettings) -> No | |||||||
|     result = dashboard_settings.rel_path("123", "456.789") |     result = dashboard_settings.rel_path("123", "456.789") | ||||||
|     expected = dashboard_settings.config_dir / "123" / "456.789" |     expected = dashboard_settings.config_dir / "123" / "456.789" | ||||||
|     assert result == expected |     assert result == expected | ||||||
|  |  | ||||||
|  |  | ||||||
|  | def test_config_path_parent_resolves_to_config_dir(tmp_path: Path) -> None: | ||||||
|  |     """Test that CORE.config_path.parent resolves to config_dir after parse_args. | ||||||
|  |  | ||||||
|  |     This is a regression test for issue #11280 where binary download failed | ||||||
|  |     when using packages with secrets after the Path migration in 2025.10.0. | ||||||
|  |  | ||||||
|  |     The issue was that after switching from os.path to Path: | ||||||
|  |     - Before: os.path.dirname("/config/.") → "/config" | ||||||
|  |     - After: Path("/config/.").parent → Path("/") (normalized first!) | ||||||
|  |  | ||||||
|  |     The fix uses a sentinel file so .parent returns the correct directory: | ||||||
|  |     - Fixed: Path("/config/___DASHBOARD_SENTINEL___.yaml").parent → Path("/config") | ||||||
|  |     """ | ||||||
|  |     # Create test directory structure with secrets and packages | ||||||
|  |     config_dir = tmp_path / "config" | ||||||
|  |     config_dir.mkdir() | ||||||
|  |  | ||||||
|  |     # Create secrets.yaml with obviously fake test values | ||||||
|  |     secrets_file = config_dir / "secrets.yaml" | ||||||
|  |     secrets_file.write_text( | ||||||
|  |         "wifi_ssid: TEST-DUMMY-SSID\n" | ||||||
|  |         "wifi_password: not-a-real-password-just-for-testing\n" | ||||||
|  |     ) | ||||||
|  |  | ||||||
|  |     # Create package file that uses secrets | ||||||
|  |     package_file = config_dir / "common.yaml" | ||||||
|  |     package_file.write_text( | ||||||
|  |         "wifi:\n  ssid: !secret wifi_ssid\n  password: !secret wifi_password\n" | ||||||
|  |     ) | ||||||
|  |  | ||||||
|  |     # Create main device config that includes the package | ||||||
|  |     device_config = config_dir / "test-device.yaml" | ||||||
|  |     device_config.write_text( | ||||||
|  |         "esphome:\n  name: test-device\n\npackages:\n  common: !include common.yaml\n" | ||||||
|  |     ) | ||||||
|  |  | ||||||
|  |     # Set up dashboard settings with our test config directory | ||||||
|  |     settings = DashboardSettings() | ||||||
|  |     args = Namespace( | ||||||
|  |         configuration=str(config_dir), | ||||||
|  |         password=None, | ||||||
|  |         username=None, | ||||||
|  |         ha_addon=False, | ||||||
|  |         verbose=False, | ||||||
|  |     ) | ||||||
|  |     settings.parse_args(args) | ||||||
|  |  | ||||||
|  |     # Verify that CORE.config_path.parent correctly points to the config directory | ||||||
|  |     # This is critical for secret resolution in yaml_util.py which does: | ||||||
|  |     #   main_config_dir = CORE.config_path.parent | ||||||
|  |     #   main_secret_yml = main_config_dir / "secrets.yaml" | ||||||
|  |     assert CORE.config_path.parent == config_dir.resolve() | ||||||
|  |     assert (CORE.config_path.parent / "secrets.yaml").exists() | ||||||
|  |     assert (CORE.config_path.parent / "common.yaml").exists() | ||||||
|  |  | ||||||
|  |     # Verify that CORE.config_path itself uses the sentinel file | ||||||
|  |     assert CORE.config_path.name == "___DASHBOARD_SENTINEL___.yaml" | ||||||
|  |     assert not CORE.config_path.exists()  # Sentinel file doesn't actually exist | ||||||
|   | |||||||
| @@ -1,5 +1,6 @@ | |||||||
| from __future__ import annotations | from __future__ import annotations | ||||||
|  |  | ||||||
|  | from argparse import Namespace | ||||||
| import asyncio | import asyncio | ||||||
| from collections.abc import Generator | from collections.abc import Generator | ||||||
| from contextlib import asynccontextmanager | from contextlib import asynccontextmanager | ||||||
| @@ -17,6 +18,8 @@ from tornado.ioloop import IOLoop | |||||||
| from tornado.testing import bind_unused_port | from tornado.testing import bind_unused_port | ||||||
| from tornado.websocket import WebSocketClientConnection, websocket_connect | from tornado.websocket import WebSocketClientConnection, websocket_connect | ||||||
|  |  | ||||||
|  | from esphome import yaml_util | ||||||
|  | from esphome.core import CORE | ||||||
| from esphome.dashboard import web_server | from esphome.dashboard import web_server | ||||||
| from esphome.dashboard.const import DashboardEvent | from esphome.dashboard.const import DashboardEvent | ||||||
| from esphome.dashboard.core import DASHBOARD | from esphome.dashboard.core import DASHBOARD | ||||||
| @@ -1302,3 +1305,71 @@ async def test_dashboard_subscriber_refresh_event( | |||||||
|  |  | ||||||
|         # Give it a moment to clean up |         # Give it a moment to clean up | ||||||
|         await asyncio.sleep(0.01) |         await asyncio.sleep(0.01) | ||||||
|  |  | ||||||
|  |  | ||||||
|  | @pytest.mark.asyncio | ||||||
|  | async def test_dashboard_yaml_loading_with_packages_and_secrets( | ||||||
|  |     tmp_path: Path, | ||||||
|  | ) -> None: | ||||||
|  |     """Test dashboard YAML loading with packages referencing secrets. | ||||||
|  |  | ||||||
|  |     This is a regression test for issue #11280 where binary download failed | ||||||
|  |     when using packages with secrets after the Path migration in 2025.10.0. | ||||||
|  |  | ||||||
|  |     This test verifies that CORE.config_path initialization in the dashboard | ||||||
|  |     allows yaml_util.load_yaml() to correctly resolve secrets from packages. | ||||||
|  |     """ | ||||||
|  |     # Create test directory structure with secrets and packages | ||||||
|  |     config_dir = tmp_path / "config" | ||||||
|  |     config_dir.mkdir() | ||||||
|  |  | ||||||
|  |     # Create secrets.yaml with obviously fake test values | ||||||
|  |     secrets_file = config_dir / "secrets.yaml" | ||||||
|  |     secrets_file.write_text( | ||||||
|  |         "wifi_ssid: TEST-DUMMY-SSID\n" | ||||||
|  |         "wifi_password: not-a-real-password-just-for-testing\n" | ||||||
|  |     ) | ||||||
|  |  | ||||||
|  |     # Create package file that uses secrets | ||||||
|  |     package_file = config_dir / "common.yaml" | ||||||
|  |     package_file.write_text( | ||||||
|  |         "wifi:\n  ssid: !secret wifi_ssid\n  password: !secret wifi_password\n" | ||||||
|  |     ) | ||||||
|  |  | ||||||
|  |     # Create main device config that includes the package | ||||||
|  |     device_config = config_dir / "test-download-secrets.yaml" | ||||||
|  |     device_config.write_text( | ||||||
|  |         "esphome:\n  name: test-download-secrets\n  platform: ESP32\n  board: esp32dev\n\n" | ||||||
|  |         "packages:\n  common: !include common.yaml\n" | ||||||
|  |     ) | ||||||
|  |  | ||||||
|  |     # Initialize DASHBOARD settings with our test config directory | ||||||
|  |     # This is what sets CORE.config_path - the critical code path for the bug | ||||||
|  |     args = Namespace( | ||||||
|  |         configuration=str(config_dir), | ||||||
|  |         password=None, | ||||||
|  |         username=None, | ||||||
|  |         ha_addon=False, | ||||||
|  |         verbose=False, | ||||||
|  |     ) | ||||||
|  |     DASHBOARD.settings.parse_args(args) | ||||||
|  |  | ||||||
|  |     # With the fix: CORE.config_path should be config_dir / "___DASHBOARD_SENTINEL___.yaml" | ||||||
|  |     # so CORE.config_path.parent would be config_dir | ||||||
|  |     # Without the fix: CORE.config_path is config_dir / "." which normalizes to config_dir | ||||||
|  |     # so CORE.config_path.parent would be tmp_path (the parent of config_dir) | ||||||
|  |  | ||||||
|  |     # The fix ensures CORE.config_path.parent points to config_dir | ||||||
|  |     assert CORE.config_path.parent == config_dir.resolve(), ( | ||||||
|  |         f"CORE.config_path.parent should point to config_dir. " | ||||||
|  |         f"Got {CORE.config_path.parent}, expected {config_dir.resolve()}. " | ||||||
|  |         f"CORE.config_path is {CORE.config_path}" | ||||||
|  |     ) | ||||||
|  |  | ||||||
|  |     # Now load the YAML with packages that reference secrets | ||||||
|  |     # This is where the bug would manifest - yaml_util.load_yaml would fail | ||||||
|  |     # to find secrets.yaml because CORE.config_path.parent pointed to the wrong place | ||||||
|  |     config = yaml_util.load_yaml(device_config) | ||||||
|  |     # If we get here, secret resolution worked! | ||||||
|  |     assert "esphome" in config | ||||||
|  |     assert config["esphome"]["name"] == "test-download-secrets" | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user