From 7da9cc7edfb8b23198238a41a8516c915d739c90 Mon Sep 17 00:00:00 2001 From: Kolade Fajimi <107228310+koladefaj@users.noreply.github.com> Date: Tue, 8 Sep 2026 07:15:34 +0100 Subject: [PATCH] fix(vertex): log the credentials path instead of returning it to the caller --- litellm/llms/vertex_ai/credentials_source.py | 61 +++++++++------ .../llms/vertex_ai/test_vertex_llm_base.py | 78 ++++++++++--------- tests/test_litellm/test_secret_redaction.py | 6 +- 3 files changed, 85 insertions(+), 60 deletions(-) diff --git a/litellm/llms/vertex_ai/credentials_source.py b/litellm/llms/vertex_ai/credentials_source.py index 1c931edc11a..b59520d0fbe 100644 --- a/litellm/llms/vertex_ai/credentials_source.py +++ b/litellm/llms/vertex_ai/credentials_source.py @@ -13,7 +13,7 @@ from typing import Final, NoReturn, TypeAlias from pydantic import TypeAdapter, ValidationError from typing_extensions import assert_never -from litellm.litellm_core_utils.secret_redaction import redact_string +from litellm._logging import verbose_logger @dataclass(frozen=True, slots=True) @@ -47,6 +47,9 @@ VertexCredentialsFailure: TypeAlias = ( VertexCredentialsFileUnreadable | VertexCredentialsFileNotJson | VertexCredentialsInlineNotJson ) VertexCredentialsSource: TypeAlias = VertexCredentialsJson | VertexCredentialsFailure +_VertexCredentialsFile: TypeAlias = ( + VertexCredentialsJson | VertexCredentialsFileUnreadable | VertexCredentialsFileNotJson +) _JSON_OBJECT_ADAPTER: Final = TypeAdapter(dict[str, object]) @@ -62,6 +65,21 @@ def _parse_json_object(raw: str) -> Mapping[str, object] | _NotAJsonObject: return _NotAJsonObject("; ".join(detail["msg"] for detail in e.errors())) +def _read_json_file(path: str) -> _VertexCredentialsFile: + try: + with open(path, encoding="utf-8") as f: + contents: Final = f.read() + except OSError as e: + return VertexCredentialsFileUnreadable(path, f"{e.strerror or e} ({type(e).__name__})") + except UnicodeDecodeError: + return VertexCredentialsFileNotJson(path, "file is not UTF-8 text") + + parsed: Final = _parse_json_object(contents) + if isinstance(parsed, _NotAJsonObject): + return VertexCredentialsFileNotJson(path, parsed.detail) + return VertexCredentialsJson(parsed) + + def is_inline_credentials_json(credentials: str) -> bool: """Whether *credentials* carries the JSON itself rather than a path to a file holding it.""" return credentials.lstrip().startswith("{") @@ -74,42 +92,39 @@ def load_vertex_credentials_source(credentials: str) -> VertexCredentialsSource: failure recognisable: `os.path.exists()` answers False for an unreadable path as well as an absent one, so both used to reach the inline branch and be reported as malformed JSON. """ - if is_inline_credentials_json(credentials): - inline: Final = _parse_json_object(credentials) - if isinstance(inline, _NotAJsonObject): - return VertexCredentialsInlineNotJson(inline.detail) + if not is_inline_credentials_json(credentials): + return _read_json_file(credentials) + + inline: Final = _parse_json_object(credentials) + if not isinstance(inline, _NotAJsonObject): return VertexCredentialsJson(inline) - try: - with open(credentials, encoding="utf-8") as f: - contents: Final = f.read() - except OSError as e: - return VertexCredentialsFileUnreadable(credentials, f"{e.strerror or e} ({type(e).__name__})") - except UnicodeDecodeError: - return VertexCredentialsFileNotJson(credentials, "file is not UTF-8 text") - - from_file: Final = _parse_json_object(contents) - if isinstance(from_file, _NotAJsonObject): - return VertexCredentialsFileNotJson(credentials, from_file.detail) - return VertexCredentialsJson(from_file) + # A file can legitimately be named "{vertex}.json", so a brace-prefixed value that does + # not parse is still given the file read it used to get before dispatch moved to shape. + from_file: Final = _read_json_file(credentials) + if isinstance(from_file, VertexCredentialsJson): + return from_file + return VertexCredentialsInlineNotJson(inline.detail) def raise_vertex_credentials_failure(failure: VertexCredentialsFailure) -> NoReturn: - """Map a load failure onto the ValueError the auth flow already surfaces as a 500. + """Map a load failure onto the ValueError the auth flow already surfaces to the caller. - A path names itself in the message so the operator's log says which file failed. The - proxy scrubs filesystem paths out of what it hands back to the API caller, and the value - is redacted first so a non-path value misconfigured here cannot leak instead. + The caller is told which of the three faults it was; the path goes to the proxy log + instead, because the message reaches whoever sent the request and the operator who can + act on the path is reading the log anyway. """ match failure: case VertexCredentialsFileUnreadable(path=path, reason=reason): + verbose_logger.error("Vertex: cannot read the credentials file at %s: %s", path, reason) raise ValueError( - f"Unable to read the vertex credentials file at {redact_string(path)}: {reason}. " + f"Unable to read the vertex credentials file: {reason}. The proxy log names the path. " "Set `vertex_credentials` to a readable file path, or to the credentials JSON itself." ) case VertexCredentialsFileNotJson(path=path, detail=detail): + verbose_logger.error("Vertex: credentials file at %s is not valid JSON: %s", path, detail) raise ValueError( - f"The vertex credentials file at {redact_string(path)} is not valid JSON: {detail}. " + f"The vertex credentials file is not valid JSON: {detail}. The proxy log names the path. " "Check for unescaped newlines in private_key." ) case VertexCredentialsInlineNotJson(detail=detail): diff --git a/tests/test_litellm/llms/vertex_ai/test_vertex_llm_base.py b/tests/test_litellm/llms/vertex_ai/test_vertex_llm_base.py index 1edceff1123..d55901e79fe 100644 --- a/tests/test_litellm/llms/vertex_ai/test_vertex_llm_base.py +++ b/tests/test_litellm/llms/vertex_ai/test_vertex_llm_base.py @@ -1,5 +1,6 @@ import asyncio import json +import logging from unittest.mock import MagicMock, call, patch import pytest @@ -2204,39 +2205,45 @@ class TestVertexCredentialsSource: JSONDecodeError blaming the key material. """ - def test_missing_credentials_file_names_the_path_not_the_json(self, tmp_path): + def test_missing_credentials_file_names_the_read_failure(self, tmp_path, caplog): missing = tmp_path / "vertexai.json" - with pytest.raises(ValueError, match="No such file or directory") as exc_info: - VertexBase().load_auth(credentials=str(missing), project_id="p") + with caplog.at_level(logging.ERROR, logger="LiteLLM"): + with pytest.raises(ValueError, match="No such file or directory") as exc_info: + VertexBase().load_auth(credentials=str(missing), project_id="p") message = str(exc_info.value) - assert str(missing) in message assert "not valid JSON" not in message + assert str(missing) not in message + assert str(missing) in caplog.text - def test_unreadable_credentials_file_names_the_read_failure(self, tmp_path): + def test_unreadable_credentials_file_names_the_read_failure(self, tmp_path, caplog): # Present but not openable as a file, the same shape as a path on a mount # that has stopped serving reads. unreadable = tmp_path / "vertexai.json" unreadable.mkdir() - with pytest.raises(ValueError, match="Unable to read the vertex credentials file") as exc_info: - VertexBase().load_auth(credentials=str(unreadable), project_id="p") + with caplog.at_level(logging.ERROR, logger="LiteLLM"): + with pytest.raises(ValueError, match="Unable to read the vertex credentials file") as exc_info: + VertexBase().load_auth(credentials=str(unreadable), project_id="p") message = str(exc_info.value) - assert str(unreadable) in message assert "not valid JSON" not in message + assert str(unreadable) not in message + assert str(unreadable) in caplog.text - def test_malformed_credentials_file_names_the_file_and_keeps_the_private_key_hint(self, tmp_path): + def test_malformed_credentials_file_keeps_the_private_key_hint(self, tmp_path, caplog): malformed = tmp_path / "vertexai.json" malformed.write_text('{"type": "service_account", "private_key": "-----BEGIN\nPRIVATE KEY-----"}') - with pytest.raises(ValueError, match="is not valid JSON") as exc_info: - VertexBase().load_auth(credentials=str(malformed), project_id="p") + with caplog.at_level(logging.ERROR, logger="LiteLLM"): + with pytest.raises(ValueError, match="is not valid JSON") as exc_info: + VertexBase().load_auth(credentials=str(malformed), project_id="p") message = str(exc_info.value) - assert str(malformed) in message assert "private_key" in message + assert str(malformed) not in message + assert str(malformed) in caplog.text def test_malformed_inline_credentials_do_not_echo_the_credential(self): inline = ( @@ -2249,6 +2256,15 @@ class TestVertexCredentialsSource: ) as exc_info: VertexBase().load_auth(credentials=inline, project_id="p") + assert "MIIEvQIBADA" not in str(exc_info.value) + + def test_a_credential_misconfigured_as_a_path_is_not_echoed(self): + """A value that is neither a path nor JSON must not come back in the message.""" + pem = "-----BEGIN PRIVATE KEY-----\nMIIEvQIBADA\n-----END PRIVATE KEY-----" + + with pytest.raises(ValueError, match="Unable to read the vertex credentials file") as exc_info: + VertexBase().load_auth(credentials=pem, project_id="p") + message = str(exc_info.value) assert "MIIEvQIBADA" not in message assert "BEGIN PRIVATE KEY" not in message @@ -2288,28 +2304,20 @@ class TestVertexCredentialsSource: assert project_id == "from-inline" assert from_sa.call_args.args[0] == {"type": "service_account", "project_id": "from-inline"} - def test_credentials_path_reaches_the_operator_log_but_not_the_api_caller(self, tmp_path): - """The path is the actionable detail, so it is in the message the proxy logs. - What the proxy hands back to the caller goes through redact_internal_details.""" - from litellm.litellm_core_utils.secret_redaction import redact_internal_details + def test_a_file_whose_name_starts_with_a_brace_is_still_read(self, tmp_path): + """Shape dispatch must not strand a real file that happens to be named like JSON.""" + braced = tmp_path / "{vertex}.json" + braced.write_text(json.dumps({"type": "service_account", "project_id": "from-braced-file"})) + vertex_base = VertexBase() + mock_creds = MagicMock() + mock_creds.project_id = "from-braced-file" - missing = tmp_path / "vertexai.json" + with ( + patch.object(vertex_base, "_credentials_from_service_account", return_value=mock_creds) as from_sa, + patch.object(vertex_base, "refresh_auth"), + ): + creds, project_id = vertex_base.load_auth(credentials=str(braced), project_id=None) - with pytest.raises(ValueError, match="Unable to read the vertex credentials file") as exc_info: - VertexBase().load_auth(credentials=str(missing), project_id="p") - - logged = str(exc_info.value) - assert str(missing) in logged - assert str(missing) not in redact_internal_details(logged) - - def test_a_credential_misconfigured_as_a_path_is_redacted_not_echoed(self): - """A value that is neither a path nor JSON still lands in the file branch, - so it is scrubbed before it reaches the message.""" - pem = "-----BEGIN PRIVATE KEY-----\nMIIEvQIBADA\n-----END PRIVATE KEY-----" - - with pytest.raises(ValueError, match="Unable to read the vertex credentials file") as exc_info: - VertexBase().load_auth(credentials=pem, project_id="p") - - message = str(exc_info.value) - assert "MIIEvQIBADA" not in message - assert "BEGIN PRIVATE KEY" not in message + assert creds is mock_creds + assert project_id == "from-braced-file" + assert from_sa.call_args.args[0] == {"type": "service_account", "project_id": "from-braced-file"} diff --git a/tests/test_litellm/test_secret_redaction.py b/tests/test_litellm/test_secret_redaction.py index 730a869ddb7..65bdd799f87 100644 --- a/tests/test_litellm/test_secret_redaction.py +++ b/tests/test_litellm/test_secret_redaction.py @@ -462,9 +462,10 @@ def test_vertex_error_message_no_credential_leak(): raise_vertex_credentials_failure, ) + path = "/etc/litellm/vertexai.json" failures = ( - VertexCredentialsFileUnreadable("/etc/litellm/vertexai.json", "No such file or directory (FileNotFoundError)"), - VertexCredentialsFileNotJson("/etc/litellm/vertexai.json", "Invalid control character at: line 1 column 55"), + VertexCredentialsFileUnreadable(path, "No such file or directory (FileNotFoundError)"), + VertexCredentialsFileNotJson(path, "Invalid control character at: line 1 column 55"), VertexCredentialsInlineNotJson("Expecting value: line 1 column 1 (char 0)"), ) @@ -472,6 +473,7 @@ def test_vertex_error_message_no_credential_leak(): with pytest.raises(ValueError, match="vertex") as exc_info: raise_vertex_credentials_failure(failure) message = str(exc_info.value) + assert path not in message # the path goes to the log, not to the API caller assert _redact_string(message) == message # nothing to redact