diff --git a/packages/deno/src/integrations/contextlines.ts b/packages/deno/src/integrations/contextlines.ts index fe0a3708850a..e367003d8437 100644 --- a/packages/deno/src/integrations/contextlines.ts +++ b/packages/deno/src/integrations/contextlines.ts @@ -1,7 +1,10 @@ import type { Event, IntegrationFn, StackFrame } from '@sentry/core'; -import { addContextToFrame, defineIntegration, LRUMap } from '@sentry/core'; +import { addContextToFrame, defineIntegration, LRUMap, stripUrlQueryAndFragment } from '@sentry/core'; const INTEGRATION_NAME = 'ContextLines' as const; +// Frame filenames come from the parsed `error.stack` string, which application +// code can overwrite with external input, so only read source files. +const SOURCE_FILE_EXTENSION = /\.(?:[cm]?[jt]sx?|vue|svelte|astro|mdx)$/i; const FILE_CONTENT_CACHE = new LRUMap(100); const DEFAULT_LINES_OF_CONTEXT = 5; @@ -91,7 +94,12 @@ async function addSourceContext(event: Event, contextLines: number): Promise { for (const frame of frames) { // Only add context if we have a filename and it hasn't already been added - if (frame.filename && frame.in_app && frame.context_line === undefined) { + if ( + frame.filename && + frame.in_app && + frame.context_line === undefined && + SOURCE_FILE_EXTENSION.test(stripUrlQueryAndFragment(frame.filename)) + ) { const permission = await Deno.permissions.query({ name: 'read', path: frame.filename, diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts index 12cc79c90366..9ff0d22cc48f 100644 --- a/packages/node/src/integrations/contextlines.ts +++ b/packages/node/src/integrations/contextlines.ts @@ -1,9 +1,10 @@ import { createReadStream } from 'node:fs'; import { createInterface } from 'node:readline'; import type { Event, IntegrationFn, StackFrame } from '@sentry/core'; -import { debug, defineIntegration, LRUMap, snipLine } from '@sentry/core'; +import { debug, defineIntegration, LRUMap, snipLine, stripUrlQueryAndFragment } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; +const SOURCE_FILE_EXTENSION = /\.(?:[cm]?[jt]sx?|vue|svelte|astro|mdx)$/; const LRU_FILE_CONTENTS_CACHE = new LRUMap>(10); const LRU_FILE_CONTENTS_FS_READ_FAILED = new LRUMap(20); const DEFAULT_LINES_OF_CONTEXT = 5; @@ -31,6 +32,7 @@ interface ContextLinesOptions { */ export function resetFileContentCache(): void { LRU_FILE_CONTENTS_CACHE.clear(); + LRU_FILE_CONTENTS_FS_READ_FAILED.clear(); } /** @@ -49,19 +51,22 @@ function emplace, K extends string, V>(map: T, key: K, co /** * Determines if context lines should be skipped for a file. - * - .min.(mjs|cjs|js) files are and not useful since they dont point to the original source * - node: prefixed modules are part of the runtime and cannot be resolved to a file * - data: skip json, wasm and inline js https://nodejs.org/api/esm.html#data-imports + * - .min.(mjs|cjs|js) files are and not useful since they dont point to the original source + * - anything that is not a JavaScript or TypeScript source file. Frame filenames come + * from the parsed `error.stack` string, which application code can overwrite with + * external input, so we must not read arbitrary files off the filesystem. */ function shouldSkipContextLinesForFile(path: string): boolean { - // Test the most common prefix and extension first. These are the ones we - // are most likely to see in user applications and are the ones we can break out of first. if (path.startsWith('node:')) return true; - if (path.endsWith('.min.js')) return true; - if (path.endsWith('.min.cjs')) return true; - if (path.endsWith('.min.mjs')) return true; if (path.startsWith('data:')) return true; - return false; + + const pathname = stripUrlQueryAndFragment(path).toLowerCase(); + if (pathname.endsWith('.min.js')) return true; + if (pathname.endsWith('.min.cjs')) return true; + if (pathname.endsWith('.min.mjs')) return true; + return !SOURCE_FILE_EXTENSION.test(pathname); } /** diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts index af49e1da01cc..c8a5d17ce287 100644 --- a/packages/node/test/integrations/contextlines.test.ts +++ b/packages/node/test/integrations/contextlines.test.ts @@ -69,6 +69,44 @@ describe('ContextLines', () => { }); }); + describe('file filtering', () => { + test.each([ + '/var/task/.env', + '/etc/passwd', + '/home/user/.ssh/id_rsa', + 'file:///var/task/config.json', + 'file:///var/task/index.js.bak', + 'file:///var/task/index.min.js?v=abc', + ])('does not read non-source file %s', async filename => { + expect.assertions(1); + const frames: StackFrame[] = [{ colno: 1, filename, lineno: 1, function: 'fxn1' }]; + + const readStreamSpy = vi.spyOn(fs, 'createReadStream'); + await addContext(frames); + expect(readStreamSpy).not.toHaveBeenCalled(); + }); + + test.each([ + 'file:///var/task/index.js', + 'file:///var/task/index.mjs', + 'file:///var/task/index.cjs', + 'file:///var/task/index.ts', + 'file:///var/task/index.tsx', + 'file:///var/task/Index.JS', + 'file:///var/task/index.js?v=abc123', + 'file:///var/task/index.js#hash', + 'file:///var/task/Component.vue', + 'file:///var/task/page.mdx', + ])('reads source file %s', async filename => { + expect.assertions(1); + const frames: StackFrame[] = [{ colno: 1, filename, lineno: 1, function: 'fxn1' }]; + + const readStreamSpy = vi.spyOn(fs, 'createReadStream'); + await addContext(frames); + expect(readStreamSpy).toHaveBeenCalledTimes(1); + }); + }); + describe('lru file cache', () => { test('parseStack when file does not exist', async () => { expect.assertions(4);