Skip to content

Commit ba25101

Browse files
antonisclaude
andcommitted
fix(core): Harden metro dev helpers
- Restrict source-context middleware reads to files under the project root. - Escape release-constants values when injected into the generated bundle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ab73a74 commit ba25101

3 files changed

Lines changed: 117 additions & 49 deletions

File tree

packages/core/src/js/tools/metroMiddleware.ts

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { InputConfigT, Middleware } from 'metro-config';
44

55
import { addContextToFrame, debug } from '@sentry/core';
66
import { readFile } from 'fs';
7+
import * as path from 'path';
78
import { promisify } from 'util';
89

910
import { SENTRY_CONTEXT_REQUEST_PATH, SENTRY_OPEN_URL_REQUEST_PATH } from '../metro/constants';
@@ -15,42 +16,44 @@ const readFileAsync = promisify(readFile);
1516
/**
1617
* Accepts Sentry formatted stack frames and
1718
* adds source context to the in app frames.
19+
*
20+
* Filenames are resolved relative to `projectRoot` and must remain within it.
1821
*/
19-
export const stackFramesContextMiddleware: Middleware = async (
20-
request: IncomingMessage,
21-
response: ServerResponse,
22-
_next: () => void,
23-
): Promise<void> => {
24-
debug.log('[@sentry/react-native/metro] Received request for stack frames context.');
25-
request.setEncoding('utf8');
26-
const rawBody = await getRawBody(request);
27-
28-
let body: {
29-
stack?: Partial<StackFrame>[];
30-
} = {};
31-
try {
32-
body = JSON.parse(rawBody);
33-
} catch (e) {
34-
debug.log('[@sentry/react-native/metro] Could not parse request body.', e);
35-
badRequest(response, 'Invalid request body. Expected a JSON object.');
36-
return;
37-
}
22+
export const createStackFramesContextMiddleware = (projectRoot: string): Middleware => {
23+
const normalizedRoot = path.resolve(projectRoot);
3824

39-
const stack = body.stack;
40-
if (!Array.isArray(stack)) {
41-
debug.log('[@sentry/react-native/metro] Invalid stack frames.', stack);
42-
badRequest(response, 'Invalid stack frames. Expected an array.');
43-
return;
44-
}
25+
return async (request: IncomingMessage, response: ServerResponse, _next: () => void): Promise<void> => {
26+
debug.log('[@sentry/react-native/metro] Received request for stack frames context.');
27+
request.setEncoding('utf8');
28+
const rawBody = await getRawBody(request);
4529

46-
const stackWithSourceContext = await Promise.all(stack.map(addSourceContext));
47-
response.setHeader('Content-Type', 'application/json');
48-
response.statusCode = 200;
49-
response.end(JSON.stringify({ stack: stackWithSourceContext }));
50-
debug.log('[@sentry/react-native/metro] Sent stack frames context.');
30+
let body: {
31+
stack?: Partial<StackFrame>[];
32+
} = {};
33+
try {
34+
body = JSON.parse(rawBody);
35+
} catch (e) {
36+
debug.log('[@sentry/react-native/metro] Could not parse request body.', e);
37+
badRequest(response, 'Invalid request body. Expected a JSON object.');
38+
return;
39+
}
40+
41+
const stack = body.stack;
42+
if (!Array.isArray(stack)) {
43+
debug.log('[@sentry/react-native/metro] Invalid stack frames.', stack);
44+
badRequest(response, 'Invalid stack frames. Expected an array.');
45+
return;
46+
}
47+
48+
const stackWithSourceContext = await Promise.all(stack.map(frame => addSourceContext(frame, normalizedRoot)));
49+
response.setHeader('Content-Type', 'application/json');
50+
response.statusCode = 200;
51+
response.end(JSON.stringify({ stack: stackWithSourceContext }));
52+
debug.log('[@sentry/react-native/metro] Sent stack frames context.');
53+
};
5154
};
5255

53-
async function addSourceContext(frame: StackFrame): Promise<StackFrame> {
56+
async function addSourceContext(frame: StackFrame, projectRoot: string): Promise<StackFrame> {
5457
if (!frame.in_app) {
5558
return frame;
5659
}
@@ -61,7 +64,14 @@ async function addSourceContext(frame: StackFrame): Promise<StackFrame> {
6164
return frame;
6265
}
6366

64-
const source = await readFileAsync(frame.filename, { encoding: 'utf8' });
67+
const resolvedPath = path.resolve(projectRoot, frame.filename);
68+
const relative = path.relative(projectRoot, resolvedPath);
69+
if (relative === '' || relative.startsWith('..') || path.isAbsolute(relative)) {
70+
debug.warn('[@sentry/react-native/metro] Skipping frame whose filename is outside the project root.');
71+
return frame;
72+
}
73+
74+
const source = await readFileAsync(resolvedPath, { encoding: 'utf8' });
6575
const lines = source.split('\n');
6676
addContextToFrame(lines, frame);
6777
} catch (error) {
@@ -78,7 +88,12 @@ function badRequest(response: ServerResponse, message: string): void {
7888
/**
7989
* Creates a middleware that adds source context to the Sentry formatted stack frames.
8090
*/
81-
export const createSentryMetroMiddleware = (middleware: Middleware): Middleware => {
91+
export const createSentryMetroMiddleware = (middleware: Middleware, projectRoot: string): Middleware => {
92+
const stackFramesContextMiddleware = createStackFramesContextMiddleware(projectRoot) as (
93+
req: IncomingMessage,
94+
res: ServerResponse,
95+
next: () => void,
96+
) => void;
8297
return (request: IncomingMessage, response: ServerResponse, next: () => void) => {
8398
if (request.url?.startsWith(`/${SENTRY_CONTEXT_REQUEST_PATH}`)) {
8499
return stackFramesContextMiddleware(request, response, next);
@@ -102,9 +117,10 @@ export const withSentryMiddleware = (config: InputConfigT): InputConfigT => {
102117
config.server = {};
103118
}
104119

120+
const projectRoot = (config as { projectRoot?: string }).projectRoot || process.cwd();
105121
const originalEnhanceMiddleware = config.server.enhanceMiddleware;
106122
config.server.enhanceMiddleware = (middleware, server) => {
107-
const sentryMiddleware = createSentryMetroMiddleware(middleware);
123+
const sentryMiddleware = createSentryMetroMiddleware(middleware, projectRoot);
108124
return originalEnhanceMiddleware ? originalEnhanceMiddleware(sentryMiddleware, server) : sentryMiddleware;
109125
};
110126
return config;

packages/core/src/js/tools/sentryReleaseInjector.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,5 @@ function createSentryReleaseModule({
4343
}
4444

4545
function createReleaseConstantsSnippet({ name, version }: { name: string; version: string }): string {
46-
return `var SENTRY_RELEASE;SENTRY_RELEASE={name: "${name}", version: "${version}"};`;
46+
return `var SENTRY_RELEASE;SENTRY_RELEASE={name: ${JSON.stringify(name)}, version: ${JSON.stringify(version)}};`;
4747
}

packages/core/test/tools/metroMiddleware.test.ts

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ import type { StackFrame } from '@sentry/core';
22

33
import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals';
44
import * as fs from 'fs';
5+
import * as path from 'path';
56

67
import * as openUrlMiddlewareModule from '../../src/js/metro/openUrlMiddleware';
78
import * as metroMiddleware from '../../src/js/tools/metroMiddleware';
89

9-
const { withSentryMiddleware, createSentryMetroMiddleware, stackFramesContextMiddleware } = metroMiddleware;
10+
const { withSentryMiddleware, createSentryMetroMiddleware, createStackFramesContextMiddleware } = metroMiddleware;
11+
12+
const TEST_PROJECT_ROOT = path.resolve('/tmp/sentry-rn-test-project');
1013

1114
jest.mock('../../src/js/tools/metroMiddleware', () => jest.requireActual('../../src/js/tools/metroMiddleware'));
1215
jest.mock('fs', () => {
@@ -60,65 +63,69 @@ describe('metroMiddleware', () => {
6063
const next = jest.fn();
6164
const response = {} as any;
6265

63-
let spiedStackFramesContextMiddleware: jest.Spied<typeof stackFramesContextMiddleware>;
66+
let spiedCreateStackFramesContextMiddleware: jest.Spied<typeof createStackFramesContextMiddleware>;
67+
let mockedStackFramesContextMiddleware: jest.Mock;
6468

6569
beforeEach(() => {
6670
jest.clearAllMocks();
67-
spiedStackFramesContextMiddleware = jest
68-
.spyOn(metroMiddleware, 'stackFramesContextMiddleware')
69-
.mockReturnValue(undefined);
71+
mockedStackFramesContextMiddleware = jest.fn();
72+
spiedCreateStackFramesContextMiddleware = jest
73+
.spyOn(metroMiddleware, 'createStackFramesContextMiddleware')
74+
.mockReturnValue(mockedStackFramesContextMiddleware);
7075
});
7176

7277
afterEach(() => {
7378
jest.resetAllMocks();
7479
});
7580

7681
it('should call stackFramesContextMiddleware for sentry context requests', () => {
77-
const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware);
82+
const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware, TEST_PROJECT_ROOT);
7883

7984
const sentryRequest = {
8085
url: '/__sentry/context',
8186
} as any;
8287
testedMiddleware(sentryRequest, response, next);
8388
expect(defaultMiddleware).not.toHaveBeenCalled();
84-
expect(spiedStackFramesContextMiddleware).toHaveBeenCalledWith(sentryRequest, response, next);
89+
expect(spiedCreateStackFramesContextMiddleware).toHaveBeenCalledWith(TEST_PROJECT_ROOT);
90+
expect(mockedStackFramesContextMiddleware).toHaveBeenCalledWith(sentryRequest, response, next);
8591
});
8692

8793
it('should call openURLMiddleware for sentry open-url requests', () => {
8894
const spiedOpenURLMiddleware = jest
8995
.spyOn(openUrlMiddlewareModule, 'openURLMiddleware')
9096
.mockReturnValue(undefined as any);
9197

92-
const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware);
98+
const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware, TEST_PROJECT_ROOT);
9399

94100
const openUrlRequest = {
95101
url: '/__sentry/open-url',
96102
} as any;
97103
testedMiddleware(openUrlRequest, response, next);
98104
expect(defaultMiddleware).not.toHaveBeenCalled();
99-
expect(spiedStackFramesContextMiddleware).not.toHaveBeenCalled();
105+
expect(mockedStackFramesContextMiddleware).not.toHaveBeenCalled();
100106
expect(spiedOpenURLMiddleware).toHaveBeenCalledWith(openUrlRequest, response);
101107

102108
spiedOpenURLMiddleware.mockRestore();
103109
});
104110

105111
it('should call default middleware for non-sentry requests', () => {
106-
const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware);
112+
const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware, TEST_PROJECT_ROOT);
107113

108114
const regularRequest = {
109115
url: '/regular/path',
110116
} as any;
111117
testedMiddleware(regularRequest, response, next);
112118
expect(defaultMiddleware).toHaveBeenCalledWith(regularRequest, response, next);
113119
expect(defaultMiddleware).toHaveBeenCalledTimes(1);
114-
expect(spiedStackFramesContextMiddleware).not.toHaveBeenCalled();
120+
expect(mockedStackFramesContextMiddleware).not.toHaveBeenCalled();
115121
});
116122
});
117123

118124
describe('stackFramesContextMiddleware', () => {
119125
let request: any;
120126
let response: any;
121127
const next = jest.fn();
128+
const stackFramesContextMiddleware = createStackFramesContextMiddleware(TEST_PROJECT_ROOT);
122129

123130
let testData: string = '';
124131

@@ -201,7 +208,7 @@ describe('metroMiddleware', () => {
201208
],
202209
} satisfies { stack: StackFrame[] });
203210

204-
mockReadFileOnce(readFileSpy, 'test.js', 'line1\nline2\nline3\nline4\nline5');
211+
mockReadFileOnce(readFileSpy, path.join(TEST_PROJECT_ROOT, 'test.js'), 'line1\nline2\nline3\nline4\nline5');
205212

206213
await stackFramesContextMiddleware(request, response, next);
207214

@@ -282,10 +289,55 @@ describe('metroMiddleware', () => {
282289
});
283290
});
284291

292+
it('should skip frames whose filename escapes the project root', async () => {
293+
const readFileSpy = jest.spyOn(fs, 'readFile');
294+
testData = JSON.stringify({
295+
stack: [
296+
{
297+
in_app: true,
298+
filename: '/etc/passwd',
299+
function: 'testFunction',
300+
lineno: 1,
301+
colno: 1,
302+
},
303+
{
304+
in_app: true,
305+
filename: '../outside.js',
306+
function: 'testFunction',
307+
lineno: 1,
308+
colno: 1,
309+
},
310+
],
311+
} satisfies { stack: StackFrame[] });
312+
313+
await stackFramesContextMiddleware(request, response, next);
314+
315+
expect(readFileSpy).not.toHaveBeenCalled();
316+
expect(response.statusCode).toBe(200);
317+
expect(JSON.parse(response.end.mock.calls[0][0])).toEqual({
318+
stack: [
319+
{
320+
in_app: true,
321+
filename: '/etc/passwd',
322+
function: 'testFunction',
323+
lineno: 1,
324+
colno: 1,
325+
},
326+
{
327+
in_app: true,
328+
filename: '../outside.js',
329+
function: 'testFunction',
330+
lineno: 1,
331+
colno: 1,
332+
},
333+
],
334+
});
335+
});
336+
285337
it('should handle mixed frame types correctly', async () => {
286338
const readFileSpy = jest.spyOn(fs, 'readFile');
287-
mockReadFileOnce(readFileSpy, 'app1.js', 'line1\nline2\nline3\nline4\nline5');
288-
mockReadFileOnce(readFileSpy, 'app2.js', 'code1\ncode2\ncode3\ncode4\ncode5');
339+
mockReadFileOnce(readFileSpy, path.join(TEST_PROJECT_ROOT, 'app1.js'), 'line1\nline2\nline3\nline4\nline5');
340+
mockReadFileOnce(readFileSpy, path.join(TEST_PROJECT_ROOT, 'app2.js'), 'code1\ncode2\ncode3\ncode4\ncode5');
289341

290342
testData = JSON.stringify({
291343
stack: [

0 commit comments

Comments
 (0)