Skip to content

Commit 3d14441

Browse files
qazbnm456claude
andcommitted
feat(tools): promote the resolved-IP SSRF re-check into a reusable primitive
is_safe_url is only syntactic; the DNS-rebinding re-check (re-resolve each hop, refuse a private/reserved address) was left to each consumer's fetcher — so every consumer re-derived it, and each independently broke behind a fake-IP proxy. Add rlm_kit.tools.resolved_host_is_safe(host, port, *, allow_nets=()) + parse_cidrs: the resolved-address guard now ships once, with an allow_nets carve-out for a host behind a fake-IP proxy / split-DNS VPN (Clash/Mihomo/Surge map every public host into the reserved 198.18.0.0/16, which the strict re-check would refuse — starving the model of all fetched source). Empty allow_nets = unchanged strictness: is_safe_url still refuses localhost/metadata regardless of allow_nets. _hostname_is_blocked_literal_ip and the new _ip_blocked share one _ip_in_blocked_range, so the block-category list has a single home. Additive public surface (tools __all__); no schema/contract change. Consumer-driven hardening — surfaced by a downstream direct fetcher refusing every host behind such a proxy. 166 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 0623e8d commit 3d14441

4 files changed

Lines changed: 129 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,15 @@ surfaced by dogfooding a real downstream consumer.
1212

1313
### Added
1414

15+
- **Reusable resolved-IP SSRF guard for the `direct`-fetch pattern** (`rlm_kit.tools.resolved_host_is_safe`
16+
+ `parse_cidrs`). `is_safe_url` is only syntactic; the DNS-rebinding re-check (re-resolve each hop,
17+
refuse a private/reserved address) was left to each consumer's fetcher — and every consumer re-derived
18+
it. `resolved_host_is_safe(host, port, *, allow_nets=())` now ships that check ONCE, with an
19+
`allow_nets` carve-out (`parse_cidrs(["198.18.0.0/16"])`) for a host behind a fake-IP proxy / split-DNS
20+
VPN (Clash/Mihomo/Surge map every public host into the reserved `198.18.0.0/16`, which the strict
21+
re-check would refuse — starving the model of all fetched source). Empty `allow_nets` = unchanged
22+
strictness (`is_safe_url` still refuses localhost/metadata regardless). Consumer-driven: surfaced by a
23+
downstream `direct` fetcher refusing every host behind such a proxy.
1524
- **`max_output_chars` is now configurable** (`RLMConfig.max_output_chars`, env
1625
`RLM_MAX_OUTPUT_CHARS`, default `10000` — dspy's own default, so behaviour is
1726
unchanged). dspy.RLM head+tail-truncates each REPL output to this many CHARACTERS

rlm_kit/tools/__init__.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
"""Reusable tools that RLM tasks can expose to the model inside the REPL."""
22

3-
from .fetch import is_safe_url, make_fetch_tool
3+
from .fetch import (
4+
is_safe_url,
5+
make_fetch_tool,
6+
parse_cidrs,
7+
resolved_host_is_safe,
8+
)
49
from .model import ModelToolResult, make_model_tool
510
from .search import make_web_search_tool, normalise_search_results
611
from .validation import make_schema_validator
712

