Skip to content
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

[Security Solution] When duplicating a prebuilt rule, 'Related Integrations' and 'Required Fields' values are not inherited from the original rule #190628

Closed
Tracked by #174168
pborgonovi opened this issue Aug 15, 2024 · 6 comments · Fixed by #191065
Assignees
Labels
8.16 candidate bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules Feature:Rule Management Security Solution Detection Rule Management fixed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.1 v8.16.0

Comments

@pborgonovi
Copy link

Describe the bug:
When duplicating a prebuilt rule, 'Related Integrations' and 'Required Fields' values are not inherited from the original rule

Kibana/Elasticsearch Stack version:
8.15

Server OS version:

Browser and Browser OS versions:

Elastic Endpoint version:

Original install method (e.g. download page, yum, from source, etc.):

Functional Area (e.g. Endpoint management, timelines, resolver, etc.):

Steps to reproduce:

  1. Duplicate a prebuilt rule containing related integrations and required fields
  2. Open the duplicated rule and validate the fields

Current behavior:
'Related Integrations' and 'Required Fields' values are not inherited from the original rule

Expected behavior:
'Related Integrations' and 'Required Fields' values should be inherited from the original rule as explicitly specified as requirement of:
#173595
#173594

Screenshots (if relevant):

Screen.Recording.2024-08-15.at.8.58.38.AM.mov

Errors in browser console (if relevant):

Provide logs and/or server output (if relevant):

{
    "id": "94c7ce40-4be5-4c80-a899-ca433d48ba60",
    "updated_at": "2024-08-15T15:59:25.696Z",
    "updated_by": "19276298",
    "created_at": "2024-08-15T15:59:25.055Z",
    "created_by": "19276298",
    "name": "Endpoint Security [Duplicate]",
    "tags": [
        "Data Source: Elastic Defend"
    ],
    "interval": "5m",
    "enabled": false,
    "revision": 0,
    "description": "Generates a detection alert each time an Elastic Endpoint Security alert is received. Enabling this rule allows you to immediately begin investigating your Endpoint alerts.",
    "risk_score": 47,
    "severity": "medium",
    "license": "Elastic License v2",
    "output_index": "",
    "rule_name_override": "message",
    "timestamp_override": "event.ingested",
    "author": [
        "Elastic"
    ],
    "false_positives": [],
    "from": "now-10m",
    "rule_id": "00887f99-2fe8-4a9f-986f-a3abbc612db7",
    "max_signals": 10000,
    "risk_score_mapping": [
        {
            "field": "event.risk_score",
            "operator": "equals",
            "value": ""
        }
    ],
    "severity_mapping": [
        {
            "field": "event.severity",
            "operator": "equals",
            "severity": "low",
            "value": "21"
        },
        {
            "field": "event.severity",
            "operator": "equals",
            "severity": "medium",
            "value": "47"
        },
        {
            "field": "event.severity",
            "operator": "equals",
            "severity": "high",
            "value": "73"
        },
        {
            "field": "event.severity",
            "operator": "equals",
            "severity": "critical",
            "value": "99"
        }
    ],
    "threat": [],
    "to": "now",
    "references": [],
    "version": 103,
    "exceptions_list": [],
    "immutable": false,
    "rule_source": {
        "type": "internal"
    },
    "related_integrations": [],
    "required_fields": [],
    "setup": "## Setup\n\nThis rule is configured to generate more **Max alerts per run** than the default 1000 alerts per run set for all rules. This is to ensure that it captures as many alerts as possible.\n\n**IMPORTANT:** The rule's **Max alerts per run** setting can be superseded by the `xpack.alerting.rules.run.alerts.max` Kibana config setting, which determines the maximum alerts generated by _any_ rule in the Kibana alerting framework. For example, if `xpack.alerting.rules.run.alerts.max` is set to 1000, this rule will still generate no more than 1000 alerts even if its own **Max alerts per run** is set higher.\n\nTo make sure this rule can generate as many alerts as it's configured in its own **Max alerts per run** setting, increase the `xpack.alerting.rules.run.alerts.max` system setting accordingly.\n\n**NOTE:** Changing `xpack.alerting.rules.run.alerts.max` is not possible in Serverless projects.",
    "type": "query",
    "language": "kuery",
    "index": [
        "logs-endpoint.alerts-*"
    ],
    "query": "event.kind:alert and event.module:(endpoint and not endgame)\n",
    "actions": []
}

