diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 8660e9dbb6f5..ac4461a592b2 100644 --- a/ee/server/service/software_installers.go +++ b/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)) } diff --git a/ee/server/service/software_installers_test.go b/ee/server/service/software_installers_test.go index 54c7712d83e0..6abd95085eb5 100644 --- a/ee/server/service/software_installers_test.go +++ b/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,7 +42,6 @@ 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"}, @@ -50,13 +49,32 @@ quotes and braces for "com.foo" 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) { diff --git a/pkg/file/scripts/uninstall_pkg.sh b/pkg/file/scripts/uninstall_pkg.sh index 3bddaaf57fd7..0096d3f48da8 100644 --- a/pkg/file/scripts/uninstall_pkg.sh +++ b/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" \ No newline at end of file +# 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 diff --git a/pkg/file/testdata/scripts/uninstall_pkg.sh.golden b/pkg/file/testdata/scripts/uninstall_pkg.sh.golden index 3bddaaf57fd7..0096d3f48da8 100644 --- a/pkg/file/testdata/scripts/uninstall_pkg.sh.golden +++ b/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" \ No newline at end of file +# 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 diff --git a/server/datastore/mysql/migrations/tables/20241002104104_UpdateUninstallScript.go b/server/datastore/mysql/migrations/tables/20241002104104_UpdateUninstallScript.go index 6f2833751e37..99b6dac45ec9 100644 --- a/server/datastore/mysql/migrations/tables/20241002104104_UpdateUninstallScript.go +++ b/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 { diff --git a/server/datastore/mysql/migrations/tables/data/uninstall_pkg.sh b/server/datastore/mysql/migrations/tables/data/uninstall_pkg.sh index 3bddaaf57fd7..0096d3f48da8 100644 --- a/server/datastore/mysql/migrations/tables/data/uninstall_pkg.sh +++ b/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" \ No newline at end of file +# 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 diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 14e036a01f2a..3d6673afd3b6 100644 --- a/server/service/integration_enterprise_test.go +++ b/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)