diff --git a/.gitignore b/.gitignore index 0bb3ba4..0554d6e 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,6 @@ repositories/ __pycache__/ *.pyc flask_session/ + +# Runtime lock files +config/*.lock diff --git a/simple_org_chart/app_main.py b/simple_org_chart/app_main.py index a54b0da..3bc002b 100644 --- a/simple_org_chart/app_main.py +++ b/simple_org_chart/app_main.py @@ -1200,7 +1200,10 @@ def import_settings(): elif key in email_keys: email_data[key] = value - settings_ok = save_settings(settings_data) + # Only save settings if at least one valid settings key was provided + settings_ok = True + if settings_data: + settings_ok = save_settings(settings_data) email_ok = True if email_data: diff --git a/simple_org_chart/email_config.py b/simple_org_chart/email_config.py index 57ef6aa..b4019ab 100644 --- a/simple_org_chart/email_config.py +++ b/simple_org_chart/email_config.py @@ -31,6 +31,12 @@ # Keys that belong to the email configuration _EMAIL_KEYS = set(DEFAULT_EMAIL_CONFIG.keys()) + +def _filter_email_keys(source: Dict[str, Any]) -> Dict[str, Any]: + """Return a new dict containing only keys that belong to the email config.""" + return {key: source[key] for key in _EMAIL_KEYS if key in source} + + def get_smtp_config() -> Dict[str, Any]: """Load SMTP configuration from environment variables.""" # Get encryption setting (TLS, SSL, or None) @@ -61,14 +67,16 @@ def load_email_config() -> Dict[str, Any]: try: with SETTINGS_FILE.open("r", encoding="utf-8") as handle: stored = json.load(handle) - merged = DEFAULT_EMAIL_CONFIG.copy() - for key in _EMAIL_KEYS: - if key in stored: - merged[key] = stored[key] - return merged except Exception as error: logger.error("Error loading email config: %s", error) - + else: + if not isinstance(stored, dict): + logger.warning("app_settings.json does not contain a JSON object; using email defaults") + return DEFAULT_EMAIL_CONFIG.copy() + merged = DEFAULT_EMAIL_CONFIG.copy() + merged.update(_filter_email_keys(stored)) + return merged + return DEFAULT_EMAIL_CONFIG.copy() @@ -76,9 +84,9 @@ def save_email_config(config: Dict[str, Any]) -> bool: """Save email configuration into app_settings.json.""" SETTINGS_FILE.parent.mkdir(parents=True, exist_ok=True) - # Merge with defaults + # Restrict to known email keys only — never let caller overwrite unrelated settings persisted = DEFAULT_EMAIL_CONFIG.copy() - persisted.update(config) + persisted.update(_filter_email_keys(config)) if 'lastSent' not in config: # lastSent will be preserved from the existing file inside the lock below diff --git a/simple_org_chart/settings.py b/simple_org_chart/settings.py index 67763d1..2913637 100644 --- a/simple_org_chart/settings.py +++ b/simple_org_chart/settings.py @@ -13,10 +13,83 @@ from .config import SETTINGS_FILE -# Shared in-process lock protecting all reads/writes to SETTINGS_FILE. + +class _InterProcessSettingsFileLock: + """Combined thread + process lock for protecting SETTINGS_FILE. + + Serialises concurrent access across both threads within a single worker + and across multiple Gunicorn worker processes by combining a + ``threading.Lock`` with an ``fcntl.flock`` advisory file lock. On + platforms where ``fcntl`` is unavailable (e.g. Windows) the class + falls back gracefully to thread-only locking. + """ + + def __init__(self, lock_file_path: str) -> None: + self._lock_file_path = lock_file_path + self._thread_lock = threading.Lock() + self._fd: int | None = None + + def acquire(self, blocking: bool = True) -> bool: + acquired = self._thread_lock.acquire(blocking) + if not acquired: + return False + fd = None + try: + import fcntl + fd = os.open(self._lock_file_path, os.O_CREAT | os.O_RDWR, 0o600) + flags = fcntl.LOCK_EX | (0 if blocking else fcntl.LOCK_NB) + fcntl.flock(fd, flags) + self._fd = fd + except ImportError: + # fcntl not available (Windows); thread lock is sufficient. + if fd is not None: + os.close(fd) + except BlockingIOError: + if fd is not None: + os.close(fd) + self._thread_lock.release() + return False + except Exception: + if fd is not None: + os.close(fd) + self._thread_lock.release() + raise + return True + + def release(self) -> None: + try: + if self._fd is not None: + try: + import fcntl + fcntl.flock(self._fd, fcntl.LOCK_UN) + except (ImportError, OSError): + pass + finally: + try: + os.close(self._fd) + except OSError: + pass + self._fd = None + finally: + self._thread_lock.release() + + def __enter__(self) -> "_InterProcessSettingsFileLock": + self.acquire() + return self + + def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None: + self.release() + + +# Shared inter-process lock protecting all reads/writes to SETTINGS_FILE. # Both settings.py and email_config.py import this lock so that concurrent -# writes from different code paths (scheduler, HTTP handlers) are serialised. -_settings_file_lock = threading.Lock() +# writes from different code paths (scheduler, HTTP handlers, workers) are +# serialised across all Gunicorn worker processes. +_settings_lock_file = os.path.join( + os.fspath(SETTINGS_FILE.parent), + f"{SETTINGS_FILE.name}.lock", +) +_settings_file_lock = _InterProcessSettingsFileLock(_settings_lock_file) logger = logging.getLogger(__name__)