From d0819c2683a8b62b8cc4e7ac64ddf84b1e83978a Mon Sep 17 00:00:00 2001 From: Oleksandr Kozachuk Date: Mon, 13 Jul 2026 18:10:54 +0200 Subject: [PATCH] url reading tool: fetch_url with ssrf guard, html->text, page images to vision cache --- config.toml | 6 ++ fjerkroa_bot/openai_responder.py | 67 ++++++++----- fjerkroa_bot/url_reader.py | 163 +++++++++++++++++++++++++++++++ specs/SPEC-011-url-reading.md | 57 +++++++++++ tests/test_spec_url.py | 143 +++++++++++++++++++++++++++ 5 files changed, 410 insertions(+), 26 deletions(-) create mode 100644 fjerkroa_bot/url_reader.py create mode 100644 specs/SPEC-011-url-reading.md create mode 100644 tests/test_spec_url.py diff --git a/config.toml b/config.toml index 98d526c..fbade2e 100644 --- a/config.toml +++ b/config.toml @@ -67,3 +67,9 @@ enable-game-info = true # idle-impulse-hours = 12 # taskgen-interval-hours = 6 # Staff: !bot tasks | task-approve | task-cancel +# URL reading (SPEC-011, FDB-018) — DEFAULT OFF; web pages are hostile input: +# enable-url-reading = true +# url-max-bytes = 2097152 # 2 MB fetch cap +# url-max-chars = 6000 # text handed to the model +# url-max-images = 2 # page images into the vision cache +# url-daily-per-user = 20 diff --git a/fjerkroa_bot/openai_responder.py b/fjerkroa_bot/openai_responder.py index 06b2a7a..f2e41a0 100644 --- a/fjerkroa_bot/openai_responder.py +++ b/fjerkroa_bot/openai_responder.py @@ -12,6 +12,7 @@ from .ai_responder import AIResponder, exponential_backoff, sanitize_external_te from .igdblib import IGDBQuery from .leonardo_draw import LeonardoAIDrawMixIn from .quota import QuotaLedger +from .url_reader import FETCH_URL_TOOL, URLReader # The response envelope, enforced server-side via structured outputs # (ENV-19). All fields required, closed object, nullable where the @@ -153,6 +154,33 @@ class OpenAIResponder(AIResponder, LeonardoAIDrawMixIn): else: logging.warning("❌ IGDB integration DISABLED - missing configuration or disabled in config") + # URL reading tool (SPEC-011); shares the image cache for page images + self.url_reader = URLReader(lambda: self.config, self.image_cache) + + def _available_tools(self) -> List[Dict[str, Any]]: + """Assemble the function-tool list from every enabled provider (URL-01).""" + functions: List[Dict[str, Any]] = [] + if self.igdb and self.config.get("enable-game-info", False): + try: + igdb_functions = self.igdb.get_openai_functions() + if isinstance(igdb_functions, list): + functions.extend(igdb_functions) + except (TypeError, AttributeError) as err: + logging.warning(f"Error setting up IGDB functions: {err}") + if self.url_reader.enabled(): + functions.append(FETCH_URL_TOOL) + return functions + + async def _dispatch_tool(self, name: str, args: Dict[str, Any], author: str) -> Any: + """Route a tool call to its provider (IGDB or URL reader).""" + if name == "fetch_url": + per_user_cap = int(self.config.get("url-daily-per-user", 20)) + if self.ledger._get(f"url-fetch:{author}") >= per_user_cap: # URL-07 + return {"error": "daily URL fetch limit reached"} + self.ledger._add(f"url-fetch:{author}", 1) + return await self.url_reader.fetch(str(args.get("url", "")), self.channel, author or "user") + return await self._execute_igdb_function(name, args) + async def draw_openai(self, description: str, count: int = 1) -> List[BytesIO]: if not self.ledger.budget_ok(): raise RuntimeError("daily budget exhausted - refusing image call") @@ -241,24 +269,13 @@ class OpenAIResponder(AIResponder, LeonardoAIDrawMixIn): # hashed, never the raw Discord name (SAF-10) chat_kwargs["safety_identifier"] = "discord-" + hashlib.sha256(author.encode()).hexdigest()[:16] - if self.igdb and self.config.get("enable-game-info", False): - try: - igdb_functions = self.igdb.get_openai_functions() - if igdb_functions and isinstance(igdb_functions, list): - chat_kwargs["tools"] = [{"type": "function", "function": func} for func in igdb_functions] - chat_kwargs["tool_choice"] = "auto" - # gpt-5.6 rejects tools + reasoning on chat/completions (ENV-21) - chat_kwargs["reasoning_effort"] = self.config.get("reasoning-effort", "none") - logging.info(f"🎮 IGDB functions available to AI: {[f['name'] for f in igdb_functions]}") - logging.debug(f" Full chat_kwargs with tools: {list(chat_kwargs.keys())}") - except (TypeError, AttributeError) as e: - logging.warning(f"Error setting up IGDB functions: {e}") - else: - logging.debug( - "🎮 IGDB not available for this request (igdb={}, enabled={})".format( - self.igdb is not None, self.config.get("enable-game-info", False) - ) - ) + available_tools = self._available_tools() + if available_tools: + chat_kwargs["tools"] = [{"type": "function", "function": func} for func in available_tools] + chat_kwargs["tool_choice"] = "auto" + # gpt-5.6 rejects tools + reasoning on chat/completions (ENV-21) + chat_kwargs["reasoning_effort"] = self.config.get("reasoning-effort", "none") + logging.info(f"🔧 Tools available to AI: {[func['name'] for func in available_tools]}") result = await openai_chat(self.client, **chat_kwargs) self._record_usage(result) @@ -278,10 +295,8 @@ class OpenAIResponder(AIResponder, LeonardoAIDrawMixIn): tool_names = [tc.function.name for tc in message.tool_calls] logging.info(f"🔧 OpenAI requested function calls: {tool_names}") - # Check if we have function/tool calls and IGDB is enabled - has_tool_calls = ( - hasattr(message, "tool_calls") and message.tool_calls and self.igdb and self.config.get("enable-game-info", False) - ) + # Any offered tool may have been called (IGDB or fetch_url) + has_tool_calls = bool(hasattr(message, "tool_calls") and message.tool_calls and available_tools) # Clean up any existing tool messages in the history to avoid conflicts if has_tool_calls: @@ -304,12 +319,12 @@ class OpenAIResponder(AIResponder, LeonardoAIDrawMixIn): function_name = tool_call.function.name function_args = json.loads(tool_call.function.arguments) - logging.info(f"🎮 Executing IGDB function: {function_name} with args: {function_args}") + logging.info(f"🔧 Executing tool: {function_name} with args: {function_args}") - # Execute IGDB function - function_result = await self._execute_igdb_function(function_name, function_args) + # Route to the right provider (IGDB or URL reader) + function_result = await self._dispatch_tool(function_name, function_args, self._last_author(messages) or "") - logging.info(f"🎮 IGDB function result: {type(function_result)} - {str(function_result)[:200]}...") + logging.info(f"🔧 Tool result: {type(function_result)} - {str(function_result)[:200]}...") messages.append( { diff --git a/fjerkroa_bot/url_reader.py b/fjerkroa_bot/url_reader.py new file mode 100644 index 0000000..2343e2f --- /dev/null +++ b/fjerkroa_bot/url_reader.py @@ -0,0 +1,163 @@ +"""URL reading tool (SPEC-011, FDB-018). + +The model calls `fetch_url`; this module fetches safely and returns +readable text plus prominent image URLs. Web pages are hostile input: +every fetch is SSRF-guarded (no private/loopback/link-local targets, +http/https only, redirects re-validated) and every byte of text is +sanitized before it can reach the prompt. +""" + +import ipaddress +import logging +import re +import socket +from html.parser import HTMLParser +from typing import Any, Callable, Dict, List, Optional, Tuple +from urllib.parse import urljoin, urlparse + +import aiohttp + +from .ai_responder import sanitize_external_text + +DEFAULT_MAX_BYTES = 2 * 1024 * 1024 +DEFAULT_MAX_CHARS = 6000 +DEFAULT_MAX_IMAGES = 2 +FETCH_TIMEOUT_S = 15 +MAX_REDIRECTS = 5 + +FETCH_URL_TOOL = { + "name": "fetch_url", + "description": "Fetch a public web page and return its readable text plus prominent image links. " + "Use when the user shares a URL and asks about it, or to get details behind a news link.", + "parameters": { + "type": "object", + "properties": {"url": {"type": "string", "description": "The http/https URL to read."}}, + "required": ["url"], + }, +} + + +class _Extractor(HTMLParser): + def __init__(self) -> None: + super().__init__() + self._skip = 0 + self.parts: List[str] = [] + self.images: List[str] = [] + self.og_image: Optional[str] = None + + def handle_starttag(self, tag: str, attrs) -> None: + if tag in ("script", "style", "noscript", "svg"): + self._skip += 1 + attr = dict(attrs) + src = attr.get("src") + if tag == "img" and src: + self.images.append(src) + if tag == "meta" and attr.get("property") == "og:image" and attr.get("content"): + self.og_image = attr["content"] + + def handle_endtag(self, tag: str) -> None: + if tag in ("script", "style", "noscript", "svg") and self._skip > 0: + self._skip -= 1 + + def handle_data(self, data: str) -> None: + if self._skip == 0 and data.strip(): + self.parts.append(data.strip()) + + +def _ip_is_public(ip_str: str) -> bool: + try: + ip = ipaddress.ip_address(ip_str) + except ValueError: + return False + return not (ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_reserved or ip.is_unspecified) + + +def guard_url(url: str) -> Optional[str]: + """Return None if safe to fetch, else a human-readable refusal reason (URL-02/03).""" + parsed = urlparse(url) + if parsed.scheme not in ("http", "https"): + return f"refused scheme {parsed.scheme!r} (only http/https)" + host = parsed.hostname + if not host: + return "refused: no host" + try: + literal = ipaddress.ip_address(host) + return None if _ip_is_public(str(literal)) else f"refused non-public address {host}" + except ValueError: + pass + try: + infos = socket.getaddrinfo(host, None) + except socket.gaierror: + return f"refused: cannot resolve {host}" + for info in infos: + if not _ip_is_public(str(info[4][0])): + return f"refused: {host} resolves to non-public address" + return None + + +class URLReader: + def __init__(self, config_getter: Callable[[], Dict[str, Any]], image_cache) -> None: + self._config = config_getter + self.image_cache = image_cache + + def enabled(self) -> bool: + return bool(self._config().get("enable-url-reading", False)) + + async def _get(self, session, url: str, max_bytes: int) -> Tuple[str, bytes]: + """Manual redirect handling so every hop is re-guarded (URL-04).""" + current = url + for _ in range(MAX_REDIRECTS): + reason = guard_url(current) + if reason: + raise ValueError(reason) + async with session.get(current, allow_redirects=False) as response: + if response.status in (301, 302, 303, 307, 308) and response.headers.get("Location"): + current = urljoin(current, response.headers["Location"]) + continue + response.raise_for_status() + return str(response.url), await response.content.read(max_bytes + 1) + raise ValueError("too many redirects") + + async def fetch(self, url: str, channel: str, user: str) -> Dict[str, Any]: + config = self._config() + max_bytes = int(config.get("url-max-bytes", DEFAULT_MAX_BYTES)) + timeout = aiohttp.ClientTimeout(total=FETCH_TIMEOUT_S) + try: + async with aiohttp.ClientSession(timeout=timeout, headers={"User-Agent": "FjerkroaBot/1.0"}) as session: + final_url, body = await self._get(session, url, max_bytes) + except Exception as err: + return {"error": str(err)} + text = self._to_text(body.decode("utf-8", "ignore")) + clean = sanitize_external_text(text, int(config.get("url-max-chars", DEFAULT_MAX_CHARS))) + images = await self._ingest_images(body.decode("utf-8", "ignore"), final_url, channel, user) + return {"url": final_url, "text": clean, "images_cached": images} + + def _to_text(self, html: str) -> str: + extractor = _Extractor() + try: + extractor.feed(html) + except Exception as err: + logging.debug(f"html parse (text) failed: {err!r}") + return re.sub(r"\s+\n", "\n", " ".join(extractor.parts)) + + async def _ingest_images(self, html: str, base_url: str, channel: str, user: str) -> int: + if self.image_cache is None: + return 0 + extractor = _Extractor() + try: + extractor.feed(html) + except Exception as err: + logging.debug(f"html parse (images) failed: {err!r}") + candidates = ([extractor.og_image] if extractor.og_image else []) + extractor.images + limit = int(self._config().get("url-max-images", DEFAULT_MAX_IMAGES)) + cached = 0 + for src in candidates: + if cached >= limit: + break + absolute = urljoin(base_url, src) + if guard_url(absolute) is not None: + continue + sha = await self.image_cache.ingest_url(absolute, channel, user, None) + if sha is not None: + cached += 1 + return cached diff --git a/specs/SPEC-011-url-reading.md b/specs/SPEC-011-url-reading.md new file mode 100644 index 0000000..011438e --- /dev/null +++ b/specs/SPEC-011-url-reading.md @@ -0,0 +1,57 @@ +# SPEC-011 — URL reading + +A `fetch_url` tool alongside IGDB: the model decides when to read a +link (user pastes a URL + question; a news item links an article +Luma wants details on). Web pages are the number-one injection +vector, so everything fetched is sanitized (SAF-03) and the fetch +itself is SSRF-guarded — the bot runs on shared hosting. Active only +when `enable-url-reading = true`. + +### URL-01 — fetch_url is offered as a tool (coverage: test) + +When `enable-url-reading` is true, the chat call's `tools` list +includes a `fetch_url` function (url string param) next to any IGDB +tools. When false, it is absent. + +### URL-02 — Only http/https are fetched (coverage: test) + +`file:`, `ftp:`, `data:`, `gopher:` and schemeless inputs are +refused before any network call, with an error result the model can +relay. + +### URL-03 — SSRF guard blocks non-public addresses (coverage: test) + +Before fetching, the host is resolved and every resulting IP is +checked; the fetch is refused when any is private, loopback, +link-local, or otherwise non-global (RFC1918, 127/8, 169.254/16, +::1, fc00::/7, etc.). A URL literal that is already such an IP is +refused without DNS. + +### URL-04 — Redirects are re-validated (coverage: test) + +Redirects are followed manually; each hop's target passes URL-02 and +URL-03 again. A public URL that 302-redirects to `localhost` or an +internal IP is refused at the redirect, not fetched. + +### URL-05 — Fetched text is bounded and sanitized (coverage: test) + +Responses are capped at `url-max-bytes` (default 2 MB) with a +download timeout; HTML is reduced to readable text (script/style +dropped, tags stripped, whitespace collapsed) and passed through +`sanitize_external_text` before it reaches the model, truncated to +`url-max-chars` (default 6000). + +### URL-06 — Page images feed the cache (coverage: test) + +Up to `url-max-images` (default 2) prominent images (og:image, then +large ``) are ingested into the ImageCache for the requesting +channel (SSRF-guarded like the page), so the model can see them and +`picture_edit` can remix them. Ingestion failures are skipped, never +fatal to the text result. + +### URL-07 — Fetches are metered and capped (coverage: test) + +Each fetch increments a per-user daily counter; over +`url-daily-per-user` (default 20) `fetch_url` refuses with an error +result. The budget gate (SAF-04) still applies to the surrounding +model calls. diff --git a/tests/test_spec_url.py b/tests/test_spec_url.py new file mode 100644 index 0000000..9928bea --- /dev/null +++ b/tests/test_spec_url.py @@ -0,0 +1,143 @@ +"""Unit coverage for SPEC-011 URL reading (URL-01..07).""" + +import unittest +from unittest.mock import AsyncMock, patch + +from fjerkroa_bot.openai_responder import OpenAIResponder +from fjerkroa_bot.url_reader import FETCH_URL_TOOL, URLReader, guard_url + +CONFIG = {"openai-token": "t", "model": "m", "system": "s", "history-limit": 5} + + +class TestToolOffered(unittest.TestCase): + def test_tool_present_only_when_enabled(self): + """URL-01: fetch_url appears in the tool list only with enable-url-reading.""" + off = OpenAIResponder(CONFIG, "chat") + self.assertNotIn("fetch_url", [f["name"] for f in off._available_tools()]) + on = OpenAIResponder(dict(CONFIG, **{"enable-url-reading": True}), "chat") + self.assertIn("fetch_url", [f["name"] for f in on._available_tools()]) + self.assertEqual(FETCH_URL_TOOL["name"], "fetch_url") + + +class TestSchemeGuard(unittest.TestCase): + def test_non_http_schemes_refused(self): + """URL-02: only http/https pass the guard.""" + self.assertIsNone(guard_url("https://example.com/article")) + for bad in ("file:///etc/passwd", "ftp://host/x", "data:text/html,x", "gopher://h", "no-scheme.com/x"): + self.assertIsNotNone(guard_url(bad)) + + +class TestSSRFGuard(unittest.TestCase): + def test_private_and_loopback_refused(self): + """URL-03: private/loopback/link-local literals are refused without DNS.""" + for bad in ( + "http://127.0.0.1/admin", + "http://localhost/x", # resolves to loopback + "http://10.0.0.5/x", + "http://192.168.1.1/x", + "http://169.254.169.254/latest/meta-data", # cloud metadata + "http://[::1]/x", + ): + self.assertIsNotNone(guard_url(bad), f"{bad} should be refused") + + def test_public_ip_allowed(self): + """URL-03: a public IP literal passes.""" + self.assertIsNone(guard_url("http://93.184.216.34/")) + + @patch("fjerkroa_bot.url_reader.socket.getaddrinfo") + def test_dns_to_private_refused(self, getaddrinfo): + """URL-03: a hostname resolving to a private IP is refused.""" + getaddrinfo.return_value = [(2, 1, 6, "", ("10.1.2.3", 0))] + self.assertIsNotNone(guard_url("http://evil.example.com/x")) + + +class TestRedirectRevalidation(unittest.IsolatedAsyncioTestCase): + async def test_redirect_to_internal_refused(self): + """URL-04: a public URL redirecting to localhost is refused at the hop.""" + reader = URLReader(lambda: {}, None) + + class FakeResp: + status = 302 + headers = {"Location": "http://127.0.0.1/secret"} + url = "http://safe.example.com" + + async def __aenter__(self): + return self + + async def __aexit__(self, *a): + return False + + def raise_for_status(self): + pass + + class FakeSession: + def get(self, url, allow_redirects=False): + return FakeResp() + + with patch("fjerkroa_bot.url_reader.guard_url", side_effect=[None, "refused internal"]): + with self.assertRaises(ValueError): + await reader._get(FakeSession(), "http://safe.example.com", 1000) + + +class TestTextExtraction(unittest.TestCase): + def test_html_reduced_to_text(self): + """URL-05: scripts/styles dropped, tags stripped.""" + reader = URLReader(lambda: {}, None) + html = "

Titel

Inhalt hier

" + text = reader._to_text(html) + self.assertIn("Titel", text) + self.assertIn("Inhalt hier", text) + self.assertNotIn("evil", text) + self.assertNotIn("x{}", text) + + +class TestFetchSanitizes(unittest.IsolatedAsyncioTestCase): + async def test_fetch_result_is_sanitized_and_capped(self): + """URL-05: fetch output is length-capped and @everyone-neutralized.""" + reader = URLReader(lambda: {"url-max-chars": 50}, None) + payload = ("

@everyone " + "x" * 5000 + "

").encode() + with patch.object(reader, "_get", new=AsyncMock(return_value=("http://x.com", payload))): + result = await reader.fetch("http://x.com", "chat", "alice") + self.assertLessEqual(len(result["text"]), 50) + self.assertNotIn("@everyone", result["text"]) + + async def test_fetch_error_is_reported_not_raised(self): + """URL-05: a fetch failure returns an error dict the model can relay.""" + reader = URLReader(lambda: {}, None) + with patch.object(reader, "_get", new=AsyncMock(side_effect=ValueError("refused non-public address"))): + result = await reader.fetch("http://10.0.0.1", "chat", "alice") + self.assertIn("error", result) + + +class TestImageIngest(unittest.IsolatedAsyncioTestCase): + async def test_page_images_go_to_cache_ssrf_guarded(self): + """URL-06: og:image + ingested (cap honored), internal srcs skipped.""" + cache = type("C", (), {})() + cache.ingest_url = AsyncMock(side_effect=["sha1", "sha2", "sha3"]) + reader = URLReader(lambda: {"url-max-images": 2}, cache) + html = ( + '' + '' + ) + + # guard by scheme/loopback only, no real DNS in the test + def fake_guard(url): + return "refused" if "127.0.0.1" in url else None + + with patch("fjerkroa_bot.url_reader.guard_url", side_effect=fake_guard): + count = await reader._ingest_images(html, "https://example.com", "chat", "alice") + self.assertEqual(count, 2) # og:image + first public img, cap 2 + ingested = [call.args[0] for call in cache.ingest_url.await_args_list] + self.assertNotIn("http://127.0.0.1/internal.png", ingested) + + +class TestPerUserCap(unittest.IsolatedAsyncioTestCase): + async def test_dispatch_caps_fetches(self): + """URL-07: over url-daily-per-user, fetch_url returns an error without fetching.""" + responder = OpenAIResponder(dict(CONFIG, **{"enable-url-reading": True, "url-daily-per-user": 2}), "chat") + responder.url_reader.fetch = AsyncMock(return_value={"url": "x", "text": "ok"}) + for _ in range(2): + await responder._dispatch_tool("fetch_url", {"url": "http://x.com"}, "alice") + blocked = await responder._dispatch_tool("fetch_url", {"url": "http://x.com"}, "alice") + self.assertIn("error", blocked) + self.assertEqual(responder.url_reader.fetch.await_count, 2)