diff --git a/.changeset/oauth-header-spread-order.md b/.changeset/oauth-header-spread-order.md new file mode 100644 index 0000000000..777bcf0447 --- /dev/null +++ b/.changeset/oauth-header-spread-order.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Fix the spread order in `StreamableHTTPClientTransport._commonHeaders()` and `SSEClientTransport._commonHeaders()` so SDK-derived common headers (including fresh OAuth tokens from `authProvider`) win over caller-supplied headers in `requestInit.headers`. Previously a caller-supplied `Authorization` placeholder (e.g. an env-var API key) was merged after the SDK-computed value, silently overriding OAuth-refreshed tokens and breaking the auth-refresh flow once the placeholder went stale. Closes #2208. diff --git a/packages/client/src/client/sse.ts b/packages/client/src/client/sse.ts index 1c81928f10..723510407b 100644 --- a/packages/client/src/client/sse.ts +++ b/packages/client/src/client/sse.ts @@ -168,11 +168,17 @@ export class SSEClientTransport implements Transport { headers['mcp-protocol-version'] = this._protocolVersion; } + // Order matters: caller-supplied headers (e.g. an `Authorization` placeholder + // for an env-var API key) are merged first, then the SDK-derived common + // headers spread on top. This lets OAuth-derived tokens override any + // stale user-supplied header (matching the order used in streamableHttp + // and the rest of the SDK) without requiring the caller to know which + // common headers the client will compute at request time. See #2208. const extraHeaders = normalizeHeaders(this._requestInit?.headers); return new Headers({ - ...headers, - ...extraHeaders + ...extraHeaders, + ...headers }); } diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index 9067fd1ec4..190b5a9cd1 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -434,11 +434,17 @@ export class StreamableHTTPClientTransport implements Transport { headers['mcp-protocol-version'] = this._protocolVersion; } + // Order matters: caller-supplied headers (e.g. an `Authorization` placeholder + // for an env-var API key) are merged first, then the SDK-derived common + // headers spread on top. This lets OAuth-derived tokens override any + // stale user-supplied header (matching the order used in the rest of + // the SDK) without requiring the caller to know which common headers + // the client will compute at request time. See #2208. const extraHeaders = normalizeHeaders(this._requestInit?.headers); return new Headers({ - ...headers, - ...extraHeaders + ...extraHeaders, + ...headers }); } diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index a36bbc0ad3..fcee3b8ab9 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -702,6 +702,42 @@ describe('StreamableHTTPClientTransport', () => { expect(globalThis.fetch).toHaveBeenCalledTimes(2); }); + it('OAuth-derived Authorization overrides a stale caller-supplied Authorization', async () => { + // Regression test for #2208: the SDK-derived common headers must be + // merged on top of caller-supplied headers, so OAuth-derived tokens + // (or any SDK-computed value) win over a stale placeholder the + // caller might have set in `requestInit.headers` (e.g. an env-var + // API key that's no longer valid). + const tokens: OAuthTokens = { + access_token: 'oauth-access-token', + token_type: 'Bearer' + }; + mockAuthProvider.tokens.mockResolvedValue(tokens); + const requestInit = { + headers: { + Authorization: 'Bearer stale-placeholder', + 'X-Caller-Header': 'preserved' + } + }; + transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { + requestInit, + authProvider: mockAuthProvider + }); + + let actualReqInit: RequestInit = {}; + (globalThis.fetch as Mock).mockImplementation(async (_url, reqInit) => { + actualReqInit = reqInit; + return new Response(null, { status: 200, headers: { 'content-type': 'text/event-stream' } }); + }); + + await transport.start(); + await transport['_startOrAuthSse']({}); + // OAuth-derived token wins over the stale placeholder. + expect((actualReqInit.headers as Headers).get('authorization')).toBe('Bearer oauth-access-token'); + // Caller-supplied non-auth headers still pass through. + expect((actualReqInit.headers as Headers).get('x-caller-header')).toBe('preserved'); + }); + it('should always send specified custom headers (Headers class)', async () => { const requestInit = { headers: new Headers({