Allen Houchins
51044325e7
<!-- 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 -->
File scripts
This folder contains scripts to install/remove software for different types of installers.
Scripts are stored on their own files for two reasons:
- Some of them are read and displayed in the UI.
- It's helpful to have good syntax highlighting and easy ways to run them.
Scripts
install_*.*: Default installer scripts for each platform.uninstall_*.*: Default uinstaller scripts for each platform.remove_*.*: Uninstaller scripts used when the uninstall script is not set (for packages added before the uninstall feature was released) or empty uninstaller scripts.
Variables
The scripts in this folder accept variables like $VAR_NAME that will be replaced/populated by fleetd when they run.
Supported variables are:
$INSTALLER_PATHpath to the installer file.