From becec5b02b89f66a16ed040397b4e78ef9ec4f98 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sun, 4 Oct 2026 00:38:13 +0200 Subject: [PATCH] fix: validate tool download destination before network Signed-off-by: Lukas Geiger --- tests/test_tool_manager.py | 47 +++++++++++++++++++++++++++++++++++++- tool_manager.py | 18 ++++++++++++++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/test_tool_manager.py b/tests/test_tool_manager.py index 05b1332..354ad6e 100644 --- a/tests/test_tool_manager.py +++ b/tests/test_tool_manager.py @@ -333,13 +333,58 @@ def test_download_tool_returns_error_when_destination_is_a_file(tm, tmp_path, mo download_root = tmp_path / "downloads" download_root.mkdir() - (download_root / "universal_mail_cleaner").write_text("blocked", encoding="utf-8") + sentinel = download_root / "universal_mail_cleaner" + sentinel.write_text("blocked", encoding="utf-8") monkeypatch.setattr(tool_manager, "_DOWNLOAD_DIR", download_root) + network_calls = [] + + def fail_fast_urlopen(*args, **kwargs): + network_calls.append((args, kwargs)) + raise AssertionError("unexpected network request for an invalid destination") + + monkeypatch.setattr(tool_manager.urllib.request, "urlopen", fail_fast_urlopen) + err = tm.download_tool("universal_mail_cleaner") + + assert err is not None + assert err.startswith("Download error: cannot prepare destination:") + assert network_calls == [] + assert sentinel.read_text(encoding="utf-8") == "blocked" + + +def test_download_tool_returns_error_when_destination_stat_fails(tm, tmp_path, monkeypatch): + import tool_manager + + download_root = tmp_path / "downloads" + download_root.mkdir() + dest_dir = download_root / "universal_mail_cleaner" + dest_dir.mkdir() + sentinel = dest_dir / "keep.txt" + sentinel.write_text("keep", encoding="utf-8") + monkeypatch.setattr(tool_manager, "_DOWNLOAD_DIR", download_root) + + original_stat = Path.stat + + def fail_destination_stat(path, *args, **kwargs): + if path == dest_dir: + raise PermissionError("synthetic destination stat failure") + return original_stat(path, *args, **kwargs) + + monkeypatch.setattr(Path, "stat", fail_destination_stat) + network_calls = [] + + def fail_fast_urlopen(*args, **kwargs): + network_calls.append((args, kwargs)) + raise AssertionError("unexpected network request after a destination stat failure") + + monkeypatch.setattr(tool_manager.urllib.request, "urlopen", fail_fast_urlopen) err = tm.download_tool("universal_mail_cleaner") assert err is not None assert err.startswith("Download error: cannot prepare destination:") + assert "synthetic destination stat failure" in err + assert network_calls == [] + assert sentinel.read_text(encoding="utf-8") == "keep" def test_download_tool_extracts_flat_archive(tm, tmp_path, monkeypatch): diff --git a/tool_manager.py b/tool_manager.py index 2b8fe9d..f57a8f6 100644 --- a/tool_manager.py +++ b/tool_manager.py @@ -4,6 +4,7 @@ import os import re import shutil +import stat import subprocess import sys import urllib.request @@ -302,6 +303,22 @@ def download_tool(self, tool_id: str, return "No GitHub repo configured for this tool" repo = meta["github_repo"] + # Validate the local destination before making any network request. + dest_dir = _DOWNLOAD_DIR / tool_id + for candidate in (dest_dir, *dest_dir.parents): + try: + destination_stat = candidate.stat() + except FileNotFoundError: + continue + except OSError as exc: + return f"Download error: cannot prepare destination: {exc}" + if not stat.S_ISDIR(destination_stat.st_mode): + return ( + "Download error: cannot prepare destination: " + f"not a directory: {candidate}" + ) + break + # Fetch release metadata api_url = f"https://api.github.com/repos/{repo}/releases/latest" try: @@ -320,7 +337,6 @@ def download_tool(self, tool_id: str, return "No zipball_url in release info" # Prepare download directory - dest_dir = _DOWNLOAD_DIR / tool_id try: dest_dir.mkdir(parents=True, exist_ok=True) except OSError as exc: