From c6eba6d9102fadaceab4f52de4e6d0051e6210eb Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 27 Oct 2026 17:11:42 -0700 Subject: [PATCH 1/6] cp --- .../1.31/user-scope-elevation-guards.md | 9 ++++++++ src/AppInstallerCLICore/Resources.h | 3 +-- .../Workflows/RepairFlow.cpp | 1 + .../ShellExecuteInstallerHandler.cpp | 15 ------------ .../Workflows/ShellExecuteInstallerHandler.h | 5 ++-- .../Workflows/WorkflowBase.cpp | 23 +++++++++++++++++++ .../Workflows/WorkflowBase.h | 15 ++++++++++++ .../Shared/Strings/en-us/winget.resw | 11 ++++----- .../AppInstallerCLITests.vcxproj | 1 + .../AppInstallerCLITests.vcxproj.filters | 3 +++ .../Manifest/ManifestCommon.cpp | 12 ++++++++++ .../Public/winget/ManifestCommon.h | 3 +++ src/AppInstallerSharedLib/Errors.cpp | 2 +- .../Converters.h | 2 +- 14 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 doc/release-notes/1.31/user-scope-elevation-guards.md diff --git a/doc/release-notes/1.31/user-scope-elevation-guards.md b/doc/release-notes/1.31/user-scope-elevation-guards.md new file mode 100644 index 0000000000..928cce9f72 --- /dev/null +++ b/doc/release-notes/1.31/user-scope-elevation-guards.md @@ -0,0 +1,9 @@ +## Bug Fixes + +### Consistent elevation checks for user-scope package operations + +Added elevated-process protections for user-scope MSI uninstall and repair, and portable upgrades. Repair and uninstall-previous operations reject unsupported elevation before preparing the action. These operations report a consistent error directing callers to retry from a non-administrator context. + +Fresh portable installations remain supported from an elevated process and no longer reuse pre-existing installation metadata or cleanup records. + +The checks apply to the elevated half of a split UAC token. Existing behavior with UAC disabled is unchanged. diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 4b574f5155..61c2e578ee 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -23,6 +23,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(AcceptSourceAgreementsArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(AdjoinedNotFlagError); WINGET_DEFINE_RESOURCE_STRINGID(AdjoinedNotFoundError); + WINGET_DEFINE_RESOURCE_STRINGID(AdminContextActionProhibited); WINGET_DEFINE_RESOURCE_STRINGID(AdminSettingDisabled); WINGET_DEFINE_RESOURCE_STRINGID(AdminSettingDisableDescription); WINGET_DEFINE_RESOURCE_STRINGID(AdminSettingEnabled); @@ -499,8 +500,6 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotFound); WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotSpecified); WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotSupported); - WINGET_DEFINE_RESOURCE_STRINGID(NoAdminRepairForUserScopePackage); - WINGET_DEFINE_RESOURCE_STRINGID(NoAdminUninstallForUserScopePackage); WINGET_DEFINE_RESOURCE_STRINGID(NoApplicableInstallers); WINGET_DEFINE_RESOURCE_STRINGID(NoExperimentalFeaturesMessage); WINGET_DEFINE_RESOURCE_STRINGID(NoInstalledFontFound); diff --git a/src/AppInstallerCLICore/Workflows/RepairFlow.cpp b/src/AppInstallerCLICore/Workflows/RepairFlow.cpp index 404c0e3c79..970d05bb3b 100644 --- a/src/AppInstallerCLICore/Workflows/RepairFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/RepairFlow.cpp @@ -455,6 +455,7 @@ namespace AppInstaller::CLI::Workflow void RepairSinglePackage(Execution::Context& context) { context << + EnsureSupportForInstalledPackageOperation << RepairApplicabilityCheck << GetRepairInfo << ReportExecutionStage(ExecutionStage::Execution) << diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index b4674f7f2f..18df7d0624 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -375,21 +375,6 @@ namespace AppInstaller::CLI::Workflow std::wstring commandUtf16 = Utility::ConvertToUTF16(context.Get()); - // When running as admin, block attempt to repair user scope installed package. - // [NOTE:] This check is to address the security concern related to above scenario. - if (Runtime::IsRunningWithNonDefaultFullToken()) - { - auto installedPackageVersion = context.Get(); - const std::string installedScopeString = installedPackageVersion->GetMetadata()[PackageVersionMetadata::InstalledScope]; - auto scopeEnum = ConvertToScopeEnum(installedScopeString); - - if (scopeEnum == ScopeEnum::User) - { - context.Reporter.Error() << Resource::String::NoAdminRepairForUserScopePackage << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); - } - } - // Parse the command string as application and command line for CreateProcess wil::unique_cotaskmem_string app = nullptr; wil::unique_cotaskmem_string args = nullptr; diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.h b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.h index 830698c705..b3bf9819c1 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.h +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.h @@ -35,9 +35,10 @@ namespace AppInstaller::CLI::Workflow // Outputs: InstallerArgs void GetInstallerArgs(Execution::Context& context); - // Repair is done through invoking ShellExecute on downloaded installer. + // Runs the prepared modify/uninstaller repair command. + // Requires the installed-package check performed by RepairSinglePackage. // Required Args: None - // Inputs: Manifest?, InstallerPath, InstallerArgs + // Inputs: RepairString // Outputs: OperationReturnCode void ShellExecuteRepairImpl(Execution::Context& context); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index d4994cf59d..d63e3b1626 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -1728,6 +1728,29 @@ namespace AppInstaller::CLI::Workflow } } + void EnsureNotRunningWithNonDefaultFullToken(Execution::Context& context) + { + if (Runtime::IsRunningWithNonDefaultFullToken()) + { + context.Reporter.Error() << Resource::String::AdminContextActionProhibited << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); + } + } + + void EnsureSupportForInstalledPackageOperation(Execution::Context& context) + { + const auto& installedVersion = context.Get(); + THROW_HR_IF(E_UNEXPECTED, !installedVersion); + auto metadata = installedVersion->GetMetadata(); + const auto scope = Manifest::ConvertToScopeEnum(metadata[PackageVersionMetadata::InstalledScope]); + const auto installedType = Manifest::ConvertToInstallerTypeEnum(metadata[PackageVersionMetadata::InstalledType]); + + if (scope == Manifest::ScopeEnum::User && Manifest::InstallerDataWritableUnelevatedForUserScope(installedType)) + { + context << EnsureNotRunningWithNonDefaultFullToken; + } + } + void EnsureFeatureEnabled::operator()(Execution::Context& context) const { if (!Settings::ExperimentalFeature::IsEnabled(m_feature)) diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h index 3c026e0310..e80e8d67b8 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.h +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -400,6 +400,21 @@ namespace AppInstaller::CLI::Workflow // Outputs: None void EnsureRunningAsAdmin(Execution::Context& context); + // Rejects actions on user-writable data from the full half of a split token. + // Default tokens, including administrators with UAC disabled, are unchanged. + // Required Args: None + // Inputs: None + // Outputs: None + void EnsureNotRunningWithNonDefaultFullToken(Execution::Context& context); + + // Checks installed scope before acting on installed-package data. + // Scope and type must come from the installed source, not caller options. + // Identity-based MSIX and Store operations remain platform-mediated. + // Required Args: None + // Inputs: InstalledPackageVersion + // Outputs: None + void EnsureSupportForInstalledPackageOperation(Execution::Context& context); + // Ensures that the feature is enabled. // Required Args: the desired feature // Inputs: None diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index af04b9f319..e34d6c3b30 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -2915,11 +2915,13 @@ Please specify one of them using the --source option to proceed. The installer technology in use does not support repair. - - The package installed for user scope cannot be repaired when running with administrator privileges. + + This operation is not permitted from an elevated process because it uses data writable without administrator privileges. Try again from a non-administrator context. + An operation was blocked because the process is running with the elevated half of a split UAC token and would act on user-writable data. Ask the caller to retry without elevation; this does not require changing accounts. - The requested operation is not permitted from an administrator context on packages installed within the user scope. + This operation is not permitted from an elevated process because it uses data writable without administrator privileges. Try again from a non-administrator context. + An operation was blocked because the process is running with the elevated half of a split UAC token and would act on user-writable data. Ask the caller to retry without elevation; this does not require changing accounts. Repair failed with exit code: {0} @@ -3681,7 +3683,4 @@ An unlocalized JSON fragment will follow on another line. The DSC processor path no longer refers to the file that was verified. An error message shown when the file that a configuration processor (DSC) executable path points to has been replaced or swapped after it was verified. - - The package installed for user scope cannot be uninstalled when running with administrator privileges. - \ No newline at end of file diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 07f7dca337..b624a03c11 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -340,6 +340,7 @@ + diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 0311271140..df392ed1d0 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -329,6 +329,9 @@ Source Files\CLI + + Source Files\CLI + Source Files\CLI diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index 18fb24e5fa..bd55afa597 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -955,6 +955,18 @@ namespace AppInstaller::Manifest installerType == InstallerTypeEnum::Msix; } + bool InstallerDataWritableUnelevatedForUserScope(InstallerTypeEnum installerType) + { + switch (installerType) + { + case InstallerTypeEnum::Msix: + case InstallerTypeEnum::MSStore: + return false; + default: + return true; + } + } + bool DoesInstallerTypeRequireRepairBehaviorForRepair(InstallerTypeEnum installerType) { return diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index 8831d7cab8..3d9d258b37 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -500,6 +500,9 @@ namespace AppInstaller::Manifest // Gets a value indicating whether the given installer requires admin for install. bool DoesInstallerTypeRequireAdminForMachineScopeInstall(InstallerTypeEnum installerType); + // Whether user-scope installed data must be treated as writable without elevation. + bool InstallerDataWritableUnelevatedForUserScope(InstallerTypeEnum installerType); + // Gets a value indicating whether the given installer requires RepairBehavior for repair. bool DoesInstallerTypeRequireRepairBehaviorForRepair(InstallerTypeEnum installerType); diff --git a/src/AppInstallerSharedLib/Errors.cpp b/src/AppInstallerSharedLib/Errors.cpp index 4c666b2f29..77f6197aaf 100644 --- a/src/AppInstallerSharedLib/Errors.cpp +++ b/src/AppInstallerSharedLib/Errors.cpp @@ -213,7 +213,7 @@ namespace AppInstaller WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_REPAIR_NOT_APPLICABLE, "Repair operation is not applicable."), WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_EXEC_REPAIR_FAILED, "Repair operation failed."), WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_REPAIR_NOT_SUPPORTED, "The installer technology in use doesn't support repair."), - WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED, "The requested operation is not permitted from an administrator context on packages installed within the user scope."), + WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED, "This operation is not permitted from an elevated process because it uses data writable without administrator privileges. Try again from a non-administrator context."), WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_SQLITE_CONNECTION_TERMINATED, "The SQLite connection was terminated to prevent corruption."), WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_DISPLAYCATALOG_API_FAILED, "Failed to get Microsoft Store package catalog."), WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_NO_APPLICABLE_DISPLAYCATALOG_PACKAGE, "No applicable Microsoft Store package found from Microsoft Store package catalog."), diff --git a/src/Microsoft.Management.Deployment/Converters.h b/src/Microsoft.Management.Deployment/Converters.h index b04a52f628..d7ebf9ea17 100644 --- a/src/Microsoft.Management.Deployment/Converters.h +++ b/src/Microsoft.Management.Deployment/Converters.h @@ -98,7 +98,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation WINGET_GET_OPERATION_RESULT_STATUS(InternalError, InternalError, InternalError, RepairError); break; case APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED: - WINGET_GET_OPERATION_RESULT_STATUS(InternalError, UninstallError, InternalError, RepairError); + WINGET_GET_OPERATION_RESULT_STATUS(InstallError, UninstallError, InternalError, RepairError); break; case APPINSTALLER_CLI_ERROR_PACKAGE_AGREEMENTS_NOT_ACCEPTED: WINGET_GET_OPERATION_RESULT_STATUS(PackageAgreementsNotAccepted, InternalError, PackageAgreementsNotAccepted, PackageAgreementsNotAccepted); From 436767eb5b456befd3ea087d648573b3b935abe0 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 28 Oct 2026 17:38:43 -0700 Subject: [PATCH 2/6] Block acting on user scope from elevated --- src/AppInstallerCLICore/PortableInstaller.cpp | 84 ++++++-- src/AppInstallerCLICore/PortableInstaller.h | 19 +- .../Workflows/InstallFlow.cpp | 10 + .../Workflows/PortableFlow.cpp | 79 ++++---- .../Workflows/PortableFlow.h | 8 +- .../Workflows/UninstallFlow.cpp | 29 +-- src/AppInstallerCLITests/InstallFlow.cpp | 133 +++++++++++- .../PortableInstaller.cpp | 191 ++++++++++++++++-- src/AppInstallerCLITests/RepairFlow.cpp | 41 ++-- src/AppInstallerCLITests/SQLiteWrapper.cpp | 47 +++++ src/AppInstallerCLITests/UninstallFlow.cpp | 10 +- src/AppInstallerCLITests/UpdateFlow.cpp | 16 ++ src/AppInstallerCLITests/WorkflowBase.cpp | 86 ++++++++ .../Public/winget/SQLiteStorageBase.h | 4 + .../SQLiteStorageBase.cpp | 18 +- 15 files changed, 635 insertions(+), 140 deletions(-) create mode 100644 src/AppInstallerCLITests/WorkflowBase.cpp diff --git a/src/AppInstallerCLICore/PortableInstaller.cpp b/src/AppInstallerCLICore/PortableInstaller.cpp index 130d672b5b..4152a1f95f 100644 --- a/src/AppInstallerCLICore/PortableInstaller.cpp +++ b/src/AppInstallerCLICore/PortableInstaller.cpp @@ -259,11 +259,10 @@ namespace AppInstaller::CLI::Portable } } - // TODO: Optimize by applying the difference between expected and desired state. - void PortableInstaller::ApplyDesiredState() + void PortableInstaller::RemoveExpectedState() { std::filesystem::path existingIndexPath = InstallLocation / GetPortableIndexFileName(); - if (std::filesystem::exists(existingIndexPath)) + if (UsesExistingInstallData() && !InstallLocation.empty() && std::filesystem::exists(existingIndexPath)) { bool deleteIndex = false; { @@ -297,7 +296,7 @@ namespace AppInstaller::CLI::Portable if (deleteIndex) { - std::filesystem::remove(existingIndexPath); + SQLiteStorageBase::DeleteSQLiteDatabase(existingIndexPath); AICLI_LOG(CLI, Info, << "Portable index deleted: " << existingIndexPath); } } @@ -324,16 +323,34 @@ namespace AppInstaller::CLI::Portable } } - // Check if existing install location differs from the target install location for proper cleanup. - if (!TargetInstallLocation.empty() && TargetInstallLocation != InstallLocation) + if (!UsesExistingInstallData()) { - RemoveInstallDirectory(); + SQLiteStorageBase::DeleteSQLiteDatabase(TargetInstallLocation / GetPortableIndexFileName()); + } + } + + // TODO: Optimize by applying the difference between expected and desired state. + void PortableInstaller::ApplyDesiredState() + { + if (!UsesExistingInstallData()) + { + // Replace the selected index slot without opening any pre-existing database or journal. + SQLiteStorageBase::DeleteSQLiteDatabase(TargetInstallLocation / GetPortableIndexFileName()); + } + else + { + RemoveExpectedState(); + + if (TargetInstallLocation != InstallLocation) + { + RemoveInstallDirectory(); + } } if (RecordToIndex) { std::filesystem::path targetIndexPath = TargetInstallLocation / GetPortableIndexFileName(); - PortableIndex targetIndex = std::filesystem::exists(targetIndexPath) ? + PortableIndex targetIndex = UsesExistingInstallData() && std::filesystem::exists(targetIndexPath) ? PortableIndex::Open(targetIndexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite) : PortableIndex::CreateNew(targetIndexPath.u8string()); @@ -366,19 +383,24 @@ namespace AppInstaller::CLI::Portable return true; } - void PortableInstaller::Install(Workflow::OperationType operation) + void PortableInstaller::Install() { + THROW_HR_IF(E_UNEXPECTED, !SupportsInstall()); + // If the operation is an install, the ARP entry should be created first so that a catastrophic failure // leaves the system in a state where an uninstall may be possible - if (operation == Workflow::OperationType::Install) + if (!UsesExistingInstallData()) { RegisterARPEntry(); } - CreateTargetInstallDirectory(); + const bool targetInstallDirectoryCreated = CreateTargetInstallDirectory(); ApplyDesiredState(); + InstallLocation = TargetInstallLocation; + InstallDirectoryCreated = targetInstallDirectoryCreated; + if (!InstallDirectoryAddedToPath) { AddToPathVariable(GetPortableLinksLocation(GetScope())); @@ -386,7 +408,7 @@ namespace AppInstaller::CLI::Portable // If the operation is an upgrade, the ARP entry should be created last so that a catastrophic failure // leaves the system in a state where an upgrade can be re-attempted - if (operation == Workflow::OperationType::Upgrade) + if (UsesExistingInstallData()) { RegisterARPEntry(); } @@ -394,7 +416,9 @@ namespace AppInstaller::CLI::Portable void PortableInstaller::Uninstall() { - ApplyDesiredState(); + THROW_HR_IF(E_UNEXPECTED, !SupportsUninstall() || (!UsesExistingInstallData() && TargetInstallLocation.empty())); + + RemoveExpectedState(); RemoveInstallDirectory(); @@ -412,15 +436,25 @@ namespace AppInstaller::CLI::Portable AICLI_LOG(CLI, Info, << "PortableARPEntry deleted."); } - void PortableInstaller::CreateTargetInstallDirectory() + bool PortableInstaller::CreateTargetInstallDirectory() { - if (std::filesystem::create_directories(TargetInstallLocation)) + const bool created = std::filesystem::create_directories(TargetInstallLocation); + if (created) { AICLI_LOG(Core, Info, << "Created target install directory: " << TargetInstallLocation); - CommitToARPEntry(PortableValueName::InstallDirectoryCreated, true); } + const bool targetInstallDirectoryCreated = created || (TargetInstallLocation == InstallLocation && InstallDirectoryCreated); + if (!UsesExistingInstallData()) + { + InstallLocation = TargetInstallLocation; + InstallDirectoryCreated = targetInstallDirectoryCreated; + } + + CommitToARPEntry(PortableValueName::InstallDirectoryCreated, targetInstallDirectoryCreated); CommitToARPEntry(PortableValueName::InstallLocation, TargetInstallLocation); + + return targetInstallDirectoryCreated; } void PortableInstaller::RemoveInstallDirectory() @@ -533,7 +567,7 @@ namespace AppInstaller::CLI::Portable { const auto& indexPath = InstallLocation / GetPortableIndexFileName(); - if (std::filesystem::exists(indexPath)) + if (!InstallLocation.empty() && std::filesystem::exists(indexPath)) { PortableIndex portableIndex = PortableIndex::Open(indexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite); m_expectedEntries = portableIndex.GetAllPortableFiles(); @@ -576,9 +610,17 @@ namespace AppInstaller::CLI::Portable } } - PortableInstaller::PortableInstaller(Manifest::ScopeEnum scope, Utility::Architecture arch, const std::string& productCode) : + PortableInstaller::PortableInstaller(Manifest::ScopeEnum scope, Utility::Architecture arch, const std::string& productCode, Workflow::OperationType operation) : + m_operation(operation), m_portableARPEntry(PortableARPEntry(scope, arch, productCode)) { + THROW_HR_IF(E_INVALIDARG, !SupportsInstall() && !SupportsUninstall()); + + if (!UsesExistingInstallData()) + { + return; + } + if (ARPEntryExists()) { DisplayName = GetStringValue(PortableValueName::DisplayName); @@ -604,6 +646,12 @@ namespace AppInstaller::CLI::Portable void PortableInstaller::RegisterARPEntry() { + if (!UsesExistingInstallData()) + { + // Stale registration values must not become installed state for a later operation. + Registry::Key::DeleteTree(m_portableARPEntry.GetKey(), {}); + } + CommitToARPEntry(PortableValueName::WinGetPackageIdentifier, WinGetPackageIdentifier); CommitToARPEntry(PortableValueName::WinGetSourceIdentifier, WinGetSourceIdentifier); CommitToARPEntry(PortableValueName::UninstallString, "winget uninstall --product-code " + GetProductCode()); diff --git a/src/AppInstallerCLICore/PortableInstaller.h b/src/AppInstallerCLICore/PortableInstaller.h index 812a1724ed..56b727536e 100644 --- a/src/AppInstallerCLICore/PortableInstaller.h +++ b/src/AppInstallerCLICore/PortableInstaller.h @@ -17,7 +17,7 @@ namespace AppInstaller::CLI::Portable // Object representation of the metadata and functionality required for installing a Portable package. struct PortableInstaller { - // These values are initialized based on the values from the entry in ARP + // Existing ARP values are loaded only for upgrade and uninstall operations. std::string DisplayName; std::string DisplayVersion; std::string HelpLink; @@ -37,14 +37,17 @@ namespace AppInstaller::CLI::Portable // If we fail to create a symlink, add install directory to PATH variable bool InstallDirectoryAddedToPath = false; - bool IsUpdate = false; bool Purge = false; bool RecordToIndex = false; // This is the incoming target install location determined from the context args. std::filesystem::path TargetInstallLocation; - PortableInstaller(Manifest::ScopeEnum scope, Utility::Architecture arch, const std::string& productCode); + PortableInstaller(Manifest::ScopeEnum scope, Utility::Architecture arch, const std::string& productCode, Workflow::OperationType operation); + + bool IsUpdate() const { return m_operation == Workflow::OperationType::Upgrade; } + + bool UsesExistingInstallData() const { return m_operation != Workflow::OperationType::Install; } bool VerifyExpectedState(); @@ -57,9 +60,10 @@ namespace AppInstaller::CLI::Portable { m_expectedEntries = m_desiredEntries; m_desiredEntries = {}; + InstallLocation = TargetInstallLocation; } - void Install(AppInstaller::CLI::Workflow::OperationType operation = Workflow::OperationType::Install); + void Install(); void Uninstall(); @@ -94,11 +98,15 @@ namespace AppInstaller::CLI::Portable AppInstaller::Manifest::AppsAndFeaturesEntry GetAppsAndFeaturesEntry(); private: + Workflow::OperationType m_operation; PortableARPEntry m_portableARPEntry; std::vector m_desiredEntries; std::vector m_expectedEntries; std::stringstream m_stream; + bool SupportsInstall() const { return !UsesExistingInstallData() || IsUpdate(); } + bool SupportsUninstall() const { return !UsesExistingInstallData() || m_operation == Workflow::OperationType::Uninstall; } + std::string GetStringValue(PortableValueName valueName); std::filesystem::path GetPathValue(PortableValueName valueName); bool GetBoolValue(PortableValueName valueName); @@ -107,10 +115,11 @@ namespace AppInstaller::CLI::Portable void RegisterARPEntry(); void ApplyDesiredState(); + void RemoveExpectedState(); void InstallFile(AppInstaller::Portable::PortableFileEntry& desiredState); void RemoveFile(AppInstaller::Portable::PortableFileEntry& desiredState); - void CreateTargetInstallDirectory(); + bool CreateTargetInstallDirectory(); void RemoveInstallDirectory(); void AddToPathVariable(std::filesystem::path value); diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index f0eb5731a1..cb40c0c423 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -392,6 +392,11 @@ namespace AppInstaller::CLI::Workflow UpdateBehaviorEnum updateBehavior = context.Get().value().UpdateBehavior; bool doUninstallPrevious = isUpdate && (updateBehavior == UpdateBehaviorEnum::UninstallPrevious || context.Args.Contains(Execution::Args::Type::UninstallPrevious)); + if (doUninstallPrevious) + { + AICLI_RETURN_IF_TERMINATED(context << EnsureSupportForInstalledPackageOperation); + } + Synchronization::CrossProcessInstallLock lock; if (!ExemptFromSingleInstallLocking(m_installerType)) { @@ -707,6 +712,11 @@ namespace AppInstaller::CLI::Workflow AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALL_UPGRADE_NOT_SUPPORTED); } + if (isUpdate && (updateBehavior == UpdateBehaviorEnum::UninstallPrevious || context.Args.Contains(Execution::Args::Type::UninstallPrevious))) + { + AICLI_RETURN_IF_TERMINATED(context << EnsureSupportForInstalledPackageOperation); + } + context << Workflow::EnsureRunningAsAdminForMachineScopeInstall << Workflow::EnsureSupportForPortableInstall << diff --git a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp index 8d0a0ddfd1..717d4b6504 100644 --- a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp @@ -23,6 +23,30 @@ namespace AppInstaller::CLI::Workflow { constexpr std::string_view s_DefaultSource = "*DefaultSource"sv; + ScopeEnum GetPortableInstallScope(Execution::Context& context) + { + if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate) && + context.Contains(Execution::Data::InstalledPackageVersion)) + { + const auto& installedVersion = context.Get(); + if (installedVersion) + { + auto metadata = installedVersion->GetMetadata(); + auto scope = metadata.find(PackageVersionMetadata::InstalledScope); + return scope != metadata.end() ? ConvertToScopeEnum(scope->second) : ScopeEnum::Unknown; + } + } + + if (context.Args.Contains(Execution::Args::Type::InstallScope)) + { + return ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope)); + } + + const auto requiredScope = Settings::User().Get(); + return requiredScope != ScopeEnum::Unknown ? + requiredScope : Settings::User().Get(); + } + std::string GetPortableProductCode(Execution::Context& context) { const std::string& packageId = context.Get().Id; @@ -61,7 +85,7 @@ namespace AppInstaller::CLI::Workflow void EnsureVolumeSupportsReparsePoints(Execution::Context& context) { - Manifest::ScopeEnum scope = ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope)); + Manifest::ScopeEnum scope = GetPortableInstallScope(context); const std::filesystem::path& symlinkDirectory = GetPortableLinksLocation(scope); if (!AppInstaller::Filesystem::SupportsReparsePoints(symlinkDirectory)) @@ -97,7 +121,7 @@ namespace AppInstaller::CLI::Workflow } PortableInstaller& portableInstaller = context.Get(); - if (portableInstaller.ARPEntryExists()) + if (portableInstaller.UsesExistingInstallData() && portableInstaller.ARPEntryExists()) { if (packageIdentifier != portableInstaller.WinGetPackageIdentifier || sourceIdentifier != portableInstaller.WinGetSourceIdentifier) { @@ -121,43 +145,18 @@ namespace AppInstaller::CLI::Workflow void InitializePortableInstaller(Execution::Context& context) { - Manifest::ScopeEnum scope = Manifest::ScopeEnum::Unknown; - bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); - std::shared_ptr installedVersion; - if (context.Contains(Execution::Data::InstalledPackageVersion)) + const bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); + const auto scope = GetPortableInstallScope(context); + if (isUpdate && scope != ScopeEnum::Machine) { - installedVersion = context.Get(); - } - if (isUpdate && installedVersion) - { - IPackageVersion::Metadata installationMetadata = installedVersion->GetMetadata(); - auto installerScopeItr = installationMetadata.find(Repository::PackageVersionMetadata::InstalledScope); - if (installerScopeItr != installationMetadata.end()) - { - scope = Manifest::ConvertToScopeEnum(installerScopeItr->second); - } - } - else - { - if (context.Args.Contains(Execution::Args::Type::InstallScope)) - { - scope = Manifest::ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope)); - } - else - { - Manifest::ScopeEnum requiredScope = Settings::User().Get(); - Manifest::ScopeEnum preferredScope = Settings::User().Get(); - - scope = requiredScope != Manifest::ScopeEnum::Unknown ? requiredScope : preferredScope; - } + AICLI_RETURN_IF_TERMINATED(context << EnsureNotRunningWithNonDefaultFullToken); } const auto& installer = context.Get().value(); Utility::Architecture arch = installer.Arch; const std::string& productCode = GetPortableProductCode(context); - PortableInstaller portableInstaller = PortableInstaller(scope, arch, productCode); - portableInstaller.IsUpdate = isUpdate; + PortableInstaller portableInstaller = PortableInstaller(scope, arch, productCode, isUpdate ? OperationType::Upgrade : OperationType::Install); if (IsArchiveType(installer.BaseInstallerType) && installer.ArchiveBinariesDependOnPath) { @@ -302,8 +301,6 @@ namespace AppInstaller::CLI::Workflow void PortableInstallImpl(Execution::Context& context) { - OperationType installType = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate) ? OperationType::Upgrade : OperationType::Install; - PortableInstaller& portableInstaller = context.Get(); try { @@ -327,7 +324,7 @@ namespace AppInstaller::CLI::Workflow } } - portableInstaller.Install(installType); + portableInstaller.Install(); context.Add({ portableInstaller.GetAppsAndFeaturesEntry() }); context.Add(ERROR_SUCCESS); context.Reporter.Warn() << portableInstaller.GetOutputMessage(); @@ -336,11 +333,11 @@ namespace AppInstaller::CLI::Workflow { context.Add(Workflow::HandleException(context, std::current_exception())); - if (!portableInstaller.IsUpdate) + if (!portableInstaller.UsesExistingInstallData()) { context.Reporter.Warn() << Resource::String::PortableInstallFailed << std::endl; portableInstaller.PrepareForCleanUp(); -; portableInstaller.Uninstall(); + portableInstaller.Uninstall(); AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_PORTABLE_UNINSTALL_FAILED); } } @@ -368,7 +365,7 @@ namespace AppInstaller::CLI::Workflow } portableInstaller.Purge = context.Args.Contains(Execution::Args::Type::Purge) || - (!portableInstaller.IsUpdate && Settings::User().Get() && !context.Args.Contains(Execution::Args::Type::Preserve)); + (!portableInstaller.IsUpdate() && Settings::User().Get() && !context.Args.Contains(Execution::Args::Type::Preserve)); portableInstaller.Uninstall(); context.Add(ERROR_SUCCESS); @@ -386,6 +383,12 @@ namespace AppInstaller::CLI::Workflow if (installerType == InstallerTypeEnum::Portable) { + if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate) && + GetPortableInstallScope(context) != ScopeEnum::Machine) + { + AICLI_RETURN_IF_TERMINATED(context << EnsureNotRunningWithNonDefaultFullToken); + } + context << EnsureValidArgsForPortableInstall << EnsureVolumeSupportsReparsePoints; diff --git a/src/AppInstallerCLICore/Workflows/PortableFlow.h b/src/AppInstallerCLICore/Workflows/PortableFlow.h index cb7ae19d06..c5d1104dcb 100644 --- a/src/AppInstallerCLICore/Workflows/PortableFlow.h +++ b/src/AppInstallerCLICore/Workflows/PortableFlow.h @@ -19,17 +19,17 @@ namespace AppInstaller::CLI::Workflow // Verifies that the portable install operation is supported. // Required Args: None - // Inputs: Scope, Rename + // Inputs: Installer, InstalledPackageVersion? // Outputs: None void EnsureSupportForPortableInstall(Execution::Context& context); - // Initializes the portable installer. + // Initializes the portable installer, loading installed state only for an update. // Required Args: None - // Inputs: Scope, Architecture, Manifest, Installer + // Inputs: Architecture, Manifest, Installer, InstalledPackageVersion? // Outputs: None void InitializePortableInstaller(Execution::Context& context); - // Verifies that the package identifier and the source identifier match the ARP entry. + // For updates, verifies that the package and source identifiers match the loaded ARP entry. // Required Args: None // Inputs: Manifest, PackageVersion, PortableInstaller // Outputs: None diff --git a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp index 5c24bfc3d6..79f5ab2099 100644 --- a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp @@ -67,21 +67,6 @@ namespace AppInstaller::CLI::Workflow std::vector Items; }; - - bool AdminExecutionShouldBlockUserScopePackages(InstallerTypeEnum installerType) - { - switch (installerType) - { - case InstallerTypeEnum::Exe: - case InstallerTypeEnum::Burn: - case InstallerTypeEnum::Inno: - case InstallerTypeEnum::Nullsoft: - case InstallerTypeEnum::Portable: - return true; - default: - return false; - } - } } void UninstallSinglePackage(Execution::Context& context) @@ -232,16 +217,7 @@ namespace AppInstaller::CLI::Workflow auto packageMetadata = installedPackageVersion->GetMetadata(); auto installedType = ConvertToInstallerTypeEnum(packageMetadata[PackageVersionMetadata::InstalledType]); - // When running as admin, block attempt to uninstall user scope package to prevent elevation of privilege paths. - if (AdminExecutionShouldBlockUserScopePackages(installedType) && Runtime::IsRunningWithNonDefaultFullToken()) - { - auto scopeEnum = ConvertToScopeEnum(packageMetadata[PackageVersionMetadata::InstalledScope]); - if (scopeEnum == ScopeEnum::User) - { - context.Reporter.Error() << Resource::String::NoAdminUninstallForUserScopePackage << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); - } - } + AICLI_RETURN_IF_TERMINATED(context << EnsureSupportForInstalledPackageOperation); switch (installedType) { @@ -313,7 +289,8 @@ namespace AppInstaller::CLI::Workflow PortableInstaller portableInstaller = PortableInstaller( Manifest::ConvertToScopeEnum(installedScope), Utility::ConvertToArchitectureEnum(installedArch), - productCodes[0]); + productCodes[0], + OperationType::Uninstall); context.Add(std::move(portableInstaller)); break; } diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 30bf3da050..8317adf84d 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -712,6 +713,8 @@ TEST_CASE("MsiInstallFlow_DirectMsi", "[InstallFlow][workflow]") TEST_CASE("InstallFlow_Portable", "[InstallFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false); TestCommon::TempFile portableInstallResultPath("TestPortableInstalled.txt"); @@ -763,7 +766,7 @@ TEST_CASE("PortableInstallFlow_RejectsEscapingPathsAtPointOfUse", "[InstallFlow] } AppInstaller::CLI::Portable::PortableInstaller portableInstaller{ - ScopeEnum::User, Architecture::X64, "TestProductCode" }; + ScopeEnum::User, Architecture::X64, "TestProductCode", OperationType::Install }; portableInstaller.TargetInstallLocation = targetDirectory.GetPath(); context.Add(installer); @@ -777,6 +780,8 @@ TEST_CASE("PortableInstallFlow_RejectsEscapingPathsAtPointOfUse", "[InstallFlow] TEST_CASE("InstallFlow_Portable_SymlinkCreationFail", "[InstallFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false); std::ostringstream installOutput; TestContext installContext{ installOutput, std::cin }; @@ -813,8 +818,134 @@ TEST_CASE("InstallFlow_Portable_SymlinkCreationFail", "[InstallFlow][workflow]") REQUIRE_FALSE(std::filesystem::exists(portableTargetPath)); } +TEST_CASE("InstallFlow_AdminContextWithUserScopeUninstallPrevious", "[InstallFlow][workflow]") +{ + using namespace AppInstaller::Manifest; + using namespace AppInstaller::Repository; + + const bool explicitUninstallPrevious = GENERATE(false, true); + CAPTURE(explicitUninstallPrevious); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(true); + + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); + auto installer = manifest.Installers.at(0); + installer.UpdateBehavior = explicitUninstallPrevious ? UpdateBehaviorEnum::Install : UpdateBehaviorEnum::UninstallPrevious; + + std::ostringstream output; + TestContext context{ output, std::cin }; + context.SetFlags(Execution::ContextFlag::InstallerExecutionUseUpdate); + context.Add(installer); + context.Add(TestPackageVersion::Make( + manifest, + TestPackageVersion::MetadataMap{ + { PackageVersionMetadata::InstalledType, "Msi" }, + { PackageVersionMetadata::InstalledScope, "User" }, + })); + if (explicitUninstallPrevious) + { + context.Args.AddArg(Execution::Args::Type::UninstallPrevious); + } + + context << Workflow::EnsureSupportForInstall; + + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); +} + +TEST_CASE("PortableInstallFlow_AdminContextRejectsUserScopeUpdateBeforePreparation", "[InstallFlow][workflow]") +{ + using namespace AppInstaller::Manifest; + using namespace AppInstaller::Repository; + using namespace AppInstaller::Settings; + + const bool initialize = GENERATE(false, true); + const auto installedScope = GENERATE("User", "Unknown"); + CAPTURE(initialize, installedScope); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(true); + TestUserSettings settings; + settings.Set(ScopeEnum::Unknown); + settings.Set(ScopeEnum::Machine); + + std::ostringstream output; + TestContext context{ output, std::cin }; + context.SetFlags(Execution::ContextFlag::InstallerExecutionUseUpdate); + context.Args.AddArg(Execution::Args::Type::InstallScope, "machine"sv); + context.Add(TestPackageVersion::Make( + AppInstaller::Manifest::Manifest{}, + TestPackageVersion::MetadataMap{ + { PackageVersionMetadata::InstalledType, "Portable" }, + { PackageVersionMetadata::InstalledScope, installedScope }, + })); + + if (initialize) + { + context << Workflow::InitializePortableInstaller; + } + else + { + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Portable.yaml")); + context.Add(manifest.Installers.at(0)); + context << Workflow::EnsureSupportForPortableInstall; + } + + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); + REQUIRE_FALSE(context.Contains(Execution::Data::PortableInstaller)); +} + +TEST_CASE("PortableInstallFlow_AdminContextAllowsFreshUserScope", "[InstallFlow][workflow]") +{ + const auto scopeSource = GENERATE("argument", "requirement", "preference"); + CAPTURE(scopeSource); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(true); + TestUserSettings settings; + settings.Set(ScopeEnum::Unknown); + settings.Set(ScopeEnum::Machine); + TempDirectory targetDirectory("PortableFreshElevation", false); + + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Portable.yaml")); + manifest.Id = "Portable.FreshElevation"; + const auto productCode = MakeSuitablePathPart(manifest.Id + "_*DefaultSource"); + AppInstaller::Registry::Portable::PortableARPEntry arpEntry(ScopeEnum::User, Architecture::X64, productCode); + auto cleanup = wil::scope_exit([&]() { arpEntry.Delete(); }); + arpEntry.SetValue(AppInstaller::Registry::Portable::PortableValueName::WinGetPackageIdentifier, "old-package"sv); + arpEntry.SetValue(AppInstaller::Registry::Portable::PortableValueName::WinGetSourceIdentifier, "old-source"sv); + + std::ostringstream output; + TestContext context{ output, std::cin }; + context.Args.AddArg(Execution::Args::Type::InstallLocation, targetDirectory); + if (std::string_view{ scopeSource } == "argument") + { + context.Args.AddArg(Execution::Args::Type::InstallScope, "user"sv); + } + else if (std::string_view{ scopeSource } == "requirement") + { + settings.Set(ScopeEnum::User); + } + else + { + settings.Set(ScopeEnum::User); + } + context.Add(manifest.Installers.at(0)); + context.Add(std::move(manifest)); + + context << + Workflow::EnsureSupportForPortableInstall << + Workflow::InitializePortableInstaller << + Workflow::VerifyPackageAndSourceMatch; + + INFO(output.str()); + REQUIRE_FALSE(context.IsTerminated()); + auto& portableInstaller = context.Get(); + REQUIRE_FALSE(portableInstaller.IsUpdate()); + REQUIRE(portableInstaller.GetScope() == ScopeEnum::User); + REQUIRE(portableInstaller.TargetInstallLocation == targetDirectory.GetPath()); + REQUIRE(portableInstaller.WinGetPackageIdentifier == "Portable.FreshElevation"); + REQUIRE(portableInstaller.WinGetSourceIdentifier == "*DefaultSource"); +} + TEST_CASE("PortableInstallFlow_UserScope", "[InstallFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false); TestCommon::TempFile portableInstallResultPath("TestPortableInstalled.txt"); diff --git a/src/AppInstallerCLITests/PortableInstaller.cpp b/src/AppInstallerCLITests/PortableInstaller.cpp index e5259e7651..9c098b10e8 100644 --- a/src/AppInstallerCLITests/PortableInstaller.cpp +++ b/src/AppInstallerCLITests/PortableInstaller.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "TestCommon.h" +#include #include #include #include @@ -24,6 +25,172 @@ using namespace AppInstaller::Utility; using namespace AppInstaller::CLI::Workflow; using namespace TestCommon; +TEST_CASE("PortableInstaller_FreshInstallIgnoresExistingState", "[PortableInstaller]") +{ + const bool recordToIndex = GENERATE(false, true); + CAPTURE(recordToIndex); + const std::string productCode = "PortableFreshStateTest"; + TempDirectory oldDirectory("PortableOldState", true); + TempDirectory targetDirectory("PortableFreshState", true); + TempFile source("PortableFreshSource.exe"); + std::ofstream(source.GetPath()) << "fresh"; + const auto oldFile = oldDirectory.GetPath() / "untouched.exe"; + std::ofstream(oldFile) << "old"; + const auto oldIndexPath = oldDirectory.GetPath() / (productCode + ".db"); + std::ofstream(oldIndexPath) << "not a database"; + const auto targetPath = targetDirectory.GetPath() / "fresh.exe"; + std::ofstream(targetPath) << "replace me"; + const auto targetIndexPath = targetDirectory.GetPath() / (productCode + ".db"); + { + auto index = PortableIndex::CreateNew(targetIndexPath.u8string()); + index.AddPortableFile(PortableFileEntry::CreateFileEntry(oldFile, oldFile, {})); + } + for (std::wstring_view suffix : { L"-journal", L"-wal", L"-shm" }) + { + auto sidecarPath = targetIndexPath; + sidecarPath += suffix; + std::ofstream(sidecarPath) << "old journal"; + } + + PortableARPEntry arpEntry(ScopeEnum::User, Architecture::X64, productCode); + auto cleanup = wil::scope_exit([&]() { arpEntry.Delete(); }); + arpEntry.SetValue(PortableValueName::InstallLocation, oldDirectory.GetPath()); + arpEntry.SetValue(PortableValueName::PortableTargetFullPath, oldFile); + arpEntry.SetValue(PortableValueName::PortableSymlinkFullPath, oldDirectory.GetPath() / "old-link.exe"); + arpEntry.SetValue(PortableValueName::WinGetPackageIdentifier, "old-package"s); + arpEntry.SetValue(PortableValueName::WinGetSourceIdentifier, "old-source"s); + bool oldFlag = true; + arpEntry.SetValue(PortableValueName::InstallDirectoryCreated, oldFlag); + arpEntry.SetValue(PortableValueName::InstallDirectoryAddedToPath, oldFlag); + + PortableInstaller installer(ScopeEnum::User, Architecture::X64, productCode, OperationType::Install); + REQUIRE(installer.InstallLocation.empty()); + REQUIRE(installer.WinGetPackageIdentifier.empty()); + REQUIRE(installer.VerifyExpectedState()); + installer.TargetInstallLocation = targetDirectory.GetPath(); + installer.WinGetPackageIdentifier = "new-package"; + installer.WinGetSourceIdentifier = "new-source"; + installer.RecordToIndex = recordToIndex; + installer.SetDesiredState({ PortableFileEntry::CreateFileEntry(source.GetPath(), targetPath, {}) }); + installer.Install(); + + REQUIRE(std::filesystem::exists(oldFile)); + REQUIRE(std::filesystem::exists(oldIndexPath)); + REQUIRE(std::filesystem::exists(targetPath)); + { + std::ifstream installedFile(targetPath); + std::string contents; + std::getline(installedFile, contents); + REQUIRE(contents == "fresh"); + } + for (std::wstring_view suffix : { L"-journal", L"-wal", L"-shm" }) + { + auto sidecarPath = targetIndexPath; + sidecarPath += suffix; + REQUIRE_FALSE(std::filesystem::exists(sidecarPath)); + } + if (recordToIndex) + { + auto index = PortableIndex::Open(targetIndexPath.u8string(), SQLiteStorageBase::OpenDisposition::Read); + auto files = index.GetAllPortableFiles(); + REQUIRE(files.size() == 1); + REQUIRE(files.at(0).GetFilePath() == targetPath); + } + else + { + REQUIRE_FALSE(std::filesystem::exists(targetIndexPath)); + } + + PortableInstaller uninstaller(ScopeEnum::User, Architecture::X64, productCode, OperationType::Uninstall); + REQUIRE(uninstaller.InstallLocation == targetDirectory.GetPath()); + REQUIRE_FALSE(uninstaller.InstallDirectoryCreated); + REQUIRE_FALSE(uninstaller.InstallDirectoryAddedToPath); + REQUIRE(uninstaller.WinGetPackageIdentifier == "new-package"); + REQUIRE(uninstaller.WinGetSourceIdentifier == "new-source"); + if (recordToIndex) + { + REQUIRE(uninstaller.PortableTargetFullPath.empty()); + REQUIRE(uninstaller.PortableSymlinkFullPath.empty()); + } + uninstaller.Uninstall(); + REQUIRE_FALSE(std::filesystem::exists(targetPath)); + REQUIRE(std::filesystem::exists(targetDirectory.GetPath())); + REQUIRE(std::filesystem::exists(oldFile)); + REQUIRE(std::filesystem::exists(oldIndexPath)); +} + +TEST_CASE("PortableInstaller_UpgradeLoadsAndReplacesInstalledState", "[PortableInstaller]") +{ + const bool recordToIndex = GENERATE(false, true); + const bool targetAlreadyExists = GENERATE(false, true); + CAPTURE(recordToIndex, targetAlreadyExists); + const std::string productCode = "PortableUpgradeStateTest"; + TempDirectory oldDirectory("PortableBeforeUpgrade", false); + TempDirectory newDirectory("PortableAfterUpgrade", targetAlreadyExists); + TempFile oldSource("PortableBeforeUpgrade.exe"); + TempFile newSource("PortableAfterUpgrade.exe"); + std::ofstream(oldSource.GetPath()) << "before"; + std::ofstream(newSource.GetPath()) << "after"; + const auto oldPath = oldDirectory.GetPath() / "before.exe"; + const auto newPath = newDirectory.GetPath() / "after.exe"; + PortableARPEntry arpEntry(ScopeEnum::User, Architecture::X64, productCode); + auto cleanup = wil::scope_exit([&]() { arpEntry.Delete(); }); + + PortableInstaller installer(ScopeEnum::User, Architecture::X64, productCode, OperationType::Install); + installer.TargetInstallLocation = oldDirectory.GetPath(); + installer.RecordToIndex = recordToIndex; + installer.SetDesiredState({ PortableFileEntry::CreateFileEntry(oldSource.GetPath(), oldPath, std::string(64, '0')) }); + installer.Install(); + + PortableInstaller updater(ScopeEnum::User, Architecture::X64, productCode, OperationType::Upgrade); + REQUIRE_FALSE(updater.VerifyExpectedState()); + updater.TargetInstallLocation = newDirectory.GetPath(); + updater.RecordToIndex = recordToIndex; + updater.SetDesiredState({ PortableFileEntry::CreateFileEntry(newSource.GetPath(), newPath, {}) }); + updater.Install(); + REQUIRE_FALSE(std::filesystem::exists(oldDirectory.GetPath())); + REQUIRE(std::filesystem::exists(newPath)); + + PortableInstaller uninstaller(ScopeEnum::User, Architecture::X64, productCode, OperationType::Uninstall); + REQUIRE(uninstaller.InstallLocation == newDirectory.GetPath()); + uninstaller.Uninstall(); + REQUIRE(std::filesystem::exists(newDirectory.GetPath()) == targetAlreadyExists); +} + +TEST_CASE("PortableInstaller_FreshRollbackUsesInMemoryState", "[PortableInstaller]") +{ + const std::string productCode = "PortableRollbackStateTest"; + TempDirectory targetDirectory("PortableRollbackState", false); + TempDirectory blockedAlias("PortableBlockedAlias", true); + TempFile source("PortableRollbackSource.exe"); + std::ofstream(source.GetPath()) << "fresh"; + const auto targetPath = targetDirectory.GetPath() / "fresh.exe"; + PortableARPEntry arpEntry(ScopeEnum::User, Architecture::X64, productCode); + auto cleanup = wil::scope_exit([&]() { arpEntry.Delete(); }); + + PortableInstaller installer(ScopeEnum::User, Architecture::X64, productCode, OperationType::Install); + installer.TargetInstallLocation = targetDirectory.GetPath(); + installer.RecordToIndex = true; + installer.SetDesiredState({ + PortableFileEntry::CreateFileEntry(source.GetPath(), targetPath, {}), + PortableFileEntry::CreateSymlinkEntry(blockedAlias.GetPath(), targetPath), + }); + REQUIRE_THROWS_HR(installer.Install(), APPINSTALLER_CLI_ERROR_PORTABLE_SYMLINK_PATH_IS_DIRECTORY); + REQUIRE(std::filesystem::exists(targetPath)); + + const auto indexPath = targetDirectory.GetPath() / installer.GetPortableIndexFileName(); + std::filesystem::remove(indexPath); + { + std::ofstream corruptIndex(indexPath); + corruptIndex << "not a database"; + REQUIRE(corruptIndex.good()); + } + installer.PrepareForCleanUp(); + installer.Uninstall(); + REQUIRE_FALSE(std::filesystem::exists(targetDirectory.GetPath())); + REQUIRE(std::filesystem::exists(blockedAlias.GetPath())); +} + TEST_CASE("PortableInstaller_InstallToRegistry", "[PortableInstaller]") { TempDirectory tempDirectory = TestCommon::TempDirectory("TempDirectory", false); @@ -40,17 +207,17 @@ TEST_CASE("PortableInstaller_InstallToRegistry", "[PortableInstaller]") desiredTestState.emplace_back(std::move(PortableFileEntry::CreateFileEntry(testPortable.GetPath(), targetPath, {}))); desiredTestState.emplace_back(std::move(PortableFileEntry::CreateSymlinkEntry(symlinkPath, targetPath))); - PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode"); + PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode", OperationType::Install); portableInstaller.TargetInstallLocation = tempDirectory.GetPath(); portableInstaller.SetDesiredState(desiredTestState); REQUIRE(portableInstaller.VerifyExpectedState()); - portableInstaller.Install(AppInstaller::CLI::Workflow::OperationType::Install); + portableInstaller.Install(); auto entry = portableInstaller.GetAppsAndFeaturesEntry(); REQUIRE(entry.ProductCode == portableInstaller.GetProductCode()); - PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode"); + PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode", OperationType::Uninstall); REQUIRE(portableInstaller2.ARPEntryExists()); REQUIRE(std::filesystem::exists(portableInstaller2.PortableTargetFullPath)); REQUIRE(AppInstaller::Filesystem::SymlinkExists(portableInstaller2.PortableSymlinkFullPath)); @@ -90,13 +257,13 @@ TEST_CASE("PortableInstaller_InstallToIndex_CreateInstallRoot", "[PortableInstal desiredTestState.emplace_back(std::move(PortableFileEntry::CreateSymlinkEntry(symlinkPath2, targetPath2))); desiredTestState.emplace_back(std::move(PortableFileEntry::CreateDirectoryEntry(testDirectoryFolder.GetPath(), directoryPath))); - PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode"); + PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode", OperationType::Install); portableInstaller.TargetInstallLocation = installRootDirectory.GetPath(); portableInstaller.RecordToIndex = true; portableInstaller.SetDesiredState(desiredTestState); REQUIRE(portableInstaller.VerifyExpectedState()); - portableInstaller.Install(AppInstaller::CLI::Workflow::OperationType::Install); + portableInstaller.Install(); REQUIRE(std::filesystem::exists(installRootPath / portableInstaller.GetPortableIndexFileName())); REQUIRE(std::filesystem::exists(targetPath)); @@ -105,7 +272,7 @@ TEST_CASE("PortableInstaller_InstallToIndex_CreateInstallRoot", "[PortableInstal REQUIRE(AppInstaller::Filesystem::SymlinkExists(symlinkPath2)); REQUIRE(std::filesystem::exists(directoryPath)); - PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode"); + PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode", OperationType::Uninstall); REQUIRE(portableInstaller2.ARPEntryExists()); portableInstaller2.Uninstall(); @@ -148,13 +315,13 @@ TEST_CASE("PortableInstaller_InstallToIndex_ExistingInstallRoot", "[PortableInst desiredTestState.emplace_back(std::move(PortableFileEntry::CreateSymlinkEntry(symlinkPath2, targetPath2))); desiredTestState.emplace_back(std::move(PortableFileEntry::CreateDirectoryEntry(testDirectoryFolder.GetPath(), directoryPath))); - PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode"); + PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode", OperationType::Install); portableInstaller.TargetInstallLocation = installRootDirectory.GetPath(); portableInstaller.RecordToIndex = true; portableInstaller.SetDesiredState(desiredTestState); REQUIRE(portableInstaller.VerifyExpectedState()); - portableInstaller.Install(AppInstaller::CLI::Workflow::OperationType::Install); + portableInstaller.Install(); REQUIRE(std::filesystem::exists(installRootPath / portableInstaller.GetPortableIndexFileName())); REQUIRE(std::filesystem::exists(targetPath)); @@ -163,7 +330,7 @@ TEST_CASE("PortableInstaller_InstallToIndex_ExistingInstallRoot", "[PortableInst REQUIRE(AppInstaller::Filesystem::SymlinkExists(symlinkPath2)); REQUIRE(std::filesystem::exists(directoryPath)); - PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode"); + PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode", OperationType::Uninstall); REQUIRE(portableInstaller2.ARPEntryExists()); portableInstaller2.Uninstall(); @@ -196,14 +363,14 @@ TEST_CASE("PortableInstaller_UnicodeSymlinkPath", "[PortableInstaller]") desiredTestState.emplace_back(std::move(PortableFileEntry::CreateFileEntry(testPortable.GetPath(), targetPath, {}))); desiredTestState.emplace_back(std::move(PortableFileEntry::CreateSymlinkEntry(symlinkPath, targetPath))); - PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode"); + PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode", OperationType::Install); portableInstaller.TargetInstallLocation = testInstallLocation; portableInstaller.SetDesiredState(desiredTestState); REQUIRE(portableInstaller.VerifyExpectedState()); - portableInstaller.Install(AppInstaller::CLI::Workflow::OperationType::Install); + portableInstaller.Install(); - PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode"); + PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode", OperationType::Uninstall); REQUIRE(portableInstaller2.ARPEntryExists()); REQUIRE(std::filesystem::exists(portableInstaller2.PortableTargetFullPath)); REQUIRE(AppInstaller::Filesystem::SymlinkExists(portableInstaller2.PortableSymlinkFullPath)); diff --git a/src/AppInstallerCLITests/RepairFlow.cpp b/src/AppInstallerCLITests/RepairFlow.cpp index 40a8c6a73e..4a44f6e861 100644 --- a/src/AppInstallerCLITests/RepairFlow.cpp +++ b/src/AppInstallerCLITests/RepairFlow.cpp @@ -4,44 +4,29 @@ #include "TestHooks.h" #include "TestSource.h" #include "WorkflowCommon.h" -#include #include -#include +#include using namespace TestCommon; using namespace AppInstaller::CLI::Execution; -using namespace AppInstaller::Manifest; using namespace AppInstaller::Repository; -// Tests that when the process is running with a non-default full (elevated UAC) token, -// repairing a package that was installed at user scope is blocked with -// APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED. -// This single test covers both the CLI (RepairCommand) and COM (COMRepairCommand) code paths -// because they both invoke the same ShellExecuteRepairImpl workflow step. -TEST_CASE("RepairFlow_AdminContextWithUserScopeInstall", "[RepairFlow][workflow]") +TEST_CASE("RepairFlow_AdminContextRejectsUserScopeBeforePreparation", "[RepairFlow][workflow]") { - // Simulate running with an elevated (non-default full) token. + const auto installedType = GENERATE("Burn", "Msi"); + CAPTURE(installedType); TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(true); - std::ostringstream repairOutput; - TestContext context{ repairOutput, std::cin }; - auto previousThreadGlobals = context.SetForCurrentThread(); + std::ostringstream output; + TestContext context{ output, std::cin }; + context.Add(TestPackageVersion::Make( + AppInstaller::Manifest::Manifest{}, + TestPackageVersion::MetadataMap{ + { PackageVersionMetadata::InstalledType, installedType }, + { PackageVersionMetadata::InstalledScope, "User" }, + })); - // Build an installed package version with Burn type installed at user scope. - // ShellExecuteRepairImpl blocks Burn (and other Exe-family) packages installed at user scope. - auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); - TestPackageVersion::MetadataMap metadata - { - { PackageVersionMetadata::InstalledType, "Burn" }, - { PackageVersionMetadata::InstalledScope, "User" }, - }; - context.Add(TestPackageVersion::Make(manifest, metadata)); + context << AppInstaller::CLI::Workflow::RepairSinglePackage; - // Provide a dummy repair string; the admin-context check fires before the command is executed. - context.Add(std::string("C:\\repair.exe /silent")); - - context << AppInstaller::CLI::Workflow::ShellExecuteRepairImpl; - - INFO(repairOutput.str()); REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); } diff --git a/src/AppInstallerCLITests/SQLiteWrapper.cpp b/src/AppInstallerCLITests/SQLiteWrapper.cpp index 23ee5601e2..8232d110e3 100644 --- a/src/AppInstallerCLITests/SQLiteWrapper.cpp +++ b/src/AppInstallerCLITests/SQLiteWrapper.cpp @@ -7,11 +7,58 @@ #include #include #include +#include using namespace AppInstaller::SQLite; using namespace std::string_literals; using namespace std::string_view_literals; +TEST_CASE("SQLiteStorageBase_RenameAndDeleteAuxiliaryFiles", "[sqlite]") +{ + TestCommon::TempDirectory directory("SQLiteDatabaseFiles"); + const auto source = directory.GetPath() / "source.db"; + const auto destination = directory.GetPath() / "destination.db"; + for (std::wstring_view suffix : { L"", L"-journal", L"-wal", L"-shm" }) + { + auto sourceFile = source; + sourceFile += suffix; + auto destinationFile = destination; + destinationFile += suffix; + std::ofstream(sourceFile) << "source"; + std::ofstream(destinationFile) << "destination"; + } + + SQLiteStorageBase::RenameSQLiteDatabase(source, destination, true); + for (std::wstring_view suffix : { L"", L"-journal", L"-wal", L"-shm" }) + { + auto sourceFile = source; + sourceFile += suffix; + auto destinationFile = destination; + destinationFile += suffix; + REQUIRE_FALSE(std::filesystem::exists(sourceFile)); + std::ifstream file(destinationFile); + std::string contents; + std::getline(file, contents); + REQUIRE(contents == "source"); + } + + SQLiteStorageBase::DeleteSQLiteDatabase(destination); + for (std::wstring_view suffix : { L"", L"-journal", L"-wal", L"-shm" }) + { + auto file = destination; + file += suffix; + REQUIRE_FALSE(std::filesystem::exists(file)); + } + REQUIRE_NOTHROW(SQLiteStorageBase::DeleteSQLiteDatabase(destination)); + REQUIRE_THROWS_HR(SQLiteStorageBase::DeleteSQLiteDatabase({}), E_INVALIDARG); + + std::filesystem::create_directory(destination); + const auto retainedFile = destination / "retained.txt"; + std::ofstream(retainedFile) << "retain"; + REQUIRE_THROWS_AS(SQLiteStorageBase::DeleteSQLiteDatabase(destination), std::filesystem::filesystem_error); + REQUIRE(std::filesystem::exists(retainedFile)); +} + static const char* s_firstColumn = "first"; static const char* s_secondColumn = "second"; static const char* s_tableName = "simpletest"; diff --git a/src/AppInstallerCLITests/UninstallFlow.cpp b/src/AppInstallerCLITests/UninstallFlow.cpp index 52ce44b812..85ce506c31 100644 --- a/src/AppInstallerCLITests/UninstallFlow.cpp +++ b/src/AppInstallerCLITests/UninstallFlow.cpp @@ -215,6 +215,9 @@ TEST_CASE("UninstallFlow_UninstallMultiple_NotAllInstalled", "[UninstallFlow][wo TEST_CASE("UninstallFlow_AdminContextWithUserScopeInstall", "[UninstallFlow][workflow]") { + const auto installedType = GENERATE("Burn", "Msi", "Wix", "Portable"); + CAPTURE(installedType); + // Simulate running with an elevated (non-default full) token. TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(true); @@ -222,12 +225,11 @@ TEST_CASE("UninstallFlow_AdminContextWithUserScopeInstall", "[UninstallFlow][wor TestContext context{ uninstallOutput, std::cin }; auto previousThreadGlobals = context.SetForCurrentThread(); - // Create a test source with a Burn installer installed at user scope. - // AdminExecutionShouldBlockUserScopePackages returns true for Burn/Exe/Inno/Nullsoft/Portable. + // Create a test source with a package installed at user scope. auto testSource = CreateTestSource({ TestSourceResult( "AppInstallerCliTest.TestBurnInstallerUserScope"sv, - [](std::vector& matches, std::weak_ptr source) { + [installedType](std::vector& matches, std::weak_ptr source) { auto manifest = AppInstaller::Manifest::YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); matches.emplace_back( AppInstaller::Repository::ResultMatch( @@ -235,7 +237,7 @@ TEST_CASE("UninstallFlow_AdminContextWithUserScopeInstall", "[UninstallFlow][wor manifest, TestCompositePackage::MetadataMap { - { AppInstaller::Repository::PackageVersionMetadata::InstalledType, "Burn" }, + { AppInstaller::Repository::PackageVersionMetadata::InstalledType, installedType }, { AppInstaller::Repository::PackageVersionMetadata::InstalledScope, "User" }, }, std::vector{ manifest }, diff --git a/src/AppInstallerCLITests/UpdateFlow.cpp b/src/AppInstallerCLITests/UpdateFlow.cpp index 9d70283c78..aa247639cd 100644 --- a/src/AppInstallerCLITests/UpdateFlow.cpp +++ b/src/AppInstallerCLITests/UpdateFlow.cpp @@ -163,6 +163,8 @@ TEST_CASE("UpdateFlow_UpdateZip_Exe", "[UpdateFlow][workflow]") TEST_CASE("UpdateFlow_UpdatePortable", "[UpdateFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempFile updateResultPath("TestPortableInstalled.txt"); std::ostringstream updateOutput; @@ -180,6 +182,8 @@ TEST_CASE("UpdateFlow_UpdatePortable", "[UpdateFlow][workflow]") TEST_CASE("UpdateFlow_Portable_SymlinkCreationFail", "[UpdateFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + // Update portable with symlink creation failure verify that it succeeds. TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false); std::ostringstream updateOutput; @@ -333,6 +337,8 @@ TEST_CASE("UpdateFlow_IncludeUnknown", "[UpdateFlow][workflow]") TEST_CASE("UpdateFlow_UpdatePortableWithManifest", "[UpdateFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempFile updateResultPath("TestPortableInstalled.txt"); std::ostringstream updateOutput; @@ -519,6 +525,8 @@ TEST_CASE("UpdateFlow_UpdateExeSpecificVersionNotApplicable", "[UpdateFlow][work TEST_CASE("UpdateFlow_UpdateAllApplicable", "[UpdateFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempFile updateExeResultPath("TestExeInstalled.txt"); TestCommon::TempFile updateMsixResultPath("TestMsixInstalled.txt"); TestCommon::TempFile updateMSStoreResultPath("TestMSStoreUpdated.txt"); @@ -558,6 +566,8 @@ TEST_CASE("UpdateFlow_UpdateAllApplicable", "[UpdateFlow][workflow]") TEST_CASE("UpdateFlow_UpdateAll_IncludeUnknown", "[UpdateFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempFile updateExeResultPath("TestExeInstalled.txt"); TestCommon::TempFile updateMsixResultPath("TestMsixInstalled.txt"); TestCommon::TempFile updateMSStoreResultPath("TestMSStoreUpdated.txt"); @@ -693,6 +703,8 @@ TEST_CASE("UpdateFlow_LicenseAgreement_NotAccepted", "[UpdateFlow][workflow]") TEST_CASE("UpdateFlow_All_LicenseAgreement", "[UpdateFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempFile updateExeResultPath("TestExeInstalled.txt"); TestCommon::TempFile updateMsixResultPath("TestMsixInstalled.txt"); TestCommon::TempFile updateMSStoreResultPath("TestMSStoreUpdated.txt"); @@ -774,6 +786,8 @@ TEST_CASE("UpdateFlow_All_LicenseAgreement_NotAccepted", "[UpdateFlow][workflow] TEST_CASE("UpdateFlow_RequireExplicit", "[UpdateFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempFile updateExeResultPath("TestExeInstalled.txt"); TestCommon::TempFile updateMsixResultPath("TestMsixInstalled.txt"); @@ -918,6 +932,8 @@ TEST_CASE("InstallFlow_FoundInstalledAndUpgradeNotAvailable", "[UpdateFlow][work TEST_CASE("UpdateFlow_UpdateAll_ForwardArgs", "[UpdateFlow][workflow]") { + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(false); + TestCommon::TempFile updateExeResultPath("TestExeInstalled.txt"); TestCommon::TempFile updateMsixResultPath("TestMsixInstalled.txt"); TestCommon::TempFile updateMSStoreResultPath("TestMSStoreUpdated.txt"); diff --git a/src/AppInstallerCLITests/WorkflowBase.cpp b/src/AppInstallerCLITests/WorkflowBase.cpp new file mode 100644 index 0000000000..ea0a6c47eb --- /dev/null +++ b/src/AppInstallerCLITests/WorkflowBase.cpp @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "TestHooks.h" +#include "WorkflowCommon.h" +#include + +using namespace AppInstaller::CLI; +using namespace TestCommon; +using namespace std::string_view_literals; + +TEST_CASE("WorkflowBase_EnsureNotRunningWithNonDefaultFullToken", "[WorkflowBase]") +{ + const bool fullToken = GENERATE(false, true); + const bool comContext = GENERATE(false, true); + CAPTURE(fullToken, comContext); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride{ fullToken }; + + std::ostringstream output; + std::unique_ptr context; + if (comContext) + { + context = std::make_unique(); + } + else + { + context = std::make_unique(output, std::cin); + } + + *context << + Workflow::EnsureNotRunningWithNonDefaultFullToken << + [](Execution::Context& nextContext) + { + nextContext.Add(0); + }; + + if (fullToken) + { + REQUIRE_TERMINATED_WITH((*context), APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); + REQUIRE_FALSE(context->Contains(Execution::Data::OperationReturnCode)); + if (!comContext) + { + REQUIRE(output.str().find("non-administrator context") != std::string::npos); + } + } + else + { + REQUIRE_FALSE(context->IsTerminated()); + REQUIRE(context->Contains(Execution::Data::OperationReturnCode)); + REQUIRE(output.str().empty()); + } +} + +TEST_CASE("WorkflowBase_EnsureSupportForInstalledPackageOperation", "[WorkflowBase]") +{ + const bool fullToken = GENERATE(false, true); + const auto installedType = GENERATE("exe", "msi", "wix", "portable", "msix", "msstore", "unknown"); + const auto installedScope = GENERATE("user", "machine"); + CAPTURE(fullToken, installedType, installedScope); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride{ fullToken }; + + std::ostringstream output; + Execution::Context context{ output, std::cin }; + context.Add(std::make_shared( + AppInstaller::Manifest::Manifest{}, + TestPackageVersion::MetadataMap{ + { AppInstaller::Repository::PackageVersionMetadata::InstalledType, installedType }, + { AppInstaller::Repository::PackageVersionMetadata::InstalledScope, installedScope }, + })); + context.Args.AddArg(Execution::Args::Type::InstallScope, + std::string_view{ installedScope } == "user" ? "machine"sv : "user"sv); + + context << Workflow::EnsureSupportForInstalledPackageOperation; + + const bool blocked = fullToken && std::string_view{ installedScope } == "user" && + std::string_view{ installedType } != "msix" && std::string_view{ installedType } != "msstore"; + if (blocked) + { + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); + } + else + { + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(output.str().empty()); + } +} diff --git a/src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h b/src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h index cfc04b5b1e..6a658ab3fa 100644 --- a/src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h +++ b/src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h @@ -34,6 +34,10 @@ namespace AppInstaller::SQLite // If overwrite is given, existing destination files will be removed first. static void RenameSQLiteDatabase(const std::filesystem::path& source, const std::filesystem::path& destination, bool overwrite = false); + // Deletes the database file and any auxiliary files without opening them. + // Should only be used on an inactive database. Missing files are ignored. + static void DeleteSQLiteDatabase(const std::filesystem::path& path); + protected: SQLiteStorageBase(const std::string& target, const Version& version, size_t pageSize = 0); diff --git a/src/AppInstallerSharedLib/SQLiteStorageBase.cpp b/src/AppInstallerSharedLib/SQLiteStorageBase.cpp index 50d5e5e60a..3789fedb45 100644 --- a/src/AppInstallerSharedLib/SQLiteStorageBase.cpp +++ b/src/AppInstallerSharedLib/SQLiteStorageBase.cpp @@ -10,6 +10,8 @@ namespace AppInstaller::SQLite { namespace { + constexpr std::wstring_view s_SQLiteFileSuffixes[] = { L"", L"-journal", L"-wal", L"-shm" }; + static char const* const GetOpenDispositionString(SQLiteStorageBase::OpenDisposition disposition) { switch (disposition) @@ -74,14 +76,12 @@ namespace AppInstaller::SQLite void SQLiteStorageBase::RenameSQLiteDatabase(const std::filesystem::path& source, const std::filesystem::path& destination, bool overwrite) { - auto fileSuffixes = { L"", L"-journal", L"-wal" }; - THROW_WIN32_IF(ERROR_FILE_NOT_FOUND, !std::filesystem::exists(source)); THROW_WIN32_IF(ERROR_DIRECTORY, std::filesystem::is_directory(source)); if (overwrite) { - for (const auto& suffix : fileSuffixes) + for (const auto& suffix : s_SQLiteFileSuffixes) { std::filesystem::path target = AddSuffix(destination, suffix); @@ -92,7 +92,7 @@ namespace AppInstaller::SQLite } } - for (const auto& suffix : fileSuffixes) + for (const auto& suffix : s_SQLiteFileSuffixes) { std::filesystem::path target = AddSuffix(source, suffix); @@ -103,6 +103,16 @@ namespace AppInstaller::SQLite } } + void SQLiteStorageBase::DeleteSQLiteDatabase(const std::filesystem::path& path) + { + THROW_HR_IF(E_INVALIDARG, path.empty()); + + for (const auto& suffix : s_SQLiteFileSuffixes) + { + std::filesystem::remove(AddSuffix(path, suffix)); + } + } + SQLiteStorageBase::SQLiteStorageBase(const std::string& filePath, OpenDisposition disposition, Utility::ManagedFile&& file) : SQLiteStorageBase(DatabaseSpecifier{ filePath, disposition }, std::move(file)) { From 747f0674987b02ad5746be8bde168ba9778ff752 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 29 Oct 2026 09:33:06 -0700 Subject: [PATCH 3/6] font protection --- .../1.31/user-scope-elevation-guards.md | 4 +- .../Workflows/FontFlow.cpp | 40 +++++++--- src/AppInstallerCLICore/Workflows/FontFlow.h | 11 ++- .../Workflows/InstallFlow.cpp | 1 + src/AppInstallerCLITests/InstallFlow.cpp | 74 +++++++++++++++++++ src/AppInstallerCLITests/UninstallFlow.cpp | 38 +++++++++- src/AppInstallerCLITests/WorkflowBase.cpp | 2 +- 7 files changed, 156 insertions(+), 14 deletions(-) diff --git a/doc/release-notes/1.31/user-scope-elevation-guards.md b/doc/release-notes/1.31/user-scope-elevation-guards.md index 928cce9f72..1449e87c63 100644 --- a/doc/release-notes/1.31/user-scope-elevation-guards.md +++ b/doc/release-notes/1.31/user-scope-elevation-guards.md @@ -2,7 +2,9 @@ ### Consistent elevation checks for user-scope package operations -Added elevated-process protections for user-scope MSI uninstall and repair, and portable upgrades. Repair and uninstall-previous operations reject unsupported elevation before preparing the action. These operations report a consistent error directing callers to retry from a non-administrator context. +Added elevated-process protections for user-scope MSI uninstall and repair, portable upgrades, and font installation and removal. Repair and uninstall-previous operations reject unsupported elevation before preparing the action. These operations report a consistent error directing callers to retry from a non-administrator context. + +Font protection also covers initial and forced installations, which can inspect or replace existing font registrations. Fresh portable installations remain supported from an elevated process and no longer reuse pre-existing installation metadata or cleanup records. diff --git a/src/AppInstallerCLICore/Workflows/FontFlow.cpp b/src/AppInstallerCLICore/Workflows/FontFlow.cpp index 87fb26682b..199879164b 100644 --- a/src/AppInstallerCLICore/Workflows/FontFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/FontFlow.cpp @@ -4,6 +4,7 @@ #include "pch.h" #include "FontFlow.h" #include "TableOutput.h" +#include "WorkflowBase.h" #include #include using namespace AppInstaller::Utility; @@ -15,6 +16,13 @@ namespace AppInstaller::CLI::Workflow namespace { + Manifest::ScopeEnum GetFontInstallScope(Execution::Context& context) + { + return context.Args.Contains(Args::Type::InstallScope) ? + Manifest::ConvertToScopeEnum(context.Args.GetArg(Args::Type::InstallScope)) : + Manifest::ScopeEnum::User; + } + struct InstalledFontFamiliesTableLine { InstalledFontFamiliesTableLine(Utility::LocIndString familyName, int faceCount) @@ -189,17 +197,25 @@ namespace AppInstaller::CLI::Workflow } } - void FontInstallImpl(Execution::Context& context) + void EnsureSupportForFontInstall(Execution::Context& context) { - context.Reporter.Info() << Resource::String::InstallFlowStartingPackageInstall << std::endl; + if (context.Get().value().EffectiveInstallerType() == Manifest::InstallerTypeEnum::Font && + GetFontInstallScope(context) != Manifest::ScopeEnum::Machine) + { + context << EnsureNotRunningWithNonDefaultFullToken; + } + } - // We will default to User scope. - Manifest::ScopeEnum scope = Manifest::ScopeEnum::User; - if (context.Args.Contains(Execution::Args::Type::InstallScope)) + void FontInstallImpl(Execution::Context& context) + { + const auto scope = GetFontInstallScope(context); + if (scope != Manifest::ScopeEnum::Machine) { - scope = Manifest::ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope)); + AICLI_RETURN_IF_TERMINATED(context << EnsureNotRunningWithNonDefaultFullToken); } + context.Reporter.Info() << Resource::String::InstallFlowStartingPackageInstall << std::endl; + Fonts::FontContext fontContext; fontContext.InstallerSource = InstallerSource::WinGet; fontContext.Scope = scope; @@ -286,17 +302,23 @@ namespace AppInstaller::CLI::Workflow void FontUninstallImpl(Execution::Context& context) { - context.Reporter.Info() << Resource::String::UninstallFlowStartingPackageUninstall << std::endl; - try { // We will default to installed scope. - auto scope = Manifest::ConvertToScopeEnum(context.Get()->GetMetadata()[Repository::PackageVersionMetadata::InstalledScope]); + const auto installedScope = Manifest::ConvertToScopeEnum(context.Get()->GetMetadata()[Repository::PackageVersionMetadata::InstalledScope]); + auto scope = installedScope; if (context.Args.Contains(Execution::Args::Type::InstallScope)) { scope = Manifest::ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope)); } + if (installedScope != Manifest::ScopeEnum::Machine || scope != Manifest::ScopeEnum::Machine) + { + AICLI_RETURN_IF_TERMINATED(context << EnsureNotRunningWithNonDefaultFullToken); + } + + context.Reporter.Info() << Resource::String::UninstallFlowStartingPackageUninstall << std::endl; + Fonts::FontContext fontContext; fontContext.InstallerSource = InstallerSource::WinGet; if (context.Args.Contains(Execution::Args::Type::Manifest)) diff --git a/src/AppInstallerCLICore/Workflows/FontFlow.h b/src/AppInstallerCLICore/Workflows/FontFlow.h index 6f38c3294c..ede1cb8383 100644 --- a/src/AppInstallerCLICore/Workflows/FontFlow.h +++ b/src/AppInstallerCLICore/Workflows/FontFlow.h @@ -11,15 +11,22 @@ namespace AppInstaller::CLI::Workflow // Outputs: None void ReportInstalledFonts(Execution::Context& context); + // Checks elevation support before font installation consumes installed state. + // Required Args: None + // Inputs: Installer + // Outputs: None + void EnsureSupportForFontInstall(Execution::Context& context); + // Installs the font package. // Required Args: None - // Inputs: Manifest, Scope, Rename, Location + // Inputs: Manifest, Installer, InstallerPath // Outputs: None void FontInstallImpl(Execution::Context& context); // Uninstalls the font package. + // Checks both the selected package scope and the effective target scope before using installed data. // Required Args: None - // Inputs: Manifest, Scope, Rename, Location + // Inputs: InstalledPackageVersion, Manifest? // Outputs: None void FontUninstallImpl(Execution::Context& context); } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index cb40c0c423..44f47a3007 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -720,6 +720,7 @@ namespace AppInstaller::CLI::Workflow context << Workflow::EnsureRunningAsAdminForMachineScopeInstall << Workflow::EnsureSupportForPortableInstall << + Workflow::EnsureSupportForFontInstall << Workflow::EnsureValidNestedInstallerMetadataForArchiveInstall; } diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 8317adf84d..6690f44ff7 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -851,6 +852,79 @@ TEST_CASE("InstallFlow_AdminContextWithUserScopeUninstallPrevious", "[InstallFlo REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); } +TEST_CASE("FontInstallFlow_ScopeAndElevation", "[InstallFlow][workflow]") +{ + const bool fullToken = GENERATE(false, true); + const auto installerType = GENERATE(InstallerTypeEnum::Font, InstallerTypeEnum::Zip, InstallerTypeEnum::Exe); + const auto scope = GENERATE("", "user", "machine", "unknown"); + CAPTURE(fullToken, installerType, scope); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(fullToken); + + ManifestInstaller installer; + installer.BaseInstallerType = installerType; + if (installerType == InstallerTypeEnum::Zip) + { + installer.NestedInstallerType = InstallerTypeEnum::Font; + } + + std::ostringstream output; + TestContext context{ output, std::cin }; + context.Add(installer); + if (!std::string_view{ scope }.empty()) + { + context.Args.AddArg(Execution::Args::Type::InstallScope, scope); + } + + context << EnsureSupportForFontInstall; + + if (fullToken && installerType != InstallerTypeEnum::Exe && std::string_view{ scope } != "machine") + { + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); + } + else + { + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(output.str().empty()); + } +} + +TEST_CASE("FontInstallFlow_AdminContextRejectsUserScopeBeforePreparation", "[InstallFlow][workflow]") +{ + const bool execute = GENERATE(false, true); + const bool archive = GENERATE(false, true); + const bool update = GENERATE(false, true); + CAPTURE(execute, archive, update); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(true); + + ManifestInstaller installer; + installer.BaseInstallerType = archive ? InstallerTypeEnum::Zip : InstallerTypeEnum::Font; + if (archive) + { + installer.NestedInstallerType = InstallerTypeEnum::Font; + } + + std::ostringstream output; + TestContext context{ output, std::cin }; + context.Add(installer); + context.Args.AddArg(Execution::Args::Type::Force); + if (update) + { + context.SetFlags(Execution::ContextFlag::InstallerExecutionUseUpdate); + } + + if (execute) + { + context << FontInstallImpl; + } + else + { + context << EnsureSupportForInstall; + } + + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); + REQUIRE_FALSE(context.Contains(Execution::Data::OperationReturnCode)); +} + TEST_CASE("PortableInstallFlow_AdminContextRejectsUserScopeUpdateBeforePreparation", "[InstallFlow][workflow]") { using namespace AppInstaller::Manifest; diff --git a/src/AppInstallerCLITests/UninstallFlow.cpp b/src/AppInstallerCLITests/UninstallFlow.cpp index 85ce506c31..7663d61871 100644 --- a/src/AppInstallerCLITests/UninstallFlow.cpp +++ b/src/AppInstallerCLITests/UninstallFlow.cpp @@ -5,6 +5,7 @@ #include "WorkflowCommon.h" #include #include +#include #include #include #include @@ -215,7 +216,7 @@ TEST_CASE("UninstallFlow_UninstallMultiple_NotAllInstalled", "[UninstallFlow][wo TEST_CASE("UninstallFlow_AdminContextWithUserScopeInstall", "[UninstallFlow][workflow]") { - const auto installedType = GENERATE("Burn", "Msi", "Wix", "Portable"); + const auto installedType = GENERATE("Burn", "Msi", "Wix", "Portable", "Font"); CAPTURE(installedType); // Simulate running with an elevated (non-default full) token. @@ -256,3 +257,38 @@ TEST_CASE("UninstallFlow_AdminContextWithUserScopeInstall", "[UninstallFlow][wor REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); } + +TEST_CASE("FontUninstallFlow_AdminContextRejectsUserScopeOverrides", "[UninstallFlow][workflow]") +{ + using namespace AppInstaller::Repository; + + const bool prepare = GENERATE(false, true); + const auto scopeSource = GENERATE("installed", "user-argument", "machine-argument"); + CAPTURE(prepare, scopeSource); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(true); + + std::ostringstream output; + TestContext context{ output, std::cin }; + context.Add(TestPackageVersion::Make( + AppInstaller::Manifest::Manifest{}, + TestPackageVersion::MetadataMap{ + { PackageVersionMetadata::InstalledType, "Font" }, + { PackageVersionMetadata::InstalledScope, !prepare && std::string_view{ scopeSource } == "user-argument" ? "Machine" : "User" }, + })); + if (std::string_view{ scopeSource } != "installed") + { + context.Args.AddArg(Execution::Args::Type::InstallScope, std::string_view{ scopeSource } == "machine-argument" ? "machine"sv : "user"sv); + } + + if (prepare) + { + context << GetUninstallInfo; + } + else + { + context << FontUninstallImpl; + } + + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); + REQUIRE_FALSE(context.Contains(Execution::Data::OperationReturnCode)); +} diff --git a/src/AppInstallerCLITests/WorkflowBase.cpp b/src/AppInstallerCLITests/WorkflowBase.cpp index ea0a6c47eb..4cc2aa40c0 100644 --- a/src/AppInstallerCLITests/WorkflowBase.cpp +++ b/src/AppInstallerCLITests/WorkflowBase.cpp @@ -54,7 +54,7 @@ TEST_CASE("WorkflowBase_EnsureNotRunningWithNonDefaultFullToken", "[WorkflowBase TEST_CASE("WorkflowBase_EnsureSupportForInstalledPackageOperation", "[WorkflowBase]") { const bool fullToken = GENERATE(false, true); - const auto installedType = GENERATE("exe", "msi", "wix", "portable", "msix", "msstore", "unknown"); + const auto installedType = GENERATE("exe", "msi", "wix", "portable", "font", "msix", "msstore", "unknown"); const auto installedScope = GENERATE("user", "machine"); CAPTURE(fullToken, installedType, installedScope); TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride{ fullToken }; From 925a4eb4c82656da0742c7718994e7c90d0eb504 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 29 Oct 2026 10:27:48 -0700 Subject: [PATCH 4/6] build fix --- src/AppInstallerCLITests/InstallFlow.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 6690f44ff7..faefb7e8ba 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -856,7 +856,7 @@ TEST_CASE("FontInstallFlow_ScopeAndElevation", "[InstallFlow][workflow]") { const bool fullToken = GENERATE(false, true); const auto installerType = GENERATE(InstallerTypeEnum::Font, InstallerTypeEnum::Zip, InstallerTypeEnum::Exe); - const auto scope = GENERATE("", "user", "machine", "unknown"); + const auto scope = GENERATE(""sv, "user"sv, "machine"sv, "unknown"sv); CAPTURE(fullToken, installerType, scope); TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(fullToken); @@ -870,14 +870,14 @@ TEST_CASE("FontInstallFlow_ScopeAndElevation", "[InstallFlow][workflow]") std::ostringstream output; TestContext context{ output, std::cin }; context.Add(installer); - if (!std::string_view{ scope }.empty()) + if (!scope.empty()) { context.Args.AddArg(Execution::Args::Type::InstallScope, scope); } context << EnsureSupportForFontInstall; - if (fullToken && installerType != InstallerTypeEnum::Exe && std::string_view{ scope } != "machine") + if (fullToken && installerType != InstallerTypeEnum::Exe && scope != "machine") { REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); } From feed58b7fcf55ad33c1a2e36145140188c89af5d Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 29 Oct 2026 10:41:58 -0700 Subject: [PATCH 5/6] spelling --- doc/release-notes/1.31/user-scope-elevation-guards.md | 2 +- src/AppInstallerCLICore/PortableInstaller.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/1.31/user-scope-elevation-guards.md b/doc/release-notes/1.31/user-scope-elevation-guards.md index 1449e87c63..2bc6721805 100644 --- a/doc/release-notes/1.31/user-scope-elevation-guards.md +++ b/doc/release-notes/1.31/user-scope-elevation-guards.md @@ -6,6 +6,6 @@ Added elevated-process protections for user-scope MSI uninstall and repair, port Font protection also covers initial and forced installations, which can inspect or replace existing font registrations. -Fresh portable installations remain supported from an elevated process and no longer reuse pre-existing installation metadata or cleanup records. +Fresh portable installations remain supported from an elevated process and no longer reuse preexisting installation metadata or cleanup records. The checks apply to the elevated half of a split UAC token. Existing behavior with UAC disabled is unchanged. diff --git a/src/AppInstallerCLICore/PortableInstaller.cpp b/src/AppInstallerCLICore/PortableInstaller.cpp index 4152a1f95f..6e065664f2 100644 --- a/src/AppInstallerCLICore/PortableInstaller.cpp +++ b/src/AppInstallerCLICore/PortableInstaller.cpp @@ -334,7 +334,7 @@ namespace AppInstaller::CLI::Portable { if (!UsesExistingInstallData()) { - // Replace the selected index slot without opening any pre-existing database or journal. + // Replace the selected index slot without opening any preexisting database or journal. SQLiteStorageBase::DeleteSQLiteDatabase(TargetInstallLocation / GetPortableIndexFileName()); } else From f13e168997cfe81304e1cba59b9ac3c5c195fb96 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 29 Oct 2026 16:02:25 -0700 Subject: [PATCH 6/6] Test and product fixes --- .../1.31/user-scope-elevation-guards.md | 4 ++ .../Workflows/DependenciesFlow.cpp | 9 +++ .../Workflows/DependenciesFlow.h | 3 +- .../InstallDependenciesFlow.cpp | 72 +++++++++++++++++++ .../PortableInstaller.cpp | 2 +- 5 files changed, 88 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/1.31/user-scope-elevation-guards.md b/doc/release-notes/1.31/user-scope-elevation-guards.md index 2bc6721805..61b10475e5 100644 --- a/doc/release-notes/1.31/user-scope-elevation-guards.md +++ b/doc/release-notes/1.31/user-scope-elevation-guards.md @@ -9,3 +9,7 @@ Font protection also covers initial and forced installations, which can inspect Fresh portable installations remain supported from an elevated process and no longer reuse preexisting installation metadata or cleanup records. The checks apply to the elevated half of a split UAC token. Existing behavior with UAC disabled is unchanged. + +### Minor Bug Fixes + +* Fixed installing new dependencies during an upgrade and applying upgrade protections when an installed dependency needs an update. diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp index 089091fc1e..e9155fa6f6 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp @@ -341,6 +341,15 @@ namespace AppInstaller::CLI::Workflow dependencyContext.Add(itr->second.InstalledPackageVersion); dependencyContext.Add(itr->second.Installer); + if (itr->second.InstalledPackageVersion) + { + dependencyContext.SetFlags(Execution::ContextFlag::InstallerExecutionUseUpdate); + } + else + { + dependencyContext.ClearFlags(Execution::ContextFlag::InstallerExecutionUseUpdate); + } + if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerDownloadOnly)) { dependencyContext.Add(context.Get() / L"Dependencies"); diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.h b/src/AppInstallerCLICore/Workflows/DependenciesFlow.h index bcba4f86d0..c2477b8bf7 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.h +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.h @@ -40,9 +40,10 @@ namespace AppInstaller::CLI::Workflow void GetDependenciesInfoForUninstall(Execution::Context& context); // Builds the dependency graph and creates the sub contexts for each package dependency. + // Each dependency's installed version determines whether its operation is an install or upgrade. // Required Args: None // Inputs: Manifest, Installer and DependencySource - // Outputs: Dependencies + // Outputs: PackageSubContexts struct CreateDependencySubContexts : public WorkflowTask { CreateDependencySubContexts( diff --git a/src/AppInstallerCLITests/InstallDependenciesFlow.cpp b/src/AppInstallerCLITests/InstallDependenciesFlow.cpp index f7afd9b7af..09c3323cdc 100644 --- a/src/AppInstallerCLITests/InstallDependenciesFlow.cpp +++ b/src/AppInstallerCLITests/InstallDependenciesFlow.cpp @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" +#include "TestHooks.h" #include "WorkflowCommon.h" #include "DependenciesTestSource.h" #include @@ -96,6 +97,77 @@ TEST_CASE("DependencyGraph_validMinVersions", "[InstallFlow][workflow][dependenc REQUIRE(dependencyPackages.at(0)->Get().Version == "1.0"); } +TEST_CASE("DependencyGraph_UsesDependencyInstalledState", "[InstallFlow][workflow][dependencyGraph][dependencies]") +{ + const bool parentIsUpdate = GENERATE(false, true); + const bool dependencyIsInstalled = GENERATE(false, true); + const bool fullToken = GENERATE(false, true); + CAPTURE(parentIsUpdate, dependencyIsInstalled, fullToken); + TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(fullToken); + + auto manifest = CreateFakeManifestWithDependencies("Root"); + manifest.Installers[0].Dependencies.Add(Dependency(DependencyType::Package, "Dependency", "2.0")); + + auto dependencyManifest = CreateFakeManifestWithDependencies("Dependency"); + dependencyManifest.Version = "2.0"; + dependencyManifest.Installers[0].Scope = ScopeEnum::User; + dependencyManifest.Installers[0].UpdateBehavior = UpdateBehaviorEnum::UninstallPrevious; + auto source = std::make_shared(); + auto package = TestCompositePackage::Make(std::vector{ dependencyManifest }, source); + if (dependencyIsInstalled) + { + auto installedManifest = dependencyManifest; + installedManifest.Version = "1.0"; + package->Installed = TestPackage::Make( + installedManifest, + TestPackageVersion::MetadataMap{ + { PackageVersionMetadata::InstalledType, "Exe" }, + { PackageVersionMetadata::InstalledScope, "User" }, + }, + source); + } + source->SearchFunction = [package](const SearchRequest&) + { + SearchResult result; + result.Matches.emplace_back(package, PackageMatchFilter(PackageMatchField::Id, MatchType::CaseInsensitive, "Dependency")); + return result; + }; + + std::ostringstream output; + TestContext context{ output, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + if (parentIsUpdate) + { + context.SetFlags(Execution::ContextFlag::InstallerExecutionUseUpdate); + } + context.Add(manifest); + context.Add(manifest.Installers[0]); + context.Override({ Workflow::OpenDependencySource, [source](TestContext& c) + { + c.Add(Source{ source }); + } }); + + context << CreateDependencySubContexts(Resource::String::PackageRequiresDependencies); + REQUIRE_FALSE(context.IsTerminated()); + auto& dependencies = context.Get(); + REQUIRE(dependencies.size() == 1); + auto& dependencyContext = *dependencies.at(0); + auto dependencyThreadGlobals = dependencyContext.SetForCurrentThread(); + REQUIRE(WI_IsFlagSet(dependencyContext.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate) == dependencyIsInstalled); + REQUIRE(WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate) == parentIsUpdate); + + REQUIRE_NOTHROW(dependencyContext << EnsureSupportForInstall); + INFO(output.str()); + if (dependencyIsInstalled && fullToken) + { + REQUIRE_TERMINATED_WITH(dependencyContext, APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_ACTION_PROHIBITED); + } + else + { + REQUIRE_FALSE(dependencyContext.IsTerminated()); + } +} + TEST_CASE("DependencyGraph_PathNoLoop", "[InstallFlow][workflow][dependencyGraph][dependencies]", ) { std::ostringstream installOutput; diff --git a/src/AppInstallerCLITests/PortableInstaller.cpp b/src/AppInstallerCLITests/PortableInstaller.cpp index 9c098b10e8..697895cd50 100644 --- a/src/AppInstallerCLITests/PortableInstaller.cpp +++ b/src/AppInstallerCLITests/PortableInstaller.cpp @@ -94,7 +94,7 @@ TEST_CASE("PortableInstaller_FreshInstallIgnoresExistingState", "[PortableInstal auto index = PortableIndex::Open(targetIndexPath.u8string(), SQLiteStorageBase::OpenDisposition::Read); auto files = index.GetAllPortableFiles(); REQUIRE(files.size() == 1); - REQUIRE(files.at(0).GetFilePath() == targetPath); + REQUIRE(std::filesystem::equivalent(files.at(0).GetFilePath(), targetPath)); } else {