[Security Solution][Alerts] Add validation for historyWindowStart (#138182)

* Add validation to ensure that 'historyWindowStart' is earlier than 'from'

* Fix tests

* Fix test again

* Add comment

x-pack/plugins/security_solution/cypress/integration/detection_rules/new_terms_rule.spec.ts

@@ -129,7 +129,7 @@ describe('New Terms rules', () => {
         getDetails(RULE_TYPE_DETAILS).should('have.text', 'New Terms');
         getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
         getDetails(NEW_TERMS_FIELDS_DETAILS).should('have.text', 'host.name');
-        getDetails(NEW_TERMS_HISTORY_WINDOW_DETAILS).should('have.text', '50000h');
+        getDetails(NEW_TERMS_HISTORY_WINDOW_DETAILS).should('have.text', '51000h');
       });
       cy.get(SCHEDULE_DETAILS).within(() => {
         getDetails(RUNS_EVERY_DETAILS).should(

x-pack/plugins/security_solution/cypress/objects/rule.ts

@@ -363,7 +363,12 @@ export const getNewTermsRule = (): NewTermsRule => ({
   mitre: [getMitre1(), getMitre2()],
   note: '# test markdown',
   newTermsFields: ['host.name'],
-  historyWindowSize: getLookBack(),
+  historyWindowSize: {
+    // historyWindowSize needs to be larger than the rule's lookback value
+    interval: '51000',
+    timeType: 'Hours',
+    type: 'h',
+  },
   runsEvery: getRunsEvery(),
   lookBack: getLookBack(),
   timeline: getTimeline(),

x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/new_terms/create_new_terms_alert_type.ts

@@ -6,7 +6,6 @@
  */
 
 import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
-import dateMath from '@elastic/datemath';
 import { validateNonExact } from '@kbn/securitysolution-io-ts-utils';
 import { NEW_TERMS_RULE_TYPE_ID } from '@kbn/securitysolution-rules';
 import { SERVER_APP_ID } from '../../../../../common/constants';
@@ -35,6 +34,7 @@ import {
 } from './build_timestamp_runtime_mapping';
 import type { SignalSource } from '../../signals/types';
 import { validateImmutable, validateIndexPatterns } from '../utils';
+import { parseDateString, validateHistoryWindowStart } from './utils';
 
 interface BulkCreateResults {
   bulkCreateTimes: string[];
@@ -81,6 +81,10 @@ export const createNewTermsAlertType = (
           if (validated == null) {
             throw new Error('Validation of rule params failed');
           }
+          validateHistoryWindowStart({
+            historyWindowStart: validated.historyWindowStart,
+            from: validated.from,
+          });
           return validated;
         },
         /**
@@ -129,6 +133,13 @@ export const createNewTermsAlertType = (
         spaceId,
       } = execOptions;
 
+      // Validate the history window size compared to `from` at runtime as well as in the `validate`
+      // function because rule preview does not use the `validate` function defined on the rule type
+      validateHistoryWindowStart({
+        historyWindowStart: params.historyWindowStart,
+        from: params.from,
+      });
+
       const filter = await getFilter({
         filters: params.filters,
         index: inputIndex,
@@ -140,12 +151,11 @@ export const createNewTermsAlertType = (
         lists: exceptionItems,
       });
 
-      const parsedHistoryWindowSize = dateMath.parse(params.historyWindowStart, {
+      const parsedHistoryWindowSize = parseDateString({
+        date: params.historyWindowStart,
         forceNow: tuple.to.toDate(),
+        name: 'historyWindowStart',
       });
-      if (parsedHistoryWindowSize == null) {
-        throw Error(`Failed to parse 'historyWindowStart'`);
-      }
 
       let afterKey;
       let bulkCreateResults: BulkCreateResults = {

x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/new_terms/utils.test.ts

@@ -0,0 +1,67 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { parseDateString, validateHistoryWindowStart } from './utils';
+
+describe('new terms utils', () => {
+  describe('parseDateString', () => {
+    test('should correctly parse a static date', () => {
+      const date = '2022-08-04T16:31:18.000Z';
+      // forceNow shouldn't matter when we give a static date
+      const forceNow = new Date();
+      const parsedDate = parseDateString({ date, forceNow });
+      expect(parsedDate.toISOString()).toEqual(date);
+    });
+
+    test('should correctly parse a relative date', () => {
+      const date = 'now-5m';
+      const forceNow = new Date('2022-08-04T16:31:18.000Z');
+      const parsedDate = parseDateString({ date, forceNow });
+      expect(parsedDate.toISOString()).toEqual('2022-08-04T16:26:18.000Z');
+    });
+
+    test(`should throw an error without a name if the string can't be parsed as a date`, () => {
+      const date = 'notValid';
+      const forceNow = new Date();
+      expect(() => parseDateString({ date, forceNow })).toThrowError(
+        `Failed to parse 'date string'`
+      );
+    });
+
+    test(`should throw an error with a name if the string can't be parsed as a date`, () => {
+      const date = 'notValid';
+      const forceNow = new Date();
+      expect(() => parseDateString({ date, forceNow, name: 'historyWindowStart' })).toThrowError(
+        `Failed to parse 'historyWindowStart'`
+      );
+    });
+  });
+
+  describe('validateHistoryWindowStart', () => {
+    test('should not throw if historyWindowStart is earlier than from', () => {
+      const historyWindowStart = 'now-7m';
+      const from = 'now-6m';
+      validateHistoryWindowStart({ historyWindowStart, from });
+    });
+
+    test('should throw if historyWindowStart is equal to from', () => {
+      const historyWindowStart = 'now-7m';
+      const from = 'now-7m';
+      expect(() => validateHistoryWindowStart({ historyWindowStart, from })).toThrowError(
+        `History window size too small, 'historyWindowStart' must be earlier than 'from'`
+      );
+    });
+
+    test('should throw if historyWindowStart is later than from', () => {
+      const historyWindowStart = 'now-7m';
+      const from = 'now-8m';
+      expect(() => validateHistoryWindowStart({ historyWindowStart, from })).toThrowError(
+        `History window size too small, 'historyWindowStart' must be earlier than 'from'`
+      );
+    });
+  });
+});

x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/new_terms/utils.ts

@@ -0,0 +1,46 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import dateMath from '@elastic/datemath';
+import moment from 'moment';
+
+export const parseDateString = ({
+  date,
+  forceNow,
+  name,
+}: {
+  date: string;
+  forceNow: Date;
+  name?: string;
+}): moment.Moment => {
+  const parsedDate = dateMath.parse(date, {
+    forceNow,
+  });
+  if (parsedDate == null || !parsedDate.isValid()) {
+    throw Error(`Failed to parse '${name ?? 'date string'}'`);
+  }
+  return parsedDate;
+};
+
+export const validateHistoryWindowStart = ({
+  historyWindowStart,
+  from,
+}: {
+  historyWindowStart: string;
+  from: string;
+}) => {
+  const forceNow = moment().toDate();
+  const parsedHistoryWindowStart = parseDateString({
+    date: historyWindowStart,
+    forceNow,
+    name: 'historyWindowStart',
+  });
+  const parsedFrom = parseDateString({ date: from, forceNow, name: 'from' });
+  if (parsedHistoryWindowStart.isSameOrAfter(parsedFrom)) {
+    throw Error(`History window size too small, 'historyWindowStart' must be earlier than 'from'`);
+  }
+};

x-pack/test/detection_engine_api_integration/security_and_spaces/group1/create_new_terms.ts

@@ -77,6 +77,22 @@ export default ({ getService }: FtrProviderContext) => {
       expect(rule?.execution_summary?.last_execution.status).to.eql('succeeded');
     });
 
+    it('should not be able to create a new terms rule with too small history window', async () => {
+      const rule = {
+        ...getCreateNewTermsRulesSchemaMock('rule-1'),
+        history_window_start: 'now-5m',
+      };
+      const response = await supertest
+        .post(DETECTION_ENGINE_RULES_URL)
+        .set('kbn-xsrf', 'true')
+        .send(rule);
+
+      expect(response.status).to.equal(400);
+      expect(response.body.message).to.equal(
+        "params invalid: History window size too small, 'historyWindowStart' must be earlier than 'from'"
+      );
+    });
+
     const removeRandomValuedProperties = (alert: DetectionAlert | undefined) => {
       if (!alert) {
         return undefined;
@@ -277,8 +293,8 @@ export default ({ getService }: FtrProviderContext) => {
         ...getCreateNewTermsRulesSchemaMock('rule-1', true),
         new_terms_fields: ['host.name'],
         from: '2019-02-19T20:42:00.000Z',
-        // Set the history_window_start equal to 'from' so we should alert on all terms in the time range
-        history_window_start: '2019-02-19T20:42:00.000Z',
+        // Set the history_window_start close to 'from' so we should alert on all terms in the time range
+        history_window_start: '2019-02-19T20:41:59.000Z',
       };
 
       const createdRule = await createRule(supertest, log, rule);
@@ -328,8 +344,8 @@ export default ({ getService }: FtrProviderContext) => {
           index: ['timestamp-fallback-test', 'myfakeindex-3'],
           new_terms_fields: ['host.name'],
           from: '2020-12-16T16:00:00.000Z',
-          // Set the history_window_start equal to 'from' so we should alert on all terms in the time range
-          history_window_start: '2020-12-16T16:00:00.000Z',
+          // Set the history_window_start close to 'from' so we should alert on all terms in the time range
+          history_window_start: '2020-12-16T15:59:00.000Z',
           timestamp_override: 'event.ingested',
         };
 
@@ -352,8 +368,8 @@ export default ({ getService }: FtrProviderContext) => {
         ...getCreateNewTermsRulesSchemaMock('rule-1', true),
         new_terms_fields: ['host.name'],
         from: '2019-02-19T20:42:00.000Z',
-        // Set the history_window_start equal to 'from' so we should alert on all terms in the time range
-        history_window_start: '2019-02-19T20:42:00.000Z',
+        // Set the history_window_start close to 'from' so we should alert on all terms in the time range
+        history_window_start: '2019-02-19T20:41:59.000Z',
       };
       const createdRule = await createRuleWithExceptionEntries(supertest, log, rule, [
         [
@@ -390,8 +406,8 @@ export default ({ getService }: FtrProviderContext) => {
         ...getCreateNewTermsRulesSchemaMock('rule-1', true),
         new_terms_fields: ['process.pid'],
         from: '2018-02-19T20:42:00.000Z',
-        // Set the history_window_start equal to 'from' so we should alert on all terms in the time range
-        history_window_start: '2018-02-19T20:42:00.000Z',
+        // Set the history_window_start close to 'from' so we should alert on all terms in the time range
+        history_window_start: '2018-02-19T20:41:59.000Z',
         max_signals: maxSignals,
       };