fix(security): harden explicit-proxy SSRF pinning

This commit is contained in:
Peter Steinberger
2026-03-22 23:04:52 -07:00
parent f52eb934d6
commit 55ad5d7bd7
8 changed files with 57 additions and 10 deletions

View File

@@ -110,6 +110,7 @@ Docs: https://docs.openclaw.ai
- Configure/startup: move outbound send-deps resolution into a lightweight helper so `openclaw configure` no longer stalls after the banner while eagerly loading channel plugins. (#46301) Thanks @scoootscooob.
- Mattermost/threading: honor `replyToMode: "off"` for already-threaded inbound posts so threaded follow-ups can fall back to top-level replies when configured. (#52543) Thanks @RichardCao.
- Security/exec approvals: escape blank Hangul filler code points in approval prompts across gateway/chat and the macOS native approval UI so visually empty Unicode padding cannot hide reviewed command text.
- Security/network: harden explicit-proxy SSRF pinning by translating target-hop transport hints onto HTTPS proxy tunnels and failing closed for plain HTTP guarded fetches that cannot preserve pinned DNS.
- Telegram/replies: set `allow_sending_without_reply` on reply-targeted sends and media-error notices so deleted parent messages no longer drop otherwise valid replies. (#52524) Thanks @moltbot886.
- Gateway/status: resolve env-backed `gateway.auth.*` SecretRefs before read-only probe auth checks so status no longer reports false probe failures when auth is configured through SecretRef. (#52513) Thanks @CodeForgeNet.
- Agents/exec: return plain-text failed tool output for timeouts and other non-success exec outcomes so models no longer parrot raw JSON error payloads back to users. (#52508) Thanks @martingarramon.

View File

@@ -81,6 +81,7 @@ function getDispatcherFromUndiciCall(nth: number) {
options?: {
connect?: Record<string, unknown>;
proxyTls?: Record<string, unknown>;
requestTls?: Record<string, unknown>;
};
}
| undefined;
@@ -126,10 +127,11 @@ function expectStickyAutoSelectDispatcher(
options?: {
connect?: Record<string, unknown>;
proxyTls?: Record<string, unknown>;
requestTls?: Record<string, unknown>;
};
}
| undefined,
field: "connect" | "proxyTls" = "connect",
field: "connect" | "proxyTls" | "requestTls" = "connect",
): void {
expect(dispatcher?.options?.[field]).toEqual(
expect.objectContaining({
@@ -189,7 +191,7 @@ function expectCallerDispatcherPreserved(callIndexes: number[], dispatcher: unkn
async function expectNoStickyRetryWithSameDispatcher(params: {
resolved: ReturnType<typeof resolveTelegramFetchOrThrow>;
expectedAgentCtor: typeof ProxyAgentCtor | typeof EnvHttpProxyAgentCtor;
field: "connect" | "proxyTls";
field: "connect" | "proxyTls" | "requestTls";
}) {
await expect(params.resolved("https://api.telegram.org/botx/sendMessage")).rejects.toThrow(
"fetch failed",
@@ -349,7 +351,7 @@ describe("resolveTelegramFetch", () => {
uri: "http://127.0.0.1:7890",
}),
);
expect(dispatcher?.options?.proxyTls).toEqual(
expect(dispatcher?.options?.requestTls).toEqual(
expect.objectContaining({
autoSelectFamily: false,
}),
@@ -372,7 +374,7 @@ describe("resolveTelegramFetch", () => {
await expectNoStickyRetryWithSameDispatcher({
resolved,
expectedAgentCtor: ProxyAgentCtor,
field: "proxyTls",
field: "requestTls",
});
});

View File

@@ -254,11 +254,11 @@ function createTelegramDispatcher(policy: PinnedDispatcherPolicy): {
effectivePolicy: PinnedDispatcherPolicy;
} {
if (policy.mode === "explicit-proxy") {
const proxyTlsOptions = withPinnedLookup(policy.proxyTls, policy.pinnedHostname);
const proxyOptions = proxyTlsOptions
const requestTlsOptions = withPinnedLookup(policy.proxyTls, policy.pinnedHostname);
const proxyOptions = requestTlsOptions
? ({
uri: policy.proxyUrl,
proxyTls: proxyTlsOptions,
requestTls: requestTlsOptions,
} satisfies ConstructorParameters<typeof ProxyAgent>[0])
: policy.proxyUrl;
try {

View File

@@ -140,6 +140,21 @@ describe("fetchWithSsrFGuard hardening", () => {
expect(result.response.status).toBe(200);
});
it("fails closed for plain HTTP targets when explicit proxy mode requires pinned DNS", async () => {
const fetchImpl = vi.fn();
await expect(
fetchWithSsrFGuard({
url: "http://public.example/resource",
fetchImpl,
dispatcherPolicy: {
mode: "explicit-proxy",
proxyUrl: "http://127.0.0.1:7890",
},
}),
).rejects.toThrow(/explicit proxy ssrf pinning requires https targets/i);
expect(fetchImpl).not.toHaveBeenCalled();
});
it("blocks redirect chains that hop to private hosts", async () => {
const lookupFn = createPublicLookup();
const fetchImpl = await expectRedirectFailure({

View File

@@ -91,6 +91,22 @@ function resolveGuardedFetchMode(params: GuardedFetchOptions): GuardedFetchMode
return GUARDED_FETCH_MODE.STRICT;
}
function assertExplicitProxySupportsPinnedDns(
url: URL,
dispatcherPolicy?: PinnedDispatcherPolicy,
pinDns?: boolean,
): void {
if (
pinDns !== false &&
dispatcherPolicy?.mode === "explicit-proxy" &&
url.protocol !== "https:"
) {
throw new Error(
"Explicit proxy SSRF pinning requires HTTPS targets; plain HTTP targets are not supported",
);
}
}
function isRedirectStatus(status: number): boolean {
return status === 301 || status === 302 || status === 303 || status === 307 || status === 308;
}
@@ -190,6 +206,7 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
let dispatcher: Dispatcher | null = null;
try {
assertExplicitProxySupportsPinnedDns(parsedUrl, params.dispatcherPolicy, params.pinDns);
const pinned = await resolvePinnedHostnameWithPolicy(parsedUrl.hostname, {
lookupFn: params.lookupFn,
policy: params.policy,

View File

@@ -217,8 +217,9 @@ describe("createPinnedDispatcher", () => {
expect(proxyAgentCtor).toHaveBeenCalledWith({
uri: "http://127.0.0.1:7890",
proxyTls: {
requestTls: {
autoSelectFamily: false,
lookup,
},
});
});

View File

@@ -418,12 +418,16 @@ export function createPinnedDispatcher(
}
const proxyUrl = policy.proxyUrl.trim();
if (!policy.proxyTls) {
const requestTls = withPinnedLookup(lookup, policy.proxyTls);
if (!requestTls) {
return new ProxyAgent(proxyUrl);
}
return new ProxyAgent({
uri: proxyUrl,
proxyTls: { ...policy.proxyTls },
// `PinnedDispatcherPolicy.proxyTls` historically carried target-hop
// transport hints for explicit proxies. Translate that to undici's
// `requestTls` so HTTPS proxy tunnels keep the pinned DNS lookup.
requestTls,
});
}

View File

@@ -167,12 +167,19 @@ describe("fetchRemoteMedia telegram network policy", () => {
dispatcher?: {
options?: {
uri?: string;
requestTls?: Record<string, unknown>;
};
};
})
| undefined;
expect(init?.dispatcher?.options?.uri).toBe("http://127.0.0.1:7890");
expect(init?.dispatcher?.options?.requestTls).toEqual(
expect.objectContaining({
autoSelectFamily: false,
lookup: expect.any(Function),
}),
);
expect(undiciMocks.proxyAgentCtor).toHaveBeenCalled();
});