-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix: backport #27892 to litellm_1.84.0rc2 #27903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| from litellm.litellm_core_utils.url_utils import SSRFError, validate_url | ||
| from litellm.proxy._types import * | ||
| from litellm.types.router import CONFIGURABLE_CLIENTSIDE_AUTH_PARAMS | ||
| from litellm.types.utils import CustomPricingLiteLLMParams | ||
|
|
||
|
|
||
| def _get_request_ip_address( | ||
|
|
@@ -276,6 +277,7 @@ def _build_banned_observability_params() -> FrozenSet[str]: | |
| # integrations are covered automatically. Sorted for stable iteration | ||
| # order and reviewable diffs. | ||
| *sorted(_build_banned_observability_params()), | ||
| *sorted(CustomPricingLiteLLMParams.model_fields.keys()), | ||
| ) | ||
|
Comment on lines
279
to
281
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,3 +128,66 @@ def test_internal_user_rag_ingest_without_vector_store_id_allowed(client_interna | |
| f"internal_user should be allowed to create new vector stores. " | ||
| f"Response: {response.json()}" | ||
| ) | ||
|
|
||
|
|
||
| class TestRagIngestSSRFBlocked: | ||
| """ | ||
| aws_sts_endpoint and related credential-redirect fields must be rejected | ||
| in ingest_options.vector_store. Without this guard, any authenticated | ||
| client can coerce the proxy to make a signed STS AssumeRole call to an | ||
| attacker-controlled server, leaking the instance profile credentials. | ||
| """ | ||
|
|
||
| @pytest.mark.parametrize( | ||
| "field,value", | ||
| [ | ||
| ("aws_sts_endpoint", "https://attacker.example/sts"), | ||
| ("aws_web_identity_token", "fake-token"), | ||
| ("aws_bedrock_runtime_endpoint", "https://attacker.example/bedrock"), | ||
| ], | ||
| ) | ||
| def test_ssrf_field_in_vector_store_config_rejected( | ||
| self, field, value, client_internal_user | ||
| ): | ||
| payload = { | ||
| "file_url": "https://example.com/doc.pdf", | ||
| "ingest_options": { | ||
| "vector_store": { | ||
| "custom_llm_provider": "bedrock", | ||
| field: value, | ||
| } | ||
| }, | ||
| } | ||
| response = client_internal_user.post( | ||
| "/v1/rag/ingest", | ||
| json=payload, | ||
| ) | ||
| assert response.status_code == 400, ( | ||
| f"{field} in ingest_options.vector_store should be rejected (400), " | ||
| f"got {response.status_code}: {response.json()}" | ||
| ) | ||
| body = response.json() | ||
| detail = body.get("detail", {}) | ||
| error_text = ( | ||
| detail.get("error", "") if isinstance(detail, dict) else str(detail) | ||
| ) | ||
| assert field in error_text, f"Error should name the offending field: {error_text}" | ||
|
|
||
| def test_clean_bedrock_ingest_options_not_rejected(self, client_internal_user): | ||
| with patch( | ||
| "litellm.proxy.rag_endpoints.endpoints.litellm.aingest", | ||
| new_callable=AsyncMock, | ||
| return_value={"vector_store_id": "vs_bedrock", "file_id": "file_123"}, | ||
| ): | ||
| response = client_internal_user.post( | ||
| "/v1/rag/ingest", | ||
| json={ | ||
| "file_url": "https://example.com/doc.pdf", | ||
| "ingest_options": { | ||
| "vector_store": {"custom_llm_provider": "bedrock"} | ||
| }, | ||
| }, | ||
| ) | ||
| assert response.status_code != 400, ( | ||
| f"Clean Bedrock ingest_options should not be rejected: {response.json()}" | ||
| ) | ||
|
Comment on lines
+191
to
+193
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High: Pricing overrides can be bypassed by the credentials opt-in
Adding custom pricing fields to
_BANNED_REQUEST_BODY_PARAMSmakes them subject to the existingallow_client_side_credentialsearly return in_check_banned_params. On deployments that enable client-side credentials, an authenticated client can sendinput_cost_per_tokenandoutput_cost_per_tokenin a completion request and update the shared model cost entry, letting them undercount or zero out spend for that model. Keep custom pricing in a separate deny list that is not bypassed byallow_client_side_credentials, or require a pricing-specific admin opt-in.