From ed44e9a38ff8e62cd2b2024c2c90dae5bab58ccd Mon Sep 17 00:00:00 2001 From: Georgii Gorbachev Date: Tue, 27 Aug 2024 22:12:58 +0200 Subject: [PATCH] [8.15] [Security Solution] Fix prebuilt rule duplication logic to copy related integrations and required fields from the original rule (#191065) (#191493) # Backport This will backport the following commits from `main` to `8.15`: - [[Security Solution] Fix prebuilt rule duplication logic to copy related integrations and required fields from the original rule (#191065)](https://github.com/elastic/kibana/pull/191065) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) --- .../logic/actions/duplicate_rule.test.ts | 232 +++++++----------- .../logic/actions/duplicate_rule.ts | 26 +- 2 files changed, 98 insertions(+), 160 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts index 6684b7980ff6c..2cbd9f6381ec4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts @@ -7,6 +7,7 @@ import { v4 as uuidv4 } from 'uuid'; import type { SanitizedRule } from '@kbn/alerting-plugin/common'; + import type { RuleParams } from '../../../rule_schema'; import { duplicateRule } from './duplicate_rule'; @@ -35,13 +36,25 @@ describe('duplicateRule', () => { meta: undefined, maxSignals: 100, responseActions: [], - relatedIntegrations: [], - requiredFields: [], + relatedIntegrations: [ + { + package: 'aws', + version: '~1.2.3', + integration: 'route53', + }, + ], + requiredFields: [ + { + name: 'event.action', + type: 'keyword', + ecs: true, + }, + ], riskScore: 42, riskScoreMapping: [], severity: 'low', severityMapping: [], - setup: 'Some setup guide.', + setup: `## Config\n\nThe 'Audit Detailed File Share' audit policy must be configured...`, threat: [], to: 'now', references: [], @@ -93,105 +106,23 @@ describe('duplicateRule', () => { jest.clearAllMocks(); }); - it('returns an object with fields copied from a given rule', async () => { - const rule = createTestRule(); - const result = await duplicateRule({ - rule, - }); - - expect(result).toEqual({ - name: expect.anything(), // covered in a separate test - params: { - ...rule.params, - ruleSource: { - type: 'internal', - }, - ruleId: expect.anything(), // covered in a separate test - }, - tags: rule.tags, - alertTypeId: rule.alertTypeId, - consumer: rule.consumer, - schedule: rule.schedule, - actions: rule.actions, - enabled: false, // covered in a separate test - }); - }); - - it('appends [Duplicate] to the name', async () => { - const rule = createTestRule(); - rule.name = 'PowerShell Keylogging Script'; - const result = await duplicateRule({ - rule, - }); - - expect(result).toEqual( - expect.objectContaining({ - name: 'PowerShell Keylogging Script [Duplicate]', - }) - ); - }); - - it('generates a new ruleId', async () => { - const rule = createTestRule(); - const result = await duplicateRule({ - rule, - }); - - expect(result).toEqual( - expect.objectContaining({ - params: expect.objectContaining({ - ruleId: 'new ruleId', - }), - }) - ); - }); - - it('makes sure the duplicated rule is disabled', async () => { - const rule = createTestRule(); - rule.enabled = true; - const result = await duplicateRule({ - rule, - }); - - expect(result).toEqual( - expect.objectContaining({ - enabled: false, - }) - ); - }); - - describe('when duplicating a prebuilt (immutable) rule', () => { - const createPrebuiltRule = () => { + describe('when duplicating any kind of rule', () => { + it('appends [Duplicate] to the name', async () => { const rule = createTestRule(); - rule.params.immutable = true; - return rule; - }; - - it('transforms it to a custom (mutable) rule', async () => { - const rule = createPrebuiltRule(); + rule.name = 'PowerShell Keylogging Script'; const result = await duplicateRule({ rule, }); expect(result).toEqual( expect.objectContaining({ - params: expect.objectContaining({ - immutable: false, - }), + name: 'PowerShell Keylogging Script [Duplicate]', }) ); }); - it('resets related integrations to an empty array', async () => { - const rule = createPrebuiltRule(); - rule.params.relatedIntegrations = [ - { - package: 'aws', - version: '~1.2.3', - integration: 'route53', - }, - ]; - + it('generates a new ruleId', async () => { + const rule = createTestRule(); const result = await duplicateRule({ rule, }); @@ -199,45 +130,40 @@ describe('duplicateRule', () => { expect(result).toEqual( expect.objectContaining({ params: expect.objectContaining({ - relatedIntegrations: [], + ruleId: 'new ruleId', }), }) ); }); - it('resets required fields to an empty array', async () => { - const rule = createPrebuiltRule(); - rule.params.requiredFields = [ - { - name: 'event.action', - type: 'keyword', - ecs: true, - }, - ]; - + it('makes sure the duplicated rule is disabled', async () => { + const rule = createTestRule(); + rule.enabled = true; const result = await duplicateRule({ rule, }); expect(result).toEqual( expect.objectContaining({ - params: expect.objectContaining({ - requiredFields: [], - }), + enabled: false, }) ); }); }); - describe('when duplicating a custom (mutable) rule', () => { - const createCustomRule = () => { + describe('when duplicating a prebuilt rule', () => { + const createPrebuiltRule = () => { const rule = createTestRule(); - rule.params.immutable = false; + rule.params.immutable = true; + rule.params.ruleSource = { + type: 'external', + isCustomized: false, + }; return rule; }; - it('keeps it custom', async () => { - const rule = createCustomRule(); + it('transforms it to a custom rule', async () => { + const rule = createPrebuiltRule(); const result = await duplicateRule({ rule, }); @@ -246,44 +172,50 @@ describe('duplicateRule', () => { expect.objectContaining({ params: expect.objectContaining({ immutable: false, + ruleSource: { + type: 'internal', + }, }), }) ); }); - it('copies related integrations as is', async () => { - const rule = createCustomRule(); - rule.params.relatedIntegrations = [ - { - package: 'aws', - version: '~1.2.3', - integration: 'route53', - }, - ]; - + it('copies fields from the original rule', async () => { + const rule = createPrebuiltRule(); const result = await duplicateRule({ rule, }); - expect(result).toEqual( - expect.objectContaining({ - params: expect.objectContaining({ - relatedIntegrations: rule.params.relatedIntegrations, - }), - }) - ); + expect(result).toEqual({ + name: expect.anything(), // covered in a separate test + params: { + ...rule.params, + ruleId: expect.anything(), // covered in a separate test + immutable: expect.anything(), // covered in a separate test + ruleSource: expect.anything(), // covered in a separate test + }, + tags: rule.tags, + alertTypeId: rule.alertTypeId, + consumer: rule.consumer, + schedule: rule.schedule, + actions: rule.actions, + enabled: false, // covered in a separate test + }); }); + }); - it('copies required fields as is', async () => { - const rule = createCustomRule(); - rule.params.requiredFields = [ - { - name: 'event.action', - type: 'keyword', - ecs: true, - }, - ]; + describe('when duplicating a custom rule', () => { + const createCustomRule = () => { + const rule = createTestRule(); + rule.params.immutable = false; + rule.params.ruleSource = { + type: 'internal', + }; + return rule; + }; + it('keeps it custom', async () => { + const rule = createCustomRule(); const result = await duplicateRule({ rule, }); @@ -291,26 +223,34 @@ describe('duplicateRule', () => { expect(result).toEqual( expect.objectContaining({ params: expect.objectContaining({ - requiredFields: rule.params.requiredFields, + immutable: false, + ruleSource: { + type: 'internal', + }, }), }) ); }); - it('copies setup guide as is', async () => { + it('copies fields from the original rule', async () => { const rule = createCustomRule(); - rule.params.setup = `## Config\n\nThe 'Audit Detailed File Share' audit policy must be configured...`; const result = await duplicateRule({ rule, }); - expect(result).toEqual( - expect.objectContaining({ - params: expect.objectContaining({ - setup: rule.params.setup, - }), - }) - ); + expect(result).toEqual({ + name: expect.anything(), // covered in a separate test + params: { + ...rule.params, + ruleId: expect.anything(), // covered in a separate test + }, + tags: rule.tags, + alertTypeId: rule.alertTypeId, + consumer: rule.consumer, + schedule: rule.schedule, + actions: rule.actions, + enabled: false, // covered in a separate test + }); }); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts index dd22dac3adc77..9dafff69f8b98 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts @@ -12,7 +12,6 @@ import type { SanitizedRule } from '@kbn/alerting-plugin/common'; import { SERVER_APP_ID } from '../../../../../../common/constants'; import type { InternalRuleCreate, RuleParams } from '../../../rule_schema'; import { transformToActionFrequency } from '../../normalization/rule_actions'; -import { convertImmutableToRuleSource } from '../../normalization/rule_converters'; const DUPLICATE_TITLE = i18n.translate( 'xpack.securitySolution.detectionEngine.rules.cloneRule.duplicateTitle', @@ -27,17 +26,18 @@ interface DuplicateRuleParams { export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise => { // Generate a new static ruleId - const ruleId = uuidv4(); - - // If it's a prebuilt rule, reset Related Integrations, Required Fields and Setup Guide. - // We do this because for now we don't allow the users to edit these fields for custom rules. - const isPrebuilt = rule.params.immutable; - const relatedIntegrations = isPrebuilt ? [] : rule.params.relatedIntegrations; - const requiredFields = isPrebuilt ? [] : rule.params.requiredFields; - const actions = transformToActionFrequency(rule.actions, rule.throttle); + const ruleId: InternalRuleCreate['params']['ruleId'] = uuidv4(); // Duplicated rules are always considered custom rules - const immutable = false; + const immutable: InternalRuleCreate['params']['immutable'] = false; + const ruleSource: InternalRuleCreate['params']['ruleSource'] = { + type: 'internal', + }; + + const actions: InternalRuleCreate['actions'] = transformToActionFrequency( + rule.actions, + rule.throttle + ); return { name: `${rule.name} [${DUPLICATE_TITLE}]`, @@ -46,11 +46,9 @@ export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise