fix: enforce ownership check on recipe deletion (GHSA-x5v9-9jvh-7c7q) (#7625)

This commit is contained in:
Hayden
2026-05-14 12:04:04 -05:00
committed by GitHub
parent eddb0c30e0
commit 742b498c1d
3 changed files with 60 additions and 14 deletions

View File

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

View File

@@ -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])

View File

@@ -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
):