[8.18] [Security Solution] Normalizes `filters` field before rule diff comparison (#206344) (#209531)

# Backport This will backport the following commits from `main` to `8.18`: - [[Security Solution] Normalizes `filters` field before rule diff comparison (#206344)](https://github.com/elastic/kibana/pull/206344)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
2025-04-24 09:48:58 -04:00 · 2025-02-05 01:40:33 +11:00 · 2025-02-05 01:40:33 +11:00 · f25de199b7
commit f25de199b7
parent f6e739b19e
6 changed files with 182 additions and 6 deletions
--- a/src/platform/plugins/shared/data/public/query/filter_manager/lib/map_filter.ts
+++ b/src/platform/plugins/shared/data/public/query/filter_manager/lib/map_filter.ts
@ -75,7 +75,7 @@ export function mapFilter(filter: Filter) {
  mappedFilter.meta.params = mapped.params;
  mappedFilter.meta.disabled = Boolean(mappedFilter.meta.disabled);
  mappedFilter.meta.negate = Boolean(mappedFilter.meta.negate);
-  mappedFilter.meta.alias = mappedFilter.meta.alias || null;
+  mappedFilter.meta.alias = mappedFilter.meta.alias;

  return mappedFilter;
 }
--- a/src/platform/plugins/shared/data/public/query/filter_manager/lib/mappers/map_combined.test.ts
+++ b/src/platform/plugins/shared/data/public/query/filter_manager/lib/mappers/map_combined.test.ts
@ -44,7 +44,7 @@ describe('filter manager utilities', () => {
          "params": Array [
            Object {
              "meta": Object {
-                "alias": null,
+                "alias": undefined,
                "disabled": false,
                "index": "logstash-*",
                "key": "bytes",
--- a/x-pack/solutions/security/plugins/security_solution/common/detection_engine/prebuilt_rules/diff/extract_rule_data_query.test.ts
+++ b/x-pack/solutions/security/plugins/security_solution/common/detection_engine/prebuilt_rules/diff/extract_rule_data_query.test.ts
@ -5,6 +5,7 @@
 * 2.0.
 */

+import type { Filter } from '@kbn/es-query';
 import { KqlQueryType } from '../../../api/detection_engine';
 import {
  extractRuleEqlQuery,
@ -12,6 +13,24 @@ import {
  extractRuleKqlQuery,
 } from './extract_rule_data_query';

+const mockFilter: Filter = {
+  meta: {
+    alias: null,
+    negate: false,
+    disabled: false,
+    type: 'phrase',
+    key: 'test',
+    params: {
+      query: 'value',
+    },
+  },
+  query: {
+    term: {
+      field: 'value',
+    },
+  },
+};
+
 describe('extract rule data queries', () => {
  describe('extractRuleKqlQuery', () => {
    it('extracts a trimmed version of the query field for inline query types', () => {
@ -24,6 +43,39 @@ describe('extract rule data queries', () => {
        filters: [],
      });
    });
+
+    it('normalizes filters', () => {
+      const extractedKqlQuery = extractRuleKqlQuery(
+        'event.kind:alert',
+        'kuery',
+        [mockFilter],
+        undefined
+      );
+
+      expect(extractedKqlQuery).toEqual({
+        type: KqlQueryType.inline_query,
+        query: 'event.kind:alert',
+        language: 'kuery',
+        filters: [
+          {
+            meta: {
+              negate: false,
+              disabled: false,
+              type: 'phrase',
+              key: 'test',
+              params: {
+                query: 'value',
+              },
+            },
+            query: {
+              term: {
+                field: 'value',
+              },
+            },
+          },
+        ],
+      });
+    });
  });

  describe('extractRuleEqlQuery', () => {
--- a/x-pack/solutions/security/plugins/security_solution/common/detection_engine/prebuilt_rules/diff/extract_rule_data_query.ts
+++ b/x-pack/solutions/security/plugins/security_solution/common/detection_engine/prebuilt_rules/diff/extract_rule_data_query.ts
@ -5,6 +5,7 @@
 * 2.0.
 */

+import type { Filter } from '@kbn/es-query';
 import type {
  EqlQueryLanguage,
  EsqlQueryLanguage,
@ -48,7 +49,7 @@ export const extractInlineKqlQuery = (
    type: KqlQueryType.inline_query,
    query: query?.trim() ?? '',
    language: language ?? 'kuery',
-    filters: filters ?? [],
+    filters: normalizeFilterArray(filters),
  };
 };

@ -65,7 +66,7 @@ export const extractRuleEqlQuery = (params: ExtractRuleEqlQueryParams): RuleEqlQ
  return {
    query: params.query.trim(),
    language: params.language,
-    filters: params.filters ?? [],
+    filters: normalizeFilterArray(params.filters),
    event_category_override: params.eventCategoryOverride,
    timestamp_field: params.timestampField,
    tiebreaker_field: params.tiebreakerField,
@ -81,3 +82,22 @@ export const extractRuleEsqlQuery = (
    language,
  };
 };
+
+/**
+ * Removes the null `alias` field that gets appended from the internal kibana filter util for comparison
+ * Relevant issue: https://github.com/elastic/kibana/issues/202966
+ */
+const normalizeFilterArray = (filters: RuleFilterArray | undefined): RuleFilterArray => {
+  if (!filters?.length) {
+    return [];
+  }
+  return (filters as Filter[]).map((filter) => ({
+    ...filter,
+    meta: filter.meta
+      ? {
+          ...filter.meta,
+          alias: filter.meta.alias ?? undefined,
+        }
+      : undefined,
+  }));
+};
--- a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/components/query_bar_field/query_field.tsx
+++ b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/components/query_bar_field/query_field.tsx
@ -175,8 +175,8 @@ export const QueryBarField = ({
  // if saved query fetched, reset values in queryBar input and filters to saved query's values
  useEffect(() => {
    if (resetToSavedQuery && savedQuery) {
-      const newFiledValue = savedQueryToFieldValue(savedQuery);
-      setFieldValue(newFiledValue);
+      const newFieldValue = savedQueryToFieldValue(savedQuery);
+      setFieldValue(newFieldValue);
    }
  }, [resetToSavedQuery, savedQuery, setFieldValue]);

--- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/upgrade_review_prebuilt_rules.kql_query_fields.ts
+++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/upgrade_review_prebuilt_rules.kql_query_fields.ts
@ -179,6 +179,110 @@ export default ({ getService }: FtrProviderContext): void => {
            expect(reviewResponse.stats.num_rules_with_non_solvable_conflicts).toBe(0);
          });
        });
+
+        describe('when all query versions have filters with alias fields set to null', () => {
+          it('should not show in the upgrade/_review API response', async () => {
+            // Install base prebuilt detection rule
+            await createHistoricalPrebuiltRuleAssetSavedObjects(es, [
+              createRuleAssetSavedObject({
+                rule_id: 'rule-1',
+                version: 1,
+                type: 'query',
+                query: 'query string = true',
+                language: 'kuery',
+                filters: [
+                  {
+                    meta: {
+                      negate: false,
+                      disabled: false,
+                      type: 'phrase',
+                      key: 'test',
+                      params: {
+                        query: 'value',
+                      },
+                    },
+                    query: {
+                      term: {
+                        field: 'value',
+                      },
+                    },
+                  },
+                ],
+              }),
+            ]);
+            await installPrebuiltRules(es, supertest);
+
+            // Customize a kql_query field on the installed rule
+            await updateRule(supertest, {
+              ...getPrebuiltRuleMock(),
+              rule_id: 'rule-1',
+              type: 'query',
+              query: 'query string = true',
+              language: 'kuery',
+              filters: [
+                {
+                  meta: {
+                    alias: null,
+                    negate: false,
+                    disabled: false,
+                    type: 'phrase',
+                    key: 'test',
+                    params: {
+                      query: 'value',
+                    },
+                  },
+                  query: {
+                    term: {
+                      field: 'value',
+                    },
+                  },
+                },
+              ],
+              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',
+                language: 'kuery',
+                filters: [
+                  {
+                    meta: {
+                      negate: false,
+                      disabled: false,
+                      type: 'phrase',
+                      key: 'test',
+                      params: {
+                        query: 'value',
+                      },
+                    },
+                    query: {
+                      term: {
+                        field: 'value',
+                      },
+                    },
+                  },
+                ],
+              }),
+            ];
+            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", () => {