Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
michaelnebel wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3ac0f78
C#: Remove the explicit feed timeout fallback.
michaelnebel Aug 13, 2026
79c272e
C#: Remove the timeout bool logic in the feed manager.
michaelnebel Aug 13, 2026
f6c6322
C#: Update integration tests.
michaelnebel Aug 13, 2026
3067101
C#: Use ArgumentList instead of Arguments for ProcessStartInfo when i…
michaelnebel Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 19 additions & 22 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private void Info()
// Allow up to four attempts (with up to three retries) to run `dotnet --info`, to mitigate transient issues
for (int attempt = 0; attempt < 4; attempt++)
{
var exitCode = dotnetCliInvoker.RunCommandExitCode("--info", silent: false);
var exitCode = dotnetCliInvoker.RunCommandExitCode(["--info"], silent: false);
switch (exitCode)
{
case 0:
Expand All @@ -63,9 +63,9 @@ private void Info()
}
}

private string GetRestoreArgs(RestoreSettings restoreSettings)
private List<string> GetRestoreArgs(RestoreSettings restoreSettings)
{
var args = $"restore --no-dependencies \"{restoreSettings.File}\" --packages \"{restoreSettings.PackageDirectory}\" /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal";
List<string> args = ["restore", "--no-dependencies", restoreSettings.File, "--packages", restoreSettings.PackageDirectory, "/p:DisableImplicitNuGetFallbackFolder=true", "--verbosity", "normal"];

if (restoreSettings.ForceDotnetRefAssemblyFetching)
{
Expand All @@ -77,23 +77,20 @@ private string GetRestoreArgs(RestoreSettings restoreSettings)
Directory.CreateDirectory(path);
}

args += $" /p:TargetFrameworkRootPath=\"{path}\" /p:NetCoreTargetingPackRoot=\"{path}\" /p:AllowMissingPrunePackageData=true";
args.AddRange([$"/p:TargetFrameworkRootPath={path}", $"/p:NetCoreTargetingPackRoot={path}", "/p:AllowMissingPrunePackageData=true"]);
}

if (restoreSettings.ForceReevaluation)
{
args += " --force";
args.Add("--force");
}

if (restoreSettings.TargetWindows)
{
args += " /p:EnableWindowsTargeting=true";
args.Add("/p:EnableWindowsTargeting=true");
}

if (restoreSettings.NugetSources is not null)
{
args += $" {restoreSettings.NugetSources}";
}
args.AddRange(restoreSettings.NugetSources);

return args;
}
Expand All @@ -107,48 +104,48 @@ public RestoreResult Restore(RestoreSettings restoreSettings)

public bool New(string folder)
{
var args = $"new console --no-restore --output \"{folder}\"";
List<string> args = ["new", "console", "--no-restore", "--output", folder];
return dotnetCliInvoker.RunCommand(args);
}

public bool AddPackage(string folder, string package)
{
var args = $"add \"{folder}\" package \"{package}\" --no-restore";
List<string> args = ["add", folder, "package", package, "--no-restore"];
return dotnetCliInvoker.RunCommand(args);
}

public IList<string> GetListedRuntimes() => GetResultList("--list-runtimes");
public IList<string> GetListedRuntimes() => GetResultList(["--list-runtimes"]);

public IList<string> GetListedSdks() => GetResultList("--list-sdks");
public IList<string> GetListedSdks() => GetResultList(["--list-sdks"]);

private IList<string> GetResultList(string args, string? workingDirectory = null, bool silent = true)
private IList<string> GetResultList(List<string> args, string? workingDirectory = null, bool silent = true)
{
if (dotnetCliInvoker.RunCommand(args, workingDirectory, out var results, silent))
{
return results;
}
logger.LogWarning($"Running 'dotnet {args}' failed.");
return [];
logger.LogWarning($"Running 'dotnet {string.Join(" ", args)}' failed.");
return new List<string>();
}

public bool Exec(string execArgs)
public bool Exec(List<string> execArgs)
{
var args = $"exec {execArgs}";
List<string> args = ["exec", .. execArgs];
return dotnetCliInvoker.RunCommand(args);
}

private const string nugetListSourceCommand = "nuget list source --format Short";
private static readonly IReadOnlyList<string> nugetListSourceCommandArgs = ["nuget", "list", "source", "--format", "Short"];

public IList<string> GetNugetFeeds(string nugetConfig)
{
logger.LogInfo($"Getting NuGet feeds from '{nugetConfig}'...");
return GetResultList($"{nugetListSourceCommand} --configfile \"{nugetConfig}\"");
return GetResultList([.. nugetListSourceCommandArgs, "--configfile", nugetConfig]);
}

