fix(auth): stop re-caching stale auth objects so key updates propagate across replicas - #33560
fix(auth): stop re-caching stale auth objects so key updates propagate across replicas#33560yassin-berriai wants to merge 1 commit into
Conversation
|
|
Greptile SummaryThis PR stops stale auth objects from being re-written back into
Confidence Score: 5/5Safe to merge — the changes are narrowly scoped deletions of post-auth and post-spend cache writes, with no new code paths added to the auth hot path. All three legs of the fix (post-auth write removal, spend-writeback removal, Redis-to-memory TTL injection) are independently correct and each backed by a focused regression test. The PR description includes live multi-replica proof with Redis TTL observations, and an independent e2e recording. No new code paths are introduced in the auth critical path — only writes are removed. The No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/caching/dual_cache.py | Injects default_in_memory_ttl into the Redis-to-memory backfill in get_cache, async_get_cache, and async_batch_get_cache, preventing Redis-loaded values from being held in memory for up to 600 s instead of the configured auth-cache TTL. |
| litellm/proxy/auth/user_api_key_auth.py | Removes the post-auth _cache_key_object call that was re-writing the (potentially mutated) auth object back into DualCache on every request, closing the stale-cache feedback loop. |
| litellm/proxy/proxy_server.py | Removes key-spend writeback from _update_key_cache (auth object no longer appended to values_to_update_in_cache) and adds an emptiness guard so no no-op pipeline task is created for token-only requests. |
| tests/test_litellm/caching/test_dual_cache.py | Adds two new tests verifying that the Redis-to-memory backfill in async_get_cache and async_batch_get_cache respects default_in_memory_ttl. |
| tests/test_litellm/proxy/auth/test_user_api_key_auth.py | Adds test_auth_does_not_rewrite_cached_key_object_back_into_cache — runs full auth builder against a primed cache and asserts the cached payload is byte-for-byte unchanged afterwards; task synchronization uses asyncio.wait with a 5-second cap. |
| tests/test_litellm/proxy/test_proxy_server.py | Adds test_spend_tracking_never_writes_the_auth_object_back — patches async_set_cache_pipeline and async_set_cache, invokes update_cache, drains async tasks, then asserts zero pipeline writes targeting the hashed token. |
Reviews (5): Last reviewed commit: "fix(auth): stop re-caching stale auth ob..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@greptileai please review the current head d154fd3 |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
|
@greptileai please review the current head 0f2ef6b |
|
@greptileai please review the current head f021f42 |
|
@greptileai please review the current head 7f200ab |
f021f42 to
7f200ab
Compare
…e across replicas After every successful virtual-key auth the proxy unconditionally wrote the in-memory auth object back into DualCache. With enable_redis_auth_cache and multiple replicas, a replica holding a stale in-memory token republished it to shared Redis with a full TTL on every request, so /key/update and /key/delete never took effect while the key kept calling. Both load paths already cache the pristine object at load time (IdentityStore._resolve_key, get_key_object), so the post-auth write only served to persist request-mutated state. Two more writers of the same state carried the same defect. The per-request key-spend writeback in update_cache republished the full auth object after every priced request, and even scoped to the local pod it could race an invalidation and re-insert a revoked key with a fresh TTL; it is removed, as spend is tracked through the spend:key:* counters that budget enforcement reads first. The DualCache Redis-to-memory backfill wrote values into the in-memory cache without a ttl, so replicas hydrated from Redis held auth objects for InMemoryCache's 600s default instead of user_api_key_cache_ttl; the backfill now injects default_in_memory_ttl like async_set_cache does. Resolves LIT-4350
7f200ab to
eaca0bf
Compare
|
Consolidated into #33565, which now carries this branch's approach (post-auth re-cache removed outright, key auth object never written back by spend tracking) together with the local-only split for the remaining spend writebacks, the shared global proxy spend scalar, and the DualCache backfill TTL fix. LIT-4350 is marked as a duplicate of LIT-4219, which #33565 resolves |
Pull request was closed
Relevant issues
Linear ticket
Resolves LIT-4350
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
The branch was rebased on current staging and squashed to a single commit after all proofs were captured; the commit hashes below refer to the pre-squash history and the diff is unchanged (verified hunk-for-hunk against the squashed head).
All runs against two live proxy replicas sharing Postgres and Redis, with
enable_redis_auth_cache: trueand realgpt-5.2calls through the OpenAI API. Config:Replica A on port 44350, replica B on port 44351, launched with
python litellm/proxy/proxy_cli.py --config config.yaml --port <port>Before the fix (commit 69a491e, current staging)
Generate a key with
model_rpm_limit: {"gpt-5.2": 3}, prime BOTH replicas with one real completion each, then raise the limit on replica A:Keep traffic on replica B (stale in-memory auth). The stale blob reappears in Redis with the OLD limit and a fresh TTL:
Deleting the key is worse: it keeps working indefinitely under continuous traffic:
After the fix (commit 365c8fb)
Same rig, same steps. The stale replica no longer republishes its in-memory token; Redis stays clean after
/key/update:Delete now converges within one auth-cache TTL even under continuous traffic (the exact feedback-loop condition, request interval far below the 60s TTL):
DB-load caching is unchanged: after priming, the auth blob is present in Redis with the correct metadata and TTL, so the cache hit rate for steady-state traffic is the same as before
Independent e2e verification (Devin)
Screen recording
Recorded at commit
f021f429bawith two live proxy replicas sharing PostgreSQL and Redis and realgpt-5.2calls. Replica A populated Redis before replica B's first request, exercising the Redis-to-memory backfill; updating the key cleared Redis and ten stale-replica requests over 15 seconds did not recreate it. After deletion, replica B changed from HTTP 200 to 401 at 45.1 seconds, 61.9 seconds after hydration, with no restarts or manual Redis changes.Type
🐛 Bug Fix
Changes
user_api_key_auth.pyran_cache_key_objectafter every successful key auth, writing the request's auth object back into DualCache regardless of whether it had been loaded from cache or from the DB. Both load paths already cache the pristine object at load time (IdentityStore._resolve_keyandget_key_object), so this write only ever persisted request-mutated state, and underenable_redis_auth_cachewith multiple replicas it created a feedback loop: a replica holding a stale in-memory token re-published it to shared Redis with a full TTL on every request, so/key/update,/key/delete,/key/block, and rate-limit changes never took effect while the key kept calling. In a multi-region topology (separate Redis per region, shared Postgres) there is no reliable way to kill a runaway key at all without coordinating a DB update, worker restarts, and Redis deletes across every region at once. This PR removes that post-auth writeThe per-request key-spend writeback in
proxy_server.update_cacherepublished the full cached auth object the same way after every priced request. It is removed entirely: an earlier revision made it a local-only in-memory write, but review flagged that even that can race an invalidation (an in-flight priced request reads the old object,/key/deleteclears the cache, and the deferred task re-inserts the revoked object with a fresh TTL on that worker), so spend tracking now performs no auth-object cache write at all. Cross-pod spend is tracked by thespend:key:*Redis counters, which are unaffected, and budget enforcement reads throughget_current_spendwhich prefers those counters. The soft-budget projected-limit alerting that lived in the same function is retainedIndependent e2e verification caught a third leg of the same invariant: a replica whose in-memory copy of the auth object was populated by the DualCache Redis-to-memory backfill (rather than a DB load) held it for
InMemoryCache's own 600s default because the backfill passed no ttl, so a deleted key converged at 586s instead of within the 60s auth TTL. The backfill inget_cache,async_get_cache, andasync_batch_get_cachenow injectsdefault_in_memory_ttlexactly likeasync_set_cachealready didRegression tests:
test_auth_does_not_rewrite_cached_key_object_back_into_cacheruns the full auth builder against a primedUserApiKeyCacheand asserts the cached payload is byte-for-byte untouched afterwards (fails on the old code), andtest_spend_tracking_never_writes_the_auth_object_backasserts spend tracking performs zero auth-object cache writes (fails on the old code)Final Attestation