diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 4a84edf..5ee71a3 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -123,6 +123,8 @@ With multiple copies of the same class name in different modules, `isinstance` c ### Task 5 — Resolve config_file_service.py god object and dual implementations +**Status:** In progress + **Severity:** High **Where:** @@ -230,6 +232,7 @@ The current inconsistency (five services at 10 s, three at 5 s) appears to be ac --- ### Task 8 — Remove app/helpers/ indirection layer; fix circular imports structurally +**Status:** Completed **Severity:** Medium diff --git a/backend/app/helpers/config_file_helpers.py b/backend/app/helpers/config_file_helpers.py deleted file mode 100644 index 42f5f71..0000000 --- a/backend/app/helpers/config_file_helpers.py +++ /dev/null @@ -1,19 +0,0 @@ -"""Shared config-file helpers reused across service modules.""" - -from __future__ import annotations - -from app.services.config_file_service import ( - _build_inactive_jail, - _get_active_jail_names, - _ordered_config_files, - _parse_jails_sync, - _validate_jail_config_sync, -) - -__all__ = [ - "_ordered_config_files", - "_parse_jails_sync", - "_build_inactive_jail", - "_get_active_jail_names", - "_validate_jail_config_sync", -] diff --git a/backend/app/helpers/jail_helpers.py b/backend/app/helpers/jail_helpers.py deleted file mode 100644 index c6ba9a0..0000000 --- a/backend/app/helpers/jail_helpers.py +++ /dev/null @@ -1,23 +0,0 @@ -"""Shared jail management helpers for service-layer code.""" - -from __future__ import annotations - -from typing import TYPE_CHECKING - -from app.services.jail_service import reload_all - -if TYPE_CHECKING: - from collections.abc import Sequence - - -async def reload_jails( - socket_path: str, - include_jails: Sequence[str] | None = None, - exclude_jails: Sequence[str] | None = None, -) -> None: - """Reload fail2ban jails using the shared jail service helper.""" - await reload_all( - socket_path, - include_jails=list(include_jails) if include_jails is not None else None, - exclude_jails=list(exclude_jails) if exclude_jails is not None else None, - ) diff --git a/backend/app/helpers/log_helpers.py b/backend/app/helpers/log_helpers.py deleted file mode 100644 index 3e5bf6f..0000000 --- a/backend/app/helpers/log_helpers.py +++ /dev/null @@ -1,21 +0,0 @@ -"""Shared log-related helpers for service-layer code.""" - -from __future__ import annotations - -from typing import TYPE_CHECKING - -from app.services.log_service import preview_log as _preview_log -from app.services.log_service import test_regex as _test_regex - -if TYPE_CHECKING: - from app.models.config import LogPreviewRequest, LogPreviewResponse, RegexTestRequest, RegexTestResponse - - -def preview_log(req: LogPreviewRequest) -> LogPreviewResponse: - """Return log preview results using the shared log service.""" - return _preview_log(req) - - -def test_regex(req: RegexTestRequest) -> RegexTestResponse: - """Validate a regex pattern using the shared log service.""" - return _test_regex(req) diff --git a/backend/app/services/action_config_service.py b/backend/app/services/action_config_service.py index 3711e00..2585b8f 100644 --- a/backend/app/services/action_config_service.py +++ b/backend/app/services/action_config_service.py @@ -23,14 +23,15 @@ from app.exceptions import ( ActionNotFoundError, ActionReadonlyError, ConfigWriteError, - JailNameError, JailNotFoundInConfigError, ) -from app.helpers.config_file_helpers import ( - _get_active_jail_names, - _parse_jails_sync, +from app.services.config_file_service import ( + build_parser, + get_active_jail_names, + parse_jails_sync, + safe_jail_name, ) -from app.helpers.jail_helpers import reload_jails +from app.services.jail_service import reload_all from app.models.config import ( ActionConfig, ActionConfigUpdate, @@ -53,85 +54,15 @@ _SOCKET_TIMEOUT: float = 10.0 # Allowlist pattern for action names used in path construction. _SAFE_ACTION_NAME_RE: re.Pattern[str] = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") -# Allowlist pattern for jail names used in path construction. -_SAFE_JAIL_NAME_RE: re.Pattern[str] = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") - # Sections that are not jail definitions. _META_SECTIONS: frozenset[str] = frozenset({"INCLUDES", "DEFAULT"}) -# True-ish values for the ``enabled`` key. -_TRUE_VALUES: frozenset[str] = frozenset({"true", "yes", "1"}) - -# False-ish values for the ``enabled`` key. -_FALSE_VALUES: frozenset[str] = frozenset({"false", "no", "0"}) - # --------------------------------------------------------------------------- # Internal helpers # --------------------------------------------------------------------------- -def _safe_jail_name(name: str) -> str: - """Validate *name* and return it unchanged or raise :class:`JailNameError`. - - Args: - name: Proposed jail name. - - Returns: - The name unchanged if valid. - - Raises: - JailNameError: If *name* contains unsafe characters. - """ - if not _SAFE_JAIL_NAME_RE.match(name): - raise JailNameError( - f"Jail name {name!r} contains invalid characters. " - "Only alphanumeric characters, hyphens, underscores, and dots are " - "allowed; must start with an alphanumeric character." - ) - return name - - -def _build_parser() -> configparser.RawConfigParser: - """Create a :class:`configparser.RawConfigParser` for fail2ban configs. - - Returns: - Parser with interpolation disabled and case-sensitive option names. - """ - parser = configparser.RawConfigParser(interpolation=None, strict=False) - # fail2ban keys are lowercase but preserve case to be safe. - parser.optionxform = str # type: ignore[assignment] - return parser - - -def _is_truthy(value: str) -> bool: - """Return ``True`` if *value* is a fail2ban boolean true string. - - Args: - value: Raw string from config (e.g. ``"true"``, ``"yes"``, ``"1"``). - - Returns: - ``True`` when the value represents enabled. - """ - return value.strip().lower() in _TRUE_VALUES - - -def _parse_multiline(raw: str) -> list[str]: - """Split a multi-line INI value into individual non-blank lines. - - Args: - raw: Raw multi-line string from configparser. - - Returns: - List of stripped, non-empty, non-comment strings. - """ - result: list[str] = [] - for line in raw.splitlines(): - stripped = line.strip() - if stripped and not stripped.startswith("#"): - result.append(stripped) - return result - # --------------------------------------------------------------------------- # Internal helpers @@ -329,7 +260,7 @@ def _append_jail_action_sync( local_path = jail_d / f"{jail_name}.local" - parser = _build_parser() + parser = build_parser() if local_path.is_file(): try: parser.read(str(local_path), encoding="utf-8") @@ -418,7 +349,7 @@ def _remove_jail_action_sync( if not local_path.is_file(): return - parser = _build_parser() + parser = build_parser() try: parser.read(str(local_path), encoding="utf-8") except (configparser.Error, OSError) as exc: @@ -554,8 +485,8 @@ async def list_actions( raw_actions: list[tuple[str, str, str, bool, str]] = await run_blocking( _parse_actions_sync, action_d) all_jails_result, active_names = await asyncio.gather( - run_blocking( _parse_jails_sync, Path(config_dir)), - _get_active_jail_names(socket_path), + run_blocking(parse_jails_sync, Path(config_dir)), + get_active_jail_names(socket_path), ) all_jails, _source_files = all_jails_result @@ -652,8 +583,8 @@ async def get_action( cfg = conffile_parser.parse_action_file(content, name=base_name, filename=f"{base_name}.conf") all_jails_result, active_names = await asyncio.gather( - run_blocking( _parse_jails_sync, Path(config_dir)), - _get_active_jail_names(socket_path), + run_blocking(parse_jails_sync, Path(config_dir)), + get_active_jail_names(socket_path), ) all_jails, _source_files = all_jails_result action_to_jails = _build_action_to_jails_map(all_jails, active_names) @@ -738,7 +669,7 @@ async def update_action( if do_reload: try: - await reload_jails(socket_path) + await reload_all(socket_path) except Exception as exc: # noqa: BLE001 log.warning( "reload_after_action_update_failed", @@ -806,7 +737,7 @@ async def create_action( if do_reload: try: - await reload_jails(socket_path) + await reload_all(socket_path) except Exception as exc: # noqa: BLE001 log.warning( "reload_after_action_create_failed", @@ -898,11 +829,11 @@ async def assign_action_to_jail( ``action.d/``. ConfigWriteError: If writing fails. """ - _safe_jail_name(jail_name) + safe_jail_name(jail_name) _safe_action_name(req.action_name) - all_jails, _src = await run_blocking( _parse_jails_sync, Path(config_dir)) + all_jails, _src = await run_blocking(parse_jails_sync, Path(config_dir)) if jail_name not in all_jails: raise JailNotFoundInConfigError(jail_name) @@ -932,7 +863,7 @@ async def assign_action_to_jail( if do_reload: try: - await reload_jails(socket_path) + await reload_all(socket_path) except Exception as exc: # noqa: BLE001 log.warning( "reload_after_assign_action_failed", @@ -975,11 +906,11 @@ async def remove_action_from_jail( JailNotFoundError: If *jail_name* is not defined in any config. ConfigWriteError: If writing fails. """ - _safe_jail_name(jail_name) + safe_jail_name(jail_name) _safe_action_name(action_name) - all_jails, _src = await run_blocking( _parse_jails_sync, Path(config_dir)) + all_jails, _src = await run_blocking(parse_jails_sync, Path(config_dir)) if jail_name not in all_jails: raise JailNotFoundInConfigError(jail_name) @@ -991,7 +922,7 @@ async def remove_action_from_jail( if do_reload: try: - await reload_jails(socket_path) + await reload_all(socket_path) except Exception as exc: # noqa: BLE001 log.warning( "reload_after_remove_action_failed", diff --git a/backend/app/services/config_file_service.py b/backend/app/services/config_file_service.py index ddbf11f..32264c2 100644 --- a/backend/app/services/config_file_service.py +++ b/backend/app/services/config_file_service.py @@ -49,7 +49,7 @@ from app.exceptions import ( JailNotFoundError, JailNotFoundInConfigError, ) -from app.helpers.jail_helpers import reload_jails +from app.services.jail_service import reload_all from app.models.config import ( ActionConfig, ActionConfigUpdate, @@ -96,7 +96,7 @@ class _JailServiceProxy: kwargs["include_jails"] = include_jails if exclude_jails is not None: kwargs["exclude_jails"] = exclude_jails - await reload_jails(socket_path, **kwargs) + await reload_all(socket_path, **kwargs) jail_service = _JailServiceProxy() @@ -114,7 +114,7 @@ async def _reload_all( if exclude_jails is not None: kwargs["exclude_jails"] = exclude_jails - await jail_service.reload_all(socket_path, **kwargs) + await reload_all(socket_path, **kwargs) # --------------------------------------------------------------------------- @@ -1003,6 +1003,20 @@ def _set_jail_local_key_sync( ) +# Public shared helpers for config file services. +ordered_config_files = _ordered_config_files +build_parser = _build_parser +is_truthy = _is_truthy +parse_multiline = _parse_multiline +parse_jails_sync = _parse_jails_sync +build_inactive_jail = _build_inactive_jail +get_active_jail_names = _get_active_jail_names +validate_jail_config_sync = _validate_jail_config_sync +set_jail_local_key_sync = _set_jail_local_key_sync +safe_jail_name = _safe_jail_name +safe_filter_name = _safe_filter_name + + # --------------------------------------------------------------------------- # Public API # --------------------------------------------------------------------------- diff --git a/backend/app/services/config_service.py b/backend/app/services/config_service.py index cb8c27e..a11965b 100644 --- a/backend/app/services/config_service.py +++ b/backend/app/services/config_service.py @@ -30,8 +30,8 @@ if TYPE_CHECKING: from app import __version__ from app.exceptions import ConfigOperationError, ConfigValidationError, JailNotFoundError -from app.helpers.log_helpers import preview_log as util_preview_log -from app.helpers.log_helpers import test_regex as util_test_regex +from app.services.log_service import preview_log as util_preview_log +from app.services.log_service import test_regex as util_test_regex from app.models.config import ( AddLogPathRequest, BantimeEscalation, diff --git a/backend/app/services/filter_config_service.py b/backend/app/services/filter_config_service.py index 143516e..2593bbd 100644 --- a/backend/app/services/filter_config_service.py +++ b/backend/app/services/filter_config_service.py @@ -7,10 +7,7 @@ for fail2ban filter configurations. from __future__ import annotations import asyncio -from app.utils.async_utils import run_blocking -import configparser import contextlib -import io import os import re import tempfile @@ -22,17 +19,18 @@ from app.exceptions import ( ConfigWriteError, FilterAlreadyExistsError, FilterInvalidRegexError, - FilterNameError, FilterNotFoundError, FilterReadonlyError, - JailNameError, JailNotFoundInConfigError, ) -from app.helpers.config_file_helpers import ( - _get_active_jail_names, - _parse_jails_sync, +from app.services.config_file_service import ( + get_active_jail_names, + parse_jails_sync, + safe_filter_name, + safe_jail_name, + set_jail_local_key_sync, ) -from app.helpers.jail_helpers import reload_jails +from app.services.jail_service import reload_all from app.models.config import ( AssignFilterRequest, FilterConfig, @@ -41,8 +39,8 @@ from app.models.config import ( FilterListResponse, FilterUpdateRequest, ) -from app.services.config_file_service import _TRUE_VALUES from app.utils import conffile_parser +from app.utils.async_utils import run_blocking log: structlog.stdlib.BoundLogger = structlog.get_logger() @@ -51,93 +49,6 @@ log: structlog.stdlib.BoundLogger = structlog.get_logger() # --------------------------------------------------------------------------- -_SAFE_FILTER_NAME_RE: re.Pattern[str] = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") -_SAFE_JAIL_NAME_RE: re.Pattern[str] = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") - - -def _safe_filter_name(name: str) -> str: - """Validate *name* and return it unchanged or raise :class:`FilterNameError`. - - Args: - name: Proposed filter name (without extension). - - Returns: - The name unchanged if valid. - - Raises: - FilterNameError: If *name* contains unsafe characters. - """ - if not _SAFE_FILTER_NAME_RE.match(name): - raise FilterNameError( - f"Filter name {name!r} contains invalid characters. " - "Only alphanumeric characters, hyphens, underscores, and dots are " - "allowed; must start with an alphanumeric character." - ) - return name - - -def _safe_jail_name(name: str) -> str: - """Validate *name* and return it unchanged or raise :class:`JailNameError`. - - Args: - name: Proposed jail name. - - Returns: - The name unchanged if valid. - - Raises: - JailNameError: If *name* contains unsafe characters. - """ - if not _SAFE_JAIL_NAME_RE.match(name): - raise JailNameError( - f"Jail name {name!r} contains invalid characters. " - "Only alphanumeric characters, hyphens, underscores, and dots are " - "allowed; must start with an alphanumeric character." - ) - return name - - -def _build_parser() -> configparser.RawConfigParser: - """Create a :class:`configparser.RawConfigParser` for fail2ban configs. - - Returns: - Parser with interpolation disabled and case-sensitive option names. - """ - parser = configparser.RawConfigParser(interpolation=None, strict=False) - # fail2ban keys are lowercase but preserve case to be safe. - parser.optionxform = str # type: ignore[assignment] - return parser - - -def _is_truthy(value: str) -> bool: - """Return ``True`` if *value* is a fail2ban boolean true string. - - Args: - value: Raw string from config (e.g. ``"true"``, ``"yes"``, ``"1"``). - - Returns: - ``True`` when the value represents enabled. - """ - return value.strip().lower() in _TRUE_VALUES - - -def _parse_multiline(raw: str) -> list[str]: - """Split a multi-line INI value into individual non-blank lines. - - Args: - raw: Raw multi-line string from configparser. - - Returns: - List of stripped, non-empty, non-comment strings. - """ - result: list[str] = [] - for line in raw.splitlines(): - stripped = line.strip() - if stripped and not stripped.startswith("#"): - result.append(stripped) - return result - - def _resolve_filter(raw_filter: str, jail_name: str, mode: str) -> str: """Resolve fail2ban variable placeholders in a filter string. @@ -156,87 +67,11 @@ def _resolve_filter(raw_filter: str, jail_name: str, mode: str) -> str: result = result.replace("%(mode)s", mode) return result - # --------------------------------------------------------------------------- # Internal helpers - from config_file_service for local use # --------------------------------------------------------------------------- -def _set_jail_local_key_sync( - config_dir: Path, - jail_name: str, - key: str, - value: str, -) -> None: - """Update ``jail.d/{jail_name}.local`` to set a single key in the jail section. - - If the ``.local`` file already exists it is read, the key is updated (or - added), and the file is written back atomically without disturbing other - settings. If the file does not exist a new one is created containing - only the BanGUI header comment, the jail section, and the requested key. - - Args: - config_dir: The fail2ban configuration root directory. - jail_name: Validated jail name (used as section name and filename stem). - key: Config key to set inside the jail section. - value: Config value to assign. - - Raises: - ConfigWriteError: If writing fails. - """ - jail_d = config_dir / "jail.d" - try: - jail_d.mkdir(parents=True, exist_ok=True) - except OSError as exc: - raise ConfigWriteError(f"Cannot create jail.d directory: {exc}") from exc - - local_path = jail_d / f"{jail_name}.local" - - parser = _build_parser() - if local_path.is_file(): - try: - parser.read(str(local_path), encoding="utf-8") - except (configparser.Error, OSError) as exc: - log.warning( - "jail_local_read_for_update_error", - jail=jail_name, - error=str(exc), - ) - - if not parser.has_section(jail_name): - parser.add_section(jail_name) - parser.set(jail_name, key, value) - - # Serialize: write a BanGUI header then the parser output. - buf = io.StringIO() - buf.write("# Managed by BanGUI — do not edit manually\n\n") - parser.write(buf) - content = buf.getvalue() - - try: - with tempfile.NamedTemporaryFile( - mode="w", - encoding="utf-8", - dir=jail_d, - delete=False, - suffix=".tmp", - ) as tmp: - tmp.write(content) - tmp_name = tmp.name - os.replace(tmp_name, local_path) - except OSError as exc: - with contextlib.suppress(OSError): - os.unlink(tmp_name) # noqa: F821 - raise ConfigWriteError(f"Failed to write {local_path}: {exc}") from exc - - log.info( - "jail_local_key_set", - jail=jail_name, - key=key, - path=str(local_path), - ) - - def _extract_filter_base_name(filter_raw: str) -> str: """Extract the base filter name from a raw fail2ban filter string. @@ -461,15 +296,14 @@ async def list_filters( ``used_by_jails`` lists. """ filter_d = Path(config_dir) / "filter.d" - loop = asyncio.get_event_loop() # Run the synchronous scan in a thread-pool executor. raw_filters: list[tuple[str, str, str, bool, str]] = await run_blocking( _parse_filters_sync, filter_d) # Fetch active jail names and their configs concurrently. all_jails_result, active_names = await asyncio.gather( - run_blocking( _parse_jails_sync, Path(config_dir)), - _get_active_jail_names(socket_path), + run_blocking(parse_jails_sync, Path(config_dir)), + get_active_jail_names(socket_path), ) all_jails, _source_files = all_jails_result @@ -538,7 +372,6 @@ async def get_filter( filter_d = Path(config_dir) / "filter.d" conf_path = filter_d / f"{base_name}.conf" local_path = filter_d / f"{base_name}.local" - loop = asyncio.get_event_loop() def _read() -> tuple[str, bool, str]: """Read filter content and return (content, has_local_override, source_path).""" @@ -568,8 +401,8 @@ async def get_filter( cfg = conffile_parser.parse_filter_file(content, name=base_name, filename=f"{base_name}.conf") all_jails_result, active_names = await asyncio.gather( - run_blocking( _parse_jails_sync, Path(config_dir)), - _get_active_jail_names(socket_path), + run_blocking(parse_jails_sync, Path(config_dir)), + get_active_jail_names(socket_path), ) all_jails, _source_files = all_jails_result filter_to_jails = _build_filter_to_jails_map(all_jails, active_names) @@ -634,7 +467,7 @@ async def update_filter( ConfigWriteError: If writing the ``.local`` file fails. """ base_name = name[:-5] if name.endswith(".conf") or name.endswith(".local") else name - _safe_filter_name(base_name) + safe_filter_name(base_name) # Validate regex patterns before touching the filesystem. patterns: list[str] = [] @@ -659,12 +492,11 @@ async def update_filter( content = conffile_parser.serialize_filter_config(merged) filter_d = Path(config_dir) / "filter.d" - loop = asyncio.get_event_loop() await run_blocking( _write_filter_local_sync, filter_d, base_name, content) if do_reload: try: - await reload_jails(socket_path) + await reload_all(socket_path) except Exception as exc: # noqa: BLE001 log.warning( "reload_after_filter_update_failed", @@ -706,7 +538,7 @@ async def create_filter( FilterInvalidRegexError: If any regex pattern is invalid. ConfigWriteError: If writing fails. """ - _safe_filter_name(req.name) + safe_filter_name(req.name) filter_d = Path(config_dir) / "filter.d" conf_path = filter_d / f"{req.name}.conf" @@ -716,7 +548,6 @@ async def create_filter( if conf_path.is_file() or local_path.is_file(): raise FilterAlreadyExistsError(req.name) - loop = asyncio.get_event_loop() await run_blocking( _check_not_exists) # Validate regex patterns. @@ -739,7 +570,7 @@ async def create_filter( if do_reload: try: - await reload_jails(socket_path) + await reload_all(socket_path) except Exception as exc: # noqa: BLE001 log.warning( "reload_after_filter_create_failed", @@ -776,14 +607,12 @@ async def delete_filter( ConfigWriteError: If deletion of the ``.local`` file fails. """ base_name = name[:-5] if name.endswith(".conf") or name.endswith(".local") else name - _safe_filter_name(base_name) + safe_filter_name(base_name) filter_d = Path(config_dir) / "filter.d" conf_path = filter_d / f"{base_name}.conf" local_path = filter_d / f"{base_name}.local" - loop = asyncio.get_event_loop() - def _delete() -> None: has_conf = conf_path.is_file() has_local = local_path.is_file() @@ -833,13 +662,11 @@ async def assign_filter_to_jail( ``filter.d/``. ConfigWriteError: If writing fails. """ - _safe_jail_name(jail_name) - _safe_filter_name(req.filter_name) - - loop = asyncio.get_event_loop() + safe_jail_name(jail_name) + safe_filter_name(req.filter_name) # Verify the jail exists in config. - all_jails, _src = await run_blocking( _parse_jails_sync, Path(config_dir)) + all_jails, _src = await run_blocking(parse_jails_sync, Path(config_dir)) if jail_name not in all_jails: raise JailNotFoundInConfigError(jail_name) @@ -854,7 +681,7 @@ async def assign_filter_to_jail( await run_blocking( _check_filter) - await run_blocking(_set_jail_local_key_sync, + await run_blocking(set_jail_local_key_sync, Path(config_dir), jail_name, "filter", @@ -863,7 +690,7 @@ async def assign_filter_to_jail( if do_reload: try: - await reload_jails(socket_path) + await reload_all(socket_path) except Exception as exc: # noqa: BLE001 log.warning( "reload_after_assign_filter_failed", diff --git a/backend/app/services/jail_config_service.py b/backend/app/services/jail_config_service.py index bae61e4..139c6b2 100644 --- a/backend/app/services/jail_config_service.py +++ b/backend/app/services/jail_config_service.py @@ -9,9 +9,7 @@ overrides in jail.d/*.local files. from __future__ import annotations import asyncio -import configparser import contextlib -import io import os import re import tempfile @@ -24,17 +22,19 @@ from app.exceptions import ( ConfigWriteError, JailAlreadyActiveError, JailAlreadyInactiveError, - JailNameError, JailNotFoundError, JailNotFoundInConfigError, ) -from app.helpers.config_file_helpers import ( - _build_inactive_jail, - _get_active_jail_names, - _parse_jails_sync, - _validate_jail_config_sync, +from app.services.config_file_service import ( + build_inactive_jail, + get_active_jail_names, + parse_jails_sync, + safe_jail_name, + start_daemon, + validate_jail_config_sync, + wait_for_fail2ban, ) -from app.helpers.jail_helpers import reload_jails +from app.services.jail_service import reload_all from app.models.config import ( ActivateJailRequest, InactiveJail, @@ -64,18 +64,9 @@ log: structlog.stdlib.BoundLogger = structlog.get_logger() _SOCKET_TIMEOUT: float = 10.0 -# Allowlist pattern for jail names used in path construction. -_SAFE_JAIL_NAME_RE: re.Pattern[str] = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") - # Sections that are not jail definitions. _META_SECTIONS: frozenset[str] = frozenset({"INCLUDES", "DEFAULT"}) -# True-ish values for the ``enabled`` key. -_TRUE_VALUES: frozenset[str] = frozenset({"true", "yes", "1"}) - -# False-ish values for the ``enabled`` key. -_FALSE_VALUES: frozenset[str] = frozenset({"false", "no", "0"}) - # Seconds to wait between fail2ban liveness probes after a reload. _POST_RELOAD_PROBE_INTERVAL: float = 2.0 @@ -88,51 +79,6 @@ _POST_RELOAD_MAX_ATTEMPTS: int = 4 # --------------------------------------------------------------------------- -def _safe_jail_name(name: str) -> str: - """Validate *name* and return it unchanged or raise :class:`JailNameError`. - - Args: - name: Proposed jail name. - - Returns: - The name unchanged if valid. - - Raises: - JailNameError: If *name* contains unsafe characters. - """ - if not _SAFE_JAIL_NAME_RE.match(name): - raise JailNameError( - f"Jail name {name!r} contains invalid characters. " - "Only alphanumeric characters, hyphens, underscores, and dots are " - "allowed; must start with an alphanumeric character." - ) - return name - - -def _build_parser() -> configparser.RawConfigParser: - """Create a :class:`configparser.RawConfigParser` for fail2ban configs. - - Returns: - Parser with interpolation disabled and case-sensitive option names. - """ - parser = configparser.RawConfigParser(interpolation=None, strict=False) - # fail2ban keys are lowercase but preserve case to be safe. - parser.optionxform = str # type: ignore[assignment] - return parser - - -def _is_truthy(value: str) -> bool: - """Return ``True`` if *value* is a fail2ban boolean true string. - - Args: - value: Raw string from config (e.g. ``"true"``, ``"yes"``, ``"1"``). - - Returns: - ``True`` when the value represents enabled. - """ - return value.strip().lower() in _TRUE_VALUES - - def _write_local_override_sync( config_dir: Path, jail_name: str, @@ -275,81 +221,6 @@ def _validate_regex_patterns(patterns: list[str]) -> None: raise FilterInvalidRegexError(pattern, str(exc)) from exc -def _set_jail_local_key_sync( - config_dir: Path, - jail_name: str, - key: str, - value: str, -) -> None: - """Update ``jail.d/{jail_name}.local`` to set a single key in the jail section. - - If the ``.local`` file already exists it is read, the key is updated (or - added), and the file is written back atomically without disturbing other - settings. If the file does not exist a new one is created containing - only the BanGUI header comment, the jail section, and the requested key. - - Args: - config_dir: The fail2ban configuration root directory. - jail_name: Validated jail name (used as section name and filename stem). - key: Config key to set inside the jail section. - value: Config value to assign. - - Raises: - ConfigWriteError: If writing fails. - """ - jail_d = config_dir / "jail.d" - try: - jail_d.mkdir(parents=True, exist_ok=True) - except OSError as exc: - raise ConfigWriteError(f"Cannot create jail.d directory: {exc}") from exc - - local_path = jail_d / f"{jail_name}.local" - - parser = _build_parser() - if local_path.is_file(): - try: - parser.read(str(local_path), encoding="utf-8") - except (configparser.Error, OSError) as exc: - log.warning( - "jail_local_read_for_update_error", - jail=jail_name, - error=str(exc), - ) - - if not parser.has_section(jail_name): - parser.add_section(jail_name) - parser.set(jail_name, key, value) - - # Serialize: write a BanGUI header then the parser output. - buf = io.StringIO() - buf.write("# Managed by BanGUI — do not edit manually\n\n") - parser.write(buf) - content = buf.getvalue() - - try: - with tempfile.NamedTemporaryFile( - mode="w", - encoding="utf-8", - dir=jail_d, - delete=False, - suffix=".tmp", - ) as tmp: - tmp.write(content) - tmp_name = tmp.name - os.replace(tmp_name, local_path) - except OSError as exc: - with contextlib.suppress(OSError): - os.unlink(tmp_name) # noqa: F821 - raise ConfigWriteError(f"Failed to write {local_path}: {exc}") from exc - - log.info( - "jail_local_key_set", - jail=jail_name, - key=key, - path=str(local_path), - ) - - async def _probe_fail2ban_running(socket_path: str) -> bool: """Return ``True`` if the fail2ban socket responds to a ping. @@ -367,67 +238,8 @@ async def _probe_fail2ban_running(socket_path: str) -> bool: return False -async def wait_for_fail2ban( - socket_path: str, - max_wait_seconds: float = 10.0, - poll_interval: float = 2.0, -) -> bool: - """Poll the fail2ban socket until it responds or the timeout expires. - - Args: - socket_path: Path to the fail2ban Unix domain socket. - max_wait_seconds: Total time budget in seconds. - poll_interval: Delay between probe attempts in seconds. - - Returns: - ``True`` if fail2ban came online within the budget. - """ - elapsed = 0.0 - while elapsed < max_wait_seconds: - if await _probe_fail2ban_running(socket_path): - return True - await asyncio.sleep(poll_interval) - elapsed += poll_interval - return False - - -async def start_daemon(start_cmd_parts: list[str]) -> bool: - """Start the fail2ban daemon using *start_cmd_parts*. - - Uses :func:`asyncio.create_subprocess_exec` (no shell interpretation) - to avoid command injection. - - Args: - start_cmd_parts: Command and arguments, e.g. - ``["fail2ban-client", "start"]``. - - Returns: - ``True`` when the process exited with code 0. - """ - if not start_cmd_parts: - log.warning("fail2ban_start_cmd_empty") - return False - try: - proc = await asyncio.create_subprocess_exec( - *start_cmd_parts, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - await asyncio.wait_for(proc.wait(), timeout=30.0) - success = proc.returncode == 0 - if not success: - log.warning( - "fail2ban_start_cmd_nonzero", - cmd=start_cmd_parts, - returncode=proc.returncode, - ) - return success - except (TimeoutError, OSError) as exc: - log.warning("fail2ban_start_cmd_error", cmd=start_cmd_parts, error=str(exc)) - return False - - -# Shared functions from config_file_service are imported from app.helpers.config_file_helpers +# Shared functions from config_file_service are imported directly from the +# canonical shared helper module. # --------------------------------------------------------------------------- @@ -458,11 +270,11 @@ async def list_inactive_jails( inactive jails. """ parsed_result: tuple[dict[str, dict[str, str]], dict[str, str]] = await run_blocking( - _parse_jails_sync, + parse_jails_sync, Path(config_dir), ) all_jails, source_files = parsed_result - active_names: set[str] = await _get_active_jail_names(socket_path) + active_names: set[str] = await get_active_jail_names(socket_path) inactive: list[InactiveJail] = [] for jail_name, settings in sorted(all_jails.items()): @@ -471,7 +283,7 @@ async def list_inactive_jails( continue source = source_files.get(jail_name, config_dir) - inactive.append(_build_inactive_jail(jail_name, settings, source, Path(config_dir))) + inactive.append(build_inactive_jail(jail_name, settings, source, Path(config_dir))) log.info( "inactive_jails_listed", @@ -541,21 +353,21 @@ async def _activate_jail( ~app.utils.fail2ban_client.Fail2BanConnectionError: If the fail2ban socket is unreachable during reload. """ - _safe_jail_name(name) + safe_jail_name(name) - all_jails, _source_files = await run_blocking( _parse_jails_sync, Path(config_dir)) + all_jails, _source_files = await run_blocking(parse_jails_sync, Path(config_dir)) if name not in all_jails: raise JailNotFoundInConfigError(name) - active_names = await _get_active_jail_names(socket_path) + active_names = await get_active_jail_names(socket_path) if name in active_names: raise JailAlreadyActiveError(name) # ---------------------------------------------------------------------- # # Pre-activation validation — collect warnings but do not block # # ---------------------------------------------------------------------- # - validation_result: JailValidationResult = await run_blocking(_validate_jail_config_sync, Path(config_dir), name + validation_result: JailValidationResult = await run_blocking(validate_jail_config_sync, Path(config_dir), name ) warnings: list[str] = [f"{i.field}: {i.message}" for i in validation_result.issues] if warnings: @@ -609,7 +421,7 @@ async def _activate_jail( # Activation reload — if it fails, roll back immediately # # ---------------------------------------------------------------------- # try: - await reload_jails(socket_path, include_jails=[name]) + await reload_all(socket_path, include_jails=[name]) except JailNotFoundError as exc: # Jail configuration is invalid (e.g. missing logpath that prevents # fail2ban from loading the jail). Roll back and provide a specific error. @@ -680,7 +492,7 @@ async def _activate_jail( ) # Verify the jail actually started (config error may prevent it silently). - post_reload_names = await _get_active_jail_names(socket_path) + post_reload_names = await get_active_jail_names(socket_path) actually_running = name in post_reload_names if not actually_running: log.warning( @@ -751,7 +563,7 @@ async def _rollback_activation_async( # Step 2 — reload fail2ban with the restored config. try: - await reload_jails(socket_path) + await reload_all(socket_path) log.info("jail_activation_rollback_reload_ok", jail=name) except Exception as exc: # noqa: BLE001 log.warning("jail_activation_rollback_reload_failed", jail=name, error=str(exc)) @@ -813,14 +625,14 @@ async def _deactivate_jail( ~app.utils.fail2ban_client.Fail2BanConnectionError: If the fail2ban socket is unreachable during reload. """ - _safe_jail_name(name) + safe_jail_name(name) - all_jails, _source_files = await run_blocking( _parse_jails_sync, Path(config_dir)) + all_jails, _source_files = await run_blocking(parse_jails_sync, Path(config_dir)) if name not in all_jails: raise JailNotFoundInConfigError(name) - active_names = await _get_active_jail_names(socket_path) + active_names = await get_active_jail_names(socket_path) if name not in active_names: raise JailAlreadyInactiveError(name) @@ -832,7 +644,7 @@ async def _deactivate_jail( ) try: - await reload_jails(socket_path, exclude_jails=[name]) + await reload_all(socket_path, exclude_jails=[name]) except Exception as exc: # noqa: BLE001 log.warning("reload_after_deactivate_failed", jail=name, error=str(exc)) @@ -868,14 +680,14 @@ async def delete_jail_local_override( delete the live config file). ConfigWriteError: If the file cannot be deleted. """ - _safe_jail_name(name) + safe_jail_name(name) - all_jails, _source_files = await run_blocking( _parse_jails_sync, Path(config_dir)) + all_jails, _source_files = await run_blocking(parse_jails_sync, Path(config_dir)) if name not in all_jails: raise JailNotFoundInConfigError(name) - active_names = await _get_active_jail_names(socket_path) + active_names = await get_active_jail_names(socket_path) if name in active_names: raise JailAlreadyActiveError(name) @@ -908,8 +720,8 @@ async def validate_jail_config( Raises: JailNameError: If *name* contains invalid characters. """ - _safe_jail_name(name) - return await run_blocking(_validate_jail_config_sync, + safe_jail_name(name) + return await run_blocking(validate_jail_config_sync, Path(config_dir), name, ) @@ -958,7 +770,7 @@ async def _rollback_jail( JailNameError: If *name* contains invalid characters. ConfigWriteError: If writing the ``.local`` file fails. """ - _safe_jail_name(name) + safe_jail_name(name) # Write enabled=false — this must succeed even when fail2ban is down. @@ -979,7 +791,7 @@ async def _rollback_jail( active_jails = 0 if fail2ban_running: - names = await _get_active_jail_names(socket_path) + names = await get_active_jail_names(socket_path) active_jails = len(names) if fail2ban_running: