From ec253d9b7a6c379fc8b3ce0e1c1b9d72b68fad69 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 26 Apr 2026 14:27:33 +0200 Subject: [PATCH] TASK-021: Implement atomic writes for set_jail_config_enabled and write_jail_config_file --- Docs/Backend-Development.md | 15 ++++++-- backend/app/services/config_file_helpers.py | 35 +++++++++++++++++++ backend/app/services/raw_config_io_service.py | 29 ++++++--------- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index 9e899c2..c92510a 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -841,14 +841,25 @@ except OSError as exc: - Creating the temp file in `target.parent` ensures atomicity. - On Linux containers, this prevents config corruption and service degradation. +**Atomic write helper:** + +A shared `atomic_write(path: Path, content: str)` helper is available in `app/services/config_file_helpers.py`. This is the preferred way to perform atomic writes — it handles all the temp file and cleanup logic: + +```python +from app.services.config_file_helpers import atomic_write + +atomic_write(path, updated_content) # Atomic write, auto-cleanup on error +``` + **Files requiring atomic writes:** -- All config files under `jail.d/` (created/modified by `_write_conf_file` and `_create_conf_file`) +- All config files under `jail.d/` (created/modified by `_write_conf_file`, `_create_conf_file`, `set_jail_config_enabled`, and `write_jail_config_file`) - Any critical state files that fail2ban relies on **Examples in the codebase:** -- `app/services/config_file_helpers.py`: `_write_conf_file`, `_create_conf_file` +- `app/services/config_file_helpers.py`: `_write_conf_file`, `_create_conf_file`, `atomic_write` +- `app/services/raw_config_io_service.py`: `set_jail_config_enabled`, `write_jail_config_file` - `app/services/jail_config_service.py`: `_write_local_file_sync`, `_restore_local_file_sync` --- diff --git a/backend/app/services/config_file_helpers.py b/backend/app/services/config_file_helpers.py index 0cf351a..fac3e48 100644 --- a/backend/app/services/config_file_helpers.py +++ b/backend/app/services/config_file_helpers.py @@ -257,3 +257,38 @@ def _create_conf_file(subdir: Path, name: str, content: str) -> str: raise ConfigFileWriteError(f"Cannot create {name!r}: {exc}") from exc return target.name + + +def atomic_write(path: Path, content: str) -> None: + """Write content to a file atomically using temp file + rename. + + This function ensures that if the process is killed during the write, + the target file is not corrupted. Uses a temporary file in the same + directory as the target, then atomically renames it into place. + + Args: + path: Target file path. + content: Content to write. + + Raises: + ConfigFileWriteError: If the file cannot be written. + """ + _validate_content(content) + + tmp_name: str | None = None + try: + with tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + dir=path.parent, + delete=False, + suffix=".tmp", + ) as tmp: + tmp.write(content) + tmp_name = tmp.name + os.replace(tmp_name, path) + except OSError as exc: + with contextlib.suppress(OSError): + if tmp_name is not None: + os.unlink(tmp_name) + raise ConfigFileWriteError(f"Cannot write {path.name!r}: {exc}") from exc diff --git a/backend/app/services/raw_config_io_service.py b/backend/app/services/raw_config_io_service.py index 52574b0..e3ae763 100644 --- a/backend/app/services/raw_config_io_service.py +++ b/backend/app/services/raw_config_io_service.py @@ -15,19 +15,16 @@ traversal attacks. from __future__ import annotations -from app.utils.async_utils import run_blocking -from app.exceptions import ( - ConfigFileNameError, - ConfigFileNotFoundError, - ConfigFileWriteError, -) import configparser import re -from pathlib import Path from typing import TYPE_CHECKING import structlog +from app.exceptions import ( + ConfigFileNameError, + ConfigFileNotFoundError, +) from app.models.file_config import ( ConfFileContent, ConfFileCreateRequest, @@ -46,9 +43,13 @@ from app.services.config_file_helpers import ( _resolve_subdir, _validate_content, _write_conf_file, + atomic_write, ) +from app.utils.async_utils import run_blocking if TYPE_CHECKING: + from pathlib import Path + from app.models.config import ( ActionConfig, ActionConfigUpdate, @@ -260,12 +261,7 @@ async def set_jail_config_enabled( original = path.read_text(encoding="utf-8", errors="replace") updated = _set_enabled_in_content(original, enabled) - try: - path.write_text(updated, encoding="utf-8") - except OSError as exc: - raise ConfigFileWriteError( - f"Cannot write {filename!r}: {exc}" - ) from exc + atomic_write(path, updated) log.info( "jail_config_file_enabled_set", filename=filename, @@ -336,12 +332,7 @@ async def write_jail_config_file( ) if not path.is_file(): raise ConfigFileNotFoundError(filename) - try: - path.write_text(req.content, encoding="utf-8") - except OSError as exc: - raise ConfigFileWriteError( - f"Cannot write {filename!r}: {exc}" - ) from exc + atomic_write(path, req.content) log.info("jail_config_file_written", filename=filename) await run_blocking( _do)