Merge branch 'main' of https://github.com/microsoft/sbom-tool into copilot/fix-1093

Author: JoseRenan

src/Microsoft.Sbom.Api/Executors/PackageInfoJsonWriter.cs

@@ -19,11 +19,11 @@ namespace Microsoft.Sbom.Api.Executors;
 /// </summary>
 public class PackageInfoJsonWriter
 {
-    private readonly ManifestGeneratorProvider manifestGeneratorProvider;
+    private readonly IManifestGeneratorProvider manifestGeneratorProvider;
     private readonly ILogger log;
 
     public PackageInfoJsonWriter(
-        ManifestGeneratorProvider manifestGeneratorProvider,
+        IManifestGeneratorProvider manifestGeneratorProvider,
         ILogger log)
     {
         if (manifestGeneratorProvider is null)
@@ -54,7 +54,7 @@ public PackageInfoJsonWriter(
         return (result, errors);
     }
 
-    private async Task GenerateJson(
+    internal async Task GenerateJson(
         IList<ISbomConfig> packagesArraySupportingConfigs,
         SbomPackage packageInfo,
         Channel<JsonDocWithSerializer> result,
@@ -67,14 +67,22 @@ private async Task GenerateJson(
                 var generationResult =
                     manifestGeneratorProvider.Get(sbomConfig.ManifestInfo).GenerateJsonDocument(packageInfo);
 
+                var recordedAnyDependencies = false;
+
                 if (generationResult?.ResultMetadata?.DependOn != null)
                 {
                     foreach (var dependency in generationResult?.ResultMetadata?.DependOn)
                     {
                         sbomConfig.Recorder.RecordPackageId(generationResult?.ResultMetadata?.EntityId, dependency);
+                        recordedAnyDependencies = true;
                     }
                 }
 
+                if (!recordedAnyDependencies)
+                {
+                    sbomConfig.Recorder.RecordPackageId(generationResult?.ResultMetadata?.EntityId, null);
+                }
+
                 await result.Writer.WriteAsync((generationResult?.Document, sbomConfig.JsonSerializer));
             }
         }

src/Microsoft.Sbom.Api/Manifest/IManifestGeneratorProvider.cs

@@ -0,0 +1,17 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Generic;
+using Microsoft.Sbom.Extensions;
+using Microsoft.Sbom.Extensions.Entities;
+
+namespace Microsoft.Sbom.Api.Manifest;
+
+public interface IManifestGeneratorProvider
+{
+    public IManifestGenerator Get(ManifestInfo manifestInfo);
+
+    public IEnumerable<ManifestInfo> GetSupportedManifestInfos();
+
+    public ManifestGeneratorProvider Init();
+}

src/Microsoft.Sbom.Api/Manifest/ManifestGeneratorProvider.cs

@@ -13,7 +13,7 @@ namespace Microsoft.Sbom.Api.Manifest;
 /// Factory class that returns the correct implementation of the <see cref="IManifestGenerator"/>
 /// at runtime based on the 'ManifestInfo' parameter.
 /// </summary>
-public class ManifestGeneratorProvider
+public class ManifestGeneratorProvider : IManifestGeneratorProvider
 {
     private readonly IEnumerable<IManifestGenerator> manifestGenerators;
     private readonly IDictionary<string, IManifestGenerator> manifestMap = new Dictionary<string, IManifestGenerator>(StringComparer.OrdinalIgnoreCase);

src/Microsoft.Sbom.Api/Providers/FilesProviders/CGScannedExternalDocumentReferenceFileProvider.cs

@@ -51,7 +51,7 @@ public CGScannedExternalDocumentReferenceFileProvider(
 
     public override bool IsSupported(ProviderType providerType)
     {
-        if (providerType == ProviderType.Files)
+        if (providerType == ProviderType.Files && Configuration.ManifestToolAction == ManifestToolActions.Generate)
         {
             Log.Debug($"Using the {nameof(CGScannedExternalDocumentReferenceFileProvider)} provider for the files workflow.");
             return true;

src/Microsoft.Sbom.Api/Workflows/SbomAggregationWorkflow.cs

@@ -127,7 +127,7 @@ private IEnumerable<AggregationSource> GetSbomsToAggregate(string artifactPath,
         var isValidSpdxFormat = spdxFormatDetector.TryGetSbomsWithVersion(manifestDirPath, out var detectedSboms);
         if (!isValidSpdxFormat)
         {
-            logger.Information($"No SBOMs located in {manifestDirPath} of a recognized SPDX format.");
+            logger.Warning($"No SBOMs located in {manifestDirPath} of a recognized SPDX format.");
             return null;
         }
 
@@ -148,6 +148,7 @@ private async Task<bool> ValidateSourceSbomsAsync(IEnumerable<AggregationSource>
                 configuration.IgnoreMissing = new ConfigurationSetting<bool>(source.ArtifactInfo.IgnoreMissingFiles ?? false);
                 configuration.BuildDropPath = new ConfigurationSetting<string>(source.BuildDropPath);
                 configuration.OutputPath = new ConfigurationSetting<string>(fileSystemUtils.JoinPaths(workingDir, $"validation-results-{identifier}.json"));
+                configuration.ManifestInfo = new ConfigurationSetting<IList<ManifestInfo>>(new List<ManifestInfo>() { source.SbomConfig.ManifestInfo });
                 configuration.ManifestDirPath = BuildManifestDirPathForSource(source);
 
                 Console.WriteLine($"Running validation for {source.SbomConfig.ManifestJsonFilePath} with identifier {identifier}. Writing output results to {configuration.OutputPath.Value}.");

src/Microsoft.Sbom.Api/Workflows/SbomGenerationWorkflow.cs

@@ -186,11 +186,8 @@ private async Task<IEnumerable<FileValidationResult>> CallGeneratorsAync(IEnumer
         // Write all the JSON documents from the generationResults to the manifest based on the manifestInfo.
         // When aggregating, only call the packages generator. A helper method might make this more compact,
         // but it would be less readable.
-        if (configuration.ManifestToolAction == ManifestToolActions.Generate)
-        {
-            var fileGeneratorResult = await fileArrayGenerator.GenerateAsync(targetConfigs, elementsSpdxIdList);
-            validErrors.AddRange(fileGeneratorResult.Errors);
-        }
+        var fileGeneratorResult = await fileArrayGenerator.GenerateAsync(targetConfigs, elementsSpdxIdList);
+        validErrors.AddRange(fileGeneratorResult.Errors);
 
         var packageGeneratorResult = await packageArrayGenerator.GenerateAsync(targetConfigs, elementsSpdxIdList);
         validErrors.AddRange(packageGeneratorResult.Errors);
@@ -201,11 +198,8 @@ private async Task<IEnumerable<FileValidationResult>> CallGeneratorsAync(IEnumer
             validErrors.AddRange(externalDocumentReferenceGeneratorResult.Errors);
         }
 
-        if (configuration.ManifestToolAction == ManifestToolActions.Generate)
-        {
-            var relationshipGeneratorResult = await relationshipsArrayGenerator.GenerateAsync(targetConfigs, elementsSpdxIdList);
-            validErrors.AddRange(relationshipGeneratorResult.Errors);
-        }
+        var relationshipGeneratorResult = await relationshipsArrayGenerator.GenerateAsync(targetConfigs, elementsSpdxIdList);
+        validErrors.AddRange(relationshipGeneratorResult.Errors);
 
         return validErrors;
     }

src/Microsoft.Sbom.Common/Config/IConfiguration.cs

@@ -120,8 +120,8 @@ public interface IConfiguration
 
     /// <summary>
     /// Gets or sets a list of <see cref="SbomFile"/> files provided to us from the API.
-    /// We won't traverse the build root path to get a list of files if this is set, and
-    /// use the list provided here instead.
+    /// We will use the list provided here to populate the files section in addition to
+    /// select file providers.
     /// </summary>
     public ConfigurationSetting<IEnumerable<SbomFile>> FilesList { get; set; }
 

src/Microsoft.Sbom.Common/Utils/CommonSPDXUtils.cs

@@ -5,6 +5,7 @@
 using System.Security.Cryptography;
 using System.Text;
 using System.Text.RegularExpressions;
+using Microsoft.Sbom.Contracts;
 
 namespace Microsoft.Sbom.Common.Utils;
 
@@ -23,6 +24,32 @@ public static class CommonSPDXUtils
     /// </summary>
     public static string GenerateSpdxPackageId(string id) => $"{Constants.SPDXRefPackage}-{GetStringHash(id)}";
 
+    /// <summary>
+    /// Returns the SPDX-compliant package ID from an SbomPackage object.
+    /// </summary>
+    public static string GenerateSpdxPackageId(SbomPackage packageInfo)
+    {
+        if (packageInfo is null)
+        {
+            throw new ArgumentNullException(nameof(packageInfo));
+        }
+
+        // Special case to preserve incoming SPDX ID's during aggregation
+        if (packageInfo.Id is not null && packageInfo.Id.StartsWith(Constants.SPDXRefPackage, StringComparison.OrdinalIgnoreCase))
+        {
+            return packageInfo.Id;
+        }
+
+        // Get package identity as package name and package version. If version is empty, just use package name
+        var packageIdentity = $"{packageInfo.Type}-{packageInfo.PackageName}";
+        if (!string.IsNullOrWhiteSpace(packageInfo.PackageVersion))
+        {
+            packageIdentity = string.Join("-", packageInfo.Type, packageInfo.PackageName, packageInfo.PackageVersion);
+        }
+
+        return GenerateSpdxPackageId(packageInfo.Id ?? packageIdentity);
+    }
+
     /// <summary>
     /// Returns the SPDX-compliant file ID.
     /// </summary>

src/Microsoft.Sbom.Extensions.DependencyInjection/ServiceCollectionExtensions.cs

@@ -87,6 +87,7 @@ public static IServiceCollection AddSbomTool(this IServiceCollection services, L
             .AddTransient<IHashCodeGenerator, HashCodeGenerator>()
             .AddTransient<IManifestPathConverter, SbomToolManifestPathConverter>()
             .AddTransient<ManifestGeneratorProvider>()
+            .AddTransient<IManifestGeneratorProvider, ManifestGeneratorProvider>()
             .AddTransient<HashValidator>()
             .AddTransient<ValidationResultGenerator>()
             .AddTransient<IOutputWriter, FileOutputWriter>()

src/Microsoft.Sbom.Parsers.Spdx22SbomParser/MergeableContentProvider.cs

@@ -6,12 +6,15 @@
 using System.IO;
 using System.Linq;
 using Microsoft.Sbom.Common;
+using Microsoft.Sbom.Common.Utils;
 using Microsoft.Sbom.Contracts;
 using Microsoft.Sbom.Extensions;
 using Microsoft.Sbom.Extensions.Entities;
 using Microsoft.Sbom.JsonAsynchronousNodeKit;
 using Microsoft.Sbom.Parser;
 using Microsoft.Sbom.Parsers.Spdx22SbomParser.Entities;
+using Microsoft.Sbom.Utils;
+using Serilog;
 
 namespace Microsoft.Sbom.Parsers.Spdx22SbomParser;
 
@@ -20,11 +23,15 @@ namespace Microsoft.Sbom.Parsers.Spdx22SbomParser;
 /// </summary>
 public class MergeableContentProvider : IMergeableContentProvider
 {
+    private const string DependsOn = "DEPENDS_ON";
+
     private readonly IFileSystemUtils fileSystemUtils;
+    private readonly ILogger logger;
 
-    public MergeableContentProvider(IFileSystemUtils fileSystemUtils)
+    public MergeableContentProvider(IFileSystemUtils fileSystemUtils, ILogger logger)
     {
         this.fileSystemUtils = fileSystemUtils ?? throw new ArgumentNullException(nameof(fileSystemUtils));
+        this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
     }
 
     /// <summary>
@@ -44,6 +51,7 @@ public bool TryGetContent(string filePath, out MergeableContent mergeableContent
 
         if (!fileSystemUtils.FileExists(filePath))
         {
+            logger.Debug($"File '{filePath}' does not exist.");
             mergeableContent = null;
             return false;
         }
@@ -52,16 +60,19 @@ public bool TryGetContent(string filePath, out MergeableContent mergeableContent
 
         if (stream == null)
         {
+            logger.Debug($"Unable to open stream for file '{filePath}'.");
             mergeableContent = null;
             return false;
         }
 
         try
         {
+            logger.Debug($"Attempting to parse SPDX file at '{filePath}'.");
             return GetMergeableContent(stream, out mergeableContent);
         }
         catch (Exception)
         {
+            logger.Debug($"Failed to parse SPDX file at '{filePath}'. It may not be a valid SPDX 2.2 file.");
             mergeableContent = null;
             return false;
         }
@@ -73,9 +84,9 @@ public bool TryGetContent(string filePath, out MergeableContent mergeableContent
     private bool GetMergeableContent(Stream stream, out MergeableContent mergeableContent)
     {
         // Skip sections that are expensive to parse and would require additional logic to properly ignore.
-        var parser = new SPDXParser(stream, ["files", "externalDocumentRefs", "relationships"]);
-        var packages = Enumerable.Empty<SbomPackage>();
-        var relationships = Enumerable.Empty<SbomRelationship>();
+        var parser = new SPDXParser(stream, new[] { "files", "externalDocumentRefs" });
+        IList<SbomPackage> packages = new List<SbomPackage>();
+        IList<SbomRelationship> relationships = new List<SbomRelationship>();
 
         ParserStateResult result = null;
         do
@@ -86,7 +97,10 @@ private bool GetMergeableContent(Stream stream, out MergeableContent mergeableCo
                 switch (result)
                 {
                     case PackagesResult packagesResult:
-                        packages = ProcessPackages(packagesResult.Packages);
+                        packages = ProcessPackages(packagesResult.Packages.ToList());  // ToList() ensures correct parsing of the data
+                        break;
+                    case RelationshipsResult relationshipsResult:
+                        relationships = ProcessRelationships(relationshipsResult.Relationships.ToList());  // ToList() ensures correct parsing of the data
                         break;
                     default:
                         break;
@@ -95,33 +109,105 @@ private bool GetMergeableContent(Stream stream, out MergeableContent mergeableCo
         }
         while (result is not null);
 
-        mergeableContent = new MergeableContent(packages, relationships);
+        mergeableContent = CreateRemappedMergeableContent(packages, relationships);
         return true;
     }
 
     /// <summary>
     /// Process packages and return the collection of <see cref="SbomPackage"/> objects.
     /// </summary>
-    private IEnumerable<SbomPackage> ProcessPackages(IEnumerable<SPDXPackage> spdxPackages)
+    private IList<SbomPackage> ProcessPackages(IReadOnlyList<SPDXPackage> spdxPackages)
     {
         var packages = new List<SbomPackage>();
         foreach (var spdxPackage in spdxPackages)
         {
-            var sbomPackage = new SbomPackage
-            {
-                PackageName = spdxPackage.Name,
-                PackageVersion = spdxPackage.VersionInfo,
-                Id = spdxPackage.SpdxId,
-                FilesAnalyzed = spdxPackage.FilesAnalyzed,
-                LicenseInfo = new LicenseInfo
-                {
-                    Concluded = spdxPackage.LicenseConcluded,
-                    Declared = spdxPackage.LicenseDeclared
-                },
-            };
+            var sbomPackage = spdxPackage.ToSbomPackage();
             packages.Add(sbomPackage);
         }
 
         return packages;
     }
+
+    private IList<SbomRelationship> ProcessRelationships(IReadOnlyList<SPDXRelationship> spdxRelationships)
+    {
+        var relationships = new List<SbomRelationship>();
+
+        foreach (var spdxRelationship in spdxRelationships)
+        {
+            if (IsDependsOnRelationship(spdxRelationship))
+            {
+                var relationship = new SbomRelationship
+                {
+                    SourceElementId = spdxRelationship.SourceElementId,
+                    TargetElementId = spdxRelationship.TargetElementId,
+                    RelationshipType = DependsOn, // Force output consistency
+                };
+                relationships.Add(relationship);
+            }
+        }
+
+        return relationships;
+    }
+
+    private static bool IsDependsOnRelationship(SPDXRelationship spdxRelationship)
+    {
+        // Include both "DEPENDS_ON" and "DEPENDSON" to cover variations in SPDX files, and ignore case.
+        return spdxRelationship.RelationshipType.Equals("DEPENDS_ON", StringComparison.InvariantCultureIgnoreCase) ||
+               spdxRelationship.RelationshipType.Equals("DEPENDSON", StringComparison.InvariantCultureIgnoreCase);
+    }
+
+    /// <summary>
+    /// Remaps the root package ID, updates relationships accordingly, then creates a new <see cref="MergeableContent"/> object.
+    /// </summary>
+    private MergeableContent CreateRemappedMergeableContent(IList<SbomPackage> packages, IList<SbomRelationship> relationships)
+    {
+        var mappedRootPackageId = GetAdjustedRootPackageId(packages);
+
+        AdjustRootPackageRelationships(relationships, mappedRootPackageId);
+
+        logger.Debug($"MergeableContent includes {packages.Count} package(s) and {relationships.Count} relationship(s).");
+
+        return new MergeableContent(packages, relationships);
+    }
+
+    private string GetAdjustedRootPackageId(IList<SbomPackage> packages)
+    {
+        foreach (var package in packages)
+        {
+            if (package.Id == Constants.RootPackageIdValue)
+            {
+                package.Id = null;
+                var newSpdxId = CommonSPDXUtils.GenerateSpdxPackageId(package);
+                package.Id = newSpdxId;
+                logger.Debug($"Remapped root package ID from '{Constants.RootPackageIdValue}' to '{newSpdxId}'");
+                return newSpdxId;
+            }
+        }
+
+        throw new InvalidDataException("No root package found in the SPDX document.");
+    }
+
+    private void AdjustRootPackageRelationships(IList<SbomRelationship> relationships, string mappedRootPackageId)
+    {
+        // Update relationships where the source is the root package.
+        foreach (var relationship in relationships)
+        {
+            if (relationship.SourceElementId == Constants.RootPackageIdValue)
+            {
+                // Update the source element ID to the remapped root package ID.
+                logger.Verbose($"Remapped root package dependency on '{relationship.TargetElementId}'");
+                relationship.SourceElementId = mappedRootPackageId;
+            }
+        }
+
+        // Make the output root depend on the remapped root.
+        var newRootRelationship = new SbomRelationship
+        {
+            SourceElementId = Constants.RootPackageIdValue,
+            TargetElementId = mappedRootPackageId,
+            RelationshipType = DependsOn, // Force output consistency
+        };
+        relationships.Add(newRootRelationship);
+        logger.Debug($"Added new root relationship from '{Constants.RootPackageIdValue}' to '{mappedRootPackageId}'");
+    }
 }