* fix: prevent redirect-based SSRF in get_image_base64_from_url
Cohort follow-up to PR #24491. That PR patched three call sites
(SafeWebBaseLoader._scrape, get_content_from_url, load_url_image) to
pass allow_redirects=False on the underlying HTTP client; this fourth
call site in utils/files.py was missed.
get_image_base64_from_url() is invoked from convert_url_images_to_base64
in utils/middleware.py on every /api/chat/completions request whose
message content includes an image_url part. validate_url() is called on
the originally-submitted URL only; the aiohttp session.get() call had
no allow_redirects argument and the shared session pool does not
override the aiohttp default (allow_redirects=True). An authenticated
user sending a chat message with image_url pointing at an attacker host
that 302-redirects to 169.254.169.254 / 127.0.0.1 / RFC1918 reached the
internal target. This is the most reachable variant in the redirect
cluster: no special endpoint, no admin permission, no feature flag.
Apply the same one-line fix as the other three call sites: pass
allow_redirects=AIOHTTP_CLIENT_ALLOW_REDIRECTS (defaults to False).
Reported by nayakchinmohan in GHSA-88jq-grjp-jx6f; consolidated under
GHSA-rh5x-h6pp-cjj6.
Co-authored-by: nayakchinmohan <nayakchinmohan@users.noreply.github.com>
* fix: enforce collection write access on process_file endpoint
Cohort follow-up to ba83613ff. That commit added _validate_collection_access
to process_text and process_web (the user-supplied collection_name path)
but missed process_file in the same router.
process_file accepts a user-supplied collection_name and writes the file's
embedded content into that collection via save_docs_to_vector_db. The
file_id is gated by file ownership (line 1562) but collection_name was
unchecked, so an authenticated user could append content from a file they
own into another user's knowledge-base collection by passing the victim's
KB UUID as collection_name. Identical pattern to the process_text and
process_web gaps that ba83613ff closed.
Apply the same one-line gate as the sibling endpoints: when
collection_name is user-supplied (not the default file-{file.id} fallback),
require write access via _validate_collection_access. The shared validator
delegates to filter_accessible_collections, which already correctly
handles file-* prefixes (via has_access_to_file) and KB UUIDs
(via Knowledges.check_access_by_user_id) — admins bypass.
Reported by tenbbughunters (Tenable) in GHSA-4g37-7p2c-38r9 (the
comprehensive write-path filing covering process_text / process_file /
process_web / process_youtube and the _validate_collection_access UUID
root cause), and independently re-identified for the missed process_file
call site by kodareef5 in GHSA-4m74-3cmc-293g.
Co-authored-by: tenbbughunters <tenbbughunters@users.noreply.github.com>
Co-authored-by: kodareef5 <kodareef5@users.noreply.github.com>
* fix: enforce collection write access on process_files_batch endpoint
Cohort follow-up to ba83613ff and the prior process_file fix on this
branch. process_files_batch (line 2604) is the third write endpoint in
the same router that accepts a user-supplied collection_name; it was
covered in the same Tenable filing as process_file and was missed by
the same cohort fix. The endpoint validates per-file ownership at line
2642 but does not check whether the caller has write access to the
target collection_name before save_docs_to_vector_db writes into it
at line 2683-2690 with add=True.
Apply the same one-line gate as the sibling endpoints. Validate only
when collection_name is user-supplied (truthy) so the existing fall
through behavior for the None case is unchanged.
Same Tenable / kodareef5 cohort as the previous commit.
Co-authored-by: tenbbughunters <tenbbughunters@users.noreply.github.com>
Co-authored-by: kodareef5 <kodareef5@users.noreply.github.com>
---------
Co-authored-by: nayakchinmohan <nayakchinmohan@users.noreply.github.com>
Co-authored-by: tenbbughunters <tenbbughunters@users.noreply.github.com>
Co-authored-by: kodareef5 <kodareef5@users.noreply.github.com>
The `get_status()` handler at retrieval.py:263 (`@router.get('/')`) returned
the live RAG pipeline configuration (CHUNK_SIZE, CHUNK_OVERLAP, RAG_TEMPLATE,
RAG_EMBEDDING_ENGINE, RAG_EMBEDDING_MODEL, RAG_RERANKING_MODEL, etc.) without
any authentication dependency, while every adjacent endpoint on the same
router (/embedding, /embedding/update, /config, /config/update) requires
get_admin_user.
Exhaustive search of the repository confirms the endpoint has no callers:
- Frontend (src/): no `RETRIEVAL_API_BASE_URL}/'`-style fetch; the existing
`getRAGConfig()` in src/lib/apis/retrieval/index.ts targets `/config`,
not the root, and is the only consumer of admin-level retrieval state.
- Backend self-references: none.
- Cypress e2e (chat, documents, registration, settings): none.
- Backend tests (backend/open_webui/test/): none.
- Build/CI scripts (scripts/): none.
- Direct symbol import of `get_status` from this router: none.
The endpoint is dead code, almost certainly a relic from before the
/config GET split. Removing it has zero UX impact and eliminates the
unauthenticated-config-disclosure surface raised in advisory triage on
GHSA-65pg-qhhw-mxwg. External monitoring scripts that may have hit the
bare root will receive a 404 and can switch to the existing /config
endpoint, which returns the same fields plus the rest of the RAG config
under admin auth.
Surface raised by 0xRyuzak1 in GHSA-65pg-qhhw-mxwg. The advisory was closed
as not-a-vulnerability per SECURITY.md Rule 1 (no security boundary
crossed in default config — RAG_TEMPLATE default is a citation-format
instruction, not a system prompt; no integrity/availability impact); this
removal is independent code-hygiene that aligns the router cohort.
Reported-by: 0xRyuzak1 <https://github.com/0xRyuzak1>
Add configurable reranker batch size (env var RAG_RERANKING_BATCH_SIZE,
default 32) following the same pattern as RAG_EMBEDDING_BATCH_SIZE.
- config.py: PersistentConfig for RAG_RERANKING_BATCH_SIZE
- main.py: import, state init, pass to get_reranking_function
- colbert.py: accept batch_size param in predict() (was hardcoded 32)
- utils.py: get_reranking_function passes batch_size at call time
- retrieval.py: expose in config GET/POST endpoints and ConfigForm
- Documents.svelte: add Reranking Batch Size input in admin settings
Closes#23730
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>
* 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>
Replace bare except clauses with except Exception to follow Python best practices and avoid catching unexpected system exceptions like KeyboardInterrupt and SystemExit.
* fix: enforce ownership check on user-memory collection queries
fix: enforce ownership check on user-memory collection queries
Prevent authenticated users from querying other users' memory
collections via the /query/doc and /query/collection endpoints.
A new _validate_collection_access helper rejects requests for
user-memory-{UUID} collections where the UUID does not match
the requesting user. Admins bypass the check.
* Update retrieval.py
* Update retrieval.py
* fix: prevent worker death during document upload by using run_coroutine_threadsafe
Replace asyncio.run() with asyncio.run_coroutine_threadsafe() in
save_docs_to_vector_db() to prevent uvicorn worker health check failures.
The issue: asyncio.run() creates a new event loop and blocks the thread
completely, preventing the worker from responding to health checks during
long-running embedding operations (>5 seconds default timeout).
The fix: Schedule the async embedding work on the main event loop using
run_coroutine_threadsafe(). This keeps the main loop responsive to health
check pings while the sync caller waits for the result.
Changes:
- main.py: Store main event loop reference in app.state.main_loop at startup
- retrieval.py: Use run_coroutine_threadsafe() instead of asyncio.run()
https://claude.ai/code/session_01UQSYvSTkXb57sFb7M85Kcw
* add env var
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix: remove invalid expunge call on Pydantic FileModel
Files.get_file_by_id() returns a Pydantic FileModel, not an SQLAlchemy
ORM object. Calling db.expunge() on a Pydantic model fails with
UnmappedInstanceError since it lacks _sa_instance_state.
The expunge was also unnecessary because subsequent DB updates already
use fresh sessions via get_db() context manager.
Fixes#20925
