Skip to content

Commit 09bf8fd

Browse files
JoseRenanJoseRenan
authored
Merge branch 'main' into copilot/fix-1093
2 parents ec28a81 + 3a58457 commit 09bf8fd

9 files changed

Lines changed: 251 additions & 36 deletions

File tree

‎src/Microsoft.Sbom.Api/Hashing/Algorithms/Sha1HashAlgorithm.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@ namespace Microsoft.Sbom.Api.Hashing.Algorithms;
1212
#pragma warning disable CA5350 // Suppress Do Not Use Weak Cryptographic Algorithms as we use SHA1 intentionally
1313
public class Sha1HashAlgorithm : IHashAlgorithm
1414
{
15-
public byte[] ComputeHash(Stream stream) => SHA1.Create().ComputeHash(stream);
15+
public byte[] ComputeHash(Stream stream) => SHA1.Create().ComputeHash(stream); // CodeQL [SM02196] Sha1 is required per the SPDX spec.
1616
}

‎src/Microsoft.Sbom.Contracts/Contracts/Enums/AlgorithmName.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public static AlgorithmName FromString(string name)
9797
/// Gets equivalent to <see cref="HashAlgorithmName.SHA1"/>.
9898
/// </summary>
9999
#pragma warning disable CA5350 // Suppress Do Not Use Weak Cryptographic Algorithms as we use SHA1 intentionally
100-
public static AlgorithmName SHA1 => new AlgorithmName(nameof(SHA1), stream => System.Security.Cryptography.SHA1.Create().ComputeHash(stream));
100+
public static AlgorithmName SHA1 => new AlgorithmName(nameof(SHA1), stream => System.Security.Cryptography.SHA1.Create().ComputeHash(stream)); // CodeQL [SM02196] Sha1 is required per the SPDX spec.
101101
#pragma warning restore CA5350
102102

103103
/// <summary>

‎src/Microsoft.Sbom.Extensions/MergeableContentExtensions.cs‎

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,100 @@ namespace Microsoft.Sbom.Extensions;
1010

