Compare commits

...
Author SHA1 Message Date
ScottyAmr d9725fbb6a fix(speech): close temp file before transcribing to fix Windows EACCES in faster-whisper backend (#638)
faster-whisper's transcribe() was handed the path of a still-open NamedTemporaryFile; on Windows the open handle is exclusive, so PyAV's reopen failed with EACCES and local STT was broken. Switch to delete=False, close before transcribing, and unlink in a finally. The write is wrapped in the file's context manager so the handle closes even if write() raises, and unlink failures are logged at debug.
2026-07-16 17:09:45 -07:00
Syed Osama Ali ShahandElliot Slusky 23f04264f9 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>
2026-07-16 16:36:31 -07:00
github-actions[bot] 8b59eb87e0 chore: update clone traffic data [skip ci] 2026-07-16 08:08:14 +00:00
6 changed files with 149 additions and 21 deletions
+1 -1
View File
@@ -1,7 +1,7 @@
{
"schemaVersion": 1,
"label": "Git Clones",
"message": "156,960",
"message": "159,322",
"color": "green",
"namedLogo": "git"
}
+4 -3
View File
@@ -1,6 +1,6 @@
{
"total_clones": 156960,
"last_updated": "2026-07-15T08:04:25Z",
"total_clones": 159322,
"last_updated": "2026-07-16T08:08:14Z",
"daily": {
"2026-03-27": 2189,
"2026-03-28": 1874,
@@ -111,6 +111,7 @@
"2026-07-11": 2185,
"2026-07-12": 1917,
"2026-07-13": 2102,
"2026-07-14": 2337
"2026-07-14": 2337,
"2026-07-15": 2362
}
}
+18 -4
View File
@@ -3,6 +3,7 @@
from __future__ import annotations
import logging
import os
import tempfile
from typing import List, Optional
@@ -105,11 +106,15 @@ class FasterWhisperBackend(SpeechBackend):
try:
model = self._ensure_model()
# Write audio to a temp file (faster-whisper needs a file path)
# Write audio to a temp file (faster-whisper needs a file path).
# delete=False + manual unlink: on Windows an open
# NamedTemporaryFile holds an exclusive handle, so PyAV's reopen
# of tmp.name inside model.transcribe() fails with EACCES.
suffix = f".{format}" if not format.startswith(".") else format
with tempfile.NamedTemporaryFile(suffix=suffix, delete=True) as tmp:
tmp.write(audio)
tmp.flush()
tmp = tempfile.NamedTemporaryFile(suffix=suffix, delete=False)
try:
with tmp:
tmp.write(audio)
kwargs = {}
if language:
@@ -117,6 +122,15 @@ class FasterWhisperBackend(SpeechBackend):
segments_iter, info = model.transcribe(tmp.name, **kwargs)
segments_list = list(segments_iter)
finally:
try:
os.unlink(tmp.name)
except OSError as unlink_exc:
logger.debug(
"Could not remove temp audio file %s: %s",
tmp.name,
unlink_exc,
)
except Exception as exc:
self._last_error = str(exc)
raise
+28 -13
View File
@@ -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}",
+63
View File
@@ -53,6 +53,69 @@ def test_faster_whisper_transcribe():
assert result.duration_seconds == 1.5
def test_faster_whisper_transcribe_temp_file_reopenable_and_removed():
"""The temp file must be closed before the model reads it, and gone after.
On Windows, an open NamedTemporaryFile holds an exclusive handle, so
PyAV's reopen of the path inside model.transcribe() fails with EACCES
unless the file is closed first. Opening the path inside the mocked
transcribe reproduces that failure mode on Windows.
"""
import os
mock_info = MagicMock()
mock_info.language = "en"
mock_info.language_probability = 0.95
mock_info.duration = 1.5
seen = {}
def fake_transcribe(path, **kwargs):
seen["path"] = path
with open(path, "rb") as fh:
seen["content"] = fh.read()
return iter(()), mock_info
mock_model = MagicMock()
mock_model.transcribe.side_effect = fake_transcribe
with patch(
"openjarvis.speech.faster_whisper.WhisperModel",
return_value=mock_model,
):
backend = FasterWhisperBackend(model_size="base", device="cpu")
backend.transcribe(b"fake audio bytes")
assert seen["content"] == b"fake audio bytes"
assert not os.path.exists(seen["path"])
def test_faster_whisper_transcribe_removes_temp_file_on_error():
"""The temp file is cleaned up even when transcription fails."""
import os
seen = {}
def fake_transcribe(path, **kwargs):
seen["path"] = path
raise RuntimeError("decode failed")
mock_model = MagicMock()
mock_model.transcribe.side_effect = fake_transcribe
with patch(
"openjarvis.speech.faster_whisper.WhisperModel",
return_value=mock_model,
):
backend = FasterWhisperBackend(model_size="base", device="cpu")
with pytest.raises(RuntimeError, match="decode failed"):
backend.transcribe(b"fake audio bytes")
assert "path" in seen
assert not os.path.exists(seen["path"])
assert "decode failed" in (backend.last_error() or "")
def test_faster_whisper_falls_back_from_unsupported_float16():
mock_model = MagicMock()
+35
View File
@@ -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