Skip to content

Commit 9aa98f7

Browse files
Copilotbcollamore
andcommitted
feat(PH2155): Remove backward compatibility, only support combined package+license format
Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>
1 parent 5e93a09 commit 9aa98f7

3 files changed

Lines changed: 40 additions & 146 deletions

File tree

Documentation/Diagnostics/PH2155.md

Lines changed: 20 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ In the package's `.nuspec` file:
5252
### Diagnostic message
5353

5454
```
55-
Package 'SomeGplPackage' version '1.0.0' has an unacceptable license 'GPL-3.0'. Consider adding to Allowed.Licenses.txt if license is acceptable.
55+
Package 'SomeGplPackage' version '1.0.0' has an unacceptable license 'GPL-3.0'. Add 'SomeGplPackage GPL-3.0' to Allowed.Licenses.txt if this specific package and license combination is acceptable.
5656
```
5757

5858
```
59-
Package 'SomePackageWithFileLicense' version '1.0.0' has an unacceptable license 'UNKNOWN_FILE_LICENSE'. Consider adding to Allowed.Licenses.txt if license is acceptable.
59+
Package 'SomePackageWithFileLicense' version '1.0.0' has an unacceptable license 'UNKNOWN_FILE_LICENSE'. Add 'SomePackageWithFileLicense UNKNOWN_FILE_LICENSE' to Allowed.Licenses.txt if this specific package and license combination is acceptable.
6060
```
6161

6262
### Resolution options
@@ -70,37 +70,18 @@ Package 'SomePackageWithFileLicense' version '1.0.0' has an unacceptable license
7070
</ItemGroup>
7171
```
7272

73-
```text
74-
# Allowed.Licenses.txt - Add acceptable licenses to this file
75-
# Commercial license for specific vendor
76-
CommercialLicense-Vendor-X
77-
# Allow GPL for this specific use case
78-
GPL-3.0
79-
# Combined format (recommended for maximum security)
80-
SomePackage GPL-3.0
81-
```
82-
83-
#### Option 2: Use Combined Format (Recommended)
84-
85-
The safest approach is to use the combined package + license format:
86-
87-
```xml
88-
<!-- In your project file -->
89-
<ItemGroup>
90-
<AdditionalFiles Include="Allowed.Licenses.txt" />
91-
</ItemGroup>
92-
```
93-
9473
```text
9574
# Allowed.Licenses.txt - Combined format for maximum security
96-
# Only accept SomeGplPackage if it specifically has GPL-3.0 license
75+
# Only accept specific packages with specific licenses
9776
SomeGplPackage GPL-3.0
77+
SomeCommercialPackage CommercialLicense-Vendor-X
78+
SomePackageWithFileLicense UNKNOWN_FILE_LICENSE
9879
9980
# This prevents both license change issues and file name collisions
100-
# If the package later changes to a different license, it will be flagged again
81+
# If a package later changes to a different license, it will be flagged again
10182
```
10283

103-
#### Option 3: Find alternative package
84+
#### Option 2: Find alternative package
10485

10586
Replace the package with one that has a more permissive license:
10687

@@ -109,7 +90,7 @@ Replace the package with one that has a more permissive license:
10990
<PackageReference Include="AlternativePackage" Version="2.0.0" />
11091
```
11192

112-
#### Option 4: Remove the dependency
93+
#### Option 3: Remove the dependency
11394

11495
If the package is not essential, remove it entirely:
11596

@@ -183,27 +164,23 @@ Configure allowed licenses by creating an `Allowed.Licenses.txt` file in your pr
183164
```
184165

185166
The `Allowed.Licenses.txt` file supports:
186-
- One license identifier per line
167+
- Combined package + license format: `packagename license`
187168
- Comments using `#` at the beginning of lines
188169
- Custom license names for commercial or proprietary packages
189170
- SPDX license identifiers
190-
- **Package names** for whitelisting specific packages (safer than license file names)
171+
- Entries are case-insensitive
191172

192173
### Whitelisting Specific Packages
193174

194-
For packages with unacceptable or unknown licenses that you need to use, you have several whitelisting options in `Allowed.Licenses.txt`:
175+
For packages with unacceptable or unknown licenses that you need to use, use the combined package + license format in `Allowed.Licenses.txt`:
195176

196-
#### 1. Combined Package + License Format (Recommended - Safest)
177+
#### Combined Package + License Format (Required)
197178

198-
The safest approach is to use the combined format: `packagename license`. This provides both package identity verification and license verification, preventing both license change vulnerabilities and license file name collisions.
179+
The analyzer only supports the secure combined format: `packagename license`. This provides both package identity verification and license verification, preventing both license change vulnerabilities and license file name collisions.
199180

