mirror of
https://github.com/CyberMind-FR/secubox-deb.git
synced 2026-07-29 13:59:40 +00:00
Anti-regression for the double-cache pattern documented in CLAUDE.md (« Performance Patterns — Double Caching »). Catches the bug class that caused PR #146 (secubox-waf 17 % sustained CPU under SOC polling) before it lands. What's added ============ - scripts/check-dashboard-cache.py — AST-based audit. Walks every packages/secubox-*/api/main.py, flags hot dashboard routes (/stats, /alerts, /summary, /dashboard, /metrics, /overview, /events, /history, /recent, /bans, /sessions, /connections) whose handler body does per-request I/O (open, read_text, read_bytes, subprocess.run, Popen) AND doesn't go through a cache (module-level _cache instance + .get() call, OR asyncio.create_task(refresh_*) at startup). * --check : exit 1 on unjustified violations (CI mode) * --report : human-readable table (default) * --json : machine-readable - .cache-lint-allowlist.toml — TOML allowlist for justified exceptions. Each entry needs a daemon + route + justification (issue link expected). Seeded with the 4 current legacy cases: secubox-waf (covered by PR #146 — will drop the entries when merged), secubox-config-advisor /history (admin drill-in, not polled), secubox-netdiag /connections (real-time TCP state), secubox-tor /summary (small file read, follow-up cleanup). - .github/workflows/dashboard-cache-check.yml — mirror of license-check.yml. Runs the lint + the self-tests on PRs that touch packages/**/api/main.py, the lint script, the allowlist, or the workflow itself. - docs/CACHE-PATTERN.md — short remediation guide. Three accepted shapes (read-through, background-refresh, pure in-memory), code example, allowlist syntax, references to the canonical implementations (secubox-crowdsec for read-through, secubox-system for background-refresh). Tests (14/14 pass) ================== - StatsCache pattern detection (compliant cases) - Non-compliant detection: open, subprocess.run, multiple I/O ops - Pure in-memory handler = compliant - Background-refresh task = compliant - Non-hot route (/health) not audited - @router.get treated like @app.get - Allowlist suppresses by exact (daemon, route) key - Allowlist doesn't cross-pollinate across routes - Missing allowlist file = no error - CLI --check exits 1 on unjustified - CLI --check exits 0 when clean - CLI --json output well-formed - **Real repo audit with seeded allowlist passes --check** (sanity gate: catches regression if anyone removes an allowlist entry without fixing the daemon) Baseline against the real packages/ tree ======================================== 122 daemons audited, 118 compliant, 4 with findings (all allowlisted). When PR #146 merges, the secubox-waf entries can be removed from the allowlist — the next lint run will then prove the fix is wired correctly. Follow-up ========= - Drop secubox-waf allowlist entries when #146 merges. - Per-daemon follow-up issues for the remaining 3 (config-advisor, netdiag, tor) — not blocking, allowlist documents the rationale. - Consider extending HOT_ROUTES to /health / /status once the small daemons that call `systemctl is-active` adopt a 10 s cache. Co-authored-by: CyberMind-FR <gandalf@Gk2.net> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
312 lines
12 KiB
Python
312 lines
12 KiB
Python
# SPDX-License-Identifier: LicenseRef-CMSD-1.0
|
|
# Copyright (c) 2026 CyberMind — Gérald Kerma <devel@cybermind.fr>
|
|
# Source-Disclosed License — All rights reserved except as expressly granted.
|
|
# See LICENCE-CMSD-1.0.md for terms.
|
|
"""Tests for scripts/check-dashboard-cache.py."""
|
|
from __future__ import annotations
|
|
|
|
import importlib.util
|
|
import sys
|
|
import textwrap
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
# Load the script as a module (hyphenated filename → import via spec loader).
|
|
_SCRIPT = Path(__file__).resolve().parents[1] / "check-dashboard-cache.py"
|
|
_spec = importlib.util.spec_from_file_location("cdc", _SCRIPT)
|
|
assert _spec and _spec.loader
|
|
cdc = importlib.util.module_from_spec(_spec)
|
|
sys.modules["cdc"] = cdc
|
|
_spec.loader.exec_module(cdc)
|
|
|
|
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
# Helpers
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
|
|
def make_fake_packages(root: Path, daemons: dict[str, str]) -> Path:
|
|
"""Materialise ``packages/<name>/api/main.py`` for each entry.
|
|
|
|
``daemons`` maps daemon-name → source code for ``main.py``.
|
|
"""
|
|
pkg_root = root / "packages"
|
|
pkg_root.mkdir()
|
|
for name, src in daemons.items():
|
|
d = pkg_root / name / "api"
|
|
d.mkdir(parents=True)
|
|
(d / "main.py").write_text(textwrap.dedent(src))
|
|
return pkg_root
|
|
|
|
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
# Core detection
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
|
|
def test_compliant_handler_with_read_through_cache(tmp_path):
|
|
"""``stats_cache.get(...)`` in body → no finding."""
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-foo": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
stats_cache = object() # placeholder; module-level instance
|
|
@app.get("/stats")
|
|
async def get_stats():
|
|
cached = stats_cache.get("stats")
|
|
if cached is not None:
|
|
return cached
|
|
with open("/var/log/foo.log") as f:
|
|
return f.read()
|
|
""",
|
|
})
|
|
reports, unjustified = cdc.run(pkg, Path("/nonexistent"))
|
|
assert not unjustified
|
|
assert len(reports[0].findings) == 0
|
|
|
|
|
|
def test_handler_doing_open_without_cache_is_flagged(tmp_path):
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-bad": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/stats")
|
|
async def get_stats():
|
|
with open("/var/log/foo.log") as f:
|
|
return f.read()
|
|
""",
|
|
})
|
|
reports, unjustified = cdc.run(pkg, Path("/nonexistent"))
|
|
assert len(unjustified) == 1
|
|
f = unjustified[0]
|
|
assert f.daemon == "secubox-bad"
|
|
assert f.route == "/stats"
|
|
assert any("open@" in c for c in f.io_calls)
|
|
|
|
|
|
def test_handler_doing_subprocess_run_is_flagged(tmp_path):
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-bad": """
|
|
import subprocess
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/metrics")
|
|
async def get_metrics():
|
|
return subprocess.run(["expensive"], capture_output=True)
|
|
""",
|
|
})
|
|
_, unjustified = cdc.run(pkg, Path("/nonexistent"))
|
|
assert len(unjustified) == 1
|
|
assert "run@" in unjustified[0].io_calls[0]
|
|
|
|
|
|
def test_handler_with_no_io_is_compliant(tmp_path):
|
|
"""Pure in-memory handler doesn't need a cache."""
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-pure": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
STATE = {"x": 1}
|
|
@app.get("/stats")
|
|
async def get_stats():
|
|
return STATE
|
|
""",
|
|
})
|
|
_, unjustified = cdc.run(pkg, Path("/nonexistent"))
|
|
assert not unjustified
|
|
|
|
|
|
def test_handler_with_bg_refresh_task_is_compliant(tmp_path):
|
|
"""``asyncio.create_task(refresh_cache())`` at startup covers all hot routes."""
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-bg": """
|
|
import asyncio
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
|
|
async def refresh_cache():
|
|
pass
|
|
|
|
@app.on_event("startup")
|
|
async def startup():
|
|
asyncio.create_task(refresh_cache())
|
|
|
|
@app.get("/stats")
|
|
async def get_stats():
|
|
with open("/var/log/foo.log") as f:
|
|
return f.read()
|
|
""",
|
|
})
|
|
_, unjustified = cdc.run(pkg, Path("/nonexistent"))
|
|
assert not unjustified
|
|
|
|
|
|
def test_non_hot_route_is_not_audited(tmp_path):
|
|
"""``/health`` is NOT in HOT_ROUTES — even with I/O it should not flag."""
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-meh": """
|
|
import subprocess
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/health")
|
|
async def get_health():
|
|
return subprocess.run(["systemctl", "is-active", "foo"])
|
|
""",
|
|
})
|
|
_, unjustified = cdc.run(pkg, Path("/nonexistent"))
|
|
assert not unjustified
|
|
|
|
|
|
def test_router_decorator_is_recognised(tmp_path):
|
|
"""``@router.get`` is treated like ``@app.get``."""
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-r": """
|
|
from fastapi import APIRouter
|
|
router = APIRouter()
|
|
@router.get("/summary")
|
|
async def get_summary():
|
|
with open("/var/log/foo.log") as f:
|
|
return f.read()
|
|
""",
|
|
})
|
|
_, unjustified = cdc.run(pkg, Path("/nonexistent"))
|
|
assert len(unjustified) == 1
|
|
assert unjustified[0].route == "/summary"
|
|
|
|
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
# Allowlist
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
|
|
def test_allowlist_suppresses_known_violation(tmp_path):
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-bad": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/stats")
|
|
async def get_stats():
|
|
with open("/var/log/foo.log") as f:
|
|
return f.read()
|
|
""",
|
|
})
|
|
allowlist = tmp_path / "allow.toml"
|
|
allowlist.write_text(textwrap.dedent("""
|
|
[[entries]]
|
|
daemon = "secubox-bad"
|
|
route = "/stats"
|
|
justification = "tracked in issue #999"
|
|
"""))
|
|
reports, unjustified = cdc.run(pkg, allowlist)
|
|
# Finding still recorded, but allowlist suppresses the failure
|
|
assert len(reports[0].findings) == 1
|
|
assert not unjustified
|
|
|
|
|
|
def test_allowlist_only_matches_exact_route(tmp_path):
|
|
"""Allowlisting daemon::/stats must not silence daemon::/alerts."""
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-multi": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/stats")
|
|
async def get_stats():
|
|
with open("/x") as f: f.read()
|
|
@app.get("/alerts")
|
|
async def get_alerts():
|
|
with open("/y") as f: f.read()
|
|
""",
|
|
})
|
|
allowlist = tmp_path / "allow.toml"
|
|
allowlist.write_text(textwrap.dedent("""
|
|
[[entries]]
|
|
daemon = "secubox-multi"
|
|
route = "/stats"
|
|
justification = "x"
|
|
"""))
|
|
_, unjustified = cdc.run(pkg, allowlist)
|
|
assert len(unjustified) == 1
|
|
assert unjustified[0].route == "/alerts"
|
|
|
|
|
|
def test_missing_allowlist_file_is_ignored(tmp_path):
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-pure": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/stats")
|
|
async def get_stats(): return {}
|
|
""",
|
|
})
|
|
reports, unjustified = cdc.run(pkg, tmp_path / "does-not-exist.toml")
|
|
assert not unjustified
|
|
assert len(reports) == 1
|
|
|
|
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
# CLI
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
|
|
def test_cli_check_exits_1_on_unjustified(tmp_path, capsys):
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-bad": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/stats")
|
|
async def get_stats():
|
|
with open("/x") as f: f.read()
|
|
""",
|
|
})
|
|
rc = cdc.main(["--check", "--packages", str(pkg),
|
|
"--allowlist", str(tmp_path / "noop.toml")])
|
|
out = capsys.readouterr().out
|
|
assert rc == 1
|
|
assert "unjustified violation" in out
|
|
|
|
|
|
def test_cli_check_exits_0_when_clean(tmp_path):
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-ok": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/stats")
|
|
async def get_stats(): return {}
|
|
""",
|
|
})
|
|
rc = cdc.main(["--check", "--packages", str(pkg),
|
|
"--allowlist", str(tmp_path / "noop.toml")])
|
|
assert rc == 0
|
|
|
|
|
|
def test_cli_json_output(tmp_path, capsys):
|
|
pkg = make_fake_packages(tmp_path, {
|
|
"secubox-bad": """
|
|
from fastapi import FastAPI
|
|
app = FastAPI()
|
|
@app.get("/stats")
|
|
async def get_stats():
|
|
with open("/x") as f: f.read()
|
|
""",
|
|
})
|
|
rc = cdc.main(["--json", "--packages", str(pkg),
|
|
"--allowlist", str(tmp_path / "noop.toml")])
|
|
out = capsys.readouterr().out
|
|
import json as _json
|
|
data = _json.loads(out)
|
|
assert rc == 0 # --json doesn't exit non-zero (only --check does)
|
|
assert data["summary"]["unjustified_violations"] == 1
|
|
assert data["unjustified"][0]["daemon"] == "secubox-bad"
|
|
|
|
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
# Repo integration
|
|
# ──────────────────────────────────────────────────────────────────────
|
|
|
|
def test_real_repo_with_seeded_allowlist_passes_check():
|
|
"""Sanity: running on the real packages/ with .cache-lint-allowlist.toml
|
|
must be clean. Catches regressions if someone adds a new violation
|
|
or removes an allowlist entry without fixing the daemon."""
|
|
repo = Path(__file__).resolve().parents[2]
|
|
rc = cdc.main(["--check",
|
|
"--packages", str(repo / "packages"),
|
|
"--allowlist", str(repo / ".cache-lint-allowlist.toml")])
|
|
assert rc == 0, "real repo audit must pass with seeded allowlist"
|