From 7b950c64a677985187bc60abe58e4422eaf2fcc9 Mon Sep 17 00:00:00 2001 From: Jonathan Katz <44128041+jkatz01@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:08:23 -0400 Subject: [PATCH] Add duplicate patch policy check to GitOps (#48896) **Related issue:** Resolves #46193 Adds a client-side check for duplicate patch policies, similar to the existing policy name and label duplicate checks. # 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 Adding two patch policies for the same fma slug results in this error: ``` Error: 1 error occurred: * Couldn't add multiple policies with type "patch" for "fleet_maintained_app_slug": "google-chrome/darwin". ``` ## Summary by CodeRabbit * **New Features** * Added validation to GitOps application checks to prevent multiple patch policies from targeting the same app slug. * Improved error reporting when patch policy slugs are duplicated or missing from the configured app list. * **Bug Fixes** * Prevented duplicate patch policies from being silently accepted, reducing the risk of one policy overwriting another. * Existing valid combinations, such as different patch slugs or certain mixed policy types, continue to work as expected. --- .../46193-duplicate-patch-policy-gitops-check | 1 + pkg/spec/gitops.go | 7 + pkg/spec/gitops_test.go | 150 ++++++++++++++++++ 3 files changed, 158 insertions(+) create mode 100644 changes/46193-duplicate-patch-policy-gitops-check diff --git a/changes/46193-duplicate-patch-policy-gitops-check b/changes/46193-duplicate-patch-policy-gitops-check new file mode 100644 index 0000000000..589f6d7cef --- /dev/null +++ b/changes/46193-duplicate-patch-policy-gitops-check @@ -0,0 +1 @@ +- Added a check for duplicate patch policies when applying GitOps. diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index bb3856da41..5b08cca5a7 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -1797,6 +1797,7 @@ func parsePolicies(top map[string]json.RawMessage, result *GitOps, baseDir strin } } // Make sure team name is correct, and do additional validation + var patchSlugs []string for _, item := range result.Policies { if item.Name == "" { multiError = multierror.Append(multiError, errors.New("policy name is required for each policy")) @@ -1820,6 +1821,9 @@ func parsePolicies(top map[string]json.RawMessage, result *GitOps, baseDir strin ), ) } + if item.FleetMaintainedAppSlug != "" { + patchSlugs = append(patchSlugs, item.FleetMaintainedAppSlug) + } } if result.TeamName != nil { item.Team = *result.TeamName @@ -1838,6 +1842,9 @@ func parsePolicies(top map[string]json.RawMessage, result *GitOps, baseDir strin if len(duplicates) > 0 { multiError = multierror.Append(multiError, fmt.Errorf("duplicate policy names: %v", duplicates)) } + for _, slug := range getDuplicateNames(patchSlugs, func(s string) string { return s }) { + multiError = multierror.Append(multiError, fmt.Errorf(`Couldn't add multiple policies with type "patch" for "fleet_maintained_app_slug": %q.`, slug)) + } return multiError } diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index e28f531744..45b3c66be4 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -4863,3 +4863,153 @@ func TestGitOpsFMACategoriesPresence(t *testing.T) { assert.Equal(t, []string{"somevalue"}, cats.Value) }) } + +func TestDuplicatePatchPolicySlug(t *testing.T) { + t.Parallel() + + // Every slug referenced by a patch policy must be declared under software.fleet_maintained_apps. + fmaSoftware := ` +software: + fleet_maintained_apps: + - slug: google-chrome/darwin + - slug: 1password/darwin + - slug: firefox/darwin +` + + tests := []struct { + name string + policies string + // wantErrs empty means the config must apply cleanly. + wantErrs []string + }{ + { + // Before this check the second patch policy silently overwrote the first. + name: "two patch policies with the same slug", + policies: ` +policies: + - name: Chrome up to date + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin + - name: Chrome up to date again + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin +`, + wantErrs: []string{`Couldn't add multiple policies with type "patch" for "fleet_maintained_app_slug": "google-chrome/darwin".`}, + }, + { + // Each duplicated slug gets its own error, driven by the slug in the config. + name: "two slugs each duplicated report one error per slug", + policies: ` +policies: + - name: Chrome up to date + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin + - name: Chrome up to date again + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin + - name: 1Password up to date + type: patch + platform: darwin + fleet_maintained_app_slug: 1password/darwin + - name: 1Password up to date again + type: patch + platform: darwin + fleet_maintained_app_slug: 1password/darwin +`, + wantErrs: []string{ + `Couldn't add multiple policies with type "patch" for "fleet_maintained_app_slug": "google-chrome/darwin".`, + `Couldn't add multiple policies with type "patch" for "fleet_maintained_app_slug": "1password/darwin".`, + }, + }, + { + // A slug used by three patch policies is still reported a single time. + name: "slug used three times is reported once", + policies: ` +policies: + - name: Chrome A + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin + - name: Chrome B + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin + - name: Chrome C + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin +`, + wantErrs: []string{`Couldn't add multiple policies with type "patch" for "fleet_maintained_app_slug": "google-chrome/darwin".`}, + }, + { + // Duplicate names and duplicate patch slug surface together. + name: "duplicate names and duplicate patch slug both reported", + policies: ` +policies: + - name: Same name + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin + - name: Same name + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin +`, + wantErrs: []string{ + "duplicate policy names", + `Couldn't add multiple policies with type "patch" for "fleet_maintained_app_slug": "google-chrome/darwin".`, + }, + }, + { + // A dynamic install_software policy and a patch policy may share a slug. + name: "dynamic install_software and patch with the same slug is allowed", + policies: ` +policies: + - name: Chrome installed + platform: darwin + query: SELECT 1 FROM apps WHERE bundle_identifier = 'com.google.Chrome'; + install_software: + fleet_maintained_app_slug: google-chrome/darwin + - name: Chrome up to date + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin +`, + }, + { + name: "two patch policies with different slugs is allowed", + policies: ` +policies: + - name: Chrome up to date + type: patch + platform: darwin + fleet_maintained_app_slug: google-chrome/darwin + - name: Firefox up to date + type: patch + platform: darwin + fleet_maintained_app_slug: firefox/darwin +`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + config := getTeamConfig([]string{"policies"}) + fmaSoftware + tc.policies + path, basePath := createTempFile(t, "", config) + _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) + if len(tc.wantErrs) == 0 { + require.NoError(t, err) + return + } + require.Error(t, err) + for _, want := range tc.wantErrs { + assert.ErrorContains(t, err, want) + } + }) + } +}