diff --git a/litellm/proxy/common_utils/callback_config_validation.py b/litellm/proxy/common_utils/callback_config_validation.py index c13e07c00aa..805c8897b9d 100644 --- a/litellm/proxy/common_utils/callback_config_validation.py +++ b/litellm/proxy/common_utils/callback_config_validation.py @@ -77,7 +77,7 @@ def cross_entry_family_error( callback_vars: Mapping[str, str] | None, stored_vars_by_entry: Sequence[Mapping[str, str]], ) -> str | None: - """Reject an entry that joins a credential family another entry already holds. + """Reject an entry that changes a credential family another entry already holds. Every stored entry's variables are flattened into one dict before a request reads them, and the flattened dict is what the exporter authenticates and @@ -86,20 +86,31 @@ def cross_entry_family_error( with the key from the first, and the request carries that key to the new host. - Requiring one entry to own a family end to end removes the pairing. Only the - writers this endpoint newly admits are held to it, because a proxy admin - already holds every credential the proxy has. A team admin who does want to - move a family deletes the entry holding it first, which reveals nothing. + Requiring one entry to own a family end to end removes the pairing. Repeating + a value the holding entry already stores is allowed, because flattening then + produces the same dict either way -- that is how the same integration gets + registered for both the success and the failure event. A team admin who does + want to move a family deletes the entry holding it first, which reveals + nothing. + + Only the writers this endpoint newly admits are held to this, because a proxy + admin already holds every credential the proxy has. + + ``stored_vars_by_entry`` has to arrive decrypted; the credential values are + encrypted at rest and ciphertext never equals the plaintext coming in. """ if not callback_vars: return None - held: Final = {family for entry in stored_vars_by_entry for family in map(_family_of, entry) if family is not None} + stored: Final = { # mutable-ok: local view of the stored entries, never stored + var: value for entry in stored_vars_by_entry for var, value in entry.items() if _family_of(var) is not None + } + held: Final = {_family_of(var) for var in stored} # mutable-ok: local index, never stored return next( ( f"{family} is already configured by another callback entry on this team. " f"Remove that entry before setting {var} here." - for var, family in ((v, _family_of(v)) for v in callback_vars) - if family is not None and family in held + for var, value, family in ((v, callback_vars[v], _family_of(v)) for v in callback_vars) + if family in held and stored.get(var) != value ), None, ) diff --git a/litellm/proxy/management_endpoints/team_callback_endpoints.py b/litellm/proxy/management_endpoints/team_callback_endpoints.py index c9cedd47e26..136ccdac4da 100644 --- a/litellm/proxy/management_endpoints/team_callback_endpoints.py +++ b/litellm/proxy/management_endpoints/team_callback_endpoints.py @@ -347,11 +347,16 @@ async def add_team_callbacks( # flattened into one dict before a request reads them, so an entry # naming only a destination would pair with a key written on another # entry and carry it to that destination -- a key a team admin can read - # back nowhere. Proxy admins are exempt: they already hold every - # credential the proxy has. + # back nowhere. Repeating a value the owning entry already stores is + # fine, which is how one integration covers both events. Proxy admins + # are exempt: they already hold every credential the proxy has. if user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN: + # Decrypted, because the check compares the incoming values against + # the stored ones and the credentials are encrypted at rest. + decrypted_logging: Final = decrypt_callback_vars(team_metadata).get("logging") + stored_entries: Final = decrypted_logging if isinstance(decrypted_logging, list) else () stored_entry_vars: Final = [ # mutable-ok: read-only input to the check, never stored - entry.get("callback_vars") or {} for entry in team_callback_settings + entry.get("callback_vars") or {} for entry in stored_entries ] family_error: Final = cross_entry_family_error(data.callback_vars, stored_entry_vars) if family_error is not None: diff --git a/tests/test_litellm/proxy/management_endpoints/test_team_callback_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_team_callback_endpoints.py index 4f4904b97d9..7b812fd6e61 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_team_callback_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_team_callback_endpoints.py @@ -1538,6 +1538,11 @@ async def test_proxy_admin_still_told_the_team_is_unknown(): ({"langsmith_api_key": "k"}, [{"dd_api_key": "k"}], False), # variables that configure no backend carry nothing to redirect ({"turn_off_message_logging": "true"}, [{"langfuse_secret_key": "sk"}], False), + # the same integration registered for a second event: identical values + # flatten to the identical dict, so there is nothing to redirect + ({"langfuse_host": "https://us.cloud.langfuse.com", "langfuse_public_key": "pk", "langfuse_secret_key": "sk"}, [{"langfuse_host": "https://us.cloud.langfuse.com", "langfuse_public_key": "pk", "langfuse_secret_key": "sk"}], False), + # the same shape with one value moved is the redirect again + ({"langfuse_host": "http://attacker.invalid", "langfuse_public_key": "pk", "langfuse_secret_key": "sk"}, [{"langfuse_host": "https://us.cloud.langfuse.com", "langfuse_public_key": "pk", "langfuse_secret_key": "sk"}], True), ], ) def test_one_entry_owns_a_credential_family(new_vars, stored, rejected):