fixes trimming bug

elastic · Dec 9, 2024 · e47b15a · e47b15a
1 parent 58b8b47
commit e47b15a
Show file tree

Hide file tree

Showing 5 changed files with 188 additions and 3 deletions.
diff --git a/...rity_solution/common/detection_engine/prebuilt_rules/diff/extract_rule_data_query.test.ts b/...rity_solution/common/detection_engine/prebuilt_rules/diff/extract_rule_data_query.test.ts
@@ -0,0 +1,61 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { KqlQueryType } from '../../../api/detection_engine';
+import {
+  extractRuleEqlQuery,
+  extractRuleEsqlQuery,
+  extractRuleKqlQuery,
+} from './extract_rule_data_query';
+
+describe('extract rule data queries', () => {
+  describe('extractRuleKqlQuery', () => {
+    it('extracts a trimmed version of the query field for inline query types', () => {
+      const extractedKqlQuery = extractRuleKqlQuery('\nevent.kind:alert\n', 'kuery', [], undefined);
+
+      expect(extractedKqlQuery).toEqual({
+        type: KqlQueryType.inline_query,
+        query: 'event.kind:alert',
+        language: 'kuery',
+        filters: [],
+      });
+    });
+  });
+
+  describe('extractRuleEqlQuery', () => {
+    it('extracts a trimmed version of the query field', () => {
+      const extractedEqlQuery = extractRuleEqlQuery({
+        query: '\n\nquery where true\n\n',
+        language: 'eql',
+        filters: [],
+        eventCategoryOverride: undefined,
+        timestampField: undefined,
+        tiebreakerField: undefined,
+      });
+
+      expect(extractedEqlQuery).toEqual({
+        query: 'query where true',
+        language: 'eql',
+        filters: [],
+        event_category_override: undefined,
+        timestamp_field: undefined,
+        tiebreaker_field: undefined,
+      });
+    });
+  });
+
+  describe('extractRuleEsqlQuery', () => {
+    it('extracts a trimmed version of the query field', () => {
+      const extractedEsqlQuery = extractRuleEsqlQuery('\nFROM * where true\t\n', 'esql');
+
+      expect(extractedEsqlQuery).toEqual({
+        query: 'FROM * where true',
+        language: 'esql',
+      });
+    });
+  });
+});
diff --git a/.../security_solution/common/detection_engine/prebuilt_rules/diff/extract_rule_data_query.ts b/.../security_solution/common/detection_engine/prebuilt_rules/diff/extract_rule_data_query.ts
@@ -46,7 +46,7 @@ export const extractInlineKqlQuery = (
 ): InlineKqlQuery => {
   return {
     type: KqlQueryType.inline_query,
-    query: query ?? '',
+    query: query?.trim() ?? '',
     language: language ?? 'kuery',
     filters: filters ?? [],
   };
@@ -63,7 +63,7 @@ interface ExtractRuleEqlQueryParams {
 
 export const extractRuleEqlQuery = (params: ExtractRuleEqlQueryParams): RuleEqlQuery => {
   return {
-    query: params.query,
+    query: params.query.trim(),
     language: params.language,
     filters: params.filters ?? [],
     event_category_override: params.eventCategoryOverride,
@@ -77,7 +77,7 @@ export const extractRuleEsqlQuery = (
   language: EsqlQueryLanguage
 ): RuleEsqlQuery => {
   return {
-    query,
+    query: query.trim(),
     language,
   };
 };
diff --git a/.../management/trial_license_complete_tier/upgrade_review_prebuilt_rules.eql_query_fields.ts b/.../management/trial_license_complete_tier/upgrade_review_prebuilt_rules.eql_query_fields.ts
@@ -80,6 +80,46 @@ export default ({ getService }: FtrProviderContext): void => {
           expect(reviewResponse.stats.num_rules_with_conflicts).toBe(0);
           expect(reviewResponse.stats.num_rules_with_non_solvable_conflicts).toBe(0);
         });
+
+        it('should trim all whitespace before version comparison', async () => {
+          // Install base prebuilt detection rule
+          await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
+          await installPrebuiltRules(es, supertest);
+
+          // Customize an eql_query field on the installed rule
+          await updateRule(supertest, {
+            ...getPrebuiltRuleMock(),
+            rule_id: 'rule-1',
+            type: 'eql',
+            query: '\nquery where true\n',
+            language: 'eql',
+            filters: [],
+          } as RuleUpdateProps);
+
+          // Add a v2 rule asset to make the upgrade possible, do NOT update the related eql_query field, and create the new rule assets
+          const updatedRuleAssetSavedObjects = [
+            createRuleAssetSavedObject({
+              rule_id: 'rule-1',
+              version: 2,
+              type: 'eql',
+              query: '\nquery where true',
+              language: 'eql',
+              filters: [],
+            }),
+          ];
+          await createHistoricalPrebuiltRuleAssetSavedObjects(es, updatedRuleAssetSavedObjects);
+
+          // Call the upgrade review prebuilt rules endpoint and check that there is 1 rule eligible for update but eql_query field is NOT returned
+          const reviewResponse = await reviewPrebuiltRulesToUpgrade(supertest);
+          const fieldDiffObject = reviewResponse.rules[0].diff.fields as AllFieldsDiff;
+          expect(fieldDiffObject.eql_query).toBeUndefined();
+
+          expect(reviewResponse.rules[0].diff.num_fields_with_updates).toBe(1); // `version` is considered an updated field
+          expect(reviewResponse.rules[0].diff.num_fields_with_conflicts).toBe(0);
+          expect(reviewResponse.rules[0].diff.num_fields_with_non_solvable_conflicts).toBe(0);
+          expect(reviewResponse.stats.num_rules_with_conflicts).toBe(0);
+          expect(reviewResponse.stats.num_rules_with_non_solvable_conflicts).toBe(0);
+        });
       });
 
       describe("when rule field doesn't have an update but has a custom value - scenario ABA", () => {

diff --git a/...management/trial_license_complete_tier/upgrade_review_prebuilt_rules.esql_query_fields.ts b/...management/trial_license_complete_tier/upgrade_review_prebuilt_rules.esql_query_fields.ts
@@ -78,6 +78,44 @@ export default ({ getService }: FtrProviderContext): void => {
           expect(reviewResponse.stats.num_rules_with_conflicts).toBe(0);
           expect(reviewResponse.stats.num_rules_with_non_solvable_conflicts).toBe(0);
         });
+
+        it('should trim all whitespace before version comparison', async () => {
+          // Install base prebuilt detection rule
+          await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
+          await installPrebuiltRules(es, supertest);
+
+          // Customize an esql_query field on the installed rule
+          await updateRule(supertest, {
+            ...getPrebuiltRuleMock(),
+            rule_id: 'rule-1',
+            type: 'esql',
+            query: '\tFROM query WHERE true\t',
+            language: 'esql',
+          } as RuleUpdateProps);
+
+          // Add a v2 rule asset to make the upgrade possible, do NOT update the related esql_query field, and create the new rule assets
+          const updatedRuleAssetSavedObjects = [
+            createRuleAssetSavedObject({
+              rule_id: 'rule-1',
+              version: 2,
+              type: 'esql',
+              query: '\n\nFROM query WHERE true\n\n',
+              language: 'esql',
+            }),
+          ];
+          await createHistoricalPrebuiltRuleAssetSavedObjects(es, updatedRuleAssetSavedObjects);
+
+          // Call the upgrade review prebuilt rules endpoint and check that there is 1 rule eligible for update but esql_query field is NOT returned
+          const reviewResponse = await reviewPrebuiltRulesToUpgrade(supertest);
+          const fieldDiffObject = reviewResponse.rules[0].diff.fields as AllFieldsDiff;
+          expect(fieldDiffObject.esql_query).toBeUndefined();
+
+          expect(reviewResponse.rules[0].diff.num_fields_with_updates).toBe(1); // `version` is considered an updated field
+          expect(reviewResponse.rules[0].diff.num_fields_with_conflicts).toBe(0);
+          expect(reviewResponse.rules[0].diff.num_fields_with_non_solvable_conflicts).toBe(0);
+          expect(reviewResponse.stats.num_rules_with_conflicts).toBe(0);
+          expect(reviewResponse.stats.num_rules_with_non_solvable_conflicts).toBe(0);
+        });
       });
 
       describe("when rule field doesn't have an update but has a custom value - scenario ABA", () => {

diff --git a/.../management/trial_license_complete_tier/upgrade_review_prebuilt_rules.kql_query_fields.ts b/.../management/trial_license_complete_tier/upgrade_review_prebuilt_rules.kql_query_fields.ts
@@ -133,6 +133,52 @@ export default ({ getService }: FtrProviderContext): void => {
             expect(reviewResponse.stats.num_rules_with_non_solvable_conflicts).toBe(0);
           });
         });
