Merge pull request #18850 from github/mbg/csharp/inject-proxy-urls

mbg · web-flow · commit 2aee47b257c4 · 2025-03-27T10:11:05.000Z
C#: Automatically use configured private registry feeds
diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs
@@ -1,14 +1,22 @@
 using System;
-using System.Diagnostics;
+using System.Collections.Generic;
 using System.IO;
 using System.Security.Cryptography.X509Certificates;
 using Semmle.Util;
 using Semmle.Util.Logging;
+using Newtonsoft.Json;
 
 namespace Semmle.Extraction.CSharp.DependencyFetching
 {
     public class DependabotProxy : IDisposable
     {
+        /// <summary>
+        /// Represents configurations for package registries.
+        /// </summary>
+        /// <param name="Type">The type of package registry.</param>
+        /// <param name="URL">The URL of the package registry.</param>
+        public record class RegistryConfig(string Type, string URL);
+
         private readonly string host;
         private readonly string port;
 
@@ -17,6 +25,10 @@ public class DependabotProxy : IDisposable
         /// </summary>
         internal string Address { get; }
         /// <summary>
+        /// The URLs of package registries that are configured for the proxy.
+        /// </summary>
+        internal HashSet<string> RegistryURLs { get; }
+        /// <summary>
         /// The path to the temporary file where the certificate is stored.
         /// </summary>
         internal string? CertificatePath { get; private set; }
@@ -67,6 +79,39 @@ public class DependabotProxy : IDisposable
                 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<List<RegistryConfig>>(registryURLs);
+                    if (array is not 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 (JsonException ex)
+                {
+                    logger.LogError($"Unable to parse '{EnvironmentVariableNames.ProxyURLs}': {ex.Message}");
+                }
+            }
+
             return result;
         }
 
@@ -75,6 +120,7 @@ private DependabotProxy(string host, string port)
             this.host = host;
             this.port = port;
             this.Address = $"http://{this.host}:{this.port}";
+            this.RegistryURLs = new HashSet<string>();
         }
 
         public void Dispose()
diff --git 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 Newtonsoft.Json.Linq;
 
 using Semmle.Util;
@@ -77,6 +76,11 @@ private string GetRestoreArgs(RestoreSettings restoreSettings)
                 args += " /p:EnableWindowsTargeting=true";
             }
 
+            if (restoreSettings.ExtraArgs is not null)
+            {
+                args += $" {restoreSettings.ExtraArgs}";
+            }
+
             return args;
         }
 
diff --git 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.
         /// </summary>
         public const string ProxyCertificate = "CODEQL_PROXY_CA_CERTIFICATE";
+
+        /// <summary>
+        /// Contains the URLs of private nuget registries as a JSON array.
+        /// </summary>
+        public const string ProxyURLs = "CODEQL_PROXY_URLS";
     }
 }
