diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-disabled.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-disabled.js new file mode 100644 index 000000000000..e9695c1a4aae --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-disabled.js @@ -0,0 +1,24 @@ +/* eslint-disable no-unused-vars */ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + includeLocalVariables: true, + dataCollection: { stackFrameVariables: false }, + transport: loggingTransport, +}); + +process.on('uncaughtException', () => { + // do nothing - this will prevent the Error below from closing this process +}); + +function one(name) { + const keepVar = 'keep me'; + + throw new Error('Enough!'); +} + +setTimeout(() => { + one('some name'); +}, 1000); diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-filtered.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-filtered.js new file mode 100644 index 000000000000..48e6e36b83a3 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-filtered.js @@ -0,0 +1,25 @@ +/* eslint-disable no-unused-vars */ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + includeLocalVariables: true, + dataCollection: { stackFrameVariables: { deny: ['secretVar'] } }, + transport: loggingTransport, +}); + +process.on('uncaughtException', () => { + // do nothing - this will prevent the Error below from closing this process +}); + +function one(name) { + const keepVar = 'keep me'; + const secretVar = 'filter me'; + + throw new Error('Enough!'); +} + +setTimeout(() => { + one('some name'); +}, 1000); diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts index 6c042d3ecf1f..b0802b96f263 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts @@ -130,6 +130,36 @@ module.exports = { out_of_app_function };`, .completed(); }); + test('Filters local variables by name via dataCollection.stackFrameVariables', async () => { + await createRunner(__dirname, 'local-variables-filtered.js') + .expect({ + event: event => { + const frame = event.exception?.values?.[0]?.stacktrace?.frames?.find(frame => frame.function === 'one'); + + expect(frame?.vars).toEqual({ + name: 'some name', + keepVar: 'keep me', + secretVar: '[Filtered]', + }); + }, + }) + .start() + .completed(); + }); + + test('Does not attach local variables when dataCollection.stackFrameVariables is false', async () => { + await createRunner(__dirname, 'local-variables-disabled.js') + .expect({ + event: event => { + for (const frame of event.exception?.values?.[0]?.stacktrace?.frames || []) { + expect(frame.vars).toBeUndefined(); + } + }, + }) + .start() + .completed(); + }); + test('Should handle different function name formats', async () => { await createRunner(__dirname, 'local-variables-name-matching.js') .expect({ diff --git a/packages/core/src/types/datacollection.ts b/packages/core/src/types/datacollection.ts index b160170761d7..8d87b07f9577 100644 --- a/packages/core/src/types/datacollection.ts +++ b/packages/core/src/types/datacollection.ts @@ -91,9 +91,16 @@ export interface DataCollection { /** * Capture local variable values in stack frames. + * + * Accepts a Boolean (`true` collects all variables, `false` collects none) or a `CollectBehavior` to filter which + * variables are sent by name (`{ allow: [...] }` / `{ deny: [...] }`), matching against variable names. + * + * Note: filtering by name requires knowing the variable names **as they appear after bundling**. Minifiers and other + * build-time transforms frequently rename local variables (e.g. `password` becomes `a`), so allow/deny terms + * configured against source names may not match the names captured at runtime. * @default true */ - stackFrameVariables?: boolean; + stackFrameVariables?: boolean | CollectBehavior; /** * Number of source code context lines to capture around stack frames. diff --git a/packages/core/src/utils/data-collection/filterKeyValueData.ts b/packages/core/src/utils/data-collection/filterKeyValueData.ts index 0d8b00736f87..3cc85ca8eb75 100644 --- a/packages/core/src/utils/data-collection/filterKeyValueData.ts +++ b/packages/core/src/utils/data-collection/filterKeyValueData.ts @@ -13,18 +13,18 @@ function isSensitiveKey(lower: string, denySnippets: string[]): boolean { * * @param additionalDenyTerms - Additional sensitive snippets to check beyond the built-in denylist. */ -export function filterKeyValueData( - data: Record, +export function filterKeyValueData( + data: Record, behavior: CollectBehavior, additionalDenyTerms?: string[], -): Record { +): Record { if (behavior === false) { return {}; } const denySnippets = additionalDenyTerms != null ? [...SENSITIVE_KEY_SNIPPETS, ...additionalDenyTerms] : SENSITIVE_KEY_SNIPPETS; - const result: Record = {}; + const result: Record = {}; if (behavior === true) { for (const key of Object.keys(data)) { diff --git a/packages/core/test/lib/utils/data-collection/filterKeyValueData.test.ts b/packages/core/test/lib/utils/data-collection/filterKeyValueData.test.ts index b472574dc546..2daab52d0bc3 100644 --- a/packages/core/test/lib/utils/data-collection/filterKeyValueData.test.ts +++ b/packages/core/test/lib/utils/data-collection/filterKeyValueData.test.ts @@ -108,6 +108,33 @@ describe('filterKeyValueData', () => { }); }); + describe('non-string values', () => { + const mixedData: Record = { + count: 42, + enabled: true, + nested: { a: 1 }, + password: 'hunter2', + }; + + it('preserves non-string values verbatim when kept', () => { + const result = filterKeyValueData(mixedData, true); + + expect(result.count).toBe(42); + expect(result.enabled).toBe(true); + expect(result.nested).toEqual({ a: 1 }); + // "password" matches the built-in sensitive denylist + expect(result.password).toBe('[Filtered]'); + }); + + it('replaces filtered non-string values with the string placeholder', () => { + const result = filterKeyValueData(mixedData, { allow: ['count'] }); + + expect(result.count).toBe(42); + expect(result.enabled).toBe('[Filtered]'); + expect(result.nested).toBe('[Filtered]'); + }); + }); + describe('edge cases', () => { it('handles empty record', () => { expect(filterKeyValueData({}, true)).toEqual({}); diff --git a/packages/core/test/lib/utils/data-collection/resolveDataCollectionOptions.test.ts b/packages/core/test/lib/utils/data-collection/resolveDataCollectionOptions.test.ts index 9f554a1897e2..8d6ec21a0ab6 100644 --- a/packages/core/test/lib/utils/data-collection/resolveDataCollectionOptions.test.ts +++ b/packages/core/test/lib/utils/data-collection/resolveDataCollectionOptions.test.ts @@ -166,6 +166,28 @@ describe('resolveDataCollectionOptions', () => { expect(result.databaseQueryData).toBe(false); }); + + it('supports allow/deny list for stack frame variables', () => { + expect( + resolveDataCollectionOptions({ dataCollection: { stackFrameVariables: { allow: ['user'] } } }) + .stackFrameVariables, + ).toEqual({ allow: ['user'] }); + + expect( + resolveDataCollectionOptions({ dataCollection: { stackFrameVariables: { deny: ['password'] } } }) + .stackFrameVariables, + ).toEqual({ deny: ['password'] }); + }); + + it('supports turning off stack frame variables', () => { + const result = resolveDataCollectionOptions({ + dataCollection: { + stackFrameVariables: false, + }, + }); + + expect(result.stackFrameVariables).toBe(false); + }); }); describe('return type completeness', () => { diff --git a/packages/node-core/src/integrations/local-variables/common.ts b/packages/node-core/src/integrations/local-variables/common.ts index f86988b4cbfc..aeb77ddf09e3 100644 --- a/packages/node-core/src/integrations/local-variables/common.ts +++ b/packages/node-core/src/integrations/local-variables/common.ts @@ -1,7 +1,19 @@ import type { Debugger } from 'node:inspector'; +import type { CollectBehavior } from '@sentry/core'; +import { _INTERNAL_filterKeyValueData } from '@sentry/core'; export type Variables = Record; +/** + * Filters captured frame variables by name according to a `dataCollection.stackFrameVariables` behavior. + * + * `true` keeps all variables (built-in sensitive names are still scrubbed), `false` drops them all, and the + * `{ allow: [...] }` / `{ deny: [...] }` forms filter by variable name. + */ +export function filterFrameVariables(vars: Variables, behavior: CollectBehavior): Variables { + return _INTERNAL_filterKeyValueData(vars, behavior); +} + export type RateLimitIncrement = () => void; /** diff --git a/packages/node-core/src/integrations/local-variables/local-variables-async.ts b/packages/node-core/src/integrations/local-variables/local-variables-async.ts index 6d2070988d00..b3552f99fd02 100644 --- a/packages/node-core/src/integrations/local-variables/local-variables-async.ts +++ b/packages/node-core/src/integrations/local-variables/local-variables-async.ts @@ -1,10 +1,10 @@ import { Worker } from 'node:worker_threads'; -import type { Event, EventHint, Exception, IntegrationFn } from '@sentry/core'; -import { debug, defineIntegration } from '@sentry/core'; +import type { CollectBehavior, Event, EventHint, Exception, IntegrationFn } from '@sentry/core'; +import { debug, defineIntegration, getClient } from '@sentry/core'; import type { NodeClient } from '../../sdk/client'; import { isDebuggerEnabled } from '../../utils/debug'; import type { FrameVariables, LocalVariablesIntegrationOptions, LocalVariablesWorkerArgs } from './common'; -import { functionNamesMatch, LOCAL_VARIABLES_KEY } from './common'; +import { filterFrameVariables, functionNamesMatch, LOCAL_VARIABLES_KEY } from './common'; // This string is a placeholder that gets overwritten with the worker code. export const base64WorkerScript = '###LocalVariablesWorkerScript###'; @@ -19,7 +19,16 @@ function log(...args: unknown[]): void { export const localVariablesAsyncIntegration = defineIntegration((( integrationOptions: LocalVariablesIntegrationOptions = {}, ) => { - function addLocalVariablesToException(exception: Exception, localVariables: FrameVariables[]): void { + function addLocalVariablesToException( + exception: Exception, + localVariables: FrameVariables[], + behavior: CollectBehavior, + ): void { + // When disabled, nothing is collected so we don't attach empty `vars` to frames + if (behavior === false) { + return; + } + // Filter out frames where the function name is `new Promise` since these are in the error.stack frames // but do not appear in the debugger call frames const frames = (exception.stacktrace?.frames || []).filter(frame => frame.function !== 'new Promise'); @@ -47,7 +56,7 @@ export const localVariablesAsyncIntegration = defineIntegration((( continue; } - frame.vars = frameLocalVariables.vars; + frame.vars = filterFrameVariables(frameLocalVariables.vars, behavior); } } @@ -58,8 +67,10 @@ export const localVariablesAsyncIntegration = defineIntegration((( LOCAL_VARIABLES_KEY in hint.originalException && Array.isArray(hint.originalException[LOCAL_VARIABLES_KEY]) ) { + const behavior = getClient()?.getDataCollectionOptions().stackFrameVariables ?? true; + for (const exception of event.exception?.values || []) { - addLocalVariablesToException(exception, hint.originalException[LOCAL_VARIABLES_KEY]); + addLocalVariablesToException(exception, hint.originalException[LOCAL_VARIABLES_KEY], behavior); } hint.originalException[LOCAL_VARIABLES_KEY] = undefined; diff --git a/packages/node-core/src/integrations/local-variables/local-variables-sync.ts b/packages/node-core/src/integrations/local-variables/local-variables-sync.ts index 8ae1201732c8..043132fcb275 100644 --- a/packages/node-core/src/integrations/local-variables/local-variables-sync.ts +++ b/packages/node-core/src/integrations/local-variables/local-variables-sync.ts @@ -1,5 +1,5 @@ import type { Debugger, InspectorNotification, Runtime, Session } from 'node:inspector'; -import type { Event, Exception, IntegrationFn, StackFrame, StackParser } from '@sentry/core'; +import type { CollectBehavior, Event, Exception, IntegrationFn, StackFrame, StackParser } from '@sentry/core'; import { debug, defineIntegration, getClient, LRUMap } from '@sentry/core'; import { NODE_MAJOR } from '../../nodeVersion'; import type { NodeClient } from '../../sdk/client'; @@ -11,7 +11,7 @@ import type { RateLimitIncrement, Variables, } from './common'; -import { createRateLimiter, functionNamesMatch } from './common'; +import { createRateLimiter, filterFrameVariables, functionNamesMatch } from './common'; /** Creates a unique hash from stack frames */ export function hashFrames(frames: StackFrame[] | undefined): string | undefined { @@ -234,7 +234,7 @@ const _localVariablesSyncIntegration = (( let rateLimiter: RateLimitIncrement | undefined; let shouldProcessEvent = false; - function addLocalVariablesToException(exception: Exception): void { + function addLocalVariablesToException(exception: Exception, behavior: CollectBehavior): void { const hash = hashFrames(exception.stacktrace?.frames); if (hash === undefined) { @@ -245,7 +245,8 @@ const _localVariablesSyncIntegration = (( // remove is identical to get but also removes the entry from the cache const cachedFrame = cachedFrames.remove(hash); - if (cachedFrame === undefined) { + // When disabled, nothing is collected so we don't attach empty `vars` to frames + if (cachedFrame === undefined || behavior === false) { return; } @@ -276,13 +277,13 @@ const _localVariablesSyncIntegration = (( continue; } - frameVariable.vars = cachedFrameVariable.vars; + frameVariable.vars = filterFrameVariables(cachedFrameVariable.vars, behavior); } } function addLocalVariablesToEvent(event: Event): Event { for (const exception of event.exception?.values || []) { - addLocalVariablesToException(exception); + addLocalVariablesToException(exception, getClient()?.getDataCollectionOptions().stackFrameVariables ?? true); } return event; diff --git a/packages/node-core/test/integrations/localvariables.test.ts b/packages/node-core/test/integrations/localvariables.test.ts index 0c7fd8b52689..82e43ef34135 100644 --- a/packages/node-core/test/integrations/localvariables.test.ts +++ b/packages/node-core/test/integrations/localvariables.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { createRateLimiter } from '../../src/integrations/local-variables/common'; +import { createRateLimiter, filterFrameVariables } from '../../src/integrations/local-variables/common'; import { createCallbackList } from '../../src/integrations/local-variables/local-variables-sync'; import { NODE_MAJOR } from '../../src/nodeVersion'; @@ -14,6 +14,34 @@ describeIf(NODE_MAJOR >= 18)('LocalVariables', () => { vi.useRealTimers(); }); + describe('filterFrameVariables', () => { + const vars = { user: 'bob', password: 'hunter2', count: 42 }; + + it('keeps all variables on `true` but scrubs sensitive names', () => { + expect(filterFrameVariables(vars, true)).toEqual({ user: 'bob', password: '[Filtered]', count: 42 }); + }); + + it('drops all variables on `false`', () => { + expect(filterFrameVariables(vars, false)).toEqual({}); + }); + + it('keeps only allowed variable names', () => { + expect(filterFrameVariables(vars, { allow: ['user', 'count'] })).toEqual({ + user: 'bob', + password: '[Filtered]', + count: 42, + }); + }); + + it('filters denied variable names', () => { + expect(filterFrameVariables(vars, { deny: ['count'] })).toEqual({ + user: 'bob', + password: '[Filtered]', + count: '[Filtered]', + }); + }); + }); + describe('createCallbackList', () => { it('Should call callbacks in reverse order', () => new Promise(done => {