1111
public static class MergeableContentExtensions
1212
{
13-
public static IEnumerable<SbomPackage> ToMergedPackages(this IEnumerable<MergeableContent> contents)
13+
/// <summary>
14+
/// Merges multiple <see cref="MergeableContent"/> instances into a distinct collection of <see cref="SbomPackage"/>
15+
/// objects, including their transitive dependencies.
16+
/// </summary>
17+
public static IEnumerable<SbomPackage> ToMergedPackages(this IEnumerable<MergeableContent> mergeableContents)
1418
{
15-
if (contents == null)
19+
ArgumentNullException.ThrowIfNull(mergeableContents, nameof(mergeableContents));
20+
21+
var distinctPackages = mergeableContents.CollectDistinctPackages();
22+
23+
var distinctDependencies = mergeableContents.CollectDistinctDependencies(distinctPackages);
24+
25+
var packageCallersDictionary = BuildPackageCallersDictionary(distinctDependencies);
26+
27+
// Save the callers in the list of packages
28+
UpdatePackageDependencies(distinctPackages, packageCallersDictionary);
29+
30+
return distinctPackages;
31+
}
32+
33+
/// <summary>
34+
/// Collect the distinct set of packages from all of the MergeableContent objects. In the future, this code
35+
/// will also merge the package data, creating a single package that contains the most complete information
36+
/// that is available to us. For now, it simply returns 1 package (the first we encounter) per unique Id.
37+
/// </summary>
38+
private static IEnumerable<SbomPackage> CollectDistinctPackages(this IEnumerable<MergeableContent> mergeableContents)
39+
{
40+
return mergeableContents
41+
.SelectMany(c => c.Packages)
42+
.GroupBy(p => p.Id)
43+
.Select(g => g.First());
44+
}
45+
46+
/// <summary>
47+
/// Collect the distinct set of package relationships from all of the MergeableContent objects.
48+
/// </summary>
49+
private static IEnumerable<KeyValuePair<string, string>> CollectDistinctDependencies(
50+
this IEnumerable<MergeableContent> mergeableContents, IEnumerable<SbomPackage> distinctPackages)
51+
{
52+
var dependencies = new List<KeyValuePair<string, string>>();
53+
foreach (var mergeableContent in mergeableContents)
1654
{
17-
throw new ArgumentNullException(nameof(contents));
55+
dependencies.AddRange(mergeableContent.Relationships
56+
.Select(r => new KeyValuePair<string, string>(r.SourceElementId, r.TargetElementId)));
1857
}
1958

20-
return contents.SelectMany(c => c.Packages).Distinct();
59+
return dependencies.Distinct();
60+
}
61+
62+
/// <summary>
63+
/// Create a dictionary that maps each package ID to a set of the package IDs that depend on it.
64+
/// </summary>
65+
private static IReadOnlyDictionary<string, ISet<string>> BuildPackageCallersDictionary(IEnumerable<KeyValuePair<string, string>> uniqueDependencies)
66+
{
67+
var packageCallersDictionary = new Dictionary<string, ISet<string>>();
68+
69+
// Build the list of callers for each package.
70+
foreach (var dependency in uniqueDependencies)
71+
{
72+
// Skip the root package reference, which needs to translate to a null entry in the dictionary.
73+
// This constant is the same as Microsoft.Sbom.Api.Constants.SPDXRefRootPackage, but we get
74+
// into a dependency cycle if we try to reference it from here.
75+
if (dependency.Key == "SPDXRef-RootPackage")
76+
{
77+
continue;
78+
}
79+
80+
if (!packageCallersDictionary.TryGetValue(dependency.Value, out var callers))
81+
{
82+
callers = new HashSet<string>();
83+
packageCallersDictionary.Add(dependency.Value, callers);
84+
}
85+
86+
callers.Add(dependency.Key);
87+
}
88+
89+
return packageCallersDictionary;
90+
}
91+
92+
/// <summary>
93+
/// Apply the dependency mappings from the packageCallersDictionary to the distinctPackages.
94+
/// </summary>
95+
private static void UpdatePackageDependencies(IEnumerable<SbomPackage> distinctPackages, IReadOnlyDictionary<string, ISet<string>> packageCallersDictionary)
96+
{
97+
foreach (var package in distinctPackages)
98+
{
99+
if (packageCallersDictionary.TryGetValue(package.Id, out var callers))
100+
{
101+
package.DependOn = callers.ToList();
102+
}
103+
else
104+
{
105+
package.DependOn = null;
106+
}
107+
}
21108
}
22109
}

‎src/Microsoft.Sbom.Parsers.Spdx22SbomParser/Generator.cs‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Text;
99
using System.Text.Json;
1010
using Microsoft.Sbom.Common;
11+
using Microsoft.Sbom.Common.Config;
1112
using Microsoft.Sbom.Common.Utils;
1213
using Microsoft.Sbom.Contracts;
1314
using Microsoft.Sbom.Contracts.Enums;
@@ -25,6 +26,8 @@ namespace Microsoft.Sbom.Parsers.Spdx22SbomParser;
2526
/// </summary>
2627
public class Generator : IManifestGenerator
2728
{
29+
private readonly IConfiguration configuration;
30+
2831
public AlgorithmName[] RequiredHashAlgorithms => new[] { AlgorithmName.SHA256, AlgorithmName.SHA1 };
2932

3033
public string Version { get; set; } = string.Join("-", Constants.SPDXName, Constants.SPDXVersion);
@@ -37,6 +40,11 @@ public class Generator : IManifestGenerator
3740

3841
public string ExternalDocumentRefArrayHeaderName => Constants.ExternalDocumentRefArrayHeaderName;
3942

43+
public Generator(IConfiguration configuration)
44+
{
45+
this.configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));
46+
}
47+
4048
public GenerationResult GenerateJsonDocument(InternalSbomFileInfo fileInfo)
4149
{
4250
if (fileInfo is null)
@@ -161,7 +169,7 @@ public GenerationResult GenerateJsonDocument(SbomPackage packageInfo)
161169

162170
var dependOnIds = (packageInfo.DependOn ?? Enumerable.Empty<string>())
163171
.Where(id => id is not null)
164-
.Select(id => id == Constants.RootPackageIdValue ? id : CommonSPDXUtils.GenerateSpdxPackageId(id))
172+
.Select(id => ShouldWeKeepTheExistingId(id) ? id : CommonSPDXUtils.GenerateSpdxPackageId(id))
165173
.ToList();
166174

167175
return new GenerationResult
@@ -175,6 +183,19 @@ public GenerationResult GenerateJsonDocument(SbomPackage packageInfo)
175183
};
176184
}
177185

