mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-10 08:28:25 +00:00
fix(cli): keep the browser off lite's pipes and name an old proxy on sign-in failure
The browser launcher now runs with stdin, stdout and stderr on /dev/null, so a browser that lite starts on Linux no longer holds an agent's output pipe open after lite exits. A proxy that answers 404 for browser sign-in discovery now gets an error saying it is likely older than v1.99.0 instead of a hint at lite login --pkce, which that proxy cannot run
This commit is contained in:
parent
d3ad6dd4a6
commit
86d90e1be4
3 changed files with 56 additions and 9 deletions
|
|
@ -43,8 +43,14 @@ def _ssh_session_without_a_display(environ: Mapping[str, str]) -> bool:
|
|||
_OPEN_URL_IN_BROWSER: Final = "import sys, webbrowser; sys.exit(0 if webbrowser.open(sys.argv[1]) else 1)"
|
||||
|
||||
|
||||
def open_browser_off_stdout(url: str) -> PkceFailure | None:
|
||||
launcher: Final = subprocess.run((sys.executable, "-I", "-c", _OPEN_URL_IN_BROWSER, url), stdout=2, check=False)
|
||||
def open_browser_detached(url: str) -> PkceFailure | None:
|
||||
launcher: Final = subprocess.run(
|
||||
(sys.executable, "-I", "-c", _OPEN_URL_IN_BROWSER, url),
|
||||
stdin=subprocess.DEVNULL,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
check=False,
|
||||
)
|
||||
return None if launcher.returncode == 0 else PkceFailure("the web browser did not open")
|
||||
|
||||
|
||||
|
|
@ -53,7 +59,7 @@ def pkce_browser_sign_in(
|
|||
echo: Callable[[str], None],
|
||||
find_browser: Callable[[], webbrowser.BaseBrowser] = webbrowser.get,
|
||||
environ: Mapping[str, str] = os.environ,
|
||||
open_browser: Callable[[str], object] = open_browser_off_stdout,
|
||||
open_browser: Callable[[str], object] = open_browser_detached,
|
||||
) -> PkceCredential | PkceFailure:
|
||||
if _runs_in_ci(environ):
|
||||
return PkceFailure("the CI environment variable is set, so lite does not open a browser")
|
||||
|
|
@ -140,17 +146,26 @@ def _attempt_with_newer_stored_key(
|
|||
return _attempt_with_key(run, ctx, stored_key)
|
||||
|
||||
|
||||
def _sign_in_failure(base_url: str, failure: PkceFailure) -> str:
|
||||
if failure.proxy_predates_pkce:
|
||||
return (
|
||||
f"The proxy at {base_url} has no browser sign-in for lite, so it is likely older than v1.99.0. "
|
||||
"Upgrade it, run `lite login` if it uses SSO, or set LITELLM_PROXY_API_KEY."
|
||||
)
|
||||
return (
|
||||
f"Could not sign in to {base_url} through the browser: {failure.reason}. "
|
||||
"Run `lite login --pkce` from a terminal on a machine with a browser, `lite login` on one without, "
|
||||
"or set LITELLM_PROXY_API_KEY."
|
||||
)
|
||||
|
||||
|
||||
def _sign_in(ctx: click.Context, context: _SignInContext) -> str:
|
||||
base_url: Final = context["base_url"]
|
||||
_echo_to_stderr(f"The proxy at {base_url} needs a sign-in.")
|
||||
sign_in: Final = context.get("browser_sign_in") or pkce_browser_sign_in
|
||||
credential: Final = sign_in(base_url, _echo_to_stderr)
|
||||
if isinstance(credential, PkceFailure):
|
||||
raise click.ClickException(
|
||||
f"Could not sign in to {base_url} through the browser: {credential.reason}. "
|
||||
"Run `lite login --pkce` from a terminal on a machine with a browser, `lite login` on one without, "
|
||||
"or set LITELLM_PROXY_API_KEY."
|
||||
)
|
||||
raise click.ClickException(_sign_in_failure(base_url, credential))
|
||||
stored: Final = replace_stored_token(
|
||||
pkce_token_record(base_url, credential), requests.Session(), context_secret_vault(ctx)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import webbrowser
|
|||
from base64 import urlsafe_b64encode
|
||||
from collections.abc import Callable, Mapping, Sequence
|
||||
from dataclasses import dataclass
|
||||
from http import HTTPStatus
|
||||
from http.server import BaseHTTPRequestHandler, HTTPServer
|
||||
from types import MappingProxyType
|
||||
from typing import TYPE_CHECKING, Final, Literal, Protocol
|
||||
|
|
@ -68,6 +69,7 @@ class _TokenResponse(LiteLLMBaseModel):
|
|||
@dataclass(frozen=True, slots=True)
|
||||
class PkceFailure:
|
||||
reason: str
|
||||
proxy_predates_pkce: bool = False
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
|
|
@ -208,7 +210,8 @@ def discover_cli_auth(base_url: str, http: Http) -> CliAuthContract | PkceFailur
|
|||
return PkceFailure(f"could not reach {url}: {exc}")
|
||||
if response.status_code != 200:
|
||||
return PkceFailure(
|
||||
f"{url} answered {response.status_code}; this proxy version does not support `lite login --pkce`"
|
||||
f"{url} answered {response.status_code}; this proxy version does not support `lite login --pkce`",
|
||||
proxy_predates_pkce=response.status_code == HTTPStatus.NOT_FOUND,
|
||||
)
|
||||
try:
|
||||
contract: Final = CliAuthContract.model_validate(response.json())
|
||||
|
|
|
|||
|
|
@ -385,6 +385,35 @@ def test_a_failed_sign_in_names_the_reason_and_points_at_lite_login(
|
|||
assert get_stored_api_key(BASE_URL, vault=vault) is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("discovery_status", "advice"),
|
||||
[
|
||||
(404, "so it is likely older than v1.99.0. Upgrade it, run `lite login` if it uses SSO"),
|
||||
(503, "Run `lite login --pkce` from a terminal on a machine with a browser"),
|
||||
],
|
||||
ids=["proxy_without_browser_sign_in", "proxy_erroring"],
|
||||
)
|
||||
def test_a_proxy_without_browser_sign_in_is_named_as_too_old_instead_of_pointing_at_lite_login_pkce(
|
||||
requests_mock: Mocker, secret_vault_factory: type[FakeSecretVault], discovery_status: int, advice: str
|
||||
) -> None:
|
||||
requests_mock.post(f"{BASE_URL}/key/generate", status_code=401, json=REJECTED)
|
||||
requests_mock.get(f"{BASE_URL}/.well-known/litellm-cli-auth", status_code=discovery_status)
|
||||
|
||||
def sign_in_with_a_picked_browser(base_url: str, echo: Callable[[str], None]) -> PkceCredential | PkceFailure:
|
||||
return pkce_browser_sign_in(
|
||||
base_url, echo, find_browser=lambda: webbrowser.GenericBrowser("w3m"), environ={"BROWSER": "w3m"}
|
||||
)
|
||||
|
||||
result: Final = CliRunner().invoke(
|
||||
cli,
|
||||
["--base-url", BASE_URL, "keys", "generate"],
|
||||
obj={"secret_vault": secret_vault_factory(), "browser_sign_in": sign_in_with_a_picked_browser},
|
||||
)
|
||||
|
||||
assert result.exit_code == 1
|
||||
assert advice in result.stderr
|
||||
|
||||
|
||||
def test_a_signed_in_key_the_proxy_also_rejects_is_not_followed_by_another_sign_in(
|
||||
requests_mock: Mocker, secret_vault_factory: type[FakeSecretVault]
|
||||
) -> None:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue