fix(rate-limit): fail closed on unrecognized OVER_LIMIT descriptor
If atomic_check_and_increment_by_n returns overall_code=OVER_LIMIT but no status entry matches a descriptor key the dynamic limiter dispatcher knows how to translate into a 429 (`model_saturation_check` or `priority_model`), the for-loop previously exited cleanly and execution fell through to the priority-tracking increment + the data["litellm_proxy_rate_limit_response"] write — silently admitting an over-limit request. This is the fail-open path a future contributor would hit by wiring a new descriptor type into enforced_descriptors without updating the dispatcher. Refuse the request with a generic 429 carrying the offending descriptor metadata so the operator can see what slipped past, and emit an error log to surface the wiring gap. Adds a regression test (test_dynamic_rate_limiter_v3_fails_closed_on_unknown_descriptor) that drives the limiter with a synthetic OVER_LIMIT response carrying an unrecognized descriptor_key and asserts a 429 is raised. Tests: 65 passed (1 skipped), 0 regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6496e58417
commit
eba0cdf3f5
@ -528,6 +528,37 @@ class _PROXY_DynamicRateLimitHandlerV3(CustomLogger):
|
||||
},
|
||||
)
|
||||
|
||||
# Fail-closed guard: overall_code says OVER_LIMIT but no status
|
||||
# matched a descriptor key we know how to translate into a 429.
|
||||
# Refuse the request rather than silently fall through and let an
|
||||
# over-limit request proceed to the model. Without this, a future
|
||||
# caller wiring an unfamiliar descriptor into enforced_descriptors
|
||||
# would silently bypass the rate limit.
|
||||
offending = next(
|
||||
(s for s in atomic_response["statuses"] if s["code"] == "OVER_LIMIT"),
|
||||
None,
|
||||
)
|
||||
verbose_proxy_logger.error(
|
||||
f"Dynamic rate limiter: OVER_LIMIT response with unknown "
|
||||
f"descriptor_key(s) — refusing request. response={atomic_response}"
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=429,
|
||||
detail={
|
||||
"error": "Rate limit exceeded",
|
||||
"descriptor_key": (
|
||||
offending["descriptor_key"] if offending else "unknown"
|
||||
),
|
||||
"rate_limit_type": (
|
||||
str(offending["rate_limit_type"]) if offending else "unknown"
|
||||
),
|
||||
},
|
||||
headers={
|
||||
"retry-after": str(self.v3_limiter.window_size),
|
||||
"x-litellm-priority": priority or "default",
|
||||
},
|
||||
)
|
||||
|
||||
# If priority is NOT enforced (saturation below threshold) but
|
||||
# priority_descriptors exist, increment them for tracking only — no
|
||||
# check, no rollback. This matches the prior tracking semantics.
|
||||
|
||||
@ -412,3 +412,78 @@ async def test_batch_zero_token_consumes_rpm_only():
|
||||
batch_usage=zero_batch,
|
||||
)
|
||||
assert exc.value.status_code == 429
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dynamic_rate_limiter_v3_fails_closed_on_unknown_descriptor():
|
||||
"""
|
||||
Fail-closed guard: when atomic_check_and_increment_by_n returns
|
||||
overall_code=OVER_LIMIT but with a descriptor_key the dispatcher does
|
||||
not recognize, the dynamic limiter must raise 429 rather than silently
|
||||
fall through.
|
||||
|
||||
Reproduces by patching atomic_check_and_increment_by_n to return an
|
||||
OVER_LIMIT response carrying an unknown descriptor_key.
|
||||
"""
|
||||
from fastapi import HTTPException
|
||||
|
||||
os.environ["LITELLM_LICENSE"] = "test-license-key"
|
||||
litellm.priority_reservation = {"high": 0.9, "low": 0.1}
|
||||
|
||||
dual_cache = DualCache()
|
||||
handler = DynamicRateLimitHandler(internal_usage_cache=dual_cache)
|
||||
|
||||
model = "fail-closed-model"
|
||||
llm_router = Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": model,
|
||||
"litellm_params": {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"api_key": "test-key",
|
||||
"api_base": "test-base",
|
||||
"tpm": 1000,
|
||||
},
|
||||
}
|
||||
]
|
||||
)
|
||||
handler.update_variables(llm_router=llm_router)
|
||||
|
||||
async def fake_atomic(*args, **kwargs):
|
||||
return {
|
||||
"overall_code": "OVER_LIMIT",
|
||||
"statuses": [
|
||||
{
|
||||
"code": "OVER_LIMIT",
|
||||
"current_limit": 100,
|
||||
"limit_remaining": 0,
|
||||
"rate_limit_type": "tokens",
|
||||
"descriptor_key": "future_unrecognized_descriptor",
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
handler.v3_limiter.atomic_check_and_increment_by_n = fake_atomic
|
||||
|
||||
from litellm.types.router import ModelGroupInfo
|
||||
|
||||
user = UserAPIKeyAuth(api_key=hash_token("fail-closed-key"))
|
||||
user.metadata = {"priority": "high"}
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await handler._check_rate_limits(
|
||||
model=model,
|
||||
model_group_info=ModelGroupInfo(
|
||||
model_group=model,
|
||||
providers=["openai"],
|
||||
rpm=None,
|
||||
tpm=1000,
|
||||
),
|
||||
user_api_key_dict=user,
|
||||
priority="high",
|
||||
saturation=0.0,
|
||||
data={},
|
||||
)
|
||||
assert (
|
||||
exc.value.status_code == 429
|
||||
), f"Expected 429 fail-closed on unknown descriptor; got {exc.value.status_code}"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user