From fdcdb5a200dccf6d862807571f90b7478b7c7e5f 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 +++ src/AppInstallerCLIE2ETests/InstallCommand.cs | 46 ++++++++++++ src/AppInstallerCLIE2ETests/TestCommon.cs | 71 ++++++++++++++++++- .../TestExeInstaller_InstallMSIX.yaml | 20 ++++++ src/AppInstallerCommonCore/Deployment.cpp | 10 +++ .../Public/AppInstallerDeployment.h | 3 + .../WinGetUtilInterop.UnitTests.csproj | 2 +- 8 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller_InstallMSIX.yaml diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 4cc5b32c88..ed0c3718a8 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -268,7 +268,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 54b67fa6eb..31b6212128 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -672,6 +672,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/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs index a17ff1f2b9..2fa48d4b33 100644 --- a/src/AppInstallerCLIE2ETests/InstallCommand.cs +++ b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -647,5 +647,51 @@ public void InstallWithWindowsFeatureDependency_Force() Assert.True(installResult.StdOut.Contains("Successfully installed")); 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 b430c6173e..e1f660b9f0 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; @@ -701,8 +702,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); @@ -770,6 +770,73 @@ public static void TearDownTestSource() RunAICLICommand("source reset", "--force"); } + /// + /// Ensures that a module is in the desired state. + /// + /// The module. + /// Whether the module is present or not. + /// The repository to get the module from if needed. + public static void EnsureModuleState(string moduleName, bool present, string repository = null) + { + var result = RunCommandWithResult("pwsh", $"-Command \"Get-Module {moduleName} -ListAvailable\""); + bool isPresent = !string.IsNullOrWhiteSpace(result.StdOut); + + if (isPresent && !present) + { + RunCommand("pwsh", $"-Command \"Uninstall-Module {moduleName}\""); + } + else if (!isPresent && present) + { + if (string.IsNullOrEmpty(repository)) + { + RunCommand("pwsh", $"-Command \"Install-Module {moduleName} -Force\""); + } + else + { + RunCommand("pwsh", $"-Command \"Install-Module {moduleName} -Repository {repository} -Force\""); + } + } + } + + /// + /// 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