commit 12dbc22b75f04a94c19f85828122f06c8daa48db parent 9799443c77bb59e20af8a448964e145f8add6d20 Author: Chacón <lechacon@users.noreply.github.com> Date: Wed, 13 Oct 2021 15:57:15 -0700 Split COM install command into download and install stages (#1528) Diffstat:
16 files changed, 678 insertions(+), 394 deletions(-)
diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt @@ -385,6 +385,7 @@ regex regexp removemanifest repolibtest +requeue rescap resheader resmimetype diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj @@ -275,6 +275,7 @@ <ClInclude Include="VTSupport.h" /> <ClInclude Include="PackageCollection.h" /> <ClInclude Include="Workflows\CompletionFlow.h" /> + <ClInclude Include="Workflows\DownloadFlow.h" /> <ClInclude Include="Workflows\ImportExportFlow.h" /> <ClInclude Include="Workflows\MsiInstallFlow.h" /> <ClInclude Include="Workflows\MSStoreInstallerHandler.h" /> @@ -324,6 +325,7 @@ <ClCompile Include="Resources.cpp" /> <ClCompile Include="VTSupport.cpp" /> <ClCompile Include="Workflows\CompletionFlow.cpp" /> + <ClCompile Include="Workflows\DownloadFlow.cpp" /> <ClCompile Include="Workflows\ImportExportFlow.cpp" /> <ClCompile Include="Workflows\MsiInstallFlow.cpp" /> <ClCompile Include="Workflows\MSStoreInstallerHandler.cpp" /> diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters @@ -170,6 +170,9 @@ <ClInclude Include="Workflows\SettingsFlow.h"> <Filter>Workflows</Filter> </ClInclude> + <ClInclude Include="Workflows\DownloadFlow.h"> + <Filter>Workflows</Filter> + </ClInclude> </ItemGroup> <ItemGroup> <ClCompile Include="pch.cpp"> @@ -307,6 +310,9 @@ <ClCompile Include="Workflows\SettingsFlow.cpp"> <Filter>Workflows</Filter> </ClCompile> + <ClCompile Include="Workflows\DlownloadFlow.cpp"> + <Filter>Workflows</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> diff --git a/src/AppInstallerCLICore/Commands/COMInstallCommand.cpp b/src/AppInstallerCLICore/Commands/COMInstallCommand.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "COMInstallCommand.h" +#include "Workflows/DownloadFlow.h" #include "Workflows/InstallFlow.h" #include "Workflows/WorkflowBase.h" @@ -13,12 +14,21 @@ using namespace AppInstaller::Utility::literals; namespace AppInstaller::CLI { // IMPORTANT: To use this command, the caller should have already retrieved the package manifest (GetManifest()) and added it to the Context Data - void COMInstallCommand::ExecuteInternal(Context& context) const + void COMDownloadCommand::ExecuteInternal(Context& context) const { context << Workflow::ReportExecutionStage(ExecutionStage::Discovery) << Workflow::SelectInstaller << Workflow::EnsureApplicableInstaller << - Workflow::InstallSinglePackage; + Workflow::DownloadSinglePackage; + } + + // IMPORTANT: To use this command, the caller should have already executed the COMDownloadCommand + void COMInstallCommand::ExecuteInternal(Context& context) const + { + context << + Workflow::GetInstallerHash << + Workflow::VerifyInstallerHash << + Workflow::InstallPackageInstaller; } } diff --git a/src/AppInstallerCLICore/Commands/COMInstallCommand.h b/src/AppInstallerCLICore/Commands/COMInstallCommand.h @@ -6,6 +6,15 @@ namespace AppInstaller::CLI { // IMPORTANT: To use this command, the caller should have already retrieved the package manifest (GetManifest()) and added it to the Context Data + struct COMDownloadCommand final : public Command + { + COMDownloadCommand(std::string_view parent) : Command("download", parent) {} + + protected: + void ExecuteInternal(Execution::Context& context) const override; + }; + + // IMPORTANT: To use this command, the caller should have already retrieved the package manifest (GetManifest()) and added it to the Context Data struct COMInstallCommand final : public Command { COMInstallCommand(std::string_view parent) : Command("install", parent) {} diff --git a/src/AppInstallerCLICore/Commands/RootCommand.h b/src/AppInstallerCLICore/Commands/RootCommand.h @@ -7,7 +7,9 @@ namespace AppInstaller::CLI { struct RootCommand final : public Command { - RootCommand() : Command("root", {}) {} + constexpr static std::string_view CommandName = "root"sv; + + RootCommand() : Command(CommandName, {}) {} std::vector<std::unique_ptr<Command>> GetCommands() const override; std::vector<Argument> GetArguments() const override; diff --git a/src/AppInstallerCLICore/ContextOrchestrator.cpp b/src/AppInstallerCLICore/ContextOrchestrator.cpp @@ -59,6 +59,13 @@ namespace AppInstaller::CLI::Execution } } + void ContextOrchestrator::RequeueItem(OrchestratorQueueItem& item) + { + std::lock_guard<std::mutex> lockQueue{ m_queueLock }; + + item.SetState(OrchestratorQueueItemState::Queued); + } + void ContextOrchestrator::EnqueueAndRunItem(std::shared_ptr<OrchestratorQueueItem> item) { EnqueueItem(item); @@ -101,11 +108,10 @@ namespace AppInstaller::CLI::Execution HRESULT terminationHR = S_OK; try { - ::AppInstaller::CLI::RootCommand rootCommand; + std::unique_ptr<Command> command = item->PopNextCommand(); std::unique_ptr<AppInstaller::ThreadLocalStorage::PreviousThreadGlobals> setThreadGlobalsToPreviousState = item->GetContext().GetThreadGlobals().SetForCurrentThread(); - std::unique_ptr<::AppInstaller::CLI::Command> command = std::make_unique<::AppInstaller::CLI::COMInstallCommand>(rootCommand.Name()); item->GetContext().GetThreadGlobals().GetTelemetryLogger().LogCommand(command->FullName()); command->ValidateArguments(item->GetContext().Args); @@ -123,7 +129,16 @@ namespace AppInstaller::CLI::Execution item->GetContext().SetTerminationHR(terminationHR); } - RemoveItemInState(*item, OrchestratorQueueItemState::Running); + item->GetContext().EnableCtrlHandler(false); + + if (FAILED(terminationHR) || item->IsComplete()) + { + RemoveItemInState(*item, OrchestratorQueueItemState::Running); + } + else + { + RequeueItem(*item); + } item = GetNextItem(); } @@ -180,7 +195,10 @@ namespace AppInstaller::CLI::Execution std::unique_ptr<OrchestratorQueueItem> OrchestratorQueueItemFactory::CreateItemForInstall(std::wstring packageId, std::wstring sourceId, std::unique_ptr<COMContext> context) { - return std::make_unique<OrchestratorQueueItem>(OrchestratorQueueItemId(std::move(packageId), std::move(sourceId)), std::move(context)); + std::unique_ptr<OrchestratorQueueItem> item = std::make_unique<OrchestratorQueueItem>(OrchestratorQueueItemId(std::move(packageId), std::move(sourceId)), std::move(context)); + item->AddCommand(std::make_unique<::AppInstaller::CLI::COMDownloadCommand>(RootCommand::CommandName)); + item->AddCommand(std::make_unique<::AppInstaller::CLI::COMInstallCommand>(RootCommand::CommandName)); + return item; } } diff --git a/src/AppInstallerCLICore/ContextOrchestrator.h b/src/AppInstallerCLICore/ContextOrchestrator.h @@ -6,6 +6,7 @@ #include "ExecutionArgs.h" #include "ExecutionContextData.h" #include "CompletionData.h" +#include "Command.h" #include "COMContext.h" #include <string_view> @@ -40,11 +41,20 @@ namespace AppInstaller::CLI::Execution COMContext& GetContext() const { return *m_context; } const wil::unique_event& GetCompletedEvent() const { return m_completedEvent; } const OrchestratorQueueItemId& GetId() const { return m_id; } + void AddCommand(std::unique_ptr<Command> command) { m_commands.push_back(std::move(command)); } + std::unique_ptr<Command> PopNextCommand() + { + std::unique_ptr<Command> command = std::move(m_commands.front()); + m_commands.pop_front(); + return command; + } + bool IsComplete() const { return m_commands.empty(); } private: OrchestratorQueueItemState m_state = OrchestratorQueueItemState::NotQueued; std::unique_ptr<COMContext> m_context; wil::unique_event m_completedEvent{ wil::EventOptions::ManualReset }; OrchestratorQueueItemId m_id; + std::deque<std::unique_ptr<Command>> m_commands; }; struct OrchestratorQueueItemFactory @@ -67,6 +77,7 @@ namespace AppInstaller::CLI::Execution void RunItems(); std::shared_ptr<OrchestratorQueueItem> GetNextItem(); void EnqueueItem(std::shared_ptr<OrchestratorQueueItem> item); + void RequeueItem(OrchestratorQueueItem& item); void RemoveItemInState(const OrchestratorQueueItem& item, OrchestratorQueueItemState state); _Requires_lock_held_(m_queueLock) diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp @@ -0,0 +1,496 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "DownloadFlow.h" + +#include <AppInstallerMsixInfo.h> + +namespace AppInstaller::CLI::Workflow +{ + using namespace AppInstaller::Manifest; + using namespace AppInstaller::Repository; + using namespace AppInstaller::Utility; + using namespace std::string_view_literals; + + namespace + { + // Get the base download path for the installer path. + // This path does not include the file extension, which will be added + // after verifying the file hash to prevent it from being ShellExecute-d + std::filesystem::path GetInstallerBaseDownloadPath(Execution::Context& context) + { + const auto& manifest = context.Get<Execution::Data::Manifest>(); + std::filesystem::path tempInstallerPath = Runtime::GetPathTo(Runtime::PathName::Temp); + tempInstallerPath /= Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version); + return tempInstallerPath; + } + + // Get the file extension to be used for the installer file. + std::wstring_view GetInstallerFileExtension(Execution::Context& context) + { + const auto& installer = context.Get<Execution::Data::Installer>(); + switch (installer->InstallerType) + { + case InstallerTypeEnum::Burn: + case InstallerTypeEnum::Exe: + case InstallerTypeEnum::Inno: + case InstallerTypeEnum::Nullsoft: + return L".exe"sv; + case InstallerTypeEnum::Msi: + case InstallerTypeEnum::Wix: + return L".msi"sv; + case InstallerTypeEnum::Msix: + // Note: We may need to distinguish between .msix and .msixbundle in the future. + return L".msix"sv; + case InstallerTypeEnum::Zip: + return L".zip"sv; + default: + THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED)); + } + } + + // Try to remove the installer file, ignoring any errors. + void RemoveInstallerFile(const std::filesystem::path& path) + { + try + { + std::filesystem::remove(path); + } + catch (const std::exception& e) + { + AICLI_LOG(CLI, Warning, << "Failed to remove installer file. Reason: " << e.what()); + } + catch (...) + { + AICLI_LOG(CLI, Warning, << "Failed to remove installer file. Reason unknown."); + } + + } + + // Checks the file hash for an existing installer file. + // Returns true if the file exists and its hash matches, false otherwise. + // If the hash does not match, deletes the file. + bool ExistingInstallerFileHasHashMatch(const SHA256::HashBuffer& expectedHash, const std::filesystem::path& filePath, SHA256::HashBuffer& fileHash) + { + if (std::filesystem::exists(filePath)) + { + AICLI_LOG(CLI, Info, << "Found existing installer file at '" << filePath << "'. Verifying file hash."); + std::ifstream inStream{ filePath, std::ifstream::binary }; + fileHash = SHA256::ComputeHash(inStream); + + if (SHA256::AreEqual(expectedHash, fileHash)) + { + return true; + } + + AICLI_LOG(CLI, Info, << "Hash does not match. Removing existing installer file " << filePath); + RemoveInstallerFile(filePath); + } + + return false; + } + + // Complicated rename algorithm due to somewhat arbitrary failures. + // 1. First, try to rename. + // 2. Then, create an empty file for the target, and attempt to rename. + // 3. Then, try repeatedly for 500ms in case it is a timing thing. + // 4. Attempt to use a hard link if available. + // 5. Copy the file if nothing else has worked so far. + void RenameFile(const std::filesystem::path& from, const std::filesystem::path& to) + { + // 1. First, try to rename. + try + { + // std::filesystem::rename() handles motw correctly if applicable. + std::filesystem::rename(from, to); + return; + } + CATCH_LOG(); + + // 2. Then, create an empty file for the target, and attempt to rename. + // This seems to fix things in certain cases, so we do it. + try + { + { + std::ofstream targetFile{ to }; + } + std::filesystem::rename(from, to); + return; + } + CATCH_LOG(); + + // 3. Then, try repeatedly for 500ms in case it is a timing thing. + for (int i = 0; i < 5; ++i) + { + try + { + std::this_thread::sleep_for(100ms); + std::filesystem::rename(from, to); + return; + } + CATCH_LOG(); + } + + // 4. Attempt to use a hard link if available. + if (Runtime::SupportsHardLinks(from)) + { + try + { + // Create a hard link to the file; the installer will be left in the temp directory afterward + // but it is better to succeed the operation and leave a file around than to fail. + // First we have to remove the target file as the function will not overwrite. + std::filesystem::remove(to); + std::filesystem::create_hard_link(from, to); + return; + } + CATCH_LOG(); + } + + // 5. Copy the file if nothing else has worked so far. + // Create a copy of the file; the installer will be left in the temp directory afterward + // but it is better to succeed the operation and leave a file around than to fail. + std::filesystem::copy_file(from, to, std::filesystem::copy_options::overwrite_existing); + } + } + + void DownloadInstaller(Execution::Context& context) + { + // Check if file was already downloaded. + // This may happen after a failed installation or if the download was done + // separately before, e.g. on COM scenarios. + context << + ReportExecutionStage(ExecutionStage::Download) << + CheckForExistingInstaller; + if (context.IsTerminated()) + { + return; + } + + // CheckForExistingInstaller will set the InstallerPath if found + if (!context.Contains(Execution::Data::InstallerPath)) + { + const auto& installer = context.Get<Execution::Data::Installer>().value(); + switch (installer.InstallerType) + { + case InstallerTypeEnum::Exe: + case InstallerTypeEnum::Burn: + case InstallerTypeEnum::Inno: + case InstallerTypeEnum::Msi: + case InstallerTypeEnum::Nullsoft: + case InstallerTypeEnum::Wix: + context << DownloadInstallerFile; + break; + case InstallerTypeEnum::Msix: + if (installer.SignatureSha256.empty()) + { + context << DownloadInstallerFile; + } + else + { + // Signature hash provided. No download needed. Just verify signature hash. + context << GetMsixSignatureHash; + } + break; + case InstallerTypeEnum::MSStore: + // Nothing to do here + return; + default: + THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED)); + } + } + + context << + VerifyInstallerHash << + UpdateInstallerFileMotwIfApplicable << + RenameDownloadedInstaller; + } + + void CheckForExistingInstaller(Execution::Context& context) + { + const auto& installer = context.Get<Execution::Data::Installer>().value(); + if (installer.InstallerType == InstallerTypeEnum::MSStore) + { + // No installer is downloaded in this case + return; + } + + // Try looking for the file with and without extension. + auto installerPath = GetInstallerBaseDownloadPath(context); + SHA256::HashBuffer fileHash; + if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath, fileHash)) + { + installerPath += GetInstallerFileExtension(context); + if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath, fileHash)) + { + // No match + return; + } + } + + AICLI_LOG(CLI, Info, << "Existing installer file hash matches. Will use existing installer."); + context.Add<Execution::Data::InstallerPath>(installerPath); + context.Add<Execution::Data::HashPair>(std::make_pair(installer.Sha256, fileHash)); + } + + void GetInstallerDownloadPath(Execution::Context& context) + { + if (!context.Contains(Execution::Data::InstallerPath)) + { + auto tempInstallerPath = GetInstallerBaseDownloadPath(context); + AICLI_LOG(CLI, Info, << "Generated temp download path: " << tempInstallerPath); + context.Add<Execution::Data::InstallerPath>(std::move(tempInstallerPath)); + } + } + + void DownloadInstallerFile(Execution::Context& context) + { + context << GetInstallerDownloadPath; + if (context.IsTerminated()) + { + return; + } + + const auto& installer = context.Get<Execution::Data::Installer>().value(); + const auto& installerPath = context.Get<Execution::Data::InstallerPath>(); + + Utility::DownloadInfo downloadInfo{}; + downloadInfo.DisplayName = Resource::GetFixedString(Resource::FixedString::ProductName); + // Use the SHA256 hash of the installer as the identifier for the download + downloadInfo.ContentId = SHA256::ConvertToString(installer.Sha256); + + context.Reporter.Info() << "Downloading " << Execution::UrlEmphasis << installer.Url << std::endl; + + std::optional<std::vector<BYTE>> hash; + + const int MaxRetryCount = 2; + for (int retryCount = 0; retryCount < MaxRetryCount; ++retryCount) + { + bool success = false; + try + { + hash = context.Reporter.ExecuteWithProgress(std::bind(Utility::Download, + installer.Url, + installerPath, + Utility::DownloadType::Installer, + std::placeholders::_1, + true, + downloadInfo)); + + success = true; + } + catch (...) + { + if (retryCount < MaxRetryCount - 1) + { + AICLI_LOG(CLI, Info, << "Failed to download, waiting a bit and retry. Url: " << installer.Url); + Sleep(500); + } + else + { + throw; + } + } + + if (success) + { + break; + } + } + + if (!hash) + { + context.Reporter.Info() << "Package download canceled." << std::endl; + AICLI_TERMINATE_CONTEXT(E_ABORT); + } + + context.Add<Execution::Data::HashPair>(std::make_pair(installer.Sha256, hash.value())); + } + + void GetMsixSignatureHash(Execution::Context& context) + { + // We use this when the server won't support streaming install to swap to download. + bool downloadInstead = false; + + try + { + const auto& installer = context.Get<Execution::Data::Installer>().value(); + + Msix::MsixInfo msixInfo(installer.Url); + auto signature = msixInfo.GetSignature(); + + auto signatureHash = SHA256::ComputeHash(signature.data(), static_cast<uint32_t>(signature.size())); + + context.Add<Execution::Data::HashPair>(std::make_pair(installer.SignatureSha256, signatureHash)); + } + catch (const winrt::hresult_error& e) + { + if (static_cast<HRESULT>(e.code()) == HRESULT_FROM_WIN32(ERROR_NO_RANGES_PROCESSED) || + HRESULT_FACILITY(e.code()) == FACILITY_HTTP) + { + // Failed to get signature hash through HttpStream, use download + downloadInstead = true; + } + else + { + throw; + } + } + + if (downloadInstead) + { + context << DownloadInstallerFile; + } + } + + void VerifyInstallerHash(Execution::Context& context) + { + const auto& hashPair = context.Get<Execution::Data::HashPair>(); + + if (!std::equal( + hashPair.first.begin(), + hashPair.first.end(), + hashPair.second.begin())) + { + bool overrideHashMismatch = context.Args.Contains(Execution::Args::Type::HashOverride); + + const auto& manifest = context.Get<Execution::Data::Manifest>(); + Logging::Telemetry().LogInstallerHashMismatch(manifest.Id, manifest.Version, manifest.Channel, hashPair.first, hashPair.second, overrideHashMismatch); + + // If running as admin, do not allow the user to override the hash failure. + if (Runtime::IsRunningAsAdmin()) + { + context.Reporter.Error() << Resource::String::InstallerHashMismatchAdminBlock << std::endl; + } + else if (overrideHashMismatch) + { + context.Reporter.Warn() << Resource::String::InstallerHashMismatchOverridden << std::endl; + return; + } + else if (Settings::GroupPolicies().IsEnabled(Settings::TogglePolicy::Policy::HashOverride)) + { + context.Reporter.Error() << Resource::String::InstallerHashMismatchOverrideRequired << std::endl; + } + else + { + context.Reporter.Error() << Resource::String::InstallerHashMismatchError << std::endl; + } + + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALLER_HASH_MISMATCH); + } + else + { + AICLI_LOG(CLI, Info, << "Installer hash verified"); + context.Reporter.Info() << Resource::String::InstallerHashVerified << std::endl; + + context.SetFlags(Execution::ContextFlag::InstallerHashMatched); + + if (context.Contains(Execution::Data::PackageVersion) && + context.Get<Execution::Data::PackageVersion>()->GetSource() != nullptr && + WI_IsFlagSet(context.Get<Execution::Data::PackageVersion>()->GetSource()->GetDetails().TrustLevel, SourceTrustLevel::Trusted)) + { + context.SetFlags(Execution::ContextFlag::InstallerTrusted); + } + } + } + + void UpdateInstallerFileMotwIfApplicable(Execution::Context& context) + { + if (context.Contains(Execution::Data::InstallerPath)) + { + if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerTrusted)) + { + Utility::ApplyMotwIfApplicable(context.Get<Execution::Data::InstallerPath>(), URLZONE_TRUSTED); + } + else if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerHashMatched)) + { + const auto& installer = context.Get<Execution::Data::Installer>(); + HRESULT hr = Utility::ApplyMotwUsingIAttachmentExecuteIfApplicable(context.Get<Execution::Data::InstallerPath>(), installer.value().Url, URLZONE_INTERNET); + + // Not using SUCCEEDED(hr) to check since there are cases file is missing after a successful scan + if (hr != S_OK) + { + switch (hr) + { + case INET_E_SECURITY_PROBLEM: + context.Reporter.Error() << Resource::String::InstallerBlockedByPolicy << std::endl; + break; + case E_FAIL: + context.Reporter.Error() << Resource::String::InstallerFailedVirusScan << std::endl; + break; + default: + context.Reporter.Error() << Resource::String::InstallerFailedSecurityCheck << std::endl; + } + + AICLI_LOG(Fail, Error, << "Installer failed security check. Url: " << installer.value().Url << " Result: " << WINGET_OSTREAM_FORMAT_HRESULT(hr)); + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALLER_SECURITY_CHECK_FAILED); + } + } + } + } + + void GetInstallerHash(Execution::Context& context) + { + const auto& installer = context.Get<Execution::Data::Installer>().value(); + + if (context.Contains(Execution::Data::InstallerPath)) + { + // Get the hash from the installer file + const auto& installerPath = context.Get<Execution::Data::InstallerPath>(); + std::ifstream inStream{ installerPath, std::ifstream::binary }; + auto existingFileHash = SHA256::ComputeHash(inStream); + context.Add<Execution::Data::HashPair>(std::make_pair(installer.Sha256, existingFileHash)); + } + else if (installer.InstallerType == InstallerTypeEnum::MSStore) + { + // No installer file in this case + return; + } + else if (installer.InstallerType == InstallerTypeEnum::Msix && !installer.SignatureSha256.empty()) + { + // We didn't download the installer file before. Just verify the signature hash again. + context << GetMsixSignatureHash; + } + else + { + // No installer downloaded + AICLI_LOG(CLI, Error, << "Installer file not found."); + AICLI_TERMINATE_CONTEXT(HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)); + } + } + + void RenameDownloadedInstaller(Execution::Context& context) + { + if (!context.Contains(Execution::Data::InstallerPath)) + { + // No installer downloaded, no need to rename anything. + return; + } + + auto& installerPath = context.Get<Execution::Data::InstallerPath>(); + auto installerExtension = GetInstallerFileExtension(context); + if (installerPath.extension() == installerExtension) + { + // Installer file already has expected extension. + return; + } + + std::filesystem::path renamedDownloadedInstaller(installerPath); + renamedDownloadedInstaller += installerExtension; + + RenameFile(installerPath, renamedDownloadedInstaller); + + installerPath.assign(renamedDownloadedInstaller); + AICLI_LOG(CLI, Info, << "Successfully renamed downloaded installer. Path: " << installerPath); + } + + void RemoveInstaller(Execution::Context& context) + { + // Path may not be present if installed from a URL for MSIX + if (context.Contains(Execution::Data::InstallerPath)) + { + const auto& path = context.Get<Execution::Data::InstallerPath>(); + AICLI_LOG(CLI, Info, << "Removing installer: " << path); + RemoveInstallerFile(path); + } + } +} diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.h b/src/AppInstallerCLICore/Workflows/DownloadFlow.h @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "ExecutionContext.h" + +namespace AppInstaller::CLI::Workflow +{ + // Composite flow that chooses what to do based on the installer type. + // Required Args: None + // Inputs: Manifest, Installer + // Outputs: None + void DownloadInstaller(Execution::Context& context); + + // Check if the desired installer has already been downloaded. + // Required Args: None + // Inputs: Manifest, Installer + // Outputs: HashPair, InstallerPath (only if found) + void CheckForExistingInstaller(Execution::Context& context); + + // Computes the download path for the installer file. Does nothing if already determined + // Required Args: None + // Inputs: Installer, Manifest + // Outputs: InstallerPath + void GetInstallerDownloadPath(Execution::Context& context); + + // Downloads the file referenced by the Installer. + // Required Args: None + // Inputs: Installer, Manifest + // Outputs: HashPair, InstallerPath + void DownloadInstallerFile(Execution::Context& context); + + // Computes the hash of the MSIX signature file. + // Required Args: None + // Inputs: Installer + // Outputs: HashPair + void GetMsixSignatureHash(Execution::Context& context); + + // Gets the hash of the downloaded installer. + // Downloading already computes the hash, so this is only needed to re-verify the installer hash. + // Required Args: None + // Inputs: InstallerPath, Installer + // Outputs: HashPair + void GetInstallerHash(Execution::Context& context); + + // Verifies that the downloaded installer hash matches the hash in the manifest. + // Required Args: None + // Inputs: HashPair + // Outputs: None + void VerifyInstallerHash(Execution::Context& context); + + // Update Motw of the downloaded installer if applicable + // Required Args: None + // Inputs: HashPair, InstallerPath?, SourceId? + // Outputs: None + void UpdateInstallerFileMotwIfApplicable(Execution::Context& context); + + // This method appends appropriate extension to the downloaded installer. + // ShellExecute uses file extension to launch the installer appropriately. + // Required Args: None + // Inputs: Installer, InstallerPath + // Modifies: InstallerPath + // Outputs: None + void RenameDownloadedInstaller(Execution::Context& context); + + // Deletes the installer file. + // Required Args: None + // Inputs: InstallerPath + // Outputs: None + void RemoveInstaller(Execution::Context& context); +} diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "InstallFlow.h" +#include "DownloadFlow.h" #include "UninstallFlow.h" #include "ShowFlow.h" #include "Resources.h" @@ -12,7 +13,6 @@ #include "Workflows/DependenciesFlow.h" #include <AppInstallerDeployment.h> -#include <AppInstallerMsixInfo.h> namespace AppInstaller::CLI::Workflow { @@ -20,7 +20,6 @@ namespace AppInstaller::CLI::Workflow using namespace winrt::Windows::Foundation; using namespace winrt::Windows::Foundation::Collections; using namespace winrt::Windows::Management::Deployment; - using namespace AppInstaller::Utility; using namespace AppInstaller::Manifest; using namespace AppInstaller::Repository; using namespace AppInstaller::Settings; @@ -214,225 +213,6 @@ namespace AppInstaller::CLI::Workflow } } - void DownloadInstaller(Execution::Context& context) - { - const auto& installer = context.Get<Execution::Data::Installer>().value(); - - switch (installer.InstallerType) - { - case InstallerTypeEnum::Exe: - case InstallerTypeEnum::Burn: - case InstallerTypeEnum::Inno: - case InstallerTypeEnum::Msi: - case InstallerTypeEnum::Nullsoft: - case InstallerTypeEnum::Wix: - context << DownloadInstallerFile << VerifyInstallerHash << UpdateInstallerFileMotwIfApplicable; - break; - case InstallerTypeEnum::Msix: - if (installer.SignatureSha256.empty()) - { - context << DownloadInstallerFile << VerifyInstallerHash << UpdateInstallerFileMotwIfApplicable; - } - else - { - // Signature hash provided. No download needed. Just verify signature hash. - context << GetMsixSignatureHash << VerifyInstallerHash << UpdateInstallerFileMotwIfApplicable; - } - break; - case InstallerTypeEnum::MSStore: - // Nothing to do here - break; - default: - THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED)); - } - } - - void DownloadInstallerFile(Execution::Context& context) - { - const auto& manifest = context.Get<Execution::Data::Manifest>(); - const auto& installer = context.Get<Execution::Data::Installer>().value(); - - std::filesystem::path tempInstallerPath = Runtime::GetPathTo(Runtime::PathName::Temp); - tempInstallerPath /= Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version); - - Utility::DownloadInfo downloadInfo{}; - downloadInfo.DisplayName = Resource::GetFixedString(Resource::FixedString::ProductName); - // Use the SHA256 hash of the installer as the identifier for the download - downloadInfo.ContentId = SHA256::ConvertToString(installer.Sha256); - - AICLI_LOG(CLI, Info, << "Generated temp download path: " << tempInstallerPath); - - context.Reporter.Info() << "Downloading " << Execution::UrlEmphasis << installer.Url << std::endl; - - std::optional<std::vector<BYTE>> hash; - - const int MaxRetryCount = 2; - for (int retryCount = 0; retryCount < MaxRetryCount; ++retryCount) - { - bool success = false; - try - { - hash = context.Reporter.ExecuteWithProgress(std::bind(Utility::Download, - installer.Url, - tempInstallerPath, - Utility::DownloadType::Installer, - std::placeholders::_1, - true, - downloadInfo)); - - success = true; - } - catch (...) - { - if (retryCount < MaxRetryCount - 1) - { - AICLI_LOG(CLI, Info, << "Failed to download, waiting a bit and retry. Url: " << installer.Url); - Sleep(500); - } - else - { - throw; - } - } - - if (success) - { - break; - } - } - - if (!hash) - { - context.Reporter.Info() << "Package download canceled." << std::endl; - AICLI_TERMINATE_CONTEXT(E_ABORT); - } - - context.Add<Execution::Data::HashPair>(std::make_pair(installer.Sha256, hash.value())); - context.Add<Execution::Data::InstallerPath>(std::move(tempInstallerPath)); - } - - void GetMsixSignatureHash(Execution::Context& context) - { - // We use this when the server won't support streaming install to swap to download. - bool downloadInstead = false; - - try - { - const auto& installer = context.Get<Execution::Data::Installer>().value(); - - Msix::MsixInfo msixInfo(installer.Url); - auto signature = msixInfo.GetSignature(); - - auto signatureHash = SHA256::ComputeHash(signature.data(), static_cast<uint32_t>(signature.size())); - - context.Add<Execution::Data::HashPair>(std::make_pair(installer.SignatureSha256, signatureHash)); - } - catch (const winrt::hresult_error& e) - { - if (static_cast<HRESULT>(e.code()) == HRESULT_FROM_WIN32(ERROR_NO_RANGES_PROCESSED) || - HRESULT_FACILITY(e.code()) == FACILITY_HTTP) - { - // Failed to get signature hash through HttpStream, use download - downloadInstead = true; - } - else - { - throw; - } - } - - if (downloadInstead) - { - context << DownloadInstallerFile; - } - } - - void VerifyInstallerHash(Execution::Context& context) - { - const auto& hashPair = context.Get<Execution::Data::HashPair>(); - - if (!std::equal( - hashPair.first.begin(), - hashPair.first.end(), - hashPair.second.begin())) - { - bool overrideHashMismatch = context.Args.Contains(Execution::Args::Type::HashOverride); - - const auto& manifest = context.Get<Execution::Data::Manifest>(); - Logging::Telemetry().LogInstallerHashMismatch(manifest.Id, manifest.Version, manifest.Channel, hashPair.first, hashPair.second, overrideHashMismatch); - - // If running as admin, do not allow the user to override the hash failure. - if (Runtime::IsRunningAsAdmin()) - { - context.Reporter.Error() << Resource::String::InstallerHashMismatchAdminBlock << std::endl; - } - else if (overrideHashMismatch) - { - context.Reporter.Warn() << Resource::String::InstallerHashMismatchOverridden << std::endl; - return; - } - else if (Settings::GroupPolicies().IsEnabled(Settings::TogglePolicy::Policy::HashOverride)) - { - context.Reporter.Error() << Resource::String::InstallerHashMismatchOverrideRequired << std::endl; - } - else - { - context.Reporter.Error() << Resource::String::InstallerHashMismatchError << std::endl; - } - - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALLER_HASH_MISMATCH); - } - else - { - AICLI_LOG(CLI, Info, << "Installer hash verified"); - context.Reporter.Info() << Resource::String::InstallerHashVerified << std::endl; - - context.SetFlags(Execution::ContextFlag::InstallerHashMatched); - - if (context.Contains(Execution::Data::PackageVersion) && - context.Get<Execution::Data::PackageVersion>()->GetSource() != nullptr && - WI_IsFlagSet(context.Get<Execution::Data::PackageVersion>()->GetSource()->GetDetails().TrustLevel, SourceTrustLevel::Trusted)) - { - context.SetFlags(Execution::ContextFlag::InstallerTrusted); - } - } - } - - void UpdateInstallerFileMotwIfApplicable(Execution::Context& context) - { - if (context.Contains(Execution::Data::InstallerPath)) - { - if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerTrusted)) - { - Utility::ApplyMotwIfApplicable(context.Get<Execution::Data::InstallerPath>(), URLZONE_TRUSTED); - } - else if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerHashMatched)) - { - const auto& installer = context.Get<Execution::Data::Installer>(); - HRESULT hr = Utility::ApplyMotwUsingIAttachmentExecuteIfApplicable(context.Get<Execution::Data::InstallerPath>(), installer.value().Url, URLZONE_INTERNET); - - // Not using SUCCEEDED(hr) to check since there are cases file is missing after a successful scan - if (hr != S_OK) - { - switch (hr) - { - case INET_E_SECURITY_PROBLEM: - context.Reporter.Error() << Resource::String::InstallerBlockedByPolicy << std::endl; - break; - case E_FAIL: - context.Reporter.Error() << Resource::String::InstallerFailedVirusScan << std::endl; - break; - default: - context.Reporter.Error() << Resource::String::InstallerFailedSecurityCheck << std::endl; - } - - AICLI_LOG(Fail, Error, << "Installer failed security check. Url: " << installer.value().Url << " Result: " << WINGET_OSTREAM_FORMAT_HRESULT(hr)); - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALLER_SECURITY_CHECK_FAILED); - } - } - } - } - void ExecuteInstaller(Execution::Context& context) { const auto& installer = context.Get<Execution::Data::Installer>().value(); @@ -480,7 +260,6 @@ namespace AppInstaller::CLI::Workflow { context << GetInstallerArgs << - RenameDownloadedInstaller << ShellExecuteInstallImpl << ReportInstallerResult("ShellExecute"sv, APPINSTALLER_CLI_ERROR_SHELLEXEC_INSTALL_FAILED); } @@ -489,7 +268,6 @@ namespace AppInstaller::CLI::Workflow { context << GetInstallerArgs << - RenameDownloadedInstaller << DirectMSIInstallImpl << ReportInstallerResult("MsiInstallProduct"sv, APPINSTALLER_CLI_ERROR_MSI_INSTALL_FAILED); } @@ -576,30 +354,6 @@ namespace AppInstaller::CLI::Workflow } } - void RemoveInstaller(Execution::Context& context) - { - // Path may not be present if installed from a URL for MSIX - if (context.Contains(Execution::Data::InstallerPath)) - { - const auto& path = context.Get<Execution::Data::InstallerPath>(); - AICLI_LOG(CLI, Info, << "Removing installer: " << path); - - try - { - // best effort - std::filesystem::remove(path); - } - catch (const std::exception& e) - { - AICLI_LOG(CLI, Warning, << "Failed to remove installer file after execution. Reason: " << e.what()); - } - catch (...) - { - AICLI_LOG(CLI, Warning, << "Failed to remove installer file after execution. Reason unknown."); - } - } - } - void ReportIdentityAndInstallationDisclaimer(Execution::Context& context) { context << @@ -610,8 +364,6 @@ namespace AppInstaller::CLI::Workflow void InstallPackageInstaller(Execution::Context& context) { context << - Workflow::ReportExecutionStage(ExecutionStage::Download) << - Workflow::DownloadInstaller << Workflow::ReportExecutionStage(ExecutionStage::PreExecution) << Workflow::SnapshotARPEntries << Workflow::ReportExecutionStage(ExecutionStage::Execution) << @@ -621,13 +373,20 @@ namespace AppInstaller::CLI::Workflow Workflow::RemoveInstaller; } - void InstallSinglePackage(Execution::Context& context) + void DownloadSinglePackage(Execution::Context& context) { context << Workflow::ReportIdentityAndInstallationDisclaimer << Workflow::ShowPackageAgreements(/* ensureAcceptance */ true) << - Workflow::GetDependenciesFromInstaller << + Workflow::GetDependenciesFromInstaller << Workflow::ReportDependencies(Resource::String::InstallAndUpgradeCommandsReportDependencies) << + Workflow::DownloadInstaller; + } + + void InstallSinglePackage(Execution::Context& context) + { + context << + Workflow::DownloadSinglePackage << Workflow::InstallPackageInstaller; } @@ -670,6 +429,7 @@ namespace AppInstaller::CLI::Workflow installContext << Workflow::ReportIdentityAndInstallationDisclaimer << + Workflow::DownloadInstaller << Workflow::InstallPackageInstaller; installContext.Reporter.Info() << std::endl; diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h @@ -62,36 +62,6 @@ namespace AppInstaller::CLI::Workflow // Composite flow that chooses what to do based on the installer type. // Required Args: None - // Inputs: Manifest, Installer - // Outputs: None - void DownloadInstaller(Execution::Context& context); - - // Downloads the file referenced by the Installer. - // Required Args: None - // Inputs: Installer - // Outputs: HashPair, InstallerPath - void DownloadInstallerFile(Execution::Context& context); - - // Computes the hash of the MSIX signature file. - // Required Args: None - // Inputs: Installer - // Outputs: HashPair - void GetMsixSignatureHash(Execution::Context& context); - - // Gets the source list, filtering it if SourceName is present. - // Required Args: None - // Inputs: HashPair - // Outputs: SourceList - void VerifyInstallerHash(Execution::Context& context); - - // Update Motw of the downloaded installer if applicable - // Required Args: None - // Inputs: HashPair, InstallerPath?, SourceId? - // Outputs: None - void UpdateInstallerFileMotwIfApplicable(Execution::Context& context); - - // Composite flow that chooses what to do based on the installer type. - // Required Args: None // Inputs: Installer, InstallerPath // Outputs: None void ExecuteInstaller(Execution::Context& context); @@ -134,13 +104,6 @@ namespace AppInstaller::CLI::Workflow bool m_isHResult; }; - - // Deletes the installer file. - // Required Args: None - // Inputs: InstallerPath - // Outputs: None - void RemoveInstaller(Execution::Context& context); - // Reports manifest identity and shows installation disclaimer // Required Args: None // Inputs: Manifest @@ -149,11 +112,17 @@ namespace AppInstaller::CLI::Workflow // Installs a specific package installer. See also InstallSinglePackage & InstallMultiplePackages. // Required Args: None - // Inputs: Manifest, Installer, PackageVersion, InstalledPackageVersion? + // Inputs: InstallerPath, Manifest, Installer, PackageVersion, InstalledPackageVersion? // Outputs: None void InstallPackageInstaller(Execution::Context& context); - // Installs a single package. This also does the reporting and user interaction + // Downloads the installer for a single package. This also does all the reporting and user interaction needed. + // Required Args: None + // Inputs: Manifest, Installer + // Outputs: InstallerPath + void DownloadSinglePackage(Execution::Context& context); + + // Installs a single package. This also does the reporting, user interaction, and installer download // for single-package installation. // RequiredArgs: None // Inputs: Manifest, Installer, PackageVersion, InstalledPackageVersion? diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -181,68 +181,6 @@ namespace AppInstaller::CLI::Workflow return args; } - - // Complicated rename algorithm due to somewhat arbitrary failures. - // 1. First, try to rename. - // 2. Then, create an empty file for the target, and attempt to rename. - // 3. Then, try repeatedly for 500ms in case it is a timing thing. - // 4. Attempt to use a hard link if available. - // 5. Copy the file if nothing else has worked so far. - void RenameFile(const std::filesystem::path& from, const std::filesystem::path& to) - { - // 1. First, try to rename. - try - { - // std::filesystem::rename() handles motw correctly if applicable. - std::filesystem::rename(from, to); - return; - } - CATCH_LOG(); - - // 2. Then, create an empty file for the target, and attempt to rename. - // This seems to fix things in certain cases, so we do it. - try - { - { - std::ofstream targetFile{ to }; - } - std::filesystem::rename(from, to); - return; - } - CATCH_LOG(); - - // 3. Then, try repeatedly for 500ms in case it is a timing thing. - for (int i = 0; i < 5; ++i) - { - try - { - std::this_thread::sleep_for(100ms); - std::filesystem::rename(from, to); - return; - } - CATCH_LOG(); - } - - // 4. Attempt to use a hard link if available. - if (Runtime::SupportsHardLinks(from)) - { - try - { - // Create a hard link to the file; the installer will be left in the temp directory afterward - // but it is better to succeed the operation and leave a file around than to fail. - // First we have to remove the target file as the function will not overwrite. - std::filesystem::remove(to); - std::filesystem::create_hard_link(from, to); - return; - } - CATCH_LOG(); - } - - // 5. Copy the file if nothing else has worked so far. - // Create a copy of the file; the installer will be left in the temp directory afterward - // but it is better to succeed the operation and leave a file around than to fail. - std::filesystem::copy_file(from, to, std::filesystem::copy_options::overwrite_existing); - } } void ShellExecuteInstallImpl(Execution::Context& context) @@ -285,31 +223,6 @@ namespace AppInstaller::CLI::Workflow context.Add<Execution::Data::InstallerArgs>(std::move(installerArgs)); } - void RenameDownloadedInstaller(Execution::Context& context) - { - auto& installerPath = context.Get<Execution::Data::InstallerPath>(); - std::filesystem::path renamedDownloadedInstaller(installerPath); - - switch(context.Get<Execution::Data::Installer>()->InstallerType) - { - case InstallerTypeEnum::Burn: - case InstallerTypeEnum::Exe: - case InstallerTypeEnum::Inno: - case InstallerTypeEnum::Nullsoft: - renamedDownloadedInstaller += L".exe"; - break; - case InstallerTypeEnum::Msi: - case InstallerTypeEnum::Wix: - renamedDownloadedInstaller += L".msi"; - break; - } - - RenameFile(installerPath, renamedDownloadedInstaller); - - installerPath.assign(renamedDownloadedInstaller); - AICLI_LOG(CLI, Info, << "Successfully renamed downloaded installer. Path: " << installerPath); - } - void ShellExecuteUninstallImpl(Execution::Context& context) { context.Reporter.Info() << Resource::String::UninstallFlowStartingPackageUninstall << std::endl; diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.h b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.h @@ -34,12 +34,4 @@ namespace AppInstaller::CLI::Workflow // Inputs: Manifest?, Installer, InstallerPath // Outputs: InstallerArgs void GetInstallerArgs(Execution::Context& context); - - // This method appends appropriate extension to the downloaded installer. - // ShellExecute uses file extension to launch the installer appropriately. - // Required Args: None - // Inputs: Installer, InstallerPath - // Modifies: InstallerPath - // Outputs: None - void RenameDownloadedInstaller(Execution::Context& context); } \ No newline at end of file diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp @@ -10,6 +10,7 @@ #include <AppInstallerDownloader.h> #include <AppInstallerStrings.h> #include <Workflows/ImportExportFlow.h> +#include <Workflows/DownloadFlow.h> #include <Workflows/InstallFlow.h> #include <Workflows/MsiInstallFlow.h> #include <Workflows/UninstallFlow.h> @@ -400,8 +401,17 @@ void OverrideForUpdateInstallerMotw(TestContext& context) } }); } +void OverrideForCheckExistingInstaller(TestContext& context) +{ + context.Override({ CheckForExistingInstaller, [](TestContext&) + { + } }); +} + void OverrideForShellExecute(TestContext& context) { + OverrideForCheckExistingInstaller(context); + context.Override({ DownloadInstallerFile, [](TestContext& context) { context.Add<Data::HashPair>({ {}, {} }); @@ -417,6 +427,8 @@ void OverrideForShellExecute(TestContext& context) void OverrideForDirectMsi(TestContext& context) { + OverrideForCheckExistingInstaller(context); + context.Override({ DownloadInstallerFile, [](TestContext& context) { context.Add<Data::HashPair>({ {}, {} }); @@ -684,6 +696,7 @@ TEST_CASE("MsixInstallFlow_StreamingFlow", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; OverrideForMSIX(context); + OverrideForCheckExistingInstaller(context); // Todo: point to files from our repo when the repo goes public context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Msix_StreamingFlow.yaml").GetPath().u8string()); diff --git a/src/AppInstallerCommonCore/Downloader.cpp b/src/AppInstallerCommonCore/Downloader.cpp @@ -275,11 +275,23 @@ namespace AppInstaller::Utility Microsoft::WRL::ComPtr<IZoneIdentifier> zoneIdentifier; THROW_IF_FAILED(CoCreateInstance(CLSID_PersistentZoneIdentifier, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&zoneIdentifier))); - THROW_IF_FAILED(zoneIdentifier->Remove()); Microsoft::WRL::ComPtr<IPersistFile> persistFile; THROW_IF_FAILED(zoneIdentifier.As(&persistFile)); - THROW_IF_FAILED(persistFile->Save(filePath.c_str(), TRUE)); + + auto hr = persistFile->Load(filePath.c_str(), STGM_READ); + if (hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)) + { + // IPersistFile::Load returns same error for "file not found" and "motw not found". + // Check if the file exists to be sure we are on the "motw not found" case. + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), !std::filesystem::exists(filePath)); + + AICLI_LOG(Core, Info, << "File does not contain motw. Skipped removing motw"); + return; + } + + THROW_IF_FAILED(zoneIdentifier->Remove()); + THROW_IF_FAILED(persistFile->Save(NULL, TRUE)); AICLI_LOG(Core, Info, << "Finished removing motw"); }