Enable signing already signed NuGet package (#569)

Resolve #547.

src/Sign.Core/Containers/ContainerProvider.cs

@@ -15,6 +15,7 @@ internal sealed class ContainerProvider : IContainerProvider
         private readonly IKeyVaultService _keyVaultService;
         private readonly ILogger _logger;
         private readonly IMakeAppxCli _makeAppxCli;
+        private readonly HashSet<string> _nuGetExtensions;
         private readonly HashSet<string> _zipExtensions;
 
         // Dependency injection requires a public constructor.
@@ -53,13 +54,17 @@ public ContainerProvider(
                 ".msix"
             };
 
+            _nuGetExtensions = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
+            {
+                ".nupkg",
+                ".snupkg"
+            };
+
             _zipExtensions = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
             {
                 ".appxupload",
                 ".clickonce",
                 ".msixupload",
-                ".nupkg",
-                ".snupkg",
                 ".vsix",
                 ".zip"
             };
@@ -79,6 +84,13 @@ public bool IsAppxContainer(FileInfo file)
             return _appxExtensions.Contains(file.Extension);
         }
 
+        public bool IsNuGetContainer(FileInfo file)
+        {
+            ArgumentNullException.ThrowIfNull(file, nameof(file));
+
+            return _nuGetExtensions.Contains(file.Extension);
+        }
+
         public bool IsZipContainer(FileInfo file)
         {
             ArgumentNullException.ThrowIfNull(file, nameof(file));
@@ -105,6 +117,11 @@ public bool IsZipContainer(FileInfo file)
                 return new ZipContainer(file, _directoryService, _fileMatcher, _logger);
             }
 
+            if (IsNuGetContainer(file))
+            {
+                return new NuGetContainer(file, _directoryService, _fileMatcher, _logger);
+            }
+
             return null;
         }
     }

src/Sign.Core/Containers/IContainerProvider.cs

@@ -8,6 +8,7 @@ internal interface IContainerProvider
     {
         bool IsAppxBundleContainer(FileInfo file);
         bool IsAppxContainer(FileInfo file);
+        bool IsNuGetContainer(FileInfo file);
         bool IsZipContainer(FileInfo file);
         IContainer? GetContainer(FileInfo file);
     }

src/Sign.Core/Containers/NuGetContainer.cs

@@ -0,0 +1,40 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE.txt file in the project root for more information.
+
+using Microsoft.Extensions.Logging;
+using NuGet.Packaging.Signing;
+
+namespace Sign.Core
+{
+    internal sealed class NuGetContainer : ZipContainer
+    {
+        internal NuGetContainer(
+            FileInfo zipFile,
+            IDirectoryService directoryService,
+            IFileMatcher fileMatcher, ILogger logger)
+            : base(zipFile, directoryService, fileMatcher, logger)
+        {
+        }
+
+        public override ValueTask SaveAsync()
+        {
+            if (TemporaryDirectory is null)
+            {
+                throw new InvalidOperationException();
+            }
+
+            FileInfo signatureFile = new(
+                Path.Combine(
+                    TemporaryDirectory.Directory.FullName,
+                    SigningSpecifications.V1.SignaturePath));
+
+            if (signatureFile.Exists)
+            {
+                signatureFile.Delete();
+            }
+
+            return base.SaveAsync();
+        }
+    }
+}

src/Sign.Core/Containers/ZipContainer.cs

