Issue #938: Redact DATABASE_URL password before logging at startup#951
Open
bjagg wants to merge 1 commit into
Open
Issue #938: Redact DATABASE_URL password before logging at startup#951bjagg wants to merge 1 commit into
bjagg wants to merge 1 commit into
Conversation
… at startup `database_setup.py` was logging the full `DATABASE_URL` (including the PG password) on every MDR API task startup. The shared `dev`/`demo` CloudWatch log group has broader read permissions than the SSM parameter that backs the password, so anyone with `logs:FilterLogEvents` could recover the credential by tailing the stream during startup — and every restart leaves another copy in retention history. Add a small `_redact_url` helper that uses `urlparse`/`urlunparse` to swap the password segment for `***` while preserving scheme, username, host, port, and database. The helper is best-effort: any parsing surprise (malformed URL, port that isn't an integer because an env var was unset) returns a `<unparseable-url>` sentinel rather than raising, so a logging path can never take down MDR startup. Adds the first tests under `test/components/lif/mdr_utils/`: - `__init__.py` (package marker matching siblings) - `conftest.py` seeds dummy POSTGRESQL_* env vars so importing `database_setup` doesn't fail at engine-construction time (these pure-Python helper tests never touch a real DB) - `test_database_setup.py` — five cases for `_redact_url` covering typical password redaction, special characters, missing port, no-password URLs (e.g. IAM auth), and unparseable input. Operational follow-up (out of scope here): rotate the dev + demo DB passwords since the previous values are in CloudWatch history. Other LIF services that share this logging pattern (graphql_*, translator_*, advisor_api, etc.) are also flagged for an audit pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 27, 2026
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
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.
Summary
database_setup.pywas logging the fullDATABASE_URL(with PG password) on every MDR API task startup. Shared CloudWatch retention means anyone withlogs:FilterLogEventscould recover the dev/demo DB credential. Closes #938.Fix
Small
_redact_url(url) -> strhelper usingurlparse/urlunparse:Output before:
Output after:
Helper is best-effort: any parsing surprise (malformed URL, missing env var producing a non-integer port) returns a
<unparseable-url>sentinel rather than raising — a logging path should never take down MDR startup.Tests
First tests under
test/components/lif/mdr_utils/(new directory):_redact_urlcovers typical password redaction, special characters in the password, no-port URLs, no-password URLs (IAM auth), and unparseable input.conftest.pyseeds dummyPOSTGRESQL_*env vars so importingdatabase_setupdoesn't fail at engine-construction time (these pure-Python tests never touch a real DB).uv run pytest test/components/lif/mdr_utils/— 5 passed.Follow-up (out of scope here)
graphql_*,translator_*,advisor_api, etc. Same pattern, same fix shape.Both worth filing as separate issues once this lands.
Risk
Very low. Pure-Python helper added, single log line changed from f-string to lazy-format with redaction. No behavior change for the engine itself —
create_async_enginestill gets the rawDATABASE_URL(it needs the credential to actually connect).🤖 Generated with Claude Code