[Security Solution] Unfriendly error message when creating an invalid…

… exception (#168213) (#170764) ## Summary Addresses #168213 With this PR we limit the length of the comments in exceptions to 30K characters. We took same approach as used in Cases. User will not be allowed to create/edit exception with the comment longer than 30K and validation error will be shown. Right now if user tries to add a very long comment (above 32K characters) the server throws an exception due to the length limitation of the `keyword` type. After the fix, user will see a validation error on putting very long text as a comment <img width="1294" alt="Screenshot 2023-11-07 at 16 47 10" src="https://github.com/elastic/kibana/assets/2700761/16c284a8-ab63-45d7-80dd-e50f48a3f5e2">
elastic · Nov 8, 2023 · d644c70 · d644c70
1 parent ed9956d
commit d644c70
Show file tree

Hide file tree

Showing 14 changed files with 300 additions and 32 deletions.
diff --git a/x-pack/plugins/security_solution/common/constants.ts b/x-pack/plugins/security_solution/common/constants.ts
@@ -505,3 +505,8 @@ export const DEFAULT_ALERT_TAGS_VALUE = [
   i18n.FALSE_POSITIVE,
   i18n.FURTHER_INVESTIGATION_REQUIRED,
 ] as const;
+
+/**
+ * Max length for the comments within security solution
+ */
+export const MAX_COMMENT_LENGTH = 30000 as const;
diff --git a/...on/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.test.tsx b/...on/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.test.tsx
@@ -8,7 +8,7 @@
 import React from 'react';
 import type { ReactWrapper } from 'enzyme';
 import { mount, shallow } from 'enzyme';
-import { waitFor, render } from '@testing-library/react';
+import { act, fireEvent, render, waitFor } from '@testing-library/react';
 
 import { getExceptionListSchemaMock } from '@kbn/lists-plugin/common/schemas/response/exception_list_schema.mock';
 import { getExceptionBuilderComponentLazy } from '@kbn/lists-plugin/public';
@@ -35,6 +35,7 @@ import {
 import type { AlertData } from '../../utils/types';
 import { useFindRules } from '../../../rule_management/logic/use_find_rules';
 import { useFindExceptionListReferences } from '../../logic/use_find_references';
+import { MAX_COMMENT_LENGTH } from '../../../../../common/constants';
 
 jest.mock('../../../../detections/containers/detection_engine/alerts/use_signal_index');
 jest.mock('../../../../common/lib/kibana');
@@ -1305,5 +1306,46 @@ describe('When the add exception modal is opened', () => {
         wrapper.find('button[data-test-subj="addExceptionConfirmButton"]').getDOMNode()
       ).toBeDisabled();
     });
+
+    test('when there is a comment error has submit button disabled', async () => {
+      const { getByLabelText, queryByText, getByTestId } = render(
+        <TestProviders>
+          <AddExceptionFlyout
+            rules={[
+              {
+                ...getRulesEqlSchemaMock(),
+                query:
+                  'sequence [process where process.name = "test.exe"] [process where process.name = "explorer.exe"]',
+              } as Rule,
+            ]}
+            isBulkAction={false}
+            alertData={alertDataMock}
+            isAlertDataLoading={false}
+            alertStatus="open"
+            isEndpointItem={false}
+            showAlertCloseOptions
+            onCancel={jest.fn()}
+            onConfirm={jest.fn()}
+          />
+        </TestProviders>
+      );
+
+      const commentInput = getByLabelText('Comment Input');
+
+      const commentErrorMessage = `The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH} characters.`;
+      expect(queryByText(commentErrorMessage)).toBeNull();
+
+      // Put comment with the length above maximum allowed
+      act(() => {
+        fireEvent.change(commentInput, {
+          target: {
+            value: [...new Array(MAX_COMMENT_LENGTH + 1).keys()].map((_) => 'a').join(''),
+          },
+        });
+        fireEvent.blur(commentInput);
+      });
+      expect(queryByText(commentErrorMessage)).not.toBeNull();
+      expect(getByTestId('addExceptionConfirmButton')).toBeDisabled();
+    });
   });
 });
diff --git a/...olution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx b/...olution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx
@@ -157,6 +157,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
       selectedRulesToAddTo,
       exceptionListsToAddTo,
       newComment,
+      commentErrorExists,
       itemConditionValidationErrorExists,
       errorSubmitting,
       expireTime,
