fix(proxy): preserve master_key set by initialize() during lifespan startup - #22396
Closed
giulio-leone wants to merge 1 commit into
Closed
fix(proxy): preserve master_key set by initialize() during lifespan startup#22396giulio-leone wants to merge 1 commit into
giulio-leone wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis PR fixes a bug where
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | Adds a guard so master_key is only overwritten from the LITELLM_MASTER_KEY env var when it has a truthy value, preserving any value previously set by initialize(). Correct and minimal fix. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["initialize() called programmatically"] --> B["master_key set from YAML config"]
B --> C["proxy_startup_event() runs"]
C --> D{"LITELLM_MASTER_KEY\nenv var set?"}
D -->|Yes| E["master_key = env var value"]
D -->|No - Before fix| F["master_key = None ❌\n(config value lost)"]
D -->|No - After fix| G["master_key preserved ✅\n(config value kept)"]
E --> H["user_api_key_auth() works correctly"]
F --> I["All requests treated as INTERNAL_USER\n→ 403 errors"]
G --> H
Last reviewed commit: 2f98969
…tartup proxy_startup_event() unconditionally overwrote master_key with the value of the LITELLM_MASTER_KEY env var, discarding a valid master_key already configured by initialize() from the YAML config. When neither CONFIG_FILE_PATH nor WORKER_CONFIG env vars are set (e.g. programmatic startup or test harnesses), the config is not reloaded during lifespan, so master_key stayed None. This caused user_api_key_auth() to return INTERNAL_USER for every request — including the admin key — resulting in 403 errors for MCP tool calls and list_tools. Only overwrite master_key from the env var when it has a truthy value, preserving any previously configured value. Fixes #22330
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #22330
Root Cause
proxy_startup_event()unconditionally overwritesmaster_keywithget_secret_str("LITELLM_MASTER_KEY"), discarding a validmaster_keyalready configured byinitialize()from the YAML config.When neither
CONFIG_FILE_PATHnorWORKER_CONFIGenv vars are set (e.g. programmatic startup or test harnesses), the config is not reloaded during the ASGI lifespan, somaster_keystaysNone.This causes
user_api_key_auth()to returnINTERNAL_USERfor every request — including the admin key — resulting in 403 errors for MCP tool calls and list_tools.Fix
Only overwrite
master_keyfrom the environment variable when it has a truthy value, preserving any previously configured value frominitialize().How I found it
The issue description attributes the 403 errors to ContextVar propagation failing when
StreamableHTTPSessionManagerspawns tasks. I proved this is incorrect — ContextVar propagation works correctly (anyio 4.x on asyncio backend copies context forcreate_task). The actual root cause is the master_key being wiped during lifespan startup.Testing
main)test_independent_clients_no_shared_session) also fails onmaindue to a separate stdio connection issue — not affected by this change