Treat MSI reboot-required exit codes as success and quote the msiexec log path - #50407
Merged
Merged
Conversation
…ript The default MSI install script exited with msiexec's raw exit code, so an install that succeeded but requested a reboot (3010 = ERROR_SUCCESS_REBOOT_REQUIRED, 1641 = ERROR_SUCCESS_REBOOT_INITIATED) was reported as a failed install. Both default MSI uninstall scripts already treat these codes as success, so install was the outlier. Add the same $successCodes carve-out used by the uninstall scripts. This covers both user-uploaded MSI packages and the install script generated for MSI-based Fleet-maintained apps, removing the need for per-app scripts that exist only to add this check.
allenhouchins
temporarily deployed
to
Docker Hub
August 3, 2026 01:55 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR updates Fleet’s default Windows MSI install PowerShell script to treat “successful but reboot required” MSI exit codes (3010, 1641) as success, matching the behavior already present in the default MSI uninstall scripts. This prevents installs that completed correctly (but requested/initiated a reboot) from being reported as failures.
Changes:
- Add
$successCodes = @(0, 3010, 1641)to the default MSI install script and exit0when msiexec returns a reboot-required success code. - Update the corresponding golden test fixture to reflect the new script contents.
- Add a changes entry (content excluded from diff by policy).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/file/scripts/install_msi.ps1 | Treat MSI reboot-required “success” exit codes (3010/1641) as success by exiting 0 instead of propagating the raw msiexec exit code. |
| pkg/file/testdata/scripts/install_msi.ps1.golden | Update golden script output to match the new install script behavior and comments. |
| changes/msi-install-reboot-required-exit-codes | Release note entry (not reviewable here due to content exclusion policy). |
Files excluded by content exclusion policy (1)
- changes/msi-install-reboot-required-exit-codes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
WalkthroughMSI installation scripts now treat exit codes 0, 3010, and 1641 as successful and return process exit code 0. Other exit codes remain unchanged. MSI installation and removal commands now quote temporary log-file paths passed with Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #50407 +/- ##
==========================================
- Coverage 68.20% 68.20% -0.01%
==========================================
Files 3941 3941
Lines 251274 251274
Branches 13442 13442
==========================================
- Hits 171389 171384 -5
- Misses 64533 64538 +5
Partials 15352 15352
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
allenhouchins
temporarily deployed
to
Docker Hub
August 3, 2026 04:00 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
The /lv argument passed the log path unquoted. Start-Process appends a
single-string -ArgumentList to the command line verbatim, so when TEMP
contains a space msiexec tokenizes the path: /lv gets the chunk up to the
first space and the remainder is left as a stray token, which msiexec
rejects as an invalid command line. The install/uninstall fails outright
rather than just misplacing the log.
Quote it the same way ${env:INSTALLER_PATH} already is in the same string.
Covers the embedded install/remove MSI scripts (and their goldens) plus the
nine hand-written winget install scripts that predate the quoted form used
by the newer ones. The inline copies in outputs/*/windows.json pick this up
on the next regeneration.
allenhouchins
temporarily deployed
to
Docker Hub
August 3, 2026 05:32 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ee/maintained-apps/inputs/winget/scripts/bluej_install.ps1 (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required PowerShell file encoding.
PSScriptAnalyzer reports
PSUseBOMForUnicodeEncodedFilefor this file. If Windows PowerShell encoding compatibility is required, savebluej_install.ps1as UTF-8 with BOM, or configure the repository to permit the current encoding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/maintained-apps/inputs/winget/scripts/bluej_install.ps1` at line 10, Save bluej_install.ps1 using UTF-8 with a BOM to satisfy the PSUseBOMForUnicodeEncodedFile requirement and preserve Windows PowerShell compatibility; do not alter the installer argument logic.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ee/maintained-apps/inputs/winget/scripts/bluej_install.ps1`:
- Line 10: Save bluej_install.ps1 using UTF-8 with a BOM to satisfy the
PSUseBOMForUnicodeEncodedFile requirement and preserve Windows PowerShell
compatibility; do not alter the installer argument logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2359610f-c38f-4287-bb95-66ade21fdff6
📒 Files selected for processing (13)
ee/maintained-apps/inputs/winget/scripts/azure-functions-core-tools_install.ps1ee/maintained-apps/inputs/winget/scripts/bluej_install.ps1ee/maintained-apps/inputs/winget/scripts/crisisgo_install.ps1ee/maintained-apps/inputs/winget/scripts/delinea-connection-manager_install.ps1ee/maintained-apps/inputs/winget/scripts/geogebra-classic_install.ps1ee/maintained-apps/inputs/winget/scripts/google-ads-editor_install.ps1ee/maintained-apps/inputs/winget/scripts/gotomeeting_install.ps1ee/maintained-apps/inputs/winget/scripts/imageglass_install.ps1ee/maintained-apps/inputs/winget/scripts/sourcetree_install.ps1pkg/file/scripts/install_msi.ps1pkg/file/scripts/remove_msi.ps1pkg/file/testdata/scripts/install_msi.ps1.goldenpkg/file/testdata/scripts/remove_msi.ps1.golden
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/file/testdata/scripts/install_msi.ps1.golden
- pkg/file/scripts/install_msi.ps1
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (4)
ee/maintained-apps/inputs/winget/scripts/sourcetree_install.ps1:7
- This per-app MSI install script still exits with msiexec’s raw exit code. If the installer succeeds but returns 3010 (reboot required) or 1641 (reboot initiated), Fleet will treat it as a failed install. Consider using the same $successCodes carve-out as the default MSI install script so reboot-required installs report success.
$installProcess = Start-Process msiexec.exe `
-ArgumentList "/quiet /norestart /lv `"${logFile}`" /i `"${env:INSTALLER_PATH}`" ACCEPTEULA=1" `
-PassThru -Verb RunAs -Wait
ee/maintained-apps/inputs/winget/scripts/gotomeeting_install.ps1:12
- This per-app MSI install script still exits with msiexec’s raw exit code. If the installer succeeds but returns 3010 (reboot required) or 1641 (reboot initiated), Fleet will treat it as a failed install. Consider using the same $successCodes carve-out as the default MSI install script so reboot-required installs report success.
$installProcess = Start-Process msiexec.exe `
-ArgumentList "/quiet /norestart /lv `"${logFile}`" /i `"${env:INSTALLER_PATH}`" G2MINSTALLFORALLUSERS=1" `
-PassThru -Verb RunAs -Wait
ee/maintained-apps/inputs/winget/scripts/bluej_install.ps1:11
- This per-app MSI install script still exits with msiexec’s raw exit code. If the installer succeeds but returns 3010 (reboot required) or 1641 (reboot initiated), Fleet will treat it as a failed install. Consider using the same $successCodes carve-out as the default MSI install script so reboot-required installs report success.
$installProcess = Start-Process msiexec.exe `
-ArgumentList "/quiet /norestart /lv `"${logFile}`" /i `"${env:INSTALLER_PATH}`" ALLUSERS=2" `
-PassThru -Verb RunAs -Wait
pkg/file/scripts/remove_msi.ps1:5
- This MSI remove script still exits with msiexec’s raw exit code. Like the other MSI uninstall scripts, it should treat 3010 (reboot required) and 1641 (reboot initiated) as successful outcomes; otherwise a successful uninstall that requests reboot will be reported as a failure.
$removeProcess = Start-Process msiexec.exe `
-ArgumentList "/quiet /norestart /lv `"${logFile}`" /x `"${env:INSTALLER_PATH}`"" `
-PassThru -Verb RunAs -Wait
tux234
approved these changes
Aug 3, 2026
tux234
left a comment
tux234
left a comment
Contributor
There was a problem hiding this comment.
Took a look and it looks good to me. Good find!
This was referenced Aug 3, 2026
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 withExit $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_REQUIRED1641—ERROR_SUCCESS_REBOOT_INITIATEDBoth default MSI uninstall scripts (
uninstall_msi.ps1anduninstall_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
/lvargumentThe default MSI install and remove scripts passed the log path unquoted:
Start-Processappends a single-string-ArgumentListto the command line verbatim — it adds no quoting of its own.${env:INSTALLER_PATH}was already protected by escaped quotes;${logFile}was not. So whenTEMPcontains a space, msiexec tokenizes the path on whitespace:/lvreceives 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: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, soTEMPisC:\Windows\TEMP— no spaces. It bites when the systemTEMPis 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:ee/server/service/software_installers.go).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:
install_script_refis content-addressed, and existingoutputs/*/windows.jsonfiles 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.scribe_install.ps1from Add Scribe as a Windows Fleet-maintained app #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.ps1forcingALLUSERS=1) still need to keep their own copy — those got the quoting fix instead.uninstall_msi.ps1oruninstall_msi_with_upgrade_code.ps1: they already handle the reboot codes, and they build-ArgumentListas an array with no/lvargument at all.install_exe.ps1deliberately left alone — EXE installers have no standard exit-code convention, which is why they use per-app scripts.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/oree/fleetd-chrome/changes.See 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.
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
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 Scriptandgo test ./ee/server/service/ -run TestGetInstallScriptpass. Goldens can be regenerated withgo test ./pkg/file/... -update.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
TEMPpointed at a path containing a space.Summary by CodeRabbit