From 5645560077728c8885a315b080ff0cd17852a086 Mon Sep 17 00:00:00 2001 From: Davis Plumlee <56367316+dplumlee@users.noreply.github.com> Date: Fri, 15 Nov 2024 10:17:37 -0500 Subject: [PATCH] [Security Solution] Fixes `required_fields` being removed after rule `PATCH` calls (#199901) **Fixes https://github.com/elastic/kibana/issues/199665** ## Summary Fixes the `required_fields` field being removed from the existing rule when not present in the rule `PATCH` API call. ### Checklist Delete any items that are not applicable to this PR. - [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] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels) - [ ] This will appear in the **Release Notes** and follow the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Georgii Gorbachev Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit f716948053c1b6f4a9f1dda27d4f5a501631b692) --- .../mergers/apply_rule_patch.test.ts | 32 +++++++++++++++++++ .../mergers/apply_rule_patch.ts | 4 ++- .../patch_rules.ts | 27 ++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.test.ts index 49592aff28f95..abf90c3f4dfc4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.test.ts @@ -453,4 +453,36 @@ describe('applyRulePatch', () => { }) ).rejects.toThrowError('new_terms_fields: Expected array, received string'); }); + + test('should retain existing required_fields when not present in rule patch body', async () => { + const rulePatch = { + name: 'new name', + } as PatchRuleRequestBody; + const existingRule = { + ...getRulesSchemaMock(), + required_fields: [ + { + name: 'event.action', + type: 'keyword', + ecs: true, + }, + ], + }; + const patchedRule = await applyRulePatch({ + rulePatch, + existingRule, + prebuiltRuleAssetClient, + }); + expect(patchedRule).toEqual( + expect.objectContaining({ + required_fields: [ + { + name: 'event.action', + type: 'keyword', + ecs: true, + }, + ], + }) + ); + }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.ts index becc68f3d0075..9f5b167322491 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.ts @@ -92,7 +92,9 @@ export const applyRulePatch = async ({ meta: rulePatch.meta ?? existingRule.meta, max_signals: rulePatch.max_signals ?? existingRule.max_signals, related_integrations: rulePatch.related_integrations ?? existingRule.related_integrations, - required_fields: addEcsToRequiredFields(rulePatch.required_fields), + required_fields: rulePatch.required_fields + ? addEcsToRequiredFields(rulePatch.required_fields) + : existingRule.required_fields, risk_score: rulePatch.risk_score ?? existingRule.risk_score, risk_score_mapping: rulePatch.risk_score_mapping ?? existingRule.risk_score_mapping, rule_name_override: rulePatch.rule_name_override ?? existingRule.rule_name_override, diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_patch/basic_license_essentials_tier/patch_rules.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_patch/basic_license_essentials_tier/patch_rules.ts index 41f207c90f319..378a1f7bb0187 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_patch/basic_license_essentials_tier/patch_rules.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_patch/basic_license_essentials_tier/patch_rules.ts @@ -284,6 +284,33 @@ export default ({ getService }: FtrProviderContext) => { ); }); }); + + it('should not change required_fields when not present in patch body', async () => { + await securitySolutionApi.createRule({ + body: getCustomQueryRuleParams({ + rule_id: 'rule-1', + required_fields: [ + { + name: 'event.action', + type: 'keyword', + }, + ], + }), + }); + + // patch a simple rule's name + const { body: patchedRule } = await securitySolutionApi + .patchRule({ body: { rule_id: 'rule-1', name: 'some other name' } }) + .expect(200); + + expect(patchedRule.required_fields).toEqual([ + { + name: 'event.action', + type: 'keyword', + ecs: true, + }, + ]); + }); }); }); };