fix(security): restore GHSA-j4mq auth enforcement, fix SPARQL comment-regex bug

Two issues in the last round of commits:

1. explorer/auth.py added a new, opt-in APIKeyAuthMiddleware
   (EXPLORER_API_KEY) and wired it into create_app(), but in doing so
   removed the Depends(require_auth) dependency from every router and
   deleted the /ws/graph-updates handshake check entirely. The new
   middleware also fails OPEN (allows all requests) when its key is
   unset, the opposite of require_auth's fail-closed design. Since
   GHSA-j4mq-hprp-987v (the unauthenticated-Explorer-API advisory) is
   already merged into main via require_auth, this would have reverted
   a merged Critical fix the moment this branch merges. Removed
   explorer/auth.py, restored the per-router dependencies and the
   WebSocket auth check. Kept auth.py's one genuine improvement (adding
   X-API-Key to the CORS allow_headers list) by folding it into the
   existing CORS middleware config.

2. sparql.py's new _is_read_only_query() hardening (comment/PREFIX
   stripping + forbidden-keyword scan) used `#[^\n]*` to strip SPARQL
   comments, but a bare '#' also appears inside standard RDF namespace
   IRIs (e.g. ".../1999/02/22-rdf-syntax-ns#") — the regex struck
   everything after that '#' as a "comment", corrupting the query and
   rejecting any legitimate SELECT using rdf:/rdfs:-style PREFIX
   declarations. Confirmed by the fact the new hardening's own inlined
   test copy failed against two of its own cases. Fixed by only
   treating '#' as a comment-start at line-start or after whitespace,
   which distinguishes ".../ns#" (preceded by a word character) from an
   actual comment (preceded by whitespace/newline in every realistic
   case, including the attacker's own comment-hiding PoC). Also fixed
   the companion PREFIX/BASE regex, which required a prefix-name token
   between the keyword and the IRI even for bare `BASE <...>`
   declarations (which have none).

tests/test_security_regression.py's SPARQL section now imports the real
_is_read_only_query instead of maintaining a parallel inlined copy that
had silently drifted from — and shared the same bug as — the real
implementation; removed its TestAPIKeyAuth class (tested the now-deleted
auth.py) since equivalent, more thorough coverage already exists in
tests/explorer/test_explorer_auth.py. Updated tests/explorer/test_sparql_route.py's
multi-statement-injection test to reflect that the keyword scan now
catches "SELECT ... ; DROP ALL" itself rather than relying on rdflib's
parser, and added a new test confirming the parser still catches
multi-statement syntax that doesn't contain any forbidden keyword.

Full explorer/vector_store/security-regression/age_store suite: 543
passed (the only failures are 6 pre-existing, unrelated Pinecone-client
mocking issues).
This commit is contained in:
KaifAhmad1
2026-08-11 15:36:49 +05:30
parent 44f585ffce
commit abc10bc8e0
5 changed files with 119 additions and 207 deletions
+42 -21
View File
@@ -8,16 +8,16 @@ from contextlib import asynccontextmanager
from pathlib import Path
from typing import Optional
from fastapi import FastAPI, HTTPException, Request, WebSocket, WebSocketDisconnect
from fastapi import Depends, FastAPI, HTTPException, Request, WebSocket, WebSocketDisconnect
from fastapi.middleware.cors import CORSMiddleware
from fastapi.responses import FileResponse, HTMLResponse, JSONResponse
from fastapi.staticfiles import StaticFiles
from .. import __version__
from ..context.context_graph import ContextGraph
from .dependencies import anonymous_access_allowed, get_expected_api_key, is_valid_api_key, require_auth
from .session import GraphSession
from .ws import ConnectionManager
from .auth import APIKeyAuthMiddleware, warn_if_unauthenticated
def _read_int_env(name: str, default: int) -> int:
@@ -98,6 +98,22 @@ def create_app(
@asynccontextmanager
async def lifespan(app: FastAPI):
import logging as _lifespan_logging
_lifespan_logger = _lifespan_logging.getLogger(__name__)
if anonymous_access_allowed():
_lifespan_logger.warning(
"Explorer is running with SEMANTICA_ALLOW_ANONYMOUS=true — "
"all API routes are unauthenticated. Do not expose this "
"process beyond localhost."
)
elif get_expected_api_key():
_lifespan_logger.info("Explorer API authentication: enabled (SEMANTICA_API_KEY set).")
else:
_lifespan_logger.warning(
"Explorer API authentication: NOT CONFIGURED. All protected "
"routes will return 503 until SEMANTICA_API_KEY is set."
)
app.state.event_loop = asyncio.get_running_loop()
app.state.ws_manager = ConnectionManager()
app.state.session = active_session
@@ -114,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,
@@ -128,10 +144,6 @@ def create_app(
max_age=600,
)
# API key authentication (opt-in via EXPLORER_API_KEY env var)
app.add_middleware(APIKeyAuthMiddleware)
warn_if_unauthenticated()
import logging as _logging
_logger = _logging.getLogger(__name__)
@@ -164,22 +176,31 @@ def create_app(
from .routes.temporal import router as temporal_router
from .routes.vocabulary import router as vocabulary_router
app.include_router(graph_router)
app.include_router(analytics_router)
app.include_router(decisions_router)
app.include_router(temporal_router)
app.include_router(enrich_router)
app.include_router(export_import_router)
app.include_router(annotations_router)
app.include_router(sparql_router)
app.include_router(provenance_router)
app.include_router(vocabulary_router)
app.include_router(ontology_router)
_auth = [Depends(require_auth)]
app.include_router(graph_router, dependencies=_auth)
app.include_router(analytics_router, dependencies=_auth)
app.include_router(decisions_router, dependencies=_auth)
app.include_router(temporal_router, dependencies=_auth)
app.include_router(enrich_router, dependencies=_auth)
app.include_router(export_import_router, dependencies=_auth)
app.include_router(annotations_router, dependencies=_auth)
app.include_router(sparql_router, dependencies=_auth)
app.include_router(provenance_router, dependencies=_auth)
app.include_router(vocabulary_router, dependencies=_auth)
app.include_router(ontology_router, dependencies=_auth)
_WS_MAX_MESSAGE_BYTES = 64 * 1024 # 64 KB — control messages only
@app.websocket("/ws/graph-updates")
async def websocket_endpoint(websocket: WebSocket):
# Browsers can't set custom headers on a WebSocket handshake, so
# accept the key via header (non-browser clients) or query param
# (browser clients), same SEMANTICA_API_KEY the REST routes check.
candidate = websocket.headers.get("x-api-key") or websocket.query_params.get("api_key")
if not is_valid_api_key(candidate):
await websocket.close(code=4401) # unauthorized
return
manager: ConnectionManager = app.state.ws_manager
await manager.connect(websocket)
await manager.send_personal(websocket, "connection_ack", {"connected": True})
-102
View File
@@ -1,102 +0,0 @@
"""
Semantica Explorer : Authentication Middleware
Provides opt-in API key authentication for all Explorer API routes.
Enable by setting the ``EXPLORER_API_KEY`` environment variable. When set,
every request to ``/api/*`` must include either:
- An ``Authorization: Bearer <key>`` header, or
- An ``X-API-Key: <key>`` header.
When ``EXPLORER_API_KEY`` is not set, authentication is disabled and the
Explorer operates in open/development mode (with a startup warning).
"""
import hmac
import logging
import os
from typing import Optional
from fastapi import HTTPException, Request, status
from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
from starlette.responses import Response
_logger = logging.getLogger(__name__)
def _get_api_key() -> Optional[str]:
"""Read the configured API key from the environment."""
return os.environ.get("EXPLORER_API_KEY")
def _extract_token(request: Request) -> Optional[str]:
"""Extract the API key from the request headers."""
# Check Authorization: Bearer <key>
auth_header = request.headers.get("Authorization", "")
if auth_header.startswith("Bearer "):
return auth_header[7:].strip()
# Check X-API-Key: <key>
api_key_header = request.headers.get("X-API-Key", "")
if api_key_header:
return api_key_header.strip()
return None
class APIKeyAuthMiddleware(BaseHTTPMiddleware):
"""
Middleware that enforces API key authentication on ``/api/*`` routes.
Skips authentication for:
- Non-API routes (static files, health checks, WebSocket, docs)
- OPTIONS requests (CORS preflight)
- When ``EXPLORER_API_KEY`` is not configured (open mode)
"""
async def dispatch(
self, request: Request, call_next: RequestResponseEndpoint
) -> Response:
api_key = _get_api_key()
# If no API key is configured, allow all requests (open mode)
if not api_key:
return await call_next(request)
# Skip authentication for non-API paths
path = request.url.path
if not path.startswith("/api/"):
return await call_next(request)
# Skip CORS preflight
if request.method == "OPTIONS":
return await call_next(request)
# Validate the token
token = _extract_token(request)
if not token:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Missing API key. Provide via 'Authorization: Bearer <key>' or 'X-API-Key: <key>' header.",
headers={"WWW-Authenticate": "Bearer"},
)
# Constant-time comparison to prevent timing attacks
if not hmac.compare_digest(token, api_key):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid API key.",
)
return await call_next(request)
def warn_if_unauthenticated() -> None:
"""Log a warning at startup if no API key is configured."""
if not _get_api_key():
_logger.warning(
"EXPLORER_API_KEY is not set. The Explorer API is running WITHOUT "
"authentication. Set EXPLORER_API_KEY to enable API key protection "
"for all /api/* endpoints."
)
+20 -8
View File
@@ -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.
"""
@@ -38,9 +43,16 @@ _FORBIDDEN_KEYWORDS = re.compile(
re.IGNORECASE,
)
# Matches SPARQL single-line comments (# ...) and PREFIX declarations
_COMMENT_LINE = re.compile(r"#[^\n]*", re.MULTILINE)
_PREFIX_DECL = re.compile(r"^\s*(?:PREFIX|BASE)\s+\S+\s*<[^>]*>\s*", re.IGNORECASE | re.MULTILINE)
# 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:
+18 -5
View File
@@ -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()
+39 -71
View File
@@ -2,48 +2,29 @@
Regression tests for security fixes in PR #898.
Covers:
1. API Key Authentication (Explorer)
2. Cypher Injection Prevention (AGE Store)
3. SPARQL Injection Prevention (read-only query validation)
4. XXE Protection (rdf_parser fail-closed)
5. Vector save numpy serialization
6. SPARQL graph cap error handling
7. SSRF redirect handling (relative URLs, resp.close)
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 re
import pytest
# ===================================================================
# 1. SPARQL read-only query validation (injection prevention)
# ===================================================================
# Inline the validation logic so tests don't require full app context
_ALLOWED_QUERY_TYPES = re.compile(
r"^(SELECT|ASK|CONSTRUCT|DESCRIBE)\b",
re.IGNORECASE,
)
_FORBIDDEN_KEYWORDS = re.compile(
r"\b(INSERT|DELETE|DROP|LOAD|CLEAR|CREATE|COPY|MOVE|ADD)\b",
re.IGNORECASE,
)
_COMMENT_LINE = re.compile(r"#[^\n]*", re.MULTILINE)
_PREFIX_DECL = re.compile(
r"^\s*(?:PREFIX|BASE)\s+\S+\s*<[^>]*>\s*",
re.IGNORECASE | re.MULTILINE,
)
def _is_read_only_query(query: str) -> bool:
cleaned = _COMMENT_LINE.sub("", query)
cleaned = _PREFIX_DECL.sub("", cleaned)
cleaned = cleaned.strip()
if not _ALLOWED_QUERY_TYPES.match(cleaned):
return False
if _FORBIDDEN_KEYWORDS.search(cleaned):
return False
return True
# 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:
@@ -122,6 +103,26 @@ class TestSparqlReadOnlyValidation:
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
@@ -270,38 +271,5 @@ class TestSSRFRedirectHandling:
assert result == "https://example.com/api/data.ttl"
# ===================================================================
# 6. API Key Auth
# ===================================================================
class TestAPIKeyAuth:
"""Regression tests for API key authentication."""
def test_auth_module_importable(self):
from semantica.explorer.auth import APIKeyAuthMiddleware
assert APIKeyAuthMiddleware is not None
def test_extract_bearer_token(self):
from semantica.explorer.auth import _extract_token
from unittest.mock import MagicMock
req = MagicMock()
req.headers = {"Authorization": "Bearer test-key-123"}
assert _extract_token(req) == "test-key-123"
def test_extract_api_key_header(self):
from semantica.explorer.auth import _extract_token
from unittest.mock import MagicMock
req = MagicMock()
req.headers = {"X-API-Key": "my-secret-key", "Authorization": ""}
assert _extract_token(req) == "my-secret-key"
def test_extract_no_token(self):
from semantica.explorer.auth import _extract_token
from unittest.mock import MagicMock
req = MagicMock()
req.headers = {}
assert _extract_token(req) is None
if __name__ == "__main__":
pytest.main([__file__, "-v"])