@@ -267,6 +268,16 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
     [dispatch]
   );
 
+  const setCommentError = useCallback(
+    (errorExists: boolean): void => {
+      dispatch({
+        type: 'setCommentError',
+        errorExists,
+      });
+    },
+    [dispatch]
+  );
+
   const setBulkCloseIndex = useCallback(
     (index: string[] | undefined): void => {
       dispatch({
@@ -445,6 +456,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
       exceptionItemName.trim() === '' ||
       exceptionItems.every((item) => item.entries.length === 0) ||
       itemConditionValidationErrorExists ||
+      commentErrorExists ||
       expireErrorExists ||
       (addExceptionToRadioSelection === 'add_to_lists' && isEmpty(exceptionListsToAddTo)) ||
       (addExceptionToRadioSelection === 'select_rules_to_add_to' &&
@@ -462,6 +474,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
       expireErrorExists,
       selectedRulesToAddTo,
       listType,
+      commentErrorExists,
     ]
   );
 
@@ -555,6 +568,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
             initialIsOpen={!!newComment}
             newCommentValue={newComment}
             newCommentOnChange={setComment}
+            setCommentError={setCommentError}
           />
           {listType !== ExceptionListTypeEnum.ENDPOINT && (
             <>

diff --git a/...lution/public/detection_engine/rule_exceptions/components/add_exception_flyout/reducer.ts b/...lution/public/detection_engine/rule_exceptions/components/add_exception_flyout/reducer.ts
@@ -21,6 +21,7 @@ export interface State {
   initialItems: ExceptionsBuilderExceptionItem[];
   exceptionItems: ExceptionsBuilderReturnExceptionItem[];
   newComment: string;
+  commentErrorExists: boolean;
   addExceptionToRadioSelection: string;
   itemConditionValidationErrorExists: boolean;
   closeSingleAlert: boolean;
@@ -40,6 +41,7 @@ export const initialState: State = {
   exceptionItems: [],
   exceptionItemMeta: { name: '' },
   newComment: '',
+  commentErrorExists: false,
   itemConditionValidationErrorExists: false,
   closeSingleAlert: false,
   bulkCloseAlerts: false,
@@ -76,6 +78,10 @@ export type Action =
       type: 'setComment';
       comment: string;
     }
+  | {
+      type: 'setCommentError';
+      errorExists: boolean;
+    }
   | {
       type: 'setCloseSingleAlert';
       close: boolean;
@@ -127,6 +133,7 @@ export type Action =
 
 export const createExceptionItemsReducer =
   () =>
+  /* eslint complexity: ["error", 21]*/
   (state: State, action: Action): State => {
     switch (action.type) {
       case 'setExceptionItemMeta': {
@@ -172,6 +179,14 @@ export const createExceptionItemsReducer =
           newComment: comment,
         };
       }
+      case 'setCommentError': {
+        const { errorExists } = action;
+
+        return {
+          ...state,
+          commentErrorExists: errorExists,
+        };
+      }
       case 'setCloseSingleAlert': {
         const { close } = action;
 

diff --git a/...n/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.test.tsx b/...n/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.test.tsx
@@ -6,7 +6,7 @@
  */
 
 import React from 'react';
-import { waitFor } from '@testing-library/react';
+import { act, fireEvent, render, waitFor } from '@testing-library/react';
 import { ThemeProvider } from 'styled-components';
 import type { ReactWrapper } from 'enzyme';
 import { mount } from 'enzyme';
@@ -34,6 +34,7 @@ import { useFetchIndexPatterns } from '../../logic/use_exception_flyout_data';
 import { useCreateOrUpdateException } from '../../logic/use_create_update_exception';
 import { useFindExceptionListReferences } from '../../logic/use_find_references';
 import * as i18n from './translations';
+import { MAX_COMMENT_LENGTH } from '../../../../../common/constants';
 
 const mockTheme = getMockTheme({
   eui: {
@@ -693,5 +694,60 @@ describe('When the edit exception modal is opened', () => {
         wrapper.find('button[data-test-subj="editExceptionConfirmButton"]').getDOMNode()
       ).toBeDisabled();
     });
+
+    test('when there is a comment error has submit button disabled', async () => {
+      const { getByLabelText, queryByText, getByTestId } = render(
+        <TestProviders>
+          <EditExceptionFlyout
+            list={{
+              ...getExceptionListSchemaMock(),
+              type: 'detection',
+              namespace_type: 'single',
+              list_id: 'my_list_id',
+              id: '1234',
+            }}
+            rule={
+              {
+                ...getRulesEqlSchemaMock(),
+                id: '345',
+                name: 'My rule',
+                rule_id: 'my_rule_id',
+                query:
+                  'sequence [process where process.name = "test.exe"] [process where process.name = "explorer.exe"]',
+                exceptions_list: [
+                  {
+                    id: '1234',
+                    list_id: 'my_list_id',
+                    type: 'detection',
+                    namespace_type: 'single',
+                  },
+                ],
+              } as Rule
+            }
+            itemToEdit={getExceptionListItemSchemaMock()}
+            showAlertCloseOptions
+            onCancel={jest.fn()}
+            onConfirm={jest.fn()}
+          />
+        </TestProviders>
+      );
+
+      const commentInput = getByLabelText('Comment Input');
+
+      const commentErrorMessage = `The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH} characters.`;
+      expect(queryByText(commentErrorMessage)).toBeNull();
+
+      // Put comment with the length above maximum allowed
+      act(() => {
+        fireEvent.change(commentInput, {
+          target: {
+            value: [...new Array(MAX_COMMENT_LENGTH + 1).keys()].map((_) => 'a').join(''),
+          },
+        });
+        fireEvent.blur(commentInput);
+      });
+      expect(queryByText(commentErrorMessage)).not.toBeNull();
+      expect(getByTestId('editExceptionConfirmButton')).toBeDisabled();
+    });
   });
 });
diff --git a/...lution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.tsx b/...lution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.tsx
@@ -117,6 +117,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
       exceptionItems,
       exceptionItemMeta: { name: exceptionItemName },
       newComment,
+      commentErrorExists,
       bulkCloseAlerts,
       disableBulkClose,
       bulkCloseIndex,
@@ -129,6 +130,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
     exceptionItems: [itemToEdit],
     exceptionItemMeta: { name: itemToEdit.name },
     newComment: '',
+    commentErrorExists: false,
     bulkCloseAlerts: false,
     disableBulkClose: true,
     bulkCloseIndex: undefined,
@@ -197,6 +199,16 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
     [dispatch]
   );
 
+  const setCommentError = useCallback(
+    (errorExists: boolean): void => {
+      dispatch({
+        type: 'setCommentError',
+        errorExists,
+      });
+    },
+    [dispatch]
+  );
+
   const setBulkCloseAlerts = useCallback(
     (bulkClose: boolean): void => {
       dispatch({
@@ -337,8 +349,17 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
       exceptionItems.every((item) => item.entries.length === 0) ||
       isLoading ||
       entryErrorExists ||
+      expireErrorExists ||
+      commentErrorExists,
+    [
+      isLoading,
+      entryErrorExists,
+      exceptionItems,
+      isSubmitting,
+      isClosingAlerts,
       expireErrorExists,
-    [isLoading, entryErrorExists, exceptionItems, isSubmitting, isClosingAlerts, expireErrorExists]
+      commentErrorExists,
+    ]
   );
 
   return (
@@ -398,6 +419,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
           exceptionItemComments={itemToEdit.comments}
           newCommentValue={newComment}
           newCommentOnChange={setComment}
+          setCommentError={setCommentError}
         />
         {listType !== ExceptionListTypeEnum.ENDPOINT && (
           <>

diff --git a/...ution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/reducer.ts b/...ution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/reducer.ts
@@ -12,6 +12,7 @@ export interface State {
   exceptionItems: ExceptionsBuilderReturnExceptionItem[];
   exceptionItemMeta: { name: string };
   newComment: string;
+  commentErrorExists: boolean;
   bulkCloseAlerts: boolean;
   disableBulkClose: boolean;
   bulkCloseIndex: string[] | undefined;
@@ -29,6 +30,10 @@ export type Action =
       type: 'setComment';
       comment: string;
     }
+  | {
+      type: 'setCommentError';
+      errorExists: boolean;
+    }
   | {
       type: 'setBulkCloseAlerts';
       bulkClose: boolean;
@@ -81,6 +86,14 @@ export const createExceptionItemsReducer =
           newComment: comment,
         };
       }
+      case 'setCommentError': {
+        const { errorExists } = action;
+
+        return {
+          ...state,
+          commentErrorExists: errorExists,
+        };
+      }
       case 'setBulkCloseAlerts': {
         const { bulkClose } = action;