fix(tests): isolate session route import stubs
Keeps src.request_models real and restores both sys.modules and parent routes.session_routes package attributes after temporary test stubs. Restores one focused part of the Python CI baseline tracked in #2580.
This commit is contained in:
committed by
GitHub
parent
e69298888b
commit
3426e0cb5e
@@ -29,16 +29,61 @@ _REPO = Path(__file__).resolve().parent.parent
|
|||||||
# caches routes.session_routes after the first import, so stubbing auth_helpers /
|
# caches routes.session_routes after the first import, so stubbing auth_helpers /
|
||||||
# session_manager here would poison the shared module for the sibling session
|
# session_manager here would poison the shared module for the sibling session
|
||||||
# tests (whichever file is collected first wins). Matching their stub set keeps
|
# 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()
|
_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 = {name: sys.modules.get(name, _ABSENT) for name in _TEMP_STUBS}
|
||||||
_saved["core.session_manager"] = sys.modules.get("core.session_manager", _ABSENT)
|
_saved["core.session_manager"] = sys.modules.get("core.session_manager", _ABSENT)
|
||||||
|
_sr_saved = _save_module_and_parent_attr("routes.session_routes")
|
||||||
try:
|
try:
|
||||||
for _name in _TEMP_STUBS:
|
for _name in _TEMP_STUBS:
|
||||||
sys.modules[_name] = MagicMock(name=_name)
|
sys.modules[_name] = MagicMock(name=_name)
|
||||||
if isinstance(sys.modules.get("core.session_manager"), MagicMock):
|
if isinstance(sys.modules.get("core.session_manager"), MagicMock):
|
||||||
del sys.modules["core.session_manager"]
|
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")
|
importlib.import_module("core.session_manager")
|
||||||
import routes.session_routes as SR # noqa: E402
|
import routes.session_routes as SR # noqa: E402
|
||||||
finally:
|
finally:
|
||||||
@@ -47,6 +92,7 @@ finally:
|
|||||||
sys.modules.pop(_name, None)
|
sys.modules.pop(_name, None)
|
||||||
else:
|
else:
|
||||||
sys.modules[_name] = _val
|
sys.modules[_name] = _val
|
||||||
|
_restore_module_and_parent_attr("routes.session_routes", *_sr_saved)
|
||||||
|
|
||||||
|
|
||||||
# ── backend: GET /api/sessions model redaction ─────────────────────────────
|
# ── backend: GET /api/sessions model redaction ─────────────────────────────
|
||||||
|
|||||||
@@ -1015,50 +1015,37 @@ def test_gmail_mcp_preset_uses_contained_oauth_paths():
|
|||||||
|
|
||||||
# -- export/gallery filename hardening ----------------------------------------
|
# -- export/gallery filename hardening ----------------------------------------
|
||||||
|
|
||||||
def _install_route_import_stubs(monkeypatch):
|
def _drop_route_module_cache(dotted_name):
|
||||||
core_mod = types.ModuleType("core")
|
"""Evict a cached route module from both sys.modules and the parent package
|
||||||
core_mod.__path__ = []
|
attribute. The next import then re-binds against the live core.database
|
||||||
|
instead of reusing a stale (possibly stub-polluted) module object — Python
|
||||||
db_mod = types.ModuleType("core.database")
|
can reach a module via either path, so both must be cleared."""
|
||||||
db_mod.SessionLocal = lambda: None
|
sys.modules.pop(dotted_name, None)
|
||||||
for name in (
|
pkg_name, _, attr = dotted_name.rpartition(".")
|
||||||
"Session",
|
pkg = sys.modules.get(pkg_name)
|
||||||
"Document",
|
if pkg is not None and hasattr(pkg, attr):
|
||||||
"GalleryImage",
|
delattr(pkg, attr)
|
||||||
"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 _import_session_routes_for_filename(monkeypatch):
|
def _import_session_routes_for_filename():
|
||||||
_install_route_import_stubs(monkeypatch)
|
# Only the pure _sanitize_export_filename helper is exercised here, so import
|
||||||
monkeypatch.delitem(sys.modules, "routes.session_routes", raising=False)
|
# against the REAL core.database. Importing under a stub Session class would
|
||||||
from routes import session_routes
|
# leak a stub-bound DbSession into the cached module and break later tests
|
||||||
return session_routes
|
# 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):
|
def _import_gallery_routes_for_filename():
|
||||||
_install_route_import_stubs(monkeypatch)
|
# Same rationale as the session route helper: import _sanitize_gallery_filename
|
||||||
monkeypatch.delitem(sys.modules, "routes.gallery_helpers", raising=False)
|
# against the real core.database and leave a clean, real module cached.
|
||||||
monkeypatch.delitem(sys.modules, "routes.gallery_routes", raising=False)
|
_drop_route_module_cache("routes.gallery_routes")
|
||||||
from routes import gallery_routes
|
_drop_route_module_cache("routes.gallery_helpers")
|
||||||
return gallery_routes
|
return importlib.import_module("routes.gallery_routes")
|
||||||
|
|
||||||
|
|
||||||
def test_export_filename_sanitizer_blocks_header_and_path_chars(monkeypatch):
|
def test_export_filename_sanitizer_blocks_header_and_path_chars():
|
||||||
mod = _import_session_routes_for_filename(monkeypatch)
|
mod = _import_session_routes_for_filename()
|
||||||
|
|
||||||
out = mod._sanitize_export_filename('chat.md\r\nX-Test: yes/..\\evil;quote".txt\x00')
|
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
|
assert ch not in out
|
||||||
|
|
||||||
|
|
||||||
def test_export_filename_sanitizer_preserves_safe_names(monkeypatch):
|
def test_export_filename_sanitizer_preserves_safe_names():
|
||||||
mod = _import_session_routes_for_filename(monkeypatch)
|
mod = _import_session_routes_for_filename()
|
||||||
|
|
||||||
assert mod._sanitize_export_filename("conversation_20260602.md") == "conversation_20260602.md"
|
assert mod._sanitize_export_filename("conversation_20260602.md") == "conversation_20260602.md"
|
||||||
assert mod._sanitize_export_filename("") == ""
|
assert mod._sanitize_export_filename("") == ""
|
||||||
|
|
||||||
|
|
||||||
def test_gallery_replace_filename_sanitizer_uses_basename(monkeypatch):
|
def test_gallery_replace_filename_sanitizer_uses_basename():
|
||||||
mod = _import_gallery_routes_for_filename(monkeypatch)
|
mod = _import_gallery_routes_for_filename()
|
||||||
|
|
||||||
out = mod._sanitize_gallery_filename("../../etc/cron.d/evil image.png")
|
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):
|
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"))
|
monkeypatch.setattr(mod.uuid, "uuid4", lambda: types.SimpleNamespace(hex="abcdef1234567890"))
|
||||||
|
|
||||||
assert mod._sanitize_gallery_filename("../") == "abcdef123456"
|
assert mod._sanitize_gallery_filename("../") == "abcdef123456"
|
||||||
|
|||||||
@@ -27,17 +27,61 @@ import pytest
|
|||||||
# MagicMock sqlalchemy stub. The real core.database defines declarative classes
|
# MagicMock sqlalchemy stub. The real core.database defines declarative classes
|
||||||
# that blow up under that stub, so temporarily swap in MagicMock module objects
|
# that blow up under that stub, so temporarily swap in MagicMock module objects
|
||||||
# (auto-creating attributes satisfy any `from core.database import X`). Crucially
|
# (auto-creating attributes satisfy any `from core.database import X`). Crucially
|
||||||
# we RESTORE sys.modules immediately after import so these stubs never leak into
|
# we RESTORE both sys.modules AND the parent `routes` package attribute after
|
||||||
# sibling test modules — the imported SM/SR objects keep their captured bindings.
|
# 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()
|
_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 = {name: sys.modules.get(name, _ABSENT) for name in _TEMP_STUBS}
|
||||||
_saved["core.session_manager"] = sys.modules.get("core.session_manager", _ABSENT)
|
_saved["core.session_manager"] = sys.modules.get("core.session_manager", _ABSENT)
|
||||||
|
_sr_saved = _save_module_and_parent_attr("routes.session_routes")
|
||||||
try:
|
try:
|
||||||
for _name in _TEMP_STUBS:
|
for _name in _TEMP_STUBS:
|
||||||
sys.modules[_name] = MagicMock(name=_name)
|
sys.modules[_name] = MagicMock(name=_name)
|
||||||
if isinstance(sys.modules.get("core.session_manager"), MagicMock):
|
if isinstance(sys.modules.get("core.session_manager"), MagicMock):
|
||||||
del sys.modules["core.session_manager"]
|
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")
|
SM = importlib.import_module("core.session_manager")
|
||||||
import routes.session_routes as SR # noqa: E402
|
import routes.session_routes as SR # noqa: E402
|
||||||
finally:
|
finally:
|
||||||
@@ -46,6 +90,7 @@ finally:
|
|||||||
sys.modules.pop(_name, None)
|
sys.modules.pop(_name, None)
|
||||||
else:
|
else:
|
||||||
sys.modules[_name] = _val
|
sys.modules[_name] = _val
|
||||||
|
_restore_module_and_parent_attr("routes.session_routes", *_sr_saved)
|
||||||
|
|
||||||
from fastapi import HTTPException # noqa: E402
|
from fastapi import HTTPException # noqa: E402
|
||||||
|
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ Follows the direct-helper + mocked-DB style of tests/test_null_owner_gates.py.
|
|||||||
|
|
||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
import types
|
import importlib
|
||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
from unittest.mock import MagicMock
|
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__))))
|
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
|
# Stub heavy ORM modules so routes.session_routes can be imported under
|
||||||
# under conftest's sqlalchemy/* MagicMock stubs (declarative classes). Stub them
|
# conftest's MagicMock sqlalchemy shim. Both the stubs and the cached route
|
||||||
# so we can import the module and exercise _verify_session_owner with a mock DB.
|
# module — including the parent `routes` package attribute — are restored in the
|
||||||
_STUBS = {
|
# finally block to prevent poisoning later tests via `import routes.session_routes`.
|
||||||
"core.database": {"Session": MagicMock(), "SessionLocal": MagicMock(),
|
_ABSENT = object()
|
||||||
"Document": MagicMock(), "GalleryImage": MagicMock()},
|
|
||||||
"core.session_manager": {"SessionManager": MagicMock()},
|
|
||||||
"core.models": {"ChatMessage": MagicMock()},
|
def _save_module_and_parent_attr(dotted_name):
|
||||||
"src.request_models": {"SessionResponse": MagicMock()},
|
"""Capture a module's sys.modules entry *and* its parent-package attribute.
|
||||||
}
|
|
||||||
for _name, _attrs in _STUBS.items():
|
Importing ``routes.session_routes`` also sets ``session_routes`` on the
|
||||||
if _name not in sys.modules:
|
parent ``routes`` package object, and ``import routes.session_routes as X``
|
||||||
_m = types.ModuleType(_name)
|
resolves ``X`` through that parent attribute — so restoring sys.modules
|
||||||
for _k, _v in _attrs.items():
|
alone leaves the stale stub-bound module reachable. Returns a (module, attr)
|
||||||
setattr(_m, _k, _v)
|
pair to hand back to _restore_module_and_parent_attr.
|
||||||
sys.modules[_name] = _m
|
"""
|
||||||
|
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 fastapi import HTTPException # noqa: E402
|
||||||
|
|
||||||
from src.auth_helpers import effective_user # noqa: E402
|
from src.auth_helpers import effective_user # noqa: E402
|
||||||
import routes.session_routes as SR # noqa: E402
|
|
||||||
|
|
||||||
|
|
||||||
def _req(**state):
|
def _req(**state):
|
||||||
|
|||||||
Reference in New Issue
Block a user