Cover the full litellm.rerank()/arerank() path with HTTP mocked, asserting
metadata.requester_metadata reaches the Discovery Engine :rank body as
userLabels (and stays absent when no metadata is set). Catches plumbing
regressions that unit tests on transform_rerank_request alone would miss.
The wrapper had no production callers after transform_parsed_response
was refactored to call _resolve_json_mode_non_streaming directly.
Updated the parametrized test to call the underlying method.
Two Greptile P2s addressed:
1. (security) The audit-log row for an ``add_team_callbacks`` call would
serialize the entire ``callback_vars`` block — including
``langfuse_secret_key``, ``langsmith_api_key``, and the GCS service
account path — verbatim into ``LiteLLM_AuditLogs``. Anyone with read
access to the audit table could harvest team callback credentials.
Same risk for ``disable_team_logging`` when the team's existing row
has populated ``callback_settings.callback_vars``.
Add ``_redact_callback_secrets``: deep-copies the metadata snapshot
and replaces every ``callback_vars`` value with ``***REDACTED***``.
The keys are kept so an auditor can still see *which* fields
changed. Applied to both before and after snapshots.
2. ``asyncio.create_task`` is fire-and-forget; if the audit-log write
raises (transient DB error etc.) the exception is silently
discarded by the event loop and the audit row is just missing —
the exact gap this PR is closing. Attach a ``done_callback`` that
logs the exception at warning level via ``verbose_proxy_logger`` so
the operator sees there's a gap.
Tests assert that callback values are not present in the serialized
audit payload (both for ``add_team_callbacks`` and for
``disable_team_logging`` when the team's existing row has
populated secrets).
The two mutating endpoints in team_callback_endpoints.py
(``/team/{id}/callback`` POST and ``/team/{id}/disable_logging``) wrote
team metadata without emitting an audit-log row. The disable variant
is the worst case: a logging-control action that itself isn't logged,
so an admin (or compromised admin) could zero out a team's
observability with no forensic trail.
Add ``_emit_team_callback_audit_log`` mirroring the
``store_audit_logs``-gated pattern already used in team_endpoints.py
for /team/new and /team/update. When ``litellm.store_audit_logs`` is
True, both endpoints now emit an ``LiteLLM_AuditLogs`` row capturing
the calling user, the API key, and the before/after team metadata.
When the flag is False the helper is a no-op, so non-Enterprise
deployments are unaffected.
The ``litellm_changed_by`` header is now also accepted on
``/team/{id}/disable_logging`` to match the existing
``add_team_callbacks`` shape; the header is optional so existing
callers are unaffected.
Variant scope: the file has three endpoints — both mutating variants
are now logged. The read-only ``GET /team/{id}/callback`` is unchanged.
Other unlogged callback / logging-control admin endpoints elsewhere in
the proxy (e.g. ``/cache/settings``, ``/config_overrides/hashicorp_vault``)
are out of scope here and would be addressed in a separate PR.
Tests cover both endpoints in both ``store_audit_logs`` states and
verify that the captured before/after metadata reflects the actual
mutation, plus that the ``litellm-changed-by`` header overrides the
auth user_id when supplied.
Greptile P2: the bypass removal in update_team_member_permissions had
no dedicated regression test. Adds an integration-style test that
posts to /team/permissions_update as a non-admin caller while
``_is_available_team`` is mocked True, and asserts a 403 — pinning
the bypass-removal against future regressions in the same way the
new member-add unit tests pin the self-join enforcement.
Two paths previously treated ``_is_available_team`` as a blanket
authorization bypass — the function was meant to let standard users
self-join a public team but was wired into the broader admin gate
without bounding the action being performed. Three concrete
exposures resulted:
1. ``/team/member_add``: the bypass let an unprivileged caller add
themselves as a Team Admin, or add an arbitrary other ``user_id``
into the team.
2. ``/team/permissions_update``: the same bypass let any authenticated
user overwrite a team's ``team_member_permissions`` array, mutating
the access policy for every member.
3. (Read endpoint ``/team/permissions_list`` is unchanged — it leaks
read-only policy state to non-members but is out of scope of the
advisory's recommendation; tracking separately.)
This commit:
- Splits ``_validate_team_member_add_permissions`` into early-return
admin checks followed by an available-team self-join branch that
enforces ``member.user_id == caller.user_id`` AND
``member.role == "user"`` for every member entry in the request.
The bulk shape (``member: List[Member]``) is checked the same way,
so a list with one valid self-entry plus one ``role=admin`` entry
is rejected. Email-only members are rejected on the self-join
path: matching by ``user_id`` is the only safe primitive at
pre-validation time (resolving email→user_id earlier would let
unauthenticated callers probe user existence).
- Removes the ``_is_available_team`` clause from
``update_team_member_permissions`` entirely. Only proxy / team /
org admins can update permission policies.
Tests:
- Update the two existing ``_validate_team_member_add_permissions``
unit tests to pass the new ``data`` argument.
- Add six regression tests covering the privesc shape (role=admin),
the cross-user-injection shape (other user_id), the no-caller-uid
fail-closed case, the email-only rejection, and the bulk shape.
- ``test_team_endpoints.py`` 133/133 pass.
PR #26484 substitutes LITELLM_PROXY_MASTER_KEY_ALIAS for
hash_token(master_key) in UserAPIKeyAuth so the master key (or its
hash) never reaches spend logs / metrics. The otel prometheus tests
still hardcoded the SHA-256 of "sk-1234"
("88dc28d0f030c55ed4ab77ed8faf098196cb1c05df778539800c9f1243fe6b4b"),
so the metric labels no longer matched and test_proxy_failure_metrics
failed. Reference the alias constant directly.
https://claude.ai/code/session_01UkzyZKiADEkZDbZFwB98yV
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Three follow-ups to the OAuth-discovery SSRF guard:
1. Greptile P1 (redirect bypass): the validated origin could return a
3xx whose ``Location`` points at an internal address, and httpx
would follow without re-checking the new target. Pass
``follow_redirects=False`` to both gated httpx GETs. Spec-compliant
OAuth/OIDC metadata endpoints serve the JSON directly, so this
doesn't affect legitimate providers.
2. Greptile P2 (empty getaddrinfo): POSIX doesn't strictly forbid an
empty success-list from ``getaddrinfo``. Add an explicit
``if not infos: return False`` so the guard fails closed instead of
falling through to ``return True``.
3. Mypy: ``info[4][0]`` is typed ``str | int``; narrow at the
boundary with an ``isinstance`` check (fail-closed if non-str).
Adds two regression tests verifying ``follow_redirects=False`` is
passed at both gated fetch sites, and one verifying the empty-list
case rejects the URL.
npm's `min-release-age` config has type `[null, Number]`. The value `3d`
parses to NaN, which propagates into `before = new Date(NaN)` (Invalid
Date). Pacote then calls `.toISOString()` on it and throws
`RangeError: Invalid time value`, breaking every local `npm install`.
Drop the `d` suffix in all six `.npmrc` files. The `<days>` in npm's
type hint is a label, not part of the value.
This is a no-op for CI (`npm ci` ignores this setting per the comment
in the file) but unblocks local `npm install`.
The OAuth discovery code in mcp_server_manager followed two
attacker-influenceable URLs without validation: the
``resource_metadata`` URL parsed out of a ``WWW-Authenticate``
challenge, and the ``authorization_servers[0]`` field of the
PRM JSON returned by the resource server. A malicious MCP server
could point those at a cloud-instance-metadata service, an internal
admin panel, or a loopback debug endpoint and the proxy would issue
a blind GET on its behalf.
Add ``_is_safe_metadata_url(url, server_url)`` and gate both follow-
up fetch sites on it. A URL is allowed when:
- it shares scheme + host + port with ``server_url`` (well-known
endpoints constructed from the admin's URL, and PRM published at
the resource server itself per RFC 9728 §3.3), or
- it resolves to publicly-routable IPs only (covers federated
authorization servers — Azure Entra, Google, Okta, GitHub —
hosted cross-origin from the resource server).
URLs that resolve to private / loopback / link-local / cloud-metadata
addresses, or that don't resolve at all, are rejected. ``http`` and
``https`` are the only schemes accepted. The IP block list is
provided by the existing ``_is_blocked_ip`` helper from
``litellm_core_utils.url_utils`` so the policy stays consistent with
the rest of the proxy.
The guard does not protect against active DNS rebinding between
this resolution and the subsequent httpx GET — the same-authority
pin remains the primary mitigation; the IP check is defence in
depth. The surface only triggers on config load / add-server, not
per request, so the synchronous ``getaddrinfo`` is acceptable.
Threads ``server_url`` through ``_fetch_oauth_metadata_from_resource``,
``_fetch_authorization_server_metadata``, and
``_fetch_single_authorization_server_metadata``. Existing tests for
those helpers updated for the new signature; new
``TestOAuthDiscoverySSRFGuard`` covers same-authority allow,
private-IP rejection across IPv4 and IPv6, multi-A-record dual-
stack rejection, unresolvable hosts, non-http schemes, and
end-to-end "no network call when guard denies".
Strip module-level docstrings and per-test/per-block prose from the
LIT-2642 fix and tests. Keep one short comment in each streaming site
that flags the GeneratorExit-vs-Exception subtlety, since that's the
non-obvious reason the flush lives in finally rather than after the loop.
Pure cleanup; no behavior change. All 12 regression tests still pass.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
When a client disconnects mid-stream from a Bedrock pass-through endpoint,
Starlette calls aclose() on the async generator, raising GeneratorExit
(a BaseException, not Exception) at the suspended yield. The previous
`except Exception` blocks in _async_streaming/_sync_streaming
(litellm/passthrough/main.py) and PassThroughStreamingHandler.chunk_processor
did not catch GeneratorExit, so the post-loop flush that hands collected
raw bytes to async_flush_passthrough_collected_chunks /
_route_streaming_logging_to_handler never ran. All per-chunk usage data
was silently dropped, undercounting spend for interrupted Bedrock invoke
and converse streams.
Move the flush into a finally block in all three sites and guard with a
`flush_scheduled` flag so the success path still flushes exactly once.
Also pull raise_for_status() out of the chunk-collection try block in
_async_streaming so 4xx/5xx responses still raise and don't enter the
flush path with zero bytes (preserving the behavior tested by
test_async_streaming_error_propagation.py).
Add regression coverage:
- test_async_streaming_flushes_on_client_disconnect
- test_async_streaming_flushes_on_upstream_exception_with_partial_data
- test_sync_streaming_flushes_on_early_close
- test_chunk_processor_logs_on_client_disconnect
plus baseline tests for normal completion and the 4xx no-flush path.
Fixes LIT-2642.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
After dedaf74a5e, _async_retrieve_batch wraps the GET in async_safe_get,
which inspects response.is_redirect. test_avertex_batch_prediction's
MagicMock response left is_redirect unset, so it auto-generated a truthy
mock, sent the redirect-follow loop into _extract_redirect_url, and
httpx.URL().join(<MagicMock>) raised TypeError. Set is_redirect=False
so the response is treated as terminal.
Greptile P1: this PR encrypts LiteLLM_MCPUserCredentials rows under the
salt key, but the /key/regenerate rotation endpoint had no
corresponding step for that table. Rotating the master key would
leave every BYOK and OAuth2 user credential permanently unreadable.
Adds rotate_mcp_user_credentials_master_key, mirroring the existing
rotate_mcp_server_credentials_master_key pattern: read each row with
the current key (via _decode_user_credential, which also handles
unmigrated legacy plaintext rows), re-encrypt under the new master
key, write back. One bad row is logged and skipped instead of
aborting the whole rotation.
Wired into key_management_endpoints.py as step 4b, alongside the
existing server-credentials rotation, with the same try/except shape
so a transient DB error on this table doesn't kill the whole
regenerate-key flow.
Tests cover: round-trip through rotation under a new key, automatic
re-encryption of legacy plaintext rows (rotation also acts as a
migration trigger), and a corrupt row not aborting the rotation.
Three minor fixes from Greptile review:
1. _decode_user_credential now also catches TypeError so a null
credential_b64 value returns None instead of propagating, matching
the documented "returns None when neither path yields a valid
string" contract.
2. The OAuth2 BYOK guard error no longer claims the existing row is a
BYOK credential — after a salt-key rotation, an OAuth2 row can fail
to decrypt and reach the same guard. Reword to "could not be
verified as an OAuth2 token", which is accurate for both cases.
3. Drop the no-op sys.path.insert in the new test file (other tests
in the directory don't need it; pytest picks up the package via
the installed editable wheel).
Adds a regression test for the None-input case.
