CHORE: Reduce DevSkim false positives in test and pipeline files#524
Open
bewithgaurav wants to merge 1 commit intomainfrom
Open
CHORE: Reduce DevSkim false positives in test and pipeline files#524bewithgaurav wants to merge 1 commit intomainfrom
bewithgaurav wants to merge 1 commit intomainfrom
Conversation
bewithgaurav
force-pushed
the
bewithgaurav/devskim-exclude-tests
branch
from
3422a00 to
ce4ecbf
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the DevSkim GitHub Actions workflow configuration to reduce false positives and speed up scanning by excluding certain paths and rules from analysis.
Changes:
- Configure DevSkim to skip scanning archives.
- Exclude
tests/**,benchmarks/**, andtest_*.pyfrom scanning via ignore globs. - Globally suppress DevSkim rules DS176209 (TODO comments) and DS137138 (localhost usage).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bewithgaurav
force-pushed
the
bewithgaurav/devskim-exclude-tests
branch
from
ce4ecbf to
db5d72b
Compare
📊 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.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 67.8%
mssql_python.row.py: 70.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 74.2%
mssql_python.pybind.connection.connection.cpp: 75.8%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.2%🔗 Quick Links
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
bewithgaurav
force-pushed
the
bewithgaurav/devskim-exclude-tests
branch
5 times, most recently
from
852afee to
2c1f0ee
Compare
- Exclude tests/, **/test_*.py, and benchmarks/ from DevSkim scanning (localhost in test connection strings is not debug code) - Suppress DS176209 (TODO comments) — informational, not a security concern - DS137138 (localhost) left enabled — path exclusions handle test files, keeps coverage for accidental localhost in production code - Fix test_bulkcopy_without_database_parameter: remove USE statement which is not supported on Azure SQL Database, use fully qualified table names with the default database instead
bewithgaurav
force-pushed
the
bewithgaurav/devskim-exclude-tests
branch
from
2c1f0ee to
0794968
Compare
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
This pull request makes improvements to the DevSkim workflow configuration and refines the bulk copy test logic in
test_019_bulkcopy.py. The main focus is on enhancing test reliability and accuracy when handling database parameters and improving DevSkim scan efficiency.DevSkim workflow configuration:
.github/workflows/devskim.ymlto ignore test and benchmark files during security scans, and to suppress a specific non-security rule related to TODO comments. This helps reduce noise in scan results and focuses attention on relevant security issues.Bulk copy test improvements:
test_bulkcopy_without_database_parameterintest_019_bulkcopy.pyto always use the actual current database (as determined after connecting) for bulk copy operations, rather than relying on the original database from the connection string. This ensures the test is robust even if the default database differs from the original one and eliminates unnecessary switching between databases. [1] [2] [3]test_bulkcopy_with_server_synonymsby switching from multi-line f-strings to the preferredcursor.executewith triple-quoted strings, enhancing readability and consistency. [1] [2]