From 4ace5f4a9e9825123c75df6bf6a2e5a2557bc2c3 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Sat, 24 Aug 2024 12:15:44 -0700 Subject: [PATCH 01/47] Discover Net tools installed plugins --- .../Plugins/PluginDiscoverer.cs | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 830ba4d4142..b5f6f96a274 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -4,6 +4,8 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using NuGet.Common; @@ -76,6 +78,9 @@ public async Task> DiscoverAsync(Cancellation } _pluginFiles = GetPluginFiles(cancellationToken); + + // Get plugins added using .Net Tools + _pluginFiles.AddRange(GetNetToolsPluginFiles(cancellationToken)); var results = new List(); for (var i = 0; i < _pluginFiles.Count; ++i) @@ -124,6 +129,57 @@ private List GetPluginFiles(CancellationToken cancellationToken) return files; } + private List GetNetToolsPluginFiles(CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + var pluginFiles = new List(); + var paths = Environment.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); + + foreach (var path in paths) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (Directory.Exists(path)) + { + var directoryInfo = new DirectoryInfo(path); + var files = directoryInfo.GetFiles("nuget-plugin-*"); + + foreach (var file in files) + { + if (IsValidPluginFile(file)) + { + var state = new Lazy(() => _verifier.IsValid(file.FullName) ? PluginFileState.Valid : PluginFileState.InvalidEmbeddedSignature); + pluginFiles.Add(new PluginFile(file.FullName, state)); + } + } + } + } + + return pluginFiles; + } + + private static bool IsValidPluginFile(FileInfo fileInfo) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return fileInfo.Extension.Equals(".exe", StringComparison.OrdinalIgnoreCase) || + fileInfo.Extension.Equals(".bat", StringComparison.OrdinalIgnoreCase); + } + else + { + return fileInfo.Exists && (fileInfo.Attributes & FileAttributes.ReparsePoint) == 0 && IsExecutable(fileInfo); + } + } + + private static bool IsExecutable(FileInfo fileInfo) + { + // TODO + // check if the files executable bit is set + return true; + } + + private IEnumerable GetPluginFilePaths() { if (string.IsNullOrEmpty(_rawPluginPaths)) From e23db9570c64146af3dfe33c204feb9cf51fb3e0 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 3 Sep 2024 11:09:42 -0700 Subject: [PATCH 02/47] Read unix executable plugins --- .../Plugins/PluginDiscoverer.cs | 55 +++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index b5f6f96a274..22c7bde82d6 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -3,8 +3,8 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; -using System.Linq; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -129,6 +129,13 @@ private List GetPluginFiles(CancellationToken cancellationToken) return files; } + /// + /// Gets auth plugins installed using dotnet tools. This is done by iterating through each file in directories found int eh + /// `PATH` environment variable. + /// The files are also expected to have a name that starts with `nuget-plugin-` + /// + /// + /// private List GetNetToolsPluginFiles(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); @@ -159,6 +166,13 @@ private List GetNetToolsPluginFiles(CancellationToken cancellationTo return pluginFiles; } + /// + /// Checks whether a file is a valid plugin file for windows/Unix. + /// Windows: It should be either .bat or .exe + /// Unix: It should be executable + /// + /// + /// private static bool IsValidPluginFile(FileInfo fileInfo) { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -172,14 +186,47 @@ private static bool IsValidPluginFile(FileInfo fileInfo) } } + /// + /// Checks whether a file is executable or not in Unix. + /// This is done by running bash code: `if [ -x {fileInfo.FullName} ]; then echo yes; else echo no; fi` + /// + /// + /// private static bool IsExecutable(FileInfo fileInfo) { - // TODO - // check if the files executable bit is set - return true; +#pragma warning disable CA1031 // Do not catch general exception types + try + { + string output; + using (var process = new Process()) + { + // Use a shell command to check if the file is executable + process.StartInfo.FileName = "sh"; + process.StartInfo.Arguments = $"-c \"if [ -x {fileInfo.FullName} ]; then echo yes; else echo no; fi\""; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + + process.Start(); + if (!process.WaitForExit(1000) || process.ExitCode != 0) + { + return false; + } + + output = process.StandardOutput.ReadToEnd().Trim(); + } + + // Check if the output is "yes" + return output.Equals("yes", StringComparison.OrdinalIgnoreCase); + } + catch + { + return false; + } +#pragma warning restore CA1031 // Do not catch general exception types } + private IEnumerable GetPluginFilePaths() { if (string.IsNullOrEmpty(_rawPluginPaths)) From 73dc448b94c7c0443edfb71fb54633751c0a5df2 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 3 Sep 2024 12:21:11 -0700 Subject: [PATCH 03/47] Add tests --- .../Plugins/PluginDiscoverer.cs | 22 ++++--- .../Plugins/PluginDiscovererTests.cs | 59 +++++++++++++++++++ 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 22c7bde82d6..1e3617c4236 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Linq; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -79,8 +80,6 @@ public async Task> DiscoverAsync(Cancellation _pluginFiles = GetPluginFiles(cancellationToken); - // Get plugins added using .Net Tools - _pluginFiles.AddRange(GetNetToolsPluginFiles(cancellationToken)); var results = new List(); for (var i = 0; i < _pluginFiles.Count; ++i) @@ -134,19 +133,14 @@ private List GetPluginFiles(CancellationToken cancellationToken) /// `PATH` environment variable. /// The files are also expected to have a name that starts with `nuget-plugin-` /// - /// /// - private List GetNetToolsPluginFiles(CancellationToken cancellationToken) + private static List GetNetToolsPluginFiles() { - cancellationToken.ThrowIfCancellationRequested(); - - var pluginFiles = new List(); + var pluginFiles = new List(); var paths = Environment.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); foreach (var path in paths) { - cancellationToken.ThrowIfCancellationRequested(); - if (Directory.Exists(path)) { var directoryInfo = new DirectoryInfo(path); @@ -156,8 +150,7 @@ private List GetNetToolsPluginFiles(CancellationToken cancellationTo { if (IsValidPluginFile(file)) { - var state = new Lazy(() => _verifier.IsValid(file.FullName) ? PluginFileState.Valid : PluginFileState.InvalidEmbeddedSignature); - pluginFiles.Add(new PluginFile(file.FullName, state)); + pluginFiles.Add(file.FullName); } } } @@ -236,7 +229,12 @@ private IEnumerable GetPluginFilePaths() // Internal plugins are only supported for .NET Framework scenarios, namely msbuild.exe directories.Add(PluginDiscoveryUtility.GetInternalPlugins()); #endif - return PluginDiscoveryUtility.GetConventionBasedPlugins(directories); + var plugins = PluginDiscoveryUtility.GetConventionBasedPlugins(directories).ToList(); + + // Get plugins added using .Net Tools + plugins.AddRange(GetNetToolsPluginFiles()); + + return plugins; } return _rawPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index f48460dee65..16b1a4219b5 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -153,5 +153,64 @@ public async Task DiscoverAsync_IsIdempotent() } } } + + [Theory] + [InlineData("nuget-plugin-myPlugin.exe")] + [InlineData("nuget-plugin-myPlugin.bat")] + public async Task DiscoverAsync_withValidDotNetToolsPlugin_FindsThePlugin(string fileName) + { + using (var testDirectory = TestDirectory.Create()) + { + // Arrange + var pluginPath = Path.Combine(testDirectory.Path, "myPlugin"); + Directory.CreateDirectory(pluginPath); + var myPlugin = Path.Combine(pluginPath, fileName); + + Environment.SetEnvironmentVariable("PATH", pluginPath); + File.WriteAllText(myPlugin, string.Empty); + var verifierSpy = new Mock(); + verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) + .Returns(true); + + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object)) + { + // Act + var result = await discoverer.DiscoverAsync(CancellationToken.None); + + // Assert + var discovered = false; + + foreach (PluginDiscoveryResult discoveryResult in result) + { + if (myPlugin == discoveryResult.PluginFile.Path) discovered = true; + } + + Assert.True(discovered); + } + } + } + + private sealed class EmbeddedSignatureVerifierStub : EmbeddedSignatureVerifier + { + private readonly Dictionary _responses; + + internal int IsValidCallCount { get; private set; } + + public EmbeddedSignatureVerifierStub(Dictionary responses) + { + _responses = responses; + } + + public override bool IsValid(string filePath) + { + ++IsValidCallCount; + + bool value; + + Assert.True(_responses.TryGetValue(filePath, out value)); + + return value; + } + } } } From d5ba119d9aeda49bef4f3cd2e0ea3e12e31764be Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 3 Sep 2024 12:47:12 -0700 Subject: [PATCH 04/47] Linux test --- .../Plugins/PluginDiscovererTests.cs | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 16b1a4219b5..0b036414d24 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; @@ -154,10 +155,10 @@ public async Task DiscoverAsync_IsIdempotent() } } - [Theory] + [PlatformTheory(Platform.Windows)] [InlineData("nuget-plugin-myPlugin.exe")] [InlineData("nuget-plugin-myPlugin.bat")] - public async Task DiscoverAsync_withValidDotNetToolsPlugin_FindsThePlugin(string fileName) + public async Task DiscoverAsync_withValidDotNetToolsPluginWindows_FindsThePlugin(string fileName) { using (var testDirectory = TestDirectory.Create()) { @@ -190,6 +191,53 @@ public async Task DiscoverAsync_withValidDotNetToolsPlugin_FindsThePlugin(string } } + [PlatformFact(Platform.Linux)] + public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() + { + using (var testDirectory = TestDirectory.Create()) + { + // Arrange + var pluginPath = Path.Combine(testDirectory.Path, "myPlugins"); + Directory.CreateDirectory(pluginPath); + var myPlugin = Path.Combine(pluginPath, "MyPlugin"); + + using (var process = new Process()) + { + // Use a shell command to check if the file is executable + process.StartInfo.FileName = "sh"; + process.StartInfo.Arguments = $"-c \"chmod +x {myPlugin} ]\""; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + + process.Start(); + if (!process.WaitForExit(1000) || process.ExitCode != 0) + { + Environment.SetEnvironmentVariable("PATH", pluginPath); + File.WriteAllText(myPlugin, string.Empty); + var verifierSpy = new Mock(); + verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) + .Returns(true); + + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object)) + { + // Act + var result = await discoverer.DiscoverAsync(CancellationToken.None); + + // Assert + var discovered = false; + + foreach (PluginDiscoveryResult discoveryResult in result) + { + if (myPlugin == discoveryResult.PluginFile.Path) discovered = true; + } + + Assert.True(discovered); + } + } + } + } + } + private sealed class EmbeddedSignatureVerifierStub : EmbeddedSignatureVerifier { private readonly Dictionary _responses; From 3ad5aef9c2b4541ec04f3eb3c83b832e89a94e86 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 3 Sep 2024 12:47:48 -0700 Subject: [PATCH 05/47] Linux test --- .../NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 0b036414d24..ab3d0b09a45 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -203,7 +203,7 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() using (var process = new Process()) { - // Use a shell command to check if the file is executable + // Use a shell command to make the file executable process.StartInfo.FileName = "sh"; process.StartInfo.Arguments = $"-c \"chmod +x {myPlugin} ]\""; process.StartInfo.UseShellExecute = false; From 6d0a92f52442983ae28a3c3807a3445362fd1ab9 Mon Sep 17 00:00:00 2001 From: Nigusu Solomon Yenework <59111203+Nigusu-Allehu@users.noreply.github.com> Date: Tue, 3 Sep 2024 12:22:37 -0700 Subject: [PATCH 06/47] cleanup --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 1e3617c4236..b9c50c984fd 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -218,8 +218,6 @@ private static bool IsExecutable(FileInfo fileInfo) #pragma warning restore CA1031 // Do not catch general exception types } - - private IEnumerable GetPluginFilePaths() { if (string.IsNullOrEmpty(_rawPluginPaths)) From 29f9482cea897bac3d9f691c6467425ac4e56c18 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Wed, 4 Sep 2024 15:54:33 -0700 Subject: [PATCH 07/47] Use GetUnixFileMode --- src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj | 2 +- .../NuGet.Protocol/Plugins/PluginDiscoverer.cs | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj b/src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj index 4b075d8c266..fe7eb1a2a0c 100644 --- a/src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj +++ b/src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj @@ -1,6 +1,6 @@ - $(TargetFrameworksLibraryForSigning) + $(TargetFrameworksLibraryForSigning);$(NETCoreTargetFramework) $(NoWarn);CS1591;CS1573;CS0012;RS0041 nuget protocol diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index b9c50c984fd..573b4d918f6 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -175,7 +175,16 @@ private static bool IsValidPluginFile(FileInfo fileInfo) } else { +#if NET8_0 + var fileMode = File.GetUnixFileMode(fileInfo.FullName); + + return fileInfo.Exists && + ((fileMode & UnixFileMode.UserExecute) != 0 || + (fileMode & UnixFileMode.GroupExecute) != 0 || + (fileMode & UnixFileMode.OtherExecute) != 0); +#else return fileInfo.Exists && (fileInfo.Attributes & FileAttributes.ReparsePoint) == 0 && IsExecutable(fileInfo); +#endif } } From ec680b42a6fb6b8ade141949d94ca2582b75c2ee Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 5 Sep 2024 10:15:31 -0700 Subject: [PATCH 08/47] Fix test --- .../NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index ab3d0b09a45..659e7ea93ee 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -205,7 +205,7 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() { // Use a shell command to make the file executable process.StartInfo.FileName = "sh"; - process.StartInfo.Arguments = $"-c \"chmod +x {myPlugin} ]\""; + process.StartInfo.Arguments = $"-c \"chmod +x {myPlugin}\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; From eb008758ae576fa2c2188da72a9c51682add5274 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 5 Sep 2024 10:30:52 -0700 Subject: [PATCH 09/47] Add more tests --- .../Plugins/PluginDiscovererTests.cs | 85 ++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 659e7ea93ee..7cebe1f9bc2 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -191,6 +191,42 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginWindows_FindsThePlugin } } + [PlatformTheory(Platform.Windows)] + [InlineData("nugetplugin-myPlugin.exe")] + [InlineData("nugetplugin-myPlugin.bat")] + public async Task DiscoverAsync_withInValidDotNetToolsPluginNameWindows_DoesNotFindThePlugin(string fileName) + { + using (var testDirectory = TestDirectory.Create()) + { + // Arrange + var pluginPath = Path.Combine(testDirectory.Path, "myPlugin"); + Directory.CreateDirectory(pluginPath); + var myPlugin = Path.Combine(pluginPath, fileName); + + Environment.SetEnvironmentVariable("PATH", pluginPath); + File.WriteAllText(myPlugin, string.Empty); + var verifierSpy = new Mock(); + verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) + .Returns(true); + + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object)) + { + // Act + var result = await discoverer.DiscoverAsync(CancellationToken.None); + + // Assert + var discovered = false; + + foreach (PluginDiscoveryResult discoveryResult in result) + { + if (myPlugin == discoveryResult.PluginFile.Path) discovered = true; + } + + Assert.True(discovered); + } + } + } + [PlatformFact(Platform.Linux)] public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() { @@ -199,7 +235,7 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() // Arrange var pluginPath = Path.Combine(testDirectory.Path, "myPlugins"); Directory.CreateDirectory(pluginPath); - var myPlugin = Path.Combine(pluginPath, "MyPlugin"); + var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); using (var process = new Process()) { @@ -238,6 +274,53 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() } } + [PlatformFact(Platform.Linux)] + public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_DoesNotFindThePlugin() + { + using (var testDirectory = TestDirectory.Create()) + { + // Arrange + var pluginPath = Path.Combine(testDirectory.Path, "myPlugins"); + Directory.CreateDirectory(pluginPath); + var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); + + using (var process = new Process()) + { + // Use a shell command to make the file executable + process.StartInfo.FileName = "sh"; + process.StartInfo.Arguments = $"-c \"chmod -x {myPlugin}\""; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + + process.Start(); + if (!process.WaitForExit(1000) || process.ExitCode != 0) + { + Environment.SetEnvironmentVariable("PATH", pluginPath); + File.WriteAllText(myPlugin, string.Empty); + var verifierSpy = new Mock(); + verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) + .Returns(true); + + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object)) + { + // Act + var result = await discoverer.DiscoverAsync(CancellationToken.None); + + // Assert + var discovered = false; + + foreach (PluginDiscoveryResult discoveryResult in result) + { + if (myPlugin == discoveryResult.PluginFile.Path) discovered = true; + } + + Assert.False(discovered); + } + } + } + } + } + private sealed class EmbeddedSignatureVerifierStub : EmbeddedSignatureVerifier { private readonly Dictionary _responses; From e6f3e9f553ac24d9c3098adbe9f03d1b1e4001ec Mon Sep 17 00:00:00 2001 From: Nigusu Solomon Yenework <59111203+Nigusu-Allehu@users.noreply.github.com> Date: Thu, 5 Sep 2024 10:19:23 -0700 Subject: [PATCH 10/47] white space --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 573b4d918f6..6f6bdc5c245 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -79,7 +79,6 @@ public async Task> DiscoverAsync(Cancellation } _pluginFiles = GetPluginFiles(cancellationToken); - var results = new List(); for (var i = 0; i < _pluginFiles.Count; ++i) From 3ac5084ba5c97572e1bb06b75f269d5791ce9f04 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 5 Sep 2024 10:51:37 -0700 Subject: [PATCH 11/47] fix tests --- .../Plugins/PluginDiscovererTests.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 7cebe1f9bc2..a4af876e1e2 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -236,6 +236,7 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() var pluginPath = Path.Combine(testDirectory.Path, "myPlugins"); Directory.CreateDirectory(pluginPath); var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); + File.WriteAllText(myPlugin, string.Empty); using (var process = new Process()) { @@ -246,10 +247,10 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() process.StartInfo.RedirectStandardOutput = true; process.Start(); - if (!process.WaitForExit(1000) || process.ExitCode != 0) + process.WaitForExit(); + if (process.ExitCode == 0) { Environment.SetEnvironmentVariable("PATH", pluginPath); - File.WriteAllText(myPlugin, string.Empty); var verifierSpy = new Mock(); verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); @@ -283,6 +284,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does var pluginPath = Path.Combine(testDirectory.Path, "myPlugins"); Directory.CreateDirectory(pluginPath); var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); + File.WriteAllText(myPlugin, string.Empty); using (var process = new Process()) { @@ -293,10 +295,10 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does process.StartInfo.RedirectStandardOutput = true; process.Start(); - if (!process.WaitForExit(1000) || process.ExitCode != 0) + process.WaitForExit(); + if (process.ExitCode == 0) { Environment.SetEnvironmentVariable("PATH", pluginPath); - File.WriteAllText(myPlugin, string.Empty); var verifierSpy = new Mock(); verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); From de6bd476a419921475c61d3ed24796a1a2d7cbe6 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 5 Sep 2024 10:52:41 -0700 Subject: [PATCH 12/47] Comment --- .../NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index a4af876e1e2..0e444de38bb 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -288,7 +288,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does using (var process = new Process()) { - // Use a shell command to make the file executable + // Use a shell command to make the file not executable process.StartInfo.FileName = "sh"; process.StartInfo.Arguments = $"-c \"chmod -x {myPlugin}\""; process.StartInfo.UseShellExecute = false; From b928c1ab50b7ea72c73a476321eb46624bf2f610 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 5 Sep 2024 11:18:44 -0700 Subject: [PATCH 13/47] use /bin/bash --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 4 ++-- .../NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 6f6bdc5c245..673218143a6 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -202,8 +202,8 @@ private static bool IsExecutable(FileInfo fileInfo) using (var process = new Process()) { // Use a shell command to check if the file is executable - process.StartInfo.FileName = "sh"; - process.StartInfo.Arguments = $"-c \"if [ -x {fileInfo.FullName} ]; then echo yes; else echo no; fi\""; + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"if [ -x {fileInfo.FullName} ]; then echo yes; else echo no; fi"; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 0e444de38bb..53514f5b05b 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -241,8 +241,8 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() using (var process = new Process()) { // Use a shell command to make the file executable - process.StartInfo.FileName = "sh"; - process.StartInfo.Arguments = $"-c \"chmod +x {myPlugin}\""; + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"chmod +x {myPlugin}"; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; @@ -289,8 +289,8 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does using (var process = new Process()) { // Use a shell command to make the file not executable - process.StartInfo.FileName = "sh"; - process.StartInfo.Arguments = $"-c \"chmod -x {myPlugin}\""; + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"chmod -x {myPlugin}"; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; From 1ec9a79b9ea71ced5fa19a137add6283408a5f45 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 5 Sep 2024 11:47:55 -0700 Subject: [PATCH 14/47] Fix assert --- .../NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 53514f5b05b..f7d486e37c7 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -222,7 +222,7 @@ public async Task DiscoverAsync_withInValidDotNetToolsPluginNameWindows_DoesNotF if (myPlugin == discoveryResult.PluginFile.Path) discovered = true; } - Assert.True(discovered); + Assert.False(discovered); } } } @@ -245,9 +245,9 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() process.StartInfo.Arguments = $"chmod +x {myPlugin}"; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; - process.Start(); process.WaitForExit(); + if (process.ExitCode == 0) { Environment.SetEnvironmentVariable("PATH", pluginPath); @@ -293,9 +293,9 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does process.StartInfo.Arguments = $"chmod -x {myPlugin}"; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; - process.Start(); process.WaitForExit(); + if (process.ExitCode == 0) { Environment.SetEnvironmentVariable("PATH", pluginPath); From 68e86eec5de969cae07fbf5d2e76ea405c1c17be Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 10 Sep 2024 10:57:37 -0700 Subject: [PATCH 15/47] Cleanup testing --- .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 2 ++ .../Plugins/PluginDiscoverer.cs | 35 +++++++++++++------ .../NuGet.Protocol/Plugins/PluginFile.cs | 16 +++++++++ .../PublicAPI/net472/PublicAPI.Unshipped.txt | 2 ++ .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 2 ++ .../netstandard2.0/PublicAPI.Unshipped.txt | 2 ++ .../Plugins/PluginDiscovererTests.cs | 22 +++++++----- 7 files changed, 62 insertions(+), 19 deletions(-) diff --git a/src/NuGet.Core/NuGet.Credentials/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Credentials/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 7dc5c58110b..53b5306a726 100644 --- a/src/NuGet.Core/NuGet.Credentials/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Credentials/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1 +1,3 @@ #nullable enable +NuGet.Protocol.Plugins.PluginFile.IsDotnetToolsPlugin.get -> bool +~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy state, bool isDotnetToolsPlugin) -> void diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 673218143a6..24cecc2b75a 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -23,6 +23,13 @@ public sealed class PluginDiscoverer : IPluginDiscoverer private readonly string _rawPluginPaths; private IEnumerable _results; private readonly SemaphoreSlim _semaphore; + private readonly EmbeddedSignatureVerifier _verifier; + private IEnvironmentVariableReader _environmentVariableReader; + + internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verifier, IEnvironmentVariableReader environmentVariableReader) : this(rawPluginPaths, verifier) + { + _environmentVariableReader = environmentVariableReader; + } /// /// Instantiates a new class. @@ -79,6 +86,7 @@ public async Task> DiscoverAsync(Cancellation } _pluginFiles = GetPluginFiles(cancellationToken); + _pluginFiles.AddRange(GetNetToolsPluginFiles()); var results = new List(); for (var i = 0; i < _pluginFiles.Count; ++i) @@ -133,10 +141,16 @@ private List GetPluginFiles(CancellationToken cancellationToken) /// The files are also expected to have a name that starts with `nuget-plugin-` /// /// - private static List GetNetToolsPluginFiles() + private List GetNetToolsPluginFiles() { - var pluginFiles = new List(); - var paths = Environment.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); + var pluginFiles = new List(); + + if (_environmentVariableReader is null) + { + _environmentVariableReader = EnvironmentVariableWrapper.Instance; + } + + var paths = _environmentVariableReader.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); foreach (var path in paths) { @@ -149,7 +163,9 @@ private static List GetNetToolsPluginFiles() { if (IsValidPluginFile(file)) { - pluginFiles.Add(file.FullName); + var state = new Lazy(() => _verifier.IsValid(file.FullName) ? PluginFileState.Valid : PluginFileState.InvalidEmbeddedSignature); + PluginFile pluginFile = new PluginFile(file.FullName, state, isDotnetToolsPlugin: true); + pluginFiles.Add(pluginFile); } } } @@ -174,7 +190,7 @@ private static bool IsValidPluginFile(FileInfo fileInfo) } else { -#if NET8_0 +#if NET8_0_OR_GREATER var fileMode = File.GetUnixFileMode(fileInfo.FullName); return fileInfo.Exists && @@ -187,6 +203,7 @@ private static bool IsValidPluginFile(FileInfo fileInfo) } } +#if !NET8_0_OR_GREATER /// /// Checks whether a file is executable or not in Unix. /// This is done by running bash code: `if [ -x {fileInfo.FullName} ]; then echo yes; else echo no; fi` @@ -225,6 +242,7 @@ private static bool IsExecutable(FileInfo fileInfo) } #pragma warning restore CA1031 // Do not catch general exception types } +#endif private IEnumerable GetPluginFilePaths() { @@ -235,12 +253,7 @@ private IEnumerable GetPluginFilePaths() // Internal plugins are only supported for .NET Framework scenarios, namely msbuild.exe directories.Add(PluginDiscoveryUtility.GetInternalPlugins()); #endif - var plugins = PluginDiscoveryUtility.GetConventionBasedPlugins(directories).ToList(); - - // Get plugins added using .Net Tools - plugins.AddRange(GetNetToolsPluginFiles()); - - return plugins; + return PluginDiscoveryUtility.GetConventionBasedPlugins(directories); } return _rawPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs index 9753e0285bf..9f7f2599b06 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs @@ -20,6 +20,22 @@ public sealed class PluginFile /// public Lazy State { get; } + /// + /// Is the plugin file, a dotnet tools plugin file? + /// + public bool IsDotnetToolsPlugin { get; } + + /// + /// Instantiates a new class. + /// + /// The plugin's file path. + /// A lazy that evaluates the plugin file state. + /// Is the plugin file, a dotnet tools plugin file? + public PluginFile(string filePath, Lazy state, bool isDotnetToolsPlugin) : this(filePath, state) + { + IsDotnetToolsPlugin = isDotnetToolsPlugin; + } + /// /// Instantiates a new class. /// diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt index 9f62b32fec7..e6677064e57 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,2 +1,4 @@ #nullable enable ~NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer(string rawPluginPaths) -> void +NuGet.Protocol.Plugins.PluginFile.IsDotnetToolsPlugin.get -> bool +~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy state, bool isDotnetToolsPlugin) -> void diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 9f62b32fec7..e6677064e57 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,2 +1,4 @@ #nullable enable ~NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer(string rawPluginPaths) -> void +NuGet.Protocol.Plugins.PluginFile.IsDotnetToolsPlugin.get -> bool +~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy state, bool isDotnetToolsPlugin) -> void diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index 9f62b32fec7..e6677064e57 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,2 +1,4 @@ #nullable enable ~NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer(string rawPluginPaths) -> void +NuGet.Protocol.Plugins.PluginFile.IsDotnetToolsPlugin.get -> bool +~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy state, bool isDotnetToolsPlugin) -> void diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index f7d486e37c7..505b75c73aa 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -8,6 +8,8 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Moq; +using NuGet.Common; using NuGet.Test.Utility; using Xunit; @@ -166,14 +168,15 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginWindows_FindsThePlugin var pluginPath = Path.Combine(testDirectory.Path, "myPlugin"); Directory.CreateDirectory(pluginPath); var myPlugin = Path.Combine(pluginPath, fileName); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(It.IsAny())).Returns(pluginPath); - Environment.SetEnvironmentVariable("PATH", pluginPath); File.WriteAllText(myPlugin, string.Empty); var verifierSpy = new Mock(); verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object)) + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -202,14 +205,15 @@ public async Task DiscoverAsync_withInValidDotNetToolsPluginNameWindows_DoesNotF var pluginPath = Path.Combine(testDirectory.Path, "myPlugin"); Directory.CreateDirectory(pluginPath); var myPlugin = Path.Combine(pluginPath, fileName); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(It.IsAny())).Returns(pluginPath); - Environment.SetEnvironmentVariable("PATH", pluginPath); File.WriteAllText(myPlugin, string.Empty); var verifierSpy = new Mock(); verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object)) + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -237,6 +241,8 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() Directory.CreateDirectory(pluginPath); var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); File.WriteAllText(myPlugin, string.Empty); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(It.IsAny())).Returns(pluginPath); using (var process = new Process()) { @@ -250,12 +256,11 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() if (process.ExitCode == 0) { - Environment.SetEnvironmentVariable("PATH", pluginPath); var verifierSpy = new Mock(); verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object)) + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -285,6 +290,8 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does Directory.CreateDirectory(pluginPath); var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); File.WriteAllText(myPlugin, string.Empty); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(It.IsAny())).Returns(pluginPath); using (var process = new Process()) { @@ -298,12 +305,11 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does if (process.ExitCode == 0) { - Environment.SetEnvironmentVariable("PATH", pluginPath); var verifierSpy = new Mock(); verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object)) + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); From 346b6d657665853dc526464adea8668c2ffeb5a2 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 12 Sep 2024 11:42:15 -0700 Subject: [PATCH 16/47] Add NUGET_PLUGIN_PATHS --- .../Plugins/PluginDiscoverer.cs | 14 +-- .../Plugins/PluginDiscovererTests.cs | 87 +++++++++++++++++++ 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 24cecc2b75a..daeeef2bc60 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -24,7 +24,7 @@ public sealed class PluginDiscoverer : IPluginDiscoverer private IEnumerable _results; private readonly SemaphoreSlim _semaphore; private readonly EmbeddedSignatureVerifier _verifier; - private IEnvironmentVariableReader _environmentVariableReader; + private readonly IEnvironmentVariableReader _environmentVariableReader = EnvironmentVariableWrapper.Instance; internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verifier, IEnvironmentVariableReader environmentVariableReader) : this(rawPluginPaths, verifier) { @@ -145,13 +145,17 @@ private List GetNetToolsPluginFiles() { var pluginFiles = new List(); - if (_environmentVariableReader is null) + string[] paths = Array.Empty(); + + // The path to the plugins installed using dotnet tools, should be specified in the NUGET_PLUGIN_PATHS environment variable. + var envNuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable("NUGET_PLUGIN_PATHS")?.Split(Path.PathSeparator) ?? Array.Empty(); + + if (envNuGetPluginPaths.Length == 0) { - _environmentVariableReader = EnvironmentVariableWrapper.Instance; + // If NUGET_PLUGIN_PATHS is not specified, read all the paths in PATH + paths = _environmentVariableReader.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); } - var paths = _environmentVariableReader.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); - foreach (var path in paths) { if (Directory.Exists(path)) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 505b75c73aa..4c22df9344a 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -194,6 +194,43 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginWindows_FindsThePlugin } } + [PlatformTheory(Platform.Windows)] + [InlineData("nuget-plugin-myPlugin.exe")] + [InlineData("nuget-plugin-myPlugin.bat")] + public async Task DiscoverAsync_WithPluginPathSpecifiedInNuGetPluginPathsEnvVariableWindows_FindsThePlugin(string fileName) + { + using (var testDirectory = TestDirectory.Create()) + { + // Arrange + var pluginPath = Path.Combine(testDirectory.Path, "myPlugin"); + Directory.CreateDirectory(pluginPath); + var myPlugin = Path.Combine(pluginPath, fileName); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("NUGET_PLUGIN_PATHS")).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATHS")).Returns(""); + File.WriteAllText(myPlugin, string.Empty); + var verifierSpy = new Mock(); + verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) + .Returns(true); + + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) + { + // Act + var result = await discoverer.DiscoverAsync(CancellationToken.None); + + // Assert + var discovered = false; + + foreach (PluginDiscoveryResult discoveryResult in result) + { + if (myPlugin == discoveryResult.PluginFile.Path) discovered = true; + } + + Assert.True(discovered); + } + } + } + [PlatformTheory(Platform.Windows)] [InlineData("nugetplugin-myPlugin.exe")] [InlineData("nugetplugin-myPlugin.bat")] @@ -280,6 +317,56 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() } } + [PlatformFact(Platform.Linux)] + public async Task DiscoverAsync_WithPluginPathSpecifiedInNuGetPluginPathsEnvVariableLinux_FindsThePlugin() + { + using (var testDirectory = TestDirectory.Create()) + { + // Arrange + var pluginPath = Path.Combine(testDirectory.Path, "myPlugins"); + Directory.CreateDirectory(pluginPath); + var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); + File.WriteAllText(myPlugin, string.Empty); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("NUGET_PLUGIN_PATHS")).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATHS")).Returns(""); + + using (var process = new Process()) + { + // Use a shell command to make the file executable + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"chmod +x {myPlugin}"; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + process.WaitForExit(); + + if (process.ExitCode == 0) + { + var verifierSpy = new Mock(); + verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) + .Returns(true); + + using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) + { + // Act + var result = await discoverer.DiscoverAsync(CancellationToken.None); + + // Assert + var discovered = false; + + foreach (PluginDiscoveryResult discoveryResult in result) + { + if (myPlugin == discoveryResult.PluginFile.Path) discovered = true; + } + + Assert.True(discovered); + } + } + } + } + } + [PlatformFact(Platform.Linux)] public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_DoesNotFindThePlugin() { From ce0b9b456a56b0d594724afa6e2b59b2b8a60b00 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 12 Sep 2024 12:07:38 -0700 Subject: [PATCH 17/47] Clean code --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index daeeef2bc60..d4fd32efa48 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -148,9 +148,9 @@ private List GetNetToolsPluginFiles() string[] paths = Array.Empty(); // The path to the plugins installed using dotnet tools, should be specified in the NUGET_PLUGIN_PATHS environment variable. - var envNuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable("NUGET_PLUGIN_PATHS")?.Split(Path.PathSeparator) ?? Array.Empty(); + paths = _environmentVariableReader.GetEnvironmentVariable("NUGET_PLUGIN_PATHS")?.Split(Path.PathSeparator) ?? Array.Empty(); - if (envNuGetPluginPaths.Length == 0) + if (paths.Length == 0) { // If NUGET_PLUGIN_PATHS is not specified, read all the paths in PATH paths = _environmentVariableReader.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); From ae23b52fc9ec9084fe5dc2fa94ac67c7a5c0a74d Mon Sep 17 00:00:00 2001 From: Nigusu Solomon Yenework <59111203+Nigusu-Allehu@users.noreply.github.com> Date: Thu, 12 Sep 2024 13:33:28 -0700 Subject: [PATCH 18/47] use empty body constructor --- .../NuGet.Protocol/Plugins/PluginDiscoverer.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index d4fd32efa48..b279a461463 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -24,11 +24,11 @@ public sealed class PluginDiscoverer : IPluginDiscoverer private IEnumerable _results; private readonly SemaphoreSlim _semaphore; private readonly EmbeddedSignatureVerifier _verifier; - private readonly IEnvironmentVariableReader _environmentVariableReader = EnvironmentVariableWrapper.Instance; + private readonly IEnvironmentVariableReader _environmentVariableReader; - internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verifier, IEnvironmentVariableReader environmentVariableReader) : this(rawPluginPaths, verifier) + public PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verifier) + : this(rawPluginPaths, verifier, EnvironmentVariableWrapper.Instance) { - _environmentVariableReader = environmentVariableReader; } /// @@ -36,9 +36,14 @@ internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verif /// /// The raw semicolon-delimited list of supposed plugin file paths. public PluginDiscoverer(string rawPluginPaths) + /// An embedded signature verifier. + /// A reader for environmental varibales. + /// Thrown if is . + internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verifier, IEnvironmentVariableReader environmentVariableReader) { _rawPluginPaths = rawPluginPaths; _semaphore = new SemaphoreSlim(initialCount: 1, maxCount: 1); + _environmentVariableReader = environmentVariableReader; } /// From 0082b0c23e8ed22f145ee5432b8fd9f96bd88772 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Fri, 13 Sep 2024 15:14:55 -0700 Subject: [PATCH 19/47] NuGetPluginPaths Env variable used by dotnet tools only --- .../NuGet.Protocol/Plugins/PluginDiscoverer.cs | 11 ++++++++--- .../NuGet.Protocol/Plugins/PluginManager.cs | 5 ----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index b279a461463..9e671356887 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -91,7 +91,13 @@ public async Task> DiscoverAsync(Cancellation } _pluginFiles = GetPluginFiles(cancellationToken); - _pluginFiles.AddRange(GetNetToolsPluginFiles()); + + if (string.IsNullOrEmpty(_rawPluginPaths)) + { + // Nuget plugin configuration environmental variables were not used to discover the configuration files + _pluginFiles.AddRange(GetNetToolsPluginFiles()); + } + var results = new List(); for (var i = 0; i < _pluginFiles.Count; ++i) @@ -149,11 +155,10 @@ private List GetPluginFiles(CancellationToken cancellationToken) private List GetNetToolsPluginFiles() { var pluginFiles = new List(); - string[] paths = Array.Empty(); // The path to the plugins installed using dotnet tools, should be specified in the NUGET_PLUGIN_PATHS environment variable. - paths = _environmentVariableReader.GetEnvironmentVariable("NUGET_PLUGIN_PATHS")?.Split(Path.PathSeparator) ?? Array.Empty(); + paths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)?.Split(Path.PathSeparator) ?? Array.Empty(); if (paths.Length == 0) { diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs index 53dfa731769..bc4248b1280 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs @@ -317,11 +317,6 @@ private void Initialize(IEnvironmentVariableReader reader, #else _rawPluginPaths = reader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); #endif - if (string.IsNullOrEmpty(_rawPluginPaths)) - { - _rawPluginPaths = reader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); - } - _connectionOptions = ConnectionOptions.CreateDefault(reader); var idleTimeoutInSeconds = EnvironmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.IdleTimeout); From b0cbeba9130ed6e07585712e6b90a0db97446745 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Sun, 6 Oct 2024 14:54:52 -0700 Subject: [PATCH 20/47] Test --- .../Plugins/PluginDiscoverer.cs | 10 +- .../Plugins/PluginDiscovererTests.cs | 166 ++++++++++++++++++ 2 files changed, 171 insertions(+), 5 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 9e671356887..e300ebf87b4 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -152,7 +152,7 @@ private List GetPluginFiles(CancellationToken cancellationToken) /// The files are also expected to have a name that starts with `nuget-plugin-` /// /// - private List GetNetToolsPluginFiles() + internal List GetNetToolsPluginFiles() { var pluginFiles = new List(); string[] paths = Array.Empty(); @@ -177,8 +177,8 @@ private List GetNetToolsPluginFiles() { if (IsValidPluginFile(file)) { - var state = new Lazy(() => _verifier.IsValid(file.FullName) ? PluginFileState.Valid : PluginFileState.InvalidEmbeddedSignature); - PluginFile pluginFile = new PluginFile(file.FullName, state, isDotnetToolsPlugin: true); + // .NET SDK recently package signature verification for .NET tools, as a result we expect them to be valid. + PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); pluginFiles.Add(pluginFile); } } @@ -195,7 +195,7 @@ private List GetNetToolsPluginFiles() /// /// /// - private static bool IsValidPluginFile(FileInfo fileInfo) + internal static bool IsValidPluginFile(FileInfo fileInfo) { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { @@ -224,7 +224,7 @@ private static bool IsValidPluginFile(FileInfo fileInfo) /// /// /// - private static bool IsExecutable(FileInfo fileInfo) + internal static bool IsExecutable(FileInfo fileInfo) { #pragma warning disable CA1031 // Do not catch general exception types try diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 4c22df9344a..70cd7d0ee30 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -416,6 +416,172 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does } } + [Fact] + public void GetNetToolsPluginFiles_WithNuGetPluginPaths_ReturnsPluginsInNuGetPluginPathOnly() + { + // Arrange + TestDirectory pluginPathDirectory = TestDirectory.Create(); + TestDirectory pathDirectory = TestDirectory.Create(); + var pluginInNuGetPluginPathDirectoryFilePath = Path.Combine(pluginPathDirectory.Path, "nuget-plugin-auth.exe"); + var pluginInPathDirectoryFilePath = Path.Combine(pathDirectory.Path, "nuget-plugin-in-path-directory.exe"); + File.Create(pluginInNuGetPluginPathDirectoryFilePath); + File.Create(pluginInPathDirectoryFilePath); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(Directory.GetParent(pluginInNuGetPluginPathDirectoryFilePath).FullName); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(Directory.GetParent(pluginInPathDirectoryFilePath).FullName); + PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetNetToolsPluginFiles(); + + // Assert + Assert.Single(plugins); + Assert.Equal(pluginInNuGetPluginPathDirectoryFilePath, plugins[0].Path); + } + + [Fact] + public void GetNetToolsPluginFiles_NoNuGetPluginPaths_UsesPathEnvironment() + { + // Arrange + TestDirectory testDirectory = TestDirectory.Create(); + var workingPath = testDirectory.Path; + var pluginFilePath = Path.Combine(workingPath, "nuget-plugin-auth.exe"); + File.Create(pluginFilePath); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(Directory.GetParent(pluginFilePath).FullName); + PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetNetToolsPluginFiles(); + + // Assert + Assert.Single(plugins); + Assert.Equal(pluginFilePath, plugins[0].Path); + } + + [Fact] + public void GetNetToolsPluginFiles_NoPluginsFound_ReturnsEmptyList() + { + // Arrange + TestDirectory testDirectory = TestDirectory.Create(); + var workingPath = testDirectory.Path; + Mock environmentalVariableReader = new Mock(); + PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetNetToolsPluginFiles(); + + // Assert + Assert.Empty(plugins); + } + + [PlatformFact(Platform.Windows)] + public void IsValidPluginFile_ExeFile_ReturnsTrue() + { + // Arrange + TestDirectory testDirectory = TestDirectory.Create(); + var workingPath = testDirectory.Path; + var pluginFilePath = Path.Combine(workingPath, "plugin.exe"); + File.Create(pluginFilePath); + var fileInfo = new FileInfo(pluginFilePath); + + // Act + bool result = PluginDiscoverer.IsValidPluginFile(fileInfo); + + // Assert + Assert.True(result); + } + + [PlatformFact(Platform.Windows)] + public void IsValidPluginFile_Windows_NonExecutableFile_ReturnsFalse() + { + // Arrange + TestDirectory testDirectory = TestDirectory.Create(); + var workingPath = testDirectory.Path; + var nonPluginFilePath = Path.Combine(workingPath, "plugin.txt"); + File.Create(nonPluginFilePath); + var fileInfo = new FileInfo(nonPluginFilePath); + + // Act + bool result = PluginDiscoverer.IsValidPluginFile(fileInfo); + + // Assert + Assert.False(result); + } + + [PlatformFact(Platform.Linux)] + public void IsValidPluginFile_Unix_ExecutableFile_ReturnsTrue() + { + // Arrange + TestDirectory testDirectory = TestDirectory.Create(); + var workingPath = testDirectory.Path; + var pluginFilePath = Path.Combine(workingPath, "plugin"); + File.Create(pluginFilePath).Dispose(); + +#if NET8_0_OR_GREATER + // Set execute permissions + File.SetUnixFileMode(pluginFilePath, UnixFileMode.UserExecute | UnixFileMode.UserRead); +#else + // Use chmod to set execute permissions + var chmodProcess = Process.Start("chmod", $"+x {pluginFilePath}"); + chmodProcess.WaitForExit(); +#endif + + var fileInfo = new FileInfo(pluginFilePath); + + // Act + bool result = PluginDiscoverer.IsValidPluginFile(fileInfo); + + // Assert + Assert.True(result); + } + +#if !NET8_0_OR_GREATER + [PlatformFact(Platform.Linux)] + public void IsExecutable_FileIsExecutable_ReturnsTrue() + { + // Arrange + TestDirectory testDirectory = TestDirectory.Create(); + var workingPath = testDirectory.Path; + var pluginFilePath = Path.Combine(workingPath, "plugin"); + File.Create(pluginFilePath); + + // Set execute permissions + var chmodProcess = Process.Start("chmod", $"+x {pluginFilePath}"); + chmodProcess.WaitForExit(); + + var fileInfo = new FileInfo(pluginFilePath); + + // Act + bool result = PluginDiscoverer.IsExecutable(fileInfo); + + // Assert + Assert.True(result); + } + + [PlatformFact(Platform.Linux)] + public void IsExecutable_FileIsNotExecutable_ReturnsFalse() + { + // Arrange + TestDirectory testDirectory = TestDirectory.Create(); + var workingPath = testDirectory.Path; + var pluginFilePath = Path.Combine(workingPath, "plugin"); + File.Create(pluginFilePath); + + // Remove execute permissions + var chmodProcess = Process.Start("chmod", $"-x {pluginFilePath}"); + chmodProcess.WaitForExit(); + + var fileInfo = new FileInfo(pluginFilePath); + + // Act + bool result = PluginDiscoverer.IsExecutable(fileInfo); + + // Assert + Assert.False(result); + } +#endif + private sealed class EmbeddedSignatureVerifierStub : EmbeddedSignatureVerifier { private readonly Dictionary _responses; From f68c137c96609c22392f3ea05abee36b781a3ae3 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Sun, 6 Oct 2024 15:12:58 -0700 Subject: [PATCH 21/47] Clean up --- .../NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 70cd7d0ee30..c0cd0f71bc4 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -416,7 +416,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does } } - [Fact] + [PlatformFact(Platform.Windows)] public void GetNetToolsPluginFiles_WithNuGetPluginPaths_ReturnsPluginsInNuGetPluginPathOnly() { // Arrange @@ -439,7 +439,7 @@ public void GetNetToolsPluginFiles_WithNuGetPluginPaths_ReturnsPluginsInNuGetPlu Assert.Equal(pluginInNuGetPluginPathDirectoryFilePath, plugins[0].Path); } - [Fact] + [PlatformFact(Platform.Windows)] public void GetNetToolsPluginFiles_NoNuGetPluginPaths_UsesPathEnvironment() { // Arrange From 83295008ecbd21d595f867b2b2068c92ed1577e9 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Sun, 6 Oct 2024 15:26:18 -0700 Subject: [PATCH 22/47] fix process --- .../Plugins/PluginDiscovererTests.cs | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index c0cd0f71bc4..d1256dc8158 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -523,8 +523,13 @@ public void IsValidPluginFile_Unix_ExecutableFile_ReturnsTrue() File.SetUnixFileMode(pluginFilePath, UnixFileMode.UserExecute | UnixFileMode.UserRead); #else // Use chmod to set execute permissions - var chmodProcess = Process.Start("chmod", $"+x {pluginFilePath}"); - chmodProcess.WaitForExit(); + var process = new Process(); + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"chmod +x {pluginFilePath}"; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + process.WaitForExit(); #endif var fileInfo = new FileInfo(pluginFilePath); @@ -547,8 +552,13 @@ public void IsExecutable_FileIsExecutable_ReturnsTrue() File.Create(pluginFilePath); // Set execute permissions - var chmodProcess = Process.Start("chmod", $"+x {pluginFilePath}"); - chmodProcess.WaitForExit(); + var process = new Process(); + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"chmod +x {pluginFilePath}"; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + process.WaitForExit(); var fileInfo = new FileInfo(pluginFilePath); @@ -569,8 +579,13 @@ public void IsExecutable_FileIsNotExecutable_ReturnsFalse() File.Create(pluginFilePath); // Remove execute permissions - var chmodProcess = Process.Start("chmod", $"-x {pluginFilePath}"); - chmodProcess.WaitForExit(); + var process = new Process(); + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"chmod -x {pluginFilePath}"; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + process.WaitForExit(); var fileInfo = new FileInfo(pluginFilePath); From 4e8effc392a372a61406792c63d5a1bb59a88320 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 8 Oct 2024 13:55:55 -0700 Subject: [PATCH 23/47] Plugins path env --- .../Plugins/PluginDiscoverer.cs | 63 +++++--- .../NuGet.Protocol/Plugins/PluginManager.cs | 14 +- .../Plugins/PluginDiscovererTests.cs | 141 +++++++++++++++++- 3 files changed, 187 insertions(+), 31 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index e300ebf87b4..3291086e01d 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -20,7 +20,7 @@ public sealed class PluginDiscoverer : IPluginDiscoverer { private bool _isDisposed; private List _pluginFiles; - private readonly string _rawPluginPaths; + private readonly string _envVariablePluginPaths; private IEnumerable _results; private readonly SemaphoreSlim _semaphore; private readonly EmbeddedSignatureVerifier _verifier; @@ -42,6 +42,13 @@ public PluginDiscoverer(string rawPluginPaths) internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verifier, IEnvironmentVariableReader environmentVariableReader) { _rawPluginPaths = rawPluginPaths; + if (verifier == null) + { + throw new ArgumentNullException(nameof(verifier)); + } + + _envVariablePluginPaths = rawPluginPaths; + _verifier = verifier; _semaphore = new SemaphoreSlim(initialCount: 1, maxCount: 1); _environmentVariableReader = environmentVariableReader; } @@ -92,10 +99,11 @@ public async Task> DiscoverAsync(Cancellation _pluginFiles = GetPluginFiles(cancellationToken); - if (string.IsNullOrEmpty(_rawPluginPaths)) + if (string.IsNullOrEmpty(_envVariablePluginPaths)) { - // Nuget plugin configuration environmental variables were not used to discover the configuration files - _pluginFiles.AddRange(GetNetToolsPluginFiles()); + // Nuget plugin configuration environmental variables: NUGET_NETFX_PLUGIN_PATHS, NUGET_NETCORE_PLUGIN_PATHS + // were not used to point to the credential provider plugins. + _pluginFiles.AddRange(GetPluginsInNuGetPluginPathsAndPath()); } var results = new List(); @@ -147,28 +155,45 @@ private List GetPluginFiles(CancellationToken cancellationToken) } /// - /// Gets auth plugins installed using dotnet tools. This is done by iterating through each file in directories found int eh - /// `PATH` environment variable. - /// The files are also expected to have a name that starts with `nuget-plugin-` + /// Retrieves authentication plugins by searching through directories specified in the `NuGET_PLUGIN_PATHS` and `PATH` + /// environment variables. The method looks for files prefixed with 'nuget-plugin-' and verifies their validity for .net tools plugins. /// - /// - internal List GetNetToolsPluginFiles() + /// A list of valid objects representing the discovered plugins. + internal List GetPluginsInNuGetPluginPathsAndPath() { var pluginFiles = new List(); - string[] paths = Array.Empty(); + var nugetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); + string[] paths; - // The path to the plugins installed using dotnet tools, should be specified in the NUGET_PLUGIN_PATHS environment variable. - paths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)?.Split(Path.PathSeparator) ?? Array.Empty(); - - if (paths.Length == 0) + if (string.IsNullOrEmpty(nugetPluginPaths)) { - // If NUGET_PLUGIN_PATHS is not specified, read all the paths in PATH - paths = _environmentVariableReader.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); + nugetPluginPaths = _environmentVariableReader.GetEnvironmentVariable("PATH"); } + paths = nugetPluginPaths?.Split(Path.PathSeparator) ?? Array.Empty(); + foreach (var path in paths) { - if (Directory.Exists(path)) + if (File.Exists(path)) + { + FileInfo fileInfo = new FileInfo(path); + if (fileInfo.Name.StartsWith("nuget-plugin-", StringComparison.CurrentCultureIgnoreCase)) + { + // A DotNet tool plugin + if (IsValidPluginFile(fileInfo)) + { + PluginFile pluginFile = new PluginFile(fileInfo.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); + pluginFiles.Add(pluginFile); + } + } + else + { + // A non DotNet tool plugin file + var state = new Lazy(() => _verifier.IsValid(fileInfo.FullName) ? PluginFileState.Valid : PluginFileState.InvalidEmbeddedSignature); + pluginFiles.Add(new PluginFile(fileInfo.FullName, state)); + } + } + else if (Directory.Exists(path)) { var directoryInfo = new DirectoryInfo(path); var files = directoryInfo.GetFiles("nuget-plugin-*"); @@ -260,7 +285,7 @@ internal static bool IsExecutable(FileInfo fileInfo) private IEnumerable GetPluginFilePaths() { - if (string.IsNullOrEmpty(_rawPluginPaths)) + if (string.IsNullOrEmpty(_envVariablePluginPaths)) { var directories = new List { PluginDiscoveryUtility.GetNuGetHomePluginsPath() }; #if IS_DESKTOP @@ -270,7 +295,7 @@ private IEnumerable GetPluginFilePaths() return PluginDiscoveryUtility.GetConventionBasedPlugins(directories); } - return _rawPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); + return _envVariablePluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); } } } diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs index bc4248b1280..d77fb0cdc9e 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs @@ -33,7 +33,7 @@ public sealed class PluginManager : IPluginManager, IDisposable private IPluginFactory _pluginFactory; private ConcurrentDictionary>>> _pluginOperationClaims; private ConcurrentDictionary> _pluginUtilities; - private string _rawPluginPaths; + private string _envVariablePluginPaths; private static Lazy _currentProcessId = new Lazy(GetCurrentProcessId); private Lazy _pluginsCacheDirectoryPath; @@ -119,7 +119,7 @@ public async Task> CreatePluginsAsync( var pluginCreationResults = new List(); // Fast path - if (source.PackageSource.IsHttp && IsPluginPossiblyAvailable()) + if (source.PackageSource.IsHttp) { var serviceIndex = await source.GetResourceAsync(cancellationToken); @@ -313,9 +313,9 @@ private void Initialize(IEnvironmentVariableReader reader, throw new ArgumentNullException(nameof(pluginFactoryCreator)); } #if IS_DESKTOP - _rawPluginPaths = reader.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths); + _envVariablePluginPaths = reader.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths); #else - _rawPluginPaths = reader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); + _envVariablePluginPaths = reader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); #endif _connectionOptions = ConnectionOptions.CreateDefault(reader); @@ -355,12 +355,14 @@ private async Task> GetPluginOperationClaimsAsync( private PluginDiscoverer InitializeDiscoverer() { - return new PluginDiscoverer(_rawPluginPaths); + var verifier = EmbeddedSignatureVerifier.Create(); + + return new PluginDiscoverer(_envVariablePluginPaths, verifier); } private bool IsPluginPossiblyAvailable() { - return !string.IsNullOrEmpty(_rawPluginPaths); + return !string.IsNullOrEmpty(_envVariablePluginPaths); } private void OnPluginClosed(object sender, EventArgs e) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index d1256dc8158..a3c935b31fd 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -417,7 +417,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does } [PlatformFact(Platform.Windows)] - public void GetNetToolsPluginFiles_WithNuGetPluginPaths_ReturnsPluginsInNuGetPluginPathOnly() + public void GetPluginsInNuGetPluginPathsAndPath_WithNuGetPluginPaths_ReturnsPluginsInNuGetPluginPathOnly() { // Arrange TestDirectory pluginPathDirectory = TestDirectory.Create(); @@ -432,15 +432,144 @@ public void GetNetToolsPluginFiles_WithNuGetPluginPaths_ReturnsPluginsInNuGetPlu PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetNetToolsPluginFiles(); + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); // Assert Assert.Single(plugins); Assert.Equal(pluginInNuGetPluginPathDirectoryFilePath, plugins[0].Path); } + [Fact] + public void GetPluginsInNuGetPluginPathsAndPath_WithoutNuGetPluginPaths_FallsBackToPath() + { + // Arrange + var pathDirectory = TestDirectory.Create(); + var pluginFilePath = Path.Combine(pathDirectory.Path, "nuget-plugin-fallback.exe"); + File.Create(pluginFilePath); + + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(string.Empty); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pathDirectory.Path); + + var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + + // Assert + Assert.Single(plugins); + Assert.Equal(pluginFilePath, plugins[0].Path); + } + + [PlatformFact(Platform.Windows)] + public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToAFile_TreatsAsPlugin() + { + // Arrange + var pluginFilePath = Path.Combine(TestDirectory.Create().Path, "nuget-plugin-auth.exe"); + File.Create(pluginFilePath); + + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginFilePath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); + + var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + + // Assert + Assert.Single(plugins); + Assert.Equal(pluginFilePath, plugins[0].Path); + } + + [PlatformFact(Platform.Windows)] + public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToAFileThatDoesNotStartWithNugetPlugin_ReturnsNonDotnetPlugin() + { + // Arrange + var pluginFilePath = Path.Combine(TestDirectory.Create().Path, "other-plugin.exe"); + File.Create(pluginFilePath); + + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginFilePath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); + + var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + + // Assert + Assert.Single(plugins); + Assert.False(plugins[0].IsDotnetToolsPlugin); + } + + [PlatformFact(Platform.Windows)] + public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToADirectory_ContainsValidPluginFiles() + { + // Arrange + var pluginPathDirectory = TestDirectory.Create(); + var validPluginFile = Path.Combine(pluginPathDirectory.Path, "nuget-plugin-auth.exe"); + var invalidPluginFile = Path.Combine(pluginPathDirectory.Path, "not-a-nuget-plugin.exe"); + File.Create(validPluginFile); + File.Create(invalidPluginFile); + + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginPathDirectory.Path); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); + + var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + + // Assert + Assert.Single(plugins); + Assert.Equal(validPluginFile, plugins[0].Path); + } + + [PlatformFact(Platform.Windows)] + public void GetPluginsInNuGetPluginPathsAndPath_NoEnvironmentVariables_ReturnsNoPlugins() + { + // Arrange + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(string.Empty); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); + + var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + + // Assert + Assert.Empty(plugins); + } + + [PlatformFact(Platform.Windows)] + public void GetPluginsInNuGetPluginPathsAndPath_PathContainsDirectoriesWithValidAndInvalidPlugins_ReturnsOnlyValidPlugins() + { + // Arrange + var pathDirectory = TestDirectory.Create(); + var validPluginFile = Path.Combine(pathDirectory.Path, "nuget-plugin-valid.exe"); + var invalidPluginFile = Path.Combine(pathDirectory.Path, "random-file.exe"); + File.Create(validPluginFile); + File.Create(invalidPluginFile); + + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(string.Empty); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pathDirectory.Path); + + var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + + // Assert + Assert.Single(plugins); + Assert.Equal(validPluginFile, plugins[0].Path); + } + [PlatformFact(Platform.Windows)] - public void GetNetToolsPluginFiles_NoNuGetPluginPaths_UsesPathEnvironment() + public void GetPluginsInNuGetPluginPathsAndPath_NoNuGetPluginPaths_UsesPathEnvironment() { // Arrange TestDirectory testDirectory = TestDirectory.Create(); @@ -452,7 +581,7 @@ public void GetNetToolsPluginFiles_NoNuGetPluginPaths_UsesPathEnvironment() PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetNetToolsPluginFiles(); + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); // Assert Assert.Single(plugins); @@ -460,7 +589,7 @@ public void GetNetToolsPluginFiles_NoNuGetPluginPaths_UsesPathEnvironment() } [Fact] - public void GetNetToolsPluginFiles_NoPluginsFound_ReturnsEmptyList() + public void GetPluginsInNuGetPluginPathsAndPath_NoPluginsFound_ReturnsEmptyList() { // Arrange TestDirectory testDirectory = TestDirectory.Create(); @@ -469,7 +598,7 @@ public void GetNetToolsPluginFiles_NoPluginsFound_ReturnsEmptyList() PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetNetToolsPluginFiles(); + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); // Assert Assert.Empty(plugins); From c273b7b27c969a4f96b7bfb334edb9a6e343924d Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 8 Oct 2024 15:23:20 -0700 Subject: [PATCH 24/47] fix tests --- .../NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs | 2 +- .../NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index a3c935b31fd..de12630f1e7 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -439,7 +439,7 @@ public void GetPluginsInNuGetPluginPathsAndPath_WithNuGetPluginPaths_ReturnsPlug Assert.Equal(pluginInNuGetPluginPathDirectoryFilePath, plugins[0].Path); } - [Fact] + [PlatformFact(Platform.Windows)] public void GetPluginsInNuGetPluginPathsAndPath_WithoutNuGetPluginPaths_FallsBackToPath() { // Arrange diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs index 787f6b6db73..d436de575e7 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs @@ -267,6 +267,7 @@ internal PluginResourceProviderNegativeTest(string serviceIndexJson, string sour _pluginDiscoverer.Setup(x => x.DiscoverAsync(It.IsAny())) .ReturnsAsync(pluginDiscoveryResults); + _pluginDiscoverer.Setup(x => x.Dispose()); _testDirectory = TestDirectory.Create(); From 3d1e8781c7743c66b2332c4362d5d46c2b434900 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 8 Oct 2024 15:46:44 -0700 Subject: [PATCH 25/47] fix linux bash call --- .../Plugins/PluginDiscovererTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index de12630f1e7..1ee28aca304 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -285,7 +285,7 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() { // Use a shell command to make the file executable process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"chmod +x {myPlugin}"; + process.StartInfo.Arguments = $"-c \"chmod +x {myPlugin}\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; process.Start(); @@ -335,7 +335,7 @@ public async Task DiscoverAsync_WithPluginPathSpecifiedInNuGetPluginPathsEnvVari { // Use a shell command to make the file executable process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"chmod +x {myPlugin}"; + process.StartInfo.Arguments = $"-c \"chmod +x {myPlugin}\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; process.Start(); @@ -384,7 +384,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does { // Use a shell command to make the file not executable process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"chmod -x {myPlugin}"; + process.StartInfo.Arguments = $"-c \"chmod -x {myPlugin}\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; process.Start(); @@ -654,7 +654,7 @@ public void IsValidPluginFile_Unix_ExecutableFile_ReturnsTrue() // Use chmod to set execute permissions var process = new Process(); process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"chmod +x {pluginFilePath}"; + process.StartInfo.Arguments = $"-c \"chmod +x {pluginFilePath}\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; process.Start(); @@ -683,7 +683,7 @@ public void IsExecutable_FileIsExecutable_ReturnsTrue() // Set execute permissions var process = new Process(); process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"chmod +x {pluginFilePath}"; + process.StartInfo.Arguments = $"-c \"chmod +x {pluginFilePath}\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; process.Start(); @@ -710,7 +710,7 @@ public void IsExecutable_FileIsNotExecutable_ReturnsFalse() // Remove execute permissions var process = new Process(); process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"chmod -x {pluginFilePath}"; + process.StartInfo.Arguments = $"-c \"chmod -x {pluginFilePath}\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; process.Start(); From e3c1b57e739aa6f8cd5d822381b2a74f33daa6db Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 8 Oct 2024 16:10:22 -0700 Subject: [PATCH 26/47] fix bash command --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 3291086e01d..a0d9111fb37 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -259,7 +259,7 @@ internal static bool IsExecutable(FileInfo fileInfo) { // Use a shell command to check if the file is executable process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"if [ -x {fileInfo.FullName} ]; then echo yes; else echo no; fi"; + process.StartInfo.Arguments = $" -c \"if [ -x {fileInfo.FullName} ]; then echo yes; else echo no; fi\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; From bceba8ebb0c3b42bceda2a82722646fddcf22355 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 8 Oct 2024 16:30:06 -0700 Subject: [PATCH 27/47] remove some apis --- .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 2 -- src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/NuGet.Core/NuGet.Credentials/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Credentials/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 53b5306a726..7dc5c58110b 100644 --- a/src/NuGet.Core/NuGet.Credentials/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Credentials/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,3 +1 @@ #nullable enable -NuGet.Protocol.Plugins.PluginFile.IsDotnetToolsPlugin.get -> bool -~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy state, bool isDotnetToolsPlugin) -> void diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs index 9f7f2599b06..96c24f2c86f 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs @@ -23,7 +23,7 @@ public sealed class PluginFile /// /// Is the plugin file, a dotnet tools plugin file? /// - public bool IsDotnetToolsPlugin { get; } + internal bool IsDotnetToolsPlugin { get; } /// /// Instantiates a new class. @@ -31,7 +31,7 @@ public sealed class PluginFile /// The plugin's file path. /// A lazy that evaluates the plugin file state. /// Is the plugin file, a dotnet tools plugin file? - public PluginFile(string filePath, Lazy state, bool isDotnetToolsPlugin) : this(filePath, state) + internal PluginFile(string filePath, Lazy state, bool isDotnetToolsPlugin) : this(filePath, state) { IsDotnetToolsPlugin = isDotnetToolsPlugin; } From b2a23e176ae887739c96ff0a415b61c120456eea Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 8 Oct 2024 16:35:31 -0700 Subject: [PATCH 28/47] remove symlink check --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index a0d9111fb37..e00f0a6e706 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -237,7 +237,7 @@ internal static bool IsValidPluginFile(FileInfo fileInfo) (fileMode & UnixFileMode.GroupExecute) != 0 || (fileMode & UnixFileMode.OtherExecute) != 0); #else - return fileInfo.Exists && (fileInfo.Attributes & FileAttributes.ReparsePoint) == 0 && IsExecutable(fileInfo); + return fileInfo.Exists && IsExecutable(fileInfo); #endif } } From 6cec63b73c98b87f61bc36628ac8ea45c6fac50a Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 8 Oct 2024 20:33:32 -0700 Subject: [PATCH 29/47] cleanup --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index e00f0a6e706..5dbe2d074f4 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; -using System.Linq; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; From daccb030cd22789bf065a9588b1b1a6471378dc0 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 8 Oct 2024 20:47:45 -0700 Subject: [PATCH 30/47] cleanup --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 5dbe2d074f4..cfbe500a63a 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; using System.Threading; @@ -254,7 +253,7 @@ internal static bool IsExecutable(FileInfo fileInfo) try { string output; - using (var process = new Process()) + using (var process = new System.Diagnostics.Process()) { // Use a shell command to check if the file is executable process.StartInfo.FileName = "/bin/bash"; From 97c1b2382a374daf2dc580bd0a9aba42afb3bc46 Mon Sep 17 00:00:00 2001 From: Nigusu Solomon Yenework <59111203+Nigusu-Allehu@users.noreply.github.com> Date: Wed, 9 Oct 2024 11:39:12 -0700 Subject: [PATCH 31/47] cleanup --- src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj b/src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj index fe7eb1a2a0c..4b075d8c266 100644 --- a/src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj +++ b/src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj @@ -1,6 +1,6 @@ - $(TargetFrameworksLibraryForSigning);$(NETCoreTargetFramework) + $(TargetFrameworksLibraryForSigning) $(NoWarn);CS1591;CS1573;CS0012;RS0041 nuget protocol From 607744859ba691b00d7caa679ad9dd572c23d743 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Fri, 11 Oct 2024 16:33:48 -0700 Subject: [PATCH 32/47] comments --- .../NuGet.Protocol/Plugins/PluginDiscoverer.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index cfbe500a63a..aa311abea31 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -18,7 +18,8 @@ public sealed class PluginDiscoverer : IPluginDiscoverer { private bool _isDisposed; private List _pluginFiles; - private readonly string _envVariablePluginPaths; + private readonly string _envVariableNetCoreAndNetFXPluginPaths; + private readonly string _envVariableNuGetPluginPaths; private IEnumerable _results; private readonly SemaphoreSlim _semaphore; private readonly EmbeddedSignatureVerifier _verifier; @@ -45,10 +46,11 @@ internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verif throw new ArgumentNullException(nameof(verifier)); } - _envVariablePluginPaths = rawPluginPaths; + _environmentVariableReader = environmentVariableReader; + _envVariableNetCoreAndNetFXPluginPaths = rawPluginPaths; + _envVariableNuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); _verifier = verifier; _semaphore = new SemaphoreSlim(initialCount: 1, maxCount: 1); - _environmentVariableReader = environmentVariableReader; } /// @@ -97,11 +99,11 @@ public async Task> DiscoverAsync(Cancellation _pluginFiles = GetPluginFiles(cancellationToken); - if (string.IsNullOrEmpty(_envVariablePluginPaths)) + if (string.IsNullOrEmpty(_envVariableNetCoreAndNetFXPluginPaths)) { // Nuget plugin configuration environmental variables: NUGET_NETFX_PLUGIN_PATHS, NUGET_NETCORE_PLUGIN_PATHS // were not used to point to the credential provider plugins. - _pluginFiles.AddRange(GetPluginsInNuGetPluginPathsAndPath()); + _pluginFiles.AddRange(GetPluginsInNuGetPluginPathsAndPath() ?? new List()); } var results = new List(); @@ -283,7 +285,7 @@ internal static bool IsExecutable(FileInfo fileInfo) private IEnumerable GetPluginFilePaths() { - if (string.IsNullOrEmpty(_envVariablePluginPaths)) + if (string.IsNullOrEmpty(_envVariableNetCoreAndNetFXPluginPaths) && string.IsNullOrEmpty(_envVariableNuGetPluginPaths)) { var directories = new List { PluginDiscoveryUtility.GetNuGetHomePluginsPath() }; #if IS_DESKTOP @@ -293,7 +295,7 @@ private IEnumerable GetPluginFilePaths() return PluginDiscoveryUtility.GetConventionBasedPlugins(directories); } - return _envVariablePluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); + return _envVariableNetCoreAndNetFXPluginPaths != null ? _envVariableNetCoreAndNetFXPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries) : new List(); } } } From e655a610f942d35d504320e8f3b489a61774758d Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Sat, 12 Oct 2024 12:39:49 -0700 Subject: [PATCH 33/47] factorize and clean up --- .../Plugins/PluginDiscoverer.cs | 149 +++++++++++------- .../Plugins/PluginDiscovererTests.cs | 119 +++++--------- 2 files changed, 134 insertions(+), 134 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index aa311abea31..1d0a4b96c90 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -97,13 +97,31 @@ public async Task> DiscoverAsync(Cancellation return _results; } - _pluginFiles = GetPluginFiles(cancellationToken); - - if (string.IsNullOrEmpty(_envVariableNetCoreAndNetFXPluginPaths)) + if (!string.IsNullOrEmpty(_envVariableNetCoreAndNetFXPluginPaths)) + { + // NUGET_NETFX_PLUGIN_PATHS, NUGET_NETCORE_PLUGIN_PATHS have been set. + var filePaths = _envVariableNetCoreAndNetFXPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); + _pluginFiles = GetPluginFiles(filePaths, cancellationToken); + } + else if (!string.IsNullOrEmpty(_envVariableNuGetPluginPaths)) { - // Nuget plugin configuration environmental variables: NUGET_NETFX_PLUGIN_PATHS, NUGET_NETCORE_PLUGIN_PATHS - // were not used to point to the credential provider plugins. - _pluginFiles.AddRange(GetPluginsInNuGetPluginPathsAndPath() ?? new List()); + // NUGET_PLUGIN_PATHS has been set + _pluginFiles = GetPluginsInNuGetPluginPaths(); + } + else + { + // restore to default plugins search. + // Search for plugins in %user%/.nuget/plugins + var directories = new List { PluginDiscoveryUtility.GetNuGetHomePluginsPath() }; +#if IS_DESKTOP + // Internal plugins are only supported for .NET Framework scenarios, namely msbuild.exe + directories.Add(PluginDiscoveryUtility.GetInternalPlugins()); +#endif + var filePaths = PluginDiscoveryUtility.GetConventionBasedPlugins(directories); + _pluginFiles = GetPluginFiles((string[])filePaths, cancellationToken); + + // Search for .Net tools plugins in PATH + _pluginFiles.AddRange(GetPluginsInPATH() ?? new List()); } var results = new List(); @@ -127,12 +145,10 @@ public async Task> DiscoverAsync(Cancellation return _results; } - private List GetPluginFiles(CancellationToken cancellationToken) + private List GetPluginFiles(string[] filePaths, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - var filePaths = GetPluginFilePaths(); - var files = new List(); foreach (var filePath in filePaths) @@ -155,59 +171,97 @@ private List GetPluginFiles(CancellationToken cancellationToken) } /// - /// Retrieves authentication plugins by searching through directories specified in the `NuGET_PLUGIN_PATHS` and `PATH` - /// environment variables. The method looks for files prefixed with 'nuget-plugin-' and verifies their validity for .net tools plugins. + /// Retrieves authentication plugins by searching through directories and files specified in the `NuGET_PLUGIN_PATHS` + /// environment variable. The method looks for files prefixed with 'nuget-plugin-' and verifies their validity for .net tools plugins. /// /// A list of valid objects representing the discovered plugins. - internal List GetPluginsInNuGetPluginPathsAndPath() + internal List GetPluginsInNuGetPluginPaths() { var pluginFiles = new List(); - var nugetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); - string[] paths; - - if (string.IsNullOrEmpty(nugetPluginPaths)) - { - nugetPluginPaths = _environmentVariableReader.GetEnvironmentVariable("PATH"); - } - - paths = nugetPluginPaths?.Split(Path.PathSeparator) ?? Array.Empty(); + string[] paths = _envVariableNuGetPluginPaths?.Split(Path.PathSeparator) ?? Array.Empty(); foreach (var path in paths) { - if (File.Exists(path)) + if (PathValidator.IsValidLocalPath(path) || PathValidator.IsValidUncPath(path)) { - FileInfo fileInfo = new FileInfo(path); - if (fileInfo.Name.StartsWith("nuget-plugin-", StringComparison.CurrentCultureIgnoreCase)) + if (File.Exists(path)) { - // A DotNet tool plugin - if (IsValidPluginFile(fileInfo)) + FileInfo fileInfo = new FileInfo(path); + if (fileInfo.Name.StartsWith("nuget-plugin-", StringComparison.CurrentCultureIgnoreCase)) + { + // A DotNet tool plugin + if (IsValidPluginFile(fileInfo)) + { + PluginFile pluginFile = new PluginFile(fileInfo.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); + pluginFiles.Add(pluginFile); + } + } + else { - PluginFile pluginFile = new PluginFile(fileInfo.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); - pluginFiles.Add(pluginFile); + // A non DotNet tool plugin file + var state = new Lazy(() => _verifier.IsValid(fileInfo.FullName) ? PluginFileState.Valid : PluginFileState.InvalidEmbeddedSignature); + pluginFiles.Add(new PluginFile(fileInfo.FullName, state)); } } - else + else if (Directory.Exists(path)) { - // A non DotNet tool plugin file - var state = new Lazy(() => _verifier.IsValid(fileInfo.FullName) ? PluginFileState.Valid : PluginFileState.InvalidEmbeddedSignature); - pluginFiles.Add(new PluginFile(fileInfo.FullName, state)); + var directoryInfo = new DirectoryInfo(path); + var files = directoryInfo.GetFiles("nuget-plugin-*"); + + foreach (var file in files) + { + if (IsValidPluginFile(file)) + { + // .NET SDK recently package signature verification for .NET tools, as a result we expect them to be valid. + PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); + pluginFiles.Add(pluginFile); + } + } } } - else if (Directory.Exists(path)) + else { - var directoryInfo = new DirectoryInfo(path); - var files = directoryInfo.GetFiles("nuget-plugin-*"); + pluginFiles.Add(new PluginFile(path, new Lazy(() => PluginFileState.InvalidFilePath))); + } + } + + return pluginFiles; + } - foreach (var file in files) + /// + /// Retrieves .NET tools authentication plugins by searching through directories specified in `PATH` + /// + /// A list of valid objects representing the discovered plugins. + internal List GetPluginsInPATH() + { + var pluginFiles = new List(); + var nugetPluginPaths = _environmentVariableReader.GetEnvironmentVariable("PATH"); + string[] paths = nugetPluginPaths?.Split(Path.PathSeparator) ?? Array.Empty(); + + foreach (var path in paths) + { + if (PathValidator.IsValidLocalPath(path) || PathValidator.IsValidUncPath(path)) + { + if (Directory.Exists(path)) { - if (IsValidPluginFile(file)) + var directoryInfo = new DirectoryInfo(path); + var files = directoryInfo.GetFiles("nuget-plugin-*"); + + foreach (var file in files) { - // .NET SDK recently package signature verification for .NET tools, as a result we expect them to be valid. - PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); - pluginFiles.Add(pluginFile); + if (IsValidPluginFile(file)) + { + // .NET SDK recently package signature verification for .NET tools, as a result we expect them to be valid. + PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); + pluginFiles.Add(pluginFile); + } } } } + else + { + pluginFiles.Add(new PluginFile(path, new Lazy(() => PluginFileState.InvalidFilePath))); + } } return pluginFiles; @@ -282,20 +336,5 @@ internal static bool IsExecutable(FileInfo fileInfo) #pragma warning restore CA1031 // Do not catch general exception types } #endif - - private IEnumerable GetPluginFilePaths() - { - if (string.IsNullOrEmpty(_envVariableNetCoreAndNetFXPluginPaths) && string.IsNullOrEmpty(_envVariableNuGetPluginPaths)) - { - var directories = new List { PluginDiscoveryUtility.GetNuGetHomePluginsPath() }; -#if IS_DESKTOP - // Internal plugins are only supported for .NET Framework scenarios, namely msbuild.exe - directories.Add(PluginDiscoveryUtility.GetInternalPlugins()); -#endif - return PluginDiscoveryUtility.GetConventionBasedPlugins(directories); - } - - return _envVariableNetCoreAndNetFXPluginPaths != null ? _envVariableNetCoreAndNetFXPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries) : new List(); - } } } diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 1ee28aca304..7c3c22ebe0f 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -417,7 +417,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does } [PlatformFact(Platform.Windows)] - public void GetPluginsInNuGetPluginPathsAndPath_WithNuGetPluginPaths_ReturnsPluginsInNuGetPluginPathOnly() + public void GetPluginsInNuGetPluginPaths_WithNuGetPluginPathsSet_ReturnsPluginsInNuGetPluginPathOnly() { // Arrange TestDirectory pluginPathDirectory = TestDirectory.Create(); @@ -432,15 +432,16 @@ public void GetPluginsInNuGetPluginPathsAndPath_WithNuGetPluginPaths_ReturnsPlug PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); // Assert Assert.Single(plugins); Assert.Equal(pluginInNuGetPluginPathDirectoryFilePath, plugins[0].Path); + Assert.True(plugins[0].IsDotnetToolsPlugin); } [PlatformFact(Platform.Windows)] - public void GetPluginsInNuGetPluginPathsAndPath_WithoutNuGetPluginPaths_FallsBackToPath() + public void GetPluginsInNuGetPluginPaths_WithoutNuGetPluginPaths_ReturnsEmpty() { // Arrange var pathDirectory = TestDirectory.Create(); @@ -448,21 +449,43 @@ public void GetPluginsInNuGetPluginPathsAndPath_WithoutNuGetPluginPaths_FallsBac File.Create(pluginFilePath); var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(string.Empty); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pathDirectory.Path); var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); + + // Assert + Assert.Empty(plugins); + } + + [PlatformFact(Platform.Windows)] + public void GetPluginsInPATH_WithPATHSet_ReturnsPlugin() + { + // Arrange + TestDirectory pluginPathDirectory = TestDirectory.Create(); + TestDirectory pathDirectory = TestDirectory.Create(); + var pluginInNuGetPluginPathDirectoryFilePath = Path.Combine(pluginPathDirectory.Path, "nuget-plugin-auth.exe"); + var pluginInPathDirectoryFilePath = Path.Combine(pathDirectory.Path, "nuget-plugin-in-path-directory.exe"); + File.Create(pluginInNuGetPluginPathDirectoryFilePath); + File.Create(pluginInPathDirectoryFilePath); + Mock environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(Directory.GetParent(pluginInNuGetPluginPathDirectoryFilePath).FullName); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(Directory.GetParent(pluginInPathDirectoryFilePath).FullName); + PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + + // Act + var plugins = pluginDiscoverer.GetPluginsInPATH(); // Assert Assert.Single(plugins); - Assert.Equal(pluginFilePath, plugins[0].Path); + Assert.Equal(pluginInPathDirectoryFilePath, plugins[0].Path); + Assert.True(plugins[0].IsDotnetToolsPlugin); } [PlatformFact(Platform.Windows)] - public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToAFile_TreatsAsPlugin() + public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFile_TreatsAsPlugin() { // Arrange var pluginFilePath = Path.Combine(TestDirectory.Create().Path, "nuget-plugin-auth.exe"); @@ -475,15 +498,16 @@ public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToAFile_Tr var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); // Assert Assert.Single(plugins); Assert.Equal(pluginFilePath, plugins[0].Path); + Assert.True(plugins[0].IsDotnetToolsPlugin); } [PlatformFact(Platform.Windows)] - public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToAFileThatDoesNotStartWithNugetPlugin_ReturnsNonDotnetPlugin() + public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFileThatDoesNotStartWithNugetPlugin_ReturnsNonDotnetPlugin() { // Arrange var pluginFilePath = Path.Combine(TestDirectory.Create().Path, "other-plugin.exe"); @@ -496,7 +520,7 @@ public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToAFileTha var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); // Assert Assert.Single(plugins); @@ -504,7 +528,7 @@ public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToAFileTha } [PlatformFact(Platform.Windows)] - public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToADirectory_ContainsValidPluginFiles() + public void GetPluginsInPATH_PATHPointsToADirectory_ContainsValidPluginFiles() { // Arrange var pluginPathDirectory = TestDirectory.Create(); @@ -514,91 +538,28 @@ public void GetPluginsInNuGetPluginPathsAndPath_NuGetPluginPathsPointsToADirecto File.Create(invalidPluginFile); var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginPathDirectory.Path); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pluginPathDirectory.Path); var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + var plugins = pluginDiscoverer.GetPluginsInPATH(); // Assert Assert.Single(plugins); Assert.Equal(validPluginFile, plugins[0].Path); + Assert.True(plugins[0].IsDotnetToolsPlugin); } [PlatformFact(Platform.Windows)] - public void GetPluginsInNuGetPluginPathsAndPath_NoEnvironmentVariables_ReturnsNoPlugins() + public void GetPluginsInNuGetPluginPaths_NoEnvironmentVariables_ReturnsNoPlugins() { // Arrange var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(string.Empty); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); - var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); - - // Assert - Assert.Empty(plugins); - } - - [PlatformFact(Platform.Windows)] - public void GetPluginsInNuGetPluginPathsAndPath_PathContainsDirectoriesWithValidAndInvalidPlugins_ReturnsOnlyValidPlugins() - { - // Arrange - var pathDirectory = TestDirectory.Create(); - var validPluginFile = Path.Combine(pathDirectory.Path, "nuget-plugin-valid.exe"); - var invalidPluginFile = Path.Combine(pathDirectory.Path, "random-file.exe"); - File.Create(validPluginFile); - File.Create(invalidPluginFile); - - var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(string.Empty); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pathDirectory.Path); - - var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); - - // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); - - // Assert - Assert.Single(plugins); - Assert.Equal(validPluginFile, plugins[0].Path); - } - - [PlatformFact(Platform.Windows)] - public void GetPluginsInNuGetPluginPathsAndPath_NoNuGetPluginPaths_UsesPathEnvironment() - { - // Arrange - TestDirectory testDirectory = TestDirectory.Create(); - var workingPath = testDirectory.Path; - var pluginFilePath = Path.Combine(workingPath, "nuget-plugin-auth.exe"); - File.Create(pluginFilePath); - Mock environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(Directory.GetParent(pluginFilePath).FullName); - PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); - - // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); - - // Assert - Assert.Single(plugins); - Assert.Equal(pluginFilePath, plugins[0].Path); - } - - [Fact] - public void GetPluginsInNuGetPluginPathsAndPath_NoPluginsFound_ReturnsEmptyList() - { - // Arrange - TestDirectory testDirectory = TestDirectory.Create(); - var workingPath = testDirectory.Path; - Mock environmentalVariableReader = new Mock(); - PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); - - // Act - var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPathsAndPath(); + var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); // Assert Assert.Empty(plugins); From 66983c170576bdda8adbe9a5848d6e36b392ae13 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Sat, 12 Oct 2024 12:47:33 -0700 Subject: [PATCH 34/47] Cleanup --- src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 1d0a4b96c90..97e7e7b5129 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -118,7 +119,7 @@ public async Task> DiscoverAsync(Cancellation directories.Add(PluginDiscoveryUtility.GetInternalPlugins()); #endif var filePaths = PluginDiscoveryUtility.GetConventionBasedPlugins(directories); - _pluginFiles = GetPluginFiles((string[])filePaths, cancellationToken); + _pluginFiles = GetPluginFiles(filePaths.ToArray(), cancellationToken); // Search for .Net tools plugins in PATH _pluginFiles.AddRange(GetPluginsInPATH() ?? new List()); From d0a77e78aeb898bae19a315eeb710d4310474a74 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 15 Oct 2024 10:02:02 -0700 Subject: [PATCH 35/47] Update PluginDiscoverer API --- .../Plugins/PluginDiscoverer.cs | 37 +++++++++++++------ .../NuGet.Protocol/Plugins/PluginManager.cs | 21 ++++++----- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 97e7e7b5129..cbd89dc5fe9 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -19,15 +19,16 @@ public sealed class PluginDiscoverer : IPluginDiscoverer { private bool _isDisposed; private List _pluginFiles; - private readonly string _envVariableNetCoreAndNetFXPluginPaths; - private readonly string _envVariableNuGetPluginPaths; + private readonly string _netCoreAndNetFXPluginPaths; + private readonly string _nuGetPluginPaths; private IEnumerable _results; private readonly SemaphoreSlim _semaphore; private readonly EmbeddedSignatureVerifier _verifier; private readonly IEnvironmentVariableReader _environmentVariableReader; + private readonly string _pluginPaths; - public PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verifier) - : this(rawPluginPaths, verifier, EnvironmentVariableWrapper.Instance) + public PluginDiscoverer(EmbeddedSignatureVerifier verifier) + : this(verifier, EnvironmentVariableWrapper.Instance) { } @@ -39,7 +40,7 @@ public PluginDiscoverer(string rawPluginPaths) /// An embedded signature verifier. /// A reader for environmental varibales. /// Thrown if is . - internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verifier, IEnvironmentVariableReader environmentVariableReader) + internal PluginDiscoverer(EmbeddedSignatureVerifier verifier, IEnvironmentVariableReader environmentVariableReader) { _rawPluginPaths = rawPluginPaths; if (verifier == null) @@ -48,8 +49,12 @@ internal PluginDiscoverer(string rawPluginPaths, EmbeddedSignatureVerifier verif } _environmentVariableReader = environmentVariableReader; - _envVariableNetCoreAndNetFXPluginPaths = rawPluginPaths; - _envVariableNuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); +#if IS_DESKTOP + _netCoreAndNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths); +#else + _netCoreAndNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); +#endif + _nuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); _verifier = verifier; _semaphore = new SemaphoreSlim(initialCount: 1, maxCount: 1); } @@ -98,13 +103,13 @@ public async Task> DiscoverAsync(Cancellation return _results; } - if (!string.IsNullOrEmpty(_envVariableNetCoreAndNetFXPluginPaths)) + if (!string.IsNullOrEmpty(_netCoreAndNetFXPluginPaths)) { // NUGET_NETFX_PLUGIN_PATHS, NUGET_NETCORE_PLUGIN_PATHS have been set. - var filePaths = _envVariableNetCoreAndNetFXPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); + var filePaths = _netCoreAndNetFXPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); _pluginFiles = GetPluginFiles(filePaths, cancellationToken); } - else if (!string.IsNullOrEmpty(_envVariableNuGetPluginPaths)) + else if (!string.IsNullOrEmpty(_nuGetPluginPaths)) { // NUGET_PLUGIN_PATHS has been set _pluginFiles = GetPluginsInNuGetPluginPaths(); @@ -122,7 +127,14 @@ public async Task> DiscoverAsync(Cancellation _pluginFiles = GetPluginFiles(filePaths.ToArray(), cancellationToken); // Search for .Net tools plugins in PATH - _pluginFiles.AddRange(GetPluginsInPATH() ?? new List()); + if (_pluginFiles != null) + { + _pluginFiles.AddRange(GetPluginsInPATH() ?? new List()); + } + else + { + _pluginFiles = GetPluginsInPATH() ?? new List(); + } } var results = new List(); @@ -179,7 +191,7 @@ private List GetPluginFiles(string[] filePaths, CancellationToken ca internal List GetPluginsInNuGetPluginPaths() { var pluginFiles = new List(); - string[] paths = _envVariableNuGetPluginPaths?.Split(Path.PathSeparator) ?? Array.Empty(); + string[] paths = _nuGetPluginPaths?.Split(Path.PathSeparator) ?? Array.Empty(); foreach (var path in paths) { @@ -214,6 +226,7 @@ internal List GetPluginsInNuGetPluginPaths() if (IsValidPluginFile(file)) { // .NET SDK recently package signature verification for .NET tools, as a result we expect them to be valid. + // As a result the plugin created here has PluginFileState.Valid. PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); pluginFiles.Add(pluginFile); } diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs index d77fb0cdc9e..e9439f490ba 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs @@ -33,7 +33,6 @@ public sealed class PluginManager : IPluginManager, IDisposable private IPluginFactory _pluginFactory; private ConcurrentDictionary>>> _pluginOperationClaims; private ConcurrentDictionary> _pluginUtilities; - private string _envVariablePluginPaths; private static Lazy _currentProcessId = new Lazy(GetCurrentProcessId); private Lazy _pluginsCacheDirectoryPath; @@ -119,7 +118,7 @@ public async Task> CreatePluginsAsync( var pluginCreationResults = new List(); // Fast path - if (source.PackageSource.IsHttp) + if (source.PackageSource.IsHttp && IsPluginPossiblyAvailable()) { var serviceIndex = await source.GetResourceAsync(cancellationToken); @@ -312,11 +311,7 @@ private void Initialize(IEnvironmentVariableReader reader, { throw new ArgumentNullException(nameof(pluginFactoryCreator)); } -#if IS_DESKTOP - _envVariablePluginPaths = reader.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths); -#else - _envVariablePluginPaths = reader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); -#endif + _connectionOptions = ConnectionOptions.CreateDefault(reader); var idleTimeoutInSeconds = EnvironmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.IdleTimeout); @@ -357,12 +352,20 @@ private PluginDiscoverer InitializeDiscoverer() { var verifier = EmbeddedSignatureVerifier.Create(); - return new PluginDiscoverer(_envVariablePluginPaths, verifier); + return new PluginDiscoverer(verifier); } private bool IsPluginPossiblyAvailable() { - return !string.IsNullOrEmpty(_envVariablePluginPaths); + string pluginEnvVariable; + +#if IS_DESKTOP + pluginEnvVariable = EnvironmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths); +#else + pluginEnvVariable = EnvironmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); +#endif + pluginEnvVariable ??= EnvironmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); + return !string.IsNullOrEmpty(pluginEnvVariable); } private void OnPluginClosed(object sender, EventArgs e) From 704e61d8963b31aaab55cbfb1639fc426520df8c Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 15 Oct 2024 11:04:42 -0700 Subject: [PATCH 36/47] Cleanup --- .../Plugins/PluginDiscoverer.cs | 1 - .../Plugins/PluginDiscovererTests.cs | 142 ++++++++++++++---- 2 files changed, 116 insertions(+), 27 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index cbd89dc5fe9..bccdcd9854d 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -25,7 +25,6 @@ public sealed class PluginDiscoverer : IPluginDiscoverer private readonly SemaphoreSlim _semaphore; private readonly EmbeddedSignatureVerifier _verifier; private readonly IEnvironmentVariableReader _environmentVariableReader; - private readonly string _pluginPaths; public PluginDiscoverer(EmbeddedSignatureVerifier verifier) : this(verifier, EnvironmentVariableWrapper.Instance) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 7c3c22ebe0f..049c963a08a 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -23,25 +23,65 @@ public class PluginDiscovererTests [InlineData(" ")] public void Constructor_AcceptsAnyString(string rawPluginPaths) { - using (new PluginDiscoverer(rawPluginPaths)) + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); + using (new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object)) { } } + [Fact] + public void DiscoverAsync_ThrowsForNullVerifier() + { + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(""); + var exception = Assert.Throws( + () => new PluginDiscoverer(verifier: null, environmentalVariableReader.Object)); + + Assert.Equal("verifier", exception.ParamName); + } + [Fact] public async Task DiscoverAsync_ThrowsIfCancelled() { - using (var discoverer = new PluginDiscoverer(rawPluginPaths: "")) + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(""); + using (var discoverer = new PluginDiscoverer( + verifier: Mock.Of(), + environmentalVariableReader.Object)) { await Assert.ThrowsAsync( () => discoverer.DiscoverAsync(new CancellationToken(canceled: true))); } } + [Fact] + public async Task DiscoverAsync_ThrowsPlatformNotSupportedIfEmbeddedSignatureVerifierIsRequired() + { + using (var testDirectory = TestDirectory.Create()) + { + var pluginPath = Path.Combine(testDirectory.Path, "a"); + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + File.WriteAllText(pluginPath, string.Empty); + + using (var discoverer = new PluginDiscoverer(new FallbackEmbeddedSignatureVerifier(), environmentalVariableReader.Object)) + { + var plugins = await discoverer.DiscoverAsync(CancellationToken.None); + Assert.Throws( + () => plugins.SingleOrDefault().PluginFile.State.Value); + } + } + } + [Fact] public async Task DiscoverAsync_DoesNotThrowIfNoValidFilePathsAndFallbackEmbeddedSignatureVerifier() { - using (var discoverer = new PluginDiscoverer(rawPluginPaths: ";")) + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(";"); + using (var discoverer = new PluginDiscoverer( + verifier: new FallbackEmbeddedSignatureVerifier(), + environmentalVariableReader.Object)) { var pluginFiles = await discoverer.DiscoverAsync(CancellationToken.None); @@ -57,13 +97,15 @@ public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce() var pluginPath = Path.Combine(testDirectory.Path, "a"); File.WriteAllText(pluginPath, string.Empty); + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); var responses = new Dictionary() { { pluginPath, true } }; - using (var discoverer = new PluginDiscoverer(pluginPath)) + using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); @@ -77,6 +119,37 @@ public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce() results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); Assert.Equal(1, results.Length); + Assert.Equal(1, verifierStub.IsValidCallCount); + } + } + } + + [Fact] + public async Task DiscoverAsync_SignatureIsVerifiedLazily() + { + using (var testDirectory = TestDirectory.Create()) + { + var pluginPath = Path.Combine(testDirectory.Path, "a"); + + File.WriteAllText(pluginPath, string.Empty); + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + var responses = new Dictionary() + { + { pluginPath, true } + }; + var verifierStub = new EmbeddedSignatureVerifierStub(responses); + + using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) + { + var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); + + Assert.Equal(1, results.Length); + Assert.Equal(0, verifierStub.IsValidCallCount); + + var pluginState = results.SingleOrDefault().PluginFile.State.Value; + + Assert.Equal(1, verifierStub.IsValidCallCount); } } } @@ -92,10 +165,19 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates() File.WriteAllText(pluginPaths[1], string.Empty); - string rawPluginPaths = - $"{pluginPaths[0]};{pluginPaths[1]};c"; - - using (var discoverer = new PluginDiscoverer(rawPluginPaths)) + var responses = new Dictionary() + { + { pluginPaths[0], false }, + { pluginPaths[1], false }, + { pluginPaths[2], true }, + { pluginPaths[3], false }, + { "e", true } + }; + var verifierStub = new EmbeddedSignatureVerifierStub(responses); + var rawPluginPaths = string.Join(";", responses.Keys); + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); + using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); @@ -124,8 +206,10 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates() public async Task DiscoverAsync_DisallowsNonRootedFilePaths(string pluginPath) { var responses = new Dictionary() { { pluginPath, true } }; - - using (var discoverer = new PluginDiscoverer(pluginPath)) + var verifierStub = new EmbeddedSignatureVerifierStub(responses); + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); @@ -143,8 +227,14 @@ public async Task DiscoverAsync_IsIdempotent() var pluginPath = Path.Combine(testDirectory.Path, "a"); File.WriteAllText(pluginPath, string.Empty); + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + var verifierSpy = new Mock(); + + verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) + .Returns(true); - using (var discoverer = new PluginDiscoverer(pluginPath)) + using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) { var firstResult = await discoverer.DiscoverAsync(CancellationToken.None); var firstState = firstResult.SingleOrDefault().PluginFile.State.Value; @@ -169,14 +259,14 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginWindows_FindsThePlugin Directory.CreateDirectory(pluginPath); var myPlugin = Path.Combine(pluginPath, fileName); Mock environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(It.IsAny())).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginPath); File.WriteAllText(myPlugin, string.Empty); var verifierSpy = new Mock(); verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -213,7 +303,7 @@ public async Task DiscoverAsync_WithPluginPathSpecifiedInNuGetPluginPathsEnvVari verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -250,7 +340,7 @@ public async Task DiscoverAsync_withInValidDotNetToolsPluginNameWindows_DoesNotF verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -297,7 +387,7 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -347,7 +437,7 @@ public async Task DiscoverAsync_WithPluginPathSpecifiedInNuGetPluginPathsEnvVari verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -396,7 +486,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) .Returns(true); - using (var discoverer = new PluginDiscoverer("", verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -429,7 +519,7 @@ public void GetPluginsInNuGetPluginPaths_WithNuGetPluginPathsSet_ReturnsPluginsI Mock environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(Directory.GetParent(pluginInNuGetPluginPathDirectoryFilePath).FullName); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(Directory.GetParent(pluginInPathDirectoryFilePath).FullName); - PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + PluginDiscoverer pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -451,7 +541,7 @@ public void GetPluginsInNuGetPluginPaths_WithoutNuGetPluginPaths_ReturnsEmpty() var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pathDirectory.Path); - var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -473,7 +563,7 @@ public void GetPluginsInPATH_WithPATHSet_ReturnsPlugin() Mock environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(Directory.GetParent(pluginInNuGetPluginPathDirectoryFilePath).FullName); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(Directory.GetParent(pluginInPathDirectoryFilePath).FullName); - PluginDiscoverer pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + PluginDiscoverer pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInPATH(); @@ -495,7 +585,7 @@ public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFile_TreatsAsP environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginFilePath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); - var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -517,7 +607,7 @@ public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFileThatDoesNo environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginFilePath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); - var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -531,7 +621,7 @@ public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFileThatDoesNo public void GetPluginsInPATH_PATHPointsToADirectory_ContainsValidPluginFiles() { // Arrange - var pluginPathDirectory = TestDirectory.Create(); + using var pluginPathDirectory = TestDirectory.Create(); var validPluginFile = Path.Combine(pluginPathDirectory.Path, "nuget-plugin-auth.exe"); var invalidPluginFile = Path.Combine(pluginPathDirectory.Path, "not-a-nuget-plugin.exe"); File.Create(validPluginFile); @@ -540,7 +630,7 @@ public void GetPluginsInPATH_PATHPointsToADirectory_ContainsValidPluginFiles() var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pluginPathDirectory.Path); - var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInPATH(); @@ -556,7 +646,7 @@ public void GetPluginsInNuGetPluginPaths_NoEnvironmentVariables_ReturnsNoPlugins { // Arrange var environmentalVariableReader = new Mock(); - var pluginDiscoverer = new PluginDiscoverer("", Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); From 32cab44435182a9791fc70209b0c47e08afb0453 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 15 Oct 2024 11:22:57 -0700 Subject: [PATCH 37/47] cleanup --- .../NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs index d436de575e7..787f6b6db73 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginResourceProviderTests.cs @@ -267,7 +267,6 @@ internal PluginResourceProviderNegativeTest(string serviceIndexJson, string sour _pluginDiscoverer.Setup(x => x.DiscoverAsync(It.IsAny())) .ReturnsAsync(pluginDiscoveryResults); - _pluginDiscoverer.Setup(x => x.Dispose()); _testDirectory = TestDirectory.Create(); From 842423759adc50883cd271303d9f2ad8dae76e26 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 15 Oct 2024 11:50:46 -0700 Subject: [PATCH 38/47] dispose directories --- Directory.Packages.props | 2 +- .../Plugins/PluginDiscovererTests.cs | 26 ++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 9b0f0c57040..266ee00edfc 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -5,7 +5,7 @@ 3.3.4 13.0.3 8.0.1 - 8.0.4 + 8.0.5 4.3.0 2.0.0-beta4.23307.1 3.4.3 diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 049c963a08a..73740dbfb23 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -510,8 +510,8 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does public void GetPluginsInNuGetPluginPaths_WithNuGetPluginPathsSet_ReturnsPluginsInNuGetPluginPathOnly() { // Arrange - TestDirectory pluginPathDirectory = TestDirectory.Create(); - TestDirectory pathDirectory = TestDirectory.Create(); + using TestDirectory pluginPathDirectory = TestDirectory.Create(); + using TestDirectory pathDirectory = TestDirectory.Create(); var pluginInNuGetPluginPathDirectoryFilePath = Path.Combine(pluginPathDirectory.Path, "nuget-plugin-auth.exe"); var pluginInPathDirectoryFilePath = Path.Combine(pathDirectory.Path, "nuget-plugin-in-path-directory.exe"); File.Create(pluginInNuGetPluginPathDirectoryFilePath); @@ -534,7 +534,7 @@ public void GetPluginsInNuGetPluginPaths_WithNuGetPluginPathsSet_ReturnsPluginsI public void GetPluginsInNuGetPluginPaths_WithoutNuGetPluginPaths_ReturnsEmpty() { // Arrange - var pathDirectory = TestDirectory.Create(); + using var pathDirectory = TestDirectory.Create(); var pluginFilePath = Path.Combine(pathDirectory.Path, "nuget-plugin-fallback.exe"); File.Create(pluginFilePath); @@ -554,8 +554,8 @@ public void GetPluginsInNuGetPluginPaths_WithoutNuGetPluginPaths_ReturnsEmpty() public void GetPluginsInPATH_WithPATHSet_ReturnsPlugin() { // Arrange - TestDirectory pluginPathDirectory = TestDirectory.Create(); - TestDirectory pathDirectory = TestDirectory.Create(); + using TestDirectory pluginPathDirectory = TestDirectory.Create(); + using TestDirectory pathDirectory = TestDirectory.Create(); var pluginInNuGetPluginPathDirectoryFilePath = Path.Combine(pluginPathDirectory.Path, "nuget-plugin-auth.exe"); var pluginInPathDirectoryFilePath = Path.Combine(pathDirectory.Path, "nuget-plugin-in-path-directory.exe"); File.Create(pluginInNuGetPluginPathDirectoryFilePath); @@ -578,7 +578,8 @@ public void GetPluginsInPATH_WithPATHSet_ReturnsPlugin() public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFile_TreatsAsPlugin() { // Arrange - var pluginFilePath = Path.Combine(TestDirectory.Create().Path, "nuget-plugin-auth.exe"); + using TestDirectory testDirectory = TestDirectory.Create(); + var pluginFilePath = Path.Combine(testDirectory.Path, "nuget-plugin-auth.exe"); File.Create(pluginFilePath); var environmentalVariableReader = new Mock(); @@ -600,7 +601,8 @@ public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFile_TreatsAsP public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFileThatDoesNotStartWithNugetPlugin_ReturnsNonDotnetPlugin() { // Arrange - var pluginFilePath = Path.Combine(TestDirectory.Create().Path, "other-plugin.exe"); + using TestDirectory testDirectory = TestDirectory.Create(); + var pluginFilePath = Path.Combine(testDirectory.Path, "other-plugin.exe"); File.Create(pluginFilePath); var environmentalVariableReader = new Mock(); @@ -659,7 +661,7 @@ public void GetPluginsInNuGetPluginPaths_NoEnvironmentVariables_ReturnsNoPlugins public void IsValidPluginFile_ExeFile_ReturnsTrue() { // Arrange - TestDirectory testDirectory = TestDirectory.Create(); + using TestDirectory testDirectory = TestDirectory.Create(); var workingPath = testDirectory.Path; var pluginFilePath = Path.Combine(workingPath, "plugin.exe"); File.Create(pluginFilePath); @@ -676,7 +678,7 @@ public void IsValidPluginFile_ExeFile_ReturnsTrue() public void IsValidPluginFile_Windows_NonExecutableFile_ReturnsFalse() { // Arrange - TestDirectory testDirectory = TestDirectory.Create(); + using TestDirectory testDirectory = TestDirectory.Create(); var workingPath = testDirectory.Path; var nonPluginFilePath = Path.Combine(workingPath, "plugin.txt"); File.Create(nonPluginFilePath); @@ -693,7 +695,7 @@ public void IsValidPluginFile_Windows_NonExecutableFile_ReturnsFalse() public void IsValidPluginFile_Unix_ExecutableFile_ReturnsTrue() { // Arrange - TestDirectory testDirectory = TestDirectory.Create(); + using TestDirectory testDirectory = TestDirectory.Create(); var workingPath = testDirectory.Path; var pluginFilePath = Path.Combine(workingPath, "plugin"); File.Create(pluginFilePath).Dispose(); @@ -726,7 +728,7 @@ public void IsValidPluginFile_Unix_ExecutableFile_ReturnsTrue() public void IsExecutable_FileIsExecutable_ReturnsTrue() { // Arrange - TestDirectory testDirectory = TestDirectory.Create(); + using TestDirectory testDirectory = TestDirectory.Create(); var workingPath = testDirectory.Path; var pluginFilePath = Path.Combine(workingPath, "plugin"); File.Create(pluginFilePath); @@ -753,7 +755,7 @@ public void IsExecutable_FileIsExecutable_ReturnsTrue() public void IsExecutable_FileIsNotExecutable_ReturnsFalse() { // Arrange - TestDirectory testDirectory = TestDirectory.Create(); + using TestDirectory testDirectory = TestDirectory.Create(); var workingPath = testDirectory.Path; var pluginFilePath = Path.Combine(workingPath, "plugin"); File.Create(pluginFilePath); From ed12e5119adef1efd64d77f82813f26b3bc6702f Mon Sep 17 00:00:00 2001 From: Nigusu Solomon Yenework <59111203+Nigusu-Allehu@users.noreply.github.com> Date: Tue, 15 Oct 2024 11:57:57 -0700 Subject: [PATCH 39/47] undo --- Directory.Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 266ee00edfc..9b0f0c57040 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -5,7 +5,7 @@ 3.3.4 13.0.3 8.0.1 - 8.0.5 + 8.0.4 4.3.0 2.0.0-beta4.23307.1 3.4.3 From 7466578f8721234440efb8f4cf018017bd5202bb Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 15 Oct 2024 14:03:03 -0700 Subject: [PATCH 40/47] fix netcore test --- .../Plugins/PluginDiscovererTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 73740dbfb23..9f73da7b277 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -25,6 +25,7 @@ public void Constructor_AcceptsAnyString(string rawPluginPaths) { var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(rawPluginPaths); using (new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object)) { } @@ -35,6 +36,7 @@ public void DiscoverAsync_ThrowsForNullVerifier() { var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(""); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(""); var exception = Assert.Throws( () => new PluginDiscoverer(verifier: null, environmentalVariableReader.Object)); @@ -46,6 +48,7 @@ public async Task DiscoverAsync_ThrowsIfCancelled() { var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(""); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(""); using (var discoverer = new PluginDiscoverer( verifier: Mock.Of(), environmentalVariableReader.Object)) @@ -63,6 +66,7 @@ public async Task DiscoverAsync_ThrowsPlatformNotSupportedIfEmbeddedSignatureVer var pluginPath = Path.Combine(testDirectory.Path, "a"); var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); File.WriteAllText(pluginPath, string.Empty); using (var discoverer = new PluginDiscoverer(new FallbackEmbeddedSignatureVerifier(), environmentalVariableReader.Object)) @@ -79,6 +83,7 @@ public async Task DiscoverAsync_DoesNotThrowIfNoValidFilePathsAndFallbackEmbedde { var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(";"); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(";"); using (var discoverer = new PluginDiscoverer( verifier: new FallbackEmbeddedSignatureVerifier(), environmentalVariableReader.Object)) @@ -99,6 +104,7 @@ public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce() File.WriteAllText(pluginPath, string.Empty); var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); var responses = new Dictionary() { @@ -134,6 +140,7 @@ public async Task DiscoverAsync_SignatureIsVerifiedLazily() File.WriteAllText(pluginPath, string.Empty); var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); var responses = new Dictionary() { { pluginPath, true } @@ -177,6 +184,7 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates() var rawPluginPaths = string.Join(";", responses.Keys); var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(rawPluginPaths); using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); @@ -209,6 +217,7 @@ public async Task DiscoverAsync_DisallowsNonRootedFilePaths(string pluginPath) var verifierStub = new EmbeddedSignatureVerifierStub(responses); var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); @@ -229,6 +238,7 @@ public async Task DiscoverAsync_IsIdempotent() File.WriteAllText(pluginPath, string.Empty); var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); var verifierSpy = new Mock(); verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) From 47f4d5dab83d50c919177139ee5c690207305911 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 15 Oct 2024 14:29:25 -0700 Subject: [PATCH 41/47] Fix linux tests --- .../NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 9f73da7b277..794ab64b0d3 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -379,7 +379,7 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); File.WriteAllText(myPlugin, string.Empty); Mock environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(It.IsAny())).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginPath); using (var process = new Process()) { @@ -428,7 +428,7 @@ public async Task DiscoverAsync_WithPluginPathSpecifiedInNuGetPluginPathsEnvVari var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); File.WriteAllText(myPlugin, string.Empty); Mock environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("NUGET_PLUGIN_PATHS")).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginPath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATHS")).Returns(""); using (var process = new Process()) @@ -478,7 +478,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does var myPlugin = Path.Combine(pluginPath, "nuget-plugin-MyPlugin"); File.WriteAllText(myPlugin, string.Empty); Mock environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(It.IsAny())).Returns(pluginPath); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginPath); using (var process = new Process()) { From 3eab1a779c6dd40f5ab8304c428a11813bb4b494 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 15 Oct 2024 16:01:35 -0700 Subject: [PATCH 42/47] Cleanup --- .../Plugins/PluginDiscoverer.cs | 29 +-- .../NuGet.Protocol/Plugins/PluginManager.cs | 4 +- .../PublicAPI/net472/PublicAPI.Unshipped.txt | 4 +- .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 4 +- .../netstandard2.0/PublicAPI.Unshipped.txt | 4 +- .../Plugins/PluginDiscovererTests.cs | 165 +++--------------- 6 files changed, 30 insertions(+), 180 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index bccdcd9854d..9ea07b835fa 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -23,30 +23,15 @@ public sealed class PluginDiscoverer : IPluginDiscoverer private readonly string _nuGetPluginPaths; private IEnumerable _results; private readonly SemaphoreSlim _semaphore; - private readonly EmbeddedSignatureVerifier _verifier; private readonly IEnvironmentVariableReader _environmentVariableReader; - public PluginDiscoverer(EmbeddedSignatureVerifier verifier) - : this(verifier, EnvironmentVariableWrapper.Instance) + public PluginDiscoverer() + : this(EnvironmentVariableWrapper.Instance) { } - /// - /// Instantiates a new class. - /// - /// The raw semicolon-delimited list of supposed plugin file paths. - public PluginDiscoverer(string rawPluginPaths) - /// An embedded signature verifier. - /// A reader for environmental varibales. - /// Thrown if is . - internal PluginDiscoverer(EmbeddedSignatureVerifier verifier, IEnvironmentVariableReader environmentVariableReader) + internal PluginDiscoverer(IEnvironmentVariableReader environmentVariableReader) { - _rawPluginPaths = rawPluginPaths; - if (verifier == null) - { - throw new ArgumentNullException(nameof(verifier)); - } - _environmentVariableReader = environmentVariableReader; #if IS_DESKTOP _netCoreAndNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths); @@ -54,7 +39,6 @@ internal PluginDiscoverer(EmbeddedSignatureVerifier verifier, IEnvironmentVariab _netCoreAndNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); #endif _nuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); - _verifier = verifier; _semaphore = new SemaphoreSlim(initialCount: 1, maxCount: 1); } @@ -157,7 +141,7 @@ public async Task> DiscoverAsync(Cancellation return _results; } - private List GetPluginFiles(string[] filePaths, CancellationToken cancellationToken) + private static List GetPluginFiles(string[] filePaths, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); @@ -211,7 +195,7 @@ internal List GetPluginsInNuGetPluginPaths() else { // A non DotNet tool plugin file - var state = new Lazy(() => _verifier.IsValid(fileInfo.FullName) ? PluginFileState.Valid : PluginFileState.InvalidEmbeddedSignature); + var state = new Lazy(() => PluginFileState.Valid); pluginFiles.Add(new PluginFile(fileInfo.FullName, state)); } } @@ -224,8 +208,6 @@ internal List GetPluginsInNuGetPluginPaths() { if (IsValidPluginFile(file)) { - // .NET SDK recently package signature verification for .NET tools, as a result we expect them to be valid. - // As a result the plugin created here has PluginFileState.Valid. PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); pluginFiles.Add(pluginFile); } @@ -264,7 +246,6 @@ internal List GetPluginsInPATH() { if (IsValidPluginFile(file)) { - // .NET SDK recently package signature verification for .NET tools, as a result we expect them to be valid. PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); pluginFiles.Add(pluginFile); } diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs index e9439f490ba..6ff4407f862 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs @@ -350,9 +350,7 @@ private async Task> GetPluginOperationClaimsAsync( private PluginDiscoverer InitializeDiscoverer() { - var verifier = EmbeddedSignatureVerifier.Create(); - - return new PluginDiscoverer(verifier); + return new PluginDiscoverer(); } private bool IsPluginPossiblyAvailable() diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt index e6677064e57..7e98009cd58 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,4 +1,2 @@ #nullable enable -~NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer(string rawPluginPaths) -> void -NuGet.Protocol.Plugins.PluginFile.IsDotnetToolsPlugin.get -> bool -~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy state, bool isDotnetToolsPlugin) -> void +NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer() -> void diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt index e6677064e57..7e98009cd58 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,4 +1,2 @@ #nullable enable -~NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer(string rawPluginPaths) -> void -NuGet.Protocol.Plugins.PluginFile.IsDotnetToolsPlugin.get -> bool -~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy state, bool isDotnetToolsPlugin) -> void +NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer() -> void diff --git a/src/NuGet.Core/NuGet.Protocol/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Protocol/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index e6677064e57..7e98009cd58 100644 --- a/src/NuGet.Core/NuGet.Protocol/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Protocol/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,4 +1,2 @@ #nullable enable -~NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer(string rawPluginPaths) -> void -NuGet.Protocol.Plugins.PluginFile.IsDotnetToolsPlugin.get -> bool -~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy state, bool isDotnetToolsPlugin) -> void +NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer() -> void diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 794ab64b0d3..b795572f5d0 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -26,74 +26,24 @@ public void Constructor_AcceptsAnyString(string rawPluginPaths) var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(rawPluginPaths); - using (new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object)) + using (new PluginDiscoverer()) { } } - [Fact] - public void DiscoverAsync_ThrowsForNullVerifier() - { - var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(""); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(""); - var exception = Assert.Throws( - () => new PluginDiscoverer(verifier: null, environmentalVariableReader.Object)); - - Assert.Equal("verifier", exception.ParamName); - } - [Fact] public async Task DiscoverAsync_ThrowsIfCancelled() { var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(""); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(""); - using (var discoverer = new PluginDiscoverer( - verifier: Mock.Of(), - environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { await Assert.ThrowsAsync( () => discoverer.DiscoverAsync(new CancellationToken(canceled: true))); } } - [Fact] - public async Task DiscoverAsync_ThrowsPlatformNotSupportedIfEmbeddedSignatureVerifierIsRequired() - { - using (var testDirectory = TestDirectory.Create()) - { - var pluginPath = Path.Combine(testDirectory.Path, "a"); - var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); - File.WriteAllText(pluginPath, string.Empty); - - using (var discoverer = new PluginDiscoverer(new FallbackEmbeddedSignatureVerifier(), environmentalVariableReader.Object)) - { - var plugins = await discoverer.DiscoverAsync(CancellationToken.None); - Assert.Throws( - () => plugins.SingleOrDefault().PluginFile.State.Value); - } - } - } - - [Fact] - public async Task DiscoverAsync_DoesNotThrowIfNoValidFilePathsAndFallbackEmbeddedSignatureVerifier() - { - var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(";"); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(";"); - using (var discoverer = new PluginDiscoverer( - verifier: new FallbackEmbeddedSignatureVerifier(), - environmentalVariableReader.Object)) - { - var pluginFiles = await discoverer.DiscoverAsync(CancellationToken.None); - - Assert.Empty(pluginFiles); - } - } - [Fact] public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce() { @@ -106,12 +56,7 @@ public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce() environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); - var responses = new Dictionary() - { - { pluginPath, true } - }; - - using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); @@ -125,7 +70,6 @@ public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce() results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); Assert.Equal(1, results.Length); - Assert.Equal(1, verifierStub.IsValidCallCount); } } } @@ -141,22 +85,14 @@ public async Task DiscoverAsync_SignatureIsVerifiedLazily() var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); - var responses = new Dictionary() - { - { pluginPath, true } - }; - var verifierStub = new EmbeddedSignatureVerifierStub(responses); - using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); Assert.Equal(1, results.Length); - Assert.Equal(0, verifierStub.IsValidCallCount); var pluginState = results.SingleOrDefault().PluginFile.State.Value; - - Assert.Equal(1, verifierStub.IsValidCallCount); } } } @@ -171,21 +107,11 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates() .ToArray(); File.WriteAllText(pluginPaths[1], string.Empty); - - var responses = new Dictionary() - { - { pluginPaths[0], false }, - { pluginPaths[1], false }, - { pluginPaths[2], true }, - { pluginPaths[3], false }, - { "e", true } - }; - var verifierStub = new EmbeddedSignatureVerifierStub(responses); - var rawPluginPaths = string.Join(";", responses.Keys); + var rawPluginPaths = string.Join(";", pluginPaths); var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(rawPluginPaths); - using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); @@ -214,11 +140,10 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates() public async Task DiscoverAsync_DisallowsNonRootedFilePaths(string pluginPath) { var responses = new Dictionary() { { pluginPath, true } }; - var verifierStub = new EmbeddedSignatureVerifierStub(responses); var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); - using (var discoverer = new PluginDiscoverer(verifierStub, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); @@ -239,12 +164,8 @@ public async Task DiscoverAsync_IsIdempotent() var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); - var verifierSpy = new Mock(); - - verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) - .Returns(true); - using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { var firstResult = await discoverer.DiscoverAsync(CancellationToken.None); var firstState = firstResult.SingleOrDefault().PluginFile.State.Value; @@ -272,11 +193,8 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginWindows_FindsThePlugin environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginPath); File.WriteAllText(myPlugin, string.Empty); - var verifierSpy = new Mock(); - verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) - .Returns(true); - using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -309,11 +227,8 @@ public async Task DiscoverAsync_WithPluginPathSpecifiedInNuGetPluginPathsEnvVari environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("NUGET_PLUGIN_PATHS")).Returns(pluginPath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATHS")).Returns(""); File.WriteAllText(myPlugin, string.Empty); - var verifierSpy = new Mock(); - verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) - .Returns(true); - using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -346,11 +261,8 @@ public async Task DiscoverAsync_withInValidDotNetToolsPluginNameWindows_DoesNotF environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(It.IsAny())).Returns(pluginPath); File.WriteAllText(myPlugin, string.Empty); - var verifierSpy = new Mock(); - verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) - .Returns(true); - using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -393,11 +305,7 @@ public async Task DiscoverAsync_withValidDotNetToolsPluginLinux_FindsThePlugin() if (process.ExitCode == 0) { - var verifierSpy = new Mock(); - verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) - .Returns(true); - - using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -443,11 +351,7 @@ public async Task DiscoverAsync_WithPluginPathSpecifiedInNuGetPluginPathsEnvVari if (process.ExitCode == 0) { - var verifierSpy = new Mock(); - verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) - .Returns(true); - - using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -492,11 +396,7 @@ public async Task DiscoverAsync_withNoExecutableValidDotNetToolsPluginLinux_Does if (process.ExitCode == 0) { - var verifierSpy = new Mock(); - verifierSpy.Setup(spy => spy.IsValid(It.IsAny())) - .Returns(true); - - using (var discoverer = new PluginDiscoverer(verifierSpy.Object, environmentalVariableReader.Object)) + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { // Act var result = await discoverer.DiscoverAsync(CancellationToken.None); @@ -529,7 +429,7 @@ public void GetPluginsInNuGetPluginPaths_WithNuGetPluginPathsSet_ReturnsPluginsI Mock environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(Directory.GetParent(pluginInNuGetPluginPathDirectoryFilePath).FullName); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(Directory.GetParent(pluginInPathDirectoryFilePath).FullName); - PluginDiscoverer pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); + PluginDiscoverer pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -551,7 +451,7 @@ public void GetPluginsInNuGetPluginPaths_WithoutNuGetPluginPaths_ReturnsEmpty() var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pathDirectory.Path); - var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -573,7 +473,7 @@ public void GetPluginsInPATH_WithPATHSet_ReturnsPlugin() Mock environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(Directory.GetParent(pluginInNuGetPluginPathDirectoryFilePath).FullName); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(Directory.GetParent(pluginInPathDirectoryFilePath).FullName); - PluginDiscoverer pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); + PluginDiscoverer pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInPATH(); @@ -596,7 +496,7 @@ public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFile_TreatsAsP environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginFilePath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); - var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -619,7 +519,7 @@ public void GetPluginsInNuGetPluginPaths_NuGetPluginPathsPointsToAFileThatDoesNo environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths)).Returns(pluginFilePath); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(string.Empty); - var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -642,7 +542,7 @@ public void GetPluginsInPATH_PATHPointsToADirectory_ContainsValidPluginFiles() var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable("PATH")).Returns(pluginPathDirectory.Path); - var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInPATH(); @@ -658,7 +558,7 @@ public void GetPluginsInNuGetPluginPaths_NoEnvironmentVariables_ReturnsNoPlugins { // Arrange var environmentalVariableReader = new Mock(); - var pluginDiscoverer = new PluginDiscoverer(Mock.Of(), environmentalVariableReader.Object); + var pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act var plugins = pluginDiscoverer.GetPluginsInNuGetPluginPaths(); @@ -788,28 +688,5 @@ public void IsExecutable_FileIsNotExecutable_ReturnsFalse() Assert.False(result); } #endif - - private sealed class EmbeddedSignatureVerifierStub : EmbeddedSignatureVerifier - { - private readonly Dictionary _responses; - - internal int IsValidCallCount { get; private set; } - - public EmbeddedSignatureVerifierStub(Dictionary responses) - { - _responses = responses; - } - - public override bool IsValid(string filePath) - { - ++IsValidCallCount; - - bool value; - - Assert.True(_responses.TryGetValue(filePath, out value)); - - return value; - } - } } } From 2a36928785e8800e31dde933f7416acf85620ed3 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 15 Oct 2024 16:30:28 -0700 Subject: [PATCH 43/47] clean up test conflict --- .../Plugins/PluginDiscovererTests.cs | 41 ++++++++----------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index b795572f5d0..723b1793a0d 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -44,6 +44,20 @@ await Assert.ThrowsAsync( } } + [Fact] + public async Task DiscoverAsync_DoesNotThrowIfNoValidFilePathsAndFallbackEmbeddedSignatureVerifier() + { + var environmentalVariableReader = new Mock(); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(";"); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(";"); + using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) + { + var pluginFiles = await discoverer.DiscoverAsync(CancellationToken.None); + + Assert.Empty(pluginFiles); + } + } + [Fact] public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce() { @@ -74,29 +88,6 @@ public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce() } } - [Fact] - public async Task DiscoverAsync_SignatureIsVerifiedLazily() - { - using (var testDirectory = TestDirectory.Create()) - { - var pluginPath = Path.Combine(testDirectory.Path, "a"); - - File.WriteAllText(pluginPath, string.Empty); - var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(pluginPath); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(pluginPath); - - using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) - { - var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray(); - - Assert.Equal(1, results.Length); - - var pluginState = results.SingleOrDefault().PluginFile.State.Value; - } - } - } - [Fact] public async Task DiscoverAsync_HandlesAllPluginFileStates() { @@ -107,7 +98,9 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates() .ToArray(); File.WriteAllText(pluginPaths[1], string.Empty); - var rawPluginPaths = string.Join(";", pluginPaths); + + string rawPluginPaths = + $"{pluginPaths[0]};{pluginPaths[1]};c"; var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(rawPluginPaths); From 3f8b8db9ba250952147c8edecda064651a7f8bb8 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 17 Oct 2024 11:35:03 -0700 Subject: [PATCH 44/47] Address comments --- .../Plugins/PluginDiscoverer.cs | 58 +++++++++---------- .../Plugins/PluginDiscovererTests.cs | 28 +++++++++ 2 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 9ea07b835fa..09ac40637a4 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -89,7 +89,7 @@ public async Task> DiscoverAsync(Cancellation if (!string.IsNullOrEmpty(_netCoreAndNetFXPluginPaths)) { // NUGET_NETFX_PLUGIN_PATHS, NUGET_NETCORE_PLUGIN_PATHS have been set. - var filePaths = _netCoreAndNetFXPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); + var filePaths = _netCoreAndNetFXPluginPaths.Split(new[] { Path.PathSeparator }, StringSplitOptions.RemoveEmptyEntries); _pluginFiles = GetPluginFiles(filePaths, cancellationToken); } else if (!string.IsNullOrEmpty(_nuGetPluginPaths)) @@ -201,17 +201,7 @@ internal List GetPluginsInNuGetPluginPaths() } else if (Directory.Exists(path)) { - var directoryInfo = new DirectoryInfo(path); - var files = directoryInfo.GetFiles("nuget-plugin-*"); - - foreach (var file in files) - { - if (IsValidPluginFile(file)) - { - PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); - pluginFiles.Add(pluginFile); - } - } + pluginFiles.AddRange(GetNetToolsPluginsInDirectory(path) ?? new List()); } } else @@ -237,20 +227,7 @@ internal List GetPluginsInPATH() { if (PathValidator.IsValidLocalPath(path) || PathValidator.IsValidUncPath(path)) { - if (Directory.Exists(path)) - { - var directoryInfo = new DirectoryInfo(path); - var files = directoryInfo.GetFiles("nuget-plugin-*"); - - foreach (var file in files) - { - if (IsValidPluginFile(file)) - { - PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); - pluginFiles.Add(pluginFile); - } - } - } + pluginFiles.AddRange(GetNetToolsPluginsInDirectory(path) ?? new List()); } else { @@ -261,6 +238,28 @@ internal List GetPluginsInPATH() return pluginFiles; } + private static List GetNetToolsPluginsInDirectory(string directoryPath) + { + var pluginFiles = new List(); + + if (Directory.Exists(directoryPath)) + { + var directoryInfo = new DirectoryInfo(directoryPath); + var files = directoryInfo.GetFiles("nuget-plugin-*"); + + foreach (var file in files) + { + if (IsValidPluginFile(file)) + { + PluginFile pluginFile = new PluginFile(file.FullName, new Lazy(() => PluginFileState.Valid), isDotnetToolsPlugin: true); + pluginFiles.Add(pluginFile); + } + } + } + + return pluginFiles; + } + /// /// Checks whether a file is a valid plugin file for windows/Unix. /// Windows: It should be either .bat or .exe @@ -307,17 +306,18 @@ internal static bool IsExecutable(FileInfo fileInfo) { // Use a shell command to check if the file is executable process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $" -c \"if [ -x {fileInfo.FullName} ]; then echo yes; else echo no; fi\""; + process.StartInfo.Arguments = $" -c \"if [ -x '{fileInfo.FullName}' ]; then echo yes; else echo no; fi\""; process.StartInfo.UseShellExecute = false; process.StartInfo.RedirectStandardOutput = true; process.Start(); + output = process.StandardOutput.ReadToEnd().Trim(); + if (!process.WaitForExit(1000) || process.ExitCode != 0) { + process.Kill(); return false; } - - output = process.StandardOutput.ReadToEnd().Trim(); } // Check if the output is "yes" diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 723b1793a0d..50de5fce092 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -680,6 +680,34 @@ public void IsExecutable_FileIsNotExecutable_ReturnsFalse() // Assert Assert.False(result); } + + [PlatformFact(Platform.Linux)] + public void IsExecutable_FileWithSpace_ReturnsTrue() + { + // Arrange + using TestDirectory testDirectory = TestDirectory.Create(); + var workingPath = testDirectory.Path; + var pluginFilePath = Path.Combine(workingPath, "plugin with space"); + File.Create(pluginFilePath).Dispose(); + + // Set execute permissions + var process = new Process(); + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"-c \"chmod +x '{pluginFilePath}'\""; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + process.WaitForExit(); + + var fileInfo = new FileInfo(pluginFilePath); + + // Act + bool result = PluginDiscoverer.IsExecutable(fileInfo); + + // Assert + Assert.True(result); + } + #endif } } From 691c76e689a2b2c5a585c988b8e7a80aa31e949b Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Thu, 17 Oct 2024 11:55:10 -0700 Subject: [PATCH 45/47] Address comments --- .../Plugins/PluginDiscoverer.cs | 20 +++++++++++-------- .../Plugins/PluginDiscovererTests.cs | 6 +++--- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 09ac40637a4..704963192ca 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -19,7 +18,7 @@ public sealed class PluginDiscoverer : IPluginDiscoverer { private bool _isDisposed; private List _pluginFiles; - private readonly string _netCoreAndNetFXPluginPaths; + private readonly string _netCoreOrNetFXPluginPaths; private readonly string _nuGetPluginPaths; private IEnumerable _results; private readonly SemaphoreSlim _semaphore; @@ -34,9 +33,9 @@ internal PluginDiscoverer(IEnvironmentVariableReader environmentVariableReader) { _environmentVariableReader = environmentVariableReader; #if IS_DESKTOP - _netCoreAndNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths); + _netCoreOrNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths); #else - _netCoreAndNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); + _netCoreOrNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); #endif _nuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); _semaphore = new SemaphoreSlim(initialCount: 1, maxCount: 1); @@ -86,10 +85,10 @@ public async Task> DiscoverAsync(Cancellation return _results; } - if (!string.IsNullOrEmpty(_netCoreAndNetFXPluginPaths)) + if (!string.IsNullOrEmpty(_netCoreOrNetFXPluginPaths)) { // NUGET_NETFX_PLUGIN_PATHS, NUGET_NETCORE_PLUGIN_PATHS have been set. - var filePaths = _netCoreAndNetFXPluginPaths.Split(new[] { Path.PathSeparator }, StringSplitOptions.RemoveEmptyEntries); + var filePaths = _netCoreOrNetFXPluginPaths.Split(new[] { Path.PathSeparator }, StringSplitOptions.RemoveEmptyEntries); _pluginFiles = GetPluginFiles(filePaths, cancellationToken); } else if (!string.IsNullOrEmpty(_nuGetPluginPaths)) @@ -107,7 +106,7 @@ public async Task> DiscoverAsync(Cancellation directories.Add(PluginDiscoveryUtility.GetInternalPlugins()); #endif var filePaths = PluginDiscoveryUtility.GetConventionBasedPlugins(directories); - _pluginFiles = GetPluginFiles(filePaths.ToArray(), cancellationToken); + _pluginFiles = GetPluginFiles(filePaths, cancellationToken); // Search for .Net tools plugins in PATH if (_pluginFiles != null) @@ -141,12 +140,17 @@ public async Task> DiscoverAsync(Cancellation return _results; } - private static List GetPluginFiles(string[] filePaths, CancellationToken cancellationToken) + private static List GetPluginFiles(IEnumerable filePaths, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); var files = new List(); + if (filePaths == null) + { + return files; + } + foreach (var filePath in filePaths) { var pluginFile = new PluginFile(filePath, new Lazy(() => diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 50de5fce092..afbfefb906b 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -48,8 +48,8 @@ await Assert.ThrowsAsync( public async Task DiscoverAsync_DoesNotThrowIfNoValidFilePathsAndFallbackEmbeddedSignatureVerifier() { var environmentalVariableReader = new Mock(); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(";"); - environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(";"); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(Path.PathSeparator.ToString()); + environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(Path.PathSeparator.ToString()); using (var discoverer = new PluginDiscoverer(environmentalVariableReader.Object)) { var pluginFiles = await discoverer.DiscoverAsync(CancellationToken.None); @@ -100,7 +100,7 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates() File.WriteAllText(pluginPaths[1], string.Empty); string rawPluginPaths = - $"{pluginPaths[0]};{pluginPaths[1]};c"; + $"{pluginPaths[0]}{Path.PathSeparator}{pluginPaths[1]}{Path.PathSeparator}c"; var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(rawPluginPaths); From 6abbc3d46b28f8ac5f52efcc8dd3669e19792199 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 22 Oct 2024 09:41:49 -0700 Subject: [PATCH 46/47] Address comments --- .../NuGet.Protocol/Plugins/PluginDiscoverer.cs | 13 +++++++++---- .../Plugins/PluginDiscovererTests.cs | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 704963192ca..4bd8c4a520c 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -37,7 +37,12 @@ internal PluginDiscoverer(IEnvironmentVariableReader environmentVariableReader) #else _netCoreOrNetFXPluginPaths = environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths); #endif - _nuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); + + if (string.IsNullOrEmpty(_netCoreOrNetFXPluginPaths)) + { + _nuGetPluginPaths = _environmentVariableReader.GetEnvironmentVariable(EnvironmentVariableConstants.PluginPaths); + } + _semaphore = new SemaphoreSlim(initialCount: 1, maxCount: 1); } @@ -111,11 +116,11 @@ public async Task> DiscoverAsync(Cancellation // Search for .Net tools plugins in PATH if (_pluginFiles != null) { - _pluginFiles.AddRange(GetPluginsInPATH() ?? new List()); + _pluginFiles.AddRange(GetPluginsInPath()); } else { - _pluginFiles = GetPluginsInPATH() ?? new List(); + _pluginFiles = GetPluginsInPath(); } } @@ -221,7 +226,7 @@ internal List GetPluginsInNuGetPluginPaths() /// Retrieves .NET tools authentication plugins by searching through directories specified in `PATH` /// /// A list of valid objects representing the discovered plugins. - internal List GetPluginsInPATH() + internal List GetPluginsInPath() { var pluginFiles = new List(); var nugetPluginPaths = _environmentVariableReader.GetEnvironmentVariable("PATH"); diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index afbfefb906b..18120e7eec5 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -469,7 +469,7 @@ public void GetPluginsInPATH_WithPATHSet_ReturnsPlugin() PluginDiscoverer pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetPluginsInPATH(); + var plugins = pluginDiscoverer.GetPluginsInPath(); // Assert Assert.Single(plugins); @@ -538,7 +538,7 @@ public void GetPluginsInPATH_PATHPointsToADirectory_ContainsValidPluginFiles() var pluginDiscoverer = new PluginDiscoverer(environmentalVariableReader.Object); // Act - var plugins = pluginDiscoverer.GetPluginsInPATH(); + var plugins = pluginDiscoverer.GetPluginsInPath(); // Assert Assert.Single(plugins); From 7f017a4e5ec9142fafbdd79f6c7cdfb2a4a73631 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 22 Oct 2024 11:06:48 -0700 Subject: [PATCH 47/47] cleanup --- .../NuGet.Protocol/Plugins/PluginDiscoverer.cs | 12 ++++++++---- .../Plugins/PluginDiscovererTests.cs | 10 ++++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs index 4bd8c4a520c..d342827a370 100644 --- a/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs +++ b/src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs @@ -322,15 +322,19 @@ internal static bool IsExecutable(FileInfo fileInfo) process.Start(); output = process.StandardOutput.ReadToEnd().Trim(); - if (!process.WaitForExit(1000) || process.ExitCode != 0) + if (!process.HasExited && !process.WaitForExit(1000)) { process.Kill(); return false; } - } + else if (process.ExitCode != 0) + { + return false; + } - // Check if the output is "yes" - return output.Equals("yes", StringComparison.OrdinalIgnoreCase); + // Check if the output is "yes" + return output.Equals("yes", StringComparison.OrdinalIgnoreCase); + } } catch { diff --git a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs index 18120e7eec5..f27749db149 100644 --- a/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs @@ -26,9 +26,15 @@ public void Constructor_AcceptsAnyString(string rawPluginPaths) var environmentalVariableReader = new Mock(); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.DesktopPluginPaths)).Returns(rawPluginPaths); environmentalVariableReader.Setup(env => env.GetEnvironmentVariable(EnvironmentVariableConstants.CorePluginPaths)).Returns(rawPluginPaths); - using (new PluginDiscoverer()) + + Exception exception = Record.Exception(() => { - } + using (new PluginDiscoverer()) + { + } + }); + + Assert.Null(exception); } [Fact]