Uh oh!
There was an error while loading. Please reload this page.
FIX: security issue SEC101/037 : SQLLegacyCredentials fix - #369
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.cpp: 66.2%
mssql_python.row.py: 66.2%
mssql_python.helpers.py: 67.5%
mssql_python.pybind.connection.connection.cpp: 73.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.connection.py: 83.5%
mssql_python.cursor.py: 84.3%
mssql_python.__init__.py: 84.9%🔗 Quick Links
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a security issue (SEC101/037: SQLLegacyCredentials) by removing hardcoded credentials from test connection strings. The fix replaces UID and PWD parameters with Trusted_Connection=yes across multiple test files and introduces environment variable usage for server and database names in select tests.
Key changes:
- Removed all hardcoded username/password credentials from test connection strings
- Replaced credential parameters with
Trusted_Connection=yesfor Windows authentication - Added environment variable support (
TEST_SERVER,TEST_DATABASE) in one test function to enable configurable test environments
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_002_types.py | Updated three connection strings in test_invalid_surrogate_handling() to use Trusted_Connection=yes and added environment variable support for server/database names with os.getenv() |
| tests/test_013_sqlwchar_conversions.py | Systematically replaced all hardcoded UID=user;PWD=pass with Trusted_Connection=yes across 21 connection strings in various test methods |
| tests/test_014_ddbc_bindings_coverage.py | Replaced all hardcoded UID=u;PWD=p with Trusted_Connection=yes across 12 connection strings in various test methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…anner PR #562 introduced test code with UID/PWD credentials that trigger the Azure DevOps secret scanner (SEC101/037: SqlLegacyCredentials), blocking the GitHub-to-ADO sync pipeline. Changed all connection strings in test_connect_runtime_error_mapped_to_correct_dbapi_exception to use Trusted_Connection=yes instead of UID/PWD parameters. These are mocked tests (RuntimeError is raised before connection), so Windows Authentication works without affecting test behavior. Follows the pattern from PR #369 which fixed similar scanner issues by replacing SQL Auth with Windows Auth in test connection strings.
Work Item / Issue Reference
Summary
This pull request updates several test files to improve security and consistency in database connection strings. The main change is replacing hardcoded usernames and passwords with
Trusted_Connection=yesin all connection strings, and in some cases, using environment variables for server and database names. This prevents the exposure of credentials and avoids related security warnings.Test connection string improvements:
Replaced all hardcoded
UIDandPWDparameters in connection strings withTrusted_Connection=yesintests/test_002_types.py,tests/test_013_sqlwchar_conversions.py, andtests/test_014_ddbc_bindings_coverage.py, enhancing security and aligning with best practices. [1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20][21][22][23][24][25][26][27][28][29][30]Updated some tests in
tests/test_002_types.pyto useos.getenvforTEST_SERVERandTEST_DATABASE, allowing for configurable test environments and reducing hardcoded values. [1][2][3]These changes make the test suite safer to run in different environments and prevent accidental credential leaks.