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

License and Security concerns with how Tools and Analyzers are packaged #12941

Open
EdLichtman opened this issue Oct 16, 2023 · 2 comments
Open
Labels
Functionality:Pack Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Style:PackageReference Type:Feature

Comments

@EdLichtman
Copy link

EdLichtman commented Oct 16, 2023

NuGet Product(s) Affected

NuGet.exe, NuGet SDK

Current Behavior

As I understand it right now:

As a package developer, in order to use an assembly in a package, I need to copy the dll of the package assembly into the analyzers folder or the tools folder.

See evidence here -- the cookbook suggests that you package up Newtonsoft.Json in the same folder as the assembly that uses it:
https://github.com/dotnet/roslyn/blob/main/docs/features/source-generators.cookbook.md#use-functionality-from-nuget-packages

Besides the evidence I can also attest to my own anecdotal experience from packaging libraries.

Desired Behavior

As a package developer, I should be able to map the usage of the Dependency assets to my own library.

For example, if I need to use Newtonsoft.Json -> Compile Assets in MyPackage -> Analyzers folder, there should be a way to list that as a dependency, not as a packaged dll.

Just spitballing here, but an idea could be something like

<PackageReference Include="Newtonsoft.Json" PrivateAssets="all"/>
<PackageReferenceAsset Include="Newtonsoft.Json" PackagePath="analyzers/cs" Dependency="compile"/>
<PackageReferenceAsset Include="Newtonsoft.Json" PackagePath="tools/any" Dependency="compile"/>

<PackageReference Include="MyLibraryWithContent"/>
<PackageReferenceAsset Include="MyLibraryWithContent" PackagePath="content" Dependency="content"/>

Bonus benefits

  1. You could also make "PackageReferenceAsset" be available for Project References
  2. You could also make "PackageReferenceAsset" not require a PackageReference. Some sort of rule like, If %(PackageReferenceAsset.Identity) is not in the @(PackageReference) Item, and that %(PackageReferenceAsset.Identity) does not contain PackagePath and Dependency, then it's just listed as a straight-up dependency for the child importing it. There's another issue I'm watching that's meant to solve this problem, and as I was writing my issue I realized this could be an answer to how to solve it.
<PackageReferenceAsset Include="MyLibraryThatIsNotReferencedInPackage"/>

Additional Context

The reason why it's important for me is multiple:

  1. Security concerns
  • By packaging the dll up separately, you have to inspect the package and truly grok the package to understand that the dependency exists. Since it doesn't show up in NuGet as a Dependency, you won't know if there's a Zero Day exploit on the package version as someone who's importing the package.
  • By packaging up a specific version of the dll, you need to update the entire library to keep up to date with any dependencies. Not only that, but you break parity with the idea of "enforce the lowest required dependency version, and let the developer choose the version they want". It means you can have Version 12.0.0 of Newtonsoft.json packaged while the developer only has version 9.0.0 installed.
  • Conversely, and to the first point, by packaging up a specific version of the dll, if a developer uses your library and you don't keep republishing every time a version gets updated, then if a developer requires version 13.0.3 because 12.0.0 has critical security defects, the developer doesn't know not to use your library. Or if they're using the proper tooling to discern security vulnerabilities, they can't use your library due to policy restrictions on vulnerable libraries.
  1. License concerns
  • By packaging up the dll separately, you put the onus on the package developer to ensure they are following guidelines for licensing. For example, if a person publishing a package incorporates a GNU license, they have to abide by the GNU GPL license terms. This is absolutely correct. However, if the library owner fails to abide by this license, and someone imports the library, then regardless of who's at fault, you have quite the headache on your hands.
  1. Generally preferring a better way of developing
  • Adding the whole GeneratePathProperty="true" and the "TargetPathWithTargetPlatformMoniker" thing, as well as packaging up the "None" element can get very verbose and not incredibly intuitive. Especially when you're reading through your own CSProj file
@EdLichtman EdLichtman added Triage:Untriaged Type:DCR Design Change Request labels Oct 16, 2023
@nkolev92 nkolev92 added Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Pipeline:Icebox labels Oct 30, 2023
@EdLichtman
Copy link
Author

@nkolev92 What does putting it in the icebox mean? right now I'm packaging up newtonsoft 12.0.2 into an analyzer, which has security vulnerabilities, and the library that imports the package will be none the wiser to these security vulnerabilities. This seems honestly concerning.

@EdLichtman
Copy link
Author

Here's a working example -- I just wanted to check if the "AssemblyBindings" would override the "compile assets", and it seems to be the case. Notice though how if you run this, there is absolutely nothing suggesting to the library importing this package that there is a dependency on a security vulnerability.

https://github.com/EdLichtman/NUGET_HOME_ISSUES_12941/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:Pack Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Style:PackageReference Type:Feature
Projects
None yet
Development

No branches or pull requests

4 participants