[Security Solution] Fix losing data upon prebuilt rule upgrade to a new version in which the rule's type is different

x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md

@@ -13,6 +13,7 @@ Status: `in progress`. The current test plan matches `Milestone 2` of the [Rule
   - [Non-functional requirements](#non-functional-requirements)
   - [Functional requirements](#functional-requirements)
 - [Scenarios](#scenarios)
+
   - [Package installation](#package-installation)
     - [**Scenario: Package is installed via Fleet**](#scenario-package-is-installed-via-fleet)
     - [**Scenario: Package is installed via bundled Fleet package in Kibana**](#scenario-package-is-installed-via-bundled-fleet-package-in-kibana)
@@ -63,6 +64,9 @@ Status: `in progress`. The current test plan matches `Milestone 2` of the [Rule
     - [**Scenario: Properties with semantically equal values should not be shown as modified**](#scenario-properties-with-semantically-equal-values-should-not-be-shown-as-modified)
     - [**Scenario: Unchanged sections of a rule should be hidden by default**](#scenario-unchanged-sections-of-a-rule-should-be-hidden-by-default)
     - [**Scenario: Properties should be sorted alphabetically**](#scenario-properties-should-be-sorted-alphabetically)
+  - [Rule upgrade workflow: preserving rule bound data](#rule-upgrade-workflow-preserving-rule-bound-data)
+    - [**Scenario: Rule bound data is preserved after upgrading a rule to a newer version with the same rule type**](#scenario-rule-bound-data-is-preserved-after-upgrading-a-rule-to-a-newer-version-with-the-same-rule-type)
+    - [**Scenario: Rule bound data is preserved after upgrading a rule to a newer version with a different rule type**](#scenario-rule-bound-data-is-preserved-after-upgrading-a-rule-to-a-newer-version-with-a-different-rule-type)
   - [Rule upgrade workflow: misc cases](#rule-upgrade-workflow-misc-cases)
     - [**Scenario: User doesn't see the Rule Updates tab until the package installation is completed**](#scenario-user-doesnt-see-the-rule-updates-tab-until-the-package-installation-is-completed)
   - [Error handling](#error-handling)
@@ -949,6 +953,36 @@ When a user expands all hidden sections
 Then all properties of the rule should be sorted alphabetically
 ```
 
+### Rule upgrade workflow: preserving rule bound data
+
+#### **Scenario: Rule bound data is preserved after upgrading a rule to a newer version with the same rule type**
+
+**Automation**: 1 unit test per case, 1 integration test
+
+```Gherkin
+Given a prebuilt rule is installed in Kibana
+And this rule has an update available
+And the update has the same rule type
+When a user upgrades the rule
+Then the rule bound data should be preserved
+```
+
+Examples: generated alerts, exception lists (rule exception list, shared exception list, endpoint exception list), timeline reference, actions, enabled state, execution results and execution events.
+
+#### **Scenario: Rule bound data is preserved after upgrading a rule to a newer version with a different rule type**
+
+**Automation**: 1 unit test per case, 1 integration test
+
+```Gherkin
+Given a prebuilt rule is installed in Kibana
+And this rule has an update available
+And the update has a different rule type
+When a user upgrades the rule
+Then the rule bound data should be preserved
+```
+
+Examples: generated alerts, exception lists (rule exception list, shared exception list, endpoint exception list), timeline reference, actions, enabled state, execution results and execution events.
+
 ### Rule upgrade workflow: misc cases
 
 #### **Scenario: User doesn't see the Rule Updates tab until the package installation is completed**

x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/rule_objects/upgrade_prebuilt_rules.test.ts

@@ -5,17 +5,23 @@
  * 2.0.
  */
 
+import { omit } from 'lodash';
 import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks';
 import {
   getRuleMock,
   getFindResultWithSingleHit,
+  getFindResultWithMultiHits,
 } from '../../../routes/__mocks__/request_responses';
 import { upgradePrebuiltRules } from './upgrade_prebuilt_rules';
 import { patchRules } from '../../../rule_management/logic/crud/patch_rules';
+import { createRules } from '../../../rule_management/logic/crud/create_rules';
+import { deleteRules } from '../../../rule_management/logic/crud/delete_rules';
 import { getPrebuiltRuleMock, getPrebuiltThreatMatchRuleMock } from '../../mocks';
-import { getThreatRuleParams } from '../../../rule_schema/mocks';
+import { getQueryRuleParams, getThreatRuleParams } from '../../../rule_schema/mocks';
 
 jest.mock('../../../rule_management/logic/crud/patch_rules');
+jest.mock('../../../rule_management/logic/crud/create_rules');
+jest.mock('../../../rule_management/logic/crud/delete_rules');
 
 describe('updatePrebuiltRules', () => {
   let rulesClient: ReturnType<typeof rulesClientMock.create>;
@@ -24,35 +30,173 @@ describe('updatePrebuiltRules', () => {
     rulesClient = rulesClientMock.create();
   });
 
-  it('should omit actions and enabled when calling patchRules', async () => {
+  describe('when upgrading a prebuilt rule to a newer version with the same rule type', () => {
+    const prepackagedRule = getPrebuiltRuleMock({
+      rule_id: 'rule-to-upgrade',
+    });
+
+    beforeEach(() => {
+      const installedRule = getRuleMock(
+        getQueryRuleParams({
+          ruleId: 'rule-to-upgrade',
+        })
+      );
+
+      rulesClient.find.mockResolvedValue(
+        getFindResultWithMultiHits({
+          data: [installedRule],
+        })
+      );
+    });
+
+    it('patches existing rule with incoming version data', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(patchRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          nextParams: expect.objectContaining(prepackagedRule),
+        })
+      );
+    });
+
+    it('makes sure enabled state is preserved', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(patchRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          nextParams: expect.objectContaining({
+            enabled: undefined,
+          }),
+        })
+      );
+    });
+  });
+
+  describe('when upgrading a prebuilt rule to a newer version with a different rule type', () => {
+    const prepackagedRule = getPrebuiltRuleMock({
+      rule_id: 'rule-to-upgrade',
+      type: 'eql',
+      language: 'eql',
+      query: 'host where host.name == "something"',
+    });
     const actions = [
       {
         group: 'group',
         id: 'id',
-        action_type_id: 'action_type_id',
+        actionTypeId: 'action_type_id',
         params: {},
       },
     ];
-    const prepackagedRule = getPrebuiltRuleMock();
-    rulesClient.find.mockResolvedValue(getFindResultWithSingleHit());
+    const installedRule = getRuleMock(
+      getQueryRuleParams({
+        ruleId: 'rule-to-upgrade',
+        type: 'query',
+        exceptionsList: [
+          {
+            id: 'exception_list_1',
+            list_id: 'exception_list_1',
+            namespace_type: 'agnostic',
+            type: 'rule_default',
+          },
+        ],
+        timelineId: 'some-timeline-id',
+        timelineTitle: 'Some timeline title',
+      }),
+      {
+        id: 'installed-rule-so-id',
+        actions,
+      }
+    );
 
-    await upgradePrebuiltRules(rulesClient, [{ ...prepackagedRule, actions }]);
+    beforeEach(() => {
+      rulesClient.find.mockResolvedValue(
+        getFindResultWithMultiHits({
+          data: [installedRule],
+        })
+      );
+    });
 
-    expect(patchRules).toHaveBeenCalledWith(
-      expect.objectContaining({
-        nextParams: expect.objectContaining({
-          actions: undefined,
-        }),
-      })
-    );
+    it('deletes rule before creation', async () => {
+      let lastCalled!: string;
 
-    expect(patchRules).toHaveBeenCalledWith(
-      expect.objectContaining({
-        nextParams: expect.objectContaining({
-          enabled: undefined,
-        }),
-      })
-    );
+      (deleteRules as jest.Mock).mockImplementation(() => (lastCalled = 'deleteRules'));
+      (createRules as jest.Mock).mockImplementation(() => (lastCalled = 'createRules'));
+
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(deleteRules).toHaveBeenCalledTimes(1);
+      expect(createRules).toHaveBeenCalledTimes(1);
+      expect(lastCalled).toBe('createRules');
+    });
+
+    it('recreates a rule with incoming version data', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          immutable: true,
+          params: expect.objectContaining(prepackagedRule),
+        })
+      );
+    });
+
+    it('restores saved object id', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          id: 'installed-rule-so-id',
+        })
+      );
+    });
+
+    it('restores enabled state', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          params: expect.objectContaining({ enabled: installedRule.enabled }),
+        })
+      );
+    });
+
+    it('restores actions', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          params: expect.objectContaining({
+            actions: actions.map((a) => ({
+              ...omit(a, 'actionTypeId'),
+              action_type_id: a.actionTypeId,
+            })),
+          }),
+        })
+      );
+    });
+
+    it('restores exceptions list', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          params: expect.objectContaining({ exceptions_list: installedRule.params.exceptionsList }),
+        })
+      );
+    });
+
+    it('restores timeline reference', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          params: expect.objectContaining({
+            timeline_id: installedRule.params.timelineId,
+            timeline_title: installedRule.params.timelineTitle,
+          }),
+        })
+      );
+    });
   });
 
   it('should update threat match rules', async () => {

x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/rule_objects/upgrade_prebuilt_rules.ts

@@ -72,36 +72,39 @@ const upgradeRule = async (
     return createRules({
       rulesClient,
       immutable: true,
+      id: existingRule.id,
       params: {
         ...rule,
         // Force the prepackaged rule to use the enabled state from the existing rule,
         // regardless of what the prepackaged rule says
         enabled: existingRule.enabled,
+        exceptions_list: existingRule.params.exceptionsList,
         actions: existingRule.actions.map(transformAlertToRuleAction),
+        timeline_id: existingRule.params.timelineId,
+        timeline_title: existingRule.params.timelineTitle,
       },
     });
-  } else {
-    await patchRules({
-      rulesClient,
-      existingRule,
-      nextParams: {
-        ...rule,
-        // Force enabled to use the enabled state from the existing rule by passing in undefined to patchRules
-        enabled: undefined,
-        actions: undefined,
-      },
-    });
+  }
 
-    const updatedRule = await readRules({
-      rulesClient,
-      ruleId: rule.rule_id,
-      id: undefined,
-    });
+  await patchRules({
+    rulesClient,
+    existingRule,
+    nextParams: {
+      ...rule,
+      // Force enabled to use the enabled state from the existing rule by passing in undefined to patchRules
+      enabled: undefined,
+    },
+  });
 
-    if (!updatedRule) {
-      throw new PrepackagedRulesError(`Rule ${rule.rule_id} not found after upgrade`, 500);
-    }
+  const updatedRule = await readRules({
+    rulesClient,
+    ruleId: rule.rule_id,
+    id: undefined,
+  });
 
-    return updatedRule;
+  if (!updatedRule) {
+    throw new PrepackagedRulesError(`Rule ${rule.rule_id} not found after upgrade`, 500);
   }
+
+  return updatedRule;
 };

x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/model/rule_assets/prebuilt_rule_asset.mock.ts

@@ -7,17 +7,19 @@
 
 import type { PrebuiltRuleAsset } from './prebuilt_rule_asset';
 
-export const getPrebuiltRuleMock = (): PrebuiltRuleAsset => ({
-  description: 'some description',
-  name: 'Query with a rule id',
-  query: 'user.name: root or user.name: admin',
-  severity: 'high',
-  type: 'query',
-  risk_score: 55,
-  language: 'kuery',
-  rule_id: 'rule-1',
-  version: 1,
-});
+export const getPrebuiltRuleMock = (rewrites?: Partial<PrebuiltRuleAsset>): PrebuiltRuleAsset =>
+  ({
+    description: 'some description',
+    name: 'Query with a rule id',
+    query: 'user.name: root or user.name: admin',
+    severity: 'high',
+    type: 'query',
+    risk_score: 55,
+    language: 'kuery',
+    rule_id: 'rule-1',
+    version: 1,
+    ...rewrites,
+  } as PrebuiltRuleAsset);
 
 export const getPrebuiltRuleWithExceptionsMock = (): PrebuiltRuleAsset => ({
   description: 'A rule with an exception list',

x-pack/test/security_solution_api_integration/config/ess/config.base.ts

@@ -8,6 +8,7 @@
 import { CA_CERT_PATH } from '@kbn/dev-utils';
 import { FtrConfigProviderContext, kbnTestConfig, kibanaTestUser } from '@kbn/test';
 import { services } from '../../../api_integration/services';
+import { PRECONFIGURED_ACTION_CONNECTORS } from '../shared';
 
 interface CreateTestConfigOptions {
   license: string;
@@ -85,20 +86,7 @@ export function createTestConfig(options: CreateTestConfigOptions, testFiles?: s
             'alertSuppressionForIndicatorMatchRuleEnabled',
           ])}`,
           '--xpack.task_manager.poll_interval=1000',
-          `--xpack.actions.preconfigured=${JSON.stringify({
-            'my-test-email': {
-              actionTypeId: '.email',
-              name: 'TestEmail#xyz',
-              config: {
-                from: '[email protected]',
-                service: '__json',
-              },
-              secrets: {
-                user: 'user',
-                password: 'password',
-              },
-            },
-          })}`,
+          `--xpack.actions.preconfigured=${JSON.stringify(PRECONFIGURED_ACTION_CONNECTORS)}`,
           ...(ssl
             ? [
                 `--elasticsearch.hosts=${servers.elasticsearch.protocol}://${servers.elasticsearch.hostname}:${servers.elasticsearch.port}`,