-
Notifications
You must be signed in to change notification settings - Fork 587
[New Rules] External Promotion Alerts #4903
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
base: main
Are you sure you want to change the base?
Conversation
Enhancement - GuidelinesThese guidelines serve as a reminder set of considerations when addressing adding a new schema feature to the code. Documentation and Context
Code Standards and Practices
Testing
Additional Schema Related Checks
|
⛔️ Test failed Results
|
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.
LGTM, If we use tag "Promotion: External Alerts" tag, let's also update the "External Alerts" rule tag.
⛔️ Test failed Results
|
@peluja1012 will this cause confusion upstream? |
Hey @Mikaayenson, which rules have the "External alerts" tag currently? We wouldn't want AI4DSOC users to have promotion rule automatically installed that are not relevant for the AI4DSOC use case. Looking at the code, it looks like we are checking for both the presence of the tag AND for a related_integration reference before we automatically install the rule. So if a customer has the Crowdstrike integration installed in AI4DSOC, for example, and there is more than one rule with the tag "Promotion: External alerts" that references the Crowdstrike integration via the |
Based on our discussion, if we accidentally add the tag to another rule that meets similar criteria it will cause issues with the AI4DSOC, so we will not add the tag to other rules. cc. @approksiu |
⛔️ Test failed Results
|
⛔️ Test failed Results
|
⛔️ Test failed Results
|
⛔️ Test failed Results
|
⛔️ Test failed Results
|
⛔️ Test failed Results
|
promotion = true | ||
min_stack_version = "8.18.0" | ||
min_stack_comments = "Introduced support for SentinelOne alert promotion" | ||
updated_date = "2025/08/05" |
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.
Should these updated dates be in the future? It seems like this is incorrect unless I am missing something.
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.
place holder until we're ready to merge
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.
LGTM!
interval = "1m" | ||
language = "kuery" | ||
license = "Elastic License v2" | ||
max_signals = 1000 |
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.
Any specific reason we set this to 1000?
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.
consistency with #4556
field = "event.severity" | ||
operator = "equals" | ||
severity = "low" | ||
value = "1" |
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.
Are these mappings of 1, 2 and 3 a sentinal_one thing?
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.
that's correct, per https://github.com/elastic/security-team/issues/12173#issuecomment-2863155356
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.
Once placeholder dates are updated this looks good to merge (ref). Data checks look good too 👍
Just waiting to test the remaining rules and then we can merge. |
⛔️ Test failed Results
|
Pull Request
Issue link(s):
data_stream.dataset
#4929Summary - What I changed
data_stream.dataset
per integration team guidanceHow To Test
Stack Testing
Note: Didn't have test data for:
Testing should be coming the week the last week of July.
Testing Data
Checklist
bug
,enhancement
,schema
,maintenance
,Rule: New
,Rule: Deprecation
,Rule: Tuning
,Hunt: New
, orHunt: Tuning
so guidelines can be generatedmeta:rapid-merge
label if planning to merge within 24 hours