diff --git a/CHANGELOG.md b/CHANGELOG.md index 1840adf3..785461d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,11 @@ OpenAI, `https://api.openai.com/v1`) and drop any `provider/` prefix from `CLOUD - **`litellm` is gone from the image (QP-11, ADR-4).** It carried CVE-2026-114612, in a proxy server LocalChat never ran, and every fixed version pulls `boto3` into the image. The `pip-audit` suppression for it is removed; `SECURITY.md` item 12 is resolved. +- **Retrieval no longer consults the query cache.** Its key names no workspace, knowledge + tier or document filter, so a hit would have served one workspace's results to another. + The path was never reached (`_get_app_cache()` returns `None`), which is why it is + removed rather than fixed: re-enabling it would have reopened the leak. Found by the + separate review of #527. ### Changed diff --git a/src/cache/managers.py b/src/cache/managers.py index 941be46e..9c27d2db 100644 --- a/src/cache/managers.py +++ b/src/cache/managers.py @@ -37,7 +37,12 @@ def get_stats(self) -> CacheStats | dict[str, Any]: class QueryCache: - """Parameter-hashed cache for RAG retrieval results.""" + """Parameter-hashed cache for RAG retrieval results. + + The key names no workspace, knowledge tier or document filter, so it must never hold + a scoped retrieval's results: they would be served to another workspace. Retrieval + used to consult it on a path that was never reached, and no longer does. + """ def __init__( self, diff --git a/src/rag/retrieval.py b/src/rag/retrieval.py index e715c3b2..03dac4fb 100644 --- a/src/rag/retrieval.py +++ b/src/rag/retrieval.py @@ -194,19 +194,6 @@ def _log_similarity_miss( best = max(all_results.values(), key=lambda x: x['semantic_score']) logger.warning(f"[RAG] Best similarity was {best['semantic_score']:.3f}") - def _store_query_cache( - self, - cache: Any, - query_clean: str, - top_k: int, - min_similarity: float, - use_hybrid_search: bool, - output: list, - ) -> None: - """Populate the query cache when results are available.""" - if cache is not None and output: - cache.set(query_clean, top_k, min_similarity, use_hybrid_search, output) - def _run_retrieval_pipeline( self, query_clean: str, @@ -436,17 +423,8 @@ def retrieve_context( except Exception as graph_exc: # noqa: BLE001 — GraphRAG expansion is optional; the query proceeds unexpanded logger.debug(f"[GraphRAG] Expansion skipped: {graph_exc}") - _app_query_cache = self._get_app_cache() _app_emb_cache = self._get_app_cache() - cached_result = ( - _app_query_cache.get(query_clean, top_k, min_similarity, use_hybrid_search) - if _app_query_cache is not None else None - ) - if cached_result is not None: - logger.info("[RAG] Query cache hit") - return cached_result - embedding_model = self._ollama_client.get_embedding_model() if not embedding_model: logger.error("[RAG] No embedding model available") @@ -497,7 +475,6 @@ def retrieve_context( total_time = time.time() - start_time logger.info(f"[RAG] Retrieved {len(output)} chunks in {total_time:.3f}s") - self._store_query_cache(_app_query_cache, query_clean, top_k, min_similarity, use_hybrid_search, output) return output def _get_cached_embedding(self, text: str, model: str, app_cache=None) -> list[float] | None: diff --git a/tests/unit/test_global_knowledge_retrieval.py b/tests/unit/test_global_knowledge_retrieval.py index 4e26dced..3eccf669 100644 --- a/tests/unit/test_global_knowledge_retrieval.py +++ b/tests/unit/test_global_knowledge_retrieval.py @@ -165,3 +165,13 @@ def test_the_workspace_tier_is_unchanged(method): assert "d.workspace_id = %s" in sql assert GLOBAL_MARKER not in sql assert WORKSPACE in params + + +def test_no_cached_result_is_served_in_place_of_a_scoped_search(retriever): + """The query cache's key names no workspace; a hit from another one must never be returned.""" + foreign = MagicMock() + # The query cache's lookup took four arguments; the embedding cache's (text, model) misses. + foreign.get.side_effect = lambda *args: ["another workspace's chunk"] if len(args) == 4 else None + with patch.object(Retriever, "_get_app_cache", return_value=foreign): + results = retriever.retrieve_context("migration lessons", scope=WORKSPACE) + assert (_ids(results), {scope for _, scope, _ in retriever._db.calls}) == ({1}, {WORKSPACE})