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..61b10475e5 --- /dev/null +++ b/doc/release-notes/1.31/user-scope-elevation-guards.md @@ -0,0 +1,15 @@ +## Bug Fixes + +### Consistent elevation checks for user-scope package operations + +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 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/PortableInstaller.cpp b/src/AppInstallerCLICore/PortableInstaller.cpp index 130d672b5b..6e065664f2 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 preexisting 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/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/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/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 f0eb5731a1..44f47a3007 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,9 +712,15 @@ 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 << + Workflow::EnsureSupportForFontInstall << Workflow::EnsureValidNestedInstallerMetadataForArchiveInstall; } 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/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/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/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/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/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 30bf3da050..faefb7e8ba 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -17,7 +17,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -712,6 +714,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 +767,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 +781,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 +819,207 @@ 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("FontInstallFlow_ScopeAndElevation", "[InstallFlow][workflow]") +{ + const bool fullToken = GENERATE(false, true); + const auto installerType = GENERATE(InstallerTypeEnum::Font, InstallerTypeEnum::Zip, InstallerTypeEnum::Exe); + const auto scope = GENERATE(""sv, "user"sv, "machine"sv, "unknown"sv); + 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 (!scope.empty()) + { + context.Args.AddArg(Execution::Args::Type::InstallScope, scope); + } + + context << EnsureSupportForFontInstall; + + if (fullToken && installerType != InstallerTypeEnum::Exe && 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; + 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..697895cd50 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(std::filesystem::equivalent(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..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,6 +216,9 @@ TEST_CASE("UninstallFlow_UninstallMultiple_NotAllInstalled", "[UninstallFlow][wo TEST_CASE("UninstallFlow_AdminContextWithUserScopeInstall", "[UninstallFlow][workflow]") { + const auto installedType = GENERATE("Burn", "Msi", "Wix", "Portable", "Font"); + CAPTURE(installedType); + // Simulate running with an elevated (non-default full) token. TestHook::SetIsRunningWithNonDefaultFullToken_Override tokenOverride(true); @@ -222,12 +226,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 +238,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 }, @@ -254,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/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..4cc2aa40c0 --- /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", "font", "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/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/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)) { 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);