Skip to content

Commit 554668c

Browse files
authored
fix(node): Only read JavaScript and TypeScript source files in ContextLines (#25535)
`ContextLines` read context lines from any `filename` in a parsed stack trace. An app that copies external input into `error.stack` before `captureException` let that input name any file on disk, such as `.env`, and the SDK sent its lines to Sentry. The integration now reads only files with a JS or TS source extension, plus `.vue`, `.svelte`, `.astro` and `.mdx`. Query strings and hashes are stripped first so Vite style `index.js?v=abc` frames still pass. The Deno SDK gets the same check. An extension allowlist was chosen over `require.cache` or a project root check: the module cache does not cover ESM, and root checks break monorepos and symlinked installs.
1 parent d6c8d82 commit 554668c

3 files changed

Lines changed: 61 additions & 10 deletions

File tree

‎packages/deno/src/integrations/contextlines.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import type { Event, IntegrationFn, StackFrame } from '@sentry/core';
2-
import { addContextToFrame, defineIntegration, LRUMap } from '@sentry/core';
2+
import { addContextToFrame, defineIntegration, LRUMap, stripUrlQueryAndFragment } from '@sentry/core';
33

44
const INTEGRATION_NAME = 'ContextLines' as const;
5+
// Frame filenames come from the parsed `error.stack` string, which application
6+
// code can overwrite with external input, so only read source files.
7+
const SOURCE_FILE_EXTENSION = /\.(?:[cm]?[jt]sx?|vue|svelte|astro|mdx)$/i;
58
const FILE_CONTENT_CACHE = new LRUMap<string, string | null>(100);
69
const DEFAULT_LINES_OF_CONTEXT = 5;
710

@@ -91,7 +94,12 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
9194
async function addSourceContextToFrames(frames: StackFrame[], contextLines: number): Promise<void> {
9295
for (const frame of frames) {
9396
// Only add context if we have a filename and it hasn't already been added
94-
if (frame.filename && frame.in_app && frame.context_line === undefined) {
97+
if (
98+
frame.filename &&
99+
frame.in_app &&
100+
frame.context_line === undefined &&
101+
SOURCE_FILE_EXTENSION.test(stripUrlQueryAndFragment(frame.filename))
102+
) {
95103
const permission = await Deno.permissions.query({
96104
name: 'read',
97105
path: frame.filename,

‎packages/node/src/integrations/contextlines.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { createReadStream } from 'node:fs';
22
import { createInterface } from 'node:readline';
33
import type { Event, IntegrationFn, StackFrame } from '@sentry/core';
4-
import { debug, defineIntegration, LRUMap, snipLine } from '@sentry/core';
4+
import { debug, defineIntegration, LRUMap, snipLine, stripUrlQueryAndFragment } from '@sentry/core';
55
import { DEBUG_BUILD } from '../debug-build';
66

7+
const SOURCE_FILE_EXTENSION = /\.(?:[cm]?[jt]sx?|vue|svelte|astro|mdx)$/;
78
const LRU_FILE_CONTENTS_CACHE = new LRUMap<string, Record<number, string>>(10);
89
const LRU_FILE_CONTENTS_FS_READ_FAILED = new LRUMap<string, 1>(20);
910
const DEFAULT_LINES_OF_CONTEXT = 5;
@@ -31,6 +32,7 @@ interface ContextLinesOptions {
3132
*/
3233
export function resetFileContentCache(): void {
3334
LRU_FILE_CONTENTS_CACHE.clear();
35+
LRU_FILE_CONTENTS_FS_READ_FAILED.clear();
3436
}
3537

3638
/**
@@ -49,19 +51,22 @@ function emplace<T extends LRUMap<K, V>, K extends string, V>(map: T, key: K, co
4951

5052
/**
5153
* Determines if context lines should be skipped for a file.
52-
* - .min.(mjs|cjs|js) files are and not useful since they dont point to the original source
5354
* - node: prefixed modules are part of the runtime and cannot be resolved to a file
5455
* - data: skip json, wasm and inline js https://nodejs.org/api/esm.html#data-imports
56+
* - .min.(mjs|cjs|js) files are and not useful since they dont point to the original source
57+
* - anything that is not a JavaScript or TypeScript source file. Frame filenames come
58+
* from the parsed `error.stack` string, which application code can overwrite with
59+
* external input, so we must not read arbitrary files off the filesystem.
5560
*/
5661
function shouldSkipContextLinesForFile(path: string): boolean {
57-
// Test the most common prefix and extension first. These are the ones we
58-
// are most likely to see in user applications and are the ones we can break out of first.
5962
if (path.startsWith('node:')) return true;
60-
if (path.endsWith('.min.js')) return true;
61-
if (path.endsWith('.min.cjs')) return true;
62-
if (path.endsWith('.min.mjs')) return true;
6363
if (path.startsWith('data:')) return true;
64-
return false;
64+
65+
const pathname = stripUrlQueryAndFragment(path).toLowerCase();
66+
if (pathname.endsWith('.min.js')) return true;
67+
if (pathname.endsWith('.min.cjs')) return true;
68+
if (pathname.endsWith('.min.mjs')) return true;
69+
return !SOURCE_FILE_EXTENSION.test(pathname);
6570
}
6671

6772
/**

‎packages/node/test/integrations/contextlines.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,44 @@ describe('ContextLines', () => {
6969
});
7070
});
7171

72+
describe('file filtering', () => {
73+
test.each([
74+
'/var/task/.env',
75+
'/etc/passwd',
76+
'/home/user/.ssh/id_rsa',
77+
'file:///var/task/config.json',
78+
'file:///var/task/index.js.bak',
79+
'file:///var/task/index.min.js?v=abc',
80+
])('does not read non-source file %s', async filename => {
81+
expect.assertions(1);
82+
const frames: StackFrame[] = [{ colno: 1, filename, lineno: 1, function: 'fxn1' }];
83+
84+
const readStreamSpy = vi.spyOn(fs, 'createReadStream');
85+
await addContext(frames);
86+
expect(readStreamSpy).not.toHaveBeenCalled();
87+
});
88+
89+
test.each([
90+
'file:///var/task/index.js',
91+
'file:///var/task/index.mjs',
92+
'file:///var/task/index.cjs',
93+
'file:///var/task/index.ts',
94+
'file:///var/task/index.tsx',
95+
'file:///var/task/Index.JS',
96+
'file:///var/task/index.js?v=abc123',
97+
'file:///var/task/index.js#hash',
98+
'file:///var/task/Component.vue',
99+
'file:///var/task/page.mdx',
100+
])('reads source file %s', async filename => {
101+
expect.assertions(1);
102+
const frames: StackFrame[] = [{ colno: 1, filename, lineno: 1, function: 'fxn1' }];
103+
104+
const readStreamSpy = vi.spyOn(fs, 'createReadStream');
105+
await addContext(frames);
106+
expect(readStreamSpy).toHaveBeenCalledTimes(1);
107+
});
108+
});
109+
72110
describe('lru file cache', () => {
73111
test('parseStack when file does not exist', async () => {
74112
expect.assertions(4);

0 commit comments

Comments
 (0)