ClickOnce: improve application manifest file detection

src/Sign.Core/DataFormatSigners/ClickOnceSigner.cs

@@ -2,9 +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.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
+using System.Xml;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.FileSystemGlobbing.Abstractions;
 using Microsoft.Extensions.Logging;
@@ -20,6 +22,7 @@ internal sealed class ClickOnceSigner : RetryingSigner, IDataFormatSigner
         private readonly IManifestSigner _manifestSigner;
         private readonly ParallelOptions _parallelOptions = new() { MaxDegreeOfParallelism = 4 };
         private readonly IFileMatcher _fileMatcher;
+        private readonly IXmlDocumentLoader _xmlDocumentLoader;
 
         // Dependency injection requires a public constructor.
         public ClickOnceSigner(
@@ -29,7 +32,8 @@ public ClickOnceSigner(
             IMageCli mageCli,
             IManifestSigner manifestSigner,
             ILogger<IDataFormatSigner> logger,
-            IFileMatcher fileMatcher)
+            IFileMatcher fileMatcher,
+            IXmlDocumentLoader xmlDocumentLoader)
             : base(logger)
         {
             ArgumentNullException.ThrowIfNull(signatureAlgorithmProvider, nameof(signatureAlgorithmProvider));
@@ -38,12 +42,14 @@ public ClickOnceSigner(
             ArgumentNullException.ThrowIfNull(mageCli, nameof(mageCli));
             ArgumentNullException.ThrowIfNull(manifestSigner, nameof(manifestSigner));
             ArgumentNullException.ThrowIfNull(fileMatcher, nameof(fileMatcher));
+            ArgumentNullException.ThrowIfNull(xmlDocumentLoader, nameof(xmlDocumentLoader));
 
             _signatureAlgorithmProvider = signatureAlgorithmProvider;
             _certificateProvider = certificateProvider;
             _mageCli = mageCli;
             _manifestSigner = manifestSigner;
             _fileMatcher = fileMatcher;
+            _xmlDocumentLoader = xmlDocumentLoader;
 
             // Need to delay this as it'd create a dependency loop if directly in the ctor
             _aggregatingSigner = new Lazy<IAggregatingDataFormatSigner>(() => serviceProvider.GetService<IAggregatingDataFormatSigner>()!);
@@ -126,14 +132,17 @@ await Parallel.ForEachAsync(files, _parallelOptions, async (file, state) =>
 
                     // Inner files are now signed
                     // now look for the manifest file and sign that if we have one
+                    FileInfo? applicationManifestFile = null;
+                    if (TryGetApplicationManifestFileName(file, out string? fileName))
+                    {
+                        applicationManifestFile = filteredFiles.SingleOrDefault(f => f.Name.Equals(fileName, StringComparison.OrdinalIgnoreCase));
+                    }
 
-                    FileInfo? manifestFile = filteredFiles.SingleOrDefault(f => ".manifest".Equals(f.Extension, StringComparison.OrdinalIgnoreCase));
-
-                    string fileArgs = $@"-update ""{manifestFile}"" {args}";
+                    string fileArgs = $@"-update ""{applicationManifestFile}"" {args}";
 
-                    if (manifestFile is not null && !await SignAsync(fileArgs, manifestFile, rsaPrivateKey, certificate, options))
+                    if (applicationManifestFile is not null && !await SignAsync(fileArgs, applicationManifestFile, rsaPrivateKey, certificate, options))
                     {
-                        string message = string.Format(CultureInfo.CurrentCulture, Resources.SigningFailed, manifestFile.FullName);
+                        string message = string.Format(CultureInfo.CurrentCulture, Resources.SigningFailed, applicationManifestFile.FullName);
 
                         throw new Exception(message);
                     }
@@ -143,7 +152,7 @@ await Parallel.ForEachAsync(files, _parallelOptions, async (file, state) =>
                     if (string.IsNullOrEmpty(options.PublisherName))
                     {
                         string publisherName = certificate.SubjectName.Name;
- 
+
                         // get the DN. it may be quoted
                         publisherParam = $@"-pub ""{publisherName.Replace("\"", "")}""";
                     }
@@ -165,9 +174,9 @@ await Parallel.ForEachAsync(files, _parallelOptions, async (file, state) =>
                     foreach (FileInfo deploymentManifestFile in deploymentManifestFiles)
                     {
                         fileArgs = $@"-update ""{deploymentManifestFile.FullName}"" {args} {publisherParam}";
-                        if (manifestFile is not null)
+                        if (applicationManifestFile is not null)
                         {
-                            fileArgs += $@" -appm ""{manifestFile.FullName}""";
+                            fileArgs += $@" -appm ""{applicationManifestFile.FullName}""";
                         }
                         if (options.DescriptionUrl is not null)
                         {
@@ -227,7 +236,6 @@ protected override async Task<bool> SignCoreAsync(string? args, FileInfo file, R
             return false;
         }
 
-
         private IEnumerable<FileInfo> GetFiles(DirectoryInfo clickOnceRoot)
         {
             return clickOnceRoot.EnumerateFiles("*", SearchOption.AllDirectories);
@@ -273,5 +281,60 @@ public void CopySigningDependencies(FileInfo deploymentManifestFile, DirectoryIn
                 }
             }
         }
+
+        /// <summary>
+        /// Try and find the application manifest (.manifest) file from a ClickOnce application manifest (.application / .vsto
+        /// There might not be one, if the user is attempting to only re-sign the deployment manifest without touching other files.
+        /// This is necessary because there might be multiple *.manifest files present, e.g. if a DLL that's part of the ClickOnce
+        /// package ships its own assembly manifest which isn't a ClickOnce application manifest.
+        /// </summary>
+        /// <param name="deploymentManifest">A <see cref="FileInfo"/> representing a deployment manifest file.</param>
+        /// <param name="applicationManifestFileName">A <see cref="string?"/> representing a manifest file name or <c>null</c> if one isn't found.</param>
+        /// <returns><c>true</c> if the application manifest file name was found; otherwise, <c>false</c>.</returns>
+        /// <remarks>This is non-private only for unit testing.</remarks>
+        internal bool TryGetApplicationManifestFileName(FileInfo deploymentManifest, [NotNullWhen(true)] out string? applicationManifestFileName)
+        {
+            applicationManifestFileName = null;
+
+            XmlDocument xmlDoc = _xmlDocumentLoader.Load(deploymentManifest);
+
+            // there should only be a single result here, if the file is a valid clickonce manifest.
+            XmlNodeList dependentAssemblies = xmlDoc.GetElementsByTagName("dependentAssembly");
+            if (dependentAssemblies.Count != 1)
+            {
+                Logger.LogDebug(Resources.ApplicationManifestNotFound);
+                return false;
+            }
+
+            XmlNode? node = dependentAssemblies.Item(0);
+            if (node is null || node.Attributes is null)
+            {
+                Logger.LogDebug(Resources.ApplicationManifestNotFound);
+                return false;
+            }
+
+            XmlAttribute? codebaseAttribute = node.Attributes["codebase"];
+            if (codebaseAttribute is null || string.IsNullOrEmpty(codebaseAttribute.Value))
+            {
+                Logger.LogDebug(Resources.ApplicationManifestNotFound);
+                return false;
+            }
+
+            // The codebase attribute can be a relative file path (e.g. Application Files\MyApp_1_0_0_0\MyApp.dll.manifest) or
+            // a URI (e.g. https://my.cdn.com/clickonce/MyApp/ApplicationFiles/MyApp_1_0_0_0/MyApp.dll.manifest) so we need to
+            // handle both cases and extract just the file name part.
+            //
+            // We only try and parse absolute URI's, because a relative URI can just be treated like a file path for our purposes.
+            if (Uri.TryCreate(codebaseAttribute.Value, UriKind.Absolute, out Uri? uri))
+            {
+                applicationManifestFileName = Path.GetFileName(uri.LocalPath); // works for http(s) and file:// uris
+            }
+            else
+            {
+                applicationManifestFileName = Path.GetFileName(codebaseAttribute.Value);
+            }
+
+            return !string.IsNullOrEmpty(applicationManifestFileName);
+        }
     }
-}
+}

src/Sign.Core/Resources.resx

@@ -275,4 +275,8 @@
   <data name="VSIXSignToolUnsupportedAlgorithm" xml:space="preserve">
     <value>The algorithm selected is not supported.</value>
   </data>
+  <data name="ApplicationManifestNotFound" xml:space="preserve">
+    <value>Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</value>
+    <comment>{Locked="&lt;dependentAssembly&gt;"} is an XML element.  {Locked="codebase"} is an XML attribute.</comment>
+  </data>
 </root>

src/Sign.Core/ServiceProvider.cs

@@ -75,6 +75,7 @@ internal static ServiceProvider CreateDefault(
             services.AddSingleton<INuGetSignTool, NuGetSignTool>();
             services.AddSingleton<IVsixSignTool, VsixSignTool>();
             services.AddSingleton<ICertificateVerifier, CertificateVerifier>();
+            services.AddSingleton<IXmlDocumentLoader, XmlDocumentLoader>();
             services.AddSingleton<ISigner, Signer>();
 
             if (addServices is not null)
@@ -85,4 +86,4 @@ internal static ServiceProvider CreateDefault(
             return new ServiceProvider(services.BuildServiceProvider());
         }
     }
-}
+}

src/Sign.Core/Xml/IXmlDocumentLoader.cs

@@ -0,0 +1,13 @@
+// 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.Xml;
+
+namespace Sign.Core
+{
+    internal interface IXmlDocumentLoader
+    {
+        XmlDocument Load(FileInfo file);
+    }
+}

src/Sign.Core/Xml/XmlDocumentLoader.cs

