Skip to content

fix: enforce tool access and skip stale socket sessions - #26

Draft
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/critical-bug-investigation-f1f0
Draft

cursor[bot] wants to merge 1 commit into
mainfrom
cursor/critical-bug-investigation-f1f0

Conversation

@cursor

@cursor cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

Pull Request Checklist

Note to first-time contributors: Please open a discussion post in Discussions and describe your changes before submitting a pull request.

Before submitting, make sure you've checked the following:

  • Target branch: This automation uses the repository's configured PR base.
  • Description: Fixes two high-impact regressions found while auditing recent changes.
  • Changelog: Not added to source files; changelog details are included below.
  • Documentation: Not applicable for this security/correctness fix.
  • Dependencies: No new dependencies.
  • Testing: Focused compile checks and dependency-light regression tests were run manually.
  • Code review: Self-reviewed the scoped diff.
  • Prefix: Title uses the fix prefix.

Changelog Entry

Description

  • Enforce tool read access before chat completion registers server-side tool callables, and make channel notification setup tolerate stale socket room participants.

Added

  • Dependency-light helpers and regression coverage for tool authorization and socket session filtering.

Changed

  • Chat tool loading now applies owner/admin/read-access checks to database tools and configured tool servers.
  • Channel socket room user lookup now skips stale or empty session entries.

Deprecated

  • None.

Removed

  • None.

Fixed

  • Unauthorized users could submit arbitrary tool_ids directly and execute tools hidden by UI access controls.
  • Posting a channel message could persist and broadcast successfully but still return an error if a stale socket participant remained in the room.

Security

  • Prevents restricted tool execution through direct chat completion payloads or URL-populated tool IDs.

Breaking Changes

  • None.

Additional Information

Bug and impact:

  • Restricted tools were filtered from the tools list UI, but the chat execution helper trusted client-supplied tool_ids; a user could invoke a private/shared-only tool by ID.
  • get_user_ids_from_room() assumed every socket room participant had a live SESSION_POOL entry; after a disconnect race, channel message creation had already committed and emitted before notification setup crashed and returned HTTP 400, encouraging duplicate retries.

Root cause:

  • The server-side get_tools() callable loader did not reuse the access checks from the tools listing endpoints.
  • Socket room membership and session pool cleanup can become temporarily inconsistent, but the room lookup dereferenced SESSION_POOL.get(...)["id"] unconditionally.

Fix:

  • Added a shared can_use_tool() helper and applied it to database-backed tools and configured tool servers before registering callables.
  • Added a session filtering helper that ignores stale/malformed session entries and wired get_user_ids_from_room() through it.

Validation performed:

  • python3 -m py_compile backend/open_webui/utils/tool_access.py backend/open_webui/utils/tools.py backend/open_webui/socket/sessions.py backend/open_webui/socket/main.py tests/test_tool_and_socket_regressions.py
  • Manual execution of all functions in tests/test_tool_and_socket_regressions.py (4 passed).
  • git diff --cached --check

Tooling note:

  • python3 -m pytest tests/test_tool_and_socket_regressions.py could not run because pytest is not installed in the base image.
  • python3 -m black ... could not run because black is not installed in the base image.

Screenshots or Videos

  • Not applicable; backend-only changes.

Contributor License Agreement

By submitting this pull request, I confirm that I have read and fully agree to the Contributor License Agreement (CLA), and I am providing my contributions under its terms.

Open in Web View Automation 

Co-authored-by: mkatwi <mkatwi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant