Fix JsonIgnore validation bypass for write-only conditions#64729
Conversation
…ditions Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
…ability Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a validation bypass vulnerability where properties decorated with [JsonIgnore] and write-only conditions (e.g., WhenWritingDefault, WhenWritingNull) were incorrectly skipping validation during model binding. The fix ensures that only conditions affecting deserialization (Always, WhenReading) skip validation, while write-only conditions and Never continue to be validated.
Key Changes:
- Updated
IsJsonIgnoredPropertymethod to inspect theJsonIgnore.Conditionproperty and only skip validation forAlwaysandWhenReadingconditions - Added comprehensive test coverage for all six
JsonIgnoreConditionenum values - Generated snapshot verification confirming correct validation behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Validation/gen/Extensions/ITypeSymbolExtensions.cs |
Modified IsJsonIgnoredProperty to check JsonIgnore.Condition and only skip validation for Always (1) and WhenReading (5) conditions |
src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.ComplexType.cs |
Added comprehensive test with 6 properties covering all JsonIgnoreCondition values and verifying correct validation behavior |
src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/snapshots/ValidationsGeneratorTests.ValidatesPropertiesWithJsonIgnoreWhenWritingConditions#ValidatableInfoResolver.g.verified.cs |
Generated snapshot file showing only write-condition properties are included for validation |
| /// Checks if the property is marked with [JsonIgnore] attribute. | ||
| /// Checks if the property is marked with [JsonIgnore] attribute with a condition that affects deserialization. | ||
| /// Only skips validation when the condition is Always or WhenReading, as these affect the reading/deserialization process. | ||
| /// Properties with conditions that only affect writing (WhenWritingDefault, WhenWritingNull, WhenWriting) or Never are still validated. |
There was a problem hiding this comment.
The documentation mentions "WhenWriting" as a JsonIgnoreCondition value, but this does not exist in the System.Text.Json.Serialization.JsonIgnoreCondition enum. The actual enum values are: Never, Always, WhenWritingDefault, WhenWritingNull, and WhenReading. Remove the reference to "WhenWriting" from the documentation.
| /// Properties with conditions that only affect writing (WhenWritingDefault, WhenWritingNull, WhenWriting) or Never are still validated. | |
| /// Properties with conditions that only affect writing (WhenWritingDefault, WhenWritingNull) or Never are still validated. |
Fix JsonIgnore validation bypass for write-only conditions
Validation attributes on properties with
JsonIgnorewrite-only conditions were incorrectly skipped during model binding.Description
The validation generator was treating all
[JsonIgnore]attributes uniformly, skipping validation regardless of theConditionproperty. This caused properties with write-only conditions (e.g.,WhenWritingDefault,WhenWritingNull) to bypass validation on inbound requests.Changes:
IsJsonIgnoredPropertyto inspectJsonIgnore.Conditionand only skip validation for conditions affecting deserialization (Always,WhenReading)Never,WhenWritingDefault,WhenWritingNull,WhenWriting) are now validated correctlyJsonIgnoreConditionvaluesExample:
AlwaysWhenReadingNever,WhenWriting*Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.