From dbacc081eab5c1d37d73bbfdd7b8eb9c69a81306 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 30 Oct 2026 07:20:16 -0700 Subject: [PATCH] fix(cli): redact a 401 body before and after removing control characters and never print an empty reason --- .../proxy/client/cli/commands/auto_sign_in.py | 7 ++- .../proxy/client/cli/test_auto_sign_in.py | 59 ++++++++++++++----- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/litellm/proxy/client/cli/commands/auto_sign_in.py b/litellm/proxy/client/cli/commands/auto_sign_in.py index cd718c1f1b5..fce24a2aab7 100644 --- a/litellm/proxy/client/cli/commands/auto_sign_in.py +++ b/litellm/proxy/client/cli/commands/auto_sign_in.py @@ -42,6 +42,7 @@ _DISPLAY_VARIABLES: Final = ("DISPLAY", "WAYLAND_DISPLAY") _SIGNED_IN_LOGIN_WITHOUT_KEY_ROW: Final = HTTPStatus.NOT_FOUND _WAIT_FOR_ANOTHER_SIGN_IN_SECONDS: Final = LOGIN_TIMEOUT_SECONDS + 60 _PROXY_REASON_CHARS: Final = 500 +_NO_PROXY_REASON: Final = "no reason given" _TERMINAL_CONTROL_CHARACTERS: Final = re.compile(r"[\x00-\x08\x0b-\x1f\x7f-\x9f]") @@ -210,15 +211,15 @@ def _attempt_with_newer_stored_key( return _attempt_with_key(run, ctx, stored_key) -def _printable(text: str) -> str: - return _TERMINAL_CONTROL_CHARACTERS.sub("", text).strip() +def _printable_and_redacted(text: str) -> str: + return redact_string(_TERMINAL_CONTROL_CHARACTERS.sub("", redact_string(text)).strip()) def _proxy_reason(error: UnauthorizedError) -> str: original: Final = error.orig_exception response: Final = original.response if isinstance(original, requests.HTTPError) else None body: Final = response.text if response is not None else "" - reason: Final = redact_string(_printable(body) or _printable(str(error))) + reason: Final = _printable_and_redacted(body) or _printable_and_redacted(str(error)) or _NO_PROXY_REASON return reason if len(reason) <= _PROXY_REASON_CHARS else f"{reason[:_PROXY_REASON_CHARS]}..." diff --git a/tests/unit/proxy/client/cli/test_auto_sign_in.py b/tests/unit/proxy/client/cli/test_auto_sign_in.py index e2851132899..7ea67c53449 100644 --- a/tests/unit/proxy/client/cli/test_auto_sign_in.py +++ b/tests/unit/proxy/client/cli/test_auto_sign_in.py @@ -22,6 +22,7 @@ if TYPE_CHECKING: from requests_mock.request import Request BASE_URL: Final = "https://proxy.example.com" +FAKE_SECRET: Final = "a1b2c3d4" * 4 SIGNED_IN_KEY: Final = "sk-signed-in" GENERATED: Final = {"key": "sk-generated", "key_alias": "experiment-1"} REJECTED: Final = {"error": {"message": "Authentication Error, No api key passed in.", "type": "auth_error"}} @@ -652,17 +653,26 @@ def test_a_long_401_body_is_cut_before_it_reaches_the_terminal( assert sign_in.calls == [] +_ROUTES_THAT_RAISE_EACH_401_ERROR_TYPE: Final = pytest.mark.parametrize( + "command, method, route", + [(["keys", "generate"], "POST", "/key/generate"), (["users", "list"], "GET", "/user/list")], + ids=["http_error", "plain_string_error"], +) + + +@_ROUTES_THAT_RAISE_EACH_401_ERROR_TYPE def test_terminal_control_characters_in_a_401_body_never_reach_the_terminal( - requests_mock: Mocker, secret_vault_factory: type[FakeSecretVault] + requests_mock: Mocker, secret_vault_factory: type[FakeSecretVault], command: list[str], method: str, route: str ) -> None: - requests_mock.post( - f"{BASE_URL}/key/generate", + requests_mock.register_uri( + method, + f"{BASE_URL}{route}", status_code=401, text="\x1b]0;title\x07refused\x1b[2J\rnext\tcolumn\nsecond line\x9b31m", ) sign_in: Final = FakeBrowserSignIn(_credential()) - result: Final = _lite(["keys", "generate"], secret_vault_factory(), sign_in, ["--api-key", "sk-explicit"]) + result: Final = _lite(command, secret_vault_factory(), sign_in, ["--api-key", "sk-explicit"]) assert result.exit_code == 1 assert "refused" in result.stderr @@ -671,28 +681,49 @@ def test_terminal_control_characters_in_a_401_body_never_reach_the_terminal( assert sign_in.calls == [] -def test_a_key_split_by_a_control_character_in_a_401_body_stays_redacted( - requests_mock: Mocker, secret_vault_factory: type[FakeSecretVault] +@_ROUTES_THAT_RAISE_EACH_401_ERROR_TYPE +@pytest.mark.parametrize( + "body", + [ + f"refused sk-{FAKE_SECRET[:9]}\x07{FAKE_SECRET[9:]}", + f"refused Bearer\r{FAKE_SECRET}", + f"refused client_secret\r{FAKE_SECRET}", + ], + ids=["key_split_by_the_character", "bearer_token_after_the_character", "named_secret_after_the_character"], +) +def test_a_secret_beside_a_control_character_in_a_401_body_stays_redacted( + requests_mock: Mocker, + secret_vault_factory: type[FakeSecretVault], + command: list[str], + method: str, + route: str, + body: str, ) -> None: - key: Final = "sk-" + "a1b2c3d4" * 4 - requests_mock.post(f"{BASE_URL}/key/generate", status_code=401, text=f"refused {key[:12]}\x07{key[12:]}") + requests_mock.register_uri(method, f"{BASE_URL}{route}", status_code=401, text=body) result: Final = _lite( - ["keys", "generate"], secret_vault_factory(), FakeBrowserSignIn(_credential()), ["--api-key", "sk-explicit"] + command, secret_vault_factory(), FakeBrowserSignIn(_credential()), ["--api-key", "sk-explicit"] ) assert result.exit_code == 1 assert "refused" in result.stderr - assert key not in result.stderr + assert FAKE_SECRET[9:] not in result.stderr -def test_a_401_body_of_only_control_characters_still_gives_a_reason( - requests_mock: Mocker, secret_vault_factory: type[FakeSecretVault] +@_ROUTES_THAT_RAISE_EACH_401_ERROR_TYPE +@pytest.mark.parametrize("body", ["", " \n", "\x1b\x07\r\x9b"], ids=["empty", "blank", "control_characters"]) +def test_a_401_body_with_nothing_printable_still_gives_a_reason( + requests_mock: Mocker, + secret_vault_factory: type[FakeSecretVault], + command: list[str], + method: str, + route: str, + body: str, ) -> None: - requests_mock.post(f"{BASE_URL}/key/generate", status_code=401, text="\x1b\x07\r\x9b") + requests_mock.register_uri(method, f"{BASE_URL}{route}", status_code=401, text=body) result: Final = _lite( - ["keys", "generate"], secret_vault_factory(), FakeBrowserSignIn(_credential()), ["--api-key", "sk-explicit"] + command, secret_vault_factory(), FakeBrowserSignIn(_credential()), ["--api-key", "sk-explicit"] ) assert result.exit_code == 1