[Security Solution] Extend the /upgrade/_review endpoint contract and functionality

x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/index.ts

@@ -20,4 +20,5 @@ export * from './model/diff/rule_diff/fields_diff';
 export * from './model/diff/rule_diff/rule_diff';
 export * from './model/diff/three_way_diff/three_way_diff_outcome';
 export * from './model/diff/three_way_diff/three_way_diff';
+export * from './model/diff/three_way_diff/three_way_diff_conflict';
 export * from './model/diff/three_way_diff/three_way_merge_outcome';

x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/rule_diff/rule_diff.ts

@@ -54,24 +54,27 @@ export type RuleFieldsDiff = CommonFieldsDiff &
     | NewTermsFieldsDiff
   );
 
+interface BaseRuleDiff {
+  num_fields_with_updates: number;
+  num_fields_with_conflicts: number;
+  num_fields_with_non_solvable_conflicts: number;
+}
 /**
  * Full rule diff contains diffs for all the top-level rule fields.
  * Even if there's no change at all to a given field, its diff will be included in this object.
  * This diff can be useful for internal server-side calculations or debugging.
  * Note that this is a pretty large object so returning it from the API might be undesirable.
  */
-export interface FullRuleDiff {
+export interface FullRuleDiff extends BaseRuleDiff {
   fields: RuleFieldsDiff;
-  has_conflict: boolean;
 }
 
 /**
  * Partial rule diff contains diffs only for those rule fields that have some changes to them.
  * This diff can be useful for returning info from REST API endpoints because its size is tolerable.
  */
-export interface PartialRuleDiff {
+export interface PartialRuleDiff extends BaseRuleDiff {
   fields: Partial<RuleFieldsDiff>;
-  has_conflict: boolean;
 }
 
 export type RuleFieldsDiffWithDataSource =

x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff.ts

@@ -6,6 +6,7 @@
  */
 
 import type { ThreeWayDiffOutcome } from './three_way_diff_outcome';
+import type { ThreeWayDiffConflict } from './three_way_diff_conflict';
 import type { ThreeWayMergeOutcome } from './three_way_merge_outcome';
 
 /**
@@ -66,7 +67,27 @@ export interface ThreeVersionsOf<TValue> {
  * 6. base=A, current=B, target=C => merged=C, conflict=true
  *    Customized rule, the value has changed, conflict between B and C couldn't be resolved automatically.
  */
-export interface ThreeWayDiff<TValue> extends ThreeVersionsOf<TValue> {
+export interface ThreeWayDiff<TValue> {
+  /**
+   * Corresponds to the stock version of the currently installed prebuilt rule.
+   * This field is optional because the base version is not always available in the package.
+   * This type is part of the API response, so the type of this field is replaced from possibly
+   * a MissingVersion (a JS Symbol), which can't be serialized in JSON, to possibly `undefined`.
+   */
+  base_version: TValue | undefined;
+
+  /**
+   * Corresponds exactly to the currently installed prebuilt rule:
+   *   - to the customized version (if it's customized)
+   *   - to the stock version (if it's not customized)
+   */
+  current_version: TValue;
+
+  /**
+   * Corresponds to the "new" stock version that the user is trying to upgrade to.
+   */
+  target_version: TValue;
+
   /**
    * The result of an automatic three-way merge of three values:
    *   - base version
@@ -92,12 +113,19 @@ export interface ThreeWayDiff<TValue> extends ThreeVersionsOf<TValue> {
 
   /**
    * The type of result of an automatic three-way merge of three values:
-   *   - base version
    *   - current version
    *   - target version
+   *   - merged version
    */
   merge_outcome: ThreeWayMergeOutcome;
 
+  /**
+   * Boolean which determines if a base version was found and returned for the three-way-diff of the field
+   * - true: the base version of the field was found and is either defined or undefined
+   * - false: the base version of the field was not found
+   */
+  has_base_version: boolean;
+
   /**
    * Tells if the value has changed in the target version and the current version could be updated.
    * True if:
@@ -107,15 +135,9 @@ export interface ThreeWayDiff<TValue> extends ThreeVersionsOf<TValue> {
   has_update: boolean;
 
   /**
-   * True if:
-   *   - current != target and we couldn't automatically resolve the conflict between them
-   *
-   * False if:
-   *   - current == target (value won't change)
-   *   - current != target && current == base (stock rule will get a new value)
-   *   - current != target and we automatically resolved the conflict between them
+   * Enum of possible conflict outcomes of a three-way diff.
    */
-  has_conflict: boolean;
+  conflict: ThreeWayDiffConflict;
 }
 
 /**

x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff_conflict.ts

@@ -0,0 +1,21 @@
+/*
+ * 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.
+ */
+
+/**
+ * Enum of possible conflict outcomes of a three-way diff:
+ *   - NON_SOLVABLE_CONFLICT: current != target and we couldn't automatically resolve the conflict between them
+ *   - SOLVABLE_CONFLICT: current != target and we automatically resolved the conflict between them
+ *   - NO_CONFLICT:
+ *       - current == target (value won't change)
+ *       - current != target && current == base (stock rule will get a new value)
+ * See RFC: https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/docs/rfcs/detection_response/prebuilt_rules_customization.md#concrete-field-diff-algorithms-by-type
+ */
+export enum ThreeWayDiffConflict {
+  NONE = 'NONE',
+  SOLVABLE = 'SOLVABLE',
+  NON_SOLVABLE = 'NON_SOLVABLE',
+}

x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff_outcome.ts

@@ -27,6 +27,12 @@ export enum ThreeWayDiffOutcome {
 
   /** Customized rule, the value has changed in the target version and is not equal to the current version. */
   CustomizedValueCanUpdate = 'BASE=A, CURRENT=B, TARGET=C',
+
+  /** Missing  base, the value hasn't changed in the target version. */
+  MissingBaseNoUpdate = 'BASE=-, CURRENT=A, TARGET=A',
+
+  /** Missing  base, the value changed in the target version. */
+  MissingBaseCanUpdate = 'BASE=-, CURRENT=A, TARGET=B',
 }
 
 export const determineDiffOutcome = <TValue>(
@@ -85,12 +91,12 @@ const getThreeWayDiffOutcome = ({
   if (!hasBaseVersion) {
     /**
      * We couldn't find the base version of the rule in the package so further
-     * version comparison is not possible. We assume that the rule is not
+     * version comparison is not possible. We assume that the rule is
      * customized and the value can be updated if there's an update.
      */
     return currentEqlTarget
-      ? ThreeWayDiffOutcome.StockValueNoUpdate
-      : ThreeWayDiffOutcome.StockValueCanUpdate;
+      ? ThreeWayDiffOutcome.MissingBaseNoUpdate
+      : ThreeWayDiffOutcome.MissingBaseCanUpdate;
   }
 
   if (baseEqlCurrent) {
@@ -111,6 +117,7 @@ const getThreeWayDiffOutcome = ({
 export const determineIfValueCanUpdate = (diffCase: ThreeWayDiffOutcome): boolean => {
   return (
     diffCase === ThreeWayDiffOutcome.StockValueCanUpdate ||
-    diffCase === ThreeWayDiffOutcome.CustomizedValueCanUpdate
+    diffCase === ThreeWayDiffOutcome.CustomizedValueCanUpdate ||
+    diffCase === ThreeWayDiffOutcome.MissingBaseCanUpdate
   );
 };

x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_merge_outcome.ts

@@ -6,10 +6,10 @@
  */
 
 /**
- * Type of result of an automatic three-way merge of three values:
- *   - base version
+ * The type of result of an automatic three-way merge of three values:
  *   - current version
  *   - target version
+ *   - merged version
  */
 export enum ThreeWayMergeOutcome {
   /** Took current version and returned as the merged one. */
@@ -20,7 +20,4 @@ export enum ThreeWayMergeOutcome {
 
   /** Merged three versions successfully into a new one. */
   Merged = 'MERGED',
-
-  /** Couldn't merge three versions because of a conflict. */
-  Conflict = 'CONFLICT',
 }

x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/index.ts

@@ -12,4 +12,4 @@ export * from './diff/rule_diff/fields_diff';
 export * from './diff/rule_diff/rule_diff';
 export * from './diff/three_way_diff/three_way_diff_outcome';
 export * from './diff/three_way_diff/three_way_diff';
-export * from './diff/three_way_diff/three_way_merge_outcome';
+export * from './diff/three_way_diff/three_way_diff_conflict';

x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/review_rule_upgrade/review_rule_upgrade_route.ts

@@ -21,6 +21,12 @@ export interface RuleUpgradeStatsForReview {
   /** Number of installed prebuilt rules available for upgrade (stock + customized) */
   num_rules_to_upgrade_total: number;
 
+  /** Number of installed prebuilt rules with upgrade conflicts (SOLVABLE or NON_SOLVALBE) */
+  num_rules_with_conflicts: number;
+
+  /** Number of installed prebuilt rules with NON_SOLVABLE upgrade conflicts */
+  num_rules_with_non_solvable_conflicts: number;
+
   /** A union of all tags of all rules available for upgrade */
   tags: RuleTagArray;
 }

x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/helpers.ts

@@ -52,7 +52,8 @@ export const filterUnsupportedDiffOutcomes = (
       const diff = value as ThreeWayDiff<unknown>;
       return (
         diff.diff_outcome !== ThreeWayDiffOutcome.CustomizedValueNoUpdate &&
-        diff.diff_outcome !== ThreeWayDiffOutcome.CustomizedValueSameUpdate
+        diff.diff_outcome !== ThreeWayDiffOutcome.CustomizedValueSameUpdate &&
+        diff.diff_outcome !== ThreeWayDiffOutcome.MissingBaseNoUpdate
       );
     })
   );

x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_rule_diff_tab.test.tsx

@@ -7,6 +7,7 @@
 
 import {
   KqlQueryType,
+  ThreeWayDiffConflict,
   ThreeWayDiffOutcome,
   ThreeWayMergeOutcome,
 } from '../../../../../common/api/detection_engine';
@@ -18,8 +19,9 @@ import { PerFieldRuleDiffTab } from './per_field_rule_diff_tab';
 
 const ruleFieldsDiffBaseFieldsMock = {
   diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate,
-  has_conflict: false,
+  conflict: ThreeWayDiffConflict.NONE,
   has_update: true,
+  has_base_version: true,
   merge_outcome: ThreeWayMergeOutcome.Target,
 };
 
@@ -33,7 +35,9 @@ const ruleFieldsDiffMock: PartialRuleDiff = {
       target_version: 2,
     },
   },
-  has_conflict: false,
+  num_fields_with_updates: 1,
+  num_fields_with_conflicts: 0,
+  num_fields_with_non_solvable_conflicts: 0,
 };
 
 const renderPerFieldRuleDiffTab = (ruleDiff: PartialRuleDiff) => {

x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/review_rule_upgrade/review_rule_upgrade_route.ts

@@ -87,12 +87,34 @@ export const reviewRuleUpgradeRoute = (router: SecuritySolutionPluginRouter) =>
 };
 
 const calculateRuleStats = (results: CalculateRuleDiffResult[]): RuleUpgradeStatsForReview => {
-  const allTags = new Set<string>(
-    results.flatMap((result) => result.ruleVersions.input.current?.tags ?? [])
+  const allTags = new Set<string>();
+
+  const stats = results.reduce(
+    (acc, result) => {
+      acc.num_rules_to_upgrade_total += 1;
+
+      if (result.ruleDiff.num_fields_with_conflicts > 0) {
+        acc.num_rules_with_conflicts += 1;
+      }
+
+      if (result.ruleDiff.num_fields_with_non_solvable_conflicts > 0) {
+        acc.num_rules_with_non_solvable_conflicts += 1;
+      }
+
+      result.ruleVersions.input.current?.tags.forEach((tag) => allTags.add(tag));
+
+      return acc;
+    },
+    {
+      num_rules_to_upgrade_total: 0,
+      num_rules_with_conflicts: 0,
+      num_rules_with_non_solvable_conflicts: 0,
+    }
   );
+
   return {
-    num_rules_to_upgrade_total: results.length,
-    tags: [...allTags].sort((a, b) => a.localeCompare(b)),
+    ...stats,
+    tags: Array.from(allTags),
   };
 };
 
@@ -123,9 +145,13 @@ const calculateRuleInfos = (results: CalculateRuleDiffResult[]): RuleUpgradeInfo
       diff: {
         fields: pickBy<ThreeWayDiff<unknown>>(
           ruleDiff.fields,
-          (fieldDiff) => fieldDiff.diff_outcome !== ThreeWayDiffOutcome.StockValueNoUpdate
+          (fieldDiff) =>
+            fieldDiff.diff_outcome !== ThreeWayDiffOutcome.StockValueNoUpdate &&
+            fieldDiff.diff_outcome !== ThreeWayDiffOutcome.MissingBaseNoUpdate
         ),
-        has_conflict: ruleDiff.has_conflict,
+        num_fields_with_updates: ruleDiff.num_fields_with_updates,
+        num_fields_with_conflicts: ruleDiff.num_fields_with_conflicts,
+        num_fields_with_non_solvable_conflicts: ruleDiff.num_fields_with_non_solvable_conflicts,
       },
     };
   });

x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculate_rule_diff.ts

@@ -9,8 +9,12 @@ import type {
   DiffableRule,
   FullRuleDiff,
   ThreeWayDiff,
+  RuleFieldsDiff,
+} from '../../../../../../common/api/detection_engine/prebuilt_rules';
+import {
+  MissingVersion,
+  ThreeWayDiffConflict,
 } from '../../../../../../common/api/detection_engine/prebuilt_rules';
-import { MissingVersion } from '../../../../../../common/api/detection_engine/prebuilt_rules';
 import type { RuleResponse } from '../../../../../../common/api/detection_engine/model/rule_schema';
 import { invariant } from '../../../../../../common/utils/invariant';
 import type { PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';
@@ -75,14 +79,18 @@ export const calculateRuleDiff = (args: RuleVersions): CalculateRuleDiffResult =
     target_version: diffableTargetVersion,
   });
 
-  const hasAnyFieldConflict = Object.values<ThreeWayDiff<unknown>>(fieldsDiff).some(
-    (fieldDiff) => fieldDiff.has_conflict
-  );
+  const {
+    numberFieldsWithUpdates,
+    numberFieldsWithConflicts,
+    numberFieldsWithNonSolvableConflicts,
+  } = getNumberOfFieldsByChangeType(fieldsDiff);
 
   return {
     ruleDiff: {
       fields: fieldsDiff,
-      has_conflict: hasAnyFieldConflict,
+      num_fields_with_updates: numberFieldsWithUpdates,
+      num_fields_with_conflicts: numberFieldsWithConflicts,
+      num_fields_with_non_solvable_conflicts: numberFieldsWithNonSolvableConflicts,
     },
     ruleVersions: {
       input: {
@@ -98,3 +106,31 @@ export const calculateRuleDiff = (args: RuleVersions): CalculateRuleDiffResult =
     },
   };
 };
+
+const getNumberOfFieldsByChangeType = (fieldsDiff: RuleFieldsDiff) =>
+  Object.values<ThreeWayDiff<unknown>>(fieldsDiff).reduce<{
+    numberFieldsWithUpdates: number;
+    numberFieldsWithConflicts: number;
+    numberFieldsWithNonSolvableConflicts: number;
+  }>(
+    (counts, fieldDiff) => {
+      if (fieldDiff.has_update) {
+        counts.numberFieldsWithUpdates += 1;
+      }
+
+      if (fieldDiff.conflict !== ThreeWayDiffConflict.NONE) {
+        counts.numberFieldsWithConflicts += 1;
+
+        if (fieldDiff.conflict === ThreeWayDiffConflict.NON_SOLVABLE) {
+          counts.numberFieldsWithNonSolvableConflicts += 1;
+        }
+      }
+
+      return counts;
+    },
+    {
+      numberFieldsWithUpdates: 0,
+      numberFieldsWithConflicts: 0,
+      numberFieldsWithNonSolvableConflicts: 0,
+    }
+  );