-
Notifications
You must be signed in to change notification settings - Fork 314
Add CodeQL suppression for DefaultAzureCredential use in Production #3542
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
Conversation
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.
Pull Request Overview
This PR adds a CodeQL suppression comment to address a security analysis warning about DefaultAzureCredential
instantiation in production code. The change restructures the comment placement to comply with CodeQL's requirement that suppression comments appear on the same line as the flagged code.
- Moved CodeQL suppression comment to the same line as the
DefaultAzureCredential
instantiation - Added explanatory documentation about CodeQL suppression placement requirements
- Split the credential instantiation and return statement for better code organization
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ActiveDirectoryAuthenticationProvider.cs:599
- The variable name 'cred' is too abbreviated and unclear. Consider using a more descriptive name like 'credential' or 'azureCredential' to improve code readability.
DefaultAzureCredential cred = new(defaultAzureCredentialOptions); // CodeQL [SM05137] See above for justification.
- Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code.
1449402
to
f01bd3b
Compare
- Adding catch for macOS socket error to log and ignore.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3542 +/- ##
==========================================
+ Coverage 60.32% 64.58% +4.25%
==========================================
Files 270 270
Lines 62098 62110 +12
==========================================
+ Hits 37463 40113 +2650
+ Misses 24635 21997 -2638
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This will be back-ported to release branches as well? |
Yes, we can backport for the ReceiveTimeout change. The CodeQL part is benign on non-default branches AFAIK. |
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs
Show resolved
Hide resolved
…3542) * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code. * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adding catch for macOS socket error to log and ignore.
…3542) * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code. * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adding catch for macOS socket error to log and ignore.
…3542) (#3550) * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code. * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adding catch for macOS socket error to log and ignore.
… CI failures (#3551) * Add CodeQL suppression for DefaultAzureCredential use in Production (#3542) * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code. * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adding catch for macOS socket error to log and ignore. * Fixed SniTcpHandle -> SNITCPHandle class case difference.
…3542) - Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code. - Adding catch for macOS socket error to log and ignore.
…3542) - Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code. - Adding catch for macOS socket error to log and ignore.
Uh oh!
There was an error while loading. Please reload this page.