From ae875199383adb48cceac204cde72fa7fe34a0fc Mon Sep 17 00:00:00 2001
From: Tim Lee <timlee@fleetdm.com>
Date: Mon, 29 Jan 2024 13:04:26 -0700
Subject: [PATCH] 16356 Unreleased bugfix for osversions filter (#16391)

---
 server/datastore/mysql/hosts.go         | 20 ++++++--------------
 server/datastore/mysql/hosts_test.go    |  8 ++++++++
 server/service/integration_core_test.go |  7 ++++++-
 server/vulnerabilities/msrc/analyzer.go |  6 ++----
 4 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go
index 9c15f2e860..fe4f6ca4a6 100644
--- a/server/datastore/mysql/hosts.go
+++ b/server/datastore/mysql/hosts.go
@@ -4416,26 +4416,18 @@ WHERE
 		}
 	}
 
-	// filter counts by platform
-	if platform != nil {
-		var filtered []fleet.OSVersion
-		for _, os := range counts {
-			if *platform == os.Platform {
-				filtered = append(filtered, os)
-			}
+	// filter by platform, name, and version
+	var filtered []fleet.OSVersion
+	for _, os := range counts {
+		if (platform == nil || *platform == os.Platform) && (name == nil || version == nil || (*name == os.NameOnly && *version == os.Version)) {
+			filtered = append(filtered, os)
 		}
-		counts = filtered
 	}
+	counts = filtered
 
 	// aggregate counts by name and version
 	byNameVers := make(map[string]fleet.OSVersion)
 	for _, os := range counts {
-		if name != nil &&
-			version != nil &&
-			*name != os.NameOnly &&
-			*version != os.Version {
-			continue
-		}
 		key := fmt.Sprintf("%s %s", os.NameOnly, os.Version)
 		val, ok := byNameVers[key]
 		if !ok {
diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go
index 74060c3e97..9f651fe7bf 100644
--- a/server/datastore/mysql/hosts_test.go
+++ b/server/datastore/mysql/hosts_test.go
@@ -6087,6 +6087,14 @@ func testOSVersions(t *testing.T, ds *Datastore) {
 	}
 	require.Equal(t, expected, osVersions.OSVersions)
 
+	// filter by operating system that has multiple versions
+	expected = []fleet.OSVersion{
+		{HostsCount: 3, Name: "macOS 12.3.0", NameOnly: "macOS", Version: "12.3.0", Platform: "darwin"},
+	}
+	osVersions, err = ds.OSVersions(ctx, nil, nil, ptr.String("macOS"), ptr.String("12.3.0"))
+	require.NoError(t, err)
+	require.Equal(t, expected, osVersions.OSVersions)
+
 	// team 1
 	osVersions, err = ds.OSVersions(ctx, &team1.ID, nil, nil, nil)
 	require.NoError(t, err)
diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go
index 72b6345c96..3b49c3b59d 100644
--- a/server/service/integration_core_test.go
+++ b/server/service/integration_core_test.go
@@ -7398,7 +7398,6 @@ func (s *integrationTestSuite) TestOSVersions() {
 	// get OS versions
 	osv, err := s.ds.ListOperatingSystems(context.Background())
 	require.NoError(t, err)
-	require.Len(t, osv, 6) // includes fooOS from another test
 
 	osvMap := make(map[string]fleet.OperatingSystem)
 	for _, os := range osv {
@@ -7463,6 +7462,12 @@ func (s *integrationTestSuite) TestOSVersions() {
 		},
 	}, osVersionsResp.OSVersions[0])
 
+	// name and version filters
+	s.DoJSON("GET", "/api/latest/fleet/os_versions", nil, http.StatusOK, &osVersionsResp, "os_name", "Windows 11 Pro 21H2", "os_version", "10.0.22000.2")
+	require.Len(t, osVersionsResp.OSVersions, 1)
+	require.Equal(t, "Windows 11 Pro 21H2 10.0.22000.2", osVersionsResp.OSVersions[0].Name)
+	require.Len(t, osVersionsResp.OSVersions[0].Vulnerabilities, 2)
+
 	// name without version
 	s.DoJSON("GET", "/api/latest/fleet/os_versions", nil, http.StatusBadRequest, &osVersionsResp, "os_name", "Windows 11 Pro 21H2")
 
diff --git a/server/vulnerabilities/msrc/analyzer.go b/server/vulnerabilities/msrc/analyzer.go
index f629cc67bf..ed5130e41d 100644
--- a/server/vulnerabilities/msrc/analyzer.go
+++ b/server/vulnerabilities/msrc/analyzer.go
@@ -138,10 +138,8 @@ func patched(
 		}
 
 		isGreater, err := winBuildVersionGreaterOrEqual(fix.FixedBuild, os.KernelVersion)
-		if err != nil {
-			continue
-		}
-		if isGreater {
+		// Return true on errors to prevent false positives
+		if err != nil || isGreater {
 			return true
 		}
 	}