Veria AI / Cursor Bugbot findings on the prior admission-scoped ContextVar
fix: a streaming response's success/failure event fires from a task the
proxy forks independently of admission's own, so it never carries the
same context token, and the earlier fix silently stopped releasing every
completed stream's concurrency reservation until the safety TTL.
async_log_success_event/async_log_failure_event go back to releasing
unconditionally; only _release_stale_hop_reservations, which only ever
runs synchronously within admission's own hop sequence, keeps the token
check.
resolve_any's routing-group fallback also always restamped resolved_group
with the candidate model_name, discarding the team_public_model_name
_build_limits_index already stamped there for a team-owned deployment --
splitting one team's bucket depending on whether a call reached it via its
alias or a routing group. Now left untouched when already set.
The pending-reservations cache mirror (the async_post_call_failure_hook
fallback for when model_call_details itself is unavailable) is keyed by
(call_id, key_hash), both identical across every branch of one
abatch_completion dispatch. Mirroring now merges onto whatever's already
cached instead of overwriting it, and releasing removes only the specific
entries actually released instead of deleting the whole key, so one
branch's own write or release can no longer erase a still-live sibling
branch's own mirrored reservation before it's ever read.
Bugbot finding: async_log_success_event/async_log_failure_event still
called _pop_pending_concurrency_keys with no filter, so the first
finishing branch of an abatch_completion dispatch released every
reservation on the shared model_call_details, including a still-live
sibling branch's own slot.
Reservations are now tagged with an admission-scoped token from a
ContextVar rather than a raw asyncio.Task: create_task snapshots the
current Context, so a task explicitly forked from within one hop's own
admission (litellm's own logging dispatch explicitly propagates context)
still reads back the same token, while abatch_completion's sibling
branches, forked before any of them call admission, each mint their own
distinct token on first use. Both the stale-hop cleanup and the terminal
release hooks (success/failure) now require this token to match before
releasing; the disconnect hook is left unfiltered, since its own scenario
(mid-stream disconnect) cannot co-occur with abatch_completion's combined,
non-streaming response.
Bugbot finding: resolve_authoritative_metadata_variable_name treated any
non-empty litellm_metadata as authoritative, but a caller can populate it
with unrelated keys on an ordinary route where "metadata" is the real
field. Now requires the unconditionally-stamped, strip-protected
"user_api_key_auth" marker instead of mere truthiness.
Veria AI finding: Router.abatch_completion's comma-separated multi-model
dispatch runs branches concurrently as separate asyncio Tasks that all
share one litellm_logging_obj, so a still-live sibling branch's own
concurrency reservation could sit in the same model_call_details a new
hop's admission was cleaning up. _release_stale_hop_reservations now only
reclaims entries queued by its own asyncio.Task, leaving a differently
tasked (still-live) entry alone.
Bugbot finding: stamping team_scope on both the team-alias and internal
model_name paths didn't unify their Redis keys, since _hash_tag still
hashed the caller-visible name by default and that name differs between
the two paths. Both index branches now also stamp resolved_group with the
team's own public alias, so a team calling its own internal model_name and
the same team calling its public alias land on one shared bucket.
Veria AI finding: admission resolved the authoritative metadata bucket via
get_metadata_variable_name_from_kwargs, which only checks key presence. A
caller could forge an empty (or None) litellm_metadata to make admission
read no tags/team at all and bypass every configured limit. Admission now
reuses the same truthiness-checking resolver already used for success
accounting, renamed to reflect both call sites.
model_based_tag_rate_limits_hook.py had 3 real type errors present since its
introduction, invisible in scoped per-file checks but visible once codebase-wide
upstream drift finally pushed the totals over ceiling: sorted()/groupby() keyed
by a raw dict lookup returning object rather than a provably orderable type, and
a tuple passed where async_increment_tokens_with_ttl_preservation expects a
list. Adds a small typed _model_name_of accessor for the first two, and drops
an unneeded tuple() conversion for the third since the value was already a
list. Ran make lint-budget-update per repo convention.
LiteLLM_Params and ModelInfo instead of raw dicts, plus assert narrowing on
two Router calls that can return None, to satisfy basedpyright's strict
Deployment(...) construction path.
resolve_any dedups a routing group's divergent per-deployment entries by
picking the alphabetically first member model_name sharing a signature
(resolved_group). Admission and success each independently rebuilt
candidate_model_names from the router's live routing-group membership at
their own point in time, so a deployment added or removed mid-request (a
hot-reload) could make success pick a different resolved_group than
admission did, hashing to a different Redis key and letting real token or
dollar usage escape the bucket admission actually checked. Stashes
admission's own candidate set on model_call_details, mirroring the existing
admission-time-timestamp fix, so success reuses the identical snapshot.
bugbot caught this on review.
Two Bugbot findings from the same round:
- The Redis refresh_ttl fix never reached the in-memory fallback path:
async_set_cache called through unconditionally, and InMemoryCache's
allow_ttl_override left a still-live ttl untouched regardless. Adds a
refresh_ttl kwarg to InMemoryCache.set_cache/async_set_cache that bypasses
that guard, wired through from the hook's own refresh_ttl flag.
- A team-owned deployment resolved via its team_public_model_name alias got
team_scope stamped into its bucket key, but the identical deployment
resolved via its own internal model_name (which Router.should_include_deployment
also permits for same-team or team-unconstrained callers) did not -- letting
a caller split its usage across two independent counters by alternating
which name it called with. Stamps the same team_scope onto the by_model_name
entry whenever any deployment in that group has a team alias, so both paths
resolve to the identical bucket.
bugbot flagged this as a bug (per-deployment-scoped limits should filter the
over-limit deployment out of healthy_deployments and let a sibling serve,
not reject the whole routing attempt). This is deliberate, documented design
intent from the original plan: an earlier draft considered filter-and-retry
semantics and rejected it, since rejecting the whole hop is simpler and
avoids a caller silently succeeding against a deployment whose limit
configuration they didn't intend to satisfy. Adding that reasoning as an
inline comment so it doesn't get re-flagged as a bug on a future review.
async_increment_pipeline dropped each RedisPipelineIncrementOperation's own
ttl field, so a counter created through it (Router's TPM/RPM tracking,
parallel_request_limiter_v3's token/dollar accounting when Redis is absent,
and this PR's own tag-based token/dollar limits) always fell back to the
cache's 600-second default_ttl regardless of a real, often much longer,
configured window. An hourly or daily limit's counter would silently expire
and reset mid-window. allow_ttl_override already leaves a still-live ttl
untouched on a later call, so threading the operation's ttl through on every
increment only ever takes effect the first time. bugbot caught this on review.
TAG_RL_CHECK_AND_INCR_SCRIPT only called EXPIRE when a key had no TTL at all,
so a concurrency counter's expiry was fixed from its first admission and never
pushed out by later ones. A concurrency bucket isn't epoch-windowed like
requests/tokens/dollars -- its TTL exists purely as a crash-safety net for a
reservation whose explicit release never runs -- so a still-active bucket
under sustained traffic would expire mid-flight, silently admitting past the
cap and letting a later release decrement an unrelated, newer cohort's
counter. Adds a refresh_ttl script argument, true only for the concurrency
caller, and verified against a real Redis instance since the in-memory
fallback (which already refreshes unconditionally) can't reproduce this.
bugbot caught this on review.
Every other optional hook on CustomLogger ships as an empty method a subclass
can override; this one didn't, so _release_disconnect_state_on_all_callbacks
calling it on any callback that doesn't implement it (nearly all of them)
raised AttributeError, caught and debug-logged on every single disconnect.
bugbot caught this on review.
order_tags_for_identity_resolution only checked the top level of the metadata
dict, which is correct for admission's flat request_kwargs but never present at
async_log_success_event time -- Logging.model_call_details only ever nests
metadata under kwargs["litellm_params"]. Admission correctly preferred the
key-backed identity tag, but token/dollar accounting fell through to the
caller-forged one instead, charging a different bucket than the one admission
actually checked. Adds the same litellm_params fallback _get_tags_from_request_kwargs
already relies on. bugbot caught this on review.
extract_identity/entry_applies resolve a tag_id via first-match-by-prefix over
metadata.tags, but _merge_tags keeps caller-supplied tags ahead of key/team/
project tags in that merged list. An authenticated caller could submit e.g.
company_id:attacker-chosen ahead of the calling key's real company_id:real-company
tag and have every rate-limit entry scoped to company_id resolve to the caller's
own value instead of the key's.
Adds order_tags_for_identity_resolution, which puts metadata.inherited_tags (the
server-computed snapshot of only the tags the calling key/team/project's own
config contributed) ahead of the full tags list before either lookup runs, and
wires it into both call sites in model_based_tag_rate_limits_hook.py. veria-ai
caught this on review.
The disconnect-state-release hook added in the previous commit dropped the
existing has_buffered_provider_output guard and the STREAM_SSE_KEEPALIVE_PING_BYTES
exclusion while rewiring the streaming generator's cleanup path, so a client
disconnecting after only keepalive pings (or while an agentic stream holds back
real output) got refunded to input cost even when billable output had already
been generated. Restores both checks; veria-ai caught this on review, and the
existing test_streaming_cancel_after_only_keepalive_pings_reconciles_to_input_cost
regression test now passes again.
A client disconnect throws GeneratorExit/CancelledError into the
request path, so neither the success nor failure logging callback
runs and a concurrency slot reserved at admission leaks until its own
safety TTL. Gives every registered CustomLogger a chance to release
such state via the new async_release_disconnect_state_hook, called
from both the streaming and non-streaming cancel-on-disconnect paths.
Enforces token, request, dollar, and concurrency limits scoped to a
request tag (end_user_id by default), configured per deployment under
model_info.tag_rate_limits and admitted once per routing hop. Supports
chain-wide and per-deployment-scoped buckets, team-aliased routing
groups, and per-entry scoping via enabled_for/disabled_for/
apply_to_key_alias/apply_to_models.
Registers as the model_based_tag_rate_limits_hook callback and reuses
the identity extraction, policy fingerprinting, and bucket-key hashing
primitives from tag_rate_limits_shared.py.
litellm/types/router.py is imported by plain SDK users, not just the proxy;
the previous ValueError messages for limit/key_ttl_seconds explained the
proxy rate-limit hook's internal admission mechanics (atomic
check-and-increment, read-only tokens/dollars check, cache TTL rollover),
leaking implementation details across the SDK/proxy boundary. Move that
mechanistic reasoning into code comments for future maintainers and keep
the raised messages generic, per Greptile's finding on PR #38289.
Adds regression tests asserting the three affected validators reject their
invalid inputs without leaking proxy-internal enforcement jargon.
Introduces the tag-scoped rate limit config schema (TagRateLimitEntry,
TagRateLimitScope, TagRateLimitGroup, TagRateLimits) and wires it onto
ModelInfo.tag_rate_limits, giving tag-based rate limiting hooks a
config shape to validate and consume.
Both called active.get(...) after an `or EMPTY_MAPPING` fallback that only
triggers on a falsy value, so a truthy non-Mapping (metadata can arrive as
an unparsed JSON string on multipart/extra_body routes) raised
AttributeError instead of falling through. order_tags_for_identity_resolution
already guarded the same pattern; applied the same isinstance check here.
Bugbot finding on commit 41be4a8782.
resolve_success_event_metadata_variable_name treated any non-empty
litellm_metadata as authoritative, but a caller can populate it with
unrelated content (e.g. {"marker": true}) on a route where metadata is
the field the proxy actually wrote authenticated tags/identity into,
causing both rate-limit hooks to find no tag and admit past the
configured limit.
add_user_api_key_auth_to_request_metadata unconditionally stamps a
user_api_key_auth marker into whichever bucket it resolves as
authoritative, overwriting anything a caller pre-populated there.
Requiring that marker's presence instead of mere truthiness can't be
forged onto the wrong side.
Veria AI finding, surfaced on PR #38347 but the vulnerable function is
this PR's own; ported the same fix pattern already hardened on #38292.
extract_identity/entry_applies (this module's own functions) resolve a tag_id
via first-match-by-prefix over metadata.tags, but _merge_tags
(litellm_pre_call_utils.py) keeps caller-supplied tags ahead of key/team/
project tags in that merged list. An authenticated caller could submit e.g.
company_id:attacker-chosen ahead of the calling key's real
company_id:real-company tag and have every rate-limit entry scoped to
company_id resolve to the caller's own value instead of the key's.
Adds order_tags_for_identity_resolution, which puts metadata.inherited_tags
(the server-computed snapshot of only the tags the calling key/team/project's
own config contributed) ahead of the full tags list before either lookup
runs. veria-ai caught this while reviewing #38292 (whose branch currently
carries this module's commits); porting the fix here since the vulnerable
functions it defends are this PR's own. #38292 will wire the call sites in
once it rebases onto this branch instead of carrying its own duplicate copy.
litellm/types/router.py is imported by plain SDK users, not just the proxy;
the previous ValueError messages for limit/key_ttl_seconds explained the
proxy rate-limit hook's internal admission mechanics (atomic
check-and-increment, read-only tokens/dollars check, cache TTL rollover),
leaking implementation details across the SDK/proxy boundary. Move that
mechanistic reasoning into code comments for future maintainers and keep
the raised messages generic, per Greptile's finding on PR #38289.
Adds regression tests asserting the three affected validators reject their
invalid inputs without leaking proxy-internal enforcement jargon.
Keeps the dashboard's generated API types in sync with the new
TagRateLimitEntry/TagRateLimitScope/TagRateLimitGroup/TagRateLimits
schema on ModelInfo.
Both tag-scoped rate limiting hooks need the same identity/scope
extraction, policy fingerprinting, bucket-key hashing, and cache
partitioning primitives. Moving them into their own module lets a
model-independent global hook consume them without reaching into a
model-based hook's private internals, which is how the two hooks
previously shared this logic.
Introduces the tag-scoped rate limit config schema (TagRateLimitEntry,
TagRateLimitScope, TagRateLimitGroup, TagRateLimits) and wires it onto
ModelInfo.tag_rate_limits, giving tag-based rate limiting hooks a
config shape to validate and consume.
* fix: stop a cleared Organization field from failing key creation
Clearing the Organization combobox in the Create Key modal left organization_id set to an empty string, so /key/generate looked up an organization named "" and failed with "Organization doesn't exist in db. Organization=".
OrganizationDropdown now emits null on clear, and GenerateKeyRequest normalizes an empty organization_id or project_id to None the same way it already does for team_id.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test: drop customer-specific docstring from key request normalization test
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: yassin <yassin@berri.ai>
* fix(spend_tracking): leave SpendLogs.session_id null when no client session id was established
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* chore(lint): ratchet basedpyright budget after session_id fix
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(spend_tracking): ignore trace ids as session ids
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(spend_tracking): gate null SpendLogs.session_id behind missing_session_id: omit
Unset, generate and reject keep the legacy trace id fallback. omit records only
metadata.session_id, the key Langfuse reads, so a trace id copied into
litellm_session_id by get_litellm_params never becomes a session.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(spend_tracking): stamp the omit decision on the request so a config reload cannot fabricate a session
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(spend_tracking): keep omit covering requests the pre-call stamp never reaches
Router-model provider pass-through calls allm_passthrough_route directly and
skips add_litellm_data_to_request, so those requests never run the pre-call
helper and carry no omit stamp. Reading only the stamp made POST
/anthropic/v1/messages write a fabricated uuid into SpendLogs.session_id under
missing_session_id: omit while its Langfuse trace had no session, the exact
divergence the policy exists to remove.
The stamp now only pins omit on, and an unstamped request falls back to the
configured policy, so a config reload still cannot fabricate a session for a
request that was decided pre-call.
* fix(spend_tracking): make the session-omission marker proxy-owned so clients cannot forge it
* fix(spend_tracking): strip the client-sent omission marker from both metadata buckets before they merge
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(spend_tracking): strip the session-omission marker from both metadata buckets
The pre-call policy ran before litellm_metadata is merged into metadata, so a
client that planted the marker in litellm_metadata had it copied back into the
route's own bucket after the strip and still got a null SpendLogs.session_id.
---------
Co-authored-by: yassin <yassin@berri.ai>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
* fix(proxy): return persisted team memberships from /user/new
new_user attached default teams after building its response from the
pre-membership snapshot, so NewUserResponse.teams was always empty for
users created with default_internal_user_params.teams. The CLI SSO flow
reads that response on a user's first login and minted a teamless JWT,
which skipped the default team's model allowlist.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): return team ids as a tuple to satisfy LIT001
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy/db): translate libpq sslrootcert and verify-* into Prisma's strict TLS params
Prisma silently drops sslrootcert and treats sslmode=verify-ca/verify-full as
prefer, so a DATABASE_URL copied from the RDS docs connected over TLS without
checking the server certificate. The URL handed to Prisma (writer, DIRECT_URL,
read replica, componentized entrypoints) now carries sslmode=require,
sslcert=<bundle> and sslaccept=strict instead.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* style(proxy/db): ruff format translate_libpq_ssl_params
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: yassin <yassin@berri.ai>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* feat(ui): keyset-paginate request logs by session trace
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): keep session grouping within type discipline budget
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* style(proxy): ruff format session grouping helpers
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(proxy): only group sessions when group_by_session is an explicit true
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(ui): reset session cursor on custom range and live tail toggles
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test(ui): cover cursor reset on custom range toggle
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(ui): ignore next page clicks while the grouped page is still fetching
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(ui): only block next page while grouped placeholder data is shown
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: yassin <yassin@berri.ai>
