diff --git a/docs/cloudflare/oauth-architecture.md b/docs/cloudflare/oauth-architecture.md index bf05b71a7..a52da228f 100644 --- a/docs/cloudflare/oauth-architecture.md +++ b/docs/cloudflare/oauth-architecture.md @@ -243,6 +243,7 @@ interface WorkerProps { scope: string; // MCP permissions granted grantedSkills?: string[]; // Skills granted (primary authorization) // grantedScopes is deprecated and will be removed Jan 1, 2026 + upstreamScopes?: string[]; // Scopes Sentry granted on the upstream token } ``` diff --git a/docs/operations/security.md b/docs/operations/security.md index 4ff6e78c2..670ce6120 100644 --- a/docs/operations/security.md +++ b/docs/operations/security.md @@ -104,6 +104,10 @@ Today, runtime authorization for remote HTTP sessions is driven primarily by: `grantedScopes` still exists on tokens for backward compatibility with older clients, but it is transitional. Skills are the primary authorization model. +Note that `grantedScopes` is derived from the selected skills, not from what +Sentry granted. The scopes Sentry actually granted on the upstream token (from +the `scope` field of Sentry's token response) are stored separately as +`upstreamScopes`. Grants created before `upstreamScopes` existed do not have it. Direct `Sentry-Bearer` sessions default to all active skills and can be narrowed with `skills` or `disable-skills` query parameters. They do not get OAuth diff --git a/packages/mcp-cloudflare/src/server/oauth/callback.test.ts b/packages/mcp-cloudflare/src/server/oauth/callback.test.ts index dd69a614a..9f2fec656 100644 --- a/packages/mcp-cloudflare/src/server/oauth/callback.test.ts +++ b/packages/mcp-cloudflare/src/server/oauth/callback.test.ts @@ -664,6 +664,82 @@ describe("oauth callback routes", () => { expect(body.result?.protocolVersion).toBeDefined(); }); + it("stores the scopes Sentry actually granted alongside skill-derived grantedScopes", async () => { + // Sentry returns the token's real scopes in the token response. With + // only `inspect` selected, skill-derived grantedScopes are read-only, + // but the upstream token can still carry write scopes. + exchangeCodeForAccessToken.mockResolvedValueOnce([ + { + access_token: "sentry-access-token", + refresh_token: "sentry-refresh-token", + expires_in: 3600, + expires_at: new Date(Date.now() + 3600_000).toISOString(), + token_type: "bearer", + user: { id: "user-123", name: "Test User" }, + scope: "team:write org:read event:write project:write alerts:write", + }, + null, + ]); + + const testEnv = createTestEnv(); + const completeAuthorization = vi.spyOn( + testEnv.OAUTH_PROVIDER, + "completeAuthorization", + ); + const oauthApp = createTestApp(); + const client = await createClient(testEnv); + const cookie = await approveClient(oauthApp, testEnv, client.clientId); + const state = await createSignedCallbackState(client.clientId); + + const callbackResponse = await callCallback(oauthApp, testEnv, { + state, + code: "test-code", + cookie, + }); + + expect(callbackResponse.status).toBe(302); + expect(completeAuthorization).toHaveBeenCalledOnce(); + const props = completeAuthorization.mock.calls[0]![0].props as Record< + string, + unknown + >; + + expect(props.upstreamScopes).toEqual([ + "alerts:write", + "event:write", + "org:read", + "project:write", + "team:write", + ]); + expect(props.grantedSkills).toEqual(["inspect"]); + expect(props.grantedScopes).not.toEqual(props.upstreamScopes); + }); + + it("omits upstreamScopes when Sentry's token response has no scope", async () => { + const testEnv = createTestEnv(); + const completeAuthorization = vi.spyOn( + testEnv.OAUTH_PROVIDER, + "completeAuthorization", + ); + const oauthApp = createTestApp(); + const client = await createClient(testEnv); + const cookie = await approveClient(oauthApp, testEnv, client.clientId); + const state = await createSignedCallbackState(client.clientId); + + const callbackResponse = await callCallback(oauthApp, testEnv, { + state, + code: "test-code", + cookie, + }); + + expect(callbackResponse.status).toBe(302); + const props = completeAuthorization.mock.calls[0]![0].props as Record< + string, + unknown + >; + expect(props.upstreamScopes).toBeUndefined(); + }); + it("completes callback for a scoped resource, exchanges the code, and uses the token at the same scoped /mcp path", async () => { const scopedResource = "http://localhost/mcp/sentry-mcp-evals/cloudflare-mcp"; diff --git a/packages/mcp-cloudflare/src/server/oauth/helpers.ts b/packages/mcp-cloudflare/src/server/oauth/helpers.ts index 41e3e85b2..c75ef446e 100644 --- a/packages/mcp-cloudflare/src/server/oauth/helpers.ts +++ b/packages/mcp-cloudflare/src/server/oauth/helpers.ts @@ -588,6 +588,7 @@ export async function tokenExchangeCallback( clientId: rawProps.clientId as string, scope: rawProps.scope as string, grantedScopes: rawProps.grantedScopes, + upstreamScopes: rawProps.upstreamScopes, grantedSkills: rawProps.grantedSkills, } satisfies WorkerProps & Record; diff --git a/packages/mcp-cloudflare/src/server/oauth/routes/callback.ts b/packages/mcp-cloudflare/src/server/oauth/routes/callback.ts index 806af7539..0a7698769 100644 --- a/packages/mcp-cloudflare/src/server/oauth/routes/callback.ts +++ b/packages/mcp-cloudflare/src/server/oauth/routes/callback.ts @@ -42,6 +42,18 @@ function getUpstreamTokenExpiryTimestamp(payload: { : Date.now() + payload.expires_in * 1000; } +/** + * Parse the space-delimited `scope` from Sentry's token response (RFC 6749 + * ยง5.1) into a sorted list. Returns undefined when Sentry omitted it so we + * don't record an empty list that looks like "no scopes granted". + */ +function parseUpstreamScopes(scope: string | undefined): string[] | undefined { + if (typeof scope !== "string") { + return undefined; + } + return Array.from(new Set(scope.split(/\s+/).filter(Boolean))).sort(); +} + function getUpstreamUserLabel(payload: { user: { name: string | null; email?: string | null; id: string }; }): string { @@ -323,6 +335,9 @@ export default new Hono<{ Bindings: Env }>().get("/", async (c) => { // Scopes derived from skills - for backward compatibility with old MCP clients // that don't support grantedSkills and only understand grantedScopes grantedScopes: Array.from(grantedScopes), + // Scopes Sentry actually granted on the upstream token (from the token + // response), which can differ from the skill-derived grantedScopes + upstreamScopes: parseUpstreamScopes(payload.scope), grantedSkills, // Primary authorization method constraintOrganizationSlug, diff --git a/packages/mcp-cloudflare/src/server/types.ts b/packages/mcp-cloudflare/src/server/types.ts index 2a91d6816..5d493089e 100644 --- a/packages/mcp-cloudflare/src/server/types.ts +++ b/packages/mcp-cloudflare/src/server/types.ts @@ -30,6 +30,14 @@ export type WorkerProps = { * This field exists only for backward compatibility during the transition period. */ grantedScopes?: string[]; + /** + * Scopes Sentry actually granted on the upstream access token, as returned + * in the `scope` field of Sentry's OAuth token response. Unlike + * `grantedScopes` (derived from selected skills), this reflects what the + * stored Sentry token can really do. Absent on grants created before this + * field existed. + */ + upstreamScopes?: string[]; /** Primary authorization method - array of skill strings */ grantedSkills?: string[];