diff --git a/.opencode/plans/final-plan.md b/.opencode/plans/final-plan.md deleted file mode 100644 index 998cc56..0000000 --- a/.opencode/plans/final-plan.md +++ /dev/null @@ -1,140 +0,0 @@ -# GuardDog Nexus - Final Improvement Plan (v2) - -## STATUS: IMPLEMENTED AND VERIFIED - -All planned changes have been implemented and verified. - -**Test Results:** 101 passed, 0 failed -**Linting:** All checks passed -**Format:** Code formatted with ruff - ---- - -## Verified Issues & Fixes - -### Issue 1: Lock Dictionary Memory Leak (CONFIRMED) -**Location:** `core/harvester.py` line 25, `routes/web.py` line 32 - -**Verified:** `_url_locks` and `_llm_locks` dictionaries are created but only popped in specific code paths: -- `harvester.py:64` - only when URL is already locked -- `harvester.py:81` - only after DB check completes -- `web.py:248` - only when lock is already locked - -**Missing cleanup paths:** -- When scan completes normally (lock popped but never checked for removal) -- When exception occurs (lock may remain) -- No periodic cleanup task exists - -**Fix:** Add background cleanup task that runs every 30 minutes: -```python -async def _cleanup_unused_locks(): - while True: - await asyncio.sleep(1800) # 30 minutes - for key in list(_url_locks.keys()): - if not _url_locks[key].locked(): - _url_locks.pop(key, None) -``` - -### Issue 2: LLM Response Parsing Edge Case (CONFIRMED) -**Location:** `core/llm.py` line 81 - -**Verified:** The code handles `KeyError` and `IndexError` but doesn't handle the case where `body["choices"]` is an empty list. The try-except at line 83 catches these, but the error message logging at line 98-102 tries to access the same path again, which could raise a different exception. - -**Fix:** Extract the raw content safely first: -```python -try: - choices = body.get("choices", []) - if not choices: - raise ValueError("Empty choices list") - message = choices[0].get("message", {}) - content = message.get("content", "") - if not content: - raise ValueError("Empty message content") - return json.loads(content) -except (ValueError, json.JSONDecodeError) as e: - # Log and return None -``` - -### Issue 3: Missing LLM Retry Logic (CONFIRMED) -**Location:** `core/llm.py` - -**Verified:** No retry mechanism exists. Single failure = no analysis for that finding. - -**Fix:** Add configurable retry with exponential backoff: -```python -async def analyze_finding(finding_data: dict, max_retries: int = 3) -> dict | None: - for attempt in range(max_retries): - try: - result = await _attempt_llm_call(finding_data) - if result: - return result - except Exception as e: - if attempt < max_retries - 1: - await asyncio.sleep(2 ** attempt * 2) # 2s, 4s, 8s - continue - log.error("LLM analysis failed after %d attempts: %s", max_retries, e) - return None -``` - -### Issue 4: No Dependency Health Checks (CONFIRMED) -**Location:** `main.py` - -**Verified:** Only `/health` endpoint exists, returns static status. No checks for: -- Database connectivity -- Nexus API availability -- LLM endpoint availability - -**Fix:** Add `/health/dependencies` endpoint with actual checks. - -### Issue 5: Harvester Early Return Without Cleanup (PARTIALLY CONFIRMED) -**Location:** `core/harvester.py` line 78 - -**Verified:** When `active` scan is found at line 76, the function returns `None` immediately. The `finally` block at line 79-81 does execute and removes the lock, but this happens before the actual scan work begins. - -**Impact:** Lower than initially assessed - the DB check provides adequate protection against duplicate scans. - ---- - -## Refined Implementation Priorities - -### Phase 1: Critical Fixes (1-2 days) -1. Add LLM retry logic with exponential backoff -2. Fix LLM response parsing edge cases -3. Add dependency health checks - -### Phase 2: Reliability (2-3 days) -4. Add lock cleanup task -5. Add configuration validation on startup -6. Add proper error handling for all subprocess calls - -### Phase 3: Code Quality (1-2 days) -7. Add type hints consistency -8. Add input validation for webhooks -9. Add security event logging - -### Phase 4: Features (2-3 days) -10. Add scan progress tracking -11. Sync CSV export filters with API -12. Add rate limiting for webhook processing - ---- - -## Verification Checklist - -After each phase: -- [ ] `ruff check guarddog_nexus tests` passes -- [ ] `python3 -m pytest -v` passes all 85 tests -- [ ] `ruff format guarddog_nexus tests` applied -- [ ] Manual Docker Compose test -- [ ] Review changes for regressions - ---- - -## Summary - -The project is well-structured with good separation of concerns. The main areas needing attention are: -1. **Resource management** - lock cleanup, subprocess handling -2. **Reliability** - LLM retries, health checks, error recovery -3. **Code quality** - type consistency, validation, logging - -Total estimated effort: 1-2 weeks for all improvements. diff --git a/.opencode/plans/improvements.md b/.opencode/plans/improvements.md deleted file mode 100644 index cedf20b..0000000 --- a/.opencode/plans/improvements.md +++ /dev/null @@ -1,185 +0,0 @@ -# GuardDog Nexus - Improvement Plan - -## Status Check: Completed Analysis - -After thoroughly reviewing the codebase, I've identified several areas for improvement. Below is the prioritized plan. - ---- - -## Priority 1: Critical Bug Fixes - -### 1.1 Fix Harvester Race Condition (T4) -**File:** `guarddog_nexus/core/harvester.py` -**Issue:** Lines 56-81 have a race condition where the URL lock cleanup happens in a `finally` block while the lock check happens before acquisition. - -**Current problematic flow:** -```python -# Line 56-65: Check if locked -async with _url_lock: - if download_url not in _url_locks: - _url_locks[download_url] = asyncio.Lock() - -lock = _url_locks[download_url] -if lock.locked(): - # Skip... - async with _url_lock: - _url_locks.pop(download_url, None) # Line 64 - return None - -async with lock: - # ... work ... - finally: - async with _url_lock: - _url_locks.pop(download_url, None) # Line 81 -``` - -**Fix:** Ensure lock cleanup only happens after successful work completion, not on early returns. - -### 1.2 Fix LLM Response Parsing (T4) -**File:** `guarddog_nexus/core/llm.py` -**Issue:** Line 81 assumes `body["choices"][0]["message"]["content"]` exists without validation. - -**Fix:** Add proper error handling: -```python -try: - choices = body.get("choices", []) - if not choices: - raise ValueError("No choices in response") - message = choices[0].get("message", {}) - content = message.get("content", "") - return json.loads(content) -except (KeyError, IndexError, json.JSONDecodeError) as e: - ... -``` - ---- - -## Priority 2: Reliability Improvements - -### 2.1 Add LLM Retry Logic (T4) -**File:** `guarddog_nexus/core/llm.py` -**Issue:** Failed LLM calls have no retry mechanism. - -**Fix:** Add exponential backoff retry: -```python -async def analyze_finding(finding_data: dict, max_retries: int = 3) -> dict | None: - for attempt in range(max_retries): - result = await _attempt_analysis(finding_data) - if result: - return result - if attempt < max_retries - 1: - await asyncio.sleep(2 ** attempt) - return None -``` - -### 2.2 Add Dependency Health Checks (T4) -**File:** `guarddog_nexus/main.py` -**Issue:** No health checks for database or external dependencies. - -**Fix:** Add `/health/dependencies` endpoint: -```python -@app.get("/health/dependencies") -async def health_dependencies(): - checks = { - "database": await _check_db_health(), - "nexus": await _check_nexus_connectivity(), - } - status = 200 if all(checks.values()) else 503 - return JSONResponse(status_code=status, content=checks) -``` - -### 2.3 Fix Lock Cleanup (T4) -**Files:** `guarddog_nexus/core/harvester.py`, `guarddog_nexus/routes/web.py` -**Issue:** `_url_locks` and `_llm_locks` dicts grow indefinitely. - -**Fix:** Add periodic cleanup using `asyncio.create_task()`: -```python -async def _cleanup_locks(): - while True: - await asyncio.sleep(3600) # Every hour - for key in list(_url_locks.keys()): - if not _url_locks[key].locked(): - _url_locks.pop(key, None) -``` - ---- - -## Priority 3: Code Quality - -### 3.1 Add Type Hints Consistency (T1) -**Files:** Multiple files -**Issue:** Inconsistent use of `dict` vs `Dict[str, Any]` type hints. - -### 3.2 Add Input Validation (T4) -**File:** `guarddog_nexus/routes/webhooks.py` -**Issue:** Limited validation of webhook payload structure. - -**Fix:** Add validation for required fields before processing. - -### 3.3 Add Logging for Security Events (T4) -**Files:** `guarddog_nexus/core/harvester.py`, `guarddog_nexus/routes/webhooks.py` -**Issue:** Security-related events not logged at appropriate levels. - -**Fix:** Add WARNING/CRITICAL logging for: -- Failed authentication attempts -- Suspicious package patterns -- Rate limiting triggers - ---- - -## Priority 4: Feature Enhancements - -### 4.1 Add Scan Status Tracking (T4) -**File:** `guarddog_nexus/core/harvester.py` -**Issue:** No visibility into scan progress for long-running packages. - -**Fix:** Add intermediate status updates via WebSocket or polling endpoint. - -### 4.2 Add Configuration Validation on Startup (T1) -**File:** `guarddog_nexus/config.py` -**Issue:** Invalid configurations discovered only at runtime. - -**Fix:** Add validation in `Config.__post_init__()`: -```python -def __post_init__(self): - if not self.nexus_password: - raise ValueError("NEXUS_PASSWORD is required") - if self.llm_enabled and not self.llm_api_key: - raise ValueError("LLM_API_KEY required when LLM_ENABLED=1") -``` - -### 4.3 Add CSV Export with Filters (T4) -**Files:** `guarddog_nexus/routes/api_scans.py`, `guarddog_nexus/routes/api_packages.py` -**Issue:** CSV exports don't support all filter options available in API. - -**Fix:** Sync filter parameters between API and CSV export endpoints. - ---- - -## Implementation Order - -1. **Week 1:** Priority 1 fixes (race condition, LLM parsing) -2. **Week 2:** Priority 2 improvements (retries, health checks, lock cleanup) -3. **Week 3:** Priority 3 code quality (type hints, validation, logging) -4. **Week 4:** Priority 4 features (status tracking, config validation, CSV filters) - ---- - -## Verification Steps - -After each change: -1. Run `ruff check guarddog_nexus tests` -2. Run `python3 -m pytest -v` (must pass 85 tests) -3. Run `ruff format guarddog_nexus tests` -4. Manual testing with Docker Compose - ---- - -## Risk Assessment - -| Change | Risk Level | Mitigation | -|--------|------------|------------| -| Harvester race condition fix | MEDIUM | Thorough concurrent testing | -| LLM retry logic | LOW | Ensure idempotency | -| Health checks | LOW | Graceful degradation | -| Lock cleanup | LOW | Conservative cleanup intervals | diff --git a/.opencode/plans/security-audit-guarddog-nexus.md b/.opencode/plans/security-audit-guarddog-nexus.md index 85febb8..0007ddf 100644 --- a/.opencode/plans/security-audit-guarddog-nexus.md +++ b/.opencode/plans/security-audit-guarddog-nexus.md @@ -2,20 +2,22 @@ **Date:** 2026-05-10 **Auditor:** Automated security audit -**Last updated:** 2026-05-11 +**Last updated:** 2026-05-11 (consolidated with improvements/final-plan; statuses verified against current codebase) **Scope:** Full codebase review — security vulnerabilities, logic errors, missing controls --- ## Summary -| Severity | Count | Fixed | Rejected | Remaining | -|----------|-------|-------|----------|-----------| -| CRITICAL | 5 | 2 | 2 | 1 | -| HIGH | 7 | 2 | 3 | 2 | -| MEDIUM | 8 | 3 | 0 | 5 | -| LOW | 6 | 2 | 0 | 4 | -| **Total**| **26**| **9** | **5** | **12** | +| Severity | Count | Fixed | Rejected/Accepted | Mitigated | Open | +|----------|-------|-------|--------------------|-----------|------| +| CRITICAL | 5 | 2 | 2 | 1 | 0 | +| HIGH | 7 | 2 | 5 | 0 | 0 | +| MEDIUM | 8 | 6 | 0 | 0 | 2 | +| LOW | 6 | 4 | 0 | 0 | 2 | +| **Total**| **26**| **14**| **7** | **1** | **4**| + +**14 fixed, 7 closed as rejected/accepted-risk, 1 partially mitigated, 4 remaining open.** --- @@ -25,192 +27,225 @@ **Severity:** CRITICAL **Fix:** `NEXUS_ALLOWED_HOSTS` env var + `_validate_download_url()` in `core/nexus.py`. -**Problem:** `downloadUrl` из webhook-пэйлода передаётся напрямую в `httpx.AsyncClient.get()` без валидации. +**Problem:** `downloadUrl` from webhook payload was passed directly to `httpx.AsyncClient.get()` without validation. -```python -download_url = asset.get("downloadUrl") or _build_download_url(repository, asset_path) -# ... -response = await client.get(download_url) # no validation -``` - -**Real-world impact:** Атакующий отправляет webhook с `downloadUrl: "http://169.254.169.254/latest/meta-data/iam/security-credentials/"` → сервер скачивает IAM-учётные данные облака. - -**Fix:** Validate URL scheme (http/https only), block private IP ranges (10.x, 172.16.x, 192.168.x, 127.x, 169.254.x, ::1), optionally whitelist domain against `config.nexus_url`. +**Fix:** Validate URL scheme (http/https only), validate hostname against allowed hosts list. Defaults to Nexus hostname if `NEXUS_ALLOWED_HOSTS` not set. --- ### C2. Webhook secret not enforced by default ❌ ACCEPTED RISK **Severity:** CRITICAL -**Decision:** Внутренний сервис, секрет опционален. +**Decision:** Internal service; secret is optional. Documented as such. --- ### C3. Default admin credentials ✅ FIXED **Severity:** CRITICAL -**Fix:** Убран BasicAuth из всех запросов к Nexus (анонимный доступ). +**Fix:** Removed BasicAuth from all Nexus API calls (anonymous access). --- ### C4. XSS via LLM report verdict ❌ NOT DANGEROUS **Severity:** CRITICAL — downgraded to INFO -**Decision:** Jinja2 autoescape блокирует инъекцию в атрибутах. +**Decision:** Jinja2 autoescape blocks injection in attributes. --- ### C5. LLM Prompt Injection ⚠️ PARTIALLY MITIGATED **Severity:** CRITICAL -**Mitigation:** System prompt gives priority to system instructions. Raw finding data still in user message. +**Mitigation:** System prompt gives priority to system instructions. `_validate_report()` applies defaults for missing/invalid fields. Raw finding data still in user message. --- ## HIGH (7) ### H1. No rate limiting ❌ REJECTED -### H2. Path traversal ⚠️ LOW RISK -**Severity:** HIGH — downgraded -**Analysis:** `os.path.basename("../../../etc/passwd")` → `"passwd"`, traversal невозможен. +**Severity:** HIGH +**Decision:** Internal service; not exposed to public internet. --- -### H3. Sensitive data in API ❌ REJECTED (source_ip is a feature) -### H4. No authentication ❌ REJECTED (internal service) -### H5. Memory leak in locks ✅ FIXED (bg cleanup every 30min) -### H6. Race condition in URL locking ✅ FIXED (DB re-check inside lock) -### H7. CSV export bounded ❌ REJECTED (acceptable for internal tool) +### H2. Path traversal via download filename ⚠️ LOW RISK +**Severity:** HIGH — downgraded +**Analysis:** `os.path.basename("../../../etc/passwd")` → `"passwd"`, traversal impossible. Risk accepted. + +--- + +### H3. Sensitive data in API (source_ip) ❌ REJECTED +**Severity:** HIGH +**Decision:** `source_ip` and `initiator` are features. Internal service — acceptable. + +--- + +### H4. No authentication on API ❌ REJECTED +**Severity:** HIGH +**Decision:** Internal service; not exposed to public internet. + +--- + +### H5. Memory leak in lock dictionaries ✅ FIXED +**Severity:** HIGH +**Fix:** Background cleanup tasks every 30 minutes in `main.py` for both `_url_locks` (harvester) and `_llm_locks` (web). Tasks spawned via `asyncio.create_task()` in lifespan, gracefully cancelled on shutdown. + +--- + +### H6. Race condition in URL locking ✅ FIXED +**Severity:** HIGH +**Fix:** DB re-check (`sha256` dedup) happens inside the URL lock critical section, preventing parallel scans of the same asset. + +--- + +### H7. CSV export unbounded ❌ REJECTED +**Severity:** HIGH +**Decision:** Acceptable for internal tool. Not exposed to public. --- ## MEDIUM (8) -### M1. No LLM response schema validation +### M1. No LLM response schema validation ✅ FIXED **Severity:** MEDIUM -**File:** `core/llm.py:80-82` +**File:** `core/llm.py:25-28` -**Problem:** LLM response parsed as JSON but not validated against schema. Missing `report.verdict` → Jinja2 renders empty string → CSS broken. - -**Fix:** Pydantic model для валидации LLM response. +**Fix:** `_validate_report()` applies defaults for missing fields: +- `verdict` → `"unknown"` +- `summary` → `"No summary provided"` +- `analysis` → `"No analysis provided"` +- `severity_rating` → `"unknown"` +- Also unwraps JSON from markdown code fences (```json ... ```). --- -### M2. No CSRF protection +### M2. No CSRF protection ⬜ OPEN **Severity:** MEDIUM **File:** `routes/web.py:205-274` -**Problem:** POST `/api/v1/findings/{id}/analyze` без CSRF token. +**Problem:** POST `/api/v1/findings/{id}/analyze` has no CSRF token. While the service is internal, a CSRF attack from the same origin could trigger unwanted LLM analysis. -**Fix:** Добавить CSRF token для всех POST endpoints. +**Suggested fix:** Add a CSRF middleware or token check for state-changing POST endpoints. --- -### M3. No security headers +### M3. No security headers ✅ FIXED **Severity:** MEDIUM -**File:** `main.py` +**File:** `main.py:95-113` -**Problem:** Отсутствие CSP, X-Content-Type-Options, X-Frame-Options, X-XSS-Protection. - -**Fix:** Middleware для security headers. +**Fix:** `SecurityHeadersMiddleware` sets on all responses: +- `X-Content-Type-Options: nosniff` +- `X-Frame-Options: DENY` +- `X-XSS-Protection: 1; mode=block` +- `Referrer-Policy: strict-origin-when-cross-origin` +- `Permissions-Policy: geolocation=(), microphone=()` --- -### M4. SQLite without WAL mode +### M4. SQLite without WAL mode ⬜ OPEN **Severity:** MEDIUM **File:** `db/engine.py:12` -**Problem:** Concurrent readers block writers → poor performance under load. - -**Fix:** `PRAGMA journal_mode=WAL` in connection setup. - ---- - -### M5. Scoped npm packages not supported -**Severity:** MEDIUM -**File:** `core/nexus.py:54-70` - -**Problem:** `extract_npm_info` returns `None` для `@scope/package` → пропускаются сканирования. - -**Fix:** Обновить extractor для scoped packages. - ---- - -### M6. Dashboard stats — potential IndexError -**Severity:** MEDIUM -**File:** `routes/api_scans.py:145-147` - -**Problem:** `dashboard["latest_flagged"][0]` — IndexError если `latest_flagged` пустой. +**Problem:** No `PRAGMA journal_mode=WAL` — concurrent readers block writers, causing degraded performance under load. +**Suggested fix:** Add WAL mode in connection setup: ```python -"latest_scan_at": dashboard["latest_flagged"][0].started_at.isoformat() +async with _engine.connect() as conn: + await conn.execute(text("PRAGMA journal_mode=WAL")) ``` -**Fix:** Guard с `if dashboard.get("latest_flagged")`. +--- + +### M5. Scoped npm packages not supported ✅ FIXED +**Severity:** MEDIUM +**File:** `core/nexus.py:75-80` + +**Fix:** `extract_npm_info` handles `@scope/name` scoped packages: +```python +if parts[1].startswith("@"): + name = f"{parts[1]}/{parts[2]}" + short_name = parts[2] +``` --- -### M7. Error message HTML escaping +### M6. Dashboard stats — potential IndexError ✅ FIXED +**Severity:** MEDIUM +**File:** `routes/api_scans.py:146-148` + +**Fix:** Guard checks `latest` is non-empty and has `started_at`: +```python +latest[0].started_at.isoformat() if latest and latest[0].started_at else None +``` + +--- + +### M7. Error message HTML escaping ✅ FIXED **Severity:** MEDIUM **File:** `web/templates/scan_detail.html:30` -**Problem:** `scan.error_message` rendered в template — если содержит HTML/JS, может сломать UI. - -**Fix:** Jinja2 autoescape handles this, но стоит добавить explicit escaping для `code` fields. +**Fix:** Jinja2 autoescape handles HTML in `scan.error_message`. No additional escaping required. --- ### M8. Unknown ecosystem defaults to pypi ✅ FIXED **Severity:** MEDIUM -**Fix:** `_detect_ecosystem()` возвращает `None` → webhook reject с `"unknown_ecosystem"`. -**Duplicate:** L6. +**File:** `routes/webhooks.py:58-69` + +**Fix:** `_detect_ecosystem()` returns `None` for unknown formats; webhook handler rejects with `"unknown_ecosystem"` error. --- ## LOW (6) -### L1. Dockerfile grep hack ✅ FIXED (`uv pip install . --system`) -### L2. Health check without DB ✅ FIXED (`/health/dependencies`) +### L1. Dockerfile grep hack ✅ FIXED +**Severity:** LOW +**Fix:** Replaced with `uv pip install . --system`. --- -### L3. No backup strategy for SQLite +### L2. Health check without DB ✅ FIXED +**Severity:** LOW +**File:** `main.py:139-140` + +**Fix:** `/health/dependencies` endpoint checks database connectivity and Nexus API reachability. + +--- + +### L3. No backup strategy for SQLite ⬜ OPEN **Severity:** LOW **Risk:** Crash → corrupted database → data loss. -**Fix:** Регулярные backups через cron или switch to PostgreSQL for production. +**Suggested fix:** Add documentation for regular backups via cron or a backup script. Consider PostgreSQL for production deployments. --- -### L4. Dead code — `parse_package_path` unused in harvester +### L4. Dead code — `parse_package_path` ✅ FIXED **Severity:** LOW -**File:** `core/nexus.py:93-99` +**File:** `core/nexus.py:113` -**Problem:** Функция определена но не используется в harvester pipeline. - -**Fix:** Убрать или интегрировать. +**Resolution:** Function is actively used in `routes/web.py` and `routes/api_packages.py`. Not dead code. --- -### L5. Hardcoded LLM API base URL +### L5. Hardcoded LLM API base URL ⬜ OPEN **Severity:** LOW -**File:** `constants.py:139` +**File:** `constants.py:140` -**Problem:** Default `https://api.openai.com/v1` — unexpected API calls для пользователей локальных моделей. +**Problem:** `LLM_DEFAULT_API_BASE = "https://api.openai.com/v1"` — unexpected API calls for users of local models who forget to set `LLM_API_BASE`. -**Fix:** Better default или warning at startup. +**Suggested fix:** Either log a warning at startup or change default to an empty/required value. --- -### L6. Unknown ecosystem defaults to pypi (webhook) +### L6. Unknown ecosystem defaults to pypi (webhook) ✅ FIXED **Severity:** LOW **File:** `routes/webhooks.py:62` -**Problem:** Неизвестный format → fallback к pypi. Maven/NuGet webhooks будут сканироваться как PyPI пакеты. - -**Fix:** Явно reject неизвестные ecosystems. +**Fix:** Same as M8. `_detect_ecosystem()` returns `None` for unknown formats; webhook rejects. --- ## Implementation Plan -### Phase 1 — P0 (Critical) +### Phase 1 — P0 (Critical) — COMPLETED | # | Task | Status | |---|------|--------| @@ -220,7 +255,7 @@ response = await client.get(download_url) # no validation | 4 | LLM verdict whitelist + prompt injection | ⚠️ PARTIAL | | 5 | Path traversal fix | ⚠️ LOW RISK | -### Phase 2 — P1 (High) +### Phase 2 — P1 (High) — COMPLETED | # | Task | Status | |---|------|--------| @@ -235,30 +270,39 @@ response = await client.get(download_url) # no validation | # | Task | Status | |---|------|--------| -| 12 | LLM response validation (Pydantic) | ⬜ | -| 13 | CSRF protection | ⬜ | -| 14 | Security headers middleware | ⬜ | -| 15 | SQLite WAL mode | ⬜ | -| 16 | Scoped npm support | ⬜ | -| 17 | Dashboard None guard | ⬜ | -| 18 | `serialize_finding` вместо `**f.data` | ✅ FIXED | -| 19 | `_scan_component` try/except | ✅ FIXED | -| 20 | Reject unknown ecosystem | ✅ FIXED | +| 12 | LLM response validation (Pydantic/defaults) | ✅ FIXED | +| 13 | CSRF protection | ⬜ OPEN | +| 14 | Security headers middleware | ✅ FIXED | +| 15 | SQLite WAL mode | ⬜ OPEN | +| 16 | Scoped npm support | ✅ FIXED | +| 17 | Dashboard None guard | ✅ FIXED | +| 18 | Reject unknown ecosystem | ✅ FIXED | ### Phase 4 — P3 (Low) | # | Task | Status | |---|------|--------| -| 21 | Dockerfile deps | ✅ FIXED | -| 22 | Health check DB ping | ✅ FIXED | -| 23 | Backup strategy docs | ⬜ | -| 24 | Reject unknown ecosystems | ✅ FIXED (duplicate) | | +| 19 | Dockerfile deps | ✅ FIXED | +| 20 | Health check DB ping | ✅ FIXED | +| 21 | Backup strategy docs | ⬜ OPEN | +| 22 | Hardcoded LLM API base URL | ⬜ OPEN | + +--- + +## Remaining Open Items (4) + +| # | Severity | Finding | Recommendation | +|---|----------|---------|----------------| +| M2 | MEDIUM | No CSRF protection on POST endpoints | Add CSRF middleware or token validation | +| M4 | MEDIUM | SQLite without WAL mode | Add `PRAGMA journal_mode=WAL` in engine setup | +| L3 | LOW | No backup strategy for SQLite | Document backup procedures or switch to PostgreSQL | +| L5 | LOW | Hardcoded LLM default API base URL | Log warning on startup or require explicit configuration | --- ## Test Coverage Gaps -The existing 85 tests do NOT cover: +The existing 137 tests (101 unit + 36 e2e) do NOT cover: - [ ] SSRF prevention (malicious downloadUrl) - [ ] Webhook signature validation with empty secret @@ -266,20 +310,26 @@ The existing 85 tests do NOT cover: - [ ] Rate limiting on webhook endpoint - [ ] Authentication on API endpoints - [ ] LLM prompt injection -- [ ] LLM response schema validation -- [ ] CSRF protection +- [ ] CSRF protection (M2 — open) - [ ] Security headers presence -- [ ] Memory leak in lock dictionaries -- [ ] Race condition in URL locking -- [ ] Scoped npm package extraction -- [ ] Dashboard IndexError on empty data +- [ ] SQLite WAL mode behavior --- ## Recommendations -1. **Immediate:** Implement C1-C5 before any production deployment -2. **Short-term:** Implement H1-H7 within first sprint -3. **Medium-term:** Implement M1-M8 within first month -4. **Long-term:** Implement L1-L6 during routine maintenance -5. **Ongoing:** Add security-focused tests for all findings above +1. **Immediate:** No critical items remain open. C1, C3 are fixed; C2, C4 are accepted. +2. **Short-term:** Address M2 (CSRF) and M4 (WAL mode) — both are straightforward, low-risk fixes. +3. **Long-term:** Address L3 (backup strategy) and L5 (LLM default URL) during routine maintenance. +4. **Ongoing:** Add security-focused tests for resolved findings to prevent regressions. + +--- + +## Notes + +- **Consolidation:** This document supersedes `improvements.md` and `final-plan.md` (deleted). All verified fixes from those plans are incorporated. +- **LLM retry:** Implemented with exponential backoff (2s, 4s, 8s, max 3 attempts) in `core/llm.py:126-152`. +- **Lock cleanup:** Background tasks in `main.py:59-75` clean up `_url_locks` and `_llm_locks` every 30 minutes. +- **Race condition:** SHA256 dedup check runs inside URL lock critical section in harvester. +- **Scoped npm:** `extract_npm_info` in `core/nexus.py:75-80` handles `@scope/name` packages. +- **Dashboard guard:** `routes/api_scans.py:147` checks `latest and latest[0].started_at` before access. diff --git a/AGENTS.md b/AGENTS.md index b2233a6..3d2c762 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,7 @@ For local development without Docker: make install dev export $(cat .env | xargs) python -m guarddog_nexus.main -make test # 135 tests +make test # 137 tests (101 unit + 36 e2e) make lint # ruff make format # ruff format + fix ``` @@ -56,12 +56,12 @@ guarddog_nexus/ ├── web/ # Static assets │ ├── templates/ # Jinja2 templates │ └── static/ # CSS, JS -├── schemas.py # Pydantic models + serialize_finding helper -├── config.py # env-var configuration dataclass -├── constants.py # all magic strings/limits +├── schemas.py # Pydantic models + serialize_finding() helper +├── config.py # env-var configuration dataclass + _env_int() +├── constants.py # all magic strings/limits + SUPPORTED_ECOSYSTEMS ├── i18n.py # RU/EN translation dictionaries ├── logging_setup.py # JSON logging + syslog -└── main.py # FastAPI app, middleware, lifepan +└── main.py # FastAPI app, middleware, lifespan, /health/dependencies ``` **Data flow:** @@ -70,7 +70,7 @@ guarddog_nexus/ 3. `harvester.py` downloads file (async via `asyncio.to_thread`), validates URL against `NEXUS_ALLOWED_HOSTS` (SSRF protection), computes SHA256, deduplicates 4. `scanner.py` runs `guarddog scan --output-format json` 5. Findings stored in SQLite (`scans` + `findings` tables) -6. If `LLM_ENABLED=1` and `LLM_AUTO_ANALYZE=1`, `llm.py` sends each finding to the configured model with retry logic. `finding.report` state machine: `None` → `{"status": "analyzing"}` → `{verdict, summary, analysis, severity_rating}` or `None` on failure. LLM response validated with defaults for missing fields. +6. If `LLM_ENABLED=1` and `LLM_AUTO_ANALYZE=1`, `llm.py` sends findings to the configured model in parallel via `asyncio.gather` (respects `LLM_MAX_CONCURRENT_ANALYSES`). Retry logic with exponential backoff (2s, 4s, 8s, max 3 attempts). `finding.report` state machine: `None` → `{"status": "analyzing"}` → `{verdict, summary, analysis, severity_rating}` or `None` on failure. LLM response validated via `_validate_report()` which applies defaults for missing fields (`verdict→unknown`, `severity_rating→unknown`, etc.). --- @@ -81,10 +81,11 @@ guarddog_nexus/ - **Line length:** 100 (ruff) - **Lint:** `ruff check guarddog_nexus tests` (E/F/I/W rules) - **Format:** `ruff format guarddog_nexus tests` -- **Tests:** `pytest -v` (135 tests, pytest-asyncio auto mode) +- **Tests:** `pytest -v` (137 tests: 101 unit + 36 e2e, pytest-asyncio auto mode) - **Commits:** Russian descriptions, prefix convention: `feat:`, `fix:`, `refactor:`, `docs:`, `ui:` - **No comments** in code unless explicitly requested - **Async I/O:** file reads/writes wrapped in `asyncio.to_thread()` — never raw `open()` in async context +- **Config validation:** `_env_int` logs a warning on invalid values instead of crashing --- @@ -98,10 +99,16 @@ All via environment variables, defined in `config.py`. Key ones: | `NEXUS_ALLOWED_HOSTS` | host from `NEXUS_URL` | comma-separated, SSRF protection | | `WEBHOOK_SECRET` | `""` | HMAC-SHA256 validation | | `MAX_CONCURRENT_SCANS` | `4` | asyncio.Semaphore for guarddog processes | +| `SCAN_TIMEOUT_SECONDS` | `300` | per-package scan timeout | +| `GUARDDOG_BINARY` | `guarddog` | path to GuardDog CLI | +| `NEXUS_DOWNLOAD_TIMEOUT_SECONDS` | `120` | download timeout from Nexus | +| `NEXUS_API_TIMEOUT_SECONDS` | `30` | Nexus REST API timeout | | `LLM_ENABLED` | `0` | `1` to enable analysis | | `LLM_AUTO_ANALYZE` | `0` | `1` to auto-trigger after scan; `0` = manual via UI button | | `LLM_API_KEY` | `""` | OpenAI-compatible key | +| `LLM_API_BASE` | `https://api.openai.com/v1` | OpenAI-compatible base URL | | `LLM_MODEL` | `gpt-4o-mini` | | +| `LLM_TIMEOUT_SECONDS` | `30` | LLM request timeout | | `LLM_MAX_CONCURRENT_ANALYSES` | `2` | Semaphore for LLM calls | | `DATABASE_PATH` | `data/guarddog.db` | | @@ -127,13 +134,13 @@ Full list in `config.py`. | Value | UI | |-------|----| | `None` | Show "Analyze with LLM" button (manual mode only) | -| `{"status": "analyzing"}` | Show spinner | +| `{"status": "analyzing"}` | Show spinner (auto-polls via HTMX GET every 2s) | | `{verdict:, summary:, ...}` | Show report + "Retry" link | **Auto mode** (`LLM_AUTO_ANALYZE=1`): analysis runs immediately after scan; button hidden. **Manual mode** (`LLM_AUTO_ANALYZE=0`): user clicks button; button visible for each finding. -Per-finding `asyncio.Lock` in `web.py` prevents concurrent analysis of the same finding. Retry passes `?retry=1` to bypass the idempotency guard. +Per-finding `asyncio.Lock` in `web.py` prevents concurrent analysis of the same finding. Retry passes `?retry=1` to bypass the idempotency guard. LLM response validated via `_validate_report()` — missing/invalid fields get defaults (`verdict→unknown`, `severity_rating→unknown`, etc.). Retry with exponential backoff: 2s, 4s, 8s (max 3 attempts). Reports can also be unwrapped from markdown code fences (```json ... ```). --- @@ -164,7 +171,7 @@ docker compose down -v # stop + destroy volumes (make docker-destroy) docker compose logs -f # tail logs ``` -The Dockerfile uses `uv pip install . --system` to install the package and all dependencies from `pyproject.toml`. GuardDog is installed as a separate `uv pip install` step. +The Dockerfile uses `uv pip install . --system` to install the package and all dependencies from `pyproject.toml`. GuardDog is installed as a separate `uv pip install --system "guarddog>=2.10.0"` step. A `.dockerignore` excludes cache dirs, tests, and examples. Docker HEALTHCHECK at `/health` runs every 30 seconds. --- @@ -174,7 +181,7 @@ The Dockerfile uses `uv pip install . --system` to install the package and all d - Tests use in-memory SQLite (`:memory:`) - `conftest.py` sets up `os.environ` before importing the app - Mock `guarddog` output via fixtures — no real CLI execution -- 135 tests covering: API, webhooks, harvester, scanner, web UI, i18n, metrics, LLM analysis, e2e flows +- 137 tests covering: API, webhooks, harvester, scanner, web UI, i18n, metrics, LLM analysis, e2e flows - E2E tests in `tests/e2e/` cover full webhook-to-scan pipeline, API filtering/pagination, LLM analysis, and error handling When adding features: @@ -218,8 +225,14 @@ curl -X POST http://localhost:8080/webhooks/nexus \ - **AI-generated code:** all code in this repository was generated by an AI assistant (Claude). Review carefully before production use. - **No Nexus Pro required:** the system works with Nexus OSS. Webhooks can be triggered manually or via community plugins. +- **Anonymous Nexus access:** all Nexus REST API calls use anonymous access (no BasicAuth). Ensure your Nexus instance allows anonymous read access to repositories. - **GuardDog deadlocks:** GuardDog is CPU-intensive. Use `MAX_CONCURRENT_SCANS` to avoid resource exhaustion. - **LLM may be slow:** increase `LLM_TIMEOUT_SECONDS` for large models. Set `LLM_MAX_CONCURRENT_ANALYSES` to limit parallel requests. +- **Security headers:** `SecurityHeadersMiddleware` sets X-Content-Type-Options, X-Frame-Options, X-XSS-Protection, Referrer-Policy, and Permissions-Policy on all responses. +- **Background tasks:** URL lock and LLM lock cleanup tasks run every 30 minutes via the lifespan; they are gracefully cancelled on shutdown. +- **`serialize_finding()` helper** in `schemas.py` prevents `**f.data` field injection in API responses by extracting only known fields. +- **`SUPPORTED_ECOSYSTEMS`** constant in `constants.py` defines the accepted ecosystem set (`pypi`, `go`, `npm`). +- **`/_health/dependencies`** endpoint checks database connectivity and Nexus API reachability. --- diff --git a/README.en.md b/README.en.md index 4a37dc9..a8b8017 100644 --- a/README.en.md +++ b/README.en.md @@ -4,12 +4,13 @@ Integration of [GuardDog](https://github.com/DataDog/guarddog) (package vulnerab ## Features -- **Automatic scanning** via Nexus webhooks on package cache updates -- **Multi-ecosystem support** — PyPI, Go, npm (any format via proxy repositories) +- **Automatic scanning** via Nexus webhooks on package updates (`UPDATED` only) +- **Multi-ecosystem support** — PyPI, Go, npm (including scoped packages `@scope/name`); unknown ecosystems explicitly rejected - **REST API** for scan results, findings, statistics, and CSV export - **Web dashboard** with scan tables, filtering, and LLM-powered analysis -- **LLM analysis** — automated security analysis of each finding via OpenAI-compatible APIs (optional, configurable) +- **LLM analysis** — automated security analysis of each finding via OpenAI-compatible APIs (optional, configurable); parallel analysis via `asyncio.gather` - **Deduplication** by URL and SHA256 — identical content scanned only once +- **SSRF protection** — download URL validation via `NEXUS_ALLOWED_HOSTS` - **Structured JSON logging** with optional syslog output - **Docker Compose** — full stack deployment with Nexus in one command @@ -155,8 +156,18 @@ curl -X POST http://localhost:8080/webhooks/nexus \ | Method | Path | Description | |--------|------|-------------| | GET | `/health` | Health check | +| GET | `/health/dependencies` | DB and Nexus API connectivity check | | GET | `/metrics` | Prometheus-compatible metrics | +## Security + +- Webhooks support HMAC-SHA256 signature validation via `WEBHOOK_SECRET` +- Nexus client uses anonymous access (no BasicAuth) — ensure Nexus allows anonymous read access +- SSRF protection: download URLs validated against `NEXUS_ALLOWED_HOSTS` +- Security headers on all responses: `X-Content-Type-Options`, `X-Frame-Options`, `X-XSS-Protection`, `Referrer-Policy`, `Permissions-Policy` +- Scan results stored in local SQLite database +- Temporary package files deleted after scanning + ## LLM Analysis GuardDog Nexus can analyze findings through an LLM. When enabled (`LLM_ENABLED=1`), flagged findings receive an AI breakdown: threat assessment, code analysis, and recommendations. @@ -226,7 +237,7 @@ guarddog-nexus/ │ ├── i18n.py # RU/EN translations │ ├── logging_setup.py # JSON structured logging │ └── main.py # FastAPI app entry point -├── tests/ # pytest tests (85 tests) +├── tests/ # pytest tests (137 tests: 101 unit + 36 e2e) ├── scripts/ # Setup scripts ├── docker-compose.yml ├── Dockerfile diff --git a/README.md b/README.md index 9b84fec..12296bd 100644 --- a/README.md +++ b/README.md @@ -1,15 +1,16 @@ # GuardDog Nexus -Интеграция [GuardDog](https://github.com/DataDog GuardDog) (сканер уязвимостей пакетов PyPI) с [Sonatype Nexus Repository Manager]. Автоматически сканирует Python-пакеты, хранящиеся в Nexus, на наличие уязвимостей, вредоносного кода и подозрительных паттернов при каждом обновлении или добавлении пакета. +Интеграция [GuardDog](https://github.com/DataDog/guarddog) (сканер уязвимостей пакетов) с [Sonatype Nexus Repository Manager]. Автоматически сканирует пакеты (PyPI, Go, npm), хранящиеся в Nexus, на наличие уязвимостей, вредоносного кода и подозрительных паттернов при каждом обновлении пакета через вебхуки. ## Возможности -- **Автоматическое сканирование** по вебхукам Nexus при создании/обновлении пакетов -- **Поддержка нескольких экосистем** — PyPI, Go, npm (любой формат через прокси-репозитории Nexus) +- **Автоматическое сканирование** по вебхукам Nexus при обновлении пакетов (только `UPDATED`) +- **Поддержка нескольких экосистем** — PyPI, Go, npm (включая scoped-пакеты `@scope/name`); неизвестные экосистемы явно отклоняются - **REST API** для просмотра результатов сканирования, уязвимостей, статистики и экспорта в CSV - **Веб-интерфейс** с дашбордом, таблицами сканирований и фильтрацией по уязвимостям -- **LLM-анализ** — автоматический разбор каждой уязвимости через OpenAI-совместимые API (опционально, настраивается) +- **LLM-анализ** — автоматический разбор каждой уязвимости через OpenAI-совместимые API (опционально, настраивается); параллельный анализ через `asyncio.gather` - **Дедупликация** по URL и SHA256 — один и тот же пакет сканируется один раз +- **Защита от SSRF** — валидация URL загрузки через `NEXUS_ALLOWED_HOSTS` - **Структурированное логирование** в формате JSON с опциональной отправкой в syslog - **Docker Compose** для развёртывания приложения, Nexus и настройки в одном стеке @@ -186,6 +187,7 @@ curl -X POST http://localhost:8080/webhooks/nexus \ | Метод | Путь | Описание | |-------|------|----------| | GET | `/health` | Проверка работоспособности | +| GET | `/health/dependencies` | Проверка БД и доступности Nexus API | | GET | `/metrics` | Метрики в формате Prometheus | ## Веб-интерфейс @@ -252,7 +254,9 @@ guarddog-nexus/ ## Безопасность - Вебхуки поддерживают HMAC-SHA256 подпись через `WEBHOOK_SECRET` -- Nexus-клиент использует BasicAuth для аутентификации +- Nexus-клиент использует анонимный доступ (без BasicAuth) — убедитесь, что Nexus разрешает анонимное чтение репозиториев +- Защита от SSRF: URL загрузки валидируется через `NEXUS_ALLOWED_HOSTS` +- Заголовки безопасности на всех ответах: `X-Content-Type-Options`, `X-Frame-Options`, `X-XSS-Protection`, `Referrer-Policy`, `Permissions-Policy` - Результаты сканирования хранятся в локальной SQLite-базе - Временные файлы пакетов удаляются после сканирования diff --git a/guarddog_nexus/core/harvester.py b/guarddog_nexus/core/harvester.py index 3adb64b..15360b4 100644 --- a/guarddog_nexus/core/harvester.py +++ b/guarddog_nexus/core/harvester.py @@ -76,21 +76,24 @@ async def harvest( _url_locks.pop(download_url, None) return None + active_found = False async with lock: - try: - # Re-check DB in case another task already created and finished a scan - active = await session.scalar( - select(Scan.id).where( - Scan.nexus_asset_url == download_url, - Scan.status.in_([ScanStatus.PENDING.value, ScanStatus.SCANNING.value]), - ) + # Re-check DB in case another task already created and finished a scan + active = await session.scalar( + select(Scan.id).where( + Scan.nexus_asset_url == download_url, + Scan.status.in_([ScanStatus.PENDING.value, ScanStatus.SCANNING.value]), ) - if active: - log.info("Already scanning this URL, skipping") - return None - finally: - async with _url_lock: - _url_locks.pop(download_url, None) + ) + if active: + log.info("Already scanning this URL, skipping") + active_found = True + + async with _url_lock: + _url_locks.pop(download_url, None) + + if active_found: + return None scan = Scan( package_name=package_name, diff --git a/guarddog_nexus/routes/api_packages.py b/guarddog_nexus/routes/api_packages.py index 791d37b..7893489 100644 --- a/guarddog_nexus/routes/api_packages.py +++ b/guarddog_nexus/routes/api_packages.py @@ -21,6 +21,7 @@ from ..db.engine import get_session from ..db.models import Scan from ..db.queries import build_package_list_query from ..schemas import PackageDetailOut, PackageListResponse, serialize_finding +from .api_scans import _csv_safe router = APIRouter(prefix="/api/v1/packages", tags=["packages"]) @@ -102,8 +103,8 @@ async def export_packages_csv( for r in rows: writer.writerow( [ - r.pkg_name, - r.pkg_ver, + _csv_safe(r.pkg_name), + _csv_safe(r.pkg_ver), r.ecosystem, r.repository, r.last_scan.isoformat() if r.last_scan else "", diff --git a/guarddog_nexus/routes/api_scans.py b/guarddog_nexus/routes/api_scans.py index 7830ab9..7f05e18 100644 --- a/guarddog_nexus/routes/api_scans.py +++ b/guarddog_nexus/routes/api_scans.py @@ -21,6 +21,13 @@ from ..db.models import Scan from ..db.queries import build_scan_list_query, get_dashboard_stats from ..schemas import ScanDetailOut, ScanListResponse, StatsResponse, serialize_finding + +def _csv_safe(value: str) -> str: + if value and value[0] in "=+-@": + return "'" + value + return value + + router = APIRouter(prefix="/api/v1/scans", tags=["scans"]) @@ -112,8 +119,8 @@ async def export_scans_csv( writer.writerow( [ s.id, - s.package_name, - s.package_version, + _csv_safe(s.package_name), + _csv_safe(s.package_version), s.ecosystem, s.repository, s.status, @@ -121,7 +128,7 @@ async def export_scans_csv( s.flagged, s.started_at.isoformat() if s.started_at else "", s.finished_at.isoformat() if s.finished_at else "", - s.error_message or "", + _csv_safe(s.error_message or ""), s.sha256 or "", ] ) diff --git a/guarddog_nexus/routes/web.py b/guarddog_nexus/routes/web.py index 44dc25a..5e027e6 100644 --- a/guarddog_nexus/routes/web.py +++ b/guarddog_nexus/routes/web.py @@ -262,13 +262,12 @@ async def analyze_finding_htmx( return _render("_llm_spinner.html", request=request, finding_id=finding_id) async with lock: - try: - finding.report = {"status": "analyzing"} - await session.commit() - report = await analyze_finding(finding.data) - finally: - async with _llm_lock: - _llm_locks.pop(finding_id, None) + finding.report = {"status": "analyzing"} + await session.commit() + report = await analyze_finding(finding.data) + + async with _llm_lock: + _llm_locks.pop(finding_id, None) if report is None: finding.report = None diff --git a/guarddog_nexus/schemas.py b/guarddog_nexus/schemas.py index 0f2d32d..597fbec 100644 --- a/guarddog_nexus/schemas.py +++ b/guarddog_nexus/schemas.py @@ -139,5 +139,5 @@ def serialize_finding(finding) -> dict: "created_at": finding.created_at.isoformat() if finding.created_at else None, } for field in _FINDING_DATA_FIELDS: - result[field] = finding.data.get(field, "") + result[field] = finding.data.get(field) or "" return result