diff --git a/packages/opencode/test/mcp/oauth-scope-merge.test.ts b/packages/opencode/test/mcp/oauth-scope-merge.test.ts new file mode 100644 index 000000000000..5819ed10fe8c --- /dev/null +++ b/packages/opencode/test/mcp/oauth-scope-merge.test.ts @@ -0,0 +1,125 @@ +import { test, expect, describe } from "bun:test" +import { createRequire } from "node:module" + +// Other mcp tests `mock.module("@modelcontextprotocol/sdk/client/auth.js", ...)` +// (those mocks only export UnauthorizedError) and bun leaks that across files in +// the suite. Import the REAL module by resolved absolute path — the mock is keyed +// on the bare specifier and does not intercept an absolute-path import. +const require = createRequire(import.meta.url) +const { auth } = await import(require.resolve("@modelcontextprotocol/sdk/client/auth.js")) + +// These tests pin the patched SDK scope-selection behavior (see +// patches/@modelcontextprotocol%2Fsdk@1.29.0.patch): the config `scope` from +// clientMetadata must be MERGED on top of the server-advertised resource +// scopes, not used only as a last-resort fallback. This is what lets +// `offline_access` (a client<->AS scope a resource server never advertises) be +// requested so the AS issues a refresh token. See opencode issue #34034. + +const SERVER_URL = "https://mcp.example.com/mcp" +const AS_URL = "https://as.example.com/" +const AUTHORIZE = "https://as.example.com/authorize" + +// Drive the real SDK auth() flow with no network: returning discoveryState() +// makes authInternal skip RFC 9728 discovery, and returning clientInformation() +// skips dynamic client registration. The flow reaches startAuthorization() and +// hands the final authorization URL to redirectToAuthorization(), which we +// capture to inspect the requested `scope`. +async function capturedAuthorizationScope(opts: { + advertisedScopes?: string[] + configScope?: string +}): Promise { + const captured: { url?: URL } = {} + + const provider = { + get redirectUrl() { + return "http://127.0.0.1:19876/mcp/oauth/callback" + }, + get clientMetadata() { + return { + redirect_uris: ["http://127.0.0.1:19876/mcp/oauth/callback"], + client_name: "OpenCode", + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + ...(opts.configScope ? { scope: opts.configScope } : {}), + } + }, + async clientInformation() { + return { client_id: "test-client" } + }, + async tokens() { + return undefined + }, + async state() { + return "state-123" + }, + async discoveryState() { + return { + authorizationServerUrl: AS_URL, + resourceMetadata: { + resource: "https://mcp.example.com/", + ...(opts.advertisedScopes ? { scopes_supported: opts.advertisedScopes } : {}), + }, + authorizationServerMetadata: { + issuer: AS_URL, + authorization_endpoint: AUTHORIZE, + token_endpoint: "https://as.example.com/token", + response_types_supported: ["code"], + code_challenge_methods_supported: ["S256"], + }, + } + }, + // Skip the `resource` param + RFC 8707 compatibility check for the test. + async validateResourceURL() { + return undefined + }, + async saveCodeVerifier() {}, + async saveClientInformation() {}, + async saveTokens() {}, + async redirectToAuthorization(url: URL) { + captured.url = url + }, + } + + const result = await auth(provider, { serverUrl: SERVER_URL }) + expect(result).toBe("REDIRECT") + return captured.url?.searchParams.get("scope") ?? null +} + +const asSet = (scope: string | null) => new Set((scope ?? "").split(/\s+/).filter(Boolean)) + +describe("MCP SDK scope selection (patched)", () => { + test("merges config scope on top of advertised resource scopes", async () => { + const scope = await capturedAuthorizationScope({ + advertisedScopes: ["mcp:read", "mcp:write"], + configScope: "openid offline_access mcp:read mcp:write", + }) + + expect(asSet(scope)).toEqual(new Set(["mcp:read", "mcp:write", "openid", "offline_access"])) + }) + + test("adds offline_access when config lists only the extra scope", async () => { + const scope = await capturedAuthorizationScope({ + advertisedScopes: ["mcp:read"], + configScope: "offline_access", + }) + + expect(asSet(scope)).toEqual(new Set(["mcp:read", "offline_access"])) + }) + + test("leaves advertised scopes untouched when no config scope is set", async () => { + const scope = await capturedAuthorizationScope({ + advertisedScopes: ["mcp:read", "mcp:write"], + }) + + expect(asSet(scope)).toEqual(new Set(["mcp:read", "mcp:write"])) + }) + + test("falls back to config scope when server advertises none (#28810)", async () => { + const scope = await capturedAuthorizationScope({ + configScope: "openid offline_access", + }) + + expect(asSet(scope)).toEqual(new Set(["openid", "offline_access"])) + }) +}) diff --git a/patches/@modelcontextprotocol%2Fsdk@1.29.0.patch b/patches/@modelcontextprotocol%2Fsdk@1.29.0.patch index ae25202873b4..0a465f424b92 100644 --- a/patches/@modelcontextprotocol%2Fsdk@1.29.0.patch +++ b/patches/@modelcontextprotocol%2Fsdk@1.29.0.patch @@ -1,5 +1,36 @@ +diff --git a/dist/cjs/client/auth.js b/dist/cjs/client/auth.js +index c2e4fa91d26f5336889f6afa416147db75fc4872..179a6152ab5d354ac7e5275b8cb50d4e9330ede3 100644 +--- a/dist/cjs/client/auth.js ++++ b/dist/cjs/client/auth.js +@@ -241,12 +241,21 @@ async function authInternal(provider, { serverUrl, authorizationCode, scope, res + }); + } + const resource = await selectResourceURL(serverUrl, provider, resourceMetadata); +- // Apply scope selection strategy (SEP-835): +- // 1. WWW-Authenticate scope (passed via `scope` param) +- // 2. PRM scopes_supported +- // 3. Client metadata scope (user-configured fallback) ++ // Apply scope selection strategy (SEP-835 + opencode #34034): ++ // Server-advertised resource scopes (WWW-Authenticate, then PRM ++ // scopes_supported) are MERGED with the user-configured ++ // clientMetadata.scope instead of the config scope being a last-resort ++ // fallback. This lets client<->AS scopes like `offline_access` (which a ++ // resource server never advertises) be requested on top of the resource ++ // scopes, so the AS issues a refresh token. Order preserved, deduped. + // The resolved scope is used consistently for both DCR and the authorization request. +- const resolvedScope = scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope; ++ const advertisedScope = scope || resourceMetadata?.scopes_supported?.join(' '); ++ const resolvedScope = [advertisedScope, provider.clientMetadata.scope] ++ .filter(Boolean) ++ .flatMap((s) => s.split(/\s+/)) ++ .filter(Boolean) ++ .filter((value, index, all) => all.indexOf(value) === index) ++ .join(' ') || undefined; + // Handle client registration if needed + let clientInformation = await Promise.resolve(provider.clientInformation()); + if (!clientInformation) { diff --git a/dist/cjs/client/index.d.ts b/dist/cjs/client/index.d.ts -index 1822bf749aec71d2bb295083d832114ee187bb67..58b859a7b32222fb5cb9f2011fdc5d010f3d05fb 100644 +index 6f567a193626587a2730b5a49293ca5dfd4181ea..5b7c841c000508e389ce617f559f7c2a5126ca9f 100644 --- a/dist/cjs/client/index.d.ts +++ b/dist/cjs/client/index.d.ts @@ -428,6 +428,8 @@ export declare class Client>; -+ callTool(params: CallToolRequest['params'], resultSchema: T, options?: RequestOptions): Promise>; - callTool(params: CallToolRequest['params'], resultSchema?: typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema, options?: RequestOptions): Promise<{ - [x: string]: unknown; - content: ({ -diff --git a/dist/esm/client/index.d.ts b/dist/esm/client/index.d.ts -index 1822bf749aec71d2bb295083d832114ee187bb67..58b859a7b32222fb5cb9f2011fdc5d010f3d05fb 100644 ---- a/dist/esm/client/index.d.ts -+++ b/dist/esm/client/index.d.ts -@@ -428,6 +428,8 @@ export declare class Client>; + callTool(params: CallToolRequest['params'], resultSchema: T, options?: RequestOptions): Promise>; callTool(params: CallToolRequest['params'], resultSchema?: typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema, options?: RequestOptions): Promise<{ [x: string]: unknown; content: ({ diff --git a/dist/cjs/client/index.js b/dist/cjs/client/index.js -index 6ac1da14dc7f6211ae70f7711c124b76098816d8..adb5b7bd45514a406a0f7e40b64631c101584c84 100644 +index 6ac1da14dc7f6211ae70f7711c124b76098816d8..88e58b90b673cb0f9c60920edec7d2ebb16f112b 100644 --- a/dist/cjs/client/index.js +++ b/dist/cjs/client/index.js @@ -288,41 +288,16 @@ class Client extends protocol_js_1.Protocol { @@ -113,7 +131,7 @@ index 6ac1da14dc7f6211ae70f7711c124b76098816d8..adb5b7bd45514a406a0f7e40b64631c1 * After initialization has completed, this will be populated with the server's reported capabilities. */ diff --git a/dist/cjs/client/streamableHttp.js b/dist/cjs/client/streamableHttp.js -index a29a7d3a0f14d9cd800ef5b296485237350c666f..c362ae5fe6c62c8c8eae7e2e61de1eedff5443c9 100644 +index a29a7d3a0f14d9cd800ef5b296485237350c666f..6c7cbbca4777def85c4bcfc573717c0685857bbb 100644 --- a/dist/cjs/client/streamableHttp.js +++ b/dist/cjs/client/streamableHttp.js @@ -290,7 +290,38 @@ class StreamableHTTPClientTransport { @@ -204,7 +222,7 @@ index a29a7d3a0f14d9cd800ef5b296485237350c666f..c362ae5fe6c62c8c8eae7e2e61de1eed } throw new StreamableHTTPError(response.status, `Error POSTing to endpoint: ${text}`); diff --git a/dist/cjs/shared/protocol.js b/dist/cjs/shared/protocol.js -index 3617e787f0ba70447c99501aee7aa67584d89758..4a96d6a0328fa348b96f3869ab7e0bb77538182b 100644 +index 3617e787f0ba70447c99501aee7aa67584d89758..4ee4d158391558fdc1f977f5134b7cacfc45e8c3 100644 --- a/dist/cjs/shared/protocol.js +++ b/dist/cjs/shared/protocol.js @@ -744,7 +744,12 @@ class Protocol { @@ -221,8 +239,52 @@ index 3617e787f0ba70447c99501aee7aa67584d89758..4a96d6a0328fa348b96f3869ab7e0bb7 this._cleanupTimeout(messageId); reject(error); }); +diff --git a/dist/esm/client/auth.js b/dist/esm/client/auth.js +index e183040fc2bba22ca1ccc784984f3310854403b7..fefc74a3f405b649639c3d69969349a23a5f3df2 100644 +--- a/dist/esm/client/auth.js ++++ b/dist/esm/client/auth.js +@@ -216,12 +216,21 @@ async function authInternal(provider, { serverUrl, authorizationCode, scope, res + }); + } + const resource = await selectResourceURL(serverUrl, provider, resourceMetadata); +- // Apply scope selection strategy (SEP-835): +- // 1. WWW-Authenticate scope (passed via `scope` param) +- // 2. PRM scopes_supported +- // 3. Client metadata scope (user-configured fallback) ++ // Apply scope selection strategy (SEP-835 + opencode #34034): ++ // Server-advertised resource scopes (WWW-Authenticate, then PRM ++ // scopes_supported) are MERGED with the user-configured ++ // clientMetadata.scope instead of the config scope being a last-resort ++ // fallback. This lets client<->AS scopes like `offline_access` (which a ++ // resource server never advertises) be requested on top of the resource ++ // scopes, so the AS issues a refresh token. Order preserved, deduped. + // The resolved scope is used consistently for both DCR and the authorization request. +- const resolvedScope = scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope; ++ const advertisedScope = scope || resourceMetadata?.scopes_supported?.join(' '); ++ const resolvedScope = [advertisedScope, provider.clientMetadata.scope] ++ .filter(Boolean) ++ .flatMap((s) => s.split(/\s+/)) ++ .filter(Boolean) ++ .filter((value, index, all) => all.indexOf(value) === index) ++ .join(' ') || undefined; + // Handle client registration if needed + let clientInformation = await Promise.resolve(provider.clientInformation()); + if (!clientInformation) { +diff --git a/dist/esm/client/index.d.ts b/dist/esm/client/index.d.ts +index 6f567a193626587a2730b5a49293ca5dfd4181ea..5b7c841c000508e389ce617f559f7c2a5126ca9f 100644 +--- a/dist/esm/client/index.d.ts ++++ b/dist/esm/client/index.d.ts +@@ -428,6 +428,8 @@ export declare class Client>; ++ callTool(params: CallToolRequest['params'], resultSchema: T, options?: RequestOptions): Promise>; + callTool(params: CallToolRequest['params'], resultSchema?: typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema, options?: RequestOptions): Promise<{ + [x: string]: unknown; + content: ({ diff --git a/dist/esm/client/index.js b/dist/esm/client/index.js -index 49b12c6cd918c457420fef7ad5528a9443d1a191..2afe2e22e960f26c9d516ef135d89f8eb9e4caff 100644 +index 49b12c6cd918c457420fef7ad5528a9443d1a191..339153cb7e9299b7a9bdec0e56e41cedf425fb31 100644 --- a/dist/esm/client/index.js +++ b/dist/esm/client/index.js @@ -284,41 +284,16 @@ export class Client extends Protocol { @@ -310,7 +372,7 @@ index 49b12c6cd918c457420fef7ad5528a9443d1a191..2afe2e22e960f26c9d516ef135d89f8e * After initialization has completed, this will be populated with the server's reported capabilities. */ diff --git a/dist/esm/client/streamableHttp.js b/dist/esm/client/streamableHttp.js -index 624172aa24ae255a67c083f9c19053343e4a0581..ac75b14545fda44aff7ff4d97cc5da884fcc627a 100644 +index 624172aa24ae255a67c083f9c19053343e4a0581..0a08610045189f33ab029d5dec60262a9c5e859d 100644 --- a/dist/esm/client/streamableHttp.js +++ b/dist/esm/client/streamableHttp.js @@ -1,5 +1,5 @@ @@ -408,7 +470,7 @@ index 624172aa24ae255a67c083f9c19053343e4a0581..ac75b14545fda44aff7ff4d97cc5da88 } throw new StreamableHTTPError(response.status, `Error POSTing to endpoint: ${text}`); diff --git a/dist/esm/shared/protocol.js b/dist/esm/shared/protocol.js -index bfa2b7120a0f50c569364ea5264e6f811076f44f..abd8dfd707c155f71dae7aeeeeaf7547368ac749 100644 +index bfa2b7120a0f50c569364ea5264e6f811076f44f..dec477d16a0fd796854542c1144279a6e86567f2 100644 --- a/dist/esm/shared/protocol.js +++ b/dist/esm/shared/protocol.js @@ -740,7 +740,12 @@ export class Protocol {