From caa5b8c6b34572575bb12946c14ba9c56b81257c Mon Sep 17 00:00:00 2001 From: lior-k Date: Wed, 17 Jun 2026 16:36:17 +0300 Subject: [PATCH] fix(guardrails): Alice WonderFence rejects non-string credential overrides resolve_api_key / resolve_app_id guarded each source with a bare truthiness check, so a non-string metadata.alice_wonderfence_api_key / _app_id (list, dict, number) was returned as-is. It then reached the SDK/cache, raised a type error, and with fail_open=True the broad handler in apply_guardrail swallowed it and returned the request unscanned. Validate every source (request, key, team, default) as a non-empty string; an invalid type is ignored, so app_id with no other source raises WonderFenceMissingSecrets -> HTTP 500, which is never fail-open. Regression tests cover the resolver level and an apply_guardrail fail_open=True path that must 500 without calling the SDK. --- .../alice_wonderfence/credentials.py | 59 ++++++++++++------- .../alice_wonderfence/test_apply_guardrail.py | 23 ++++++++ .../alice_wonderfence/test_credentials.py | 42 +++++++++++++ 3 files changed, 103 insertions(+), 21 deletions(-) diff --git a/litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/credentials.py b/litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/credentials.py index 802d0188ca6..78cacf56d55 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/credentials.py +++ b/litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/credentials.py @@ -18,10 +18,22 @@ 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, Literal, Optional, Tuple +from typing import TYPE_CHECKING, Any, Literal, Optional, Tuple from .exceptions import WonderFenceMissingSecrets + +def _nonempty_str(value: Any) -> Optional[str]: + """Return ``value`` only if it is a non-empty/non-blank string, else None. + + Credential sources (request body, key/team metadata, config default) are + only honored when they carry a real string. A truthy non-string override + (list, dict, number) must not pass through to the SDK, where it would raise + a type error that ``fail_open`` could swallow into a skipped scan. + """ + return value if isinstance(value, str) and value.strip() else None + + if TYPE_CHECKING: from litellm.litellm_core_utils.litellm_logging import ( Logging as LiteLLMLoggingObj, @@ -83,22 +95,25 @@ def resolve_api_key( metadata = get_metadata(request_data) key_metadata = metadata.get("user_api_key_metadata") or {} - if isinstance(key_metadata, dict) and key_metadata.get("alice_wonderfence_api_key"): - return key_metadata["alice_wonderfence_api_key"] + if isinstance(key_metadata, dict): + val = _nonempty_str(key_metadata.get("alice_wonderfence_api_key")) + if val: + return val team_metadata = metadata.get("user_api_key_team_metadata") or {} - if isinstance(team_metadata, dict) and team_metadata.get( - "alice_wonderfence_api_key" - ): - return team_metadata["alice_wonderfence_api_key"] + if isinstance(team_metadata, dict): + val = _nonempty_str(team_metadata.get("alice_wonderfence_api_key")) + if val: + return val if allow_request_metadata_override: - req_api_key = metadata.get("alice_wonderfence_api_key") - if req_api_key: - return req_api_key + val = _nonempty_str(metadata.get("alice_wonderfence_api_key")) + if val: + return val - if default_api_key: - return default_api_key + val = _nonempty_str(default_api_key) + if val: + return val raise WonderFenceMissingSecrets( "No alice_wonderfence_api_key found in API-key metadata, team " @@ -117,19 +132,21 @@ def resolve_app_id(request_data: dict, allow_request_metadata_override: bool) -> metadata = get_metadata(request_data) key_metadata = metadata.get("user_api_key_metadata") or {} - if isinstance(key_metadata, dict) and key_metadata.get("alice_wonderfence_app_id"): - return key_metadata["alice_wonderfence_app_id"] + if isinstance(key_metadata, dict): + val = _nonempty_str(key_metadata.get("alice_wonderfence_app_id")) + if val: + return val team_metadata = metadata.get("user_api_key_team_metadata") or {} - if isinstance(team_metadata, dict) and team_metadata.get( - "alice_wonderfence_app_id" - ): - return team_metadata["alice_wonderfence_app_id"] + if isinstance(team_metadata, dict): + val = _nonempty_str(team_metadata.get("alice_wonderfence_app_id")) + if val: + return val if allow_request_metadata_override: - req_app_id = metadata.get("alice_wonderfence_app_id") - if req_app_id: - return req_app_id + val = _nonempty_str(metadata.get("alice_wonderfence_app_id")) + if val: + return val raise WonderFenceMissingSecrets( "No alice_wonderfence_app_id found in API-key metadata, team " diff --git a/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_apply_guardrail.py b/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_apply_guardrail.py index 17ef299356f..67149c635ad 100644 --- a/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_apply_guardrail.py +++ b/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_apply_guardrail.py @@ -692,3 +692,26 @@ def test_build_analysis_context_falls_back_to_slash_split(monkeypatch, make_guar kwargs = AnalysisContext.call_args.kwargs assert kwargs["provider"] == "myorg" assert kwargs["model_name"] == "custom-llm" + + +@pytest.mark.asyncio +async def test_malformed_override_does_not_fail_open(make_guardrail, make_request_data): + """A non-string request-metadata app_id override must not slip through under + fail_open: it resolves to a config error (500), not a swallowed exception + that skips scanning. The SDK is never called with a malformed value.""" + guardrail, client = make_guardrail( + fail_open=True, allow_request_metadata_override=True + ) + guardrail._client_cache["default-api-key"] = client + + with pytest.raises(HTTPException) as exc: + await guardrail.apply_guardrail( + inputs={"texts": ["hi"]}, + request_data=make_request_data( + metadata={"alice_wonderfence_app_id": ["not", "a", "string"]} + ), + input_type="request", + ) + assert exc.value.status_code == 500 + assert "alice_wonderfence_app_id" in exc.value.detail["exception"] + client.evaluate_prompt.assert_not_awaited() diff --git a/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_credentials.py b/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_credentials.py index a1df1ee7cb6..ab142cffd04 100644 --- a/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_credentials.py +++ b/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_credentials.py @@ -348,3 +348,45 @@ def test_stashed_api_key_not_present_in_model_call_details(make_logging_obj): def test_recover_returns_none_when_nothing_stashed(make_logging_obj): obj = make_logging_obj() assert recover_resolved(obj, "guard-1") is None + + +# --------------- malformed (non-string) credential overrides --------------- + + +def test_resolve_api_key_ignores_non_string_request_override(): + """A truthy non-string request override must not be returned (it would reach + the SDK and raise, which fail_open could swallow); fall back to default.""" + data = _data(metadata={"alice_wonderfence_api_key": ["not", "a", "string"]}) + assert ( + resolve_api_key( + data, default_api_key="default", allow_request_metadata_override=True + ) + == "default" + ) + + +def test_resolve_app_id_non_string_request_override_raises(): + data = _data(metadata={"alice_wonderfence_app_id": {"bad": 1}}) + with pytest.raises(WonderFenceMissingSecrets): + resolve_app_id(data, allow_request_metadata_override=True) + + +def test_resolve_api_key_ignores_blank_string_override(): + data = _data(metadata={"alice_wonderfence_api_key": " "}) + assert ( + resolve_api_key( + data, default_api_key="default", allow_request_metadata_override=True + ) + == "default" + ) + + +def test_resolve_app_id_non_string_key_metadata_falls_through(): + """A non-string admin value is also rejected rather than passed to the SDK.""" + data = _data( + metadata={ + "user_api_key_metadata": {"alice_wonderfence_app_id": 12345}, + "user_api_key_team_metadata": {"alice_wonderfence_app_id": "team-app"}, + } + ) + assert resolve_app_id(data, allow_request_metadata_override=False) == "team-app"