diff --git a/.eslintrc.js b/.eslintrc.js index 124289e01..5f0bcfb1e 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -12,6 +12,7 @@ module.exports = { ], plugins: [ + 'mocha', 'more', ], @@ -33,6 +34,9 @@ module.exports = { ignoreUrls: true, }], + // prevents us from accidentally checking in exclusive tests (`.only`): + 'mocha/no-exclusive-tests': 'error', + // encourage consistent use of `async` / `await` instead of `then` 'more/no-then': 'error', diff --git a/js/background.js b/js/background.js index 506874823..f78836b36 100644 --- a/js/background.js +++ b/js/background.js @@ -14,7 +14,7 @@ ;(function() { 'use strict'; - const { Message } = window.Signal.Types; + const { Errors, Message } = window.Signal.Types; // Implicitly used in `indexeddb-backbonejs-adapter`: // https://github.com/signalapp/Signal-Desktop/blob/4033a9f8137e62ed286170ed5d4941982b1d3a64/components/indexeddb-backbonejs-adapter/backbone-indexeddb.js#L569 @@ -414,7 +414,7 @@ }); var error = c.validateNumber(); if (error) { - console.log('Invalid contact received', error && error.stack ? error.stack : error); + console.log('Invalid contact received:', Errors.toLogFormat(error)); return; } @@ -483,7 +483,7 @@ .catch(function(error) { console.log( 'onContactReceived error:', - error && error.stack ? error.stack : error + Errors.toLogFormat(error) ); }); } @@ -670,7 +670,7 @@ return resolve(false); }); }).catch(function(error) { - console.log('isMessageDuplicate error:', error && error.stack ? error.stack : error); + console.log('isMessageDuplicate error:', Errors.toLogFormat(error)); return false; }); } @@ -691,7 +691,7 @@ function onError(ev) { var error = ev.error; - console.log('background onError:', error && error.stack ? error.stack : error); + console.log('background onError:', Errors.toLogFormat(error)); if (error.name === 'HTTPError' && (error.code == 401 || error.code == 403)) { Whisper.Registration.remove(); @@ -804,8 +804,8 @@ var error = c.validateNumber(); if (error) { console.log( - 'Invalid verified sync received', - error && error.stack ? error.stack : error + 'Invalid verified sync received:', + Errors.toLogFormat(error) ); return; } diff --git a/js/logging.js b/js/logging.js index 45cd0a7e5..2538503b0 100644 --- a/js/logging.js +++ b/js/logging.js @@ -7,11 +7,9 @@ const bunyan = require('bunyan'); const _ = require('lodash'); const debuglogs = require('./modules/debuglogs'); - +const Privacy = require('./modules/privacy'); const ipc = electron.ipcRenderer; -const PHONE_REGEX = /\+\d{7,12}(\d{3})/g; -const GROUP_REGEX = /(group\()([^)]+)(\))/g; // Default Bunyan levels: https://github.com/trentm/node-bunyan#levels // To make it easier to visually scan logs, we make all levels the same length @@ -25,20 +23,7 @@ const LEVELS = { 10: 'trace', }; - // Backwards-compatible logging, simple strings and no level (defaulted to INFO) - -function redactPhone(text) { - return text.replace(PHONE_REGEX, '+[REDACTED]$1'); -} - -function redactGroup(text) { - return text.replace( - GROUP_REGEX, - (match, before, id, after) => `${before}[REDACTED]${id.slice(-3)}${after}` - ); -} - function now() { const date = new Date(); return date.toJSON(); @@ -60,8 +45,9 @@ function log(...args) { return item; }); - const toSend = redactGroup(redactPhone(str.join(' '))); - ipc.send('log-info', toSend); + + const logText = Privacy.redactAll(str.join(' ')); + ipc.send('log-info', logText); } if (window.console) { @@ -95,7 +81,7 @@ function formatLine(entry) { } function format(entries) { - return redactGroup(redactPhone(entries.map(formatLine).join('\n'))); + return Privacy.redactAll(entries.map(formatLine).join('\n')); } function fetch() { diff --git a/js/modules/global_errors.js b/js/modules/global_errors.js new file mode 100644 index 000000000..b868bd0a5 --- /dev/null +++ b/js/modules/global_errors.js @@ -0,0 +1,17 @@ +const addUnhandledErrorHandler = require('electron-unhandled'); + +const Errors = require('./types/errors'); + + +// addHandler :: Unit -> Unit +exports.addHandler = () => { + addUnhandledErrorHandler({ + logger: (error) => { + console.error( + 'Uncaught error or unhandled promise rejection:', + Errors.toLogFormat(error) + ); + }, + showDialog: false, + }); +}; diff --git a/js/modules/privacy.js b/js/modules/privacy.js new file mode 100644 index 000000000..db0529376 --- /dev/null +++ b/js/modules/privacy.js @@ -0,0 +1,67 @@ +/* eslint-env node */ + +const Path = require('path'); + +const compose = require('lodash/fp/compose'); +const escapeRegExp = require('lodash/escapeRegExp'); +const isRegExp = require('lodash/isRegExp'); +const isString = require('lodash/isString'); + + +const PHONE_NUMBER_PATTERN = /\+\d{7,12}(\d{3})/g; +const GROUP_ID_PATTERN = /(group\()([^)]+)(\))/g; + +const APP_ROOT_PATH = Path.join(__dirname, '..', '..', '..'); +const APP_ROOT_PATH_PATTERN = (() => { + try { + // Safe `String::replaceAll`: + // https://github.com/lodash/lodash/issues/1084#issuecomment-86698786 + return new RegExp(escapeRegExp(APP_ROOT_PATH), 'g'); + } catch (error) { + return null; + } +})(); + +const REDACTION_PLACEHOLDER = '[REDACTED]'; + +// redactPhoneNumbers :: String -> String +exports.redactPhoneNumbers = (text) => { + if (!isString(text)) { + throw new TypeError('`text` must be a string'); + } + + return text.replace(PHONE_NUMBER_PATTERN, `+${REDACTION_PLACEHOLDER}$1`); +}; + +// redactGroupIds :: String -> String +exports.redactGroupIds = (text) => { + if (!isString(text)) { + throw new TypeError('`text` must be a string'); + } + + return text.replace( + GROUP_ID_PATTERN, + (match, before, id, after) => + `${before}${REDACTION_PLACEHOLDER}${id.slice(-3)}${after}` + ); +}; + +// redactSensitivePaths :: String -> String +exports.redactSensitivePaths = (text) => { + if (!isString(text)) { + throw new TypeError('`text` must be a string'); + } + + if (!isRegExp(APP_ROOT_PATH_PATTERN)) { + return text; + } + + return text.replace(APP_ROOT_PATH_PATTERN, REDACTION_PLACEHOLDER); +}; + +// redactAll :: String -> String +exports.redactAll = compose( + exports.redactSensitivePaths, + exports.redactGroupIds, + exports.redactPhoneNumbers +); diff --git a/js/modules/types/errors.js b/js/modules/types/errors.js new file mode 100644 index 000000000..0914eacc2 --- /dev/null +++ b/js/modules/types/errors.js @@ -0,0 +1,12 @@ +// toLogFormat :: Error -> String +exports.toLogFormat = (error) => { + if (!error) { + return error; + } + + if (error && error.stack) { + return error.stack; + } + + return error.toString(); +}; diff --git a/main.js b/main.js index a90ed0009..0ea657e45 100644 --- a/main.js +++ b/main.js @@ -18,10 +18,13 @@ const packageJson = require('./package.json'); const autoUpdate = require('./app/auto_update'); const createTrayIcon = require('./app/tray_icon'); +const GlobalErrors = require('./js/modules/global_errors'); const logging = require('./app/logging'); const windowState = require('./app/window_state'); const { createTemplate } = require('./app/menu'); +GlobalErrors.addHandler(); + const appUserModelId = `org.whispersystems.${packageJson.name}`; console.log('Set Windows Application User Model ID (AUMID)', { appUserModelId }); app.setAppUserModelId(appUserModelId); @@ -415,6 +418,7 @@ app.on('ready', () => { logging.initialize().catch((error) => { loggingSetupError = error; }).then(() => { + /* eslint-enable more/no-then */ logger = logging.getLogger(); logger.info('app ready'); @@ -439,7 +443,6 @@ app.on('ready', () => { setupMenu(); }); - /* eslint-enable more/no-then */ }); function setupMenu(options) { diff --git a/package.json b/package.json index d2101e3c4..4e40bd8c1 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,7 @@ "electron-config": "^1.0.0", "electron-editor-context-menu": "^1.1.1", "electron-is-dev": "^0.3.0", + "electron-unhandled": "https://github.com/gasi/electron-unhandled.git#1edf81fe542e505368fafaeef27609dc21678f8c", "electron-updater": "^2.21.0", "emoji-datasource": "4.0.0", "emoji-datasource-apple": "4.0.0", @@ -85,6 +86,7 @@ "eslint": "^4.14.0", "eslint-config-airbnb-base": "^12.1.0", "eslint-plugin-import": "^2.8.0", + "eslint-plugin-mocha": "^4.12.1", "eslint-plugin-more": "^0.3.1", "extract-zip": "^1.6.6", "grunt": "^1.0.1", diff --git a/preload.js b/preload.js index 9c99e38c0..ba280ab20 100644 --- a/preload.js +++ b/preload.js @@ -104,6 +104,7 @@ window.Signal.OS = require('./js/modules/os'); window.Signal.Types = window.Signal.Types || {}; window.Signal.Types.Attachment = require('./js/modules/types/attachment'); + window.Signal.Types.Errors = require('./js/modules/types/errors'); window.Signal.Types.Message = require('./js/modules/types/message'); window.Signal.Types.MIME = require('./js/modules/types/mime'); window.Signal.Types.Settings = require('./js/modules/types/settings'); diff --git a/test/modules/privacy_test.js b/test/modules/privacy_test.js new file mode 100644 index 000000000..156d3e826 --- /dev/null +++ b/test/modules/privacy_test.js @@ -0,0 +1,56 @@ +const Path = require('path'); + +const { assert } = require('chai'); + +const Privacy = require('../../js/modules/privacy'); + + +const APP_ROOT_PATH = Path.join(__dirname, '..', '..', '..'); + +describe('Privacy', () => { + describe('redactPhoneNumbers', () => { + it('should redact all phone numbers', () => { + const text = 'This is a log line with a phone number +12223334455\n' + + 'and another one +13334445566'; + + const actual = Privacy.redactPhoneNumbers(text); + const expected = 'This is a log line with a phone number +[REDACTED]455\n' + + 'and another one +[REDACTED]566'; + assert.equal(actual, expected); + }); + }); + + describe('redactGroupIds', () => { + it('should redact all group IDs', () => { + const text = 'This is a log line with two group IDs: group(123456789)\n' + + 'and group(abcdefghij)'; + + const actual = Privacy.redactGroupIds(text); + const expected = 'This is a log line with two group IDs: group([REDACTED]789)\n' + + 'and group([REDACTED]hij)'; + assert.equal(actual, expected); + }); + }); + + describe('redactAll', () => { + it('should redact all sensitive information', () => { + const text = 'This is a log line with sensitive information:\n' + + `path1 ${APP_ROOT_PATH}/main.js\n` + + 'phone1 +12223334455 ipsum\n' + + 'group1 group(123456789) doloret\n' + + `path2 file:///${APP_ROOT_PATH}/js/background.js.` + + 'phone2 +13334445566 lorem\n' + + 'group2 group(abcdefghij) doloret\n'; + + const actual = Privacy.redactAll(text); + const expected = 'This is a log line with sensitive information:\n' + + 'path1 [REDACTED]/main.js\n' + + 'phone1 +[REDACTED]455 ipsum\n' + + 'group1 group([REDACTED]789) doloret\n' + + 'path2 file:///[REDACTED]/js/background.js.' + + 'phone2 +[REDACTED]566 lorem\n' + + 'group2 group([REDACTED]hij) doloret\n'; + assert.equal(actual, expected); + }); + }); +}); diff --git a/test/modules/types/errors_test.js b/test/modules/types/errors_test.js new file mode 100644 index 000000000..9cca14062 --- /dev/null +++ b/test/modules/types/errors_test.js @@ -0,0 +1,43 @@ +const Path = require('path'); + +const { assert } = require('chai'); + +const Errors = require('../../../js/modules/types/errors'); + + +const APP_ROOT_PATH = Path.join(__dirname, '..', '..', '..'); + +describe('Errors', () => { + describe('toLogFormat', () => { + it('should return error stack trace if present', () => { + const error = new Error('boom'); + assert.typeOf(error, 'Error'); + + const formattedError = Errors.toLogFormat(error); + assert.include(formattedError, 'errors_test.js'); + assert.include(formattedError, APP_ROOT_PATH, 'Formatted stack has app path'); + }); + + it('should return error string representation if stack is missing', () => { + const error = new Error('boom'); + error.stack = null; + assert.typeOf(error, 'Error'); + assert.isNull(error.stack); + + const formattedError = Errors.toLogFormat(error); + assert.strictEqual(formattedError, 'Error: boom'); + }); + + [ + 0, + false, + null, + undefined, + ].forEach((value) => { + it(`should return \`${value}\` argument`, () => { + const formattedNonError = Errors.toLogFormat(value); + assert.strictEqual(formattedNonError, value); + }); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 39d5b3ead..f08d6c6b2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1538,6 +1538,10 @@ electron-publisher-s3@^20.2.0: fs-extra-p "^4.5.2" mime "^2.2.0" +"electron-unhandled@https://github.com/gasi/electron-unhandled.git#1edf81fe542e505368fafaeef27609dc21678f8c": + version "1.0.0" + resolved "https://github.com/gasi/electron-unhandled.git#1edf81fe542e505368fafaeef27609dc21678f8c" + electron-updater@^2.21.0: version "2.21.0" resolved "https://registry.yarnpkg.com/electron-updater/-/electron-updater-2.21.0.tgz#3c8765af946090100f7df982127e4c3412cbc1af" @@ -1678,6 +1682,12 @@ eslint-plugin-import@^2.8.0: minimatch "^3.0.3" read-pkg-up "^2.0.0" +eslint-plugin-mocha@^4.12.1: + version "4.12.1" + resolved "https://registry.yarnpkg.com/eslint-plugin-mocha/-/eslint-plugin-mocha-4.12.1.tgz#dbacc543b178b4536ec5b19d7f8e8864d85404bf" + dependencies: + ramda "^0.25.0" + eslint-plugin-more@^0.3.1: version "0.3.1" resolved "https://registry.yarnpkg.com/eslint-plugin-more/-/eslint-plugin-more-0.3.1.tgz#ff688fb3fa8f153c8bfd5d70c15a68dc222a1b31" @@ -4314,6 +4324,10 @@ querystring@0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/querystring/-/querystring-0.2.0.tgz#b209849203bb25df820da756e747005878521620" +ramda@^0.25.0: + version "0.25.0" + resolved "https://registry.yarnpkg.com/ramda/-/ramda-0.25.0.tgz#8fdf68231cffa90bc2f9460390a0cb74a29b29a9" + randomatic@^1.1.3: version "1.1.7" resolved "https://registry.yarnpkg.com/randomatic/-/randomatic-1.1.7.tgz#c7abe9cc8b87c0baa876b19fde83fd464797e38c"