TASK-021: Implement atomic writes for set_jail_config_enabled and write_jail_config_file
This commit is contained in:
@@ -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`
|
||||
|
||||
---
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user