mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
chore(vector-stores): also gate /vector_store/update; upstream credentials plural in masker
Two architectural extensions to the credential-redaction in the previous
commit:
1. ``/vector_store/update`` had two gaps:
- No per-store access control. Any authenticated principal that
passed the premium-feature gate could mutate *any* vector store,
including stores belonging to other teams.
- The response returned the full DB row including ``litellm_params``,
so the caller could read another team's persisted provider
credentials by submitting a no-op metadata change.
Mirror the access-control check ``/vector_store/info`` already
performs (``_check_vector_store_access`` against the existing row),
redact ``litellm_params`` in the response, and add an
``except HTTPException: raise`` guard so the 403/404 responses don't
get rewritten as 500 by the catch-all.
2. ``SensitiveDataMasker``'s default ``sensitive_patterns`` set used
segment-exact matching, so ``credential`` matched ``vertex_credential``
but not ``vertex_credentials`` (the actual Vertex field name). The
previous commit worked around this with a per-call extension; this
commit puts the plural in the upstream defaults so every caller
(Redis config dump, MCP debug headers, cache routes, ...) gets the
correct behavior. The local override in
``vector_store_endpoints/management_endpoints.py`` is removed.
Also updates ``test_excluded_keys_exact_match`` which relied on
``credentials`` *not* being a sensitive pattern to demonstrate
case-sensitive ``excluded_keys`` matching. The intent of the test
(case-sensitive match) is preserved; the assertion now reflects that
when ``excluded_keys`` fails to apply (wrong case), the field falls
through to standard pattern-based masking instead of being passed
through unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
a99943ec49
commit
51d560ba2e
3 changed files with 167 additions and 7 deletions
|
|
@ -21,6 +21,8 @@ class SensitiveDataMasker:
|
|||
"auth",
|
||||
"authorization",
|
||||
"credential",
|
||||
# Plural form: Vertex uses ``vertex_credentials``; segment-exact
|
||||
# matching otherwise misses it because "credential" != "credentials".
|
||||
"credentials",
|
||||
"access",
|
||||
"private",
|
||||
|
|
|
|||
|
|
@ -40,12 +40,7 @@ from litellm.vector_stores.vector_store_registry import VectorStoreRegistry
|
|||
|
||||
router = APIRouter()
|
||||
|
||||
# Inherit the default sensitive-key heuristics and add the plural
|
||||
# ``credentials`` so segment-exact matching catches Vertex's
|
||||
# ``vertex_credentials`` (the singular ``credential`` pattern misses it).
|
||||
_LITELLM_PARAMS_MASKER = SensitiveDataMasker(
|
||||
sensitive_patterns={*SensitiveDataMasker().sensitive_patterns, "credentials"},
|
||||
)
|
||||
_LITELLM_PARAMS_MASKER = SensitiveDataMasker()
|
||||
|
||||
|
||||
def _redact_sensitive_litellm_params(
|
||||
|
|
@ -812,6 +807,25 @@ async def update_vector_store(
|
|||
update_data = data.model_dump(exclude_unset=True)
|
||||
vector_store_id = update_data.pop("vector_store_id")
|
||||
|
||||
# Per-store access control: anyone authenticated who passes the
|
||||
# premium-feature gate could otherwise update *any* vector store —
|
||||
# including stores belonging to other teams. Mirror the check
|
||||
# ``/vector_store/info`` already performs.
|
||||
existing = await prisma_client.db.litellm_managedvectorstorestable.find_unique(
|
||||
where={"vector_store_id": vector_store_id}
|
||||
)
|
||||
if existing is None:
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
detail=f"Vector store with ID {vector_store_id} not found",
|
||||
)
|
||||
existing_typed = LiteLLM_ManagedVectorStore(**existing.model_dump())
|
||||
if not await _check_vector_store_access(existing_typed, user_api_key_dict):
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail="Access denied: You do not have permission to update this vector store",
|
||||
)
|
||||
|
||||
# Handle metadata serialization
|
||||
if update_data.get("vector_store_metadata") is not None:
|
||||
update_data["vector_store_metadata"] = safe_dumps(
|
||||
|
|
@ -859,11 +873,24 @@ async def update_vector_store(
|
|||
f"Updated vector store {vector_store_id} in both database and in-memory registry"
|
||||
)
|
||||
|
||||
# The DB row is returned in full, so the response would otherwise
|
||||
# echo the persisted ``litellm_params`` (including provider
|
||||
# credentials) back to the caller — even when the caller only
|
||||
# changed unrelated fields like ``vector_store_description``.
|
||||
response_vs = LiteLLM_ManagedVectorStore(**updated_vs)
|
||||
response_vs["litellm_params"] = _redact_sensitive_litellm_params(
|
||||
updated_vs.get("litellm_params")
|
||||
)
|
||||
return {
|
||||
"status": "success",
|
||||
"message": f"Vector store {vector_store_id} updated successfully",
|
||||
"vector_store": updated_vs,
|
||||
"vector_store": response_vs,
|
||||
}
|
||||
except HTTPException:
|
||||
# Preserve 403/404 responses from the access-control / not-found
|
||||
# checks above; the catch-all below would otherwise rewrite them
|
||||
# as 500 with the original status code embedded in the detail.
|
||||
raise
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.exception(f"Error updating vector store: {str(e)}")
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
|
|
|
|||
|
|
@ -1950,3 +1950,134 @@ class TestRedactSensitiveLitellmParams:
|
|||
snapshot = dict(original)
|
||||
_redact_sensitive_litellm_params(original)
|
||||
assert original == snapshot, "input dict must not be mutated"
|
||||
|
||||
|
||||
class TestUpdateVectorStoreAccessControlAndRedaction:
|
||||
"""
|
||||
``/vector_store/update`` previously skipped per-store access control
|
||||
(only the premium-feature gate ran), letting any authenticated
|
||||
premium principal mutate *any* vector store. It also returned the
|
||||
full DB row including ``litellm_params``, leaking provider
|
||||
credentials to the caller. Both are fixed at the endpoint level.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_denied_when_caller_cannot_access_store(self):
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
from litellm.proxy.vector_store_endpoints.management_endpoints import (
|
||||
update_vector_store,
|
||||
)
|
||||
from litellm.types.vector_stores import VectorStoreUpdateRequest
|
||||
|
||||
existing_row = MagicMock()
|
||||
existing_row.model_dump = MagicMock(
|
||||
return_value={
|
||||
"vector_store_id": "vs_other_team",
|
||||
"team_id": "team-A",
|
||||
"litellm_params": {"api_key": "sk-team-A-secret"},
|
||||
}
|
||||
)
|
||||
|
||||
mock_prisma_client = MagicMock()
|
||||
mock_prisma_client.db.litellm_managedvectorstorestable.find_unique = AsyncMock(
|
||||
return_value=existing_row
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy.vector_store_endpoints.management_endpoints.check_feature_access_for_user",
|
||||
new_callable=AsyncMock,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.vector_store_endpoints.management_endpoints._check_vector_store_access",
|
||||
new_callable=AsyncMock,
|
||||
return_value=False,
|
||||
),
|
||||
patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client),
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await update_vector_store(
|
||||
data=VectorStoreUpdateRequest(
|
||||
vector_store_id="vs_other_team",
|
||||
vector_store_description="hijacked",
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(
|
||||
user_id="attacker", team_id="team-B"
|
||||
),
|
||||
)
|
||||
assert exc_info.value.status_code == 403
|
||||
# The attacker must NOT see the existing credential in the
|
||||
# error message either.
|
||||
assert "sk-team-A-secret" not in str(exc_info.value.detail)
|
||||
# And the DB update must not have been called.
|
||||
mock_prisma_client.db.litellm_managedvectorstorestable.update.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_response_redacts_litellm_params(self):
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
from litellm.constants import REDACTED_BY_LITELM_STRING
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
from litellm.proxy.vector_store_endpoints.management_endpoints import (
|
||||
update_vector_store,
|
||||
)
|
||||
from litellm.types.vector_stores import VectorStoreUpdateRequest
|
||||
|
||||
existing_row = MagicMock()
|
||||
existing_row.model_dump = MagicMock(
|
||||
return_value={
|
||||
"vector_store_id": "vs_owned",
|
||||
"team_id": "team-A",
|
||||
"litellm_params": {
|
||||
"api_key": "sk-real-openai-key-123",
|
||||
"api_base": "https://api.openai.com/v1",
|
||||
},
|
||||
}
|
||||
)
|
||||
updated_row = MagicMock()
|
||||
updated_row.model_dump = MagicMock(
|
||||
return_value={
|
||||
"vector_store_id": "vs_owned",
|
||||
"team_id": "team-A",
|
||||
"vector_store_description": "new desc",
|
||||
"litellm_params": {
|
||||
"api_key": "sk-real-openai-key-123",
|
||||
"api_base": "https://api.openai.com/v1",
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
mock_prisma_client = MagicMock()
|
||||
mock_prisma_client.db.litellm_managedvectorstorestable.find_unique = AsyncMock(
|
||||
return_value=existing_row
|
||||
)
|
||||
mock_prisma_client.db.litellm_managedvectorstorestable.update = AsyncMock(
|
||||
return_value=updated_row
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy.vector_store_endpoints.management_endpoints.check_feature_access_for_user",
|
||||
new_callable=AsyncMock,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.vector_store_endpoints.management_endpoints._check_vector_store_access",
|
||||
new_callable=AsyncMock,
|
||||
return_value=True,
|
||||
),
|
||||
patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client),
|
||||
patch("litellm.vector_store_registry", None),
|
||||
):
|
||||
response = await update_vector_store(
|
||||
data=VectorStoreUpdateRequest(
|
||||
vector_store_id="vs_owned",
|
||||
vector_store_description="new desc",
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(user_id="owner", team_id="team-A"),
|
||||
)
|
||||
|
||||
params = response["vector_store"]["litellm_params"]
|
||||
assert params["api_key"] == REDACTED_BY_LITELM_STRING
|
||||
assert params["api_base"] == "https://api.openai.com/v1"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue