fix: restrict folder deletion to the owner or an admin (#27003)

* fix: restrict folder deletion to the owner or an admin

Deleting a folder cascades into the folder owner's chats, messages and the
entire subfolder subtree; the cascade is bound to the folder's owner, not the
caller. The delete handler only enforced owner/admin for root folders.
Subfolder deletion required merely write access, and a write grant on a shared
root folder is inherited by every descendant subfolder. A write-collaborator
could therefore permanently delete the owner's chats by deleting a subfolder of
a shared folder, data they do not own. With delete_contents=false the same path
force-moved the owner's chats out of the folder instead.

This also contradicted the documented sharing model: only the owner or an admin
may delete a shared folder, and write access covers adding and editing chats and
subfolders, not removing the folder.

Because any folder deletion cascades into the owner's data, restrict it to the
owner or an admin for root and subfolders alike, replacing the root/subfolder
split with a single check. Owners and admins are unaffected, and a
write-collaborator can still create, rename and add to shared folders and delete
subfolders they own.

Co-authored-by: legobattman <302282032+legobattman@users.noreply.github.com>

* style: condense the folder deletion authorization comment

Shorten the multi-line comment above the owner-or-admin check to a single line stating why deletion is restricted. The full rationale lives in the pull request description and does not need to be narrated in the code.

---------

Co-authored-by: legobattman <302282032+legobattman@users.noreply.github.com>
This commit is contained in:
Classic298
2026-07-27 07:46:58 +02:00
committed by GitHub
parent 305880f2e2
commit 915ef7d079
+7 -15
View File
@@ -626,26 +626,18 @@ async def delete_folder_by_id(
folder = await Folders.get_folder_by_id_and_user_id(id, user.id, db=db)
if not folder:
# Check if it's a shared subfolder with write access
# Deletion cascades into the owner's data, so only the owner or an admin may delete
folder = await Folders.get_folder_by_id(id, db=db)
if folder and folder.parent_id:
if user.role != 'admin' and not await _has_folder_access(user.id, folder, 'write', db):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=ERROR_MESSAGES.ACCESS_PROHIBITED,
)
elif folder and not folder.parent_id:
# Root shared folders can only be deleted by owner/admin
if user.role != 'admin':
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=ERROR_MESSAGES.ACCESS_PROHIBITED,
)
else:
if not folder:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=ERROR_MESSAGES.NOT_FOUND,
)
if user.role != 'admin':
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=ERROR_MESSAGES.ACCESS_PROHIBITED,
)
folder_owner_id = folder.user_id