Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ee/maintained-apps/ingesters/homebrew/ingester.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,13 @@ func (i *brewIngester) ingestOne(ctx context.Context, input inputApp) (*maintain
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,
)
}

return out, nil
}
Expand Down
19 changes: 14 additions & 5 deletions ee/maintained-apps/ingesters/homebrew/ingester_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestIngestValidations(t *testing.T) {
Version: "1.0",
}

case "ok", "install_script_path", "uninstall_script_path", "uninstall_script_path_with_pre", "uninstall_script_path_with_post", "patch_policy_path":
case "ok", "docker-desktop", "install_script_path", "uninstall_script_path", "uninstall_script_path_with_pre", "uninstall_script_path_with_post", "patch_policy_path":
cask = brewCask{
Token: appToken,
Name: []string{appToken},
Expand Down Expand Up @@ -120,6 +120,7 @@ func TestIngestValidations(t *testing.T) {
{"missing URL for cask nourl", inputApp{Token: "nourl", UniqueIdentifier: "abc", InstallerFormat: "pkg"}},
{"parse URL for cask invalidurl", inputApp{Token: "invalidurl", UniqueIdentifier: "abc", InstallerFormat: "pkg"}},
{"", inputApp{Token: "ok", UniqueIdentifier: "abc", InstallerFormat: "pkg"}},
{"", inputApp{Token: "docker-desktop", UniqueIdentifier: "com.electron.dockerdesktop", InstallerFormat: "dmg", Name: "Docker Desktop", Slug: "docker-desktop/darwin"}},
{"", inputApp{Token: "install_script_path", UniqueIdentifier: "abc", InstallerFormat: "pkg", InstallScriptPath: path.Join(tempDir, "install_script.sh")}},
{"", inputApp{Token: "uninstall_script_path", UniqueIdentifier: "abc", InstallerFormat: "pkg", UninstallScriptPath: path.Join(tempDir, "uninstall_script.sh")}},
{"cannot provide pre-uninstall scripts if uninstall script is provided", inputApp{Token: "uninstall_script_path_with_pre", UniqueIdentifier: "abc", InstallerFormat: "pkg", UninstallScriptPath: path.Join(tempDir, "uninstall_script.sh"), PreUninstallScripts: []string{"foo", "bar"}}},
Expand Down Expand Up @@ -149,10 +150,18 @@ func TestIngestValidations(t *testing.T) {
require.Equal(t, testUninstallScriptContents, out.UninstallScript)
}

require.Equal(t,
fmt.Sprintf("SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps WHERE bundle_identifier = '%s' AND version_compare(bundle_short_version, '%s') < 0);", c.inputApp.UniqueIdentifier, out.Version),
out.Queries.Patched,
)
if c.inputApp.Token == "docker-desktop" {
require.Equal(t, "SELECT 1 FROM apps WHERE bundle_identifier = 'com.electron.dockerdesktop';", out.Queries.Exists)
require.Equal(t,
"SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps WHERE bundle_identifier = 'com.electron.dockerdesktop' AND path NOT LIKE '%.back' AND version_compare(bundle_short_version, '1.0') < 0);",
out.Queries.Patched,
)
} else {
require.Equal(t,
fmt.Sprintf("SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps WHERE bundle_identifier = '%s' AND version_compare(bundle_short_version, '%s') < 0);", c.inputApp.UniqueIdentifier, out.Version),
out.Queries.Patched,
)
}

})
}
Expand Down
3 changes: 2 additions & 1 deletion ee/maintained-apps/inputs/homebrew/docker-desktop.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"unique_identifier": "com.electron.dockerdesktop",
"token": "docker-desktop",
"installer_format": "dmg",
"default_categories": ["Developer tools"]
"default_categories": ["Developer tools"],
"install_script_path": "ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.sh"
}
122 changes: 122 additions & 0 deletions ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
#!/bin/bash

