fix: Security Patches (#6743)

Author: michael-genson

docs/docs/overrides/api.html

@@ -14,7 +14,7 @@
       <BaseButton
         download
         size="small"
-        :download-url="`/api/recipes/bulk-actions/export/download?path=${item.path}`"
+        :download-url="`/api/recipes/bulk-actions/export/${item.id}/download`"
       />
     </template>
   </v-data-table>

frontend/components/Domain/Group/GroupExportData.vue

@@ -29,7 +29,7 @@ export default defineNuxtComponent({
           "ul", "ol", "li", "dl", "dt", "dd", "abbr", "a", "img", "blockquote", "iframe",
           "del", "ins", "table", "thead", "tbody", "tfoot", "tr", "th", "td", "colgroup",
         ],
-        ADD_ATTR: [
+        ALLOWED_ATTR: [
           "href", "src", "alt", "height", "width", "class", "allow", "title", "allowfullscreen", "frameborder",
           "scrolling", "cite", "datetime", "name", "abbr", "target", "border",
         ],

frontend/components/global/SafeMarkdown.vue

@@ -2,6 +2,7 @@
 from pathlib import Path
 
 from fastapi import APIRouter, HTTPException
+from pydantic import UUID4
 
 from mealie.core.dependencies.dependencies import get_temporary_zip_path
 from mealie.core.security import create_file_token
@@ -48,14 +49,15 @@ def bulk_export_recipes(self, export_recipes: ExportRecipes):
         with get_temporary_zip_path() as temp_path:
             self.service.export_recipes(temp_path, export_recipes.recipes)
 
-    @router.get("/export/download")
-    def get_exported_data_token(self, path: Path):
+    @router.get("/export/{export_id}/download")
+    def get_exported_data_token(self, export_id: UUID4):
         """Returns a token to download a file"""
-        path = Path(path).resolve()
 
-        if not path.is_relative_to(self.folders.DATA_DIR):
-            raise HTTPException(400, "path must be relative to data directory")
+        export = self.service.get_export(export_id)
+        if not export:
+            raise HTTPException(404, "export not found")
 
+        path = Path(export.path).resolve()
         return {"fileToken": create_file_token(path)}
 
     @router.get("/export", response_model=list[GroupDataExport])

mealie/routes/recipe/bulk_actions.py

@@ -17,8 +17,12 @@ async def download_file(file_path: Path = Depends(validate_file_token)):
     file_path = Path(file_path).resolve()
 
     dirs = get_app_dirs()
+    allowed_dirs = [
+        dirs.BACKUP_DIR,  # admin backups
+        dirs.GROUPS_DIR,  # group exports
+    ]
 
-    if not file_path.is_relative_to(dirs.DATA_DIR):
+    if not any(file_path.is_relative_to(allowed_dir) for allowed_dir in allowed_dirs):
         raise HTTPException(status.HTTP_400_BAD_REQUEST)
 
     if not file_path.is_file():

mealie/routes/utility_routes.py

@@ -1,11 +1,14 @@
 from pathlib import Path
 
+from pydantic import UUID4
+
 from mealie.core.exceptions import UnexpectedNone
 from mealie.repos.repository_factory import AllRepositories
 from mealie.schema.group.group_exports import GroupDataExport
 from mealie.schema.recipe import CategoryBase
 from mealie.schema.recipe.recipe_category import TagBase
 from mealie.schema.recipe.recipe_settings import RecipeSettings
+from mealie.schema.response.pagination import PaginationQuery
 from mealie.schema.user.user import GroupInDB, PrivateUser
 from mealie.services._base_service import BaseService
 from mealie.services.exporter import Exporter, RecipeExporter
@@ -25,7 +28,11 @@ def export_recipes(self, temp_path: Path, slugs: list[str]) -> None:
         exporter.run(self.repos)
 
     def get_exports(self) -> list[GroupDataExport]:
-        return self.repos.group_exports.multi_query({"group_id": self.group.id})
+        exports_page = self.repos.group_exports.page_all(PaginationQuery(per_page=-1))
+        return exports_page.items
+
+    def get_export(self, id: UUID4) -> GroupDataExport | None:
+        return self.repos.group_exports.get_one(id)
 
     def purge_exports(self) -> int:
         all_exports = self.get_exports()

mealie/services/recipe/recipe_bulk_service.py

@@ -123,11 +123,12 @@ def test_bulk_export_recipes(api_client: TestClient, unique_user: TestUser, ten_
     response_data = response.json()
     assert len(response_data) == 1
 
+    export_id = response_data[0]["id"]
     export_path = response_data[0]["path"]
 
     # Get Export Token
     response = api_client.get(
-        f"{api_routes.recipes_bulk_actions_export_download}?path={export_path}", headers=unique_user.token
+        f"{api_routes.recipes_bulk_actions_export_export_id_download(export_id)}", headers=unique_user.token
     )
     assert response.status_code == 200
 

tests/integration_tests/user_recipe_tests/test_recipe_bulk_action.py

@@ -2,10 +2,11 @@
 
 import ldap
 import pytest
+from fastapi import HTTPException
 from pytest import MonkeyPatch
 
 from mealie.core import security
-from mealie.core.config import get_app_settings
+from mealie.core.config import get_app_dirs, get_app_settings
 from mealie.core.dependencies import validate_file_token
 from mealie.core.security.providers.credentials_provider import (
     CredentialsProvider,
@@ -15,6 +16,7 @@
 from mealie.db.db_setup import session_context
 from mealie.db.models.users.users import AuthMethod
 from mealie.repos.repository_factory import AllRepositories
+from mealie.routes.utility_routes import download_file
 from mealie.schema.user.auth import CredentialsRequestForm
 from mealie.schema.user.user import PrivateUser
 from tests.utils import random_string
@@ -122,6 +124,46 @@ def test_create_file_token():
     assert file_path == validate_file_token(file_token)
 
 
+@pytest.mark.asyncio
+async def test_download_file_security_restrictions():
+    dirs = get_app_dirs()
+
+    # Test 1: File in DATA_DIR but outside allowed dirs should be blocked
+    secret_file = dirs.DATA_DIR / ".secret"
+
+    with pytest.raises(HTTPException) as exc_info:
+        await download_file(secret_file)
+    assert exc_info.value.status_code == 400
+
+    # Test 2: File in BACKUP_DIR should be allowed (but only if it exists)
+    backup_file = dirs.BACKUP_DIR / "test.zip"
+    dirs.BACKUP_DIR.mkdir(parents=True, exist_ok=True)
+    backup_file.write_text("test backup content")
+
+    try:
+        response = await download_file(backup_file)
+        assert response.media_type == "application/octet-stream"
+        assert response.path == backup_file
+    finally:
+        backup_file.unlink(missing_ok=True)
+
+    # Test 3: File in GROUPS_DIR should be allowed (but only if it exists)
+    export_dir = dirs.GROUPS_DIR / "some-group-id" / "export"
+    export_dir.mkdir(parents=True, exist_ok=True)
+    export_file = export_dir / "test.zip"
+    export_file.write_text("test export content")
+
+    try:
+        response = await download_file(export_file)
+        assert response.media_type == "application/octet-stream"
+        assert response.path == export_file
+    finally:
+        export_file.unlink(missing_ok=True)
+        # Clean up the directory structure
+        export_dir.rmdir()
+        (dirs.GROUPS_DIR / "some-group-id").rmdir()
+
+
 def get_provider(session, username: str, password: str):
     request_data = CredentialsRequest(username=username, password=password)
     return LDAPProvider(session, request_data)

tests/unit_tests/test_security.py

@@ -141,8 +141,6 @@
 """`/api/recipes/bulk-actions/delete`"""
 recipes_bulk_actions_export = "/api/recipes/bulk-actions/export"
 """`/api/recipes/bulk-actions/export`"""
-recipes_bulk_actions_export_download = "/api/recipes/bulk-actions/export/download"
-"""`/api/recipes/bulk-actions/export/download`"""
 recipes_bulk_actions_export_purge = "/api/recipes/bulk-actions/export/purge"
 """`/api/recipes/bulk-actions/export/purge`"""
 recipes_bulk_actions_settings = "/api/recipes/bulk-actions/settings"
@@ -463,6 +461,11 @@ def organizers_tools_slug_tool_slug(tool_slug):
     return f"{prefix}/organizers/tools/slug/{tool_slug}"
 
 
+def recipes_bulk_actions_export_export_id_download(export_id):
+    """`/api/recipes/bulk-actions/export/{export_id}/download`"""
+    return f"{prefix}/recipes/bulk-actions/export/{export_id}/download"
+
+
 def recipes_shared_token_id(token_id):
     """`/api/recipes/shared/{token_id}`"""
     return f"{prefix}/recipes/shared/{token_id}"