[Security Solution][Resolver] Fix resolver isStart event bug #73357
Conversation
jonathan-buttner
added
v8.0.0
release_note:skip
jonathan-buttner
requested review from
oatkiller,
bkimmel,
michaelolo24,
james-elastic,
achuguy and
kqualters-elastic
|
Pinging @elastic/endpoint-app-team (Feature:Endpoint) |
|
Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility) |
| const event = generator.generateEvent({ eventCategory: 'registry', extensions }); | ||
| expect(descriptiveName(event)).toEqual({ subject: `HKLM/Windows/Software/abc` }); | ||
| }); | ||
| describe('Event descriptive names', () => { |
There was a problem hiding this comment.
These were just wrapped in a new describe block and indented
| if (isLegacyEvent(event)) { | ||
| return event.event?.type === 'process_start' || event.event?.action === 'fork_event'; | ||
| } | ||
|
|
||
| if (Array.isArray(event.event.type)) { | ||
| return event.event.type.length === 1 && event.event.type[0] === 'start'; |
There was a problem hiding this comment.
🔴 https://www.elastic.co/guide/en/ecs/current/ecs-event.html The type entry doesn't say anything about start having to come first or the array needing to have a length of one. Should we check instead to see that it .includes start
There was a problem hiding this comment.
Yeah good point 👍 I'll update it.
💚 Build SucceededBuild metricsasync chunks size
History
To update your PR or re-run it, just comment with: |
| if (isLegacyEvent(event)) { | ||
| return event.event?.type === 'process_start' || event.event?.action === 'fork_event'; | ||
| } | ||
|
|
||
| if (Array.isArray(event.event.type)) { | ||
| return event.event.type.includes('start'); |
There was a problem hiding this comment.
Not necessary for this PR if you don't want to wait for CI all over again 😅 , but I would put start, process_start, for_event, etc... all in some kind of constants file. It'll help avoid spelling bugs in any future updates / refactors and also give us a central place to see all these terms.
There was a problem hiding this comment.
consider using typescript types and interfaces for that kind of thing.
|
Nice catch |
jonathan-buttner
changed the title
…#73357) * Check if category is array * Adding more tests and renaming to isStart * Handling the case where start is not at the front
…#73357) * Check if category is array * Adding more tests and renaming to isStart * Handling the case where start is not at the front
…#73448) * Check if category is array * Adding more tests and renaming to isStart * Handling the case where start is not at the front Co-authored-by: Elastic Machine <[email protected]>
This PR fixes a bug in the resolver backend where process start events were not correctly being identified because the comparison was not handling
event.typebeing an array. This caused the backend to never return children.isStartshould now handleevent.typeas an array or a string. If it is an array is should only be of length 1 and be['start'].Our api_integration did not cover this because the resolver generator is creating process events with
event.type === 'start'.I switched it to be inline with how the endpoint defines process events where
event.typewill be an array and I added some more unit tests to make sure we handle both cases.