[BuildCheck Suggestion]: Flag Process.Start usages

[JanKrivanek]

Summary

Process.Start from within custom task should be flagged and usage of Exec or ToolTask tasks should be suggested.

On Hold (!)

Let's not action upon this yet. Let's first have a sample demonstrative case that'd be breaking the MSBuild server (plus see if it actually isn't already broken with the curent long lived nodes) and only then decide whether we want to restrict it.

Then we might possibly want to resort to compiler analyzer (or banned API) if runtime checks proves complicted

Background and Motivation

Process.Start creates child processes that cannot be easily controlled by MSBuild engine (as opposed to using Exec or ToolTask tasks). This can e.g. lead to issues during MSBuild server adoption (issue with redirecting outputs)

Notes

Possible ways to detect the Process.Start (need to be investigated if doable):

• Injecting reroute function for Process.Start (akin unittest mocking)
• .net profiling API
• Monitor for child process creation (akin child process debugging)

[KalleOlaviNiemitalo]

• Scanning ECMA-355 metadata tables to detect if the task assembly references the Process.Start method
  • Con: could cause a false warning if the reference is in a method that is not called at run time.
• DiagnosticSource notification from Process.Start
  • Con: needs changes in .NET Runtime.
• Operating system features, e.g. JOB_OBJECT_MSG_NEW_PROCESS on Windows
  • Con: needs separate implementations for Windows, Linux, and macOS.
  • Con: needs extra logic to not warn about child processes started by a process that was started by Exec or ToolTask.

[baronfel]

IMO v1 of this would be to rely on the BannedAPIAnalyzer, right? That seems like the lowest effort-to-reward ratio.

[rainersigwald]

I don't understand what the concern is with Process.Start -- can you elaborate a bit more?

Detecting this at runtime seems basically impossible.