[Security Solution][Detections] Indicator path followup (#91747)

* Remove eslint issue by not permuting our input We instead return a new object from our enrich function. * Fixes editing of non-indicator rules If the user edits a rule without visiting the About tab, they will receive a value of threatIndicatorPath: '' which we'll then try to send to the backend, but it gets rejected. By removing this defaulting logic we get the correct behavior: existing rules default to threatIndicatorPath: undefined, which gets stripped before being sent to the backend. If the rule is an indicator rule, the value will be persisted as expected. * Move default indicator path to common constant
elastic · Feb 17, 2021 · da780f5 · da780f5
1 parent 9ca4143
commit da780f5
Show file tree

Hide file tree

Showing 10 changed files with 48 additions and 40 deletions.
diff --git a/x-pack/plugins/security_solution/common/constants.ts b/x-pack/plugins/security_solution/common/constants.ts
@@ -45,6 +45,11 @@ export const DEFAULT_RULE_REFRESH_INTERVAL_VALUE = 60000; // ms
 export const DEFAULT_RULE_REFRESH_IDLE_VALUE = 2700000; // ms
 export const DEFAULT_RULE_NOTIFICATION_QUERY_SIZE = 100;
 
+// Document path where threat indicator fields are expected. Used as
+// both the source of enrichment fields and the destination for enrichment in
+// the generated detection alert
+export const DEFAULT_INDICATOR_PATH = 'threat.indicator';
+
 export enum SecurityPageName {
   detections = 'detections',
   overview = 'overview',

diff --git a/...ck/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.mock.ts b/...ck/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.mock.ts
@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { DEFAULT_INDICATOR_PATH } from '../../../constants';
 import {
   MachineLearningCreateSchema,
   MachineLearningUpdateSchema,
@@ -56,7 +57,7 @@ export const getCreateThreatMatchRulesSchemaMock = (
   rule_id: ruleId,
   threat_query: '*:*',
   threat_index: ['list-index'],
-  threat_indicator_path: 'threat.indicator',
+  threat_indicator_path: DEFAULT_INDICATOR_PATH,
   threat_mapping: [
     {
       entries: [

diff --git a/.../plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts b/.../plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts
@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { DEFAULT_INDICATOR_PATH } from '../../../constants';
 import { getListArrayMock } from '../types/lists.mock';
 
 import { RulesSchema } from './rules_schema';
@@ -150,7 +151,7 @@ export const getThreatMatchingSchemaPartialMock = (enabled = false): Partial<Rul
     language: 'kuery',
     threat_query: '*:*',
     threat_index: ['list-index'],
-    threat_indicator_path: 'threat.indicator',
+    threat_indicator_path: DEFAULT_INDICATOR_PATH,
     threat_mapping: [
       {
         entries: [

diff --git a/...ins/security_solution/public/detections/components/rules/step_about_rule/default_value.ts b/...ins/security_solution/public/detections/components/rules/step_about_rule/default_value.ts
@@ -29,7 +29,6 @@ export const stepAboutDefaultValue: AboutStepRule = {
   license: '',
   ruleNameOverride: '',
   tags: [],
-  threatIndicatorPath: '',
   timestampOverride: '',
   threat: threatDefault,
   note: '',

diff --git a/...ck/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx b/...ck/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx
@@ -41,6 +41,7 @@ import { RiskScoreField } from '../risk_score_mapping';
 import { AutocompleteField } from '../autocomplete_field';
 import { useFetchIndex } from '../../../../common/containers/source';
 import { isThreatMatchRule } from '../../../../../common/detection_engine/utils';
+import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
 
 const CommonUseField = getUseField({ component: Field });
 
@@ -309,7 +310,7 @@ const StepAboutRuleComponent: FC<StepAboutRuleProps> = ({
                     euiFieldProps: {
                       fullWidth: true,
                       disabled: isLoading,
-                      placeholder: 'threat.indicator',
+                      placeholder: DEFAULT_INDICATOR_PATH,
                     },
                   }}
                 />

diff --git a/...plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx b/...plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx
@@ -116,7 +116,6 @@ describe('rule helpers', () => {
         severity: { value: 'low', mapping: fillEmptySeverityMappings([]), isMappingChecked: false },
         tags: ['tag1', 'tag2'],
         threat: getThreatMock(),
-        threatIndicatorPath: '',
         timestampOverride: 'event.ingested',
       };
       const scheduleRuleStepData = { from: '0s', interval: '5m' };

diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.tsx
@@ -180,7 +180,7 @@ export const getAboutStepsData = (rule: Rule, detailsView: boolean): AboutStepRu
     },
     falsePositives,
     threat: threat as Threats,
-    threatIndicatorPath: threatIndicatorPath ?? '',
+    threatIndicatorPath,
   };
 };
 

diff --git a/...ty_solution/server/lib/detection_engine/signals/threat_mapping/build_threat_enrichment.ts b/...ty_solution/server/lib/detection_engine/signals/threat_mapping/build_threat_enrichment.ts
@@ -5,13 +5,12 @@
  * 2.0.
  */
 
+import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
 import { SignalSearchResponse, SignalsEnrichment } from '../types';
 import { enrichSignalThreatMatches } from './enrich_signal_threat_matches';
 import { getThreatList } from './get_threat_list';
 import { BuildThreatEnrichmentOptions, GetMatchedThreats } from './types';
 
-const DEFAULT_INDICATOR_PATH = 'threat.indicator';
-
 export const buildThreatEnrichment = ({
   buildRuleMessage,
   exceptionItems,

diff --git a/...n/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.test.ts b/...n/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.test.ts
@@ -6,6 +6,7 @@
  */
 
 import { get } from 'lodash';
+import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
 
 import { getThreatListItemMock } from './build_threat_mapping_filter.mock';
 import {
@@ -93,7 +94,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries: [],
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(indicators).toEqual([]);
@@ -103,7 +104,7 @@ describe('buildMatchedIndicator', () => {
     const [indicator] = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(get(indicator, 'matched.atomic')).toEqual('domain_1');
@@ -113,7 +114,7 @@ describe('buildMatchedIndicator', () => {
     const [indicator] = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(get(indicator, 'matched.field')).toEqual('event.field');
@@ -123,7 +124,7 @@ describe('buildMatchedIndicator', () => {
     const [indicator] = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(get(indicator, 'matched.type')).toEqual('type_1');
@@ -152,7 +153,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(indicators).toHaveLength(queries.length);
@@ -162,7 +163,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(indicators).toEqual([
@@ -227,7 +228,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(indicators).toEqual([
@@ -252,7 +253,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(indicators).toEqual([
@@ -284,7 +285,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: 'threat.indicator',
+      indicatorPath: DEFAULT_INDICATOR_PATH,
     });
 
     expect(indicators).toEqual([
@@ -316,7 +317,7 @@ describe('buildMatchedIndicator', () => {
       buildMatchedIndicator({
         queries,
         threats,
-        indicatorPath: 'threat.indicator',
+        indicatorPath: DEFAULT_INDICATOR_PATH,
       })
     ).toThrowError('Expected indicator field to be an object, but found: not an object');
   });
@@ -337,7 +338,7 @@ describe('buildMatchedIndicator', () => {
       buildMatchedIndicator({
         queries,
         threats,
-        indicatorPath: 'threat.indicator',
+        indicatorPath: DEFAULT_INDICATOR_PATH,
       })
     ).toThrowError('Expected indicator field to be an object, but found: not an object');
   });
@@ -366,7 +367,7 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      'threat.indicator'
+      DEFAULT_INDICATOR_PATH
     );
 
     expect(enrichedSignals.hits.hits).toEqual([]);
@@ -381,10 +382,10 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      'threat.indicator'
+      DEFAULT_INDICATOR_PATH
     );
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, 'threat.indicator');
+    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
 
     expect(indicators).toEqual([
       { existing: 'indicator' },
@@ -406,10 +407,10 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      'threat.indicator'
+      DEFAULT_INDICATOR_PATH
     );
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, 'threat.indicator');
+    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
 
     expect(indicators).toEqual([
       {
@@ -427,10 +428,10 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      'threat.indicator'
+      DEFAULT_INDICATOR_PATH
     );
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, 'threat.indicator');
+    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
 
     expect(indicators).toEqual([
       { existing: 'indicator' },
@@ -450,7 +451,7 @@ describe('enrichSignalThreatMatches', () => {
     });
     const signals = getSignalsResponseMock([signalHit]);
     await expect(() =>
-      enrichSignalThreatMatches(signals, getMatchedThreats, 'threat.indicator')
+      enrichSignalThreatMatches(signals, getMatchedThreats, DEFAULT_INDICATOR_PATH)
     ).rejects.toThrowError('Expected threat field to be an object, but found: whoops');
   });
 
@@ -486,7 +487,7 @@ describe('enrichSignalThreatMatches', () => {
       'custom_threat.custom_indicator'
     );
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, 'threat.indicator');
+    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
 
     expect(indicators).toEqual([
       {
@@ -529,13 +530,13 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      'threat.indicator'
+      DEFAULT_INDICATOR_PATH
     );
     expect(enrichedSignals.hits.total).toEqual(expect.objectContaining({ value: 1 }));
     expect(enrichedSignals.hits.hits).toHaveLength(1);
 
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, 'threat.indicator');
+    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
 
     expect(indicators).toEqual([
       {

diff --git a/...lution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.ts b/...lution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.ts
@@ -6,6 +6,7 @@
  */
 
 import { get, isObject } from 'lodash';
+import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
 
 import type { SignalSearchResponse, SignalSourceHit } from '../types';
 import type {
@@ -91,7 +92,7 @@ export const enrichSignalThreatMatches = async (
     if (!isObject(threat)) {
       throw new Error(`Expected threat field to be an object, but found: ${threat}`);
     }
-    const existingIndicatorValue = get(signalHit._source, 'threat.indicator') ?? [];
+    const existingIndicatorValue = get(signalHit._source, DEFAULT_INDICATOR_PATH) ?? [];
     const existingIndicators = [existingIndicatorValue].flat(); // ensure indicators is an array
 
     return {
@@ -105,14 +106,15 @@ export const enrichSignalThreatMatches = async (
       },
     };
   });
-  /* eslint-disable require-atomic-updates */
-  signals.hits.hits = enrichedSignals;
-  if (isObject(signals.hits.total)) {
-    signals.hits.total.value = enrichedSignals.length;
-  } else {
-    signals.hits.total = enrichedSignals.length;
-  }
-  /* eslint-enable require-atomic-updates */
 
-  return signals;
+  return {
+    ...signals,
+    hits: {
+      ...signals.hits,
+      hits: enrichedSignals,
+      total: isObject(signals.hits.total)
+        ? { ...signals.hits.total, value: enrichedSignals.length }
+        : enrichedSignals.length,
+    },
+  };
 };