PR Feedback

* Stricter threshold type * More robust date parsing * More informative log/error messages * Remove redundant runtime checks
elastic · Mar 18, 2020 · abfcfc1 · abfcfc1
1 parent eed90df
commit abfcfc1
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 11 deletions.
diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/schemas/schemas.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/schemas/schemas.ts
@@ -7,7 +7,10 @@
 import Joi from 'joi';
 
 /* eslint-disable @typescript-eslint/camelcase */
-export const anomaly_threshold = Joi.number();
+export const anomaly_threshold = Joi.number()
+  .integer()
+  .greater(-1)
+  .less(101);
 export const description = Joi.string();
 export const enabled = Joi.boolean();
 export const exclude_export_details = Joi.boolean();

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/find_ml_signals.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/find_ml_signals.ts
@@ -20,8 +20,8 @@ export const findMlSignals = async (
   const params = {
     jobIds: [jobId],
     threshold: anomalyThreshold,
-    earliestMs: dateMath.parse(from)!.valueOf(),
-    latestMs: dateMath.parse(to)!.valueOf(),
+    earliestMs: dateMath.parse(from)?.valueOf() ?? 0,
+    latestMs: dateMath.parse(to)?.valueOf() ?? 0,
   };
   const relevantAnomalies = await getAnomalies(params, callCluster);
 

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/get_filter.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/get_filter.ts
@@ -108,7 +108,9 @@ export const getFilter = async ({
       }
     }
     case 'machine_learning': {
-      throw new Error('getFilter called with a ML Rule');
+      throw new BadRequestError(
+        'Unsupported Rule of type "machine_learning" supplied to getFilter'
+      );
     }
   }
   return assertUnreachable(type);

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/search_after_bulk_create.ts
@@ -54,7 +54,9 @@ export const searchAfterAndBulkCreate = async ({
   }
   const { index, from, to } = ruleParams;
   if (index == null) {
-    throw new Error('Attempted to bulk create signals, but rule had no indexPattern');
+    throw new Error(
+      `Attempted to bulk create signals, but rule id: ${id}, name: ${name}, signals index: ${signalsIndex} has no index pattern`
+    );
   }
 
   logger.debug('[+] starting bulk insertion');

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.ts
@@ -97,7 +97,9 @@ export const signalRulesAlertType = ({
       try {
         if (type === 'machine_learning') {
           if (mlJobId == null || anomalyThreshold == null) {
-            throw new Error('Attempted to execute ML Rule, but missing jobId or anomalyThreshold');
+            throw new Error(
+              `Attempted to execute machine learning rule, but it is missing job id and/or anomaly threshold for rule id: "${ruleId}", name: "${name}", signals index: "${outputIndex}", job id: "${mlJobId}", anomaly threshold: "${anomalyThreshold}"`
+            );
           }
 
           const anomalyResults = await findMlSignals(
@@ -133,7 +135,9 @@ export const signalRulesAlertType = ({
           });
         } else {
           if (index == null) {
-            throw new Error('Attempted to execute Query Rule, but no index was specified');
+            throw new Error(
+              `Attempted to execute query rule, but it is missing index pattern for rule id: "${ruleId}", name: "${name}", signals index: "${outputIndex}", index pattern: "${index}"`
+            );
           }
 
           const inputIndex = await getInputIndex(services, version, index);
@@ -195,7 +199,7 @@ export const signalRulesAlertType = ({
 
         if (creationSucceeded) {
           logger.debug(
-            `Finished signal rule name: "${name}", id: "${alertId}", rule_id: "${ruleId}"`
+            `Finished signal rule name: "${name}", id: "${alertId}", rule_id: "${ruleId}", output_index: "${outputIndex}"`
           );
           await writeCurrentStatusSucceeded({
             services,
@@ -207,7 +211,7 @@ export const signalRulesAlertType = ({
             alertId,
             currentStatusSavedObject,
             logger,
-            message: `Bulk Indexing signals failed. Check logs for further details \nRule name: "${name}"\nid: "${alertId}"\nrule_id: "${ruleId}"\n`,
+            message: `Bulk Indexing signals failed. Check logs for further details Rule name: "${name}" id: "${alertId}" rule_id: "${ruleId}" output_index: "${outputIndex}"`,
             services,
             ruleStatusSavedObjects,
             ruleId: ruleId ?? '(unknown rule id)',

diff --git a/x-pack/legacy/plugins/siem/server/lib/machine_learning/index.ts b/x-pack/legacy/plugins/siem/server/lib/machine_learning/index.ts
@@ -54,8 +54,7 @@ export const getAnomalies = async (
 
 const buildCriteria = (params: AnomaliesSearchParams): object[] => {
   const { earliestMs, jobIds, latestMs, threshold } = params;
-  const jobIdsFilterable =
-    jobIds && jobIds.length > 0 && !(jobIds.length === 1 && jobIds[0] === '*');
+  const jobIdsFilterable = jobIds.length > 0 && !(jobIds.length === 1 && jobIds[0] === '*');
 
   const boolCriteria: object[] = [
     {