diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 805163cc135b..9e8fca0b1f62 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -95,6 +95,8 @@ function getClientOptions() { // which would otherwise make the next error look like a different stack (background timers // usually run after beforeSend finished; rapid UI captures often dedupe first). beforeSend: (report) => rewriteReport(safeCloneReport(report)), + beforeSendTransaction: (report) => + rewriteTransactionReport(safeCloneReport(report)), debug: METAMASK_DEBUG, dist: isManifestV3 ? 'mv3' : 'mv2', dsn: sentryTarget, @@ -291,8 +293,7 @@ export function beforeBreadcrumb() { ) { return null; } - const newBreadcrumb = removeUrlsFromBreadCrumb(breadcrumb); - return newBreadcrumb; + return breadcrumb; }; } @@ -338,8 +339,9 @@ export function shouldCreateSpanForRequest(url) { /** * Receives a Sentry breadcrumb object and potentially removes urls - * from its `data` property, it particular those possibly found at - * data.from, data.to and data.url + * from its `data` property, in particular those possibly found at + * data.from, data.to and data.url. Performs a deep address scrub for use when + * an event is about to be sent (not on every breadcrumb capture). * * @param {object} breadcrumb - A Sentry breadcrumb object: https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/ * @returns {object} A modified Sentry breadcrumb object. @@ -365,6 +367,35 @@ export function removeUrlsFromBreadCrumb(breadcrumb) { return breadcrumb; } +/** + * Deep-scrubs all breadcrumbs attached to an outbound Sentry event (errors and + * transactions). The report must already be cloned (see `safeCloneReport` in + * `beforeSend` / `beforeSendTransaction`) so breadcrumb mutation is safe. + * + * @param {object} report - A Sentry event object. + */ +export function sanitizeBreadcrumbsInReport(report) { + if (!Array.isArray(report.breadcrumbs)) { + return; + } + for (let i = 0; i < report.breadcrumbs.length; i++) { + removeUrlsFromBreadCrumb(report.breadcrumbs[i]); + } +} + +/** + * Scrubs breadcrumb payloads on performance transaction events before send. + * {@link rewriteReport} handles errors via `beforeSend`; transactions use + * `beforeSendTransaction` instead. + * + * @param {object} report - A Sentry transaction event object. + * @returns {object} The modified report (same reference). + */ +export function rewriteTransactionReport(report) { + sanitizeBreadcrumbsInReport(report); + return report; +} + /** * Receives a Sentry event object and modifies it before the error is sent to Sentry. * Sanitizes messages/URLs and attaches app state. @@ -383,6 +414,8 @@ export function rewriteReport(report) { // but putting the code here as well gives public visibility to how we are handling // privacy with respect to sentry. sanitizeAddressesFromErrorMessages(report); + // Deep-scrub breadcrumb payloads only when an error is being sent. + sanitizeBreadcrumbsInReport(report); // Remove addresses from other error parameters (extra, contexts). // Done before attaching appState below so the (already masked) appState is // not re-walked. diff --git a/app/scripts/lib/setupSentry.test.js b/app/scripts/lib/setupSentry.test.js index 83fecf3e94cc..c92de7c01b9b 100644 --- a/app/scripts/lib/setupSentry.test.js +++ b/app/scripts/lib/setupSentry.test.js @@ -1,6 +1,8 @@ +import { cloneDeep } from 'lodash'; import { removeUrlsFromBreadCrumb, rewriteReport, + rewriteTransactionReport, shouldCreateSpanForRequest, } from './setupSentry'; @@ -274,6 +276,74 @@ describe('Setup Sentry', () => { rewriteReport(testReport); expect(testReport.contexts.account.address).toStrictEqual('**'); }); + + it('removes addresses from breadcrumbs when sending an error report', () => { + const testReport = { + message: 'An error occurred', + breadcrumbs: [ + { + message: 'console.error', + data: { + arguments: [ + new Error( + 'Failed for 0x790A8A9E9bc1C9dB991D8721a92e461Db4CfB235', + ), + ], + }, + }, + ], + request: {}, + }; + rewriteReport(testReport); + expect(testReport.breadcrumbs[0].data.arguments[0].message).toStrictEqual( + 'Failed for 0x**', + ); + }); + + it('scrubs breadcrumbs without mutating live source objects', () => { + const liveError = new Error( + 'Failed for 0x790A8A9E9bc1C9dB991D8721a92e461Db4CfB235', + ); + const liveArgs = [liveError]; + const testReport = cloneDeep({ + message: 'An error occurred', + breadcrumbs: [ + { + message: 'console.error', + data: { arguments: liveArgs, logger: 'console' }, + }, + ], + request: {}, + }); + + rewriteReport(testReport); + + expect(testReport.breadcrumbs[0].data.arguments[0].message).toStrictEqual( + 'Failed for 0x**', + ); + expect(liveError.message).toStrictEqual( + 'Failed for 0x790A8A9E9bc1C9dB991D8721a92e461Db4CfB235', + ); + }); + }); + + describe('rewriteTransactionReport', () => { + it('removes addresses from breadcrumbs when sending a transaction', () => { + const testReport = { + type: 'transaction', + transaction: 'ui.popup', + breadcrumbs: [ + { + message: 'fetch', + data: { + url: 'https://api.example.com/0x790A8A9E9bc1C9dB991D8721a92e461Db4CfB235', + }, + }, + ], + }; + rewriteTransactionReport(testReport); + expect(testReport.breadcrumbs[0].data.url).toStrictEqual(''); + }); }); describe('shouldCreateSpanForRequest', () => { diff --git a/ui/pages/onboarding-flow/creation-successful/wallet-ready-animation.tsx b/ui/pages/onboarding-flow/creation-successful/wallet-ready-animation.tsx index 73b2aae025db..ae7f285d29ea 100644 --- a/ui/pages/onboarding-flow/creation-successful/wallet-ready-animation.tsx +++ b/ui/pages/onboarding-flow/creation-successful/wallet-ready-animation.tsx @@ -56,7 +56,7 @@ export default function WalletReadyAnimation() { // Trigger the animation start when rive is loaded useEffect(() => { if (rive && isWasmReady && !bufferLoading && buffer) { - console.log('rive is loaded', rive); + console.log('rive is loaded'); const inputs = rive.stateMachineInputs('OnboardingLoader'); if (inputs) { const darkToggle = inputs.find((input) => input.name === 'Dark mode');