Skip to content

C#: Use ProcessStartInfo built-in for correct escaping/quoting in the Dependency manager. - #22377

Open
michaelnebel wants to merge 1 commit into
github:mainfrom
michaelnebel:csharp/dotnetquoting
Open

C#: Use ProcessStartInfo built-in for correct escaping/quoting in the Dependency manager.#22377
michaelnebel wants to merge 1 commit into
github:mainfrom
michaelnebel:csharp/dotnetquoting

Conversation

@michaelnebel

@michaelnebel michaelnebel commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Instead of escaping/quoting of arguments when we invoke dotnet in the dependency manager, we should rely on the built-in support in ProcessStartInfo. That is, we should supply the arguments to the call via ArgumentList instead of Arguments (documentation can be seen here).

DCA looks good.

Comment thread csharp/extractor/Semmle.Extraction.Tests/DotNet.cs Fixed
@michaelnebel
michaelnebel force-pushed the csharp/dotnetquoting branch 2 times, most recently from 11e1ed7 to 882812b Compare August 21, 2026 09:20
@michaelnebel
michaelnebel requested a balanced review from Copilot August 24, 2026 06:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates dependency-manager process invocation from manually quoted strings to structured argument lists for reliable escaping.

Changes:

  • Refactors .NET and NuGet invocations to use argument lists.
  • Updates feed-source and restore APIs accordingly.
  • Adapts tests and source-generator execution.
Show a summary per file
File Description
Runtime.cs Updates the IDotNet test stub.
DotNet.cs (tests) Adapts command-construction tests.
DotnetSourceGeneratorWrapper.cs Passes compiler arguments separately.
PackagesConfigRestorer.cs Uses structured NuGet arguments.
NugetPackageRestorer.cs Propagates source argument lists.
IDotNetCliInvoker.cs Changes invoker methods to list arguments.
IDotNet.cs Updates public command and restore contracts.
FeedManager.cs Produces structured source arguments.
DotNetCliInvoker.cs Starts processes with argument lists.
DotNet.cs (dependency fetching) Constructs all CLI commands as argument lists.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 10/10 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread csharp/extractor/Semmle.Extraction.Tests/DotNet.cs Outdated
@michaelnebel
michaelnebel force-pushed the csharp/dotnetquoting branch 2 times, most recently from edeaaa0 to 9fd3396 Compare August 24, 2026 12:19
@michaelnebel
michaelnebel requested a balanced review from Copilot August 24, 2026 12:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@michaelnebel
michaelnebel requested a review from hvitved August 25, 2026 07:35
@michaelnebel
michaelnebel marked this pull request as ready for review August 25, 2026 07:35
@michaelnebel
michaelnebel requested a review from a team as a code owner August 25, 2026 07:35
@michaelnebel michaelnebel added the no-change-note-required This PR does not need a change note label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants