feat: Task 3 — invalid jail config recovery (pre-validation, crash detection, rollback)

- Backend: extend activate_jail() with pre-validation and 4-attempt post-reload health probe; add validate_jail_config() and rollback_jail() service functions - Backend: new endpoints POST /api/config/jails/{name}/validate, GET /api/config/pending-recovery, POST /api/config/jails/{name}/rollback - Backend: extend JailActivationResponse with fail2ban_running + validation_warnings; add JailValidationIssue, JailValidationResult, PendingRecovery, RollbackResponse models - Backend: health_check task tracks last_activation and creates PendingRecovery record when fail2ban goes offline within 60 s of an activation - Backend: add fail2ban_start_command setting (configurable start cmd for rollback) - Frontend: ActivateJailDialog — pre-validation on open, crash-detected callback, extended spinner text during activation+verify - Frontend: JailsTab — Validate Config button for inactive jails, validation result panels (blocking errors + advisory warnings) - Frontend: RecoveryBanner component — polls pending-recovery, shows full-width alert with Disable & Restart / View Logs buttons - Frontend: MainLayout — mount RecoveryBanner at layout level - Tests: 19 new backend service tests (validate, rollback, filter/action parsing) + 6 health_check crash-detection tests + 11 router tests; 5 RecoveryBanner frontend tests; fix mock setup in existing activate_jail tests
2026-03-14 14:13:07 +01:00
parent ab11ece001
commit 0966f347c4
17 changed files with 1862 additions and 26 deletions
--- a/backend/tests/test_services/test_config_file_service.py
+++ b/backend/tests/test_services/test_config_file_service.py
@@ -443,6 +443,10 @@ class TestActivateJail:
                new=AsyncMock(side_effect=[set(), {"apache-auth"}]),
            ),
            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),
+            ),
        ):
            mock_js.reload_all = AsyncMock()
            result = await activate_jail(str(tmp_path), "/fake.sock", "apache-auth", req)
@@ -494,9 +498,13 @@ class TestActivateJail:
        with (
            patch(
                "app.services.config_file_service._get_active_jail_names",
-                new=AsyncMock(return_value=set()),
+                new=AsyncMock(side_effect=[set(), 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),
+            ),
        ):
            mock_js.reload_all = AsyncMock()
            await activate_jail(str(tmp_path), "/fake.sock", "apache-auth", req)
@@ -2513,6 +2521,10 @@ class TestActivateJailReloadArgs:
                new=AsyncMock(side_effect=[set(), {"apache-auth"}]),
            ),
            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),
+            ),
        ):
            mock_js.reload_all = AsyncMock()
            await activate_jail(str(tmp_path), "/fake.sock", "apache-auth", req)
@@ -2535,6 +2547,10 @@ class TestActivateJailReloadArgs:
                new=AsyncMock(side_effect=[set(), {"apache-auth"}]),
            ),
            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),
