fix: scope CLI stored token to base_url to prevent cross-domain credential leakage - #26945
Conversation
|
|
Greptile SummaryThis PR fixes a credential-leakage CVE by binding stored CLI tokens to the server URL they were issued for. The three-layer fix (token file, CLI entrypoint, SDK Confidence Score: 4/5Safe to merge; known edge cases (stored_url not normalized on read, case-sensitive host comparison) are flagged in prior review threads but do not break the core security invariant for the common case Score capped at 4 due to open P1s already noted in previous review threads (stored_url read without rstrip, RFC 3986 case-insensitive host comparison). No new P0/P1 issues found in this pass. The
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/cli_token_utils.py | Adds optional expected_base_url parameter to gate key retrieval on origin match; stored_url is compared without normalization (trailing-slash and case issues noted in prior thread) |
| litellm/proxy/client/cli/commands/auth.py | Login now persists base_url in token.json; get_stored_api_key delegates the new expected_base_url parameter correctly |
| litellm/proxy/client/cli/main.py | CLI entrypoint now passes base_url when loading stored key, preventing cross-domain credential use |
| litellm/proxy/client/client.py | Two bugs fixed: priority flipped so explicit api_key takes precedence over stored key, and self.http is now initialized with the resolved self._api_key instead of the raw None parameter |
| tests/test_litellm/proxy/client/cli/test_auth_commands.py | Four new mock-only test cases cover URL match, trailing-slash normalisation, URL mismatch, and legacy tokens; existing tests unchanged |
Reviews (2): Last reviewed commit: "fix: initialize self.http with resolved ..." | Re-trigger Greptile
| if expected_base_url is not None: | ||
| stored_url = token_data.get("base_url") | ||
| if stored_url != expected_base_url.rstrip("/"): | ||
| return None |
There was a problem hiding this comment.
stored_url is not normalized before comparison
expected_base_url is rstripped but stored_url is taken verbatim from the file. New tokens are saved with rstrip("/") in auth.py, but if a token was written by any other code path (or manually edited) with a trailing slash, the comparison will silently reject a legitimate match. Normalizing both sides is safer:
if stored_url is None or stored_url.rstrip("/") != expected_base_url.rstrip("/"):
return None| if expected_base_url is not None: | ||
| stored_url = token_data.get("base_url") | ||
| if stored_url != expected_base_url.rstrip("/"): | ||
| return None |
There was a problem hiding this comment.
URL comparison is case-sensitive; RFC 3986 requires case-insensitive host/scheme matching
The comparison is a raw string equality check. A stored URL with a different-cased host (e.g. https://Proxy.internal.com vs https://proxy.internal.com) will fail to match even though they refer to the same server. Lowercasing before comparison prevents this false negative.
| if api_key is None: | ||
| api_key = get_stored_api_key() | ||
| api_key = get_stored_api_key(expected_base_url=base_url) |
There was a problem hiding this comment.
Silent token rejection gives users no hint to re-login
When the stored token has no base_url (old format) or the URL doesn't match, get_stored_api_key returns None and the CLI proceeds unauthenticated. Users will see generic authorization errors from the server with no indication they need to run litellm-proxy login again. Emitting a warning when a stored token exists but was rejected would significantly improve the UX for users migrating from older token formats.
Two security-shaped PRs in litellm: - BerriAI/litellm#26945: merge-as-is, scope stored CLI key to base_url via expected_base_url kwarg in get_litellm_gateway_api_key, with symmetric rstrip normalization at write+read and fail-closed for legacy tokens missing base_url; 4-test contract pin - BerriAI/litellm#26924: merge-as-is, total deletion of urlToken handler at LoginPage.tsx:69-80 closing session-fixation; 2 regression tests pin anti-behavior including silent-overwrite arm
231c430
into
litellm_internal_staging
…ntial leakage (BerriAI#26945) * fix: add expected_base_url origin check to get_litellm_gateway_api_key * fix: scope get_stored_api_key and save base_url on login * fix: pass base_url to get_stored_api_key in CLI entrypoint * fix: scope ProxyClient stored key to base_url * test: add expected_base_url coverage for get_stored_api_key * fix: initialize self.http with resolved api_key not raw param * fix: black formatting in client.py and test_auth_commands.py
…ntial leakage (BerriAI#26945) * fix: add expected_base_url origin check to get_litellm_gateway_api_key * fix: scope get_stored_api_key and save base_url on login * fix: pass base_url to get_stored_api_key in CLI entrypoint * fix: scope ProxyClient stored key to base_url * test: add expected_base_url coverage for get_stored_api_key * fix: initialize self.http with resolved api_key not raw param * fix: black formatting in client.py and test_auth_commands.py
Relevant issues
Security CVE — cross-domain credential leakage in the LiteLLM CLI and SDK client.
Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🐛 Bug Fix (Security)
Changes
Problem: The CLI stored the API key globally in
~/.litellm/token.jsonwithout recording which server it was issued for. Any subsequent CLI invocation — even one pointed at a malicious server — would pick up the stored key and forward it viaAuthorization: Bearerto the attacker.Fix: Three targeted changes, 5 files:
cli_token_utils.py—get_litellm_gateway_api_key()now acceptsexpected_base_url. When provided, it only returns the stored key if it was issued for that URL (checked against the newbase_urlfield in the token file).auth.py—get_stored_api_key()gains the sameexpected_base_urlparameter and delegates down. Thelogincommand now persistsbase_urlintoken.jsonso future calls can verify origin.main.py— CLI entrypoint passesbase_urlwhen loading the stored key.client.py—ProxyClientwas also vulnerable (called unscopedget_litellm_gateway_api_key()). Now passesexpected_base_url=self._base_url.test_auth_commands.py— 4 new test cases: URL match, trailing-slash normalisation, URL mismatch, old token withoutbase_urlfield.Before (attacker scenario):
After: the stored key is only returned when
base_urlmatches the stored origin. Request toevil.comgets no key.