1915ca98db* fetch bare hostnames as https instead of refusing them * normalize host:port URLs and route schemeless github repos to the readme API * only rewrite dotted host:port URLs with in-range ports * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * reject relative paths and oversized ports in url normalization * Match web-fetch ports as ASCII digits so a unicode digit cannot raise str.isdigit() is True for digit-class characters int() refuses (superscript two, circled digit one), so _normalize_url_scheme reached int(port) and raised ValueError out of _fetch_url_raw, which runs before its try block. A web_search url of "example.com:<superscript two>" surfaced a generic tool exception instead of the Blocked: message it returned before this branch. Match the port against an anchored [0-9]{1,5} instead; the five-digit cap that kept the range check from converting an unbounded integer is now in the pattern. * Apply the invalid-port guard to redirect targets too _fetch_url_raw wraps the initial parsed.port in try/except ValueError, but the redirect hop reads rp.port unguarded, so a server answering Location: https://example.org:99999/next fell through to the broad handler as "Failed to fetch URL: Port out of range 0-65535" rather than a deliberate block. No request is dispatched either way; this just makes the two paths report the same way. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep the redirect-port test compact The formatter expands a signature carrying a spaced kwarg default, which put the stub opener on eleven lines. **kw absorbs the timeout the fetch loop passes and leaves the whole stub on four. * Never let a malformed URL escape _fetch_url_raw as an exception The URL is model-supplied, so every bad form should come back as one of the documented (error, body, content_type) strings. Three gaps remained: urlparse itself raises on an unmatched IPv6 bracket and on a netloc that NFKC-decomposes into a delimiter (//exam(fullwidth-solidus)ple.com), and both calls sat outside a guard. getaddrinfo raises UnicodeError, which is a ValueError and not the OSError _validate_and_resolve_host catches, when IDNA encoding rejects a hostname. Over a 3158 URL corpus that injects tabs, newlines, C0 controls, delimiters and NFKC confusables at every position, main raises 42 times and this raises none. Also strip surrounding whitespace in _normalize_url_scheme. _web_search already stripped, but normalization moved down to the fetch layer, so a direct _fetch_page_text caller did not get it. * Name the host in the status badge and tool card for bare URLs status_for_tool and the web-search tool card both required an explicit scheme before reading the hostname, so every URL this branch newly makes fetchable showed the generic "Reading page..." and "Read page" instead of the host. Under permission_mode=ask that means the approval card named no destination for exactly the inputs the branch enables. The backend reuses _normalize_url_scheme. The frontend cannot, since new URL() throws on a bare host, so RE_BARE_HOST mirrors the same grammar: only a dotted host with an optional in-range port gets the https prefix, leaving /login, javascript: and userinfo forms to render generically as before. Also mention bare hostnames in the url parameter description, since they are part of the accepted interface now. * Do not let a malformed URL in the status badge kill the tool turn status_for_tool runs inside prepare_call, before the fetch and outside the handler that wraps tool execution, so a ValueError from urlparse ends the whole turn instead of letting _fetch_url_raw return its blocked message. _normalize_url_scheme catches its own parse error and hands back the original string, so the parse here still has to be guarded. Reachable with https://[::1 or a host that NFKC-decomposes into a delimiter. This predates the branch, main raises identically, but the badge is one of the lines this branch touches and the rest of it already promises no malformed URL escapes as an exception. * Tighten the comments added by this branch * Revert the web_search url description change The premise of this branch is that models already emit bare hostnames unprompted, which is why the fetch layer had to stop refusing them. Advertising the bare form in the tool schema does not enable anything, it just steers models toward it, and that is the form carrying every edge case: ambiguous with dotted custom schemes, and unlike an explicit scheme it does not cover IPv6 literals, IDN or trailing-dot FQDNs. The fetch layer tolerates bare hosts. The schema should keep recommending a full URL. This also drops the one change here with no regression test. * Match the backend port rule in the tool card host The card's bare-host pattern required at least one digit after the colon, but the backend fetches an empty port (example.com: and example.com:/path go to the default HTTPS port), so a successful fetch rendered as "Read page" with no host. Allowing an empty port alone would have swung it the other way: example.com:0 is refused by the backend but new URL() accepts it, so the card would have named a host that is never fetched. That mismatch was there before this change too. Mirror the backend rule instead, an empty port or one in 1-65535, checked against every case in the normalizer's own matrix. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
170 lines
5.5 KiB
Python
170 lines
5.5 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
||
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
||
|
||
"""Bare hosts ("google.com") must be fetched as https, not refused."""
|
||
|
||
from __future__ import annotations
|
||
|
||
import sys
|
||
from pathlib import Path
|
||
|
||
import pytest
|
||
|
||
_BACKEND = Path(__file__).resolve().parent.parent
|
||
if str(_BACKEND) not in sys.path:
|
||
sys.path.insert(0, str(_BACKEND))
|
||
|
||
from core.inference import tools # noqa: E402
|
||
|
||
|
||
@pytest.fixture
|
||
def resolved(monkeypatch):
|
||
seen: dict = {}
|
||
|
||
def fake_resolve(hostname, port, deadline, cancel_event):
|
||
seen["hostname"] = hostname
|
||
seen["port"] = port
|
||
return False, "stopped", None
|
||
|
||
monkeypatch.setattr(tools, "_resolve_with_budget", fake_resolve)
|
||
return seen
|
||
|
||
|
||
@pytest.mark.parametrize(
|
||
"url, hostname, port",
|
||
[
|
||
("google.com", "google.com", 443),
|
||
("www.google.com/x", "www.google.com", 443),
|
||
("//google.com", "google.com", 443),
|
||
("https://google.com", "google.com", 443),
|
||
("http://google.com", "google.com", 80),
|
||
("example.com:8443/path", "example.com", 8443),
|
||
("example.com:8443", "example.com", 8443),
|
||
("sub.example.co.uk:8080", "sub.example.co.uk", 8080),
|
||
],
|
||
)
|
||
def test_schemeless_urls_are_fetched_as_https(resolved, url, hostname, port):
|
||
err, _, _ = tools._fetch_url_raw(url)
|
||
assert resolved["hostname"] == hostname
|
||
assert resolved["port"] == port
|
||
assert "only http/https" not in (err or "")
|
||
|
||
|
||
@pytest.mark.parametrize(
|
||
"url",
|
||
[
|
||
"ftp://x.com",
|
||
"file:///etc/passwd",
|
||
"javascript:alert(1)",
|
||
"mailto:a@b.c",
|
||
# scheme:digits must not masquerade as host:port
|
||
"file:80",
|
||
"javascript:443/path",
|
||
"mailto:25",
|
||
# out-of-range ports are not host:port either
|
||
"example.com:99999",
|
||
"example.com:0",
|
||
# ports must match ASCII [0-9]: str.isdigit() is True for digits int() refuses
|
||
"example.com:²",
|
||
"example.com:²/x",
|
||
"example.com:①",
|
||
"example.com:1²",
|
||
"//example.com:²",
|
||
# non-ASCII decimal digits int() accepts are ports urlparse then refuses
|
||
"example.com:٤٤٣",
|
||
# root-relative paths have no host to fetch
|
||
"/login",
|
||
"/github.com/owner/repo",
|
||
],
|
||
)
|
||
def test_non_http_schemes_still_blocked(url):
|
||
err, _, _ = tools._fetch_url_raw(url)
|
||
assert err and "only http/https" in err
|
||
|
||
|
||
def test_absurdly_long_port_does_not_raise():
|
||
err, _, _ = tools._fetch_url_raw("example.com:" + "9" * 4400)
|
||
assert err and "only http/https" in err
|
||
|
||
|
||
def test_out_of_range_port_returns_error_instead_of_raising():
|
||
# check_url_access owns the wording; what matters is a string, not a raise.
|
||
err, _, _ = tools._fetch_url_raw("https://example.com:99999")
|
||
assert err and err.startswith("Blocked:")
|
||
|
||
|
||
def test_redirect_to_out_of_range_port_is_blocked(monkeypatch):
|
||
# A redirect target reads .port too, so it needs the same guard.
|
||
import urllib.request
|
||
from urllib.error import HTTPError
|
||
|
||
monkeypatch.setattr(
|
||
tools,
|
||
"_resolve_with_budget",
|
||
lambda host, port, deadline, cancel: (True, "", "93.184.216.34"),
|
||
)
|
||
|
||
class _Redirecting:
|
||
def open(self, req, **kw):
|
||
hdrs = {"Location": "https://example.org:99999/next"}
|
||
raise HTTPError(req.full_url, 302, "Found", hdrs, None)
|
||
|
||
monkeypatch.setattr(urllib.request, "build_opener", lambda *handlers: _Redirecting())
|
||
err, _, _ = tools._fetch_url_raw("https://example.com")
|
||
assert err and err.startswith("Blocked:")
|
||
|
||
|
||
@pytest.mark.parametrize(
|
||
"url",
|
||
[
|
||
# urlparse raises on these; a model-supplied URL must still return a string
|
||
"//exam／ple.com", # NFKC-decomposes into "/"
|
||
"//example.com＠", # NFKC-decomposes into "@"
|
||
"//example.com：", # NFKC-decomposes into ":"
|
||
"https://[::1", # unmatched IPv6 bracket
|
||
"https://::1]",
|
||
],
|
||
)
|
||
def test_malformed_url_is_blocked_instead_of_raising(url):
|
||
err, _, _ = tools._fetch_url_raw(url)
|
||
assert err and err.startswith("Blocked:")
|
||
|
||
|
||
def test_idna_failure_is_reported_instead_of_raising(monkeypatch):
|
||
# getaddrinfo raises UnicodeError, not OSError, when IDNA encoding fails.
|
||
import socket
|
||
|
||
def boom(*a, **k):
|
||
raise UnicodeError("encoding with 'idna' codec failed")
|
||
|
||
monkeypatch.setattr(socket, "getaddrinfo", boom)
|
||
err, _, _ = tools._fetch_url_raw("https://münich.example")
|
||
assert err and err.startswith("Failed to resolve host:")
|
||
|
||
|
||
@pytest.mark.parametrize(
|
||
"url, hostname",
|
||
[
|
||
(" google.com", "google.com"),
|
||
("google.com\n", "google.com"),
|
||
("\t example.com:8443 ", "example.com"),
|
||
],
|
||
)
|
||
def test_surrounding_whitespace_is_stripped(resolved, url, hostname):
|
||
# _web_search strips, but direct callers of the fetch layer do not.
|
||
tools._fetch_url_raw(url)
|
||
assert resolved["hostname"] == hostname
|
||
|
||
|
||
@pytest.mark.parametrize("url", ["127.0.0.1", "169.254.169.254", "10.0.0.1", "192.168.1.1"])
|
||
def test_normalization_does_not_bypass_ssrf_guard(url):
|
||
err, _, _ = tools._fetch_url_raw(url, timeout = 3)
|
||
assert err and "non-public address" in err
|
||
|
||
|
||
def test_schemeless_github_repo_still_routes_to_readme_api():
|
||
# Must run before _github_repo_readme_api_url, else a bare repo URL scrapes HTML.
|
||
normalized = tools._normalize_url_scheme("github.com/unslothai/unsloth")
|
||
assert tools._github_repo_readme_api_url(normalized) == (
|
||
"https://api.github.com/repos/unslothai/unsloth/readme"
|
||
)
|