mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-28 17:43:05 +00:00
fix(telegram): unify transport fallback chain (#49148)
* fix(telegram): unify transport fallback chain * fix: address telegram fallback review comments * fix: validate pinned SSRF overrides * fix: unify telegram fallback retries (#49148)
This commit is contained in:
@@ -198,7 +198,7 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
|
||||
if (canUseTrustedEnvProxy) {
|
||||
dispatcher = new EnvHttpProxyAgent();
|
||||
} else if (params.pinDns !== false) {
|
||||
dispatcher = createPinnedDispatcher(pinned, params.dispatcherPolicy);
|
||||
dispatcher = createPinnedDispatcher(pinned, params.dispatcherPolicy, params.policy);
|
||||
}
|
||||
|
||||
const init: RequestInit & { dispatcher?: Dispatcher } = {
|
||||
|
||||
@@ -80,6 +80,58 @@ describe("createPinnedDispatcher", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("replaces the pinned lookup when a dispatcher override hostname is provided", () => {
|
||||
const originalLookup = vi.fn() as unknown as PinnedHostname["lookup"];
|
||||
const pinned: PinnedHostname = {
|
||||
hostname: "api.telegram.org",
|
||||
addresses: ["149.154.167.221"],
|
||||
lookup: originalLookup,
|
||||
};
|
||||
|
||||
createPinnedDispatcher(pinned, {
|
||||
mode: "direct",
|
||||
pinnedHostname: {
|
||||
hostname: "api.telegram.org",
|
||||
addresses: ["149.154.167.220"],
|
||||
},
|
||||
});
|
||||
|
||||
const firstCallArg = agentCtor.mock.calls.at(-1)?.[0] as
|
||||
| { connect?: { lookup?: PinnedHostname["lookup"] } }
|
||||
| undefined;
|
||||
expect(firstCallArg?.connect?.lookup).toBeTypeOf("function");
|
||||
|
||||
const lookup = firstCallArg?.connect?.lookup;
|
||||
const callback = vi.fn();
|
||||
lookup?.("api.telegram.org", callback);
|
||||
|
||||
expect(callback).toHaveBeenCalledWith(null, "149.154.167.220", 4);
|
||||
expect(originalLookup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects pinned override addresses that violate SSRF policy", () => {
|
||||
const originalLookup = vi.fn() as unknown as PinnedHostname["lookup"];
|
||||
const pinned: PinnedHostname = {
|
||||
hostname: "api.telegram.org",
|
||||
addresses: ["149.154.167.221"],
|
||||
lookup: originalLookup,
|
||||
};
|
||||
|
||||
expect(() =>
|
||||
createPinnedDispatcher(
|
||||
pinned,
|
||||
{
|
||||
mode: "direct",
|
||||
pinnedHostname: {
|
||||
hostname: "api.telegram.org",
|
||||
addresses: ["127.0.0.1"],
|
||||
},
|
||||
},
|
||||
undefined,
|
||||
),
|
||||
).toThrow(/private|internal|blocked/i);
|
||||
});
|
||||
|
||||
it("keeps env proxy route while pinning the direct no-proxy path", () => {
|
||||
const lookup = vi.fn() as unknown as PinnedHostname["lookup"];
|
||||
const pinned: PinnedHostname = {
|
||||
|
||||
@@ -99,6 +99,15 @@ describe("ssrf pinning", () => {
|
||||
expect(result.address).toBe("1.2.3.4");
|
||||
});
|
||||
|
||||
it("fails loud when a pinned lookup is created without any addresses", () => {
|
||||
expect(() =>
|
||||
createPinnedLookup({
|
||||
hostname: "example.com",
|
||||
addresses: [],
|
||||
}),
|
||||
).toThrow("Pinned lookup requires at least one address for example.com");
|
||||
});
|
||||
|
||||
it("enforces hostname allowlist when configured", async () => {
|
||||
const lookup = vi.fn(async () => [
|
||||
{ address: "93.184.216.34", family: 4 },
|
||||
|
||||
@@ -67,6 +67,13 @@ export function isPrivateNetworkAllowedByPolicy(policy?: SsrFPolicy): boolean {
|
||||
return policy?.dangerouslyAllowPrivateNetwork === true || policy?.allowPrivateNetwork === true;
|
||||
}
|
||||
|
||||
function shouldSkipPrivateNetworkChecks(hostname: string, policy?: SsrFPolicy): boolean {
|
||||
return (
|
||||
isPrivateNetworkAllowedByPolicy(policy) ||
|
||||
normalizeHostnameSet(policy?.allowedHostnames).has(hostname)
|
||||
);
|
||||
}
|
||||
|
||||
function resolveIpv4SpecialUseBlockOptions(policy?: SsrFPolicy): Ipv4SpecialUseBlockOptions {
|
||||
return {
|
||||
allowRfc2544BenchmarkRange: policy?.allowRfc2544BenchmarkRange === true,
|
||||
@@ -198,6 +205,9 @@ export function createPinnedLookup(params: {
|
||||
fallback?: typeof dnsLookupCb;
|
||||
}): typeof dnsLookupCb {
|
||||
const normalizedHost = normalizeHostname(params.hostname);
|
||||
if (params.addresses.length === 0) {
|
||||
throw new Error(`Pinned lookup requires at least one address for ${params.hostname}`);
|
||||
}
|
||||
const fallback = params.fallback ?? dnsLookupCb;
|
||||
const fallbackLookup = fallback as unknown as (
|
||||
hostname: string,
|
||||
@@ -255,20 +265,28 @@ export type PinnedHostname = {
|
||||
lookup: typeof dnsLookupCb;
|
||||
};
|
||||
|
||||
export type PinnedHostnameOverride = {
|
||||
hostname: string;
|
||||
addresses: string[];
|
||||
};
|
||||
|
||||
export type PinnedDispatcherPolicy =
|
||||
| {
|
||||
mode: "direct";
|
||||
connect?: Record<string, unknown>;
|
||||
pinnedHostname?: PinnedHostnameOverride;
|
||||
}
|
||||
| {
|
||||
mode: "env-proxy";
|
||||
connect?: Record<string, unknown>;
|
||||
proxyTls?: Record<string, unknown>;
|
||||
pinnedHostname?: PinnedHostnameOverride;
|
||||
}
|
||||
| {
|
||||
mode: "explicit-proxy";
|
||||
proxyUrl: string;
|
||||
proxyTls?: Record<string, unknown>;
|
||||
pinnedHostname?: PinnedHostnameOverride;
|
||||
};
|
||||
|
||||
function dedupeAndPreferIpv4(results: readonly LookupAddress[]): string[] {
|
||||
@@ -298,11 +316,8 @@ export async function resolvePinnedHostnameWithPolicy(
|
||||
throw new Error("Invalid hostname");
|
||||
}
|
||||
|
||||
const allowPrivateNetwork = isPrivateNetworkAllowedByPolicy(params.policy);
|
||||
const allowedHostnames = normalizeHostnameSet(params.policy?.allowedHostnames);
|
||||
const hostnameAllowlist = normalizeHostnameAllowlist(params.policy?.hostnameAllowlist);
|
||||
const isExplicitAllowed = allowedHostnames.has(normalized);
|
||||
const skipPrivateNetworkChecks = allowPrivateNetwork || isExplicitAllowed;
|
||||
const skipPrivateNetworkChecks = shouldSkipPrivateNetworkChecks(normalized, params.policy);
|
||||
|
||||
if (!matchesHostnameAllowlist(normalized, hostnameAllowlist)) {
|
||||
throw new SsrFBlockedError(`Blocked hostname (not in allowlist): ${hostname}`);
|
||||
@@ -352,19 +367,50 @@ function withPinnedLookup(
|
||||
return connect ? { ...connect, lookup } : { lookup };
|
||||
}
|
||||
|
||||
function resolvePinnedDispatcherLookup(
|
||||
pinned: PinnedHostname,
|
||||
override?: PinnedHostnameOverride,
|
||||
policy?: SsrFPolicy,
|
||||
): PinnedHostname["lookup"] {
|
||||
if (!override) {
|
||||
return pinned.lookup;
|
||||
}
|
||||
const normalizedOverrideHost = normalizeHostname(override.hostname);
|
||||
if (!normalizedOverrideHost || normalizedOverrideHost !== pinned.hostname) {
|
||||
throw new Error(
|
||||
`Pinned dispatcher override hostname mismatch: expected ${pinned.hostname}, got ${override.hostname}`,
|
||||
);
|
||||
}
|
||||
const records = override.addresses.map((address) => ({
|
||||
address,
|
||||
family: address.includes(":") ? 6 : 4,
|
||||
}));
|
||||
if (!shouldSkipPrivateNetworkChecks(pinned.hostname, policy)) {
|
||||
assertAllowedResolvedAddressesOrThrow(records, policy);
|
||||
}
|
||||
return createPinnedLookup({
|
||||
hostname: pinned.hostname,
|
||||
addresses: [...override.addresses],
|
||||
fallback: pinned.lookup,
|
||||
});
|
||||
}
|
||||
|
||||
export function createPinnedDispatcher(
|
||||
pinned: PinnedHostname,
|
||||
policy?: PinnedDispatcherPolicy,
|
||||
ssrfPolicy?: SsrFPolicy,
|
||||
): Dispatcher {
|
||||
const lookup = resolvePinnedDispatcherLookup(pinned, policy?.pinnedHostname, ssrfPolicy);
|
||||
|
||||
if (!policy || policy.mode === "direct") {
|
||||
return new Agent({
|
||||
connect: withPinnedLookup(pinned.lookup, policy?.connect),
|
||||
connect: withPinnedLookup(lookup, policy?.connect),
|
||||
});
|
||||
}
|
||||
|
||||
if (policy.mode === "env-proxy") {
|
||||
return new EnvHttpProxyAgent({
|
||||
connect: withPinnedLookup(pinned.lookup, policy.connect),
|
||||
connect: withPinnedLookup(lookup, policy.connect),
|
||||
...(policy.proxyTls ? { proxyTls: { ...policy.proxyTls } } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -22,7 +22,7 @@ vi.mock("undici", () => ({
|
||||
}));
|
||||
|
||||
let resolveTelegramTransport: typeof import("../../extensions/telegram/src/fetch.js").resolveTelegramTransport;
|
||||
let shouldRetryTelegramIpv4Fallback: typeof import("../../extensions/telegram/src/fetch.js").shouldRetryTelegramIpv4Fallback;
|
||||
let shouldRetryTelegramTransportFallback: typeof import("../../extensions/telegram/src/fetch.js").shouldRetryTelegramTransportFallback;
|
||||
let fetchRemoteMedia: typeof import("./fetch.js").fetchRemoteMedia;
|
||||
|
||||
describe("fetchRemoteMedia telegram network policy", () => {
|
||||
@@ -30,7 +30,7 @@ describe("fetchRemoteMedia telegram network policy", () => {
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.resetModules();
|
||||
({ resolveTelegramTransport, shouldRetryTelegramIpv4Fallback } =
|
||||
({ resolveTelegramTransport, shouldRetryTelegramTransportFallback } =
|
||||
await import("../../extensions/telegram/src/fetch.js"));
|
||||
({ fetchRemoteMedia } = await import("./fetch.js"));
|
||||
});
|
||||
@@ -70,7 +70,7 @@ describe("fetchRemoteMedia telegram network policy", () => {
|
||||
await fetchRemoteMedia({
|
||||
url: "https://api.telegram.org/file/bottok/photos/1.jpg",
|
||||
fetchImpl: telegramTransport.sourceFetch,
|
||||
dispatcherPolicy: telegramTransport.pinnedDispatcherPolicy,
|
||||
dispatcherAttempts: telegramTransport.dispatcherAttempts,
|
||||
lookupFn,
|
||||
maxBytes: 1024,
|
||||
ssrfPolicy: {
|
||||
@@ -120,7 +120,7 @@ describe("fetchRemoteMedia telegram network policy", () => {
|
||||
await fetchRemoteMedia({
|
||||
url: "https://api.telegram.org/file/bottok/files/1.pdf",
|
||||
fetchImpl: telegramTransport.sourceFetch,
|
||||
dispatcherPolicy: telegramTransport.pinnedDispatcherPolicy,
|
||||
dispatcherAttempts: telegramTransport.dispatcherAttempts,
|
||||
lookupFn,
|
||||
maxBytes: 1024,
|
||||
ssrfPolicy: {
|
||||
@@ -167,9 +167,8 @@ describe("fetchRemoteMedia telegram network policy", () => {
|
||||
await fetchRemoteMedia({
|
||||
url: "https://api.telegram.org/file/bottok/photos/2.jpg",
|
||||
fetchImpl: telegramTransport.sourceFetch,
|
||||
dispatcherPolicy: telegramTransport.pinnedDispatcherPolicy,
|
||||
fallbackDispatcherPolicy: telegramTransport.fallbackPinnedDispatcherPolicy,
|
||||
shouldRetryFetchError: shouldRetryTelegramIpv4Fallback,
|
||||
dispatcherAttempts: telegramTransport.dispatcherAttempts,
|
||||
shouldRetryFetchError: shouldRetryTelegramTransportFallback,
|
||||
lookupFn,
|
||||
maxBytes: 1024,
|
||||
ssrfPolicy: {
|
||||
@@ -214,14 +213,83 @@ describe("fetchRemoteMedia telegram network policy", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves both primary and fallback errors when Telegram media retry fails twice", async () => {
|
||||
it("retries Telegram file downloads with pinned Telegram IP after IPv4 fallback fails", async () => {
|
||||
const lookupFn = vi.fn(async () => [
|
||||
{ address: "149.154.167.221", family: 4 },
|
||||
{ address: "2001:67c:4e8:f004::9", family: 6 },
|
||||
]) as unknown as LookupFn;
|
||||
undiciMocks.fetch
|
||||
.mockRejectedValueOnce(createTelegramFetchFailedError("EHOSTUNREACH"))
|
||||
.mockRejectedValueOnce(createTelegramFetchFailedError("ETIMEDOUT"))
|
||||
.mockResolvedValueOnce(
|
||||
new Response(new Uint8Array([0xff, 0xd8, 0xff, 0x00]), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
}),
|
||||
);
|
||||
|
||||
const telegramTransport = resolveTelegramTransport(undefined, {
|
||||
network: {
|
||||
autoSelectFamily: true,
|
||||
dnsResultOrder: "ipv4first",
|
||||
},
|
||||
});
|
||||
|
||||
await fetchRemoteMedia({
|
||||
url: "https://api.telegram.org/file/bottok/photos/3.jpg",
|
||||
fetchImpl: telegramTransport.sourceFetch,
|
||||
dispatcherAttempts: telegramTransport.dispatcherAttempts,
|
||||
shouldRetryFetchError: shouldRetryTelegramTransportFallback,
|
||||
lookupFn,
|
||||
maxBytes: 1024,
|
||||
ssrfPolicy: {
|
||||
allowedHostnames: ["api.telegram.org"],
|
||||
allowRfc2544BenchmarkRange: true,
|
||||
},
|
||||
});
|
||||
|
||||
const thirdInit = undiciMocks.fetch.mock.calls[2]?.[1] as
|
||||
| (RequestInit & {
|
||||
dispatcher?: {
|
||||
options?: {
|
||||
connect?: Record<string, unknown>;
|
||||
};
|
||||
};
|
||||
})
|
||||
| undefined;
|
||||
const callback = vi.fn();
|
||||
(
|
||||
thirdInit?.dispatcher?.options?.connect?.lookup as
|
||||
| ((
|
||||
hostname: string,
|
||||
callback: (err: null, address: string, family: number) => void,
|
||||
) => void)
|
||||
| undefined
|
||||
)?.("api.telegram.org", callback);
|
||||
|
||||
expect(undiciMocks.fetch).toHaveBeenCalledTimes(3);
|
||||
expect(thirdInit?.dispatcher?.options?.connect).toEqual(
|
||||
expect.objectContaining({
|
||||
family: 4,
|
||||
autoSelectFamily: false,
|
||||
lookup: expect.any(Function),
|
||||
}),
|
||||
);
|
||||
expect(callback).toHaveBeenCalledWith(null, "149.154.167.220", 4);
|
||||
});
|
||||
|
||||
it("preserves both primary and final fallback errors when Telegram media retry chain fails", async () => {
|
||||
const lookupFn = vi.fn(async () => [
|
||||
{ address: "149.154.167.220", family: 4 },
|
||||
{ address: "2001:67c:4e8:f004::9", family: 6 },
|
||||
]) as unknown as LookupFn;
|
||||
const primaryError = createTelegramFetchFailedError("EHOSTUNREACH");
|
||||
const ipv4Error = createTelegramFetchFailedError("ETIMEDOUT");
|
||||
const fallbackError = createTelegramFetchFailedError("ETIMEDOUT");
|
||||
undiciMocks.fetch.mockRejectedValueOnce(primaryError).mockRejectedValueOnce(fallbackError);
|
||||
undiciMocks.fetch
|
||||
.mockRejectedValueOnce(primaryError)
|
||||
.mockRejectedValueOnce(ipv4Error)
|
||||
.mockRejectedValueOnce(fallbackError);
|
||||
|
||||
const telegramTransport = resolveTelegramTransport(undefined, {
|
||||
network: {
|
||||
@@ -232,11 +300,10 @@ describe("fetchRemoteMedia telegram network policy", () => {
|
||||
|
||||
await expect(
|
||||
fetchRemoteMedia({
|
||||
url: "https://api.telegram.org/file/bottok/photos/3.jpg",
|
||||
url: "https://api.telegram.org/file/bottok/photos/4.jpg",
|
||||
fetchImpl: telegramTransport.sourceFetch,
|
||||
dispatcherPolicy: telegramTransport.pinnedDispatcherPolicy,
|
||||
fallbackDispatcherPolicy: telegramTransport.fallbackPinnedDispatcherPolicy,
|
||||
shouldRetryFetchError: shouldRetryTelegramIpv4Fallback,
|
||||
dispatcherAttempts: telegramTransport.dispatcherAttempts,
|
||||
shouldRetryFetchError: shouldRetryTelegramTransportFallback,
|
||||
lookupFn,
|
||||
maxBytes: 1024,
|
||||
ssrfPolicy: {
|
||||
@@ -250,6 +317,7 @@ describe("fetchRemoteMedia telegram network policy", () => {
|
||||
cause: expect.objectContaining({
|
||||
name: "Error",
|
||||
cause: fallbackError,
|
||||
attemptErrors: [primaryError, ipv4Error, fallbackError],
|
||||
primaryError,
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -26,6 +26,11 @@ export class MediaFetchError extends Error {
|
||||
|
||||
export type FetchLike = (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
|
||||
|
||||
export type FetchDispatcherAttempt = {
|
||||
dispatcherPolicy?: PinnedDispatcherPolicy;
|
||||
lookupFn?: LookupFn;
|
||||
};
|
||||
|
||||
type FetchMediaOptions = {
|
||||
url: string;
|
||||
fetchImpl?: FetchLike;
|
||||
@@ -37,8 +42,7 @@ type FetchMediaOptions = {
|
||||
readIdleTimeoutMs?: number;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
lookupFn?: LookupFn;
|
||||
dispatcherPolicy?: PinnedDispatcherPolicy;
|
||||
fallbackDispatcherPolicy?: PinnedDispatcherPolicy;
|
||||
dispatcherAttempts?: FetchDispatcherAttempt[];
|
||||
shouldRetryFetchError?: (error: unknown) => boolean;
|
||||
};
|
||||
|
||||
@@ -101,8 +105,7 @@ export async function fetchRemoteMedia(options: FetchMediaOptions): Promise<Fetc
|
||||
readIdleTimeoutMs,
|
||||
ssrfPolicy,
|
||||
lookupFn,
|
||||
dispatcherPolicy,
|
||||
fallbackDispatcherPolicy,
|
||||
dispatcherAttempts,
|
||||
shouldRetryFetchError,
|
||||
} = options;
|
||||
const sourceUrl = redactMediaUrl(url);
|
||||
@@ -110,7 +113,11 @@ export async function fetchRemoteMedia(options: FetchMediaOptions): Promise<Fetc
|
||||
let res: Response;
|
||||
let finalUrl = url;
|
||||
let release: (() => Promise<void>) | null = null;
|
||||
const runGuardedFetch = async (policy?: PinnedDispatcherPolicy) =>
|
||||
const attempts =
|
||||
dispatcherAttempts && dispatcherAttempts.length > 0
|
||||
? dispatcherAttempts
|
||||
: [{ dispatcherPolicy: undefined, lookupFn }];
|
||||
const runGuardedFetch = async (attempt: FetchDispatcherAttempt) =>
|
||||
await fetchWithSsrFGuard(
|
||||
withStrictGuardedFetchMode({
|
||||
url,
|
||||
@@ -118,32 +125,43 @@ export async function fetchRemoteMedia(options: FetchMediaOptions): Promise<Fetc
|
||||
init: requestInit,
|
||||
maxRedirects,
|
||||
policy: ssrfPolicy,
|
||||
lookupFn,
|
||||
dispatcherPolicy: policy,
|
||||
lookupFn: attempt.lookupFn ?? lookupFn,
|
||||
dispatcherPolicy: attempt.dispatcherPolicy,
|
||||
}),
|
||||
);
|
||||
try {
|
||||
let result;
|
||||
try {
|
||||
result = await runGuardedFetch(dispatcherPolicy);
|
||||
} catch (err) {
|
||||
if (
|
||||
fallbackDispatcherPolicy &&
|
||||
typeof shouldRetryFetchError === "function" &&
|
||||
shouldRetryFetchError(err)
|
||||
) {
|
||||
try {
|
||||
result = await runGuardedFetch(fallbackDispatcherPolicy);
|
||||
} catch (fallbackErr) {
|
||||
const combined = new Error(
|
||||
`Primary fetch failed and fallback fetch also failed for ${sourceUrl}`,
|
||||
{ cause: fallbackErr },
|
||||
);
|
||||
(combined as Error & { primaryError?: unknown }).primaryError = err;
|
||||
throw combined;
|
||||
let result!: Awaited<ReturnType<typeof fetchWithSsrFGuard>>;
|
||||
const attemptErrors: unknown[] = [];
|
||||
for (let i = 0; i < attempts.length; i += 1) {
|
||||
try {
|
||||
result = await runGuardedFetch(attempts[i]);
|
||||
break;
|
||||
} catch (err) {
|
||||
if (
|
||||
typeof shouldRetryFetchError !== "function" ||
|
||||
!shouldRetryFetchError(err) ||
|
||||
i === attempts.length - 1
|
||||
) {
|
||||
if (attemptErrors.length > 0) {
|
||||
const combined = new Error(
|
||||
`Primary fetch failed and fallback fetch also failed for ${sourceUrl}`,
|
||||
{ cause: err },
|
||||
);
|
||||
(
|
||||
combined as Error & {
|
||||
primaryError?: unknown;
|
||||
attemptErrors?: unknown[];
|
||||
}
|
||||
).primaryError = attemptErrors[0];
|
||||
(combined as Error & { attemptErrors?: unknown[] }).attemptErrors = [
|
||||
...attemptErrors,
|
||||
err,
|
||||
];
|
||||
throw combined;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
} else {
|
||||
throw err;
|
||||
attemptErrors.push(err);
|
||||
}
|
||||
}
|
||||
res = result.response;
|
||||
|
||||
Reference in New Issue
Block a user