Skip to content

Commit

Permalink
Do not attempt post install ARP correlation if PackageFamilyName is p…
Browse files Browse the repository at this point in the history
…rovided 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.
  • Loading branch information
JohnMcPMS authored and Ryan Fu committed Jul 10, 2023
1 parent fe19c57 commit fdcdb5a
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 4 deletions.
2 changes: 1 addition & 1 deletion azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Execution::Data::Installer>();

if (installer && !installer->PackageFamilyName.empty() && Deployment::IsRegistered(installer->PackageFamilyName))
{
return;
}

const auto& manifest = context.Get<Execution::Data::Manifest>();
auto& arpCorrelationData = context.Get<Execution::Data::ARPCorrelationData>();

Expand Down
46 changes: 46 additions & 0 deletions src/AppInstallerCLIE2ETests/InstallCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -647,5 +647,51 @@ public void InstallWithWindowsFeatureDependency_Force()
Assert.True(installResult.StdOut.Contains("Successfully installed"));
Assert.True(TestCommon.VerifyTestExeInstalledAndCleanup(testDir));
}

/// <summary>
/// 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.
/// </summary>
[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);
}
}
}
71 changes: 69 additions & 2 deletions src/AppInstallerCLIE2ETests/TestCommon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -701,8 +702,7 @@ public static bool VerifyTestMsixUninstalled(bool isProvisioned = false)
/// <param name="value">Value.</param>
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);
Expand Down Expand Up @@ -770,6 +770,73 @@ public static void TearDownTestSource()
RunAICLICommand("source reset", "--force");
}

/// <summary>
/// Ensures that a module is in the desired state.
/// </summary>
/// <param name="moduleName">The module.</param>
/// <param name="present">Whether the module is present or not.</param>
/// <param name="repository">The repository to get the module from if needed.</param>
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\"");
}
}
}

/// <summary>
/// Creates an ARP entry from the given values.
/// </summary>
/// <param name="productCode">Product code of the entry.</param>
/// <param name="properties">The properties to set in the entry.</param>
/// <param name="scope">Scope of the entry.</param>
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));
}
}
}

/// <summary>
/// Removes an ARP entry.
/// </summary>
/// <param name="productCode">Product code of the entry.</param>
/// <param name="scope">Scope of the entry.</param>
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);
}
}

/// <summary>
/// Run command result.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -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: <EXEHASH>
InstallerType: exe
PackageFamilyName: 6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e_8wekyb3d8bbwe
Switches:
Custom: /execustom
SilentWithProgress: /exeswp
Silent: /exesilent
Interactive: /exeinteractive
Language: /exeenus
Log: /LogFile <LOGPATH>
InstallLocation: /InstallDir <INSTALLPATH>
ManifestVersion: 0.1.0
10 changes: 10 additions & 0 deletions src/AppInstallerCommonCore/Deployment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
3 changes: 3 additions & 0 deletions src/AppInstallerCommonCore/Public/AppInstallerDeployment.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netcoreapp3.1</TargetFramework>
<TargetFramework>net6.0</TargetFramework>
</PropertyGroup>

<ItemGroup>
Expand Down

0 comments on commit fdcdb5a

Please sign in to comment.