From 4e2724cc76b08732fd860aa4017e1a43a88a5a95 Mon Sep 17 00:00:00 2001 From: Davis Plumlee Date: Fri, 10 Jan 2025 13:57:36 -0500 Subject: [PATCH 1/2] adds normalization in diffable rule conversion --- .../diff/extract_rule_data_query.test.ts | 52 +++++++++++++++++++ .../diff/extract_rule_data_query.ts | 22 +++++++- .../query_bar_field/query_field.tsx | 4 +- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git 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 index 03dc7ecbe5331..7ba6eaceb35b2 100644 --- 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', () => { diff --git 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 index 04460dd0cad75..5e05c2f5c8078 100644 --- 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,20 @@ 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 && filters.length > 0) { + return (filters as Filter[]).map((filter) => { + if (filter.meta.alias == null) { + return { ...filter, meta: { ...filter.meta, alias: undefined } }; + } + return filter; + }); + } else { + return []; + } +}; diff --git 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 index 44dad72b09c49..5694b7b432c85 100644 --- 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]); From 1697d6598d4c3e5c4ad96fbfd063c63e946e5a0f Mon Sep 17 00:00:00 2001 From: Davis Plumlee Date: Fri, 10 Jan 2025 18:00:06 -0500 Subject: [PATCH 2/2] updates tests --- .../diff/extract_rule_data_query.ts | 2 +- ..._review_prebuilt_rules.kql_query_fields.ts | 104 ++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git 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 index 5e05c2f5c8078..4cf7b8bc5abc1 100644 --- 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 @@ -90,7 +90,7 @@ export const extractRuleEsqlQuery = ( const normalizeFilterArray = (filters: RuleFilterArray | undefined): RuleFilterArray => { if (filters && filters.length > 0) { return (filters as Filter[]).map((filter) => { - if (filter.meta.alias == null) { + if (filter?.meta.alias == null) { return { ...filter, meta: { ...filter.meta, alias: undefined } }; } return filter; diff --git 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 index d00d2d842f2ba..50bdb83744f04 100644 --- 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", () => {