Update Docker Desktop FMA install script - #45244
Merged
Merged
Conversation
Support providing a custom patch policy file to the Homebrew ingester: if input.PatchPolicyPath is set the ingester reads the file, substitutes __VERSION__ and uses that as the patched query; otherwise it generates the policy as before. Tests updated to cover the override behavior. Add Docker Desktop-specific assets: input JSON now references an install script and patch policy, a patch SQL file and a bash install script are added, and the darwin output is updated to ignore Docker.app.back bundles in the patched query and to point to the new install script ref.
1 task
fleet-release
previously approved these changes
May 12, 2026
Contributor
Script Diff Resultsee/maintained-apps/outputs/docker-desktop/darwin.json=== Install // fe655671 -> 1c6cc2fd ===
--- /tmp/old.etPIo7 2026-05-12 16:13:07.381713401 +0000
+++ /tmp/new.4xkAnA 2026-05-12 16:13:07.381713401 +0000
@@ -104,6 +104,11 @@
if [ -d "$APPDIR/Docker.app" ]; then
sudo mv "$APPDIR/Docker.app" "$TMPDIR/Docker.app.bkp"
fi
+# Docker Desktop's own in-app updater leaves a Docker.app.back bundle alongside
+# Docker.app when it self-updates. osquery's apps table still picks up the
+# stale bundle by its bundle_identifier, which causes Fleet patch policies to
+# report Docker as out of date even after a successful upgrade.
+sudo rm -rf "$APPDIR/Docker.app.back"
sudo cp -R "$TMPDIR/Docker.app" "$APPDIR"
relaunch_application 'com.electron.dockerdesktop'
mkdir -p /usr/local/cli-plugins
=== Uninstall Script (no changes) === |
Simplify Homebrew ingester by removing support for a provided patch policy file: always call patch_policy.GenerateQueryForManifest to populate out.Queries.Patched. Update tests accordingly (remove temporary patch policy file and related assertions). Remove the docker_desktop.sql input file and its reference in the docker-desktop input JSON, and adjust the expected docker-desktop output patched query to match the generated query (removed the path NOT LIKE '%.back' condition).
fleet-release
approved these changes
May 12, 2026
allenhouchins
changed the title
fleet-release
previously approved these changes
May 12, 2026
Handle Docker Desktop's updater creating Docker.app.back by excluding paths ending with `.back` when computing patch status. Adds a special-case patched query in the Homebrew ingester to ignore `.back` installs, updates unit tests to cover the `docker-desktop` token and assert the modified query, and updates the docker-desktop darwin output fixture to match the new SQL that filters out `%.back` paths.
fleet-release
approved these changes
May 12, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #45244 +/- ##
==========================================
+ Coverage 66.73% 66.77% +0.03%
==========================================
Files 2732 2728 -4
Lines 218551 218358 -193
Branches 10803 10613 -190
==========================================
- Hits 145857 145802 -55
+ Misses 59480 59388 -92
+ Partials 13214 13168 -46
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…nd-self-service-only-if-the-top-level-app-is-outdated
fleet-release
approved these changes
May 13, 2026
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
fleet-release
approved these changes
May 13, 2026
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
cdcme
approved these changes
May 13, 2026
Contributor
WalkthroughThis PR extends the Homebrew maintained-apps pipeline to handle Docker Desktop installation with special patched-status logic. A new Bash install script detects whether Docker Desktop is running, quits and relaunches it when appropriate, installs from a DMG, removes stale Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.sh`:
- Around line 1-6: Add strict failure handling and guaranteed DMG detach: add
"set -euo pipefail" at the top of the script, initialize a global MOUNT_POINT=""
variable before any hdiutil operations, and implement a cleanup function
(registered via trap EXIT) that checks MOUNT_POINT and calls hdiutil detach if
mounted; also make quit_and_track_application() return a non-zero exit (or exit)
on failure so the script stops instead of continuing to sudo cp when Docker
hasn't quit. Ensure every hdiutil attach error causes immediate exit and the
trap still runs to avoid leaving mounts.
- Around line 49-51: The script currently logs when Docker Desktop failed to
quit (using the quit_success variable and bundle_id) but continues installation;
update both places where quit_success is checked (the block referencing
quit_success and bundle_id around line 49 and the similar check around line 103)
to abort the script on failure by printing a clear error to stderr and exiting
with a non-zero status (e.g., exit 1) so the install stops if Docker Desktop did
not quit; ensure the error message includes the bundle_id for context and that
any cleanup/rollback steps (if present) run before exiting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 16306a1e-19ee-4129-9edc-b1a8f5e18ac7
📒 Files selected for processing (5)
ee/maintained-apps/ingesters/homebrew/ingester.goee/maintained-apps/ingesters/homebrew/ingester_test.goee/maintained-apps/inputs/homebrew/docker-desktop.jsonee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.shee/maintained-apps/outputs/docker-desktop/darwin.json
Comment on lines
+1
to
+6
| #!/bin/bash | ||
|
|
||
| # variables | ||
| APPDIR="/Applications/" | ||
| TMPDIR=$(dirname "$(realpath "$INSTALLER_PATH")") | ||
| # functions |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.shRepository: fleetdm/fleet
Length of output: 4756
Fail fast on DMG operations and always detach mounts.
The script lacks set -e and cleanup mechanisms, allowing it to proceed when hdiutil attach, sudo cp, or hdiutil detach fail. This can leave a partial install or a mounted DMG. Additionally, quit_and_track_application() logs quit failures but doesn't stop execution, risking copy/install failures if Docker is still running.
Add set -euo pipefail at the start, initialize MOUNT_POINT="" before use, and add a cleanup trap to ensure detach runs on exit:
Proposed hardening
#!/bin/bash
+set -euo pipefail
# variables
APPDIR="/Applications/"
TMPDIR=$(dirname "$(realpath "$INSTALLER_PATH")")
+MOUNT_POINT=""
+
+cleanup() {
+ if [[ -n "${MOUNT_POINT}" && -d "${MOUNT_POINT}" ]]; then
+ hdiutil detach "${MOUNT_POINT}" >/dev/null 2>&1 || true
+ fi
+}
+trap cleanup EXIT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #!/bin/bash | |
| # variables | |
| APPDIR="/Applications/" | |
| TMPDIR=$(dirname "$(realpath "$INSTALLER_PATH")") | |
| # functions | |
| #!/bin/bash | |
| set -euo pipefail | |
| # variables | |
| APPDIR="/Applications/" | |
| TMPDIR=$(dirname "$(realpath "$INSTALLER_PATH")") | |
| MOUNT_POINT="" | |
| cleanup() { | |
| if [[ -n "${MOUNT_POINT}" && -d "${MOUNT_POINT}" ]]; then | |
| hdiutil detach "${MOUNT_POINT}" >/dev/null 2>&1 || true | |
| fi | |
| } | |
| trap cleanup EXIT | |
| # functions |
🤖 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/homebrew/scripts/docker_desktop_install.sh` around
lines 1 - 6, Add strict failure handling and guaranteed DMG detach: add "set
-euo pipefail" at the top of the script, initialize a global MOUNT_POINT=""
variable before any hdiutil operations, and implement a cleanup function
(registered via trap EXIT) that checks MOUNT_POINT and calls hdiutil detach if
mounted; also make quit_and_track_application() return a non-zero exit (or exit)
on failure so the script stops instead of continuing to sudo cp when Docker
hasn't quit. Ensure every hdiutil attach error causes immediate exit and the
trap still runs to avoid leaving mounts.
Comment on lines
+49
to
+51
| if [[ "$quit_success" = false ]]; then | ||
| echo "Application '$bundle_id' did not quit." | ||
| fi |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's locate and read the file
find . -name "docker_desktop_install.sh" -type fRepository: fleetdm/fleet
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Read the file to examine the context
cat -n ./ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.shRepository: fleetdm/fleet
Length of output: 5610
🏁 Script executed:
#!/bin/bash
# Check for error handling flags (set -e) and confirm the function behavior
head -20 ./ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.shRepository: fleetdm/fleet
Length of output: 557
Abort install when Docker Desktop does not quit.
If Docker Desktop continues running when the script attempts to replace its app bundle, the installation will proceed anyway, risking file locks, permission errors, or a corrupted/partially updated installation.
Add error handling to stop the script if quit fails:
Proposed fix
if [[ "$quit_success" = false ]]; then
echo "Application '$bundle_id' did not quit."
+ return 1
fi
}
...
-quit_and_track_application 'com.electron.dockerdesktop'
+quit_and_track_application 'com.electron.dockerdesktop' || {
+ echo "Aborting install because Docker Desktop is still running."
+ exit 1
+}Also applies to: line 103
🤖 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/homebrew/scripts/docker_desktop_install.sh` around
lines 49 - 51, The script currently logs when Docker Desktop failed to quit
(using the quit_success variable and bundle_id) but continues installation;
update both places where quit_success is checked (the block referencing
quit_success and bundle_id around line 49 and the similar check around line 103)
to abort the script on failure by printing a clear error to stderr and exiting
with a non-zero status (e.g., exit 1) so the install stops if Docker Desktop did
not quit; ensure the error message includes the bundle_id for context and that
any cleanup/rollback steps (if present) run before exiting.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Fleet-maintained Docker Desktop (macOS) manifest so installs remove Docker.app.back artifacts and patch compliance no longer gets tripped up by Docker Desktop’s .back bundle behavior. It also adds an ingester-side Docker Desktop override and expands ingestion tests accordingly.
Changes:
- Add a Docker Desktop-specific Homebrew install script and wire it up via
install_script_path. - Update Docker Desktop’s generated
patchedquery to ignore bundles whosepathends with.back. - Add Homebrew ingester/test logic to special-case Docker Desktop’s
patchedquery.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ee/maintained-apps/outputs/docker-desktop/darwin.json | Updates patch query and install script ref; embeds new install script content. |
| ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.sh | New Docker Desktop DMG install script that removes Docker.app.back before copying. |
| ee/maintained-apps/inputs/homebrew/docker-desktop.json | Points Docker Desktop input to the new install script override. |
| ee/maintained-apps/ingesters/homebrew/ingester.go | Adds Docker Desktop-specific override for the generated patched query. |
| ee/maintained-apps/ingesters/homebrew/ingester_test.go | Adds coverage for the Docker Desktop patched query override. |
Comments suppressed due to low confidence (1)
ee/maintained-apps/outputs/docker-desktop/darwin.json:20
- In the embedded uninstall script, commands like
sudo rmdir '~/.docker/bin'andsudo rmdir '~/Library/Caches/…'wrap~in single quotes, so tilde expansion will not occur and those directories will not be removed. Consider expanding to an absolute path (e.g. via $HOME or /Users/$LOGGED_IN_USER) or using the existingtrashhelper for these locations.
"f8ed2624": "#!/bin/bash\n\n# variables\nAPPDIR=\"/Applications/\"\nLOGGED_IN_USER=$(scutil <<< \"show State:/Users/ConsoleUser\" | awk '/Name :/ { print $3 }')\n# functions\n\nquit_application() {\n local bundle_id=\"$1\"\n local timeout_duration=10\n\n # check if the application is running\n local app_running\n app_running=$(osascript -e \"application id \\\"$bundle_id\\\" is running\" 2>/dev/null)\n if [[ \"$app_running\" != \"true\" ]]; then\n return\n fi\n\n local console_user\n console_user=$(stat -f \"%Su\" /dev/console)\n if [[ -z \"$console_user\" || \"$console_user\" == \"root\" || \"$console_user\" == \"loginwindow\" ]]; then\n echo \"Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'.\"\n return\n fi\n\n echo \"Quitting application '$bundle_id'...\"\n\n # try to quit the application within the timeout period\n local quit_success=false\n SECONDS=0\n while (( SECONDS < timeout_duration )); do\n if osascript -e \"tell application id \\\"$bundle_id\\\" to quit\" >/dev/null 2>&1; then\n if ! pgrep -f \"$bundle_id\" >/dev/null 2>&1; then\n echo \"Application '$bundle_id' quit successfully.\"\n quit_success=true\n break\n fi\n fi\n sleep 1\n done\n\n if [[ \"$quit_success\" = false ]]; then\n echo \"Application '$bundle_id' did not quit.\"\n fi\n}\n\n\nremove_launchctl_service() {\n local service=\"$1\"\n local booleans=(\"true\" \"false\")\n local plist_status\n local paths\n local should_sudo\n\n echo \"Removing launchctl service ${service}\"\n\n for should_sudo in \"${booleans[@]}\"; do\n plist_status=$(launchctl list \"${service}\" 2>/dev/null)\n\n if [[ $plist_status == \\{* ]]; then\n if [[ $should_sudo == \"true\" ]]; then\n sudo launchctl remove \"${service}\"\n else\n launchctl remove \"${service}\"\n fi\n sleep 1\n fi\n\n paths=(\n \"/Library/LaunchAgents/${service}.plist\"\n \"/Library/LaunchDaemons/${service}.plist\"\n )\n\n # if not using sudo, prepend the home directory to the paths\n if [[ $should_sudo == \"false\" ]]; then\n for i in \"${!paths[@]}\"; do\n paths[i]=\"${HOME}${paths[i]}\"\n done\n fi\n\n for path in \"${paths[@]}\"; do\n if [[ -e \"$path\" ]]; then\n if [[ $should_sudo == \"true\" ]]; then\n sudo rm -f -- \"$path\"\n else\n rm -f -- \"$path\"\n fi\n fi\n done\n done\n}\n\ntrash() {\n local logged_in_user=\"$1\"\n local target_file=\"$2\"\n local timestamp=\"$(date +%Y-%m-%d-%s)\"\n local rand=\"$(jot -r 1 0 99999)\"\n\n # replace ~ with /Users/$logged_in_user\n if [[ \"$target_file\" == ~* ]]; then\n target_file=\"/Users/$logged_in_user${target_file:1}\"\n fi\n\n local trash=\"/Users/$logged_in_user/.Trash\"\n local file_name=\"$(basename \"${target_file}\")\"\n\n if [[ -e \"$target_file\" ]]; then\n echo \"removing $target_file.\"\n mv -f \"$target_file\" \"$trash/${file_name}_${timestamp}_${rand}\"\n else\n echo \"$target_file doesn't exist.\"\n fi\n}\n\nremove_launchctl_service 'com.docker.helper'\nremove_launchctl_service 'com.docker.socket'\nremove_launchctl_service 'com.docker.vmnetd'\nquit_application 'com.docker.docker'\nquit_application 'com.electron.dockerdesktop'\nsudo rm -rf '/Library/PrivilegedHelperTools/com.docker.socket'\nsudo rm -rf '/Library/PrivilegedHelperTools/com.docker.vmnetd'\nsudo rmdir '~/.docker/bin'\nsudo rm -rf \"$APPDIR/Docker.app\"\nsudo rm -rf '/usr/local/cli-plugins/docker-compose'\nsudo rm -rf '/usr/local/bin/hub-tool'\nsudo rm -rf '/usr/local/bin/kubectl.docker'\nsudo rm -rf '/usr/local/bin/docker'\nsudo rm -rf '/usr/local/bin/docker-credential-desktop'\nsudo rm -rf '/usr/local/bin/docker-credential-ecr-login'\nsudo rm -rf '/usr/local/bin/docker-credential-osxkeychain'\nsudo rmdir '~/Library/Caches/com.plausiblelabs.crashreporter.data'\nsudo rmdir '~/Library/Caches/KSCrashReports'\ntrash $LOGGED_IN_USER '/usr/local/bin/docker-compose.backup'\ntrash $LOGGED_IN_USER '/usr/local/bin/docker.backup'\ntrash $LOGGED_IN_USER '~/.docker'\ntrash $LOGGED_IN_USER '~/Library/Application Scripts/com.docker.helper'\ntrash $LOGGED_IN_USER '~/Library/Application Scripts/group.com.docker'\ntrash $LOGGED_IN_USER '~/Library/Application Support/com.apple.sharedfilelist/com.apple.LSSharedFileList.ApplicationRecentDocuments/com.docker.helper.sfl*'\ntrash $LOGGED_IN_USER '~/Library/Application Support/com.apple.sharedfilelist/com.apple.LSSharedFileList.ApplicationRecentDocuments/com.electron.dockerdesktop.sfl*'\ntrash $LOGGED_IN_USER '~/Library/Application Support/com.bugsnag.Bugsnag/com.docker.docker'\ntrash $LOGGED_IN_USER '~/Library/Application Support/Docker Desktop'\ntrash $LOGGED_IN_USER '~/Library/Caches/com.docker.docker'\ntrash $LOGGED_IN_USER '~/Library/Caches/com.plausiblelabs.crashreporter.data/com.docker.docker'\ntrash $LOGGED_IN_USER '~/Library/Caches/KSCrashReports/Docker'\ntrash $LOGGED_IN_USER '~/Library/Containers/com.docker.docker'\ntrash $LOGGED_IN_USER '~/Library/Containers/com.docker.helper'\ntrash $LOGGED_IN_USER '~/Library/Group Containers/group.com.docker'\ntrash $LOGGED_IN_USER '~/Library/HTTPStorages/com.docker.docker'\ntrash $LOGGED_IN_USER '~/Library/HTTPStorages/com.docker.docker.binarycookies'\ntrash $LOGGED_IN_USER '~/Library/Logs/Docker Desktop'\ntrash $LOGGED_IN_USER '~/Library/Preferences/com.docker.docker.plist'\ntrash $LOGGED_IN_USER '~/Library/Preferences/com.electron.docker-frontend.plist'\ntrash $LOGGED_IN_USER '~/Library/Preferences/com.electron.dockerdesktop.plist'\ntrash $LOGGED_IN_USER '~/Library/Saved Application State/com.electron.docker-frontend.savedState'\ntrash $LOGGED_IN_USER '~/Library/Saved Application State/com.electron.dockerdesktop.savedState'\n"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
187
to
+202
| // create patch policy | ||
| out.Queries.Patched, err = patch_policy.GenerateQueryForManifest(patch_policy.PolicyData{ | ||
| Platform: "darwin", | ||
| Version: out.Version, | ||
| ExistsQuery: out.Queries.Exists, | ||
| }) | ||
| if err != nil { | ||
| return nil, ctxerr.Wrap(ctx, err, "creating patch policy") | ||
| } | ||
| if input.Token == "docker-desktop" { | ||
| // Docker's updater can leave Docker.app.back; do not treat it as the installed app for patch status. | ||
| out.Queries.Patched = fmt.Sprintf( | ||
| "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps WHERE bundle_identifier = '%s' AND path NOT LIKE '%%.back' AND version_compare(bundle_short_version, '%s') < 0);", | ||
| out.UniqueIdentifier, out.Version, | ||
| ) | ||
| } |
| @@ -4,10 +4,10 @@ | |||
| "version": "4.73.0", | |||
| "queries": { | |||
| "exists": "SELECT 1 FROM apps WHERE bundle_identifier = 'com.electron.dockerdesktop';", | |||
fleet-release
approved these changes
May 13, 2026
allenhouchins
deleted the
45242-show-update-available-status-library-and-self-service-only-if-the-top-level-app-is-outdated
branch
May 13, 2026 20:22
This was referenced May 14, 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.
This pull request improves the handling and installation of Docker Desktop in Fleet, with a focus on addressing issues caused by leftover
.backapp bundles after in-app updates. It also introduces a custom installation script for Docker Desktop to ensure correct app replacement and relaunch behavior. The test coverage and configuration for Docker Desktop have been updated accordingly.Docker Desktop patch policy and installation improvements:
ingester.goto ignore any.backapp bundles, preventing false "out of date" patch status when stale bundles are present after self-updates.docker_desktop_install.sh) that safely quits Docker Desktop, removes.backbundles, moves the new app into place, relaunches the app if it was running, and sets up CLI symlinks.docker-desktop.json).Test enhancements:
ingester_test.goto include Docker Desktop, verifying the correct patch and exists queries for this special case. [1] [2] [3]