From 6f03010f6c68b02facf7a93a7efe8c1f0b56f0da Mon Sep 17 00:00:00 2001 From: Michael Genson <71845777+michael-genson@users.noreply.github.com> Date: Thu, 18 Dec 2025 16:54:16 -0600 Subject: [PATCH] fix: Security Patches (#6743) --- docs/docs/overrides/api.html | 2 +- .../Domain/Group/GroupExportData.vue | 2 +- frontend/components/global/SafeMarkdown.vue | 2 +- mealie/routes/recipe/bulk_actions.py | 12 ++--- mealie/routes/utility_routes.py | 6 ++- mealie/services/recipe/recipe_bulk_service.py | 9 +++- .../test_recipe_bulk_action.py | 3 +- tests/unit_tests/test_security.py | 44 ++++++++++++++++++- tests/utils/api_routes/__init__.py | 7 ++- 9 files changed, 73 insertions(+), 14 deletions(-) diff --git a/docs/docs/overrides/api.html b/docs/docs/overrides/api.html index 39aa3d36a..61db12ccc 100644 --- a/docs/docs/overrides/api.html +++ b/docs/docs/overrides/api.html @@ -14,7 +14,7 @@
diff --git a/frontend/components/Domain/Group/GroupExportData.vue b/frontend/components/Domain/Group/GroupExportData.vue index 82549c043..a9af743e7 100644 --- a/frontend/components/Domain/Group/GroupExportData.vue +++ b/frontend/components/Domain/Group/GroupExportData.vue @@ -14,7 +14,7 @@ diff --git a/frontend/components/global/SafeMarkdown.vue b/frontend/components/global/SafeMarkdown.vue index 8cb4c9606..0d9f57a68 100644 --- a/frontend/components/global/SafeMarkdown.vue +++ b/frontend/components/global/SafeMarkdown.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", ], diff --git a/mealie/routes/recipe/bulk_actions.py b/mealie/routes/recipe/bulk_actions.py index e45201349..4d34b40ea 100644 --- a/mealie/routes/recipe/bulk_actions.py +++ b/mealie/routes/recipe/bulk_actions.py @@ -2,6 +2,7 @@ from functools import cached_property 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 @@ class RecipeBulkActionsController(BaseUserController): 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]) diff --git a/mealie/routes/utility_routes.py b/mealie/routes/utility_routes.py index cc67f348d..ce1d4e281 100644 --- a/mealie/routes/utility_routes.py +++ b/mealie/routes/utility_routes.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(): diff --git a/mealie/services/recipe/recipe_bulk_service.py b/mealie/services/recipe/recipe_bulk_service.py index a5395682a..32601af25 100644 --- a/mealie/services/recipe/recipe_bulk_service.py +++ b/mealie/services/recipe/recipe_bulk_service.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 @@ class RecipeBulkActionsService(BaseService): 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() diff --git a/tests/integration_tests/user_recipe_tests/test_recipe_bulk_action.py b/tests/integration_tests/user_recipe_tests/test_recipe_bulk_action.py index 3b2d2cea8..87ef349a5 100644 --- a/tests/integration_tests/user_recipe_tests/test_recipe_bulk_action.py +++ b/tests/integration_tests/user_recipe_tests/test_recipe_bulk_action.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 diff --git a/tests/unit_tests/test_security.py b/tests/unit_tests/test_security.py index 5aad76616..89412d011 100644 --- a/tests/unit_tests/test_security.py +++ b/tests/unit_tests/test_security.py @@ -2,10 +2,11 @@ from pathlib import Path 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.core.security.providers.ldap_provider import LDAPProvider 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) diff --git a/tests/utils/api_routes/__init__.py b/tests/utils/api_routes/__init__.py index 43708712d..4da4ba601 100644 --- a/tests/utils/api_routes/__init__.py +++ b/tests/utils/api_routes/__init__.py @@ -141,8 +141,6 @@ recipes_bulk_actions_delete = "/api/recipes/bulk-actions/delete" """`/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}"