fix(download): prevent path traversal in download template resolution (#1028)

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(

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
 
 

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()

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},