The default-allow-GET fix in route_checks unblocked the route layer, but a
second class of bug remained: handlers that gate on `user_role !=
PROXY_ADMIN` (or a private `_require_proxy_admin` helper) reject admin
viewer at the handler before the route's HTTP method even matters.
Backend: relax handler role checks on read endpoints to allow
PROXY_ADMIN_VIEW_ONLY (same `_user_has_admin_view` helper used elsewhere).
- /v1/access_group GET (list) + /v1/access_group/{id} GET — split
`_require_proxy_admin` into a parallel `_require_admin_view` for the
two read handlers; writes (POST / PUT / DELETE) keep the strict gate.
- /cloudzero/settings GET, /vantage/settings GET — read-only views.
- /config_overrides/hashicorp_vault GET — read-only config view.
- /team/permissions_list GET — let admin viewer see permissions like
a Proxy Admin would.
- /jwt/key/mapping/list, /jwt/key/mapping/info — JWT mapping reads.
- /v1/mcp/discover, /v1/mcp/openapi-registry — MCP picker views.
- /schedule/anthropic_beta_headers_reload/status — read-only status.
- /adaptive_router/state — read-only live snapshot.
UI: hide write buttons that admin viewer should not see (button click
would fail the backend write gate, but the UX expectation is no button).
- Internal Users: hide "Invite User" button.
- Access Groups: hide "Create Access Group" + Delete row action.
- Budgets: hide "+ Create Budget" + Edit/Delete row actions.
- Prompts: hide "+ Add New Prompt" / "Upload .prompt File"; gate the
prompt-table Edit/Delete actions on `isProxyAdminRole` (was
`isAdminRole` which incorrectly included admin viewer).
- Router Settings → Fallbacks: hide AddFallbacks panel + per-row Test
+ Delete actions.
- AI Hub: hide "Select Models / Agents / MCP Servers / Skills to Make
Public" + "Useful Links Management" (writes).
These pages remain VISIBLE for admin viewer (read parity); only the
write entry points are hidden.
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.
The openai SDK returns ResponseOutputMessage and ResponseOutputText as
raw Pydantic v2 models that lack .get() (unlike LiteLLM's own wrapper
objects). Add a _to_dict() helper that normalizes plain dicts,
BaseLiteLLMOpenAIResponseObject (has .get()), and raw Pydantic models
(has .model_dump()) into a consistent dict interface.
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".
Address Greptile feedback: the bare `except Exception: pass` in the
finally blocks of _sync_streaming / _async_streaming silently dropped
errors from executor.submit() / asyncio.create_task() (e.g. saturated
thread pool, closed event loop). Since the entire point of the fix is
that spend tracking should not silently lose data, mirror the peer
streaming_handler.py logging pattern so any scheduling failure is
diagnosable in production.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
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>
