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.
Add explicit warnings when Presidio apply_to_output skips masking for mixed/unknown stream event shapes, and add regression coverage for the mixed stream flush path to ensure chunk order is preserved and warnings are emitted.
Made-with: Cursor
Keep Presidio apply-to-output streaming in passthrough mode when responses-style events are emitted, preventing dropped lifecycle events that caused Codex stream disconnects. Add a regression test to assert unknown streaming events are preserved in order.
Made-with: Cursor
The redirect-following added to async_safe_get checks response.is_redirect
on every hop. Two vertex batch tests stub AsyncHTTPHandler.get with a bare
MagicMock, whose default-truthy is_redirect made the redirect path fire,
then crashed in httpx.URL().join() because headers.get('location') was
also a MagicMock instead of a string. Set is_redirect=False explicitly so
the mocked response models a non-redirect terminal response.
Also tighten _extract_redirect_url to raise SSRFError on non-string
Location values (defense-in-depth — a real httpx Response always returns
str|None, but this avoids a confusing TypeError if anything else ever
slips through).
This is an unrelated CI fix piggybacked on the SCIM PR to unblock the
batches test suite.
Root cause: admin_viewer_routes was an explicit allowlist, so every newly-added
GET endpoint anywhere in the codebase silently 403'd for admin viewer until
someone remembered to add it. We had whacked /spend/logs/ui, /customer/list,
/guardrails/list, /policies/attachments/list, /invitation/info, and several
others in serial — but the next round still surfaced /in_product_nudges,
/health/latest, /credentials, /v1/mcp/network/client-ip, /claude-code/plugins,
/policy/templates. This pattern keeps repeating because the model is wrong.
Structural fix in `_check_proxy_admin_viewer_access`:
- Default-allow safe HTTP methods (GET / HEAD / OPTIONS) on any
non-inference route. Admin Viewer's principle is read parity with
Proxy Admin; HTTP semantics already mark GET as side-effect-free, so
using the method as the allow signal is the correct primitive.
- Unsafe methods (POST/PUT/PATCH/DELETE) still go through the existing
explicit allowlists + the hard-blocked write set
(/user/new, /team/new, /key/generate, …).
- LLM/inference routes still 403 (cost-incurring).
The existing admin_viewer_routes list is retained as a backstop for the
small set of routes implemented as POST but semantically read (e.g.
/spend/calculate). Adding new GET endpoints no longer requires touching
this list.
Models page tab/panel off-by-one (UI bug for Admin Viewer):
Tremor's TabList filters falsy children but TabPanels does not, so
conditionally hiding "Add Model" with `{!shouldHideAddModelTab && ...}`
left a phantom panel slot — clicking "LLM Credentials" showed nothing,
and clicking "Pass-Through Endpoints" showed the credentials panel.
Refactor to a single source-of-truth `visibleTabs` array; tab and
panel indices now can never desync.
Tests:
- 12 parametrized tests covering the 6 user-reported endpoints + 4
hypothetical-future endpoints + 2 already-fixed ones, all asserting
Admin Viewer GET succeeds via the default-allow path (no allowlist
entry needed).
- 5 parametrized tests for POST writes still 403'ing
(random-future-write, /user/new, /team/new, /key/generate, /model/new).
- All 207 existing route_checks tests still pass — backward-compatible.
User reported six more 403s and "still restricts access to keys + models" after
the first round. Root causes:
1. Six read endpoints were missing from admin_viewer_routes:
- /guardrails/list, /v2/guardrails/list (Guardrails page)
- /guardrails/submissions, /guardrails/submissions/{guardrail_id}
- /guardrails/usage/overview (Guardrails Monitor page)
- /policies/attachments/list (Policies page)
- /get/mcp_semantic_filter_settings (Settings page)
2. /guardrails/submissions handler treated admin viewer as non-admin, filtering
them to only their team submissions. Switch to _user_has_admin_view() so
admin viewer sees all submissions (read parity with Proxy Admin).
3. UI Keys page (user_dashboard.tsx) and Models page (ModelsAndEndpointsView.tsx)
each had a hard "Access Denied" block specifically for "Admin Viewer" — a
leftover from the pre-parity era. Remove the blocks; gate the "Create Key"
button on the Keys page so admin viewer can read keys but not mint them.
Also drop the post-login redirect that forced admin viewers to /usage on
sign-in (page.tsx).
Tests:
- Extend ADMIN_VIEWER_SETTINGS_ROUTES parametrize list to cover all 7 new
routes (route-checks layer is now the layer production traffic actually
hits, vs. the dependency-override-bypass that was masking the gap).
The Prisma schema declares LiteLLM_VerificationToken.blocked as a
nullable Boolean with no default, so virtual keys created via the
key management endpoint persist with blocked=NULL. SQL equality
(`blocked = false`) never matches NULL rows, so the previous
`where={'blocked': not blocked}` filter silently skipped virtually
all real keys when SCIM tried to block them. This made SCIM
deprovisioning a no-op — and especially dangerous in DELETE flows
where the user row is removed afterwards, leaving orphaned but
fully-functional keys.
Match both `False` and `None` when blocking, and only `True`
when unblocking, so the state flip (and cache invalidation) actually
fires for the keys it should.
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>
