From aeb5284c1193784e39d1828b5a761847be6ce998 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 6 Jul 2023 10:55:34 -0700 Subject: [PATCH] Do not attempt post install ARP correlation if PackageFamilyName is provided and present for the user (#3391) The simple change is to not attempt the confidence interval-based correlation after installing an installer that has a `PackageFamilyName` value present, when that family name is found to be registered for the user. --- azure-pipelines.yml | 2 +- .../Workflows/InstallFlow.cpp | 9 ++++ .../AppInstallerCLIE2ETests.csproj | 9 ---- src/AppInstallerCLIE2ETests/InstallCommand.cs | 46 +++++++++++++++++++ src/AppInstallerCLIE2ETests/TestCommon.cs | 43 ++++++++++++++++- .../TestExeInstaller_InstallMSIX.yaml | 20 ++++++++ src/AppInstallerCommonCore/Deployment.cpp | 10 ++++ .../Public/AppInstallerDeployment.h | 3 ++ .../WinGetUtilInterop.UnitTests.csproj | 2 +- 9 files changed, 131 insertions(+), 13 deletions(-) create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller_InstallMSIX.yaml diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 90d62965b7..3a2c65ccef 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -275,7 +275,7 @@ jobs: - task: CopyFiles@2 displayName: 'Copy Files: WinGetUtilInterop.UnitTests' inputs: - SourceFolder: '$(Build.SourcesDirectory)\src\WinGetUtilInterop.UnitTests\bin\$(BuildConfiguration)\netcoreapp3.1' + SourceFolder: '$(Build.SourcesDirectory)\src\WinGetUtilInterop.UnitTests\bin\$(BuildConfiguration)\net6.0' TargetFolder: '$(Build.ArtifactStagingDirectory)\WinGetUtilInterop.UnitTests\' CleanTargetFolder: true OverWrite: true diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index c547a2e8ad..6a911995fc 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -691,6 +691,15 @@ namespace AppInstaller::CLI::Workflow return; } + // If the installer claims to have a PackageFamilyName, and that family name is currently registered for the user, + // let that be the correlated item and skip any attempt at further ARP correlation. + const auto& installer = context.Get(); + + if (installer && !installer->PackageFamilyName.empty() && Deployment::IsRegistered(installer->PackageFamilyName)) + { + return; + } + const auto& manifest = context.Get(); auto& arpCorrelationData = context.Get(); diff --git a/src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj b/src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj index 21e1934a61..74ca8a6503 100644 --- a/src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj +++ b/src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj @@ -48,15 +48,6 @@ - - - - - - - - - PreserveNewest diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs index 1cba1373cd..82f0b4eccc 100644 --- a/src/AppInstallerCLIE2ETests/InstallCommand.cs +++ b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -671,5 +671,51 @@ public void InstallWithPackageDependency_RefreshPathVariable() TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true); Assert.True(TestCommon.VerifyTestExeInstalledAndCleanup(testDir)); } + + /// + /// This test flow is intended to test an EXE that actually installs an MSIX internally, and whose name+publisher + /// information resembles an existing installation. Given this, the goal is to get correlation to stick to the + /// MSIX rather than the ARP entry that we would match with in the absence of the package family name being present. + /// + [Test] + public void InstallExeThatInstallsMSIX() + { + string targetPackageIdentifier = "AppInstallerTest.TestExeInstallerInstallsMSIX"; + string fakeProductCode = "e35f5799-cce3-41fd-886c-c36fcb7104fe"; + + // Insert fake ARP entry as if a non-MSIX version of the package is already installed. + // The name here must not match the normalized name of the package, but be close enough to meet + // the confidence requirements for correlation after an install operation (so we drop one word here). + TestCommon.CreateARPEntry(fakeProductCode, new + { + DisplayName = "EXE Installer that Installs MSIX", + Publisher = "AppInstallerTest", + DisplayVersion = "1.0.0", + }); + + // We should not find it before installing because the normalized name doesn't match + var result = TestCommon.RunAICLICommand("list", targetPackageIdentifier); + Assert.AreEqual(Constants.ErrorCode.ERROR_NO_APPLICATIONS_FOUND, result.ExitCode); + + // Add the MSIX to simulate an installer doing it + TestCommon.InstallMsix(TestCommon.MsixInstallerPath); + + // Install our exe that "installs" the MSIX + result = TestCommon.RunAICLICommand("install", $"{targetPackageIdentifier} --force"); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + + // We should find the package now, and it should be correlated to the MSIX (although we don't actually know that from this probe) + result = TestCommon.RunAICLICommand("list", targetPackageIdentifier); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + + // Remove the MSIX outside of winget's knowledge to keep the tracking data. + TestCommon.RemoveMsix(Constants.MsixInstallerName); + + // We should not find the package now that the MSIX is gone, confirming that it was correlated + result = TestCommon.RunAICLICommand("list", targetPackageIdentifier); + Assert.AreEqual(Constants.ErrorCode.ERROR_NO_APPLICATIONS_FOUND, result.ExitCode); + + TestCommon.RemoveARPEntry(fakeProductCode); + } } } \ No newline at end of file diff --git a/src/AppInstallerCLIE2ETests/TestCommon.cs b/src/AppInstallerCLIE2ETests/TestCommon.cs index 8c23dfe292..549b5ebce3 100644 --- a/src/AppInstallerCLIE2ETests/TestCommon.cs +++ b/src/AppInstallerCLIE2ETests/TestCommon.cs @@ -9,6 +9,7 @@ namespace AppInstallerCLIE2ETests using System; using System.Diagnostics; using System.IO; + using System.Reflection; using System.Threading; using Microsoft.Win32; using NUnit.Framework; @@ -695,8 +696,7 @@ public static bool VerifyTestMsixUninstalled(bool isProvisioned = false) /// Value. public static void ModifyPortableARPEntryValue(string productCode, string name, string value) { - const string uninstallSubKey = @"Software\Microsoft\Windows\CurrentVersion\Uninstall"; - using (RegistryKey uninstallRegistryKey = Registry.CurrentUser.OpenSubKey(uninstallSubKey, true)) + using (RegistryKey uninstallRegistryKey = Registry.CurrentUser.OpenSubKey(Constants.UninstallSubKey, true)) { RegistryKey entry = uninstallRegistryKey.OpenSubKey(productCode, true); entry.SetValue(name, value); @@ -792,6 +792,45 @@ public static void EnsureModuleState(string moduleName, bool present, string rep } } + /// + /// Creates an ARP entry from the given values. + /// + /// Product code of the entry. + /// The properties to set in the entry. + /// Scope of the entry. + public static void CreateARPEntry( + string productCode, + object properties, + Scope scope = Scope.User) + { + RegistryKey baseKey = (scope == Scope.User) ? Registry.CurrentUser : Registry.LocalMachine; + using (RegistryKey uninstallRegistryKey = baseKey.OpenSubKey(Constants.UninstallSubKey, true)) + { + RegistryKey entry = uninstallRegistryKey.CreateSubKey(productCode, true); + + foreach (PropertyInfo property in properties.GetType().GetProperties()) + { + entry.SetValue(property.Name, property.GetValue(properties)); + } + } + } + + /// + /// Removes an ARP entry. + /// + /// Product code of the entry. + /// Scope of the entry. + public static void RemoveARPEntry( + string productCode, + Scope scope = Scope.User) + { + RegistryKey baseKey = (scope == Scope.User) ? Registry.CurrentUser : Registry.LocalMachine; + using (RegistryKey uninstallRegistryKey = baseKey.OpenSubKey(Constants.UninstallSubKey, true)) + { + uninstallRegistryKey.DeleteSubKey(productCode); + } + } + /// /// Run command result. /// diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller_InstallMSIX.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller_InstallMSIX.yaml new file mode 100644 index 0000000000..c73d69200e --- /dev/null +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller_InstallMSIX.yaml @@ -0,0 +1,20 @@ +Id: AppInstallerTest.TestExeInstallerInstallsMSIX +Name: EXE Installer that Installs MSIX - extra +Version: 1.0.0.0 +Publisher: AppInstallerTest +License: Test +Installers: + - Arch: x86 + Url: https://localhost:5001/TestKit/AppInstallerTestExeInstaller/AppInstallerTestExeInstaller.exe + Sha256: + InstallerType: exe + PackageFamilyName: 6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e_8wekyb3d8bbwe + Switches: + Custom: /execustom + SilentWithProgress: /exeswp + Silent: /exesilent + Interactive: /exeinteractive + Language: /exeenus + Log: /LogFile + InstallLocation: /InstallDir +ManifestVersion: 0.1.0 diff --git a/src/AppInstallerCommonCore/Deployment.cpp b/src/AppInstallerCommonCore/Deployment.cpp index 71ac9a75cb..10add79822 100644 --- a/src/AppInstallerCommonCore/Deployment.cpp +++ b/src/AppInstallerCommonCore/Deployment.cpp @@ -312,4 +312,14 @@ namespace AppInstaller::Deployment RemovePackage(packageFullName, RemovalOptions::RemoveForAllUsers, progress); } } + + bool IsRegistered(std::string_view packageFamilyName) + { + std::wstring wideFamilyName = Utility::ConvertToUTF16(packageFamilyName); + + PackageManager packageManager; + auto packages = packageManager.FindPackagesForUser({}, wideFamilyName); + + return packages.begin() != packages.end(); + } } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h b/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h index 5e0ab1f862..7aa52ccdf6 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h @@ -44,4 +44,7 @@ namespace AppInstaller::Deployment std::string_view packageFamilyName, std::string_view packageFullName, IProgressCallback& callback); + + // Calls winrt::Windows::Management::Deployment::PackageManager::FindPackagesForUser + bool IsRegistered(std::string_view packageFamilyName); } diff --git a/src/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj b/src/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj index fd93d95f55..553cbbc531 100644 --- a/src/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj +++ b/src/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj @@ -1,7 +1,7 @@  - netcoreapp3.1 + net6.0