mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 12:00:44 +00:00
fix(telegram): trust explicit proxy DNS for media downloads (#66461)
This commit is contained in:
@@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Browser/SSRF: preserve explicit strict browser navigation mode for legacy `browser.ssrfPolicy.allowPrivateNetwork: false` configs by normalizing the legacy alias to the canonical strict marker instead of silently widening those installs to the default non-strict hostname-navigation path.
|
||||
- Agents/subagents: emit the subagent registry lazy-runtime stub on the stable dist path that both source and bundled runtime imports resolve, so the follow-up dist fix no longer still fails with `ERR_MODULE_NOT_FOUND` at runtime. (#66420) Thanks @obviyus.
|
||||
- Media-understanding/proxy env: auto-upgrade provider HTTP helper requests to trusted env-proxy mode only when `HTTP_PROXY`/`HTTPS_PROXY` is active and the target is not bypassed by `NO_PROXY`, so remote media-understanding and transcription requests stop failing local DNS pre-resolution in proxy-only environments without widening SSRF bypasses. (#52162) Thanks @mjamiv and @vincentkoc.
|
||||
- Telegram/media downloads: let Telegram media fetches trust an operator-configured explicit proxy for target DNS resolution after hostname-policy checks, so proxy-backed installs stop failing `could not download media` on Bot API file downloads after the DNS-pinning regression. (#66245) Thanks @dawei41468 and @vincentkoc.
|
||||
- Browser: keep loopback CDP readiness checks reachable under strict SSRF defaults so OpenClaw can reconnect to locally started managed Chrome. (#66354) Thanks @hxy91819.
|
||||
- Agents/context engine: compact engine-owned sessions from the first tool-loop delta and preserve ingest fallback when `afterTurn` is absent, so long-running tool loops can stay bounded without dropping engine state. (#63555) Thanks @Bikkies.
|
||||
- Discord/native commands: return the real status card for native `/status` interactions instead of falling through to the synthetic `✅ Done.` ack when the generic dispatcher produces no visible reply. (#54629) Thanks @tkozzer and @vincentkoc.
|
||||
|
||||
@@ -332,7 +332,15 @@ describe("resolveMedia getFile retry", () => {
|
||||
it("uses caller-provided fetch impl for file downloads", async () => {
|
||||
const getFile = vi.fn().mockResolvedValue({ file_path: "documents/file_42.pdf" });
|
||||
const callerFetch = vi.fn() as unknown as typeof fetch;
|
||||
const dispatcherAttempts = [{ dispatcherPolicy: { mode: "direct" as const } }];
|
||||
const dispatcherAttempts = [
|
||||
{
|
||||
dispatcherPolicy: {
|
||||
mode: "explicit-proxy" as const,
|
||||
proxyUrl: "http://localhost:6152",
|
||||
allowPrivateProxy: true,
|
||||
},
|
||||
},
|
||||
];
|
||||
const callerTransport = {
|
||||
fetch: callerFetch,
|
||||
sourceFetch: callerFetch,
|
||||
@@ -357,6 +365,7 @@ describe("resolveMedia getFile retry", () => {
|
||||
expect.objectContaining({
|
||||
fetchImpl: callerFetch,
|
||||
dispatcherAttempts,
|
||||
trustExplicitProxyDns: true,
|
||||
shouldRetryFetchError: expect.any(Function),
|
||||
readIdleTimeoutMs: 30_000,
|
||||
ssrfPolicy: {
|
||||
|
||||
@@ -157,6 +157,14 @@ function resolveRequiredTelegramTransport(transport?: TelegramTransport): Telegr
|
||||
/** Default idle timeout for Telegram media downloads (30 seconds). */
|
||||
const TELEGRAM_DOWNLOAD_IDLE_TIMEOUT_MS = 30_000;
|
||||
|
||||
function usesTrustedTelegramExplicitProxy(transport: TelegramTransport): boolean {
|
||||
return (
|
||||
transport.dispatcherAttempts?.some(
|
||||
(attempt) => attempt.dispatcherPolicy?.mode === "explicit-proxy",
|
||||
) ?? false
|
||||
);
|
||||
}
|
||||
|
||||
function resolveTrustedLocalTelegramRoot(
|
||||
filePath: string,
|
||||
trustedLocalFileRoots?: readonly string[],
|
||||
@@ -225,6 +233,7 @@ async function downloadAndSaveTelegramFile(params: {
|
||||
url,
|
||||
fetchImpl: transport.sourceFetch,
|
||||
dispatcherAttempts: transport.dispatcherAttempts,
|
||||
trustExplicitProxyDns: usesTrustedTelegramExplicitProxy(transport),
|
||||
shouldRetryFetchError: shouldRetryTelegramTransportFallback,
|
||||
filePathHint: params.filePath,
|
||||
maxBytes: params.maxBytes,
|
||||
|
||||
@@ -1018,6 +1018,75 @@ describe("fetchWithSsrFGuard hardening", () => {
|
||||
await result.release();
|
||||
});
|
||||
|
||||
it("skips target DNS pinning in trusted explicit-proxy mode after hostname-policy checks", async () => {
|
||||
(globalThis as Record<string, unknown>)[TEST_UNDICI_RUNTIME_DEPS_KEY] = {
|
||||
Agent: agentCtor,
|
||||
EnvHttpProxyAgent: envHttpProxyAgentCtor,
|
||||
ProxyAgent: proxyAgentCtor,
|
||||
fetch: vi.fn(async () => okResponse()),
|
||||
};
|
||||
const lookupFn: LookupFn = vi.fn(async (hostname: string) => {
|
||||
if (hostname === "localhost") {
|
||||
return [{ address: "127.0.0.1", family: 4 }];
|
||||
}
|
||||
throw new Error(`unexpected target DNS lookup for ${hostname}`);
|
||||
}) as unknown as LookupFn;
|
||||
const fetchImpl = vi.fn(async () => okResponse());
|
||||
|
||||
const result = await fetchWithSsrFGuard({
|
||||
url: "https://api.telegram.org/file/bot123/photos/test.jpg",
|
||||
fetchImpl,
|
||||
lookupFn,
|
||||
mode: GUARDED_FETCH_MODE.TRUSTED_EXPLICIT_PROXY,
|
||||
policy: { hostnameAllowlist: ["api.telegram.org"] },
|
||||
dispatcherPolicy: {
|
||||
mode: "explicit-proxy",
|
||||
proxyUrl: "http://localhost:6152",
|
||||
allowPrivateProxy: true,
|
||||
},
|
||||
});
|
||||
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(1);
|
||||
expect(lookupFn).toHaveBeenCalledOnce();
|
||||
expect(lookupFn).toHaveBeenCalledWith("localhost", { all: true });
|
||||
await result.release();
|
||||
});
|
||||
|
||||
it("still blocks off-allowlist targets in trusted explicit-proxy mode", async () => {
|
||||
(globalThis as Record<string, unknown>)[TEST_UNDICI_RUNTIME_DEPS_KEY] = {
|
||||
Agent: agentCtor,
|
||||
EnvHttpProxyAgent: envHttpProxyAgentCtor,
|
||||
ProxyAgent: proxyAgentCtor,
|
||||
fetch: vi.fn(async () => okResponse()),
|
||||
};
|
||||
const lookupFn: LookupFn = vi.fn(async (hostname: string) => {
|
||||
if (hostname === "localhost") {
|
||||
return [{ address: "127.0.0.1", family: 4 }];
|
||||
}
|
||||
throw new Error(`unexpected target DNS lookup for ${hostname}`);
|
||||
}) as unknown as LookupFn;
|
||||
const fetchImpl = vi.fn(async () => okResponse());
|
||||
|
||||
await expect(
|
||||
fetchWithSsrFGuard({
|
||||
url: "https://cdn.telegram.org/file/bot123/photos/test.jpg",
|
||||
fetchImpl,
|
||||
lookupFn,
|
||||
mode: GUARDED_FETCH_MODE.TRUSTED_EXPLICIT_PROXY,
|
||||
policy: { hostnameAllowlist: ["api.telegram.org"] },
|
||||
dispatcherPolicy: {
|
||||
mode: "explicit-proxy",
|
||||
proxyUrl: "http://localhost:6152",
|
||||
allowPrivateProxy: true,
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow(/allowlist|blocked/i);
|
||||
|
||||
expect(fetchImpl).not.toHaveBeenCalled();
|
||||
expect(lookupFn).toHaveBeenCalledOnce();
|
||||
expect(lookupFn).toHaveBeenCalledWith("localhost", { all: true });
|
||||
});
|
||||
|
||||
it("still blocks explicit proxy on localhost when allowPrivateProxy is false", async () => {
|
||||
(globalThis as Record<string, unknown>)[TEST_UNDICI_RUNTIME_DEPS_KEY] = {
|
||||
Agent: agentCtor,
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
type DispatcherAwareRequestInit,
|
||||
} from "./runtime-fetch.js";
|
||||
import {
|
||||
assertHostnameAllowedWithPolicy,
|
||||
closeDispatcher,
|
||||
createPinnedDispatcher,
|
||||
resolvePinnedHostnameWithPolicy,
|
||||
@@ -29,6 +30,7 @@ type FetchLike = (input: RequestInfo | URL, init?: RequestInit) => Promise<Respo
|
||||
export const GUARDED_FETCH_MODE = {
|
||||
STRICT: "strict",
|
||||
TRUSTED_ENV_PROXY: "trusted_env_proxy",
|
||||
TRUSTED_EXPLICIT_PROXY: "trusted_explicit_proxy",
|
||||
} as const;
|
||||
|
||||
export type GuardedFetchMode = (typeof GUARDED_FETCH_MODE)[keyof typeof GUARDED_FETCH_MODE];
|
||||
@@ -89,6 +91,12 @@ export function withTrustedEnvProxyGuardedFetchMode(
|
||||
return { ...params, mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY };
|
||||
}
|
||||
|
||||
export function withTrustedExplicitProxyGuardedFetchMode(
|
||||
params: GuardedFetchPresetOptions,
|
||||
): GuardedFetchOptions {
|
||||
return { ...params, mode: GUARDED_FETCH_MODE.TRUSTED_EXPLICIT_PROXY };
|
||||
}
|
||||
|
||||
function resolveGuardedFetchMode(params: GuardedFetchOptions): GuardedFetchMode {
|
||||
if (params.mode) {
|
||||
return params.mode;
|
||||
@@ -318,12 +326,24 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
|
||||
|
||||
let dispatcher: Dispatcher | null = null;
|
||||
try {
|
||||
assertExplicitProxySupportsPinnedDns(parsedUrl, params.dispatcherPolicy, params.pinDns);
|
||||
const usesTrustedExplicitProxyMode =
|
||||
mode === GUARDED_FETCH_MODE.TRUSTED_EXPLICIT_PROXY &&
|
||||
params.dispatcherPolicy?.mode === "explicit-proxy";
|
||||
assertExplicitProxySupportsPinnedDns(
|
||||
parsedUrl,
|
||||
params.dispatcherPolicy,
|
||||
usesTrustedExplicitProxyMode ? false : params.pinDns,
|
||||
);
|
||||
await assertExplicitProxyAllowed(params.dispatcherPolicy, params.lookupFn, params.policy);
|
||||
const canUseTrustedEnvProxy =
|
||||
mode === GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY && hasProxyEnvConfigured();
|
||||
if (canUseTrustedEnvProxy) {
|
||||
dispatcher = createHttp1EnvHttpProxyAgent();
|
||||
} else if (usesTrustedExplicitProxyMode) {
|
||||
// Explicit proxy targets are still checked against the caller's hostname
|
||||
// policy, but the proxy does the DNS resolution for the final target.
|
||||
assertHostnameAllowedWithPolicy(parsedUrl.hostname, params.policy);
|
||||
dispatcher = createPolicyDispatcherWithoutPinnedDns(params.dispatcherPolicy);
|
||||
} else if (params.pinDns === false) {
|
||||
await resolvePinnedHostnameWithPolicy(parsedUrl.hostname, {
|
||||
lookupFn: params.lookupFn,
|
||||
|
||||
@@ -191,6 +191,33 @@ function assertAllowedHostOrIpOrThrow(hostnameOrIp: string, policy?: SsrFPolicy)
|
||||
}
|
||||
}
|
||||
|
||||
function resolveHostnamePolicyChecks(
|
||||
hostname: string,
|
||||
policy?: SsrFPolicy,
|
||||
): {
|
||||
normalized: string;
|
||||
skipPrivateNetworkChecks: boolean;
|
||||
} {
|
||||
const normalized = normalizeHostname(hostname);
|
||||
if (!normalized) {
|
||||
throw new Error("Invalid hostname");
|
||||
}
|
||||
|
||||
const hostnameAllowlist = normalizeHostnameAllowlist(policy?.hostnameAllowlist);
|
||||
const skipPrivateNetworkChecks = shouldSkipPrivateNetworkChecks(normalized, policy);
|
||||
|
||||
if (!matchesHostnameAllowlist(normalized, hostnameAllowlist)) {
|
||||
throw new SsrFBlockedError(`Blocked hostname (not in allowlist): ${hostname}`);
|
||||
}
|
||||
|
||||
if (!skipPrivateNetworkChecks) {
|
||||
// Fail fast for literal hosts/IPs before any DNS lookup side-effects.
|
||||
assertAllowedHostOrIpOrThrow(normalized, policy);
|
||||
}
|
||||
|
||||
return { normalized, skipPrivateNetworkChecks };
|
||||
}
|
||||
|
||||
function assertAllowedResolvedAddressesOrThrow(
|
||||
results: readonly LookupAddress[],
|
||||
policy?: SsrFPolicy,
|
||||
@@ -323,22 +350,10 @@ export async function resolvePinnedHostnameWithPolicy(
|
||||
hostname: string,
|
||||
params: { lookupFn?: LookupFn; policy?: SsrFPolicy } = {},
|
||||
): Promise<PinnedHostname> {
|
||||
const normalized = normalizeHostname(hostname);
|
||||
if (!normalized) {
|
||||
throw new Error("Invalid hostname");
|
||||
}
|
||||
|
||||
const hostnameAllowlist = normalizeHostnameAllowlist(params.policy?.hostnameAllowlist);
|
||||
const skipPrivateNetworkChecks = shouldSkipPrivateNetworkChecks(normalized, params.policy);
|
||||
|
||||
if (!matchesHostnameAllowlist(normalized, hostnameAllowlist)) {
|
||||
throw new SsrFBlockedError(`Blocked hostname (not in allowlist): ${hostname}`);
|
||||
}
|
||||
|
||||
if (!skipPrivateNetworkChecks) {
|
||||
// Phase 1: fail fast for literal hosts/IPs before any DNS lookup side-effects.
|
||||
assertAllowedHostOrIpOrThrow(normalized, params.policy);
|
||||
}
|
||||
const { normalized, skipPrivateNetworkChecks } = resolveHostnamePolicyChecks(
|
||||
hostname,
|
||||
params.policy,
|
||||
);
|
||||
|
||||
const lookupFn = params.lookupFn ?? dnsLookup;
|
||||
const results = normalizeLookupResults(
|
||||
@@ -367,6 +382,10 @@ export async function resolvePinnedHostnameWithPolicy(
|
||||
};
|
||||
}
|
||||
|
||||
export function assertHostnameAllowedWithPolicy(hostname: string, policy?: SsrFPolicy): string {
|
||||
return resolveHostnamePolicyChecks(hostname, policy).normalized;
|
||||
}
|
||||
|
||||
export async function resolvePinnedHostname(
|
||||
hostname: string,
|
||||
lookupFn: LookupFn = dnsLookup,
|
||||
|
||||
@@ -5,6 +5,10 @@ const fetchWithSsrFGuardMock = vi.hoisted(() => vi.fn());
|
||||
vi.mock("../infra/net/fetch-guard.js", () => ({
|
||||
fetchWithSsrFGuard: (...args: unknown[]) => fetchWithSsrFGuardMock(...args),
|
||||
withStrictGuardedFetchMode: <T>(params: T) => params,
|
||||
withTrustedExplicitProxyGuardedFetchMode: <T>(params: T) => ({
|
||||
...params,
|
||||
mode: "trusted_explicit_proxy",
|
||||
}),
|
||||
}));
|
||||
|
||||
type FetchRemoteMedia = typeof import("./fetch.js").fetchRemoteMedia;
|
||||
@@ -286,4 +290,34 @@ describe("fetchRemoteMedia", () => {
|
||||
|
||||
await expectBoundedErrorBodyCase(testCase.fetchImpl);
|
||||
});
|
||||
|
||||
it("uses trusted explicit-proxy mode when the caller opts in for proxy-side DNS", async () => {
|
||||
const fetchImpl = vi.fn(async () => new Response("ok", { status: 200 }));
|
||||
|
||||
await fetchRemoteMedia({
|
||||
url: "https://api.telegram.org/file/bot123/photos/test.jpg",
|
||||
fetchImpl,
|
||||
lookupFn: makeLookupFn(),
|
||||
trustExplicitProxyDns: true,
|
||||
dispatcherAttempts: [
|
||||
{
|
||||
dispatcherPolicy: {
|
||||
mode: "explicit-proxy",
|
||||
proxyUrl: "http://localhost:8888",
|
||||
allowPrivateProxy: true,
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(fetchWithSsrFGuardMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
mode: "trusted_explicit_proxy",
|
||||
dispatcherPolicy: expect.objectContaining({
|
||||
mode: "explicit-proxy",
|
||||
proxyUrl: "http://localhost:8888",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import path from "node:path";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import { fetchWithSsrFGuard, withStrictGuardedFetchMode } from "../infra/net/fetch-guard.js";
|
||||
import {
|
||||
fetchWithSsrFGuard,
|
||||
withStrictGuardedFetchMode,
|
||||
withTrustedExplicitProxyGuardedFetchMode,
|
||||
} from "../infra/net/fetch-guard.js";
|
||||
import type { LookupFn, PinnedDispatcherPolicy, SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
import { redactSensitiveText } from "../logging/redact.js";
|
||||
import { detectMime, extensionForMime } from "./mime.js";
|
||||
@@ -44,6 +48,11 @@ type FetchMediaOptions = {
|
||||
lookupFn?: LookupFn;
|
||||
dispatcherAttempts?: FetchDispatcherAttempt[];
|
||||
shouldRetryFetchError?: (error: unknown) => boolean;
|
||||
/**
|
||||
* Allow an operator-configured explicit proxy to resolve target DNS after
|
||||
* hostname-policy checks instead of forcing local pinned-DNS first.
|
||||
*/
|
||||
trustExplicitProxyDns?: boolean;
|
||||
};
|
||||
|
||||
function stripQuotes(value: string): string {
|
||||
@@ -106,6 +115,7 @@ export async function fetchRemoteMedia(options: FetchMediaOptions): Promise<Fetc
|
||||
lookupFn,
|
||||
dispatcherAttempts,
|
||||
shouldRetryFetchError,
|
||||
trustExplicitProxyDns,
|
||||
} = options;
|
||||
const sourceUrl = redactMediaUrl(url);
|
||||
|
||||
@@ -118,7 +128,9 @@ export async function fetchRemoteMedia(options: FetchMediaOptions): Promise<Fetc
|
||||
: [{ dispatcherPolicy: undefined, lookupFn }];
|
||||
const runGuardedFetch = async (attempt: FetchDispatcherAttempt) =>
|
||||
await fetchWithSsrFGuard(
|
||||
withStrictGuardedFetchMode({
|
||||
(trustExplicitProxyDns && attempt.dispatcherPolicy?.mode === "explicit-proxy"
|
||||
? withTrustedExplicitProxyGuardedFetchMode
|
||||
: withStrictGuardedFetchMode)({
|
||||
url,
|
||||
fetchImpl,
|
||||
init: requestInit,
|
||||
|
||||
Reference in New Issue
Block a user