Skip to content

Commit

Permalink
[Security Solution] Rule upgrade JSON diff: Hide runtime and internal…
Browse files Browse the repository at this point in the history
… properties (elastic#174789)

**Resolves: elastic#174844

## Summary
Hides technical/runtime fields that shouldn't be displayed in the JSON
diff view.
We used to hide only the `revision` field because it can be confused
with `version`. This PR hides more fields.

Properties that might be displayed as having diff, but shouldn't:
- `actions`: shown as diff if user defined an action for a rule
- `exceptions_list`: shown as diff if user defined an exception list for
a rule
- `execution_summary`: shown as diff if user has enabled a rule and it
executed at least once
- `enabled`: shown as diff if user enabled a rule that's disabled by
default (or vice versa)
- `updated_at`: always shown as diff because its value is a timestamp of
when an API request made
- `from`: might be shown as diff if user has clicked "save" after
editing a rule, because edit screen's FE code converts value to a
different time unit, like 2h -> 7200s
- `note`: shown as diff if an old version of a rule didn't define this
property, but a new version of a rule has it defined as ''
- `threat`: might be shown as diff if user has clicked "save" after
editing a rule, because edit screen's FE code adds empty arrays as
defaults if threats/techniques/subtechniques weren't set by the user
- `machine_learning_job_id`: might be shown as diff if a prebuilt rule
uses the legacy string format for this property. On installation, the
value is converted into a new array format, which creates a difference
between the installed rule (array format) and the update (string format)
- `threat_filters`: might be shown as diff if user has clicked "save"
after editing a rule, because edit screen's FE code adds null as a
default value for "meta" subproperty
- `filters`: might be shown as diff if user has clicked "save" after
editing a rule, because edit screen's FE code adds [] as a default value
- `timestamp_override_fallback_disabled`: might be shown as diff if user
has clicked "save" after editing a rule
- `meta`: might be shown as diff if user has clicked "save" after
editing a rule
- `output_index`: unused, shouldn't be shown
- `updated_at`, `updated_by`, `created_at`, `created_by`: should be
hidden because these are not relevant for the upgrade flow

#### Before
<img width="1271" alt="Scherm­afbeelding 2024-01-16 om 13 50 00"
src="https://github.com/elastic/kibana/assets/15949146/f72283dc-9a8a-4c95-a9b6-daa84d9356da">

#### After
<img width="1271" alt="Scherm­afbeelding 2024-01-16 om 13 50 36"
src="https://github.com/elastic/kibana/assets/15949146/080ef2ea-c108-4d05-8814-0a2ce7f5a0b0">

(cherry picked from commit 5bf935b)
  • Loading branch information
nikitaindik committed Jan 25, 2024
1 parent 4dc81df commit 882136b
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React, { useMemo } from 'react';
import { omit } from 'lodash';
import { omit, pick } from 'lodash';
import stringify from 'json-stable-stringify';
import {
EuiSpacer,
Expand All @@ -16,22 +16,125 @@ import {
EuiTitle,
EuiIconTip,
} from '@elastic/eui';
import type { Filter } from '@kbn/es-query';
import { normalizeMachineLearningJobIds } from '../../../../../common/detection_engine/utils';
import {
formatScheduleStepData,
filterEmptyThreats,
} from '../../../rule_creation_ui/pages/rule_creation/helpers';
import type { RuleResponse } from '../../../../../common/api/detection_engine/model/rule_schema/rule_schemas.gen';
import { DiffView } from './json_diff/diff_view';
import * as i18n from './json_diff/translations';
import { getHumanizedDuration } from '../../../../detections/pages/detection_engine/rules/helpers';

/* Inclding these properties in diff display might be confusing to users. */
const HIDDEN_PROPERTIES = [
/*
By default, prebuilt rules don't have any actions or exception lists. So if a user has defined actions or exception lists for a rule, it'll show up as diff. This looks confusing as the user might think that their actions and exceptions lists will get removed after the upgrade, which is not the case - they will be preserved.
*/
'actions',
'exceptions_list',

/*
Most prebuilt rules are installed as "disabled". Once user enables a rule, it'll show as diff, which doesn't add value.
*/
'enabled',

/* Technical property. Hidden because it can be confused with "version" by users. */
'revision',

/*
"updated_at" value is regenerated on every '/upgrade/_review' endpoint run
and will therefore always show a diff. It adds no value to display it to the user.
*/
'updated_at',
];

const sortAndStringifyJson = (jsObject: Record<string, unknown>): string =>
stringify(jsObject, { space: 2 });

/**
* Normalizes the rule object, making it suitable for comparison with another normalized rule.
*
* Normalization is needed to avoid showing diffs for semantically equal property values.
* Saving a rule via the rule editing UI might change the format of some properties or set default values.
* This function compensates for those changes.
*
* @param {RuleResponse} originalRule - Rule of a RuleResponse type.
* @returns {RuleResponse} - The updated normalized rule object.
*/
const normalizeRule = (originalRule: RuleResponse): RuleResponse => {
const rule = { ...originalRule };

/*
Convert the "from" property value to a humanized duration string, like 'now-1m' or 'now-2h'.
Conversion is needed to skip showing the diff for the "from" property when the same
duration is represented in different time units. For instance, 'now-1h' and 'now-3600s'
indicate a one-hour duration.
The same helper is used in the rule editing UI to format "from" before submitting the edits.
So, after the rule is saved, the "from" property unit/value might differ from what's in the package.
*/
rule.from = formatScheduleStepData({
interval: rule.interval,
from: getHumanizedDuration(rule.from, rule.interval),
to: rule.to,
}).from;

/*
Default "note" to an empty string if it's not present.
Sometimes, in a new version of a rule, the "note" value equals an empty string, while
in the old version, it wasn't specified at all (undefined becomes ''). In this case,
it doesn't make sense to show diff, so we default falsy values to ''.
*/
rule.note = rule.note ?? '';

/*
Removes empty arrays (techniques, subtechniques) from the MITRE ATT&CK value.
The same processing is done in the rule editing UI before submitting the edits.
*/
rule.threat = filterEmptyThreats(rule.threat);

/*
The "machine_learning_job_id" property is converted from the legacy string format
to the new array format during installation and upgrade. Thus, all installed rules
use the new format. For correct comparison, we must ensure that the rule update is
also in the new format before showing the diff.
*/
if ('machine_learning_job_id' in rule) {
rule.machine_learning_job_id = normalizeMachineLearningJobIds(rule.machine_learning_job_id);
}

/*
Default the "alias" property to null for all threat filters that don't have it.
Setting a default is needed to match the behavior of the rule editing UI,
which also defaults the "alias" property to null.
*/
if (rule.type === 'threat_match' && Array.isArray(rule.threat_filters)) {
const threatFiltersWithDefaultMeta = (rule.threat_filters as Filter[]).map((filter) => {
if (!filter.meta.alias) {
return { ...filter, meta: { ...filter.meta, alias: null } };
}
return filter;
});

rule.threat_filters = threatFiltersWithDefaultMeta;
}

return rule;
};

interface RuleDiffTabProps {
oldRule: RuleResponse;
newRule: RuleResponse;
}

export const RuleDiffTab = ({ oldRule, newRule }: RuleDiffTabProps) => {
const [oldSource, newSource] = useMemo(() => {
const visibleOldRuleProperties = omit(oldRule, 'revision');
const visibleNewRuleProperties = omit(newRule, 'revision');
const visibleNewRuleProperties = omit(normalizeRule(newRule), ...HIDDEN_PROPERTIES);
const visibleOldRuleProperties = omit(
/* Only compare properties that are present in the update. */
pick(normalizeRule(oldRule), Object.keys(visibleNewRuleProperties))
);

return [
sortAndStringifyJson(visibleOldRuleProperties),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,7 @@ export const convertPrebuiltRuleAssetToRuleResponse = (
related_integrations: [],
required_fields: [],
setup: '',
note: '',
references: [],
threat: [],
tags: [],
Expand Down

0 comments on commit 882136b

Please sign in to comment.