186+
private bool ShouldWeKeepTheExistingId(string spdxId)
187+
{
188+
switch (configuration.ManifestToolAction)
189+
{
190+
case ManifestToolActions.Aggregate:
191+
return true;
192+
case ManifestToolActions.Generate:
193+
return spdxId.Equals(Constants.RootPackageIdValue, StringComparison.OrdinalIgnoreCase);
194+
}
195+
196+
return false;
197+
}
198+
178199
public GenerationResult GenerateRootPackage(
179200
IInternalMetadataProvider internalMetadataProvider)
180201
{
@@ -338,7 +359,7 @@ private PackageVerificationCode GetPackageVerificationCode(IInternalMetadataProv
338359

339360
var packageChecksumString = string.Concat(sha1Checksums.OrderBy(s => s));
340361
#pragma warning disable CA5350 // Suppress Do Not Use Weak Cryptographic Algorithms as we use SHA1 intentionally
341-
var sha1Hasher = SHA1.Create();
362+
var sha1Hasher = SHA1.Create(); // CodeQL [SM02196] Sha1 is required per the SPDX spec.
342363
#pragma warning restore CA5350
343364
var hashByteArray = sha1Hasher.ComputeHash(Encoding.Default.GetBytes(packageChecksumString));
344365

‎src/Microsoft.Sbom.Parsers.Spdx22SbomParser/Utils/SPDXExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public static string AddExternalReferenceSpdxId(this SpdxExternalDocumentReferen
123123

124124
if (checksums is null || !checksums.Any(c => c.Algorithm == AlgorithmName.SHA1))
125125
{
126-
throw new MissingHashValueException($"The external reference {name} is missing the {HashAlgorithmName.SHA1} hash value.");
126+
throw new MissingHashValueException($"The external reference {name} is missing the {HashAlgorithmName.SHA1} hash value."); // CodeQL [SM02196] Sha1 is required per the SPDX spec.
127127
}
128128

129129
// Get the SHA1 for this file.

‎src/Microsoft.Sbom.Parsers.Spdx30SbomParser/Generator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ private PackageVerificationCode GetPackageVerificationCode(IInternalMetadataProv
654654

655655
var packageChecksumString = string.Concat(sha1Checksums.OrderBy(s => s));
656656
#pragma warning disable CA5350 // Suppress Do Not Use Weak Cryptographic Algorithms as we use SHA1 intentionally
657-
var sha1Hasher = SHA1.Create();
657+
var sha1Hasher = SHA1.Create(); // CodeQL [SM02196] Sha1 is required per the SPDX spec.
658658
#pragma warning restore CA5350
659659
var hashByteArray = sha1Hasher.ComputeHash(Encoding.Default.GetBytes(packageChecksumString));
660660

‎src/Microsoft.Sbom.Parsers.Spdx30SbomParser/Utils/SPDXExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public static string AddExternalSpdxId(this ExternalMap reference, string name,
7272
var sha1checksums = checksums.Where(c => c.Algorithm == AlgorithmName.SHA1);
7373
if (checksums is null || !sha1checksums.Any())
7474
{
75-
throw new MissingHashValueException($"The external reference {name} is missing the {HashAlgorithmName.SHA1} hash value.");
75+
throw new MissingHashValueException($"The external reference {name} is missing the {HashAlgorithmName.SHA1} hash value."); // CodeQL [SM02196] Sha1 is required per the SPDX spec.
7676
}
7777

7878
// Get the SHA1 for this file.

‎test/Microsoft.Sbom.Api.Tests/Workflows/SbomAggregationWorkflowTests.cs‎

Lines changed: 71 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.IO;
67
using System.Linq;
@@ -34,6 +35,8 @@ public class SbomAggregationWorkflowTests
3435
private const string PathToSpdx22ManifestForArtifactKey2 = ExternalManifestDir2 + RelativePathToSpdx22Manifest;
3536
private const string PathToSpdx30ManifestForArtifactKey2 = ExternalManifestDir2 + RelativePathToSpdx30Manifest;
3637
private const string TempDirPath = "temp-dir";
38+
private const string PackageId1 = "package-id-1";
39+
private const string PackageId2 = "package-id-2";
3740

3841
private Mock<ILogger> loggerMock;
3942
private Mock<IRecorder> recorderMock;
@@ -50,6 +53,8 @@ public class SbomAggregationWorkflowTests
5053
private Mock<IMetadataBuilderFactory> metadataBuilderFactoryMock;
5154
private SbomAggregationWorkflow testSubject;
5255

56+
private List<SbomPackage> actualPackageList;
57+
5358
private Dictionary<string, ArtifactInfo> artifactInfoMapStub = new Dictionary<string, ArtifactInfo>()
5459
{
5560
{ ArtifactKey1, new ArtifactInfo() { IgnoreMissingFiles = true, SkipSigningCheck = true } },
@@ -92,6 +97,8 @@ public void BeforeEachTest()
9297
fileSystemUtilsMock.Object,
9398
metadataBuilderFactoryMock.Object,
9499
new[] { mergeableContent22ProviderMock.Object, mergeableContent30ProviderMock.Object });
100+
101+
actualPackageList = null;
95102
}
96103

97104
[TestCleanup]
@@ -118,7 +125,9 @@ public async Task RunAsync_ReturnsFalseOnNoArtifactInfoMapInput()
118125
.Returns(new ConfigurationSetting<Dictionary<string, ArtifactInfo>>(new Dictionary<string, ArtifactInfo>()));
119126

120127
var result = await testSubject.RunAsync();
128+
121129
Assert.IsFalse(result);
130+
Assert.IsNull(actualPackageList);
122131
}
123132

124133
[TestMethod]
@@ -144,7 +153,9 @@ public async Task RunAsync_ReturnsFalseOnNoValidSpdxSbomsFound()
144153
}
145154

146155
var result = await testSubject.RunAsync();
156+
147157
Assert.IsFalse(result);
158+
Assert.IsNull(actualPackageList);
148159
}
149160

150161
[TestMethod]
@@ -182,16 +193,21 @@ public async Task RunAsync_ReturnsFalseOnOnlySpdx30SbomsFound()
182193
}
183194

