lukas.pupkalipinski/BanGUI
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Complete tasks 1-5: UI cleanup, pie chart fix, log path allowlist, activation hardening

Browse Source 
Task 1: Remove ActiveBansSection from JailsPage
- Delete buildBanColumns, fmtTimestamp, ActiveBansSection
- Remove Dialog/Delete/Dismiss imports, ActiveBan type
- Update JSDoc to reflect three sections

Task 2: Remove JailDistributionChart from Dashboard
- Delete import and JSX block from DashboardPage.tsx

Task 3: Fix transparent pie chart (TopCountriesPieChart)
- Add Cell import and per-slice <Cell fill={slice.fill}> children inside <Pie>
- Suppress @typescript-eslint/no-deprecated (recharts v3 types)

Task 4: Allow /config/log as safe log prefix
- Add '/config/log' to _SAFE_LOG_PREFIXES in config_service.py
- Update error message to list both allowed directories

Task 5: Block jail activation on missing filter/logpath
- activate_jail refuses to proceed when filter/logpath issues found
- ActivateJailDialog treats all validation issues as blocking
- Trigger immediate _run_probe after activation in config router
- /api/health now reports fail2ban online/offline from cached probe
- Add TestActivateJailBlocking tests; fix existing tests to mock validation
This commit is contained in:
Lukas
2026-03-14 18:57:01 +01:00
parent 68d8056d2e
commit ee7412442a
11 changed files with 425 additions and 656 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
backend/app/routers/config.py
View File

@@ -97,6 +97,7 @@ from app.services.config_service import (
ConfigValidationError,
JailNotFoundError,
)
from app.tasks.health_check import _run_probe
from app.utils.fail2ban_client import Fail2BanConnectionError
router: APIRouter = APIRouter(prefix="/api/config", tags=["Config"])
@@ -654,6 +655,10 @@ async def activate_jail(
detected_at=datetime.datetime.now(tz=datetime.UTC),
)
# Force an immediate health probe so the cached status reflects the current
# fail2ban state without waiting for the next scheduled check.
await _run_probe(request.app)
return result

30
backend/app/routers/health.py
View File

