License and Security concerns with how Tools and Analyzers are packaged

EdLichtman commented 11 months ago

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.

2) 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.

3) 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 commented 10 months ago

@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 commented 10 months ago

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/

NuGet / Home