Skip to content

Commit 111282b

Browse files
authored
Loosen constraints on SBOM/SPDX validation (#572)
* Previously, we have been using enums to model ExternalRepositoryType and RelationshipType in SPDX files. This worked acceptably when we only ever had to validate SPDXs that we ourselves created (so we controlled the possible range of values). However, this requires us to know and enumerate every possible legal value for each enum, and it is vulnerable to breaking changes between minor versions of SPDX when deserializing. For example, when processing 3P SBOMs that were produced by other tools, we may encounter values that are legal by the SPDX spec, but not defined in our enums; this prevents us from parsing those files successfully. It also is a factor preventing us from flexibly handling all 2.x SPDX rather than just 2.2.1, which is a feature we require for redaction. This change models RelationshipType and ExternalRepositoryType as strings, rather than enums. That allows us to tolerate a wide range of possible input values without worrying if they align with our known range. The SBOM generation code still uses the enum types to enforce expected output values, simply converting to string at serialization time. The test case that asserted known values for RelationshipType has been removed since it is no longer relevant; an unrecognized RelationshipType should not cause failure anymore. * We will no longer require license and copyright related fields on SPDXFile and SPDXPackage entities. It's not clear why we previously required them, but they are not required by the SPDX v2 spec nor by the NTIA minimum required elements for an SBOM. Loosening this constraint improves our ability to handle all 2.x SPDXs, including those produced by 3P tools that may not follow the same generation conventions as sbom-tool. In addition to removing the JsonRequired attribute from several fields, remove the test cases that check that the attribute is appropriately applied, since they are no longer relevant.
1 parent 8141070 commit 111282b

File tree

9 files changed

+17
-17
lines changed

9 files changed

+17
-17
lines changed

src/Microsoft.Sbom.Parsers.Spdx22SbomParser/Entities/ExternalReference.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public class ExternalReference
2525
/// https://spdx.github.io/spdx-spec/appendix-VI-external-repository-identifiers/.
2626
/// </summary>
2727
[JsonPropertyName("referenceType")]
28-
public ExternalRepositoryType Type { get; set; }
28+
public string Type { get; set; }
2929

3030
/// <summary>
3131
/// Gets or sets a unique string without any spaces that specifies a location where the package specific information

src/Microsoft.Sbom.Parsers.Spdx22SbomParser/Entities/SPDXPackage.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ public class SPDXPackage
5454
/// <summary>
5555
/// Gets or sets contain the license the SPDX file creator has concluded as the package or alternative values.
5656
/// </summary>
57-
[JsonRequired]
5857
[JsonPropertyName("licenseConcluded")]
5958
public string LicenseConcluded { get; set; }
6059

@@ -68,14 +67,12 @@ public class SPDXPackage
6867
/// <summary>
6968
/// Gets or sets contains a list of licenses the have been declared by the authors of the package.
7069
/// </summary>
71-
[JsonRequired]
7270
[JsonPropertyName("licenseDeclared")]
7371
public string LicenseDeclared { get; set; }
7472

7573
/// <summary>
7674
/// Gets or sets copyright holder of the package, as well as any dates present.
7775
/// </summary>
78-
[JsonRequired]
7976
[JsonPropertyName("copyrightText")]
8077
public string CopyrightText { get; set; }
8178

src/Microsoft.Sbom.Parsers.Spdx22SbomParser/Entities/SPDXRelationship.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public class SPDXRelationship
1515
/// Gets or sets defines the type of the relationship between the source and the target element.
1616
/// </summary>
1717
[JsonPropertyName("relationshipType")]
18-
public SPDXRelationshipType RelationshipType { get; set; }
18+
public string RelationshipType { get; set; }
1919

2020
/// <summary>
2121
/// Gets or sets the id of the target element with whom the source element has a relationship.

src/Microsoft.Sbom.Parsers.Spdx22SbomParser/Entities/SpdxFile.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,21 +34,18 @@ public class SPDXFile
3434
/// <summary>
3535
/// Gets or sets contain the license the SPDX file creator has concluded as the package or alternative values.
3636
/// </summary>
37-
[JsonRequired]
3837
[JsonPropertyName("licenseConcluded")]
3938
public string LicenseConcluded { get; set; }
4039

4140
/// <summary>
4241
/// Gets or sets contains the license information actually found in the file, if any.
4342
/// </summary>
44-
[JsonRequired]
4543
[JsonPropertyName("licenseInfoInFiles")]
4644
public IEnumerable<string> LicenseInfoInFiles { get; set; }
4745

4846
/// <summary>
4947
/// Gets or sets copyright holder of the package, as well as any dates present.
5048
/// </summary>
51-
[JsonRequired]
5249
[JsonPropertyName("copyrightText")]
5350
public string FileCopyrightText { get; set; }
5451

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ public GenerationResult GenerateRootPackage(
206206
new ExternalReference
207207
{
208208
ReferenceCategory = ReferenceCategory.PACKAGE_MANAGER.ToNormalizedString(),
209-
Type = ExternalRepositoryType.purl,
209+
Type = ExternalRepositoryType.purl.ToString(),
210210
Locator = internalMetadataProvider.GetSwidTagId()
211211
}
212212
},
@@ -248,7 +248,7 @@ public GenerationResult GenerateJsonDocument(Relationship relationship)
248248
var spdxRelationship = new SPDXRelationship
249249
{
250250
SourceElementId = sourceElement,
251-
RelationshipType = GetSPDXRelationshipType(relationship.RelationshipType),
251+
RelationshipType = GetSPDXRelationshipType(relationship.RelationshipType).ToString(),
252252
TargetElementId = targetElement
253253
};
254254

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public static void AddPackageUrls(this SPDXPackage spdxPackage, SbomPackage pack
7777
var extRef = new ExternalReference
7878
{
7979
ReferenceCategory = ReferenceCategory.PACKAGE_MANAGER.ToNormalizedString(),
80-
Type = ExternalRepositoryType.purl,
80+
Type = ExternalRepositoryType.purl.ToString(),
8181
Locator = packageInfo.PackageUrl,
8282
};
8383

test/Microsoft.Sbom.Parsers.Spdx22SbomParser.Tests/Parser/SbomFileParserTests.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,6 @@ public void MissingPropertiesTest_ThrowsSHA256()
104104
[DataRow(SbomFileJsonStrings.JsonWith1FileMissingNameString)]
105105
[DataRow(SbomFileJsonStrings.JsonWith1FileMissingIDString)]
106106
[DataRow(SbomFileJsonStrings.JsonWith1FileMissingChecksumsString)]
107-
[DataRow(SbomFileJsonStrings.JsonWith1FileMissingLicenseConcludedString)]
108-
[DataRow(SbomFileJsonStrings.JsonWith1FileMissingLicenseInfoInFilesString)]
109-
[DataRow(SbomFileJsonStrings.JsonWith1FileMissingCopyrightString)]
110107
[DataRow(SbomFileJsonStrings.JsonWith1FileMissingCopyrightAndPathString)]
111108
[TestMethod]
112109
public void MissingPropertiesTest_Throws(string json)

test/Microsoft.Sbom.Parsers.Spdx22SbomParser.Tests/Parser/SbomRelationshipParserTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ public void IgnoresAdditionalPropertiesTest(string json)
7373
}
7474

7575
[DataTestMethod]
76-
[DataRow(RelationshipStrings.MalformedJsonRelationshipsStringBadRelationshipType)]
7776
[DataRow(RelationshipStrings.MalformedJsonRelationshipsString)]
7877
[TestMethod]
7978
public void MalformedJsonTest_Throws(string json)

test/Microsoft.Sbom.Parsers.Spdx22SbomParser.Tests/Utils/SPDXExtensionsTest.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,12 @@ public void AddPackageUrlsTest_Success()
4444
spdxPackage.AddPackageUrls(packageInfo);
4545
var externalRef = spdxPackage.ExternalReferences.First();
4646
Assert.AreEqual(ReferenceCategory.PACKAGE_MANAGER.ToNormalizedString(), externalRef.ReferenceCategory);
47-
Assert.AreEqual(ExternalRepositoryType.purl, externalRef.Type);
47+
48+
// ExternalRepositoryTypes are deserialized as strings for portability when handling 3P SBOMs,
49+
// but in the context of this test we expect the value to align with a known enum value. So
50+
// convert to enum for comparison.
51+
Enum.TryParse<ExternalRepositoryType>(externalRef.Type, out var refType);
52+
Assert.AreEqual(ExternalRepositoryType.purl, refType);
4853
Assert.AreEqual(PackageUrl, externalRef.Locator);
4954
}
5055

@@ -84,7 +89,12 @@ public void AddPackageUrlsTest_WithSpecialCharacter_Success(string inputUrl, str
8489
var externalRef = spdxPackage.ExternalReferences.First();
8590

8691
Assert.AreEqual(ReferenceCategory.PACKAGE_MANAGER.ToNormalizedString(), externalRef.ReferenceCategory);
87-
Assert.AreEqual(ExternalRepositoryType.purl, externalRef.Type);
92+
93+
// ExternalRepositoryTypes are deserialized as strings for portability when handling 3P SBOMs,
94+
// but in the context of this test we expect the value to align with a known enum value. So
95+
// convert to enum for comparison.
96+
Enum.TryParse<ExternalRepositoryType>(externalRef.Type, out var refType);
97+
Assert.AreEqual(ExternalRepositoryType.purl, refType);
8898
Assert.AreEqual(expectedUrl, externalRef.Locator);
8999
}
90100

0 commit comments

Comments
 (0)