diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp index 1745ab7539..3339ace469 100644 --- a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp @@ -338,8 +338,8 @@ namespace AppInstaller::CLI::Workflow context << VerifyInstallerHash << - UpdateInstallerFileMotwIfApplicable << - RenameDownloadedInstaller; + RenameDownloadedInstaller << + UpdateInstallerFileMotwIfApplicable; if (installerDownloadOnly) { diff --git a/src/AppInstallerCommonCore/Downloader.cpp b/src/AppInstallerCommonCore/Downloader.cpp index a073f59861..6b73cc06cb 100644 --- a/src/AppInstallerCommonCore/Downloader.cpp +++ b/src/AppInstallerCommonCore/Downloader.cpp @@ -481,9 +481,11 @@ namespace AppInstaller::Utility THROW_IF_FAILED(zoneIdentifier.As(&persistFile)); auto hr = persistFile->Load(filePath.c_str(), STGM_READ); - if (hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)) + if (hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) || + hr == HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND)) { - // IPersistFile::Load returns same error for "file not found" and "motw not found". + // IPersistFile::Load can return ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND for + // both "file not found" and "motw not found" depending on the Windows build. // 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)); @@ -491,6 +493,8 @@ namespace AppInstaller::Utility return; } + THROW_IF_FAILED(hr); + THROW_IF_FAILED(zoneIdentifier->Remove()); THROW_IF_FAILED(persistFile->Save(NULL, TRUE)); @@ -516,8 +520,17 @@ namespace AppInstaller::Utility RETURN_IF_FAILED(attachmentExecute->SetLocalPath(filePath.c_str())); RETURN_IF_FAILED(attachmentExecute->SetSource(Utility::ConvertToUTF16(source).c_str())); - // IAttachmentExecute::Save() expects the local file to be clean(i.e. it won't clear existing motw if it thinks the source url is trusted) - RemoveMotwIfApplicable(filePath); + // IAttachmentExecute::Save() expects the local file to be clean (i.e. it won't clear existing motw if it thinks the source url is trusted). + // If removal fails for any reason, log a warning and proceed — a removal failure should not abort the security check. + try + { + RemoveMotwIfApplicable(filePath); + } + catch (...) + { + HRESULT hrRemoveMotw = wil::ResultFromCaughtException(); + AICLI_LOG(Core, Warning, << "RemoveMotwIfApplicable failed before IAttachmentExecute::Save(). Result: " << hrRemoveMotw << ". Proceeding with Save()"); + } aesSaveResult = attachmentExecute->Save(); @@ -536,21 +549,31 @@ namespace AppInstaller::Utility std::thread aesThread([&]() { - hr = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); - if (FAILED(hr)) + try { - return; - } + hr = LOG_IF_FAILED(CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED)); + if (FAILED(hr)) + { + return; + } - hr = updateMotw(); - CoUninitialize(); + hr = updateMotw(); + CoUninitialize(); + } + catch (...) + { + hr = wil::ResultFromCaughtException(); + AICLI_LOG(Core, Error, << "Exception in IAttachmentExecute thread. Result: " << hr); + } }); aesThread.join(); AICLI_LOG(Core, Info, << "Finished applying motw using IAttachmentExecute. Result: " << hr << " IAttachmentExecute::Save() result: " << aesSaveResult); - return aesSaveResult; + // Return the thread's hr when aesSaveResult was never updated (e.g. CoInitializeEx failure or exception + // before IAttachmentExecute::Save() was called), so the caller sees the real failure instead of S_OK. + return (FAILED(hr) && aesSaveResult == S_OK) ? hr : aesSaveResult; } Microsoft::WRL::ComPtr GetReadOnlyStreamFromURI(std::string_view uriStr)