Fix dotnet detector: don't attribute component to global.json when resolved SDK version differs#1671
Fix dotnet detector: don't attribute component to global.json when resolved SDK version differs#1671
Conversation
…n resolved SDK version differs Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
… to .gitignore Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetComponentDetector.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue in the DotNet detector where SDK components were being incorrectly attributed to global.json when the .NET SDK rolls forward to a different version than declared in the file. The fix ensures that components are only registered against global.json when the resolved SDK version matches the declared version, giving users clearer guidance about where to make changes.
Changes:
- Modified SDK version detection to compare declared vs resolved versions and only attribute to
global.jsonon exact match - Fixed a JsonDocument disposal leak by wrapping in a using statement
- Added comprehensive tests for both matching and mismatched version scenarios
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetComponentDetector.cs | Refactored GetSdkVersionAsync to read global.json version first, compare with resolved version, and only register component when versions match; fixed JsonDocument disposal leak |
| test/Microsoft.ComponentDetection.Detectors.Tests/DotNetComponentDetectorTests.cs | Updated roll-forward test to verify no global.json component is emitted on mismatch; added new test to verify component is registered when versions match |
| .gitignore | Added .nuget/ directory to ignore list |
| discoveredComponents.Where(component => component.Component.Id == "8.0.808 unknown unknown - DotNet").Should().BeEmpty(); | ||
| } | ||
|
|
||
| [TestMethod] |
There was a problem hiding this comment.
Add a test case for when global.json exists without a version specification but dotnet --version succeeds. This scenario is currently not covered by tests. The expected behavior should be that the SDK version is returned but NOT registered against global.json (since global.json didn't specify a version).
| [TestMethod] | |
| [TestMethod] | |
| public async Task TestDotNetDetectorGlobalJsonWithoutVersion_UsesDotNetVersionAndDoesNotRegisterAgainstGlobalJson() | |
| { | |
| // When global.json exists without an SDK version but dotnet --version succeeds, | |
| // the SDK version should be inferred from dotnet --version but NOT registered | |
| // against global.json (since global.json didn't specify a version). | |
| var projectPath = Path.Combine(RootDir, "path", "to", "project"); | |
| var projectAssets = ProjectAssets("projectName", "does-not-exist", projectPath, "net8.0"); | |
| var globalJson = @"{ ""sdk"": { } }"; | |
| this.AddFile(projectPath, null); | |
| this.AddFile(Path.Combine(RootDir, "path", "global.json"), globalJson); | |
| this.SetCommandResult(0, "8.0.808"); | |
| var (scanResult, componentRecorder) = await this.DetectorTestUtility | |
| .WithFile("project.assets.json", projectAssets) | |
| .ExecuteDetectorAsync(); | |
| scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); | |
| var detectedComponents = componentRecorder.GetDetectedComponents(); | |
| detectedComponents.Should().ContainSingle(); | |
| var discoveredComponents = detectedComponents.ToArray(); | |
| discoveredComponents.Where(component => component.Component.Id == "8.0.808 net8.0 unknown - DotNet").Should().ContainSingle(); | |
| discoveredComponents.Where(component => component.Component.Id == "8.0.808 unknown unknown - DotNet").Should().BeEmpty(); | |
| } | |
| [TestMethod] |
| // Only register against global.json when the resolved version matches what global.json declares. | ||
| // If there is a mismatch (e.g. roll-forward selected a newer SDK), the component should not be | ||
| // attributed to global.json because changing that file would not fix the reported version. | ||
| if (!string.IsNullOrWhiteSpace(globalJsonVersion) && | ||
| !sdkVersion.Equals(globalJsonVersion, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| this.Logger.LogInformation( | ||
| "Resolved SDK version {ResolvedVersion} does not match global.json version {GlobalJsonVersion} in {GlobalJsonPath}. Not registering component against global.json.", | ||
| resolvedVersion, | ||
| globalJsonVersion, | ||
| globalJsonPath); | ||
| return sdkVersion; | ||
| } |
There was a problem hiding this comment.
When global.json exists but doesn't specify a version (globalJsonVersion is null/empty), and dotnet --version succeeds (resolvedVersion is not null/empty), the condition at line 300 will be false due to the null check on globalJsonVersion. This causes the code to skip the mismatch check and fall through to line 317, where it will register the resolved version against global.json even though global.json didn't specify any SDK version.
To fix this, the version match check should only allow registration when globalJsonVersion is actually specified. Consider adding an additional condition to ensure globalJsonVersion is not null/empty before registering, or move the early return outside the mismatch condition.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1671 +/- ##
=====================================
Coverage 90.8% 90.8%
=====================================
Files 451 451
Lines 40144 40178 +34
Branches 2443 2447 +4
=====================================
+ Hits 36456 36489 +33
Misses 3188 3188
- Partials 500 501 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When
dotnet --versionresolves a different SDK version than what's declared inglobal.json(e.g., via roll-forward), the detector was still registering a component againstglobal.jsonusing the resolved version. This gives users a false signal that they need to changeglobal.json, when the real fix is updating their build environment.Changes
DotNetComponentDetector.cs—GetSdkVersionAsync: Readglobal.json's declared version first, then compare againstdotnet --versionoutput. Only register againstglobal.jsonwhen the versions match. On mismatch, log an informational message and return the resolved version without attributing it to the file.dotnet --versionfailure still falls back toglobal.json(existing behavior preserved). Also fixes aJsonDocumentdisposal leak.DotNetComponentDetectorTests.cs: Updated the roll-forward test (TestDotNetDetectorGlobalJsonRollForward) to assert noglobal.jsoncomponent is emitted on version mismatch, and addedTestDotNetDetectorGlobalJsonMatchingVersion_RegistersAgainstGlobalJsonto confirm attribution is preserved when versions match.Before (global.json declares
8.0.100, dotnet resolves8.0.808):After:
New Detector Checklist
IDefaultOffComponentDetectorWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
4m6vsblobprodcus384.vsblob.vsassets.io/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/86123D718A0A7D5E3D8DDB87D50A845A/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/D93C69A3F90DACDB68555AD6306FA9CB/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)5rqvsblobprodcus385.vsblob.vsassets.io/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/D93C69A3F90DACDB68555AD6306FA9CB/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)azuresdkartifacts.blob.core.windows.net/opt/hostedtoolcache/CodeQL/2.24.0/x64/codeql/csharp/tools/linux64/Semmle.Autobuild.CSharp /opt/hostedtoolcache/CodeQL/2.24.0/x64/codeql/csharp/tools/linux64/Semmle.Autobuild.CSharp(dns block)c78vsblobprodcus322.vsblob.vsassets.io/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/2D3586D8C912F6D195254B43F0F0042B/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/D93C69A3F90DACDB68555AD6306FA9CB/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/C8FA87C461E8D947D5E484A1822A0126/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)ckzvsblobprodcus347.vsblob.vsassets.io/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/D93C69A3F90DACDB68555AD6306FA9CB/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)k4kvsblobprodcus344.vsblob.vsassets.io/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/2D3586D8C912F6D195254B43F0F0042B/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/86123D718A0A7D5E3D8DDB87D50A845A/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/D93C69A3F90DACDB68555AD6306FA9CB/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)pdfvsblobprodcus380.vsblob.vsassets.io/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/2D3586D8C912F6D195254B43F0F0042B/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/D93C69A3F90DACDB68555AD6306FA9CB/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/C8FA87C461E8D947D5E484A1822A0126/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)u6ovsblobprodcus377.vsblob.vsassets.io/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/86123D718A0A7D5E3D8DDB87D50A845A/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)ytvvsblobprodcus310.vsblob.vsassets.io/home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/A57A5A89CDBA36A2878C8A42DA58182C/missingpackages_workingdir --packages /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/missingpackages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal --configfile /home/REDACTED/work/component-detection/.codeql-scratch/dbs/csharp/working/nugetconfig/nuget.config --force(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.