Repository navigation
feat(core): capture MCP OAuth client names per request #24508
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
|
|
||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| tracesSampleRate: 1, | ||
| transport: loggingTransport, | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { AsyncLocalStorage } from 'node:async_hooks'; | ||
| import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; | ||
| import { WebStandardStreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js'; | ||
| import { startSpan, wrapMcpServerWithSentry } from '@sentry/node'; | ||
|
|
||
| const requestContext = new AsyncLocalStorage(); | ||
| let resolverCalls = 0; | ||
| const server = wrapMcpServerWithSentry(new McpServer({ name: 'test-server', version: '1.0.0' }), { | ||
| getOAuthClientName(authInfo) { | ||
| resolverCalls++; | ||
| return authInfo?.extra?.clientName ?? requestContext.getStore(); | ||
| }, | ||
| }); | ||
| server.registerTool('echo', {}, async () => ({ content: [{ type: 'text', text: 'ok' }] })); | ||
| const transport = new WebStandardStreamableHTTPServerTransport({ | ||
| sessionIdGenerator: () => 'test-session', | ||
| enableJsonResponse: true, | ||
| }); | ||
|
|
||
| async function request(id, method, clientName, contextName) { | ||
| const params = | ||
| method === 'initialize' | ||
| ? { protocolVersion: '2025-11-25', capabilities: {}, clientInfo: { name: 'protocol-v1', version: '1.0.0' } } | ||
| : {}; | ||
| const response = await requestContext.run(contextName, async () => { | ||
| await new Promise(resolve => setImmediate(resolve)); | ||
| return transport.handleRequest( | ||
| new Request('https://example.test/mcp', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'application/json, text/event-stream', | ||
| 'MCP-Protocol-Version': '2025-11-25', | ||
| ...(method !== 'initialize' && { 'Mcp-Session-Id': 'test-session' }), | ||
| }, | ||
| body: JSON.stringify({ jsonrpc: '2.0', id, method, params }), | ||
| }), | ||
| { | ||
| authInfo: clientName | ||
| ? { clientId: id, token: 'test-token', scopes: ['test-scope'], extra: { clientName } } | ||
| : undefined, | ||
| }, | ||
| ); | ||
| }); | ||
| if (id === undefined) { | ||
| assert.equal(response.status, 202); | ||
| return; | ||
| } | ||
| assert.equal(response.status, 200); | ||
| const body = await response.json(); | ||
| assert.equal(body.id, id); | ||
| if (method === 'resources/list') { | ||
| assert.equal(body.error.code, -32601); | ||
| } else { | ||
| assert.equal(body.error, undefined); | ||
| if (method === 'tools/list') { | ||
| assert.equal(body.result.tools[0].name, 'echo'); | ||
| } else { | ||
| assert.equal(body.result.protocolVersion, '2025-11-25'); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| startSpan({ name: 'oauth requests' }, async span => { | ||
| await server.connect(transport); | ||
| await request('discovery', 'initialize', 'Registered Discovery'); | ||
| await request(undefined, 'notifications/initialized', 'Notification Client'); | ||
| await Promise.all([ | ||
| request('context', 'tools/list', undefined, 'Registered Context'), | ||
| request('unsupported', 'resources/list', 'Registered Unsupported'), | ||
| request('missing', 'tools/list'), | ||
| ]); | ||
| assert.equal(resolverCalls, 4); | ||
| await server.close(); | ||
| span.setAttribute('test.responses.checked', true); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { AsyncLocalStorage } from 'node:async_hooks'; | ||
| import { createMcpHandler, McpServer } from '@modelcontextprotocol/server'; | ||
| import { startSpan, wrapMcpServerWithSentry } from '@sentry/node'; | ||
|
|
||
| const requestContext = new AsyncLocalStorage(); | ||
| let resolverCalls = 0; | ||
| const handler = createMcpHandler(async () => { | ||
| await new Promise(resolve => setImmediate(resolve)); | ||
| const server = wrapMcpServerWithSentry(new McpServer({ name: 'test-server', version: '1.0.0' }), { | ||
| getOAuthClientName(authInfo) { | ||
| resolverCalls++; | ||
| return authInfo?.extra?.clientName ?? requestContext.getStore(); | ||
| }, | ||
| }); | ||
| server.registerTool('echo', {}, async () => ({ content: [{ type: 'text', text: 'ok' }] })); | ||
| return server; | ||
| }); | ||
|
|
||
| async function request(id, method, clientName, contextName) { | ||
| const params = { | ||
| _meta: { | ||
| 'io.modelcontextprotocol/protocolVersion': '2026-07-28', | ||
| 'io.modelcontextprotocol/clientCapabilities': {}, | ||
| ...(id !== 'missing' && { | ||
| 'io.modelcontextprotocol/clientInfo': { name: `protocol-${id}`, version: '1.0.0' }, | ||
| }), | ||
| }, | ||
| }; | ||
| const response = await requestContext.run(contextName, () => | ||
| handler.fetch( | ||
| new Request('https://example.test/mcp', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'application/json, text/event-stream', | ||
| 'MCP-Protocol-Version': '2026-07-28', | ||
| 'MCP-Method': method, | ||
| }, | ||
| body: JSON.stringify({ jsonrpc: '2.0', id, method, params }), | ||
| }), | ||
| { | ||
| authInfo: clientName | ||
| ? { clientId: id, token: 'test-token', scopes: ['test-scope'], extra: { clientName } } | ||
| : undefined, | ||
| }, | ||
| ), | ||
| ); | ||
| const body = await response.json(); | ||
| assert.equal(body.id, id); | ||
| if (method === 'resources/list') { | ||
| assert.equal(response.status, 404); | ||
| assert.equal(body.error.code, -32601); | ||
| } else { | ||
| assert.equal(response.status, 200); | ||
| assert.equal(body.error, undefined); | ||
| if (method === 'tools/list') { | ||
| assert.equal(body.result.tools[0].name, 'echo'); | ||
| } else { | ||
| assert.ok(body.result.supportedVersions.includes('2026-07-28')); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| startSpan({ name: 'oauth requests' }, async span => { | ||
| await Promise.all([ | ||
| request('discovery', 'server/discover', 'Registered Discovery'), | ||
| request('context', 'tools/list', undefined, 'Registered Context'), | ||
| request('unsupported', 'resources/list', 'Registered Unsupported'), | ||
| request('missing', 'tools/list'), | ||
| ]); | ||
| assert.equal(resolverCalls, 4); | ||
| await handler.close(); | ||
| span.setAttribute('test.responses.checked', true); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import { afterAll, describe, expect } from 'vitest'; | ||
| import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; | ||
|
|
||
| describe('MCP OAuth client attribution', () => { | ||
| afterAll(() => { | ||
| cleanupChildProcesses(); | ||
| }); | ||
|
|
||
| for (const version of ['v1', 'v2']) { | ||
| createEsmAndCjsTests( | ||
| __dirname, | ||
| `scenario-${version}.mjs`, | ||
| 'instrument.mjs', | ||
| (createTestRunner, test) => { | ||
| test(`keeps ${version} OAuth identity request-local and separate from MCP identity`, async () => { | ||
| await createTestRunner() | ||
| .unordered() | ||
| .expect({ | ||
| span: container => { | ||
| const root = container.items.find(item => item.name === 'oauth requests'); | ||
| expect(root?.attributes['test.responses.checked']?.value).toBe(true); | ||
| const requests = container.items.filter( | ||
| item => item.attributes['sentry.op']?.value === 'mcp.server' && item.attributes['mcp.request.id'], | ||
| ); | ||
| expect(requests).toHaveLength(4); | ||
| for (const [id, method, oauthName] of [ | ||
| ['discovery', version === 'v1' ? 'initialize' : 'server/discover', 'Registered Discovery'], | ||
| ['context', 'tools/list', 'Registered Context'], | ||
| ['unsupported', 'resources/list', 'Registered Unsupported'], | ||
| ['missing', 'tools/list', undefined], | ||
| ]) { | ||
| const request = requests.find(item => item.attributes['mcp.request.id']?.value === id); | ||
| expect(request).toBeDefined(); | ||
| expect(request?.attributes['mcp.method.name']?.value).toBe(method); | ||
| expect(request?.attributes['mcp.auth.client.name']?.value).toBe(oauthName); | ||
| expect(request?.attributes['mcp.client.name']?.value).toBe( | ||
| version === 'v1' ? 'protocol-v1' : id === 'missing' ? undefined : `protocol-${id}`, | ||
| ); | ||
| } | ||
|
There was a problem hiding this comment. One test loops over scenariosLow Severity A single test iterates the four request identities and mixes v1/v2 conditionals for method and   Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit f965bba. Configure here. |
||
| expect(JSON.stringify(container)).not.toContain('Notification Client'); | ||
| }, | ||
|
There was a problem hiding this comment. Integration test can miss split spansMedium Severity The new OAuth test expects the parent Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit f965bba. Configure here. |
||
| }) | ||
| .start() | ||
| .completed(); | ||
| }); | ||
| }, | ||
| version === 'v1' ? { additionalDependencies: { '@modelcontextprotocol/sdk': '1.30.0' } } : undefined, | ||
| ); | ||
| } | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests hard-code span attribute names
Low Severity
New assertions compare
sentry.op,mcp.server,mcp.request.id,mcp.method.name,mcp.auth.client.name, andmcp.client.nameas string literals.@sentry/conventionsalready exportsSENTRY_OP,MCP_SERVER,MCP_REQUEST_ID,MCP_METHOD_NAME,MCP_AUTH_CLIENT_NAME, andMCP_CLIENT_NAME. I flagged this because the review rules require those constants in test assertions as well as production code.Additional Locations (2)
packages/core/test/lib/integrations/mcp-server/oauthClient.test.ts#L56-L58packages/core/test/lib/integrations/mcp-server/oauthClient.test.ts#L83-L90Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit f965bba. Configure here.