From 91f8fcec6da60025e5ed9393389f72429f3e1fca Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Mon, 27 Apr 2026 12:26:15 -0700 Subject: [PATCH] fix OPDS FK constraint failure when session row is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit iOS Panels (and other Basic-Auth OPDS clients) intermittently hit sqlite3.IntegrityError: FOREIGN KEY constraint failed when settings or bookmarks were saved. Two interacting bugs caused it: - Janitor cleanup_sessions used `if not session.get_decoded():` to detect "corrupt" sessions. get_decoded() returns {} for both real decode failures and legitimate anonymous sessions with no stored data — exactly what Basic-Auth OPDS clients produce. The nightly task was wiping valid session rows. Replaced with a direct signing.loads() call so only genuine signature/decode failures are flagged. - _ensure_session_key returned the cookie's session_key without verifying the row still exists. With cached_db the session loads from cache without rechecking, so a stale cookie key would slip through and cause an FK violation when used as SettingsBrowser / SettingsReader.session_id. Now we verify existence and flush+save to cycle the key when the row is gone. Either fix alone closes the user-visible error; both together also stop the underlying churn that created the bad state. Co-Authored-By: Claude Opus 4.7 --- codex/librarian/scribe/janitor/cleanup.py | 20 +++++++++++++++++--- codex/views/settings.py | 19 +++++++++++++++---- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/codex/librarian/scribe/janitor/cleanup.py b/codex/librarian/scribe/janitor/cleanup.py index e83451626..bd0b808a3 100644 --- a/codex/librarian/scribe/janitor/cleanup.py +++ b/codex/librarian/scribe/janitor/cleanup.py @@ -4,6 +4,7 @@ from types import MappingProxyType from django.contrib.sessions.models import Session +from django.core import signing from django.db.models.functions.datetime import Now from codex.librarian.scribe.janitor.failed_imports import JanitorUpdateFailedImports @@ -172,7 +173,7 @@ def cleanup_custom_covers(self) -> None: self.status_controller.finish(status) def cleanup_sessions(self) -> None: - """Delete corrupt sessions.""" + """Delete expired and corrupt sessions.""" status = JanitorCleanupSessionsStatus() try: self.status_controller.start(status) @@ -181,9 +182,22 @@ def cleanup_sessions(self) -> None: if count: self.log.info(f"Deleted {count} expired sessions.") bad_session_keys = set() + store = Session.get_session_store_class()() + salt = store.key_salt # pyright: ignore[reportAttributeAccessIssue], # ty: ignore[unresolved-attribute] + serializer = store.serializer for encoded_session in Session.objects.all(): - session = encoded_session.get_decoded() - if not session: + # Session.get_decoded() swallows decode errors and returns + # an empty dict, which is also the legitimate state for an + # anonymous session with no stored data — so we can't use + # it to detect corruption. Call signing.loads directly so a + # genuine signature/decode failure raises. + try: + signing.loads( + encoded_session.session_data, + salt=salt, + serializer=serializer, + ) + except Exception: bad_session_keys.add(encoded_session.session_key) if bad_session_keys: diff --git a/codex/views/settings.py b/codex/views/settings.py index fc00a933e..61a3c39e0 100644 --- a/codex/views/settings.py +++ b/codex/views/settings.py @@ -6,6 +6,7 @@ from types import MappingProxyType from django.contrib.auth.models import AbstractBaseUser, AnonymousUser +from django.contrib.sessions.models import Session from loguru import logger from codex.choices.browser import DEFAULT_BROWSER_ROUTE @@ -56,10 +57,20 @@ class SettingsBaseView(AuthFilterGenericAPIView, ABC): # ── Session / user helpers ────────────────────────────────────── def _ensure_session_key(self) -> str | None: - """Ensure the Django session is saved and return its key.""" - if not self.request.session.session_key: - self.request.session.save() - return self.request.session.session_key + """Ensure a Django session row exists in the DB and return its key.""" + # The cookie may carry a session_key for a row that has been removed + # from the DB (e.g. by sessions cleanup or expiry). The cached_db + # backend serves such sessions from cache without rechecking, so + # session.session_key alone is not safe to use as an FK target. + session = self.request.session + if ( + session.session_key + and not Session.objects.filter(session_key=session.session_key).exists() + ): + session.flush() + if not session.session_key: + session.save() + return session.session_key def _get_request_user(self): """Return the authenticated user or None."""