184195
var result = await testSubject.RunAsync();
196+
185197
Assert.IsFalse(result);
198+
Assert.IsNull(actualPackageList);
186199
}
187200

188201
[TestMethod]
189202
public async Task RunAsync_ReturnsFalseOnFailedValidationWorkflow()
190203
{
191204
SetUpSbomsToValidate();
192205
SetUpMinimalValidation(false);
206+
193207
var result = await testSubject.RunAsync();
208+
194209
Assert.IsFalse(result);
210+
Assert.IsNull(actualPackageList);
195211
}
196212

197213
[TestMethod]
@@ -212,7 +228,9 @@ public async Task RunAsync_MixOfValidAndInvalidInputSboms()
212228
sbomValidationWorkflowMock1.Setup(x => x.RunAsync()).ReturnsAsync(false);
213229

214230
var result = await testSubject.RunAsync();
231+
215232
Assert.IsFalse(result);
233+
Assert.IsNull(actualPackageList);
216234
}
217235

218236
[TestMethod]
@@ -226,29 +244,70 @@ public async Task RunAsync_MinimalHappyPath_CallsGenerationWorkflow(bool expecte
226244
SetupMinimalMergeableContentProviderMocks();
227245

228246
var result = await testSubject.RunAsync();
247+
229248
Assert.AreEqual(expectedResult, result);
249+
ValidateActualPackageList();
230250
}
231251

