-
Notifications
You must be signed in to change notification settings - Fork 46
REFACTOR: Migrate Connection string sanitization from regex to parser-based #522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9916cb1
e5b2873
393c38c
fb80de5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -314,20 +314,84 @@ def test_pwd_sanitization(self, cleanup_logger): | |||||||||||||||||||||||||||||||||||||||||||||
| assert "secret123" not in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_pwd_case_insensitive(self, cleanup_logger): | ||||||||||||||||||||||||||||||||||||||||||||||
| """PWD/Pwd/pwd should all be sanitized (case-insensitive)""" | ||||||||||||||||||||||||||||||||||||||||||||||
| """PWD/Pwd/pwd should all be sanitized to canonical PWD=***""" | ||||||||||||||||||||||||||||||||||||||||||||||
| from mssql_python.helpers import sanitize_connection_string | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| test_cases = [ | ||||||||||||||||||||||||||||||||||||||||||||||
| ("Server=localhost;PWD=secret;Database=test", "PWD=***"), | ||||||||||||||||||||||||||||||||||||||||||||||
| ("Server=localhost;Pwd=secret;Database=test", "Pwd=***"), | ||||||||||||||||||||||||||||||||||||||||||||||
| ("Server=localhost;pwd=secret;Database=test", "pwd=***"), | ||||||||||||||||||||||||||||||||||||||||||||||
| "Server=localhost;PWD=secret;Database=test", | ||||||||||||||||||||||||||||||||||||||||||||||
|
bewithgaurav marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||||||||||||||
| "Server=localhost;Pwd=secret;Database=test", | ||||||||||||||||||||||||||||||||||||||||||||||
|
bewithgaurav marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||||||||||||||
| "Server=localhost;pwd=secret;Database=test", | ||||||||||||||||||||||||||||||||||||||||||||||
|
bewithgaurav marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| for conn_str, expected in test_cases: | ||||||||||||||||||||||||||||||||||||||||||||||
| for conn_str in test_cases: | ||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = sanitize_connection_string(conn_str) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert expected in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "PWD=***" in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "secret" not in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_pwd_braced_value_with_semicolon(self, cleanup_logger): | ||||||||||||||||||||||||||||||||||||||||||||||
| """PWD with braced value containing semicolons must be fully masked.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| from mssql_python.helpers import sanitize_connection_string | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| conn_str = "Server=localhost;PWD={Top;Secret};Database=test" | ||||||||||||||||||||||||||||||||||||||||||||||
|
bewithgaurav marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = sanitize_connection_string(conn_str) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| assert "PWD=***" in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "Top" not in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "Secret" not in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_pwd_braced_value_with_escaped_braces(self, cleanup_logger): | ||||||||||||||||||||||||||||||||||||||||||||||
| """PWD with escaped closing braces (}}) must be fully masked.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| from mssql_python.helpers import sanitize_connection_string | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| conn_str = "Server=localhost;PWD={p}}w{{d};Database=test" | ||||||||||||||||||||||||||||||||||||||||||||||
|
bewithgaurav marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = sanitize_connection_string(conn_str) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| assert "PWD=***" in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "p}w{d" not in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_pwd_braced_value_multiple_semicolons(self, cleanup_logger): | ||||||||||||||||||||||||||||||||||||||||||||||
| """PWD with multiple semicolons inside braces must be fully masked.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| from mssql_python.helpers import sanitize_connection_string | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| conn_str = "Server=localhost;PWD={a;b;c;d};Database=test" | ||||||||||||||||||||||||||||||||||||||||||||||
|
bewithgaurav marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = sanitize_connection_string(conn_str) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| assert "PWD=***" in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| for fragment in ("a;b;c;d", "{a;", "b;c", "c;d}"): | ||||||||||||||||||||||||||||||||||||||||||||||
| assert fragment not in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_pwd_at_end_of_string(self, cleanup_logger): | ||||||||||||||||||||||||||||||||||||||||||||||
| """PWD at end of connection string (no trailing semicolon) must be masked.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| from mssql_python.helpers import sanitize_connection_string | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| conn_str = "Server=localhost;Database=test;PWD=secret" | ||||||||||||||||||||||||||||||||||||||||||||||
|
bewithgaurav marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = sanitize_connection_string(conn_str) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| assert "PWD=***" in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "secret" not in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_no_pwd_unchanged(self, cleanup_logger): | ||||||||||||||||||||||||||||||||||||||||||||||
| """Connection string without PWD should be returned intact.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| from mssql_python.helpers import sanitize_connection_string | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| conn_str = "Server=localhost;Database=test;UID=user" | ||||||||||||||||||||||||||||||||||||||||||||||
|
bewithgaurav marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||||||||||||||
| sanitized = sanitize_connection_string(conn_str) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| assert "Server=" in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "Database=" in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "UID=" in sanitized | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+373
to
+382
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_no_pwd_unchanged(self, cleanup_logger): | |
| """Connection string without PWD should be returned intact.""" | |
| from mssql_python.helpers import sanitize_connection_string | |
| conn_str = "Server=localhost;Database=test;UID=user" | |
| sanitized = sanitize_connection_string(conn_str) | |
| assert "Server=" in sanitized | |
| assert "Database=" in sanitized | |
| assert "UID=" in sanitized | |
| def test_no_pwd_preserves_non_sensitive_fields(self, cleanup_logger): | |
| """Connection string without PWD should preserve non-sensitive fields, even if reformatted.""" | |
| from mssql_python.helpers import sanitize_connection_string | |
| conn_str = "Server=localhost;Database=test;UID=user" | |
| sanitized = sanitize_connection_string(conn_str) | |
| assert "Server=localhost" in sanitized | |
| assert "Database=test" in sanitized | |
| assert "UID=user" in sanitized | |
| assert "PWD=***" not in sanitized | |
| assert "redacted" not in sanitized.lower() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sanitize_connection_string() claims to mask both PWD and Password, but when the input uses the key "Password" the parser lowercases it to "password" and normalize_key() returns None (since "password" isn’t in _ALLOWED_CONNECTION_STRING_PARAMS). This causes the sanitized output key to be "password=", which won’t satisfy existing expectations that look for "Password=" (e.g., logging integration tests). Consider adding a canonical mapping for "password" (or special-casing it here) so the emitted key is consistently "Password" or "PWD".