NuGet/VSIX: propagate SignAsync(...) failures

src/Sign.Core/DataFormatSigners/ClickOnceSigner.cs

@@ -135,7 +135,7 @@ await Parallel.ForEachAsync(files, _parallelOptions, async (file, state) =>
                     {
                         string message = string.Format(CultureInfo.CurrentCulture, Resources.SigningFailed, manifestFile.FullName);
 
-                        throw new Exception(message);
+                        throw new SigningException(message);
                     }
 
                     string publisherParam = string.Empty;
@@ -178,7 +178,7 @@ await Parallel.ForEachAsync(files, _parallelOptions, async (file, state) =>
                         {
                             string message = string.Format(CultureInfo.CurrentCulture, Resources.SigningFailed, deploymentManifestFile.FullName);
 
-                            throw new Exception(message);
+                            throw new SigningException(message);
                         }
                     }
 
@@ -274,4 +274,4 @@ public void CopySigningDependencies(FileInfo deploymentManifestFile, DirectoryIn
             }
         }
     }
-}
+}

src/Sign.Core/DataFormatSigners/NuGetSigner.cs

@@ -2,6 +2,7 @@
 // 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.Globalization;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
 using Microsoft.Extensions.Logging;
@@ -47,9 +48,28 @@ public async Task SignAsync(IEnumerable<FileInfo> files, SignOptions options)
             using (X509Certificate2 certificate = await _certificateProvider.GetCertificateAsync())
             using (RSA rsa = await _signatureAlgorithmProvider.GetRsaAsync())
             {
-                IEnumerable<Task<bool>> tasks = files.Select(file => SignAsync(args: null, file, rsa, certificate, options));
+                var fileTaskPairs = files
+                    .Select(file => new
+                    {
+                        File = file,
+                        Task = SignAsync(args: null, file, rsa, certificate, options)
+                    })
+                    .ToList();
 
-                await Task.WhenAll(tasks);
+                await Task.WhenAll(fileTaskPairs.Select(pair => pair.Task));
+
+                List<string> failedFiles = fileTaskPairs
+                    .Where(pair => !pair.Task.Result)
+                    .Select(pair => pair.File.FullName)
+                    .ToList();
+
+                if (failedFiles.Count > 0)
+                {
+                    string failedFilePaths = string.Join(", ", failedFiles);
+                    string message = string.Format(CultureInfo.CurrentCulture, Resources.SigningFailed, failedFilePaths);
+
+                    throw new SigningException(message);
+                }
             }
         }
 
@@ -58,4 +78,4 @@ protected override Task<bool> SignCoreAsync(string? args, FileInfo file, RSA rsa
             return _nuGetSignTool.SignAsync(file, rsaPrivateKey, certificate, options);
         }
     }
-}
+}

src/Sign.Core/DataFormatSigners/RetryingSigner.cs

@@ -12,6 +12,9 @@ internal abstract class RetryingSigner
     {
         protected ILogger Logger { get; }
 
+        // Non-private for testing purposes.
+        internal TimeSpan Retry { get; set; } = TimeSpan.FromSeconds(5);
+
         protected RetryingSigner(ILogger logger)
         {
             ArgumentNullException.ThrowIfNull(logger, nameof(logger));
@@ -24,7 +27,7 @@ protected RetryingSigner(ILogger logger)
         // Inspired from https://github.com/squaredup/bettersigntool/blob/master/bettersigntool/bettersigntool/SignCommand.cs
         protected async Task<bool> SignAsync(string? args, FileInfo file, RSA rsaPrivateKey, X509Certificate2 publicCertificate, SignOptions options)
         {
-            var retry = TimeSpan.FromSeconds(5);
+            TimeSpan retry = Retry;
             const int maxAttempts = 3;
             var attempt = 1;
 
@@ -52,4 +55,4 @@ protected async Task<bool> SignAsync(string? args, FileInfo file, RSA rsaPrivate
             return false;
         }
     }
-}
+}

src/Sign.Core/DataFormatSigners/VsixSigner.cs

@@ -2,6 +2,7 @@
 // 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.Globalization;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
 using Microsoft.Extensions.Logging;
@@ -48,9 +49,28 @@ public async Task SignAsync(IEnumerable<FileInfo> files, SignOptions options)
             using (X509Certificate2 certificate = await _certificateProvider.GetCertificateAsync())
             using (RSA rsa = await _signatureAlgorithmProvider.GetRsaAsync())
             {
-                IEnumerable<Task<bool>> tasks = files.Select(file => SignAsync(args: null, file, rsa, certificate, options));
+                var fileTaskPairs = files
+                    .Select(file => new
+                    {
+                        File = file,
+                        Task = SignAsync(args: null, file, rsa, certificate, options)
+                    })
+                    .ToList();
 
-                await Task.WhenAll(tasks);
+                await Task.WhenAll(fileTaskPairs.Select(pair => pair.Task));
+
+                List<string> failedFiles = fileTaskPairs
+                    .Where(pair => !pair.Task.Result)
+                    .Select(pair => pair.File.FullName)
+                    .ToList();
+
+                if (failedFiles.Count > 0)
+                {
+                    string failedFilePaths = string.Join(", ", failedFiles);
+                    string message = string.Format(CultureInfo.CurrentCulture, Resources.SigningFailed, failedFilePaths);
+
+                    throw new SigningException(message);
+                }
             }
         }
 
