[Security Solution] [Platform] Migrate legacy actions whenever user interacts with the rule

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

@@ -171,6 +171,7 @@ export const createPrepackagedRules = async (
   );
   await updatePrepackagedRules(
     rulesClient,
+    savedObjectsClient,
     context.securitySolution.getSpaceId(),
     ruleStatusClient,
     rulesToUpdate,

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

@@ -273,6 +273,7 @@ export const importRulesRoute = (
                       } else if (rule != null && request.query.overwrite) {
                         await patchRules({
                           rulesClient,
+                          savedObjectsClient,
                           author,
                           buildingBlockType,
                           spaceId: context.securitySolution.getSpaceId(),

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

@@ -136,6 +136,7 @@ export const patchRulesBulkRoute = (
             const rule = await patchRules({
               rule: existingRule,
               rulesClient,
+              savedObjectsClient,
               author,
               buildingBlockType,
               description,

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

@@ -136,6 +136,7 @@ export const patchRulesRoute = (
 
         const rule = await patchRules({
           rulesClient,
+          savedObjectsClient,
           author,
           buildingBlockType,
           description,

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

@@ -73,6 +73,7 @@ export const updateRulesBulkRoute = (
               spaceId: context.securitySolution.getSpaceId(),
               rulesClient,
               ruleStatusClient,
+              savedObjectsClient,
               defaultOutputIndex: siemClient.getSignalsIndex(),
               ruleUpdate: payloadRule,
               isRuleRegistryEnabled,

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

@@ -64,6 +64,7 @@ export const updateRulesRoute = (
           isRuleRegistryEnabled,
           rulesClient,
           ruleStatusClient,
+          savedObjectsClient,
           ruleUpdate: request.body,
           spaceId: context.securitySolution.getSpaceId(),
         });

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

@@ -7,6 +7,7 @@
 
 import { PatchRulesOptions } from './types';
 import { rulesClientMock } from '../../../../../alerting/server/mocks';
+import { savedObjectsClientMock } from '../../../../../../../src/core/server/mocks';
 import { getAlertMock } from '../routes/__mocks__/request_responses';
 import { getMlRuleParams, getQueryRuleParams } from '../schemas/rule_schemas.mock';
 import { ruleExecutionLogClientMock } from '../rule_execution_log/__mocks__/rule_execution_log_client';
@@ -15,6 +16,7 @@ export const getPatchRulesOptionsMock = (isRuleRegistryEnabled: boolean): PatchR
   author: ['Elastic'],
   buildingBlockType: undefined,
   rulesClient: rulesClientMock.create(),
+  savedObjectsClient: savedObjectsClientMock.create(),
   spaceId: 'default',
   ruleStatusClient: ruleExecutionLogClientMock.create(),
   anomalyThreshold: undefined,
@@ -68,6 +70,7 @@ export const getPatchMlRulesOptionsMock = (isRuleRegistryEnabled: boolean): Patc
   author: ['Elastic'],
   buildingBlockType: undefined,
   rulesClient: rulesClientMock.create(),
+  savedObjectsClient: savedObjectsClientMock.create(),
   spaceId: 'default',
   ruleStatusClient: ruleExecutionLogClientMock.create(),
   anomalyThreshold: 55,

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

@@ -21,6 +21,7 @@ import {
   calculateInterval,
   calculateName,
   calculateVersion,
+  legacyMigrate,
   maybeMute,
   removeUndefined,
   transformToAlertThrottle,
@@ -37,6 +38,7 @@ class PatchError extends Error {
 
 export const patchRules = async ({
   rulesClient,
+  savedObjectsClient,
   author,
   buildingBlockType,
   ruleStatusClient,
@@ -92,6 +94,12 @@ export const patchRules = async ({
     return null;
   }
 
+  const { migratedActions, migratedThrottle, migratedNotifyWhen } = await legacyMigrate({
+    rulesClient,
+    savedObjectsClient,
+    id: rule.id,
+  });
+
   const calculatedVersion = calculateVersion(rule.params.immutable, rule.params.version, {
     author,
     buildingBlockType,
@@ -191,14 +199,20 @@ export const patchRules = async ({
 
   const newRule = {
     tags: addTags(tags ?? rule.tags, rule.params.ruleId, rule.params.immutable),
-    throttle: throttle !== undefined ? transformToAlertThrottle(throttle) : rule.throttle,
-    notifyWhen: throttle !== undefined ? transformToNotifyWhen(throttle) : rule.notifyWhen,
     name: calculateName({ updatedName: name, originalName: rule.name }),
     schedule: {
       interval: calculateInterval(interval, rule.schedule.interval),
     },
-    actions: actions?.map(transformRuleToAlertAction) ?? rule.actions,
     params: removeUndefined(nextParams),
+    actions: actions?.map(transformRuleToAlertAction) ?? migratedActions ?? rule.actions,
+    throttle:
+      throttle !== undefined
+        ? transformToAlertThrottle(throttle)
+        : migratedThrottle ?? rule.throttle,
+    notifyWhen:
+      throttle !== undefined
+        ? transformToNotifyWhen(throttle)
+        : migratedNotifyWhen ?? rule.notifyWhen,
   };
 
   const [validated, errors] = validate(newRule, internalRuleUpdate);

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

@@ -8,7 +8,12 @@
 import { get } from 'lodash/fp';
 import { Readable } from 'stream';
 
-import { SavedObject, SavedObjectAttributes, SavedObjectsFindResult } from 'kibana/server';
+import {
+  SavedObject,
+  SavedObjectAttributes,
+  SavedObjectsClientContract,
+  SavedObjectsFindResult,
+} from 'kibana/server';
 import type {
   MachineLearningJobIdOrUndefined,
   From,
@@ -271,12 +276,14 @@ export interface UpdateRulesOptions {
   rulesClient: RulesClient;
   defaultOutputIndex: string;
   ruleUpdate: UpdateRulesSchema;
+  savedObjectsClient: SavedObjectsClientContract;
 }
 
 export interface PatchRulesOptions {
   spaceId: string;
   ruleStatusClient: IRuleExecutionLogClient;
   rulesClient: RulesClient;
+  savedObjectsClient: SavedObjectsClientContract;
   anomalyThreshold: AnomalyThresholdOrUndefined;
   author: AuthorOrUndefined;
   buildingBlockType: BuildingBlockTypeOrUndefined;
@@ -351,3 +358,9 @@ export interface FindRuleOptions {
   fields: FieldsOrUndefined;
   sortOrder: SortOrderOrUndefined;
 }
+
+export interface LegacyMigrateParams {
+  rulesClient: RulesClient;
+  savedObjectsClient: SavedObjectsClientContract;
+  id: string;
+}

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

@@ -6,6 +6,7 @@
  */
 
 import { rulesClientMock } from '../../../../../alerting/server/mocks';
+import { savedObjectsClientMock } from '../../../../../../../src/core/server/mocks';
 import { getFindResultWithSingleHit } from '../routes/__mocks__/request_responses';
 import { updatePrepackagedRules } from './update_prepacked_rules';
 import { patchRules } from './patch_rules';
@@ -19,10 +20,12 @@ describe.each([
 ])('updatePrepackagedRules - %s', (_, isRuleRegistryEnabled) => {
   let rulesClient: ReturnType<typeof rulesClientMock.create>;
   let ruleStatusClient: ReturnType<typeof ruleExecutionLogClientMock.create>;
+  let savedObjectsClient: ReturnType<typeof savedObjectsClientMock.create>;
 
   beforeEach(() => {
     rulesClient = rulesClientMock.create();
     ruleStatusClient = ruleExecutionLogClientMock.create();
+    savedObjectsClient = savedObjectsClientMock.create();
   });
 
   it('should omit actions and enabled when calling patchRules', async () => {
@@ -40,6 +43,7 @@ describe.each([
 
     await updatePrepackagedRules(
       rulesClient,
+      savedObjectsClient,
       'default',
       ruleStatusClient,
       [{ ...prepackagedRule, actions }],

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

@@ -6,6 +6,7 @@
  */
 
 import { chunk } from 'lodash/fp';
+import { SavedObjectsClientContract } from 'kibana/server';
 import { AddPrepackagedRulesSchemaDecoded } from '../../../../common/detection_engine/schemas/request/add_prepackaged_rules_schema';
 import { RulesClient, PartialAlert } from '../../../../../alerting/server';
 import { patchRules } from './patch_rules';
@@ -51,6 +52,7 @@ export const UPDATE_CHUNK_SIZE = 50;
  */
 export const updatePrepackagedRules = async (
   rulesClient: RulesClient,
+  savedObjectsClient: SavedObjectsClientContract,
   spaceId: string,
   ruleStatusClient: IRuleExecutionLogClient,
   rules: AddPrepackagedRulesSchemaDecoded[],
@@ -61,6 +63,7 @@ export const updatePrepackagedRules = async (
   for (const ruleChunk of ruleChunks) {
     const rulePromises = createPromises(
       rulesClient,
+      savedObjectsClient,
       spaceId,
       ruleStatusClient,
       ruleChunk,
@@ -82,6 +85,7 @@ export const updatePrepackagedRules = async (
  */
 export const createPromises = (
   rulesClient: RulesClient,
+  savedObjectsClient: SavedObjectsClientContract,
   spaceId: string,
   ruleStatusClient: IRuleExecutionLogClient,
   rules: AddPrepackagedRulesSchemaDecoded[],
@@ -150,6 +154,7 @@ export const createPromises = (
     // or enable rules on the user when they were not expecting it if a rule updates
     return patchRules({
       rulesClient,
+      savedObjectsClient,
       author,
       buildingBlockType,
       description,

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

@@ -6,6 +6,7 @@
  */
 
 import { rulesClientMock } from '../../../../../alerting/server/mocks';
+import { savedObjectsClientMock } from '../../../../../../../src/core/server/mocks';
 import {
   getUpdateMachineLearningSchemaMock,
   getUpdateRulesSchemaMock,
@@ -16,6 +17,7 @@ export const getUpdateRulesOptionsMock = (isRuleRegistryEnabled: boolean) => ({
   spaceId: 'default',
   rulesClient: rulesClientMock.create(),
   ruleStatusClient: ruleExecutionLogClientMock.create(),
+  savedObjectsClient: savedObjectsClientMock.create(),
   defaultOutputIndex: '.siem-signals-default',
   ruleUpdate: getUpdateRulesSchemaMock(),
   isRuleRegistryEnabled,
@@ -25,6 +27,7 @@ export const getUpdateMlRulesOptionsMock = (isRuleRegistryEnabled: boolean) => (
   spaceId: 'default',
   rulesClient: rulesClientMock.create(),
   ruleStatusClient: ruleExecutionLogClientMock.create(),
+  savedObjectsClient: savedObjectsClientMock.create(),
   defaultOutputIndex: '.siem-signals-default',
   ruleUpdate: getUpdateMachineLearningSchemaMock(),
   isRuleRegistryEnabled,

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

@@ -6,17 +6,25 @@
  */
 
 /* eslint-disable complexity */
-
+import { validate } from '@kbn/securitysolution-io-ts-utils';
 import { DEFAULT_MAX_SIGNALS } from '../../../../common/constants';
 import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions';
 import { PartialAlert } from '../../../../../alerting/server';
 import { readRules } from './read_rules';
 import { UpdateRulesOptions } from './types';
 import { addTags } from './add_tags';
 import { typeSpecificSnakeToCamel } from '../schemas/rule_converters';
-import { RuleParams } from '../schemas/rule_schemas';
+import { internalRuleUpdate, RuleParams } from '../schemas/rule_schemas';
 import { enableRule } from './enable_rule';
-import { maybeMute, transformToAlertThrottle, transformToNotifyWhen } from './utils';
+import { legacyMigrate, maybeMute, transformToAlertThrottle, transformToNotifyWhen } from './utils';
+
+class UpdateError extends Error {
+  public readonly statusCode: number;
+  constructor(message: string, statusCode: number) {
+    super(message);
+    this.statusCode = statusCode;
+  }
+}
 
 export const updateRules = async ({
   isRuleRegistryEnabled,
@@ -25,6 +33,7 @@ export const updateRules = async ({
   ruleStatusClient,
   defaultOutputIndex,
   ruleUpdate,
+  savedObjectsClient,
 }: UpdateRulesOptions): Promise<PartialAlert<RuleParams> | null> => {
   const existingRule = await readRules({
     isRuleRegistryEnabled,
@@ -36,6 +45,12 @@ export const updateRules = async ({
     return null;
   }
 
+  const { migratedActions, migratedThrottle, migratedNotifyWhen } = await legacyMigrate({
+    rulesClient,
+    savedObjectsClient,
+    id: existingRule.id,
+  });
+
   const typeSpecificParams = typeSpecificSnakeToCamel(ruleUpdate);
   const enabled = ruleUpdate.enabled ?? true;
   const newInternalRule = {
@@ -77,14 +92,28 @@ export const updateRules = async ({
       ...typeSpecificParams,
     },
     schedule: { interval: ruleUpdate.interval ?? '5m' },
-    actions: ruleUpdate.actions != null ? ruleUpdate.actions.map(transformRuleToAlertAction) : [],
-    throttle: transformToAlertThrottle(ruleUpdate.throttle),
-    notifyWhen: transformToNotifyWhen(ruleUpdate.throttle),
+    actions:
+      ruleUpdate.actions != null
+        ? ruleUpdate.actions.map(transformRuleToAlertAction)
+        : migratedActions ?? [],
+    throttle:
+      ruleUpdate.throttle !== existingRule.throttle
+        ? transformToAlertThrottle(ruleUpdate.throttle)
+        : migratedThrottle ?? transformToAlertThrottle(ruleUpdate.throttle),
+    notifyWhen:
+      transformToNotifyWhen(ruleUpdate.throttle) !== transformToNotifyWhen(existingRule.throttle)
+        ? transformToNotifyWhen(ruleUpdate.throttle)
+        : migratedNotifyWhen ?? transformToNotifyWhen(ruleUpdate.throttle),
   };
 
+  const [validated, errors] = validate(newInternalRule, internalRuleUpdate);
+  if (errors != null || validated === null) {
+    throw new UpdateError(`Applying update would create invalid rule: ${errors}`, 400);
+  }
+
   const update = await rulesClient.update({
     id: existingRule.id,
-    data: newInternalRule,
+    data: validated,
   });
 
   await maybeMute({