C#: Improve ASP.NET Core MVC controller discovery by JarLob · Pull Request #22265 · github/codeql

JarLob · 2026-07-31T14:53:27Z

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
*Controller classes as remote-input sources and did not fully account for
application parts, endpoint mappings, or inherited action methods.

This change:

Separates broad controller helper ownership from runtime controller discovery,
preserving redirect and response-helper sink coverage.
Requires controllers to be public, concrete, top-level, and closed classes.
Recognizes controller identity through the Controller suffix,
ControllerBase, or an inherited [Controller] attribute.
Honors inherited [NonController] exclusions.
Restricts suffix-only POCO controllers using MVC registration,
application-part, and endpoint-mapping evidence.
Models application parts from the application assembly,
[ApplicationPart], and explicit AddApplicationPart calls.
Distinguishes attribute routing from conventional controller routing.
Includes inherited action methods while excluding non-public, static,
abstract, generic, [NonAction], object override, and
IDisposable.Dispose methods.
Excludes explicitly service-injected action parameters from remote-input
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:

Suffix-only POCO controllers require visible MVC registration,
application-part, and endpoint-mapping evidence. They may be missed when this
configuration is in unextracted code or external framework helpers.
Application-part modeling covers the application assembly, generated
[ApplicationPart] attributes, and direct
AddApplicationPart(typeof(T).Assembly) calls. Custom feature providers,
arbitrary ConfigureApplicationPartManager mutations, reflection, and
dynamically loaded plugin assemblies are not modeled.
Endpoint modeling recognizes the standard ASP.NET Core attribute and
conventional routing APIs. Custom routing mechanisms are only recognized
when their implementation contains an extracted call to one of those APIs.
Classes deriving from ControllerBase or carrying an inherited
[Controller] attribute retain a conservative fallback and do not require
visible registration or routing evidence.
Service-parameter exclusion covers explicit IFromServiceMetadata and
[FromKeyedServices] attributes, including derived attributes. Runtime
inference of unannotated service parameters from the DI container is not
modeled.

Copilot

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

Copilot

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 through MapFallbackToController and MapFallbackToAreaController. 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

Copilot

Review details

Files reviewed: 35/35 changed files
Comments generated: 0 new
Review effort level: Balanced

michaelnebel

Wow, that is a very impressive improvement!
Will trigger a DCA run.

michaelnebel · 2026-08-14T09:30:55Z

