scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-11 03:38:38 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 84

fix(auto_router): drop the shunt CLI allow rules

Browse source 
Both Greptile and veria-ai flagged these as auto-approving too much: Claude Code
matches an allow rule's text up to its first wildcard with no host awareness, so
"Bash(curl -sS -F question=*)" approves a curl to any destination, not just this
proxy. A prompt-injected repo instruction could get a local file uploaded
somewhere without the user ever seeing a permission prompt.

lite autoroute up already prompts for genuinely new commands and lets a user
persist their own approval once they've seen it, which is the safer place for
that decision to live. Removes SHUNT_BASH_ALLOW_RULES and the settings.json
permissions merge entirely; the static-token env merge that up already did is
unaffected.
This commit is contained in:
moe-berri 2026-09-07 15:00:42 -07:00
parent a71dd731f2
commit 0798a83d36
3 changed files with 29 additions and 153 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
litellm/proxy/client/cli/commands/autoroute/commands.py
View file

@ -37,7 +37,7 @@ from .process import (
terminate,
write_pid_record,
)
from .settings import merge_claude_settings_shunt_permissions, merge_claude_settings_static_token
from .settings import merge_claude_settings_static_token
from .wizard import run_configure_wizard
_GENERATED_CONFIG_ADAPTER: Final = TypeAdapter(dict[str, JsonValue])
@ -156,16 +156,10 @@ def up(port: int) -> None:
ClaudeBackupRecord(existed=original_existed, content=original_settings if original_existed else None),
AUTOROUTE_BACKUP_PATH,
)
with_token: Final = merge_claude_settings_static_token(original_settings, base_url, master_key)
# Applied unconditionally: an unused allow rule matches no real command and has no
# runtime effect, so this stays correct whether or not the running config actually
# arms shunt (`configure` has no shunt option yet; this is ready for when it does).
merged: Final = merge_claude_settings_shunt_permissions(with_token)
merged: Final = merge_claude_settings_static_token(original_settings, base_url, master_key)
CLAUDE_SETTINGS_PATH.parent.mkdir(parents=True, exist_ok=True)
with secure_create(CLAUDE_SETTINGS_PATH) as f:
# merged nests MappingProxyType at every level (see merge_claude_settings_shunt_
# permissions); default=dict is what the JSON encoder needs to serialize those.
json.dump(merged, f, indent=2, default=dict)
json.dump(merged, f, indent=2)
except ClaudeSettingsError as e:
terminate(process.pid)
clear_pid_record()

50
litellm/proxy/client/cli/commands/autoroute/settings.py
View file

