From 3c4ef72a5115242ff5e08010ca2cc87a230647eb Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Fri, 26 Jun 2026 19:06:12 -0400 Subject: [PATCH] fix(#5022): surface auth persistence failures --- api/auth.py | 80 +++++++++++++++++++++----- tests/test_auth_session_persistence.py | 67 +++++++++++++++++++-- 2 files changed, 128 insertions(+), 19 deletions(-) diff --git a/api/auth.py b/api/auth.py index 8dd05de6..3d24b389 100644 --- a/api/auth.py +++ b/api/auth.py @@ -14,6 +14,7 @@ import secrets import tempfile import threading import time +from pathlib import Path from api.config import STATE_DIR, load_settings @@ -85,6 +86,18 @@ def _resolve_cookie_name() -> str: return COOKIE_NAME +def _warn_auth_persistence_failure(prefix: str, artifact: Path, exc: Exception, consequence: str) -> None: + logger.warning( + '%s at %s (STATE_DIR=%s): %s: %s; %s', + prefix, + artifact, + STATE_DIR, + exc.__class__.__name__, + exc, + consequence, + ) + + _SESSIONS_FILE = STATE_DIR / '.sessions.json' @@ -95,16 +108,38 @@ def _load_sessions() -> dict[str, float]: blocked by a corrupt or missing sessions file. """ try: - if _SESSIONS_FILE.exists(): - data = json.loads(_SESSIONS_FILE.read_text(encoding='utf-8')) - if not isinstance(data, dict): - raise ValueError('malformed sessions file — expected dict') - now = time.time() - return {t: exp for t, exp in data.items() - if isinstance(t, str) and isinstance(exp, (int, float)) and exp > now} - except Exception as e: - logger.debug("Failed to load sessions file, starting fresh: %s", e) - return {} + if not _SESSIONS_FILE.exists(): + return {} + raw = _SESSIONS_FILE.read_text(encoding='utf-8') + except OSError as e: + _warn_auth_persistence_failure( + 'Auth session store read failed', + _SESSIONS_FILE, + e, + 'starting fresh with an empty session table', + ) + return {} + try: + data = json.loads(raw) + except json.JSONDecodeError as e: + _warn_auth_persistence_failure( + 'Ignoring malformed auth session store', + _SESSIONS_FILE, + e, + 'starting fresh with an empty session table', + ) + return {} + if not isinstance(data, dict): + _warn_auth_persistence_failure( + 'Ignoring malformed auth session store', + _SESSIONS_FILE, + ValueError('malformed sessions file: expected dict'), + 'starting fresh with an empty session table', + ) + return {} + now = time.time() + return {t: exp for t, exp in data.items() + if isinstance(t, str) and isinstance(exp, (int, float)) and exp > now} def _save_sessions(sessions: dict[str, float]) -> None: @@ -128,7 +163,12 @@ def _save_sessions(sessions: dict[str, float]) -> None: pass raise except Exception as e: - logger.debug("Failed to persist sessions: %s", e) + _warn_auth_persistence_failure( + 'Auth session persistence failed', + _SESSIONS_FILE, + e, + 'keeping the in-process session table available', + ) # Active sessions: token -> expiry timestamp (persisted across restarts via STATE_DIR) @@ -231,15 +271,25 @@ def _load_key(filename: str) -> bytes: raw = key_file.read_bytes() if len(raw) >= 32: return raw[:32] - except OSError: - logger.debug("Failed to read key %s", filename) + except OSError as e: + _warn_auth_persistence_failure( + 'Auth key read failed', + key_file, + e, + 'generating a new key and continuing', + ) key = secrets.token_bytes(32) try: STATE_DIR.mkdir(parents=True, exist_ok=True) key_file.write_bytes(key) key_file.chmod(0o600) - except OSError: - logger.debug("Failed to persist key %s", filename) + except OSError as e: + _warn_auth_persistence_failure( + 'Auth key persistence failed', + key_file, + e, + 'returning the generated key so startup can continue', + ) return key diff --git a/tests/test_auth_session_persistence.py b/tests/test_auth_session_persistence.py index bbe729f3..5295b372 100644 --- a/tests/test_auth_session_persistence.py +++ b/tests/test_auth_session_persistence.py @@ -12,6 +12,7 @@ import sys import tempfile import time import unittest +from unittest import mock from pathlib import Path # Isolate state dir so tests never touch real sessions @@ -28,9 +29,12 @@ class TestSessionPersistence(unittest.TestCase): def setUp(self) -> None: auth._sessions.clear() - sessions_file = _TEST_STATE / '.sessions.json' - if sessions_file.exists(): - sessions_file.unlink() + auth._PBKDF2_KEY_CACHE = None + auth._SIGNING_KEY_CACHE = None + for name in ('.sessions.json', '.signing_key', '.pbkdf2_key'): + path = _TEST_STATE / name + if path.exists(): + path.unlink() def _simulate_restart(self) -> None: """Reload auth module to simulate a fresh process start. @@ -98,9 +102,64 @@ class TestSessionPersistence(unittest.TestCase): """A corrupt sessions file must not crash auth — start with empty dict.""" sessions_file = _TEST_STATE / '.sessions.json' sessions_file.write_text("not valid json {{{{") - self._simulate_restart() + with self.assertLogs('api.auth', level='WARNING') as captured: + self._simulate_restart() self.assertEqual(auth._sessions, {}, "Corrupt sessions file must result in empty session dict") + warning = '\n'.join(captured.output) + self.assertIn('Ignoring malformed auth session store', warning) + self.assertIn(str(sessions_file), warning) + self.assertIn(str(_TEST_STATE), warning) + + def test_session_read_failure_warns_with_state_dir_and_starts_fresh(self) -> None: + """Unreadable sessions files must warn and start with an empty dict.""" + sessions_file = _TEST_STATE / '.sessions.json' + sessions_file.write_text(json.dumps({"token": time.time() + 3600})) + with mock.patch.object(Path, 'read_text', side_effect=OSError('read failed')): + with self.assertLogs('api.auth', level='WARNING') as captured: + self._simulate_restart() + self.assertEqual(auth._sessions, {}) + warning = '\n'.join(captured.output) + self.assertIn('Auth session store read failed', warning) + self.assertIn(str(sessions_file), warning) + self.assertIn(str(_TEST_STATE), warning) + + def test_session_save_failure_warns_with_state_dir_and_keeps_in_process_session(self) -> None: + """Write failures must warn but keep the live session usable in-process.""" + with mock.patch.object(auth.os, 'replace', side_effect=OSError('replace failed')): + with self.assertLogs('api.auth', level='WARNING') as captured: + cookie = auth.create_session() + self.assertTrue(auth.verify_session(cookie)) + warning = '\n'.join(captured.output) + self.assertIn('Auth session persistence failed', warning) + self.assertIn('.sessions.json', warning) + self.assertIn(str(_TEST_STATE), warning) + + def test_signing_key_read_failure_warns_with_state_dir_and_generates_key(self) -> None: + """Unreadable signing keys must warn and fall back to a fresh key.""" + key_file = _TEST_STATE / '.signing_key' + key_file.write_text('stub') + with mock.patch.object(Path, 'read_bytes', side_effect=OSError('read failed')): + with self.assertLogs('api.auth', level='WARNING') as captured: + key = auth._load_key('.signing_key') + self.assertIsInstance(key, bytes) + self.assertEqual(len(key), 32) + warning = '\n'.join(captured.output) + self.assertIn('Auth key read failed', warning) + self.assertIn('.signing_key', warning) + self.assertIn(str(_TEST_STATE), warning) + + def test_signing_key_persist_failure_warns_with_state_dir(self) -> None: + """Key write failures must warn and still return a generated key.""" + with mock.patch.object(Path, 'write_bytes', side_effect=OSError('write failed')): + with self.assertLogs('api.auth', level='WARNING') as captured: + key = auth._load_key('.signing_key') + self.assertIsInstance(key, bytes) + self.assertEqual(len(key), 32) + warning = '\n'.join(captured.output) + self.assertIn('Auth key persistence failed', warning) + self.assertIn('.signing_key', warning) + self.assertIn(str(_TEST_STATE), warning) def test_sessions_file_wrong_type_starts_fresh(self) -> None: """A sessions file containing a non-dict must be ignored."""