From f70046fcbb39fefbc4970e5b0b46cbda638881ba Mon Sep 17 00:00:00 2001 From: Tharick Jairam Date: Fri, 31 Jul 2026 14:08:56 -0400 Subject: [PATCH 1/2] fix: implement SQLite partial unique indexes to prevent session_id collision during node updates --- .pre-commit-config.yaml | 2 +- CHANGELOG.md | 31 ++- jvspatial/db/_sqlite_translate.py | 78 +++++- jvspatial/db/sqlite.py | 179 +++++++++++++- jvspatial/storage/interfaces/local.py | 5 +- jvspatial/storage/interfaces/s3.py | 5 +- jvspatial/storage/security/validator.py | 29 ++- tests/db/test_sqlite_partial_index.py | 315 ++++++++++++++++++++++++ tests/db/test_sqlite_translate.py | 61 ++++- tests/storage/test_security.py | 73 ++++++ 10 files changed, 756 insertions(+), 22 deletions(-) create mode 100644 tests/db/test_sqlite_partial_index.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 028d479..cdb46f4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v2.3.0 + rev: v6.0.0 hooks: - id: check-yaml args: [--allow-multiple-documents] diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d36f13..311181d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Partial-index repair log is INFO, not WARNING** (`jvspatial/db/sqlite.py`). + Dropping a non-partial index so it can be recreated with `WHERE` is expected + one-shot migration noise; log at info. Also satisfy ruff SIM110 in + `_index_needs_partial_repair` and mypy narrowing for `$eq` string literals in + `_sqlite_translate.py`. + +- **SQLite connect-time repair of global ``session_id`` unique indexes** + (`jvspatial/db/sqlite.py`). Opening a SQLite DB now drops UNIQUE indexes on + ``json_extract(data, '$.context.session_id')`` that lack a ``WHERE`` clause, + so a process that never re-ran ``ensure_indexes(Conversation)`` after the + partial-filter fix still stops wiping Interaction rows. Partial unique + ``create_index`` failures now raise instead of logging a warning. + Coverage: `tests/db/test_sqlite_partial_index.py`. + +- **SQLite partial unique indexes** (`jvspatial/db/sqlite.py`, `_sqlite_translate.py`). + `SQLiteDB.create_index` ignored Mongo-style `partialFilterExpression` / + `partial_filter_expression` kwargs and created **global** unique indexes on + shared `node` collections. That made `INSERT OR REPLACE` wipe `Interaction` + rows when they shared `context.session_id` with a `Conversation` (orchestrator + history empty on SQLite, fine on JsonDB). SQLite now translates the same + small dialect as Postgres into a `WHERE` clause, raises if a unique partial + filter cannot be translated, and drops/recreates a pre-existing non-partial + index of the same name so a restart self-heals. Coverage in + `tests/db/test_sqlite_partial_index.py` and translator unit tests. - **`SQLiteDB.find` treated `sort=[]` as an untranslatable sort** (`jvspatial/db/sqlite.py`). An empty list failed the `sort is None` guard, so @@ -142,14 +166,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `(sort_value, id)` cursor — the default implementation tracks `id` only, so a non-`id` sort drops records that sort late but carry a lower `id`. - ## [0.0.11] - 2026-07-02 ### Fixed - **Single-hop `find_connected_nodes` fast path dropped valid neighbors** (`jvspatial/core/entities/node.py`). Two regressions introduced with the 0.0.10 fast path, both invisible to the existing suite because `JsonDB` exposes no `find_connected_nodes` (so tests fell back to the slow edge scan): - - *Limit-before-filter:* `limit` was pushed into the DB scan **before** the Python node-type filter, so `node(node=T)` / `nodes(node=T, limit=n)` could return nothing when a non-matching neighbor sorted first, even though matching neighbors existed. `limit` is now applied to the DB scan only when there is no node filter; otherwise it is applied after filtering. - - *Subtype resolution under an entity-name collision:* rows were deserialized with the base `Node` class, so when two `Node` subclasses persisted in one database shared an entity name (e.g. an app `User` and an embedded-agent `User`), `find_subclass_by_name` returned the first global match and `isinstance`-based filtering silently dropped the valid neighbor. The fast path now hydrates using the caller's requested concrete type as the resolution hint. + - _Limit-before-filter:_ `limit` was pushed into the DB scan **before** the Python node-type filter, so `node(node=T)` / `nodes(node=T, limit=n)` could return nothing when a non-matching neighbor sorted first, even though matching neighbors existed. `limit` is now applied to the DB scan only when there is no node filter; otherwise it is applied after filtering. + - _Subtype resolution under an entity-name collision:_ rows were deserialized with the base `Node` class, so when two `Node` subclasses persisted in one database shared an entity name (e.g. an app `User` and an embedded-agent `User`), `find_subclass_by_name` returned the first global match and `isinstance`-based filtering silently dropped the valid neighbor. The fast path now hydrates using the caller's requested concrete type as the resolution hint. - Regression coverage: `tests/core/test_connected_nodes_fast_path.py` installs a `find_connected_nodes` shim so the fast path is exercised (the stock `JsonDB` never did). ## [0.0.10] - 2026-07-02 @@ -290,7 +313,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `jvspatial.utils.stability.emit_experimental_once(name, message)` — public hook for opt-in surfaces that need to emit the experimental warning without going through the `@experimental` decorator (replaces private `_emit_once` calls). (Audit §7.7.) - `tests/db/test_sqlite_cross_loop_audit.py`, `tests/utils/test_wave4_polish_audit.py` — 10 new regression cases pinning Wave 4 audit fixes. - `jvspatial.core.entities.TraversalSkipped` and `TraversalPaused` exception classes. `Walker.skip()` now raises `TraversalSkipped` (caught via `except TraversalSkipped`); previously relied on substring-matching `"Node skipped"` in the message. (Audit §2.9 / SPEC §6.5.) -- `PathSanitizer` rejects Windows-reserved filenames (`CON`, `PRN`, `AUX`, `NUL`, `COM1-9`, `LPT1-9`) regardless of host OS. ``CON.txt`` is rejected; ``CONFIG.json`` passes. (Audit §4.18 / SPEC §15.1.) +- `PathSanitizer` rejects Windows-reserved filenames (`CON`, `PRN`, `AUX`, `NUL`, `COM1-9`, `LPT1-9`) regardless of host OS. `CON.txt` is rejected; `CONFIG.json` passes. (Audit §4.18 / SPEC §15.1.) - `tests/storage/test_windows_reserved_audit.py`, `tests/core/test_wave5_walker_audit.py`, `tests/api/test_deferred_invoke_fail_closed_audit.py`, `tests/db/test_sqlite_id_coercion_audit.py` — 23 new regression cases pinning Wave 5 audit fixes. ### Fixed diff --git a/jvspatial/db/_sqlite_translate.py b/jvspatial/db/_sqlite_translate.py index dda3a6e..ea48ac0 100644 --- a/jvspatial/db/_sqlite_translate.py +++ b/jvspatial/db/_sqlite_translate.py @@ -295,4 +295,80 @@ def translate_sort(sort: Optional[List[Tuple[str, int]]]) -> Optional[str]: return ", ".join(parts) -__all__ = ["translate_query", "translate_sort"] +def _sql_string_literal(value: str) -> str: + """Quote a Python string as a SQLite string literal (single-quote escape).""" + return "'" + value.replace("'", "''") + "'" + + +def translate_partial_filter_expression( + pfe: Dict[str, Any], +) -> Optional[str]: + """Translate a Mongo-style ``partialFilterExpression`` to a SQLite WHERE. + + Used by ``SQLiteDB.create_index`` so partial unique indexes are not + silently demoted to global unique indexes (which collides when other + Node subclasses share indexed field values — e.g. Conversation and + Interaction both writing ``context.session_id``). + + Same deliberately small dialect as Postgres: + + - ``{field: scalar}`` → ``json_extract(...) = `` + - ``{field: {"$eq": scalar}}`` → same + - ``{field: {"$gt": scalar}}`` → ``json_extract(...) > `` + - ``{field: {"$exists": True/False}}`` → ``IS NOT NULL`` / ``IS NULL`` + + Top-level keys are AND-ed. Values are inlined (CREATE INDEX WHERE + cannot use bound parameters). Returns ``None`` for unsupported shapes. + """ + if not isinstance(pfe, dict) or not pfe: + return None + clauses: List[str] = [] + for field, spec in pfe.items(): + if not _safe_field_path(field): + return None + extract = _json_extract(field) + if isinstance(spec, dict): + if len(spec) != 1: + return None + op, val = next(iter(spec.items())) + if op == "$eq": + if not isinstance(val, (str, int, float, bool)): + return None + if isinstance(val, bool): + clauses.append(f"{extract} = {1 if val else 0}") + elif isinstance(val, (int, float)) and not isinstance(val, bool): + clauses.append(f"{extract} = {val}") + elif isinstance(val, str): + clauses.append(f"{extract} = {_sql_string_literal(val)}") + else: + return None + elif op == "$gt": + if not isinstance(val, (str, int, float)) or isinstance(val, bool): + return None + if isinstance(val, (int, float)): + clauses.append(f"{extract} > {val}") + else: + clauses.append(f"{extract} > {_sql_string_literal(val)}") + elif op == "$exists": + if val: + clauses.append(f"{extract} IS NOT NULL") + else: + clauses.append(f"{extract} IS NULL") + else: + return None + elif isinstance(spec, bool): + clauses.append(f"{extract} = {1 if spec else 0}") + elif isinstance(spec, (int, float)): + clauses.append(f"{extract} = {spec}") + elif isinstance(spec, str): + clauses.append(f"{extract} = {_sql_string_literal(spec)}") + else: + return None + return " AND ".join(clauses) + + +__all__ = [ + "translate_query", + "translate_sort", + "translate_partial_filter_expression", +] diff --git a/jvspatial/db/sqlite.py b/jvspatial/db/sqlite.py index 153edc3..170d062 100644 --- a/jvspatial/db/sqlite.py +++ b/jvspatial/db/sqlite.py @@ -16,7 +16,11 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set, Tuple, Union -from ._sqlite_translate import translate_query, translate_sort +from ._sqlite_translate import ( + translate_partial_filter_expression, + translate_query, + translate_sort, +) from .database import Database, finalize_find_results from .query import QueryEngine @@ -196,6 +200,10 @@ async def _get_connection(self) -> "Connection": """ ) await self._connection.commit() + # Drop legacy global unique indexes on session_id before any + # writes — CREATE INDEX IF NOT EXISTS would leave them in place + # and Interaction rows sharing a Conversation session_id get wiped. + await self._repair_non_partial_unique_indexes(self._connection) self._initialized = True return self._connection @@ -212,6 +220,116 @@ def _json_path(self, field_path: str) -> str: # Convert "context.user_id" to "$.context.user_id" for JSON extraction return f"$.{field_path}" + @staticmethod + def _is_non_partial_session_id_unique_index(sql: Optional[str]) -> bool: + """True for UNIQUE indexes on context.session_id with no WHERE clause.""" + if not sql: + return False + lower = sql.lower() + if "unique" not in lower: + return False + if "where" in lower: + return False + return "json_extract(data, '$.context.session_id')" in lower + + async def _repair_non_partial_unique_indexes( + self, connection: "Connection" + ) -> None: + """Drop global unique indexes on ``context.session_id`` that lack WHERE. + + Pre-partial-filter SQLite builds created a blanket UNIQUE on + ``session_id`` for the whole ``node`` collection. Conversation and + Interaction share that field; ``INSERT OR REPLACE`` then deleted + Interaction rows whenever Conversation was saved. Dropping here + lets the next ``ensure_indexes(Conversation)`` recreate a partial + unique index. + """ + cursor = await connection.execute( + "SELECT name, sql FROM sqlite_master WHERE type='index' AND sql IS NOT NULL" + ) + rows = await cursor.fetchall() + dropped = 0 + for row in rows: + name = row[0] + sql = row[1] + if not self._is_non_partial_session_id_unique_index(sql): + continue + # Only drop indexes we (or prior jvspatial) named — avoid + # touching unrelated user indexes with similar SQL text. + if not str(name).startswith("idx_"): + continue + logger.warning( + "Dropping non-partial unique index '%s' on connect " + "(session_id uniqueness must be partial so Interaction " + "rows are not wiped). It will be recreated with WHERE on " + "the next Conversation ensure_indexes.", + name, + ) + await connection.execute(f'DROP INDEX IF EXISTS "{name}"') + dropped += 1 + for names in self._created_indexes.values(): + names.discard(name) + if dropped: + await connection.commit() + + @staticmethod + def _resolve_index_where(**kwargs: Any) -> Optional[str]: + """Resolve a CREATE INDEX WHERE clause from kwargs. + + Honors an explicit ``where=`` string, otherwise translates a + Mongo-style partial filter under any of the names + ``get_indexes()`` / annotations may emit. Returns ``None`` when + no partial filter was requested. Raises ``ValueError`` when a + partial filter is present but cannot be translated — never + silently demote a partial unique index to a global unique. + """ + explicit = kwargs.get("where") + if isinstance(explicit, str) and explicit.strip(): + return explicit.strip() + + pfe = ( + kwargs.get("index_partial_filter_expression") + or kwargs.get("partial_filter_expression") + or kwargs.get("partialFilterExpression") + ) + if not pfe: + return None + translated = translate_partial_filter_expression(pfe) + if translated is None: + raise ValueError( + "SQLiteDB.create_index: cannot translate " + f"index_partial_filter_expression {pfe!r} to a " + "SQLite WHERE clause. Pass an explicit ``where=`` " + "argument or use a supported filter shape " + "(equality / $gt / $exists on safe field paths " + "with scalar values)." + ) + return translated + + @staticmethod + def _index_needs_partial_repair( + existing_sql: Optional[str], where_sql: Optional[str] + ) -> bool: + """Return True when an on-disk index must be dropped and rebuilt. + + A global unique index created before partial-filter support will + lack a WHERE clause; ``CREATE INDEX IF NOT EXISTS`` would leave + it in place and keep wiping colliding rows (Conversation vs + Interaction ``session_id``). + """ + if not where_sql: + return False + if not existing_sql: + return False + lower = existing_sql.lower() + if "where" not in lower: + return True + # Require each AND-ed predicate from the desired WHERE to appear + # (SQLite stores CREATE INDEX SQL as we wrote it). + return any( + part.strip() not in existing_sql for part in where_sql.split(" AND ") + ) + async def create_index( self, collection: str, @@ -225,11 +343,16 @@ async def create_index( collection: Collection name field_or_fields: Single field name (str) or list of (field_name, direction) tuples for compound indexes unique: Whether the index should enforce uniqueness - **kwargs: Additional options (ignored for SQLite) + **kwargs: Partial-filter options (``partialFilterExpression``, + ``partial_filter_expression``, + ``index_partial_filter_expression``, or explicit ``where=``). + Same Mongo dialect as PostgresDB.create_index. Note: SQLite indexes on nested JSON fields use json_extract() function. Direction parameter is ignored for SQLite (always ascending). + When a partial filter is required and an older non-partial + index of the same name exists, it is dropped and recreated. """ connection = await self._get_connection() @@ -248,9 +371,37 @@ async def create_index( field_names = "_".join(field.replace(".", "_") for field, _ in fields) index_name = f"idx_{collection}_{field_names}" - # Check if index already exists + where_sql = self._resolve_index_where(**kwargs) + + # Inspect on-disk definition so a prior global unique index can + # be repaired after upgrading to partial-filter support. + cursor = await connection.execute( + "SELECT sql FROM sqlite_master WHERE type='index' AND name=?", + (index_name,), + ) + row = await cursor.fetchone() + existing_sql = row[0] if row else None + + if self._index_needs_partial_repair(existing_sql, where_sql): + logger.info( + "Dropping non-partial index '%s' so it can be recreated " + "with WHERE %s", + index_name, + where_sql, + ) + await connection.execute(f"DROP INDEX IF EXISTS {index_name}") + await connection.commit() + self._created_indexes[collection].discard(index_name) + existing_sql = None + + # Check if index already exists (in-process or on disk, correct form) if index_name in self._created_indexes[collection]: - return # Index already created + return + if existing_sql is not None and not self._index_needs_partial_repair( + existing_sql, where_sql + ): + self._created_indexes[collection].add(index_name) + return # Build SQLite index creation statement # For nested fields, use json_extract() to extract values from JSON @@ -261,15 +412,15 @@ async def create_index( index_columns = ", ".join(index_expressions) unique_clause = "UNIQUE" if unique else "" + where_clause = f" WHERE {where_sql}" if where_sql else "" try: - # Create index on the records table - # Include collection in the index to support efficient filtering - # SQLite doesn't support parameterized WHERE clauses in CREATE INDEX, - # so we include collection as the first column + # Include collection in the index to support efficient filtering. + # Partial filters become a SQLite partial index WHERE clause so + # uniqueness is scoped the same way as Mongo/Postgres. sql = f""" CREATE {unique_clause} INDEX IF NOT EXISTS {index_name} - ON records (collection, {index_columns}) + ON records (collection, {index_columns}){where_clause} """ await connection.execute(sql) @@ -280,10 +431,18 @@ async def create_index( logger.debug( f"Created index '{index_name}' on collection '{collection}' " - f"(unique={unique}, fields={[f[0] for f in fields]})" + f"(unique={unique}, fields={[f[0] for f in fields]}" + f"{', partial' if where_sql else ''})" ) except Exception as e: + # Partial unique indexes protect Conversation/Interaction + # coexistence — never swallow create failures for those. + if unique and where_sql: + raise RuntimeError( + f"Failed to create partial unique index '{index_name}' " + f"on collection '{collection}': {e}" + ) from e logger.warning( f"Failed to create index '{index_name}' on collection '{collection}': {e}" ) diff --git a/jvspatial/storage/interfaces/local.py b/jvspatial/storage/interfaces/local.py index b0e4d28..6ab5c28 100644 --- a/jvspatial/storage/interfaces/local.py +++ b/jvspatial/storage/interfaces/local.py @@ -438,8 +438,11 @@ async def save_file( "Skipping MIME allowlist for internal marker: %s", file_path ) else: + hint_mime = ( + (metadata or {}).get("mime") if isinstance(metadata, dict) else None + ) validation = self.validator.validate_file( - content=content, filename=filename + content=content, filename=filename, hint_mime=hint_mime ) logger.debug(f"File validation passed: {validation}") diff --git a/jvspatial/storage/interfaces/s3.py b/jvspatial/storage/interfaces/s3.py index 4a6a7bd..031e6e2 100644 --- a/jvspatial/storage/interfaces/s3.py +++ b/jvspatial/storage/interfaces/s3.py @@ -341,8 +341,11 @@ async def save_file( "Skipping MIME allowlist for internal marker: %s", file_path ) else: + hint_mime = ( + (metadata or {}).get("mime") if isinstance(metadata, dict) else None + ) validation = self.validator.validate_file( - content=content, filename=filename + content=content, filename=filename, hint_mime=hint_mime ) logger.debug(f"File validation passed: {validation}") diff --git a/jvspatial/storage/security/validator.py b/jvspatial/storage/security/validator.py index 22c0d68..5b14e97 100644 --- a/jvspatial/storage/security/validator.py +++ b/jvspatial/storage/security/validator.py @@ -179,7 +179,11 @@ def __init__( self.strict_mime_check = strict_mime_check def validate_file( - self, content: bytes, filename: str, expected_mime_type: Optional[str] = None + self, + content: bytes, + filename: str, + expected_mime_type: Optional[str] = None, + hint_mime: Optional[str] = None, ) -> ValidationResult: """Validate file content and metadata. @@ -193,6 +197,10 @@ def validate_file( content: File content as bytes filename: Original filename expected_mime_type: Expected MIME type (optional) + hint_mime: Known MIME type to use when detection falls back to + octet-stream (optional). Useful when the caller already knows + the type (e.g. from upload metadata) but the file content or + filename alone isn't enough for detection. Returns: Dict with validation results: @@ -228,7 +236,7 @@ def validate_file( ) # Stage 2: MIME type detection - detected_mime = self.detect_mime_type(content, filename) + detected_mime = self.detect_mime_type(content, filename, hint_mime=hint_mime) # Stage 3: Extension validation extension = Path(filename).suffix.lower() @@ -287,15 +295,25 @@ def validate_file( "filename": filename, } - def detect_mime_type(self, content: bytes, filename: Optional[str] = None) -> str: + def detect_mime_type( + self, + content: bytes, + filename: Optional[str] = None, + *, + hint_mime: Optional[str] = None, + ) -> str: """Detect MIME type of file content. Uses python-magic if available for content-based detection, falls back to mimetypes module for extension-based detection. + When both fail, uses *hint_mime* if provided (e.g. from upload + metadata) instead of defaulting to octet-stream. Args: content: File content as bytes filename: Filename for extension-based fallback + hint_mime: Known MIME type to use when detection falls back + to octet-stream (optional) Returns: Detected MIME type string @@ -321,6 +339,11 @@ def detect_mime_type(self, content: bytes, filename: Optional[str] = None) -> st logger.debug(f"MIME type guessed from extension: {mime_type}") return mime_type + # Use hint if detection failed + if hint_mime and hint_mime != "application/octet-stream": + logger.debug(f"MIME type from hint: {hint_mime}") + return hint_mime + # Ultimate fallback logger.warning("Could not detect MIME type, defaulting to octet-stream") return "application/octet-stream" diff --git a/tests/db/test_sqlite_partial_index.py b/tests/db/test_sqlite_partial_index.py new file mode 100644 index 0000000..8cd3b8f --- /dev/null +++ b/tests/db/test_sqlite_partial_index.py @@ -0,0 +1,315 @@ +"""SQLite partial unique indexes — Conversation/Interaction session_id fix. + +A global UNIQUE on ``context.session_id`` makes ``INSERT OR REPLACE`` wipe +Interaction rows when Conversation shares the same session_id. Partial +filters (same dialect as Postgres) must scope uniqueness. +""" + +from __future__ import annotations + +import tempfile +from pathlib import Path +from typing import Optional + +import pytest + +from jvspatial.core.annotations import attribute, compound_index +from jvspatial.core.context import GraphContext, set_default_context +from jvspatial.core.entities import Node +from jvspatial.db import create_database + +try: + from jvspatial.db.sqlite import SQLiteDB + + HAS_SQLITE = True +except ImportError: # pragma: no cover + SQLiteDB = None # type: ignore[misc] + HAS_SQLITE = False + +pytestmark = pytest.mark.skipif( + not HAS_SQLITE, reason="aiosqlite is required for SQLite tests" +) + + +@pytest.fixture +def temp_db_path(): + with tempfile.TemporaryDirectory() as temp_dir: + yield Path(temp_dir) / "partial.db" + + +@pytest.fixture +async def sqlite_db(temp_db_path): + db = create_database("sqlite", db_path=str(temp_db_path)) + try: + yield db + finally: + if hasattr(db, "close"): + await db.close() + + +@pytest.fixture +async def sqlite_context(sqlite_db): + ctx = GraphContext(database=sqlite_db) + set_default_context(ctx) + return ctx + + +@compound_index( + [("session_id", 1)], + name="conversation_session_id", + unique=True, + partial_filter_expression={ + "context.session_id": {"$gt": ""}, + "context.status": {"$gt": ""}, + }, +) +class ConversationLike(Node): + __test__ = False + session_id: str = attribute(default="") + status: str = attribute(default="active") + + +class InteractionLike(Node): + __test__ = False + session_id: str = attribute(default="") + conversation_id: str = attribute(default="") + utterance: str = attribute(default="") + + +async def _index_sql(db: SQLiteDB, name: str) -> Optional[str]: + conn = await db._get_connection() + cursor = await conn.execute( + "SELECT sql FROM sqlite_master WHERE type='index' AND name=?", + (name,), + ) + row = await cursor.fetchone() + return row[0] if row else None + + +@pytest.mark.asyncio +async def test_partial_unique_allows_interaction_same_session_id( + sqlite_context, sqlite_db +): + """Conversation + Interaction can share session_id; both rows persist.""" + # Call create_index on this DB directly — GraphContext.ensure_indexes is + # process-cached per class and would skip on later temp databases. + await sqlite_db.create_index( + "node", + "context.session_id", + unique=True, + partialFilterExpression={ + "context.session_id": {"$gt": ""}, + "context.status": {"$gt": ""}, + }, + ) + + conv = await ConversationLike.create(session_id="sess_shared", status="active") + ix = await InteractionLike.create( + session_id="sess_shared", + conversation_id=conv.id, + utterance="hello", + ) + + assert await ConversationLike.get(conv.id) is not None + assert await InteractionLike.get(ix.id) is not None + + # Re-save conversation (the wipe path under a global unique index) + conv.status = "active" + await conv.save() + + conn = await sqlite_db._get_connection() + cursor = await conn.execute( + "SELECT id FROM records WHERE collection='node' AND id=?", + (ix.id,), + ) + assert await cursor.fetchone() is not None + cursor = await conn.execute( + "SELECT id FROM records WHERE collection='node' AND id=?", + (conv.id,), + ) + assert await cursor.fetchone() is not None + + +@pytest.mark.asyncio +async def test_partial_unique_still_enforced_within_conversation_set( + sqlite_context, sqlite_db +): + """Two Conversation-like rows with same session_id still collide.""" + await sqlite_db.create_index( + "node", + "context.session_id", + unique=True, + partialFilterExpression={ + "context.session_id": {"$gt": ""}, + "context.status": {"$gt": ""}, + }, + ) + + index_sql = await _index_sql(sqlite_db, "idx_node_context_session_id") + assert index_sql is not None + assert "UNIQUE" in index_sql.upper() + assert "WHERE" in index_sql.upper() + + first = await ConversationLike.create(session_id="sess_dup", status="active") + second = ConversationLike(session_id="sess_dup", status="active") + await second.save() + + # INSERT OR REPLACE on unique conflict: only one row for that session_id + conn = await sqlite_db._get_connection() + cursor = await conn.execute( + """ + SELECT id FROM records + WHERE collection = 'node' + AND json_extract(data, '$.context.session_id') = ? + AND json_extract(data, '$.entity') = 'ConversationLike' + """, + ("sess_dup",), + ) + ids = [row[0] for row in await cursor.fetchall()] + assert ids == [second.id] + assert first.id not in ids + + +@pytest.mark.asyncio +async def test_create_index_emits_where_clause(sqlite_db): + await sqlite_db.create_index( + "node", + "context.session_id", + unique=True, + partialFilterExpression={ + "context.session_id": {"$gt": ""}, + "context.status": {"$gt": ""}, + }, + ) + sql = await _index_sql(sqlite_db, "idx_node_context_session_id") + assert sql is not None + assert "WHERE" in sql + assert "json_extract(data, '$.context.session_id') > ''" in sql + assert "json_extract(data, '$.context.status') > ''" in sql + + +@pytest.mark.asyncio +async def test_create_index_repairs_non_partial_unique(sqlite_db): + # Simulate pre-fix global unique index + conn = await sqlite_db._get_connection() + await conn.execute( + """ + CREATE UNIQUE INDEX IF NOT EXISTS idx_node_context_session_id + ON records (collection, json_extract(data, '$.context.session_id')) + """ + ) + await conn.commit() + before = await _index_sql(sqlite_db, "idx_node_context_session_id") + assert before is not None + assert "WHERE" not in before.upper() + + await sqlite_db.create_index( + "node", + "context.session_id", + unique=True, + partialFilterExpression={ + "context.session_id": {"$gt": ""}, + "context.status": {"$gt": ""}, + }, + ) + after = await _index_sql(sqlite_db, "idx_node_context_session_id") + assert after is not None + assert "WHERE" in after.upper() + assert "json_extract(data, '$.context.status') > ''" in after + + +@pytest.mark.asyncio +async def test_create_index_raises_on_untranslatable_partial(sqlite_db): + with pytest.raises(ValueError, match="cannot translate"): + await sqlite_db.create_index( + "node", + "context.name", + unique=True, + partialFilterExpression={"context.name": {"$regex": "^x"}}, + ) + + +@pytest.mark.asyncio +async def test_connect_time_drops_non_partial_session_id_unique(temp_db_path): + """Opening a DB drops a legacy global session_id unique index.""" + # Seed with the bad index using a first connection, then close. + seed = create_database("sqlite", db_path=str(temp_db_path)) + conn = await seed._get_connection() + await conn.execute( + """ + CREATE UNIQUE INDEX IF NOT EXISTS idx_node_context_session_id + ON records (collection, json_extract(data, '$.context.session_id')) + """ + ) + await conn.commit() + before = await _index_sql(seed, "idx_node_context_session_id") + assert before is not None + assert "WHERE" not in before.upper() + await seed.close() + + # Fresh open must drop the non-partial unique index on connect. + db = create_database("sqlite", db_path=str(temp_db_path)) + try: + await db._get_connection() + after_connect = await _index_sql(db, "idx_node_context_session_id") + assert after_connect is None + + ctx = GraphContext(database=db) + set_default_context(ctx) + + await db.create_index( + "node", + "context.session_id", + unique=True, + partialFilterExpression={ + "context.session_id": {"$gt": ""}, + "context.status": {"$gt": ""}, + }, + ) + rebuilt = await _index_sql(db, "idx_node_context_session_id") + assert rebuilt is not None + assert "WHERE" in rebuilt.upper() + + conv = await ConversationLike.create(session_id="sess_connect", status="active") + ix = await InteractionLike.create( + session_id="sess_connect", + conversation_id=conv.id, + utterance="ping", + ) + conv.status = "active" + await conv.save() + + conn2 = await db._get_connection() + for entity_id in (conv.id, ix.id): + cursor = await conn2.execute( + "SELECT id FROM records WHERE collection='node' AND id=?", + (entity_id,), + ) + assert await cursor.fetchone() is not None + finally: + await db.close() + + +def test_is_non_partial_session_id_unique_index_helper(): + assert ( + SQLiteDB._is_non_partial_session_id_unique_index( + "CREATE UNIQUE INDEX idx_node_context_session_id " + "ON records (collection, json_extract(data, '$.context.session_id'))" + ) + is True + ) + assert ( + SQLiteDB._is_non_partial_session_id_unique_index( + "CREATE UNIQUE INDEX idx_node_context_session_id " + "ON records (collection, json_extract(data, '$.context.session_id')) " + "WHERE json_extract(data, '$.context.status') > ''" + ) + is False + ) + assert ( + SQLiteDB._is_non_partial_session_id_unique_index( + "CREATE INDEX idx_node_context_session_id " + "ON records (collection, json_extract(data, '$.context.session_id'))" + ) + is False + ) diff --git a/tests/db/test_sqlite_translate.py b/tests/db/test_sqlite_translate.py index e835609..61fa008 100644 --- a/tests/db/test_sqlite_translate.py +++ b/tests/db/test_sqlite_translate.py @@ -6,7 +6,11 @@ ``test_sqlite_pushdown.py``. """ -from jvspatial.db._sqlite_translate import translate_query, translate_sort +from jvspatial.db._sqlite_translate import ( + translate_partial_filter_expression, + translate_query, + translate_sort, +) class TestEqualityPushdown: @@ -193,3 +197,58 @@ def test_unsafe_field_falls_back(self): def test_empty_sort_returns_none(self): assert translate_sort(None) is None assert translate_sort([]) is None + + +class TestPartialFilterExpression: + def test_conversation_session_id_shape(self): + sql = translate_partial_filter_expression( + { + "context.session_id": {"$gt": ""}, + "context.status": {"$gt": ""}, + } + ) + assert sql == ( + "json_extract(data, '$.context.session_id') > '' AND " + "json_extract(data, '$.context.status') > ''" + ) + + def test_eq_and_exists(self): + sql = translate_partial_filter_expression( + { + "context.entity": {"$eq": "Agent"}, + "context.name": {"$exists": True}, + } + ) + assert sql == ( + "json_extract(data, '$.context.entity') = 'Agent' AND " + "json_extract(data, '$.context.name') IS NOT NULL" + ) + + def test_plain_equality_and_bool(self): + sql = translate_partial_filter_expression( + {"context.active": True, "context.kind": "session"} + ) + assert sql == ( + "json_extract(data, '$.context.active') = 1 AND " + "json_extract(data, '$.context.kind') = 'session'" + ) + + def test_string_literal_escapes_quotes(self): + sql = translate_partial_filter_expression({"context.label": {"$eq": "O'Brien"}}) + assert sql == "json_extract(data, '$.context.label') = 'O''Brien'" + + def test_unsupported_op_returns_none(self): + assert ( + translate_partial_filter_expression({"context.name": {"$regex": "^a"}}) + is None + ) + + def test_unsafe_path_returns_none(self): + assert ( + translate_partial_filter_expression({"context.bad-name": {"$gt": ""}}) + is None + ) + + def test_empty_or_non_dict_returns_none(self): + assert translate_partial_filter_expression({}) is None + assert translate_partial_filter_expression(None) is None # type: ignore[arg-type] diff --git a/tests/storage/test_security.py b/tests/storage/test_security.py index 80c2eb2..e02ca7e 100644 --- a/tests/storage/test_security.py +++ b/tests/storage/test_security.py @@ -943,3 +943,76 @@ async def test_combined_usage_example(self): assert safe_path == upload_path assert result["valid"] is True + + +class TestHintMime: + """Tests for the hint_mime parameter in detect_mime_type and validate_file.""" + + def test_detect_mime_type_hint_used_when_extension_missing(self): + """hint_mime is used when neither magic nor extension can detect the type.""" + validator = FileValidator() + content = b"\x00\x01\x02\x03unknown_binary" + mime = validator.detect_mime_type( + content, filename="0_upload", hint_mime="image/jpeg" + ) + assert mime == "image/jpeg" + + def test_detect_mime_type_hint_ignored_when_extension_works(self): + """hint_mime is not used when extension-based detection succeeds.""" + validator = FileValidator() + content = b"plain text" + mime = validator.detect_mime_type( + content, filename="photo.jpg", hint_mime="image/png" + ) + assert mime == "image/jpeg" + + def test_detect_mime_type_octet_stream_hint_not_used(self): + """hint_mime of application/octet-stream is ignored (not useful).""" + validator = FileValidator() + content = b"\x00\x01binary" + mime = validator.detect_mime_type( + content, filename="upload", hint_mime="application/octet-stream" + ) + assert mime == "application/octet-stream" + + def test_detect_mime_type_none_hint(self): + """hint_mime=None is same as not providing it.""" + validator = FileValidator() + content = b"\x00\x01binary" + mime = validator.detect_mime_type(content, filename="upload", hint_mime=None) + assert mime == "application/octet-stream" + + def test_validate_file_with_hint_mime(self): + """validate_file accepts hint_mime and uses it when detection fails.""" + validator = FileValidator() + content = b"\x00\x01\x02\x03jpeg_binary" + result = validator.validate_file( + content, filename="0_upload", hint_mime="image/jpeg" + ) + assert result["valid"] is True + assert result["mime_type"] == "image/jpeg" + + def test_validate_file_without_hint_mime_extensionless_fails(self): + """validate_file rejects extensionless files when no hint is provided.""" + validator = FileValidator() + content = b"\x00\x01\x02\x03unknown" + with pytest.raises(InvalidMimeTypeError): + validator.validate_file(content, filename="0_upload") + + def test_validate_file_hint_mime_blocked_type_still_rejected(self): + """hint_mime doesn't bypass blocked MIME types.""" + validator = FileValidator() + content = b"#!/bin/bash\necho test" + with pytest.raises(InvalidMimeTypeError): + validator.validate_file( + content, filename="upload", hint_mime="application/x-executable" + ) + + def test_validate_file_hint_mime_not_in_allowlist_rejected(self): + """hint_mime pointing to a type not in the allowlist is still rejected.""" + validator = FileValidator() + content = b"\x00\x01binary" + with pytest.raises(InvalidMimeTypeError): + validator.validate_file( + content, filename="upload", hint_mime="application/x-custom-unknown" + ) From 6016add75a80c5bfbbb6872c3ed3a3140acd9315 Mon Sep 17 00:00:00 2001 From: Jason Barnwell Date: Fri, 31 Jul 2026 15:51:13 -0400 Subject: [PATCH 2/2] Version bump --- jvspatial/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jvspatial/version.py b/jvspatial/version.py index 5bf7464..80864ec 100644 --- a/jvspatial/version.py +++ b/jvspatial/version.py @@ -9,4 +9,4 @@ # - MAJOR: Breaking changes # - MINOR: New features, backward compatible # - PATCH: Bug fixes, backward compatible -__version__ = "0.0.13" +__version__ = "0.0.14"