Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/utils/data-collection/filterKeyValueData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>,
export function filterKeyValueData<T>(
data: Record<string, T>,
behavior: CollectBehavior,
additionalDenyTerms?: string[],
): Record<string, string> {
): Record<string, T | string> {
if (behavior === false) {
return {};
}

const denySnippets =
additionalDenyTerms != null ? [...SENSITIVE_KEY_SNIPPETS, ...additionalDenyTerms] : SENSITIVE_KEY_SNIPPETS;
const result: Record<string, string> = {};
const result: Record<string, T | string> = {};

if (behavior === true) {
for (const key of Object.keys(data)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,33 @@ describe('filterKeyValueData', () => {
});
});

describe('non-string values', () => {
const mixedData: Record<string, unknown> = {
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({});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
12 changes: 12 additions & 0 deletions packages/node-core/src/integrations/local-variables/common.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>;

/**
* 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;

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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###';
Expand All @@ -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');
Expand Down Expand Up @@ -47,7 +56,7 @@ export const localVariablesAsyncIntegration = defineIntegration(((
continue;
}

frame.vars = frameLocalVariables.vars;
frame.vars = filterFrameVariables(frameLocalVariables.vars, behavior);
}
}

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
30 changes: 29 additions & 1 deletion packages/node-core/test/integrations/localvariables.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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<void>(done => {
Expand Down
Loading