dotnet audit & dotnet audit fix for NuGet packages. #11549
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,296 @@ | ||||||
| # dotnet audit & dotnet audit fix | ||||||
|
|
||||||
| - [Jon Douglas](https://github.com/JonDouglas/) | ||||||
| - Start Date (2021-04-01) | ||||||
| - [#8087](https://github.com/NuGet/Home/issues/8087) | ||||||
|
|
||||||
| ## Summary | ||||||
|
|
||||||
| <!-- One-paragraph description of the proposal. --> | ||||||
| `dotnet audit` & `dotnet audit fix` helps you find, fix, and monitor known security vulnerabilities, deprecated packages, and outdated versions in your .NET projects & solutions. | ||||||
|
|
||||||
| ## Motivation | ||||||
|
There was a problem hiding this comment. In my time in the NuGet team, there have been a few times when we've been looking at some old feature and trying to understand WHY it was done a particular way. So, in the interest of helping people in the future understand, can you please add something (anywhere in the document, not necessarily here) about why we're not just adding a |
||||||
|
|
||||||
| <!-- Why are we doing this? What pain points does this solve? What is the expected outcome? --> | ||||||
| .NET developers manage their dependencies on a daily basis. However, it's become more and more difficult to know exactly what dependencies in a software supply chain might be vulnerable, deprecated, or even outdated. All of these scenarios require a developers direct attention to resolve. Even knowing about a dependency that falls into one of these categories, there is then the question about what to do with said dependency such as: | ||||||
|
|
||||||
| - Removing it | ||||||
| - Replacing it | ||||||
| - Updating it | ||||||
| - Ignoring it | ||||||
|
|
||||||
| The goal of this proposal is to provide developers a single command to audit their dependencies, resolve them, and alert the user to manually take action towards a resolution if the tooling cannot. | ||||||
|
|
||||||
| ## Explanation | ||||||
|
|
||||||
| ### Functional explanation | ||||||
|
|
||||||
| <!-- Explain the proposal as if it were already implemented and you're teaching it to another person. --> | ||||||
| <!-- Introduce new concepts, functional designs with real life examples, and low-fidelity mockups or pseudocode to show how this proposal would look. --> | ||||||
|
|
||||||
| #### dotnet audit | ||||||
|
|
||||||
| The `dotnet audit` command will scan your project and solution's dependencies and look for known vulnerabilities, deprecations, and outdated packages. If any vulnerabilities, deprecations, or outdated packages are found, a remediation will be calculated to then `fix` which will be applied to the resulting package graph. | ||||||
|
|
||||||
| Example: | ||||||
|
|
||||||
|  | ||||||
|
There was a problem hiding this comment. personal comment, I don't find the screenshot + text below useful. It just makes the document longer for me. |
||||||
|
|
||||||
| ``` | ||||||
| $ dotnet audit | ||||||
|
|
||||||
| Fetching package metadata from: 'https://api.nuget.org/v3/index.json' | ||||||
|
There was a problem hiding this comment. I think the command should consider There was a problem hiding this comment. Will other feeds such as There was a problem hiding this comment. I do not believe they support these features. The MVP is designed around the central registry. There was a problem hiding this comment. I don't think the extra comment about the packages helps tbh. |
||||||
| Loaded 23 security advisories from 'https://api.nuget.org/v3/index.json' | ||||||
|
There was a problem hiding this comment. What does 23 advisories mean in this case? |
||||||
| Scanning ContosoUniversity.sln (36 NuGet dependencies) | ||||||
|
|
||||||
| error: Vulnerable packages found! | ||||||
| [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 | ||||||
|
|
||||||
| 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. | ||||||
|
There was a problem hiding this comment. 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, There was a problem hiding this comment. 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 There was a problem hiding this comment. 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 | ||||||
|
|
||||||
| Found 1 top-level Legacy deprecated package(s) in 36 scanned packages. | ||||||
|
|
||||||
| 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 | ||||||
|
|
||||||
| Found 2 top-level outdated package(s) in 36 scanned packages. | ||||||
|
There was a problem hiding this comment. 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. There was a problem hiding this comment. 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. |
||||||
|
|
||||||
| Run 'dotnet audit fix' to fix them. | ||||||
|
There was a problem hiding this comment. Say there are 10 packages with advisories. Does fix update them one by one? (this can be super slow, something we don't do in VS). What if the updates fial to bring us to a successful restore state? I think the answer is we our best, and if we fail, say conflicts xyz and add details about potential actions the customer can take. Note that because of the graph flattening, failures to upgrade are more common compared to something like NPM. |
||||||
| ``` | ||||||
|
|
||||||
| ##### Endpoints | ||||||
|
|
||||||
| NuGet will use existing endpoints to optimize the speed of audit results. | ||||||
|
|
||||||
| - [Deprecation](https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#package-deprecation) | ||||||
|
There was a problem hiding this comment. Does audit check nuget.org if nuget.org is not part of the list? Probably should call it out. |
||||||
| - [Vulnerability](https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#vulnerabilities) | ||||||
| - Outdated - No existing endpoint, will need to call a source. | ||||||
|
|
||||||
| ##### dotnet audit --json | ||||||
|
There was a problem hiding this comment. If we're doing this, should it match the dotnet list package design? There was a problem hiding this comment. Yes, given that design is still open and being iterated on, whatever comes from it should be reflected here for consistency and used. |
||||||
|
|
||||||
| To get a detailed audit report in a JSON format. | ||||||
|
|
||||||
| ``` | ||||||
| { | ||||||
| "audit": { | ||||||
| "vulnerabilities": { | ||||||
| "low": "0", | ||||||
| "moderate": "2", | ||||||
| "high": "0", | ||||||
| "critical": "0", | ||||||
|
There was a problem hiding this comment. Should we remove these fields? From my understand you can get the same information by inspecting the There was a problem hiding this comment. These are to aggregate the number from the audit. If there's another way to do that, we can. Wanted to give a summary of json at the top so it's clear where attention should be spent and not looking at each node. |
||||||
| "dependencies": [ | ||||||
| { | ||||||
| "name": "UmbracoForms", | ||||||
| "requestedVersion": "8.4.1", | ||||||
| "resolvedVersion": "8.4.1", | ||||||
| "vulnerabilitySeverity": "Moderate", | ||||||
|
There was a problem hiding this comment. Line 109 uses property name
Suggested change
|
||||||
| "advisoryUrl": "https://github.com/advisories/GHSA-8m73-w2r2-6xxj", | ||||||
| "transitiveDependencies": [ | ||||||
|
There was a problem hiding this comment. What would this look like if a transitive dependency has a vulnerability but all top-level dependencies are OK? |
||||||
| { | ||||||
| "name": "Microsoft.Data.OData", | ||||||
| "resolvedVersion": "5.2.0", | ||||||
| "severity": "Moderate", | ||||||
| "advisoryUrl": "https://github.com/advisories/GHSA-mv2r-q4g5-j8q5" | ||||||
| } | ||||||
| ] | ||||||
| } | ||||||
| ] | ||||||
| }, | ||||||
| "deprecations": { | ||||||
| "legacy": "1", | ||||||
| "critical-bugs": "0", | ||||||
| "other": "0", | ||||||
|
Comment on lines
+117
to
+119
There was a problem hiding this comment. Should we remove these fields? From my understanding, you can get the same information by inspecting the There was a problem hiding this comment. I don't think so. We want to use these as high level counts for the known reason category per dependency. Is there a way we can associate a number with the reason field? |
||||||
| "dependencies": [ | ||||||
| { | ||||||
| "name": "anurse.testing.TestDeprecatedPackage", | ||||||
| "requestedVersion": "1.0.0", | ||||||
| "resolvedVersion": "1.0.0", | ||||||
| "reason": "Legacy", | ||||||
| "alternativeDependencies": [ | ||||||
| { | ||||||
| "name": "Microsoft.AspNetCore.MVC", | ||||||
| "version": "0.0.0" | ||||||
| } | ||||||
| ] | ||||||
| } | ||||||
| ] | ||||||
| }, | ||||||
| "outdated": { | ||||||
| "packages": "2", | ||||||
| "dependencies": [ | ||||||
| { | ||||||
| "name": "anurse.testing.TestDeprecatedPackage", | ||||||
| "resolvedVersion": "1.0.0", | ||||||
| "latestVersion": "2.0.0" | ||||||
| }, | ||||||
| { | ||||||
| "name": "UmbracoForms", | ||||||
| "resolvedVersion": "8.4.1", | ||||||
| "latestVersion": "8.7.1" | ||||||
| } | ||||||
| ] | ||||||
| }, | ||||||
| "scannedDependencies": "36" | ||||||
| } | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| ##### dotnet audit Exit Codes | ||||||
|
|
||||||
| - 0 - The command will exit with a 0 exit code if no vulnerabilities, deprecations, or outdated packages were found. | ||||||
| - 1 - The command will exit with a 1 exit code if a vulnerability, deprecation, or outdated package was found. | ||||||
| - 2 - The command will exit with a 2 exit code if it unexpectedly failed. | ||||||
| - 3 - The command will exit with a 3 exit code if an unsupported project is detected. | ||||||
|
There was a problem hiding this comment. What's the value of splitting exit codes 2 and 3? It may be simpler to have a single exit code for "NuGet was unable to determine if there vulnerabilities, deprecations, or outdated packages. This may due to unexpected failure or unsupported projects" There was a problem hiding this comment. Personally I think unsupported vs. failure are different scenarios. Why have someone guess? |
||||||
|
|
||||||
| #### dotnet audit fix | ||||||
|
There was a problem hiding this comment. I'm sure we have more duplicates of the same request, but Unless we're planning on implementing There was a problem hiding this comment. This is quite a hard challenge indeed. I think we can do our best job designing this feature for the majority of packages that aren't tied directly to a .NET version in NuGet. Even then, there is value for when vulnerabilities or deprecations are found. i.e. https://www.nuget.org/packages/System.DirectoryServices.Protocols/ may have vulnerabilities in NET5 & prompt a user to migrate to NET6 or higher. I don't think this tool will be the panacea of modernization, but it can help bring awareness to the benefit of doing so or even point users to the respective modernization tools if needed. If we do the work in the issue that allows for such functionality, we can port it over to this tool / try our best to detect it from the package lists to be smarter in these cases. I'll add similar notes in the proposal. |
||||||
|
|
||||||
| The `dotnet audit fix` command will provide a remediation that is calculated with an implicit `dotnet audit` to then apply directly to a resulting package graph. It can add packages, remove packages, and update packages depending on the problem it's attempting to resolve. It does not take into consideration downgrading to a compatible version if a higher one has already been specified. | ||||||
|
There was a problem hiding this comment. I assume it fails if there is no alternative, right? There was a problem hiding this comment. If there's no alternative found, the tool would likely throw a manual review for the user to remove the dependency or ignore it in the future. |
||||||
|
|
||||||
|  | ||||||
|
|
||||||
| ``` | ||||||
| $ dotnet audit fix | ||||||
|
|
||||||
| Fixing vulnerable packages in ContosoUniversity.sln | ||||||
| Upgrading UmbracoForms 8.4.1 to 8.7.1 | ||||||
|
|
||||||
| Fixing deprecated packages in ContosoUniversity.sln | ||||||
| Replacing anurse.testing.TestDeprecatedPackage 1.0.0 with Microsoft.AspNetCore.Mvc 2.2.0 | ||||||
|
|
||||||
| Fixing outdated packages in ContosoUniversity.sln | ||||||
| Packages are currently up-to-date. | ||||||
|
|
||||||
| Fixed 2 packages in 36 scanned packages. | ||||||
| ``` | ||||||
|
|
||||||
| ##### dotnet audit fix --dry-run | ||||||
|
|
||||||
| Does a dry run to give an idea of what `audit fix` will do. Provides output, but does not commit the fix. | ||||||
|
|
||||||
| ##### dotnet audit fix --json | ||||||
|
|
||||||
| ```json | ||||||
| "added": [ | ||||||
| { | ||||||
| "name": "Microsoft.AspNetCore.MVC", | ||||||
| "version": "2.2.0" | ||||||
| } | ||||||
| ], | ||||||
| "removed": [ | ||||||
| { | ||||||
| "name": "anurse.testing.TestDeprecatedPackage", | ||||||
| "version": "1.0.0" | ||||||
| } | ||||||
| ], | ||||||
| "updated": [ | ||||||
| { | ||||||
| "name": "UmbracoForms", | ||||||
| "version": "8.7.1", | ||||||
| "previousVersion": "8.4.1" | ||||||
| } | ||||||
| ], | ||||||
| "failures": [], | ||||||
| "warnings": [] | ||||||
| ``` | ||||||
|
|
||||||
| ##### dotnet audit fix Exit Codes | ||||||
|
|
||||||
| - 0 - The command will exit with a 0 exit code if no vulnerabilities, deprecations, or outdated packages were found *or* remediation was able to fix all issues. | ||||||
| - 1 - The command will exit with a 1 exit code if a vulnerability, deprecation, or outdated package was found *and* remediation is not able to fix all issues. | ||||||
| - 2 - The command will exit with a 2 exit code if it unexpectedly failed. | ||||||
| - 3 - The command will exit with a 3 exit code if an unsupported project is detected. | ||||||
|
|
||||||
| #### Vulnerabilities | ||||||
|
|
||||||
| When vulnerable packages are detected, an error is thrown by default. | ||||||
|
There was a problem hiding this comment. Could you explain why an error is thrown instead of a warning for vulnerabilities? A reason I ask is because vulnerabilities in the Javascript ecosystem often noise or very low impact. There was a problem hiding this comment. I'll add more rationale in the proposal. I believe with our lower amount of known vulnerabilities, high importance of the types of vulnerabilities issued for .NET, and the nature of shifting left, these should be treated as errors for now. |
||||||
|
|
||||||
| #### Deprecation | ||||||
|
|
||||||
| When deprecated packages are detected, a warning is thrown by default. | ||||||
|
|
||||||
| #### Outdated | ||||||
|
|
||||||
| When outdated packages are detected, a warning is thrown by default. | ||||||
|
|
||||||
| #### CLI Usage | ||||||
|
|
||||||
| ``` | ||||||
| dotnet audit --help | ||||||
| dotnet audit [<PROJECT>|<SOLUTION>|<Directory.Packages.props>] [-v|--verbosity <LEVEL>] [--json] [--interactive] | ||||||
| ``` | ||||||
|
|
||||||
| ``` | ||||||
| dotnet audit fix --help | ||||||
| dotnet audit fix [<PROJECT>|<SOLUTION>|<Directory.Packages.props>] [-v|--verbosity <LEVEL>] [--dry-run] [--json] [--interactive] | ||||||
|
There was a problem hiding this comment. Consider the following scenario.
This may be an implementation detail how There was a problem hiding this comment. The |
||||||
| ``` | ||||||
|
|
||||||
| ### Technical explanation | ||||||
|
|
||||||
| <!-- Explain the proposal in sufficient detail with implementation details, interaction models, and clarification of corner cases. --> | ||||||
|
There was a problem hiding this comment. Would be nice to outline how
There was a problem hiding this comment. I'll add a flowchart concept to the proposal soon. |
||||||
|
|
||||||
| ## Drawbacks | ||||||
|
|
||||||
| <!-- Why should we not do this? --> | ||||||
| There's a number of [dotnet tools](https://github.com/natemcmaster/dotnet-tools) that exist to solve many of these painpoints. Much of the information to put together this experience exists within NuGet.org's Server API & could created as a dotnet tool. | ||||||
|
|
||||||
| There are a number of auditing tools in other ecosystems that are third-party / developed by the community. For other ecosystems, these features are built into the first-party CLI experiences. | ||||||
|
|
||||||
| ## Rationale and alternatives | ||||||
|
|
||||||
| <!-- Why is this the best design compared to other designs? --> | ||||||
| <!-- What other designs have been considered and why weren't they chosen? --> | ||||||
| <!-- What is the impact of not doing this? --> | ||||||
| There already exists a dotnet CLI commands called `dotnet list package` with options such as `--vulnerable, --deprecated, --outdated` which will list any known vulnerabilities, deprecations, and outdated packages in a project or solution. These options currently cannot be combined. Although this provides valuable information to understand the state of your dependencies, there does not exist a tool that allows you to quickly audit a project/solution & provide a way to act further on. | ||||||
|
|
||||||
| When a developer is prompted with information such as a known vulnerability, deprecated package, or outdated version, they should have a clear understanding of how to best proceed with the information provided. In most cases, simply updating the dependency will suffice. | ||||||
|
|
||||||
| Additionally, there already exists many third-party solutions that try to solve this problem with varying degrees of success. Some solutions can help alert about known vulnerabilities and some can help alert about outdated packages. No single tool seems to check for a "healthy" dependency list & that's another reason why we're looking to combine these experiences into a single command. | ||||||
|
|
||||||
| ## Prior Art | ||||||
|
|
||||||
| <!-- What prior art, both good and bad are related to this proposal? --> | ||||||
| <!-- Do other features exist in other ecosystems and what experience have their community had? --> | ||||||
| <!-- What lessons from other communities can we learn from? --> | ||||||
| <!-- Are there any resources that are relevent to this proposal? --> | ||||||
| - [snyk](https://snyk.io/) | ||||||
| - [npm audit](https://docs.npmjs.com/cli/v7/commands/npm-audit) | ||||||
| - [cargo audit](https://github.com/RustSec/cargo-audit) | ||||||
| - [dotnet outdated](https://github.com/dotnet-outdated/dotnet-outdated) | ||||||
| - [dotnet retire](https://github.com/retirenet/dotnet-retire) | ||||||
| - [NuGet Defense](https://github.com/digitalcoyote/NuGetDefense) | ||||||
|
|
||||||
| ## Unresolved Questions | ||||||
|
|
||||||
| <!-- What parts of the proposal do you expect to resolve before this gets accepted? --> | ||||||
| <!-- What parts of the proposal need to be resolved before the proposal is stabilized? --> | ||||||
| <!-- What related issues would you consider out of scope for this proposal but can be addressed in the future? --> | ||||||
| - Should the command be named `audit` or `check`? | ||||||
| - `audit` is the more consistent name for package manager tooling with other ecosystems. | ||||||
| - `check` is the more consistent name with dotnet CLI. | ||||||
|
There was a problem hiding this comment. check is taken in |
||||||
| - Should this command only audit `vulnerabilities`? Or should it audit & be proactive as to `vulnerabilities`, `deprecations`, and `outdated` packages? | ||||||
|
There was a problem hiding this comment. I think vulnerabilities and deprecations should be accounted for. Outdated packages is the one that gives me a pause (at least by default), due to the fact updating packages to the latest is not necessarily always the right solution. There was a problem hiding this comment. Makes sense. I'd like to see what other thoughts are about this 3-in-1 tool and then can iterate further. |
||||||
| - How much information should be present in the --json output? | ||||||
|
|
||||||
| ## Future Possibilities | ||||||
|
There was a problem hiding this comment. Could we make There was a problem hiding this comment. Yeah if pack can resolve at the time of packing we should definitely do that. |
||||||
|
|
||||||
| <!-- What future possibilities can you think of that this proposal would help with? --> | ||||||
| - `dotnet audit` can be run on every `restore` which can throw warnings or errors to the user to take action against a vulnerable, deprecated, or outdated software supply chain. | ||||||
|
There was a problem hiding this comment. Could customer suppress warnings/errors? It could be low impact noise. Also, even package has vulnerability and there is no alternative then customer stuck with it and took other mitigation steps (isolate network from outside world), then keep throwing error could discourage further usage. There was a problem hiding this comment. Yes I think we can figure out something. I am not so sure what arguments that might be right now as this experience will require an internet connection or a local cache. It might be something like |
||||||
| - `dotnet audit` and `dotnet audit fix` output & resolutions can be extended by the .NET ecosystem to build tooling & new experiences around. | ||||||
| - `dotnet audit` and `dotnet audit fix` experiences can be extended into Visual Studio IDEs providing users with more visualizations of potential problems in their dependencies & ways to resolve them with a single click experience. | ||||||
| - `dotnet audit` can be added to CI/CD environments for an extra layer of monitoring such as a GitHub Action template. | ||||||
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.
It may be crazy but finding unused packages would be awesome.
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.
Here's some a community project that does this: https://github.com/spectresystems/snitch
FYI this is a hard problem since a package may be used through reflection. But .NET trimming effort aligns well with this idea.
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.
Visual Studio does this today. I think it have much more context & tooling to do so.
Would be cool for sure! Not sure if feasible until a technical pass.