Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a71d337
adds feature
dplumlee Jun 10, 2025
78348c1
Merge remote-tracking branch 'upstream/main' into prebuilt-rule-rever…
dplumlee Jun 10, 2025
3229873
Merge remote-tracking branch 'upstream/main' into prebuilt-rule-rever…
dplumlee Jun 11, 2025
44ff894
fix types
dplumlee Jun 13, 2025
e1cbace
adds unit tests
dplumlee Jun 13, 2025
a1ae0e5
Merge remote-tracking branch 'upstream/main' into prebuilt-rule-rever…
dplumlee Jun 16, 2025
69e50af
fix types
dplumlee Jun 16, 2025
6d0a3de
adds tests
dplumlee Jun 18, 2025
891e81e
Merge remote-tracking branch 'upstream/main' into prebuilt-rule-rever…
dplumlee Jun 18, 2025
e0563c8
adds threshold normalization
dplumlee Jun 19, 2025
2421863
fixes threshold normalization
dplumlee Jun 19, 2025
a326beb
Merge remote-tracking branch 'upstream/main' into prebuilt-rule-rever…
dplumlee Jun 20, 2025
0e6c999
fixes tests
dplumlee Jun 20, 2025
1a0b0fe
fixes tests again
dplumlee Jun 20, 2025
39c9683
Merge remote-tracking branch 'upstream/main' into prebuilt-rule-rever…
dplumlee Jun 22, 2025
0c4c2e2
addresses comments
dplumlee Jun 23, 2025
b39590d
fixes tests:
dplumlee Jun 24, 2025
c370d9c
fixes tests
dplumlee Jun 24, 2025
73ad4a4
Merge remote-tracking branch 'upstream/main' into prebuilt-rule-rever…
dplumlee Jun 24, 2025
3bcdfbf
adds more user friendly error handling
dplumlee Jun 24, 2025
1acdf79
remove testing code
dplumlee Jun 24, 2025
78db1b9
updates test assertions
dplumlee Jun 24, 2025
2ebde50
switches endpoint to GET
dplumlee Jun 24, 2025
917c65c
addresses non test comments
dplumlee Jun 25, 2025
0174558
adds concurrency controls
dplumlee Jun 25, 2025
f2beeea
adds tests
dplumlee Jun 25, 2025
20f5c26
Merge branch 'main' into prebuilt-rule-reversion
maximpn Jun 26, 2025
32bb30d
auto-close revert prebuilt rule flyout upon base version disappearing
maximpn Jun 26, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ import type { RuleResponse } from './rule_schemas.gen';
export function isCustomizedPrebuiltRule(rule: RuleResponse): boolean {
return rule.rule_source?.type === 'external' && rule.rule_source.is_customized;
}

