Skip to content

Ensure MOTW failures are called out in logs#6127

Open
Trenly wants to merge 5 commits intomicrosoft:masterfrom
Trenly:Downloader
Open

Ensure MOTW failures are called out in logs#6127
Trenly wants to merge 5 commits intomicrosoft:masterfrom
Trenly:Downloader

Conversation

@Trenly
Copy link
Copy Markdown
Contributor

@Trenly Trenly commented Apr 6, 2026

Fix MOTW security check failures for downloaded installers

Problem

All installer downloads were failing the security check with 0x80070003 (ERROR_PATH_NOT_FOUND), causing every install to terminate with "Installer failed security check".

Two root causes were identified:

  • Wrong file scanned by IAttachmentExecute — UpdateInstallerFileMotwIfApplicable ran before RenameDownloadedInstaller in the workflow. This meant the Attachment Execution Service (AES) was scanning the
    temporary pre-hash-validation file (a raw SHA256 hex string with no extension, e.g. aab2dc8e…) rather than the final installer (e.g. BadlionClient.exe).

Important

Shell32's AES relies on file extension for MIME-type detection, scan policy, and zone assignment, so scanning an extensionless file produced unreliable results

  • RemoveMotwIfApplicable failures aborted the security check — If removing the existing Zone.Identifier ADS failed, the exception propagated through the AES thread, leaving IAttachmentExecute::Save() never
    called and the whole check returning a failure code. Additionally, a missing THROW_IF_FAILED(hr) after IPersistFile::Load meant unexpected load errors were silently ignored, leading to Remove()/Save()
    being called on a non-loaded object.

Changes

DownloadFlow.cpp

  • Reorder workflow: RenameDownloadedInstaller now runs before UpdateInstallerFileMotwIfApplicable, so AES always scans the properly-named installer file.

Downloader.cpp — RemoveMotwIfApplicable

  • Also handle ERROR_PATH_NOT_FOUND (in addition to ERROR_FILE_NOT_FOUND) from IPersistFile::Load — different Windows builds return different codes when no Zone.Identifier ADS is present.
  • Restore missing THROW_IF_FAILED(hr) to correctly surface any other Load failures rather than falling through to Remove()/Save() on an unloaded object.

Downloader.cpp — ApplyMotwUsingIAttachmentExecuteIfApplicable

  • Wrap RemoveMotwIfApplicable in a try/catch inside the updateMotw lambda — log a warning but proceed to IAttachmentExecute::Save(). MOTW removal is best-effort; a removal failure should not abort the
    security scan.
  • Wrap the entire AES thread body in try/catch and log exceptions via AICLI_LOG, so unhandled errors are surfaced rather than silently leaving hr in an indeterminate state. Use LOG_IF_FAILED for
    CoInitializeEx so failures are captured through WIL.
  • Fix the return value: previously aesSaveResult was always returned, reporting S_OK when Save() was never reached (e.g. after CoInitializeEx failure). Now returns the thread's hr when aesSaveResult was
    never updated

Microsoft Reviewers: Open in CodeFlow

@Trenly Trenly marked this pull request as ready for review April 6, 2026 21:25
@Trenly Trenly requested a review from a team as a code owner April 6, 2026 21:25
@Trenly Trenly changed the title Surface errors instead of swallowing Ensure MOTW failures are called out in logs Apr 6, 2026
Comment on lines +498 to +503
auto hrRemove = zoneIdentifier->Remove();
if (FAILED(hrRemove))
{
AICLI_LOG(Core, Error, << "IZoneIdentifier::Remove failed. Result: " << hrRemove);
THROW_IF_FAILED(hrRemove);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary, the WIL logging infrastructure will take care of this enough that we won't have lost any information.

Same with Save call below and other places in the file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JohnMcPMS - Believe I've resolved in the latest commit

@github-actions

This comment has been minimized.

@Dragon1573
Copy link
Copy Markdown

Sorry to ask here. 🙏🏼

Is there any unknown reason that blocks this PR? A new preview version of winget.exe was released but I still unable to install packages from local manifests ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants