From fe20f5061f1f8c963d02ef54131e17c1f89db8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C8=98tefan=20Daniel=20Mih=C4=83il=C4=83?= Date: Wed, 15 Mar 2017 04:05:48 +0000 Subject: [PATCH] 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 | 9 +- lib/shared/utils.js | 52 +++++++++ npm-shrinkwrap.json | 5 + package.json | 1 + tests/shared/utils.spec.js | 209 +++++++++++++++++++++++++++++++++++ 5 files changed, 272 insertions(+), 4 deletions(-) diff --git a/lib/gui/modules/analytics.js b/lib/gui/modules/analytics.js index 130b649b..75057217 100644 --- a/lib/gui/modules/analytics.js +++ b/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; diff --git a/lib/shared/utils.js b/lib/shared/utils.js index 99fac3b6..726f0f37 100644 --- a/lib/shared/utils.js +++ b/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; + }); + +}; + diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 22f3dc55..a95289e1 100644 --- a/npm-shrinkwrap.json +++ b/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", diff --git a/package.json b/package.json index 2899f38c..ea071058 100644 --- a/package.json +++ b/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", diff --git a/tests/shared/utils.spec.js b/tests/shared/utils.spec.js index a57b2cda..234dd03b 100644 --- a/tests/shared/utils.spec.js +++ b/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' + } ] + }); + }); + + }); + + }); + });