From d04f2b3643605c1eecc853e431d8f7f3d80a6ad7 Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Fri, 1 May 2026 15:12:04 +0530 Subject: [PATCH] fix(explorer): address Qodo review findings for ontology hub (#518) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug 1 — Broken registry filters: fetchRegistry no longer sends format/kind values (owl/skos/internal/external) as the status query param; those filters are applied client-side via filteredEntries which already had the correct logic. Only the text search param q is delegated to the backend. Bug 2 — Toggle/refresh URI corruption: Removed removesuffix('/toggle') and removesuffix('/refresh') from toggle_ontology and refresh_ontology. Starlette's route regex already strips the literal suffix from the captured path param; the removesuffix call was a no-op for normal URIs but corrupted any ontology URI that legitimately ends with /toggle or /refresh. Bug 3 — SSRF in URL fetch: Added _validate_fetch_url() which rejects non-http/https schemes and resolves the hostname to block private, loopback, link-local, reserved, and multicast addresses before requests.get() is called. Applied to all three fetch sites: preview, load, and refresh. Bug 5 — Inconsistent XML hardening: _parse_rdf_sync now calls _safe_parse_rdf() from semantica/explorer/utils/rdf_parser.py instead of g.parse() directly, applying the existing defusedxml-based XXE protection for RDF/XML inputs. Bug 6 — Search scans whole graph: search_entities now calls session.search(q, limit*6) which hits the GraphSearchIndex instead of fetching up to 999,999 nodes and doing a linear Python substring scan. Results are post-filtered by _SEARCHABLE_TYPES and entity_type before being returned up to the requested limit. --- .../OntologyWorkspace/OntologyManager.tsx | 3 +- semantica/explorer/routes/ontology.py | 57 +++++++++++++------ 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/explorer/src/workspaces/OntologyWorkspace/OntologyManager.tsx b/explorer/src/workspaces/OntologyWorkspace/OntologyManager.tsx index 844915bf..26058149 100644 --- a/explorer/src/workspaces/OntologyWorkspace/OntologyManager.tsx +++ b/explorer/src/workspaces/OntologyWorkspace/OntologyManager.tsx @@ -255,7 +255,8 @@ export function OntologyManager() { try { const params = new URLSearchParams(); if (searchQ) params.set("q", searchQ); - if (statusFilter !== "all") params.set("status", statusFilter); + // format/kind filters (owl/skos/internal/external) are applied client-side + // via filteredEntries; only text search is delegated to the backend const res = await fetch(`/api/ontology/registry?${params}`); if (!res.ok) throw new Error("Failed to load registry"); setEntries(await res.json()); diff --git a/semantica/explorer/routes/ontology.py b/semantica/explorer/routes/ontology.py index aa49124a..92e36fe0 100644 --- a/semantica/explorer/routes/ontology.py +++ b/semantica/explorer/routes/ontology.py @@ -3,16 +3,20 @@ Ontology Hub routes: registry, URL/file loading, preview, creation, entity searc """ import asyncio +import ipaddress import re +import socket import uuid from datetime import UTC, datetime from typing import Any, Dict, List, Literal, Optional +from urllib.parse import urlparse from fastapi import APIRouter, Depends, HTTPException, Query, Request from pydantic import BaseModel, Field from ..dependencies import get_session from ..session import GraphSession +from ..utils.rdf_parser import _safe_parse_rdf router = APIRouter(prefix="/api/ontology", tags=["Ontology"]) @@ -273,7 +277,32 @@ def _normalize_format(fmt: Optional[str]) -> str: return _FORMAT_ALIASES.get(lower, lower) +def _validate_fetch_url(url: str) -> None: + """Reject non-HTTP(S) schemes and private/loopback/link-local targets.""" + parsed = urlparse(url) + if parsed.scheme not in ("http", "https"): + raise HTTPException(status_code=422, detail="Only http and https URLs are allowed.") + hostname = parsed.hostname + if not hostname: + raise HTTPException(status_code=422, detail="Invalid URL: missing hostname.") + try: + addrinfos = socket.getaddrinfo(hostname, None) + except socket.gaierror as exc: + raise HTTPException(status_code=422, detail=f"Cannot resolve hostname '{hostname}': {exc}") from exc + for _family, _type, _proto, _canonname, sockaddr in addrinfos: + try: + ip = ipaddress.ip_address(sockaddr[0]) + except ValueError: + continue + if ip.is_loopback or ip.is_private or ip.is_link_local or ip.is_reserved or ip.is_multicast: + raise HTTPException( + status_code=422, + detail="Fetching from private, loopback, or reserved network addresses is not allowed.", + ) + + def _fetch_url_sync(url: str) -> bytes: + _validate_fetch_url(url) import requests as _req try: resp = _req.get( @@ -281,6 +310,7 @@ def _fetch_url_sync(url: str) -> bytes: headers={"Accept": "text/turtle, application/rdf+xml, application/ld+json, */*;q=0.1"}, timeout=30, stream=True, + allow_redirects=True, ) resp.raise_for_status() chunks: List[bytes] = [] @@ -312,7 +342,7 @@ def _parse_rdf_sync(content: bytes, fmt: str) -> tuple: g = rdflib.Graph() try: - g.parse(data=content, format=parse_fmt) + _safe_parse_rdf(g, content, parse_fmt) except Exception as exc: raise HTTPException(status_code=422, detail=f"RDF parse error: {exc}") from exc @@ -654,11 +684,12 @@ async def search_entities( limit: int = Query(default=50, ge=1, le=200), session: GraphSession = Depends(get_session), ): - all_nodes, _ = await asyncio.to_thread(session.get_nodes, skip=0, limit=999_999) + # Use the session's indexed search; over-fetch to allow post-filtering by entity type + raw_hits = await asyncio.to_thread(session.search, q, limit * 6) results: List[OntologySearchResult] = [] - q_lower = q.lower() - for node in all_nodes: + for hit in raw_hits: + node = hit.get("node", hit) # session.search returns {"node": ..., "score": ...} ntype = node.get("type", "") if ntype not in _SEARCHABLE_TYPES: continue @@ -673,9 +704,6 @@ async def search_entities( or props.get("skos:definition") or props.get("description") ) - corpus = " ".join(filter(None, [label, node.get("id", ""), definition or ""])).lower() - if q_lower not in corpus: - continue results.append(OntologySearchResult( uri=node.get("id", ""), @@ -821,14 +849,12 @@ async def remove_ontology(ontology_uri: str, request: Request): @router.patch("/{ontology_uri:path}/toggle", response_model=ToggleResponse) async def toggle_ontology(ontology_uri: str, request: Request): - # Strip the /toggle suffix that FastAPI includes when path ends with it - uri = ontology_uri.removesuffix("/toggle") registry = _get_registry(request) - if uri not in registry: + if ontology_uri not in registry: raise HTTPException(status_code=404, detail="Ontology not found in registry.") - entry = registry[uri] + entry = registry[ontology_uri] entry.enabled = not entry.enabled - return ToggleResponse(uri=uri, enabled=entry.enabled) + return ToggleResponse(uri=ontology_uri, enabled=entry.enabled) @router.post("/{ontology_uri:path}/refresh", response_model=RefreshResponse) @@ -837,11 +863,10 @@ async def refresh_ontology( request: Request, session: GraphSession = Depends(get_session), ): - uri = ontology_uri.removesuffix("/refresh") registry = _get_registry(request) - if uri not in registry: + if ontology_uri not in registry: raise HTTPException(status_code=404, detail="Ontology not found in registry.") - entry = registry[uri] + entry = registry[ontology_uri] if not entry.source_url: raise HTTPException(status_code=422, detail="No source URL to refresh from.") @@ -861,4 +886,4 @@ async def refresh_ontology( edges_added = await asyncio.to_thread(session.add_edges, edges) entry.loaded_at = datetime.now(UTC).isoformat() - return RefreshResponse(uri=uri, nodes_added=nodes_added, edges_added=edges_added) + return RefreshResponse(uri=ontology_uri, nodes_added=nodes_added, edges_added=edges_added)