[Security Solution] Fix losing data upon prebuilt rule upgrade to a n…

…ew version in which the rule's type is different (elastic#176421)

**Fixes:** elastic#169480

## Summary

This PR fixes losing the following rule data upon prebuilt rule upgrade to a new version in which the rule's type is different

- Saved Object id
- exceptions list (default and shared)
- Timeline id
- Timeline title

## Details

The problem occurs when user upgrades a prebuilt rule to a newer version which has a different rule type.

Checking the code it's not so hard to find [`upgradeRule()`](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/rule_objects/upgrade_prebuilt_rules.ts#L49) function which performs prebuilt rule upgrade. It has the following comment

> If we're trying to change the type of a prepackaged rule, we need to delete the old one and replace it with the new rule, keeping the enabled setting, actions, throttle, id, and exception lists from the old rule.

Looking below in the code it's clear that only enabled state and actions get restored upon rule upgrade. Missing to restore `exceptions lists` leads to disappearing exceptions upon rule upgrade.

On top of this `execution results` and `execution events` also get lost due to missing to restore saved object `id`. Execution log isn't gone anywhere but can't be bound to a new id. Direct links to rule details page won't work neither after upgrade.

This PR fixes the problem by restoring rule bound data after upgrade.

FTR tests were restructured to accommodate extra tests to cover this bug fix.

### Checklist

- [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

(cherry picked from commit ffdcc34)

# Conflicts:
#	x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
#	x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/management/install_and_upgrade_prebuilt_rules.ts
#	x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/management/install_prebuilt_rules.ts
#	x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/management/install_prebuilt_rules_with_historical_versions.ts
#	x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/management/upgrade_prebuilt_rules.ts
#	x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/management/upgrade_prebuilt_rules_with_historical_versions.ts
#	x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_import_export/trial_license_complete_tier/export_rules.ts

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;
@@ -83,20 +84,7 @@ export function createTestConfig(options: CreateTestConfigOptions, testFiles?: s
             'riskScoringRoutesEnabled',
           ])}`,
           '--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}`,

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

@@ -12,6 +12,7 @@ export interface CreateTestConfigOptions {
   kbnTestServerEnv?: Record<string, string>;
 }
 import { services } from '../../../../test_serverless/api_integration/services';
+import { PRECONFIGURED_ACTION_CONNECTORS } from '../shared';
 
 export function createTestConfig(options: CreateTestConfigOptions) {
   return async ({ readConfigFile }: FtrConfigProviderContext) => {
@@ -28,6 +29,7 @@ export function createTestConfig(options: CreateTestConfigOptions) {
         serverArgs: [
           ...svlSharedConfig.get('kbnTestServer.serverArgs'),
           '--serverless=security',
+          `--xpack.actions.preconfigured=${JSON.stringify(PRECONFIGURED_ACTION_CONNECTORS)}`,
           ...(options.kbnTestServerArgs || []),
         ],
         env: {

x-pack/test/security_solution_api_integration/config/shared.ts

@@ -0,0 +1,32 @@
+/*
+ * 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 { Connector } from '@kbn/actions-plugin/server/application/connector/types';
+
+interface PreconfiguredConnector extends Pick<Connector, 'name' | 'actionTypeId' | 'config'> {
+  secrets: {
+    user: string;
+    password: string;
+  };
+}
+
+export const PRECONFIGURED_EMAIL_ACTION_CONNECTOR_ID = 'my-test-email';
+
+export const PRECONFIGURED_ACTION_CONNECTORS: Record<string, PreconfiguredConnector> = {
+  [PRECONFIGURED_EMAIL_ACTION_CONNECTOR_ID]: {
+    actionTypeId: '.email',
+    name: 'TestEmail#xyz',
+    config: {
+      from: '[email protected]',
+      service: '__json',
+    },
+    secrets: {
+      user: 'user',
+      password: 'password',
+    },
+  },
+};

x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/management/index.ts

@@ -11,7 +11,10 @@ export default ({ loadTestFile }: FtrProviderContext): void => {
   describe('Detection Engine API - Prebuilt Rules Management', function () {
     loadTestFile(require.resolve('./get_prebuilt_rules_status'));
     loadTestFile(require.resolve('./get_prebuilt_timelines_status'));
-    loadTestFile(require.resolve('./install_and_upgrade_prebuilt_rules'));
+    loadTestFile(require.resolve('./install_prebuilt_rules'));
+    loadTestFile(require.resolve('./install_prebuilt_rules_with_historical_versions'));
+    loadTestFile(require.resolve('./upgrade_prebuilt_rules'));
+    loadTestFile(require.resolve('./upgrade_prebuilt_rules_with_historical_versions'));
     loadTestFile(require.resolve('./fleet_integration'));
   });
 };