From 7685cd122675e37d4abffa4d29fb47ea1d82947f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Sat, 28 Mar 2026 22:11:07 +0100 Subject: [PATCH] CmdPal: Fix binary file corruption in Create Extension (#46490) ## Summary of the Pull Request This PR fixes a problem with invisible icons in newly create Command Palette extensions, when created through built-in command: - Avoids modifying binary files during extension creation from the template to prevent corruption. - Refactors template expansion and physical extension creation into a separate ExtensionTemplateService. - Adds unit tests. ## PR Checklist - [x] Closes: #46448 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .../Commands/NewExtensionForm.cs | 64 +----- .../Services/ExtensionTemplateService.cs | 129 ++++++++++++ .../Services/IExtensionTemplateService.cs | 20 ++ .../ExtensionTemplateServiceTests.cs | 188 ++++++++++++++++++ ...soft.CmdPal.UI.ViewModels.UnitTests.csproj | 6 + 5 files changed, 354 insertions(+), 53 deletions(-) create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ExtensionTemplateService.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/IExtensionTemplateService.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionTemplateServiceTests.cs diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/NewExtensionForm.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/NewExtensionForm.cs index c7f23535cd..4bdee41bc2 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/NewExtensionForm.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/NewExtensionForm.cs @@ -2,9 +2,9 @@ // The Microsoft Corporation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.IO.Compression; using System.Text.Json; using System.Text.Json.Nodes; +using Microsoft.CmdPal.UI.ViewModels.Services; using Microsoft.CommandPalette.Extensions; using Microsoft.CommandPalette.Extensions.Toolkit; @@ -13,6 +13,7 @@ namespace Microsoft.CmdPal.UI.ViewModels.BuiltinCommands; internal sealed partial class NewExtensionForm : NewExtensionFormBase { private static readonly string _creatingText = "Creating new extension..."; + private readonly IExtensionTemplateService _extensionTemplateService; private readonly StatusMessage _creatingMessage = new() { Message = _creatingText, @@ -20,7 +21,15 @@ internal sealed partial class NewExtensionForm : NewExtensionFormBase }; public NewExtensionForm() + : this(new ExtensionTemplateService()) { + } + + private NewExtensionForm(IExtensionTemplateService extensionTemplateService) + { + ArgumentNullException.ThrowIfNull(extensionTemplateService); + + _extensionTemplateService = extensionTemplateService; TemplateJson = $$""" { "$schema": "http://adaptivecards.io/schemas/adaptive-card.json", @@ -115,7 +124,7 @@ internal sealed partial class NewExtensionForm : NewExtensionFormBase try { - CreateExtension(extensionName, displayName, outputPath); + _extensionTemplateService.CreateExtension(extensionName, displayName, outputPath); BuiltinsExtensionHost.Instance.HideStatus(_creatingMessage); @@ -131,57 +140,6 @@ internal sealed partial class NewExtensionForm : NewExtensionFormBase return CommandResult.KeepOpen(); } - private void CreateExtension(string extensionName, string newDisplayName, string outputPath) - { - var newGuid = Guid.NewGuid().ToString(); - - // Unzip `template.zip` to a temp dir: - var tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - - // Does the output path exist? - if (!Directory.Exists(outputPath)) - { - Directory.CreateDirectory(outputPath); - } - - var assetsPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory.ToString(), "Microsoft.CmdPal.UI.ViewModels\\Assets\\template.zip"); - ZipFile.ExtractToDirectory(assetsPath, tempDir); - - var files = Directory.GetFiles(tempDir, "*", SearchOption.AllDirectories); - foreach (var file in files) - { - var text = File.ReadAllText(file); - - // Replace all the instances of `FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF` with a new random guid: - text = text.Replace("FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF", newGuid); - - // Then replace all the `TemplateCmdPalExtension` with `extensionName` - text = text.Replace("TemplateCmdPalExtension", extensionName); - - // Then replace all the `TemplateDisplayName` with `newDisplayName` - text = text.Replace("TemplateDisplayName", newDisplayName); - - // We're going to write the file to the same relative location in the output path - var relativePath = Path.GetRelativePath(tempDir, file); - - var newFileName = Path.Combine(outputPath, relativePath); - - // if the file name had `TemplateCmdPalExtension` in it, replace it with `extensionName` - newFileName = newFileName.Replace("TemplateCmdPalExtension", extensionName); - - // Make sure the directory exists - Directory.CreateDirectory(Path.GetDirectoryName(newFileName)!); - - File.WriteAllText(newFileName, text); - - // Delete the old file - File.Delete(file); - } - - // Delete the temp dir - Directory.Delete(tempDir, true); - } - private string FormatJsonString(string str) => // Escape the string for JSON diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ExtensionTemplateService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ExtensionTemplateService.cs new file mode 100644 index 0000000000..1e2b9a7938 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ExtensionTemplateService.cs @@ -0,0 +1,129 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.IO.Compression; + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +internal sealed class ExtensionTemplateService : IExtensionTemplateService +{ + internal enum TemplateFileHandling + { + ReplaceTokens, + CopyAsIs, + } + + private const string TemplateArchiveRelativePath = "Microsoft.CmdPal.UI.ViewModels\\Assets\\template.zip"; + + private static readonly HashSet _replaceTokensTemplateExtensions = new(StringComparer.OrdinalIgnoreCase) + { + ".appxmanifest", + ".config", + ".cs", + ".csproj", + ".json", + ".manifest", + ".props", + ".pubxml", + ".sln", + }; + + private static readonly HashSet _copyAsIsTemplateExtensions = new(StringComparer.OrdinalIgnoreCase) + { + ".png", + }; + + private readonly string _templateZipPath; + + public ExtensionTemplateService() + : this(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, TemplateArchiveRelativePath)) + { + } + + internal ExtensionTemplateService(string templateZipPath) + { + ArgumentException.ThrowIfNullOrEmpty(templateZipPath); + _templateZipPath = templateZipPath; + } + + internal static IReadOnlyCollection ReplaceTokensTemplateExtensions => _replaceTokensTemplateExtensions; + + internal static IReadOnlyCollection CopyAsIsTemplateExtensions => _copyAsIsTemplateExtensions; + + public void CreateExtension(string extensionName, string displayName, string outputPath) + { + var newGuid = Guid.NewGuid().ToString(); + + // Unzip `template.zip` to a temp dir: + var tempDir = Directory.CreateTempSubdirectory("CmdPal_ExtTemplate"); + + if (!Directory.Exists(outputPath)) + { + Directory.CreateDirectory(outputPath); + } + + ZipFile.ExtractToDirectory(_templateZipPath, tempDir.FullName); + + try + { + foreach (var file in Directory.GetFiles(tempDir.FullName, "*", SearchOption.AllDirectories)) + { + CopyTemplateFile(tempDir.FullName, file, outputPath, extensionName, displayName, newGuid); + } + } + finally + { + tempDir.Delete(recursive: true); + } + } + + internal static void CopyTemplateFile(string templateRoot, string sourceFile, string outputPath, string extensionName, string displayName, string newGuid) + { + var relativePath = Path.GetRelativePath(templateRoot, sourceFile); + var newFileName = Path.Combine(outputPath, relativePath).Replace("TemplateCmdPalExtension", extensionName, StringComparison.Ordinal); + + Directory.CreateDirectory(Path.GetDirectoryName(newFileName)!); + + switch (GetTemplateFileHandling(sourceFile)) + { + case TemplateFileHandling.ReplaceTokens: + var sourceText = File.ReadAllText(sourceFile); + var updatedText = ReplaceTemplateTokens(sourceText, extensionName, displayName, newGuid); + if (string.Equals(sourceText, updatedText, StringComparison.Ordinal)) + { + File.Copy(sourceFile, newFileName, overwrite: true); + break; + } + + File.WriteAllText(newFileName, updatedText); + break; + case TemplateFileHandling.CopyAsIs: + default: + File.Copy(sourceFile, newFileName, overwrite: true); + break; + } + } + + internal static TemplateFileHandling GetTemplateFileHandling(string filePath) + { + var extension = Path.GetExtension(filePath); + if (_replaceTokensTemplateExtensions.Contains(extension)) + { + return TemplateFileHandling.ReplaceTokens; + } + + if (_copyAsIsTemplateExtensions.Contains(extension)) + { + return TemplateFileHandling.CopyAsIs; + } + + throw new InvalidOperationException($"Template file '{filePath}' has unsupported extension '{extension}'. Update the template file handling lists in {nameof(ExtensionTemplateService)}."); + } + + private static string ReplaceTemplateTokens(string text, string extensionName, string displayName, string newGuid) => + text + .Replace("FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF", newGuid, StringComparison.Ordinal) + .Replace("TemplateCmdPalExtension", extensionName, StringComparison.Ordinal) + .Replace("TemplateDisplayName", displayName, StringComparison.Ordinal); +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/IExtensionTemplateService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/IExtensionTemplateService.cs new file mode 100644 index 0000000000..663f2c90cc --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/IExtensionTemplateService.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +/// +/// Creates new Command Palette extensions from the built-in project template. +/// +internal interface IExtensionTemplateService +{ + /// + /// Scaffolds a new extension project by extracting the template archive, + /// replacing placeholder tokens, and writing the result to . + /// + /// The code-safe name used for the project and namespaces. + /// The human-readable name shown in the extension catalog. + /// The directory where the new extension project will be created. + void CreateExtension(string extensionName, string displayName, string outputPath); +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionTemplateServiceTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionTemplateServiceTests.cs new file mode 100644 index 0000000000..e584cb69f0 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ExtensionTemplateServiceTests.cs @@ -0,0 +1,188 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.IO.Compression; +using System.Linq; +using System.Text; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +[TestClass] +public class ExtensionTemplateServiceTests +{ + private string _templateRoot = null!; + private string _outputRoot = null!; + + [TestInitialize] + public void Setup() + { + var tempRoot = Path.Combine(Path.GetTempPath(), $"{nameof(ExtensionTemplateServiceTests)}_{Guid.NewGuid():N}"); + _templateRoot = Path.Combine(tempRoot, "template"); + _outputRoot = Path.Combine(tempRoot, "output"); + + Directory.CreateDirectory(_templateRoot); + Directory.CreateDirectory(_outputRoot); + } + + [TestCleanup] + public void Cleanup() + { + var tempRoot = Directory.GetParent(_templateRoot)?.FullName; + if (!string.IsNullOrEmpty(tempRoot) && Directory.Exists(tempRoot)) + { + Directory.Delete(tempRoot, recursive: true); + } + } + + [TestMethod] + public void CreateExtension_BuildsExtensionFromTemplateArchive() + { + // Arrange + var archiveRoot = Path.Combine(_templateRoot, "archive"); + var templateProjectRoot = Path.Combine(archiveRoot, "TemplateCmdPalExtension", "TemplateCmdPalExtension"); + Directory.CreateDirectory(Path.Combine(templateProjectRoot, "Assets")); + + File.WriteAllText( + Path.Combine(templateProjectRoot, "Program.cs"), + "TemplateCmdPalExtension TemplateDisplayName FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF"); + File.WriteAllBytes(Path.Combine(templateProjectRoot, "Assets", "Logo.png"), [0x89, 0x50, 0x4E, 0x47]); + + var templateZipPath = Path.Combine(_templateRoot, "template.zip"); + ZipFile.CreateFromDirectory(archiveRoot, templateZipPath); + + var service = new ExtensionTemplateService(templateZipPath); + + // Act + service.CreateExtension("MyExtension", "My Display Name", _outputRoot); + + // Assert + var programFile = Path.Combine(_outputRoot, "MyExtension", "MyExtension", "Program.cs"); + var imageFile = Path.Combine(_outputRoot, "MyExtension", "MyExtension", "Assets", "Logo.png"); + + Assert.IsTrue(File.Exists(programFile)); + Assert.IsTrue(File.Exists(imageFile)); + StringAssert.Contains(File.ReadAllText(programFile), "MyExtension"); + StringAssert.Contains(File.ReadAllText(programFile), "My Display Name"); + Assert.IsFalse(File.ReadAllText(programFile).Contains("FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF", StringComparison.Ordinal)); + CollectionAssert.AreEqual(new byte[] { 0x89, 0x50, 0x4E, 0x47 }, File.ReadAllBytes(imageFile)); + } + + [TestMethod] + public void CopyTemplateFile_RewritesTextFiles() + { + // Arrange + var sourceFile = Path.Combine(_templateRoot, "TemplateCmdPalExtension", "Program.cs"); + Directory.CreateDirectory(Path.GetDirectoryName(sourceFile)!); + File.WriteAllText(sourceFile, "TemplateCmdPalExtension TemplateDisplayName FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF"); + + // Act + ExtensionTemplateService.CopyTemplateFile(_templateRoot, sourceFile, _outputRoot, "MyExtension", "My Display Name", "11111111-1111-1111-1111-111111111111"); + + // Assert + var outputFile = Path.Combine(_outputRoot, "MyExtension", "Program.cs"); + Assert.IsTrue(File.Exists(outputFile)); + Assert.AreEqual( + "MyExtension My Display Name 11111111-1111-1111-1111-111111111111", + File.ReadAllText(outputFile)); + } + + [TestMethod] + public void CopyTemplateFile_CopiesUnchangedTextFilesVerbatim() + { + // Arrange + var sourceFile = Path.Combine(_templateRoot, "TemplateCmdPalExtension", "Properties", "launchSettings.json"); + Directory.CreateDirectory(Path.GetDirectoryName(sourceFile)!); + + var sourceBytes = Encoding.UTF8.GetPreamble() + .Concat(Encoding.UTF8.GetBytes("{\"profiles\":{\"CmdPal\":{}}}")) + .ToArray(); + File.WriteAllBytes(sourceFile, sourceBytes); + + // Act + ExtensionTemplateService.CopyTemplateFile(_templateRoot, sourceFile, _outputRoot, "MyExtension", "My Display Name", "11111111-1111-1111-1111-111111111111"); + + // Assert + var outputFile = Path.Combine(_outputRoot, "MyExtension", "Properties", "launchSettings.json"); + Assert.IsTrue(File.Exists(outputFile)); + CollectionAssert.AreEqual(sourceBytes, File.ReadAllBytes(outputFile)); + } + + [TestMethod] + public void CopyTemplateFile_CopiesBinaryFilesWithoutRewritingContents() + { + // Arrange + var sourceFile = Path.Combine(_templateRoot, "TemplateCmdPalExtension", "Assets", "Logo.png"); + Directory.CreateDirectory(Path.GetDirectoryName(sourceFile)!); + + var binaryContent = new byte[] + { + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, + }; + var embeddedText = Encoding.UTF8.GetBytes("TemplateCmdPalExtension TemplateDisplayName"); + File.WriteAllBytes(sourceFile, [.. binaryContent, .. embeddedText]); + + // Act + ExtensionTemplateService.CopyTemplateFile(_templateRoot, sourceFile, _outputRoot, "MyExtension", "My Display Name", "11111111-1111-1111-1111-111111111111"); + + // Assert + var outputFile = Path.Combine(_outputRoot, "MyExtension", "Assets", "Logo.png"); + Assert.IsTrue(File.Exists(outputFile)); + CollectionAssert.AreEqual(File.ReadAllBytes(sourceFile), File.ReadAllBytes(outputFile)); + } + + [TestMethod] + public void TemplateFileHandling_ThrowsForUnknownExtension() + { + var ex = Assert.ThrowsException(() => ExtensionTemplateService.GetTemplateFileHandling("template.svg")); + + StringAssert.Contains(ex.Message, ".svg"); + } + + [TestMethod] + public void TemplateExtensionCategories_AreDisjointAndCoverTemplateZip() + { + var replaceTokens = ExtensionTemplateService.ReplaceTokensTemplateExtensions.ToHashSet(StringComparer.OrdinalIgnoreCase); + var copyAsIs = ExtensionTemplateService.CopyAsIsTemplateExtensions.ToHashSet(StringComparer.OrdinalIgnoreCase); + var allAccountedFor = replaceTokens.Concat(copyAsIs).ToHashSet(StringComparer.OrdinalIgnoreCase); + var templateZipExtensions = GetTemplateZipExtensions(); + + CollectionAssert.AreEqual(Array.Empty(), replaceTokens.Intersect(copyAsIs, StringComparer.OrdinalIgnoreCase).ToArray()); + CollectionAssert.AreEquivalent(templateZipExtensions.OrderBy(x => x).ToArray(), allAccountedFor.OrderBy(x => x).ToArray()); + } + + [TestMethod] + public void TemplateZipFiles_AllUseKnownHandling() + { + using var archive = ZipFile.OpenRead(TemplateZipPath); + var replaceTokens = ExtensionTemplateService.ReplaceTokensTemplateExtensions.ToHashSet(StringComparer.OrdinalIgnoreCase); + var copyAsIs = ExtensionTemplateService.CopyAsIsTemplateExtensions.ToHashSet(StringComparer.OrdinalIgnoreCase); + + foreach (var entry in archive.Entries.Where(entry => !string.IsNullOrEmpty(Path.GetExtension(entry.FullName)))) + { + var extension = Path.GetExtension(entry.FullName); + var expectedHandling = replaceTokens.Contains(extension) + ? ExtensionTemplateService.TemplateFileHandling.ReplaceTokens + : ExtensionTemplateService.TemplateFileHandling.CopyAsIs; + + Assert.AreEqual(expectedHandling, ExtensionTemplateService.GetTemplateFileHandling(entry.FullName), entry.FullName); + Assert.IsTrue(replaceTokens.Contains(extension) || copyAsIs.Contains(extension), entry.FullName); + } + } + + private static string TemplateZipPath => Path.Combine(AppContext.BaseDirectory, "Assets", "template.zip"); + + private static HashSet GetTemplateZipExtensions() + { + using var archive = ZipFile.OpenRead(TemplateZipPath); + return archive.Entries + .Where(entry => !string.IsNullOrEmpty(Path.GetExtension(entry.FullName))) + .Select(entry => Path.GetExtension(entry.FullName)) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/Microsoft.CmdPal.UI.ViewModels.UnitTests.csproj b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/Microsoft.CmdPal.UI.ViewModels.UnitTests.csproj index fa7a95b5fb..bd392e0e04 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/Microsoft.CmdPal.UI.ViewModels.UnitTests.csproj +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/Microsoft.CmdPal.UI.ViewModels.UnitTests.csproj @@ -22,4 +22,10 @@ + + + + PreserveNewest + +