[Security Solution] Makes rule_source a required field in `RuleResp…

…onse` (elastic#193636) **Resolves elastic#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]> (cherry picked from commit 484f95e)
kibanamachine · Oct 7, 2024 · dc9ba6d · dc9ba6d
1 parent 04ca4b5
commit dc9ba6d
Show file tree

Hide file tree

Showing 23 changed files with 47 additions and 10 deletions.
diff --git a/oas_docs/output/kibana.serverless.staging.yaml b/oas_docs/output/kibana.serverless.staging.yaml
@@ -26316,6 +26316,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
@@ -26316,6 +26316,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
@@ -34318,6 +34318,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
@@ -34318,6 +34318,7 @@ components:
         - id
         - rule_id
         - immutable
+        - rule_source
         - updated_at
         - updated_by
         - created_at

diff --git a/...urity_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.mock.ts b/...urity_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/...urity_solution/common/api/detection_engine/model/rule_schema/rule_response_schema.test.ts b/...urity_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/...ugins/security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.gen.ts b/...ugins/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/.../security_solution/common/api/detection_engine/model/rule_schema/rule_schemas.schema.yaml b/.../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/...ty_solution/common/api/detection_engine/rule_management/bulk_crud/response_schema.test.ts b/...ty_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/...solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml b/...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/...n/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml b/...n/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/.../detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx b/.../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/...ublic/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts b/...ublic/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/...public/detection_engine/rule_details_ui/pages/rule_details/use_rule_details_tabs.test.tsx b/...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/...lic/detection_engine/rule_management/components/rule_details/json_diff/json_diff.test.tsx b/...lic/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(<RuleDiffTab oldRule={oldRule} newRule={newRule} />);

diff --git a/...olution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx b/...olution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx
@@ -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 =>

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
@@ -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/...ity_solution/public/detection_engine/rule_management/logic/use_rule_with_fallback.test.ts b/...ity_solution/public/detection_engine/rule_management/logic/use_rule_with_fallback.test.ts
@@ -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,
 });
diff --git a/...ution/public/detection_engine/rule_management_ui/components/rules_table/__mocks__/mock.ts b/...ution/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/...e/rule_management/logic/detection_rules_client/converters/common_params_camel_to_snake.ts b/...e/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/.../rule_management/logic/detection_rules_client/converters/internal_rule_to_api_response.ts b/.../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/...n_engine/rule_management/logic/detection_rules_client/converters/normalize_rule_params.ts b/...n_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/.../plugins/security_solution/server/lib/detection_engine/rule_types/__mocks__/es_results.ts b/.../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,
   },