[SecuritySolution] Use correct queries and filters for prevalence cal…

…ls (elastic#154544) ## Summary Bug ticket elastic#131967 describes an issue where the alert prevalence count is not correct for fields that have array values (such as `process.args`). ## Solution Getting the correct count for those fields involved adding more `term` conditions to the prevalence query and the timeline filter. This ensures that only alerts with the *exact* same array values match instead of partial matches as before. https://user-images.githubusercontent.com/68591/231395154-b5a1c968-8308-49fb-a218-f3611f8331c3.mov ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Get approval from the product team --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
nikitaindik · Apr 25, 2023 · 5c2f98f · 5c2f98f
1 parent e498344
commit 5c2f98f
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 41 deletions.
diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/investigate_in_timeline.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/investigate_in_timeline.cy.ts
@@ -8,6 +8,7 @@
 import { closeTimeline } from '../../tasks/timeline';
 import { getNewRule } from '../../objects/rule';
 import { PROVIDER_BADGE, QUERY_TAB_BUTTON, TIMELINE_TITLE } from '../../screens/timeline';
+import { FILTER_BADGE } from '../../screens/alerts';
 
 import { expandFirstAlert, investigateFirstAlertInTimeline } from '../../tasks/alerts';
 import { createRule } from '../../tasks/api_calls/rules';
@@ -23,7 +24,6 @@ import {
   INSIGHTS_RELATED_ALERTS_BY_ANCESTRY,
   INSIGHTS_RELATED_ALERTS_BY_SESSION,
   SUMMARY_VIEW_INVESTIGATE_IN_TIMELINE_BUTTON,
-  SUMMARY_VIEW_PREVALENCE_CELL,
 } from '../../screens/alerts_details';
 import { verifyInsightCount } from '../../tasks/alerts_details';
 
@@ -63,19 +63,29 @@ describe('Investigate in timeline', { testIsolation: false }, () => {
     });
 
     it('should open a new timeline from a prevalence field', () => {
-      cy.get(SUMMARY_VIEW_PREVALENCE_CELL)
-        .first()
-        .invoke('text')
-        .then((alertCount) => {
-          // Click on the first button that lets us investigate in timeline
-          cy.get(ALERT_FLYOUT).find(SUMMARY_VIEW_INVESTIGATE_IN_TIMELINE_BUTTON).first().click();
+      // Only one alert matches the exact process args in this case
+      const alertCount = 1;
 
-          // Make sure a new timeline is created and opened
-          cy.get(TIMELINE_TITLE).should('contain.text', 'Untitled timeline');
+      // Click on the last button that lets us investigate in timeline.
+      // We expect this to be the `process.args` row.
+      const investigateButton = cy
+        .get(ALERT_FLYOUT)
+        .find(SUMMARY_VIEW_INVESTIGATE_IN_TIMELINE_BUTTON)
+        .last();
+      investigateButton.should('have.text', alertCount);
+      investigateButton.click();
 
-          // The alert count in this timeline should match the count shown on the alert flyout
-          cy.get(QUERY_TAB_BUTTON).should('contain.text', alertCount);
-        });
+      // Make sure a new timeline is created and opened
+      cy.get(TIMELINE_TITLE).should('have.text', 'Untitled timeline');
+
+      // The alert count in this timeline should match the count shown on the alert flyout
+      cy.get(QUERY_TAB_BUTTON).should('contain.text', alertCount);
+
+      // The correct filter is applied to the timeline query
+      cy.get(FILTER_BADGE).should(
+        'have.text',
+        ' {"bool":{"must":[{"term":{"process.args":"-zsh"}},{"term":{"process.args":"unique"}}]}}'
+      );
     });
 
     it('should open a new timeline from an insights module', () => {

diff --git a/x-pack/plugins/security_solution/cypress/screens/alerts_details.ts b/x-pack/plugins/security_solution/cypress/screens/alerts_details.ts
@@ -68,8 +68,6 @@ export const UPDATE_ENRICHMENT_RANGE_BUTTON = '[data-test-subj="enrichment-butto
 
 export const OVERVIEW_TAB = '[data-test-subj="overviewTab"]';
 
-export const SUMMARY_VIEW_PREVALENCE_CELL = `${SUMMARY_VIEW} [data-test-subj='alert-prevalence']`;
-
 export const SUMMARY_VIEW_INVESTIGATE_IN_TIMELINE_BUTTON = `${SUMMARY_VIEW} [aria-label='Investigate in timeline']`;
 
 export const INSIGHTS_RELATED_ALERTS_BY_SESSION = `[data-test-subj='related-alerts-by-session']`;

diff --git a/...lugins/security_solution/public/common/components/event_details/table/prevalence_cell.tsx b/...lugins/security_solution/public/common/components/event_details/table/prevalence_cell.tsx
@@ -56,6 +56,7 @@ const PrevalenceCell: React.FC<AlertSummaryRow['description']> = ({
       <InvestigateInTimelineButton
         asEmptyButton={true}
         dataProviders={cellDataProviders.dataProviders}
+        filters={cellDataProviders.filters}
       >
         <span data-test-subj="alert-prevalence">{count}</span>
       </InvestigateInTimelineButton>

diff --git a/...ty_solution/public/common/components/event_details/table/use_action_cell_data_provider.ts b/...ty_solution/public/common/components/event_details/table/use_action_cell_data_provider.ts
@@ -7,6 +7,7 @@
 
 /* eslint-disable complexity */
 
+import type { Filter } from '@kbn/es-query';
 import { escapeDataProviderId } from '@kbn/securitysolution-t-grid';
 import { isArray, isEmpty, isString } from 'lodash/fp';
 import { useMemo } from 'react';
@@ -47,6 +48,7 @@ export interface UseActionCellDataProvider {
 export interface ActionCellValuesAndDataProvider {
   values: string[];
   dataProviders: DataProvider[];
+  filters: Filter[];
 }
 
 export const getDataProvider = (
@@ -93,6 +95,23 @@ export const useActionCellDataProvider = ({
   const cellData = useMemo(() => {
     if (values === null || values === undefined) return null;
     const arrayValues = Array.isArray(values) ? values : [values];
+
+    // For fields with multiple values we need add an extra filter that makes sure
+    // that only fields that match ALL the values are queried later on.
+    let filters: Filter[] = [];
+    if (arrayValues.length > 1) {
+      filters = [
+        {
+          meta: {},
+          query: {
+            bool: {
+              must: arrayValues.map((value) => ({ term: { [field]: value } })),
+            },
+          },
+        },
+      ];
+    }
+
     return arrayValues.reduce<ActionCellValuesAndDataProvider>(
       (memo, value, index) => {
         let id: string = '';
@@ -157,7 +176,7 @@ export const useActionCellDataProvider = ({
         memo.dataProviders.push(getDataProvider(field, id, value));
         return memo;
       },
-      { values: [], dataProviders: [] }
+      { values: [], dataProviders: [], filters }
     );
   }, [
     contextId,

diff --git a/x-pack/plugins/security_solution/public/common/containers/alerts/use_alert_prevalence.ts b/x-pack/plugins/security_solution/public/common/containers/alerts/use_alert_prevalence.ts
@@ -68,13 +68,7 @@ export const useAlertPrevalence = ({
   if (data) {
     const buckets = data.aggregations?.[ALERT_PREVALENCE_AGG]?.buckets;
     if (buckets && buckets.length > 0) {
-      /**
-       * Currently for array fields like `process.args` or potentially any `ip` fields
-       * We show the combined count of all occurences of the value, even though those values
-       * could be shared across multiple documents. To make this clearer, we should separate
-       * these values into separate table rows
-       */
-      count = buckets?.reduce((sum, bucket) => sum + (bucket?.doc_count ?? 0), 0);
+      count = buckets[0].doc_count;
     }
   }
 
@@ -132,31 +126,23 @@ const generateAlertPrevalenceQuery = (
     };
   }
 
+  // If we search for the prevalence of a field that has multiple values (e.g. process.args),
+  // we want to find alerts with the exact same values.
   if (Array.isArray(value) && value.length > 1) {
-    const shouldValues = value.map((val) => ({ match: { [field]: val } }));
     query = {
       bool: {
-        minimum_should_match: 1,
-        should: shouldValues,
+        must: value.map((term) => ({ term: { [field]: term } })) as object[],
       },
     };
     if (from !== undefined && to !== undefined) {
-      query = {
-        ...query,
-        bool: {
-          ...query.bool,
-          must: [
-            {
-              range: {
-                '@timestamp': {
-                  gte: from,
-                  lte: to,
-                },
-              },
-            },
-          ],
+      query.bool.must.push({
+        range: {
+          '@timestamp': {
+            gte: from,
+            lte: to,
+          },
         },
-      };
+      });
     }
   }
 

diff --git a/x-pack/test/security_solution_cypress/es_archives/auditbeat/data.json b/x-pack/test/security_solution_cypress/es_archives/auditbeat/data.json
@@ -204,7 +204,8 @@
         "executable" : "/bin/zsh",
         "name" : "zsh",
         "args" : [
-          "-zsh"
+          "-zsh",
+          "unique"
         ],
         "entity_id" : "q6pltOhTWlQx3BCE",
         "entry_leader": {