+            ),
        ):
            mock_js.reload_all = AsyncMock()
            result = await activate_jail(
@@ -2558,12 +2574,17 @@ class TestActivateJailReloadArgs:

        req = ActivateJailRequest()
        # Pre-reload: jail not running.  Post-reload: still not running (boot failed).
+        # fail2ban is up (probe succeeds) but the jail didn't appear.
        with (
            patch(
                "app.services.config_file_service._get_active_jail_names",
                new=AsyncMock(side_effect=[set(), 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),
+            ),
        ):
            mock_js.reload_all = AsyncMock()
            result = await activate_jail(
@@ -2600,3 +2621,212 @@ class TestDeactivateJailReloadArgs:
            "/fake.sock", exclude_jails=["sshd"]
        )

+
+# ---------------------------------------------------------------------------
+# _validate_jail_config_sync (Task 3)
+# ---------------------------------------------------------------------------
+
+from app.services.config_file_service import (  # noqa: E402  (added after block)
+    _validate_jail_config_sync,
+    _extract_filter_base_name,
+    _extract_action_base_name,
+    validate_jail_config,
+    rollback_jail,
+)
+
+
+class TestExtractFilterBaseName:
+    def test_plain_name(self) -> None:
+        assert _extract_filter_base_name("sshd") == "sshd"
+
+    def test_strips_mode_suffix(self) -> None:
+        assert _extract_filter_base_name("sshd[mode=aggressive]") == "sshd"
+
+    def test_strips_whitespace(self) -> None:
+        assert _extract_filter_base_name("  nginx  ") == "nginx"
+
+
+class TestExtractActionBaseName:
+    def test_plain_name(self) -> None:
+        assert _extract_action_base_name("iptables-multiport") == "iptables-multiport"
+
+    def test_strips_option_suffix(self) -> None:
+        assert _extract_action_base_name("iptables[name=SSH]") == "iptables"
+
+    def test_returns_none_for_variable_interpolation(self) -> None:
+        assert _extract_action_base_name("%(action_)s") is None
+
+    def test_returns_none_for_dollar_variable(self) -> None:
+        assert _extract_action_base_name("${action}") is None
+
+
+class TestValidateJailConfigSync:
+    """Tests for _validate_jail_config_sync — the sync validation core."""
+
+    def _setup_config(self, config_dir: Path, jail_cfg: str) -> None:
+        """Write a minimal fail2ban directory layout with *jail_cfg* content."""
+        _write(config_dir / "jail.d" / "test.conf", jail_cfg)
+
+    def test_valid_config_no_issues(self, tmp_path: Path) -> None:
+        """A jail whose filter exists and has a valid regex should pass."""
+        # Create a real filter file so the existence check passes.
+        filter_d = tmp_path / "filter.d"
+        filter_d.mkdir(parents=True, exist_ok=True)
+        (filter_d / "sshd.conf").write_text("[Definition]\nfailregex = Host .* <HOST>\n")
+
+        self._setup_config(
+            tmp_path,
+            "[sshd]\nenabled = false\nfilter = sshd\nlogpath = /no/such/log\n",
+        )
+
+        result = _validate_jail_config_sync(tmp_path, "sshd")
+        # logpath advisory warning is OK; no blocking errors expected.
+        blocking = [i for i in result.issues if i.field != "logpath"]
+        assert blocking == [], blocking
+
+    def test_missing_filter_reported(self, tmp_path: Path) -> None:
+        """A jail whose filter file does not exist must report a filter issue."""
+        self._setup_config(
+            tmp_path,
+            "[bad-jail]\nenabled = false\nfilter = nonexistent-filter\n",
+        )
+
+        result = _validate_jail_config_sync(tmp_path, "bad-jail")
+        assert not result.valid
+        fields = [i.field for i in result.issues]
+        assert "filter" in fields
+
+    def test_bad_failregex_reported(self, tmp_path: Path) -> None:
+        """A jail with an un-compilable failregex must report a failregex issue."""
+        self._setup_config(
+            tmp_path,
+            "[broken]\nenabled = false\nfailregex = [invalid regex(\n",
+        )
+
+        result = _validate_jail_config_sync(tmp_path, "broken")
+        assert not result.valid
+        fields = [i.field for i in result.issues]
+        assert "failregex" in fields
+
+    def test_missing_log_path_is_advisory(self, tmp_path: Path) -> None:
+        """A missing log path should be reported in the logpath field."""
+        self._setup_config(
+            tmp_path,
+            "[myjail]\nenabled = false\nlogpath = /no/such/path.log\n",
+        )
+
+        result = _validate_jail_config_sync(tmp_path, "myjail")
+        fields = [i.field for i in result.issues]
+        assert "logpath" in fields
+
+    def test_missing_action_reported(self, tmp_path: Path) -> None:
+        """A jail referencing a non-existent action file must report an action issue."""
+        self._setup_config(
+            tmp_path,
+            "[myjail]\nenabled = false\naction = nonexistent-action\n",
+        )
+
+        result = _validate_jail_config_sync(tmp_path, "myjail")
+        fields = [i.field for i in result.issues]
+        assert "action" in fields
+
+    def test_unknown_jail_name(self, tmp_path: Path) -> None:
+        """Requesting validation for a jail not in any config returns an invalid result."""
+        (tmp_path / "jail.d").mkdir(parents=True, exist_ok=True)
+
+        result = _validate_jail_config_sync(tmp_path, "ghost")
+        assert not result.valid
+        assert any(i.field == "name" for i in result.issues)
+
+    def test_variable_action_not_flagged(self, tmp_path: Path) -> None:
+        """An action like ``%(action_)s`` should not be checked for file existence."""
+        self._setup_config(
+            tmp_path,
+            "[myjail]\nenabled = false\naction = %(action_)s\n",
+        )
+        result = _validate_jail_config_sync(tmp_path, "myjail")
+        # Ensure no action file-missing error (the variable expression is skipped).
+        action_errors = [i for i in result.issues if i.field == "action"]
+        assert action_errors == []
+
+
+@pytest.mark.asyncio
+class TestValidateJailConfigAsync:
+    """Tests for the public async wrapper validate_jail_config."""
+
+    async def test_returns_jail_validation_result(self, tmp_path: Path) -> None:
+        (tmp_path / "jail.d").mkdir(parents=True, exist_ok=True)
+        _write(
+            tmp_path / "jail.d" / "test.conf",
+            "[testjail]\nenabled = false\n",
+        )
+
+        result = await validate_jail_config(str(tmp_path), "testjail")
+        assert result.jail_name == "testjail"
+
+    async def test_rejects_unsafe_name(self, tmp_path: Path) -> None:
+        with pytest.raises(JailNameError):
+            await validate_jail_config(str(tmp_path), "../evil")
+
+
+@pytest.mark.asyncio
+class TestRollbackJail:
+    """Tests for rollback_jail (Task 3)."""
+
+    async def test_rollback_success(self, tmp_path: Path) -> None:
+        """When fail2ban comes back online, rollback returns fail2ban_running=True."""
+        _write(tmp_path / "jail.d" / "sshd.conf", "[sshd]\nenabled = true\n")
+
+        with (
+            patch(
+                "app.services.config_file_service._start_daemon",
+                new=AsyncMock(return_value=True),
+            ),
+            patch(
+                "app.services.config_file_service._wait_for_fail2ban",
+                new=AsyncMock(return_value=True),
+            ),
+            patch(
+                "app.services.config_file_service._get_active_jail_names",
+                new=AsyncMock(return_value=set()),
+            ),
+        ):
+            result = await rollback_jail(
+                str(tmp_path), "/fake.sock", "sshd", ["fail2ban-client", "start"]
+            )
+
+        assert result.disabled is True
+        assert result.fail2ban_running is True
+        assert result.jail_name == "sshd"
+        # .local file must have enabled=false
+        local = tmp_path / "jail.d" / "sshd.local"
+        assert local.is_file()
+        assert "enabled = false" in local.read_text()
+
+    async def test_rollback_fail2ban_still_down(self, tmp_path: Path) -> None:
+        """When fail2ban does not come back, rollback returns fail2ban_running=False."""
+        _write(tmp_path / "jail.d" / "sshd.conf", "[sshd]\nenabled = true\n")
+
+        with (
+            patch(
+                "app.services.config_file_service._start_daemon",
+                new=AsyncMock(return_value=False),
+            ),
+            patch(
+                "app.services.config_file_service._wait_for_fail2ban",
+                new=AsyncMock(return_value=False),
+            ),
+        ):
+            result = await rollback_jail(
+                str(tmp_path), "/fake.sock", "sshd", ["fail2ban-client", "start"]
+            )
+
+        assert result.fail2ban_running is False
+        assert result.disabled is True
+
+    async def test_rollback_rejects_unsafe_name(self, tmp_path: Path) -> None:
+        with pytest.raises(JailNameError):
+            await rollback_jail(
+                str(tmp_path), "/fake.sock", "../evil", ["fail2ban-client", "start"]
+            )
+