From 23b3a69bc26839bfa74edd1be6bfa2568ae902f4 Mon Sep 17 00:00:00 2001 From: Classic298 <27028174+Classic298@users.noreply.github.com> Date: Mon, 24 Aug 2026 23:06:19 +0200 Subject: [PATCH] fix: keep folder parent references acyclic (#28748) Moving a folder under one of its own subfolders was accepted. A folder in a parent loop is never a root, so it and everything under it silently disappeared from the sidebar, and there was no way to get it back from the UI. The move is now rejected with a 400, folders whose parent chain loops are put back at the root on the next folder list, and the folder tree traversals skip ids they have already visited so existing data in that state stays workable. --- backend/open_webui/models/folders.py | 18 ++++++++--- backend/open_webui/routers/folders.py | 22 +++++++++++-- .../utils/access_control/folders.py | 32 ++++++++++--------- 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/backend/open_webui/models/folders.py b/backend/open_webui/models/folders.py index 24bb5e5af0..1db732ed7e 100644 --- a/backend/open_webui/models/folders.py +++ b/backend/open_webui/models/folders.py @@ -202,10 +202,14 @@ class FolderTable: try: async with get_async_db_context(db) as db: folders = [] + seen_ids = {id} async def get_children(folder): children = await self.get_folders_by_parent_id_and_user_id(folder.id, user_id, db=db) for child in children: + if child.id in seen_ids: + continue + seen_ids.add(child.id) await get_children(child) folders.append(child) @@ -267,15 +271,17 @@ class FolderTable: if not folder: return [] - folder_ids = [folder.id] + folder_ids = {folder.id} folders = [FolderModel.model_validate(folder)] while folders: current_folder = folders.pop() children = await self.get_folders_by_parent_id_and_user_id(current_folder.id, user_id, db=db) - folder_ids.extend(child.id for child in children) - folders.extend(children) + for child in children: + if child.id not in folder_ids: + folder_ids.add(child.id) + folders.append(child) - return folder_ids + return list(folder_ids) async def update_folder_parent_id_by_id_and_user_id( self, @@ -385,11 +391,15 @@ class FolderTable: return folder_ids folder_ids.append(folder.id) + seen_ids = {folder.id} # Delete all children folders async def delete_children(folder): folder_children = await self.get_folders_by_parent_id_and_user_id(folder.id, user_id, db=db) for folder_child in folder_children: + if folder_child.id in seen_ids: + continue + seen_ids.add(folder_child.id) await delete_children(folder_child) folder_ids.append(folder_child.id) diff --git a/backend/open_webui/routers/folders.py b/backend/open_webui/routers/folders.py index 9a6ff1c4df..7bc8641d68 100644 --- a/backend/open_webui/routers/folders.py +++ b/backend/open_webui/routers/folders.py @@ -97,7 +97,15 @@ async def get_folders( await check_folders_permission(request, user, db=db) folders = await Folders.get_folders_by_user_id(user.id, db=db) - folder_ids = {folder.id for folder in folders} + parent_by_id = {folder.id: folder.parent_id for folder in folders} + + def is_in_parent_cycle(folder_id): + seen_ids = {folder_id} + current_id = parent_by_id.get(folder_id) + while current_id and current_id not in seen_ids: + seen_ids.add(current_id) + current_id = parent_by_id.get(current_id) + return current_id == folder_id user_group_ids = None if user.role != 'admin' and any(folder.data and 'files' in folder.data for folder in folders): @@ -106,7 +114,9 @@ async def get_folders( # Verify folder data integrity folder_list = [] for folder in folders: - if folder.parent_id and folder.parent_id not in folder_ids: + # A missing or looping parent hides the folder from the tree, so put it back at the root + if folder.parent_id and (folder.parent_id not in parent_by_id or is_in_parent_cycle(folder.id)): + parent_by_id[folder.id] = None folder = await Folders.update_folder_parent_id_by_id_and_user_id(folder.id, user.id, None, db=db) if folder.data and 'files' in folder.data: @@ -397,6 +407,14 @@ async def update_folder_parent_id_by_id( detail=ERROR_MESSAGES.DEFAULT('Folder already exists'), ) + if form_data.parent_id and form_data.parent_id in await Folders.get_folder_ids_by_id_and_user_id_in_subtree( + id, user.id, db=db + ): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ERROR_MESSAGES.DEFAULT('Cannot move a folder into itself or one of its subfolders'), + ) + try: folder = await Folders.update_folder_parent_id_by_id_and_user_id(id, user.id, form_data.parent_id, db=db) await publish_event( diff --git a/backend/open_webui/utils/access_control/folders.py b/backend/open_webui/utils/access_control/folders.py index 1eb32b660c..1a632ba01b 100644 --- a/backend/open_webui/utils/access_control/folders.py +++ b/backend/open_webui/utils/access_control/folders.py @@ -5,22 +5,24 @@ from sqlalchemy.ext.asyncio import AsyncSession async def has_folder_access(user_id: str, folder: FolderModel, permission: str, db: AsyncSession | None) -> bool: """Check if user has access to folder directly or via ancestor inheritance.""" - if folder.user_id == user_id: - return True + # A corrupt parent loop must not spin forever + seen_ids = set() + while folder and folder.id not in seen_ids: + seen_ids.add(folder.id) - if await AccessGrants.has_access( - user_id=user_id, - resource_type='folder', - resource_id=folder.id, - permission=permission, - db=db, - ): - return True - # Check ancestor chain for inherited access - if folder.parent_id: - parent = await Folders.get_folder_by_id(folder.parent_id, db=db) - if parent: - return await has_folder_access(user_id, parent, permission, db) + if folder.user_id == user_id: + return True + + if await AccessGrants.has_access( + user_id=user_id, + resource_type='folder', + resource_id=folder.id, + permission=permission, + db=db, + ): + return True + + folder = await Folders.get_folder_by_id(folder.parent_id, db=db) if folder.parent_id else None return False