[Security Solution] Fixes required_fields being removed after rule …

…`PATCH` calls (elastic#199901) **Fixes elastic#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 <[email protected]> Co-authored-by: kibanamachine <[email protected]>
tiansivive · Nov 15, 2024 · f716948 · f716948
1 parent b591362
commit f716948
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 1 deletion.
diff --git a/...tion_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.test.ts b/...tion_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/...detection_engine/rule_management/logic/detection_rules_client/mergers/apply_rule_patch.ts b/...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/...ections_response/rules_management/rule_patch/basic_license_essentials_tier/patch_rules.ts b/...ections_response/rules_management/rule_patch/basic_license_essentials_tier/patch_rules.ts
@@ -285,6 +285,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,
+          },
+        ]);
+      });
     });
   });
 };