fix(mcp): treat empty and null optional OAuth fields as absent

Empty or null optional fields in OAuth token and registration responses
(scope, refresh_token, id_token, client_secret) failed sign-in, and
expires_in: null marked tokens as expired. Also fall back to the server
origin when resource metadata has invalid URLs, skip empty scopes when
selecting the requested scope, and end pagination at an empty or null
nextCursor.

closes #10266
This commit is contained in:
Armin Ronacher 2026-10-01 02:05:35 +02:00
parent b56702ad34
commit 8ce69e9d2b
8 changed files with 60 additions and 15 deletions

View file

@ -9,6 +9,7 @@
### Fixed
- Fixed MCP OAuth sign-in accepting an authorization response whose `iss` parameter names another authorization server; the code is now rejected before it is exchanged (RFC 9207).
- Fixed MCP OAuth sign-in failing with `Invalid scope` when the token response contains `"scope": ""`, and similar failures for other empty or `null` optional OAuth fields ([#10266](https://github.com/earendil-works/pi/issues/10266)).
## [0.99.2] - 2026-09-30

View file

@ -7,6 +7,14 @@
- Added an `authorizationServerMetadataUrl` option to `authorizeMcp()` to use a configured authorization server metadata document instead of discovery ([#10172](https://github.com/earendil-works/pi/issues/10172)).
- Added an `iss` option to `authorizeMcp()`. The authorization code is only exchanged when `iss` names the flow's authorization server, or is absent and the server's metadata does not set `authorization_response_iss_parameter_supported` (RFC 9207).
### Fixed
- Fixed OAuth token and client registration responses with an empty or `null` optional field (such as `"scope": ""`) failing with `Invalid scope` and similar errors; such fields are now treated as absent ([#10266](https://github.com/earendil-works/pi/issues/10266)).
- Fixed `expires_in: null` in a token response marking the access token as already expired.
- Fixed invalid URLs in protected resource metadata aborting OAuth discovery instead of falling back to the MCP server's origin.
- Fixed an empty requested scope, from `scope=""` in a `WWW-Authenticate` challenge or an empty `scopes_supported`, overriding the next scope source.
- Fixed list pagination failing with a duplicate cursor error when a server ends pagination with `nextCursor: ""` or `null`.
## [0.99.2] - 2026-09-30
### Fixed

View file

@ -100,10 +100,10 @@ function validateListPage(
for (const item of items) {
if (!isObject(item) || !isItem(item)) throw invalid(`Invalid entry in MCP ${method} result`);
}
if (value.nextCursor !== undefined && typeof value.nextCursor !== "string") {
throw invalid(`Invalid MCP ${method} cursor`);
}
return { items, ...(value.nextCursor === undefined ? {} : { nextCursor: value.nextCursor }) };
// Some servers end pagination with `null` or `""` instead of omitting the cursor.
const nextCursor = value.nextCursor === null || value.nextCursor === "" ? undefined : value.nextCursor;
if (nextCursor !== undefined && typeof nextCursor !== "string") throw invalid(`Invalid MCP ${method} cursor`);
return { items, ...(nextCursor === undefined ? {} : { nextCursor }) };
}
const isTool = (tool: Record<string, unknown>) => typeof tool.name === "string" && isObject(tool.inputSchema);

View file

@ -32,7 +32,8 @@ function pathSuffix(pathname: string): string {
function field(header: string, name: string): string | undefined {
const match = header.match(new RegExp(String.raw`(?:^|[,\s])${name}=(?:"([^"]*)"|([^\s,]+))`, "i"));
return match?.[1] ?? match?.[2];
// An empty value (`scope=""`) carries no information, so it counts as absent.
return match?.[1] || match?.[2] || undefined;
}
export function parseWwwAuthenticate(header: string | null): OAuthChallenge {

View file

@ -292,8 +292,9 @@ async function runFlow(provider: OAuthClientProvider, options: OAuthFlowOptions)
}
const metadata = discovered.authorizationServerMetadata;
const resource = selectResource(options.serverUrl, discovered.resourceMetadata);
// `||`, not `??`: an empty scope (for example from `scopes_supported: []`) falls through to the next source.
const scope =
options.scope ?? discovered.resourceMetadata?.scopes_supported?.join(" ") ?? provider.clientMetadata.scope;
options.scope || discovered.resourceMetadata?.scopes_supported?.join(" ") || provider.clientMetadata.scope;
let client = await provider.clientInformation();
if (!client) {
if (options.authorizationCode) throw new Error("OAuth client information is missing during code exchange");

View file

@ -102,26 +102,33 @@ function requiredString(value: unknown, name: string): string {
return value;
}
/** Treats `null` and `""` as absent: servers send them for fields they have no value for, like `scope: ""`. */
function absent(value: unknown): value is undefined | null | "" {
return value === undefined || value === null || value === "";
}
function optionalString(value: unknown, name: string): string | undefined {
if (value === undefined) return undefined;
if (absent(value)) return undefined;
return requiredString(value, name);
}
function optionalStrings(value: unknown, name: string): string[] | undefined {
if (value === undefined) return undefined;
if (value === undefined || value === null) return undefined;
if (!Array.isArray(value) || value.some((item) => typeof item !== "string")) throw new Error(`Invalid ${name}`);
return [...value];
}
function safeUrl(value: unknown, name: string): string {
const text = requiredString(value, name);
// URL parsing throws a `TypeError`, which discovery reserves for network failures.
if (!URL.canParse(text)) throw new Error(`Invalid ${name}`);
const url = new URL(text);
if (["javascript:", "data:", "vbscript:"].includes(url.protocol)) throw new Error(`Invalid ${name}`);
return text;
}
function optionalUrl(value: unknown, name: string): string | undefined {
return value === undefined ? undefined : safeUrl(value, name);
return absent(value) ? undefined : safeUrl(value, name);
}
export function parseProtectedResourceMetadata(value: unknown): OAuthProtectedResourceMetadata {
@ -170,7 +177,8 @@ export function parseAuthorizationServerMetadata(value: unknown): AuthorizationS
export function parseOAuthTokens(value: unknown): OAuthTokens {
const input = object(value, "OAuth token response");
const expires = input.expires_in === undefined ? undefined : Number(input.expires_in);
// `Number(null)` is 0, which would mark the token as expired at once.
const expires = absent(input.expires_in) ? undefined : Number(input.expires_in);
if (expires !== undefined && !Number.isFinite(expires)) throw new Error("Invalid expires_in");
return compact({
access_token: requiredString(input.access_token, "access_token"),

View file

@ -109,6 +109,8 @@ describe("McpClient", () => {
annotations: { readOnlyHint: true },
},
],
// Some servers end pagination with an empty cursor instead of omitting it.
nextCursor: "",
};
});
expect(await client.listTools()).toEqual([

View file

@ -126,7 +126,8 @@ describe("MCP OAuth", () => {
if (url.pathname === "/register") {
const metadata = JSON.parse(await readBody(request)) as Record<string, unknown>;
response.writeHead(201, { "content-type": "application/json" });
response.end(JSON.stringify({ ...metadata, client_id: "test-client" }));
// Empty and null optional fields count as absent (#10266).
response.end(JSON.stringify({ ...metadata, client_id: "test-client", client_secret: "" }));
return;
}
if (url.pathname === "/authorize") {
@ -142,7 +143,14 @@ describe("MCP OAuth", () => {
if (params.get("grant_type") === "refresh_token") {
refreshes++;
response.setHeader("content-type", "application/json");
response.end(JSON.stringify({ access_token: "refreshed-token", token_type: "Bearer" }));
response.end(
JSON.stringify({
access_token: "refreshed-token",
token_type: "Bearer",
refresh_token: "",
expires_in: null,
}),
);
return;
}
const challenge = createHash("sha256")
@ -155,7 +163,12 @@ describe("MCP OAuth", () => {
}
response.setHeader("content-type", "application/json");
response.end(
JSON.stringify({ access_token: "first-token", refresh_token: "refresh-token", token_type: "Bearer" }),
JSON.stringify({
access_token: "first-token",
refresh_token: "refresh-token",
token_type: "Bearer",
scope: "",
}),
);
return;
}
@ -178,7 +191,8 @@ describe("MCP OAuth", () => {
if (token !== "Bearer first-token" && token !== "Bearer refreshed-token") {
await readBody(request);
response.writeHead(401, {
"www-authenticate": `Bearer resource_metadata="${serverOrigin}/.well-known/oauth-protected-resource/mcp", scope="org:read"`,
// An empty scope falls through to the resource metadata's scopes_supported.
"www-authenticate": `Bearer resource_metadata="${serverOrigin}/.well-known/oauth-protected-resource/mcp", scope=""`,
});
response.end("Unauthorized");
return;
@ -251,7 +265,11 @@ describe("MCP OAuth", () => {
openGetStream: false,
}),
);
expect(provider.tokenSet?.access_token).toBe("refreshed-token");
expect(provider.tokenSet).toEqual({
access_token: "refreshed-token",
refresh_token: "refresh-token",
token_type: "Bearer",
});
expect(refreshes).toBe(1);
await refreshedClient.close();
await callback.close();
@ -261,6 +279,12 @@ describe("MCP OAuth", () => {
const grants: string[] = [];
const origin = await listen(async (request, response, serverOrigin) => {
const url = new URL(request.url ?? "/", serverOrigin);
if (url.pathname === "/.well-known/oauth-protected-resource/mcp") {
// Invalid resource metadata falls back to the server origin instead of failing discovery.
response.setHeader("content-type", "application/json");
response.end(JSON.stringify({ resource: `${serverOrigin}/mcp`, authorization_servers: ["not a url"] }));
return;
}
if (url.pathname === "/.well-known/oauth-authorization-server") {
response.setHeader("content-type", "application/json");
response.end(