diff --git a/docs/cloudflare/oauth-architecture.md b/docs/cloudflare/oauth-architecture.md index 8a8221af..616a3843 100644 --- a/docs/cloudflare/oauth-architecture.md +++ b/docs/cloudflare/oauth-architecture.md @@ -185,12 +185,13 @@ MCP tokens contain encrypted properties including Sentry tokens: ```typescript interface WorkerProps { id: string; // Sentry user ID - name: string; // User name accessToken: string; // Sentry access token refreshToken?: string; // Sentry refresh token accessTokenExpiresAt?: number; // Sentry token expiry timestamp + clientId: string; // MCP client ID scope: string; // MCP permissions granted - grantedScopes?: string[]; // Sentry API scopes + grantedSkills?: string[]; // Skills granted (primary authorization) + // grantedScopes is deprecated and will be removed Jan 1, 2026 } ``` @@ -234,12 +235,12 @@ const { redirectTo } = await c.env.OAUTH_PROVIDER.completeAuthorization({ // ... other params props: { id: payload.user.id, // From Sentry - name: payload.user.name, // From Sentry accessToken: payload.access_token, // Sentry's access token refreshToken: payload.refresh_token, // Sentry's refresh token accessTokenExpiresAt: Date.now() + payload.expires_in * 1000, + clientId: oauthReqInfo.clientId, // MCP client ID scope: oauthReqInfo.scope.join(" "), // MCP scopes - grantedScopes: Array.from(grantedScopes), // Sentry API scopes + grantedSkills: Array.from(validSkills), // Skills granted (primary authorization) // ... other fields } }); diff --git a/docs/releases/cloudflare.mdc b/docs/releases/cloudflare.mdc index 43d4a19d..5451c395 100644 --- a/docs/releases/cloudflare.mdc +++ b/docs/releases/cloudflare.mdc @@ -81,7 +81,7 @@ const mcpHandler: ExportedHandler = { userId: oauthCtx.props.userId, clientId: oauthCtx.props.clientId, accessToken: oauthCtx.props.accessToken, - grantedScopes: expandedScopes, + grantedSkills, // Primary authorization method constraints: verification.constraints, sentryHost, mcpUrl: oauthCtx.props.mcpUrl, diff --git a/docs/security.mdc b/docs/security.mdc index 0975e265..316526d3 100644 --- a/docs/security.mdc +++ b/docs/security.mdc @@ -57,7 +57,8 @@ interface ServerContext { userId?: string; clientId: string; accessToken: string; - grantedScopes?: Set; + grantedSkills: Set; // Primary authorization method + // grantedScopes is deprecated and will be removed Jan 1, 2026 constraints: Constraints; sentryHost: string; mcpUrl?: string; diff --git a/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts b/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts index 1ae3fbca..fe0b1f55 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts @@ -38,7 +38,7 @@ describe("mcp-handler", () => { id: "test-user-123", clientId: "test-client", accessToken: "test-token", - grantedScopes: ["org:read", "project:read"], + grantedSkills: ["inspect", "docs"], }, }; }); @@ -109,4 +109,54 @@ describe("mcp-handler", () => { // Verify successful response expect(response.status).toBe(200); }); + + it("returns 401 and revokes grant for legacy tokens without grantedSkills", async () => { + const legacyCtx = { + waitUntil: vi.fn(), + passThroughOnException: vi.fn(), + props: { + id: "test-user-123", + clientId: "test-client", + accessToken: "test-token", + // Legacy token: has grantedScopes but no grantedSkills + grantedScopes: ["org:read", "project:read"], + }, + }; + + // Mock the OAuth provider for grant revocation + const mockRevokeGrant = vi.fn(); + const mockListUserGrants = vi.fn().mockResolvedValue({ + items: [{ id: "grant-123", clientId: "test-client" }], + }); + const envWithOAuth = { + ...env, + OAUTH_PROVIDER: { + listUserGrants: mockListUserGrants, + revokeGrant: mockRevokeGrant, + }, + } as unknown as Env; + + const request = new Request("https://test.mcp.sentry.io/mcp"); + + const response = await handler.fetch!( + request as any, + envWithOAuth, + legacyCtx as any, + ); + + // Verify 401 response with re-auth message + expect(response.status).toBe(401); + expect(await response.text()).toContain("re-authorize"); + + // Verify waitUntil was called for background grant revocation + expect(legacyCtx.waitUntil).toHaveBeenCalled(); + + // Wait for the background task to complete + const waitUntilPromise = legacyCtx.waitUntil.mock.calls[0][0]; + await waitUntilPromise; + + // Verify grant was looked up and revoked + expect(mockListUserGrants).toHaveBeenCalledWith("test-user-123"); + expect(mockRevokeGrant).toHaveBeenCalledWith("grant-123", "test-user-123"); + }); }); diff --git a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts index 1cefeb76..b081acc0 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts @@ -10,13 +10,8 @@ import { createMcpHandler } from "agents/mcp"; import { buildServer } from "@sentry/mcp-core/server"; -import { - expandScopes, - parseScopes, - type Scope, -} from "@sentry/mcp-core/permissions"; -import { parseSkills, type Skill } from "@sentry/mcp-core/skills"; -import { logIssue, logWarn } from "@sentry/mcp-core/telem/logging"; +import { parseSkills } from "@sentry/mcp-core/skills"; +import { logWarn } from "@sentry/mcp-core/telem/logging"; import type { ServerContext } from "@sentry/mcp-core/types"; import type { Env } from "../types"; import { verifyConstraintsAccess } from "./constraint-utils"; @@ -85,67 +80,61 @@ const mcpHandler: ExportedHandler = { }); } - // Parse and expand granted scopes (LEGACY - for backward compatibility) - let expandedScopes: Set | undefined; - if (oauthCtx.props.grantedScopes) { - const { valid, invalid } = parseScopes( - oauthCtx.props.grantedScopes as string[], + // Parse and validate granted skills (primary authorization method) + // Legacy tokens without grantedSkills are no longer supported + if (!oauthCtx.props.grantedSkills) { + const userId = oauthCtx.props.id as string; + const clientId = oauthCtx.props.clientId as string; + + logWarn("Legacy token without grantedSkills detected - revoking grant", { + loggerScope: ["cloudflare", "mcp-handler"], + extra: { clientId, userId }, + }); + + // Revoke the grant in the background (don't block the response) + ctx.waitUntil( + (async () => { + try { + // Find the grant for this user/client combination + const grants = await env.OAUTH_PROVIDER.listUserGrants(userId); + const grant = grants.items.find((g) => g.clientId === clientId); + + if (grant) { + await env.OAUTH_PROVIDER.revokeGrant(grant.id, userId); + } + } catch (err) { + logWarn("Failed to revoke legacy grant", { + loggerScope: ["cloudflare", "mcp-handler"], + extra: { error: String(err), clientId, userId }, + }); + } + })(), ); - if (invalid.length > 0) { - logWarn("Ignoring invalid scopes from OAuth provider", { - loggerScope: ["cloudflare", "mcp-handler"], - extra: { - invalidScopes: invalid, - }, - }); - } - expandedScopes = expandScopes(new Set(valid)); - } - // Parse and validate granted skills (NEW - primary authorization method) - let grantedSkills: Set | undefined; - if (oauthCtx.props.grantedSkills) { - const { valid, invalid } = parseSkills( - oauthCtx.props.grantedSkills as string[], + return new Response( + "Your authorization has expired. Please re-authorize to continue using Sentry MCP.", + { status: 401 }, ); - if (invalid.length > 0) { - logWarn("Ignoring invalid skills from OAuth provider", { - loggerScope: ["cloudflare", "mcp-handler"], - extra: { - invalidSkills: invalid, - }, - }); - } - grantedSkills = new Set(valid); - - // Validate that at least one valid skill was granted - if (grantedSkills.size === 0) { - return new Response( - "Authorization failed: No valid skills were granted. Please re-authorize and select at least one permission.", - { status: 400 }, - ); - } } - // Validate that at least one authorization system is active - // This should never happen in practice - indicates a bug in OAuth flow - if (!grantedSkills && !expandedScopes) { - logIssue( - new Error( - "No authorization grants found - server would expose no tools", - ), - { - loggerScope: ["cloudflare", "mcp-handler"], - extra: { - clientId: oauthCtx.props.clientId, - hasGrantedSkills: !!oauthCtx.props.grantedSkills, - hasGrantedScopes: !!oauthCtx.props.grantedScopes, - }, + const { valid: validSkills, invalid: invalidSkills } = parseSkills( + oauthCtx.props.grantedSkills as string[], + ); + + if (invalidSkills.length > 0) { + logWarn("Ignoring invalid skills from OAuth provider", { + loggerScope: ["cloudflare", "mcp-handler"], + extra: { + invalidSkills, }, - ); + }); + } + + // Validate that at least one valid skill was granted + if (validSkills.size === 0) { return new Response( - "Authorization failed: No valid permissions were granted. Please re-authorize and select at least one permission.", - { status: 401 }, + "Authorization failed: No valid skills were granted. Please re-authorize and select at least one permission.", + { status: 400 }, ); } @@ -154,10 +143,7 @@ const mcpHandler: ExportedHandler = { userId: oauthCtx.props.id as string | undefined, clientId: oauthCtx.props.clientId as string, accessToken: oauthCtx.props.accessToken as string, - // Scopes derived from skills - for backward compatibility with old MCP clients - // that don't support grantedSkills and only understand grantedScopes - grantedScopes: expandedScopes, - grantedSkills, // Primary authorization method + grantedSkills: validSkills, constraints: verification.constraints, sentryHost, mcpUrl: env.MCP_URL, diff --git a/packages/mcp-cloudflare/src/server/oauth/routes/callback.ts b/packages/mcp-cloudflare/src/server/oauth/routes/callback.ts index 23458361..a3c0103f 100644 --- a/packages/mcp-cloudflare/src/server/oauth/routes/callback.ts +++ b/packages/mcp-cloudflare/src/server/oauth/routes/callback.ts @@ -6,7 +6,7 @@ import { SENTRY_TOKEN_URL } from "../constants"; import { exchangeCodeForAccessToken } from "../helpers"; import { verifyAndParseState, type OAuthState } from "../state"; import { logWarn } from "@sentry/mcp-core/telem/logging"; -import { parseSkills, getScopesForSkills } from "@sentry/mcp-core/skills"; +import { parseSkills } from "@sentry/mcp-core/skills"; /** * Extended AuthRequest that includes skills @@ -151,9 +151,6 @@ export default new Hono<{ Bindings: Env }>().get("/", async (c) => { ); } - // Calculate Sentry API scopes from validated skills - const grantedScopes = await getScopesForSkills(validSkills); - // Convert valid skills Set to array for OAuth props const grantedSkills = Array.from(validSkills); @@ -178,9 +175,6 @@ export default new Hono<{ Bindings: Env }>().get("/", async (c) => { accessTokenExpiresAt: Date.now() + payload.expires_in * 1000, clientId: oauthReqInfo.clientId, scope: oauthReqInfo.scope.join(" "), - // Scopes derived from skills - for backward compatibility with old MCP clients - // that don't support grantedSkills and only understand grantedScopes - grantedScopes: Array.from(grantedScopes), grantedSkills, // Primary authorization method // Note: constraints are NOT included here - they're extracted per-request from URL diff --git a/packages/mcp-cloudflare/src/server/types.ts b/packages/mcp-cloudflare/src/server/types.ts index e11a0469..915e245e 100644 --- a/packages/mcp-cloudflare/src/server/types.ts +++ b/packages/mcp-cloudflare/src/server/types.ts @@ -20,10 +20,14 @@ export type WorkerProps = { accessTokenExpiresAt?: number; // Timestamp when the upstream access token expires clientId: string; scope: string; - // Scopes derived from skills - for backward compatibility with old MCP clients - // that don't support grantedSkills and only understand grantedScopes + /** + * @deprecated grantedScopes is deprecated and will be removed on Jan 1, 2026. + * Use grantedSkills instead. Skills are the primary authorization method. + * This field exists only for backward compatibility during the transition period. + */ grantedScopes?: string[]; - grantedSkills?: string[]; // Array of skill strings (primary authorization method) + /** Primary authorization method - array of skill strings */ + grantedSkills?: string[]; // Note: constraints are NOT included - they're extracted per-request from URL // Note: sentryHost and mcpUrl come from env, not OAuth props diff --git a/packages/mcp-core/src/server.ts b/packages/mcp-core/src/server.ts index 299cdbf7..7f932244 100644 --- a/packages/mcp-core/src/server.ts +++ b/packages/mcp-core/src/server.ts @@ -37,7 +37,6 @@ import { logIssue, type LogIssueOptions } from "./telem/logging"; import { formatErrorForUser } from "./internal/error-handling"; import { LIB_VERSION } from "./version"; import { MCP_SERVER_NAME } from "./constants"; -import { isToolAllowed, type Scope } from "./permissions"; import { hasRequiredSkills, type Skill } from "./skills"; import { getConstraintParametersToInject, @@ -47,7 +46,7 @@ import { /** * Creates and configures a complete MCP server with Sentry instrumentation. * - * The server is built with tools filtered based on the granted scopes in the context. + * The server is built with tools filtered based on the granted skills in the context. * Context is captured in tool handler closures and passed directly to handlers. * * @example Usage with stdio transport @@ -105,11 +104,11 @@ export function buildServer({ } /** - * Configures an MCP server with tools filtered by granted skills or scopes. + * Configures an MCP server with tools filtered by granted skills. * * Internal function used by buildServer(). Use buildServer() instead for most cases. - * Tools are filtered at registration time based on grantedSkills OR grantedScopes - * (either system can grant access), and context is captured in closures for tool handler execution. + * Tools are filtered at registration time based on grantedSkills, and context is + * captured in closures for tool handler execution. * * In agent mode, only the use_sentry tool is registered, bypassing authorization checks. */ @@ -132,15 +131,11 @@ function configureServer({ ? { use_sentry: tools.use_sentry } : (customTools ?? tools); - // Get granted skills and scopes from context for tool filtering + // Get granted skills from context for tool filtering const grantedSkills: Set | undefined = context.grantedSkills ? new Set(context.grantedSkills) : undefined; - const grantedScopes: Set | undefined = context.grantedScopes - ? new Set(context.grantedScopes) - : undefined; - server.server.onerror = (error) => { const transportLogOptions: LogIssueOptions = { loggerScope: ["server", "transport"] as const, @@ -156,40 +151,25 @@ function configureServer({ for (const [toolKey, tool] of Object.entries(toolsToRegister)) { /** - * Authorization System Precedence - * ================================ - * - * The server supports two authorization systems: - * 1. **Skills System (NEW)** - User-facing permission groups (inspect, triage, etc.) - * 2. **Scopes System (LEGACY)** - Low-level API permissions (event:read, project:write, etc.) + * Skills-Based Authorization + * ========================== * - * IMPORTANT: These systems are **MUTUALLY EXCLUSIVE** - only one is active per session: + * Tools are filtered at registration time based on grantedSkills. + * Tool must have non-empty `requiredSkills` array to be exposed. + * Empty `requiredSkills: []` means intentionally excluded from skills system. * - * ## Skills Mode (when grantedSkills is set): - * - ONLY skills are checked (scopes are ignored) - * - Tool must have non-empty `requiredSkills` array to be exposed - * - Empty `requiredSkills: []` means intentionally excluded from skills system - * - Authorization: `allowed = hasRequiredSkills(grantedSkills, tool.requiredSkills)` - * - * ## Scopes Mode (when grantedSkills is NOT set, but grantedScopes is set): - * - Falls back to legacy scope checking - * - Empty `requiredScopes: []` means no scopes required (always allowed) - * - Authorization: `allowed = isToolAllowed(tool.requiredScopes, grantedScopes)` - * - * ## Tool Visibility: - * - If not allowed by active authorization system: tool is NOT registered - * - Only registered tools are visible to MCP clients + * In agent mode, authorization is skipped - use_sentry handles it internally. * * ## Examples: * ```typescript * // Tool available in "triage" skill only: - * { requiredSkills: ["triage"], requiredScopes: ["event:write"] } + * { requiredSkills: ["triage"] } * * // Tool available to ALL skills (foundational tool like whoami): - * { requiredSkills: ALL_SKILLS, requiredScopes: [] } + * { requiredSkills: ALL_SKILLS } * * // Tool excluded from skills system (like use_sentry in agent mode): - * { requiredSkills: [], requiredScopes: [] } + * { requiredSkills: [] } * ``` */ let allowed = false; @@ -198,19 +178,13 @@ function configureServer({ if (agentMode) { allowed = true; } - // Skills system takes precedence when set + // Skills system: tool must have non-empty requiredSkills to be exposed else if (grantedSkills) { - // Tool must have non-empty requiredSkills to be exposed in skills mode if (tool.requiredSkills && tool.requiredSkills.length > 0) { allowed = hasRequiredSkills(grantedSkills, tool.requiredSkills); } // Empty requiredSkills means NOT exposed via skills system } - // Legacy fallback: Check scopes if not using skills - else if (grantedScopes) { - // isToolAllowed handles empty requiredScopes correctly (returns true) - allowed = isToolAllowed(tool.requiredScopes, grantedScopes); - } // Skip tool if not allowed by active authorization system if (!allowed) { diff --git a/packages/mcp-core/src/test-utils/context.ts b/packages/mcp-core/src/test-utils/context.ts index f9540fe8..8a21a344 100644 --- a/packages/mcp-core/src/test-utils/context.ts +++ b/packages/mcp-core/src/test-utils/context.ts @@ -1,5 +1,5 @@ import type { ServerContext } from "../types"; -import type { Scope } from "../permissions"; +import { SKILLS, type Skill } from "../skills"; /** * Create a test context with default values for testing tools @@ -7,15 +7,12 @@ import type { Scope } from "../permissions"; export function createTestContext( overrides: Partial = {}, ): ServerContext { + // Default to all skills for testing + const allSkills = Object.keys(SKILLS) as Skill[]; return { accessToken: "test-access-token", constraints: {}, - grantedScopes: new Set([ - "org:read", - "project:write", - "team:write", - "event:write", - ]), + grantedSkills: new Set(allSkills), ...overrides, }; } diff --git a/packages/mcp-core/src/tools/use-sentry/handler.test.ts b/packages/mcp-core/src/tools/use-sentry/handler.test.ts index 4b212b6a..bebd4c82 100644 --- a/packages/mcp-core/src/tools/use-sentry/handler.test.ts +++ b/packages/mcp-core/src/tools/use-sentry/handler.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect, vi, beforeEach, type Mock } from "vitest"; import useSentry from "./handler"; import type { ServerContext } from "../../types"; -import type { Scope } from "../../permissions"; import type { Skill } from "../../skills"; // Mock the embedded agent @@ -13,19 +12,6 @@ vi.mock("./agent", () => ({ import { useSentryAgent } from "./agent"; const mockUseSentryAgent = useSentryAgent as Mock; -// Use all scopes for testing to ensure all tools are available -const ALL_SCOPES: Scope[] = [ - "org:read", - "org:write", - "project:read", - "project:write", - "team:read", - "team:write", - "event:read", - "event:write", - "project:releases", -]; - // Use all skills for testing to ensure all tools are available const ALL_SKILLS: Skill[] = [ "inspect", @@ -41,7 +27,6 @@ const mockContext: ServerContext = { userId: "1", clientId: "test-client", constraints: {}, - grantedScopes: new Set(ALL_SCOPES), grantedSkills: new Set(ALL_SKILLS), }; diff --git a/packages/mcp-core/src/tools/use-sentry/tool-wrapper.test.ts b/packages/mcp-core/src/tools/use-sentry/tool-wrapper.test.ts index a2ab7d53..b22b7d65 100644 --- a/packages/mcp-core/src/tools/use-sentry/tool-wrapper.test.ts +++ b/packages/mcp-core/src/tools/use-sentry/tool-wrapper.test.ts @@ -41,7 +41,6 @@ describe("wrapToolForAgent", () => { userId: "1", clientId: "test-client", constraints: {}, - grantedScopes: new Set([]), }; const wrappedTool = wrapToolForAgent(mockTool, { context }); @@ -72,7 +71,6 @@ describe("wrapToolForAgent", () => { constraints: { organizationSlug: "constrained-org", }, - grantedScopes: new Set([]), }; const wrappedTool = wrapToolForAgent(mockTool, { context }); @@ -102,7 +100,6 @@ describe("wrapToolForAgent", () => { organizationSlug: "constrained-org", projectSlug: "constrained-project", }, - grantedScopes: new Set([]), }; const wrappedTool = wrapToolForAgent(mockTool, { context }); @@ -132,7 +129,6 @@ describe("wrapToolForAgent", () => { constraints: { organizationSlug: "constrained-org", }, - grantedScopes: new Set([]), }; const wrappedTool = wrapToolForAgent(mockTool, { context }); @@ -176,7 +172,6 @@ describe("wrapToolForAgent", () => { userId: "1", clientId: "test-client", constraints: {}, - grantedScopes: new Set([]), }; const wrappedTool = wrapToolForAgent(errorTool, { context }); diff --git a/packages/mcp-core/src/types.ts b/packages/mcp-core/src/types.ts index 0261ccd6..4b8b8586 100644 --- a/packages/mcp-core/src/types.ts +++ b/packages/mcp-core/src/types.ts @@ -5,7 +5,6 @@ * and server context. Uses advanced TypeScript patterns for type-safe parameter * extraction and handler registration. */ -import type { Scope } from "./permissions"; import type { Skill } from "./skills"; /** @@ -35,11 +34,8 @@ export type ServerContext = { openaiBaseUrl?: string; userId?: string | null; clientId?: string; - // Granted skills for tool access control (user-facing capabilities, primary authorization method) + /** Primary authorization method - granted skills for tool access control */ grantedSkills?: Set | ReadonlySet; - // Granted scopes derived from skills - for backward compatibility with old MCP clients - // that don't support grantedSkills and only understand grantedScopes - grantedScopes?: Set | ReadonlySet; // URL-based session constraints constraints: Constraints; }; diff --git a/packages/mcp-server-evals/src/bin/start-mock-stdio.ts b/packages/mcp-server-evals/src/bin/start-mock-stdio.ts index 69e225c7..b8e496f6 100644 --- a/packages/mcp-server-evals/src/bin/start-mock-stdio.ts +++ b/packages/mcp-server-evals/src/bin/start-mock-stdio.ts @@ -3,8 +3,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { startStdio } from "@sentry/mcp-server/transports/stdio"; import { mswServer } from "@sentry/mcp-server-mocks"; -import type { Scope } from "@sentry/mcp-core/permissions"; -import { ALL_SCOPES } from "@sentry/mcp-core/permissions"; +import { SKILLS, type Skill } from "@sentry/mcp-core/skills"; mswServer.listen({ onUnhandledRequest: (req, print) => { @@ -20,17 +19,18 @@ mswServer.listen({ const accessToken = "mocked-access-token"; -// Grant all available scopes for evals to ensure MSW mocks apply broadly +// Grant all available skills for evals to ensure MSW mocks apply broadly +const allSkills = Object.keys(SKILLS) as Skill[]; const server = new McpServer({ name: "Sentry MCP", version: "0.1.0", }); -// Run in-process MCP with all scopes so MSW mocks apply +// Run in-process MCP with all skills so MSW mocks apply startStdio(server, { accessToken, - grantedScopes: new Set(ALL_SCOPES), + grantedSkills: new Set(allSkills), constraints: { organizationSlug: null, projectSlug: null, diff --git a/packages/mcp-server/src/cli/parse.test.ts b/packages/mcp-server/src/cli/parse.test.ts index 80e1d099..46fa5ab1 100644 --- a/packages/mcp-server/src/cli/parse.test.ts +++ b/packages/mcp-server/src/cli/parse.test.ts @@ -10,9 +10,7 @@ describe("cli/parseArgv", () => { "--mcp-url=https://mcp.example.com", "--sentry-dsn=dsn", "--openai-base-url=https://api.example.com/v1", - "--scopes=org:read", - "--add-scopes=event:write", - "--all-scopes", + "--skills=inspect,triage", "-h", "-v", ]); @@ -22,9 +20,7 @@ describe("cli/parseArgv", () => { expect(parsed.mcpUrl).toBe("https://mcp.example.com"); expect(parsed.sentryDsn).toBe("dsn"); expect(parsed.openaiBaseUrl).toBe("https://api.example.com/v1"); - expect(parsed.scopes).toBe("org:read"); - expect(parsed.addScopes).toBe("event:write"); - expect(parsed.allScopes).toBe(true); + expect(parsed.skills).toBe("inspect,triage"); expect(parsed.help).toBe(true); expect(parsed.version).toBe(true); expect(parsed.unknownArgs).toEqual([]); @@ -49,16 +45,12 @@ describe("cli/parseEnv", () => { SENTRY_HOST: "envhost", MCP_URL: "envmcp", SENTRY_DSN: "envdsn", - MCP_SCOPES: "org:read", - MCP_ADD_SCOPES: "event:write", MCP_SKILLS: "inspect,triage", } as any); expect(env.accessToken).toBe("envtok"); expect(env.host).toBe("envhost"); expect(env.mcpUrl).toBe("envmcp"); expect(env.sentryDsn).toBe("envdsn"); - expect(env.scopes).toBe("org:read"); - expect(env.addScopes).toBe("event:write"); expect(env.skills).toBe("inspect,triage"); }); }); @@ -70,8 +62,6 @@ describe("cli/merge", () => { SENTRY_HOST: "envhost", MCP_URL: "envmcp", SENTRY_DSN: "envdsn", - MCP_SCOPES: "org:read", - MCP_ADD_SCOPES: "event:write", } as any); const cli = parseArgv([ "--access-token=clitok", @@ -79,8 +69,6 @@ describe("cli/merge", () => { "--mcp-url=climcp", "--sentry-dsn=clidsn", "--openai-base-url=https://api.cli/v1", - "--scopes=org:admin", - "--add-scopes=project:write", ]); const merged = merge(cli, env); expect(merged.accessToken).toBe("clitok"); @@ -88,8 +76,6 @@ describe("cli/merge", () => { expect(merged.mcpUrl).toBe("climcp"); expect(merged.sentryDsn).toBe("clidsn"); expect(merged.openaiBaseUrl).toBe("https://api.cli/v1"); - expect(merged.scopes).toBe("org:admin"); - expect(merged.addScopes).toBe("project:write"); }); it("applies precedence for skills: CLI over env", () => { diff --git a/packages/mcp-server/src/cli/parse.ts b/packages/mcp-server/src/cli/parse.ts index b194de91..499e5163 100644 --- a/packages/mcp-server/src/cli/parse.ts +++ b/packages/mcp-server/src/cli/parse.ts @@ -12,9 +12,6 @@ export function parseArgv(argv: string[]): CliArgs { "openai-model": { type: "string" as const }, "organization-slug": { type: "string" as const }, "project-slug": { type: "string" as const }, - scopes: { type: "string" as const }, - "add-scopes": { type: "string" as const }, - "all-scopes": { type: "boolean" as const }, skills: { type: "string" as const }, agent: { type: "boolean" as const }, help: { type: "boolean" as const, short: "h" as const }, @@ -58,9 +55,6 @@ export function parseArgv(argv: string[]): CliArgs { openaiModel: values["openai-model"] as string | undefined, organizationSlug: values["organization-slug"] as string | undefined, projectSlug: values["project-slug"] as string | undefined, - scopes: values.scopes as string | undefined, - addScopes: values["add-scopes"] as string | undefined, - allScopes: (values["all-scopes"] as boolean | undefined) === true, skills: values.skills as string | undefined, agent: (values.agent as boolean | undefined) === true, help: (values.help as boolean | undefined) === true, @@ -80,31 +74,13 @@ export function parseEnv(env: NodeJS.ProcessEnv): EnvArgs { fromEnv.sentryDsn = env.SENTRY_DSN || env.DEFAULT_SENTRY_DSN; if (env.OPENAI_MODEL) fromEnv.openaiModel = env.OPENAI_MODEL; - - // LEGACY - deprecated environment variables - if (env.MCP_SCOPES) { - fromEnv.scopes = env.MCP_SCOPES; - console.warn("⚠️ Warning: MCP_SCOPES environment variable is deprecated."); - console.warn(" Consider using MCP_SKILLS instead."); - console.warn(""); - } - if (env.MCP_ADD_SCOPES) { - fromEnv.addScopes = env.MCP_ADD_SCOPES; - console.warn( - "⚠️ Warning: MCP_ADD_SCOPES environment variable is deprecated.", - ); - console.warn(" Consider using MCP_SKILLS instead."); - console.warn(""); - } - - // NEW - primary authorization method if (env.MCP_SKILLS) fromEnv.skills = env.MCP_SKILLS; return fromEnv; } export function merge(cli: CliArgs, env: EnvArgs): MergedArgs { // CLI wins over env - const merged: MergedArgs = { + return { accessToken: cli.accessToken ?? env.accessToken, // If CLI provided url/host, prefer those; else fall back to env url: cli.url ?? env.url, @@ -113,10 +89,6 @@ export function merge(cli: CliArgs, env: EnvArgs): MergedArgs { sentryDsn: cli.sentryDsn ?? env.sentryDsn, openaiBaseUrl: cli.openaiBaseUrl, openaiModel: cli.openaiModel ?? env.openaiModel, - // Scopes precedence: CLI scopes/add-scopes override their env counterparts - scopes: cli.scopes ?? env.scopes, - addScopes: cli.addScopes ?? env.addScopes, - allScopes: cli.allScopes === true, // Skills precedence: CLI skills override env skills: cli.skills ?? env.skills, agent: cli.agent === true, @@ -126,11 +98,4 @@ export function merge(cli: CliArgs, env: EnvArgs): MergedArgs { version: cli.version === true, unknownArgs: cli.unknownArgs, }; - - // If CLI provided scopes, ignore additive env var - if (cli.scopes) merged.addScopes = cli.addScopes; - // If CLI provided add-scopes, ensure scopes override isn't pulled from env - if (cli.addScopes) merged.scopes = cli.scopes; - - return merged; } diff --git a/packages/mcp-server/src/cli/resolve.test.ts b/packages/mcp-server/src/cli/resolve.test.ts index 3428c7dc..d7599ecd 100644 --- a/packages/mcp-server/src/cli/resolve.test.ts +++ b/packages/mcp-server/src/cli/resolve.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect } from "vitest"; import { finalize } from "./resolve"; -import { ALL_SCOPES } from "@sentry/mcp-core/permissions"; describe("cli/finalize", () => { it("throws on missing access token", () => { @@ -9,43 +8,6 @@ describe("cli/finalize", () => { ); }); - it("throws on invalid scopes", () => { - expect(() => - finalize({ accessToken: "tok", scopes: "foo", unknownArgs: [] }), - ).toThrow(/Invalid scopes provided: foo/); - }); - - it("expands implied scopes for --scopes", () => { - const cfg = finalize({ - accessToken: "tok", - scopes: "event:write", - unknownArgs: [], - }); - expect(cfg.finalScopes?.has("event:write")).toBe(true); - expect(cfg.finalScopes?.has("event:read")).toBe(true); - }); - - it("merges defaults for --add-scopes and expands", () => { - const cfg = finalize({ - accessToken: "tok", - addScopes: "project:write", - unknownArgs: [], - }); - expect(cfg.finalScopes?.has("project:write")).toBe(true); - // Defaults include project:read - expect(cfg.finalScopes?.has("project:read")).toBe(true); - }); - - it("grants all scopes with --all-scopes", () => { - const cfg = finalize({ - accessToken: "tok", - allScopes: true, - unknownArgs: [], - }); - expect(cfg.finalScopes?.size).toBe(ALL_SCOPES.length); - expect(cfg.finalScopes?.has("org:admin")).toBe(true); - }); - it("normalizes host from URL", () => { const cfg = finalize({ accessToken: "tok", diff --git a/packages/mcp-server/src/cli/resolve.ts b/packages/mcp-server/src/cli/resolve.ts index 38ad23bb..0eb5b318 100644 --- a/packages/mcp-server/src/cli/resolve.ts +++ b/packages/mcp-server/src/cli/resolve.ts @@ -1,11 +1,4 @@ -import { - ALL_SCOPES, - parseScopes, - resolveScopes, - type Scope, -} from "@sentry/mcp-core/permissions"; import { parseSkills, SKILLS, type Skill } from "@sentry/mcp-core/skills"; -import { DEFAULT_SCOPES } from "@sentry/mcp-core/constants"; import { validateAndParseSentryUrlThrows, validateOpenAiBaseUrlThrows, @@ -13,11 +6,6 @@ import { } from "@sentry/mcp-core/utils/url-utils"; import type { MergedArgs, ResolvedConfig } from "./types"; -export function formatInvalid(invalid: string[], envName?: string): string { - const where = envName ? `${envName} provided` : "Invalid scopes provided"; - return `Error: ${where}: ${invalid.join(", ")}\nAvailable scopes: ${ALL_SCOPES.join(", ")}`; -} - export function formatInvalidSkills( invalid: string[], envName?: string, @@ -44,67 +32,6 @@ export function finalize(input: MergedArgs): ResolvedConfig { sentryHost = input.host; } - // Scopes resolution (LEGACY - deprecated in favor of skills) - let finalScopes: Set | undefined = undefined; - if (input.allScopes) { - console.warn( - "⚠️ Warning: --all-scopes is deprecated. Consider using skills instead:", - ); - console.warn(" --skills=inspect,triage,project-management,seer,docs"); - console.warn(" Learn more about skills in the documentation."); - console.warn(""); - finalScopes = new Set(ALL_SCOPES as ReadonlyArray); - } else if (input.scopes || input.addScopes) { - // Show deprecation warning - if (input.scopes) { - console.warn( - "⚠️ Warning: --scopes is deprecated. Consider using skills instead:", - ); - console.warn( - " For example, instead of --scopes=event:write,project:write", - ); - console.warn(" Use: --skills=triage,project-management"); - console.warn(" Learn more about skills in the documentation."); - console.warn(""); - } else if (input.addScopes) { - console.warn( - "⚠️ Warning: --add-scopes is deprecated. Consider using --skills instead:", - ); - console.warn(" For example, instead of --add-scopes=event:write"); - console.warn(" Use: --skills=triage"); - console.warn(" Learn more about skills in the documentation."); - console.warn(""); - } - - // Strict validation: any invalid token is an error - if (input.scopes) { - const { valid, invalid } = parseScopes(input.scopes); - if (invalid.length > 0) { - throw new Error(formatInvalid(invalid)); - } - if (valid.size === 0) { - throw new Error( - "Error: Invalid scopes provided. No valid scopes found.", - ); - } - finalScopes = resolveScopes({ - override: valid, - defaults: DEFAULT_SCOPES, - }); - } else if (input.addScopes) { - const { valid, invalid } = parseScopes(input.addScopes); - if (invalid.length > 0) { - throw new Error(formatInvalid(invalid)); - } - if (valid.size === 0) { - throw new Error( - "Error: Invalid additional scopes provided. No valid scopes found.", - ); - } - finalScopes = resolveScopes({ add: valid, defaults: DEFAULT_SCOPES }); - } - } - // Skills resolution // // IMPORTANT: stdio (CLI) intentionally defaults to ALL skills when no --skills flag is provided @@ -150,7 +77,6 @@ export function finalize(input: MergedArgs): ResolvedConfig { sentryDsn: input.sentryDsn, openaiBaseUrl: resolvedOpenAiBaseUrl, openaiModel: input.openaiModel, - finalScopes, finalSkills, organizationSlug: input.organizationSlug, projectSlug: input.projectSlug, diff --git a/packages/mcp-server/src/cli/types.ts b/packages/mcp-server/src/cli/types.ts index 60139577..082813af 100644 --- a/packages/mcp-server/src/cli/types.ts +++ b/packages/mcp-server/src/cli/types.ts @@ -1,4 +1,3 @@ -import type { Scope } from "@sentry/mcp-core/permissions"; import type { Skill } from "@sentry/mcp-core/skills"; export type CliArgs = { @@ -9,10 +8,7 @@ export type CliArgs = { sentryDsn?: string; openaiBaseUrl?: string; openaiModel?: string; - scopes?: string; // LEGACY - for backward compatibility - addScopes?: string; // LEGACY - for backward compatibility - allScopes?: boolean; // LEGACY - for backward compatibility - skills?: string; // NEW - primary authorization method + skills?: string; agent?: boolean; organizationSlug?: string; projectSlug?: string; @@ -28,9 +24,7 @@ export type EnvArgs = { mcpUrl?: string; sentryDsn?: string; openaiModel?: string; - scopes?: string; // LEGACY - for backward compatibility - addScopes?: string; // LEGACY - for backward compatibility - skills?: string; // NEW - primary authorization method + skills?: string; }; export type MergedArgs = { @@ -41,10 +35,7 @@ export type MergedArgs = { sentryDsn?: string; openaiBaseUrl?: string; openaiModel?: string; - scopes?: string; // LEGACY - for backward compatibility - addScopes?: string; // LEGACY - for backward compatibility - allScopes?: boolean; // LEGACY - for backward compatibility - skills?: string; // NEW - primary authorization method + skills?: string; agent?: boolean; organizationSlug?: string; projectSlug?: string; @@ -60,8 +51,8 @@ export type ResolvedConfig = { sentryDsn?: string; openaiBaseUrl?: string; openaiModel?: string; - finalScopes?: Set; // LEGACY - for backward compatibility - finalSkills?: Set; // NEW - primary authorization method + /** Primary authorization method */ + finalSkills?: Set; organizationSlug?: string; projectSlug?: string; }; diff --git a/packages/mcp-server/src/cli/usage.ts b/packages/mcp-server/src/cli/usage.ts index 1875e3e4..faf9b3a1 100644 --- a/packages/mcp-server/src/cli/usage.ts +++ b/packages/mcp-server/src/cli/usage.ts @@ -1,11 +1,7 @@ -import type { Scope } from "@sentry/mcp-core/permissions"; import type { Skill } from "@sentry/mcp-core/skills"; export function buildUsage( packageName: string, - defaultScopes: ReadonlyArray, - allScopes: ReadonlyArray, - defaultSkills: ReadonlyArray, allSkills: ReadonlyArray, ): string { return `Usage: ${packageName} --access-token= [--host=] @@ -24,19 +20,11 @@ Session constraints: --organization-slug Force all calls to an organization --project-slug Optional project constraint -Skill controls (recommended): +Skill controls: --skills Specify which skills to grant (default: all skills) All skills: ${allSkills.join(", ")} -Scope controls (legacy - deprecated, use skills instead): - --scopes Override default scopes - --add-scopes Add scopes to defaults - --all-scopes Grant every available scope - -Default scopes: ${defaultScopes.join(", ")} -All scopes: ${allScopes.join(", ")} - Examples: ${packageName} --access-token=TOKEN ${packageName} --access-token=TOKEN --skills=inspect,triage diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index ecd2c6cb..cc4ee7c8 100644 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -22,8 +22,6 @@ import { buildUsage } from "./cli/usage"; import { parseArgv, parseEnv, merge } from "./cli/parse"; import { finalize } from "./cli/resolve"; import { sentryBeforeSend } from "@sentry/mcp-core/telem/sentry"; -import { ALL_SCOPES } from "@sentry/mcp-core/permissions"; -import { DEFAULT_SCOPES, DEFAULT_SKILLS } from "@sentry/mcp-core/constants"; import { SKILLS } from "@sentry/mcp-core/skills"; import { setOpenAIBaseUrl } from "@sentry/mcp-core/internal/agents/openai-provider"; @@ -31,13 +29,7 @@ const packageName = "@sentry/mcp-server"; const allSkills = Object.keys(SKILLS) as ReadonlyArray< (typeof SKILLS)[keyof typeof SKILLS]["id"] >; -const usageText = buildUsage( - packageName, - DEFAULT_SCOPES, - ALL_SCOPES, - DEFAULT_SKILLS, - allSkills, -); +const usageText = buildUsage(packageName, allSkills); function die(message: string): never { console.error(message); @@ -129,7 +121,6 @@ const SENTRY_TIMEOUT = 5000; // 5 seconds // Build context once for server configuration and runtime const context = { accessToken: cfg.accessToken, - grantedScopes: cfg.finalScopes, grantedSkills: cfg.finalSkills, constraints: { organizationSlug: cfg.organizationSlug ?? null, @@ -140,7 +131,7 @@ const context = { openaiBaseUrl: cfg.openaiBaseUrl, }; -// Build server with context to filter tools based on granted scopes +// Build server with context to filter tools based on granted skills // Use agentMode when --agent flag is set (only exposes use_sentry tool) const server = buildServer({ context,