From 8dca102eb70b9f79a6c752b5fbcb75ec6a154195 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 27 May 2024 16:05:32 +0200 Subject: [PATCH] fix(browser): Improve browser extension error message check (#12146) Make our check for when we abort and log an error due to `Sentry.init` being used in a browser extension a bit more fine-gained. In particular, we now do not abort the SDK initialization if we detect that the SDK is running in a browser-extension dedicated window (e.g. a URL starting with `chrome-extension://`). --- packages/browser/src/sdk.ts | 40 ++++++++++++++++---------- packages/browser/test/unit/sdk.test.ts | 31 ++++++++++++++++++-- 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index ba058c966754..ea24a475d959 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -60,22 +60,32 @@ function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions { return { ...defaultOptions, ...optionsArg }; } +type ExtensionProperties = { + chrome?: Runtime; + browser?: Runtime; +}; +type Runtime = { + runtime?: { + id?: string; + }; +}; + function shouldShowBrowserExtensionError(): boolean { - const windowWithMaybeChrome = WINDOW as typeof WINDOW & { chrome?: { runtime?: { id?: string } } }; - const isInsideChromeExtension = - windowWithMaybeChrome && - windowWithMaybeChrome.chrome && - windowWithMaybeChrome.chrome.runtime && - windowWithMaybeChrome.chrome.runtime.id; - - const windowWithMaybeBrowser = WINDOW as typeof WINDOW & { browser?: { runtime?: { id?: string } } }; - const isInsideBrowserExtension = - windowWithMaybeBrowser && - windowWithMaybeBrowser.browser && - windowWithMaybeBrowser.browser.runtime && - windowWithMaybeBrowser.browser.runtime.id; - - return !!isInsideBrowserExtension || !!isInsideChromeExtension; + const windowWithMaybeExtension = WINDOW as typeof WINDOW & ExtensionProperties; + + const extensionKey = windowWithMaybeExtension.chrome ? 'chrome' : 'browser'; + const extensionObject = windowWithMaybeExtension[extensionKey]; + + const runtimeId = extensionObject && extensionObject.runtime && extensionObject.runtime.id; + const href = (WINDOW.location && WINDOW.location.href) || ''; + + const extensionProtocols = ['chrome-extension:', 'moz-extension:', 'ms-browser-extension:']; + + // Running the SDK in a dedicated extension page and calling Sentry.init is fine; no risk of data leakage + const isDedicatedExtensionPage = + !!runtimeId && WINDOW === WINDOW.top && extensionProtocols.some(protocol => href.startsWith(`${protocol}//`)); + + return !!runtimeId && !isDedicatedExtensionPage; } /** diff --git a/packages/browser/test/unit/sdk.test.ts b/packages/browser/test/unit/sdk.test.ts index f8f5125ff896..b0af70fcd652 100644 --- a/packages/browser/test/unit/sdk.test.ts +++ b/packages/browser/test/unit/sdk.test.ts @@ -135,6 +135,8 @@ describe('init', () => { new MockIntegration('MockIntegration 0.2'), ]; + const originalLocation = WINDOW.location || {}; + const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, defaultIntegrations: DEFAULT_INTEGRATIONS }); afterEach(() => { @@ -142,7 +144,7 @@ describe('init', () => { Object.defineProperty(WINDOW, 'browser', { value: undefined, writable: true }); }); - it('should log a browser extension error if executed inside a Chrome extension', () => { + it('logs a browser extension error if executed inside a Chrome extension', () => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); Object.defineProperty(WINDOW, 'chrome', { @@ -160,7 +162,7 @@ describe('init', () => { consoleErrorSpy.mockRestore(); }); - it('should log a browser extension error if executed inside a Firefox/Safari extension', () => { + it('logs a browser extension error if executed inside a Firefox/Safari extension', () => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); Object.defineProperty(WINDOW, 'browser', { value: { runtime: { id: 'mock-extension-id' } }, writable: true }); @@ -175,7 +177,30 @@ describe('init', () => { consoleErrorSpy.mockRestore(); }); - it('should not log a browser extension error if executed inside regular browser environment', () => { + it.each(['chrome-extension', 'moz-extension', 'ms-browser-extension'])( + "doesn't log a browser extension error if executed inside an extension running in a dedicated page (%s)", + extensionProtocol => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + // @ts-expect-error - this is a hack to simulate a dedicated page in a browser extension + delete WINDOW.location; + // @ts-expect-error - this is a hack to simulate a dedicated page in a browser extension + WINDOW.location = { + href: `${extensionProtocol}://mock-extension-id/dedicated-page.html`, + }; + + Object.defineProperty(WINDOW, 'browser', { value: { runtime: { id: 'mock-extension-id' } }, writable: true }); + + init(options); + + expect(consoleErrorSpy).toBeCalledTimes(0); + + consoleErrorSpy.mockRestore(); + WINDOW.location = originalLocation; + }, + ); + + it("doesn't log a browser extension error if executed inside regular browser environment", () => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); init(options);