mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
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.
This commit is contained in:
parent
b2fb6dd4bb
commit
dc35845d9e
9 changed files with 192 additions and 16 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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 "<withheld: not a secret reference>"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -90,7 +90,7 @@ export default function AddProviderPanel() {
|
|||
const [step, setStep] = React.useState<WizardStep>("provider");
|
||||
const [selectedProvider, setSelectedProvider] = React.useState<Providers | null>(null);
|
||||
const [credentialName, setCredentialName] = React.useState("");
|
||||
const [savedCredentialName, setSavedCredentialName] = React.useState<string | null>(null);
|
||||
const [savedCredential, setSavedCredential] = React.useState<{ name: string; provider: string } | null>(null);
|
||||
const [savedValues, setSavedValues] = React.useState<Record<string, unknown>>({});
|
||||
const [federationRuleId, setFederationRuleId] = React.useState("");
|
||||
const [jwks, setJwks] = React.useState<AnthropicJwks | null>(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 : "",
|
||||
);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue