fix(url_safety): exclude 198.18.0.0/15 benchmark range from SSRF private-IP blocking

[liuhao1024]

What does this PR do?

Excludes the 198.18.0.0/15 benchmark/DNS-filtering range from SSRF private-IP blocking, fixing false-positive blocks on public sites like cdimage.ubuntu.com when accessed from networks using DNS-based content-filtering services.

Related Issue

Fixes #35423

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• tools/url_safety.py: Added _BENCHMARK_RANGE (198.18.0.0/15) and excluded it from _is_blocked_ip() for both IPv4 and IPv4-mapped IPv6 addresses. Updated _TRUSTED_PRIVATE_IP_HOSTS comment to note the new range exclusion.
• tests/tools/test_url_safety.py: Updated existing tests and added regression tests for benchmark range IP allowance, including cdimage.ubuntu.com specific test case and IPv4-mapped IPv6 benchmark range test.

How to Test

1. Run pytest tests/tools/test_url_safety.py -v — all 115 tests should pass
2. Verify cdimage.ubuntu.com (resolves to 198.18.0.65 on DNS-filtered networks) is no longer blocked by is_safe_url()
3. Verify that RFC 1918 private IPs (10.x, 172.16.x, 192.168.x) and cloud metadata (169.254.x.x) are still blocked
4. Verify CGNAT range (100.64.0.0/10) is still blocked

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

• Analyzed: tools/url_safety.py:_is_blocked_ip (called by is_safe_url, is_always_blocked_url; callers: web_tools.py, browser_tool.py, vision_tools.py)
• Blast radius: LOW — narrow change to IP classification logic; cloud metadata, RFC 1918, and CGNAT blocking unchanged
• Related patterns: _TRUSTED_PRIVATE_IP_HOSTS (QQ media per-host exception for same range), _CGNAT_NETWORK (similar range-exclusion pattern)

[alt-glitch]

Related: #3777, #19992, #35423. The existing workaround is security.allow_private_urls: true (merged in #14166), but that's a broad toggle. This PR unconditionally unblocks the 198.18.0.0/15 benchmark range by default.

See also open PR #26064 which adds a more granular allow_benchmark_ips config toggle for the same range.

[tonydwb]

Code Review Summary

Verdict: Approved (well-analyzed security fix)

Looks Good

• Correct root cause analysis: 198.18.0.0/15 is marked is_private by Python's ipaddress module, but it's the IANA-specified Benchmarking range (RFC 2544) — not an actual private network. DNS-based content filters (OpenDNS/Cisco Umbrella, CleanBrowsing) resolve sinkholed domains here, causing false-positive SSRF blocks on legitimate public sites.
• Minimal blast radius: Only 198.18.0.0/15 is excluded. RFC 1918 (10.x, 172.16.x, 192.168.x), link-local (169.254.x.x), CGNAT (100.64.0.0/10), multicast, loopback all remain blocked.
• Both IPv4 and IPv4-mapped IPv6 paths handled: The early return in the IPv6 path (if embedded_ip in _BENCHMARK_RANGE: return False) and the check before the is_private return in the standard path both correctly exempt the range.
• Cloud metadata remains blocked: _ALWAYS_BLOCKED_IPS and _ALWAYS_BLOCKED_NETWORKS (169.254.169.254, etc.) are checked before _is_blocked_ip in is_safe_url, so this change cannot bypass cloud metadata protection.
• Test coverage is thorough:
  • test_benchmark_range_allowed_for_any_host — general benchmark IP now allowed
  • test_cdimage_ubuntu_allowed_on_filtered_network — real-world regression test
  • test_qq_multimedia_subdomain_also_allowed_via_benchmark_range — verifies previous allowlist exception is now subsumed
  • test_qq_multimedia_http_allowed_via_benchmark_range — HTTP (previously blocked) now allowed
  • Updated test_blocked_ips parametrize: removed 198.18.0.23 from blocked list
  • Updated test_allowed_ips parametrize: added 198.18.0.23, 198.19.255.255
  • Updated test_ipv4_mapped_allowed_ips: added ::ffff:198.18.0.65

Comments on the change to QQ hostname tests

• Previously sub.multimedia.nt.qq.com.cn and HTTP-only QQ URLs were blocked (enforcing narrow allowlist). Now they're allowed because any host resolving to the benchmark range passes. This is a behavioral relaxation but it's the correct tradeoff — the benchmark range is not a real private network, so there's no SSRF risk. The QQ allowlist comment was also updated to clarify it now only covers hosts resolving to other private ranges.

No Security Concerns

• The 198.18.0.0/15 range cannot be used for SSRF attacks (cloud metadata is on 169.254.169.254, internal services are on RFC 1918). This is a genuine false-positive fix.
• _TRUSTED_PRIVATE_IP_HOSTS still provides protection for any QQ-like hosts that resolve to real private ranges.

---

Reviewed by Hermes Agent

[tonydwb]

Code Review Summary

Verdict: Approved ✅

Overall

Clean, well-scoped fix for a real-world SSRF false-positive issue. The benchmark range (198.18.0.0/15) is used by DNS-based content-filtering services (OpenDNS, CleanBrowsing) to sinkhole blocked domains — blocking it creates false positives on filtered networks without meaningful SSRF protection.

What's Good

• Correct range choice: 198.18.0.0/15 is the documented IANA benchmark range, not a real private network
• Proper reasoning: RFC 1918, cloud metadata (169.254.x.x), and CGNAT (100.64.0.0/10) remain fully blocked — actual SSRF targets are protected
• Dual-path coverage: Both IPv4 and IPv4-mapped IPv6 paths are covered
• Comprehensive tests: Regression test for cdimage.ubuntu.com, updated all existing benchmark IP tests, parametrized allowed-IP lists updated, existing QQ hostname tests properly reflect new behavior
• Good docstrings: Explain why each test expectation changed, making reviewer's job easy
• Clean blast radius: LOW — narrow change to IP classification logic only

No Issues Found

• No edge cases missed (the range is a single /15, no sub-ranges to worry about)
• No security regression — RFC 1918, loopback, link-local, multicast, CGNAT all unchanged
• Tests were correctly updated to reflect the new behavioral expectations

---

Reviewed by Hermes Agent

[teknium1]

Thanks for documenting the RFC 2544 false-positive and covering both ordinary and IPv4-mapped address classification.

Problems

• The proposed default exemption removes an intentional current-main boundary. Main allows 198.18.0.0/15 only for one exact HTTPS QQ host (tools/url_safety.py:179-184, 379-381); tests deliberately keep generic hosts, subdomains, and HTTP blocked (tests/tools/test_url_safety.py:247-269). The PR changes all three cases to allowed by default.

Suggested changes

• Preserve the default boundary and, if maintainers want broader support, re-scope it as an explicit config.yaml opt-in. The member discussion already points to the narrower allow_benchmark_ips direction in #26064.
• Add an end-to-end is_safe_url() test for an IPv4-mapped benchmark address under any new scoped option, while retaining coverage that RFC 1918, CGNAT, and link-local addresses remain blocked.

Automated hermes-sweeper review.

[teknium1]

This default-wide bypass changes the deliberate current policy from one exact HTTPS host to every hostname and scheme resolving into 198.18.0.0/15. Please preserve the default SSRF boundary and make broader allowance an explicit config.yaml opt-in if maintainers choose that direction.