From ba78ebea2792904f63c4ad8b3dfd80e0efe6a124 Mon Sep 17 00:00:00 2001 From: Classic298 <27028174+Classic298@users.noreply.github.com> Date: Wed, 13 May 2026 19:57:57 +0200 Subject: [PATCH] fix: validate folder_id ownership on chat create + folder-update endpoints (#24588) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/v1/chats/new and POST /api/v1/chats/{id}/folder accepted a caller-supplied folder_id with no validation — neither ownership, nor existence, nor UUID format. The row was persisted with the supplied value verbatim, so the DB ended up with chat rows whose folder_id referenced another user's folder, a non-existent UUID, or even a non-UUID string. No read path surfaces this across users — every chat-folder read is user_id-filtered on both sides — so this is referential-integrity hardening rather than a security boundary fix. But there's no reason to accept dangling references either, and the downstream consumers shouldn't have to assume the column is clean. Add a Folders.get_folder_by_id_and_user_id() lookup at both writers: if a folder_id is supplied, it must match a folder owned by the caller. None remains allowed (chat-without-folder is the default). Non-existent and non-UUID values fall through to 404. Reported by ShigekiTsuchiyama in GHSA-4vrg-2vcq-q7jc. Co-authored-by: ShigekiTsuchiyama --- backend/open_webui/routers/chats.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/backend/open_webui/routers/chats.py b/backend/open_webui/routers/chats.py index c2f3da563d..9344173807 100644 --- a/backend/open_webui/routers/chats.py +++ b/backend/open_webui/routers/chats.py @@ -557,6 +557,18 @@ async def create_new_chat( user=Depends(get_verified_user), db: AsyncSession = Depends(get_async_session), ): + # Reject a folder_id that doesn't belong to the caller. Without this the + # row is persisted with a dangling foreign reference — no read path + # surfaces it across users (all chat reads are user_id-filtered), but + # the row state is meaningless and downstream consumers shouldn't have + # to assume the column is clean. Also catches non-UUID / nonexistent IDs. + if form_data.folder_id is not None: + if not await Folders.get_folder_by_id_and_user_id(form_data.folder_id, user.id, db=db): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=ERROR_MESSAGES.NOT_FOUND, + ) + try: chat = await Chats.insert_new_chat(str(uuid4()), user.id, form_data, db=db) return ChatResponse(**chat.model_dump()) @@ -1484,6 +1496,15 @@ async def update_chat_folder_id_by_id( ): chat = await Chats.get_chat_by_id_and_user_id(id, user.id, db=db) if chat: + # Same ownership check as the create path — reject foreign / dangling + # folder_id values. None is allowed (moves the chat out of any folder). + if form_data.folder_id is not None: + if not await Folders.get_folder_by_id_and_user_id(form_data.folder_id, user.id, db=db): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=ERROR_MESSAGES.NOT_FOUND, + ) + chat = await Chats.update_chat_folder_id_by_id_and_user_id(id, user.id, form_data.folder_id, db=db) return ChatResponse(**chat.model_dump()) else: