fix(knowledge_sql): match write keywords on word boundaries (allow valid SELECTs) (#640)

* fix(knowledge_sql): match write keywords on word boundaries

The read-only guard rejected a query if any of DROP/DELETE/INSERT/UPDATE/
ALTER/CREATE/ATTACH appeared as a bare substring of the uppercased text. That
wrongly blocks valid SELECTs whose column/alias/literal merely contains one --
e.g. "deleted_at" (DELETE), "created_at" (CREATE), "updated_content" (UPDATE).
The knowledge_chunks table actually has deleted_at/created_at columns and the
store's own retrieval filters on "WHERE deleted_at IS NULL", so realistic
read queries were refused. Match on word boundaries with a compiled regex,
mirroring the sibling tool db_query.py. Add a regression test.

* fix(knowledge_sql): ignore string literals in keyword scan, broaden error handling

- Strip single-quoted literals before the forbidden-keyword scan so
  SELECTs whose data merely mentions a write keyword (e.g. LIKE
  '%delete%') are not rejected.
- Catch sqlite3.Error instead of only OperationalError so multi-
  statement strings return a failed ToolResult instead of raising.
- Document created_at/deleted_at in the tool's schema description.

---------

Co-authored-by: Elliot Slusky <elliot@slusky.com>

src/openjarvis/tools/knowledge_sql.py

@@ -6,6 +6,7 @@ and filtering operations that BM25 search cannot handle.
 
 from __future__ import annotations
 
+import re
 import sqlite3
 from typing import Any, Optional
 
@@ -16,10 +17,25 @@ from openjarvis.tools._stubs import BaseTool, ToolSpec
 
 _MAX_ROWS = 50
 
+# Write keywords are matched on word boundaries (mirroring db_query.py) so that
+# a read-only SELECT is not rejected just because a column/alias/literal happens
+# to contain one as a substring (e.g. "deleted_at", "created_at").
+_FORBIDDEN_RE = re.compile(
+    r"\b(DROP|DELETE|INSERT|UPDATE|ALTER|CREATE|TRUNCATE|ATTACH)\b",
+    re.IGNORECASE,
+)
+
+# String literals are stripped before the keyword scan so that data mentioning
+# a write keyword (e.g. WHERE content LIKE '%delete%') is not rejected. A write
+# "hidden" in a literal still cannot execute: the query must start with SELECT
+# and sqlite3 refuses multi-statement strings.
+_STRING_LITERAL_RE = re.compile(r"'[^']*'")
+
 _SCHEMA_DESCRIPTION = (
     "Table: knowledge_chunks\n"
     "Columns: id, content, source, doc_type, doc_id, title, author, "
-    "participants, timestamp, thread_id, url, metadata, chunk_index"
+    "participants, timestamp, thread_id, url, metadata, chunk_index, "
+    "created_at, deleted_at (NULL for active rows)"
 )
 
 
@@ -84,21 +100,20 @@ class KnowledgeSQLTool(BaseTool):
                 success=False,
             )
 
-        _FORBIDDEN = ("DROP", "DELETE", "INSERT", "UPDATE", "ALTER", "CREATE", "ATTACH")
-        for forbidden in _FORBIDDEN:
-            if forbidden in normalized:
-                return ToolResult(
-                    tool_name="knowledge_sql",
-                    content=(
-                        f"Query contains forbidden keyword: {forbidden}."
-                        " Only SELECT queries allowed."
-                    ),
-                    success=False,
-                )
+        forbidden = _FORBIDDEN_RE.search(_STRING_LITERAL_RE.sub("''", query))
+        if forbidden:
+            return ToolResult(
+                tool_name="knowledge_sql",
+                content=(
+                    f"Query contains forbidden keyword: {forbidden.group(1).upper()}."
+                    " Only SELECT queries allowed."
+                ),
+                success=False,
+            )
 
         try:
             rows = self._store._conn.execute(query).fetchmany(_MAX_ROWS)
-        except sqlite3.OperationalError as exc:
+        except sqlite3.Error as exc:
             return ToolResult(
                 tool_name="knowledge_sql",
                 content=f"SQL error: {exc}",

tests/tools/test_knowledge_sql.py

@@ -64,6 +64,41 @@ def test_rejects_drop(store: KnowledgeStore) -> None:
     assert not result.success
 
 
+def test_allows_select_with_keyword_substring(store: KnowledgeStore) -> None:
+    """A read-only SELECT must not be rejected because a column/alias merely
+    contains a write keyword as a substring (e.g. 'created' -> CREATE)."""
+    from openjarvis.tools.knowledge_sql import KnowledgeSQLTool
+
+    tool = KnowledgeSQLTool(store=store)
+    result = tool.execute(
+        query="SELECT author AS created_author FROM knowledge_chunks"
+    )
+    assert result.success, result.content
+    assert "Alice" in result.content
+
+
+def test_allows_keyword_inside_string_literal(store: KnowledgeStore) -> None:
+    """A write keyword appearing only inside a string literal must not be
+    treated as a forbidden statement."""
+    from openjarvis.tools.knowledge_sql import KnowledgeSQLTool
+
+    tool = KnowledgeSQLTool(store=store)
+    result = tool.execute(
+        query="SELECT content FROM knowledge_chunks WHERE content LIKE '%delete%'"
+    )
+    assert result.success, result.content
+
+
+def test_rejects_multi_statement(store: KnowledgeStore) -> None:
+    """Multi-statement strings fail with a ToolResult, not an exception."""
+    from openjarvis.tools.knowledge_sql import KnowledgeSQLTool
+
+    tool = KnowledgeSQLTool(store=store)
+    result = tool.execute(query="SELECT 1; VACUUM")
+    assert not result.success
+    assert "error" in result.content.lower()
+
+
 def test_handles_bad_sql(store: KnowledgeStore) -> None:
     from openjarvis.tools.knowledge_sql import KnowledgeSQLTool