Skip to content

Commit

Permalink
[Security Solution] Handle missing base versions during rule upgrade (#…
Browse files Browse the repository at this point in the history
…154571)

**Resolves: #152836
**Resolves: #148183
**Resolves partially: #148189
  • Loading branch information
xcrzx authored Apr 19, 2023
1 parent f8c16c1 commit 4e5ad09
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ export interface GetPrebuiltRulesStatusResponseBody {
}

export interface PrebuiltRulesStatusStats {
/** Total number of existing (known) prebuilt rules */
num_prebuilt_rules_total: number;

/** Number of installed prebuilt rules */
num_prebuilt_rules_installed: number;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,8 @@ export interface RuleUpgradeStatsForReview {
/** Number of installed prebuilt rules available for upgrade (stock + customized) */
num_rules_to_upgrade_total: number;

/** Number of installed prebuilt rules available for upgrade which are stock (non-customized) */
num_rules_to_upgrade_not_customized: number;

/** Number of installed prebuilt rules available for upgrade which are customized by the user */
num_rules_to_upgrade_customized: number;

/** A union of all tags of all rules available for upgrade */
tags: RuleTagArray;

/** A union of all fields "to be upgraded" across all the rules available for upgrade. An array of field names. */
fields: string[];
}

export interface RuleUpgradeInfoForReview {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,26 @@
import type { ThreeWayDiffOutcome } from './three_way_diff_outcome';
import type { ThreeWayMergeOutcome } from './three_way_merge_outcome';

/**
* A symbol that represents a missing value and used when a base version of a
* rule is not available. We need a mechanism that helps us distinguish two
* situations:
* - the base version is found, and its value is `undefined` or `null`
* - the base version is not found
*
*/
export const MissingVersion = Symbol('MissingVersion');
export type MissingVersion = typeof MissingVersion;

/**
* Three versions of a value to pass to a diff algorithm.
*/
export interface ThreeVersionsOf<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.
*/
base_version: TValue;
base_version: TValue | MissingVersion;

/**
* Corresponds exactly to the currently installed prebuilt rule:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { isEqual } from 'lodash';
import { MissingVersion } from './three_way_diff';

/**
* Result of comparing three versions of a value against each other.
Expand All @@ -29,14 +30,25 @@ export enum ThreeWayDiffOutcome {
}

export const determineDiffOutcome = <TValue>(
baseVersion: TValue,
baseVersion: TValue | MissingVersion,
currentVersion: TValue,
targetVersion: TValue
): ThreeWayDiffOutcome => {
const baseEqlCurrent = isEqual(baseVersion, currentVersion);
const baseEqlTarget = isEqual(baseVersion, targetVersion);
const currentEqlTarget = isEqual(currentVersion, targetVersion);

if (baseVersion === MissingVersion) {
/**
* 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
* customized and the value can be updated if there's an update.
*/
return currentEqlTarget
? ThreeWayDiffOutcome.StockValueNoUpdate
: ThreeWayDiffOutcome.StockValueCanUpdate;
}

if (baseEqlCurrent) {
return currentEqlTarget
? ThreeWayDiffOutcome.StockValueNoUpdate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,9 @@ export const getPrebuiltRulesStatusRoute = (router: SecuritySolutionPluginRouter
};

const calculateRuleStats = (buckets: VersionBuckets): PrebuiltRulesStatusStats => {
const { latestVersions, installedVersions, latestVersionsToInstall, installedVersionsToUpgrade } =
buckets;
const { installedVersions, latestVersionsToInstall, installedVersionsToUpgrade } = buckets;

return {
num_prebuilt_rules_total: latestVersions.length,
num_prebuilt_rules_installed: installedVersions.length,
num_prebuilt_rules_to_install: latestVersionsToInstall.length,
num_prebuilt_rules_to_upgrade: installedVersionsToUpgrade.length,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export const reviewRuleInstallationRoute = (router: SecuritySolutionPluginRouter

const getAggregatedTags = (rules: PrebuiltRuleAsset[]): string[] => {
const set = new Set<string>(rules.flatMap((rule) => rule.tags || []));
return Array.from(set.values());
return Array.from(set.values()).sort((a, b) => a.localeCompare(b));
};

const calculateRuleStats = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { buildSiemResponse } from '../../../routes/utils';
import { createPrebuiltRuleAssetsClient } from '../../logic/rule_assets/prebuilt_rule_assets_client';
import { createPrebuiltRuleObjectsClient } from '../../logic/rule_objects/prebuilt_rule_objects_client';
import { getVersionBuckets } from '../../model/rule_versions/get_version_buckets';
import { invariant } from '../../../../../../common/utils/invariant';

export const reviewRuleUpgradeRoute = (router: SecuritySolutionPluginRouter) => {
router.post(
Expand Down Expand Up @@ -114,28 +115,27 @@ const getRuleDiffCalculationArgs = (
const baseRule = baseRulesMap.get(ruleId);
const latestRule = latestRulesMap.get(ruleId);

// TODO: https://github.com/elastic/kibana/issues/148189
// Make base versions optional for diff calculation. We need to support this in order to be able
// to still show diffs for rule assets coming from packages without historical versions.
if (installedRule != null && baseRule != null && latestRule != null) {
result.push({
currentVersion: installedRule,
baseVersion: baseRule,
targetVersion: latestRule,
});
}
// baseRule can be undefined if the rule has no historical versions, but other versions should always be present
invariant(installedRule != null, `installedRule is not found for rule_id: ${ruleId}`);
invariant(latestRule != null, `latestRule is not found for rule_id: ${ruleId}`);

result.push({
currentVersion: installedRule,
baseVersion: baseRule,
targetVersion: latestRule,
});
});

return result;
};

const calculateRuleStats = (results: CalculateRuleDiffResult[]): RuleUpgradeStatsForReview => {
const allTags = new Set<string>(
results.flatMap((result) => result.ruleVersions.input.current.tags)
);
return {
num_rules_to_upgrade_total: results.length,
num_rules_to_upgrade_not_customized: results.length,
num_rules_to_upgrade_customized: 0,
tags: [],
fields: [],
tags: [...allTags].sort((a, b) => a.localeCompare(b)),
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import type { DiffableRule } from '../../../../../../common/detection_engine/prebuilt_rules/model/diff/diffable_rule/diffable_rule';
import type { FullRuleDiff } from '../../../../../../common/detection_engine/prebuilt_rules/model/diff/rule_diff/rule_diff';
import type { ThreeWayDiff } from '../../../../../../common/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff';
import { MissingVersion } from '../../../../../../common/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff';
import type { RuleResponse } from '../../../../../../common/detection_engine/rule_schema';
import type { PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';

Expand All @@ -16,7 +17,7 @@ import { convertRuleToDiffable } from './normalization/convert_rule_to_diffable'

export interface CalculateRuleDiffArgs {
currentVersion: RuleResponse;
baseVersion: PrebuiltRuleAsset;
baseVersion?: PrebuiltRuleAsset;
targetVersion: PrebuiltRuleAsset;
}

Expand All @@ -25,12 +26,12 @@ export interface CalculateRuleDiffResult {
ruleVersions: {
input: {
current: RuleResponse;
base: PrebuiltRuleAsset;
base?: PrebuiltRuleAsset;
target: PrebuiltRuleAsset;
};
output: {
current: DiffableRule;
base: DiffableRule;
base?: DiffableRule;
target: DiffableRule;
};
};
Expand Down Expand Up @@ -60,12 +61,12 @@ export const calculateRuleDiff = (args: CalculateRuleDiffArgs): CalculateRuleDif

const { baseVersion, currentVersion, targetVersion } = args;

const diffableBaseVersion = convertRuleToDiffable(baseVersion);
const diffableBaseVersion = baseVersion ? convertRuleToDiffable(baseVersion) : undefined;
const diffableCurrentVersion = convertRuleToDiffable(currentVersion);
const diffableTargetVersion = convertRuleToDiffable(targetVersion);

const fieldsDiff = calculateRuleFieldsDiff({
base_version: diffableBaseVersion,
base_version: diffableBaseVersion || MissingVersion,
current_version: diffableCurrentVersion,
target_version: diffableTargetVersion,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,11 @@ export const simpleDiffAlgorithm = <TValue>(
const diffOutcome = determineDiffOutcome(baseVersion, currentVersion, targetVersion);
const valueCanUpdate = determineIfValueCanUpdate(diffOutcome);

const { mergeOutcome, mergedVersion } = mergeVersions(
baseVersion,
const { mergeOutcome, mergedVersion } = mergeVersions({
currentVersion,
targetVersion,
diffOutcome
);
diffOutcome,
});

return {
base_version: baseVersion,
Expand All @@ -54,12 +53,17 @@ interface MergeResult<TValue> {
mergedVersion: TValue;
}

const mergeVersions = <TValue>(
baseVersion: TValue,
currentVersion: TValue,
targetVersion: TValue,
diffOutcome: ThreeWayDiffOutcome
): MergeResult<TValue> => {
interface MergeArgs<TValue> {
currentVersion: TValue;
targetVersion: TValue;
diffOutcome: ThreeWayDiffOutcome;
}

const mergeVersions = <TValue>({
currentVersion,
targetVersion,
diffOutcome,
}: MergeArgs<TValue>): MergeResult<TValue> => {
switch (diffOutcome) {
case ThreeWayDiffOutcome.StockValueNoUpdate:
case ThreeWayDiffOutcome.CustomizedValueNoUpdate:
Expand Down
Loading

0 comments on commit 4e5ad09

Please sign in to comment.