[Security Solution] "Automatically overwrite" option when importing rules does not completely overwrite existing rule #93342
Assignees
Labels
8.13 candidate
bug
Fixes for quality problems that affect the customer experience
Feature:Rule Import/Export
Security Solution Detection Rule Import & Export workflow
Feature:Rule Management
Security Solution Detection Rule Management area
fixed
impact:medium
Addressing this issue will have a medium level of impact on the quality/strength of our product.
QA:Validated
Issue has been validated by QA
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.13.0
v8.14.0
Comments
marshallmain
added
bug
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
marshallmain
added
impact:medium
MadameSheema
added
the
Team: SecuritySolution
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
MadameSheema
added
the
Feature:Detection Rules
peluja1012
added
Team:Detection Rule Management
banderror
added
Feature:Rule Import/Export
2 tasks
maximpn
added a commit
that referenced
this issue
Feb 19, 2024
…orting rules (#176166) **Fixes: #93342 **Fixes: #118166 ## Summary This PR fixes not complete existing rule overwrite when importing rules. ## Details When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule. The fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause. ### Checklist - [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 - [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this issue
Feb 19, 2024
…orting rules (elastic#176166) **Fixes: elastic#93342 **Fixes: elastic#118166 ## Summary This PR fixes not complete existing rule overwrite when importing rules. ## Details When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule. The fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause. ### Checklist - [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 - [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167)) (cherry picked from commit 53aaab4)
|
@vgomez-el Could you please validate the bug is fixed by #176421? It has been backported to 8.12 and 8.13. It should be released in 8.12.3 and 8.13.1. |
kibanamachine
referenced
this issue
Feb 19, 2024
…hen importing rules (#176166) (#177196) # Backport This will backport the following commits from `main` to `8.13`: - [[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)](#176166) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maxim Palenov","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-02-19T13:35:32Z","message":"[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)\n\n**Fixes: https://github.com/elastic/kibana/issues/93342**\r\n**Fixes: https://github.com/elastic/kibana/issues/118166**\r\n\r\n## Summary\r\n\r\nThis PR fixes not complete existing rule overwrite when importing rules.\r\n\r\n## Details\r\n\r\nWhen importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule.\r\n\r\nThe fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause.\r\n\r\n### Checklist\r\n\r\n- [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\r\n- [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))","sha":"53aaab47322fd15ad232d71a1749fd2df8a5dde4","branchLabelMapping":{"^v8.14.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:Rule Import/Export","v8.13.0","v8.12.2","v8.14.0"],"title":"[Security Solution] Fix not complete existing rule overwrite when importing rules","number":176166,"url":"https://github.com/elastic/kibana/pull/176166","mergeCommit":{"message":"[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)\n\n**Fixes: https://github.com/elastic/kibana/issues/93342**\r\n**Fixes: https://github.com/elastic/kibana/issues/118166**\r\n\r\n## Summary\r\n\r\nThis PR fixes not complete existing rule overwrite when importing rules.\r\n\r\n## Details\r\n\r\nWhen importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule.\r\n\r\nThe fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause.\r\n\r\n### Checklist\r\n\r\n- [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\r\n- [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))","sha":"53aaab47322fd15ad232d71a1749fd2df8a5dde4"}},"sourceBranch":"main","suggestedTargetBranches":["8.13","8.12"],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.12","label":"v8.12.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/176166","number":176166,"mergeCommit":{"message":"[Security Solution] Fix not complete existing rule overwrite when importing rules (#176166)\n\n**Fixes: https://github.com/elastic/kibana/issues/93342**\r\n**Fixes: https://github.com/elastic/kibana/issues/118166**\r\n\r\n## Summary\r\n\r\nThis PR fixes not complete existing rule overwrite when importing rules.\r\n\r\n## Details\r\n\r\nWhen importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule.\r\n\r\nThe fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause.\r\n\r\n### Checklist\r\n\r\n- [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\r\n- [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))","sha":"53aaab47322fd15ad232d71a1749fd2df8a5dde4"}}]}] BACKPORT--> Co-authored-by: Maxim Palenov <[email protected]>
maximpn
added a commit
to maximpn/kibana
that referenced
this issue
Feb 20, 2024
…orting rules (elastic#176166) **Fixes: elastic#93342 **Fixes: elastic#118166 ## Summary This PR fixes not complete existing rule overwrite when importing rules. ## Details When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule. The fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause. ### Checklist - [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 - [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167)) (cherry picked from commit 53aaab4) # Conflicts: # x-pack/test/detection_engine_api_integration/basic/tests/export_rules.ts # x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts # x-pack/test/detection_engine_api_integration/basic/tests/import_rules_with_overwrite.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_connectors.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_export_rules.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules_with_overwrite.ts # x-pack/test/detection_engine_api_integration/utils/get_rules_as_ndjson.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/alerts/assignments/index.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_import_export/trial_license_complete_tier/export_rules.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_import_export/trial_license_complete_tier/import_rules_ess.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_import_export/trial_license_complete_tier/index.ts # x-pack/test/security_solution_api_integration/test_suites/detections_response/utils/rules/index.ts
|
Bug is fixed and validated in 8.13 BC3. Thanks @maximpn for the fix and @marshallmain for the report! REC-20240301141555.mp4 |
fkanout
pushed a commit
to fkanout/kibana
that referenced
this issue
Mar 4, 2024
…orting rules (elastic#176166) **Fixes: elastic#93342 **Fixes: elastic#118166 ## Summary This PR fixes not complete existing rule overwrite when importing rules. ## Details When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from saved_query to query we would provide a new rule that doesn't have a saved_id but since saved_id was defined on the old saved_query rule it will be included in the new query rule. The fix simply swaps out the `patchRules()` for `updateRules()`. Patching rules preserves previous field values if an incoming update doesn't have such fields while updating doesn't do that. The diff in `import_rules_utils.test.ts` looks bigger due to removing unnecessary `else` clause. ### Checklist - [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 - [x] Ran successfully in Flaky test runner ([basic/essentials license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5166) and [trial/complete tier license FTR tests](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5167))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Kibana version:
8.0.0
Describe the bug:
When importing a rule and attempting to overwrite an existing rule, if the new rule does not define a field that the existing rule did define then the newly imported rule will include the field from the existing rule. This can cause issues if we want to overwrite a rule with a rule of a different type, e.g. going from
saved_querytoquerywe would provide a new rule that doesn't have asaved_idbut sincesaved_idwas defined on the oldsaved_queryrule it will be included in the newqueryrule.Steps to reproduce:
timestamp_override.ndjsonfile.ndjsonfile and check the "Automatically overwrite existing rule" option in the UI.ndjsonfileExpected behavior:
If the "Automatically overwrite" option is checked then fields not defined in the rule to import should not be in newly created rule SO. Can we simply swap out the
patchhere forupdate, or are there other reasons we need to use patch to preserve existing fields?The text was updated successfully, but these errors were encountered: