diff --git a/rag_engine.py b/rag_engine.py index 8a50283..c8b2e21 100644 --- a/rag_engine.py +++ b/rag_engine.py @@ -394,7 +394,7 @@ def close_server_mode(): client.close() logger.info("Closed Milvus client: %s", path) except Exception as e: - logger.warning("Error closing Milvus client %s: %s", path, e) + logger.warning("Error closing Milvus client %s: %s", path, _scrub_exception(e)) _persistent_clients.clear() _fts.close_all() # Nil the semaphore and lock FIRST so any coroutine that wakes up while we @@ -416,7 +416,7 @@ def _get_persistent_client(db_path: str) -> MilvusClient: _persistent_clients[db_path].has_collection(COLLECTION_NAME) return _persistent_clients[db_path] except Exception as e: - logger.warning("Stale Milvus client for %s: %s — reconnecting", db_path, e) + logger.warning("Stale Milvus client for %s: %s — reconnecting", db_path, _scrub_exception(e)) try: _persistent_clients[db_path].close() except Exception: @@ -441,7 +441,7 @@ def _get_persistent_client(db_path: str) -> MilvusClient: ) logger.info("Opened client: %s", db_path) except Exception as e: - logger.error("Failed to connect to Milvus at %s: %s", db_path, e) + logger.error("Failed to connect to Milvus at %s: %s", db_path, _scrub_exception(e)) raise return _persistent_clients[db_path] @@ -834,7 +834,7 @@ def add_turns(turns: List[Dict], db_path: Optional[str] = None) -> int: if results: existing_ids.add(doc_id) except Exception as e: - logger.warning("Dedup check failed for doc_id %s: %s", doc_id, e) + logger.warning("Dedup check failed for doc_id %s: %s", doc_id, _scrub_exception(e)) new_turns = [t for t in turns if t["doc_id"] not in existing_ids] if not new_turns: @@ -1537,7 +1537,7 @@ def _passes(entry: Dict) -> bool: filters=fts_filters or None, db_path=db_path, ) except Exception as e: - logger.warning("FTS fallback failed for issue timeline %s (non-fatal): %s", canonical, e) + logger.warning("FTS fallback failed for issue timeline %s (non-fatal): %s", canonical, _scrub_exception(e)) fts_hits = [] for row in fts_hits: @@ -1951,7 +1951,7 @@ def delete_by_session(session_id: str, db_path: Optional[str] = None) -> int: _fts.delete(conn, "session_id", session_id) _fts.close_ephemeral(conn) except Exception as e: - logger.warning("FTS delete by session failed (non-fatal): %s", e) + logger.warning("FTS delete by session failed (non-fatal): %s", _scrub_exception(e)) return len(results) @@ -2217,7 +2217,7 @@ def delete_by_branch(git_branch: str, db_path: Optional[str] = None) -> int: _fts.delete(conn, "git_branch", git_branch) _fts.close_ephemeral(conn) except Exception as e: - logger.warning("FTS delete by branch failed (non-fatal): %s", e) + logger.warning("FTS delete by branch failed (non-fatal): %s", _scrub_exception(e)) return len(results) @@ -2256,7 +2256,7 @@ def delete_older_than(max_age_days: int, db_path: Optional[str] = None) -> int: _fts.delete_where(conn, "timestamp < ? AND timestamp != ''", (cutoff_str,)) _fts.close_ephemeral(conn) except Exception as e: - logger.warning("FTS delete older_than failed (non-fatal): %s", e) + logger.warning("FTS delete older_than failed (non-fatal): %s", _scrub_exception(e)) return len(results) diff --git a/tests/test_secret_redaction.py b/tests/test_secret_redaction.py index 64bd324..ce4d313 100644 --- a/tests/test_secret_redaction.py +++ b/tests/test_secret_redaction.py @@ -545,6 +545,37 @@ def boom(conn, records): assert AWS_KEY not in logged # FTS failure is non-fatal AND scrubbed +@pytest.mark.parametrize( + "invoke", + [ + lambda rag: rag.delete_by_session("sess-1", db_path=DB), + lambda rag: rag.delete_by_branch("feature/x", db_path=DB), + lambda rag: rag.delete_older_than(30, db_path=DB), + ], + ids=["by_session", "by_branch", "older_than"], +) +def test_ac17_fts_delete_error_is_scrubbed(add_turns_harness, monkeypatch, caplog, invoke): + """AC-17: a secret in any FTS delete failure message is scrubbed from the log. + + Covers the three cleanup entry points (``delete_by_session`` / ``delete_by_branch`` + use ``_fts.delete``; ``delete_older_than`` uses ``_fts.delete_where``). A SQLite + error string can echo bound parameters or row content, so the non-fatal handler + must route ``e`` through ``_scrub_exception`` rather than logging it raw. + """ + rag_engine, _ = add_turns_harness + + def boom(*args, **kwargs): + raise RuntimeError(f"delete failed near {AWS_KEY}") + + monkeypatch.setattr(rag_engine._fts, "delete", boom, raising=False) + monkeypatch.setattr(rag_engine._fts, "delete_where", boom, raising=False) + with caplog.at_level("WARNING"): + invoke(rag_engine) # non-fatal: the forced FTS error must not propagate + logged = " ".join(r.getMessage() for r in caplog.records) + assert AWS_KEY not in logged # delete failure is non-fatal AND scrubbed + assert "[REDACTED:AWS]" in logged # positive control: the scrub actually fired + + def test_ac17_embedding_error_is_scrubbed(add_turns_harness, monkeypatch): """AC-17: a secret in a raised embedding error is scrubbed before re-raise.""" rag_engine, _ = add_turns_harness