fix(proxy/auth): honor user_api_key_cache_ttl for management-object cache writes - #31504
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR makes management-object cache writes honor the configured
Confidence Score: 5/5The change is narrowly scoped to management-object cache TTL selection and includes focused test coverage for the helper, auth cache writes, and spend-update writeback. No blocking code issues were identified in the reviewed changes, and the tests described cover the main paths affected by the TTL behavior change.
 T-Rex LogsWhat T-Rex did
Reviews (4): Last reviewed commit: "fix(proxy/auth): honor user_api_key_cach..." | Re-trigger Greptile |
Greptile SummaryThis PR updates management-object cache writes to honor the configured user API key cache TTL. The main changes are:
Confidence Score: 4/5The cache TTL changes are narrowly scoped and covered by targeted tests across the helper, auth cache writer, and spend writeback paths. The implementation consistently routes management-object cache writes through the configured TTL resolver while preserving the dedicated MCP negative-cache behavior.
T-Rex LogsWhat T-Rex did
|
…ache writes general_settings.user_api_key_cache_ttl was ignored for every management-object write into user_api_key_cache. The configured value is propagated to the cache's default_in_memory_ttl at startup, but DualCache only applies that default when no explicit ttl kwarg is passed, and every management-object writer passed ttl=DEFAULT_MANAGEMENT_OBJECT_IN_MEMORY_CACHE_TTL (60s), which always won. So keys, teams, users, budgets, object permissions, vector stores, JWT user syncs and MCP caches all expired after 60s regardless of the setting. Adds get_management_object_ttl(cache) in user_api_key_cache.py, which returns the configured default_in_memory_ttl and falls back to the 60s constant only when no default is set, and routes every management-object writer through it. The helper takes a DualCache so it works at the many call sites that are typed UserApiKeyCache but exercised with a bare DualCache. Also covers the spend-update writeback in update_cache (async_set_cache_pipeline), which hardcoded ttl=60 on the same key/user/team objects and reset an active key's cache entry back to 60s on every priced request, so the configured TTL was never observed for keys receiving traffic. Resolves LIT-3338
b0cb7f7 to
284fdcb
Compare
|
@greptileai please re-review the current HEAD. Two notes: The P1 about stale pass-through routes after reload ( The HEAD was amended after the first review: |
|
@greptileai the sole remaining finding is filed under Comments Outside Diff and concerns pass-through route cleanup ( |
…ache writes (BerriAI#31504) general_settings.user_api_key_cache_ttl was ignored for every management-object write into user_api_key_cache. The configured value is propagated to the cache's default_in_memory_ttl at startup, but DualCache only applies that default when no explicit ttl kwarg is passed, and every management-object writer passed ttl=DEFAULT_MANAGEMENT_OBJECT_IN_MEMORY_CACHE_TTL (60s), which always won. So keys, teams, users, budgets, object permissions, vector stores, JWT user syncs and MCP caches all expired after 60s regardless of the setting. Adds get_management_object_ttl(cache) in user_api_key_cache.py, which returns the configured default_in_memory_ttl and falls back to the 60s constant only when no default is set, and routes every management-object writer through it. The helper takes a DualCache so it works at the many call sites that are typed UserApiKeyCache but exercised with a bare DualCache. Also covers the spend-update writeback in update_cache (async_set_cache_pipeline), which hardcoded ttl=60 on the same key/user/team objects and reset an active key's cache entry back to 60s on every priced request, so the configured TTL was never observed for keys receiving traffic. Resolves LIT-3338
3 participants
Relevant issues
Resolves LIT-3338
Linear ticket
LIT-3338
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Root cause
general_settings.user_api_key_cache_ttlis read at startup and propagated ontouser_api_key_cache.default_in_memory_ttlviaupdate_cache_ttl. ButDualCache.async_set_cacheonly fills indefault_in_memory_ttlwhen nottlkwarg is present, and every management-object writer passedttl=DEFAULT_MANAGEMENT_OBJECT_IN_MEMORY_CACHE_TTL(60s) explicitly, so the configured value was always overridden. The reported pathget_key_object -> _cache_key_object -> _cache_management_objectis one of those writers; keys, teams, users, budgets, object permissions, vector stores, JWT user syncs and the MCP caches all behaved the same way. On top of that, the spend-tracking writeback inupdate_cachere-cached the same key/user/team objects with a hardcodedttl=60on every priced request, so even an active key got its entry reset to 60s right after auth had cached itBefore / after on the exact production functions
The metric here is the in-memory cache entry TTL, which has no HTTP surface, so the proof drives the real production functions (
auth_checks._cache_key_objectandproxy_server.update_cache) against auser_api_key_cacheconfigured exactly the wayload_configconfigures it foruser_api_key_cache_ttl: 300, and reads back the in-memoryttl_dict. BEFORE is a clean checkout of the base commit; AFTER is this branchLive proxy, real key auth, real LLM call
Booted the proxy on this branch with
general_settings.user_api_key_cache_ttl: 300against a real Postgres, created a virtual key, and made a realgpt-4o-minicall so the fullget_key_object -> _cache_key_objectauth-cache path runs end to endThe proxy log confirms the setting loaded into
general_settings('user_api_key_cache_ttl': 300)Type
🐛 Bug Fix
Changes
Adds
get_management_object_ttl(cache)inlitellm/proxy/common_utils/user_api_key_cache.py, which returns the cache's configureddefault_in_memory_ttland falls back toDEFAULT_MANAGEMENT_OBJECT_IN_MEMORY_CACHE_TTLonly when no default is set, then routes every management-object writer through it instead of the hardcoded constant. The helper takes aDualCachebecause most of those call sites are typedUserApiKeyCachebut exercised in tests with a bareDualCache, so a method on the subclass would not resolve there. Call sites updated acrossauth_checks.py,handle_jwt.py,mcp_server_manager.py,user_api_key_auth_mcp.pyandproxy_server.py(_is_mcp_access_group_cached); the MCP access-group negative-cache branch keeps its short dedicated TTLThe spend-update writeback in
update_cacheis also brought under the setting. This is what makes the fix observable for keys that receive traffic; it does mean in-memory spend values live for the configured TTL before a DB re-sync, which is exactly whatuser_api_key_cache_ttlis meant to controlTests:
test_user_api_key_cache.pycovers the resolver (configured default wins, fallback when unset, and that it resolves on a plainDualCache);test_auth_checks.pycovers_cache_management_objecthonoring the configured TTL;test_proxy_server.pycovers theupdate_cachepipeline writeback honoring it. Each core assertion was confirmed to fail when the resolver or the pipeline ttl is reverted to the old constant