[PR #23542] [CLOSED] fix: null-check file owner, timing-safe SCIM token, GCS nested paths #82120

Closed
opened 2026-05-13 16:34:38 -05:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/open-webui/open-webui/pull/23542
Author: @barry3406
Created: 4/9/2026
Status: Closed

Base: devHead: fix/backend-bugs-v2


📝 Commits (3)

  • 57a7996 fix: use timing-safe comparison for SCIM bearer token
  • 13e7aeb fix: correct GCS filename extraction for nested paths
  • 3f6e575 fix: add null check for file owner in get_html_file_content_by_id

📊 Changes

3 files changed (+5 additions, -4 deletions)

View changed files

📝 backend/open_webui/routers/files.py (+1 -1)
📝 backend/open_webui/routers/scim.py (+2 -1)
📝 backend/open_webui/storage/provider.py (+2 -2)

📄 Description

Pull Request Checklist

  • Target branch: Verify that the pull request targets the dev branch.
  • Description: Provided below.
  • Changelog: Provided below.
  • Testing: Manually verified the logic of each fix.
  • Agentic AI Code: This PR has gone through human review and manual testing.
  • Code review: Self-reviewed.
  • Git Hygiene: Atomic commits, rebased on dev.
  • Title Prefix: fix:

Changelog Entry

Description

Three independent backend bug fixes found during code audit.

Fixed

  • files.py: get_html_file_content_by_id crashes with 500 when accessing a file whose owner has been deleted. Users.get_user_by_id() returns None but .role is accessed without null check. Added not file_user or guard.
  • scim.py: SCIM bearer token compared with != which is vulnerable to timing side-channel attacks. Replaced with hmac.compare_digest(), consistent with the existing pattern in utils/auth.py.
  • provider.py: GCS get_file and delete_file use split('/')[1] to extract filename, which returns the wrong segment for nested paths (e.g. gs://bucket/subdir/file.txt returns subdir). Changed to split('/')[-1], consistent with the S3 provider.

Security

  • SCIM token comparison now uses constant-time hmac.compare_digest to prevent timing attacks

Additional Information

  • The file owner null-check bug is triggered when an admin deletes a user but their uploaded files remain
  • The GCS bug only affects deployments using GCS with key prefixes or subdirectories
  • No new dependencies added

Contributor License Agreement


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/open-webui/open-webui/pull/23542 **Author:** [@barry3406](https://github.com/barry3406) **Created:** 4/9/2026 **Status:** ❌ Closed **Base:** `dev` ← **Head:** `fix/backend-bugs-v2` --- ### 📝 Commits (3) - [`57a7996`](https://github.com/open-webui/open-webui/commit/57a7996ab531f3e9f91c709b1e7b4c75f31cc6bc) fix: use timing-safe comparison for SCIM bearer token - [`13e7aeb`](https://github.com/open-webui/open-webui/commit/13e7aeb0f3d861ef358516411e7e520753131826) fix: correct GCS filename extraction for nested paths - [`3f6e575`](https://github.com/open-webui/open-webui/commit/3f6e575519690d297fe098564f17fa0bb2535bdf) fix: add null check for file owner in get_html_file_content_by_id ### 📊 Changes **3 files changed** (+5 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `backend/open_webui/routers/files.py` (+1 -1) 📝 `backend/open_webui/routers/scim.py` (+2 -1) 📝 `backend/open_webui/storage/provider.py` (+2 -2) </details> ### 📄 Description # Pull Request Checklist - [x] **Target branch:** Verify that the pull request targets the `dev` branch. - [x] **Description:** Provided below. - [x] **Changelog:** Provided below. - [x] **Testing:** Manually verified the logic of each fix. - [x] **Agentic AI Code:** This PR has gone through human review and manual testing. - [x] **Code review:** Self-reviewed. - [x] **Git Hygiene:** Atomic commits, rebased on `dev`. - [x] **Title Prefix:** `fix:` # Changelog Entry ### Description Three independent backend bug fixes found during code audit. ### Fixed - **files.py**: `get_html_file_content_by_id` crashes with 500 when accessing a file whose owner has been deleted. `Users.get_user_by_id()` returns `None` but `.role` is accessed without null check. Added `not file_user or` guard. - **scim.py**: SCIM bearer token compared with `!=` which is vulnerable to timing side-channel attacks. Replaced with `hmac.compare_digest()`, consistent with the existing pattern in `utils/auth.py`. - **provider.py**: GCS `get_file` and `delete_file` use `split('/')[1]` to extract filename, which returns the wrong segment for nested paths (e.g. `gs://bucket/subdir/file.txt` returns `subdir`). Changed to `split('/')[-1]`, consistent with the S3 provider. ### Security - SCIM token comparison now uses constant-time `hmac.compare_digest` to prevent timing attacks --- ### Additional Information - The file owner null-check bug is triggered when an admin deletes a user but their uploaded files remain - The GCS bug only affects deployments using GCS with key prefixes or subdirectories - No new dependencies added ### Contributor License Agreement - [x] By submitting this pull request, I confirm that I have read and fully agree to the [Contributor License Agreement (CLA)](https://github.com/open-webui/open-webui/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT), and I am providing my contributions under its terms. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-05-13 16:34:38 -05:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/open-webui#82120