From 43448d55f10734af714685038e049f9bc30ce0c3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:23:24 -0600 Subject: [PATCH] Guard against None firmware_bin_path and mock subprocess in tests - Add None check for storage_json.firmware_bin_path before computing base_dir (covers configs from StorageJSON.from_wizard()). - Mock async_run_system_command in path traversal tests so paths that pass validation but don't exist return 404 deterministically. - Add test for firmware_bin_path=None case. --- esphome/dashboard/web_server.py | 4 ++++ tests/dashboard/test_web_server.py | 36 ++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/esphome/dashboard/web_server.py b/esphome/dashboard/web_server.py index 24a0bacf05b..a12b2fd5791 100644 --- a/esphome/dashboard/web_server.py +++ b/esphome/dashboard/web_server.py @@ -1063,6 +1063,10 @@ class DownloadBinaryRequestHandler(BaseHandler): f"{storage_json.name}-{file_name}", ) + if storage_json.firmware_bin_path is None: + self.send_error(404) + return + base_dir = storage_json.firmware_bin_path.parent.resolve() path = base_dir.joinpath(file_name).resolve() try: diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index d216652162e..19e19c7d8db 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -573,8 +573,16 @@ async def test_download_binary_handler_path_traversal_protection( mock_storage.firmware_bin_path = firmware_file mock_storage_json.load.return_value = mock_storage - # Attempt path traversal attack - should be blocked - with pytest.raises(HTTPClientError) as exc_info: + # Mock async_run_system_command so paths that pass validation but don't exist + # return 404 deterministically without spawning a real subprocess. + with ( + patch( + "esphome.dashboard.web_server.async_run_system_command", + new_callable=AsyncMock, + return_value=(2, "", ""), + ), + pytest.raises(HTTPClientError) as exc_info, + ): await dashboard.fetch( f"/download.bin?configuration=test.yaml&file={attack_path}", method="GET", @@ -582,6 +590,30 @@ async def test_download_binary_handler_path_traversal_protection( assert exc_info.value.code == expected_code +@pytest.mark.asyncio +@pytest.mark.usefixtures("mock_ext_storage_path") +async def test_download_binary_handler_no_firmware_bin_path( + dashboard: DashboardTestHelper, + mock_storage_json: MagicMock, +) -> None: + """Test that download returns 404 when firmware_bin_path is None. + + This covers configs created by StorageJSON.from_wizard() where no + firmware has been compiled yet. + """ + mock_storage = Mock() + mock_storage.name = "test_device" + mock_storage.firmware_bin_path = None + mock_storage_json.load.return_value = mock_storage + + with pytest.raises(HTTPClientError) as exc_info: + await dashboard.fetch( + "/download.bin?configuration=test.yaml&file=firmware.bin", + method="GET", + ) + assert exc_info.value.code == 404 + + @pytest.mark.asyncio @pytest.mark.usefixtures("mock_ext_storage_path") async def test_download_binary_handler_multiple_subdirectory_levels(