GH-4238: DataAnnotations validation must not trip the user's ServiceLocationPolicy - #4244
Merged
Merged
Conversation
…ocationPolicy The DataAnnotations validation middleware could not be used together with ServiceLocationPolicy.NotAllowed, which is the Wolverine 6 default. The application threw InvalidServiceLocationException at bootstrap and could not start at all. This is fallout from GH-4171. That change deliberately removed IServiceProvider from the derived HttpContext variables, so it would stop being answered silently out of httpContext.RequestServices no matter what ServiceProviderSource said, and would instead go through the normal service-variable machinery where it is reported to ServiceLocationPolicy. That is right for user code. But DataAnnotationsHttpValidationExecutor.Validate takes an IServiceProvider so it can build the ValidationContext, so Wolverine's own middleware started being reported as a service location in the user's chain. The policy now supplies that argument itself rather than leaving the codegen to source it, bound to httpContext.RequestServices. RequestServices is the right provider here regardless of ServiceProviderSource. The only thing the ValidationContext's provider ever does is answer GetService for a ValidationAttribute or an IValidatableObject; it runs against the inbound request before any handler scope is relevant; and it is the same provider ASP.NET Core's own model validation hands to a ValidationContext. Routing it through IWolverineRuntime, as the issue suggested, would add an indirection to arrive in the same place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 2, 2026
jeremydmiller
added a commit
that referenced
this pull request
Sep 3, 2026
) * GH-4238: fix DataAnnotations validation on message handlers too #4244 fixed this for HTTP chains and stopped there. A message handler had the identical defect and kept it, and six tests in Wolverine.DataAnnotationsValidation.Tests had been failing on main to say so -- unseen, because the extension test projects run in no CI workflow. Same shape as the HTTP half: Validate<T> takes an IServiceProvider to build its ValidationContext, an unsupplied one is sourced from the service container and reported to ServiceLocationPolicy, and under the Wolverine 6 default of NotAllowed that made Wolverine's own validation middleware unusable on a message handler at all -- InvalidServiceLocationException at bootstrap. The policy supplies context.Runtime.Services itself, for the same reasons the HTTP twin supplies httpContext.RequestServices: the only thing the ValidationContext's provider ever does is answer GetService for a ValidationAttribute or an IValidatableObject, and it runs before any handler scope is relevant. Naming "context" directly is safe -- it is the generated HandleAsync parameter, and ContextVariable.OverrideName is a deliberate no-op so any other frame wanting that name is renamed instead. Wolverine.DataAnnotationsValidation.Tests: 12 passed, from 6 passed 6 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Give the extension test projects a CI lane None of them ran anywhere. TestExtensions gathers five, but only the Test and Full targets depend on it and no workflow invokes either -- dotnet.yml runs `ci` (CoreTests + CIMessageRouting) and tests.yml runs its own CI* matrix. Wolverine.Protobuf.Tests is worse: it is in wolverine.slnx so it compiles on every build, but it runs in no Nuke target at all, not even TestExtensions. VerifyCITargetCoverage did not report this and is not wrong to have missed it: it audits targets whose names begin with CI, which is the convention for "this is a lane". AiTests and its siblings never claimed to be one. Giving them a lane is the fix, and the guard covers them from here on -- it now walks 43 CI* targets and reports every one reachable. What it cost, found the moment the lane first ran: six failing tests in Wolverine.DataAnnotationsValidation.Tests, fixed in the previous commit, and Wolverine.AI.Tests red on main with the first WolverineFx.AI release in flight (#4257). Note that the AI failure surfaces here as "1 passed on retry" rather than as a failure, because the flaky-retry harness absorbs it -- the lane alone would not have made that one obvious. No docker services: none of these projects reference a persistence package or touch Servers, which is what keeps the lane cheap. CIExtensions locally: AI 40, DataAnnotations 12, FluentValidation 24, MemoryPack 3, MessagePack 5, Protobuf 1 -- all green, 27 seconds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4238.
The DataAnnotations validation middleware could not be used together with
ServiceLocationPolicy.NotAllowed-- the Wolverine 6 default. The application threwInvalidServiceLocationExceptionat bootstrap and could not start at all:Why
This is fallout from GH-4171, already on main. That change deliberately removed
IServiceProviderfrom the derived
HttpContextvariables so it would stop being answered silently out ofhttpContext.RequestServicesno matter whatServiceProviderSourcesaid, and would instead gothrough the normal service-variable machinery -- where it is reported to
ServiceLocationPolicy.Right for user code. But
DataAnnotationsHttpValidationExecutor.Validatetakes anIServiceProviderso it can build theValidationContext, so Wolverine's own middleware beganbeing reported as a service location inside the user's chain.
The fix
The policy supplies that argument itself via
TrySetArgumentrather than leaving the codegen tosource it, bound to
httpContext.RequestServices.RequestServicesis the right provider here regardless ofServiceProviderSource. The only thingthe
ValidationContext's provider ever does is answerGetServicefor aValidationAttributeor anIValidatableObject; it runs against the inbound request before any handler scope is relevant; andit is the same provider ASP.NET Core's own model validation hands to a
ValidationContext. Routingit through
IWolverineRuntime, as the issue suggested, would add an indirection to arrive in thesame place.
Tests
Bug_4238_dataannotations_with_service_location_not_allowedbootstraps an endpoint carryingDataAnnotations under
NotAllowedand exercises both the invalid (400) and valid paths. Before thefix it threw at bootstrap, so neither was reachable. A second test pins
AlwaysAllowed, which wasnever broken and must stay unbroken.
Full
Wolverine.Http.Tests: 1017 passed, 10 skipped. The one failure in that run,use_cascaded_messages_with_http, reproduces on clean main without this change -- and a differentmethod in that same class fails from run to run, so it is a pre-existing tracked-session bleed in
that class rather than anything here.
🤖 Generated with Claude Code