Validate installer functionality and error handling - #92
AmirMS (AmelBawa-msft) wants to merge 1 commit into
Conversation
ranm-msft
left a comment
There was a problem hiding this comment.
The Authenticode and publisher check is the right addition, and failing closed with the file removed is the right shape.
One gap it does not close: this resource declares securityContext: elevated, but $env:TEMP\vs_BuildTools.exe is a fixed, predictable path. In the normal split-token local-admin case the elevated process runs as the same account and gets that account's per-user TEMP, so a medium-integrity process under the same user can pre-create that exact path. Invoke-WebRequest overwriting an existing file does not establish that the pathname is exclusively owned by the elevated process, so Get-AuthenticodeSignature and Start-Process are not guaranteed to be looking at the same bytes.
Would you consider staging into a directory the elevated process creates itself with an admin-only ACL, rejecting a pre-existing or reparse-point target, so the verified file cannot be swapped before launch? A random subdirectory name on its own only reduces predictability - the ACL is what makes the check meaningful through to execution.
To be fair, that staging shape predates this PR and these three lines do not introduce it. Raising it because this change is what turns the verification into a stated guarantee.
No description provided.