diff --git a/server/datastore/mysql/in_house_apps.go b/server/datastore/mysql/in_house_apps.go index 4342f6e09c..a7f080dbbe 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 a4911d2bc3..0d799c510a 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -3674,10 +3674,38 @@ func (ds *Datastore) checkSoftwareConflictsByIdentifier(ctx context.Context, pay } } - if payload.Platform == "windows" { - exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.Title, payload.Platform, softwareTypeInstaller) + // 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) 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. + // 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 for title identifier") + 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 af67054fc7..7cf3fad07c 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,109 @@ 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(), "already has an installer") + }) + + // ---- 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(), "already has an installer") + }) + + // ---- 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(), "already has an installer") + }) + + // ---- 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(), "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 + 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() diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 8140f55856..ad60b8200d 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))