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

[8.x] [Security Solution] Makes `rule_source` a required field in `RuleResponse` (#193636) #195303

Merged
merged 1 commit into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions oas_docs/output/kibana.serverless.staging.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26316,6 +26316,7 @@ components:
- id
- rule_id
- immutable
- rule_source
- updated_at
- updated_by
- created_at
Expand Down
1 change: 1 addition & 0 deletions oas_docs/output/kibana.serverless.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26316,6 +26316,7 @@ components:
- id
- rule_id
- immutable
- rule_source
- updated_at
- updated_by
- created_at
Expand Down
1 change: 1 addition & 0 deletions oas_docs/output/kibana.staging.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34318,6 +34318,7 @@ components:
- id
- rule_id
- immutable
- rule_source
- updated_at
- updated_by
- created_at
Expand Down
1 change: 1 addition & 0 deletions oas_docs/output/kibana.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34318,6 +34318,7 @@ components:
- id
- rule_id
- immutable
- rule_source
- updated_at
- updated_by
- created_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const getResponseBaseParams = (anchorDate: string = ANCHOR_DATE): SharedResponse
risk_score: 55,
risk_score_mapping: [],
rule_id: 'query-rule-id',
rule_source: { type: 'internal' },
interval: '5m',
exceptions_list: getListArrayMock(),
related_integrations: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,13 @@ describe('rule_source', () => {
expect(result.data).toEqual(payload);
});

test('it should validate a rule with "rule_source" set to undefined', () => {
test('it should not validate a rule with "rule_source" set to undefined', () => {
const payload = getRulesSchemaMock();
payload.rule_source = undefined;
// @ts-expect-error
delete payload.rule_source;

const result = RuleResponse.safeParse(payload);
expectParseSuccess(result);
expect(result.data).toEqual(payload);
expectParseError(result);
expect(stringifyZodError(result.error)).toMatchInlineSnapshot(`"rule_source: Required"`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export const ResponseFields = z.object({
id: RuleObjectId,
rule_id: RuleSignatureId,
immutable: IsRuleImmutable,
rule_source: RuleSource.optional(),
rule_source: RuleSource,
updated_at: z.string().datetime(),
updated_by: z.string(),
created_at: z.string().datetime(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ components:
- id
- rule_id
- immutable
- rule_source
- updated_at
- updated_by
- created_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('Bulk CRUD rules response schema', () => {
const result = BulkCrudRulesResponse.safeParse(payload);
expectParseError(result);
expect(stringifyZodError(result.error)).toMatchInlineSnapshot(
`"0.name: Required, 0.error: Required, 0: Unrecognized key(s) in object: 'author', 'created_at', 'updated_at', 'created_by', 'description', 'enabled', 'false_positives', 'from', 'immutable', 'references', 'revision', 'severity', 'severity_mapping', 'updated_by', 'tags', 'to', 'threat', 'version', 'output_index', 'max_signals', 'risk_score', 'risk_score_mapping', 'interval', 'exceptions_list', 'related_integrations', 'required_fields', 'setup', 'throttle', 'actions', 'building_block_type', 'note', 'license', 'outcome', 'alias_target_id', 'alias_purpose', 'timeline_id', 'timeline_title', 'meta', 'rule_name_override', 'timestamp_override', 'timestamp_override_fallback_disabled', 'namespace', 'investigation_fields', 'query', 'type', 'language', 'index', 'data_view_id', 'filters', 'saved_id', 'response_actions', 'alert_suppression'"`
`"0.name: Required, 0.error: Required, 0: Unrecognized key(s) in object: 'author', 'created_at', 'updated_at', 'created_by', 'description', 'enabled', 'false_positives', 'from', 'immutable', 'references', 'revision', 'severity', 'severity_mapping', 'updated_by', 'tags', 'to', 'threat', 'version', 'output_index', 'max_signals', 'risk_score', 'risk_score_mapping', 'rule_source', 'interval', 'exceptions_list', 'related_integrations', 'required_fields', 'setup', 'throttle', 'actions', 'building_block_type', 'note', 'license', 'outcome', 'alias_target_id', 'alias_purpose', 'timeline_id', 'timeline_title', 'meta', 'rule_name_override', 'timestamp_override', 'timestamp_override_fallback_disabled', 'namespace', 'investigation_fields', 'query', 'type', 'language', 'index', 'data_view_id', 'filters', 'saved_id', 'response_actions', 'alert_suppression'"`
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4890,6 +4890,7 @@ components:
- id
- rule_id
- immutable
- rule_source
- updated_at
- updated_by
- created_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4043,6 +4043,7 @@ components:
- id
- rule_id
- immutable
- rule_source
- updated_at
- updated_by
- created_at
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,5 @@ const mockRule: Rule = {
related_integrations: [],
required_fields: [],
setup: '',
rule_source: { type: 'internal' },
};
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,5 @@ const mockRule: Rule = {
related_integrations: [],
required_fields: [],
setup: '',
rule_source: { type: 'internal' },
};
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const mockRule: Rule = {
related_integrations: [],
required_fields: [],
setup: '',
rule_source: { type: 'internal' },
};

describe('useRuleDetailsTabs', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ describe('Rule upgrade workflow: viewing rule changes in JSON diff view', () =>
});

describe('Technical properties should not be included in preview', () => {
it.each(['revision', 'created_at', 'created_by', 'updated_at', 'updated_by'])(
it.each(['revision', 'created_at', 'created_by', 'updated_at', 'updated_by', 'rule_source'])(
'Should not include "%s" in preview',
(property) => {
const oldRule: RuleResponse = {
Expand All @@ -190,6 +190,7 @@ describe('Rule upgrade workflow: viewing rule changes in JSON diff view', () =>
created_by: 'mockUserOne',
updated_at: '01/01/2024T00:00:000z',
updated_by: 'mockUserTwo',
rule_source: { type: 'internal' },
};

const newRule: RuleResponse = {
Expand All @@ -200,6 +201,7 @@ describe('Rule upgrade workflow: viewing rule changes in JSON diff view', () =>
created_by: 'mockUserOne',
updated_at: '02/02/2024T00:00:001z',
updated_by: 'mockUserThree',
rule_source: { type: 'external', is_customized: true },
};

render(<RuleDiffTab oldRule={oldRule} newRule={newRule} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ const HIDDEN_PROPERTIES: Array<keyof RuleResponse> = [
'updated_by',
'created_at',
'created_by',
/*
* Another technical property that is used for logic under the hood the user doesn't need to be aware of
*/
'rule_source',
];

const sortAndStringifyJson = (jsObject: Record<string, unknown>): string =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const savedRuleMock: RuleResponse = {
references: [],
related_integrations: [],
required_fields: [],
rule_source: { type: 'internal' },
setup: '',
severity: 'high',
severity_mapping: [],
Expand Down Expand Up @@ -99,6 +100,7 @@ export const rulesMock: FetchRulesResponse = {
version: 1,
revision: 1,
exceptions_list: [],
rule_source: { type: 'internal' },
},
{
actions: [],
Expand Down Expand Up @@ -138,6 +140,7 @@ export const rulesMock: FetchRulesResponse = {
version: 1,
revision: 1,
exceptions_list: [],
rule_source: { type: 'internal' },
},
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ const getMockRule = (overwrites: Pick<Rule, 'investigation_fields'>): Rule => ({
updated_by: 'elastic',
related_integrations: [],
required_fields: [],
rule_source: { type: 'internal' },
setup: '',
...overwrites,
});
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export const mockRule = (id: string): SavedQueryRule => ({
version: 1,
revision: 1,
exceptions_list: [],
rule_source: { type: 'internal' },
});

export const mockRuleWithEverything = (id: string): RuleResponse => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
* 2.0.
*/

import snakecaseKeys from 'snakecase-keys';
import { convertObjectKeysToSnakeCase } from '../../../../../../utils/object_case_converters';
import type { BaseRuleParams } from '../../../../rule_schema';
import { migrateLegacyInvestigationFields } from '../../../utils/utils';
import type { NormalizedRuleParams } from './normalize_rule_params';

export const commonParamsCamelToSnake = (params: BaseRuleParams) => {
return {
Expand Down Expand Up @@ -45,3 +47,10 @@ export const commonParamsCamelToSnake = (params: BaseRuleParams) => {
setup: params.setup ?? '',
};
};

export const normalizedCommonParamsCamelToSnake = (params: NormalizedRuleParams) => {
return {
...commonParamsCamelToSnake(params),
rule_source: snakecaseKeys(params.ruleSource, { deep: true }),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
transformToActionFrequency,
} from '../../../normalization/rule_actions';
import { typeSpecificCamelToSnake } from './type_specific_camel_to_snake';
import { commonParamsCamelToSnake } from './common_params_camel_to_snake';
import { normalizedCommonParamsCamelToSnake } from './common_params_camel_to_snake';
import { normalizeRuleParams } from './normalize_rule_params';

export const internalRuleToAPIResponse = (
Expand Down Expand Up @@ -58,7 +58,7 @@ export const internalRuleToAPIResponse = (
enabled: rule.enabled,
revision: rule.revision,
// Security solution shared rule params
...commonParamsCamelToSnake(normalizedRuleParams),
...normalizedCommonParamsCamelToSnake(normalizedRuleParams),
// Type specific security solution rule params
...typeSpecificCamelToSnake(rule.params),
// Actions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ interface NormalizeRuleSourceParams {
ruleSource: BaseRuleParams['ruleSource'];
}

export interface NormalizedRuleParams extends BaseRuleParams {
ruleSource: RuleSourceCamelCased;
}

/*
* Since there's no mechanism to migrate all rules at the same time,
* we cannot guarantee that the ruleSource params is present in all rules.
Expand All @@ -36,7 +40,7 @@ export const normalizeRuleSource = ({
return ruleSource;
};

export const normalizeRuleParams = (params: BaseRuleParams) => {
export const normalizeRuleParams = (params: BaseRuleParams): NormalizedRuleParams => {
return {
...params,
// Fields to normalize
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ export const sampleSignalHit = (): SignalHit => ({
saved_id: undefined,
alert_suppression: undefined,
investigation_fields: undefined,
rule_source: { type: 'internal' },
},
depth: 1,
},
Expand Down