Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Remove exceptions_list, author and license from Diffable Rule #196561

Merged
merged 11 commits into from
Oct 18, 2024
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,13 +40,7 @@ export const FIELDS_TO_UPGRADE_TO_CURRENT_VERSION = [
'items_per_search',
] as const;

export const NON_UPGRADEABLE_DIFFABLE_FIELDS = [
'type',
'rule_id',
'version',
'author',
'license',
] as const;
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,4 @@ import type { DiffableAllFields } from '../../../../../../../common/api/detectio
type NonEditableFields = Readonly<Set<keyof DiffableAllFields>>;

/* These fields are not visible in the comparison UI and are not editable */
export const HIDDEN_FIELDS: NonEditableFields = new Set([
'alert_suppression',
Copy link
Contributor Author

@jpdjere jpdjere Oct 16, 2024

Choose a reason for hiding this comment

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

@nikitaindik alert_suppression actually needs to be displayed in the diff and be able to be customized by the user. Can you make a note of that for the work that you and Maxim are currently doing?

Copy link
Contributor

@nikitaindik nikitaindik Oct 17, 2024

Choose a reason for hiding this comment

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

Will do.

Actually, can you please remove this whole file since these constants are not used anywhere. It's a duplicate of this constant. I thought I removed it but it looks like it got resurrected during a merge.

I wonder if I can just reuse your FIELDS_TO_UPGRADE_TO_TARGET_VERSION constant to hide non-editable fields from the "left side / right side" in UI?

'author',
'rule_id',
'license',
'version',
]);
export const HIDDEN_FIELDS: NonEditableFields = new Set(['rule_id', 'version']);
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nikitaindik Deleted the author and license components since they won't be needed, and the DiffableRule type doesn't allow it anymore anyways. Let me know if this makes sense.

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