Conversation
Greptile SummaryThis PR resolves managed vector-store registry settings for RAG queries and forwards provider-specific retrieval parameters into vector-store searches.
Confidence Score: 4/5This PR should not merge until client-supplied retrieval settings are safety-validated before being forwarded to vector-store providers. The registry credential fix works for its tested case, but the same broad forwarding path permits authenticated callers to influence provider connection and authentication settings. Files Needing Attention: litellm/rag/main.py and litellm/proxy/rag_endpoints/endpoints.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/rag_endpoints/endpoints.py | Resolves managed vector-store registry configuration before authorization scanning and the RAG query call. |
| litellm/rag/main.py | Forwards provider-specific retrieval fields into vector-store search, but currently also forwards unsafe client-controlled connection settings. |
| tests/test_litellm/proxy/rag_endpoints/test_rag_endpoints.py | Adds endpoint coverage proving managed registry credentials reach vector-store search, alongside formatting-only changes. |
| tests/test_litellm/rag/test_main.py | Adds core-pipeline coverage for forwarding provider credentials to vector-store search. |
Reviews (1): Last reviewed commit: "fix(rag): resolve vector_store_registry ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| extra_search_params = {k: v for k, v in retrieval_config.items() if k not in _reserved_retrieval_keys} | ||
| search_params = {**extra_search_params, **kwargs} | ||
| for reserved in ("vector_store_id", "query", "max_num_results", "custom_llm_provider"): | ||
| search_params.pop(reserved, None) |
There was a problem hiding this comment.
When an authenticated caller includes provider connection or credential fields in retrieval_config, this code forwards them to litellm.vector_stores.asearch without the proxy's existing request-safety validation, allowing the caller to alter the vector-store search's outbound connection or authentication settings. How this was verified: The query path preserves unmatched caller fields during registry merging and passes every non-reserved retrieval field to providers that consume those connection settings.
Rule Used: What: For security fix PRs, NEVER mention what sec... (source)
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
|
|
||
| # 2. Search vector store | ||
| _reserved_retrieval_keys = {"vector_store_id", "top_k", "custom_llm_provider"} | ||
| extra_search_params = {k: v for k, v in retrieval_config.items() if k not in _reserved_retrieval_keys} |
There was a problem hiding this comment.
Medium: Client-controlled endpoint receives proxy credentials
An authenticated caller can supply fields such as api_base through retrieval_config. For example, selecting vertex_ai/search_api with an attacker-controlled api_base makes the provider obtain the proxy's configured Vertex access token and send it in the Authorization header to that URL. Validate the original client payload before registry resolution and reject client-supplied credential and endpoint fields, while continuing to forward credentials added by the server-side registry.
| status_code=400, | ||
| detail={"error": "retrieval_config must contain 'vector_store_id'"}, | ||
| ) | ||
| retrieval_config = await _update_request_data_with_litellm_managed_vector_store_registry( |
There was a problem hiding this comment.
Low: Resolved credentials are written to debug logs
This helper adds cleartext fields such as api_key to retrieval_config, which line 674 then logs verbatim. Anyone with access to proxy debug logs can recover the vector-store credentials after a query; log only non-sensitive retrieval fields or redact credential values before logging.
PR overviewThis pull request updates the Two security issues remain open. An authenticated caller can direct a provider request to an attacker-controlled endpoint and receive the proxy’s configured access token, creating a concrete credential-exfiltration path. Resolved vector-store credentials may also be exposed in debug logs to anyone with log access, and no issues have yet been addressed. Open issues (2)
Fixed/addressed: 0 · PR risk: 7/10 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
620fac2 to
5b02f73
Compare
|
Fixed? |
3 participants
TLDR
Problem this solves:
How it solves it:
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes
QA runbook
Final Attestation