Merge pull request #898 from Sunil56224972/security/fix-critical-vulnerabilities

security: fix 4 critical vulnerabilities (RCE, SSRF, XXE, DoS)
2026-08-29 04:26:20 +00:00 · 2026-08-11 15:50:06 +05:30
parent 3496d62335 6002965c55
commit 6ec546b551
12 changed files with 652 additions and 61 deletions
@@ -244,6 +244,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ### Security

+- **4 critical/high vulnerabilities in the Explorer API and vector store: RCE, SSRF, XXE, and DoS, plus Cypher/SPARQL injection hardening found along the way** (#898) by @Sunil56224972
+  - **[CWE-502] Arbitrary code execution via `pickle.load()`**: `VectorStore.save()`/`load()` used `pickle` for the on-disk `store_data.pkl`; a crafted `.pkl` file placed in the store directory (file upload, shared filesystem, or supply-chain compromise) could execute arbitrary code on deserialization. Replaced with JSON — vectors and metadata are fully JSON-serializable, so nothing is lost — and `load()` now refuses any legacy `.pkl` file it finds with a migration error rather than deserializing it
+  - **[CWE-918] SSRF via redirect bypass in `ontology.py`'s URL fetcher**: `_validate_fetch_url()` correctly blocked private/loopback/reserved addresses on the caller-supplied URL, but `_fetch_url_sync()` fetched with `allow_redirects=True`, so a validated *public* first hop could 302 to `http://169.254.169.254/...` (cloud instance metadata) or an internal service, and `requests` followed it with no re-check. Redirects are now followed manually, capped at 5 hops, with `_validate_fetch_url()` re-run against every hop's target — including relative `Location` headers, resolved via `urljoin()` before validation — and every response (redirect or final) is explicitly closed to avoid leaking connections back to the pool
+  - **[CWE-611] XXE injection in the RDF/XML parser**: `_safe_parse_rdf()` depended on `defusedxml` for XXE protection, but `defusedxml` wasn't declared in `pyproject.toml`'s `explorer` extra, so it was silently absent in normal installs and the code fell back to a bare warning plus unsafe parsing — a crafted RDF/XML ontology with an external entity could read arbitrary server files. Added `defusedxml>=0.7.1` to the extra, and `_safe_parse_rdf()` now fails closed: it raises rather than parsing untrusted RDF/XML if `defusedxml` isn't importable, replacing an earlier regex-based DOCTYPE-stripping fallback that was reviewed and rejected as bypassable
+  - **[CWE-770] DoS via unbounded SPARQL graph materialization**: `_build_rdflib_graph()` loaded up to 999,999 nodes and 999,999 edges into memory per query, and with up to 4 concurrent SPARQL requests permitted, an attacker could exhaust server memory. Added a 50,000 node/edge cap (`_SPARQL_MAX_GRAPH_NODES`); oversized graphs now return a clean error instead of attempting materialization
+  - **Cypher injection via Apache AGE's `graph_name` and `$$`-delimiter breakout**: `graph_name` was interpolated unvalidated into `cypher('{graph_name}', $$ ... $$)`, and raw Cypher query text containing `$$` could close AGE's dollar-quoted string delimiter early and append arbitrary SQL. `graph_name` is now validated against the same identifier allowlist `age_store.py` already used for labels/relationship types, and any query containing `$$` is rejected outright
+  - **SPARQL Explorer route (`/api/sparql`) hardened against comment/PREFIX-hiding bypass**: `_is_read_only_query()` now strips comments and PREFIX/BASE declarations before checking the leading keyword, and additionally scans the full query body for SPARQL Update keywords (INSERT/DELETE/DROP/LOAD/CLEAR/CREATE/COPY/MOVE/ADD) — so `SELECT ... ; DROP ALL` is now rejected by the keyword scan itself rather than relying solely on rdflib's parser
+  - **Fixed along the way** (maintainer follow-up, addressing automated review findings and a regression introduced across several rounds of iteration on the original fix):
+    - `VectorStore.save()`'s numpy handling used `list(v)` for the JSON fallback path, which produces `numpy.float32` elements that `json.dump()` can't serialize — changed to `v.tolist()`
+    - the SPARQL graph-size `ValueError` was raised outside `execute_sparql()`'s exception handling and surfaced as an unhandled 500 instead of a clean API error — moved inside
+    - every streamed `requests` response in the ontology redirect loop, including the one actually read and returned, is now closed in a `finally` block — a connection-pool leak that a rework of the redirect logic had briefly reintroduced after an earlier fix
+    - a later commit meant to add opt-in API-key auth (`explorer/auth.py`, gated on `EXPLORER_API_KEY`) instead **replaced and silently disabled** the `Depends(require_auth)` enforcement already merged into `main` for GHSA-j4mq-hprp-987v (Critical — unauthenticated Explorer API), removed the `/ws/graph-updates` handshake check, and — unlike `require_auth` — failed *open* (allowed all requests) whenever its key was unset. Merging that version would have silently reverted an already-fixed Critical CVE the moment this branch landed. Removed `explorer/auth.py`; restored the per-router `Depends(require_auth)` wiring and the WebSocket auth check; kept the one genuine improvement in that commit (adding `X-API-Key` to the CORS `allow_headers` list) by folding it into the existing CORS config
+    - the new SPARQL keyword-scan's comment-stripping regex (`#[^\n]*`) also matched the `#` inside standard RDF namespace IRIs (e.g. `.../1999/02/22-rdf-syntax-ns#`), corrupting any query with a normal `rdf:`/`rdfs:`-style `PREFIX` declaration — caught because the hardening's own bundled tests failed against two of its own cases. Fixed by only treating `#` as a comment-start at line-start or after whitespace; the companion `PREFIX`/`BASE` regex was also fixed to accept bare `BASE <...>` declarations, which have no prefix-name token between the keyword and the IRI
+  - New/updated regression tests: `tests/explorer/test_ontology_ssrf.py` (redirect re-validation, relative-redirect resolution, response closing, redirect-cap enforcement), `tests/test_security_regression.py` (Cypher/SPARQL injection, XXE, numpy serialization, SSRF redirect handling), plus additions to `tests/explorer/test_sparql_route.py`, `tests/vector_store/test_vector_store.py`, and `tests/explorer/test_explorer_auth.py`
+  - Note: the Cypher-injection hardening here is scoped to `age_store.py`'s `graph_name`/`$$` breakout, found while reviewing this PR. The broader label/property-key/relationship-type injection across the Neptune, Neo4j, and FalkorDB backends (GHSA-482h-hw99-h62p, #910) and the triplet-store SPARQL injection across Blazegraph/RDF4J/Jena (GHSA-8vgg-8mr4-r236, #911) are covered by separate, still-open PRs, as is the unauthenticated-Explorer-API fix referenced above (GHSA-j4mq-hprp-987v, #909, already merged)
+
 - **CI/CD supply-chain hardening against mutable-tag Action compromise (LiteLLM/Trivy-class attack)** (#824) by @KaifAhmad1
  - Every third-party GitHub Action across all 8 workflows is now pinned to a full commit SHA instead of a mutable tag (`@v7` → `@3d3c42e... # v7`), closing the exact vector used against LiteLLM in March 2026 (a compromised Trivy Action tag stole a long-lived publishing token)
  - Added `verify-action-pins.yml` + `.github/scripts/verify-action-pins.sh`: a CI check that fails closed on any `uses:` reference that isn't a full SHA (catching a newly introduced mutable tag, not just auditing existing pins) and re-verifies every pin against the GitHub API on each workflow change, on push to `main`, and weekly; an unresolvable API lookup is treated as a failure rather than a silent skip
@@ -233,7 +233,8 @@ explorer = [
  "fastapi>=0.100.0",
  "uvicorn[standard]>=0.22.0",
  "websockets>=15.0.1",
-  "python-multipart>=0.0.6"
+  "python-multipart>=0.0.6",
+  "defusedxml>=0.7.1"
 ]
 explorer-lite = [
  "streamlit>=1.25.0",
@@ -130,10 +130,10 @@ def create_app(
    app.state.explorer_settings = settings

    # allow_credentials lets browsers send cookies/auth headers cross-origin.
-    # The Explorer has no authentication, so credentials serve no purpose and
-    # enabling them when origins are broadened creates cross-site request risk.
-    # Set EXPLORER_CORS_CREDENTIALS=true explicitly to opt in (e.g. for a
-    # reverse-proxy setup that injects its own auth layer).
+    # Credentials aren't needed for the X-API-Key auth scheme below, and
+    # enabling them when origins are broadened creates cross-site request
+    # risk. Set EXPLORER_CORS_CREDENTIALS=true explicitly to opt in (e.g.
+    # for a reverse-proxy setup that injects its own cookie-based auth).
    _allow_credentials = os.environ.get("EXPLORER_CORS_CREDENTIALS", "false").lower() == "true"
    app.add_middleware(
        CORSMiddleware,
@@ -11,7 +11,7 @@ import uuid
 from datetime import datetime, UTC
 from difflib import SequenceMatcher
 from typing import Any, Dict, List, Optional, Tuple
-from urllib.parse import urlparse
+from urllib.parse import urljoin, urlparse
 from typing_extensions import Literal

 from fastapi import APIRouter, Depends, HTTPException, Query, Request
@@ -1005,23 +1005,42 @@ def _validate_fetch_url(url: str) -> None:
 def _fetch_url_sync(url: str) -> bytes:
    _validate_fetch_url(url)
    import requests as _req
+    _MAX_REDIRECTS = 5
+    current_url = url
    try:
-        resp = _req.get(
-            url,
-            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] = []
-        total = 0
-        for chunk in resp.iter_content(65536):
-            total += len(chunk)
-            if total > _MAX_FETCH_BYTES:
-                raise HTTPException(status_code=413, detail="Remote resource exceeds 20 MB limit.")
-            chunks.append(chunk)
-        return b"".join(chunks)
+        for _ in range(_MAX_REDIRECTS + 1):
+            resp = _req.get(
+                current_url,
+                headers={"Accept": "text/turtle, application/rdf+xml, application/ld+json, */*;q=0.1"},
+                timeout=30,
+                stream=True,
+                allow_redirects=False,  # SECURITY: follow redirects manually
+            )
+            if resp.is_redirect or resp.is_permanent_redirect:
+                redirect_url = resp.headers.get("Location")
+                resp.close()  # Release the streamed connection before following the redirect
+                if not redirect_url:
+                    raise HTTPException(status_code=502, detail="Redirect without Location header.")
+                # Resolve relative redirects (e.g. /ontology.ttl) against the current URL
+                redirect_url = urljoin(current_url, redirect_url)
+                # Re-validate the redirect target to prevent SSRF via
+                # open-redirect to internal/cloud-metadata endpoints.
+                _validate_fetch_url(redirect_url)
+                current_url = redirect_url
+                continue
+            try:
+                resp.raise_for_status()
+                chunks: List[bytes] = []
+                total = 0
+                for chunk in resp.iter_content(65536):
+                    total += len(chunk)
+                    if total > _MAX_FETCH_BYTES:
+                        raise HTTPException(status_code=413, detail="Remote resource exceeds 20 MB limit.")
+                    chunks.append(chunk)
+                return b"".join(chunks)
+            finally:
+                resp.close()  # Release the streamed connection once fully read (or on error)
+        raise HTTPException(status_code=502, detail=f"Too many redirects (max {_MAX_REDIRECTS}).")
    except HTTPException:
        raise
    except Exception as exc:
@@ -3,11 +3,16 @@ SPARQL routes backed by an in-memory rdflib projection of the current graph.

 Security contract
 -----------------
-* Only SELECT, ASK, CONSTRUCT, and DESCRIBE are accepted (allowlist enforced
-  before graph construction so rejected queries never touch the session).
-* Multi-statement injections that start with an allowed keyword (e.g.
-  ``SELECT ... ; DROP ALL``) pass the prefix check and reach rdflib, which
-  rejects non-SELECT/ASK/CONSTRUCT/DESCRIBE update syntax in the parser.
+* Only SELECT, ASK, CONSTRUCT, and DESCRIBE are accepted, and the query
+  body is scanned for SPARQL Update keywords (INSERT/DELETE/DROP/LOAD/
+  CLEAR/CREATE/COPY/MOVE/ADD) after stripping comments and PREFIX/BASE
+  declarations — both enforced before graph construction, so rejected
+  queries never touch the session. A multi-statement injection appended
+  after an allowed keyword (e.g. ``SELECT ... ; DROP ALL``) is caught by
+  the keyword scan itself, not left to rdflib's parser.
+* rdflib's parser remains a second line of defense for malformed multi-
+  statement syntax that doesn't contain any forbidden keyword (e.g.
+  ``SELECT ... ; ASK ...``), which SPARQL 1.1 Query doesn't permit.
 * The in-memory rdflib graph is a read-only projection — the live
  ``GraphSession`` is never mutated by this route.
 """
@@ -26,14 +31,54 @@ from ..session import GraphSession
 router = APIRouter(prefix="/api/sparql", tags=["Power User Tools"])

 _ALLOWED_QUERY_TYPES = re.compile(
-    r"^\s*(SELECT|ASK|CONSTRUCT|DESCRIBE)\b",
+    r"^(SELECT|ASK|CONSTRUCT|DESCRIBE)\b",
    re.IGNORECASE,
 )

+# SPARQL Update keywords that must never appear in read-only queries.
+# These are checked AFTER comment/prefix stripping to prevent bypass via
+# comments like: # INSERT DATA { ... }\nSELECT ...
+_FORBIDDEN_KEYWORDS = re.compile(
+    r"\b(INSERT|DELETE|DROP|LOAD|CLEAR|CREATE|COPY|MOVE|ADD)\b",
+    re.IGNORECASE,
+)
+
+# Matches SPARQL single-line comments (# ...) and PREFIX/BASE declarations.
+# The comment regex only treats '#' as a comment-starter at line-start or
+# after whitespace — not mid-token — since RDF namespace IRIs commonly
+# contain a literal '#' (e.g. ".../1999/02/22-rdf-syntax-ns#"), and a naive
+# `#[^\n]*` would truncate every such PREFIX declaration's IRI, corrupting
+# the query. BASE declarations have no prefix name between the keyword and
+# the IRI (`BASE <...>`, vs. `PREFIX ex: <...>`), so the prefix-name token
+# is optional.
+_COMMENT_LINE = re.compile(r"(?:^|(?<=\s))#[^\n]*", re.MULTILINE)
+_PREFIX_DECL = re.compile(r"^\s*(?:PREFIX\s+\S+|BASE)\s*<[^>]*>\s*", re.IGNORECASE | re.MULTILINE)
+

 def _is_read_only_query(query: str) -> bool:
-    """Return True only for SELECT / ASK / CONSTRUCT / DESCRIBE queries."""
-    return bool(_ALLOWED_QUERY_TYPES.match(query))
+    """Return True only for genuine read-only SPARQL queries.
+
+    Strips comments, PREFIX/BASE declarations, and leading whitespace before
+    checking the first keyword. Also rejects queries containing SPARQL Update
+    keywords anywhere in the body, preventing injection via embedded strings
+    or multi-statement tricks.
+    """
+    # 1. Remove single-line comments that could hide the real query type
+    cleaned = _COMMENT_LINE.sub("", query)
+    # 2. Remove PREFIX/BASE declarations
+    cleaned = _PREFIX_DECL.sub("", cleaned)
+    # 3. Strip remaining whitespace
+    cleaned = cleaned.strip()
+
+    # 4. Check that the first keyword is a read-only query type
+    if not _ALLOWED_QUERY_TYPES.match(cleaned):
+        return False
+
+    # 5. Block any forbidden (mutating) keywords anywhere in the query
+    if _FORBIDDEN_KEYWORDS.search(cleaned):
+        return False
+
+    return True


 class SparqlRequest(BaseModel):
@@ -59,8 +104,27 @@ def _build_rdflib_graph(session: GraphSession) -> rdflib.Graph:
    graph.bind("ent", NS)
    graph.bind("prop", PROP)

-    nodes, _ = session.get_nodes(skip=0, limit=999_999)
-    edges, _ = session.get_edges(skip=0, limit=999_999)
+    # SECURITY: Cap the number of entities materialized into memory to
+    # prevent denial-of-service via memory exhaustion.  Without this guard
+    # an attacker can send concurrent SPARQL queries that each load ~1M
+    # nodes/edges into rdflib Graph objects, consuming gigabytes of RAM.
+    nodes, total_nodes = session.get_nodes(skip=0, limit=_SPARQL_MAX_GRAPH_NODES + 1)
+    if len(nodes) > _SPARQL_MAX_GRAPH_NODES:
+        raise ValueError(
+            f"Graph has more than {_SPARQL_MAX_GRAPH_NODES:,} nodes. "
+            f"SPARQL queries are limited to graphs with at most "
+            f"{_SPARQL_MAX_GRAPH_NODES:,} nodes to prevent excessive "
+            f"memory usage. Use the REST API for large graph operations."
+        )
+
+    edges, _ = session.get_edges(skip=0, limit=_SPARQL_MAX_GRAPH_NODES + 1)
+    if len(edges) > _SPARQL_MAX_GRAPH_NODES:
+        raise ValueError(
+            f"Graph has more than {_SPARQL_MAX_GRAPH_NODES:,} edges. "
+            f"SPARQL queries are limited to graphs with at most "
+            f"{_SPARQL_MAX_GRAPH_NODES:,} edges to prevent excessive "
+            f"memory usage. Use the REST API for large graph operations."
+        )

    for node in nodes:
        subject = NS[str(node.get("id", ""))]
@@ -91,6 +155,7 @@ def _build_rdflib_graph(session: GraphSession) -> rdflib.Graph:
 _SPARQL_MAX_ROWS = 5_000     # hard cap on returned rows
 _SPARQL_TIMEOUT_S = 30       # seconds before abandoning the await
 _SPARQL_MAX_CONCURRENT = 4   # semaphore: max simultaneous executions
+_SPARQL_MAX_GRAPH_NODES = 50_000  # cap on graph nodes/edges to prevent OOM

 # Semaphore caps how many graph.query calls run concurrently so that
 # timed-out threads (which keep running in the pool) cannot crowd out
@@ -127,7 +192,15 @@ async def execute_sparql(
            error="Only SELECT, ASK, CONSTRUCT, and DESCRIBE queries are permitted.",
        )

-    graph = await asyncio.to_thread(_build_rdflib_graph, session)
+    try:
+        graph = await asyncio.to_thread(_build_rdflib_graph, session)
+    except ValueError as exc:
+        return SparqlResponse(
+            columns=[],
+            rows=[],
+            total=0,
+            error=str(exc),
+        )

    async with _sparql_semaphore:
        try:
@@ -14,21 +14,23 @@ _HAS_DEFUSEDXML = importlib.util.find_spec("defusedxml") is not None


 def _safe_parse_rdf(g: rdflib.Graph, data: bytes, rdf_format: str) -> None:
-    """Parse RDF bytes into *g*, guarding against XXE for XML-based formats."""
+    """Parse RDF bytes into *g*, guarding against XXE for XML-based formats.
+
+    Raises:
+        ImportError: If ``defusedxml`` is not installed and the format is XML-based.
+    """
    xml_formats = {"xml", "rdf", "rdf/xml", "application/rdf+xml"}
    if rdf_format.lower() in xml_formats:
-        if _HAS_DEFUSEDXML:
-            # defusedxml patches xml.etree so rdflib's XML parser inherits the fix
-            import defusedxml
-            defusedxml.defuse_stdlib()
-        else:
-            # Warn once; best-effort protection via rdflib's own parser
-            import warnings
-            warnings.warn(
-                "defusedxml is not installed. Install it (`pip install defusedxml`) "
-                "to protect RDF/XML parsing against XXE attacks.",
-                stacklevel=4,
+        if not _HAS_DEFUSEDXML:
+            # Fail closed: refuse to parse untrusted XML without XXE protection.
+            raise ImportError(
+                "defusedxml is required to safely parse RDF/XML content but is "
+                "not installed. Install it with: pip install defusedxml  "
+                "(or install semantica with the explorer extra: "
+                "pip install 'semantica[explorer]')"
            )
+        import defusedxml
+        defusedxml.defuse_stdlib()
    g.parse(data=data, format=rdf_format)

 def _get_best_label(graph: rdflib.Graph, subject: rdflib.URIRef, predicate: rdflib.URIRef) -> str:
@@ -338,6 +338,13 @@ class ApacheAgeStore:
            "host=localhost dbname=agedb user=postgres password=postgres",
        )
        self.graph_name = graph_name or config.get("graph_name", "semantica")
+        # SECURITY: Sanitize graph_name to prevent SQL injection in cypher() calls.
+        # The graph_name is interpolated into SQL: cypher('{graph_name}', $$ ... $$)
+        if not re.match(r"^[A-Za-z_][A-Za-z0-9_]*$", self.graph_name):
+            raise ValidationError(
+                f"Invalid graph_name '{self.graph_name}': must contain only "
+                "alphanumeric characters and underscores."
+            )

        self._conn = None

@@ -445,7 +452,20 @@ class ApacheAgeStore:

        Returns:
            List of raw row tuples from the cursor.
+
+        Raises:
+            ValidationError: If the query contains ``$$`` which could break
+                out of the AGE dollar-quoted string delimiter.
        """
+        # SECURITY: Reject queries containing $$ to prevent breakout from
+        # AGE's dollar-quoted string delimiter. An attacker who injects $$
+        # into the Cypher query can terminate the cypher() argument and
+        # append arbitrary SQL.
+        if "$$" in cypher:
+            raise ValidationError(
+                "Query contains forbidden '$$' sequence. "
+                "Dollar-quoted delimiters are not allowed in Cypher queries."
+            )
        self._ensure_connection()
        sql = (
            f"SELECT * FROM cypher('{self.graph_name}', $$ {cypher} $$) "
@@ -573,8 +573,8 @@ class VectorStore:
        Args:
            path: Directory path to save to
        """
+        import json
        import os
-        import pickle
        
        os.makedirs(path, exist_ok=True)

@@ -586,17 +586,20 @@ class VectorStore:
        elif self._backend_store is not None and hasattr(self._backend_store, "save_index"):
            self._backend_store.save_index(os.path.join(path, "index.bin"))

-        # Save Python-level data
+        # Save Python-level data using JSON (safe serialization).
+        # pickle is intentionally avoided to prevent arbitrary code execution
+        # if a malicious .pkl file is placed in the store directory.
        data = {
-            "vectors": getattr(self, "vectors", {}),
+            "vectors": {k: v.tolist() if hasattr(v, "tolist") else list(v)
+                    for k, v in getattr(self, "vectors", {}).items()},
            "metadata": getattr(self, "metadata", {}),
            "config": self.config,
            "backend": self.backend,
            "dimension": self.dimension
        }
        
-        with open(os.path.join(path, "store_data.pkl"), "wb") as f:
-            pickle.dump(data, f)
+        with open(os.path.join(path, "store_data.json"), "w", encoding="utf-8") as f:
+            json.dump(data, f)
            
        self.logger.info(f"Saved vector store to {path}")

@@ -606,17 +609,33 @@ class VectorStore:
        
        Args:
            path: Directory path to load from
+            
+        Raises:
+            RuntimeError: If only a legacy pickle file is found (security risk).
        """
+        import json
        import os
-        import pickle
        
-        data_path = os.path.join(path, "store_data.pkl")
-        if not os.path.exists(data_path):
-            self.logger.warning(f"Store data not found: {data_path}")
+        json_path = os.path.join(path, "store_data.json")
+        legacy_pkl_path = os.path.join(path, "store_data.pkl")
+        
+        if os.path.exists(json_path):
+            data_path = json_path
+        elif os.path.exists(legacy_pkl_path):
+            # Refuse to load pickle files to prevent arbitrary code execution.
+            # A crafted .pkl file can execute arbitrary Python when deserialized.
+            raise RuntimeError(
+                f"Legacy pickle file found at {legacy_pkl_path}. "
+                "Pickle deserialization is disabled for security (arbitrary code "
+                "execution risk). Please re-save the vector store to migrate "
+                "to the safe JSON format: vs.save(path)"
+            )
+        else:
+            self.logger.warning(f"Store data not found in: {path}")
            return
            
-        with open(data_path, "rb") as f:
-            data = pickle.load(f)
+        with open(data_path, "r", encoding="utf-8") as f:
+            data = json.load(f)
            
        self.vectors = data.get("vectors", {})
        self.metadata = data.get("metadata", {})
@@ -0,0 +1,93 @@
+"""Regression tests for outbound URL fetching in ontology.py (SSRF hardening).
+
+`_fetch_url_sync` disables `requests`' automatic redirect following and
+re-validates every hop with `_validate_fetch_url` (see GHSA-8c7v-62gr-hj6g:
+unvalidated redirect targets previously let a public first hop 302 the
+server into fetching cloud metadata / loopback services).
+
+These tests cover the redirect-handling logic itself: relative `Location`
+headers must resolve correctly instead of being rejected outright, redirect
+targets that resolve to private/loopback addresses must still be blocked,
+and every response must be closed (no leaked connections across hops).
+"""
+
+import socket
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from semantica.explorer.routes import ontology as ontology_mod
+
+
+def _fake_getaddrinfo(host, *args, **kwargs):
+    # These tests are about the redirect-handling logic, not the address
+    # classifier itself, so every host resolves to a public IP unless a
+    # test overrides the side_effect to simulate an internal target.
+    return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("93.184.216.34", 0))]
+
+
+def _make_response(is_redirect=False, is_permanent=False, location=None, body=b"ok"):
+    resp = MagicMock()
+    resp.is_redirect = is_redirect
+    resp.is_permanent_redirect = is_permanent
+    resp.headers = {"Location": location} if location else {}
+    resp.raise_for_status = MagicMock()
+    resp.iter_content = MagicMock(return_value=iter([body]))
+    resp.close = MagicMock()
+    return resp
+
+
+@patch.object(ontology_mod.socket, "getaddrinfo", side_effect=_fake_getaddrinfo)
+def test_relative_redirect_location_is_resolved(mock_getaddrinfo):
+    """A relative Location header (e.g. '/ontology.ttl') must resolve against
+    the current URL via urljoin, not be rejected as a malformed URL."""
+    redirect_resp = _make_response(is_redirect=True, location="/ontology.ttl")
+    final_resp = _make_response(body=b"final content")
+
+    with patch("requests.get", side_effect=[redirect_resp, final_resp]) as mock_get:
+        result = ontology_mod._fetch_url_sync("http://example.org/start")
+
+    assert result == b"final content"
+    second_call_url = mock_get.call_args_list[1].args[0]
+    assert second_call_url == "http://example.org/ontology.ttl"
+    redirect_resp.close.assert_called_once()
+    final_resp.close.assert_called_once()
+
+
+@patch.object(ontology_mod.socket, "getaddrinfo", side_effect=_fake_getaddrinfo)
+def test_redirect_to_private_ip_is_rejected(mock_getaddrinfo):
+    """Re-validation must reject a redirect target resolving to a private
+    address even though the first hop was a validated public URL — this is
+    the exact GHSA-8c7v scenario: public first hop, malicious redirect."""
+    def getaddrinfo_side_effect(host, *a, **k):
+        if host == "internal.example":
+            return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("169.254.169.254", 0))]
+        return _fake_getaddrinfo(host, *a, **k)
+
+    mock_getaddrinfo.side_effect = getaddrinfo_side_effect
+    redirect_resp = _make_response(is_redirect=True, location="http://internal.example/latest/meta-data/")
+
+    with patch("requests.get", side_effect=[redirect_resp]):
+        with pytest.raises(ontology_mod.HTTPException) as exc_info:
+            ontology_mod._fetch_url_sync("http://example.org/start")
+
+    assert exc_info.value.status_code == 422
+    redirect_resp.close.assert_called_once()
+
+
+@patch.object(ontology_mod.socket, "getaddrinfo", side_effect=_fake_getaddrinfo)
+def test_final_response_is_closed(mock_getaddrinfo):
+    final_resp = _make_response(body=b"content")
+    with patch("requests.get", side_effect=[final_resp]):
+        ontology_mod._fetch_url_sync("http://example.org/start")
+    final_resp.close.assert_called_once()
+
+
+@patch.object(ontology_mod.socket, "getaddrinfo", side_effect=_fake_getaddrinfo)
+def test_redirect_chain_exceeding_cap_is_rejected(mock_getaddrinfo):
+    responses = [_make_response(is_redirect=True, location=f"/hop{i}") for i in range(10)]
+    with patch("requests.get", side_effect=responses):
+        with pytest.raises(ontology_mod.HTTPException) as exc_info:
+            ontology_mod._fetch_url_sync("http://example.org/start")
+    assert exc_info.value.status_code == 502
+    assert all(r.close.called for r in responses[:6])
@@ -199,14 +199,27 @@ def test_allowlist_rejected_query_never_touches_the_graph(client, query):
    mock_build.assert_not_called()


-def test_multi_statement_injection_reaches_graph_but_fails_in_parser(client):
-    """Confirms the distinction between allowlist rejection and parser rejection:
-    a string starting with SELECT passes _is_read_only_query and builds a graph,
-    but rdflib.Graph.query() rejects the trailing '; DROP ALL' syntax."""
+def test_multi_statement_injection_is_rejected_by_forbidden_keyword_check(client):
+    """A string starting with an allowed keyword (SELECT) but containing a
+    forbidden Update keyword later in the body (';  DROP ALL') is now
+    rejected by _is_read_only_query's keyword scan itself, before a graph
+    is ever built — a stronger, earlier rejection than relying solely on
+    rdflib's parser to reject the syntax."""
+    with patch.object(sparql_mod, "_build_rdflib_graph") as mock_build:
+        resp = _post(client, "SELECT ?s WHERE { ?s ?p ?o } ; DROP ALL")
+    assert resp.status_code == 200
+    assert resp.json()["error"] is not None
+    mock_build.assert_not_called()
+
+
+def test_malformed_syntax_without_forbidden_keywords_still_fails_in_parser(client):
+    """The parser remains a real second line of defense for malformed
+    queries that don't contain any forbidden keyword — these pass
+    _is_read_only_query and reach rdflib, which rejects the syntax."""
    with patch.object(
        sparql_mod, "_build_rdflib_graph", wraps=sparql_mod._build_rdflib_graph
    ) as spy_build:
-        resp = _post(client, "SELECT ?s WHERE { ?s ?p ?o } ; DROP ALL")
+        resp = _post(client, "SELECT ?s WHERE { ?s ?p ?o } ; ASK { ?x ?y ?z }")
    assert resp.status_code == 200
    assert resp.json()["error"] is not None
    spy_build.assert_called_once()
@@ -290,6 +303,20 @@ def test_query_timeout_returns_clean_error_not_a_crash(client):
    assert payload["rows"] == []


+def test_oversized_graph_returns_clean_error_not_a_crash(client):
+    """The DoS-prevention node cap (GHSA-8c7v-adjacent hardening) must return
+    a normal SparqlResponse error, not an unhandled 500. Regression test for
+    a bug where _build_rdflib_graph's ValueError was raised outside of
+    execute_sparql's try/except, before the semaphore block."""
+    with patch.object(sparql_mod, "_SPARQL_MAX_GRAPH_NODES", 1):
+        resp = _post(client, "SELECT ?s WHERE { ?s a <http://semantica.local/entity/language> }")
+    assert resp.status_code == 200
+    payload = resp.json()
+    assert payload["error"] is not None
+    assert "more than" in payload["error"].lower()
+    assert payload["rows"] == []
+
+
 # ---------------------------------------------------------------------------
 # Data-mapping fidelity: does the graph->RDF projection reflect session state?
 # ---------------------------------------------------------------------------
@@ -0,0 +1,275 @@
+"""
+Regression tests for security fixes in PR #898.
+
+Covers:
+  1. Cypher Injection Prevention (AGE Store)
+  2. SPARQL Injection Prevention (read-only query validation)
+  3. XXE Protection (rdf_parser fail-closed)
+  4. Vector save numpy serialization
+  5. SPARQL graph cap error handling
+  6. SSRF redirect handling (relative URLs, resp.close)
+
+Explorer API-key authentication (GHSA-j4mq-hprp-987v) has its own, more
+thorough test suite at tests/explorer/test_explorer_auth.py — it isn't
+duplicated here.
+"""
+
+import pytest
+
+# Import the real implementation rather than re-declaring the regexes here:
+# an earlier version of this file inlined a copy that silently drifted from
+# semantica/explorer/routes/sparql.py's actual behavior (the inlined
+# _COMMENT_LINE regex stripped '#' mid-token, corrupting any PREFIX
+# declaration using a namespace IRI with a literal '#', e.g. the standard
+# rdf:/rdfs: namespaces) and neither the code nor this test caught it,
+# since both had the same bug. Importing the real function makes that class
+# of drift impossible.
+from semantica.explorer.routes.sparql import _is_read_only_query
+
+
+class TestSparqlReadOnlyValidation:
+    """Regression tests for SPARQL injection prevention."""
+
+    def test_select_allowed(self):
+        assert _is_read_only_query("SELECT ?s ?p ?o WHERE { ?s ?p ?o }")
+
+    def test_ask_allowed(self):
+        assert _is_read_only_query("ASK { ?s ?p ?o }")
+
+    def test_construct_allowed(self):
+        assert _is_read_only_query("CONSTRUCT { ?s ?p ?o } WHERE { ?s ?p ?o }")
+
+    def test_describe_allowed(self):
+        assert _is_read_only_query("DESCRIBE <http://example.org>")
+
+    def test_insert_blocked(self):
+        assert not _is_read_only_query("INSERT DATA { <s> <p> <o> }")
+
+    def test_delete_blocked(self):
+        assert not _is_read_only_query("DELETE WHERE { ?s ?p ?o }")
+
+    def test_drop_blocked(self):
+        assert not _is_read_only_query("DROP GRAPH <http://example.org>")
+
+    def test_comment_bypass_blocked(self):
+        """Attacker hides INSERT behind a comment, SELECT follows."""
+        query = "# innocent comment\nINSERT DATA { <s> <p> <o> }"
+        assert not _is_read_only_query(query)
+
+    def test_comment_hiding_real_query(self):
+        """Comment at top with SELECT visible, but INSERT in body."""
+        query = "# SELECT everything\nINSERT DATA { <s> <p> <o> }"
+        assert not _is_read_only_query(query)
+
+    def test_prefix_before_select_allowed(self):
+        """PREFIX declarations before SELECT should still be allowed."""
+        query = "PREFIX ex: <http://example.org/>\nSELECT ?s WHERE { ?s ex:p ?o }"
+        assert _is_read_only_query(query)
+
+    def test_prefix_before_insert_blocked(self):
+        """PREFIX declarations can't disguise an INSERT."""
+        query = "PREFIX ex: <http://example.org/>\nINSERT DATA { ex:s ex:p ex:o }"
+        assert not _is_read_only_query(query)
+
+    def test_multiple_prefixes_then_select(self):
+        query = (
+            "PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>\n"
+            "PREFIX rdfs: <http://www.w3.org/2000/01/rdf-schema#>\n"
+            "SELECT ?s ?label WHERE { ?s rdfs:label ?label }"
+        )
+        assert _is_read_only_query(query)
+
+    def test_select_with_insert_keyword_blocked(self):
+        """Even if SELECT is first, INSERT in body should be blocked."""
+        query = "SELECT ?s WHERE { ?s ?p ?o } ; INSERT DATA { <a> <b> <c> }"
+        assert not _is_read_only_query(query)
+
+    def test_case_insensitive_insert(self):
+        assert not _is_read_only_query("insert data { <s> <p> <o> }")
+
+    def test_load_blocked(self):
+        assert not _is_read_only_query("LOAD <http://evil.com/data.ttl>")
+
+    def test_clear_blocked(self):
+        assert not _is_read_only_query("CLEAR ALL")
+
+    def test_empty_query_rejected(self):
+        assert not _is_read_only_query("")
+
+    def test_whitespace_only_rejected(self):
+        assert not _is_read_only_query("   \n\t  ")
+
+    def test_base_before_select(self):
+        query = "BASE <http://example.org/>\nSELECT ?s WHERE { ?s ?p ?o }"
+        assert _is_read_only_query(query)
+
+    def test_namespace_iri_with_hash_fragment_not_treated_as_comment(self):
+        """A '#' inside a PREFIX declaration's IRI (standard for RDF/RDFS/OWL
+        namespaces) must not be mistaken for a comment-start — a naive
+        `#[^\\n]*` strip corrupts the IRI and truncates the rest of the
+        query with it."""
+        query = (
+            "PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>\n"
+            "SELECT ?s WHERE { ?s rdf:type ?o }"
+        )
+        assert _is_read_only_query(query)
+
+    def test_real_comment_after_namespace_iri_still_stripped(self):
+        """A genuine trailing comment must still be recognized even on a
+        line that also contains a '#'-bearing IRI earlier in the query."""
+        query = (
+            "PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>\n"
+            "SELECT ?s WHERE { ?s rdf:type ?o } # trailing comment INSERT DATA"
+        )
+        assert _is_read_only_query(query)
+
+
+# ===================================================================
+# 2. Cypher injection prevention
+# ===================================================================
+
+class TestCypherInjection:
+    """Regression tests for Cypher/SQL injection prevention."""
+
+    def test_sanitize_label_valid(self):
+        from semantica.graph_store.age_store import _sanitize_label
+        assert _sanitize_label("Entity") == "Entity"
+        assert _sanitize_label("my_label_123") == "my_label_123"
+
+    def test_sanitize_label_injection(self):
+        from semantica.graph_store.age_store import _sanitize_label
+        with pytest.raises(Exception):  # ValidationError
+            _sanitize_label("Entity') OR 1=1--")
+
+    def test_sanitize_label_special_chars(self):
+        from semantica.graph_store.age_store import _sanitize_label
+        with pytest.raises(Exception):
+            _sanitize_label("Entity;DROP TABLE")
+
+    def test_value_to_cypher_literal_string_escaping(self):
+        from semantica.graph_store.age_store import _value_to_cypher_literal
+        result = _value_to_cypher_literal("O'Brien")
+        assert "\\'" in result  # Single quote should be escaped
+
+    def test_value_to_cypher_literal_backslash(self):
+        from semantica.graph_store.age_store import _value_to_cypher_literal
+        result = _value_to_cypher_literal("path\\to\\file")
+        assert "\\\\" in result
+
+    def test_dollar_dollar_breakout_blocked(self):
+        """$$ in a Cypher query would break out of AGE's delimiter."""
+        from semantica.graph_store.age_store import ApacheAgeStore
+        store = ApacheAgeStore.__new__(ApacheAgeStore)
+        store.graph_name = "test_graph"
+        store._conn = None
+        with pytest.raises(Exception):  # ValidationError
+            store._execute_cypher("MATCH (n) RETURN n $$ ) AS (x agtype); DROP TABLE users; --")
+
+    def test_graph_name_sanitization(self):
+        """Graph name with SQL injection should be rejected."""
+        from semantica.graph_store.age_store import ApacheAgeStore
+        with pytest.raises(Exception):  # ValidationError
+            ApacheAgeStore(
+                connection_string="host=localhost",
+                graph_name="test'); DROP TABLE--"
+            )
+
+    def test_graph_name_valid(self):
+        from semantica.graph_store.age_store import ApacheAgeStore
+        store = ApacheAgeStore(
+            connection_string="host=localhost",
+            graph_name="my_graph_123"
+        )
+        assert store.graph_name == "my_graph_123"
+
+    def test_property_key_injection(self):
+        from semantica.graph_store.age_store import _props_to_cypher_literal
+        with pytest.raises(Exception):
+            _props_to_cypher_literal({"key; DROP": "value"})
+
+
+# ===================================================================
+# 3. XXE Protection (fail-closed)
+# ===================================================================
+
+class TestXXEProtection:
+    """Regression tests for XXE prevention in rdf_parser."""
+
+    def test_defusedxml_check_exists(self):
+        """The _HAS_DEFUSEDXML flag must exist."""
+        from semantica.explorer.utils.rdf_parser import _HAS_DEFUSEDXML
+        assert isinstance(_HAS_DEFUSEDXML, bool)
+
+    def test_safe_parse_rdf_function_exists(self):
+        """_safe_parse_rdf must be importable."""
+        from semantica.explorer.utils.rdf_parser import _safe_parse_rdf
+        assert callable(_safe_parse_rdf)
+
+
+# ===================================================================
+# 4. Numpy vector serialization
+# ===================================================================
+
+class TestVectorSerialization:
+    """Regression test for numpy array serialization in vector_store."""
+
+    def test_tolist_on_numpy_like(self):
+        """Objects with tolist() should use it instead of list()."""
+
+        class FakeNumpyArray:
+            def __init__(self, data):
+                self._data = data
+
+            def tolist(self):
+                return self._data
+
+            def __iter__(self):
+                # list() would call this and fail for multi-dim arrays
+                raise TypeError("Use tolist() for numpy arrays")
+
+        arr = FakeNumpyArray([1.0, 2.0, 3.0])
+        # Simulate the fixed logic
+        result = arr.tolist() if hasattr(arr, "tolist") else list(arr)
+        assert result == [1.0, 2.0, 3.0]
+
+    def test_regular_list_still_works(self):
+        """Regular lists (no tolist) should use list()."""
+        data = [1.0, 2.0, 3.0]
+        result = data.tolist() if hasattr(data, "tolist") else list(data)
+        assert result == [1.0, 2.0, 3.0]
+
+
+# ===================================================================
+# 5. SSRF redirect handling
+# ===================================================================
+
+class TestSSRFRedirectHandling:
+    """Regression tests for SSRF redirect fixes."""
+
+    def test_urljoin_resolves_relative(self):
+        """Relative Location headers must be resolved against current URL."""
+        from urllib.parse import urljoin
+        base = "https://example.com/api/ontology"
+        relative = "/ontology.ttl"
+        result = urljoin(base, relative)
+        assert result == "https://example.com/ontology.ttl"
+
+    def test_urljoin_absolute_passthrough(self):
+        """Absolute Location headers should pass through unchanged."""
+        from urllib.parse import urljoin
+        base = "https://example.com/api/ontology"
+        absolute = "https://other.com/data.ttl"
+        result = urljoin(base, absolute)
+        assert result == "https://other.com/data.ttl"
+
+    def test_urljoin_relative_path(self):
+        """Relative path without leading slash."""
+        from urllib.parse import urljoin
+        base = "https://example.com/api/v1/resource"
+        relative = "../data.ttl"
+        result = urljoin(base, relative)
+        assert result == "https://example.com/api/data.ttl"
+
+
+if __name__ == "__main__":
+    pytest.main([__file__, "-v"])
@@ -1,3 +1,6 @@
+import json
+import shutil
+import tempfile
 import unittest
 from unittest.mock import MagicMock, patch
 import numpy as np
@@ -134,5 +137,48 @@ class TestVectorStore(unittest.TestCase):
        
        self.assertTrue(mock_backend.called)

+    def test_save_load_roundtrip_numpy_vectors(self):
+        """save()/load() must handle numpy float32 vectors without raising.
+
+        Regression test: json.dump() rejects numpy scalar types, so a naive
+        `list(v)` conversion (which yields np.float32 elements, not native
+        floats) raises TypeError. `v.tolist()` converts recursively to
+        native Python floats and must be used instead.
+        """
+        store = VectorStore(backend="inmemory", dimension=3)
+        store.vectors = {"v1": np.array([0.1, 0.2, 0.3], dtype=np.float32)}
+        store.metadata = {"v1": {"id": "1"}}
+
+        tmpdir = tempfile.mkdtemp()
+        try:
+            store.save(tmpdir)  # must not raise TypeError
+
+            # The JSON file itself must be valid and free of numpy types.
+            with open(f"{tmpdir}/store_data.json", "r", encoding="utf-8") as f:
+                data = json.load(f)
+            self.assertTrue(all(isinstance(x, float) for x in data["vectors"]["v1"]))
+
+            loaded = VectorStore(backend="inmemory", dimension=3)
+            loaded.load(tmpdir)
+            np.testing.assert_allclose(
+                loaded.vectors["v1"], [0.1, 0.2, 0.3], rtol=1e-6
+            )
+            self.assertEqual(loaded.metadata["v1"], {"id": "1"})
+        finally:
+            shutil.rmtree(tmpdir, ignore_errors=True)
+
+    def test_load_rejects_legacy_pickle(self):
+        """load() must refuse legacy .pkl stores rather than deserializing them."""
+        store = VectorStore(backend="inmemory", dimension=3)
+        tmpdir = tempfile.mkdtemp()
+        try:
+            with open(f"{tmpdir}/store_data.pkl", "wb") as f:
+                f.write(b"not a real pickle, just needs to exist")
+            with self.assertRaises(RuntimeError):
+                store.load(tmpdir)
+        finally:
+            shutil.rmtree(tmpdir, ignore_errors=True)
+
+
 if __name__ == '__main__':
    unittest.main()