Remove helper indirection and import shared service helpers directly
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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",
|
||||
]
|
||||
@@ -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,
|
||||
)
|
||||
@@ -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)
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user