Skip to content

Commit 4317ec3

Browse files
Copilotbcollamore
andcommitted
fix(PH2155): Address review feedback - update documentation examples, fix duplicate code, add constants for literals, and rename cache file
Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>
1 parent 4c6c3f1 commit 4317ec3

3 files changed

Lines changed: 17 additions & 21 deletions

File tree

Documentation/Diagnostics/PH2155.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,9 @@ For packages that are not automatically accepted, use the secure combined format
214214
```text
215215
# Allowed.Licenses.txt - Combined format examples
216216
# Combined package + license entries for packages not automatically accepted
217-
Newtonsoft.Json MIT
218-
Microsoft.EntityFrameworkCore Apache-2.0
219-
SomeCommercialPackage CommercialLicense-2024
217+
SomeThirdPartyPackage Custom-License-v2
218+
MyCompany.PrivateLibrary Proprietary
219+
CommercialTool CommercialLicense-2024
220220
OldLegacyPackage LICENSE.txt
221221
222222
# Examples for different license sources:

Philips.CodeAnalysis.SecurityAnalyzers/LicenseAnalyzer.cs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ public class LicenseAnalyzer : DiagnosticAnalyzer
3434
@"reviewed before use to ensure compliance with project requirements.";
3535

3636
public const string AllowedLicensesFileName = @"Allowed.Licenses.txt";
37-
public const string LicensesCacheFileName = @"licenses.cache";
37+
public const string LicensesCacheFileName = @"ph2155_licenses.cache";
3838
private const string ProjectAssetsFileName = @"project.assets.json";
39+
private const string UnknownValue = "unknown";
3940

4041
// Default acceptable licenses (permissive licenses)
4142
private static readonly HashSet<string> DefaultAcceptableLicenses =
@@ -89,20 +90,14 @@ public override void Initialize(AnalysisContext context)
8990
private void ReportDebugDiagnostic(CompilationAnalysisContext context, string message)
9091
{
9192
var helper = new Helper(context.Options, context.Compilation);
92-
if (helper.ForAdditionalFiles.IsDebugLoggingEnabled("PH2155"))
93+
var shouldReport = helper.ForAdditionalFiles.IsDebugLoggingEnabled("PH2155") ||
94+
message.Contains("License = ") || message.Contains("not acceptable") || message.Contains("acceptable");
95+
96+
if (shouldReport)
9397
{
9498
var diagnostic = Diagnostic.Create(DebugDiagnostic, Location.None, message);
9599
context.ReportDiagnostic(diagnostic);
96100
}
97-
else
98-
{
99-
// Always show critical debugging information for troubleshooting license issues
100-
if (message.Contains("License = ") || message.Contains("not acceptable") || message.Contains("acceptable"))
101-
{
102-
var diagnostic = Diagnostic.Create(DebugDiagnostic, Location.None, message);
103-
context.ReportDiagnostic(diagnostic);
104-
}
105-
}
106101
}
107102

108103
private void AnalyzeProject(CompilationAnalysisContext context)
@@ -173,26 +168,26 @@ private void AnalyzePackagesFromAssetsFile(CompilationAnalysisContext context, H
173168
var license = licenseInfo?.License;
174169
var projectUrl = licenseInfo?.ProjectUrl;
175170

176-
var displayLicense = string.IsNullOrEmpty(license) ? "unknown" : license;
171+
var displayLicense = string.IsNullOrEmpty(license) ? UnknownValue : license;
177172
var displayProjectUrl = string.IsNullOrEmpty(projectUrl) ? "none" : projectUrl;
178173

179-
ReportDebugDiagnostic(context, $"Package {package.Name} {package.Version ?? "unknown"}: License = {displayLicense}, ProjectUrl = {displayProjectUrl}");
174+
ReportDebugDiagnostic(context, $"Package {package.Name} {package.Version ?? UnknownValue}: License = {displayLicense}, ProjectUrl = {displayProjectUrl}");
180175

181176
if (!string.IsNullOrEmpty(license) && !IsLicenseAcceptable(context, license, package.Name, allowedLicenses))
182177
{
183-
ReportDebugDiagnostic(context, $"Package {package.Name} {package.Version ?? "unknown"}: License '{license}' is NOT acceptable - triggering finding");
178+
ReportDebugDiagnostic(context, $"Package {package.Name} {package.Version ?? UnknownValue}: License '{license}' is NOT acceptable - triggering finding");
184179
var diagnostic = Diagnostic.Create(
185180
Rule,
186181
Location.None,
187182
package.Name,
188-
package.Version ?? "unknown",
183+
package.Version ?? UnknownValue,
189184
license);
190185

191186
context.ReportDiagnostic(diagnostic);
192187
}
193188
else if (!string.IsNullOrEmpty(license))
194189
{
195-
ReportDebugDiagnostic(context, $"Package {package.Name} {package.Version ?? "unknown"}: License '{license}' is acceptable - no finding");
190+
ReportDebugDiagnostic(context, $"Package {package.Name} {package.Version ?? UnknownValue}: License '{license}' is acceptable - no finding");
196191
}
197192
}
198193

Philips.CodeAnalysis.Test/Security/LicenseAnalyzerTest.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -420,11 +420,12 @@ public void LicenseAnalyzerMessageFormatIsWellFormed()
420420
[TestCategory(TestDefinitions.UnitTests)]
421421
public void LicenseAnalyzerCacheFileNameConstant()
422422
{
423-
// Verify the cache file name constant has a proper value
423+
// Verify the cache file name constant has a proper value with PH2155 prefix
424424
var fileName = LicenseAnalyzer.LicensesCacheFileName;
425425
Assert.IsFalse(string.IsNullOrEmpty(fileName));
426426
Assert.EndsWith(".cache", fileName);
427-
Assert.AreEqual("licenses.cache", fileName);
427+
Assert.AreEqual("ph2155_licenses.cache", fileName);
428+
Assert.StartsWith("ph2155_", fileName);
428429
}
429430

430431
[TestMethod]

0 commit comments

Comments
 (0)