+}
+
+private predicate isMicrosoftAspNetCoreMvcApplicationPart(Compilation application, Assembly part) {
+  part = application.getOutputAssembly()


This makes a relation between compilation and assemblies in general (which is a bit confusing considering the name of the predicate).
Perhaps consider moving isMicrosoftAspNetCoreMvcApplication(application) into this predicate. Something like

isMicrosoftAspNetCoreMvcApplication(application) and ( part = application.getOutputAssembly() or exists(AssemblyAttribute attr, StringLiteral assemblyName | ...

michaelnebel · 2026-08-14T09:31:30Z

+}
+
+private predicate isInMicrosoftAspNetCoreMvcApplication(Class controller, Compilation application) {
+  isMicrosoftAspNetCoreMvcApplication(application) and


Can be removed, if pushed into the isMicrosoftAspNetCoreMvcApplicationPart predicate.

michaelnebel · 2026-08-14T09:44:31Z

+    not result.getUnboundDeclaration() instanceof UnboundGenericMethod and
+    not result.getOverridee*().getAnAttribute() instanceof MicrosoftAspNetCoreMvcNonActionAttribute and
+    not result.getOverridee*().getDeclaringType() instanceof ObjectType and
+    not result instanceof DisposeMethod


Why is that needed?

michaelnebel · 2026-08-14T09:50:12Z

+  exists(MethodCall addPart, PropertyAccess assemblyAccess, TypeofExpr typeOf, Type partType |
+    application.getAFileCompiled() = addPart.getFile() and
+    isMicrosoftAspNetCoreMvcAddApplicationPart(addPart) and
+    assemblyAccess = addPart.getArgumentForName("assembly") and
+    assemblyAccess.getTarget().hasName("Assembly") and
+    assemblyAccess.getQualifier() = typeOf and
+    partType = typeOf.getTypeAccess().getTarget() and
+    part = getAnAssemblyFor(partType)
+  )


Nit suggestion.

Suggested change

exists(MethodCall addPart, PropertyAccess assemblyAccess, TypeofExpr typeOf, Type partType |

application.getAFileCompiled() = addPart.getFile() and

isMicrosoftAspNetCoreMvcAddApplicationPart(addPart) and

assemblyAccess = addPart.getArgumentForName("assembly") and

assemblyAccess.getTarget().hasName("Assembly") and

assemblyAccess.getQualifier() = typeOf and

partType = typeOf.getTypeAccess().getTarget() and

part = getAnAssemblyFor(partType)

)

exists(MethodCall addPart, PropertyAccess assemblyAccess, Type partType |

application.getAFileCompiled() = addPart.getFile() and

isMicrosoftAspNetCoreMvcAddApplicationPart(addPart) and

assemblyAccess = addPart.getArgumentForName("assembly") and

assemblyAccess.getTarget().hasName("Assembly") and

assemblyAccess.getQualifier().(TypeofExpr).getTypeAccess().getTarget() = partType and

part = getAnAssemblyFor(partType)

)

JarLob added 7 commits July 31, 2026 17:24

C#: Separate ASP.NET Core controller helper methods

5b8d0ff

C#: Align ASP.NET Core controller discovery

1f7d000

C#: Exclude injected MVC services from remote input

9921b89

C#: Scope convention-based controllers to MVC apps

526f438

C#: Account for MVC endpoint mappings

93dac2c

C#: Reduce ASP.NET Core action lookup cost

17d4936

C#: Document ASP.NET Core controller discovery

1d59e69

Copilot AI review requested due to automatic review settings July 31, 2026 14:53

JarLob requested a review from a team as a code owner July 31, 2026 14:53

github-actions Bot added C# documentation labels Jul 31, 2026

Copilot started reviewing on behalf of JarLob July 31, 2026 14:54 View session

github-advanced-security AI found potential problems Jul 31, 2026

View reviewed changes

Comment thread csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll Fixed

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Comment thread csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll Outdated

Address pull request comments

4cdb612

JarLob requested a review from Copilot July 31, 2026 15:13

Copilot started reviewing on behalf of JarLob July 31, 2026 15:13 View session

github-advanced-security AI found potential problems Jul 31, 2026

View reviewed changes

Comment thread csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll Fixed

Copilot AI reviewed Jul 31, 2026

View reviewed changes

JarLob added 2 commits July 31, 2026 18:20

Use don't care expression

dac7892

C#: Model MVC fallback controller mappings

731af15

JarLob requested a review from Copilot July 31, 2026 15:53

Copilot started reviewing on behalf of JarLob July 31, 2026 15:54 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

C#: Fix controller discovery test fixtures

c38a11e

michaelnebel reviewed Aug 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

C#: Improve ASP.NET Core MVC controller discovery - #22265

C#: Improve ASP.NET Core MVC controller discovery#22265
JarLob wants to merge 11 commits into
github:mainfrom
JarLob:aspnetcore-controller-discovery

JarLob commented Jul 31, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

michaelnebel left a comment

Uh oh!

michaelnebel Aug 14, 2026

Uh oh!

michaelnebel Aug 14, 2026

Uh oh!

michaelnebel Aug 14, 2026

Uh oh!

michaelnebel Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

JarLob commented Jul 31, 2026

Summary

Limitations

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

michaelnebel left a comment

Choose a reason for hiding this comment

Uh oh!

michaelnebel Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

michaelnebel Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

michaelnebel Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

michaelnebel Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants