<!-- 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 -->
<!-- Add the related story/sub-task/bug number, like Resolves#123, or
remove if NA -->
**Related issue:** Resolves#48102
Changes:
- Renames `ExtractIPAMetadata` to `ExtractZIPMetadata` because the magic
bytes for zip based installers (.ipa, .msix, .zip, etc) are the same so
any zip file reaches it. If the zip does not contain an `Info.plist`
file it will now fail with `ErrInvalidType`.
- Did **NOT** make typeFromBytes return "zip" instead of "ipa" because
meta.Extension is set from that which has downstream effects.
- Added test files
The actual error message is still just "invalid file type".
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
- [x] 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.
- [ ] 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.
- [ ] Timeouts are implemented and retries are limited to avoid infinite
loops
- [ ] If paths of existing endpoints are modified without backwards
compatibility, checked the frontend/CLI for any necessary changes
## Testing
- [x] Added/updated automated tests
- [ ] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)
- [x] QA'd all new/changed functionality manually
- Tested adding a valid `.ipa`, a macos FMA that uses a .zip file
(alt-tab/darwin), and a windows FMA that uses a .zip file
(vnc-server/windows).
- Tested an msix file (renamed or not) cannot be uploaded or edited for
an existing msi installer
- Also tested the same things via GitOps
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved installer type detection so ZIP-based packages are less
likely to be misidentified.
* Fixed an error message that incorrectly referred to the wrong file
type when detection fails.
* MSIX packages are now reported more accurately when they don’t match
IPA parsing rules.
* **Refactor**
* Cleaned up installer metadata handling for ZIP-based archives.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This pull request updates the uninstall scripts to treat additional MSI
exit codes that indicate a successful uninstall (but may require a
reboot) as success, rather than failure. This improves the robustness of
the uninstall process by not incorrectly flagging these scenarios as
errors.
**Improvements to exit code handling in uninstall scripts:**
* Added support for treating MSI exit codes `3010`
(ERROR_SUCCESS_REBOOT_REQUIRED) and `1641`
(ERROR_SUCCESS_REBOOT_INITIATED) as success, in addition to `0`, in both
`uninstall_msi.ps1` and `uninstall_msi_with_upgrade_code.ps1`. This is
achieved by introducing a `$successCodes` array and updating the exit
code checks to use it.
[[1]](diffhunk://#diff-09e225a2a28fbf997ddf571274119a20d9210539e5bdd49749beb2226e6de5aaR15-R20)
[[2]](diffhunk://#diff-c24faec992d742fed7d16c8621f140f7048ecb2cc88bd135fcf02cbd8653f77bR5-R8)
[[3]](diffhunk://#diff-c24faec992d742fed7d16c8621f140f7048ecb2cc88bd135fcf02cbd8653f77bL17-R21)
**Test updates:**
* Updated the golden test data for `uninstall_msi.ps1` to reflect the
new logic for handling successful exit codes.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved MSI uninstall handling to recognize additional success
conditions, including scenarios requiring system restart, enhancing the
reliability of software removal operations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
**Related issue:** Resolves#32084
This PR modifies `isValidAppFilePath` to allow subdirectors in
`Applications/`, like in this case `Applications/Cisco/Cisco Secure
Client.app`.
This also changes the metadata extraction from packageinfo to trim
`.app` from the name in all cases.
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
- [x] 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.
- [ ] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [ ] If paths of existing endpoints are modified without backwards
compatibility, checked the frontend/CLI for any necessary changes
## Testing
- [x] Added/updated automated tests
- [ ] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)
- [x] QA'd all new/changed functionality manually
### Test plan:
---
I ran this on my local environment and it seemed fine
- Have environment with the bug recreated, it has two software titles
for "Cisco Secure Client", and the one with the bundle id
`com.cisco.pkg.anyconnect.vpn` is used by the installer.
- URL to pkg:
https://fndtnfleetmsp.blob.core.windows.net/fndtnpkgs/cisco-secure-client-macos-5.1.3.62-core-vpn-webdeploy-k9.pkg
- Cisco Secure Client doesn't show as installed in UI even after
installing.
- Run the new migration.
- Cisco Secure Client shows as installed now in ui, software title with
bundle id `com.cisco.pkg.anyconnect.vpn` is gone from the database, and
the software installer references the correct title
(`com.cisco.secureclient.gui`).
- Check that deleting and reuploading the installer doesn't recreate the
bad software title.
### QA Note:
---
There are some problems with the install script, but that is probably a
different scope than this ticket.
`Reinstall` wont work, it says Cisco Secure Client is already installed.
Uninstalling through Fleet then Installing again works fine though.
**Related issue:** Resolves#32083
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
- [x] 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.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
## Testing
- [x] Added/updated automated tests
- [x] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)
- [x] QA'd all new/changed functionality manually
## Database migrations
- [x] Checked table schema to confirm autoupdate
- [x] Checked schema for all modified table for columns that will
auto-update timestamps during migration.
- [x] Confirmed that updating the timestamps is acceptable, and will not
cause unwanted side effects.
Closes#31077.
- Added logic to wait for the uninstall command to finish running before
exiting the script.
- Also added the `/norestart` flag so users who click uninstall in
self-service aren't at risk of a sudden and unintentional reboot as the
result of software uninstalling.
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
<!-- Note that API documentation changes are now addressed by the
product design team. -->
- [x] 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.
---------
Co-authored-by: Ian Littman <iansltx@gmail.com>
Fixes#25587. SubEthaEdit packgeInfo file is a bit bigger, but the only
thing different is the list of package IDs included, and that's not what
was broken/fixed here, so went with an abbreviated version that better
demonstrates what got fixed here.
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
<!-- Note that API documentation changes are now addressed by the
product design team. -->
- [x] 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.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated automated tests
- [x] Manual QA for all new/changed functionality
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Improved extraction of application names from uploaded PKG packages by
using the install path as a fallback method.
* **Tests**
* Added a new test case to verify correct name extraction from PKG
packages using the install path.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
> For #24873
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
<!-- Note that API documentation changes are now addressed by the
product design team. -->
- [x] 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/Committing-Changes.md#changes-files)
for more information.
- [x] Added/updated automated tests
- [x] A detailed QA plan exists on the associated ticket (if it isn't
there, work with the product group's QA engineer to add it)
- [x] Manual QA for all new/changed functionality
---------
Co-authored-by: Ian Littman <iansltx@gmail.com>
#22571
- [X] 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/Committing-Changes.md#changes-files)
for more information.
- [X] Added/updated tests
- [X] If database migrations are included, checked table schema to
confirm autoupdate
- For database migrations:
- [X] Checked schema for all modified table for columns that will
auto-update timestamps during migration.
- [X] Confirmed that updating the timestamps is acceptable, and will not
cause unwanted side effects.
- [X] Ensured the correct collation is explicitly set for character
columns (`COLLATE utf8mb4_unicode_ci`).
- [X] Manual QA for all new/changed functionality
Work in progress for #20000
The biggest change here is the update to `uninstall_exe.ps1` so that it
is not completely broken.
I'd like to get these changes onto main for testing while I switch to
working on unreleased bugs.
# Windows EXE testing notes (in progress)
## FileZilla
https://filezilla-project.org/download.php?platform=win64
In uninstall script, use /S as $uninstallArgs
## Firefox
Get the full installer like:
https://download.mozilla.org/?product=firefox-latest&os=win&lang=en-US
DO NOT get product=firefox-stub
In uninstall script, use -ms as $uninstallArgs
Zoom offers two installers:
- Zoom for IT admins (already covered previously)
- "Regular" Zoom (covered here)
This tweaks the logic made as part of #19144 to ensure we cover both
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
<!-- Note that API documentation changes are now addressed by the
product design team. -->
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
for #19039 and #19041 this:
- fixes the install/remove scripts to read the env variable the proper
way
- truncates output before storing in the databse in case its longer than
MySQL's TEXT size
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
<!-- Note that API documentation changes are now addressed by the
product design team. -->
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
for https://github.com/fleetdm/fleet/issues/19020
- Fixes the rollback logic to get the right script for the software
being installed
- Fixes the messages displayed in the install results
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
<!-- Note that API documentation changes are now addressed by the
product design team. -->
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
Feature cleanup
# Checklist for submitter
If some of the following don't apply, delete the relevant line.
<!-- Note that API documentation changes are now addressed by the
product design team. -->
- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Manual QA for all new/changed functionality
Allen Houchins