# variables
APPDIR="/Applications/"
TMPDIR=$(dirname "$(realpath "$INSTALLER_PATH")")
# functions
Comment on lines +1 to +6

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.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat ee/maintained-apps/inputs/homebrew/scripts/docker_desktop_install.sh

Repository: 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.


quit_and_track_application() {
local bundle_id="$1"
local 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)
if [[ "$app_running" != "true" ]]; then
eval "export $var_name=0"
return
fi

local console_user
console_user=$(stat -f "%Su" /dev/console)
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
fi

# App was running, mark it for relaunch
eval "export $var_name=1"
echo "Application '$bundle_id' was running; will relaunch after installation."

echo "Quitting application '$bundle_id'..."

# try to quit the application within the timeout period
local quit_success=false
SECONDS=0
while (( SECONDS < timeout_duration )); do
if osascript -e "tell application id \"$bundle_id\" to quit" >/dev/null 2>&1; then
if ! pgrep -f "$bundle_id" >/dev/null 2>&1; then
echo "Application '$bundle_id' quit successfully."
quit_success=true
break
fi
fi
sleep 1
done

if [[ "$quit_success" = false ]]; then
echo "Application '$bundle_id' did not quit."
fi
Comment on lines +49 to +51

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's locate and read the file
find . -name "docker_desktop_install.sh" -type f

Repository: 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.sh

Repository: 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.sh

Repository: 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.

}


relaunch_application() {
local bundle_id="$1"
local 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"
if [[ "$was_running" != "1" ]]; then
return
fi

local console_user
console_user=$(stat -f "%Su" /dev/console)
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
fi

echo "Relaunching application '$bundle_id'..."

# Launch the app in the logged-in user's GUI session. Apps launched by root
# won't register with the user's Dock/GUI, so run 'open' as the console user.
# Use 'launchctl asuser' to bootstrap into the console user's Mach namespace
# and GUI session — 'sudo -u' alone doesn't do this, which can cause
# LSOpenURLsWithRole() failures even when 'open' exits 0.
local open_status=0
if [[ $EUID -eq 0 ]]; then
local console_uid
console_uid=$(id -u "$console_user")
/bin/launchctl asuser "$console_uid" sudo -u "$console_user" open -b "$bundle_id" >/dev/null 2>&1 || open_status=$?
else
open -b "$bundle_id" >/dev/null 2>&1 || open_status=$?
fi

if [[ $open_status -eq 0 ]]; then
echo "Application '$bundle_id' relaunched successfully."
else
echo "Failed to relaunch application '$bundle_id'."
fi
}


# extract contents
MOUNT_POINT=$(mktemp -d /tmp/dmg_mount_XXXXXX)
hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH"
sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"
hdiutil detach "$MOUNT_POINT"
# copy to the applications folder
quit_and_track_application 'com.electron.dockerdesktop'
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
/bin/ln -h -f -s -- "$APPDIR/Docker.app/Contents/Resources/cli-plugins/docker-compose" "/usr/local/cli-plugins/docker-compose"
mkdir -p /usr/local/bin
/bin/ln -h -f -s -- "$APPDIR/Docker.app/Contents/Resources/bin/hub-tool" "/usr/local/bin/hub-tool"
/bin/ln -h -f -s -- "$APPDIR/Docker.app/Contents/Resources/bin/kubectl" "/usr/local/bin/kubectl.docker"
/bin/ln -h -f -s -- "$APPDIR/Docker.app/Contents/Resources/bin/docker" "/usr/local/bin/docker"
/bin/ln -h -f -s -- "$APPDIR/Docker.app/Contents/Resources/bin/docker-credential-desktop" "/usr/local/bin/docker-credential-desktop"
/bin/ln -h -f -s -- "$APPDIR/Docker.app/Contents/Resources/bin/docker-credential-ecr-login" "/usr/local/bin/docker-credential-ecr-login"
/bin/ln -h -f -s -- "$APPDIR/Docker.app/Contents/Resources/bin/docker-credential-osxkeychain" "/usr/local/bin/docker-credential-osxkeychain"
Loading
Loading