diff --git a/CHANGELOG.md b/CHANGELOG.md index c7587e99e9ac..a6569ff2b737 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai - **Diffs rendering:** render viewer and image output from one SSR preload, preserve language-pack highlighting through hydration, normalize language hints case-insensitively, skip identical before/after inputs with an explicit `changed` result, report truthful file-render and input errors, cache hash-pinned viewer runtimes, and prefer canonical file settings over stale aliases. (#100487) - **Remote browser reliability:** bound persistent Playwright tab enumeration by the existing remote CDP timeout budget and retire timed-out connection attempts so late completions cannot restore a stuck connection. (#80147, #58968) Thanks @HemantSudarshan and @KeaneYan. +- **MCP OAuth response bounds:** reject body-less foreign error bodies without calling their inherently unbounded `text()` fallback, while preserving HTTP status and headers for safe SDK diagnostics. (#98143) Thanks @Pick-cat. - **Control UI approval prompts:** keep stale resolve failures and busy-state cleanup from leaking across newer approvals or Gateway reconnects. (#98394) Thanks @haruaiclone-droid. - **Agent empty replies:** surface a visible failure when a completed interactive turn has no deliverable reply, including queued follow-ups, while preserving explicit silence, pending continuations, and committed side effects. (#100456) Thanks @mushuiyu886. - **Child process output safety:** prevent stdout/stderr pipe failures from crashing agent exec sessions, local TUI shell commands, and bounded process execution. (#100407, #100406, #100410) Thanks @cxbAsDev. diff --git a/src/agents/mcp-http-fetch.test.ts b/src/agents/mcp-http-fetch.test.ts index d2a957ea406c..f53d5c1ffffc 100644 --- a/src/agents/mcp-http-fetch.test.ts +++ b/src/agents/mcp-http-fetch.test.ts @@ -33,6 +33,34 @@ class TestProxyAgent { constructor(readonly options: unknown) {} } +function useBodylessForeignResponse(params: { text: string; contentLength?: string }) { + const text = vi.fn(async () => params.text); + const headers = new Headers({ "content-type": "application/json" }); + if (params.contentLength !== undefined) { + headers.set("content-length", params.contentLength); + } + testGlobal[TEST_UNDICI_RUNTIME_DEPS_KEY] = { + Agent: TestAgent, + EnvHttpProxyAgent: TestEnvHttpProxyAgent, + ProxyAgent: TestProxyAgent, + fetch: async () => + ({ + status: 400, + statusText: "Bad Request", + headers, + body: null, + ok: false, + text, + }) as unknown as Response, + }; + return text; +} + +async function fetchOAuthRegistrationError(): Promise { + const fetch = buildMcpHttpFetch({ resourceUrl: "https://mcp.example.com/mcp" }); + return await fetch("https://auth.example.com/oauth/register", { method: "POST" }); +} + function redirectResponse(location: string, status = 302): Response { return new Response(null, { status, @@ -212,37 +240,36 @@ describe("MCP HTTP fetch helpers", () => { expect(calls[1]?.[1]?.headers).toBeUndefined(); }); - it("returns fetch responses compatible with MCP SDK OAuth error parsing", async () => { - class ForeignResponse { - status = 400; - statusText = "Bad Request"; - headers = new Headers({ "content-type": "application/json" }); - body = null; - get ok() { - return false; - } - async text() { - return '{"error":"invalid_client_metadata","error_description":"bad redirect"}'; - } - } + it.each([undefined, "64", "1048577"])( + "drops body-less foreign OAuth text without trusting Content-Length %s", + async (contentLength) => { + const text = useBodylessForeignResponse({ + text: '{"error_description":"unbounded"}', + contentLength, + }); - testGlobal[TEST_UNDICI_RUNTIME_DEPS_KEY] = { - Agent: TestAgent, - EnvHttpProxyAgent: TestEnvHttpProxyAgent, - ProxyAgent: TestProxyAgent, - fetch: async (url: string | URL | Request, init?: unknown) => { - fetchCalls.push({ url, init }); - return new ForeignResponse() as unknown as Response; - }, - }; - const fetch = buildMcpHttpFetch({ - resourceUrl: "https://mcp.example.com/mcp", + const response = await fetchOAuthRegistrationError(); + + expect(response).toBeInstanceOf(Response); + expect(response.status).toBe(400); + expect(response.body).toBeNull(); + expect(text).not.toHaveBeenCalled(); + const error = await parseErrorResponse(response); + expect(error.message).toContain("HTTP 400"); + }, + ); + + it("never materializes a body-less foreign response with a lying safe length", async () => { + const text = useBodylessForeignResponse({ + text: "x".repeat(1024 * 1024 + 1), + contentLength: "64", }); - const response = await fetch("https://auth.example.com/oauth/register", { method: "POST" }); + const response = await fetchOAuthRegistrationError(); + expect(response).toBeInstanceOf(Response); - const error = await parseErrorResponse(response); - expect(error.message).toContain("bad redirect"); - expect(error.message).not.toContain("[object Response]"); + expect(response.status).toBe(400); + expect(response.body).toBeNull(); + expect(text).not.toHaveBeenCalled(); }); }); diff --git a/src/agents/mcp-http-fetch.ts b/src/agents/mcp-http-fetch.ts index c2ff26f3f6d7..83bf9ebf28b1 100644 --- a/src/agents/mcp-http-fetch.ts +++ b/src/agents/mcp-http-fetch.ts @@ -61,10 +61,8 @@ async function ensureGlobalFetchResponse(response: Response): Promise if (response.status === 204 || response.status === 205 || response.status === 304) { return new Response(null, init); } - if (typeof response.text === "function") { - const text = await response.text(); - return new Response(text, init); - } + // A body-less foreign Response exposes no bounded reader. Calling text() or + // arrayBuffer() can allocate an attacker-controlled body before any cap applies. return new Response(null, init); }