From b487a57400b8f97ae493a4c5527822bfa95410e2 Mon Sep 17 00:00:00 2001 From: Daniel Ayala <14967941+danielayala94@users.noreply.github.com> Date: Tue, 7 Jan 2025 09:18:33 -0800 Subject: [PATCH 1/3] [Telemetry] Expand field sanitization to codedError.data (#14161) * Scrub PII from codedError, add unit test * Nit: comment updates * Lint fixes * Change files * Enhanced bug fix using recursion, added test field cases * Lint fix * Sanitize any, and test fixes * Lint fix, again --- ...-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json | 7 ++ .../telemetry/src/e2etest/telemetry.test.ts | 82 +++++++++++++++++++ .../telemetry/src/telemetry.ts | 25 ++++++ 3 files changed, 114 insertions(+) create mode 100644 change/@react-native-windows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json diff --git a/change/@react-native-windows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json b/change/@react-native-windows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json new file mode 100644 index 00000000000..4505d14de6a --- /dev/null +++ b/change/@react-native-windows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Expand sanitization checks for error telemetry instances", + "packageName": "@react-native-windows/telemetry", + "email": "14967941+danielayala94@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/@react-native-windows/telemetry/src/e2etest/telemetry.test.ts b/packages/@react-native-windows/telemetry/src/e2etest/telemetry.test.ts index 790ff617a9f..ab238b6b14a 100644 --- a/packages/@react-native-windows/telemetry/src/e2etest/telemetry.test.ts +++ b/packages/@react-native-windows/telemetry/src/e2etest/telemetry.test.ts @@ -740,3 +740,85 @@ test.each(testTelemetryOptions)( }); }, ); + +test.each(testTelemetryOptions)( + 'Telemetry run test command end to end with CodedError, verifies PII is scrubbed if present in CodedError.', + async options => { + await TelemetryTest.startTest(options); + + const codedErrorInfo = new errorUtils.CodedError( + 'MSBuildError', // type + 'test error', // message + { + fieldWithPath: + 'Test Error occurred at C:\\some\\file\\path\\project.build.appxrecipe', // expectation: replace the whole C:\\... thing with "[path]". + fieldWithNoPath: 'Test Error data', // expectation: no changes to this string. + fieldWithNoString: 14, // expectation: no changes to this value. + arrayField: [ + 'No path', + 15, + 'Clean this path: C:\\some\\file\\path2\\project.build.appxrecipe', + [ + 'No path', + 150, + 'Also clean this: C:\\some\\file\\path2\\project.build.appxrecipe', + ], + ], + someObject: { + fieldWithPath: + 'Test Error occurred at C:\\some\\file\\path3\\project.build.appxrecipe', // expectation: replace the whole C:\\... thing with "[path]". + fieldWithNoPath: 'Test Error data 2', // expectation: no changes to this string. + fieldWithNoString: 16, // expectation: no changes to this value. + nestedObject: { + fieldWithPath: + 'Test Error occurred at C:\\some\\file\\path4\\project.build.appxrecipe', // expectation: replace the whole C:\\... thing with "[path]". + fieldWithNoPath: 'Test Error data 3', // expectation: no changes to this string. + fieldWithNoString: 17, // expectation: no changes to this value. + }, + }, + }, // data + ); + + const expectedError = new errorUtils.CodedError( + 'MSBuildError', // type + 'test error', // message + { + fieldWithPath: 'Test Error occurred at [path]', + fieldWithNoPath: 'Test Error data', + fieldWithNoString: 14, + arrayField: [ + 'No path', + 15, + 'Clean this path: [path]', + ['No path', 150, 'Also clean this: [path]'], + ], + someObject: { + fieldWithPath: 'Test Error occurred at [path]', + fieldWithNoPath: 'Test Error data 2', + fieldWithNoString: 16, + nestedObject: { + fieldWithPath: 'Test Error occurred at [path]', + fieldWithNoPath: 'Test Error data 3', + fieldWithNoString: 17, + }, + }, + }, // data + ); + + const caughtErrors: Error[] = []; + TelemetryTest.addTelemetryInitializer( + verifyTestCommandTelemetryProcessor( + caughtErrors, + expectedError.type, + expectedError, + ), + ); + + await runTestCommandE2E(() => testCommandBody(codedErrorInfo)); + + TelemetryTest.endTest(() => { + // Check if any errors were thrown + expect(caughtErrors).toHaveLength(0); + }); + }, +); diff --git a/packages/@react-native-windows/telemetry/src/telemetry.ts b/packages/@react-native-windows/telemetry/src/telemetry.ts index 509a578ac03..00303817482 100644 --- a/packages/@react-native-windows/telemetry/src/telemetry.ts +++ b/packages/@react-native-windows/telemetry/src/telemetry.ts @@ -467,6 +467,9 @@ export class Telemetry { } } + // Scrub any potential PII present in codedError.data array, as long as the data is a string. + codedErrorStruct.data = Telemetry.sanitizeAny(codedErrorStruct.data); + // Break down TS Error object into Exception Data const exceptionData = Telemetry.convertErrorIntoExceptionData(error); @@ -531,4 +534,26 @@ export class Telemetry { return exceptionData; } + + static sanitizeAny(data: any): any { + if (Array.isArray(data)) { + // This is an array, sanitize each element recursively. + return data.map(item => Telemetry.sanitizeAny(item)); + } else if (typeof data === 'object' && data !== null) { + // This is an object, sanitize each field recursively. + const sanitizedObject: Record = {}; + for (const key in data) { + if (Object.prototype.hasOwnProperty.call(data, key)) { + sanitizedObject[key] = Telemetry.sanitizeAny(data[key]); + } + } + return sanitizedObject; + } else if (typeof data === 'string') { + // The base case: this is a string, sanitize it. + return errorUtils.sanitizeErrorMessage(data); + } + + // Not a string, return the data unchanged. + return data; + } } From 5f8048308576e9fda6d93412d3bcafcb61983191 Mon Sep 17 00:00:00 2001 From: Daniel Ayala <14967941+danielayala94@users.noreply.github.com> Date: Tue, 7 Jan 2025 11:40:21 -0800 Subject: [PATCH 2/3] Change files --- ...ows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json | 7 +++++++ ...ows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 change/@react-native-windows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json delete mode 100644 change/@react-native-windows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json diff --git a/change/@react-native-windows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json b/change/@react-native-windows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json new file mode 100644 index 00000000000..9605afcbb44 --- /dev/null +++ b/change/@react-native-windows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "Expand telemetry field sanitization to codedError.data", + "packageName": "@react-native-windows/telemetry", + "email": "14967941+danielayala94@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/change/@react-native-windows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json b/change/@react-native-windows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json deleted file mode 100644 index 4505d14de6a..00000000000 --- a/change/@react-native-windows-telemetry-d7dc63f1-c900-4229-bcb4-8d7cd683293c.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "prerelease", - "comment": "Expand sanitization checks for error telemetry instances", - "packageName": "@react-native-windows/telemetry", - "email": "14967941+danielayala94@users.noreply.github.com", - "dependentChangeType": "patch" -} From 7f4bd55a8f2790de195afaa8d02ca622bbfda32d Mon Sep 17 00:00:00 2001 From: Daniel Ayala <14967941+danielayala94@users.noreply.github.com> Date: Tue, 7 Jan 2025 11:42:22 -0800 Subject: [PATCH 3/3] Change files --- ...ndows-telemetry-f5290e12-785f-4a77-bc1c-b91363b61740.json} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename change/{@react-native-windows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json => @react-native-windows-telemetry-f5290e12-785f-4a77-bc1c-b91363b61740.json} (76%) diff --git a/change/@react-native-windows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json b/change/@react-native-windows-telemetry-f5290e12-785f-4a77-bc1c-b91363b61740.json similarity index 76% rename from change/@react-native-windows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json rename to change/@react-native-windows-telemetry-f5290e12-785f-4a77-bc1c-b91363b61740.json index 9605afcbb44..476165ed4ac 100644 --- a/change/@react-native-windows-telemetry-94e3e17e-9be2-41cd-b5a6-712f3d7ff036.json +++ b/change/@react-native-windows-telemetry-f5290e12-785f-4a77-bc1c-b91363b61740.json @@ -1,7 +1,7 @@ { - "type": "none", + "type": "prerelease", "comment": "Expand telemetry field sanitization to codedError.data", "packageName": "@react-native-windows/telemetry", "email": "14967941+danielayala94@users.noreply.github.com", - "dependentChangeType": "none" + "dependentChangeType": "patch" }