Repository navigation
Suppress NU1902/NU1903 so a new advisory doesn't break clean builds - #6
Open
johnnyxmas wants to merge 2 commits into
Open
johnnyxmas wants to merge 2 commits into
johnnyxmas wants to merge 2 commits into
Conversation
TreatWarningsAsErrors is enabled project-wide, which makes NuGet's restore-time security-advisory audit fatal for any dependency with a published advisory. As of this commit that's MailKit 4.15.1 (GHSA-9j88-vvj5-vhgr) and Microsoft.OpenApi 2.4.1 (GHSA-v5pm-xwqc-g5wc), but the underlying issue isn't those two packages specifically -- it's that a clean `dotnet restore` with no NuGet package cache breaks entirely the moment any advisory is published for anything in the dependency graph, independent of anything in this repo's own code. Ran into this doing a from-scratch Docker build (no local NuGet cache to mask it), which is presumably why it hadn't surfaced before. Suppressed via the existing NoWarn convention rather than disabling the audit outright, so advisories stay visible in build output.
The explanatory comment in the previous commit contained a literal '--' sequence, which XML disallows inside comments. That made src/Directory.Build.props unparseable, so ManagePackageVersionsCentrally was never applied and every PackageReference fell back to its lowest available version, failing restore with NU1015 and spurious NU1107 version conflicts.
Author
|
Correction: the first push of this branch had a bug of my own making. The explanatory comment I added alongside the suppression contained a literal Fixed in 1870d07 (reworded to avoid the |
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.
Summary
TreatWarningsAsErrors is enabled project-wide in src/Directory.Build.props, which makes NuGet's restore-time security-advisory audit fatal for any dependency with a published advisory. As of this writing that's MailKit 4.15.1 (GHSA-9j88-vvj5-vhgr) and Microsoft.OpenApi 2.4.1 (GHSA-v5pm-xwqc-g5wc), but the underlying issue isn't those two packages specifically -- it's that a clean dotnet restore with no NuGet package cache breaks entirely the moment any advisory is published for anything in the dependency graph, independent of anything in this repo's own code.
I ran into this doing a from-scratch Docker build (docker build . with no local NuGet cache to mask it), which is presumably why it hasn't come up before -- a machine with these packages already restored from before the advisories existed won't re-trigger the audit on incremental builds.
Change
Suppressed via the existing NoWarn convention (same pattern already used for CS1591/CA1416 two lines up) rather than disabling the audit outright, so advisories stay visible in build output for awareness -- they just don't hard-fail the build.
Test plan