200181
```text
201-
# Allowed.Licenses.txt - Combined format examples (RECOMMENDED)
202-
# Standard license identifiers (backward compatibility)
203-
MIT
204-
Apache-2.0
205-
206-
# Combined package + license entries (safest approach)
182+
# Allowed.Licenses.txt - Combined format examples
183+
# Combined package + license entries (only supported format)
207184
Newtonsoft.Json MIT
208185
Microsoft.EntityFrameworkCore Apache-2.0
209186
SomeCommercialPackage CommercialLicense-2024
@@ -214,39 +191,12 @@ OldLegacyPackage UNKNOWN_FILE_LICENSE
214191
# - License file name collisions (entry is specific to one package)
215192
```
216193

217-
#### 2. Package Name Only (Backward Compatibility)
218-
219-
For backward compatibility, you can still whitelist packages by name only, though this is less secure:
220-
221-
```text
222-
# Package name whitelisting (less secure - license changes not detected)
223-
SomeTrustedPackage
224-
AnotherWhitelistedPackage
225-
```
226-
227-
#### 3. License Only (Standard Licenses)
228-
229-
Standard license identifiers continue to work for auto-accepting all packages with those licenses:
230-
231-
```text
232-
# License-only entries (auto-accept any package with these licenses)
233-
MIT
234-
Apache-2.0
235-
BSD-3-Clause
236-
```
237-
238-
### Security Comparison
239-
240-
| Format | Security Level | Pros | Cons |
241-
|--------|---------------|------|------|
242-
| `packagename license` | **Highest** | Prevents license changes AND file name collisions | Requires maintenance when package versions change license |
243-
| `packagename` | Medium | Unique package identification | Vulnerable to license changes |
244-
| `license` | Basic | Simple to maintain | May accept unintended packages with same license file name |
194+
### Security Benefits
245195

246-
**Recommendation**: Use the combined format (`packagename license`) for maximum security, especially for packages with:
247-
- Commercial or proprietary licenses
248-
- Unknown or file-based licenses (`UNKNOWN_FILE_LICENSE`)
249-
- Packages where license compliance is critical
196+
The combined format provides the highest security level:
197+
- **Prevents license changes**: If a whitelisted package changes license, it will be flagged again
198+
- **Prevents file name collisions**: Generic names like "LICENSE.md" can't accidentally whitelist multiple packages
199+
- **Unique identification**: Each entry is specific to one package with one license
250200

251201
### Debug Logging Configuration
252202

