diff --git a/src/google/adk/sessions/database_session_service.py b/src/google/adk/sessions/database_session_service.py index 54755a1fbc..d2d5054ebb 100644 --- a/src/google/adk/sessions/database_session_service.py +++ b/src/google/adk/sessions/database_session_service.py @@ -71,6 +71,22 @@ logger = logging.getLogger("google_adk." + __name__) + +def _redact_db_url(db_url: str) -> str: + """Returns the database URL with any embedded password removed. + + The engine-creation error messages below include the database URL to aid + debugging, but a URL may embed credentials + (e.g. ``postgresql://user:password@host/db``). Rendering it with + ``hide_password=True`` keeps the useful parts (dialect, host, database) while + preventing the password from leaking into exceptions and logs. + """ + try: + return make_url(db_url).render_as_string(hide_password=True) + except Exception: # noqa: BLE001 - fall back to a fully redacted placeholder + return "[redacted]" + + _STALE_SESSION_ERROR_MESSAGE = ( "The session has been modified in storage since it was loaded. " "Please reload the session before appending more events." @@ -285,16 +301,17 @@ def __init__( event.listen(db_engine.sync_engine, "connect", _set_sqlite_pragma) except Exception as e: + safe_db_url = _redact_db_url(db_url) if isinstance(e, ArgumentError): raise ValueError( - f"Invalid database URL format or argument '{db_url}'." + f"Invalid database URL format or argument '{safe_db_url}'." ) from e if isinstance(e, ImportError): raise ValueError( - f"Database related module not found for URL '{db_url}'." + f"Database related module not found for URL '{safe_db_url}'." ) from e raise ValueError( - f"Failed to create database engine for URL '{db_url}'" + f"Failed to create database engine for URL '{safe_db_url}'" ) from e else: self._owns_db_engine = False diff --git a/tests/unittests/sessions/test_session_service.py b/tests/unittests/sessions/test_session_service.py index 9ecb3d7eb7..c552759787 100644 --- a/tests/unittests/sessions/test_session_service.py +++ b/tests/unittests/sessions/test_session_service.py @@ -38,6 +38,7 @@ import pytest from sqlalchemy import delete from sqlalchemy import text +from sqlalchemy.exc import ArgumentError from sqlalchemy.ext.asyncio import create_async_engine from sqlalchemy.pool import StaticPool @@ -87,6 +88,52 @@ async def session_service(request, tmp_path): ) +_DB_URL_WITH_PASSWORD = ( + 'postgresql+psycopg2://db_user:sup3rs3cret@localhost:5432/prod' +) +_DB_PASSWORD = 'sup3rs3cret' + + +def test_redact_db_url_masks_embedded_password(): + redacted = database_session_service._redact_db_url(_DB_URL_WITH_PASSWORD) + + assert _DB_PASSWORD not in redacted + # The non-sensitive parts stay available for debugging. + assert 'localhost' in redacted + assert 'prod' in redacted + assert 'db_user' in redacted + + +def test_redact_db_url_falls_back_to_placeholder_for_unparsable_url(): + assert database_session_service._redact_db_url('::not a url::') == '[redacted]' + + +@pytest.mark.parametrize( + 'raised_error', + [ + ArgumentError('invalid url'), + ImportError('missing driver'), + RuntimeError('unexpected failure'), + ], +) +def test_database_session_service_init_error_does_not_leak_password( + raised_error, +): + """Engine-creation failures must not surface the URL password.""" + with mock.patch.object( + database_session_service, + 'create_async_engine', + side_effect=raised_error, + ): + with pytest.raises(ValueError) as exc_info: + database_session_service.DatabaseSessionService(_DB_URL_WITH_PASSWORD) + + message = str(exc_info.value) + assert _DB_PASSWORD not in message + # The redacted URL keeps the host so the error stays actionable. + assert 'localhost' in message + + def test_database_session_service_enables_pool_pre_ping_by_default(): captured_kwargs = {}