diff --git a/.jules/sentinel.md b/.jules/sentinel.md index c704035b..b1beb440 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -57,3 +57,8 @@ **Vulnerability:** Database driver exceptions can echo DSN fragments, query parameters, or assignment-style secrets after connection failures, leaking plaintext passwords through snapshot error messages and queue logs. **Learning:** Redacting only the literal DSN is not enough. Error messages may contain decoded, percent-encoded, query-string, or `password=`/`api_key=` style forms of the same secret. **Prevention:** Sanitize snapshot job errors before persisting or re-raising them, and raise sanitized exceptions with `from None` so Python exception chaining does not reattach the original secret-bearing exception. + +## 2025-07-06 - [urllib.parse Scheme Underscore Bug] +**Vulnerability:** DSNs containing underscores in the scheme (e.g., `invalid_scheme://...`) caused `urllib.parse.urlsplit` to silently fail, resulting in an empty scheme. This bypassed credential extraction during DSN redaction, meaning passwords inside such DSNs could be leaked directly into error logs unredacted. +**Learning:** `urllib.parse.urlsplit` strictly adheres to RFC specs which don't allow underscores in schemes. Instead of raising an error, Python evaluates it as an empty scheme, leading to unpredictable parsing for the rest of the URL structure. +**Prevention:** Before parsing DSNs specifically for secret redaction or SSRF protection, normalize the scheme portion (e.g., replace `_` with `-`) or perform regex-based extractions. diff --git a/backend/app/dsn_redaction.py b/backend/app/dsn_redaction.py index 3342c3ae..8b05d6fa 100644 --- a/backend/app/dsn_redaction.py +++ b/backend/app/dsn_redaction.py @@ -16,9 +16,17 @@ ) +def _normalize_dsn_for_parsing(dsn: str) -> str: + match = re.match(r"^([a-zA-Z][a-zA-Z0-9+\-._]*):", dsn) + if match and "_" in match.group(1): + scheme = match.group(1).replace("_", "-") + return f"{scheme}:{dsn[match.end():]}" + return dsn + + def _password_candidates_from_dsn(dsn: str) -> set[str]: candidates: set[str] = set() - parsed = urlsplit(dsn) + parsed = urlsplit(_normalize_dsn_for_parsing(dsn)) if parsed.password: candidates.add(parsed.password) diff --git a/backend/tests/test_dsn_redaction.py b/backend/tests/test_dsn_redaction.py new file mode 100644 index 00000000..230de429 --- /dev/null +++ b/backend/tests/test_dsn_redaction.py @@ -0,0 +1,55 @@ +from app.dsn_redaction import redact_dsn_error_message + +def test_redact_dsn_error_message_with_underscore_scheme(): + dsn = "invalid_scheme://user:supersecretpass@host/db" + msg = "Error connecting to invalid_scheme://user:supersecretpass@host/db" + redacted = redact_dsn_error_message(msg, dsn) + assert "supersecretpass" not in redacted + assert "user:***@host" in redacted or "***" in redacted + +def test_redact_dsn_error_message_normal(): + dsn = "postgres://user:supersecretpass@host/db" + msg = "Error: password supersecretpass is wrong" + redacted = redact_dsn_error_message(msg, dsn) + assert "supersecretpass" not in redacted + assert "Error: password *** is wrong" in redacted + +def test_redact_dsn_error_message_with_query_params(): + dsn = "postgres://user:pass@host/db?password=supersecret&token=abc12345" + msg = "Failed with password=supersecret and token abc12345" + redacted = redact_dsn_error_message(msg, dsn) + assert "supersecret" not in redacted + assert "abc12345" not in redacted + +def test_redact_dsn_error_message_with_query_params_encoded(): + dsn = "postgres://user:pass@host/db?password=super%20secret&token=abc%2B123" + msg = "Failed with password=super secret and token abc+123" + redacted = redact_dsn_error_message(msg, dsn) + assert "super secret" not in redacted + assert "abc+123" not in redacted + +def test_redact_dsn_no_password_but_assignment(): + dsn = "postgres://user@host/db" + msg = "Some internal error: secret_key = mysecretvalue123" + redacted = redact_dsn_error_message(msg, dsn) + assert "mysecretvalue123" not in redacted + assert "secret_key = ***" in redacted + +def test_redact_dsn_no_match(): + dsn = "postgres://user:pass@host/db" + msg = "Connection timed out" + redacted = redact_dsn_error_message(msg, dsn) + assert redacted == "Connection timed out" + +def test_redact_dsn_empty_query_param(): + dsn = "postgres://user:pass@host/db?token=" + msg = "Failed to connect" + redacted = redact_dsn_error_message(msg, dsn) + assert redacted == msg + +def test_redact_dsn_irrelevant_query_param(): + dsn = "postgres://user:pass@host/db?timeout=30" + msg = "Failed after 30s" + redacted = redact_dsn_error_message(msg, dsn) + # The '30' should not be redacted because it's not a secret key + assert "30" in redacted