mirror of
https://github.com/moltbot/moltbot.git
synced 2026-04-25 23:47:20 +00:00
fix(fetch-guard): drop request body on cross-origin unsafe-method redirects [AI-assisted] (#62357)
* fix: address issue * fix: address review feedback * docs(changelog): add fetch guard redirect body entry --------- Co-authored-by: Devin Robison <drobison@nvidia.com>
This commit is contained in:
@@ -53,6 +53,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Agents/exec: preserve explicit `host=node` routing under elevated defaults when `tools.exec.host=auto`, fail loud on invalid elevated cross-host overrides, and keep `strictInlineEval` commands blocked after approval timeouts instead of falling through to automatic execution. (#61739) Thanks @obviyus.
|
||||
- Commands/allowlist: require owner authorization for `/allowlist add` and `/allowlist remove` before channel resolution, so non-owner but command-authorized senders can no longer persistently rewrite allowlist policy state. (#62383) Thanks @pgondhi987.
|
||||
- Feishu/docx uploads: honor `tools.fs.workspaceOnly` for local `upload_file` and `upload_image` paths by forwarding workspace-constrained `localRoots` into the media loader, so docx uploads can no longer read host-local files outside the workspace when workspace-only mode is active. (#62369) Thanks @pgondhi987.
|
||||
- Network/fetch guard: drop request bodies and body-describing headers on cross-origin `307` and `308` redirects by default, so attacker-controlled redirect hops cannot receive secret-bearing POST payloads from SSRF-guarded fetch flows unless a caller explicitly opts in. (#62357) Thanks @pgondhi987.
|
||||
- Providers/Ollama: honor the selected provider's `baseUrl` during streaming so multi-Ollama setups stop routing every stream to the first configured Ollama endpoint. (#61678)
|
||||
- Browser/remote CDP: retry the DevTools websocket once after remote browser restarts so healthy remote browser profiles do not fail availability checks during CDP warm-up. (#57397) Thanks @ThanhNguyxn07.
|
||||
- Browser/SSRF: treat main-frame `document` redirect hops as navigations even when Playwright does not flag them as `isNavigationRequest()`, so strict private-network blocking still stops forbidden redirect pivots before the browser reaches the internal target. (#62355) Thanks @pgondhi987.
|
||||
|
||||
@@ -93,6 +93,9 @@ export function buildGuardedModelFetch(model: Model<Api>): typeof fetch {
|
||||
url,
|
||||
init: requestInit ?? init,
|
||||
dispatcherPolicy,
|
||||
// Provider transport intentionally keeps the secure default and never
|
||||
// replays unsafe request bodies across cross-origin redirects.
|
||||
allowCrossOriginUnsafeRedirectReplay: false,
|
||||
...(requestConfig.allowPrivateNetwork ? { policy: { allowPrivateNetwork: true } } : {}),
|
||||
});
|
||||
return buildManagedResponse(result.response, result.release);
|
||||
|
||||
@@ -693,7 +693,7 @@ describe("fetchWithSsrFGuard hardening", () => {
|
||||
await result.release();
|
||||
});
|
||||
|
||||
it("preserves body while stripping auth headers for cross-origin 307 redirects", async () => {
|
||||
it("drops unsafe bodies while stripping auth headers for cross-origin 307 redirects", async () => {
|
||||
const lookupFn = createPublicLookup();
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
@@ -719,6 +719,113 @@ describe("fetchWithSsrFGuard hardening", () => {
|
||||
},
|
||||
});
|
||||
|
||||
const secondInit = getSecondRequestInit(fetchImpl);
|
||||
const headers = getSecondRequestHeaders(fetchImpl);
|
||||
expect(secondInit.method).toBe("POST");
|
||||
expect(secondInit.body).toBeUndefined();
|
||||
expect(headers.get("authorization")).toBeNull();
|
||||
expect(headers.get("content-type")).toBeNull();
|
||||
await result.release();
|
||||
});
|
||||
|
||||
it("preserves unsafe cross-origin 307 bodies only when explicitly enabled", async () => {
|
||||
const lookupFn = createPublicLookup();
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(
|
||||
new Response(null, {
|
||||
status: 307,
|
||||
headers: { location: "https://cdn.example.com/upload-2" },
|
||||
}),
|
||||
)
|
||||
.mockResolvedValueOnce(okResponse());
|
||||
|
||||
const result = await fetchWithSsrFGuard({
|
||||
url: "https://api.example.com/upload",
|
||||
fetchImpl,
|
||||
lookupFn,
|
||||
allowCrossOriginUnsafeRedirectReplay: true,
|
||||
init: {
|
||||
method: "POST",
|
||||
headers: {
|
||||
Authorization: "Bearer secret",
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
body: '{"secret":"123"}',
|
||||
},
|
||||
});
|
||||
|
||||
const secondInit = getSecondRequestInit(fetchImpl);
|
||||
const headers = getSecondRequestHeaders(fetchImpl);
|
||||
expect(secondInit.method).toBe("POST");
|
||||
expect(secondInit.body).toBe('{"secret":"123"}');
|
||||
expect(headers.get("authorization")).toBeNull();
|
||||
expect(headers.get("content-type")).toBe("application/json");
|
||||
await result.release();
|
||||
});
|
||||
|
||||
it("drops unsafe bodies while stripping auth headers for cross-origin 308 redirects", async () => {
|
||||
const lookupFn = createPublicLookup();
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(
|
||||
new Response(null, {
|
||||
status: 308,
|
||||
headers: { location: "https://cdn.example.com/upload-2" },
|
||||
}),
|
||||
)
|
||||
.mockResolvedValueOnce(okResponse());
|
||||
|
||||
const result = await fetchWithSsrFGuard({
|
||||
url: "https://api.example.com/upload",
|
||||
fetchImpl,
|
||||
lookupFn,
|
||||
init: {
|
||||
method: "POST",
|
||||
headers: {
|
||||
Authorization: "Bearer secret",
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
body: '{"secret":"123"}',
|
||||
},
|
||||
});
|
||||
|
||||
const secondInit = getSecondRequestInit(fetchImpl);
|
||||
const headers = getSecondRequestHeaders(fetchImpl);
|
||||
expect(secondInit.method).toBe("POST");
|
||||
expect(secondInit.body).toBeUndefined();
|
||||
expect(headers.get("authorization")).toBeNull();
|
||||
expect(headers.get("content-type")).toBeNull();
|
||||
await result.release();
|
||||
});
|
||||
|
||||
it("preserves unsafe cross-origin 308 bodies only when explicitly enabled", async () => {
|
||||
const lookupFn = createPublicLookup();
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(
|
||||
new Response(null, {
|
||||
status: 308,
|
||||
headers: { location: "https://cdn.example.com/upload-2" },
|
||||
}),
|
||||
)
|
||||
.mockResolvedValueOnce(okResponse());
|
||||
|
||||
const result = await fetchWithSsrFGuard({
|
||||
url: "https://api.example.com/upload",
|
||||
fetchImpl,
|
||||
lookupFn,
|
||||
allowCrossOriginUnsafeRedirectReplay: true,
|
||||
init: {
|
||||
method: "POST",
|
||||
headers: {
|
||||
Authorization: "Bearer secret",
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
body: '{"secret":"123"}',
|
||||
},
|
||||
});
|
||||
|
||||
const secondInit = getSecondRequestInit(fetchImpl);
|
||||
const headers = getSecondRequestHeaders(fetchImpl);
|
||||
expect(secondInit.method).toBe("POST");
|
||||
|
||||
@@ -37,6 +37,12 @@ export type GuardedFetchOptions = {
|
||||
fetchImpl?: FetchLike;
|
||||
init?: RequestInit;
|
||||
maxRedirects?: number;
|
||||
/**
|
||||
* Allow replaying unsafe request methods and bodies across cross-origin redirects.
|
||||
* Sensitive cross-origin headers (for example Authorization/Cookie) are still stripped.
|
||||
* Defaults to false.
|
||||
*/
|
||||
allowCrossOriginUnsafeRedirectReplay?: boolean;
|
||||
timeoutMs?: number;
|
||||
signal?: AbortSignal;
|
||||
policy?: SsrFPolicy;
|
||||
@@ -229,6 +235,27 @@ function rewriteRedirectInitForMethod(params: {
|
||||
};
|
||||
}
|
||||
|
||||
function rewriteRedirectInitForCrossOrigin(params: {
|
||||
init?: RequestInit;
|
||||
allowUnsafeReplay: boolean;
|
||||
}): RequestInit | undefined {
|
||||
const { init, allowUnsafeReplay } = params;
|
||||
if (!init || allowUnsafeReplay) {
|
||||
return init;
|
||||
}
|
||||
|
||||
const currentMethod = init.method?.toUpperCase() ?? "GET";
|
||||
if (currentMethod === "GET" || currentMethod === "HEAD") {
|
||||
return init;
|
||||
}
|
||||
|
||||
return {
|
||||
...init,
|
||||
body: undefined,
|
||||
headers: dropBodyHeaders(init.headers),
|
||||
};
|
||||
}
|
||||
|
||||
export { fetchWithRuntimeDispatcher } from "./runtime-fetch.js";
|
||||
|
||||
export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<GuardedFetchResult> {
|
||||
@@ -336,6 +363,10 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
|
||||
}
|
||||
currentInit = rewriteRedirectInitForMethod({ init: currentInit, status: response.status });
|
||||
if (nextParsedUrl.origin !== parsedUrl.origin) {
|
||||
currentInit = rewriteRedirectInitForCrossOrigin({
|
||||
init: currentInit,
|
||||
allowUnsafeReplay: params.allowCrossOriginUnsafeRedirectReplay === true,
|
||||
});
|
||||
currentInit = retainSafeHeadersForCrossOriginRedirect(currentInit);
|
||||
}
|
||||
visited.add(nextUrl);
|
||||
|
||||
Reference in New Issue
Block a user