[8.18] [Security Solution] Adds normalization for filter meta field diff (#210191) (#212318)

# Backport

This will backport the following commits from `main` to `8.18`:
- [[Security Solution] Adds normalization for filter `meta` field diff
(#210191)](https://github.com/elastic/kibana/pull/210191)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Davis
Plumlee","email":"56367316+dplumlee@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-02-25T00:19:55Z","message":"[Security
Solution] Adds normalization for filter `meta` field diff
(#210191)\n\n**Fixes:
https://github.com/elastic/kibana/issues/206527**\n**Partially
addresses: https://github.com/elastic/kibana/issues/209518**\n\n##
Summary\n\nAdds a normalization to the `filters` field in the rule
diffing\ncalculation that omits all filter fields other than the `query`
field\nand the `negate` and `disabled` fields within the `meta` object.
This\nmakes our diffing logic much more robust and resilient as we
only\ncompare data in the rule fields that have an impact on the query
itself\nand not the fields that relate to UI implementation (`alias`,
`key`,\netc).\n\n### To test\n\n- Open a prebuilt rule with `filters` in
the non-customized rule\nparameters (e.g. `PowerShell Script with
Discovery Capabilities`)\n- Edit the rule and save without editing\n-
The rule should remain unmodified even though more fields have
been\nadded to the rule's `filters` field\n\nUnless the user adds or
deletes a filter on the rule, the rule should\nonly be marked as
customized under 3 circumstances:\n\n- The user negates the filter (adds
NOT to the beginning of the filter)\n- The user disables the filter\n-
The user changes the filter query\n\nAll other scenarios (such as adding
a custom name for the filter) should\nnot change the rule's customized
status\n\n### Checklist\n\nCheck the PR satisfies following conditions.
\n\nReviewers should verify this PR satisfies this list as well.\n\n-
[x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"3f3c8c8a4898217c3d3c9314e50e6eb46ec4b9e0","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Prebuilt Detection
Rules","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[Security
Solution] Adds normalization for filter `meta` field
diff","number":210191,"url":"https://github.com/elastic/kibana/pull/210191","mergeCommit":{"message":"[Security
Solution] Adds normalization for filter `meta` field diff
(#210191)\n\n**Fixes:
https://github.com/elastic/kibana/issues/206527**\n**Partially
addresses: https://github.com/elastic/kibana/issues/209518**\n\n##
Summary\n\nAdds a normalization to the `filters` field in the rule
diffing\ncalculation that omits all filter fields other than the `query`
field\nand the `negate` and `disabled` fields within the `meta` object.
This\nmakes our diffing logic much more robust and resilient as we
only\ncompare data in the rule fields that have an impact on the query
itself\nand not the fields that relate to UI implementation (`alias`,
`key`,\netc).\n\n### To test\n\n- Open a prebuilt rule with `filters` in
the non-customized rule\nparameters (e.g. `PowerShell Script with
Discovery Capabilities`)\n- Edit the rule and save without editing\n-
The rule should remain unmodified even though more fields have
been\nadded to the rule's `filters` field\n\nUnless the user adds or
deletes a filter on the rule, the rule should\nonly be marked as
customized under 3 circumstances:\n\n- The user negates the filter (adds
NOT to the beginning of the filter)\n- The user disables the filter\n-
The user changes the filter query\n\nAll other scenarios (such as adding
a custom name for the filter) should\nnot change the rule's customized
status\n\n### Checklist\n\nCheck the PR satisfies following conditions.
\n\nReviewers should verify this PR satisfies this list as well.\n\n-
[x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"3f3c8c8a4898217c3d3c9314e50e6eb46ec4b9e0"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/210191","number":210191,"mergeCommit":{"message":"[Security
Solution] Adds normalization for filter `meta` field diff
(#210191)\n\n**Fixes:
https://github.com/elastic/kibana/issues/206527**\n**Partially
addresses: https://github.com/elastic/kibana/issues/209518**\n\n##
Summary\n\nAdds a normalization to the `filters` field in the rule
diffing\ncalculation that omits all filter fields other than the `query`
field\nand the `negate` and `disabled` fields within the `meta` object.
This\nmakes our diffing logic much more robust and resilient as we
only\ncompare data in the rule fields that have an impact on the query
itself\nand not the fields that relate to UI implementation (`alias`,
`key`,\netc).\n\n### To test\n\n- Open a prebuilt rule with `filters` in
the non-customized rule\nparameters (e.g. `PowerShell Script with
Discovery Capabilities`)\n- Edit the rule and save without editing\n-
The rule should remain unmodified even though more fields have
been\nadded to the rule's `filters` field\n\nUnless the user adds or
deletes a filter on the rule, the rule should\nonly be marked as
customized under 3 circumstances:\n\n- The user negates the filter (adds
NOT to the beginning of the filter)\n- The user disables the filter\n-
The user changes the filter query\n\nAll other scenarios (such as adding
a custom name for the filter) should\nnot change the rule's customized
status\n\n### Checklist\n\nCheck the PR satisfies following conditions.
\n\nReviewers should verify this PR satisfies this list as well.\n\n-
[x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"3f3c8c8a4898217c3d3c9314e50e6eb46ec4b9e0"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
This commit is contained in:
Kibana Machine 2025-02-25 13:08:26 +11:00 committed by GitHub
parent 2deb11215f
commit 17a2932b9b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 108 additions and 31 deletions

1
.github/CODEOWNERS vendored
View file

@ -2382,6 +2382,7 @@ x-pack/test/security_solution_cypress/cypress/tasks/expandable_flyout @elastic/
/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/prebuilt_rules @elastic/security-detection-rule-management
/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/rule_management @elastic/security-detection-rule-management
/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/rule_monitoring @elastic/security-detection-rule-management
/x-pack/solutions/security/plugins/security_solution/common/detection_engine/prebuilt_rules @elastic/security-detection-rule-management
/x-pack/solutions/security/plugins/security_solution/common/detection_engine/rule_management @elastic/security-detection-rule-management
/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management @elastic/security-detection-rule-management

View file

@ -5,7 +5,7 @@
* 2.0.
*/
import type { Filter } from '@kbn/es-query';
import { FilterStateStore, type Filter } from '@kbn/es-query';
import { KqlQueryType } from '../../../api/detection_engine';
import {
extractRuleEqlQuery,
@ -29,6 +29,9 @@ const mockFilter: Filter = {
field: 'value',
},
},
$state: {
store: FilterStateStore.APP_STATE,
},
};
describe('extract rule data queries', () => {
@ -44,36 +47,107 @@ describe('extract rule data queries', () => {
});
});
it('normalizes filters', () => {
const extractedKqlQuery = extractRuleKqlQuery(
'event.kind:alert',
'kuery',
[mockFilter],
undefined
);
describe('filters normalization', () => {
it('normalizes filters[].query when all fields present', () => {
const extractedKqlQuery = extractRuleKqlQuery(
'some:query',
'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',
expect(extractedKqlQuery).toMatchObject({
filters: [
{
query: {
term: {
field: 'value',
},
},
},
query: {
term: {
field: 'value',
},
],
});
});
it('normalizes filters[].query when query object is missing', () => {
const extractedKqlQuery = extractRuleKqlQuery(
'some:query',
'kuery',
[{ ...mockFilter, query: undefined }],
undefined
);
expect(extractedKqlQuery).not.toMatchObject({
filters: [
{
query: expect.anything(),
},
],
});
});
it.each([
{
caseName: 'when all fields present',
filter: mockFilter,
expectedFilterMeta: {
negate: false,
disabled: false,
},
],
},
{
caseName: 'when disabled field is missing',
filter: { ...mockFilter, meta: { ...mockFilter.meta, disabled: undefined } },
expectedFilterMeta: {
negate: false,
disabled: false,
},
},
{
caseName: 'when negate field is missing',
filter: { ...mockFilter, meta: { ...mockFilter.meta, negate: undefined } },
expectedFilterMeta: {
disabled: false,
},
},
{
caseName: 'when query object is missing',
filter: { ...mockFilter, query: undefined },
expectedFilterMeta: {
negate: false,
disabled: false,
},
},
])('normalizes filters[].meta $caseName', ({ filter, expectedFilterMeta }) => {
const extractedKqlQuery = extractRuleKqlQuery('some:query', 'kuery', [filter], undefined);
expect(extractedKqlQuery).toMatchObject({
filters: [
{
meta: expectedFilterMeta,
},
],
});
});
it('normalizes filters[].meta when query object is missing', () => {
const extractedKqlQuery = extractRuleKqlQuery(
'some:query',
'kuery',
[{ ...mockFilter, query: undefined }],
undefined
);
expect(extractedKqlQuery).toMatchObject({
filters: [
{
meta: {
negate: false,
disabled: false,
},
},
],
});
});
});
});

View file

@ -84,19 +84,21 @@ export const extractRuleEsqlQuery = (
};
/**
* 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
* Normalizes filter properties to only include ones relevant to the query itself
* Relevant issues:
* - https://github.com/elastic/kibana/issues/202966
* - https://github.com/elastic/kibana/issues/206527
*/
const normalizeFilterArray = (filters: RuleFilterArray | undefined): RuleFilterArray => {
if (!filters?.length) {
return [];
}
return (filters as Filter[]).map((filter) => ({
...filter,
query: filter.query,
meta: filter.meta
? {
...filter.meta,
alias: filter.meta.alias ?? undefined,
negate: filter.meta.negate,
disabled: filter.meta.disabled !== undefined ? filter.meta.disabled : false,
}
: undefined,
}));