diff --git a/docs/Rules/MA0153.md b/docs/Rules/MA0153.md index dae274b1b..5a694738e 100644 --- a/docs/Rules/MA0153.md +++ b/docs/Rules/MA0153.md @@ -4,7 +4,8 @@ Source: [DoNotLogClassifiedDataAnalyzer.cs](https://github.com/meziantou/Meziant Detects when a log parameter is decorated with an attribute that inherits from `Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute`. -Most of the time, these values should not be used with `[LogProperties]` to redact values. + +Optionally (when configured), it can also detect when logging an object whose type contains members (properties or fields) decorated with such attributes. Since there could be multiple log providers, any type that has sensitive attributes should not be logged directly, as some providers could log inner fields/properties and ignore the attributes. ````c# using Microsoft.Extensions.Logging; @@ -14,16 +15,39 @@ ILogger logger; // non-compliant as Prop is decorated with an attribute that inherits from DataClassificationAttribute logger.LogInformation("{Prop}", new Dummy().Prop); +// non-compliant as PatientInfo contains properties decorated with DataClassificationAttribute (when MA0153.report_types_with_data_classification_attributes is enabled) +PatientInfo patient = new(); +logger.LogInformation("{Patient}", patient); + class Dummy { [PiiAttribute] public string Prop { get; set; } } +class PatientInfo +{ + [PiiAttribute] + public string PatientId { get; set; } + + public ulong RecordId { get; set; } + + [PiiAttribute] + public string FirstName { get; set; } +} + class PiiAttribute : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute { - public TaxonomyAttribute() : base(default) + public PiiAttribute() : base(default) { } } ```` + +# Configuration (`.editorconfig`) + +```` +[*.cs] +# Enable or disable reporting objects containing properties/fields with DataClassificationAttribute +MA0153.report_types_with_data_classification_attributes = true|false # Default: false +```` diff --git a/src/Meziantou.Analyzer/Rules/DoNotLogClassifiedDataAnalyzer.cs b/src/Meziantou.Analyzer/Rules/DoNotLogClassifiedDataAnalyzer.cs index 3349c5a27..78cb5dc25 100644 --- a/src/Meziantou.Analyzer/Rules/DoNotLogClassifiedDataAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/DoNotLogClassifiedDataAnalyzer.cs @@ -1,4 +1,5 @@ using System.Collections.Immutable; +using Meziantou.Analyzer.Configurations; using Meziantou.Analyzer.Internals; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; @@ -61,6 +62,8 @@ public void AnalyzeInvocationDeclaration(OperationAnalysisContext context) var operation = (IInvocationOperation)context.Operation; if (operation.TargetMethod.ContainingType.IsEqualTo(LoggerExtensionsSymbol) && FindLogParameters(operation.TargetMethod, out var argumentsParameter)) { + var reportTypesWithDataClassification = context.Options.GetConfigurationValue(operation, RuleIdentifiers.DoNotLogClassifiedData + ".report_types_with_data_classification_attributes", defaultValue: false); + foreach (var argument in operation.Arguments) { var parameter = argument.Parameter; @@ -71,26 +74,26 @@ public void AnalyzeInvocationDeclaration(OperationAnalysisContext context) { if (argument.ArgumentKind == ArgumentKind.ParamArray && argument.Value is IArrayCreationOperation arrayCreation && arrayCreation.Initializer is not null) { - ValidateDataClassification(context, arrayCreation.Initializer.ElementValues); + ValidateDataClassification(context, arrayCreation.Initializer.ElementValues, reportTypesWithDataClassification); } } } } } - private void ValidateDataClassification(DiagnosticReporter diagnosticReporter, IEnumerable operations) + private void ValidateDataClassification(DiagnosticReporter diagnosticReporter, IEnumerable operations, bool reportTypesWithDataClassification) { foreach (var operation in operations) { - ValidateDataClassification(diagnosticReporter, operation); + ValidateDataClassification(diagnosticReporter, operation, reportTypesWithDataClassification); } } - private void ValidateDataClassification(DiagnosticReporter diagnosticReporter, IOperation operation) + private void ValidateDataClassification(DiagnosticReporter diagnosticReporter, IOperation operation, bool reportTypesWithDataClassification) { - ValidateDataClassification(diagnosticReporter, operation, operation, DataClassificationAttributeSymbol!); + ValidateDataClassification(diagnosticReporter, operation, operation, DataClassificationAttributeSymbol!, reportTypesWithDataClassification); - static void ValidateDataClassification(DiagnosticReporter diagnosticReporter, IOperation operation, IOperation reportOperation, INamedTypeSymbol dataClassificationAttributeSymbol) + static void ValidateDataClassification(DiagnosticReporter diagnosticReporter, IOperation operation, IOperation reportOperation, INamedTypeSymbol dataClassificationAttributeSymbol, bool reportTypesWithDataClassification) { operation = operation.UnwrapConversionOperations(); if (operation is IParameterReferenceOperation { Parameter: var parameter }) @@ -99,6 +102,10 @@ static void ValidateDataClassification(DiagnosticReporter diagnosticReporter, IO { diagnosticReporter.ReportDiagnostic(Rule, reportOperation); } + else if (reportTypesWithDataClassification && TypeContainsMembersWithDataClassification(parameter.Type, dataClassificationAttributeSymbol)) + { + diagnosticReporter.ReportDiagnostic(Rule, reportOperation); + } } else if (operation is IPropertyReferenceOperation { Property: var property }) { @@ -116,11 +123,60 @@ static void ValidateDataClassification(DiagnosticReporter diagnosticReporter, IO } else if (operation is IArrayElementReferenceOperation arrayElementReferenceOperation) { - ValidateDataClassification(diagnosticReporter, arrayElementReferenceOperation.ArrayReference, reportOperation, dataClassificationAttributeSymbol); + ValidateDataClassification(diagnosticReporter, arrayElementReferenceOperation.ArrayReference, reportOperation, dataClassificationAttributeSymbol, reportTypesWithDataClassification); + } + else if (reportTypesWithDataClassification) + { + // Check if the operation's type contains members with DataClassificationAttribute + var type = operation.Type; + if (type is not null) + { + // Early exit for value types (enums, structs) from well-known namespaces + if (type.IsValueType && type is INamedTypeSymbol namedType) + { + var ns = namedType.ContainingNamespace?.ToDisplayString(); + if (ns is not null && (ns.StartsWith("System.", StringComparison.Ordinal) || ns == "System")) + { + return; + } + } + + if (TypeContainsMembersWithDataClassification(type, dataClassificationAttributeSymbol)) + { + diagnosticReporter.ReportDiagnostic(Rule, reportOperation); + } + } } } } + private static bool TypeContainsMembersWithDataClassification(ITypeSymbol type, INamedTypeSymbol dataClassificationAttributeSymbol) + { + if (type is null) + return false; + + // Don't check primitive types, strings, or common system types + if (type.SpecialType != SpecialType.None) + return false; + + // Check all members (properties and fields) including inherited members + foreach (var member in type.GetAllMembers()) + { + if (member is IPropertySymbol property) + { + if (property.HasAttribute(dataClassificationAttributeSymbol, inherits: true)) + return true; + } + else if (member is IFieldSymbol field) + { + if (field.HasAttribute(dataClassificationAttributeSymbol, inherits: true)) + return true; + } + } + + return false; + } + private static bool FindLogParameters(IMethodSymbol methodSymbol, out IParameterSymbol? arguments) { IParameterSymbol? message = null; diff --git a/tests/Meziantou.Analyzer.Test/Rules/DoNotLogClassifiedDataAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/DoNotLogClassifiedDataAnalyzerTests.cs index 0f085143e..be9719afa 100755 --- a/tests/Meziantou.Analyzer.Test/Rules/DoNotLogClassifiedDataAnalyzerTests.cs +++ b/tests/Meziantou.Analyzer.Test/Rules/DoNotLogClassifiedDataAnalyzerTests.cs @@ -195,4 +195,231 @@ await CreateProjectBuilder() .WithSourceCode(SourceCode) .ValidateAsync(); } + + [Fact] + public async Task Logger_LogInformation_DataClassification_TypeWithClassifiedProperty() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +PatientInfo p = new(); +logger.LogInformation("{Patient}", [|p|]); + +class PatientInfo +{ + [PiiData] public string PatientId { get; set; } + public ulong RecordId { get; set; } + [PiiData] public string FirstName { get; set; } +} + +class PiiData : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public PiiData() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .AddAnalyzerConfiguration("MA0153.report_types_with_data_classification_attributes", "true") + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Logger_LogInformation_DataClassification_TypeWithClassifiedField() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +PatientInfo p = new(); +logger.LogInformation("{Patient}", [|p|]); + +class PatientInfo +{ + [PiiData] public string PatientId; + public ulong RecordId; +} + +class PiiData : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public PiiData() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .AddAnalyzerConfiguration("MA0153.report_types_with_data_classification_attributes", "true") + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Logger_LogInformation_NoDataClassification_TypeWithNoClassifiedMembers() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +PatientInfo p = new(); +logger.LogInformation("{Patient}", p); + +class PatientInfo +{ + public string PatientId { get; set; } + public ulong RecordId { get; set; } +} + +class PiiData : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public PiiData() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Logger_LogInformation_DataClassification_ObjectCreationWithClassifiedProperty() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +logger.LogInformation("{Patient}", [|new PatientInfo()|]); + +class PatientInfo +{ + [PiiData] public string PatientId { get; set; } + public ulong RecordId { get; set; } +} + +class PiiData : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public PiiData() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .AddAnalyzerConfiguration("MA0153.report_types_with_data_classification_attributes", "true") + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Logger_LogInformation_NoDataClassification_PrimitiveType() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +int value = 42; +logger.LogInformation("{Value}", value); + +class PiiData : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public PiiData() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Logger_LogInformation_NoDataClassification_StringType() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +string value = "test"; +logger.LogInformation("{Value}", value); + +class PiiData : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public PiiData() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Logger_LogInformation_DataClassification_TypeWithClassifiedProperty_ConfigDisabled() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +PatientInfo p = new(); +logger.LogInformation("{Patient}", p); + +class PatientInfo +{ + [PiiData] public string PatientId { get; set; } + public ulong RecordId { get; set; } +} + +class PiiData : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public PiiData() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .AddAnalyzerConfiguration("MA0153.report_types_with_data_classification_attributes", "false") + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Logger_LogInformation_DataClassification_TypeWithClassifiedProperty_ConfigEnabled() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +PatientInfo p = new(); +logger.LogInformation("{Patient}", [|p|]); + +class PatientInfo +{ + [PiiData] public string PatientId { get; set; } + public ulong RecordId { get; set; } +} + +class PiiData : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public PiiData() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .AddAnalyzerConfiguration("MA0153.report_types_with_data_classification_attributes", "true") + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Logger_LogInformation_DataClassification_DirectProperty_ConfigDisabled() + { + const string SourceCode = """ +using Microsoft.Extensions.Logging; + +ILogger logger = null; +logger.LogInformation("{Prop}", [|new Dummy().Prop|]); + +class Dummy +{ + [TaxonomyAttribute()] + public string Prop { get; set; } +} + +class TaxonomyAttribute : Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute +{ + public TaxonomyAttribute() : base(Microsoft.Extensions.Compliance.Classification.DataClassification.Unknown) { } +} +"""; + await CreateProjectBuilder() + .AddAnalyzerConfiguration("MA0153.report_types_with_data_classification_attributes", "false") + .WithSourceCode(SourceCode) + .ValidateAsync(); + } }