diff --git a/tests/test_routes_settings.py b/tests/test_routes_settings.py index 0bdd9c5bb..4471b916e 100644 --- a/tests/test_routes_settings.py +++ b/tests/test_routes_settings.py @@ -330,3 +330,161 @@ async def test_result_fields_present(self, tmp_path): assert r.rebuilt is True assert r.success is False assert r.message == "x" + + +class TestMemoryUrlSettings: + """Contract: GET /api/settings/memory-url returns {url, is_local, reachable} + and PUT probes the new target before returning (#1911).""" + + @pytest.mark.asyncio + async def test_get_returns_default_url(self, client): + """Default URL is localhost:7900, is_local=True, reachable depends on env.""" + resp = await client.get("/api/settings/memory-url") + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "http://localhost:7900" + assert data["is_local"] is True + assert isinstance(data["reachable"], bool) + + @pytest.mark.asyncio + async def test_get_returns_is_local_true_for_localhost(self, client, app): + """Setting the config URL to a loopback address must report is_local=True.""" + app.state.config.memory_url = "http://127.0.0.1:7900" + resp = await client.get("/api/settings/memory-url") + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "http://127.0.0.1:7900" + assert data["is_local"] is True + + @pytest.mark.asyncio + async def test_get_returns_is_local_false_for_remote(self, client, app): + """A remote URL must report is_local=False.""" + app.state.config.memory_url = "https://memory.example.com:7900" + resp = await client.get("/api/settings/memory-url") + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "https://memory.example.com:7900" + assert data["is_local"] is False + + @pytest.mark.asyncio + async def test_put_sets_url_and_probes(self, client, app): + """PUT persists the URL to config and returns probe result.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http://192.168.1.100:7900"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "http://192.168.1.100:7900" + assert data["is_local"] is True # 192.168.x.x is private + assert isinstance(data["reachable"], bool) + # Verify it persisted on config + assert app.state.config.memory_url == "http://192.168.1.100:7900" + + @pytest.mark.asyncio + async def test_put_rejects_empty_url(self, client): + """Empty URL must return 400.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": ""}, + ) + assert resp.status_code == 400 + + @pytest.mark.asyncio + async def test_put_rejects_non_http_scheme(self, client): + """A non-http scheme must return 400.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "ftp://localhost:7900"}, + ) + assert resp.status_code == 400 + assert "http" in resp.json()["error"].lower() + + @pytest.mark.asyncio + async def test_get_reflects_put(self, client, app): + """After PUT, GET must return the new URL.""" + await client.put( + "/api/settings/memory-url", + json={"url": "https://taosmd.local:8443"}, + ) + resp = await client.get("/api/settings/memory-url") + assert resp.status_code == 200 + data = resp.json() + assert data["url"] == "https://taosmd.local:8443" + assert data["is_local"] is False + + @pytest.mark.asyncio + async def test_is_local_url_helper(self): + """Unit-test _is_local_url directly.""" + from tinyagentos.routes.settings import _is_local_url + + assert _is_local_url("http://localhost:7900") is True + assert _is_local_url("http://127.0.0.1:7900") is True + assert _is_local_url("http://[::1]:7900") is True + assert _is_local_url("http://10.0.0.1:7900") is True # private + assert _is_local_url("http://192.168.1.1:7900") is True # private + assert _is_local_url("https://example.com:7900") is False + assert _is_local_url("http://8.8.8.8:7900") is False # public IP + + @pytest.mark.asyncio + async def test_put_probes_and_reports_unreachable(self, client): + """When taosmd is not running, reachable must be False.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http://localhost:19999"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["reachable"] is False # no taosmd on that port + + @pytest.mark.asyncio + async def test_put_strips_trailing_slash(self, client, app): + """Trailing slash on the URL should be stripped.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http://localhost:7900/"}, + ) + assert resp.status_code == 200 + assert resp.json()["url"] == "http://localhost:7900" + assert app.state.config.memory_url == "http://localhost:7900" + + @pytest.mark.asyncio + async def test_put_rejects_url_without_hostname(self, client): + """A URL without a valid hostname must return 400 (hostname check).""" + # path-only: http:///path (hostname is empty string) + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http:///path"}, + ) + assert resp.status_code == 400 + + # port-only: http://:7900 (hostname is empty, netloc non-empty — regression for netloc→hostname fix) + resp = await client.put( + "/api/settings/memory-url", + json={"url": "http://:7900"}, + ) + assert resp.status_code == 400 + + @pytest.mark.asyncio + async def test_put_response_includes_is_local_and_reachable(self, client): + """PUT response must include is_local and reachable alongside url.""" + resp = await client.put( + "/api/settings/memory-url", + json={"url": "https://remote.example.com:7900"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert "url" in data + assert "is_local" in data + assert "reachable" in data + assert data["is_local"] is False # remote hostname + + @pytest.mark.asyncio + async def test_put_rejects_malformed_json(self, client): + """Malformed JSON body must return 422 (Pydantic validation), not 500.""" + resp = await client.put( + "/api/settings/memory-url", + content=b"not json", + headers={"Content-Type": "application/json"}, + ) + assert resp.status_code == 422 diff --git a/tinyagentos/routes/settings.py b/tinyagentos/routes/settings.py index e4c611e15..c904b4334 100644 --- a/tinyagentos/routes/settings.py +++ b/tinyagentos/routes/settings.py @@ -1,12 +1,15 @@ from __future__ import annotations import asyncio import datetime +import httpx import io +import ipaddress import logging import os import shutil import tarfile import time +from collections import OrderedDict from pathlib import Path from urllib.parse import urlparse @@ -1075,31 +1078,92 @@ async def set_update_channel(request: Request, body: UpdateChannel): # Memory URL (taOSmd) # --------------------------------------------------------------------------- +_MEMORY_URL_PROBE_CACHE: OrderedDict = OrderedDict() # {url: (timestamp, bool)} — capped at 32 entries via FIFO eviction + + +def _is_local_url(url: str) -> bool: + """Return True if *url* points to the local machine (loopback or private).""" + parsed = urlparse(url) + hostname = parsed.hostname + if not hostname: + return False + if hostname in ("localhost", "127.0.0.1", "::1"): + return True + try: + addr = ipaddress.ip_address(hostname) + return addr.is_loopback or addr.is_private + except ValueError: + return False + + +async def _probe_taosmd(request: Request, url: str) -> bool: + """Probe the taOSmd /health endpoint. Returns True if reachable.""" + try: + client = request.app.state.http_client + resp = await client.get( + f"{url}/health", + timeout=httpx.Timeout(3.0, connect=2.0), + ) + return resp.status_code == 200 + except (httpx.RequestError, httpx.InvalidURL): + return False + + class MemoryUrlUpdate(BaseModel): url: str @router.get("/api/settings/memory-url") async def get_memory_url(request: Request): - """Return the current taOSmd memory URL.""" + """Return the current taOSmd memory URL with local/reachable probes.""" config = request.app.state.config - return {"url": config.memory_url} + url = config.memory_url + is_local = _is_local_url(url) + + # Return cached probe result if fresh (< 30 s), else re-probe. + cached = _MEMORY_URL_PROBE_CACHE.get(url) + now = time.monotonic() + if cached and (now - cached[0]) < 30: + reachable = cached[1] + else: + reachable = await _probe_taosmd(request, url) + _MEMORY_URL_PROBE_CACHE[url] = (now, reachable) + if len(_MEMORY_URL_PROBE_CACHE) > 32: + _MEMORY_URL_PROBE_CACHE.popitem(last=False) + + return {"url": url, "is_local": is_local, "reachable": reachable} @router.put("/api/settings/memory-url") async def set_memory_url(request: Request, body: MemoryUrlUpdate): - """Update the taOSmd memory URL and persist to config.""" + """Update the taOSmd memory URL, persist to config, and probe reachability. + + The URL is always accepted and persisted. The *reachable* field reports + the probe result but does not gate the save.""" url = body.url.strip() if not url: return JSONResponse({"error": "URL must not be empty"}, status_code=400) + # Strip a single trailing slash (not a character set). + if url.endswith("/"): + url = url[:-1] parsed = urlparse(url) if parsed.scheme not in ("http", "https"): return JSONResponse({"error": "URL must use http:// or https:// scheme"}, status_code=400) - if not parsed.netloc: + if not parsed.hostname: return JSONResponse({"error": "URL must include a hostname"}, status_code=400) + + is_local = _is_local_url(url) + reachable = await _probe_taosmd(request, url) + config = request.app.state.config config.memory_url = url await save_config_locked(config, request.app.state.config_path) # Keep app.state.taosmd_url in sync for runtime access request.app.state.taosmd_url = url - return {"status": "saved", "url": url} + + # Cache the probe result so the next GET is instant. + _MEMORY_URL_PROBE_CACHE[url] = (time.monotonic(), reachable) + if len(_MEMORY_URL_PROBE_CACHE) > 32: + _MEMORY_URL_PROBE_CACHE.popitem(last=False) + + return {"url": url, "is_local": is_local, "reachable": reachable}