public IList<string> GetNugetFeedsFromFolder(string folderPath)
{
logger.LogInfo($"Getting NuGet feeds in folder '{folderPath}'...");
return GetResultList(nugetListSourceCommand, folderPath);
return GetResultList(nugetListSourceCommandArgs.ToList(), folderPath);
}

// The version number should be kept in sync with the version .NET version used for building the application.
Expand Down
20 changes: 10 additions & 10 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNetCliInvoker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using Semmle.Util;
using Semmle.Util.Logging;
Expand All @@ -25,14 +24,15 @@ public DotNetCliInvoker(ILogger logger, string exec, DependabotProxy? dependabot
logger.LogInfo($"Using .NET CLI executable: '{Exec}'");
}

private ProcessStartInfo MakeDotnetStartInfo(string args, string? workingDirectory)
private ProcessStartInfo MakeDotnetStartInfo(List<string> args, string? workingDirectory)
{
var startInfo = new ProcessStartInfo(Exec, args)
{
UseShellExecute = false,
RedirectStandardOutput = true,
RedirectStandardError = true
};

if (!string.IsNullOrWhiteSpace(workingDirectory))
{
startInfo.WorkingDirectory = workingDirectory;
Expand All @@ -57,39 +57,39 @@ private ProcessStartInfo MakeDotnetStartInfo(string args, string? workingDirecto
return startInfo;
}

private int RunCommandExitCodeAux(string args, string? workingDirectory, out IList<string> output, out string dirLog, bool silent)
private int RunCommandExitCodeAux(List<string> args, string? workingDirectory, out IList<string> output, out string dirLog, bool silent)
{
dirLog = string.IsNullOrWhiteSpace(workingDirectory) ? "" : $" in {workingDirectory}";
var pi = MakeDotnetStartInfo(args, workingDirectory);
var threadId = Environment.CurrentManagedThreadId;
void onOut(string s) => logger.Log(silent ? Severity.Debug : Severity.Info, s, threadId);
void onError(string s) => logger.LogError(s, threadId);
logger.LogInfo($"Running '{Exec} {args}'{dirLog}");
logger.LogInfo($"Running '{Exec} {string.Join(" ", args)}'{dirLog}");
var exitCode = pi.ReadOutput(out output, onOut, onError);
return exitCode;
}

private bool RunCommandAux(string args, string? workingDirectory, out IList<string> output, bool silent)
private bool RunCommandAux(List<string> args, string? workingDirectory, out IList<string> output, bool silent)
{
var exitCode = RunCommandExitCodeAux(args, workingDirectory, out output, out var dirLog, silent);
if (exitCode != 0)
{
logger.LogError($"Command '{Exec} {args}'{dirLog} failed with exit code {exitCode}");
logger.LogError($"Command '{Exec} {string.Join(" ", args)}'{dirLog} failed with exit code {exitCode}");
return false;
}
return true;
}

public bool RunCommand(string args, bool silent = true) =>
public bool RunCommand(List<string> args, bool silent = true) =>
RunCommandAux(args, null, out _, silent);

public int RunCommandExitCode(string args, bool silent = true) =>
public int RunCommandExitCode(List<string> args, bool silent = true) =>
RunCommandExitCodeAux(args, null, out _, out _, silent);

public bool RunCommand(string args, out IList<string> output, bool silent = true) =>
public bool RunCommand(List<string> args, out IList<string> output, bool silent = true) =>
RunCommandAux(args, null, out output, silent);

public bool RunCommand(string args, string? workingDirectory, out IList<string> output, bool silent = true) =>
public bool RunCommand(List<string> args, string? workingDirectory, out IList<string> output, bool silent = true) =>
RunCommandAux(args, workingDirectory, out output, silent);
}
}
74 changes: 21 additions & 53 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,12 @@ internal sealed partial class FeedManager : IDisposable
/// </summary>
public ImmutableHashSet<string> InheritedFeeds => AllFeeds.Except(ExplicitFeeds).ToImmutableHashSet();

private readonly Lazy<(bool, ImmutableHashSet<string>)> lazyReachableExplicitFeeds;

/// <summary>
/// Gets whether there was a timeout when checking the reachability of the explicitly configured NuGet feeds.
/// </summary>
public bool ExplicitFeedTimeout => lazyReachableExplicitFeeds.Value.Item1;
private readonly Lazy<ImmutableHashSet<string>> lazyReachableExplicitFeeds;

/// <summary>
/// Gets the list of reachable NuGet feeds that are explicitly configured.
/// </summary>
public ImmutableHashSet<string> ReachableExplicitFeeds => lazyReachableExplicitFeeds.Value.Item2;
public ImmutableHashSet<string> ReachableExplicitFeeds => lazyReachableExplicitFeeds.Value;

