Skip to content

Report MA0153 when the type of a logged property or field is classified - #1418

Merged
meziantou merged 1 commit into
mainfrom
feature/do-not-log-classified-data-types-752e0d
Sep 6, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/do-not-log-classified-data-types-752e0d

Conversation

@meziantou

Copy link
Copy Markdown
Owner

What

MA0153 checked the declaring type for logged properties and fields, but the value type for parameters. An attribute inheriting from DataClassificationAttribute placed on the type of a logged property or field was therefore never reported:

[Taxonomy] class Credentials { }

void A(Credentials p) => logger.LogInformation("{P}", p);   // reported
logger.LogInformation("{Prop}", new Dummy().Prop);          // Dummy.Prop is Credentials -> not reported

Since a property or field is the most common way classified data is exposed, the rule gave a false sense of coverage for exactly the data it is meant to protect.

How

The parameter, property and field branches were three near-duplicates that had drifted apart. They now share one ValidateSymbol local function that checks, in order:

  1. the attribute on the symbol itself,
  2. the attribute on the symbol's type,
  3. the attribute on the declaring type (members only — null for parameters, whose declaring type is the enclosing method's type and says nothing about the value).

Unifying the branches also closed a second hole of the same shape: the MA0153.report_types_with_data_classification_attributes opt-in — reporting a value whose type contains classified members — was implemented only for parameters, so a property or field of such a type was silently missed even with the option on. It now applies to all three.

Reviewer notes

  • The opt-in option is off by default, so the behavior change it unlocks does not affect anyone who has not enabled it.
  • The new detection on the value type does report more code by default. That is the intent of the fix: the attribute means the value is classified, wherever the value comes from.
  • Existing behavior is unchanged for parameters, and the ContainingType check for members is preserved (a new test guards it).

Tests

5 cases added to DoNotLogClassifiedDataAnalyzerTests: attribute on a property's type, on a field's type, on a property's declaring type (regression guard), and the property/type-with-classified-members case with the option both enabled and disabled.

21/21 MA0153 tests pass on all five Roslyn versions (4.8, 4.14, 5.0, 5.6, 5.9); the full solution builds with 0 warnings. dotnet run --project src/DocumentationGenerator exits 0 with no further changes after the docs/Rules/MA0153.md update, which documents the three attribute locations and adds a sample.

The parameter, property and field branches of MA0153 had drifted apart:
parameters were checked against the type of their value, while properties
and fields were only checked against their declaring type. An attribute
inheriting from DataClassificationAttribute placed on the type of a logged
property or field was therefore ignored, which is the most common way
classified data is exposed.

The three branches now share a single check that looks at the symbol
itself, the type of its value, and, for members, their declaring type.
This also applies the report_types_with_data_classification_attributes
option to properties and fields, as it was only implemented for
parameters. That option is disabled by default.
@meziantou
meziantou merged commit da3d015 into main Sep 6, 2026
13 checks passed
@meziantou
meziantou deleted the feature/do-not-log-classified-data-types-752e0d branch September 6, 2026 05:22
This was referenced Sep 6, 2026
This was referenced Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant