fix(GUI): don't log absolute paths in Mixpanel (#1161)

The event data may contain absolute paths that contain user information that
should not be logged in Mixpanel. Instead, we replace absolute path properties
with their base name.

Change-Type: patch
Changelog-Entry: Don't include user paths in Mixpanel analytics events.

lib/gui/modules/analytics.js

@@ -141,14 +141,15 @@ analytics.service('AnalyticsService', function($log, $window, $mixpanel, Setting
    * });
    */
   this.logEvent = (message, data) => {
-    const flatStartCaseData = utils.makeFlatStartCaseObject(data);
+    const debugData = utils.hideAbsolutePathsInObject(utils.makeFlatStartCaseObject(data));
+
     if (SettingsModel.get('errorReporting') && isRunningInAsar()) {
-      $mixpanel.track(message, flatStartCaseData);
+      $mixpanel.track(message, debugData);
     }
 
     const debugMessage = _.attempt(() => {
-      if (flatStartCaseData) {
-        return `${message} (${JSON.stringify(flatStartCaseData)})`;
+      if (debugData) {
+        return `${message} (${JSON.stringify(debugData)})`;
       }
 
       return message;

lib/shared/utils.js

@@ -17,8 +17,10 @@
 'use strict';
 
 const _ = require('lodash');
+_.mixin(require('lodash-deep'));
 const flatten = require('flat').flatten;
 const deepMapKeys = require('deep-map-keys');
+const path = require('path');
 
 /**
  * @summary Create a flattened copy of the object with all keys transformed in start case
@@ -81,3 +83,53 @@ exports.makeFlatStartCaseObject = (object) => {
   });
 
 };
+
+/**
+ * @summary Create an object clone with all absolute paths replaced with the path basename
+ * @function
+ * @public
+ *
+ * @param {Object} object - original object
+ * @returns {Object} transformed object
+ *
+ * @example
+ * const anonymized = utils.hideAbsolutePathsInObject({
+ *   path1: '/home/john/rpi.img',
+ *   simpleProperty: null,
+ *   nested: {
+ *     path2: '/home/john/another-image.img',
+ *     path3: 'yet-another-image.img',
+ *     otherProperty: false
+ *   }
+ * });
+ *
+ * console.log(anonymized);
+ * > {
+ * >   path1: 'rpi.img',
+ * >   simpleProperty: null,
+ * >   nested: {
+ * >     path2: 'another-image.img',
+ * >     path3: 'yet-another-image.img',
+ * >     otherProperty: false
+ * >   }
+ * > }
+ */
+exports.hideAbsolutePathsInObject = (object) => {
+  return _.deepMapValues(object, (value) => {
+    if (!_.isString(value)) {
+      return value;
+    }
+
+    // Don't alter disk devices, even though they appear as full paths
+    if (_.some([
+      _.startsWith(value, '/dev/'),
+      _.startsWith(value, '\\\\.\\')
+    ])) {
+      return value;
+    }
+
+    return path.isAbsolute(value) ? path.basename(value) : value;
+  });
+
+};
+

npm-shrinkwrap.json

@@ -597,6 +597,11 @@
       "from": "lodash@>=4.5.1 <5.0.0",
       "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.13.1.tgz"
     },
+    "lodash-deep": {
+      "version": "2.0.0",
+      "from": "lodash-deep@2.0.0",
+      "resolved": "https://registry.npmjs.org/lodash-deep/-/lodash-deep-2.0.0.tgz"
+    },
     "lodash-es": {
       "version": "4.13.1",
       "from": "lodash-es@>=4.2.1 <5.0.0",

package.json

@@ -87,6 +87,7 @@
     "is-elevated": "^1.0.0",
     "isstream": "^0.1.2",
     "lodash": "^4.5.1",
+    "lodash-deep": "^2.0.0",
     "lzma-native": "^1.5.2",
     "node-ipc": "^8.9.2",
     "node-stream-zip": "^1.3.4",

tests/shared/utils.spec.js

@@ -18,6 +18,7 @@
 
 const m = require('mochainon');
 const utils = require('../../lib/shared/utils');
+const path = require('path');
 
 describe('Shared: Utils', function() {
 
@@ -124,4 +125,212 @@ describe('Shared: Utils', function() {
 
   });
 
+  describe('hideAbsolutePathsInObject()', function() {
+
+    it('should return undefined if given undefined', function() {
+      m.chai.expect(utils.hideAbsolutePathsInObject(undefined)).to.be.undefined;
+    });
+
+    it('should return null if given null', function() {
+      m.chai.expect(utils.hideAbsolutePathsInObject(null)).to.be.null;
+    });
+
+    it('should return a clone of the object if there are no paths in the object', function() {
+      const object = {
+        numberProperty: 1,
+        nested: {
+          otherProperty: 'value'
+        }
+      };
+      m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.not.equal(object);
+      m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal(object);
+    });
+
+    describe('given UNIX paths', function() {
+
+      beforeEach(function() {
+        this.isAbsolute = path.isAbsolute;
+        this.basename = path.basename;
+        path.isAbsolute = path.posix.isAbsolute;
+        path.basename = path.posix.basename;
+      });
+
+      afterEach(function() {
+        path.isAbsolute = this.isAbsolute;
+        path.basename = this.basename;
+      });
+
+      it('should replace absolute paths with the basename', function() {
+        const object = {
+          prop1: 'some value',
+          prop2: '/home/john/rpi.img'
+        };
+
+        m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal({
+          prop1: 'some value',
+          prop2: 'rpi.img'
+        });
+      });
+
+      it('should replace nested absolute paths with the basename', function() {
+        const object = {
+          nested: {
+            path: '/home/john/rpi.img'
+          }
+        };
+
+        m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal({
+          nested: {
+            path: 'rpi.img'
+          }
+        });
+      });
+
+      it('should not alter /dev/sdb', function() {
+        const object = {
+          nested: {
+            path: '/dev/sdb'
+          }
+        };
+
+        m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal({
+          nested: {
+            path: '/dev/sdb'
+          }
+        });
+      });
+
+      it('should not alter relative paths', function() {
+        const object = {
+          path: 'foo/bar'
+        };
+
+        m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal({
+          path: 'foo/bar'
+        });
+      });
+
+      it('should handle arrays', function() {
+        m.chai.expect(utils.hideAbsolutePathsInObject({
+          foo: 'foo',
+          bar: [
+            {
+              path: '/foo/bar/baz'
+            },
+            {
+              path: '/foo/bar/baz'
+            },
+            {
+              path: '/foo/bar/baz'
+            }
+          ]
+        })).to.deep.equal({
+          foo: 'foo',
+          bar: [
+            {
+              path: 'baz'
+            },
+            {
+              path: 'baz'
+            },
+            {
+              path: 'baz'
+            }
+          ]
+        });
+      });
+
+    });
+
+    describe('given Windows paths', function() {
+
+      beforeEach(function() {
+        this.isAbsolute = path.isAbsolute;
+        this.basename = path.basename;
+        path.isAbsolute = path.win32.isAbsolute;
+        path.basename = path.win32.basename;
+      });
+
+      afterEach(function() {
+        path.isAbsolute = this.isAbsolute;
+        path.basename = this.basename;
+      });
+
+      it('should replace absolute paths with the basename', function() {
+        const object = {
+          prop1: 'some value',
+          prop2: 'C:\\Users\\John\\rpi.img'
+        };
+
+        m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal({
+          prop1: 'some value',
+          prop2: 'rpi.img'
+        });
+
+      });
+
+      it('should replace nested absolute paths with the basename', function() {
+        const object = {
+          nested: {
+            path: 'C:\\Users\\John\\rpi.img'
+          }
+        };
+
+        m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal({
+          nested: {
+            path: 'rpi.img'
+          }
+        });
+      });
+
+      it('should not alter \\\\.\\PHYSICALDRIVE1', function() {
+        const object = {
+          nested: {
+            path: '\\\\.\\PHYSICALDRIVE1'
+          }
+        };
+
+        m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal({
+          nested: {
+            path: '\\\\.\\PHYSICALDRIVE1'
+          }
+        });
+      });
+
+      it('should not alter relative paths', function() {
+        const object = {
+          path: 'foo\\bar'
+        };
+
+        m.chai.expect(utils.hideAbsolutePathsInObject(object)).to.deep.equal({
+          path: 'foo\\bar'
+        });
+      });
+
+      it('should handle arrays', function() {
+        m.chai.expect(utils.hideAbsolutePathsInObject({
+          foo: 'foo',
+          bar: [ {
+            path: 'C:\\foo\\bar\\baz'
+          }, {
+            path: 'C:\\foo\\bar\\baz'
+          }, {
+            path: 'C:\\foo\\bar\\baz'
+          } ]
+        })).to.deep.equal({
+          foo: 'foo',
+          bar: [ {
+            path: 'baz'
+          }, {
+            path: 'baz'
+          }, {
+            path: 'baz'
+          } ]
+        });
+      });
+
+    });
+
+  });
+
 });