From 742b498c1d0f6e30c5338f3b33f9458ce3275eb7 Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Thu, 14 May 2026 12:04:04 -0500 Subject: [PATCH] fix: enforce ownership check on recipe deletion (GHSA-x5v9-9jvh-7c7q) (#7625) --- mealie/services/recipe/recipe_service.py | 15 ++++++- .../test_recipe_cross_household.py | 17 +++----- .../user_recipe_tests/test_recipe_owner.py | 42 +++++++++++++++++++ 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/mealie/services/recipe/recipe_service.py b/mealie/services/recipe/recipe_service.py index 680d72d80..1f6c9bde1 100644 --- a/mealie/services/recipe/recipe_service.py +++ b/mealie/services/recipe/recipe_service.py @@ -71,8 +71,19 @@ class RecipeService(RecipeServiceBase): def can_delete(self, recipe_slugs: list[str]) -> bool: if self.user.admin: return True - else: - return self.can_update(recipe_slugs) + + # Deletion requires ownership; collaborative editing rules (can_update) do not apply + model = self.group_recipes.model + owned_count = self.group_recipes.session.scalar( + sa.select(sa.func.count()) + .select_from(model) + .where( + model.slug.in_(recipe_slugs), + model.group_id == self.user.group_id, + model.user_id == self.user.id, + ) + ) + return owned_count == len(recipe_slugs) def can_update(self, recipe_slugs: list[str]) -> bool: sql = dedent( diff --git a/tests/integration_tests/user_recipe_tests/test_recipe_cross_household.py b/tests/integration_tests/user_recipe_tests/test_recipe_cross_household.py index 5b649e151..f0e40373e 100644 --- a/tests/integration_tests/user_recipe_tests/test_recipe_cross_household.py +++ b/tests/integration_tests/user_recipe_tests/test_recipe_cross_household.py @@ -201,19 +201,12 @@ def test_delete_recipes_from_other_households( assert recipe_json["id"] == h2_recipe_id response = api_client.delete(api_routes.recipes_slug(recipe_json["slug"]), headers=unique_user.token) - if household_lock_recipe_edits: - assert response.status_code == 403 + assert response.status_code == 403 - # confirm the recipe still exists - response = api_client.get(api_routes.recipes_slug(h2_recipe_id), headers=unique_user.token) - assert response.status_code == 200 - assert response.json()["id"] == h2_recipe_id - else: - assert response.status_code == 200 - - # confirm the recipe was deleted - response = api_client.get(api_routes.recipes_slug(h2_recipe_id), headers=unique_user.token) - assert response.status_code == 404 + # confirm the recipe still exists + response = api_client.get(api_routes.recipes_slug(h2_recipe_id), headers=unique_user.token) + assert response.status_code == 200 + assert response.json()["id"] == h2_recipe_id @pytest.mark.parametrize("is_private_household", [True, False]) diff --git a/tests/integration_tests/user_recipe_tests/test_recipe_owner.py b/tests/integration_tests/user_recipe_tests/test_recipe_owner.py index 86397d61d..e0b5ebb42 100644 --- a/tests/integration_tests/user_recipe_tests/test_recipe_owner.py +++ b/tests/integration_tests/user_recipe_tests/test_recipe_owner.py @@ -160,6 +160,24 @@ def test_other_user_cant_delete_recipe(api_client: TestClient, user_tuple: list[ assert response.status_code == 403 +def test_other_user_cant_delete_unlocked_recipe(api_client: TestClient, user_tuple: list[TestUser]): + """Non-owner must not delete an unlocked recipe — BOLA regression (GHSA-x5v9-9jvh-7c7q).""" + slug = random_string(10) + unique_user, other_user = user_tuple + + unique_user.repos.recipes.create( + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=slug, + settings=RecipeSettings(locked=False), + ) + ) + + response = api_client.delete(api_routes.recipes_slug(slug), headers=other_user.token) + assert response.status_code == 403 + + def test_other_user_bulk_delete(api_client: TestClient, user_tuple: list[TestUser]): slug_locked = random_string(10) slug_unlocked = random_string(10) @@ -190,6 +208,30 @@ def test_other_user_bulk_delete(api_client: TestClient, user_tuple: list[TestUse assert response.status_code == 403 +def test_other_user_cant_bulk_delete_unlocked_recipes(api_client: TestClient, user_tuple: list[TestUser]): + """Non-owner must not bulk-delete unlocked recipes — BOLA regression (GHSA-x5v9-9jvh-7c7q).""" + slug_1 = random_string(10) + slug_2 = random_string(10) + unique_user, other_user = user_tuple + + for slug in (slug_1, slug_2): + unique_user.repos.recipes.create( + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=slug, + settings=RecipeSettings(locked=False), + ) + ) + + response = api_client.post( + api_routes.recipes_bulk_actions_delete, + json={"recipes": [slug_1, slug_2]}, + headers=other_user.token, + ) + assert response.status_code == 403 + + def test_admin_can_delete_locked_recipe_owned_by_another_user( api_client: TestClient, unfiltered_database: AllRepositories, unique_user: TestUser, admin_user: TestUser ):