export function isNonCustomizedPrebuiltRule(rule: RuleResponse): boolean {
return rule.rule_source?.type === 'external' && rule.rule_source.is_customized === false;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 { z } from '@kbn/zod';
import type { RuleResponse } from '../../model/rule_schema/rule_schemas.gen';
import type { PartialRuleDiff } from '../model';

export type GetPrebuiltRuleBaseVersionRequest = z.infer<typeof GetPrebuiltRuleBaseVersionRequest>;
export const GetPrebuiltRuleBaseVersionRequest = z.object({
id: z.string(),
});

export interface GetPrebuiltRuleBaseVersionResponseBody {
/** The base version of the rule */
base_version: RuleResponse;

/** The current version of the rule */
current_version: RuleResponse;

/** The resulting diff between the base and current versions of the rule */
diff: PartialRuleDiff;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an improvement you could return fieldsDiff: Partial<RuleFieldsDiff> and add the necessary fallback fields for example in x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/base_version_diff/base_version_flyout.tsx or in the upstream data fetching hook.

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,5 @@ 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';
export * from './common/prebuilt_rules_filter';
export * from './revert_prebuilt_rule/revert_prebuilt_rule_route';
export * from './get_prebuilt_rule_base_version/get_prebuilt_rule_base_version_route';
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* 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 { z } from '@kbn/zod';
import { RuleResponse } from '../../model/rule_schema/rule_schemas.gen';
import { NormalizedRuleError } from '../../rule_management';

export type RevertPrebuiltRulesRequest = z.infer<typeof RevertPrebuiltRulesRequest>;
export const RevertPrebuiltRulesRequest = z.object({
/** ID of rule to revert */
id: z.string(),

/** Revision of rule to guard against concurrence */
revision: z.number(),

/** Version of rule to guard against concurrence */
version: z.number(),
});

export type BulkRevertSkipReason = z.infer<typeof BulkRevertSkipReason>;
export const BulkRevertSkipReason = z.enum(['RULE_NOT_PREBUILT', 'RULE_NOT_CUSTOMIZED']);
export type BulkRevertSkipReasonEnum = typeof BulkRevertSkipReason.enum;
export const BulkRevertSkipReasonEnum = BulkRevertSkipReason.enum;

export type BulkActionReversionSkipResult = z.infer<typeof BulkActionReversionSkipResult>;
export const BulkActionReversionSkipResult = z.object({
id: z.string(),
skip_reason: BulkRevertSkipReason,
});

export type RevertPrebuiltRulesResponseBody = z.infer<typeof RevertPrebuiltRulesResponseBody>;
export const RevertPrebuiltRulesResponseBody = z.object({
success: z.boolean().optional(),
status_code: z.number().int().optional(),
message: z.string().optional(),
rules_count: z.number().int().optional(),
attributes: z.object({
results: z.object({
updated: z.array(RuleResponse), // An array of the rule objects reverted to their base version
created: z.array(RuleResponse),
deleted: z.array(RuleResponse),
skipped: z.array(BulkActionReversionSkipResult), // An array of the rule ids and reasons that were skipped during reversion (due to being already non-customized)
}),
summary: z.object({
failed: z.number().int(),
skipped: z.number().int(),
succeeded: z.number().int(),
total: z.number().int(),
}),
errors: z.array(NormalizedRuleError).optional(), // An array of error objects, something containing the id of the rule causing the error and the reason behind it (e.g. no base version, rule is not a prebuilt Elastic rule)
}),
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ export const PREBUILT_RULES_URL = LEGACY_BASE_URL;
export const PREBUILT_RULES_STATUS_URL = `${LEGACY_BASE_URL}/_status` as const;

export const GET_PREBUILT_RULES_STATUS_URL = `${BASE_URL}/status` as const;
export const GET_PREBUILT_RULES_BASE_VERSION_URL = `${BASE_URL}/base_version` as const;
export const BOOTSTRAP_PREBUILT_RULES_URL = `${BASE_URL}/_bootstrap` as const;
export const REVIEW_RULE_UPGRADE_URL = `${BASE_URL}/upgrade/_review` as const;
export const PERFORM_RULE_UPGRADE_URL = `${BASE_URL}/upgrade/_perform` as const;
export const REVIEW_RULE_INSTALLATION_URL = `${BASE_URL}/installation/_review` as const;
export const PERFORM_RULE_INSTALLATION_URL = `${BASE_URL}/installation/_perform` as const;
export const REVERT_PREBUILT_RULES_URL = `${BASE_URL}/revert` as const;
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import { extractRuleSchedule } from './extract_rule_schedule';
import { extractTimelineTemplateReference } from './extract_timeline_template_reference';
import { extractTimestampOverrideObject } from './extract_timestamp_override_object';
import { extractThreatArray } from './extract_threat_array';
import { normalizeRuleThreshold } from './normalize_rule_threshold';

/**
* Normalizes a given rule to the form which is suitable for passing to the diff algorithm.
Expand Down Expand Up @@ -224,7 +225,7 @@ const extractDiffableThresholdFieldsFromRuleObject = (
type: rule.type,
kql_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id),
data_source: extractRuleDataSource(rule.index, rule.data_view_id),
threshold: rule.threshold,
threshold: normalizeRuleThreshold(rule.threshold),
alert_suppression: rule.alert_suppression,
};
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* 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 { normalizeRuleThreshold } from './normalize_rule_threshold';

describe('normalizeRuleThreshold', () => {
it('returns threshold without cardinality field when array is empty', () => {
const normalizedField = normalizeRuleThreshold({
value: 30,
field: ['field'],
cardinality: [],
});

expect(normalizedField).toEqual({ value: 30, field: ['field'] });
});

it('returns cardinality field when it is populated', () => {
const normalizedField = normalizeRuleThreshold({
value: 30,
field: ['field'],
cardinality: [{ value: 20, field: 'field-cardinality' }],
});

expect(normalizedField).toEqual({
value: 30,
field: ['field'],
cardinality: [{ value: 20, field: 'field-cardinality' }],
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* 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 type { Threshold } from '../../../api/detection_engine/model/rule_schema';

export const normalizeRuleThreshold = (threshold: Threshold): Threshold => {
const cardinality =
threshold.cardinality && threshold.cardinality.length ? threshold.cardinality : undefined;
return {
value: threshold.value,
field: threshold.field,
cardinality,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,28 @@ export const EXPORT_RULE = i18n.translate(
}
);

export const REVERT_RULE = i18n.translate(
'xpack.securitySolution.detectionEngine.rules.allRules.actions.revertRuleDescription',
{
defaultMessage: 'Revert to Elastic version',
}
);

export const REVERT_RULE_TOOLTIP_TITLE = i18n.translate(
'xpack.securitySolution.detectionEngine.rules.allRules.actions.revertRuleTooltipTitle',
{
defaultMessage: 'Unable to revert rule',
}
);

export const REVERT_RULE_TOOLTIP_CONTENT = i18n.translate(
'xpack.securitySolution.detectionEngine.rules.allRules.actions.revertRuleTooltipContent',
{
defaultMessage:
"This rule hasn't been updated in a while and there's no available version to revert to. We recommend updating this rule instead.",
}
);

export const DELETE_RULE = i18n.translate(
'xpack.securitySolution.detectionEngine.rules.allRules.actions.deleteRuleDescription',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ import { useLegacyUrlRedirect } from './use_redirect_legacy_url';
import { RuleDetailTabs, useRuleDetailsTabs } from './use_rule_details_tabs';
import { useIsExperimentalFeatureEnabled } from '../../../../common/hooks/use_experimental_features';
import { useRuleUpdateCallout } from '../../../rule_management/hooks/use_rule_update_callout';
import { usePrebuiltRulesViewBaseDiff } from '../../../rule_management/hooks/use_prebuilt_rules_view_base_diff';

const RULE_EXCEPTION_LIST_TYPES = [
ExceptionListTypeEnum.DETECTION,
Expand Down Expand Up @@ -432,6 +433,18 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({
onUpgrade: refreshRule,
});

const {
baseVersionFlyout,
openFlyout,
doesBaseVersionExist,
isLoading: isBaseVersionLoading,
} = usePrebuiltRulesViewBaseDiff({ rule, onRevert: refreshRule });

const isRevertBaseVersionDisabled = useMemo(
() => !doesBaseVersionExist || isBaseVersionLoading,
[doesBaseVersionExist, isBaseVersionLoading]
);

const ruleStatusInfo = useMemo(() => {
return (
<>
Expand Down Expand Up @@ -608,6 +621,7 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({
<NeedAdminForUpdateRulesCallOut />
<MissingPrivilegesCallOut />
{upgradeCallout}
{baseVersionFlyout}
{isBulkDuplicateConfirmationVisible && (
<BulkActionDuplicateExceptionsConfirmation
onCancel={cancelRuleDuplication}
Expand Down Expand Up @@ -722,6 +736,8 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({
showBulkDuplicateExceptionsConfirmation={showBulkDuplicateConfirmation}
showManualRuleRunConfirmation={showManualRuleRunConfirmation}
confirmDeletion={confirmDeletion}
isRevertBaseVersionDisabled={isRevertBaseVersionDisabled}
openRuleDiffFlyout={openFlyout}
Comment on lines +739 to +740
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like inversion of control is required for RuleActionsOverflow. Increasing the number of component props has a limited capacity. In general it reduces composability, readability and maintainability. In that case we have isRevertBaseVersionDisabled and openFlyout leaking through. Though it might be simpler to use the flyout management hook under the hood.

Anyway the refactoring might be quite bulky and should happen outside of this PR.

/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Loading