tinglesoftware / dependabot-azure-devops

Tools for updating dependencies in Azure DevOps repositories using https://dependabot.com
MIT License
196 stars 65 forks source link

vNext finding incorrect Nuget Transitive Dependencies #1247

Closed james-asebp closed 2 weeks ago

james-asebp commented 3 months ago

Running dependabot extension in DevOps with the new useUpdateScriptvNext flag set. We are seeing in the pipeline logs that it is resolving dependencies with versions too low.

For example we use Microsoft.ApplicationInsights.AspNetCore v2.22.0 in a net6.0 targeted project. One of the deep down Transitive dependencies under .net 6.0 for this package is System.Drawing.Common >= 6.0.0.

During the dependency analysis phase the log outputs this as part of the massive json blob

{
  "Name": "System.Drawing.Common",
  "Version": "4.7.0",
  "Type": "Unknown",
  "EvaluationResult": null,
  "TargetFrameworks": [
    "net6.0"
  ],
  "IsDevDependency": false,
  "IsDirect": false,
  "IsTransitive": true,
  "IsOverride": false,
  "IsUpdate": false,
  "InfoUrl": null
},

As you can see the dependency version found is 4.7.0, which is a Vulnerable verion of this package. Thus the remaining part of the process takes over and happily adds the v4.7.2 version of the package System.Drawing.Common to the csproj file as a packageReference. The problem arises because this is a incompatible version to the version required by the deep dependency of Microsoft.ApplicationInsights.AspNetCore and subsequently fails our PR build.

Categorization

Expected behavior The expectation that the transitive dependency checking is correctly "walking" the dependency tree to resolve the correct package versions.

Screenshots image image image image image

Extension (please complete the following information):

Additional context We are trying out the vNext script to work around the recent nuget auth issues introduced with the removal of the 1.24 docker image

rhyskoedijk commented 3 months ago

This might be an issue with dependabot-core itself:

The post-1.24 NuGet auth fix has been backported to the original update script too since 1.30.2, so you might be able to try again with useUpdateScriptvNext: false and see if you get a different result.

If you don't get this issue with vNext disabled, I can dig deeper in to this and see if it is a specific issue with the vNext script.

If you do get this issue with vNext disabled, then I'm not sure if there is anything practical that can be done in the scope of this project to fix it (assuming it is an issue with the NuGetUpdater.Core native tool). In that case, I would definitely go give that issue an upvote or share your scenario there so it gets more attention to the team.

james-asebp commented 3 months ago

@rhyskoedijk,

I ran our pipeline again with useUpdateScriptvNext set to false this morning. The dependency System.Drawing.Common does appear in the giant json blob again, twice, once as v6.0.0 and again as v4.7.0. But later in the log it does not appear in the dependency summary at all and therefore the update process does not attempt to update this dependency.

This makes me think either the grouping logic or the transient dependency vulnerability checking in vNext is triggering the issue. I understand that vNext might be using more of the dependabot-core code but I am having difficulty following the process in source because my ruby experience is null.

rhyskoedijk commented 3 months ago

This makes me think either the grouping logic or the transient dependency vulnerability checking in vNext is triggering the issue.

I will have a look in to this and see what I can find.

rhyskoedijk commented 2 months ago

I have not been able to reproduce this but have noticed a few new fixes to dependabot-core around transitive dependencies and ensuring dependencies are valid for target framework:

This issue might be resolved when 0.272.0 is released and merged here.

james-asebp commented 2 weeks ago

Closing this issue as V2 seems to be working and instead it is logic in the base dependabot code that is instead blocking us.

Opened #10791 over there.