@@ -7,7 +7,7 @@
 
 namespace Sign.Core
 {
-    internal sealed class ZipContainer : Container
+    internal class ZipContainer : Container
     {
         private readonly IDirectoryService _directoryService;
         private readonly ILogger _logger;

src/Sign.Core/SignatureProviders/AggregatingSignatureProvider.cs

@@ -64,7 +64,7 @@ public async Task SignAsync(IEnumerable<FileInfo> files, SignOptions options)
 
             // See if any of them are archives
             List<FileInfo> archives = (from file in files
-                                       where _containerProvider.IsZipContainer(file)
+                                       where _containerProvider.IsZipContainer(file) || _containerProvider.IsNuGetContainer(file)
                                        select file).ToList();
 
             // expand the archives and sign recursively first

test/Sign.Core.Test/Containers/ContainerProviderTests.cs

@@ -91,7 +91,6 @@ public void Constructor_WhenLoggerIsNull_Throws()
             Assert.Equal("logger", exception.ParamName);
         }
 
-
         [Fact]
         public void IsAppxBundleContainer_WhenFileIsNull_Throws()
         {
@@ -158,6 +157,35 @@ public void IsAppxContainer_WhenFileExtensionMatches_ReturnsTrue(string extensio
             Assert.True(_provider.IsAppxContainer(file));
         }
 
+        [Fact]
+        public void IsNuGetContainer_WhenFileIsNull_Throws()
+        {
+            ArgumentNullException exception = Assert.Throws<ArgumentNullException>(
+                () => _provider.IsNuGetContainer(file: null!));
+
+            Assert.Equal("file", exception.ParamName);
+        }
+
+        [Theory]
+        [InlineData(".zip")]
+        public void IsNuGetContainer_WhenFileExtensionDoesNotMatch_ReturnsFalse(string extension)
+        {
+            FileInfo file = new($"file{extension}");
+
+            Assert.False(_provider.IsNuGetContainer(file));
+        }
+
+        [Theory]
+        [InlineData(".nupkg")]
+        [InlineData(".snupkg")]
+        [InlineData(".NuPkg")] // test case insensitivity
+        public void IsNuGetContainer_WhenFileExtensionMatches_ReturnsTrue(string extension)
+        {
+            FileInfo file = new($"file{extension}");
+
+            Assert.True(_provider.IsNuGetContainer(file));
+        }
+
         [Fact]
         public void IsZipContainer_WhenFileIsNull_Throws()
         {
@@ -182,8 +210,6 @@ public void IsZipContainer_WhenFileExtensionDoesNotMatch_ReturnsFalse(string ext
         [InlineData(".appxupload")]
         [InlineData(".clickonce")]
         [InlineData(".msixupload")]
-        [InlineData(".nupkg")]
-        [InlineData(".snupkg")]
         [InlineData(".vsix")]
         [InlineData(".zip")]
         [InlineData(".ZIP")] // test case insensitivity

test/Sign.Core.Test/Containers/NuGetContainerTests.cs

@@ -0,0 +1,181 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE.txt file in the project root for more information.
+
+using System.IO.Compression;
+using System.Text;
+using Microsoft.Extensions.Logging;
+using Moq;
+using NuGet.Packaging.Signing;
+
+namespace Sign.Core.Test
+{
+    public class NuGetContainerTests
+    {
+        [Fact]
+        public void Constructor_WhenZipFileIsNull_Throws()
+        {
+            ArgumentNullException exception = Assert.Throws<ArgumentNullException>(
+                () => new NuGetContainer(
+                    zipFile: null!,
+                    Mock.Of<IDirectoryService>(),
+                    Mock.Of<IFileMatcher>(),
+                    Mock.Of<ILogger>()));
+
+            Assert.Equal("zipFile", exception.ParamName);
+        }
+
+        [Fact]
+        public void Constructor_WhenDirectoryServiceIsNull_Throws()
+        {
+            ArgumentNullException exception = Assert.Throws<ArgumentNullException>(
+                () => new NuGetContainer(
+                    new FileInfo("a"),
+                    directoryService: null!,
+                    Mock.Of<IFileMatcher>(),
+                    Mock.Of<ILogger>()));
+
+            Assert.Equal("directoryService", exception.ParamName);
+        }
+
+        [Fact]
+        public void Constructor_WhenFileMatcherIsNull_Throws()
+        {
+            ArgumentNullException exception = Assert.Throws<ArgumentNullException>(
+                () => new NuGetContainer(
+                    new FileInfo("a"),
+                    Mock.Of<IDirectoryService>(),
+                    fileMatcher: null!,
+                    Mock.Of<ILogger>()));
+
+            Assert.Equal("fileMatcher", exception.ParamName);
+        }
+
+        [Fact]
+        public void Constructor_WhenLoggerIsNull_Throws()
+        {
+            ArgumentNullException exception = Assert.Throws<ArgumentNullException>(
+                () => new NuGetContainer(
+                    new FileInfo("a"),
+                    Mock.Of<IDirectoryService>(),
+                    Mock.Of<IFileMatcher>(),
+                    logger: null!));
+
+            Assert.Equal("logger", exception.ParamName);
+        }
+
+        [Fact]
+        public async Task Dispose_WhenOpened_RemovesTemporaryDirectory()
+        {
+            string[] expectedFileNames = new[] { "a" };
+            FileInfo zipFile = CreateZipFile(expectedFileNames);
+
+            using (DirectoryServiceStub directoryService = new())
+            {
+                DirectoryInfo? directory;
+
+                using (NuGetContainer container = new(zipFile, directoryService, Mock.Of<IFileMatcher>(), Mock.Of<ILogger>()))
+                {
+                    await container.OpenAsync();
+
+                    directory = Assert.Single(directoryService.Directories);
+
+                    Assert.True(directory.Exists);
+                }
+
+                directory = Assert.Single(directoryService.Directories);
+
+                Assert.False(directory.Exists);
+            }
+        }
+
+        [Fact]
+        public async Task OpenAsync_WhenNupkgFileIsNonEmpty_ExtractsNupkgToDirectory()
+        {
+            string[] expectedFileNames = new[] { ".a", "b", "c.d" };
+            FileInfo zipFile = CreateZipFile(expectedFileNames);
+
+            using (DirectoryServiceStub directoryService = new())
+            using (NuGetContainer container = new(zipFile, directoryService, Mock.Of<IFileMatcher>(), Mock.Of<ILogger>()))
+            {
+                await container.OpenAsync();
+
+                FileInfo[] actualFiles = directoryService.Directories[0].GetFiles("*", SearchOption.AllDirectories);
+                string[] actualFileNames = actualFiles
+                    .Select(file => file.FullName.Substring(directoryService.Directories[0].FullName.Length + 1))
+                    .ToArray();
+
+                Assert.Equal(expectedFileNames, actualFileNames);
+            }
+        }
+
+        [Fact]
+        public async Task SaveAsync_WhenNupkgFileIsNonEmpty_CompressesNupkgFromDirectory()
+        {
+            string[] fileNames = new[] { "a" };
+            FileInfo zipFile = CreateZipFile(fileNames);
+
+            using (DirectoryServiceStub directoryService = new())
+            using (NuGetContainer container = new(zipFile, directoryService, Mock.Of<IFileMatcher>(), Mock.Of<ILogger>()))
+            {
+                await container.OpenAsync();
+
+                File.WriteAllText(Path.Combine(directoryService.Directories[0].FullName, "b"), "b");
+
+                await container.SaveAsync();
+            }
+
+            using (FileStream stream = zipFile.OpenRead())
+            using (ZipArchive zip = new(stream, ZipArchiveMode.Read))
+            {
+                Assert.Equal(2, zip.Entries.Count);
+                Assert.NotNull(zip.GetEntry("a"));
+                Assert.NotNull(zip.GetEntry("b"));
+            }
+        }
+
+        [Fact]
+        public async Task SaveAsync_WhenNupkgFileHasSignatureFile_RemovesSignatureFile()
+        {
+            string[] fileNames = new[] { "a", SigningSpecifications.V1.SignaturePath };
+            FileInfo zipFile = CreateZipFile(fileNames);
+
+            using (DirectoryServiceStub directoryService = new())
+            using (NuGetContainer container = new(zipFile, directoryService, Mock.Of<IFileMatcher>(), Mock.Of<ILogger>()))
+            {
+                await container.OpenAsync();
+                await container.SaveAsync();
+            }
+
+            using (FileStream stream = zipFile.OpenRead())
+            using (ZipArchive zip = new(stream, ZipArchiveMode.Read))
+            {
+                ZipArchiveEntry entry = Assert.Single(zip.Entries);
+                Assert.Equal(fileNames[0], entry.Name);
+            }
+        }
+
+        private static FileInfo CreateZipFile(params string[] entryNames)
+        {
+            FileInfo file = new(Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()));
+
+            using (FileStream stream = file.OpenWrite())
+            using (ZipArchive zip = new(stream, ZipArchiveMode.Create))
+            {
+                foreach (string entryName in entryNames)
+                {
+                    ZipArchiveEntry entry = zip.CreateEntry(entryName);
+
+                    using (Stream entryStream = entry.Open())
+                    {
+                        byte[] bytes = Encoding.UTF8.GetBytes(entryName);
+
+                        entryStream.Write(bytes);
+                    }
+                }
+            }
+
+            return file;
+        }
+    }
+}