C#: Improve ASP.NET Core MVC controller discovery - #22265
Open
JarLob wants to merge 11 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Improves ASP.NET Core MVC controller/action discovery while preserving response-helper sink coverage.
Changes:
- Adds registration, application-part, routing, controller, and action discovery rules.
- Excludes service-injected parameters from remote-input modeling.
- Adds library and multi-project integration coverage.
Show a summary per file
| File | Description |
|---|---|
csharp/ql/test/library-tests/frameworks/microsoft/AspNetCoreActions.ql |
Queries discovered controller actions. |
csharp/ql/test/library-tests/frameworks/microsoft/AspNetCoreActions.expected |
Records expected actions. |
csharp/ql/test/library-tests/frameworks/microsoft/AspNetCore.expected |
Updates expected controllers. |
csharp/ql/test/library-tests/frameworks/microsoft/AspNetCore.cs |
Adds controller and action cases. |
csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected |
Updates expected remote sources. |
csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs |
Tests service-injected parameters. |
csharp/ql/lib/semmle/code/csharp/security/dataflow/UrlRedirectQuery.qll |
Uses broad response-helper ownership. |
csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll |
Excludes service-injected action parameters. |
csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll |
Implements refined discovery. |
csharp/ql/lib/change-notes/2026-07-31-aspnet-core-controller-discovery.md |
Documents the analysis improvement. |
.../WebApp/WebPocoController.cs |
Adds an application controller. |
.../WebApp/WebApp.csproj |
Defines the main web project. |
.../WebApp/Program.cs |
Registers MVC, application parts, and routing. |
.../WebApp/ApplicationParts.cs |
Declares a generated application part. |
.../Unrelated/Unrelated.csproj |
Defines a non-web project. |
.../Unrelated/ThrottlingController.cs |
Tests unrelated suffix exclusion. |
.../UnmappedWebApp/UnmappedWebApp.csproj |
Defines an unmapped web project. |
.../UnmappedWebApp/UnmappedPocoController.cs |
Tests endpoint-evidence requirements. |
.../UnmappedWebApp/StructuralController.cs |
Tests structural fallback discovery. |
.../UnmappedWebApp/Program.cs |
Registers MVC without endpoint mapping. |
.../aspnetcore_controller_discovery/test.py |
Builds all integration fixtures. |
.../IncludedControllers/IncludedControllers.csproj |
Defines an explicitly included part. |
.../IncludedControllers/IncludedController.cs |
Adds the explicitly included controller. |
.../aspnetcore_controller_discovery/global.json |
Pins the .NET SDK. |
.../GeneratedControllers/GeneratedControllers.csproj |
Defines an attributed application part. |
.../GeneratedControllers/GeneratedController.cs |
Adds the attributed controller. |
.../aspnetcore_controller_discovery/Controllers.ql |
Queries discovered controllers. |
.../aspnetcore_controller_discovery/Controllers.expected |
Records integration expectations. |
.../AttributeWebApp/Program.cs |
Configures attribute routing. |
.../AttributeWebApp/ConventionOnlyController.cs |
Tests convention-only exclusion. |
.../AttributeWebApp/AttributeWebApp.csproj |
Defines the attribute-routed app. |
.../AttributeWebApp/AttributePocoController.cs |
Tests attribute-routed POCO discovery. |
Review details
- Files reviewed: 32/32 changed files
- Comments generated: 1
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (1)
csharp/ql/integration-tests/all-platforms/aspnetcore_controller_discovery/WebApp/Program.cs:11
- The advertised support for standard conventional controller endpoints is incomplete: this test only covers
MapControllerRoute, while ASP.NET Core also routes to MVC actions throughMapFallbackToControllerandMapFallbackToAreaController. A suffix-only POCO exposed exclusively by either fallback API is therefore omitted from discovery. Please recognize those endpoint methods (ideally using their controller argument to avoid broadening every controller) and add an integration scenario for them.
app.MapControllerRoute("default", "{controller}/{action}");
- Files reviewed: 32/32 changed files
- Comments generated: 0 new
- Review effort level: Balanced
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.
Summary
Improve ASP.NET Core MVC controller and action discovery so that it more closely
matches the framework's runtime behavior.
Previously, the ASP.NET Core controller abstraction combined broad heuristics
needed for controller response helpers with the stricter concept of a
runtime-discovered MVC controller. This could classify unrelated
*Controllerclasses as remote-input sources and did not fully account forapplication parts, endpoint mappings, or inherited action methods.
This change:
preserving redirect and response-helper sink coverage.
Controllersuffix,ControllerBase, or an inherited[Controller]attribute.[NonController]exclusions.application-part, and endpoint-mapping evidence.
[ApplicationPart], and explicitAddApplicationPartcalls.abstract, generic,
[NonAction],objectoverride, andIDisposable.Disposemethods.modeling because they are resolved from application services rather than
supplied by the HTTP client.
Classic ASP.NET MVC and Web API controller modeling is unchanged.
Limitations
This remains a static approximation of ASP.NET Core MVC discovery:
application-part, and endpoint-mapping evidence. They may be missed when this
configuration is in unextracted code or external framework helpers.
[ApplicationPart]attributes, and directAddApplicationPart(typeof(T).Assembly)calls. Custom feature providers,arbitrary
ConfigureApplicationPartManagermutations, reflection, anddynamically loaded plugin assemblies are not modeled.
conventional routing APIs. Custom routing mechanisms are only recognized
when their implementation contains an extracted call to one of those APIs.
ControllerBaseor carrying an inherited[Controller]attribute retain a conservative fallback and do not requirevisible registration or routing evidence.
IFromServiceMetadataand[FromKeyedServices]attributes, including derived attributes. Runtimeinference of unannotated service parameters from the DI container is not
modeled.