fix: enforce features.direct_tool_servers on chat-completion tool_servers (#24693)

* 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 <berkant-koc@users.noreply.github.com>

* chore: trim verbose comment on tool_servers permission check

---------

Co-authored-by: berkant-koc <berkant-koc@users.noreply.github.com>
This commit is contained in:
Classic298
2026-05-14 06:08:33 +02:00
committed by GitHub
parent ae06e199d5
commit 5cc1eb5170

View File

@@ -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', {}),