From c13584aa0358b6ae9938a868c18ac31a90719f98 Mon Sep 17 00:00:00 2001 From: jwvanderstam Date: Sun, 8 Nov 2026 22:53:05 +0100 Subject: [PATCH 1/2] fix(security): retrieval no longer consults the scope-blind query cache QueryCache's key is (query, top_k, min_similarity, use_hybrid): no workspace, knowledge tier or document filter. retrieve_context looked it up first and stored into it last, on a path that was never reached only because _get_app_cache() returns None. Re-enabling it would have served one workspace's results to another. The dead read and write are removed, and QueryCache's docstring says it must never hold a scoped retrieval's results. Found by the separate review of #527. The regression test hands retrieval a cache answering with another workspace's chunk and fails on the old code. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KkWPqAcWbxfqz1aCP6Qsdx --- CHANGELOG.md | 5 ++++ src/cache/managers.py | 7 +++++- src/rag/retrieval.py | 23 ------------------- tests/unit/test_global_knowledge_retrieval.py | 9 ++++++++ 4 files changed, 20 insertions(+), 24 deletions(-) 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..07429772 100644 --- a/tests/unit/test_global_knowledge_retrieval.py +++ b/tests/unit/test_global_knowledge_retrieval.py @@ -165,3 +165,12 @@ 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() + foreign.get.return_value = ["another workspace's chunk"] + with patch.object(Retriever, "_get_app_cache", return_value=foreign): + results = retriever.retrieve_context("migration lessons", scope=WORKSPACE) + assert _ids(results) == {1} From 429780d7f928597c2fe2f578b3e442e783ff697d Mon Sep 17 00:00:00 2001 From: jwvanderstam Date: Mon, 9 Nov 2026 09:00:46 +0100 Subject: [PATCH 2/2] test: the query-cache regression misses on the embedding cache and checks the search's scope Review Low-1: the mock answered the embedding lookup too, so the search ran on a string as its embedding. It now answers only the query cache's four-argument lookup, and the test asserts that every search ran in the caller's workspace. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KkWPqAcWbxfqz1aCP6Qsdx --- tests/unit/test_global_knowledge_retrieval.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_global_knowledge_retrieval.py b/tests/unit/test_global_knowledge_retrieval.py index 07429772..3eccf669 100644 --- a/tests/unit/test_global_knowledge_retrieval.py +++ b/tests/unit/test_global_knowledge_retrieval.py @@ -170,7 +170,8 @@ def test_the_workspace_tier_is_unchanged(method): 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() - foreign.get.return_value = ["another workspace's chunk"] + # 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) == {1} + assert (_ids(results), {scope for _, scope, _ in retriever._db.calls}) == ({1}, {WORKSPACE})