From ea80c2224c0cb0e00433cf623d050e28026c47bd Mon Sep 17 00:00:00 2001 From: Will Miao Date: Mon, 20 Jul 2026 21:08:18 +0800 Subject: [PATCH] fix(download): prevent path traversal in download template resolution (#1028) --- py/services/download_manager.py | 22 +++++++ py/utils/utils.py | 6 ++ tests/services/test_download_manager_basic.py | 59 +++++++++++++++++++ tests/utils/test_utils.py | 32 ++++++++++ 4 files changed, 119 insertions(+) diff --git a/py/services/download_manager.py b/py/services/download_manager.py index da5c20e3..85ddfdcb 100644 --- a/py/services/download_manager.py +++ b/py/services/download_manager.py @@ -1389,7 +1389,17 @@ class DownloadManager: # Update save directory with relative path if provided if relative_path: + base_save_dir = save_dir save_dir = os.path.join(save_dir, relative_path) + # Security: validate path containment after joining + resolved_dir = os.path.realpath(os.path.normpath(save_dir)) + base_dir = os.path.realpath(os.path.normpath(base_save_dir)) + if not resolved_dir.startswith(base_dir + os.sep) and resolved_dir != base_dir: + logger.warning( + "Path traversal detected: %s escapes %s", + resolved_dir, base_dir, + ) + return {"success": False, "error": "Download path is outside allowed directory"} # Create directory if it doesn't exist os.makedirs(save_dir, exist_ok=True) @@ -1827,6 +1837,9 @@ class DownloadManager: model_tags, model_type ) + if not first_tag: + first_tag = "no tags" # Default if no tags available + # Format the template with available data formatted_path = path_template formatted_path = formatted_path.replace("{base_model}", mapped_base_model) @@ -1842,6 +1855,15 @@ class DownloadManager: if model_type == "embedding": formatted_path = formatted_path.replace(" ", "_") + # Sanitize the resolved path to prevent path traversal: + # - Strip leading slashes (prevents os.path.join from treating path as absolute) + # - Collapse double slashes from empty placeholder substitutions + # - Strip trailing slashes for cleanliness + formatted_path = formatted_path.lstrip("/") + while "//" in formatted_path: + formatted_path = formatted_path.replace("//", "/") + formatted_path = formatted_path.rstrip("/") + return formatted_path async def _execute_download( diff --git a/py/utils/utils.py b/py/utils/utils.py index 8ed20924..e61be515 100644 --- a/py/utils/utils.py +++ b/py/utils/utils.py @@ -488,6 +488,12 @@ def calculate_relative_path_for_model( if model_type == "embedding": formatted_path = formatted_path.replace(" ", "_") + # Sanitize the resolved path to prevent path traversal + formatted_path = formatted_path.lstrip("/") + while "//" in formatted_path: + formatted_path = formatted_path.replace("//", "/") + formatted_path = formatted_path.rstrip("/") + return formatted_path diff --git a/tests/services/test_download_manager_basic.py b/tests/services/test_download_manager_basic.py index 22c7e42d..4f717949 100644 --- a/tests/services/test_download_manager_basic.py +++ b/tests/services/test_download_manager_basic.py @@ -1189,6 +1189,65 @@ def test_relative_path_sanitizes_model_and_version_placeholders(): assert relative_path == "Fancy_Model/Version_One" +def test_relative_path_empty_first_tag_fallback(): + """Test that empty first_tag falls back to 'no tags'.""" + manager = DownloadManager() + settings_manager = get_settings_manager() + settings_manager.settings["download_path_templates"]["lora"] = ( + "{base_model}/{first_tag}" + ) + + version_info = { + "baseModel": "SDXL", + "model": {"name": "Test Model", "tags": []}, + "creator": {"username": "Author"}, + } + + relative_path = manager._calculate_relative_path(version_info, "lora") + + assert relative_path == "SDXL/no tags" + + +def test_relative_path_empty_base_model_and_first_tag(): + """Test that empty base_model + empty first_tag does NOT produce a leading slash.""" + manager = DownloadManager() + settings_manager = get_settings_manager() + settings_manager.settings["download_path_templates"]["lora"] = ( + "{base_model}/{first_tag}" + ) + + version_info = { + "baseModel": "", + "model": {"name": "Test Model", "tags": []}, + "creator": {"username": "Author"}, + } + + relative_path = manager._calculate_relative_path(version_info, "lora") + + assert not relative_path.startswith("/") + assert relative_path == "no tags" + + +def test_relative_path_sanitizes_double_slashes(): + """Test that empty placeholder substitutions don't produce double slashes.""" + manager = DownloadManager() + settings_manager = get_settings_manager() + settings_manager.settings["download_path_templates"]["lora"] = ( + "{base_model}/{first_tag}/{author}" + ) + + version_info = { + "baseModel": "SDXL", + "model": {"name": "Test Model", "tags": []}, + "creator": {"username": "Author"}, + } + + relative_path = manager._calculate_relative_path(version_info, "lora") + + assert "//" not in relative_path + assert relative_path == "SDXL/no tags/Author" + + def test_distribute_preview_to_entries_moves_and_copies(tmp_path): """Test that preview distribution moves file to first entry and copies to others.""" manager = DownloadManager() diff --git a/tests/utils/test_utils.py b/tests/utils/test_utils.py index 5886b519..85a7907e 100644 --- a/tests/utils/test_utils.py +++ b/tests/utils/test_utils.py @@ -114,6 +114,38 @@ def test_calculate_relative_path_sanitizes_model_and_version_names(isolated_sett assert relative_path == "Fancy_Model/Version_One" +def test_calculate_relative_path_sanitizes_leading_slash(isolated_settings): + """Test that empty base_model does NOT produce a leading slash in the path.""" + isolated_settings["download_path_templates"]["lora"] = "{base_model}/{first_tag}" + + model_data = { + "base_model": "", + "tags": [], + "civitai": {"id": 1, "creator": {"username": "Author"}}, + } + + relative_path = calculate_relative_path_for_model(model_data, "lora") + + assert not relative_path.startswith("/") + assert relative_path == "no tags" + + +def test_calculate_relative_path_sanitizes_double_slashes(isolated_settings): + """Test that empty substitutions don't produce double slashes.""" + isolated_settings["download_path_templates"]["lora"] = "{base_model}/{first_tag}/{author}" + + model_data = { + "base_model": "", + "tags": [], + "civitai": {"id": 1, "creator": {"username": "Author"}}, + } + + relative_path = calculate_relative_path_for_model(model_data, "lora") + + assert "//" not in relative_path + assert relative_path == "no tags/Author" + + def test_calculate_recipe_fingerprint_filters_and_sorts(): loras = [ {"hash": "ABC", "strength": 0.1234},