Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(browser): Prevent initialization in browser extensions #10844

Merged
merged 4 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

// We mock this here to simulate a Firefox/Safari browser extension
window.browser = { runtime: { id: 'mock-extension-id' } };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
window.browser = { runtime: { id: 'mock-extension-id' } };
// We mock this here to simulate a browser extension
window.browser = { runtime: { id: 'mock-extension-id' } };

maybe leave a comment here for our future selves to know why this exists xD


Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { expect } from '@playwright/test';
import { sentryTest } from '../../../utils/fixtures';

sentryTest(
'should not initialize when inside a Firefox/Safari browser extension',
async ({ getLocalTestUrl, page }) => {
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const errorLogs: string[] = [];

page.on('console', message => {
if (message.type() === 'error') errorLogs.push(message.text());
});

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);

const isInitialized = await page.evaluate(() => {
return !!(window as any).Sentry.isInitialized();
});

expect(isInitialized).toEqual(false);
expect(errorLogs.length).toEqual(1);
expect(errorLogs[0]).toEqual(
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
);
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

// We mock this here to simulate a Chrome browser extension
window.chrome = { runtime: { id: 'mock-extension-id' } };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
window.chrome = { runtime: { id: 'mock-extension-id' } };
// We mock this here to simulate a browser extension
window.chrome = { runtime: { id: 'mock-extension-id' } };


Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect } from '@playwright/test';
import { sentryTest } from '../../../utils/fixtures';

sentryTest('should not initialize when inside a Chrome browser extension', async ({ getLocalTestUrl, page }) => {
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const errorLogs: string[] = [];

page.on('console', message => {
if (message.type() === 'error') errorLogs.push(message.text());
});

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);

const isInitialized = await page.evaluate(() => {
return !!(window as any).Sentry.isInitialized();
});

expect(isInitialized).toEqual(false);
expect(errorLogs.length).toEqual(1);
expect(errorLogs[0]).toEqual(
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
);
});
4 changes: 2 additions & 2 deletions packages/astro/test/client/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('Sentry client SDK', () => {
...tracingOptions,
});

const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations;
const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations || [];
const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing');

expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
Expand All @@ -93,7 +93,7 @@ describe('Sentry client SDK', () => {
enableTracing: true,
});

const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations;
const integrationsToInit = browserInit.mock.calls[0][0]?.defaultIntegrations || [];
const browserTracing = getClient<BrowserClient>()?.getIntegrationByName('BrowserTracing');

expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
Expand Down
64 changes: 45 additions & 19 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
import type { DsnLike, Integration, Options, UserFeedback } from '@sentry/types';
import {
addHistoryInstrumentationHandler,
consoleSandbox,
logger,
stackParserFromStackParserOptions,
supportsFetch,
Expand Down Expand Up @@ -43,6 +44,40 @@ export function getDefaultIntegrations(_options: Options): Integration[] {
];
}

function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions {
const defaultOptions: BrowserOptions = {
defaultIntegrations: getDefaultIntegrations(optionsArg),
release:
typeof __SENTRY_RELEASE__ === 'string' // This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value
? __SENTRY_RELEASE__
: WINDOW.SENTRY_RELEASE && WINDOW.SENTRY_RELEASE.id // This supports the variable that sentry-webpack-plugin injects
? WINDOW.SENTRY_RELEASE.id
: undefined,
autoSessionTracking: true,
sendClientReports: true,
};

return { ...defaultOptions, ...optionsArg };
}

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;
}

/**
* A magic string that build tooling can leverage in order to inject a release value into the SDK.
*/
Expand Down Expand Up @@ -94,26 +129,17 @@ declare const __SENTRY_RELEASE__: string | undefined;
*
* @see {@link BrowserOptions} for documentation on configuration options.
*/
export function init(options: BrowserOptions = {}): void {
if (options.defaultIntegrations === undefined) {
options.defaultIntegrations = getDefaultIntegrations(options);
}
if (options.release === undefined) {
// This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value
if (typeof __SENTRY_RELEASE__ === 'string') {
options.release = __SENTRY_RELEASE__;
}
export function init(browserOptions: BrowserOptions = {}): void {
const options = applyDefaultOptions(browserOptions);

// This supports the variable that sentry-webpack-plugin injects
if (WINDOW.SENTRY_RELEASE && WINDOW.SENTRY_RELEASE.id) {
options.release = WINDOW.SENTRY_RELEASE.id;
}
}
if (options.autoSessionTracking === undefined) {
options.autoSessionTracking = true;
}
if (options.sendClientReports === undefined) {
options.sendClientReports = true;
if (shouldShowBrowserExtensionError()) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.error(
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
);
});
return;
}

if (DEBUG_BUILD) {
Expand Down
58 changes: 58 additions & 0 deletions packages/browser/test/unit/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Client, Integration } from '@sentry/types';
import { resolvedSyncPromise } from '@sentry/utils';

import type { BrowserOptions } from '../../src';
import { WINDOW } from '../../src';
import { init } from '../../src/sdk';

const PUBLIC_DSN = 'https://username@domain/123';
Expand Down Expand Up @@ -127,4 +128,61 @@ describe('init', () => {
expect(newIntegration.setupOnce as jest.Mock).toHaveBeenCalledTimes(1);
expect(DEFAULT_INTEGRATIONS[1].setupOnce as jest.Mock).toHaveBeenCalledTimes(0);
});

describe('initialization error in browser extension', () => {
const DEFAULT_INTEGRATIONS: Integration[] = [
new MockIntegration('MockIntegration 0.1'),
new MockIntegration('MockIntegration 0.2'),
];

const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, defaultIntegrations: DEFAULT_INTEGRATIONS });

afterEach(() => {
Object.defineProperty(WINDOW, 'chrome', { value: undefined, writable: true });
Object.defineProperty(WINDOW, 'browser', { value: undefined, writable: true });
});

it('should log a browser extension error if executed inside a Chrome extension', () => {
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});

Object.defineProperty(WINDOW, 'chrome', {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add an afterEach block that resets this back to undefined, to ensure we don't leak this into other tests!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this block to reset it and I also added the test for the regular browser environment at last to make sure this fails directly if browser or chrome is still part of the globals.

value: { runtime: { id: 'mock-extension-id' } },
writable: true,
});

init(options);

expect(consoleErrorSpy).toBeCalledTimes(1);
expect(consoleErrorSpy).toHaveBeenCalledWith(
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
);

consoleErrorSpy.mockRestore();
});

it('should log 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 });

init(options);

expect(consoleErrorSpy).toBeCalledTimes(1);
expect(consoleErrorSpy).toHaveBeenCalledWith(
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
);

consoleErrorSpy.mockRestore();
});

it('should not log a browser extension error if executed inside regular browser environment', () => {
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});

init(options);

expect(consoleErrorSpy).toBeCalledTimes(0);

consoleErrorSpy.mockRestore();
});
});
});
Loading