Redact password from DatabaseSessionService engine-creation errors#6485
Open
herdiyana256 wants to merge 1 commit into
Open
Redact password from DatabaseSessionService engine-creation errors#6485herdiyana256 wants to merge 1 commit into
herdiyana256 wants to merge 1 commit into
Conversation
DatabaseSessionService.__init__ embedded the raw db_url in three ValueError messages raised when engine creation fails (invalid URL argument, missing driver module, or any other error). A SQLAlchemy URL for a networked backend carries the password inline, e.g. postgresql+asyncpg://user:password@host/db, so any of these failures (for example a driver that is not installed) surfaced the database password verbatim in the exception and in any logs that record it. Render the URL with hide_password=True before including it in the messages, falling back to a redacted placeholder when the URL cannot be parsed. The messages keep the useful dialect/host/database context while no longer exposing the password.
adk-bot
added
the
services
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DatabaseSessionService.init includes the database URL in the ValueError it raises when engine creation fails. There are three such messages: one for an invalid URL argument (ArgumentError), one for a missing driver module (ImportError), and a generic one for any other failure. Each interpolated the raw db_url directly into the message text.
A SQLAlchemy URL for a networked backend carries the password inline, for example postgresql+asyncpg://user:password@host/db. As a result, any of these failures surfaces the database password verbatim in the exception and in any logs that record it. The most common trigger is entirely benign: configuring a Postgres or MySQL URL without having installed the corresponding async driver raises the ImportError branch, which then prints the full URL including the password. The underlying SQLAlchemy exceptions in these paths do not contain the password, so the disclosure is introduced solely by these messages.
This change renders the URL with hide_password=True before including it in the messages, so the password is replaced with *** while the dialect, host, and database remain visible for debugging. When the URL cannot be parsed at all, it falls back to a redacted placeholder rather than echoing the raw string.
Verified against the current published class: before the change, instantiating DatabaseSessionService with a password-bearing Postgres URL whose driver is not installed raises
... 'postgresql+asyncpg://admin:SUPER_SECRET_PW@db.internal:5432/prod'with the password in clear text; after the change the same call reports...admin:***@db.internal:5432/prodand the password no longer appears in the error for any of the three branches.