mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(batch_rate_limiter): drop forgeable per-model skip to close quota bypass
The per-model skip matched skip_batch_input_file_rate_limiting_for_models against the model bound to the input file id. That model comes from decode_model_from_file_id / the unified file id, both unsigned base64 the caller fully controls, so a caller could re-encode an accessible provider file id with a skip-listed model while the JSONL still routes rate-limited body.model entries and bypass the batch RPM/TPM counters. The models a batch actually runs are its JSONL body.model entries, which cannot be known without reading the file, so no caller-influenced model identifier can safely gate a skip. Remove the per-model skip entirely. The provider skip stays because the provider is resolved from trusted deployment credentials and the batch is constrained to run on that provider; the global disable and no-applicable-limits skips stay because they do not depend on caller input.
This commit is contained in:
parent
00cd3ed5cb
commit
7a7e24b120
2 changed files with 34 additions and 53 deletions
|
|
@ -102,11 +102,9 @@ class _PROXY_BatchRateLimiter(CustomLogger):
|
|||
def _get_file_bound_batch_model(self, data: Dict) -> Optional[str]:
|
||||
"""Resolve the model bound to the batch input file ID.
|
||||
|
||||
The model embedded in a ``file-<base64>`` ID or a unified managed
|
||||
file's target model is fixed when the file is created, so it reflects
|
||||
the model the batch will actually run. Unlike the client-supplied
|
||||
top-level ``model`` field, it cannot be swapped per request to point a
|
||||
skip decision at a deployment the JSONL never routes to.
|
||||
Used only as a fallback routing model when the request omits a
|
||||
top-level ``model``; the provider is then read from that deployment's
|
||||
trusted credentials for the provider-level skip decision.
|
||||
"""
|
||||
input_file_id = data.get("input_file_id")
|
||||
if not isinstance(input_file_id, str) or not input_file_id:
|
||||
|
|
@ -168,16 +166,6 @@ class _PROXY_BatchRateLimiter(CustomLogger):
|
|||
provider = credentials.get("custom_llm_provider")
|
||||
return provider if isinstance(provider, str) and provider else None
|
||||
|
||||
def _matches_skip_list(self, value: str, skip_list: List[str]) -> bool:
|
||||
if not skip_list:
|
||||
return False
|
||||
for entry in skip_list:
|
||||
if not isinstance(entry, str) or not entry:
|
||||
continue
|
||||
if value == entry or value.startswith(f"{entry}/"):
|
||||
return True
|
||||
return False
|
||||
|
||||
def _create_batch_rate_limit_descriptors(
|
||||
self,
|
||||
user_api_key_dict: UserAPIKeyAuth,
|
||||
|
|
@ -197,21 +185,25 @@ class _PROXY_BatchRateLimiter(CustomLogger):
|
|||
user_api_key_dict: UserAPIKeyAuth,
|
||||
) -> Tuple[bool, Optional[List["RateLimitDescriptor"]]]:
|
||||
"""
|
||||
Skip downloading batch input files when configured or when there is
|
||||
nothing to enforce (no applicable rate limits and no model allowlist).
|
||||
Skip downloading batch input files when the operator disabled batch
|
||||
input-file rate limiting, when the batch runs entirely on a skip-listed
|
||||
provider, or when there is nothing to enforce (no applicable rate
|
||||
limits).
|
||||
|
||||
When the caller's key has a model allowlist to enforce, no skip path
|
||||
is honored: the JSONL must still be downloaded so
|
||||
A skip is only honored for keys with unrestricted model access. When
|
||||
the key has a model allowlist, the JSONL must still be downloaded so
|
||||
``_enforce_batch_file_model_access`` can validate every ``body.model``
|
||||
entry. Otherwise a restricted key could smuggle unauthorized models
|
||||
into the file via an admin-configured skip (global disable,
|
||||
per-model, per-provider).
|
||||
entry, otherwise a restricted key could smuggle unauthorized models
|
||||
into the file via an admin-configured skip.
|
||||
|
||||
The per-model skip is matched against the file-bound model only, never
|
||||
the client-supplied top-level ``model``. The latter selects routing
|
||||
credentials but not the models the batch runs (those are the JSONL
|
||||
``body.model`` entries), so honoring it would let a caller name a
|
||||
skip-listed deployment while routing a different, rate-limited model.
|
||||
The skip is never keyed on a specific model name. The models a batch
|
||||
actually runs are its JSONL ``body.model`` entries, and any model
|
||||
identifier the caller can influence (the top-level ``model`` or the
|
||||
unsigned model embedded in a ``file-...`` id) can be pointed at a
|
||||
skip-listed deployment while the file routes a different, rate-limited
|
||||
model. The provider skip is safe because the provider is read from the
|
||||
routing deployment's trusted credentials and the batch is constrained
|
||||
to run on that provider.
|
||||
|
||||
Returns ``(should_skip, descriptors)`` where ``descriptors`` is the
|
||||
rate-limit descriptor list computed for the no-limits check, so the
|
||||
|
|
@ -225,16 +217,6 @@ class _PROXY_BatchRateLimiter(CustomLogger):
|
|||
if general_settings.get("disable_batch_input_file_rate_limiting") is True:
|
||||
return True, None
|
||||
|
||||
skip_models = (
|
||||
general_settings.get("skip_batch_input_file_rate_limiting_for_models") or []
|
||||
)
|
||||
file_bound_model = self._get_file_bound_batch_model(data)
|
||||
if file_bound_model and self._matches_skip_list(file_bound_model, skip_models):
|
||||
verbose_proxy_logger.debug(
|
||||
f"Skipping batch input file processing for model={file_bound_model}"
|
||||
)
|
||||
return True, None
|
||||
|
||||
skip_providers = (
|
||||
general_settings.get("skip_batch_input_file_rate_limiting_for_providers")
|
||||
or []
|
||||
|
|
|
|||
|
|
@ -571,14 +571,6 @@ def test_get_batch_routing_model_uses_unified_file_id_target():
|
|||
)
|
||||
|
||||
|
||||
def test_matches_skip_list_handles_empty_and_entry_shapes():
|
||||
rate_limiter = _make_rate_limiter()
|
||||
assert rate_limiter._matches_skip_list("gpt-4o", []) is False
|
||||
assert rate_limiter._matches_skip_list("gpt-4o", ["gpt-4o"]) is True
|
||||
assert rate_limiter._matches_skip_list("vertex_ai/gemini", ["vertex_ai"]) is True
|
||||
assert rate_limiter._matches_skip_list("gpt-4o", [None, "", "claude"]) is False
|
||||
|
||||
|
||||
def test_key_requires_batch_model_access_check_branches():
|
||||
from litellm.proxy.hooks.batch_rate_limiter import _PROXY_BatchRateLimiter
|
||||
|
||||
|
|
@ -660,12 +652,19 @@ def test_should_skip_ignores_client_supplied_metadata_flag():
|
|||
assert should_skip is False
|
||||
|
||||
|
||||
def test_should_skip_honors_per_model_skip_list():
|
||||
"""The per-model skip fires for a model bound to the input file ID (here a
|
||||
model-embedded ``file-<base64>``), which reflects the model the batch runs."""
|
||||
def test_should_not_skip_for_forged_model_embedded_file_id():
|
||||
"""A ``file-<base64>`` id embeds an unsigned model name the caller fully
|
||||
controls, so a caller can re-encode any accessible provider file id with a
|
||||
skip-listed model while the JSONL still routes rate-limited ``body.model``
|
||||
entries. The per-model skip must therefore never fire: with applicable rate
|
||||
limits, a forged skip-listed file-bound model still falls through to file
|
||||
processing and counter enforcement."""
|
||||
import base64
|
||||
|
||||
rate_limiter = _make_rate_limiter()
|
||||
rate_limiter.parallel_request_limiter._create_rate_limit_descriptors.return_value = [
|
||||
{"rate_limit": {"requests_per_unit": 5}}
|
||||
]
|
||||
user = UserAPIKeyAuth(api_key="sk", models=["*"])
|
||||
encoded = (
|
||||
base64.urlsafe_b64encode(b"litellm:file-xyz;model,gpt-4o-mini")
|
||||
|
|
@ -682,15 +681,15 @@ def test_should_skip_honors_per_model_skip_list():
|
|||
user_api_key_dict=user,
|
||||
)
|
||||
)
|
||||
assert should_skip is True
|
||||
assert descriptors is None
|
||||
assert should_skip is False
|
||||
assert descriptors is not None
|
||||
|
||||
|
||||
def test_should_not_skip_per_model_for_spoofed_top_level_model():
|
||||
def test_should_not_skip_for_skip_listed_top_level_model():
|
||||
"""A caller must not bypass batch rate limits by naming a skip-listed model
|
||||
in the top-level ``model`` while routing a different model through the JSONL
|
||||
``body.model`` entries. The per-model skip only trusts the file-bound model,
|
||||
so a skip-listed top-level model over a plain file still gets processed."""
|
||||
``body.model`` entries. No per-model skip exists, so a skip-listed model over
|
||||
a plain file still gets processed."""
|
||||
rate_limiter = _make_rate_limiter()
|
||||
rate_limiter.parallel_request_limiter._create_rate_limit_descriptors.return_value = [
|
||||
{"rate_limit": {"requests_per_unit": 5}}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue