<!-- Add the related story/sub-task/bug number, like Resolves#123, or
remove if NA -->
**Related issue:** NA
## What & why
Two independent defects in the default MSI scripts. Neither has any
server-side handling, so the script text is the only place either can be
fixed.
### 1. Reboot-required exit codes reported as install failures
The default MSI install script (`pkg/file/scripts/install_msi.ps1`)
ended with `Exit $installProcess.ExitCode`, passing msiexec's raw exit
code straight through. An install that **succeeded but requested a
reboot** therefore reported as a failed install:
- `3010` — `ERROR_SUCCESS_REBOOT_REQUIRED`
- `1641` — `ERROR_SUCCESS_REBOOT_INITIATED`
Both default MSI *uninstall* scripts (`uninstall_msi.ps1` and
`uninstall_msi_with_upgrade_code.ps1`) already carve these out via
`$successCodes = @(0, 3010, 1641)` — install was the only MSI script
missing it. This change adds the same check, using the identical idiom
and comment wording as the uninstall scripts.
### 2. Unquoted log file path in the `/lv` argument
The default MSI install and remove scripts passed the log path unquoted:
```powershell
-ArgumentList "/quiet /norestart /lv ${logFile} /i `"${env:INSTALLER_PATH}`""
```
`Start-Process` appends a single-string `-ArgumentList` to the command
line verbatim — it adds no quoting of its own. `${env:INSTALLER_PATH}`
was already protected by escaped quotes; `${logFile}` was not. So when
`TEMP` contains a space, msiexec tokenizes the path on whitespace: `/lv`
receives only the chunk up to the first space (`C:\Users\John`), and the
remainder (`Smith\AppData\...\fleet-install-software.log`) is left as a
stray token, which msiexec rejects as an invalid command line (`1639`).
The install fails outright rather than merely writing its log somewhere
unexpected.
The fix quotes it the way `${env:INSTALLER_PATH}` already was:
```powershell
-ArgumentList "/quiet /norestart /lv `"${logFile}`" /i `"${env:INSTALLER_PATH}`""
```
**On severity:** this is latent under normal fleetd operation. Install
scripts inherit `os.Environ()` from orbit
(`orbit/pkg/installer/installer.go`), which runs as a LocalSystem
service, so `TEMP` is `C:\Windows\TEMP` — no spaces. It bites when the
system `TEMP` is redirected to a path containing a space, or when an
admin copies the script (Fleet renders it in the UI) and runs it in a
user context whose profile name contains a space. Not reproduced on a
Windows host; the analysis is from msiexec's whitespace tokenizing, not
from an observed failure.
The newer hand-written FMA scripts (`mozilla-vpn_install.ps1`,
`egnyte_install.ps1`, `vnc-server_install.ps1`,
`vnc-viewer_install.ps1`, `agent-ransack_install.ps1`) already used the
quoted form. This brings the older ones in line with them.
## Scope
`GetInstallScript("msi")` feeds two paths, both fixed by change 1:
1. The default install script for **user-uploaded MSI packages**
(`ee/server/service/software_installers.go`).
2. The generated install script for **MSI-based Fleet-maintained apps**
(`ee/maintained-apps/ingesters/winget/ingester.go`).
Change 2 additionally covers `remove_msi.ps1` (the uninstall script used
for packages added before the uninstall feature shipped) and the nine
hand-written winget install scripts that still carried the unquoted
form: `azure-functions-core-tools`, `bluej`, `crisisgo`,
`delinea-connection-manager`, `geogebra-classic`, `google-ads-editor`,
`gotomeeting`, `imageglass`, `sourcetree`.
Notes:
- **FMA outputs are not regenerated here.** `install_script_ref` is
content-addressed, and existing `outputs/*/windows.json` files carry
both the ref and the script text, so they stay internally consistent.
The ingest workflow runs every 4 hours and will roll the refs for
MSI-based apps forward on its own. Regenerating them in this PR would
produce a huge diff and trigger Windows FMA validation for every MSI
app.
- Several per-app FMA install scripts exist **only** to add the
exit-code carve-out and become redundant once this lands (for example
`scribe_install.ps1` from #50341). They are harmless duplicates of the
new default and can be removed in follow-up. Per-app scripts that do
other work too (e.g. `delinea-connection-manager_install.ps1` forcing
`ALLUSERS=1`) still need to keep their own copy — those got the quoting
fix instead.
- Neither change applies to `uninstall_msi.ps1` or
`uninstall_msi_with_upgrade_code.ps1`: they already handle the reboot
codes, and they build `-ArgumentList` as an array with no `/lv` argument
at all.
- `install_exe.ps1` deliberately left alone — EXE installers have no
standard exit-code convention, which is why they use per-app scripts.
- The per-app example scripts embedded in `articles/` (CrowdStrike,
Cloudflare WARP, SentinelOne) are separate copy-paste content and are
not touched.
# Checklist for submitter
- [ ] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.
No changes file is currently in this PR — the earlier one was removed.
Both fixes change user-visible install/uninstall outcomes, so one may be
warranted before merge.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements), JS
inline code is prevented especially for url redirects, and untrusted
data interpolated into shell scripts/commands is validated against shell
metacharacters.
Change 2 is precisely this: a path interpolated into a command line is
now quoted so whitespace can't split it into extra arguments.
## Testing
- [x] Added/updated automated tests
`pkg/file`'s golden test (`TestGetInstallAndRemoveScript`) covers the
script contents; each script and its golden were changed in lockstep, so
they remain byte-identical. `go test ./pkg/file/ -run Script` and `go
test ./ee/server/service/ -run TestGetInstallScript` pass. Goldens can
be regenerated with `go test ./pkg/file/... -update`.
- [ ] QA'd all new/changed functionality manually
Not QA'd on a Windows host. Change 1 needs an MSI that returns 3010
under Fleet's SYSTEM context to confirm the install now reports success.
Change 2 needs an MSI install run with `TEMP` pointed at a path
containing a space.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved MSI installation and removal reliability when log-file paths
contain spaces.
* MSI installations requiring a restart are now recognized as
successful.
* Standard MSI success and restart-required results are handled
consistently while other errors remain available for troubleshooting.
* Updated supported application installers to use the more reliable
logging behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Allen Houchins
51044325e7