Repository navigation
feat(cli): fixed sk-1234 master key and debug flags for lite autoroute up - #33651
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThis PR makes two changes to the
Confidence Score: 4/5Safe to merge for a local dev CLI; the fixed key is a deliberate convenience trade-off and the proxy is loopback-only. The core logic — embedding the key in the generated config, forwarding debug flags to the subprocess, printing startup info — is straightforward and well-tested. The main open question is whether
|
| Filename | Overview |
|---|---|
| litellm/proxy/client/cli/commands/autoroute/commands.py | Replaces random per-session master key with hardcoded sk-1234, adds --debug/--detailed-debug flags forwarded to the subprocess, and prints config/log/port/key at startup; key change trades local security isolation for dev convenience. |
| litellm/proxy/client/cli/commands/autoroute/process.py | Adds debug and detailed_debug keyword-only parameters to launch_proxy, appending --debug/--detailed_debug to the subprocess command tuple; refactor is clean and the file-handle scope is unchanged. |
| tests/test_litellm/proxy/client/cli/autoroute/test_commands.py | Removes secrets.token_urlsafe monkeypatches, pins assertions to sk-1234, adds test_passes_debug_flags_to_proxy verifying launch_proxy receives debug/detailed_debug kwargs; coverage is good. |
| tests/test_litellm/proxy/client/cli/autoroute/test_config.py | Changes test_embeds_master_key_under_general_settings to pass "sk-1234" and assert "sk-1234" back — using the same literal for input and expected output weakens the test's ability to detect a regression where the function ignores its parameter. |
| tests/test_litellm/proxy/client/cli/autoroute/test_process.py | Adds test_forwards_debug_flags that patches subprocess.Popen and verifies --debug and --detailed_debug appear at the end of the command args; clear and correct. |
| litellm/proxy/client/cli/README.md | Updates the lite autoroute up description to reflect the fixed sk-1234 key and new --debug/--detailed-debug flags; documentation is accurate and matches the code changes. |
Reviews (1): Last reviewed commit: "feat(cli): improve autoroute up debuggab..." | Re-trigger Greptile
| AUTOROUTE_BACKUP_PATH = AUTOROUTE_DIR / "claude_settings_backup.json" | ||
|
|
||
| _GENERATED_CONFIG_ADAPTER = TypeAdapter(dict[str, JsonValue]) | ||
| MASTER_KEY = "sk-1234" |
There was a problem hiding this comment.
Well-known key lowers bar for lateral local API access
Replacing the per-session random key with the fixed sk-1234 means any process running on the same machine — including scripts, CI runners, or other user-space tools — that is aware of this convention can authenticate to the ephemeral proxy and make real Anthropic API calls on the developer's behalf without reading ~/.claude/settings.json. The previous random key required reading that file (permissions 0o600) to learn the credential; sk-1234 requires no file access at all. The proxy does bind only to 127.0.0.1, and the teardown message already cautions against multi-tenant hosts, so this is a deliberate trade-off for dev convenience. However, it's worth considering a compromise — e.g. a short fixed prefix like sk-local- followed with a few random bytes — that retains discoverability while avoiding a universally guessable credential.
| def test_embeds_master_key_under_general_settings(self): | ||
| config = _base_config() | ||
| proxy_config = build_generated_proxy_config(config, "sk-master-123") | ||
| assert proxy_config["general_settings"] == {"master_key": "sk-master-123"} | ||
| proxy_config = build_generated_proxy_config(config, "sk-1234") | ||
| assert proxy_config["general_settings"] == {"master_key": "sk-1234"} |
There was a problem hiding this comment.
Test no longer catches key-parameter regression
build_generated_proxy_config still accepts a master_key parameter, but the test now passes "sk-1234" and asserts "sk-1234" back — the same literal for both input and expected output. If a future change hardcodes MASTER_KEY inside build_generated_proxy_config instead of using the argument, this test would still pass and the regression would go undetected. Using a distinct value (e.g. "sk-sentinel-test-key") for the test argument would ensure the function's parameter is actually plumbed through.
Rule Used: What: Flag any modifications to existing tests and... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Relevant issues
Linear ticket
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
Captured live at commit
eb9154d545e9a7f2d46ade32594685058c02ba8c, hitting the real Anthropic APIReal completion against the ephemeral proxy with the fixed key
A wrong key is still rejected
~/.litellm/autorouter/proxy.logconfirms the flag reached the proxy subprocessA second
lite autoroute upafter a clean SIGTERM teardown printedMaster key: sk-1234again, so the key no longer changes between runsType
🆕 New Feature
Changes
lite autoroute upused to mint a randomsecrets.token_urlsafe(32)master key on every run, which made it painful to curl the local proxy while debugging. Since this is a local dev CLI, the key is now always the fixedsk-1234;_mint_and_embed_master_keybecomes_embed_master_keyand still writes it undergeneral_settings.master_keyin the generated configTo make the command easier to debug,
upnow prints the generated config path, log file path, port, and master key at startup, and grows--debug/--detailed-debugflags that are forwarded to thelitellm.proxy.proxy_clisubprocess as--debug/--detailed_debugvia new keyword-only params onlaunch_proxyTests pin the fixed key end to end (generated config,
ANTHROPIC_AUTH_TOKENin Claude settings, startup output) and assert the debug flags propagate to the subprocess argv. The CLI README section forlite autoroute upis updated to matchFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/531f86a2375b45f6985488c598f1f06d
Requested by: @krrish-berri-2