Replaces the previous intersect-with-team.access_group_ids check, which
made the override unreachable in practice (the team-gate fallback already
covered every case the intersection allowed). The override now resolves
each of the key's access_group_ids via get_access_object and accepts the
group only if its assigned_team_ids includes the key's team_id, or its
assigned_key_ids includes the key's token. This fulfills the original ask
(a key can extend a team's allow-list via a group the admin granted to
that team or that specific key) while still rejecting foreign groups
referenced by team members of other teams.
`os.path.relpath` with no `start` arg uses the current working
directory, so running pytest from a subdirectory produced a
different Redis key than running from the repo root. CI-recorded
cassettes and locally-replayed runs would silently miss each
other's cache.
Anchor the path to the repo root (derived from `__file__`) so the
key is stable regardless of CWD.
https://claude.ai/code/session_018uCx7pcrkdUJZrCVMaTdPx
Greptile P1: the unsafe-method branch of `_check_proxy_admin_viewer_access`
ended with a blanket `if route in management_routes: return`. That set is a
mix of reads (info/list — handled via the safe-method GET branch above) and
writes. The fallback let Admin Viewer POST to write endpoints not enumerated
in `_ADMIN_VIEWER_BLOCKED_WRITE_ROUTES`, including:
- /team/block, /team/unblock, /team/permissions_update
- /jwt/key/mapping/{new,update,delete}
- /key/bulk_update
- /key/{key_id}/reset_spend
Remove the fallback. The two remaining allow sets (admin_viewer_routes and
global_spend_tracking_routes) are both read-only, so removal does not affect
the legitimate POST-as-read cases (e.g. /spend/calculate, which is in
spend_tracking_routes ⊂ admin_viewer_routes).
Tests:
- 8 new parametrized cases pinning each previously-leaking management write
endpoint to 403 on POST for PROXY_ADMIN_VIEW_ONLY.
The reservation path (PR #26845) atomically pre-fills `spend:user:{user_id}`
and admits at the strict-`<` boundary. The legacy `_PROXY_MaxBudgetLimiter`
pre-call hook re-reads the same counter with `>=`, so a reservation that
fills the counter to exactly `max_budget` (e.g. a request without a
`max_tokens` cap that falls back to reserving the smallest remaining
headroom) is rejected by the hook even though the reservation already
admitted it.
Skip the hook when the request's active `budget_reservation` covers
`spend:user:{user_id}`. The reservation is the source of truth for that
counter cross-pod; the legacy `>=` path remains in place for requests
without a reservation (e.g. paths that bypass the reservation entirely).
Reproduces as `tests/otel_tests/test_prometheus.py::test_user_budget_metrics`
on a fresh user with `max_budget=10` calling `fake-openai-endpoint` without
`max_tokens`. Adds focused unit coverage in
`tests/test_litellm/proxy/hooks/test_max_budget_limiter.py`.
A team member could set any access_group_ids on their key (e.g. a group
assigned only to a different team) and override the team's model
restriction. Intersect the key's access_group_ids with team_object.access_group_ids
in _key_access_group_grants_model so foreign groups are dropped before
model expansion. Adds a regression test that asserts expansion is never
called for foreign groups.
Greptile flagged two follow-ups on the OpenAPI/local-registry pre-call
check:
1. **P1 runtime crash via None proxy_logging_obj.**
`kwargs.get("proxy_logging_obj")` is `None` on the MCP entry path,
and `pre_call_tool_check` calls `proxy_logging_obj._create_mcp_request_object_from_kwargs`
unconditionally after the security checks, which would have crashed
every legitimate call with `AttributeError`. Source the logging
object from `litellm.proxy.proxy_server` the same way
`_handle_managed_mcp_tool` already does.
2. **P2 authorization-bypass window when mcp_server is None.**
Previously the new check was guarded by `if mcp_server is not None`,
so any local tool whose registry entry had no resolvable server (a
startup-race window before `_initialize_tool_name_to_mcp_server_name_mapping`
completes, or an orphaned registry entry) ran without the security
check. Tools registered via openapi_to_mcp_generator are always tied
to a server, so a missing one is a configuration/timing fault — fail
the call with 503 instead of dispatching unguarded.
Tests: existing two pass with an added assertion that
`proxy_logging_obj` is non-None at the call site, plus a new test that
covers the 503 deny branch when the tool→server mapping is missing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The endpoint builder in BedrockCountTokensConfig.get_bedrock_count_tokens_endpoint
percent-encodes the model id as a single path segment (d4dd865b1a, path-traversal
hardening). Update the four endpoint-URL assertions in TestBedrockCountTokensEndpoint
to expect `amazon.nova-lite-v1%3A0` instead of the literal `:0`, matching production
behavior already covered by test_count_tokens_endpoint_encodes_model_id.
Greptile flagged that the new `created_by` fallback in
`/agent/daily/activity` resolves to `WHERE created_by IS NULL` when
`user_api_key_dict.user_id` is `None`, which would expose every
ownerless agent's rows to a service-account-style caller without a
user_id. Skip the fallback query entirely in that case so the caller
is treated as having no permitted agents (empty page, no DB hit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`execute_mcp_tool` dispatches in two ways: managed MCP servers go
through `_handle_managed_mcp_tool`, which calls
`MCPServerManager.pre_call_tool_check` to enforce allowed/banned tool
lists, key/team `object_permission` tool grants, and parameter
validation. OpenAPI-backed tools, however, were resolved via
`global_mcp_tool_registry` and dispatched directly to
`_handle_local_mcp_tool` — entirely skipping `pre_call_tool_check`.
A caller could invoke any registered OpenAPI tool regardless of their
key/team permissions, including administrative or destructive
operations on the upstream API.
Run `pre_call_tool_check` before the local-registry dispatch whenever
the resolved server is set (the same condition used to surface server
context to the managed path). Honor any guardrail-modified arguments
the hook returns. Errors raised by the hook propagate up before
`_handle_local_mcp_tool` runs.
Tests cover both directions: the pre-call hook fires when the local
tool resolves alongside a server, and a hook-raised HTTPException
prevents the local handler from being invoked.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related cross-tenant leaks in the daily-activity endpoints:
1. `/team/daily/activity` set a single `has_full_team_view` flag that
went True if the caller was admin/perm-holder of ANY one of the
requested teams. An admin of team A could pass `team_ids=A,B` and
read team B's per-API-key breakdown even when they were only a
plain member of team B. Require admin/permission on EVERY requested
team for the unfiltered view; otherwise force fallback to the
caller's own API keys for the whole request. Callers wanting wider
coverage can split into separate requests.
2. `/agent/daily/activity` initialized an empty `where_condition` and
returned every agent's spend/token rows on the proxy when
`agent_ids` was omitted — the dashboard's "Top Agents Driving
Spend" panel triggered this for any authenticated user. For
non-admin callers, scope the query to agents they're permitted to
invoke (`AgentRequestHandler.get_allowed_agents`) or, when their
key/team has no explicit agent allowlist, to agents they created
(`created_by`). Explicit `agent_ids` is intersected with the same
permitted set rather than trusted. When the resolved set is empty,
return an empty paginated page without issuing the query.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`get_daily_spend_from_prometheus` was interpolating the `api_key`
query parameter into a PromQL `hashed_api_key="..."` label matcher
with an f-string. Any caller of `/global/spend/logs` could inject a
bare `"` to terminate the matcher and append arbitrary PromQL
operators or extra metric selectors, exfiltrating cross-tenant
telemetry from the connected Prometheus instance.
Replace the f-string with `_quote_promql_string_literal`, which uses
`json.dumps` to render a complete Go-compatible double-quoted literal.
PromQL string literals follow Go's escape rules per
https://prometheus.io/docs/prometheus/latest/querying/basics/, and
JSON's quoting is a strict subset, so the same escape covers
backslash, embedded quote, and control-character cases without rolling
a bespoke escape table.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove explanatory comments that restated what the code already says.
Kept only those that document non-obvious external contracts (the aiohttp
record-path patch's reason for re-feeding the body, and the warning
messages inside save_cassette that reach the user).
Two related authorization gaps in management endpoints:
1. `/project/update` evaluated permission against the team_id supplied in
the request body. By passing `data.team_id` pointing at a team they
admin, a caller could hijack any project — `_check_user_permission_for_project`
was given the attacker's team_object and happily checked admin
membership against that. Drop the team_object kwarg so the helper
re-fetches the existing project's team. Also require admin rights on
the destination team when reassigning a project across teams, so a
team admin cannot shed projects into another team's namespace.
2. `/key/update` accepted any `organization_id` and only checked that
the org existed before applying limits. A caller could thereby point
their key at an arbitrary org. Add `_validate_caller_can_assign_key_org`
which enforces the same membership rule already applied on the
`/key/list` filter path (`validate_key_list_check`); proxy admins and
no-change updates skip the check.
Tests cover both helpers in isolation: existing-team-admin allow,
unrelated-team admin deny, proxy-admin shortcut, org-member allow,
non-member deny, missing user_id deny, no-memberships deny.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous attempt wrote to sys.__stderr__ from the test fixture. Under
xdist, fixtures run inside worker subprocesses whose stderr is captured
by the controller and only released to the live log on test failure —
so passing tests' verdicts were silently swallowed.
Round-trip via report.user_properties: the worker-side fixture stashes
the verdict on user_properties, xdist serializes it onto the report,
and a controller-side pytest_runtest_logreport hook writes it via the
TerminalReporter (the same plugin that emits PASSED/FAILED markers).
TerminalReporter is resolved lazily on first hook call because it's
not yet registered when conftest's pytest_configure runs.
Verified locally in both serial and xdist modes.
The GitHub merge conflict resolver concatenated both test sets but left
`from litellm.proxy._lazy_openapi_snapshot import _normalize_operation_ids`
stranded between functions instead of at the top of the file.
Raw github serves application/octet-stream which OpenAI/Gemini reject
when LiteLLM fetches the URL client-side. jsDelivr serves the same
file with content-type: application/pdf. Pin to a commit SHA so the
asset is immutable and jsDelivr can cache it for a year.
Previously, the per-test [VCR HIT/MISS/...] line was written via
TerminalReporter.write_line from inside fixture teardown. Pytest
captures that stream by default and only surfaces it on FAILED tests
(under 'Captured stdout teardown'), so passing tests' verdicts were
invisible in CI logs and the user couldn't tell whether the cache
was working.
Write directly to sys.__stderr__ so the line bypasses pytest's
capture entirely. Under xdist each worker has its own __stderr__
which CircleCI aggregates into the live job log alongside the
PASSED/FAILED markers.