@ -1,5 +1,3 @@
from collections.abc import Mapping
from types import MappingProxyType
from typing import Final
from pydantic import JsonValue
@ -7,8 +5,6 @@ from pydantic import JsonValue
from .config import AUTOROUTER_MODEL_NAME
ENV_KEY: Final = "env"
PERMISSIONS_KEY: Final = "permissions"
ALLOW_KEY: Final = "allow"
API_KEY_HELPER_KEY: Final = "apiKeyHelper"
ANTHROPIC_API_KEY_KEY: Final = "ANTHROPIC_API_KEY"
ANTHROPIC_AUTH_TOKEN_KEY: Final = "ANTHROPIC_AUTH_TOKEN"
@ -26,25 +22,6 @@ ANTHROPIC_DEFAULT_MODEL_ENV_KEYS: Final = (
"ANTHROPIC_DEFAULT_OPUS_MODEL",
)
# Reduces prompts for the curl commands the shunt guardrail's Read/Bash rewrite generates
# (litellm/proxy/guardrails/auto_router_shunt.py); it is NOT a security boundary. Claude Code
# matches an allow rule's text before its first "*" verbatim, with no argument- or host-aware
# matching, so a rule narrow enough to name "curl" at all cannot also pin the destination the
# trailing "*" is free to name any URL. A real boundary needs a PreToolUse hook instead, which
# is the docs' own recommendation for exactly this case.
#
# One rule per command shape the rewrite emits, matched on the literal prefix each one starts
# with. The bounded read needs its own rule because it opens with the `wc -l` size check rather
# than with `curl`, so a curl-prefixed rule would never match the main large-file path.
SHUNT_BASH_ALLOW_RULES: Final = (
"Bash(L=$(wc -l < *)",
"Bash(curl -sS -F question=*)",
"Bash(curl -sS -F spec=*)",
)
_NO_MAPPING: Final[Mapping[str, JsonValue]] = MappingProxyType({})
_NO_RULES: Final[tuple[JsonValue, ...]] = ()
def merge_claude_settings_static_token(
settings: dict[str, JsonValue], base_url: str, auth_token: str
@ -71,29 +48,4 @@ def merge_claude_settings_static_token(
return merged
def merge_claude_settings_shunt_permissions(settings: Mapping[str, JsonValue]) -> Mapping[str, JsonValue]:
"""Add the shunt allow rules to `permissions.allow`, preserving every other permissions key.
A naive top-level `{**settings, "permissions": {...}}` would replace the whole `permissions`
object, silently dropping any `deny`/`ask` rules the caller already has -- exactly the trap
`merge_claude_settings_static_token` avoids for `env` by merging that key explicitly instead
of replacing it. This does the same for `permissions.allow`: read what's there, add only the
two shunt rules if they're not already present, and leave `deny`/`ask`/anything else alone.
Returns a `MappingProxyType` nested at every level rather than a plain dict, so the caller's
`json.dump(..., default=dict)` is what converts it back to something the JSON encoder accepts
-- the one place a concrete mutable mapping is genuinely needed, kept out of this function.
"""
raw_permissions: Final = settings.get(PERMISSIONS_KEY, _NO_MAPPING)
base_permissions: Final = raw_permissions if isinstance(raw_permissions, Mapping) else _NO_MAPPING
raw_allow: Final = base_permissions.get(ALLOW_KEY, _NO_RULES)
base_allow: Final = raw_allow if isinstance(raw_allow, (list, tuple)) else _NO_RULES
new_allow: Final = (*base_allow, *(rule for rule in SHUNT_BASH_ALLOW_RULES if rule not in base_allow))
permissions: Final = MappingProxyType({**base_permissions, ALLOW_KEY: new_allow})
return MappingProxyType({**settings, PERMISSIONS_KEY: permissions})
__all__ = [ # mutable-ok: __all__ must be a list per Python convention
"merge_claude_settings_shunt_permissions",
"merge_claude_settings_static_token",
]
__all__ = ["merge_claude_settings_static_token"] # mutable-ok: __all__ must be a list per Python convention

120
tests/test_litellm/proxy/client/cli/autoroute/test_settings.py
View file

@ -1,9 +1,5 @@
import json
from litellm.proxy.client.cli.commands.autoroute.settings import (
ANTHROPIC_DEFAULT_MODEL_ENV_KEYS,
SHUNT_BASH_ALLOW_RULES,
merge_claude_settings_shunt_permissions,
merge_claude_settings_static_token,
)
@ -13,49 +9,36 @@ def test_preserves_unrelated_top_level_keys():
assert merged["theme"] == "dark"
def test_preserves_unrelated_env_keys():
def test_sets_base_url_and_auth_token():
merged = merge_claude_settings_static_token({}, "http://127.0.0.1:4000", "token-abc")
assert merged["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:4000"
assert merged["env"]["ANTHROPIC_AUTH_TOKEN"] == "token-abc"
def test_strips_trailing_slash_from_base_url():
merged = merge_claude_settings_static_token({}, "http://127.0.0.1:4000/", "token-abc")
assert merged["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:4000"
def test_clears_existing_api_key_env_var():
settings = {"env": {"ANTHROPIC_API_KEY": "sk-old"}}
merged = merge_claude_settings_static_token(settings, "http://127.0.0.1:4000", "token-abc")
assert "ANTHROPIC_API_KEY" not in merged["env"]
def test_clears_existing_api_key_helper():
settings = {"apiKeyHelper": "some-script.sh"}
merged = merge_claude_settings_static_token(settings, "http://127.0.0.1:4000", "token-abc")
assert "apiKeyHelper" not in merged
def test_preserves_other_env_vars():
settings = {"env": {"SOME_OTHER_VAR": "value"}}
merged = merge_claude_settings_static_token(settings, "http://127.0.0.1:4000", "token-abc")
assert merged["env"]["SOME_OTHER_VAR"] == "value"
def test_sets_base_url_and_auth_token():
merged = merge_claude_settings_static_token({}, "http://127.0.0.1:4000/", "token-abc")
assert merged["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:4000"
assert merged["env"]["ANTHROPIC_AUTH_TOKEN"] == "token-abc"
assert merged["env"]["ENABLE_TOOL_SEARCH"] == "true"
def test_preserves_existing_tool_search():
settings = {"env": {"ENABLE_TOOL_SEARCH": "false"}}
merged = merge_claude_settings_static_token(settings, "http://127.0.0.1:4000", "token-abc")
assert merged["env"]["ENABLE_TOOL_SEARCH"] == "false"
def test_drops_stray_api_key():
settings = {"env": {"ANTHROPIC_API_KEY": "leaked-key"}}
merged = merge_claude_settings_static_token(settings, "http://127.0.0.1:4000", "token-abc")
assert "ANTHROPIC_API_KEY" not in merged["env"]
def test_removes_existing_api_key_helper():
settings = {"apiKeyHelper": "/usr/local/bin/lite auth print-token"}
merged = merge_claude_settings_static_token(settings, "http://127.0.0.1:4000", "token-abc")
assert "apiKeyHelper" not in merged
def test_does_not_mutate_input():
settings = {"env": {"FOO": "bar"}, "apiKeyHelper": "old-helper"}
merge_claude_settings_static_token(settings, "http://127.0.0.1:4000", "token-abc")
assert settings == {"env": {"FOO": "bar"}, "apiKeyHelper": "old-helper"}
def test_forces_all_claude_code_default_model_tiers_to_the_autorouter():
# A bare "*" model_name deployment looks like the obvious way to catch every request
# regardless of which model Claude Code thinks it's using, but Router's auto-router
# registry is keyed by the literal requested model string with no wildcard resolution
# (litellm/router.py:10711-10717) -- so the only reliable way to make every one of Claude
# Code's own tiers hit the auto-router is to override the env vars it reads per tier.
def test_sets_every_default_model_env_key_to_autorouter():
merged = merge_claude_settings_static_token({}, "http://127.0.0.1:4000", "token-abc")
for key in ANTHROPIC_DEFAULT_MODEL_ENV_KEYS:
assert merged["env"][key] == "autorouter"
@ -65,56 +48,3 @@ def test_overrides_a_preexisting_default_model_env_var():
settings = {"env": {"ANTHROPIC_DEFAULT_SONNET_MODEL": "claude-opus-4-8"}}
merged = merge_claude_settings_static_token(settings, "http://127.0.0.1:4000", "token-abc")
assert merged["env"]["ANTHROPIC_DEFAULT_SONNET_MODEL"] == "autorouter"
def test_adds_shunt_allow_rules_to_empty_settings():
merged = merge_claude_settings_shunt_permissions({})
assert tuple(merged["permissions"]["allow"]) == SHUNT_BASH_ALLOW_RULES
def test_preserves_unrelated_top_level_keys_for_permissions_merge():
merged = merge_claude_settings_shunt_permissions({"theme": "dark"})
assert merged["theme"] == "dark"
def test_preserves_existing_allow_rules():
settings = {"permissions": {"allow": ["Bash(npm run *)"]}}
merged = merge_claude_settings_shunt_permissions(settings)
assert "Bash(npm run *)" in merged["permissions"]["allow"]
for rule in SHUNT_BASH_ALLOW_RULES:
assert rule in merged["permissions"]["allow"]
def test_preserves_deny_and_ask_rules():
# Regression: a naive {**settings, "permissions": {...}} replaces the whole permissions
# object, silently dropping deny/ask rules the caller already had.
settings = {"permissions": {"deny": ["Bash(git push *)"], "ask": ["Bash(rm *)"]}}
merged = merge_claude_settings_shunt_permissions(settings)
assert merged["permissions"]["deny"] == ["Bash(git push *)"]
assert merged["permissions"]["ask"] == ["Bash(rm *)"]
for rule in SHUNT_BASH_ALLOW_RULES:
assert rule in merged["permissions"]["allow"]
def test_does_not_duplicate_shunt_rules_already_present():
settings = {"permissions": {"allow": list(SHUNT_BASH_ALLOW_RULES)}}
merged = merge_claude_settings_shunt_permissions(settings)
assert tuple(merged["permissions"]["allow"]) == SHUNT_BASH_ALLOW_RULES
def test_merged_settings_survive_the_json_round_trip_commands_py_writes():
# Regression: the merge returns MappingProxyType nested at every level, which the JSON
# encoder rejects without `default=dict` -- the exact call commands.py makes. Without it
# `lite autoroute up` raises instead of writing settings.json.
settings = {"permissions": {"allow": ["Bash(npm run *)"], "deny": ["Bash(rm *)"]}, "theme": "dark"}
merged = merge_claude_settings_shunt_permissions(settings)
reloaded = json.loads(json.dumps(merged, default=dict))
assert reloaded["theme"] == "dark"
assert reloaded["permissions"]["deny"] == ["Bash(rm *)"]
assert reloaded["permissions"]["allow"] == ["Bash(npm run *)", *SHUNT_BASH_ALLOW_RULES]
def test_does_not_mutate_input_for_permissions_merge():
settings = {"permissions": {"allow": ["Bash(npm run *)"], "deny": ["Bash(rm *)"]}}
merge_claude_settings_shunt_permissions(settings)
assert settings == {"permissions": {"allow": ["Bash(npm run *)"], "deny": ["Bash(rm *)"]}}