From 03fe9df5339e94a058ec31d1be93aa8e864f5a20 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Thu, 9 Apr 2026 19:21:57 -0500 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=A4=96=20Fix=20installer=20dedupe=20f?= =?UTF-8?q?or=20software=20that=20doesn't=20have=20a=20bundle=20ID?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt below (Android apps were determined not to be affected): It looks like we aren't properly deduplicating Windows and Linux (e.g. tarball, EXE) package-based installers on add, causing multiple installers to be add-able on top of each other, and deletions of one to reveal the previously uploaded installer rather than going to zero installers. Diagnose when this issue started and why it happened. Then build a fix that works cross-platform. Looking for instances of `CantAddSoftwareConflictMessage` in the server-side codebase may help triage this. This might also be an issue with Android VPP apps; check those too. --- server/datastore/mysql/software_installers.go | 29 +++++- .../mysql/software_installers_test.go | 92 +++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index a4911d2bc35..05e6585e8a6 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -3674,10 +3674,35 @@ func (ds *Datastore) checkSoftwareConflictsByIdentifier(ctx context.Context, pay } } - if payload.Platform == "windows" { + // For platforms/installers without a bundle identifier, the DB unique + // constraint (global_or_team_id, title_id, version) is too loose to + // prevent duplicates (it was relaxed for FMA version rollback). We + // need explicit checks that mirror the unique_identifier generated + // column: COALESCE(bundle_identifier, application_id, + // NULLIF(upgrade_code,''), name). + // + // Windows MSI installers surface an upgrade_code that becomes the + // unique_identifier, so we must check by upgrade_code (not title). + // EXE/Linux/macOS-pkg-without-bundle-id all fall back to name. + if payload.BundleIdentifier == "" { + // 1. If the installer carries an UpgradeCode (Windows MSI), check + // by that value because unique_identifier = upgrade_code. + if payload.UpgradeCode != "" { + exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.UpgradeCode, payload.Platform, softwareTypeInstaller) + if err != nil { + return ctxerr.Wrap(ctx, err, "check if installer exists by upgrade code") + } + if exists { + return alreadyExists("installer", payload.Title) + } + } + + // 2. Always check by title (= name), which is the final + // COALESCE fallback and covers EXE, DEB, RPM, tar.gz, + // pkg-without-bundle-id, and MSI without an upgrade code. exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.Title, payload.Platform, softwareTypeInstaller) if err != nil { - return ctxerr.Wrap(ctx, err, "check if installer exists for title identifier") + return ctxerr.Wrap(ctx, err, "check if installer exists by title") } if exists { return alreadyExists("installer", payload.Title) diff --git a/server/datastore/mysql/software_installers_test.go b/server/datastore/mysql/software_installers_test.go index af67054fc7d..4a2f2906cbc 100644 --- a/server/datastore/mysql/software_installers_test.go +++ b/server/datastore/mysql/software_installers_test.go @@ -58,6 +58,7 @@ func TestSoftwareInstallers(t *testing.T) { {"AddSoftwareTitleToMatchingSoftware", testAddSoftwareTitleToMatchingSoftware}, {"FleetMaintainedAppInstallerUpdates", testFleetMaintainedAppInstallerUpdates}, {"RepointCustomPackagePolicyToNewInstaller", testRepointPolicyToNewInstaller}, + {"MatchOrCreateSoftwareInstallerCrossPlatformDedup", testMatchOrCreateSoftwareInstallerCrossPlatformDedup}, } for _, c := range cases { @@ -4144,6 +4145,97 @@ func testSoftwareTitleDisplayName(t *testing.T, ds *Datastore) { require.Contains(t, names, "ipa_foo") } +func testMatchOrCreateSoftwareInstallerCrossPlatformDedup(t *testing.T, ds *Datastore) { + ctx := context.Background() + user := test.NewUser(t, ds, "Alice", "alice-dedup@example.com", true) + team, err := ds.NewTeam(ctx, &fleet.Team{Name: "dedup-team"}) + require.NoError(t, err) + + mkPayload := func(title, ext, platform, source, version, bundleID, upgradeCode, storageID string) *fleet.UploadSoftwareInstallerPayload { + tfr, err := fleet.NewTempFileReader(strings.NewReader(storageID), t.TempDir) + require.NoError(t, err) + return &fleet.UploadSoftwareInstallerPayload{ + InstallerFile: tfr, + Title: title, + Extension: ext, + Platform: platform, + Source: source, + Version: version, + BundleIdentifier: bundleID, + UpgradeCode: upgradeCode, + StorageID: storageID, + Filename: title + "." + ext, + InstallScript: "echo install", + UninstallScript: "echo uninstall", + UserID: user.ID, + TeamID: &team.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + } + } + + // ---- Linux .deb ---- + t.Run("linux_deb_dedup", func(t *testing.T) { + p1 := mkPayload("my-deb-pkg", "deb", "linux", "deb_packages", "1.0", "", "", "deb-hash-1") + _, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, p1) + require.NoError(t, err) + + // Same title, different version → should be blocked by conflict check + p2 := mkPayload("my-deb-pkg", "deb", "linux", "deb_packages", "2.0", "", "", "deb-hash-2") + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) + require.Error(t, err, "adding a second .deb with the same title should fail") + assert.Contains(t, err.Error(), fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, "my-deb-pkg", team.Name)) + }) + + // ---- Linux .rpm ---- + t.Run("linux_rpm_dedup", func(t *testing.T) { + p1 := mkPayload("my-rpm-pkg", "rpm", "linux", "rpm_packages", "1.0", "", "", "rpm-hash-1") + _, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, p1) + require.NoError(t, err) + + p2 := mkPayload("my-rpm-pkg", "rpm", "linux", "rpm_packages", "2.0", "", "", "rpm-hash-2") + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) + require.Error(t, err, "adding a second .rpm with the same title should fail") + assert.Contains(t, err.Error(), fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, "my-rpm-pkg", team.Name)) + }) + + // ---- Windows .exe ---- + t.Run("windows_exe_dedup", func(t *testing.T) { + p1 := mkPayload("My App", "exe", "windows", "programs", "1.0", "", "", "exe-hash-1") + _, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, p1) + require.NoError(t, err) + + p2 := mkPayload("My App", "exe", "windows", "programs", "2.0", "", "", "exe-hash-2") + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) + require.Error(t, err, "adding a second .exe with the same title should fail") + assert.Contains(t, err.Error(), fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, "My App", team.Name)) + }) + + // ---- Windows .msi with UpgradeCode ---- + t.Run("windows_msi_upgrade_code_dedup", func(t *testing.T) { + p1 := mkPayload("MSI App", "msi", "windows", "programs", "1.0", "", "{UPGRADE-CODE-1}", "msi-hash-1") + _, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, p1) + require.NoError(t, err) + + // Same upgrade code but different title and version → should still be blocked + p2 := mkPayload("MSI App Renamed", "msi", "windows", "programs", "2.0", "", "{UPGRADE-CODE-1}", "msi-hash-2") + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) + require.Error(t, err, "adding a second .msi with the same upgrade code should fail") + assert.Contains(t, err.Error(), fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, "MSI App Renamed", team.Name)) + }) + + // ---- Cross-platform: different platforms should NOT conflict ---- + t.Run("different_platforms_no_conflict", func(t *testing.T) { + // A Linux package and a Windows package with the same title should not conflict + p1 := mkPayload("cross-plat-app", "deb", "linux", "deb_packages", "1.0", "", "", "cross-hash-1") + _, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, p1) + require.NoError(t, err) + + p2 := mkPayload("cross-plat-app", "exe", "windows", "programs", "1.0", "", "", "cross-hash-2") + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) + require.NoError(t, err, "same title on different platforms should be allowed") + }) +} + func testMatchOrCreateSoftwareInstallerDuplicateHash(t *testing.T, ds *Datastore) { ctx := context.Background() From 10a1e9a1b5e02ff8ab8b677f83003940dd19b1fa Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Thu, 9 Apr 2026 19:58:13 -0500 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A4=96=20Fix=20failure=20caught=20by?= =?UTF-8?q?=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: `TestIntegrationsEnterprise/TestSoftwareInstallerUploadDownloadAndDelete` is failing. Fix either the test if it's wrong or the code if it's wrong. --- server/datastore/mysql/software_installers_test.go | 8 ++++---- server/service/integration_enterprise_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/datastore/mysql/software_installers_test.go b/server/datastore/mysql/software_installers_test.go index 4a2f2906cbc..8ea33a46315 100644 --- a/server/datastore/mysql/software_installers_test.go +++ b/server/datastore/mysql/software_installers_test.go @@ -4183,7 +4183,7 @@ func testMatchOrCreateSoftwareInstallerCrossPlatformDedup(t *testing.T, ds *Data p2 := mkPayload("my-deb-pkg", "deb", "linux", "deb_packages", "2.0", "", "", "deb-hash-2") _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) require.Error(t, err, "adding a second .deb with the same title should fail") - assert.Contains(t, err.Error(), fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, "my-deb-pkg", team.Name)) + assert.Contains(t, err.Error(), "already has an installer") }) // ---- Linux .rpm ---- @@ -4195,7 +4195,7 @@ func testMatchOrCreateSoftwareInstallerCrossPlatformDedup(t *testing.T, ds *Data p2 := mkPayload("my-rpm-pkg", "rpm", "linux", "rpm_packages", "2.0", "", "", "rpm-hash-2") _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) require.Error(t, err, "adding a second .rpm with the same title should fail") - assert.Contains(t, err.Error(), fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, "my-rpm-pkg", team.Name)) + assert.Contains(t, err.Error(), "already has an installer") }) // ---- Windows .exe ---- @@ -4207,7 +4207,7 @@ func testMatchOrCreateSoftwareInstallerCrossPlatformDedup(t *testing.T, ds *Data p2 := mkPayload("My App", "exe", "windows", "programs", "2.0", "", "", "exe-hash-2") _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) require.Error(t, err, "adding a second .exe with the same title should fail") - assert.Contains(t, err.Error(), fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, "My App", team.Name)) + assert.Contains(t, err.Error(), "already has an installer") }) // ---- Windows .msi with UpgradeCode ---- @@ -4220,7 +4220,7 @@ func testMatchOrCreateSoftwareInstallerCrossPlatformDedup(t *testing.T, ds *Data p2 := mkPayload("MSI App Renamed", "msi", "windows", "programs", "2.0", "", "{UPGRADE-CODE-1}", "msi-hash-2") _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) require.Error(t, err, "adding a second .msi with the same upgrade code should fail") - assert.Contains(t, err.Error(), fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, "MSI App Renamed", team.Name)) + assert.Contains(t, err.Error(), "already has an installer") }) // ---- Cross-platform: different platforms should NOT conflict ---- diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 8140f558562..ad60b8200de 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -12554,7 +12554,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD s.lastActivityMatches(fleet.ActivityTypeAddedSoftware{}.ActivityName(), activityData, 0) // upload again fails - s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already exists") + s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already has an installer") // update should succeed s.updateSoftwareInstaller(t, &fleet.UpdateSoftwareInstallerPayload{ @@ -12759,7 +12759,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD s.lastActivityOfTypeMatches(fleet.ActivityTypeAddedSoftware{}.ActivityName(), activityData, 0) // upload again fails - s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already exists") + s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already has an installer") // download the installer r := s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package?alt=media", titleID), nil, http.StatusOK, "team_id", fmt.Sprintf("%d", *payload.TeamID)) @@ -12872,7 +12872,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD fmt.Sprintf(`{"software_title": "ruby", "software_package": "ruby.deb", "team_name": null, "team_id": 0, "fleet_name": null, "fleet_id": 0, "self_service": true, "software_title_id": %d}`, titleID), 0) // upload again fails - s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already exists") + s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already has an installer") // download the installer r := s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package?alt=media", titleID), nil, http.StatusOK, "team_id", fmt.Sprintf("%d", 0)) From 7db4385f55a9ed4987a191f042403fcbf77e542a Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Fri, 10 Apr 2026 00:46:36 -0500 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=A4=96=20Fix=20more=20test=20failures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zed + Opus 4.6; prompt: Fix failing `TestIntegrationsEnterprise/TestAutomaticPolicies` and `https://github.com/fleetdm/fleet/actions/runs/24220989052/job/70712227718?pr=43385` tests --- server/datastore/mysql/in_house_apps.go | 30 +++++++++++++++++++ server/datastore/mysql/software_installers.go | 11 ++++--- .../mysql/software_installers_test.go | 12 ++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/server/datastore/mysql/in_house_apps.go b/server/datastore/mysql/in_house_apps.go index 4342f6e09c5..a7f080dbbe9 100644 --- a/server/datastore/mysql/in_house_apps.go +++ b/server/datastore/mysql/in_house_apps.go @@ -1548,6 +1548,36 @@ WHERE return exists == 1, nil } +// checkInstallerExistsByTitleAndSource checks whether a software installer +// already exists for the given title name, source, and platform within a team. +// This is used for the name-based fallback dedup (when there is no +// bundle_identifier or upgrade_code) so that packages with the same name but +// different sources (e.g. ruby from deb_packages vs ruby from rpm_packages) +// are not treated as duplicates. +func (ds *Datastore) checkInstallerExistsByTitleAndSource(ctx context.Context, q sqlx.QueryerContext, teamID *uint, title, source, platform string) (bool, error) { + const stmt = ` +SELECT 1 +FROM + software_titles st + INNER JOIN software_installers ON st.id = software_installers.title_id AND software_installers.global_or_team_id = ? +WHERE + st.name = ? + AND st.source = ? + AND software_installers.platform = ? +` + + var globalOrTeamID uint + if teamID != nil { + globalOrTeamID = *teamID + } + var exists int + err := sqlx.GetContext(ctx, q, &exists, stmt, globalOrTeamID, title, source, platform) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return false, ctxerr.Wrap(ctx, err, "check installer exists by title and source") + } + return exists == 1, nil +} + func (ds *Datastore) checkInHouseAppExistsForAdamID(ctx context.Context, q sqlx.QueryerContext, teamID *uint, appID fleet.VPPAppID) (exists bool, title string, err error) { const stmt = ` SELECT st.name diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index 05e6585e8a6..0d799c510ac 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -3697,12 +3697,15 @@ func (ds *Datastore) checkSoftwareConflictsByIdentifier(ctx context.Context, pay } } - // 2. Always check by title (= name), which is the final - // COALESCE fallback and covers EXE, DEB, RPM, tar.gz, + // 2. Always check by title (= name) AND source, which is the + // final COALESCE fallback and covers EXE, DEB, RPM, tar.gz, // pkg-without-bundle-id, and MSI without an upgrade code. - exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.Title, payload.Platform, softwareTypeInstaller) + // We include source so that packages with the same name but + // different sources (e.g. ruby from deb_packages vs ruby + // from rpm_packages) are not treated as duplicates. + exists, err := ds.checkInstallerExistsByTitleAndSource(ctx, ds.reader(ctx), payload.TeamID, payload.Title, payload.Source, payload.Platform) if err != nil { - return ctxerr.Wrap(ctx, err, "check if installer exists by title") + return ctxerr.Wrap(ctx, err, "check if installer exists by title and source") } if exists { return alreadyExists("installer", payload.Title) diff --git a/server/datastore/mysql/software_installers_test.go b/server/datastore/mysql/software_installers_test.go index 8ea33a46315..7cf3fad07c6 100644 --- a/server/datastore/mysql/software_installers_test.go +++ b/server/datastore/mysql/software_installers_test.go @@ -4223,6 +4223,18 @@ func testMatchOrCreateSoftwareInstallerCrossPlatformDedup(t *testing.T, ds *Data assert.Contains(t, err.Error(), "already has an installer") }) + // ---- Same platform, different sources (.deb vs .rpm) should NOT conflict ---- + t.Run("same_platform_different_sources_no_conflict", func(t *testing.T) { + // A .deb and .rpm with the same title are different packages (different sources) + p1 := mkPayload("ruby", "deb", "linux", "deb_packages", "1.0", "", "", "ruby-deb-hash-1") + _, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, p1) + require.NoError(t, err) + + p2 := mkPayload("ruby", "rpm", "linux", "rpm_packages", "1.0", "", "", "ruby-rpm-hash-1") + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, p2) + require.NoError(t, err, "same title with different sources (deb vs rpm) should be allowed") + }) + // ---- Cross-platform: different platforms should NOT conflict ---- t.Run("different_platforms_no_conflict", func(t *testing.T) { // A Linux package and a Windows package with the same title should not conflict