-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Security Solution][Detections] Set default indicator path to reduce friction with new filebeat modules #92081
Changes from 3 commits
7c90377
d35a8fc
c9ec203
40c280b
4790415
d5c24d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ | |
*/ | ||
|
||
import { get } from 'lodash'; | ||
import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants'; | ||
import { INDICATOR_DESTINATION_PATH } from '../../../../../common/constants'; | ||
|
||
import { getThreatListItemMock } from './build_threat_mapping_filter.mock'; | ||
import { | ||
|
@@ -94,7 +94,7 @@ describe('buildMatchedIndicator', () => { | |
const indicators = buildMatchedIndicator({ | ||
queries: [], | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the intention is to keep this explicit, no need for modification - although it looks like we could declare an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed on DRYing this up, I think I'll do that here 👍. The intention with hardcoding the path here here was to make the tests robust to default values changing; since defaulting happens outside of these functions I only want those integration/UI tests to change if those defaults do. |
||
}); | ||
|
||
expect(indicators).toEqual([]); | ||
|
@@ -104,7 +104,7 @@ describe('buildMatchedIndicator', () => { | |
const [indicator] = buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}); | ||
|
||
expect(get(indicator, 'matched.atomic')).toEqual('domain_1'); | ||
|
@@ -114,7 +114,7 @@ describe('buildMatchedIndicator', () => { | |
const [indicator] = buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}); | ||
|
||
expect(get(indicator, 'matched.field')).toEqual('event.field'); | ||
|
@@ -124,7 +124,7 @@ describe('buildMatchedIndicator', () => { | |
const [indicator] = buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}); | ||
|
||
expect(get(indicator, 'matched.type')).toEqual('type_1'); | ||
|
@@ -153,7 +153,7 @@ describe('buildMatchedIndicator', () => { | |
const indicators = buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}); | ||
|
||
expect(indicators).toHaveLength(queries.length); | ||
|
@@ -163,7 +163,7 @@ describe('buildMatchedIndicator', () => { | |
const indicators = buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}); | ||
|
||
expect(indicators).toEqual([ | ||
|
@@ -228,7 +228,7 @@ describe('buildMatchedIndicator', () => { | |
const indicators = buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}); | ||
|
||
expect(indicators).toEqual([ | ||
|
@@ -253,7 +253,7 @@ describe('buildMatchedIndicator', () => { | |
const indicators = buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}); | ||
|
||
expect(indicators).toEqual([ | ||
|
@@ -285,7 +285,7 @@ describe('buildMatchedIndicator', () => { | |
const indicators = buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}); | ||
|
||
expect(indicators).toEqual([ | ||
|
@@ -317,7 +317,7 @@ describe('buildMatchedIndicator', () => { | |
buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}) | ||
).toThrowError('Expected indicator field to be an object, but found: not an object'); | ||
}); | ||
|
@@ -338,7 +338,7 @@ describe('buildMatchedIndicator', () => { | |
buildMatchedIndicator({ | ||
queries, | ||
threats, | ||
indicatorPath: DEFAULT_INDICATOR_PATH, | ||
indicatorPath: 'threat.indicator', | ||
}) | ||
).toThrowError('Expected indicator field to be an object, but found: not an object'); | ||
}); | ||
|
@@ -367,7 +367,7 @@ describe('enrichSignalThreatMatches', () => { | |
const enrichedSignals = await enrichSignalThreatMatches( | ||
signals, | ||
getMatchedThreats, | ||
DEFAULT_INDICATOR_PATH | ||
'threat.indicator' | ||
); | ||
|
||
expect(enrichedSignals.hits.hits).toEqual([]); | ||
|
@@ -382,10 +382,10 @@ describe('enrichSignalThreatMatches', () => { | |
const enrichedSignals = await enrichSignalThreatMatches( | ||
signals, | ||
getMatchedThreats, | ||
DEFAULT_INDICATOR_PATH | ||
'threat.indicator' | ||
); | ||
const [enrichedHit] = enrichedSignals.hits.hits; | ||
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH); | ||
const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH); | ||
|
||
expect(indicators).toEqual([ | ||
{ existing: 'indicator' }, | ||
|
@@ -407,10 +407,10 @@ describe('enrichSignalThreatMatches', () => { | |
const enrichedSignals = await enrichSignalThreatMatches( | ||
signals, | ||
getMatchedThreats, | ||
DEFAULT_INDICATOR_PATH | ||
'threat.indicator' | ||
); | ||
const [enrichedHit] = enrichedSignals.hits.hits; | ||
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH); | ||
const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH); | ||
|
||
expect(indicators).toEqual([ | ||
{ | ||
|
@@ -428,10 +428,10 @@ describe('enrichSignalThreatMatches', () => { | |
const enrichedSignals = await enrichSignalThreatMatches( | ||
signals, | ||
getMatchedThreats, | ||
DEFAULT_INDICATOR_PATH | ||
'threat.indicator' | ||
); | ||
const [enrichedHit] = enrichedSignals.hits.hits; | ||
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH); | ||
const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH); | ||
|
||
expect(indicators).toEqual([ | ||
{ existing: 'indicator' }, | ||
|
@@ -451,7 +451,7 @@ describe('enrichSignalThreatMatches', () => { | |
}); | ||
const signals = getSignalsResponseMock([signalHit]); | ||
await expect(() => | ||
enrichSignalThreatMatches(signals, getMatchedThreats, DEFAULT_INDICATOR_PATH) | ||
enrichSignalThreatMatches(signals, getMatchedThreats, 'threat.indicator') | ||
).rejects.toThrowError('Expected threat field to be an object, but found: whoops'); | ||
}); | ||
|
||
|
@@ -487,7 +487,7 @@ describe('enrichSignalThreatMatches', () => { | |
'custom_threat.custom_indicator' | ||
); | ||
const [enrichedHit] = enrichedSignals.hits.hits; | ||
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH); | ||
const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH); | ||
|
||
expect(indicators).toEqual([ | ||
{ | ||
|
@@ -530,13 +530,13 @@ describe('enrichSignalThreatMatches', () => { | |
const enrichedSignals = await enrichSignalThreatMatches( | ||
signals, | ||
getMatchedThreats, | ||
DEFAULT_INDICATOR_PATH | ||
'threat.indicator' | ||
); | ||
expect(enrichedSignals.hits.total).toEqual(expect.objectContaining({ value: 1 })); | ||
expect(enrichedSignals.hits.hits).toHaveLength(1); | ||
|
||
const [enrichedHit] = enrichedSignals.hits.hits; | ||
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH); | ||
const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH); | ||
|
||
expect(indicators).toEqual([ | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,6 @@ | |
*/ | ||
|
||
import { get, isObject } from 'lodash'; | ||
import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants'; | ||
|
||
import type { SignalSearchResponse, SignalSourceHit } from '../types'; | ||
import type { | ||
|
@@ -92,7 +91,7 @@ export const enrichSignalThreatMatches = async ( | |
if (!isObject(threat)) { | ||
throw new Error(`Expected threat field to be an object, but found: ${threat}`); | ||
} | ||
const existingIndicatorValue = get(signalHit._source, DEFAULT_INDICATOR_PATH) ?? []; | ||
const existingIndicatorValue = get(signalHit._source, 'threat.indicator') ?? []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could use the previously declared There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm inclined to leave this hardcoded string for now, as the check above and the object generation below this line both make assumptions about this string in different ways. |
||
const existingIndicators = [existingIndicatorValue].flat(); // ensure indicators is an array | ||
|
||
return { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be simplified as
threatIndicatorPath || DEFAULT_INDICATOR_SOURCE_PATH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks!