From 6b2f348c4c68d7dd77227e4c7837f48f3fd6fe49 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 6 Jan 2025 11:06:58 +0000 Subject: [PATCH 01/21] C#: Add `CODEQL_PROXY_URLS` environment variable --- .../EnvironmentVariableNames.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/EnvironmentVariableNames.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/EnvironmentVariableNames.cs index d825e5daeb03..589e72d21265 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/EnvironmentVariableNames.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/EnvironmentVariableNames.cs @@ -89,5 +89,10 @@ internal static class EnvironmentVariableNames /// Contains the certificate used by the Dependabot proxy. /// public const string ProxyCertificate = "CODEQL_PROXY_CA_CERTIFICATE"; + + /// + /// Contains the URLs of private nuget registries as a JSON array. + /// + public const string ProxyURLs = "CODEQL_PROXY_URLS"; } } From 63d5517d7cf44a3506355ad5cc1622447c69635b Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 7 Jan 2025 11:45:00 +0000 Subject: [PATCH 02/21] C#: Add list of registries to `DependabotProxy` --- .../DependabotProxy.cs | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs index cad7d33f472b..497e74815ab1 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs @@ -1,5 +1,5 @@ using System; -using System.Diagnostics; +using System.Collections.Generic; using System.IO; using System.Security.Cryptography.X509Certificates; using Semmle.Util; @@ -9,6 +9,21 @@ namespace Semmle.Extraction.CSharp.DependencyFetching { public class DependabotProxy : IDisposable { + /// + /// Represents configurations for package registries. + /// + public struct RegistryConfig + { + /// + /// The type of package registry. + /// + public string Type { get; set; } + /// + /// The URL of the package registry. + /// + public string URL { get; set; } + } + private readonly string host; private readonly string port; @@ -17,6 +32,10 @@ public class DependabotProxy : IDisposable /// internal string Address { get; } /// + /// The URLs of package registries that are configured for the proxy. + /// + internal HashSet RegistryURLs { get; } + /// /// The path to the temporary file where the certificate is stored. /// internal string? CertificatePath { get; private set; } @@ -75,6 +94,7 @@ private DependabotProxy(string host, string port) this.host = host; this.port = port; this.Address = $"http://{this.host}:{this.port}"; + this.RegistryURLs = new HashSet(); } public void Dispose() From 11efb55aa1c414535152611b67ddd1ff40c46274 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 7 Jan 2025 15:28:29 +0000 Subject: [PATCH 03/21] C#: Parse environment variables to obtain list of registry URLs --- .../DependabotProxy.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs index 497e74815ab1..c4af0736ef2e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs @@ -4,6 +4,7 @@ using System.Security.Cryptography.X509Certificates; using Semmle.Util; using Semmle.Util.Logging; +using Newtonsoft.Json; namespace Semmle.Extraction.CSharp.DependencyFetching { @@ -86,6 +87,39 @@ public struct RegistryConfig result.Certificate = X509Certificate2.CreateFromPem(cert); } + // Try to obtain the list of private registry URLs. + var registryURLs = Environment.GetEnvironmentVariable(EnvironmentVariableNames.ProxyURLs); + + if (!string.IsNullOrWhiteSpace(registryURLs)) + { + try + { + // The value of the environment variable should be a JSON array of objects, such as: + // [ { "type": "nuget_feed", "url": "https://nuget.pkg.github.com/org/index.json" } ] + var array = JsonConvert.DeserializeObject>(registryURLs); + if (array != null) + { + foreach (RegistryConfig config in array) + { + // The array contains all configured private registries, not just ones for C#. + // We ignore the non-C# ones here. + if (!config.Type.Equals("nuget_feed")) + { + logger.LogDebug($"Ignoring registry at '{config.URL}' since it is not of type 'nuget_feed'."); + continue; + } + + logger.LogInfo($"Found private registry at '{config.URL}'"); + result.RegistryURLs.Add(config.URL); + } + } + } + catch (Exception ex) + { + logger.LogError($"Unable to parse '{EnvironmentVariableNames.ProxyURLs}': {ex.Message}"); + } + } + return result; } From 726123c0cbe69978d54d894d78a5a727abc1d6e3 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 7 Jan 2025 11:41:04 +0000 Subject: [PATCH 04/21] C#: Allow specifying package feeds for `dotnet restore` as command line arguments --- .../DotNet.cs | 10 ++++++++++ .../IDotNet.cs | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs index dfabb7446186..cf9f7f4105db 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs @@ -67,6 +67,16 @@ private string GetRestoreArgs(RestoreSettings restoreSettings) args += $" --configfile \"{restoreSettings.PathToNugetConfig}\""; } + // Add package sources. If any are present, they override all sources specified in + // the configuration file(s). + if (restoreSettings.Sources != null) + { + foreach (string source in restoreSettings.Sources) + { + args += $" -s {source}"; + } + } + if (restoreSettings.ForceReevaluation) { args += " --force"; diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs index 2c10afa80ef2..44bd42167030 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs @@ -17,7 +17,7 @@ public interface IDotNet IList GetNugetFeedsFromFolder(string folderPath); } - public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false); + public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, IList? Sources = null, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false); public partial record class RestoreResult(bool Success, IList Output) { From 0db6a269e4410aa30c21a38ea699827844504a2b Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 24 Feb 2025 13:33:12 +0000 Subject: [PATCH 05/21] C#: Propagate explicit feeds to `RestoreProjects` --- .../NugetPackageRestorer.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 393e37579b71..3141cf6cc041 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -156,7 +156,7 @@ public HashSet Restore() var restoredProjects = RestoreSolutions(out var container); var projects = fileProvider.Projects.Except(restoredProjects); - RestoreProjects(projects, out var containers); + RestoreProjects(projects, explicitFeeds, out var containers); var dependencies = containers.Flatten(container); @@ -260,8 +260,12 @@ private IEnumerable RestoreSolutions(out DependencyContainer dependencie /// Populates dependencies with the relative paths to the assets files generated by the restore. /// /// A list of paths to project files. - private void RestoreProjects(IEnumerable projects, out ConcurrentBag dependencies) + private void RestoreProjects(IEnumerable projects, HashSet? configuredSources, out ConcurrentBag dependencies) { + var sources = configuredSources ?? new(); + sources.Add(PublicNugetOrgFeed); + this.dependabotProxy?.RegistryURLs.ForEach(url => sources.Add(url)); + var successCount = 0; var nugetSourceFailures = 0; ConcurrentBag collectedDependencies = []; @@ -276,7 +280,7 @@ private void RestoreProjects(IEnumerable projects, out ConcurrentBag Date: Mon, 3 Mar 2025 15:12:40 +0000 Subject: [PATCH 06/21] C#: Fix test failures --- csharp/extractor/Semmle.Extraction.Tests/DotNet.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.Tests/DotNet.cs b/csharp/extractor/Semmle.Extraction.Tests/DotNet.cs index c584b607ec8e..904ad04ce82f 100644 --- a/csharp/extractor/Semmle.Extraction.Tests/DotNet.cs +++ b/csharp/extractor/Semmle.Extraction.Tests/DotNet.cs @@ -123,7 +123,7 @@ public void TestDotnetRestoreProjectToDirectory2() var dotnet = MakeDotnet(dotnetCliInvoker); // Execute - var res = dotnet.Restore(new("myproject.csproj", "mypackages", false, "myconfig.config")); + var res = dotnet.Restore(new("myproject.csproj", "mypackages", false, null, "myconfig.config")); // Verify var lastArgs = dotnetCliInvoker.GetLastArgs(); @@ -141,7 +141,7 @@ public void TestDotnetRestoreProjectToDirectory3() var dotnet = MakeDotnet(dotnetCliInvoker); // Execute - var res = dotnet.Restore(new("myproject.csproj", "mypackages", false, "myconfig.config", true)); + var res = dotnet.Restore(new("myproject.csproj", "mypackages", false, null, "myconfig.config", true)); // Verify var lastArgs = dotnetCliInvoker.GetLastArgs(); From a8dde15a878c58d5dbc1f69c7e27035db8107230 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 14 Mar 2025 13:47:05 +0000 Subject: [PATCH 07/21] C#: Only provide feeds on command line if Dependabot proxy is enabled --- .../NugetPackageRestorer.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 3141cf6cc041..c8619030ed23 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -262,9 +262,21 @@ private IEnumerable RestoreSolutions(out DependencyContainer dependencie /// A list of paths to project files. private void RestoreProjects(IEnumerable projects, HashSet? configuredSources, out ConcurrentBag dependencies) { - var sources = configuredSources ?? new(); - sources.Add(PublicNugetOrgFeed); - this.dependabotProxy?.RegistryURLs.ForEach(url => sources.Add(url)); + // Conservatively, we only set this to a non-null value if a Dependabot proxy is enabled. + // This ensures that we continue to get the old behaviour where feeds are taken from + // `nuget.config` files instead of the command-line arguments. + HashSet? sources = null; + + if (this.dependabotProxy != null) + { + // If the Dependabot proxy is configured, then our main goal is to make `dotnet` aware + // of the private registry feeds. However, since providing them as command-line arguments + // to `dotnet` ignores other feeds that may be configured, we also need to add the feeds + // we have discovered from analysing `nuget.config` files. + sources = configuredSources ?? new(); + sources.Add(PublicNugetOrgFeed); + this.dependabotProxy?.RegistryURLs.ForEach(url => sources.Add(url)); + } var successCount = 0; var nugetSourceFailures = 0; From 95605935fa030a431df6b83cfa0408842ec946a4 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 14 Mar 2025 14:02:38 +0000 Subject: [PATCH 08/21] C#: Fix `.ToList()` being called on `null` --- .../NugetPackageRestorer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index c8619030ed23..a4be22fd60bf 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -292,7 +292,7 @@ private void RestoreProjects(IEnumerable projects, HashSet? conf foreach (var project in projectGroup) { logger.LogInfo($"Restoring project {project}..."); - var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, sources.ToList(), TargetWindows: isWindows)); + var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, sources?.ToList(), TargetWindows: isWindows)); assets.AddDependenciesRange(res.AssetsFilePaths); lock (sync) { From b6c74fe306d6ef8204e623af6b4d89a076ff7b2b Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 14 Mar 2025 14:05:27 +0000 Subject: [PATCH 09/21] C#: Narrow `Exception` to `JsonException` --- .../DependabotProxy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs index c4af0736ef2e..9c9b7edb2eee 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs @@ -114,7 +114,7 @@ public struct RegistryConfig } } } - catch (Exception ex) + catch (JsonException ex) { logger.LogError($"Unable to parse '{EnvironmentVariableNames.ProxyURLs}': {ex.Message}"); } From 284f612965ed1bbf115acff94921af4f5f16db96 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 14 Mar 2025 14:06:48 +0000 Subject: [PATCH 10/21] C#: Use `StringBuilder` for feed arguments in `GetRestoreArgs` --- .../Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs index cf9f7f4105db..36c85cba9e20 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs @@ -2,7 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; - +using System.Text; using Newtonsoft.Json.Linq; using Semmle.Util; @@ -71,10 +71,13 @@ private string GetRestoreArgs(RestoreSettings restoreSettings) // the configuration file(s). if (restoreSettings.Sources != null) { + var feedArgs = new StringBuilder(); foreach (string source in restoreSettings.Sources) { - args += $" -s {source}"; + feedArgs.Append($" -s {source}"); } + + args += feedArgs.ToString(); } if (restoreSettings.ForceReevaluation) From 51874b8ef0e38a0015ac0cfde44eaa8aea7c2bb8 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 17 Mar 2025 14:24:04 +0000 Subject: [PATCH 11/21] Apply suggestions from code review Co-authored-by: Michael Nebel --- .../DependabotProxy.cs | 2 +- .../NugetPackageRestorer.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs index 9c9b7edb2eee..cdc204c22c67 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs @@ -97,7 +97,7 @@ public struct RegistryConfig // The value of the environment variable should be a JSON array of objects, such as: // [ { "type": "nuget_feed", "url": "https://nuget.pkg.github.com/org/index.json" } ] var array = JsonConvert.DeserializeObject>(registryURLs); - if (array != null) + if (array is not null) { foreach (RegistryConfig config in array) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index a4be22fd60bf..4f65432a561f 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -267,7 +267,7 @@ private void RestoreProjects(IEnumerable projects, HashSet? conf // `nuget.config` files instead of the command-line arguments. HashSet? sources = null; - if (this.dependabotProxy != null) + if (this.dependabotProxy is not null) { // If the Dependabot proxy is configured, then our main goal is to make `dotnet` aware // of the private registry feeds. However, since providing them as command-line arguments @@ -275,7 +275,7 @@ private void RestoreProjects(IEnumerable projects, HashSet? conf // we have discovered from analysing `nuget.config` files. sources = configuredSources ?? new(); sources.Add(PublicNugetOrgFeed); - this.dependabotProxy?.RegistryURLs.ForEach(url => sources.Add(url)); + this.dependabotProxy.RegistryURLs.ForEach(url => sources.Add(url)); } var successCount = 0; From 7a92a72a9aff72a9d5c45dcd4ae35015a8990d23 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 18 Mar 2025 16:45:41 +0000 Subject: [PATCH 12/21] C#: Change `RegistryConfig` to a record class --- .../DependabotProxy.cs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs index cdc204c22c67..be5f137548c4 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs @@ -13,17 +13,9 @@ public class DependabotProxy : IDisposable /// /// Represents configurations for package registries. /// - public struct RegistryConfig - { - /// - /// The type of package registry. - /// - public string Type { get; set; } - /// - /// The URL of the package registry. - /// - public string URL { get; set; } - } + /// The type of package registry. + /// The URL of the package registry. + public record class RegistryConfig(string Type, string URL); private readonly string host; private readonly string port; From d564529f3cf6fb925419654ae9ae5797df861ce7 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 24 Mar 2025 17:08:05 +0000 Subject: [PATCH 13/21] C#: Change `RestoreSettings` to have general `extraArgs` parameter This allows the string of package feeds to be constructed once and used repeatedly in the parallel restore loop as well. --- .../DotNet.cs | 18 +++++------------- .../IDotNet.cs | 2 +- .../NugetPackageRestorer.cs | 16 +++++++++++++--- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs index 36c85cba9e20..a798048b933d 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs @@ -67,19 +67,6 @@ private string GetRestoreArgs(RestoreSettings restoreSettings) args += $" --configfile \"{restoreSettings.PathToNugetConfig}\""; } - // Add package sources. If any are present, they override all sources specified in - // the configuration file(s). - if (restoreSettings.Sources != null) - { - var feedArgs = new StringBuilder(); - foreach (string source in restoreSettings.Sources) - { - feedArgs.Append($" -s {source}"); - } - - args += feedArgs.ToString(); - } - if (restoreSettings.ForceReevaluation) { args += " --force"; @@ -90,6 +77,11 @@ private string GetRestoreArgs(RestoreSettings restoreSettings) args += " /p:EnableWindowsTargeting=true"; } + if (restoreSettings.ExtraArgs != null) + { + args += $" {restoreSettings.ExtraArgs}"; + } + return args; } diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs index 44bd42167030..eec6a2b8d3b2 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs @@ -17,7 +17,7 @@ public interface IDotNet IList GetNugetFeedsFromFolder(string folderPath); } - public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, IList? Sources = null, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false); + public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? ExtraArgs = null, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false); public partial record class RestoreResult(bool Success, IList Output) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 4f65432a561f..cd9f80b60b33 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -265,7 +265,7 @@ private void RestoreProjects(IEnumerable projects, HashSet? conf // Conservatively, we only set this to a non-null value if a Dependabot proxy is enabled. // This ensures that we continue to get the old behaviour where feeds are taken from // `nuget.config` files instead of the command-line arguments. - HashSet? sources = null; + string? extraArgs = null; if (this.dependabotProxy is not null) { @@ -273,9 +273,19 @@ private void RestoreProjects(IEnumerable projects, HashSet? conf // of the private registry feeds. However, since providing them as command-line arguments // to `dotnet` ignores other feeds that may be configured, we also need to add the feeds // we have discovered from analysing `nuget.config` files. - sources = configuredSources ?? new(); + var sources = configuredSources ?? new(); sources.Add(PublicNugetOrgFeed); this.dependabotProxy.RegistryURLs.ForEach(url => sources.Add(url)); + + // Add package sources. If any are present, they override all sources specified in + // the configuration file(s). + var feedArgs = new StringBuilder(); + foreach (string source in sources) + { + feedArgs.Append($" -s {source}"); + } + + extraArgs = feedArgs.ToString(); } var successCount = 0; @@ -292,7 +302,7 @@ private void RestoreProjects(IEnumerable projects, HashSet? conf foreach (var project in projectGroup) { logger.LogInfo($"Restoring project {project}..."); - var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, sources?.ToList(), TargetWindows: isWindows)); + var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, extraArgs, TargetWindows: isWindows)); assets.AddDependenciesRange(res.AssetsFilePaths); lock (sync) { From 92eab47def3e2a45151020311ebb8acaf0892a79 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 24 Mar 2025 17:15:49 +0000 Subject: [PATCH 14/21] C#: Refactor `CheckFeeds` to have an overloaded variant that accepts a given set of feeds. --- .../NugetPackageRestorer.cs | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index cd9f80b60b33..7d9c548a143c 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -700,11 +700,36 @@ private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount, return (timeoutMilliSeconds, tryCount); } + /// + /// Checks that we can connect to all Nuget feeds that are explicitly configured in configuration files. + /// + /// Outputs the set of explicit feeds. + /// True if all feeds are reachable or false otherwise. private bool CheckFeeds(out HashSet explicitFeeds) { - logger.LogInfo("Checking Nuget feeds..."); (explicitFeeds, var allFeeds) = GetAllFeeds(); + var allFeedsReachable = this.CheckFeeds(explicitFeeds); + + var inheritedFeeds = allFeeds.Except(explicitFeeds).ToHashSet(); + if (inheritedFeeds.Count > 0) + { + logger.LogInfo($"Inherited Nuget feeds (not checked for reachability): {string.Join(", ", inheritedFeeds.OrderBy(f => f))}"); + compilationInfoContainer.CompilationInfos.Add(("Inherited Nuget feed count", inheritedFeeds.Count.ToString())); + } + + return allFeedsReachable; + } + + /// + /// Checks that we can connect to the specified Nuget feeds. + /// + /// The set of package feeds to check. + /// True if all feeds are reachable or false otherwise. + private bool CheckFeeds(HashSet feeds) + { + logger.LogInfo("Checking that Nuget feeds are reachable..."); + var excludedFeeds = EnvironmentVariables.GetURLs(EnvironmentVariableNames.ExcludedNugetFeedsFromResponsivenessCheck) .ToHashSet(); @@ -715,7 +740,7 @@ private bool CheckFeeds(out HashSet explicitFeeds) var (initialTimeout, tryCount) = GetFeedRequestSettings(isFallback: false); - var allFeedsReachable = explicitFeeds.All(feed => excludedFeeds.Contains(feed) || IsFeedReachable(feed, initialTimeout, tryCount)); + var allFeedsReachable = feeds.All(feed => excludedFeeds.Contains(feed) || IsFeedReachable(feed, initialTimeout, tryCount)); if (!allFeedsReachable) { logger.LogWarning("Found unreachable Nuget feed in C# analysis with build-mode 'none'. This may cause missing dependencies in the analysis."); @@ -730,14 +755,6 @@ private bool CheckFeeds(out HashSet explicitFeeds) } compilationInfoContainer.CompilationInfos.Add(("All Nuget feeds reachable", allFeedsReachable ? "1" : "0")); - - var inheritedFeeds = allFeeds.Except(explicitFeeds).ToHashSet(); - if (inheritedFeeds.Count > 0) - { - logger.LogInfo($"Inherited Nuget feeds (not checked for reachability): {string.Join(", ", inheritedFeeds.OrderBy(f => f))}"); - compilationInfoContainer.CompilationInfos.Add(("Inherited Nuget feed count", inheritedFeeds.Count.ToString())); - } - return allFeedsReachable; } From 44483693239f120acfbf6dd9509f79e102537505 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 24 Mar 2025 17:27:22 +0000 Subject: [PATCH 15/21] C#: Check that private package registry feeds are reachable --- .../NugetPackageRestorer.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 7d9c548a143c..828265781b21 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -708,8 +708,13 @@ private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount, private bool CheckFeeds(out HashSet explicitFeeds) { (explicitFeeds, var allFeeds) = GetAllFeeds(); + HashSet feedsToCheck = explicitFeeds; - var allFeedsReachable = this.CheckFeeds(explicitFeeds); + // If private package registries are configured for C#, then check those + // in addition to the ones that are configured in `nuget.config` files. + this.dependabotProxy?.RegistryURLs.ForEach(url => feedsToCheck.Add(url)); + + var allFeedsReachable = this.CheckFeeds(feedsToCheck); var inheritedFeeds = allFeeds.Except(explicitFeeds).ToHashSet(); if (inheritedFeeds.Count > 0) From 7cea2addda21b09c15838ef9ec7cc7eee3cccf8e Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 25 Mar 2025 10:02:29 +0000 Subject: [PATCH 16/21] Apply suggestions from code review Co-authored-by: Michael Nebel --- .../Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs index a798048b933d..49d35c944bd8 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.IO; using System.Linq; -using System.Text; using Newtonsoft.Json.Linq; using Semmle.Util; @@ -77,7 +76,7 @@ private string GetRestoreArgs(RestoreSettings restoreSettings) args += " /p:EnableWindowsTargeting=true"; } - if (restoreSettings.ExtraArgs != null) + if (restoreSettings.ExtraArgs is not null) { args += $" {restoreSettings.ExtraArgs}"; } From d2b88ae5a86f5e2c369ebfd9e31198d1ca16f3c8 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 25 Mar 2025 10:07:08 +0000 Subject: [PATCH 17/21] C#: Rename overloaded `CheckFeeds` method and fix comment --- .../NugetPackageRestorer.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 828265781b21..a7659c89defd 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -701,7 +701,8 @@ private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount, } /// - /// Checks that we can connect to all Nuget feeds that are explicitly configured in configuration files. + /// Checks that we can connect to all Nuget feeds that are explicitly configured in configuration files + /// as well as any private package registry feeds that are configured. /// /// Outputs the set of explicit feeds. /// True if all feeds are reachable or false otherwise. @@ -714,7 +715,7 @@ private bool CheckFeeds(out HashSet explicitFeeds) // in addition to the ones that are configured in `nuget.config` files. this.dependabotProxy?.RegistryURLs.ForEach(url => feedsToCheck.Add(url)); - var allFeedsReachable = this.CheckFeeds(feedsToCheck); + var allFeedsReachable = this.CheckSpecifiedFeeds(feedsToCheck); var inheritedFeeds = allFeeds.Except(explicitFeeds).ToHashSet(); if (inheritedFeeds.Count > 0) @@ -731,7 +732,7 @@ private bool CheckFeeds(out HashSet explicitFeeds) /// /// The set of package feeds to check. /// True if all feeds are reachable or false otherwise. - private bool CheckFeeds(HashSet feeds) + private bool CheckSpecifiedFeeds(HashSet feeds) { logger.LogInfo("Checking that Nuget feeds are reachable..."); From 4d3b0246b5c2da3e86129cbb1232baac0cb68a6f Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 25 Mar 2025 10:14:03 +0000 Subject: [PATCH 18/21] C#: Do not manually add public feed when private registries are used --- .../NugetPackageRestorer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index a7659c89defd..32107f44991b 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -274,7 +274,6 @@ private void RestoreProjects(IEnumerable projects, HashSet? conf // to `dotnet` ignores other feeds that may be configured, we also need to add the feeds // we have discovered from analysing `nuget.config` files. var sources = configuredSources ?? new(); - sources.Add(PublicNugetOrgFeed); this.dependabotProxy.RegistryURLs.ForEach(url => sources.Add(url)); // Add package sources. If any are present, they override all sources specified in From 73ca2eb2c55afb59fd1272b9811f65476d4afb4e Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 25 Mar 2025 10:44:29 +0000 Subject: [PATCH 19/21] C#: Use `allFeeds` rather than `explicitFeeds` for `RestoreProjects` --- .../NugetPackageRestorer.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 32107f44991b..3a0f953fc614 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -103,10 +103,11 @@ public HashSet Restore() compilationInfoContainer.CompilationInfos.Add(("NuGet feed responsiveness checked", checkNugetFeedResponsiveness ? "1" : "0")); HashSet? explicitFeeds = null; + HashSet? allFeeds = null; try { - if (checkNugetFeedResponsiveness && !CheckFeeds(out explicitFeeds)) + if (checkNugetFeedResponsiveness && !CheckFeeds(out explicitFeeds, out allFeeds)) { // todo: we could also check the reachability of the inherited nuget feeds, but to use those in the fallback we would need to handle authentication too. var unresponsiveMissingPackageLocation = DownloadMissingPackagesFromSpecificFeeds(explicitFeeds); @@ -156,7 +157,7 @@ public HashSet Restore() var restoredProjects = RestoreSolutions(out var container); var projects = fileProvider.Projects.Except(restoredProjects); - RestoreProjects(projects, explicitFeeds, out var containers); + RestoreProjects(projects, allFeeds, out var containers); var dependencies = containers.Flatten(container); @@ -704,10 +705,11 @@ private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount, /// as well as any private package registry feeds that are configured. /// /// Outputs the set of explicit feeds. + /// Outputs the set of all feeds (explicit and inherited). /// True if all feeds are reachable or false otherwise. - private bool CheckFeeds(out HashSet explicitFeeds) + private bool CheckFeeds(out HashSet explicitFeeds, out HashSet allFeeds) { - (explicitFeeds, var allFeeds) = GetAllFeeds(); + (explicitFeeds, allFeeds) = GetAllFeeds(); HashSet feedsToCheck = explicitFeeds; // If private package registries are configured for C#, then check those From be95d335b7286a91ba743390cf5c8480d788ebeb Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 25 Mar 2025 11:29:06 +0000 Subject: [PATCH 20/21] C#: Obtain all feeds from source directory if there are no `nuget.config` files anywhere --- .../NugetPackageRestorer.cs | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 3a0f953fc614..d487bc37572a 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -810,23 +810,33 @@ private IEnumerable GetFeeds(Func> getNugetFeeds) } // todo: this could be improved. - // We don't have to get the feeds from each of the folders from below, it would be enought to check the folders that recursively contain the others. - var allFeeds = nugetConfigs - .Select(config => - { - try - { - return new FileInfo(config).Directory?.FullName; - } - catch (Exception exc) + HashSet? allFeeds = null; + + if (nugetConfigs.Count > 0) + { + // We don't have to get the feeds from each of the folders from below, it would be enought to check the folders that recursively contain the others. + allFeeds = nugetConfigs + .Select(config => { - logger.LogWarning($"Failed to get directory of '{config}': {exc}"); - } - return null; - }) - .Where(folder => folder != null) - .SelectMany(folder => GetFeeds(() => dotnet.GetNugetFeedsFromFolder(folder!))) - .ToHashSet(); + try + { + return new FileInfo(config).Directory?.FullName; + } + catch (Exception exc) + { + logger.LogWarning($"Failed to get directory of '{config}': {exc}"); + } + return null; + }) + .Where(folder => folder != null) + .SelectMany(folder => GetFeeds(() => dotnet.GetNugetFeedsFromFolder(folder!))) + .ToHashSet(); + } + else + { + // If we haven't found any `nuget.config` files, then obtain a list of feeds from the root source directory. + allFeeds = GetFeeds(() => dotnet.GetNugetFeedsFromFolder(this.fileProvider.SourceDir.FullName)).ToHashSet(); + } logger.LogInfo($"Found {allFeeds.Count} Nuget feeds (with inherited ones) in nuget.config files: {string.Join(", ", allFeeds.OrderBy(f => f))}"); From fe1c098624420ae60ace655546d33d9773369de7 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 25 Mar 2025 12:39:37 +0000 Subject: [PATCH 21/21] C#: Accept changes to `.expected` files --- .../all-platforms/standalone_resx/CompilationInfo.expected | 1 + .../all-platforms/standalone_winforms/CompilationInfo.expected | 1 + 2 files changed, 2 insertions(+) diff --git a/csharp/ql/integration-tests/all-platforms/standalone_resx/CompilationInfo.expected b/csharp/ql/integration-tests/all-platforms/standalone_resx/CompilationInfo.expected index 48cca2534533..ee27a1cd9120 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone_resx/CompilationInfo.expected +++ b/csharp/ql/integration-tests/all-platforms/standalone_resx/CompilationInfo.expected @@ -1,6 +1,7 @@ | All Nuget feeds reachable | 1.0 | | Failed project restore with package source error | 0.0 | | Failed solution restore with package source error | 0.0 | +| Inherited Nuget feed count | 1.0 | | NuGet feed responsiveness checked | 1.0 | | Project files on filesystem | 1.0 | | Reachable fallback Nuget feed count | 1.0 | diff --git a/csharp/ql/integration-tests/all-platforms/standalone_winforms/CompilationInfo.expected b/csharp/ql/integration-tests/all-platforms/standalone_winforms/CompilationInfo.expected index f87af9b7599d..cf2e7f2db702 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone_winforms/CompilationInfo.expected +++ b/csharp/ql/integration-tests/all-platforms/standalone_winforms/CompilationInfo.expected @@ -1,6 +1,7 @@ | All Nuget feeds reachable | 1.0 | | Failed project restore with package source error | 0.0 | | Failed solution restore with package source error | 0.0 | +| Inherited Nuget feed count | 1.0 | | NuGet feed responsiveness checked | 1.0 | | Project files on filesystem | 1.0 | | Reachable fallback Nuget feed count | 1.0 |