Commit graph

6 commits

Author SHA1 Message Date
yucheng-berri
07b9ea8c3b
fix(anthropic): require caller api_key and SSRF-validate api_base in advisor tool (#32093)
* fix(anthropic): require caller api_key and SSRF-validate api_base in advisor tool

The advisor_20260301 interceptor honored a caller-supplied api_base once
allow_client_side_credentials was enabled, even without a caller-supplied
api_key. AnthropicModelInfo.get_auth_header() then fell back to the proxy's
own ANTHROPIC_API_KEY/ANTHROPIC_AUTH_TOKEN, so the server's real credentials
plus the conversation history got sent to a caller-chosen destination

_resolve_advisor_credentials() now only honors api_base alongside a
non-empty caller-supplied api_key, requires the https scheme, and validates
api_base via validate_url() before use, mirroring check_complete_credentials
in auth_utils.py. https is required because validate_url only DNS-pins the
connection for http; for https with TLS verification on it returns the URL
unchanged and relies on certificate validation to block DNS rebinding

* fix(anthropic): also reject advisor api_base when ssl_verify is disabled

validate_url only DNS-pins the connection for http, or for https with
litellm.ssl_verify disabled; the previous https-only check missed the
ssl_verify=False case, where validate_url's rewritten URL was still being
discarded, per Greptile's review of this PR. Reject api_base outright when
ssl_verify is False so the discarded rewrite can no longer matter
2026-07-04 12:06:09 -07:00
yucheng-berri
1667b8f740
fix: reject model_list in proxy body and gate advisor client credentials (#30585)
* fix: validate proxy request body and nested fields

Ensure caller-supplied request fields cannot override server-side deployment
configuration, and apply request-body validation consistently to nested
structures. Adjusts router kwarg handling and client-side credential handling
for base-url overrides

* test: cover router strip ordering and advisor clientside credential gate

* fix: clear deployment credentials on client base-url override

When a request overrides api_base/base_url, recompute the deployment's
litellm_params (clearing the deployment's own api_key) and drop the cached
client built for the original endpoint, so the deployment credential is not
reused for the client-supplied endpoint. Adds regression tests that assert the
credentials actually forwarded to litellm.completion/acompletion.

* fix(proxy): require api_key alongside api_base override

A request that overrides api_base/base_url but supplies no api_key still
left the proxy carrying a server credential: once the override clears the
banned-param opt-in, the provider re-resolves a key from the environment
(api_key or get_secret("OPENAI_API_KEY") and ~30 sibling chains in
main.py) and forwards it to the caller-controlled URL. Popping the
deployment api_key only changed which server key leaked.

Gate is_request_body_safe so a permitted api_base/base_url override must
also carry a non-empty caller api_key; reject otherwise. The env
resolution in main.py is left as the provider boundary.

* fix(proxy): extend request-body banlist with five additional credential and session targeting fields

Yuneng's review found five deployment-owned request-body params still missing
from the denylist and the router strip set. Each lets a caller reach the
operator's provider credentials or retarget the outbound request:
aws_profile_name selects a local AWS profile, oci_compartment_id and oci_region
retarget the OCI request, litellm_credential_name selects any server-loaded
credential by name with no ownership check, and runtimeSessionId resumes a
Bedrock AgentCore runtime session (AWS does not enforce session-to-user
mapping, so this is a cross-tenant session-resume vector).

Add all five to _BANNED_REQUEST_BODY_PARAMS in auth_utils.py and to
_DEPLOYMENT_OWNED_CREDENTIAL_KWARGS in router.py. Deployment litellm_params and
SDK direct calls are unaffected: the banlist gates the request body only, and
the router strip drops caller kwargs, never deployment["litellm_params"].

* test: rename arbitrary canary values in security tests to neutral placeholders

* fix(proxy): apply api_key co-presence to nested base override and warn on Router credential strip

P1-A: is_request_body_safe descended into _NESTED_CONFIG_KEYS
(litellm_embedding_config, extra_body) for the banned-param check but not for
the api_key co-presence check, so a base override smuggled into one of those
nested dicts cleared the client-side-credentials opt-in without a paired
api_key and let the provider re-resolve a server credential from the
environment. Run _check_base_override_has_api_key on each nested config dict
too, so the requirement applies wherever a base override is permitted.

P1-B: the deployment-owned credential strip in the Router runs unconditionally
on every _completion/_acompletion, which is security-correct but silently
drops per-call api_version/vertex_project/etc. for SDK Router callers. Emit a
single warning (key names only, never values) when the strip removes a
non-empty value, so the backwards-incompatible behavior is visible without
gating the strip on a context flag that does not exist.

* fix(proxy): apply api_key co-presence to tool-entry base override

is_request_body_safe scans three surfaces (root, _NESTED_CONFIG_KEYS, and
tools[]); the previous commit extended the api_key co-presence rule to root
and nested config dicts but not to tool entries. With
allow_client_side_credentials enabled, a tool entry carrying api_base/base_url
and no paired api_key cleared the gate, letting a provider interceptor fall
back to a server-side credential for a caller-controlled URL. Add the same
_check_base_override_has_api_key call to each tool dict and its nested function
dict, mirroring the symmetry already applied to the nested config keys. The
rule is unchanged: api_key must live in the same dict as the base override it
accompanies.

* test(proxy/auth): require paired api_key under extra_body opt-in

* fix(router): gate deployment-owned kwarg strip on litellm.proxy_is_running

* fix(advisor): narrow proxy-import guard to ImportError-family

* fix(router): gate api_key clear on base override behind litellm.proxy_is_running

* test(proxy/auth): scope proxy_is_running flag to dynamic-params class with autouse fixture

* style: use built-in generics in PR-added type annotations

* revert: drop proxy_is_running flag and router-level credential strip; rely on proxy gate

* revert: scope PR to LIT-3828 + LIT-3834 only; drop LIT-3830/LIT-3833 changes

* style: black-format advisor orchestration test
2026-06-22 11:34:20 -07:00
Ishaan Jaffer
fa5258466d
test(advisor): add unit tests for max_uses=0, missing model, default fallback 2026-04-11 18:16:56 -07:00
Ishaan Jaffer
844e34b68b
test(advisor): remove live e2e test file (tests run locally via script) 2026-04-11 17:52:06 -07:00
Ishaan Jaffer
742e2fe1aa
test(advisor): add live e2e tests for advisor orchestration against real proxy 2026-04-11 17:46:17 -07:00
Ishaan Jaffer
ce3d039bcd
test(advisor): add unit tests for orchestration loop (mocked backends, 8 tests) 2026-04-11 17:43:26 -07:00