[Response Ops][Cases] Add more granular checks before decoding user actions (#124896) (#125468)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Christos Nasikas <christos.nasikas@elastic.co>
(cherry picked from commit 97bd50da88)

# Conflicts:
#	x-pack/plugins/cases/server/saved_object_types/migrations/user_actions/alerts.test.ts
#	x-pack/plugins/cases/server/saved_object_types/migrations/user_actions/alerts.ts

Co-authored-by: Jonathan Buttner <56361221+jonathan-buttner@users.noreply.github.com>

x-pack/plugins/cases/server/saved_object_types/migrations/user_actions.test.ts

@@ -7,7 +7,7 @@
 
 /* eslint-disable @typescript-eslint/naming-convention */
 
-import { cloneDeep, omit } from 'lodash';
+import { cloneDeep, omit, set } from 'lodash';
 import {
   SavedObject,
   SavedObjectMigrationContext,
@@ -610,6 +610,89 @@ describe('user action migrations', () => {
       context = migrationMocks.createContext();
     });
 
+    describe('JSON.stringify throws an error', () => {
+      let jsonStringifySpy: jest.SpyInstance;
+      beforeEach(() => {
+        jsonStringifySpy = jest.spyOn(JSON, 'stringify').mockImplementation(() => {
+          throw new Error('failed to stringify');
+        });
+      });
+
+      afterEach(() => {
+        jsonStringifySpy.mockRestore();
+      });
+
+      it('logs an error when an error is thrown outside of the JSON.parse function', () => {
+        const doc = {
+          id: '123',
+          attributes: {
+            action: 'create',
+            action_field: ['comment'],
+            new_value:
+              '{"type":"generated_alert","alertId":"4eb4cd05b85bc65c7b9f22b776e0136f970f7538eb0d1b2e6e8c7d35b2e875cb","index":".internal.alerts-security.alerts-default-000001","rule":{"id":"43104810-7875-11ec-abc6-6f72e72f6004","name":"A rule"},"owner":"securitySolution"}',
+          },
+          type: 'abc',
+          references: [],
+        };
+
+        expect(removeRuleInformation(doc, context)).toEqual(doc);
+
+        const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;
+        expect(log.error.mock.calls[0]).toMatchInlineSnapshot(`
+          Array [
+            "Failed to migrate user action alerts with doc id: 123 version: 8.0.0 error: failed to stringify",
+            Object {
+              "migrations": Object {
+                "userAction": Object {
+                  "id": "123",
+                },
+              },
+            },
+          ]
+        `);
+      });
+    });
+
+    describe('JSON.parse spy', () => {
+      let jsonParseSpy: jest.SpyInstance;
+      beforeEach(() => {
+        jsonParseSpy = jest.spyOn(JSON, 'parse');
+      });
+
+      afterEach(() => {
+        jsonParseSpy.mockRestore();
+      });
+
+      it.each([
+        ['update', ['status']],
+        ['update', ['comment']],
+        [undefined, ['comment']],
+        ['create', ['status']],
+        ['create', []],
+      ])(
+        'does not modify the document and does not call JSON.parse when action: %s and action_field: %s',
+        (action, actionField) => {
+          const doc = {
+            id: '123',
+            attributes: {
+              action,
+              action_field: actionField,
+              new_value: 'open',
+            },
+            type: 'abc',
+            references: [],
+          };
+
+          expect(removeRuleInformation(doc, context)).toEqual(doc);
+
+          expect(jsonParseSpy).not.toBeCalled();
+
+          const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;
+          expect(log.error).not.toBeCalled();
+        }
+      );
+    });
+
     it('does not modify non-alert user action', () => {
       const doc = {
         id: '123',
@@ -639,18 +722,7 @@ describe('user action migrations', () => {
       expect(removeRuleInformation(doc, context)).toEqual(doc);
 
       const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;
-      expect(log.error.mock.calls[0]).toMatchInlineSnapshot(`
-        Array [
-          "Failed to migrate user action alerts with doc id: 123 version: 8.0.0 error: Unexpected end of JSON input",
-          Object {
-            "migrations": Object {
-              "userAction": Object {
-                "id": "123",
-              },
-            },
-          },
-        ]
-      `);
+      expect(log.error).not.toBeCalled();
     });
 
     it('does not modify the document when new_value is null', () => {
@@ -717,6 +789,26 @@ describe('user action migrations', () => {
       expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
     });
 
+    it('sets the rule fields to null when the alert has an extra field', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['comment'],
+          new_value:
+            '{"anExtraField": "some value","type":"alert","alertId":"4eb4cd05b85bc65c7b9f22b776e0136f970f7538eb0d1b2e6e8c7d35b2e875cb","index":".internal.alerts-security.alerts-default-000001","rule":{"id":"43104810-7875-11ec-abc6-6f72e72f6004","name":"A rule"},"owner":"securitySolution"}',
+        },
+        type: 'abc',
+        references: [],
+      };
+
+      const newDoc = removeRuleInformation(doc, context) as SavedObject<{ new_value: string }>;
+      ensureRuleFieldsAreNull(newDoc);
+
+      expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
+      expectParsedDocsToEqual(newDoc, doc);
+    });
+
     it('sets the rule fields to null for a generated alert', () => {
       const doc = {
         id: '123',
@@ -753,10 +845,38 @@ describe('user action migrations', () => {
       ensureRuleFieldsAreNull(newDoc);
 
       expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
+      expectParsedDocsToEqual(newDoc, doc);
     });
   });
 });
 
+const expectParsedDocsToEqual = (
+  nulledRuleDoc: SavedObject<{ new_value: string }>,
+  originalDoc: SavedObject<{ new_value: string }>
+) => {
+  expect(parseDoc(nulledRuleDoc)).toEqual(injectNullRuleFields(parseDoc(originalDoc)));
+};
+
+const parseDoc = (doc: SavedObject<{ new_value: string }>): SavedObject<{ new_value: {} }> => {
+  const copyOfDoc = cloneDeep(doc);
+
+  const decodedNewValue = JSON.parse(doc.attributes.new_value);
+  set(copyOfDoc, 'attributes.new_value', decodedNewValue);
+
+  return copyOfDoc;
+};
+
+const injectNullRuleFields = (doc: SavedObject<{ new_value: {} }>) => {
+  const copyOfDoc = cloneDeep(doc);
+
+  set(copyOfDoc, 'attributes.new_value', {
+    ...doc.attributes.new_value,
+    rule: { id: null, name: null },
+  });
+
+  return copyOfDoc;
+};
+
 const docWithoutNewValue = (doc: {}) => {
   const copyOfDoc = cloneDeep(doc);
   return omit(copyOfDoc, 'attributes.new_value');

x-pack/plugins/cases/server/saved_object_types/migrations/user_actions.ts

@@ -119,9 +119,13 @@ export function removeRuleInformation(
   try {
     const { new_value, action, action_field } = doc.attributes;
 
+    if (!isCreateComment(action, action_field)) {
+      return originalDocWithReferences;
+    }
+
     const decodedNewValueData = decodeNewValue(new_value);
 
-    if (!isAlertUserAction(action, action_field, decodedNewValueData)) {
+    if (!isAlertUserAction(decodedNewValueData)) {
       return originalDocWithReferences;
     }
 
@@ -159,21 +163,15 @@ function decodeNewValue(data?: string | null): unknown | null {
     return null;
   }
 
-  return JSON.parse(data);
+  try {
+    return JSON.parse(data);
+  } catch (error) {
+    return null;
+  }
 }
 
-function isAlertUserAction(
-  action?: string,
-  actionFields?: string[],
-  newValue?: unknown | null
-): newValue is AlertCommentOptional {
-  return isCreateComment(action, actionFields) && isAlertObject(newValue);
-}
-
-type AlertCommentOptional = Partial<CommentRequestAlertType>;
-
-function isAlertObject(data?: unknown | null): boolean {
-  const unsafeAlertData = data as AlertCommentOptional;
+function isAlertUserAction(newValue?: unknown | null): newValue is AlertCommentOptional {
+  const unsafeAlertData = newValue as AlertCommentOptional;
 
   return (
     unsafeAlertData !== undefined &&
@@ -183,6 +181,8 @@ function isAlertObject(data?: unknown | null): boolean {
   );
 }
 
+type AlertCommentOptional = Partial<CommentRequestAlertType>;
+
 export const userActionsMigrations = {
   '7.10.0': (doc: SavedObjectUnsanitizedDoc<UserActions>): SavedObjectSanitizedDoc<UserActions> => {
     const { action_field, new_value, old_value, ...restAttributes } = doc.attributes;