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

Prevent NuGet audit warnings from failing the CI builds (Lombiq Technologies: OCORE-184) #16317

Merged
merged 12 commits into from
Jun 17, 2024

Conversation

Piedone
Copy link
Member

@Piedone Piedone commented Jun 13, 2024

NuGet security audit warnings should still show up in VS, and also in CI builds, but as warnings.

Formerly, due to CI builds treating warnings as errors (to prevent introducing new build warnings) these also failed CI builds. This is a problem because security issues being discovered in NuGet packages we use can happen any time, and thus all our CI builds could break anytime, without us changing the code

These warnings should still be addressed ASAP, of course, but they shouldn't block all other development. After #16284 this will happen automatically.

I tried to do this in a way that we still see the warning even in CI builds but couldn't succeed.

FYI what does NOT prevent these warnings from being treated as errors:

  • <WarningsNotAsErrors>NU1901;NU1902;NU1903;NU1904</WarningsNotAsErrors> in any or all of the files below (though it should work from a Directory.Build.props, see Solution level NuGetAuditLevel settings NuGet/Home#13022).
    image
    I also tried changing these two to include $(WarningsNotAsErrors); but that didn't help:
    image
    This only works when set in a csproj file, the one of the project that has an offending dependency. This obviously doesn't scale.
  • Adding the following switches to dotnet build: /p:WarningsNotAsErrors=NU1902, -warnNotAsError:NU1902, /warnaserror-:NU1902 or -warnaserror-:NU1902 (old switch, is not accepted at all), -warnAsMessage:NU1902, /property:WarningsNotAsErrors=NU1902.

@Piedone Piedone marked this pull request as ready for review June 13, 2024 18:31
@Piedone Piedone marked this pull request as draft June 13, 2024 18:32
@Piedone Piedone marked this pull request as ready for review June 13, 2024 18:37
@Piedone Piedone requested a review from hishamco June 16, 2024 22:15
Comment on lines 28 to 31
# We disable NuGet audit warnings, see https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu1901-nu1904.
# Security issues being discovered in NuGet packages we use can happen any time, and thus all our CI builds that
# treat warnings as errors could break anytime, without us changing the code. This prevents that. Treaing them as
# warnings and other better approaches don't work, see https://github.com/OrchardCMS/OrchardCore/pull/16317.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# We disable NuGet audit warnings, see https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu1901-nu1904.
# Security issues being discovered in NuGet packages we use can happen any time, and thus all our CI builds that
# treat warnings as errors could break anytime, without us changing the code. This prevents that. Treaing them as
# warnings and other better approaches don't work, see https://github.com/OrchardCMS/OrchardCore/pull/16317.
# We disable NuGet audit warnings, see https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu1901-nu1904.
# Security issues being discovered in NuGet packages we use can happen at any time, and thus all our CI builds that
# treat warnings as errors that could break anytime, without us changing the code. This prevents that. Treating them as
# warnings and other better approaches don't work, see https://github.com/OrchardCMS/OrchardCore/pull/16317.

@Piedone Piedone requested a review from hishamco June 17, 2024 15:16
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