+
+        describe('when all query versions have different surrounding whitespace', () => {
+          it('should not show in the upgrade/_review API response', async () => {
+            // Install base prebuilt detection rule
+            await createHistoricalPrebuiltRuleAssetSavedObjects(
+              es,
+              getQueryRuleAssetSavedObjects()
+            );
+            await installPrebuiltRules(es, supertest);
+
+            // Customize a kql_query field on the installed rule
+            await updateRule(supertest, {
+              ...getPrebuiltRuleMock(),
+              rule_id: 'rule-1',
+              type: 'query',
+              query: '\nquery string = true',
+              language: 'kuery',
+              filters: [],
+              saved_id: undefined,
+            } as RuleUpdateProps);
+
+            // Add a v2 rule asset to make the upgrade possible, do NOT update the related kql_query field, and create the new rule assets
+            const updatedRuleAssetSavedObjects = [
+              createRuleAssetSavedObject({
+                rule_id: 'rule-1',
+                version: 2,
+                type: 'query',
+                query: 'query string = true\n',
+                language: 'kuery',
+                filters: [],
+              }),
+            ];
+            await createHistoricalPrebuiltRuleAssetSavedObjects(es, updatedRuleAssetSavedObjects);
+
+            // Call the upgrade review prebuilt rules endpoint and check that there is 1 rule eligible for update but kql_query field is NOT returned
+            const reviewResponse = await reviewPrebuiltRulesToUpgrade(supertest);
+            const fieldDiffObject = reviewResponse.rules[0].diff.fields as AllFieldsDiff;
+            expect(fieldDiffObject.kql_query).toBeUndefined();
+
+            expect(reviewResponse.rules[0].diff.num_fields_with_updates).toBe(1); // `version` is considered an updated field
+            expect(reviewResponse.rules[0].diff.num_fields_with_conflicts).toBe(0);
+            expect(reviewResponse.rules[0].diff.num_fields_with_non_solvable_conflicts).toBe(0);
+            expect(reviewResponse.stats.num_rules_with_conflicts).toBe(0);
+            expect(reviewResponse.stats.num_rules_with_non_solvable_conflicts).toBe(0);
+          });
+        });
       });
 
       describe("when rule field doesn't have an update but has a custom value - scenario ABA", () => {