mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix(model-filter): mirror inference-time wildcard semantics in discovery filter
Greptile (4/5) flagged that `filter_models_by_user_access` was using `fnmatch.fnmatchcase` while the inference-time check `is_model_allowed_by_pattern` runs a regex (`*` -> `.*`). The PR description claimed these mirror each other, but they diverge whenever a pattern carries a literal `.`: fnmatch treats `.` as a literal byte, the regex treats it as "any char". For a configured pattern `bedrock/anthropic.claude*`, the inference check lets through model names where any character sits in the dotted slot, while the discovery filter only matches the literal `.` byte. Net effect: a user who can successfully call a model at `/v1/messages` could see it missing from `/v1/models`. The exact bug class the PR claims to close. Fix: switch the wildcard branch of `filter_models_by_user_access` to `is_model_allowed_by_pattern`. The exact-match short-circuit a few lines above is untouched and still handles patterns without `*` (which `is_model_allowed_by_pattern` returns False for by design). Tests: extend `tests/test_litellm/proxy/auth/test_model_checks.py` with five unit tests around the filter, including a parity assertion that runs both functions across the same `(model, pattern)` pairs and fails if they ever disagree -- the canonical regression for this Greptile finding.
This commit is contained in:
parent
cafc71934f
commit
ebda8a7ce7
2 changed files with 100 additions and 3 deletions
|
|
@ -1,13 +1,13 @@
|
|||
# What is this?
|
||||
## Common checks for /v1/models and `/model/info`
|
||||
import copy
|
||||
import fnmatch
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
import litellm
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.litellm_core_utils.credential_accessor import CredentialAccessor
|
||||
from litellm.proxy._types import SpecialModelNames, UserAPIKeyAuth
|
||||
from litellm.proxy.auth.auth_checks import is_model_allowed_by_pattern
|
||||
from litellm.repositories.object_permission_repository import ObjectPermissionRepository
|
||||
from litellm.router import Router
|
||||
from litellm.router_utils.fallback_event_handlers import get_fallback_model_group
|
||||
|
|
@ -239,7 +239,12 @@ def filter_models_by_user_access(
|
|||
"""
|
||||
Return the subset of `models` that the user is allowed to see, given
|
||||
the (already-expanded) `user_allowed_models` list. Supports exact
|
||||
match plus `fnmatch` wildcards (e.g. `anthropic/*`, `*`).
|
||||
match plus `*` wildcards (e.g. `anthropic/*`, `*`).
|
||||
|
||||
Wildcard semantics mirror the inference-time check
|
||||
`is_model_allowed_by_pattern` (regex-based, `*` -> `.*`) so a model
|
||||
accepted by `can_user_call_model` cannot be hidden by this filter,
|
||||
and vice versa.
|
||||
|
||||
Caller is responsible for short-circuiting before calling when
|
||||
`user_allowed_models` is empty, contains `all-proxy-models`
|
||||
|
|
@ -252,7 +257,7 @@ def filter_models_by_user_access(
|
|||
for m in models:
|
||||
if m in exact:
|
||||
out.append(m)
|
||||
elif patterns and any(fnmatch.fnmatchcase(m, p) for p in patterns):
|
||||
elif patterns and any(is_model_allowed_by_pattern(m, p) for p in patterns):
|
||||
out.append(m)
|
||||
return out
|
||||
|
||||
|
|
|
|||
|
|
@ -686,3 +686,95 @@ def test_expand_wildcard_invalid_litellm_params_passthrough():
|
|||
# Even if LiteLLM_Params construction fails the deployment should survive
|
||||
result = expand_wildcard_deployments_for_model_info([deployment])
|
||||
assert result == [deployment]
|
||||
|
||||
|
||||
def test_filter_models_by_user_access_exact_match():
|
||||
from litellm.proxy.auth.model_checks import filter_models_by_user_access
|
||||
|
||||
result = filter_models_by_user_access(
|
||||
models=["claude-3-haiku", "claude-3-sonnet", "gpt-4o"],
|
||||
user_allowed_models=["claude-3-haiku"],
|
||||
)
|
||||
assert result == ["claude-3-haiku"]
|
||||
|
||||
|
||||
def test_filter_models_by_user_access_wildcard_preserves_inference_parity():
|
||||
"""
|
||||
Regression for Greptile finding on PR #29748: the discovery-side
|
||||
`filter_models_by_user_access` and the inference-side
|
||||
`is_model_allowed_by_pattern` must agree on every model+pattern pair.
|
||||
|
||||
The previous `fnmatch.fnmatchcase` implementation treated `.` as a
|
||||
literal character while the inference-time check treats it as a regex
|
||||
metacharacter (`.*` substitution applied to `*` only, leaving `.` as
|
||||
"any char"). For real Bedrock-style names like
|
||||
`bedrock/anthropic.claude-3-5-sonnet`, both matchers would agree on
|
||||
the canonical input. The divergence appears in edge inputs where
|
||||
something other than `.` sits in the dotted position: regex would
|
||||
let the model through at call time, while fnmatch would silently
|
||||
hide it from `/v1/models` (over-filter). The user experience is
|
||||
"I can call this model but I cannot see it in the catalog".
|
||||
|
||||
Asserts parity between the two functions across both cases.
|
||||
"""
|
||||
from litellm.proxy.auth.auth_checks import is_model_allowed_by_pattern
|
||||
from litellm.proxy.auth.model_checks import filter_models_by_user_access
|
||||
|
||||
models = [
|
||||
"bedrock/anthropic.claude-3-5-sonnet",
|
||||
"bedrock/anthropicXclaude-3-5-sonnet",
|
||||
]
|
||||
pattern = "bedrock/anthropic.claude*"
|
||||
filtered = filter_models_by_user_access(
|
||||
models=models, user_allowed_models=[pattern]
|
||||
)
|
||||
|
||||
for m in models:
|
||||
inference_allows = is_model_allowed_by_pattern(m, pattern)
|
||||
if inference_allows:
|
||||
assert m in filtered, (
|
||||
f"discovery hid {m!r} (pattern={pattern!r}) that inference "
|
||||
f"would let through; fnmatch vs regex divergence regressed"
|
||||
)
|
||||
else:
|
||||
assert m not in filtered, (
|
||||
f"discovery exposed {m!r} (pattern={pattern!r}) that inference "
|
||||
f"would reject"
|
||||
)
|
||||
|
||||
|
||||
def test_filter_models_by_user_access_wildcard_provider_prefix():
|
||||
"""`anthropic/*` matches every model under the anthropic/ namespace."""
|
||||
from litellm.proxy.auth.model_checks import filter_models_by_user_access
|
||||
|
||||
result = filter_models_by_user_access(
|
||||
models=[
|
||||
"anthropic/claude-3-haiku",
|
||||
"anthropic/claude-3-sonnet",
|
||||
"openai/gpt-4o",
|
||||
"bedrock/anthropic.claude-3-5-sonnet",
|
||||
],
|
||||
user_allowed_models=["anthropic/*"],
|
||||
)
|
||||
assert result == [
|
||||
"anthropic/claude-3-haiku",
|
||||
"anthropic/claude-3-sonnet",
|
||||
]
|
||||
|
||||
|
||||
def test_filter_models_by_user_access_wildcard_global_star():
|
||||
"""Bare `*` matches every model (parity with inference catch-all)."""
|
||||
from litellm.proxy.auth.model_checks import filter_models_by_user_access
|
||||
|
||||
models = ["claude-3-haiku", "openai/gpt-4o", "bedrock/foo.bar"]
|
||||
assert filter_models_by_user_access(models, ["*"]) == models
|
||||
|
||||
|
||||
def test_filter_models_by_user_access_preserves_input_order():
|
||||
from litellm.proxy.auth.model_checks import filter_models_by_user_access
|
||||
|
||||
result = filter_models_by_user_access(
|
||||
models=["b-model", "a-model", "c-model"],
|
||||
user_allowed_models=["*"],
|
||||
)
|
||||
assert result == ["b-model", "a-model", "c-model"]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue