Skip to content

fix(build): reliable signtool resolution on clean Windows SDK hosts#1

Merged
rodchristiansen merged 2 commits into
mainfrom
fix/signtool-resolver
Apr 26, 2026
Merged

fix(build): reliable signtool resolution on clean Windows SDK hosts#1
rodchristiansen merged 2 commits into
mainfrom
fix/signtool-resolver

Conversation

@rodchristiansen
Copy link
Copy Markdown
Contributor

Summary

Test-SignTool was looking for the Windows SDK under "$env:ProgramFiles(x86)\Windows Kits\10\bin". That string is a PowerShell parse trap: parentheses cannot terminate an interpolated env-var name, so the string actually expands to "C:\Program Files(x86)\Windows Kits\10\bin" (no space). Test-Path short-circuits the whole search, we fall through to a PATH-only lookup, and on boxes running the deploy pipeline from a pwsh session without the Windows SDK on PATH (Intune runners, fresh build VMs) Test-SignTool silently leaves PATH untouched. Every downstream & signtool.exe … call then fails to launch signtool.

Caught this in production: the Cimian deploy pipeline hit App build failed on both x64 and arm64 during the BootstrapMate.App publish + sign pass, even though MSI signing (which ran earlier in the same session, before an inherited PATH expired) succeeded. Had to manually sign the two App exes and re-run the pipeline.

Fix

  • Resolve signtool via [Environment]::GetFolderPath('ProgramFilesX86'), matching the CimianTools build.ps1 pattern. Handles the (x86) path on every locale and avoids the PowerShell interpolation trap.
  • Also check the 64-bit Program Files root (SDKs installed to the 64-bit path) and the KitsRoot10 registry key.
  • Prefer x64 > arm64 > x86 and sort by SDK version directory descending so we pick the newest SDK on boxes with multiple installs.
  • Cache the resolved absolute path on $script:SignToolPath. Expose Get-SignToolPath so every call site uses the absolute path — no more reliance on PATH being set correctly at child-process spawn time.
  • Still prepend the signtool directory to $env:Path as a belt-and-braces safety for any child process that calls signtool.exe by bare name.
  • Updated every call site that used & signtool.exe … or passed the bare filename to sudo. Nothing calls signtool by bare name anymore.

Test plan

  • pwsh -NoProfile session with a scrubbed PATH that does NOT include the Windows SDK bin\*\x64 directory — signtool is still resolved and every sign step succeeds.
  • x64 build, signing: App exe, MSI, and .intunewin all signed end-to-end.
  • arm64 build, signing: same.
  • No functional change when signtool is already on PATH — fast path through Get-Command signtool.exe is preserved.
  • [System.Management.Automation.Language.Parser]::ParseFile(...) clean on the modified script.

Test-SignTool was looking for the Windows SDK under
  "$env:ProgramFiles(x86)\Windows Kits\10\bin"
which is a PowerShell parse trap: parens cannot terminate an interpolated
env var name, so the string expanded to
  "C:\Program Files(x86)\Windows Kits\10\bin"
(no space). Test-Path short-circuited the whole search and we fell
through to the PATH-only lookup. On boxes that ran the deploy pipeline
from a pwsh session without the Windows SDK on PATH (Intune runners,
fresh build VMs) Test-SignTool silently left PATH untouched, and every
subsequent `& signtool.exe …` call failed to launch. The symptom was
"App build failed" on both x64 and arm64 during the BootstrapMate.App
publish + sign pass, even though MSI signing (which happened earlier
in the same session, before an inherited PATH expired) succeeded.

Fix:
- Resolve signtool via [Environment]::GetFolderPath, matching the
  CimianTools pattern. Handles the (x86) path on every locale and
  avoids the interpolation trap.
- Also check the Program Files root (64-bit SDK installs) and the
  KitsRoot10 registry key as fallbacks.
- Prefer x64 > arm64 > x86 and sort by SDK version descending so we
  pick the newest SDK when multiple are installed.
- Cache the resolved absolute path on $script:SignToolPath. Expose
  Get-SignToolPath so every call site uses the absolute path and
  stops depending on PATH being set correctly at child-process
  spawn time.
- Still prepend the signtool directory to $env:Path as a belt-and-
  braces safety for any child process that calls signtool.exe by
  bare name.

Updated every call site that used `& signtool.exe …` or passed the
bare filename to sudo so they all go through the resolved path:
  * primary sign + fallback TSA loop
  * legacy timestamp append
  * signature verify
  * sudo-elevated sign
  * sudo-signed verify

No functional change when signtool is already on PATH.
Copilot AI review requested due to automatic review settings April 24, 2026 22:54
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves Windows code-signing reliability in build.ps1 by resolving and caching an absolute signtool.exe path (avoiding PATH/env-var interpolation pitfalls) and updating signing call sites to use that absolute path.

Changes:

  • Add Get-SignToolPath and $script:SignToolPath cache; refactor Test-SignTool to populate PATH defensively.
  • Expand Windows SDK discovery (Program Files x86/64 + KitsRoot10) and select newest SDK version with arch preference.
  • Update signing invocations to call the resolved absolute signtool path instead of signtool.exe by bare name.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread build.ps1
Comment on lines 181 to 185
$c = Get-Command signtool.exe -ErrorAction SilentlyContinue
if ($c) {
Write-Log "Found signtool.exe in PATH: $($c.Source)" "SUCCESS"
return
if ($c -and $c.Source -match '\\x64\\') {
$script:SignToolPath = $c.Source
return $script:SignToolPath
}
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

Get-SignToolPath only accepts a PATH-resolved signtool if the resolved path contains \x64\. This regresses the previous behavior where any PATH-resolved signtool.exe was accepted, and will incorrectly return $null on hosts where signtool is available via PATH but the path is not under an x64 directory (e.g., x86/arm64-only installs or non-SDK distributions). Consider accepting any $c.Source from Get-Command (and only using \x64\ as a preference), or falling back to other architectures instead of rejecting the PATH hit outright.

Copilot uses AI. Check for mistakes.
Comment thread build.ps1 Outdated
Write-Log "Running: signtool.exe $($signArgs -join ' ')" "INFO"

& signtool.exe @signArgs
$signtool = Get-SignToolPath
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

Several signing call sites now invoke Get-SignToolPath and then immediately execute & $signtool ... without validating the result. If Test-SignTool hasn’t been called earlier in the session (or if resolution fails), this will attempt to invoke a null/empty command and produce a confusing error. Consider calling Test-SignTool at the start of Invoke-SignArtifact (and/or throwing a clear error if Get-SignToolPath returns $null) so the failure mode is deterministic and actionable.

Suggested change
$signtool = Get-SignToolPath
$signtool = Get-SignToolPath
if ([string]::IsNullOrWhiteSpace($signtool)) {
throw "SignTool could not be resolved. Ensure the Windows SDK SignTool is installed and available, or run Test-SignTool before invoking signing."
}

Copilot uses AI. Check for mistakes.
Comment thread build.ps1 Outdated
Comment on lines +593 to +616
@@ -593,7 +613,7 @@ function Invoke-SignArtifact {

# Verify the signature
Write-Log "Verifying sudo-signed signature..." "INFO"
& signtool.exe verify /pa "$Path"
& (Get-SignToolPath) verify /pa "$Path"
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

In the sudo signing branch, Get-SignToolPath is invoked multiple times (building $sudoArgs, then again for verify). Even with caching, this repeats work and risks inconsistencies if the cache becomes invalidated mid-run. Consider resolving once into a local $signtool variable and using it for the log message, $sudoArgs, and verification.

Copilot uses AI. Check for mistakes.
Addresses Copilot review feedback on the signtool resolver.

PATH regression. Get-SignToolPath rejected a PATH hit unless its
location matched \x64\, which silently broke hosts where signtool was
on PATH but lived in an arm64- or x86-only SDK install (or a
non-default location). Restore the previous behaviour: take any PATH
hit, but only short-circuit when it's the preferred x64 build. If the
hit isn't x64, run the SDK search anyway so we can upgrade to a better
candidate, and fall back to the PATH hit if nothing better exists.

Null-result handling. Several signing call sites called
Get-SignToolPath inline as the executable name (`& (Get-SignToolPath)
...` or interpolated into $sudoArgs). When resolution failed they
produced a confusing "&: cannot be invoked on null" error instead of
something actionable. Add Resolve-SignToolPath that throws a clear
"install the Windows SDK Signing Tools" message, and switch every
signing call site to it.

Sudo-branch dedup. The sudo-elevated signing branch was calling
Get-SignToolPath three times — once for the log line, once into
$sudoArgs, once for verify. Resolve once into $signtool and reuse
across all three so the cache can never be invalidated mid-run.
@rodchristiansen rodchristiansen merged commit e346355 into main Apr 26, 2026
3 checks passed
@rodchristiansen rodchristiansen deleted the fix/signtool-resolver branch April 26, 2026 20:25
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.

2 participants