Skip to content

Commit

Permalink
[Security Solution] Fixes incorrect from field transform logic in `…
Browse files Browse the repository at this point in the history
…upgrade/_perform` route (elastic#202824)

**Fixes: elastic#202575
**Fixes: elastic#201631
**Partially addresses: elastic#202715

## Summary

All bugs have the same source

> [!NOTE]  
> This bug/related fix is only visible with the
`prebuiltRulesCustomizationEnabled` feature flag turned on.

Fixes an issue where unedited prebuilt rules were being marked as
"Modified" when upgraded due to a bug in the `upgrade/_perform` endpoint
where the `from` field was incorrectly calculated via the `lookback`
field. Solves multiple bugs where prebuilt rules were marked as
"Modified" incorrectly when they were upgraded

See reproduce steps in related tickets
([example](elastic#202575 (comment)))

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
dplumlee authored and CAWilson94 committed Dec 12, 2024
1 parent 2bb10e9 commit 00701d5
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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 { transformDiffableFieldValues } from './diffable_rule_fields_mappings';

describe('transformDiffableFieldValues', () => {
it('transforms rule_schedule into "from" value', () => {
const result = transformDiffableFieldValues('from', { interval: '5m', lookback: '4m' });
expect(result).toEqual({ type: 'TRANSFORMED_FIELD', value: 'now-540s' });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
} from '../../../../../../common/api/detection_engine';
import { type AllFieldsDiff } from '../../../../../../common/api/detection_engine';
import type { PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';
import { calculateFromValue } from '../../../rule_types/utils/utils';

/**
* Retrieves and transforms the value for a specific field from a DiffableRule group.
Expand Down Expand Up @@ -201,7 +202,8 @@ export const transformDiffableFieldValues = (
diffableFieldValue: RuleSchedule | InlineKqlQuery | unknown
): TransformValuesReturnType => {
if (fieldName === 'from' && isRuleSchedule(diffableFieldValue)) {
return { type: 'TRANSFORMED_FIELD', value: `now-${diffableFieldValue.lookback}` };
const from = calculateFromValue(diffableFieldValue.interval, diffableFieldValue.lookback);
return { type: 'TRANSFORMED_FIELD', value: from };
} else if (fieldName === 'to') {
return { type: 'TRANSFORMED_FIELD', value: `now` };
} else if (fieldName === 'saved_id' && isInlineQuery(diffableFieldValue)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
* 2.0.
*/

import moment from 'moment';
import { parseInterval } from '@kbn/data-plugin/common/search/aggs/utils/date_interval_utils';
import type { RuleParamsModifierResult } from '@kbn/alerting-plugin/server/rules_client/methods/bulk_edit';
import type { ExperimentalFeatures } from '../../../../../../common';
import type { InvestigationFieldsCombined, RuleAlertType } from '../../../rule_schema';
Expand All @@ -17,6 +15,7 @@ import type {
} from '../../../../../../common/api/detection_engine/rule_management';
import { BulkActionEditTypeEnum } from '../../../../../../common/api/detection_engine/rule_management';
import { invariant } from '../../../../../../common/utils/invariant';
import { calculateFromValue } from '../../../rule_types/utils/utils';

export const addItemsToArray = <T>(arr: T[], items: T[]): T[] =>
Array.from(new Set([...arr, ...items]));
Expand Down Expand Up @@ -256,18 +255,15 @@ const applyBulkActionEditToRuleParams = (
}
// update look-back period in from and meta.from fields
case BulkActionEditTypeEnum.set_schedule: {
const interval = parseInterval(action.value.interval) ?? moment.duration(0);
const parsedFrom = parseInterval(action.value.lookback) ?? moment.duration(0);

const from = parsedFrom.asSeconds() + interval.asSeconds();
const from = calculateFromValue(action.value.interval, action.value.lookback);

ruleParams = {
...ruleParams,
meta: {
...ruleParams.meta,
from: action.value.lookback,
},
from: `now-${from}s`,
from,
};

break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
addToSearchAfterReturn,
getUnprocessedExceptionsWarnings,
getDisabledActionsWarningText,
calculateFromValue,
} from './utils';
import type { BulkResponseErrorAggregation, SearchAfterAndBulkCreateReturnType } from '../types';
import {
Expand Down Expand Up @@ -586,6 +587,23 @@ describe('utils', () => {
});
});

describe('calculateFromValue', () => {
test('should return formatted `from` value from rule schedule fields', () => {
const from = calculateFromValue('5m', '4m');
expect(from).toEqual('now-540s');
});

test('should return formatted `from` value from rule schedule fields with no lookback', () => {
const from = calculateFromValue('5m', '0m');
expect(from).toEqual('now-300s');
});

test('should return formatted `from` value from rule schedule fields with invalid moment fields', () => {
const from = calculateFromValue('5', '5');
expect(from).toEqual('now-0s');
});
});

describe('getMaxCatchupRatio', () => {
test('should return 0 if gap is 0', () => {
const catchup = getNumCatchupIntervals({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,20 @@ export const getCatchupTuples = ({
return catchupTuples;
};

/**
* Takes the rule schedule fields `interval` and `lookback` and uses them to calculate the `from` value for a rule
*
* @param interval string representing the interval on which the rule runs
* @param lookback string representing the rule's additional lookback
* @returns string representing the rule's 'from' property
*/
export const calculateFromValue = (interval: string, lookback: string) => {
const parsedInterval = parseInterval(interval) ?? moment.duration(0);
const parsedFrom = parseInterval(lookback) ?? moment.duration(0);
const duration = parsedFrom.asSeconds() + parsedInterval.asSeconds();
return `now-${duration}s`;
};

/**
* Given errors from a search query this will return an array of strings derived from the errors.
* @param errors The errors to derive the strings from
Expand Down

0 comments on commit 00701d5

Please sign in to comment.