Commit Graph

3972 Commits

Author SHA1 Message Date
Timothy Jaeryang Baek ecd74f220c refac 2026-04-14 17:22:54 -05:00
Timothy Jaeryang Baek 8bd23b9145 refac 2026-04-14 16:47:43 -05:00
Timothy Jaeryang Baek a4ed16999e refac 2026-04-14 16:08:14 -05:00
Classic298 a3ea7bf043 fix(retrieval): offload Loader.load to a worker thread so file uploads stop blocking the event loop (#23705)
Loader.load() dispatches to the underlying langchain document loaders
(PyMuPDF, Unstructured, python-docx, Tika, …) which are all
synchronous and CPU/IO-bound. process_file() awaited it directly on
the event loop, so parsing a non-trivial PDF/DOCX would freeze the
entire FastAPI app for the duration of the parse — which is what users
experience as "the server hangs whenever I upload a file."

Add an `aload()` async wrapper on Loader that runs the sync load on a
worker thread via asyncio.to_thread, and update process_file() to
await it. The sync API is preserved so existing callers that already
run inside run_in_threadpool (e.g. save_docs_to_vector_db) are
unaffected.

https://claude.ai/code/session_01JSr4NZSskEUQvoJnavVXh8

Co-authored-by: Claude <noreply@anthropic.com>
2026-04-14 10:55:46 -05:00
Timothy Jaeryang Baek 4866bec0f2 refac 2026-04-14 10:55:11 -05:00
Classic298 804f9f3153 fix(retrieval): offload sync VECTOR_DB_CLIENT calls in async paths via AsyncVectorDBClient (#23706)
* fix(retrieval): offload sync VECTOR_DB_CLIENT calls in async paths via AsyncVectorDBClient

The vector DB backends (Chroma, pgvector, Qdrant, Milvus, Pinecone,
Weaviate, …) are uniformly synchronous and their methods perform
blocking network or disk I/O. Multiple async route handlers and helpers
were calling them directly on the event loop — file processing,
memories, knowledge bases, hybrid search bookkeeping — so a single
upsert/delete/search would freeze every other in-flight request for the
duration of the call.

Introduce `AsyncVectorDBClient`, a thin async facade that wraps the
existing sync client and dispatches each method through
`asyncio.to_thread`. It mirrors `VectorDBBase` exactly and forwards
*args/**kwargs so backend-specific extra parameters keep working.

Update every async-context call site (routers/retrieval, routers/files,
routers/memories, routers/knowledge, retrieval/utils,
tools/builtin) to await `ASYNC_VECTOR_DB_CLIENT` instead of calling the
sync client directly. Two helpers that were sync-only also acquire
async siblings or are awaited via `asyncio.to_thread` at their async
call site (`remove_knowledge_base_metadata_embedding`,
`get_all_items_from_collections`, `query_doc`).

The original sync `VECTOR_DB_CLIENT` is unchanged, so callers that
already run inside `run_in_threadpool` (e.g. `save_docs_to_vector_db`
and the sync `query_doc`/`get_doc` helpers) are unaffected.

https://claude.ai/code/session_01JSr4NZSskEUQvoJnavVXh8

* fix(retrieval): restore explicit AsyncVectorDBClient signatures matching VectorDBBase

Per PR review: the original *args/**kwargs forwarding lost type
safety and IDE/static-analysis support. Restore explicit signatures
that mirror VectorDBBase exactly, so:

  * Bad kwargs fail at the facade boundary instead of inside the
    worker thread (where the resulting TypeError tends to be
    swallowed by surrounding `try/except`).
  * IDE autocomplete and static analysis work as expected.
  * The stated intent ("mirror VectorDBBase exactly") now holds at
    the API contract level, not just behaviourally.

While doing this, surface a pre-existing bug in
`delete_entries_from_collection` that the stricter typing flagged:
the call passed `metadata={'hash': hash}` which is not a parameter
on `VectorDBBase.delete` nor any backend. The TypeError raised
inside the sync delete was silently swallowed by `except Exception`
so the endpoint always reported `{'status': False}` for every
request instead of actually deleting matching vectors. Replace with
`filter=...` to do what the endpoint name promises.

The thorough review's other note (no concurrency/backpressure on
the shared default threadpool) is intentionally not addressed here:
asyncio.to_thread on the shared executor is the right primitive for
this use case; per-domain bounded executors would add lifecycle
complexity disproportionate to the problem and the loop is no
longer blocked, which was the actual bug.

https://claude.ai/code/session_01JSr4NZSskEUQvoJnavVXh8

* fix(retrieval): parallelize hybrid-search collection prefetch; document async facade contracts

Address PR review findings:

1. Hybrid-search prefetch was sequential
   `query_collection_with_hybrid_search` previously awaited
   `ASYNC_VECTOR_DB_CLIENT.get(name)` once per collection in a for
   loop. Each call already off-loaded to a worker thread, but
   awaiting them serially meant total prefetch latency scaled
   linearly with the number of collections. Run them concurrently
   with `asyncio.gather` so multi-collection queries actually
   benefit from the threadpool. Per-collection exception handling
   is preserved by wrapping each fetch in a small helper that
   logs and returns `(name, None)` on failure, so a single bad
   collection cannot poison the whole gather.

2. Document the thread-safety expectation explicitly
   The facade now formally states what was always implicit: the
   sync `VECTOR_DB_CLIENT` is shared across worker threads, so the
   underlying backend driver must be thread-safe. This is not a
   new exposure — `save_docs_to_vector_db` already called the sync
   client from `run_in_threadpool`. Adding a global lock here
   would defeat the responsiveness the facade exists to provide;
   backends that cannot tolerate concurrent access should grow
   their own internal serialization.

3. Document the API-surface choice and `.sync` escape hatch
   The strict `VectorDBBase` mirror was a deliberate choice (the
   previous `*args/**kwargs` revision let a `metadata=` typo
   silently break an endpoint). Document it, and call out the
   `.sync` escape hatch with an example for callers that genuinely
   need a backend-specific parameter not on `VectorDBBase`.

https://claude.ai/code/session_01JSr4NZSskEUQvoJnavVXh8

* fix(retrieval): guard /delete against null file.hash and let HTTPException reach the client

Address PR review finding on the `metadata=` → `filter=` change in
`delete_entries_from_collection`.

The new `filter={'hash': hash}` query was correct for files that
have a hash, but did not handle `file.hash is None` (unprocessed,
failed, or legacy records). The match semantics of a null filter
value are backend-dependent — some ignore the key entirely, some
treat it as "metadata field absent" and match every such row — so
issuing the query risked deleting unrelated entries.

  * Reject `hash is None` up front with a 400 explaining the file
    has no hash to target.

  * Narrow the surrounding `except Exception` so it no longer
    swallows `HTTPException`. Without this fix the new 400 (and the
    pre-existing 404 for missing files) would be silently re-shaped
    into `{'status': False}` and the caller could not distinguish a
    bad-request input from a backend error.

https://claude.ai/code/session_01JSr4NZSskEUQvoJnavVXh8

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-04-14 10:50:18 -05:00
Classic298 ee28032fb9 fix(middleware): replace BaseHTTPMiddleware HTTP middlewares with pure ASGI implementations (#23709)
* fix(middleware): replace BaseHTTPMiddleware HTTP middlewares with pure ASGI implementations

Starlette's BaseHTTPMiddleware (and the @app.middleware('http')
decorator that uses it) wraps the downstream app in an anyio task
group whose cancel scope tears down the inner task on every exit —
client disconnect, response complete, or any outer middleware bailing.
That CancelledError gets injected into whatever the inner task was
awaiting, so DB queries, embedding calls, and other long awaits get
killed mid-flight. Under aiosqlite the cleanup path then logs a
multi-page `terminate_force_close() not implemented` traceback at
ERROR for every cancelled DB call.

Open WebUI had four such middlewares stacked
(`commit_session_after_request`, `check_url`, `inspect_websocket`,
`RedirectMiddleware`) so a single cancellation would compound through
all four.

Move the four middlewares to a new `open_webui.utils.asgi_middleware`
module as plain ASGI classes (`__call__(scope, receive, send)`):

  * `CommitSessionMiddleware`   — was `commit_session_after_request`;
                                  now also rolls back if commit fails
                                  before releasing the connection.
  * `AuthTokenMiddleware`       — was `check_url`; sets request.state
                                  token + enable_api_keys + stamps
                                  X-Process-Time via a wrapped send.
  * `WebsocketUpgradeGuardMiddleware`
                                — was `inspect_websocket`; rejects
                                  /ws/socket.io HTTP requests that
                                  claim transport=websocket without a
                                  proper Upgrade/Connection header.
  * `RedirectMiddleware`        — was the BaseHTTPMiddleware subclass;
                                  same /watch + share-target rewrites.

Pure ASGI does not introduce a cancel scope around the downstream app,
so client disconnects propagate via `receive()` (the way ASGI was
designed) instead of being injected as CancelledError. Middleware
ordering is preserved.

https://claude.ai/code/session_01JSr4NZSskEUQvoJnavVXh8

* fix(middleware): CommitSessionMiddleware — rollback on downstream error, never commit failed requests

The first cut put commit() in a finally block, which meant that even
when a downstream handler raised, the middleware would still commit
whatever partial sync writes that handler had made before the
failure. That regressed the previous BaseHTTPMiddleware semantics
where commit only ran on the success path.

Restructure the failure handling:

* Downstream raised → rollback any pending sync work, release the
  connection, re-raise so the outer error middleware turns it into
  an error response. We never commit a request that did not complete.
* Downstream returned → commit. On commit failure, log loudly,
  rollback, and re-raise. ScopedSession.remove() always runs in
  finally so the connection cannot leak.

Document the inherent pure-ASGI limitation explicitly: by the time
`await self.app(...)` returns the response messages have already
been emitted, so a commit failure can no longer change what the
client sees on the wire. Buffering the response to gate it on commit
success would break streaming responses (chat completions, SSE) which
are core to Open WebUI; the trade-off is intentional. Routes that
need commit-before-send must manage the sync session explicitly.

Also drop unused `typing` imports flagged by review.

https://claude.ai/code/session_01JSr4NZSskEUQvoJnavVXh8

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-04-14 10:47:48 -05:00
Timothy Jaeryang Baek 37658fd541 refac 2026-04-14 01:17:39 -05:00
Timothy Jaeryang Baek cced77b584 refac 2026-04-14 00:07:50 -05:00
Timothy Jaeryang Baek 45e49d33e5 refac 2026-04-13 21:52:19 -05:00
Timothy Jaeryang Baek cf4218e688 refac 2026-04-13 21:29:03 -05:00
Algorithm5838 33a4d1b412 fix: image url to base64 conversion (#23685) 2026-04-13 19:15:22 -05:00
Timothy Jaeryang Baek c767bcaa73 refac 2026-04-13 18:20:46 -05:00
Timothy Jaeryang Baek 715cf9797a refac 2026-04-13 16:25:44 -05:00
Classic298 8979987eed fix: drop extra='allow' on FolderForm and FolderUpdateForm (#23648)
* fix: drop extra='allow' on FolderForm and FolderUpdateForm

These request models were configured to accept arbitrary extra fields,
which were then merged into the folder row via form_data.model_dump().
In insert_new_folder the server-assigned user_id is placed before the
form spread, so a client-supplied user_id in the request body would
override it and the folder would be persisted against another account.

Strictly typed inputs are the correct shape for these endpoints — the
client has no legitimate reason to send fields beyond the declared
ones, and dropping extra='allow' closes the mass-assignment sink at
the validation layer instead of relying on every callsite to merge
fields in the right order.

* fix: reject unknown fields on FolderForm and FolderUpdateForm

Address review feedback: dropping extra='allow' fell back to Pydantic
v2's default extra='ignore', which only silently drops unknown fields
instead of rejecting them. The intent for these request models is a
strict input contract — fail fast when a client sends anything the
server does not expect — so explicitly set extra='forbid'. This also
makes the hardening visible in the form definition rather than implicit
in the default.
2026-04-13 16:14:00 -05:00
Timothy Jaeryang Baek 8dba798cce refac 2026-04-13 16:03:36 -05:00
Timothy Jaeryang Baek 611fe0c8a9 refac 2026-04-13 15:14:55 -05:00
Timothy Jaeryang Baek 31406caa79 refac 2026-04-13 15:13:14 -05:00
Timothy Jaeryang Baek 9c64d84ad9 refac 2026-04-13 15:03:22 -05:00
Timothy Jaeryang Baek 40f5b3d135 refac 2026-04-13 14:51:09 -05:00
Timothy Jaeryang Baek 869cf9e848 refac 2026-04-13 14:33:23 -05:00
Timothy Jaeryang Baek 2ddcb30b9a refac 2026-04-13 14:29:27 -05:00
Timothy Jaeryang Baek 96265cf042 refac 2026-04-13 14:19:15 -05:00
Timothy Jaeryang Baek 050c4b97a9 refac 2026-04-13 14:13:03 -05:00
Timothy Jaeryang Baek d0188f3fe1 refac 2026-04-13 14:08:58 -05:00
Timothy Jaeryang Baek 8936721414 refac 2026-04-13 13:44:44 -05:00
Timothy Jaeryang Baek d1a0fbe292 refac 2026-04-13 13:36:54 -05:00
Timothy Jaeryang Baek 22cfb3c673 refac 2026-04-13 13:26:13 -05:00
Timothy Jaeryang Baek 51765b619c refac 2026-04-13 13:13:45 -05:00
Timothy Jaeryang Baek 20544d412e chore: format 2026-04-12 22:11:10 -05:00
Timothy Jaeryang Baek cb6e77be3e refac 2026-04-12 22:10:43 -05:00
Timothy Jaeryang Baek d4b90f93bd refac 2026-04-12 22:08:27 -05:00
Timothy Jaeryang Baek 26b8ca5b5e refac 2026-04-12 19:41:02 -05:00
Timothy Jaeryang Baek 57784706e4 refac 2026-04-12 19:34:45 -05:00
Timothy Jaeryang Baek 8172c7e3d5 refac 2026-04-12 19:08:30 -05:00
Timothy Jaeryang Baek 498ff8cdc3 refac 2026-04-12 19:05:25 -05:00
Timothy Jaeryang Baek facb194a07 refac 2026-04-12 19:02:51 -05:00
Timothy Jaeryang Baek c3c8c605d7 refac 2026-04-12 18:56:04 -05:00
Timothy Jaeryang Baek 3c2c611ba9 refac 2026-04-12 18:49:34 -05:00
Timothy Jaeryang Baek a359262616 refac 2026-04-12 18:48:06 -05:00
Classic298 d59b933bf2 fix: apply REDIS_KEY_PREFIX to tool_servers and terminal_servers cache keys (#23649)
These four Redis calls were the only places in the backend still using
bare cache keys. Every other call already namespaces through
REDIS_KEY_PREFIX, which is how multiple Open WebUI instances sharing a
Redis database stay isolated. Two instances with different
TOOL_SERVER_CONNECTIONS or TERMINAL_SERVER_CONNECTIONS would otherwise
clobber each other's cached OpenAPI specs.
2026-04-12 18:29:21 -05:00
Timothy Jaeryang Baek 25898116ea chore: format 2026-04-12 18:12:59 -05:00
Classic298 4292358bd5 feat: log provider errors to console for better insights (#23379)
* fix: log provider errors that were silently swallowed

* Update main.py

* fix: wrap non-JSON SSE error responses in JSON so middleware handles them
2026-04-12 18:07:20 -05:00
Classic298 67023037f8 fix: replace brittle profile_image_url allowlist with safe-scheme validation (#23389)
* fix: replace brittle profile_image_url allowlist with safe-scheme validation

The previous validation used a hardcoded allowlist of specific static
paths and a single Gravatar prefix. This rejected OWUI's own internal
API paths (e.g. /api/v1/users/{id}/profile/image) and external OAuth
avatar URLs, making it impossible to save user profiles from the admin
panel.

Replace with scheme-based validation that allows relative paths,
HTTP(S) URLs, and data:image URIs while blocking dangerous schemes
like javascript:, file:, and ftp:.

Fixes open-webui#23387

* fix: harden profile image URL validation per review feedback

- Restrict data URIs to safe raster formats (png/jpeg/gif/webp);
  SVG is excluded because it can carry embedded scripts.
- Block scheme-relative URLs (//host/path) which browsers resolve
  against the current protocol, bypassing the relative-path check.

* fix: use structural validation instead of prefix checks

- Use urlparse for HTTP(S) URLs: gives case-insensitive scheme
  matching and rejects bare schemes with no host (e.g. https://).
- Use a compiled regex for data URIs: enforces the ;base64, boundary,
  restricts to safe raster formats, and is case-insensitive per spec.
- Removes the startswith-based prefix tuple in favour of proper
  URL and data URI parsing.

* fix: validate hostname not netloc, fix misleading comment

- Use parsed.hostname instead of parsed.netloc so URLs like
  http://:80/path (non-empty netloc but no actual host) are rejected.
- Update data URI comment to accurately state we validate MIME type
  and structure, not base64 payload integrity.

* fix: constrain relative paths to known-safe prefixes

Accepting any relative path starting with / allowed a user to set
their profile_image_url to an arbitrary internal GET endpoint. When
another user (e.g. an admin) views that profile, the browser fires
the GET with the viewer's session cookies — an authenticated GET
trigger surface.

Constrain to known-safe prefixes (/api/v1/users/, /static/) and
exact matches (/user.png, /favicon.png) which are the only relative
paths OWUI itself generates.

* fix: use exact matches and anchored regex, eliminate all prefix wildcarding

Replace all startswith-based path checks with:
- frozenset exact matches for static assets (/user.png, /favicon.png,
  /static/favicon.png)
- Anchored regex for the OWUI profile image API route that accepts
  only /api/v1/users/{id}/profile/image (no trailing components,
  no path traversal across segments)

This eliminates every prefix-based attack surface:
- /api/v1/users/{id}/anything-else is rejected
- /static/../../etc/passwd is rejected
- /api/v1/users/../../admin/config is rejected
- Arbitrary internal GET triggers are no longer possible

* fix: exclude query/fragment delimiters from user-ID regex segment

Change [^/]+ to [^/?#]+ so that inputs like
/api/v1/users/alice?x=1/profile/image are rejected — the browser
would interpret ? as the query string start, making the actual
request target /api/v1/users/alice instead of the intended route.
2026-04-12 17:57:49 -05:00
Classic298 e7ff4768f8 fix: Add ownership checks to global task endpoints (#23454)
* Add ownership checks to global task endpoints

- Restrict GET /api/tasks and POST /api/tasks/stop/{task_id} to admin-only
- Add new scoped POST /api/tasks/chat/{chat_id}/stop endpoint with ownership
  check so regular users can stop their own chat tasks
- Allow admins to access the scoped chat task endpoints alongside owners
- Update frontend to use the new scoped stop endpoint when a chatId is available

https://claude.ai/code/session_01K7zPDvvjRu8AxJ4Br2HhZc

* Handle temporary (local:) chat IDs in scoped task endpoints

Temporary chats use local:<socketId> as chat_id which doesn't exist in
the DB. The scoped endpoints now skip ownership checks for local: IDs
(they aren't enumerable) and use {chat_id:path} to handle the colon in
the URL path.

https://claude.ai/code/session_01K7zPDvvjRu8AxJ4Br2HhZc

* Verify session ownership for local: chat IDs and URL-encode chat_id

- For local:<socketId> chat IDs, look up the socket's owner in
  SESSION_POOL and verify it matches the requesting user (or admin)
- URL-encode chat_id in frontend fetch calls to handle special
  characters (colon in local: IDs) safely

https://claude.ai/code/session_01K7zPDvvjRu8AxJ4Br2HhZc

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-04-12 17:56:43 -05:00
Timothy Jaeryang Baek c47dd7b771 refac 2026-04-12 17:22:06 -05:00
Timothy Jaeryang Baek 4498e6faf2 refac 2026-04-12 16:59:05 -05:00
Timothy Jaeryang Baek 47d413ce7b refac 2026-04-12 16:47:23 -05:00
Classic298 0753409e7b fix: use ipaddress stdlib for IPv6 SSRF protection (#23453)
The validators.ipv6(ip, private=True) call always returns a falsy ValidationError because validators==0.35.0 does not support the private kwarg for IPv6. This means any hostname resolving to a private IPv6 address (::1, fd00::*, ::ffff:169.254.169.254) bypasses SSRF protection entirely, circumventing the fix for CVE-2025-65958.

Replace both the IPv4 and IPv6 validators-based private checks with Python's stdlib ipaddress module using an allowlist approach (not addr.is_global). This blocks all non-globally-routable addresses — private, loopback, link-local, reserved, multicast, and unspecified — for both IPv4 and IPv6, including IPv4-mapped IPv6 addresses.
2026-04-12 16:34:13 -05:00
Classic298 b78dabb442 fix: reject empty passwords in LDAP authentication to prevent unauthenticated binds (#23633)
Per RFC 4513, a Simple Bind with a non-empty DN but empty password is unauthenticated simple authentication. Many LDAP servers (OpenLDAP default, some AD configs) accept these binds, allowing account takeover without valid credentials.

Rejects empty and whitespace-only passwords before attempting the LDAP bind.
2026-04-12 16:33:57 -05:00