[Security Solution] Adds normalization for query fields before diff algorithm comparison (#203482)

## Summary

Fixes https://github.com/elastic/kibana/issues/203151

Adds a normalization for the `kql_query`, `eql_query`, and `esql_query`
fields that trims the whitespace from the beginning and end of query
strings for a more robust comparison in the diff algorithms. Since
whitespace before or after the query string is purely a formatting
choice and doesn't impact the query itself, we discard the excess
whitespace characters before the direct string comparison.

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [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
This commit is contained in:
Davis Plumlee 2024-12-12 22:58:50 -05:00 committed by GitHub
parent abfd590d4d
commit 0294838a95
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 224 additions and 4 deletions

View file

@ -118,7 +118,7 @@ const extractDiffableCommonFields = (
version: rule.version,
// Main domain fields
name: rule.name,
name: rule.name.trim(),
tags: rule.tags ?? [],
description: rule.description,
severity: rule.severity,

View file

@ -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',
});
});
});
});

View file

@ -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,
};
};

View file

@ -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", () => {

View file

@ -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", () => {

View file

@ -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", () => {

View file

@ -69,6 +69,41 @@ 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 a single line string field on the installed rule
await patchRule(supertest, log, {
rule_id: 'rule-1',
name: 'A\n',
});
// Increment the version of the installed rule, do NOT update the related single line string field, and create the new rule assets
const updatedRuleAssetSavedObjects = [
createRuleAssetSavedObject({
rule_id: 'rule-1',
name: '\nA',
version: 2,
}),
];
await createHistoricalPrebuiltRuleAssetSavedObjects(es, updatedRuleAssetSavedObjects);
// Call the upgrade review prebuilt rules endpoint and check that there is 1 rule eligible for update
// but single line string field (name) is NOT returned
const reviewResponse = await reviewPrebuiltRulesToUpgrade(supertest);
expect(reviewResponse.rules[0].diff.fields.name).toBeUndefined();
expect(reviewResponse.rules[0].diff.num_fields_with_updates).toBe(1);
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_to_upgrade_total).toBe(1);
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", () => {