Skip to content

Commit

Permalink
[Security Solution] Makes rule_source a required field in `RuleResp…
Browse files Browse the repository at this point in the history
…onse` (#193636)

**Resolves #180270

## Summary

Sets `rule_source` to be a required field in the `RuleResponse` type

### Checklist

Delete any items that are not applicable to this PR.

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
3 people authored Oct 7, 2024
1 parent 10271a2 commit 484f95e
Show file tree
Hide file tree
Showing 23 changed files with 47 additions and 10 deletions.
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

0 comments on commit 484f95e

Please sign in to comment.