From a39e4210c9a5d263e37ac4fe700939ac1286dc97 Mon Sep 17 00:00:00 2001 From: Lightning Pixel Date: Wed, 11 Nov 2026 10:25:29 +0100 Subject: [PATCH 1/2] feat(agent): ping Ollama and explain why it isn't answering Add GET /agent/ollama/health: checks the configured Ollama server (/api/tags, 3s timeout) and reports ok / unreachable / model_missing / no_model with the model name and pulled models. The chat panel pings it on mount and when the Ollama URL or model changes, and shows a banner with the fix ("ollama serve" or "ollama pull ", copyable) and a Retry button. A failed Ollama turn re-pings and replaces the raw provider error with the same advice. Only shown when Ollama is the active provider. Closes #303 --- api/routers/agent.py | 8 + api/services/ollama_health.py | 53 +++++++ api/tests/test_agent_ollama_health.py | 146 ++++++++++++++++++ package.json | 2 +- src/areas/generate/components/ChatPanel.tsx | 23 +++ .../generate/components/OllamaBanner.tsx | 85 ++++++++++ src/shared/utils/ollamaHealth.test.ts | 59 +++++++ src/shared/utils/ollamaHealth.ts | 74 +++++++++ 8 files changed, 449 insertions(+), 1 deletion(-) create mode 100644 api/services/ollama_health.py create mode 100644 api/tests/test_agent_ollama_health.py create mode 100644 src/areas/generate/components/OllamaBanner.tsx create mode 100644 src/shared/utils/ollamaHealth.test.ts create mode 100644 src/shared/utils/ollamaHealth.ts diff --git a/api/routers/agent.py b/api/routers/agent.py index ab6a49be..0e61a043 100644 --- a/api/routers/agent.py +++ b/api/routers/agent.py @@ -14,6 +14,7 @@ from pydantic import BaseModel, Field from services import llm_server +from services import ollama_health as ollama_health_service from services.llm_server import llama_pool from services.mesh_ops import MeshOp, MeshOpNotFoundError, MeshOpsRegistry, mesh_ops_registry @@ -572,6 +573,13 @@ async def list_external_models(req: ExternalModelsRequest): return {"models": []} +@router.get("/ollama/health") +async def ollama_health(base_url: str = "http://localhost:11434/v1", model: str = ""): + """Is Ollama up, and is `model` pulled? `status` is `ok`, `unreachable`, + `model_missing` or `no_model`; the chat panel turns it into a banner.""" + return await ollama_health_service.check(base_url, model) + + async def _unload_llm_after_workflow(request: AgentChatRequest, actions_done: list[ActionDone]) -> None: """Free the local LLM's VRAM once a workflow has been dispatched, so the workflow gets the full GPU. Best-effort — never fail the chat over it.""" diff --git a/api/services/ollama_health.py b/api/services/ollama_health.py new file mode 100644 index 00000000..9cfa38c3 --- /dev/null +++ b/api/services/ollama_health.py @@ -0,0 +1,53 @@ +""" +Ollama health check for the chat agent: is the server reachable, and is the +configured model pulled? Answers in a structured status the UI turns into an +actionable message ("ollama serve" / "ollama pull "). +""" +import httpx + +# Short: this runs when the chat panel opens, and a down Ollama must not stall it. +TIMEOUT_S = 3.0 + + +def native_base_url(base_url: str) -> str: + """The chat goes through Ollama's OpenAI-compatible API under `/v1`; the + native API (`/api/tags`) lives at the server root.""" + url = base_url.strip().rstrip("/") + if url.endswith("/v1"): + url = url[: -len("/v1")] + return url + + +def _with_tag(name: str) -> str: + # Ollama resolves an untagged name to `:latest`. + return name if ":" in name else f"{name}:latest" + + +async def check(base_url: str, model: str) -> dict: + root = native_base_url(base_url) + model = model.strip() + result: dict = {"status": "ok", "base_url": root, "model": model} + + try: + async with httpx.AsyncClient(timeout=TIMEOUT_S) as client: + r = await client.get(f"{root}/api/tags") + if r.status_code != 200: + return {**result, "status": "unreachable", "detail": f"HTTP {r.status_code} from {root}/api/tags"} + entries = r.json().get("models", []) + except httpx.HTTPError as e: + return {**result, "status": "unreachable", "detail": str(e) or type(e).__name__} + except (ValueError, AttributeError): + return {**result, "status": "unreachable", "detail": f"{root} did not answer like an Ollama server"} + + available = sorted({ + str(m.get("name") or m.get("model")) + for m in entries + if isinstance(m, dict) and (m.get("name") or m.get("model")) + }) + result["available"] = available + + if not model: + return {**result, "status": "no_model"} + if _with_tag(model) not in {_with_tag(n) for n in available}: + return {**result, "status": "model_missing"} + return result diff --git a/api/tests/test_agent_ollama_health.py b/api/tests/test_agent_ollama_health.py new file mode 100644 index 00000000..7052fd63 --- /dev/null +++ b/api/tests/test_agent_ollama_health.py @@ -0,0 +1,146 @@ +import asyncio +import unittest +from unittest import mock + +import httpx + +import routers.agent as agent +from services import ollama_health + + +class _MockClientFactory: + """Real AsyncClients wired to a MockTransport, so the health check talks to a + fake Ollama instead of the network.""" + + def __init__(self, handler) -> None: + self._handler = handler + self._real = httpx.AsyncClient + self.kwargs: list[dict] = [] + + def __call__(self, *args, **kwargs): + self.kwargs.append(dict(kwargs)) + kwargs["transport"] = httpx.MockTransport(self._handler) + return self._real(*args, **kwargs) + + +def _tags(*names: str) -> dict: + return {"models": [{"name": n, "model": n} for n in names]} + + +def _check(handler, base_url: str = "http://localhost:11434/v1", model: str = "qwen2.5:3b"): + factory = _MockClientFactory(handler) + with mock.patch.object(ollama_health.httpx, "AsyncClient", factory): + result = asyncio.run(agent.ollama_health(base_url=base_url, model=model)) + return result, factory + + +class OllamaHealthTests(unittest.TestCase): + def test_ok_when_reachable_and_model_pulled(self) -> None: + requests = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + return httpx.Response(200, json=_tags("llama3:8b", "qwen2.5:3b")) + + result, _ = _check(handler) + + self.assertEqual(result["status"], "ok") + self.assertEqual(result["model"], "qwen2.5:3b") + # The native API lives at the root, not under the OpenAI-compatible /v1. + self.assertEqual(str(requests[0].url), "http://localhost:11434/api/tags") + + def test_uses_the_configured_base_url(self) -> None: + requests = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + return httpx.Response(200, json=_tags("qwen2.5:3b")) + + result, _ = _check(handler, base_url="http://192.168.1.20:9999/v1/") + + self.assertEqual(result["status"], "ok") + self.assertEqual(str(requests[0].url), "http://192.168.1.20:9999/api/tags") + self.assertEqual(result["base_url"], "http://192.168.1.20:9999") + + def test_model_missing_when_not_pulled(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_tags("llama3:8b")) + + result, _ = _check(handler) + + self.assertEqual(result["status"], "model_missing") + self.assertEqual(result["model"], "qwen2.5:3b") + self.assertEqual(result["available"], ["llama3:8b"]) + + def test_untagged_model_matches_latest(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_tags("mistral:latest")) + + result, _ = _check(handler, model="mistral") + + self.assertEqual(result["status"], "ok") + + def test_tagged_model_does_not_match_a_different_tag(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_tags("qwen2.5:latest")) + + result, _ = _check(handler, model="qwen2.5:3b") + + self.assertEqual(result["status"], "model_missing") + + def test_no_model_configured(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_tags("llama3:8b")) + + result, _ = _check(handler, model=" ") + + self.assertEqual(result["status"], "no_model") + + def test_unreachable_on_connection_error(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("connection refused", request=request) + + result, _ = _check(handler) + + self.assertEqual(result["status"], "unreachable") + self.assertEqual(result["base_url"], "http://localhost:11434") + self.assertIn("connection refused", result["detail"]) + + def test_unreachable_on_timeout(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ReadTimeout("timed out", request=request) + + result, _ = _check(handler) + + self.assertEqual(result["status"], "unreachable") + + def test_unreachable_when_something_else_answers(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(404, text="not found") + + result, _ = _check(handler) + + self.assertEqual(result["status"], "unreachable") + self.assertIn("404", result["detail"]) + + def test_unreachable_on_non_json_body(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, text="hello") + + result, _ = _check(handler) + + self.assertEqual(result["status"], "unreachable") + + def test_uses_a_short_timeout(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_tags("qwen2.5:3b")) + + _, factory = _check(handler) + + timeout = factory.kwargs[0].get("timeout") + self.assertIsNotNone(timeout) + self.assertLessEqual(float(timeout), 5.0) + + +if __name__ == "__main__": + unittest.main() diff --git a/package.json b/package.json index 4c9732c1..315d90d1 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "prepare-resources": "node scripts/download-python-embed.js", "test": "npm run test:py && npm run test:node", "test:py": "node scripts/run-pytests.mjs", - "test:node": "node --test --experimental-strip-types --experimental-loader ./scripts/node-ts-extensionless-loader.mjs src/shared/types/assetLibrary.test.ts src/areas/generate/assetLibraryProjection.test.ts src/areas/generate/assetLibraryService.test.ts src/areas/generate/assetLibraryUi.test.ts src/areas/generate/orcaSlicerLink.test.ts src/shared/utils/agentStream.test.ts electron/main/artifact-registry-service.test.ts electron/main/extension-path-guard.test.ts electron/main/storage-directory-guard.test.ts electron/main/storage-ownership.test.ts electron/main/startup-error.test.ts electron/main/api-port.test.ts electron/preload/artifact-registry-preload.test.ts && node --test electron/main/*.test.mjs src/**/*.test.mjs", + "test:node": "node --test --experimental-strip-types --experimental-loader ./scripts/node-ts-extensionless-loader.mjs src/shared/types/assetLibrary.test.ts src/areas/generate/assetLibraryProjection.test.ts src/areas/generate/assetLibraryService.test.ts src/areas/generate/assetLibraryUi.test.ts src/areas/generate/orcaSlicerLink.test.ts src/shared/utils/agentStream.test.ts src/shared/utils/ollamaHealth.test.ts electron/main/artifact-registry-service.test.ts electron/main/extension-path-guard.test.ts electron/main/storage-directory-guard.test.ts electron/main/storage-ownership.test.ts electron/main/startup-error.test.ts electron/main/api-port.test.ts electron/preload/artifact-registry-preload.test.ts && node --test electron/main/*.test.mjs src/**/*.test.mjs", "package": "cross-env CSC_IDENTITY_AUTO_DISCOVERY=false npm run build && npm run prepare-resources && electron-builder", "package:mac": "cross-env CSC_IDENTITY_AUTO_DISCOVERY=false npm run build && npm run prepare-resources && electron-builder --mac --arm64", "lint": "eslint ." diff --git a/src/areas/generate/components/ChatPanel.tsx b/src/areas/generate/components/ChatPanel.tsx index 60b07e60..cf9ad2bb 100644 --- a/src/areas/generate/components/ChatPanel.tsx +++ b/src/areas/generate/components/ChatPanel.tsx @@ -7,6 +7,8 @@ import { useExtensionsStore } from '@shared/stores/extensionsStore' import { useWorkflowRunStore } from '@areas/workflows/workflowRunStore' import { buildAllWorkflowExtensions } from '@areas/workflows/mockExtensions' import { readAgentEvents, toolStopTitle } from '@shared/utils/agentStream' +import { adviceText, ollamaAdvice } from '@shared/utils/ollamaHealth' +import OllamaBanner, { useOllamaHealth } from './OllamaBanner' // ─── Types ──────────────────────────────────────────────────────────────────── @@ -302,6 +304,9 @@ export default function ChatPanel(): JSX.Element { const pushMeshUrl = useAppStore((s) => s.pushMeshUrl) const undoMesh = useAppStore((s) => s.undoMesh) + const isOllama = provider === 'ollama' + const ollama = useOllamaHealth(isOllama, apiUrl, providerBaseUrl(provider, externalConfig), model) + const workflows = useWorkflowsStore((s) => s.workflows) const saveWorkflow = useWorkflowsStore((s) => s.save) const setActiveWorkflow = useWorkflowsStore((s) => s.setActive) @@ -374,6 +379,7 @@ export default function ChatPanel(): JSX.Element { setError(null) const id = `a-${Date.now()}` let finished = false + let failure: string | null = null try { const context = { ...buildContext(), ...extraContext } @@ -440,6 +446,8 @@ export default function ChatPanel(): JSX.Element { break case 'done': finished = true + // A guardrail stop means the LLM answered: nothing to ask Ollama. + if (event.error && !event.stopped) failure = event.message patch((m) => ({ ...m, // An error comes after whatever text had already streamed; a @@ -457,6 +465,7 @@ export default function ChatPanel(): JSX.Element { } } if (!finished) setError('The response was interrupted.') + if (failure !== null && isOllama) await explainOllamaFailure(id, failure) } catch (e: unknown) { const msg = e instanceof Error ? e.message : String(e) setError(msg.includes('fetch') ? 'Cannot reach Modly API. Is the backend running?' : msg) @@ -472,6 +481,17 @@ export default function ChatPanel(): JSX.Element { } } + // A failed Ollama turn: re-ping it (refreshing the banner) and, when it's + // down or the model isn't pulled, say so instead of the raw provider error. + async function explainOllamaFailure(id: string, rawError: string) { + const health = await ollama.check() + const advice = health ? ollamaAdvice(health) : null + if (!advice) return + setMessages((prev) => prev.map((m) => (m.id === id + ? { ...m, content: m.content.replace(rawError, adviceText(advice)) } + : m))) + } + async function applyToolPayload(payload: AgentActionPayload | null) { if (payload?.type === 'mesh_update' && payload.url) { updateCurrentJob({ outputUrl: payload.url }) @@ -663,6 +683,9 @@ export default function ChatPanel(): JSX.Element { + {/* Ollama down / model not pulled */} + { void ollama.check() }} /> + {/* Input bar */}
(null) + const [checking, setChecking] = useState(false) + const seq = useRef(0) + + const check = useCallback(async (): Promise => { + if (!enabled) return null + const n = ++seq.current + setChecking(true) + let result: OllamaHealth | null = null + try { + const res = await fetch(ollamaHealthUrl(apiUrl, baseUrl, model)) + result = res.ok ? parseOllamaHealth(await res.json()) : null + } catch { + result = null + } + // A later check (model switched mid-flight) wins. + if (n === seq.current) { + setHealth(result) + setChecking(false) + } + return result + }, [enabled, apiUrl, baseUrl, model]) + + useEffect(() => { void check() }, [check]) + + return { health: enabled ? health : null, checking, check } +} + +interface Props { + health: OllamaHealth | null + checking: boolean + onRetry: () => void +} + +export default function OllamaBanner({ health, checking, onRetry }: Props): JSX.Element | null { + const [copied, setCopied] = useState(false) + const advice = health ? ollamaAdvice(health) : null + if (!advice) return null + + function copy(command: string) { + navigator.clipboard?.writeText(command).then(() => { + setCopied(true) + setTimeout(() => setCopied(false), 1500) + }).catch(() => {}) + } + + return ( +
+
+

{advice.title}

+ +
+

{advice.body}

+ {advice.command && ( +
+ {advice.command} + +
+ )} +
+ ) +} diff --git a/src/shared/utils/ollamaHealth.test.ts b/src/shared/utils/ollamaHealth.test.ts new file mode 100644 index 00000000..5c39d64b --- /dev/null +++ b/src/shared/utils/ollamaHealth.test.ts @@ -0,0 +1,59 @@ +import assert from 'node:assert/strict' +import test from 'node:test' + +import { adviceText, ollamaAdvice, ollamaHealthUrl, parseOllamaHealth } from './ollamaHealth.ts' + +const base = { base_url: 'http://localhost:11434', model: 'qwen2.5:3b' } + +test('the health URL carries the configured base URL and model, encoded', () => { + const url = ollamaHealthUrl('http://127.0.0.1:8765', 'http://localhost:11434/v1', 'qwen2.5:3b') + assert.equal( + url, + 'http://127.0.0.1:8765/agent/ollama/health?base_url=http%3A%2F%2Flocalhost%3A11434%2Fv1&model=qwen2.5%3A3b', + ) +}) + +test('ok needs no advice', () => { + assert.equal(ollamaAdvice({ ...base, status: 'ok' }), null) +}) + +test('unreachable tells the user to start Ollama, with the command', () => { + const advice = ollamaAdvice({ ...base, status: 'unreachable', detail: 'connection refused' }) + assert.ok(advice) + assert.match(advice.title, /not running/i) + assert.match(advice.body, /localhost:11434/) + assert.equal(advice.command, 'ollama serve') +}) + +test('model_missing tells the user to pull the configured model', () => { + const advice = ollamaAdvice({ ...base, status: 'model_missing', available: ['llama3:8b'] }) + assert.ok(advice) + assert.match(advice.title, /qwen2\.5:3b/) + assert.equal(advice.command, 'ollama pull qwen2.5:3b') +}) + +test('no_model points at Settings and has no command', () => { + const advice = ollamaAdvice({ ...base, model: '', status: 'no_model' }) + assert.ok(advice) + assert.match(advice.body, /Settings → Agent/) + assert.equal(advice.command, undefined) +}) + +test('adviceText flattens the advice into one plain-text chat line', () => { + assert.equal( + adviceText({ title: 'Ollama is not running', body: 'Start it, or run:', command: 'ollama serve' }), + 'Ollama is not running. Start it, or run: ollama serve', + ) + assert.equal(adviceText({ title: 'No model', body: 'Pick one.' }), 'No model. Pick one.') +}) + +test('a well-formed response parses', () => { + const parsed = parseOllamaHealth({ ...base, status: 'model_missing', available: ['a'], extra: 1 }) + assert.deepEqual(parsed, { ...base, status: 'model_missing', available: ['a'], detail: undefined }) +}) + +test('an unknown status or a non-object is rejected', () => { + assert.equal(parseOllamaHealth({ ...base, status: 'weird' }), null) + assert.equal(parseOllamaHealth(null), null) + assert.equal(parseOllamaHealth('ok'), null) +}) diff --git a/src/shared/utils/ollamaHealth.ts b/src/shared/utils/ollamaHealth.ts new file mode 100644 index 00000000..bab642b8 --- /dev/null +++ b/src/shared/utils/ollamaHealth.ts @@ -0,0 +1,74 @@ +// GET /agent/ollama/health — is Ollama up, and is the configured model pulled? + +export type OllamaStatus = 'ok' | 'unreachable' | 'model_missing' | 'no_model' + +const STATUSES: readonly OllamaStatus[] = ['ok', 'unreachable', 'model_missing', 'no_model'] + +export interface OllamaHealth { + status: OllamaStatus + /** Ollama's server root (no `/v1`). */ + base_url: string + model: string + /** Models pulled in Ollama (absent when it is unreachable). */ + available?: string[] + detail?: string +} + +export interface OllamaAdvice { + title: string + body: string + /** A shell command that fixes it, shown copyable. */ + command?: string +} + +export function ollamaHealthUrl(apiUrl: string, baseUrl: string, model: string): string { + const query = new URLSearchParams({ base_url: baseUrl, model }) + return `${apiUrl}/agent/ollama/health?${query.toString()}` +} + +export function parseOllamaHealth(data: unknown): OllamaHealth | null { + if (!data || typeof data !== 'object') return null + const d = data as Record + if (!STATUSES.includes(d.status as OllamaStatus)) return null + return { + status: d.status as OllamaStatus, + base_url: typeof d.base_url === 'string' ? d.base_url : '', + model: typeof d.model === 'string' ? d.model : '', + ...(Array.isArray(d.available) ? { available: d.available.filter((m): m is string => typeof m === 'string') } : {}), + detail: typeof d.detail === 'string' ? d.detail : undefined, + } +} + +function host(baseUrl: string): string { + return baseUrl.replace(/^https?:\/\//, '') || 'its configured address' +} + +/** The advice as one plain-text line, for a chat message. */ +export function adviceText(advice: OllamaAdvice): string { + return [`${advice.title}.`, advice.body, advice.command].filter(Boolean).join(' ') +} + +/** What to tell the user, or null when Ollama is ready. */ +export function ollamaAdvice(health: OllamaHealth): OllamaAdvice | null { + switch (health.status) { + case 'ok': + return null + case 'unreachable': + return { + title: 'Ollama is not running', + body: `The agent can't reach Ollama at ${host(health.base_url)}. Start the Ollama app, or run:`, + command: 'ollama serve', + } + case 'model_missing': + return { + title: `Model "${health.model}" is not pulled`, + body: 'Ollama is running but doesn\'t have this model yet. Download it with:', + command: `ollama pull ${health.model}`, + } + case 'no_model': + return { + title: 'No Ollama model selected', + body: 'Pick the model to chat with in Settings → Agent.', + } + } +} From 374b8e5613b7e2213be56848d6fda0b304ce635c Mon Sep 17 00:00:00 2001 From: Lightning Pixel Date: Wed, 11 Nov 2026 10:31:29 +0100 Subject: [PATCH 2/2] fix(agent): only probe local Ollama addresses from the health check The API answers any origin (CORS *), so a web page could call /agent/ollama/health with any base_url and read the result, turning it into a probe of the user's network. Accept only http(s) loopback URLs without credentials (400 otherwise), don't follow redirects, and report generic reasons ("connection refused", "timed out", "not an Ollama server") instead of echoing the upstream status or exception text. Refs #303 --- api/routers/agent.py | 10 ++++-- api/services/ollama_health.py | 37 ++++++++++++++++++--- api/tests/test_agent_ollama_health.py | 46 +++++++++++++++++++++++---- 3 files changed, 78 insertions(+), 15 deletions(-) diff --git a/api/routers/agent.py b/api/routers/agent.py index 0e61a043..1eabd6b3 100644 --- a/api/routers/agent.py +++ b/api/routers/agent.py @@ -9,7 +9,7 @@ from typing import Optional import httpx -from fastapi import APIRouter +from fastapi import APIRouter, HTTPException from fastapi.responses import StreamingResponse from pydantic import BaseModel, Field @@ -576,8 +576,12 @@ async def list_external_models(req: ExternalModelsRequest): @router.get("/ollama/health") async def ollama_health(base_url: str = "http://localhost:11434/v1", model: str = ""): """Is Ollama up, and is `model` pulled? `status` is `ok`, `unreachable`, - `model_missing` or `no_model`; the chat panel turns it into a banner.""" - return await ollama_health_service.check(base_url, model) + `model_missing` or `no_model`; the chat panel turns it into a banner. + Only local addresses are accepted (400 otherwise).""" + try: + return await ollama_health_service.check(base_url, model) + except ValueError as e: + raise HTTPException(400, str(e)) async def _unload_llm_after_workflow(request: AgentChatRequest, actions_done: list[ActionDone]) -> None: diff --git a/api/services/ollama_health.py b/api/services/ollama_health.py index 9cfa38c3..190608c6 100644 --- a/api/services/ollama_health.py +++ b/api/services/ollama_health.py @@ -3,11 +3,32 @@ configured model pulled? Answers in a structured status the UI turns into an actionable message ("ollama serve" / "ollama pull "). """ +from urllib.parse import urlsplit + import httpx # Short: this runs when the chat panel opens, and a down Ollama must not stall it. TIMEOUT_S = 3.0 +# The API answers any origin (CORS *), so a web page could call this endpoint. +# Only local addresses are probed, and nothing of the upstream answer is echoed, +# so it can't be used to map the user's network. +LOOPBACK_HOSTS = {"localhost", "127.0.0.1", "::1"} + + +def is_local_url(url: str) -> bool: + try: + parts = urlsplit(url) + host = parts.hostname + except ValueError: + return False + return ( + parts.scheme in ("http", "https") + and not parts.username + and not parts.password + and host in LOOPBACK_HOSTS + ) + def native_base_url(base_url: str) -> str: """The chat goes through Ollama's OpenAI-compatible API under `/v1`; the @@ -24,20 +45,26 @@ def _with_tag(name: str) -> str: async def check(base_url: str, model: str) -> dict: + """Raises ValueError when `base_url` is not a local http(s) address.""" root = native_base_url(base_url) + if not is_local_url(root): + raise ValueError("base_url must be a local http(s) address (localhost, 127.0.0.1 or ::1)") model = model.strip() result: dict = {"status": "ok", "base_url": root, "model": model} + not_ollama = {**result, "status": "unreachable", "detail": "not an Ollama server"} try: - async with httpx.AsyncClient(timeout=TIMEOUT_S) as client: + async with httpx.AsyncClient(timeout=TIMEOUT_S, follow_redirects=False) as client: r = await client.get(f"{root}/api/tags") if r.status_code != 200: - return {**result, "status": "unreachable", "detail": f"HTTP {r.status_code} from {root}/api/tags"} + return not_ollama entries = r.json().get("models", []) - except httpx.HTTPError as e: - return {**result, "status": "unreachable", "detail": str(e) or type(e).__name__} + except httpx.TimeoutException: + return {**result, "status": "unreachable", "detail": "timed out"} + except httpx.HTTPError: + return {**result, "status": "unreachable", "detail": "connection refused"} except (ValueError, AttributeError): - return {**result, "status": "unreachable", "detail": f"{root} did not answer like an Ollama server"} + return not_ollama available = sorted({ str(m.get("name") or m.get("model")) diff --git a/api/tests/test_agent_ollama_health.py b/api/tests/test_agent_ollama_health.py index 7052fd63..8452722b 100644 --- a/api/tests/test_agent_ollama_health.py +++ b/api/tests/test_agent_ollama_health.py @@ -3,6 +3,7 @@ from unittest import mock import httpx +from fastapi import HTTPException import routers.agent as agent from services import ollama_health @@ -56,11 +57,41 @@ def handler(request: httpx.Request) -> httpx.Response: requests.append(request) return httpx.Response(200, json=_tags("qwen2.5:3b")) - result, _ = _check(handler, base_url="http://192.168.1.20:9999/v1/") + result, _ = _check(handler, base_url="http://127.0.0.1:9999/v1/") self.assertEqual(result["status"], "ok") - self.assertEqual(str(requests[0].url), "http://192.168.1.20:9999/api/tags") - self.assertEqual(result["base_url"], "http://192.168.1.20:9999") + self.assertEqual(str(requests[0].url), "http://127.0.0.1:9999/api/tags") + self.assertEqual(result["base_url"], "http://127.0.0.1:9999") + + def test_rejects_urls_that_are_not_local(self) -> None: + # The API answers any origin (CORS *): a web page must not be able to use + # this endpoint to probe the user's network. + requests = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + return httpx.Response(200, json=_tags("qwen2.5:3b")) + + for url in ( + "http://192.168.1.20:11434/v1", + "http://example.com/v1", + "http://user:pass@localhost:11434/v1", + "file:///etc/passwd", + "ftp://localhost/v1", + "http://localhost.evil.com:11434", + ): + with self.assertRaises(HTTPException, msg=url) as ctx: + _check(handler, base_url=url) + self.assertEqual(ctx.exception.status_code, 400, url) + self.assertEqual(requests, []) + + def test_accepts_loopback_hosts(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=_tags("qwen2.5:3b")) + + for url in ("http://localhost:11434/v1", "http://127.0.0.1:11434", "http://[::1]:11434/v1"): + result, _ = _check(handler, base_url=url) + self.assertEqual(result["status"], "ok", url) def test_model_missing_when_not_pulled(self) -> None: def handler(request: httpx.Request) -> httpx.Response: @@ -98,13 +129,13 @@ def handler(request: httpx.Request) -> httpx.Response: def test_unreachable_on_connection_error(self) -> None: def handler(request: httpx.Request) -> httpx.Response: - raise httpx.ConnectError("connection refused", request=request) + raise httpx.ConnectError("[Errno 111] internal detail", request=request) result, _ = _check(handler) self.assertEqual(result["status"], "unreachable") self.assertEqual(result["base_url"], "http://localhost:11434") - self.assertIn("connection refused", result["detail"]) + self.assertEqual(result["detail"], "connection refused") def test_unreachable_on_timeout(self) -> None: def handler(request: httpx.Request) -> httpx.Response: @@ -116,12 +147,13 @@ def handler(request: httpx.Request) -> httpx.Response: def test_unreachable_when_something_else_answers(self) -> None: def handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(404, text="not found") + return httpx.Response(404, text="secret upstream body") result, _ = _check(handler) self.assertEqual(result["status"], "unreachable") - self.assertIn("404", result["detail"]) + # Generic: nothing of the upstream answer is echoed back to the caller. + self.assertEqual(result["detail"], "not an Ollama server") def test_unreachable_on_non_json_body(self) -> None: def handler(request: httpx.Request) -> httpx.Response: