diff --git a/CHANGELOG.md b/CHANGELOG.md index a78b933a47..791600a4fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ ### Fixes - Stop the Hermes sampling profiler on React instance teardown to prevent `pthread_kill` SIGABRT when the JS thread is torn down with profiling active ([#6035](https://github.com/getsentry/sentry-react-native/pull/6035)) +- Restrict the Metro source-context middleware to files within the project root ([#6044](https://github.com/getsentry/sentry-react-native/pull/6044)) +- Escape `name` and `version` values when injecting release constants into the web bundle ([#6044](https://github.com/getsentry/sentry-react-native/pull/6044)) ### Dependencies diff --git a/packages/core/src/js/tools/metroMiddleware.ts b/packages/core/src/js/tools/metroMiddleware.ts index 8aca80d3c7..650a360073 100644 --- a/packages/core/src/js/tools/metroMiddleware.ts +++ b/packages/core/src/js/tools/metroMiddleware.ts @@ -3,7 +3,8 @@ import type { IncomingMessage, ServerResponse } from 'http'; import type { InputConfigT, Middleware } from 'metro-config'; import { addContextToFrame, debug } from '@sentry/core'; -import { readFile } from 'fs'; +import { readFile, realpath, realpathSync } from 'fs'; +import * as path from 'path'; import { promisify } from 'util'; import { SENTRY_CONTEXT_REQUEST_PATH, SENTRY_OPEN_URL_REQUEST_PATH } from '../metro/constants'; @@ -11,46 +12,59 @@ import { getRawBody } from '../metro/getRawBody'; import { openURLMiddleware } from '../metro/openUrlMiddleware'; const readFileAsync = promisify(readFile); +const realpathAsync = promisify(realpath); /** * Accepts Sentry formatted stack frames and * adds source context to the in app frames. + * + * Relative filenames are resolved against the first entry in `allowedRoots`. + * Both the resolved filename and the allowed roots are canonicalized via + * `fs.realpath`, so a symlink inside an allowed root pointing outside of it + * cannot escape the containment check. */ -export const stackFramesContextMiddleware: Middleware = async ( - request: IncomingMessage, - response: ServerResponse, - _next: () => void, -): Promise => { - debug.log('[@sentry/react-native/metro] Received request for stack frames context.'); - request.setEncoding('utf8'); - const rawBody = await getRawBody(request); - - let body: { - stack?: Partial[]; - } = {}; - try { - body = JSON.parse(rawBody); - } catch (e) { - debug.log('[@sentry/react-native/metro] Could not parse request body.', e); - badRequest(response, 'Invalid request body. Expected a JSON object.'); - return; - } +export const createStackFramesContextMiddleware = (allowedRoots: string[]): Middleware => { + const canonicalRoots = allowedRoots.map(root => { + const resolved = path.resolve(root); + try { + return realpathSync(resolved); + } catch { + return resolved; + } + }); - const stack = body.stack; - if (!Array.isArray(stack)) { - debug.log('[@sentry/react-native/metro] Invalid stack frames.', stack); - badRequest(response, 'Invalid stack frames. Expected an array.'); - return; - } + return async (request: IncomingMessage, response: ServerResponse, _next: () => void): Promise => { + debug.log('[@sentry/react-native/metro] Received request for stack frames context.'); + request.setEncoding('utf8'); + const rawBody = await getRawBody(request); + + let body: { + stack?: Partial[]; + } = {}; + try { + body = JSON.parse(rawBody); + } catch (e) { + debug.log('[@sentry/react-native/metro] Could not parse request body.', e); + badRequest(response, 'Invalid request body. Expected a JSON object.'); + return; + } + + const stack = body.stack; + if (!Array.isArray(stack)) { + debug.log('[@sentry/react-native/metro] Invalid stack frames.', stack); + badRequest(response, 'Invalid stack frames. Expected an array.'); + return; + } - const stackWithSourceContext = await Promise.all(stack.map(addSourceContext)); - response.setHeader('Content-Type', 'application/json'); - response.statusCode = 200; - response.end(JSON.stringify({ stack: stackWithSourceContext })); - debug.log('[@sentry/react-native/metro] Sent stack frames context.'); + const stackWithSourceContext = await Promise.all(stack.map(frame => addSourceContext(frame, canonicalRoots))); + response.setHeader('Content-Type', 'application/json'); + response.statusCode = 200; + response.end(JSON.stringify({ stack: stackWithSourceContext })); + debug.log('[@sentry/react-native/metro] Sent stack frames context.'); + }; }; -async function addSourceContext(frame: StackFrame): Promise { +async function addSourceContext(frame: StackFrame, canonicalRoots: string[]): Promise { if (!frame.in_app) { return frame; } @@ -61,7 +75,30 @@ async function addSourceContext(frame: StackFrame): Promise { return frame; } - const source = await readFileAsync(frame.filename, { encoding: 'utf8' }); + if (canonicalRoots.length === 0) { + debug.warn('[@sentry/react-native/metro] Skipping frame: no allowed roots configured.'); + return frame; + } + + const resolvedPath = path.resolve(canonicalRoots[0]!, frame.filename); + let canonicalPath: string; + try { + canonicalPath = await realpathAsync(resolvedPath); + } catch { + debug.warn('[@sentry/react-native/metro] Skipping frame: could not canonicalize filename.'); + return frame; + } + + const isInside = canonicalRoots.some(root => { + const relative = path.relative(root, canonicalPath); + return relative !== '' && !relative.startsWith('..') && !path.isAbsolute(relative); + }); + if (!isInside) { + debug.warn('[@sentry/react-native/metro] Skipping frame whose filename is outside the allowed roots.'); + return frame; + } + + const source = await readFileAsync(canonicalPath, { encoding: 'utf8' }); const lines = source.split('\n'); addContextToFrame(lines, frame); } catch (error) { @@ -78,7 +115,12 @@ function badRequest(response: ServerResponse, message: string): void { /** * Creates a middleware that adds source context to the Sentry formatted stack frames. */ -export const createSentryMetroMiddleware = (middleware: Middleware): Middleware => { +export const createSentryMetroMiddleware = (middleware: Middleware, allowedRoots: string[]): Middleware => { + const stackFramesContextMiddleware = createStackFramesContextMiddleware(allowedRoots) as ( + req: IncomingMessage, + res: ServerResponse, + next: () => void, + ) => void; return (request: IncomingMessage, response: ServerResponse, next: () => void) => { if (request.url?.startsWith(`/${SENTRY_CONTEXT_REQUEST_PATH}`)) { return stackFramesContextMiddleware(request, response, next); @@ -102,9 +144,13 @@ export const withSentryMiddleware = (config: InputConfigT): InputConfigT => { config.server = {}; } + const projectRoot = config.projectRoot || process.cwd(); + const watchFolders = config.watchFolders || []; + const allowedRoots = [projectRoot, ...watchFolders]; + const originalEnhanceMiddleware = config.server.enhanceMiddleware; config.server.enhanceMiddleware = (middleware, server) => { - const sentryMiddleware = createSentryMetroMiddleware(middleware); + const sentryMiddleware = createSentryMetroMiddleware(middleware, allowedRoots); return originalEnhanceMiddleware ? originalEnhanceMiddleware(sentryMiddleware, server) : sentryMiddleware; }; return config; diff --git a/packages/core/src/js/tools/sentryReleaseInjector.ts b/packages/core/src/js/tools/sentryReleaseInjector.ts index dca2400ab9..85453632fe 100644 --- a/packages/core/src/js/tools/sentryReleaseInjector.ts +++ b/packages/core/src/js/tools/sentryReleaseInjector.ts @@ -43,5 +43,5 @@ function createSentryReleaseModule({ } function createReleaseConstantsSnippet({ name, version }: { name: string; version: string }): string { - return `var SENTRY_RELEASE;SENTRY_RELEASE={name: "${name}", version: "${version}"};`; + return `var SENTRY_RELEASE;SENTRY_RELEASE={name: ${JSON.stringify(name)}, version: ${JSON.stringify(version)}};`; } diff --git a/packages/core/test/tools/metroMiddleware.test.ts b/packages/core/test/tools/metroMiddleware.test.ts index 9422af7fa9..ab93ce8171 100644 --- a/packages/core/test/tools/metroMiddleware.test.ts +++ b/packages/core/test/tools/metroMiddleware.test.ts @@ -2,16 +2,21 @@ import type { StackFrame } from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'; import * as fs from 'fs'; +import * as path from 'path'; import * as openUrlMiddlewareModule from '../../src/js/metro/openUrlMiddleware'; import * as metroMiddleware from '../../src/js/tools/metroMiddleware'; -const { withSentryMiddleware, createSentryMetroMiddleware, stackFramesContextMiddleware } = metroMiddleware; +const { withSentryMiddleware, createSentryMetroMiddleware, createStackFramesContextMiddleware } = metroMiddleware; + +const TEST_PROJECT_ROOT = path.resolve('/tmp/sentry-rn-test-project'); jest.mock('../../src/js/tools/metroMiddleware', () => jest.requireActual('../../src/js/tools/metroMiddleware')); jest.mock('fs', () => { return { readFile: jest.fn(), + realpath: jest.fn((p: string, cb: (err: NodeJS.ErrnoException | null, resolved: string) => void) => cb(null, p)), + realpathSync: jest.fn((p: string) => p), }; }); @@ -60,13 +65,15 @@ describe('metroMiddleware', () => { const next = jest.fn(); const response = {} as any; - let spiedStackFramesContextMiddleware: jest.Spied; + let spiedCreateStackFramesContextMiddleware: jest.Spied; + let mockedStackFramesContextMiddleware: jest.Mock; beforeEach(() => { jest.clearAllMocks(); - spiedStackFramesContextMiddleware = jest - .spyOn(metroMiddleware, 'stackFramesContextMiddleware') - .mockReturnValue(undefined); + mockedStackFramesContextMiddleware = jest.fn(); + spiedCreateStackFramesContextMiddleware = jest + .spyOn(metroMiddleware, 'createStackFramesContextMiddleware') + .mockReturnValue(mockedStackFramesContextMiddleware); }); afterEach(() => { @@ -74,14 +81,15 @@ describe('metroMiddleware', () => { }); it('should call stackFramesContextMiddleware for sentry context requests', () => { - const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware); + const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware, [TEST_PROJECT_ROOT]); const sentryRequest = { url: '/__sentry/context', } as any; testedMiddleware(sentryRequest, response, next); expect(defaultMiddleware).not.toHaveBeenCalled(); - expect(spiedStackFramesContextMiddleware).toHaveBeenCalledWith(sentryRequest, response, next); + expect(spiedCreateStackFramesContextMiddleware).toHaveBeenCalledWith([TEST_PROJECT_ROOT]); + expect(mockedStackFramesContextMiddleware).toHaveBeenCalledWith(sentryRequest, response, next); }); it('should call openURLMiddleware for sentry open-url requests', () => { @@ -89,21 +97,21 @@ describe('metroMiddleware', () => { .spyOn(openUrlMiddlewareModule, 'openURLMiddleware') .mockReturnValue(undefined as any); - const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware); + const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware, [TEST_PROJECT_ROOT]); const openUrlRequest = { url: '/__sentry/open-url', } as any; testedMiddleware(openUrlRequest, response, next); expect(defaultMiddleware).not.toHaveBeenCalled(); - expect(spiedStackFramesContextMiddleware).not.toHaveBeenCalled(); + expect(mockedStackFramesContextMiddleware).not.toHaveBeenCalled(); expect(spiedOpenURLMiddleware).toHaveBeenCalledWith(openUrlRequest, response); spiedOpenURLMiddleware.mockRestore(); }); it('should call default middleware for non-sentry requests', () => { - const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware); + const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware, [TEST_PROJECT_ROOT]); const regularRequest = { url: '/regular/path', @@ -111,7 +119,7 @@ describe('metroMiddleware', () => { testedMiddleware(regularRequest, response, next); expect(defaultMiddleware).toHaveBeenCalledWith(regularRequest, response, next); expect(defaultMiddleware).toHaveBeenCalledTimes(1); - expect(spiedStackFramesContextMiddleware).not.toHaveBeenCalled(); + expect(mockedStackFramesContextMiddleware).not.toHaveBeenCalled(); }); }); @@ -119,10 +127,17 @@ describe('metroMiddleware', () => { let request: any; let response: any; const next = jest.fn(); + const stackFramesContextMiddleware = createStackFramesContextMiddleware([TEST_PROJECT_ROOT]); let testData: string = ''; beforeEach(() => { + // afterEach resetAllMocks wipes the default fs impls installed via jest.mock, so reinstate. + (fs.realpath as unknown as jest.Mock).mockImplementation( + (p: string, cb: (err: NodeJS.ErrnoException | null, resolved: string) => void) => cb(null, p), + ); + (fs.realpathSync as unknown as jest.Mock).mockImplementation((p: string) => p); + request = { setEncoding: jest.fn(), on: jest.fn((event: string, cb: (data?: string) => void) => { @@ -201,7 +216,7 @@ describe('metroMiddleware', () => { ], } satisfies { stack: StackFrame[] }); - mockReadFileOnce(readFileSpy, 'test.js', 'line1\nline2\nline3\nline4\nline5'); + mockReadFileOnce(readFileSpy, path.join(TEST_PROJECT_ROOT, 'test.js'), 'line1\nline2\nline3\nline4\nline5'); await stackFramesContextMiddleware(request, response, next); @@ -282,10 +297,147 @@ describe('metroMiddleware', () => { }); }); + it('should add source context for frames under additional allowed roots (watchFolders)', async () => { + const watchFolder = path.resolve('/tmp/sentry-rn-test-workspace-pkg'); + const scopedMiddleware = createStackFramesContextMiddleware([TEST_PROJECT_ROOT, watchFolder]); + const readFileSpy = jest.spyOn(fs, 'readFile'); + mockReadFileOnce(readFileSpy, path.join(watchFolder, 'shared.js'), 'one\ntwo\nthree\nfour\nfive'); + + testData = JSON.stringify({ + stack: [ + { + in_app: true, + filename: path.join(watchFolder, 'shared.js'), + function: 'sharedFn', + lineno: 3, + colno: 1, + }, + ], + } satisfies { stack: StackFrame[] }); + + await scopedMiddleware(request, response, next); + + expect(response.statusCode).toBe(200); + expect(JSON.parse(response.end.mock.calls[0][0])).toEqual({ + stack: [ + { + in_app: true, + filename: path.join(watchFolder, 'shared.js'), + function: 'sharedFn', + lineno: 3, + colno: 1, + pre_context: ['one', 'two'], + context_line: 'three', + post_context: ['four', 'five'], + }, + ], + }); + }); + + it('should skip frames whose filename escapes the project root', async () => { + const readFileSpy = jest.spyOn(fs, 'readFile'); + testData = JSON.stringify({ + stack: [ + { + in_app: true, + filename: '/etc/passwd', + function: 'testFunction', + lineno: 1, + colno: 1, + }, + { + in_app: true, + filename: '../outside.js', + function: 'testFunction', + lineno: 1, + colno: 1, + }, + ], + } satisfies { stack: StackFrame[] }); + + await stackFramesContextMiddleware(request, response, next); + + expect(readFileSpy).not.toHaveBeenCalled(); + expect(response.statusCode).toBe(200); + expect(JSON.parse(response.end.mock.calls[0][0])).toEqual({ + stack: [ + { + in_app: true, + filename: '/etc/passwd', + function: 'testFunction', + lineno: 1, + colno: 1, + }, + { + in_app: true, + filename: '../outside.js', + function: 'testFunction', + lineno: 1, + colno: 1, + }, + ], + }); + }); + + it('should skip frames whose realpath resolves outside the allowed roots', async () => { + const realpathSpy = jest.spyOn(fs, 'realpath') as unknown as jest.Mock; + const readFileSpy = jest.spyOn(fs, 'readFile'); + + // Simulate a symlink: the file lives at /link/file.js inside the project, + // but its realpath is /etc/shadow — outside every allowed root. + realpathSpy.mockImplementationOnce( + (_p: string, cb: (err: NodeJS.ErrnoException | null, resolved: string) => void) => cb(null, '/etc/shadow'), + ); + + testData = JSON.stringify({ + stack: [ + { + in_app: true, + filename: 'link/file.js', + function: 'testFunction', + lineno: 1, + colno: 1, + }, + ], + } satisfies { stack: StackFrame[] }); + + await stackFramesContextMiddleware(request, response, next); + + expect(readFileSpy).not.toHaveBeenCalled(); + expect(response.statusCode).toBe(200); + }); + + it('should skip frames whose realpath cannot be resolved', async () => { + const realpathSpy = jest.spyOn(fs, 'realpath') as unknown as jest.Mock; + const readFileSpy = jest.spyOn(fs, 'readFile'); + + const enoent: NodeJS.ErrnoException = Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + realpathSpy.mockImplementationOnce( + (_p: string, cb: (err: NodeJS.ErrnoException | null, resolved: string) => void) => cb(enoent, ''), + ); + + testData = JSON.stringify({ + stack: [ + { + in_app: true, + filename: 'missing.js', + function: 'testFunction', + lineno: 1, + colno: 1, + }, + ], + } satisfies { stack: StackFrame[] }); + + await stackFramesContextMiddleware(request, response, next); + + expect(readFileSpy).not.toHaveBeenCalled(); + expect(response.statusCode).toBe(200); + }); + it('should handle mixed frame types correctly', async () => { const readFileSpy = jest.spyOn(fs, 'readFile'); - mockReadFileOnce(readFileSpy, 'app1.js', 'line1\nline2\nline3\nline4\nline5'); - mockReadFileOnce(readFileSpy, 'app2.js', 'code1\ncode2\ncode3\ncode4\ncode5'); + mockReadFileOnce(readFileSpy, path.join(TEST_PROJECT_ROOT, 'app1.js'), 'line1\nline2\nline3\nline4\nline5'); + mockReadFileOnce(readFileSpy, path.join(TEST_PROJECT_ROOT, 'app2.js'), 'code1\ncode2\ncode3\ncode4\ncode5'); testData = JSON.stringify({ stack: [ diff --git a/packages/core/test/tools/sentryReleaseInjector.test.ts b/packages/core/test/tools/sentryReleaseInjector.test.ts index fa120839c2..76e2e18069 100644 --- a/packages/core/test/tools/sentryReleaseInjector.test.ts +++ b/packages/core/test/tools/sentryReleaseInjector.test.ts @@ -47,4 +47,25 @@ describe('Sentry Release Injector', () => { 'var SENTRY_RELEASE;SENTRY_RELEASE={name: "TestApp", version: "1.0.0"};', ); }); + + test('unstableReleaseConstantsPlugin escapes values that would break out of the string literal', () => { + mockedExpoConfigRequire.mockReturnValue({ + exp: { + name: 'App", evil();//', + version: '1.0\\"', + }, + }); + const projectRoot = '/some/project/root'; + const graph = { + transformOptions: { platform: 'web' }, + } as unknown as ReadOnlyGraph; + const premodules = [{ path: 'someModule.js' }] as Module[]; + + const plugin = unstableReleaseConstantsPlugin(projectRoot); + const result = plugin({ graph, premodules }); + + expect(result[0]?.getSource().toString()).toEqual( + 'var SENTRY_RELEASE;SENTRY_RELEASE={name: "App\\", evil();//", version: "1.0\\\\\\""};', + ); + }); });