Clone saved object document before migrating to prevent accidental mutations (#30475)

* Clone saved object document before migrating to prevent accidental mutations

* Add tests

* Add example comment

* Use latest version of lodash cloneDeep

* Revise spaces functional tests to use same sample data.

* Fix broken test

package.json

@@ -109,6 +109,7 @@
     "@kbn/pm": "1.0.0",
     "@kbn/test-subj-selector": "0.2.1",
     "@kbn/ui-framework": "1.0.0",
+    "@types/lodash.clonedeep": "^4.5.4",
     "JSONStream": "1.1.1",
     "abortcontroller-polyfill": "^1.1.9",
     "angular": "1.6.9",
@@ -170,6 +171,7 @@
     "less": "2.7.1",
     "less-loader": "4.1.0",
     "lodash": "npm:@elastic/lodash@3.10.1-kibana1",
+    "lodash.clonedeep": "^4.5.0",
     "lru-cache": "4.1.1",
     "markdown-it": "^8.4.1",
     "mini-css-extract-plugin": "0.4.4",

src/legacy/server/saved_objects/migrations/core/document_migrator.test.ts

@@ -109,6 +109,29 @@ describe('DocumentMigrator', () => {
     });
   });
 
+  it(`doesn't mutate the original document`, () => {
+    const migrator = new DocumentMigrator({
+      ...testOpts(),
+      migrations: {
+        user: {
+          '1.2.3': (doc: RawSavedObjectDoc) => {
+            _.set(doc, 'attributes.name', 'Mike');
+            return doc;
+          },
+        },
+      },
+    });
+    const originalDoc = {
+      id: 'me',
+      type: 'user',
+      attributes: {},
+      migrationVersion: {},
+    };
+    const migratedDoc = migrator.migrate(originalDoc);
+    expect(_.get(originalDoc, 'attributes.name')).toBeUndefined();
+    expect(_.get(migratedDoc, 'attributes.name')).toBe('Mike');
+  });
+
   it('migrates meta properties', () => {
     const migrator = new DocumentMigrator({
       ...testOpts(),

src/legacy/server/saved_objects/migrations/core/document_migrator.ts

@@ -62,6 +62,7 @@
 
 import Boom from 'boom';
 import _ from 'lodash';
+import cloneDeep from 'lodash.clonedeep';
 import Semver from 'semver';
 import { MigrationVersion, RawSavedObjectDoc } from '../../serialization';
 import { LogFn, Logger, MigrationLogger } from './migration_logger';
@@ -147,7 +148,11 @@ export class DocumentMigrator implements VersionedTransformer {
    * @memberof DocumentMigrator
    */
   public migrate = (doc: RawSavedObjectDoc): RawSavedObjectDoc => {
-    return this.transformDoc(doc);
+    // Clone the document to prevent accidental mutations on the original data
+    // Ex: Importing sample data that is cached at import level, migrations would
+    // execute on mutated data the second time.
+    const clonedDoc = cloneDeep(doc);
+    return this.transformDoc(clonedDoc);
   };
 }
 

x-pack/test/functional/apps/spaces/spaces_selection.ts

@@ -79,23 +79,20 @@ export default function spaceSelectorFunctonalTests({ getService, getPageObjects
             pathname: `/s/${spaceId}${homePath}`,
           },
         });
-        await PageObjects.home.addSampleDataSet('flights');
+        await PageObjects.home.addSampleDataSet('logs');
       });
 
       after(async () => {
+        // No need to remove the same sample data in both spaces, the index
+        // data will be removed in the first call. By feature limitation,
+        // the created saved objects in the second space will be broken but removed
+        // when we call esArchiver.unload('spaces').
         await PageObjects.common.navigateToApp('home', {
           appConfig: {
             hash: sampleDataHash,
           },
         });
         await PageObjects.home.removeSampleDataSet('logs');
-        await PageObjects.common.navigateToApp('home', {
-          appConfig: {
-            hash: sampleDataHash,
-            pathname: `/s/${spaceId}${homePath}`,
-          },
-        });
-        await PageObjects.home.removeSampleDataSet('flights');
         await PageObjects.security.logout();
         await esArchiver.unload('spaces');
       });
@@ -112,7 +109,7 @@ export default function spaceSelectorFunctonalTests({ getService, getPageObjects
               pathname: `/s/${spaceId}${dashboardPath}`,
             },
           });
-          await expectDashboardRenders('[Flights] Global Flight Dashboard');
+          await expectDashboardRenders('[Logs] Web Traffic');
         });
       });
     });

yarn.lock

@@ -1584,6 +1584,13 @@
   dependencies:
     "@types/node" "*"
 
+"@types/lodash.clonedeep@^4.5.4":
+  version "4.5.4"
+  resolved "https://registry.yarnpkg.com/@types/lodash.clonedeep/-/lodash.clonedeep-4.5.4.tgz#2515c5f08bc95afebfb597711871b0497f5d7da7"
+  integrity sha512-+rCVPIZOJaub++wU/lmyp/SxiKlqXQaXI5LryzjuHBKFj51ApVt38Xxk9psLWNGMuR/obEQNTH0l/yDfG4ANNQ==
+  dependencies:
+    "@types/lodash" "*"
+
 "@types/lodash.clonedeepwith@^4.5.3":
   version "4.5.3"
   resolved "https://registry.yarnpkg.com/@types/lodash.clonedeepwith/-/lodash.clonedeepwith-4.5.3.tgz#8057f074de743bdcff59fdbf26cd04c674a186cc"
@@ -13847,7 +13854,7 @@ lodash.clone@^4.5.0:
   resolved "https://registry.yarnpkg.com/lodash.clone/-/lodash.clone-4.5.0.tgz#195870450f5a13192478df4bc3d23d2dea1907b6"
   integrity sha1-GVhwRQ9aExkkeN9Lw9I9LeoZB7Y=
 
-lodash.clonedeep@^4.3.2:
+lodash.clonedeep@^4.3.2, lodash.clonedeep@^4.5.0:
   version "4.5.0"
   resolved "https://registry.yarnpkg.com/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz#e23f3f9c4f8fbdde872529c1071857a086e5ccef"
   integrity sha1-4j8/nE+Pvd6HJSnBBxhXoIblzO8=