fix: критические баги и качество кода — полный аудит

Критические фиксы: - main.py: монтировать /static из web/static/ (CSS не грузился совсем) - api/scans.py: filtered total count (был всегда общий, игнорируя фильтры) - web/routes.py: исправлен VALID_SORT_FIELDS (отсутствовали ключи packages) - web/routes.py: filtered total count для web scans list - package_detail.html: f.data.X вместо f.X (findings не отображались) Чистка мёртвого кода: - config.py: удалён _parse_repos и nexus_repositories (не использовались) - web/routes.py: удалён completed_scans/failed_scans (не отображались) - удалён мёртвый guarddog_nexus/static/style.css (67-байтный стаб) Качество кода: - web/routes.py: Jinja2 Environment кэшируется на уровне модуля - Вынесен дублирующийся JS в web/static/app.js - Вынесены дублирующиеся inline-стили в CSS-классы - Исправлен duplicate class attribute в списках - Удалены гигантские SVG из empty states Тесты: - 20 новых edge-case тестов (CSV export, search/filter/sort, 404, pagination) - Добавлен sample_flagged_scan fixture - Итого: 50 тестов, все зелёные
2026-05-10 03:46:05 +03:00
parent 6c8e89c95e
commit c43e7c4c9b
15 changed files with 329 additions and 145 deletions
--- a/guarddog_nexus/api/scans.py
+++ b/guarddog_nexus/api/scans.py
@@ -36,18 +36,22 @@ async def list_scans(
    session: AsyncSession = Depends(get_session),
 ):
    q = select(Scan)
+    count_q = select(func.count(Scan.id))

    if flagged is not None:
        q = q.where(Scan.flagged == flagged)
+        count_q = count_q.where(Scan.flagged == flagged)
    if status:
        q = q.where(Scan.status == status)
+        count_q = count_q.where(Scan.status == status)
    if repository:
        q = q.where(Scan.repository == repository)
+        count_q = count_q.where(Scan.repository == repository)
    if search:
        pattern = f"%{search}%"
-        q = q.where(
-            Scan.package_name.ilike(pattern) | Scan.package_version.ilike(pattern)
-        )
+        condition = Scan.package_name.ilike(pattern) | Scan.package_version.ilike(pattern)
+        q = q.where(condition)
+        count_q = count_q.where(condition)

    sort_field = VALID_SORT_FIELDS.get(sort_by, Scan.started_at)
    sort_dir = "asc" if sort_dir.lower() == "asc" else "desc"
@@ -55,7 +59,7 @@ async def list_scans(

    q = q.offset(offset).limit(limit)

-    total = await session.scalar(select(func.count(Scan.id)))
+    total = await session.scalar(count_q)

    scans = (await session.execute(q)).scalars().all()
    return {
--- a/guarddog_nexus/config.py
+++ b/guarddog_nexus/config.py
@@ -1,7 +1,7 @@
 """Configuration via environment variables."""

 import os
-from dataclasses import dataclass, field
+from dataclasses import dataclass


@dataclass
@@ -9,7 +9,6 @@ class Config:
    nexus_url: str = os.getenv("NEXUS_URL", "http://localhost:8081")
    nexus_username: str = os.getenv("NEXUS_USERNAME", "admin")
    nexus_password: str = os.getenv("NEXUS_PASSWORD", "admin123")
-    nexus_repositories: list[str] = field(default_factory=lambda: _parse_repos())

    database_path: str = os.getenv("DATABASE_PATH", "data/guarddog.db")

@@ -26,9 +25,4 @@ class Config:
    temp_dir: str = os.getenv("TEMP_DIR", "/tmp/guarddog-nexus")


-def _parse_repos() -> list[str]:
-    raw = os.getenv("NEXUS_REPOSITORIES", "")
-    return [r.strip() for r in raw.split(",") if r.strip()]
-
-
 config = Config()
--- a/guarddog_nexus/main.py
+++ b/guarddog_nexus/main.py
@@ -14,7 +14,7 @@ from guarddog_nexus.logging_setup import log
 from guarddog_nexus.web.routes import router as web_router
 from guarddog_nexus.webhooks import router as webhook_router

-STATIC_DIR = os.path.join(os.path.dirname(__file__), "static")
+STATIC_DIR = os.path.join(os.path.dirname(__file__), "web", "static")


@asynccontextmanager
--- a/guarddog_nexus/static/style.css
+++ b/guarddog_nexus/static/style.css
@@ -1 +0,0 @@
-/* static/style.css - minimal overrides for Pico.css dark theme */
--- a/guarddog_nexus/web/routes.py
+++ b/guarddog_nexus/web/routes.py
@@ -4,6 +4,7 @@ import datetime

 from fastapi import APIRouter, Depends, Request
 from fastapi.responses import HTMLResponse
+from jinja2 import Environment, PackageLoader, select_autoescape
 from sqlalchemy import Integer, cast, func, select, text
 from sqlalchemy.ext.asyncio import AsyncSession

@@ -12,7 +13,12 @@ from guarddog_nexus.models import Finding, Scan

 router = APIRouter(tags=["web"])

-VALID_SORT_FIELDS = {
+_jinja_env = Environment(
+    loader=PackageLoader("guarddog_nexus", "web/templates"),
+    autoescape=select_autoescape(),
+)
+
+SCAN_SORT_FIELDS = {
    "id": Scan.id,
    "package_name": Scan.package_name,
    "started_at": Scan.started_at,
@@ -20,15 +26,16 @@ VALID_SORT_FIELDS = {
    "total_findings": Scan.total_findings,
 }

+PACKAGE_SORT_FIELDS = {
+    "name": Scan.package_name,
+    "last_scanned_at": Scan.started_at,
+    "total_findings": Scan.total_findings,
+    "flagged": Scan.flagged,
+}
+

 def _render(name: str, **context) -> HTMLResponse:
-    from jinja2 import Environment, PackageLoader, select_autoescape
-
-    env = Environment(
-        loader=PackageLoader("guarddog_nexus", "web/templates"),
-        autoescape=select_autoescape(),
-    )
-    template = env.get_template(name)
+    template = _jinja_env.get_template(name)
    return HTMLResponse(template.render(**context))


@@ -53,10 +60,6 @@ async def _dashboard_data(session: AsyncSession) -> dict:
            Scan.started_at >= func.datetime("now", "-7 days"),
        )
    )
-    completed_scans = await session.scalar(
-        select(func.count(Scan.id)).where(Scan.status == "completed")
-    )
-    failed_scans = await session.scalar(select(func.count(Scan.id)).where(Scan.status == "failed"))
    total_findings = await session.scalar(select(func.count(Finding.id)))

    warnings_count = await session.scalar(
@@ -115,7 +118,6 @@ async def _dashboard_data(session: AsyncSession) -> dict:

    max_findings = max((r.total for r in most_flagged), default=1)

-    # Heatmap: scans per day for last 14 days
    days_raw = (
        await session.execute(
            select(
@@ -133,8 +135,6 @@ async def _dashboard_data(session: AsyncSession) -> dict:
        "total_scans": total_scans or 0,
        "flagged_scans": flagged_scans or 0,
        "recent_flagged": recent_flagged or 0,
-        "completed_scans": completed_scans or 0,
-        "failed_scans": failed_scans or 0,
        "total_findings": total_findings or 0,
        "warnings_count": warnings_count or 0,
        "errors_count": errors_count or 0,
@@ -162,23 +162,27 @@ async def scans_list(
    per_page = 50
    offset = (page - 1) * per_page

+    count_q = select(func.count(Scan.id))
    q = select(Scan)
+
    if flagged == "1":
        q = q.where(Scan.flagged == True)
+        count_q = count_q.where(Scan.flagged == True)
    if status:
        q = q.where(Scan.status == status)
+        count_q = count_q.where(Scan.status == status)
    if search:
        pattern = f"%{search}%"
-        q = q.where(
-            Scan.package_name.ilike(pattern) | Scan.package_version.ilike(pattern)
-        )
+        condition = Scan.package_name.ilike(pattern) | Scan.package_version.ilike(pattern)
+        q = q.where(condition)
+        count_q = count_q.where(condition)

-    sort_field = VALID_SORT_FIELDS.get(sort_by, Scan.started_at)
+    sort_field = SCAN_SORT_FIELDS.get(sort_by, Scan.started_at)
    q = q.order_by(sort_field.desc() if sort_dir == "desc" else sort_field.asc())
    q = q.offset(offset).limit(per_page)

    scans = (await session.execute(q)).scalars().all()
-    total = await session.scalar(select(func.count(Scan.id)))
+    total = await session.scalar(count_q)

    return _render(
        "scans_list.html",
@@ -240,17 +244,17 @@ async def packages_list(
            Scan.package_name.ilike(pattern) | Scan.package_version.ilike(pattern)
        )

-    sort_field = VALID_SORT_FIELDS.get(sort_by, Scan.started_at)
+    sort_field = PACKAGE_SORT_FIELDS.get(sort_by, Scan.started_at)
    sort_col = func.max(sort_field)
    subq = subq.order_by(
        sort_col.desc() if sort_dir == "desc" else sort_col.asc()
    )

-    subq = subq.subquery()
-    total = await session.scalar(select(func.count()).select_from(subq))
+    sq = subq.subquery()
+    total = await session.scalar(select(func.count()).select_from(sq))
    rows = (
        await session.execute(
-            select(subq).offset(offset).limit(per_page)
+            select(sq).offset(offset).limit(per_page)
        )
    ).all()

--- a/guarddog_nexus/web/static/app.js
+++ b/guarddog_nexus/web/static/app.js
@@ -0,0 +1,25 @@
+// GuardDog Nexus — shared UI utilities
+
+function toggleFindings() {
+    var container = document.getElementById('findings-container');
+    if (!container) return;
+    var details = container.querySelectorAll('details');
+    if (details.length === 0) return;
+    var isOpen = details[0].open;
+    details.forEach(function (d) { d.open = !isOpen; });
+    var btn = document.querySelector('.toggle-all-btn');
+    if (btn) btn.textContent = isOpen ? 'Expand All' : 'Collapse All';
+}
+
+function copyCode(btn, codeId) {
+    var el = document.getElementById(codeId);
+    if (!el) return;
+    navigator.clipboard.writeText(el.textContent).then(function () {
+        btn.textContent = 'Copied!';
+        btn.classList.add('copied');
+        setTimeout(function () {
+            btn.textContent = 'Copy';
+            btn.classList.remove('copied');
+        }, 2000);
+    });
+}
--- a/guarddog_nexus/web/static/style.css
+++ b/guarddog_nexus/web/static/style.css
@@ -238,19 +238,9 @@ nav.sticky {
 /* Empty states */
 .empty-state {
    text-align: center;
-    padding: 3rem 1rem;
-    opacity: 0.6;
-}
-
-.empty-state svg {
-    width: 64px;
-    height: 64px;
-    margin-bottom: 1rem;
-    opacity: 0.4;
-}
-
-.empty-state h3 {
-    margin-bottom: 0.5rem;
+    padding: 2rem 1rem;
+    opacity: 0.5;
+    font-style: italic;
 }

 /* Filter bar */
@@ -352,7 +342,37 @@ th.sortable.active .sort-icon {
    }
 }

-/* Expand/Collapse all button */
+/* Finding header row */
+.finding-header-row {
+    display: flex;
+    justify-content: space-between;
+    align-items: center;
+    margin-bottom: 0.5rem;
+}
+
+/* Finding summary */
+.finding-summary {
+    cursor: pointer;
+    list-style: none;
+    display: flex;
+    align-items: center;
+    gap: 0.5rem;
+    padding: 0.25rem 0;
+}
+
+/* Finding summary hint */
+.finding-summary-hint {
+    margin-left: auto;
+    font-size: 0.8rem;
+    opacity: 0.5;
+}
+
+/* Code block toolbar */
+.code-toolbar {
+    display: flex;
+    justify-content: flex-end;
+    margin-bottom: 0.25rem;
+}
 .toggle-all-btn {
    font-size: 0.8rem;
    margin-bottom: 0.5rem;
--- a/guarddog_nexus/web/templates/base.html
+++ b/guarddog_nexus/web/templates/base.html
@@ -7,6 +7,7 @@
    <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@picocss/pico@2/css/pico.min.css">
    <script src="https://unpkg.com/htmx.org@2.0.4"></script>
    <link rel="stylesheet" href="/static/style.css">
+    <script src="/static/app.js"></script>
 </head>
 <body>
    <main class="container">
--- a/guarddog_nexus/web/templates/dashboard_stats.html
+++ b/guarddog_nexus/web/templates/dashboard_stats.html
@@ -40,11 +40,7 @@
    </div>
 </div>
 {% else %}
-<div class="empty-state" style="padding: 1rem;">
-    <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z"/></svg>
-    <h3>No findings yet</h3>
-    <small>Scan results will appear here once packages are processed.</small>
-</div>
+<p class="empty-state">No findings yet — scan results will appear here once packages are processed.</p>
 {% endif %}

 {% if days %}
--- a/guarddog_nexus/web/templates/package_detail.html
+++ b/guarddog_nexus/web/templates/package_detail.html
@@ -32,7 +32,7 @@
    </tbody>
 </table>

-<div style="display: flex; justify-content: space-between; align-items: center; margin-bottom: 0.5rem;">
+<div class="finding-header-row">
    <h2>Findings ({{ findings|length }})</h2>
    {% if findings|length > 1 %}
    <button class="toggle-all-btn" onclick="toggleFindings()">Collapse All</button>
@@ -42,56 +42,26 @@
 {% if findings %}
    <div id="findings-container">
    {% for f in findings %}
-    <details class="finding-card {{ f.severity }}" data-finding-id="{{ f.id }}">
-        <summary style="cursor: pointer; list-style: none; display: flex; align-items: center; gap: 0.5rem; padding: 0.25rem 0;">
-            <strong class="severity-{{ f.severity }}">[{{ f.severity }}]</strong>
-            <strong>{{ f.rule }}</strong>
-            {% if f.location %}<small> @ {{ f.location }}</small>{% endif %}
-            <span style="margin-left: auto; font-size: 0.8rem; opacity: 0.5;">click to expand</span>
+    <details class="finding-card {{ f.data.severity }}" data-finding-id="{{ f.id }}">
+        <summary class="finding-summary">
+            <strong class="severity-{{ f.data.severity }}">[{{ f.data.severity }}]</strong>
+            <strong>{{ f.data.rule }}</strong>
+            {% if f.data.location %}<small> @ {{ f.data.location }}</small>{% endif %}
+            <span class="finding-summary-hint">click to expand</span>
        </summary>
        <div class="finding-details">
-            <p>{{ f.message }}</p>
-            {% if f.code %}
-            <div style="display: flex; justify-content: flex-end; margin-bottom: 0.25rem;">
+            <p>{{ f.data.message }}</p>
+            {% if f.data.code %}
+            <div class="code-toolbar">
                <button class="copy-btn" onclick="copyCode(this, 'code-{{ f.id }}')">Copy</button>
            </div>
-            <pre><code id="code-{{ f.id }}">{{ f.code }}</code></pre>
+            <pre><code id="code-{{ f.id }}">{{ f.data.code }}</code></pre>
            {% endif %}
        </div>
    </details>
    {% endfor %}
    </div>
 {% else %}
-    <div class="empty-state" style="padding: 1rem;">
-        <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z"/></svg>
-        <h3>No findings</h3>
-        <small>Package looks clean.</small>
-    </div>
+    <p class="empty-state">No findings — package looks clean.</p>
 {% endif %}
 {% endblock %}
-
-{% block scripts %}
-<script>
-function toggleFindings() {
-    const container = document.getElementById('findings-container');
-    const details = container.querySelectorAll('details');
-    const first = details[0];
-    const isOpen = first && first.open;
-    details.forEach(d => d.open = !isOpen);
-    const btn = container.parentElement.querySelector('.toggle-all-btn');
-    if (btn) btn.textContent = isOpen ? 'Expand All' : 'Collapse All';
-}
-
-function copyCode(btn, codeId) {
-    const code = document.getElementById(codeId).textContent;
-    navigator.clipboard.writeText(code).then(() => {
-        btn.textContent = 'Copied!';
-        btn.classList.add('copied');
-        setTimeout(() => {
-            btn.textContent = 'Copy';
-            btn.classList.remove('copied');
-        }, 2000);
-    });
-}
-</script>
-{% endblock %}
--- a/guarddog_nexus/web/templates/packages_list.html
+++ b/guarddog_nexus/web/templates/packages_list.html
@@ -12,7 +12,7 @@

 <div class="filter-bar">
    <input type="text" id="search-input" placeholder="Search packages..." value="{{ search }}" hx-get="/packages" hx-trigger="input changed, keyup[entered] delay:300ms" hx-target="#packages-table-container" hx-swap="innerHTML">
-    <a href="?flagged={% if flagged_filter == '1' %}0{% else %}1{% endif %}" class="filter-btn" role="button" class="outline">
+    <a href="?flagged={% if flagged_filter == '1' %}0{% else %}1{% endif %}" role="button" class="outline">
        {% if flagged_filter == '1' %}Show all{% else %}Flagged only{% endif %}
    </a>
    <a href="/api/v1/packages/export?flagged={{ flagged_filter }}&search={{ search }}" role="button" class="outline">Export CSV</a>
@@ -53,11 +53,7 @@
    {% endfor %}
    {% if not packages %}
        <tr>
-            <td colspan="7" class="empty-state">
-                <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M20 7l-8-4-8 4m16 0l-8 4m8-4v10l-8 4m0-10L4 7m8 4v10M4 7v10l8 4"/></svg>
-                <h3>No packages found</h3>
-                <small>Try adjusting your search or filters.</small>
-            </td>
+            <td colspan="7" class="empty-state">No packages yet — packages will appear here once scans are processed.</td>
        </tr>
    {% endif %}
    </tbody>
--- a/guarddog_nexus/web/templates/scan_detail.html
+++ b/guarddog_nexus/web/templates/scan_detail.html
@@ -26,7 +26,7 @@
    {% if scan.error_message %}<tr><td><strong>Error</strong></td><td><span class="flagged">{{ scan.error_message }}</span></td></tr>{% endif %}
 </table>

-<div style="display: flex; justify-content: space-between; align-items: center; margin-bottom: 0.5rem;">
+<div class="finding-header-row">
    <h2>Findings ({{ scan.findings|length }})</h2>
    {% if scan.findings|length > 1 %}
    <button class="toggle-all-btn" onclick="toggleFindings()">Collapse All</button>
@@ -37,16 +37,16 @@
    <div id="findings-container">
    {% for f in scan.findings %}
    <details class="finding-card {{ f.data.severity }}" data-finding-id="{{ f.id }}">
-        <summary style="cursor: pointer; list-style: none; display: flex; align-items: center; gap: 0.5rem; padding: 0.25rem 0;">
+        <summary class="finding-summary">
            <strong class="severity-{{ f.data.severity }}">[{{ f.data.severity }}]</strong>
            <strong>{{ f.data.rule }}</strong>
            {% if f.data.location %}<small> @ {{ f.data.location }}</small>{% endif %}
-            <span style="margin-left: auto; font-size: 0.8rem; opacity: 0.5;">click to expand</span>
+            <span class="finding-summary-hint">click to expand</span>
        </summary>
        <div class="finding-details">
            <p>{{ f.data.message }}</p>
            {% if f.data.code %}
-            <div style="display: flex; justify-content: flex-end; margin-bottom: 0.25rem;">
+            <div class="code-toolbar">
                <button class="copy-btn" onclick="copyCode(this, 'code-{{ f.id }}')">Copy</button>
            </div>
            <pre><code id="code-{{ f.id }}">{{ f.data.code }}</code></pre>
@@ -56,36 +56,6 @@
    {% endfor %}
    </div>
 {% else %}
-    <div class="empty-state" style="padding: 1rem;">
-        <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z"/></svg>
-        <h3>No findings</h3>
-        <small>Package looks clean.</small>
-    </div>
+    <p class="empty-state">No findings — package looks clean.</p>
 {% endif %}
 {% endblock %}
-
-{% block scripts %}
-<script>
-function toggleFindings() {
-    const container = document.getElementById('findings-container');
-    const details = container.querySelectorAll('details');
-    const first = details[0];
-    const isOpen = first && first.open;
-    details.forEach(d => d.open = !isOpen);
-    const btn = container.parentElement.querySelector('.toggle-all-btn');
-    if (btn) btn.textContent = isOpen ? 'Expand All' : 'Collapse All';
-}
-
-function copyCode(btn, codeId) {
-    const code = document.getElementById(codeId).textContent;
-    navigator.clipboard.writeText(code).then(() => {
-        btn.textContent = 'Copied!';
-        btn.classList.add('copied');
-        setTimeout(() => {
-            btn.textContent = 'Copy';
-            btn.classList.remove('copied');
-        }, 2000);
-    });
-}
-</script>
-{% endblock %}
--- a/guarddog_nexus/web/templates/scans_list.html
+++ b/guarddog_nexus/web/templates/scans_list.html
@@ -19,7 +19,7 @@
        <option value="completed" {% if status_filter == 'completed' %}selected{% endif %}>Completed</option>
        <option value="failed" {% if status_filter == 'failed' %}selected{% endif %}>Failed</option>
    </select>
-    <a href="?flagged={% if flagged_filter == '1' %}0{% else %}1{% endif %}" class="filter-btn" role="button" class="outline">
+    <a href="?flagged={% if flagged_filter == '1' %}0{% else %}1{% endif %}" role="button" class="outline">
        {% if flagged_filter == '1' %}Show all{% else %}Flagged only{% endif %}
    </a>
    <a href="/api/v1/scans/export?flagged={{ flagged_filter }}&search={{ search }}&status={{ status_filter }}" role="button" class="outline">Export CSV</a>
@@ -64,11 +64,7 @@
    {% endfor %}
    {% if not scans %}
        <tr>
-            <td colspan="7" class="empty-state">
-                <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M21 21l-6-6m2-5a7 7 0 11-14 0 7 7 0 0114 0z"/></svg>
-                <h3>No scans found</h3>
-                <small>Try adjusting your search or filters.</small>
-            </td>
+            <td colspan="7" class="empty-state">No scans yet — scans will appear here once packages are processed.</td>
        </tr>
    {% endif %}
    </tbody>
				`@@ -1 +0,0 @@`
				`/* static/style.css - minimal overrides for Pico.css dark theme */`