[Security Solution][Detections] Disables add exception for ML and threshold rules #75802
Conversation
dplumlee
added
Feature:Detection Rules
dplumlee
force-pushed
the
context-menu-disable-fix
branch
from
f699291
to
834362c
Compare
| data: nonEcsRowData, | ||
| fieldName: 'signal.rule.type', | ||
| }); | ||
| const [ruleType] = ruleTypes as Array<Rule['type']>; |
There was a problem hiding this comment.
weird typecheck work around, better suggestions welcome
There was a problem hiding this comment.
RuleType is a more direct type reference, but up to you on Rule['type'] vs RuleType.
If getMappedNonEcsValue can return something other than a string[] maybe it's worth making it generic (to allow getMappedNonEcsValue<RuleType[]>, otherwise I'd say that refining our string to our enum is probably the best we can do here, as far as I'm aware.
Alternately we could loosen the restrictions on those predicate functions and just have them accept strings instead of RuleTypes, but that would have some (IMO) negative downstream consequences.
|
Pinging @elastic/endpoint-response (Team:Endpoint Response) |
|
Pinging @elastic/siem (Team:SIEM) |
There was a problem hiding this comment.
Looks good to me! I had one nit about the predicate name and a few options for addressing the typescript issue, but this is straightforward and good to merge.
It would be nice to have a regression test here; do we maybe have an existing integration test to which we could add an assertion?
| data: nonEcsRowData, | ||
| fieldName: 'signal.rule.type', | ||
| }); | ||
| const [ruleType] = ruleTypes as Array<Rule['type']>; |
There was a problem hiding this comment.
RuleType is a more direct type reference, but up to you on Rule['type'] vs RuleType.
If getMappedNonEcsValue can return something other than a string[] maybe it's worth making it generic (to allow getMappedNonEcsValue<RuleType[]>, otherwise I'd say that refining our string to our enum is probably the best we can do here, as far as I'm aware.
Alternately we could loosen the restrictions on those predicate functions and just have them accept strings instead of RuleTypes, but that would have some (IMO) negative downstream consequences.
| @@ -317,6 +321,15 @@ export const getAlertActions = ({ | |||
| return module === 'endpoint' && kind === 'alert'; | |||
| }; | |||
|
|
|||
| const isFromValidRule = () => { | |||
There was a problem hiding this comment.
"valid rule" doesn't relay much info here, I'd say we be more explicit:
| const isFromValidRule = () => { | |
| const exceptionsAreAllowed = () => { |
|
@MadameSheema We were thinking of adding tests after this pr (#73228) was merged given that it refactors a lot of this and we need to unskip many of the tests for this file anyway. We could open a ticket for adding them back once it's merged |
💚 Build SucceededBuild metricsasync chunks size
History
To update your PR or re-run it, just comment with: |
|
@MadameSheema @rylnd #75934 this is the issue for updating and adding test coverage for the event viewer once patryk's pr is merged |
MindyRS
added
the
Team: SecuritySolution
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Summary
Disables the add exception feature for exceptions created by Machine learning and threshold based rules (#75154)
Checklist
Delete any items that are not applicable to this PR.
For maintainers