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] Makes rule_source a required field in RuleResponse #193636

Merged
merged 12 commits into from
Oct 7, 2024
Merged
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 @@ -44211,6 +44211,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 @@ -44211,6 +44211,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 @@ -52949,6 +52949,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 @@ -52949,6 +52949,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