Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

Fix software panic (#28674)

Browse Source 
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.
This commit is contained in:
Scott Gress
2025-04-30 17:00:28 -05:00
committed by GitHub
parent 6ef76be4be
commit e23fa23a49
1 changed files with 25 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/datastore/mysql/software.go
+25 -8
View File
@@ -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)
}