From dc35845d9e7702719196435f20d2eb83939326b0 Mon Sep 17 00:00:00 2001 From: derhornspieler <15236687+derhornspieler@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:36:29 -0400 Subject: [PATCH] fix(anthropic): finish the caller-credential sweep and stop echoing a mistyped secret ref Skills merged the deployment credential over the caller's headers the way files and batches did, so it now uses the same helper. Sweeping every Anthropic surface for that shape turned up one more the reviewers had not reported: the experimental passthrough decides whether to mint by checking whether the caller already sent a credential, and that check was case-sensitive, so an SDK caller passing X-Api-Key through extra_headers slipped it and would have sent their key beside a minted Bearer. Both checks now match case-insensitively, and honor the caller's credential as they were meant to. The files handler was checked and left alone: it builds its header dict from scratch, so no caller header reaches it. The unresolved-reference errors named the reference, which is deliberate and is what tells an operator which setting failed. But these only fail when what was written is not resolvable, and an operator who pasted the secret itself has made the field's value the secret. The reference is now echoed only when it looks like one, and withheld otherwise, so the existing behavior survives for real references. The Add Provider wizard treated a credential as identified by name alone, so keeping the name while switching providers took the update path and would have rewritten the first provider's credential as the second's. A credential is identified by name and provider together. --- .../messages/transformation.py | 15 +++++- .../llms/anthropic/skills/transformation.py | 16 +++--- .../llms/base_llm/auth/client_credentials.py | 4 +- litellm/llms/base_llm/auth/identity_source.py | 20 ++++++++ litellm/llms/base_llm/auth/internal_issuer.py | 6 ++- .../anthropic/test_anthropic_common_utils.py | 46 +++++++++++++++++ .../base_llm/auth/test_client_credentials.py | 39 ++++++++++++++ .../AddProviderPanel.integration.test.tsx | 51 +++++++++++++++++++ .../panels/add-provider/AddProviderPanel.tsx | 11 ++-- 9 files changed, 192 insertions(+), 16 deletions(-) diff --git a/litellm/llms/anthropic/experimental_pass_through/messages/transformation.py b/litellm/llms/anthropic/experimental_pass_through/messages/transformation.py index 1741a093d75..f11b8ff63bb 100644 --- a/litellm/llms/anthropic/experimental_pass_through/messages/transformation.py +++ b/litellm/llms/anthropic/experimental_pass_through/messages/transformation.py @@ -34,6 +34,17 @@ from ...common_utils import ( DEFAULT_ANTHROPIC_API_VERSION: Final = "2023-06-01" +_CALLER_CREDENTIAL_HEADERS: Final = frozenset({"x-api-key", "authorization"}) + + +def _carries_caller_credential(headers: Mapping[str, str]) -> bool: + """Whether the caller sent their own Anthropic credential, in which case this passthrough + honors it and never mints. Matched case-insensitively: an SDK caller passing ``X-Api-Key`` + through extra_headers would otherwise slip the check and end up sending their key beside a + minted federation Bearer.""" + return any(name.lower() in _CALLER_CREDENTIAL_HEADERS for name in headers) + + DROP_UNSUPPORTED_ADAPTIVE_EFFORT_WARNING: Final = ( "Dropping adaptive `thinking`/`output_config.effort` for model=%s: the model " "does not support extended thinking, or max_tokens is too small to fit the " @@ -310,7 +321,7 @@ class AnthropicMessagesConfig(BaseAnthropicMessagesConfig): # Check for Anthropic OAuth token in Authorization header headers, api_key = optionally_handle_anthropic_oauth(headers=headers, api_key=api_key) - if "x-api-key" not in headers and "authorization" not in headers: + if not _carries_caller_credential(headers): self._apply_env_auth_header( headers, AnthropicModelInfo.get_auth_header( @@ -347,7 +358,7 @@ class AnthropicMessagesConfig(BaseAnthropicMessagesConfig): ) oauth_headers, oauth_api_key = optionally_handle_anthropic_oauth(headers=headers, api_key=api_key) - if "x-api-key" not in oauth_headers and "authorization" not in oauth_headers: + if not _carries_caller_credential(oauth_headers): self._apply_env_auth_header( oauth_headers, await AnthropicModelInfo.aget_auth_header( diff --git a/litellm/llms/anthropic/skills/transformation.py b/litellm/llms/anthropic/skills/transformation.py index a9f6fad3f98..be349463071 100644 --- a/litellm/llms/anthropic/skills/transformation.py +++ b/litellm/llms/anthropic/skills/transformation.py @@ -37,6 +37,7 @@ class AnthropicSkillsConfig(BaseSkillsAPIConfig): from litellm.llms.anthropic.common_utils import ( AnthropicModelInfo, merge_anthropic_beta_headers, + without_caller_credential_headers, ) auth_header: Final = AnthropicModelInfo.get_auth_header( @@ -52,12 +53,15 @@ class AnthropicSkillsConfig(BaseSkillsAPIConfig): merge_anthropic_beta_headers(headers.get("anthropic-beta"), auth_header.get("anthropic-beta")), ANTHROPIC_SKILLS_API_BETA_VERSION, ) - headers.update(auth_header) - headers["anthropic-version"] = "2023-06-01" - headers["anthropic-beta"] = merged_beta - headers["content-type"] = "application/json" - - return headers + # The deployment's own credential is applied here, so a caller-supplied one must not ride + # along upstream beside a minted federation Bearer. + return { # mutable-ok: validate_environment's contract returns a real dict, which httpx then consumes + **without_caller_credential_headers(headers), + **auth_header, + "anthropic-version": "2023-06-01", + "anthropic-beta": merged_beta, + "content-type": "application/json", + } def get_complete_url( self, diff --git a/litellm/llms/base_llm/auth/client_credentials.py b/litellm/llms/base_llm/auth/client_credentials.py index b06f69bff01..7df03a334b2 100644 --- a/litellm/llms/base_llm/auth/client_credentials.py +++ b/litellm/llms/base_llm/auth/client_credentials.py @@ -21,7 +21,7 @@ import httpx from pydantic import BaseModel, SecretStr, ValidationError from typing_extensions import assert_never -from litellm.llms.base_llm.auth.identity_source import KeycloakSource +from litellm.llms.base_llm.auth.identity_source import KeycloakSource, ref_for_error_message from litellm.llms.base_llm.auth.token_exchange import ( MAX_RESPONSE_BYTES, redact_oauth_error_body, @@ -140,7 +140,7 @@ def _prepared_request(config: KeycloakSource, client_secret: str) -> tuple[bytes def _resolve_client_secret(config: KeycloakSource, secret_reader: SecretReader) -> str: secret: Final = secret_reader(config.client_secret_ref) if not secret: - raise ValueError(f"keycloak client secret {config.client_secret_ref} could not be read") + raise ValueError(f"keycloak client secret {ref_for_error_message(config.client_secret_ref)} could not be read") return secret diff --git a/litellm/llms/base_llm/auth/identity_source.py b/litellm/llms/base_llm/auth/identity_source.py index 9b9df7783f9..f8f02249b3c 100644 --- a/litellm/llms/base_llm/auth/identity_source.py +++ b/litellm/llms/base_llm/auth/identity_source.py @@ -54,3 +54,23 @@ def identity_source_ref(config: AnthropicIdentitySourceConfig) -> str: whenever any field does, including a ``*_ref`` pointer NAME (never the secret it points to).""" digest: Final = hashlib.sha256(config.model_dump_json().encode()).hexdigest()[:_REF_HASH_HEX_LENGTH] return f"oidc/{config.kind.value}/{digest}" + + +_POINTER_REF_PREFIXES: Final = ( + "oidc/", + "os.environ/", + "hashicorp_vault/", + "aws_secret_manager/", + "google_secret_manager/", +) + + +def ref_for_error_message(ref: str) -> str: + """A ``*_ref`` rendered for an operator-facing error. + + Naming the pointer is deliberate: it is what tells an operator which setting failed to + resolve. But these fields only ever fail to resolve when what was written is not a pointer, + and an operator who pasted the secret itself has made the field's value the secret. So the + value is echoed only when it is recognizably a pointer, and withheld otherwise. + """ + return ref if ref.startswith(_POINTER_REF_PREFIXES) else "" diff --git a/litellm/llms/base_llm/auth/internal_issuer.py b/litellm/llms/base_llm/auth/internal_issuer.py index 6b350ef6330..444ca7fd5b2 100644 --- a/litellm/llms/base_llm/auth/internal_issuer.py +++ b/litellm/llms/base_llm/auth/internal_issuer.py @@ -14,7 +14,7 @@ from collections.abc import Callable, Mapping from types import MappingProxyType from typing import Final, TypeAlias -from litellm.llms.base_llm.auth.identity_source import InternalIssuerSource +from litellm.llms.base_llm.auth.identity_source import InternalIssuerSource, ref_for_error_message from litellm.llms.base_llm.auth.jwt_signing import jwks_document_json, sign_es256_jwt SigningKeyReader: TypeAlias = Callable[[str], str | None] # mutable-ok: Callable param-list syntax, not a list @@ -46,7 +46,9 @@ def _claims(config: InternalIssuerSource, issued_at: int) -> Mapping[str, object def _resolve_signing_key(config: InternalIssuerSource, key_reader: SigningKeyReader) -> str: pem: Final = key_reader(config.signing_key_ref) if not pem: - raise ValueError(f"internal_issuer signing key {config.signing_key_ref} could not be read") + raise ValueError( + f"internal_issuer signing key {ref_for_error_message(config.signing_key_ref)} could not be read" + ) return pem diff --git a/tests/test_litellm/llms/anthropic/test_anthropic_common_utils.py b/tests/test_litellm/llms/anthropic/test_anthropic_common_utils.py index 46bf44b77c6..12d97b200ae 100644 --- a/tests/test_litellm/llms/anthropic/test_anthropic_common_utils.py +++ b/tests/test_litellm/llms/anthropic/test_anthropic_common_utils.py @@ -2349,6 +2349,52 @@ class TestWifServerOwnedAuthHeaderStrip: assert all(caller_key not in value for value in headers.values()) assert headers["user-agent"] == "caller/1.0" + @pytest.mark.parametrize("header_name", PROXY_CREDENTIAL_HEADER_NAMES) + def test_skills_surface_strips_caller_credentials_too(self, monkeypatch, wif_engine, header_name): + """Skills builds its own headers as well; every minting surface needs the same strip.""" + from litellm.llms.anthropic.skills.transformation import AnthropicSkillsConfig + + for name, value in WIF_ENV.items(): + monkeypatch.setenv(name, value) + caller_key = "sk-litellm-CALLER-VIRTUAL-KEY" + + headers = AnthropicSkillsConfig().validate_environment( + headers={header_name.title(): caller_key, "user-agent": "caller/1.0"}, + litellm_params=None, + ) + + assert headers["authorization"] == f"Bearer {FAKE_MINTED_TOKEN}" + assert header_name == "authorization" or header_name not in {name.lower() for name in headers} + assert all(caller_key not in value for value in headers.values()) + assert headers["user-agent"] == "caller/1.0" + + def test_passthrough_honors_a_case_variant_caller_key_instead_of_minting(self, monkeypatch, wif_engine): + """The passthrough surface hands the caller's own credential upstream rather than minting. + That check was case-sensitive, so X-Api-Key slipped past it and the caller's key would have + travelled beside a minted Bearer.""" + from litellm.llms.anthropic.experimental_pass_through.messages.transformation import ( + AnthropicMessagesConfig, + ) + + for name, value in WIF_ENV.items(): + monkeypatch.setenv(name, value) + poster, _ = wif_engine + caller_key = "sk-ant-CALLER-SUPPLIED" + + headers, _ = AnthropicMessagesConfig().validate_anthropic_messages_environment( + headers={"X-Api-Key": caller_key}, + model="claude-sonnet-4-5", + messages=[], + optional_params={}, + litellm_params={}, + api_key=None, + api_base=None, + ) + + assert headers["X-Api-Key"] == caller_key + assert "authorization" not in {name.lower() for name in headers} + assert len(poster.requests) == 0 + @pytest.mark.parametrize("header_name", PROXY_CREDENTIAL_HEADER_NAMES) def test_batches_surface_strips_caller_credentials_too(self, monkeypatch, wif_engine, header_name): """Batches builds its own headers on the create path, so it needs the same strip: the diff --git a/tests/test_litellm/llms/base_llm/auth/test_client_credentials.py b/tests/test_litellm/llms/base_llm/auth/test_client_credentials.py index d58c2673c45..e006aa1a12a 100644 --- a/tests/test_litellm/llms/base_llm/auth/test_client_credentials.py +++ b/tests/test_litellm/llms/base_llm/auth/test_client_credentials.py @@ -412,3 +412,42 @@ class TestOversizedSuccessBody: fetch_keycloak_assertion( make_config(), poster=ScriptedPoster([oversized]), secret_reader=DEFAULT_SECRET_READER ) + + +class TestUnresolvedSecretRefIsNotEchoed: + """An operator who pastes the secret itself into the *_ref field turns that field INTO the + secret, and this error reaches model callers, so it must never echo the value.""" + + def test_keycloak_ref_value_is_not_in_the_error(self): + from litellm.llms.base_llm.auth.client_credentials import keycloak_assertion_source + from litellm.llms.base_llm.auth.identity_source import KeycloakSource + + pasted_secret = "sUp3r-s3cret-value-not-a-pointer" + config = KeycloakSource( + token_url="https://keycloak.example.com/realms/p/protocol/openid-connect/token", + client_id="litellm", + client_secret_ref=pasted_secret, + ) + + with pytest.raises(ValueError) as excinfo: + keycloak_assertion_source(config, secret_reader=lambda _ref: None)() + + assert pasted_secret not in str(excinfo.value) + assert "withheld" in str(excinfo.value) + + def test_internal_issuer_ref_value_is_not_in_the_error(self): + from litellm.llms.base_llm.auth.identity_source import InternalIssuerSource + from litellm.llms.base_llm.auth.internal_issuer import internal_issuer_assertion_source + + pasted_pem = "-----BEGIN PRIVATE KEY-----MIGHAgEA-----END PRIVATE KEY-----" + config = InternalIssuerSource( + issuer_url="https://proxy.example.com", + subject="litellm-proxy", + signing_key_ref=pasted_pem, + ) + + with pytest.raises(ValueError) as excinfo: + internal_issuer_assertion_source(config, key_reader=lambda _ref: None)() + + assert pasted_pem not in str(excinfo.value) + assert "withheld" in str(excinfo.value) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx index 04452271e65..fdc491c6a07 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx @@ -94,6 +94,13 @@ vi.mock("@/app/(dashboard)/hooks/providers/useProviderFields", () => ({ ], }, }, + { + provider: "OpenAI", + provider_display_name: "OpenAI", + litellm_provider: "openai", + default_model_placeholder: "gpt-4o", + credential_fields: [{ key: "api_key", label: "API Key", field_type: "password" }], + }, ], isLoading: false, error: null, @@ -394,6 +401,50 @@ describe("AddProviderPanel", () => { expect(credentialUpdateCall).not.toHaveBeenCalled(); }); + it("creates rather than PATCHes when the provider changes under the same credential name", async () => { + discoverProviderModelsCall.mockResolvedValue({ models: ["claude-3-opus"] }); + const { user } = await setup(); + + await chooseProvider(user, "Anthropic"); + await user.type(screen.getByLabelText("Credential name"), "shared-name"); + await user.click(screen.getByRole("button", { name: /Next/ })); + + await chooseSelectOption( + user, + await screen.findByRole("combobox", { name: "Authentication method" }), + "Workload Identity Federation (LiteLLM-signed)", + ); + fireEvent.change(await screen.findByLabelText("Organization ID"), { target: { value: "org-1" } }); + fireEvent.change(screen.getByLabelText("Issuer URL"), { target: { value: "https://proxy.example.com" } }); + fireEvent.change(screen.getByLabelText("Issuer Subject"), { target: { value: "litellm-proxy" } }); + fireEvent.change(screen.getByLabelText("Signing Key Reference"), { target: { value: "os.environ/SIGNING_KEY" } }); + await user.click(screen.getByRole("button", { name: "Save credential" })); + + // A credential is (name, provider). Keeping the name but switching provider must not PATCH the + // Anthropic credential into an OpenAI one. + await screen.findByText("Register this JWKS with Anthropic"); + await user.click(screen.getByRole("button", { name: /Back/ })); + await user.click(await screen.findByRole("button", { name: /Back/ })); + await chooseProvider(user, "OpenAI"); + await user.click(screen.getByRole("button", { name: /Next/ })); + + credentialCreateCall.mockClear(); + credentialUpdateCall.mockClear(); + await user.type(await screen.findByLabelText("API Key"), "sk-openai-test"); + await user.click(screen.getByRole("button", { name: /Save credential/ })); + + await waitFor(() => + expect(credentialCreateCall).toHaveBeenCalledWith( + "test-access-token", + expect.objectContaining({ + credential_name: "shared-name", + credential_info: { custom_llm_provider: "openai" }, + }), + ), + ); + expect(credentialUpdateCall).not.toHaveBeenCalled(); + }); + it("blocks creation while any model name is blank", async () => { discoverProviderModelsCall.mockResolvedValue({ models: ["claude-3-opus"] }); const { user } = await setup(); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.tsx index ccf5f65f4ec..0cb32fd0fb0 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.tsx @@ -90,7 +90,7 @@ export default function AddProviderPanel() { const [step, setStep] = React.useState("provider"); const [selectedProvider, setSelectedProvider] = React.useState(null); const [credentialName, setCredentialName] = React.useState(""); - const [savedCredentialName, setSavedCredentialName] = React.useState(null); + const [savedCredential, setSavedCredential] = React.useState<{ name: string; provider: string } | null>(null); const [savedValues, setSavedValues] = React.useState>({}); const [federationRuleId, setFederationRuleId] = React.useState(""); const [jwks, setJwks] = React.useState(null); @@ -125,11 +125,14 @@ export default function AddProviderPanel() { ); const litellmProvider = selectedProviderInfo?.litellm_provider ?? ""; - const credentialSaved = savedCredentialName !== null && savedCredentialName === credentialName; + // A credential is identified by name AND provider: switching provider under the same name must + // create a new one, not PATCH the previous provider's credential into a different provider. + const credentialSaved = + savedCredential !== null && savedCredential.name === credentialName && savedCredential.provider === litellmProvider; const nameCollision = credentialName.length > 0 && - credentialName !== savedCredentialName && + !credentialSaved && (credentialsResponse?.credentials ?? []).some((c) => c.credential_name === credentialName); const goTo = (next: WizardStep) => setStep(next); @@ -164,7 +167,7 @@ export default function AddProviderPanel() { await credentialUpdateCall(accessToken, credentialName, updatePayload); } setSavedValues(values); - setSavedCredentialName(credentialName); + setSavedCredential({ name: credentialName, provider: litellmProvider }); setFederationRuleId( typeof values.anthropic_federation_rule_id === "string" ? values.anthropic_federation_rule_id : "", );