From 0a8d2b5cb5b56ab87f830d38a89ba243ec701b9d Mon Sep 17 00:00:00 2001 From: Greg Villicana Date: Fri, 15 Nov 2024 18:56:23 -0800 Subject: [PATCH 1/2] Fix detector component filepath custom overrides --- .../DefaultGraphTranslationService.cs | 8 ++- .../DefaultGraphTranslationServiceTests.cs | 68 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs index cb00860a4..c6b3c1208 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs @@ -103,7 +103,11 @@ private IEnumerable GatherSetOfDetectedComponentsUnmerged(IEn // clone custom locations and make them relative to root. var declaredRawFilePaths = component.FilePaths ?? []; var componentCustomLocations = JsonConvert.DeserializeObject>(JsonConvert.SerializeObject(declaredRawFilePaths)); - component.FilePaths?.Clear(); + + if (updateLocations) + { + component.FilePaths?.Clear(); + } // Information about each component is relative to all of the graphs it is present in, so we take all graphs containing a given component and apply the graph data. foreach (var graphKvp in dependencyGraphsByLocation.Where(x => x.Value.Contains(component.Component.Id))) @@ -269,7 +273,7 @@ private HashSet MakeFilePathsRelative(ILogger logger, DirectoryInfo root try { var relativePath = rootUri.MakeRelativeUri(new Uri(path)).ToString(); - if (!relativePath.StartsWith('/')) + if (!relativePath.StartsWith('/') && !relativePath.Contains(':', StringComparison.CurrentCultureIgnoreCase)) { relativePath = "/" + relativePath; } diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DefaultGraphTranslationServiceTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DefaultGraphTranslationServiceTests.cs index b3ed0df1a..df16f4a80 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DefaultGraphTranslationServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DefaultGraphTranslationServiceTests.cs @@ -97,4 +97,72 @@ public void GenerateScanResultFromResult_WithCustomLocations() actualNpmComponent.Should().BeEquivalentTo(expectedNpmComponent); actualNugetComponent.Should().BeEquivalentTo(expectedNugetComponent); } + + [TestMethod] + public void GenerateScanResultFromResult_WithCustomLocations_WithExperimentsDryRun() + { + var detectedFilePath = "/some/file/path"; + var npmCustomPath = "/custom/path.js"; + var npmCustomPath2 = "D:/dummy/engtools/packages.lock.json"; + var nugetCustomPath = "/custom/path2.csproj"; + var relatedFilePath = "/generic/relevant/path"; + + var singleFileComponentRecorder = this.componentRecorder.CreateSingleFileComponentRecorder(Path.Join(this.sourceDirectory.FullName, detectedFilePath)); + var processingResult = new DetectorProcessingResult + { + ResultCode = ProcessingResultCode.Success, + ContainersDetailsMap = new Dictionary + { + { + this.sampleContainerDetails.Id, this.sampleContainerDetails + }, + }, + ComponentRecorders = [(this.componentDetectorMock.Object, this.componentRecorder)], + }; + + var expectedNpmComponent = new NpmComponent("npm-component", "1.2.3"); + var expectedNugetComponent = new NuGetComponent("nugetComponent", "4.5.6"); + var detectedNpmComponent = new DetectedComponent(expectedNpmComponent); + var detectedNugetComponent = new DetectedComponent(expectedNugetComponent); + + // Any Related File will be reported for ALL components found in this graph + singleFileComponentRecorder.AddAdditionalRelatedFile(Path.Join(this.sourceDirectory.FullName, relatedFilePath)); + + // Registering components in same manifest with different custom paths + detectedNpmComponent.AddComponentFilePath(Path.Join(this.sourceDirectory.FullName, npmCustomPath)); + detectedNpmComponent.AddComponentFilePath(npmCustomPath2); + detectedNugetComponent.AddComponentFilePath(Path.Join(this.sourceDirectory.FullName, nugetCustomPath)); + + singleFileComponentRecorder.RegisterUsage(detectedNpmComponent, isDevelopmentDependency: false); + singleFileComponentRecorder.RegisterUsage(detectedNugetComponent, isDevelopmentDependency: true); + + var settings = new ScanSettings + { + SourceDirectory = this.sourceDirectory, + }; + + // Experiments tool generates the graph but should not update the locations at all. + var result = this.serviceUnderTest.GenerateScanResultFromProcessingResult(processingResult, settings, updateLocations: false); + result.Should().NotBeNull(); + result.ComponentsFound.Should().HaveCount(2); + result.ResultCode.Should().Be(ProcessingResultCode.Success); + + // next run will record the actual locations from non-experimental detectors + result = this.serviceUnderTest.GenerateScanResultFromProcessingResult(processingResult, settings); + result.Should().NotBeNull(); + result.ComponentsFound.Should().HaveCount(2); + result.ResultCode.Should().Be(ProcessingResultCode.Success); + + var resultNpmComponent = result.ComponentsFound.Single(c => c.Component.Type == ComponentType.Npm); + var resultNugetComponent = result.ComponentsFound.Single(c => c.Component.Type == ComponentType.NuGet); + + resultNpmComponent.LocationsFoundAt.Should().BeEquivalentTo([npmCustomPath, detectedFilePath, relatedFilePath, npmCustomPath2]); + resultNugetComponent.LocationsFoundAt.Should().BeEquivalentTo([nugetCustomPath, detectedFilePath, relatedFilePath]); + + var actualNpmComponent = resultNpmComponent.Component as NpmComponent; + var actualNugetComponent = resultNugetComponent.Component as NuGetComponent; + + actualNpmComponent.Should().BeEquivalentTo(expectedNpmComponent); + actualNugetComponent.Should().BeEquivalentTo(expectedNugetComponent); + } } From 16cf57aea14d7eb30f54c70b1d142fd9647e4dda Mon Sep 17 00:00:00 2001 From: Greg Villicana Date: Sat, 16 Nov 2024 17:51:05 -0800 Subject: [PATCH 2/2] nit: leave symbolic links untouched for now --- .../GraphTranslation/DefaultGraphTranslationService.cs | 2 +- .../Services/DefaultGraphTranslationServiceTests.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs index c6b3c1208..ceed83e84 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs @@ -273,7 +273,7 @@ private HashSet MakeFilePathsRelative(ILogger logger, DirectoryInfo root try { var relativePath = rootUri.MakeRelativeUri(new Uri(path)).ToString(); - if (!relativePath.StartsWith('/') && !relativePath.Contains(':', StringComparison.CurrentCultureIgnoreCase)) + if (!relativePath.StartsWith('/')) { relativePath = "/" + relativePath; } diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DefaultGraphTranslationServiceTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DefaultGraphTranslationServiceTests.cs index df16f4a80..6979806f8 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DefaultGraphTranslationServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DefaultGraphTranslationServiceTests.cs @@ -156,7 +156,8 @@ public void GenerateScanResultFromResult_WithCustomLocations_WithExperimentsDryR var resultNpmComponent = result.ComponentsFound.Single(c => c.Component.Type == ComponentType.Npm); var resultNugetComponent = result.ComponentsFound.Single(c => c.Component.Type == ComponentType.NuGet); - resultNpmComponent.LocationsFoundAt.Should().BeEquivalentTo([npmCustomPath, detectedFilePath, relatedFilePath, npmCustomPath2]); + // for now there is a bug that adds a forward slash to the path for symbolic links. This will be fixed in a future PR if we can parse dependency graph better for those. + resultNpmComponent.LocationsFoundAt.Should().BeEquivalentTo([npmCustomPath, detectedFilePath, relatedFilePath, $"/{npmCustomPath2}"]); resultNugetComponent.LocationsFoundAt.Should().BeEquivalentTo([nugetCustomPath, detectedFilePath, relatedFilePath]); var actualNpmComponent = resultNpmComponent.Component as NpmComponent;