From e23fa23a4977a95324c1b07cc1c2c0226c7795f6 Mon Sep 17 00:00:00 2001
From: Scott Gress <scottmgress@gmail.com>
Date: Wed, 30 Apr 2025 17:00:28 -0500
Subject: [PATCH] Fix software panic (#28674)

For #28626

This PR updates the logic that decides when to update host software
records. Previously, records were never updated if the incoming software
item had no "last opened" time. This is now amended to still perform an
update in that case IFF the current software item _also_ has no "last
opened" time, _and_ the software item has been marked as having a name
change. Otherwise, updates are only performed if the new item has been
opened much more recently than the current one.

I was unable to reproduce a real-world scenario that would have led to
the panic, but by setting a software item to always be marked as having
no "last opened" time in the code, I was able to force the condition on
the main branch. On this branch, the code executes without error even
with that hacked software item in place.

My suspicion is that this could be caused by having multiple copies of a
software item in different locations on a system (e.g. in Applications
and Downloads), but I wasn't able to get the conditions quite right to
prove it.
---
 server/datastore/mysql/software.go | 33 ++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go
index b719f285ee..5547da97b9 100644
--- a/server/datastore/mysql/software.go
+++ b/server/datastore/mysql/software.go
@@ -16,6 +16,7 @@ import (
 	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/fleet"
 	"github.com/fleetdm/fleet/v4/server/ptr"
+	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
 	"github.com/google/uuid"
 	"github.com/jmoiron/sqlx"
@@ -388,7 +389,7 @@ func (ds *Datastore) applyChangesForNewSoftwareDB(
 				return err
 			}
 
-			if err = updateModifiedHostSoftwareDB(ctx, tx, hostID, current, incoming, existingBundleIDsToUpdate, ds.minLastOpenedAtDiff); err != nil {
+			if err = updateModifiedHostSoftwareDB(ctx, tx, hostID, current, incoming, existingBundleIDsToUpdate, ds.minLastOpenedAtDiff, ds.logger); err != nil {
 				return err
 			}
 
@@ -957,18 +958,34 @@ func updateModifiedHostSoftwareDB(
 	incomingMap map[string]fleet.Software,
 	existingBundleIDsToUpdate map[string]fleet.Software,
 	minLastOpenedAtDiff time.Duration,
+	logger log.Logger,
 ) error {
 	var keysToUpdate []string
 	for key, newSw := range incomingMap {
 		curSw, ok := currentMap[key]
-		if !ok || newSw.LastOpenedAt == nil {
-			// software must also exist in current map, and new software must have a
-			// last opened at timestamp (otherwise we don't overwrite the old one)
-			if _, ok := existingBundleIDsToUpdate[newSw.BundleIdentifier]; !ok {
-				continue
-			}
+		// software must exist in current map for us to update it.
+		if !ok {
+			continue
 		}
-
+		// if the new software has no last opened timestamp, we only
+		// update if the current software has no last opened timestamp
+		// and is marked as having a name change.
+		if newSw.LastOpenedAt == nil {
+			if _, ok := existingBundleIDsToUpdate[newSw.BundleIdentifier]; ok && curSw.LastOpenedAt == nil {
+				keysToUpdate = append(keysToUpdate, key)
+			}
+			// Log cases where the new software has no last opened timestamp, the current software does,
+			// and the software is marked as having a name change.
+			if ok && curSw.LastOpenedAt != nil {
+				level.Warn(logger).Log(
+					"msg", "updateModifiedHostSoftwareDB: last opened at is nil for new software, but not for current software",
+					"new_software", newSw.Name, "current_software", curSw.Name,
+					"bundle_identifier", newSw.BundleIdentifier,
+				)
+			}
+			continue
+		}
+		// update if the new software has been opened more recently.
 		if curSw.LastOpenedAt == nil || newSw.LastOpenedAt.Sub(*curSw.LastOpenedAt) >= minLastOpenedAtDiff {
 			keysToUpdate = append(keysToUpdate, key)
 		}