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.
A non-admin scoped to ["model-a"] could call /health?model_id=id-b
(where id-b belongs to a deployment outside their scope) and the
background-cache code path would return id-b's cached health entry. The
helper returned {model_id} unconditionally, so the cache filter was
driven by an unvalidated id and the global cache leaked the entry — the
ternary `targeted_ids if not None else allowed_model_ids` skipped any
intersection with the caller's allowed deployments.
Make _resolve_targeted_model_ids walk the supplied model_list for both
the model and model_id branches. Callers pass an already-scoped list
(filtered to allowed model_names for non-admins, full list for admins),
so an out-of-scope model_id resolves to an empty set and the cache
filter drops every entry — matching the live path's existing behavior.
When JWT auth is enabled but `JWT_AUDIENCE` is unset, `auth_jwt`
disabled audience verification entirely. Tokens minted by any other
application that shared the same IdP signing keys (Azure AD, Okta,
etc.) were accepted as long as their signature checked out, even
though their `aud` and `iss` claims pointed at unrelated apps. The
proxy then fell into the no-team / no-user branch where access checks
default-allow.
This change:
1. Adds support for the `JWT_ISSUER` env var. When set, PyJWT verifies
the token's `iss` claim — turning on the same defense for tokens
that share an audience but come from a different IdP tenant.
2. Refactors the duplicated `jwt.decode` calls (RSA/EC/OKP path and
x509 path) into a single `_build_decode_kwargs` helper that
computes audience, issuer, and the corresponding `verify_*` opt-outs
once per call.
3. Logs a single startup-time warning when JWT auth is enabled but
neither `JWT_AUDIENCE` nor `JWT_ISSUER` is configured, so operators
running the insecure default see a flag in their logs without
getting spammed per-request.
Default behavior (no env vars) is preserved for backward compatibility.
Setting `JWT_AUDIENCE` and/or `JWT_ISSUER` opts into the verification.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GitHub serves PDFs from raw.githubusercontent.com, github.com/.../raw/..., LFS, and Releases as application/octet-stream by deliberate anti-hotlinking policy. Anyone who passes a GitHub-hosted PDF URL as an OpenAI / Gemini / Bedrock file_id hits "unsupported MIME type 'application/octet-stream'" because _process_image_response inlines the URL with whatever Content-Type the server sent.
When the server-provided Content-Type is application/octet-stream or binary/octet-stream and the URL extension maps to a known MIME type (.pdf, .png, .jpg, etc.), trust the extension instead. Specific Content-Types (image/png, application/pdf) still win over the extension; the override only applies to generic binary types.
Also restores the Greptile SHA-pinned raw.githubusercontent.com URL on the file_id integration test so we test against the same hosting real users hit, no third-party CDN.
Anthropic's URL fetcher intermittently returns 400 'Unable to download
the file' for the Wikipedia URL the test was using. Point it at the
repo's existing tests/llm_translation/fixtures/dummy.pdf via raw
GitHub instead — small, deterministic, reliably fetchable.
With a stable URL the test no longer needs to be opted out of VCR;
remove it from the incompatible list so it can replay from cassette.
`_check_proxy_admin_viewer_access` enumerates write routes a
PROXY_ADMIN_VIEW_ONLY caller may not invoke, then falls through to
"allow" for any management route not listed. Several write endpoints
were never added to the blocklist, so a viewer could:
- block or unblock any team via `/team/block` / `/team/unblock`
- mutate team permissions via `/team/permissions_update` and
`/team/permissions_bulk_update`
- create, update, or delete JWT key mappings via
`/jwt/key/mapping/{new,update,delete}`
- bulk-edit keys via `/key/bulk_update`
- reset key spend via the path-parameterized `/key/{id}/reset_spend`
Hoist the blocklist into a module-level frozenset and a tuple of
suffix patterns so it's clear what to extend when a new write route
is added, and pull the existing key write routes from the
`KeyManagementRoutes` enum so the two stay in sync. Adds parametrized
tests over the newly-blocked routes plus baseline coverage for routes
that should remain allowed (info / list / daily-activity reads).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolve URL conflict: keep Greptile's commit-SHA pin for immutability, but route through jsDelivr (cdn.jsdelivr.net/gh/BerriAI/litellm@<sha>/...) so the response Content-Type is application/pdf instead of application/octet-stream. Without this, OpenAI / Gemini / Router PDF tests reject the inlined file_data with "unsupported MIME type 'application/octet-stream'".
Per review: `assert response.status_code != 503` is satisfied by 404,
500, or any other non-503 code, so a regression that returned the wrong
non-503 status would slip through. Switch to `== 200` so the assertions
verify the actual expected status, not just the absence of one specific
failure.
A test that produces non-deterministic request bodies (e.g. uuid in
the prompt) under record_mode=new_episodes never replays — every CI
run appends fresh unmatched episodes. The cassette grows unbounded
over time and silently inflates Redis (we observed one cassette at
22 episodes / ~860KB after ~5 CI runs).
Refuse the save when episode count exceeds MAX_EPISODES_PER_CASSETTE
so the pathology surfaces with a loud warning that points to the
opt-out fix instead of festering invisibly.
The previous URL switch to raw.githubusercontent.com fixed Anthropic's "Unable to download" failure but caused OpenAI / Gemini / Router PDF tests to fail with "unsupported MIME type 'application/octet-stream'": those providers download the URL and inline it as data:<Content-Type>;base64,..., and raw.githubusercontent.com serves PDFs as application/octet-stream.
jsDelivr proxies the same in-repo fixture (cdn.jsdelivr.net/gh/BerriAI/litellm@main/...) and returns the correct Content-Type: application/pdf, so all providers (Anthropic forwards the URL natively; OpenAI/Gemini/Bedrock fetch and inline) get the right MIME type without changing transformer code.
Some tests can't benefit from cassette replay because they assert on
state that only exists in the live provider between two calls (e.g.
prompt-cache propagation, intermittent provider quirks). Marking them
with @pytest.mark.vcr just wastes cycles trying to record cassettes
they will never replay against successfully.
Opt-out by nodeid suffix so subclassed/parametrized variants are
covered:
- ::test_prompt_caching — Anthropic/Bedrock prompt-cache propagation
isn't deterministic in the 0–1s window the test gives it.
- ::test_async_pdf_handling_with_file_id — flaky upstream Wikipedia
fetch through the Anthropic Files API.
- TestBedrockInvokeNovaJson::test_json_response_pydantic_obj —
Bedrock Nova returns tool_call vs JSON nondeterministically (other
providers' subclasses are healthy).
- ::test_bedrock_converse__streaming_passthrough — Bedrock streaming
response_cost calc returns None intermittently.
These tests keep their existing @pytest.mark.flaky retry behavior.
When use_background_health_checks is enabled, /health?model=foo returned
the full cached aggregate across every model — so an unhealthy foo
combined with any other healthy deployment kept healthy_count > 0 and
the targeted-503 path never fired.
Resolve the targeted model/model_id to a deployment-id set first
(mirroring perform_health_check's match-on-model_name-or-litellm_model
semantics) and narrow the cache to those IDs before _post_process
evaluates healthy_count, so the 503 contract holds for both the live
and cache code paths.
Two follow-ups to the managed-resource isolation fix:
1. Rename the new composite indexes to match Prisma's auto-generated naming
convention (`<Table>_created_by_team_id_created_at_idx`). The previous
`*_team_owner_created_at_idx` names left `prisma migrate diff` reporting
an outstanding `RENAME INDEX`, failing `test_aaaasschema_migration_check`.
2. Make `build_owner_filter` return an OR clause when the caller has both
a `user_id` and a `team_id`, so listings include team-shared resources
the same way `can_access_resource` already permits reading them. Without
this a user could fetch a team-shared resource by id but never see it
in their list view.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A test that fails (incl. all the failing retries before a passing one)
can otherwise overwrite a known-good cassette with a 'bad luck'
recording. Tests like test_prompt_caching, which assert on provider
state across two calls, can produce a 200 response that semantically
fails the assertion — the 2xx filter doesn't catch this because the
HTTP layer is fine.
- pytest_runtest_makereport hook attaches each phase report to the
pytest item.
- _vcr_outcome_gate fixture (combining the verbose-mode reporter)
reads the call-phase outcome at teardown and informs the persister
via mark_test_outcome_for_cassette before vcrpy's Cassette.__exit__
triggers save_cassette.
- save_cassette consults the per-key 'did the test pass?' flag and
short-circuits when False, leaving any prior good recording intact.
- Defaults to passed=True when no marker is present so non-test
usage of the persister still works.
Service-account API keys are issued without a `user_id`, and managed
file/batch/vector-store ownership checks compared
`resource.created_by == user_api_key_dict.user_id`. Because Python
evaluates `None == None` as True, any service-account key passed
ownership checks for any resource also created without a user id, and
listing endpoints skipped the `created_by` filter entirely when the
caller had no user id — returning every tenant's records.
Replace the bare equality with an identity-aware helper:
- Admins (PROXY_ADMIN, PROXY_ADMIN_VIEW_ONLY) keep their unscoped view.
- Callers with a `user_id` are scoped to records they created.
- Callers without a `user_id` but with a `team_id` are scoped to records
created within their team via a new `created_by_team_id` column.
- Callers with no admin role and no identifying ids are denied — the
listing path returns an empty page without issuing a query.
Schema migration adds `created_by_team_id` to LiteLLM_ManagedFileTable,
LiteLLM_ManagedObjectTable, and LiteLLM_ManagedVectorStoreTable, plus
indexes for the new filter. Writes in BaseManagedResource and the
enterprise managed_files hook now stamp the column from
`user_api_key_dict.team_id`. Reads in `can_user_access_unified_resource_id`,
`can_user_call_unified_file_id`, `can_user_call_unified_object_id`,
`list_user_resources`, `list_user_batches`, and `get_user_created_file_ids`
all delegate to the new helper.
Tests cover the helper in isolation, the base-class listing/access paths,
and the enterprise file-access hook (including a regression test for the
original `None == None` bypass).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
