Skip to content

Verify build tools are Microsoft-signed before running them - #738

Merged
Nikola Metulev (nmetulev) merged 8 commits into
mainfrom
azchohfi-verify-downloaded-tool-signatures
Aug 12, 2026
Merged

Nikola Metulev (nmetulev) merged 8 commits into
mainfrom
azchohfi-verify-downloaded-tool-signatures

Conversation

@azchohfi

Copy link
Copy Markdown
Collaborator

What

The CLI downloads the Windows SDK build tools over the network and then executes them. winapp tool <name> goes further and lets the caller name any executable in that package:

var toolName = args[0];
var toolPath = await buildToolsService.EnsureBuildToolAvailableAsync(toolName, ...);

Nothing checked those binaries before launching them. AuthenticodeVerifier.IsTrustedMicrosoftSigned already existed for the WinDbg provider acquisition path but was never wired here.

How

Gate the tool at the point of use, in EnsureBuildToolAvailableAsync. That covers winapp package, winapp sign, and the arbitrary-name winapp tool path in one place.

RunBuildToolAsync is gated too, because a caller can pass toolPathOverride and bypass resolution entirely — AzureSignToolService does exactly that when it swaps to an architecture-matched signtool after resolving. Verifying only at resolution would have left that path unchecked.

Results are memoized on path | length | lastWriteTimeUtc, so a packaging run verifies makeappx.exe once rather than on every invocation, while a binary replaced on disk is re-verified rather than inheriting the old verdict.

Why at the point of use, and not over the whole package

Verifying every binary at download time looks stricter but is not viable. Microsoft.Windows.SDK.BuildTools 10.0.19041.1 ships 19 PE files with no Authenticode signature:

bin\10.0.19041.0\x64\PackageEditor.exe
bin\10.0.19041.0\x64\ComparePackage.exe
bin\10.0.19041.0\x86\WinAppDeployCmd.exe
bin\10.0.19041.0\x86\SirepClient.dll        (+14 more)

That is the original 2020 release; 10.0.22000.194 and every version since is clean, including current 10.0.28000.2526. Confirmed with both Get-AuthenticodeSignature and signtool verify /pa (which also consults catalogs), against a fresh download from nuget.org rather than a local cache, with a signed binary in the same directory as a control.

Package-wide verification would therefore break that release outright. Checking at the point of use keeps it working — the four tools the CLI actually runs are signed in every published version, including 19041.1 — while still refusing anything unsigned, and needs no allowlist to drift as new tools are invoked.

Error reporting

Signature failures throw BuildToolSignatureException (deriving from InvalidOperationException, so existing handlers still work). Without it, ToolCommand's existing catch (InvalidOperationException) prefixed the failure with "Could not install or find Windows SDK Build Tools", which is wrong — the tool was found, it was refused. Runtime testing caught that; the unit tests did not.

Validation

Unit tests, BuildToolsSignatureVerificationTests: signed passes, unsigned throws, the message names both versions, memoization verifies once, a replaced binary re-verifies, and one test asserts the production default really is AuthenticodeVerifier rather than a permanently-open gate.

Runtime, against a published Release build rather than dotnet run:

# unsigned tool in an isolated cache -> refused
> winapp tool mt
[ERROR] - 'mt.exe' is not validly signed by Microsoft, so it was not run (...\10.0.19041.1\...\mt.exe).
Several auxiliary binaries in Microsoft.Windows.SDK.BuildTools 10.0.19041.1 shipped without a
signature; pin 10.0.22000.194 or newer in winapp.yaml to use them.
exit=1

# real signed tool -> runs
> winapp tool makeappx
Microsoft (R) MakeAppx Tool
Version 10.0.28000.2526

makeappx with no arguments exits 1 on its own — verified by invoking makeappx.exe directly — so that is its usage error passed through, not the gate.

Full suite: 4560 tests, 0 failed.

Note on the test seam

SignatureVerifier is a static seam, matching WinDbgJsProviderAcquirer. Fixtures across the suite stand in dummy unsigned files for real SDK binaries, so the gate is opened once in GlobalTestSetup.AssemblyInitialize. Setting it per test races, because the assembly runs Parallelize(Scope = MethodLevel) — the class that drives the gate directly is [DoNotParallelize], consistent with how NugetServiceOfflineTests and WinDbgJsProviderAcquirerTests handle their own static seams.

The CLI downloads Windows SDK build tools over the network and then executes
them, and winapp tool lets the caller name any executable in that package, so
an unsigned or tampered binary in the NuGet cache would be launched without
challenge. AuthenticodeVerifier already existed for the WinDbg provider but was
never wired to this path.

Gate the tool at the point of use in EnsureBuildToolAvailableAsync, and again in
RunBuildToolAsync because callers may supply a toolPathOverride that never went
through resolution -- AzureSignToolService does exactly that when it swaps to an
architecture-matched signtool. Verification is memoized on path, length and last
write time, so a packaging run pays for it once per binary while a file replaced
on disk is still re-checked.

Verifying every binary in the package was not viable: Microsoft.Windows.SDK.
BuildTools 10.0.19041.1 ships 19 unsigned auxiliaries (PackageEditor.exe,
WinAppDeployCmd.exe and others). Later releases are clean, and the tools the CLI
actually runs are signed in every published version, so checking at the point of
use keeps that release working while still refusing anything unsigned.

Signature failures throw BuildToolSignatureException rather than a plain
InvalidOperationException so ToolCommand can say the tool was refused instead of
claiming it could not be found.
Copilot AI balanced review requested due to automatic review settings August 12, 2026 02:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds Microsoft Authenticode verification before executing downloaded Windows SDK build tools.

Changes:

  • Verifies resolved and overridden tool paths.
  • Memoizes verification results by file metadata.
  • Adds targeted error handling and verification tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
BuildToolsService.cs Adds signature verification and caching.
BuildToolSignatureException.cs Defines signature-refusal exception.
ToolCommand.cs Reports signature failures accurately.
GlobalTestSetup.cs Configures the test verification seam.
BuildToolsSignatureVerificationTests.cs Tests verification behavior and caching.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/winapp-CLI/WinApp.Cli/Services/BuildToolsService.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Commands/ToolCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli.Tests/BuildToolsSignatureVerificationTests.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Services/BuildToolsService.cs Outdated
@github-actions

github-actions Bot commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Build Metrics Report

Binary Sizes

Artifact Baseline Current Delta
CLI (ARM64) 38.62 MB 38.62 MB 📈 +1.5 KB (+0.00%)
CLI (x64) 38.73 MB 38.73 MB 📈 +1.5 KB (+0.00%)
MSIX (ARM64) N/A 16.02 MB N/A
MSIX (x64) N/A 17.01 MB N/A
NPM Package N/A 33.42 MB N/A
NuGet Package N/A 33.46 MB N/A

Test Results

✅ 4562 passed, 5 skipped out of 4567 tests in 614.1s (+8 tests, -104.4s vs. baseline)

Test Coverage

✅ 89.1% line coverage, 82.4% branch coverage · ✅ no change vs. baseline

CLI Startup Time

48ms median (x64, winapp --version) · ⚠️ +14ms vs. baseline

Try This Build

Installs the MSIX for your architecture, replacing any previously installed build. Needs the GitHub CLI — the command offers to install it and sign you in if it is missing.

& ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) 738
Switching between builds often?

Put the tool on your PATH once:

& ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPath

Then this build is just:

winapp-pr 738

Run winapp-pr with no arguments to pick from a list of open PRs.


Updated 2026-08-12 18:54:52 UTC · commit bdd8bfd · workflow run

The cache was keyed on path, length and last-write time. All three are
reproducible by whoever can replace the file, so a "signed" verdict could be
inherited by different bytes -- which is precisely the guarantee the gate
exists to make. Verification is cheap relative to launching a process, so it
now runs on every resolution.

Also addresses the rest of the review:

- Assert the production verifier captured before the suite opens the gate,
  rather than one the test assigns itself.
- Cover the BuildToolSignatureException branch in ToolCommandTests.
- Document the refusal and the 10.0.22000.194 remedy under `winapp tool`.
- Path.Join over Path.Combine in the new test fixture.
10.0.19041.1 is unlisted on NuGet, so a normal restore never selects it and
telling people to pin away from it is noise. The advice was also wrong as a
floor: 10.0.20348.19 and 10.0.18362.3-preview are both listed and older than
the 10.0.22000.194 the message recommended.

The message now says what a failure actually means -- the file on disk is not
what Microsoft published -- and gives the one useful action: clear the NuGet
cache entry and re-download.
Unlisting does not break an existing pin, so a winapp.yaml that pinned
10.0.19041.1 before it was pulled still restores it. That reader is the only
one who needs the version named -- and the generic "re-download it" advice is
wrong for them, because the package really does ship unsigned binaries and no
amount of re-downloading changes that.

Generic cause now leads; the pin is called out as the exception where the
remedy is a newer version rather than a fresh download.
The version is unlisted and reaching it needs a pin predating its removal, so
naming it in current docs is clutter for a case effectively nobody hits. The
generic guidance stands on its own.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants