From 856830b7ca5dc2f7afac7ed1e4e669e1e670bf9c Mon Sep 17 00:00:00 2001 From: Jonathan Katz <44128041+jkatz01@users.noreply.github.com> Date: Tue, 7 Apr 2026 15:58:29 -0400 Subject: [PATCH] Delete unnecessary patch policies in batch set software installers (#43112) **Related issue:** Resolves #42991 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [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/guides/committing-changes.md#changes-files) for more information. - [ ] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [ ] Timeouts are implemented and retries are limited to avoid infinite loops - [ ] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [ ] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually --- changes/42991-patch-policy-gitops-bug | 1 + server/datastore/mysql/software_installers.go | 28 ++++++++ server/service/integration_enterprise_test.go | 72 +++++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 changes/42991-patch-policy-gitops-bug diff --git a/changes/42991-patch-policy-gitops-bug b/changes/42991-patch-policy-gitops-bug new file mode 100644 index 0000000000..e584e8a95b --- /dev/null +++ b/changes/42991-patch-policy-gitops-bug @@ -0,0 +1 @@ +- Fixed bug where adding a patch policy for a new installer in the UI caused gitops runs that didn't include that installer to fail. diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index 1cb6ef7280..a4911d2bc3 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -2112,6 +2112,14 @@ WHERE team_id = ? ` + const deleteAllPatchPolicies = ` +DELETE FROM + policies +WHERE + team_id = ? AND + type = 'patch' +` + const deleteAllPendingUninstallScriptExecutions = ` DELETE FROM host_script_results WHERE execution_id IN ( SELECT execution_id FROM host_software_installs WHERE status = 'pending_uninstall' @@ -2256,6 +2264,14 @@ WHERE ) ` + const deletePatchPoliciesWithInstallersNotInList = ` +DELETE FROM + policies +WHERE + team_id = ? AND + patch_software_title_id NOT IN (?) +` + const countInstallDuringSetupNotInList = ` SELECT COUNT(*) @@ -2486,6 +2502,10 @@ WHERE return ctxerr.Wrap(ctx, err, "unset all obsolete installers in policies") } + if _, err := tx.ExecContext(ctx, deleteAllPatchPolicies, globalOrTeamID); err != nil { + return ctxerr.Wrap(ctx, err, "delete all obsolete patch policies") + } + if _, err := tx.ExecContext(ctx, deleteAllPendingUninstallScriptExecutions, globalOrTeamID); err != nil { return ctxerr.Wrap(ctx, err, "delete all pending uninstall script executions") } @@ -2586,6 +2606,14 @@ WHERE return ctxerr.Wrap(ctx, err, "unset obsolete software installers from policies") } + stmt, args, err = sqlx.In(deletePatchPoliciesWithInstallersNotInList, globalOrTeamID, titleIDs) + if err != nil { + return ctxerr.Wrap(ctx, err, "build statement to delete obsolete patch policies") + } + if _, err := tx.ExecContext(ctx, stmt, args...); err != nil { + return ctxerr.Wrap(ctx, err, "delete obsolete patch policies") + } + // check if any in the list are install_during_setup, fail if there is one if !replacingInstallDuringSetup { stmt, args, err = sqlx.In(countInstallDuringSetupNotInList, globalOrTeamID, titleIDs) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index aad3c67c14..2848c626e7 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -28184,3 +28184,75 @@ func (s *integrationEnterpriseTestSuite) TestPinMajorVersion() { ) }) } + +func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersDeletesObsoletePatchPolicy() { + t := s.T() + + team, err := s.ds.NewTeam(context.Background(), &fleet.Team{Name: "team_" + t.Name()}) + require.NoError(t, err) + + // Set up two FMAs: zoom/windows and 1password/darwin. + states := make(map[string]*fmaTestState, 2) + states["/zoom/windows.json"] = &fmaTestState{ + version: "1.0", + installerBytes: []byte("xyz"), + installerPath: "/zoom.msi", + } + states["/1password/darwin.json"] = &fmaTestState{ + version: "1.0", + installerBytes: []byte("abc"), + installerPath: "/1password.pkg", + } + startFMAServers(t, s.ds, states) + + // Setup: batch set only zoom/windows. + var resp batchSetSoftwareInstallersResponse + s.DoJSON("POST", "/api/latest/fleet/software/batch", + batchSetSoftwareInstallersRequest{Software: []*fleet.SoftwareInstallerPayload{{Slug: ptr.String("zoom/windows")}}, TeamName: team.Name}, + http.StatusAccepted, &resp, + "team_name", team.Name, "team_id", fmt.Sprint(team.ID), + ) + waitBatchSetSoftwareInstallersCompleted(t, &s.withServer, team.Name, resp.RequestUUID) + + // Batch set both zoom/windows and 1password/darwin. + s.DoJSON("POST", "/api/latest/fleet/software/batch", + batchSetSoftwareInstallersRequest{Software: []*fleet.SoftwareInstallerPayload{ + {Slug: ptr.String("zoom/windows")}, + {Slug: ptr.String("1password/darwin")}, + }, TeamName: team.Name}, + http.StatusAccepted, &resp, + "team_name", team.Name, "team_id", fmt.Sprint(team.ID), + ) + waitBatchSetSoftwareInstallersCompleted(t, &s.withServer, team.Name, resp.RequestUUID) + + // Create a patch policy for 1password/darwin. + applyResp := fleet.ApplyPolicySpecsResponse{} + s.DoJSON("POST", "/api/latest/fleet/spec/policies", + fleet.ApplyPolicySpecsRequest{Specs: []*fleet.PolicySpec{{ + Name: "1password patch policy", + Team: team.Name, + Type: fleet.PolicyTypePatch, + FleetMaintainedAppSlug: "1password/darwin", + }}}, + http.StatusOK, &applyResp, + ) + + // Verify the patch policy exists. + listPolResp := fleet.ListTeamPoliciesResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/fleets/%d/policies", team.ID), fleet.ListTeamPoliciesRequest{}, http.StatusOK, &listPolResp, "page", "0") + require.Len(t, listPolResp.Policies, 1) + require.Equal(t, fleet.PolicyTypePatch, listPolResp.Policies[0].Type) + + // Batch set only zoom/windows — should not fail and should delete the obsolete patch policy. + s.DoJSON("POST", "/api/latest/fleet/software/batch", + batchSetSoftwareInstallersRequest{Software: []*fleet.SoftwareInstallerPayload{{Slug: ptr.String("zoom/windows")}}, TeamName: team.Name}, + http.StatusAccepted, &resp, + "team_name", team.Name, "team_id", fmt.Sprint(team.ID), + ) + waitBatchSetSoftwareInstallersCompleted(t, &s.withServer, team.Name, resp.RequestUUID) + + // Verify the patch policy was deleted. + listPolResp = fleet.ListTeamPoliciesResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/fleets/%d/policies", team.ID), fleet.ListTeamPoliciesRequest{}, http.StatusOK, &listPolResp, "page", "0") + require.Empty(t, listPolResp.Policies) +}