mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-05 08:07:05 +00:00
fix(cli): quote the Claude Code apiKeyHelper for cmd.exe on Windows (#39174)
* fix(cli): quote the Claude Code apiKeyHelper for cmd.exe on Windows lite up and lite login --config-claude wrote the helper command with POSIX shlex quoting, so a backslashed Windows install path came out wrapped in single quotes that cmd.exe and PowerShell take literally. Quote every token with the cmd.exe rules already used for agent shims when running on Windows, and keep the POSIX output unchanged elsewhere. Resolves LIT-6627 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(cli): split the Windows apiKeyHelper with cmd.exe and C runtime rules The invocation test pulled tokens back out with a regex, which cannot see the doubled quotes or the percent guard quote_for_cmd emits. Model the two parsers that read the helper on Windows instead and check argv round trips for backslashed, spaced, metacharacter, percent and quoted tokens --------- Co-authored-by: yassin <yassin@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
bad55da9bf
commit
f9c6eda909
5 changed files with 151 additions and 25 deletions
|
|
@ -9,6 +9,7 @@ import click
|
|||
import requests
|
||||
|
||||
from .auth import context_secret_vault, get_stored_api_key, login
|
||||
from .cmd_quoting import quote_for_cmd
|
||||
|
||||
ANTHROPIC_BASE_URL_ENV: Final = "ANTHROPIC_BASE_URL"
|
||||
ANTHROPIC_AUTH_TOKEN_ENV: Final = "ANTHROPIC_AUTH_TOKEN"
|
||||
|
|
@ -151,31 +152,9 @@ def verify_proxy_key(
|
|||
|
||||
|
||||
_WINDOWS_SHIM_SUFFIXES: Final[frozenset[str]] = frozenset({".cmd", ".bat"})
|
||||
_CMD_PERCENT_GUARD: Final = "%%cd:~,%"
|
||||
_CMD_LINE_BREAKS: Final = ("\r", "\n")
|
||||
|
||||
|
||||
def _double_trailing_backslashes(segment: str) -> str:
|
||||
bare: Final = segment.rstrip("\\")
|
||||
return bare + "\\" * 2 * (len(segment) - len(bare))
|
||||
|
||||
|
||||
def _quote_for_cmd(token: str) -> str:
|
||||
"""Quote one token so both parsers that read it see the original text.
|
||||
|
||||
Follows the algorithm the Rust standard library settled on for batch files
|
||||
after CVE-2024-24576. Two parsers see this token: cmd.exe, which ends a
|
||||
quoted string on a lone `"` and so wants an embedded one doubled, and the
|
||||
shim's own interpreter, which re-splits `%*` under C runtime rules where a
|
||||
backslash escapes the quote that follows it, so every backslash run standing
|
||||
before a quote is doubled. Quoting cannot stop cmd expanding `%VAR%`, so each
|
||||
`%` is prefixed with `%%cd:~,`: the zero-length substring of the always
|
||||
defined `cd` expands to nothing and leaves no `%` pair for cmd to match.
|
||||
"""
|
||||
escaped: Final = '""'.join(_double_trailing_backslashes(part) for part in token.split('"'))
|
||||
return '"' + escaped.replace("%", _CMD_PERCENT_GUARD) + '"'
|
||||
|
||||
|
||||
def _windows_command(path: str, args: Sequence[str]) -> str | tuple[str, ...]:
|
||||
"""Build what CreateProcess runs, routing batch shims through cmd.exe.
|
||||
|
||||
|
|
@ -202,7 +181,7 @@ def _windows_command(path: str, args: Sequence[str]) -> str | tuple[str, ...]:
|
|||
f"Cannot pass an argument containing a line break to `{os.path.basename(path)}` on "
|
||||
"Windows: cmd.exe ends the command line there, so the agent would silently lose it."
|
||||
)
|
||||
inner: Final = " ".join(_quote_for_cmd(token) for token in (path, *rest))
|
||||
inner: Final = " ".join(quote_for_cmd(token) for token in (path, *rest))
|
||||
return f'cmd.exe /d /e:on /v:off /s /c "{inner}"'
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ live here rather than in either command module.
|
|||
|
||||
import shlex
|
||||
import shutil
|
||||
import sys
|
||||
from collections.abc import Mapping, Sequence
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
|
@ -17,6 +18,8 @@ from pydantic import JsonValue, TypeAdapter, ValidationError
|
|||
|
||||
from litellm.litellm_core_utils.private_json import write_private_json
|
||||
|
||||
from .cmd_quoting import quote_for_cmd
|
||||
|
||||
ENV_KEY: Final = "env"
|
||||
API_KEY_HELPER_KEY: Final = "apiKeyHelper"
|
||||
ANTHROPIC_BASE_URL_KEY: Final = "ANTHROPIC_BASE_URL"
|
||||
|
|
@ -87,9 +90,12 @@ def merge_claude_settings(
|
|||
return {**settings, ENV_KEY: env, API_KEY_HELPER_KEY: api_key_helper}
|
||||
|
||||
|
||||
def resolve_api_key_helper(base_url: str) -> str:
|
||||
def resolve_api_key_helper(base_url: str, platform: str = sys.platform) -> str:
|
||||
"""Build the shell command Claude Code should run for its apiKeyHelper.
|
||||
|
||||
Claude Code hands the string to the system shell, `sh` on POSIX and cmd.exe
|
||||
on Windows, so every token is quoted for the shell that will read it.
|
||||
|
||||
Resolves `lite` to an absolute path so the helper works regardless of the
|
||||
PATH visible to whatever subprocess Claude Code spawns it from. Passing
|
||||
--base-url explicitly (rather than relying on the bare invocation Claude
|
||||
|
|
@ -106,7 +112,8 @@ def resolve_api_key_helper(base_url: str) -> str:
|
|||
raise ClaudeSettingsError(
|
||||
"Could not find `lite` on your PATH. Claude Code's apiKeyHelper needs an absolute path to it."
|
||||
)
|
||||
return f"{shlex.quote(lite_path)} --base-url {shlex.quote(base_url)} auth print-token"
|
||||
quote: Final = quote_for_cmd if platform.startswith("win") else shlex.quote
|
||||
return " ".join(quote(token) for token in (lite_path, "--base-url", base_url, "auth", "print-token"))
|
||||
|
||||
|
||||
def write_claude_settings(base_url: str, settings_path: Path, owners: Sequence[SettingsFileOwner]) -> None:
|
||||
|
|
|
|||
26
litellm/proxy/client/cli/commands/cmd_quoting.py
Normal file
26
litellm/proxy/client/cli/commands/cmd_quoting.py
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
"""Quoting for command lines that cmd.exe reads before handing them to a program."""
|
||||
|
||||
from typing import Final
|
||||
|
||||
_CMD_PERCENT_GUARD: Final = "%%cd:~,%"
|
||||
|
||||
|
||||
def _double_trailing_backslashes(segment: str) -> str:
|
||||
bare: Final = segment.rstrip("\\")
|
||||
return bare + "\\" * 2 * (len(segment) - len(bare))
|
||||
|
||||
|
||||
def quote_for_cmd(token: str) -> str:
|
||||
"""Quote one token so both parsers that read it see the original text.
|
||||
|
||||
Follows the algorithm the Rust standard library settled on for batch files
|
||||
after CVE-2024-24576. Two parsers see this token: cmd.exe, which ends a
|
||||
quoted string on a lone `"` and so wants an embedded one doubled, and the
|
||||
program's own C runtime argv split, where a backslash escapes the quote that
|
||||
follows it, so every backslash run standing before a quote is doubled.
|
||||
Quoting cannot stop cmd expanding `%VAR%`, so each `%` is prefixed with
|
||||
`%%cd:~,`: the zero-length substring of the always defined `cd` expands to
|
||||
nothing and leaves no `%` pair for cmd to match.
|
||||
"""
|
||||
escaped: Final = '""'.join(_double_trailing_backslashes(part) for part in token.split('"'))
|
||||
return '"' + escaped.replace("%", _CMD_PERCENT_GUARD) + '"'
|
||||
|
|
@ -26,6 +26,64 @@ def _owners(*backup_paths):
|
|||
|
||||
CLAUDE_SETTINGS_MODULE = "litellm.proxy.client.cli.commands.claude_settings"
|
||||
AUTH_MODULE = "litellm.proxy.client.cli.commands.auth"
|
||||
WINDOWS_LITE_EXE = "C:\\Users\\u\\AppData\\Local\\Programs\\Python\\Python313\\Scripts\\lite.EXE"
|
||||
|
||||
CMD_METACHARACTERS = frozenset("&|<>^()")
|
||||
CMD_PERCENT_GUARD = "%%cd:~,%"
|
||||
|
||||
|
||||
def _through_cmd_exe(command):
|
||||
"""The line cmd.exe hands to CreateProcess after reading the apiKeyHelper.
|
||||
|
||||
A `"` toggles cmd's quote state and the metacharacters only act outside it. cmd expands
|
||||
`%VAR%` even inside quotes, so every `%` has to arrive as the `%%cd:~,%` guard: the first
|
||||
`%` has no variable name and stays literal, and `%cd:~,%` is a zero length substring of `cd`.
|
||||
"""
|
||||
assert not any(CMD_METACHARACTERS & set(run) for run in command.split('"')[::2]), command
|
||||
assert command.count("%") == 3 * command.count(CMD_PERCENT_GUARD), command
|
||||
return command.replace(CMD_PERCENT_GUARD, "%")
|
||||
|
||||
|
||||
def _through_c_runtime(command_line):
|
||||
"""argv as the Microsoft C runtime builds it for the `lite` executable.
|
||||
|
||||
Outside quotes whitespace ends an argument. A `"` toggles quoting, and inside quotes `""`
|
||||
is a literal quote. Backslashes are literal unless they run up to a `"`, where each pair
|
||||
is one backslash and an odd one left over makes the quote literal.
|
||||
"""
|
||||
argv = []
|
||||
current = None
|
||||
quoted = False
|
||||
i = 0
|
||||
while i < len(command_line):
|
||||
ch = command_line[i]
|
||||
if ch in " \t" and not quoted:
|
||||
if current is not None:
|
||||
argv.append(current)
|
||||
current = None
|
||||
i += 1
|
||||
continue
|
||||
if current is None:
|
||||
current = ""
|
||||
if ch == "\\":
|
||||
run = len(command_line[i:]) - len(command_line[i:].lstrip("\\"))
|
||||
before_quote = command_line[i + run : i + run + 1] == '"'
|
||||
current += "\\" * (run // 2 if before_quote else run)
|
||||
if before_quote and run % 2:
|
||||
current += '"'
|
||||
i += 1
|
||||
i += run
|
||||
elif ch == '"':
|
||||
if quoted and command_line[i + 1 : i + 2] == '"':
|
||||
current += '"'
|
||||
i += 1
|
||||
else:
|
||||
quoted = not quoted
|
||||
i += 1
|
||||
else:
|
||||
current += ch
|
||||
i += 1
|
||||
return argv if current is None else [*argv, current]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
|
|
@ -199,6 +257,36 @@ class TestApiKeyHelperIsActuallyInvocable:
|
|||
|
||||
assert "Not authenticated for this server" in result.output
|
||||
|
||||
def _windows_argv(self, lite_exe, base_url):
|
||||
with patch(f"{CLAUDE_SETTINGS_MODULE}.shutil.which", return_value=lite_exe):
|
||||
helper = resolve_api_key_helper(base_url, platform="win32")
|
||||
return _through_c_runtime(_through_cmd_exe(helper))
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("lite_exe", "base_url"),
|
||||
[
|
||||
(WINDOWS_LITE_EXE, "http://localhost:4000"),
|
||||
("C:\\Program Files\\LiteLLM\\lite.EXE", "https://gateway.example.com/?a=1&b=2"),
|
||||
("C:\\Users\\u\\Scripts\\lite.EXE", "https://gateway.example.com/team%20a/%7Eproxy"),
|
||||
('C:\\odd "dir"\\lite.EXE', "http://localhost:4000/x\\"),
|
||||
],
|
||||
)
|
||||
def test_the_windows_command_survives_cmd_exe_and_the_c_runtime(self, lite_exe, base_url):
|
||||
assert self._windows_argv(lite_exe, base_url) == [lite_exe, "--base-url", base_url, "auth", "print-token"]
|
||||
|
||||
def test_the_windows_command_carries_the_base_url_through_cmd_quoting(self):
|
||||
stale = CliTokenRecord(
|
||||
base_url="http://other-proxy.example.com",
|
||||
key="sk-stale",
|
||||
timestamp=time.time(),
|
||||
)
|
||||
argv = self._windows_argv(WINDOWS_LITE_EXE, "http://localhost:4000")
|
||||
with patch(f"{AUTH_MODULE}.load_cli_token", return_value=stale):
|
||||
result = CliRunner().invoke(cli, argv[1:])
|
||||
|
||||
assert argv[0] == WINDOWS_LITE_EXE
|
||||
assert "Not authenticated for this server" in result.output
|
||||
|
||||
|
||||
class TestConflictingOwnersOfTheSettingsFile:
|
||||
"""Both `lite up` and `lite autoroute up` restore a backup when they stop.
|
||||
|
|
|
|||
|
|
@ -225,6 +225,32 @@ class TestResolveApiKeyHelper:
|
|||
with pytest.raises(ClaudeSettingsError, match="Could not find `lite`"):
|
||||
resolve_api_key_helper("http://localhost:4000")
|
||||
|
||||
def test_windows_quotes_for_cmd_exe_instead_of_posix_sh(self, monkeypatch):
|
||||
"""cmd.exe takes a single quote literally, so a POSIX-quoted backslashed path is unrunnable."""
|
||||
lite_exe = "C:\\Users\\u\\AppData\\Local\\Programs\\Python\\Python313\\Scripts\\lite.EXE"
|
||||
monkeypatch.setattr(shutil, "which", lambda name: lite_exe)
|
||||
|
||||
helper = resolve_api_key_helper("https://gateway.example.com", platform="win32")
|
||||
|
||||
assert helper == f'"{lite_exe}" "--base-url" "https://gateway.example.com" "auth" "print-token"'
|
||||
|
||||
def test_windows_keeps_a_spaced_path_and_a_metacharacter_url_as_single_tokens(self, monkeypatch):
|
||||
monkeypatch.setattr(shutil, "which", lambda name: "C:\\Program Files\\LiteLLM\\lite.EXE")
|
||||
|
||||
helper = resolve_api_key_helper("https://gateway.example.com/?a=1&b=2", platform="win32")
|
||||
|
||||
assert helper == (
|
||||
'"C:\\Program Files\\LiteLLM\\lite.EXE" "--base-url" "https://gateway.example.com/?a=1&b=2" '
|
||||
'"auth" "print-token"'
|
||||
)
|
||||
|
||||
def test_non_windows_platforms_keep_posix_quoting(self, monkeypatch):
|
||||
monkeypatch.setattr(shutil, "which", lambda name: "/usr/local/bin/lite")
|
||||
|
||||
helper = resolve_api_key_helper("http://example.com/path; rm -rf /", platform="darwin")
|
||||
|
||||
assert helper == "/usr/local/bin/lite --base-url 'http://example.com/path; rm -rf /' auth print-token"
|
||||
|
||||
|
||||
def _make_ctx(base_url):
|
||||
return click.Context(click.Command("test"), obj={"base_url": base_url})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue