From 6023b481c027b9b9570da4bc4057fd7fb4479b68 Mon Sep 17 00:00:00 2001 From: Davis Plumlee <56367316+dplumlee@users.noreply.github.com> Date: Fri, 9 May 2025 12:20:27 -0400 Subject: [PATCH 1/5] [Security Solution] Fixes exceptions list and actions being overwritten when using legacy prebuilt rule upgrade (#218519) ## Summary Fixes https://github.com/elastic/kibana/issues/218000 Fixes issues that caused the `exceptions_list` and `actions` fields to get overwritten when the legacy prebuilt rule upgrade methods (`api/detection_engine/rules/prepackaged`) were used. ### Testing 1. Install an outdated rules package 1. Install all rules from the package 1. Add actions and exceptions to the installed rules (actions can be added using bulk edit) 1. Install the latest available prebuilt rules package 1. Call the legacy API to upgrade installed rules to the latest versions: `/api/detection_engine/rules/prepackaged` 1. Observe all exceptions lists and actions are maintained through upgrade process ### 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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed --------- Co-authored-by: Elastic Machine (cherry picked from commit 0eeb5ffcff27a04b2f3d509a582043b17395e2a3) --- .../logic/get_rules_to_update.test.ts | 310 +----------------- .../logic/get_rules_to_update.ts | 36 +- ...rules_client.upgrade_prebuilt_rule.test.ts | 81 +++++ .../methods/upgrade_prebuilt_rule.ts | 11 +- .../detection_rules_client/utils.test.ts | 116 +++++++ .../logic/detection_rules_client/utils.ts | 32 ++ ...prebuilt_rules_with_historical_versions.ts | 105 ++++++ 7 files changed, 347 insertions(+), 344 deletions(-) create mode 100644 x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/utils.test.ts diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/get_rules_to_update.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/get_rules_to_update.test.ts index 89d59ec8f77e3..9cedfc101a932 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/get_rules_to_update.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/get_rules_to_update.test.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { filterInstalledRules, getRulesToUpdate, mergeExceptionLists } from './get_rules_to_update'; +import { filterInstalledRules, getRulesToUpdate } from './get_rules_to_update'; import { getRuleMock } from '../../routes/__mocks__/request_responses'; import { getPrebuiltRuleMock } from '../mocks'; import { getQueryRuleParams } from '../../rule_schema/mocks'; @@ -111,207 +111,6 @@ describe('get_rules_to_update', () => { ); expect(update).toEqual([ruleAsset1, ruleAsset2]); }); - - test('should add back an exception_list if it was removed by the end user on an immutable rule during an upgrade', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = []; - - const [update] = getRulesToUpdate([ruleAsset1], rulesToMap([installedRule1])); - expect(update.exceptions_list).toEqual(ruleAsset1.exceptions_list); - }); - - test('should not remove an additional exception_list if an additional one was added by the end user on an immutable rule during an upgrade', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = [ - { - id: 'second_exception_list', - list_id: 'some-other-id', - namespace_type: 'single', - type: 'detection', - }, - ]; - - const [update] = getRulesToUpdate([ruleAsset1], rulesToMap([installedRule1])); - expect(update.exceptions_list).toEqual([ - ...ruleAsset1.exceptions_list, - ...installedRule1.params.exceptionsList, - ]); - }); - - test('should not remove an existing exception_list if they are the same between the current installed one and the upgraded one', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - - const [update] = getRulesToUpdate([ruleAsset1], rulesToMap([installedRule1])); - expect(update.exceptions_list).toEqual(ruleAsset1.exceptions_list); - }); - - test('should not remove an existing exception_list if the rule has an empty exceptions list', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = []; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - - const [update] = getRulesToUpdate([ruleAsset1], rulesToMap([installedRule1])); - expect(update.exceptions_list).toEqual(installedRule1.params.exceptionsList); - }); - - test('should not remove an existing exception_list if the rule has an empty exceptions list for multiple rules', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = []; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const ruleAsset2 = getPrebuiltRuleMock(); - ruleAsset2.exceptions_list = []; - ruleAsset2.rule_id = 'rule-2'; - ruleAsset2.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - const installedRule2 = getRuleMock(getQueryRuleParams()); - installedRule2.params.ruleId = 'rule-2'; - installedRule2.params.version = 1; - installedRule2.params.exceptionsList = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - - const [update1, update2] = getRulesToUpdate( - [ruleAsset1, ruleAsset2], - rulesToMap([installedRule1, installedRule2]) - ); - expect(update1.exceptions_list).toEqual(installedRule1.params.exceptionsList); - expect(update2.exceptions_list).toEqual(installedRule2.params.exceptionsList); - }); - - test('should not remove an existing exception_list if the rule has an empty exceptions list for mixed rules', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = []; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const ruleAsset2 = getPrebuiltRuleMock(); - ruleAsset2.exceptions_list = []; - ruleAsset2.rule_id = 'rule-2'; - ruleAsset2.version = 2; - ruleAsset2.exceptions_list = [ - { - id: 'second_list', - list_id: 'second_list', - namespace_type: 'single', - type: 'detection', - }, - ]; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - - const installedRule2 = getRuleMock(getQueryRuleParams()); - installedRule2.params.ruleId = 'rule-2'; - installedRule2.params.version = 1; - installedRule2.params.exceptionsList = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - - const [update1, update2] = getRulesToUpdate( - [ruleAsset1, ruleAsset2], - rulesToMap([installedRule1, installedRule2]) - ); - expect(update1.exceptions_list).toEqual(installedRule1.params.exceptionsList); - expect(update2.exceptions_list).toEqual([ - ...ruleAsset2.exceptions_list, - ...installedRule2.params.exceptionsList, - ]); - }); }); describe('filterInstalledRules', () => { @@ -365,110 +164,3 @@ describe('filterInstalledRules', () => { expect(shouldUpdate).toEqual(true); }); }); - -describe('mergeExceptionLists', () => { - test('should add back an exception_list if it was removed by the end user on an immutable rule during an upgrade', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = []; - - const update = mergeExceptionLists(ruleAsset1, rulesToMap([installedRule1])); - expect(update.exceptions_list).toEqual(ruleAsset1.exceptions_list); - }); - - test('should not remove an additional exception_list if an additional one was added by the end user on an immutable rule during an upgrade', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = [ - { - id: 'second_exception_list', - list_id: 'some-other-id', - namespace_type: 'single', - type: 'detection', - }, - ]; - - const update = mergeExceptionLists(ruleAsset1, rulesToMap([installedRule1])); - expect(update.exceptions_list).toEqual([ - ...ruleAsset1.exceptions_list, - ...installedRule1.params.exceptionsList, - ]); - }); - - test('should not remove an existing exception_list if they are the same between the current installed one and the upgraded one', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - - const update = mergeExceptionLists(ruleAsset1, rulesToMap([installedRule1])); - expect(update.exceptions_list).toEqual(ruleAsset1.exceptions_list); - }); - - test('should not remove an existing exception_list if the rule has an empty exceptions list', () => { - const ruleAsset1 = getPrebuiltRuleMock(); - ruleAsset1.exceptions_list = []; - ruleAsset1.rule_id = 'rule-1'; - ruleAsset1.version = 2; - - const installedRule1 = getRuleMock(getQueryRuleParams()); - installedRule1.params.ruleId = 'rule-1'; - installedRule1.params.version = 1; - installedRule1.params.exceptionsList = [ - { - id: 'endpoint_list', - list_id: 'endpoint_list', - namespace_type: 'agnostic', - type: 'endpoint', - }, - ]; - - const update = mergeExceptionLists(ruleAsset1, rulesToMap([installedRule1])); - expect(update.exceptions_list).toEqual(installedRule1.params.exceptionsList); - }); -}); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/get_rules_to_update.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/get_rules_to_update.ts index e25ca4f1348e1..dac43534e7420 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/get_rules_to_update.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/get_rules_to_update.ts @@ -19,9 +19,9 @@ export const getRulesToUpdate = ( latestPrebuiltRules: PrebuiltRuleAsset[], installedRules: Map ) => { - return latestPrebuiltRules - .filter((latestRule) => filterInstalledRules(latestRule, installedRules)) - .map((latestRule) => mergeExceptionLists(latestRule, installedRules)); + return latestPrebuiltRules.filter((latestRule) => + filterInstalledRules(latestRule, installedRules) + ); }; /** @@ -38,33 +38,3 @@ export const filterInstalledRules = ( return !!installedRule && installedRule.params.version < latestPrebuiltRule.version; }; - -/** - * Given a rule from the file system and the set of installed rules this will merge the exception lists - * from the installed rules onto the rules from the file system. - * @param latestPrebuiltRule The latest prepackaged rule version that might have exceptions_lists - * @param installedRules The installed rules which might have user driven exceptions_lists - */ -export const mergeExceptionLists = ( - latestPrebuiltRule: PrebuiltRuleAsset, - installedRules: Map -): PrebuiltRuleAsset => { - if (latestPrebuiltRule.exceptions_list != null) { - const installedRule = installedRules.get(latestPrebuiltRule.rule_id); - - if (installedRule != null && installedRule.params.exceptionsList != null) { - const installedExceptionList = installedRule.params.exceptionsList; - const fileSystemExceptions = latestPrebuiltRule.exceptions_list.filter((potentialDuplicate) => - installedExceptionList.every((item) => item.list_id !== potentialDuplicate.list_id) - ); - return { - ...latestPrebuiltRule, - exceptions_list: [...fileSystemExceptions, ...installedRule.params.exceptionsList], - }; - } else { - return latestPrebuiltRule; - } - } else { - return latestPrebuiltRule; - } -}; diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts index 3860fed1dabe6..eed63e9bd0aa3 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts @@ -168,6 +168,23 @@ describe('DetectionRulesClient.upgradePrebuiltRule', () => { }; // Installed version is "eql" const installedRule = getRulesEqlSchemaMock(); + installedRule.actions = [ + { + group: 'default', + id: 'test_id', + action_type_id: '.index', + params: {}, + }, + ]; + installedRule.exceptions_list = [ + { + id: 'exception_list', + list_id: 'some-id', + namespace_type: 'single', + type: 'detection', + }, + ]; + beforeEach(() => { (getRuleByRuleId as jest.Mock).mockResolvedValue(installedRule); }); @@ -175,6 +192,56 @@ describe('DetectionRulesClient.upgradePrebuiltRule', () => { it('patches the existing rule with the new params from the rule asset', async () => { rulesClient.update.mockResolvedValue(getRuleMock(getEqlRuleParams())); + await detectionRulesClient.upgradePrebuiltRule({ ruleAsset }); + expect(rulesClient.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + name: ruleAsset.name, + tags: ruleAsset.tags, + // actions are kept from original rule + actions: [ + expect.objectContaining({ + actionTypeId: '.index', + group: 'default', + id: 'test_id', + params: {}, + }), + ], + params: expect.objectContaining({ + index: ruleAsset.index, + description: ruleAsset.description, + exceptionsList: [ + { + id: 'exception_list', + list_id: 'some-id', + namespace_type: 'single', + type: 'detection', + }, + ], + }), + }), + id: installedRule.id, + }) + ); + }); + + it('merges exceptions lists for existing rule and new rule asset', async () => { + rulesClient.update.mockResolvedValue(getRuleMock(getEqlRuleParams())); + ruleAsset.exceptions_list = [ + { + id: 'exception_list', + list_id: 'some-id', + namespace_type: 'single', + type: 'detection', + }, + { + id: 'second_exception_list', + list_id: 'some-other-id', + namespace_type: 'single', + type: 'detection', + }, + ]; + await detectionRulesClient.upgradePrebuiltRule({ ruleAsset }); expect(rulesClient.update).toHaveBeenCalledWith( expect.objectContaining({ @@ -184,6 +251,20 @@ describe('DetectionRulesClient.upgradePrebuiltRule', () => { params: expect.objectContaining({ index: ruleAsset.index, description: ruleAsset.description, + exceptionsList: [ + { + id: 'second_exception_list', + list_id: 'some-other-id', + namespace_type: 'single', + type: 'detection', + }, + { + id: 'exception_list', + list_id: 'some-id', + namespace_type: 'single', + type: 'detection', + }, + ], }), }), id: installedRule.id, diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts index 64486bed14304..7c2bda89180e2 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts @@ -15,7 +15,7 @@ import type { IPrebuiltRuleAssetsClient } from '../../../../prebuilt_rules/logic import { convertAlertingRuleToRuleResponse } from '../converters/convert_alerting_rule_to_rule_response'; import { convertRuleResponseToAlertingRule } from '../converters/convert_rule_response_to_alerting_rule'; import { applyRuleUpdate } from '../mergers/apply_rule_update'; -import { ClientError, validateMlAuth } from '../utils'; +import { ClientError, validateMlAuth, mergeExceptionLists } from '../utils'; import { createRule } from './create_rule'; import { getRuleByRuleId } from './get_rule_by_rule_id'; @@ -75,9 +75,16 @@ export const upgradePrebuiltRule = async ({ ruleUpdate: ruleAsset, }); + // We want to preserve existing actions from existing rule on upgrade + if (existingRule.actions.length) { + updatedRule.actions = existingRule.actions; + } + + const updatedRuleWithMergedExceptions = mergeExceptionLists(updatedRule, existingRule); + const updatedInternalRule = await rulesClient.update({ id: existingRule.id, - data: convertRuleResponseToAlertingRule(updatedRule, actionsClient), + data: convertRuleResponseToAlertingRule(updatedRuleWithMergedExceptions, actionsClient), }); return convertAlertingRuleToRuleResponse(updatedInternalRule); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/utils.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/utils.test.ts new file mode 100644 index 0000000000000..8f8aff876a950 --- /dev/null +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/utils.test.ts @@ -0,0 +1,116 @@ +/* + * 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 { getRulesSchemaMock } from '../../../../../../common/api/detection_engine/model/rule_schema/rule_response_schema.mock'; +import { mergeExceptionLists } from './utils'; + +describe('mergeExceptionLists', () => { + test('should add back an exception_list if it was removed by the end user on an immutable rule during an upgrade', () => { + const ruleAsset1 = getRulesSchemaMock(); + ruleAsset1.exceptions_list = [ + { + id: 'endpoint_list', + list_id: 'endpoint_list', + namespace_type: 'agnostic', + type: 'endpoint', + }, + ]; + ruleAsset1.rule_id = 'rule-1'; + ruleAsset1.version = 2; + + const installedRule1 = getRulesSchemaMock(); + installedRule1.rule_id = 'rule-1'; + installedRule1.version = 1; + installedRule1.exceptions_list = []; + + const update = mergeExceptionLists(ruleAsset1, installedRule1); + expect(update.exceptions_list).toEqual(ruleAsset1.exceptions_list); + }); + + test('should not remove an additional exception_list if an additional one was added by the end user on an immutable rule during an upgrade', () => { + const ruleAsset1 = getRulesSchemaMock(); + ruleAsset1.exceptions_list = [ + { + id: 'endpoint_list', + list_id: 'endpoint_list', + namespace_type: 'agnostic', + type: 'endpoint', + }, + ]; + ruleAsset1.rule_id = 'rule-1'; + ruleAsset1.version = 2; + + const installedRule1 = getRulesSchemaMock(); + installedRule1.rule_id = 'rule-1'; + installedRule1.version = 1; + installedRule1.exceptions_list = [ + { + id: 'second_exception_list', + list_id: 'some-other-id', + namespace_type: 'single', + type: 'detection', + }, + ]; + + const update = mergeExceptionLists(ruleAsset1, installedRule1); + expect(update.exceptions_list).toEqual([ + ...ruleAsset1.exceptions_list, + ...installedRule1.exceptions_list, + ]); + }); + + test('should not remove an existing exception_list if they are the same between the current installed one and the upgraded one', () => { + const ruleAsset1 = getRulesSchemaMock(); + ruleAsset1.exceptions_list = [ + { + id: 'endpoint_list', + list_id: 'endpoint_list', + namespace_type: 'agnostic', + type: 'endpoint', + }, + ]; + ruleAsset1.rule_id = 'rule-1'; + ruleAsset1.version = 2; + + const installedRule1 = getRulesSchemaMock(); + installedRule1.rule_id = 'rule-1'; + installedRule1.version = 1; + installedRule1.exceptions_list = [ + { + id: 'endpoint_list', + list_id: 'endpoint_list', + namespace_type: 'agnostic', + type: 'endpoint', + }, + ]; + + const update = mergeExceptionLists(ruleAsset1, installedRule1); + expect(update.exceptions_list).toEqual(ruleAsset1.exceptions_list); + }); + + test('should not remove an existing exception_list if the rule has an empty exceptions list', () => { + const ruleAsset1 = getRulesSchemaMock(); + ruleAsset1.exceptions_list = []; + ruleAsset1.rule_id = 'rule-1'; + ruleAsset1.version = 2; + + const installedRule1 = getRulesSchemaMock(); + installedRule1.rule_id = 'rule-1'; + installedRule1.version = 1; + installedRule1.exceptions_list = [ + { + id: 'endpoint_list', + list_id: 'endpoint_list', + namespace_type: 'agnostic', + type: 'endpoint', + }, + ]; + + const update = mergeExceptionLists(ruleAsset1, installedRule1); + expect(update.exceptions_list).toEqual(installedRule1.exceptions_list); + }); +}); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/utils.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/utils.ts index 3b0fcace501cd..61e1b73ec8c51 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/utils.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/utils.ts @@ -61,3 +61,35 @@ export class RuleResponseValidationError extends Error { this.ruleId = ruleId; } } + +/** + * Given a rule from the file system and the set of installed rules this will merge the exception lists + * from the installed rules onto the rules from the file system. + * @param latestPrebuiltRule The latest prepackaged rule version that might have exceptions_lists + * @param existingRule The installed rules which might have user driven exceptions_lists + */ +export const mergeExceptionLists = ( + latestPrebuiltRule: RuleResponse, + existingRule: RuleResponse +): RuleResponse => { + if (latestPrebuiltRule.exceptions_list != null) { + if (existingRule.exceptions_list != null) { + const uniqueExceptionsLists = latestPrebuiltRule.exceptions_list.filter( + (potentialDuplicateList) => + existingRule.exceptions_list.every( + (list) => list.list_id !== potentialDuplicateList.list_id + ) + ); + return { + ...latestPrebuiltRule, + exceptions_list: [...uniqueExceptionsLists, ...existingRule.exceptions_list], + }; + } else { + return latestPrebuiltRule; + } + } else { + // Carry over the previous version's exception list + latestPrebuiltRule.exceptions_list = existingRule.exceptions_list; + return latestPrebuiltRule; + } +}; diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts index 21e546ff91bdc..20f27e5a3de11 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts @@ -16,6 +16,7 @@ import { getPrebuiltRulesStatus, installPrebuiltRules, getInstalledRules, + getWebHookAction, } from '../../../../utils'; import { deleteAllRules, deleteRule } from '../../../../../../../common/utils/security_solution'; @@ -23,6 +24,7 @@ export default ({ getService }: FtrProviderContext): void => { const es = getService('es'); const supertest = getService('supertest'); const log = getService('log'); + const securitySolutionApi = getService('securitySolutionApi'); describe('@ess @serverless @skipInServerlessMKI install prebuilt rules from package with historical versions with mock rule assets', () => { const getRuleAssetSavedObjects = () => [ @@ -98,6 +100,109 @@ export default ({ getService }: FtrProviderContext): void => { expect(response.rules_installed).toBe(1); expect(response.rules_updated).toBe(0); }); + + it('should not overwrite existing actions', async () => { + // Install prebuilt detection rule + await createHistoricalPrebuiltRuleAssetSavedObjects(es, [ + createRuleAssetSavedObject({ rule_id: 'rule-1', version: 1 }), + ]); + await installPrebuiltRulesAndTimelines(es, supertest); + + const { body: hookAction } = await supertest + .post('/api/actions/connector') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + await securitySolutionApi + .patchRule({ + body: { + rule_id: 'rule-1', + actions: [ + { + group: 'default', + id: hookAction.id, + action_type_id: hookAction.connector_type_id, + params: {}, + }, + ], + }, + }) + .expect(200); + + // Install new rule version + await createHistoricalPrebuiltRuleAssetSavedObjects(es, [ + createRuleAssetSavedObject({ rule_id: 'rule-1', version: 2 }), + ]); + + // Install/update prebuilt rules again + const response = await installPrebuiltRulesAndTimelines(es, supertest); + expect(response.rules_installed).toBe(0); + expect(response.rules_updated).toBe(1); + + const { body: prebuiltRule } = await securitySolutionApi.readRule({ + query: { rule_id: 'rule-1' }, + }); + + // Check the actions field of existing prebuilt rules is not overwritten + expect(prebuiltRule.actions).toEqual([ + expect.objectContaining({ + action_type_id: hookAction.connector_type_id, + frequency: { notifyWhen: 'onActiveAlert', summary: true, throttle: null }, + group: 'default', + id: hookAction.id, + params: {}, + }), + ]); + }); + + it('should not overwrite existing exceptions lists', async () => { + // Install prebuilt detection rule + await createHistoricalPrebuiltRuleAssetSavedObjects(es, [ + createRuleAssetSavedObject({ rule_id: 'rule-1', version: 1 }), + ]); + await installPrebuiltRulesAndTimelines(es, supertest); + + await securitySolutionApi + .patchRule({ + body: { + rule_id: 'rule-1', + exceptions_list: [ + { + id: 'some_uuid', + list_id: 'list_id_single', + namespace_type: 'single', + type: 'detection', + }, + ], + }, + }) + .expect(200); + + // Install new rule version + await createHistoricalPrebuiltRuleAssetSavedObjects(es, [ + createRuleAssetSavedObject({ rule_id: 'rule-1', version: 2 }), + ]); + + // Install/update prebuilt rules again + const response = await installPrebuiltRulesAndTimelines(es, supertest); + expect(response.rules_installed).toBe(0); + expect(response.rules_updated).toBe(1); + + const { body: prebuiltRule } = await securitySolutionApi.readRule({ + query: { rule_id: 'rule-1' }, + }); + + // Check the exceptions_list field of existing prebuilt rules is not overwritten + expect(prebuiltRule.exceptions_list).toEqual([ + expect.objectContaining({ + id: 'some_uuid', + list_id: 'list_id_single', + namespace_type: 'single', + type: 'detection', + }), + ]); + }); }); describe('using current endpoint', () => { From 40109a1276e840cc59e7e71b927653d37ec1f480 Mon Sep 17 00:00:00 2001 From: Davis Plumlee Date: Fri, 9 May 2025 15:07:47 -0400 Subject: [PATCH 2/5] fixes test --- ...prebuilt_rules_with_historical_versions.ts | 55 +++++++++++-------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts index 20f27e5a3de11..2061c0a852ec6 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts @@ -26,7 +26,7 @@ export default ({ getService }: FtrProviderContext): void => { const log = getService('log'); const securitySolutionApi = getService('securitySolutionApi'); - describe('@ess @serverless @skipInServerlessMKI install prebuilt rules from package with historical versions with mock rule assets', () => { + describe.only('@ess @serverless @skipInServerlessMKI install prebuilt rules from package with historical versions with mock rule assets', () => { const getRuleAssetSavedObjects = () => [ createRuleAssetSavedObject({ rule_id: 'rule-1', version: 1 }), createRuleAssetSavedObject({ rule_id: 'rule-1', version: 2 }), @@ -106,26 +106,43 @@ export default ({ getService }: FtrProviderContext): void => { await createHistoricalPrebuiltRuleAssetSavedObjects(es, [ createRuleAssetSavedObject({ rule_id: 'rule-1', version: 1 }), ]); + console await installPrebuiltRulesAndTimelines(es, supertest); - const { body: hookAction } = await supertest - .post('/api/actions/connector') - .set('kbn-xsrf', 'true') - .send(getWebHookAction()) - .expect(200); + // create connector/action + const createConnector = async (payload: Record) => + ( + await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(payload) + .expect(200) + ).body; + + const createWebHookConnector = () => createConnector(getWebHookAction()); + + const webHookAction = await createWebHookConnector(); + + const defaultRuleAction = { + id: webHookAction.id, + action_type_id: '.webhook' as const, + group: 'default' as const, + params: { + body: '{"test":"a default action"}', + }, + frequency: { + notifyWhen: 'onThrottleInterval' as const, + summary: true, + throttle: '1h' as const, + }, + uuid: 'd487ec3d-05f2-44ad-8a68-11c97dc92202', + }; await securitySolutionApi .patchRule({ body: { rule_id: 'rule-1', - actions: [ - { - group: 'default', - id: hookAction.id, - action_type_id: hookAction.connector_type_id, - params: {}, - }, - ], + actions: [defaultRuleAction], }, }) .expect(200); @@ -145,15 +162,7 @@ export default ({ getService }: FtrProviderContext): void => { }); // Check the actions field of existing prebuilt rules is not overwritten - expect(prebuiltRule.actions).toEqual([ - expect.objectContaining({ - action_type_id: hookAction.connector_type_id, - frequency: { notifyWhen: 'onActiveAlert', summary: true, throttle: null }, - group: 'default', - id: hookAction.id, - params: {}, - }), - ]); + expect(prebuiltRule.actions).toEqual([defaultRuleAction]); }); it('should not overwrite existing exceptions lists', async () => { From 03c3924ff9ed3690f559f0b3f246e35ae94e466c Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Fri, 9 May 2025 19:31:59 +0000 Subject: [PATCH 3/5] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../install_prebuilt_rules_with_historical_versions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts index 2061c0a852ec6..78646958fde89 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts @@ -106,7 +106,7 @@ export default ({ getService }: FtrProviderContext): void => { await createHistoricalPrebuiltRuleAssetSavedObjects(es, [ createRuleAssetSavedObject({ rule_id: 'rule-1', version: 1 }), ]); - console + console; await installPrebuiltRulesAndTimelines(es, supertest); // create connector/action From fbb8e4145fcd60f937e36a24e3488a5e41382104 Mon Sep 17 00:00:00 2001 From: Davis Plumlee Date: Fri, 9 May 2025 16:00:50 -0400 Subject: [PATCH 4/5] fix linting --- .../install_prebuilt_rules_with_historical_versions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts index 78646958fde89..c4cd251b66633 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts @@ -26,7 +26,7 @@ export default ({ getService }: FtrProviderContext): void => { const log = getService('log'); const securitySolutionApi = getService('securitySolutionApi'); - describe.only('@ess @serverless @skipInServerlessMKI install prebuilt rules from package with historical versions with mock rule assets', () => { + describe('@ess @serverless @skipInServerlessMKI install prebuilt rules from package with historical versions with mock rule assets', () => { const getRuleAssetSavedObjects = () => [ createRuleAssetSavedObject({ rule_id: 'rule-1', version: 1 }), createRuleAssetSavedObject({ rule_id: 'rule-1', version: 2 }), From b03a94ab393039f51acc0db99dc575565e57e944 Mon Sep 17 00:00:00 2001 From: Davis Plumlee Date: Fri, 9 May 2025 16:06:10 -0400 Subject: [PATCH 5/5] fix linting --- .../install_prebuilt_rules_with_historical_versions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts index c4cd251b66633..79180e6ef2f2f 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/management/trial_license_complete_tier/install_prebuilt_rules_with_historical_versions.ts @@ -106,7 +106,6 @@ export default ({ getService }: FtrProviderContext): void => { await createHistoricalPrebuiltRuleAssetSavedObjects(es, [ createRuleAssetSavedObject({ rule_id: 'rule-1', version: 1 }), ]); - console; await installPrebuiltRulesAndTimelines(es, supertest); // create connector/action