diff --git 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<string> 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, string? ExtraArgs = null, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false);
 
     public partial record class RestoreResult(bool Success, IList<string> Output)
     {
diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs
@@ -103,10 +103,11 @@ public HashSet<AssemblyLookupLocation> Restore()
             compilationInfoContainer.CompilationInfos.Add(("NuGet feed responsiveness checked", checkNugetFeedResponsiveness ? "1" : "0"));
 
             HashSet<string>? explicitFeeds = null;
+            HashSet<string>? 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<AssemblyLookupLocation> Restore()
 
             var restoredProjects = RestoreSolutions(out var container);
             var projects = fileProvider.Projects.Except(restoredProjects);
-            RestoreProjects(projects, out var containers);
+            RestoreProjects(projects, allFeeds, out var containers);
 
             var dependencies = containers.Flatten(container);
 
@@ -260,8 +261,33 @@ private IEnumerable<string> RestoreSolutions(out DependencyContainer dependencie
         /// Populates dependencies with the relative paths to the assets files generated by the restore.
         /// </summary>
         /// <param name="projects">A list of paths to project files.</param>
-        private void RestoreProjects(IEnumerable<string> projects, out ConcurrentBag<DependencyContainer> dependencies)
+        private void RestoreProjects(IEnumerable<string> projects, HashSet<string>? configuredSources, out ConcurrentBag<DependencyContainer> dependencies)
         {
+            // 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.
+            string? extraArgs = 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
+                // 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();
+                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;
             var nugetSourceFailures = 0;
             ConcurrentBag<DependencyContainer> collectedDependencies = [];
@@ -276,7 +302,7 @@ private void RestoreProjects(IEnumerable<string> projects, out ConcurrentBag<Dep
                 foreach (var project in projectGroup)
                 {
                     logger.LogInfo($"Restoring project {project}...");
-                    var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, TargetWindows: isWindows));
+                    var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, extraArgs, TargetWindows: isWindows));
                     assets.AddDependenciesRange(res.AssetsFilePaths);
                     lock (sync)
                     {
@@ -680,10 +706,42 @@ private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount,
             return (timeoutMilliSeconds, tryCount);
         }
 
-        private bool CheckFeeds(out HashSet<string> explicitFeeds)
+        /// <summary>
+        /// 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.
+        /// </summary>
+        /// <param name="explicitFeeds">Outputs the set of explicit feeds.</param>
+        /// <param name="allFeeds">Outputs the set of all feeds (explicit and inherited).</param>
+        /// <returns>True if all feeds are reachable or false otherwise.</returns>
+        private bool CheckFeeds(out HashSet<string> explicitFeeds, out HashSet<string> allFeeds)
+        {
+            (explicitFeeds, allFeeds) = GetAllFeeds();
+            HashSet<string> feedsToCheck = 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.CheckSpecifiedFeeds(feedsToCheck);
+
+            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;
+        }
+
+        /// <summary>
+        /// Checks that we can connect to the specified Nuget feeds.
+        /// </summary>
+        /// <param name="feeds">The set of package feeds to check.</param>
+        /// <returns>True if all feeds are reachable or false otherwise.</returns>
+        private bool CheckSpecifiedFeeds(HashSet<string> feeds)
         {
-            logger.LogInfo("Checking Nuget feeds...");
-            (explicitFeeds, var allFeeds) = GetAllFeeds();
+            logger.LogInfo("Checking that Nuget feeds are reachable...");
 
             var excludedFeeds = EnvironmentVariables.GetURLs(EnvironmentVariableNames.ExcludedNugetFeedsFromResponsivenessCheck)
                 .ToHashSet();
@@ -695,7 +753,7 @@ private bool CheckFeeds(out HashSet<string> 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.");
@@ -710,14 +768,6 @@ private bool CheckFeeds(out HashSet<string> 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;
         }
 
@@ -766,23 +816,33 @@ private IEnumerable<string> GetFeeds(Func<IList<string>> 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<string>? 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))}");
 
diff --git 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();
diff --git 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
@@ -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 |

Original file line number	Diff line number	Diff line change
`@@ -89,5 +89,10 @@ internal static class EnvironmentVariableNames`
`89`	`89`	`/// Contains the certificate used by the Dependabot proxy.`
`90`	`90`	`/// </summary>`
`91`	`91`	`public const string ProxyCertificate = "CODEQL_PROXY_CA_CERTIFICATE";`
	`92`	`+`
	`93`	`+ /// <summary>`
	`94`	`+ /// Contains the URLs of private nuget registries as a JSON array.`
	`95`	`+ /// </summary>`
	`96`	`+ public const string ProxyURLs = "CODEQL_PROXY_URLS";`
`92`	`97`	`}`
`93`	`98`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ public interface IDotNet`
`17`	`17`	`IList<string> GetNugetFeedsFromFolder(string folderPath);`
`18`	`18`	`}`
`19`	`19`
`20`		`- public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false);`
	`20`	`+ public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? ExtraArgs = null, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false);`
`21`	`21`
`22`	`22`	`public partial record class RestoreResult(bool Success, IList<string> Output)`
`23`	`23`	`{`