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

dotnet audit & dotnet audit fix for NuGet packages. #11549

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
Prev Previous commit
Next Next commit
Formatting... 🤦‍♂️🤦‍♂️🤦‍♂️🤦‍♂️🤦‍♂️🤦‍♂️
  • Loading branch information
JonDouglas committed Apr 14, 2021
commit ede9ac4e126ab411e4ba4cd9cc660c155954401a
14 changes: 7 additions & 7 deletions proposed/2021/DotNetAudit.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ Scanning ContosoUniversity.sln (36 NuGet dependencies)
error: Vulnernable packages found!
JonDouglas marked this conversation as resolved.
Show resolved Hide resolved
[net5.0]:
Top-level Package Requested Resolved Severity Advisory URL
> UmbracoForms 8.4.1 8.4.1 Moderate https://github.com/advisories/GHSA-8m73-w2r2-6xxj
> UmbracoForms 8.4.1 8.4.1 Moderate https://github.com/advisories/GHSA-8m73-w2r2-6xxj

Transitive Package Resolved Severity Advisory URL
> Microsoft.Data.OData 5.2.0 Moderate https://github.com/advisories/GHSA-mv2r-q4g5-j8q5
Transitive Package Resolved Severity Advisory URL
> Microsoft.Data.OData 5.2.0 Moderate https://github.com/advisories/GHSA-mv2r-q4g5-j8q5

Found 1 top-level Moderate severity vulnerability & 1 transtive Moderate severity vulnerability package(s) in 36 scanned packages.

Run 'dotnet audit fix' to fix them.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about a scenario where top-level package has no vulnerability but a transitive one has a warning? AFAIK, we can't update the version of transitive dependency unless we add it as top-level dependency.

@zivkan answered this question in an offline review that, making it a top-level dependency is the design for how to upgrade transitive packages.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we need to document this under a better heading, but we've also documented it publicly: https://docs.microsoft.com/en-us/nuget/concepts/dependency-resolution#cousin-dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope to answer this further with a flowchart or similar. Thanks for bringing it up.


warning: Deprecated packages found!
[net5.0]:
Top-level Package Requested Resolved Reason(s) Alternative
> anurse.testing.TestDeprecatedPackage 1.0.0 1.0.0 Legacy Microsoft.AspNetCore.Mvc > 0.0.0
Top-level Package Requested Resolved Reason(s) Alternative
> anurse.testing.TestDeprecatedPackage 1.0.0 1.0.0 Legacy Microsoft.AspNetCore.Mvc > 0.0.0

Found 1 top-level Legacy deprecated package(s) in 36 scanned packages.

Expand All @@ -67,8 +67,8 @@ Run 'dotnet audit fix' to fix them.
warning: Outdated packages found!
[net5.0]:
Top-level Package Resolved Latest
> anurse.testing.TestDeprecatedPackage 1.0.0 2.0.0
> UmbracoForms 8.4.1 8.7.1
> anurse.testing.TestDeprecatedPackage 1.0.0 2.0.0
> UmbracoForms 8.4.1 8.7.1

Found 2 top-level outdated package(s) in 36 scanned packages.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this report transitive packages that are outdated? Would users have to hoist outdated transitive dependencies to update them to the latest version? That may be a frustrating experience for customers.

Perhaps we should only report outdated top-level dependencies. If so, consider adding a note on that limitation somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It currently follows the outdated experience which doesn't include transitives. I'm also not so sure what value seeing outdated transitives would have unless one wanted to promote those explicitly. Will have to circle back here.


Expand Down