Hotfix remove actions on update (#117910)

* if actions array is empty in the rule update payload, remove the actions whether migrating or not

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts

@@ -89,8 +89,7 @@ export const updateRulesBulkRoute = (
               rulesClient,
               ruleStatusClient,
               defaultOutputIndex: siemClient.getSignalsIndex(),
-              existingRule,
-              migratedRule,
+              existingRule: migratedRule,
               ruleUpdate: payloadRule,
               isRuleRegistryEnabled,
             });

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts

@@ -79,8 +79,7 @@ export const updateRulesRoute = (
           isRuleRegistryEnabled,
           rulesClient,
           ruleStatusClient,
-          existingRule,
-          migratedRule,
+          existingRule: migratedRule,
           ruleUpdate: request.body,
           spaceId: context.securitySolution.getSpaceId(),
         });

x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts

@@ -268,7 +268,6 @@ export interface UpdateRulesOptions {
   rulesClient: RulesClient;
   defaultOutputIndex: string;
   existingRule: SanitizedAlert<RuleParams> | null | undefined;
-  migratedRule: SanitizedAlert<RuleParams> | null | undefined;
   ruleUpdate: UpdateRulesSchema;
 }
 

x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.ts

@@ -16,13 +16,7 @@ import { addTags } from './add_tags';
 import { typeSpecificSnakeToCamel } from '../schemas/rule_converters';
 import { internalRuleUpdate, RuleParams } from '../schemas/rule_schemas';
 import { enableRule } from './enable_rule';
