Repository navigation
macOS FMA uninstalls: move trashed files as the user, not root (6/11) - #57091
Merged
Merged
Conversation
allenhouchins
temporarily deployed
to
Docker Hub
October 31, 2026 18:26 — with 
GitHub Actions
Inactive
fleet-release
approved these changes
Oct 31, 2026
2 of 3 tasks
allenhouchins
added a commit
that referenced
this pull request
Nov 1, 2026
…#57086) Part 1 of 11, split from #56846. That PR changes 992 files, and the auto-approval only covers PRs under 100 files. The 11 PRs touch disjoint files, so they merge in any order without conflicts, and together they match #56846's tree exactly. Merge this one first so the generator change lands before, or with, the rest of the outputs. Other parts: - 2/11: #57087, `aldente` through `captain` - 3/11: #57088, `capto` through `dia` - 4/11: #57089, `dialpad` through `free-download-manager` - 5/11: #57090, `front` through `julia-app` - 6/11: #57091, `jump-desktop` through `microsoft-excel` - 7/11: #57092, `microsoft-onenote` through `openboard` - 8/11: #57093, `opencloud` through `raycast` - 9/11: #57094, `readest` through `soundsource` - 10/11: #57095, `sourcetree` through `twingate` - 11/11: #57096, `typeface` through `zwift` ## Summary Hardening for the `trash()` helper in generated macOS FMA uninstall scripts. Fleet runs these scripts as root. `trash()` moved the cask's `zap` paths into the console user's `~/.Trash` with a root `mv -f`. Those paths and `~/.Trash` both belong to that user, so root shouldn't be the one writing there. - **Paths in the user's home (`~/...`)** are moved to the Trash as the user, with `sudo -u "$logged_in_user" mv -f`. If the user's move fails, the script logs `Failed to move <path> to the Trash.` and root doesn't take over. - **Other paths** are deleted as root by a new `remove_system_path` helper instead of being moved into the user's Trash, such as `/Library/LaunchDaemons/*.plist`, `/Library/PrivilegedHelperTools/*` and `/Users/Shared/*`. The helper already deleted these at the login window (#56457). - **A symlinked parent folder can't redirect a root delete.** `rm -rf` doesn't follow a symlink at the end of a path, but it does follow one in a parent folder. For example, a user who creates `/Users/Shared/TechSmith` as a symlink first could aim Camtasia's `/Users/Shared/TechSmith/Camtasia` delete at a root-only folder. `remove_system_path` uses `cd -P` to enter the real parent folder, checks that it's the folder the path names, and deletes `./<name>` from there, so a symlink swapped in after the check has no effect. Only top-level symlinks such as `/tmp` and `/etc` are followed, because users can't write to `/`. A path that fails the check is left in place and logged as `Skipped <path>: a folder in its path is a symlink.` - At the login window, `~` paths are still skipped, and cleanup stays best-effort: `trash()` never fails the uninstall. **Behavior change:** system paths listed in a cask's `zap trash:` are deleted, not moved to the user's Trash. A system path with a symlink in its parent folders below the top level is skipped. A shell without Full Disk Access can still move files into `~/.Trash` by name (only listing it is blocked), so the move as the user doesn't depend on fleetd's TCC grants. ## Outputs Across all 11 parts, 974 macOS outputs change. Each changes only its uninstall script and `uninstall_script_ref`. Version, URL, sha256, install script and queries are byte-identical to `main`. This part has the generator, its tests, the 16 custom uninstall scripts, and 73 outputs: - **16 custom-script outputs**: each embeds its updated input script. The custom scripts carry a copy of `trash()`, now the new one plus `remove_system_path()`, including Citrix Workspace's, added on main in #57081. `TestCustomUninstallScriptsUseCurrentTrash` keeps both identical to the generator. - **20 more frozen outputs**: the ingest job never rewrites these, so they ship here. (WezTerm is both custom and frozen.) - **37 generated outputs**, `010-editor` through `alcove`. Parts 2–11 have the other 901 generated outputs. The outputs were built and cross-checked against `cmd/maintained-apps` regeneration in #56846. Because these outputs change, the darwin validator runs on each part's apps. Expect some failures from apps that had already drifted upstream. The Check Script Diff job will fail here: its comment is over GitHub's 65,536-character limit at this size. It isn't a required check. # Checklist for submitter - [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. ## Testing - [x] Added/updated automated tests - [ ] QA'd all new/changed functionality manually `TestUninstallScriptTrash` (renamed from `TestUninstallScriptTrashWithoutGUIUser`) runs generated `trash` calls in bash with the user's home under a temp dir. A bare `mv` is stubbed to fail, so the test fails if root ever moves a file. It checks that: - home paths really land in `~/.Trash` through `sudo -u` - system paths are deleted - a system path under a symlinked parent folder, given directly or through a glob, is skipped, and the files the symlink points to stay in place - a failed user move is reported and nothing is removed as root The test fails against the old helper, and also fails with the symlink check in `remove_system_path` turned off. Also checked: - `go test ./cmd/maintained-apps/... ./ee/maintained-apps/...` passes on this branch alone, and on each of parts 2–11 alone - every custom script passes `bash -n` - run directly on macOS, `remove_system_path` deletes a normal path, skips one under a symlinked folder, removes a symlink at the end of the path without touching its target, and follows `/tmp` Not yet run as root by fleetd on a real Mac. ## AI **AI:** Claude Code (claude-opus-5-5)
allenhouchins
marked this pull request as ready for review
November 1, 2026 01:51
fleet-release
approved these changes
Nov 1, 2026
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The privileged macOS uninstall changes need real-Mac validation through fleetd because snapshot checks and stubbed sudo tests cannot verify actual user permissions, Trash access, or TCC behavior.
No open issues 🎉
What changed in this PR
Part 6 of the macOS Fleet-maintained app uninstall hardening rollout, applying the shared helper changes from #57086 to generated app manifests.
Changes:
- Moves home-directory cleanup targets to the console user’s Trash using
sudo -u, without falling back to root. - Adds
remove_system_pathfor root-level deletion, skipping paths with symlinked parent folders below the top level. - Updates embedded uninstall scripts and their references; leaves app versions, installer metadata, and install scripts unchanged.
| File | Description |
|---|---|
| ee/maintained-apps/outputs/merlin-project/darwin.json | Updates Merlin Project’s uninstall helper and reference. |
| ee/maintained-apps/outputs/memoryanalyzer/darwin.json | Updates MemoryAnalyzer’s uninstall helper and reference. |
| ee/maintained-apps/outputs/meetingbar/darwin.json | Updates MeetingBar’s uninstall helper and reference. |
| ee/maintained-apps/outputs/mattermost/darwin.json | Updates Mattermost’s uninstall helper and reference. |
| ee/maintained-apps/outputs/marvel/darwin.json | Updates Marvel’s uninstall helper and reference. |
| ee/maintained-apps/outputs/mark-text/darwin.json | Updates MarkText’s uninstall helper and reference. |
| ee/maintained-apps/outputs/magicquit/darwin.json | Updates MagicQuit’s uninstall helper and reference. |
| ee/maintained-apps/outputs/mactracker/darwin.json | Updates Mactracker’s uninstall helper and reference. |
| ee/maintained-apps/outputs/macjournal/darwin.json | Updates MacJournal’s uninstall helper and reference. |
| ee/maintained-apps/outputs/mace/darwin.json | Updates MACE’s uninstall helper and reference. |
| ee/maintained-apps/outputs/maccy/darwin.json | Updates Maccy’s uninstall helper and reference. |
| ee/maintained-apps/outputs/lunasea/darwin.json | Updates LunaSea’s uninstall helper and reference. |
| ee/maintained-apps/outputs/lunacy/darwin.json | Updates Lunacy’s uninstall helper and reference. |
| ee/maintained-apps/outputs/loop/darwin.json | Updates Loop’s uninstall helper and reference. |
| ee/maintained-apps/outputs/loom/darwin.json | Updates Loom’s uninstall helper and reference. |
| ee/maintained-apps/outputs/local/darwin.json | Updates Local’s uninstall helper and reference. |
| ee/maintained-apps/outputs/lapce/darwin.json | Updates Lapce’s uninstall helper and reference. |
| ee/maintained-apps/outputs/knockknock/darwin.json | Updates KnockKnock’s uninstall helper and reference. |
| ee/maintained-apps/outputs/kitty/darwin.json | Updates kitty’s uninstall helper and reference. |
| ee/maintained-apps/outputs/keycastr/darwin.json | Updates KeyCastr’s uninstall helper and reference. |
| ee/maintained-apps/outputs/keeweb/darwin.json | Updates KeeWeb’s uninstall helper and reference. |
| ee/maintained-apps/outputs/kdenlive/darwin.json | Updates Kdenlive’s uninstall helper and reference. |
Review effort: Balanced
This branch was previously deployed
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.
Part 6 of 11, split from #56846. That PR changes 992 files, and the auto-approval only covers PRs under 100 files. Part 1, #57086, has the generator change, the tests, the custom scripts, and the frozen outputs; see it for the full explanation. The 11 PRs touch disjoint files, so they merge in any order without conflicts, and together they match #56846's tree exactly. Merge #57086 first.
Change
Generated macOS FMA uninstall scripts now move
~paths from a cask'szap trash:into the console user's Trash as that user, withsudo -u, instead of as root. Other paths are deleted as root by a newremove_system_pathhelper, which skips a path when a folder in it below the top level is a symlink.This PR regenerates 90 macOS outputs,
jump-desktopthroughmicrosoft-excel. Each changes only its uninstall script anduninstall_script_ref: no versions, SHAs, URLs, install scripts or queries.The darwin validator runs on these apps, so expect some failures from apps that had already drifted upstream. The Check Script Diff job will fail here: its comment is over GitHub's 65,536-character limit at this size. It isn't a required check.
Checklist for submitter
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.Testing
go test ./cmd/maintained-apps/... ./ee/maintained-apps/...passes on this branch alone.AI
AI: Claude Code (claude-opus-5-5)