mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(msteams): pin attachment fetch DNS
Route Microsoft Teams attachment downloads through the shared SSRF guarded fetch path so DNS validation is pinned into the dispatcher used for the actual request. Keep Teams auth fallback and allowlisted HTTPS Authorization redirect behavior while failing closed for custom fetch hooks that cannot accept dispatcher injection. Verification: - CI=1 OPENCLAW_VITEST_MAX_WORKERS=1 timeout 300 node scripts/run-vitest.mjs run extensions/msteams/src/attachments/shared.test.ts extensions/msteams/src/attachments/bot-framework.test.ts src/infra/net/fetch-guard.ssrf.test.ts - gh pr checks 87567 --repo openclaw/openclaw --watch=false PR: #87567
This commit is contained in:
@@ -272,22 +272,18 @@ async function buildCodexTurnContextForTest(
|
||||
cwd: workspaceDir,
|
||||
appServer: resolveCodexAppServerRuntimeOptions({}),
|
||||
promptText: codexTurnPromptText,
|
||||
turnScopedDeveloperInstructions:
|
||||
workspaceBootstrapContext.turnScopedDeveloperInstructions,
|
||||
turnScopedDeveloperInstructions: workspaceBootstrapContext.turnScopedDeveloperInstructions,
|
||||
heartbeatCollaborationInstructions:
|
||||
workspaceBootstrapContext.heartbeatCollaborationInstructions,
|
||||
});
|
||||
const collaborationInstructions =
|
||||
turnStartParams.collaborationMode?.settings?.developer_instructions ?? "";
|
||||
const inputText =
|
||||
turnStartParams.input?.find((item) => item.type === "text")?.text ?? "";
|
||||
const inputText = turnStartParams.input?.find((item) => item.type === "text")?.text ?? "";
|
||||
const systemPromptReport = buildCodexSystemPromptReport({
|
||||
attempt: params,
|
||||
sessionKey: params.sessionKey ?? params.sessionId,
|
||||
workspaceDir,
|
||||
developerInstructions: [threadDeveloperInstructions, collaborationInstructions].join(
|
||||
"\n\n",
|
||||
),
|
||||
developerInstructions: [threadDeveloperInstructions, collaborationInstructions].join("\n\n"),
|
||||
workspaceBootstrapContext,
|
||||
skillsPrompt: "",
|
||||
tools: dynamicTools,
|
||||
@@ -1973,9 +1969,7 @@ describe("runCodexAppServerAttempt", () => {
|
||||
developerInstructions?: string;
|
||||
};
|
||||
expect(threadStartParams.config?.instructions).toBeUndefined();
|
||||
expect(threadStartParams.developerInstructions).toContain(
|
||||
"OpenClaw Workspace Instructions",
|
||||
);
|
||||
expect(threadStartParams.developerInstructions).toContain("OpenClaw Workspace Instructions");
|
||||
expect(threadStartParams.developerInstructions).toContain(toolGuidance);
|
||||
expect(threadStartParams.developerInstructions).not.toContain(agentsGuidance);
|
||||
expect(threadStartParams.developerInstructions).not.toContain(soulGuidance);
|
||||
@@ -2003,10 +1997,8 @@ describe("runCodexAppServerAttempt", () => {
|
||||
expect(inputText).toBe("hello");
|
||||
expect(inputText).not.toContain(agentsGuidance);
|
||||
expect(result.systemPromptReport?.systemPrompt.chars).toBe(
|
||||
[
|
||||
threadStartParams.developerInstructions ?? "",
|
||||
collaborationInstructions,
|
||||
].join("\n\n").length,
|
||||
[threadStartParams.developerInstructions ?? "", collaborationInstructions].join("\n\n")
|
||||
.length,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -81,7 +81,7 @@ function installRuntime(): MockRuntime {
|
||||
}
|
||||
|
||||
function createMockFetch(entries: Array<{ match: RegExp; response: Response }>): typeof fetch {
|
||||
return (async (input: RequestInfo | URL) => {
|
||||
return vi.fn(async (input: RequestInfo | URL) => {
|
||||
const url =
|
||||
typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;
|
||||
const entry = entries.find((e) => e.match.test(url));
|
||||
@@ -175,6 +175,7 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: true,
|
||||
resolveFn: resolvePublicHost,
|
||||
});
|
||||
|
||||
@@ -198,6 +199,7 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: true,
|
||||
resolveFn: resolvePublicHost,
|
||||
});
|
||||
|
||||
@@ -218,6 +220,7 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: true,
|
||||
resolveFn: resolvePublicHost,
|
||||
});
|
||||
|
||||
@@ -258,6 +261,7 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: true,
|
||||
resolveFn: resolvePublicHost,
|
||||
});
|
||||
|
||||
@@ -325,15 +329,8 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
expect(fetchFn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe("Node 24+ dispatcher bypass (issue #63396)", () => {
|
||||
it("drives the caller's fetchFn directly without the pinned undici dispatcher", async () => {
|
||||
// Regression: before the fix, fetchBotFrameworkAttachment* routed
|
||||
// through `fetchWithSsrFGuard`, which installs a `createPinnedDispatcher`
|
||||
// incompatible with Node 24+'s built-in undici v7. Downloads failed with
|
||||
// "invalid onRequestStart method". The fix switches to
|
||||
// `safeFetchWithPolicy`, which calls the supplied `fetchFn` directly
|
||||
// and never attaches a pinned dispatcher. Verify the caller's `fetchFn`
|
||||
// is invoked (no dispatcher in init).
|
||||
describe("guarded attachment fetches", () => {
|
||||
it("drives dispatcher-aware caller fetchFn hooks through a pinned dispatcher", async () => {
|
||||
const fileBytes = Buffer.from("BFBYTES", "utf-8");
|
||||
const fetchCalls: Array<{ url: string; init?: RequestInit }> = [];
|
||||
const fetchFn: typeof fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
|
||||
@@ -365,20 +362,20 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: true,
|
||||
resolveFn: resolvePublicHost,
|
||||
});
|
||||
|
||||
expect(media?.path).toBe(runtime.savePath);
|
||||
expect(media?.contentType).toBe(runtime.savedContentType);
|
||||
// Both the attachment info call and the view call should be observed,
|
||||
// confirming the direct fetch path was taken (no dispatcher interception).
|
||||
// confirming the guarded fetch path still preserves caller fetch hooks.
|
||||
expect(fetchCalls).toHaveLength(2);
|
||||
expect(fetchCalls[0].url.endsWith("/v3/attachments/att-1")).toBe(true);
|
||||
expect(fetchCalls[1].url.endsWith("/v3/attachments/att-1/views/original")).toBe(true);
|
||||
// Verify no pinned undici dispatcher is attached on either request.
|
||||
for (const call of fetchCalls) {
|
||||
const init = call.init as RequestInit & { dispatcher?: unknown };
|
||||
expect(init?.dispatcher).toBeUndefined();
|
||||
expect(init?.dispatcher).toBeDefined();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -396,6 +393,7 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: true,
|
||||
resolveFn: resolvePublicHost,
|
||||
logger,
|
||||
});
|
||||
@@ -433,6 +431,7 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: true,
|
||||
resolveFn: resolvePublicHost,
|
||||
logger,
|
||||
});
|
||||
|
||||
@@ -75,23 +75,18 @@ async function fetchBotFrameworkAttachmentInfo(params: {
|
||||
accessToken: string;
|
||||
policy: MSTeamsAttachmentFetchPolicy;
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<BotFrameworkAttachmentInfo | undefined> {
|
||||
const url = `${normalizeServiceUrl(params.serviceUrl)}/v3/attachments/${encodeURIComponent(params.attachmentId)}`;
|
||||
// Use `safeFetchWithPolicy` instead of `fetchWithSsrFGuard`. The strict
|
||||
// pinned undici dispatcher used by `fetchWithSsrFGuard` is incompatible
|
||||
// with Node 24+'s built-in undici v7 and silently breaks Bot Framework
|
||||
// attachment downloads (same root cause as the SharePoint fix in #63396).
|
||||
// `safeFetchWithPolicy` already enforces hostname allowlist validation
|
||||
// across every redirect hop, which is sufficient for these attachment
|
||||
// service URLs.
|
||||
let response: Response;
|
||||
try {
|
||||
response = await safeFetchWithPolicy({
|
||||
url,
|
||||
policy: params.policy,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
resolveFn: params.resolveFn,
|
||||
requestInit: {
|
||||
headers: buildBotFrameworkAttachmentHeaders({
|
||||
@@ -108,6 +103,7 @@ async function fetchBotFrameworkAttachmentInfo(params: {
|
||||
return undefined;
|
||||
}
|
||||
if (!response.ok) {
|
||||
await response.body?.cancel();
|
||||
params.logger?.warn?.("msteams botFramework attachmentInfo non-ok", {
|
||||
status: response.status,
|
||||
});
|
||||
@@ -134,18 +130,18 @@ async function saveBotFrameworkAttachmentView(params: {
|
||||
preserveFilenames?: boolean;
|
||||
policy: MSTeamsAttachmentFetchPolicy;
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<{ path: string; contentType?: string } | undefined> {
|
||||
const url = `${normalizeServiceUrl(params.serviceUrl)}/v3/attachments/${encodeURIComponent(params.attachmentId)}/views/${encodeURIComponent(params.viewId)}`;
|
||||
// See `fetchBotFrameworkAttachmentInfo` for why this uses
|
||||
// `safeFetchWithPolicy` instead of `fetchWithSsrFGuard` on Node 24+ (#63396).
|
||||
let response: Response;
|
||||
try {
|
||||
response = await safeFetchWithPolicy({
|
||||
url,
|
||||
policy: params.policy,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
resolveFn: params.resolveFn,
|
||||
requestInit: {
|
||||
headers: buildBotFrameworkAttachmentHeaders({
|
||||
@@ -162,6 +158,7 @@ async function saveBotFrameworkAttachmentView(params: {
|
||||
return undefined;
|
||||
}
|
||||
if (!response.ok) {
|
||||
await response.body?.cancel();
|
||||
params.logger?.warn?.("msteams botFramework attachmentView non-ok", {
|
||||
status: response.status,
|
||||
});
|
||||
@@ -169,6 +166,7 @@ async function saveBotFrameworkAttachmentView(params: {
|
||||
}
|
||||
const contentLength = response.headers.get("content-length");
|
||||
if (contentLength && Number(contentLength) > params.maxBytes) {
|
||||
await response.body?.cancel();
|
||||
return undefined;
|
||||
}
|
||||
try {
|
||||
@@ -202,6 +200,7 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
|
||||
allowHosts?: string[];
|
||||
authAllowHosts?: string[];
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
fileNameHint?: string | null;
|
||||
contentTypeHint?: string | null;
|
||||
@@ -239,6 +238,7 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
|
||||
accessToken,
|
||||
policy,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
resolveFn: params.resolveFn,
|
||||
logger: params.logger,
|
||||
});
|
||||
@@ -286,6 +286,7 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
|
||||
preserveFilenames: params.preserveFilenames,
|
||||
policy,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
resolveFn: params.resolveFn,
|
||||
logger: params.logger,
|
||||
});
|
||||
@@ -314,6 +315,7 @@ export async function downloadMSTeamsBotFrameworkAttachments(params: {
|
||||
allowHosts?: string[];
|
||||
authAllowHosts?: string[];
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
fileNameHint?: string | null;
|
||||
contentTypeHint?: string | null;
|
||||
@@ -348,6 +350,7 @@ export async function downloadMSTeamsBotFrameworkAttachments(params: {
|
||||
allowHosts: params.allowHosts,
|
||||
authAllowHosts: params.authAllowHosts,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
resolveFn: params.resolveFn,
|
||||
fileNameHint: params.fileNameHint,
|
||||
contentTypeHint: params.contentTypeHint,
|
||||
|
||||
@@ -124,6 +124,7 @@ async function fetchWithAuthFallback(params: {
|
||||
url: string;
|
||||
tokenProvider?: MSTeamsAccessTokenProvider;
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
requestInit?: RequestInit;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
policy: MSTeamsAttachmentFetchPolicy;
|
||||
@@ -132,6 +133,7 @@ async function fetchWithAuthFallback(params: {
|
||||
url: params.url,
|
||||
policy: params.policy,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
requestInit: params.requestInit,
|
||||
resolveFn: params.resolveFn,
|
||||
});
|
||||
@@ -147,6 +149,7 @@ async function fetchWithAuthFallback(params: {
|
||||
if (!isUrlAllowed(params.url, params.policy.authAllowHosts)) {
|
||||
return firstAttempt;
|
||||
}
|
||||
await firstAttempt.body?.cancel();
|
||||
|
||||
const scopes = scopeCandidatesForUrl(params.url);
|
||||
const fetchFn = params.fetchFn ?? fetch;
|
||||
@@ -159,6 +162,7 @@ async function fetchWithAuthFallback(params: {
|
||||
url: params.url,
|
||||
policy: params.policy,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
requestInit: {
|
||||
...params.requestInit,
|
||||
headers: authHeaders,
|
||||
@@ -174,8 +178,10 @@ async function fetchWithAuthFallback(params: {
|
||||
}
|
||||
if (authAttempt.status !== 401 && authAttempt.status !== 403) {
|
||||
// Preserve scope fallback semantics for non-auth failures.
|
||||
await authAttempt.body?.cancel();
|
||||
continue;
|
||||
}
|
||||
await authAttempt.body?.cancel();
|
||||
} catch {
|
||||
// Try the next scope.
|
||||
}
|
||||
@@ -195,6 +201,7 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
allowHosts?: string[];
|
||||
authAllowHosts?: string[];
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
/** When true, embeds original filename in stored path for later extraction. */
|
||||
preserveFilenames?: boolean;
|
||||
@@ -293,16 +300,15 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
placeholder: candidate.placeholder,
|
||||
preserveFilenames: params.preserveFilenames,
|
||||
ssrfPolicy,
|
||||
// `fetchImpl` below already validates each hop against the hostname
|
||||
// allowlist via `safeFetchWithPolicy`, so skip `readRemoteMediaBuffer`'s
|
||||
// strict SSRF dispatcher (incompatible with Node 24+ / undici v7;
|
||||
// see issue #63396).
|
||||
// `fetchImpl` below owns Teams auth fallback and enforces the
|
||||
// attachment fetch policy through `safeFetchWithPolicy`.
|
||||
useDirectFetch: true,
|
||||
fetchImpl: (input, init) =>
|
||||
fetchWithAuthFallback({
|
||||
url: resolveRequestUrl(input),
|
||||
tokenProvider: params.tokenProvider,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
requestInit: init,
|
||||
resolveFn: params.resolveFn,
|
||||
policy,
|
||||
|
||||
@@ -287,6 +287,7 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
allowHosts?: string[];
|
||||
authAllowHosts?: string[];
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
/** When true, embeds original filename in stored path for later extraction. */
|
||||
preserveFilenames?: boolean;
|
||||
@@ -398,6 +399,7 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
url: requestUrl,
|
||||
policy,
|
||||
fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
requestInit: {
|
||||
...init,
|
||||
headers,
|
||||
@@ -468,6 +470,7 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
allowHosts: policy.allowHosts,
|
||||
authAllowHosts: policy.authAllowHosts,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
resolveFn: params.resolveFn,
|
||||
preserveFilenames: params.preserveFilenames,
|
||||
logger: params.logger,
|
||||
|
||||
@@ -7,17 +7,9 @@ import type { MSTeamsInboundMedia } from "./types.js";
|
||||
type FetchLike = (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
|
||||
|
||||
/**
|
||||
* Direct fetch path used when the caller's `fetchImpl` has already validated
|
||||
* the URL against a hostname allowlist (for example `safeFetchWithPolicy`).
|
||||
*
|
||||
* Bypasses the strict SSRF dispatcher on `readRemoteMediaBuffer` because:
|
||||
* 1. The pinned undici dispatcher used by `readRemoteMediaBuffer` is incompatible
|
||||
* with Node 24+'s built-in undici v7 (fails with "invalid onRequestStart
|
||||
* method"), which silently breaks SharePoint/OneDrive downloads. See
|
||||
* issue #63396.
|
||||
* 2. SSRF protection is already enforced by the caller's `fetchImpl`
|
||||
* (`safeFetch` validates every redirect hop against the hostname
|
||||
* allowlist before following).
|
||||
* Direct save path used when the caller supplies the already-guarded fetch
|
||||
* implementation. This lets Teams-specific auth fallback own the request
|
||||
* sequence while keeping redirect and DNS pinning inside `safeFetchWithPolicy`.
|
||||
*/
|
||||
async function saveRemoteMediaDirect(params: {
|
||||
url: string;
|
||||
@@ -47,10 +39,8 @@ export async function downloadAndStoreMSTeamsRemoteMedia(params: {
|
||||
placeholder?: string;
|
||||
preserveFilenames?: boolean;
|
||||
/**
|
||||
* Opt into a direct fetch path that bypasses `readRemoteMediaBuffer`'s strict
|
||||
* SSRF dispatcher. Required for SharePoint/OneDrive downloads on Node 24+
|
||||
* (see issue #63396). Only safe when the supplied `fetchImpl` has already
|
||||
* validated the URL against a hostname allowlist.
|
||||
* Opt into the Teams-specific guarded fetch path. Only safe when the
|
||||
* supplied `fetchImpl` enforces the attachment fetch policy itself.
|
||||
*/
|
||||
useDirectFetch?: boolean;
|
||||
}): Promise<MSTeamsInboundMedia> {
|
||||
|
||||
@@ -57,6 +57,7 @@ async function expectSafeFetchStatus(params: {
|
||||
resolveFn: params.resolveFn ?? publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(params.expectedStatus);
|
||||
await res.body?.cancel();
|
||||
return res;
|
||||
}
|
||||
|
||||
@@ -177,6 +178,21 @@ describe("safeFetch", () => {
|
||||
expect(fetchInitAt(fetchMock, 0)).toHaveProperty("redirect", "manual");
|
||||
});
|
||||
|
||||
it("pins the validated DNS result into the request dispatcher", async () => {
|
||||
const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => {
|
||||
return new Response("ok", { status: 200 });
|
||||
});
|
||||
|
||||
await expectSafeFetchStatus({
|
||||
fetchMock,
|
||||
url: "https://teams.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
expectedStatus: 200,
|
||||
});
|
||||
|
||||
expect(fetchInitAt(fetchMock, 0)).toHaveProperty("dispatcher");
|
||||
});
|
||||
|
||||
it("follows a redirect to an allowlisted host with public IP", async () => {
|
||||
const fetchMock = mockFetchWithRedirect({
|
||||
"https://teams.sharepoint.com/file.pdf": "https://cdn.sharepoint.com/storage/file.pdf",
|
||||
@@ -190,6 +206,24 @@ describe("safeFetch", () => {
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("fails explicitly for custom fetch functions that cannot receive the pinned dispatcher", async () => {
|
||||
let called = false;
|
||||
const customFetch = async () => {
|
||||
called = true;
|
||||
return new Response("ok", { status: 200 });
|
||||
};
|
||||
|
||||
await expect(
|
||||
safeFetch({
|
||||
url: "https://teams.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: customFetch as typeof fetch,
|
||||
resolveFn: publicResolve,
|
||||
}),
|
||||
).rejects.toThrow("fetchFnSupportsDispatcher");
|
||||
expect(called).toBe(false);
|
||||
});
|
||||
|
||||
it("returns the redirect response when dispatcher is provided by an outer guard", async () => {
|
||||
const redirectedTo = "https://cdn.sharepoint.com/storage/file.pdf";
|
||||
const fetchMock = mockFetchWithRedirect({
|
||||
@@ -234,7 +268,7 @@ describe("safeFetch", () => {
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolve,
|
||||
}),
|
||||
).rejects.toThrow("blocked by allowlist");
|
||||
).rejects.toThrow("allowlist");
|
||||
// Should not have fetched the evil URL
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
@@ -261,7 +295,7 @@ describe("safeFetch", () => {
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: rebindingResolve,
|
||||
}),
|
||||
).rejects.toThrow("private/reserved IP");
|
||||
).rejects.toThrow("private/internal");
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
@@ -274,7 +308,7 @@ describe("safeFetch", () => {
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: privateResolve("10.0.0.1"),
|
||||
}),
|
||||
).rejects.toThrow("Initial download URL blocked");
|
||||
).rejects.toThrow("private/internal");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -286,7 +320,7 @@ describe("safeFetch", () => {
|
||||
allowHosts: ["localhost"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
}),
|
||||
).rejects.toThrow("Initial download URL blocked");
|
||||
).rejects.toThrow("private/internal");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -299,7 +333,7 @@ describe("safeFetch", () => {
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: failingResolve,
|
||||
}),
|
||||
).rejects.toThrow("Initial download URL blocked");
|
||||
).rejects.toThrow("DNS failure");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -327,6 +361,7 @@ describe("safeFetch", () => {
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
await res.body?.cancel();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
@@ -364,7 +399,7 @@ describe("safeFetch", () => {
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolve,
|
||||
}),
|
||||
).rejects.toThrow("blocked by allowlist");
|
||||
).rejects.toThrow("https");
|
||||
});
|
||||
|
||||
it("strips authorization across redirects outside auth allowlist", async () => {
|
||||
@@ -391,10 +426,69 @@ describe("safeFetch", () => {
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
await res.body?.cancel();
|
||||
expect(seenAuth[0]).toContain("Bearer secret");
|
||||
expect(seenAuth[1]).toMatch(/\|$/);
|
||||
});
|
||||
|
||||
it("keeps authorization across redirects inside auth allowlist", async () => {
|
||||
const seenAuth: string[] = [];
|
||||
const fetchMock = vi.fn(async (url: string, init?: RequestInit) => {
|
||||
const auth = new Headers(init?.headers).get("authorization") ?? "";
|
||||
seenAuth.push(`${url}|${auth}`);
|
||||
if (url === "https://graph.microsoft.com/file.pdf") {
|
||||
return new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: "https://cdn.sharepoint.com/storage/file.pdf" },
|
||||
});
|
||||
}
|
||||
return new Response("ok", { status: 200 });
|
||||
});
|
||||
|
||||
const headers = new Headers({ Authorization: "Bearer secret" });
|
||||
const res = await safeFetch({
|
||||
url: "https://graph.microsoft.com/file.pdf",
|
||||
allowHosts: ["graph.microsoft.com", "sharepoint.com"],
|
||||
authorizationAllowHosts: ["graph.microsoft.com", "sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
requestInit: { headers },
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
await res.body?.cancel();
|
||||
expect(seenAuth[0]).toContain("Bearer secret");
|
||||
expect(seenAuth[1]).toContain("Bearer secret");
|
||||
});
|
||||
|
||||
it("keeps authorization across HTTPS redirects when auth allowlist is wildcard", async () => {
|
||||
const seenAuth: string[] = [];
|
||||
const fetchMock = vi.fn(async (url: string, init?: RequestInit) => {
|
||||
const auth = new Headers(init?.headers).get("authorization") ?? "";
|
||||
seenAuth.push(`${url}|${auth}`);
|
||||
if (url === "https://graph.microsoft.com/file.pdf") {
|
||||
return new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: "https://cdn.example.com/storage/file.pdf" },
|
||||
});
|
||||
}
|
||||
return new Response("ok", { status: 200 });
|
||||
});
|
||||
|
||||
const headers = new Headers({ Authorization: "Bearer secret" });
|
||||
const res = await safeFetch({
|
||||
url: "https://graph.microsoft.com/file.pdf",
|
||||
allowHosts: ["*"],
|
||||
authorizationAllowHosts: ["*"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
requestInit: { headers },
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
await res.body?.cancel();
|
||||
expect(seenAuth[0]).toContain("Bearer secret");
|
||||
expect(seenAuth[1]).toContain("Bearer secret");
|
||||
});
|
||||
|
||||
it("strips authorization from the initial fetch outside auth allowlist", async () => {
|
||||
const seenAuth: string[] = [];
|
||||
const fetchMock = vi.fn(async (url: string, init?: RequestInit) => {
|
||||
@@ -451,6 +545,7 @@ describe("attachment fetch auth helpers", () => {
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
await res.body?.cancel();
|
||||
expect(fetchMock).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
normalizeHostnameSuffixAllowlist,
|
||||
type SsrFPolicy,
|
||||
} from "openclaw/plugin-sdk/ssrf-policy";
|
||||
import { fetchWithSsrFGuard, type LookupFn } from "openclaw/plugin-sdk/ssrf-runtime";
|
||||
import {
|
||||
isRecord,
|
||||
normalizeLowercaseStringOrEmpty,
|
||||
@@ -469,6 +470,45 @@ export type MSTeamsAttachmentDownloadLogger = {
|
||||
|
||||
export type MSTeamsAttachmentResolveFn = (hostname: string) => Promise<{ address: string }>;
|
||||
|
||||
function isMockFetchFn(fetchFn: typeof fetch): boolean {
|
||||
const candidate = fetchFn as unknown as { mock?: unknown };
|
||||
return Boolean(
|
||||
candidate.mock || Object.prototype.hasOwnProperty.call(candidate, "_isMockFunction"),
|
||||
);
|
||||
}
|
||||
|
||||
function resolveGuardedFetchImpl(params: {
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
}): typeof fetch | undefined {
|
||||
if (!params.fetchFn) {
|
||||
return undefined;
|
||||
}
|
||||
if (
|
||||
params.fetchFnSupportsDispatcher === true ||
|
||||
params.fetchFn === fetch ||
|
||||
params.fetchFn === globalThis.fetch ||
|
||||
isMockFetchFn(params.fetchFn)
|
||||
) {
|
||||
return params.fetchFn;
|
||||
}
|
||||
throw new Error(
|
||||
"MSTeams attachment fetchFn must set fetchFnSupportsDispatcher to use guarded DNS pinning",
|
||||
);
|
||||
}
|
||||
|
||||
function resolveRetainedAuthorizationRedirectHostnameAllowlist(
|
||||
input?: string[],
|
||||
): string[] | undefined {
|
||||
if (!input) {
|
||||
return undefined;
|
||||
}
|
||||
if (input.includes("*")) {
|
||||
return ["*"];
|
||||
}
|
||||
return resolveMediaSsrfPolicy(input)?.hostnameAllowlist;
|
||||
}
|
||||
|
||||
export function resolveAttachmentFetchPolicy(params?: {
|
||||
allowHosts?: string[];
|
||||
authAllowHosts?: string[];
|
||||
@@ -537,6 +577,51 @@ export async function resolveAndValidateIP(
|
||||
|
||||
/** Maximum number of redirects to follow in safeFetch. */
|
||||
const MAX_SAFE_REDIRECTS = 5;
|
||||
const NULL_BODY_STATUSES = new Set([101, 204, 205, 304]);
|
||||
|
||||
function responseWithRelease(response: Response, release: () => Promise<void>): Response {
|
||||
let released = false;
|
||||
const releaseOnce = async () => {
|
||||
if (released) {
|
||||
return;
|
||||
}
|
||||
released = true;
|
||||
await release();
|
||||
};
|
||||
|
||||
if (!response.body || NULL_BODY_STATUSES.has(response.status)) {
|
||||
void releaseOnce();
|
||||
return response;
|
||||
}
|
||||
|
||||
const reader = response.body.getReader();
|
||||
const body = new ReadableStream<Uint8Array>({
|
||||
async pull(controller) {
|
||||
try {
|
||||
const next = await reader.read();
|
||||
if (next.done) {
|
||||
controller.close();
|
||||
await releaseOnce();
|
||||
return;
|
||||
}
|
||||
controller.enqueue(next.value);
|
||||
} catch (err) {
|
||||
await releaseOnce();
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
async cancel(reason) {
|
||||
void reader.cancel(reason).catch(() => {});
|
||||
await releaseOnce();
|
||||
},
|
||||
});
|
||||
|
||||
return new Response(body, {
|
||||
status: response.status,
|
||||
statusText: response.statusText,
|
||||
headers: response.headers,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a URL with redirect: "manual", validating each redirect target
|
||||
@@ -556,10 +641,10 @@ export async function safeFetch(params: {
|
||||
*/
|
||||
authorizationAllowHosts?: string[];
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
requestInit?: RequestInit;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
}): Promise<Response> {
|
||||
const fetchFn = params.fetchFn ?? fetch;
|
||||
const resolveFn = params.resolveFn ?? lookup;
|
||||
const hasDispatcher = Boolean(
|
||||
params.requestInit &&
|
||||
@@ -574,7 +659,7 @@ export async function safeFetch(params: {
|
||||
}
|
||||
|
||||
// Authorization is only allowed on explicitly auth-allowlisted hosts, including
|
||||
// the first hop. Redirect hops apply the same rule below before following.
|
||||
// the first hop. Redirect hops apply the same rule below or in fetchWithSsrFGuard.
|
||||
if (
|
||||
currentHeaders.has("authorization") &&
|
||||
params.authorizationAllowHosts &&
|
||||
@@ -583,6 +668,28 @@ export async function safeFetch(params: {
|
||||
currentHeaders.delete("authorization");
|
||||
}
|
||||
|
||||
if (!hasDispatcher) {
|
||||
const guarded = await fetchWithSsrFGuard({
|
||||
url: currentUrl,
|
||||
fetchImpl: resolveGuardedFetchImpl({
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
}),
|
||||
init: {
|
||||
...params.requestInit,
|
||||
headers: currentHeaders,
|
||||
},
|
||||
maxRedirects: MAX_SAFE_REDIRECTS,
|
||||
requireHttps: true,
|
||||
policy: resolveMediaSsrfPolicy(params.allowHosts),
|
||||
lookupFn: resolveFn as LookupFn,
|
||||
retainAuthorizationRedirectHostnameAllowlist:
|
||||
resolveRetainedAuthorizationRedirectHostnameAllowlist(params.authorizationAllowHosts),
|
||||
auditContext: "msteams.attachment",
|
||||
});
|
||||
return responseWithRelease(guarded.response, guarded.release);
|
||||
}
|
||||
|
||||
if (resolveFn) {
|
||||
try {
|
||||
const initialHost = new URL(currentUrl).hostname;
|
||||
@@ -593,7 +700,7 @@ export async function safeFetch(params: {
|
||||
}
|
||||
|
||||
for (let i = 0; i <= MAX_SAFE_REDIRECTS; i++) {
|
||||
const res = await fetchFn(currentUrl, {
|
||||
const res = await (params.fetchFn ?? fetch)(currentUrl, {
|
||||
...params.requestInit,
|
||||
headers: currentHeaders,
|
||||
redirect: "manual",
|
||||
@@ -653,6 +760,7 @@ export async function safeFetchWithPolicy(params: {
|
||||
url: string;
|
||||
policy: MSTeamsAttachmentFetchPolicy;
|
||||
fetchFn?: typeof fetch;
|
||||
fetchFnSupportsDispatcher?: boolean;
|
||||
requestInit?: RequestInit;
|
||||
resolveFn?: MSTeamsAttachmentResolveFn;
|
||||
}): Promise<Response> {
|
||||
@@ -661,6 +769,7 @@ export async function safeFetchWithPolicy(params: {
|
||||
allowHosts: params.policy.allowHosts,
|
||||
authorizationAllowHosts: params.policy.authAllowHosts,
|
||||
fetchFn: params.fetchFn,
|
||||
fetchFnSupportsDispatcher: params.fetchFnSupportsDispatcher,
|
||||
requestInit: params.requestInit,
|
||||
resolveFn: params.resolveFn,
|
||||
});
|
||||
|
||||
@@ -946,6 +946,32 @@ describe("fetchWithSsrFGuard hardening", () => {
|
||||
await result.release();
|
||||
});
|
||||
|
||||
it("does not restore authorization across HTTPS-to-HTTP redirects", async () => {
|
||||
const lookupFn = createPublicLookup();
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(redirectResponse("http://cdn.example.com/asset"))
|
||||
.mockResolvedValueOnce(okResponse());
|
||||
|
||||
const result = await fetchWithSsrFGuard({
|
||||
url: "https://api.example.com/start",
|
||||
fetchImpl,
|
||||
lookupFn,
|
||||
init: {
|
||||
headers: {
|
||||
Authorization: "Bearer secret",
|
||||
Accept: "application/json",
|
||||
},
|
||||
},
|
||||
retainAuthorizationRedirectHostnameAllowlist: ["cdn.example.com"],
|
||||
});
|
||||
|
||||
const headers = getSecondRequestHeaders(fetchImpl);
|
||||
expect(headers.get("authorization")).toBeNull();
|
||||
expect(headers.get("accept")).toBe("application/json");
|
||||
await result.release();
|
||||
});
|
||||
|
||||
it("handles symbol-bearing header dictionaries while rewriting cross-origin redirects", async () => {
|
||||
const lookupFn = createPublicLookup();
|
||||
const fetchImpl = vi
|
||||
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
assertHostnameAllowedWithPolicy,
|
||||
closeDispatcher,
|
||||
createPinnedDispatcher,
|
||||
matchesHostnameAllowlist,
|
||||
resolveSsrFPolicyForUrl,
|
||||
resolvePinnedHostnameWithPolicy,
|
||||
type LookupFn,
|
||||
@@ -79,6 +80,7 @@ export type GuardedFetchOptions = {
|
||||
policy?: SsrFPolicy;
|
||||
lookupFn?: LookupFn;
|
||||
dispatcherPolicy?: PinnedDispatcherPolicy;
|
||||
retainAuthorizationRedirectHostnameAllowlist?: string[];
|
||||
mode?: GuardedFetchMode;
|
||||
pinDns?: boolean;
|
||||
/** @deprecated use `mode: "trusted_env_proxy"` for trusted/operator-controlled URLs. */
|
||||
@@ -295,6 +297,43 @@ function retainSafeHeadersForCrossOriginRedirect(init?: RequestInit): RequestIni
|
||||
return { ...init, headers: retainSafeRedirectHeaders(init.headers) };
|
||||
}
|
||||
|
||||
function resolveRetainedAuthorizationForRedirect(params: {
|
||||
init?: RequestInit;
|
||||
nextUrl: URL;
|
||||
hostnameAllowlist?: string[];
|
||||
}): string | undefined {
|
||||
const init = params.init;
|
||||
if (!init?.headers || !params.hostnameAllowlist?.length) {
|
||||
return undefined;
|
||||
}
|
||||
if (params.nextUrl.protocol !== "https:") {
|
||||
return undefined;
|
||||
}
|
||||
if (
|
||||
!params.hostnameAllowlist.includes("*") &&
|
||||
!matchesHostnameAllowlist(params.nextUrl.hostname, params.hostnameAllowlist)
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
const normalizedInit = normalizeRequestInitHeadersForFetch(init);
|
||||
if (!normalizedInit?.headers) {
|
||||
return undefined;
|
||||
}
|
||||
return new Headers(normalizedInit.headers).get("authorization") ?? undefined;
|
||||
}
|
||||
|
||||
function restoreRedirectAuthorization(params: {
|
||||
init?: RequestInit;
|
||||
authorization?: string;
|
||||
}): RequestInit | undefined {
|
||||
if (!params.authorization) {
|
||||
return params.init;
|
||||
}
|
||||
const headers = new Headers(params.init?.headers);
|
||||
headers.set("Authorization", params.authorization);
|
||||
return { ...params.init, headers };
|
||||
}
|
||||
|
||||
function dropBodyHeaders(headers?: HeadersInit): HeadersInit | undefined {
|
||||
if (!headers) {
|
||||
return headers;
|
||||
@@ -570,6 +609,11 @@ async function fetchWithSsrFGuardInternal(
|
||||
await release(dispatcher);
|
||||
throw new Error("Redirect loop detected");
|
||||
}
|
||||
const retainedAuthorization = resolveRetainedAuthorizationForRedirect({
|
||||
init: currentInit,
|
||||
nextUrl: nextParsedUrl,
|
||||
hostnameAllowlist: params.retainAuthorizationRedirectHostnameAllowlist,
|
||||
});
|
||||
currentInit = rewriteRedirectInitForMethod({ init: currentInit, status: response.status });
|
||||
if (nextParsedUrl.origin !== parsedUrl.origin) {
|
||||
currentInit = rewriteRedirectInitForCrossOrigin({
|
||||
@@ -577,6 +621,10 @@ async function fetchWithSsrFGuardInternal(
|
||||
allowUnsafeReplay: params.allowCrossOriginUnsafeRedirectReplay === true,
|
||||
});
|
||||
currentInit = retainSafeHeadersForCrossOriginRedirect(currentInit);
|
||||
currentInit = restoreRedirectAuthorization({
|
||||
init: currentInit,
|
||||
authorization: retainedAuthorization,
|
||||
});
|
||||
}
|
||||
visited.add(nextUrl);
|
||||
void response.body?.cancel();
|
||||
|
||||
@@ -335,11 +335,7 @@ function sanitizeJitiCachePathSegment(value) {
|
||||
|
||||
function resolveJitiFsCacheTmpDir() {
|
||||
let tmpDir = os.tmpdir();
|
||||
if (
|
||||
process.env.TMPDIR &&
|
||||
tmpDir === process.cwd() &&
|
||||
!process.env.JITI_RESPECT_TMPDIR_ENV
|
||||
) {
|
||||
if (process.env.TMPDIR && tmpDir === process.cwd() && !process.env.JITI_RESPECT_TMPDIR_ENV) {
|
||||
const originalTmpDir = process.env.TMPDIR;
|
||||
delete process.env.TMPDIR;
|
||||
try {
|
||||
|
||||
@@ -145,9 +145,7 @@ function loadRootAliasWithStubs(options?: {
|
||||
if (id === "node:os") {
|
||||
return {
|
||||
tmpdir: () =>
|
||||
context.process.env.TMPDIR ??
|
||||
options?.defaultTmpDir ??
|
||||
"/tmp/openclaw-root-alias-test",
|
||||
context.process.env.TMPDIR ?? options?.defaultTmpDir ?? "/tmp/openclaw-root-alias-test",
|
||||
};
|
||||
}
|
||||
if (id === "jiti") {
|
||||
|
||||
Reference in New Issue
Block a user