mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(msteams): preserve guarded dispatcher redirects
This commit is contained in:
committed by
Peter Steinberger
parent
cceecc8bd4
commit
c582a54554
@@ -1,4 +1,3 @@
|
||||
import { fetchWithBearerAuthScopeFallback } from "openclaw/plugin-sdk";
|
||||
import { getMSTeamsRuntime } from "../runtime.js";
|
||||
import { downloadAndStoreMSTeamsRemoteMedia } from "./remote-media.js";
|
||||
import {
|
||||
@@ -12,6 +11,7 @@ import {
|
||||
resolveRequestUrl,
|
||||
resolveAuthAllowedHosts,
|
||||
resolveAllowedHosts,
|
||||
safeFetch,
|
||||
} from "./shared.js";
|
||||
import type {
|
||||
MSTeamsAccessTokenProvider,
|
||||
@@ -91,16 +91,14 @@ async function fetchWithAuthFallback(params: {
|
||||
tokenProvider?: MSTeamsAccessTokenProvider;
|
||||
fetchFn?: typeof fetch;
|
||||
requestInit?: RequestInit;
|
||||
allowHosts: string[];
|
||||
authAllowHosts: string[];
|
||||
}): Promise<Response> {
|
||||
return await fetchWithBearerAuthScopeFallback({
|
||||
const firstAttempt = await safeFetch({
|
||||
url: params.url,
|
||||
scopes: scopeCandidatesForUrl(params.url),
|
||||
tokenProvider: params.tokenProvider,
|
||||
allowHosts: params.allowHosts,
|
||||
fetchFn: params.fetchFn,
|
||||
requestInit: params.requestInit,
|
||||
requireHttps: true,
|
||||
shouldAttachAuth: (url) => isUrlAllowed(url, params.authAllowHosts),
|
||||
});
|
||||
if (firstAttempt.ok) {
|
||||
return firstAttempt;
|
||||
@@ -116,6 +114,7 @@ async function fetchWithAuthFallback(params: {
|
||||
}
|
||||
|
||||
const scopes = scopeCandidatesForUrl(params.url);
|
||||
const fetchFn = params.fetchFn ?? fetch;
|
||||
for (const scope of scopes) {
|
||||
try {
|
||||
const token = await params.tokenProvider.getAccessToken(scope);
|
||||
@@ -129,7 +128,6 @@ async function fetchWithAuthFallback(params: {
|
||||
...params.requestInit,
|
||||
headers: authHeaders,
|
||||
},
|
||||
resolveFn: params.resolveFn,
|
||||
});
|
||||
if (authAttempt.ok) {
|
||||
return authAttempt;
|
||||
@@ -139,25 +137,6 @@ async function fetchWithAuthFallback(params: {
|
||||
// be handled by the caller's redirect/error policy.
|
||||
return authAttempt;
|
||||
}
|
||||
|
||||
const finalUrl =
|
||||
typeof authAttempt.url === "string" && authAttempt.url ? authAttempt.url : "";
|
||||
if (!finalUrl || finalUrl === params.url || !isUrlAllowed(finalUrl, params.authAllowHosts)) {
|
||||
continue;
|
||||
}
|
||||
const redirectedAuthAttempt = await safeFetch({
|
||||
url: finalUrl,
|
||||
allowHosts: params.allowHosts,
|
||||
fetchFn,
|
||||
requestInit: {
|
||||
...params.requestInit,
|
||||
headers: authHeaders,
|
||||
},
|
||||
resolveFn: params.resolveFn,
|
||||
});
|
||||
if (redirectedAuthAttempt.ok) {
|
||||
return redirectedAuthAttempt;
|
||||
}
|
||||
} catch {
|
||||
// Try the next scope.
|
||||
}
|
||||
@@ -262,6 +241,7 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
tokenProvider: params.tokenProvider,
|
||||
fetchFn: params.fetchFn,
|
||||
requestInit: init,
|
||||
allowHosts,
|
||||
authAllowHosts,
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -1,11 +1,33 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
isPrivateOrReservedIP,
|
||||
isUrlAllowed,
|
||||
resolveAndValidateIP,
|
||||
resolveAllowedHosts,
|
||||
resolveAuthAllowedHosts,
|
||||
resolveMediaSsrfPolicy,
|
||||
safeFetch,
|
||||
} from "./shared.js";
|
||||
|
||||
const publicResolve = async () => ({ address: "13.107.136.10" });
|
||||
const privateResolve = (ip: string) => async () => ({ address: ip });
|
||||
const failingResolve = async () => {
|
||||
throw new Error("DNS failure");
|
||||
};
|
||||
|
||||
function mockFetchWithRedirect(redirectMap: Record<string, string>, finalBody = "ok") {
|
||||
return vi.fn(async (url: string, init?: RequestInit) => {
|
||||
const target = redirectMap[url];
|
||||
if (target && init?.redirect === "manual") {
|
||||
return new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: target },
|
||||
});
|
||||
}
|
||||
return new Response(finalBody, { status: 200 });
|
||||
});
|
||||
}
|
||||
|
||||
describe("msteams attachment allowlists", () => {
|
||||
it("normalizes wildcard host lists", () => {
|
||||
expect(resolveAllowedHosts(["*", "graph.microsoft.com"])).toEqual(["*"]);
|
||||
@@ -19,6 +41,13 @@ describe("msteams attachment allowlists", () => {
|
||||
expect(isUrlAllowed("https://evil.example.com/file.png", allowHosts)).toBe(false);
|
||||
});
|
||||
|
||||
it("builds shared SSRF policy from suffix allowlist", () => {
|
||||
expect(resolveMediaSsrfPolicy(["sharepoint.com"])).toEqual({
|
||||
hostnameAllowlist: ["sharepoint.com", "*.sharepoint.com"],
|
||||
});
|
||||
expect(resolveMediaSsrfPolicy(["*"])).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["999.999.999.999", true],
|
||||
["256.0.0.1", true],
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { lookup } from "node:dns/promises";
|
||||
import {
|
||||
buildHostnameAllowlistPolicyFromSuffixAllowlist,
|
||||
isHttpsUrlAllowedByHostnameSuffixAllowlist,
|
||||
isPrivateIpAddress,
|
||||
normalizeHostnameSuffixAllowlist,
|
||||
} from "openclaw/plugin-sdk";
|
||||
import type { SsrFPolicy } from "openclaw/plugin-sdk";
|
||||
@@ -268,6 +270,10 @@ export function isUrlAllowed(url: string, allowlist: string[]): boolean {
|
||||
return isHttpsUrlAllowedByHostnameSuffixAllowlist(url, allowlist);
|
||||
}
|
||||
|
||||
export function resolveMediaSsrfPolicy(allowHosts: string[]): SsrFPolicy | undefined {
|
||||
return buildHostnameAllowlistPolicyFromSuffixAllowlist(allowHosts);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if the given IPv4 or IPv6 address is in a private, loopback,
|
||||
* or link-local range that must never be reached from media downloads.
|
||||
@@ -304,11 +310,11 @@ const MAX_SAFE_REDIRECTS = 5;
|
||||
|
||||
/**
|
||||
* Fetch a URL with redirect: "manual", validating each redirect target
|
||||
* against the hostname allowlist and DNS-resolved IP (anti-SSRF).
|
||||
* against the hostname allowlist and optional DNS-resolved IP (anti-SSRF).
|
||||
*
|
||||
* This prevents:
|
||||
* - Auto-following redirects to non-allowlisted hosts
|
||||
* - DNS rebinding attacks where an allowlisted domain resolves to a private IP
|
||||
* - DNS rebinding attacks when a lookup function is provided
|
||||
*/
|
||||
export async function safeFetch(params: {
|
||||
url: string;
|
||||
@@ -326,14 +332,19 @@ export async function safeFetch(params: {
|
||||
);
|
||||
let currentUrl = params.url;
|
||||
|
||||
// Validate the initial URL's resolved IP
|
||||
try {
|
||||
const initialHost = new URL(currentUrl).hostname;
|
||||
await resolveAndValidateIP(initialHost, resolveFn);
|
||||
} catch {
|
||||
if (!isUrlAllowed(currentUrl, params.allowHosts)) {
|
||||
throw new Error(`Initial download URL blocked: ${currentUrl}`);
|
||||
}
|
||||
|
||||
if (resolveFn) {
|
||||
try {
|
||||
const initialHost = new URL(currentUrl).hostname;
|
||||
await resolveAndValidateIP(initialHost, resolveFn);
|
||||
} catch {
|
||||
throw new Error(`Initial download URL blocked: ${currentUrl}`);
|
||||
}
|
||||
}
|
||||
|
||||
for (let i = 0; i <= MAX_SAFE_REDIRECTS; i++) {
|
||||
const res = await fetchFn(currentUrl, {
|
||||
...params.requestInit,
|
||||
@@ -369,8 +380,10 @@ export async function safeFetch(params: {
|
||||
}
|
||||
|
||||
// Validate redirect target's resolved IP
|
||||
const redirectHost = new URL(redirectUrl).hostname;
|
||||
await resolveAndValidateIP(redirectHost, resolveFn);
|
||||
if (resolveFn) {
|
||||
const redirectHost = new URL(redirectUrl).hostname;
|
||||
await resolveAndValidateIP(redirectHost, resolveFn);
|
||||
}
|
||||
|
||||
currentUrl = redirectUrl;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user