mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
fix(guardrails): Alice WonderFence keeps the resolved api_key out of logged callback state
The post_call bridge stashed the resolved (api_key, app_id) under logging_obj.model_call_details, which LiteLLM forwards verbatim as kwargs to every success/failure callback and logging exporter; the redaction layer only scrubs message input/output and known StandardLoggingPayload fields, not arbitrary custom keys, so a tenant-specific WonderFence api_key leaked into logs. Move the stash to a private instance attribute on the same logging_obj. It is request scoped and visible across the pre/during/post hooks and the asyncio.gather task boundary exactly as before (same object passed by reference), but it is not part of the kwargs dict handed to callbacks. Tests use a real LiteLLMLoggingObj (not a Mock, whose attribute auto-creation would hide whether the attribute is genuinely settable/readable) and assert the api_key never appears in model_call_details; that assertion fails on the prior implementation. The post_call bridge tests now run against the real object too.
This commit is contained in:
parent
abda857ed8
commit
0c0bba57c7
3 changed files with 92 additions and 25 deletions
|
|
@ -18,7 +18,7 @@ The stash bridges pre_call resolution into post_call where request metadata is
|
|||
gone — see ``stash_resolved`` for the full rationale.
|
||||
"""
|
||||
|
||||
from typing import TYPE_CHECKING, Dict, Literal, Optional, Tuple
|
||||
from typing import TYPE_CHECKING, Literal, Optional, Tuple
|
||||
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
||||
|
|
@ -33,9 +33,15 @@ if TYPE_CHECKING:
|
|||
logger = verbose_proxy_logger.getChild("alice_wonderfence")
|
||||
|
||||
|
||||
# Key used to stash per-request resolved (api_key, app_id) on
|
||||
# logging_obj.model_call_details so post_call can recover it.
|
||||
_LOGGING_OBJ_STASH_KEY = "alice_wonderfence_resolved"
|
||||
# Attribute on the request-scoped logging_obj where the resolved
|
||||
# (api_key, app_id) is stashed so post_call can recover it. This is a private
|
||||
# instance attribute, NOT a model_call_details key, because model_call_details
|
||||
# is forwarded verbatim as ``kwargs`` to every logging callback / exporter
|
||||
# (litellm_logging.py) — stashing the resolved WonderFence api_key there would
|
||||
# leak a tenant secret into logs. A private attribute on the same object gives
|
||||
# the identical cross-hook / cross-task visibility (see stash_resolved) without
|
||||
# entering the logged payload.
|
||||
_STASH_ATTR = "_alice_wonderfence_resolved"
|
||||
|
||||
|
||||
def get_metadata(request_data: dict) -> dict:
|
||||
|
|
@ -149,32 +155,35 @@ def stash_resolved(
|
|||
lives) is dropped. Without a bridge, post_call resolution fails even
|
||||
though the request explicitly supplied the value.
|
||||
|
||||
Why logging_obj.model_call_details (and not a ContextVar):
|
||||
Why a private attribute on logging_obj (and not model_call_details):
|
||||
``model_call_details`` is forwarded verbatim as ``kwargs`` to every
|
||||
logging callback / exporter (``litellm_logging.py`` passes
|
||||
``kwargs=self.model_call_details`` to success/failure handlers and
|
||||
logging hooks), and the redaction layer only scrubs message
|
||||
input/output and known StandardLoggingPayload fields, not arbitrary
|
||||
custom keys. Stashing the resolved WonderFence ``api_key`` there leaks
|
||||
a tenant secret into logs. A private instance attribute is request
|
||||
scoped on the same object but is not part of the logged ``kwargs``.
|
||||
|
||||
Why an attribute on logging_obj (and not a ContextVar):
|
||||
during_call hooks run via ``asyncio.gather`` in
|
||||
``litellm/proxy/utils.py:1500``, which wraps each coroutine in its own
|
||||
asyncio Task with a *copied* context. ContextVar writes in a child
|
||||
Task are not visible to the parent Task that runs post_call, so a
|
||||
ContextVar bridge silently fails. ``logging_obj`` is passed through
|
||||
every hook by reference (same object across pre_call, during_call,
|
||||
and post_call), so mutations to its ``model_call_details`` dict are
|
||||
visible regardless of task boundary.
|
||||
|
||||
Why this isn't a layering hack:
|
||||
Despite the name, ``model_call_details`` is used throughout LiteLLM
|
||||
as a generic request-scoped state bag (see ``main.py:6444``,
|
||||
``proxy/utils.py:1885-1895``, every passthrough handler under
|
||||
``proxy/pass_through_endpoints/``). It stores things like ``model``,
|
||||
``custom_llm_provider``, ``response_cost``, ``messages``, ``client``,
|
||||
``litellm_call_id`` — well beyond log payload material.
|
||||
and post_call), so mutations to it are visible regardless of task
|
||||
boundary.
|
||||
|
||||
Keyed by ``guardrail_name`` so multiple alice_wonderfence instances
|
||||
configured on the same proxy don't collide.
|
||||
"""
|
||||
if logging_obj is None:
|
||||
return
|
||||
container: Dict[str, Tuple[str, str]] = logging_obj.model_call_details.setdefault(
|
||||
_LOGGING_OBJ_STASH_KEY, {}
|
||||
)
|
||||
container = getattr(logging_obj, _STASH_ATTR, None)
|
||||
if not isinstance(container, dict):
|
||||
container = {}
|
||||
setattr(logging_obj, _STASH_ATTR, container)
|
||||
container[guardrail_name] = (api_key, app_id)
|
||||
|
||||
|
||||
|
|
@ -203,8 +212,8 @@ def recover_resolved(
|
|||
"""
|
||||
if logging_obj is None:
|
||||
return None
|
||||
container = logging_obj.model_call_details.get(_LOGGING_OBJ_STASH_KEY)
|
||||
if not container:
|
||||
container = getattr(logging_obj, _STASH_ATTR, None)
|
||||
if not isinstance(container, dict) or not container:
|
||||
return None
|
||||
own = container.get(guardrail_name)
|
||||
if own is not None:
|
||||
|
|
|
|||
|
|
@ -72,11 +72,25 @@ def _request_data(**overrides):
|
|||
return base
|
||||
|
||||
|
||||
def _make_logging_obj() -> Mock:
|
||||
"""Mock the LiteLLMLoggingObj surface we use: only ``model_call_details``."""
|
||||
obj = Mock()
|
||||
obj.model_call_details = {}
|
||||
return obj
|
||||
def _make_logging_obj():
|
||||
"""Build a real ``LiteLLMLoggingObj``.
|
||||
|
||||
The post_call bridge stashes resolved credentials on a private attribute of
|
||||
this object, so tests must use the real class (not a Mock, whose attribute
|
||||
auto-creation would mask whether the attribute is genuinely settable and
|
||||
readable) to validate that the stash survives request -> response.
|
||||
"""
|
||||
from litellm.litellm_core_utils.litellm_logging import Logging
|
||||
|
||||
return Logging(
|
||||
model="gpt-4",
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
stream=False,
|
||||
call_type="completion",
|
||||
start_time=0,
|
||||
litellm_call_id="alice-wonderfence-test",
|
||||
function_id="alice-wonderfence-test",
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
|
|
|
|||
|
|
@ -6,10 +6,14 @@ directly with explicit args instead of constructing a guardrail instance.
|
|||
|
||||
import pytest
|
||||
|
||||
import json
|
||||
|
||||
from litellm.proxy.guardrails.guardrail_hooks.alice_wonderfence.credentials import (
|
||||
get_metadata,
|
||||
recover_resolved,
|
||||
resolve_api_key,
|
||||
resolve_app_id,
|
||||
stash_resolved,
|
||||
)
|
||||
from litellm.proxy.guardrails.guardrail_hooks.alice_wonderfence.exceptions import (
|
||||
WonderFenceMissingSecrets,
|
||||
|
|
@ -301,3 +305,43 @@ def test_responses_route_admin_pin_beats_caller_metadata_api_key():
|
|||
)
|
||||
== "admin-pinned"
|
||||
)
|
||||
|
||||
|
||||
# --------------- stash storage: secret must not leak to logged payload ---------------
|
||||
|
||||
|
||||
def test_logging_obj_allows_private_stash_attr_off_model_call_details(make_logging_obj):
|
||||
"""Guard: the real LiteLLMLoggingObj must accept a private attribute that is
|
||||
NOT part of model_call_details. Fails if Logging becomes slotted/pydantic or
|
||||
the stash is moved back into the logged dict."""
|
||||
obj = make_logging_obj()
|
||||
obj._alice_wonderfence_resolved = {"g": ("k", "a")}
|
||||
assert obj._alice_wonderfence_resolved == {"g": ("k", "a")}
|
||||
assert "_alice_wonderfence_resolved" not in obj.model_call_details
|
||||
|
||||
|
||||
def test_stash_round_trips_on_real_logging_obj(make_logging_obj):
|
||||
obj = make_logging_obj()
|
||||
stash_resolved(obj, "guard-1", "wf-key-abc", "app-1")
|
||||
assert recover_resolved(obj, "guard-1") == ("wf-key-abc", "app-1")
|
||||
|
||||
|
||||
def test_stashed_api_key_not_present_in_model_call_details(make_logging_obj):
|
||||
"""Regression: model_call_details is forwarded verbatim as kwargs to logging
|
||||
callbacks/exporters, so a resolved tenant api_key stashed there leaks. The
|
||||
stash must live off model_call_details. Fails on the prior implementation
|
||||
that stored it under model_call_details["alice_wonderfence_resolved"]."""
|
||||
secret = "wf-super-secret-key-9f3a"
|
||||
obj = make_logging_obj()
|
||||
stash_resolved(obj, "guard-1", secret, "app-1")
|
||||
|
||||
dumped = json.dumps(obj.model_call_details, default=str)
|
||||
assert secret not in dumped
|
||||
assert "alice_wonderfence_resolved" not in obj.model_call_details
|
||||
# recovery still works from the private attribute
|
||||
assert recover_resolved(obj, "guard-1") == (secret, "app-1")
|
||||
|
||||
|
||||
def test_recover_returns_none_when_nothing_stashed(make_logging_obj):
|
||||
obj = make_logging_obj()
|
||||
assert recover_resolved(obj, "guard-1") is None
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue