mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 09:03:58 +00:00
fix(agents): bound body-less MCP HTTP text responses (#98143)
* fix(agents): bound body-less MCP HTTP text responses * fix(agents): fail closed on unbounded MCP text fallback * test(agents): cover lying MCP bodyless content length * fix(agents): reject malformed MCP content length * chore: drop unused FinalizationRegistry after rebase Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(mcp): consolidate bounded response cases Co-authored-by: Pick-cat <huang.ting3@xydigit.com> * fix(mcp): reject unbounded bodyless responses Co-authored-by: Pick-cat <huang.ting3@xydigit.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<Response> {
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -61,10 +61,8 @@ async function ensureGlobalFetchResponse(response: Response): Promise<Response>
|
||||
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);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user