CHORE: align connection string sanitizer with its documented key masking#653
Open
bewithgaurav wants to merge 3 commits into
Open
CHORE: align connection string sanitizer with its documented key masking#653bewithgaurav wants to merge 3 commits into
bewithgaurav wants to merge 3 commits into
Conversation
sanitize_connection_string documents masking of the PWD and Password keys, but _SENSITIVE_KEYS only listed pwd, so the password synonym passed through unmasked. add password to _SENSITIVE_KEYS so the implementation matches the docstring, and add regression tests covering password / Password / PASSWORD and a braced value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.pybind.ddbc_bindings.cpp: 76.3%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%🔗 Quick Links
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns sanitize_connection_string behavior with its documented contract by ensuring the Password synonym is treated as sensitive and masked, and adds regression tests to cover common password casings and braced values.
Changes:
- Add
"password"to the sanitizer’s_SENSITIVE_KEYSset soPassword=values are masked. - Add tests covering
password/Password/PASSWORDand a braced password value containing semicolons.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
mssql_python/connection_string_parser.py |
Expands sensitive key masking to include password alongside pwd. |
tests/test_007_logging.py |
Adds regression tests for password synonym masking, casing variants, and braced values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t absence address review feedback: the password synonym tests asserted only that the secret substring was absent, which would also pass under full-redaction fallback. assert password=*** is present and that the output did not fall back to the redacted placeholder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
gargsaumya
approved these changes
Jul 1, 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.
Work Item / Issue Reference
Summary
sanitize_connection_string documents masking of the PWD and Password keys, but
_SENSITIVE_KEYS only listed pwd, so the Password synonym passed through unmasked.
this adds password to _SENSITIVE_KEYS so the implementation matches its documented
behavior, plus regression tests for password / Password / PASSWORD and a braced value.