diff --git a/tests/test_blind_compare_redaction.py b/tests/test_blind_compare_redaction.py index 127df00..10e0d98 100644 --- a/tests/test_blind_compare_redaction.py +++ b/tests/test_blind_compare_redaction.py @@ -29,16 +29,61 @@ _REPO = Path(__file__).resolve().parent.parent # caches routes.session_routes after the first import, so stubbing auth_helpers / # session_manager here would poison the shared module for the sibling session # tests (whichever file is collected first wins). Matching their stub set keeps -# the cached module identical regardless of collection order. +# the cached module identical regardless of collection order. We restore both +# sys.modules AND the parent `routes` package attribute so the stub-bound module +# never leaks into sibling modules via `import routes.session_routes as X`. _ABSENT = object() -_TEMP_STUBS = ("core.database", "core.models", "src.request_models") + + +def _save_module_and_parent_attr(dotted_name): + """Capture a module's sys.modules entry *and* its parent-package attribute. + + Importing ``routes.session_routes`` also sets ``session_routes`` on the + parent ``routes`` package object, and ``import routes.session_routes as X`` + resolves ``X`` through that parent attribute — so restoring sys.modules + alone leaves the stale stub-bound module reachable. Returns a (module, attr) + pair to hand back to _restore_module_and_parent_attr. + """ + saved_module = sys.modules.get(dotted_name, _ABSENT) + pkg_name, _, attr = dotted_name.rpartition(".") + pkg = sys.modules.get(pkg_name) + saved_attr = getattr(pkg, attr, _ABSENT) if pkg is not None else _ABSENT + return saved_module, saved_attr + + +def _restore_module_and_parent_attr(dotted_name, saved_module, saved_attr): + """Restore (or remove) both the sys.modules entry and the parent attribute. + + Passing _ABSENT for both clears the cache, which is how we drop any stale + entry before the stubbed import. + """ + if saved_module is _ABSENT: + sys.modules.pop(dotted_name, None) + else: + sys.modules[dotted_name] = saved_module + pkg_name, _, attr = dotted_name.rpartition(".") + pkg = sys.modules.get(pkg_name) + if pkg is None: + return + if saved_attr is _ABSENT: + if hasattr(pkg, attr): + delattr(pkg, attr) + else: + setattr(pkg, attr, saved_attr) + + +_TEMP_STUBS = ("core.database", "core.models") _saved = {name: sys.modules.get(name, _ABSENT) for name in _TEMP_STUBS} _saved["core.session_manager"] = sys.modules.get("core.session_manager", _ABSENT) +_sr_saved = _save_module_and_parent_attr("routes.session_routes") try: for _name in _TEMP_STUBS: sys.modules[_name] = MagicMock(name=_name) if isinstance(sys.modules.get("core.session_manager"), MagicMock): del sys.modules["core.session_manager"] + # Clear the sys.modules entry AND the parent `routes` attribute so the + # stubbed import below produces a fresh module with no stale binding behind it. + _restore_module_and_parent_attr("routes.session_routes", _ABSENT, _ABSENT) importlib.import_module("core.session_manager") import routes.session_routes as SR # noqa: E402 finally: @@ -47,6 +92,7 @@ finally: sys.modules.pop(_name, None) else: sys.modules[_name] = _val + _restore_module_and_parent_attr("routes.session_routes", *_sr_saved) # ── backend: GET /api/sessions model redaction ───────────────────────────── diff --git a/tests/test_security_regressions.py b/tests/test_security_regressions.py index 8e30986..2ca468f 100644 --- a/tests/test_security_regressions.py +++ b/tests/test_security_regressions.py @@ -1015,50 +1015,37 @@ def test_gmail_mcp_preset_uses_contained_oauth_paths(): # -- export/gallery filename hardening ---------------------------------------- -def _install_route_import_stubs(monkeypatch): - core_mod = types.ModuleType("core") - core_mod.__path__ = [] - - db_mod = types.ModuleType("core.database") - db_mod.SessionLocal = lambda: None - for name in ( - "Session", - "Document", - "GalleryImage", - "GalleryAlbum", - "ModelEndpoint", - ): - setattr(db_mod, name, type(name, (), {})) - - session_manager_mod = types.ModuleType("core.session_manager") - session_manager_mod.SessionManager = type("SessionManager", (), {}) - - models_mod = types.ModuleType("core.models") - models_mod.ChatMessage = type("ChatMessage", (), {}) - - monkeypatch.setitem(sys.modules, "core", core_mod) - monkeypatch.setitem(sys.modules, "core.database", db_mod) - monkeypatch.setitem(sys.modules, "core.session_manager", session_manager_mod) - monkeypatch.setitem(sys.modules, "core.models", models_mod) +def _drop_route_module_cache(dotted_name): + """Evict a cached route module from both sys.modules and the parent package + attribute. The next import then re-binds against the live core.database + instead of reusing a stale (possibly stub-polluted) module object — Python + can reach a module via either path, so both must be cleared.""" + sys.modules.pop(dotted_name, None) + pkg_name, _, attr = dotted_name.rpartition(".") + pkg = sys.modules.get(pkg_name) + if pkg is not None and hasattr(pkg, attr): + delattr(pkg, attr) -def _import_session_routes_for_filename(monkeypatch): - _install_route_import_stubs(monkeypatch) - monkeypatch.delitem(sys.modules, "routes.session_routes", raising=False) - from routes import session_routes - return session_routes +def _import_session_routes_for_filename(): + # Only the pure _sanitize_export_filename helper is exercised here, so import + # against the REAL core.database. Importing under a stub Session class would + # leak a stub-bound DbSession into the cached module and break later tests + # that reuse routes.session_routes (e.g. the archived-sessions filter). + _drop_route_module_cache("routes.session_routes") + return importlib.import_module("routes.session_routes") -def _import_gallery_routes_for_filename(monkeypatch): - _install_route_import_stubs(monkeypatch) - monkeypatch.delitem(sys.modules, "routes.gallery_helpers", raising=False) - monkeypatch.delitem(sys.modules, "routes.gallery_routes", raising=False) - from routes import gallery_routes - return gallery_routes +def _import_gallery_routes_for_filename(): + # Same rationale as the session route helper: import _sanitize_gallery_filename + # against the real core.database and leave a clean, real module cached. + _drop_route_module_cache("routes.gallery_routes") + _drop_route_module_cache("routes.gallery_helpers") + return importlib.import_module("routes.gallery_routes") -def test_export_filename_sanitizer_blocks_header_and_path_chars(monkeypatch): - mod = _import_session_routes_for_filename(monkeypatch) +def test_export_filename_sanitizer_blocks_header_and_path_chars(): + mod = _import_session_routes_for_filename() out = mod._sanitize_export_filename('chat.md\r\nX-Test: yes/..\\evil;quote".txt\x00') @@ -1068,15 +1055,15 @@ def test_export_filename_sanitizer_blocks_header_and_path_chars(monkeypatch): assert ch not in out -def test_export_filename_sanitizer_preserves_safe_names(monkeypatch): - mod = _import_session_routes_for_filename(monkeypatch) +def test_export_filename_sanitizer_preserves_safe_names(): + mod = _import_session_routes_for_filename() assert mod._sanitize_export_filename("conversation_20260602.md") == "conversation_20260602.md" assert mod._sanitize_export_filename("") == "" -def test_gallery_replace_filename_sanitizer_uses_basename(monkeypatch): - mod = _import_gallery_routes_for_filename(monkeypatch) +def test_gallery_replace_filename_sanitizer_uses_basename(): + mod = _import_gallery_routes_for_filename() out = mod._sanitize_gallery_filename("../../etc/cron.d/evil image.png") @@ -1086,7 +1073,7 @@ def test_gallery_replace_filename_sanitizer_uses_basename(monkeypatch): def test_gallery_replace_filename_sanitizer_falls_back_when_empty(monkeypatch): - mod = _import_gallery_routes_for_filename(monkeypatch) + mod = _import_gallery_routes_for_filename() monkeypatch.setattr(mod.uuid, "uuid4", lambda: types.SimpleNamespace(hex="abcdef1234567890")) assert mod._sanitize_gallery_filename("../") == "abcdef123456" diff --git a/tests/test_session_ghost_delete.py b/tests/test_session_ghost_delete.py index dc6a4c9..bba12fa 100644 --- a/tests/test_session_ghost_delete.py +++ b/tests/test_session_ghost_delete.py @@ -27,17 +27,61 @@ import pytest # MagicMock sqlalchemy stub. The real core.database defines declarative classes # that blow up under that stub, so temporarily swap in MagicMock module objects # (auto-creating attributes satisfy any `from core.database import X`). Crucially -# we RESTORE sys.modules immediately after import so these stubs never leak into -# sibling test modules — the imported SM/SR objects keep their captured bindings. +# we RESTORE both sys.modules AND the parent `routes` package attribute after +# import, so these stubs never leak into sibling modules — the local SM/SR +# bindings keep their captured stub modules for this file's own assertions. _ABSENT = object() -_TEMP_STUBS = ("core.database", "core.models", "src.request_models") + + +def _save_module_and_parent_attr(dotted_name): + """Capture a module's sys.modules entry *and* its parent-package attribute. + + Importing ``routes.session_routes`` also sets ``session_routes`` on the + parent ``routes`` package object, and ``import routes.session_routes as X`` + resolves ``X`` through that parent attribute — so restoring sys.modules + alone leaves the stale stub-bound module reachable. Returns a (module, attr) + pair to hand back to _restore_module_and_parent_attr. + """ + saved_module = sys.modules.get(dotted_name, _ABSENT) + pkg_name, _, attr = dotted_name.rpartition(".") + pkg = sys.modules.get(pkg_name) + saved_attr = getattr(pkg, attr, _ABSENT) if pkg is not None else _ABSENT + return saved_module, saved_attr + + +def _restore_module_and_parent_attr(dotted_name, saved_module, saved_attr): + """Restore (or remove) both the sys.modules entry and the parent attribute. + + Passing _ABSENT for both clears the cache, which is how we drop any stale + entry before the stubbed import. + """ + if saved_module is _ABSENT: + sys.modules.pop(dotted_name, None) + else: + sys.modules[dotted_name] = saved_module + pkg_name, _, attr = dotted_name.rpartition(".") + pkg = sys.modules.get(pkg_name) + if pkg is None: + return + if saved_attr is _ABSENT: + if hasattr(pkg, attr): + delattr(pkg, attr) + else: + setattr(pkg, attr, saved_attr) + + +_TEMP_STUBS = ("core.database", "core.models") _saved = {name: sys.modules.get(name, _ABSENT) for name in _TEMP_STUBS} _saved["core.session_manager"] = sys.modules.get("core.session_manager", _ABSENT) +_sr_saved = _save_module_and_parent_attr("routes.session_routes") try: for _name in _TEMP_STUBS: sys.modules[_name] = MagicMock(name=_name) if isinstance(sys.modules.get("core.session_manager"), MagicMock): del sys.modules["core.session_manager"] + # Clear the sys.modules entry AND the parent `routes` attribute so the + # stubbed import below produces a fresh module with no stale binding behind it. + _restore_module_and_parent_attr("routes.session_routes", _ABSENT, _ABSENT) SM = importlib.import_module("core.session_manager") import routes.session_routes as SR # noqa: E402 finally: @@ -46,6 +90,7 @@ finally: sys.modules.pop(_name, None) else: sys.modules[_name] = _val + _restore_module_and_parent_attr("routes.session_routes", *_sr_saved) from fastapi import HTTPException # noqa: E402 diff --git a/tests/test_session_owner_attribution.py b/tests/test_session_owner_attribution.py index 504634c..cae5983 100644 --- a/tests/test_session_owner_attribution.py +++ b/tests/test_session_owner_attribution.py @@ -10,7 +10,7 @@ Follows the direct-helper + mocked-DB style of tests/test_null_owner_gates.py. import os import sys -import types +import importlib from types import SimpleNamespace from unittest.mock import MagicMock @@ -18,27 +18,73 @@ import pytest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -# routes.session_routes imports several heavy modules at import time that blow up -# under conftest's sqlalchemy/* MagicMock stubs (declarative classes). Stub them -# so we can import the module and exercise _verify_session_owner with a mock DB. -_STUBS = { - "core.database": {"Session": MagicMock(), "SessionLocal": MagicMock(), - "Document": MagicMock(), "GalleryImage": MagicMock()}, - "core.session_manager": {"SessionManager": MagicMock()}, - "core.models": {"ChatMessage": MagicMock()}, - "src.request_models": {"SessionResponse": MagicMock()}, -} -for _name, _attrs in _STUBS.items(): - if _name not in sys.modules: - _m = types.ModuleType(_name) - for _k, _v in _attrs.items(): - setattr(_m, _k, _v) - sys.modules[_name] = _m +# Stub heavy ORM modules so routes.session_routes can be imported under +# conftest's MagicMock sqlalchemy shim. Both the stubs and the cached route +# module — including the parent `routes` package attribute — are restored in the +# finally block to prevent poisoning later tests via `import routes.session_routes`. +_ABSENT = object() + + +def _save_module_and_parent_attr(dotted_name): + """Capture a module's sys.modules entry *and* its parent-package attribute. + + Importing ``routes.session_routes`` also sets ``session_routes`` on the + parent ``routes`` package object, and ``import routes.session_routes as X`` + resolves ``X`` through that parent attribute — so restoring sys.modules + alone leaves the stale stub-bound module reachable. Returns a (module, attr) + pair to hand back to _restore_module_and_parent_attr. + """ + saved_module = sys.modules.get(dotted_name, _ABSENT) + pkg_name, _, attr = dotted_name.rpartition(".") + pkg = sys.modules.get(pkg_name) + saved_attr = getattr(pkg, attr, _ABSENT) if pkg is not None else _ABSENT + return saved_module, saved_attr + + +def _restore_module_and_parent_attr(dotted_name, saved_module, saved_attr): + """Restore (or remove) both the sys.modules entry and the parent attribute. + + Passing _ABSENT for both clears the cache, which is how we drop any stale + entry before the stubbed import. + """ + if saved_module is _ABSENT: + sys.modules.pop(dotted_name, None) + else: + sys.modules[dotted_name] = saved_module + pkg_name, _, attr = dotted_name.rpartition(".") + pkg = sys.modules.get(pkg_name) + if pkg is None: + return + if saved_attr is _ABSENT: + if hasattr(pkg, attr): + delattr(pkg, attr) + else: + setattr(pkg, attr, saved_attr) + + +_TEMP_STUBS = ("core.database", "core.models") +_saved = {name: sys.modules.get(name, _ABSENT) for name in _TEMP_STUBS} +_saved["core.session_manager"] = sys.modules.get("core.session_manager", _ABSENT) +_sr_saved = _save_module_and_parent_attr("routes.session_routes") +try: + for _name in _TEMP_STUBS: + sys.modules[_name] = MagicMock(name=_name) + sys.modules.pop("core.session_manager", None) + # Clear the sys.modules entry AND the parent `routes` attribute so the + # stubbed import below produces a fresh module with no stale binding behind it. + _restore_module_and_parent_attr("routes.session_routes", _ABSENT, _ABSENT) + importlib.import_module("core.session_manager") + import routes.session_routes as SR # noqa: E402 +finally: + for _name, _val in _saved.items(): + if _val is _ABSENT: + sys.modules.pop(_name, None) + else: + sys.modules[_name] = _val + _restore_module_and_parent_attr("routes.session_routes", *_sr_saved) from fastapi import HTTPException # noqa: E402 - from src.auth_helpers import effective_user # noqa: E402 -import routes.session_routes as SR # noqa: E402 def _req(**state):