From 9f04f13df41adb34ab7bea71e818ff10ad466f47 Mon Sep 17 00:00:00 2001 From: deacon-mp Date: Fri, 3 Apr 2026 17:38:10 -0400 Subject: [PATCH 1/4] fix: --fresh fails to clean cookie_storage, auth_svc crashes on key mismatch - Add data/cookie_storage to DATA_FILE_GLOBS so --fresh removes it - Catch SystemExit in auth_svc when file_svc._read() fails to decrypt stale cookie_storage; regenerate session key instead of crashing - Add tests: DATA_FILE_GLOBS membership + stale cookie recovery Fixes crash when switching between --insecure and secure mode after PR #3264 introduced persistent session cookies. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/service/auth_svc.py | 16 ++++-- app/service/data_svc.py | 1 + tests/services/test_fresh_cleanup.py | 74 ++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 tests/services/test_fresh_cleanup.py diff --git a/app/service/auth_svc.py b/app/service/auth_svc.py index 9dd6e15aa..0acb447e9 100644 --- a/app/service/auth_svc.py +++ b/app/service/auth_svc.py @@ -83,16 +83,22 @@ 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: + # 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) + 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..51d13ca49 100644 --- a/app/service/data_svc.py +++ b/app/service/data_svc.py @@ -35,6 +35,7 @@ 'data/results/*', 'data/sources/*', 'data/object_store', + 'data/cookie_storage', ) PAYLOADS_CONFIG_STANDARD_KEY = 'standard_payloads' diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py new file mode 100644 index 000000000..21eca2455 --- /dev/null +++ b/tests/services/test_fresh_cleanup.py @@ -0,0 +1,74 @@ +"""Tests for --fresh cleanup and auth_svc cookie recovery.""" +import copy +import os + +import pytest + +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 + + +class TestDataFileGlobs: + """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 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 p for p 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_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._app_configuration = self._saved_config + + 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 + ) + FileSvc() + + @pytest.mark.asyncio + async def test_stale_cookie_does_not_crash_server(self): + """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') + auth1 = AuthService() + await auth1.apply(app=web.Application(), users=BaseWorld.get_config('users')) + assert os.path.exists(self.cookie_path), 'cookie_storage should be created' + + # Round 2: switch to key B — before fix this was sys.exit(1) + self._apply_config('KEY_BETA_456') + auth2 = AuthService() + await auth2.apply(app=web.Application(), users=BaseWorld.get_config('users')) + assert os.path.exists(self.cookie_path), 'cookie_storage should be regenerated' From 0c4c9925f34a9f1f6a4a98b7719b8252dd86fdc7 Mon Sep 17 00:00:00 2001 From: Fiona McCrae Date: Fri, 10 Apr 2026 11:01:21 -0400 Subject: [PATCH 2/4] corrected tests to look for exact filepath in DATA_FILE_GLOBS --- tests/services/test_fresh_cleanup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py index 21eca2455..a2ebee46a 100644 --- a/tests/services/test_fresh_cleanup.py +++ b/tests/services/test_fresh_cleanup.py @@ -14,11 +14,11 @@ class TestDataFileGlobs: """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 p for p in DATA_FILE_GLOBS), \ + assert any('data/cookie_storage' == 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 p for p in DATA_FILE_GLOBS), \ + assert any('data/object_store' == p for p in DATA_FILE_GLOBS), \ 'data/object_store must be in DATA_FILE_GLOBS' From 277cc66fa77f570389332112e53b5c6ecef47f95 Mon Sep 17 00:00:00 2001 From: Fiona McCrae Date: Mon, 13 Apr 2026 17:54:28 -0400 Subject: [PATCH 3/4] reverting changes to auth_svc and file_svc, deleting test_fresh_cleanup since only necessary change to fix issue is adding data/cookie_storage to DATA_FILE_GLOBS --- app/service/auth_svc.py | 14 ++---- app/service/file_svc.py | 9 ++-- tests/services/test_fresh_cleanup.py | 74 ---------------------------- 3 files changed, 8 insertions(+), 89 deletions(-) delete mode 100644 tests/services/test_fresh_cleanup.py diff --git a/app/service/auth_svc.py b/app/service/auth_svc.py index 0acb447e9..7df42e6eb 100644 --- a/app/service/auth_svc.py +++ b/app/service/auth_svc.py @@ -84,21 +84,15 @@ async def apply(self, app, users): max_age = None try: 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: - # 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) - secret_key = os.urandom(32) - file_svc._save(cookie_path, secret_key, encrypt=True) + secret_key = file_svc._read(cookie_path) + self.log.debug('Loaded persistent session key from data/cookie_storage') 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/file_svc.py b/app/service/file_svc.py index ae032b4a0..73270276f 100644 --- a/app/service/file_svc.py +++ b/app/service/file_svc.py @@ -6,11 +6,10 @@ import os import re import subprocess -import sys from aiohttp import web from multidict import CIMultiDict -from cryptography.fernet import Fernet, InvalidToken +from cryptography.fernet import Fernet from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC @@ -282,15 +281,15 @@ def _read(self, filename): if self.encryptor and buf.startswith(bytes(FILE_ENCRYPTION_FLAG, encoding='utf-8')): try: buf = self.encryptor.decrypt(buf[len(FILE_ENCRYPTION_FLAG):]) - except InvalidToken: - self.log.error('Failed to decrypt saved Caldera state due to incorrect encryption key.\n' + except Exception as e: + self.log.error('Failed to decrypt saved Caldera state. This may be due to an incorrect encryption key.\n' ' - If attempting to restore secure backup, verify that conf/local.yml exists with ' 'correct encryption_key value, and that the server is being run without --insecure.\n' ' - If attempting to restore insecure backup, verify that conf/default.yml exists ' 'with correct encryption_key value, and that the server is being run with --insecure.\n' ' - If correct encryption_key value cannot be recovered, rerun the server with --fresh ' 'to disregard stored server state.') - sys.exit(1) + raise e return buf def _get_encryptor(self): diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py deleted file mode 100644 index a2ebee46a..000000000 --- a/tests/services/test_fresh_cleanup.py +++ /dev/null @@ -1,74 +0,0 @@ -"""Tests for --fresh cleanup and auth_svc cookie recovery.""" -import copy -import os - -import pytest - -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 - - -class TestDataFileGlobs: - """Verify that critical encrypted files are in the --fresh cleanup list.""" - - def test_cookie_storage_in_data_file_globs(self): - assert any('data/cookie_storage' == 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('data/object_store' == p for p 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_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._app_configuration = self._saved_config - - 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 - ) - FileSvc() - - @pytest.mark.asyncio - async def test_stale_cookie_does_not_crash_server(self): - """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') - auth1 = AuthService() - await auth1.apply(app=web.Application(), users=BaseWorld.get_config('users')) - assert os.path.exists(self.cookie_path), 'cookie_storage should be created' - - # Round 2: switch to key B — before fix this was sys.exit(1) - self._apply_config('KEY_BETA_456') - auth2 = AuthService() - await auth2.apply(app=web.Application(), users=BaseWorld.get_config('users')) - assert os.path.exists(self.cookie_path), 'cookie_storage should be regenerated' From 45d0589425ea9962afff9c4cb936504170e284ff Mon Sep 17 00:00:00 2001 From: uruwhy <58484522+uruwhy@users.noreply.github.com> Date: Mon, 13 Apr 2026 16:42:26 -0400 Subject: [PATCH 4/4] revert file_svc --- app/service/file_svc.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/service/file_svc.py b/app/service/file_svc.py index 73270276f..ae032b4a0 100644 --- a/app/service/file_svc.py +++ b/app/service/file_svc.py @@ -6,10 +6,11 @@ import os import re import subprocess +import sys from aiohttp import web from multidict import CIMultiDict -from cryptography.fernet import Fernet +from cryptography.fernet import Fernet, InvalidToken from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC @@ -281,15 +282,15 @@ def _read(self, filename): if self.encryptor and buf.startswith(bytes(FILE_ENCRYPTION_FLAG, encoding='utf-8')): try: buf = self.encryptor.decrypt(buf[len(FILE_ENCRYPTION_FLAG):]) - except Exception as e: - self.log.error('Failed to decrypt saved Caldera state. This may be due to an incorrect encryption key.\n' + except InvalidToken: + self.log.error('Failed to decrypt saved Caldera state due to incorrect encryption key.\n' ' - If attempting to restore secure backup, verify that conf/local.yml exists with ' 'correct encryption_key value, and that the server is being run without --insecure.\n' ' - If attempting to restore insecure backup, verify that conf/default.yml exists ' 'with correct encryption_key value, and that the server is being run with --insecure.\n' ' - If correct encryption_key value cannot be recovered, rerun the server with --fresh ' 'to disregard stored server state.') - raise e + sys.exit(1) return buf def _get_encryptor(self):