From 7f557ae5767b51c1ab5b5620a8a00ce2960de8b3 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 1 Apr 2021 14:47:28 -0400 Subject: [PATCH 01/13] merge multiple timestamp queries into one single search --- .../signals/build_events_query.test.ts | 222 +++++++++++++++--- .../signals/build_events_query.ts | 137 ++++++++--- .../signals/search_after_bulk_create.ts | 110 ++++----- .../signals/single_search_after.ts | 11 +- .../lib/detection_engine/signals/utils.ts | 5 + 5 files changed, 352 insertions(+), 133 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts index 4b74f865c6a53..8ef2a020e4655 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts @@ -39,12 +39,19 @@ describe('create_signals', () => { bool: { filter: [ { - range: { - '@timestamp': { - gte: 'now-5m', - lte: 'today', - format: 'strict_date_optional_time', - }, + bool: { + minimum_should_match: 1, + should: [ + { + range: { + '@timestamp': { + gte: 'now-5m', + lte: 'today', + format: 'strict_date_optional_time', + }, + }, + }, + ], }, }, ], @@ -73,6 +80,114 @@ describe('create_signals', () => { }, }); }); + + test('it builds a now-5m up to today filter with timestsampOverride', () => { + const query = buildEventsSearchQuery({ + index: ['auditbeat-*'], + from: 'now-5m', + to: 'today', + filter: {}, + size: 100, + searchAfterSortId: undefined, + timestampOverride: 'event.ingested', + excludeDocsWithTimestampOverride: false, + }); + expect(query).toEqual({ + allow_no_indices: true, + index: ['auditbeat-*'], + size: 100, + ignore_unavailable: true, + body: { + docvalue_fields: [ + { + field: 'event.ingested', + format: 'strict_date_optional_time', + }, + { + field: '@timestamp', + format: 'strict_date_optional_time', + }, + ], + query: { + bool: { + filter: [ + {}, + { + bool: { + filter: [ + { + bool: { + should: [ + { + range: { + 'event.ingested': { + gte: 'now-5m', + lte: 'today', + format: 'strict_date_optional_time', + }, + }, + }, + { + bool: { + filter: [ + { + range: { + '@timestamp': { + gte: 'now-5m', + lte: 'today', + format: 'strict_date_optional_time', + }, + }, + }, + { + bool: { + must_not: { + exists: { + field: 'event.ingested', + }, + }, + }, + }, + ], + }, + }, + ], + minimum_should_match: 1, + }, + }, + ], + }, + }, + { + match_all: {}, + }, + ], + }, + }, + fields: [ + { + field: '*', + include_unmapped: true, + }, + ], + sort: [ + { + 'event.ingested': { + order: 'asc', + unmapped_type: 'date', + }, + }, + { + '@timestamp': { + order: 'asc', + unmapped_type: 'date', + }, + }, + ], + }, + }); + }); + test('if searchAfterSortId is an empty string it should not be included', () => { const query = buildEventsSearchQuery({ index: ['auditbeat-*'], @@ -104,12 +219,19 @@ describe('create_signals', () => { bool: { filter: [ { - range: { - '@timestamp': { - gte: 'now-5m', - lte: 'today', - format: 'strict_date_optional_time', - }, + bool: { + minimum_should_match: 1, + should: [ + { + range: { + '@timestamp': { + gte: 'now-5m', + lte: 'today', + format: 'strict_date_optional_time', + }, + }, + }, + ], }, }, ], @@ -170,12 +292,19 @@ describe('create_signals', () => { bool: { filter: [ { - range: { - '@timestamp': { - gte: 'now-5m', - lte: 'today', - format: 'strict_date_optional_time', - }, + bool: { + minimum_should_match: 1, + should: [ + { + range: { + '@timestamp': { + gte: 'now-5m', + lte: 'today', + format: 'strict_date_optional_time', + }, + }, + }, + ], }, }, ], @@ -237,12 +366,19 @@ describe('create_signals', () => { bool: { filter: [ { - range: { - '@timestamp': { - gte: 'now-5m', - lte: 'today', - format: 'strict_date_optional_time', - }, + bool: { + minimum_should_match: 1, + should: [ + { + range: { + '@timestamp': { + gte: 'now-5m', + lte: 'today', + format: 'strict_date_optional_time', + }, + }, + }, + ], }, }, ], @@ -303,12 +439,19 @@ describe('create_signals', () => { bool: { filter: [ { - range: { - '@timestamp': { - gte: 'now-5m', - lte: 'today', - format: 'strict_date_optional_time', - }, + bool: { + minimum_should_match: 1, + should: [ + { + range: { + '@timestamp': { + gte: 'now-5m', + lte: 'today', + format: 'strict_date_optional_time', + }, + }, + }, + ], }, }, ], @@ -371,12 +514,19 @@ describe('create_signals', () => { bool: { filter: [ { - range: { - '@timestamp': { - gte: 'now-5m', - lte: 'today', - format: 'strict_date_optional_time', - }, + bool: { + minimum_should_match: 1, + should: [ + { + range: { + '@timestamp': { + gte: 'now-5m', + lte: 'today', + format: 'strict_date_optional_time', + }, + }, + }, + ], }, }, ], diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts index e086c862262c1..93f4796b8105f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts @@ -5,6 +5,8 @@ * 2.0. */ import type { estypes } from '@elastic/elasticsearch'; +import { SortResults } from '@elastic/elasticsearch/api/types'; +import { isEmpty } from 'lodash'; import { SortOrderOrUndefined, TimestampOverrideOrUndefined, @@ -18,7 +20,7 @@ interface BuildEventsSearchQuery { filter?: estypes.QueryContainer; size: number; sortOrder?: SortOrderOrUndefined; - searchAfterSortId: string | number | undefined; + searchAfterSortIds: SortResults | undefined; timestampOverride: TimestampOverrideOrUndefined; excludeDocsWithTimestampOverride: boolean; } @@ -30,7 +32,7 @@ export const buildEventsSearchQuery = ({ to, filter, size, - searchAfterSortId, + searchAfterSortIds, sortOrder, timestampOverride, excludeDocsWithTimestampOverride, @@ -43,36 +45,78 @@ export const buildEventsSearchQuery = ({ format: 'strict_date_optional_time', })); - const sortField = - timestampOverride != null && !excludeDocsWithTimestampOverride - ? timestampOverride - : '@timestamp'; + // const sortField = + // timestampOverride != null && !excludeDocsWithTimestampOverride + // ? timestampOverride + // : '@timestamp'; - const rangeFilter: estypes.QueryContainer[] = [ - { - range: { - [sortField]: { - lte: to, - gte: from, - format: 'strict_date_optional_time', - }, - }, - }, - ]; - if (excludeDocsWithTimestampOverride) { - rangeFilter.push({ - bool: { - must_not: { - exists: { - field: timestampOverride, + const rangeFilter: estypes.QueryContainer[] = + timestampOverride != null + ? [ + { + range: { + [timestampOverride]: { + lte: to, + gte: from, + format: 'strict_date_optional_time', + }, + }, }, - }, - }, - }); - } - // @ts-expect-error undefined in not assignable to QueryContainer - // but tests contain undefined, so I suppose it's desired behaviour - const filterWithTime: estypes.QueryContainer[] = [filter, { bool: { filter: rangeFilter } }]; + { + bool: { + filter: [ + { + range: { + '@timestamp': { + lte: to, + gte: from, + // @ts-expect-error + format: 'strict_date_optional_time', + }, + }, + }, + { + bool: { + must_not: { + exists: { + field: 'event.ingested', + }, + }, + }, + }, + ], + }, + }, + ] + : [ + { + range: { + '@timestamp': { + lte: to, + gte: from, + format: 'strict_date_optional_time', + }, + }, + }, + ]; + // if (excludeDocsWithTimestampOverride) { + // rangeFilter.push({ + // bool: { + // must_not: { + // exists: { + // field: timestampOverride, + // }, + // }, + // }, + // }); + // } + + const filterWithTime: estypes.QueryContainer[] = [ + // but tests contain undefined, so I suppose it's desired behaviour + // @ts-expect-error undefined in not assignable to QueryContainer + filter, + { bool: { filter: [{ bool: { should: [...rangeFilter], minimum_should_match: 1 } }] } }, + ]; const searchQuery = { allow_no_indices: true, @@ -99,22 +143,39 @@ export const buildEventsSearchQuery = ({ ], ...(aggregations ? { aggregations } : {}), sort: [ - { - [sortField]: { - order: sortOrder ?? 'asc', - unmapped_type: 'date', - }, - }, + ...(timestampOverride != null + ? [ + { + [timestampOverride]: { + order: sortOrder ?? 'asc', + unmapped_type: 'date', + }, + }, + { + '@timestamp': { + order: sortOrder ?? 'asc', + unmapped_type: 'date', + }, + }, + ] + : [ + { + '@timestamp': { + order: sortOrder ?? 'asc', + unmapped_type: 'date', + }, + }, + ]), ], }, }; - if (searchAfterSortId) { + if (searchAfterSortIds != null && !isEmpty(searchAfterSortIds)) { return { ...searchQuery, body: { ...searchQuery.body, - search_after: [searchAfterSortId], + search_after: [...searchAfterSortIds], }, }; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts index 0bc0039b54dba..75e4fa1104d02 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts @@ -5,9 +5,8 @@ * 2.0. */ -/* eslint-disable complexity */ - import { identity } from 'lodash'; +import { SortResults } from '@elastic/elasticsearch/api/types'; import { singleSearchAfter } from './single_search_after'; import { singleBulkCreate } from './single_bulk_create'; import { filterEventsAgainstList } from './filters/filter_events_against_list'; @@ -17,6 +16,7 @@ import { createSearchResultReturnType, createSearchAfterReturnTypeFromResponse, createTotalHitsFromSearchResult, + hasSafeSortIds, mergeReturns, mergeSearchResults, } from './utils'; @@ -44,10 +44,10 @@ export const searchAfterAndBulkCreate = async ({ let toReturn = createSearchAfterReturnType(); // sortId tells us where to start our next consecutive search_after query - let sortId: string | undefined; + let sortIds: SortResults | undefined; let hasSortId = true; // default to true so we execute the search on initial run - let backupSortId: string | undefined; - let hasBackupSortId = ruleParams.timestampOverride ? true : false; + // let backupSortId: string | undefined; + // let hasBackupSortId = ruleParams.timestampOverride ? true : false; // signalsCreatedCount keeps track of how many signals we have created, // to ensure we don't exceed maxSignals @@ -69,60 +69,60 @@ export const searchAfterAndBulkCreate = async ({ while (signalsCreatedCount < tuple.maxSignals) { try { let mergedSearchResults = createSearchResultReturnType(); - logger.debug(buildRuleMessage(`sortIds: ${sortId}`)); + logger.debug(buildRuleMessage(`sortIds: ${sortIds}`)); // if there is a timestampOverride param we always want to do a secondary search against @timestamp - if (ruleParams.timestampOverride != null && hasBackupSortId) { - // only execute search if we have something to sort on or if it is the first search - const { - searchResult: searchResultB, - searchDuration: searchDurationB, - searchErrors: searchErrorsB, - } = await singleSearchAfter({ - buildRuleMessage, - searchAfterSortId: backupSortId, - index: inputIndexPattern, - from: tuple.from.toISOString(), - to: tuple.to.toISOString(), - services, - logger, - // @ts-expect-error please, declare a type explicitly instead of unknown - filter, - pageSize: Math.ceil(Math.min(tuple.maxSignals, pageSize)), - timestampOverride: ruleParams.timestampOverride, - excludeDocsWithTimestampOverride: true, - }); + // if (ruleParams.timestampOverride != null && hasBackupSortId) { + // // only execute search if we have something to sort on or if it is the first search + // const { + // searchResult: searchResultB, + // searchDuration: searchDurationB, + // searchErrors: searchErrorsB, + // } = await singleSearchAfter({ + // buildRuleMessage, + // searchAfterSortId: backupSortId, + // index: inputIndexPattern, + // from: tuple.from.toISOString(), + // to: tuple.to.toISOString(), + // services, + // logger, + // // @ts-expect-error please, declare a type explicitly instead of unknown + // filter, + // pageSize: Math.ceil(Math.min(tuple.maxSignals, pageSize)), + // timestampOverride: ruleParams.timestampOverride, + // excludeDocsWithTimestampOverride: true, + // }); - // call this function setSortIdOrExit() - const lastSortId = searchResultB?.hits?.hits[searchResultB.hits.hits.length - 1]?.sort; - if (lastSortId != null && lastSortId.length !== 0) { - // @ts-expect-error @elastic/elasticsearch SortResults contains null not assignable to backupSortId - backupSortId = lastSortId[0]; - hasBackupSortId = true; - } else { - logger.debug(buildRuleMessage('backupSortIds was empty on searchResultB')); - hasBackupSortId = false; - } + // // call this function setSortIdOrExit() + // const lastSortId = searchResultB?.hits?.hits[searchResultB.hits.hits.length - 1]?.sort; + // if (lastSortId != null && lastSortId.length !== 0) { + // // @ts-expect-error @elastic/elasticsearch SortResults contains null not assignable to backupSortId + // backupSortId = lastSortId[0]; + // hasBackupSortId = true; + // } else { + // logger.debug(buildRuleMessage('backupSortIds was empty on searchResultB')); + // hasBackupSortId = false; + // } - mergedSearchResults = mergeSearchResults([mergedSearchResults, searchResultB]); + // mergedSearchResults = mergeSearchResults([mergedSearchResults, searchResultB]); - toReturn = mergeReturns([ - toReturn, - createSearchAfterReturnTypeFromResponse({ - searchResult: mergedSearchResults, - timestampOverride: undefined, - }), - createSearchAfterReturnType({ - searchAfterTimes: [searchDurationB], - errors: searchErrorsB, - }), - ]); - } + // toReturn = mergeReturns([ + // toReturn, + // createSearchAfterReturnTypeFromResponse({ + // searchResult: mergedSearchResults, + // timestampOverride: undefined, + // }), + // createSearchAfterReturnType({ + // searchAfterTimes: [searchDurationB], + // errors: searchErrorsB, + // }), + // ]); + // } if (hasSortId) { const { searchResult, searchDuration, searchErrors } = await singleSearchAfter({ buildRuleMessage, - searchAfterSortId: sortId, + searchAfterSortIds: sortIds, index: inputIndexPattern, from: tuple.from.toISOString(), to: tuple.to.toISOString(), @@ -147,10 +147,9 @@ export const searchAfterAndBulkCreate = async ({ }), ]); - const lastSortId = searchResult.hits.hits[searchResult.hits.hits.length - 1]?.sort; - if (lastSortId != null && lastSortId.length !== 0) { - // @ts-expect-error @elastic/elasticsearch SortResults contains null not assignable to sortId - sortId = lastSortId[0]; + const lastSortIds = searchResult.hits.hits[searchResult.hits.hits.length - 1]?.sort; + if (lastSortIds != null && lastSortIds.length !== 0 && hasSafeSortIds(lastSortIds)) { + sortIds = lastSortIds; hasSortId = true; } else { hasSortId = false; @@ -236,7 +235,8 @@ export const searchAfterAndBulkCreate = async ({ sendAlertTelemetryEvents(logger, eventsTelemetry, filteredEvents, buildRuleMessage); } - if (!hasSortId && !hasBackupSortId) { + // if (!hasSortId && !hasBackupSortId) { + if (!hasSortId) { logger.debug(buildRuleMessage('ran out of sort ids to sort on')); break; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts index 9dcec1861f15d..f28c80680b887 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts @@ -6,6 +6,7 @@ */ import type { estypes } from '@elastic/elasticsearch'; import { performance } from 'perf_hooks'; +import { SearchRequest, SortResults } from '@elastic/elasticsearch/api/types'; import { AlertInstanceContext, AlertInstanceState, @@ -23,7 +24,7 @@ import { interface SingleSearchAfterParams { aggregations?: Record; - searchAfterSortId: string | undefined; + searchAfterSortIds: SortResults | undefined; index: string[]; from: string; to: string; @@ -40,7 +41,7 @@ interface SingleSearchAfterParams { // utilize search_after for paging results into bulk. export const singleSearchAfter = async ({ aggregations, - searchAfterSortId, + searchAfterSortIds, index, from, to, @@ -66,7 +67,7 @@ export const singleSearchAfter = async ({ filter, size: pageSize, sortOrder, - searchAfterSortId, + searchAfterSortIds, timestampOverride, excludeDocsWithTimestampOverride, }); @@ -74,7 +75,9 @@ export const singleSearchAfter = async ({ const start = performance.now(); const { body: nextSearchAfterResult, - } = await services.scopedClusterClient.asCurrentUser.search(searchAfterQuery); + } = await services.scopedClusterClient.asCurrentUser.search( + searchAfterQuery as SearchRequest + ); const end = performance.now(); const searchErrors = createErrorsFromShard({ errors: nextSearchAfterResult._shards.failures ?? [], diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index 54ed44956c8b3..f3cf746f2c2f1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -13,6 +13,7 @@ import type { estypes } from '@elastic/elasticsearch'; import { isEmpty, partition } from 'lodash'; import { ApiResponse, Context } from '@elastic/elasticsearch/lib/Transport'; +import { SortResults } from '@elastic/elasticsearch/api/types'; import { TimestampOverrideOrUndefined, Privilege, @@ -846,3 +847,7 @@ export const isThreatParams = (params: RuleParams): params is ThreatRuleParams = params.type === 'threat_match'; export const isMachineLearningParams = (params: RuleParams): params is MachineLearningRuleParams => params.type === 'machine_learning'; + +export const hasSafeSortIds = (sortIds: SortResults) => { + return sortIds?.every((sortId) => sortId != null && sortId < Number.MAX_SAFE_INTEGER); +}; From bf50a3768382a7d4aec8af4ca94d97db9b387121 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 1 Apr 2021 18:02:59 -0400 Subject: [PATCH 02/13] fix types and unit tests --- .../signals/build_events_query.test.ts | 20 +++++++++---------- .../signals/build_events_query.ts | 8 ++++++-- .../signals/single_search_after.test.ts | 14 ++++++------- .../find_previous_threshold_signals.ts | 2 +- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts index 8ef2a020e4655..5db80eb6aaee3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts @@ -15,7 +15,7 @@ describe('create_signals', () => { to: 'today', filter: {}, size: 100, - searchAfterSortId: undefined, + searchAfterSortIds: undefined, timestampOverride: undefined, excludeDocsWithTimestampOverride: false, }); @@ -88,7 +88,7 @@ describe('create_signals', () => { to: 'today', filter: {}, size: 100, - searchAfterSortId: undefined, + searchAfterSortIds: undefined, timestampOverride: 'event.ingested', excludeDocsWithTimestampOverride: false, }); @@ -188,14 +188,14 @@ describe('create_signals', () => { }); }); - test('if searchAfterSortId is an empty string it should not be included', () => { + test('if searchAfterSortIds is an empty string it should not be included', () => { const query = buildEventsSearchQuery({ index: ['auditbeat-*'], from: 'now-5m', to: 'today', filter: {}, size: 100, - searchAfterSortId: '', + searchAfterSortIds: [''], timestampOverride: undefined, excludeDocsWithTimestampOverride: false, }); @@ -260,7 +260,7 @@ describe('create_signals', () => { }, }); }); - test('if searchAfterSortId is a valid sortId string', () => { + test('if searchAfterSortIds is a valid sortId string', () => { const fakeSortId = '123456789012'; const query = buildEventsSearchQuery({ index: ['auditbeat-*'], @@ -268,7 +268,7 @@ describe('create_signals', () => { to: 'today', filter: {}, size: 100, - searchAfterSortId: fakeSortId, + searchAfterSortIds: [fakeSortId], timestampOverride: undefined, excludeDocsWithTimestampOverride: false, }); @@ -334,7 +334,7 @@ describe('create_signals', () => { }, }); }); - test('if searchAfterSortId is a valid sortId number', () => { + test('if searchAfterSortIds is a valid sortId number', () => { const fakeSortIdNumber = 123456789012; const query = buildEventsSearchQuery({ index: ['auditbeat-*'], @@ -342,7 +342,7 @@ describe('create_signals', () => { to: 'today', filter: {}, size: 100, - searchAfterSortId: fakeSortIdNumber, + searchAfterSortIds: [fakeSortIdNumber], timestampOverride: undefined, excludeDocsWithTimestampOverride: false, }); @@ -415,7 +415,7 @@ describe('create_signals', () => { to: 'today', filter: {}, size: 100, - searchAfterSortId: undefined, + searchAfterSortIds: undefined, timestampOverride: undefined, excludeDocsWithTimestampOverride: false, }); @@ -495,7 +495,7 @@ describe('create_signals', () => { to: 'today', filter: {}, size: 100, - searchAfterSortId: undefined, + searchAfterSortIds: undefined, timestampOverride: undefined, excludeDocsWithTimestampOverride: false, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts index 93f4796b8105f..3bd2d315ce67d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts @@ -170,12 +170,16 @@ export const buildEventsSearchQuery = ({ }, }; - if (searchAfterSortIds != null && !isEmpty(searchAfterSortIds)) { + if ( + searchAfterSortIds != null && + !isEmpty(searchAfterSortIds) && + searchAfterSortIds.filter((sortId) => !isEmpty(sortId?.toString())).length > 0 + ) { return { ...searchQuery, body: { ...searchQuery.body, - search_after: [...searchAfterSortIds], + search_after: searchAfterSortIds.filter((sortId) => !isEmpty(sortId?.toString())), }, }; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts index cbffac6e7b455..8a0328041f6fc 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts @@ -34,7 +34,7 @@ describe('singleSearchAfter', () => { elasticsearchClientMock.createSuccessTransportRequestPromise(sampleDocSearchResultsNoSortId()) ); const { searchResult } = await singleSearchAfter({ - searchAfterSortId: undefined, + searchAfterSortIds: undefined, index: [], from: 'now-360s', to: 'now', @@ -53,7 +53,7 @@ describe('singleSearchAfter', () => { elasticsearchClientMock.createSuccessTransportRequestPromise(sampleDocSearchResultsNoSortId()) ); const { searchErrors } = await singleSearchAfter({ - searchAfterSortId: undefined, + searchAfterSortIds: undefined, index: [], from: 'now-360s', to: 'now', @@ -104,7 +104,7 @@ describe('singleSearchAfter', () => { }) ); const { searchErrors } = await singleSearchAfter({ - searchAfterSortId: undefined, + searchAfterSortIds: undefined, index: [], from: 'now-360s', to: 'now', @@ -121,14 +121,14 @@ describe('singleSearchAfter', () => { ]); }); test('if singleSearchAfter works with a given sort id', async () => { - const searchAfterSortId = '1234567891111'; + const searchAfterSortIds = ['1234567891111']; mockService.scopedClusterClient.asCurrentUser.search.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( sampleDocSearchResultsWithSortId() ) ); const { searchResult } = await singleSearchAfter({ - searchAfterSortId, + searchAfterSortIds, index: [], from: 'now-360s', to: 'now', @@ -143,13 +143,13 @@ describe('singleSearchAfter', () => { expect(searchResult).toEqual(sampleDocSearchResultsWithSortId()); }); test('if singleSearchAfter throws error', async () => { - const searchAfterSortId = '1234567891111'; + const searchAfterSortIds = ['1234567891111']; mockService.scopedClusterClient.asCurrentUser.search.mockResolvedValueOnce( elasticsearchClientMock.createErrorTransportRequestPromise(new Error('Fake Error')) ); await expect( singleSearchAfter({ - searchAfterSortId, + searchAfterSortIds, index: [], from: 'now-360s', to: 'now', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts index 06e718b646ffa..b4e11ca497f50 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts @@ -71,7 +71,7 @@ export const findPreviousThresholdSignals = async ({ }; return singleSearchAfter({ - searchAfterSortId: undefined, + searchAfterSortIds: undefined, timestampOverride, index: indexPattern, from, From bd7252c7feca6f7a86a2168727d9053f125965e4 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 2 Apr 2021 10:53:23 -0400 Subject: [PATCH 03/13] remove unused code for sending secondary search --- .../signals/build_events_query.test.ts | 7 --- .../signals/build_events_query.ts | 18 ------- .../signals/search_after_bulk_create.ts | 48 ------------------- .../signals/single_search_after.ts | 1 - 4 files changed, 74 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts index 5db80eb6aaee3..d5d75682455c2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts @@ -17,7 +17,6 @@ describe('create_signals', () => { size: 100, searchAfterSortIds: undefined, timestampOverride: undefined, - excludeDocsWithTimestampOverride: false, }); expect(query).toEqual({ allow_no_indices: true, @@ -90,7 +89,6 @@ describe('create_signals', () => { size: 100, searchAfterSortIds: undefined, timestampOverride: 'event.ingested', - excludeDocsWithTimestampOverride: false, }); expect(query).toEqual({ allow_no_indices: true, @@ -197,7 +195,6 @@ describe('create_signals', () => { size: 100, searchAfterSortIds: [''], timestampOverride: undefined, - excludeDocsWithTimestampOverride: false, }); expect(query).toEqual({ allow_no_indices: true, @@ -270,7 +267,6 @@ describe('create_signals', () => { size: 100, searchAfterSortIds: [fakeSortId], timestampOverride: undefined, - excludeDocsWithTimestampOverride: false, }); expect(query).toEqual({ allow_no_indices: true, @@ -344,7 +340,6 @@ describe('create_signals', () => { size: 100, searchAfterSortIds: [fakeSortIdNumber], timestampOverride: undefined, - excludeDocsWithTimestampOverride: false, }); expect(query).toEqual({ allow_no_indices: true, @@ -417,7 +412,6 @@ describe('create_signals', () => { size: 100, searchAfterSortIds: undefined, timestampOverride: undefined, - excludeDocsWithTimestampOverride: false, }); expect(query).toEqual({ allow_no_indices: true, @@ -497,7 +491,6 @@ describe('create_signals', () => { size: 100, searchAfterSortIds: undefined, timestampOverride: undefined, - excludeDocsWithTimestampOverride: false, }); expect(query).toEqual({ allow_no_indices: true, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts index 3bd2d315ce67d..c5a116ebc1a0e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts @@ -22,7 +22,6 @@ interface BuildEventsSearchQuery { sortOrder?: SortOrderOrUndefined; searchAfterSortIds: SortResults | undefined; timestampOverride: TimestampOverrideOrUndefined; - excludeDocsWithTimestampOverride: boolean; } export const buildEventsSearchQuery = ({ @@ -35,7 +34,6 @@ export const buildEventsSearchQuery = ({ searchAfterSortIds, sortOrder, timestampOverride, - excludeDocsWithTimestampOverride, }: BuildEventsSearchQuery) => { const defaultTimeFields = ['@timestamp']; const timestamps = @@ -45,11 +43,6 @@ export const buildEventsSearchQuery = ({ format: 'strict_date_optional_time', })); - // const sortField = - // timestampOverride != null && !excludeDocsWithTimestampOverride - // ? timestampOverride - // : '@timestamp'; - const rangeFilter: estypes.QueryContainer[] = timestampOverride != null ? [ @@ -99,17 +92,6 @@ export const buildEventsSearchQuery = ({ }, }, ]; - // if (excludeDocsWithTimestampOverride) { - // rangeFilter.push({ - // bool: { - // must_not: { - // exists: { - // field: timestampOverride, - // }, - // }, - // }, - // }); - // } const filterWithTime: estypes.QueryContainer[] = [ // but tests contain undefined, so I suppose it's desired behaviour diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts index 75e4fa1104d02..a66242944a201 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts @@ -71,54 +71,6 @@ export const searchAfterAndBulkCreate = async ({ let mergedSearchResults = createSearchResultReturnType(); logger.debug(buildRuleMessage(`sortIds: ${sortIds}`)); - // if there is a timestampOverride param we always want to do a secondary search against @timestamp - // if (ruleParams.timestampOverride != null && hasBackupSortId) { - // // only execute search if we have something to sort on or if it is the first search - // const { - // searchResult: searchResultB, - // searchDuration: searchDurationB, - // searchErrors: searchErrorsB, - // } = await singleSearchAfter({ - // buildRuleMessage, - // searchAfterSortId: backupSortId, - // index: inputIndexPattern, - // from: tuple.from.toISOString(), - // to: tuple.to.toISOString(), - // services, - // logger, - // // @ts-expect-error please, declare a type explicitly instead of unknown - // filter, - // pageSize: Math.ceil(Math.min(tuple.maxSignals, pageSize)), - // timestampOverride: ruleParams.timestampOverride, - // excludeDocsWithTimestampOverride: true, - // }); - - // // call this function setSortIdOrExit() - // const lastSortId = searchResultB?.hits?.hits[searchResultB.hits.hits.length - 1]?.sort; - // if (lastSortId != null && lastSortId.length !== 0) { - // // @ts-expect-error @elastic/elasticsearch SortResults contains null not assignable to backupSortId - // backupSortId = lastSortId[0]; - // hasBackupSortId = true; - // } else { - // logger.debug(buildRuleMessage('backupSortIds was empty on searchResultB')); - // hasBackupSortId = false; - // } - - // mergedSearchResults = mergeSearchResults([mergedSearchResults, searchResultB]); - - // toReturn = mergeReturns([ - // toReturn, - // createSearchAfterReturnTypeFromResponse({ - // searchResult: mergedSearchResults, - // timestampOverride: undefined, - // }), - // createSearchAfterReturnType({ - // searchAfterTimes: [searchDurationB], - // errors: searchErrorsB, - // }), - // ]); - // } - if (hasSortId) { const { searchResult, searchDuration, searchErrors } = await singleSearchAfter({ buildRuleMessage, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts index f28c80680b887..00928ff008617 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts @@ -69,7 +69,6 @@ export const singleSearchAfter = async ({ sortOrder, searchAfterSortIds, timestampOverride, - excludeDocsWithTimestampOverride, }); const start = performance.now(); From b52d1dbb74f7fc228076230eebb7bd97327eaf37 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 2 Apr 2021 15:05:03 -0400 Subject: [PATCH 04/13] removes unused excludeDocsWithTimestampOverride --- .../lib/detection_engine/signals/search_after_bulk_create.ts | 1 - .../lib/detection_engine/signals/single_search_after.test.ts | 5 ----- .../lib/detection_engine/signals/single_search_after.ts | 2 -- .../signals/threshold/find_previous_threshold_signals.ts | 1 - .../signals/threshold/find_threshold_signals.ts | 1 - 5 files changed, 10 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts index a66242944a201..0f68bbf5525f3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts @@ -84,7 +84,6 @@ export const searchAfterAndBulkCreate = async ({ filter, pageSize: Math.ceil(Math.min(tuple.maxSignals, pageSize)), timestampOverride: ruleParams.timestampOverride, - excludeDocsWithTimestampOverride: false, }); mergedSearchResults = mergeSearchResults([mergedSearchResults, searchResult]); toReturn = mergeReturns([ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts index 8a0328041f6fc..a40459d312b9f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts @@ -44,7 +44,6 @@ describe('singleSearchAfter', () => { filter: undefined, timestampOverride: undefined, buildRuleMessage, - excludeDocsWithTimestampOverride: false, }); expect(searchResult).toEqual(sampleDocSearchResultsNoSortId()); }); @@ -63,7 +62,6 @@ describe('singleSearchAfter', () => { filter: undefined, timestampOverride: undefined, buildRuleMessage, - excludeDocsWithTimestampOverride: false, }); expect(searchErrors).toEqual([]); }); @@ -114,7 +112,6 @@ describe('singleSearchAfter', () => { filter: undefined, timestampOverride: undefined, buildRuleMessage, - excludeDocsWithTimestampOverride: false, }); expect(searchErrors).toEqual([ 'index: "index-123" reason: "some reason" type: "some type" caused by reason: "some reason" caused by type: "some type"', @@ -138,7 +135,6 @@ describe('singleSearchAfter', () => { filter: undefined, timestampOverride: undefined, buildRuleMessage, - excludeDocsWithTimestampOverride: false, }); expect(searchResult).toEqual(sampleDocSearchResultsWithSortId()); }); @@ -159,7 +155,6 @@ describe('singleSearchAfter', () => { filter: undefined, timestampOverride: undefined, buildRuleMessage, - excludeDocsWithTimestampOverride: false, }) ).rejects.toThrow('Fake Error'); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts index 00928ff008617..57ed05bcb27cf 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.ts @@ -35,7 +35,6 @@ interface SingleSearchAfterParams { filter?: estypes.QueryContainer; timestampOverride: TimestampOverrideOrUndefined; buildRuleMessage: BuildRuleMessage; - excludeDocsWithTimestampOverride: boolean; } // utilize search_after for paging results into bulk. @@ -52,7 +51,6 @@ export const singleSearchAfter = async ({ sortOrder, timestampOverride, buildRuleMessage, - excludeDocsWithTimestampOverride, }: SingleSearchAfterParams): Promise<{ searchResult: SignalSearchResponse; searchDuration: string; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts index b4e11ca497f50..1a2bfbf3a962d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts @@ -81,6 +81,5 @@ export const findPreviousThresholdSignals = async ({ filter, pageSize: 10000, // TODO: multiple pages? buildRuleMessage, - excludeDocsWithTimestampOverride: false, }); }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_threshold_signals.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_threshold_signals.ts index 33ffa5b71a65c..986393d6d3454 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_threshold_signals.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_threshold_signals.ts @@ -141,6 +141,5 @@ export const findThresholdSignals = async ({ pageSize: 1, sortOrder: 'desc', buildRuleMessage, - excludeDocsWithTimestampOverride: false, }); }; From 1ff505091940874906dd2420e21a497351474b04 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 14 Apr 2021 18:05:45 -0400 Subject: [PATCH 05/13] adds integration tests to cover cases that should / should not generate signals when timestamp override is present in rule --- .../tests/generating_signals.ts | 65 ++++++++++++++++++ .../timestamp_override/data.json.gz | Bin 217 -> 255 bytes 2 files changed, 65 insertions(+) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts index 08fb9222e1789..8d3e0f93c81fe 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts @@ -807,5 +807,70 @@ export default ({ getService }: FtrProviderContext) => { }); }); }); + + /** + * Here we test the functionality of timestamp overrides. If the rule specifies a timestamp override, + * then the documents will be queried and sorted using the timestamp overrid field. + * If no timestamp override field exists, the search will default to using `@timestamp` field + */ + describe('Signals generated from events with timestamp override fields', () => { + beforeEach(async () => { + await createSignalsIndex(supertest); + await esArchiver.load('security_solution/timestamp_override'); + }); + + afterEach(async () => { + await deleteSignalsIndex(supertest); + await deleteAllAlerts(supertest); + await esArchiver.unload('security_solution/timestamp_override'); + }); + + const executeRuleAndGetSignals = async (rule: QueryCreateSchema) => { + const { id } = await createRule(supertest, rule); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); + await waitForSignalsToBePresent(supertest, 3, [id, id, id]); + const signalsResponse = await getSignalsByIds(supertest, [id, id, id], 3); + const signals = signalsResponse.hits.hits.map((hit) => hit._source); + const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); + return signalsOrderedByEventId; + }; + + it('should generate signals with event.ingested, @timestamp and (event.ingested + timestamp)', async () => { + const rule: QueryCreateSchema = { + ...getRuleForSignalTesting(['myfa*']), + timestamp_override: 'event.ingested', + }; + + const signals = await executeRuleAndGetSignals(rule); + + expect(signals.length).equal(3); + }); + + it('should generate 2 signals with @timestamp', async () => { + const rule: QueryCreateSchema = getRuleForSignalTesting(['myfa*']); + + const { id } = await createRule(supertest, rule); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); + await waitForSignalsToBePresent(supertest, undefined, [id, id]); + const signalsResponse = await getSignalsByIds(supertest, [id]); + const signals = signalsResponse.hits.hits.map((hit) => hit._source); + + expect(signals.length).equal(2); + }); + + it('should generate 2 signals when timestamp override does not exist', async () => { + const rule: QueryCreateSchema = { + ...getRuleForSignalTesting(['myfa*']), + timestamp_override: 'event.fakeingestfield', + }; + const { id } = await createRule(supertest, rule); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); + await waitForSignalsToBePresent(supertest, undefined, [id]); + + const signalsResponse = await getSignalsByIds(supertest, [id]); + const signals = signalsResponse.hits.hits.map((hit) => hit._source); + expect(signals.length).equal(2); + }); + }); }); }; diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override/data.json.gz b/x-pack/test/functional/es_archives/security_solution/timestamp_override/data.json.gz index be351495c2f2e11b896d0b657c6285d0713cd418..a2c561471289f8510dd7b4c8d29bb345bad14953 100644 GIT binary patch literal 255 zcmVN4uUWcMfZF~)3q2{1q~}d!j&6CWJDrRBlQ*1f48lb zhm-_$$4u{K?wL&GAcXnj1UN)EW;;Aljcb%$RHu@KEYl)}yAtG&eRKp}x zU89zr9Lh3E^e+#PWenZeIg1g&S|E(?J$0+b%9e11<5bZE$T6=1bIJK4D)P35?Lc$g`i2XKmya zPo<1U`DqN_C@R#X?vq*AhUn()oEE-e!nSifW5;7Vg|CmHIfvHjtjja&djca;Jj|2> F007Rrb_oCg literal 217 zcmV;~04Dz*iwFP!000026Qz&M4uUWcgztTdre`s<1T>s{2v1%Nffb2h2^5r&zPoMZ zM^vKGTX*)G?o5j?CZfrK_?SdIBnvInL0W00Rf8Ina|BlnWX&Nsff+4oP_-?2RfHq0 zlnlx;h|QNrNK=k4yhtNVi2-Ei>#y$hStUs%5o)KqKG932xm2vf-{qQ5Ho6o8HJru7 zW{$;B3W&6m+03>mv#7VFu1`Imu9xo0-jTr|yO$ioJeR#QsxZd?R(5`>>^xT9h(uu` T$ntOceQb3Dk3QF;RRRD2u~uS3 From a4de3be002cff28ac4c70c49afbe0e043edce418 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 14 Apr 2021 20:16:10 -0400 Subject: [PATCH 06/13] adds integration test to ensure unmapped sort fields do not break search after functionality of detection rules --- .../signals/build_events_query.ts | 2 +- .../signals/search_after_bulk_create.ts | 8 ++-- .../lib/detection_engine/signals/utils.ts | 20 +++++++++ .../tests/generating_signals.ts | 44 +++++++++++++++++-- 4 files changed, 67 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts index c5a116ebc1a0e..8d4a346dddd6b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts @@ -161,7 +161,7 @@ export const buildEventsSearchQuery = ({ ...searchQuery, body: { ...searchQuery.body, - search_after: searchAfterSortIds.filter((sortId) => !isEmpty(sortId?.toString())), + search_after: searchAfterSortIds, }, }; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts index 0f68bbf5525f3..4a778dd3cee00 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts @@ -16,9 +16,9 @@ import { createSearchResultReturnType, createSearchAfterReturnTypeFromResponse, createTotalHitsFromSearchResult, - hasSafeSortIds, mergeReturns, mergeSearchResults, + getSafeSortIds, } from './utils'; import { SearchAfterAndBulkCreateParams, SearchAfterAndBulkCreateReturnType } from './types'; @@ -98,8 +98,10 @@ export const searchAfterAndBulkCreate = async ({ }), ]); - const lastSortIds = searchResult.hits.hits[searchResult.hits.hits.length - 1]?.sort; - if (lastSortIds != null && lastSortIds.length !== 0 && hasSafeSortIds(lastSortIds)) { + const lastSortIds = getSafeSortIds( + searchResult.hits.hits[searchResult.hits.hits.length - 1]?.sort + ); + if (lastSortIds != null && lastSortIds.length !== 0) { sortIds = lastSortIds; hasSortId = true; } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index f3cf746f2c2f1..8debf53e53115 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -851,3 +851,23 @@ export const isMachineLearningParams = (params: RuleParams): params is MachineLe export const hasSafeSortIds = (sortIds: SortResults) => { return sortIds?.every((sortId) => sortId != null && sortId < Number.MAX_SAFE_INTEGER); }; + +/** + * Prevent javascript from returning Number.MAX_SAFE_INTEGER when Elasticsearch expects + * Java's Long.MAX_VALUE. This happens when sorting fields by date which are + * unmapped in the provided index + * + * Ref: https://github.com/elastic/elasticsearch/issues/28806#issuecomment-369303620 + * + * return stringified Long.MAX_VALUE if we receive Number.MAX_SAFE_INTEGER + * @param sortIds SortResults | undefined + * @returns SortResults + */ +export const getSafeSortIds = (sortIds: SortResults | undefined) => { + return sortIds?.map((sortId) => { + if (sortId != null && sortId >= Number.MAX_SAFE_INTEGER) { + return '9223372036854775807'; + } + return sortId; + }); +}; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts index 8d3e0f93c81fe..b9d2714d358a3 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts @@ -810,10 +810,11 @@ export default ({ getService }: FtrProviderContext) => { /** * Here we test the functionality of timestamp overrides. If the rule specifies a timestamp override, - * then the documents will be queried and sorted using the timestamp overrid field. - * If no timestamp override field exists, the search will default to using `@timestamp` field + * then the documents will be queried and sorted using the timestamp override field. + * If no timestamp override field exists in the indices but one was provided to the rule, + * the rule's query will additionally search for events using the `@timestamp` field */ - describe('Signals generated from events with timestamp override fields', () => { + describe('Signals generated from events with timestamp override field', () => { beforeEach(async () => { await createSignalsIndex(supertest); await esArchiver.load('security_solution/timestamp_override'); @@ -835,6 +836,43 @@ export default ({ getService }: FtrProviderContext) => { return signalsOrderedByEventId; }; + describe('Signals generated from events with timestamp override field and ensures search_after continues to work when documents are missing timestamp override field', () => { + beforeEach(async () => { + await esArchiver.load('auditbeat/hosts'); + }); + + afterEach(async () => { + await esArchiver.unload('auditbeat/hosts'); + }); + + /** + * This represents our worst case scenario where this field is not mapped on any index + * We want to check that our logic continues to function within the constraints of search after + * Elasticsearch returns java's long.MAX_VALUE for unmapped date fields + * Javascript does not support numbers this large, but without passing in a number of this size + * The search_after will continue to return the same results and not iterate to the next set + * So to circumvent this limitation of javascript we return the stringified version of Java's + * Long.MAX_VALUE so that search_after does not enter into an infinite loop. + * + * ref: https://github.com/elastic/elasticsearch/issues/28806#issuecomment-369303620 + */ + it('should generate 200 signals when timestamp override does not exist', async () => { + const rule: QueryCreateSchema = { + ...getRuleForSignalTesting(['auditbeat-*']), + timestamp_override: 'event.fakeingested', + max_signals: 200, + }; + + const { id } = await createRule(supertest, rule); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); + await waitForSignalsToBePresent(supertest, 200, [id]); + const signalsResponse = await getSignalsByIds(supertest, [id], 200); + const signals = signalsResponse.hits.hits.map((hit) => hit._source); + + expect(signals.length).equal(200); + }); + }); + it('should generate signals with event.ingested, @timestamp and (event.ingested + timestamp)', async () => { const rule: QueryCreateSchema = { ...getRuleForSignalTesting(['myfa*']), From 35679091a5517a34135b38832cc5529e3fef2d2d Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 14 Apr 2021 21:28:31 -0400 Subject: [PATCH 07/13] Need to figure out why moving the tests around fixed them... --- .../tests/generating_signals.ts | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts index b9d2714d358a3..6d05609275730 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts @@ -826,22 +826,15 @@ export default ({ getService }: FtrProviderContext) => { await esArchiver.unload('security_solution/timestamp_override'); }); - const executeRuleAndGetSignals = async (rule: QueryCreateSchema) => { - const { id } = await createRule(supertest, rule); - await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); - await waitForSignalsToBePresent(supertest, 3, [id, id, id]); - const signalsResponse = await getSignalsByIds(supertest, [id, id, id], 3); - const signals = signalsResponse.hits.hits.map((hit) => hit._source); - const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); - return signalsOrderedByEventId; - }; - describe('Signals generated from events with timestamp override field and ensures search_after continues to work when documents are missing timestamp override field', () => { beforeEach(async () => { + await createSignalsIndex(supertest); await esArchiver.load('auditbeat/hosts'); }); afterEach(async () => { + await deleteSignalsIndex(supertest); + await deleteAllAlerts(supertest); await esArchiver.unload('auditbeat/hosts'); }); @@ -870,30 +863,22 @@ export default ({ getService }: FtrProviderContext) => { const signals = signalsResponse.hits.hits.map((hit) => hit._source); expect(signals.length).equal(200); + await deleteSignalsIndex(supertest); }); }); - it('should generate signals with event.ingested, @timestamp and (event.ingested + timestamp)', async () => { - const rule: QueryCreateSchema = { - ...getRuleForSignalTesting(['myfa*']), - timestamp_override: 'event.ingested', - }; - - const signals = await executeRuleAndGetSignals(rule); - - expect(signals.length).equal(3); - }); - it('should generate 2 signals with @timestamp', async () => { const rule: QueryCreateSchema = getRuleForSignalTesting(['myfa*']); const { id } = await createRule(supertest, rule); await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); - await waitForSignalsToBePresent(supertest, undefined, [id, id]); + await waitForSignalsToBePresent(supertest, 2, [id, id]); const signalsResponse = await getSignalsByIds(supertest, [id]); const signals = signalsResponse.hits.hits.map((hit) => hit._source); + const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); - expect(signals.length).equal(2); + expect(signalsOrderedByEventId.length).equal(2); + await deleteSignalsIndex(supertest); }); it('should generate 2 signals when timestamp override does not exist', async () => { @@ -903,11 +888,30 @@ export default ({ getService }: FtrProviderContext) => { }; const { id } = await createRule(supertest, rule); await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); - await waitForSignalsToBePresent(supertest, undefined, [id]); + await waitForSignalsToBePresent(supertest, 2, [id, id]); + const signalsResponse = await getSignalsByIds(supertest, [id, id]); + const signals = signalsResponse.hits.hits.map((hit) => hit._source); + const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); - const signalsResponse = await getSignalsByIds(supertest, [id]); + expect(signalsOrderedByEventId.length).equal(2); + await deleteSignalsIndex(supertest); + }); + + it('should generate signals with event.ingested, @timestamp and (event.ingested + timestamp)', async () => { + const rule: QueryCreateSchema = { + ...getRuleForSignalTesting(['myfa*']), + timestamp_override: 'event.ingested', + }; + + const { id } = await createRule(supertest, rule); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); + await waitForSignalsToBePresent(supertest, 3, [id, id, id]); + const signalsResponse = await getSignalsByIds(supertest, [id, id, id], 3); const signals = signalsResponse.hits.hits.map((hit) => hit._source); - expect(signals.length).equal(2); + const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); + + expect(signalsOrderedByEventId.length).equal(3); + await deleteSignalsIndex(supertest); }); }); }); From cc5269bd501b6cd022d41620924b96f6522cc285 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 15 Apr 2021 11:00:27 -0400 Subject: [PATCH 08/13] updates tests with new es archive data and fixes bug where exclusion filter was hardcoded to event.ingested :yikes: --- .../signals/build_events_query.ts | 2 +- .../tests/generating_signals.ts | 128 ++++++++++-------- .../timestamp_override/mappings.json | 30 ++-- .../timestamp_override_1/data.json | 10 ++ .../timestamp_override_1/mappings.json | 19 +++ .../timestamp_override_2/data.json | 13 ++ .../timestamp_override_2/mappings.json | 26 ++++ .../timestamp_override_3/data.json | 11 ++ .../timestamp_override_3/mappings.json | 22 +++ .../timestamp_override_4/data.json | 14 ++ .../timestamp_override_4/mappings.json | 29 ++++ 11 files changed, 228 insertions(+), 76 deletions(-) create mode 100644 x-pack/test/functional/es_archives/security_solution/timestamp_override_1/data.json create mode 100644 x-pack/test/functional/es_archives/security_solution/timestamp_override_1/mappings.json create mode 100644 x-pack/test/functional/es_archives/security_solution/timestamp_override_2/data.json create mode 100644 x-pack/test/functional/es_archives/security_solution/timestamp_override_2/mappings.json create mode 100644 x-pack/test/functional/es_archives/security_solution/timestamp_override_3/data.json create mode 100644 x-pack/test/functional/es_archives/security_solution/timestamp_override_3/mappings.json create mode 100644 x-pack/test/functional/es_archives/security_solution/timestamp_override_4/data.json create mode 100644 x-pack/test/functional/es_archives/security_solution/timestamp_override_4/mappings.json diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts index 8d4a346dddd6b..337429b6d9796 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts @@ -72,7 +72,7 @@ export const buildEventsSearchQuery = ({ bool: { must_not: { exists: { - field: 'event.ingested', + field: timestampOverride, }, }, }, diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts index 6d05609275730..eca9f12f58e9f 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts @@ -808,77 +808,102 @@ export default ({ getService }: FtrProviderContext) => { }); }); + describe('Signals generated from events with timestamp override field and ensures search_after continues to work when documents are missing timestamp override field', () => { + beforeEach(async () => { + await createSignalsIndex(supertest); + await esArchiver.load('auditbeat/hosts'); + }); + + afterEach(async () => { + await deleteSignalsIndex(supertest); + await deleteAllAlerts(supertest); + await esArchiver.unload('auditbeat/hosts'); + }); + + /** + * This represents our worst case scenario where this field is not mapped on any index + * We want to check that our logic continues to function within the constraints of search after + * Elasticsearch returns java's long.MAX_VALUE for unmapped date fields + * Javascript does not support numbers this large, but without passing in a number of this size + * The search_after will continue to return the same results and not iterate to the next set + * So to circumvent this limitation of javascript we return the stringified version of Java's + * Long.MAX_VALUE so that search_after does not enter into an infinite loop. + * + * ref: https://github.com/elastic/elasticsearch/issues/28806#issuecomment-369303620 + */ + it('should generate 200 signals when timestamp override does not exist', async () => { + const rule: QueryCreateSchema = { + ...getRuleForSignalTesting(['auditbeat-*']), + timestamp_override: 'event.fakeingested', + max_signals: 200, + }; + + const { id } = await createRule(supertest, rule); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); + await waitForSignalsToBePresent(supertest, 200, [id]); + const signalsResponse = await getSignalsByIds(supertest, [id], 200); + const signals = signalsResponse.hits.hits.map((hit) => hit._source); + + expect(signals.length).equal(200); + await deleteSignalsIndex(supertest); + }); + }); + /** * Here we test the functionality of timestamp overrides. If the rule specifies a timestamp override, * then the documents will be queried and sorted using the timestamp override field. * If no timestamp override field exists in the indices but one was provided to the rule, * the rule's query will additionally search for events using the `@timestamp` field */ - describe('Signals generated from events with timestamp override field', () => { + describe('Signals generated from events with timestamp override field', async () => { beforeEach(async () => { + // await esArchiver.unload('security_solution/timestamp_override'); + await deleteSignalsIndex(supertest); await createSignalsIndex(supertest); - await esArchiver.load('security_solution/timestamp_override'); + await esArchiver.load('security_solution/timestamp_override_1'); + await esArchiver.load('security_solution/timestamp_override_2'); + await esArchiver.load('security_solution/timestamp_override_3'); + await esArchiver.load('security_solution/timestamp_override_4'); }); afterEach(async () => { await deleteSignalsIndex(supertest); await deleteAllAlerts(supertest); - await esArchiver.unload('security_solution/timestamp_override'); + await esArchiver.unload('security_solution/timestamp_override_1'); + await esArchiver.unload('security_solution/timestamp_override_2'); + await esArchiver.unload('security_solution/timestamp_override_3'); + await esArchiver.unload('security_solution/timestamp_override_4'); }); - describe('Signals generated from events with timestamp override field and ensures search_after continues to work when documents are missing timestamp override field', () => { - beforeEach(async () => { - await createSignalsIndex(supertest); - await esArchiver.load('auditbeat/hosts'); - }); - - afterEach(async () => { - await deleteSignalsIndex(supertest); - await deleteAllAlerts(supertest); - await esArchiver.unload('auditbeat/hosts'); - }); + it('should generate signals with event.ingested, @timestamp and (event.ingested + timestamp)', async () => { + const rule: QueryCreateSchema = { + ...getRuleForSignalTesting(['myfa*']), + timestamp_override: 'event.ingested', + }; - /** - * This represents our worst case scenario where this field is not mapped on any index - * We want to check that our logic continues to function within the constraints of search after - * Elasticsearch returns java's long.MAX_VALUE for unmapped date fields - * Javascript does not support numbers this large, but without passing in a number of this size - * The search_after will continue to return the same results and not iterate to the next set - * So to circumvent this limitation of javascript we return the stringified version of Java's - * Long.MAX_VALUE so that search_after does not enter into an infinite loop. - * - * ref: https://github.com/elastic/elasticsearch/issues/28806#issuecomment-369303620 - */ - it('should generate 200 signals when timestamp override does not exist', async () => { - const rule: QueryCreateSchema = { - ...getRuleForSignalTesting(['auditbeat-*']), - timestamp_override: 'event.fakeingested', - max_signals: 200, - }; + const { id } = await createRule(supertest, rule); - const { id } = await createRule(supertest, rule); - await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); - await waitForSignalsToBePresent(supertest, 200, [id]); - const signalsResponse = await getSignalsByIds(supertest, [id], 200); - const signals = signalsResponse.hits.hits.map((hit) => hit._source); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); + await waitForSignalsToBePresent(supertest, 3, [id]); + const signalsResponse = await getSignalsByIds(supertest, [id], 3); + const signals = signalsResponse.hits.hits.map((hit) => hit._source); + const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); - expect(signals.length).equal(200); - await deleteSignalsIndex(supertest); - }); + expect(signalsOrderedByEventId.length).equal(3); }); it('should generate 2 signals with @timestamp', async () => { const rule: QueryCreateSchema = getRuleForSignalTesting(['myfa*']); const { id } = await createRule(supertest, rule); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); - await waitForSignalsToBePresent(supertest, 2, [id, id]); + await waitForSignalsToBePresent(supertest, 2, [id]); const signalsResponse = await getSignalsByIds(supertest, [id]); const signals = signalsResponse.hits.hits.map((hit) => hit._source); const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); expect(signalsOrderedByEventId.length).equal(2); - await deleteSignalsIndex(supertest); }); it('should generate 2 signals when timestamp override does not exist', async () => { @@ -887,31 +912,14 @@ export default ({ getService }: FtrProviderContext) => { timestamp_override: 'event.fakeingestfield', }; const { id } = await createRule(supertest, rule); + await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); - await waitForSignalsToBePresent(supertest, 2, [id, id]); + await waitForSignalsToBePresent(supertest, 2, [id]); const signalsResponse = await getSignalsByIds(supertest, [id, id]); const signals = signalsResponse.hits.hits.map((hit) => hit._source); const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); expect(signalsOrderedByEventId.length).equal(2); - await deleteSignalsIndex(supertest); - }); - - it('should generate signals with event.ingested, @timestamp and (event.ingested + timestamp)', async () => { - const rule: QueryCreateSchema = { - ...getRuleForSignalTesting(['myfa*']), - timestamp_override: 'event.ingested', - }; - - const { id } = await createRule(supertest, rule); - await waitForRuleSuccessOrStatus(supertest, id, 'partial failure'); - await waitForSignalsToBePresent(supertest, 3, [id, id, id]); - const signalsResponse = await getSignalsByIds(supertest, [id, id, id], 3); - const signals = signalsResponse.hits.hits.map((hit) => hit._source); - const signalsOrderedByEventId = orderBy(signals, 'signal.parent.id', 'asc'); - - expect(signalsOrderedByEventId.length).equal(3); - await deleteSignalsIndex(supertest); }); }); }); diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override/mappings.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override/mappings.json index 28de7eeb2eb01..085ab34a3d58a 100644 --- a/x-pack/test/functional/es_archives/security_solution/timestamp_override/mappings.json +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override/mappings.json @@ -1,19 +1,19 @@ { - "type": "index", - "value": { - "index": "myfakeindex-1", - "mappings" : { - "properties" : { - "message" : { - "type" : "text", - "fields" : { - "keyword" : { - "type" : "keyword", - "ignore_above" : 256 - } - } - } + "type": "index", + "value": { + "index": "myfakeindex-1", + "mappings": { + "properties": { + "message": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 } } + } + } } -} \ No newline at end of file + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override_1/data.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override_1/data.json new file mode 100644 index 0000000000000..a07bf9fdd653b --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override_1/data.json @@ -0,0 +1,10 @@ +{ + "type": "doc", + "value": { + "index": "myfakeindex-1", + "source": { + "message": "hello world 1" + }, + "type": "_doc" + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override_1/mappings.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override_1/mappings.json new file mode 100644 index 0000000000000..085ab34a3d58a --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override_1/mappings.json @@ -0,0 +1,19 @@ +{ + "type": "index", + "value": { + "index": "myfakeindex-1", + "mappings": { + "properties": { + "message": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + } + } + } + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override_2/data.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override_2/data.json new file mode 100644 index 0000000000000..24ba2aa42fb82 --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override_2/data.json @@ -0,0 +1,13 @@ +{ + "type": "doc", + "value": { + "index": "myfakeindex-2", + "source": { + "message": "hello world 2", + "event": { + "ingested": "2020-12-16T15:16:18.570Z" + } + }, + "type": "_doc" + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override_2/mappings.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override_2/mappings.json new file mode 100644 index 0000000000000..49a27a423cdaa --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override_2/mappings.json @@ -0,0 +1,26 @@ +{ + "type": "index", + "value": { + "index": "myfakeindex-2", + "mappings": { + "properties": { + "message": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "event": { + "properties": { + "ingested": { + "type": "date" + } + } + } + } + } + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override_3/data.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override_3/data.json new file mode 100644 index 0000000000000..56b0c8dff6eba --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override_3/data.json @@ -0,0 +1,11 @@ +{ + "type": "doc", + "value": { + "index": "myfakeindex-3", + "source": { + "message": "hello world 3", + "@timestamp": "2020-12-16T15:16:18.570Z" + }, + "type": "_doc" + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override_3/mappings.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override_3/mappings.json new file mode 100644 index 0000000000000..736584386a705 --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override_3/mappings.json @@ -0,0 +1,22 @@ +{ + "type": "index", + "value": { + "index": "myfakeindex-3", + "mappings": { + "properties": { + "message": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "@timestamp": { + "type": "date" + } + } + } + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override_4/data.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override_4/data.json new file mode 100644 index 0000000000000..ca7025b36154c --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override_4/data.json @@ -0,0 +1,14 @@ +{ + "type": "doc", + "value": { + "index": "myfakeindex-4", + "source": { + "message": "hello world 4", + "@timestamp": "2020-12-16T15:16:18.570Z", + "event": { + "ingested": "2020-12-16T15:16:18.570Z" + } + }, + "type": "_doc" + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/timestamp_override_4/mappings.json b/x-pack/test/functional/es_archives/security_solution/timestamp_override_4/mappings.json new file mode 100644 index 0000000000000..ab4edc9f300e1 --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/timestamp_override_4/mappings.json @@ -0,0 +1,29 @@ +{ + "type": "index", + "value": { + "index": "myfakeindex-4", + "mappings": { + "properties": { + "@timestamp": { + "type": "date" + }, + "message": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "event": { + "properties": { + "ingested": { + "type": "date" + } + } + } + } + } + } +} From 8aad3e1ce5bce3950ab21c6ec28da9a9399073c0 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 15 Apr 2021 11:11:23 -0400 Subject: [PATCH 09/13] remove dead commented out code --- .../lib/detection_engine/signals/search_after_bulk_create.ts | 3 --- .../security_and_spaces/tests/generating_signals.ts | 1 - 2 files changed, 4 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts index 4a778dd3cee00..08f8abe384d0f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts @@ -46,8 +46,6 @@ export const searchAfterAndBulkCreate = async ({ // sortId tells us where to start our next consecutive search_after query let sortIds: SortResults | undefined; let hasSortId = true; // default to true so we execute the search on initial run - // let backupSortId: string | undefined; - // let hasBackupSortId = ruleParams.timestampOverride ? true : false; // signalsCreatedCount keeps track of how many signals we have created, // to ensure we don't exceed maxSignals @@ -188,7 +186,6 @@ export const searchAfterAndBulkCreate = async ({ sendAlertTelemetryEvents(logger, eventsTelemetry, filteredEvents, buildRuleMessage); } - // if (!hasSortId && !hasBackupSortId) { if (!hasSortId) { logger.debug(buildRuleMessage('ran out of sort ids to sort on')); break; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts index eca9f12f58e9f..1c7381f17fb66 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts @@ -857,7 +857,6 @@ export default ({ getService }: FtrProviderContext) => { */ describe('Signals generated from events with timestamp override field', async () => { beforeEach(async () => { - // await esArchiver.unload('security_solution/timestamp_override'); await deleteSignalsIndex(supertest); await createSignalsIndex(supertest); await esArchiver.load('security_solution/timestamp_override_1'); From d29c16f277ddcd2de34316709e669e30498feb53 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 19 Apr 2021 22:10:06 -0400 Subject: [PATCH 10/13] fixes typo in test file, removes redundant delete signals call in integration test, fixes logic for possibility of receving a null value in sort ids, removes unused utility function for checking valid sort ids --- .../detection_engine/signals/build_events_query.test.ts | 2 +- .../lib/detection_engine/signals/build_events_query.ts | 6 +----- .../server/lib/detection_engine/signals/utils.ts | 8 +++----- .../security_and_spaces/tests/generating_signals.ts | 1 - 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts index d5d75682455c2..88dc7e9849102 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts @@ -80,7 +80,7 @@ describe('create_signals', () => { }); }); - test('it builds a now-5m up to today filter with timestsampOverride', () => { + test('it builds a now-5m up to today filter with timestampOverride', () => { const query = buildEventsSearchQuery({ index: ['auditbeat-*'], from: 'now-5m', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts index 337429b6d9796..86fb51e4785ad 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts @@ -152,11 +152,7 @@ export const buildEventsSearchQuery = ({ }, }; - if ( - searchAfterSortIds != null && - !isEmpty(searchAfterSortIds) && - searchAfterSortIds.filter((sortId) => !isEmpty(sortId?.toString())).length > 0 - ) { + if (searchAfterSortIds != null && !isEmpty(searchAfterSortIds)) { return { ...searchQuery, body: { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index 8debf53e53115..33eaa5431c7d8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -848,10 +848,6 @@ export const isThreatParams = (params: RuleParams): params is ThreatRuleParams = export const isMachineLearningParams = (params: RuleParams): params is MachineLearningRuleParams => params.type === 'machine_learning'; -export const hasSafeSortIds = (sortIds: SortResults) => { - return sortIds?.every((sortId) => sortId != null && sortId < Number.MAX_SAFE_INTEGER); -}; - /** * Prevent javascript from returning Number.MAX_SAFE_INTEGER when Elasticsearch expects * Java's Long.MAX_VALUE. This happens when sorting fields by date which are @@ -865,7 +861,9 @@ export const hasSafeSortIds = (sortIds: SortResults) => { */ export const getSafeSortIds = (sortIds: SortResults | undefined) => { return sortIds?.map((sortId) => { - if (sortId != null && sortId >= Number.MAX_SAFE_INTEGER) { + // haven't determined when we would receive a null value for a sort id + // but in case we do, default to sending the stringified Java max_int + if (sortId == null || sortId >= Number.MAX_SAFE_INTEGER) { return '9223372036854775807'; } return sortId; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts index 1c7381f17fb66..b5cdb6d1b42b4 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/generating_signals.ts @@ -845,7 +845,6 @@ export default ({ getService }: FtrProviderContext) => { const signals = signalsResponse.hits.hits.map((hit) => hit._source); expect(signals.length).equal(200); - await deleteSignalsIndex(supertest); }); }); From b3d932a662cd02029751543f79085fec77b09f0f Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 20 Apr 2021 09:37:35 -0400 Subject: [PATCH 11/13] a unit test for checking if an empty string of a sort id is present was failing because we moved the logic for checking that out of the build search query function and up into the big loop. So I moved that unit test into the search after bulk create test file. --- .../signals/__mocks__/es_results.ts | 7 +- .../signals/build_events_query.test.ts | 71 ----------------- .../signals/search_after_bulk_create.test.ts | 78 +++++++++++++++++++ .../lib/detection_engine/signals/utils.ts | 2 +- 4 files changed, 84 insertions(+), 74 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts index 2ef72c22bbecf..1590a4f0fbb04 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts @@ -115,6 +115,7 @@ export const sampleDocNoSortIdNoVersion = (someUuid: string = sampleIdGuid): Sig export const sampleDocWithSortId = ( someUuid: string = sampleIdGuid, + sortIds: string[] = ['1234567891111', '2233447556677'], ip?: string | string[], destIp?: string | string[] ): SignalSourceHit => ({ @@ -139,7 +140,7 @@ export const sampleDocWithSortId = ( 'source.ip': ip ? (Array.isArray(ip) ? ip : [ip]) : ['127.0.0.1'], 'destination.ip': destIp ? (Array.isArray(destIp) ? destIp : [destIp]) : ['127.0.0.1'], }, - sort: ['1234567891111'], + sort: sortIds, }); export const sampleDocNoSortId = ( @@ -630,7 +631,8 @@ export const repeatedSearchResultsWithSortId = ( pageSize: number, guids: string[], ips?: Array, - destIps?: Array + destIps?: Array, + sortIds?: string[] ): SignalSearchResponse => ({ took: 10, timed_out: false, @@ -646,6 +648,7 @@ export const repeatedSearchResultsWithSortId = ( hits: Array.from({ length: pageSize }).map((x, index) => ({ ...sampleDocWithSortId( guids[index], + sortIds, ips ? ips[index] : '127.0.0.1', destIps ? destIps[index] : '127.0.0.1' ), diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts index 88dc7e9849102..3f4a17dc091ab 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts @@ -186,77 +186,6 @@ describe('create_signals', () => { }); }); - test('if searchAfterSortIds is an empty string it should not be included', () => { - const query = buildEventsSearchQuery({ - index: ['auditbeat-*'], - from: 'now-5m', - to: 'today', - filter: {}, - size: 100, - searchAfterSortIds: [''], - timestampOverride: undefined, - }); - expect(query).toEqual({ - allow_no_indices: true, - index: ['auditbeat-*'], - size: 100, - ignore_unavailable: true, - body: { - docvalue_fields: [ - { - field: '@timestamp', - format: 'strict_date_optional_time', - }, - ], - query: { - bool: { - filter: [ - {}, - { - bool: { - filter: [ - { - bool: { - minimum_should_match: 1, - should: [ - { - range: { - '@timestamp': { - gte: 'now-5m', - lte: 'today', - format: 'strict_date_optional_time', - }, - }, - }, - ], - }, - }, - ], - }, - }, - { - match_all: {}, - }, - ], - }, - }, - fields: [ - { - field: '*', - include_unmapped: true, - }, - ], - sort: [ - { - '@timestamp': { - order: 'asc', - unmapped_type: 'date', - }, - }, - ], - }, - }); - }); test('if searchAfterSortIds is a valid sortId string', () => { const fakeSortId = '123456789012'; const query = buildEventsSearchQuery({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts index 9d9eefe844532..0c7723b6f4cc2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts @@ -426,6 +426,84 @@ describe('searchAfterAndBulkCreate', () => { expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000')); }); + test('should return success when empty string sortId present', async () => { + mockService.scopedClusterClient.asCurrentUser.bulk.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise({ + took: 100, + errors: false, + items: [ + { + create: { + _id: someGuids[0], + _index: 'myfakeindex', + status: 201, + }, + }, + { + create: { + _id: someGuids[1], + _index: 'myfakeindex', + status: 201, + }, + }, + { + create: { + _id: someGuids[2], + _index: 'myfakeindex', + status: 201, + }, + }, + { + create: { + _id: someGuids[3], + _index: 'myfakeindex', + status: 201, + }, + }, + ], + }) + ); + mockService.scopedClusterClient.asCurrentUser.search + .mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise( + repeatedSearchResultsWithSortId( + 4, + 4, + someGuids.slice(0, 3), + ['1.1.1.1', '2.2.2.2', '2.2.2.2', '2.2.2.2'], + // this is the case we are testing, if we receive an empty string for one of the sort ids. + ['', '2222222222222'] + ) + ) + ) + .mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise( + sampleDocSearchResultsNoSortIdNoHits() + ) + ); + + const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ + ruleSO, + tuples, + listClient, + exceptionsList: [], + services: mockService, + logger: mockLogger, + eventsTelemetry: undefined, + id: sampleRuleGuid, + inputIndexPattern, + signalsIndex: DEFAULT_SIGNALS_INDEX, + pageSize: 1, + filter: undefined, + refresh: false, + buildRuleMessage, + }); + expect(success).toEqual(true); + expect(mockService.scopedClusterClient.asCurrentUser.search).toHaveBeenCalledTimes(2); + expect(createdSignalsCount).toEqual(4); + expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000')); + }); + test('should return success when all search results are in the allowlist and no sortId present', async () => { const searchListItems: SearchListItemArraySchema = [ { ...getSearchListItemResponseMock(), value: ['1.1.1.1'] }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index 33eaa5431c7d8..50f42a4bd432c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -863,7 +863,7 @@ export const getSafeSortIds = (sortIds: SortResults | undefined) => { return sortIds?.map((sortId) => { // haven't determined when we would receive a null value for a sort id // but in case we do, default to sending the stringified Java max_int - if (sortId == null || sortId >= Number.MAX_SAFE_INTEGER) { + if (sortId == null || isEmpty(sortId) || sortId >= Number.MAX_SAFE_INTEGER) { return '9223372036854775807'; } return sortId; From 40b9b2758b8e327154729a57a787a0fba9702257 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 20 Apr 2021 10:33:53 -0400 Subject: [PATCH 12/13] fix types --- .../create_field_and_set_tuples.test.ts | 32 +++++++++++++------ .../create_set_to_filter_against.test.ts | 16 +++++++--- .../signals/filters/filter_events.test.ts | 15 +++++---- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/create_field_and_set_tuples.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/create_field_and_set_tuples.test.ts index aac0f47c28295..3fa5d1178b3ec 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/create_field_and_set_tuples.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/create_field_and_set_tuples.test.ts @@ -17,7 +17,7 @@ import { buildRuleMessageMock as buildRuleMessage } from '../rule_messages.mock' describe('filterEventsAgainstList', () => { let listClient = listMock.getListClient(); let exceptionItem = getExceptionListItemSchemaMock(); - let events = [sampleDocWithSortId('123', '1.1.1.1')]; + let events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; beforeEach(() => { jest.clearAllMocks(); @@ -44,7 +44,7 @@ describe('filterEventsAgainstList', () => { }, ], }; - events = [sampleDocWithSortId('123', '1.1.1.1')]; + events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; }); afterEach(() => { @@ -111,7 +111,7 @@ describe('filterEventsAgainstList', () => { }); test('it returns a single matched set as a JSON.stringify() set from the "events"', async () => { - events = [sampleDocWithSortId('123', '1.1.1.1')]; + events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; (exceptionItem.entries[0] as EntryList).field = 'source.ip'; const [{ matchedSet }] = await createFieldAndSetTuples({ listClient, @@ -124,7 +124,10 @@ describe('filterEventsAgainstList', () => { }); test('it returns two matched sets as a JSON.stringify() set from the "events"', async () => { - events = [sampleDocWithSortId('123', '1.1.1.1'), sampleDocWithSortId('456', '2.2.2.2')]; + events = [ + sampleDocWithSortId('123', undefined, '1.1.1.1'), + sampleDocWithSortId('456', undefined, '2.2.2.2'), + ]; (exceptionItem.entries[0] as EntryList).field = 'source.ip'; const [{ matchedSet }] = await createFieldAndSetTuples({ listClient, @@ -137,7 +140,7 @@ describe('filterEventsAgainstList', () => { }); test('it returns an array as a set as a JSON.stringify() array from the "events"', async () => { - events = [sampleDocWithSortId('123', ['1.1.1.1', '2.2.2.2'])]; + events = [sampleDocWithSortId('123', undefined, ['1.1.1.1', '2.2.2.2'])]; (exceptionItem.entries[0] as EntryList).field = 'source.ip'; const [{ matchedSet }] = await createFieldAndSetTuples({ listClient, @@ -150,7 +153,10 @@ describe('filterEventsAgainstList', () => { }); test('it returns 2 fields when given two exception list items', async () => { - events = [sampleDocWithSortId('123', '1.1.1.1'), sampleDocWithSortId('456', '2.2.2.2')]; + events = [ + sampleDocWithSortId('123', undefined, '1.1.1.1'), + sampleDocWithSortId('456', undefined, '2.2.2.2'), + ]; exceptionItem.entries = [ { field: 'source.ip', @@ -182,7 +188,10 @@ describe('filterEventsAgainstList', () => { }); test('it returns two matched sets from two different events, one excluded, and one included', async () => { - events = [sampleDocWithSortId('123', '1.1.1.1'), sampleDocWithSortId('456', '2.2.2.2')]; + events = [ + sampleDocWithSortId('123', undefined, '1.1.1.1'), + sampleDocWithSortId('456', undefined, '2.2.2.2'), + ]; exceptionItem.entries = [ { field: 'source.ip', @@ -215,7 +224,10 @@ describe('filterEventsAgainstList', () => { }); test('it returns two fields from two different events', async () => { - events = [sampleDocWithSortId('123', '1.1.1.1'), sampleDocWithSortId('456', '2.2.2.2')]; + events = [ + sampleDocWithSortId('123', undefined, '1.1.1.1'), + sampleDocWithSortId('456', undefined, '2.2.2.2'), + ]; exceptionItem.entries = [ { field: 'source.ip', @@ -249,8 +261,8 @@ describe('filterEventsAgainstList', () => { test('it returns two matches from two different events', async () => { events = [ - sampleDocWithSortId('123', '1.1.1.1', '3.3.3.3'), - sampleDocWithSortId('456', '2.2.2.2', '5.5.5.5'), + sampleDocWithSortId('123', undefined, '1.1.1.1', '3.3.3.3'), + sampleDocWithSortId('456', undefined, '2.2.2.2', '5.5.5.5'), ]; exceptionItem.entries = [ { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/create_set_to_filter_against.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/create_set_to_filter_against.test.ts index aae4a7aae2b9e..743218f9ed940 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/create_set_to_filter_against.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/create_set_to_filter_against.test.ts @@ -14,7 +14,7 @@ import { buildRuleMessageMock as buildRuleMessage } from '../rule_messages.mock' describe('createSetToFilterAgainst', () => { let listClient = listMock.getListClient(); - let events = [sampleDocWithSortId('123', '1.1.1.1')]; + let events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; beforeEach(() => { jest.clearAllMocks(); @@ -27,7 +27,7 @@ describe('createSetToFilterAgainst', () => { })) ) ); - events = [sampleDocWithSortId('123', '1.1.1.1')]; + events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; }); afterEach(() => { @@ -49,7 +49,7 @@ describe('createSetToFilterAgainst', () => { }); test('it returns 1 field if the list returns a single item', async () => { - events = [sampleDocWithSortId('123', '1.1.1.1')]; + events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; const field = await createSetToFilterAgainst({ events, field: 'source.ip', @@ -68,7 +68,10 @@ describe('createSetToFilterAgainst', () => { }); test('it returns 2 fields if the list returns 2 items', async () => { - events = [sampleDocWithSortId('123', '1.1.1.1'), sampleDocWithSortId('123', '2.2.2.2')]; + events = [ + sampleDocWithSortId('123', undefined, '1.1.1.1'), + sampleDocWithSortId('123', undefined, '2.2.2.2'), + ]; const field = await createSetToFilterAgainst({ events, field: 'source.ip', @@ -87,7 +90,10 @@ describe('createSetToFilterAgainst', () => { }); test('it returns 0 fields if the field does not match up to a valid field within the event', async () => { - events = [sampleDocWithSortId('123', '1.1.1.1'), sampleDocWithSortId('123', '2.2.2.2')]; + events = [ + sampleDocWithSortId('123', undefined, '1.1.1.1'), + sampleDocWithSortId('123', undefined, '2.2.2.2'), + ]; const field = await createSetToFilterAgainst({ events, field: 'nonexistent.field', // field does not exist diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/filter_events.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/filter_events.test.ts index eb5c69e8abfe8..45a058b55d84b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/filter_events.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/filter_events.test.ts @@ -14,7 +14,7 @@ import { FieldSet } from './types'; describe('filterEvents', () => { let listClient = listMock.getListClient(); - let events = [sampleDocWithSortId('123', '1.1.1.1')]; + let events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; beforeEach(() => { jest.clearAllMocks(); @@ -27,7 +27,7 @@ describe('filterEvents', () => { })) ) ); - events = [sampleDocWithSortId('123', '1.1.1.1')]; + events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; }); afterEach(() => { @@ -35,7 +35,7 @@ describe('filterEvents', () => { }); test('it filters out the event if it is "included"', () => { - events = [sampleDocWithSortId('123', '1.1.1.1')]; + events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; const fieldAndSetTuples: FieldSet[] = [ { field: 'source.ip', @@ -51,7 +51,7 @@ describe('filterEvents', () => { }); test('it does not filter out the event if it is "excluded"', () => { - events = [sampleDocWithSortId('123', '1.1.1.1')]; + events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; const fieldAndSetTuples: FieldSet[] = [ { field: 'source.ip', @@ -67,7 +67,7 @@ describe('filterEvents', () => { }); test('it does NOT filter out the event if the field is not found', () => { - events = [sampleDocWithSortId('123', '1.1.1.1')]; + events = [sampleDocWithSortId('123', undefined, '1.1.1.1')]; const fieldAndSetTuples: FieldSet[] = [ { field: 'madeup.nonexistent', // field does not exist @@ -83,7 +83,10 @@ describe('filterEvents', () => { }); test('it does NOT filter out the event if it is in both an inclusion and exclusion list', () => { - events = [sampleDocWithSortId('123', '1.1.1.1'), sampleDocWithSortId('123', '2.2.2.2')]; + events = [ + sampleDocWithSortId('123', undefined, '1.1.1.1'), + sampleDocWithSortId('123', undefined, '2.2.2.2'), + ]; const fieldAndSetTuples: FieldSet[] = [ { field: 'source.ip', From d155fed1764666fcb1d25441d5a3b4dc3a820685 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 20 Apr 2021 13:01:14 -0400 Subject: [PATCH 13/13] removes isEmpty since it doesn't check for empty strings --- .../server/lib/detection_engine/signals/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index 50f42a4bd432c..bd37cf62c74b0 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -863,7 +863,7 @@ export const getSafeSortIds = (sortIds: SortResults | undefined) => { return sortIds?.map((sortId) => { // haven't determined when we would receive a null value for a sort id // but in case we do, default to sending the stringified Java max_int - if (sortId == null || isEmpty(sortId) || sortId >= Number.MAX_SAFE_INTEGER) { + if (sortId == null || sortId === '' || sortId >= Number.MAX_SAFE_INTEGER) { return '9223372036854775807'; } return sortId;