diff --git a/backend/app/services/backup_service.py b/backend/app/services/backup_service.py index 603dbf64..687888de 100644 --- a/backend/app/services/backup_service.py +++ b/backend/app/services/backup_service.py @@ -15,6 +15,7 @@ import schedule from app import paths from app.utils import backup_crypto +from app.utils.config_store import load_json_config, save_json_config from app.utils.formatting import format_bytes from app.utils.system import is_command_available from app.services.telemetry_service import TelemetryService, generate_correlation_id @@ -53,14 +54,7 @@ class BackupService: @classmethod def get_config(cls) -> Dict: """Get backup configuration.""" - if os.path.exists(cls.BACKUP_CONFIG): - try: - with open(cls.BACKUP_CONFIG, 'r') as f: - return json.load(f) - except Exception: - pass - - return { + return load_json_config(cls.BACKUP_CONFIG, { 'enabled': False, 'retention_days': 30, 'encrypt_backups': False, @@ -70,18 +64,12 @@ class BackupService: 'on_failure': True, 'email': '' } - } + }) @classmethod def save_config(cls, config: Dict) -> Dict: """Save backup configuration.""" - try: - os.makedirs(cls.CONFIG_DIR, exist_ok=True) - with open(cls.BACKUP_CONFIG, 'w') as f: - json.dump(config, f, indent=2) - return {'success': True, 'message': 'Configuration saved'} - except Exception as e: - return {'success': False, 'error': str(e)} + return save_json_config(cls.BACKUP_CONFIG, config) @classmethod def backup_application(cls, app_name: str, app_path: str, diff --git a/backend/app/services/build_service.py b/backend/app/services/build_service.py index 5799209d..9e6b903f 100644 --- a/backend/app/services/build_service.py +++ b/backend/app/services/build_service.py @@ -22,6 +22,7 @@ from typing import Dict, List, Optional, Callable, Generator from pathlib import Path from app import paths +from app.utils.config_store import load_json_config class BuildService: @@ -68,13 +69,7 @@ class BuildService: @classmethod def get_config(cls) -> Dict: """Get build configuration.""" - if os.path.exists(cls.BUILD_CONFIG): - try: - with open(cls.BUILD_CONFIG, 'r') as f: - return json.load(f) - except Exception: - pass - return {'apps': {}} + return load_json_config(cls.BUILD_CONFIG, {'apps': {}}) @classmethod def save_config(cls, config: Dict) -> Dict: diff --git a/backend/app/services/git_service.py b/backend/app/services/git_service.py index 4b0e6db3..86f3508a 100644 --- a/backend/app/services/git_service.py +++ b/backend/app/services/git_service.py @@ -9,6 +9,7 @@ from typing import Dict, List, Optional from pathlib import Path from app import paths +from app.utils.config_store import load_json_config, save_json_config from app.utils.git_security import ( git_argv, git_env, @@ -28,28 +29,15 @@ class GitService: @classmethod def get_config(cls) -> Dict: """Get deployment configuration.""" - if os.path.exists(cls.DEPLOY_CONFIG): - try: - with open(cls.DEPLOY_CONFIG, 'r') as f: - return json.load(f) - except Exception: - pass - - return { + return load_json_config(cls.DEPLOY_CONFIG, { 'apps': {}, # app_id -> deployment config 'webhook_secret': cls._generate_secret() - } + }) @classmethod def save_config(cls, config: Dict) -> Dict: """Save deployment configuration.""" - try: - os.makedirs(cls.CONFIG_DIR, exist_ok=True) - with open(cls.DEPLOY_CONFIG, 'w') as f: - json.dump(config, f, indent=2) - return {'success': True, 'message': 'Configuration saved'} - except Exception as e: - return {'success': False, 'error': str(e)} + return save_json_config(cls.DEPLOY_CONFIG, config) @staticmethod def _generate_secret() -> str: diff --git a/backend/app/services/notification_service.py b/backend/app/services/notification_service.py index 355bdff2..606a299f 100644 --- a/backend/app/services/notification_service.py +++ b/backend/app/services/notification_service.py @@ -20,6 +20,7 @@ from typing import Dict, List, Optional from pathlib import Path from app import paths +from app.utils.config_store import load_json_config, save_json_config class NotificationService: @@ -40,14 +41,7 @@ class NotificationService: @classmethod def get_config(cls) -> Dict: """Get notification configuration.""" - if os.path.exists(cls.NOTIFICATIONS_CONFIG): - try: - with open(cls.NOTIFICATIONS_CONFIG, 'r') as f: - return json.load(f) - except Exception: - pass - - return { + return load_json_config(cls.NOTIFICATIONS_CONFIG, { 'discord': { 'enabled': False, 'webhook_url': '', @@ -87,18 +81,12 @@ class NotificationService: 'headers': {}, 'notify_on': ['critical', 'warning'] } - } + }) @classmethod def save_config(cls, config: Dict) -> Dict: """Save notification configuration.""" - try: - os.makedirs(cls.CONFIG_DIR, exist_ok=True) - with open(cls.NOTIFICATIONS_CONFIG, 'w') as f: - json.dump(config, f, indent=2) - return {'success': True, 'message': 'Configuration saved'} - except Exception as e: - return {'success': False, 'error': str(e)} + return save_json_config(cls.NOTIFICATIONS_CONFIG, config) @classmethod def update_channel_config(cls, channel: str, settings: Dict) -> Dict: diff --git a/backend/app/services/security_service.py b/backend/app/services/security_service.py index 42042836..8b42bd81 100644 --- a/backend/app/services/security_service.py +++ b/backend/app/services/security_service.py @@ -20,6 +20,7 @@ from pathlib import Path from .notification_service import NotificationService from app import paths +from app.utils.config_store import load_json_config, save_json_config from app.utils.system import ( PackageManager, ServiceControl, @@ -44,14 +45,7 @@ class SecurityService: @classmethod def get_config(cls) -> Dict: """Get security configuration.""" - if os.path.exists(cls.SECURITY_CONFIG): - try: - with open(cls.SECURITY_CONFIG, 'r') as f: - return json.load(f) - except Exception: - pass - - return { + return load_json_config(cls.SECURITY_CONFIG, { 'clamav': { 'enabled': True, 'scan_paths': ['/var/www', '/home'], @@ -84,18 +78,12 @@ class SecurityService: 'on_suspicious_activity': True, 'severity': 'critical' } - } + }) @classmethod def save_config(cls, config: Dict) -> Dict: """Save security configuration.""" - try: - os.makedirs(cls.CONFIG_DIR, exist_ok=True) - with open(cls.SECURITY_CONFIG, 'w') as f: - json.dump(config, f, indent=2) - return {'success': True, 'message': 'Configuration saved'} - except Exception as e: - return {'success': False, 'error': str(e)} + return save_json_config(cls.SECURITY_CONFIG, config) # ========================================== # CLAMAV INTEGRATION diff --git a/backend/app/utils/config_store.py b/backend/app/utils/config_store.py new file mode 100644 index 00000000..8ca181de --- /dev/null +++ b/backend/app/utils/config_store.py @@ -0,0 +1,44 @@ +"""JSON-file config persistence — one door for the load-or-default / +dump-or-error mechanics (plan 75 §F4). + +Five services shipped byte-identical ``save_config`` copies and seven more the +same ``get_config`` skeleton, each a place a fix (an atomic write, a better +error) would have had to land N times. These two helpers own the mechanics; +the per-service parts stay per-service: the path, the default, and any extra +behaviour around the write (cache invalidation, secret merging — those keep +their own methods and may call these helpers inside). + +Deliberate parity with the copies they replace: plain ``open(..., 'w')``, no +locking, no atomic rename — none of the originals had them, and adding them +here would be a behaviour change smuggled in with a consolidation. If atomic +writes are ever wanted, they land here, once. +""" +import json +import os + + +def load_json_config(path, default): + """JSON file → dict; *default* on missing or corrupt. + + A corrupt config reads as the default, never raises — the same contract + the hand-rolled copies had, and the one callers rely on at boot. + """ + if os.path.exists(path): + try: + with open(path, 'r') as f: + return json.load(f) + except Exception: # noqa: BLE001 + pass + return default + + +def save_json_config(path, config): + """dict → JSON file (indented), creating the parent dir. Returns the + service-dict shape every caller already returns.""" + try: + os.makedirs(os.path.dirname(path) or '.', exist_ok=True) + with open(path, 'w') as f: + json.dump(config, f, indent=2) + return {'success': True, 'message': 'Configuration saved'} + except Exception as e: # noqa: BLE001 + return {'success': False, 'error': str(e)} diff --git a/backend/tests/test_config_store.py b/backend/tests/test_config_store.py new file mode 100644 index 00000000..ed4c5e7e --- /dev/null +++ b/backend/tests/test_config_store.py @@ -0,0 +1,79 @@ +"""The ConfigStore contract (plan 75 §F4). + +Five services shipped byte-identical save_config copies; the helpers in +``app.utils.config_store`` are now the one place those mechanics live. These +tests pin the behaviour the copies had — byte-for-byte compatibility is the +point of the consolidation: indented JSON, parent dir created, corrupt file +reads as the default, and the service-dict return shape. +""" +import json +import os + +from app.utils.config_store import load_json_config, save_json_config + + +def test_missing_file_reads_as_the_default(tmp_path): + default = {'enabled': False} + assert load_json_config(str(tmp_path / 'cfg.json'), default) == default + + +def test_the_default_is_not_shared_between_calls(tmp_path): + """Callers mutate what they get back; a shared default would leak.""" + default = {'items': []} + first = load_json_config(str(tmp_path / 'cfg.json'), default) + first['items'].append('x') + assert load_json_config(str(tmp_path / 'cfg.json'), default) == default + + +def test_corrupt_file_reads_as_the_default_never_raises(tmp_path): + path = tmp_path / 'cfg.json' + path.write_text('{"truncated":') + assert load_json_config(str(path), {'a': 1}) == {'a': 1} + + +def test_save_creates_the_parent_dir_and_writes_indented_json(tmp_path): + path = tmp_path / 'nested' / 'cfg.json' + result = save_json_config(str(path), {'b': 2}) + + assert result == {'success': True, 'message': 'Configuration saved'} + assert json.loads(path.read_text()) == {'b': 2} + assert path.read_text() == json.dumps({'b': 2}, indent=2) + + +def test_save_failure_returns_the_error_shape(tmp_path): + blocked = tmp_path / 'a-file' + blocked.write_text('x') + result = save_json_config(str(blocked / 'cfg.json'), {'b': 2}) + + assert result['success'] is False + assert result['error'] + + +def test_round_trip(tmp_path): + path = str(tmp_path / 'cfg.json') + save_json_config(path, {'retention_days': 30}) + assert load_json_config(path, {}) == {'retention_days': 30} + + +# --------------------------------------------------------------------------- # +# The converted services actually go through the one door +# --------------------------------------------------------------------------- # + +def test_converted_services_use_the_store(tmp_path, monkeypatch): + """Spot-check two of the converted services end to end: their get/save + config must round-trip through the store with unchanged semantics.""" + from app.services.backup_service import BackupService + from app.services.git_service import GitService + + backup_path = str(tmp_path / 'backup.json') + monkeypatch.setattr(BackupService, 'BACKUP_CONFIG', backup_path) + assert BackupService.get_config()['retention_days'] == 30 + assert BackupService.save_config({'retention_days': 7})['success'] is True + assert BackupService.get_config() == {'retention_days': 7} + + git_path = str(tmp_path / 'deployments.json') + monkeypatch.setattr(GitService, 'DEPLOY_CONFIG', git_path) + default = GitService.get_config() + assert 'webhook_secret' in default and default['apps'] == {} + assert GitService.save_config({'apps': {'1': {}}})['success'] is True + assert GitService.get_config() == {'apps': {'1': {}}}