From bbf017d5cffd682090d39e1d97aaf6cfaf2a3cc5 Mon Sep 17 00:00:00 2001 From: apex-mochen <2756823972@qq.com> Date: Thu, 5 Nov 2026 19:28:37 +0800 Subject: [PATCH] fix(proxy): let accounting callbacks opt in to no-log requests by attribute A `no-log: true` request asks that its content not be logged; it is not an opt-out of billing. The filter decided by class name, so the router's budget limiter (`RouterBudgetLimiting`) was silently skipped and deployment, provider and tag budgets could be bypassed -- reported by a reviewer as six out of six no-log calls returning 200 under a 0.05 deployment `max_budget`. Replace the name list with an inherited `CustomLogger.runs_on_no_log_requests` attribute mirroring `enforces_request_content`, set it on `_ProxyDBLogger` and `RouterBudgetLimiting`, and keep the internal `_PROXY_` prefix as a back-compat signal for hooks that predate the attribute. Close the second leak of the same kind: `sanitize_error_information_for_spend_logs` only consulted the proxy-wide `store_prompts_in_spend_logs`, so a no-log request with prompt storage enabled still persisted the prompt echoed back inside provider errors. Thread `litellm_params` through it so it follows the per-request rule, as the vector-store payload path already does. Tests: attribute opt-in and default filtering, the real classes carry the mark, and error_information redaction across the `no-log` x `global_disable_no_log_param` matrix. Signed-off-by: apex-mochen <2756823972@qq.com> --- litellm/integrations/custom_logger.py | 15 +++++ litellm/litellm_core_utils/litellm_logging.py | 16 ++--- .../proxy/hooks/proxy_track_cost_callback.py | 3 + .../spend_tracking/spend_tracking_utils.py | 10 +++- litellm/router_strategy/budget_limiter.py | 3 + .../test_litellm_logging.py | 59 ++++++++++++++++++- .../test_no_log_request_spend_body_privacy.py | 29 ++++++++- 7 files changed, 116 insertions(+), 19 deletions(-) diff --git a/litellm/integrations/custom_logger.py b/litellm/integrations/custom_logger.py index 0579eb15baf..5103370ff77 100644 --- a/litellm/integrations/custom_logger.py +++ b/litellm/integrations/custom_logger.py @@ -85,6 +85,21 @@ class CustomLogger: # https://docs.litellm.ai/docs/observability/custom_callbac consulted, so a subclass that does not override ``async_pre_call_hook`` inherits nothing. """ + runs_on_no_log_requests: bool = False + """ + Whether this callback must still run for a request that asked for ``no-log: true``. + + ``no-log`` asks that the request's *content* not be logged; it is not a request to stop + accounting for it. A callback that only moves numbers -- the proxy's spend writer, the + router's budget limiter -- sets this True, so the request is still billed and still counts + against budgets. Exporters and hooks that ship the payload to a third party leave it False. + + Opting in by attribute rather than by class name is deliberate: a class-name check breaks + silently when the class is renamed, which is how a ``no-log`` request started being served + without being charged. The attribute inherits to subclasses and reads as intent rather than + as a naming convention. + """ + def __init__( self, turn_off_message_logging: bool = False, diff --git a/litellm/litellm_core_utils/litellm_logging.py b/litellm/litellm_core_utils/litellm_logging.py index e9bf66df419..c0592f57341 100644 --- a/litellm/litellm_core_utils/litellm_logging.py +++ b/litellm/litellm_core_utils/litellm_logging.py @@ -473,11 +473,6 @@ _INPUT_PRICING_KEY_PREFIXES: Final = ( _OUTPUT_PRICING_KEY_PREFIXES: Final = ("output_cost_per_token",) _BATCH_PRICING_KEY_SUFFIX: Final = "_batches" -# Proxy-internal callbacks that must run even on `no-log` requests: the proxy still has to do -# its own spend accounting. The `_PROXY_` handler prefix marks most of them; the spend writer -# `_ProxyDBLogger` predates that prefix, so it is listed explicitly (exact names, no substring). -_PROXY_INTERNAL_CALLBACK_CLASS_NAMES: Final = frozenset({"_ProxyDBLogger"}) - _NO_CARRIED_RATES: Final[Mapping[str, object]] = MappingProxyType({}) @@ -2927,13 +2922,14 @@ class Logging(LiteLLMLoggingBaseClass): return True if litellm_params.get("no-log", False) is True: - # Proxy-internal callbacks must still run on `no-log` requests: the proxy's own - # spend accounting has to record the request. They use the internal `_PROXY_` - # handler prefix or -- for the spend writer, which predates that prefix -- an - # explicit entry in `_PROXY_INTERNAL_CALLBACK_CLASS_NAMES`. + # A `no-log` request still has to be accounted for: billing, budgets and rate + # limits all read the same success path. Callbacks that only move numbers opt in + # through `runs_on_no_log_requests`; the internal `_PROXY_` handler prefix stays + # as a back-compat signal for hooks that predate the attribute. Exporters stay + # filtered, which is the point of `no-log` in the first place. callback_class_name: Final = callback.__class__.__name__ runs_on_no_log: Final = isinstance(callback, CustomLogger) and ( - callback_class_name.startswith("_PROXY_") or callback_class_name in _PROXY_INTERNAL_CALLBACK_CLASS_NAMES + callback.runs_on_no_log_requests or callback_class_name.startswith("_PROXY_") ) if not runs_on_no_log: verbose_logger.debug("no-log request, skipping logging for %s event", event_hook) diff --git a/litellm/proxy/hooks/proxy_track_cost_callback.py b/litellm/proxy/hooks/proxy_track_cost_callback.py index a0104887c63..569804277d5 100644 --- a/litellm/proxy/hooks/proxy_track_cost_callback.py +++ b/litellm/proxy/hooks/proxy_track_cost_callback.py @@ -395,6 +395,9 @@ def _attempt_spend_row_kwargs( class _ProxyDBLogger(CustomLogger): + # The proxy still bills a `no-log` request; only its content is withheld. + runs_on_no_log_requests = True + def __init__( self, spend_event_producer: SpendEventProducer | None = None, diff --git a/litellm/proxy/spend_tracking/spend_tracking_utils.py b/litellm/proxy/spend_tracking/spend_tracking_utils.py index 73b2a3e6d52..d484cc35806 100644 --- a/litellm/proxy/spend_tracking/spend_tracking_utils.py +++ b/litellm/proxy/spend_tracking/spend_tracking_utils.py @@ -910,7 +910,9 @@ def get_logging_payload( if drop_requester_ip_address: clean_metadata["requester_ip_address"] = None clean_metadata["error_information"] = sanitize_error_information_for_spend_logs( - clean_metadata["error_information"], original_exception=failure_exception + clean_metadata["error_information"], + original_exception=failure_exception, + request_litellm_params=cast("Mapping[str, object] | None", litellm_params), ) if litellm.cache is None: @@ -1593,6 +1595,7 @@ def _redact_prompt_fields_in_guardrail_entry( def sanitize_error_information_for_spend_logs( error_information: StandardLoggingPayloadErrorInformation | None, original_exception: BaseException | None = None, + request_litellm_params: Mapping[str, object] | None = None, ) -> StandardLoggingPayloadErrorInformation | None: """ Sanitize ``error_information`` before it lands in ``LiteLLM_SpendLogs.metadata``. @@ -1603,7 +1606,8 @@ def sanitize_error_information_for_spend_logs( - Always: cap ``error_message`` and ``traceback`` with the existing ``MAX_STRING_LENGTH_PROMPT_IN_DB`` DB-storage safeguard. - - When ``store_prompts_in_spend_logs`` is False: additionally redact + - When the request asked for ``no-log``, or ``store_prompts_in_spend_logs`` is False, + additionally redact ``'input'`` / ``'messages'`` / ``'prompt'`` values *and* Pydantic v2 ``input_value=...`` assignments inside both ``error_message`` and ``traceback`` so prompts cannot leak through either field. @@ -1621,7 +1625,7 @@ def sanitize_error_information_for_spend_logs( ) sanitized = cast(dict, {**persisted}) - if not should_store_prompts_and_responses_in_spend_logs(): + if not should_store_prompts_and_responses_in_spend_logs(request_litellm_params): for field in ("error_message", "traceback"): value = sanitized.get(field) if isinstance(value, str): diff --git a/litellm/router_strategy/budget_limiter.py b/litellm/router_strategy/budget_limiter.py index 98dd39b40e3..a4dbae7a2d3 100644 --- a/litellm/router_strategy/budget_limiter.py +++ b/litellm/router_strategy/budget_limiter.py @@ -120,6 +120,9 @@ def budget_window_seconds(started_at: float, budget_duration: str) -> int: class RouterBudgetLimiting(CustomLogger): + # Deployment, provider and tag budgets are accounting: a `no-log` request still spends. + runs_on_no_log_requests = True + def __init__( self, dual_cache: DualCache, diff --git a/tests/unit/litellm_core_utils/test_litellm_logging.py b/tests/unit/litellm_core_utils/test_litellm_logging.py index 4a957f543c2..96174c01d4d 100644 --- a/tests/unit/litellm_core_utils/test_litellm_logging.py +++ b/tests/unit/litellm_core_utils/test_litellm_logging.py @@ -14740,9 +14740,7 @@ async def test_logger_whose_name_mentions_proxy_runs_without_no_log( assert len(logger.logged) == 1 and logger.logged[0] is result -def test_no_log_keeps_running_proxy_prefixed_handlers( - logging_obj: LitellmLogging, monkeypatch: pytest.MonkeyPatch -): +def test_no_log_keeps_running_proxy_prefixed_handlers(logging_obj: LitellmLogging, monkeypatch: pytest.MonkeyPatch): """The proxy's `_PROXY_*` handler family must keep running on no-log requests.""" monkeypatch.setattr(litellm, "global_disable_no_log_param", False) assert ( @@ -14753,3 +14751,58 @@ def test_no_log_keeps_running_proxy_prefixed_handlers( ) is True ) + + +class _AccountingStub(CustomLogger): + """Stands in for an accounting callback (router budgets) that opts in by attribute.""" + + runs_on_no_log_requests = True + + +class _ExportingStub(CustomLogger): + """Default: a callback that ships content stays filtered on `no-log`.""" + + +def test_no_log_runs_callback_that_opts_in_by_attribute(logging_obj: LitellmLogging, monkeypatch: pytest.MonkeyPatch): + """A callback that only moves numbers opts in with `runs_on_no_log_requests`.""" + monkeypatch.setattr(litellm, "global_disable_no_log_param", False) + assert ( + logging_obj.should_run_callback( + callback=_AccountingStub(), + litellm_params={"no-log": True}, + event_hook="success", + ) + is True + ) + + +def test_no_log_filters_default_callback_without_the_attribute( + logging_obj: LitellmLogging, monkeypatch: pytest.MonkeyPatch +): + """Control: the attribute is an opt-in, so a default CustomLogger stays filtered.""" + monkeypatch.setattr(litellm, "global_disable_no_log_param", False) + assert ( + logging_obj.should_run_callback( + callback=_ExportingStub(), + litellm_params={"no-log": True}, + event_hook="success", + ) + is False + ) + assert ( + logging_obj.should_run_callback( + callback=_ExportingStub(), + litellm_params={"no-log": False}, + event_hook="success", + ) + is True + ) + + +def test_accounting_callbacks_are_marked_in_the_real_classes(): + """#44503 follow-up: the spend writer and the router budget limiter must both opt in.""" + from litellm.proxy.hooks.proxy_track_cost_callback import _ProxyDBLogger + from litellm.router_strategy.budget_limiter import RouterBudgetLimiting + + assert _ProxyDBLogger.runs_on_no_log_requests is True + assert RouterBudgetLimiting.runs_on_no_log_requests is True diff --git a/tests/unit/proxy/spend_tracking/test_no_log_request_spend_body_privacy.py b/tests/unit/proxy/spend_tracking/test_no_log_request_spend_body_privacy.py index d9616aa62f7..8c7a9db0297 100755 --- a/tests/unit/proxy/spend_tracking/test_no_log_request_spend_body_privacy.py +++ b/tests/unit/proxy/spend_tracking/test_no_log_request_spend_body_privacy.py @@ -61,9 +61,7 @@ def _payload(monkeypatch, no_log: bool, store_prompts: bool) -> dict: monkeypatch.setattr(proxy_server, "general_settings", {"store_prompts_in_spend_logs": store_prompts}) now: Final = datetime.datetime.now(timezone.utc) - return get_logging_payload( - kwargs=_kwargs(no_log=no_log), response_obj=_response(), start_time=now, end_time=now - ) + return get_logging_payload(kwargs=_kwargs(no_log=no_log), response_obj=_response(), start_time=now, end_time=now) @pytest.mark.parametrize("store_prompts", [True, False]) @@ -166,3 +164,28 @@ def test_vector_store_content_follows_request_no_log(monkeypatch, no_log: bool, payload: Final = get_logging_payload(kwargs=kwargs, response_obj=_response(), start_time=now, end_time=now) assert (VECTOR_STORE_SECRET in json.dumps(payload, default=str)) is (not no_log or global_disable_no_log_param) + + +ERROR_SECRET: Final = "ERROR-SECRET-1d4b9f" + + +@pytest.mark.parametrize("global_disable_no_log_param", [False, True]) +@pytest.mark.parametrize("no_log", [True, False]) +def test_error_information_follows_request_no_log(monkeypatch, no_log: bool, global_disable_no_log_param: bool): + """A provider error can echo the body, so the same per-request rule must redact it.""" + from litellm.proxy import proxy_server + from litellm.proxy.spend_tracking.spend_tracking_utils import ( + sanitize_error_information_for_spend_logs, + ) + + monkeypatch.setattr(litellm, "global_disable_no_log_param", global_disable_no_log_param) + monkeypatch.setattr(proxy_server, "general_settings", {"store_prompts_in_spend_logs": True}) + error_information: Final = { + "error_message": "validation error [type=string_type, input_value=" + repr(ERROR_SECRET) + ", input_type=str]", + "traceback": "payload {'messages': [{'role': 'user', 'content': " + repr(ERROR_SECRET) + "}]}", + } + + sanitized = sanitize_error_information_for_spend_logs(error_information, request_litellm_params={"no-log": no_log}) + + content_expected: Final = not (no_log and not global_disable_no_log_param) + assert (ERROR_SECRET in json.dumps(sanitized, default=str)) is content_expected