@@ -0,0 +1,30 @@
+// 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.Xml;
+
+namespace Sign.Core
+{
+    internal sealed class XmlDocumentLoader : IXmlDocumentLoader
+    {
+        public XmlDocument Load(FileInfo file)
+        {
+            ArgumentNullException.ThrowIfNull(file);
+
+            XmlReaderSettings settings = new()
+            {
+                DtdProcessing = DtdProcessing.Prohibit,
+                XmlResolver = null // prevent external entity resolution
+            };
+            XmlDocument xmlDoc = new();
+
+            using (XmlReader reader = XmlReader.Create(file.FullName, settings))
+            {
+                xmlDoc.Load(reader);
+            }
+
+            return xmlDoc;
+        }
+    }
+}

src/Sign.Core/xlf/Resources.cs.xlf

@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="cs" original="../Resources.resx">
     <body>
+      <trans-unit id="ApplicationManifestNotFound">
+        <source>Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</source>
+        <target state="new">Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</target>
+        <note>{Locked="&lt;dependentAssembly&gt;"} is an XML element.  {Locked="codebase"} is an XML attribute.</note>
+      </trans-unit>
       <trans-unit id="AzureSignToolSignatureProviderSigning">
         <source>Signing SignTool job with {count} files.</source>
         <target state="translated">Podepisování úlohy SignTool s tímto počtem souborů: {count}.</target>

src/Sign.Core/xlf/Resources.de.xlf

@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="de" original="../Resources.resx">
     <body>
+      <trans-unit id="ApplicationManifestNotFound">
+        <source>Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</source>
+        <target state="new">Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</target>
+        <note>{Locked="&lt;dependentAssembly&gt;"} is an XML element.  {Locked="codebase"} is an XML attribute.</note>
+      </trans-unit>
       <trans-unit id="AzureSignToolSignatureProviderSigning">
         <source>Signing SignTool job with {count} files.</source>
         <target state="translated">Der SignTool-Auftrag wird mit {count} Dateien signiert.</target>

src/Sign.Core/xlf/Resources.es.xlf

@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="es" original="../Resources.resx">
     <body>
+      <trans-unit id="ApplicationManifestNotFound">
+        <source>Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</source>
+        <target state="new">Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</target>
+        <note>{Locked="&lt;dependentAssembly&gt;"} is an XML element.  {Locked="codebase"} is an XML attribute.</note>
+      </trans-unit>
       <trans-unit id="AzureSignToolSignatureProviderSigning">
         <source>Signing SignTool job with {count} files.</source>
         <target state="translated">Firmando el trabajo de SignTool con {count} archivos.</target>

src/Sign.Core/xlf/Resources.fr.xlf

@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="fr" original="../Resources.resx">
     <body>
+      <trans-unit id="ApplicationManifestNotFound">
+        <source>Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</source>
+        <target state="new">Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</target>
+        <note>{Locked="&lt;dependentAssembly&gt;"} is an XML element.  {Locked="codebase"} is an XML attribute.</note>
+      </trans-unit>
       <trans-unit id="AzureSignToolSignatureProviderSigning">
         <source>Signing SignTool job with {count} files.</source>
         <target state="translated">Signature du travail SignTool avec {count} fichiers.</target>

src/Sign.Core/xlf/Resources.it.xlf

@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="it" original="../Resources.resx">
     <body>
+      <trans-unit id="ApplicationManifestNotFound">
+        <source>Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</source>
+        <target state="new">Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</target>
+        <note>{Locked="&lt;dependentAssembly&gt;"} is an XML element.  {Locked="codebase"} is an XML attribute.</note>
+      </trans-unit>
       <trans-unit id="AzureSignToolSignatureProviderSigning">
         <source>Signing SignTool job with {count} files.</source>
         <target state="translated">Firma del processo SignTool con {count} file.</target>

src/Sign.Core/xlf/Resources.ja.xlf

@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="ja" original="../Resources.resx">
     <body>
+      <trans-unit id="ApplicationManifestNotFound">
+        <source>Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</source>
+        <target state="new">Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</target>
+        <note>{Locked="&lt;dependentAssembly&gt;"} is an XML element.  {Locked="codebase"} is an XML attribute.</note>
+      </trans-unit>
       <trans-unit id="AzureSignToolSignatureProviderSigning">
         <source>Signing SignTool job with {count} files.</source>
         <target state="translated">{count} 個のファイルを使用して SignTool ジョブに署名しています。</target>

src/Sign.Core/xlf/Resources.ko.xlf

@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="ko" original="../Resources.resx">
     <body>
+      <trans-unit id="ApplicationManifestNotFound">
+        <source>Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</source>
+        <target state="new">Did not find exactly 1 &lt;dependentAssembly&gt; element with a non-empty 'codebase' attribute within the deployment manifest.</target>
+        <note>{Locked="&lt;dependentAssembly&gt;"} is an XML element.  {Locked="codebase"} is an XML attribute.</note>
+      </trans-unit>
       <trans-unit id="AzureSignToolSignatureProviderSigning">
         <source>Signing SignTool job with {count} files.</source>
         <target state="translated">{count} 파일로 SignTool 작업에 서명하는 중입니다.</target>