Any additional context (logs, chat logs, magical formulas, etc.):

@pborgonovi pborgonovi added bug Fixes for quality problems that affect the customer experience triage_needed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team labels Aug 15, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@banderror
Copy link
Contributor

@pborgonovi Fantastic catch, appreciate it a lot! This PR will fix this bug: #191065

It's a shame that we missed this during the implementation, then code review, and finally acceptance and exploratory testing for #173595 and #173594 - two very similar features in a row. While acceptance testing is not focused on finding bugs, all the rest of the stages of this process require comprehensive testing with checking all the edge cases, especially when they are listed in the Acceptance Criteria. Fortunately, the bug's impact is not high and we can backport it to a patch version.

cc @nikitaindik @maximpn

banderror added a commit to banderror/kibana that referenced this issue Aug 27, 2024
…ed integrations and required fields from the original rule (elastic#191065)

**Fixes: elastic#190628
**Related to:** elastic#173595,
elastic#173594

## Summary

As stated in the bug ticket, when duplicating a prebuilt rule, the
"Related Integrations" and "Required Fields" values should be inherited
from the original rule, as it was specified in the Acceptance Criteria
for elastic#173595 and
elastic#173594.

This PR:

- Removes the logic that resets these fields to empty arrays for
duplicated prebuilt rules - we needed this logic in the past because
these fields were not editable in the UI, but we don't need it anymore.
- Updates the corresponding unit tests.

## Screenshots

These screenshots were taken after introducing the fixes.

**Original prebuilt rule:**

<img width="1463" alt="Screenshot_2024-08-23_at_13_25_07"
src="https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119">

**Duplicated prebuilt rule:**

<img width="1469" alt="Screenshot_2024-08-23_at_13_25_43"
src="https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b">

### Checklist

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit b144c05)

# Conflicts:
#	x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts
#	x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts
@banderror banderror reopened this Aug 27, 2024
@banderror
Copy link
Contributor

@pborgonovi The bug has been fixed in #191065 and I'm waiting for the backport to get merged to 8.15. I hope it will be merged soon and the fix will make it to v8.15.1.

banderror referenced this issue Aug 27, 2024
…y related integrations and required fields from the original rule (#191065) (#191493)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Security Solution] Fix prebuilt rule duplication logic to copy
related integrations and required fields from the original rule
(#191065)](#191065)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Georgii
Gorbachev","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-08-26T13:42:52Z","message":"[Security
Solution] Fix prebuilt rule duplication logic to copy related
integrations and required fields from the original rule
(#191065)\n\n**Fixes:
https://github.com/elastic/kibana/issues/190628**\r\n**Related to:**
https://github.com/elastic/kibana/issues/173595,\r\nhttps://github.com/elastic/kibana/issues/173594\r\n\r\n##
Summary\r\n\r\nAs stated in the bug ticket, when duplicating a prebuilt
rule, the\r\n\"Related Integrations\" and \"Required Fields\" values
should be inherited\r\nfrom the original rule, as it was specified in
the Acceptance Criteria\r\nfor
#173595
and\r\nhttps://github.com//issues/173594.\r\n\r\nThis
PR:\r\n\r\n- Removes the logic that resets these fields to empty arrays
for\r\nduplicated prebuilt rules - we needed this logic in the past
because\r\nthese fields were not editable in the UI, but we don't need
it anymore.\r\n- Updates the corresponding unit tests.\r\n\r\n##
Screenshots\r\n\r\nThese screenshots were taken after introducing the
fixes.\r\n\r\n**Original prebuilt rule:**\r\n\r\n<img width=\"1463\"
alt=\"Screenshot_2024-08-23_at_13_25_07\"\r\nsrc=\"https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119\">\r\n\r\n**Duplicated
prebuilt rule:**\r\n\r\n<img width=\"1469\"
alt=\"Screenshot_2024-08-23_at_13_25_43\"\r\nsrc=\"https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b\">\r\n\r\n###
Checklist\r\n\r\n- [
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"b144c05e8f39f28dd9551b7c62daa01cfa1d2cd5","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","impact:medium","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Management","Team:Detection Rule Management","Feature:Prebuilt Detection
Rules","v8.16.0","v8.15.1"],"number":191065,"url":"https://github.com/elastic/kibana/pull/191065","mergeCommit":{"message":"[Security
Solution] Fix prebuilt rule duplication logic to copy related
integrations and required fields from the original rule
(#191065)\n\n**Fixes:
https://github.com/elastic/kibana/issues/190628**\r\n**Related to:**
https://github.com/elastic/kibana/issues/173595,\r\nhttps://github.com/elastic/kibana/issues/173594\r\n\r\n##
Summary\r\n\r\nAs stated in the bug ticket, when duplicating a prebuilt
rule, the\r\n\"Related Integrations\" and \"Required Fields\" values
should be inherited\r\nfrom the original rule, as it was specified in
the Acceptance Criteria\r\nfor
#173595
and\r\nhttps://github.com//issues/173594.\r\n\r\nThis
PR:\r\n\r\n- Removes the logic that resets these fields to empty arrays
for\r\nduplicated prebuilt rules - we needed this logic in the past
because\r\nthese fields were not editable in the UI, but we don't need
it anymore.\r\n- Updates the corresponding unit tests.\r\n\r\n##
Screenshots\r\n\r\nThese screenshots were taken after introducing the
fixes.\r\n\r\n**Original prebuilt rule:**\r\n\r\n<img width=\"1463\"
alt=\"Screenshot_2024-08-23_at_13_25_07\"\r\nsrc=\"https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119\">\r\n\r\n**Duplicated
prebuilt rule:**\r\n\r\n<img width=\"1469\"
alt=\"Screenshot_2024-08-23_at_13_25_43\"\r\nsrc=\"https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b\">\r\n\r\n###
Checklist\r\n\r\n- [
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"b144c05e8f39f28dd9551b7c62daa01cfa1d2cd5"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/191065","number":191065,"mergeCommit":{"message":"[Security
Solution] Fix prebuilt rule duplication logic to copy related
integrations and required fields from the original rule
(#191065)\n\n**Fixes:
https://github.com/elastic/kibana/issues/190628**\r\n**Related to:**
https://github.com/elastic/kibana/issues/173595,\r\nhttps://github.com/elastic/kibana/issues/173594\r\n\r\n##
Summary\r\n\r\nAs stated in the bug ticket, when duplicating a prebuilt
rule, the\r\n\"Related Integrations\" and \"Required Fields\" values
should be inherited\r\nfrom the original rule, as it was specified in
the Acceptance Criteria\r\nfor
#173595
and\r\nhttps://github.com//issues/173594.\r\n\r\nThis
PR:\r\n\r\n- Removes the logic that resets these fields to empty arrays
for\r\nduplicated prebuilt rules - we needed this logic in the past
because\r\nthese fields were not editable in the UI, but we don't need
it anymore.\r\n- Updates the corresponding unit tests.\r\n\r\n##
Screenshots\r\n\r\nThese screenshots were taken after introducing the
fixes.\r\n\r\n**Original prebuilt rule:**\r\n\r\n<img width=\"1463\"
alt=\"Screenshot_2024-08-23_at_13_25_07\"\r\nsrc=\"https://github.com/user-attachments/assets/ad8673f5-aba3-40c8-ae91-bbd7d334b119\">\r\n\r\n**Duplicated
prebuilt rule:**\r\n\r\n<img width=\"1469\"
alt=\"Screenshot_2024-08-23_at_13_25_43\"\r\nsrc=\"https://github.com/user-attachments/assets/03761a2b-6f53-4bab-bf4c-a71c6860802b\">\r\n\r\n###
Checklist\r\n\r\n- [
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"b144c05e8f39f28dd9551b7c62daa01cfa1d2cd5"}},{"branch":"8.15","label":"v8.15.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
@pborgonovi
Copy link
Author

pborgonovi commented Aug 29, 2024

Retest

8.16 SNAPSHOT - Passed ✅

Screen.Recording.2024-08-29.at.8.32.25.AM.mov

8.15.1 SNAPSHOT - Passed ✅

Screen.Recording.2024-08-29.at.9.39.43.AM.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.16 candidate bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules Feature:Rule Management Security Solution Detection Rule Management fixed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.1 v8.16.0
Projects
None yet
3 participants