From 1f157ffdc65e04481f57f0850b0f66f501d9ed72 Mon Sep 17 00:00:00 2001 From: Carlo DiCelico Date: Wed, 15 Jul 2026 10:08:02 -0400 Subject: [PATCH 1/4] fix multi-package host software details --- server/datastore/mysql/software.go | 156 ++++++++++--- server/datastore/mysql/software_test.go | 208 ++++++++++++++++++ server/service/integration_enterprise_test.go | 41 ++++ 3 files changed, 373 insertions(+), 32 deletions(-) diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index 919fdcb36be..7c6b9ca2d43 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -3673,14 +3673,13 @@ func hostSoftwareInstalls(ds *Datastore, ctx context.Context, hostID uint) ([]*h ua.activity_type = 'software_install' ) ) - -- Resolve to the title's currently-active installer so list and install agree on - -- label scope after an FMA replacement (old row kept with is_active=0). LEFT JOIN - -- yields installer_id=NULL when no active installer exists; filterSoftwareInstallersByLabel - -- tolerates that. lsia columns are listed explicitly to avoid lsia.installer_id colliding - -- with the projected active id (sqlx maps last-wins). - SELECT - software_installers.id AS installer_id, - software_installers.self_service AS package_self_service, + -- Keep active install records keyed by their actual installer. After an FMA replacement, + -- map the inactive recorded installer to the title's currently-active installer so its + -- install history remains visible on the replacement. LEFT JOIN yields installer_id=NULL + -- when an inactive installer has no active replacement. + SELECT + matched_installer.id AS installer_id, + matched_installer.self_service AS package_self_service, software_titles.id AS id, lsia.last_install_install_uuid, lsia.last_install_installed_at, @@ -3692,16 +3691,15 @@ func hostSoftwareInstalls(ds *Datastore, ctx context.Context, hostID uint) ([]*h INNER JOIN software_titles ON recorded_si.title_id = software_titles.id LEFT JOIN - software_installers ON software_installers.title_id = recorded_si.title_id - AND software_installers.global_or_team_id = recorded_si.global_or_team_id - AND software_installers.is_active = 1 - -- collapse to the first-added active package so multiple active packages don't fan out rows - AND software_installers.id = ( - SELECT MIN(si_first.id) FROM software_installers si_first - WHERE si_first.title_id = recorded_si.title_id - AND si_first.global_or_team_id = recorded_si.global_or_team_id - AND si_first.is_active = 1 + software_installers matched_installer ON matched_installer.id = CASE + WHEN recorded_si.is_active = 1 THEN recorded_si.id + ELSE ( + SELECT MIN(si_active.id) FROM software_installers si_active + WHERE si_active.title_id = recorded_si.title_id + AND si_active.global_or_team_id = recorded_si.global_or_team_id + AND si_active.is_active = 1 ) + END ` var softwareInstalls []*hostSoftware err := sqlx.SelectContext(ctx, ds.reader(ctx), &softwareInstalls, softwareInstallsStmt, hostID, hostID) @@ -3770,9 +3768,9 @@ func hostSoftwareUninstalls(ds *Datastore, ctx context.Context, hostID uint) ([] ua.activity_type = 'software_uninstall' ) ) - -- Resolve to active installer; see hostSoftwareInstalls for rationale. - SELECT - software_installers.id AS installer_id, + -- Resolve the installer used for matching; see hostSoftwareInstalls for rationale. + SELECT + matched_installer.id AS installer_id, software_titles.id AS id, host_script_results.exit_code AS exit_code, lsua.last_uninstall_script_execution_id, @@ -3785,16 +3783,15 @@ func hostSoftwareUninstalls(ds *Datastore, ctx context.Context, hostID uint) ([] INNER JOIN software_titles ON recorded_si.title_id = software_titles.id LEFT JOIN - software_installers ON software_installers.title_id = recorded_si.title_id - AND software_installers.global_or_team_id = recorded_si.global_or_team_id - AND software_installers.is_active = 1 - -- collapse to the first-added active package so multiple active packages don't fan out rows - AND software_installers.id = ( - SELECT MIN(si_first.id) FROM software_installers si_first - WHERE si_first.title_id = recorded_si.title_id - AND si_first.global_or_team_id = recorded_si.global_or_team_id - AND si_first.is_active = 1 + software_installers matched_installer ON matched_installer.id = CASE + WHEN recorded_si.is_active = 1 THEN recorded_si.id + ELSE ( + SELECT MIN(si_active.id) FROM software_installers si_active + WHERE si_active.title_id = recorded_si.title_id + AND si_active.global_or_team_id = recorded_si.global_or_team_id + AND si_active.is_active = 1 ) + END LEFT OUTER JOIN host_script_results ON host_script_results.host_id = ? AND host_script_results.execution_id = lsua.last_uninstall_script_execution_id ` @@ -5204,6 +5201,93 @@ func filterOutOfScopeFailedHostSoftwareInstalls( } } +// mergeInstallDataByInstaller records the most recent install for a title's specific installer, +// keyed by (title id, installer id). Keeping install data per installer (rather than collapsing to +// one row per title) is what lets ListHostSoftware later surface the install belonging to the +// resolved (displayed) installer instead of an arbitrary sibling's. No-op when the row has no +// installer (e.g. an inactive installer with no active replacement). +func mergeInstallDataByInstaller(installDataByTitleInstaller map[uint]map[uint]*hostSoftware, s *hostSoftware) { + if s.InstallerID == nil { + return + } + byInstaller := installDataByTitleInstaller[s.ID] + if byInstaller == nil { + byInstaller = make(map[uint]*hostSoftware) + installDataByTitleInstaller[s.ID] = byInstaller + } + existing := byInstaller[*s.InstallerID] + if existing == nil || existing.LastInstallInstalledAt == nil || + (s.LastInstallInstalledAt != nil && s.LastInstallInstalledAt.After(*existing.LastInstallInstalledAt)) { + installData := *s + byInstaller[*s.InstallerID] = &installData + } +} + +// mergeUninstallDataByInstaller folds a title's uninstall record into the per-(title, installer) +// index built by mergeInstallDataByInstaller, so uninstall recency is evaluated against the same +// installer's install record (not across sibling installers). +func mergeUninstallDataByInstaller(installDataByTitleInstaller map[uint]map[uint]*hostSoftware, s *hostSoftware) { + if s.InstallerID == nil { + return + } + byInstaller := installDataByTitleInstaller[s.ID] + if byInstaller == nil { + byInstaller = make(map[uint]*hostSoftware) + installDataByTitleInstaller[s.ID] = byInstaller + } + installData := byInstaller[*s.InstallerID] + if installData == nil { + uninstallData := *s + byInstaller[*s.InstallerID] = &uninstallData + return + } + if (installData.LastInstallInstalledAt == nil || + s.LastUninstallUninstalledAt != nil && s.LastUninstallUninstalledAt.After(*installData.LastInstallInstalledAt)) && + (installData.LastUninstallUninstalledAt == nil || + s.LastUninstallUninstalledAt != nil && s.LastUninstallUninstalledAt.After(*installData.LastUninstallUninstalledAt)) { + installData.Status = s.Status + installData.LastUninstallUninstalledAt = s.LastUninstallUninstalledAt + installData.LastUninstallScriptExecutionID = s.LastUninstallScriptExecutionID + installData.ExitCode = s.ExitCode + } +} + +// applyResolvedInstallerStatus pins each title's status/last-install/last-uninstall fields to the +// installer that ListHostSoftware resolved for display (resolvedInstallers[titleID].ID), so those +// fields describe the SAME installer as the shown name/version. If the resolved installer has no +// install record on the host, the fields are left cleared (available) rather than borrowing a +// sibling installer's data. This is the fix for #49208. +func applyResolvedInstallerStatus( + bySoftwareTitleID map[uint]*hostSoftware, + installDataByTitleInstaller map[uint]map[uint]*hostSoftware, + resolvedInstallers map[uint]resolvedInstaller, +) { + for titleID, resolved := range resolvedInstallers { + software := bySoftwareTitleID[titleID] + if software == nil { + continue + } + + software.Status = nil + software.LastInstallInstalledAt = nil + software.LastInstallInstallUUID = nil + software.LastUninstallUninstalledAt = nil + software.LastUninstallScriptExecutionID = nil + software.ExitCode = nil + + installData := installDataByTitleInstaller[titleID][resolved.ID] + if installData == nil { + continue + } + software.Status = installData.Status + software.LastInstallInstalledAt = installData.LastInstallInstalledAt + software.LastInstallInstallUUID = installData.LastInstallInstallUUID + software.LastUninstallUninstalledAt = installData.LastUninstallUninstalledAt + software.LastUninstallScriptExecutionID = installData.LastUninstallScriptExecutionID + software.ExitCode = installData.ExitCode + } +} + func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opts fleet.HostSoftwareTitleListOptions) ([]*fleet.HostSoftwareWithInstaller, *fleet.PaginationMetadata, error) { if !opts.VulnerableOnly && (opts.MinimumCVSS > 0 || opts.MaximumCVSS > 0 || opts.KnownExploit) { return nil, nil, fleet.NewInvalidArgumentError( @@ -5252,6 +5336,7 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt bySoftwareTitleID := make(map[uint]*hostSoftware) bySoftwareID := make(map[uint]*hostSoftware) + installDataByTitleInstaller := make(map[uint]map[uint]*hostSoftware) var err error var hostSoftwareInstallsList []*hostSoftware @@ -5261,11 +5346,11 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt return nil, nil, err } for _, s := range hostSoftwareInstallsList { + mergeInstallDataByInstaller(installDataByTitleInstaller, s) + // Only ensure the title is present here; its status/last-install fields are pinned to + // the resolved installer later by applyResolvedInstallerStatus. if _, ok := bySoftwareTitleID[s.ID]; !ok { bySoftwareTitleID[s.ID] = s - } else { - bySoftwareTitleID[s.ID].LastInstallInstalledAt = s.LastInstallInstalledAt - bySoftwareTitleID[s.ID].LastInstallInstallUUID = s.LastInstallInstallUUID } } } @@ -5276,6 +5361,12 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt return nil, nil, err } for _, s := range hostSoftwareUninstalls { + mergeUninstallDataByInstaller(installDataByTitleInstaller, s) + // The Status/LastUninstall* fields written to bySoftwareTitleID below are provisional and, + // for installer-backed titles, are superseded by applyResolvedInstallerStatus. They are kept + // because the uninstallQuarantineSet control flow (which removes titles the host uninstalled + // unless osquery still reports them installed) depends on this block for the + // non-available-for-install inventory path. if _, ok := bySoftwareTitleID[s.ID]; !ok { if opts.OnlyAvailableForInstall || opts.IncludeAvailableForInstall { bySoftwareTitleID[s.ID] = s @@ -6141,6 +6232,7 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt if err != nil { return nil, nil, err } + applyResolvedInstallerStatus(bySoftwareTitleID, installDataByTitleInstaller, resolvedInstallers) // filter out VPP apps due to label scoping filteredByVPPAdamID, otherVppAppsInInventory, err := filterVPPAppsByLabel( diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index f0f21f95a69..dff7be93e72 100644 --- a/server/datastore/mysql/software_test.go +++ b/server/datastore/mysql/software_test.go @@ -107,6 +107,7 @@ func TestSoftware(t *testing.T) { {"ListSoftwareVersionsVulnerabilityFilters", testListSoftwareVersionsVulnerabilityFilters}, {"TestListHostSoftwareWithLabelScoping", testListHostSoftwareWithLabelScoping}, {"ListHostSoftwareMultiplePackagesPrecedence", testListHostSoftwareMultiplePackagesPrecedence}, + {"ListHostSoftwareMultiplePackagesInstallDetails", testListHostSoftwareMultiplePackagesInstallDetails}, {"TestListHostSoftwareVulnerableAndVPP", testListHostSoftwareVulnerableAndVPP}, {"TestListHostSoftwareQuerySearching", testListHostSoftwareQuerySearching}, {"TestListHostSoftwareWithLabelScopingVPP", testListHostSoftwareWithLabelScopingVPP}, @@ -12385,8 +12386,13 @@ func testListHostSoftwareFMAReplacedInstallerInScopeShowsActiveMetadata(t *testi } require.NotNil(t, fmaRow, "FMA App should appear in list (in-scope active installer)") require.NotNil(t, fmaRow.SoftwarePackage, "software_package must be populated when active installer is in scope") + require.Equal(t, "fma.pkg", fmaRow.SoftwarePackage.Name) + require.Equal(t, "2.0", fmaRow.SoftwarePackage.Version) require.NotNil(t, fmaRow.SoftwarePackage.SelfService, "self_service flag should be set") require.True(t, *fmaRow.SoftwarePackage.SelfService, "self_service should reflect the ACTIVE installer's value (true), not the recorded inactive one (false)") + require.Equal(t, new(fleet.SoftwareInstalled), fmaRow.Status) + require.NotNil(t, fmaRow.SoftwarePackage.LastInstall, "install history from the inactive installer should remain visible on its replacement") + require.Equal(t, hostInstall, fmaRow.SoftwarePackage.LastInstall.InstallUUID) } // When osquery inventory matches multiple installer rows for the same title, the active @@ -13736,3 +13742,205 @@ func testListHostSoftwareMultiplePackagesPrecedence(t *testing.T, ds *Datastore) require.NotEqual(t, titleID, s.ID, "title should not be available when the host is in scope for no package") } } + +func testListHostSoftwareMultiplePackagesInstallDetails(t *testing.T, ds *Datastore) { + ctx := t.Context() + user := test.NewUser(t, ds, "Alice", "alice-multipkg-installs@example.com", true) + labelA, err := ds.NewLabel(ctx, &fleet.Label{Name: "labelA" + t.Name()}) + require.NoError(t, err) + + newPackage := func(storageID, filename, version, contents string, labels fleet.LabelIdentsWithScope) (uint, uint) { + tfr, err := fleet.NewTempFileReader(strings.NewReader(contents), t.TempDir) + require.NoError(t, err) + installerID, titleID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + InstallScript: "install", + UninstallScript: "uninstall", + InstallerFile: tfr, + StorageID: storageID, + Filename: filename, + Title: "MultiPackageInstallDetails", + Version: version, + Source: "apps", + BundleIdentifier: "com.example.multi-package-install-details", + UserID: user.ID, + Platform: "darwin", + SelfService: true, + ValidatedLabels: &labels, + }) + require.NoError(t, err) + return installerID, titleID + } + + installerA, titleID := newPackage("install-details-a", "package-a.pkg", "1.0", "package a", fleet.LabelIdentsWithScope{ + LabelScope: fleet.LabelScopeIncludeAny, + ByName: map[string]fleet.LabelIdent{labelA.Name: {LabelName: labelA.Name, LabelID: labelA.ID}}, + }) + installerB, titleIDB := newPackage("install-details-b", "package-b.pkg", "2.0", "package b", fleet.LabelIdentsWithScope{}) + require.Equal(t, titleID, titleIDB) + require.Less(t, installerA, installerB) + + newHost := func(name string, inScopeForA bool) *fleet.Host { + host := test.NewHost(t, ds, name, "", name+"-key", name+"-uuid", time.Now(), test.WithPlatform("darwin")) + if inScopeForA { + require.NoError(t, ds.AddLabelsToHost(ctx, host.ID, []uint{labelA.ID})) + } + host.LabelUpdatedAt = time.Now() + require.NoError(t, ds.UpdateHost(ctx, host)) + return host + } + + seedInstall := func(hostID, installerID uint, executionID string, at time.Time, exitCode int) { + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, ` + INSERT INTO host_software_installs + (execution_id, host_id, software_installer_id, install_script_exit_code, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?)`, + executionID, hostID, installerID, exitCode, at, at) + return err + }) + } + seedUninstall := func(hostID, installerID uint, executionID string, at time.Time) { + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, ` + INSERT INTO host_software_installs + (execution_id, host_id, software_installer_id, uninstall, uninstall_script_exit_code, created_at, updated_at) + VALUES (?, ?, ?, 1, 0, ?, ?)`, + executionID, hostID, installerID, at, at) + return err + }) + } + seedUpcoming := func(hostID, installerID uint, executionID, activityType string, at time.Time) { + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + result, err := q.ExecContext(ctx, ` + INSERT INTO upcoming_activities + (host_id, priority, fleet_initiated, activity_type, execution_id, payload, created_at) + VALUES (?, 0, 1, ?, ?, JSON_OBJECT('self_service', false), ?)`, hostID, activityType, executionID, at) + if err != nil { + return err + } + activityID, err := result.LastInsertId() + if err != nil { + return err + } + _, err = q.ExecContext(ctx, ` + INSERT INTO software_install_upcoming_activities (upcoming_activity_id, software_installer_id) + VALUES (?, ?)`, activityID, installerID) + return err + }) + } + getTitle := func(t *testing.T, host *fleet.Host) *fleet.HostSoftwareWithInstaller { + t.Helper() + software, _, err := ds.ListHostSoftware(ctx, host, fleet.HostSoftwareTitleListOptions{ + ListOptions: fleet.ListOptions{PerPage: 20, IncludeMetadata: true, OrderKey: "name"}, + IncludeAvailableForInstall: true, + }) + require.NoError(t, err) + for _, item := range software { + if item.ID == titleID { + require.NotNil(t, item.SoftwarePackage) + return item + } + } + require.FailNow(t, "software title not found") + return nil + } + assertPackage := func(t *testing.T, got *fleet.HostSoftwareWithInstaller, name, version string) { + t.Helper() + require.NotNil(t, got.SoftwarePackage) + require.Equal(t, name, got.SoftwarePackage.Name) + require.Equal(t, version, got.SoftwarePackage.Version) + } + + baseTime := time.Now().Add(-time.Hour).UTC().Truncate(time.Microsecond) + + t.Run("first added in scope installer", func(t *testing.T) { + host := newHost("multi-install-details-both", true) + seedInstall(host.ID, installerA, "both-a", baseTime, 0) + seedInstall(host.ID, installerB, "both-b", baseTime.Add(time.Minute), 0) + + got := getTitle(t, host) + assertPackage(t, got, "package-a.pkg", "1.0") + require.Equal(t, new(fleet.SoftwareInstalled), got.Status) + require.NotNil(t, got.SoftwarePackage.LastInstall) + require.Equal(t, "both-a", got.SoftwarePackage.LastInstall.InstallUUID) + }) + + t.Run("only second installer in scope", func(t *testing.T) { + host := newHost("multi-install-details-second", false) + seedInstall(host.ID, installerB, "second-b", baseTime, 0) + seedInstall(host.ID, installerA, "second-a", baseTime.Add(time.Minute), 0) + + got := getTitle(t, host) + assertPackage(t, got, "package-b.pkg", "2.0") + require.Equal(t, new(fleet.SoftwareInstalled), got.Status) + require.NotNil(t, got.SoftwarePackage.LastInstall) + require.Equal(t, "second-b", got.SoftwarePackage.LastInstall.InstallUUID) + }) + + t.Run("resolved installer has no install", func(t *testing.T) { + host := newHost("multi-install-details-sibling-only", true) + seedInstall(host.ID, installerB, "sibling-only-b", baseTime, 0) + + got := getTitle(t, host) + assertPackage(t, got, "package-a.pkg", "1.0") + require.Nil(t, got.Status) + require.Nil(t, got.SoftwarePackage.LastInstall) + }) + + t.Run("most recent install for resolved installer", func(t *testing.T) { + host := newHost("multi-install-details-recency", true) // resolved installer = A + seedInstall(host.ID, installerA, "recency-a-old", baseTime, 0) + seedInstall(host.ID, installerA, "recency-a-new", baseTime.Add(time.Minute), 0) + // Sibling B installed more recently than either A install. The result must be the resolved + // installer's own most-recent install, not the globally-most-recent (B) one — this is what + // discriminates the fix from the pre-#49208 behavior, where an unordered merge could surface + // B's UUID next to A's name/version. + seedInstall(host.ID, installerB, "recency-b-newest", baseTime.Add(2*time.Minute), 0) + + got := getTitle(t, host) + assertPackage(t, got, "package-a.pkg", "1.0") + require.Equal(t, new(fleet.SoftwareInstalled), got.Status) + require.NotNil(t, got.SoftwarePackage.LastInstall) + require.Equal(t, "recency-a-new", got.SoftwarePackage.LastInstall.InstallUUID) + }) + + t.Run("status and last install use resolved installer", func(t *testing.T) { + host := newHost("multi-install-details-status", true) + seedInstall(host.ID, installerA, "status-a-failed", baseTime, 1) + seedInstall(host.ID, installerB, "status-b-installed", baseTime.Add(time.Minute), 0) + + got := getTitle(t, host) + assertPackage(t, got, "package-a.pkg", "1.0") + require.Equal(t, new(fleet.SoftwareInstallFailed), got.Status) + require.NotNil(t, got.SoftwarePackage.LastInstall) + require.Equal(t, "status-a-failed", got.SoftwarePackage.LastInstall.InstallUUID) + }) + + t.Run("uninstall recency is per installer", func(t *testing.T) { + host := newHost("multi-install-details-uninstall", true) + seedInstall(host.ID, installerA, "uninstall-a-install", baseTime, 0) + seedUninstall(host.ID, installerA, "uninstall-a", baseTime.Add(time.Minute)) + seedInstall(host.ID, installerB, "uninstall-b-install", baseTime.Add(2*time.Minute), 0) + + got := getTitle(t, host) + assertPackage(t, got, "package-a.pkg", "1.0") + require.Nil(t, got.Status) + require.NotNil(t, got.SoftwarePackage.LastInstall) + require.Equal(t, "uninstall-a-install", got.SoftwarePackage.LastInstall.InstallUUID) + require.NotNil(t, got.SoftwarePackage.LastUninstall) + require.Equal(t, "uninstall-a", got.SoftwarePackage.LastUninstall.ExecutionID) + }) + + t.Run("pending uninstall without install uses resolved installer", func(t *testing.T) { + host := newHost("multi-install-details-pending-uninstall", true) + seedUpcoming(host.ID, installerA, "pending-uninstall-a", "software_uninstall", baseTime) + seedInstall(host.ID, installerB, "pending-uninstall-b-install", baseTime.Add(time.Minute), 0) + + got := getTitle(t, host) + assertPackage(t, got, "package-a.pkg", "1.0") + require.Equal(t, new(fleet.SoftwareUninstallPending), got.Status) + require.Nil(t, got.SoftwarePackage.LastInstall) + require.NotNil(t, got.SoftwarePackage.LastUninstall) + require.Equal(t, "pending-uninstall-a", got.SoftwarePackage.LastUninstall.ExecutionID) + }) +} diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 422a0dbd166..77cb3e87c1d 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -33958,6 +33958,47 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareMultiplePackagesInstallPrec s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", hostBoth.ID, titleID), nil, http.StatusAccepted, &resp) require.Equal(t, installerA, queuedInstallerID(hostBoth.ID)) + var installAUUID string + mysqltest.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &installAUUID, ` + SELECT execution_id FROM host_software_installs + WHERE host_id = ? AND software_installer_id = ? + ORDER BY id DESC LIMIT 1`, hostBoth.ID, installerA) + }) + _, err = s.ds.SetHostSoftwareInstallResult(ctx, &fleet.HostSoftwareInstallResultPayload{ + HostID: hostBoth.ID, + InstallUUID: installAUUID, + InstallScriptExitCode: new(int(0)), + }, nil) + require.NoError(t, err) + + installBUUID, err := s.ds.InsertSoftwareInstallRequest(ctx, hostBoth.ID, installerB, fleet.HostSoftwareInstallOptions{}) + require.NoError(t, err) + _, err = s.ds.SetHostSoftwareInstallResult(ctx, &fleet.HostSoftwareInstallResultPayload{ + HostID: hostBoth.ID, + InstallUUID: installBUUID, + InstallScriptExitCode: new(int(0)), + }, nil) + require.NoError(t, err) + + var hostSoftwareResp getHostSoftwareResponse + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", hostBoth.ID), nil, http.StatusOK, &hostSoftwareResp, + "include_available_for_install", "true") + var title *fleet.HostSoftwareWithInstaller + for _, software := range hostSoftwareResp.Software { + if software.ID == titleID { + title = software + break + } + } + require.NotNil(t, title) + require.NotNil(t, title.SoftwarePackage) + require.Equal(t, "pkgA.deb", title.SoftwarePackage.Name) + require.Equal(t, "1.0", title.SoftwarePackage.Version) + require.Equal(t, new(fleet.SoftwareInstalled), title.Status) + require.NotNil(t, title.SoftwarePackage.LastInstall) + require.Equal(t, installAUUID, title.SoftwarePackage.LastInstall.InstallUUID) + // Host in only labelB matches only pkgB → pkgB installs (never the first-added pkgA). hostSecond := newLinuxHost("second", []uint{labelB.ID}) s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", hostSecond.ID, titleID), nil, http.StatusAccepted, &resp) From 37cd3bfbbecfdf8ee33bb837e93606cc7b6caaf7 Mon Sep 17 00:00:00 2001 From: Carlo DiCelico Date: Wed, 15 Jul 2026 10:53:56 -0400 Subject: [PATCH 2/4] fix out-of-scope install pruning --- server/datastore/mysql/software.go | 22 +++++--- server/datastore/mysql/software_test.go | 71 +++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index 7c6b9ca2d43..ba96fc300a7 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -3700,6 +3700,9 @@ func hostSoftwareInstalls(ds *Datastore, ctx context.Context, hostID uint) ([]*h AND si_active.is_active = 1 ) END + -- deterministic order so the first row kept per title (and its self_service) does not depend + -- on unordered UNION output; matched installer first, NULLs last. + ORDER BY software_titles.id, matched_installer.id IS NULL, matched_installer.id, lsia.installer_id ` var softwareInstalls []*hostSoftware err := sqlx.SelectContext(ctx, ds.reader(ctx), &softwareInstalls, softwareInstallsStmt, hostID, hostID) @@ -3794,6 +3797,8 @@ func hostSoftwareUninstalls(ds *Datastore, ctx context.Context, hostID uint) ([] END LEFT OUTER JOIN host_script_results ON host_script_results.host_id = ? AND host_script_results.execution_id = lsua.last_uninstall_script_execution_id + -- deterministic order so the first row kept per title does not depend on unordered UNION output. + ORDER BY software_titles.id, matched_installer.id IS NULL, matched_installer.id, lsua.installer_id ` var softwareUninstalls []*hostSoftware err := sqlx.SelectContext(ctx, ds.reader(ctx), &softwareUninstalls, softwareUninstallsStmt, hostID, hostID, hostID) @@ -5254,17 +5259,18 @@ func mergeUninstallDataByInstaller(installDataByTitleInstaller map[uint]map[uint // applyResolvedInstallerStatus pins each title's status/last-install/last-uninstall fields to the // installer that ListHostSoftware resolved for display (resolvedInstallers[titleID].ID), so those -// fields describe the SAME installer as the shown name/version. If the resolved installer has no -// install record on the host, the fields are left cleared (available) rather than borrowing a -// sibling installer's data. This is the fix for #49208. +// fields describe the same installer as the shown name/version. If the resolved installer has no +// install record on the host, the fields are cleared (available) rather than borrowing a sibling +// installer's data. Only in-scope titles (those in filteredBySoftwareTitleID) are pinned: out-of-scope +// titles keep their provisional status so downstream pruning and self-service filtering still see it. func applyResolvedInstallerStatus( - bySoftwareTitleID map[uint]*hostSoftware, + filteredBySoftwareTitleID map[uint]*hostSoftware, installDataByTitleInstaller map[uint]map[uint]*hostSoftware, resolvedInstallers map[uint]resolvedInstaller, ) { - for titleID, resolved := range resolvedInstallers { - software := bySoftwareTitleID[titleID] - if software == nil { + for titleID, software := range filteredBySoftwareTitleID { + resolved, ok := resolvedInstallers[titleID] + if !ok || software == nil { continue } @@ -6232,7 +6238,7 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt if err != nil { return nil, nil, err } - applyResolvedInstallerStatus(bySoftwareTitleID, installDataByTitleInstaller, resolvedInstallers) + applyResolvedInstallerStatus(filteredBySoftwareTitleID, installDataByTitleInstaller, resolvedInstallers) // filter out VPP apps due to label scoping filteredByVPPAdamID, otherVppAppsInInventory, err := filterVPPAppsByLabel( diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index dff7be93e72..8db4bd0489f 100644 --- a/server/datastore/mysql/software_test.go +++ b/server/datastore/mysql/software_test.go @@ -108,6 +108,7 @@ func TestSoftware(t *testing.T) { {"TestListHostSoftwareWithLabelScoping", testListHostSoftwareWithLabelScoping}, {"ListHostSoftwareMultiplePackagesPrecedence", testListHostSoftwareMultiplePackagesPrecedence}, {"ListHostSoftwareMultiplePackagesInstallDetails", testListHostSoftwareMultiplePackagesInstallDetails}, + {"ListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned", testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned}, {"TestListHostSoftwareVulnerableAndVPP", testListHostSoftwareVulnerableAndVPP}, {"TestListHostSoftwareQuerySearching", testListHostSoftwareQuerySearching}, {"TestListHostSoftwareWithLabelScopingVPP", testListHostSoftwareWithLabelScopingVPP}, @@ -13892,9 +13893,8 @@ func testListHostSoftwareMultiplePackagesInstallDetails(t *testing.T, ds *Datast seedInstall(host.ID, installerA, "recency-a-old", baseTime, 0) seedInstall(host.ID, installerA, "recency-a-new", baseTime.Add(time.Minute), 0) // Sibling B installed more recently than either A install. The result must be the resolved - // installer's own most-recent install, not the globally-most-recent (B) one — this is what - // discriminates the fix from the pre-#49208 behavior, where an unordered merge could surface - // B's UUID next to A's name/version. + // installer's own most-recent install, not the globally-most-recent (B) one. An unordered + // merge could otherwise surface B's UUID next to A's name/version. seedInstall(host.ID, installerB, "recency-b-newest", baseTime.Add(2*time.Minute), 0) got := getTitle(t, host) @@ -13944,3 +13944,68 @@ func testListHostSoftwareMultiplePackagesInstallDetails(t *testing.T, ds *Datast require.Equal(t, "pending-uninstall-a", got.SoftwarePackage.LastUninstall.ExecutionID) }) } + +// testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned verifies that a title the host is out +// of scope for, whose only trace is a failed install on a sibling package, is pruned from the +// available-for-install list rather than leaking in. Guards against applyResolvedInstallerStatus +// clearing the failed status before the out-of-scope pruning reads it. +func testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned(t *testing.T, ds *Datastore) { + ctx := t.Context() + user := test.NewUser(t, ds, "Bob", "bob-oos-failed@example.com", true) + labelA, err := ds.NewLabel(ctx, &fleet.Label{Name: "oosA" + t.Name()}) + require.NoError(t, err) + labelB, err := ds.NewLabel(ctx, &fleet.Label{Name: "oosB" + t.Name()}) + require.NoError(t, err) + + newScopedPackage := func(storageID, filename, version string, label *fleet.Label) (uint, uint) { + tfr, err := fleet.NewTempFileReader(strings.NewReader(filename), t.TempDir) + require.NoError(t, err) + installerID, titleID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + InstallScript: "install", + UninstallScript: "uninstall", + InstallerFile: tfr, + StorageID: storageID, + Filename: filename, + Title: "OOSFailedPrune", + Version: version, + Source: "apps", + BundleIdentifier: "com.example.oos-failed-prune", + UserID: user.ID, + Platform: "darwin", + ValidatedLabels: &fleet.LabelIdentsWithScope{ + LabelScope: fleet.LabelScopeIncludeAny, + ByName: map[string]fleet.LabelIdent{label.Name: {LabelName: label.Name, LabelID: label.ID}}, + }, + }) + require.NoError(t, err) + return installerID, titleID + } + + _, titleID := newScopedPackage("oos-a", "oos-a.pkg", "1.0", labelA) + installerB, titleIDB := newScopedPackage("oos-b", "oos-b.pkg", "2.0", labelB) + require.Equal(t, titleID, titleIDB) + + // Host is a member of neither label, so it is out of scope for both packages. + host := test.NewHost(t, ds, "oos-failed-host", "", "oos-failed-key", "oos-failed-uuid", time.Now(), test.WithPlatform("darwin")) + host.LabelUpdatedAt = time.Now() + require.NoError(t, ds.UpdateHost(ctx, host)) + + // A failed install recorded against sibling B (non-zero exit code yields failed_install). + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, ` + INSERT INTO host_software_installs + (execution_id, host_id, software_installer_id, install_script_exit_code, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?)`, + "oos-b-failed", host.ID, installerB, 1, time.Now(), time.Now()) + return err + }) + + software, _, err := ds.ListHostSoftware(ctx, host, fleet.HostSoftwareTitleListOptions{ + ListOptions: fleet.ListOptions{PerPage: 50, IncludeMetadata: true, OrderKey: "name"}, + IncludeAvailableForInstall: true, + }) + require.NoError(t, err) + for _, item := range software { + require.NotEqualf(t, titleID, item.ID, "out-of-scope title with only a failed sibling install should be pruned, not listed") + } +} From f58029b4fa65b1470eba46b6f5e9a549f77ea3f0 Mon Sep 17 00:00:00 2001 From: Carlo DiCelico Date: Wed, 15 Jul 2026 11:30:01 -0400 Subject: [PATCH 3/4] add prune positive-control test; document status coupling --- server/datastore/mysql/software.go | 6 +++ server/datastore/mysql/software_test.go | 60 ++++++++++++++++--------- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index ba96fc300a7..dd5443c30a1 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -5166,6 +5166,12 @@ func (a *hostSoftwareTitleAssembler) addRecord( // filterOutOfScopeFailedHostSoftwareInstalls removes failed install entries that are not in // the osquery inventory and whose installer is out of label scope, so they don't surface // as available software on the host. Maps are mutated in place. +// filterOutOfScopeFailedHostSoftwareInstalls drops titles the host is out of scope for and not +// osquery-reporting as installed, whose status is a failed install, so a stale failed attempt on a +// title the host can no longer install doesn't linger. For a multi-package title the Status read +// here is the provisional per-title value (the first-added installer with a record, ordered by the +// query): out-of-scope titles are never pinned by applyResolvedInstallerStatus, so the prune +// deliberately reflects the first-added installer's outcome. Non-failed out-of-scope titles are kept. func filterOutOfScopeFailedHostSoftwareInstalls( bySoftwareTitleID map[uint]*hostSoftware, byVPPAdamID map[string]*hostSoftware, diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index 8db4bd0489f..6d8421a28e7 100644 --- a/server/datastore/mysql/software_test.go +++ b/server/datastore/mysql/software_test.go @@ -13945,10 +13945,11 @@ func testListHostSoftwareMultiplePackagesInstallDetails(t *testing.T, ds *Datast }) } -// testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned verifies that a title the host is out -// of scope for, whose only trace is a failed install on a sibling package, is pruned from the -// available-for-install list rather than leaking in. Guards against applyResolvedInstallerStatus -// clearing the failed status before the out-of-scope pruning reads it. +// testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned verifies that the out-of-scope +// failed-install prune stays selective for multi-package titles. A title the host is out of scope +// for, whose first-added installer failed, is pruned even if a sibling later succeeded (it is not +// installable and not in inventory). A title whose first-added installer succeeded is kept, proving +// the prune keys on the (provisional) status rather than blanket-dropping every out-of-scope title. func testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned(t *testing.T, ds *Datastore) { ctx := t.Context() user := test.NewUser(t, ds, "Bob", "bob-oos-failed@example.com", true) @@ -13957,8 +13958,8 @@ func testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned(t *testing.T, labelB, err := ds.NewLabel(ctx, &fleet.Label{Name: "oosB" + t.Name()}) require.NoError(t, err) - newScopedPackage := func(storageID, filename, version string, label *fleet.Label) (uint, uint) { - tfr, err := fleet.NewTempFileReader(strings.NewReader(filename), t.TempDir) + newScopedPackage := func(title, bundleID, storageID, filename, version string, label *fleet.Label) (uint, uint) { + tfr, err := fleet.NewTempFileReader(strings.NewReader(storageID), t.TempDir) require.NoError(t, err) installerID, titleID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ InstallScript: "install", @@ -13966,10 +13967,10 @@ func testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned(t *testing.T, InstallerFile: tfr, StorageID: storageID, Filename: filename, - Title: "OOSFailedPrune", + Title: title, Version: version, Source: "apps", - BundleIdentifier: "com.example.oos-failed-prune", + BundleIdentifier: bundleID, UserID: user.ID, Platform: "darwin", ValidatedLabels: &fleet.LabelIdentsWithScope{ @@ -13981,31 +13982,46 @@ func testListHostSoftwareMultiPackageOutOfScopeFailedInstallPruned(t *testing.T, return installerID, titleID } - _, titleID := newScopedPackage("oos-a", "oos-a.pkg", "1.0", labelA) - installerB, titleIDB := newScopedPackage("oos-b", "oos-b.pkg", "2.0", labelB) - require.Equal(t, titleID, titleIDB) + seedInstall := func(hostID, installerID uint, executionID string, exitCode int) { + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, ` + INSERT INTO host_software_installs + (execution_id, host_id, software_installer_id, install_script_exit_code, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?)`, + executionID, hostID, installerID, exitCode, time.Now(), time.Now()) + return err + }) + } - // Host is a member of neither label, so it is out of scope for both packages. + // Host is a member of neither label, so it is out of scope for every package below. host := test.NewHost(t, ds, "oos-failed-host", "", "oos-failed-key", "oos-failed-uuid", time.Now(), test.WithPlatform("darwin")) host.LabelUpdatedAt = time.Now() require.NoError(t, ds.UpdateHost(ctx, host)) - // A failed install recorded against sibling B (non-zero exit code yields failed_install). - ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { - _, err := q.ExecContext(ctx, ` - INSERT INTO host_software_installs - (execution_id, host_id, software_installer_id, install_script_exit_code, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?)`, - "oos-b-failed", host.ID, installerB, 1, time.Now(), time.Now()) - return err - }) + // Title 1: two active packages; first-added A failed, sibling B later succeeded. Out of scope and + // not in osquery inventory, so it is pruned (a stale failed attempt on an uninstallable title). + installerA, prunedTitleID := newScopedPackage("OOSFailedPrune", "com.example.oos-failed-prune", "oos-a", "oos-a.pkg", "1.0", labelA) + installerB, titleIDB := newScopedPackage("OOSFailedPrune", "com.example.oos-failed-prune", "oos-b", "oos-b.pkg", "2.0", labelB) + require.Equal(t, prunedTitleID, titleIDB) + require.Less(t, installerA, installerB) + seedInstall(host.ID, installerA, "oos-a-failed", 1) + seedInstall(host.ID, installerB, "oos-b-success", 0) + + // Title 2 (positive control): out of scope, first-added installer succeeded, so the prune must not + // drop it. Guards against a regression that blanket-removes every out-of-scope title. + installerC, keptTitleID := newScopedPackage("OOSSuccessKept", "com.example.oos-success-kept", "oos-c", "oos-c.pkg", "1.0", labelA) + require.NotEqual(t, prunedTitleID, keptTitleID) + seedInstall(host.ID, installerC, "oos-c-success", 0) software, _, err := ds.ListHostSoftware(ctx, host, fleet.HostSoftwareTitleListOptions{ ListOptions: fleet.ListOptions{PerPage: 50, IncludeMetadata: true, OrderKey: "name"}, IncludeAvailableForInstall: true, }) require.NoError(t, err) + listed := make(map[uint]struct{}, len(software)) for _, item := range software { - require.NotEqualf(t, titleID, item.ID, "out-of-scope title with only a failed sibling install should be pruned, not listed") + listed[item.ID] = struct{}{} } + require.NotContains(t, listed, prunedTitleID, "out-of-scope title whose first-added installer failed should be pruned") + require.Contains(t, listed, keptTitleID, "out-of-scope title whose first-added installer succeeded must remain (prune is status-selective)") } From 9409365b85dd4a00543c7550621924a37c7d36c1 Mon Sep 17 00:00:00 2001 From: Carlo DiCelico Date: Wed, 15 Jul 2026 11:32:44 -0400 Subject: [PATCH 4/4] clean up duplicate doc comment --- server/datastore/mysql/software.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index dd5443c30a1..324df2f77cb 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -5163,15 +5163,13 @@ func (a *hostSoftwareTitleAssembler) addRecord( } } -// filterOutOfScopeFailedHostSoftwareInstalls removes failed install entries that are not in -// the osquery inventory and whose installer is out of label scope, so they don't surface -// as available software on the host. Maps are mutated in place. // filterOutOfScopeFailedHostSoftwareInstalls drops titles the host is out of scope for and not // osquery-reporting as installed, whose status is a failed install, so a stale failed attempt on a // title the host can no longer install doesn't linger. For a multi-package title the Status read // here is the provisional per-title value (the first-added installer with a record, ordered by the // query): out-of-scope titles are never pinned by applyResolvedInstallerStatus, so the prune // deliberately reflects the first-added installer's outcome. Non-failed out-of-scope titles are kept. +// Maps are mutated in place. func filterOutOfScopeFailedHostSoftwareInstalls( bySoftwareTitleID map[uint]*hostSoftware, byVPPAdamID map[string]*hostSoftware,