813
__all__ = [
914
"make_schema_validator",
1015
"is_safe_url",
16+
"resolved_host_is_safe",
17+
"parse_cidrs",
1118
"make_fetch_tool",
1219
"make_web_search_tool",
1320
"normalise_search_results",

rlm_kit/tools/fetch.py

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,26 @@
1111
``RLMTask(tools=…)`` — dspy.RLM invokes tools synchronously, so the tool must be sync.
1212
1313
NOTE: a syntactic guard does not stop DNS rebinding (a public hostname that
14-
resolves to a private address). Full protection requires re-checking the
15-
*resolved* address at connection time inside your fetcher.
14+
resolves to a private address). ``resolved_host_is_safe`` is that missing
15+
resolved-address re-check — call it INSIDE your fetcher at connection time (and
16+
on every redirect hop). Its ``allow_nets`` carve-out (build with ``parse_cidrs``)
17+
accommodates a fake-IP proxy / split-DNS VPN that maps public hosts into a
18+
reserved range; it is a re-usable primitive so each consumer's ``direct`` fetcher
19+
shares ONE correct implementation instead of re-deriving it.
1620
"""
1721

1822
from __future__ import annotations
1923

2024
import ipaddress
25+
import logging
26+
import socket
2127
from typing import Callable
2228
from urllib.parse import urlparse
2329

2430
from ..trace import record_tool_call
2531

32+
logger = logging.getLogger(__name__)
33+
2634
ALLOWED_SCHEMES = frozenset({"http", "https"})
2735

2836
# Hostnames that must never be fetched regardless of resolution.
@@ -34,13 +42,9 @@
3442
)
3543

3644

37-
def _hostname_is_blocked_literal_ip(hostname: str) -> bool:
38-
try:
39-
ip = ipaddress.ip_address(hostname)
40-
except ValueError:
41-
return False
42-
# Block loopback, private, link-local (incl. 169.254.169.254 cloud metadata),
43-
# reserved, and unspecified ranges.
45+
def _ip_in_blocked_range(ip) -> bool:
46+
"""True if a parsed ``ip_address`` falls in a loopback / private / link-local (incl.
47+
169.254.169.254 cloud metadata) / reserved / unspecified / multicast range."""
4448
return (
4549
ip.is_private
4650
or ip.is_loopback
@@ -51,6 +55,57 @@ def _hostname_is_blocked_literal_ip(hostname: str) -> bool:
5155
)
5256

5357

58+
def _hostname_is_blocked_literal_ip(hostname: str) -> bool:
59+
try:
60+
ip = ipaddress.ip_address(hostname)
61+
except ValueError:
62+
return False
63+
return _ip_in_blocked_range(ip)
64+
65+
66+
def parse_cidrs(cidrs) -> tuple:
67+
"""Parse an iterable of CIDR strings into networks for ``resolved_host_is_safe(allow_nets=…)``.
68+
An unparseable entry is warned-and-skipped (never fatal) so a typo can't sink a run."""
69+
nets = []
70+
for c in cidrs or ():
71+
try:
72+
nets.append(ipaddress.ip_network(c, strict=False))
73+
except ValueError:
74+
logger.warning("ignoring invalid allow-CIDR %r", c)
75+
return tuple(nets)
76+
77+
78+
def _ip_blocked(ip_str: str, allow_nets=()) -> bool:
79+
"""True if ``ip_str`` must be refused. Unparseable → blocked (fail closed). An operator-listed
80+
``allow_nets`` range is treated as external (the proxy, not the resolved address, is the real
81+
endpoint) — everything else falls through to the standard blocked-range check."""
82+
try:
83+
ip = ipaddress.ip_address(ip_str)
84+
except ValueError:
85+
return True
86+
if any(ip in n for n in allow_nets):
87+
return False
88+
return _ip_in_blocked_range(ip)
89+
90+
91+
def resolved_host_is_safe(host: str, port: int, *, allow_nets=()) -> bool:
92+
"""The DNS-rebinding defence: resolve ``host`` and return True only if EVERY resolved address is
93+
external. Call this INSIDE your fetcher, re-checking each redirect hop — ``is_safe_url`` is
94+
syntactic and cannot see what a hostname resolves to.
95+
96+
``allow_nets`` (from ``parse_cidrs``) carves out operator-trusted ranges: a fake-IP proxy /
97+
split-DNS VPN (e.g. Clash/Mihomo/Surge default ``198.18.0.0/16``) maps every public host into a
98+
RESERVED range that would otherwise be refused, starving the model of source. Empty by default =
99+
full strictness (``is_safe_url`` still refuses localhost/metadata regardless of ``allow_nets``)."""
100+
if not host:
101+
return False
102+
try:
103+
infos = socket.getaddrinfo(host, port, proto=socket.IPPROTO_TCP)
104+
except OSError:
105+
return False
106+
return bool(infos) and all(not _ip_blocked(info[4][0], allow_nets) for info in infos)
107+
108+
54109
def is_safe_url(url: str) -> bool:
55110
"""Return True if ``url`` is an http(s) URL not pointing at an obvious
56111
internal/loopback/metadata target.

tests/test_tools.py

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@
44
from pydantic import BaseModel
55

66
from rlm_kit.optimize import exact_field_metric, schema_valid_metric
7-
from rlm_kit.tools.fetch import is_safe_url, make_fetch_tool
7+
import rlm_kit.tools.fetch as fetch_mod
8+
from rlm_kit.tools.fetch import (
9+
_ip_blocked,
10+
is_safe_url,
11+
make_fetch_tool,
12+
parse_cidrs,
13+
resolved_host_is_safe,
14+
)
815
from rlm_kit.tools.model import ModelToolResult, make_model_tool
916
from rlm_kit.tools.search import make_web_search_tool, normalise_search_results
1017
from rlm_kit.tools.validation import make_schema_validator
@@ -171,6 +178,46 @@ def test_unsafe_urls_blocked(url):
171178
assert is_safe_url(url) is False
172179

173180

181+
# ---- resolved-IP re-check (DNS-rebinding defence) + allow-list -----------
182+
183+
def test_parse_cidrs_skips_bad_entries():
184+
nets = parse_cidrs(("198.18.0.0/16", "garbage", "10.0.0.0/8"))
185+
assert len(nets) == 2 # the bad entry is dropped, not fatal
186+
assert parse_cidrs(()) == () and parse_cidrs(None) == ()
187+
188+
189+
def test_ip_blocked_default_allow_list_and_fail_closed():
190+
assert _ip_blocked("8.8.8.8") is False # public → allowed
191+
assert _ip_blocked("198.18.2.128") is True # reserved (benchmarking) → blocked
192+
assert _ip_blocked("not-an-ip") is True # unparseable → fail closed
193+
nets = parse_cidrs(("198.18.0.0/16",))
194+
assert _ip_blocked("198.18.2.128", nets) is False # allow-listed proxy range → external
195+
assert _ip_blocked("127.0.0.1", nets) is True # loopback still blocked
196+
assert _ip_blocked("169.254.169.254", nets) is True # cloud metadata still blocked
197+
198+
199+
def test_resolved_host_is_safe_honors_allow_list(monkeypatch):
200+
# A fake-IP proxy resolves every host into 198.18.x.x; refused by default, allowed when listed.
201+
monkeypatch.setattr(fetch_mod.socket, "getaddrinfo",
202+
lambda host, port, *a, **k: [(2, 1, 6, "", ("198.18.2.128", port))])
203+
assert resolved_host_is_safe("plugins.svn.example", 443) is False
204+
assert resolved_host_is_safe("plugins.svn.example", 443,
205+
allow_nets=parse_cidrs(("198.18.0.0/16",))) is True
206+
# a host that resolves to a REAL public address is safe without any allow-list
207+
monkeypatch.setattr(fetch_mod.socket, "getaddrinfo",
208+
lambda host, port, *a, **k: [(2, 1, 6, "", ("93.184.216.34", port))])
209+
assert resolved_host_is_safe("example.com", 443) is True
210+
211+
212+
def test_resolved_host_is_safe_fails_closed_on_resolution_error(monkeypatch):
213+
def boom(*a, **k):
214+
raise OSError("no DNS")
215+
216+
monkeypatch.setattr(fetch_mod.socket, "getaddrinfo", boom)
217+
assert resolved_host_is_safe("x", 443, allow_nets=parse_cidrs(("198.18.0.0/16",))) is False
218+
assert resolved_host_is_safe("", 443) is False # empty host → refused
219+
220+
174221
def test_fetch_tool_blocks_before_calling_fetcher():
175222
called = {"n": 0}
176223

0 commit comments

Comments
 (0)