-
Notifications
You must be signed in to change notification settings - Fork 50
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
Manual updates 20240502 security wave 1 #883
base: main
Are you sure you want to change the base?
Conversation
…yCodeScan.VS2019"
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.
If these NuGet Auditing warnings are important, we need to make them errors rather than warnings. Our build currently has over 15,000 warnings, we will never see any new warnings that show up.
https://learn.microsoft.com/en-us/nuget/concepts/auditing-packages#warning-codes
Note that this means our build can break at any time when a new advisory is published. We just need to be aware that sometimes it will break when we haven't pushed any changes to our repository.
@@ -188,4 +193,11 @@ | |||
</ItemGroup> | |||
} | |||
|
|||
<ItemGroup> | |||
<PackageReference Include="SecurityCodeScan.VS2019" Version="5.6.7"> |
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.
From Discord:
It is for Nuget Auditing only, though I tried to add SAST nuget but it causes some issues I need to check.
If this is true, then let's not add this package.
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.
Also note that our pipeline(s) already run the CodeQL static code analysis tool recommended and required by Microsoft security. We should likely rely on their expertise rather than trying to come up with our own solution here.
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.
If this is true, then let's not add this package.
I pushed to see whether the same problems can be reproduced on CI, but CI built OK. Now I know that updates borked my workloads locally. I had to nuke dotnet
installations completely and after clean installation and workloads everything was OK.
Few additional thoughts:
Reporting and analysis is also easier if done locally (on premise). Before I started writing this comment I found out that CodeQL has some issues for our builds. |
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.
If these NuGet Auditing warnings are important, we need to make them errors rather than warnings. Our build currently has over 15,000 warnings, we will never see any new warnings that show up.
This does not seem to be addressed.
Also note that our pipeline(s) already run the CodeQL static code analysis tool recommended and required by Microsoft security. We should likely rely on their expertise rather than trying to come up with our own solution here.
I'm still sticking with this, and I believe we should do what Microsoft security requires we do rather than try to come up with our own security policies. I especially have concerns as this package hasn't been updated in over 2 years and it feels like a security product should be constantly maintained as security is a fast moving field.
If you want this, you'll need to get @jonpryor to approve it.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Does this change any of the generated binding API's?
No.
Describe your contribution
C&AI Security Wave 1
This is 1st part of set of security improvements for AX repo
https://learn.microsoft.com/en-us/nuget/concepts/auditing-packages
Needs investigation.
Adding NuGet
SecurityCodeScan.VS2019
causes build issues (see comments)https://owasp.org/www-community/Source_Code_Analysis_Tools
EDIT:
Builds with
net9.0-rc1
revealed few security warnings#996
.NET is designed with security in mind, but security issues can happen especially through transitive dependencies which is scanned by NuGetAudit, while SecurityCodeScan.VS2019 is a static analysis tool that can help identify security vulnerabilities in the code used in this repo. Admittedly this repo does not use much security critical .NET BCL APIs, but this NuGet will act preventive if such API calls might be added.