From bddeb20a6c26d934524f6fd1828dd012541a763a Mon Sep 17 00:00:00 2001 From: deacon-mp Date: Fri, 3 Apr 2026 16:21:55 -0400 Subject: [PATCH 1/6] fix: --fresh now cleans cookie_storage and fact_store; auth_svc recovers from stale cookies - Add data/fact_store and data/cookie_storage to DATA_FILE_GLOBS so --fresh removes them - Handle decryption failure in auth_svc gracefully: regenerate session key instead of crashing - Fixes persistent encryption key mismatch error after switching --insecure mode or keys Co-Authored-By: Claude Opus 4.6 (1M context) --- app/service/auth_svc.py | 15 ++++++++++----- app/service/data_svc.py | 2 ++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/service/auth_svc.py b/app/service/auth_svc.py index 9dd6e15aa..22ea40326 100644 --- a/app/service/auth_svc.py +++ b/app/service/auth_svc.py @@ -83,16 +83,21 @@ async def apply(self, app, users): except (ValueError, TypeError): max_age = None try: - if os.path.exists(os.path.join('data', cookie_file)): - secret_key = file_svc._read(cookie_path) - self.log.debug('Loaded persistent session key from data/cookie_storage') + if os.path.exists(cookie_path): + try: + secret_key = file_svc._read(cookie_path) + self.log.debug('Loaded persistent session key from data/cookie_storage') + except (SystemExit, Exception): + self.log.warning('Failed to decrypt cookie_storage (encryption key mismatch). ' + 'Regenerating session key — existing sessions will be invalidated.') + os.remove(cookie_path) + secret_key = os.urandom(32) + file_svc._save(cookie_path, secret_key, encrypt=True) else: - # Generate a new random 32-byte key for AES encryption if no valid key is found in the config or data folder secret_key = os.urandom(32) file_svc._save(cookie_path, secret_key, encrypt=True) self.log.debug('Generated and saved new persistent session key.') except Exception as e: - # Fallback if file operations fail self.log.warning('Could not manage persistent key file, falling back to ephemeral: %s', e) secret_key = os.urandom(32) if len(secret_key) != 32: diff --git a/app/service/data_svc.py b/app/service/data_svc.py index d5e9d8ccf..c1e4152a7 100644 --- a/app/service/data_svc.py +++ b/app/service/data_svc.py @@ -35,6 +35,8 @@ 'data/results/*', 'data/sources/*', 'data/object_store', + 'data/fact_store', + 'data/cookie_storage', ) PAYLOADS_CONFIG_STANDARD_KEY = 'standard_payloads' From 35c7acddeb42eca33ec9472eab3524f3e18eacd2 Mon Sep 17 00:00:00 2001 From: deacon-mp Date: Fri, 3 Apr 2026 16:25:06 -0400 Subject: [PATCH 2/6] Add tests proving --fresh cleanup and stale cookie recovery --- tests/services/test_fresh_cleanup.py | 84 ++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 tests/services/test_fresh_cleanup.py diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py new file mode 100644 index 000000000..bac001aa8 --- /dev/null +++ b/tests/services/test_fresh_cleanup.py @@ -0,0 +1,84 @@ +"""Tests proving that --fresh cleanup and auth_svc recovery work correctly. + +These tests verify: +1. data/cookie_storage and data/fact_store are cleaned by --fresh (via DATA_FILE_GLOBS) +2. auth_svc recovers gracefully when cookie_storage was encrypted with a different key +""" +import os + +import pytest + +from aiohttp import web +from app.service.auth_svc import AuthService, CONFIG_API_KEY_RED, COOKIE_SESSION +from app.service.data_svc import DATA_FILE_GLOBS +from app.service.file_svc import FileSvc +from app.utility.base_world import BaseWorld + +pytestmark = pytest.mark.asyncio + + +class TestDataFileGlobs: + """Verify that critical encrypted files are included in the --fresh cleanup list.""" + + def test_cookie_storage_in_data_file_globs(self): + assert any('cookie_storage' in pattern for pattern in DATA_FILE_GLOBS), \ + 'data/cookie_storage must be in DATA_FILE_GLOBS so --fresh cleans it up' + + def test_fact_store_in_data_file_globs(self): + assert any('fact_store' in pattern for pattern in DATA_FILE_GLOBS), \ + 'data/fact_store must be in DATA_FILE_GLOBS so --fresh cleans it up' + + def test_object_store_in_data_file_globs(self): + assert any('object_store' in pattern for pattern in DATA_FILE_GLOBS), \ + 'data/object_store must be in DATA_FILE_GLOBS' + + +class TestAuthSvcCookieRecovery: + """Verify auth_svc recovers when cookie_storage has a stale encryption key.""" + + @pytest.fixture(autouse=True) + def setup_and_cleanup(self): + self.cookie_path = os.path.join('data', 'cookie_storage') + if os.path.exists(self.cookie_path): + os.remove(self.cookie_path) + yield + if os.path.exists(self.cookie_path): + os.remove(self.cookie_path) + + def _apply_config(self, encryption_key): + BaseWorld.clear_config() + BaseWorld.apply_config( + name='main', + config={ + CONFIG_API_KEY_RED: 'abc123', + 'crypt_salt': 'REPLACE_WITH_RANDOM_VALUE', + 'encryption_key': encryption_key, + 'users': { + 'red': {'reduser': 'redpass'}, + 'blue': {'blueuser': 'bluepass'} + }, + }, + apply_hash=True + ) + # Ensure file_svc is registered so auth_svc can use it + FileSvc() + + async def test_stale_cookie_does_not_crash_server(self): + """Prove that a cookie_storage encrypted with key A doesn't crash when loaded with key B.""" + # Step 1: Create cookie_storage with encryption key A + self._apply_config('KEY_ALPHA_123') + app1 = web.Application() + auth1 = AuthService() + await auth1.apply(app=app1, users=BaseWorld.get_config('users')) + assert os.path.exists(self.cookie_path), 'cookie_storage should be created' + + # Step 2: Switch to encryption key B and re-init auth + # Before fix: this would sys.exit(1) due to InvalidToken in file_svc._read() + # After fix: auth_svc catches the error, regenerates the key, and continues + self._apply_config('KEY_BETA_456') + app2 = web.Application() + auth2 = AuthService() + # This should NOT crash + await auth2.apply(app=app2, users=BaseWorld.get_config('users')) + # cookie_storage should still exist (regenerated with new key) + assert os.path.exists(self.cookie_path), 'cookie_storage should be regenerated' From dae58d3efae2ce7aa8fb6bf8636014f543ae9e27 Mon Sep 17 00:00:00 2001 From: deacon-mp Date: Fri, 3 Apr 2026 16:48:29 -0400 Subject: [PATCH 3/6] Address Copilot: remove fact_store from DATA_FILE_GLOBS, narrow SystemExit catch - Remove data/fact_store from DATA_FILE_GLOBS (knowledge_svc handles its own cleanup) - Narrow inner catch to SystemExit only (let IO/permission errors propagate) - Test already covers the SystemExit recovery path --- app/service/auth_svc.py | 3 ++- app/service/data_svc.py | 1 - tests/services/test_fresh_cleanup.py | 6 +----- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/app/service/auth_svc.py b/app/service/auth_svc.py index 22ea40326..0acb447e9 100644 --- a/app/service/auth_svc.py +++ b/app/service/auth_svc.py @@ -87,7 +87,8 @@ async def apply(self, app, users): try: secret_key = file_svc._read(cookie_path) self.log.debug('Loaded persistent session key from data/cookie_storage') - except (SystemExit, Exception): + except SystemExit: + # file_svc._read() calls sys.exit(1) on InvalidToken (encryption key mismatch). self.log.warning('Failed to decrypt cookie_storage (encryption key mismatch). ' 'Regenerating session key — existing sessions will be invalidated.') os.remove(cookie_path) diff --git a/app/service/data_svc.py b/app/service/data_svc.py index c1e4152a7..51d13ca49 100644 --- a/app/service/data_svc.py +++ b/app/service/data_svc.py @@ -35,7 +35,6 @@ 'data/results/*', 'data/sources/*', 'data/object_store', - 'data/fact_store', 'data/cookie_storage', ) diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py index bac001aa8..9aa9fbb60 100644 --- a/tests/services/test_fresh_cleanup.py +++ b/tests/services/test_fresh_cleanup.py @@ -1,7 +1,7 @@ """Tests proving that --fresh cleanup and auth_svc recovery work correctly. These tests verify: -1. data/cookie_storage and data/fact_store are cleaned by --fresh (via DATA_FILE_GLOBS) +1. data/cookie_storage is cleaned by --fresh (via DATA_FILE_GLOBS) 2. auth_svc recovers gracefully when cookie_storage was encrypted with a different key """ import os @@ -24,10 +24,6 @@ def test_cookie_storage_in_data_file_globs(self): assert any('cookie_storage' in pattern for pattern in DATA_FILE_GLOBS), \ 'data/cookie_storage must be in DATA_FILE_GLOBS so --fresh cleans it up' - def test_fact_store_in_data_file_globs(self): - assert any('fact_store' in pattern for pattern in DATA_FILE_GLOBS), \ - 'data/fact_store must be in DATA_FILE_GLOBS so --fresh cleans it up' - def test_object_store_in_data_file_globs(self): assert any('object_store' in pattern for pattern in DATA_FILE_GLOBS), \ 'data/object_store must be in DATA_FILE_GLOBS' From a6e5101d6a5fa6615341436c247c9e797a1e36f5 Mon Sep 17 00:00:00 2001 From: deacon-mp Date: Fri, 3 Apr 2026 16:59:26 -0400 Subject: [PATCH 4/6] Fix flake8 F401, asyncio mark, and test teardown cleanup - Remove unused COOKIE_SESSION import - Use per-test @pytest.mark.asyncio instead of global pytestmark - Clear BaseWorld config in fixture teardown to avoid state leakage --- tests/services/test_fresh_cleanup.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py index 9aa9fbb60..0d03a1b2d 100644 --- a/tests/services/test_fresh_cleanup.py +++ b/tests/services/test_fresh_cleanup.py @@ -9,14 +9,11 @@ import pytest from aiohttp import web -from app.service.auth_svc import AuthService, CONFIG_API_KEY_RED, COOKIE_SESSION +from app.service.auth_svc import AuthService, CONFIG_API_KEY_RED from app.service.data_svc import DATA_FILE_GLOBS from app.service.file_svc import FileSvc from app.utility.base_world import BaseWorld -pytestmark = pytest.mark.asyncio - - class TestDataFileGlobs: """Verify that critical encrypted files are included in the --fresh cleanup list.""" @@ -40,6 +37,7 @@ def setup_and_cleanup(self): yield if os.path.exists(self.cookie_path): os.remove(self.cookie_path) + BaseWorld.clear_config() def _apply_config(self, encryption_key): BaseWorld.clear_config() @@ -59,6 +57,7 @@ def _apply_config(self, encryption_key): # Ensure file_svc is registered so auth_svc can use it FileSvc() + @pytest.mark.asyncio async def test_stale_cookie_does_not_crash_server(self): """Prove that a cookie_storage encrypted with key A doesn't crash when loaded with key B.""" # Step 1: Create cookie_storage with encryption key A From d406cb30ad97ae75e3db615a88d763c1f74f0a64 Mon Sep 17 00:00:00 2001 From: deacon-mp Date: Fri, 3 Apr 2026 17:24:01 -0400 Subject: [PATCH 5/6] Fix flake8 E302: add missing blank line before class --- tests/services/test_fresh_cleanup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py index 0d03a1b2d..8c22174ce 100644 --- a/tests/services/test_fresh_cleanup.py +++ b/tests/services/test_fresh_cleanup.py @@ -14,6 +14,7 @@ from app.service.file_svc import FileSvc from app.utility.base_world import BaseWorld + class TestDataFileGlobs: """Verify that critical encrypted files are included in the --fresh cleanup list.""" From 60bb5ca8cf464e8225467f524528f8b8eae24a91 Mon Sep 17 00:00:00 2001 From: deacon-mp Date: Fri, 3 Apr 2026 17:31:33 -0400 Subject: [PATCH 6/6] =?UTF-8?q?Rewrite=20test=20to=20save/restore=20BaseWo?= =?UTF-8?q?rld=20state=20=E2=80=94=20no=20leakage=20to=20other=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/services/test_fresh_cleanup.py | 42 ++++++++++++---------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py index 8c22174ce..21eca2455 100644 --- a/tests/services/test_fresh_cleanup.py +++ b/tests/services/test_fresh_cleanup.py @@ -1,14 +1,9 @@ -"""Tests proving that --fresh cleanup and auth_svc recovery work correctly. - -These tests verify: -1. data/cookie_storage is cleaned by --fresh (via DATA_FILE_GLOBS) -2. auth_svc recovers gracefully when cookie_storage was encrypted with a different key -""" +"""Tests for --fresh cleanup and auth_svc cookie recovery.""" +import copy import os import pytest -from aiohttp import web from app.service.auth_svc import AuthService, CONFIG_API_KEY_RED from app.service.data_svc import DATA_FILE_GLOBS from app.service.file_svc import FileSvc @@ -16,14 +11,14 @@ class TestDataFileGlobs: - """Verify that critical encrypted files are included in the --fresh cleanup list.""" + """Verify that critical encrypted files are in the --fresh cleanup list.""" def test_cookie_storage_in_data_file_globs(self): - assert any('cookie_storage' in pattern for pattern in DATA_FILE_GLOBS), \ + assert any('cookie_storage' in p for p in DATA_FILE_GLOBS), \ 'data/cookie_storage must be in DATA_FILE_GLOBS so --fresh cleans it up' def test_object_store_in_data_file_globs(self): - assert any('object_store' in pattern for pattern in DATA_FILE_GLOBS), \ + assert any('object_store' in p for p in DATA_FILE_GLOBS), \ 'data/object_store must be in DATA_FILE_GLOBS' @@ -31,14 +26,18 @@ class TestAuthSvcCookieRecovery: """Verify auth_svc recovers when cookie_storage has a stale encryption key.""" @pytest.fixture(autouse=True) - def setup_and_cleanup(self): + def setup_and_teardown(self): self.cookie_path = os.path.join('data', 'cookie_storage') + # Save existing state + self._saved_config = copy.deepcopy(BaseWorld._app_configuration) + # Clean pre-existing cookie if os.path.exists(self.cookie_path): os.remove(self.cookie_path) yield + # Restore original state — leave no trace if os.path.exists(self.cookie_path): os.remove(self.cookie_path) - BaseWorld.clear_config() + BaseWorld._app_configuration = self._saved_config def _apply_config(self, encryption_key): BaseWorld.clear_config() @@ -55,26 +54,21 @@ def _apply_config(self, encryption_key): }, apply_hash=True ) - # Ensure file_svc is registered so auth_svc can use it FileSvc() @pytest.mark.asyncio async def test_stale_cookie_does_not_crash_server(self): - """Prove that a cookie_storage encrypted with key A doesn't crash when loaded with key B.""" - # Step 1: Create cookie_storage with encryption key A + """Cookie encrypted with key A must not crash server when loaded with key B.""" + from aiohttp import web + + # Round 1: create cookie_storage with key A self._apply_config('KEY_ALPHA_123') - app1 = web.Application() auth1 = AuthService() - await auth1.apply(app=app1, users=BaseWorld.get_config('users')) + await auth1.apply(app=web.Application(), users=BaseWorld.get_config('users')) assert os.path.exists(self.cookie_path), 'cookie_storage should be created' - # Step 2: Switch to encryption key B and re-init auth - # Before fix: this would sys.exit(1) due to InvalidToken in file_svc._read() - # After fix: auth_svc catches the error, regenerates the key, and continues + # Round 2: switch to key B — before fix this was sys.exit(1) self._apply_config('KEY_BETA_456') - app2 = web.Application() auth2 = AuthService() - # This should NOT crash - await auth2.apply(app=app2, users=BaseWorld.get_config('users')) - # cookie_storage should still exist (regenerated with new key) + await auth2.apply(app=web.Application(), users=BaseWorld.get_config('users')) assert os.path.exists(self.cookie_path), 'cookie_storage should be regenerated'