Skip to content

Commit

Permalink
[8.16] [Security Solution] Remove `exceptions_list`, `…
Browse files Browse the repository at this point in the history
…author` and `license` from Diffable Rule (#196561) (#196903)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Security Solution] Remove `exceptions_list`,
`author` and `license` from Diffable Rule
(#196561)](#196561)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Juan Pablo
Djeredjian","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-18T14:50:36Z","message":"[Security
Solution] Remove `exceptions_list`, `author` and `license` from Diffable
Rule (#196561)\n\nResolves:
https://github.com/elastic/kibana/issues/196213\r\n\r\n##
Summary\r\n\r\nExcludes the fields `exceptions_list`, `author` and
`license` from the\r\n`DiffableRule` definition.\r\n\r\nThis
will:\r\n\r\n- prevent them from appearing in the Three Way Diff
component\r\n- prevent them from being able to be passed as a value in
the `fields`\r\nobject of the `/upgrade/_perform` endpoint to set a
specific\r\n`pick_version` for it (NOTE: the current logic already
forces\r\n`exceptions_list` to upgrade to the CURRENT version, but
removing it\r\nfrom DiffableRule, will completely remove the from the
payload schema,\r\nand the endpoint will then throw a validation error
if included, rather\r\nthan silently ignoring it)\r\n\r\n##
Screenshots\r\n\r\n###
Before\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/aacd0b43-bb29-46d0-990d-c669224c1451)\r\n\r\n\r\n###
After\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/e568ca7f-03fc-42d6-8879-d3f23558ae9d)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"716fdb23c7d42c9f7c29525af793ff1594ad67f0","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Prebuilt Detection
Rules","backport:prev-minor","v8.16.0"],"title":"[Security Solution]
Remove `exceptions_list`, `author` and `license` from Diffable
Rule","number":196561,"url":"https://github.com/elastic/kibana/pull/196561","mergeCommit":{"message":"[Security
Solution] Remove `exceptions_list`, `author` and `license` from Diffable
Rule (#196561)\n\nResolves:
https://github.com/elastic/kibana/issues/196213\r\n\r\n##
Summary\r\n\r\nExcludes the fields `exceptions_list`, `author` and
`license` from the\r\n`DiffableRule` definition.\r\n\r\nThis
will:\r\n\r\n- prevent them from appearing in the Three Way Diff
component\r\n- prevent them from being able to be passed as a value in
the `fields`\r\nobject of the `/upgrade/_perform` endpoint to set a
specific\r\n`pick_version` for it (NOTE: the current logic already
forces\r\n`exceptions_list` to upgrade to the CURRENT version, but
removing it\r\nfrom DiffableRule, will completely remove the from the
payload schema,\r\nand the endpoint will then throw a validation error
if included, rather\r\nthan silently ignoring it)\r\n\r\n##
Screenshots\r\n\r\n###
Before\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/aacd0b43-bb29-46d0-990d-c669224c1451)\r\n\r\n\r\n###
After\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/e568ca7f-03fc-42d6-8879-d3f23558ae9d)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"716fdb23c7d42c9f7c29525af793ff1594ad67f0"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196561","number":196561,"mergeCommit":{"message":"[Security
Solution] Remove `exceptions_list`, `author` and `license` from Diffable
Rule (#196561)\n\nResolves:
https://github.com/elastic/kibana/issues/196213\r\n\r\n##
Summary\r\n\r\nExcludes the fields `exceptions_list`, `author` and
`license` from the\r\n`DiffableRule` definition.\r\n\r\nThis
will:\r\n\r\n- prevent them from appearing in the Three Way Diff
component\r\n- prevent them from being able to be passed as a value in
the `fields`\r\nobject of the `/upgrade/_perform` endpoint to set a
specific\r\n`pick_version` for it (NOTE: the current logic already
forces\r\n`exceptions_list` to upgrade to the CURRENT version, but
removing it\r\nfrom DiffableRule, will completely remove the from the
payload schema,\r\nand the endpoint will then throw a validation error
if included, rather\r\nthan silently ignoring it)\r\n\r\n##
Screenshots\r\n\r\n###
Before\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/aacd0b43-bb29-46d0-990d-c669224c1451)\r\n\r\n\r\n###
After\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/e568ca7f-03fc-42d6-8879-d3f23558ae9d)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"716fdb23c7d42c9f7c29525af793ff1594ad67f0"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Juan Pablo Djeredjian <[email protected]>
  • Loading branch information
kibanamachine and jpdjere authored Oct 18, 2024
1 parent 0bbb55e commit 568cfe8
Show file tree
Hide file tree
Showing 15 changed files with 22 additions and 209 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,8 @@ import {
RequiredFieldArray,
RiskScore,
RiskScoreMapping,
RuleAuthorArray,
RuleDescription,
RuleExceptionList,
RuleFalsePositiveArray,
RuleLicense,
RuleName,
RuleReferenceArray,
RuleSignatureId,
Expand Down Expand Up @@ -82,12 +79,9 @@ export const DiffableCommonFields = z.object({
setup: SetupGuide,
related_integrations: RelatedIntegrationArray,
required_fields: RequiredFieldArray,
author: RuleAuthorArray,
license: RuleLicense,

// Other domain fields
rule_schedule: RuleSchedule, // NOTE: new field
exceptions_list: z.array(RuleExceptionList),
max_signals: MaxSignals,

// Optional fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,18 @@ export const FIELDS_TO_UPGRADE_TO_CURRENT_VERSION = [
'items_per_search',
] as const;

export const NON_UPGRADEABLE_DIFFABLE_FIELDS = [
export const FIELDS_TO_UPGRADE_TO_TARGET_VERSION = [
'type',
'rule_id',
'version',
'author',
'license',
] as const;

// Fields which are part of DiffableRule but are not upgradeable
// and need to be omittted from the DiffableUpgradableFields
export const NON_UPGRADEABLE_DIFFABLE_FIELDS = ['type', 'rule_id', 'version'] as const;

type NON_UPGRADEABLE_DIFFABLE_FIELDS_TO_OMIT_TYPE = {
readonly [key in (typeof NON_UPGRADEABLE_DIFFABLE_FIELDS)[number]]: true;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,9 @@ const extractDiffableCommonFields = (
setup: rule.setup ?? '',
related_integrations: rule.related_integrations ?? [],
required_fields: addEcsToRequiredFields(rule.required_fields),
author: rule.author ?? [],
license: rule.license ?? '',

// Other domain fields
rule_schedule: extractRuleSchedule(rule),
exceptions_list: rule.exceptions_list ?? [],
max_signals: rule.max_signals ?? DEFAULT_MAX_SIGNALS,

// --------------------- OPTIONAL FIELDS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export const ABOUT_UPGRADE_FIELD_ORDER: Array<keyof DiffableAllFields> = [
'version',
'name',
'description',
'author',
'building_block',
'investigation_fields',
'severity',
Expand All @@ -23,7 +22,6 @@ export const ABOUT_UPGRADE_FIELD_ORDER: Array<keyof DiffableAllFields> = [
'risk_score_mapping',
'references',
'false_positives',
'license',
'rule_name_override',
'threat',
'threat_indicator_path',
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ import { NameReadOnly } from './fields/name/name';
import { TagsReadOnly } from './fields/tags/tags';
import { DescriptionReadOnly } from './fields/description/description';
import { assertUnreachable } from '../../../../../../../common/utility_types';
import { AuthorReadOnly } from './fields/author/author';
import { BuildingBlockReadOnly } from './fields/building_block/building_block';
import { InvestigationFieldsReadOnly } from './fields/investigation_fields/investigation_fields';
import { FalsePositivesReadOnly } from './fields/false_positives/false_positives';
import { LicenseReadOnly } from './fields/license/license';
import { MaxSignalsReadOnly } from './fields/max_signals/max_signals';
import { NoteReadOnly } from './fields/note/note';
import { RuleScheduleReadOnly } from './fields/rule_schedule/rule_schedule';
Expand All @@ -46,23 +44,16 @@ export function CommonRuleFieldReadOnly({
finalDiffableRule,
}: CommonRuleFieldReadOnlyProps) {
switch (fieldName) {
case 'author':
return <AuthorReadOnly author={finalDiffableRule.author} />;
case 'building_block':
return <BuildingBlockReadOnly />;
case 'description':
return <DescriptionReadOnly description={finalDiffableRule.description} />;
case 'exceptions_list':
/* Exceptions are not used in prebuilt rules */
return null;
case 'investigation_fields':
return (
<InvestigationFieldsReadOnly investigationFields={finalDiffableRule.investigation_fields} />
);
case 'false_positives':
return <FalsePositivesReadOnly falsePositives={finalDiffableRule.false_positives} />;
case 'license':
return <LicenseReadOnly license={finalDiffableRule.license} />;
case 'max_signals':
return <MaxSignalsReadOnly maxSignals={finalDiffableRule.max_signals} />;
case 'name':
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,10 @@ const commonDiffableRuleFields: DiffableCommonFields = {
setup: '',
related_integrations: [],
required_fields: [],
author: [],
license: '',

rule_schedule: {
interval: '5m',
lookback: '360s',
},
exceptions_list: [],
max_signals: DEFAULT_MAX_SIGNALS,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { DiffableCommonFields } from '../../../../../common/api/detection_engine';
import type {
DiffableCustomQueryFields,
DiffableEqlFields,
DiffableEsqlFields,
DiffableMachineLearningFields,
DiffableNewTermsFields,
DiffableSavedQueryFields,
DiffableThreatMatchFields,
DiffableThresholdFields,
RuleFieldsDiff,
import {
DiffableCommonFields,
NON_UPGRADEABLE_DIFFABLE_FIELDS,
type DiffableCustomQueryFields,
type DiffableEqlFields,
type DiffableEsqlFields,
type DiffableMachineLearningFields,
type DiffableNewTermsFields,
type DiffableSavedQueryFields,
type DiffableThreatMatchFields,
type DiffableThresholdFields,
type RuleFieldsDiff,
} from '../../../../../common/api/detection_engine';

export type NonUpgradeableDiffableFields = (typeof NON_UPGRADEABLE_DIFFABLE_FIELDS)[number];
Expand Down Expand Up @@ -61,14 +61,6 @@ export type UpgradeableNewTermsFields = Exclude<
NonUpgradeableDiffableFields
>;

export const NON_UPGRADEABLE_DIFFABLE_FIELDS = [
'author',
'license',
'rule_id',
'type',
'version',
] as const;

export const COMMON_FIELD_NAMES = DiffableCommonFields.keyof().options;

export function isCommonFieldName(fieldName: string): fieldName is keyof DiffableCommonFields {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import {
FIELDS_TO_UPGRADE_TO_CURRENT_VERSION,
NON_UPGRADEABLE_DIFFABLE_FIELDS,
FIELDS_TO_UPGRADE_TO_TARGET_VERSION,
} from '../../../../../../common/api/detection_engine';
import { type PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';
import type { RuleTriad } from '../../model/rule_groups/get_rule_groups';
Expand All @@ -24,10 +24,10 @@ type GetFieldPredefinedValueReturnType =
* a predefined value or is customizable), and returns the value if it is predefined.
*
* This function checks whether a field can be upgraded via API contract and how it should
* be handled during the rule upgrade process. It uses the `NON_UPGRADEABLE_DIFFABLE_FIELDS` and
* be handled during the rule upgrade process. It uses the `FIELDS_TO_UPGRADE_TO_TARGET_VERSION` and
* `FIELDS_TO_UPGRADE_TO_CURRENT_VERSION` constants to make this determination.
*
* `NON_UPGRADEABLE_DIFFABLE_FIELDS` includes fields that are not upgradeable: 'type', 'rule_id',
* `FIELDS_TO_UPGRADE_TO_TARGET_VERSION` includes fields that are not upgradeable: 'type', 'rule_id',
* 'version', 'author', and 'license', and are always upgraded to the target version.
*
* `FIELDS_TO_UPGRADE_TO_CURRENT_VERSION` includes fields that should be updated to their
Expand All @@ -46,8 +46,8 @@ export const getFieldPredefinedValue = (
upgradeableRule: RuleTriad
): GetFieldPredefinedValueReturnType => {
if (
NON_UPGRADEABLE_DIFFABLE_FIELDS.includes(
fieldName as (typeof NON_UPGRADEABLE_DIFFABLE_FIELDS)[number]
FIELDS_TO_UPGRADE_TO_TARGET_VERSION.includes(
fieldName as (typeof FIELDS_TO_UPGRADE_TO_TARGET_VERSION)[number]
)
) {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,7 @@ const commonFieldsDiffAlgorithms: FieldsDiffAlgorithmsFor<DiffableCommonFields>
setup: multiLineStringDiffAlgorithm,
related_integrations: simpleDiffAlgorithm,
required_fields: simpleDiffAlgorithm,
author: scalarArrayDiffAlgorithm,
license: singleLineStringDiffAlgorithm,
rule_schedule: simpleDiffAlgorithm,
exceptions_list: simpleDiffAlgorithm,
max_signals: numberDiffAlgorithm,
rule_name_override: simpleDiffAlgorithm,
timestamp_override: simpleDiffAlgorithm,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,6 @@ export default ({ getService }: FtrProviderContext): void => {

// Create resolved values different from current values
const resolvedValues: { [key: string]: unknown } = {
exceptions_list: [],
alert_suppression: {
group_by: ['test'],
duration: { value: 10, unit: 'm' as const },
Expand Down

0 comments on commit 568cfe8

Please sign in to comment.