diff --git a/CHANGELOG.md b/CHANGELOG.md index da676fc70..885231619 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed unary Zoekt searches retaining a gRPC channel after every request by closing each client on completion. [#1591](https://github.com/sourcebot-dev/sourcebot/pull/1591) +- [EE] Fixed MCP protocol traffic marking users as active by recording activity only for tool calls. [#1613](https://github.com/sourcebot-dev/sourcebot/pull/1613) ## [5.1.8] - 2026-08-19 diff --git a/packages/web/src/app/api/(server)/ee/mcp/route.test.ts b/packages/web/src/app/api/(server)/ee/mcp/route.test.ts new file mode 100644 index 000000000..bcf2762d0 --- /dev/null +++ b/packages/web/src/app/api/(server)/ee/mcp/route.test.ts @@ -0,0 +1,109 @@ +import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { NextRequest } from 'next/server'; + +const mocks = vi.hoisted(() => ({ + hasEntitlement: vi.fn(), + withOptionalAuth: vi.fn(), +})); + +vi.mock('@/lib/apiHandler', () => ({ + apiHandler: (handler: unknown) => handler, +})); +vi.mock('@/middleware/sew', () => ({ + sew: (callback: () => unknown) => callback(), +})); +vi.mock('@/lib/entitlements', () => ({ + hasEntitlement: mocks.hasEntitlement, +})); +vi.mock('@/middleware/withAuth', () => ({ + withOptionalAuth: mocks.withOptionalAuth, +})); +vi.mock('@/ee/features/mcp/server', () => ({ + createMcpServer: vi.fn(), +})); +vi.mock('@/lib/utils', () => ({ + isServiceError: () => false, +})); +vi.mock('@sourcebot/shared', () => ({ + env: { + AUTH_URL: 'https://sourcebot.example.com', + EXPERIMENT_ASK_GH_ENABLED: 'false', + }, +})); + +const { DELETE, POST } = await import('./route'); + +function createPostRequest(body: unknown) { + return new NextRequest('https://sourcebot.example.com/api/mcp', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: typeof body === 'string' ? body : JSON.stringify(body), + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + mocks.hasEntitlement.mockResolvedValue(true); + mocks.withOptionalAuth.mockResolvedValue(new Response(null, { status: 204 })); +}); + +describe('MCP activity recording', () => { + test('does not record activity for protocol messages', async () => { + const response = await POST(createPostRequest({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { + protocolVersion: '2025-11-25', + capabilities: {}, + clientInfo: { name: 'test-client', version: '1.0.0' }, + }, + })); + + expect(response.status).toBe(204); + expect(mocks.withOptionalAuth).toHaveBeenCalledWith( + expect.any(Function), + { recordActivity: false }, + ); + }); + + test('records activity for a valid tool call', async () => { + const response = await POST(createPostRequest({ + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { + name: 'grep', + arguments: { query: 'activity' }, + }, + })); + + expect(response.status).toBe(204); + expect(mocks.withOptionalAuth).toHaveBeenCalledWith( + expect.any(Function), + { recordActivity: true }, + ); + }); + + test('leaves malformed JSON for the transport and does not record activity', async () => { + const response = await POST(createPostRequest('{"jsonrpc":')); + + expect(response.status).toBe(204); + expect(mocks.withOptionalAuth).toHaveBeenCalledWith( + expect.any(Function), + { recordActivity: false }, + ); + }); + + test('does not record activity when closing a session', async () => { + const response = await DELETE(new NextRequest('https://sourcebot.example.com/api/mcp', { + method: 'DELETE', + })); + + expect(response.status).toBe(204); + expect(mocks.withOptionalAuth).toHaveBeenCalledWith( + expect.any(Function), + { recordActivity: false }, + ); + }); +}); diff --git a/packages/web/src/app/api/(server)/ee/mcp/route.ts b/packages/web/src/app/api/(server)/ee/mcp/route.ts index d82be950a..d5e57224d 100644 --- a/packages/web/src/app/api/(server)/ee/mcp/route.ts +++ b/packages/web/src/app/api/(server)/ee/mcp/route.ts @@ -13,6 +13,7 @@ import { apiHandler } from '@/lib/apiHandler'; import { env } from '@sourcebot/shared'; import { hasEntitlement } from '@/lib/entitlements'; import { SOURCEBOT_OAUTH_SCOPES } from '@/ee/features/oauth/constants'; +import { isMcpActivityMessage } from '@/ee/features/mcp/activity'; // On 401, tell MCP clients where to find the OAuth protected resource metadata (RFC 9728) // so they can discover the authorization server and initiate the authorization code flow. @@ -74,6 +75,14 @@ export const POST = apiHandler(async (request: NextRequest) => { }); } + let jsonRpcMessage: unknown; + try { + jsonRpcMessage = await request.clone().json(); + } catch { + jsonRpcMessage = undefined; + } + const recordActivity = isMcpActivityMessage(jsonRpcMessage); + const response = await sew(() => withOptionalAuth(async ({ user, principal }) => { if (env.EXPERIMENT_ASK_GH_ENABLED === 'true' && !user) { @@ -121,7 +130,7 @@ export const POST = apiHandler(async (request: NextRequest) => { await mcpServer.connect(transport); return transport.handleRequest(request); - }) + }, { recordActivity }) ); if (isServiceError(response)) { @@ -165,7 +174,7 @@ export const DELETE = apiHandler(async (request: NextRequest) => { } return session.transport.handleRequest(request); - }) + }, { recordActivity: false }) ); if (isServiceError(result)) { diff --git a/packages/web/src/ee/features/mcp/activity.test.ts b/packages/web/src/ee/features/mcp/activity.test.ts new file mode 100644 index 000000000..a8072558c --- /dev/null +++ b/packages/web/src/ee/features/mcp/activity.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, test } from 'vitest'; +import { isMcpActivityMessage } from './activity'; + +describe('isMcpActivityMessage', () => { + test('treats a valid tool call as activity', () => { + expect(isMcpActivityMessage({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { + name: 'grep', + arguments: { query: 'activity' }, + }, + })).toBe(true); + }); + + test('treats a tool call in a JSON-RPC batch as activity', () => { + expect(isMcpActivityMessage([ + { + jsonrpc: '2.0', + id: 1, + method: 'ping', + }, + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'grep' }, + }, + ])).toBe(true); + }); + + test.each([ + ['initialize', { + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { + protocolVersion: '2025-11-25', + capabilities: {}, + clientInfo: { name: 'test-client', version: '1.0.0' }, + }, + }], + ['initialized notification', { + jsonrpc: '2.0', + method: 'notifications/initialized', + }], + ['ping', { + jsonrpc: '2.0', + id: 2, + method: 'ping', + }], + ['tool discovery', { + jsonrpc: '2.0', + id: 3, + method: 'tools/list', + }], + ['malformed tool call', { + jsonrpc: '2.0', + id: 4, + method: 'tools/call', + params: {}, + }], + ['tool call without a JSON-RPC envelope', { + method: 'tools/call', + params: { name: 'grep' }, + }], + ])('does not treat %s as activity', (_name, message) => { + expect(isMcpActivityMessage(message)).toBe(false); + }); +}); diff --git a/packages/web/src/ee/features/mcp/activity.ts b/packages/web/src/ee/features/mcp/activity.ts new file mode 100644 index 000000000..6e871611f --- /dev/null +++ b/packages/web/src/ee/features/mcp/activity.ts @@ -0,0 +1,9 @@ +import { CallToolRequestSchema, JSONRPCRequestSchema } from '@modelcontextprotocol/sdk/types.js'; + +export function isMcpActivityMessage(message: unknown): boolean { + const messages = Array.isArray(message) ? message : [message]; + return messages.some((candidate) => + JSONRPCRequestSchema.safeParse(candidate).success && + CallToolRequestSchema.safeParse(candidate).success + ); +} diff --git a/packages/web/src/middleware/withAuth.test.ts b/packages/web/src/middleware/withAuth.test.ts index d331bf991..ea8af9264 100644 --- a/packages/web/src/middleware/withAuth.test.ts +++ b/packages/web/src/middleware/withAuth.test.ts @@ -519,6 +519,37 @@ describe('getAuthenticatedUser', () => { }); describe('getAuthContext', () => { + test('does not record activity or activate a pending membership when activity recording is disabled', async () => { + const userId = 'test-user-id'; + prisma.user.findUnique.mockResolvedValue({ + ...MOCK_USER_WITH_ACCOUNTS, + id: userId, + }); + prisma.org.findUnique.mockResolvedValue(MOCK_ORG); + prisma.userToOrg.findUnique.mockResolvedValue({ + joinedAt: new Date(), + userId, + orgId: MOCK_ORG.id, + suspendedAt: null, + scimExternalId: null, + lastActiveAt: null, + role: OrgRole.MEMBER, + }); + setMockSession(createMockSession({ user: { id: userId } })); + + const authContext = await getAuthContext({ recordActivity: false }); + + expect(authContext).toMatchObject({ + user: { id: userId }, + org: MOCK_ORG, + role: OrgRole.MEMBER, + }); + expect(prisma.user.update).not.toHaveBeenCalled(); + expect(prisma.userToOrg.updateMany).not.toHaveBeenCalled(); + expect(prisma.$transaction).not.toHaveBeenCalled(); + expect(mocks.syncWithLighthouse).not.toHaveBeenCalled(); + }); + test('should pass scoped access token repository IDs to the Prisma extension', async () => { const scopedAccessToken = createMockScopedAccessToken(); prisma.scopedAccessToken.findUnique.mockResolvedValue(scopedAccessToken); diff --git a/packages/web/src/middleware/withAuth.ts b/packages/web/src/middleware/withAuth.ts index b004d6169..6ab5dc550 100644 --- a/packages/web/src/middleware/withAuth.ts +++ b/packages/web/src/middleware/withAuth.ts @@ -57,6 +57,8 @@ export type AuthResult = { type AuthOptions = { requiredOAuthScopes?: readonly string[]; requiredAuthSource?: AuthSource; + /** Whether this request should update user activity and activate a pending membership. Defaults to true. */ + recordActivity?: boolean; }; export const withAuth = async (fn: (params: RequiredAuthContext) => Promise, options: AuthOptions = {}) => { @@ -92,6 +94,11 @@ export const withOptionalAuth = async (fn: (params: OptionalAuthContext) => P }; export const getAuthContext = async (options: AuthOptions = {}): Promise => { + const { + requiredOAuthScopes, + requiredAuthSource, + recordActivity = true, + } = options; const authResult = await getAuthenticatedUser(); const user = authResult?.user; @@ -145,8 +152,8 @@ export const getAuthContext = async (options: AuthOptions = {}): Promise