Skip to content
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

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

moljac
Copy link
Member

@moljac moljac commented May 3, 2024

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

  1. Added NuGetAudit properties
    https://learn.microsoft.com/en-us/nuget/concepts/auditing-packages
  2. TODO: Added SAST tool .NET Security Guard
    Needs investigation.
    Adding NuGet SecurityCodeScan.VS2019causes 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.

@moljac moljac changed the title Manual updates 20240502 security wave 1 - added SAST tool Manual updates 20240502 security wave 1 May 19, 2024
Copy link
Contributor

@jpobst jpobst left a 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">
Copy link
Contributor

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.

Copy link
Contributor

@jpobst jpobst Oct 7, 2024

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.

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=10305045&view=logs&j=784e4eae-0a8d-50ee-7be1-df4337debdeb&t=fbdff2d1-992e-564e-2a8b-113c89c83f2b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants