WIP: Elasticsearch Matcher #28
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi there! Thanks a lot for the PR, this is great and I'll be happy to actively help with it. At a high level, it would be desirable to have the following:
Otherwise looks good overall, let me know when you'd like me to test this / a more in-depth review! |
bkaznowski
reviewed
Nov 30, 2023
Comment on lines
+13
to
+25
| var containsUuid bool | ||
| for _,alert := range alerts { | ||
| containsUuid = false | ||
| for _,v := range alert.Source { | ||
| if strings.Contains(v.(string), uuid) { | ||
| containsUuid = true | ||
| break | ||
| } | ||
| } | ||
| if containsUuid { | ||
| filteredAlerts = append(filteredAlerts, alert) | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| var containsUuid bool | |
| for _,alert := range alerts { | |
| containsUuid = false | |
| for _,v := range alert.Source { | |
| if strings.Contains(v.(string), uuid) { | |
| containsUuid = true | |
| break | |
| } | |
| } | |
| if containsUuid { | |
| filteredAlerts = append(filteredAlerts, alert) | |
| } | |
| } | |
| for _, alert := range alerts { | |
| containsUuid := false | |
| for _, v := range alert.Source { | |
| if strings.Contains(v.(string), uuid) { | |
| containsUuid = true | |
| break | |
| } | |
| } | |
| if containsUuid { | |
| filteredAlerts = append(filteredAlerts, alert) | |
| } | |
| } |
Spacing in the for loops and moving the variable inside the loop
| // Parse the response | ||
| strippedResponse, err := StripHTTPStatusCode(res.String()) | ||
| if err != nil { | ||
| log.Fatal("Error while stripping prepended HTTP status code") |
There was a problem hiding this comment.
Do you want to include the error here? Fatal will exit immediately and so the return won't do anything and you will lose the original error. Although maybe you don't need it because there is only one error it can be 🤔
| } | ||
| var data ElasticsearchQueryResponse | ||
| if err := json.Unmarshal([]byte(strippedResponse), &data); err != nil { | ||
| log.Fatal("Error unmarshalling JSON string into ElasticsearchQueryResponse struct") |
There was a problem hiding this comment.
You may want to log the error here so it is easier to debug
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WORK IN PROGRESS!
This PR is by no means ready, just wanted to have it out there so structural issues and next steps can be discussed sooner rather than later! It currently comes with no documentation and no unit tests.
What does this PR do?
.siem-signals-defaultindex and retrieve recent, open alerts associated with a particular rule that have thedetonationUuidin a specified fieldMotivation
This implements issue #24 : Alert Matching with Elastic
Want to see E2E detection testing brought to our organisation. We use Elasticsearch instead of Datadog. No reason this project couldn't be extended to add an Elasticsearch matcher.
Checklist