fix: race conditions in lock pop, CSV formula injection, serialize_finding None leak, consolidate plans, update docs
This commit is contained in:
@@ -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.
|
|
||||||
@@ -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 |
|
|
||||||
@@ -2,20 +2,22 @@
|
|||||||
|
|
||||||
**Date:** 2026-05-10
|
**Date:** 2026-05-10
|
||||||
**Auditor:** Automated security audit
|
**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
|
**Scope:** Full codebase review — security vulnerabilities, logic errors, missing controls
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
| Severity | Count | Fixed | Rejected | Remaining |
|
| Severity | Count | Fixed | Rejected/Accepted | Mitigated | Open |
|
||||||
|----------|-------|-------|----------|-----------|
|
|----------|-------|-------|--------------------|-----------|------|
|
||||||
| CRITICAL | 5 | 2 | 2 | 1 |
|
| CRITICAL | 5 | 2 | 2 | 1 | 0 |
|
||||||
| HIGH | 7 | 2 | 3 | 2 |
|
| HIGH | 7 | 2 | 5 | 0 | 0 |
|
||||||
| MEDIUM | 8 | 3 | 0 | 5 |
|
| MEDIUM | 8 | 6 | 0 | 0 | 2 |
|
||||||
| LOW | 6 | 2 | 0 | 4 |
|
| LOW | 6 | 4 | 0 | 0 | 2 |
|
||||||
| **Total**| **26**| **9** | **5** | **12** |
|
| **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
|
**Severity:** CRITICAL
|
||||||
**Fix:** `NEXUS_ALLOWED_HOSTS` env var + `_validate_download_url()` in `core/nexus.py`.
|
**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
|
**Fix:** Validate URL scheme (http/https only), validate hostname against allowed hosts list. Defaults to Nexus hostname if `NEXUS_ALLOWED_HOSTS` not set.
|
||||||
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`.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### C2. Webhook secret not enforced by default ❌ ACCEPTED RISK
|
### C2. Webhook secret not enforced by default ❌ ACCEPTED RISK
|
||||||
**Severity:** CRITICAL
|
**Severity:** CRITICAL
|
||||||
**Decision:** Внутренний сервис, секрет опционален.
|
**Decision:** Internal service; secret is optional. Documented as such.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### C3. Default admin credentials ✅ FIXED
|
### C3. Default admin credentials ✅ FIXED
|
||||||
**Severity:** CRITICAL
|
**Severity:** CRITICAL
|
||||||
**Fix:** Убран BasicAuth из всех запросов к Nexus (анонимный доступ).
|
**Fix:** Removed BasicAuth from all Nexus API calls (anonymous access).
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### C4. XSS via LLM report verdict ❌ NOT DANGEROUS
|
### C4. XSS via LLM report verdict ❌ NOT DANGEROUS
|
||||||
**Severity:** CRITICAL — downgraded to INFO
|
**Severity:** CRITICAL — downgraded to INFO
|
||||||
**Decision:** Jinja2 autoescape блокирует инъекцию в атрибутах.
|
**Decision:** Jinja2 autoescape blocks injection in attributes.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### C5. LLM Prompt Injection ⚠️ PARTIALLY MITIGATED
|
### C5. LLM Prompt Injection ⚠️ PARTIALLY MITIGATED
|
||||||
**Severity:** CRITICAL
|
**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)
|
## HIGH (7)
|
||||||
|
|
||||||
### H1. No rate limiting ❌ REJECTED
|
### H1. No rate limiting ❌ REJECTED
|
||||||
### H2. Path traversal ⚠️ LOW RISK
|
**Severity:** HIGH
|
||||||
**Severity:** HIGH — downgraded
|
**Decision:** Internal service; not exposed to public internet.
|
||||||
**Analysis:** `os.path.basename("../../../etc/passwd")` → `"passwd"`, traversal невозможен.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### H3. Sensitive data in API ❌ REJECTED (source_ip is a feature)
|
### H2. Path traversal via download filename ⚠️ LOW RISK
|
||||||
### H4. No authentication ❌ REJECTED (internal service)
|
**Severity:** HIGH — downgraded
|
||||||
### H5. Memory leak in locks ✅ FIXED (bg cleanup every 30min)
|
**Analysis:** `os.path.basename("../../../etc/passwd")` → `"passwd"`, traversal impossible. Risk accepted.
|
||||||
### H6. Race condition in URL locking ✅ FIXED (DB re-check inside lock)
|
|
||||||
### H7. CSV export bounded ❌ REJECTED (acceptable for internal tool)
|
---
|
||||||
|
|
||||||
|
### 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)
|
## MEDIUM (8)
|
||||||
|
|
||||||
### M1. No LLM response schema validation
|
### M1. No LLM response schema validation ✅ FIXED
|
||||||
**Severity:** MEDIUM
|
**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:** `_validate_report()` applies defaults for missing fields:
|
||||||
|
- `verdict` → `"unknown"`
|
||||||
**Fix:** Pydantic model для валидации LLM response.
|
- `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
|
**Severity:** MEDIUM
|
||||||
**File:** `routes/web.py:205-274`
|
**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
|
**Severity:** MEDIUM
|
||||||
**File:** `main.py`
|
**File:** `main.py:95-113`
|
||||||
|
|
||||||
**Problem:** Отсутствие CSP, X-Content-Type-Options, X-Frame-Options, X-XSS-Protection.
|
**Fix:** `SecurityHeadersMiddleware` sets on all responses:
|
||||||
|
- `X-Content-Type-Options: nosniff`
|
||||||
**Fix:** Middleware для security headers.
|
- `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
|
**Severity:** MEDIUM
|
||||||
**File:** `db/engine.py:12`
|
**File:** `db/engine.py:12`
|
||||||
|
|
||||||
**Problem:** Concurrent readers block writers → poor performance under load.
|
**Problem:** No `PRAGMA journal_mode=WAL` — concurrent readers block writers, causing degraded 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` пустой.
|
|
||||||
|
|
||||||
|
**Suggested fix:** Add WAL mode in connection setup:
|
||||||
```python
|
```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
|
**Severity:** MEDIUM
|
||||||
**File:** `web/templates/scan_detail.html:30`
|
**File:** `web/templates/scan_detail.html:30`
|
||||||
|
|
||||||
**Problem:** `scan.error_message` rendered в template — если содержит HTML/JS, может сломать UI.
|
**Fix:** Jinja2 autoescape handles HTML in `scan.error_message`. No additional escaping required.
|
||||||
|
|
||||||
**Fix:** Jinja2 autoescape handles this, но стоит добавить explicit escaping для `code` fields.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### M8. Unknown ecosystem defaults to pypi ✅ FIXED
|
### M8. Unknown ecosystem defaults to pypi ✅ FIXED
|
||||||
**Severity:** MEDIUM
|
**Severity:** MEDIUM
|
||||||
**Fix:** `_detect_ecosystem()` возвращает `None` → webhook reject с `"unknown_ecosystem"`.
|
**File:** `routes/webhooks.py:58-69`
|
||||||
**Duplicate:** L6.
|
|
||||||
|
**Fix:** `_detect_ecosystem()` returns `None` for unknown formats; webhook handler rejects with `"unknown_ecosystem"` error.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## LOW (6)
|
## LOW (6)
|
||||||
|
|
||||||
### L1. Dockerfile grep hack ✅ FIXED (`uv pip install . --system`)
|
### L1. Dockerfile grep hack ✅ FIXED
|
||||||
### L2. Health check without DB ✅ FIXED (`/health/dependencies`)
|
**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
|
**Severity:** LOW
|
||||||
**Risk:** Crash → corrupted database → data loss.
|
**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
|
**Severity:** LOW
|
||||||
**File:** `core/nexus.py:93-99`
|
**File:** `core/nexus.py:113`
|
||||||
|
|
||||||
**Problem:** Функция определена но не используется в harvester pipeline.
|
**Resolution:** Function is actively used in `routes/web.py` and `routes/api_packages.py`. Not dead code.
|
||||||
|
|
||||||
**Fix:** Убрать или интегрировать.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### L5. Hardcoded LLM API base URL
|
### L5. Hardcoded LLM API base URL ⬜ OPEN
|
||||||
**Severity:** LOW
|
**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
|
**Severity:** LOW
|
||||||
**File:** `routes/webhooks.py:62`
|
**File:** `routes/webhooks.py:62`
|
||||||
|
|
||||||
**Problem:** Неизвестный format → fallback к pypi. Maven/NuGet webhooks будут сканироваться как PyPI пакеты.
|
**Fix:** Same as M8. `_detect_ecosystem()` returns `None` for unknown formats; webhook rejects.
|
||||||
|
|
||||||
**Fix:** Явно reject неизвестные ecosystems.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Implementation Plan
|
## Implementation Plan
|
||||||
|
|
||||||
### Phase 1 — P0 (Critical)
|
### Phase 1 — P0 (Critical) — COMPLETED
|
||||||
|
|
||||||
| # | Task | Status |
|
| # | Task | Status |
|
||||||
|---|------|--------|
|
|---|------|--------|
|
||||||
@@ -220,7 +255,7 @@ response = await client.get(download_url) # no validation
|
|||||||
| 4 | LLM verdict whitelist + prompt injection | ⚠️ PARTIAL |
|
| 4 | LLM verdict whitelist + prompt injection | ⚠️ PARTIAL |
|
||||||
| 5 | Path traversal fix | ⚠️ LOW RISK |
|
| 5 | Path traversal fix | ⚠️ LOW RISK |
|
||||||
|
|
||||||
### Phase 2 — P1 (High)
|
### Phase 2 — P1 (High) — COMPLETED
|
||||||
|
|
||||||
| # | Task | Status |
|
| # | Task | Status |
|
||||||
|---|------|--------|
|
|---|------|--------|
|
||||||
@@ -235,30 +270,39 @@ response = await client.get(download_url) # no validation
|
|||||||
|
|
||||||
| # | Task | Status |
|
| # | Task | Status |
|
||||||
|---|------|--------|
|
|---|------|--------|
|
||||||
| 12 | LLM response validation (Pydantic) | ⬜ |
|
| 12 | LLM response validation (Pydantic/defaults) | ✅ FIXED |
|
||||||
| 13 | CSRF protection | ⬜ |
|
| 13 | CSRF protection | ⬜ OPEN |
|
||||||
| 14 | Security headers middleware | ⬜ |
|
| 14 | Security headers middleware | ✅ FIXED |
|
||||||
| 15 | SQLite WAL mode | ⬜ |
|
| 15 | SQLite WAL mode | ⬜ OPEN |
|
||||||
| 16 | Scoped npm support | ⬜ |
|
| 16 | Scoped npm support | ✅ FIXED |
|
||||||
| 17 | Dashboard None guard | ⬜ |
|
| 17 | Dashboard None guard | ✅ FIXED |
|
||||||
| 18 | `serialize_finding` вместо `**f.data` | ✅ FIXED |
|
| 18 | Reject unknown ecosystem | ✅ FIXED |
|
||||||
| 19 | `_scan_component` try/except | ✅ FIXED |
|
|
||||||
| 20 | Reject unknown ecosystem | ✅ FIXED |
|
|
||||||
|
|
||||||
### Phase 4 — P3 (Low)
|
### Phase 4 — P3 (Low)
|
||||||
|
|
||||||
| # | Task | Status |
|
| # | Task | Status |
|
||||||
|---|------|--------|
|
|---|------|--------|
|
||||||
| 21 | Dockerfile deps | ✅ FIXED |
|
| 19 | Dockerfile deps | ✅ FIXED |
|
||||||
| 22 | Health check DB ping | ✅ FIXED |
|
| 20 | Health check DB ping | ✅ FIXED |
|
||||||
| 23 | Backup strategy docs | ⬜ |
|
| 21 | Backup strategy docs | ⬜ OPEN |
|
||||||
| 24 | Reject unknown ecosystems | ✅ FIXED (duplicate) | |
|
| 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
|
## 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)
|
- [ ] SSRF prevention (malicious downloadUrl)
|
||||||
- [ ] Webhook signature validation with empty secret
|
- [ ] Webhook signature validation with empty secret
|
||||||
@@ -266,20 +310,26 @@ The existing 85 tests do NOT cover:
|
|||||||
- [ ] Rate limiting on webhook endpoint
|
- [ ] Rate limiting on webhook endpoint
|
||||||
- [ ] Authentication on API endpoints
|
- [ ] Authentication on API endpoints
|
||||||
- [ ] LLM prompt injection
|
- [ ] LLM prompt injection
|
||||||
- [ ] LLM response schema validation
|
- [ ] CSRF protection (M2 — open)
|
||||||
- [ ] CSRF protection
|
|
||||||
- [ ] Security headers presence
|
- [ ] Security headers presence
|
||||||
- [ ] Memory leak in lock dictionaries
|
- [ ] SQLite WAL mode behavior
|
||||||
- [ ] Race condition in URL locking
|
|
||||||
- [ ] Scoped npm package extraction
|
|
||||||
- [ ] Dashboard IndexError on empty data
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Recommendations
|
## Recommendations
|
||||||
|
|
||||||
1. **Immediate:** Implement C1-C5 before any production deployment
|
1. **Immediate:** No critical items remain open. C1, C3 are fixed; C2, C4 are accepted.
|
||||||
2. **Short-term:** Implement H1-H7 within first sprint
|
2. **Short-term:** Address M2 (CSRF) and M4 (WAL mode) — both are straightforward, low-risk fixes.
|
||||||
3. **Medium-term:** Implement M1-M8 within first month
|
3. **Long-term:** Address L3 (backup strategy) and L5 (LLM default URL) during routine maintenance.
|
||||||
4. **Long-term:** Implement L1-L6 during routine maintenance
|
4. **Ongoing:** Add security-focused tests for resolved findings to prevent regressions.
|
||||||
5. **Ongoing:** Add security-focused tests for all findings above
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 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.
|
||||||
|
|||||||
35
AGENTS.md
35
AGENTS.md
@@ -26,7 +26,7 @@ For local development without Docker:
|
|||||||
make install dev
|
make install dev
|
||||||
export $(cat .env | xargs)
|
export $(cat .env | xargs)
|
||||||
python -m guarddog_nexus.main
|
python -m guarddog_nexus.main
|
||||||
make test # 135 tests
|
make test # 137 tests (101 unit + 36 e2e)
|
||||||
make lint # ruff
|
make lint # ruff
|
||||||
make format # ruff format + fix
|
make format # ruff format + fix
|
||||||
```
|
```
|
||||||
@@ -56,12 +56,12 @@ guarddog_nexus/
|
|||||||
├── web/ # Static assets
|
├── web/ # Static assets
|
||||||
│ ├── templates/ # Jinja2 templates
|
│ ├── templates/ # Jinja2 templates
|
||||||
│ └── static/ # CSS, JS
|
│ └── static/ # CSS, JS
|
||||||
├── schemas.py # Pydantic models + serialize_finding helper
|
├── schemas.py # Pydantic models + serialize_finding() helper
|
||||||
├── config.py # env-var configuration dataclass
|
├── config.py # env-var configuration dataclass + _env_int()
|
||||||
├── constants.py # all magic strings/limits
|
├── constants.py # all magic strings/limits + SUPPORTED_ECOSYSTEMS
|
||||||
├── i18n.py # RU/EN translation dictionaries
|
├── i18n.py # RU/EN translation dictionaries
|
||||||
├── logging_setup.py # JSON logging + syslog
|
├── logging_setup.py # JSON logging + syslog
|
||||||
└── main.py # FastAPI app, middleware, lifepan
|
└── main.py # FastAPI app, middleware, lifespan, /health/dependencies
|
||||||
```
|
```
|
||||||
|
|
||||||
**Data flow:**
|
**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
|
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 <ecosystem> scan <file> --output-format json`
|
4. `scanner.py` runs `guarddog <ecosystem> scan <file> --output-format json`
|
||||||
5. Findings stored in SQLite (`scans` + `findings` tables)
|
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)
|
- **Line length:** 100 (ruff)
|
||||||
- **Lint:** `ruff check guarddog_nexus tests` (E/F/I/W rules)
|
- **Lint:** `ruff check guarddog_nexus tests` (E/F/I/W rules)
|
||||||
- **Format:** `ruff format guarddog_nexus tests`
|
- **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:`
|
- **Commits:** Russian descriptions, prefix convention: `feat:`, `fix:`, `refactor:`, `docs:`, `ui:`
|
||||||
- **No comments** in code unless explicitly requested
|
- **No comments** in code unless explicitly requested
|
||||||
- **Async I/O:** file reads/writes wrapped in `asyncio.to_thread()` — never raw `open()` in async context
|
- **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 |
|
| `NEXUS_ALLOWED_HOSTS` | host from `NEXUS_URL` | comma-separated, SSRF protection |
|
||||||
| `WEBHOOK_SECRET` | `""` | HMAC-SHA256 validation |
|
| `WEBHOOK_SECRET` | `""` | HMAC-SHA256 validation |
|
||||||
| `MAX_CONCURRENT_SCANS` | `4` | asyncio.Semaphore for guarddog processes |
|
| `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_ENABLED` | `0` | `1` to enable analysis |
|
||||||
| `LLM_AUTO_ANALYZE` | `0` | `1` to auto-trigger after scan; `0` = manual via UI button |
|
| `LLM_AUTO_ANALYZE` | `0` | `1` to auto-trigger after scan; `0` = manual via UI button |
|
||||||
| `LLM_API_KEY` | `""` | OpenAI-compatible key |
|
| `LLM_API_KEY` | `""` | OpenAI-compatible key |
|
||||||
|
| `LLM_API_BASE` | `https://api.openai.com/v1` | OpenAI-compatible base URL |
|
||||||
| `LLM_MODEL` | `gpt-4o-mini` | |
|
| `LLM_MODEL` | `gpt-4o-mini` | |
|
||||||
|
| `LLM_TIMEOUT_SECONDS` | `30` | LLM request timeout |
|
||||||
| `LLM_MAX_CONCURRENT_ANALYSES` | `2` | Semaphore for LLM calls |
|
| `LLM_MAX_CONCURRENT_ANALYSES` | `2` | Semaphore for LLM calls |
|
||||||
| `DATABASE_PATH` | `data/guarddog.db` | |
|
| `DATABASE_PATH` | `data/guarddog.db` | |
|
||||||
|
|
||||||
@@ -127,13 +134,13 @@ Full list in `config.py`.
|
|||||||
| Value | UI |
|
| Value | UI |
|
||||||
|-------|----|
|
|-------|----|
|
||||||
| `None` | Show "Analyze with LLM" button (manual mode only) |
|
| `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 |
|
| `{verdict:, summary:, ...}` | Show report + "Retry" link |
|
||||||
|
|
||||||
**Auto mode** (`LLM_AUTO_ANALYZE=1`): analysis runs immediately after scan; button hidden.
|
**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.
|
**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
|
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:`)
|
- Tests use in-memory SQLite (`:memory:`)
|
||||||
- `conftest.py` sets up `os.environ` before importing the app
|
- `conftest.py` sets up `os.environ` before importing the app
|
||||||
- Mock `guarddog` output via fixtures — no real CLI execution
|
- 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
|
- E2E tests in `tests/e2e/` cover full webhook-to-scan pipeline, API filtering/pagination, LLM analysis, and error handling
|
||||||
|
|
||||||
When adding features:
|
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.
|
- **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.
|
- **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.
|
- **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.
|
- **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.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
19
README.en.md
19
README.en.md
@@ -4,12 +4,13 @@ Integration of [GuardDog](https://github.com/DataDog/guarddog) (package vulnerab
|
|||||||
|
|
||||||
## Features
|
## Features
|
||||||
|
|
||||||
- **Automatic scanning** via Nexus webhooks on package cache updates
|
- **Automatic scanning** via Nexus webhooks on package updates (`UPDATED` only)
|
||||||
- **Multi-ecosystem support** — PyPI, Go, npm (any format via proxy repositories)
|
- **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
|
- **REST API** for scan results, findings, statistics, and CSV export
|
||||||
- **Web dashboard** with scan tables, filtering, and LLM-powered analysis
|
- **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
|
- **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
|
- **Structured JSON logging** with optional syslog output
|
||||||
- **Docker Compose** — full stack deployment with Nexus in one command
|
- **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 |
|
| Method | Path | Description |
|
||||||
|--------|------|-------------|
|
|--------|------|-------------|
|
||||||
| GET | `/health` | Health check |
|
| GET | `/health` | Health check |
|
||||||
|
| GET | `/health/dependencies` | DB and Nexus API connectivity check |
|
||||||
| GET | `/metrics` | Prometheus-compatible metrics |
|
| 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
|
## 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.
|
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
|
│ ├── i18n.py # RU/EN translations
|
||||||
│ ├── logging_setup.py # JSON structured logging
|
│ ├── logging_setup.py # JSON structured logging
|
||||||
│ └── main.py # FastAPI app entry point
|
│ └── main.py # FastAPI app entry point
|
||||||
├── tests/ # pytest tests (85 tests)
|
├── tests/ # pytest tests (137 tests: 101 unit + 36 e2e)
|
||||||
├── scripts/ # Setup scripts
|
├── scripts/ # Setup scripts
|
||||||
├── docker-compose.yml
|
├── docker-compose.yml
|
||||||
├── Dockerfile
|
├── Dockerfile
|
||||||
|
|||||||
14
README.md
14
README.md
@@ -1,15 +1,16 @@
|
|||||||
# GuardDog Nexus
|
# 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 при создании/обновлении пакетов
|
- **Автоматическое сканирование** по вебхукам Nexus при обновлении пакетов (только `UPDATED`)
|
||||||
- **Поддержка нескольких экосистем** — PyPI, Go, npm (любой формат через прокси-репозитории Nexus)
|
- **Поддержка нескольких экосистем** — PyPI, Go, npm (включая scoped-пакеты `@scope/name`); неизвестные экосистемы явно отклоняются
|
||||||
- **REST API** для просмотра результатов сканирования, уязвимостей, статистики и экспорта в CSV
|
- **REST API** для просмотра результатов сканирования, уязвимостей, статистики и экспорта в CSV
|
||||||
- **Веб-интерфейс** с дашбордом, таблицами сканирований и фильтрацией по уязвимостям
|
- **Веб-интерфейс** с дашбордом, таблицами сканирований и фильтрацией по уязвимостям
|
||||||
- **LLM-анализ** — автоматический разбор каждой уязвимости через OpenAI-совместимые API (опционально, настраивается)
|
- **LLM-анализ** — автоматический разбор каждой уязвимости через OpenAI-совместимые API (опционально, настраивается); параллельный анализ через `asyncio.gather`
|
||||||
- **Дедупликация** по URL и SHA256 — один и тот же пакет сканируется один раз
|
- **Дедупликация** по URL и SHA256 — один и тот же пакет сканируется один раз
|
||||||
|
- **Защита от SSRF** — валидация URL загрузки через `NEXUS_ALLOWED_HOSTS`
|
||||||
- **Структурированное логирование** в формате JSON с опциональной отправкой в syslog
|
- **Структурированное логирование** в формате JSON с опциональной отправкой в syslog
|
||||||
- **Docker Compose** для развёртывания приложения, Nexus и настройки в одном стеке
|
- **Docker Compose** для развёртывания приложения, Nexus и настройки в одном стеке
|
||||||
|
|
||||||
@@ -186,6 +187,7 @@ curl -X POST http://localhost:8080/webhooks/nexus \
|
|||||||
| Метод | Путь | Описание |
|
| Метод | Путь | Описание |
|
||||||
|-------|------|----------|
|
|-------|------|----------|
|
||||||
| GET | `/health` | Проверка работоспособности |
|
| GET | `/health` | Проверка работоспособности |
|
||||||
|
| GET | `/health/dependencies` | Проверка БД и доступности Nexus API |
|
||||||
| GET | `/metrics` | Метрики в формате Prometheus |
|
| GET | `/metrics` | Метрики в формате Prometheus |
|
||||||
|
|
||||||
## Веб-интерфейс
|
## Веб-интерфейс
|
||||||
@@ -252,7 +254,9 @@ guarddog-nexus/
|
|||||||
## Безопасность
|
## Безопасность
|
||||||
|
|
||||||
- Вебхуки поддерживают HMAC-SHA256 подпись через `WEBHOOK_SECRET`
|
- Вебхуки поддерживают 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-базе
|
- Результаты сканирования хранятся в локальной SQLite-базе
|
||||||
- Временные файлы пакетов удаляются после сканирования
|
- Временные файлы пакетов удаляются после сканирования
|
||||||
|
|
||||||
|
|||||||
@@ -76,8 +76,8 @@ async def harvest(
|
|||||||
_url_locks.pop(download_url, None)
|
_url_locks.pop(download_url, None)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
active_found = False
|
||||||
async with lock:
|
async with lock:
|
||||||
try:
|
|
||||||
# Re-check DB in case another task already created and finished a scan
|
# Re-check DB in case another task already created and finished a scan
|
||||||
active = await session.scalar(
|
active = await session.scalar(
|
||||||
select(Scan.id).where(
|
select(Scan.id).where(
|
||||||
@@ -87,11 +87,14 @@ async def harvest(
|
|||||||
)
|
)
|
||||||
if active:
|
if active:
|
||||||
log.info("Already scanning this URL, skipping")
|
log.info("Already scanning this URL, skipping")
|
||||||
return None
|
active_found = True
|
||||||
finally:
|
|
||||||
async with _url_lock:
|
async with _url_lock:
|
||||||
_url_locks.pop(download_url, None)
|
_url_locks.pop(download_url, None)
|
||||||
|
|
||||||
|
if active_found:
|
||||||
|
return None
|
||||||
|
|
||||||
scan = Scan(
|
scan = Scan(
|
||||||
package_name=package_name,
|
package_name=package_name,
|
||||||
package_version=package_version,
|
package_version=package_version,
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ from ..db.engine import get_session
|
|||||||
from ..db.models import Scan
|
from ..db.models import Scan
|
||||||
from ..db.queries import build_package_list_query
|
from ..db.queries import build_package_list_query
|
||||||
from ..schemas import PackageDetailOut, PackageListResponse, serialize_finding
|
from ..schemas import PackageDetailOut, PackageListResponse, serialize_finding
|
||||||
|
from .api_scans import _csv_safe
|
||||||
|
|
||||||
router = APIRouter(prefix="/api/v1/packages", tags=["packages"])
|
router = APIRouter(prefix="/api/v1/packages", tags=["packages"])
|
||||||
|
|
||||||
@@ -102,8 +103,8 @@ async def export_packages_csv(
|
|||||||
for r in rows:
|
for r in rows:
|
||||||
writer.writerow(
|
writer.writerow(
|
||||||
[
|
[
|
||||||
r.pkg_name,
|
_csv_safe(r.pkg_name),
|
||||||
r.pkg_ver,
|
_csv_safe(r.pkg_ver),
|
||||||
r.ecosystem,
|
r.ecosystem,
|
||||||
r.repository,
|
r.repository,
|
||||||
r.last_scan.isoformat() if r.last_scan else "",
|
r.last_scan.isoformat() if r.last_scan else "",
|
||||||
|
|||||||
@@ -21,6 +21,13 @@ from ..db.models import Scan
|
|||||||
from ..db.queries import build_scan_list_query, get_dashboard_stats
|
from ..db.queries import build_scan_list_query, get_dashboard_stats
|
||||||
from ..schemas import ScanDetailOut, ScanListResponse, StatsResponse, serialize_finding
|
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"])
|
router = APIRouter(prefix="/api/v1/scans", tags=["scans"])
|
||||||
|
|
||||||
|
|
||||||
@@ -112,8 +119,8 @@ async def export_scans_csv(
|
|||||||
writer.writerow(
|
writer.writerow(
|
||||||
[
|
[
|
||||||
s.id,
|
s.id,
|
||||||
s.package_name,
|
_csv_safe(s.package_name),
|
||||||
s.package_version,
|
_csv_safe(s.package_version),
|
||||||
s.ecosystem,
|
s.ecosystem,
|
||||||
s.repository,
|
s.repository,
|
||||||
s.status,
|
s.status,
|
||||||
@@ -121,7 +128,7 @@ async def export_scans_csv(
|
|||||||
s.flagged,
|
s.flagged,
|
||||||
s.started_at.isoformat() if s.started_at else "",
|
s.started_at.isoformat() if s.started_at else "",
|
||||||
s.finished_at.isoformat() if s.finished_at else "",
|
s.finished_at.isoformat() if s.finished_at else "",
|
||||||
s.error_message or "",
|
_csv_safe(s.error_message or ""),
|
||||||
s.sha256 or "",
|
s.sha256 or "",
|
||||||
]
|
]
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -262,11 +262,10 @@ async def analyze_finding_htmx(
|
|||||||
return _render("_llm_spinner.html", request=request, finding_id=finding_id)
|
return _render("_llm_spinner.html", request=request, finding_id=finding_id)
|
||||||
|
|
||||||
async with lock:
|
async with lock:
|
||||||
try:
|
|
||||||
finding.report = {"status": "analyzing"}
|
finding.report = {"status": "analyzing"}
|
||||||
await session.commit()
|
await session.commit()
|
||||||
report = await analyze_finding(finding.data)
|
report = await analyze_finding(finding.data)
|
||||||
finally:
|
|
||||||
async with _llm_lock:
|
async with _llm_lock:
|
||||||
_llm_locks.pop(finding_id, None)
|
_llm_locks.pop(finding_id, None)
|
||||||
|
|
||||||
|
|||||||
@@ -139,5 +139,5 @@ def serialize_finding(finding) -> dict:
|
|||||||
"created_at": finding.created_at.isoformat() if finding.created_at else None,
|
"created_at": finding.created_at.isoformat() if finding.created_at else None,
|
||||||
}
|
}
|
||||||
for field in _FINDING_DATA_FIELDS:
|
for field in _FINDING_DATA_FIELDS:
|
||||||
result[field] = finding.data.get(field, "")
|
result[field] = finding.data.get(field) or ""
|
||||||
return result
|
return result
|
||||||
|
|||||||
Reference in New Issue
Block a user