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 2e2cdee3d2cf01d6b8a0d4d3cc21ac11497e5647 Mon Sep 17 00:00:00 2001 From: RachHavoc <76081641+RachHavoc@users.noreply.github.com> Date: Fri, 10 Apr 2026 19:39:42 +0000 Subject: [PATCH 2/4] sanitize technique field --- app/api/v2/managers/ability_api_manager.py | 30 ++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/app/api/v2/managers/ability_api_manager.py b/app/api/v2/managers/ability_api_manager.py index 8dc6be270..3ccbe64be 100644 --- a/app/api/v2/managers/ability_api_manager.py +++ b/app/api/v2/managers/ability_api_manager.py @@ -2,9 +2,7 @@ import uuid import os import yaml - from typing import Any - from app.api.v2.managers.base_api_manager import BaseApiManager from app.api.v2.responses import JsonHttpBadRequest from app.objects.c_ability import AbilitySchema @@ -14,7 +12,7 @@ class AbilityApiManager(BaseApiManager): def __init__(self, data_svc, file_svc): super().__init__(data_svc=data_svc, file_svc=file_svc) - + async def create_on_disk_object(self, data: dict, access: dict, ram_key: str, id_property: str, obj_class: type): self._validate_ability_data(create=True, data=data) obj_id = data.get('id') @@ -23,7 +21,7 @@ async def create_on_disk_object(self, data: dict, access: dict, ram_key: str, id await self._save_and_reload_object(file_path, data, obj_class, allowed) await self._data_svc.create_or_update_everything_adversary() return next(self.find_objects(ram_key, {id_property: obj_id})) - + async def replace_on_disk_object(self, obj: Any, data: dict, ram_key: str, id_property: str): self._validate_ability_data(create=True, data=data) obj_id = getattr(obj, id_property) @@ -33,11 +31,11 @@ async def replace_on_disk_object(self, obj: Any, data: dict, ram_key: str, id_pr file_path = self._create_ability_filepath(data.get('tactic'), obj_id) await self._save_and_reload_object(file_path, data, type(obj), obj.access) return next(self.find_objects(ram_key, {id_property: obj_id})) - + async def remove_object_from_disk_by_id(self, identifier: str, ram_key: str): await super().remove_object_from_disk_by_id(identifier, ram_key) await self._data_svc.create_or_update_everything_adversary() - + async def update_on_disk_object(self, obj: Any, data: dict, ram_key: str, id_property: str, obj_class: type): obj_id = getattr(obj, id_property) file_path = await self._get_existing_object_file_path(obj_id, ram_key) @@ -49,11 +47,10 @@ async def update_on_disk_object(self, obj: Any, data: dict, ram_key: str, id_pro file_path = self._create_ability_filepath(data.get('tactic'), obj_id) await self._save_and_reload_object(file_path, existing_obj_data, obj_class, obj.access) return next(self.find_objects(ram_key, {id_property: obj_id})) - + def _validate_ability_data(self, create: bool, data: dict): # Correct ability_id key for ability file saving. data['id'] = data.pop('ability_id', '') - # If a new ability is being created, ensure required fields present. if create: # Set ability ID if undefined @@ -70,20 +67,27 @@ def _validate_ability_data(self, create: bool, data: dict): if 'id' in data and not validator.match(data['id']): raise JsonHttpBadRequest(f'Invalid ability ID {data["id"]}. IDs can only contain ' 'alphanumeric characters, hyphens, and underscores.') - # Validate tactic, used for directory creation, lower case if present if 'tactic' in data: if not validator.match(data['tactic']): raise JsonHttpBadRequest(f'Invalid ability tactic {data["tactic"]}. Tactics can only contain ' 'alphanumeric characters, hyphens, and underscores.') data['tactic'] = data['tactic'].lower() - + # PATCH: Validate technique_name to block HTML/script injection. + # Allowlist covers all legitimate ATT&CK technique name characters: + # letters, digits, spaces, hyphens, underscores, parentheses, + # slashes, dots, commas, colons, and ampersands. + # Excludes < > " ' ; which are not present in any ATT&CK technique name. + technique_name_validator = re.compile(r'^[a-zA-Z0-9\s\-_()/.,&:]+$') + if data.get('technique_name') and not technique_name_validator.match(data['technique_name']): + raise JsonHttpBadRequest(f'Invalid technique_name "{data["technique_name"]}". technique_name can only ' + 'contain alphanumeric characters, spaces, hyphens, underscores, ' + 'parentheses, slashes, dots, commas, colons, and ampersands.') if 'executors' in data and not data.get('executors'): raise JsonHttpBadRequest(f'Cannot create ability {data["id"]}: at least one executor required') - if 'name' in data and not data.get('name'): raise JsonHttpBadRequest(f'Cannot create ability {data["id"]} due to missing name') - + def _create_ability_filepath(self, tactic: str, obj_id: str): tactic_dir = os.path.join('data', 'abilities', tactic) if not os.path.exists(tactic_dir): @@ -94,4 +98,4 @@ async def _save_and_reload_object(self, file_path: str, data: dict, obj_type: ty await self._file_svc.save_file(file_path, yaml.dump([data], encoding='utf-8', sort_keys=False), '', encrypt=False) await self._data_svc.remove('abilities', dict(ability_id=data['id'])) - await self._data_svc.load_ability_file(file_path, access) + await self._data_svc.load_ability_file(file_path, access) \ No newline at end of file From 983e87c16fb0ac1244c149919b21b64fc00beb4a Mon Sep 17 00:00:00 2001 From: Daniel Matthews <58484522+uruwhy@users.noreply.github.com> Date: Tue, 14 Apr 2026 09:54:38 -0400 Subject: [PATCH 3/4] revert changes to auth_svc.py --- app/service/auth_svc.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/service/auth_svc.py b/app/service/auth_svc.py index 09ec4da1e..7df42e6eb 100644 --- a/app/service/auth_svc.py +++ b/app/service/auth_svc.py @@ -87,10 +87,12 @@ async def apply(self, app, users): 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: From 02e3e2bb488d0383639ea14608008568db0f756c Mon Sep 17 00:00:00 2001 From: Daniel Matthews <58484522+uruwhy@users.noreply.github.com> Date: Tue, 14 Apr 2026 09:55:30 -0400 Subject: [PATCH 4/4] removing unnecessary file --- tests/services/test_fresh_cleanup.py | 74 ---------------------------- 1 file changed, 74 deletions(-) delete mode 100644 tests/services/test_fresh_cleanup.py diff --git a/tests/services/test_fresh_cleanup.py b/tests/services/test_fresh_cleanup.py deleted file mode 100644 index 21eca2455..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('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'