Elgato_dark/fleet
1
0
Fork 0
mirror of https://github.com/fleetdm/fleet synced 2026-05-23 08:58:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 111

Fix software naming migration duplicate software collection query, add host software installed path remapping to migration to avoid orphaned paths (#28601)

Browse source 
For #28586.

Originally hotfixed into 4.67.2 in #28588. Merging back into `main`
here.
This commit is contained in:
Ian Littman 2025-04-28 09:52:57 -05:00 committed by GitHub
parent e761f85b27
commit 37adfd4535
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 201 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
changes/28586-hotfix-migration Normal file
View file

@ -0,0 +1,2 @@
* Fixed software deduplication when migrating from < 4.67.0 for cases where exactly two software entries would be merged into one, and for cases where the same bundle ID has more than one version, each with more than one that needs to be converted into a single software entry.
* Included host software installed paths migration in the above database migration, instead of waiting for software ingestion to repopulate/clean up affected rows.

106
server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames.go
View file

@ -3,6 +3,8 @@ package tables
import (
"database/sql"
"fmt"
"strconv"
"strings"
"github.com/jmoiron/sqlx"
"github.com/jmoiron/sqlx/reflectx"
@ -13,7 +15,6 @@ func init() {
MigrationClient.AddMigration(Up_20250410104321, Down_20250410104321)
}
// TODO(JVE): add bundleid != null to all constraints
func Up_20250410104321(tx *sql.Tx) error {
titleStmt := `UPDATE software_titles SET name = TRIM( TRAILING '.app' FROM name ) WHERE source = 'apps' AND bundle_identifier IS NOT NULL`
_, err := tx.Exec(titleStmt)
@ -21,44 +22,52 @@ func Up_20250410104321(tx *sql.Tx) error {
return fmt.Errorf("updating software_titles.name: %w", err)
}
dupeIDsStmt := `SELECT
s1.id AS id, s1.bundle_identifier AS bundle_identifier FROM software s1
JOIN software s2 ON s1.bundle_identifier = s2.bundle_identifier
AND s1.version = s2.version
AND s1.title_id = s2.title_id
AND s1.source = s2.source
AND` + " s1.`release` = s2.`release` " +
`AND s1.arch = s2.arch
AND s1.vendor = s2.vendor
AND s1.browser = s2.browser
AND s1.extension_id = s2.extension_id
WHERE
s1.source = 'apps'
dupeIDsStmt := `SELECT GROUP_CONCAT(id) AS ids, MD5(
-- simulate new hash
CONCAT_WS(CHAR(0),
version,
source,
bundle_identifier,
` + "`release`" + `,
arch,
vendor,
browser,
extension_id
)
) AS new_checksum
FROM software
WHERE source = 'apps' AND bundle_identifier IS NOT NULL AND bundle_identifier != ''
GROUP BY
id
HAVING
COUNT(*) > 1`
version, source, bundle_identifier,` + "`release`" + `, arch, vendor, browser, extension_id
HAVING COUNT(*) > 1`
txx := sqlx.Tx{Tx: tx, Mapper: reflectx.NewMapperFunc("db", sqlx.NameMapper)}
selectedIDs := make(map[string]uint)
excludedIDs := make(map[string][]uint)
var softwareIDs []struct {
ID uint `db:"id"`
BundleIdentifier string `db:"bundle_identifier"`
selectedIDs := make(map[string]uint64)
idsToMergeByNewChecksum := make(map[string][]uint64)
var softwareGroups []struct {
IDs string `db:"ids"`
NewChecksum string `db:"new_checksum"`
}
if err := txx.Select(&softwareIDs, dupeIDsStmt); err != nil {
if err := txx.Select(&softwareGroups, dupeIDsStmt); err != nil {
if !errors.Is(err, sql.ErrNoRows) {
return fmt.Errorf("selecting duplicate software rows: %w", err)
}
}
for _, s := range softwareIDs {
if _, ok := selectedIDs[s.BundleIdentifier]; !ok {
selectedIDs[s.BundleIdentifier] = s.ID
continue
}
for _, s := range softwareGroups {
for _, idStr := range strings.Split(s.IDs, ",") {
id, err := strconv.ParseUint(idStr, 10, 64)
if err != nil {
return fmt.Errorf("building duplicate IDs list %q: %w", idStr, err)
}
excludedIDs[s.BundleIdentifier] = append(excludedIDs[s.BundleIdentifier], s.ID)
if _, ok := selectedIDs[s.NewChecksum]; !ok {
selectedIDs[s.NewChecksum] = id
continue
}
idsToMergeByNewChecksum[s.NewChecksum] = append(idsToMergeByNewChecksum[s.NewChecksum], id)
}
}
getRecordToUpdateStmt := `
@ -76,38 +85,49 @@ WHERE
WHERE
hs2.software_id = ?
AND hs2.host_id = hs1.host_id)`
updateHostSoftwareInstalledPathsStmt := `UPDATE host_software_installed_paths SET software_id = ? WHERE software_id IN (?)`
var allExcludedIDs []uint
for bid, excluded := range excludedIDs {
allExcludedIDs = append(allExcludedIDs, excluded...)
var hs []struct {
var allExcludedIDs []uint64
for newChecksum, idsToMerge := range idsToMergeByNewChecksum {
allExcludedIDs = append(allExcludedIDs, idsToMerge...)
var hostIDRecordList []struct {
HostID uint `db:"host_id"`
}
selectedID, ok := selectedIDs[bid]
selectedID, ok := selectedIDs[newChecksum]
if !ok {
return fmt.Errorf("%s had excluded IDs but no selected ID", bid)
return fmt.Errorf("%v excluded IDs but no selected ID", idsToMerge)
}
stmt, args, err := sqlx.In(getRecordToUpdateStmt, excluded, selectedID)
stmt, args, err := sqlx.In(getRecordToUpdateStmt, idsToMerge, selectedID)
if err != nil {
return fmt.Errorf("sqlx.In for getting host software records to update for bundle_id %s: %w", bid, err)
return fmt.Errorf("sqlx.In for getting host software records to update for old software IDs %v: %w", idsToMerge, err)
}
if err := txx.Select(&hs, stmt, args...); err != nil {
if err := txx.Select(&hostIDRecordList, stmt, args...); err != nil {
if errors.Is(err, sql.ErrNoRows) {
// if there are no rows, this means the host is already pointed at the selected software
// ID, so no update needed
continue
}
return fmt.Errorf("getting host software record to update for bundle_id %s: %w", bid, err)
return fmt.Errorf("getting host software record to update for old software IDs %v: %w", idsToMerge, err)
}
for _, h := range hs {
for _, h := range hostIDRecordList {
_, err = tx.Exec(`INSERT INTO host_software (host_id, software_id) VALUES (?, ?)`, h.HostID, selectedID)
if err != nil {
return fmt.Errorf("updating host_software.software_id for bundle_id %s: %w", bid, err)
return fmt.Errorf("updating host_software.software_id for old software IDs %v: %w", idsToMerge, err)
}
}
// repoint host software installed paths to the software ID we're keeping
stmt, args, err = sqlx.In(updateHostSoftwareInstalledPathsStmt, selectedID, idsToMerge)
if err != nil {
return fmt.Errorf("sqlx.In for updating host software installed paths records for old software IDs %v: %w", idsToMerge, err)
}
if _, err := tx.Exec(stmt, args...); err != nil {
return fmt.Errorf("updating host software installed paths records for old software IDs %v: %w", idsToMerge, err)
}
}
// at this point, every host that needs one has a pointer to the selected ID, so we can delete
@ -117,7 +137,7 @@ WHERE
deleteSoftwareCVEStmt := `DELETE FROM software_cve WHERE software_id IN (?)`
deleteSoftwareCVEStmt, args, err := sqlx.In(deleteSoftwareCVEStmt, allExcludedIDs)
if err != nil {
return fmt.Errorf("sqlx.In for deleting excluded ids from software_cve: %w:", err)
return fmt.Errorf("sqlx.In for deleting excluded ids from software_cve: %w", err)
}
if _, err := tx.Exec(deleteSoftwareCVEStmt, args...); err != nil {
return fmt.Errorf("deleting excluded ids from software_cve: %w", err)
@ -134,7 +154,7 @@ WHERE
return fmt.Errorf("deleting excluded ids from host_software: %w", err)
}
deleteSoftwareDupesStmt := `DELETE FROM software WHERE id IN (?)`
deleteSoftwareDupesStmt := `DELETE FROM software WHERE id IN (?) AND bundle_identifier IS NOT NULL AND bundle_identifier != ''`
deleteSoftwareDupesStmt, args, err = sqlx.In(deleteSoftwareDupesStmt, allExcludedIDs)
if err != nil {
return fmt.Errorf("sqlx.In for deleting duplicates from software: %w", err)

147
server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames_test.go
View file

@ -1,6 +1,7 @@
package tables
import (
"crypto/md5" // nolint:gosec // used only to hash for efficient comparisons
"fmt"
"strings"
"testing"
@ -11,9 +12,23 @@ import (
"github.com/stretchr/testify/require"
)
func computeRawChecksumIncludingName(sw fleet.Software) ([]byte, error) {
h := md5.New() //nolint:gosec // This hash is used as a DB optimization for software row lookup, not security
cols := []string{sw.Name, sw.Version, sw.Source, sw.BundleIdentifier, sw.Release, sw.Arch, sw.Vendor, sw.Browser, sw.ExtensionID}
_, err := fmt.Fprint(h, strings.Join(cols, "\x00"))
if err != nil {
return nil, err
}
return h.Sum(nil), nil
}
func TestUp_20250410104321(t *testing.T) {
db := applyUpToPrev(t)
// 16 pieces of software, 14 (pre-dedupe) of which are macOS apps, 11 of which are unique (relative to software table)
// Each piece of software is on a different host, other than MacApp Duplicate 3, which is on the same host
// as another of the MacApps (same bundle ID). This means we'll start with 16 host_software entries and
// expect one of those entries to go away.
softwares := []fleet.Software{
{Name: "MacApp.app", Source: "apps", BundleIdentifier: "com.example.foo", Version: "1"},
{Name: "MacApp Duplicate.app", Source: "apps", BundleIdentifier: "com.example.foo", Version: "1"},
@ -22,15 +37,24 @@ func TestUp_20250410104321(t *testing.T) {
{Name: "no_bundle_id.app", Source: "apps", BundleIdentifier: "", Version: "42"},
{Name: "no_bundle_id_2.app", Source: "apps", BundleIdentifier: "", Version: "24"},
{Name: "MacApp2.app", Source: "apps", BundleIdentifier: "com.example.foo2", Version: "2"},
{Name: "MacApp2 2.app", Source: "apps", BundleIdentifier: "com.example.foo2", Version: "2"},
{Name: "MacApp2.1.app", Source: "apps", BundleIdentifier: "com.example.foo2", Version: "2.1"}, // should be a different software post-migration
{Name: "MacApp2.1 2.app", Source: "apps", BundleIdentifier: "com.example.foo2", Version: "2.1"}, // should be the same software as the line above
{Name: "Chrome Extension", Source: "chrome_extensions", Browser: "chrome", Version: "3"},
{Name: "Microsoft Teams.exe", Source: "programs", Version: "4"},
{Name: "Live Captions.app", Source: "apps", BundleIdentifier: "com.apple.accessibility.LiveTranscriptionAgent", Version: "1.0"},
{Name: "LiveTranscriptionAgent.app", Source: "apps", BundleIdentifier: "com.apple.accessibility.LiveTranscriptionAgent", Version: "1.0"},
{Name: "Postman Helper (Renderer).app", Source: "apps", BundleIdentifier: "com.postmanlabs.mac.helper", Version: ""},
{Name: "Postman Helper.app", Source: "apps", BundleIdentifier: "com.postmanlabs.mac.helper", Version: ""},
}
// add some software titles
dataStmt := `INSERT INTO software_titles (name, source, browser, bundle_identifier) VALUES (?, ?, ?, ?)`
for i, s := range softwares {
if i > 0 && s.BundleIdentifier == "com.example.foo" {
if (i > 0 && s.BundleIdentifier == "com.example.foo") ||
s.Name == "LiveTranscriptionAgent.app" ||
s.Name == "Postman Helper.app" ||
s.Version == "2.1" || s.Name == "MacApp2 2.app" {
continue
}
var bid any = ptr.String(s.BundleIdentifier)
@ -38,8 +62,7 @@ func TestUp_20250410104321(t *testing.T) {
bid = nil
}
id := execNoErrLastID(t, db, dataStmt, s.Name, s.Source, s.Browser, bid)
if s.BundleIdentifier == "com.example.foo" {
// All the duplicates should map to the same title ID
if s.BundleIdentifier == "com.example.foo" { // All the initial duplicates should map to the same title ID
for i := range 4 {
softwares[i].TitleID = ptr.Uint(uint(id)) //nolint:gosec // dismiss G115
}
@ -47,6 +70,17 @@ func TestUp_20250410104321(t *testing.T) {
}
softwares[i].TitleID = ptr.Uint(uint(id)) //nolint:gosec // dismiss G115
// More duplicate mapping to existing software title ID
if s.BundleIdentifier == "com.example.foo2" {
softwares[8].TitleID = ptr.Uint(uint(id)) //nolint:gosec // dismiss G115
}
if s.BundleIdentifier == "com.apple.accessibility.LiveTranscriptionAgent" {
softwares[12].TitleID = ptr.Uint(uint(id)) //nolint:gosec // dismiss G115
}
if s.BundleIdentifier == "com.postmanlabs.mac.helper" {
softwares[14].TitleID = ptr.Uint(999) // throwing in a broken title ID to reflect some deployed environments
}
}
// add some software entries and host_software entries
@ -57,7 +91,17 @@ func TestUp_20250410104321(t *testing.T) {
var softwareIDs []uint
for i, s := range softwares {
id := execNoErrLastID(t, db, dataStmt, s.Name, s.Version, s.Source, s.BundleIdentifier, "", "", "", s.Browser, "", fmt.Sprintf("foo%d", i), s.TitleID)
checksum, err := computeRawChecksumIncludingName(softwares[i])
require.NoError(t, err)
id := execNoErrLastID(
t,
db,
dataStmt,
s.Name, s.Version, s.Source, s.BundleIdentifier, "", "", "", s.Browser, "",
checksum,
s.TitleID,
)
softwareIDs = append(softwareIDs, uint(id)) //nolint:gosec // dismiss G115
softwares[i].ID = uint(id) //nolint:gosec // dismiss G115
@ -67,6 +111,15 @@ func TestUp_20250410104321(t *testing.T) {
hostID = uint(i) //nolint:gosec // dismiss G115
}
execNoErr(t, db, "INSERT INTO host_software (host_id, software_id) VALUES (?, ?)", hostID, uint(id)) //nolint:gosec // dismiss G115
// insert installed paths for macOS apps to make sure all get migrated over
if s.Source == "apps" {
execNoErr(t, db, "INSERT INTO host_software_installed_paths (host_id, software_id, installed_path) VALUES (?, ?, ?)",
hostID,
uint(id), //nolint:gosec // dismiss G115
"/Applications/"+s.Name,
)
}
}
noBundleID1 := softwares[4]
@ -88,14 +141,14 @@ func TestUp_20250410104321(t *testing.T) {
// macOS apps should be modified, others should not
var gotSoftware []fleet.Software
err = db.Select(&gotSoftware, `SELECT id, name, checksum, name_source FROM software`)
err = db.Select(&gotSoftware, `SELECT id, name, checksum, name_source, version FROM software ORDER BY id ASC`)
require.NoError(t, err)
require.Len(t, gotSoftware, 6)
require.Len(t, gotSoftware, 9)
var gotSoftwareTitles []fleet.SoftwareTitle
err = db.Select(&gotSoftwareTitles, "SELECT id, name, source, browser, bundle_identifier FROM software_titles")
require.NoError(t, err)
require.Len(t, gotSoftwareTitles, 6)
require.Len(t, gotSoftwareTitles, 8) // two versions of MacApp2
for _, got := range gotSoftwareTitles {
switch got.ID {
@ -117,18 +170,46 @@ func TestUp_20250410104321(t *testing.T) {
default:
require.NotContains(t, got.Name, ".app")
require.Equal(t, "basic", got.NameSource)
require.NotContains(t, got.Name, ".app")
}
}
// Rows in the set are MacApp (duplicates deleted), no_bundle_id, no_bundle_id_2, MacApp2 v2, MacApp2 v2.1, etc.
require.Equal(t, "MacApp2", gotSoftware[3].Name)
require.Equal(t, "2", gotSoftware[3].Version)
require.Equal(t, "MacApp2.1", gotSoftware[4].Name)
require.Equal(t, "2.1", gotSoftware[4].Version)
var count int
err = db.Get(&count, "SELECT COUNT(*) FROM software_cve")
require.NoError(t, err)
require.Equal(t, 6, count)
require.Equal(t, 9, count)
err = db.Get(&count, "SELECT COUNT(*) FROM host_software")
require.NoError(t, err)
require.Equal(t, 8, count)
require.Equal(t, 15, count)
// ensure no orphaned host software installed paths
err = db.Get(&count, `SELECT COUNT(*) FROM host_software_installed_paths WHERE software_id NOT IN (SELECT id FROM software)`)
require.NoError(t, err)
require.Zero(t, count)
err = db.Get(&count, `SELECT COUNT(*) FROM host_software_installed_paths WHERE software_id IN (SELECT id FROM software)`)
require.NoError(t, err)
require.Equal(t, 14, count) // one per install; one host has two paths
// ensure we have the expected number of unique software IDs on host software installed paths (same as software count with source apps)
err = db.Get(&count, `SELECT COUNT(DISTINCT software_id) FROM host_software_installed_paths`)
require.NoError(t, err)
require.Equal(t, 7, count)
// ensure both copies of the same app are listed in installed paths for the host that has this case
var getSoftwarePaths []fleet.HostSoftwareInstalledPath
err = db.Select(&getSoftwarePaths, "SELECT host_id, software_id, installed_path FROM host_software_installed_paths WHERE host_id = 3 ORDER BY installed_path")
require.NoError(t, err)
require.Len(t, getSoftwarePaths, 2)
require.Equal(t, getSoftwarePaths[0].SoftwareID, getSoftwarePaths[1].SoftwareID)
require.Equal(t, "/Applications/MacApp Duplicate 2.app", getSoftwarePaths[0].InstalledPath)
require.Equal(t, "/Applications/MacApp Duplicate 3.app", getSoftwarePaths[1].InstalledPath)
err = db.Get(&count, `SELECT COUNT(*) FROM host_software WHERE software_id IN (?, ?)`, softwareIDs[1], softwareIDs[2])
require.NoError(t, err)
@ -137,4 +218,48 @@ func TestUp_20250410104321(t *testing.T) {
err = db.Get(&count, `SELECT COUNT(*) FROM host_software WHERE software_id = ?`, softwareIDs[0])
require.NoError(t, err)
require.Equal(t, count, 3)
var hostIDs []uint
// ensure MacApp2 v2 has the expected (two) hosts associated
err = db.Select(&hostIDs, `SELECT host_id FROM host_software JOIN software ON software.id = host_software.software_id
WHERE bundle_identifier = "com.example.foo2" AND version = "2" ORDER BY host_id`)
require.NoError(t, err)
require.Len(t, hostIDs, 2)
require.Equal(t, uint(7), hostIDs[0])
require.Equal(t, uint(8), hostIDs[1])
// ensure installed paths map from the correct host to the correct software for MacApp2 v2
err = db.Select(&getSoftwarePaths, `SELECT host_id, software_id, installed_path FROM host_software_installed_paths
JOIN software ON software.id = host_software_installed_paths.software_id
WHERE bundle_identifier = "com.example.foo2" AND version = "2" ORDER BY host_id`)
require.NoError(t, err)
require.Len(t, getSoftwarePaths, 2)
require.Equal(t, uint(7), getSoftwarePaths[0].HostID)
require.Equal(t, gotSoftware[3].ID, getSoftwarePaths[0].SoftwareID)
require.Equal(t, "/Applications/MacApp2.app", getSoftwarePaths[0].InstalledPath)
require.Equal(t, uint(8), getSoftwarePaths[1].HostID)
require.Equal(t, gotSoftware[3].ID, getSoftwarePaths[1].SoftwareID)
require.Equal(t, "/Applications/MacApp2 2.app", getSoftwarePaths[1].InstalledPath)
// ensure MacApp2 v2.1 has the expected (two) hosts associated
err = db.Select(&hostIDs, `SELECT host_id FROM host_software JOIN software ON software.id = host_software.software_id
WHERE bundle_identifier = "com.example.foo2" AND version = "2.1" ORDER BY host_id`)
require.NoError(t, err)
require.Len(t, hostIDs, 2)
require.Equal(t, uint(9), hostIDs[0])
require.Equal(t, uint(10), hostIDs[1])
// ensure installed paths map from the correct host to the correct software for MacApp2 v2.1
err = db.Select(&getSoftwarePaths, `SELECT host_id, software_id, installed_path FROM host_software_installed_paths
JOIN software ON software.id = host_software_installed_paths.software_id
WHERE bundle_identifier = "com.example.foo2" AND version = "2.1" ORDER BY host_id`)
require.NoError(t, err)
require.Len(t, getSoftwarePaths, 2)
require.Equal(t, uint(9), getSoftwarePaths[0].HostID)
require.Equal(t, gotSoftware[4].ID, getSoftwarePaths[0].SoftwareID)
require.Equal(t, "/Applications/MacApp2.1.app", getSoftwarePaths[0].InstalledPath)
require.Equal(t, uint(10), getSoftwarePaths[1].HostID)
require.Equal(t, gotSoftware[4].ID, getSoftwarePaths[1].SoftwareID)
require.Equal(t, "/Applications/MacApp2.1 2.app", getSoftwarePaths[1].InstalledPath)
}