Skip to content

Improve Docker Desktop installer script - #45425

Merged
allenhouchins merged 2 commits into
mainfrom
allenhouchins-update-docker-script
May 14, 2026
Merged

Improve Docker Desktop installer script#45425
allenhouchins merged 2 commits into
mainfrom
allenhouchins-update-docker-script

Conversation

@allenhouchins

@allenhouchins allenhouchins commented May 13, 2026

Copy link
Copy Markdown
Member

Enable strict shell flags and add a trap-based cleanup for DMG mountpoints to ensure detach and removal on exit. Add explicit error handling for hdiutil attach (fail fast with message) and reset MOUNT_POINT after detach. Harden app quit/relaunch logic by using safer variable assignments, providing fallbacks for osascript/stat, and returning explicit status codes. Minor safety improvements (sudo cp, set -euo pipefail) to make the installation script more reliable.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Docker Desktop macOS installation reliability with enhanced error handling and validation.
    • Installation now fails clearly with descriptive error messages if critical operations encounter problems, instead of proceeding silently.
    • Enhanced cleanup procedures to properly release system resources during installation.

Review Change Stack

Enable strict shell flags and add a trap-based cleanup for DMG mountpoints to ensure detach and removal on exit. Add explicit error handling for hdiutil attach (fail fast with message) and reset MOUNT_POINT after detach. Harden app quit/relaunch logic by using safer variable assignments, providing fallbacks for osascript/stat, and returning explicit status codes. Minor safety improvements (sudo cp, set -euo pipefail) to make the installation script more reliable.
fleet-release
fleet-release previously approved these changes May 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Script Diff Results

ee/maintained-apps/outputs/docker-desktop/darwin.json

=== Install // 1c6cc2fd -> 2c3a200a ===

--- /tmp/old.1IffRK	2026-05-13 20:43:47.068154541 +0000
+++ /tmp/new.wHQJax	2026-05-13 20:43:47.068154541 +0000
@@ -1,29 +1,45 @@
 #!/bin/bash
 
+set -euo pipefail
+
 # variables
 APPDIR="/Applications/"
 TMPDIR=$(dirname "$(realpath "$INSTALLER_PATH")")
+MOUNT_POINT=""
+
+cleanup() {
+  local mp="${MOUNT_POINT:-}"
+  if [[ -n "$mp" ]]; then
+    if mount | grep -q " on $mp "; then
+      hdiutil detach "$mp" >/dev/null 2>&1 || true
+    fi
+    rmdir "$mp" >/dev/null 2>&1 || true
+  fi
+}
+trap cleanup EXIT
+
 # functions
 
 quit_and_track_application() {
   local bundle_id="$1"
-  local var_name="APP_WAS_RUNNING_$(echo "$bundle_id" | tr '.-' '__')"
+  local var_name
+  var_name="APP_WAS_RUNNING_$(echo "$bundle_id" | tr '.-' '__')"
   local timeout_duration=10
 
   # check if the application is running
   local app_running
-  app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+  app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null || echo "false")
   if [[ "$app_running" != "true" ]]; then
     eval "export $var_name=0"
-    return
+    return 0
   fi
 
   local console_user
-  console_user=$(stat -f "%Su" /dev/console)
+  console_user=$(stat -f "%Su" /dev/console 2>/dev/null || echo "")
   if [[ -z "$console_user" || "$console_user" == "root" || "$console_user" == "loginwindow" ]]; then
     echo "Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'."
     eval "export $var_name=0"
-    return
+    return 0
   fi
 
   # App was running, mark it for relaunch
@@ -47,24 +63,26 @@
   done
 
   if [[ "$quit_success" = false ]]; then
-    echo "Application '$bundle_id' did not quit."
+    echo "Application '$bundle_id' did not quit within ${timeout_duration}s; aborting install." >&2
+    return 1
   fi
 }
 
 
 relaunch_application() {
   local bundle_id="$1"
-  local var_name="APP_WAS_RUNNING_$(echo "$bundle_id" | tr '.-' '__')"
+  local var_name
+  var_name="APP_WAS_RUNNING_$(echo "$bundle_id" | tr '.-' '__')"
   local was_running
 
   # Check if the app was running before installation
-  eval "was_running=\$$var_name"
+  eval "was_running=\${$var_name:-0}"
   if [[ "$was_running" != "1" ]]; then
     return
   fi
 
   local console_user
-  console_user=$(stat -f "%Su" /dev/console)
+  console_user=$(stat -f "%Su" /dev/console 2>/dev/null || echo "")
   if [[ -z "$console_user" || "$console_user" == "root" || "$console_user" == "loginwindow" ]]; then
     echo "Not logged into a non-root GUI; skipping relaunching application ID '$bundle_id'."
     return
@@ -96,9 +114,13 @@
 
 # extract contents
 MOUNT_POINT=$(mktemp -d /tmp/dmg_mount_XXXXXX)
-hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH"
+if ! hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH"; then
+  echo "Failed to mount DMG '$INSTALLER_PATH'." >&2
+  exit 1
+fi
 sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"
 hdiutil detach "$MOUNT_POINT"
+MOUNT_POINT=""
 # copy to the applications folder
 quit_and_track_application 'com.electron.dockerdesktop'
 if [ -d "$APPDIR/Docker.app" ]; then

=== Uninstall Script (no changes) ===

@allenhouchins
allenhouchins marked this pull request as ready for review May 14, 2026 02:02
Copilot AI review requested due to automatic review settings May 14, 2026 02:02

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@allenhouchins
allenhouchins merged commit ab39208 into main May 14, 2026
14 of 15 checks passed
@allenhouchins
allenhouchins deleted the allenhouchins-update-docker-script branch May 14, 2026 02:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Docker Desktop maintained-app install script with strict shell flags, a trap-based cleanup for the DMG mountpoint, explicit error handling for hdiutil attach, and safer variable assignment patterns to play well with set -euo pipefail. The regenerated outputs/docker-desktop/darwin.json reflects the new install_script_ref.

Changes:

  • Adds set -euo pipefail and a trap cleanup EXIT that detaches the DMG and removes the mountpoint directory.
  • Splits local var=$(...) into separate local/assignment pairs and adds || echo "" fallbacks so failed subshells don't kill the script under set -e/pipefail; also uses ${var:-0} in the relaunch lookup for set -u safety.
  • Fails fast (with stderr message and non-zero exit) when hdiutil attach fails, and when the app cannot be quit within the timeout.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.sh Source script: strict mode, cleanup trap, hardened helper functions, explicit hdiutil error handling.
ee/maintained-apps/outputs/docker-desktop/darwin.json Regenerated manifest pointing install_script_ref to the new 2c3a200a script body.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 60702eb9-dca5-4847-80c6-631ee356e674

📥 Commits

Reviewing files that changed from the base of the PR and between d96355a and f4fd473.

📒 Files selected for processing (2)
  • ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.sh
  • ee/maintained-apps/outputs/docker-desktop/darwin.json

Walkthrough

This PR hardens the Docker Desktop macOS installer script by introducing strict shell error handling and defensive operations. The script now enables set -euo pipefail for strict mode, adds a cleanup trap to automatically unmount the DMG on exit, and improves error resilience in application quit/relaunch logic. DMG mounting now validates and fails explicitly on errors. The JSON manifest is updated to reference the new script version 2c3a200a.

Possibly related PRs

  • fleetdm/fleet#45244: Adjusts ingestion and patch policy to ignore *.back artifacts during Docker Desktop installation, directly related to the removal of stale Docker.app.back bundles in this improved installer script.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch allenhouchins-update-docker-script

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants