Use pkgutil approach to be more effective at uninstalling (#22618)

#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

ee/server/service/software_installers.go

@@ -105,13 +105,17 @@ func preProcessUninstallScript(payload *fleet.UploadSoftwareInstallerPayload) {
 	var packageID string
 	switch payload.Extension {
 	case "pkg":
-		// For pkgs we've decided to use the app's name instead of relying on pkgutil.
-		// See https://github.com/fleetdm/fleet/issues/22571.
-		// In future iterations we may start using the stored package_ids.
-		packageID = fmt.Sprintf("\"%s\"", payload.Title)
+		var sb strings.Builder
+		_, _ = sb.WriteString("(\n")
+		for _, pkgID := range payload.PackageIDs {
+			_, _ = sb.WriteString(fmt.Sprintf("  \"%s\"\n", pkgID))
+		}
+		_, _ = sb.WriteString(")") // no ending newline
+		packageID = sb.String()
 	default:
 		packageID = fmt.Sprintf("\"%s\"", payload.PackageIDs[0])
 	}
+
 	payload.UninstallScript = packageIDRegex.ReplaceAllString(payload.UninstallScript, fmt.Sprintf("%s${suffix}", packageID))
 }
 

ee/server/service/software_installers_test.go

@@ -15,7 +15,7 @@ import (
 
 func TestPreProcessUninstallScript(t *testing.T) {
 	t.Parallel()
-	input := `
+	var input = `
 blah$PACKAGE_IDS
 pkgids=$PACKAGE_ID
 they are $PACKAGE_ID, right $MY_SECRET?
@@ -42,21 +42,39 @@ quotes and braces for "com.foo"
 	assert.Equal(t, expected, payload.UninstallScript)
 
 	payload = fleet.UploadSoftwareInstallerPayload{
-		Title:           "Foo bar",
 		Extension:       "pkg",
 		UninstallScript: input,
 		PackageIDs:      []string{"com.foo", "com.bar"},
 	}
 	preProcessUninstallScript(&payload)
 	expected = `
 blah$PACKAGE_IDS
-pkgids="Foo bar"
-they are "Foo bar", right $MY_SECRET?
-quotes for "Foo bar"
-blah"Foo bar"withConcat
-quotes and braces for "Foo bar"
-"Foo bar"`
+pkgids=(
+  "com.foo"
+  "com.bar"
+)
+they are (
+  "com.foo"
+  "com.bar"
+), right $MY_SECRET?
+quotes for (
+  "com.foo"
+  "com.bar"
+)
+blah(
+  "com.foo"
+  "com.bar"
+)withConcat
+quotes and braces for (
+  "com.foo"
+  "com.bar"
+)
+(
+  "com.foo"
+  "com.bar"
+)`
 	assert.Equal(t, expected, payload.UninstallScript)
+
 }
 
 func TestInstallUninstallAuth(t *testing.T) {
@@ -75,8 +93,7 @@ func TestInstallUninstallAuth(t *testing.T) {
 		}, nil
 	}
 	ds.GetSoftwareInstallerMetadataByTeamAndTitleIDFunc = func(ctx context.Context, teamID *uint, titleID uint,
-		withScriptContents bool,
-	) (*fleet.SoftwareInstaller, error) {
+		withScriptContents bool) (*fleet.SoftwareInstaller, error) {
 		return &fleet.SoftwareInstaller{
 			Name:     "installer.pkg",
 			Platform: "darwin",
@@ -87,16 +104,14 @@ func TestInstallUninstallAuth(t *testing.T) {
 		return nil, nil
 	}
 	ds.InsertSoftwareInstallRequestFunc = func(ctx context.Context, hostID uint, softwareInstallerID uint, selfService bool) (string,
-		error,
-	) {
+		error) {
 		return "request_id", nil
 	}
 	ds.GetAnyScriptContentsFunc = func(ctx context.Context, id uint) ([]byte, error) {
 		return []byte("script"), nil
 	}
 	ds.NewHostScriptExecutionRequestFunc = func(ctx context.Context, request *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult,
-		error,
-	) {
+		error) {
 		return &fleet.HostScriptResult{
 			ExecutionID: "execution_id",
 		}, nil
@@ -182,6 +197,7 @@ func TestUninstallSoftwareTitle(t *testing.T) {
 	// Host scripts disabled
 	host.ScriptsEnabled = ptr.Bool(false)
 	require.ErrorContains(t, svc.UninstallSoftwareTitle(context.Background(), 1, 10), fleet.RunScriptsOrbitDisabledErrMsg)
+
 }
 
 func checkAuthErr(t *testing.T, shouldFail bool, err error) {

pkg/file/scripts/uninstall_pkg.sh

@@ -1,18 +1,22 @@
 #!/bin/sh
 
-package_app_name="$PACKAGE_ID"
+# Fleet extracts and saves package IDs.
+pkg_ids=$PACKAGE_ID
 
-# Make sure PACKAGE_ID is not empty.
-if [[ -z "$package_app_name" ]]; then
-  echo "Empty PACKAGE_ID variable."
-  exit 1
-fi
-
-# Make sure the PACKAGE_ID doesn't have "../" or is "."
-if [[ "$package_app_name" == *".."* || "$package_app_name" == "." ]]; then
-  echo "Invalid PACKAGE_ID value."
-  exit 1
-fi
-
-echo "Removing \"/Applications/$package_app_name\"..."
-rm -rf "/Applications/$package_app_name"
+# For each package id, get all .app folders associated with the package and remove them.
+for pkg_id in "${pkg_ids[@]}"
+do
+  # Get volume and location of the package.
+  volume=$(pkgutil --pkg-info "$pkg_id" | grep -i "volume" | awk '{if (NF>1) print $NF}')
+  location=$(pkgutil --pkg-info "$pkg_id" | grep -i "location" | awk '{if (NF>1) print $NF}')
+  # Check if this package id corresponds to a valid/installed package
+  if [[ ! -z "$volume" ]]; then
+    # Remove individual directories that end with ".app" belonging to the package.
+    # Only process directories that end with ".app" to prevent Fleet from removing top level directories.
+    pkgutil --only-dirs --files "$pkg_id" | grep "\.app$" | sed -e 's@^@'"$volume""$location"'/@' | tr '\n' '\0' | xargs -n 1 -0 rm -rf
+    # Remove receipts
+    pkgutil --forget "$pkg_id"
+  else
+    echo "WARNING: volume is empty for package ID $pkg_id"
+  fi
+done

pkg/file/testdata/scripts/uninstall_pkg.sh.golden

@@ -1,18 +1,22 @@
 #!/bin/sh
 
-package_app_name="$PACKAGE_ID"
+# Fleet extracts and saves package IDs.
+pkg_ids=$PACKAGE_ID
 
-# Make sure PACKAGE_ID is not empty.
-if [[ -z "$package_app_name" ]]; then
-  echo "Empty PACKAGE_ID variable."
-  exit 1
-fi
-
-# Make sure the PACKAGE_ID doesn't have "../" or is "."
-if [[ "$package_app_name" == *".."* || "$package_app_name" == "." ]]; then
-  echo "Invalid PACKAGE_ID value."
-  exit 1
-fi
-
-echo "Removing \"/Applications/$package_app_name\"..."
-rm -rf "/Applications/$package_app_name"
+# For each package id, get all .app folders associated with the package and remove them.
+for pkg_id in "${pkg_ids[@]}"
+do
+  # Get volume and location of the package.
+  volume=$(pkgutil --pkg-info "$pkg_id" | grep -i "volume" | awk '{if (NF>1) print $NF}')
+  location=$(pkgutil --pkg-info "$pkg_id" | grep -i "location" | awk '{if (NF>1) print $NF}')
+  # Check if this package id corresponds to a valid/installed package
+  if [[ ! -z "$volume" ]]; then
+    # Remove individual directories that end with ".app" belonging to the package.
+    # Only process directories that end with ".app" to prevent Fleet from removing top level directories.
+    pkgutil --only-dirs --files "$pkg_id" | grep "\.app$" | sed -e 's@^@'"$volume""$location"'/@' | tr '\n' '\0' | xargs -n 1 -0 rm -rf
+    # Remove receipts
+    pkgutil --forget "$pkg_id"
+  else
+    echo "WARNING: volume is empty for package ID $pkg_id"
+  fi
+done

server/datastore/mysql/migrations/tables/20241002104104_UpdateUninstallScript.go

@@ -44,15 +44,13 @@ func Up_20241002104104(tx *sql.Tx) error {
 
 	// Get script ids for uninstall scripts from software_installers platform = "darwin" and extension = "pkg"
 	getUninstallScriptIDs := `
-	SELECT si.id, si.uninstall_script_content_id, st.name
-	FROM software_installers si
-	JOIN software_titles st ON si.title_id = st.id
-	WHERE si.platform = "darwin" AND si.extension = "pkg"
+	SELECT id, uninstall_script_content_id
+	FROM software_installers
+	WHERE platform = "darwin" AND extension = "pkg"
 `
 	type scripts struct {
-		ID              uint   `db:"id"`
-		ScriptContentID uint   `db:"uninstall_script_content_id"`
-		AppName         string `db:"name"`
+		ID              uint `db:"id"`
+		ScriptContentID uint `db:"uninstall_script_content_id"`
 	}
 
 	var uninstallScripts []scripts
@@ -102,8 +100,10 @@ ON DUPLICATE KEY UPDATE
 		// Check if script contents match the regex
 		matches := existingUninstallScript.FindStringSubmatch(contents)
 		if matches != nil {
+			packageIDs := matches[existingUninstallScript.SubexpIndex("packageIDs")]
+
 			// Prepare new script
-			newContents := strings.ReplaceAll(newScript, "$PACKAGE_ID", script.AppName)
+			newContents := strings.ReplaceAll(newScript, "$PACKAGE_ID", fmt.Sprintf("(%s)", packageIDs))
 			// Write new script
 			newID, err := insertScriptContents(newContents)
 			if err != nil {

server/datastore/mysql/migrations/tables/data/uninstall_pkg.sh

@@ -1,18 +1,22 @@
 #!/bin/sh
 
-package_app_name="$PACKAGE_ID"
+# Fleet extracts and saves package IDs.
+pkg_ids=$PACKAGE_ID
 
-# Make sure PACKAGE_ID is not empty.
-if [[ -z "$package_app_name" ]]; then
-  echo "Empty PACKAGE_ID variable."
-  exit 1
-fi
-
-# Make sure the PACKAGE_ID doesn't have "../" or is "."
-if [[ "$package_app_name" == *".."* || "$package_app_name" == "." ]]; then
-  echo "Invalid PACKAGE_ID value."
-  exit 1
-fi
-
-echo "Removing \"/Applications/$package_app_name\"..."
-rm -rf "/Applications/$package_app_name"
+# For each package id, get all .app folders associated with the package and remove them.
+for pkg_id in "${pkg_ids[@]}"
+do
+  # Get volume and location of the package.
+  volume=$(pkgutil --pkg-info "$pkg_id" | grep -i "volume" | awk '{if (NF>1) print $NF}')
+  location=$(pkgutil --pkg-info "$pkg_id" | grep -i "location" | awk '{if (NF>1) print $NF}')
+  # Check if this package id corresponds to a valid/installed package
+  if [[ ! -z "$volume" ]]; then
+    # Remove individual directories that end with ".app" belonging to the package.
+    # Only process directories that end with ".app" to prevent Fleet from removing top level directories.
+    pkgutil --only-dirs --files "$pkg_id" | grep "\.app$" | sed -e 's@^@'"$volume""$location"'/@' | tr '\n' '\0' | xargs -n 1 -0 rm -rf
+    # Remove receipts
+    pkgutil --forget "$pkg_id"
+  else
+    echo "WARNING: volume is empty for package ID $pkg_id"
+  fi
+done

server/service/integration_enterprise_test.go

@@ -11593,7 +11593,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerHostRequests() {
 	assert.NotEmpty(t, respTitle.SoftwareTitle.SoftwarePackage.InstallScript)
 	assert.NotEmpty(t, respTitle.SoftwareTitle.SoftwarePackage.UninstallScript)
 	assert.NotContains(t, respTitle.SoftwareTitle.SoftwarePackage.UninstallScript, "$PACKAGE_ID")
-	assert.Contains(t, respTitle.SoftwareTitle.SoftwarePackage.UninstallScript, "\"DummyApp.app\"")
+	assert.Contains(t, respTitle.SoftwareTitle.SoftwarePackage.UninstallScript, "com.example.dummy")
 
 	// install/uninstall request fails for the wrong platform
 	s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", h.ID, pkgTitleID), nil, http.StatusBadRequest, &resp)