From d4d04491d26129a808d049eb49cf4596d50da9e2 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 13:41:00 +0100 Subject: [PATCH] Add Deactivate Jail button for inactive jails with local override - Add has_local_override field to InactiveJail model - Update _build_inactive_jail and list_inactive_jails to compute the field - Add delete_jail_local_override() service function - Add DELETE /api/config/jails/{name}/local router endpoint - Surface has_local_override in frontend InactiveJail type - Show Deactivate Jail button in JailsTab when has_local_override is true - Add tests: TestBuildInactiveJail, TestListInactiveJails, TestDeleteJailLocalOverride --- backend/app/models/config.py | 8 + backend/app/routers/config.py | 54 +++++ backend/app/services/config_file_service.py | 61 +++++- .../test_services/test_config_file_service.py | 195 ++++++++++++++++++ frontend/src/api/config.ts | 32 ++- frontend/src/api/endpoints.ts | 5 +- frontend/src/components/config/JailsTab.tsx | 41 +++- .../__tests__/ActivateJailDialog.test.tsx | 29 +-- frontend/src/types/config.ts | 19 +- 9 files changed, 367 insertions(+), 77 deletions(-) diff --git a/backend/app/models/config.py b/backend/app/models/config.py index b336018..3f2569a 100644 --- a/backend/app/models/config.py +++ b/backend/app/models/config.py @@ -807,6 +807,14 @@ class InactiveJail(BaseModel): "inactive jails that appear in this list." ), ) + has_local_override: bool = Field( + default=False, + description=( + "``True`` when a ``jail.d/{name}.local`` file exists for this jail. " + "Only meaningful for inactive jails; indicates that a cleanup action " + "is available." + ), + ) class InactiveJailListResponse(BaseModel): diff --git a/backend/app/routers/config.py b/backend/app/routers/config.py index 9ab0da7..8bee91d 100644 --- a/backend/app/routers/config.py +++ b/backend/app/routers/config.py @@ -798,6 +798,60 @@ async def deactivate_jail( return result +@router.delete( + "/jails/{name}/local", + status_code=status.HTTP_204_NO_CONTENT, + summary="Delete the jail.d override file for an inactive jail", +) +async def delete_jail_local_override( + request: Request, + _auth: AuthDep, + name: _NamePath, +) -> None: + """Remove the ``jail.d/{name}.local`` override file for an inactive jail. + + This endpoint is the clean-up action for inactive jails that still carry + a ``.local`` override file (e.g. one written with ``enabled = false`` by a + previous deactivation). The file is deleted without modifying fail2ban's + running state, since the jail is already inactive. + + Args: + request: FastAPI request object. + _auth: Validated session. + name: Name of the jail whose ``.local`` file should be removed. + + Raises: + HTTPException: 400 if *name* contains invalid characters. + HTTPException: 404 if *name* is not found in any config file. + HTTPException: 409 if the jail is currently active. + HTTPException: 500 if the file cannot be deleted. + HTTPException: 502 if fail2ban is unreachable. + """ + config_dir: str = request.app.state.settings.fail2ban_config_dir + socket_path: str = request.app.state.settings.fail2ban_socket + + try: + await config_file_service.delete_jail_local_override( + config_dir, socket_path, name + ) + except JailNameError as exc: + raise _bad_request(str(exc)) from exc + except JailNotFoundInConfigError: + raise _not_found(name) from None + except JailAlreadyActiveError: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"Jail {name!r} is currently active; deactivate it first.", + ) from None + except ConfigWriteError as exc: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to delete config override: {exc}", + ) from exc + except Fail2BanConnectionError as exc: + raise _bad_gateway(exc) from exc + + # --------------------------------------------------------------------------- # Jail validation & rollback endpoints (Task 3) # --------------------------------------------------------------------------- diff --git a/backend/app/services/config_file_service.py b/backend/app/services/config_file_service.py index 9169bfc..b5dc1eb 100644 --- a/backend/app/services/config_file_service.py +++ b/backend/app/services/config_file_service.py @@ -429,6 +429,7 @@ def _build_inactive_jail( name: str, settings: dict[str, str], source_file: str, + config_dir: Path | None = None, ) -> InactiveJail: """Construct an :class:`~app.models.config.InactiveJail` from raw settings. @@ -436,6 +437,8 @@ def _build_inactive_jail( name: Jail section name. settings: Merged key→value dict (DEFAULT values already applied). source_file: Path of the file that last defined this section. + config_dir: Absolute path to the fail2ban configuration directory, used + to check whether a ``jail.d/{name}.local`` override file exists. Returns: Populated :class:`~app.models.config.InactiveJail`. @@ -513,6 +516,11 @@ def _build_inactive_jail( bantime_escalation=bantime_escalation, source_file=source_file, enabled=enabled, + has_local_override=( + (config_dir / "jail.d" / f"{name}.local").is_file() + if config_dir is not None + else False + ), ) @@ -1111,7 +1119,7 @@ async def list_inactive_jails( continue source = source_files.get(jail_name, config_dir) - inactive.append(_build_inactive_jail(jail_name, settings, source)) + inactive.append(_build_inactive_jail(jail_name, settings, source, Path(config_dir))) log.info( "inactive_jails_listed", @@ -1469,6 +1477,57 @@ async def deactivate_jail( ) +async def delete_jail_local_override( + config_dir: str, + socket_path: str, + name: str, +) -> None: + """Delete the ``jail.d/{name}.local`` override file for an inactive jail. + + This is the clean-up action shown in the config UI when an inactive jail + still has a ``.local`` override file (e.g. ``enabled = false``). The + file is deleted outright; no fail2ban reload is required because the jail + is already inactive. + + Args: + config_dir: Absolute path to the fail2ban configuration directory. + socket_path: Path to the fail2ban Unix domain socket. + name: Name of the jail whose ``.local`` file should be removed. + + Raises: + JailNameError: If *name* contains invalid characters. + JailNotFoundInConfigError: If *name* is not defined in any config file. + JailAlreadyActiveError: If the jail is currently active (refusing to + delete the live config file). + ConfigWriteError: If the file cannot be deleted. + """ + _safe_jail_name(name) + + loop = asyncio.get_event_loop() + all_jails, _source_files = await loop.run_in_executor( + None, _parse_jails_sync, Path(config_dir) + ) + + if name not in all_jails: + raise JailNotFoundInConfigError(name) + + active_names = await _get_active_jail_names(socket_path) + if name in active_names: + raise JailAlreadyActiveError(name) + + local_path = Path(config_dir) / "jail.d" / f"{name}.local" + try: + await loop.run_in_executor( + None, lambda: local_path.unlink(missing_ok=True) + ) + except OSError as exc: + raise ConfigWriteError( + f"Failed to delete {local_path}: {exc}" + ) from exc + + log.info("jail_local_override_deleted", jail=name, path=str(local_path)) + + async def validate_jail_config( config_dir: str, name: str, diff --git a/backend/tests/test_services/test_config_file_service.py b/backend/tests/test_services/test_config_file_service.py index 5a9f80f..e648fe8 100644 --- a/backend/tests/test_services/test_config_file_service.py +++ b/backend/tests/test_services/test_config_file_service.py @@ -290,6 +290,28 @@ class TestBuildInactiveJail: jail = _build_inactive_jail("active-jail", settings, "/etc/fail2ban/jail.conf") assert jail.enabled is True + def test_has_local_override_absent(self, tmp_path: Path) -> None: + """has_local_override is False when no .local file exists.""" + jail = _build_inactive_jail( + "sshd", {}, "/etc/fail2ban/jail.d/sshd.conf", config_dir=tmp_path + ) + assert jail.has_local_override is False + + def test_has_local_override_present(self, tmp_path: Path) -> None: + """has_local_override is True when jail.d/{name}.local exists.""" + local = tmp_path / "jail.d" / "sshd.local" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_text("[sshd]\nenabled = false\n") + jail = _build_inactive_jail( + "sshd", {}, "/etc/fail2ban/jail.d/sshd.conf", config_dir=tmp_path + ) + assert jail.has_local_override is True + + def test_has_local_override_no_config_dir(self) -> None: + """has_local_override is False when config_dir is not provided.""" + jail = _build_inactive_jail("sshd", {}, "/etc/fail2ban/jail.conf") + assert jail.has_local_override is False + # --------------------------------------------------------------------------- # _write_local_override_sync @@ -425,6 +447,121 @@ class TestListInactiveJails: assert "sshd" in names assert "apache-auth" in names + async def test_has_local_override_true_when_local_file_exists( + self, tmp_path: Path + ) -> None: + """has_local_override is True for a jail whose jail.d .local file exists.""" + _write(tmp_path / "jail.conf", JAIL_CONF) + local = tmp_path / "jail.d" / "apache-auth.local" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_text("[apache-auth]\nenabled = false\n") + with patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ): + result = await list_inactive_jails(str(tmp_path), "/fake.sock") + jail = next(j for j in result.jails if j.name == "apache-auth") + assert jail.has_local_override is True + + async def test_has_local_override_false_when_no_local_file( + self, tmp_path: Path + ) -> None: + """has_local_override is False when no jail.d .local file exists.""" + _write(tmp_path / "jail.conf", JAIL_CONF) + with patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ): + result = await list_inactive_jails(str(tmp_path), "/fake.sock") + jail = next(j for j in result.jails if j.name == "apache-auth") + assert jail.has_local_override is False + + +# --------------------------------------------------------------------------- +# delete_jail_local_override +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestDeleteJailLocalOverride: + """Tests for :func:`~app.services.config_file_service.delete_jail_local_override`.""" + + async def test_deletes_local_file(self, tmp_path: Path) -> None: + """delete_jail_local_override removes the jail.d/.local file.""" + from app.services.config_file_service import delete_jail_local_override + + _write(tmp_path / "jail.conf", JAIL_CONF) + local = tmp_path / "jail.d" / "apache-auth.local" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_text("[apache-auth]\nenabled = false\n") + + with patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ): + await delete_jail_local_override(str(tmp_path), "/fake.sock", "apache-auth") + + assert not local.exists() + + async def test_no_error_when_local_file_missing(self, tmp_path: Path) -> None: + """delete_jail_local_override succeeds silently when no .local file exists.""" + from app.services.config_file_service import delete_jail_local_override + + _write(tmp_path / "jail.conf", JAIL_CONF) + with patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ): + # Must not raise even though there is no .local file. + await delete_jail_local_override(str(tmp_path), "/fake.sock", "apache-auth") + + async def test_raises_jail_not_found(self, tmp_path: Path) -> None: + """delete_jail_local_override raises JailNotFoundInConfigError for unknown jail.""" + from app.services.config_file_service import ( + JailNotFoundInConfigError, + delete_jail_local_override, + ) + + _write(tmp_path / "jail.conf", JAIL_CONF) + with ( + patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ), + pytest.raises(JailNotFoundInConfigError), + ): + await delete_jail_local_override(str(tmp_path), "/fake.sock", "nonexistent") + + async def test_raises_jail_already_active(self, tmp_path: Path) -> None: + """delete_jail_local_override raises JailAlreadyActiveError when jail is running.""" + from app.services.config_file_service import ( + JailAlreadyActiveError, + delete_jail_local_override, + ) + + _write(tmp_path / "jail.conf", JAIL_CONF) + local = tmp_path / "jail.d" / "sshd.local" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_text("[sshd]\nenabled = false\n") + with ( + patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value={"sshd"}), + ), + pytest.raises(JailAlreadyActiveError), + ): + await delete_jail_local_override(str(tmp_path), "/fake.sock", "sshd") + + async def test_raises_jail_name_error(self, tmp_path: Path) -> None: + """delete_jail_local_override raises JailNameError for invalid jail names.""" + from app.services.config_file_service import ( + JailNameError, + delete_jail_local_override, + ) + + with pytest.raises(JailNameError): + await delete_jail_local_override(str(tmp_path), "/fake.sock", "../evil") + # --------------------------------------------------------------------------- # activate_jail @@ -3174,6 +3311,64 @@ class TestActivateJailRollback: # Verify the error message mentions logpath issues. assert "logpath" in result.message.lower() or "check that all logpath" in result.message.lower() + async def test_activate_jail_rollback_deletes_file_when_no_prior_local( + self, tmp_path: Path + ) -> None: + """Rollback deletes the .local file when none existed before activation. + + When a jail had no .local override before activation, activate_jail + creates one with enabled = true. If reload then crashes, rollback must + delete that file (leaving the jail in the same state as before the + activation attempt). + + Expects: + - The .local file is absent after rollback. + - The response indicates recovered=True. + """ + from app.models.config import ActivateJailRequest, JailValidationResult + + _write(tmp_path / "jail.conf", JAIL_CONF) + (tmp_path / "jail.d").mkdir(parents=True, exist_ok=True) + local_path = tmp_path / "jail.d" / "apache-auth.local" + # No .local file exists before activation. + assert not local_path.exists() + + req = ActivateJailRequest() + reload_call_count = 0 + + async def reload_side_effect(socket_path: str, **kwargs: object) -> None: + nonlocal reload_call_count + reload_call_count += 1 + if reload_call_count == 1: + raise RuntimeError("fail2ban crashed") + # Recovery reload succeeds. + + with ( + patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ), + patch("app.services.config_file_service.jail_service") as mock_js, + patch( + "app.services.config_file_service._probe_fail2ban_running", + new=AsyncMock(return_value=True), + ), + patch( + "app.services.config_file_service._validate_jail_config_sync", + return_value=JailValidationResult( + jail_name="apache-auth", valid=True + ), + ), + ): + mock_js.reload_all = AsyncMock(side_effect=reload_side_effect) + result = await activate_jail( + str(tmp_path), "/fake.sock", "apache-auth", req + ) + + assert result.active is False + assert result.recovered is True + assert not local_path.exists() + # --------------------------------------------------------------------------- # rollback_jail diff --git a/frontend/src/api/config.ts b/frontend/src/api/config.ts index 738ae3e..2fe5d32 100644 --- a/frontend/src/api/config.ts +++ b/frontend/src/api/config.ts @@ -39,10 +39,8 @@ import type { LogPreviewResponse, MapColorThresholdsResponse, MapColorThresholdsUpdate, - PendingRecovery, RegexTestRequest, RegexTestResponse, - RollbackResponse, ServerSettingsResponse, ServerSettingsUpdate, JailFileConfig, @@ -552,6 +550,18 @@ export async function deactivateJail( ); } +/** + * Delete the ``jail.d/{name}.local`` override file for an inactive jail. + * + * Only valid when the jail is **not** currently active. Use this to clean up + * leftover ``.local`` files after a jail has been fully deactivated. + * + * @param name - The jail name. + */ +export async function deleteJailLocalOverride(name: string): Promise { + await del(ENDPOINTS.configJailLocalOverride(name)); +} + // --------------------------------------------------------------------------- // fail2ban log viewer (Task 2) // --------------------------------------------------------------------------- @@ -593,21 +603,3 @@ export async function validateJailConfig( ): Promise { return post(ENDPOINTS.configJailValidate(name), undefined); } - -/** - * Fetch the pending crash-recovery record, if any. - * - * Returns null when fail2ban is healthy and no recovery is pending. - */ -export async function fetchPendingRecovery(): Promise { - return get(ENDPOINTS.configPendingRecovery); -} - -/** - * Rollback a bad jail — disables it and attempts to restart fail2ban. - * - * @param name - Name of the jail to disable. - */ -export async function rollbackJail(name: string): Promise { - return post(ENDPOINTS.configJailRollback(name), undefined); -} diff --git a/frontend/src/api/endpoints.ts b/frontend/src/api/endpoints.ts index df6f9f1..0a43a2f 100644 --- a/frontend/src/api/endpoints.ts +++ b/frontend/src/api/endpoints.ts @@ -71,11 +71,10 @@ export const ENDPOINTS = { `/config/jails/${encodeURIComponent(name)}/activate`, configJailDeactivate: (name: string): string => `/config/jails/${encodeURIComponent(name)}/deactivate`, + configJailLocalOverride: (name: string): string => + `/config/jails/${encodeURIComponent(name)}/local`, configJailValidate: (name: string): string => `/config/jails/${encodeURIComponent(name)}/validate`, - configJailRollback: (name: string): string => - `/config/jails/${encodeURIComponent(name)}/rollback`, - configPendingRecovery: "/config/pending-recovery" as string, configGlobal: "/config/global", configReload: "/config/reload", configRestart: "/config/restart", diff --git a/frontend/src/components/config/JailsTab.tsx b/frontend/src/components/config/JailsTab.tsx index a776d8c..4b38b50 100644 --- a/frontend/src/components/config/JailsTab.tsx +++ b/frontend/src/components/config/JailsTab.tsx @@ -35,6 +35,7 @@ import { ApiError } from "../../api/client"; import { addLogPath, deactivateJail, + deleteJailLocalOverride, deleteLogPath, fetchInactiveJails, fetchJailConfigFileContent, @@ -573,7 +574,7 @@ function JailConfigDetail({ )} - {readOnly && (onActivate !== undefined || onValidate !== undefined) && ( + {readOnly && (onActivate !== undefined || onValidate !== undefined || onDeactivate !== undefined) && (
{onValidate !== undefined && ( + )} {onActivate !== undefined && (