Harden preview_log path validation and add regression test
This commit is contained in:
@@ -7,9 +7,10 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import re
|
||||
import structlog
|
||||
from pathlib import Path
|
||||
from typing import TypeVar, cast
|
||||
from typing import cast
|
||||
|
||||
import structlog
|
||||
|
||||
from app.exceptions import ConfigOperationError
|
||||
from app.models.config import (
|
||||
@@ -42,7 +43,7 @@ _SAFE_LOG_PREFIXES: tuple[str, ...] = ("/var/log", "/config/log")
|
||||
def _ok(response: object) -> object:
|
||||
"""Extract the payload from a fail2ban ``(return_code, data)`` response."""
|
||||
try:
|
||||
code, data = cast(Fail2BanResponse, response)
|
||||
code, data = cast("Fail2BanResponse", response)
|
||||
except (TypeError, ValueError) as exc:
|
||||
raise ValueError(
|
||||
f"Unexpected fail2ban response shape: {response!r}"
|
||||
@@ -80,16 +81,13 @@ async def _safe_get(
|
||||
return default
|
||||
|
||||
|
||||
T = TypeVar("T")
|
||||
|
||||
|
||||
async def _safe_get_typed(
|
||||
client: Fail2BanClient,
|
||||
command: list[str],
|
||||
default: T,
|
||||
) -> T:
|
||||
"""Send a command and return the result typed as ``default``'s type."""
|
||||
return cast("T", await _safe_get(client, command, default))
|
||||
default: object | None = None,
|
||||
) -> object | None:
|
||||
"""Send a command and return a typed result or *default* if it fails."""
|
||||
return await _safe_get(client, command, default)
|
||||
|
||||
|
||||
async def read_fail2ban_log(
|
||||
@@ -210,7 +208,31 @@ async def preview_log(req: LogPreviewRequest) -> LogPreviewResponse:
|
||||
)
|
||||
|
||||
path = Path(req.log_path)
|
||||
if not path.is_file():
|
||||
try:
|
||||
resolved = path.resolve(strict=False)
|
||||
except (ValueError, OSError) as exc:
|
||||
return LogPreviewResponse(
|
||||
lines=[],
|
||||
total_lines=0,
|
||||
matched_count=0,
|
||||
regex_error=f"Cannot resolve log path {req.log_path!r}: {exc}",
|
||||
)
|
||||
|
||||
if not any(
|
||||
resolved.is_relative_to(Path(prefix))
|
||||
for prefix in _SAFE_LOG_PREFIXES
|
||||
):
|
||||
return LogPreviewResponse(
|
||||
lines=[],
|
||||
total_lines=0,
|
||||
matched_count=0,
|
||||
regex_error=(
|
||||
f"Log path {str(resolved)!r} is outside the allowed directory. "
|
||||
"Only paths under /var/log or /config/log are permitted."
|
||||
),
|
||||
)
|
||||
|
||||
if not resolved.is_file():
|
||||
return LogPreviewResponse(
|
||||
lines=[],
|
||||
total_lines=0,
|
||||
@@ -221,7 +243,7 @@ async def preview_log(req: LogPreviewRequest) -> LogPreviewResponse:
|
||||
try:
|
||||
raw_lines = await run_blocking(
|
||||
_read_tail_lines,
|
||||
str(path),
|
||||
str(resolved),
|
||||
req.num_lines,
|
||||
)
|
||||
except OSError as exc:
|
||||
|
||||
@@ -592,13 +592,28 @@ class TestPreviewLog:
|
||||
|
||||
assert result.regex_error is not None
|
||||
|
||||
async def test_rejects_log_paths_outside_allowed_directories(self) -> None:
|
||||
"""preview_log rejects files outside the configured safe log directories."""
|
||||
req = LogPreviewRequest(
|
||||
log_path="/etc/passwd",
|
||||
fail_regex=r"root",
|
||||
)
|
||||
result = await config_service.preview_log(req)
|
||||
|
||||
assert result.regex_error is not None
|
||||
assert "outside the allowed directory" in result.regex_error
|
||||
|
||||
async def test_matches_lines_in_file(self, tmp_path: Any) -> None:
|
||||
"""preview_log correctly identifies matching and non-matching lines."""
|
||||
log_file = tmp_path / "test.log"
|
||||
log_file.write_text("FAIL login from 1.2.3.4\nOK normal line\nFAIL from 5.6.7.8\n")
|
||||
|
||||
req = LogPreviewRequest(log_path=str(log_file), fail_regex=r"FAIL")
|
||||
result = await config_service.preview_log(req)
|
||||
with patch(
|
||||
"app.services.log_service._SAFE_LOG_PREFIXES",
|
||||
(str(tmp_path),),
|
||||
):
|
||||
result = await config_service.preview_log(req)
|
||||
|
||||
assert result.total_lines == 3
|
||||
assert result.matched_count == 2
|
||||
@@ -612,7 +627,11 @@ class TestPreviewLog:
|
||||
log_path=str(log_file),
|
||||
fail_regex=r"from (\d+\.\d+\.\d+\.\d+)",
|
||||
)
|
||||
result = await config_service.preview_log(req)
|
||||
with patch(
|
||||
"app.services.log_service._SAFE_LOG_PREFIXES",
|
||||
(str(tmp_path),),
|
||||
):
|
||||
result = await config_service.preview_log(req)
|
||||
|
||||
matched = [ln for ln in result.lines if ln.matched]
|
||||
assert len(matched) == 1
|
||||
@@ -624,7 +643,11 @@ class TestPreviewLog:
|
||||
log_file.write_text("\n".join(f"line {i}" for i in range(500)) + "\n")
|
||||
|
||||
req = LogPreviewRequest(log_path=str(log_file), fail_regex=r"line", num_lines=50)
|
||||
result = await config_service.preview_log(req)
|
||||
with patch(
|
||||
"app.services.log_service._SAFE_LOG_PREFIXES",
|
||||
(str(tmp_path),),
|
||||
):
|
||||
result = await config_service.preview_log(req)
|
||||
|
||||
assert result.total_lines <= 50
|
||||
|
||||
|
||||
Reference in New Issue
Block a user