Add filter to default unintaller for pkgs to only remove .app folders (…

…#22585)

#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] Manual QA for all new/changed functionality

changes/22571-fix-pkg-uninstall

@@ -0,0 +1 @@
+* Fixed software uninstaller script for `pkg`s to only remove '.app' directories installed by the package.

ee/server/service/software_installers.go

@@ -105,17 +105,13 @@ func preProcessUninstallScript(payload *fleet.UploadSoftwareInstallerPayload) {
 	var packageID string
 	switch payload.Extension {
 	case "pkg":
-		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()
+		// 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)
 	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()
-	var input = `
+	input := `
 blah$PACKAGE_IDS
 pkgids=$PACKAGE_ID
 they are $PACKAGE_ID, right $MY_SECRET?
@@ -42,39 +42,21 @@ 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=(
-  "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"
-)`
+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"`
 	assert.Equal(t, expected, payload.UninstallScript)
-
 }
 
 func TestInstallUninstallAuth(t *testing.T) {
@@ -93,7 +75,8 @@ 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",
@@ -104,14 +87,16 @@ 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
@@ -197,7 +182,6 @@ 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,21 +1,18 @@
 #!/bin/sh
 
-# Fleet extracts and saves package IDs.
-pkg_ids=$PACKAGE_ID
+package_app_name="$PACKAGE_ID"
 
-# Get all files associated with package and remove them
-for pkg_id in "${pkg_ids[@]}"
-do
-  # Get volume and location of package
-  volume=$(pkgutil --pkg-info "$pkg_id" | grep -i "volume" | awk '{for (i=2; i<NF; i++) printf $i " "; print $NF}')
-  location=$(pkgutil --pkg-info "$pkg_id" | grep -i "location" | awk '{for (i=2; i<NF; i++) printf $i " "; print $NF}')
-  # Check if this package id corresponds to a valid/installed package
-  if [[ ! -z "$volume" && ! -z "$location" ]]; then
-    # Remove individual files/directories belonging to package
-    pkgutil --files "$pkg_id" | sed -e 's@^@'"$volume""$location"'/@' | tr '\n' '\0' | xargs -n 1 -0 rm -rf
-    # Remove receipts
-    pkgutil --forget "$pkg_id"
-  else
-    echo "WARNING: volume or location are empty for package ID $pkg_id"
-  fi
-done
+# 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"

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

@@ -1,21 +1,18 @@
 #!/bin/sh
 
-# Fleet extracts and saves package IDs.
-pkg_ids=$PACKAGE_ID
+package_app_name="$PACKAGE_ID"
 
-# Get all files associated with package and remove them
-for pkg_id in "${pkg_ids[@]}"
-do
-  # Get volume and location of package
-  volume=$(pkgutil --pkg-info "$pkg_id" | grep -i "volume" | awk '{for (i=2; i<NF; i++) printf $i " "; print $NF}')
-  location=$(pkgutil --pkg-info "$pkg_id" | grep -i "location" | awk '{for (i=2; i<NF; i++) printf $i " "; print $NF}')
-  # Check if this package id corresponds to a valid/installed package
-  if [[ ! -z "$volume" && ! -z "$location" ]]; then
-    # Remove individual files/directories belonging to package
-    pkgutil --files "$pkg_id" | sed -e 's@^@'"$volume""$location"'/@' | tr '\n' '\0' | xargs -n 1 -0 rm -rf
-    # Remove receipts
-    pkgutil --forget "$pkg_id"
-  else
-    echo "WARNING: volume or location are empty for package ID $pkg_id"
-  fi
-done
+# 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"

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, "com.example.dummy")
+	assert.Contains(t, respTitle.SoftwareTitle.SoftwarePackage.UninstallScript, "\"DummyApp.app\"")
 
 	// 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)