From de6653a997242ce0baa7d2f59f0a8e5c73d8d5a0 Mon Sep 17 00:00:00 2001 From: Ramya Achutha Rao Date: Tue, 3 Apr 2018 16:40:58 -0700 Subject: [PATCH] Retain paths starting from node_modules and match escaped backslashes --- .../telemetry/common/telemetryService.ts | 7 +- .../electron-browser/telemetryService.test.ts | 82 ++++++++++++++++--- 2 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/vs/platform/telemetry/common/telemetryService.ts b/src/vs/platform/telemetry/common/telemetryService.ts index 2f713dd11afd8..992f3e36a9625 100644 --- a/src/vs/platform/telemetry/common/telemetryService.ts +++ b/src/vs/platform/telemetry/common/telemetryService.ts @@ -130,15 +130,16 @@ export class TelemetryService implements ITelemetryService { } } - const fileRegex = /(file:\/\/)?([a-z,A-Z]:)?([\\\/]\w+)+/g; + const nodeModulesRegex = /^[\\\/]?(node_modules|node_modules\.asar)[\\\/]/; + const fileRegex = /(file:\/\/)?([a-zA-Z]:(\\\\|\\|\/)|(\\\\|\\|\/))?([\w-\._]+(\\\\|\\|\/))+[\w-\._]*/g; let updatedStack = stack; while (true) { const result = fileRegex.exec(stack); if (!result) { break; } - // Anoynimize user file paths that do not need cleanup. - if (cleanUpIndexes.every(([x, y]) => result.index < x || result.index >= y)) { + // Anoynimize user file paths that do not need to be retained or cleaned up. + if (!nodeModulesRegex.test(result[0]) && cleanUpIndexes.every(([x, y]) => result.index < x || result.index >= y)) { updatedStack = updatedStack.slice(0, result.index) + result[0].replace(/./g, 'a') + updatedStack.slice(fileRegex.lastIndex); } } diff --git a/src/vs/platform/telemetry/test/electron-browser/telemetryService.test.ts b/src/vs/platform/telemetry/test/electron-browser/telemetryService.test.ts index d3062bc7ff2c0..39fe577f0e556 100644 --- a/src/vs/platform/telemetry/test/electron-browser/telemetryService.test.ts +++ b/src/vs/platform/telemetry/test/electron-browser/telemetryService.test.ts @@ -50,8 +50,10 @@ class ErrorTestingSettings { public noSuchFilePrefix: string; public noSuchFileMessage: string; public stack: string[]; - public randomUserFile: string = 'a/path/that/doesnt/contain/code/names'; - public anonymizedRandomUserFile: string = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; + public randomUserFile: string = 'a/path/that/doe_snt/con-tain/code/names.js'; + public anonymizedRandomUserFile: string = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; + public nodeModulePathToRetain: string = 'node_modules/path/that/shouldbe/retained/names.js:14:15854'; + public nodeModuleAsarPathToRetain: string = 'node_modules.asar/path/that/shouldbe/retained/names.js:14:12354'; constructor() { this.personalInfo = 'DANGEROUS/PATH'; @@ -66,16 +68,16 @@ class ErrorTestingSettings { this.noSuchFilePrefix = 'ENOENT: no such file or directory'; this.noSuchFileMessage = this.noSuchFilePrefix + ' \'' + this.personalInfo + '\''; - this.stack = [`at e._modelEvents (${this.randomUserFile}.js:11:7309)`, - ` at t.AllWorkers (${this.randomUserFile}.js:6:8844)`, - ` at e.(anonymous function) [as _modelEvents] (${this.randomUserFile}.js:5:29552)`, - ` at Function. (${this.randomUserFile}.js:6:8272)`, - ` at e.dispatch (${this.randomUserFile}.js:5:26931)`, - ` at e.request (${this.randomUserFile}.js:14:1745)`, - ' at t._handleMessage (another/path/that/doesnt/contain/code/names.js:14:17447)', - ' at t._onmessage (another/path/that/doesnt/contain/code/names.js:14:16976)', - ' at t.onmessage (another/path/that/doesnt/contain/code/names.js:14:15854)', - ' at DedicatedWorkerGlobalScope.self.onmessage', + this.stack = [`at e._modelEvents (${this.randomUserFile}:11:7309)`, + ` at t.AllWorkers (${this.randomUserFile}:6:8844)`, + ` at e.(anonymous function) [as _modelEvents] (${this.randomUserFile}:5:29552)`, + ` at Function. (${this.randomUserFile}:6:8272)`, + ` at e.dispatch (${this.randomUserFile}:5:26931)`, + ` at e.request (/${this.nodeModuleAsarPathToRetain})`, + ` at t._handleMessage (${this.nodeModuleAsarPathToRetain})`, + ` at t._onmessage (/${this.nodeModulePathToRetain})`, + ` at t.onmessage (${this.nodeModulePathToRetain})`, + ` at DedicatedWorkerGlobalScope.self.onmessage`, this.dangerousPathWithImportantInfo, this.dangerousPathWithoutImportantInfo, this.missingModelMessage, @@ -461,6 +463,62 @@ suite('TelemetryService', () => { service.dispose(); })); + test('Unexpected Error Telemetry removes PII but preserves Code file path with node modules', sinon.test(function (this: any) { + + let origErrorHandler = Errors.errorHandler.getUnexpectedErrorHandler(); + Errors.setUnexpectedErrorHandler(() => { }); + + try { + let settings = new ErrorTestingSettings(); + let testAppender = new TestTelemetryAppender(); + let service = new TelemetryService({ appender: testAppender }, undefined); + const errorTelemetry = new ErrorTelemetry(service); + + let dangerousPathWithImportantInfoError: any = new Error(settings.dangerousPathWithImportantInfo); + dangerousPathWithImportantInfoError.stack = settings.stack; + + + Errors.onUnexpectedError(dangerousPathWithImportantInfoError); + this.clock.tick(ErrorTelemetry.ERROR_FLUSH_TIMEOUT); + + assert.notEqual(testAppender.events[0].data.stack.indexOf('(' + settings.nodeModuleAsarPathToRetain), -1); + assert.notEqual(testAppender.events[0].data.stack.indexOf('(' + settings.nodeModulePathToRetain), -1); + assert.notEqual(testAppender.events[0].data.stack.indexOf('(/' + settings.nodeModuleAsarPathToRetain), -1); + assert.notEqual(testAppender.events[0].data.stack.indexOf('(/' + settings.nodeModulePathToRetain), -1); + + errorTelemetry.dispose(); + service.dispose(); + } + finally { + Errors.setUnexpectedErrorHandler(origErrorHandler); + } + })); + + test('Uncaught Error Telemetry removes PII but preserves Code file path', sinon.test(function (this: any) { + let errorStub = sinon.stub(); + window.onerror = errorStub; + let settings = new ErrorTestingSettings(); + let testAppender = new TestTelemetryAppender(); + let service = new TelemetryService({ appender: testAppender }, undefined); + const errorTelemetry = new ErrorTelemetry(service); + + let dangerousPathWithImportantInfoError: any = new Error('dangerousPathWithImportantInfo'); + dangerousPathWithImportantInfoError.stack = settings.stack; + (window.onerror)(settings.dangerousPathWithImportantInfo, 'test.js', 2, 42, dangerousPathWithImportantInfoError); + this.clock.tick(ErrorTelemetry.ERROR_FLUSH_TIMEOUT); + + assert.equal(errorStub.callCount, 1); + + assert.notEqual(testAppender.events[0].data.stack.indexOf('(' + settings.nodeModuleAsarPathToRetain), -1); + assert.notEqual(testAppender.events[0].data.stack.indexOf('(' + settings.nodeModulePathToRetain), -1); + assert.notEqual(testAppender.events[0].data.stack.indexOf('(/' + settings.nodeModuleAsarPathToRetain), -1); + assert.notEqual(testAppender.events[0].data.stack.indexOf('(/' + settings.nodeModulePathToRetain), -1); + + errorTelemetry.dispose(); + service.dispose(); + })); + + test('Unexpected Error Telemetry removes PII but preserves Code file path when PIIPath is configured', sinon.test(function (this: any) { let origErrorHandler = Errors.errorHandler.getUnexpectedErrorHandler();