-import {
-  maybeMute,
-  transformToAlertThrottle,
-  transformToNotifyWhen,
-  updateActions,
-  updateThrottleNotifyWhen,
-} from './utils';
+import { maybeMute, transformToAlertThrottle, transformToNotifyWhen } from './utils';
 
 class UpdateError extends Error {
   public readonly statusCode: number;
@@ -38,7 +32,6 @@ export const updateRules = async ({
   ruleStatusClient,
   defaultOutputIndex,
   existingRule,
-  migratedRule,
   ruleUpdate,
 }: UpdateRulesOptions): Promise<PartialAlert<RuleParams> | null> => {
   if (existingRule == null) {
@@ -86,24 +79,9 @@ export const updateRules = async ({
       ...typeSpecificParams,
     },
     schedule: { interval: ruleUpdate.interval ?? '5m' },
-    actions: updateActions(
-      transformRuleToAlertAction,
-      migratedRule?.actions,
-      existingRule.actions,
-      ruleUpdate?.actions
-    ),
-    throttle: updateThrottleNotifyWhen(
-      transformToAlertThrottle,
-      migratedRule?.throttle,
-      existingRule.throttle,
-      ruleUpdate?.throttle
-    ),
-    notifyWhen: updateThrottleNotifyWhen(
-      transformToNotifyWhen,
-      migratedRule?.notifyWhen,
-      existingRule.notifyWhen,
-      ruleUpdate?.throttle
-    ),
+    actions: ruleUpdate.actions != null ? ruleUpdate.actions.map(transformRuleToAlertAction) : [],
+    throttle: transformToAlertThrottle(ruleUpdate.throttle),
+    notifyWhen: transformToNotifyWhen(ruleUpdate.throttle),
   };
 
   const [validated, errors] = validate(newInternalRule, internalRuleUpdate);

x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.ts

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { pickBy, isEmpty, isEqual } from 'lodash/fp';
+import { pickBy, isEmpty } from 'lodash/fp';
 import type {
   FromOrUndefined,
   MachineLearningJobIdOrUndefined,
@@ -64,14 +64,10 @@ import { RulesClient } from '../../../../../alerting/server';
 // eslint-disable-next-line no-restricted-imports
 import { LegacyRuleActions } from '../rule_actions/legacy_types';
 import { FullResponseSchema } from '../../../../common/detection_engine/schemas/request';
-import {
-  transformAlertToRuleAction,
-  transformRuleToAlertAction,
-} from '../../../../common/detection_engine/transform_actions';
+import { transformAlertToRuleAction } from '../../../../common/detection_engine/transform_actions';
 // eslint-disable-next-line no-restricted-imports
 import { legacyRuleActionsSavedObjectType } from '../rule_actions/legacy_saved_object_mappings';
 import { LegacyMigrateParams } from './types';
-import { RuleAlertAction } from '../../../../common/detection_engine/types';
 
 export const calculateInterval = (
   interval: string | undefined,
@@ -368,35 +364,3 @@ export const legacyMigrate = async ({
   }
   return rule;
 };
-
-export const updateThrottleNotifyWhen = (
-  transform: typeof transformToAlertThrottle | typeof transformToNotifyWhen,
-  migratedRuleThrottle: string | null | undefined,
-  existingRuleThrottle: string | null | undefined,
-  ruleUpdateThrottle: string | null | undefined
-) => {
-  if (existingRuleThrottle == null && ruleUpdateThrottle == null && migratedRuleThrottle != null) {
-    return migratedRuleThrottle;
-  }
-  return transform(ruleUpdateThrottle);
-};
-
-export const updateActions = (
-  transform: typeof transformRuleToAlertAction,
-  migratedRuleActions: AlertAction[] | null | undefined,
-  existingRuleActions: AlertAction[] | null | undefined,
-  ruleUpdateActions: RuleAlertAction[] | null | undefined
-) => {
-  // if the existing rule actions and the rule update actions are equivalent (aka no change)
-  // but the migrated actions and the ruleUpdateActions (or existing rule actions, associatively)
-  // are not equivalent, we know that the rules' actions were migrated and we need to apply
-  // that change to the update request so it is not overwritten by the rule update payload
-  if (
-    existingRuleActions?.length === 0 &&
-    ruleUpdateActions == null &&
-    !isEqual(existingRuleActions, migratedRuleActions)
-  ) {
-    return migratedRuleActions;
-  }
-  return ruleUpdateActions != null ? ruleUpdateActions.map(transform) : [];
-};

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules.ts

@@ -132,6 +132,59 @@ export default ({ getService }: FtrProviderContext) => {
         expect(bodyToCompare).to.eql(outputRule);
       });
 
+      it('should update a single rule property and remove the action', async () => {
+        const [connector1] = await Promise.all([
+          supertest
+            .post(`/api/actions/connector`)
+            .set('kbn-xsrf', 'foo')
+            .send({
+              name: 'My action',
+              connector_type_id: '.slack',
+              secrets: {
+                webhookUrl: 'http://localhost:1234',
+              },
+            }),
+        ]);
+
+        const action1 = {
+          group: 'default',
+          id: connector1.body.id,
+          action_type_id: connector1.body.connector_type_id,
+          params: {
+            message: 'message',
+          },
+        };
+
+        const ruleWithConnector: ReturnType<typeof getSimpleRule> = {
+          ...getSimpleRule('rule-1'),
+          actions: [action1],
+        };
+        const createdRule = await createRule(supertest, log, ruleWithConnector);
+        expect(createdRule.actions.length).to.eql(1);
+
+        // update a simple rule's name and remove the actions
+        const updatedRule = getSimpleRuleUpdate('rule-1');
+        updatedRule.rule_id = ruleWithConnector.rule_id;
+        updatedRule.name = 'some other name';
+        delete updatedRule.id;
+
+        const { body } = await supertest
+          .put(DETECTION_ENGINE_RULES_URL)
+          .set('kbn-xsrf', 'true')
+          .send(updatedRule)
+          .expect(200);
+
+        const outputRule = getSimpleRuleOutputWithoutRuleId();
+        outputRule.name = 'some other name';
+        outputRule.version = 2;
+        // Expect an empty array
+        outputRule.actions = [];
+        // Expect "no_actions"
+        outputRule.throttle = 'no_actions';
+        const bodyToCompare = removeServerGeneratedPropertiesIncludingRuleId(body);
+        expect(bodyToCompare).to.eql(outputRule);
+      });
+
       it('should update a single rule property of name using an auto-generated rule_id and migrate the actions', async () => {
         const rule = getSimpleRule('rule-1');
         delete rule.rule_id;
@@ -150,10 +203,20 @@ export default ({ getService }: FtrProviderContext) => {
         ]);
         await createLegacyRuleAction(supertest, createRuleBody.id, connector.body.id);
 
+        const action1 = {
+          group: 'default',
+          id: connector.body.id,
+          action_type_id: connector.body.connector_type_id,
+          params: {
+            message: 'Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts',
+          },
+        };
         // update a simple rule's name
         const updatedRule = getSimpleRuleUpdate('rule-1');
         updatedRule.rule_id = createRuleBody.rule_id;
         updatedRule.name = 'some other name';
+        updatedRule.actions = [action1];
+        updatedRule.throttle = '1m';
         delete updatedRule.id;
 
         const { body } = await supertest

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules_bulk.ts

@@ -6,6 +6,7 @@
  */
 
 import expect from '@kbn/expect';
+import { FullResponseSchema } from '../../../../plugins/security_solution/common/detection_engine/schemas/request';
 
 import { DETECTION_ENGINE_RULES_URL } from '../../../../plugins/security_solution/common/constants';
 import { FtrProviderContext } from '../../common/ftr_provider_context';
@@ -97,42 +98,50 @@ export default ({ getService }: FtrProviderContext) => {
       });
 
       it('should update two rule properties of name using the two rules rule_id and migrate actions', async () => {
-        const [connector, rule1, rule2] = await Promise.all([
-          supertest
-            .post(`/api/actions/connector`)
-            .set('kbn-xsrf', 'foo')
-            .send({
-              name: 'My action',
-              connector_type_id: '.slack',
-              secrets: {
-                webhookUrl: 'http://localhost:1234',
-              },
-            }),
-          createRule(supertest, log, getSimpleRule('rule-1')),
-          createRule(supertest, log, getSimpleRule('rule-2')),
+        const connector = await supertest
+          .post(`/api/actions/connector`)
+          .set('kbn-xsrf', 'foo')
+          .send({
+            name: 'My action',
+            connector_type_id: '.slack',
+            secrets: {
+              webhookUrl: 'http://localhost:1234',
+            },
+          });
+        const action1 = {
+          group: 'default',
+          id: connector.body.id,
+          action_type_id: connector.body.connector_type_id,
+          params: {
+            message: 'Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts',
+          },
+        };
+        const [rule1, rule2] = await Promise.all([
+          createRule(supertest, log, { ...getSimpleRule('rule-1'), actions: [action1] }),
+          createRule(supertest, log, { ...getSimpleRule('rule-2'), actions: [action1] }),
         ]);
         await Promise.all([
           createLegacyRuleAction(supertest, rule1.id, connector.body.id),
           createLegacyRuleAction(supertest, rule2.id, connector.body.id),
         ]);
 
-        expect(rule1.actions).to.eql([]);
-        expect(rule2.actions).to.eql([]);
-
         const updatedRule1 = getSimpleRuleUpdate('rule-1');
         updatedRule1.name = 'some other name';
+        updatedRule1.actions = [action1];
+        updatedRule1.throttle = '1m';
 
         const updatedRule2 = getSimpleRuleUpdate('rule-2');
         updatedRule2.name = 'some other name';
+        updatedRule2.actions = [action1];
+        updatedRule2.throttle = '1m';
 
         // update both rule names
-        const { body } = await supertest
+        const { body }: { body: FullResponseSchema[] } = await supertest
           .put(`${DETECTION_ENGINE_RULES_URL}/_bulk_update`)
           .set('kbn-xsrf', 'true')
           .send([updatedRule1, updatedRule2])
           .expect(200);
 
-        // @ts-expect-error
         body.forEach((response) => {
           const outputRule = getSimpleRuleOutput(response.rule_id);
           outputRule.name = 'some other name';
@@ -154,6 +163,58 @@ export default ({ getService }: FtrProviderContext) => {
         });
       });
 
+      it('should update two rule properties of name using the two rules rule_id and remove actions', async () => {
+        const connector = await supertest
+          .post(`/api/actions/connector`)
+          .set('kbn-xsrf', 'foo')
+          .send({
+            name: 'My action',
+            connector_type_id: '.slack',
+            secrets: {
+              webhookUrl: 'http://localhost:1234',
+            },
+          });
+        const action1 = {
+          group: 'default',
+          id: connector.body.id,
+          action_type_id: connector.body.connector_type_id,
+          params: {
+            message: 'message',
+          },
+        };
+        const [rule1, rule2] = await Promise.all([
+          createRule(supertest, log, { ...getSimpleRule('rule-1'), actions: [action1] }),
+          createRule(supertest, log, { ...getSimpleRule('rule-2'), actions: [action1] }),
+        ]);
+        await Promise.all([
+          createLegacyRuleAction(supertest, rule1.id, connector.body.id),
+          createLegacyRuleAction(supertest, rule2.id, connector.body.id),
+        ]);
+
+        const updatedRule1 = getSimpleRuleUpdate('rule-1');
+        updatedRule1.name = 'some other name';
+
+        const updatedRule2 = getSimpleRuleUpdate('rule-2');
+        updatedRule2.name = 'some other name';
+
+        // update both rule names
+        const { body }: { body: FullResponseSchema[] } = await supertest
+          .put(`${DETECTION_ENGINE_RULES_URL}/_bulk_update`)
+          .set('kbn-xsrf', 'true')
+          .send([updatedRule1, updatedRule2])
+          .expect(200);
+
+        body.forEach((response) => {
+          const outputRule = getSimpleRuleOutput(response.rule_id);
+          outputRule.name = 'some other name';
+          outputRule.version = 2;
+          outputRule.actions = [];
+          outputRule.throttle = 'no_actions';
+          const bodyToCompare = removeServerGeneratedProperties(response);
+          expect(bodyToCompare).to.eql(outputRule);
+        });
+      });
+
       it('should update a single rule property of name using an id', async () => {
         const createRuleBody = await createRule(supertest, log, getSimpleRule('rule-1'));