From e0b843eb1f2fdc04332804bf26b5d4e5fd76a66b Mon Sep 17 00:00:00 2001
From: Ashwin Upadhyay <ashwinupadhyay09@gmail.com>
Date: Sun, 17 May 2026 00:04:26 +0530
Subject: [PATCH] refactor(proxy): use on-path tracking in
 resolve_nested_groups

Previously the visited set tracked every node ever seen during a top-level
resolution. That meant DAG-shared subtrees (A -> [B, C], B -> D, C -> D)
triggered a spurious 'cycle detected' warning on D's second visit.

Switch to on-path tracking: discard the node from visited when its frame
unwinds. Real cycles still fire the warning; DAG re-traversal is silent.
Caller already dedups the final list.

Adds a regression test (test_dag_shared_group_subtree_no_false_cycle_warning)
that asserts no warning is logged for the legitimate DAG case.

Refs #28032
---
 litellm/proxy/auth/model_checks.py            | 34 ++++++++++++-------
 .../proxy/auth/test_nested_access_groups.py   | 28 +++++++++++++++
 2 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/litellm/proxy/auth/model_checks.py b/litellm/proxy/auth/model_checks.py
index a513beecabf..a03ce9f4dd0 100644
--- a/litellm/proxy/auth/model_checks.py
+++ b/litellm/proxy/auth/model_checks.py
@@ -52,8 +52,14 @@ def resolve_nested_groups(
     Expand a group name to the full list of model names it transitively includes,
     following parent -> child edges in `group_memberships`.
 
-    Runs on the auth path, so cyclic edges are logged and skipped rather than raised:
-    a malformed row must not 500 the proxy.
+    Cycle handling: `visited` tracks the *current recursion path* (on-path),
+    not every node ever seen. A revisit while still on the path is a real
+    cycle - we log a warning and skip the cyclic edge. DAG-shared subtrees
+    (e.g. A -> [B, C], B -> [D], C -> [D]) re-traverse D, and the caller
+    deduplicates the final list.
+
+    Cyclic edges are logged and skipped rather than raised: this runs on the
+    auth path and a malformed row must not 500 the proxy.
     """
     if group_name in visited:
         verbose_proxy_logger.warning(
@@ -62,18 +68,20 @@ def resolve_nested_groups(
         )
         return []
     visited.add(group_name)
-
-    resolved: List[str] = list(model_access_groups.get(group_name, []))
-    for child in group_memberships.get(group_name, []):
-        resolved.extend(
-            resolve_nested_groups(
-                group_name=child,
-                model_access_groups=model_access_groups,
-                group_memberships=group_memberships,
-                visited=visited,
+    try:
+        resolved: List[str] = list(model_access_groups.get(group_name, []))
+        for child in group_memberships.get(group_name, []):
+            resolved.extend(
+                resolve_nested_groups(
+                    group_name=child,
+                    model_access_groups=model_access_groups,
+                    group_memberships=group_memberships,
+                    visited=visited,
+                )
             )
-        )
-    return resolved
+        return resolved
+    finally:
+        visited.discard(group_name)
 
 
 def _get_models_from_access_groups(
diff --git a/tests/test_litellm/proxy/auth/test_nested_access_groups.py b/tests/test_litellm/proxy/auth/test_nested_access_groups.py
index 64061958698..15ce0e0f4c8 100644
--- a/tests/test_litellm/proxy/auth/test_nested_access_groups.py
+++ b/tests/test_litellm/proxy/auth/test_nested_access_groups.py
@@ -244,6 +244,34 @@ def test_validate_models_exist_backwards_compatible_without_groups():
 # ---------------------------------------------------------------------------
 
 
+def test_dag_shared_group_subtree_no_false_cycle_warning(caplog):
+    """
+    DAG with a shared group subtree:
+        A -> [B, C]
+        B -> [D]
+        C -> [D]
+        D -> [model-1]
+
+    On-path cycle tracking lets D be re-traversed via both B and C without
+    a spurious 'cycle detected' warning. Caller dedups.
+    """
+    model_access_groups = {"D": ["model-1"]}
+    group_memberships = {"A": ["B", "C"], "B": ["D"], "C": ["D"]}
+
+    with caplog.at_level("WARNING"):
+        result = resolve_nested_groups(
+            group_name="A",
+            model_access_groups=model_access_groups,
+            group_memberships=group_memberships,
+            visited=set(),
+        )
+
+    assert sorted(set(result)) == ["model-1"]
+    assert not any(
+        "cycle detected" in m.lower() for m in caplog.messages
+    ), "DAG-shared subtree must not trigger cycle warnings"
+
+
 def test_diamond_inheritance_resolves_once():
     """
     Diamond shape: