From 5cc1eb517094e3507915664817285f1e6e37a16d Mon Sep 17 00:00:00 2001 From: Classic298 <27028174+Classic298@users.noreply.github.com> Date: Thu, 14 May 2026 06:08:33 +0200 Subject: [PATCH] fix: enforce features.direct_tool_servers on chat-completion tool_servers (#24693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: enforce features.direct_tool_servers on chat-completion tool_servers The features.direct_tool_servers per-user permission was correctly enforced on the storage path (routers/users.py user/settings/update, which strips toolServers from saved settings when the caller lacks the permission), but the inference path (/api/chat/completions) popped tool_servers straight from the request body into metadata with no permission check. The middleware (utils/middleware.py:2799) then consumed direct_tool_servers to inject system_prompt into the message array and register external tool specs that get invoked during the completion. End result: any authenticated user could bypass the admin-set per-user feature toggle and use inline tool_servers in their chat-completion requests, even when admin had explicitly denied the permission. Default for USER_PERMISSIONS_FEATURES_DIRECT_TOOL_SERVERS is False (config.py:2750), so under default config no regular user is supposed to be able to use direct tool servers — making this a real boundary bypass on out-of-the-box deployments rather than a corner case. Mirror the storage-side behaviour at the inference entry point: pop tool_servers from the request body, then silently drop the value if the caller is non-admin and lacks features.direct_tool_servers. Admins always pass; users with the explicit grant always pass; everyone else gets None propagated into metadata, which the middleware already handles as the no-tool-servers case. Reported by berkant-koc in GHSA-f582-c373-jjf6. Co-authored-by: berkant-koc * chore: trim verbose comment on tool_servers permission check --------- Co-authored-by: berkant-koc --- backend/open_webui/main.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/backend/open_webui/main.py b/backend/open_webui/main.py index 4835cb4d17..45cc2fcadc 100644 --- a/backend/open_webui/main.py +++ b/backend/open_webui/main.py @@ -913,7 +913,7 @@ app.state.config.ENABLE_EVALUATION_ARENA_MODELS = ENABLE_EVALUATION_ARENA_MODELS app.state.config.EVALUATION_ARENA_MODELS = EVALUATION_ARENA_MODELS # Migrate legacy access_control → access_grants on boot -from open_webui.utils.access_control import migrate_access_control +from open_webui.utils.access_control import has_permission, migrate_access_control connections = app.state.config.TOOL_SERVER_CONNECTIONS if any('access_control' in c.get('config', {}) for c in connections): @@ -1746,6 +1746,21 @@ async def chat_completion( form_data.pop('id', None) user_message = form_data.pop('user_message', None) or form_data.pop('parent_message', None) + + # Drop tool_servers if caller lacks features.direct_tool_servers — + # mirrors the storage-side strip in user/settings/update. + tool_servers = form_data.pop('tool_servers', None) + if ( + tool_servers + and user.role != 'admin' + and not await has_permission( + user.id, + 'features.direct_tool_servers', + request.app.state.config.USER_PERMISSIONS, + ) + ): + tool_servers = None + metadata = { 'user_id': user.id, 'chat_id': form_data.pop('chat_id', None), @@ -1756,7 +1771,7 @@ async def chat_completion( 'folder_id': form_data.pop('folder_id', None), 'filter_ids': form_data.pop('filter_ids', []), 'tool_ids': form_data.get('tool_ids', None), - 'tool_servers': form_data.pop('tool_servers', None), + 'tool_servers': tool_servers, 'files': form_data.get('files', None), 'features': form_data.get('features', {}), 'variables': form_data.get('variables', {}),