@@ -66,4 +86,4 @@ protected override async Task<bool> SignCoreAsync(string? args, FileInfo file, R
             return await _vsixSignTool.SignAsync(file, configuration, options);
         }
     }
-}
+}

src/Sign.Core/Sign.Core.csproj

@@ -31,6 +31,7 @@
     <InternalsVisibleTo Include="Sign.SignatureProviders.KeyVault.Test" />
     <InternalsVisibleTo Include="Sign.SignatureProviders.TrustedSigning" />
     <InternalsVisibleTo Include="Sign.SignatureProviders.TrustedSigning.Test" />
+    <InternalsVisibleTo Include="Sign.TestInfrastructure" />
   </ItemGroup>
 
   <ItemGroup>

src/Sign.Core/Tools/IVsixSignTool.cs

@@ -2,12 +2,10 @@
 // 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 Sign.Core;
-
 namespace Sign.Core
 {
     internal interface IVsixSignTool : ITool
     {
         Task<bool> SignAsync(FileInfo file, SignConfigurationSet configuration, SignOptions options);
     }
-}
+}

test/Sign.Core.Test/DataFormatSigners/ClickOnceSignerTests.cs

@@ -197,6 +197,86 @@ public async Task SignAsync_WhenOptionsIsNull_Throws()
             Assert.Equal("options", exception.ParamName);
         }
 
+        [Fact]
+        public async Task SignAsync_WhenSigningFails_Throws()
+        {
+            using (TemporaryDirectory temporaryDirectory = new(_directoryService))
+            {
+                FileInfo clickOnceFile = new(
+                    Path.Combine(
+                        temporaryDirectory.Directory.FullName,
+                        $"{Path.GetRandomFileName()}.clickonce"));
+
+                ContainerSpy containerSpy = new(clickOnceFile);
+
+                FileInfo applicationFile = AddFile(
+                    containerSpy,
+                    temporaryDirectory.Directory,
+                    string.Empty,
+                    "MyApp.application");
+
+                SignOptions options = new(
+                    "ApplicationName",
+                    "PublisherName",
+                    "Description",
+                    new Uri("https://description.test"),
+                    HashAlgorithmName.SHA256,
+                    HashAlgorithmName.SHA256,
+                    new Uri("http://timestamp.test"),
+                    matcher: null,
+                    antiMatcher: null);
+
+                using (X509Certificate2 certificate = SelfIssuedCertificateCreator.CreateCertificate())
+                using (RSA privateKey = certificate.GetRSAPrivateKey()!)
+                {
+                    Mock<ISignatureAlgorithmProvider> signatureAlgorithmProvider = new();
+                    Mock<ICertificateProvider> certificateProvider = new();
+
+                    certificateProvider.Setup(x => x.GetCertificateAsync(It.IsAny<CancellationToken>()))
+                        .ReturnsAsync(certificate);
+
+                    signatureAlgorithmProvider.Setup(x => x.GetRsaAsync(It.IsAny<CancellationToken>()))
+                        .ReturnsAsync(privateKey);
+
+                    Mock<IServiceProvider> serviceProvider = new();
+                    AggregatingSignerSpy aggregatingSignerSpy = new();
+
+                    serviceProvider.Setup(x => x.GetService(It.IsAny<Type>()))
+                        .Returns(aggregatingSignerSpy);
+
+                    Mock<IMageCli> mageCli = new();
+
+                    mageCli.Setup(x => x.RunAsync(
+                            It.IsAny<string>()))
+                        .ReturnsAsync(1);
+
+                    Mock<IManifestSigner> manifestSigner = new();
+                    Mock<IFileMatcher> fileMatcher = new();
+                    Mock<ILogger<IDataFormatSigner>> logger = new();
+
+                    manifestSigner.Setup(
+                        x => x.Sign(
+                            It.Is<FileInfo>(fi => fi.Name == applicationFile.Name),
+                            It.Is<X509Certificate2>(c => ReferenceEquals(certificate, c)),
+                            It.Is<RSA>(rsa => ReferenceEquals(privateKey, rsa)),
+                            It.Is<SignOptions>(o => ReferenceEquals(options, o))));
+
+                    ClickOnceSigner signer = new(
+                        signatureAlgorithmProvider.Object,
+                        certificateProvider.Object,
+                        serviceProvider.Object,
+                        mageCli.Object,
+                        manifestSigner.Object,
+                        logger.Object,
+                        fileMatcher.Object);
+
+                    signer.Retry = TimeSpan.FromMicroseconds(1);
+
+                    await Assert.ThrowsAsync<SigningException>(() => signer.SignAsync(new[] { applicationFile }, options));
+                }
+            }
+        }
+
         [Theory]
         [InlineData(null)]
         [InlineData("PublisherName")]

test/Sign.Core.Test/DataFormatSigners/NuGetSignerTests.cs

@@ -2,8 +2,11 @@
 // 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.Security.Cryptography;
+using System.Security.Cryptography.X509Certificates;
 using Microsoft.Extensions.Logging;
 using Moq;
+using Sign.TestInfrastructure;
 
 namespace Sign.Core.Test
 {
@@ -99,5 +102,51 @@ public void CanSign_WhenFileExtensionDoesNotMatch_ReturnsFalse()
 
             Assert.False(_signer.CanSign(file));
         }
+
+        [Fact]
+        public async Task SignAsync_WhenSigningFails_Throws()
+        {
+            Mock<INuGetSignTool> nuGetSignTool = new();
+
+            nuGetSignTool.Setup(
+                x => x.SignAsync(
+                    It.IsNotNull<FileInfo>(),
+                    It.IsNotNull<RSA>(),
+                    It.IsNotNull<X509Certificate2>(),
+                    It.IsNotNull<SignOptions>()))
+                .Returns(Task.FromResult(false));
+
+            NuGetSigner signer = new(
+                Mock.Of<ISignatureAlgorithmProvider>(),
+                Mock.Of<ICertificateProvider>(),
+                nuGetSignTool.Object,
+                Mock.Of<ILogger<IDataFormatSigner>>());
+
+            signer.Retry = TimeSpan.FromMicroseconds(1);
+
+            SignOptions options = new(
+                "ApplicationName",
+                "PublisherName",
+                "Description",
+                new Uri("https://description.test"),
+                HashAlgorithmName.SHA384,
+                HashAlgorithmName.SHA384,
+                new Uri("http://timestamp.test"),
+                matcher: null,
+                antiMatcher: null);
+
+            DirectoryService directoryService = new(Mock.Of<ILogger<IDirectoryService>>());
+
+            using (TemporaryDirectory temporaryDirectory = new(directoryService))
+            {
+                FileInfo nupkgFile = TestFileCreator.CreateEmptyZipFile(temporaryDirectory, fileExtension: ".nupkg");
+
+                using (X509Certificate2 certificate = SelfIssuedCertificateCreator.CreateCertificate())
+                using (RSA privateKey = certificate.GetRSAPrivateKey()!)
+                {
+                    await Assert.ThrowsAsync<SigningException>(() => signer.SignAsync([nupkgFile], options));
+                }
+            }
+        }
     }
-}
+}

test/Sign.Core.Test/DataFormatSigners/VsixSignerTests.cs

@@ -2,8 +2,11 @@
 // 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.Security.Cryptography;
+using System.Security.Cryptography.X509Certificates;
 using Microsoft.Extensions.Logging;
 using Moq;
+using Sign.TestInfrastructure;
 
 namespace Sign.Core.Test
 {
@@ -101,5 +104,58 @@ public void CanSign_WhenFileExtensionDoesNotMatch_ReturnsFalse(string extension)
 
             Assert.False(_signer.CanSign(file));
         }
+
+        [Fact]
+        public async Task SignAsync_WhenSigningFails_Throws()
+        {
+            SignOptions options = new(
+                "ApplicationName",
+                "PublisherName",
+                "Description",
+                new Uri("https://description.test"),
+                HashAlgorithmName.SHA384,
+                HashAlgorithmName.SHA384,
+                new Uri("http://timestamp.test"),
+                matcher: null,
+                antiMatcher: null);
+
+            DirectoryService directoryService = new(Mock.Of<ILogger<IDirectoryService>>());
+
+            using (TemporaryDirectory temporaryDirectory = new(directoryService))
+            {
+                FileInfo vsixFile = TestFileCreator.CreateEmptyZipFile(temporaryDirectory, fileExtension: ".vsix");
+
+                using (X509Certificate2 certificate = SelfIssuedCertificateCreator.CreateCertificate())
+                using (RSA privateKey = certificate.GetRSAPrivateKey()!)
+                {
+                    Mock<IVsixSignTool> vsixSignTool = new();
+
+                    SignConfigurationSet configuration = new(
+                        options.FileHashAlgorithm,
+                        options.FileHashAlgorithm,
+                        privateKey,
+                        certificate);
+
+                    vsixSignTool.Setup(
+                        x => x.SignAsync(
+                            It.IsNotNull<FileInfo>(),
+                            It.IsNotNull<SignConfigurationSet>(),
+                            It.IsNotNull<SignOptions>()))
+                        .Returns(Task.FromResult(false));
+
+                    VsixSigner signer = new(
+                        Mock.Of<ISignatureAlgorithmProvider>(),
+                        Mock.Of<ICertificateProvider>(),
+                        vsixSignTool.Object,
+                        Mock.Of<ILogger<IDataFormatSigner>>());
+
+                    signer.Retry = TimeSpan.FromMicroseconds(1);
+
+                    await Assert.ThrowsAsync<SigningException>(() => signer.SignAsync([vsixFile], options));
+                }
+            }
+        }
+
+
     }
-}
+}