From 563bcdf18ba8ec8932c8e3694a8e10bb5fba7510 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Mon, 6 Oct 2025 11:32:26 -0500 Subject: [PATCH] Handle multiple software entries with the same bundle ID during renames. (#33479) - Adjusted logic to support multiple software versions sharing a bundle ID. - Extended tests to validate scenarios involving renamed software across versions. **Related issue:** Resolves #33468 # 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. ## Testing - [x] Added/updated automated tests - [x] Manually QA'd using osquery `--common_software_name_suffix` switch ## Summary by CodeRabbit - Bug Fixes - Improves handling of apps that share the same bundle ID, ensuring all versions are correctly linked and consistently renamed across hosts. - Reduces duplicate software entries and keeps host associations intact during rename operations. - Delivers more reliable software inventory views with accurate app names derived from bundle IDs. - Tests - Adds comprehensive coverage for scenarios with multiple versions per bundle ID to validate linking and renaming behavior across hosts. --- changes/33468-multiple-versions-bundle-ID | 1 + server/datastore/mysql/software.go | 93 ++++++++++++----------- server/datastore/mysql/software_test.go | 88 +++++++++++++++++++++ 3 files changed, 139 insertions(+), 43 deletions(-) create mode 100644 changes/33468-multiple-versions-bundle-ID diff --git a/changes/33468-multiple-versions-bundle-ID b/changes/33468-multiple-versions-bundle-ID new file mode 100644 index 0000000000..884a248dd3 --- /dev/null +++ b/changes/33468-multiple-versions-bundle-ID @@ -0,0 +1 @@ +* Fixed edge case when renaming macOS software mapped to multiple checksums. diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index eaa430c3de..550e1d6c02 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -455,16 +455,22 @@ func (ds *Datastore) applyChangesForNewSoftwareDB( // Check inserted software for renames for _, sw := range r.Inserted { if sw.BundleIdentifier != "" { - if updSoftware, needsUpdate := existingBundleIDsToUpdate[sw.BundleIdentifier]; needsUpdate { - softwareRenames[sw.ID] = updSoftware.Name + if updSoftwareList, needsUpdate := existingBundleIDsToUpdate[sw.BundleIdentifier]; needsUpdate { + // Use the first software in the list for the name (they should all have the same name) + if len(updSoftwareList) > 0 { + softwareRenames[sw.ID] = updSoftwareList[0].Name + } } } } // Check existing software for renames for _, s := range existingSoftware { if s.BundleIdentifier != nil && *s.BundleIdentifier != "" { - if updSoftware, ok := existingBundleIDsToUpdate[*s.BundleIdentifier]; ok { - softwareRenames[s.ID] = updSoftware.Name + if updSoftwareList, ok := existingBundleIDsToUpdate[*s.BundleIdentifier]; ok { + // Use the first software in the list for the name (they should all have the same name) + if len(updSoftwareList) > 0 { + softwareRenames[s.ID] = updSoftwareList[0].Name + } } } } @@ -671,15 +677,14 @@ func (ds *Datastore) getExistingSoftware( currentSoftware []softwareIDChecksum, incomingChecksumToSoftware map[string]fleet.Software, incomingChecksumToTitle map[string]fleet.SoftwareTitle, - existingBundleIDsToUpdate map[string]fleet.Software, + existingBundleIDsToUpdate map[string][]fleet.Software, err error, ) { // Compute checksums for all incoming software, which we will use for faster retrieval, since checksum is a unique index incomingChecksumToSoftware = make(map[string]fleet.Software, len(current)) newSoftware := make(map[string]struct{}) - bundleIDsToChecksum := make(map[string]string) - bundleIDsToNames := make(map[string]string) - existingBundleIDsToUpdate = make(map[string]fleet.Software) + incomingBundleIDsToNewSoftwareNames := make(map[string]string) + existingBundleIDsToUpdate = make(map[string][]fleet.Software) for uniqueName, s := range incoming { _, ok := current[uniqueName] if !ok { @@ -691,8 +696,7 @@ func (ds *Datastore) getExistingSoftware( newSoftware[string(checksum)] = struct{}{} if s.BundleIdentifier != "" { - bundleIDsToChecksum[s.BundleIdentifier] = string(checksum) - bundleIDsToNames[s.BundleIdentifier] = s.Name + incomingBundleIDsToNewSoftwareNames[s.BundleIdentifier] = s.Name } } } @@ -708,30 +712,29 @@ func (ds *Datastore) getExistingSoftware( if err != nil { return nil, nil, nil, nil, err } - for _, s := range currentSoftware { - sw, ok := incomingChecksumToSoftware[s.Checksum] + + for _, currentSoftwareItem := range currentSoftware { + incomingSoftwareItem, ok := incomingChecksumToSoftware[currentSoftwareItem.Checksum] if !ok { // This should never happen. If it does, we have a bug. return nil, nil, nil, nil, ctxerr.New( - ctx, fmt.Sprintf("current software: software not found for checksum %s", hex.EncodeToString([]byte(s.Checksum))), + ctx, fmt.Sprintf("current software: software not found for checksum %s", hex.EncodeToString([]byte(currentSoftwareItem.Checksum))), ) } - if s.BundleIdentifier != nil && s.Source == "apps" { - if name, ok := bundleIDsToNames[*s.BundleIdentifier]; ok && name != s.Name { + if currentSoftwareItem.BundleIdentifier != nil && currentSoftwareItem.Source == "apps" { + if name, ok := incomingBundleIDsToNewSoftwareNames[*currentSoftwareItem.BundleIdentifier]; ok && name != currentSoftwareItem.Name { // Then this is a software whose name has changed, so we should update the name // Copy the incoming software but with the existing software's ID - swWithID := sw - swWithID.ID = s.ID - existingBundleIDsToUpdate[*s.BundleIdentifier] = swWithID + swWithID := incomingSoftwareItem + swWithID.ID = currentSoftwareItem.ID + existingBundleIDsToUpdate[*currentSoftwareItem.BundleIdentifier] = append(existingBundleIDsToUpdate[*currentSoftwareItem.BundleIdentifier], swWithID) - // Remove from incomingChecksumToSoftware to prevent it being treated as new software - if cs, ok := bundleIDsToChecksum[*s.BundleIdentifier]; ok { - delete(incomingChecksumToSoftware, cs) - } + // Delete this checksum to prevent it from being treated as new software + delete(incomingChecksumToSoftware, currentSoftwareItem.Checksum) continue } } - delete(newSoftware, s.Checksum) + delete(newSoftware, currentSoftwareItem.Checksum) } } @@ -1132,7 +1135,7 @@ func (ds *Datastore) linkExistingBundleIDSoftware( ctx context.Context, tx sqlx.ExtContext, hostID uint, - existingBundleIDsToUpdate map[string]fleet.Software, + existingBundleIDsToUpdate map[string][]fleet.Software, ) ([]fleet.Software, error) { if len(existingBundleIDsToUpdate) == 0 { return nil, nil @@ -1140,12 +1143,14 @@ func (ds *Datastore) linkExistingBundleIDSoftware( // Collect all software IDs to verify they still exist softwareIDs := make([]uint, 0, len(existingBundleIDsToUpdate)) - for _, software := range existingBundleIDsToUpdate { - // The software.ID should already be set from getExistingSoftware - if software.ID == 0 { - return nil, ctxerr.New(ctx, "software ID not set for bundle ID match") + for _, softwareList := range existingBundleIDsToUpdate { + for _, software := range softwareList { + // The software.ID should already be set from getExistingSoftware + if software.ID == 0 { + return nil, ctxerr.New(ctx, "software ID not set for bundle ID match") + } + softwareIDs = append(softwareIDs, software.ID) } - softwareIDs = append(softwareIDs, software.ID) } // Verify software still exists (just like we do in linkSoftwareToHost) @@ -1170,19 +1175,21 @@ func (ds *Datastore) linkExistingBundleIDSoftware( var insertsHostSoftware []any var insertedSoftware []fleet.Software - for _, software := range existingBundleIDsToUpdate { - // Only link if software still exists - if _, ok := existingIDSet[software.ID]; ok { - insertsHostSoftware = append(insertsHostSoftware, hostID, software.ID, software.LastOpenedAt) - insertedSoftware = append(insertedSoftware, software) - } else { - // Log missing software but continue - level.Warn(ds.logger).Log( - "msg", "bundle ID software not found after pre-insertion", - "software_id", software.ID, - "name", software.Name, - "bundle_id", software.BundleIdentifier, - ) + for _, softwareList := range existingBundleIDsToUpdate { + for _, software := range softwareList { + // Only link if software still exists + if _, ok := existingIDSet[software.ID]; ok { + insertsHostSoftware = append(insertsHostSoftware, hostID, software.ID, software.LastOpenedAt) + insertedSoftware = append(insertedSoftware, software) + } else { + // Log missing software but continue + level.Warn(ds.logger).Log( + "msg", "bundle ID software not found after pre-insertion", + "software_id", software.ID, + "name", software.Name, + "bundle_id", software.BundleIdentifier, + ) + } } } @@ -1286,7 +1293,7 @@ func updateModifiedHostSoftwareDB( hostID uint, currentMap map[string]fleet.Software, incomingMap map[string]fleet.Software, - existingBundleIDsToUpdate map[string]fleet.Software, + existingBundleIDsToUpdate map[string][]fleet.Software, minLastOpenedAtDiff time.Duration, logger log.Logger, ) error { diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index 08028afd1d..2fb31be547 100644 --- a/server/datastore/mysql/software_test.go +++ b/server/datastore/mysql/software_test.go @@ -54,6 +54,7 @@ func TestSoftware(t *testing.T) { {"UpdateHostSoftwareSameBundleIDDifferentNames", testUpdateHostSoftwareSameBundleIDDifferentNames}, {"UpdateHostSoftwareSameNameDifferentBundleIDs", testUpdateHostSoftwareSameNameDifferentBundleIDs}, {"UpdateHostSoftwareMultipleSameBundleID", testUpdateHostSoftwareMultipleSameBundleID}, + {"UpdateHostSoftwareMultipleChecksumsPerBundleID", testUpdateHostSoftwareMultipleChecksumsPerBundleID}, {"UpdateHostSoftwareLongNameTruncation", testUpdateHostSoftwareLongNameTruncation}, {"UpdateHostBundleIDRenameOnlyNoNewSoftware", testUpdateHostBundleIDRenameOnlyNoNewSoftware}, {"UpdateHostBundleIDRenameWithNewSoftware", testUpdateHostBundleIDRenameWithNewSoftware}, @@ -2021,6 +2022,93 @@ func testUpdateHostSoftwareMultipleSameBundleID(t *testing.T, ds *Datastore) { require.Equal(t, "GoLand 2024.app", host2.Software[0].Name, "Host2 should see renamed software") } +// Test for the bug where multiple software with the same bundle ID causes +// "software not found for checksum" errors during bundle ID rename operations. +// This test specifically validates that ALL software entries with the same +// bundle ID are properly linked to hosts when renaming occurs. +func testUpdateHostSoftwareMultipleChecksumsPerBundleID(t *testing.T, ds *Datastore) { + ctx := t.Context() + + // Note: Basic multiple versions scenario is already covered in testUpdateHostSoftwareMultipleSameBundleID + // This test focuses on the specific bug fix for renamed apps with many versions + + // First, establish the software with host1 - using 10 versions to stress test + host1 := test.NewHost(t, ds, "rename-test-host1", "", "rename-key1", "rename-uuid1", time.Now()) + + // Create 10 versions to stress test the checksum tracking + var initialSoftware []fleet.Software + for i := 0; i < 10; i++ { + initialSoftware = append(initialSoftware, fleet.Software{ + Name: "TestApp.app", + Version: fmt.Sprintf("1.%d.0", i), + Source: "apps", + BundleIdentifier: "com.stresstest.app", + }) + } + + _, err := ds.UpdateHostSoftware(ctx, host1.ID, initialSoftware) + require.NoError(t, err, "Should handle 10 versions with same bundle ID") + + // Verify all 10 were inserted + err = ds.LoadHostSoftware(ctx, host1, false) + require.NoError(t, err) + require.Len(t, host1.Software, 10, "Host1 should have all 10 versions") + + // Host2 reports the same software but renamed (user renamed the apps) + // This triggers the bundle ID rename logic and tests the bug fix + host2 := test.NewHost(t, ds, "rename-test-host2", "", "rename-key2", "rename-uuid2", time.Now()) + + var renamedSoftware []fleet.Software + for i := 0; i < 10; i++ { + renamedSoftware = append(renamedSoftware, fleet.Software{ + Name: "TestApp Renamed.app", // Different name + Version: fmt.Sprintf("1.%d.0", i), + Source: "apps", + BundleIdentifier: "com.stresstest.app", + }) + } + + // This is where the bug would occur - only one software would be linked instead of all 10 + result, err := ds.UpdateHostSoftware(ctx, host2.ID, renamedSoftware) + require.NoError(t, err, "Should handle renamed apps with 10 versions without 'software not found for checksum' error") + assert.NotNil(t, result) + + // Verify the rename was processed in the database + var dbSoftware []struct { + Name string `db:"name"` + Version string `db:"version"` + NameSource string `db:"name_source"` + } + err = ds.writer(ctx).SelectContext(ctx, &dbSoftware, + `SELECT name, version, name_source FROM software + WHERE bundle_identifier = ? ORDER BY version`, + "com.stresstest.app") + require.NoError(t, err) + require.Len(t, dbSoftware, 10, "Should have 10 software entries in database") + + // All should be renamed + for _, sw := range dbSoftware { + assert.Equal(t, "TestApp Renamed.app", sw.Name, "All software should use the new name") + assert.Equal(t, "bundle_4.67", sw.NameSource, "Renamed software should have bundle_4.67 source") + } + + // Most importantly, verify that host2 has ALL 10 versions linked (this was the bug) + err = ds.LoadHostSoftware(ctx, host2, false) + require.NoError(t, err) + assert.Len(t, host2.Software, 10, "Host2 should have all 10 versions linked (bug fix verification)") + + // Verify all versions are present + versions := make(map[string]bool) + for _, sw := range host2.Software { + versions[sw.Version] = true + assert.Equal(t, "TestApp Renamed.app", sw.Name, "Should see renamed app") + } + for i := 0; i < 10; i++ { + version := fmt.Sprintf("1.%d.0", i) + assert.True(t, versions[version], "Should have version %s", version) + } +} + // Test edge case: Software with names exceeding maximum length // This validates truncation and handling of long names func testUpdateHostSoftwareLongNameTruncation(t *testing.T, ds *Datastore) {