Philips.CodeAnalysis.SecurityAnalyzers/LicenseAnalyzer.cs

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public class LicenseAnalyzer : DiagnosticAnalyzer
2929
{
3030
private const string Title = @"Avoid Packages with Unacceptable Licenses";
3131
public const string MessageFormat = @"Package '{0}' version '{1}' has an unacceptable license '{2}'. " +
32-
@"Consider adding to Allowed.Licenses.txt if license is acceptable.";
32+
@"Add '{0} {2}' to Allowed.Licenses.txt if this specific package and license combination is acceptable.";
3333
private const string Description = @"Packages with unacceptable licenses (e.g., copyleft licenses like GPL) should be " +
3434
@"reviewed before use to ensure compliance with project requirements.";
3535

@@ -816,39 +816,22 @@ private bool IsLicenseAcceptable(CompilationAnalysisContext context, string lice
816816
return false;
817817
}
818818

819-
// First check if the license itself is acceptable (backward compatibility)
820-
var isLicenseAcceptable = allowedLicenses.Contains(license);
821-
ReportDebugDiagnostic(context, $"Checking license '{license}' against {allowedLicenses.Count} allowed licenses: {(isLicenseAcceptable ? "FOUND" : "NOT FOUND")}");
822-
823-
// If license is already acceptable, return true
824-
if (isLicenseAcceptable)
819+
if (string.IsNullOrEmpty(packageName))
825820
{
826-
return true;
821+
ReportDebugDiagnostic(context, $"Package name is null or empty - not acceptable");
822+
return false;
827823
}
828824

829-
// Check for combined package name + license format (new safer approach)
830-
// Format: "packagename license" provides both package identity and license verification
831-
if (!string.IsNullOrEmpty(packageName))
832-
{
833-
var combinedEntry = $"{packageName} {license}";
834-
var isCombinedEntryAcceptable = allowedLicenses.Contains(combinedEntry);
835-
ReportDebugDiagnostic(context, $"Checking combined entry '{combinedEntry}': {(isCombinedEntryAcceptable ? "FOUND" : "NOT FOUND")}");
836-
837-
if (isCombinedEntryAcceptable)
838-
{
839-
return true;
840-
}
841-
842-
// Also check if packageName is in the allowed list (backward compatibility)
843-
// This handles cases where specific packages need to be whitelisted by name rather than by license
844-
// This is safer than checking license file names since package names are unique identifiers
845-
var isPackageNameAcceptable = allowedLicenses.Contains(packageName);
846-
ReportDebugDiagnostic(context, $"Checking package name '{packageName}': {(isPackageNameAcceptable ? "FOUND" : "NOT FOUND")}");
825+
// Check for combined package name + license format: "packagename license"
826+
// This provides both package identity and license verification, preventing
827+
// both license change vulnerabilities and license file name collisions
828+
var combinedEntry = $"{packageName} {license}";
829+
var isCombinedEntryAcceptable = allowedLicenses.Contains(combinedEntry);
830+
ReportDebugDiagnostic(context, $"Checking combined entry '{combinedEntry}': {(isCombinedEntryAcceptable ? "FOUND" : "NOT FOUND")}");
847831

848-
if (isPackageNameAcceptable)
849-
{
850-
return true;
851-
}
832+
if (isCombinedEntryAcceptable)
833+
{
834+
return true;
852835
}
853836

854837
// For debugging, show first few allowed licenses to help troubleshoot

Philips.CodeAnalysis.Test/Security/LicenseAnalyzerTest.cs

Lines changed: 7 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,10 +1516,9 @@ public void TestMethod()
15161516
[TestCategory(TestDefinitions.UnitTests)]
15171517
public void LicenseAnalyzerSupportsCombinedPackageNameAndLicenseFormat()
15181518
{
1519-
// Test the new combined format: "packagename license"
1519+
// Test the combined format: "packagename license"
15201520
// This provides the safest whitelisting approach by requiring both package identity and license verification
15211521

1522-
// Test 1: Combined entry should be more specific than individual entries
15231522
// A combined entry "Newtonsoft.Json MIT" should only accept Newtonsoft.Json with MIT license
15241523

15251524
// This test verifies the parsing and matching logic for combined entries
@@ -1529,10 +1528,10 @@ public void LicenseAnalyzerSupportsCombinedPackageNameAndLicenseFormat()
15291528
// Test that the format parsing would work correctly
15301529
var sampleAllowedLicenses = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
15311530
{
1532-
"MIT", // License-only entry (backward compatibility)
1533-
"Newtonsoft.Json", // Package-only entry (backward compatibility)
1534-
"Microsoft.Extensions.Logging MIT", // Combined entry - safest approach
1535-
"SomeCommercialPackage CommercialLicense-V1" // Combined commercial license
1531+
"Microsoft.Extensions.Logging MIT", // Combined entry - only supported format
1532+
"SomeCommercialPackage CommercialLicense-V1", // Combined commercial license
1533+
"Newtonsoft.Json MIT", // Another combined entry
1534+
"TestPackage UNKNOWN_FILE_LICENSE" // Combined entry for file license
15361535
};
15371536

15381537
// Verify that the HashSet contains the expected combined entries
@@ -1591,54 +1590,16 @@ public void LicenseAnalyzerCombinedFormatProvidesBetterSecurity()
15911590
}
15921591
}
15931592

1594-
[TestMethod]
1595-
[TestCategory(TestDefinitions.UnitTests)]
1596-
public void LicenseAnalyzerCombinedFormatBackwardCompatibility()
1597-
{
1598-
// Test that the new combined format maintains backward compatibility
1599-
// with existing license-only and package-only entries
1600-
1601-
var mixedAllowedLicenses = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
1602-
{
1603-
// Legacy license-only entries (should still work)
1604-
"MIT",
1605-
"Apache-2.0",
1606-
"BSD-3-Clause",
1607-
1608-
// Legacy package-only entries (should still work)
1609-
"SomeLegacyPackage",
1610-
"AnotherWhitelistedPackage",
1611-
1612-
// New combined entries (enhanced security)
1613-
"Newtonsoft.Json MIT",
1614-
"Microsoft.Extensions.Logging Apache-2.0",
1615-
"CommercialPackage CommercialLicense-V1"
1616-
};
1617-
1618-
// Verify all formats are preserved in the set
1619-
Assert.Contains("MIT", mixedAllowedLicenses, "Legacy license-only entries should work");
1620-
Assert.Contains("SomeLegacyPackage", mixedAllowedLicenses, "Legacy package-only entries should work");
1621-
Assert.Contains("Newtonsoft.Json MIT", mixedAllowedLicenses, "New combined entries should work");
1622-
1623-
// Verify the set has the expected count
1624-
Assert.HasCount(8, mixedAllowedLicenses, "All entries should be preserved");
1625-
}
1626-
16271593
[TestMethod]
16281594
[TestCategory(TestDefinitions.UnitTests)]
16291595
public void LicenseAnalyzerCombinedFormatExamples()
16301596
{
16311597
// Test realistic examples of how users would configure Allowed.Licenses.txt
1632-
// with the new combined format
1598+
// with the combined format (only supported format)
16331599

16341600
var realisticAllowedLicenses = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
16351601
{
1636-
// Standard permissive licenses (auto-accepted)
1637-
"MIT",
1638-
"Apache-2.0",
1639-
"BSD-3-Clause",
1640-
1641-
// Specific packages with specific licenses (safest approach)
1602+
// Specific packages with specific licenses (only supported approach)
16421603
"Newtonsoft.Json MIT", // Only accept Newtonsoft.Json if it has MIT
16431604
"Microsoft.Extensions.Configuration Apache-2.0", // Only accept if Apache-2.0
16441605
"EntityFramework Apache-2.0", // Only accept if Apache-2.0

0 commit comments

Comments
 (0)