Added software_titles unique index idx_unique_sw_titles

changes/25235-software-titles-uniqueness

@@ -0,0 +1 @@
+* Fixed a bug where only the first of multiple software titles with the same name and source but different bundle IDs would be successfully inserted into the database.

server/datastore/mysql/migrations/tables/20250124194347_UpdateSoftwareTitlesUniqueIndex.go

@@ -0,0 +1,40 @@
+package tables
+
+import (
+	"database/sql"
+	"fmt"
+)
+
+func init() {
+	MigrationClient.AddMigration(Up_20250124194347, Down_20250124194347)
+}
+
+func Up_20250124194347(tx *sql.Tx) error {
+	if _, err := tx.Exec(`
+        ALTER TABLE software_titles
+        ADD COLUMN unique_identifier VARCHAR(255) GENERATED ALWAYS AS (COALESCE(bundle_identifier, name)) VIRTUAL;
+    `); err != nil {
+		return fmt.Errorf("failed to add generated column unique_identifier: %w", err)
+	}
+
+	if _, err := tx.Exec(`
+		ALTER TABLE software_titles
+		ADD UNIQUE INDEX idx_unique_sw_titles (unique_identifier, source, browser);
+	`); err != nil {
+		return fmt.Errorf("failed to add unique index: %w", err)
+	}
+
+	if _, err := tx.Exec(`
+		ALTER TABLE software_titles
+		DROP INDEX idx_sw_titles,
+		ADD INDEX idx_sw_titles (name, source, browser);
+	`); err != nil {
+		return fmt.Errorf("failed to re-add idx_sw_titles: %w", err)
+	}
+
+	return nil
+}
+
+func Down_20250124194347(tx *sql.Tx) error {
+	return nil
+}

server/datastore/mysql/migrations/tables/20250124194347_UpdateSoftwareTitlesUniqueIndex_test.go

@@ -0,0 +1,65 @@
+package tables
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestUp_20250124194347(t *testing.T) {
+	db := applyUpToPrev(t)
+
+	insertSql := `INSERT INTO software_titles (name, source, browser, bundle_identifier) VALUES (?, ?, ?, ?);`
+	_, err := db.Exec(insertSql, "name1", "", "", "com.fleet1")
+	require.NoError(t, err)
+	_, err = db.Exec(insertSql, "name1", "", "", "com.fleet2")
+	require.Error(t, err, "Expected software insert to fail because of unique key")
+
+	applyNext(t, db)
+
+	_, err = db.Exec(insertSql, "name2", "", "", "com.fleetdm1")
+	require.NoError(t, err)
+	_, err = db.Exec(insertSql, "name2", "", "", "com.fleetdm2")
+	require.NoError(t, err, "Expected software insert to succeed")
+
+	insertSql = `INSERT INTO software_titles (name, source, browser, bundle_identifier) VALUES`
+	var valueStrings []string
+	var valueArgs []interface{}
+
+	for i := 0; i < 10; i++ {
+		valueStrings = append(valueStrings, "(?, ?, ?, ?)")
+		source := ""
+		if i%2 == 0 {
+			source = "app"
+		} else {
+			source = ""
+		}
+		valueArgs = append(valueArgs, fmt.Sprintf("name_%d", i), source, "", fmt.Sprintf("bundle_%d", i))
+	}
+	_, err = db.Exec(insertSql+strings.Join(valueStrings, ","), valueArgs...)
+	require.NoError(t, err)
+
+	result := struct {
+		ID           int     `db:"id"`
+		SelectType   string  `db:"select_type"`
+		Table        string  `db:"table"`
+		Type         string  `db:"type"`
+		PossibleKeys *string `db:"possible_keys"`
+		Key          *string `db:"key"`
+		KeyLen       *int    `db:"key_len"`
+		Ref          *string `db:"ref"`
+		Rows         int     `db:"rows"`
+		Filtered     float64 `db:"filtered"`
+		Extra        *string `db:"Extra"`
+		Partitions   *string `db:"partitions"`
+	}{}
+
+	err = db.Get(
+		&result, `EXPLAIN SELECT id from software_titles WHERE name = ? and source = ?`,
+		"name1", "app",
+	)
+	require.NoError(t, err)
+	require.Equal(t, *result.Key, "idx_sw_titles")
+}

server/datastore/mysql/software.go