232252
private void SetupMinimalMergeableContentProviderMocks()
233253
{
234-
var minimalMergeableContent = new MergeableContent(
235-
new List<SbomPackage> { new SbomPackage() }, Enumerable.Empty<SbomRelationship>());
254+
// mergeableContent 1 has the root package that depends on PackageId1.
255+
// mergeableContent 2 has the root package that depends on PackageId2, which in turn depends on PackageId1.
256+
var minimalMergeableContent1 = new MergeableContent(
257+
[
258+
new SbomPackage { Id = PackageId1 }
259+
],
260+
Enumerable.Empty<SbomRelationship>());
261+
var minimalMergeableContent2 = new MergeableContent(
262+
[
263+
new SbomPackage { Id = PackageId1 },
264+
new SbomPackage { Id = PackageId2 }
265+
],
266+
[
267+
new SbomRelationship
268+
{
269+
SourceElementId = PackageId2,
270+
TargetElementId = PackageId1,
271+
RelationshipType = "DEPENDS_ON"
272+
}
273+
]);
274+
275+
MergeableContent nullMergeableContent = null;
236276

237277
mergeableContent22ProviderMock
238-
.Setup(m => m.TryGetContent(PathToSpdx22ManifestForArtifactKey1, out minimalMergeableContent))
278+
.Setup(m => m.TryGetContent(PathToSpdx22ManifestForArtifactKey1, out minimalMergeableContent1))
239279
.Returns(true);
240280
mergeableContent22ProviderMock
241-
.Setup(m => m.TryGetContent(PathToSpdx22ManifestForArtifactKey2, out minimalMergeableContent))
281+
.Setup(m => m.TryGetContent(PathToSpdx22ManifestForArtifactKey2, out minimalMergeableContent2))
242282
.Returns(true);
243283
mergeableContent30ProviderMock
244-
.Setup(m => m.TryGetContent(PathToSpdx30ManifestForArtifactKey1, out minimalMergeableContent))
245-
.Returns(true);
284+
.Setup(m => m.TryGetContent(PathToSpdx30ManifestForArtifactKey1, out nullMergeableContent))
285+
.Returns(false);
246286
mergeableContent30ProviderMock
247-
.Setup(m => m.TryGetContent(PathToSpdx30ManifestForArtifactKey2, out minimalMergeableContent))
248-
.Returns(true);
287+
.Setup(m => m.TryGetContent(PathToSpdx30ManifestForArtifactKey2, out nullMergeableContent))
288+
.Returns(false);
289+
290+
recorderMock.Setup(m => m.RecordAggregationSource(It.IsAny<string>(), 1, 0)); // minimalMergeableContent1
291+
recorderMock.Setup(m => m.RecordAggregationSource(It.IsAny<string>(), 2, 1)); // minimalMergeableContent2
292+
}
293+
294+
private void ValidateActualPackageList()
295+
{
296+
Assert.IsNotNull(actualPackageList);
297+
Assert.AreEqual(2, actualPackageList.Count);
298+
299+
// Package order is not determined, so sort it to simplify our assertions.
300+
actualPackageList.Sort((x, y) => string.Compare(x.Id, y.Id, StringComparison.Ordinal));
301+
302+
// PackageId1 should list PackageId2 in DependOn
303+
Assert.AreEqual(PackageId1, actualPackageList[0].Id);
304+
Assert.IsNotNull(actualPackageList[0].DependOn);
305+
Assert.AreEqual(1, actualPackageList[0].DependOn.Count);
306+
Assert.AreEqual(PackageId2, actualPackageList[0].DependOn[0]);
249307

250-
recorderMock.Setup(m => m.RecordAggregationSource(
251-
It.IsAny<string>(), 1 /* package count */, 0 /* relationship count */));
308+
// PackageId2 should have a null DependOn
309+
Assert.AreEqual(PackageId2, actualPackageList[1].Id);
310+
Assert.IsNull(actualPackageList[1].DependOn);
252311
}
253312

254313
private void SetUpSbomsToValidate()
@@ -328,7 +387,8 @@ private void SetupMinimalGenerationMocks(bool expectedResult)
328387
configurationMock.SetupSet(m => m.BuildComponentPath = It.IsAny<ConfigurationSetting<string>>());
329388
configurationMock.SetupSet(m => m.BuildDropPath = It.IsAny<ConfigurationSetting<string>>());
330389
configurationMock.SetupSet(m => m.ManifestInfo = It.IsAny<ConfigurationSetting<IList<ManifestInfo>>>());
331-
configurationMock.SetupSet(m => m.PackagesList = It.IsAny<ConfigurationSetting<IEnumerable<SbomPackage>>>());
390+
configurationMock.SetupSet(m => m.PackagesList = It.IsAny<ConfigurationSetting<IEnumerable<SbomPackage>>>())
391+
.Callback<ConfigurationSetting<IEnumerable<SbomPackage>>>(c => actualPackageList = c.Value.ToList());
332392

333393
fileSystemUtilsMock.Setup(m => m.CreateDirectory(Path.Join(TempDirPath, "aggregated-build-drop"))).Returns<DirectoryInfo>(null);
334394

0 commit comments

Comments
 (0)