fix: address review findings (team-scoping leak, sentinel collision, log dedup)

Three issues flagged by automated PR review (greptile-apps, veria-ai), all confirmed real and reproduced before fixing:

1. [High/Security] Team-owned deployments entered the global canonical index
   unconditionally. A no-team (unrestricted) key could request an unclaimed
   spelling of a model (e.g. the dated Anthropic ID) whose only server was a
   team's private deployment, resolve onto it, and use that team's
   credentials/quota -- target-authorization passes for unrestricted keys and
   doesn't itself re-derive team ownership. Fixed by excluding any deployment
   with model_info.team_id set from the index entirely: a team boundary is an
   access/billing boundary exactly like the cross-provider boundary this
   module already respects, and team-scoped models remain reachable exactly as
   before, via team_public_model_name through the existing team-route
   machinery this module never touches.

2. [P1] Sentinel collision defeated the ambiguity guard: index.get(key,
   '__absent__') treated a model group literally named '__absent__' as a
   missing entry, so a second group with the same identity would silently
   overwrite it instead of triggering the ambiguity decline. Fixed with a
   proper 'in' check.

3. [P2] Log deduplication was keyed on target alone, so a second distinct
   requested spelling resolving to an already-logged target never got its own
   log line -- undercounting the (requested, target) cardinality that's the
   whole point of the observability story (sizing follow-up-resolution
   demand). Now keyed on the (requested, target) pair.

Added 5 regression tests reproducing each bug pre-fix and asserting the fixed
behavior. Full affected suite: 522 passed. router_code_coverage: 0.0% untested.
ruff-strict BLE001/PERF401: unchanged at base parity. basedpyright: new module
0 errors.

Co-Authored-By: Claude <noreply@anthropic.com>

litellm/router.py

@@ -638,9 +638,14 @@ class Router:
         # group info cache and on cost-map mutation (generation counter).
         self._canonical_model_index: dict[tuple[str, str], str | None] | None = None
         self._canonical_model_index_cost_generation: int = -1
-        # Targets already announced at INFO, so a hot path logs once per target
-        # rather than once per request.
-        self._canonical_resolution_logged: set[str] = set()
+        # (requested, target) pairs already announced at INFO, so a hot path
+        # logs once per distinct pair rather than once per request. Keyed on
+        # the pair, not just the target, so each new requested spelling that
+        # resolves to an already-seen target is still observable -- this is
+        # the signal used to size demand for possible follow-up resolution
+        # rules, so a second spelling silently sharing the first's log line
+        # would undercount it.
+        self._canonical_resolution_logged: set[tuple[str, str]] = set()
         self._init_routing_groups(None)
 
         self.deployment_affinity_ttl_seconds = deployment_affinity_ttl_seconds
@@ -10758,8 +10763,9 @@ class Router:
         # A target whose deployments have all been removed is not a live route.
         if not self.model_name_to_deployment_indices.get(target):
             return None
-        if target not in self._canonical_resolution_logged:
-            self._canonical_resolution_logged.add(target)
+        log_key: Final = (model, target)
+        if log_key not in self._canonical_resolution_logged:
+            self._canonical_resolution_logged.add(log_key)
             verbose_router_logger.info("canonical-resolution: '%s' -> '%s'", model, target)
         return target
 

litellm/router_utils/canonical_model_resolution.py

@@ -146,6 +146,17 @@ def build_canonical_index(
     the same canonical identity; mixed groups are skipped. When two groups claim
     one identity the entry is set to ``_AMBIGUOUS`` (None) so lookups decline.
 
+    Team-owned deployments (``model_info.team_id`` set) are never indexed. A
+    team boundary is an operator-drawn access/billing boundary exactly like a
+    provider boundary (see the module docstring's rule 2): auto-resolution must
+    not cross it. Concretely, without this exclusion a global (no-team) key
+    could request a team's deployment under an unclaimed spelling -- e.g. the
+    dated Anthropic ID -- and land on that team's credentials and quota, since
+    ``is_recognized_model``/target-authorization checks pass for unrestricted
+    keys and don't themselves re-derive team ownership. A team-scoped model
+    remains reachable exactly as it is today: by its team_public_model_name,
+    through the existing team-route machinery, which this module never touches.
+
     Never raises: a malformed deployment or cost-map entry degrades to a smaller
     index, never to a router that fails to boot.
     """
@@ -154,6 +165,11 @@ def build_canonical_index(
 
     for deployment in deployments:
         try:
+            model_info = deployment.get("model_info") or {}
+            if isinstance(  # pyright: ignore[reportUnnecessaryIsInstance] - config/DB rows can violate the TypedDict
+                model_info, Mapping
+            ) and model_info.get("team_id"):
+                continue
             # ``model_name``/``model`` are typed Required[str], but this index is
             # built from operator config and DB rows that can violate the type,
             # so both are validated at runtime rather than trusted.
@@ -196,10 +212,11 @@ def build_canonical_index(
 
         for spelling in spellings:
             key = (provider, spelling)
-            existing = index.get(key, "__absent__")
-            if existing == "__absent__":
+            if key not in index:
                 index[key] = model_group
-            elif existing != model_group:
+                continue
+            existing = index[key]
+            if existing != model_group:
                 # Two groups, same identity: decline rather than choose.
                 index[key] = _AMBIGUOUS
                 verbose_router_logger.info(

tests/test_litellm/router_utils/test_router_canonical_model_resolution.py

@@ -136,6 +136,70 @@ class TestBuildIndexAndLookup:
         assert lookup(index, DATED) is None
         assert lookup(index, UNDATED) is None
 
+    def test_ambiguity_guard_not_defeated_by_group_named_like_sentinel(self):
+        """Regression: the ambiguity check used to compare against the string
+        '__absent__' as an "is this key missing" sentinel. A model group
+        literally named '__absent__' collided with that sentinel and could
+        silently overwrite a same-identity entry instead of triggering the
+        ambiguity decline."""
+        index = build_canonical_index(
+            [
+                {
+                    "model_name": "__absent__",
+                    "litellm_params": {"model": "anthropic/claude-haiku-4-5"},
+                },
+                {
+                    "model_name": "haiku-2",
+                    "litellm_params": {"model": "anthropic/claude-haiku-4-5"},
+                },
+            ]
+        )
+        assert lookup(index, DATED) is None
+        assert lookup(index, UNDATED) is None
+
+    def test_team_owned_deployment_never_indexed(self):
+        """Regression: a team-owned deployment (model_info.team_id set) must
+        never enter the global canonical index. Without this, a no-team key
+        could request an unclaimed spelling of a model whose only server is a
+        team's private deployment and land on that team's credentials/quota --
+        a team boundary is an access/billing boundary exactly like the
+        cross-provider boundary and must not be crossed by inference."""
+        index = build_canonical_index(
+            [
+                {
+                    "model_name": "internal-team-model-xyz",
+                    "litellm_params": {"model": "anthropic/claude-haiku-4-5"},
+                    "model_info": {
+                        "team_id": "team-A",
+                        "team_public_model_name": "claude-haiku-4-5",
+                    },
+                },
+            ]
+        )
+        assert lookup(index, DATED) is None
+        assert lookup(index, UNDATED) is None
+
+    def test_team_owned_deployment_does_not_block_global_sibling(self):
+        """A team-owned deployment coexisting with a global deployment of the
+        same identity must not suppress resolution to the global one."""
+        index = build_canonical_index(
+            [
+                {
+                    "model_name": "internal-team-model-xyz",
+                    "litellm_params": {"model": "anthropic/claude-haiku-4-5"},
+                    "model_info": {
+                        "team_id": "team-A",
+                        "team_public_model_name": "claude-haiku-4-5",
+                    },
+                },
+                {
+                    "model_name": ANTHROPIC_GROUP,
+                    "litellm_params": {"model": "anthropic/claude-haiku-4-5"},
+                },
+            ]
+        )
+        assert lookup(index, DATED) == ANTHROPIC_GROUP
+
     def test_request_for_own_group_name_is_not_a_rewrite(self):
         index = build_canonical_index(
             [
@@ -169,6 +233,44 @@ class TestRouterResolveCanonicalModelName:
         """I1: a name the router already serves is never rewritten."""
         assert anthropic_router.resolve_canonical_model_name(ANTHROPIC_GROUP) is None
 
+    def test_no_team_caller_never_resolves_onto_team_owned_deployment(self):
+        """Regression: end-to-end version of the team-leak fix. A no-team
+        caller asking for an unclaimed spelling must not resolve onto a
+        deployment that is the sole server of that identity but is owned by a
+        team -- that would leak the team's credentials/quota to a global key."""
+        router = Router(
+            model_list=[
+                {
+                    "model_name": "internal-team-model-xyz",
+                    "litellm_params": {"model": "anthropic/claude-haiku-4-5", "api_key": "team-secret"},
+                    "model_info": {"team_id": "team-A", "team_public_model_name": "claude-haiku-4-5"},
+                },
+            ]
+        )
+        assert router.resolve_canonical_model_name(DATED, request_team_id=None) is None
+        # Even the requesting team's own id must not resolve through this path --
+        # team-scoped models are reached via the existing team-route machinery,
+        # not via canonical inference.
+        assert router.resolve_canonical_model_name(DATED, request_team_id="team-A") is None
+
+    def test_log_dedup_keyed_on_pair_not_target_alone(self, anthropic_router: Router, caplog: pytest.LogCaptureFixture):
+        """Regression: a second distinct requested spelling resolving to an
+        already-logged target must still get its own log line -- the
+        (requested, target) cardinality is the signal used to size demand for
+        follow-up resolution rules, so deduping on target alone would
+        undercount it."""
+        import logging
+
+        with caplog.at_level(logging.INFO, logger="LiteLLM Router"):
+            assert anthropic_router.resolve_canonical_model_name(DATED) == ANTHROPIC_GROUP
+            assert anthropic_router.resolve_canonical_model_name(UNDATED) == ANTHROPIC_GROUP
+            # Re-requesting the same spelling must not double-log.
+            assert anthropic_router.resolve_canonical_model_name(DATED) == ANTHROPIC_GROUP
+        messages = [r.message for r in caplog.records if "canonical-resolution" in r.message]
+        assert any(DATED in m for m in messages)
+        assert any(UNDATED in m for m in messages)
+        assert sum(1 for m in messages if DATED in m) == 1
+
     def test_strict_mode_disables_resolution(self):
         from litellm.types.router import RouterGeneralSettings