From 484f95e7335a5b8d8df0d8c321d2b2e74db668a8 Mon Sep 17 00:00:00 2001 From: Davis Plumlee <56367316+dplumlee@users.noreply.github.com> Date: Mon, 7 Oct 2024 13:56:12 -0400 Subject: [PATCH] [Security Solution] Makes `rule_source` a required field in `RuleResponse` (#193636) **Resolves https://github.com/elastic/kibana/issues/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 Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- oas_docs/output/kibana.serverless.staging.yaml | 1 + oas_docs/output/kibana.serverless.yaml | 1 + oas_docs/output/kibana.staging.yaml | 1 + oas_docs/output/kibana.yaml | 1 + .../model/rule_schema/rule_response_schema.mock.ts | 1 + .../model/rule_schema/rule_response_schema.test.ts | 9 +++++---- .../model/rule_schema/rule_schemas.gen.ts | 2 +- .../model/rule_schema/rule_schemas.schema.yaml | 1 + .../rule_management/bulk_crud/response_schema.test.ts | 2 +- ...olution_detections_api_2023_10_31.bundled.schema.yaml | 1 + ...olution_detections_api_2023_10_31.bundled.schema.yaml | 1 + .../rule_details/legacy_url_conflict_callout.test.tsx | 1 + .../pages/rule_details/use_redirect_legacy_url.test.ts | 1 + .../pages/rule_details/use_rule_details_tabs.test.tsx | 1 + .../components/rule_details/json_diff/json_diff.test.tsx | 4 +++- .../components/rule_details/rule_diff_tab.tsx | 4 ++++ .../detection_engine/rule_management/logic/mock.ts | 3 +++ .../rule_management/logic/use_rule_with_fallback.test.ts | 1 + .../components/rules_table/__mocks__/mock.ts | 1 + .../converters/common_params_camel_to_snake.ts | 9 +++++++++ .../converters/internal_rule_to_api_response.ts | 4 ++-- .../converters/normalize_rule_params.ts | 6 +++++- .../detection_engine/rule_types/__mocks__/es_results.ts | 1 + 23 files changed, 47 insertions(+), 10 deletions(-) diff --git a/oas_docs/output/kibana.serverless.staging.yaml b/oas_docs/output/kibana.serverless.staging.yaml index 4ddb7c4876eaf..8bd9bd198e5e1 100644 --- a/oas_docs/output/kibana.serverless.staging.yaml +++ b/oas_docs/output/kibana.serverless.staging.yaml @@ -44211,6 +44211,7 @@ components: - id - rule_id - immutable + - rule_source - updated_at - updated_by - created_at diff --git a/oas_docs/output/kibana.serverless.yaml b/oas_docs/output/kibana.serverless.yaml index 4ddb7c4876eaf..8bd9bd198e5e1 100644 --- a/oas_docs/output/kibana.serverless.yaml +++ b/oas_docs/output/kibana.serverless.yaml @@ -44211,6 +44211,7 @@ components: - id - rule_id - immutable + - rule_source - updated_at - updated_by - created_at diff --git a/oas_docs/output/kibana.staging.yaml b/oas_docs/output/kibana.staging.yaml index 42a30dca9cb91..aba85f8c82ca9 100644 --- a/oas_docs/output/kibana.staging.yaml +++ b/oas_docs/output/kibana.staging.yaml @@ -52949,6 +52949,7 @@ components: - id - rule_id - immutable + - rule_source - updated_at - updated_by - created_at diff --git a/oas_docs/output/kibana.yaml b/oas_docs/output/kibana.yaml index 42a30dca9cb91..aba85f8c82ca9 100644 --- a/oas_docs/output/kibana.yaml +++ b/oas_docs/output/kibana.yaml @@ -52949,6 +52949,7 @@ components: - id - rule_id - immutable + - rule_source - updated_at - updated_by - created_at diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.mock.ts b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.mock.ts index c605436576995..59b09533a6e14 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.mock.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.mock.ts @@ -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: [], diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.test.ts b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.test.ts index 30fe84514e05a..9546ab3a59b09 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.test.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.test.ts @@ -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"`); }); }); diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.gen.ts b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.gen.ts index a723eb8e7da89..da4661ae8464c 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.gen.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.gen.ts @@ -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(), diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.schema.yaml b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.schema.yaml index ca2f325c8f713..d8aba232c26f9 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.schema.yaml +++ b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.schema.yaml @@ -203,6 +203,7 @@ components: - id - rule_id - immutable + - rule_source - updated_at - updated_by - created_at diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_crud/response_schema.test.ts b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_crud/response_schema.test.ts index 2d4af1c18f6d1..fb03c9c4b18ee 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_crud/response_schema.test.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_crud/response_schema.test.ts @@ -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'"` ); }); diff --git a/x-pack/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml b/x-pack/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml index cdff061d94f22..8fca765f4fb3f 100644 --- a/x-pack/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml +++ b/x-pack/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml @@ -4890,6 +4890,7 @@ components: - id - rule_id - immutable + - rule_source - updated_at - updated_by - created_at diff --git a/x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml b/x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml index 4b7d9bed6e416..38b419972a681 100644 --- a/x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml +++ b/x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml @@ -4043,6 +4043,7 @@ components: - id - rule_id - immutable + - rule_source - updated_at - updated_by - created_at diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx index 5334143f27047..879a4570e6d03 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx @@ -114,4 +114,5 @@ const mockRule: Rule = { related_integrations: [], required_fields: [], setup: '', + rule_source: { type: 'internal' }, }; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts index e706f5fda4b39..2e005cd7ca03b 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts @@ -116,4 +116,5 @@ const mockRule: Rule = { related_integrations: [], required_fields: [], setup: '', + rule_source: { type: 'internal' }, }; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_rule_details_tabs.test.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_rule_details_tabs.test.tsx index cf4446c7fea42..1dec9aa36d21a 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_rule_details_tabs.test.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_rule_details_tabs.test.tsx @@ -53,6 +53,7 @@ const mockRule: Rule = { related_integrations: [], required_fields: [], setup: '', + rule_source: { type: 'internal' }, }; describe('useRuleDetailsTabs', () => { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/json_diff/json_diff.test.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/json_diff/json_diff.test.tsx index 2a769e4be87d4..58ee6b56528a5 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/json_diff/json_diff.test.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/json_diff/json_diff.test.tsx @@ -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 = { @@ -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 = { @@ -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(); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx index dd6d0417d3bb6..c2bf9ffa29098 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx @@ -57,6 +57,10 @@ const HIDDEN_PROPERTIES: Array = [ '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 => diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/mock.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/mock.ts index b8aba73818ac3..616469f03c1ba 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/mock.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/mock.ts @@ -41,6 +41,7 @@ export const savedRuleMock: RuleResponse = { references: [], related_integrations: [], required_fields: [], + rule_source: { type: 'internal' }, setup: '', severity: 'high', severity_mapping: [], @@ -99,6 +100,7 @@ export const rulesMock: FetchRulesResponse = { version: 1, revision: 1, exceptions_list: [], + rule_source: { type: 'internal' }, }, { actions: [], @@ -138,6 +140,7 @@ export const rulesMock: FetchRulesResponse = { version: 1, revision: 1, exceptions_list: [], + rule_source: { type: 'internal' }, }, ], }; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/use_rule_with_fallback.test.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/use_rule_with_fallback.test.ts index 81438f3708623..666a8e9fddf3e 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/use_rule_with_fallback.test.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/use_rule_with_fallback.test.ts @@ -118,6 +118,7 @@ const getMockRule = (overwrites: Pick): Rule => ({ updated_by: 'elastic', related_integrations: [], required_fields: [], + rule_source: { type: 'internal' }, setup: '', ...overwrites, }); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/__mocks__/mock.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/__mocks__/mock.ts index 0de6e5d1e0844..70d5ee6b6038f 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/__mocks__/mock.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/__mocks__/mock.ts @@ -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 => ({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/common_params_camel_to_snake.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/common_params_camel_to_snake.ts index 6f98230043e74..f86abd4f08d8d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/common_params_camel_to_snake.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/common_params_camel_to_snake.ts @@ -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 { @@ -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 }), + }; +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/internal_rule_to_api_response.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/internal_rule_to_api_response.ts index 336cd8fae0405..5c53eb2c951a1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/internal_rule_to_api_response.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/internal_rule_to_api_response.ts @@ -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 = ( @@ -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 diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/normalize_rule_params.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/normalize_rule_params.ts index eddd8b0434ba0..8d5793c04f22b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/normalize_rule_params.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/normalize_rule_params.ts @@ -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. @@ -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 diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/es_results.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/es_results.ts index 0867245a40933..a31ea28e0972a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/es_results.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/es_results.ts @@ -556,6 +556,7 @@ export const sampleSignalHit = (): SignalHit => ({ saved_id: undefined, alert_suppression: undefined, investigation_fields: undefined, + rule_source: { type: 'internal' }, }, depth: 1, },