@@ -1,21 +1,37 @@
"""Health check router.
A lightweight ``GET /api/health`` endpoint that verifies the application
is running and can serve requests. It does not probe fail2ban — that
responsibility belongs to the health service (Stage 4).
is running and can serve requests. Also reports the cached fail2ban liveness
state so monitoring tools and Docker health checks can observe daemon status
without probing the socket directly.
"""
from fastapi import APIRouter
from fastapi import APIRouter, Request
from fastapi.responses import JSONResponse
from app.models.server import ServerStatus
router: APIRouter = APIRouter(prefix="/api", tags=["Health"])
@router.get("/health", summary="Application health check")
async def health_check() -> JSONResponse:
"""Return a 200 response confirming the API is operational.
async def health_check(request: Request) -> JSONResponse:
"""Return 200 with application and fail2ban status.
HTTP 200 is always returned so Docker health checks do not restart the
backend container when fail2ban is temporarily offline. The
``fail2ban`` field in the body indicates the daemon's current state.
Args:
request: FastAPI request (used to read cached server status).
Returns:
A JSON object with ``{"status": "ok"}``.
A JSON object with ``{"status": "ok", "fail2ban": "online"|"offline"}``.
"""
return JSONResponse(content={"status": "ok"})
cached: ServerStatus = getattr(
request.app.state, "server_status", ServerStatus(online=False)
)
return JSONResponse(content={
"status": "ok",
"fail2ban": "online" if cached.online else "offline",
})

19
backend/app/services/config_file_service.py
View File

@@ -1136,6 +1136,25 @@ async def activate_jail(
warnings=warnings,
)
# Block activation on critical validation failures (missing filter or logpath).
blocking = [i for i in validation_result.issues if i.field in ("filter", "logpath")]
if blocking:
log.warning(
"jail_activation_blocked",
jail=name,
issues=[f"{i.field}: {i.message}" for i in blocking],
)
return JailActivationResponse(
name=name,
active=False,
fail2ban_running=True,
validation_warnings=warnings,
message=(
f"Jail {name!r} cannot be activated: "
+ "; ".join(i.message for i in blocking)
),
)
overrides: dict[str, Any] = {
"bantime": req.bantime,
"findtime": req.findtime,

4
backend/app/services/config_service.py
View File

@@ -768,7 +768,7 @@ _NON_FILE_LOG_TARGETS: frozenset[str] = frozenset(
)
# Only allow reading log files under these base directories (security).
_SAFE_LOG_PREFIXES: tuple[str, ...] = ("/var/log",)
_SAFE_LOG_PREFIXES: tuple[str, ...] = ("/var/log", "/config/log")
def _count_file_lines(file_path: str) -> int:
@@ -847,7 +847,7 @@ async def read_fail2ban_log(
if not any(resolved_str.startswith(safe) for safe in _SAFE_LOG_PREFIXES):
raise ConfigOperationError(
f"Log path {resolved_str!r} is outside the allowed directory. "
"Only paths under /var/log are permitted."
"Only paths under /var/log or /config/log are permitted."
)
if not resolved.is_file():

26
backend/tests/test_routers/test_config.py
View File

@@ -742,6 +742,32 @@ class TestActivateJail:
).post("/api/config/jails/sshd/activate", json={})
assert resp.status_code == 401
async def test_200_with_active_false_on_missing_logpath(self, config_client: AsyncClient) -> None:
"""POST .../activate returns 200 with active=False when the service blocks due to missing logpath."""
from app.models.config import JailActivationResponse
blocked_response = JailActivationResponse(
name="airsonic-auth",
active=False,
fail2ban_running=True,
validation_warnings=["logpath: log file '/var/log/airsonic/airsonic.log' not found"],
message="Jail 'airsonic-auth' cannot be activated: log file '/var/log/airsonic/airsonic.log' not found",
)
with patch(
"app.routers.config.config_file_service.activate_jail",
AsyncMock(return_value=blocked_response),
):
resp = await config_client.post(
"/api/config/jails/airsonic-auth/activate", json={}
)
assert resp.status_code == 200
data = resp.json()
assert data["active"] is False
assert data["fail2ban_running"] is True
assert "cannot be activated" in data["message"]
assert len(data["validation_warnings"]) == 1
# ---------------------------------------------------------------------------
# POST /api/config/jails/{name}/deactivate

127
backend/tests/test_services/test_config_file_service.py
View File

@@ -434,7 +434,7 @@ class TestListInactiveJails:
class TestActivateJail:
async def test_activates_known_inactive_jail(self, tmp_path: Path) -> None:
_write(tmp_path / "jail.conf", JAIL_CONF)
from app.models.config import ActivateJailRequest
from app.models.config import ActivateJailRequest, JailValidationResult
req = ActivateJailRequest()
with (
@@ -447,6 +447,10 @@ class TestActivateJail:
"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()
result = await activate_jail(str(tmp_path), "/fake.sock", "apache-auth", req)
@@ -492,7 +496,7 @@ class TestActivateJail:
async def test_writes_overrides_to_local_file(self, tmp_path: Path) -> None:
_write(tmp_path / "jail.conf", JAIL_CONF)
from app.models.config import ActivateJailRequest
from app.models.config import ActivateJailRequest, JailValidationResult
req = ActivateJailRequest(bantime="2h", maxretry=3)
with (
@@ -505,6 +509,10 @@ class TestActivateJail:
"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()
await activate_jail(str(tmp_path), "/fake.sock", "apache-auth", req)
@@ -2512,7 +2520,7 @@ class TestActivateJailReloadArgs:
async def test_activate_passes_include_jails(self, tmp_path: Path) -> None:
"""activate_jail must pass include_jails=[name] to reload_all."""
_write(tmp_path / "jail.conf", JAIL_CONF)
from app.models.config import ActivateJailRequest
from app.models.config import ActivateJailRequest, JailValidationResult
req = ActivateJailRequest()
with (
@@ -2525,6 +2533,10 @@ class TestActivateJailReloadArgs:
"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()
await activate_jail(str(tmp_path), "/fake.sock", "apache-auth", req)
@@ -2538,7 +2550,7 @@ class TestActivateJailReloadArgs:
) -> None:
"""activate_jail returns active=True when the jail appears in post-reload names."""
_write(tmp_path / "jail.conf", JAIL_CONF)
from app.models.config import ActivateJailRequest
from app.models.config import ActivateJailRequest, JailValidationResult
req = ActivateJailRequest()
with (
@@ -2551,6 +2563,10 @@ class TestActivateJailReloadArgs:
"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()
result = await activate_jail(
@@ -2570,7 +2586,7 @@ class TestActivateJailReloadArgs:
start the jail even though the reload command succeeded.
"""
_write(tmp_path / "jail.conf", JAIL_CONF)
from app.models.config import ActivateJailRequest
from app.models.config import ActivateJailRequest, JailValidationResult
req = ActivateJailRequest()
# Pre-reload: jail not running. Post-reload: still not running (boot failed).
@@ -2585,6 +2601,10 @@ class TestActivateJailReloadArgs:
"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()
result = await activate_jail(
@@ -2830,3 +2850,100 @@ class TestRollbackJail:
str(tmp_path), "/fake.sock", "../evil", ["fail2ban-client", "start"]
)
# ---------------------------------------------------------------------------
# activate_jail — blocking on missing filter / logpath (Task 5)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
class TestActivateJailBlocking:
"""activate_jail must refuse to proceed when validation finds critical issues."""
async def test_activate_jail_blocked_when_logpath_missing(self, tmp_path: Path) -> None:
"""activate_jail returns active=False if _validate_jail_config_sync reports a missing logpath."""
from app.models.config import ActivateJailRequest, JailValidationIssue, JailValidationResult
_write(tmp_path / "jail.conf", JAIL_CONF)
req = ActivateJailRequest()
missing_issue = JailValidationIssue(field="logpath", message="log file '/var/log/missing.log' not found")
validation = JailValidationResult(jail_name="apache-auth", valid=False, issues=[missing_issue])
with (
patch(
"app.services.config_file_service._get_active_jail_names",
new=AsyncMock(return_value=set()),
),
patch(
"app.services.config_file_service._validate_jail_config_sync",
return_value=validation,
),
patch("app.services.config_file_service.jail_service") as mock_js,
):
mock_js.reload_all = AsyncMock()
result = await activate_jail(str(tmp_path), "/fake.sock", "apache-auth", req)
assert result.active is False
assert result.fail2ban_running is True
assert "cannot be activated" in result.message
mock_js.reload_all.assert_not_awaited()
async def test_activate_jail_blocked_when_filter_missing(self, tmp_path: Path) -> None:
"""activate_jail returns active=False if a filter file is missing."""
from app.models.config import ActivateJailRequest, JailValidationIssue, JailValidationResult
_write(tmp_path / "jail.conf", JAIL_CONF)
req = ActivateJailRequest()
filter_issue = JailValidationIssue(field="filter", message="filter file 'sshd.conf' not found")
validation = JailValidationResult(jail_name="sshd", valid=False, issues=[filter_issue])
with (
patch(
"app.services.config_file_service._get_active_jail_names",
new=AsyncMock(return_value=set()),
),
patch(
"app.services.config_file_service._validate_jail_config_sync",
return_value=validation,
),
patch("app.services.config_file_service.jail_service") as mock_js,
):
mock_js.reload_all = AsyncMock()
result = await activate_jail(str(tmp_path), "/fake.sock", "sshd", req)
assert result.active is False
assert result.fail2ban_running is True
assert "cannot be activated" in result.message
mock_js.reload_all.assert_not_awaited()
async def test_activate_jail_proceeds_when_only_regex_warnings(self, tmp_path: Path) -> None:
"""activate_jail proceeds normally when only non-blocking (failregex) warnings exist."""
from app.models.config import ActivateJailRequest, JailValidationIssue, JailValidationResult
_write(tmp_path / "jail.conf", JAIL_CONF)
req = ActivateJailRequest()
advisory_issue = JailValidationIssue(field="failregex", message="no failregex defined")
# valid=True but with a non-blocking advisory issue
validation = JailValidationResult(jail_name="apache-auth", valid=True, issues=[advisory_issue])
with (
patch(
"app.services.config_file_service._get_active_jail_names",
new=AsyncMock(side_effect=[set(), {"apache-auth"}]),
),
patch(
"app.services.config_file_service._validate_jail_config_sync",
return_value=validation,
),
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)
assert result.active is True
mock_js.reload_all.assert_awaited_once()