Skip to content

Commit 6c67f51

Browse files
committed
Implement PII filtering for MCP server spans. Introduce a new utility to remove sensitive data based on the sendDefaultPii setting.
1 parent d94a4d0 commit 6c67f51

File tree

4 files changed

+258
-4
lines changed

4 files changed

+258
-4
lines changed

packages/core/src/integrations/mcp-server/correlation.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
* Handles mapping requestId to span data for correlation with handler execution
44
*/
55

6+
import { getClient } from '../../currentScopes';
67
import { withActiveSpan } from '../../tracing';
78
import type { Span } from '../../types-hoist/span';
9+
import { filterMcpPiiFromSpanData } from './piiFiltering';
810
import type { RequestId, SessionId } from './types';
911

1012
// Simplified correlation system that works with or without sessionId
@@ -68,8 +70,12 @@ export function completeSpanWithResults(requestId: RequestId, result: unknown):
6870
};
6971

7072
if (spanWithMethods.setAttributes && method === 'tools/call') {
71-
// Add tool-specific attributes
72-
const toolAttributes = extractToolResultAttributes(result);
73+
// Add tool-specific attributes with PII filtering
74+
const rawToolAttributes = extractToolResultAttributes(result);
75+
const client = getClient();
76+
const sendDefaultPii = Boolean(client?.getOptions().sendDefaultPii);
77+
const toolAttributes = filterMcpPiiFromSpanData(rawToolAttributes, sendDefaultPii);
78+
7379
spanWithMethods.setAttributes(toolAttributes);
7480

7581
// Set span status based on tool result
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* PII filtering for MCP server spans
3+
* Removes sensitive data when sendDefaultPii is false
4+
*/
5+
6+
/** PII attributes that should be removed when sendDefaultPii is false */
7+
const PII_ATTRIBUTES = new Set([
8+
'client.address',
9+
'client.port',
10+
'mcp.logging.message',
11+
'mcp.resource.uri',
12+
'mcp.tool.result.content',
13+
]);
14+
15+
/**
16+
* Removes PII attributes from span data when sendDefaultPii is false
17+
*/
18+
export function filterMcpPiiFromSpanData(
19+
spanData: Record<string, unknown>,
20+
sendDefaultPii: boolean,
21+
): Record<string, unknown> {
22+
if (sendDefaultPii) {
23+
return spanData; // Send everything when PII is allowed
24+
}
25+
26+
// Use Object.fromEntries with filter for a more functional approach
27+
return Object.fromEntries(
28+
Object.entries(spanData).filter(([key]) => {
29+
const isPiiAttribute = PII_ATTRIBUTES.has(key) || key.startsWith('mcp.request.argument.');
30+
return !isPiiAttribute;
31+
}),
32+
);
33+
}

packages/core/src/integrations/mcp-server/spans.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Span creation and management functions for MCP server instrumentation
33
*/
44

5+
import { getClient } from '../../currentScopes';
56
import {
67
SEMANTIC_ATTRIBUTE_SENTRY_OP,
78
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
@@ -18,6 +19,7 @@ import {
1819
MCP_ROUTE_SOURCE_VALUE,
1920
MCP_SERVER_OP_VALUE,
2021
} from './attributes';
22+
import { filterMcpPiiFromSpanData } from './piiFiltering';
2123
import type { ExtraHandlerData, JsonRpcNotification, JsonRpcRequest, McpSpanConfig, MCPTransport } from './types';
2224

2325
/**
@@ -77,7 +79,7 @@ function createMcpSpan(config: McpSpanConfig): unknown {
7779
}
7880

7981
// Build attributes
80-
const attributes: Record<string, string | number> = {
82+
const rawAttributes: Record<string, string | number> = {
8183
// Base attributes
8284
...buildTransportAttributes(transport, extra),
8385
// Method name (required for all spans)
@@ -88,6 +90,11 @@ function createMcpSpan(config: McpSpanConfig): unknown {
8890
...buildSentryAttributes(type),
8991
};
9092

93+
// Apply PII filtering based on sendDefaultPii setting
94+
const client = getClient();
95+
const sendDefaultPii = Boolean(client?.getOptions().sendDefaultPii);
96+
const attributes = filterMcpPiiFromSpanData(rawAttributes, sendDefaultPii) as Record<string, string | number>;
97+
9198
return startSpan(
9299
{
93100
name: spanName,
@@ -154,7 +161,7 @@ export function buildMcpServerSpanConfig(
154161
const spanName = createSpanName(method, targetInfo.target);
155162

156163
// Build comprehensive attributes
157-
const attributes: Record<string, string | number> = {
164+
const rawAttributes: Record<string, string | number> = {
158165
// Base attributes
159166
...buildTransportAttributes(transport, extra),
160167
// Method and request info
@@ -165,6 +172,11 @@ export function buildMcpServerSpanConfig(
165172
...buildSentryAttributes('request'),
166173
};
167174

175+
// Apply PII filtering based on sendDefaultPii setting
176+
const client = getClient();
177+
const sendDefaultPii = Boolean(client?.getOptions().sendDefaultPii);
178+
const attributes = filterMcpPiiFromSpanData(rawAttributes, sendDefaultPii) as Record<string, string | number>;
179+
168180
return {
169181
name: spanName,
170182
op: MCP_SERVER_OP_VALUE,

packages/core/test/lib/mcp-server.test.ts

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import * as currentScopes from '../../src/currentScopes';
23
import { wrapMcpServerWithSentry } from '../../src/integrations/mcp-server';
4+
import { filterMcpPiiFromSpanData } from '../../src/integrations/mcp-server/piiFiltering';
35
import * as tracingModule from '../../src/tracing';
46

57
describe('wrapMcpServerWithSentry', () => {
68
const startSpanSpy = vi.spyOn(tracingModule, 'startSpan');
79
const startInactiveSpanSpy = vi.spyOn(tracingModule, 'startInactiveSpan');
10+
const getClientSpy = vi.spyOn(currentScopes, 'getClient');
811

912
beforeEach(() => {
1013
vi.clearAllMocks();
14+
// Mock client to return sendDefaultPii: true for instrumentation tests
15+
getClientSpy.mockReturnValue({
16+
getOptions: () => ({ sendDefaultPii: true }),
17+
getDsn: () => ({ publicKey: 'test-key', host: 'test-host' }),
18+
emit: vi.fn(),
19+
} as any);
1120
});
1221

1322
it('should return the same instance (modified) if it is a valid MCP server instance', () => {
@@ -709,6 +718,200 @@ describe('wrapMcpServerWithSentry', () => {
709718
);
710719
});
711720
});
721+
722+
describe('PII Filtering', () => {
723+
let mockMcpServer: ReturnType<typeof createMockMcpServer>;
724+
let wrappedMcpServer: ReturnType<typeof createMockMcpServer>;
725+
let mockTransport: ReturnType<typeof createMockTransport>;
726+
727+
beforeEach(() => {
728+
mockMcpServer = createMockMcpServer();
729+
wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer);
730+
mockTransport = createMockTransport();
731+
mockTransport.sessionId = 'test-session-123';
732+
});
733+
734+
it('should include PII data when sendDefaultPii is true', async () => {
735+
// Mock client with sendDefaultPii: true
736+
getClientSpy.mockReturnValue({
737+
getOptions: () => ({ sendDefaultPii: true }),
738+
getDsn: () => ({ publicKey: 'test-key', host: 'test-host' }),
739+
emit: vi.fn(),
740+
} as any);
741+
742+
await wrappedMcpServer.connect(mockTransport);
743+
744+
const jsonRpcRequest = {
745+
jsonrpc: '2.0',
746+
method: 'tools/call',
747+
id: 'req-pii-true',
748+
params: { name: 'weather', arguments: { location: 'London', units: 'metric' } },
749+
};
750+
751+
const extraWithClientInfo = {
752+
requestInfo: {
753+
remoteAddress: '192.168.1.100',
754+
remotePort: 54321,
755+
},
756+
};
757+
758+
mockTransport.onmessage?.(jsonRpcRequest, extraWithClientInfo);
759+
760+
expect(startInactiveSpanSpy).toHaveBeenCalledWith({
761+
name: 'tools/call weather',
762+
op: 'mcp.server',
763+
forceTransaction: true,
764+
attributes: expect.objectContaining({
765+
'client.address': '192.168.1.100',
766+
'client.port': 54321,
767+
'mcp.request.argument.location': '"London"',
768+
'mcp.request.argument.units': '"metric"',
769+
'mcp.tool.name': 'weather',
770+
}),
771+
});
772+
});
773+
774+
it('should exclude PII data when sendDefaultPii is false', async () => {
775+
// Mock client with sendDefaultPii: false
776+
getClientSpy.mockReturnValue({
777+
getOptions: () => ({ sendDefaultPii: false }),
778+
getDsn: () => ({ publicKey: 'test-key', host: 'test-host' }),
779+
emit: vi.fn(),
780+
} as any);
781+
782+
await wrappedMcpServer.connect(mockTransport);
783+
784+
const jsonRpcRequest = {
785+
jsonrpc: '2.0',
786+
method: 'tools/call',
787+
id: 'req-pii-false',
788+
params: { name: 'weather', arguments: { location: 'London', units: 'metric' } },
789+
};
790+
791+
const extraWithClientInfo = {
792+
requestInfo: {
793+
remoteAddress: '192.168.1.100',
794+
remotePort: 54321,
795+
},
796+
};
797+
798+
mockTransport.onmessage?.(jsonRpcRequest, extraWithClientInfo);
799+
800+
const callArgs = startInactiveSpanSpy.mock.calls[0][0];
801+
802+
// PII data should be filtered out
803+
expect(callArgs.attributes).not.toHaveProperty('client.address');
804+
expect(callArgs.attributes).not.toHaveProperty('client.port');
805+
expect(callArgs.attributes).not.toHaveProperty('mcp.request.argument.location');
806+
expect(callArgs.attributes).not.toHaveProperty('mcp.request.argument.units');
807+
808+
// Non-PII data should remain
809+
expect(callArgs.attributes).toHaveProperty('mcp.tool.name', 'weather');
810+
expect(callArgs.attributes).toHaveProperty('mcp.method.name', 'tools/call');
811+
});
812+
813+
it('should filter tool result content when sendDefaultPii is false', async () => {
814+
// Mock client with sendDefaultPii: false
815+
getClientSpy.mockReturnValue({
816+
getOptions: () => ({ sendDefaultPii: false }),
817+
} as any);
818+
819+
await wrappedMcpServer.connect(mockTransport);
820+
821+
const mockSpan = {
822+
setAttributes: vi.fn(),
823+
setStatus: vi.fn(),
824+
end: vi.fn(),
825+
};
826+
startInactiveSpanSpy.mockReturnValueOnce(mockSpan);
827+
828+
const toolCallRequest = {
829+
jsonrpc: '2.0',
830+
method: 'tools/call',
831+
id: 'req-tool-result-filtered',
832+
params: { name: 'weather-lookup' },
833+
};
834+
835+
mockTransport.onmessage?.(toolCallRequest, {});
836+
837+
const toolResponse = {
838+
jsonrpc: '2.0',
839+
id: 'req-tool-result-filtered',
840+
result: {
841+
content: [{ type: 'text', text: 'Sensitive weather data for London' }],
842+
isError: false,
843+
},
844+
};
845+
846+
mockTransport.send?.(toolResponse);
847+
848+
// Tool result content should be filtered out
849+
const setAttributesCall = mockSpan.setAttributes.mock.calls[0][0];
850+
expect(setAttributesCall).not.toHaveProperty('mcp.tool.result.content');
851+
expect(setAttributesCall).toHaveProperty('mcp.tool.result.is_error', false);
852+
expect(setAttributesCall).toHaveProperty('mcp.tool.result.content_count', 1);
853+
});
854+
});
855+
856+
describe('PII Filtering Function', () => {
857+
it('should preserve all data when sendDefaultPii is true', () => {
858+
const spanData = {
859+
'client.address': '192.168.1.100',
860+
'client.port': 54321,
861+
'mcp.request.argument.location': '"San Francisco"',
862+
'mcp.tool.result.content': 'Weather data: 18°C',
863+
'mcp.logging.message': 'User requested weather',
864+
'mcp.resource.uri': 'file:///private/docs/secret.txt',
865+
'mcp.method.name': 'tools/call', // Non-PII should remain
866+
};
867+
868+
const result = filterMcpPiiFromSpanData(spanData, true);
869+
870+
expect(result).toEqual(spanData); // All data preserved
871+
});
872+
873+
it('should remove PII data when sendDefaultPii is false', () => {
874+
const spanData = {
875+
'client.address': '192.168.1.100',
876+
'client.port': 54321,
877+
'mcp.request.argument.location': '"San Francisco"',
878+
'mcp.request.argument.units': '"celsius"',
879+
'mcp.tool.result.content': 'Weather data: 18°C',
880+
'mcp.logging.message': 'User requested weather',
881+
'mcp.resource.uri': 'file:///private/docs/secret.txt',
882+
'mcp.method.name': 'tools/call', // Non-PII should remain
883+
'mcp.session.id': 'test-session-123', // Non-PII should remain
884+
};
885+
886+
const result = filterMcpPiiFromSpanData(spanData, false);
887+
888+
expect(result).not.toHaveProperty('client.address');
889+
expect(result).not.toHaveProperty('client.port');
890+
expect(result).not.toHaveProperty('mcp.request.argument.location');
891+
expect(result).not.toHaveProperty('mcp.request.argument.units');
892+
expect(result).not.toHaveProperty('mcp.tool.result.content');
893+
expect(result).not.toHaveProperty('mcp.logging.message');
894+
expect(result).not.toHaveProperty('mcp.resource.uri');
895+
896+
expect(result).toHaveProperty('mcp.method.name', 'tools/call');
897+
expect(result).toHaveProperty('mcp.session.id', 'test-session-123');
898+
});
899+
900+
it('should handle empty span data', () => {
901+
const result = filterMcpPiiFromSpanData({}, false);
902+
expect(result).toEqual({});
903+
});
904+
905+
it('should handle span data with no PII attributes', () => {
906+
const spanData = {
907+
'mcp.method.name': 'tools/list',
908+
'mcp.session.id': 'test-session',
909+
};
910+
911+
const result = filterMcpPiiFromSpanData(spanData, false);
912+
expect(result).toEqual(spanData);
913+
});
914+
});
712915
});
713916

714917
// Test helpers

0 commit comments

Comments
 (0)