private readonly Lazy<ImmutableHashSet<string>> lazyReachableFeeds;
/// <summary>
Expand All @@ -96,15 +91,11 @@ public FeedManager(ILogger logger, IDotNet dotnet, DependabotProxy? dependabotPr

lazyExplicitFeeds = new Lazy<ImmutableHashSet<string>>(GetExplicitFeeds);
lazyAllFeeds = new Lazy<ImmutableHashSet<string>>(GetAllFeeds);
lazyReachableExplicitFeeds = new Lazy<(bool, ImmutableHashSet<string>)>(() =>
{
var timeout = CheckSpecifiedFeeds(ExplicitFeeds, out var reachableFeeds);
return (timeout, reachableFeeds);
});
lazyReachableExplicitFeeds = new Lazy<ImmutableHashSet<string>>(() => CheckSpecifiedFeeds(ExplicitFeeds));
lazyReachableFeeds = new Lazy<ImmutableHashSet<string>>(() =>
{
// Inherited feeds should only be used, if they are indeed reachable (as they may be environment specific).
CheckSpecifiedFeeds(InheritedFeeds, out var reachableInheritedFeeds);
var reachableInheritedFeeds = CheckSpecifiedFeeds(InheritedFeeds);
return ReachableExplicitFeeds.Union(reachableInheritedFeeds).ToImmutableHashSet();
});
lazyReachableFallbackFeeds = new Lazy<ImmutableHashSet<string>>(() =>
Expand Down Expand Up @@ -167,24 +158,18 @@ private IEnumerable<string> GetFeedsFromNugetConfig(string nugetConfigPath) =>
/// </summary>
/// <param name="feeds">The list of feeds to use for the restore command.</param>
/// <param name="sourceArgumentPrefix">The prefix to use for each source argument (e.g., "-s").</param>
/// <returns>The constructed NuGet sources argument for the restore command.</returns>
public string FeedsToRestoreArgument(IEnumerable<string> feeds, string sourceArgumentPrefix)
/// <returns>The list of NuGet sources arguments for the restore command.</returns>
public List<string> FeedsToRestoreArgument(IEnumerable<string> feeds, string sourceArgumentPrefix)
{
// If there are no feeds, we want to override any default feeds that `restore` would use by passing a dummy source argument.
if (!feeds.Any())
{
return $" {sourceArgumentPrefix} \"{emptyPackageDirectory.DirInfo.FullName}\"";
return [sourceArgumentPrefix, emptyPackageDirectory.DirInfo.FullName];
}

// Add package sources. If any are present, they override all sources specified in
// the configuration file(s).
var feedArgs = new StringBuilder();
foreach (var feed in feeds)
{
feedArgs.Append($" {sourceArgumentPrefix} \"{feed}\"");
}

return feedArgs.ToString();
return feeds.SelectMany<string, string>(feed => [sourceArgumentPrefix, feed]).ToList();
}

private IEnumerable<string> FeedsToUseAux(HashSet<string> feedsToConsider)
Expand Down Expand Up @@ -221,8 +206,8 @@ public IEnumerable<string> FeedsToUse(string path)
/// Constructs the NuGet sources argument for the `dotnet restore` command based on the given feeds.
/// </summary>
/// <param name="feeds">The list of NuGet feeds to use for the restore command.</param>
/// <returns>A string representing the NuGet sources argument for the `dotnet restore` command.</returns>
public string FeedsToDotnetRestoreArgument(IEnumerable<string> feeds)
/// <returns>A list representing the NuGet sources arguments for the `dotnet restore` command.</returns>
public List<string> FeedsToDotnetRestoreArgument(IEnumerable<string> feeds)
{
return FeedsToRestoreArgument(feeds, "-s");
}
Expand All @@ -234,12 +219,12 @@ public string FeedsToDotnetRestoreArgument(IEnumerable<string> feeds)
/// </summary>
/// <param name="path">Path to project/solution</param>
/// <returns>A string representing the NuGet sources argument for the `dotnet restore` command.</returns>
public string? MakeDotnetRestoreSourcesArgument(string path)
public List<string> MakeDotnetRestoreSourcesArguments(string path)
{
// Do not construct a set of explicit NuGet sources to use for restore.
if (!CheckNugetFeedResponsiveness && !HasPrivateRegistryFeeds)
{
return null;
return [];
}

var feedsToUse = FeedsToUse(path);
Expand Down Expand Up @@ -271,7 +256,7 @@ private static async Task<HttpResponseMessage> ExecuteGetRequest(string address,
return await httpClient.GetAsync(address, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
}

private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount, out bool isTimeout)
private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount)
{
logger.LogInfo($"Checking if NuGet feed '{feed}' is reachable...");

Expand Down Expand Up @@ -304,8 +289,6 @@ private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount,

using HttpClient client = new(httpClientHandler);

isTimeout = false;

for (var i = 0; i < tryCount; i++)
{
using var cts = new CancellationTokenSource();
Expand Down Expand Up @@ -335,7 +318,6 @@ private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount,
}

logger.LogWarning($"Didn't receive answer from NuGet feed '{feed}'. Tried it {tryCount} times.");
isTimeout = true;
return false;
}

Expand All @@ -359,12 +341,8 @@ private HashSet<string> GetExcludedFeeds()
/// Checks that we can connect to the specified NuGet feeds.
/// </summary>
/// <param name="feeds">The set of package feeds to check.</param>
/// <param name="reachableFeeds">The list of feeds that were reachable.</param>
/// <returns>
/// True if there is a timeout when trying to reach the feeds (excluding any feeds that are configured
/// to be excluded from the check) or false otherwise.
/// </returns>
private bool CheckSpecifiedFeeds(ImmutableHashSet<string> feeds, out ImmutableHashSet<string> reachableFeeds)
/// <returns>The list of feeds that were reachable.</returns>
private ImmutableHashSet<string> CheckSpecifiedFeeds(ImmutableHashSet<string> feeds)
{
// Exclude any feeds from the feed check that are configured by the corresponding environment variable.
// These feeds are always assumed to be reachable.
Expand All @@ -380,12 +358,10 @@ private bool CheckSpecifiedFeeds(ImmutableHashSet<string> feeds, out ImmutableHa
return true;
}).ToHashSet();

var reachable = GetReachableNuGetFeeds(feedsToCheck, isFallback: false, out var isTimeout);
var reachable = GetReachableNuGetFeeds(feedsToCheck, isFallback: false);

// Always consider feeds excluded for the reachability check as reachable.
reachableFeeds = reachable.Union(feeds.Where(feed => excludedFeeds.Contains(feed))).ToImmutableHashSet();

return isTimeout;
return reachable.Union(feeds.Where(feed => excludedFeeds.Contains(feed))).ToImmutableHashSet();
}

/// <summary>
Expand All @@ -398,7 +374,7 @@ public bool IsDefaultFeedReachable()
if (CheckNugetFeedResponsiveness)
{
var (initialTimeout, tryCount) = GetFeedRequestSettings(isFallback: false);
return IsFeedReachable(PublicNugetOrgFeed, initialTimeout, tryCount, out var _);
return IsFeedReachable(PublicNugetOrgFeed, initialTimeout, tryCount);
}

return true;
Expand All @@ -409,22 +385,15 @@ public bool IsDefaultFeedReachable()
/// </summary>
/// <param name="feedsToCheck">The feeds to check.</param>
/// <param name="isFallback">Whether the feeds are fallback feeds or not.</param>
/// <param name="isTimeout">Whether a timeout occurred while checking the feeds.</param>
/// <returns>The list of feeds that could be reached.</returns>
private List<string> GetReachableNuGetFeeds(HashSet<string> feedsToCheck, bool isFallback, out bool isTimeout)
private List<string> GetReachableNuGetFeeds(HashSet<string> feedsToCheck, bool isFallback)
{
var fallbackStr = isFallback ? "fallback " : "";
logger.LogInfo($"Checking {fallbackStr}NuGet feed reachability on feeds: {string.Join(", ", feedsToCheck.OrderBy(f => f))}");

var (initialTimeout, tryCount) = GetFeedRequestSettings(isFallback);
var timeout = false;
var reachableFeeds = feedsToCheck
.Where(feed =>
{
var reachable = IsFeedReachable(feed, initialTimeout, tryCount, out var feedTimeout);
timeout |= feedTimeout;
return reachable;
})
.Where(feed => IsFeedReachable(feed, initialTimeout, tryCount))
.ToList();

if (reachableFeeds.Count == 0)
Expand All @@ -436,7 +405,6 @@ private List<string> GetReachableNuGetFeeds(HashSet<string> feedsToCheck, bool i
logger.LogInfo($"Reachable {fallbackStr}NuGet feeds: {string.Join(", ", reachableFeeds.OrderBy(f => f))}");
}

isTimeout = timeout;
return reachableFeeds;
}

Expand All @@ -460,7 +428,7 @@ private List<string> GetReachableFallbackNugetFeeds()
}
}

return GetReachableNuGetFeeds(fallbackFeeds, isFallback: true, out var _);
return GetReachableNuGetFeeds(fallbackFeeds, isFallback: true);
}

private ImmutableHashSet<string> GetExplicitFeeds()
Expand Down
Loading
Loading