From e32fed56bd6048e4dbb68f9690c8186aea0c6e40 Mon Sep 17 00:00:00 2001 From: Mika Ayenson Date: Wed, 14 Aug 2024 13:54:51 -0500 Subject: [PATCH 1/6] [FR] Add Alert Suppression for Addtional Rule Types --- detection_rules/cli_utils.py | 3 +++ detection_rules/rule.py | 16 +++++++++------- tests/test_all_rules.py | 12 ++++++++++-- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/detection_rules/cli_utils.py b/detection_rules/cli_utils.py index 95fdc53f2d2..79258fb8e79 100644 --- a/detection_rules/cli_utils.py +++ b/detection_rules/cli_utils.py @@ -132,6 +132,9 @@ def rule_prompt(path=None, rule_type=None, required_only=True, save=True, verbos for name, options in props.items(): + if name == 'index' and kwargs["type"] == "esql": + continue + if name == 'type': contents[name] = rule_type continue diff --git a/detection_rules/rule.py b/detection_rules/rule.py index d5a8c9a8134..f727a1c1359 100644 --- a/detection_rules/rule.py +++ b/detection_rules/rule.py @@ -758,13 +758,6 @@ def validates_index_and_data_view_id(self, data, **kwargs): if data.get('index') and data.get('data_view_id'): raise ValidationError("Only one of index or data_view_id should be set.") - @validates_schema - def validates_query_data(self, data, **kwargs): - """Custom validation for query rule type and subclasses.""" - # alert suppression is only valid for query rule type and not any of its subclasses - if data.get('alert_suppression') and data['type'] not in ('query', 'threshold'): - raise ValidationError("Alert suppression is only valid for query and threshold rule types.") - @dataclass(frozen=True) class MachineLearningRuleData(BaseRuleData): @@ -772,6 +765,7 @@ class MachineLearningRuleData(BaseRuleData): anomaly_threshold: int machine_learning_job_id: Union[str, List[str]] + alert_suppression: Optional[AlertSuppressionMapping] = field(metadata=dict(metadata=dict(min_compat="8.15"))) @dataclass(frozen=True) @@ -811,6 +805,7 @@ class HistoryWindowStart: type: Literal["new_terms"] new_terms: NewTermsMapping + alert_suppression: Optional[AlertSuppressionMapping] = field(metadata=dict(metadata=dict(min_compat="8.14"))) @pre_load def preload_data(self, data: dict, **kwargs) -> dict: @@ -849,6 +844,11 @@ class EQLRuleData(QueryRuleData): timestamp_field: Optional[str] = field(metadata=dict(metadata=dict(min_compat="8.0"))) event_category_override: Optional[str] = field(metadata=dict(metadata=dict(min_compat="8.0"))) tiebreaker_field: Optional[str] = field(metadata=dict(metadata=dict(min_compat="8.0"))) + alert_suppression: Optional[AlertSuppressionMapping] = field(metadata=dict(metadata=dict(min_compat="8.14"))) + + def __post_init__(self): + # Eagerly compute cached properties + object.__getattribute__(self, 'is_sequence') def convert_relative_delta(self, lookback: str) -> int: now = len("now") @@ -905,6 +905,7 @@ class ESQLRuleData(QueryRuleData): type: Literal["esql"] language: Literal["esql"] query: str + alert_suppression: Optional[AlertSuppressionMapping] = field(metadata=dict(metadata=dict(min_compat="8.15"))) @validates_schema def validates_esql_data(self, data, **kwargs): @@ -939,6 +940,7 @@ class ThreatMapEntry: threat_language: Optional[definitions.FilterLanguages] threat_index: List[str] threat_indicator_path: Optional[str] + alert_suppression: Optional[AlertSuppressionMapping] = field(metadata=dict(metadata=dict(min_compat="8.13"))) def validate_query(self, meta: RuleMeta) -> None: super(ThreatMatchRuleData, self).validate_query(meta) diff --git a/tests/test_all_rules.py b/tests/test_all_rules.py index 42bbfc5f87f..e59694f8a09 100644 --- a/tests/test_all_rules.py +++ b/tests/test_all_rules.py @@ -1352,8 +1352,7 @@ class TestAlertSuppression(BaseRuleTest): def test_group_field_in_schemas(self): """Test to ensure the fields are defined is in ECS/Beats/Integrations schema.""" for rule in self.all_rules: - rule_type = rule.contents.data.get('type') - if rule_type in ('query', 'threshold') and rule.contents.data.get('alert_suppression'): + if rule.contents.data.get('alert_suppression'): if isinstance(rule.contents.data.alert_suppression, AlertSuppressionMapping): group_by_fields = rule.contents.data.alert_suppression.group_by elif isinstance(rule.contents.data.alert_suppression, ThresholdAlertSuppression): @@ -1381,3 +1380,12 @@ def test_group_field_in_schemas(self): if fld not in schema.keys(): self.fail(f"{self.rule_str(rule)} alert suppression field {fld} not \ found in ECS, Beats, or non-ecs schemas") + + @unittest.skipIf(PACKAGE_STACK_VERSION < Version.parse("8.14.0"), + "Test only applicable to 8.14+ stacks for eql non-sequence rule alert suppression feature.") + def test_eql_non_sequence_support_only(self): + for rule in self.all_rules: + if rule.contents.data.get('alert_suppression') and rule.contents.data.is_sequence: + # is_sequence method not yet available during schema validation + # so we have to check in a unit test + self.fail(f'{self.rule_str(rule)} Sequence rules cannot have alert suppression') From 71b3d70ed7dd6dc4958da2a8cc715213fa076261 Mon Sep 17 00:00:00 2001 From: Mika Ayenson Date: Wed, 14 Aug 2024 13:59:57 -0500 Subject: [PATCH 2/6] cleanup testing is_sequence logic --- detection_rules/rule.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/detection_rules/rule.py b/detection_rules/rule.py index f727a1c1359..d57b9c86e3e 100644 --- a/detection_rules/rule.py +++ b/detection_rules/rule.py @@ -846,10 +846,6 @@ class EQLRuleData(QueryRuleData): tiebreaker_field: Optional[str] = field(metadata=dict(metadata=dict(min_compat="8.0"))) alert_suppression: Optional[AlertSuppressionMapping] = field(metadata=dict(metadata=dict(min_compat="8.14"))) - def __post_init__(self): - # Eagerly compute cached properties - object.__getattribute__(self, 'is_sequence') - def convert_relative_delta(self, lookback: str) -> int: now = len("now") min_length = now + len('+5m') From 8251cd85e707398506e9096d551d4ccfe6397566 Mon Sep 17 00:00:00 2001 From: Mika Ayenson Date: Wed, 14 Aug 2024 14:35:44 -0500 Subject: [PATCH 3/6] ensure test only runs across eql rules --- tests/test_all_rules.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_all_rules.py b/tests/test_all_rules.py index e59694f8a09..b2a8796bcc1 100644 --- a/tests/test_all_rules.py +++ b/tests/test_all_rules.py @@ -24,7 +24,7 @@ load_integrations_manifests, load_integrations_schemas) from detection_rules.packaging import current_stack_version -from detection_rules.rule import (AlertSuppressionMapping, QueryRuleData, QueryValidator, +from detection_rules.rule import (AlertSuppressionMapping, EQLRuleData, QueryRuleData, QueryValidator, ThresholdAlertSuppression, TOMLRuleContents) from detection_rules.rule_loader import FILE_PATTERN, RULES_CONFIG from detection_rules.rule_validators import EQLValidator, KQLValidator @@ -1385,7 +1385,8 @@ def test_group_field_in_schemas(self): "Test only applicable to 8.14+ stacks for eql non-sequence rule alert suppression feature.") def test_eql_non_sequence_support_only(self): for rule in self.all_rules: - if rule.contents.data.get('alert_suppression') and rule.contents.data.is_sequence: + if isinstance(rule.contents.data, EQLRuleData) and rule.contents.data.get('alert_suppression') and \ + rule.contents.data.is_sequence: # is_sequence method not yet available during schema validation # so we have to check in a unit test self.fail(f'{self.rule_str(rule)} Sequence rules cannot have alert suppression') From 7b0172146b4945eefd15382004755f1951bf6c82 Mon Sep 17 00:00:00 2001 From: Mika Ayenson Date: Wed, 14 Aug 2024 14:39:08 -0500 Subject: [PATCH 4/6] lint --- tests/test_all_rules.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_all_rules.py b/tests/test_all_rules.py index b2a8796bcc1..ecd3ca38ae8 100644 --- a/tests/test_all_rules.py +++ b/tests/test_all_rules.py @@ -1385,8 +1385,12 @@ def test_group_field_in_schemas(self): "Test only applicable to 8.14+ stacks for eql non-sequence rule alert suppression feature.") def test_eql_non_sequence_support_only(self): for rule in self.all_rules: - if isinstance(rule.contents.data, EQLRuleData) and rule.contents.data.get('alert_suppression') and \ - rule.contents.data.is_sequence: + if ( + isinstance(rule.contents.data, EQLRuleData) and rule.contents.data.get("alert_suppression") + and rule.contents.data.is_sequence # noqa: W503 + ): # is_sequence method not yet available during schema validation # so we have to check in a unit test - self.fail(f'{self.rule_str(rule)} Sequence rules cannot have alert suppression') + self.fail( + f"{self.rule_str(rule)} Sequence rules cannot have alert suppression" + ) From c14829af223878bd2655c27e8af3b503e339cdbb Mon Sep 17 00:00:00 2001 From: Mika Ayenson Date: Thu, 15 Aug 2024 08:28:03 -0500 Subject: [PATCH 5/6] Update detection_rules/cli_utils.py Co-authored-by: Eric Forte <119343520+eric-forte-elastic@users.noreply.github.com> --- detection_rules/cli_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detection_rules/cli_utils.py b/detection_rules/cli_utils.py index 79258fb8e79..ab0e64a843d 100644 --- a/detection_rules/cli_utils.py +++ b/detection_rules/cli_utils.py @@ -132,7 +132,7 @@ def rule_prompt(path=None, rule_type=None, required_only=True, save=True, verbos for name, options in props.items(): - if name == 'index' and kwargs["type"] == "esql": + if name == 'index' and kwargs.get("type") == "esql": continue if name == 'type': From e996bd30a2b297ac7b11efc4f6522ff35f11a3bd Mon Sep 17 00:00:00 2001 From: Mika Ayenson Date: Thu, 15 Aug 2024 12:14:05 -0500 Subject: [PATCH 6/6] save test import to tmp --- detection_rules/etc/test_cli.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detection_rules/etc/test_cli.bash b/detection_rules/etc/test_cli.bash index 1a59fb6bffa..90a1a617b74 100755 --- a/detection_rules/etc/test_cli.bash +++ b/detection_rules/etc/test_cli.bash @@ -19,7 +19,7 @@ mkdir tmp-export 2>/dev/null python -m detection_rules export-rules-from-repo --rule-id 0a97b20f-4144-49ea-be32-b540ecc445de -o tmp-export/test_rule.ndjson echo "Importing rule by ID: 0a97b20f-4144-49ea-be32-b540ecc445de" -python -m detection_rules import-rules-to-repo tmp-export/test_rule.ndjson --required-only +python -m detection_rules import-rules-to-repo tmp-export/test_rule.ndjson --required-only -s tmp-export rm -rf tmp-export echo "Updating rule data schemas"