From 9ed0c193c8542599616e86f2f41aa843f62dbcd9 Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 20 Feb 2024 09:17:07 -0700 Subject: [PATCH] Pluralize hosts_count (#16907) #16906 **IN DRAFT, WAITING ON https://github.com/fleetdm/fleet/pull/16897** - [X] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [X] Added/updated tests - [X] Manual QA for all new/changed functionality --------- Co-authored-by: Victor Lyuboslavsky Co-authored-by: Victor Lyuboslavsky --- ee/server/service/vulnerabilities.go | 2 +- server/datastore/mysql/vulnerabilities.go | 24 +++--- .../datastore/mysql/vulnerabilities_test.go | 74 +++++++++---------- server/fleet/vulnerabilities.go | 10 +-- server/service/integration_core_test.go | 8 +- server/service/integration_enterprise_test.go | 6 +- server/service/vulnerabilities.go | 2 +- server/service/vulnerabilities_test.go | 4 +- 8 files changed, 65 insertions(+), 65 deletions(-) diff --git a/ee/server/service/vulnerabilities.go b/ee/server/service/vulnerabilities.go index 0d2535a4b4..6c2730f0a4 100644 --- a/ee/server/service/vulnerabilities.go +++ b/ee/server/service/vulnerabilities.go @@ -8,7 +8,7 @@ import ( var eeValidVulnSortColumns = []string{ "cve", - "host_count", + "hosts_count", "created_at", "cvss_score", "epss_probability", diff --git a/server/datastore/mysql/vulnerabilities.go b/server/datastore/mysql/vulnerabilities.go index 237c3f5624..8e5b2b5708 100644 --- a/server/datastore/mysql/vulnerabilities.go +++ b/server/datastore/mysql/vulnerabilities.go @@ -25,8 +25,8 @@ func (ds *Datastore) Vulnerability(ctx context.Context, cve string, teamID *uint cm.cisa_known_exploit, cm.published, cm.description, - COALESCE(vhc.host_count, 0) as host_count, - COALESCE(vhc.updated_at, NOW()) as host_count_updated_at + COALESCE(vhc.host_count, 0) as hosts_count, + COALESCE(vhc.updated_at, NOW()) as hosts_count_updated_at FROM cve_meta cm JOIN ( SELECT cve @@ -49,8 +49,8 @@ func (ds *Datastore) Vulnerability(ctx context.Context, cve string, teamID *uint union_cve.cve, COALESCE(LEAST(osv.created_at, sc.created_at), NOW()) AS created_at, COALESCE(osv.source, sc.source, 0) AS source, - COALESCE(vhc.host_count, 0) as host_count, - COALESCE(vhc.updated_at, NOW()) as host_count_updated_at + COALESCE(vhc.host_count, 0) as hosts_count, + COALESCE(vhc.updated_at, NOW()) as hosts_count_updated_at FROM ( SELECT cve, created_at, source FROM operating_system_vulnerabilities @@ -94,7 +94,7 @@ func (ds *Datastore) Vulnerability(ctx context.Context, cve string, teamID *uint return nil, ctxerr.Wrap(ctx, err, "fetching vulnerability") } - if vuln.HostCount == 0 { + if vuln.HostsCount == 0 { var msg string if teamID == nil { msg = "global" @@ -198,8 +198,8 @@ func (ds *Datastore) ListVulnerabilities(ctx context.Context, opt fleet.VulnList cm.cisa_known_exploit, cm.published, COALESCE(cm.description, '') AS description, - vhc.host_count, - vhc.updated_at as host_count_updated_at + vhc.host_count as hosts_count, + vhc.updated_at as hosts_count_updated_at FROM vulnerability_host_counts vhc LEFT JOIN cve_meta cm ON cm.cve = vhc.cve @@ -212,8 +212,8 @@ func (ds *Datastore) ListVulnerabilities(ctx context.Context, opt fleet.VulnList vhc.cve, MIN(COALESCE(osv.created_at, sc.created_at, NOW())) AS created_at, COALESCE(osv.source, sc.source, 0) AS source, - vhc.host_count, - vhc.updated_at as host_count_updated_at + vhc.host_count as hosts_count, + vhc.updated_at as hosts_count_updated_at FROM vulnerability_host_counts vhc LEFT JOIN operating_system_vulnerabilities osv ON osv.cve = vhc.cve @@ -238,10 +238,10 @@ func (ds *Datastore) ListVulnerabilities(ctx context.Context, opt fleet.VulnList cm.cisa_known_exploit, cm.published, description, - vhc.host_count, - host_count_updated_at + hosts_count, + hosts_count_updated_at ` - freeGroupBy := " GROUP BY vhc.cve, source, vhc.host_count, host_count_updated_at" + freeGroupBy := " GROUP BY vhc.cve, source, hosts_count, hosts_count_updated_at" // Choose the appropriate group by statement based on EE or Free var groupBy string diff --git a/server/datastore/mysql/vulnerabilities_test.go b/server/datastore/mysql/vulnerabilities_test.go index 1edb70a9ea..27945541cd 100644 --- a/server/datastore/mysql/vulnerabilities_test.go +++ b/server/datastore/mysql/vulnerabilities_test.go @@ -108,18 +108,18 @@ func testListVulnerabilities(t *testing.T, ds *Datastore) { Published: ptr.Time(mockTime), Description: "Test CVE 2020-1234", }, - HostCount: 10, - Source: fleet.MSRCSource, + HostsCount: 10, + Source: fleet.MSRCSource, }, "CVE-2020-1235": { - CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1235"}, - HostCount: 15, - Source: fleet.MSRCSource, + CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1235"}, + HostsCount: 15, + Source: fleet.MSRCSource, }, "CVE-2020-1236": { - CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1236"}, - HostCount: 20, - Source: fleet.NVDSource, + CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1236"}, + HostsCount: 20, + Source: fleet.NVDSource, }, } list, _, err = ds.ListVulnerabilities(context.Background(), fleet.VulnListOptions{IsEE: true}) @@ -129,25 +129,25 @@ func testListVulnerabilities(t *testing.T, ds *Datastore) { expectedVuln, ok := expected[vuln.CVE] require.True(t, ok) require.Equal(t, expectedVuln.CVEMeta, vuln.CVEMeta) - require.Equal(t, expectedVuln.HostCount, vuln.HostCount) + require.Equal(t, expectedVuln.HostsCount, vuln.HostsCount) } // Test Fleet Free expected = map[string]fleet.VulnerabilityWithMetadata{ "CVE-2020-1234": { - CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1234"}, - HostCount: 10, - Source: fleet.MSRCSource, + CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1234"}, + HostsCount: 10, + Source: fleet.MSRCSource, }, "CVE-2020-1235": { - CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1235"}, - HostCount: 15, - Source: fleet.MSRCSource, + CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1235"}, + HostsCount: 15, + Source: fleet.MSRCSource, }, "CVE-2020-1236": { - CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1236"}, - HostCount: 20, - Source: fleet.NVDSource, + CVEMeta: fleet.CVEMeta{CVE: "CVE-2020-1236"}, + HostsCount: 20, + Source: fleet.NVDSource, }, } list, _, err = ds.ListVulnerabilities(context.Background(), fleet.VulnListOptions{}) @@ -157,7 +157,7 @@ func testListVulnerabilities(t *testing.T, ds *Datastore) { expectedVuln, ok := expected[vuln.CVE] require.True(t, ok) require.Equal(t, expectedVuln.CVEMeta, vuln.CVEMeta) - require.Equal(t, expectedVuln.HostCount, vuln.HostCount) + require.Equal(t, expectedVuln.HostsCount, vuln.HostsCount) } } @@ -209,23 +209,23 @@ func testVulnerabilityWithOS(t *testing.T, ds *Datastore) { CVEMeta: fleet.CVEMeta{ CVE: "CVE-2020-1234", }, - HostCount: 10, - Source: fleet.MSRCSource, + HostsCount: 10, + Source: fleet.MSRCSource, } // No CVSSScores v, err = ds.Vulnerability(ctx, "CVE-2020-1234", nil, false) require.NoError(t, err) require.Equal(t, expected.CVEMeta, v.CVEMeta) - require.Equal(t, expected.HostCount, v.HostCount) + require.Equal(t, expected.HostsCount, v.HostsCount) require.Equal(t, expected.Source, v.Source) // Team 1 - expected.HostCount = 4 + expected.HostsCount = 4 v, err = ds.Vulnerability(ctx, "CVE-2020-1234", ptr.Uint(1), false) require.NoError(t, err) require.Equal(t, expected.CVEMeta, v.CVEMeta) - require.Equal(t, expected.HostCount, v.HostCount) + require.Equal(t, expected.HostsCount, v.HostsCount) require.Equal(t, expected.Source, v.Source) expected = fleet.VulnerabilityWithMetadata{ @@ -237,15 +237,15 @@ func testVulnerabilityWithOS(t *testing.T, ds *Datastore) { Published: ptr.Time(mockTime), Description: "Test CVE 2020-1234", }, - HostCount: 10, - Source: fleet.MSRCSource, + HostsCount: 10, + Source: fleet.MSRCSource, } // With CVSSScores v, err = ds.Vulnerability(ctx, "CVE-2020-1234", nil, true) require.NoError(t, err) require.Equal(t, expected.CVEMeta, v.CVEMeta) - require.Equal(t, expected.HostCount, v.HostCount) + require.Equal(t, expected.HostsCount, v.HostsCount) require.Equal(t, expected.Source, v.Source) } @@ -293,14 +293,14 @@ func testVulnerabilityWithSoftware(t *testing.T, ds *Datastore) { CVEMeta: fleet.CVEMeta{ CVE: "CVE-2020-1234", }, - HostCount: 10, - Source: fleet.NVDSource, + HostsCount: 10, + Source: fleet.NVDSource, } v, err = ds.Vulnerability(ctx, "CVE-2020-1234", nil, false) require.NoError(t, err) require.Equal(t, expected.CVEMeta, v.CVEMeta) - require.Equal(t, expected.HostCount, v.HostCount) + require.Equal(t, expected.HostsCount, v.HostsCount) require.Equal(t, expected.Source, v.Source) // With CVSSScores @@ -313,14 +313,14 @@ func testVulnerabilityWithSoftware(t *testing.T, ds *Datastore) { Published: ptr.Time(mockTime), Description: "Test CVE 2020-1234", }, - HostCount: 10, - Source: fleet.NVDSource, + HostsCount: 10, + Source: fleet.NVDSource, } v, err = ds.Vulnerability(ctx, "CVE-2020-1234", nil, true) require.NoError(t, err) require.Equal(t, expected.CVEMeta, v.CVEMeta) - require.Equal(t, expected.HostCount, v.HostCount) + require.Equal(t, expected.HostsCount, v.HostsCount) require.Equal(t, expected.Source, v.Source) } @@ -372,7 +372,7 @@ func testVulnerabilitiesTeamFilter(t *testing.T, ds *Datastore) { } for _, vuln := range list { - require.Equal(t, checkCounts[vuln.CVE], int(vuln.HostCount), vuln.CVE) + require.Equal(t, checkCounts[vuln.CVE], int(vuln.HostsCount), vuln.CVE) } } @@ -784,7 +784,7 @@ func testVulnerabilityHostCountBatchInserts(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Len(t, list, 400) for _, vuln := range list { - require.Equal(t, uint(5), vuln.HostCount) + require.Equal(t, uint(5), vuln.HostsCount) } // assert team counts @@ -792,7 +792,7 @@ func testVulnerabilityHostCountBatchInserts(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Len(t, list, 400) for _, vuln := range list { - require.Equal(t, uint(2), vuln.HostCount) + require.Equal(t, uint(2), vuln.HostsCount) } } @@ -875,7 +875,7 @@ func assertHostCounts(t *testing.T, expected []hostCount, actual []fleet.Vulnera require.Len(t, actual, len(expected)) for i, vuln := range actual { require.Equal(t, expected[i].CVE, vuln.CVE) - require.Equal(t, expected[i].HostCount, vuln.HostCount) + require.Equal(t, expected[i].HostCount, vuln.HostsCount) } } diff --git a/server/fleet/vulnerabilities.go b/server/fleet/vulnerabilities.go index 554b4e2233..26bfcd3cfd 100644 --- a/server/fleet/vulnerabilities.go +++ b/server/fleet/vulnerabilities.go @@ -129,11 +129,11 @@ const ( type VulnerabilityWithMetadata struct { CVEMeta - HostCount uint `db:"host_count" json:"host_count"` - HostCountUpdatedAt time.Time `db:"host_count_updated_at" json:"host_count_updated_at"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - DetailsLink string `json:"details_link"` - Source VulnerabilitySource `db:"source" json:"-"` + HostsCount uint `db:"hosts_count" json:"hosts_count"` + HostsCountUpdatedAt time.Time `db:"hosts_count_updated_at" json:"hosts_count_updated_at"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + DetailsLink string `json:"details_link"` + Source VulnerabilitySource `db:"source" json:"-"` } type VulnListOptions struct { diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 10df9624c8..6248ed3e81 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -7574,7 +7574,7 @@ func (s *integrationTestSuite) TestListVulnerabilities() { for _, vuln := range resp.Vulnerabilities { expectedVuln, ok := expected[vuln.CVE] require.True(t, ok) - require.Equal(t, expectedVuln.HostCount, vuln.HostCount) + require.Equal(t, expectedVuln.HostCount, vuln.HostsCount) require.Equal(t, expectedVuln.DetailsLink, vuln.DetailsLink) require.Empty(t, vuln.CVSSScore) } @@ -7601,7 +7601,7 @@ func (s *integrationTestSuite) TestListVulnerabilities() { for _, vuln := range resp.Vulnerabilities { expectedVuln, ok := expected[vuln.CVE] require.True(t, ok) - require.Equal(t, expectedVuln.HostCount, vuln.HostCount) + require.Equal(t, expectedVuln.HostCount, vuln.HostsCount) require.Equal(t, expectedVuln.DetailsLink, vuln.DetailsLink) require.Empty(t, vuln.CVSSScore) } @@ -7620,7 +7620,7 @@ func (s *integrationTestSuite) TestListVulnerabilities() { s.DoJSON("GET", "/api/latest/fleet/vulnerabilities/CVE-2021-1234", nil, http.StatusOK, &gResp) require.Empty(t, gResp.Err) require.Equal(t, "CVE-2021-1234", gResp.Vulnerability.CVE) - require.Equal(t, uint(1), gResp.Vulnerability.HostCount) + require.Equal(t, uint(1), gResp.Vulnerability.HostsCount) require.Equal(t, "https://msrc.microsoft.com/update-guide/en-US/vulnerability/CVE-2021-1234", gResp.Vulnerability.DetailsLink) require.Empty(t, gResp.Vulnerability.Description) require.Empty(t, gResp.Vulnerability.CVSSScore) @@ -7638,7 +7638,7 @@ func (s *integrationTestSuite) TestListVulnerabilities() { s.DoJSON("GET", "/api/latest/fleet/vulnerabilities/CVE-2021-1235", nil, http.StatusOK, &gResp) require.Empty(t, gResp.Err) require.Equal(t, "CVE-2021-1235", gResp.Vulnerability.CVE) - require.Equal(t, uint(1), gResp.Vulnerability.HostCount) + require.Equal(t, uint(1), gResp.Vulnerability.HostsCount) require.Equal(t, "https://nvd.nist.gov/vuln/detail/CVE-2021-1235", gResp.Vulnerability.DetailsLink) require.Empty(t, gResp.Vulnerability.Description) require.Empty(t, gResp.Vulnerability.CVSSScore) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index e92c939dc6..4b0a97f809 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -3266,7 +3266,7 @@ func (s *integrationEnterpriseTestSuite) TestListVulnerabilities() { for _, vuln := range resp.Vulnerabilities { expectedVuln, ok := expected[vuln.CVE] require.True(t, ok) - require.Equal(t, expectedVuln.HostCount, vuln.HostCount) + require.Equal(t, expectedVuln.HostCount, vuln.HostsCount) require.Equal(t, expectedVuln.DetailsLink, vuln.DetailsLink) require.Equal(t, expectedVuln.CVEMeta, vuln.CVEMeta) } @@ -3298,7 +3298,7 @@ func (s *integrationEnterpriseTestSuite) TestListVulnerabilities() { for _, vuln := range resp.Vulnerabilities { expectedVuln, ok := expected[vuln.CVE] require.True(t, ok) - require.Equal(t, expectedVuln.HostCount, vuln.HostCount) + require.Equal(t, expectedVuln.HostCount, vuln.HostsCount) require.Equal(t, expectedVuln.DetailsLink, vuln.DetailsLink) require.Equal(t, expectedVuln.CVEMeta, vuln.CVEMeta) } @@ -3307,7 +3307,7 @@ func (s *integrationEnterpriseTestSuite) TestListVulnerabilities() { s.DoJSON("GET", "/api/latest/fleet/vulnerabilities/CVE-2021-1234", nil, http.StatusOK, &gResp) require.Empty(t, gResp.Err) require.Equal(t, "CVE-2021-1234", gResp.Vulnerability.CVE) - require.Equal(t, uint(1), gResp.Vulnerability.HostCount) + require.Equal(t, uint(1), gResp.Vulnerability.HostsCount) require.Equal(t, "https://msrc.microsoft.com/update-guide/en-US/vulnerability/CVE-2021-1234", gResp.Vulnerability.DetailsLink) require.Equal(t, "Test CVE 2021-1234", gResp.Vulnerability.Description) require.Equal(t, ptr.Float64(7.5), gResp.Vulnerability.CVSSScore) diff --git a/server/service/vulnerabilities.go b/server/service/vulnerabilities.go index 82bf22bcd1..cb46f5480b 100644 --- a/server/service/vulnerabilities.go +++ b/server/service/vulnerabilities.go @@ -12,7 +12,7 @@ import ( var freeValidVulnSortColumns = []string{ "cve", - "host_count", + "hosts_count", "host_count_updated_at", "created_at", } diff --git a/server/service/vulnerabilities_test.go b/server/service/vulnerabilities_test.go index 7c80d9b77a..e9890f08bc 100644 --- a/server/service/vulnerabilities_test.go +++ b/server/service/vulnerabilities_test.go @@ -24,8 +24,8 @@ func TestListVulnerabilities(t *testing.T) { CVE: "CVE-2019-1234", Description: "A vulnerability", }, - CreatedAt: time.Now(), - HostCount: 10, + CreatedAt: time.Now(), + HostsCount: 10, }, }, nil, nil }