From e87b9e7827b6a487a2a5b2bdf5fc96c88ad43563 Mon Sep 17 00:00:00 2001 From: yucheng-berri Date: Tue, 23 Jun 2026 12:07:11 -0700 Subject: [PATCH] proxy: cap recursive secret redaction depth at 10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match the cap on _redact_sensitive_litellm_params (the closest analog in the proxy, also recursive, key-name driven, returns a sentinel). The previous justification — bounded by operator-authored schema depth, JsonValue acyclic — is true today but is a property of the threat model, not an enforced invariant of the function. If a code path is ever added that pipes external input into general_settings (config import, migration tooling, JWT-driven settings, …) the assumption silently breaks. A local cap makes the invariant local. The cap branch fails closed: at _REDACT_SECRET_MAX_DEPTH the whole subtree is replaced with 'REDACTED' rather than returned verbatim. A future refactor that flips this to fail-open would let a deeply nested credential leak; the new regression test test_redact_secret_values_in_obj_fails_closed_at_max_depth guards against that. Updates the recursive_detector ignore-list rationale to point at the numeric cap rather than the structural argument. --- litellm/proxy/proxy_server.py | 19 ++++++++++--- .../code_coverage_tests/recursive_detector.py | 2 +- .../proxy/proxy_server/test_routes_config.py | 28 +++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index e4c14c17811..8f541072c11 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -15094,21 +15094,32 @@ def _is_secret_general_setting_field(field_name: str) -> bool: ) -def _redact_secret_values_in_obj(value: JsonValue) -> JsonValue: +# Matches the cap on _redact_sensitive_litellm_params (the closest analog in the +# proxy). Past this depth we fail closed by returning "REDACTED" for the whole +# subtree rather than recursing further — better to over-redact a pathological +# config than to silently return a deeply-nested credential verbatim +_REDACT_SECRET_MAX_DEPTH = 10 + + +def _redact_secret_values_in_obj(value: JsonValue, depth: int = 0) -> JsonValue: """Recursively redact secret leaves inside a structured field so a nested credential (e.g. aws_web_identity_token under database_args) is never - returned to a non-admin, while non-secret siblings stay visible""" + returned to a non-admin, while non-secret siblings stay visible. At + _REDACT_SECRET_MAX_DEPTH the whole subtree is replaced with "REDACTED" + so depth-overrun fails closed.""" + if depth >= _REDACT_SECRET_MAX_DEPTH: + return "REDACTED" if isinstance(value, dict): return { key: ( "REDACTED" if _is_secret_general_setting_field(key) - else _redact_secret_values_in_obj(sub) + else _redact_secret_values_in_obj(sub, depth + 1) ) for key, sub in value.items() } if isinstance(value, list): - return [_redact_secret_values_in_obj(item) for item in value] + return [_redact_secret_values_in_obj(item, depth + 1) for item in value] return value diff --git a/tests/code_coverage_tests/recursive_detector.py b/tests/code_coverage_tests/recursive_detector.py index a7ef3556585..1d11d676207 100644 --- a/tests/code_coverage_tests/recursive_detector.py +++ b/tests/code_coverage_tests/recursive_detector.py @@ -47,7 +47,7 @@ IGNORE_FUNCTIONS = [ "_read_image_bytes", # max depth set. "_get_masked_values", # max depth set (default 20) to prevent infinite recursion while masking nested sensitive config dicts. "_redact_sensitive_litellm_params", # max depth set (default 10). - "_redact_secret_values_in_obj", # config secret redaction; bounded by operator-authored general_settings schema depth, JsonValue is acyclic so no cycles possible. + "_redact_secret_values_in_obj", # max depth set (default 10, _REDACT_SECRET_MAX_DEPTH); fails closed by returning "REDACTED" at the cap. "_resolve", # OCI: $ref resolver bounded by `resolving_stack` cycle guard. "resolve_oci_schema_anyof", # OCI: bounded by JSON-schema tree depth (no cycles possible in well-formed input). "sanitize_oci_schema", # OCI: bounded by JSON-schema tree depth. diff --git a/tests/test_litellm/proxy/proxy_server/test_routes_config.py b/tests/test_litellm/proxy/proxy_server/test_routes_config.py index 3ea15f2e14c..15571d7f1da 100644 --- a/tests/test_litellm/proxy/proxy_server/test_routes_config.py +++ b/tests/test_litellm/proxy/proxy_server/test_routes_config.py @@ -353,6 +353,34 @@ def test_redact_general_setting_value_recurses_list_of_dicts(): ) +def test_redact_secret_values_in_obj_fails_closed_at_max_depth(): + """Past _REDACT_SECRET_MAX_DEPTH the whole subtree is replaced with + "REDACTED" rather than returned verbatim, so a secret buried below the cap + can never leak via depth-overrun. A future refactor that flips the cap + branch to fail-open would surface here.""" + from litellm.proxy import proxy_server as ps + + # build a non-secret-keyed wrap chain deeper than the cap, with a real + # secret at the bottom. Using a non-secret key ("wrap") forces the + # recursor down the recursion branch instead of short-circuiting on the + # key name itself. + nested: object = {"aws_web_identity_token": "sk-leak-bottom"} + for _ in range(ps._REDACT_SECRET_MAX_DEPTH + 2): + nested = {"wrap": nested} + + out = ps._redact_general_setting_value( + "some_struct_field", nested, is_full_admin=False + ) + # the secret must not survive anywhere in the returned tree + assert "sk-leak-bottom" not in repr(out) + + # full admin is unaffected by the cap — the value comes back untouched + admin_out = ps._redact_general_setting_value( + "some_struct_field", nested, is_full_admin=True + ) + assert admin_out is nested + + def test_config_list_redacts_pass_through_secret_for_view_only( client, auth_as, mock_prisma, monkeypatch ):