Skip to content

Commit 55ad5d7

Browse files
steipetesteipete
committed
fix(security): harden explicit-proxy SSRF pinning
1 parent f52eb93 commit 55ad5d7

File tree

8 files changed

+57
-10
lines changed

8 files changed

+57
-10
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ Docs: https://docs.openclaw.ai
110110
- 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.
111111
- 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.
112112
- 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.
113+
- 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.
113114
- 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.
114115
- 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.
115116
- 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.

‎extensions/telegram/src/fetch.test.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ function getDispatcherFromUndiciCall(nth: number) {
8181
options?: {
8282
connect?: Record<string, unknown>;
8383
proxyTls?: Record<string, unknown>;
84+
requestTls?: Record<string, unknown>;
8485
};
8586
}
8687
| undefined;
@@ -126,10 +127,11 @@ function expectStickyAutoSelectDispatcher(
126127
options?: {
127128
connect?: Record<string, unknown>;
128129
proxyTls?: Record<string, unknown>;
130+
requestTls?: Record<string, unknown>;
129131
};
130132
}
131133
| undefined,
132-
field: "connect" | "proxyTls" = "connect",
134+
field: "connect" | "proxyTls" | "requestTls" = "connect",
133135
): void {
134136
expect(dispatcher?.options?.[field]).toEqual(
135137
expect.objectContaining({
@@ -189,7 +191,7 @@ function expectCallerDispatcherPreserved(callIndexes: number[], dispatcher: unkn
189191
async function expectNoStickyRetryWithSameDispatcher(params: {
190192
resolved: ReturnType<typeof resolveTelegramFetchOrThrow>;
191193
expectedAgentCtor: typeof ProxyAgentCtor | typeof EnvHttpProxyAgentCtor;
192-
field: "connect" | "proxyTls";
194+
field: "connect" | "proxyTls" | "requestTls";
193195
}) {
194196
await expect(params.resolved("https://api.telegram.org/botx/sendMessage")).rejects.toThrow(
195197
"fetch failed",
@@ -349,7 +351,7 @@ describe("resolveTelegramFetch", () => {
349351
uri: "http://127.0.0.1:7890",
350352
}),
351353
);
352-
expect(dispatcher?.options?.proxyTls).toEqual(
354+
expect(dispatcher?.options?.requestTls).toEqual(
353355
expect.objectContaining({
354356
autoSelectFamily: false,
355357
}),
@@ -372,7 +374,7 @@ describe("resolveTelegramFetch", () => {
372374
await expectNoStickyRetryWithSameDispatcher({
373375
resolved,
374376
expectedAgentCtor: ProxyAgentCtor,
375-
field: "proxyTls",
377+
field: "requestTls",
376378
});
377379
});
378380

‎extensions/telegram/src/fetch.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,11 @@ function createTelegramDispatcher(policy: PinnedDispatcherPolicy): {
254254
effectivePolicy: PinnedDispatcherPolicy;
255255
} {
256256
if (policy.mode === "explicit-proxy") {
257-
const proxyTlsOptions = withPinnedLookup(policy.proxyTls, policy.pinnedHostname);
258-
const proxyOptions = proxyTlsOptions
257+
const requestTlsOptions = withPinnedLookup(policy.proxyTls, policy.pinnedHostname);
258+
const proxyOptions = requestTlsOptions
259259
? ({
260260
uri: policy.proxyUrl,
261-
proxyTls: proxyTlsOptions,
261+
requestTls: requestTlsOptions,
262262
} satisfies ConstructorParameters<typeof ProxyAgent>[0])
263263
: policy.proxyUrl;
264264
try {

‎src/infra/net/fetch-guard.ssrf.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,21 @@ describe("fetchWithSsrFGuard hardening", () => {
140140
expect(result.response.status).toBe(200);
141141
});
142142

143+
it("fails closed for plain HTTP targets when explicit proxy mode requires pinned DNS", async () => {
144+
const fetchImpl = vi.fn();
145+
await expect(
146+
fetchWithSsrFGuard({
147+
url: "http://public.example/resource",
148+
fetchImpl,
149+
dispatcherPolicy: {
150+
mode: "explicit-proxy",
151+
proxyUrl: "http://127.0.0.1:7890",
152+
},
153+
}),
154+
).rejects.toThrow(/explicit proxy ssrf pinning requires https targets/i);
155+
expect(fetchImpl).not.toHaveBeenCalled();
156+
});
157+
143158
it("blocks redirect chains that hop to private hosts", async () => {
144159
const lookupFn = createPublicLookup();
145160
const fetchImpl = await expectRedirectFailure({

‎src/infra/net/fetch-guard.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,22 @@ function resolveGuardedFetchMode(params: GuardedFetchOptions): GuardedFetchMode
9191
return GUARDED_FETCH_MODE.STRICT;
9292
}
9393

94+
function assertExplicitProxySupportsPinnedDns(
95+
url: URL,
96+
dispatcherPolicy?: PinnedDispatcherPolicy,
97+
pinDns?: boolean,
98+
): void {
99+
if (
100+
pinDns !== false &&
101+
dispatcherPolicy?.mode === "explicit-proxy" &&
102+
url.protocol !== "https:"
103+
) {
104+
throw new Error(
105+
"Explicit proxy SSRF pinning requires HTTPS targets; plain HTTP targets are not supported",
106+
);
107+
}
108+
}
109+
94110
function isRedirectStatus(status: number): boolean {
95111
return status === 301 || status === 302 || status === 303 || status === 307 || status === 308;
96112
}
@@ -190,6 +206,7 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
190206

191207
let dispatcher: Dispatcher | null = null;
192208
try {
209+
assertExplicitProxySupportsPinnedDns(parsedUrl, params.dispatcherPolicy, params.pinDns);
193210
const pinned = await resolvePinnedHostnameWithPolicy(parsedUrl.hostname, {
194211
lookupFn: params.lookupFn,
195212
policy: params.policy,

‎src/infra/net/ssrf.dispatcher.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,9 @@ describe("createPinnedDispatcher", () => {
217217

218218
expect(proxyAgentCtor).toHaveBeenCalledWith({
219219
uri: "http://127.0.0.1:7890",
220-
proxyTls: {
220+
requestTls: {
221221
autoSelectFamily: false,
222+
lookup,
222223
},
223224
});
224225
});

‎src/infra/net/ssrf.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,12 +418,16 @@ export function createPinnedDispatcher(
418418
}
419419

420420
const proxyUrl = policy.proxyUrl.trim();
421-
if (!policy.proxyTls) {
421+
const requestTls = withPinnedLookup(lookup, policy.proxyTls);
422+
if (!requestTls) {
422423
return new ProxyAgent(proxyUrl);
423424
}
424425
return new ProxyAgent({
425426
uri: proxyUrl,
426-
proxyTls: { ...policy.proxyTls },
427+
// `PinnedDispatcherPolicy.proxyTls` historically carried target-hop
428+
// transport hints for explicit proxies. Translate that to undici's
429+
// `requestTls` so HTTPS proxy tunnels keep the pinned DNS lookup.
430+
requestTls,
427431
});
428432
}
429433

‎src/media/fetch.telegram-network.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,19 @@ describe("fetchRemoteMedia telegram network policy", () => {
167167
dispatcher?: {
168168
options?: {
169169
uri?: string;
170+
requestTls?: Record<string, unknown>;
170171
};
171172
};
172173
})
173174
| undefined;
174175

175176
expect(init?.dispatcher?.options?.uri).toBe("http://127.0.0.1:7890");
177+
expect(init?.dispatcher?.options?.requestTls).toEqual(
178+
expect.objectContaining({
179+
autoSelectFamily: false,
180+
lookup: expect.any(Function),
181+
}),
182+
);
176183
expect(undiciMocks.proxyAgentCtor).toHaveBeenCalled();
177184
});
178185

0 commit comments

Comments
 (0)