@@ -411,7 +411,7 @@ func checkForDeletedInstalledSoftware(ctx context.Context, tx sqlx.ExtContext, d
 		for _, d := range deleted {
 			// We don't support installing browser plugins as of 2024/08/22
 			if d.Browser == "" {
-				deletedTitles[UniqueSoftwareTitleStr(d.Name, d.Source, d.BundleIdentifier)] = struct{}{}
+				deletedTitles[UniqueSoftwareTitleStr(BundleIdentifierOrName(d.BundleIdentifier, d.Name), d.Source)] = struct{}{}
 			}
 		}
 		for _, i := range inserted {
@@ -436,7 +436,7 @@ func checkForDeletedInstalledSoftware(ctx context.Context, tx sqlx.ExtContext, d
 			if title.BundleIdentifier != nil {
 				bundleIdentifier = *title.BundleIdentifier
 			}
-			key := UniqueSoftwareTitleStr(title.Name, title.Source, bundleIdentifier)
+			key := UniqueSoftwareTitleStr(BundleIdentifierOrName(bundleIdentifier, title.Name), title.Source)
 			if _, ok := deletedTitles[key]; ok {
 				deletedTitleIDs[title.ID] = deletedValue{vpp: title.VPPAppsCount > 0}
 			}
@@ -547,7 +547,9 @@ func (ds *Datastore) getIncomingSoftwareChecksumsToExistingTitles(
 			argsWithoutBundleIdentifier = append(argsWithoutBundleIdentifier, sw.Name, sw.Source, sw.Browser)
 		}
 		// Map software title identifier to software checksums so that we can map checksums to actual titles later.
-		uniqueTitleStrToChecksum[UniqueSoftwareTitleStr(sw.Name, sw.Source, sw.Browser)] = checksum
+		uniqueTitleStrToChecksum[UniqueSoftwareTitleStr(
+			BundleIdentifierOrName(sw.BundleIdentifier, sw.Name), sw.Source, sw.Browser,
+		)] = checksum
 	}
 
 	// Get titles for software without bundle_identifier.
@@ -593,7 +595,7 @@ func (ds *Datastore) getIncomingSoftwareChecksumsToExistingTitles(
 		}
 		// Map software titles to software checksums.
 		for _, title := range existingSoftwareTitlesForNewSoftwareWithBundleIdentifier {
-			checksum, ok := uniqueTitleStrToChecksum[UniqueSoftwareTitleStr(title.Name, title.Source, title.Browser)]
+			checksum, ok := uniqueTitleStrToChecksum[UniqueSoftwareTitleStr(*title.BundleIdentifier, title.Source, title.Browser)]
 			if ok {
 				incomingChecksumToTitle[checksum] = title
 			}
@@ -603,6 +605,14 @@ func (ds *Datastore) getIncomingSoftwareChecksumsToExistingTitles(
 	return incomingChecksumToTitle, nil
 }
 
+// BundleIdentifierOrName returns the bundle identifier if it is not empty, otherwise name
+func BundleIdentifierOrName(bundleIdentifier, name string) string {
+	if bundleIdentifier != "" {
+		return bundleIdentifier
+	}
+	return name
+}
+
 // UniqueSoftwareTitleStr creates a unique string representation of the software title
 func UniqueSoftwareTitleStr(values ...string) string {
 	return strings.Join(values, fleet.SoftwareFieldSeparator)

server/datastore/mysql/software_test.go

@@ -34,6 +34,8 @@ func TestSoftware(t *testing.T) {
 		{"SaveHost", testSoftwareSaveHost},
 		{"CPE", testSoftwareCPE},
 		{"HostDuplicates", testSoftwareHostDuplicates},
+		{"DuplicateNameDifferentBundleIdentifier", testSoftwareDuplicateNameDifferentBundleIdentifier},
+		{"DifferentNameSameBundleIdentifier", testSoftwareDifferentNameSameBundleIdentifier},
 		{"LoadVulnerabilities", testSoftwareLoadVulnerabilities},
 		{"ListSoftwareCPEs", testListSoftwareCPEs},
 		{"NothingChanged", testSoftwareNothingChanged},
@@ -71,6 +73,7 @@ func TestSoftware(t *testing.T) {
 		{"ListSoftwareVersionsVulnerabilityFilters", testListSoftwareVersionsVulnerabilityFilters},
 		{"TestListHostSoftwareWithLabelScoping", testListHostSoftwareWithLabelScoping},
 		{"TestListHostSoftwareWithLabelScopingVPP", testListHostSoftwareWithLabelScopingVPP},
+		{"DeletedInstalledSoftware", testDeletedInstalledSoftware},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -222,6 +225,142 @@ func testSoftwareCPE(t *testing.T, ds *Datastore) {
 	require.NoError(t, iterator.Close())
 }
 
+func testSoftwareDifferentNameSameBundleIdentifier(t *testing.T, ds *Datastore) {
+	host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
+
+	incoming := make(map[string]fleet.Software)
+	sw, err := fleet.SoftwareFromOsqueryRow("GoLand.app", "2024.3", "apps", "", "", "", "", "com.jetbrains.goland", "", "", "")
+	require.NoError(t, err)
+	soft2Key := sw.ToUniqueStr()
+	incoming[soft2Key] = *sw
+
+	currentSoftware, incomingChecksumToSoftware, incomingChecksumToTitle, err := ds.getExistingSoftware(
+		context.Background(), make(map[string]fleet.Software), incoming,
+	)
+	require.NoError(t, err)
+	tx, err := ds.writer(context.Background()).Beginx()
+	require.NoError(t, err)
+	_, err = ds.insertNewInstalledHostSoftwareDB(
+		context.Background(), tx, host1.ID, currentSoftware, incomingChecksumToSoftware, incomingChecksumToTitle,
+	)
+	require.NoError(t, err)
+	require.NoError(t, tx.Commit())
+
+	var software []fleet.Software
+	err = sqlx.SelectContext(context.Background(), ds.reader(context.Background()),
+		&software, `SELECT id, name, bundle_identifier, title_id FROM software`,
+	)
+	require.NoError(t, err)
+	require.Len(t, software, 1)
+	var softwareTitle []fleet.SoftwareTitle
+	err = sqlx.SelectContext(context.Background(), ds.reader(context.Background()),
+		&softwareTitle, `SELECT id, name FROM software_titles`,
+	)
+	require.NoError(t, err)
+	require.Len(t, softwareTitle, 1)
+
+	incoming = make(map[string]fleet.Software)
+	sw, err = fleet.SoftwareFromOsqueryRow("GoLand 2.app", "2024.3", "apps", "", "", "", "", "com.jetbrains.goland", "", "", "")
+	require.NoError(t, err)
+	soft3Key := sw.ToUniqueStr()
+	incoming[soft3Key] = *sw
+
+	currentSoftware, incomingChecksumToSoftware, incomingChecksumToTitle, err = ds.getExistingSoftware(
+		context.Background(), make(map[string]fleet.Software), incoming,
+	)
+	require.NoError(t, err)
+	tx, err = ds.writer(context.Background()).Beginx()
+	require.NoError(t, err)
+	_, err = ds.insertNewInstalledHostSoftwareDB(
+		context.Background(), tx, host1.ID, currentSoftware, incomingChecksumToSoftware, incomingChecksumToTitle,
+	)
+	require.NoError(t, err)
+	require.NoError(t, tx.Commit())
+
+	err = sqlx.SelectContext(context.Background(), ds.reader(context.Background()),
+		&software, `SELECT id, name, bundle_identifier, title_id FROM software`,
+	)
+	require.NoError(t, err)
+	require.Len(t, software, 2)
+	for _, s := range software {
+		require.NotEmpty(t, s.TitleID)
+	}
+
+	err = sqlx.SelectContext(context.Background(), ds.reader(context.Background()),
+		&softwareTitle, `SELECT id, name FROM software_titles`,
+	)
+	require.NoError(t, err)
+	require.Len(t, softwareTitle, 1)
+}
+
+func testSoftwareDuplicateNameDifferentBundleIdentifier(t *testing.T, ds *Datastore) {
+	host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
+
+	incoming := make(map[string]fleet.Software)
+	sw, err := fleet.SoftwareFromOsqueryRow("a", "0.0.1", "chrome_extension", "", "", "", "", "bundle_id1", "", "", "")
+	require.NoError(t, err)
+	soft2Key := sw.ToUniqueStr()
+	incoming[soft2Key] = *sw
+
+	currentSoftware, incomingChecksumToSoftware, incomingChecksumToTitle, err := ds.getExistingSoftware(
+		context.Background(), make(map[string]fleet.Software), incoming,
+	)
+	require.NoError(t, err)
+	tx, err := ds.writer(context.Background()).Beginx()
+	require.NoError(t, err)
+	_, err = ds.insertNewInstalledHostSoftwareDB(
+		context.Background(), tx, host1.ID, currentSoftware, incomingChecksumToSoftware, incomingChecksumToTitle,
+	)
+	require.NoError(t, err)
+	require.NoError(t, tx.Commit())
+
+	var software []fleet.Software
+	err = sqlx.SelectContext(context.Background(), ds.reader(context.Background()),
+		&software, `SELECT id, name, bundle_identifier, title_id FROM software`,
+	)
+	require.NoError(t, err)
+	require.Len(t, software, 1)
+	var softwareTitle []fleet.SoftwareTitle
+	err = sqlx.SelectContext(context.Background(), ds.reader(context.Background()),
+		&softwareTitle, `SELECT id, name FROM software_titles`,
+	)
+	require.NoError(t, err)
+	require.Len(t, softwareTitle, 1)
+
+	incoming = make(map[string]fleet.Software)
+	sw, err = fleet.SoftwareFromOsqueryRow("a", "0.0.1", "chrome_extension", "", "", "", "", "bundle_id2", "", "", "")
+	require.NoError(t, err)
+	soft3Key := sw.ToUniqueStr()
+	incoming[soft3Key] = *sw
+
+	currentSoftware, incomingChecksumToSoftware, incomingChecksumToTitle, err = ds.getExistingSoftware(
+		context.Background(), make(map[string]fleet.Software), incoming,
+	)
+	require.NoError(t, err)
+	tx, err = ds.writer(context.Background()).Beginx()
+	require.NoError(t, err)
+	_, err = ds.insertNewInstalledHostSoftwareDB(
+		context.Background(), tx, host1.ID, currentSoftware, incomingChecksumToSoftware, incomingChecksumToTitle,
+	)
+	require.NoError(t, err)
+	require.NoError(t, tx.Commit())
+
+	err = sqlx.SelectContext(context.Background(), ds.reader(context.Background()),
+		&software, `SELECT id, name, bundle_identifier, title_id FROM software`,
+	)
+	require.NoError(t, err)
+	require.Len(t, software, 2)
+	for _, s := range software {
+		require.NotEmpty(t, s.TitleID)
+	}
+
+	err = sqlx.SelectContext(context.Background(), ds.reader(context.Background()),
+		&softwareTitle, `SELECT id, name FROM software_titles`,
+	)
+	require.NoError(t, err)
+	require.Len(t, softwareTitle, 2)
+}
+
 func testSoftwareHostDuplicates(t *testing.T, ds *Datastore) {
 	host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
 
@@ -5765,3 +5904,66 @@ func testListHostSoftwareWithLabelScopingVPP(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	require.True(t, scoped)
 }
+
+func testDeletedInstalledSoftware(t *testing.T, ds *Datastore) {
+	ctx := context.Background()
+	host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
+	user1 := test.NewUser(t, ds, "Alice", "[email protected]", true)
+	team, err := ds.NewTeam(ctx, &fleet.Team{Name: "team 1"})
+	require.NoError(t, err)
+
+	installerID, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
+		Title:            "GoLand",
+		Source:           "app",
+		InstallScript:    "echo",
+		TeamID:           &team.ID,
+		Filename:         "foo.pkg",
+		UserID:           user1.ID,
+		BundleIdentifier: "com.jetbrains.goland",
+		ValidatedLabels:  &fleet.LabelIdentsWithScope{},
+	})
+	require.NoError(t, err)
+	_, err = ds.InsertSoftwareInstallRequest(ctx, host1.ID, installerID, false, nil)
+	require.NoError(t, err)
+
+	ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
+		_, err = q.ExecContext(ctx, `UPDATE host_software_installs SET post_install_script_exit_code = 0`)
+		require.NoError(t, err)
+		return nil
+	})
+
+	software1 := []fleet.Software{
+		{Name: "GoLand", Version: "1.0.2", Source: "app", BundleIdentifier: "com.jetbrains.goland"},
+		{Name: "GoLand", Version: "1.0.2", Source: "app", BundleIdentifier: "com.jetbrains.goland2"},
+	}
+	_, err = ds.UpdateHostSoftware(context.Background(), host1.ID, software1)
+	require.NoError(t, err)
+
+	// remove software with different bundle id same name as installed software
+	software1 = []fleet.Software{
+		{Name: "GoLand", Version: "1.0.2", Source: "app", BundleIdentifier: "com.jetbrains.goland"},
+	}
+	_, err = ds.UpdateHostSoftware(context.Background(), host1.ID, software1)
+	require.NoError(t, err)
+
+	var hostSoftwareInstalls []struct {
+		HostID              uint   `db:"host_id"`
+		SoftwareInstallerID uint   `db:"software_installer_id"`
+		Removed             bool   `db:"removed"`
+		Status              string `db:"status"`
+	}
+	err = sqlx.SelectContext(
+		ctx,
+		ds.writer(ctx),
+		&hostSoftwareInstalls,
+		`select host_id, software_installer_id, removed, status from host_software_installs where host_id = ?`,
+		host1.ID,
+	)
+	if err != nil {
+		fmt.Printf("error getting software titles: %v\n", err)
+	}
+	// Ensure installed software is not marked as removed
+	for _, value := range hostSoftwareInstalls {
+		assert.False(t, value.Removed)
+	}
+}