fix: address qodo review findings on _PREFIX_DECL and query-length guard

Two follow-up fixes to the initial ReDoS patch (CodeQL py/polynomial-redos
#1897), raised during code review:

--- Fix 1: _PREFIX_DECL regression — inline prologues and CRLF (#review-1) ---

The first ReDoS fix replaced the ambiguous trailing \s* with [ \t]*(?:\n|$),
but that introduced a behavioral regression:

  * Inline prologues — PREFIX ex: <...> SELECT ... on a single line were no
    longer stripped because the mandatory (?:\n|$) anchor never matched when
    non-whitespace content followed the IRI on the same line.
  * CRLF line endings — PREFIX ex: <...>\r\n failed because \r is not in
    [ \t]* and the anchor expected a bare \n.

Root cause: the end-of-line anchor was unnecessary; the only thing needed
to eliminate backtracking ambiguity is ensuring the IRI body character class
and the trailing whitespace quantifier are disjoint.

Fix: change the IRI body from <[^>]*> to <[^>\r\n]*>, which:
  - excludes CR and LF from the IRI match (semantically correct — SPARQL
    IRIs cannot span line boundaries)
  - makes [^>\r\n]* and the trailing [ \t]* have zero character overlap,
    eliminating all backtracking ambiguity without any end-of-line anchor

No anchor is used, so both inline prologues and CRLF/LF endings work
naturally. ReDoS payloads (base< + !< x 10,000) still complete in <1 ms.

--- Fix 2: oversized-query length guard obscured error (#review-2) ---

The initial patch placed the _SPARQL_MAX_QUERY_LEN guard inside
_is_read_only_query(), which caused execute_sparql() to return the same
generic 'Only SELECT' error for both genuinely disallowed query types and
oversized inputs. Clients could not distinguish the two rejection reasons.

Fix: move the length check out of _is_read_only_query() and into
execute_sparql() as an explicit early gate, alongside the other resource
limits (_SPARQL_MAX_ROWS, _SPARQL_MAX_GRAPH_NODES). Oversized queries now
return a specific message naming the limit, the received length, and the
remediation step. _is_read_only_query() is documented to be length-agnostic.
_SPARQL_MAX_QUERY_LEN is relocated to the resource-limits block with the
other constants.

--- Tests added ---

tests/test_security_regression.py:
  - test_inline_prefix_before_select_allowed   (Fix 1 regression)
  - test_crlf_line_endings_with_prefix         (Fix 1 regression)
  - test_crlf_multiple_prefixes_then_select    (Fix 1 regression)
  - test_inline_prefix_before_insert_still_blocked (Fix 1 security check)
  - test_long_valid_query_not_rejected_by_is_read_only (Fix 2 separation)

tests/explorer/test_sparql_route.py:
  - test_oversized_query_returns_distinct_length_error (Fix 2 error message)
  - test_oversized_query_never_touches_the_graph       (Fix 2 short-circuit)
  - test_query_exactly_at_length_limit_is_accepted     (Fix 2 boundary)

All 82 tests pass.

semantica/explorer/routes/sparql.py

@@ -52,23 +52,27 @@ _FORBIDDEN_KEYWORDS = re.compile(
 # the IRI (`BASE <...>`, vs. `PREFIX ex: <...>`), so the prefix-name token
 # is optional.
 #
-# ReDoS note: the original pattern ended with `<[^>]*>\s*` where the
-# trailing `\s*` could overlap with the `[^>]*` character class on inputs
-# that contain no closing `>`, causing polynomial backtracking (CodeQL
-# py/polynomial-redos, issue #1897).  The fix replaces the ambiguous `\s*`
-# suffix with `[ \t]*(?:\n|$)` which matches only horizontal whitespace
-# followed by a hard line boundary, so there is no character-class overlap
-# and the engine cannot split the match in multiple ways.
+# ReDoS fix (CodeQL py/polynomial-redos, issue #1897):
+#
+# The original pattern `<[^>]*>\s*` was vulnerable because `\s*` (which
+# matches newlines) could overlap with `[^>]*` on inputs that contain no
+# closing `>` (e.g. `base<!!<!<...`), forcing the engine to explore every
+# possible split between the two quantifiers — O(n²) backtracking.
+#
+# The fix uses `<[^>\r\n]*>` for the IRI body: excluding CR and LF from
+# the character class means the IRI match can never span a line boundary,
+# and the disjoint trailing `[ \t]*` (horizontal whitespace only) has zero
+# character-class overlap with `[^>\r\n]*`, so the engine has exactly one
+# way to match.  No end-of-line anchor is needed or used, which correctly
+# handles both inline prologues (`PREFIX ex: <...> SELECT ...` on one line)
+# and CRLF line endings (`\r\n`) without any special casing.
 _COMMENT_LINE = re.compile(r"(?:^|(?<=\s))#[^\n]*", re.MULTILINE)
 _PREFIX_DECL = re.compile(
-    r"^[ \t]*(?:PREFIX[ \t]+\S+|BASE)[ \t]*<[^>]*>[ \t]*(?:\n|$)",
+    r"^[ \t]*(?:PREFIX[ \t]+\S+|BASE)[ \t]*<[^>\r\n]*>[ \t]*",
     re.IGNORECASE | re.MULTILINE,
 )
 
 
-_SPARQL_MAX_QUERY_LEN = 10_000  # chars; guards regex cost on uncontrolled input
-
-
 def _is_read_only_query(query: str) -> bool:
     """Return True only for genuine read-only SPARQL queries.
 
@@ -76,12 +80,11 @@ def _is_read_only_query(query: str) -> bool:
     checking the first keyword. Also rejects queries containing SPARQL Update
     keywords anywhere in the body, preventing injection via embedded strings
     or multi-statement tricks.
-    """
-    # 0. Reject excessively long inputs before any regex work (defense-in-depth
-    #    against ReDoS even if a future regex change reintroduces ambiguity).
-    if len(query) > _SPARQL_MAX_QUERY_LEN:
-        return False
 
+    Note: callers are responsible for enforcing any input-length limit *before*
+    calling this function so that an oversized-query rejection can be surfaced
+    as a distinct, actionable error rather than the generic read-only message.
+    """
     # 1. Remove single-line comments that could hide the real query type
     cleaned = _COMMENT_LINE.sub("", query)
     # 2. Remove PREFIX/BASE declarations
@@ -175,6 +178,11 @@ _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
+# Defense-in-depth against ReDoS: reject inputs longer than this before any
+# regex work so that even a future regex regression is bounded.  Checked in
+# execute_sparql() (not inside _is_read_only_query) so the route can return
+# a distinct, actionable error message rather than the generic read-only one.
+_SPARQL_MAX_QUERY_LEN = 10_000  # chars
 
 # Semaphore caps how many graph.query calls run concurrently so that
 # timed-out threads (which keep running in the pool) cannot crowd out
@@ -203,6 +211,24 @@ async def execute_sparql(
     req: SparqlRequest,
     session: GraphSession = Depends(get_session),
 ):
+    # Resource-limit check: reject oversized queries before any regex work.
+    # This is intentionally a separate, earlier check from _is_read_only_query
+    # so clients receive a specific, actionable message rather than the generic
+    # read-only rejection, and operators can tune _SPARQL_MAX_QUERY_LEN without
+    # touching query-semantics code.
+    if len(req.query) > _SPARQL_MAX_QUERY_LEN:
+        return SparqlResponse(
+            columns=[],
+            rows=[],
+            total=0,
+            error=(
+                f"Query exceeds the maximum allowed length of "
+                f"{_SPARQL_MAX_QUERY_LEN:,} characters "
+                f"({len(req.query):,} received). "
+                f"Please shorten your query."
+            ),
+        )
+
     if not _is_read_only_query(req.query):
         return SparqlResponse(
             columns=[],

tests/explorer/test_sparql_route.py

@@ -317,6 +317,47 @@ def test_oversized_graph_returns_clean_error_not_a_crash(client):
     assert payload["rows"] == []
 
 
+def test_oversized_query_returns_distinct_length_error(client):
+    """A query exceeding _SPARQL_MAX_QUERY_LEN must be rejected with a
+    specific, actionable error message — not the generic read-only message.
+    Clients need to distinguish a size-limit rejection from an actual
+    non-read-only query rejection to react correctly (e.g. split the query
+    vs. rewrite it)."""
+    with patch.object(sparql_mod, "_SPARQL_MAX_QUERY_LEN", 10):
+        resp = _post(client, "SELECT ?s WHERE { ?s ?p ?o }")  # 30 chars > 10
+    assert resp.status_code == 200
+    payload = resp.json()
+    assert payload["error"] is not None
+    # Must mention the limit, not the generic read-only message
+    assert "length" in payload["error"].lower() or "characters" in payload["error"].lower()
+    assert "Only SELECT" not in payload["error"]
+    assert payload["rows"] == []
+    assert payload["columns"] == []
+    assert payload["total"] == 0
+
+
+def test_oversized_query_never_touches_the_graph(client):
+    """An oversized query must be rejected before _build_rdflib_graph is
+    called — the length guard must short-circuit the entire pipeline."""
+    with patch.object(sparql_mod, "_SPARQL_MAX_QUERY_LEN", 10):
+        with patch.object(sparql_mod, "_build_rdflib_graph") as mock_build:
+            resp = _post(client, "SELECT ?s WHERE { ?s ?p ?o }")
+    assert resp.status_code == 200
+    assert resp.json()["error"] is not None
+    mock_build.assert_not_called()
+
+
+def test_query_exactly_at_length_limit_is_accepted(client):
+    """A query whose length equals the limit exactly must not be rejected —
+    the guard is strictly greater-than, not greater-than-or-equal."""
+    short_query = "ASK {}"
+    with patch.object(sparql_mod, "_SPARQL_MAX_QUERY_LEN", len(short_query)):
+        resp = _post(client, short_query)
+    assert resp.status_code == 200
+    payload = resp.json()
+    assert payload["error"] is None
+
+
 # ---------------------------------------------------------------------------
 # Data-mapping fidelity: does the graph->RDF projection reflect session state?
 # ---------------------------------------------------------------------------

tests/test_security_regression.py

@@ -123,6 +123,43 @@ class TestSparqlReadOnlyValidation:
         )
         assert _is_read_only_query(query)
 
+    def test_inline_prefix_before_select_allowed(self):
+        """PREFIX declaration on the same line as the query keyword (inline
+        prologue) must be stripped correctly so SELECT is seen first.
+        Regression for the [ \\t]*(?:\\n|$) anchor that rejected this form."""
+        query = "PREFIX ex: <http://example.org/> SELECT ?s WHERE { ?s ex:p ?o }"
+        assert _is_read_only_query(query)
+
+    def test_crlf_line_endings_with_prefix(self):
+        """Windows CRLF line endings (\\r\\n) between PREFIX and SELECT must
+        be handled correctly.  The previous (?:\\n|$) anchor did not allow
+        the \\r before \\n, causing stripping to fail."""
+        query = "PREFIX ex: <http://example.org/>\r\nSELECT ?s WHERE { ?s ?p ?o }"
+        assert _is_read_only_query(query)
+
+    def test_crlf_multiple_prefixes_then_select(self):
+        """Multiple PREFIX lines with CRLF endings should all be stripped."""
+        query = (
+            "PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>\r\n"
+            "PREFIX ex: <http://example.org/>\r\n"
+            "SELECT ?s WHERE { ?s rdf:type ex:Thing }"
+        )
+        assert _is_read_only_query(query)
+
+    def test_inline_prefix_before_insert_still_blocked(self):
+        """An inline PREFIX followed by INSERT must not be allowed — the
+        inline stripping fix must not open a bypass for write operations."""
+        query = "PREFIX ex: <http://example.org/> INSERT DATA { ex:s ex:p ex:o }"
+        assert not _is_read_only_query(query)
+
+    def test_long_valid_query_not_rejected_by_is_read_only(self):
+        """_is_read_only_query must not enforce the length limit itself —
+        that responsibility belongs to execute_sparql() so the route can
+        return a distinct, actionable error.  A syntactically valid but long
+        SELECT query must still return True from this function."""
+        long_query = "SELECT ?s WHERE { ?s ?p ?o } # " + ("x" * 20_000)
+        assert _is_read_only_query(long_query)
+
 
 # ===================================================================
 # 2. Cypher injection prevention